Uh oh!
There was an error while loading. Please reload this page.
fix(cli): stop steering known slash commands into a running turn - #3310
Conversation
Typing /recap or /resume while a turn is running injected the literal text into the model as a steering message instead of running the command; whether it executed at all depended on whether the turn was still alive when the enqueue landed. Mid-turn submissions of known slash commands now route explicitly: /goal and /recap answer locally (both are independent of the running turn), every other known command is refused with a clear message, and unknown slash-prefixed text (skill invocations, paths) still steers. Refs apache#3308 Generated-by: Maka
Astro-Han
left a comment
There was a problem hiding this comment.
APPROVE.
The routing is right and the tests pin it at the observable boundary: driver.steered stays empty for /recap, /resume and /model, and /skill:review still steers, so both directions are covered rather than just the fixed one.
I checked the three claims the new comment rests on, and all three hold at 5c4ee2284:
runAgentTurnsetsbusyandturnRunningtogether (pi-tui-runner.ts:1128-1130), sorunControl'sif (busy) returnat:633really would swallow/help,/context,/newand the rest without a word. Refusing loudly is strictly better than that silent no-op, and the rationale in the comment is accurate.recap.rungoes straight torunRecap('manual')(:2857-2861) and never entersrunControl, so answering/recapmid-turn is genuinely safe rather than merely untested./goal <action>already carries its ownif (busy)notice (:2713-2722), so exempting/goalfrom the refusal does not reintroduce a silent no-op on the mutating path. That was my first suspicion and the code refutes it.
biome format is clean on both changed files at this head.
One non-blocking P3 inline. Separately, and not a review finding: statusCheckRollup is empty on this head — the test check has not run at all — so this approval covers the diff, not CI.
AI disclosure: this review was assisted by Claude (Opus) for code search and cross-checking. Everything above I re-derived myself against the source at 5c4ee2284.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Follow-up to apache#3310: the routing call site matched command names ('goal' | 'recap') to decide which slash commands answer locally while a turn is running. Move that knowledge onto MakaSlashCommand as a required midTurn field ('local' | 'refuse' | 'intercepted') declared next to each handler, so the handler that has to be safe mid-turn carries the declaration, and a newly added command must state its answer — omitting midTurn is a compile error. 'intercepted' documents /exit, /swarm and /graph, which their dedicated checks claim ahead of generic routing. Behavior is unchanged; the mid-turn routing tests from apache#3310 pin it. Generated-by: Maka
…pec (#3379) Follow-up to #3310: the routing call site matched command names ('goal' | 'recap') to decide which slash commands answer locally while a turn is running. Move that knowledge onto MakaSlashCommand as a required midTurn field ('local' | 'refuse' | 'intercepted') declared next to each handler, so the handler that has to be safe mid-turn carries the declaration, and a newly added command must state its answer — omitting midTurn is a compile error. 'intercepted' documents /exit, /swarm and /graph, which their dedicated checks claim ahead of generic routing. Behavior is unchanged; the mid-turn routing tests from #3310 pin it. Generated-by: Maka
Summary
Typing
/recapor/resumein the TUI while a turn is running never ran the command:editor.onSubmitonly special-cased exit forms,/swarm,/graph, and/goal, so every other input — including known slash commands — went tosteerRunningTurn()and the model received the literal text (Steering: /resume) as a user message. Whether the command executed at all depended on whether the turn was still alive when the enqueue landed (thefallbackpath re-routes throughsubmitPrompt).Mid-turn submissions of known slash commands now route explicitly:
/goaland/recapanswer locally (both are independent of the running turn — recap uses the separatesession.recap.generatecall behind its own in-flight lock); every other known command is refused with a clear message ("Cannot run /X while a turn is running — interrupt it (Esc) or wait for it to finish.") instead of being steered or silently no-opping on therunControlbusy gate; unknown slash-prefixed text (skill invocations like/skill:<name>, paths) still steers unchanged.Fixes#3308
Verification
npm --workspace maka-agent test: 320 pass / 0 fail, including 4 new tests underslash commands during a running turn(/recapanswers locally,/resumeand/modelrefuse with a message, unknown slash text still steers).pi-tui-runner.tsand rebuilding makes the/recap,/resume, and/modeltests fail while the unknown-slash test still passes.npm run lint,npm run format:check,npm run typecheck: pass.AI use
Select exactly one:
Tool(s) and scope: Maka (AI agent) located the routing defect, implemented the fix and tests, and ran the verification; the commit carries the
Generated-by: Makatrailer. Human contributor of record reviewed the diagnosis and the routing design and decided to submit.Checklist
Does this PR entail a change in behavior?