🤖 fix: report live workspace-turn state from task_await instead of stale settlements#3738
🤖 fix: report live workspace-turn state from task_await instead of stale settlements#3738ThomasK33 wants to merge 9 commits into
Conversation
…e settlements A workspace turn that settled terminally (provider stream error whose auto-retry was not detected, or 'interrupted after restart') could self-heal: the child auto-retries the same turn and completes it, but settleWorkspaceTurn refused to touch already-terminal records, so a parent orchestrator's task_await kept reporting the stale interrupted/error status forever. Two complementary fixes: - Event-time: the strictly turn-correlated stream-end path may now resettle a stale interrupted/error record (never completed) with the turn's real outcome, re-arming the notify_on_terminal wake-up so the parent learns the corrected result. Duplicate replays stay idempotent (no change when status+messageId already match). - Read-time: normalizeWorkspaceTurnRecord reconciles settled interrupted/error records against the child's live state. Snapshot reads (task_await) also repair from durable history when a correlated final assistant message proves the turn finished; and when the child is actively retrying the same turn (streaming or pending auto-retry, with the turn's prompt still the newest user message) the handle is revived to 'running' and re-registered so normal settlement resumes. List paths skip the per-record history read for perf and only use the cheap runtime-activity gate.
|
@codex review |
1 similar comment
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e44ca4ff8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ale workspace turn Codex P2: TerminalAttentionStore.enqueueIfAbsent leaves delivered tombstones untouched (keyed workspace_turn:<handleId>), so a corrected terminal outcome after a resettle would never wake the owner. Delete the existing notification record before enqueueing the corrected outcome, and also clear it when reviving a retrying turn so the eventual real settlement can notify.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb50f704ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2: an unrelated user prompt newer than the turn's correlated final assistant message must only disable the live-retry revive, not abort the durable-history repair scan — otherwise a turn that self-healed and completed before the child received manual input would keep reporting the stale failure.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de8ad3ba0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ce turn Codex P2: between the reconcile read and the settlement lock, the live retry itself can fail and settle a NEWER record with the same status (error -> error). Guarding on status alone would revive that fresh terminal failure back to running, stranding task_await. Compare updatedAt as record identity in both revive and repair paths.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74691ae6c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2 x2: explicit interrupts (user Esc, task_terminate, cancel reasons) persist status interrupted without the stale-restart marker; they must stay terminal even if a late correlated stream-end lands or same-turn retry evidence appears, so canceled work never resurfaces as completed/running. Gate both the stream-end resettle and the read-time reconcile on isSelfHealEligibleSettledWorkspaceTurn: status error (transient stream failures) or interrupted with the restart marker.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d434df2484
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…urns Codex round-5 finding assumed listWorkspaceTurnTasks applies the status filter at the store level before normalization; it actually reads all records, normalizes (reconcile/revive) each, then filters on the normalized status. Add a regression test demonstrating a stale error handle with a live auto-retry is revived and included in the active-only listing.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50bd171542
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…locker scan Codex P1: settled records never accumulate deferredMessageIds, so the history repair could report completed while the child still had active descendant/workflow/nested-turn work. Gate the repair on hasActiveWorkspaceTurnDeferredBlockers (lazily computed) to match the deferred stream-end semantics of active handles. Codex P2: listActiveWorkspaceTurnTaskIdsForOwner (parent turn-end blocker scan) previously asked the store for active statuses only, so a stale error handle with a live same-turn retry was invisible and the parent could end its turn. Reconcile self-heal-eligible settled records there too (runtime checks short-circuit before any history read).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 756859870f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2 (round 7): between retry streams the child can be momentarily quiet — stream ended, synthetic background-await queued/preparing, or descendant/workflow/nested work still running — and the previous cheap gate (isStreaming || hasPendingAutoRetry) returned the stale terminal record, letting the parent turn-end blocker scan finish early. Extend the liveness gate (cheap-first, blockers cached) and, when a correlated final exists but blockers are active, revive the handle with the final recorded as deferred so standard deferred recovery settles the true outcome once blockers finish.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a91068e129
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2 x2 (round 8): drop hasPendingQueuedOrPreparingTurn from the gate — queued manual input is not yet in history, so it defeated the newest-correlated-prompt guard and could revive unrelated work. The synthetic background-await continuation window it covered is a subset of blockers-active, now represented by the cheap in-memory hasActiveDescendantAgentTasks hint instead of the full hasActiveWorkspaceTurnDeferredBlockers (which reads child history and would run for every historical failed handle on parent stream-ends). Full blocker checks remain lazily evaluated in the deferred/repair branches only.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
task_await(and workspace-turn snapshots generally) could permanently report a staleinterrupted/errorstatus for a delegated workspace turn that had actually self-healed — the child auto-retried the same turn and completed it, but the parent orchestrator never learned. This PR makes workspace-turn state reconcile against the child's live state sotask_awaitreports reality at call time.Background
Workspace-turn handles (
wst_…) persist a status inTaskHandleStore. Two paths could settle a handle terminally while the child later recovered:finalizeWorkspaceTurnFromStreamErrorsettleserrorwhen the auto-retry is not detected in its race window (or the error type is not classified retryable but the child retries anyway).settleStaleWorkspaceTurnmarks the handleinterrupted after restartbefore the child session's startup auto-retry registers.settleWorkspaceTurntreated all terminal records as immutable, so even when the retried turn later emitted a strictly correlated stream-end (sameworkspaceId+turnIdvia muxMetadata — auto-retry replays the turn's synthetic prompt, so retried streams stay correlated), the record stayederror/interruptedforever. An orchestrator parent then saw the task as blocked/failed indefinitely.Implementation
Two complementary layers:
allowTerminalResettletosettleWorkspaceTurn, which may now overwrite a staleinterrupted/errorrecord with the turn's proven outcome.completedstays immutable, duplicate replays are idempotent (no change when status + messageId already match), and a resettle re-arms thenotify_on_terminalwake-up so the parent is notified of the corrected result.normalizeWorkspaceTurnRecord(shared bytask_awaitsnapshots andtask_list) reconciles settledinterrupted/errorrecords:task_awaitcall time (covers records that went stale before this fix).runningunder the settlement lock and re-registers in the active-handle map so normal settlement resumes.Both persistence helpers run under the per-handle settlement lock and re-validate the stale status before writing, so concurrent settlements win.
Validation
error→completed; duplicate replay stays unchanged (no re-notify);completednever overwritten; snapshot repairs from self-healed history while the list path intentionally does not; revive on pending auto-retry incl. active-map re-registration; a newer unrelated child prompt does not revive.taskService.test.tssuite (299 tests) plus adjacenttask_await/task_list/task_terminate/taskHandleStore/tasksuites (96 tests) pass;make static-checkgreen.Risks
completedremains immutable, and all new writes go through the existing settlement lock with stale-status revalidation.task_terminate) that a human later manually retries in the child workspace will truthfully reportrunning/completedinstead of stayinginterrupted. This matches the goal of live state at call time.task_awaitreads, not list paths.Generated with
mux• Model:anthropic:claude-fable-5• Thinking:xhigh• Cost:$21.27