fix(bridge,adapters): a turn that did not finish is not completed

devlog/_plan/260817_cursor_toolcall_decode/000_index.md

@@ -237,9 +237,9 @@ already-emitted terminal stay graceful.
 
 ### What shipped for 020
 
-Tool-result images reach Cursor as real `McpImageContent`. The final design differs
-from the original plan in three ways, each forced by a review that proved the plan
-would have broken a working request:
+The Cursor ENCODER now emits tool-result images as real `McpImageContent`. The final
+design differs from the original plan in three ways, each forced by a review that
+proved the plan would have broken a working request:
 
 1. **Bounding is post-serialization, not a byte budget.** A step is one blob shared
    with the call's arguments, text, and framing, so `toolCallStep` serializes and
@@ -263,3 +263,43 @@ measurement cycle in `030` stands, and the phase closes NOOP unless the user
 supplies a failing case. Note the probe did not isolate the top-level freeform
 surface — the cursor agent reached `apply_patch` through code mode — so this is
 "not reproduced", not "proven absent".
+
+## Final status — all phases
+
+| Phase | Outcome | Verified |
+|-------|---------|----------|
+| `010` clean-EOF terminal | **SHIPPED** `54f68daf5` | 7 rounds; lidge 608/0 |
+| `020` tool-result images | **SHIPPED** `878b067e8..cc906b0fc` | 5 rounds; byte-equality verified twice; lidge 624/0 |
+| `030` xai apply_patch | **NOT REPRODUCED** | live probe: both providers used `apply_patch` |
+| `040` server-side cancel | **SHIPPED** `f145fd513..c9681d043` | lidge 630/0 |
+| `050` terminal-less turns + #422 | **SHIPPED** `aa800ae65..1651002c5` | 8 rounds; lidge 12800/0 across 830 files |
+
+Final full suite at `1651002c59`: **12800 pass / 0 fail**, typecheck clean.
+Pre-campaign baseline was 12761/0 across 826 files.
+
+### What the campaign actually found
+
+The user's report was "Computer Use keeps disconnecting mid tool call". The decode
+found three distinct ways a Cursor turn could lose work, and the last one turned
+out not to be Cursor-specific at all:
+
+1. A clean stream EOF dropped an open tool call and reported success (`010`).
+2. Every screenshot reached the model as placeholder text, though the wire had
+   always supported images (`020`).
+3. A cancel Cursor sent us was indistinguishable from one we sent, so the turn
+   vanished entirely (`040`).
+4. Underneath all three: the bridge reported a turn with no terminal as
+   `completed` — and on a compaction turn installed its partial output as
+   replacement history (`050`). That one affected every provider.
+
+The xai `apply_patch` symptom did not reproduce when probed live, so no code was
+written for it.
+
+### Remaining follow-ups
+
+- **Kiro** `completionMode: "disabled"` and **ordinary Google mode** erase
+  truncation reasons before the bridge can act (`050`). Adapter-side, each its
+  own unit.
+- **User-message images** are still placeholdered although `SelectedImage`
+  supports blob/inline data (`002`). Separate capability.
+- **`030`** stays open as a measurement cycle pending a reproducible failing case.

devlog/_plan/260817_cursor_toolcall_decode/020_phase2-toolresult-image-passthrough.md

@@ -124,3 +124,28 @@ adapter directory)
 
 All nine pass, typecheck clean, cursor suite green on `ssh lidge`, pushed.
 
+## POST-SHIP CORRECTION (audit r1, 2026-08-18)
+
+This phase shipped the ENCODER, and the encoder is correct. It does **not** deliver
+the end-to-end capability, and the earlier wording in `000_index.md` overstated it.
+
+Every Cursor model is listed in `noVisionModels`
+(`src/providers/registry.ts:978-982`), so before the adapter ever runs, the vision
+sidecar replaces image parts with text descriptions — or strips them fail-closed
+when no sidecar plan exists (`src/server/responses/core.ts:2225-2243`). That
+preprocessing explicitly covers `toolResult` messages
+(`src/vision/index.ts:252-259`, replacement at `:565-581`).
+
+`tests/cursor-tool-result-image.test.ts` calls `encodeCursorRunRequest` directly
+with hand-built `rawMessages`, so it proves encoder support and nothing about the
+production path.
+
+The registry comment claiming "Cursor's wire protocol never forwards image parts"
+is now half-stale: still true for USER images (`request-builder.ts:206-214`
+flattens them), no longer true for tool results.
+
+Closing this gap is its own unit, not a wording fix: dropping Cursor from
+`noVisionModels` alone would leave user images with neither an image nor a
+description. It needs role-aware vision policy plus an end-to-end regression
+through the server path. Tracked as follow-up 1 in
+`devlog/_plan/260818_cursor_call_integration/000_plan.md`.

devlog/_plan/260817_cursor_toolcall_decode/050_phase5-nonstreaming-terminal.md

@@ -0,0 +1,171 @@
+# 050 — Phase 5: a buffered turn that never terminated is not "completed"
+
+The last follow-up recorded in `000_index.md`. Deferred from `010` pending
+evidence; that evidence now exists and is worse than the note assumed.
+
+## Measured, not assumed
+
+`buildResponseJSON` defaults to `"completed"` whenever no error or incomplete
+event is present (`bridge.ts:1830-1834`). Probed directly against the current
+tree:
+
+| Adapter events | Result |
+|----------------|--------|
+| `[text]`, no terminal | `status: "completed"`, no `incomplete_details` |
+| `[tool_call_start, tool_call_delta("{\"code\":\"tru")]` | `status: "completed"` with a `function_call` item, `status: "completed"`, `arguments: "{\"code\":\"tru"` |
+| same + `tool_call_end` | `status: "failed"`, item `status: "incomplete"` |
+
+The second row is the defect. A truncated tool call — invalid JSON, never closed —
+is handed back as a **successful** turn containing an apparently complete
+function call. A caller that trusts `status` will try to execute it.
+
+The third row is the same bridge, on the same arguments, getting it right. The
+rejection logic already exists (`bridge.ts:1070-1080`); it is reached only when
+an explicit `tool_call_end` arrives. When the stream simply stops, nothing runs
+it.
+
+## Why this is in scope
+
+`010` and `040` both closed *adapter-side* routes to this shape: a truncated EOF
+and a server cancel now raise typed errors, so those paths no longer reach the
+buffered default. This phase closes the default itself, which is what makes the
+guarantee hold for any adapter that ends a stream without a terminal — including
+future ones nobody has audited.
+
+## Scope warning
+
+`src/bridge.ts` is shared by **every** provider. This phase therefore:
+
+- changes only the no-terminal case, leaving every explicit `done`/`error`/
+  `incomplete` path byte-identical;
+- runs the **full** suite on `ssh lidge`, not the cursor subset. A baseline full
+  run at `6d97442839` is captured before the change so any new failure is
+  attributable.
+
+## Contract
+
+| Buffered turn | Status |
+|---------------|--------|
+| explicit `done` | `completed` — unchanged |
+| explicit `error` | `failed` — unchanged |
+| `incomplete` / `max_tokens` / `content_filter` | `incomplete` — unchanged |
+| no terminal, no open tool call | `incomplete`, `incomplete_details.reason = "adapter_eof"` |
+| no terminal, tool call left open | `incomplete`, and the item is **not** `completed` |
+
+Streaming already reports `adapter_eof` for the fourth row (`bridge.ts:1283`), so
+this aligns the buffered path with the streaming one rather than inventing a new
+signal.
+
+## Diff-level plan
+
+**`src/bridge.ts`**
+
+- In `buildResponseJSONWithBudget`, track whether any adapter terminal
+  (`done`/`error`/`incomplete`) was observed.
+- When none was, resolve `status` to `"incomplete"` with
+  `incomplete_details: { reason: "adapter_eof" }`, matching the streaming path's
+  wording exactly.
+- An unclosed tool call must not carry item `status: "completed"`. Reuse the
+  existing incomplete-item marking rather than adding a second notion of
+  "unfinished".
+- Do not touch `stopReason` handling, usage reporting, or compaction.
+
+## Tests (`tests/bridge-nonstreaming-terminal.test.ts`)
+
+1. Text with no terminal -> `incomplete` + `adapter_eof`, not `completed`. Red today.
+2. Open tool call with truncated arguments and no `tool_call_end` -> turn is not
+   `completed` and the item is not `completed`. Red today; this is the executable-
+   garbage case.
+3. Parity: the same events through streaming and buffered agree on terminal
+   status. This is the assertion that keeps the two paths from drifting again.
+4. Explicit `done` -> still `completed` (regression).
+5. Explicit `error` -> still `failed`; explicit `incomplete` -> still `incomplete`
+   with its own reason preserved (regression).
+
+## Done when
+
+All five pass, `bun run typecheck` clean, and the **full** suite on `ssh lidge`
+matches the pre-change baseline. Tests 1 and 2 demonstrated red beforehand.
+
+
+## Shipped, and what the audit chain changed
+
+Five review rounds. The plan's core claim survived; nearly every detail did not.
+
+| Commit | What it closed |
+|--------|----------------|
+| `aa800ae65` | The default itself: a buffered turn with no adapter terminal is `incomplete`/`adapter_eof`, and an open tool call is no longer emitted as a completed `function_call` with half-written JSON. |
+| `44fde398b` | The #422 compaction guard could only see explicit failure events, so a terminal-less turn still installed replacement history. |
+| `f73f09c9e` | Streaming emitted the compaction item *before* reading `stopReason`; Google's `parseResponse` dropped `finishReason` entirely. |
+| `95f73db17` | `stopReason` is an open-ended string and adapters disagree (`length`, `refusal`); canonical-only matching left the guard bypassable. |
+| `71730023a` | **My own regression:** suppressing the item without downgrading the turn produced `completed` with zero compaction items — the shape codex-rs fatals on. Suppression and status now come from one decision. |
+| `ea5e61677` | Anthropic `pause_turn` is unfinished by definition; AI SDK `error` is a failure, so Command Code emits a real error terminal instead of a stop reason. |
+
+The lesson worth keeping: each round fixed the previous round's fix. Round 4 found
+that my round-3 change had made things *worse* in one direction — a suppressed
+compaction item with a success status is more dangerous than the bug it replaced,
+because codex-rs treats zero items as fatal. Widening a guard without widening
+what it reports is not a partial fix; it is a new failure.
+
+## Open follow-ups (adapter-side, deliberately not folded in)
+
+Both erase truncation metadata **before** the bridge can defend anything, so they
+cannot be fixed here:
+
+- **Kiro** (`kiro.ts:1315`, `:1485`): in `completionMode: "disabled"` — which routed
+  compaction selects, because it removes tools — the normalized reason is observed
+  and then the final `done` omits `stopReason`. `MAX_TOKENS` and
+  `MODEL_CONTEXT_WINDOW_EXCEEDED` both vanish.
+- **Google ordinary mode** (`google.ts:779`, `:947`): only `MAX_TOKENS` and five safety
+  values are forwarded. `MALFORMED_RESPONSE`, `UNEXPECTED_TOOL_CALL`, `IMAGE_SAFETY`,
+  and `LANGUAGE` become reasonless `done` events. The Vertex/CCA fail-closed guard
+  covers only part of this.
+
+Each is its own unit with its own truncation subsystem. Recording them beats
+half-fixing them inside a bridge phase.
+
+## Final state (eight rounds)
+
+| Commit | Closed |
+|--------|--------|
+| `ea5e61677` | Anthropic `pause_turn` is unfinished by definition; AI SDK `error` is a failure, so Command Code emits a real error terminal. |
+| `6478cbb02` | Removing `error` from the shared table was safe only for Command Code — Anthropic forwards `stop_reason` verbatim, so it needed its own error terminal on the buffered and streaming paths. Both terminals carry usage. |
+| `1651002c5` | Anthropic has a **third** terminal path: the EOF branch for providers that close after `message_delta` without `message_stop` bypasses `emitDone` entirely and still reported success. |
+
+Round 8: **PASS, no findings.** Verified exhaustively that Anthropic's terminal
+paths are `message_stop`/`emitDone`, the compatible-provider EOF branch, and
+buffered `parseResponse` — and that `anthropicEofTolerance` is not a fourth,
+since it runs only when no stop reason was received and delegates back through
+`emitDone`.
+
+Final verification on `ssh lidge` at `1651002c59`: typecheck clean,
+**12800 pass / 0 fail** across 830 files. The pre-phase baseline was 12761/0.
+
+### Two tests that were protecting nothing
+
+Worth recording, because both looked like coverage:
+
+- The Anthropic streaming test was titled "carrying usage" and never asserted
+  usage. Removing usage from the error terminal kept it green.
+- The Command Code coverage hand-constructed the downstream error event instead
+  of driving the parser, so it stayed green while the adapter still emitted a
+  clean `done`.
+
+Both now drive the real parsers, and dropping usage from either error terminal
+turns the suite red.
+
+### What the round count actually bought
+
+Eight rounds, and the finding that justified them arrived at round 7 — after two
+rounds had already declared the area closed. Two of the defects were regressions
+I introduced while fixing the previous round's finding:
+
+1. Suppressing the compaction item without downgrading the turn produced
+   `completed` with zero compaction items, which codex-rs treats as fatal. A
+   half-widened guard was more dangerous than the bug it replaced.
+2. Removing `error` from the shared table fixed Command Code and silently
+   re-opened Anthropic, which forwards the same string verbatim.
+
+Both share a shape: a fix that is correct for the case in front of you and wrong
+for the one next to it. That is the argument for auditing revisions, not just
+first drafts.