Skip to content

fix(task-session): fill false-completed task parts with child session real output (#863) - #912

Closed
Jiajun0413 wants to merge 2 commits into
alvinunreal:masterfrom
Jiajun0413:fix/false-complete-fallback-reconcile
Closed

fix(task-session): fill false-completed task parts with child session real output (#863)#912
Jiajun0413 wants to merge 2 commits into
alvinunreal:masterfrom
Jiajun0413:fix/false-complete-fallback-reconcile

Conversation

@Jiajun0413

Copy link
Copy Markdown
Contributor

Problem

When a foreground task's primary model halts on a non-retryable error (e.g. 403 quota exhausted), the orchestrator receives a completed task tool part with an empty <task_result>, even though the fallback model continues on an orphan runLoop and produces the real result minutes later. The orchestrator reads the empty output, mis-judges the task as failed/empty, and self-amplifies by launching redundant replacement tasks (#863).

Verified root cause (DB + opencode 1.18.5 source)

Complete chain, anchored in opencode core:

  1. Model 1 returns 403 isRetryable:false → opencode halt (processor.ts:599) sets ctx.assistantMessage.error, publishes session.error, sets status idle
  2. process returns "stop" (processor.ts:680: if (ctx.assistantMessage.error) return "stop")
  3. prompt loop breaks → ops.prompt returns the last assistant message, which is empty (finish=undefined, parts=[]) because halt produced no content
  4. omos session.error handler triggers tryFallback (non-abort path) → promptAsync re-prompts with model 2
  5. opencode runTask (task.ts:213): result.parts.findLast(text)?.text ?? ""empty string
  6. settle Exit.succeed("") (background-job.ts:148) → status:"completed", output:""
  7. parent task part written as completed with empty <task_result>12s after launch
  8. omos tryFallback's promptAsync takes effect only after model 1's runLoop ends → model 2 runs on an orphan runLoop, producing 9346 chars over 855s — but the parent part already says completed+empty

Root cause is in opencode runTask's ?? "" fallback, which treats a halt-empty assistant message as success. omos cannot patch this as a plugin (no hook to intercept runTask settle). This is the false-complete counterpart to the false-cancel bug in PR #880.

DB evidence (reproduced session)

Time State
B-1 launch +0s
parent part completed + empty output +12s <task_result></task_result>
B-1 child continues +12s → +867s 70 parts, 9346-char audit report
orchestrator mis-judges +645s "Councillor B errored/empty — retry once"
orchestrator relaunches B-2/B-3 +692s/+693s from scratch, missing the 9346 chars

Fix

In the experimental.chat.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 task.ts:64-76).

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 the 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.

Tests

New false-complete-fallback.test.ts (5 cases):

  • Fills empty completed output with child session real assistant text
  • Does not rewrite when completed output already has real text
  • Does not rewrite when child session has no assistant text yet (still running)
  • Idempotent across consecutive transforms
  • Does not touch non-task tool parts

Full suite: 1719 pass, 0 fail. Typecheck: pass.

Relation to other work

PR/Issue Scope Relation
#863 (closed, #870 merged) self-amplify; #870 added reconcile_task tool (operation layer) This PR fixes the root cause (#870 only mitigated the operation layer)
#606 (OPEN, Fixes #595) background task: prevent fallback from aborting background task child sessions (preventive) Complementary, not conflicting#606 is preventive for background tasks; this PR is corrective for foreground tasks. File-level changes don't overlap (#606: foreground-fallback + event/options; this: board-injection + transform/import)
#595 (open) fallback detached from lifecycle Same family; #606 targets the background-task sub-case, this targets the foreground-task false-complete sub-case
#880 (my OPEN PR) false-cancel: error+cancelled parts rewritten from board record Mirror counterpart#880 handles false-cancel, this handles false-complete. Shared render template shape

AI assistance

Co-developed-with:

  • oracle (GLM-5.2) — read full omos + opencode source, confirmed precise root cause chain (runTask ?? "" on halt-empty assistant), recommended this approach over idle-reconcile竞态 alternatives

Note: the analysis council (councillor-reviewer-a/b/c) could not access the omos source tree from their workspace, so their input was based on the root-cause summary rather than direct source verification. The oracle's analysis is the source-verified basis for this fix.

… 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)
@greptile-apps

greptile-apps Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR repairs false-completed foreground task results using output recovered from the child session.

  • Adds a transform-time reconciliation pass for completed task parts with empty results.
  • Adds an XML-like renderer for recovered task output.
  • Adds tests covering recovery, preservation, delayed output, idempotence, and non-task tools.

Confidence Score: 2/5

The PR should not merge until child-session lookup failures are contained and recovered text cannot prematurely terminate the task-result wrapper.

The new transform path allows session read errors to abort valid parent turns, while its renderer permits literal closing tags in assistant output to truncate the recovered task result.

Files Needing Attention: src/hooks/task-session-manager/board-injection.ts, src/hooks/task-session-manager/index.ts, src/utils/task.ts

Important Files Changed

Filename Overview
src/hooks/task-session-manager/board-injection.ts Adds false-completion reconciliation, but an unhandled child-session read failure can abort the parent messages transform.
src/hooks/task-session-manager/index.ts Integrates the asynchronous reconciliation pass into every managed messages transform without an error boundary.
src/utils/task.ts Adds a completed-task renderer whose unescaped assistant text can prematurely terminate the task-result wrapper.
src/hooks/task-session-manager/false-complete-fallback.test.ts Covers normal reconciliation behavior but not child-session request failures or recovered text containing task-result delimiters.

Sequence Diagram

sequenceDiagram
    participant Parent as Parent transform
    participant Board as Background job board
    participant Child as Child session
    Parent->>Board: Resolve empty completed task ID
    Board-->>Parent: Job metadata
    Parent->>Child: Read assistant messages
    alt Non-empty child output
        Child-->>Parent: Recovered result
        Parent->>Parent: Rewrite task output in memory
    else No output yet
        Child-->>Parent: Empty result
        Parent->>Parent: Preserve part for later turn
    else Request fails
        Child--xParent: Error propagates and aborts transform
    end
Loading

Reviews (1): Last reviewed commit: "fix(task-session): fill false-completed ..." | Re-trigger Greptile

Comment on lines +223 to +226
const extracted = await extractSessionResult(client, childSessionId, {
directory,
includeReasoning: false,
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Child lookup aborts transforms

When the child-session messages request fails because of a transient client error, unavailable service, or missing session, the uncaught exception propagates through the new reconciliation pass and aborts the parent message transform instead of preserving the empty result for a later retry.

Comment thread src/utils/task.ts Outdated
`<task id="${taskID}" state="completed">`,
`<summary>${summary}</summary>`,
'<task_result>',
text,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Embedded tags truncate recovered results

When recovered assistant text contains a literal </task_result>—for example in code or an audit report—this unescaped interpolation closes the wrapper early, and parseTaskResultFromOutput extracts only through that first closing tag, causing the remaining task result to be omitted.

…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).
@Jiajun0413

Copy link
Copy Markdown
Contributor Author

Superseded by #913, which merges this false-complete fix (including fail-open / tag sanitize / board-running gate hardenings) with the false-cancel path from #880 into a single transform-path PR.

Please review: #913

@Jiajun0413 Jiajun0413 closed this Jul 26, 2026
mhenke pushed a commit to Jiajun0413/oh-my-opencode-slim that referenced this pull request Aug 23, 2026
…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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Fallback is detached from background task lifecycle, causing lost failures and orphaned successes

1 participant