Skip to content

fix: hide unsupported /todos slash command from menu - #8829

Merged
iamwhatever merged 1 commit into
mainfrom
fix/hide-unsupported-todos-slash-command
Sep 6, 2026
Merged

fix: hide unsupported /todos slash command from menu#8829
iamwhatever merged 1 commit into
mainfrom
fix/hide-unsupported-todos-slash-command

Conversation

@bolichen97

@bolichen97 bolichen97 commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Problem / Motivation

The dashboard slash-command menu offered /todos, but the kiro-cli ACP harness does not implement it and rejects the request with an "unknown variant" error. Selecting the menu item therefore always failed.

Why it matters

Every user who picks /todos from the menu gets an error instead of a result. Advertising a command that never works teaches a gesture that does not work, and the same inert entry was visible on the frontend's offline fallback list too.

What changed (motivation → approach → change)

Symptom: /todos in the menu → "unknown variant" error from the harness.

Root cause: /todos was listed in _SLASH_COMMANDS (so the menu advertised it) but not in _BLOCKED_SLASH_COMMANDS, the set that hides TUI-only / unsupported commands from GET /api/slash-commands and refuses them at execution time. The frontend's FALLBACK_COMMAND_NAMES (rendered before the API resolves, or when it fails) is documented as _SLASH_COMMANDS minus _BLOCKED_SLASH_COMMANDS and still listed /todos as well.

Change:

  • src/kiro_crew/dashboard/chat_utils.py: add /todos to _BLOCKED_SLASH_COMMANDS, with a comment explaining why. This reuses the existing mechanism (the /tangent precedent).
  • website/src/components/SlashCommandMenu.tsx: remove /todos from FALLBACK_COMMAND_NAMES so the fallback path hides it too, and update the comment.

Pre-existing drift left alone: /goal is in _SLASH_COMMANDS but absent from FALLBACK_COMMAND_NAMES; that is the same mirror-drift class but unrelated to /todos, so it is left for a follow-up rather than widening this fix.

Scope cut, stated explicitly: _BLOCKED_SLASH_COMMANDS is harness-global, so /todos is also hidden and refused on the claude backend. This PR deliberately does not add per-provider capability gating; if a second harness-specific gap appears, a per-backend blocked set is the right follow-up shape.

Tests

  • test/test_api_slash_commands.py: extended test_blocked_commands_absent_from_suggestions to assert /todos is not advertised by GET /api/slash-commands.
  • website/src/test/SlashCommandMenu.test.tsx: added /todos to the BLOCKED list pinned absent on the API-failure fallback path (fails with /todos still in FALLBACK_COMMAND_NAMES, passes without it).
  • pytest test/test_api_slash_commands.py → 4 passed; vitest run src/test/SlashCommandMenu.test.tsx → 14 passed; flake8 and eslint clean on touched files.

Manual verification

Built SPA, menu opened with /. Before: /todos was listed on both surfaces. After (this branch):

Fallback menu (API stubbed to 503) -- no /todos row; typing /tod matches nothing:

fallback menu without /todos
fallback menu, /tod query matches nothing

Live API menu (payload from the fixed backend) -- same result:

live menu without /todos
live menu, /tod query matches nothing

Related Issues

no linked issue: reported directly, no tracked issue exists for this defect.

Pattern harvest

Rule candidate: review-prompt
Pattern: a hand-maintained frontend fallback list documented as "mirrors backend set X minus Y" drifts silently when Y changes in a backend-only PR — a change to _BLOCKED_SLASH_COMMANDS should always be checked against FALLBACK_COMMAND_NAMES.

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

Backport: please include in release/0.6.0.

@bolichen97
bolichen97 requested a review from a team as a code owner September 5, 2026 22:14
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 21231ad8072de4dac016843e5a9590be2e4547dc and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 21231ad

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

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 21231ad8072de4dac016843e5a9590be2e4547dc — 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 evidence is in. The fix reuses the existing _BLOCKED_SLASH_COMMANDS mechanism (its real consumers: the refusal at chat_runner.py:6269 and the menu filters at agents.py:2276 and agents.py:2292), the frontend fallback list is a documented mirror of those sets, and temp-screenshots/ is the PR-template-mandated evidence directory — so the PNGs are derived, not riders. The one collateral effect (the claude backend also loses /todos, via the dynamic provider._slash_commands path at agents.py:2258) is declared in the description as a deliberate scope cut, and the alternative — per-backend blocked sets — would be a new concept with exactly one consumer today. The declared /goal fallback drift is a counted (1) sibling of the mirror-drift class, already acknowledged and deferred by the author.

First-Principles-Verdict: PASS

Removes a menu entry that always errors, using the existing blocklist and its documented frontend mirror — nothing new is invented.

What this change ships

Intent: stop the dashboard from offering /todos, which the kiro-cli harness always rejects. This is a FIX.

  1. /todos no longer appears in the slash-command menu — justified (guaranteed "unknown variant" error).
  2. Typing /todos now gets the dashboard's explicit refusal instead of a harness error — justified (existing chat_runner.py:6269 path).
  3. /todos gone from the offline fallback menu — justified (documented mirror of _SLASH_COMMANDS − _BLOCKED_SLASH_COMMANDS).
  4. /todos also hidden and refused on the claude backend — declared scope cut; collateral of the harness-global set.
  5. Four screenshots under temp-screenshots/slash-menu-no-todos/ — justified (PR-template-mandated evidence dir).

Watch

  • Item 4 is live, not hypothetical, whenever a claude provider reports todos in _slash_commands (agents.py:2258): a working command would be hidden and refused. The author names per-backend gating as the follow-up shape; today it would be a one-consumer generalization, so deferring is right — but check this before honoring the release/0.6.0 backport request.
  • /goal fallback drift: 1 unfixed sibling of the same hand-maintained-mirror cause, declared and deferred by the author.

[FIRST-PRINCIPLES-REVIEWED] 21231ad

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

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

Analysis complete. The fix reuses the existing /tangent mechanism cleanly, and the committed screenshots are an established repo pattern per .gitignore. The one real design signal: _BLOCKED_SLASH_COMMANDS previously held only terminal-only gestures unsupported everywhere; /todos is the first harness-specific gap put in it, and the execution-time block at chat_runner.py:6269 fires before provider dispatch on every backend — including the claude backend, whose adapter routes slash commands through session/prompt where its SDK may handle /todos natively.

Design-Verdict: CONCERNS

A kiro-specific capability gap is fixed with a harness-global block, downgrading /todos on the claude backend from possibly-working to a hard dashboard refusal.

Watch

  • Every prior _BLOCKED_SLASH_COMMANDS entry is terminal-only on all backends; /todos is the first entry blocked for a single harness's gap. The PR's own scope-cut note ("if a second harness-specific gap appears, a per-backend blocked set is the right follow-up shape") miscounts — this PR is the first such gap. On the claude backend, stream_command routes through session/prompt where the SDK interprets supported commands natively (providers/acp.py:1413-1430), so if that harness handles /todos, chat_runner.py:6269 now refuses a working command with "not available in the dashboard." Verify claude-backend behavior before merging; if it works there, the per-backend set is this fix's requirement, not a follow-up.

[DESIGN-REVIEWED] 21231ad

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 21231ad8072de4dac016843e5a9590be2e4547dc — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 21231ad

Verdict parsed from the review's SHA-scoped output markers for commit 21231ad8072de4dac016843e5a9590be2e4547dc.

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

@bolichen97
bolichen97 requested a review from a team September 5, 2026 23:17
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — ✅ PASS

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

UX-Verdict: PASS

Removing the dead /todos entry deletes a lying affordance; screenshots confirm both menu paths and the /tod no-match state, and the blind reader read every surface correctly.

The blind reader parsed both post-removal notices accurately ("nothing… starts with 'tod', and pressing Enter will send it as plain text") and found shots 2/4 indistinguishable — which is the fix working, since fallback and live menus are meant to mirror. The reader's unease that "Couldn't load commands" "sounds like something went wrong" concerns a pre-existing string this diff does not touch (en.manual.json:2591), and on that path the API genuinely failed, so the notice is truthful. No user-visible control is added or transformed, so no recording is needed.

[UX-REVIEWED] 21231ad

@bolichen97
bolichen97 force-pushed the fix/hide-unsupported-todos-slash-command branch from 48746b9 to ca15a11 Compare September 5, 2026 23:46
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@bolichen97
bolichen97 force-pushed the fix/hide-unsupported-todos-slash-command branch from ca15a11 to 8f5233e Compare September 6, 2026 00:40
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 6, 2026
@bolichen97
bolichen97 force-pushed the fix/hide-unsupported-todos-slash-command branch from 8f5233e to d5e7e69 Compare September 6, 2026 08:00
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator Author

Design finding 1 — "a global blocked set now encodes one harness's gap, stripping a live-advertised command from the claude backend": partially rebutted, partially accepted-and-deferred to #8939.

The claude branch of api_slash_commands filters the harness's own advertised commands (for c in cc_commands if f"/{c}" not in _BLOCKED_SLASH_COMMANDS) ... Claude Code implements /todos, so this hides and refuses a working command there

The menu half does not hold. api_slash_commands reads the claude command list
with getattr(provider, "_slash_commands", []), and that attribute is assigned
nowhere in src/ — the only assignment in the repository is a SimpleNamespace
inside test/test_api_slash_commands.py:91. With the [] default, cc_commands
always falls through to the hardcoded list on the branch itself:

cc_commands = ["compact", "clear", "context", "help", "init",
               "review", "security-review", "usage"]

plus the appended /side and /workflow. todos is not in it, so
GET /api/slash-commands never advertised /todos under claude_code — before
this PR or after it. Nothing is being stripped from that payload.

The runtime half does hold. chat_runner.py:6137 consults
_BLOCKED_SLASH_COMMANDS before the provider is considered, so a claude_code user
who types /todos now gets the dashboard refusal where is_harness_slash_command
would previously have forwarded it. That is accepted and tracked in #8939, not fixed
here, because the correct remedy turns on a fact this repository does not encode:
whether the Claude Code harness actually accepts /todos. There is no
harness-reported command list to read (see above), so scoping the block on an
unverified premise risks re-introducing the very bug this PR fixes — an advertised
or forwarded command the harness rejects — on the claude path instead. #8939
therefore starts by making the claude list harness-derived, then splits the set.

This PR's commit message already records the global block as a deliberate choice
with the per-backend set as the follow-up shape; #8939 is that follow-up, with an
owner and a due date.

@bolichen97

Copy link
Copy Markdown
Collaborator Author

Design finding 2 — "~600KB of PNGs ... once on main they are permanent history; attach verification screenshots to the PR instead and drop the directory before merge": rebutted.

~600KB of PNGs merged into the tree under temp-screenshots/ solely to back commit-pinned image URLs in the PR body; once on main they are permanent history.

The committed-under-temp-screenshots/ shape is what this repository's own gate
instructs, not an author choice. .github/workflows/screenshot-evidence.yml
prints the remedy for a missing-evidence failure verbatim:

2. Commit the images under `temp-screenshots/<feature>/` -- never under
   `![alt](https://github.com/$REPO/raw/<sha>/temp-screenshots/<feature>/<name>.png)`

The "permanent history" premise is also incorrect. .github/workflows/cleanup-temp-screenshots.yml
exists for exactly this growth concern — a weekly scheduled prune
(cron: "0 8 * * 1", default retention_days: 5) that opens a PR because main is
protected. Its header states the lifecycle:

Screenshots are embedded in PR descriptions via commit-SHA-pinned raw URLs, which keep resolving from the pinned historical commit even after the file is deleted from main's tip -- so pruning old files never breaks a past PR's images.

So these four files are pruned on a timer within days of merge, and the PR body's
images keep resolving afterwards. That property is what rules the suggested
alternative out: a user-attachment URL cannot be SHA-pinned, so it is not
reproducible against the commit it evidences, and re-pinning after every amend —
which this PR just did across all four URLs — is only possible for the same-origin
raw/<sha>/ form.

Keeping the code as-is. The 628KB is four PNGs of a menu at two states on two menu
paths, which is the evidence the gate asked for.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@bolichen97

bolichen97 commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator Author

Inherited main red on head d5e7e691 — evidence, and how it was resolved

Recording this on the PR so it does not have to be re-derived. Resolved as of head
bc64cfbaf
: the cause was a main-side regression, it has since been fixed on
main, and this PR consumed the fix with a rebase.

What was red, and the cascade

On head d5e7e691366bc57afca7223cfd8ef3d961339a21, Frontend Tests (3) failed on
website/src/i18n/deadKeys.test.ts — both cases, 30 catalog keys are referenced nowhere (baseline 29). Everything else followed from it:

Frontend Tests (3)      -> deadKeys.test.ts: 30 dead keys vs BASELINE 29
Frontend Coverage Merge -> fail-closes off it
Coverage Gate           -> "frontend-test=failure -- failing closed"  (backend-test=success)
PR Readiness            -> red, and it is the sole required check under ruleset 20088190

All four backend shards passed (Backend Tests (3.12, 1) through (3.12, 4)).

The orphaned key, and who orphaned it

The 30th dead key was pages.chatPage.dismiss_upload_error — still present in every
locale catalog, but no longer referenced anywhere in website/src. Reference count per
revision:

revision files referencing the key
31bd900ff 2
2d18f68f2#8816, "route error states through ErrorNotice (batch pages-rest-1)" 1
b75889c69#8859, "route error states through ErrorNotice (batch components-1)" 0

Two ErrorNotice routing batches replaced bespoke error copy with a shared component;
the call site went away while the catalog entry stayed. That is the failure mode the
test's own message names — "copy deleted without its key".

Measured, not inferred

An offline replication of the test's own dead computation (flatten the catalog, then
count keys with no quoted literal occurrence in website/src/**/*.{ts,tsx}), validated
by reproducing CI's 30 on this head before comparing anything:

revision dead.length
d5e7e691 — this PR's head at the time 30
cb3aa2c5borigin/main at the time 30
d580bcb2e — this PR's rebase base, pure main 30
a8205e559 — this PR's pre-rebase base 29

The rebase base already measured 30 with none of this PR's changes in it.

Why this diff cannot move the count

  • It touches no locales/, i18n/, catalog or pluralKeys.json file.
  • It adds and removes zero quoted dotted catalog keys.
  • components.slashCommandMenu.desc_todos is still literally referenced at
    SlashCommandMenu.tsx:73 ('/todos': 'components.slashCommandMenu.desc_todos'), and
    is confirmed absent from the full 30-key dead list. The printed list is
    dead.slice(0, 12) and is sorted, so that key would have appeared at position 11 —
    between components.runInTerminalBtn.run_anyway and pages.artifactDetailPage.iterate
    had it been dead.

Why it surfaced here first

#8859 merged at 07:54:57Z. This PR's rebase base d580bcb2e is dated 07:55:20Z,
so the regression was already in the base 23 seconds later. main's own runs around
07:55 were largely cancelled by concurrency, so this PR was the first place the red
became visible. It was never this PR's defect.

Resolution

02fa7c201 — "fix(i18n): drop the dead pages.chatPage.dismiss_upload_error key",
on main at 08:34:35Z — removed the key from the catalogs, which is the remedy the
test's doctrine calls for (ratchet down when keys are removed; never raise BASELINE).
origin/main now measures dead=29 against BASELINE = 29, key count 12600 → 12599.

This PR rebased onto it: head bc64cfbaf74d5377e6e011060068e5c87116c287 measures
dead=29. The code diff is byte-identical across the rebase, still one commit on base.
BASELINE was deliberately not touched from this PR — bending a repo-wide ratchet
inside a "hide an unsupported slash command" fix would have absorbed another PR's
regression under an unrelated title and left the ratchet blind to the next orphaned key.

Note for anyone hitting this class again: consume a main-side fix with a rebase.
gh run rerun re-tests the original merge-ref snapshot and fails identically.

Tracked at #8959.


Second inherited red on head bc64cfbafBackend Tests (3.12, 4)

The rebase cleared the deadKeys cascade above. What remains is a different
main-side red, also not reachable from this diff:

Backend Tests (3.12, 4) -> test/test_snapshot.py::TestNotificationCopyWhenNoLiveFileExists
Coverage Gate           -> fail-closes off it
PR Readiness            -> red, and it is the sole required check

The failing assertion is
test_a_FRESH_gateway_still_orders_the_copy_against_a_delivery
a delivery on a fresh gateway ran concurrently with the copy: 'no pool' was read as 'no writer'.

test/test_snapshot.py is not in this PR's diff, and a slash-command frozenset plus a
frontend menu list have no coupling to snapshot notification-copy ordering. The class
passes deterministically in isolation locally (25 passed, 4 consecutive runs).

It fails on main itself, alternating between the class's two ordering assertions:

run head failing method
34022725589 08:45Z 8aef8fe3f (main) test_a_FRESH_gateway_still_orders_the_copy_against_a_delivery — same method and assertion as here
34026408948 10:04Z d5955744f (main) test_a_note_delivered_during_the_copy_survives_the_READER

Interleaved with green main runs (d3e9cfe93 08:12Z success, d4c2cbf22 09:15Z
success), so the class reds shard 4 on main roughly half the time. The class arrived in
be7de18fb (#8576, "fix(snapshot): validate notification records the copy branch
installs").

Already tracked as #8893 — "flaky: TestNotificationCopyWhenNoLiveFileExists ordering
asserts race on loaded CI shards" (open), with #8915 and #8976 closed against the same
symptom.

One targeted gh run rerun --failed was spent on it (attempt 2 of run 34024712213) and
it failed identically. Note that a --failed rerun re-tests the original merge-ref
snapshot, so it re-rolls runner timing but not the tree — with the class failing ~50% of
the time on main, further reruns are a coin flip rather than a fix, so this PR stops
here rather than burning CI on them.

This PR needs no content change for it. Once #8893 lands on main, consume it with a
rebase — never gh run rerun.

@bolichen97
bolichen97 force-pushed the fix/hide-unsupported-todos-slash-command branch from d5e7e69 to bc64cfb Compare September 6, 2026 09:27
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 6, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator Author

Design finding 1 — "it ships 600KB of temp-screenshots/ into permanent git history ... upload them via GitHub's attachment flow and drop the directory before merge": rebutted, and the "never cleanly deleted" premise is incorrect.

once merged, every clone carries them forever, and the description hotlinks them at this commit SHA so they can never be cleanly deleted

This shape is what the repo's own gate instructs, not an author preference.
.github/workflows/screenshot-evidence.yml prints it as the remedy for a
missing-evidence failure:

2. Commit the images under `temp-screenshots/<feature>/` -- never under
   `![alt](https://github.com/$REPO/raw/<sha>/temp-screenshots/<feature>/<name>.png)`

The "never cleanly deleted" premise is the part that does not hold, and the repo
answers it directly. .github/workflows/cleanup-temp-screenshots.yml is a weekly
scheduled prune (cron: "0 8 * * 1", default retention_days: 5) that opens a PR
because main is protected. Its header states exactly why SHA-pinning is what makes
deletion safe:

Screenshots are embedded in PR descriptions via commit-SHA-pinned raw URLs, which keep resolving from the pinned historical commit even after the file is deleted from main's tip -- so pruning old files never breaks a past PR's images.

So the hotlink is not what traps the files; it is what lets them be pruned without
breaking this PR's description. These four PNGs are pruned on a timer within days of
merge.

That property is also what rules out the suggested alternative: a GitHub
user-attachment URL cannot be SHA-pinned, so it is not reproducible against the commit
it evidences, and it cannot be re-pinned after an amend — which this PR has now done
twice across two rebases.

Keeping the code as-is: 628KB is four PNGs covering two menu paths in two query states,
which is the evidence the gate asked for.

@bolichen97

bolichen97 commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator Author

Design finding 2 — "a claude-backend harness that actually implements /todos now has it hidden and refused ... a human should confirm the claude backend doesn't report /todos": needs-a-decision, and the "hidden" half is already disproved.

_BLOCKED_SLASH_COMMANDS is harness-global and enforced on the provider-reported path too (test_claude_code_provider_filters_blocked_commands), so a claude-backend harness that actually implements /todos now has it hidden and refused at execution time

The "hidden from the menu" half does not hold on the shipped code

The claude branch of api_slash_commands reads its command list with
getattr(provider, "_slash_commands", []), and that attribute is assigned nowhere in
src/ — the only assignment anywhere in the repository is a SimpleNamespace inside
test/test_api_slash_commands.py:91, i.e. the very test cited above constructs the
provider-reported path that production never populates. With the [] default,
cc_commands always falls through to the hardcoded list on the branch itself:

cc_commands = ["compact", "clear", "context", "help", "init",
               "review", "security-review", "usage"]

plus the appended /side and /workflow. todos is not in it, so
GET /api/slash-commands has never advertised /todos under claude_code — before this
PR or after it. Nothing is hidden from that payload.

The "refused at execution time" half does hold

chat_runner.py:6137 checks _BLOCKED_SLASH_COMMANDS before the provider is considered,
so under claude_code a typed /todos now gets the dashboard refusal where
is_harness_slash_command would previously have forwarded it.

The decision this finding asks for, put to a maintainer

The question is narrow: does the Claude Code harness accept /todos? The repository
cannot answer it — nothing populates provider._slash_commands, so there is no
harness-reported command list to read, and the hardcoded fallback is a guess written into
the branch rather than evidence. Two outcomes:

Not resolving it inside this PR, deliberately: scoping the block on an unverified premise
would re-introduce the exact class of bug this PR fixes — an advertised or forwarded
command the harness rejects — on the claude path instead. This PR's commit message
already records the harness-global block as a deliberate choice with the per-backend set
as the follow-up shape, and #8939 is that follow-up with an owner and a due date.

The dashboard slash-command menu offered /todos, but the kiro-cli ACP
harness does not implement it and rejects the request with an "unknown
variant" error, so selecting the entry always failed.

/todos was listed in _SLASH_COMMANDS (so the menu advertised it) but not in
_BLOCKED_SLASH_COMMANDS, the set that hides TUI-only / unsupported commands
from GET /api/slash-commands and refuses them at execution time. Add it
there.

The frontend keeps a hand-maintained FALLBACK_COMMAND_NAMES list that is
documented as "_SLASH_COMMANDS minus _BLOCKED_SLASH_COMMANDS" and is
rendered before the API resolves (or when it fails); drop /todos from it
too so both menu surfaces agree, and pin its absence in the offline
fallback test.

Deliberately a harness-global block with no provider-capability gating;
a per-backend blocked set is the follow-up shape if a second such command
appears.

Screenshot evidence for the visible menu change is under
temp-screenshots/slash-menu-no-todos/ (fallback menu and live-API menu,
each with a "/tod" query that now matches nothing).

Co-authored-by: Kiro Crew <noreply@kiro.dev>
@bolichen97
bolichen97 force-pushed the fix/hide-unsupported-todos-slash-command branch from bc64cfb to 21231ad Compare September 6, 2026 16:05
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 6, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator Author

Design finding — "a kiro-specific capability gap is fixed with a harness-global block, downgrading /todos on the claude backend ... verify claude-backend behavior before merging": needs-a-decision, with the new acp.py evidence verified and the miscount conceded.

On the claude backend, stream_command routes through session/prompt where the SDK interprets supported commands natively (providers/acp.py:1413-1430), so if that harness handles /todos, chat_runner.py:6269 now refuses a working command

Both cited call sites check out against source

chat_runner.py:6269 is provider-agnostic — if first_word in _BLOCKED_SLASH_COMMANDS: with no provider condition, evaluated before session acquisition. And acp.py's stream_command does exactly what the finding says, in its own words:

_kiro.dev/commands/execute is a kiro extension, so only ACP_BACKENDS_KIRO_SLASH_COMMANDS members can be sent it. Everyone else routes through session/prompt, which an adapter interprets natively for the commands its SDK supports (/compact, /help, /model, /context, …). Commands it doesn't recognise (kiro-only ones like /agent, /experiment, /hooks) flow through as conversational prompt text — a softer failure mode than a -32601 "Method not found" on the whole call.

This is a better-aimed argument than the previous round's, and it is accepted as such: it identifies the actual mechanism rather than the menu payload, which is where my earlier response showed the claim did not hold (provider._slash_commands is assigned nowhere in src/, so the claude menu never advertised /todos).

The miscount is conceded

The commit message's "if a second harness-specific gap appears" is wrong: /todos is the first entry in _BLOCKED_SLASH_COMMANDS blocked for one harness's gap rather than being terminal-only everywhere. Not amending for it — the wording error changes no behaviour, and an amend would void a converged all-green CI wave and re-arm all five review lanes to fix a sentence. Recording the correction here instead, and noted on #8939.

What the decision turns on, and why it is still a decision

The repository cannot establish whether the Claude Code SDK implements /todos: nothing populates provider._slash_commands, and acp.py's comment lists the SDK's supported set only by example. So both branches remain open, and they do not point the same way:

  • If the SDK handles /todos, this is a live regression and the per-backend set is this fix's requirement, exactly as the finding argues.
  • If it does not, the prior behaviour was /todos arriving as conversational prompt text — and an explicit "not available in the dashboard" is arguably the better of the two, not a downgrade.

That asymmetry is why this is not something to resolve by guessing: scoping the block on an unverified premise would re-introduce the class of bug this PR fixes — a forwarded command the harness does not implement — on the claude path instead. Putting the question to a maintainer, which is what the finding asks for. #8939 owns the implementation either way and starts by making the claude command list harness-derived rather than hardcoded, which is what would let a gate answer this mechanically instead of by inspection.

@iamwhatever
iamwhatever merged commit f6867db into main Sep 6, 2026
69 of 81 checks passed
@iamwhatever
iamwhatever deleted the fix/hide-unsupported-todos-slash-command branch September 6, 2026 18:17
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants