feat(sessions): report amendment/supersession

[roughcoder]

Why

A queued instruction can arrive after a child posted its terminal report. Until now the stale report stayed the record — and in the incident that motivated this, worse: the report omitted the late instruction, and the child then claimed compliance with something it had never done. A session needs a way to amend its own account.

What

post_report gains an optional supersedesReportId.

1. Validation — it must name an existing report on the calling thread. Unknown id and another thread's id are refused with the same SessionOrchestrationInvalidInputError message, so the denial cannot double as a probe for which report ids exist elsewhere. Checked before dispatch: a dangling link would be worse than a rejection, since no reader could follow it.
2. Append-only — the amendment stores a forward link (supersedes_report_id, migration 048); the superseded report keeps its row, its event, and its body. The reverse link (supersededByReportId) is derived on read via a correlated subquery, so there is no "superseded" flag to drift out of sync, and the chain reads from either end.
3. Delivery — envelopes and read_session carry both ids. An amending report's parent notification leads with AMENDED report (supersedes …) before the summary, because a parent that already acted on the superseded report has to see that first.
4. read_report — a superseded report still returns its body, plus supersededByReportId and a supersededNotice sentence: a caller paging an old report cannot be relied on to notice a field it was not looking for.
5. Child instructions — SPAWNED_SESSION_REPORT_INSTRUCTIONS (force-appended to spawned prompts) now states the rule: an instruction arriving after you reported means post an amending report, never claim retroactive compliance.

Event-sourcing safety

supersedesReportId is optional on both the thread.report.post command and the thread.report-posted payload, so already-persisted report events replay unchanged (covered by a test that decodes a pre-feature payload). supersededByReportId is deliberately not in the event: at post time no superseding report exists yet, so putting one there would make the event a lie.

Tests

@effect/vitest it.effect, extending existing files:

• Chain, both ends — real SQLite through the repository (ProjectionRepositories.test.ts): A superseded by B, read from A and from B, list ordering, superseded body preserved.
• Invalid reference and cross-thread reference denials, each asserting nothing was dispatched (handlers.test.ts).
• Envelope + delivery text for amendments (handlers.test.ts, SessionSpawnReactor.test.ts).
• Replay of a pre-amendment thread.report-posted payload (orchestration.test.ts); decider pass-through (decider.reports.test.ts); migration shape and backfill (048_…test.ts).

Verification (exit codes)

• npm run typecheck — 0
• root vp run -r test — 1, from one pre-existing unrelated failure: apps/mobile/src/features/diffs/nativeReviewDiffHighlighter.test.ts > keeps grammar state across inline comment rows (a Shiki grammar-state assertion). Confirmed pre-existing by stashing this branch's changes and reproducing it on the clean tree. Everything else passed, including apps/web 58/58 and apps/server.
• Targeted server tests (5 files, 93 tests) — 0; contracts — 0
• vp lint --report-unused-disable-directives — 0 (8 warnings, all in files this PR does not touch)

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[roughcoder]

REQUEST-CHANGES

1. [blocker] apps/server/src/persistence/Migrations.ts:121 — Do not merge/deploy this before PR #13. The runner explicitly runs only migration IDs greater than the latest recorded ID (Migrations.ts:148-160). If 048 lands first, its DB records 48; later PR #13’s 047 is permanently skipped, leaving its required table absent. The “reserved” comment is not sufficient protection. Keep the number, but make #13 a merge/deploy prerequisite (or otherwise make the runner safe for out-of-order additions).

2. [major] apps/server/src/persistence/Layers/ProjectionThreadReports.ts:172 — post_report permits multiple reports to supersede A, but the derived reverse chooses the earliest one. With A→B then A→C, read_report(A) says B is the current account while read_report(threadId) returns C as latest. This violates the documented “amendment becomes current account” / follow-the-chain contract and loses C from A’s navigation. Reject an already-superseded target (or define/implement an unambiguous latest-wins or multi-successor contract); add fork and multi-hop-chain tests.

3. [minor] apps/server/src/mcp/toolkits/sessions/handlers.ts:1011 — Same-thread validation is a projection lookup, not command/decider enforcement. Two concurrent amendments can both validate A before either projection update, creating the fork above; internal command callers can also persist arbitrary/cross-thread links. Put the invariant at the serialized authoritative boundary, not solely in the MCP handler.

4. [minor] apps/server/src/orchestration/Layers/SessionSpawnReactor.test.ts:360 — Delivery tests are formatter-only. They do show one amended string, but do not exercise reactor dispatch to prove one parent turn (not a normal plus amended duplicate), nor the stated resurrection flow: system terminal report → resumed agent report superseding it. Add an integration-style reactor/handler assertion.

Verified read-only: reverse is indexed correlated subquery (per returned report, no full scan); list/read order is created_at/report_id; read_session/read_report surface links; hasReport remains an existence check (appropriately true for superseded reports); optional event fields replay safely; archived threads cannot post because requireShell uses active shells. I did not run tests, typecheck, installs, or servers.

[roughcoder]

Fix round pushed as 0628e8187. Taking the three items in order.

1. Amendment forks (major) — fixed, linear chains only.
Superseding an already-superseded report is now refused. The caller gets SessionOrchestrationReportAlreadySupersededError carrying reportId, supersededByReportId, and chainHeadReportId, plus a message of the form "Report A is already superseded by B; supersede B instead" — and when the chain has grown further, "…already superseded by B, and the current head of that chain is C; supersede C instead", so the caller is always pointed at the head rather than merely told it lost. Amending an older unsuperseded report is still allowed: the rule bans forks, not history.

2. Validation moved into the decider (required with #1) — done.
You were right that handler-level validation cannot prevent concurrent forks or internal-command bypass. The authoritative check now runs in decideOrchestrationCommand's thread.report.post case, against the folded read model, serialized with command processing — so the second of two racing amendments loses deterministically, and internally dispatched report posts are covered too. The handler keeps a pre-check purely for error quality, and when a dispatch is rejected it re-reads the chain so a race loser gets the same structured error instead of a generic dispatch failure. Both sides call one shared checkReportSupersession and one shared message builder, so they cannot drift apart.

While wiring this I checked a dependency worth recording: the decider's check is only sound because reports is uncapped in the read-model projector (messages/checkpoints/activities are capped). Capping reports later would start rejecting legitimate amendments of aged-out reports as "unknown", so there's now a comment at the check saying so.

3. Tests — all three added.

• One-turn reactor integration (SessionSpawnReactor.test.ts): drives the real reactor over a real thread.report-posted event and asserts the turn dispatched to the parent leads with AMENDED report (supersedes …) before the summary — resolving the child, finding its spawner, and dispatching, none of which the formatter test covered. Paired with a negative case for an ordinary report.
• Fork rejection incl. the race shape (decider.reports.test.ts, handlers.test.ts): two posts superseding the same report with the second rejected, chain-head reporting through a 3-long chain, unknown-report rejection at the decider (the internal-dispatch path), and a handler test where the pre-check passes and the dispatch is then rejected — asserting the loser still gets the structured error, plus a test that an unrelated dispatch failure is not relabelled as a supersession problem.
• Synthetic-report resurrection (ProjectionRepositories.test.ts + decider): a Phoenix-synthesized terminal report superseded by the agent's own report after resume. The superseded row keeps origin: "system" and stays readable, and the amendment is origin: "agent" — pinned rather than left verified-by-inspection.

Migration ordering: understood, nothing done here — #13's 047 merges before this 048, and I renumber at rebase only if that order slips.

Validation (per the amended resource protocol — no root suite; the serialized merge gate owns CI parity):

Command	Exit
vp run --filter t3 typecheck	0
vp run --filter @t3tools/contracts typecheck	0
vp lint --report-unused-disable-directives	0
targeted vp test — 5 server files, 105 tests	0
targeted vp test — contracts, 55 tests	0

Worth flagging honestly: my first pass at this round had the tests passing but the package failing to typecheck with 51 errors — vitest does not typecheck, so the green test run hid it. Root causes were real, not cosmetic: ProjectionThreadReport.supersedesReportId is string | null while my shared link type said string | undefined (the helper has to accept both spellings), and the race-recovery path returned an error instance where an Effect was required. Both fixed, and the numbers above are from re-runs after the fixes.

[roughcoder]

Delta verdict: APPROVE — merge-ready after #13.

Original findings:

1. [major] RESOLVED — linear-chain enforcement is in the decider (apps/server/src/orchestration/decider.ts:1465-1500). The engine has one queue worker and advances its command read model only after append/project/commit (Layers/OrchestrationEngine.ts:90-215,303-320), so a second queued amendment sees the winner; same-batch pre-image is not possible.
2. [minor] RESOLVED — both handler pre-check/recheck scope listByThreadId to the calling thread (handlers.ts:1016-38,1061-86), and the decider reads only the required thread’s folded reports. Structured winner/head ids therefore do not disclose another thread’s report ids.

New evidence: the amended delivery test drives the reactor and asserts exactly one parent turn with the AMENDED prefix (SessionSpawnReactor.test.ts:478-510). Resurrection proves the old system report receives the derived reverse link and the later agent report is the ordered latest row (ProjectionRepositories.test.ts:280-319). hasReport (same-thread EXISTS) and terminal synthesis dedup (any report) coherently count superseded history.

[nit] Resurrection does not directly exercise read_session/read_report(threadId), hasReport, or reactor dedup after the amendment; source paths are correct (handlers.ts:716-17,1200-08; SessionSpawnReactor.ts:297-305).

Spot-checked 0628e8187: collateral production changes are limited to supersession/error plumbing; the rest is tests/docs/type-safe mocks. Read-only review; no tests/typecheck/lint run.

[roughcoder]

Rebased onto origin/main (now including #18's migration 047). New head: 824c63c8a26565f479d2ee3e1c83e75d355fe463 — PR is back to MERGEABLE.

Conflict and resolution: exactly as you predicted, one file — apps/server/src/persistence/Migrations.ts, both conflict hunks adjacent-line (the import and the registry entry). Kept both registrations in order:

[46, "ProjectionThreadReportAbstract", Migration0046],
[47, "ProjectionQueuedTurnReceipts", Migration0047],
[48, "ProjectionThreadReportSupersedes", Migration0048],

One non-mechanical bit: my registry carried a comment saying "47 is reserved by a sibling wave-2 slice; ids only have to be ordered, not contiguous". That's now false and misleading, so I dropped it rather than carrying a stale note past its expiry. The migrations test file did not conflict — 047 touches queued-turn receipts, a different table from projection_thread_reports — and no renumbering was needed.

Verification on the rebased head:

Command	Exit
npm run typecheck (full repo, 15 packages)	0 — 0 errors
vp test run — migrations 047 + 048 together, plus the supersession suites (handlers, decider.reports, SessionSpawnReactor, ProjectionRepositories)	0 — 6 files, 106 tests
vp test run — contracts supersession suites	0 — 2 files, 55 tests

I deliberately ran 047's migration test alongside 048's rather than only my own: the thing a migration rebase can actually break is ordering, and 048's test migrates through the full chain, so it now exercises 047 landing before it for real.

Pushed with --force-with-lease. CI is running on the new head (checks pending at the time of writing); nothing else outstanding from my side for the gate.

[roughcoder]

Rebased onto origin/main including #15. New head: ff81ae552a171f5ba0b3a5d6bea919a9077b46f4 — MERGEABLE.

Conflicts and how they were resolved. Five files, and #15's insertion points sat exactly on mine, so most hunks were additive-vs-additive:

• packages/contracts/src/sessionOrchestration.ts — 3 hunks (envelope fields, toSessionReportEnvelope body, ReadReportResult). Kept both feature sets at each, usage first to match main's ordering. Result carries usage and supersedesReportId / supersededByReportId / supersededNotice.
• apps/server/src/mcp/toolkits/sessions/handlers.ts — the one that needed real thought rather than keep-both. feat(sessions): usage snapshot for ping_session and post_report #15 now binds const caller = yield* requireShell(...) for its usage snapshot where I had a bare yield* requireShell(...); naively keeping both left a duplicated call, so I dropped mine and use caller. In the dispatch, feat(sessions): usage snapshot for ping_session and post_report #15's usage capture and usage, field now sit inside my withSupersessionRaceDetail(...) wrapper, so the race-loser path and the usage snapshot both survive.
• apps/server/src/mcp/toolkits/sessions/tools.ts — both features rewrote the same post_report description string. Merged the prose rather than picking a side: it now carries feat(sessions): usage snapshot for ping_session and post_report #15's usage-snapshot sentence and the amendment/linear-chain instructions.
• docs/internals/session-orchestration.md — two whole sections at the same anchor; kept both (## Usage snapshot, then ## Amending a report).
• apps/server/src/mcp/toolkits/sessions/handlers.test.ts — import-list hunk only.

Test expectations needed no union edits: #15's report assertions and mine touch disjoint fields, and #15's new postReport.test.ts passes unchanged against the merged handler (I ran it — see below).

One thing worth flagging: when I started, this worktree had an unfinished git merge of origin/main already in progress that I did not create (MERGE_HEAD = 86825f5ae, #15's merge commit) — possibly a leftover from the merge-preview gate. I verified my branch was exactly at the pushed head 824c63c8a with nothing uncommitted and no stashes, aborted that merge, and then did a clean rebase. Nothing of mine was at risk, but you may want to know something is leaving merge state behind in agent worktrees.

Verification on the rebased head:

Command	Exit
npm run typecheck (full repo, 15 packages)	0 — 0 errors
vp test run — contracts (sessionOrchestration, orchestration)	0 — 55 tests
vp test run — supersession + #15's postReport.test.ts and sessionUsage.test.ts + migrations 047/048	0 — 8 files, 125 tests
vp fmt --check	0
vp lint --report-unused-disable-directives	0

I pulled #15's own test files into my targeted run deliberately: I edited the code they cover, so passing my suites alone would not have shown whether the merge preserved their behaviour. vp fmt --check is there because I resolved several hunks with a script and wanted proof I had not left mangled indentation behind.

Pushed with --force-with-lease. Ready for the gate.