T22: validate checkpoint completeness inside sheal retro

[luisalima]

Summary

Implements T22 (Validate checkpoint completeness inside sheal retro), stacked on #48. Extracted from the consolidation pass's reclassification of LEARN-019/020: retro used to analyze whatever it loaded, producing hollow analysis from truncated session data.

• assessCompleteness (src/retro/completeness.ts) runs before the analyzers and reports gaps: no sessions / no transcript entries / no assistant messages / blank conversational entries / tool calls without recorded results (the truncation case) and results without calls / empty filesTouched despite file-modifying tool activity.
• Gaps surface as inputGaps: string[] in JSON reports and a yellow "Input Gaps — Analysis Degraded" section in pretty output. Report-only by design — gaps never change the exit code (deliberate scope decision; revisit with Q8 (Strict-mode semantics) if strict wiring is wanted later).
• Checkpoint loading is boundary-validated (isCheckpoint guard): corrupt JSON or missing structure yields a clean, checkpoint-named error (first-line bounded), no stack trace. The batch path (--last/--today) has the same clean-skip crash contract.

Two-review trail

• Implemented by Codex (codex exec, cross-vendor) from a TDD brief; 7 tests red-first.
• Claude review (direct expert review — small bounded diff, no new sinks, validator reduces input attack surface): one cosmetic fix.
• Codex independent review (fresh session, read-only): verdict "not met" with 8 findings — 5 applied (both-direction tool pairing, Gemini blank-content false positive, read-only filesTouched false positive, batch-path guard, the Done-when CLI tests), 1 trivial reorder (assess before analyzers), 2 dismissed with reasons in the commit trail (full-schema validation beyond the crash contract; free-text truncation detection isn't reliably detectable).

Done when status

• ✅ Incomplete input produces an explicit gap report (pretty + JSON), analysis marked degraded — unit + CLI tests.
• ✅ CLI-driven test with a truncated fixture asserts the gap report (HOME-override native-session plant), plus a corrupt-fixture no-stack-trace test.
• ✅ Suite 368/368 green, tsc clean, lint 0 errors (37 pre-existing warnings).

🤖 Generated with Claude Code

[luisalima]

🧑‍🏫 PR Tutor — human-review pass

1. TL;DR

sheal retro used to analyze whatever session data it managed to load — even truncated or corrupt data — and produce a normal-looking retrospective from garbage input. Now it inspects its input first: gaps are named in the report (an "Analysis Degraded" banner / an inputGaps JSON field), and corrupt checkpoints produce a clean one-line error instead of a stack trace. Deliberately report-only: nothing here changes exit codes.

2. The problem it solves

T22 (Validate checkpoint completeness inside sheal retro) is the product-side fix extracted from the learnings-consolidation pass: LEARN-019/020 ("truncated session data yields hollow retros") were retired from the learnings store as bug reports against sheal itself, not agent-behavior rules. The bug: retro trusted its input blindly, so an incomplete checkpoint (no assistant messages, tool calls whose results never landed, missing file lists) yielded a confident-looking but hollow analysis, and a corrupt one crashed with a stack trace.

3. Picture it

Map — where the new pieces sit in the retro pipeline:

 checkpoint on disk (native ~/.claude JSONL, or Entire.io)
        │  load
        ▼
 [NEW] loadCheckpointForRetro ──corrupt JSON / wrong shape──► 1-line named error,
        │                                                      no stack trace
        │ structurally valid Checkpoint
        ▼
 runRetrospective (engine)
        ├── [NEW] assessCompleteness ──► inputGaps: string[]   (runs BEFORE analyzers)
        ├── analyzeEffort / failure loops / learnings / health  (unchanged)
        ▼
 Retrospective report
        ├── pretty: yellow "Input Gaps — Analysis Degraded" section   [NEW]
        └── JSON:   { "inputGaps": [...] }                            [NEW]
                                              exit code: unchanged in all cases

The validator guards the disk boundary; the completeness check lives in the engine, so every path that builds a report (single, --last/--today batch, Amp) gets gap detection for free.

Decision tree — what loading a checkpoint can now yield:

 loadCheckpointForRetro(id, loader)
   ├─ loader throws SyntaxError   → "Failed to load checkpoint <id>: invalid JSON"          [NEW]
   ├─ loader throws other Error   → "Failed to load checkpoint <id>: <first line only>"     [NEW]
   ├─ returns null                → quiet return ("nothing found" — pre-existing path)
   ├─ returns wrong shape         → "Failed to load checkpoint <id>: missing required
   │      (isCheckpoint fails)       structure."                                            [NEW]
   └─ returns valid Checkpoint    → proceed to retro
                                     (all error paths: stderr, exit code 0)

Every failure is named after the checkpoint, bounded to one line, and never a stack trace.

Before → After — the behavior change for thin input:

 BEFORE                                     AFTER
 ──────                                     ─────
 3 user msgs, zero assistant msgs           same report, plus:
   → normal retro: health score,              Input Gaps — Analysis Degraded:
     learnings, no hint anything                ! No assistant messages were captured.
     is missing
 corrupt session .jsonl                     "Failed to load checkpoint corrupt-session:
   → raw stack trace                          invalid JSON"

The report is still produced (degraded, not suppressed) — the reader just can't miss that the input was thin.

4. Toy example — the tool-pairing heuristic

The least obvious of the five gap checks. Transcripts record a tool call and its result as two separate entries (some runtimes emit one self-contained entry carrying both). Entries have no correlation IDs, so the check counts each side and compares:

 transcript (type=tool entries only):
   e1: { toolName: "Read" }                     ← call, no output    → pendingCall
   e2: { toolOutput: "file contents" }          ← output, no name    → orphanResult
   e3: { toolName: "Bash", toolOutput: "ok" }   ← self-contained pair → counted in neither

 pendingCalls = 1, orphanResults = 1  → balanced → no gap ✓  (e1+e2 are a split pair)

 the truncation case (LEARN-019's original complaint):
   e1: { toolName: "Read" }        pendingCalls = 1
   …session cut off, result lost…  orphanResults = 0
   1 > 0  →  "Transcript contains a tool call without a recorded result."   ← caught

 the acknowledged blind spot (dismissed in review — no IDs to match on):
   pair A lost its RESULT, pair B lost its CALL
   pendingCalls = 1, orphanResults = 1  → balanced → silently passes ✗

The other four checks are simpler existence tests: no sessions / no transcript entries; no assistant messages; blank content on conversational entries (tool entries may legitimately be blank — Gemini carries data in toolName/toolOutput); and empty filesTouched only when file-modifying tools actually ran (read-only sessions legitimately touch nothing).

5. Walkthrough

Detection — src/retro/completeness.ts (new, 56 lines): assessCompleteness(checkpoint): string[], the five heuristics above. The precision carve-outs (Gemini blank content, read-only filesTouched, both-direction pairing) all came from the independent Codex review.

Wiring — src/retro/engine.ts:34, src/retro/types.ts:107: the assessment runs first in runRetrospective; inputGaps?: string[] is added to Retrospective and omitted entirely when empty (so clean JSON reports are unchanged).

Boundary + display — src/commands/retro.ts:65 loadCheckpointForRetro (exported for tests) wraps the loader in the error contract; isCheckpoint at :88 is a hand-written structural guard over root/sessions/transcript/prompts. The batch path (:363) gets a try/catch "clean-skip" with the same no-stack-trace contract. printRetro (:754, newly exported) renders the yellow section.

Tests — test/retro-completeness.test.ts (new, 304 lines, 13 tests): unit tests per heuristic including the false-positive carve-outs, JSON serialization, loader-error cases, exit-code invariance, and two genuine CLI-driving tests that plant a native session file under a fake HOME (truncated fixture → gap report in pretty+JSON, exit 0; corrupt fixture → no stack trace).

Docs — README.md:136: seven lines describing the behavior.

Reviewed only the delta over PR #48 (base docs/t1-t2-tension-decisions), per the stack.

6. Glossary (as this repo uses the terms)

• Checkpoint — the @liwala/agent-sessions unit of session capture: root metadata (checkpointId, strategy, filesTouched) plus sessions[], each with metadata, a transcript of entries (user/assistant/tool), and prompts. Loaded from Claude-native JSONL or Entire.io.
• sheal retro — static analysis over a checkpoint producing a Retrospective: health score, effort, failure loops, candidate learnings.
• inputGaps — the new optional Retrospective field: human-readable strings naming what the input is missing.
• LEARN-019/020 — retired learnings-store entries ("verify checkpoint data loaded completely before analysis"). The consolidation change-set (docs/adr/0001-validation/2026-07-06-change-set.md) reclassified them as product feedback, not agent-behavior rules — T22 is the resulting product fix.
• Q8 (Strict-mode semantics: how should --strict treat skipped checkers…) — the open question where strictness/exit-code semantics are parked; this PR's "report-only" stance explicitly defers to it.
• Report-only — gaps degrade the report's framing but never the exit code; a deliberate, commit-documented scope decision.
• @liwala/agent-sessions — the workspace package (packages/agent-sessions) that normalizes transcripts across Claude/Codex/Gemini/Amp, including tool-name mapping (e.g. Codex apply_patch → Edit).

7. Does it meet "Done when"?

From docs/tasks/t22-validate-checkpoint-completeness-in-retro.md:

• "sheal retro on a session with incomplete input names the gaps in its output rather than silently analyzing thin data." ✅ — assessCompleteness + engine wiring + printRetro yellow section + inputGaps in JSON; asserted by unit tests and the CLI test.
• "A test drives the CLI with a truncated fixture and asserts the gap report." ✅ — test/retro-completeness.test.ts "sheal retro (CLI, truncated and corrupt fixtures)": spawns the real CLI with a planted HOME, asserts the gap in pretty and JSON output, plus a corrupt-fixture no-stack-trace test.
• PR's own quality claims — independently reproduced in a fresh clone of the branch: tsc clean, 368/368 tests green. (Lint not re-run here.)

Note: the task's "What we need" section has a third bullet — the retro extractor should branch "product feedback vs. learning" so bug reports about sheal don't enter the learnings store — which is not implemented and not covered by Done-when. It's arguably the deeper lesson of the LEARN-019/020 reclassification. See follow-ups.

8. Loose ends & red flags

• 🟡 Task reconciliation: docs/tasks/t22-…md frontmatter and TASK_INDEX.md still say status: todo on the PR branch. No paired docs/reconcile update exists yet for T22.
• 🟡 The extractor branch is unaddressed (task bullet 3, above). Without it, the next LEARN-019-style bug report can still land in the learnings store as a pseudo-learning.
• 🟡 Load failures exit 0: a corrupt checkpoint prints a red stderr line and returns — exit code 0, and the CLI test pins that behavior. Scripts/CI cannot distinguish "retro analyzed a session" from "retro refused its input". Coherent with report-only scope, but exit semantics for load errors (vs. mere gaps) deserve an explicit line in Q8 (Strict-mode semantics) so the decision is made once, not inherited by accident.
• 🟡 Batch path parity is partial: --last/--today wraps loading in try/catch (crash contract ✓) but never runs the isCheckpoint structure guard — a parseable-but-misshapen candidate flows into the analyzers unvalidated. Also, in --format json batch mode, skips are printed only in pretty mode, so JSON consumers get no record that candidates were dropped.
• 🟡 File-modifying tool list drift: completeness.ts hardcodes {Edit, Write, MultiEdit, NotebookEdit}, while agent-sessions' own file-tools list (packages/agent-sessions/src/transcript.ts:359) also includes mcp__acp__Write/mcp__acp__Edit, and Gemini entries use displayName || name (not mapped to these names). Codex is covered via the apply_patch → Edit mapping. Consequence is only false negatives (a missed gap report), but two lists answering "which tools modify files" will drift.
• 🟢 Tests aren't typechecked: tsconfig.json includes only src/, so the "tsc clean" gate never sees test/. Concretely, the new test accesses result.error on the {checkpoint}|{error} union without narrowing — that would fail under typecheck; vitest's esbuild transform just doesn't check.
• 🟢 The two-review trail is commit-borne, not GitHub-visible: the PR has zero reviews/comments; the Claude pass and Codex's 8-findings review + triage exist only in the PR body and the message of commit 432d9d3 ("apply Codex review findings"). Internally consistent and the dismissal reasons are recorded — but not independently verifiable, and the "red-first" TDD claim can't be confirmed from the squashed history (tests and implementation land in one commit).
• 🟢 Stacked on open PR [needs-review] T21: consolidation apply stage — execute reviewed dispositions against the store #48 (base docs/t1-t2-tension-decisions, itself titled "[needs-review] T21…"): merge order matters, and this PR's file list will look bigger until [needs-review] T21: consolidation apply stage — execute reviewed dispositions against the store #48 lands.
• ADR alignment: no conflicts found — report-only matches the change-set disposition for the 019/020 row, and the Q8 deferral is explicit in code comments, commit message, and PR body. Scope creep: none observed; the slice is deliberately narrow.

9. Suggested follow-up tasks

Provisional numbers (current max across all branches is T23; /opentasks assigns the real IDs):

📋 T24 — Branch "product feedback vs. learning" in the retro extractor
   why:        the unimplemented third bullet of T22's objective; without it, bug
               reports about sheal itself re-enter the learnings store (the exact
               failure the LEARN-019/020 reclassification identified)
   done when:  a retro-extracted candidate that describes a sheal defect is routed
               to a product-feedback channel (task stub / report section), not
               offered as a learning; test with a fixture transcript
   deliverable: P0 (inferred from T22) · depends on: #49

📋 T25 — Single source of truth for file-modifying tool names
   why:        completeness.ts and agent-sessions/transcript.ts each hardcode their
               own "which tools modify files" list; they already disagree
               (mcp__acp__*, Gemini displayNames) and will drift further
   done when:  one exported constant/predicate in @liwala/agent-sessions used by
               both call sites; a test covering a Gemini- and an mcp-shaped entry
   deliverable: P0 (inferred from T22) · depends on: #49

(The load-error exit-code question is better appended to Q8 (Strict-mode semantics: how should --strict treat skipped checkers…) than filed as a task; the batch isCheckpoint parity gap is small enough to fix in this PR if you want it now.)

Say "file these" (or "file task 1") and the main session will create them via the opentasks skill.

10. Verify by hand

• On a machine with real sessions: npx tsc && node dist/index.js retro on a session you know was truncated — does the yellow section read sensibly to you? (Gap wording like "No filesTouched data was captured…" is developer-speak; judgment call whether that's fine for this audience.)
• Corrupt a copy of a session .jsonl under a scratch HOME and run retro -c <id> — confirm the one-line error and, crucially, decide whether exit 0 on that failure is acceptable for how you drive sheal from CI/scripts.
• echo $? after both runs — the report-only stance is the biggest judgment call in this PR; confirm it matches your intent before Q8 hardens it.
• Confirm merge order with PR [needs-review] T21: consolidation apply stage — execute reviewed dispositions against the store #48 (base branch) and that its review happens first.
• Skim commit 432d9d3's message — the two dismissed Codex findings (full-schema validation; free-text truncation detection) are scope decisions made on your behalf; check you agree with both.

11. Check your understanding (optional — skip freely, or say "quiz me")

1. The pairing heuristic reports no gap when orphan calls and orphan results are equal in number. Why is that the right default, and what real data loss does it therefore miss?
2. A corrupt checkpoint file now exits 0. What's the argument for that, and where is the decision parked if you want it changed?
3. Which agent runtimes could have file edits that never trigger the "No filesTouched data was captured" gap, and why?

---

1. Complete pairs arrive as a call entry + a result entry, and entries carry no correlation IDs — so "balanced" is the only available signal for "paired". It misses a transcript that lost one call and one unrelated result (counts still balance).
2. Gaps and load failures are report-only so sheal retro never breaks pipelines over degraded input; the exit-code/strictness decision is deliberately parked in Q8 (Strict-mode semantics) rather than made piecemeal here.
3. Gemini (toolName is displayName || name, never mapped to Edit/Write) and MCP-based edits (mcp__acp__Write/Edit) — the hardcoded set in completeness.ts doesn't include them, so modifiedFiles stays false and the check never fires. Codex is safe via the apply_patch → Edit mapping.

---

Tutoring pass, not an approval or a safety claim. It surfaces what changed and what to check by hand; the human reviewer decides.

[luisalima]

Tutor-pass follow-up — findings triaged and applied at 8a087e3 (T22: tutor-pass fixes):

• Batch structure guard: the --last/--today path now applies the same isCheckpoint guard as the single-session path; malformed structures skip cleanly.
• Visible JSON-mode skips: batch skip notices go to stderr in JSON mode so stdout stays parseable and a batch can't silently under-report — locked by a new CLI test (good + corrupt session, --last 2 -f json).
• Test type narrowing: the load-result union is narrowed in the tests, so the file survives typechecking if tests ever join the tsc gate (the tsconfig gap itself is noted, not fixed here).
• Task reconciliation: by design — impl PRs are code-only; T22's done status lives on the tracking branch (docs/t1-t2-tension-decisions, commit 646ed34).
• Follow-ups filed: T24 (Retro extractor: branch "product feedback vs. learning") and T25 (Share the file-modifying-tools list between agent-sessions and retro completeness). The load-error exit-semantics question is appended to Q8 (Strict-mode semantics) as Decision C, so the strictness family gets decided once.

Suite green at the new head; the qa-personas run in flight tested the prior head — its oracle probes (dry-run/gap/exit-code invariants) are unaffected by these additions.

🤖 Generated with Claude Code

[luisalima]

qa-personas results — PR #49 (T22: validate checkpoint completeness inside sheal retro)

Black-box persona QA. Deterministic oracle = the hard signal; persona findings are advisory. Nothing here merges, approves, or blocks — advisory only.

Verdict at a glance

• Deterministic oracle: 9/9 PASS. Every invariant this PR is about holds. No deterministic breach → PR left as-is (not drafted).
• Personas surfaced real issues, but all are pre-existing behavior this diff neither introduces nor touches (checkpoint-loader path handling, token/toolCounts rendering, ANSI output). Filed below as advisory + a suggested follow-up.

Vendor & coldness disclosure (read this)

• Orchestrator: fresh background agent (not the PR conductor) — cold on intent by construction.
• Maker vendor: per the PR trail this diff was implemented by Codex and reviewed by Claude + Codex — both vendors were in the loop.
• Benign persona: cold Codex (OpenAI) — opposite vendor to the Claude orchestrator, but same vendor as the implementer, so its cross-distribution value on the benign side is reduced. 16 attempts, full transcript.
• Misuse persona: cold Codex was refused by OpenAI's own cybersecurity content filter after its first probe ("flagged for possible cybersecurity risk"). Fell back to a cold Claude misuse persona (opposite vendor to the Codex implementer). This is the honest caveat: the two personas ended up on different vendors, and neither is opposite-vendor to both implementer and orchestrator at once. A human should weigh the misses accordingly.

Personas ran fully black-box against the built binary with planted fixtures in a scratch $HOME; they never saw the diff, PR text, or design intent. The repo's own .sheal/ was never touched.

---

Deterministic oracle (hard signal — reproducible by hand)

Rig: oracle.sh plants native Claude-Code JSONL fixtures into a scratch $HOME (per-run canary), then drives node dist/index.js retro. Each probe is a command + assertion; canary varied per run (re-ran twice, both 9/9).

Probe	Assertion	Result
P1a	Truncated session (user-only, ≥3 prompts) → pretty "Input Gaps — Analysis Degraded" + exit 0	PASS
P1b	Same session -f json → inputGaps[] present, JSON parseable, exit 0	PASS
P2a	Complete session → analyzed, not marked degraded, exit 0	PASS
P2b	Complete session -f json → no inputGaps key	PASS
P3a	Corrupt JSONL → clean handling, no stack trace, non-empty message, exit unchanged (0)	PASS
P3b	Corrupt JSONL -f json → no stack trace, exit unchanged	PASS
P4a	Gaps never alter exit code (truncated == dangling == complete, all 0)	PASS
P5a	Dangling tool call (truncation) reported as a gap	PASS
P6a	Batch path (--last) over mixed/corrupt fixtures → no stack trace, clean skip	PASS

All four mandated invariants (gap report on truncation & exit 0; complete not degraded; corrupt ⇒ no stack trace, exit unchanged; gaps never change exit code) hold. PR's own suite test/retro-completeness.test.ts also green (13/13).

---

Persona findings (soft signal — advisory, severity-tagged)

Scope note: every item below reproduces on the changed CLI surface but lives in code unchanged by this diff (the loader's join(dir, "${sessionId}.jsonl") and the effort/render analyzers both predate PR #49). None is a regression from T22. Listed so they're on record; a human decides whether to spin off follow-ups.

• [High · pre-existing] Path traversal / arbitrary .jsonl read via -c (misuse persona; independently reproduced by the orchestrator). A crafted checkpoint id escapes the project's slug dir and reads any .jsonl the user can access.
  Repro: with a session file at /tmp/tmpleak.jsonl,
  node dist/index.js retro -p <proj> -c "$(python3 -c 'print("../"*20+"tmp/tmpleak")')" -f json → fully analyzes the out-of-tree file (checkpointId echoes the ../…/tmp/tmpleak path). Also reaches sibling project dirs and $HOME. Root cause is the unsanitized ${sessionId}.jsonl join in @liwala/agent-sessions (present on the base branch). Suggest a dedicated follow-up task to sanitize/confine checkpoint ids to the projects dir.
• [Med · pre-existing] Out-of-range token values corrupt machine-readable output. input_tokens: 1e400 → JSON "inputTokens": null; negative tokens pass through as negative totals — breaks downstream cost/accounting consumers. Repro: -c <bignum-fixture> -f json.
• [Med · pre-existing] Prototype-chain tool names corrupt toolCounts. A tool named constructor yields toolCounts.constructor = "function Object() { [native code] }1" (string where a number is contractual); a tool named __proto__ is silently dropped and mutates the counts object's prototype.
• [Low/Med · pre-existing] ANSI-escape injection in pretty output. Attacker-controlled tool names / file paths are printed to the terminal without stripping control sequences (cursor/color/title spoofing). Repro: -c <ansi-fixture> | cat -v shows raw ^[[31m….
• [Low · pre-existing] Invalid createdAt reflected verbatim into JSON (e.g. -2026-07-17T…).
• [UX · this PR's surface] Degraded inputs still report Health Score: 100/100 and the batch summary hides degradation prevalence. Not a break (gaps are report-only by design per the PR), but the perfect score sits next to the "Analysis Degraded" banner, which a skimming user may misread. Batch --last averages degraded + complete into one Average health: 100/100 with no degraded count. Worth considering as a UX polish, consistent with T22's intent of flagging low-trust analysis.
• [UX] Unknown --format silently falls back to pretty (-f xml → pretty, exit 0) instead of rejecting.
• [UX] -c '' selects latest; --today -c <id> silently ignores -c — selector precedence is undocumented.

Correctly handled (no divergence): empty/blank/deeply-nested/NUL/bad-UTF-8/wrong-shape/NaN-literal fixtures (no crash), giant transcript at normal memory, all gap-suppression attempts (truncated stayed degraded, complete stayed complete), JSON stayed parseable throughout, absolute/suffix -c forms rejected.

---

Suggested regression seeds (for the conductor/human to land)

1. The 4 mandated invariants are already covered by test/retro-completeness.test.ts; the oracle mirrors them — no gap there.
2. If the [High] traversal is accepted as a bug: a CLI test asserting -c "../../../x" (and an absolute-escape form) does not read outside the resolved projects dir — landed against the loader in @liwala/agent-sessions, not this PR.

Method: cross-vendor black-box personas + an out-of-persona deterministic oracle. Advisory only — a human owns the merge.