fix(acp): tolerate a null final_response when a turn is cancelled

[lxman]

What & why

Cancelling a turn could kill the session with:

AttributeError: 'NoneType' object has no attribute 'startswith'

surfaced to the client as a bare JSON-RPC "Internal error". JetBrains treats that
as a non-recoverable session failure: it tears down the session and kills the
agent process, so a cancel could destroy a session that was otherwise healthy.

result.get("final_response", "") assumed the key is absent-or-string, but the
conversation loop can return it present-and-None — the truncation path returns
partial_response or None outright (agent/conversation_loop.py). A .get
default only applies to a missing key, so the value stayed None.

The rest of the codebase already treats it as optional — turn_finalizer uses
len(final_response) if final_response else 0, which is why the turn log prints
response_len=0 instead of crashing. The ACP adapter was the outlier.
Normalising at the single ingress point covers all five downstream uses.

How to test

tests/acp/test_server.py::TestPrompt::test_prompt_survives_null_final_response
drives a cancelled turn returning final_response: None. It fails with
AttributeError on an unmodified checkout and passes with the fix.

Platforms

Platform-independent. Full tests/acp suite run on Windows 11: 326 passed;
4 pre-existing failures (test_approval_isolation, test_edit_approval,
test_ping_suppression) reproduce on an unmodified checkout.

Reported as the secondary exception in #73693.

---

[teknium1]

Thanks for the focused ACP regression fix. Current main still has the reported failure: acp_adapter/server.py:1891 preserves an explicit None, and acp_adapter/server.py:1898 immediately calls .startswith() on it. The producer path is current as well: agent/conversation_loop.py:2970 returns partial_response or None.

The proposed normalization handles the null at the ACP ingress before all downstream uses, and the added prompt-level regression test exercises the cancelled response shape.

Automated hermes-sweeper review.

[lxman]

Rebased onto 2cdb30a (current main). Companion fix to #69083 (the Windows bash-probe hang fix) — this handles a related but independent crash on cancelled ACP turns. No file overlap.

[lxman]

Rebased onto 31e571acf (current main) — 699 commits of drift since the last rebase base, no conflicts. Head is f34be994f.

Re-verified the premise on current main rather than assuming it survived:

• Producer still emits the None. agent/conversation_loop.py:3543 — "final_response": partial_response or None on the truncation path.
• Consumer still can't take it. acp_adapter/server.py:2005 is result.get("final_response", ""), and the "" default only applies when the key is absent; a present-but-None value passes straight through to .startswith() at :2012.

So the crash is still reachable, and the one-line change (.get("final_response") or "") is still the fix.

tests/acp/test_server.py: 31 passed against the rebased tree.

Ran outside the canonical runner — scripts/run_tests.sh couldn't find a venv with pytest on this box, so I used a scratch venv with the repo tree and the install's site-packages on PYTHONPATH, and set TZ=UTC LANG=C.UTF-8 PYTHONHASHSEED=0 to match what the runner enforces. Worth a CI confirmation rather than taking my local run as equivalent.

[yflmq001]

Contributing a regression test that complements this fix (from the duplicate #86817 I closed).

This PR's normalization stops the None.startswith() crash, but the consequence — a prompt queued during/after an interrupted turn never draining — deserves its own test. The bug report (#86798) calls it out explicitly: "the queued user message is never executed and stays 'queued' forever." Without this test, a future regression that breaks the drain loop (while the normalization stays) would pass CI.

Test (fits alongside your existing ACP regression tests; agent's run_conversation returns {"final_response": None, "interrupted": True}):

@pytest.mark.asyncio
async def test_queued_prompt_drains_after_interrupted_turn():
    """A prompt queued after an interrupted turn must still run."""
    acp_agent, state, fake, conn = make_interrupt_agent()

    await acp_agent.prompt(
        session_id=state.session_id,
        prompt=[TextContentBlock(type="text", text="first")],
    )
    await acp_agent.prompt(
        session_id=state.session_id,
        prompt=[TextContentBlock(type="text", text="second")],
    )

    # Both prompts were dispatched to the agent — the queued one drained.
    assert fake.runs == ["first", "second"]

On the un-fixed code this test fails with the exact AttributeError from the report (mutation-checked); with your normalization it passes. Happy to fold this into the PR if you'd like it included.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(acp): tolerate a null final_response when a turn is cancelled

1. acp_adapter/server.py:2015 — result.get("final_response") or "" also coerces any other falsy value (e.g. 0 or False), not just None. final_response is expected to be a string so this is harmless in practice, but a None-specific guard (if (fr := result.get("final_response")) is None: fr = "") would keep type fidelity and make the intent unambiguous.

2. The comment cites turn_finalizer as a consumer that already tolerates None, which suggests the "key present with an explicit None" shape can appear across several producer paths (conversation_loop.py truncation, cancellation). Worth a quick sweep for other result.get("final_response", ...) / ["final_response"] consumers in the ACP adapter and the CLI/TUI paths that may hit the same present-but-None trap, so the fix covers the whole bug class rather than this one site.

3. Good regression test: test_prompt_survives_null_final_response exercises the cancelled-turn shape and asserts stop_reason == "cancelled", and the file also drops a couple of stray trailing blank lines at EOF (cosmetic).

No blocking issues found.