fix(chat): notice a tool call that leaked after the turn ran tools

[rnoack1]

Problem / Motivation

When a turn executes some tool calls and then leaks its final call into the text
channel instead of executing it, the user is told nothing.

Observed in a dashboard session: the model re-read live state over two tool calls,
wrote a confident one-line summary of what it had checked, said it was performing the
write — and then emitted the write itself as prose, an invoke block with its
parameters rendered as message text. The turn ended there. The write never ran. The
resource was unchanged.

Nothing in the chat said so. The reply reads like a completed action, because the
reads really did happen, so there is no missing output to notice and no stall to
explain. The only trace was a logger.warning in the gateway log, which the person
in the chat does not read.

has_leaked_tool_call already detects this exactly, and the runner already reaches
the branch. It logs and stops.

Why it matters

This shape hides the gap better than the zero-tool-call one the notice already
covers, not worse.

A turn that leaks its only call produces no work and no output — a user watching an
unattended loop eventually notices nothing happened. A turn that leaks its last
call produces real work plus a summary that sounds finished. The failure is silent
and confidently worded, and the user's next action is taken on the belief that the
write landed.

In the observed session the leaked call was the one the user had just explicitly
authorised, so the message said the action was being performed while the state it
described never changed.

What changed (motivation → approach → change)

Symptom → a mixed turn's leak is detected, logged, and never shown to the user.

Root cause → the leak response has two halves, and both were gated on the same
condition. should_notice_leaked_tool_call requires turn_tool_calls == 0, and its
docstring gave the reason as "a turn that executed tools and ALSO printed a block is
not the leak shape". That reason is not the real one. The real one is in the runner's
own comment: un-landing a turn whose earlier calls had side effects would
misdescribe it. That reasoning is sound — and it is a reason to withhold
un-landing, not a reason to withhold informing. A notice card queues nothing,
executes nothing and un-lands nothing.

Change → the two halves are separated at that seam.

• The existing mixed-turn branch now appends a notice in addition to its log. It does
  not set _noticed_leak, so the turn still lands, bills and consolidates exactly
  as before. No landing behaviour changes anywhere.
• Its inline condition is extracted to should_notice_mixed_turn_leak in
  chat_utils.py, beside its sibling — matching how this file already carries each
  turn-shape decision as a pure predicate with the rationale in its docstring, and
  giving the branch a testable seam it did not have.
• The predicate's conditions are exactly the four the runner already used, so the
  set of diagnosed turns does not move — only its visibility. It is deliberately not
  gated on in_stage_execution (that exclusion protects the orchestrator's stage
  accounting from a changed turn result, and a card changes no turn result) and needs
  no is_cancelled gate, since a cancelled turn never reports end_turn.
• The notice wording differs from the sibling's on purpose. "nothing was run" is
  false here, and a user who read it that way would redo work that already took
  effect — so it names the count that ran and says part of the turn landed and part
  did not.
• Two docstrings asserted the claim this change contradicts ("not the leak shape").
  Both now give the un-landing reason instead and point at the new predicate. A
  tool-heavy turn is a leak; it is just not un-landable.

Tests

Added to test/test_leaked_toolcall_notice.py, following the file's existing
convention of assembling the machine syntax from fragments rather than writing a raw
invoke block as a literal:

• test_a_turn_that_executed_tools_then_leaked_its_last_call_is_noticed — the
  observed shape now fires.
• test_the_two_predicates_partition_by_tool_count — the two predicates partition on
  tool count, neither overlapping nor leaving a gap. The runner chains them as
  if/elif, so an overlap is an ordering bug and a gap is a silent stall.
• test_a_cancelled_mixed_turn_is_not_noticed,
  test_non_end_turn_and_nested_mixed_turns_are_not_noticed — each gate declines.
• test_a_mixed_turn_whose_text_is_prose_is_not_noticed — the tool count alone never
  fires the card; it takes an actual leak.
• test_a_mixed_turn_quoting_a_fenced_block_is_not_noticed — the new path inherits
  the structural quoted-code exclusion, so explaining a leak is not one.

The comment on the existing test_a_turn_that_made_tool_calls_never_notices is
updated to name the un-landing reason; its assertion is unchanged and still passes.

Local: 27 passed in the leak suite; 305 passed across it plus
test_chat_runner_coverage.py and test_error_code_contract.py. isort, flake8,
mypy and the baselined black gate are clean on the changed files.

A passing suite proves nothing about detection, so the tool-count condition was
mutated (if turn_tool_calls <= 0 disabled) and the suite re-run: the partition test
failed with assert True is False, i.e. for the intended reason — the two predicates
would overlap — and passed again on revert.

Manual verification

N/A — the decision is a pure predicate and is unit-tested directly, matching how the
sibling notice is covered; the append site is a single slot.append reusing the
existing notice row.

The triggering condition cannot be forced on demand: the leak originates in the model
writing to the wrong channel, not in anything this layer controls. That is also why
the fix is notice-only.

Screenshots / video

N/A — no new or changed UI surface. The card is the existing notice row
(msg msg-info) already used by the zero-tool-call path; only its trigger and text
are new.

Related Issues

Context: #6112 reported the leak, and #6130 shipped the notice for the zero-tool-call
shape while leaving the mixed-turn shape as log-only. This adds the user-visible half
for that shape, without reversing the un-landing exclusion #6130 chose.

Pattern harvest

Rule candidate: review-prompt

Pattern: when a guard has an inform half and an act half, a safety condition that
only constrains the act must not gate the inform — the event ends up detected, logged
and invisible to the only person who can respond to it. The tell is a branch whose
comment explains why it must not change state, and which then also declines to say
anything.

[dwu96]

👋 Hi! This PR is currently in draft status. Workflow runs won't be auto-approved until it's marked as ready for review.

When you're ready, click "Ready for review" and the workflows will be approved on the next cycle automatically.

[iamwhatever]

👋 Hi! This PR is currently in draft status. Workflow runs won't be auto-approved until it's marked as ready for review.

When you're ready, click "Ready for review" and the workflows will be approved on the next cycle automatically.

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of 65e76a0af98d28341ceb68d419ee38e1663838e8 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Splits inform from act at the correct seam: the un-landing hazard stays gated, visibility no longer is — minimal, sibling-consistent, partition-tested.

Suggestions

• The card fires inside stage-execution turns (deliberately ungated), but its guidance — "Check what landed before re-sending (an active monitor loop retries on its next cycle)" — addresses a human driving the chat, not the orchestrator's stage loop; consider stage-aware wording or noting the mismatch in the predicate's docstring.

[DESIGN-REVIEWED] 65e76a0

[github-actions]

First Principles Review (Fable 5, fork) — ✅ PASS

Premise-level review of 65e76a0af98d28341ceb68d419ee38e1663838e8 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified against the base: the old inline elif at chat_runner.py:9821-9825 used exactly the four conditions the new predicate carries, the sibling notice card and "ℹ️" prefix pre-exist, and has_leaked_tool_call has no other consumers anywhere in src/ — no unfixed sibling of the detected-but-invisible pattern. Final review follows.

First-Principles-Verdict: PASS

A user acting on a confidently-worded turn whose final write never ran now gets told; every item traces to that observed defect.

What this change ships

Intent: make a real, observed silent failure — a turn that runs tools, announces the final write, then leaks it as text — visible to the person in the chat. This is a FIX.

1. Chat now shows a notice when a mixed turn leaks its final tool call — justified (observed defect; the confident summary hides the gap)
2. Notice wording names the count attempted, unlike the sibling's "nothing was run" — justified (sibling wording would be false here)
3. Turn still lands, bills, consolidates; un-landing untouched — justified (side-effect safety, deliberately preserved)
4. Inline condition extracted to should_notice_mixed_turn_leak (1 runtime consumer: chat_runner.py; matches the file's existing predicate convention) — justified, declared
5. Two docstrings corrected where they asserted the claim this change falsifies — rides along, but the same-commit doc rule mandates it
6. New tests including a partition test and a verified mutation run — justified

Description matches the diff exactly: I verified the "conditions are EXACTLY the four the runner already used" claim against base chat_runner.py:9821-9825 — true, so only visibility moves. Root cause (model writing to the wrong channel) is genuinely outside this layer, and the description says so.

Subtractions

• Shrink the new 25-line runner comment in chat_runner.py to a pointer at should_notice_mixed_turn_leak's docstring — the rationale now lives in both places verbatim (2 copies), and the sibling branch already models the pointer form ("Rationale in full: should_notice_leaked_tool_call's docstring").

[FIRST-PRINCIPLES-REVIEWED] 65e76a0

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 65e76a0af98d28341ceb68d419ee38e1663838e8 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 65e76a0

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 65e76a0af98d28341ceb68d419ee38e1663838e8 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 65e76a0