fix(task-session): unify FG-fallback false-cancel + false-complete reconcile (#595, #863) - #913
Conversation
Greptile SummaryThis PR adds transform-time reconciliation for foreground-fallback task parts.
Confidence Score: 2/5The PR should not merge until false-complete recovery waits for final fallback output and selects only the fallback result. The new reconciliation accepts text while the orphan fallback is still generating and extracts every assistant turn, so it can permanently store either a truncated response or a result merged with stale primary-model text. Files Needing Attention: src/hooks/task-session-manager/board-injection.ts, src/hooks/task-session-manager/false-complete-fallback.test.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant P as Parent transform
participant B as Background board
participant F as Orphan fallback run
participant C as Child-session messages
B->>B: Primary halt marks job completed + empty
F->>C: Begin streaming fallback response
P->>B: Observe completed state
P->>C: Extract all current assistant text
C-->>P: Primary text plus partial fallback text
P->>P: Store merged snapshot as completed result
F->>C: Finish remaining response
P->>P: Skip later pass because result is non-empty
Reviews (1): Last reviewed commit: "fix(task-session): unify FG-fallback fal..." | Re-trigger Greptile |
| // promote mid-generation fallback text into a "completed" part. | ||
| // False-complete already settles the board as completed+empty while | ||
| // the orphan runLoop continues — that terminal board state is enough. | ||
| if (job.state === 'running') continue; |
There was a problem hiding this comment.
In-progress fallback becomes terminal
When the orphan fallback has emitted some text but is still generating, the board is already completed, so this guard permits extraction and the first non-empty snapshot is stored as the completed result. The result then bypasses future reconciliation, permanently omitting the remainder of the fallback response from the parent history.
| try { | ||
| extracted = await extractSessionResult(client, childSessionId, { | ||
| directory, | ||
| includeReasoning: false, |
There was a problem hiding this comment.
Extraction merges separate model turns
When the primary model emits partial text before halting and the fallback model later produces the actual result, extractSessionResult concatenates text from every assistant message. This reconciliation therefore installs both the stale primary response and fallback response as one completed task result, giving the parent duplicated or contradictory output.
…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)
…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).
… 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)
…gate) Address Greptile/council P0-P1 on PR alvinunreal#912: - try/catch around extractSessionResult so child read failures never abort the parent messages transform - sanitize embedded </task_result|/task_error> in recovered body so non-greedy parseTaskResultFromOutput cannot truncate - skip rewrite while board job is still running or cancelled, avoiding mid-generation partial promotion into a completed part Tests cover throw fail-open, running-board skip, and embedded close-tag round-trip. Existing suite still green (28 pass).
c7e428d to
7955c64
Compare
Summary
Supersedes #880 and #912 — same FG-fallback orphan-runLoop family, one PR, one transform path.
When foreground fallback swaps models, opencode can leave the parent task tool part in a false terminal state while the fallback model still produces the real result:
"Task cancelled"error partrenderTaskTerminalFromBoard)runTasksettlescompleted+ empty (?? "") whiletryFallbackorphan runLoop continues<task_result>from child session (renderTaskCompletedWithText)Both passes run after
stabilizeRunningTaskPartsinexperimental.chat.messages.transform. SharedsanitizeTaskResultBodyavoids non-greedy parse truncation on recovered text.Hardening (from #912 review / Greptile)
extractSessionResult— never abort parent transformrunningorcancelled</task_result>/</task_error>in recovered bodyNot in scope
task_idresume (still LLM/prompt Session Reuse)runTaskempty settle (omos-only)reconcile_task) is board state only — not redundant with thisTests
fallback-false-cancel.test.ts(from fix(task-session): reconcile false-cancelled FG-fallback task parts (#595) #880)false-complete-fallback.test.ts(from fix(task-session): fill false-completed task parts with child session real output (#863) #912 + hardening cases)task.test.tssanitize/round-tripbun teston the three files: 35 pass.bun run typecheck: pass.Supersedes
AI assistance
Co-developed-with: oracle, councillor-reviewer-a/b/c, council synthesis (review of #912 hardenings).