feat(session-flow): adopt claim-provenance and constraint re-scan rules from handoff audit - #1560
Conversation
…es from handoff audit (#1477) Audit of session-flow:handoff against the mattpocock/skills handoff-failure corpus (#186/#306/#617/#482, fetched live) found two write-side gaps with demonstrated upstream failure modes and no existing guard in the template: - Claim provenance: inherited claims (prior handoff assertions, issue labels, remembered state) must carry an explicit UNVERIFIED (<source>) marker — an inherited claim is a claim to falsify, not a fact to forward. The existing fresh-reads discipline governed only what the writing session could probe. - Constraints re-scan: before closing "Constraints that must hold", re-scan the conversation for but/except/unless/"the exception is"/"the corner case" — mid-discussion constraints that never rose to a top-line bullet are the category a resuming session ships as a bug. The other audited findings were already covered (decision-archive template sections 7/8/9 with anti-summary framing; resume prompt leads with an execute instruction; durable <memory_dir>/handoffs/ location with chain continuity); verdict detail recorded on #1477. Verification: markdownlint clean on both edited files; handoff evals.json schema-valid against skill-quality's evals.schema.json (repo CI's eval bar); skill contract gate does not fire (no files under skills/ changed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018qLeNKmcBFxZgsfEksLomX
…nance rule The parenthetical pointed at "the citing skill's gotchas", but structure.md has two citing skills and continue-in-background ships no gotchas file; for handoff the file is load-on-demand, so the delegated rule is not guaranteed in context. The met/unmet evidence rule the reference gestured at already lives in this document's own Completion criteria section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018qLeNKmcBFxZgsfEksLomX
|
Claude finished @kyle-sexton's task in 1m 43s —— View job PR Review
Overall: approve. This is a tight, well-scoped change. No blocking issues. A few observations below.
|
|
Claude finished @kyle-sexton's task in 1m 11s —— View job Security Review — PR #1560
No security vulnerabilities identified. This PR is a prose-only diff across three files. The security surface is minimal; here is the full reasoning for each changed file.
|
| Threat class | Finding |
|---|---|
| Injection (command / path / template) | No user-controlled input is introduced. The UNVERIFIED (<source>) marker is a document convention, not an executed template. The existing bash write-procedure block (lines 274–323) was not modified by this PR. |
| Credential / secret handling | None introduced. |
| Trust-boundary weakening | The provenance rule strengthens trust boundaries: inherited claims now require an explicit UNVERIFIED (<source>) marker rather than passing silently as facts. This is a security-positive change. |
| Logic flaw creating an exploitable condition | The provenance rule and constraint re-scan rule are advisory guidance for AI agents writing handoff documents. No branch, gate, or enforcement mechanism is introduced that could be bypassed or manipulated. |
| Authorization / access-control gap | Not applicable — reference documentation with no access control surface. |
| GitHub Actions dangerous patterns | No .github/workflows files changed. |
| Supply-chain / dependency risk | No package pins, action refs, or external dependencies touched. |
The UNVERIFIED (<source>) convention was already present repo-wide (the PR description confirms this is a reuse of an existing marker, not a new convention). The constraint re-scan keyword list (but, except, unless, "the exception is", "the corner case") is plain prose with no runtime evaluation path.
Confidence: 0.97. The only residual uncertainty is that I reviewed documentation-layer guidance rather than executable code — there is no runtime surface to trace a data-flow through, so the confidence ceiling for a purely-prose diff is inherently below 1.0.
![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: 805de420ef
ℹ️ 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".
…ssless transcript post-compaction Two review-thread gaps in the handoff-hardening PR (#1477): - Claim provenance lived only in structure.md, which the prompt-only path never walks — an inherited status could ride an inline remaining-work bullet with no UNVERIFIED (<source>) marker. Hoisted the rule to save-point.md as a "mandatory on BOTH paths" section (mirroring the existing redaction pass), with structure.md and skills/handoff/SKILL.md's checklist (both full and prompt-only) pointing at it. continue-in-background inherits the fix for free — its own checklist already defers to the engine doc's save-point items rather than restating them. - The constraints re-scan targeted "the conversation," but after an unexpected compaction that's the summarizer's output, not the original turns it's supposed to catch caveats in. The section must now say explicitly whether it re-scanned the lossless on-disk transcript (retro's parser reads the same record) or is disclosing that pre-compaction turns went unscanned — silence reads as the latter never having happened, never as a pass. Verification: markdownlint clean; skill-quality's check-changed-skills.sh and check-skill-portability.sh pass against origin/main for the changed handoff skill file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 26s —— View job PR Review — Follow-up Push (
|
|
Claude finished @kyle-sexton's task in 1m 33s —— View job Security Review — PR #1560
No security vulnerabilities identified. Five files changed across three commits; all are prose-only. Full reasoning by threat class:
|
| Threat class | Finding |
|---|---|
| Injection | No user-controlled input is introduced. UNVERIFIED (<source>) is a documentary convention written by the model into a markdown file; <source> is free-form prose, not an evaluated template or shell expansion. No injection surface. |
| Trust-boundary weakening | The opposite: the rule narrows trust from inherited assertions to this-session-verified facts. Anything that does not meet that bar must carry an explicit marker. Security-positive. |
| Credential / secret handling | None introduced. The existing redaction pass (lines 53–61, unchanged) already gates secrets before any write or prompt emission; the new provenance rule operates on logical state-of-the-world claims, not credentials. |
| Logic flaw | The rule governs both full and prompt-only paths explicitly (line 71: "This governs both paths, not just the full path's body sections"). The prior Codex finding (prompt-only gap) was addressed in a3fe8cc — the rule now lives in save-point.md where both paths walk it, not solely in structure.md. No residual gap. |
| Authorization / access control | Not applicable — guidance prose. |
plugins/session-flow/reference/structure.md — provenance cross-reference + compaction re-scan rule (lines 43–47, 84–106)
| Threat class | Finding |
|---|---|
| Injection | No executable code added. The compaction section (lines 84–106) references ${CLAUDE_PLUGIN_ROOT}/skills/retro/scripts/parse_transcript.py — this is a plugin-controlled path variable, not user-supplied input; no injection surface. |
| Compaction-aware re-scan (lines 89–106) | The rule requires the section to disclose one of two things: (a) the lossless on-disk transcript was re-scanned, or (b) only the model-visible (possibly compacted) conversation was scanned and it says so explicitly. Both branches are valid; neither creates a bypassable gate or an exploitable condition — the rule is advisory guidance, not an enforcement mechanism with a security-meaningful failure mode. |
| Trust-boundary | Provenance cross-reference (line 43) points at save-point.md's "Claim provenance" rule, tightening rather than loosening the boundary. |
| Supply chain | No action refs, package pins, or external dependencies touched. |
plugins/session-flow/skills/handoff/SKILL.md — four checklist lines (lines 127, 141–142)
New tick items add "Claim provenance applied" to both the full-path and prompt-only post-write checklists, each referencing the engine doc. These are model-facing audit gates. No executable code, no data flow, no security impact.
Confidence: 0.97. Residual uncertainty is the nature of reviewing AI-guidance prose rather than executable code — there is no runtime data-flow to trace, so the ceiling for a purely-prose diff is inherently below 1.0. Within that ceiling I see no issue.
…ow continuation router (#1603) Closes #1475 Closes #1476 ## What One branch for the companion pair, per both issues' "likely one PR" note. **context-guard 0.4.0 — zone-crossing hooks + window-class token bands (#1475).** - `hooks/hooks.json` registers four fail-open handlers, each with a co-located contract-test suite: `zone-crossing-inject.sh` (PostToolBatch + UserPromptSubmit; injects continuation guidance via `additionalContext` once per transition into a worse zone — silent on unchanged/improving/`unknown`; PostToolBatch's once-per-batch semantics replace the per-tool dedupe a PostToolUse design would have needed), `zone-gate.sh` (PreToolUse over `Write|Edit|NotebookEdit|Agent|Workflow`; active only in `blocking` mode on a fresh dumb-zone snapshot past a grace budget; handoff-path writes, reads, Bash, and Skill never gated — no deadlock by construction), `post-compact-mark.sh` (PostCompact; persists the evidence-degraded marker, re-arms the grace budget, prunes stale markers). - Resolver v2: two zone shapes combined conservatively — percentage bands over `used_percentage` (distance to compaction; upstream computes it input-only) and window-class token bands over occupancy = `total_input_tokens + total_output_tokens` (distance to quality loss; 200k class 100k/160k, 1M class 200k/400k — declared judgment defaults, provenance table on #1475). Combination rule stated verbatim in the reader contract; plausibility guard covers the pre-2.1.132 cumulative token-field semantics; `zones.json` gains an optional per-shape-validated `token_bands` (absent = zero-config, every v1 file keeps working); percentage keys retained with a recorded retirement trigger. - Config per `docs/conventions/hook-config-delivery` channel B with in-script defaults; three new `userConfig` options; telemetry producers registered with data schemas; both known inliners migrated (plugin-quality (now 0.3.0) context-gate + setup seeder) and the reader contract's promised consumer drift-check lane now actually exists (`plugin-quality/scripts/zones-inline-drift.test.sh`, runs in plugin-gate). **session-flow 0.17.19 — workflow end-of-phase continuation router (#1476).** - New `context/continuation.md` spoke + `continue` argument mode + a default-mode phase-boundary step. Outcome set derived from the mechanisms the plugin actually installs plus built-ins (continue / `/clear` / handoff / continue-in-background / clean-stop / `/compact`); `reconcile`/`orient` deliberately non-terminals; every ordering edge carries its stated purpose (machine-loss asked first, zero-cost exits before writing mechanisms, compact last as the model-written lossy summary at the least-intelligent point — tradeoff pointer to handoff's "Fork beats compaction when the window is deep", not a restatement). Zone input presence-gated on the reader contract with **no inlined band values**; honors the evidence-degraded marker. Documents the worker handoff-relay convention (worker writes its own handoff, returns the path only). **CI fix (needed by this PR, correct independently):** `check-shell-portability.sh` now exempts registered cross-plugin sync copies (`scripts/cross-plugin-source-registry.txt`) — a synced `hooks/hook-utils.sh` copy can never carry a local fix or annotation, so the source is where the gate belongs; surfaced by this PR adding the first new copy since the GNU-regex-escape tokens went active (line 1114's bash glob `[\<\>]` is not a GNU regex escape). Repro-first suite case included. ## Local review outcomes (obtained before PR creation) - **/code-review** (headless, isolated worktree, full diff vs main): **5 findings, all fixed in `40aa5b69` with failing-test-first repros**, plus 2 suspicions it checked and cleared itself. (1) PostCompact marker silently unwritten for realistic payload sizes (measured 80KB+ lost to the single bounded stdin read on Git Bash pipes) → all three hooks now use a plugin-local chunked drain loop (`hooks/payload.sh`, the tee's proven `read -N` pattern) with 130–150KB regression tests; (2) same root cause failed the blocking gate open for exactly the biggest writes (verified: 2.8KB Write denied, 133KB Write allowed) → same fix + regression test; (3) the evidence-degraded marker was write-only and the grace reset let compaction disarm the gate → both zone consumers now treat a marked session as effectively dumb regardless of the resolved word, per the contract's own rule; (4) large batch payloads could skip injections → chunked reader; (5) `.compacted` markers never pruned → 14-day sweep matching the tee's. An earlier same-content review pass on the pre-rebase branch had also run every repo gate itself (portability incl. its 108-case self-test, silent-skips, changelog parity, validate-plugins, all five new/changed suites) — all pass. - **/security-review** (headless, two runs — pre-rebase full diff, then final HEAD incl. the CI-checker change): **no vulnerabilities at the reporting bar in either run**. Cleared explicitly: session_id path containment at all four entry points (hostile-id regression tests), hand-built JSON via charset-constrained values + `jq --arg`, no reflection of untrusted payload content into the model channel, strict ISO gate before `date -d`, no dynamic command construction, `umask 077`/`chmod 700` on state writes, and the portability exemption opens no silent-divergence hole (byte-identity gate owns those copies). Deliberate non-findings recorded in the review outputs (gate-bypass shapes are quality-control, not a security boundary; documented fail-open). ## Verification - New/updated suites all green at the current head: `context-zone.test.sh` 73/73, `zone-gate.test.sh` 24/24, `zone-crossing-inject.test.sh` 18/18, `post-compact-mark.test.sh` 16/16, `statusline-tee.test.sh` 47/47, `zones-inline-drift.test.sh` 11/11, `check-shell-portability.test.sh` 109/109. - `validate-plugins.sh` clean; `sync-hook-utils.sh --check` reports all 15 copies matching `lib/`; portability vs `origin/main`, silent-skip, cross-plugin-source-drift, plugin-manifest-presence, skill-leaf-names, and changelog-parity (`--check` and `--check-bump`) all clean. The repo's own `plugin-gate` on CI is the authoritative full-suite run. - Rebased/merged onto main repeatedly as the base moved. Version lanes re-slotted each time: plugin-quality 0.2.0→0.3.0 atop upstream 0.2.2; session-flow walked 0.17.14→0.17.19 as main shipped 0.17.17 and 0.17.18 underneath this branch. ## Review-queue drain All ten Codex review threads fixed and resolved, each verified against the merged tree rather than taken on the commit message: the gate grace-counter race (replaced with an O_APPEND byte counter), the `08`-as-octal grace budget, the pre-2.1.132 cumulative-token semantics (now gated on an explicit `cli_version` floor), the reader contract's capability table (now per-shape, so the table and the combination rule can no longer derive different zones), the setup skill conflating registration with activation, leftover diff3 merge markers, the unreachable `continue-in-background` outcome (explicit-background is now question 1), the injector's write-only zone state, and the PostCompact marker's false `status:ok`. Three further findings arrived in a review body rather than as threads. Two were real and are fixed here: both stateful hooks fell back to a `.`-relative state root, and zone-gate's handoff-exemption path extraction bypassed the file's own jq helper. One was a false positive — the portability checker's registry comparison quotes its RHS, so globbing is already inactive; a repro-first case now pins that. Details and the platform probe are in the PR comments. ## Related - #1475, #1476 (implemented here); #1477 / PR #1560 (merged sibling); #1478 (research these hooks build on — statusline-tee remains the interactive source, hooks fail open elsewhere); #1479 (orchestrate audit; the router documents the handoff-relay convention it flagged). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_018qLeNKmcBFxZgsfEksLomX --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
… caveat (#3019) Closes #2956 ## Summary Implements the AI Hero course lane 1 wording changes for `session-flow:handoff`: a routing-signals table mapping situation to save-point form, the session-chain/retrospective use named first-class in "When to invoke", the general do-not-duplicate rule, the promote-content-never-file rule, and the worktree caveat in the engine doc's destination section. `find-handoff`'s detection contract is untouched. ## Fix - **`skills/handoff/SKILL.md` — "When to invoke"**: names the session-chain use first-class (the escape-and-resume cadence whose handoff files carry the `session_id`/`previous_handoff` chain `/session-flow:retro` walks for retrospective reconstruction) alongside the upstream boundary-crossing taxonomy (colleague, other repo or checkout, other agent, forked mid-phase side task) — the UNION the lane decided. A new "Routing signals — which form to use when" table maps: deep-window escape with chain value → full file (default); small follow-ups, no chain value → prompt-only with its documented retro-gap cost; differing next-session focus → either form plus the purpose argument; AFK-but-work-continues → the sibling `continue-in-background` skill (explicit user request only); machine may go away → `clean-stop` semantics; boundary crossing → full file plus purpose plus the `Handoff origin:` line. The table's AFK row absorbs the former standalone "Going AFK" paragraph (same content, one home). - **`skills/handoff/SKILL.md` — new "Reference other artifacts; promote durable value" section**: states the do-not-duplicate rule mirroring upstream (content captured in specs, plans, ADRs, issues, commits, or diffs is referenced by path or URL, never restated — the existing "Summarize; never transcribe" per-section guidance stated as a general rule) and the promote-content-never-file rule (durable value is promoted into a committed artifact — topic contract, issue, PR body — while the handoff file stays ephemeral and uncommitted; cleanup of `handoffs/` remains user-controlled removal, never silent expiry — no expiry behavior added anywhere, confirming the #1477 finding-4 rejection). - **`reference/save-point.md` — "Where save-points live"**: adds the worktree caveat — a save-point written inside a `git worktree` checkout resolves its memory root within that worktree and dies with `git worktree remove`; acceptable only when the worktree completes as a merged PR unit; when pausing un-merged worktree work, write from the main checkout or rely on `/session-flow:clean-stop`'s preserve-before-remove step (which already inspects ignored content before removal and names "generated handoff data" explicitly — the existing cross-reference from worktree-touching tooling). - **Versioning**: session-flow `0.25.0` → `0.26.0` with the matching CHANGELOG entry. ## Verification - `bash scripts/validate-plugins.sh` — all manifests and the catalog validated. - `npx markdownlint-cli2` on the four touched markdown files — 0 issues. - `bash scripts/check-changelog-parity.sh --check`, `--check-bump origin/main`, `--check-order` — all pass (83 changelogs, newest-first, bump matched by entry). - `bash scripts/check-changed-skills.sh origin/main` — PASS, 0 errors (one soft-target warning: SKILL.md 230/500 lines against the 200-line soft target; the local mawk `REcompile` panics are the known #3005 artifact, non-failing). - Acceptance criteria walked: routing table present with session-chain use first-class; do-not-duplicate rule stated; worktree caveat in the destination section; promote rule stated with no expiry behavior; `skills/find-handoff/` and the detection-contract section of `save-point.md`'s resume-prompt shape untouched by the diff. ## Related Refs #2899 (lane 1 decision record), `docs/upstream/aihero-course.md` lane 1 rows. Refs #1477 / PR #1560 (cleanup sub-item rejection this confirms). Refs #2955 / PR #3017 (purpose argument, sibling item the routing table's purpose rows build on). Refs #3005 (mawk artifact noted in Verification). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QbfCrj3X9FfGL7VRZYmrn4 --- _Generated by [Claude Code](https://claude.ai/code/session_01QbfCrj3X9FfGL7VRZYmrn4)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 participant
Closes #1477
What
Hardening audit of
session-flow:handoffagainst the mattpocock/skills handoff-failure corpus (#186 / #306 / #617 / #482, all fetched live 2026-07-26). Per-finding verdicts recorded on #1477 (audit comment): finding 1 covered (two sub-items adopted/rejected), finding 2 adopted, finding 3 covered, finding 4 covered (cleanup sub-item rejected). This PR lands the two adoptions:reference/structure.md): a status claim may be stated plainly only when this session verified it; anything inherited (a prior handoff's assertion, an issue label, a remembered state) carries an explicitUNVERIFIED (<source>)marker — an inherited claim is a claim to falsify, not a fact to forward. Closes the laundering gap behind upstream's real incident (an unverified "not built yet" forwarded as fact caused a feature to be rebuilt).reference/structure.md): before closing "Constraints that must hold", re-scan the conversation for but / except / unless / "the exception is" / "the corner case" — mid-discussion constraints that never rose to a top-line bullet are the category a resuming session ships as a bug.Plus session-flow
0.17.12 → 0.17.13and the CHANGELOG entry.Local review outcomes (obtained before PR creation)
claude -p, diffmain...HEAD): 1 finding — the provenance paragraph's "(see the evidence rule in the citing skill's gotchas)" pointed at a file one of the two citing skills doesn't ship, and load-on-demand for the other. Fixed in805de420by dropping the cross-reference (the evidence rule already lives in structure.md's own Completion criteria section). Review also explicitly cleared: changelog parity (--checkand--check-bumppass), patch-bump precedent,UNVERIFIED (<source>)reusing the existing repo-wide marker convention, line-wrap conformance, and non-duplication vsreanchor/orientread-side checks.claude -p): no security vulnerabilities identified, confidence 0.95 — diff introduces no executable code, no shell invocation, no path construction, no credential handling; prose-only guidance changes.Verification
plugins/session-flow/skills/handoff/evals/evals.jsonschema-valid againstplugins/skill-quality/reference/evals.schema.json(the repo CI's eval verification bar).skills/changed).Related
🤖 Generated with Claude Code
https://claude.ai/code/session_018qLeNKmcBFxZgsfEksLomX