fix(server): changing a task's wake policy no longer reverts a checkpointed parent run#25662

open7

main

fix/issue-24322

View on GitHubTwo-way sync enabledPR last imported 2d ago

Problem

Switching a delegated task's wake policy to always (for example after a task_status wait times out) plans a completion delivery from a snapshot of the parent run. If the parent's checkpoint capture commits completed between that read and the command's commit, the stale snapshot is written back: the run returns to running, loses completedAt and checkpointId, and queued work behind it stops advancing.

Change

delegated_task.wake-policy now hands its parent run id to the same updateDelegatedCompletionOnlyRunId path that acknowledge and dispose already use on main (32bf48f428, pingdotgg/t3code#25672). EventSink.commitCommand rebuilds that run.updated from the stored run inside the transaction and applies only the planned delegatedCompletion. Every parent run the delivery planner returns differs from its input only in delegatedCompletion, so nothing else is dropped.

Refs pingdotgg/t3code#25757: the wake-policy path of the same stale-write race. #25667 fixes the accept path that stalled that thread.

Found while triaging pingdotgg/t3code#24322; it does not fix that issue's terminal negotiation timeout.

Takeover

Taken over from @juliusmarminge's automated triage run. While this PR waited, main landed the acknowledge/dispose half of the same fix in pingdotgg/t3code#25672, with a near-identical EventSink change. The merge with main resolves to main's EventSink, so this PR now adds only the wake-policy half, which the original covered without a test. The PR's acknowledge regression test passed on main unchanged and was dropped as a duplicate. Main's parametrised race test gains a wake-completed case.

Evidence

Server-only change; no client, contract or render path changes. The proof audit's capture matrix is empty, so the evidence is the regression test and a runtime transcript.

Case	main `Orchestrator.ts` (394929af76)	head (5abcaf34fe, merged forward to ceacd385f7)
`ack-failed`: acknowledge, parent fails mid-command	pass	pass
`ack-completed`: acknowledge, checkpoint completes mid-command	pass	pass
`wake-completed`: wake policy → `always`, checkpoint completes mid-command	fail: run back to `running`, checkpoint lost	pass: `completed`, `completedAt` and `checkpointId` kept, planned `delegatedCompletion` applied

Transcripts: head.txt, control.txt (head tests with main's Orchestrator.ts). Proof audit: fixture matrix, runtime bundle.

Environment: fs-26 Linux VM, vp test run in apps/server, real orchestration and EventSink persistence, provider mocked. What else changed: nothing outside Orchestrator.ts (+5/−1) and DelegatedCompletionDelivery.test.ts.

Verification log

Tests: DelegatedCompletionDelivery.test.ts 31 → 31 (PR's duplicate acknowledge test removed; wake-completed case added). 31 pass at head; with main's Orchestrator.ts only wake-completed fails.
vp run check:changed on ceacd385f7: format, lint, knip 1/2 and 2/2, graphql, typecheck (3 packages), tests (3 files) pass.
Review round 1 on 5abcaf34fe: reviewer A (GPT-6.1 Sol, Codex) MERGE; reviewer B (Claude Opus 5.5, Claude Code) MERGE. No confirmed findings. ceacd385f7 only adds a merge of main commits that do not touch apps/server.
Proof audit pass 1 (GPT-6.1 Sol, Codex): SUFFICIENT, empty capture set.
Not reproduced: the terminal negotiation timeout from the issue, and live queue advancement behind a long-running provider.
Unverified surfaces: none.
Model and harness: Claude Opus 5.5 through Claude Code (lead, tests, implementation, review B); GPT-6.1 Sol through Codex (review A, proof audit).

Original description by @juliusmarminge

Problem

A child-result acknowledgement can read a waiting parent, then commit that stale snapshot after checkpoint capture has completed the run. This puts the completed parent back into waiting and prevents queued work from advancing. The maintainer observed a checkpointed parent stuck for over nine hours with more than 110 queued runs.

Change

Rebuild delegated-completion-only run updates from the stored lifecycle inside the command transaction. Acknowledgement, disposal and wake-policy updates retain their delegated-completion changes while preserving a checkpoint completion committed after their planning read.

Scope and approval

Originally linked to https://github.com/pingdotgg/t3code/issues/24322

GitCafe: https://git.cafe/pingdotgg/t3code/issues/24322

Triaged bug via the automated issue flow; maintainer requested the retained checkpoint-race fix. This fixes the reproduced stale-acknowledgement race. The reported terminal negotiation timeout itself remains unverified; this change does not establish that all provider backlog failures share this cause.

Verification

Retained red/green proof: the regression test fails before the implementation and passes with it; both isolated reviewers returned MERGE and proof audit returned SUFFICIENT in the prior run.
Rechecked head e5dcae1e4e21a790d36dc5a4f54613b3df088d40, merged with main e2d3ded436f7: vp test run apps/server/src/orchestration-v2/DelegatedCompletionDelivery.test.ts — 29 passed.
vp run check:changed — format, lint, both knip checks, GraphQL, affected typechecks and 42 tests in three files passed.
Focused runtime sequence in the regression test: seed waiting parent with delivered child result → plan acknowledgement → commit checkpoint completion → commit acknowledgement → read projection. Result: status remains completed, completedAt and checkpointId survive, delivery is cleared.
Server-only change; capture matrix is empty. No client render paths or contracts changed. Terminal negotiation timeout and live long-running provider backlog were not reproduced in this resumed run.

Model: GPT-6.1 Sol through the Codex harness (resumption and verification); retained implementation from the earlier automated triage run.

🤖 Generated with Claude Code

Checks

22 successful · 16 skipped

All checks passed

Added 3 commits2d ago

Merge remote-tracking branch 'origin/main' into fix/issue-24322e5dcae1
fix(server): acknowledging a child result no longer reverts a checkpointed run to waitingec6c96f
test(server): acknowledging a child result can revert a checkpointed run to waitingf45888d

MmacroscopeappBotapproved these changes · review dismissed2d ago

MmacroscopeappBot2d ago

Approvability

Verdict: Approved at ceacd38

Macroscope's review found this PR approvable — This is a focused server race-condition fix that prevents wake-policy updates from overwriting a parent run’s newly committed completion and checkpoint state. The added coverage exercises the affected checkpoint race, while the test-only changes have no production impact.

^{You can add or adjust custom eligibility rules. Learn more.}

Ggithub-actionsBot2d ago

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	0 B (0.0%)	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	0 B (0.0%)	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	0 B (0.0%)	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	0 B (0.0%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.2 KiB	21.2 KiB	0 B (0.0%)	29.3 KiB	✅
Claude	Live turn messages	2	2	0 (0.0%)	8	✅

Baseline: 401b017 · PR result: ceacd38 · 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.

CcoderabbitaiBot2d ago

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info

External image Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: fix/issue-24322
File: .coderabbit.config.ts
Commit: ceacd38
Layers: Repository TS config
Review profile: CHILL
Plan: Team
Run ID: 01dd1305-771b-4205-9e3a-0f22c853acd8

External image Commits

Reviewing files that changed from the base of the PR and between e5dcae1e4e21a790d36dc5a4f54613b3df088d40 and ceacd385f7cce75e8267cc5cb67aad8fb99e1222.

External image Files selected for processing: 2

apps/server/src/orchestration-v2/DelegatedCompletionDelivery.test.ts
apps/server/src/orchestration-v2/Orchestrator.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Walkthrough

The wake-policy dispatcher now returns the affected parent run ID, and dispatchOnce records it for downstream delegated-completion handling. The race test covers acknowledgement and wake-policy updates when the run settles between update read and commit, and checks that settled run fields and planned delegated-completion state persist.

Changes: Delegated completion updates

Layer / File(s)	Summary
Identify the parent run for commit scope `apps/server/src/orchestration-v2/Orchestrator.ts`	The wake-policy handler returns the parent run ID, and `dispatchOnce` stores it in `updateDelegatedCompletionOnlyRunId`.
Test updates during run settlement `apps/server/src/orchestration-v2/DelegatedCompletionDelivery.test.ts`	The race test covers acknowledgement and wake-policy updates while a run settles. It checks that the settled run fields and planned delegated-completion state are retained.

Priority: External image Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

pingdotgg/t3code#25672 — The PR addresses the same delegated-acknowledgement race with overlapping EventSink and Orchestrator changes.

Suggested reviewers: t3dotgg

Merge Risk: External image Minimal · up to ceacd

The change preserves settled run state during the tested acknowledgement and wake-policy updates. No identified issue blocks merging after normal checks.

Pre-merge checks: ^Passed 2 · ^Failed 2

^Failed Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Linked Issues check	Warning	Issue [`#24322`] requires the code-mode host handshake to avoid dependence on an undrained provider app-server stdio stream, or to fail fast with an error that identifies the cause. The PR changes deleg…	Implement the [`#24322`] handshake requirement and add an automated test for the provider-backlog case. Alternatively, link this PR to an issue that covers the delegated-completion checkpoint race.
Out of Scope Changes check	Warning	The implementation and test address delegated completion acknowledgement, disposal, wake-policy updates, and checkpoint lifecycle preservation. Issue [`#24322`] addresses terminal code-mode host negotia…	Move the delegated-completion checkpoint-race changes to an issue that covers that race, or replace them with changes and tests for the [`#24322`] terminal negotiation and provider-backlog behavior.

^Passed Passed checks: 2 passed

Check name	Status	Explanation
Title check	Passed	The title names the main change: a fix so that changing a task's wake policy no longer reverts a checkpointed parent run. The raw summary and PR objectives confirm the wake-policy path now preserves c…
Description check	Passed	The pull request description is complete and directly addresses the required Problem, Change, Scope and approval, and Verification information. It explains the race, identifies the implementation, lin…

Full details: Title check

Explanation

The title names the main change: a fix so that changing a task's wake policy no longer reverts a checkpointed parent run. The raw summary and PR objectives confirm the wake-policy path now preserves checkpoint completion through the delegated-completion-only update. The conventional-commit format matches the repository template example. The title is 86 characters, which is slightly long but still readable.

Full details: Description check

Explanation

The pull request description is complete and directly addresses the required Problem, Change, Scope and approval, and Verification information. It explains the race, identifies the implementation, links the related issue and approval context, documents focused tests and broader checks, states unverified behavior, and identifies the model and harness. Scope and approval details appear under Takeover and the embedded Original description rather than a dedicated top-level heading, but the required information is present.

Full details: Linked Issues check

Explanation

Issue [#24322] requires the code-mode host handshake to avoid dependence on an undrained provider app-server stdio stream, or to fail fast with an error that identifies the cause. The PR changes delegated-task completion handling and adds a checkpoint-race test. It does not change terminal negotiation, provider stdio handling, timeout behavior, or a related test. The PR states that the terminal timeout remains unverified.

Resolution

Implement the [#24322] handshake requirement and add an automated test for the provider-backlog case. Alternatively, link this PR to an issue that covers the delegated-completion checkpoint race.

Full details: Out of Scope Changes check

Explanation

The implementation and test address delegated completion acknowledgement, disposal, wake-policy updates, and checkpoint lifecycle preservation. Issue [#24322] addresses terminal code-mode host negotiation during provider app-server backlog. The PR provides no code or test evidence that the delegated-completion change affects that negotiation behavior.

Resolution

Move the delegated-completion checkpoint-race changes to an issue that covers that race, or replace them with changes and tests for the [#24322] terminal negotiation and provider-backlog behavior.

Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

JJulius Marminge2d ago

The linked-issue and scope warnings were checked against the maintainer’s updated triage instructions. Those instructions explicitly identify the checkpointed parent stuck in waiting with 110+ queued runs, authorize a focused test of that event order when the live repro is unavailable, and request resuming this retained fix. This PR covers that reproduced checkpoint/acknowledgement race. It does not establish the cause of every terminal negotiation timeout; the body explicitly records that limitation. No handshake or provider-stdio change is justified by the reproduced race, so expanding this patch would exceed the requested fix. All 28 head checks pass; Macroscope approved and CodeRabbit reported no actionable code findings.

JJulius Marminge2d ago

Taking over this one

Added 4 commits2d ago

Merge remote-tracking branch 'origin/main' into fix/issue-243226e3ee22
test(server): takeover tests: wake-policy can revert a checkpointed run to running2df519e
fix(server): changing a task's wake policy no longer reverts a checkpointed parent run5abcaf3
Merge remote-tracking branch 'origin/main' into fix/issue-24322ceacd38

MmacroscopeappBotapproved these changes2d ago