feat(cli): bridge OpenCode native compaction via internal REST API (#1252)

* feat(cli): allocate a loopback port for the OpenCode ACP subprocess

opencode does not announce which port it bound when launched with
--port 0 (verified: not present in stdout/stderr even at DEBUG level),
so pick a free loopback port ourselves and pass it explicitly via
--port/--hostname. This makes the ACP subprocess's internal HTTP API
reachable at a known baseUrl for follow-up work (native /compact
bridging).

* feat(cli): add REST bridge for OpenCode native session compaction

opencode's ACP method table has no session/compact RPC, but the
opencode acp subprocess also runs an internal HTTP API. That API's
POST /api/session/:id/compact route is an unimplemented v2 stub
(always 503); the route that actually performs native AI compaction
is the legacy POST /session/:id/summarize, which requires providerID
and modelID in its payload. This adds a small client for that route
plus a helper to split ACP's combined "provider/model" wire id.

Not wired into the slash-command flow yet.

* feat(cli): trigger native OpenCode compaction from /compact

Wires the REST bridge into the OpenCode slash-command flow so /compact
performs real context compaction instead of returning a "not yet
supported" message.

- slashCommands.ts: /compact now resolves to its own `kind: 'compact'`
  (the synchronous 'handled' shape can't carry an async REST round
  trip). /clear is unchanged.
- opencodeRemoteLauncher.ts: registers a compact trigger callback once
  the ACP backend + internal HTTP baseUrl are ready, reading the
  session's current provider/model on every call so it reflects
  inline model switches. A `runExclusive` promise-chain mutex
  serializes the compact trigger against `backend.prompt()` so a
  still-in-flight prompt and a `/compact` sent moments later can't
  race against the same OpenCode session concurrently, in either
  arrival order.
- runOpencode.ts: on /compact, emits "Compaction started" as a session
  event (the same `sendSessionEvent({ type: 'message', ... })` status-
  line channel Claude/Codex already use for compaction and other
  transient state, rather than a chat message), awaits the bridge with
  no artificial timeout (compaction on a reasoning model can take
  90s+), then emits "Compaction completed" or "Compaction failed:
  <reason>" the same way. Falls back to the previous not-yet-supported
  chat message when no trigger is registered (local mode has no ACP
  backend, so this is unreachable there today). If the user cancels
  the message while compaction is queued or in flight, the eventual
  result is suppressed instead of surfacing a stray status message
  for an action the user already considered cancelled (aborting the
  in-flight HTTP call itself is left as follow-up scope).
- help text now notes /compact is remote-sessions only, since local
  mode has no ACP backend to bridge through.

Also fixes a runOpencode.test.ts fixture gap: the mocked
OpencodeSession was missing onThinkingChange, so any exception in that
call path was silently swallowed by the handler's outer catch instead
of failing the test.

* feat(cli): show compaction summary as a reasoning block

After a native OpenCode compaction succeeds, OpenCode's session
history gains a `role: "user"` marker message (a `{type:"compaction"}`
part with no text) followed by a normal assistant message holding the
actual generated summary in a `text` part. Left alone, neither is
visible in HAPI — the bridge only checked success/failure and never
looked at the resulting messages.

- opencodeCompactBridge.ts: `fetchCompactionSummary()` does one more
  GET against the session's message list after a successful compact,
  finds the most recent `type:"compaction"` marker, and extracts the
  following assistant message's text — preferring a match via the
  marker's `parentID` (order-safe) and falling back to simple array
  adjacency, both gated on `role === 'assistant'` so an unrelated
  same-shaped sibling can't be silently misattributed as the summary.
  Concatenates every `text` part in case a summary spans more than
  one. Never throws: any unexpected shape or request failure just
  yields "not found" so the caller can skip showing anything.
- opencodeRemoteLauncher.ts / runOpencode.ts: on a successful compact,
  forward the extracted summary as a `{ type: 'reasoning', text, id }`
  AgentMessage through the same `handleAgentMessage()` /
  `convertAgentMessage()` path OpenCode's own live thought-chunk
  streaming already uses, so it renders in the existing collapsible
  "Reasoning" block — no new UI component or schema field. The
  `role:"user"` marker is never constructed or forwarded at all, so
  there's nothing to filter (unlike Claude's compact summary, which
  is hidden after the fact via an `isCompactSummary` flag).

* fix(cli): disable Bun's hardcoded fetch timeout for OpenCode compaction

Isolated E2E against a real OpenCode session showed compaction always
failing with "Compaction failed: The operation timed out." after
~250s, even though session/prompt-style unlimited waits were intended.
Root cause: Bun's global fetch() hardcodes an internal ~5 minute
timeout that fires independently of any AbortSignal (or its absence) —
see oven-sh/bun#16682. The only documented workaround is passing the
non-standard `timeout: false` fetch option that Bun itself recognizes.

Cast through a local `BunFetchInit = RequestInit & { timeout?: false }`
type alias rather than `Record<string, unknown>`, so the option stays
structurally checked against the rest of the fetch call's shape.

* fix(cli): serialize /compact through the message queue instead of a mutex

HAPI Bot flagged two Major correctness issues on the PR:

- `/compact` bypassed `MessageQueue2` and entered a `runExclusive` mutex
  directly from the user-message handler. If prompt A was in flight and
  prompt B already queued behind it, `/compact` sent afterward could
  still reach the mutex before B did, running compaction ahead of an
  earlier-queued user prompt.
- `compactTriggerRef` (a closure capturing the remote backend) was
  never cleared on a remote→local handoff, so `/compact` typed after
  switching to local mode could call a stale closure targeting an
  already-disposed backend instead of the intended local-mode fallback.

Removes the mutex entirely and routes `/compact` through the same
`messageQueue` regular prompts use (a new `operation?: 'compact'` field
on `OpencodeMode`, dequeued by the launcher's single sequential
consumer loop, which branches to a new `runCompactOperation()` instead
of `backend.prompt()`). Ordering is now enforced by construction (one
queue item in flight at a time) rather than a second bolted-on lock.

Replaces the closure-based `onCompactTriggerReady` callback with a
boolean-flag `onCompactAvailabilityChange`, reset to `false` on every
entry into local mode (cold-start and handoff alike) before the remote
backend is even disposed, so there is no window where the flag says
available but the backend underneath it is gone.

* fix(cli): let /compact's cancel ack fire at dequeue time, not on queue

A follow-up HAPI Bot review caught a Major issue this PR's queue-based
/compact serialization (previous commit) left open: runOpencode.ts
still called session.emitMessagesConsumed manually and synchronously
the instant /compact was pushed onto the queue -- a leftover from
before /compact was routed through the queue at all. That beat
MessageQueue2's automatic dequeue-time ack (onBatchConsumed, wired in
sessionBase.ts, already used by every regular prompt) to the hub, so a
/compact sitting behind other queued prompts was marked "invoked"
immediately and could never actually be cancelled from the UI.

Removes the manual ack; the queue's own dequeue-time ack now covers
/compact exactly like any other queued operation. Adds a deterministic
test for the reported scenario: prompt A generating, /compact queued
behind it, cancelled before A finishes -- the compact REST bridge must
never be called.

* fix(cli): suppress live ACP updates while a compact REST call runs

Found via live use: OpenCode keeps streaming session/update
notifications (thought chunks, etc.) over the ACP transport while
/compact's REST call runs outside prompt(), and
AcpSdkBackend.handleSessionUpdate forwarded them unconditionally to
whatever messageHandler was still installed from the last real prompt
turn -- rendering the compaction summary a second time as a plain
assistant message, alongside the explicit Reasoning block this PR
already sends for the same content.

Adds a narrow, opt-in AcpSdkBackend.suppressUpdatesDuring() (shared
with Gemini, but a pure addition with no behavior change for existing
prompt() callers) that temporarily swaps out the message handler for
the duration of an async callback, restoring it afterward. Wraps the
compact bridge's REST call with it so the live stream produces no
output during that window -- the Reasoning block built from the
explicit GET response becomes the only place the summary appears.

* fix(cli): let Stop/switch-to-local interrupt an in-flight /compact REST call

triggerOpencodeCompact's HTTP request is intentionally unbounded (a
real compaction can take minutes), but the launcher awaited it with no
way to interrupt it once dequeued and running. handleAbort() only
cancels the ACP prompt() turn and resets the queue -- neither touches
this raw HTTP call -- so Stop (and switch-to-local, which routes
through the same handler) stayed blocked until the request settled on
its own: a stale "Compaction completed/failed" could still surface
afterward, queued prompts behind it were delayed, and remote->local
handoff couldn't proceed.

Add a per-call AbortController (compactAbortController) that
handleAbort() aborts before cancelling the prompt, and thread it
through triggerOpencodeCompact's new optional `signal` (kept separate
from the existing timeout:false Bun workaround, which stays
unconditional). An aborted call now folds into the same isCancelled()
check that already suppresses a stale result for the existing
queue-cancel race, so either kind of interruption behaves the same
way.

* fix(cli): structurally close remaining /compact abort/lifecycle races

Two more narrow races surfaced in review, both symptoms of ad hoc
per-site fixes rather than a shared mechanism:

1. The signal threaded through triggerOpencodeCompact (the POST) did
   not also reach fetchCompactionSummary (the GET runCompactOperation
   makes right after it), so Stop/switch-to-local could still block on
   that call alone. Introduce OpencodeCompactCallOpts with `signal`
   required (not optional) so every HTTP step opencodeCompactBridge.ts
   makes on behalf of one /compact operation is forced by the compiler
   to accept it, not left to remembering to wire it in per call site.

2. /compact availability (runOpencode.ts's compactSupported flag) was
   only reset to false on the *next* local-mode entry (loop.ts's
   runLocal callback), leaving a window between "switch/exit was
   requested" and "local mode actually started" where a /compact
   arriving mid-transition could still queue, and -- since local mode
   hands straight back to remote when it finds a non-empty queue --
   run anyway despite the user having already left remote mode. Add an
   onLeavingRemote() hook to RemoteLauncherBase (no-op default, so the
   other six flavors built on it are unaffected) called synchronously
   as the very first action of requestExit() -- closing the race at
   its source -- and again unconditionally in start()'s finally block
   as a backstop for exit paths that never call requestExit() at all
   (an exception thrown from runMainLoop, for instance).
   OpencodeRemoteLauncher overrides it to flip availability false;
   loop.ts's local-entry reset is removed as redundant now that
   leaving remote is what's authoritative.

* fix(cli): create compactAbortController before the inline model/effort switch

A hostile-review whole-feature sweep of the /compact abort/lifecycle
surface (round 5) found that the dequeue loop applied the per-batch
inline model/effort switch (real async ACP round-trips that yield to
the event loop) *before* branching into runCompactOperation(), which
is where compactAbortController used to get created. An abort firing
during that switch hit a still-null controller (a no-op), and by the
time the switch resolved and runCompactOperation() created a fresh
one, the abort was forgotten -- the compact's unbounded REST call
would then run to completion despite the user having already pressed
Stop/switch/exit.

Move controller creation to the top of the loop iteration, as soon as
the batch is known to be a compact operation, and pass it into
runCompactOperation() rather than having that function create its
own. Also: soften an overstated doc comment about what the required
`signal` field actually guarantees, and add a regression test locking
in that two sequential compact operations each get an independent
controller (no leak or cross-clearing).

* fix(cli): drain quietly before restoring the handler in suppressUpdatesDuring

A 5th PR-review round found that suppressUpdatesDuring restored the
suppressed messageHandler the instant its callback settled, but
aborting the client-side HTTP call (e.g. OpenCode's compact bridge
aborting via compactAbortController) does not necessarily stop the
agent from continuing that operation server-side -- session/update is
a separate notification channel from the HTTP request's lifecycle.
A late straggler notification from a still-running server-side
operation could leak straight into the restored handler (or into a
new one prompt() installs right after).

Reuse the same quiet-drain prompt() already runs before installing a
new handler for the next turn (waitForSessionUpdateQuiet with the
PRE_PROMPT_* constants) -- same class of race, same validated
mechanism, just on the way back in instead of the way out. No new
state or Stop-vs-switch branching: the wait is internal to
suppressUpdatesDuring and the handler stays suppressed throughout it.

* fix(cli): queue /compact during remote-mode initialization instead of rejecting it

compactSupported alone conflated two different situations: a
genuinely local-mode session (compact fundamentally can't run) versus
a session that's already in remote mode but hasn't finished ACP
initialize + session load/new yet (onCompactAvailabilityChange(true)
hasn't fired yet, but will shortly). A regular prompt sent in that
same startup window queues normally and just waits its turn; /compact
sent in the identical window instead got an immediate
not-yet-supported reply.

Check sessionWrapperRef.current?.mode (the actual OpencodeSession
instance's mode, synced synchronously by onModeChange before either
launcher starts) alongside compactSupported: only genuinely local mode
now gets the not-yet-supported reply. A session already in remote mode
but still initializing queues /compact exactly like a prompt and lets
it settle into its real FIFO position once the launcher is ready.

* fix(cli): don't let the remote-init /compact queuing fix reopen the teardown race

A hostile-review whole-feature sweep found that gating /compact
queuing on sessionWrapperRef.current?.mode alone (26344118) fixed the
startup window but silently reopened the exact race
OpencodeRemoteLauncher's onLeavingRemote() exists to close: mode stays
'remote' for the entire teardown window too (it only flips back to
'local' once runMainLoop() fully unwinds), so a /compact arriving
after switch/exit was requested -- while compactSupported has already
gone false -- queued anyway instead of getting rejected, and could
still run once local mode bounced back to remote to drain a non-empty
queue.

Add compactTeardownInProgress, true from the moment
onCompactAvailabilityChange(false) fires (which -- since the old
reset-on-local-entry was removed -- only ever means "leaving remote",
never "not ready yet") until the session next re-enters remote mode.
Wrap the onModeChange callback opencodeLoop already receives to reset
it back to false on that re-entry. The existing "stops queuing /compact
once availability is reset to false" test didn't catch this because
its mock never set mode to 'remote' during the simulated teardown,
unlike real production timing -- fixed to match.

* fix(cli): keep the dequeue loop blocked on a compact until it really finishes on plain Stop

A 6th PR-review round rejected the quiet-drain mitigation from the
previous round (bounded at ~1.2s) and asked for the originally
proposed fix instead: quiet-drain cannot guarantee a multi-minute
server-side compaction has actually finished, since session/update is
a separate notification channel from the aborted HTTP request's
lifecycle. Unconditionally aborting compactAbortController on any
abort (the previous fix) let the dequeue loop move on to the next
queued prompt while the agent could still be compacting the same
OpenCode session server-side -- breaking the core invariant this
feature's whole queue-based redesign depends on: compact and a prompt
must never touch the same session concurrently.

handleAbort() now takes a leavingRemote parameter. Plain Stop
(leavingRemote=false, the default) only sets compactResultSuppressed
-- the eventual result gets hidden, but compactAbortController is left
alone, so runCompactOperation()'s own awaits keep blocking the dequeue
loop until the real HTTP response arrives, i.e. until the server
actually finishes. Switch-to-local/exit (leavingRemote=true) still
abort the controller for real, since cleanup() disconnects the whole
ACP subprocess right after regardless -- there's no shared session left
to protect there, and the responsiveness fixed in an earlier round
still matters for that path.

The quiet-drain from the previous round (AcpSdkBackend.suppressUpdatesDuring)
is left in place -- it still helps for a compaction that finishes
quickly and for trailing session/update stragglers right after a real
completion, it's just no longer the thing plain Stop relies on for
correctness.

* fix(cli): close 3 gaps a hostile-review pass expected the next bot round to flag

Pre-emptively addresses findings a hostile-review final pass on
65729bb7 judged likely for the next external bot round, since a
communication round-trip costs more than fixing them now:

1. No dedicated test existed for the Stop/switch-to-local RPC-overlap
   message-ordering fix (handleAbort() re-reading compactAbortController's
   abort state instead of a stale snapshot). The test harness has no way
   to observe MessageBuffer/Ink content, so the decision logic that
   picks the status message and whether to clear `thinking` is extracted
   into a pure, exported selectAbortStatusMessage() function and unit
   tested directly against all four (hasCompactInFlight, leavingRemote,
   compactAborted) combinations, including the exact overlap case.

2. No test verified compactResultSuppressed doesn't leak between two
   sequential compact operations. Added a regression test: a Stop-suppressed
   compact #1 finishing for real must not silence a normally-completed
   compact #2 queued after it.

3. The "Stop requested — waiting..." status message didn't tell the user
   how to actually leave the session (switch-to-local/exit) while a
   compaction is deliberately left running. Appended that guidance.

* fix(cli): skip a compact cancelled before its request ever went out

A plain Stop landing during the inline model/effort switch that
precedes a compact batch only set compactResultSuppressed; it never
stopped the dequeue loop from calling runCompactOperation()
unconditionally once that switch resolved, so a cancelled-before-start
compact would still fire a brand new REST request and block the loop
for however long that call takes.

Skip starting the operation when compactResultSuppressed is set and
the controller was never actually aborted (plain Stop leaves the
signal alone by design). Deliberately excludes the
compactAbortController.signal.aborted case (switch/exit) and
isLocalIdCancelled: both must keep falling through to
runCompactOperation() as before, per Round 5's and the
isLocalIdCancelled suite's existing coverage.

* fix(cli): also skip a compact cancelled-before-start via isLocalIdCancelled

Round 7's pre-start skip check only covered compactResultSuppressed
(plain Stop), deliberately leaving isLocalIdCancelled out so as not
to disturb the "Compaction started is never suppressed" tests that
predated that check. But isLocalIdCancelled's backing Set can only
ever be populated during the brief ack-vs-hub-DB-write race before a
queued item's REST call is sent, never while it's actually running
(see runOpencode.ts's cancelledBeforeEnqueue doc comment) — so a true
result here unconditionally means the same "cancelled before it ever
went out" situation Round 7 already handles for plain Stop, and
deserves the same treatment: skip starting the operation instead of
sending "Compaction started" for a request already known to be
discarded.

Updates the two tests that had encoded the old "started is never
suppressed" behavior for this specific signal to their corrected
expectation, and removes a no-longer-consumed mockImplementationOnce
that would otherwise have leaked its failure response into the next
test that actually calls the REST bridge.

* fix(cli): don't resurrect /compact availability after a startup-time switch/exit

onCompactAvailabilityChange(true) fired unconditionally right after
newSession/loadSession resolved, with no way to know a terminal
switch-to-local/exit had already run during that pending ACP round
trip (RemoteLauncherBase.requestExit() sets shouldExit synchronously
before awaiting its handler, so the flag is already accurate at that
point). runOpencode.ts's compactSupported/compactTeardownInProgress
gate treats compactSupported flipping true as reason enough to ignore
compactTeardownInProgress entirely, so this belated true could let a
/compact arriving right after slip into the queue mid-teardown.

Guard the call with the same shouldExit flag requestExit() already
set. The race can only be reached via the terminal UI's onExit/
onSwitchToLocal callbacks (wired up before runMainLoop starts, well
before the RPC 'abort'/'switch' handlers exist), so the regression
test forces isTTY and captures ink's render() props to invoke them
directly.

* fix(cli): clear the hub's queued-thinking grace when a compact is skipped pre-start

The cancelledBeforeStart skip path never calls session.onThinkingChange(true)
(the whole point of skipping), but also never told the hub the queued
item was done. The hub's 15s queued-thinking grace (markMessageQueued,
sessionCache.ts) keeps thinking pinned true regardless of keepalives
until a messages-consumed ack with clearQueuedThinkingGrace arrives,
so the web UI spinner could sit stuck for the full grace window.

Applies the same pattern already used by runOpencode.ts's synchronous
slash.kind === 'handled' path (e.g. /model): an emitMessagesConsumed
ack with clearQueuedThinkingGrace, then an immediate thinking=false
keepalive. This is additive to, not a replacement for, the queue's own
unflagged onBatchConsumed ack — a second ack for an already-invoked
localId is a no-op on the hub's first-write-wins protocol, and
clearQueuedThinkingGrace is keyed by session, not localId, so both are
idempotent.
This commit is contained in:
Junmo Kim
2026-08-01 17:20:07 +08:00
committed by GitHub
parent 084d3462cf
commit 08fbea5311
16 changed files with 3323 additions and 36 deletions
@@ -1139,4 +1139,135 @@ describe('AcpSdkBackend', () => {
expect(registered.get('cursor/ask_question')).toBe(handler);
});
it('suppressUpdatesDuring drops session/update notifications that would otherwise leak into the previous turn\'s onUpdate, then restores normal forwarding', async () => {
// Reproduces the real /compact duplicate-summary bug: OpenCode keeps
// streaming session/update notifications (over the same ACP
// transport) while a raw-HTTP /compact call is in flight outside
// prompt(), and handleSessionUpdate forwards them unconditionally to
// whatever messageHandler is still installed from the last prompt()
// turn — rendering the same content a second time alongside the
// compact bridge's own explicit summary message.
//
// Fast quiet-drain timing so this test doesn't pay the real
// (production) 200ms/1200ms PRE_PROMPT_* delay suppressUpdatesDuring
// now waits through before restoring the handler.
backendStatics.PRE_PROMPT_UPDATE_QUIET_PERIOD_MS = 5;
backendStatics.PRE_PROMPT_UPDATE_DRAIN_TIMEOUT_MS = 50;
const backend = new AcpSdkBackend({ command: 'opencode' });
const backendInternal = backend as unknown as {
transport: {
sendRequest: (...args: unknown[]) => Promise<unknown>;
close: () => Promise<void>;
} | null;
handleSessionUpdate: (params: unknown) => void;
messageHandler: unknown;
};
backendInternal.transport = {
sendRequest: async () => ({ stopReason: 'end_turn' }),
close: async () => {}
};
const turn1: AgentMessage[] = [];
await backend.prompt('session-1', [{ type: 'text', text: 'hi' }], (m) => turn1.push(m));
const emitPlanUpdate = () => backendInternal.handleSessionUpdate({
sessionId: 'session-1',
update: {
sessionUpdate: ACP_SESSION_UPDATE_TYPES.plan,
entries: [{ content: 'leaked plan step', priority: 'medium', status: 'pending' }]
}
});
const handlerBeforeSuppression = backendInternal.messageHandler;
expect(handlerBeforeSuppression).not.toBeNull();
let handlerDuringSuppression: unknown = 'not-checked';
const result = await backend.suppressUpdatesDuring(async () => {
handlerDuringSuppression = backendInternal.messageHandler;
emitPlanUpdate();
return 'compact result';
});
expect(result).toBe('compact result');
expect(handlerDuringSuppression).toBeNull();
expect(turn1.some((m) => m.type === 'plan')).toBe(false);
// The previous turn's handler must be back in place afterward so
// ordinary straggler-forwarding (covered elsewhere) is unaffected.
expect(backendInternal.messageHandler).toBe(handlerBeforeSuppression);
emitPlanUpdate();
expect(turn1.some((m) => m.type === 'plan')).toBe(true);
});
it('waits for a quiet period (reusing the same drain prompt() uses before swapping handlers) before restoring the handler after suppressUpdatesDuring, so a late server-side straggler from an already-aborted operation cannot leak', async () => {
// Reproduces a hostile-review finding: aborting the client-side HTTP
// call (e.g. compactAbortController) does not mean the OpenCode
// server actually stopped the operation — session/update is a
// separate notification channel from that HTTP request's lifecycle.
// If suppressUpdatesDuring restored the handler the instant `fn`
// resolved, a straggler notification arriving moments later (while
// the server is still winding the operation down) would leak
// straight into the restored handler.
backendStatics.PRE_PROMPT_UPDATE_QUIET_PERIOD_MS = 30;
backendStatics.PRE_PROMPT_UPDATE_DRAIN_TIMEOUT_MS = 300;
const backend = new AcpSdkBackend({ command: 'opencode' });
const backendInternal = backend as unknown as {
transport: {
sendRequest: (...args: unknown[]) => Promise<unknown>;
close: () => Promise<void>;
} | null;
handleSessionUpdate: (params: unknown) => void;
messageHandler: unknown;
};
backendInternal.transport = {
sendRequest: async () => ({ stopReason: 'end_turn' }),
close: async () => {}
};
const turn1: AgentMessage[] = [];
await backend.prompt('session-1', [{ type: 'text', text: 'hi' }], (m) => turn1.push(m));
const emitPlanUpdate = () => backendInternal.handleSessionUpdate({
sessionId: 'session-1',
update: {
sessionUpdate: ACP_SESSION_UPDATE_TYPES.plan,
entries: [{ content: 'late server-side straggler', priority: 'medium', status: 'pending' }]
}
});
const handlerBeforeSuppression = backendInternal.messageHandler;
const suppressPromise = backend.suppressUpdatesDuring(async () => {
// Client gives up almost immediately (mirrors compactAbortController
// firing), but the server keeps streaming for a little longer —
// one update right away, one more 15ms later.
emitPlanUpdate();
setTimeout(emitPlanUpdate, 15);
return 'aborted-early';
});
// Sampled while suppressUpdatesDuring's own returned promise is
// still pending (fn already resolved, but the quiet-drain in its
// `finally` has not) — this is what actually proves restoration is
// *deferred*, not merely eventually correct.
await sleep(20);
const handlerDuringDrainWindow = backendInternal.messageHandler;
const result = await suppressPromise;
expect(result).toBe('aborted-early');
expect(handlerDuringDrainWindow).toBeNull();
// Neither the immediate update nor the +15ms straggler leaked —
// messageHandler was null (suppressed) for both.
expect(turn1.some((m) => m.type === 'plan')).toBe(false);
expect(backendInternal.messageHandler).toBe(handlerBeforeSuppression);
// Normal forwarding resumes once actually restored.
emitPlanUpdate();
expect(turn1.some((m) => m.type === 'plan')).toBe(true);
});
});
@@ -604,6 +604,68 @@ export class AcpSdkBackend implements AgentBackend {
this.stderrErrorHandler = handler;
}
/**
* Runs `fn` with `session/update` notifications temporarily prevented
* from reaching whatever `messageHandler` is currently installed (i.e.
* the last prompt() turn's handler), restoring it once `fn` settles.
*
* Needed for out-of-band calls that don't go through `prompt()` at all —
* e.g. OpenCode's /compact bridge, which triggers native compaction via
* a raw HTTP request to the agent subprocess instead of `session/prompt`.
* The agent keeps streaming `session/update` notifications (thought
* chunks etc.) over the same ACP transport while that HTTP call runs,
* and `handleSessionUpdate` forwards them unconditionally — with no
* prompt() turn in flight to own them, they'd otherwise land on the
* previous turn's now-stale `messageHandler` and render as a duplicate
* assistant message alongside whatever the caller explicitly displays
* from the HTTP response.
*
* `captureAvailableCommands` / `forwardSessionInfoUpdate` /
* `captureUsageUpdate` in `handleSessionUpdate` are untouched by this —
* only the `messageHandler.handleUpdate` forwarding is suppressed.
*
* Session-agnostic: this is a pure prompt()-adjacent utility with no
* Gemini/OpenCode-specific behavior, so it's safe on the shared
* AcpSdkBackend class — nothing calls it unless a caller opts in.
*
* The `this.messageHandler === null` guard on restore is defense in
* depth: normal serialization (compact and prompts run through the same
* single dequeue loop — see opencodeRemoteLauncher.ts) means `fn` should
* never overlap with a real prompt() turn, but if `disconnect()` or a
* new `prompt()` did run concurrently and changed `messageHandler`
* during `fn`, this avoids clobbering whatever it set.
*
* Restoring the handler waits for the same quiet-drain `prompt()` already
* uses before installing a *new* handler for the next turn (see its
* `PRE_PROMPT_UPDATE_QUIET_PERIOD_MS`/`_DRAIN_TIMEOUT_MS` call) — the
* same class of race, just on the way back in instead of the way out.
* Aborting `fn()` client-side (e.g. OpenCode's compact bridge aborting
* its HTTP call) does not necessarily stop the agent from continuing the
* operation server-side: `session/update` is a separate notification
* channel from that HTTP request's lifecycle (confirmed while building
* the /compact bridge — see runCompactOperation's doc comment). Without
* this wait, late notifications from a still-running server-side
* operation would immediately leak into whichever handler gets restored
* (or into a brand new one prompt() installs right after) the instant
* `fn()` returns. `messageHandler` stays null (suppression still in
* effect) for the whole drain, so nothing leaks during it either.
*/
async suppressUpdatesDuring<T>(fn: () => Promise<T>): Promise<T> {
const previousHandler = this.messageHandler;
this.messageHandler = null;
try {
return await fn();
} finally {
await this.waitForSessionUpdateQuiet(
AcpSdkBackend.PRE_PROMPT_UPDATE_QUIET_PERIOD_MS,
AcpSdkBackend.PRE_PROMPT_UPDATE_DRAIN_TIMEOUT_MS
);
if (this.messageHandler === null) {
this.messageHandler = previousHandler;
}
}
}
/**
* Returns true if currently processing a message (prompt in progress).
* Useful for checking if it's safe to perform session operations.
@@ -91,10 +91,41 @@ export abstract class RemoteLauncherBase {
rpcHandlerManager.registerHandler(RPC_METHODS.Switch, async () => {});
}
/**
* Hook for flavor-specific "we are leaving remote mode" bookkeeping.
* No-op by default — override only if a flavor has state that must stop
* being valid the instant remote mode starts tearing down (OpenCode's
* /compact availability flag is the motivating case; see
* OpencodeRemoteLauncher's override).
*
* Called from two places, both intentionally, since neither alone covers
* every way a launcher can stop being "in remote mode":
* 1. `requestExit()`, synchronously, as its very first action — before
* `shouldExit`/`exitReason` are even set, and long before the
* `handler` it's about to await (e.g. OpenCode's `handleAbort()`,
* which does real async teardown work like cancelling the ACP
* prompt) gets a chance to run. This is what actually closes a race
* window: anything gated on flavor state this hook resets can no
* longer slip through between "a switch/exit was requested" and
* "the async teardown for it finished".
* 2. `start()`'s `finally` block, unconditionally, as a backstop for
* every other way `runMainLoop()` can end — a thrown exception, for
* instance, never goes through `requestExit()` at all. Firing here
* too is what guarantees the hook always runs by the time this
* launcher's promise settles, not just on the two deliberate exit
* paths.
*
* Must stay synchronous and idempotent — it can run twice per exit (once
* from each call site above) and must never assume `handler`/`cleanup()`
* have run yet.
*/
protected onLeavingRemote(): void {}
protected async requestExit(
reason: RemoteLauncherExitReason,
handler: () => void | Promise<void>
): Promise<void> {
this.onLeavingRemote();
if (!this.exitReason) {
this.exitReason = reason;
}
@@ -121,6 +152,9 @@ export abstract class RemoteLauncherBase {
try {
await this.runMainLoop();
} finally {
// Backstop call — see onLeavingRemote()'s doc comment for why
// this needs to run here too, not just from requestExit().
this.onLeavingRemote();
await this.cleanup();
this.finalizeTerminal();
}
+109
View File
@@ -0,0 +1,109 @@
import { describe, expect, it, vi } from 'vitest';
const harness = vi.hoisted(() => ({
runLocalRemoteArgs: [] as Array<Record<string, unknown>>,
localCalls: [] as Array<{ opts: unknown }>,
remoteCalls: [] as Array<{ opts: unknown }>
}));
vi.mock('@/agent/loopBase', () => ({
runLocalRemoteSession: vi.fn(async (opts: Record<string, unknown>) => {
harness.runLocalRemoteArgs.push(opts);
})
}));
vi.mock('./opencodeLocalLauncher', () => ({
opencodeLocalLauncher: vi.fn(async (_instance: unknown, opts: unknown) => {
harness.localCalls.push({ opts });
return 'exit';
})
}));
vi.mock('./opencodeRemoteLauncher', () => ({
opencodeRemoteLauncher: vi.fn(async (_instance: unknown, opts: unknown) => {
harness.remoteCalls.push({ opts });
return 'exit';
})
}));
// loop.ts constructs a real OpencodeSession internally (not injectable) —
// mock it so this test exercises only opencodeLoop's own glue logic (option
// forwarding + the compact-availability reset below), not the full
// AgentSessionBase construction contract.
vi.mock('./session', () => ({
OpencodeSession: vi.fn().mockImplementation(function (this: { onSessionFound: () => void }) {
this.onSessionFound = vi.fn();
})
}));
vi.mock('@/ui/logger', () => ({
logger: {
debug: vi.fn(),
getLogPath: () => '/tmp/hapi-loop-test.log'
}
}));
import { opencodeLoop } from './loop';
function baseOpts(overrides: Record<string, unknown> = {}) {
return {
path: '/tmp/hapi-loop-test',
messageQueue: {} as never,
session: { rpcHandlerManager: {} } as never,
api: {} as never,
onModeChange: vi.fn(),
hookServer: { port: 1234, stop: vi.fn() } as never,
hookUrl: 'http://127.0.0.1:1234/hook/opencode',
...overrides
};
}
describe('opencodeLoop compact availability wiring', () => {
// Resetting availability to false used to be loop.ts's job, done here in
// runLocal right before every local-mode entry. That left a window
// between "a switch/exit was requested" and "runLocal actually ran"
// where availability was still stale-true — a PR-review round found a
// /compact slash command arriving in that window could still queue and
// (via local mode bouncing straight back to remote to drain a non-empty
// queue) end up running despite the user having already asked to leave
// remote mode. The reset now happens as early as possible on the
// *leaving-remote* side instead (OpencodeRemoteLauncher's
// onLeavingRemote() override, called from RemoteLauncherBase's
// requestExit()/start()) — see opencodeRemoteLauncher.test.ts's
// "flips /compact availability to false synchronously..." test for that
// half of the contract. runLocal here must NOT also reset it: by the
// time runLocal ever runs, the prior remote launcher's promise (and
// therefore its onLeavingRemote() call) has already resolved.
it('does not call onCompactAvailabilityChange from runLocal — availability is already false by the time runLocal runs, reset earlier by the remote launcher leaving', async () => {
const events: boolean[] = [];
await opencodeLoop(baseOpts({
startingMode: 'local',
onCompactAvailabilityChange: (available: boolean) => events.push(available)
}) as Parameters<typeof opencodeLoop>[0]);
const opts = harness.runLocalRemoteArgs[0] as { runLocal: (instance: unknown) => Promise<unknown> };
expect(opts.runLocal).toBeDefined();
await opts.runLocal({});
expect(events).toEqual([]);
expect(harness.localCalls.length).toBe(1);
});
it('forwards onCompactAvailabilityChange unchanged to the remote launcher', async () => {
const onCompactAvailabilityChange = vi.fn();
await opencodeLoop(baseOpts({
startingMode: 'remote',
onCompactAvailabilityChange
}) as Parameters<typeof opencodeLoop>[0]);
const opts = harness.runLocalRemoteArgs.at(-1) as { runRemote: (instance: unknown) => Promise<unknown> };
await opts.runRemote({});
expect(harness.remoteCalls.length).toBe(1);
const remoteOpts = harness.remoteCalls[0]?.opts as { onCompactAvailabilityChange?: unknown };
expect(remoteOpts.onCompactAvailabilityChange).toBe(onCompactAvailabilityChange);
});
});
+22 -1
View File
@@ -24,6 +24,13 @@ interface OpencodeLoopOptions {
hookUrl: string;
onSessionReady?: (session: OpencodeSession) => void;
onReasoningEffortRollback?: (effort: string | null) => void;
onCompactAvailabilityChange?: (available: boolean) => void;
// Consumes (delete-and-return) whether the given localId was cancelled
// after already being dequeued — needed because a queued /compact can
// still be running (its REST call can take minutes) by the time a
// cancel arrives, well past the point `messageQueue.cancelByLocalId`
// can do anything about it.
isLocalIdCancelled?: (localId: string) => boolean;
}
export async function opencodeLoop(opts: OpencodeLoopOptions): Promise<void> {
@@ -54,12 +61,26 @@ export async function opencodeLoop(opts: OpencodeLoopOptions): Promise<void> {
session,
startingMode: opts.startingMode,
logTag: 'opencode-loop',
// /compact only exists in remote mode (it needs the ACP backend +
// internal HTTP baseUrl that only opencodeRemoteLauncher owns).
// Availability is reset to false as part of *leaving* remote mode,
// not on *entering* local mode — see OpencodeRemoteLauncher's
// onLeavingRemote() override — so it's already false by the time
// runLocal below ever runs; no reset needed here. That decoupling is
// deliberate: resetting on local-entry left a window between "a
// switch/exit was requested" and "the next runLocal() call actually
// happened" where availability was still stale-true, which a
// PR-review round found could let a /compact queued in that window
// run anyway once local mode bounced straight back to remote to
// drain a non-empty queue.
runLocal: (instance) => opencodeLocalLauncher(instance, {
hookServer: opts.hookServer,
hookUrl: opts.hookUrl
}),
runRemote: (instance) => opencodeRemoteLauncher(instance, {
onReasoningEffortRollback: opts.onReasoningEffortRollback
onReasoningEffortRollback: opts.onReasoningEffortRollback,
onCompactAvailabilityChange: opts.onCompactAvailabilityChange,
isLocalIdCancelled: opts.isLocalIdCancelled
}),
onSessionReady: opts.onSessionReady
});
File diff suppressed because it is too large Load Diff
+489 -9
View File
@@ -1,4 +1,5 @@
import React from 'react';
import { randomUUID } from 'node:crypto';
import { registerAcpSessionTitleSync } from '@/agent/acpSessionTitle';
import { logger } from '@/ui/logger';
import { buildHapiMcpBridge } from '@/codex/utils/buildHapiMcpBridge';
@@ -9,21 +10,113 @@ import { OpencodeDisplay } from '@/ui/ink/OpencodeDisplay';
import type { OpencodeSession } from './session';
import type { OpencodeMode, PermissionMode } from './types';
import { RPC_METHODS } from '@hapi/protocol/rpcMethods';
import { createOpencodeBackend } from './utils/opencodeBackend';
import { allocateFreePort, createOpencodeBackend } from './utils/opencodeBackend';
import { fetchCompactionSummary, splitProviderModel, triggerOpencodeCompact } from './utils/opencodeCompactBridge';
import { OpencodePermissionHandler } from './utils/permissionHandler';
import { OPENCODE_NATIVE_TOOL_INSTRUCTION, PLAN_MODE_INSTRUCTION } from './utils/systemPrompt';
import { resolveThoughtLevelEffort } from './thoughtLevelEffort';
type OpencodeRemoteLauncherOptions = {
onReasoningEffortRollback?: (effort: string | null) => void;
// Called with `true` once the ACP backend + internal HTTP baseUrl are
// ready (so /compact can actually run) and with `false` whenever this
// session leaves remote mode. runOpencode.ts uses this to decide whether
// a `/compact` message should be queued or immediately answered with a
// "not yet supported" reply — see its `slash.kind === 'compact'` branch.
onCompactAvailabilityChange?: (available: boolean) => void;
// Consumes (delete-and-return) whether the queued item with this localId
// was cancelled via runOpencode.ts's `onCancelQueuedMessage` fallback
// branch (see the comment on `cancelledDequeuedLocalIds` there for what
// that actually covers — in practice a narrow ack-vs-hub-DB-write race,
// not "cancel while the REST call is running"). Checked once the REST
// call (and summary lookup) settles, so a cancelled request's result
// doesn't surface for an action the user no longer expects a reply from.
isLocalIdCancelled?: (localId: string) => boolean;
};
export type AbortStatusDecision = {
message: string;
shouldClearThinking: boolean;
};
/**
* Pure decision logic for handleAbort()'s final step: which status message
* to show, and whether `thinking` should be cleared. Extracted out of the
* method itself (which calls this with freshly re-read state, not a
* snapshot from before its awaits — see the call site) so it's unit
* testable without needing to observe `MessageBuffer`/Ink rendering, which
* this file's test harness (`opencodeRemoteLauncher.test.ts`) has no
* infrastructure for.
*
* A compact operation left deliberately running after a plain Stop (see
* `compactResultSuppressed`'s field doc comment on the class) is the one
* case where nothing has actually stopped yet — Stop alone cannot leave
* this remote session, only switch-to-local/exit can, so the message says
* so explicitly rather than leaving the user wondering why the UI still
* looks busy.
*/
export function selectAbortStatusMessage(opts: {
hasCompactInFlight: boolean;
leavingRemote: boolean;
compactAborted: boolean;
}): AbortStatusDecision {
const compactStillWaiting = opts.hasCompactInFlight && !opts.leavingRemote && !opts.compactAborted;
if (compactStillWaiting) {
return {
message: 'Stop requested — waiting for the in-progress compaction to finish on the server. Switch to local or exit to leave immediately.',
shouldClearThinking: false
};
}
return { message: 'Turn aborted', shouldClearThinking: true };
}
class OpencodeRemoteLauncher extends RemoteLauncherBase {
private readonly session: OpencodeSession;
private backend: ReturnType<typeof createOpencodeBackend> | null = null;
/** Loopback base URL of the OpenCode ACP subprocess's internal HTTP API, set once the backend is spawned with an explicit --port/--hostname. */
private baseUrl: string | null = null;
private permissionHandler: OpencodePermissionHandler | null = null;
private happyServer: { stop: () => void } | null = null;
private abortController = new AbortController();
// Set by the dequeue loop as soon as a batch is identified as a
// `operation:'compact'` one — deliberately *before* that batch's inline
// model/effort switch runs, not only once runCompactOperation()'s
// triggerOpencodeCompact() REST call actually starts (a hostile-review
// sweep found that creating it any later left a window during that
// switch — a real async ACP round-trip — where an abort had nothing to
// act on yet). Null whenever no compact batch is in flight. Unlike
// `abortController` above (which governs the dequeue loop's
// wait-for-next-message signal), `handleAbort()` needs this to actually
// interrupt the compact's HTTP call(s) — without it, Stop/switch-to-local
// has no way to unblock a dequeued /compact whose REST call is
// deliberately unbounded (see triggerOpencodeCompact's doc comment) and
// the launcher stays wedged until it eventually settles on its own.
private compactAbortController: AbortController | null = null;
// True from the moment handleAbort() observes a compact operation in
// flight until the dequeue loop creates the next one. A 6th PR-review
// round found that unconditionally aborting `compactAbortController` on
// *plain* Stop (not just switch/exit) broke a core invariant this
// feature's whole redesign (see the FIFO-queue comment on the dequeue
// loop) depends on: compact and a prompt must never touch the same
// OpenCode session at once. Aborting only unblocks the *client's* fetch
// — `session/update` notifications are a separate channel from that
// HTTP request's lifecycle (see AcpSdkBackend.suppressUpdatesDuring's
// doc comment), so the agent can still be compacting server-side well
// after the client gives up, and the quiet-drain there (bounded at
// ~1.2s) is not a real guarantee that a multi-minute server-side
// compaction has actually finished. If the dequeue loop moved on to a
// prompt as soon as the client-side abort settled, that prompt could
// run concurrently with a compaction still touching the same session.
//
// The fix: plain Stop only sets this flag (suppressing the eventual
// result) and leaves `compactAbortController` alone, so
// runCompactOperation()'s own awaits keep blocking the dequeue loop
// until the *real* HTTP response arrives — i.e. until the server
// actually finishes. Switch-to-local/exit still abort the controller for
// real (see handleAbort's `leavingRemote` parameter) because cleanup()
// is about to disconnect the whole ACP subprocess regardless, so there's
// no session left to protect.
private compactResultSuppressed = false;
private displayPermissionMode: PermissionMode | null = null;
private instructionsSent = false;
private currentBackendModel: string | null = null;
@@ -62,8 +155,20 @@ class OpencodeRemoteLauncher extends RemoteLauncherBase {
});
this.happyServer = happyServer;
// Pre-select a loopback port for the ACP subprocess's internal HTTP
// API and pass it explicitly via --port/--hostname. opencode does not
// announce the bound port anywhere (stdout/stderr/ACP responses) when
// launched with --port 0, so HAPI must choose it up front to be able
// to reach that HTTP API later (e.g. for /compact — see
// opencodeCompactBridge.ts).
const hostname = '127.0.0.1';
const port = await allocateFreePort(hostname);
this.baseUrl = `http://${hostname}:${port}`;
const backend = createOpencodeBackend({
cwd: session.path
cwd: session.path,
port,
hostname
});
this.backend = backend;
registerAcpSessionTitleSync(backend, session.client);
@@ -115,6 +220,34 @@ class OpencodeRemoteLauncher extends RemoteLauncherBase {
this.currentBackendEffort = thoughtLevelOption?.currentValue ?? null;
this.defaultBackendEffort = this.currentBackendEffort;
// Let the caller (runOpencode.ts) know native /compact can actually
// run now that the ACP backend + internal HTTP baseUrl exist. The
// dequeue loop below (not an externally-invoked trigger) is what
// executes it, in its actual FIFO queue position.
//
// A 9th PR-review round found a race here: a terminal
// switch-to-local/exit can land *during* the newSession/loadSession
// await above (setupTerminal() wires up onExit/onSwitchToLocal
// before runMainLoop() even starts, so this is reachable well
// before setupAbortHandlers() below registers the RPC
// 'abort'/'switch' handlers). RemoteLauncherBase.requestExit()
// already fired onLeavingRemote() (availability(false)) and set
// `this.shouldExit = true` synchronously for that switch/exit,
// before awaiting its handler — but this line used to run
// regardless once initialization finished, resurrecting
// availability(true) even though the session is already on its way
// out. runOpencode.ts's compactSupported/compactTeardownInProgress
// gate treats compactSupported flipping true as reason enough to
// ignore compactTeardownInProgress entirely (see that gate's doc
// comment), so this stray true could let a /compact arriving right
// after slip into the queue mid-teardown. Checking `shouldExit`
// here — the same flag requestExit() already set — keeps
// availability from ever un-flipping once a switch/exit is
// underway.
if (!this.shouldExit) {
this.options.onCompactAvailabilityChange?.(true);
}
// Expose the cached models metadata via per-session RPC so the hub can
// forward it to the web UI's model selector without round-tripping ACP.
session.client.rpcHandlerManager.registerHandler(RPC_METHODS.ListOpencodeModels, async () => {
@@ -149,7 +282,10 @@ class OpencodeRemoteLauncher extends RemoteLauncherBase {
this.applyDisplayMode(session.getPermissionMode() as PermissionMode);
this.setupAbortHandlers(session.client.rpcHandlerManager, {
onAbort: () => this.handleAbort(),
// Explicit `false`: plain Stop stays in this remote session, so
// an in-flight compact must not be aborted client-side — see
// handleAbort's `leavingRemote` doc comment.
onAbort: () => this.handleAbort(false),
onSwitch: () => this.handleSwitchRequest()
});
@@ -167,6 +303,29 @@ class OpencodeRemoteLauncher extends RemoteLauncherBase {
break;
}
// Created here — before the model/effort switch below — rather
// than inside runCompactOperation(), so it already exists for
// handleAbort() to act on during that switch. backend.setModel()/
// setConfigOption() are real async ACP round-trips that yield to
// the event loop; a hostile-review whole-feature sweep found
// that an abort landing in that window used to hit a still-null
// compactAbortController (a no-op) and then get silently
// forgotten once runCompactOperation() created a *fresh*
// controller afterward — the compact's unbounded REST call would
// then run to completion with no way to interrupt it, despite
// the user having already pressed Stop/switch/exit.
const isCompactBatch = batch.mode.operation === 'compact';
const compactAbortController = isCompactBatch ? new AbortController() : null;
if (compactAbortController) {
this.compactAbortController = compactAbortController;
// Reset here (as early as the controller itself — see its
// sibling field's doc comment for why that timing matters)
// rather than inside runCompactOperation(), so a plain Stop
// landing during the model/effort switch below already has
// something to suppress.
this.compactResultSuppressed = false;
}
// Inline model change via ACP RPC (session/set_model — see ACP SDK
// schema `x-method: session/set_model`). Mirrors the Gemini pattern
// from PR #543: if the running OpenCode build does not implement the
@@ -271,6 +430,118 @@ class OpencodeRemoteLauncher extends RemoteLauncherBase {
this.applyDisplayMode(batch.mode.permissionMode);
messageBuffer.addMessage(batch.message, 'user');
// /compact reaches here through the exact same dequeue loop as
// any prompt — it was pushed via messageQueue.pushIsolated(...)
// in runOpencode.ts, so it occupies its real FIFO position
// relative to prompts queued before or after it (fixes a prior
// design where /compact ran via an externally-invoked trigger
// and could execute ahead of an already-queued prompt). The
// model/effort switch above already ran for this batch just like
// any other, so compaction runs under whatever model this batch
// resolved to.
if (isCompactBatch && compactAbortController) {
// A compact batch is always a single isolated item (pushed
// via pushIsolated), so its own localId is exactly
// batch.items[0]?.localId.
const compactLocalId = batch.items[0]?.localId;
// A 7th PR-review round found that a plain Stop landing
// *during the model/effort switch above* — before this
// compact's REST request has ever actually been sent — was
// silently ignored here. Plain Stop's handleAbort(false) only
// sets `compactResultSuppressed = true`; it deliberately
// leaves compactAbortController.signal alone (see that
// field's doc comment — Round 6 needs the real HTTP request
// to keep running so the dequeue loop can wait for genuine
// server-side completion). But that logic assumed a request
// was already in flight to wait for. Here, mid-switch, none
// has been sent yet — so this branch used to call
// runCompactOperation() unconditionally once the switch
// resolved anyway, starting a brand new REST request the
// instant a cancelled compact's turn came up and blocking
// the dequeue loop for however long that takes.
//
// The fix is narrow on purpose: skip starting the operation
// only when a plain Stop landed (compactResultSuppressed)
// AND the controller was never actually aborted. If the
// controller WAS aborted, that means switch/exit's
// handleAbort(true) ran instead — and Round 5's test
// (below) established that runCompactOperation() must still
// be called in that case, threading the pre-aborted signal
// through so the fetch call rejects immediately without any
// network I/O, rather than being skipped here.
//
// An 8th PR-review round found this same reasoning also
// applies to isLocalIdCancelled, which round 7 had
// deliberately left out of this check (see runOpencode.ts's
// preparingLocalIds/cancelledBeforeEnqueue doc comment for
// the full mechanism): the localId-keyed cancel Set it reads
// can *only* ever be populated during the brief network
// round trip between the CLI emitting the /compact item's
// "invoked" ack and the hub recording it — never while a
// compact REST call is actually running. So if
// isLocalIdCancelled(compactLocalId) is already true here,
// that unconditionally means this compact was cancelled
// before its REST request was ever sent, exactly like the
// compactResultSuppressed case above — there's no
// in-flight server-side work to preserve by starting the
// operation anyway. (isLocalIdCancelled is a delete-and-
// return, one-shot callback, so checking it here consumes
// the same entry runCompactOperation()'s own isCancelled()
// would otherwise have consumed — it isn't checked twice.)
const compactCancelledByLocalId = compactLocalId
? (this.options.isLocalIdCancelled?.(compactLocalId) ?? false)
: false;
const cancelledBeforeStart =
(this.compactResultSuppressed && !compactAbortController.signal.aborted)
|| compactCancelledByLocalId;
if (cancelledBeforeStart) {
if (this.compactAbortController === compactAbortController) {
this.compactAbortController = null;
}
// A 10th PR-review round found this skip path never
// calls session.onThinkingChange(true) (that's the
// whole point of skipping) but also never told the hub
// this queued item is done, leaving the web UI spinner
// stuck: markMessageQueued's 15s "queued thinking"
// grace (hub/src/sync/sessionCache.ts) keeps thinking
// pinned true regardless of keepalives until either the
// grace expires or a messages-consumed ack with
// `clearQueuedThinkingGrace` arrives. Same situation,
// same fix, as the synchronous slash.kind === 'handled'
// path in runOpencode.ts (e.g. /model — see its
// `clearQueuedThinkingGrace` comment there): ack with
// the grace-clearing flag, then push an immediate
// thinking=false keepalive so the spinner clears
// without waiting on the grace. (This is on top of, not
// instead of, the queue's own unflagged
// onBatchConsumed ack — a second ack for an
// already-invoked localId is a no-op on the hub's
// first-write-wins queued-message protocol, and
// clearQueuedThinkingGrace itself is keyed by session,
// not by localId, so it's idempotent too.)
if (compactLocalId) {
session.client.emitMessagesConsumed([compactLocalId], { clearQueuedThinkingGrace: true });
}
session.onThinkingChange(false);
if (session.queue.size() === 0 && !this.shouldExit) {
sendReady();
}
continue;
}
session.onThinkingChange(true);
try {
await this.runCompactOperation(acpSessionId, compactAbortController, compactLocalId);
} finally {
session.onThinkingChange(false);
if (session.queue.size() === 0 && !this.shouldExit) {
sendReady();
}
}
continue;
}
// Inject title instructions on first prompt
let messageText = batch.message;
if (batch.mode.permissionMode === 'plan') {
@@ -310,6 +581,24 @@ class OpencodeRemoteLauncher extends RemoteLauncherBase {
}
}
/**
* /compact must stop being offered the instant remote mode starts
* leaving — not merely by the time it's actually torn down, and
* critically not only on the *next* local-mode entry (the previous
* mechanism, in loop.ts's `runLocal:` callback). That gap between "a
* switch/exit was requested" and "the next runLocal() call reset this"
* is exactly the window a PR-review round found: a /compact slash
* command arriving in it still queues normally (runOpencode.ts's
* `compactSupported` flag hadn't flipped yet), and since local mode
* immediately hands back to remote when it finds a non-empty queue, that
* queued compact can end up running anyway — despite the user having
* already asked to leave remote mode. See onLeavingRemote()'s doc
* comment on RemoteLauncherBase for exactly when this fires.
*/
protected onLeavingRemote(): void {
this.options.onCompactAvailabilityChange?.(false);
}
protected async cleanup(): Promise<void> {
this.clearAbortHandlers(this.session.client.rpcHandlerManager);
@@ -336,6 +625,147 @@ class OpencodeRemoteLauncher extends RemoteLauncherBase {
this.options.onReasoningEffortRollback?.(effort);
}
/**
* Executes the /compact operation for a queued `operation:'compact'`
* batch. Reached only through the main dequeue loop (so it never runs
* concurrently with a prompt turn — see the loop's doc comment), which
* is also why this needs no timeout/mutex of its own despite the REST
* call it makes potentially taking several minutes.
*
* `localId` is used to detect a cancel that runOpencode.ts's
* `isLocalIdCancelled` reports for this item (see its declaration there
* for the real — and narrow — race window that covers) — checked at each
* point below right before a result would be shown, same as the
* pre-redesign behavior where this was a single `wasCancelled()` check
* after one combined async trigger(). "Compaction started" itself is
* never suppressed (it wasn't before either).
*
* Separately, `compactAbortController`/`compactResultSuppressed` cover a
* different case: Stop/switch-to-local firing *while the REST call is
* actually in flight*, which `isLocalIdCancelled` cannot — that
* mechanism only ever observes a cancel for this item's *queue message*,
* and by this point the item has already been dequeued. `isCancelled()`
* below checks all three, so any kind of cancellation suppresses the
* eventual result the same way — but only switch/exit (`leavingRemote`
* in handleAbort()) actually aborts `compactAbortController.signal`; a
* plain Stop sets `compactResultSuppressed` alone and deliberately
* leaves the signal un-aborted, so this function's own awaits below keep
* blocking the dequeue loop until the operation *really* finishes
* server-side — see `compactResultSuppressed`'s field doc comment for
* why that invariant matters.
*
* `compactAbortController` is created by the caller (the dequeue loop),
* not here, and passed in — deliberately, before the loop's model/effort
* switch for this batch runs, not after. A hostile-review whole-feature
* sweep found that creating it in here (i.e. only once this function was
* actually entered) left a window during that switch — a real async ACP
* round-trip — where an abort had nothing to act on yet (`this
* .compactAbortController` was still null) and was silently lost by the
* time this function created a *fresh* controller afterward.
*/
private async runCompactOperation(
acpSessionId: string,
compactAbortController: AbortController,
localId?: string
): Promise<void> {
const session = this.session;
session.sendSessionEvent({ type: 'message', message: '📦 Compaction started' });
try {
const isCancelled = (): boolean =>
(localId ? (this.options.isLocalIdCancelled?.(localId) ?? false) : false)
|| compactAbortController.signal.aborted
|| this.compactResultSuppressed;
const backend = this.backend;
const baseUrl = this.baseUrl;
if (!baseUrl || !backend) {
if (!isCancelled()) {
session.sendSessionEvent({
type: 'message',
message: '📦 Compaction failed: OpenCode internal HTTP API base URL is not available.'
});
}
return;
}
const metadata = backend.getSessionModelsMetadata?.(acpSessionId);
const split = splitProviderModel(metadata?.currentModelId ?? this.currentBackendModel);
if (!split) {
if (!isCancelled()) {
session.sendSessionEvent({
type: 'message',
message: '📦 Compaction failed: OpenCode model metadata is not available; cannot determine provider/model for compaction.'
});
}
return;
}
// Suppressed: OpenCode keeps streaming session/update notifications
// (agent_thought_chunk etc.) over the ACP transport while this raw
// HTTP call runs — with no prompt() turn in flight to own them, they
// would otherwise leak into the previous turn's still-installed
// onUpdate and render as a duplicate assistant message alongside the
// explicit summary we show below (from fetchCompactionSummary).
// See AcpSdkBackend.suppressUpdatesDuring's doc comment.
//
// `signal` lets handleAbort() interrupt this specific call (see
// compactAbortController's field doc comment) — triggerOpencodeCompact
// otherwise has no deadline by design, since a real compaction can
// legitimately take minutes.
const result = await backend.suppressUpdatesDuring(() => triggerOpencodeCompact({
baseUrl,
sessionId: acpSessionId,
providerId: split.providerId,
modelId: split.modelId,
signal: compactAbortController.signal
}));
if (!result.ok) {
if (!isCancelled()) {
session.sendSessionEvent({ type: 'message', message: `📦 Compaction failed: ${result.error}` });
} else {
logger.debug('[opencode-remote] /compact failure suppressed: cancelled or aborted before it resolved');
}
return;
}
// Best-effort: fetch the actual summary text OpenCode generated
// before the final cancellation check, so a cancel landing anywhere
// during this whole operation (REST call or summary lookup)
// suppresses "Compaction completed" and the Reasoning block
// together — this mirrors the pre-redesign behavior, where both were
// produced by one combined async step checked once. `signal` is
// required on this call (see OpencodeCompactCallOpts) for exactly
// the reason a prior PR-review round flagged as missing here: the
// POST above being interruptible isn't enough on its own if this
// GET can still block Stop/switch-to-local for as long as it takes.
const summary = await fetchCompactionSummary({ baseUrl, sessionId: acpSessionId, signal: compactAbortController.signal });
if (isCancelled()) {
logger.debug('[opencode-remote] /compact result suppressed: cancelled or aborted before it resolved');
return;
}
session.sendSessionEvent({ type: 'message', message: '📦 Compaction completed' });
if (summary.found) {
const converted = convertAgentMessage({ type: 'reasoning', text: summary.text, id: randomUUID() });
if (converted) {
session.sendAgentMessage(converted);
}
}
} finally {
// Defensive: only clear if this is still the controller we set —
// mirrors the same "don't clobber a newer value" guard as
// AcpSdkBackend.suppressUpdatesDuring's restore. In practice this
// is always still the same instance, since compact runs
// serialized through the single dequeue loop (never concurrently
// with another runCompactOperation call).
if (this.compactAbortController === compactAbortController) {
this.compactAbortController = null;
}
}
}
private handleAgentMessage(message: AgentMessage): void {
const converted = convertAgentMessage(message);
if (converted) {
@@ -383,29 +813,79 @@ class OpencodeRemoteLauncher extends RemoteLauncherBase {
}
}
private async handleAbort(): Promise<void> {
/**
* `leavingRemote` distinguishes plain Stop (`false`, the default — stays
* in the same remote session) from switch-to-local/exit (`true` — the
* session is being torn down). A 6th PR-review round rejected an earlier
* fix (always aborting `compactAbortController` here) because it broke
* this feature's core invariant: compact and a prompt must never touch
* the same OpenCode session concurrently (see
* `compactResultSuppressed`'s field doc comment for the full
* reasoning). Plain Stop now only suppresses the eventual result and
* leaves the compact operation's REST call running for real — the
* dequeue loop stays blocked on it until the server actually finishes,
* exactly as it does for an un-aborted turn. Switch/exit still abort it
* for real: `cleanup()` disconnects the whole ACP subprocess right
* after, so there is no shared-session invariant left to protect and
* responsiveness (fixed in an earlier round) matters more.
*/
private async handleAbort(leavingRemote = false): Promise<void> {
// A hostile-review sweep found that a plain Stop during an in-flight
// compact — which deliberately leaves the operation running for real
// (see compactResultSuppressed's doc comment) — still unconditionally
// flipped `thinking` off and reported "Turn aborted" below, telling
// the user the turn had stopped while the dequeue loop was actually
// still blocked inside runCompactOperation() for however long the
// real server-side compaction takes (potentially minutes). Track
// that specific case so the messaging stays honest: nothing has
// actually stopped yet from the user's perspective, and the dequeue
// loop's own `finally` (once runCompactOperation() genuinely
// returns) remains the sole source of truth for when this turn is
// done.
const compactAbortController = this.compactAbortController;
if (compactAbortController) {
this.compactResultSuppressed = true;
if (leavingRemote) {
compactAbortController.abort();
}
}
const backend = this.backend;
if (backend && this.session.sessionId) {
await backend.cancelPrompt(this.session.sessionId);
}
await this.permissionHandler?.cancelAll('User aborted');
this.session.queue.reset();
this.session.onThinkingChange(false);
this.abortController.abort();
this.abortController = new AbortController();
this.messageBuffer.addMessage('Turn aborted', 'status');
// Re-read here (not the snapshot taken above, before the awaits) in
// case a concurrent leavingRemote=true call for the same compact
// interleaved with this one and already aborted it — RPC dispatch
// doesn't serialize handleAbort() calls against each other, so a
// Stop immediately followed by a switch-to-local can genuinely
// overlap. Without this, the (now-stale) plain-Stop continuation
// could append its "still waiting" message after the switch's
// "Turn aborted" already ran, showing the two in a confusing order.
const decision = selectAbortStatusMessage({
hasCompactInFlight: compactAbortController !== null,
leavingRemote,
compactAborted: compactAbortController?.signal.aborted ?? false
});
if (decision.shouldClearThinking) {
this.session.onThinkingChange(false);
}
this.messageBuffer.addMessage(decision.message, 'status');
}
private async handleExitFromUi(): Promise<void> {
await this.requestExit('exit', () => this.handleAbort());
await this.requestExit('exit', () => this.handleAbort(true));
}
private async handleSwitchFromUi(): Promise<void> {
await this.requestExit('switch', () => this.handleAbort());
await this.requestExit('switch', () => this.handleAbort(true));
}
private async handleSwitchRequest(): Promise<void> {
await this.requestExit('switch', () => this.handleAbort());
await this.requestExit('switch', () => this.handleAbort(true));
}
}
+280 -1
View File
@@ -6,7 +6,17 @@ const mockOpencodeSession = vi.hoisted(() => ({
setModelReasoningEffort: vi.fn(),
pushKeepAlive: vi.fn(),
thinking: false,
stopKeepAlive: vi.fn()
stopKeepAlive: vi.fn(),
onThinkingChange: vi.fn(),
// Mirrors AgentSessionBase's own `mode` field ('local' | 'remote',
// flipped synchronously by onModeChange before either launcher
// starts/finishes) — settable per test to simulate a session that
// started in remote mode and is still initializing (ACP backend not
// ready yet, so onCompactAvailabilityChange(true) hasn't fired), as
// opposed to a genuinely local-mode session. This becomes
// sessionWrapperRef.current in runOpencode.ts via the mocked
// opencodeLoop's onSessionReady callback below.
mode: 'local' as 'local' | 'remote'
}));
const harness = vi.hoisted(() => ({
@@ -18,7 +28,12 @@ const harness = vi.hoisted(() => ({
onUserMessage: vi.fn(),
onCancelQueuedMessage: vi.fn(),
sendAgentMessage: vi.fn(),
sendSessionEvent: vi.fn(),
emitMessagesConsumed: vi.fn(),
// Needed for createModeChangeHandler(session) (real, unmocked) to
// run without throwing when a test invokes the real onModeChange
// wrapper passed to opencodeLoop.
updateAgentState: vi.fn(),
rpcHandlerManager: {
registerHandler: vi.fn()
}
@@ -100,10 +115,14 @@ describe('runOpencode set-session-config handler', () => {
mockOpencodeSession.setPermissionMode.mockReset();
mockOpencodeSession.setModelReasoningEffort.mockReset();
mockOpencodeSession.pushKeepAlive.mockReset();
mockOpencodeSession.onThinkingChange.mockReset();
mockOpencodeSession.mode = 'local';
harness.session.onUserMessage.mockReset();
harness.session.onCancelQueuedMessage.mockReset();
harness.session.sendAgentMessage.mockReset();
harness.session.sendSessionEvent.mockReset();
harness.session.emitMessagesConsumed.mockReset();
harness.session.updateAgentState.mockReset();
harness.session.rpcHandlerManager.registerHandler.mockReset();
harness.listSlashCommands.mockReset();
harness.listSlashCommands.mockResolvedValue([]);
@@ -257,6 +276,234 @@ describe('runOpencode set-session-config handler', () => {
expect(harness.session.sendAgentMessage).toHaveBeenCalled();
});
it('queues a /compact request (isolated, with operation:"compact") once compact becomes available', async () => {
await runOpencode({});
const onCompactAvailabilityChange = harness.opencodeLoopArgs[0]?.onCompactAvailabilityChange as
((available: boolean) => void) | undefined;
expect(onCompactAvailabilityChange).toBeDefined();
onCompactAvailabilityChange!(true);
const messageQueue = harness.opencodeLoopArgs[0]?.messageQueue as
{ queue: Array<{ message: string; mode: { operation?: string }; localId?: string; isolate?: boolean }> };
const userMessageHandler = harness.session.onUserMessage.mock.calls[0]?.[0] as
((msg: { content: { text: string; attachments?: unknown[] } }, localId?: string) => void)
| undefined;
expect(userMessageHandler).toBeDefined();
userMessageHandler!({ content: { text: '/compact' } }, 'local-compact');
// Drain microtasks across the async chain: listSlashCommands -> slash
// resolve -> messageQueue.pushIsolated(...).
for (let i = 0; i < 5; i++) {
await new Promise((resolve) => setTimeout(resolve, 0));
}
// No manual emitMessagesConsumed call here (unlike the synchronous
// 'handled' branch) — the ack now happens automatically at dequeue
// time via MessageQueue2's onBatchConsumed (wired in
// AgentSessionBase's constructor onto session.queue, same as any
// regular prompt), not synchronously when queuing. A manual call
// right here used to exist and fire immediately regardless of FIFO
// position — that's what let the hub mark a still-queued /compact
// "invoked" before it was actually dequeued, breaking cancellation
// of it while queued (see the comment on this branch in
// runOpencode.ts and opencodeRemoteLauncher.test.ts's "cancelling a
// /compact operation while it is still queued behind another
// prompt" test for the fix this enables).
expect(harness.session.emitMessagesConsumed).not.toHaveBeenCalled();
// The actual REST call, "Compaction started/completed" status events,
// and Reasoning-block summary now all happen inside
// opencodeRemoteLauncher.ts's dequeue loop once this item reaches the
// front of the queue (covered by opencodeRemoteLauncher.test.ts) —
// runOpencode.ts's job for a supported /compact is only to queue it
// in its correct FIFO position, never to run it directly.
expect(messageQueue.queue).toEqual([
{
message: '',
mode: expect.objectContaining({ operation: 'compact' }),
modeHash: expect.any(String),
localId: 'local-compact',
isolate: true
}
]);
expect(harness.session.sendSessionEvent).not.toHaveBeenCalled();
expect(harness.session.sendAgentMessage).not.toHaveBeenCalled();
});
it('queues /compact like a prompt while a remote-mode session is still initializing (ACP backend not ready yet), instead of rejecting it as not-yet-supported', async () => {
// Reproduces a hostile-review finding: compactSupported alone
// conflates "genuinely local mode" with "remote mode, but ACP
// initialize + session load/new hasn't finished yet" — a regular
// prompt sent in that exact same startup window queues normally and
// just waits, but /compact used to get an immediate not-yet-supported
// reply instead, even though the session is (or is about to be) in
// remote mode. sessionWrapperRef.current?.mode (mocked here via
// mockOpencodeSession.mode, delivered through onSessionReady) is what
// now distinguishes the two — deliberately never call
// onCompactAvailabilityChange(true) here, since the whole point is
// that this must queue even while it's still false.
mockOpencodeSession.mode = 'remote';
await runOpencode({});
const messageQueue = harness.opencodeLoopArgs[0]?.messageQueue as
{ queue: Array<{ message: string; mode: { operation?: string }; localId?: string; isolate?: boolean }> };
const userMessageHandler = harness.session.onUserMessage.mock.calls[0]?.[0] as
((msg: { content: { text: string; attachments?: unknown[] } }, localId?: string) => void)
| undefined;
expect(userMessageHandler).toBeDefined();
userMessageHandler!({ content: { text: '/compact' } }, 'local-compact-pending');
for (let i = 0; i < 5; i++) {
await new Promise((resolve) => setTimeout(resolve, 0));
}
// Must not get the not-yet-supported reply.
expect(harness.session.sendAgentMessage).not.toHaveBeenCalled();
// Must be queued exactly like the compactSupported===true case above.
expect(messageQueue.queue).toEqual([
{
message: '',
mode: expect.objectContaining({ operation: 'compact' }),
modeHash: expect.any(String),
localId: 'local-compact-pending',
isolate: true
}
]);
});
it('falls back to a not-yet-supported message for /compact when compact is not available (e.g. local mode)', async () => {
await runOpencode({});
// Deliberately do not call onCompactAvailabilityChange(true) — this
// is the state a local-mode session stays in (loop.ts resets it to
// false on every local entry and opencodeLocalLauncher never sets it
// true).
const userMessageHandler = harness.session.onUserMessage.mock.calls[0]?.[0] as
((msg: { content: { text: string; attachments?: unknown[] } }, localId?: string) => void)
| undefined;
userMessageHandler!({ content: { text: '/compact' } }, 'local-compact-none');
for (let i = 0; i < 5; i++) {
await new Promise((resolve) => setTimeout(resolve, 0));
}
const messages = harness.session.sendAgentMessage.mock.calls.map((call) => (call[0] as { message: string }).message);
expect(messages).toEqual(['/compact is not yet supported in HAPI OpenCode sessions.']);
const messageQueue = harness.opencodeLoopArgs[0]?.messageQueue as { queue: unknown[] };
expect(messageQueue.queue).toEqual([]);
});
it('stops queuing /compact once availability is reset to false (e.g. a remote->local handoff mid-session)', async () => {
await runOpencode({});
// Faithful to real production timing: session.mode stays 'remote'
// throughout the whole teardown window (onLeavingRemote firing
// synchronously as the very first action of requestExit(), long
// before runMainLoop() actually returns and mode flips back to
// 'local') — see AgentSessionBase.onModeChange and
// OpencodeRemoteLauncher.onLeavingRemote's doc comments. A hostile
// review found that omitting this from the mock let a real
// regression slip through: the mode-based /compact queuing fix
// for the *startup* window (mode:'remote' but not yet ready) also
// accidentally re-opened queuing during *this* teardown window,
// since both look identical if you only check `mode !== 'remote'`.
mockOpencodeSession.mode = 'remote';
const onCompactAvailabilityChange = harness.opencodeLoopArgs[0]?.onCompactAvailabilityChange as
((available: boolean) => void) | undefined;
onCompactAvailabilityChange!(true);
onCompactAvailabilityChange!(false);
const userMessageHandler = harness.session.onUserMessage.mock.calls[0]?.[0] as
((msg: { content: { text: string; attachments?: unknown[] } }, localId?: string) => void)
| undefined;
userMessageHandler!({ content: { text: '/compact' } }, 'local-compact-reset');
for (let i = 0; i < 5; i++) {
await new Promise((resolve) => setTimeout(resolve, 0));
}
const messages = harness.session.sendAgentMessage.mock.calls.map((call) => (call[0] as { message: string }).message);
expect(messages).toEqual(['/compact is not yet supported in HAPI OpenCode sessions.']);
const messageQueue = harness.opencodeLoopArgs[0]?.messageQueue as { queue: unknown[] };
expect(messageQueue.queue).toEqual([]);
});
it('resumes queuing /compact once a torn-down session re-enters remote mode (compactTeardownInProgress resets on the next remote entry)', async () => {
// Locks in the other half of compactTeardownInProgress's contract:
// it must not get stuck true forever after one teardown, or every
// later remote re-entry's startup window (the case the "queues
// /compact like a prompt while..." test above covers) would
// incorrectly reject /compact too.
//
// Round 2 of the review-cycle that produced this test found the
// first version didn't actually discriminate the fix: it left
// `mockOpencodeSession.mode` at 'remote' throughout, so the gate's
// `mode !== 'remote'` clause was permanently false and the test
// would have passed identically even if compactTeardownInProgress
// never reset (or didn't exist at all). Faithfully modeling the
// real local interlude between the two remote attempts — mode
// actually flips to 'local' once the first remote launcher's
// runMainLoop() fully unwinds, per AgentSessionBase.onModeChange —
// is what makes this test sensitive to the reset specifically.
mockOpencodeSession.mode = 'remote';
await runOpencode({});
const onCompactAvailabilityChange = harness.opencodeLoopArgs[0]?.onCompactAvailabilityChange as
((available: boolean) => void) | undefined;
const onModeChange = harness.opencodeLoopArgs[0]?.onModeChange as
((mode: 'local' | 'remote') => void) | undefined;
expect(onModeChange).toBeDefined();
const messageQueue = harness.opencodeLoopArgs[0]?.messageQueue as
{ queue: Array<{ message: string; mode: { operation?: string }; localId?: string; isolate?: boolean }> };
const userMessageHandler = harness.session.onUserMessage.mock.calls[0]?.[0] as
((msg: { content: { text: string; attachments?: unknown[] } }, localId?: string) => void)
| undefined;
// First remote attempt becomes ready, then tears down.
onCompactAvailabilityChange!(true);
onCompactAvailabilityChange!(false);
// Local interlude — mode genuinely flips to 'local' here in
// production. Sanity-check /compact is still correctly rejected
// during it (proves the interlude is real, not cosmetic).
mockOpencodeSession.mode = 'local';
userMessageHandler!({ content: { text: '/compact' } }, 'local-compact-interlude');
for (let i = 0; i < 5; i++) {
await new Promise((resolve) => setTimeout(resolve, 0));
}
expect(harness.session.sendAgentMessage).toHaveBeenCalledTimes(1);
expect(messageQueue.queue).toEqual([]);
// The next remote attempt begins: mode flips back to 'remote' and
// onModeChange fires on that exact transition — this is what
// compactTeardownInProgress's reset actually depends on.
mockOpencodeSession.mode = 'remote';
onModeChange!('remote');
userMessageHandler!({ content: { text: '/compact' } }, 'local-compact-reentry');
for (let i = 0; i < 5; i++) {
await new Promise((resolve) => setTimeout(resolve, 0));
}
// Still only the one not-yet-supported reply from the interlude
// above — the re-entry /compact must queue, not get a second reply.
expect(harness.session.sendAgentMessage).toHaveBeenCalledTimes(1);
expect(messageQueue.queue).toEqual([
{
message: '',
mode: expect.objectContaining({ operation: 'compact' }),
modeHash: expect.any(String),
localId: 'local-compact-reentry',
isolate: true
}
]);
});
it('cancels a slash command that is cancelled before listSlashCommands resolves', async () => {
let releaseListSlashCommands: () => void = () => {};
const slashCommandsPromise = new Promise<unknown[]>((resolve) => {
@@ -288,4 +535,36 @@ describe('runOpencode set-session-config handler', () => {
expect(harness.session.sendAgentMessage).not.toHaveBeenCalled();
expect(harness.session.emitMessagesConsumed).not.toHaveBeenCalled();
});
it('bounds the unmatched-cancel tracking Set so it cannot grow unboundedly over a long session', async () => {
await runOpencode({});
const cancelHandler = harness.session.onCancelQueuedMessage.mock.calls[0]?.[0] as
((localId: string) => boolean) | undefined;
const isLocalIdCancelled = harness.opencodeLoopArgs[0]?.isLocalIdCancelled as
((localId: string) => boolean) | undefined;
expect(cancelHandler).toBeDefined();
expect(isLocalIdCancelled).toBeDefined();
// None of these localIds are in the queue or in the pre-enqueue
// preparing window, so every call falls into the fallback branch
// that records it as a possible dequeued-compact cancel. Simulate
// far more of these than could ever realistically be in flight at
// once (see the comment on `cancelledDequeuedLocalIds` in
// runOpencode.ts for why this branch is only reachable during a
// brief per-message ack race) to prove the tracking Set evicts its
// oldest entries instead of growing forever.
const localIds = Array.from({ length: 200 }, (_, i) => `unmatched-${i}`);
for (const localId of localIds) {
cancelHandler!(localId);
}
// The earliest entries must have been evicted...
expect(isLocalIdCancelled!('unmatched-0')).toBe(false);
// ...while a recent one is still tracked (delete-and-return: true
// once, then gone).
const lastLocalId = localIds[localIds.length - 1]!;
expect(isLocalIdCancelled!(lastLocalId)).toBe(true);
expect(isLocalIdCancelled!(lastLocalId)).toBe(false);
});
});
+202 -3
View File
@@ -79,10 +79,43 @@ export async function runOpencode(opts: {
// batches with different intent don't merge — the launcher uses null
// to mean "switch back to defaultBackendModel".
model: mode.model === null ? '__reset__' : mode.model ?? null,
modelReasoningEffort: mode.modelReasoningEffort ?? null
modelReasoningEffort: mode.modelReasoningEffort ?? null,
// Defense in depth: a compact item is always pushed via
// `pushIsolated` (never batches with siblings regardless of mode
// hash), but including `operation` here too means a prompt and a
// compact request could never be merged into one batch even if that
// isolation guard were ever bypassed.
operation: mode.operation ?? null
}));
const sessionWrapperRef: { current: OpencodeSession | null } = { current: null };
// Set by opencodeRemoteLauncher once the ACP backend + internal HTTP
// baseUrl are actually ready (remote mode only), and reset to false as
// early as possible whenever this session leaves remote mode
// (OpencodeRemoteLauncher's onLeavingRemote() override — see its doc
// comment on RemoteLauncherBase for exactly when that fires). While
// false, the `slash.kind === 'compact'` branch below must tell apart two
// situations that both look like "compactSupported is false, mode is
// 'remote'": a session that just entered remote mode and hasn't finished
// ACP initialize+session load/new yet (should queue /compact like a
// prompt), versus a session whose remote launcher is already tearing
// down (onLeavingRemote fired, `mode` hasn't flipped back to 'local' yet
// because that only happens once the whole launcher unwinds — see
// AgentSessionBase.onModeChange). `compactTeardownInProgress` below is
// what distinguishes them — a hostile-review sweep found that gating on
// `mode` alone (added to fix the first case) silently re-opened the
// second: it let /compact queue during the exact teardown window
// onLeavingRemote exists to protect, since `mode` stays 'remote'
// throughout it.
let compactSupported = false;
// True from the moment onCompactAvailabilityChange(false) fires (which,
// per onLeavingRemote's contract, only ever happens because remote mode
// is being left — never because remote just started) until this session
// next re-enters remote mode (see the wrapped `onModeChange` below).
// Only meaningful while `sessionWrapperRef.current?.mode === 'remote'`;
// harmless/stale otherwise since the mode check alone already rejects a
// genuinely local-mode session regardless of this flag's value.
let compactTeardownInProgress = false;
let currentPermissionMode: PermissionMode = opts.permissionMode ?? 'default';
let sessionModel: string | null = initialModel;
let sessionModelReasoningEffort: string | null = initialModelReasoningEffort;
@@ -134,6 +167,52 @@ export async function runOpencode(opts: {
// short-circuit when it resumes.
const preparingLocalIds = new Set<string>();
const cancelledBeforeEnqueue = new Set<string>();
// Mirrors `cancelledBeforeEnqueue` above, but for the other side of the
// queued-compact ack: `onCancelQueuedMessage` below can still fire for a
// localId that's neither in the queue nor in `preparingLocalIds`. Track
// it here and let the launcher consume it via `isLocalIdCancelled`
// (passed through opencodeLoop) so it can suppress the eventual
// "Compaction completed/failed" + Reasoning-block result if this really
// was that localId.
//
// Note on how narrow this window actually is: the hub only calls back
// into the CLI's `onCancelQueuedMessage` when its own DB lookup still
// finds the row queued (invoked_at IS NULL) — see
// `cancelQueuedMessage`'s Phase 1 in hub/src/sync/messageService.ts.
// `session.emitMessagesConsumed([localId])` a few lines below fires the
// "invoked" ack for the /compact message *before* it's pushed onto
// `messageQueue`, i.e. long before the launcher ever dequeues it and
// starts the REST call. So once that ack's DB write lands, every later
// cancel request short-circuits on the hub side and never reaches the
// CLI at all — this Set can only ever be populated during the brief
// network round trip between the CLI emitting that ack and the hub
// recording it, not while the compact REST call is actually running.
// That's an existing characteristic of the hub's first-write-wins
// queued-message protocol (present since Phase 1 of this feature, not
// something this change introduced or is trying to fix — a hub-side
// redesign of that protocol is out of scope here since it would affect
// cancel behavior for every flavor, not just OpenCode /compact).
//
// Nothing here distinguishes "this localId was actually a /compact
// message" from any other queued message whose cancel happened to land
// in that race window — the fallback branch below has no way to know.
// For a real /compact race, `isLocalIdCancelled` reads (and deletes) the
// entry once `runCompactOperation` checks it; for anything else, the
// entry would sit here unread for the rest of the process's life. Cap
// the Set (oldest-first eviction, relying on Set's insertion-order
// iteration) so a long-running session can't accumulate these forever —
// realistically at most a handful of entries would ever coexist, so this
// cap is a defensive bound, not something expected to trigger.
const MAX_CANCELLED_DEQUEUED_LOCAL_IDS = 50;
const cancelledDequeuedLocalIds = new Set<string>();
const addCancelledDequeuedLocalId = (localId: string): void => {
cancelledDequeuedLocalIds.add(localId);
while (cancelledDequeuedLocalIds.size > MAX_CANCELLED_DEQUEUED_LOCAL_IDS) {
const oldest = cancelledDequeuedLocalIds.values().next().value;
if (oldest === undefined) break;
cancelledDequeuedLocalIds.delete(oldest);
}
};
let userMessageChain: Promise<void> = Promise.resolve();
session.onUserMessage((message, localId) => {
@@ -167,6 +246,88 @@ export async function runOpencode(opts: {
modelReasoningEffort: sessionModelReasoningEffort
});
if (slash.kind === 'compact') {
// `compactSupported` alone conflates two different
// situations: a genuinely local-mode session (compact
// fundamentally can't run — there's no ACP backend to
// run it against) versus a session that's already in
// remote mode but hasn't finished ACP initialize +
// session load/new yet (onCompactAvailabilityChange(true)
// hasn't fired *yet*, but will shortly). A hostile-review
// sweep found the old code treated both the same way —
// an immediate not-yet-supported reply — even though a
// regular prompt sent in that exact same startup window
// queues normally and just waits.
//
// `sessionWrapperRef.current?.mode` (not the `session`
// variable in this closure, which is the lower-level
// ApiSessionClient without a `mode` field) is the actual
// OpencodeSession instance's mode — 'local' | 'remote',
// synced synchronously by onModeChange before either
// launcher starts (see AgentSessionBase). `undefined`
// (not yet set) falls through to the safe
// not-yet-supported default below, same as genuinely
// local mode.
//
// `mode !== 'remote'` alone isn't enough, though:
// `mode` stays 'remote' for the *entire* teardown
// window too (it only flips back to 'local' once the
// whole remote launcher has fully unwound), so without
// also checking `compactTeardownInProgress`, this would
// re-open queuing during exactly the window
// onLeavingRemote() exists to protect — a hostile-review
// sweep found this the first time this branch checked
// `mode` alone (see compactTeardownInProgress's
// declaration comment for the full distinction).
if (!compactSupported && (sessionWrapperRef.current?.mode !== 'remote' || compactTeardownInProgress)) {
if (localId) {
session.emitMessagesConsumed([localId], { clearQueuedThinkingGrace: true });
}
session.sendAgentMessage({
type: 'message',
message: '/compact is not yet supported in HAPI OpenCode sessions.',
id: randomUUID()
});
sessionWrapperRef.current?.onThinkingChange(false);
return;
}
// No manual emitMessagesConsumed here (unlike the
// synchronous 'handled' branch below): `messageQueue`
// (== session.queue, wired in AgentSessionBase's
// constructor — see sessionBase.ts) already acks
// automatically at dequeue time via `onBatchConsumed`,
// exactly like any regular prompt — `collectBatch()` in
// MessageQueue2.ts calls it right after shifting an
// item off the queue, which for /compact happens in
// opencodeRemoteLauncher.ts's dequeue loop. An earlier
// version of this branch (from when /compact ran via a
// trigger function invoked directly from this chain,
// bypassing the queue entirely) called
// `session.emitMessagesConsumed([localId])` manually
// right here, before the item was even queued — that
// stopped mattering for FIFO ordering once /compact
// moved to `pushIsolated` below, but it kept firing the
// hub ack immediately regardless, which is what actually
// broke cancellation of an already-queued-but-not-yet-
// dequeued /compact: the hub marked it invoked the
// instant it was queued, so `cancelByLocalId` in
// `onCancelQueuedMessage` below always found it already
// gone from the queue and could never remove it before
// that premature ack landed. Removing the manual call
// lets the automatic dequeue-time ack (and therefore
// `messageQueue.cancelByLocalId`) work the same way it
// already does for prompts — a queued /compact can now
// actually be cancelled before opencodeRemoteLauncher.ts
// dequeues it and calls `runCompactOperation()`.
//
// pushIsolated (not push): must never batch with a
// sibling prompt, but must still occupy its real FIFO
// position relative to prompts already queued ahead of
// it.
messageQueue.pushIsolated('', { ...buildMode(), operation: 'compact' }, localId);
return;
}
if (slash.kind !== 'passthrough') {
if (slash.updates) {
if (slash.updates.permissionMode !== undefined) {
@@ -246,7 +407,20 @@ export async function runOpencode(opts: {
logger.debug(`[opencode] cancelByLocalId(${localId}): marked for cancellation before enqueue`);
return true;
}
logger.debug(`[opencode] cancelByLocalId(${localId}): not found (best-effort)`);
// Not in the queue and not in the pre-enqueue preparing window. As
// explained where `cancelledDequeuedLocalIds` is declared above, the
// hub only calls this at all while its own row is still queued, so
// reaching this branch means we're in the brief race between our
// /compact ack (`emitMessagesConsumed`) being sent and the hub
// recording it — not, as the name might suggest, the compact REST
// call itself running. Remember it so the launcher can suppress the
// result if that's what this turns out to be; harmless if it doesn't
// match anything (just an unread entry that never gets consumed).
// Return value is unchanged from before this tracking existed — we
// don't actually know whether this cancelled anything real, so this
// stays "best-effort: not found".
addCancelledDequeuedLocalId(localId);
logger.debug(`[opencode] cancelByLocalId(${localId}): not found in queue; marked in case it lands in the compact ack race window (best-effort)`);
return false;
});
@@ -270,6 +444,7 @@ export async function runOpencode(opts: {
});
let crashed = false;
const notifyHubModeChange = createModeChangeHandler(session);
try {
await opencodeLoop({
@@ -285,14 +460,38 @@ export async function runOpencode(opts: {
resumeSessionId: opts.resumeSessionId,
hookServer,
hookUrl,
onModeChange: createModeChangeHandler(session),
onModeChange: (mode) => {
if (mode === 'remote') {
// A fresh remote entry is beginning (first-ever, or a
// local interlude ending) — whatever the previous
// remote attempt's teardown state was, it no longer
// applies. (The very first entry into remote mode,
// when `startingMode` is already 'remote', never calls
// onModeChange at all — see loopBase.ts — but this
// flag's initial `false` already covers that case.)
compactTeardownInProgress = false;
}
notifyHubModeChange(mode);
},
onReasoningEffortRollback: (effort) => {
sessionModelReasoningEffort = effort;
},
onSessionReady: (instance) => {
sessionWrapperRef.current = instance;
syncSessionMode();
},
onCompactAvailabilityChange: (available) => {
compactSupported = available;
if (!available) {
// onCompactAvailabilityChange(false) only ever fires
// from OpencodeRemoteLauncher's onLeavingRemote() (the
// old reset-on-next-local-entry was removed — see its
// declaration comment) — so reaching here always means
// "leaving remote", never "not ready yet".
compactTeardownInProgress = true;
}
},
isLocalIdCancelled: (localId) => cancelledDequeuedLocalIds.delete(localId)
});
} catch (error) {
crashed = true;
+8
View File
@@ -9,6 +9,14 @@ export interface OpencodeMode {
// "no change requested for this batch".
model?: string | null;
modelReasoningEffort?: string | null;
// Marks this queued item as a /compact request rather than a regular
// prompt turn. Pushed via `messageQueue.pushIsolated(...)` so it never
// batches with sibling prompts but still occupies its actual FIFO
// position — the launcher's dequeue loop branches on this instead of
// calling `backend.prompt()`, which keeps /compact from "cutting in
// line" ahead of prompts that were already queued when it arrived.
// `undefined` for normal prompts.
operation?: 'compact';
}
export type OpencodeHookEvent = {
@@ -0,0 +1,55 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
const constructorCalls: Array<{ command: string; args?: string[]; env?: Record<string, string> }> = [];
vi.mock('@/agent/backends/acp', () => ({
AcpSdkBackend: vi.fn().mockImplementation(function (
this: unknown,
opts: { command: string; args?: string[]; env?: Record<string, string> }
) {
constructorCalls.push(opts);
return { __opts: opts };
})
}));
import { allocateFreePort, createOpencodeBackend } from './opencodeBackend';
describe('allocateFreePort', () => {
it('resolves a bindable loopback port number', async () => {
const port = await allocateFreePort('127.0.0.1');
expect(typeof port).toBe('number');
expect(Number.isInteger(port)).toBe(true);
expect(port).toBeGreaterThan(0);
expect(port).toBeLessThan(65536);
});
it('releases the port so it can be reused by a later caller', async () => {
// Each call must close its probe socket before resolving, otherwise a
// consumer that immediately tries to bind the returned port (e.g. the
// spawned `opencode acp --port <port>` process) would collide with it.
const first = await allocateFreePort('127.0.0.1');
const second = await allocateFreePort('127.0.0.1');
expect(typeof second).toBe('number');
// Not asserting first !== second (OS may reuse immediately-freed ports),
// only that a second allocation does not hang or throw EADDRINUSE.
expect(first).toBeGreaterThan(0);
});
});
describe('createOpencodeBackend', () => {
beforeEach(() => {
constructorCalls.length = 0;
});
it('passes --port and --hostname args when provided', () => {
createOpencodeBackend({ cwd: '/tmp/x', port: 5555, hostname: '127.0.0.1' });
expect(constructorCalls[0]?.args).toEqual([
'acp', '--cwd', '/tmp/x', '--port', '5555', '--hostname', '127.0.0.1'
]);
});
it('omits --port/--hostname when not provided (backward compatible)', () => {
createOpencodeBackend({ cwd: '/tmp/x' });
expect(constructorCalls[0]?.args).toEqual(['acp', '--cwd', '/tmp/x']);
});
});
+42
View File
@@ -1,3 +1,4 @@
import { createServer } from 'node:net';
import { AcpSdkBackend } from '@/agent/backends/acp';
import { buildOpencodeEnv } from './config';
import { getInvokedCwd } from '@/utils/invokedCwd';
@@ -12,11 +13,52 @@ function filterEnv(env: NodeJS.ProcessEnv): Record<string, string> {
return result;
}
/**
* Reserves a free TCP port on the given loopback host by binding an
* ephemeral probe socket (`listen(0)`) and immediately closing it before
* resolving. `opencode acp` does not announce the port it actually bound
* when launched with `--port 0` (verified 2026-07-30 — no port appears in
* stdout/stderr even at DEBUG log level), so HAPI must pick the port itself
* and hand it to the subprocess explicitly via `--port`. There is a
* theoretical reuse race between this function releasing the port and the
* subprocess binding it, but both sides are loopback-only local processes,
* so the practical risk is low.
*/
export function allocateFreePort(hostname = '127.0.0.1'): Promise<number> {
return new Promise((resolve, reject) => {
const probe = createServer();
probe.once('error', reject);
probe.listen(0, hostname, () => {
const address = probe.address();
const port = address && typeof address === 'object' ? address.port : null;
probe.close((closeError) => {
if (closeError) {
reject(closeError);
return;
}
if (port === null) {
reject(new Error('Failed to allocate a free port for the OpenCode ACP server'));
return;
}
resolve(port);
});
});
});
}
export function createOpencodeBackend(opts: {
cwd?: string;
port?: number;
hostname?: string;
}): AcpSdkBackend {
const env = buildOpencodeEnv();
const args = ['acp', '--cwd', opts.cwd ?? getInvokedCwd()];
if (opts.port !== undefined) {
args.push('--port', String(opts.port));
}
if (opts.hostname !== undefined) {
args.push('--hostname', opts.hostname);
}
return new AcpSdkBackend({
command: 'opencode',
@@ -0,0 +1,418 @@
import { describe, expect, it, vi } from 'vitest';
import { fetchCompactionSummary, splitProviderModel, triggerOpencodeCompact } from './opencodeCompactBridge';
// `signal` is a required field (see OpencodeCompactCallOpts's doc comment) —
// most tests below don't exercise abort behavior at all, so this is a
// signal that's simply never aborted, just satisfying the type.
const noSignal = new AbortController().signal;
describe('splitProviderModel', () => {
it('splits a combined "provider/model" wire id on the first slash', () => {
expect(splitProviderModel('ollama/qwen3.6:35b-a3b-q8_0-mtp')).toEqual({
providerId: 'ollama',
modelId: 'qwen3.6:35b-a3b-q8_0-mtp'
});
});
it('keeps everything after the first slash as the modelId (model ids may contain slashes)', () => {
expect(splitProviderModel('openrouter/anthropic/claude-sonnet-4-5')).toEqual({
providerId: 'openrouter',
modelId: 'anthropic/claude-sonnet-4-5'
});
});
it('returns null for null/undefined input', () => {
expect(splitProviderModel(null)).toBeNull();
expect(splitProviderModel(undefined)).toBeNull();
});
it('returns null when there is no slash', () => {
expect(splitProviderModel('no-slash-here')).toBeNull();
});
it('returns null for a leading or trailing slash (empty provider or model)', () => {
expect(splitProviderModel('/model-only')).toBeNull();
expect(splitProviderModel('provider-only/')).toBeNull();
});
});
describe('triggerOpencodeCompact', () => {
it('posts to /session/:id/summarize with the required providerID/modelID payload and no artificial timeout', async () => {
const fetchImpl = vi.fn(async (url: string, init?: RequestInit) => {
expect(url).toBe('http://127.0.0.1:48273/session/ses_abc/summarize');
expect(init?.method).toBe('POST');
expect(JSON.parse(init?.body as string)).toEqual({
providerID: 'ollama',
modelID: 'qwen3.6:35b-a3b-q8_0-mtp'
});
// `signal` is always attached now (required — see
// OpencodeCompactCallOpts), but it must not act as a deadline on
// its own: the caller here never aborts it, so the request must
// run to completion regardless of how long it legitimately takes
// (90s+ verified against SER8, 2026-07-30).
expect(init?.signal).toBe(noSignal);
// Bun's global fetch() hardcodes a 5-minute idle timeout that
// fires even with no AbortSignal at all (verified via isolated
// E2E against SER8, 2026-07-30 — a real ~250s compaction call
// failed with "The operation timed out"; see oven-sh/bun#16682).
// The only documented workaround is this Bun-specific,
// non-standard `timeout: false` fetch option — needed regardless
// of whether the caller's own `signal` ever fires.
expect((init as unknown as { timeout?: boolean })?.timeout).toBe(false);
return new Response(null, { status: 204 });
});
const result = await triggerOpencodeCompact({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
providerId: 'ollama',
modelId: 'qwen3.6:35b-a3b-q8_0-mtp',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ ok: true });
expect(fetchImpl).toHaveBeenCalledTimes(1);
});
it('reports a structured failure when the server responds non-ok (e.g. the v2 compact stub 503)', async () => {
const fetchImpl = vi.fn(async () => new Response(
JSON.stringify({ _tag: 'ServiceUnavailableError', message: 'Session compact is not available yet', service: 'session.compact' }),
{ status: 503 }
));
const result = await triggerOpencodeCompact({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
providerId: 'ollama',
modelId: 'qwen3.6:35b-a3b-q8_0-mtp',
fetchImpl,
signal: noSignal
});
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.error).toContain('503');
expect(result.error).toContain('not available yet');
}
});
it('reports a structured failure when the network call throws', async () => {
const fetchImpl = vi.fn(async () => {
throw new Error('ECONNREFUSED');
});
const result = await triggerOpencodeCompact({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
providerId: 'ollama',
modelId: 'qwen3.6:35b-a3b-q8_0-mtp',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ ok: false, error: 'ECONNREFUSED' });
});
it('URL-encodes the sessionId in the path', async () => {
const fetchImpl = vi.fn(async (url: string) => {
expect(url).toBe('http://127.0.0.1:48273/session/ses%20with%20space/summarize');
return new Response(null, { status: 204 });
});
await triggerOpencodeCompact({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses with space',
providerId: 'ollama',
modelId: 'model-x',
fetchImpl,
signal: noSignal
});
expect(fetchImpl).toHaveBeenCalledTimes(1);
});
it('forwards an AbortSignal to fetch when provided, so a caller can interrupt an in-flight request', async () => {
const controller = new AbortController();
const fetchImpl = vi.fn(async (_url: string, init?: RequestInit) => {
expect(init?.signal).toBe(controller.signal);
return new Response(null, { status: 204 });
});
const result = await triggerOpencodeCompact({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
providerId: 'ollama',
modelId: 'qwen3.6:35b-a3b-q8_0-mtp',
fetchImpl,
signal: controller.signal
});
expect(result).toEqual({ ok: true });
});
it('resolves with a structured failure (not a hang or uncaught rejection) when the signal aborts mid-request', async () => {
const controller = new AbortController();
// Mirrors how a real fetch() rejects on abort: the promise only
// settles once the signal actually fires, not before.
const fetchImpl = vi.fn((_url: string, init?: RequestInit) => new Promise<Response>((_resolve, reject) => {
init?.signal?.addEventListener('abort', () => {
reject(new DOMException('The operation was aborted.', 'AbortError'));
});
}));
const resultPromise = triggerOpencodeCompact({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
providerId: 'ollama',
modelId: 'qwen3.6:35b-a3b-q8_0-mtp',
fetchImpl,
signal: controller.signal
});
controller.abort();
const result = await resultPromise;
expect(result.ok).toBe(false);
});
});
describe('fetchCompactionSummary', () => {
it('extracts the text part of the assistant message that follows the compaction marker (matched via parentID)', async () => {
const fetchImpl = vi.fn(async (url: string) => {
expect(url).toBe('http://127.0.0.1:48273/session/ses_abc/message');
return new Response(JSON.stringify([
{ info: { id: 'msg_1', role: 'user' }, parts: [{ id: 'prt_1', type: 'text', text: 'hello' }] },
{ info: { id: 'msg_2', role: 'assistant' }, parts: [{ id: 'prt_2', type: 'text', text: 'hi there' }] },
{ info: { id: 'msg_3', role: 'user' }, parts: [{ id: 'prt_3', type: 'compaction', auto: false }] },
{
info: { id: 'msg_4', role: 'assistant', parentID: 'msg_3', summary: true },
parts: [
{ id: 'prt_4a', type: 'step-start' },
{ id: 'prt_4b', type: 'reasoning', text: 'thinking about the summary' },
{ id: 'prt_4c', type: 'text', text: '## Objective\n- Did the thing' },
{ id: 'prt_4d', type: 'step-finish' }
]
}
]), { status: 200 });
});
const result = await fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ found: true, text: '## Objective\n- Did the thing' });
});
it('falls back to positional adjacency when the assistant message has no parentID', async () => {
const fetchImpl = vi.fn(async () => new Response(JSON.stringify([
{ info: { id: 'msg_3', role: 'user' }, parts: [{ id: 'prt_3', type: 'compaction', auto: false }] },
{ info: { id: 'msg_4', role: 'assistant' }, parts: [{ id: 'prt_4', type: 'text', text: 'summary via positional match' }] }
]), { status: 200 }));
const result = await fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ found: true, text: 'summary via positional match' });
});
it('rejects a parentID/positional match whose role is not assistant, even if it happens to carry a text part', async () => {
// Both the parentID-linked entry AND the positionally-adjacent entry
// have a `type:'text'` part here, but neither is role:'assistant' —
// the safe fallback (found:false) must win rather than surfacing
// whatever unrelated text these entries happen to carry.
const fetchImpl = vi.fn(async () => new Response(JSON.stringify([
{ info: { id: 'msg_3', role: 'user' }, parts: [{ id: 'prt_3', type: 'compaction', auto: false }] },
{ info: { id: 'msg_4', role: 'user', parentID: 'msg_3' }, parts: [{ id: 'prt_4', type: 'text', text: 'not actually a summary' }] }
]), { status: 200 }));
const result = await fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ found: false });
});
it('concatenates multiple text parts in order instead of only taking the first', async () => {
const fetchImpl = vi.fn(async () => new Response(JSON.stringify([
{ info: { id: 'msg_3', role: 'user' }, parts: [{ id: 'prt_3', type: 'compaction', auto: false }] },
{
info: { id: 'msg_4', role: 'assistant', parentID: 'msg_3' },
parts: [
{ id: 'prt_4a', type: 'text', text: '## Objective\n' },
{ id: 'prt_4b', type: 'step-finish' },
{ id: 'prt_4c', type: 'text', text: '- Did the thing' }
]
}
]), { status: 200 }));
const result = await fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ found: true, text: '## Objective\n- Did the thing' });
});
it('returns found:false when no compaction marker exists', async () => {
const fetchImpl = vi.fn(async () => new Response(JSON.stringify([
{ info: { id: 'msg_1', role: 'user' }, parts: [{ id: 'prt_1', type: 'text', text: 'hello' }] },
{ info: { id: 'msg_2', role: 'assistant' }, parts: [{ id: 'prt_2', type: 'text', text: 'hi' }] }
]), { status: 200 }));
const result = await fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ found: false });
});
it('returns found:false when the marker is the last message (no following assistant message yet)', async () => {
const fetchImpl = vi.fn(async () => new Response(JSON.stringify([
{ info: { id: 'msg_3', role: 'user' }, parts: [{ id: 'prt_3', type: 'compaction', auto: false }] }
]), { status: 200 }));
const result = await fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ found: false });
});
it('returns found:false when the following assistant message has no text part', async () => {
const fetchImpl = vi.fn(async () => new Response(JSON.stringify([
{ info: { id: 'msg_3', role: 'user' }, parts: [{ id: 'prt_3', type: 'compaction', auto: false }] },
{ info: { id: 'msg_4', role: 'assistant', parentID: 'msg_3' }, parts: [{ id: 'prt_4', type: 'step-finish' }] }
]), { status: 200 }));
const result = await fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ found: false });
});
it('returns found:false on a non-ok response', async () => {
const fetchImpl = vi.fn(async () => new Response(null, { status: 500 }));
const result = await fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ found: false });
});
it('returns found:false when the response is not valid JSON / not an array', async () => {
const fetchImpl = vi.fn(async () => new Response('not json', { status: 200 }));
const result = await fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ found: false });
});
it('returns found:false when the network call throws', async () => {
const fetchImpl = vi.fn(async () => {
throw new Error('ECONNREFUSED');
});
const result = await fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ found: false });
});
it('picks the LAST compaction marker when there are multiple (a session may be compacted more than once)', async () => {
const fetchImpl = vi.fn(async () => new Response(JSON.stringify([
{ info: { id: 'msg_1', role: 'user' }, parts: [{ id: 'prt_1', type: 'compaction', auto: false }] },
{ info: { id: 'msg_2', role: 'assistant', parentID: 'msg_1' }, parts: [{ id: 'prt_2', type: 'text', text: 'first summary' }] },
{ info: { id: 'msg_3', role: 'user' }, parts: [{ id: 'prt_3', type: 'text', text: 'more chat' }] },
{ info: { id: 'msg_4', role: 'user' }, parts: [{ id: 'prt_4', type: 'compaction', auto: false }] },
{ info: { id: 'msg_5', role: 'assistant', parentID: 'msg_4' }, parts: [{ id: 'prt_5', type: 'text', text: 'second summary' }] }
]), { status: 200 }));
const result = await fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: noSignal
});
expect(result).toEqual({ found: true, text: 'second summary' });
});
it('forwards an AbortSignal to fetch when provided, so a caller can interrupt an in-flight request', async () => {
const controller = new AbortController();
const fetchImpl = vi.fn(async (_url: string, init?: RequestInit) => {
expect(init?.signal).toBe(controller.signal);
return new Response(JSON.stringify([]), { status: 200 });
});
await fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: controller.signal
});
expect(fetchImpl).toHaveBeenCalledTimes(1);
});
it('resolves with found:false (not a hang or uncaught rejection) when the signal aborts mid-request', async () => {
// Reproduces the exact gap a PR-review round found: the POST
// (triggerOpencodeCompact) had a signal wired through in an earlier
// round, but this GET — which runs right after it inside
// runCompactOperation() — did not, so Stop/switch-to-local could
// still block on this call even after that fix.
const controller = new AbortController();
const fetchImpl = vi.fn((_url: string, init?: RequestInit) => new Promise<Response>((_resolve, reject) => {
init?.signal?.addEventListener('abort', () => {
reject(new DOMException('The operation was aborted.', 'AbortError'));
});
}));
const resultPromise = fetchCompactionSummary({
baseUrl: 'http://127.0.0.1:48273',
sessionId: 'ses_abc',
fetchImpl,
signal: controller.signal
});
controller.abort();
const result = await resultPromise;
expect(result).toEqual({ found: false });
});
});
@@ -0,0 +1,221 @@
export type OpencodeCompactResult =
| { ok: true; summaryText?: string }
| { ok: false; error: string };
export type CompactionSummaryResult =
| { found: true; text: string }
| { found: false };
/** Minimal fetch-shaped function signature, kept narrower than `typeof fetch` so tests can pass a plain `vi.fn()` without matching runtime-specific extras (e.g. Bun's `fetch.preconnect`). */
export type FetchLike = (url: string, init?: RequestInit) => Promise<Response>;
/**
* `RequestInit` extended with Bun's non-standard `timeout` fetch option
* (absent from `bun-types` / the standard fetch typings — see the
* `triggerOpencodeCompact` doc comment for why it's needed). Narrower than
* `Record<string, unknown>` so the cast below can't silently accept an
* unrelated typo'd option name.
*/
type BunFetchInit = RequestInit & { timeout?: false };
/**
* Every OpenCode-side HTTP call `runCompactOperation()` in
* opencodeRemoteLauncher.ts makes on behalf of a single /compact operation
* must extend this — `signal` is **required**, not optional. That launcher
* owns exactly one `AbortController` for the operation's whole lifecycle
* (`compactAbortController`, aborted by `handleAbort()` on Stop/switch/exit)
* and threads its `.signal` through every step so a user-initiated
* interruption actually reaches whichever HTTP call happens to be in flight
* at the time.
*
* This was previously opt-in (`signal?: AbortSignal`) on each function
* individually, which is exactly how a real regression happened: a second
* PR-review round later found that `triggerOpencodeCompact` (the POST) had
* been wired up but `fetchCompactionSummary` (the GET that runs right
* after it) had not, because nothing forced it. Making `signal` a required
* field of a shared base type means the compiler catches a future third
* HTTP step *implemented as a function in this file* without one — it can't
* stop someone from bypassing this file entirely with an inline `fetch()`
* call in `runCompactOperation()`, so this is a guardrail for the pattern
* this file establishes, not an architectural boundary enforced repo-wide.
*/
export type OpencodeCompactCallOpts = {
baseUrl: string;
sessionId: string;
signal: AbortSignal;
};
/**
* Splits an ACP-reported combined model id (e.g. `"ollama/qwen3.6:35b-a3b-q8_0-mtp"`)
* into the separate `providerId`/`modelId` pair required by OpenCode's internal
* `POST /session/:id/summarize` payload. Only the first `/` is treated as the
* separator — model ids may themselves contain slashes (e.g. OpenRouter-style
* `"openrouter/anthropic/claude-sonnet-4-5"`).
*/
export function splitProviderModel(combined: string | null | undefined): { providerId: string; modelId: string } | null {
if (!combined) return null;
const separatorIndex = combined.indexOf('/');
if (separatorIndex <= 0 || separatorIndex === combined.length - 1) return null;
return {
providerId: combined.slice(0, separatorIndex),
modelId: combined.slice(separatorIndex + 1)
};
}
/**
* Triggers OpenCode's native AI-compaction for a session by calling the
* legacy `POST /session/:id/summarize` route on the `opencode acp`
* subprocess's internal HTTP API.
*
* This is NOT `POST /api/session/:id/compact` — that v2-API route is an
* unimplemented stub in opencode 1.18.9 and always returns 503
* ("Session compact is not available yet"). `summarize` is the route that
* actually performs native AI compaction (verified 2026-07-30: triggering it
* appends a real `{"type":"compaction"}` message part to the session, and
* streams `agent_thought_chunk` ACP notifications while the model works).
*
* `providerID`/`modelID` are required by the endpoint (400 if omitted).
* The response can legitimately take several minutes to arrive for slow
* models, so by default no deadline is applied here, mirroring how
* `AcpSdkBackend.prompt()` uses `timeoutMs: Infinity` for `session/prompt`.
* `signal` (required — see `OpencodeCompactCallOpts`) is a caller-driven
* abort, not a deadline, so it's orthogonal to the Bun timeout workaround
* below (both apply at once).
*
* Omitting the `timeout: false` option below is NOT enough under Bun: Bun's
* global `fetch()` hardcodes its own idle timeout (~5 minutes) that fires
* independently of any AbortSignal (verified 2026-07-30 via isolated E2E
* against SER8 — a real ~250s compaction call was killed client-side with
* "The operation timed out" even with no signal attached; see upstream
* report oven-sh/bun#16682). The only documented workaround is the
* non-standard `timeout: false` fetch option Bun itself recognizes (absent
* from the standard `RequestInit` typings, hence the cast below) — kept
* unconditionally regardless of `signal` also being set.
*/
export async function triggerOpencodeCompact(opts: OpencodeCompactCallOpts & {
providerId: string;
modelId: string;
fetchImpl?: FetchLike;
}): Promise<OpencodeCompactResult> {
const fetchFn: FetchLike = opts.fetchImpl ?? fetch;
const url = `${opts.baseUrl}/session/${encodeURIComponent(opts.sessionId)}/summarize`;
try {
const init: BunFetchInit = {
method: 'POST',
headers: { 'content-type': 'application/json' },
body: JSON.stringify({ providerID: opts.providerId, modelID: opts.modelId }),
// Bun-specific: disables Bun's hardcoded ~5min fetch timeout.
timeout: false,
signal: opts.signal
};
const response = await fetchFn(url, init as RequestInit);
if (!response.ok) {
const text = await response.text().catch(() => '');
return {
ok: false,
error: `OpenCode compact request failed (${response.status}): ${text.slice(0, 300)}`
};
}
return { ok: true };
} catch (error) {
return {
ok: false,
error: error instanceof Error ? error.message : String(error)
};
}
}
type OpencodeMessagePart = { type?: unknown; text?: unknown };
type OpencodeMessageEntry = {
info?: { id?: unknown; role?: unknown; parentID?: unknown; summary?: unknown };
parts?: unknown;
};
function isObjectRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null;
}
/**
* Only an assistant message is a plausible summary carrier — without this
* check, an unrelated adjacent/linked entry that happens to carry a `text`
* part (e.g. another user message) could silently surface as the "summary",
* bypassing the safe "not found -> skip" fallback this function exists to
* provide. `info.summary === true` (observed on the real compaction
* response) is a stronger corroborating signal when present, but role is the
* one check we always enforce.
*/
function isAssistantSummaryCandidate(entry: OpencodeMessageEntry | undefined): boolean {
return entry?.info?.role === 'assistant';
}
/** Concatenates every `type:'text'` part in order — a summary can arrive as more than one text segment, and taking only the first would silently truncate it. */
function extractTextPart(entry: OpencodeMessageEntry | undefined): string | null {
if (!entry || !Array.isArray(entry.parts)) return null;
const texts = (entry.parts as unknown[])
.filter((part): part is OpencodeMessagePart => isObjectRecord(part) && part.type === 'text' && typeof part.text === 'string')
.map((part) => part.text as string);
return texts.length > 0 ? texts.join('') : null;
}
/**
* After a successful `triggerOpencodeCompact`, OpenCode's session history
* contains a `{"type":"compaction"}` marker message (role `user`, no text)
* followed by an assistant message whose `text` part holds the actual
* summary OpenCode generated (verified 2026-07-30 via isolated E2E: parts
* were `['step-start','reasoning','text','step-finish']`). This fetches the
* message list and extracts that text so HAPI can show it as a "Reasoning"
* block instead of leaving the summary invisible.
*
* Looks for the assistant message via its `parentID` pointing at the marker
* first (robust to the API returning messages in an order other than
* creation order), falling back to simple positional adjacency (the very
* next array entry) if no `parentID` link is present. If a session has been
* compacted more than once, only the most recent marker is considered.
*
* Never throws — any failure (network error, unexpected response shape, no
* marker found, no text part found, or `signal` — see `OpencodeCompactCallOpts`
* — firing mid-request) resolves to `{ found: false }` so the caller can
* silently skip showing the summary rather than surfacing an error for what
* is a purely cosmetic enhancement.
*/
export async function fetchCompactionSummary(opts: OpencodeCompactCallOpts & {
fetchImpl?: FetchLike;
}): Promise<CompactionSummaryResult> {
const fetchFn: FetchLike = opts.fetchImpl ?? fetch;
const url = `${opts.baseUrl}/session/${encodeURIComponent(opts.sessionId)}/message`;
try {
const response = await fetchFn(url, { method: 'GET', signal: opts.signal });
if (!response.ok) return { found: false };
const data: unknown = await response.json().catch(() => null);
if (!Array.isArray(data)) return { found: false };
const entries = data as OpencodeMessageEntry[];
let markerIndex = -1;
for (let i = entries.length - 1; i >= 0; i--) {
const parts = entries[i]?.parts;
if (Array.isArray(parts) && parts.some((part) => isObjectRecord(part) && part.type === 'compaction')) {
markerIndex = i;
break;
}
}
if (markerIndex === -1) return { found: false };
const markerId = entries[markerIndex]?.info?.id;
const byParentId = typeof markerId === 'string'
? entries.find((entry) => entry.info?.parentID === markerId && isAssistantSummaryCandidate(entry))
: undefined;
const positionalCandidate = entries[markerIndex + 1];
const byPosition = isAssistantSummaryCandidate(positionalCandidate) ? positionalCandidate : undefined;
const text = extractTextPart(byParentId) ?? extractTextPart(byPosition);
return text !== null ? { found: true, text } : { found: false };
} catch {
return { found: false };
}
}
+10 -4
View File
@@ -119,15 +119,19 @@ describe('resolveOpencodeSlashCommand', () => {
}
});
it('returns a not-yet-supported message for /clear and /compact', () => {
it('returns a not-yet-supported message for /clear', () => {
expect(resolveOpencodeSlashCommand('/clear', state)).toEqual({
kind: 'handled',
message: '/clear is not yet supported in HAPI OpenCode sessions.'
});
expect(resolveOpencodeSlashCommand('/compact', state)).toEqual({
kind: 'handled',
message: '/compact is not yet supported in HAPI OpenCode sessions.'
});
it('resolves /compact to a dedicated kind so the launcher can bridge to native compaction asynchronously', () => {
expect(resolveOpencodeSlashCommand('/compact', state)).toEqual({ kind: 'compact' });
});
it('resolves /compact the same way regardless of trailing arguments (compaction takes no arguments)', () => {
expect(resolveOpencodeSlashCommand('/compact now please', state)).toEqual({ kind: 'compact' });
});
it('expands custom OpenCode command prompts', () => {
@@ -163,6 +167,8 @@ describe('resolveOpencodeSlashCommand', () => {
expect(help.message).toContain('Supported OpenCode slash commands');
expect(help.message).toContain('/plan');
expect(help.message).toContain('/permissions');
expect(help.message).toContain('/compact` — compact (summarize) the OpenCode session context (remote sessions only)');
expect(help.message).toContain('/clear` is not yet supported');
}
});
+13 -2
View File
@@ -18,6 +18,12 @@ const OPENCODE_INIT_PROMPT = [
export type OpencodeSlashResolution =
| { kind: 'passthrough' }
// /compact needs an async round trip to OpenCode's internal REST API
// (native AI compaction, can take 90s+) and a "Compaction
// started/completed/failed" event sequence, which doesn't fit the
// synchronous 'handled' shape below. The launcher (runOpencode.ts)
// intercepts this kind and drives that flow itself.
| { kind: 'compact' }
| {
kind: 'handled';
message: string;
@@ -163,7 +169,11 @@ export function resolveOpencodeSlashCommand(
};
}
if (command === 'clear' || command === 'compact') {
if (command === 'compact') {
return { kind: 'compact' };
}
if (command === 'clear') {
return {
kind: 'handled',
message: `/${command} is not yet supported in HAPI OpenCode sessions.`
@@ -193,11 +203,12 @@ export function resolveOpencodeSlashCommand(
'- `/plan off` — return to default permission mode',
'- `/default` — return to default permission mode',
'- `/init [extra]` — generate or refresh AGENTS.md for this project',
'- `/compact` — compact (summarize) the OpenCode session context (remote sessions only)',
'',
'Model, reasoning effort, and permission mode have dedicated buttons in the composer. ' +
'You can still type `/model`, `/reasoning`, or `/permissions` if you prefer.',
'',
'`/clear` and `/compact` are not yet supported in HAPI OpenCode sessions.',
'`/clear` is not yet supported in HAPI OpenCode sessions.',
'',
'Custom commands from `~/.config/opencode/command` or `.opencode/command` are expanded before sending.'
].join('\n')