fix(server): child threads keep reporting to their parent after a restart (#48)

[lukemaj]

Closes #48.

Problem

Router child threads survive a server restart. The parent link and role stay in the sub.<parent>.<role>-<rand> id, and every provider resumed its native session. But the threads toolkit bridge kept each child's reportBack flag and its last reported message only in memory. After a restart, no finished child turn reached its parent. In the live run, 0 of 6 post-restart child turns were reported, against every turn before the restart.

Fix

• The bridge writes reportBack into the child's task.started row and reportedMessageId into the task.progress row that triggers a report, both on the parent.
• The first time a child's session changes after a restart, the bridge restores both values from the parent's activities. That restore lives in the new pure module childReportState.ts.
• Every idle task.progress row carries both values, and restore reads the newest row per child from one read of the parent. A child with no row left in the parent's 500-activity window restores as reportBack: true, with its current last reply counted as already reported. The remaining gap is documented in childReportState.ts.

Files:

• new apps/server/src/mcp/toolkits/threads/childReportState.ts and its test
• apps/server/src/mcp/toolkits/threads/handlers.ts: fork-owned and already allowlisted; persist and restore
• docs/fork-features.md: child-threads gains the new files and Verify Router child threads continue after a server restart #48

No upstream-owned file changed.

Proof

• childReportState.test.ts runs the real engine and projections on one SQLite file and restarts the toolkit twice against it, waiting on domain events with no sleeps:
  1. Spawn a child.
  2. After a restart, the child's finished turn is reported to the parent.
  3. After another restart, an idle transition with no new reply is not reported again.
• The restart test fails against the old handlers.ts (the report never arrives) and passes with the fix.
• Checks: vp test run src/mcp/toolkits/threads/ 30/30; apps/server tsc --noEmit exit 0; vp lint and vp fmt clean on touched files; scripts/fork-check.sh OK; fork-maintenance.test.ts 12/12.
• Live run on an isolated server, recorded on Verify Router child threads continue after a server restart #48:
  • Claude, Codex, OpenCode and Grok children, plus a Claude reviewer role child, kept their parent link, role, native session ids and earlier context across a restart.
  • A mid-task Claude child was continued automatically.
  • After the fix, post-restart Codex and OpenCode replies were reported to the parent once each.

Independent review is still open, as #48 requires.

Model and harness: Claude Opus 5.5 in Claude Code (running inside T3 Code).

🤖 Generated with Claude Code

[github-actions]

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	13.5 KiB	13.6 KiB	+64 B (+0.5%)	15.1 KiB	✅
Codex	Thread snapshot wire	7.1 KiB	7.1 KiB	−1 B (−0.0%)	7.3 KiB	✅
Codex	Live turn WebSocket wire	6.4 KiB	6.5 KiB	+65 B (+1.0%)	7.8 KiB	✅
Codex	Live turn WebSocket decoded	56.2 KiB	56.3 KiB	+44 B (+0.1%)	66.4 KiB	✅
Codex	Live turn messages	9	10	+1 (+11.1%)	21	✅
Claude	Total thread wire	13.5 KiB	13.5 KiB	−4 B (−0.0%)	15.1 KiB	✅
Claude	Thread snapshot wire	7.1 KiB	7.1 KiB	+5 B (+0.1%)	7.3 KiB	✅
Claude	Live turn WebSocket wire	6.4 KiB	6.4 KiB	−9 B (−0.1%)	7.8 KiB	✅
Claude	Live turn WebSocket decoded	57.0 KiB	57.0 KiB	0 B (0.0%)	66.4 KiB	✅
Claude	Live turn messages	9	9	0 (0.0%)	21	✅

Baseline: 8518d0d · PR result: 246dc38 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

• Codex decoded thread snapshot: 113.9 KiB
• Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

[lukemaj]

Independent review of #49 at 3cc3153843bc619f06f02f98c5c952758fcc6288

Verdict: changes requested (1 medium finding). The core fix is correct and the restart test is sound.

Verified

• Storage. Only fork-owned state. The flag and message id are extra keys in the fork's own task.* activity payloads on the parent, written through the existing thread.activity.append command. No upstream table, event type, contract or migration changes. scripts/fork-check.sh passes (14 features); handlers.ts is fork-owned; the new files are listed under child-threads in docs/fork-features.md.
• Restart and multiple restarts. State is restored lazily on a child's first thread.session-set in each process. reportedMessageId is written on every report, so the latest value wins across any number of restarts.
• No double delivery in the covered path. lastReported is set and persisted before startTurn, the same at-most-once order as before.
• Deleted parent. getThreadDetailById returns None and the default reportBack: true applies. Appending to the missing parent then fails and the bridge logs and skips the event, so nothing is delivered. This matches the pre-fix behaviour.
• Restart test. The test restarts against the same SQLite file three times and waits on domain events with no sleeps. The ordering argument in the third step holds because the bridge handles events one at a time. I ran it at this head: 3/3 pass. With handlers.ts from origin/main it fails, because the report never arrives. It fails by the 120 s vitest timeout rather than an assertion, which is acceptable.

Findings

1. Medium: restored state is read from a truncated window, so old children can deliver twice or get the wrong reportBack.

   • Where: childReportState.ts:34 (default reportBack = true) and handlers.ts:213.
   • Cause: getThreadDetailById returns only the parent's newest 500 activities (THREAD_DETAIL_ACTIVITY_LIMIT, ORDER BY sequence DESC LIMIT 500). Those 500 include all of the parent's own tool activity, so a busy orchestrating parent pushes a child's task.started row, and its last reportedMessageId row, out of the window within one working session.
   • Failure scenario A: a child spawned with reportBack: false falls out of the window. After a restart it restores as true, and its next finished turn starts an unwanted turn on the parent.
   • Failure scenario B: the child's last reportedMessageId row falls out of the window. This also applies to every child spawned before this change, which has no such row. After a restart, lastStatus is empty, so the first idle session-set with no new reply re-sends the last reply to the parent as a new turn. Idle session-set events with no new reply happen: session.started maps to ready in ProviderRuntimeIngestion, and restoreCompaction sets ready. Checking for this double delivery is part of the PR's stated goal.
   • Fix, smallest option: include reportBack and the latest reportedMessageId in every task.progress idle row the bridge writes, not only on task.started and on reporting rows. Restore then reads the newest row for the child. That narrows the gap to children with no row in the window. For those, use the child's current last assistant message id as the baseline lastReported, so a missing record means "don't re-send what already existed". A reply finished while the server was down would then need a new turn to be reported; I think that trade is better than a spurious parent turn. Whichever way you go, state the remaining gap in the childReportState.ts doc comment.

2. Low (performance): every restored child loads the parent's full detail.

   • Where: handlers.ts:209-223.
   • Cause: restoreReportState calls getThreadDetailById(parent), which decodes all of the parent's messages plus 500 activity payloads, once per child after each restart.
   • Impact: a parent with 20 children costs 20 full detail reads when those children first change session. It is lazy and runs once per process, so this is not blocking.
   • Fix: restore every child of that parent from a single read, for example by caching childReportStateFrom results per parentId until the first restore completes.

3. Low (test coverage): persisting reportBack: false across a restart is covered only by the pure unit test.

   • Where: childReportState.test.ts:57.
   • Gap: no integration test spawns with reportBack: false, restarts, and asserts that nothing is reported.
   • Fix: add that case to the restart test, since restoring false is half of the persisted state.

AGENTS.md taste is fine: a small pure module, the comments describe usage, and the replaced "process-local by design" comment is rewritten rather than appended to.

Reviewed by Claude Opus 5.5 in Claude Code (running inside T3 Code).

[lukemaj]

Review findings addressed in 246dc38a94848e5bb65465dbc2ea99d3f5694fe7:

1. Truncated window (medium).
   • Every idle task.progress row now carries reportBack and the latest reportedMessageId, and restore reads the newest row per child.
   • A child with no surviving row restores as reportBack: true, with its current last assistant message counted as already reported, so nothing old is re-sent.
   • Remaining gap, stated in the childReportState.ts doc comment: a reportBack: false child whose rows all left the window reports again after a restart.
2. One read per parent (low). The first restore for a parent reads it once and restores all of its children. Only a child with no row costs one extra read, of its own last message.
3. Coverage (low). The restart test now spawns a reportBack: false child and checks two things after the restart: its idle row still says reportBack: false, and it is never reported. Mutation check: when restore skips the parent read, the test fails on reportBack: true.

Checks at this head:

• vp test run src/mcp/toolkits/threads/: 31/31.
• apps/server tsc --noEmit: exit 0.
• vp lint and vp fmt on the touched files: clean.
• scripts/fork-check.sh: OK.

[lukemaj]

Re-review of #49: delta 3cc3153 → 246dc38a94848e5bb65465dbc2ea99d3f5694fe7

Verdict: approved. All three findings from the previous review are resolved.

1. Truncated window (medium): resolved.
   • Every idle task.progress row now carries reportBack and the latest reportedMessageId. childReportStatesFrom reads the rows oldest first and takes the newest per child, keeping an earlier reportedMessageId when a newer row lacks one.
   • A child with no surviving row takes its current last assistant message as its baseline. Restore runs on the child's first session-set after a restart, which comes before a new reply lands, so an old reply is not re-sent and a new one still is.
   • The remaining reportBack: false gap is stated in the doc comment. I accept it.
2. One read per parent (low): resolved. restoredParents means one parent read per process, plus one read of a child's own messages only for a child with no row.
3. Coverage (low): resolved. The quiet child's idle row still says reportBack: false after the restart, and the third-process assertion confirms it is never reported.

Non-blocking notes:

• If the parent read fails, the parent is still marked restored, so its children fall back to the unrecorded default for that process. This only happens on a transient read error.
• The unrecorded-child path is covered only by the pure unit test.

At this head, vp test run src/mcp/toolkits/threads/ passes 31/31, run in a temporary worktree that I have since removed.

Reviewed by Claude Opus 5.5 in Claude Code (running inside T3 Code).