fix(task-session): reconcile false-cancelled FG-fallback task parts (#595) - #880
fix(task-session): reconcile false-cancelled FG-fallback task parts (#595)#880Jiajun0413 wants to merge 2 commits into
Conversation
…lvinunreal#595) When a foreground task's child session hits a rate-limit and the ForegroundFallbackManager aborts the session to swap models, opencode's runState.cancel poisons the BackgroundJob the task tool is awaiting. The tool returns 'Task cancelled' even though the fallback model completes successfully on an orphan runLoop with no awaiter (alvinunreal#595). The board later records the real outcome (completed/error) via idle reconciliation, but the orchestrator's history still shows the spurious cancellation — board truth and tool result diverge. Fix: in the messages.transform hook, detect task tool parts written as status:'error' with a 'cancelled' message whose metadata.sessionId matches a board job in a non-cancelled terminal state, and rewrite them to reflect the board's authoritative outcome. Board-gated, idempotent, and a no-op for genuine user cancels (board terminalState stays 'cancelled'). This is the single intentional exception to stabilizeRunningTaskParts' 'terminal parts are immutable' rule, because cancelled-by-fallback is not a genuine terminal outcome — it is a transient artifact of the abort+reprompt lifecycle split. BackgroundJob has no public fulfill/reattach API (only internal settle), and ensureRunning discards new work while a runLoop is Running, so no omos-only mechanism can fix the current turn — board completion is the earliest point the LLM can see the truth. Co-developed-with: oracle (GLM-5.2), councillor-reviewer-a (grok-4.5), councillor-reviewer-b (grok-4.5), councillor-reviewer-c (grok-4.5)
Greptile SummaryThis PR reconciles foreground fallback task cancellations with terminal job-board state. The main changes are:
Confidence Score: 4/5The failed-fallback result shape and running-board timing window need fixes before merging.
src/hooks/task-session-manager/board-injection.ts and src/hooks/task-session-manager/index.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant O as Orchestrator
participant T as Task tool
participant F as Fallback run
participant B as Job board
T-->>O: Task cancelled
O->>B: Transform checks job
alt Board is terminal
B-->>O: Completed or error
O->>O: Rewrite task part
else Board is running
B-->>O: Running
O->>O: Keep cancellation
F->>B: Record terminal result later
end
Reviews (1): Last reviewed commit: "fix(task-session): reconcile false-cance..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| partState.status = terminal; | ||
| partState.output = rendered; | ||
| delete partState.error; |
There was a problem hiding this comment.
When the fallback finishes with an error, this branch sets status to error but writes the failure into output and deletes state.error. Opencode's error tool-part shape uses state.error, so callers can receive a generic tool failure instead of the board's recorded failure summary.
| // Reconcile false-cancelled foreground task parts: when a FG-fallback | ||
| // abort poisoned the awaited BackgroundJob ("Task cancelled" error part) | ||
| // but the board later recorded the real model-2 outcome, rewrite the | ||
| // part to the board's terminal truth so the orchestrator's history is |
There was a problem hiding this comment.
When this transform runs after the task reports cancellation but before the fallback records its terminal result, reconciliation sees a running board job and leaves Task cancelled in the current request. The orchestrator can act on that false result, and the later board update only repairs a future transform.
…e rewrite Greptile review: failed board jobs lost the required error text. opencode's message-v2.ts consumes part.state.error as errorText for the UI; deleting it on the error-state rewrite dropped the failure reason. Keep the error field, set to the board's resultSummary, matching the normal error-part shape opencode writes (prompt.ts:417).
Reply to Greptile reviewPoint 1 — "Failed board jobs lose their required error text" → Fixed (8de0e2e) Valid catch. `opencode`'s `message-v2.ts:325-343` consumes `part.state.error` as `errorText` for the UI; deleting it on the error-state rewrite dropped the failure reason. The error field is now preserved, set to the board's `resultSummary`, matching the normal error-part shape opencode writes (`prompt.ts:417`). Point 2 — "A fallback still running leaves the false cancellation visible to the current request" → By design, not a bug This is a timing constraint inherent to the omos-only fix path, not an oversight:
So: while the board is running, the transform correctly leaves the part as-is. Once the board records the real terminal state (completed/error via idle reconciliation, ~2s after model 2 finishes), the next transform rewrites the part to the board's authoritative outcome. The false-cancel is only visible for the window between abort and board settlement — a window where no omos-only mechanism can produce a correct result because the fallback model hasn't finished. This is documented in the code comments and covered by the "does NOT rewrite when board is still running" test case. The only way to close this window is an opencode core change (expose a `model.swap` / `retry.stop` primitive that doesn't settle the BackgroundJob as cancelled), which is out of scope for an omos-only fix. Point 3 — "Settled completed jobs are reconciled as intended" → Confirmed. |
… real output (alvinunreal#863) When a foreground task's primary model halts on a non-retryable error (e.g. 403 quota exhausted), opencode's halt produces an empty assistant message and runTask settles the BackgroundJob as completed with an empty output (result.parts.findLast(text)?.text ?? ""). omos's tryFallback then re-prompts with the fallback model on an orphan runLoop that produces the real result, but the parent task part already says completed with an empty <task_result>. The orchestrator reads the empty output, mis-judges the task as failed/empty, and self-amplifies by launching redundant replacement tasks (alvinunreal#863). Fix: in the messages.transform hook, after stabilizeRunningTaskParts, scan task tool parts with status:completed whose parsed <task_result> is empty. For each, read the child session's real assistant text via the existing extractSessionResult helper and in-place rewrite the part output with renderTaskCompletedWithText (mirrors opencode renderOutput). Idempotent and non-blocking: if the child session has not yet produced non-empty text (fallback model still running), extractSessionResult returns empty and the part is left unchanged — the next transform turn re-evaluates naturally, so no explicit wait/poll is needed. The transform hook only mutates the in-memory output.messages (not the persisted DB), so the rewrite is per-turn but real output eventually lands in history once the child session completes and opencode writes it itself. Gated: only acts on completed task parts whose parsed result is empty. True completions with real text are preserved unchanged. Non-task tool parts are not touched. This is the false-complete counterpart to PR alvinunreal#880's false-cancel fix: - alvinunreal#880 rewrites error+cancelled parts using board record (false-cancel) - this rewrites completed+empty parts using child session text (false-complete) Root cause is in opencode runTask's "?? "" fallback, which omos cannot patch as a plugin. This is the minimal omos-only reconciliation. Co-developed-with: oracle (GLM-5.2)
1 participant
Problem
When launching a foreground task (e.g.
@fixer) whose child agent has a multi-model fallback chain, and the primary model hits a rate-limit, the ForegroundFallbackManager correctly switches to the fallback model which completes successfully — but the parent orchestrator receives "Task cancelled" instead of the result.This is the "orphaned success" symptom of #595.
Root cause (verified across opencode core + omos)
The FG-fallback path uses
abortSessionWithTimeoutto break opencode's same-model infinite-backoff retry loop (retry.ts— no max-attempts), then re-prompts the same session with model 2 viapromptAsync.The abort poisons the lifecycle:
runState.cancel(run-state.ts:77) callscancelBackgroundJobs→ the BackgroundJob the task tool is awaiting is settled as"cancelled"(background-job.ts:148)runner.cancelinterrupts the runLoop →Cause.hasInterruptsOnly→ status"cancelled"background.wait()returns cancelled →task.ts:329throws"Task cancelled"Model 2 then completes on an orphan runLoop (post-abort) with no awaiter. The board later records the real outcome via idle reconciliation, but the orchestrator's tool result already says cancelled — board truth and tool result diverge.
Key findings that shaped the fix:
Agent.Info.modelis a single model — opencode has no native model fallback chain; it's entirely an omos featureensureRunning(runner.ts:120) discards new work while a runLoop is Running → omos must abort to swap models (no omos-only mechanism can avoid it)background-job.tshas no public fulfill/reattach API (only internalsettle) → no omos-only mechanism can fix the current turntool.execute.afterfires on fail butresult=undefined→ opencode writesstatus:"error", error:"Tool execution failed: Task cancelled"(prompt.ts:414-428), withstate.metadata.sessionIdpointing at the child sessionFix
In the
experimental.chat.messages.transformhook, afterstabilizeRunningTaskParts, detect task tool parts written asstatus:'error'whose error message matchescancelledand whosestate.metadata.sessionIdmatches a board job in a non-cancelled terminal state (completed/reconciled/error). Rewrite them to reflect the board's authoritative terminal outcome.Board-gated: only acts when the board holds completed/error (not cancelled) — true user cancels leave the board in
cancelledand are preserved unchanged.Idempotent: after rewrite the part is no longer
errorwith a cancelled message, so subsequent transforms skip it.This is the single intentional exception to
stabilizeRunningTaskParts' "terminal parts are immutable" rule. Cancelled-by-fallback is not a genuine terminal outcome — it is a transient artifact of the abort+reprompt lifecycle split.Why not the alternatives
ensureRunningdiscards new work while Running; no omos-only way to inject model 2 without abortTests
New
fallback-false-cancel.test.ts(7 cases):Full suite: 1732 pass, 0 fail.
Related
councillor). Agents with chains still use FG abort — this PR fixes the board/tool-result divergence on that path.AI assistance
Co-developed-with: