Skip to content

fix(spectrum): preserve successor during restart cleanup - #137

Merged
lukemaj merged 1 commit into
mainfrom
fix/118-recovery-stop-race
Oct 2, 2026
Merged

lukemaj merged 1 commit into
mainfrom
fix/118-recovery-stop-race

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

What: Preserve genuine successors during Spectrum restart cleanup.
Why: Recovery read request identity then queued an unconditional stop, leaving a physical-stop race.
So what: The server-only fix is proved, independently reviewed and CI-green on the exact head; the user merges this one PR.

Follow-up for #118, parent #113 and merged PR #130, without closing linkage. #118 stays closed. This strengthens the Spectrum outcome in the current Objective.

Recovery intent carries the unique request event sequence in its command ID. The fork-owned guard runs in the engine's existing serialized command decision, rejecting superseded stops before persistence. A reactor-only guard was discarded because the shared client reducer projects a persisted stop event as stopped even when its physical effect is skipped. No client or contract changes.

An accepted stop precedes every later request. The reactor handles persisted events in order, completes compaction cancellation, physical stop and stopped session update before admitting the later request's send. Deterministic Deferred/receipt/drain tests hold an admitted send across stale cleanup, and park the worker while persisting a successor after an accepted stop. They assert physical provider effects and session state, not rejection alone. Spectrum's outbox tolerates a rejection only after confirming that recovery intent is superseded, preserving report/settlement progress and replay.

Requirements and who asked: User requires atomic identity-based stop, physical successor survival, failing-before regression, ordinary-stop compatibility and exactly one server-only PR.
Deleted: Misleading reactor-only guard, new machinery, contracts/client changes, recovery loops, new Issue, versioning, browsers, live servers and installs.
Bottleneck: Request identity must remain valid until stop persistence, and persisted event order must protect forked provider admission.
Checked myself: Full #118/#113 and #130 body, authored diff, engine queue/reactor stop/send path, client reducer projection, actual baseline/final logs, fork registrations, clean local/remote head.

Candidate

Base 86d3269c6eac9dde4697a0fce105efc2abb907e9; head 1af9d1ab6d1dfb3c882ff67d809955ed3847f3e8. Clean pushed branch fix/118-recovery-stop-race, exclusive workspace /Users/lukaszmaj/dev/toolboxmd/chromeria-worktrees/118-recovery-stop-race. Writer stopped. No canonical/caller/sibling writes.

Four files changed: apps/server/src/mcp/toolkits/threads/spectrum.ts, spectrumIdentity.ts, spectrum.test.ts, and apps/server/src/orchestration/Layers/OrchestrationEngine.ts. Engine is already registered in scripts/fork-upstream-edits.txt:129 and docs/fork-features.md:173 (also :941), verified directly. No duplicate registry entry added.

Proof

Node 24.13.1, pnpm 11.10.0. Commands run with PATH=/Users/lukaszmaj/.nvm/versions/node/v24.13.1/bin:$PATH:

pnpm exec vp test run apps/server/src/mcp/toolkits/threads/spectrum.test.ts -t 'unbound restart|legacy Spectrum recovery stop'
pnpm exec vp test run apps/server/src/mcp/toolkits/threads/spectrum.test.ts apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts apps/server/src/scheduler/SpectrumIntegration.test.ts apps/server/src/orchestration/decider
pnpm exec vp exec tsc --noEmit -p apps/server/tsconfig.json
pnpm exec vp lint apps/server/src/mcp/toolkits/threads/spectrum.ts apps/server/src/mcp/toolkits/threads/spectrumIdentity.ts apps/server/src/mcp/toolkits/threads/spectrum.test.ts apps/server/src/orchestration/Layers/OrchestrationEngine.ts
pnpm exec vp fmt --check apps/server/src/mcp/toolkits/threads/spectrum.ts apps/server/src/mcp/toolkits/threads/spectrumIdentity.ts apps/server/src/mcp/toolkits/threads/spectrum.test.ts apps/server/src/orchestration/Layers/OrchestrationEngine.ts
node scripts/fork-features.mjs check
git diff --check

Before fix, final assertions with production restored to exact base: 2 failed, 3 passed, exit 1. Both failures observed unexpected physical stop:sub.spectrum... calls, for successor and legacy recovery. Reversed order and two existing recovery cases passed. Patch restored and matches committed production. An earlier reversed-order expected-state error was an oracle correction, not defect evidence.

Final engine candidate: 266 passed in 19 targeted files, exit 0. Typecheck, targeted lint/format, registry (23 features), diff check all pass. No earlier 113-test reactor proof reused and no repo-wide checks. The decider argument is a Vitest filename filter selecting 15 decider test files, not a directory or shell glob; expanded names will be recorded below.

Regressions prove: successor admitted between identity read and stop survives physically, no stale stop event, no interruption, final session survives; replay finishes one report and one settlement; successor persisted after accepted stop sees physical stop/start/send order and stopped write before its own writes; legacy recovery intent is refused while ordinary stop still persists and physically stops.

Logs outside workspace: /tmp/promachos-coord/118-recovery-stop-race/, notably baseline-4-engine-assertions.log, final-tests.log, final-typecheck.log, final-lint.log, final-fmt.log, final-fork-features.log, production.patch. Decisive sanitized results are retained here, no transcript uploads.

The 15 decider test files selected by the filename filter were:

  • apps/server/src/orchestration/decider.active-order.test.ts
  • apps/server/src/orchestration/decider.autoSettleSet.test.ts
  • apps/server/src/orchestration/decider.delete.test.ts
  • apps/server/src/orchestration/decider.import.test.ts
  • apps/server/src/orchestration/decider.pinned.test.ts
  • apps/server/src/orchestration/decider.projectScripts.test.ts
  • apps/server/src/orchestration/decider.projectThreadEnvMode.test.ts
  • apps/server/src/orchestration/decider.pullRequests.test.ts
  • apps/server/src/orchestration/decider.questionAttachments.test.ts
  • apps/server/src/orchestration/decider.settled.test.ts
  • apps/server/src/orchestration/decider.snoozed.test.ts
  • apps/server/src/orchestration/decider.titleRegeneration.test.ts
  • apps/server/src/orchestration/decider.turnDiffComplete.test.ts
  • apps/server/src/orchestration/decider.userInputDismiss.test.ts
  • apps/server/src/orchestration/decider.userMessageAppend.test.ts

Review and cost

Passed on exact head 1af9d1ab6d1dfb3c882ff67d809955ed3847f3e8. Independent findings and verdict. Reviewer: Prism GPT-6-Luna through Codex/T3 Code, thread sub.sub.sub.sub.46b03420-8643-4703-8756-8602f14b39fb.dispatcher-aecbb8dc7270.dispatcher-fda19fc83a22.dispatcher-52147369cc30.reviewer-a2810d15e065. Newest review/independent=success, authenticated creator lukemaj, targets those findings. No supported blocking finding; reviewer traced forked send admission, physical stop/session writes, rejected-stop projection, outbox replay and compatibility with main's stored-effort change d0fbed7482655a492d44bdb5c7893751034fba91. Requested original base retained, no rebase. Dispatcher additionally inspected main advancement to dafca350377d015811aa2d22efc5d16a1b5f7b24: #138 changes only repository-identity/Git remote parsing and registry entries, not this seam. No new candidate or transferred review verdict.

Agent Observer cost publication uses the instructed sync, task show --task toolboxmd/chromeria#137 --json, and publish --task toolboxmd/chromeria#137 --repo toolboxmd/chromeria --pr 137. Pricing is partial because GPT-6.1-Sol has no known rate; usage is not subscription spending. Refreshed after review with sync and the same publish command, which updated the existing comment successfully.

CI run 36980128289: first attempt's only named assertion failure was unchanged scripts/knip-schemas.test.ts:13, Test timed out in 20000ms, 329 script tests passed. Runner summary said four failed tasks but identified no other assertion failure, so no additional cause is claimed. The disposable fixture imports Effect schemas and no changed recovery path. PATH=/Users/lukaszmaj/.nvm/versions/node/v24.13.1/bin:$PATH pnpm exec vp test run scripts/knip-schemas.test.ts passed (1 test, 3.21 seconds, exit 0). CI run 36980128289, attempt 2, is SUCCESS on the unchanged head: the bounded rerun Test job 110754499621 passed. Check, all three server shards, Test, Rust, Release Smoke and Mobile Native Changes passed; Mobile Native Static Analysis was intentionally skipped. The initial timeout is superseded by this green retry. No unrelated code/test edits or timeout changes.

Assumptions and remaining items

Legacy fork-only recovery stops without identity are deliberately refused. A saved legacy outbox may leave the original hidden Drafter turn to finish, while Spectrum settles/reports uncertainty. Ordinary stops retain existing behavior, including upstream's lack of cancellation for an already-running non-retirement send fiber. No live verification claimed. No open implementation or proof items remain; user merge is the remaining delivery action. No versioning by design. No merge/install. Website/SEO impact none: server restart cleanup changes no website content, URLs or presentation. Task branch/workspace retained for user merge and merger-owned finalization.

Dispatcher: GPT-6.1-Sol through Codex/T3 Code. Writer: Prism worker/hard claudeAgent/claude-opus-5-5, thread sub.sub.sub.sub.46b03420-8643-4703-8756-8602f14b39fb.dispatcher-aecbb8dc7270.dispatcher-fda19fc83a22.dispatcher-52147369cc30.worker-9a3f366d6b6c, Claude Code/T3 Code. Independent reviewer: Prism GPT-6-Luna through Codex/T3 Code, exact-head verdict passed.

…quest

Restart recovery read a Drafter's latest request and then dispatched a plain
session stop, so a genuine successor request landing in between was stopped.

The stop now names the request it may end by its event sequence. The engine
refuses it, inside the serialized decide step, when the Drafter has a newer
request, and always refuses the legacy unnamed form. A refused stop is never
persisted, so neither the reactor nor any client projects it as stopped.
Spectrum's outbox moves on only when that refusal is a superseded stop, live
or replayed. Accepted stops precede every later request, which the reactor
handles only after the physical stop finishes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M labels Oct 2, 2026
@lukemaj

lukemaj commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Agent work on this PR

Estimated cost at least $7.46 · 184 responses · 73 sessions · 12.5 h wall time

Model Responses Tokens Estimated cost
claude-opus-5-5 (claude, xhigh) 86 16,656,865 $7.44
gpt-6.1-sol (codex, medium) 89 8,233,014 unknown
gpt-6-luna (codex, medium) 9 741,621 $0.02

Flags: 6 human corrections · 124 large tool outputs · 2 permission requests · 64 repeated commands · 15 repeated failures · 36 repeated reads · 11 repeated skill loads · 89 unpriced responses

Details: snapshot, prices, coverage, counters
  • Task toolboxmd/chromeria#137: outcome unknown (recorded acceptance only; a finished process never implies it).
  • Proof: fix(spectrum): preserve successor during restart cleanup #137
  • Snapshot 973350d4562b5fd923dee1c9ed516251b47855bde4fc3aeb32583643006e8f2d, records up to 2026-10-02 07:52 UTC.
  • 73 sessions on claude, codex; AgentsMD 14.6.0.
  • Not counted: 3,935 responses (at least $52.41) in sessions shared with other PRs that worked in no single PR's checkout.
  • 3,456 responses in these sessions worked on other PRs and are counted there.
  • Totals reconcile with the measured sessions: yes. Evidence complete: yes.
  • Prices: list-price estimate from T3 local rate table (path withheld) as of 2026-09-28, schedule 696aae45933d0a684a015d3f08cfb6f1aa2fef02e7d272b4689e3a692ea04fff. Unknown prices stay unknown, never zero.
    • T3 LiteLLM rate table when present; bundled schedule covers the rest.
    • Model price sources: T3 local rate table (path withheld).
    • Unpriced: unknown model (89).
  • Harness-reported cost: none reported.
  • Usage totals are not billing. Subscription spending is separate and is never posted as spend.
  • Crashed runs are counted separately: 0.

Token counters by model (native counter semantics; never added across semantics):

  • claude-opus-5-5 (claude, xhigh), claude counters: input excludes cache, output includes thinking: input 174; cache-read 16,310,161; cache-write 229,673; output 116,857; reasoning 80,348; harness total 16,656,865.
  • gpt-6.1-sol (codex, medium), codex counters: input includes cached, output includes reasoning: input 8,213,311; cache-read 8,097,408; cache-write 0; output 19,703; reasoning 1,160; harness total 8,233,014.
  • gpt-6-luna (codex, medium), codex counters: input includes cached, output includes reasoning: input 732,858; cache-read 641,792; cache-write 0; output 8,763; reasoning 5,297; harness total 741,621.

Generated tokens (reasoning and other output are disjoint; other output includes code, tool calls and replies):

Model Reasoning Other output Reasoning share Split known for
claude-opus-5-5 (claude, xhigh) 80,348 36,509 68.8% 86 of 86
gpt-6.1-sol (codex, medium) 1,160 18,543 5.9% 89 of 89
gpt-6-luna (codex, medium) 5,297 3,466 60.4% 9 of 9

Usage by work phase (this task's own responses):

Phase Model Responses Input Cache read Cache write Output Reasoning Total Priced part
unclassified claude-opus-5-5 (claude, xhigh) 86 174 16,310,161 229,673 116,857 80,348 16,656,865 $7.437252
unclassified gpt-6.1-sol (codex, medium) 89 8,213,311 8,097,408 0 19,703 1,160 8,233,014 unknown
unclassified gpt-6-luna (codex, medium) 9 732,858 641,792 0 8,763 5,297 741,621 $0.019906

Recorded activity by phase (observations, not separate token bills):

Phase Tool calls MCP results Reads File changes Failed tool results
unclassified 282 116 30 0 16

Selected rates (USD per million tokens). These rates value the report at the selected schedule date; they do not establish historical prices or subscription spending.

Model Input tier Input Cache read Cache write Other output Reasoning
claude-opus-5-5 all 4 0.2 5m: 5; 1h: 8 20 20
gpt-6-luna all 0.1 0.01 0.125 0.5 0.5
gpt-6.1-sol all unknown unknown unknown unknown unknown

Other output and reasoning are priced without double counting inclusive native output. Missing rates remain unknown.

Local measurement from native records; usage totals are not billing. Updated in place by agent-observer publish.

@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful main measurement shown below.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.5 KiB −11 B (−0.1%) 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.4 KiB −12 B (−0.2%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.2 KiB 56.2 KiB 0 B (0.0%) 66.4 KiB ✅
Codex Live turn messages 9 9 0 (0.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB +3 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 +8 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: 86d3269 · PR result: 1af9d1a · 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: 114.0 KiB
  • Claude decoded thread snapshot: 114.7 KiB

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

@lukemaj

lukemaj commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent review found no actionable defect on 1af9d1ab6d1dfb3c882ff67d809955ed3847f3e8.
Why: The identity check and stop decision run in the engine's serialized command worker, before persistence and publication; persisted event ordering then carries through the single provider reactor worker.
So what: Review passes for this exact SHA; the dispatcher is investigating the separate targeted CI fixture timeout, and the coordinator owns merge.

Review trace

  • spectrum.ts records the exact thread.turn-start-requested sequence in the fork-only stop command ID. spectrumIdentity.ts accepts only the Spectrum stop shape, matching participant ID, and an integer sequence; old sequence-less recovery IDs are refused.
  • In OrchestrationEngine.processEnvelope, the guard runs after receipt replay handling but before decider execution, transaction, event append, projection, and PubSub publication. The command worker takes one envelope at a time, so no turn request can commit between this read and the stop decision. A stale stop therefore creates no thread.session-stop-requested event for the server projector or client reducer to misread as stopped.
  • An accepted stop is committed and published before any later command. The ProviderCommandReactor subscribes to that ordered stream and enqueues stop and turn events to one drainable worker. The stop handler clears compaction-queued turns, performs physical stopSession, then persists the stopped session; only afterward can that worker admit a later request. The regression asserts stop/start/send order and that the stopped write is not the final session state. Forked sends and compaction resumes do not bypass this event admission; retirement-only interruption remains unchanged.
  • Spectrum's rejected-stop path only advances when the same named request has a later persisted turn-start event. Its outbox state is durable before effects, and replay remains command-receipt guarded; completion, report, and settlement commands remain in the outbox. The intentional compatibility cost is that an old unnamed recovery stop may be refused, allowing the original hidden Drafter turn to finish while Spectrum reports uncertainty.
  • Main advanced to d0fbed7482655a492d44bdb5c7893751034fba91 while this candidate remains based on 86d3269c6eac9dde4697a0fce105efc2abb907e9. I compared the overlapping changes: main's Spectrum edits add stored model selection to resumed/provider requests and its test; they do not change the stop identity/engine guard seam. No rebase or workspace mutation was made.

Evidence inspected

Read the complete four-file PR diff, relevant event/queue/reactor and Spectrum recovery/send/settlement consumers, current workspace AGENTS.md, full #118 and #113, PR #130 body, and CONTRIBUTING.md. Inspected the supplied baseline and final test, typecheck, lint, format, fork-registry and production-patch evidence. Baseline shows both physical-stop regressions failing (2 failed, 3 passed); final targeted suite reports 266 passed in 19 files, with typecheck, lint, format and registry checks passing. No repository-wide test, browser, live server, install, or code write was performed by this reviewer.

No versioning by design. No merge or approval requested.

@lukemaj
lukemaj merged commit 7dd561e into main Oct 2, 2026
30 of 31 checks passed
@lukemaj
lukemaj deleted the fix/118-recovery-stop-race branch October 2, 2026 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 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.

1 participant