fix(opencode): stop desktop recovery notices from triggering an extra provider turn - #148
Open
iceteaSA wants to merge 2 commits into
Open
fix(opencode): stop desktop recovery notices from triggering an extra provider turn#148iceteaSA wants to merge 2 commits into
iceteaSA wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
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
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 pinsopencode-ai@1.17.13.Cause
opencode changed its run-loop exit condition in
packages/opencode/src/session/prompt.ts:The notice is created as a
noReply,ignoreduser message. Three facts make that fatal under the new condition:MessageV2.latest()picks the newest user message viaisAfter, which comparesinfo.time.createdfirst and only falls back to id ordering.createUserMessagestampstime: { created: Date.now() }regardless of a caller-suppliedmessageID.latest()does not skip messages whose parts areignored.So the notice always becomes
lastUser, whilelastAssistant.parentIDstill 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 isignoredand never reaches the model call — which is what makes this expensive rather than merely wrong.The existing
notificationMessageIdBeforeAssistantordering trick cannot help:time.createddominates the comparison, so no choice of message id keeps the notice out oflatest().noReplyis not implicated — opencode returns beforeloop()for those messages.Why the existing deferral was not enough
Notices are already queued and flushed on
session.statusidle or on a completed assistantmessage.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/statusreturns{}for an idle session on both 1.17.13 and 1.18.18, andstatus.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:
session.updatedthat followssession.idle, rather than onsession.idleor on an assistantmessage.updated.setImmediate), so the work happens after opencode regains control.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.session.status, so a new turn cannot inherit a stale "safe" mark.sendIgnoredMessageno longer throws when assistant ordering is unavailable; themessageIDplacement 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.13to1.18.18in.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
Regression test added in
packages/opencode/src/tests/index.test.ts: it asserts no notice is sent on a completed assistantmessage.updated, onsession.statusidle, or onsession.idle, and that it is sent after the followingsession.updated. Reinstating any of the earlier flush points fails it (Expected number of calls: 0; Received number of calls: 1).Need help on this PR? Tag
@codesmith-botwith 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 onsession.idle→session.updated, escape the handler, and probe session status outside it.session.idle, mark safe on the followingsession.updated, then schedule a probe outside handlers; clear the safe mark on any non-idlesession.status.message.updatedorsession.statusidle alone.messageIDonly when available and never throw; cap the queued notices per session.Testing and rollout
opencode-ai@1.18.18; suites remain green on 1.17.13 and 1.18.18.Written for commit 2d9cac1. Summary will update on new commits.
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.
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
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 endReviews (2): Last reviewed commit: "fix(opencode): retry inconclusive notice..." | Re-trigger Greptile