Guard basename against a login shell's leading-dash argv[0] in the browser reaper #393

Merged

joshtronic merged 1 commit from fix/reaper-basename-dash into master

2026-07-16 15:00:32 +00:00

igor commented

2026-07-16 04:22:40 +00:00

Collaborator

Problem

During the maintenance/recovery tick, basename was fed an argument starting with a dash and parsed it as an option: basename: invalid option -- 'b' (igor#392). It fired repeatedly right after the recovery sweep.

Root cause

lib/browser-reap.sh (the #389 stale-headless-browser reaper) takes the first whitespace token of each ps line as the binary path:

base=$(basename "${cmd%% *}")

A login shell appears in ps with a leading-dash argv[0] (-bash), so ${cmd%% *} is -bash and basename reads -b as an option. Because every harness lib runs under set -euo pipefail, that non-zero basename aborted the whole browser_reap_select_victims read loop mid-scan — so any genuine stale-browser victims listed after a login-shell row were never selected. The tick continued (the sweep is called tolerantly), which is why it looked non-fatal while the reaping it added in #389 was silently defeated.

Fix

Add the -- end-of-options guard:

base=$(basename -- "${cmd%% *}")

Test

bin/test-browser-reap.sh gains a case for a -bash argv[0]: it asserts the predicate emits no basename/option error (fails without the guard — reproduces invalid option -- 'b' — passes with it) and that a login shell is spared. make test green; shellcheck clean on both files.

Closes #392

## Problem During the maintenance/recovery tick, `basename` was fed an argument starting with a dash and parsed it as an option: `basename: invalid option -- 'b'` (igor#392). It fired repeatedly right after the recovery sweep. ## Root cause `lib/browser-reap.sh` (the #389 stale-headless-browser reaper) takes the first whitespace token of each `ps` line as the binary path: ```sh base=$(basename "${cmd%% *}") ``` A **login shell** appears in `ps` with a leading-dash argv[0] (`-bash`), so `${cmd%% *}` is `-bash` and `basename` reads `-b` as an option. Because every harness lib runs under `set -euo pipefail`, that non-zero `basename` aborted the whole `browser_reap_select_victims` read loop mid-scan — so any genuine stale-browser victims listed *after* a login-shell row were never selected. The tick continued (the sweep is called tolerantly), which is why it looked non-fatal while the reaping it added in #389 was silently defeated. ## Fix Add the `--` end-of-options guard: ```sh base=$(basename -- "${cmd%% *}") ``` ## Test `bin/test-browser-reap.sh` gains a case for a `-bash` argv[0]: it asserts the predicate emits **no** basename/option error (fails without the guard — reproduces `invalid option -- 'b'` — passes with it) and that a login shell is spared. `make test` green; `shellcheck` clean on both files. Closes #392

igor added 1 commit

2026-07-16 04:22:40 +00:00

Guard basename against a login shell's leading-dash argv[0] in the reaper

Lint / check-sync (push) Successful in 5s

Details

Lint / check-sync (pull_request) Successful in 4s

Details

b7b275b458

A login shell appears in `ps` as `-bash`, so `${cmd%% *}` is `-bash` and
`basename` parsed the `-b` as an option ("basename: invalid option -- 'b'").
Under the harness's `set -euo pipefail`, that error aborted the whole
browser_reap_select_victims loop mid-scan, so any real stale-browser victims
listed after such a row were never selected — silently defeating the #389
reaper. Add the `--` end-of-options guard.

Regression test asserts a leading-dash argv[0] emits no basename/option error
(fails without the guard, passes with it) and that the login shell is spared.

Closes #392

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YUVfbfUWdUwifGJ2TvkP

igor commented

2026-07-16 04:23:24 +00:00

Author

Collaborator

🤖 Review — `APPROVE` (automated)

CI for b7b275b4: success

One-line: Adds basename -- to stop a login shell's leading-dash argv[0] (-bash) from being parsed as an option and aborting the reaper's selection loop — correct, focused, and tested.

Findings:

lib/browser-reap.sh:66 — basename -- "${cmd%% *}" is the right fix. -- terminates option parsing so -bash is treated as a literal path; the resulting base is -bash, which is neither protected nor a browser binary, so it's correctly spared. This resolves the invalid option -- 'b' abort under set -euo pipefail.
bin/test-browser-reap.sh:65-78 — new case genuinely exercises the bug: asserts no error on stderr for -bash argv[0] (would fail pre-fix) and that the login shell is spared. Good regression coverage.
Scope is tight (2 files, ~16 lines), description matches the diff, checklist claims (make test green, shellcheck clean) are consistent with CI success.

No security, correctness, or scope concerns. Nothing blocking.

_{Independent review by the harness on claude-opus-4-8 (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `APPROVE` _(automated)_ CI for `b7b275b4`: **success** One-line: Adds `basename --` to stop a login shell's leading-dash argv[0] (`-bash`) from being parsed as an option and aborting the reaper's selection loop — correct, focused, and tested. Findings: - `lib/browser-reap.sh:66` — `basename -- "${cmd%% *}"` is the right fix. `--` terminates option parsing so `-bash` is treated as a literal path; the resulting base is `-bash`, which is neither protected nor a browser binary, so it's correctly spared. This resolves the `invalid option -- 'b'` abort under `set -euo pipefail`. - `bin/test-browser-reap.sh:65-78` — new case genuinely exercises the bug: asserts no error on stderr for `-bash` argv[0] (would fail pre-fix) and that the login shell is spared. Good regression coverage. - Scope is tight (2 files, ~16 lines), description matches the diff, checklist claims (`make test` green, shellcheck clean) are consistent with CI `success`. No security, correctness, or scope concerns. Nothing blocking. ==== --- <sub>Independent review by the harness on `claude-opus-4-8` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-07-16 04:23:25 +00:00

15 seconds

igor requested review from joshtronic

2026-07-16 04:23:25 +00:00

igor referenced this pull request

2026-07-16 13:36:23 +00:00

Forgejo API curls lack --connect-timeout: a brief git.sherver.org blip wedges a tick ~30s and false-trips the healthcheck #395

igor referenced this pull request

2026-07-16 13:39:09 +00:00

Fail-fast Forgejo timeouts: --connect-timeout + tighter --max-time #396