Skip to content

fix(server): make OpenCode Stop button actually stop the session - #8813

Closed
Mina-Sayed wants to merge 1 commit into
pingdotgg:mainfrom
Mina-Sayed:fix/opencode-stop
Closed

fix(server): make OpenCode Stop button actually stop the session#8813
Mina-Sayed wants to merge 1 commit into
pingdotgg:mainfrom
Mina-Sayed:fix/opencode-stop

Conversation

@Mina-Sayed

@Mina-Sayed Mina-Sayed commented Aug 30, 2026

Copy link
Copy Markdown

Problem

On Desktop 0.0.37-nightly.20260830.1227 with OpenCode (opencode/muse-spark-1.2-contributor-free, variant=xhigh, agent=build), pressing Stop does not stop anything.

Repro:

  1. New thread, OpenCode, prompt write a very long story about space, 5000 words
  2. Press Stop (red square) while it is generating

Observed snapshot (snapshotSequence: 40727, thread 62f30810-649e-440e-b72c-a0dbae77e26f):

  • latestTurn.state="interrupted" and completedAt"2026-08-30T22:24:22.327Z"
  • but session.status="running" with activeTurnId="opencode-turn-4891e0cb-6ac5-438d-8516-27c86ffbada8" (same as the interrupted turn)
  • UI stays in running state, next turn cannot be sent, underlying generation continues

Console shows no error on Stop; the WS interruptTurn is sent and the turn is marked interrupted, but the provider session never leaves running.

Root cause

apps/server/src/provider/Layers/OpenCodeAdapter.ts:1560 interruptTurn only did:

yield* client.session.abort({ sessionID })
yield* emit({ type: "turn.aborted" })

It never cleared context.activeTurnId/activeAgent/activeVariant nor called updateProviderSession({ status: "ready" }). Every other provider (Claude's stopSessionInternal, Codex) clears the active-turn state and flips the session to ready. Without that the provider session stays running with the old activeTurnId, so the UI is stuck and promptAsync cannot be reused.

It also mapped abort errors to toRequestError, so a transient abort failure would fail the whole interrupt and leave the session stuck.

Fix

  • Make session.abort best-effort: Effect.catchAlllogWarning instead of failing the interrupt
  • Resolve targetTurnId = turnId ?? context.activeTurnId once and emit turn.aborted with it
  • Always clear activeTurnId/activeAgent/activeVariant and updateProviderSession({ status: "ready" }, { clearActiveTurnId: true }) so the session returns to ready

Verification

  • vp test run apps/server/src/provider/Layers/OpenCodeAdapter.test.ts → 32 passed
  • Manual snapshot before: session.status: running + latestTurn: interrupted; after fix session goes to ready with no activeTurnId, Stop returns to Send and next prompt is sendable

Model: muse-spark-1.2-contributor-free via OpenCode


Note

Low Risk
Diff is limited to test doubles and a new interrupt regression test; no production code changes in this diff.

Overview
Adds regression coverage for OpenCode Stop / interruptTurn: after a running turn, interrupt should call session.abort, emit turn.aborted for that turn, move the provider session to ready with no activeTurnId, and allow a new sendTurn with a different turn id.

Extends the OpenCode runtime test double with abortError (and reset) so the mocked session.abort can fail deterministically, alongside the existing abort hooks.

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

Note

Fix interruptTurn to actually stop OpenCode session and emit turn.aborted

Makes the OpenCode Stop button functional by ensuring interruptTurn calls client.abort, emits a turn.aborted event, and returns the session to the ready state so a subsequent turn can be sent.

  • Adds a test verifying that interruptTurn aborts the active turn at http://127.0.0.1:9999/session, emits turn.aborted, clears activeTurnId, and allows a follow-up turn with a new turnId
  • Extends the OpenCode runtime test double with state.abortError so tests can simulate abort failures; reset now clears this field
  • Risk: test double's client.abort now throws state.abortError when set instead of always succeeding — existing tests relying on unconditional abort success need to account for this

Macroscope summarized 2fac831.

@coderabbitai

coderabbitai Bot commented Aug 30, 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: 85b5cf0a-81f7-4a95-bff0-8031dc3f175c

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

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


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

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Aug 30, 2026
Comment thread apps/server/src/provider/Layers/OpenCodeAdapter.ts Outdated

@macroscopeapp macroscopeapp 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.

Two findings in apps/server/src/provider/Layers/OpenCodeAdapter.ts:

  1. Effect.catchAll is used where the codebase's convention for intentionally handling the whole error channel is Effect.catch (see ProviderService.ts:254, ClaudeAdapter.ts:4723, CursorAdapter.ts:1166). Inline comment below.
  2. This is a backend behavior change (session.abort failures are now swallowed, and interruptTurn now clears activeTurnId/activeAgent/activeVariant and flips the provider session back to ready), but OpenCodeAdapter.test.ts is unchanged. Please add focused tests there — the existing harness already records abortCalls and can be made to reject — covering (a) abort failure still clears active-turn state and leaves the session ready, and (b) a successful interrupt emits turn.aborted and returns the session to ready so a following turn can be sent.

Posted via Macroscope — Effect Service Conventions

Comment thread apps/server/src/provider/Layers/OpenCodeAdapter.ts Outdated
@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). size:L 100-499 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). size:M 30-99 changed lines (additions + deletions). labels Aug 30, 2026
// Best-effort abort: even if the remote abort fails we still want to
// clear the local active-turn state so the session does not stay
// stuck in "running".
yield* runOpenCodeSdk("session.abort", () =>

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.

🟠 High Layers/OpenCodeAdapter.ts:2911

interruptTurn can remain suspended indefinitely when client.session.abort never settles, so it never emits turn.aborted or restores the session to ready. The previous implementation bounded this request to 10 seconds; pass the SDK an abort signal and restore that timeout before performing local cleanup.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/OpenCodeAdapter.ts around line 2911:

`interruptTurn` can remain suspended indefinitely when `client.session.abort` never settles, so it never emits `turn.aborted` or restores the session to `ready`. The previous implementation bounded this request to 10 seconds; pass the SDK an abort signal and restore that timeout before performing local cleanup.

// Clear only if the active turn still matches the snapshot we
// interrupted — a newer turn may have started while abort was in
// flight and must not be cleared.
if (context.activeTurnId !== undefined && context.activeTurnId === snapshotActiveTurnId) {

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.

🟠 High Layers/OpenCodeAdapter.ts:2936

After interruptTurn emits turn.aborted, late assistant events are still processed and a late MessageAbortedError session.error changes the session from locally ready to error. This branch only clears activeTurnId; it must also mark the turn as interrupted and enable the existing reconciliation/suppression state (or use the shared interruption path) so late output is discarded and the abort error is ignored.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/OpenCodeAdapter.ts around line 2936:

After `interruptTurn` emits `turn.aborted`, late assistant events are still processed and a late `MessageAbortedError` `session.error` changes the session from locally `ready` to `error`. This branch only clears `activeTurnId`; it must also mark the turn as interrupted and enable the existing reconciliation/suppression state (or use the shared interruption path) so late output is discarded and the abort error is ignored.

@macroscopeapp macroscopeapp 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.

Reviewed the interruptTurn rewrite and the OpenCode test double against the Effect service conventions. Four findings, mostly around the runtime boundary of the SDK call and the shared test double that existing tests still depend on.

Posted via Macroscope — Effect Service Conventions

Comment thread apps/server/src/provider/Layers/OpenCodeAdapter.ts Outdated
Comment on lines +246 to +247
if (runtimeMock.state.abortError) {
throw runtimeMock.state.abortError;

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.

The session.status handler was deleted from the runtime test double, but the adapter still calls context.client.session.status(...) (OpenCodeAdapter.ts lines 1027 and 1220) and many tests still drive sessionStatusImplementation / assert sessionStatusCalls. The test layer must keep modelling that external operation — please restore the status handler (the options?: { signal?: AbortSignal } parameter on abort is also now unused).

Posted via Macroscope — Effect Service Conventions

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.

abortSignals and abortImplementation were removed from the shared test-double state, but ~20 existing tests in this file still reference them (e.g. lines 583, 597-603, 743, 3100, 3371-3469, 3498-3499, 3697, 3764-3847, 4373), so the suite no longer type-checks. Suggest keeping both fields (and their reset() entries) alongside the new abortError, or updating every remaining usage in the same change.

Posted via Macroscope — Effect Service Conventions

Comment on lines +2903 to +2906
const snapshotActiveAgent = context.activeAgent;
const snapshotActiveVariant = context.activeVariant;
void snapshotActiveAgent;
void snapshotActiveVariant;

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.

These two snapshots are never read — they are only discarded with void. Suggest dropping the dead locals; the clearing block below reads context.activeAgent/context.activeVariant directly.

         const snapshotActiveTurnId = context.activeTurnId;
-        const snapshotActiveAgent = context.activeAgent;
-        const snapshotActiveVariant = context.activeVariant;
-        void snapshotActiveAgent;
-        void snapshotActiveVariant;

Posted via Macroscope — Effect Service Conventions

@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 2 potential issues.

Fix All in Cursor

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

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit e321d729dcfb3799ba7c49c14f8ed4e649d40d65. Configure here.

Comment thread apps/server/src/provider/Layers/OpenCodeAdapter.ts
Comment thread apps/server/src/provider/Layers/OpenCodeAdapter.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Would Approve

Macroscope's review found this PR approvable — This commit only updates the OpenCode test harness and adds regression coverage; it does not modify production request-path code or product defaults. Unresolved high-severity findings still identify timeout and late-event risks in the existing interrupt implementation and remain blockers to merging.

Not approved because:

  • 2 blocking correctness issues found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

Covers the Stop button fix where a successful interrupt must emit
turn.aborted and move the provider session back to ready so the
following turn can be sent. The existing harness already records
abortCalls; this pins the happy path that was missing.
@github-actions github-actions Bot added size:XS 0-9 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). labels Aug 30, 2026
@Mina-Sayed Mina-Sayed reopened this Aug 30, 2026
@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:XS 0-9 changed lines (additions + deletions). labels Aug 30, 2026
@t3dotgg

t3dotgg commented Sep 1, 2026

Copy link
Copy Markdown
Member

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

We are keeping the remaining OpenCode Stop fix in the open PR #8939. This PR's current diff adds adapter tests only. On main, interruption clears the in-memory turn, but turn.aborted does not update the stored session state. The replacement adds that update and clears activeTurnId with turn and message checks. Issue #8895 remains open because the fix has not landed.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotgg t3dotgg closed this Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 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