fix(acp_client): end the reply's message where the live view does

[LivXue]

Summary

ACP replies and live output need the same message boundaries. A steer arriving during a report must not drop its earlier half, and a held prompt's heartbeat must not split that report. Return the last nonblank message while retaining narration in the transcript.

• A tool opening, terminal or status-less tool update, or plan opening ends a message. Updates marked pending or in_progress do not; Raven's held-prompt heartbeat can arrive mid-report.
• A steer stays inside its message with one separator. That separator spends the message's budget when it is inside returned text; a separator between messages does not.
• Leading whitespace remains visible but spends no capped-message budget because reply strips it. Blank chunks and repeated steers before the first nonblank text no longer silently truncate a reply that fits. Whitespace inside substantive message text still counts.
• Preserve the distinct closing transcript row and document the intentional report/save/sign-off behavior. Code comments and the ACP parsing design name both nonterminal statuses explicitly.

Type

• Fix
• Feature
• Docs
• CI / tooling
• Refactor
• Other

Verification

• Relevant tests pass locally
• Relevant lint / type checks pass locally
• User-facing docs or screenshots are updated when needed

Linux, Python 3.12, after rebase onto current main:

uv run --frozen --all-extras pytest tests/test_subagent_acp.py tests/test_acp_methods.py tests/test_subagent_turn_rows.py tests/test_subagent_history.py tests/test_rpc_dag.py tests/test_rpc_subagent_calls.py tests/test_subagent_openai_steps.py tests/test_subagent_openai_backend.py -q -x --tb=short

618 passed. Coverage includes the real held-prompt heartbeat, tool/plan boundaries, persisted replies, transcript rows, and downstream DAG consumers.

Native Windows, Python 3.12:

uv run --frozen pytest tests/test_subagent_acp.py -k "blank_prefixes or steered_message or steer_inside or a_steer_sets" -n 0 -q -x --tb=short

23 passed. Nine blank-prefix cases failed before the fix; the final matrix also covers prefixes after a tool boundary and explicitly preserves visible whitespace. Four isolated mutations were caught: charging stripped whitespace, treating every chunk as leading, ending the leading-prefix state on a blank chunk, and dropping visible leading whitespace. Sources were restored after each run.

Checks passed:

uv run --frozen pre-commit run --from-ref origin/main --to-ref HEAD
uv run --frozen ty check raven/acp_client/acp_agent.py raven/agent/subagent/backends/base.py raven/agent/subagent/history.py
uv run --frozen lint-imports
uv run --frozen python scripts/check_commit_messages.py origin/main..HEAD
commitlint --from origin/main --to HEAD
uv run --frozen python scripts/check_source_language.py origin/main..HEAD
uv run --frozen python scripts/check_large_files.py origin/main..HEAD
git diff --check origin/main...HEAD

All 10 import contracts kept. Full repository tests, native macOS, and the TUI suite were not rerun for this follow-up. The full ACP subprocess selection ran on Linux; Windows verification covers the collector directly.

Risk

• Security impact considered
• Backward compatibility considered
• Rollback path is clear for risky changes

ACP callers receive the last nonblank message, so a report followed by a save step and "Saved." returns the sign-off; the earlier report remains in the transcript. Leading display-only whitespace and separators between messages can make the raw stream longer than the returned-message cap. Internal separators and substantive text remain capped together. No stored format or configuration changes.

Rollback: revert the squash commit.

Related Issues

#889

[gloryfromca]

No blockers; this can merge as far as I am concerned.

Reviewed the full github/main...HEAD diff and the surrounding ACP collector, reply-clamping, activity/history, transcript, plan, tool-call, and steering paths. I also checked the relevant history and #889 behavior, repository rules and canonical terminology, backward compatibility/risk, and that the changed assertion reflects the intended steer semantics rather than weakened coverage. No pre-existing or style-only issues are being raised.

Verification: uv run --frozen --all-extras --python 3.12 pytest -q -p no:randomly tests/test_subagent_acp.py tests/test_rpc_dag.py tests/test_rpc_subagent_calls.py tests/test_subagent_history.py passed (408 tests); uv run --frozen --all-extras --python 3.12 pytest tests/integration/test_tui_rpc_steer_e2e.py -q -p no:randomly -o addopts="" passed (2 tests); source-language, large-file, focused Ruff lint, and Ruff format checks passed.

[0xKT]

Not a blocker -- smaller items from the acceptance pass on head 8149f45 (merged onto main c3d2821), beside the blocking thread on the held-prompt heartbeat.

Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.

1. A steer's break is outside the stream budget but inside the reply cap. The \n\n that sets a steer apart goes out through the unbounded _deliver (raven/acp_client/acp_agent.py:347-351), so the per-message budget does not count it, while reply keeps it and clamp_output (raven/agent/subagent/backends/base.py:87) does. A steered final message whose agent text is within 2 characters per steer of the cap streams whole and is then cut in the caller's copy, with a truncation notice pushed under a message the screen showed complete. Measured with a 200-character cap and 100 characters, a steer, 100 characters: the stream shows all 200 uncut, reply is 202 characters, and the caller gets the first 46 plus "[raven] Output truncated: this reply is the first 46 of 20..."; with 99 + 99 (a 200-character reply) nothing is cut. Main returned the 100 characters after the steer, whole. Rare at the default cap, but it is the opposite of what the comment at :348-350 and CONTEXT.md's Reply Streaming entry (:2324-2325, "acp caps each message at maxOutputChars the way the reply is capped") say.
2. Three of the new boundaries have no test. Each of these mutants leaves tests/test_subagent_acp.py + tests/test_acp_methods.py at 434 passed: deleting the steer flag's reset (self._steered = False, :336), so every chunk after a steer gets its own break; letting a status-less tool_call_update stop ending a message; and letting a plan opening stop ending one (:718). reply relies on all three now.

Checked on the merged tree: the machine gates (ruff, lint-imports 10 kept / 0 broken, commit, large-file and language checks) pass, and tests/test_subagent_acp.py + tests/test_acp_methods.py give 434 passed.

[LivXue]

@0xKT both items from the note above reproduced on 8149f45 before any change, with main c3d2821 as the control. The blocking thread is answered in its own thread.

1. A steer's break outside the stream budget, inside the reply cap. Your measurement exactly: cap 200, 100 + a steer + 100 streams all 200 characters and the break, reply is 202, and the caller gets the first 46 plus the notice; 99 + 99 comes back whole; main returns the 100 after the steer. A regression of this PR: on main a steer ended the message, so its break sat between two messages and in no reply. Fixed in 5450d22 (73a81d9 then narrows the callback's optional type for ty; behaviour unchanged): a step's break still rides past the budget, since it comes before the message it opens and is in no reply (the fix(acp_client): return only the last message of an acp sub-agent turn #889 round-one case), and a steer's break now spends the budget of the message it sits in, since the reply's cap counts it. That makes the CONTEXT.md line you quoted true rather than reworded. test_a_steered_message_streams_whole_only_when_its_reply_comes_back_whole runs the cap at the reply's length and one and two under it, asserts the stream shows reply[:limit], and ties "streamed whole" to clamp_output's own answer; the two short caps fail on 8149f45. One assertion this PR wrote flips with it: test_a_steer_inside_the_final_message_does_not_cut_the_reply expected the break to spend nothing (...ONE. \n\nREP) and now expects ...ONE. \n\nR. Mutants: the steer's break unbudgeted again, 3 failed; the step's break budgeted, 2 failed, one of them fix(acp_client): return only the last message of an acp sub-agent turn #889's test_the_live_budget_is_spent_per_message_like_the_reply, so that exemption stays covered.

2. Three boundaries with no test. All three of your mutants left 434 passed here too, beside a control (the steer's break deleted) that failed 2. Each now fails exactly one test:

   • the steer flag's reset: test_a_steer_sets_apart_only_the_words_right_after_it, one break per steer rather than one per chunk (97f5b67);
   • a status-less update ending a message: test_a_status_less_update_still_ends_the_message, fed claude-agent-acp's captured result frame, which carries no status. It went in with the heartbeat fix (9ac81ed), since it is the other side of that commit's predicate;
   • a plan opening ending a message: test_only_the_first_plan_frame_breaks_the_message fed its first plan frame with nothing said before it, so the opening's break was never observable. It now has text before the first frame and asserts both halves of its name (97f5b67).

Rebased onto main 289426c (#709, #857, #868, #894 and #859; only #709 shares a file with this branch, CONTEXT.md, in a different entry): 0b4f945 -> 765704c and 8149f45 -> 87692c0, per-commit patch-ids unchanged. At the new head 73a81d9: tests/test_subagent_acp.py + tests/test_acp_methods.py 442 passed; the full suite 27864 passed / 7 failed / 120 skipped, and main 289426c, run the same day in the same tree and interpreter, gives 27854 / 7 / 120 with the same 7 failing IDs (five proxy tests, the npm-leak install test, the root-only node-runtime test), so none is introduced; ruff, ty, lint-imports (10 kept, 0 broken) and the commit, large-file and language gates are clean.

[gloryfromca]

No blockers; this can merge as far as I am concerned.

Reviewed the refreshed github/main...HEAD diff and the full delta from 8149f4514e74, including the ACP producer/collector boundary, reply and streaming budgets, transcript/history callers, canonical context/spec text, backward compatibility, and whether the tests preserve rather than weaken the prior behavior. On this head the held-prompt heartbeat no longer splits the wake report; opening calls, terminal and status-less updates, and plan openings still end messages, while steer separators consume the same message budget counted by the reply cap. I found no additional defect.

Verification: the combined ACP/RPC/history suite passed (566 passed); the TUI steering integration passed (2 passed); source-language, large-file, commit-message, focused Ruff lint/format, and focused ty checks passed. I did not resolve the existing review thread because another reviewer opened it.

[0xKT]

Not a blocker -- two small items from the re-acceptance of head 73a81d9 (merged onto main 6126965); the heartbeat fix and the rest of the delta hold.

Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.

1. A steer after blank text spends stream budget the reply never counts. When a message opens with a whitespace-only chunk and a steer lands before any other text, the steer's break goes through the budgeted callback (raven/acp_client/acp_agent.py:355), but reply strips the blank text and the break (:821), so clamp_output returns the reply whole while the live view stops short of it with no notice. Measured with the tree's _TurnCollector at a 30-character cap: blank text, a steer, then a 30-character reply comes back whole and streams 25 of its 30 characters (main 6126965: all 30); with no steer, or with real text before the steer, both agree. It needs a reply within a few characters of max_output_chars, so it is rare.
2. The new wording says "still running"; the code also passes over pending. The design spec (docs/specs/2026-08-23-codex-acp-tool-parsing-design.md:258-259), the _BREAKING_UPDATES comment (:95-97), the branch comment (:383) and the reply docstring (:809-810) all describe the update that ends no message as one saying its call is still running; the check at :389 is status not in ("pending", "in_progress"). Naming both in the text would match it.

[gloryfromca]

No blockers; this can merge as far as I am concerned.

Reviewed the full github/main...HEAD diff and the delta from 73a81d9fd14d, including the ACP message-budget state, steer and step boundaries, reply trimming, downstream history/RPC callers, canonical context/spec wording, compatibility, and test-strength changes. The revision fixes both re-acceptance notes: leading whitespace and steer separators that reply.strip() removes no longer consume the returned message's stream budget, while separators inside nonblank reply text remain budgeted; the documentation now names both nonterminal statuses. I found no additional defect.

Verification: the combined ACP/RPC/history suite passed (584 passed); the TUI steering integration passed (2 passed); source-language, large-file, commit-message, focused Ruff lint/format, and focused ty checks passed.

[ZuyiZhou]

LGTM. Read the full diff and call sites; CI is green and the tests exercise the fix.