feat: full OpenCode V2 adaptation (beta-19425 contract, compaction context, restart task recovery) - #49
Conversation
danyel117
left a comment
There was a problem hiding this comment.
Thanks for the thorough V2 work. The documented local gate and the real beta-19425 lifecycle smoke both pass here, but restart recovery still has two reproducible correctness gaps that can violate task deferral:
-
Recovery races the first lifecycle event.
recoverTrackedTasks()is launched withvoidafter the event consumer starts, sosession.execution.succeeded/session.idlecan reachrunAutoContinue()beforesession.context()returns. I reproduced this by deferringcontext(), emittingsession.execution.succeeded, and then resolving a transcript containingtask_id: T_race / state: running; one continuation had already been sent. Please await recovery before events can continue goals, or gate each session's continuation on its recovery promise. Add a regression where the settled event arrives while transcript loading is still pending. -
Historical terminal tasks are never reconciled by later transcript messages.
recoverFromTranscript()replays task/subagent tool entries, but it never replays assistant markers. A completed task is therefore restored asterminalUnreconciledat the current time even when a later assistant message already integrated its result, blocking continuation until the task-block ceiling. I reproduced this with a completed task entry followed by a later assistant message; the expected continuation never arrived. Please replay transcript messages in order (including assistant markers) and cover completed/failed task output followed by a later assistant turn, while retaining the running-task case.
There is also a compatibility-metadata issue: changing engines.opencode from >=1.17.1 to >=0.0.0-beta-0 accepts stable releases 0.0.1 through 1.17.0, despite the PR stating that V1 support is unchanged. Please use a disjoint range that preserves the V1 floor while including the intended 0.0.0-beta-* builds (for example, >=0.0.0-beta-0 <0.0.0 || >=1.17.1) and add/adjust the package assertion.
Validation performed on f2af84d: bun run lint, bun run typecheck, all 264 existing tests, bun run build, bun run pack:dry-run, and bun run smoke:v2 against opencode2 v0.0.0-beta-19425 all passed. The two targeted negative regressions above both failed, confirming the recovery defects.
|
CI follow-up: the first approved run of the new |
|
All four items were verified against the code and reproduced before fixing. Pushed as 1. Recovery races the first lifecycle event — verified: 2. Historical terminal tasks never reconciled — verified: 3. engines.opencode range — verified: 4. Smoke timeout opacity (CI follow-up) — addressed in 366e02f: every wait stage is labeled ( Validation on 366e02f: |
danyel117
left a comment
There was a problem hiding this comment.
Thanks — the three requested follow-ups now satisfy the previous review. I revalidated the fixed diff and the functional/spec axis is clean; locally, lint, typecheck, 268 tests, build, pack dry-run, and the V2 smoke against exact beta-19425 all pass. Two new blockers remain on 366e02f:
-
[Medium] Remove the undeclared registry mirror from
bun.lock. The new lockfile now pins 581 tarballs tohttps://repo.nju.edu.cn/repository/npm/...(starting atbun.lock:35), while both the reviewed parent andmainhad zero such references.bun install --frozen-lockfiletherefore routes CI/release dependency retrieval through an unrelated third-party mirror. Please regenerate or normalize the lockfile with the official registry and verifyrg -c "repo\.nju\.edu\.cn" bun.lockreturns no matches. -
The required V2 smoke is still nondeterministic on Linux. After approving the fork workflow, run 34604475252 failed twice at
scripts/smoke-v2-lifecycle.ts:174: firstautoTurnswas 0, then 1, while the uploaded artifacts showedstatus: complete,continuationCalls: 3, and the expected value was 2. This is not the prior opaque timeout; the new diagnostics made the failure clear. The same head passes locally on macOS with Bun 1.4.2 and both beta-19425/current beta, so please isolate the Linux accounting/synchronization race and make the release gate deterministic rather than weakening the assertion.
Minor standards cleanup while touching the smoke: current lines 118, 173, and 206 exceed the documented 130-column limit; deadLineMs at line 206 is also an absolute timestamp and would read more accurately as deadlineAtMs.
CI run: https://github.com/prevalentWare/opencode-goal-plugin/actions/runs/34604475252
The previous lockfile rewrite resolved all 581 tarballs through a local mirror (repo.nju.edu.cn) because the authoring machine's global bunfig overrides the install registry. Restore official registry URLs; integrity hashes are unchanged, and bun install --frozen-lockfile verifies cleanly.
event.subscribe() delivers every session event to each loaded location's plugin instance, and session events carry no envelope location (routing metadata stays inside the host bus). The previous guard keyed on event.location, so envelope-less events passed as owned in sibling instances: both instances accounted usage, recorded progress, and ran continuations through separate in-process mutation queues, and the interleaved read-modify-write on the shared goal state lost updates (autoTurns undercounted, reservations and completion writes clobbered). This made the V2 lifecycle smoke nondeterministic on CI. Ownership is now resolved per session: an event envelope wins when present, session.created uses its data.location, and every other session event resolves the session's actual location via session.get() compared against context.location (positive and negative answers cached, lookups deduplicated; transient lookup failures are not cached). Local execution paths (prompt hook, tool hooks, command execution, delivered continuations) mark ownership directly because they only fire in the owning instance. Sibling instances keep observing child lifecycles for cross-location task deferral but never mutate shared goal state. Adds a dual-instance regression covering broadcast envelope-less events; it fails when the guard treats them as owned.
Wrap the three over-130-column diagnostics lines from the previous push (plus the equally long PASS summary) and rename the absolute timestamp field deadLineMs to deadlineAtMs.
|
All three items were verified before fixing. Pushed as 1. Undeclared registry mirror in 2. V2 smoke nondeterminism on Linux — root-caused, fixed, and reproduced before/after on the same machine. Root cause (from the uploaded artifacts plus a temporary instrumentation trail): Local reproduction: pinned to 2 CPUs to approximate the CI runner, the head (366e02f) failed 11 of 20 runs — both the Fix (37d539c): ownership is now resolved per session instead of inferred from the event envelope — an explicit envelope wins when present; Regression: Verification on the fixed build: the same pinned 20-run loop passes 20 of 20; full gate passes ( 3. Minor standards cleanup — done in 59e5d2c: the three flagged lines (now 118/174/230 after the wrap) are within the 130-column limit, the equally over-long new Heads-up: this push will again need workflow approval to start (fork-PR gating). |
danyel117
left a comment
There was a problem hiding this comment.
Thanks — the previous registry, Linux race, and smoke-cleanup requests are materially addressed, and the full local gate passes on 59e5d2c (frozen install, lint, typecheck, 269 tests, build, pack dry-run, and V2 smoke with autoTurns=2). Two new-diff items remain:
-
[Medium] Clear foreign ownership entries when a session is deleted. In
handleV2Event, a sibling instance resolves a foreign session and cachessessionOwnership.set(sessionID, false). Itssession.deletedpath only callstaskTracker.observeSessionDeleted(sessionID)and returns atsrc/server.ts:2255-2268, so the cleanup atsrc/server.ts:2393is owner-only. A long-lived shared server therefore retains one negative cache entry per deleted session per sibling location indefinitely. Please delete the ownership entry in the foreign deletion path as well (and add a regression proving the cache is cleared/re-resolved after deletion). -
[Low] Finish the requested 130-column cleanup in the new regression.
test/server-v2.test.ts:1111and:1119-1122are 132-180 columns, whileCONTRIBUTING.mdrequires 130-column lines. Please reflow those five expressions.
I did not find additional high/medium functional or spec issues in 366e02f...59e5d2c.
The foreign branch of handleV2Event handled session.deleted with only taskTracker.observeSessionDeleted, so the sessionOwnership cache entry was freed only by the owning instance. A long-lived shared server would retain one negative entry per deleted session per sibling location indefinitely, and a re-created session could never be re-resolved. The foreign deletion path now clears the entry too, and the dual-instance regression proves the cache is re-resolved (a second session.get lookup) after deletion. Also reflows the five over-130-column expressions the review flagged in the new regression.
|
Both items were verified against the code before fixing. Pushed as 1. Foreign ownership entries on session deletion — verified: the foreign branch's 2. 130-column cleanup in the regression — the five flagged expressions (the Validation on d958246: Workflow approval will be needed again for the fork CI run. |
danyel117
left a comment
There was a problem hiding this comment.
Re-reviewed the incremental fix from 59e5d2c through d958246. Both requested items are resolved: foreign session deletion clears the ownership cache and the regression proves re-resolution without sibling prompts; the five newly flagged overlong expressions are reflowed. No new high/medium findings. Local validation passed (frozen install, lint, typecheck, 269 tests, build, pack dry-run, V2 lifecycle smoke with status=complete and autoTurns=2). CI run 34744999936 is fully green.
Summary
Full OpenCode V2 adaptation. The package now targets the current published V2 contract (
@opencode/plugin@0.0.0-beta-19425, the@opencode/*npm scope) and closes the remaining V2 gaps: goal context is preserved across V2 session compaction, Task-subagent deferral state is rebuilt from persisted transcripts after a plugin/server restart, the legacysession.errorbranch is removed, TUI runtime dependencies are shared with the host viapeerDependencies, and both CI and publish are gated on a real V2 lifecycle smoke test. V1 support is unchanged.Related issue
None open. This closes the gap analysis against the current V2 beta: pinned SDK contract, missing V2 compaction context, missing V2 restart task recovery, stale
session.errorhandling, non-conforming TUI packaging, and absent V2 CI verification.Changes
choreMigrate the V2 development contract to@opencode/plugin@0.0.0-beta-19425(new@opencode/*scope for plugin/schema/client/theme; V1 runtime dep untouched).featPreserve goal context across V2 session compaction: registers thesession.compactionhook and injects the goal snapshot (compactionContext) into the request system parts, idempotently. BecauseSessionHooks.compactionlanded upstream only after beta-19425 (opencode PR #48212), the registration is defensive — a no-op on beta-19425 hosts, active on newer hosts.featRebuild task-deferral state after restart:TaskTracker.recoverFromTranscriptreplays each non-closed goal session's persisted transcript (finalizedtask/subagenttool entries) at plugin startup, compensating for the V2 plugin context having no live child-session query.refactorDrop the V2session.errorcase;session.execution.failedis the only terminal-failure event in the current public manifest. The native-retry interplay test was rewritten to current semantics.fixMove@opentui/solidandsolid-jstopeerDependencies(host instances are shared; no duplicate Solid/OpenTUI runtimes), keep dev copies;engines.opencodenow covers0.0.0-beta-*hosts.ciAdd aV2 Lifecycle Smokejob toci.ymland gatepublishon it (@opencode/cli@beta+bun run smoke:v2).docsREADME/AGENTS.md sync: pinned beta version, compaction + transcript-recovery support claims, remove the outdated "V1-only" statement.testNew V2 tests: compaction hook (injection, idempotency, no-goal no-op) and transcript-based restart recovery (deferral rebuilt from a running child in history; control continues without deferral). Extended the pre-existing flaky watchdog test'swaitFordeadlines (it also fails onmain, ~1/6 in isolation on slow filesystems).Verification
Local gate (all fresh runs on this branch):
Real V2 lifecycle smoke —
bun run smoke:v2against a privateopencode2 v0.0.0-beta-19425server with an isolated home/config/db/state and a deterministic local fixture model (no provider credentials):{ "result": "PASS", "sessionID": "ses_f754c0b91ffevDUswBtoBDXJxC", "modelCalls": 6, "continuationCalls": 3, "status": "complete", "autoTurns": 2 }A second loaded location was activated during the run and did not duplicate continuation delivery.
E2E transcript (exported from the smoke run's durable session history) — timeline of
ses_f754c0b91ffevDUswBtoBDXJxC:/goalinto the goal-mode prompt:<goal_command_arguments>Create a goal for the fixture milestone. Keep it active until the automatic continuation arrives.</goal_command_arguments>create_goal(completed)→goal.status: "active", objectiveVerify native V2 goal continuation with the local fixture modelsession.execution.succeeded)Continue working toward the active session goal.+ objective + budget blocksession.execution.succeeded)update_goal(completed)→goal.status: "complete"with evidenceFinal persisted goal state:
{ "ses_f754c0b91ffevDUswBtoBDXJxC": { "status": "complete", "autoTurns": 2, "tokensUsed": 1000, "timeUsedSeconds": 4, "completionEvidence": "A native V2 execution settled and the plugin automatically sent the next goal prompt." } }Raw tool outputs from the exported transcript
create_goaloutput:{ "goal": { "sessionID": "ses_f754c0b91ffevDUswBtoBDXJxC", "status": "active", "objective": "Verify native V2 goal continuation with the local fixture model", "maxAutoTurns": 3, ... } }update_goaloutput:{ "goal": { "status": "complete", "completionEvidence": "A native V2 execution settled and the plugin automatically sent the next goal prompt.", "autoTurns": 2 }, "completion_report": "Goal achieved. Time used: 4 seconds. Token usage: 1000/200000. Evidence: A native V2 execution settled and the plugin automatically sent the next goal prompt." }Package entrypoint resolution — the packed tarball resolves through the host's own
Host.resolve(@opencode/plugin/host), confirming theexportsmap loads correctly without a root.export:{ "server": "…/node_modules/@prevalentware/opencode-goal-plugin/dist/server.js", "tui": "…/node_modules/@prevalentware/opencode-goal-plugin/src/tui.ts" }Default export keys after
Host.load(...):[ "id", "server", "setup" ](dual V1/V2 shape).Checklist
bun run testpasses (new behavior has regression coverage)bun run lintpassesbun run typecheckpassesbun run buildpasses anddist/server.jsis committed if server code changed