fix(cli): register OpenCode 2 hooks via the setup context - #2385
HazielMagallanes wants to merge 1 commit into
Conversation
|
Thank you so much, @HazielMagallanes, this is a genuinely careful piece of work. You didn't just switch
That compatibility table is the piece we were missing when #2089 was reviewed. We checked your central claim against the published
#2204 independently reports the same registration working on 2.0.3. So the reason #2089 left On coordination with #2089: @AmirF194's PR is already reviewed and approved in the requested shape, and we've committed to landing it, so it goes first. After that, this PR becomes the delta you described yourself. Right now the two conflict in A few small things while you're in there, each with the reason:
Please also say "Fixes #2204" in the description, since this is what that issue asked for. CI is still queued on our side because the runner pool is busy, which has nothing to do with your diff. Thanks again. This fills the gap #2204 asked about without undoing anything #2089 fixed. |
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
|
Addressing the review, @DeusData — pushed c03575e:
Real-host proof (OpenCode 2.0.8, A real One note for anyone reproducing this: Still pending on our side: the rebase and folding #2089's two assertions into the single contract test — #2089 has not merged yet, so I did not want to add a second, disagreeing test in the meantime. I'll rebase and flip those assertions as soon as it lands. Description updated to say |
|
Quick update, @HazielMagallanes: #2089 is now merged. As expected, this branch now conflicts with |
|
I'll work on it in a minute ^^ |
- keep the `server(ctx)` entry that OpenCode 1.18.x dispatches hooks from - fill `setup(ctx)` with the V2 hook registrations (execute.after, compaction, context) plus the domain guard for older V2 preview loaders, which stay on the server() path - prefer `content` over `output` in appendResult and fall back to a string `output`, so the note still lands when no content surface exists - key post-compaction reinjection by session id and yield in the server() hooks once setup() has registered, preventing double appends - update the DeusData#2089 emission test in place to state the 2.x contract Fixes DeusData#2204 Refs DeusData#2077 DeusData#2089 Signed-off-by: HazielMagallanes <contactame.haziel@gmail.com>
cb307a4 to
534274c
Compare
|
Rebased onto current
CI is re-running on the new head. Thanks again for the review, and for the explicit steer on updating the existing test rather than adding one beside it. |
Summary
The generated OpenCode plugin now serves both loader generations from one default export:
default.serverand dispatches the hooks it returns (unchanged).default.server, validates{ id, setup | effect }and callssetup(ctx)with the plugin context. The module registersexecute.after,compactionandcontextthere; the augmentation is appended tocontentfirst (string, then text parts) and falls back to a stringoutput, creating a content part when a non-stringoutputhad no text surface — so the note reaches the model whichever surface a built-in tool fills.setup()returns early when the context has no tool/session domains, so older V2 preview loaders that accept the definition but expose no domains keep usingserver()instead of throwing. Theserver()hooks yield oncesetup()has registered (setupRegistered), so a host that dispatches both entries cannot append twice. Post-compaction reinjection is keyed by session id rather than one plugin-wide flag.Why
setup()is not empty#2089 settled the
server: async (ctx) => { … }, setup() {}shape against the 1.18.x loaders. On current OpenCode 2.x, however,serveris ignored: the 2.x plugin loader decodes the default export as{ id, setup } | { id, effect }and invokessetup(ctx), whose context exposesctx.tool.hook("execute.after", …),ctx.session.hook("compaction" | "context", …)andctx.location.directory(verified against OpenCode 2.0.8 with@opencode/plugin2.0.8). With an emptysetup, the plugin loads without error but registers zero hooks — a silent no-op of the kind #616 documents. This PR keeps theserverentry requested in #2089 and fillssetupwith the same behaviour, guarded for older loaders.Rebased on top of #2089 (now merged): the empty
setup()placeholder it pinned is replaced by the 2.x registrations, and #2089's emission test now states the 2.x contract in place instead of asserting the empty shape.Real-host proof
opencode run --standalone(OpenCode 2.0.8) in a re-indexed OpenSkyrim checkout, with the generated module installed as~/.config/opencode/plugins/cbm-augment.ts:msg="loading plugin" id=…/cbm-augment.ts— nofailed to load pluginline.greptool result ends with the appended graph context:(The first attempt returned empty context because the indexes predated 0.11.0;
index_repositorymigrated the format andhook-augmentemitted context again.)Verification
scripts/test.sh --suites agent_clients: 38 passed. The emission test pinsserver, the guardedsetup(ctx), the three V2 hook registrations, the content-first append order, the per-session reinjection set, the double-dispatch guard, and the removal ofexport const CodebaseMemory.make lint-formatpasses with clang-format 20.1.8 (CI's pinned version).Fixes #2204
Refs #2077 #2089