Repository navigation
docs(mt#2313): SessionEnd reliability docs: /clear fires (measured), /exit unmeasured - #3753
Conversation
…ed — /clear fires, /exit unmeasured The hook header, its architecture page, ADR-017 (amendment note), two settings.json _comments and the run-state schema docblock all said "/exit and /clear do NOT fire SessionEnd" (from Claude Code issue threads). Measured 2026-09-13 in conversation_run_state.ended_hint_reason: clear 340, other 369, prompt_input_exit 50, logout 1, resume 0 — /clear fires. /exit has no reason value to measure by and stays labelled unmeasured; crash/SIGKILL never fires; a tab-close SIGHUP can kill the hook mid-ingest. Conclusion unchanged: the watcher + sweep are the guarantee, the hook is the latency optimization. Comments and docs only; the compiled hook regenerated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147JJ3SVgqRz9JyUD4NZBz9
Minsky Reviewer StatusVerdict: APPROVED — no blocking findings Commands
|
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Docs-only PR largely aligns the hook headers, ADR-017, and the hook doc with the measured reliability boundary and ADR framing. However, one deploy-surface schema JSDoc remains stale: packages/domain/src/storage/schemas/conversation-run-state-schema.ts still asserts “/exit and /clear do not fire SessionEnd,” contradicting this PR’s measured correction that /clear DOES fire (340 rows). This is a blocking documentation defect that will mislead future readers. Non-blocking notes: clarify the resume reason’s zero-count in the measured window, and broaden the grep-based acceptance checks to catch phrasing variants so stale claims don’t slip through. SC4 (compile exclusivity) is unverifiable here; others are met. Please fix the schema JSDoc and consider the two non-blocking suggestions before merge.
Findings
- [BLOCKING] packages/domain/src/storage/schemas/conversation-run-state-schema.ts:160 — Field-level JSDoc still claims “/exit and /clear do not fire SessionEnd” — contradicts this PR’s measured correction
Inpackages/domain/src/storage/schemas/conversation-run-state-schema.ts, the JSDoc on theendedHintAtcolumn still reads: “A HINT — never as an authoritative "ended". … the cleanest exits (/exit,/clear) do not fire it.” This directly contradicts the updated header comment above and the ADR/doc changes in this PR stating that/clearDOES fireSessionEnd(340 measured rows). This stale assertion is user-facing documentation embedded in a deploy-surface schema file and will mislead future readers. Please update theendedHintAtJSDoc to match the corrected stance: crash/SIGKILL never fire; tab-close SIGHUP can kill mid-ingest;/exitis unmeasured;/clearDOES fire (measured). Also consider adding the measurement pointer consistent with the header update. - [NON-BLOCKING] docs/architecture/hooks/session-end-transcript-ingest-hook.md:60 — Reliability boundary lists
resumein hooks reference but not in measured coverage; clarify whether it ever fires and why it’s listed
The doc states the hooks reference listsclearandresumeasreasonvalues, but the measured table showsresume 0. It’s fine to report zero, but consider a brief note clarifying thatresumeis a documented reason that simply didn’t occur in the measured window, to reduce reader ambiguity about whether it should be considered part of "Covers". - [NON-BLOCKING] .claude/settings.json:430 — Grepped acceptance tests may miss stale claims due to phrasing differences
AT2/Success Criteria rely ongrep -n "do NOT fire SessionEnd"to detect stale claims. The remaining stale field-level JSDoc uses the phrase “do not fire it,” which would not match the grep and can slip through. Consider broadening the acceptance test string to cover phrasing variants (e.g.,grep -Ei "do\s+not\s+fire") or asserting positive presence of the new measured wording instead.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
1. The hook's reliability boundary — header of .minsky/hooks/transcript-ingest-on-session-end.ts (compiled to .claude/hooks/) and docs/architecture/hooks/session-end-transcript-ingest-hook.md §Reliability boundary — no longer says "Covers sessions that end normally". It states what is measured and what is not: SessionEnd fires on /clear (reason clear, 340 rows 2026-07-29 → 2026-09-13), logout, prompt_input_exit and other; /exit is UNMEASURED (no reason value distinguishes it; other may include it; Claude Code issues #17885/#6428 say it does not fire); a crash / SIGKILL never fires it; a SIGHUP (tab close) can kill the hook mid-ingest (#41577); a mid-tool kill leaves a dangling tool_use. Does NOT cover lists crash/SIGKILL, kill-before-completion, and /exit (unmeasured) — each backstopped by the mt#2051 boot sweep and mt#2234 watcher/sweep. |
Met | Updated header in .minsky/hooks/transcript-ingest-on-session-end.ts:22-38 and compiled twin .claude/hooks/transcript-ingest-on-session-end.ts:24-40 enumerate measured reasons (explicitly /clear with 340 rows) and unmeasured /exit, plus SIGKILL/SIGHUP caveats. docs/architecture/hooks/session-end-transcript-ingest-hook.md:33-71 reflects the same and removes the prior "ends normally" claim. |
| 2. Each surface names the hook a non-load-bearing latency optimization per ADR-017 and cross-references ADR-017. | Met | Hook headers state: "ADR-017 makes the coverage guarantee — this hook is the non-load-bearing latency optimization" in .minsky/hooks/transcript-ingest-on-session-end.ts:34-38 and .claude/hooks/transcript-ingest-on-session-end.ts:36-40. The doc page reiterates this framing at docs/architecture/hooks/session-end-transcript-ingest-hook.md:63-71. |
3. ADR-017 gains an amendment note under its "inverted finding" paragraph: /clear DOES fire SessionEnd (with the measurement and the hooks-reference citation); the /exit claim stands as unmeasured; the conclusion (hook = fast-path, watcher/sweep = guarantee) is unchanged. The two .claude/settings.json _comments that repeat "/exit and /clear do NOT fire SessionEnd" (mt#4035 and mt#3161 entries) and the conversation-run-state-schema.ts docblock (lines ~36–40) are corrected the same way. |
Met | ADR amended under the inverted-finding block at docs/architecture/adr-017-transcript-capture-continuous-watch.md:29-46. .claude/settings.json:430-447 updates both _comment strings to reflect measured /clear and caveats. packages/domain/src/storage/schemas/conversation-run-state-schema.ts:34-48 docblock corrected to mark /clear as firing and /exit unmeasured. |
4. mcp__minsky__compile regenerates the hook's compiled twin; git status shows only that generated file changed among generated outputs. |
Unverifiable | The diff shows .claude/hooks/transcript-ingest-on-session-end.ts updated to match the source header, but we cannot run mcp__minsky__compile or inspect git status from this review context to confirm exclusivity of generated changes. |
Documentation impact
- no-update-needed — This PR is exclusively comments and internal documentation updates aligning reliability boundaries across the hook headers, ADR-017, and the session-end hook doc page. No behavior or public API changed. I reviewed the changed docs themselves (
docs/architecture/hooks/session-end-transcript-ingest-hook.md,docs/architecture/adr-017-transcript-capture-continuous-watch.md) and the inline JSDoc inpackages/domain/src/storage/schemas/conversation-run-state-schema.ts. Apart from the BLOCKING stale sentence flagged in findings forendedHintAt, the rest of the updated prose is internally consistent with the PR’s goal. No separate external docs outside the ones updated here need changes.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
R2 verification: Most surfaces now reflect the measured reliability boundary and ADR-017 framing — the hook headers, ADR-017 amendment, and the hook doc page are consistent, and settings.json comments were corrected. However, one deploy-surface field-level JSDoc in conversation-run-state-schema.ts still asserts “the cleanest exits (/exit, /clear) do not fire it,” contradicting the new stance that /clear DOES fire (340 measured rows) — this remains a blocking documentation defect. Non-blocking notes: optionally clarify the resume reason’s 0 count in the measured window, and consider broadening grep-based acceptance checks to avoid phrasing gaps. Fix the stale endedHintAt JSDoc and this is ready to merge.
Findings
- [BLOCKING] packages/domain/src/storage/schemas/conversation-run-state-schema.ts:150 — Field-level JSDoc for
endedHintAtstill claims “the cleanest exits (/exit,/clear) do not fire it” — contradicts this PR’s measured correction that/clearDOES fire
Inpackages/domain/src/storage/schemas/conversation-run-state-schema.ts, the JSDoc immediately aboveendedHintAtstill reads: “A HINT: its absence proves nothing, because the cleanest exits (/exit,/clear) do not fire it.” This contradicts the updated header comment in the same file and the hook/ADR/docs in this PR, which state that/clearDOES fireSessionEnd(340 measured rows). Please update this field-level comment to match the corrected stance already captured earlier in the file: crash/SIGKILL never fire; tab-close SIGHUP can kill the hook mid-ingest;/exitis unmeasured; and/clearDOES fire (with the measurement reference). Leaving this stale assertion in a deploy-surface schema file risks misleading future readers. - [NON-BLOCKING] .minsky/hooks/transcript-ingest-on-session-end.ts:36 — Optional clarification:
resumelisted in hooks reference but 0 measured — consider a brief parenthetical
The reliability boundary notes that the hooks reference listsresume, and your measured table showsresume 0. Consider adding a brief parenthetical (e.g., “resumelisted in hooks reference; 0 occurrences in the measured window”) to preempt confusion. Not required for merge. - [NON-BLOCKING] .claude/settings.json:430 — Acceptance-test greps may miss stale claims with phrasing variants
Your acceptance checks rely on grepping for the exact phrase "do NOT fire SessionEnd". The staleendedHintAtcomment that slipped through uses different phrasing ("do not fire it"). Consider broadening the acceptance checks (e.g.,grep -Ei "do\s+not\s+fire") or, better, assert positive presence of the new measured wording to reduce false negatives. Not required for merge.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
1. The hook's reliability boundary — header of .minsky/hooks/transcript-ingest-on-session-end.ts (compiled to .claude/hooks/) and docs/architecture/hooks/session-end-transcript-ingest-hook.md §Reliability boundary — no longer says "Covers sessions that end normally". It states what is measured and what is not: SessionEnd fires on /clear (reason clear, 340 rows 2026-07-29 → 2026-09-13), logout, prompt_input_exit and other; /exit is UNMEASURED (no reason value distinguishes it; other may include it; Claude Code issues #17885/#6428 say it does not fire); a crash / SIGKILL never fires it; a SIGHUP (tab close) can kill the hook mid-ingest (#41577); a mid-tool kill leaves a dangling tool_use. Does NOT cover lists crash/SIGKILL, kill-before-completion, and /exit (unmeasured) — each backstopped by the mt#2051 boot sweep and mt#2234 watcher/sweep. |
Met | .minsky/hooks/transcript-ingest-on-session-end.ts:32-48 — header rewritten to enumerate measured reasons (explicit /clear with 340 rows) and unmeasured /exit, plus SIGKILL/SIGHUP caveats; .claude/hooks/transcript-ingest-on-session-end.ts:35-51 mirrors it. docs/architecture/hooks/session-end-transcript-ingest-hook.md:51-86 removes the "ends normally" language and states measured coverage/caveats. |
| 2. Each surface names the hook a non-load-bearing latency optimization per ADR-017 and cross-references ADR-017. | Met | .minsky/hooks/transcript-ingest-on-session-end.ts:44-48 — "ADR-017 makes the coverage guarantee — this hook is the non-load-bearing latency optimization"; .claude/hooks/transcript-ingest-on-session-end.ts:47-51 mirrors it. docs/architecture/hooks/session-end-transcript-ingest-hook.md:82-86 reiterates ADR-017 framing. |
3. ADR-017 gains an amendment note under its "inverted finding" paragraph: /clear DOES fire SessionEnd (with the measurement and the hooks-reference citation); the /exit claim stands as unmeasured; the conclusion (hook = fast-path, watcher/sweep = guarantee) is unchanged. The two .claude/settings.json _comments that repeat "/exit and /clear do NOT fire SessionEnd" (mt#4035 and mt#3161 entries) and the conversation-run-state-schema.ts docblock (lines ~36–40) are corrected the same way. |
Met | docs/architecture/adr-017-transcript-capture-continuous-watch.md:33-52 — new "Amended 2026-09-14" block records /clear DOES fire (340 rows), /exit unmeasured, conclusion unchanged; :182-186 marks issues superseded for /clear. .claude/settings.json:430-447 updates both _comment strings. packages/domain/src/storage/schemas/conversation-run-state-schema.ts:17-28 docblock reflects the corrected stance. Note: a separate field-level JSDoc later in this file remains stale (flagged in findings), but the named header docblock is corrected per the criterion. |
4. mcp__minsky__compile regenerates the hook's compiled twin; git status shows only that generated file changed among generated outputs. |
Unverifiable | We cannot run mcp__minsky__compile or inspect git status in this review context. The diff shows .claude/hooks/transcript-ingest-on-session-end.ts updated in lockstep with the source header, but exclusivity of generated changes cannot be verified here. |
Documentation impact
- no-update-needed — Docs-only PR: updates internal docs and comments to align reliability boundaries across hook headers, ADR-017, and the hook doc page. No behavior or API changed. I reviewed the changed docs (
docs/architecture/hooks/session-end-transcript-ingest-hook.md,docs/architecture/adr-017-transcript-capture-continuous-watch.md) and inline JSDoc inpackages/domain/src/storage/schemas/conversation-run-state-schema.ts. Apart from the one stale field-level JSDoc (flagged blocking), the rest of the updated prose is consistent; no additional external documentation changes are needed.
…r do not fire" site The claim had spread to 13 more places: the run-state schema's field JSDoc (the blocking finding), event-mapping.ts + its test, presence.ts, the guard-events ingest service, command and sweeper, the guard-events SessionEnd hook, the turn-end guard registry, both interceptor registries (catalog regenerated), and the knowledge-acquisition detector doc. Each now says: never fires on a crash/SIGKILL, can be killed mid-run by a tab-close SIGHUP, unmeasured for /exit, does fire on /clear. The hook doc also labels `resume` documented-but-unobserved. Repo-wide grep for the old phrasing family returns nothing outside the amendment text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147JJ3SVgqRz9JyUD4NZBz9
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Re-verified against the updated spec: the prior BLOCKING issue in conversation-run-state-schema.ts is addressed — the docblock now reflects measured behavior (/clear fires; /exit unmeasured) and corrects the earlier overclaim. The hook header and the dedicated hook doc both state the measured reliability boundary and frame the hook as a latency optimization per ADR-017. ADR-017 includes an explicit amendment note. Additional comment updates across guard-events, interceptors, presence mapping, and generated catalogs are consistent and non-behavioral. No new critical defects were introduced. One criterion requiring a live compile/git status run is unverifiable from this diff; all others are met. Approving as documentation/comment-only corrections with no runtime changes.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
The hook's reliability boundary — header of .minsky/hooks/transcript-ingest-on-session-end.ts (compiled to .claude/hooks/) and docs/architecture/hooks/session-end-transcript-ingest-hook.md §Reliability boundary — no longer says "Covers sessions that end normally". It states what is measured and what is not: SessionEnd fires on /clear (reason clear, 340 rows 2026-07-29 → 2026-09-13), logout, prompt_input_exit and other; /exit is UNMEASURED (no reason value distinguishes it; other may include it; Claude Code issues #17885/#6428 say it does not fire); a crash / SIGKILL never fires it; a SIGHUP (tab close) can kill the hook mid-ingest (#41577); a mid-tool kill leaves a dangling tool_use. Does NOT cover lists crash/SIGKILL, kill-before-completion, and /exit (unmeasured) — each backstopped by the mt#2051 boot sweep and mt#2234 watcher/sweep. |
Met | Verified header in .minsky/hooks/transcript-ingest-on-session-end.ts states measured /clear ("340 deliveries measured"), names logout, prompt_input_exit, other, marks /exit UNMEASURED, and lists crash/SIGKILL and SIGHUP kill-before-completion (see lines ~24–47 of the header in the opened file). The doc page docs/architecture/hooks/session-end-transcript-ingest-hook.md similarly replaces “ends normally” with a measured list and explicitly marks /exit UNMEASURED and crash/SIGKILL/SIGHUP as not covered (see the updated “Reliability boundary” section in that file). |
| Each surface names the hook a non-load-bearing latency optimization per ADR-017 and cross-references ADR-017. | Met | .minsky/hooks/transcript-ingest-on-session-end.ts header: “ADR-017 makes the coverage guarantee — this hook is the non-load-bearing latency optimization for the exits that do fire.” (header tail). docs/architecture/hooks/session-end-transcript-ingest-hook.md: “Per ADR-017 the watcher + sweep are the coverage guarantee; this hook is the non-load-bearing latency optimization…” (Reliability boundary section). Additional corroboration in .claude/hooks/guard-events-ingest-on-session-end.ts and .minsky/hooks/guard-events-ingest-on-session-end.ts comments reiterate the latency-only framing with ADR-017 refs. |
ADR-017 gains an amendment note under its "inverted finding" paragraph: /clear DOES fire SessionEnd (with the measurement and the hooks-reference citation); the /exit claim stands as unmeasured; the conclusion (hook = fast-path, watcher/sweep = guarantee) is unchanged. The two .claude/settings.json _comments that repeat "/exit and /clear do NOT fire SessionEnd" (mt#4035 and mt#3161 entries) and the conversation-run-state-schema.ts docblock (lines ~36–40) are corrected the same way. |
Met | docs/architecture/adr-017-transcript-capture-continuous-watch.md includes an explicit “Amended 2026-09-14 (mt#2313)” block under the inverted finding stating /clear DOES fire, /exit unmeasured, and that the conclusion is unchanged. .claude/settings.json SessionEnd-related _comments now say “never fires on a crash/SIGKILL … /exit is unmeasured … DOES fire on /clear — 340 measured” in the SessionEnd block (see SessionEnd entries). packages/domain/src/storage/schemas/conversation-run-state-schema.ts header docblock updated to reflect /clear does fire with measurement and /exit unmeasured (top-of-file comment, lines ~24–43) and the endedHintAt field JSDoc updated accordingly (lines ~168–184). |
mcp__minsky__compile regenerates the hook's compiled twin; git status shows .claude/hooks/transcript-ingest-on-session-end.ts as the only generated file modified. |
Unverifiable | Requires running the compile step and inspecting git status, which cannot be performed from this diff. Additionally, other generated artifacts (e.g., src/generated/interceptor-catalog.json, src/generated/completion-manifest.json) were updated in this PR due to text changes elsewhere, so live-run confirmation is needed to assert the "only generated file modified" condition. |
Documentation impact
- updated-in-pr — This PR is a documentation/comment-only correction: it amends ADR-017, updates the hook header and the dedicated hook doc’s Reliability boundary, and corrects comments in multiple code and settings files to reflect measured SessionEnd behavior (
/clearfires;/exitunmeasured). No behavior or API changes were made. Docs updated in-repo includedocs/architecture/adr-017-transcript-capture-continuous-watch.mdanddocs/architecture/hooks/session-end-transcript-ingest-hook.md; comments in.claude/settings.jsonand code docblocks were aligned accordingly.
Affected: docs/architecture/adr-017-transcript-capture-continuous-watch.md, docs/architecture/hooks/session-end-transcript-ingest-hook.md
Cockpit PreviewPreview URL: https://cockpit-preview-production.up.railway.app Deployed from commit Updated by Cockpit Preview. |
Summary
The SessionEnd transcript-ingest hook documented its reliability boundary as "covers sessions that end normally", and ADR-017 (plus two
settings.jsoncomments and the run-state schema docblock) said "/exitand/cleardo NOT fire SessionEnd" — both taken from Claude Code issue threads, neither measured. Measured 2026-09-13 inconversation_run_state.ended_hint_reason(760 rows since 2026-07-29):clear340,other369,prompt_input_exit50,logout1,resume0 —/clearfires./exithas noreasonvalue to measure by and is labelled unmeasured; a crash/SIGKILL never fires; a tab-close SIGHUP can kill the hook mid-ingest. Every surface now states that, and ADR-017's conclusion (watcher + sweep are the guarantee, the hook is the latency optimization) stands unchanged. Closes mt#2313. Comments and docs only — no behavior change.Had Claude make this correction against the planned spec; the reviewer bot drives convergence.
Key changes
.minsky/hooks/transcript-ingest-on-session-end.tsheader (compiled twin regenerated viacompile) anddocs/architecture/hooks/session-end-transcript-ingest-hook.md§Reliability boundary — Covers / Does NOT cover enumerated from the measurement,/exitmarked UNMEASURED, ADR-017 cross-referenced.docs/architecture/adr-017-transcript-capture-continuous-watch.md— amendment note under the "inverted finding" paragraph; the references line marks the/clearhalf of issues #17885/#6428 as superseded..claude/settings.json— the mt#4035 and mt#3161_comments no longer say/cleardoes not fire.packages/domain/src/storage/schemas/conversation-run-state-schema.ts— docblock corrected (the column is populated for/clear, silent for crashes).Testing
Execution evidence:
grep -rn "ends normally" .minsky/hooks/transcript-ingest-on-session-end.ts docs/architecture/hooks/session-end-transcript-ingest-hook.md→ no hits (exit 1); both surfaces list/clearas firing with the 340-row measurement.grep -n "do NOT fire SessionEnd" .claude/settings.json packages/domain/src/storage/schemas/conversation-run-state-schema.ts docs/architecture/adr-017-transcript-capture-continuous-watch.md→ no hits (exit 1); the remaining mentions are the amendment text naming the measurement.mcp__minsky__compile({task}):git statusshows.claude/hooks/transcript-ingest-on-session-end.tsas the only generated file modified; the compiled twin carries the new boundary text (grep340 deliveries measured→ 1).settings.jsonand the schema: 49 files, 1128 tests, all pass (includeshook-module-inventory,interceptor-*,enforcement-mappingcensus tests and the hook's own suite).No new test files (no behavior changed; the deliverable is text, verified by the greps above).
Deploy verification:
conversation-run-state-schema.tsis deploy surface by path (a docblock edit). After merge:deployment_wait-for-latestwithnotBefore= merge time andexpectCommitSha= merge SHA forminsky-mcp,minsky-ops,reviewer,site; image-source identity via the deploy run's pushed-image digest against the deployment'simageDigest; health bodies'servicefield asserted. Recorded in mt#2313's## Outcome.🤖 Generated with Claude Code
https://claude.ai/code/session_0147JJ3SVgqRz9JyUD4NZBz9