Skip to content

feat(server): detect stale worker turns and notify their parent - #65

Merged
lukemaj merged 4 commits into
mainfrom
feat/62-stale-detector
Sep 28, 2026
Merged

lukemaj merged 4 commits into
mainfrom
feat/62-stale-detector

Conversation

@lukemaj

@lukemaj lukemaj commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

A silent spawn_thread child currently has no server-side watcher. This adds one in-memory detector alongside the existing stream clock, exposes its stale decision on /api/prism/liveness, and sends one message to the parent per stale child turn.

Thresholds use recorded healthy gaps per provider/model/phase, switching from conservative defaults at 20 samples. Open tools and host sleep do not count as model silence. Silence requires a further 25% confirmation window; provider failure flags immediately. Saved samples include archived threads. T3 never cancels or retries a worker, and #56's recorded statistics are unchanged.

Router jobs with recognizable model-router title markers are skipped. Renamed Router threads have no cheap membership marker and may receive a duplicate parent message. The planner accepted this caveat because membership is not cheaply determinable for renamed threads and a duplicate notification is harmless.

Closes #62

Proof on 1de2ac84288fb6d25024932935abbc96095e9bbe:

  • cd apps/server && pnpm exec vp test run src/prism src/mcp: 21 files, 159 tests passed.
  • cd packages/contracts && pnpm exec vp test run src/prism.test.ts: 8 tests passed.
  • scripts/fork-check.sh --upstream upstream: passed.
  • pnpm exec tsc --noEmit --pretty false -p apps/server/tsconfig.json: passed, with two existing suggestions.
  • Targeted lint and formatting: passed.

Independent exact-head review found no blocking findings. The reviewer posted the review assessment, and review/independent is success on 1de2ac84288fb6d25024932935abbc96095e9bbe.

Per planner direction, the fork retains upstream package versions without a per-PR version bump or changelog. Website/SEO impact: none; this changes server agent behavior and no public pages or URLs. No merge, release, or installation is included.

Model: GPT-6-Astra. Harness: Codex in T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Sep 28, 2026
@lukemaj

lukemaj commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

REQUEST_CHANGES

  1. P1 — apps/server/src/prism/staleTurnMonitor.ts:31: the required CI Check unused code job fails on this exact SHA because Knip reports notifyStaleParent as an unused export. Remove the unnecessary export or otherwise make the intended use discoverable, then rerun the CI gate. The CI log: https://github.com/toolboxmd/chromeria/actions/runs/36465484681/job/109074750099

Reviewed a173631f46d706eb028c25d88f8b40c15fe26c24 against origin/main (ca3706fbbe4e38ce5cc22f354188dc134a0d8e97). The acceptance scenarios and contract are covered by the source/tests; fork feature mapping and upstream allowlist are present. The Router title-marker duplicate-message limitation is stated in the PR description. No server was started against ~/.t3/userdata.

lukemaj commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Independent review of PR #65 at a173631f46d706eb028c25d88f8b40c15fe26c24: no findings.

I reviewed the exact head against Issue #62. The tests exercise healthy 18-second reasoning gaps, threshold plus 25% confirmation within one tick, open-tool suppression, sleep window resets, immediate provider death/error, per-provider/model/phase thresholds at 20 samples with the healthy-gap floor, the liveness route and schema contract, and exactly one parent message per stale child turn. Router-managed threads are skipped when the existing title marker identifies membership. scripts/fork-check.sh --upstream upstream passes locally; the fork feature entry maps the shared edits, and the changed upstream-owned files are allowlisted.

Accepted caveat: renamed Router threads lose the cheap title marker, so membership may be indeterminate and a duplicate parent notification may be sent. Issue item 7 permits messaging when membership is not cheaply determinable, and the duplicate is harmless. No blocking finding remains.

Review target: a173631

@lukemaj lukemaj left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review at exact head a173631f46d706eb028c25d88f8b40c15fe26c24: no blocking findings. Full criterion assessment and accepted Router title-marker caveat are in the review findings comment. scripts/fork-check.sh --upstream upstream passed locally. This is a review comment, not an approval; GitHub does not allow the PR author to approve their own pull request.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

lukemaj commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Exact-current-head follow-up review of PR #65 at 1de2ac84288fb6d25024932935abbc96095e9bbe: no findings.

The only change since the reviewed a173631f46d706eb028c25d88f8b40c15fe26c24 makes notifyStaleParent module-private in apps/server/src/prism/staleTurnMonitor.ts. Its implementation and call path are unchanged. All prior Issue #62 acceptance findings still hold: the fake-stream timing, open-tool and sleep behavior, immediate provider failure handling, threshold learning and floor, liveness fields and schema contract, one parent message per stale turn, and fork allowlist/feature mapping/check.

Router-managed threads are skipped when title markers make membership determinable. Accepted caveat: renamed Router threads may receive a duplicate parent message when membership is not cheaply determinable; Issue item 7 permits that case, and the duplicate is harmless.

Reviewed candidate: 1de2ac8

@lukemaj lukemaj left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head independent follow-up at 1de2ac84288fb6d25024932935abbc96095e9bbe: no blocking findings. The only change from the prior candidate is module privacy for notifyStaleParent; all prior acceptance findings remain valid. See follow-up findings comment, including the accepted renamed-Router title-marker caveat. This is a review comment, not an approval; GitHub does not permit this PR author account to approve its own PR.

@lukemaj
lukemaj marked this pull request as ready for review September 28, 2026 18:40
@github-actions

Copy link
Copy Markdown
Contributor

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.5 KiB −12 B (−0.1%) 15.1 KiB ✅
Codex Thread snapshot wire 7.1 KiB 7.1 KiB +2 B (+0.0%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.5 KiB 6.5 KiB −14 B (−0.2%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.3 KiB 56.2 KiB −44 B (−0.1%) 66.4 KiB ✅
Codex Live turn messages 10 9 −1 (−10.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB +7 B (+0.1%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB +2 B (+0.0%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.4 KiB 6.4 KiB +5 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: ca3706f · PR result: 1de2ac8 · 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stale-turn detector: tell the parent when a worker stops streaming

1 participant