Skip to content

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

Open
iceteaSA wants to merge 3 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 3 commits into
cortexkit:mainfrom
iceteaSA:fix/e2e-stale-opus-bridge

Conversation

@iceteaSA

@iceteaSAiceteaSA 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]smithAutofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Stops OpenCode Desktop recovery notices from triggering an extra, billed provider turn on opencode-ai ≥1.18 by delivering notices only after the run loop exits. Previously we flushed inside awaited handlers (idle or assistant completion) which made the notice the latest user message and kept the loop alive; now we arm on session.idle, mark safe on the next session.updated, escape the handler, probe status with bounded retries, and flush only when idle.

  • Arm on session.idle, mark safe on the following session.updated; clear the safe mark on any non‑idle session.status; schedule a probe only when safe.
  • Probe session.status() outside handlers with up to 4 attempts (25ms × attempt); re‑arm on errors, inconclusive payloads, or busy; do not flush on message.updated or idle status alone.
  • Make notice placement best‑effort: set messageID only when available and never throw; cap queued notices per session.
  • Update ARCHITECTURE.md and changelogs to document deferred delivery; CI now installs opencode-ai@1.18.18; e2e remains green on 1.17.13 and 1.18.18. No migration actions required.

Written for commit 0ee8cd1. Summary will update on new commits.

Review in cubic

Greptile Summary

The PR defers OpenCode Desktop recovery notices until after the post-idle session update and verifies session status outside the awaited event handler.

  • Adds bounded, delayed status probes before delivering queued recovery notices.
  • Clears delivery eligibility when a session becomes non-idle or is deleted.
  • Makes assistant-relative message-ID placement best-effort.
  • Updates regression coverage and pins CI to OpenCode 1.18.18.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains.

No blocking failure remains.

Important Files Changed

FilenameOverview
packages/opencode/src/index.tsReplaces in-handler notice flushing with post-idle arming, asynchronous status probes, bounded retries, and lifecycle cleanup.
packages/opencode/src/tests/index.test.tsUpdates recovery-notice tests to assert post-idle delivery and exercise transient, malformed, and busy status-probe results.
.github/workflows/ci.ymlUpdates the globally installed OpenCode version from 1.17.13 to 1.18.18 so CI exercises the changed run-loop behavior.
ARCHITECTURE.mdDocuments the post-idle notice-delivery sequence, retry behavior, and OpenCode 1.18 compatibility rationale.

Sequence Diagram

sequenceDiagram
participant OC as OpenCode
participant Plugin
participant Status as session.status()
participant Desktop
OC->>Plugin: session.idle
Plugin->>Plugin: Arm post-idle update
OC->>Plugin: session.updated
Plugin->>Plugin: setImmediate(probe)
Plugin->>Status: Query session state
alt Busy or inconclusive
Plugin->>Plugin: Re-arm bounded delayed probe
else Idle or omitted
Plugin->>Desktop: promptAsync ignored/noReply notice
end
Loading

Reviews (3): Last reviewed commit: "docs: describe deferred desktop notice d..." | Re-trigger Greptile

Context used:

@cubic-dev-aicubic-dev-aiBot 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 threadpackages/opencode/src/index.ts
Comment threadpackages/opencode/src/index.ts Outdated
Sign up for freeto 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

@iceteaSA