Skip to content

fix(server): background subagents sharing an OpenCode 2 child session all finish - #16811

Closed
Abdullah1738 wants to merge 1 commit into
pingdotgg:mainfrom
Abdullah1738:fix/opencode2-shared-child-reports
Closed

Abdullah1738 wants to merge 1 commit into
pingdotgg:mainfrom
Abdullah1738:fix/opencode2-shared-child-reports

Conversation

@Abdullah1738

@Abdullah1738 Abdullah1738 commented Oct 7, 2026 •

Copy link
Copy Markdown

Problem

When an agent sends more than one subagent call to the same child session (resuming a subagent while it is still working, or mixing a foreground and a background call on it), the OpenCode 2 adapter can leave a background call running forever. The thread's pendingBackgroundTasks never empties, so the thread keeps showing as working long after everything finished.

Two ways this happens:

  • A report is matched to the first call on its child, so a report for one background call can settle a different call on the same child, and the call it was actually for never settles.
  • OpenCode steers a second prompt into the child's running execution and sends one report for the whole run. The call whose prompt was steered in never gets its own report.

Seen on real threads: a background "Finish shared UI audit" call stayed running after its report settled an earlier foreground call on the same child, and a "Validate backfill edge cases" call whose prompt was steered into a running execution never got a report.

Change

  • Reports go to the background call they name (the description in the report's <subagent> tag), falling back to the oldest background call on that child.
  • Each call remembers the child inbox item carrying its prompt and which child execution took it in. When a report arrives, or a foreground call on that child completes or fails, background calls answered by that same execution settle with it.
  • Prompt matching is exact (with or without the subagent preamble). Two calls sending the same prompt are left alone rather than guessed.
  • A reconnect starts a fresh execution number, so a prompt delivered after a lost stream never joins a run from before it.

If the adapter misses the events that tie a call to an execution, that call keeps the current behaviour and waits for its own report.

Scope and approval

This is a focused fix in OpenCode2AdapterV2.ts: background subagents stuck as pending work when calls share a child session. It doesn't change any contract or the UI, so I didn't open an issue first. Happy to open one if you'd prefer.

Verification

  • Added adapter tests for a report that names a later call on a shared child, one run answering two calls, a call whose prompt the child hasn't taken yet, exact prompt matching, runs kept apart across a lost stream, and a foreground failure settling a background call from the same run. The pending-prompt test guards against settling too much; the rest fail without the change.
  • vp test run src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts: 90/90 pass.
  • Server typecheck and vp lint on the changed files are clean.
  • I didn't run a live OpenCode 2 session against this branch. The scenarios come from two stuck threads in my own OpenCode 2 setup, replayed in the tests.

Built with Claude Opus 5.5 in T3 Code (reviewed with GPT 6 Astra).

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 7, 2026
@Abdullah1738
Abdullah1738 marked this pull request as ready for review October 7, 2026 12:10
@coderabbitai

coderabbitai Bot commented Oct 7, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d46cf789-32c9-4865-95f6-4e9827c51615
📥 Commits

Reviewing files that changed from the base of the PR and between 611132c and 2cfb8db.

📒 Files selected for processing (2)
  • apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts
  • apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The adapter now tracks which child execution consumes each subagent prompt. It uses that association and report descriptions to attribute reports and settle matching background calls. Replay tests cover shared child sessions, queued prompts, reconnects, and failures.

Changes

Shared subagent calls

Layer / File(s) Summary
Associate prompts with child executions
apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.ts
Subagent calls track their inbox item and child execution. Thread state tracks execution counts. Prompt matching, inbox delivery, execution-start events, and reconnects update these associations.
Attribute reports and settle shared calls
apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.ts
Report descriptions can come from metadata, payload fields, or an XML attribute. The adapter matches reports to background calls and settles calls sharing the same child execution on report, success, failure, or abort.
Validate shared-call replay behavior
apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts
Replay tests cover report matching, multiple prompts answered by one execution, queued prompts not answered, reconnects, and failed child runs.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant OpenCode as OpenCode event stream
  participant Adapter as OpenCode2AdapterV2
  participant Calls as SubagentCall state
  OpenCode->>Adapter: Enqueue a prompt for a child session
  Adapter->>Calls: Associate a uniquely matching prompt with its inbox item
  OpenCode->>Adapter: Deliver the inbox item
  Adapter->>Calls: Record the child execution count
  OpenCode->>Adapter: Report the child execution
  Adapter->>Calls: Match the background call and settle calls for that execution
Loading

Suggested reviewers: juliusmarminge

Merge Risk: 🔵 Low · up to 2cfb8

If a child run continues through a stream reconnect, one background call may remain running after the work finishes. This bounded edge case can be accepted with owner awareness or addressed before merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 2cfb8

The change is confined to how related background tasks finish and does not change existing access controls. Some recovery and repeated-message behavior still depends on delivery guarantees that were not established in this review.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The new fanout searches only the originating parent's calls and requires matching child object and execution identities. Existing failure cleanup can additionally settle descendant calls. This bounds the demonstrated effect to related call lifecycle state; broader tenant or environment isolation was not established by the available evidence.

Security Findings and Attack Paths

  • inferred — If a synthetic report is delivered repeatedly, candidate selection can move to another remaining call because neither handler rejects an already-processed inbox ID. This attribution weakness predates the PR. New shared propagation can extend the selected outcome within an execution group, but attackable producer replay or a material security worsening was not established.

Trust Boundaries and Controls

  • observed — Reports enter through synthetic inbox events, require subagent source metadata and a child ID, and select only tracked background calls for that child. Description metadata or XML text influences attribution but is not validated as an authenticated call identifier. Permission and form events are routed separately.

Resilience and Maintainability Implications

  • observed — Existing Stop handling retains calls whose child could not be interrupted, allowing another Stop to retry. Pending-work checks also account for busy child sessions and held reports, not just call entries. Added replay assertions cover shared completion and foreground failure propagation; they were inspected, not executed.

Hardening Proposals

  • proposed — Establish the producer's ordering and replay guarantees. If duplicates are possible, consider idempotent report processing and authoritative call/execution identifiers rather than relying on descriptions and oldest-call fallback.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the fix: background subagent calls that share an OpenCode 2 child session now finish.
Description check ✅ Passed The description covers the problem, change, scope and approval rationale, and focused verification results. It also states that live OpenCode 2 testing was not performed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Approvability ✅ Passed The PR changes only apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.ts and its test file. The implementation change addresses pending background subagent calls in the existing orchestra…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@juliusmarminge

Copy link
Copy Markdown
Member

Note

Grok responding on behalf of Julius.

Thanks for digging into this, @Abdullah1738. The main case here, where OpenCode steers a second subagent call into a child's running execution and sends a single report for both, is now fixed on main by #17134: every call that joined the running run settles with that report, and pendingBackgroundTasks empties. This branch also edits apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.ts, which has since moved to packages/provider-opencode/src/server/v2/adapter.ts, so it can't land as is. I'm closing it as superseded.

If you can still reproduce the other case, where a report naming a later background call on a shared child settles the wrong call, a small follow-up PR against the new adapter location with just that routing and its test would be very welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants