HOMELAB-2479: fix(forgejo): exit 4 on unproven signal-path revoke; SIGINT/SIGHUP e2e tests #124

Merged

pi-landing merged 1 commit from plane/HOMELAB-2479-122-signal-exit-fix into main 2026-11-10 12:33:57 +00:00

Conversation 2 Commits 1 Files changed 2 +114 -21

pi-landing commented 2026-11-10 12:14:21 +00:00

Collaborator

Copy link

Fix-forward of #122 (merged 6ca09e38). No behaviour outside the signal path
changes; no new capability, no new file, no new dependency.

Defect — a failed revoke on a signal never exits the documented code 4

forgejo/seat-post.ts has two exit paths that disagreed:

case	exit status
normal unwind, revoke proven dead	the post's code (0/1/2/3)
normal unwind, revoke not proven dead	4 (EXIT.revoke)
signal, revoke proven dead	130 / 143 / 129
signal, revoke not proven dead	130 / 143 / 129 — never 4

The signal handler ran reportRevoke(await doRevoke()) and then exited
unconditionally with signal === "SIGTERM" ? 143 : signal === "SIGHUP" ? 129 : 130.
Exit 4 is the only documented machine-readable way to detect "the OpenBao
token may still be alive" (header exit table, and EXIT = { …, revoke: 4 });
128+signal appears nowhere in that table. So a wrapper implementing the
documented contract — [ $? -eq 4 ] && alarm — silently missed the case that
matters most: a posting interrupted mid-flight whose revoke could not be
proven. 143 says only "terminated", which is also what a clean interruption
looks like.

Both review seats on #122 raised this and the notes were reworded into
endorsements instead of being fixed:

[#1979 seat 3 pi-extensions#122] (1) the signal path reports a failed revoke loudly but exits 128+signal rather than 4 (conventional, pre-existing). (2) SIGINT/SIGHUP branches are covered by the handler but only SIGTERM is driven end-to-end by a test.

Recovered note text lives in
working-files/infra-stability/gpu-whisper/status.md (RECOVERED SEAT NOTES).

Fix — one mapping shared by both paths, so they cannot drift again

const exitCodeAfterRevoke = (verdict: RevokeVerdict | null, fallback: number): number =>
  verdict && !verdict.dead ? EXIT.revoke : fallback;

• the signal handler now exits 4 when the revoke is not proven dead, and
  keeps 128+signal when it is;
• the finally keeps its behaviour through the same helper;
• the handler still removes the staged token dir first and still names the
  signal on stderr, and additionally says when 4 replaced 128+signal. Ordering is
  unchanged: remove staged dirs → revoke → report → exit (what closed #1973);
• 128+signal is a convention the helper reproduces, not a kernel fact — the
  handler catches the signal, so the OS already sees a normal exit. Nothing is
  silently reclassified: the signal is named on stderr either way;
• this costs nothing against the hygiene contract, which asks for a non-zero
  exit on a failed revoke — 4 is non-zero.

Header doc updated in both places that contradicted each other: the hygiene
contract now states the non-zero exit on a signal is 4, and the exit table says
4 takes precedence over 128+signal.

Tests — SIGINT and SIGHUP driven end to end, plus the failed-revoke matrix

The existing SIGTERM child-process case (the real helper, posting phase hung, a
real signal while the staged 0600 token file exists) is parametrised into
driveSignal(signal, revokeProvenDead) and now drives all three signals:

• revoke proven dead (lookup-self 403): SIGTERM → 143, SIGINT → 130,
  SIGHUP → 129 — each asserts the 0600 token file was really staged before the
  signal (otherwise the case is vacuous), the staged dir is gone afterwards, and
  POST /v1/auth/token/revoke-self was still sent;
• revoke NOT proven dead (lookup-self 200): SIGTERM, SIGINT and SIGHUP all
  exit 4, with stderr carrying NOT proven dead and instead of 143/130/129.

The in-process harness is deliberately not extended to cover signals: it
stubs onSignal, which is the structural blind spot the #1973 blocker rode
through.

Evidence

cd forgejo && npm run typecheck && npm test
 Test Files  50 passed (50)
      Tests  1213 passed (1213)

Against the pre-fix seat-post.ts the three unproven-revoke cases fail and
reproduce the defect exactly:

× SIGTERM mid-posting with an unproven revoke exits 4, not 143 → expected 143 to be 4
× SIGINT  mid-posting with an unproven revoke exits 4, not 130 → expected 130 to be 4
× SIGHUP  mid-posting with an unproven revoke exits 4, not 129 → expected 129 to be 4

Scope

Two files, no other change: forgejo/seat-post.ts (the shared helper, the
signal handler's exit code and one stderr line, two header paragraphs) and
forgejo/tests/seat-post.test.ts (the parametrised signal cases). comment-as.ts,
index.ts, api.ts, package.json are untouched.

Fix-forward of #122 (merged `6ca09e38`). No behaviour outside the signal path changes; no new capability, no new file, no new dependency. ## Defect — a failed revoke on a signal never exits the documented code 4 `forgejo/seat-post.ts` has two exit paths that disagreed: | case | exit status | |---|---| | normal unwind, revoke proven dead | the post's code (0/1/2/3) | | normal unwind, revoke **not** proven dead | **4** (`EXIT.revoke`) | | signal, revoke proven dead | 130 / 143 / 129 | | signal, revoke **not** proven dead | **130 / 143 / 129 — never 4** | The signal handler ran `reportRevoke(await doRevoke())` and then exited unconditionally with `signal === "SIGTERM" ? 143 : signal === "SIGHUP" ? 129 : 130`. Exit **4** is the only documented machine-readable way to detect "the OpenBao token may still be alive" (header exit table, and `EXIT = { …, revoke: 4 }`); 128+signal appears nowhere in that table. So a wrapper implementing the documented contract — `[ $? -eq 4 ] && alarm` — silently missed the case that matters most: a posting interrupted mid-flight whose revoke could not be proven. 143 says only "terminated", which is also what a clean interruption looks like. Both review seats on #122 raised this and the notes were reworded into endorsements instead of being fixed: > `[#1979 seat 3 pi-extensions#122] (1) the signal path reports a failed revoke loudly but exits 128+signal rather than 4 (conventional, pre-existing). (2) SIGINT/SIGHUP branches are covered by the handler but only SIGTERM is driven end-to-end by a test.` Recovered note text lives in `working-files/infra-stability/gpu-whisper/status.md` (RECOVERED SEAT NOTES). ## Fix — one mapping shared by both paths, so they cannot drift again ```ts const exitCodeAfterRevoke = (verdict: RevokeVerdict | null, fallback: number): number => verdict && !verdict.dead ? EXIT.revoke : fallback; ``` * the signal handler now exits **4** when the revoke is not proven dead, and keeps **128+signal** when it is; * the `finally` keeps its behaviour through the same helper; * the handler still **removes the staged token dir first** and still names the signal on stderr, and additionally says when 4 replaced 128+signal. Ordering is unchanged: remove staged dirs → revoke → report → exit (what closed #1973); * 128+signal is a convention the helper reproduces, not a kernel fact — the handler catches the signal, so the OS already sees a normal exit. Nothing is silently reclassified: the signal is named on stderr either way; * this costs nothing against the hygiene contract, which asks for a *non-zero* exit on a failed revoke — 4 is non-zero. Header doc updated in both places that contradicted each other: the hygiene contract now states the non-zero exit on a signal is 4, and the exit table says 4 takes precedence over 128+signal. ## Tests — SIGINT and SIGHUP driven end to end, plus the failed-revoke matrix The existing SIGTERM child-process case (the real helper, posting phase hung, a real signal while the staged 0600 token file exists) is parametrised into `driveSignal(signal, revokeProvenDead)` and now drives all three signals: * **revoke proven dead** (lookup-self 403): SIGTERM → 143, SIGINT → 130, SIGHUP → 129 — each asserts the 0600 token file was *really staged* before the signal (otherwise the case is vacuous), the staged dir is gone afterwards, and `POST /v1/auth/token/revoke-self` was still sent; * **revoke NOT proven dead** (lookup-self 200): SIGTERM, SIGINT and SIGHUP all exit **4**, with stderr carrying `NOT proven dead` and `instead of 143/130/129`. The in-process harness is deliberately *not* extended to cover signals: it stubs `onSignal`, which is the structural blind spot the #1973 blocker rode through. ### Evidence ``` cd forgejo && npm run typecheck && npm test Test Files 50 passed (50) Tests 1213 passed (1213) ``` Against the pre-fix `seat-post.ts` the three unproven-revoke cases fail and reproduce the defect exactly: ``` × SIGTERM mid-posting with an unproven revoke exits 4, not 143 → expected 143 to be 4 × SIGINT mid-posting with an unproven revoke exits 4, not 130 → expected 130 to be 4 × SIGHUP mid-posting with an unproven revoke exits 4, not 129 → expected 129 to be 4 ``` ## Scope Two files, no other change: `forgejo/seat-post.ts` (the shared helper, the signal handler's exit code and one stderr line, two header paragraphs) and `forgejo/tests/seat-post.test.ts` (the parametrised signal cases). `comment-as.ts`, `index.ts`, `api.ts`, `package.json` are untouched.

pi-landing added 1 commit 2026-11-10 12:14:21 +00:00

HOMELAB-2479: fix(forgejo): exit 4 on unproven signal-path revoke; SIGINT/SIGHUP e2e tests ... 05fc9e4bb4

The signal handler in seat-post.ts reported the revoke verdict on stderr and
then exited unconditionally with 128+signal (SIGTERM 143, SIGHUP 129,
SIGINT 130), while every other exit path uses 4 when the OpenBao token is not
proven dead. Exit 4 is the documented machine-readable "the token may be live"
code (header exit table and EXIT.revoke); 128+signal is not in the table at
all, so the two paths disagreed and a wrapper implementing the documented
contract could not see the case that matters most — an interrupted posting
whose revoke did not take.

Fix: one mapping, exitCodeAfterRevoke(), used by BOTH the `finally` and the
signal handler so they cannot drift. A signal now exits 4 when the revoke is
not proven dead, and keeps 128+signal when it is. The handler still removes the
staged token dir first and still names the signal on stderr (128+signal is a
convention this helper reproduces, not a kernel fact — the handler catches the
signal, so the OS sees a normal exit). Ordering is unchanged: remove staged
dirs -> revoke -> report -> exit. Header doc updated at both places: the
hygiene contract now says the non-zero exit on a signal is 4, and the exit
table says 4 takes precedence over 128+signal.

Tests: the SIGTERM child-process case is parametrised into driveSignal(signal,
revokeProvenDead) and drives SIGTERM/SIGINT/SIGHUP end to end:
  - revoke proven dead (lookup-self 403): exit 143/130/129, staged 0600 token
    file really staged first, staged dir gone after, revoke-self attempted;
  - revoke NOT proven dead (lookup-self 200): exit 4 for all three signals,
    stderr carries "NOT proven dead" and "instead of 143/130/129".

Evidence at head: `cd forgejo && npm run typecheck && npm test` -> 50 files,
1213 tests passed. Against the pre-fix seat-post.ts the three unproven-revoke
cases fail with "expected 143/130/129 to be 4", i.e. they reproduce the defect
and pass after the fix.

Why this is the #122 fix-forward: review seats #1979 and #1974 raised exactly
this ("the signal path reports a failed revoke loudly but exits 128+signal
rather than 4" and "only SIGTERM is driven end-to-end"), and those two notes
were reworded into endorsements instead of being fixed. Recovered note text:
working-files/infra-stability/gpu-whisper/status.md.

Refs: HOMELAB-2479, pi-extensions#122 (merged 6ca09e38).

ld-reviewer approved these changes 2026-11-10 12:23:45 +00:00

Dismissed

ld-reviewer left a comment

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/deepseek-flash
head: 05fc9e4bb4
session: seat-124-1

Approval of record — Homelab/pi-extensions#124

Pinned to exact head 05fc9e4bb4144c932c9add9aa73264cc0a2f7195.

What I measured myself

The head was staged in my seat sandbox with my own ld-reviewer credential (token file read by path). ls-remote answered 05fc9e4bb4144c932c9add9aa73264cc0a2f7195 for refs/heads/plane/HOMELAB-2479-122-signal-exit-fix, and the PR API answered base main 6ca09e3827616c2fbf855f1bb82351b10895ef37, open, mergeable, not merged. The head matches the expected pin.

1. Scope — exactly the two named files

git diff --name-status between base and head lists two entries and nothing else: forgejo/seat-post.ts and forgejo/tests/seat-post.test.ts (+30/-5 and +84/-16; 2 files, 114 insertions, 21 deletions). No CI workflow, no oversight path, no unrelated refactor. I scanned every added line for credential-shaped literals (32+ characters of token alphabet): none, and the added content is the exit mapping, its doc comments and the tests.

2. The exit mapping is genuinely one function, and it is correct

exitCodeAfterRevoke(verdict, fallback) returns EXIT.revoke only when a verdict exists and dead is false. EXIT.revoke is now produced at exactly one site in the file (line 85) — every other mention is prose — so no other path can emit 4, and no path can emit 4 for a revoke that is proven dead (a dead verdict always yields the fallback). Both callers route through it:

• the finally: code = exitCodeAfterRevoke(verdict, code), which is the same semantics as the previous inline conditional, so that contract is preserved;
• the signal handler: exitCodeAfterRevoke(verdict, signalCode) with signalCode 143 / 130 / 129 for SIGTERM / SIGINT / SIGHUP, and stderr naming the signal and the substitution on the 4 path.

The null verdict (nothing was minted, so there is nothing to revoke) yields the fallback on both paths, which is right — there is no token whose survival could be unproven. The handler's ordering is unchanged: staged dirs removed first, then revoke, then report, then exit.

3. The tests assert behaviour, and I ran them

At the head I ran npm ci then npx vitest run tests/seat-post.test.ts: 15/15 pass, including six parametrized rows — for each of SIGTERM, SIGINT and SIGHUP, the lookup-self 403 case (expects 128+signal) and the lookup-self 200 case (expects 4). Each row spawns the real helper as a child, proves the staged 0600 token file exists before the signal, then asserts the child's exit code, that the staged dir is gone afterwards, that the fake server really saw POST /v1/auth/token/revoke-self, and the stderr text. Nothing there asserts an internal helper by name.

I also ran the head test file against the base seat-post.ts in a separate worktree: the three unproven rows fail there (expected 143 to be 4, expected 130 to be 4, expected 129 to be 4) while the three proven-dead rows still pass. The tests fail on the bug and pass on the fix, which is behavioural evidence rather than implementation coupling.

npx tsc --noEmit at the head: exit 0, no diagnostics.

4. CI at this exact head

Combined status for 05fc9e4bb4144c932c9add9aa73264cc0a2f7195 is success, 15/15 contexts, including CI / forgejo (run 452 job 5) and CI / pr-title (run 452 job 0).

Acceptance

• exactly the two named files, nothing else
• one mapping; an unproven revoke yields 4 on every signal; a proven-dead revoke keeps 128+signal; the finally contract is unchanged; 4 is unreachable when the revoke is provably dead; cleanup ordering unchanged
• tests assert three signals x two revoke outcomes, staged file present before / dir absent after, revoke actually attempted, plus the negative control on the base code
• no secrets, no unrelated edits, no behaviour change outside the exit mapping and its tests
• CI green at the exact head

Verdict: approved at 05fc9e4bb4144c932c9add9aa73264cc0a2f7195.

## Verdict: APPROVED model: omniroute/deepseek-flash head: 05fc9e4bb4144c932c9add9aa73264cc0a2f7195 session: seat-124-1 ## Approval of record — Homelab/pi-extensions#124 Pinned to exact head `05fc9e4bb4144c932c9add9aa73264cc0a2f7195`. ### What I measured myself The head was staged in my seat sandbox with my own ld-reviewer credential (token file read by path). `ls-remote` answered `05fc9e4bb4144c932c9add9aa73264cc0a2f7195` for `refs/heads/plane/HOMELAB-2479-122-signal-exit-fix`, and the PR API answered base `main` `6ca09e3827616c2fbf855f1bb82351b10895ef37`, open, mergeable, not merged. The head matches the expected pin. ### 1. Scope — exactly the two named files `git diff --name-status` between base and head lists two entries and nothing else: `forgejo/seat-post.ts` and `forgejo/tests/seat-post.test.ts` (+30/-5 and +84/-16; 2 files, 114 insertions, 21 deletions). No CI workflow, no oversight path, no unrelated refactor. I scanned every added line for credential-shaped literals (32+ characters of token alphabet): none, and the added content is the exit mapping, its doc comments and the tests. ### 2. The exit mapping is genuinely one function, and it is correct `exitCodeAfterRevoke(verdict, fallback)` returns `EXIT.revoke` only when a verdict exists and `dead` is false. `EXIT.revoke` is now produced at exactly one site in the file (line 85) — every other mention is prose — so no other path can emit 4, and no path can emit 4 for a revoke that is proven dead (a dead verdict always yields the fallback). Both callers route through it: - the `finally`: `code = exitCodeAfterRevoke(verdict, code)`, which is the same semantics as the previous inline conditional, so that contract is preserved; - the signal handler: `exitCodeAfterRevoke(verdict, signalCode)` with `signalCode` 143 / 130 / 129 for SIGTERM / SIGINT / SIGHUP, and stderr naming the signal and the substitution on the 4 path. The `null` verdict (nothing was minted, so there is nothing to revoke) yields the fallback on both paths, which is right — there is no token whose survival could be unproven. The handler's ordering is unchanged: staged dirs removed first, then revoke, then report, then exit. ### 3. The tests assert behaviour, and I ran them At the head I ran `npm ci` then `npx vitest run tests/seat-post.test.ts`: 15/15 pass, including six parametrized rows — for each of SIGTERM, SIGINT and SIGHUP, the lookup-self 403 case (expects 128+signal) and the lookup-self 200 case (expects 4). Each row spawns the real helper as a child, proves the staged 0600 token file exists before the signal, then asserts the child's exit code, that the staged dir is gone afterwards, that the fake server really saw `POST /v1/auth/token/revoke-self`, and the stderr text. Nothing there asserts an internal helper by name. I also ran the head test file against the *base* `seat-post.ts` in a separate worktree: the three unproven rows fail there (`expected 143 to be 4`, `expected 130 to be 4`, `expected 129 to be 4`) while the three proven-dead rows still pass. The tests fail on the bug and pass on the fix, which is behavioural evidence rather than implementation coupling. `npx tsc --noEmit` at the head: exit 0, no diagnostics. ### 4. CI at this exact head Combined status for `05fc9e4bb4144c932c9add9aa73264cc0a2f7195` is `success`, 15/15 contexts, including `CI / forgejo` (run 452 job 5) and `CI / pr-title` (run 452 job 0). ### Acceptance - [x] exactly the two named files, nothing else - [x] one mapping; an unproven revoke yields 4 on every signal; a proven-dead revoke keeps 128+signal; the `finally` contract is unchanged; 4 is unreachable when the revoke is provably dead; cleanup ordering unchanged - [x] tests assert three signals x two revoke outcomes, staged file present before / dir absent after, revoke actually attempted, plus the negative control on the base code - [x] no secrets, no unrelated edits, no behaviour change outside the exit mapping and its tests - [x] CI green at the exact head **Verdict: approved at `05fc9e4bb4144c932c9add9aa73264cc0a2f7195`.**

ld-reviewer approved these changes 2026-11-10 12:30:02 +00:00

ld-reviewer left a comment

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/deepseek-flash
head: 05fc9e4bb4
session: seat-124-2

Seat 2 of 2 independent bridge review seats on this PR. Reviewed from primary sources with my own
measurements: the head fetched as ld-reviewer (GET /user = ld-reviewer, id 47) from
refs/heads/plane/HOMELAB-2479-122-signal-exit-fix, which resolves to
05fc9e4bb4144c932c9add9aa73264cc0a2f7195; base 6ca09e3827616c2fbf855f1bb82351b10895ef37 on main.

Acceptance checks (measured, not copied from the PR text)

1. Scope — exactly the two expected files, nothing else. git diff --name-status base..head =
M forgejo/seat-post.ts, M forgejo/tests/seat-post.test.ts; one commit; +114 / −21
(numstat 30/5 and 84/16). Nothing under .forgejo/, deploy/, .pi/, no workflow file, no
AGENTS.md, no agent instruction file. A scan of every added line for credential-shaped literals
(40+ char base64ish runs, BEGIN … PRIVATE blocks, password / secret / token: "…") returns
nothing.

2. The signal path and the normal path now share one mapping. The new
exitCodeAfterRevoke(verdict, fallback) = verdict && !verdict.dead ? EXIT.revoke : fallback is used in
both places: the handler's process.exit(exitCodeAfterRevoke(verdict, signalCode)) and the finally's
code = exitCodeAfterRevoke(verdict, code). The second is arithmetically identical to the
if (verdict && !verdict.dead) code = EXIT.revoke it replaced, so the normal-unwind contract is
unchanged, and the two paths can no longer disagree by construction. dead is still
revokeOk && lookupDenied (revoke-self 2xx AND lookup-self 403/404), so a revoke proven dead cannot
yield 4 on any signal: SIGTERM keeps 143, SIGINT 130, SIGHUP 129. A null verdict (nothing was
minted) falls through to the fallback, so the "nothing to revoke" path still exits with the signal
code, and no path below the mapping can reclassify a proven-dead revoke.

3. Ordering preserved. The handler does, in order: best-effort removal of every live staging dir
→ log.say naming the signal → await doRevoke() → reportRevoke(verdict) → the new stderr line
(naming 4 and the 128+signal code it superseded, only when the revoke is not proven dead) →
process.exit(...). That is the #1973 order with only the exit expression changed. doRevoke is
memoised on revokeState.done, so the handler and any pending finally cannot revoke twice or report
different verdicts.

4. The docs that contradicted each other were both moved. The header exit table now states 4
takes precedence over 128+signal and adds that 130/143/129 is the proven-dead convention with the
signal always named on stderr; the hygiene bullet now says a failed revoke on a signal exits 4. The
code, the table and the hygiene contract agree at the head.

5. The tests assert behaviour, and I watched them bite. In the sandbox on the head tree:
npx tsc --noEmit exit 0; npx vitest run tests/seat-post.test.ts → 15/15, with all six signal rows
listed individually. Each row of both it.each tables (three signals × proven-dead / unproven)
asserts that the 0600 token file was really staged before the signal (so a row cannot pass
vacuously), that the staged dir is gone afterwards, and that POST /v1/auth/token/revoke-self was
actually sent; the unproven rows additionally assert the code is EXIT.revoke and that stderr
carries NOT proven dead and instead of <signalCode>. Negative control I ran myself: reverting
only forgejo/seat-post.ts to 6ca09e38 makes exactly the three unproven rows fail
(expected 143/130/129 to be 4) while the three proven-dead rows still pass — the suite measures the
fix, not the implementation, and reproduces the #122 defect exactly as described.

6. No caller drifts. A tree-wide grep at the head for 143 / 130 / 129 / 128+ / SIGTERM outside
seat-post.ts and its tests found no other reader of this helper's signal exit codes, so tightening
the documented contract breaks nothing else in this repo.

7. CI green at the head. Runs #452 (all 15 jobs, forgejo included) and #453 (python) both
ran on 05fc9e4bb4 and passed.

Conclusion

APPROVED at 05fc9e4bb4144c932c9add9aa73264cc0a2f7195. The named defect is fixed in the code and
pinned by tests that fail without it; the diff is confined to the two files it claims; nothing secret,
nothing unrelated, no unrelated refactor.

## Verdict: APPROVED model: omniroute/deepseek-flash head: 05fc9e4bb4144c932c9add9aa73264cc0a2f7195 session: seat-124-2 Seat 2 of 2 independent bridge review seats on this PR. Reviewed from primary sources with my own measurements: the head fetched as `ld-reviewer` (`GET /user` = `ld-reviewer`, id 47) from `refs/heads/plane/HOMELAB-2479-122-signal-exit-fix`, which resolves to `05fc9e4bb4144c932c9add9aa73264cc0a2f7195`; base `6ca09e3827616c2fbf855f1bb82351b10895ef37` on `main`. ### Acceptance checks (measured, not copied from the PR text) **1. Scope — exactly the two expected files, nothing else.** `git diff --name-status base..head` = `M forgejo/seat-post.ts`, `M forgejo/tests/seat-post.test.ts`; one commit; +114 / −21 (numstat 30/5 and 84/16). Nothing under `.forgejo/`, `deploy/`, `.pi/`, no workflow file, no `AGENTS.md`, no agent instruction file. A scan of every added line for credential-shaped literals (40+ char base64ish runs, `BEGIN … PRIVATE` blocks, `password` / `secret` / `token: "…"`) returns nothing. **2. The signal path and the normal path now share one mapping.** The new `exitCodeAfterRevoke(verdict, fallback) = verdict && !verdict.dead ? EXIT.revoke : fallback` is used in both places: the handler's `process.exit(exitCodeAfterRevoke(verdict, signalCode))` and the `finally`'s `code = exitCodeAfterRevoke(verdict, code)`. The second is arithmetically identical to the `if (verdict && !verdict.dead) code = EXIT.revoke` it replaced, so the normal-unwind contract is unchanged, and the two paths can no longer disagree by construction. `dead` is still `revokeOk && lookupDenied` (revoke-self 2xx AND lookup-self 403/404), so a revoke proven dead cannot yield 4 on any signal: SIGTERM keeps 143, SIGINT 130, SIGHUP 129. A `null` verdict (nothing was minted) falls through to the fallback, so the "nothing to revoke" path still exits with the signal code, and no path below the mapping can reclassify a proven-dead revoke. **3. Ordering preserved.** The handler does, in order: best-effort removal of every live staging dir → `log.say` naming the signal → `await doRevoke()` → `reportRevoke(verdict)` → the new stderr line (naming 4 and the 128+signal code it superseded, only when the revoke is not proven dead) → `process.exit(...)`. That is the #1973 order with only the exit expression changed. `doRevoke` is memoised on `revokeState.done`, so the handler and any pending `finally` cannot revoke twice or report different verdicts. **4. The docs that contradicted each other were both moved.** The header exit table now states 4 takes precedence over 128+signal and adds that 130/143/129 is the proven-dead convention with the signal always named on stderr; the hygiene bullet now says a failed revoke on a signal exits 4. The code, the table and the hygiene contract agree at the head. **5. The tests assert behaviour, and I watched them bite.** In the sandbox on the head tree: `npx tsc --noEmit` exit 0; `npx vitest run tests/seat-post.test.ts` → 15/15, with all six signal rows listed individually. Each row of both `it.each` tables (three signals × proven-dead / unproven) asserts that the 0600 token file was really staged before the signal (so a row cannot pass vacuously), that the staged dir is gone afterwards, and that `POST /v1/auth/token/revoke-self` was actually sent; the unproven rows additionally assert the code is `EXIT.revoke` and that stderr carries `NOT proven dead` and `instead of <signalCode>`. Negative control I ran myself: reverting only `forgejo/seat-post.ts` to `6ca09e38` makes exactly the three unproven rows fail (`expected 143/130/129 to be 4`) while the three proven-dead rows still pass — the suite measures the fix, not the implementation, and reproduces the #122 defect exactly as described. **6. No caller drifts.** A tree-wide grep at the head for 143 / 130 / 129 / `128+` / `SIGTERM` outside `seat-post.ts` and its tests found no other reader of this helper's signal exit codes, so tightening the documented contract breaks nothing else in this repo. **7. CI green at the head.** Runs #452 (all 15 jobs, `forgejo` included) and #453 (`python`) both ran on `05fc9e4bb4` and passed. ### Conclusion APPROVED at `05fc9e4bb4144c932c9add9aa73264cc0a2f7195`. The named defect is fixed in the code and pinned by tests that fail without it; the diff is confined to the two files it claims; nothing secret, nothing unrelated, no unrelated refactor.

pi-landing merged commit 0c5ef847a3 into main 2026-11-10 12:33:57 +00:00

pi-landing referenced this pull request from a commit 2026-11-10 12:33:58 +00:00

Merge pull request 'HOMELAB-2479: fix(forgejo): exit 4 on unproven signal-path revoke; SIGINT/SIGHUP e2e tests' (#124) from plane/HOMELAB-2479-122-signal-exit-fix into main

Sign in to join this conversation.

Reviewers

No reviewers

ld-reviewer

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Homelab/pi-extensions!124

No description provided.