Skip to content

fix(server): skip stale restart-cut results for settled tasks - #329

Merged
lukemaj merged 2 commits into
mainfrom
fix/326-current-restart
Oct 14, 2026
Merged

lukemaj merged 2 commits into
mainfrom
fix/326-current-restart

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What: Restart recovery skips historical cancelled follow-up results whose delegated task already returned a result, and records why.
Why: Startup result discovery treated old untransferred legacy cuts as new results and woke their parents again.
So what: Merged into main after exact-head Sol/Grok success and green CI; no restart or deployment follows.

Closes #326. Supports #264's Resume after restart part and reliable Drafters under the Objective.

Read-only live diagnosis found no new child runs at the reported boot. The three stale wakes were transfers of old cancelled follow-ups. Recovery now recognizes the matching cancelled provider turn and already-returned task result, persists a fixed TASK_SETTLED decline through the guarded EventSink transaction, and excludes that resolved cut from SQL and memory result discovery. Existing continuation admission still rejects newer work; fresh cuts, pending continuation intents, first results and normal follow-up results keep their existing paths.

The regression reopens SQLite with fresh runtimes twice, reproduces the legacy unbound-child/event shape, and asserts no stale result transfer, a durable reason, and no rediscovery. It failed before production edits with expected true to be false on the stale transfer assertion. Final author verification passed seven focused files and 168 tests. Sol independently passed those tests plus 32 persistence/projection tests; Grok independently passed the seven files and 168 tests. The exact final regression also fails on the pinned base at the stale-transfer assertion. No server/native restart or deployment was used; adapters are faked only at the external harness boundary. No version bump. Website impact: none; this changes server recovery and a shared reason string without site or URL changes.

Requirements and who asked: The user requires no stale wakes from superseded restart cuts, recorded skip reasons, a two-restart regression, focused tests, exact-head independent Sol/Grok success and green CI before delegated merge.
Deleted: No provider or queue policy change, blanket notification mute, new design round, version bump, live restart or deployment.
Bottleneck: Resolved: both independent reviews and complete CI passed on eef68e9be143dfeab1c32f088e3be78765f8182f; merged as fbbe448316be1af24aeefb3ed124b152e8817253.
Checked myself: Matched all three live child/transfer identities through read-only queries, reproduced legacy stale result replay before editing production code, observed 168 focused passes and verified both reviewers’ exact-head findings and status identities after their comments, plus complete green CI, inspected the guarded metadata write and both discovery implementations, and checked fork ownership.

Sol review, Grok review, final failing-before proof, and green CI bind to the reviewed head. Both required named statuses were published by lukemaj after their findings comments. Scoped lint and fork ownership checks pass.

Implemented by GPT-6.1-Sol through the Codex harness in T3 Code.

@github-actions github-actions Bot added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Oct 14, 2026
@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: The reproduced stale-result fix is ready for independent review at eef68e9be1.
Why: Seven focused files pass 168 tests, including the two-restart legacy-child regression, fresh-cut continuation controls and EventSink persistence guards.
So what: Fresh Sol and Grok children review this exact head and publish their findings before named success statuses; CI must also be green before the delegated merge.

The map now links PR #329 on RS/#264. No version, lockfile, provider/queue policy or deployment changes. The new test uses fresh SQLite runtimes and the legacy persisted cancellation shape, with external harness fakes. It is not native-server restart proof. The initial stale-transfer reproduction failed before production changes with expected true to be false; the final focused run is 168 passed in seven files, exit 0. Scoped lint reports only the pre-existing unused layerUnavailable warning; diff and 36-feature ownership checks pass. No declared delivery profile deltas.

Resource correction: the unused full database backup was removed after fuser showed no open reader. /tmp use fell from 62% to 15%. All subsequent live evidence uses direct read-only queries; retained small reports/logs are on disk under the task cache.

Command: npx -y pnpm@11.10.0 exec vp test run apps/server/src/orchestration-v2/RestartRecovery.integration.test.ts apps/server/src/restartContinuation/EventSinkRestartCut.test.ts apps/server/src/restartContinuation/UnresolvedRestartCuts.test.ts apps/server/src/orchestration-v2/RestartContinuation.test.ts apps/server/src/orchestration-v2/DelegatedCompletionDelivery.test.ts apps/server/src/orchestration-v2/SubagentProjection.test.ts apps/server/src/orchestration-v2/FoundationPersistence.test.ts.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: The final committed regression also reproduces the stale startup transfer on unchanged base 555f0a00198b7aba75d8e8ce13af9c7d983712f4.
Why: Replaying the legacy cancellation under the real command:runtime-reconcile: listener exclusion isolates second-boot discovery from an ordinary terminal-listener wake.
So what: Reviewers can rely on a failing-before comparison of the exact final test, in addition to the candidate's 168 passing focused tests.

A separate task-owned detached worktree at the base received only RestartRecovery.integration.test.ts copied from exact candidate eef68e9be143dfeab1c32f088e3be78765f8182f. No production source changed there. After the prescribed pinned install, npx -y pnpm@11.10.0 exec vp test run apps/server/src/orchestration-v2/RestartRecovery.integration.test.ts -t 'two restarts' exits 1: one failed, eight intentionally unselected. The failure is expected true to be false at the stale subagent_result transfer assertion after the second fresh SQLite/runtime recovery. It is not a setup, missing-method or new-reason assertion failure. The same final test passes in the candidate's seven-file, 168-test run.

All retained logs are on disk in the private task cache. No live server, native provider, browser, deployment or restart was used. The temporary baseline worktree is test-only and will be retired with the task.

@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 5.0 KiB 5.0 KiB −7 B (−0.1%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB −7 B (−0.6%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB +24 B (+0.5%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB +24 B (+2.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB +41 B (+0.2%) 29.3 KiB ✅
Claude Live turn messages 1 2 +1 (+100.0%) 8 ✅

Baseline: 555f0a0 · PR result: eef68e9 · 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: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

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

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Sol r1 finds no supported blocking defect in PR #329 at eef68e9be143dfeab1c32f088e3be78765f8182f.
Why: The fix prevents replay of the demonstrated legacy cancelled follow-up result, records its reason durably, and preserves existing recovery paths in 200 independently passing focused tests.
So what: The assigned Sol and generic review statuses follow this comment on this exact head; root owns adjudication and must obtain the separate Grok success and complete green CI before the authorized merge.

Independent implementation review: GPT-6.1-Sol, MAX effort, Codex harness. I authored none of this implementation.

Reviewed head: eef68e9be143dfeab1c32f088e3be78765f8182f.
Pinned base and merge-base: 555f0a00198b7aba75d8e8ce13af9c7d983712f4, intended base main.
Scope: the complete cumulative eight-file diff and surrounding result discovery, finalization, EventSink transaction, continuation admission, projection and test consumers. The work supports reliable Drafters under the current Objective.

Verdict: SUCCESS for independent implementation review on this exact SHA. No supported actionable blocking finding remains in the inspected scope. There were no previous review rounds on #329. I read #326, #264 and historical PR #301 in full, including their available comments/reviews, and the current #329 body/comments. Historical #301 findings and accepted limitations inform this review; their verdicts are not transferred.

Cleared concerns:

  • Mechanism and legacy ownership: The diagnosis establishes historical result replay rather than newly started child runs. Orchestrator.ts:11177 reaches the new check through the existing app-owned child/task and delegated-run ownership rules. It requires an already-returned task, an existing result transfer, no cut metadata, and a cancelled provider turn on that run's active attempt with the same completion time. A pending continuation on the source or later work keeps the existing path. First-result finalization remains outside this suppression.
  • Durability and currentness: EventSink.ts:551 rechecks the fresh cancelled source, matching cancelled attempt/turn timestamp and settled app-owned child before admitting the new legacy ownership case. It patches metadata through the existing event/receipt transaction. Natural outcomes, an existing replacement and a final decline still win. The existing exact-owned intent branch remains. Guarded no-ops retain no standalone receipt; append failure rolls the transaction back.
  • All result-discovery paths: delegatedFollowupQueries.ts:18 and its restart-result UNION exclude the recorded settled cut. delegatedFollowups.ts:109 and ProjectionStore.ts:648 cover memory discovery and per-run finalization. The no-transfer first-result fallback remains applicable to first results. Exact-run dedup, normal completed follow-ups, other recorded decline reasons, fresh cuts and linked replacements retain their existing routes.
  • Continuation and scope: Actual continuation dispatch still checks newer work, Stop, archive/provider changes and final declines. The fix adds no age heuristic, provider policy, queue policy, transport, schema migration or service architecture. Existing modern recovery writes cut metadata, distinguishing fresh cuts from this legacy shape. The map adds PR fix(server): skip stale restart-cut results for settled tasks #329 to the existing restart part; versions and dependency inputs are unchanged.
  • Regression seam: RestartRecovery.integration.test.ts:637 uses real SQLite and fresh runtimes across two restarts. It actually starts the first continuation, returns the task result, creates the legacy unbound cancelled follow-up under the real reconciliation listener exclusion, then observes no stale transfer, a durable TASK_SETTLED reason and no rediscovery. The external harness is the fake boundary; assertions await persisted events or drain the worker without added sleeps/timeouts.

Executed proof, all on the reviewed head in this isolated review worktree:

npx -y pnpm@11.10.0 exec vp test run apps/server/src/orchestration-v2/RestartRecovery.integration.test.ts apps/server/src/restartContinuation/EventSinkRestartCut.test.ts apps/server/src/restartContinuation/UnresolvedRestartCuts.test.ts apps/server/src/orchestration-v2/RestartContinuation.test.ts apps/server/src/orchestration-v2/DelegatedCompletionDelivery.test.ts apps/server/src/orchestration-v2/SubagentProjection.test.ts apps/server/src/orchestration-v2/FoundationPersistence.test.ts
npx -y pnpm@11.10.0 exec vp test run apps/server/src/restartContinuation/RestartPersistence.test.ts apps/server/src/orchestration-v2/ProjectionRecovery.test.ts

Results: 7 files / 168 passed, exit 0, 48.69 seconds; then 2 files / 32 passed, exit 0, 8.57 seconds. The second selection checks the directly affected SQL/memory discovery consumers, including chronology, held work, monitors, natural outcomes, replacements and dedup. Runtime: Node 26.8.2, Vitest 5.0.1, Effect 4.0.4. The prescribed pnpm install prepared ignored dependencies. TMPDIR and retained logs are on disk under the private task cache. Source/index remain clean and HEAD unchanged.

Scoped lint of all seven changed TypeScript files exits 0 with only the pre-existing unused layerUnavailable warning; I confirmed that declaration in the pinned base. Cumulative git diff --check passes. No repo-wide local check ran. Focused feedback is not the complete CI gate.

Reused proof: I inspected the retained final-test baseline record and its actual failure log. The baseline checkout is pinned to 555f0a00198b7aba75d8e8ce13af9c7d983712f4, with only the final regression file changed; its bytes match the candidate test. It fails at the stale-transfer assertion with expected true to be false, before the new reason assertion. I did not rerun or modify that baseline. This is retained failing-before evidence plus my independent candidate execution.

Unverified: I did not query live databases or independently repeat the production diagnosis, launch native servers/providers/browsers, or observe the next installed restart. The SQLite/fake-adapter proof is not native-server restart proof. Deployment, restart, merge and the next live no-stale-wake observation remain outside this reviewer task. Historical #301 native evidence is context, not new exact-head native proof.

At 21:09:19Z, CI 38636834973 is still in progress on this exact head, so complete green CI remains a separate unmet merge gate. Live main and the PR base/head match the pinned identities. No CI failure or later-head review is waived.

Requirements and who asked: The user requires independent Sol r1 review of the demonstrated stale-result mechanism, preserved fresh/legacy recovery behavior, truthful focused proof, findings publication and the two assigned exact-head statuses.
Deleted: Reviewer source repairs, another design round, age-based suppression, blanket notification muting, queue/provider policy changes, native reruns, old-head verdict transfer and merge.
Bottleneck: Independent implementation review is complete; root's separate Grok and complete green CI admission still gate the authorized merge.
Checked myself: Read the complete cumulative diff, owning consumers and full tracker/review history; observed 200 focused passes; inspected transaction/ownership/discovery guards and the matching final-test baseline failure; verified scoped lint, clean source/index, exact base/head and current CI identity.

Reviewer: GPT-6.1-Sol, MAX reasoning, through the Codex harness in T3 Code.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: No blocking finding on eef68e9be143dfeab1c32f088e3be78765f8182f. Legacy cancelled follow-ups whose task already has a result are declined as TASK_SETTLED and left out of result discovery.
Why: The 22:39 wakes were startup transfers of old unbound cancelled follow-ups (roast-164-design-sol-r1 run 2, 232-fix-round3-impl run 9, repair-267-r1 run 6) after those tasks had already returned. Already-terminal rows sit outside restartCutDecision, which only annotates runs this boot cancels.
So what: review/independent-grok success is posted on this exact SHA. Root adjudicates. This review does not merge.

Requirements and who asked: The user asked Grok r1 for an independent exact-head review of #329, a findings comment, and review/independent-grok only.
Deleted: Merge, deploy, Chromeria restart, browsers, memory writes, planner messages, source edits, and generic or Sol statuses.
Bottleneck: Checking that the legacy cancelled-follow-up shape is declined and excluded while fresh cuts, pending continuation intents, first results, and completed follow-ups keep their existing paths.
Checked myself: Read the cumulative diff from 555f0a00198b7aba75d8e8ce13af9c7d983712f4, #326, #264, and PR #301, and the recovery, discovery, and EventSink paths. Ran the seven focused files on this worktree: 168 passed, exit 0.

Cleared

  • Minimality. The new reason, the orchestrator decline before finalizeAppOwnedSubagentTask, the EventSink predicate, and the SQL and memory exclusions each have a job. Recording the reason without the discovery filters would select the thread again. Filtering without the decline would skip the wake and leave no reason. Restart continuation silently skips some cut turns; Codex threads were never continued #264 and Restart continuation revives runs that were cut hours ago and already superseded #326 require the reason.
  • Current versus legacy. The new branch requires restartCut === undefined, status cancelled, a cancelled provider turn on activeAttemptId with equal completedAt, task.result !== null, and no pending continuation. Fresh fix(server): continue restart cuts and expose guarded queue resume #301 cuts are written with restartCut in ProviderRuntimeRecoveryService and stay on the continue-or-explain path.
  • Preserved paths. First results stay on the priorResults.length === 0 path. Completed follow-ups fail the cancelled check. childAwaitsRestartContinuation holds a run with a pending or running provider-runtime.continue effect. Dispatch still refuses a later run as SUPERSEDED. The existing test rediscovering a cut follow-up tells its parent and MCP while keeping the first result passed in this run.
  • EventSink guards. The patch still requires status cancelled, a completed time, and no linked replacement. An existing decline stays in place because continuation is neither scheduled nor null. Natural-outcome events are applied before the patch, so a completed status misses the update. The new predicate matches only TASK_SETTLED, with the cancelled turn's completedAt equal to the run completed_at column and an app-owned task result. Receipt and rollback behavior are unchanged. The standalone decline tests passed.
  • Discovery. delegatedFollowupRecoveryQuery excludes TASK_SETTLED from requested work and from the restart-result branch. untransferredDelegatedRuns and needsRecovery exclude a declined TASK_SETTLED cut. The native no-transfer branch can still return a child with no subagent_result row. The decline is written only from the follow-up loop after a transfer already exists, so that branch does not reselect the historical rows. The regression asserts the child is absent from getRecoveryThreadIds("subagent-results") and that a second recoverDelegatedTasks creates no transfer.
  • Regression seam. Two fresh SQLite runtimes, a real first continuation, recoverDelegatedTask for the first result, a real follow-up start, then a command:runtime-reconcile: commit of the legacy cancelled turn and run with one completedAt and no restartCut. The terminal listener ignores that command prefix. The external harness is unfinishedAdapter. The author's base replay of this final test failed expected true to be false at the stale transfer assertion (~/.cache/ch326/final-test-baseline.log). I did not re-run that red copy.

Proof

  • Executed here: npx -y pnpm@11.10.0 exec vp test run on the seven named files. 7 files, 168 tests, exit 0. Log ~/.cache/ch326/grok-r1/logs/focused.log. HEAD remained eef68e9be143dfeab1c32f088e3be78765f8182f.
  • Inspected, not re-executed: the failing-before log above.
  • Observed: CI run 38636834973 is green on this head (server shards 1-6, Test, Test Web, Lint, Typecheck, Build, Rust, Release Smoke, Check). Mobile native static analysis, EAS preview, and deploy web preview were skipped by their workflows.
  • Unverified: a native provider process and a live restart. The regression fakes the adapter at the harness boundary.

Verdict

Success. No supported actionable finding. Status review/independent-grok applies to eef68e9be143dfeab1c32f088e3be78765f8182f only.

Reviewer: Grok 4.7, XHIGH, r1. No implementation authored in this review.

@lukemaj
lukemaj marked this pull request as ready for review October 14, 2026 21:24
@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Both independent reviewers passed the exact final head eef68e9be143dfeab1c32f088e3be78765f8182f with no blocking findings.
Why: Sol independently passed 200 focused tests, Grok passed 168, and both inspected the guarded legacy decline and preserved recovery paths.
So what: The delegated implementation owner accepts both reviews and will squash-merge after a fresh live gate check; no restart or deployment follows.

Accepted Sol findings and Grok findings. The named successes and generic success are newest on this exact head, created by the authenticated lukemaj account after each corresponding findings comment. CI run 38636834973 is green. Main remains the pinned 555f0a00198b7aba75d8e8ce13af9c7d983712f4; source/index are clean. Ready-for-review automation is checked separately before merge. No finding was waived. Native restart remains unverified and outside authorization.

Requirements and who asked: The user delegated merge after exact-head Sol/Grok success and green CI.
Deleted: No stale-head review reuse, version bump, native restart or deployment.
Bottleneck: Only the fresh final merge gate remains.
Checked myself: Read both complete findings; verified full head/base, status creators and timestamps, focused proof, clean source/index and CI conclusions.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Cavallo runs 9f46c833b192 and passed its complete deployment gate.
Why: #313/#327/#329/#334 are now active there over 038745.
So what: Proceed to rocky with cavallo's signed artifact unchanged; rollback and data backups remain.

Exact SHA 9f46c833b19223e38a1dfb654d2735e54d47c1f7; signed asar SHA-256 de74283746808ce6d7a4a744904855c48690fa331230f96724e00cc438c1a441. 21 samples 23:04:38Z–23:14:39Z, 601.29 seconds, backend 31032 start 01:04:14 CEST unchanged, health/discovery 200 and embedded hash throughout. Strict signature passed. Three production threads load 200; rocky and bestia reachable/full access. Startup migration span Success; numbered ledger current at 62. Correction to the initial checklist: this build has no migration 63. Open-time ContextMD shown columns/index and user_version 1 plus all three renewal tables are verified live through read-only queries. All three read-only VACUUM backups passed integrity_check. Rollback 038745 is .app.prev; older rollback preserved. No rollback used.

No active turns or unfinished work in fresh precheck, no cancelled runs after restart and no stale wakes observed. Watchdog callback delivered once. Desktop independently emitted public commitments for rocky and bestia; approval waits until each server upgrade. Runbook pushed 70b6366b7e3b17d96eb69f9a7eb95ad33a2b9134.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Rocky runs 9f46c833b192 and passed the complete final deployment gate and real cavallo desktop renewal proof.
Why: #313/#327/#329/#334 now operate on rocky over 038745.
So what: Proceed to bestia last; rollback copies and verified backups remain. Runbook 081dd95e16353581328e27974eb9d45862274578 is pushed.

Exact source 9f46c833b19223e38a1dfb654d2735e54d47c1f7; signed asar de74283746808ce6d7a4a744904855c48690fa331230f96724e00cc438c1a441, unchanged from cavallo. Strict signature passed. After the approved desktop reconnect restart, 21 samples 23:23:42Z–23:33:43Z, 604.43 seconds, backend PID37575 start01:23:09CEST unchanged, embedded hash correct and health/discovery200 throughout. Three production thread snapshots load200. Peers cavallo and bestia reachable/full access. Startup runMigrations Success, numbered ledger current62, ContextMD shown schema/index and user_version1 plus renewal tables verified. All three VACUUM backups passed integrity_check. No rollback.

#329 observation: fresh prechecks before both restarts were empty. Three historical cuts are now durably declined with TASK_SETTLED's reason. No subagent-result context transfers after either restart; no stale wakes observed. The two watchdog callbacks were explicitly requested verification turns.

#334: the only pending commitment was the independently matched user's cavallo-chromeria-full desktop on its LAN route. Approved once, recorded above. Same session renewed to generation1, expiry Nov8→Nov13; cavallo persisted the rotated public commitment with the same epoch. Final list contains the one approved row and no pending browser or other device. No secret or re-pairing.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: No stale wakes were observed on cavallo or rocky after the 9f46c83 rollout; bestia was not restarted.
Why: Rocky durably declined three historical task-settled cuts and emitted no subagent-result transfers after either explicitly authorized restart. Cavallo had no cut turns or new result transfers.
So what: This live observation is recorded for #329. Bestia's rollout stopped before activation when Linux executable validation rejected playwright-core, so no bestia restart claim is made.

Both Macs completed ten-minute unchanged-backend proof. Rocky's second restart deliberately reconnected the newly approved cavallo desktop to prove same-session renewal; its read-only watchdog callback was explicitly requested, not a stale recovery wake. Full deployment evidence and partial-state handoff are on #323. Runbook commits cavallo70b6366, rocky081dd95, bestia1f6308a are pushed. No rollback was needed.

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.

Restart continuation revives runs that were cut hours ago and already superseded

1 participant