🤖 fix: deliver terminal wakes for kernel-launched background workflow runs - #3990
Conversation
|
@codex review |
This comment has been minimized.
This comment has been minimized.
![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: 9665489578
ℹ️ 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 review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a19c5a304c
ℹ️ 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 review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30a3437d0b
ℹ️ 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 review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89263dd6b0
ℹ️ 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 review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59ce349492
ℹ️ 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 review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76e6e8070d
ℹ️ 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".
…defer drains on indeterminate currentness
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 046286a0d1
ℹ️ 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 review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 355ac54d41
ℹ️ 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 review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32ad57027b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 32ad57027b
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
The scope preflight runs before the admission guard is acquired, so a turn completing during that await can grow history: the locked recomputation then removes only a prefix while the caller still applies every full-clear-only side effect (context epoch advance, goal/plan/retry discards). Guard the third drift direction symmetrically: requireFullDelete makes historyService refuse, under the history write lock, any all-classified truncation whose recomputed budget would leave messages; a retry re-classifies.
|
@codex review |
Conflicts: kept the branch's try/catch hardening around readAgentWorkflowRunReferences while adopting main's path.join(config.sessionsDir, id) argument style, and kept the branch's existsSync test import over the now-duplicate path import. Main's split removed Config.getSessionDir, so the branch-added call sites (94 across six files) were swept to the same sessionsDir pattern.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 0d1c089fcf
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d1c089fcf
ℹ️ 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 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". |
This comment has been minimized.
This comment has been minimized.
…archive windows Round 18 review findings, all in the terminal-wake delivery path: - A shared revalidateWorkflowPromptForDispatch helper reruns invocation currentness, the run generation (status + updatedAt vs the derivation snapshot), and the generation settlement marker as the last awaits before both the group send and the busy fallback: a Workflows UI resume/retry or a kernel-nested task_await consumption arrives without history evidence, and currentness alone would deliver a stale or already-consumed result. - truncateHistory acquires the admission guard BEFORE the scope preflight (a turn admitted mid-classification could launch a workflow whose sidecar reference the wholesale retirement then deletes) and rechecks turn activity after the retirement await for both row-removing scopes. - Unarchive workflow-attention reconciliation moved after snapshot restoration and lifecycle startup so its synthetic turn cannot run against a half-restored checkout or a rolled-back unarchive.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70b22c3631
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 70b22c3631
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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". |
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: a15a79d0fd
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| const groupRevalidation = await Promise.all( | ||
| groupCandidates.map((candidate) => | ||
| this.revalidateWorkflowPromptForDispatch(ownerWorkspaceId, candidate) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
With Dynamic Workflows enabled, this is reachable when at least two terminal runs share a launch-identity group and a clear/reset/resume lands while their final checks resolve. Promise.all preserves an early candidate's deliverable verdict even if a later candidate observes the mutation as superseded; the drain then sends the early run's retained, attacker-influenced result into the fresh context. The restored launch policy limits tools but does not prevent allowed actions. Validate and dispatch the whole group under one mutation/generation barrier, or abort the group when any candidate spans a state change.
Useful? React with 👍 / 👎.
1 participant
Summary
Background workflow runs launched from the code_execution kernel (
xum.workflow_run(...)) never delivered their terminal wake to the owning workspace: the run finished, the notify_on_terminal attention fired, and the wake was silently dropped as "superseded". The owner stayed idle until a human prodded it. This PR makes kernel-launched runs recognizable through their durableagent-workflow-runs.jsonsidecar provenance, and reworks workflow terminal wake delivery as level-triggered reconciliation: the workflow run record itself is the durable statement that a wake is owed, a sweep re-derives owed wakes from it, and settlement events merely accelerate delivery.Background
Incident: a 16-repo audit workflow launched from the kernel failed ~11 minutes in. The terminal attention was enqueued, but
buildWorkflowTerminalPromptgates delivery on invocation currentness, which only recognized directworkflow_run/workflow_resumetool parts and slash-command card messages. A kernel launch leaves neither in history, so the backward walk hit the prior real user message and classified the run as superseded; nothing ever retried.The first implementation delivered the wake as an edge-triggered one-shot: a persisted outbox record per run, with recovery hardening for every window where the edge could be lost (retained enqueue retries, reset bookkeeping, deferred retry timers, crash-resume boundary repair). Each review round found a new hole in that recovery layer and each fix added more machinery. Per maintainer decision, the delivery mechanism was replaced wholesale (89e0428) with level-triggered reconciliation; events only poke the reconciler and are never the correctness mechanism.
Implementation
attentionPolicy: "notify_on_terminal"in a terminal status owes its owner a wake unless a settlement marker exists for that terminal generation.sweepWorkflowRunTerminalAttention(startup + 5-minute interval) re-derives owed wakes into an in-memory queue (pendingWorkflowRunAttention); terminal callbacks callnoteWorkflowRunTerminalAttention, which just queues and pokes the drain without touching disk. A lost poke, crash, or removed workspace needs no compensation: the next sweep reconstructs the queue.TerminalAttentionStore.recordSettledwrites a write-once delivered/superseded marker keyed by the run's terminalupdatedAt. A later resume changesupdatedAt, so its new terminal state is a new generation that wakes normally; no reset bookkeeping exists. Delivery markers are best-effort: the synthetic wake message itself is durable history evidence, so a marker lost to a crash settles later as consumption. Legacy pending workflow outbox records are deleted by the drain; sub-agent and workspace-turn notifications keep the persisted outbox.isWorkflowInvocationCurrentkeeps its backward walk (direct tool parts and card messages still win), and the sidecar fallback decides by row identity instead of wall-clock order: launch records snapshot the newest invocation-decision row, and the run is current exactly when that row is still the newest decision row. Indeterminate (history/sidecar unreadable) defers the wake in the queue; the next drain trigger or sweep retries. Boundaryless references (pre-upgrade or downgrade-stripped) settle as superseded rather than deferring forever; the result stays retrievable viaworkflow_resume/task_await, which record fresh provenance.agentIdand strict pin, so a restricted launch's (attacker-influenced) output is never processed under another launch's tool grants. The synthetic send restores the conversation's persisted caller tool policy anddisableWorkspaceAgentsflag from the newest restriction-defining user row, stops at context reset boundaries, and defers when history is unreadable.task_awaitand foregroundworkflow_resumesettle the run's terminal generation on consumption (a kernel-nested resume leaves no top-level tool part for the history walk to find). Background resumes never settle the prior generation.Review history note (rounds 11-17)
Rounds 11-14 accreted a detached retry/repair layer around the sidecar and outbox (record retries with lifecycle generations, boundary repair timers, corrupt-node self-heal, removal drains, admission staleness probes, persisted per-row delivery provenance). 95ca943 removed most of it; the remaining recovery machinery (retained enqueues, reset bookkeeping, defer retry timers, crash-resume boundary repair) kept producing findings in the same class, and 89e0428 replaced the delivery mechanism itself with the level-triggered design above, deleting that machinery. Accepted residual windows, documented in resolved threads: one possible redelivery of a genuine result after a crash between send acceptance and the settled-marker write (reconverges via history-consumption evidence), a downgrade-stripped boundary settling the wake as superseded (result retrievable via explicit resume), and unbounded-but-tiny sidecar growth until a full clear.
Validation
make static-checkandmake typecheckpass; targeted suites (taskService, workspaceService, agentWorkflowRunReferences, WorkflowService, workflow_resume, workflow_run, task_await, terminalAttentionStore, router, agentSession.autoCompaction) run clean apart from 3 taskService and 1 workspaceService failures re-confirmed as pre-existing on this host at the unmodified base commit (tmp chown/chmod noise and bash-monitor-wake reconciliation).Risks
Low-moderate. The currentness gate only widens delivery for runs with sidecar references, which only background
workflow_run/workflow_resumetool executions write; direct-tool semantics are unchanged. The reconciliation model trades exact-delivery hardening for at-least-once evaluation with idempotent settlement: worst cases are one redundant wake (crash between accepted send and marker write) or a silently settled wake for downgrade-stripped provenance, both fail-safe and both recoverable through explicit resume. The 5-minute sweep bounds wake latency when a settlement poke is lost.Generated with
xum• Model:anthropic:claude-fable-5• Thinking:xhigh• Cost:$768.07