Skip to content

DR-SEC-01g-G4a: release ownership on signal stop - #872

Merged
mergify[bot] merged 4 commits into
mainfrom
claude/dr-sec-01g-g4a-stop
Oct 9, 2026
Merged

mergify[bot] merged 4 commits into
mainfrom
claude/dr-sec-01g-g4a-stop

Conversation

@Cuarroc

@Cuarroc Cuarroc commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Der Entscheidungsserver gibt beim Beenden per Strg+C (SIGINT) oder SIGTERM jetzt die Besitzmarke des Ledgers frei und beendet sich erst danach mit Code 0. Schlägt die Freigabe fehl, endet er mit Code 1 und einer kurzen Fehlerzeile; ein zweites Signal beendet sofort. Fehler beim Schließen stürzen den Server nicht mehr ab, sondern gehen an den Callback oder in eine Stderr-Zeile, und beide Fehlerursachen bleiben erhalten.

Gestapelter PR: Basis ist cursor/dr-sec-01g-g2-core (35fe38e, PR #866); Root setzt die Basis nach dem Merge von #866 auf main.

Report

Was geändert wurde (Tier A)

  • tools/denkraum/server.mjs (CLI-Einstieg): SIGINT/SIGTERM rufen einmal server.close(), warten auf die Store-Freigabe, dann Exit 0. Fehlschlag: Exit 1 plus eine Zeile Beenden fehlgeschlagen: <Code>. Zweites Signal während des Schließens: sofort Exit 1.
  • tools/denkraum/server.mjs (Close-Hook): kein emit('error') mehr aus dem Rejection-Handler. Der Fehler geht an den Callback, sonst genau eine Stderr-Zeile (nur der Fehlercode, kein Pfad). Scheitern natives Schließen und store.close(), bleibt der native Fehler als failure.cause ??= error erhalten.
  • tools/denkraum/store/core.mjs (#owned): error.cause ??= cause statt Überschreiben.
  • tools/denkraum/server-stop.test.mjs (neu, 6 Tests).

Evidence, Runde 0 (Arbeitsbaum, Head 88ac685)

  • Red-first: alle 6 neuen Tests an 35fe38e rot (Quellen zurückgesetzt, node --test tools/denkraum/server-stop.test.mjs: pass 0, fail 6), am Kopf grün (pass 6, fail 0).
  • node --test tools/denkraum/server-stop.test.mjs tools/denkraum/store/*.test.mjs → Exit 0 (pass 85, fail 0)
  • npm run test:denkraum → Exit 0 (Playwright 9 passed, Hygiene 0 Funde)
  • bash scripts/ci/red-first.sh → Exit 0
  • bash scripts/ci/gates.sh lane prepush → Exit 0 (214 s)
  • Trailer: 6x Test-First: tools/denkraum/server-stop.test.mjs::<Testname>, je ein neuer Test.

Architecture drift: keine neuen Treffer, keine Allowlist-Änderung.

NICHT ABGEDECKT

  • Windows: Signale verhalten sich dort anders; die beiden Signal-Prozess-Tests sind mit process.platform !== 'win32' übersprungen. Die Windows-Bahn läuft in der Merge-Queue; die #[cfg(windows)]-Tests sind auf Linux nicht übersetzt.
  • Absturz/kill -9 (verwaiste Besitzmarke) → G6.
  • R872-K2 (zwei Signale kurz hintereinander lassen die Besitzmarke zurück) → Folgepaket G6 (Stale-Owner-Recovery); hier nicht behoben.
  • Das Verlieren der Fehlerzeile auf einer Pipe (R872-K3) ist auf Linux nicht reproduzierbar (Pipes sind dort synchron); der Fix ist nur durch Review und die bestehende Stderr-Assertion belegt, Windows läuft in der Merge-Queue.
  • Admission-vor-listen und Drain laufender Anfragen → G4b. Offene aktive Verbindungen können das Schließen verzögern; das zweite Signal ist der Ausweg.

Review: Runde 1: GLM 5.2 approve (keine Findings), Kimi K3 approve mit Auflagen (unten). Runde 2 (Delta) wartet auf Root-Dispatch.

Report NACHBESSERN (Runde 1, Head 55bbb2c)

ID Quelle Schwere Befund Disposition
R872-K1 Kimi K3 medium server.mjs:163: SIGHUP (Terminal/ssh schließt) beendet den Prozess ohne Freigabe der Besitzmarke behoben in 5f59935: SIGHUP nutzt stop, nur wenn process.platform !== 'win32'; Test SIGHUP stop releases ownership… an 88ac685 rot (code: null, signal: 'SIGHUP'), am Kopf grün
R872-K2 Kimi K3 low Zwei Signale kurz hintereinander lassen die Besitzmarke zurück Folgepaket G6 (Stale-Owner-Recovery), nicht in diesem PR
R872-K3 Kimi K3 low server.mjs:159: console.error + sofortiges process.exit(1) kann die Zeile auf einer Pipe verlieren behoben in 2f89b0f: fs.writeSync(2, …) in try/catch vor dem Exit; bestehende Stderr-Assertion bleibt, Test deterministisch grün; kein neuer Test (auf Linux nicht reproduzierbar)
R872-K4 Kimi K3 low server.mjs:158-160: Stop nach fehlgeschlagenem Listen meldet Exit 0 behoben in 2f89b0f: Erfolgs-Exit nur mit listening-Flag (im listening-Handler gesetzt), sonst Exit 1 mit „nicht gestartet“; Test a stop after a failed listen exits non-zero an 88ac685 rot (code: 0), am Kopf grün (Preload hält die Event-Loop am Leben, damit das Signal sicher nach dem Listen-Fehler eintrifft)
(GLM) GLM 5.2 – approve, keine Findings –
Flake Prepush-Lauf – freePort() gab den Port vor dem Kind-Bind frei; ein paralleler Prozess nahm ihn (EADDRINUSE) 55bbb2c: startEntry wiederholt den Start bis 5-mal mit neuem Port; reiner Test-Helfer, No-Test:

Hinweis zum Ablauf: Der Pre-commit-Hook führt npm run test:denkraum aus; ein rein roter Test-Commit ist ohne --no-verify nicht möglich. Test und Fix stehen daher je Befund im selben Commit; die Rotprüfung gegen 88ac685 wurde lokal ausgeführt (Tests allein, Quelle unverändert).

Evidence, Runde 1 (Head 55bbb2c)

  • node --test tools/denkraum/server-stop.test.mjs tools/denkraum/store/*.test.mjs → Exit 0 (pass 87, fail 0); server-stop.test.mjs 3-mal hintereinander Exit 0
  • npm run test:denkraum → Exit 0
  • bash scripts/ci/red-first.sh → Exit 0
  • bash scripts/ci/gates.sh lane prepush → Exit 0 (319 s; ein früherer Lauf war wegen des oben genannten Port-Flakes rot und wurde nach dem Fix vollständig wiederholt)
  • Umfang: +161/−3 gegenüber der Basis (Limit 300).
  • Architecture drift: keine neuen Treffer, keine Allowlist-Änderung.

Review (Delta): Runde 2 wartet auf Root-Dispatch (Stufe A; die Autor-Familie Claude zählt nicht als Prüfer).

🤖 Generated with Claude Code

Root-Abnahme (09.10.2026 18:05Z, head 55bbb2c)

Tier A, author Claude (Sonnet 5.5). Reviewers outside the family: Kimi K3 (Moonshot) + GLM 5.2 (Zhipu), local Ollama. R1 on 88ac685: GLM approve (no findings), Kimi approve w/ conditions → fix1 (K1/K3/K4). R2 delta 88ac685..55bbb2c: both approve with conditions, no high.

ID Source Sev Finding Disposition
R872-K1/K3/K4 (r1) Kimi med/low/low SIGHUP, lossy stderr, exit 0 after failed listen accepted in fix1 (see table above)
R872-K2 (r1) Kimi med double signal leaves the owner record follow-up G6 (orphan recovery)
R872-r2-K1 Kimi med failed listen exits without close → owner record may remain rejected (verified): ownership is acquired lazily on the first owned operation (store/core.mjs:20-24, #owned), the notifier drain starts only on listening; a process that never listens never acquires. Restart-after-failed-listen test → follow-up G4b (admission before listen)
R872-r2-K2 Kimi low test retry decides on exit, not close (stdio may be incomplete) follow-up G4b test harness
R872-r2-K3 Kimi low marker wait without exit/timeout race can hang follow-up G4b test harness
R872-r2-G1 GLM low listen-error line still async console.error follow-up G4b
R872-r2-G2 GLM low failed-listen stop test does not assert the stderr line follow-up G4b
R872-r2-G3 GLM low SIGHUP widens the double-signal window follow-up G6 (same as R872-K2)

Two review rounds done (AGENTS.md rule 5); no open high; remaining items are low or verified-rejected and bound to G4b/G6. Ready + review-ok bound to 55bbb2c.

… errors

The CLI entry now handles SIGINT/SIGTERM by closing the server once and
waiting for the store release before exit; a second signal exits at once.
The close hook no longer emits 'error' from the rejection handler: the
failure goes to the callback or one stderr line, and a native close error is
kept as cause. Ownership release errors no longer overwrite an existing cause.

Test-First: tools/denkraum/server-stop.test.mjs::DRSEC-G4a: SIGTERM stop releases ownership and the next process can write
Test-First: tools/denkraum/server-stop.test.mjs::DRSEC-G4a: SIGINT stop releases ownership and the next process can write
Test-First: tools/denkraum/server-stop.test.mjs::DRSEC-G4a: a failing release on signal stop exits non-zero with a short stderr line
Test-First: tools/denkraum/server-stop.test.mjs::DRSEC-G4a: close without callback and failing release logs once and does not crash
Test-First: tools/denkraum/server-stop.test.mjs::DRSEC-G4a: dual close failure keeps the native error as cause of the release failure
Test-First: tools/denkraum/server-stop.test.mjs::DRSEC-G4a: a release failure does not overwrite the cause of the mutation error

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Base automatically changed from cursor/dr-sec-01g-g2-core to main October 9, 2026 17:10
@mergify

mergify Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 All 2 merge protections satisfied — ready to merge.

Show 2 satisfied protections

🟢 Paket-PR bringt seinen Bericht mit

Package branches (w<N>-, df<N>, ki-<N>, hq2-, dr-, z<N>-) must carry their report in the PR body, in a section that starts with "## Report".

  • body ~= (?m)^## Report

🟢 🚦 Auto-queue

When all merge protections are satisfied and these conditions match, this pull request will be queued automatically.

  • -conflict
  • -draft
  • base = main
  • check-success = gates (linux)
  • check-success = gates (windows)
  • check-success = red-first
  • label != do-not-merge
  • label = review-ok

Cuarroc and others added 3 commits October 9, 2026 19:14
A closed terminal or ssh session sends SIGHUP, which killed the entry
without releasing the ledger owner record. SIGHUP now takes the same stop
path as SIGINT/SIGTERM (POSIX only; Windows cannot deliver it).

Test-First: tools/denkraum/server-stop.test.mjs::DRSEC-G4a: SIGHUP stop releases ownership and the next process can write

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…e line synchronously

A stop signal that arrives after a failed listen exited 0 because the
native ERR_SERVER_NOT_RUNNING was treated as success; the clean exit is now
gated on the listening flag. The failure line goes through fs.writeSync(2)
so an immediate exit cannot drop it on a pipe (the existing stderr
assertion stays).

Test-First: tools/denkraum/server-stop.test.mjs::DRSEC-G4a: a stop after a failed listen exits non-zero

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…was taken

freePort() releases the port before the child binds it; under parallel
load another process can take it (EADDRINUSE seen in the prepush lane).
startEntry now retries that start up to five times on a fresh port.

No-Test: Test-helper only; the retry changes no assertion and the tests it serves stay as they were.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Cuarroc
Cuarroc marked this pull request as ready for review October 9, 2026 17:57
@Cuarroc Cuarroc added the review-ok Pipeline: review disposition complete, no open high finding label Oct 9, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T17:59:12.150452Z 55bbb2c Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@mergify

mergify Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

This pull request spent 19 minutes 52 seconds in the queue, including 13 minutes 9 seconds running CI.

Required conditions to merge

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

Labels

review-ok Pipeline: review disposition complete, no open high finding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant