Sign up
View stargazers13Sign up

fix(opencode): retain queued native child continuation output#19388

closed1
main
GitHub fork: fix/opencode-native-child-continuations
opened10d agobyEExotic209093
5 files changed
View on GitHubTwo-way sync enabledPR last imported 3d ago

After an OpenCode root turn completes, ingestion can stop between a child subagent’s completion and its queued native continuation. That drops the continuation’s messages and terminal update.

OpenCode now emits ordered, deduplicated snapshots of pending continuation work. The execution service uses consumed snapshots to keep owned child streams open until that work drains, while retaining the ownership-loss rule for newer root work. Adapters without this metadata keep their existing thread-scoped fallback.

Fixes #19228. The issue’s maintainer triage identifies the missing adapter signal as the required fix.

Validation on Windows x64, Node 24.19.0:

  • vp test run --root apps/server src/orchestration-v2/RunExecutionService.test.ts src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts: 151 tests passed. Service regressions cover both retained and lost root ownership and verify that the child continuation’s running event, message, and completion all arrive. Adapter coverage checks queued-report snapshots.
  • Server, provider-core, and provider-opencode package typechecks passed. Scoped lint, formatting, and git diff --check passed.
  • Independent review checked event ordering, snapshot deduplication, stop propagation, and ownership release. A real OpenCode session was not exercised.

Implemented with GPT-6.1-Sol through the Codex harness in T3 Code.

Checks

All checks passed

Added 1 commit

  • fix(opencode): retain queued native child continuation output2f38870
MmacroscopeappBot10d ago
Synced from GitHub

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This fix changes production event-stream lifetime and ownership handling across the OpenCode adapter, shared event contract, and orchestration service to retain queued child continuations. The behavior is covered by focused tests but involves a nontrivial cross-layer processing gate, so human review is appropriate.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot10d ago
Synced from GitHub

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: 94d625ee-5469-4dc0-aefc-5395178f843d
📥 Commits

Reviewing files that changed from the base of the PR and between f5b5f9dd7be29c4f55f4687dda15e4105cf966fc and 2f388703d0bd7d5342c21d51f68fe5431d3f6d40.

📒 Files selected for processing: 5
  • apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts
  • apps/server/src/orchestration-v2/RunExecutionService.test.ts
  • apps/server/src/orchestration-v2/RunExecutionService.ts
  • packages/provider-core/src/server/ProviderAdapter.ts
  • packages/provider-opencode/src/server/v2/adapter.ts

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

📝 Walkthrough

Provider-thread updates now report pending background work. Root-run orchestration tracks these updates and continues ingesting events for child threads with pending work after the root turn ends.

Changes: Child work ingestion

Layer / File(s)Summary
Publish pending-work state
packages/provider-core/src/server/ProviderAdapter.ts, packages/provider-opencode/src/server/v2/adapter.ts		The provider-thread event adds an optional pendingBackgroundWork value. The OpenCode adapter includes this value in updates and synchronizes roster changes across ancestor threads.
Continue ingestion for pending child work
apps/server/src/orchestration-v2/RunExecutionService.ts		Root-run ingestion records pending-work snapshots by provider thread. Pending work on a child thread keeps ingestion open after root completion.
Verify continuation behavior
apps/server/src/orchestration-v2/RunExecutionService.test.ts, apps/server/src/orchestration-v2/Adapters/OpenCode2AdapterV2.test.ts		Tests verify ordered child events after root completion with retained or lost root ownership, and check that provider-thread updates include a true pending-work value.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

mermaid
1sequenceDiagram
2 participant OpenCodeAdapter
3 participant ProviderEventStream
4 participant RunExecutionService
5 OpenCodeAdapter->>ProviderEventStream: Emit provider-thread pending-work update
6 ProviderEventStream->>RunExecutionService: Deliver ordered provider-thread update
7 RunExecutionService->>RunExecutionService: Track pending-work snapshot
8 ProviderEventStream->>RunExecutionService: Deliver queued child turn and message events

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 2f388

No actionable merge-blocking issue was established. The change is ready for normal merge checks.

Security architecture risk: 🔵 Low · up to 2f388

The existing event-routing and write-ownership checks remain in place. However, cleanup now depends on child snapshots that direct stop paths do not always refresh, potentially keeping completed work subscribed longer than intended.

Retained concerns

  • Medium · reliability · inferred: Clearing retained child snapshots is not guaranteed by direct nested Stop settlement. The consumer retains each non-root pendingBackgroundWork=true snapshot until a same-ID update arrives and checks these snapshots before root ownership loss. stopBackground can settle a nested call, while settleCall publishes the settled child's state and emitSubagent synchronizes only the root, leaving an intermediate ancestor's earlier true snapshot unchanged. If no later execution/inbox update repairs that ancestor, a completed-root subscription can remain active despite drained work and lost root ownership. Normal stream handling and reconnect reconciliation repair this state, so persistent retention is conditional rather than a demonstrated live-session failure. This matters to ownership release and resource containment, not to newly granted execution authority.
Security review details

Security Blast Radius

  • inferred — The newly introduced exposure is subscription lifetime within an existing provider session and its accepted root/related child threads. The inspected changes do not add credentials, tool-execution capabilities, or cross-service privileges. Repeated retained subscriptions could increase work within the live server process, but process-wide exhaustion and tenant exposure were not established.

Trust Boundaries and Controls

  • observed — The added boolean is computed by the adapter from registered native-thread state, not copied directly from an incoming boolean. The consumer accepts provider-thread updates only for an already-owned provider-thread ID or an owned app-thread ID. These controls limit the new lifetime decision to previously accepted identities; they do not solve stale snapshot retirement.

Resilience and Maintainability Implications

  • observed — Normal execution/inbox events synchronize the affected thread and its ancestors. Reconnect reconciliation clears descendant reports and wakes before publishing snapshots in reverse order. Non-completed root terminals stop ingestion immediately, and subscription cleanup runs when the ingestion fiber ends. These are concrete recovery controls, although direct Stop after an already-completed root need not create another root terminal.
Pre-merge checks: Passed 4
Passed Passed checks: 4 passed
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main fix: preserving queued native child continuation output in OpenCode.
Description checkPassedThe description explains the problem, the implementation, linked issue and maintainer triage, focused verification results, environment, and an untested real-session limitation. It covers the template…
Linked Issues checkPassedThe PR addresses #19228. RunExecutionService records consumed pendingBackgroundWork snapshots and keeps owned child ingestion open while pending continuation work remains. It preserves the owner-l…
Out of Scope Changes checkPassedThe changes stay within #19228. The provider event field, OpenCode roster synchronization, execution-service logic, and focused tests all support continuation-event retention. No unrelated product beh…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

JJulius Marminge5d ago

Taking over this one. Thanks @Exotic209093 — I will rebuild this on current main in two layers so the OpenCode pending-work snapshot and the shared ingestion lifetime change can be reviewed separately, with ordered continuation tests and a real-provider check. I will link the replacement stack here.

JJulius Marminge5d ago

Thanks @Exotic209093 — your PR correctly identified the need to retain event-position pending-work state instead of trusting a live adapter probe after the root finishes.

I rebuilt this as two focused layers on current main:

  • #21460: ordered provider pending-work snapshots and ancestor refresh.
  • #21461: keep ingestion open for queued native child continuation output, including after root ownership changes.

I compared the complete original diff against the stack. The optional metadata, native execution/inbox tracking, wake replay, reconnect cleanup and ingestion lifetime are all covered. The stale intermediate ancestor after nested Stop is fixed in the provider layer. Explicit thread emission replaces generic event interception, and sequential ingestion uses one local map. Both commits credit your contribution.

The focused suites pass 156/156 (154 baseline); four regression assertions fail without the fix. Independent Opus and Sol full-diff reviews are clean. The bodies link current-head evidence and explicitly retain the real-provider verification and human approval gates: the native probe created a child but did not reproduce its nested continuation. CI and configured bot follow-up continue on the replacement PRs. No merge was performed.

Closing this source PR in favor of those credited replacement layers.