Skip to content

fix(orchestrator): Close and drain OpenCode 1.x tool items - #4786

Closed
mwolson wants to merge 6 commits into
pingdotgg:t3code/codex-turn-mappingfrom
mwolson:fix/opencode-session-error-cleanup-drain
Closed

fix(orchestrator): Close and drain OpenCode 1.x tool items#4786
mwolson wants to merge 6 commits into
pingdotgg:t3code/codex-turn-mappingfrom
mwolson:fix/opencode-session-error-cleanup-drain

Conversation

@mwolson

@mwolson mwolson commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Close in-flight OpenCode 1.x tool items when the user interrupts a turn, instead of leaving a permanent spinner.
  • Drain session.error cleanup so a tool that finishes during OpenCode's 250ms cleanup window can land its real result before finalize.
  • This PR now owns both OpenCode 1.x finalization concerns. Review them here rather than on fix(orchestrator): Close OpenCode tool items when a turn ends under them #4759 opencode-interrupt-tool-items.

This branch is a deliberate multi-commit consolidation onto current t3code/codex-turn-mapping. Each commit is one concern. It does not include OpenCode 2.

Problem and Fix

Problem and Why it Happened Fix
Interrupting an OpenCode turn left the mid-flight tool as running because finalizeTurn flushed text and reasoning but not tool parts. Close still-open tool items when the turn ends under them, and replay an interrupt that lands mid-tool.
session.error finalized immediately, so later cleanup part updates were dropped and the tool lost its real exit/output. Drain native cleanup before finalize and keep native cleanup timestamps.

Validation

  • vp check: 0 errors, 22 inherited warnings
  • Focused OpenCode adapter plus orchestrator replay fixtures: 91 passed
  • Replayed onto current t3code/codex-turn-mapping (feat(contracts): track thread title regeneration)

Absorbed PRs

Superseded


Note

Medium Risk
Changes core OpenCode turn finalization and error/idle event ordering in orchestration-v2; wrong drain timing could leave turns open or drop late cleanup results, but behavior is heavily fixture-tested.

Overview
Fixes OpenCode 1.x turns that leave tool rows stuck on running after interrupt, abort, or session.error, and cases where cleanup emits late tool parts after the adapter would have already finalized.

Turn finalization sweep: finalizeTurn now re-emits any tool still pending/running (and interrupted cleanup tool errors) using a new terminalToolStatus mapping so item status matches the turn (completed, interrupted, cancelled, failed). emitToolPart / subagent projection accept an optional forced terminal status and only synthesize completedAt when the native part never terminalized.

Error cleanup drain: On scoped session.error with in-flight tools, finalization is deferred via pendingErrorCleanup until OpenCode’s cleanup can land—tracking assistant message IDs, tolerating pre-cleanup idle pairs, and settling on completed assistant messages or follow-up idle when publishers omit the usual signals. Unscoped errors or interrupts without cleanup still finalize immediately; subagent statuses now follow turn-item literals (e.g. interrupted stays interrupted).

Tests: Unit coverage for terminalToolStatus plus orchestrator replay fixtures for drain, fallback, multi-message, unscoped error, and mid-tool interrupt (including OpenCode on turn_interrupt_mid_tool).

Reviewed by Cursor Bugbot for commit fdb35c8. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix OpenCode 1.x tool items to close and drain on session error

  • Adds deferred turn finalization in OpenCodeAdapterV2: when a session.error occurs with in-flight tools, finalization is held until cleanup tool completions arrive rather than closing the turn immediately.
  • Introduces pendingErrorCleanup state on ActiveOpenCodeTurn to track which tool message IDs are still outstanding and whether a pre-cleanup idle has been seen.
  • Extends emitToolPart and emitSubagent to accept a terminal turn status, closing any pending/running tool items with a matching terminal status and synthesized completedAt when no native completion time exists.
  • finalizeTurn now emits non-terminal tool parts (including interrupted-error parts) before closing the turn, preventing indefinitely running items in the projection.
  • Adds eight new replay fixture scenarios (drain, fallback, no-pre-idle, idle-only, unscoped, multiple-messages, interrupt cleanup, aborted-tool interrupt) with corresponding assertion helpers to cover the new cleanup paths.
  • Risk: session idle events now gate on cleanup completion, so a missing cleanup tool completion could delay turn finalization indefinitely.

Macroscope summarized fdb35c8.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 20b560c2-8c8b-4eb6-90e9-a6ea4415290c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added size:XL 500-999 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Jul 28, 2026
@mwolson
mwolson marked this pull request as ready for review July 28, 2026 21:51
@macroscopeapp

macroscopeapp Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at fdb35c8

Macroscope's review found this PR approvable — This is a targeted bug fix that ensures OpenCode tool items are properly closed when turns end mid-flight, preventing UI spinners from being stuck forever. The ~150 lines of logic changes are well-tested with 8+ replay fixtures covering various edge cases (error cleanup, interrupts, unscoped errors), the author has recent commits on this exact code area, and all previous review comments have been addressed in the head commit.

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

@mwolson
mwolson force-pushed the fix/opencode-session-error-cleanup-drain branch 3 times, most recently from 0069b28 to 8c7575d Compare July 30, 2026 02:07
Comment thread apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.ts Outdated
@mwolson
mwolson force-pushed the fix/opencode-session-error-cleanup-drain branch from a3b3a5d to c49d36e Compare July 30, 2026 16:16
@maria-rcks
maria-rcks force-pushed the t3code/codex-turn-mapping branch from a543fd4 to 378615b Compare August 3, 2026 16:01
@mwolson
mwolson force-pushed the fix/opencode-session-error-cleanup-drain branch from c49d36e to ddaffdf Compare August 3, 2026 20:17
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch 3 times, most recently from e24b59c to 4213ac5 Compare August 5, 2026 14:35
@mwolson
mwolson force-pushed the fix/opencode-session-error-cleanup-drain branch from ddaffdf to 88e61c5 Compare August 5, 2026 18:28
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch from 25de21d to 0af2a6e Compare August 7, 2026 12:10
@mwolson
mwolson force-pushed the fix/opencode-session-error-cleanup-drain branch from 88e61c5 to 50bc97c Compare August 8, 2026 18:42
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch from 22bd872 to a27c1cc Compare August 10, 2026 17:05
@mwolson
mwolson force-pushed the fix/opencode-session-error-cleanup-drain branch from 50bc97c to 641e99b Compare August 11, 2026 13:26
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch 2 times, most recently from a186d64 to 5b1a115 Compare August 12, 2026 23:19
@mwolson
mwolson force-pushed the fix/opencode-session-error-cleanup-drain branch from 641e99b to 0af7ffd Compare August 13, 2026 21:26
@mwolson mwolson changed the title fix(orchestrator): Drain OpenCode cleanup after session errors fix(orchestrator): Close and drain OpenCode 1.x tool items Aug 13, 2026
Comment thread apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.ts
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch 4 times, most recently from 519c42a to 4c55679 Compare August 17, 2026 10:28
@mwolson
mwolson force-pushed the fix/opencode-session-error-cleanup-drain branch from 0af7ffd to db55c22 Compare August 19, 2026 15:37
CI Check fails on current CTM because layerTest now surfaces
ServerSettingsError while the ProviderSessionManager test layer requires
never. orDie matches the other test layers.
`finalizeTurn` flushed only `text` and `reasoning` parts, so a tool still
mid-flight when the turn ended kept whatever state it was last observed in.
OpenCode's `session.abort` stops the run without reporting a final state for the
running tool, so nothing ever closed the item: after a Stop, the interrupted
run's `command_execution` stayed `running` permanently and the row spun forever
in the UI.

Sweeps non-terminal tool parts during finalize and emits them with the turn's
terminal status, threaded through `emitToolPart` and `emitSubagent` as an
optional override so the existing projection logic is reused rather than
duplicated. `OrchestrationV2TurnItemStatus` and `OrchestrationV2Subagent`
declare the same status literals, so an interrupted subagent stays interrupted
rather than collapsing to failed.

Found by the new `opencode-interrupt-direct-stop` live pack, which reproduced it
as `command_execution:running` surviving on an `interrupted` run. The same pack
now reports `command_execution:interrupted` with zero running commands.
The existing tests covered only the pure status mapping, so they would still
pass with the sweep deleted, applied to already-terminal tools, or failing to
forward the override. Codex review of pingdotgg#4759 called that out.

Adds an OpenCode provider to the `turn_interrupt_mid_tool` replay fixture, which
drives the real adapter and asserts the projection. The transcript reproduces
the ordering from a live run rather than an idealised one: the tool goes
running, the abort goes out, then `session.error` and idle arrive, and only
afterwards does OpenCode's cleanup emit its own tool part update. That late
update lands on an already-finalized turn and is ignored, so the item's
`interrupted` status has to come from the sweep.

Verified the fixture catches the regression: with the sweep commented out it
fails with `expected 'running' to equal 'interrupted'`, and nothing else in the
suite breaks.
@mwolson
mwolson force-pushed the fix/opencode-session-error-cleanup-drain branch from db55c22 to 5842bb0 Compare August 19, 2026 18:05
Comment thread apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.ts

@cursor cursor Bot left a comment

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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5842bb0. Configure here.

Comment thread apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.ts
@t3dotgg t3dotgg added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. and removed vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Aug 24, 2026
@mwolson

mwolson commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Closing this as superseded. OpenCode 1.x is retired on the v2 line in favor of native OpenCode 2. The drain-before-finalize work here is specific to 1.x session.error / message.part.updated and does not apply to the OpenCode 2 event surface.

@mwolson mwolson closed this Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL 500-999 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants