-
Notifications
You must be signed in to change notification settings - Fork 511
fix(task-session): unify FG-fallback false-cancel + false-complete reconcile (#595, #863) #913
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
5519440
ef4d8af
ab27b93
cec668e
7955c64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| * ../cache-safe-injection.ts to ensure prompt cache safety. | ||
| */ | ||
| import { createHash } from 'node:crypto'; | ||
| import type { PluginInput } from '@opencode-ai/plugin'; | ||
| import type { | ||
| BackgroundJobExecution, | ||
| BackgroundJobInjectedCompletionFence, | ||
|
|
@@ -25,9 +26,12 @@ import { | |
| parseTaskStateFromOutput, | ||
| parseTaskStatusOutput, | ||
| renderRunningTaskPlaceholder, | ||
| renderTaskCompletedWithText, | ||
| renderTaskTerminalFromBoard, | ||
| } from '../../utils'; | ||
| import { isRecord } from '../../utils/guards'; | ||
| import { log } from '../../utils/logger'; | ||
| import { extractSessionResult } from '../../utils/session'; | ||
| import { | ||
| appendTaggedSyntheticPart, | ||
| appendTrailingVolatileMessage, | ||
|
|
@@ -546,6 +550,183 @@ export function stabilizeRunningTaskParts(messages: unknown[]): void { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Reconcile false-cancelled foreground task tool parts. | ||
| * | ||
| * 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 `Effect.fail("Task cancelled")` → the assistant message's | ||
| * tool part is written as `status:"error"` with `"Task cancelled"` in the | ||
| * error field. The fallback model then completes on an orphan runLoop with | ||
| * no awaiter; the board later records the real outcome (completed/error). | ||
| * | ||
| * This rewrites such false-cancelled error parts to reflect the board's | ||
| * authoritative terminal state, so the orchestrator's history shows the | ||
| * true result instead of a spurious cancellation (#595). | ||
| * | ||
| * Gated: only acts when the board holds a non-cancelled terminal state | ||
| * (completed/reconciled/error) for the child session. True user cancels | ||
| * leave the board in `cancelled`, so they are preserved unchanged. Idempotent: | ||
| * after rewrite the part is no longer an error-with-cancelled, 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. | ||
| */ | ||
| export function reconcileFallbackFalseCancel( | ||
| state: InjectionState, | ||
| messages: unknown[], | ||
| ): void { | ||
| for (const message of messages) { | ||
| if (!isMessageWithParts(message)) continue; | ||
| for (const part of message.parts) { | ||
| if (part.type !== 'tool' || part.tool !== 'task') continue; | ||
| const partState = part.state; | ||
| if (!isRecord(partState)) continue; | ||
| if (partState.status !== 'error') continue; | ||
| const errorMsg = partState.error; | ||
| if (typeof errorMsg !== 'string' || !/cancelled/i.test(errorMsg)) { | ||
| continue; | ||
| } | ||
| const metadata = partState.metadata; | ||
| if (!isRecord(metadata)) continue; | ||
| const childSessionId = metadata.sessionId; | ||
| if (typeof childSessionId !== 'string') continue; | ||
|
|
||
| const job = state.backgroundJobBoard.get(childSessionId); | ||
| if (!job) continue; | ||
| // Only rewrite when the board has a non-cancelled terminal truth. | ||
| // `reconciled` retains a `terminalState` of completed/error. | ||
| const boardState = job.state; | ||
| const terminal = | ||
| job.terminalState ?? | ||
| (boardState === 'completed' || boardState === 'error' | ||
| ? boardState | ||
| : undefined); | ||
| if (terminal !== 'completed' && terminal !== 'error') continue; | ||
|
|
||
| const rendered = renderTaskTerminalFromBoard({ | ||
| taskID: childSessionId, | ||
| state: terminal, | ||
| description: job.description, | ||
| resultSummary: job.resultSummary, | ||
| }); | ||
| partState.status = terminal; | ||
| partState.output = rendered; | ||
| if (terminal === 'error') { | ||
| // Preserve the `error` field for opencode's ToolPart error state | ||
| // (message-v2.ts consumes it as errorText for the UI). Use the | ||
| // board's resultSummary as the authoritative failure reason. | ||
| partState.error = job.resultSummary ?? 'Background task failed'; | ||
| } else { | ||
| delete partState.error; | ||
| } | ||
| log('[task-session-manager] reconciled false-cancelled task part', { | ||
| taskID: childSessionId, | ||
| alias: job.alias, | ||
| parentSessionID: job.parentSessionID, | ||
| boardState, | ||
| terminalState: job.terminalState, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Reconcile false-completed foreground task tool parts. | ||
| * | ||
| * 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 (#863). | ||
| * | ||
| * This in-place rewrites such false-completed parts with the child session's | ||
| * real assistant text once the fallback model has produced it. The pass is | ||
| * idempotent and non-blocking: if the child session has not yet produced | ||
| * non-empty text, `extractSessionResult` returns empty and the part is left | ||
| * unchanged — the next transform turn re-evaluates naturally, so no explicit | ||
| * wait/poll is needed. Because the transform hook only mutates the | ||
| * in-memory `output.messages` (not the persisted DB), the rewrite is | ||
| * per-turn but the real output eventually lands in history once the child | ||
| * session completes and opencode writes it itself. | ||
| * | ||
| * Gated: only acts on `status:"completed"` task parts whose parsed | ||
| * `<task_result>` is empty. True completions with real text are preserved | ||
| * unchanged. | ||
| */ | ||
| export async function reconcileFalseCompleteFallback( | ||
| state: InjectionState, | ||
| messages: unknown[], | ||
| client: PluginInput['client'], | ||
| directory?: string, | ||
| ): Promise<void> { | ||
| for (const message of messages) { | ||
| if (!isMessageWithParts(message)) continue; | ||
| for (const part of message.parts) { | ||
| if (part.type !== 'tool' || part.tool !== 'task') continue; | ||
| const partState = part.state; | ||
| if (!isRecord(partState)) continue; | ||
| if (partState.status !== 'completed') continue; | ||
| if (typeof partState.output !== 'string') continue; | ||
|
|
||
| const status = parseTaskStatusOutput(partState.output); | ||
| if (status?.state !== 'completed') continue; | ||
| // Only reconcile empty results — real completions are intact. | ||
| if (status.result && status.result.trim().length > 0) continue; | ||
|
|
||
| const childSessionId = status.taskID; | ||
| const job = state.backgroundJobBoard.get(childSessionId); | ||
| if (!job) continue; | ||
| // Wait until the board is no longer actively running so we do not | ||
| // 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; | ||
| if (job.state === 'cancelled' || job.terminalState === 'cancelled') { | ||
| continue; | ||
| } | ||
|
|
||
| // Fail-open: a transient child-session read must never abort the | ||
| // parent messages transform (Greptile P1). Empty/error → leave part | ||
| // unchanged; the next transform turn re-evaluates naturally. | ||
| let extracted: { text: string; empty: boolean }; | ||
| try { | ||
| extracted = await extractSessionResult(client, childSessionId, { | ||
| directory, | ||
| includeReasoning: false, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the primary model emits partial text before halting and the fallback model later produces the actual result, |
||
| }); | ||
| } catch (error) { | ||
| log('[task-session-manager] false-complete extract failed', { | ||
| taskID: childSessionId, | ||
| alias: job.alias, | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| continue; | ||
| } | ||
| if (extracted.empty) continue; | ||
|
|
||
| const summary = `Background task completed: ${job.description}`; | ||
| partState.output = renderTaskCompletedWithText( | ||
| childSessionId, | ||
| summary, | ||
| extracted.text, | ||
| ); | ||
| log('[task-session-manager] reconciled false-completed task part', { | ||
| taskID: childSessionId, | ||
| alias: job.alias, | ||
| parentSessionID: job.parentSessionID, | ||
| textLength: extracted.text.length, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export function updateFromInjectedCompletion( | ||
| state: InjectionState, | ||
| part: MessagePart, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.