feat(chat): agent-suggested follow-ups as a card above the composer - #461
Conversation
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsFINDING -- src/kiro_crew/dashboard/handlers/worktree.py:154 -- Native Windows has no sandbox backend, so False positive or not applicable? A repository writer can comment: |
Design Review (Fable 5) — 🟡 CONCERNSAdvisory design-level review of This is enough to assess the design. I have the full picture: MCP tool → gateway endpoint (re-validates, redacts, owner-only broadcast) → worktree endpoint (sandbox-routed git, allow-list, atomic branch claim) → frontend card with prefill-only actions. One thing I confirmed: Design-Verdict: CONCERNS Sound design for a real problem, but the headline "Start in new worktree" action is dead on Windows (a first-class platform) and the PR's sandbox-classification section describes a design the code reversed. Watch
Suggestions
[DESIGN-REVIEWED] b1d42b9 |
3c46e32 to
2365ba9
Compare
Round 1 disposition — head
|
93361a0 to
0d8977e
Compare
Round 2 disposition — head
|
Opus 5 Review — ✅ no blocking findingsReviewed No findings. Verdict recorded via the action's structured output for commit False positive or not applicable? A repository writer can comment: |
Arbiter — ✅ no blocking findingsArbiter found no unresolved long-term items that require action before merging Second-order review for Review detailsI have the two files. There are only three sub-threshold items to judge: the GPT 5.6 finding (worktree 503 on Windows), and the design review's two CONCERNS (worktree inert on sandboxless platforms; stale PR description). Opus found nothing. Let me assess each against the narrow blocking bar. All three describe the same underlying behavior or documentation drift, and none are one-way doors or concrete harm:
Nothing in the diff I reviewed leaks a secret or pulls in an incompatible dependency. Arbiter-Verdict: PASS No sub-threshold finding meets the long-term-impact bar. Suggested follow-ups (open as issues — non-blocking)
[ARBITER-REVIEWED] b1d42b9 False positive or not applicable? A repository writer can comment: For a broader accepted-risk deferral, apply |
bf2e3cc to
8e16a44
Compare
Round 3 disposition — head
|
8e16a44 to
c0c0d9f
Compare
Round 4 disposition — head
|
c0c0d9f to
1dc7876
Compare
Round 5 disposition — head
|
1dc7876 to
5931e92
Compare
Round 6 disposition — head
|
5931e92 to
3b138b0
Compare
Round 7 disposition — head
|
c079c78 to
ecdf8cc
Compare
Round 8 disposition — head
|
ecdf8cc to
669a207
Compare
Round 9 disposition — head
|
1d9edfa to
bbab33c
Compare
Round 12 disposition — head
|
c6568b5 to
2d45c1c
Compare
Adds a `suggest_followup` MCP tool (kirocrew-core) that lets the agent offer
up to three concrete next steps as a card above the chat composer. Each item
carries an expanded, self-contained handoff prompt and three actions: start it
in a new git worktree, add it to the current session, or skip.
Both non-skip actions PRE-FILL a composer and stop. Nothing is sent until the
user presses send, so a single click can never launch an unattended agent turn.
Backend:
- SUGGEST_FOLLOWUP_SCHEMA in validation.py gates item count, per-field types
and lengths, unknown fields, hidden Unicode, and full-matches an optional
branch name against FOLLOWUP_BRANCH_RE.
- POST /api/chat/slots/{slot}/followup re-validates the same schema (it is
reachable over loopback from inside the kiro-cli process group, so it is a
trust boundary, not a relay), redacts credentials and exfiltration URLs, and
broadcasts a `followup_card` WS event.
- POST /api/worktree/create builds a sibling worktree via
sandboxed_spawn_argv + resource_limit_preexec with an argv list and no shell,
resolves the repo to its git toplevel, refuses sensitive paths, and derives
the destination server-side.
- The tool is dashboard-only via _resolve_session_key_strict(); Slack, cron and
subagent contexts fail closed.
Frontend:
- FollowUpCard renders in the same above-composer band as the question card,
gated on slot ownership, with a single-flight guard and inline error on
worktree failure.
- Skip drops one suggestion and keeps its siblings; the card clears when its
last item is gone.
Tests: 49 backend (schema, endpoint, worktree endpoint against a real throwaway
repo) and 11 frontend (component behaviour plus the four new reducers).
2d45c1c to
b1d42b9
Compare
Round 13 disposition — head
|
…levant (#597) PR #461 shipped the suggest_followup MCP tool and its follow-up card, but nothing prompted the agent to reach for it — usage rode entirely on the tool's own description, and with MCP Tool Search enabled that spec is not in every turn's context, so the tool would rarely be surfaced. Inject a situational, dashboard-only reminder into the per-turn interactive block (the same place the [OPTIONS:] reminder lives). It is gated to dashboard: / dashboard_ session keys because the tool rejects Slack, cron, and subagent contexts, and it is framed as optional and turn-end — not per-turn — so it raises awareness without becoming noise. Co-authored-by: Kyle Seaman <kseam@dev-dsk-kseam-1b-55230d27.us-east-1.amazon.com>
…irodotdev#461) Adds a `suggest_followup` MCP tool (kirocrew-core) that lets the agent offer up to three concrete next steps as a card above the chat composer. Each item carries an expanded, self-contained handoff prompt and three actions: start it in a new git worktree, add it to the current session, or skip. Both non-skip actions PRE-FILL a composer and stop. Nothing is sent until the user presses send, so a single click can never launch an unattended agent turn. Backend: - SUGGEST_FOLLOWUP_SCHEMA in validation.py gates item count, per-field types and lengths, unknown fields, hidden Unicode, and full-matches an optional branch name against FOLLOWUP_BRANCH_RE. - POST /api/chat/slots/{slot}/followup re-validates the same schema (it is reachable over loopback from inside the kiro-cli process group, so it is a trust boundary, not a relay), redacts credentials and exfiltration URLs, and broadcasts a `followup_card` WS event. - POST /api/worktree/create builds a sibling worktree via sandboxed_spawn_argv + resource_limit_preexec with an argv list and no shell, resolves the repo to its git toplevel, refuses sensitive paths, and derives the destination server-side. - The tool is dashboard-only via _resolve_session_key_strict(); Slack, cron and subagent contexts fail closed. Frontend: - FollowUpCard renders in the same above-composer band as the question card, gated on slot ownership, with a single-flight guard and inline error on worktree failure. - Skip drops one suggestion and keeps its siblings; the card clears when its last item is gone. Tests: 49 backend (schema, endpoint, worktree endpoint against a real throwaway repo) and 11 frontend (component behaviour plus the four new reducers). Co-authored-by: Kyle Seaman <kseam@dev-dsk-kseam-1b-55230d27.us-east-1.amazon.com>
…levant (kirodotdev#597) PR kirodotdev#461 shipped the suggest_followup MCP tool and its follow-up card, but nothing prompted the agent to reach for it — usage rode entirely on the tool's own description, and with MCP Tool Search enabled that spec is not in every turn's context, so the tool would rarely be surfaced. Inject a situational, dashboard-only reminder into the per-turn interactive block (the same place the [OPTIONS:] reminder lives). It is gated to dashboard: / dashboard_ session keys because the tool rejects Slack, cron, and subagent contexts, and it is framed as optional and turn-end — not per-turn — so it raises awareness without becoming noise. Co-authored-by: Kyle Seaman <kseam@dev-dsk-kseam-1b-55230d27.us-east-1.amazon.com>
Open PR relationship auditThis is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion. Relationship findings
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
Problem
When the agent finishes a turn it often knows exactly what should happen next — the sibling endpoint that still needs the same fix, the doc that is now stale, the test gap it noticed on the way through. Today that knowledge has nowhere to go but prose. The user reads it, and then has to retype it as a new prompt, from memory, usually losing the specifics the agent had in hand. Worse, work that should not share the current working tree gets started in it anyway, because opening a worktree and a fresh session is several manual steps.
Why it matters
The expensive part of a follow-up is not deciding to do it — it is reconstructing the context. Every hand-retyped follow-up is a lossy copy of an instruction the agent could have written precisely. And every follow-up done in the current tree instead of a fresh one is a branch-hygiene problem that surfaces later as an unrelated diff in someone's PR.
Fix (symptoms → root cause → change)
Symptom: the agent's proposed next steps are unactionable prose; acting on them means retyping and losing detail.
Root cause: there is no agent-callable surface that can put a structured, actionable suggestion in front of the user. The dashboard already has an above-composer card band (the question card lives there), but nothing agent-reachable writes to it. The existing
QuestionCardpipeline cannot be reused: it waits on a nativeAskUserQuestiontool that does not exist in the kiro-cli binary, so it is dead code from the agent's side.Change: a new
suggest_followupMCP tool inkirocrew-core— agent-callable today, no dependency on unshipped CLI surface — that takes up to three{title, description, prompt, branch?}items and renders them as a card in that band.promptis the substance: the tool description instructs the model to write a complete, standalone handoff instruction on the assumption the next agent shares none of this session's context.title/descriptionare only the human-facing label.Three actions per item:
<parent>/<repo>-wt-<slug>on a new branch off the repo's default branch, opens a session scoped to it, and pre-fills that session's composer. Disabled when the session has no project directory.Both non-skip actions pre-fill and stop. Nothing is sent until the user presses send, so one click can never start an unattended turn. That constraint is enforced in the UI and promised to the model in the tool description, so it cannot quietly drift.
Worktree creation happens before the session is opened, so a git refusal (branch exists, not a repo) never leaves an orphan empty session behind — the error renders inline on the offending row and the button stays usable for a retry.
Trust model
Every string in an item is LLM-authored and one of them (
branch) reaches agitinvocation, so there are two independent gates:SUGGEST_FOLLOWUP_SCHEMAcaps item count and per-field lengths, rejects unknown fields, strips hidden Unicode, and full-matchesbranchagainstFOLLOWUP_BRANCH_RE. That grammar excludes a leading-(git would read it as a flag),..,~,^,:,?,*,[,\, and whitespace.POST /api/chat/slots/{slot}/followupre-validates against the same schema and redacts credentials + exfiltration URLs before broadcasting. This endpoint is reachable over loopback from inside the kiro-cli process group, so it is a trust boundary in its own right, not a relay for the MCP layer.POST /api/worktree/createadds its own:repomust resolve inside a directory an existing chat slot is already scoped to. The card only ever sends the active session's ownproject, so this costs nothing in practice while removing the endpoint's arbitrary-path surface — without it any authenticated dashboard caller could name any directory on the host. Both the submitted path and the git toplevel it resolves to are checked, so resolving upward out of an allowed subdirectory is refused. The prefix test is boundary-aware, so/repo-evildoes not pass as being inside/repo.BENIGN_SPAWNSwith that justification — see "Sandbox classification" below.Both endpoints emit SEL audit records.
Sandbox classification
The first revision routed git through
sandboxed_spawn_argv. CI caught why that was wrong: the chokepoint raises rather than degrading when no OS sandbox backend is available, so every worktree create returned 500 on Windows and would do the same on macOS hosts withoutsandbox-exec. That is a product bug, not a test artifact.git here is in the same class as the repo's existing local-VCS spawns (
cloud/source.py::_git_tracked_files,pod/runtime.py::_git_worktrees): a fixed binary, a server-derived destination, acwdalready constrained to an allowed slot project, and one agent-reachable input — the branch name — that has full-matchedFOLLOWUP_BRANCH_REat two independent layers. Failing a local VCS call closed on those platforms buys no isolation the user does not already have. So it is allowlisted with that reasoning, and the protections that do apply are kept at the call site.One note for future readers, documented in the code: the audit classifies a spawn by text-matching the enclosing function's source, so the rationale deliberately lives in the module docstring — naming the chokepoint helper inside the function would make an unrouted spawn read as routed.
The tool is dashboard-only via
_resolve_session_key_strict()— Slack, cron and subagent contexts fail closed rather than posting a card into another session.Scope notes
Tests
Backend — 49 new
test/test_followup_suggest.py(29): schema accepts a minimal item and an optional branch; rejects empty/oversized item lists, missingprompt, non-object items, unknown item and top-level fields, oversized titles, whitespace-only titles; accepts a prompt exactly at the limit; treats an empty branch as absent; strips a zero-width space from a title; parametrised over 10 dangerous branch names (leading--,..traversal, whitespace, shell metacharacters,@{0},~1,^,:,//, leading/). Endpoint: broadcasts the card, 404 on unknown slot, 400 on invalid JSON and on a JSON-array body, re-validates the schema independently of the MCP layer, and redacts a credential before broadcast.test/test_worktree_create.py(20): runs against a real throwaway git repo. Rejects 7 unsafe branch shapes, non-string inputs, missing directories, non-git directories, invalid JSON; creates a genuine sibling worktree and asserts it is a sibling (not nested) with the base commit's files present; proves a nested path resolves to the repo toplevel; 409 on an existing branch and on an existing destination directory; covers dir-slug derivation and theorigin/HEAD→HEADbase-ref fallback.Frontend — 11 new (
website/src/test/FollowUpCard.test.tsx): renders title/description/three actions; asserts the "nothing is sent" copy is present;onAddToSessionreceives the item;onSkipreceives the item index so siblings survive; the worktree button is disabled without a project dir while the in-session route stays enabled; a rejected worktree renders inline viarole="alert"and leaves the button retryable; a second click cannot fire while one is in flight. Plus the four reducers: set/clear, dismiss-one-keeps-rest, dismiss-last-clears, and a new card replacing an unacted-on one rather than stacking.Manual verification
Screenshots below were captured by driving the real built SPA (
website/dist) under Playwright with the network stubbed from fixtures — no gateway, no token, and no worktrees actually created. The card is driven the way the backend drives it: afollowup_cardframe pushed into the live websocket after the page has rendered, so the WS handler, the slice, the slot-ownership gate, and the composer prefill are all exercised as shipped. Harness committed atwebsite/scripts/capture-followup-card.mjsfor re-runs.Not exercised end-to-end: a real
git worktree addthrough the browser path (the endpoint's own suite covers that against a real repo, but the click-to-new-session hop is fixture-backed).Screenshots
Three suggestions stacked, most valuable first — dark:
"Add to this session" pre-filled the composer and closed the card — note the prompt is sitting in the input awaiting send, not sent:
A failed worktree create reports inline on its own row and leaves the button retryable:
Full-page context and light-theme parity
Single suggestion, dark:
Three suggestions, full page dark:
Light theme:
Local gates
pytest17208 passed ·isortclean ·flake8clean ·mypy474 files ·tsc -bclean ·vitest4508 passed / 389 files ·eslint0 errors ·jscpd0 clones · theme-colors advisory (no new literals).Remaining local failures are pre-existing or environmental, not from this change:
test_dashboard_origin::TestParseDashboardUrlMalformed(×3) andtest_skills::test_flat_copy_untouched_when_nested_missingfail identically on a cleanorigin/maincheckout on this host; a rotatingtest_apps_registrycase andtest_mcp_gateway_wedge_ping_gate::test_unknown_notification_silently_ignoredpass in isolation and are load-sensitive under xdist.Docs:
src/kiro_crew/docs/followup-suggestions.md, linked from the docs index.