fix(acp): stop error guidance naming commands that do not exist

[chenmingwei23]

Problem / Motivation

Two branches of _format_acp_error told users to run commands that are not
available to them.

Malformed request ended: start "a fresh chat (/chat new)". /chat new
exists nowhere in the product. The command to start a fresh conversation is
/new on Telegram and Discord, and a new tab on the dashboard; /chat in the
frontend is a router path (<Navigate to="/chat">), not a slash command.

Prompt busy said to "send !restart". That is wrong three ways: it is a
Slack-only bang alias (slack/handler.py:159, slack/events.py:2555), it is
owner-gated even there ("Only the owner can restart the gateway",
slack/events.py:745), and it restarts the GATEWAY rather than the session --
it is registered as "restart the gateway (owner-only)" at slack/events.py:829.

So on both branches the actionable instruction a stuck user receives is inert.

Why it matters

The malformed-request class is TERMINAL (_is_transient_raw_error returns False
for it), which is exactly why its text reaches the user verbatim: a channel
dispatcher surfaces a permanent ACP failure's own message instead of the generic
"please try again". Telegram's _user_safe_failure_reason is gated on
transient is False and does precisely this.

That reader is, by construction, in a session that cannot build a valid request.
Typing /chat new there is not a no-op: it is not a command on any surface, so
it is forwarded to the model as prompt text through the same broken session and
returns the identical Improperly formed request. Following the guidance
reproduces the failure. /chat new appears in #7213's own "What did NOT fix it"
list.

The !restart advice was also redundant: both the dashboard
(dashboard/chat_runner.py, retry-eligible branch) and Slack already reset and
re-queue the session on AcpPromptBusy by themselves. The user was being asked
to intervene in a recovery that is automatic.

What changed (motivation -> approach -> change)

Symptom: repair guidance names commands that do not exist for the reader, on the
paths whose text is handed to users unmodified.

Root cause: _format_acp_error lives in acp/client.py and cannot know which
surface will render its output, but both branches were written as if the reader
were on a particular one.

Change: name a command only if every surface understands it, and otherwise use
prose. /compact qualifies -- it reaches the backend through the prompt
transport everywhere, including Slack, which additionally offers !compact as
its own alias. The malformed-request branch keeps /compact and drops
/chat new for "start a new conversation". The prompt-busy branch names nothing
and says the condition clears itself. The spec states the rule so the next edit
does not reintroduce a surface-specific spelling, and the comments point at it
rather than restating it.

Scope note, since #7213 stacks two defects: this PR fixes only the guidance,
not the malformation and not the retry policy. The retry half was already fixed
and merged by #7218, which classifies the string as terminal so the identical
payload is never re-sent -- that is why this text now reaches users at all. The
residue #7213 still carries is untouched here, which is why this PR uses Refs
rather than a closing keyword:

1. what a session whose persisted model id is no longer advertised should do
   (silent fallback to the default vs. an explicit "pick a new model" prompt) --
   a product decision, recorded as such by three triage passes on the issue;
2. releasing or recovering a channel-bound slot without deleting the session;
3. why the ACP runtime dies on this error at all.

Tests

Both pre-existing tests of the malformed branch asserted
"/compact" in out or "/chat new" in out. That disjunction is why the defect
survived: the fabricated command satisfied the assertion on its own, so neither
test would have reddened on a reintroduction. Both are tightened to /compact.

• test/test_acp_error_surface.py --
  test_malformed_request_guidance_names_no_unknown_command and
  test_prompt_busy_guidance_names_no_unknown_command, each over both handle
  raise sites (_wait_for_response and _dispatch_events). Each asserts the
  fabricated command is absent AND that the branch's advice survived, so deleting
  a command cannot silently delete the guidance.
• test/test_acp_true_provider_error.py --
  test_rewrites_into_actionable_prose, the same disjunction, tightened the same
  way. It drives _format_acp_error directly, so the two files cover the
  formatter and both raise sites.

Mutation-verified in both directions: restoring a fresh chat (/chat new).
reddens both parametrizations of the malformed test, and restoring
send \!restart`reddens both parametrizations of the busy test; each returns green on revert. 34 passed intest_acp_error_surface.py, 24 in test_acp_true_provider_error.py. flake8andisort` clean on every touched
Python file.

docs/system-specs/modules/slack-gateway.md quoted the busy string verbatim and
moves with it, so no spec describes a message that no longer exists.

Manual verification

N/A -- unit coverage is the right bar. The change is two user-facing strings
built by a pure formatter, and the tests drive that formatter through both real
raise sites plus the direct entry point. Reproducing a live
Improperly formed request on a Telegram-bound session is not available in this
repo's test environment: pod security forces telegram.enabled=False, so no live
Telegram round trip was attempted and none is claimed.

Related Issues

Refs #7213
Refs #6022

Pattern harvest

Rule candidate: review-prompt
Pattern: user-facing guidance names a command, flag, or path that does not exist
for the reader -- and a test disjunction (assert A in out or B in out) lets the
fabricated half satisfy the assertion by itself. Invisible to every automated
gate: these strings type-check, lint, and here satisfied two separate tests
through the or branch. Three generalizable checks: when a diff adds a command
name to user-facing prose, grep the product for that command AND confirm it is
available on every surface that can render the string (not just that it exists);
treat or in an assertion about a single required affordance as a smell, since
it makes the weaker half sufficient; and when a string is built somewhere with no
surface context, prefer prose over any one surface's spelling.

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 022f563cb7b2a811c74b62969dd7c8173372fb35 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Claims verified: /chat new appears nowhere in product code, /compact is real on Telegram (transport_dispatch.py) and Slack (handler.py), and the spec rule lands in the same commit. The fix names the root cause (surface-blind formatter) and encodes the rule in the spec rather than just patching the strings; tests tighten the disjunction that let the defect pass. No design-level issues.

Design-Verdict: PASS

Surface-blind formatter now emits surface-neutral prose, with the rule pinned in spec and tests — root cause addressed, proportionate scope.

[DESIGN-REVIEWED] 022f563

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 022f563cb7b2a811c74b62969dd7c8173372fb35 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 022f563

Verdict parsed from the review's SHA-scoped output markers for commit 022f563cb7b2a811c74b62969dd7c8173372fb35.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 022f563cb7b2a811c74b62969dd7c8173372fb35: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 022f563cb7b2a811c74b62969dd7c8173372fb35 — 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. /chat new exists nowhere in the product; !restart is a Slack-only bang alias for a gateway restart (slack/handler.py:159); /compact is intercepted or forwarded on Telegram, iMessage, Teams, Slack, and the dashboard. One sibling finding: the same surface-blind formatter still says "switch to a different model in the picker" in three other branches — dashboard vocabulary that is a mislabel on Telegram (whose spelling is /model).

First-Principles-Verdict: CONCERNS

The two inert commands are gone and verified, but the author's own root cause — a surface-blind formatter naming one surface's affordance — has three counted siblings left in the same function.

What this change ships

Intent: stop stuck users receiving repair instructions that do nothing on their surface — a FIX.

1. Malformed-request error drops nonexistent /chat new, keeps /compact, says "start a new conversation" — justified
2. Prompt-busy error drops !restart, says it clears on its own; escalation is prose — justified
3. error-handling.md states the every-surface command rule — justified (same-commit spec invariant)
4. slack-gateway.md quotes the new busy string — justified (same-commit spec invariant)
5. Tests drop the or "/chat new" disjunction that let the defect pass — justified
6. Two new absence tests pinning both branches at both raise sites — justified

Watch

• The stated root cause ("_format_acp_error … cannot know which surface will render its output") has 3 unfixed siblings in the same function: grepped in the picker → acp/client.py:1708, 1757, 1773. "The picker" is dashboard vocabulary; Telegram's spelling is /model, and one string also hardcodes "e.g. sonnet". Same-size string edits, reachable in this scope.
• The spec rule added to error-handling.md governs only commands, so it would not catch a reintroduced surface-specific affordance reference like "the picker" — the rule as written under-covers its own rationale.

[FIRST-PRINCIPLES-REVIEWED] 022f563

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 022f563cb7b2a811c74b62969dd7c8173372fb35 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/acp/client.py:1808 -- "handlers reset and re-queue" contradicts Slack, which only resets for the next message -> Fix: distinguish dashboard re-queue from Slack reset.

[GPT-REVIEWED] 022f563

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 022f563cb7b2a811c74b62969dd7c8173372fb35: <one-sentence reason>

[chenmingwei23]

Disposition of First Principles round 1 (06f97653)

Subtraction: test/test_acp_true_provider_error.py:170 -- ACCEPTED and fixed.
The finding is correct and it caught a real gap in my own verification: I grepped
src/ and docs/ and never test/, which is exactly how the second copy of the
disjunction escaped. The line now reads assert "/compact" in out, the same
one-token deletion already made in the sibling file, with a comment recording why
/chat new was wrong so the or is not restored as a convenience.

I have also corrected the PR description, which claimed "the only other
occurrence in the repo is the spec sentence". That was wrong, and the review is
right that the diff contradicted it. It now states the true pre-change count:
four hits, none of them a real command -- the string, its comment, the spec
sentence, and two test assertions.

Worth naming why the two tightened assertions are not redundant. Alone,
assert "/compact" in out does not catch a reintroduction that adds /chat new
while keeping /compact; that case is caught by the dedicated absence assertion
in test_acp_error_surface.py. The pair covers both shapes, which is why I
tightened rather than replaced.

Watch: assert "new conversation" in msg.lower() pins wording -- acknowledged,
keeping it. The review marked this non-actionable and I agree, with a reason
worth stating rather than leaving implicit. This assertion exists to prove the
advice survived the deletion, so a copy edit to "fresh conversation" reddening it
is the correct outcome, not a false alarm: the author is changing the exact
sentence this test polices and should confirm the reset affordance is still
there. Loosening it to an or over both spellings would reintroduce the very
pattern this PR removes -- an assertion whose weaker half is sufficient.

New head: 3c6761bb2. 56 passed across both test files; flake8 and isort
clean on every touched Python file.

[chenmingwei23]

Disposition of First Principles round 2, plus a rebase (2858ca913)

Subtraction: four prose copies of one constraint -- ACCEPTED and applied.
The review is right that the "formatter is surface-blind, name only universal
commands" rule was written out at length in four places and would drift. The spec
(docs/system-specs/common/error-handling.md) is now the single authoritative
statement; the comment in acp/client.py and the one in
test_acp_true_provider_error.py are cut to a one-line pointer at it. The new
test's docstring in test_acp_error_surface.py is kept as prose because it is
explaining what that specific assertion pins, not restating the rule.

Worth being explicit about the timing, because it changed the answer. All five
lanes were already PASS on 3c6761bb2, so on its own this advisory subtraction
did not justify re-rolling ~70 checks and five non-deterministic review lanes to
shorten two comments. What made it free was needing a force-push anyway:

Rebased onto current main (556f08f93) to clear the dependency-audit red.
Dependency Audit / Audit Production Dependencies was failing on four fast-uri
advisories in website/electron/package-lock.json, a file this diff does not
touch; it failed identically on unrelated open PRs, so it was repo-wide and not
this PR's to fix. Main has since fixed it -- commit 0545b668e,
"fix(deps): unpin fast-uri so the patched 3.1.7 can resolve" (#7936) -- and
fast-uri now resolves to the patched 3.1.7. Rebasing cuts a fresh merge ref
that carries that fix, rather than folding a lockfile change into a
guidance-string PR.

The rebase was clean and the diff is unchanged in substance. Three of main's new
commits touch acp/client.py (#7710, #7694, #7366) but none overlap this hunk,
and the branch is still exactly one commit ahead of main.

Re-verified on the new head: 32 passed in test_acp_error_surface.py, 24 passed
in test_acp_true_provider_error.py, flake8 and isort clean on all three
touched Python files, and no non-ASCII in any added line.

One process note for the record: this push needed SCRUBGATE_OVERRIDE=1. The
override covers only main's replayed commit messages, which the rebase pulled in
and which carry other authors' internal email addresses and non-ASCII
punctuation. This branch's own single commit, its author field, and the branch
name are all pure ASCII with no internal references -- verified before
overriding.

[chenmingwei23]

Disposition of First Principles round 3 (022f563cb)

Subtraction: the !restart sibling at acp/client.py -- ACCEPTED and fixed.
The finding is correct, it is the same defect class in the same function, and
this PR's own Pattern harvest declares that class -- so shipping one instance
while leaving its twin ten lines away would have made the harvest claim hollow.

I verified every part of it before acting, and it is worse than stated. git grep '!restart' -- src/ returns this string plus two Slack sites (handler.py:159
alias map, events.py:2555 intercept). It is owner-gated (events.py:745, "Only
the owner can restart the gateway") and it is registered as "restart the gateway
(owner-only)" at events.py:829 -- so it does not reset the session, which is
what the sentence promised. And it was never needed: both the dashboard
(chat_runner.py, retry-eligible branch) and Slack reset and re-queue on
AcpPromptBusy on their own. So the text asked the user for a command that does
not exist on their surface, would be refused if it did, would do the wrong thing
if allowed, and was redundant with the automatic recovery.

The branch now reads "it clears on its own once the stale turn expires. If it
persists, start a new conversation" -- naming no command, the prose form this PR
established one branch above.

Watch: the /compact claim was imprecise -- corrected. The review is right
that Slack intercepts !compact, so "spelled the same everywhere" was not
accurate. The spec now states the real rule -- a command may be named only if
every surface UNDERSTANDS it, and /compact qualifies because it reaches the
backend through the prompt transport even on Slack. That is a better rule than
the one I wrote, because it is the property that actually matters to the reader.

Tests

test_prompt_busy_guidance_names_no_unknown_command, both raise-site
parametrizations: asserts !restart absent and the recovery advice present, so
deleting the command cannot silently delete the guidance. Mutation-verified --
restoring send \!restart` reddens both parametrizations (assert '!restart' not in ...), green again on revert. 34 passed in test_acp_error_surface.py, 24 in test_acp_true_provider_error.py; flake8andisort` clean on all touched Python files.

docs/system-specs/modules/slack-gateway.md quoted the old string verbatim and
moved with it, so the spec does not describe a message that no longer exists.

The commit title is now "stop error guidance naming commands that do not exist"
-- plural, since the change covers two branches rather than one.

[chenmingwei23]

Disposition of First Principles round 4 (022f563cb) -- declining the three "picker" strings

The verified half is welcome: the review re-confirmed independently that
/chat new exists nowhere, that !restart is a Slack-only bang alias for a
gateway restart, and that /compact is intercepted or forwarded on Telegram,
iMessage, Teams, Slack and the dashboard. That is the PR's core claim, checked
against the repo a third time.

I am declining the Watch item, because its premise does not hold. The finding
says "the picker" is "dashboard vocabulary that is a mislabel on Telegram (whose
spelling is /model)". Telegram has a model picker, and Telegram's own code is
what calls it that:

• telegram/transport_dispatch.py:226 -- class _Picker
• telegram/transport_dispatch.py:392 -- self._model_pickers: dict[str, _Picker]
• telegram/transport_dispatch.py:206-207 -- _MODEL_PICKER_TTL_SECS, _MODEL_PICKER_MAX

/model on Telegram posts an inline keyboard and is deliberately button-only;
the retained table those buttons index into is the picker. Discord uses the same
word for its own equivalents (discord/renderer.py:312, "the dispatcher's picker
registry"; discord/session_resume.py:1, "Discord session picker"). So "the
picker" is not one surface's vocabulary leaking into a surface-blind formatter --
it is the product's own cross-surface name for the affordance, and it is present
on the surface the finding names as the counter-example.

This is also why the three strings are not the defect class this PR fixes. That
class is an instruction that CANNOT BE FOLLOWED: /chat new was a command that
does not exist, !restart was a command the reader could not run and which did
something else. "Switch to a different model in the picker" names no command at
all -- it describes where to do the thing, which is precisely the prose form this
PR establishes. Those strings already satisfy the rule; changing them would swap
a correct description for a different correct description and re-roll ~70 checks
and five non-deterministic review lanes to do it.

e.g. sonnet in the throttle branch is a weaker version of the same point. It is
an illustrative model family, not an instruction, and the branch that fires when
a model is genuinely refused already names the account's own advertised set via
_model_is_unentitled. Worth noticing, not worth a round.

The spec-scope observation is fair and I am recording it rather than acting on
it. The rule I added to error-handling.md governs command names, so it would
not by itself catch a reintroduced surface-specific affordance reference. Given
that "the picker" turns out to be cross-surface, that gap is not leaving a live
hole here -- but it is a real narrowing of the rule against its own rationale. If
any further push becomes necessary on this PR I will widen the sentence to cover
affordances as well as commands; I am not spending a CI cycle on the wording
alone.

Recorded rather than silently dropped, per the standing rule that every finding
gets a written disposition. This is round 4 on a two-string fix; the two defects
the PR exists for are fixed, verified, and mutation-tested, and I am treating the
remaining items as convergence rather than scope.

[iamwhatever]

Tier 1 auto-approve: fix (5 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: ACP error guidance printed remediation commands that do not exist, so the text is corrected to only name real commands; change is confined to acp/client.py plus its two error-surface tests. Spec files changed as a ride-along (a minority of the diff on both file count and changed lines), not reviewed as a design decision: docs/system-specs/common/error-handling.md, docs/system-specs/modules/slack-gateway.md.