🤖 fix: prevent workflow task recovery after interrupts#3594
Conversation
Workflow-owned task recovery now checks the owning workflow run status before startup, queued-launch, stream-end, or stream-error recovery can restart the task. Inactive workflow owners interrupt child tasks instead, preserving queued prompts for later inspection or manual workflow resume.\n\n---\n\n_Generated with mux • Model: `openai:gpt-5.5` • Thinking: `xhigh` • Cost: `$17.65`_\n\n<!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=17.65 -->
Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@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: e0ab3551b3
ℹ️ 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".
Startup recovery now interrupts inactive workflow-owned tasks before checking whether parent tasks still have active descendants, then reloads the task index so parent awaiting_report recovery can proceed in the same startup pass.\n\n---\n\n_Generated with mux • Model: `openai:gpt-5.5` • Thinking: `xhigh` • Cost: `$17.65`_\n\n<!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=17.65 -->
|
Addressed Codex thread |
|
@codex review Please take another look after the startup recovery refresh fix. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70a0989963
ℹ️ 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".
When startup recovery interrupts inactive workflow-owned tasks, run the queue scheduler again after recovery prompts so newly freed task slots can start unrelated queued work immediately.\n\n---\n\n_Generated with mux • Model: `openai:gpt-5.5` • Thinking: `xhigh` • Cost: `$17.65`_\n\n<!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=17.65 -->
|
Addressed Codex thread |
|
@codex review Please take another look after the post-interrupt queue drain fix. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5279449f86
ℹ️ 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".
Queued workflow-owned tasks from inactive runs are now interrupted before capacity gating, so abandoned workflow steps do not remain live when all parallel task slots are occupied.\n\n---\n\n_Generated with mux • Model: `openai:gpt-5.5` • Thinking: `xhigh` • Cost: `$17.65`_\n\n<!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=17.65 -->
|
Addressed Codex thread |
|
@codex review Please take another look after the queued inactive-workflow prepass fix. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa98ca60a8
ℹ️ 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".
Inactive workflow-owner interruptions now schedule queued-task draining outside the scheduler path so freed task slots do not leave unrelated queued work stranded. Scheduler callers opt out to avoid recursive queue scans.\n\n---\n\n_Generated with mux • Model: `openai:gpt-5.5` • Thinking: `xhigh` • Cost: `$17.65`_\n\n<!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=17.65 -->
|
Addressed Codex thread |
|
@codex review Please take another look after the inactive-interrupt queue-drain scheduling fix. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f1c0627d7
ℹ️ 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".
Workflow recovery now checks every workflow owner in a task ancestry chain so an active nested workflow cannot mask an interrupted outer workflow owner.\n\n---\n\n_Generated with mux • Model: `openai:gpt-5.5` • Thinking: `xhigh` • Cost: `$17.65`_\n\n<!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=17.65 -->
|
Addressed Codex thread |
|
@codex review Please take another look after the ancestry-wide workflow owner check fix. |
|
Codex Review: Didn't find any major issues. Hooray! 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". |
…itialize Extract the awaiting_report/running candidate filters used by the startup inactive-workflow-owner recovery pass into a single listStartupRecoveryCandidates closure, mirroring the existing listQueuedTasks pattern, so the initial computation and the post-interrupt refresh reuse the exact same status filters. Behavior-preserving cleanup of #3594.
…itialize Extract the awaiting_report/running candidate filters used by the startup inactive-workflow-owner recovery pass into a single listStartupRecoveryCandidates closure, mirroring the existing listQueuedTasks pattern, so the initial computation and the post-interrupt refresh reuse the exact same status filters. Behavior-preserving cleanup of #3594.
…itialize Extract the awaiting_report/running candidate filters used by the startup inactive-workflow-owner recovery pass into a single listStartupRecoveryCandidates closure, mirroring the existing listQueuedTasks pattern, so the initial computation and the post-interrupt refresh reuse the exact same status filters. Behavior-preserving cleanup of #3594.
…itialize Extract the awaiting_report/running candidate filters used by the startup inactive-workflow-owner recovery pass into a single listStartupRecoveryCandidates closure, mirroring the existing listQueuedTasks pattern, so the initial computation and the post-interrupt refresh reuse the exact same status filters. Behavior-preserving cleanup of #3594.
…itialize Extract the awaiting_report/running candidate filters used by the startup inactive-workflow-owner recovery pass into a single listStartupRecoveryCandidates closure, mirroring the existing listQueuedTasks pattern, so the initial computation and the post-interrupt refresh reuse the exact same status filters. Behavior-preserving cleanup of #3594.
…itialize Extract the awaiting_report/running candidate filters used by the startup inactive-workflow-owner recovery pass into a single listStartupRecoveryCandidates closure, mirroring the existing listQueuedTasks pattern, so the initial computation and the post-interrupt refresh reuse the exact same status filters. Behavior-preserving cleanup of #3594.
…itialize Extract the awaiting_report/running candidate filters used by the startup inactive-workflow-owner recovery pass into a single listStartupRecoveryCandidates closure, mirroring the existing listQueuedTasks pattern, so the initial computation and the post-interrupt refresh reuse the exact same status filters. Behavior-preserving cleanup of #3594.
…itialize Extract the awaiting_report/running candidate filters used by the startup inactive-workflow-owner recovery pass into a single listStartupRecoveryCandidates closure, mirroring the existing listQueuedTasks pattern, so the initial computation and the post-interrupt refresh reuse the exact same status filters. Behavior-preserving cleanup of #3594.
## Summary Long-lived auto-cleanup PR maintained by the **Auto-Cleanup Agent**. Each pass lands at most one extremely low-risk, behavior-preserving cleanup drawn from recently merged `main` activity, then advances the checkpoint below. The prior long-lived PR (coder#3559) merged into `main`; this is its successor. Auto-cleanup checkpoint: 2af29df ## This pass Considered the one new `main` commit since the previous checkpoint — coder#3603 (`feat: add built-in loop skill`). That change adds the advertised `loop` built-in skill (`src/node/builtinSkills/loop.md`), regenerates `builtInSkillContent.generated.ts` (generated — off-limits), and extends the agent-skills discovery test expectations. The skill-content/generated surface offers no safe hand-edit, so the cleanup was drawn from the production sibling of that area: the skill-discovery service `agentSkillsService.ts`. Landed one behavior-preserving cleanup in `agentSkillsService.ts`: `readSkillDescriptorFromDir` open-coded the same invalid-skill diagnostic push — `options?.invalidSkills?.push({ directoryName, scope, displayPath: skillFilePath, message, hint })` — at six failure sites (missing/unreadable SKILL.md, SKILL.md-is-a-directory, oversized file, read failure, invalid descriptor, parse failure). Those six sites share identical identity fields (`directoryName` / `scope` / `displayPath`) and differ only in `message` + `hint`. All six now delegate to a single function-local `pushInvalidSkill(message, hint)` closure that captures the function-constant identity fields. Behavior-preserving: the pushed object shape and every per-site `message`/`hint` are unchanged, and the closure short-circuits on `options?.invalidSkills` exactly as the inline calls did. ## Branch contents (net diff against `main`) <details> <summary>Cleanups carried by this PR (not yet merged)</summary> - **`agentSkillsService.ts` — dedupe invalid-skill diagnostics.** coder#3603's new built-in skill lives in the agent-skills discovery area; `readSkillDescriptorFromDir` open-coded the same `options?.invalidSkills?.push({ directoryName, scope, displayPath: skillFilePath, message, hint })` at six failure sites that share identical identity fields and differ only in `message`/`hint`. All six now delegate to a single function-local `pushInvalidSkill(message, hint)` closure. Behavior-preserving (identical pushed shape, messages, hints, and `options?.invalidSkills` short-circuit). - **`workspaceGoalService.ts` — dedupe session file path resolution.** The three private accessors `getFilePath` / `getHistoryFilePath` / `getBoardFilePath` each open-coded the same `assert(...non-empty workspaceId)` guard and `path.join(getSessionDir(...), <FILE_CONST>)`. All three now delegate to a single `resolveSessionFilePath(workspaceId, fileName)` helper. Behavior-preserving (identical guard + join; accessor names and callers unchanged). - **`task.ts` — dedupe plan-agent explore-only error.** The plan-agent restriction error `In the plan agent you may only spawn agentId: "explore" tasks.` was open-coded twice (the workspace-turn guard and the per-launch agent-id guard). Both now reference a single module-level `PLAN_AGENT_EXPLORE_ONLY_ERROR` constant. Behavior-preserving (identical string output). - **`task_await.ts` — dedupe workspace-turn error fallback.** coder#3600's new workspace-turn await paths open-coded the fallback error message `"Workspace turn failed"` three times (`<record>.error ?? "Workspace turn failed"`). All three now reference a single module-level `WORKSPACE_TURN_DEFAULT_ERROR` constant. Behavior-preserving (identical string output). - **`goalToolUtils.tsx` / `SetGoalToolCall.tsx` — dedupe turn-count pluralization.** coder#3595's new goal toolcards open-coded `` `${n} turn${n === 1 ? "" : "s"}` `` in both `formatGoalTurns` and the set_goal toolcard's `formatOptionalTurnCap`. Both now call a shared `pluralizeTurns(count)` helper. Behavior-preserving (identical string output). - **`heartbeat.ts` — dedupe heartbeat tool success result construction.** coder#3596's new `heartbeat` tool open-coded the same `{ success: true, action, configured, settings, summary }` success object three times (`get` / `set` / `unset`), each re-calling `summarize({ action, settings })`. All three now route through a single `buildSuccessResult(action, settings)` helper that derives `configured` from `settings`. Behavior-preserving: the per-branch `configured` values (`settings != null`, `false`, `true`) all equal `settings != null` given the non-null `Ok` payload type for `set`. - **`taskService.ts` — dedupe startup recovery candidate listing.** coder#3594's `TaskService.initialize` open-coded the `awaiting_report` / `running` candidate filters twice (initial computation and post-interrupt refresh). Both call sites now reuse a single `listStartupRecoveryCandidates(sourceConfig)` closure returning `{ awaitingReportTasks, runningTasks }`, matching the existing `listQueuedTasks` pattern. Behavior-preserving. - **`ChatPane.tsx` — dedupe send-queued-immediately in-flight guard clear.** coder#3592 introduced `sendQueuedImmediatelyInFlightRef` to block duplicate send-now attempts; releasing that guard ("clear it only if it still points at this attempt") was open-coded twice — once after a failed interrupt result and once in the `catch` arm. Both call sites now invoke a single callback-local closure `clearInFlightGuardIfCurrent()`. Behavior-preserving: the failed-result site's combined `!interruptResult.success && ref === id` becomes `if (!interruptResult.success) clearInFlightGuardIfCurrent()` (logically identical); the `catch` site is a verbatim substitution. - **`WorkflowRunner.ts` — dedupe `parallelAgents` child-failure abort handling** into a single batch-local closure `applyChildFailureToBatch(error)`, shared by the per-run task closure and the bulk-create path. - **`ChatInput/index.tsx` — dedupe edit-mode attachment toast** string `"Attachments cannot be added while editing a message."` into a single module-level constant `EDIT_MODE_ATTACHMENT_ERROR_MESSAGE`, replacing three inline copies across the add-attachment / paste / drop guards. </details> <details> <summary>Prior passes (landed via coder#3559, merged)</summary> - Replaced an inline terminal-status check in `WorkflowService.interruptRunOnAbort` with the `isTerminalWorkflowRunStatus` helper. - Deduplicated the `workspaceId`-only metadata JSON Schema repeated by three id-targeted host actions in `workspaceHostActions.ts` into `WORKSPACE_ID_ONLY_INPUT_SCHEMA`. - Extracted a shared `BlockedBadge` component in `WorkflowDefinitionToolCall.tsx`. - Deduplicated the repeated `argsSchema.properties must be an object` error string in `workflowArgs.ts`. - Deduplicated mocked-module specifier strings in `useModelsFromSettings.test.ts`. - Removed a redundant `toggleWorkflowExpanded` wrapper in `WorkflowRunToolCall.tsx`. - Deduplicated `SAVE_AUTOMATION_ERROR_MESSAGE` / `REMOVE_AUTOMATION_ERROR_MESSAGE` in `AutomationModal.tsx`. - Replaced a duplicated `"wfr_"` literal in `WorkflowRunner.ts` with `isWorkflowRunTaskId()`. </details> ## Validation - `make static-check` — ESLint, both `tsgo` typecheck configs, and Prettier all pass on the rebased tree. The only failing step is `fmt-shell-check`, which requires `shfmt` (not installed in this environment); no shell scripts were touched, so CI (which has `shfmt`) is unaffected. - `bun test src/node/services/agentSkills/agentSkillsService.test.ts` — 20 pass, 0 fail (covers skill discovery + the invalid-skill diagnostics path this pass deduped); `builtInLoopSkill.test.ts` — 1 pass. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-8` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-opus-4-8 thinking=xhigh --> --------- Co-authored-by: mux-bot[bot] <264182336+mux-bot[bot]@users.noreply.github.com>
1 participant
Summary
Prevents task-level startup, queue, stream-end, and stream-error recovery from resurrecting workflow-owned child tasks after their owning workflow run is inactive.
Background
Workflow interrupt/abort is the source of truth for workflow execution. Before this change, TaskService recovery paths could still prompt or restart child agent tasks after the owning workflow was interrupted, which could burn tokens and violate workflow lifecycle boundaries.
Implementation
Validation
wfr_ed8469895aaa74a1; it applied cleanup commite0ab3551band reported targeted validation passed.MUX_ESLINT_CONCURRENCY=1 make static-checkRisks
Moderate TaskService lifecycle risk: this touches recovery paths for queued/running/awaiting-report tasks. The guard is limited to tasks that are workflow-owned directly or through ancestry, and active workflow runs still allow normal recovery.
Generated with
mux• Model:openai:gpt-5.5• Thinking:xhigh• Cost:$17.65