Skip to content

Vouch agent-initiated ACP turns so async job delivery renders - #2123

Closed
bradhallett wants to merge 2 commits into
get-bb:mainfrom
bradhallett:fix/acp-agent-initiated-turns
Closed

Vouch agent-initiated ACP turns so async job delivery renders#2123
bradhallett wants to merge 2 commits into
get-bb:mainfrom
bradhallett:fix/acp-agent-initiated-turns

Conversation

@bradhallett

Copy link
Copy Markdown
Contributor

What was wrong

ACP agents can emit session/update work output with no prompt in flight — an agent-initiated turn (OMP's async-job auto-delivery is the concrete case: a background job settles, the agent resumes and streams a follow-up unprompted). ACP brackets no turn for that, and the bridge forwarded the updates unbracketed, so the translator classified them unhandled/onlyIfNoTurn and they never reached the thread: the agent's output was silently dropped. Root cause and evidence in #2122 (DB event gap, wire probe, pre-fix permalinks).

What changed

plugins/provider-acp/src/bridge/bridge.ts — the bridge now vouches agent-initiated turns itself, the shape docs/provider-bridge-protocol.md turn lifecycle rule 3 sanctions (and the compaction path already uses):

  • Idle agent-work updates (agent_message_chunk, agent_thought_chunk, tool_call, tool_call_update, plan) open a vouched turn via the existing bridge turn-started command → turn.open.
  • A quiet window (SPONTANEOUS_TURN_IDLE_TIMEOUT_MS, 120 s, re-armed per chunk) settles the turn as end_turn; a still-open one settles before the next bb-initiated turn or compaction starts; thread/stop and session teardown cancel/clean it. A slow provider can split into multiple vouched turns; output is never lost.
  • Non-work updates (available_commands_update, usage_update, …) stay noise — no phantom turns. user_message_chunk stays classified as input-echo noise, so the replayed async result creates no phantom user row.

No wire/protocol changes; translator untouched (stays context-free). fake-acp-agent.mjs gains spontaneous-stream:N (post-prompt unsolicited user_message_chunk + N agent chunks) and spontaneous-noise behaviors plus a follow-up usage_update signal; __setSpontaneousTurnIdleTimeoutForTests shrinks the quiet window in tests.

How you verified

  • 3 new tests in src/bridge/bridge.test.ts (real bridge + real delta assembler, fake agent over stdio): vouched turn opens for unsolicited output and quiet-closes; a still-open vouched turn settles before the next user turn (3 started / 3 completed, stream intact, exactly one input.accepted); non-work idle updates open nothing.
  • Mutation check: removing agent_message_chunk from the work-kind map fails both positive tests (and not the noise test).
  • pnpm -C plugins/provider-acp test → 184 passed (14 files); tsc --noEmit clean; eslint + prettier clean on touched files.

Fixes #2122

AGENT GENERATED: by zai/glm-5.3

ACP agents can produce session updates with no prompt in flight (OMP
async-job auto-delivery). The bridge gated all work updates on an
active prompt, so the output was dropped and nothing reached the
thread. The bridge now opens a turn itself when idle agent-work
updates arrive, settles it on a quiet window, before the next
bb-initiated turn, or on stop; non-work updates stay noise.
Fold the parallel spontaneousTurnOpen boolean into activePromptKind as
an "agent" prompt kind, so every reader of the open-turn mirror — turn
dispatch, permissions, error settlement, interrupts — treats agent-
initiated turns (OMP async-job delivery) as real work:

- The turn/start guard no longer rejects while a vouched turn is open;
  runTurn/startCompaction settle it (end_turn) before opening the next
  prompt. thread/stop interrupts it as cancelled via
  settleInterruptedPrompt.
- An agent process exit or session error now settles the open vouched
  turn: emitSessionError emits the settling error delta for any open
  prompt kind (removeSession keeps clearing only the quiet timer), so
  the thread can never hang "working" after a crash mid-turn.
- Permission requests raised inside a vouched turn are no longer
  auto-cancelled: full mode auto-allows and other modes forward to the
  user, like a prompted turn.
- Derive the work-kind test from the visibility metadata
  (isAgentWorkUpdateKind) instead of a second hand-maintained map.

Adds regression tests: agent exit mid vouched turn, a failing prompt
settling an open vouched turn (error still surfaces), and a permission
request inside a vouched turn, with matching fake-agent behaviors.
@bradhallett

Copy link
Copy Markdown
Contributor Author

Rework addendum — reply to the review of #2123

Rebased onto current main (42658f987); the bridge now lives in packages/provider-bridge-acp after the #2325 restructure. The rework commit implements the reviewer's structural proposal (§6):

  1. Finding 5 (parallel boolean) — fixed structurally. spontaneousTurnOpen is gone; the vouched turn lives in activePromptKind: "turn" | "compaction" | "agent" | null. Every reader of the mirror had to decide, and now does:
    • turn/start no longer rejects while a vouched turn is open — the guard exempts "agent", and runTurn/startCompaction settle it (end_turn) before opening the next prompt (exactly one terminal state per vouched turn).
    • thread/stop with interrupt settles it as cancelled via a new case "agent" in settleInterruptedPrompt; since real work is in flight, the stop now also delivers session/cancel to the agent.
    • emitSessionError settles any open kind — finding 1 below.
    • handlePermissionRequest — finding 2 below.
  2. Finding 1 (never settled on agent exit; High) — fixed. On agent process exit, removeSession clears only the quiet timer and emitSessionError — which runs while activePromptKind still reads "agent" — emits the settling error delta: the assembler's provider.error { settlesTurn } closes the vouched turn as failed, then the thread-scoped exit error surfaces. The same covers any session error raised mid-vouched-turn. New tests: "settles a vouched turn when the agent process exits mid-turn" (asserts a second turn/completed with status: "failed" plus the error notification) and "…when a prompt fails while it is open" (vouched turn settles completed, the failing prompt's own turn settles failed, error still surfaces).
  3. Finding 2 (permission requests auto-cancelled; High) — fixed. handlePermissionRequest now treats "agent" exactly like "turn": full mode auto-allows, other modes forward to the user. Test: "answers a permission request raised inside a vouched turn (full mode)" asserts the agent receives {"optionId":"yes"} and not cancelled.
  4. Finding 4 (duplicated map) — fixed. The work-kind test is derived from the visibility metadata: isAgentWorkUpdateKind = NORMALIZED_ACP_UPDATE_KINDS minus usage_update, exported from visibility.ts; the bridge's hand-maintained AGENT_WORK_UPDATE_KINDS is deleted.
  5. Finding 3 (120 s window; Medium) — kept, deliberately. The constant and the __setSpontaneousTurnIdleTimeoutForTests hook are unchanged. With findings 1–2 fixed the window is now an upper bound, not the only exit (exit, errors, interrupts, and the next turn all settle immediately), so the crash-hang and dead-Stop-button-during-crash cases are gone; what remains is the reviewer's point about buffered final output during idle-quiet. Lowering it to 5–10 s is a one-line change, but it was flagged as a trade (more split turns) that deserves a maintainer's product call plus a recorded real OMP delivery (BB_PROVIDER_BRIDGE_RECORD_DIR) to look for a positive end signal — not silently changed in this PR. Happy to flip the constant on request.
  6. Finding 6 (missing edge tests) — added. Agent exit mid-vouched-turn, failing prompt mid-vouched-turn, permission inside a vouched turn. The teardown brittleness is addressed: the new tests set the window to 60 s via the hook and the describe's afterEach restores it; the agent-exit test pops the dead session like the existing die test.

Also per §6.5: turn/steer during a vouched turn still degrades gracefully — it reports NO_ACTIVE_TURN, the runtime maps that to a fresh turn/start, and the dispatch settles the vouched turn first. Non-work updates (available_commands_update, usage_update, …) still open nothing, and user_message_chunk stays input-echo noise (no phantom user row — #2014). A slow provider may still split into multiple vouched turns; output is never lost. No wire or protocol changes; translator untouched.

Verification: pnpm -C packages/provider-bridge-acp run test — 261 passed / 17 files (6 in the agent-initiated describe: 3 ported + 3 new); pnpm -C packages/provider-bridge-acp run typecheck — clean; pnpm exec turbo run test typecheck --filter=@bb/provider-bridge-acp --force — 3/3 green; oxfmt --check on the four touched files — clean. Mutation checks: deleting agent_message_chunk from the work-kind set fails all five positive tests while the noise test passes; reverting the emitSessionError agent-settling reintroduces the hang and fails the new agent-exit test.

AGENT GENERATED

@bradhallett
bradhallett force-pushed the fix/acp-agent-initiated-turns branch from 39a8b2b to d7b1ae6 Compare August 24, 2026 22:52
@bradhallett

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #2220 — the vouched-turn approach landed there in better shape. The fake-agent wire shape from this PR is referenced in can1357/oh-my-pi#9157 for testing the omp-side capability when it lands.

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.

provider-acp silently drops agent-initiated turns (unprompted session updates, e.g. OMP async-job delivery)

1 participant