fix(omarchy): survive an app-menu lookup that finds nothing #482

Merged

enricobuehler merged 1 commits from fix/omarchy-app-menu-abort into main 2026-08-31 20:22:55 +00:00

Conversation 0 Commits 1 Files Changed 3

+48 -3

enricobuehler commented 2026-08-31 20:20:16 +00:00

Owner

Copy Link

Copy Source

The report

Two users hit the same wall running the guided installer on Omarchy:

==> Browser trust
  trust the console's certificate in Chromium (removes the security warning)? [Y/n]
  ✓ console certificate trusted — a Chromium that is already running picks it up on restart
==> App menu
  xx that step failed — fix it and re-run (the script is safe to repeat), or follow the page
     by hand: https://docs.punktfunk.unom.io/docs/omarchy

A step header, then nothing. No warn, no bad, no stderr — the one shape that leaves an
operator with nothing to act on.

What actually happens

setup_webapp dies on its third statement. The pre-rename migration added in 8fb8656d does:

old=$(grep -rl "^Name=$WEBAPP_OLD$" "$HOME/.local/share/applications" 2>/dev/null | head -1)

Finding nothing is the normal case here — that is the whole point of a migration probe — but a
bare assignment carries the pipeline's exit status. Under the script's set -euo pipefail:

condition	status	who produces it
no file matches	1	grep, through pipefail
~/.local/share/applications absent	2	grep, through pipefail
many matches, head -1 closes the pipe	141	SIGPIPE, through pipefail

Reduced to the failing shape:

$ cat repro.sh
#!/usr/bin/env bash
set -euo pipefail
echo "==> App menu"
old=$(grep -rl "^Name=Nope$" "$PWD/nosuchdir" 2>/dev/null | head -1)
echo "reached the next line — old='$old'"

$ bash repro.sh; echo "exit=$?"
==> App menu
exit=2

So the failure is not Omarchy-specific and not machine-specific. It fires for anyone who does
not have an entry named exactly Punktfunk Console — which is every fresh install, and every
box that skipped the version that used the old name.

Blast radius

setup_webapp is called from cmd_setup at line 594, and set -e unwinds the whole script:

  setup_webapp     ← dies here
  setup_plugin              never runs
  setup_client_menu         never runs
  setup_hooks / setup_idle  never run
  write_hooks               never runs
  setup_theme               never runs
  check_conflicts           never runs
  cmd_status                never runs

The webapp entry, the Omarchy menu rows, the bar plugin, the client menu, the hooks, the theme —
none of it lands. punktfunk-omarchy setup exits non-zero, install.sh's run() sees that and
prints its generic failed-step die, which is the message the users pasted.

The fix

|| true on the assignment, and on the two siblings of the same shape rather than only the one
the report named:

• setup_webapp:289 — the migration probe (the one that fires)
• setup_webapp:299 — the entry lookup after a successful install
• setup_menu:411 — close=$(grep -n … | tail -1 | cut -d: -f1)

That third one is worth calling out: it already had an empty-case handler directly beneath it.

close=$(grep -n '^[[:space:]]*}[[:space:]]*$' "$tmp" | tail -1 | cut -d: -f1)
if [[ -z "$close" ]]; then
  rm -f "$tmp"
  warn "could not find the closing brace in $MENU_FILE — leaving it alone"
  return 0
fi

set -e made that handler unreachable — the script was already gone by the if. Defensive code
that cannot run is worse than none, because it reads as covered.

|| true is the file's existing house pattern for this class (see the comment at line 114 about
bare [[ … ]] && statements), so this stays in the idiom rather than introducing a new one.

The check, and why the obvious version was wrong

packaging/linux/omarchy/selftest.sh gains one case: setup_webapp must reach the end on a clean
box — no pre-rename entry, no applications directory at all.

The first version I wrote passed against the broken script:

if ( export HOME=… PATH=…; setup_webapp ) >/dev/null 2>&1 && …

set -e is suppressed for everything inside an if condition — subshells and called functions
included, recursively. An in-process call is therefore structurally incapable of catching this
bug. The committed version spawns a child bash that re-sources the script, so the script's own
set -euo pipefail is what governs, exactly as an operator gets it:

if env HOME="$WORK/webapp" XDG_CONFIG_HOME="$WORK/webapp/config" PATH="$WORK/bin:$PATH" \
     bash -c "source $SCRIPT help >/dev/null 2>&1; setup_webapp" >/dev/null 2>&1 &&
   grep -q '"punktfunk-host.console"' "$WORK/webapp/config/omarchy/extensions/omarchy-menu.jsonc"

Verified in both directions:

# against origin/main (unpatched)
app menu
  FAIL setup_webapp aborted before the menu on a clean box

# with the fix
app menu
  ok   setup_webapp completes with no prior entry and reaches the menu

The grep -q for the menu row is deliberate: exit 0 alone would also be satisfied by an early
return 0, so the check asserts it reached setup_menu, which is the last thing setup_webapp
does.

CI

selftest.sh was never wired into anything. It was added, extended twice, and never ran outside a
developer's terminal — which is precisely how a regression in the function directly above the
tested ones shipped.

It now runs as a job in installer-smoke.yml, with packaging/linux/omarchy/** added to the path
filter. No container: bash and python3 are all the checks touch, so it is a ~5s job, unlike the
matrix beside it.

Rejected

• A first_entry_named() helper. Two of the three sites share a shape; the third
  (grep -n | tail -1 | cut) does not. A helper covering two thirds of the class, with the odd one
  out still bare, is a worse invariant than || true on all three.
• Dropping pipefail for the script. It is load-bearing in the menu merge, where a silently
  truncated pipeline would write a user's omarchy-menu.jsonc wrong. Fix the three assignments,
  not the shell option that catches real bugs elsewhere.
• set +e around setup_webapp in cmd_setup. Suppresses the symptom at the call site and
  leaves every other caller of the same pattern exposed.

Verification

• Self-test: 17 checks green; the new case red against origin/main, green with the fix.
• shellcheck -s bash on both files: no new findings (six pre-existing info-level notes remain).
• scripts/ci/check-writing.sh: neither touched file nor the commit is flagged.
• Migration path with an old entry present, and omarchy-webapp-remove stubbed to fail so the
  rm -f fallback is the one under test: exits 0, entry removed, menu rows written.
• installer-smoke.yml parses as YAML.

Workaround until this is in a package

Give the probe something to find; setup deletes it during migration, so nothing is left behind:

mkdir -p ~/.local/share/applications
printf '[Desktop Entry]\nName=Punktfunk Console\n' > ~/.local/share/applications/pf-shim.desktop
punktfunk-omarchy setup

## The report Two users hit the same wall running the guided installer on Omarchy: ``` ==> Browser trust trust the console's certificate in Chromium (removes the security warning)? [Y/n] ✓ console certificate trusted — a Chromium that is already running picks it up on restart ==> App menu xx that step failed — fix it and re-run (the script is safe to repeat), or follow the page by hand: https://docs.punktfunk.unom.io/docs/omarchy ``` A step header, then nothing. No `warn`, no `bad`, no stderr — the one shape that leaves an operator with nothing to act on. ## What actually happens `setup_webapp` dies on its third statement. The pre-rename migration added in 8fb8656d does: ```bash old=$(grep -rl "^Name=$WEBAPP_OLD$" "$HOME/.local/share/applications" 2>/dev/null | head -1) ``` Finding nothing is the *normal* case here — that is the whole point of a migration probe — but a bare assignment carries the pipeline's exit status. Under the script's `set -euo pipefail`: | condition | status | who produces it | |---|---|---| | no file matches | 1 | `grep`, through `pipefail` | | `~/.local/share/applications` absent | 2 | `grep`, through `pipefail` | | many matches, `head -1` closes the pipe | 141 | SIGPIPE, through `pipefail` | Reduced to the failing shape: ```console $ cat repro.sh #!/usr/bin/env bash set -euo pipefail echo "==> App menu" old=$(grep -rl "^Name=Nope$" "$PWD/nosuchdir" 2>/dev/null | head -1) echo "reached the next line — old='$old'" $ bash repro.sh; echo "exit=$?" ==> App menu exit=2 ``` So the failure is not Omarchy-specific and not machine-specific. It fires for anyone who does **not** have an entry named exactly `Punktfunk Console` — which is every fresh install, and every box that skipped the version that used the old name. ## Blast radius `setup_webapp` is called from `cmd_setup` at line 594, and `set -e` unwinds the whole script: ``` setup_webapp ← dies here setup_plugin never runs setup_client_menu never runs setup_hooks / setup_idle never run write_hooks never runs setup_theme never runs check_conflicts never runs cmd_status never runs ``` The webapp entry, the Omarchy menu rows, the bar plugin, the client menu, the hooks, the theme — none of it lands. `punktfunk-omarchy setup` exits non-zero, `install.sh`'s `run()` sees that and prints its generic failed-step `die`, which is the message the users pasted. ## The fix `|| true` on the assignment, and on the two siblings of the same shape rather than only the one the report named: - `setup_webapp:289` — the migration probe (the one that fires) - `setup_webapp:299` — the entry lookup after a successful install - `setup_menu:411` — `close=$(grep -n … | tail -1 | cut -d: -f1)` That third one is worth calling out: it already had an empty-case handler directly beneath it. ```bash close=$(grep -n '^[[:space:]]*}[[:space:]]*$' "$tmp" | tail -1 | cut -d: -f1) if [[ -z "$close" ]]; then rm -f "$tmp" warn "could not find the closing brace in $MENU_FILE — leaving it alone" return 0 fi ``` `set -e` made that handler unreachable — the script was already gone by the `if`. Defensive code that cannot run is worse than none, because it reads as covered. `|| true` is the file's existing house pattern for this class (see the comment at line 114 about bare `[[ … ]] &&` statements), so this stays in the idiom rather than introducing a new one. ## The check, and why the obvious version was wrong `packaging/linux/omarchy/selftest.sh` gains one case: `setup_webapp` must reach the end on a clean box — no pre-rename entry, no applications directory at all. The first version I wrote **passed against the broken script**: ```bash if ( export HOME=… PATH=…; setup_webapp ) >/dev/null 2>&1 && … ``` `set -e` is suppressed for everything inside an `if` condition — subshells and called functions included, recursively. An in-process call is therefore structurally incapable of catching this bug. The committed version spawns a **child bash** that re-sources the script, so the script's own `set -euo pipefail` is what governs, exactly as an operator gets it: ```bash if env HOME="$WORK/webapp" XDG_CONFIG_HOME="$WORK/webapp/config" PATH="$WORK/bin:$PATH" \ bash -c "source $SCRIPT help >/dev/null 2>&1; setup_webapp" >/dev/null 2>&1 && grep -q '"punktfunk-host.console"' "$WORK/webapp/config/omarchy/extensions/omarchy-menu.jsonc" ``` Verified in both directions: ``` # against origin/main (unpatched) app menu FAIL setup_webapp aborted before the menu on a clean box # with the fix app menu ok setup_webapp completes with no prior entry and reaches the menu ``` The `grep -q` for the menu row is deliberate: exit 0 alone would also be satisfied by an early `return 0`, so the check asserts it reached `setup_menu`, which is the last thing `setup_webapp` does. ## CI `selftest.sh` was never wired into anything. It was added, extended twice, and never ran outside a developer's terminal — which is precisely how a regression in the function directly above the tested ones shipped. It now runs as a job in `installer-smoke.yml`, with `packaging/linux/omarchy/**` added to the path filter. No container: `bash` and `python3` are all the checks touch, so it is a ~5s job, unlike the matrix beside it. ## Rejected - **A `first_entry_named()` helper.** Two of the three sites share a shape; the third (`grep -n | tail -1 | cut`) does not. A helper covering two thirds of the class, with the odd one out still bare, is a worse invariant than `|| true` on all three. - **Dropping `pipefail` for the script.** It is load-bearing in the menu merge, where a silently truncated pipeline would write a user's `omarchy-menu.jsonc` wrong. Fix the three assignments, not the shell option that catches real bugs elsewhere. - **`set +e` around `setup_webapp` in `cmd_setup`.** Suppresses the symptom at the call site and leaves every other caller of the same pattern exposed. ## Verification - Self-test: 17 checks green; the new case red against `origin/main`, green with the fix. - `shellcheck -s bash` on both files: no new findings (six pre-existing info-level notes remain). - `scripts/ci/check-writing.sh`: neither touched file nor the commit is flagged. - Migration path with an old entry present, and `omarchy-webapp-remove` stubbed to fail so the `rm -f` fallback is the one under test: exits 0, entry removed, menu rows written. - `installer-smoke.yml` parses as YAML. ## Workaround until this is in a package Give the probe something to find; setup deletes it during migration, so nothing is left behind: ```sh mkdir -p ~/.local/share/applications printf '[Desktop Entry]\nName=Punktfunk Console\n' > ~/.local/share/applications/pf-shim.desktop punktfunk-omarchy setup ```

enricobuehler added 1 commit 2026-08-31 20:20:22 +00:00

fix(omarchy): survive an app-menu lookup that finds nothing ... b594d291b4

The pre-rename migration added in 8fb8656d looks for the old webapp
entry with `old=$(grep -rl … | head -1)`. Finding nothing is the normal
case on a first install, but a bare assignment carries the pipeline's
status, so under `set -euo pipefail` a no-match grep (1) or a missing
applications dir (2) ended the whole script.

It ended it silently, after `==> App menu` had already printed and
before any handler ran, taking the plugin, the menu rows, the hooks,
the theme and the closing status with it. `punktfunk-omarchy setup`
exited non-zero, so the guided installer reported a failed step with
nothing to act on.

Two more assignments of the same shape get the same guard; one of them
already had an empty-case handler that could never be reached.

The self-check now runs `setup_webapp` on a clean box in a child bash,
because `set -e` is suppressed inside an `if` condition and an
in-process call passes while the real script dies. installer-smoke
runs that self-check — it existed, but nothing called it.

enricobuehler merged commit f35eb25f1e into main 2026-08-31 20:22:55 +00:00

enricobuehler deleted branch fix/omarchy-app-menu-abort 2026-08-31 20:23:00 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

FernandoMX FlipperDan HitFrostbite bluelightspecial enricobuehler (Enrico Bühler) kszlim luxus nannku neoney sonar

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unom/punktfunk#482