Skip to content

fix(opencode): end an ACP prompt when its session goes idle - #60089

Open
2SinceJuly wants to merge 2 commits into
anomalyco:devfrom
2SinceJuly:acp-idle-outcome
Open

2SinceJuly wants to merge 2 commits into
anomalyco:devfrom
2SinceJuly:acp-idle-outcome

Conversation

@2SinceJuly

@2SinceJuly 2SinceJuly commented Oct 29, 2026 •

Copy link
Copy Markdown

Issue for this PR

Closes #60065

Type of change

  • Bug fix

What does this PR do?

When a durable write fails during an ACP turn (for example SQLITE_FULL), the session.execution.* outcome can be lost. The adapter only ended a prompt on that event, so session/prompt stayed pending and every later prompt was refused with Session already has an active ACP prompt.

  • packages/cli/src/acp/turn.ts: once the prompt is delivered, the turn also waits on POST /api/experimental/session/:id/wait. If the session goes idle before an outcome event arrives, the turn ends through the normal settle path, which still reports a step or execution error that was recorded.
  • packages/cli/src/acp/translate.ts: a session.execution.failed that arrives before session.inbox.delivered now ends the turn instead of being ignored. That turn can never deliver its prompt, so there was nothing left to wait for.

Normal turns are unaffected: the terminal event is published before awaitIdle resolves, so an event still ends the turn first.

Only the ACP side is in scope here. Core storage retries stay in fix/disk-full-session-recovery / #53704.

How did you verify your code works?

  • bun test test/acp from packages/cli: 102 pass.
  • Two new cases in test/acp/turn.test.ts cover the lost outcome and the pre-delivery failure. Both fail without the source change (the first times out at 5s, the second never gets the error) and pass with it.
  • bun run typecheck from packages/cli: no type errors.
  • bun run check at the repo root passes locally.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Added by the opencode bot (AI-generated).

Related: #52796

A failed durable write can drop session.execution.* during a turn, so the
ACP adapter waited for an outcome that never arrived and session/prompt
stayed pending. Also end the turn when session.wait reports idle, and
report a session.execution.failed that arrives before the prompt is
delivered instead of ignoring it forever.

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the hang from #60065, but it causes a regression in normal turns, so it needs changes before it can merge.

I ran the issue's fault-injection script against opencode acp from source on the base and on this branch. With the PR, both locked-session scenarios now resolve and the next prompt is accepted. In a healthy turn where the model hits the output limit, though, the base returns the core's output-limit error (Response still truncated after two output token limit continuations) every time (16/16). With this branch, every turn (17/17) returns stopReason: "max_tokens" with no error. All three model steps still run; only the error is lost. The reason: the session/wait long poll returns on a separate HTTP request, and it reaches the client before the terminal event on the SSE stream. The idle path then ends the turn and drops whatever hasn't arrived yet: the session.execution.failed, and possibly the last text, usage, finish reason or permission asks. That means real provider failures can now be reported to ACP clients as successful turns.

A related problem: when idle wins and no outcome was recorded, the turn ends as end_turn. In the lost-write scenario this reports a turn that failed as a success, and the issue expects an error there.

Idle should be a fallback that runs after the event stream has caught up, not a race against it. For example, after wait resolves, keep reading events until a terminal event or a short drain timeout. Only then end the turn, and with an error, since its outcome is unknown. Please also add a test where idle resolves before a session.execution.failed that is still in flight, and check that the error is reported.

Public API: unchanged. The adapter only starts calling the existing POST /api/experimental/session/:sessionID/wait endpoint.

Related: #52796

Comment thread packages/cli/src/acp/turn.ts Resolved
`session/wait` answers on its own request and can reach the client before the
terminal event on the SSE stream, so ending the turn as soon as it reports idle
dropped `session.execution.failed` along with the trailing text and usage. A
healthy turn that hit the output limit was reported as `max_tokens` with no
error. Idle is now a fallback: keep folding events until a terminal one arrives
or the stream stays quiet for the drain timeout. When nothing ever reports how
the turn ended, fail the prompt instead of completing it as a success.
@2SinceJuly

Copy link
Copy Markdown
Author

Both points are addressed in fe790e96.

Idle is a fallback now, not a terminator. When session/wait resolves, the turn stops racing and drains the subscription: it keeps folding events until one of them is terminal, or the stream stays quiet for IdleDrainTimeout (1s, overridable through Context.Reference like CancelDrainTimeout). So session.execution.failed, the last text, usage and finish reason all reach the client even when wait wins the race, and a healthy turn that hits the output limit reports the core's error again instead of stopReason: "max_tokens".

A lost outcome is an error, not a success. If the drain ends without a terminal event — the dropped-durable-write case — the turn records executionError: { type: "session.outcome-lost", message: "The session went idle before it reported how the turn ended" } and returns failed, so settle fails the request through the same ACPTranslate.failure path instead of falling through to end_turn.

Tests in packages/cli/test/acp/turn.test.ts:

  • reports a lost outcome when the session goes idle without a terminal event — this replaces the old ends the turn when the session goes idle… case, which asserted the behaviour you flagged. Now it expects -32603 with the message above.
  • reports the outcome that lands after the session goes idle — the regression you measured: idle resolves, then session.execution.failed arrives; the prompt fails with that error rather than reporting a successful turn.
  • The existing reports an execution failure that arrives before the prompt is delivered still passes.

bun test test/acp is 103/103, and bun run check from the root passes (35/35 tasks). I ran it with Node 22 on PATH; under the default Node 20 the oxlint JS plugin in script/oxlint/anti-slop fails to load with ERR_UNKNOWN_FILE_EXTENSION ".ts" before it reaches our files.

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new commit fixes the race and the lost-outcome reporting I raised earlier.

I re-ran the #60065 fault-injection script against this head:

  • Mid-turn lost outcome: now fails after about 1s with The session went idle before it reported how the turn ended, and the next prompt is accepted.
  • Pre-delivery failure: reports the SQLITE_FULL error, and the session isn't locked afterwards.
  • Healthy output-limit turns: 10/10 again end with the core's output-limit error, with all text delivered before the response. The earlier commit reported max_tokens here.

bun test test/acp passes (103 tests) and tsc -b is clean.

Public API: unchanged. The adapter only calls the existing POST /api/experimental/session/:sessionID/wait.

A minor note, not blocking: the idle drain only times out after 1s of silence on the whole event subscription. If the outcome is lost while another session on the same server keeps streaming, this prompt stays open until that stream goes quiet.

@2SinceJuly

Copy link
Copy Markdown
Author

Confirmed against fe790e968a: when session/wait resolves, the turn drains the event subscription under IdleDrainTimeout before ending, and an idle drain with no terminal event reports The session went idle before it reported how the turn ended instead of a silent success — the ordering your fault-injection script exercised. Resolving the thread.

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new push only rewrites the commits. The code is identical to the head I last reviewed, so my earlier result still holds: both #60065 scenarios resolve and the session isn't locked afterwards, healthy turns still report their real outcome, and the ACP tests and type check pass. I have nothing new to raise.

Public API: unchanged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ACP: session/prompt never returns after the disk fills up during a turn

1 participant