Repository navigation
Conversation
Introduce the v2 compact bridge (Basic-auth POST to /api/session/:id/compact, server password discovery from the child process environment, listener ownership scan with snapshot-diff fallback) and route opencode v2 inline model switching through session/set_config_option with the legacy set_model path retained as fallback for older builds.
Round-one review fixes folded in: pass the abort signal through the v2 compact POST, verify the result against the v2 messages route instead of the v1 marker GETs, prefer pid-owned listeners during port discovery, collapse the base-url/password pair into one invariant field, notify the user once when discovery fails, and add the missing EOF newline.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review mode: initial
Requirement — Pass
Supporting OpenCode v2 while retaining v1 sessions fits the existing integration and addresses the reported startup incompatibility.
Evidence
- The snapshot describes removed v2 ACP flags and reports verification against OpenCode 1.19.0 and 2.0.29.
- The merge-base launcher unconditionally supplies the v1 port and hostname options: cli/src/opencode/opencodeRemoteLauncher.ts:174.
- docs/guide/agents.md:21 lists OpenCode as a supported local and remote integration.
Approach — Pass
Version-specific spawning and authenticated REST helpers are a viable way to support both versions. Discovery failure appropriately preserves ordinary session use.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:108 uses spawn cwd and omits unsupported flags for v2.
- cli/src/opencode/opencodeRemoteLauncher.ts:268 discovers v2 REST access and reports capability loss on failure.
- cli/src/opencode/utils/opencodeCompactBridge.ts:156 implements authenticated v2 compaction.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff. Found gaps in Windows version detection, authenticated retry reporting, and asynchronous compaction handling. Review was static; PR code was not executed.
- [Major] Support Windows command shims during version detection —
cli/src/opencode/utils/opencodeBackend.ts:31 - [Major] Forward cancellation into the compaction polling request —
cli/src/opencode/utils/opencodeCompactBridge.ts:211 - [Minor] Authenticate the v2 event-stream subscription —
cli/src/opencode/opencodeRemoteLauncher.ts:309 - [Minor] Keep ACP updates suppressed until v2 compaction finishes —
cli/src/opencode/opencodeRemoteLauncher.ts:1027
Testing
Not run (automation; PR code execution is prohibited).
- At the fixed head, GitHub checks test, integration, and windows-codex-mcp completed successfully.
- The PR reports 3036 passing tests, typecheck success, and manual v1/v2 end-to-end verification; these were not independently rerun.
- Added tests do not cover Windows version-probe options, authenticated v2 SSE, cancellation during a pending verification GET, or delayed ACP updates during v2 compaction.
HAPI Bot
Spawn the version probe through a shell on Windows so .cmd shims resolve instead of silently classifying v2 as v1; thread the abort signal into the compaction polling GET so Stop can interrupt a stalled request; send the Basic-auth header on the v2 event-stream subscription so upstream retry notices actually reach the session; and hold ACP update suppression across the whole compaction POST plus polling so late session updates cannot duplicate the explicit summary.
…, and pin both
Three review rounds surfaced hardening gaps in the v2 compact path, all
fixed here as one unit:
- The messages endpoint returns entries newest first (measured), so
verification now matches the specific compaction id the POST admitted
instead of the oldest entry; an unparseable POST body reports
unverified immediately rather than guessing the newest entry.
- The verification poll is server-side work: the operation phase stays
'summarize' for the whole POST-plus-poll span, so a plain Stop waits
with the FIFO barrier intact (thinking preserved, late outcome
suppressed) instead of aborting an 'interruptible read'; leavingRemote
keeps its abort semantics. The poll is bounded (maxAttempts) so an
entry that never appears cannot hold the session forever.
- The v2 event stream targets its real surface: /api/event with the
{id, type, data} envelope (the v1 /event path answers with SPA HTML),
unwrapped so the existing session.status handling works unchanged.
Tests pin the barrier decisively: a plain Stop during a hung poll keeps
the signal live and the thinking state, and the suppressed outcome never
renders — assertions that fail on the pre-fix phase behavior.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Supporting OpenCode v2 alongside v1 addresses the reported startup incompatibility in an existing supported integration.
Evidence
- The snapshot describes removed v2 ACP flags and reports verification against OpenCode 1.19.0 and 2.0.29.
- The base launcher unconditionally selects the legacy port-based launch path: cli/src/opencode/opencodeRemoteLauncher.ts:174.
- docs/guide/agents.md:21 lists OpenCode as supporting local and remote sessions.
Approach — Pass
Version-specific spawning, authenticated REST helpers, and asynchronous result polling are viable. Discovery failure preserves ordinary session use. The remaining defects can be fixed within this approach.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:114 omits legacy flags and supplies spawn cwd for v2.
- cli/src/opencode/opencodeRemoteLauncher.ts:269 discovers REST access and reports capability loss on failure.
- cli/src/opencode/opencodeRemoteLauncher.ts:1047 suppresses ACP updates across compaction submission and polling.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff and prior findings. The follow-up addresses Windows probing, SSE authentication, signal forwarding, and suppression during polling. Three correctness issues remain. Review was static; PR code was not executed.
- [Major] Keep polling when Stop arrives before POST admission returns —
cli/src/opencode/opencodeRemoteLauncher.ts:1065 - [Major] Do not release the queue while compaction remains pending —
cli/src/opencode/utils/opencodeCompactBridge.ts:244 - [Minor] Preserve child listener results when the parent has no listener —
cli/src/opencode/utils/opencodeBackend.ts:278
Testing
Not run (automation; PR code execution is prohibited).
- No PR code, tests, builds, or scripts were executed, as required.
- At head 59eba92, integration and windows-codex-mcp succeeded; test was still in progress when checked.
- The snapshot reports 3036 passing tests, typecheck success, and manual v1/v2 verification; these were not independently rerun.
- Added coverage misses Stop before POST admission returns, pending compaction beyond the attempt cap, and mixed no-match/success results from multi-PID lsof scans.
HAPI Bot
…work and partial listener scans Three follow-up review findings on the v2 compact path: - A plain Stop landing while the compact POST was pending only requested output suppression, but the launcher treated it as a cancellation and skipped verification — releasing the FIFO barrier while the server-side compaction was still running. Suppression now defers to the terminal outcome; only a teardown abort (switch/exit) exits early. - The poll's attempt cap treated a visible pending compaction the same as no visibility, force-releasing the queue during prolonged provider retries. The cap is now a stall cap: observing the matched pending entry resets it, since visible server work legitimately holds the barrier and switch/exit remains the escape hatch. - On macOS (no ss), lsof exits non-zero when a pid owns no socket; that rejection discarded a sibling pid's successful scan via Promise.all and pushed discovery onto the racy snapshot-diff heuristic. Per-pid aggregation now treats no-match as an empty set while a missing lsof binary still falls back.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Supporting OpenCode v2 alongside v1 addresses a startup incompatibility in an existing supported integration.
Evidence
- The snapshot describes removed v2 ACP flags and reports verification against OpenCode 1.19.0 and 2.0.29.
- The merge-base launcher unconditionally uses the legacy port-based launch path: cli/src/opencode/opencodeRemoteLauncher.ts:175.
- docs/guide/agents.md:21 lists OpenCode as supporting local and remote sessions.
Approach — Pass
Version-specific spawning, authenticated REST access, and correlated compaction polling are viable. Discovery failure preserves ordinary session use. The remaining defect can be fixed within this approach.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:114 omits legacy flags and supplies spawn cwd for v2.
- cli/src/opencode/opencodeRemoteLauncher.ts:269 discovers REST access and reports capability loss on failure.
- cli/src/opencode/opencodeRemoteLauncher.ts:1094 polls using the admitted compaction ID while retaining update suppression.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff and reassessed prior findings. The follow-up addresses those reported scenarios, but an unsafe compaction recovery path remains. Review was static; PR code was not executed.
- [Major] Retain the queue barrier when admission loses its correlation ID —
cli/src/opencode/opencodeRemoteLauncher.ts:1082
Testing
Not run (automation; PR code execution is prohibited).
- No PR code, tests, builds, or scripts were executed, as required.
- At fixed head f09dcac, integration and windows-codex-mcp succeeded; test remained in progress when checked.
- The snapshot reports 3036 passing tests, successful typecheck, and manual v1/v2 verification; these were not independently rerun.
- The new listener regression test at cli/src/opencode/utils/opencodeBackend.test.ts:366 checks args[1], which is '-p', instead of the PID at args[2]. Its intended parent-no-match branch never executes; correct the index to exercise that recovery.
HAPI Bot
…work and discovery recovery Three review rounds on the v2 compact path converged on one invariant — the FIFO barrier must cover every state after a successful compact POST, because v2's POST only admits the work — plus one discovery fix: - A plain Stop landing while the POST is pending now defers to the terminal outcome instead of skipping verification; only a teardown abort (switch/exit) exits early. - The poll cap is a stall cap: observing the admitted compaction in a non-terminal state resets it, so visible server work legitimately holds the barrier while an entry that never appears times out. When several new compactions make the match ambiguous, pending progress is only claimed while at least one candidate is non-terminal. - When the POST response cannot be read, identity is recovered through a pre-POST snapshot of compaction ids (the v2 analogue of v1's marker snapshot); if even the snapshot failed, verification reports unverified after admission rather than guessing the newest entry. - On macOS lsof exits non-zero for a pid with no socket; per-pid aggregation keeps a sibling's successful scan instead of discarding it through Promise.all and falling back to the racy snapshot-diff heuristic. The regression test is decisive: the parent always fails, the child carries the only port.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Supporting OpenCode v2 addresses startup incompatibilities in an existing integration while preserving v1 sessions.
Evidence
- The snapshot describes removed v2 ACP flags and changed REST and model-switching APIs.
- docs/guide/agents.md:21 documents OpenCode local and remote support.
- The merge-base launcher unconditionally supplies legacy port and hostname options at cli/src/opencode/opencodeRemoteLauncher.ts:175.
Approach — Pass
Version-specific spawning, authenticated REST discovery, and correlated compaction polling are viable. The remaining cancellation and queue defects can be fixed within this approach.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:114 supplies spawn cwd and omits legacy flags for v2.
- cli/src/opencode/opencodeRemoteLauncher.ts:269 discovers REST access and reports capability loss on failure.
- cli/src/opencode/opencodeRemoteLauncher.ts:1053 captures a pre-request boundary for compaction identity recovery.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff and reassessed the previous finding. Identity recovery improves, but an unsafe queue-release path remains, and the new snapshot introduces a cancellation gap.
- [Major] Preserve queue safety when both identity sources fail —
cli/src/opencode/opencodeRemoteLauncher.ts:1096 - [Minor] Allow Stop to cancel the v2 pre-request snapshot —
cli/src/opencode/opencodeRemoteLauncher.ts:1047
Testing
Not run (automation; PR code execution is prohibited).
- Static review only; no PR code, tests, builds, or scripts were executed.
- At fixed head 12beb46, integration and windows-codex-mcp succeeded; test remained in progress when checked.
- The snapshot reports 3036 passing tests, successful typecheck, and manual v1/v2 verification; these claims were not independently rerun.
- The listener regression test now checks the correct PID argument. Added compaction tests do not verify queue safety after both identity sources fail or cancellation during the v2 snapshot.
HAPI Bot
… the v2 snapshot abortable Follow-up on the pre-POST snapshot: the capture now runs in the abortable 'snapshot' phase before anything is admitted, with a cancellation check before the POST — matching the v1 marker-snapshot pattern. When the snapshot itself fails there is no basis for attributing the outcome, so the compact is refused up front (failure message, no POST, no poll) instead of admitting server-side work that verification could never correlate. Plain-Stop-during-snapshot exits before admission, releasing no server-side work.
…hot abort ordering The block preamble still described the pre-e530c214 phase behavior; it now states the snapshot-to-summarize ordering. The plain-Stop-during- snapshot ordering (cancelled exit before the null-snapshot refusal) gets its own regression test — the refusal path rendering a failure for a user abort, or admitting a compaction, would both fail it.
…aps from the multi-model review Four independent reviews (DeepSeek V4.1 Flash, GLM 5.3 Flash, Nemotron 3.5 Super VL, GPT 6.1 Sol) converged on residual holes in the v2 compact lifecycle; this closes the accepted ones: - Transport-level POST failures are no longer definitive rejections: admission is uncertain (the server may have accepted before the connection died), so the outcome is a distinct transport-error kind and the launcher runs identity recovery instead of releasing the barrier onto running work. - Once the admitted compaction is observed pending, observation failures no longer accumulate toward the stall cap — confirmed server work holds the barrier until a terminal outcome or teardown. - An empty pre-POST snapshot is a valid identity basis: a fresh session with no prior compactions previously fell through to the newest-entry guess. - Unknown compaction statuses no longer claim pending progress; only pending/running reset the stall counter, so an unrecognized status cannot hold the barrier forever. - listChildPids unions children across all /proc tasks instead of stopping at the first nonempty one, and the macOS password scan iterates every child instead of only the first — both could miss the serve child in multi-child process trees. - The version probe now uses the same launch command source as the spawn, and a shared-backend test pins that flag-off with no flavor sends only session/set_model.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Supporting OpenCode v2 fixes incompatibilities in an existing integration while retaining v1 support.
Evidence
- docs/guide/agents.md:21 documents OpenCode ACP support.
- The merge-base launcher always supplies legacy ACP flags; the snapshot and upstream v2.0.29 implementation establish the changed launch and API contracts.
Approach — Pass
Version-specific spawning, authenticated REST discovery, and asynchronous compaction polling are viable. The identified defects can be fixed within this approach.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:114 selects the v2 spawn configuration.
- cli/src/opencode/opencodeRemoteLauncher.ts:271 discovers authenticated REST access and reports capability loss.
- cli/src/opencode/opencodeRemoteLauncher.ts:1056 establishes an abortable identity snapshot before admitting compaction.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff. Both previous findings are addressed; three implementation defects remain.
- [Minor] Discover descendants beyond the immediate ACP child —
cli/src/opencode/opencodeRemoteLauncher.ts:276 - [Minor] Normalize the actual v2 event envelope —
cli/src/opencode/utils/opencodeEventStream.ts:370 - [Minor] End ambiguous recovery once all candidates are terminal —
cli/src/opencode/utils/opencodeCompactBridge.ts:276
Testing
Not run (automation; PR code execution is prohibited).
- At fixed head d2cc655, test, integration, and windows-codex-mcp checks succeeded.
- Static review only; no PR code, tests, builds, or scripts were executed.
- The PR description reports 3036 passing tests, successful typecheck, and manual v1/v2 verification; these were not independently rerun.
- Missing coverage includes npm wrapper descendants, the actual v2 retry-event envelope, and ambiguous recovery transitioning from pending to all-terminal.
HAPI Bot
…ng, and bounded ambiguous recovery The bot's review of d2cc655 caught three residuals: - The npm entrypoint wrapper spawns the native binary which spawns serve — the listener owner can be a grandchild, so child discovery now walks descendants (bounded depth) on both /proc and pgrep paths, and the password reader benefits from the same recursion. - v2 session.status events carry the discriminator on the SSE envelope and the body fields directly (no v1 properties wrapper); the unwrap now re-attaches the envelope type and maps data to properties, so retry events are no longer silently rejected. - Ambiguous snapshot recovery ends the poll once ALL new candidates are terminal — sawPending must not hold the barrier forever on entries that will never change.
…s as ours Two residual holes found by the repository-access re-review (DeepSeek V4.1 Flash, GLM 5.3 Flash, Nemotron 3.5 Super VL against the actual PR head with upstream sources): - Owned-scan port discovery preferred the lowest port the process tree owns, which could predate this compaction's REST surface; it now prefers a port that appeared after the pre-spawn snapshot. - Exclude-based identity recovery could settle a server-initiated auto-compaction (reason != manual) as ours, ending the wait while the admitted manual compaction was still running; candidates with a reason field are now restricted to manual ones, and an auto-only pool counts as no visibility (stall cap applies). Also renames the flag-off AcpSdkBackend test to describe the actual opencode legacy contract (config-option-first with set_model fallback) and pins the shared-backend guarantee in a dedicated no-flavor test.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Supporting OpenCode v2 fixes launch incompatibilities in an existing integration while retaining v1 support.
Evidence
- docs/guide/agents.md:21 documents OpenCode ACP support.
- The merge-base backend supplies legacy ACP flags. Upstream OpenCode v2.0.29, commit 042d0832, confirms the standalone server and authenticated API contracts.
Approach — Pass
Version-specific spawning, authenticated server discovery, and asynchronous compaction polling are viable. The remaining defects can be fixed within this approach.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:114 selects the v2 spawn configuration.
- cli/src/opencode/opencodeRemoteLauncher.ts:271 discovers REST access and reports capability loss.
- cli/src/opencode/opencodeRemoteLauncher.ts:1054 captures an abortable identity boundary before compaction admission.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff. The retry-envelope and ambiguous-recovery findings are addressed. Two credential-discovery defects remain.
- [Minor] Traverse descendants when reading credentials on macOS —
cli/src/opencode/utils/opencodeCompactBridge.ts:363 - [Minor] Read the password from the discovered serving process —
cli/src/opencode/utils/opencodeCompactBridge.ts:345
Testing
Not run (automation; PR code execution is prohibited).
- Static review only; no PR code, tests, builds, or scripts were executed.
- At fixed head aaf2b34, integration and windows-codex-mcp checks succeeded; test remained in progress at the final check.
- The PR description reports 3036 passing tests, successful typecheck, and manual v1/v2 verification; these were not independently rerun.
- Coverage is missing for macOS npm-wrapper credential discovery and inherited passwords differing from the serving process credential.
HAPI Bot
Two credential-discovery defects from the bot review of aaf2b34: - The macOS path ran pgrep once against the acp parent, so with an npm wrapper (parent → native acp → serve) it examined the native process and missed the password entirely; it now walks the same recursive descendants the port discovery uses, one ps Eww per descendant. - A user-exported OPENCODE_PASSWORD is inherited by the native child, while serve gets the freshly generated one — accepting the first descendant carrying the variable returned the inherited credential and every /compact 401'd. Candidates now prefer the serve process (identified by its command line) over merely inherited values, on both the /proc and ps paths.
…arwin) The Round 13 verification found that the darwin serve preference ran its regex over the full ps Eww output including the embedded environment — an inherited env value containing the substring (e.g. a PRESERVE_DIR path segment) would misflag the native child as the serve process and resurrect the inherited-credential bug it was meant to fix. The command portion is now isolated from the env tokens, mirroring the linux /proc cmdline path; a linux multi-candidate preference test and a darwin env-substring test pin both paths.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Supporting OpenCode v2 restores an existing integration affected by upstream CLI and API changes while retaining v1 support.
Evidence
- docs/guide/agents.md:21 documents OpenCode ACP support; the merge-base backend supplies legacy launch flags.
- Upstream OpenCode v2.0.29, commit 042d0832, uses a standalone server with generated Basic-auth credentials.
Approach — Pass
Version-specific spawning, authenticated REST discovery, and polling asynchronous compaction results are viable. The remaining defects can be corrected within this approach.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:114 selects v2 spawning with a process working directory.
- cli/src/opencode/opencodeRemoteLauncher.ts:271 discovers REST access and reports capability loss.
- cli/src/opencode/opencodeRemoteLauncher.ts:1054 captures compaction identity before admission and preserves the queue barrier during polling.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff. Recursive credential discovery addresses the previous macOS issue, but credential selection and native compaction reporting still have defects.
- [Minor] Match the serve argument instead of a command-line substring —
cli/src/opencode/utils/opencodeCompactBridge.ts:358 - [Minor] Recognize successful native compactions without summary text —
cli/src/opencode/utils/opencodeCompactBridge.ts:720
Testing
Not run (automation; PR code execution is prohibited).
- Static review only; no PR code, tests, builds, or scripts were executed.
- At fixed head cda3cb6, integration and windows-codex-mcp checks succeeded; test remained in progress at the last observation.
- The PR description reports 3036 passing tests, successful typecheck, and manual v1/v2 verification; these were not independently rerun.
- Added tests do not cover executable paths containing serve or successful native compactions with empty summary text.
HAPI Bot
…ccess without summary text Two Minor findings from the bot review of cda3cb6: - Serve detection matched the /serve/ substring, so an executable under /home/server/ misflagged the acp child as the serve process and preferServe accepted its inherited credential. Both platforms now require the standalone serve argument with --stdio. - Upstream v2 native provider compaction (model.compaction?.type === 'native', verified against the v2 binary and 2.0 sources) completes with an empty summary and carries the context checkpoint in providerContext; the bridge reported it as a failure. Completed entries with an empty summary but a present providerContext are now success with empty text, and the launcher skips the synthetic summary message in that case.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Supporting OpenCode v2 restores an existing integration affected by upstream CLI and API changes while retaining v1 support.
Evidence
- docs/guide/agents.md:21 documents OpenCode ACP support; the merge-base backend supplies legacy launch flags.
- OpenCode v2.0.29 (042d0832f), packages/cli/src/commands/handlers/acp.ts:27, starts a standalone server with generated Basic-auth credentials.
Approach — Pass
Version-specific spawning, authenticated REST discovery, and polling asynchronous compaction outcomes are viable. The remaining defect can be fixed within this approach.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:114 selects v2 spawning with a process working directory.
- cli/src/opencode/opencodeRemoteLauncher.ts:1059 captures compaction identity before admission and holds the queue barrier through verification.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff. Both previous findings are addressed; v2 compaction failure messages still use the wrong error shape.
- [Minor] Read the v2 compaction error's top-level message —
cli/src/opencode/utils/opencodeCompactBridge.ts:734
Testing
Not run (automation; PR code execution is prohibited).
- Static review only; no PR code, tests, builds, or scripts were executed.
- At fixed head aa8c153, integration and windows-codex-mcp checks succeeded; test remained in progress at the last observation.
- The description reports 3036 passing tests, successful typecheck, and manual v1/v2 verification. Added failure-parsing coverage uses the v1 data.message shape rather than the actual v2 error shape.
HAPI Bot
…ource and close review residuals
The pre-publication reviews cross-referenced a stale upstream ref (the
April '2.0 exploration' branch tip) and concluded the v2 wire contract
was fabricated; this pins the actual release source instead — tag
v2.0.29, commit 042d0832f — whose openapi.json confirms every claim:
POST /api/session/:id/compact ({data: Session.Inbox.Compaction}),
GET /api/session/:id/message ({data: [Session.Message.Info...]} with
top-level compaction entries), /api/event SSE with {id, type, data}
envelopes, and cli/src/services/standalone.ts injecting a generated
OPENCODE_PASSWORD into the serve child (with an explicit comment that
an inherited value must not shadow the lease credential).
Residuals adopted from the four-model review:
- 5xx compact responses are admission-uncertain (server may have
created the compaction before failing) and route through identity
recovery; 4xx stays definitive.
- Exclude-recovery candidates without a reason field are an unknown
entry shape: settlement is refused (stall-capped) instead of guessed.
- The darwin command-portion split recognizes lowercase env names.
- Wire-contract comments cite the release commit and schema paths so
the measurements are re-exercisable instead of comment-only.
aa8c153 to
6dd00ac
Compare
heavygee
added
enhancement
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Supporting OpenCode v2 restores an existing integration affected by upstream CLI and API changes while retaining v1 support.
Evidence
- docs/guide/agents.md:21 documents OpenCode ACP support; the merge-base backend supplies legacy launch flags.
- OpenCode v2.0.29 packages/cli/src/commands/handlers/acp.ts starts a standalone server; packages/cli/src/services/standalone.ts generates Basic-auth credentials.
Approach — Pass
Version-specific spawning, process-scoped REST discovery, authenticated requests, and correlated compaction polling fit the existing integration and preserve queue serialization.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:114 selects the v2 launch contract and process working directory.
- cli/src/opencode/opencodeRemoteLauncher.ts:281 scopes listener discovery to the serving process when identified.
- cli/src/opencode/opencodeRemoteLauncher.ts:1071 captures compaction identity before admission and retains the queue barrier through verification.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff. No actionable correctness defects found. The previous v2 error-message finding is fixed with regression coverage. Runtime behavior was not independently exercised.
No reportable code issues found.
Testing
Not run (automation; PR code execution is prohibited).
- At head 6dd00ac, test, integration, and windows-codex-mcp checks succeeded.
- Static review only; no PR code, tests, builds, or scripts were executed.
- The description reports 3036 passing tests, successful typecheck, and manual v1/v2 verification; these were not independently reproduced.
- The new findOpencodeServePid implementation lacks direct regression coverage; launcher tests mock its result.
- git diff --check reports trailing whitespace in .review-task-botcond2.md:5.
HAPI Bot
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Supporting OpenCode v2 restores an existing documented integration affected by upstream CLI and API changes while retaining v1 support.
Evidence
- docs/guide/agents.md:21 documents OpenCode ACP support; the merge-base backend supplies legacy --cwd/--port/--hostname flags.
- OpenCode v2.0.29 packages/cli/src/services/standalone.ts launches a separate server with generated Basic-auth credentials.
Approach — Pass
Version-specific spawning, process-scoped API discovery, authenticated requests, and correlated compaction polling address the upstream changes within the existing CLI architecture.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:114 selects the v2 launch contract and spawn working directory.
- cli/src/opencode/opencodeRemoteLauncher.ts:281 scopes listener discovery to the identified serving process.
- cli/src/opencode/opencodeRemoteLauncher.ts:1071 captures compaction identity before admission and retains queue serialization through verification.
- cli/src/opencode/utils/opencodeCompactBridge.ts:757 accepts native.messages consistently with OpenCode v2.1.2 packages/core/src/session/history.ts:28.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff and relevant surrounding code. No actionable correctness defects found. Runtime behavior was not independently exercised.
No reportable code issues found.
Testing
Not run (automation; PR code execution is prohibited).
- At head 188863f, integration succeeded; test and windows-codex-mcp remained in progress when checked.
- No PR code, tests, builds, or scripts were executed, as required.
- The description reports 3036 passing tests, successful typecheck, and manual v1/v2 verification; these were not independently reproduced.
- New regression tests cover v2.1 native compaction success and rejection of empty native metadata. findOpencodeServePid still lacks direct regression coverage; launcher tests mock its result.
- git diff --check reports trailing whitespace in .review-task-botcond2.md:5.
HAPI Bot
|
Hello, I tested your PR it works great. There is one remaining problem with opencode 2, when an agent spawn a background task, the output of the background task is not displayed in HAPI, leading to the model thinking it presented an answer but me thinking it didn't. |
The process tree can own listeners besides the v2 REST surface (configured MCP servers bind their own ports), so picking the lowest fresh port across the whole descendant tree could select an unrelated service and pair it with the serve credential. findOpencodeServePid identifies the serve process by its command line (standalone serve argument + --stdio) and the owned scan is scoped to that pid; without a match the tree-wide scan remains the fallback.
OpenCode 2.1 dropped the providerContext field on completed compaction entries and stores a provider-native compaction's replacement window in native.messages instead. A native compaction still completes with an empty summary, so on 2.1 it was reported as "OpenCode returned an empty compaction summary." Accept the 2.1 shape with the same test upstream uses to recognize a native compaction (native.messages !== undefined); the 2.0.x providerContext path is unchanged.
188863f to
be3fb02
Compare
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Restoring the documented OpenCode integration on v2 while retaining v1 support is justified by upstream CLI and API changes.
Evidence
- docs/guide/agents.md:21 documents OpenCode ACP support; the merge-base backend passes legacy --cwd/--port/--hostname flags.
- OpenCode v2.0.29 packages/cli/src/services/standalone.ts:17-54 confirms the separate server process and generated Basic-auth credentials.
Approach — Pass
Version-specific spawning, authenticated API discovery, and correlated compaction polling fit the existing CLI architecture and preserve queue serialization.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:114 selects the v2 launch contract and spawn working directory.
- cli/src/opencode/opencodeRemoteLauncher.ts:281 scopes listener discovery to the serving process.
- cli/src/opencode/opencodeRemoteLauncher.ts:1071 captures compaction identity before admission and retains serialization through verification.
- OpenCode v2.1.2 packages/core/src/session/history.ts:28 confirms the native.messages completion predicate.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff. One v2 output-delivery gap remains; runtime behavior was not independently exercised.
- [Major] Handle v2 background-child output after the parent turn ends —
cli/src/opencode/utils/opencodeBackend.ts:118
Testing
Not run (automation; PR code execution is prohibited).
- At fixed head be3fb02, integration and windows-codex-mcp succeeded; test remained in progress.
- No PR code, tests, builds, or scripts were executed, as required.
- The description reports 3036 passing tests, successful typecheck, and manual v1/v2 verification; these were not independently reproduced.
- Added tests cover compaction cancellation, identity recovery, authentication, and native completion shapes, but omit background-child output after parent completion.
- The full merge-base diff passes git diff --check.
HAPI Bot
OpenCode 2 stops sending ordinary session/update notifications for a subagent that keeps running in the background after the parent turn has ended; it only reports that child's output through the opencode/session/child_update extension, and only to clients that advertise the opencode/child-session-updates capability. HAPI did neither, so a background subagent's work and answer never reached the timeline. Advertise the capability for v2 sessions and route child updates through the existing session/update path. OpenCode already projects them the way it would send them as parent updates, so foreground subagents render as before. Updates that arrive with no prompt in flight are flushed once the stream goes quiet instead of waiting for the next prompt.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Restoring the documented OpenCode integration on v2 while retaining v1 support addresses verified upstream CLI changes.
Evidence
- docs/guide/agents.md:21 documents OpenCode ACP support.
- The merge-base backend supplies --cwd/--port/--hostname; OpenCode v2.0.29 packages/cli/src/commands/commands.ts:89-97 defines acp without those flags.
Approach — Pass
Version-specific spawning, authenticated REST discovery, correlated compaction polling, and child-update subscription fit the existing integration. The remaining defects can be corrected within this approach.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:114 selects the v2 launch contract and spawn working directory.
- cli/src/opencode/opencodeRemoteLauncher.ts:281 scopes discovery to the serving process; :1071 captures compaction identity before admission.
- OpenCode v2.0.29 packages/cli/src/services/standalone.ts:17-54 confirms the separate server process and generated Basic-auth credentials.
- cli/src/agent/backends/acp/AcpSdkBackend.ts:210 advertises the upstream child-session capability.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff. The previous background-output omission is partially addressed, but concurrent streams can merge and compaction can still discard child answers.
- [Major] Keep child text streams separate from the foreground handler —
cli/src/agent/backends/acp/AcpSdkBackend.ts:990 - [Major] Preserve background child output during compaction —
cli/src/opencode/opencodeRemoteLauncher.ts:1098
Testing
Not run (automation; PR code execution is prohibited).
- At fixed head ca1d787, integration and windows-codex-mcp succeeded; test remained in progress.
- No PR code, tests, builds, or scripts were executed, as required.
- The description reports 3036 passing tests, successful typecheck, and manual v1/v2 verification; these were not independently reproduced.
- Added tests cover post-parent child delivery, but omit concurrent stream isolation and child delivery during compaction.
- The full merge-base diff passes git diff --check.
HAPI Bot
Background subagent updates no longer route through the parent's prompt-scoped messageHandler. Each child session gets its own handler and quiet timer, so child text cannot merge into the parent's (or another child's) text buffer, survives the /compact suppression window, and keeps flowing after the parent turn ends. The root idle-drain machinery added for the old root-path routing is removed.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Restoring OpenCode v2 remote sessions while retaining v1 support addresses verified upstream changes to a documented integration.
Evidence
- docs/guide/agents.md:21 documents OpenCode ACP support.
- OpenCode v2.0.29 packages/cli/src/commands/commands.ts:89 defines acp without the --cwd/--port/--hostname flags used by the merge-base backend.
- PR comment 6318529742 reports missing background-child output on v2.
Approach — Pass
Version-specific launching, authenticated REST discovery, correlated compaction polling, and separate child handlers fit the existing integration.
Evidence
- cli/src/opencode/utils/opencodeBackend.ts:114 selects the v2 launch contract and spawn working directory.
- cli/src/opencode/opencodeRemoteLauncher.ts:281 scopes discovery to the serving process; :1071 captures compaction identity before admission.
- cli/src/agent/backends/acp/AcpSdkBackend.ts:1011 routes child updates through independent handlers, outside root compaction suppression.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff. Both previous findings are addressed, with regression coverage. No actionable defects found. Runtime behavior was assessed statically; PR code was not executed.
No reportable code issues found.
Testing
Not run (automation; PR code execution is prohibited).
- At fixed head 8035b20, test, integration, and windows-codex-mcp checks succeeded.
- Added tests cover interleaved parent/child and child/child streams, child delivery during compaction, terminal flushing, and disconnect.
- The full merge-base diff passes git diff --check.
- No PR tests, builds, or scripts were executed. The description's manual v1/v2 verification was not independently reproduced.
HAPI Bot
This branch has not been deployed
3 participants
OpenCode 2.0 changed the surface the HAPI opencode flavor depends on: the
acpsubcommand no longer accepts--cwd,--portor--hostname, its internal HTTP API moved behind Basic auth on a random port with a new/api/session/:id/compactroute, and the ACPsession/set_modelmethod was removed in favor ofsession/set_config_option. On v2, opencode sessions therefore failed to start at all.This makes the flavor work on both v1 and v2 without changing v1 behavior. On startup the launcher runs
opencode --versiononce and caches the result; any failure falls back to the v1 path, so production v1 sessions are unaffected. For v2, the backend spawnsopencode acpwith the working directory passed as the spawn cwd, discovers the internal server by scanning loopback listeners owned by the acp process and its children (falling back to a listener-snapshot diff when ownership cannot be attributed), reads the server password from the child process environment, and drives compaction throughPOST /api/session/:id/compactwith Basic auth — polling the session messages endpoint until the compaction entry reports its outcome, since v2 returns from the compact request before the summary is written. Inline model switching goes throughsession/set_config_option, with the existing method-not-found fallback retained for older builds.When v2 discovery fails, the session still works — the user gets a one-time notice that retry status and
/compactare unavailable, matching the existing capability-loss messages.No schema, hub or web changes; everything is scoped to
cli/src/opencodeand the shared ACP backend's new optional options, which default to the previous behavior for other flavors (Gemini included).Tests:
cd cli && bun run test && bun run typecheck(3036 passing). Verified end to end with the isolated stack (isolatedHAPI_HOME, realopencode acpsubprocess): v1 regression across session start, first turn,/compactand inline model switch; v2 session start, first turn,/compactcompleting with the rendered summary, inline model switch, and the discovery-failure notice. Both binaries measured: opencode 1.19.0 (v1) and 2.0.29 (v2).