Skip to content

fix(opencode): stop desktop recovery notices from triggering an extra provider turn - #148

Open
iceteaSA wants to merge 2 commits into
cortexkit:mainfrom
iceteaSA:fix/e2e-stale-opus-bridge
Open

fix(opencode): stop desktop recovery notices from triggering an extra provider turn#148
iceteaSA wants to merge 2 commits into
cortexkit:mainfrom
iceteaSA:fix/e2e-stale-opus-bridge

Conversation

@iceteaSA

@iceteaSA iceteaSA commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

On opencode 1.18 and newer, every Fable/Opus recovery notice delivered to OpenCode Desktop causes an extra, billed provider turn. The e2e suite catches this today — tool-prefix.test.ts "bridges back to a stale Opus cache after more than 20 Fable blocks" fails 2/2 on 1.18.18 — but CI never sees it, because CI pins opencode-ai@1.17.13.

Cause

opencode changed its run-loop exit condition in packages/opencode/src/session/prompt.ts:

1.17.13:  ... && !hasToolCalls && lastUser.id < lastAssistant.id
1.18.x:   ... && !hasToolCalls && lastAssistant.parentID === lastUser.id

The notice is created as a noReply, ignored user message. Three facts make that fatal under the new condition:

  • MessageV2.latest() picks the newest user message via isAfter, which compares info.time.created first and only falls back to id ordering.
  • createUserMessage stamps time: { created: Date.now() } regardless of a caller-supplied messageID.
  • latest() does not skip messages whose parts are ignored.

So the notice always becomes lastUser, while lastAssistant.parentID still points at the original prompt. The condition is false, the loop does not exit, and opencode re-runs the provider on the same turn. The re-issued request body is byte-identical, since the notice is ignored and never reaches the model call — which is what makes this expensive rather than merely wrong.

The existing notificationMessageIdBeforeAssistant ordering trick cannot help: time.created dominates the comparison, so no choice of message id keeps the notice out of latest().

noReply is not implicated — opencode returns before loop() for those messages.

Why the existing deferral was not enough

Notices are already queued and flushed on session.status idle or on a completed assistant message.updated. Neither is safe: opencode awaits plugin event handlers before it evaluates the loop exit condition, so any flush performed inside a handler necessarily lands in the window before that check.

A status probe alone does not fix it either. GET /session/status returns {} for an idle session on both 1.17.13 and 1.18.18, and status.set(sessionID, { type: "busy" }) is the first statement of each loop iteration — so an empty map is also what you observe between iterations.

Fix

Deliver the notice only once the loop has demonstrably exited:

  1. Arm on the session.updated that follows session.idle, rather than on session.idle or on an assistant message.updated.
  2. Escape the awaited event handler before doing anything (setImmediate), so the work happens after opencode regains control.
  3. Then probe session.status() outside that critical section, and re-arm up to four times if the session is busy rather than delivering into a live turn.
  4. Clear the armed state on any non-idle session.status, so a new turn cannot inherit a stale "safe" mark.

sendIgnoredMessage no longer throws when assistant ordering is unavailable; the messageID placement is now best-effort, which is correct because the ordering only ever mattered while a loop was active.

CI pin

Bumped npm install -g opencode-ai@1.17.13 to 1.18.18 in .github/workflows/ci.yml. Without it CI cannot observe this class of bug at all. The suite is green on both versions, so the bump does not trade one blind spot for another.

Verification

  • e2e on 1.18.18: 27/27, three consecutive runs.
  • e2e on 1.17.13: 27/27, three consecutive runs.
  • typecheck, build, lint, biome check clean; opencode 1023, core and pi suites pass.

Regression test added in packages/opencode/src/tests/index.test.ts: it asserts no notice is sent on a completed assistant message.updated, on session.status idle, or on session.idle, and that it is sent after the following session.updated. Reinstating any of the earlier flush points fails it (Expected number of calls: 0; Received number of calls: 1).


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Prevents OpenCode Desktop recovery notices from causing an extra, billed provider turn on opencode-ai ≥1.18 by delivering notices only after the loop has exited. Previously we flushed inside awaited handlers where the notice became the latest user message and kept the loop alive; now we mark a safe window on session.idlesession.updated, escape the handler, and probe session status outside it.

  • Arm on session.idle, mark safe on the following session.updated, then schedule a probe outside handlers; clear the safe mark on any non-idle session.status.
  • Verify idle with a bounded, backoff probe: re-arm up to 4 times on errors, inconclusive payloads, or busy status (25ms × attempt), then flush; do not flush on assistant message.updated or session.status idle alone.
  • Make fallback notice placement best-effort: set messageID only when available and never throw; cap the queued notices per session.
  • Add regression coverage for the post-idle update flow and probe retries.

Testing and rollout

  • CI now installs opencode-ai@1.18.18; suites remain green on 1.17.13 and 1.18.18.
  • No migration actions required.

Written for commit 2d9cac1. Summary will update on new commits.

Review in cubic

Greptile Summary

Prevents desktop recovery notices from extending OpenCode’s provider loop by deferring delivery until a post-idle session update and confirming session status asynchronously.

  • Replaces completion/idle-handler flushing with a post-idle safe-session gate and bounded status probes.
  • Makes notification message ordering best-effort when assistant ordering context is unavailable.
  • Updates regression coverage and runs end-to-end CI against OpenCode 1.18.18.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains within the scope of this follow-up review.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/opencode/src/index.ts Defers recovery-notice delivery beyond awaited host event handlers, adds bounded idle probes, and relaxes assistant-ordering requirements.
packages/opencode/src/tests/index.test.ts Updates notice-delivery tests to require the idle/update sequence and exercises transient, malformed, busy, and idle status responses.
.github/workflows/ci.yml Updates the globally installed OpenCode version from 1.17.13 to 1.18.18 so CI exercises the changed run-loop behavior.

Sequence Diagram

sequenceDiagram
    participant OC as OpenCode
    participant Plugin as Auth Plugin
    participant Status as Session Status API
    participant Desktop as Desktop Notice
    OC->>Plugin: session.idle
    Plugin->>Plugin: Arm post-idle session
    OC->>Plugin: session.updated
    Plugin->>Plugin: Mark session safe
    Plugin-->>Plugin: setImmediate probe
    Plugin->>Status: session.status()
    alt Session idle or omitted
        Plugin->>Desktop: Send ignored noReply notice
    else Session busy or probe fails
        Plugin-->>Plugin: Retry with bounded delay
    end
Loading

Reviews (2): Last reviewed commit: "fix(opencode): retry inconclusive notice..." | Re-trigger Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 3 files

Architecture diagram
sequenceDiagram
    participant OC as OpenCode Loop
    participant Plugin as Auth Plugin
    participant Status as Session Status API
    participant Desktop as Desktop Recovery
    participant Prompt as Prompt API

    Note over OC,Plugin: Recovery Notice Queueing
    OC->>Plugin: recovery notice event
    Plugin->>Desktop: check TUI connected
    alt TUI offline
        Desktop->>Plugin: queue notice (max 4)
        Plugin->>Plugin: check safe session mark
        alt session marked safe
            Plugin->>Plugin: scheduleDesktopNoticeProbe()
        end
    end

    Note over OC,Plugin: Session Lifecycle Events
    OC->>Plugin: session.status (non-idle)
    Plugin->>Plugin: clear post-idle & safe marks
    Plugin->>Plugin: cancel pending probes

    OC->>Plugin: session.idle
    Plugin->>Plugin: mark session post-idle
    
    OC->>Plugin: session.updated
    alt post-idle mark present
        Plugin->>Plugin: mark session safe
        Plugin->>Plugin: scheduleDesktopNoticeProbe()
        Plugin->>Plugin: setImmediate(escape handler)
        Note over Plugin: Work happens after opencode regains control
    end

    Note over Plugin,Status: Bounded Status Probing (max 4 attempts)
    Plugin->>Status: session.status()
    alt session busy or undefined status
        Status-->>Plugin: busy
        Plugin->>Plugin: re-arm probe (attempt+1)
        Note over Plugin: Do NOT deliver into live turn
    else session idle
        Status-->>Plugin: idle
        Plugin->>Prompt: sendIgnoredMessage() with queued notice
        Prompt-->>Plugin: confirmation
    end

    Note over Plugin,Prompt: Message Placement (best-effort)
    Plugin->>Prompt: construct notice with messageID
    alt messageID available
        Prompt->>Prompt: set messageID (best-effort placement)
    else messageID unavailable
        Note over Prompt: No error - placement is optional
    end
    Prompt-->>Desktop: recovery notice delivered

    Note over OC,Plugin: Loop Exit Safety
    Note over OC: Exit condition checks lastAssistant.parentID === lastUser.id
    Note over Plugin: Deferred delivery avoids notice becoming lastUser
    Note over Plugin: ignored noReply notices never trigger extra provider turn
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/opencode/src/index.ts
Comment thread packages/opencode/src/index.ts Outdated
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.

1 participant