Repository navigation
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
The hook works as described: I ran session-runner.test.ts (234 pass; the new scenarios cover a grant, the cap of 2, a throwing voter, empty text and the reset, all through the real drain with the simulated model) and typecheck in packages/core and packages/plugin, both clean.
Public API: adds the experimental.stopping session hook to the plugin API, with the new SessionStopping and SessionStoppingDecision types in both @opencode/plugin effect and promise. As a side effect, core now writes the opencode.stopping.continuations key into session metadata, which emits durable session.metadata.updated events and shows up to clients. Additive and backwards compatible; nothing is removed or changed.
Two concerns, both inline:
- Where the counter lives.
SessionMetadatais documented as host-supplied and opaque to core. Storing a runner counter there with read-modify-write updates can overwrite the host's metadata changes, can be wiped bysetMetadatain the middle of a turn (reopening the cap), and leaves the key behind for clients, children and forks after the turn. Turn-scoped runner state seems a better home. - The extra vote where the drain starts. It runs right after the reset with
step: 0, so any wake with nothing pending can hand out two more grants with no user input. The per-turn cap doesn't hold across such wakes.
There was a problem hiding this comment.
Both concerns from my last review are resolved. The stopping counter now lives in the drain: it is carried across reloads and moves, starts at 0 on each fresh turn, and is never written to session metadata. The vote at drain start is gone, so a drain with nothing pending completes without asking voters. I re-ran session-runner.test.ts (236 pass) and core typecheck (clean).
Public API: adds the experimental.stopping session hook and the SessionStopping / SessionStoppingDecision types to @opencode/plugin (effect and promise). Additive and backwards compatible. The internal SessionRunner.Continuation type gains a stoppingContinuations field, but nothing outside core depends on it.
One thing left: prettier --check flags packages/core/src/session/runner/llm.ts (for example the long DrainResult.Reloaded(...) line) and packages/core/test/session-runner.test.ts; the base passes the same check. Please run the formatter on both files.
|
prettier --check is satisfied by 831121a |
|
prettier --check is satisfied by 831121a<831121a>
From: opencode-agent[bot] ***@***.***>
Sent: Sunday, 11 October 2026 16:17
To: anomalyco/opencode ***@***.***>
Cc: Jerome Witmann ***@***.***>; Author ***@***.***>
Subject: Re: [anomalyco/opencode] feat(session): add experimental.stopping continuation hook (PR #54487)
@opencode-agent[bot] commented on this pull request.
Both concerns from my last review are resolved. The stopping counter now lives in the drain: it is carried across reloads and moves, starts at 0 on each fresh turn, and is never written to session metadata. The vote at drain start is gone, so a drain with nothing pending completes without asking voters. I re-ran session-runner.test.ts (236 pass) and core typecheck (clean).
Public API: adds the experimental.stopping session hook and the SessionStopping / SessionStoppingDecision types to @opencode/plugin (effect and promise). Additive and backwards compatible. The internal SessionRunner.Continuation type gains a stoppingContinuations field, but nothing outside core depends on it.
One thing left: prettier --check flags packages/core/src/session/runner/llm.ts (for example the long DrainResult.Reloaded(...) line) and packages/core/test/session-runner.test.ts; the base passes the same check. Please run the formatter on both files.
Fixes #16626<#16626>
Related: #12472<#12472>
-
Reply to this email directly, view it on GitHub<#54487?email_source=notifications&email_token=AKSPIQLDBWMOVWAIGYNTOYT5TNFXHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKNBYGI3TEMBTGA2KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-5482720304>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AKSPIQLOOVXGN5MBQWCAWYL5TNFXHAVCNFSNUABFKJSXA33TNF2G64TZHM4TONJXGM2DGMJZHNEXG43VMU5TKNZZHE3DANRTGE3KC5QC>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/AKSPIQNUPJSQLEUQCDVAL5L5TNFXHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKNBYGI3TEMBTGA2KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/AKSPIQL2AQALKQEJ36QOKYT5TNFXHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKNBYGI3TEMBTGA2KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because you authored the thread.Message ID: ***@***.******@***.***>>
|
831121a to
8c38173
Compare
|
Update since the last review (831121a):
Rebased onto upstream/dev f1632d0 — CONTINUE_AFTER_OUTPUT_LIMIT and STOPPING_MAX_CONTINUATIONS coexist, both handlers wired. |
There was a problem hiding this comment.
Both earlier concerns stay resolved: the counter is kept in the drain with no metadata writes, and the vote at drain start is gone. The new commits also restart the counter when fresh input arrives mid-drain, and the vote no longer holds up abort or shutdown. I re-ran session-runner.test.ts (243 pass), core typecheck and prettier --check (all clean).
Public API: unchanged since my last review. It adds the experimental.stopping session hook and the SessionStopping / SessionStoppingDecision types to @opencode/plugin (effect and promise); additive and backwards compatible.
Blocking: the new reset makes the per-turn cap of 2 bypassable in drains that only promote steers (the ones started by interrupt with resume) whenever a queued prompt is parked. Details and a repro are inline.
Related: #12472
|
Update since ddab695 (now head faa5c01):
This also closes the open |
faa5c01 to
544f9e3
Compare
There was a problem hiding this comment.
My earlier points are addressed. The cap now holds in steer-only drains with a queued prompt parked, which matches my repro. The new commits look sound. A voter that throws is now logged as a warning, and the turn still stops. The vote's step is reported consistently, and the docs now describe a reload or move landing on the turn boundary. I re-ran session-runner.test.ts (249 pass), typecheck:tsc in core and plugin, and prettier --check, all clean. The typecheck script now calls bun check, which my installed Bun lacks, so I used typecheck:tsc; that is unrelated to this PR.
Public API: adds the experimental.stopping session hook and the SessionStopping / SessionStoppingDecision types to @opencode/plugin (effect and promise). Additive and backwards compatible. The docs moved to build/plugins/server.mdx, following an upstream rename.
No further concerns.
Pre-terminal continuation votes at natural turn end; capped synthetic-steer loop with durable counter.
Review follow-up: count rides in Continuation instead of host metadata; empty wakes Complete without voting.
544f9e3 to
dd3913a
Compare
README described the gate as continuation-not-veto without pointing at the proposed fix. Now references anomalyco/opencode#54487 (open) as the missing pre-terminal vote hook, framed as direction-not-promise.
…ssion-stopping # Conflicts: # packages/core/src/session/runner/llm.ts
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
|
Still actively maintained — rebased onto current dev on Nov 6, mergeable, CI green. This implements #60461 (pre-Complete veto with budget), which triage confirmed as distinct from the idle-loop proposals that all cross-link it as the complementary mechanism. Has a real downstream consumer waiting (completion gate in opencode-dev-framework). Please reopen. For context: four PRs have implemented this hook (#41811, #44712, #47300, #54487) and none has received a maintainer review — the only maintainer comment in the lineage is a V1→V2 redirect. This keeps getting closed by automation without ever being looked at. |
Pre-terminal continuation votes at natural turn end; capped synthetic-steer loop with drain-owned counter.
Issue for this PR
Closes #60461 (supersedes #16626, closed as not-planned V1-only)
Type of change
What does this PR do?
Adds an
experimental.stoppingsession hook that fires at natural turn end, beforeComplete(). A voter can grant a continuation (shouldContinue+ steer text, admitted as a synthetic steer) or the turn ends. Grants are capped at 2 per turn with the counter carried in the drain continuation, and everything fail-closed: default stop, throwing voter stops the turn, empty grant text abstains. Review asks from #16626: contract shape, trigger placement, counter home.Update after review: the counter no longer touches session metadata — it rides in the drain continuation (fresh drains start at 0, moves carry it). The entry-site vote is gone too; only the in-loop site votes. The vote's
stepremains a dedicated executed-steps counter, since the drain resetsstepto 1 just before mid-work votes.How did you verify your code works?
packages/core/test/session-runner.test.ts(grant, cap exhaustion, stepsLimit, throw, empty-text, voter ordering, move-pending, fresh-turn restart, no metadata writes, empty wake, move carry); fail-before proven by stashing the implementation. Full file 236/236 green,bun run checkgreen.Screenshots / recordings
N/A — no UI change.
Checklist
Added by the opencode bot (AI-generated).
Related: #12472
Public API: adds the experimental.stopping session plugin hook with SessionStopping / SessionStoppingDecision types (backwards compatible)