Repository navigation
fix(opencode): co-promote the session-URL notice with the feedback on OpenCode 2 - #1518
Merged
Merged
Conversation
… OpenCode 2 (#1515) On OpenCode 2 a plugin message is a pending inbox row; resume: false only declines the immediate wake. SessionInbox.promote publishes pending steers as a batch but queued rows one at a time, oldest first, so the queued notice #1460 introduced was promoted alone by the feedback's own wake and became its own model turn with the reviewer's annotations behind it. The notice now rides steer delivery, and feedback that follows an admitted notice rides steer too, so both enter one promotion and the agent acts on the annotations. A rejected notice, OpenCode 1, and hosts without session.synthetic keep the queue delivery unchanged.
backnotprop
added a commit
that referenced
this pull request
Sep 15, 2026
…before steering feedback (#1536) #1518 made the OpenCode 2 client post the session-URL notice as a "steer" and, while a notice had been posted, admit the reviewer's feedback as a "steer" too, so the two share one promotion (#1515). The flag behind that, `noticePending`, meant "we posted a notice during this review" rather than "our notice is still an un-promoted inbox row", and nothing ever observed promotion. The two diverge as soon as anything else wakes the session: a user typing an unrelated message promotes every pending steer as one batch (`SessionInbox.promote`), our notice included, and starts a turn. Feedback arriving minutes later was still admitted as a steer, landing in the middle of that unrelated turn — exactly the late-arrival case `FEEDBACK_DELIVERY = "queue"` exists to prevent. `createNoticePendingTracker` now answers the question from the host itself. It subscribes to `ctx.event.subscribe()` before the notice is posted and clears the flag when the host reports that row settled, speaking both live vocabularies: `session.inbox.delivered` / `session.inbox.cancelled` with `data.inboxID` (v2.0.x and dev) and the pre-rename `session.input.promoted` with `data.inputID` (the `0.0.0-next-*` host this package pins). The row id comes from `session.synthetic`'s own response. A host that reports no id, exposes no event domain, or whose stream never delivers degrades to exactly the previous behavior. The client gains `dispose()`, called in a `finally` by both call sites, so a review that ends without delivering a prompt does not leave an event subscription open on the host. A notice stranded by a review closed without feedback is unchanged: the plugin session domain exposes no way to withdraw a pending row.
backnotprop
added a commit
that referenced
this pull request
Sep 15, 2026
`stop()` aborts the controller and leaves it `undefined`, which is exactly the state the watcher starts from, so aborting alone could not express "never again": a `notifyUrl` that outlived `dispose()` re-opened a host event subscription with no owner left to abort it — the leak #1536 added `dispose()` to prevent. Both call sites dispose in a `finally` after the review returns and `toastPlannotatorUrl` never awaits `notifyUrl`, so nothing in the type orders the two. Unreachable on main today (the CLI child's pumps are torn down inside `runCli`'s own `finally`), latent the moment notice delivery outlives the command. Add a `disposed` latch, set only by `dispose` and never by `settle`: the watcher returns early and `admitted()` arms nothing, since after dispose no watcher is left that could lower the flag again. Also close the pre-existing arming window carried over from #1518. `pending` was raised only after the un-awaited `session.synthetic` round-trip, so feedback delivered while the notice was still posting read as not pending and queued behind a notice that is then promoted alone as its own model turn — the #1515 symptom. `watch` becomes `posting`, which starts the watch AND arms provisionally; the notifier wraps `synthetic` so a refusal calls the new `rejected()` and lowers the arm again, which the existing "a rejected notice leaves the feedback queued" test still pins.
1 task
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
1 participant
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.
Closes #1515. Follow-up to #1460 / #1459.
What the reporter sees
On OpenCode v2.0.2,
/plannotator-lastprints "◈ Plannotator session ready: " and the agent stays idle (correct). When Send Feedback arrives, the agent wakes but first runs a whole model turn on the URL notice, and the annotations land in the turn after it.Why, verified against the OpenCode v2.0.2 source (
ea5ae23295)session.syntheticis not a notification. It admits a pending inbox row;resume: falseonly declines the wake right then (packages/core/src/session/session.ts:168,304). It never exempts the row from the next promotion.steer | queue(packages/schema/src/session-inbox.ts:11). There is no non-promotable kind, and the pluginSessionDomainPick has no inbox cancel (packages/plugin/src/promise/session.ts:112-129), even though the server routes one.packages/core/src/session/inbox.ts:496-535): pending steers publish as a batch; queued rows publish one at a time, oldest first (limit(1)), and only after the steers.session.promptwakes the session at an idle boundary withinputscope.promote(…, "input")then takes the single oldest queued row, which is the notice fix(opencode): queue-deliver the session URL notice (#1459) #1460 pinned toqueue. A promoted synthetic is a plain user-role message (runner/to-llm-message.ts:257), so it gets a full model step.#1460's own caveat named this hole. It is not a spurious wake; it is the reviewer's feedback waking the session.
The fix
The notice rides
steerdelivery (the host default), and feedback that follows an admitted notice ridessteertoo, so the two share one promotion and enter one turn. A rejected notice, OpenCode 1 (which uses a real toast and has no inbox), and hosts withoutsession.synthetickeepFEEDBACK_DELIVERY = "queue"and its late-arrival behavior unchanged.apps/opencode-plugin/index.ts(v1) is untouched.Side effect, stated: while a notice of ours is pending, feedback submitted during a running turn lands at that turn's next step boundary instead of after it. That is bounded to sessions that posted a notice, where the queued ordering was already broken.
Also declares
deliveryon the duck-typedsyntheticsignature, which the code has passed since #1460 without a type.Not in this PR
<pkg>/tui,ctx.ui.toast.show) on hosts that exposefeatures.tui. Worth a follow-up; it would restore v1's toast semantics.session.inbox.cancelin the pluginSessionDomain.Test plan
bun test apps/opencode-plugin: 175 pass. FlippingCO_PROMOTED_DELIVERYback to"queue"fails exactly the three new/changed assertions (notice delivery, feedback-after-notice delivery, plan-review URL delivery) and nothing else.bun build apps/opencode-plugin/server.ts --target nodesucceeds.