Skip to content

Skip the orphan prompt when omarchy update runs with -y - #8894

Open
Chessing234 wants to merge 2 commits into
omacom:quattrofrom
Chessing234:fix/omarchy-update-orphan-unattended
Open

Skip the orphan prompt when omarchy update runs with -y#8894
Chessing234 wants to merge 2 commits into
omacom:quattrofrom
Chessing234:fix/omarchy-update-orphan-unattended

Conversation

@Chessing234

@Chessing234 Chessing234 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • omarchy update -y sets OMARCHY_UPDATE_UNATTENDED but still has a TTY, so the orphan step waited on gum confirm forever after the upgrade finished.
  • Treat the unattended flag like a non-interactive pipe: report orphans and move on.

Fixes #8780.

Test plan

  • bash test/shell.d/update-orphan-test.sh (bash 5)
  • Run omarchy update -y with orphans present; confirm it exits without prompting

-y sets OMARCHY_UPDATE_UNATTENDED but still has a TTY, so the orphan
step waited on gum confirm forever after the upgrade finished.
Copilot AI balanced review requested due to automatic review settings August 29, 2026 06:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prevents unattended updates from blocking on orphan-package confirmation.

Changes:

  • Skips orphan prompts when OMARCHY_UPDATE_UNATTENDED=1.
  • Adds regression coverage for unattended execution.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File Description
bin/omarchy-update-orphan-pkgs Treats unattended updates as non-interactive.
test/shell.d/update-orphan-test.sh Adds an unattended-mode test.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

# has to be checked explicitly or gum confirm blocks forever.
write_stub pacman 'if [[ $1 == "-Qtdq" ]]; then printf "old-lib\n"; exit 0; fi; exit 1'
write_stub gum 'echo "gum should not be called under -y" >&2; exit 99'
OMARCHY_UPDATE_UNATTENDED=1 run_orphan_checker >"$test_tmp/unattended.out" 2>"$test_tmp/unattended.err"
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed on the merits, with a second opinion from codex at xhigh reasoning. The production change is correct; the test that ships with it does not guard it.

The fix itself is right. bin/omarchy-update-orphan-pkgs:17 adds ${OMARCHY_UPDATE_UNATTENDED:-} == 1 to the existing guard, which is the same idiom bin/omarchy-update-system-pkgs-when-conflicted:74 already uses for the same class of problem. The variable is genuinely supplied rather than invented: bin/omarchy-update:26 exports it on -y, the arguments survive both the script re-exec and the lock re-exec, and the orphan step is called at bin/omarchy-update:52 and inherits it. Skipping the prompt takes the report-and-exit branch, so orphans are left alone rather than removed unattended — the safe direction, and the one that matters most here. The interactive path is untouched: without -y the flag is unset, both descriptors are terminals, and gum confirm still runs.

The new test passes against the unfixed code. test/shell.d/update-orphan-test.sh:47 invokes the checker as OMARCHY_UPDATE_UNATTENDED=1 run_orphan_checker >"$test_tmp/unattended.out" 2>"$test_tmp/unattended.err". Redirecting stdout to a file makes fd 1 a non-TTY, so the pre-existing ! -t 1 clause is already true on quattro and the base implementation prints the same "Re-run..." message the grep looks for. I verified this by mutation on a disposable VM: reverting only bin/omarchy-update-orphan-pkgs to the base version and leaving your test in place still gives 3 passed, 0 failed, exit 0. The assertion exercises the branch that already worked, not the one you fixed.

That matters because of what the bug actually is. As #8780 sets out, omarchy-update re-execs itself under script(1), so a -y run has a pseudo-terminal on both descriptors and the TTY clauses never fire — which is exactly the condition a redirected stdout cannot reproduce. test/shell.d/update-package-conflict-test.sh:80 has the house helper for this, run_on_terminal, which wraps the child in script -qec and drains the outer streams; the same shape applied here would fail without your production change and pass with it.

Tests run, on a throwaway VM rather than locally: update-orphan-test.sh, update-sequence-test.sh, update-lock-test.sh, update-disk-space-test.sh and update-package-conflict-test.sh all pass at fd7de002, 28 assertions with no failures. No regression anywhere — the coverage gap above is the only finding.

Codex reached the same conclusion about the test independently, including the file-descriptor reasoning, and found nothing else; its independence is not currently guaranteed, since its sandbox restricts writes and not reads. It did add one thing I had not traced myself, confirming that bin/omarchy:992 forwards arguments through the top-level dispatcher so -y reaches omarchy-update intact.

One gap neither fix for this issue covers: bin/omarchy-update-restart calls gum confirm with no guard at all, not even a TTY check, so a -y run that updates the kernel would still hang there. #8780 calls this out at the end. Worth knowing that "Fixes #8780" will close the issue with that path still open.

Waiting on the maintainer. A second pull request, #8824, fixes the same issue with the same two files, and the two conflict textually — whichever lands, the other needs a rebase rather than a merge. Which one to take is the maintainer's call, not mine, so I have deliberately not pushed a test fix to your branch: doing that would flatten a difference between the two submissions right as someone has to weigh them. If you would like to fix it yourself, the run_on_terminal helper linked above is the pattern to copy.

@Chessing234

Copy link
Copy Markdown
Contributor Author

@dhh @ryanrhughes ready for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

omarchy update -y hangs on the orphaned-packages prompt

3 participants