Repository navigation
test(denkraum): cover CLI stop signal drain budgets - #1838
Conversation
Exercise real SIGTERM and SIGHUP delivery plus a SIGBREAK process-event bridge through the public CLI entry. Measure the existing six-second and four-second drain totals while preserving failure output and ledger ownership checks. No-Test: test-only coverage of correct existing stop wiring; no production defect or artificial red phase Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Cuarroc
added
the
review-ok
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f4ee7a202
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| assert.match(result.stderr, /^Entscheidungsseite: Beenden fehlgeschlagen: DRAIN_TIMEOUT$/m); | ||
| assert.equal(await missing(`${file}.owner`), false, 'a stuck send keeps the owner record (intended)'); | ||
| }); | ||
| for (const signal of ['SIGTERM', 'SIGHUP', 'SIGBREAK']) { |
There was a problem hiding this comment.
Because this commit completes the R908-FU-1 stop-to-drain wiring tests, docs/PLAN.md:444 would become stale: the V16-FU-2 row still says the package is not dispatched and whether it was completed elsewhere is unknown. Update that row in this change to record the delivered scope and leave only the genuinely outstanding sibling work; repository policy requires the Plan-ID row to describe the post-merge state before review-ok.
AGENTS.md reference: AGENTS.md:L139-L139
Useful? React with 👍 / 👎.
| // The specified totals are 6 s for SIGTERM and 4 s for console-close signals. A 500 ms lower | ||
| // tolerance and a 1.5 s upper margin distinguish those budgets without increasing either one. | ||
| const [min, max] = signal === 'SIGTERM' ? [5500, 7500] : [3500, 5500]; | ||
| assert.ok(elapsed >= min && elapsed < max, `${signal} drain must take ${min}–${max} ms, got ${elapsed} ms`); |
There was a problem hiding this comment.
The report embedded in commit 5adc89a3 says all commands ran against candidate 1f4ee7a2 and explicitly binds review-ok to that older SHA, even though 5adc89a3 is now the proposed commit. Thus the timing assertions added here have no gate or acceptance evidence bound to the actual candidate; re-run or explicitly revalidate the required evidence and review disposition against 5adc89a3 before marking it ready.
AGENTS.md reference: AGENTS.md:L119-L125
Useful? React with 👍 / 👎.
Cuarroc
removed
the
review-ok
|
Root (date -u ~15:25Z): review-ok removed. AGENTS.md:139 at main requires the PR's own PLAN row (docs/PLAN.md:444, V16-FU-2/R908-FU-1) to be updated in the same PR before review-ok (codex-bot P2 4349723741). Root had wrongly routed it to a later reconciliation. NACHBESSERN: a PLAN row update on this branch (tier C); then review-ok again on the new head. |
|
@Mergifyio dequeue |
Merge Queue Status
This pull request spent 2 minutes 13 seconds in the queue, with no time running CI. ReasonPull request #1838 has been dequeued Queue conditions are not satisfied:
HintYou should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it. Requeued — the merge queue status continues in this comment ↓. |
Update the owned V16-FU-2 status and preserve unresolved sibling duties. Record the observed queue failure without claiming a timing cause or fix. Plan-ID: V16-FU-2 / R908-FU-1 No-Test: Documentation maintenance only; original test behavior and bytes are unchanged. Co-Authored-By: Codex (gpt-6.1-sol) <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89e96c15bf
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Ursache, Fix oder Abschluss von KI-24. Weiter auf Windows messen, bevor Fristen | ||
| oder Wiederholungen geändert werden; kein Produktfix-Paket aus diesem Nachtrag. | ||
|
|
||
| ### Offener CI-Befund: selftest-win-signal ohne Timeout-Schlusszeile (22.10.2026) |
There was a problem hiding this comment.
This V16-FU-2/R908-FU-1 change now also records an unrelated selftest-win-signal failure from PR #1835 and creates a separate follow-up assignment, even though the package report scopes the change to CLI drain coverage and claims that only the drain test changed. Keeping this independent CI investigation here bundles two packages and leaves the report, review classification, and evidence describing a different delta; move the incident record to its own package.
AGENTS.md reference: AGENTS.md:L13-L17
Useful? React with 👍 / 👎.
| | V16-RELEASE | Tag v1.6.0 (Beta-Regel x.y.0) | Nutzer | Z1 (bis DR-17), V16-KI30 (Fix-Abnahme), D5-GUARD, V16-UPD-CANCEL, V16-06, V16-CHANGELOG; #728 gemergt | PC | – | Erledigt: Tag v1.6.0 = d2582ceb (09.10. 23:22Z), release.yml 38004103979 grün, latest.json anonym 1.6.0, PC installiert; ersetzt: Freigabe: Nutzer 09.10. ~19:42Z an Root; Tag durch Root nach Merge dieses PRs (V16-RELEASE-PREP); ersetzt: Geplant: E2/E3 beantwortet; konkrete Freigabe dieses Releases weiterhin offen | jeder Release braucht Nutzerfreigabe; Environment release mit Required Reviewer Cuarroc am 09.10. per gh api bestätigt; danach Release-Pipeline grün | – | | ||
| | V16-FU | Nachläufer aus v1.6.0-Reviews und Release-Prüfung | Stabschef | v1.6.0 veröffentlicht (V16-RELEASE) | Server | B/C je Punkt | Erledigt 10.10.: SRV-2/SRV-5-Portschutz #931, SRV-3 #952, OWN-3 #930, NOT-1 und R912-FU-1 #941, NOT-2/NOT-3 #928, SRV-4 #927, REL-3 #926, REL-4 #929, R910-FU #937, R889-FU #899 (09.10.); Rest verschoben nach V16-FU-2; ursprünglich: Offen: R908-FU-1 (Wiring-Test stop→limitDrainForSignal), R908-FU-2 (Drain-Kommentar/Idempotenz); R912-FU-1 (existed-Flag statt stat, io-Seam); R910-FU, R896-FU, R899-FU K2, R889-FU; REL-3 (scripts/release.cmd veraltet), REL-4 (Rollback/releases-latest), NOT-1/2/3, SRV-2..5, OWN-3 | je Punkt eigenes Paket mit eigener Abnahme | – | | ||
| | V16-FU-2 | Rest aus V16-FU: SRV-5 gegenseitige Authentifizierung (laut #931 weiterhin geparkt), R908-FU-1 (Wiring-Test stop→limitDrainForSignal), R908-FU-2 (Drain-Kommentar/Idempotenz), R896-FU, R899-FU K2 | Stabschef | V16-FU | Server | B/C je Punkt | Geplant: nicht disponiert; ob schon anderswo erledigt: Unbekannt (zu prüfen) | je Punkt eigenes Paket mit eigener Abnahme | – | | ||
| | V16-FU-2 | Rest aus V16-FU: SRV-5 gegenseitige Authentifizierung (laut #931 weiterhin geparkt), R908-FU-1 (Wiring-Test stop→limitDrainForSignal), R908-FU-2 (Drain-Kommentar/Idempotenz), R896-FU, R899-FU K2 | Stabschef | V16-FU | Server | B/C je Punkt | In Arbeit: Teillieferung R908-FU-1 nach Merge von #1838: Wiring-Test am echten CLI-Einstieg für Linux-SIGTERM/SIGHUP und prozessseitig emittiertes SIGBREAK; kein nativer Windows-Signalnachweis. Neuer Kopf wartet auf erneute Root-Abnahme. SRV-5 gegenseitige Authentifizierung bleibt geparkt; R908-FU-2, R896-FU und R899-FU K2 bleiben offen | je Punkt eigenes Paket mit eigener Abnahme | – | |
There was a problem hiding this comment.
The fresh replacement text still mixes mutually exclusive states: it describes R908-FU-1 as delivered “after merge of #1838” and then says the new head is awaiting Root acceptance. Once this commit is merged, that acceptance cannot still be pending, so the canonical row immediately becomes stale; retain the delivered scope and outstanding sibling work but remove the pre-merge approval status.
AGENTS.md reference: AGENTS.md:L139-L139
Useful? React with 👍 / 👎.
| assert.equal(await missing(`${file}.owner`), false, 'a stuck send keeps the owner record (intended)'); | ||
| }); | ||
| for (const signal of ['SIGTERM', 'SIGHUP', 'SIGBREAK']) { | ||
| test(`DRSEC-G4b: ${signal} with a notification send that never answers prints one short line and exits non-zero and keeps the owner record`, { skip: posixOnly }, async t => { |
There was a problem hiding this comment.
On Windows, expanding the former single SIGTERM test into this loop introduces two additional tests, for SIGHUP and SIGBREAK, that are skipped via posixOnly. The report nevertheless classifies the package as Tier C and says gates suffice; repository policy explicitly makes tests that add skips C+, so this candidate needs one outside review before review-ok rather than the recorded gates-only disposition.
AGENTS.md reference: AGENTS.md:L33-L37
Useful? React with 👍 / 👎.
Cuarroc
added
the
review-ok
|
@Mergifyio queue |
Merge Queue Status
This pull request spent 41 minutes 2 seconds in the queue, including 40 minutes 22 seconds running CI. Required conditions to merge
|
Die PLAN-Zeile beschreibt jetzt die begrenzte R908-FU-1-Teillieferung und lässt alle übrigen Pflichten offen.
Der beobachtete CI-Fehler ist als offener Befund mit klaren Beleggrenzen dokumentiert.
Die vollständige lokale Prepush-Bahn und der Push-Hook sind grün; erneute Root-Abnahme und Windows-Nachweis bleiben offen.
Report
Plan-ID: V16-FU-2 / R908-FU-1
Current documentation correction (2026-10-22)
89e96c15bf3fed0c566276bbfe07caad7fe266cf; existing branchcodex/v16-r908-fu1, existing PR test(denkraum): cover CLI stop signal drain budgets #1838 only. Documentation delta:1f4ee7a20275c5cf3cb7c8bc7b97f5f05ca513da..89e96c15bf3fed0c566276bbfe07caad7fe266cf.88c10c7716ad2dca629930adbcd8a291baf5d858. Keep this separate from the historical GitHub-reported PR base tip72ae3f7582f2c5e877f0d76d281856e28c563369; fetchedorigin/mainwas5a07fc0895ff6bb813df265fa9d730d38d1b1ed9during this correction. No freshness merge, rebase, history rewrite or new PR.In Arbeit:: partial R908-FU-1 delivery through test(denkraum): cover CLI stop signal drain budgets #1838 after merge, real Linux SIGTERM/SIGHUP and process-emitted SIGBREAK coverage, no native Windows signal proof. Root reacceptance remains pending. SRV-5 mutual authentication stays parked; R908-FU-2, R896-FU and R899-FU K2 remain unresolved. Acceptance/dependency columns, all other PLAN bytes (including M4B and FU-1015), existing issue content and KI-24 remain unchanged.Behoben: queue merge queue: checking #1835 on main (fa4f2d9) #1847 / fix(diagrams): preserve literal SVG comments in CDATA (FU-1015) #1835 atfc1d4662onfa4f2d9a, run 39834041335 / job 120017507651, failurenever-finishing-run-times-out-with-3 (rc=3). The exact stdout timeout line was missing according to Root's raw-log inspection; the expected rc 3 matched. Coordinator independently read job failure and source predicate; raw excerpt is attributed to Root because gh refused raw terminal escapes. The existing one-second poll/no-sleep predicate is recorded; timing race/flake remains a hypothesis.fa4f2d9aselected no Linux execution: actual gates were skipped. Root requeued through merge queue: checking #1835 on main (fa4f2d9) #1848; fix(diagrams): preserve literal SVG comments in CDATA (FU-1015) #1835 merged at 19:07:35Z as5a07fc08. Neither selection success nor merge is asserted as a PASS of that specific selftest or as cause/fix proof. Follow-upd2d1a8a8-0882-435e-902b-c3cad4e695fbstays open/low/review-only; no TASK/ASSIGN fix.Authority and ownership: Root's accepted own-row correction
msg-0mvjouwi8-576f5f7f/ observer 4349723741, distinct-row permissionmsg-0mvjxdk5i-ebaea90d, issue-record requestmsg-0mvjwjejw-e16b0a1f; existing ticket29256698-c005-497c-ba05-db5d61e4d325. Mechanical edit dispatched/reserved by Stabschefagent-1792535935707-63zow9; supervisory Chief-of-Staffagent-chief-935795b19cd8b31fsupplies oversight only. Root owns priority, reacceptance and Mergify. No other row or package is rewritten.Native goal / runtime evidence for this job: own
get_goalinitially returnedgoal=null;create_goalestablished the bounded documentation/push/report/handover goal, and subsequentget_goalconfirmed it active. This is new job evidence, not a continuity claim about another session's goal. Parent V16-FU-2 remains open. Ownturn_contextreportsgpt-6.1-sol, efforthigh; the CLI runtime header reports OpenAI Codex v0.163.0, provideropenai, same model/effort. No AgentsRoom memory/graph tool orlage-jetztmirror was available; use fixed source and GitHub evidence, without an exhaustive runtime/graph claim. No app activation or reviewer launch.Current evidence (2026-10-22, final head)
python3 -(byte assertions againstgit show 1f4ee7a2:<file>)e59c94e6b75aa63c3b9c190fff98fa5a5277766fbaca82a07300ef8505b81823. Only the two authorized documentation files changed in this delta.node --input-type=module(existingparseGoals, before/after assertions)In Arbeit, PR 1838; no changed sibling rows, problems or existing invalid statuses.git diff --check 88c10c7716ad2dca629930adbcd8a291baf5d858 HEADgit diff --numstatproves the 81-line package.bash scripts/ci/secret-scan.shgit commit(normal hooks)No-Testand observed-model coauthor trailers recorded.bash scripts/ci/gates.sh lane prepushselftest-win-signalpassed as part of the canonical lane.bash scripts/ci/red-first.shNo-Testaccepted. No invented red phase.git push -u origin codex/v16-r908-fu1git ls-remote origin codex/v16-r908-fu1gh run view 39834041335 --json headSha,conclusion,jobs;gh run view 39832698357 --json headSha,conclusion,jobs;gh pr view 1835 --json mergedAt,mergeCommit;gh pr view 1847/1848gh pr view 1838 --json headRefOid,mergeable,isDraft,labelsdequeued; final handover snapshot has no labels. The author issued no label/queue mutation; reason for the intervening label change was not verified.An initial final-handover assertion exited 1 because it compared label equality with the start snapshot (
dequeuedthen, no labels at handover). Body/head equality had passed. No code/test check failed; no author label mutation was issued. Final body/head/clean-tree verification and the observed label state are recorded separately.Fresh admission preceded commit, full prepush and push: load1 below 8, MemAvailable above 3500 MiB, no other observed Cargo build or gate phase, and
npm run dev:start-check -- --max-cargo 2 --min-free-gb 3.5exit 0. Provider quota was checked separately against the fresh private record; start-check without--usagedoes not claim provider verification. No extra targeted signal run or timing measurement was started for this documentation change; mandatory canonical lanes reran existing tests.Architecture drift: no new finding and no allowlist change.
NICHT ABGEDECKT (current correction)
#[cfg(windows)]tests or Windows clippy arms. The Windows lane remains for the merge queue; native Windows console-close/Ctrl+Break delivery is unproven. SIGBREAK coverage here is process-emitted on Linux.Current disposition and next owner
Review: ausstehend (Stufe C). This documentation correction is gates-only; no foreign-model review is required or launched. Prior Root acceptance at
1f4ee7a2was withdrawn for the own-PLAN duty. Observer 4349723741 is addressed by89e96c15; final Root reacceptance is pending and supersedes the earlier follow-up-only disposition below. Historical test/runtime evidence and all earlier dispositions are preserved below with their original head/date.Worker delivery: documentation implemented, checked and pushed; report handed over. Root must reaccept this exact head and handle labels/queue/Mergify. The author changed no ready state, label or queue setting and performed no final merge. V16-FU-2 and its unresolved siblings remain open.
Historical report at 1f4ee7a (preserved; superseded where stated above)
Plan-ID: V16-FU-2 / R908-FU-1
Goal / benefit: protect the public stop-to-drain signal wiring with behavioral coverage.
Parent / ticket / owner: V16-FU-2; ticket
29256698-c005-497c-ba05-db5d61e4d325; assigned package worker, Root acceptance.Native evidence: this resumed session returned
get_goal: goal=null, thencreate_goalestablished the bounded package-delivery goal;get_goalconfirmed it active during verification. The previous session's reported unmet goal was not replaced or marked complete. The worker goal ends at verified draft handover; parent acceptance remains open.Adaptation: resume the preserved local commit after the prior load-admission stop, under the superseding two-heavy-job/load-below-eight rule. No implementation delta was needed; complete full local checks, normal push and draft publication.
Change and boundary
tools/denkraum/server-entry-drain.test.mjschanges: 32 additions + 19 deletions = 51 raw lines against merge-base88c10c7716ad2dca629930adbcd8a291baf5d858.1f4ee7a20275c5cf3cb7c8bc7b97f5f05ca513da; branch:codex/v16-r908-fu1; base branch:main(observed at72ae3f7582f2c5e877f0d76d281856e28c563369on 2026-10-22).server.mjsis unchanged.DRAIN_TIMEOUTline, retained ledger ownership and the corresponding time range. Production limits, existing retry/timeouts and platform exclusions are preserved.No-Testtrailer is accepted by red-first.Evidence (2026-10-22)
Commands ran in the assigned worktree at the candidate head; no hook bypasses.
node --test tools/denkraum/server-entry-drain.test.mjsbash scripts/ci/gates.sh lane prepushbash scripts/ci/red-first.shNo-Testaccepted.bash scripts/ci/secret-scan.shgit diff --check 88c10c7716ad2dca629930adbcd8a291baf5d858 HEADgit push -u origin codex/v16-r908-fu1git ls-remote origin codex/v16-r908-fu1Measured in the explicit full prepush run: SIGTERM 6009 ms, SIGHUP 4012 ms, simulated SIGBREAK 4013 ms. During the push hook: 6013 / 4012 / 4015 ms respectively.
Architecture drift: no new finding or allowlist change.
NICHT ABGEDECKT
#[cfg(windows)]tests or Windows clippy arms; the Windows lane remains for the merge queue.Review and remaining ownership
Review: ausstehend (Stufe C). Test-only coverage; the task requests gates only and no external review. Root owns final classification and acceptance.
The canonical PLAN row 444 at this frozen candidate still says not dispatched/unknown. Only the test file is authorized here; Root must reconcile the canonical V16-FU-2 row and remaining sibling duties before acceptance. This PR is a checked, pushed draft: Root acceptance, ready status, review label, queue and merge are not done.
Review disposition (Root, 2026-10-22 ~14:40Z, head 1f4ee7a)
Tier C: a single test file (
tools/denkraum/server-entry-drain.test.mjs, +32/-19), no runtime effect, so the gates suffice and no external review is required. Root read the diff: the test-only claim holds, and theNo-Testtrailer fits (it adds coverage of existing behaviour). Noted risk, not blocking: the timing window (SIGTERM 5.5–7.5 s, console-close 3.5–5.5 s) can turn flaky on a heavily loaded runner. If it does, it goes to KNOWN_ISSUES and is not retried blindly. Root decision: accept;review-okbound to 1f4ee7a.Codex bot comments (OpenAI, author family, not counted as review), Root 14:58Z:
git cat-filefails), and the branch holds exactly one commit, 1f4ee7a (gh pr view --json commits,git ls-remote). Evidence andreview-okare bound to the actual head.Codex observer on 89e96c1 (Root disposition, 2026-10-22 21:31Z)
KNOWN_ISSUES.md:89: win-signal incident bundled into this packagemsg-0mvjwjejw-e16b0a1f); it is 28 documentation lines, tier C, and the report above describes it under "Current documentation correction". Splitting would add a CI round without reducing risk. The investigation itself stays in follow-up d2d1a8a8.docs/PLAN.md:444: status cell mixes post-merge delivery with "awaiting Root acceptance"In Arbeit; the stale clause "Neuer Kopf wartet auf erneute Root-Abnahme" is removed by the PLAN owner (Stabschef) in the next V16-FU-2 sibling package. No push to this head.tools/denkraum/server-entry-drain.test.mjs:99: two new tests (SIGHUP, SIGBREAK) carryskip: posixOnly, so the package is C+, not CC+ outside review on 89e96c1 (glm-5.3, Zhipu; Root disposition 2026-10-22 21:50Z)
Review: glm-5.3:cloud via fail-closed
review-seq.sh, admission sample 21:36:07Z load1=4.26, report time 21:40:35Z, prompt sha256 570cc15e43ff7fca, diff base 88c10c7. (A first attempt at 21:30Z failed with HTTP 404 because Root passed the model name without the:cloudtag; no verdict, not counted.) Verdict: approve with conditions.server-entry-drain.test.mjs:125-126: wall-clock windows may flake on slow runnersDRSEC-G4b: <signal> with a notification send …cases ✔ on 89e96c1 merged with main, plusDR-WIN-STOP: the stop path listens for SIGINT SIGTERM S…✔; the budgets are existing production behaviour (R908, already on the merge base), this package is coverage only (justifiedNo-Test, red-first SUCCESS). The SIGBREAK = 4 s class is what production implements and the green case proves it.child.send()result / ref'd IPC channelRoot acceptance: tier C+ requirement met (one non-OpenAI outside review on this exact head), no open high finding, conditions evidenced. review-ok bound to 89e96c1.