feat: implement ExitPlanMode HITL for Tool Permission Model

[quay-devel]

Summary

Implements the Tool Permission Model spec from PR #1586, adding ExitPlanMode as a HITL (human-in-the-loop) tool that halts the event stream and waits for user approval — the same mechanism already used by AskUserQuestion.

• Runner: ExitPlanMode added to BUILTIN_FRONTEND_TOOLS halt set; plan file content injected into tool args; Tier 1 allowlist completed with all missing tools
• Backend: isAskUserQuestionToolCall → isHITLToolCall to detect both tools for status derivation and snapshot compaction; new test cases for ExitPlanMode
• Frontend: HITL detection generalized via shared hitl-tools.ts; new ExitPlanModeMessage component with approve/reject/request-changes actions

Closes #1583
Spec: #1586

Test plan

• Backend Go tests pass (go test ./websocket/...)
• Frontend build passes with 0 errors, 0 warnings (npm run build)
• Frontend tests pass (668 passed, 12 skipped via npx vitest run)
• Runner adapter syntax verified
• No panic() in Go code, no any types in TypeScript
• Manual: verify ExitPlanMode halts stream and shows plan approval UI
• Manual: verify approve/reject/request-changes sends correct tool result

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Plan Review UI: approve, reject, or request changes to plans with optional feedback; Exit Plan action supported.

• Improvements

  • Broader human-in-the-loop detection so multiple review tool types are recognized.
  • More reliable waiting-for-input detection so prompts appear when agent input is needed.
  • Plan content is automatically surfaced into plan-review prompts.

• Other

  • Expanded agent tool capabilities (notebook, web, todo, task/plan lifecycle, scheduling).

[netlify]

✅ Deploy Preview for cheerful-kitten-f556a0 canceled.

Name	Link
🔨 Latest commit	59faa1e
🔍 Latest deploy log	https://app.netlify.com/projects/cheerful-kitten-f556a0/deploys/6a1602723814b10007f622bf

[coderabbitai]

📝 Walkthrough

Walkthrough

Generalizes HITL tool detection (AskUserQuestion + ExitPlanMode), preserves HITL tool-start snapshots for status inference, centralizes frontend helpers, adds ExitPlanMode UI and plan-content enrichment in the runner, and updates tests and runner allowlist.

Changes

HITL Tool Support

Layer / File(s)	Summary
Backend HITL detection components/backend/websocket/agui_proxy.go, components/backend/websocket/agui_store.go	Introduce isHITLToolCall (normalizes names) and switch DeriveAgentStatus / compactFinishedRun to preserve/infer waiting_input for both AskUserQuestion and ExitPlanMode.
Backend tests components/backend/websocket/agui_store_test.go	Add subtest for ExitPlanMode and generalize case-insensitive tool detection to HITL tool names.
Frontend: HITL helpers components/frontend/src/lib/hitl-tools.ts	Add normalizeToolName, isAskUserQuestionTool, isExitPlanModeTool, isHITLTool, and hasToolResult.
Frontend: agent-status wiring components/frontend/src/hooks/use-agent-status.ts	Use isHITLTool when scanning messages to determine waiting_input.
Frontend: AskUserQuestion component components/frontend/src/components/session/ask-user-question.tsx	Use shared hasToolResult to determine if a question was already answered.
Frontend: stream rendering & ExitPlanMode UI components/frontend/src/components/ui/stream-message.tsx, components/frontend/src/components/session/exit-plan-mode.tsx	Stream renders ExitPlanModeMessage for ExitPlanMode tool calls. New ExitPlanModeMessage component displays plan content, optional permissions, and handles approve/reject/request-changes with optional feedback submission.
Runner: adapter enrichment components/runners/ambient-runner/ag_ui_claude_sdk/adapter.py	Add ExitPlanMode to builtin frontend tools; implement _read_plan_file() to read newest .claude/plans/*.md (UTF-8, truncated to 100KB) and inject planContent into tool call args.
Runner: allowed tools components/runners/ambient-runner/ambient_runner/bridges/claude/mcp.py	Expand and reorder DEFAULT_ALLOWED_TOOLS to include ExitPlanMode and additional tool names.

🚥 Pre-merge checks | ✅ 7 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	mcp.py's DEFAULT_ALLOWED_TOOLS expansion adds 12 unrelated tools (CronCreate, WebFetch, EnterWorktree, etc.) beyond ExitPlanMode scope, introducing risk without documented justification.	Isolate ExitPlanMode allowlist addition; gate unrelated tool additions (CronCreate, WebFetch, etc.) with feature flags or document security/persistence risk rationale separately.

✅ Passed checks (7 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title follows Conventional Commits format (feat: ...) and accurately describes the main change: implementing ExitPlanMode as a HITL tool.
Linked Issues check	✅ Passed	Changes implement core requirements from #1583: ExitPlanMode added to BUILTIN_FRONTEND_TOOLS, plan content injection implemented, backend HITL detection generalized, and frontend ExitPlanModeMessage component with approval/rejection actions added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Performance And Algorithmic Complexity	✅ Passed	No performance regressions. Backend message scanning breaks early; no N+1 patterns, unbounded caches, blocking requests, or O(n²) algorithms detected.
Security And Secret Handling	✅ Passed	No hardcoded secrets, injection vulnerabilities, unprotected endpoints, service account misuse, or sensitive data leakage detected. Path traversal is prevented.
Kubernetes Resource Safety	✅ Passed	Check is not applicable—PR modifies application code (Go, TypeScript, Python) only, with no Kubernetes manifests or container resource configurations altered.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

✨ Simplify code

• Create PR with simplified code

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@components/frontend/src/components/session/exit-plan-mode.tsx`:
- Around line 35-36: The variables planContent and allowedPrompts currently use
TypeScript assertions only; add runtime guards so planContent is set to
input.planContent if typeof input.planContent === "string" otherwise "" and set
allowedPrompts to Array.isArray(input.allowedPrompts) ? input.allowedPrompts as
AllowedPrompt[] : [] (or validate each element) before using ReactMarkdown and
.map; update the initialization of planContent and allowedPrompts in
exit-plan-mode.tsx to perform these checks so ReactMarkdown always gets a string
and .map runs on a real array.

In `@components/runners/ambient-runner/ag_ui_claude_sdk/adapter.py`:
- Around line 105-108: The truncation checks byte length but slices by character
count, which can exceed _PLAN_FILE_MAX_BYTES for multi-byte UTF-8 chars; fix by
performing the truncation in bytes: read the file text into content, encode to
bytes (e.g., content_bytes = content.encode("utf-8")), if len(content_bytes) >
_PLAN_FILE_MAX_BYTES then slice the bytes to _PLAN_FILE_MAX_BYTES, decode back
to a string with a safe error handler (e.g., errors="ignore" or "replace") and
append the "\n\n[truncated]" marker before returning; update the logic around
plan_files, content, and _PLAN_FILE_MAX_BYTES accordingly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 767b3627-8743-45b6-a69e-396e31eb5da9

📥 Commits

Reviewing files that changed from the base of the PR and between 63545c7 and 45eb7fc.

📒 Files selected for processing (10)

• components/backend/websocket/agui_proxy.go
• components/backend/websocket/agui_store.go
• components/backend/websocket/agui_store_test.go
• components/frontend/src/components/session/ask-user-question.tsx
• components/frontend/src/components/session/exit-plan-mode.tsx
• components/frontend/src/components/ui/stream-message.tsx
• components/frontend/src/hooks/use-agent-status.ts
• components/frontend/src/lib/hitl-tools.ts
• components/runners/ambient-runner/ag_ui_claude_sdk/adapter.py
• components/runners/ambient-runner/ambient_runner/bridges/claude/mcp.py

[markturansky]

Claude Code Review

Summary

Clean, well-scoped implementation that correctly generalizes AskUserQuestion's HITL machinery to cover ExitPlanMode. The DRY refactor — extracting hitl-tools.ts to replace three independent copies of the same normalization logic — is the highlight. Backend Go tests adequately cover the new ExitPlanMode status detection. No blockers or criticals.

Findings

Blocker

None.

Critical

None.

Major

None.

Minor

1. Exact string match in adapter.py inconsistent with normalized approach everywhere else
components/runners/ambient-runner/ag_ui_claude_sdk/adapter.py — the enrichment guard:

if current_tool_display_name == "ExitPlanMode":

…uses an exact case-sensitive comparison while the backend (isHITLToolCall) and frontend (normalizeToolName) both strip non-alpha chars and lowercase before matching. If the SDK ever emits exitPlanMode or exit_plan_mode (unlikely, but it has happened with tool renames), the enrichment silently skips and the frontend renders a plan-review card with no plan content. The detection still fires because BUILTIN_FRONTEND_TOOLS uses exact match too, so the inconsistency is contained — but a helper that mirrors the normalization pattern would make this robust:

def _normalize_tool_name(name: str) -> str:
    return "".join(c for c in name.lower() if c.isalpha())

if _normalize_tool_name(current_tool_display_name) == "exitplanmode":

2. _read_plan_file is untested
The function is pure (no side effects beyond I/O) and has non-trivial behavior: mtime sort, 100 KB truncation at a byte boundary, UTF-8 decode with errors="ignore". The runner test suite covers other functions — adding a few table-driven tests for this one would prevent silent regressions from filesystem-layout assumptions. At minimum: empty plans dir, single file under limit, single file over limit, non-UTF-8 edge (though errors="ignore" handles it).

3. Removed comment in ask-user-question.tsx reduced signal
Line 35 previously had: // Handle simple { question: "..." } format (e.g. from Claude Code AskUserQuestion tool). This documented a non-obvious branch — the two-shape input protocol (AskUserQuestionInput[] vs bare { question: string }). The logic is preserved but the explanation is gone. Worth reinstating as a single-line comment since the shape switch is a subtlety a future reader would wonder about.

4. exit-plan-mode.tsx is exactly 200 lines (at the convention boundary)
Frontend conventions say "Components under 200 lines." The file counts 200 lines including the displayName footer. No refactor needed now, but note that any future additions push it over the stated limit.

Nit

• input.allowedPrompts as AllowedPrompt[] — the Array.isArray guard validates it's an array but doesn't validate element shapes. Fine for now given the source is Claude tool args, but a comment noting the trust boundary would help a reader.
• Buttons in the action row (Approve / Reject / Request Changes) don't show a spinner on isSubmitting. They disable correctly, so there's no functional bug, but user feedback on the submitting state is absent for Reject and Request Changes. Could be a follow-up.

Positive Highlights

• Excellent DRY refactoring. hitl-tools.ts consolidates tool-name normalization from three independent copies (stream-message, use-agent-status, ask-user-question) into one source of truth with five well-named exports. This is exactly the right move.
• Go test quality. The new RUN_FINISHED with same-run ExitPlanMode returns waiting_input test covers the most important status-derivation path, and the extended case-insensitive table now covers both tools symmetrically.
• Proper 100 KB plan file cap. _PLAN_FILE_MAX_BYTES is defined as a named constant and enforced before sending content over the wire — prevents runaway payload sizes without silently dropping the content.
• UI state machine is correct. disabled = alreadyAnswered || submitted || isSubmitting || !isNewest correctly covers every state where the user should not be able to interact. The finally block in handleDecision ensures isSubmitting always resets even on throw.
• Zero any types, no panic(), Shadcn UI used throughout, React Query not applicable (purely prop-driven component). Conventions are clean across the stack.

Recommendations

1. (Minor) Normalize current_tool_display_name before the ExitPlanMode enrichment guard — a one-line helper keeps it consistent with the rest of the system.
2. (Minor) Add unit tests for _read_plan_file covering the truncation and empty-directory cases.
3. (Nit) Reinstate the single-line comment on the bare { question } branch in ask-user-question.tsx.

---

Amber · standards loaded from CLAUDE.md, specs/standards/backend/, specs/standards/frontend/, specs/standards/security/

[markturansky]

Amber Code Review — PR#1589: feat: implement ExitPlanMode HITL for Tool Permission Model

Summary

Solid, well-structured implementation of ExitPlanMode as a second HITL tool. The abstraction into hitl-tools.ts is exactly the right move — it eliminates three duplicate detection functions and gives a clean single source of truth. The backend status-derivation and compaction logic correctly handles the ExitPlanMode lifecycle. One critical behavioral uncertainty (tool result format), one major finding (unguarded Tier-1 tool expansion), and four minor findings.

---

Findings

Blocker

None.

Critical

1. ExitPlanMode tool result format not verified against Claude Code CLI expectations
exit-plan-mode.tsx:63–68 — The component sends JSON.stringify({decision: "approve" | "reject" | "request_changes", feedback?: "..."}) as the tool result. Whether Claude Code CLI's ExitPlanMode actually expects this structured JSON — or expects natural-language text (e.g. "Yes, proceed") — is unspecified in the diff. If the format is wrong, Claude receives a malformed tool result and may behave unexpectedly after user approval.

The test plan has [ ] (unchecked) for "verify approve/reject/request-changes sends correct tool result." This is the exact gap. The result contract with Claude Code CLI must be established before merge.

Fix: Document the expected tool result schema (from the Claude Code SDK docs or spec PR#1586). Add a test that mocks onSubmitAnswer and asserts the correct JSON structure is produced for each button path (approve, reject, request_changes).

---

Major

2. Tier-1 allowlist expansion in mcp.py without feature flags
components/runners/ambient-runner/ambient_runner/bridges/claude/mcp.py:29–51 — Fourteen new tools added to DEFAULT_ALLOWED_TOOLS, several with meaningful blast radius:

• CronCreate — allows the runner to schedule persistent jobs that fire independent of session lifecycle.
• EnterWorktree / ExitWorktree — creates isolated git worktrees; side effects persist after session ends if cleanup fails.
• WebFetch — opens server-side SSRF risk if not already mitigated elsewhere.

Per CLAUDE.md: "Feature flags strongly recommended — gate new features behind Unleash flags." No flag here means a bad behavior from any of these tools has no kill switch.

If these tools were specified by the Tier-1 allowlist in PR#1586 and their addition is intentional/non-new, add a code comment pointing at the spec. Consider a single exitplanmode-hitl flag gating the expansion. At minimum, CronCreate and EnterWorktree/ExitWorktree warrant individual flags or a clear rationale for unflagged rollout.

---

Minor

3. border-l-3 is not a standard Tailwind CSS class
exit-plan-mode.tsx:88, 92 — Tailwind's built-in border-left width scale is 0, 1, 2, 4, 8 — no border-l-3. In standard Tailwind v3 JIT, this class compiles to nothing (the accent border won't render). If the existing AskUserQuestionMessage uses the same class and it works, there's a custom config in play — worth confirming. If not, replace with border-l-2 or border-l-4.

4. handleDecision silently drops submission errors
exit-plan-mode.tsx:55–68 — The try/finally block resets isSubmitting after an onSubmitAnswer failure, but no error is surfaced to the user. They can retry (since submitted stays false), but get no indication anything went wrong. Add an error state and display an inline message or toast on failure.

5. debug log level for ExitPlanMode plan injection failure
adapter.py:1092 — logger.debug("Failed to enrich ExitPlanMode with plan content: %s", e) — if this fires, the user sees the plan approval card with no plan content. That's confusing enough to warrant logger.warning(...) so it appears in production logs without requiring debug verbosity. Optionally add a logger.debug inside _read_plan_file when the plans directory doesn't exist, to aid onboarding debugging.

6. Removed non-obvious compatibility comment in ask-user-question.tsx
ask-user-question.tsx:34 — The deleted comment # Handle simple { question: "..." } format (e.g. from Claude Code AskUserQuestion tool) was the only explanation for why this second format path exists. A future maintainer won't know this is a backward-compat branch without it. Please restore.

---

Positive Highlights

• hitl-tools.ts consolidation is exactly right. Three files had duplicate detection functions; one shared library with named exports is clean and extensible.
• Backend compaction and status derivation is logically sound. compactFinishedRun correctly preserves only ToolCallStart (not ToolCallFinished) for HITL tools, and DeriveAgentStatus correctly scopes to the most recent run so compacted events from answered prompts don't produce false waiting_input statuses.
• _read_plan_file has sensible guardrails: 100KB cap, errors="ignore" on truncated UTF-8, OSError catch, graceful None return when no plans dir exists.
• isNewest guard on all HITL components correctly prevents users from re-submitting answers to stale prompts from prior runs.
• Test coverage for ExitPlanMode mirrors the existing AskUserQuestion cases exactly in agui_store_test.go — right pattern.
• Go naming consistency: isHITLToolCall in Go mirrors isHITLTool in TypeScript — good cross-layer coherence.

---

Recommendations (prioritized)

1. (Critical — before merge) Confirm tool result schema against Claude Code CLI's ExitPlanMode expectations and add an automated test for each decision path.
2. (Major) Add an Unleash feature flag gating at minimum CronCreate, EnterWorktree, ExitWorktree additions to DEFAULT_ALLOWED_TOOLS.
3. (Minor) Replace border-l-3 with border-l-4 (or confirm it resolves correctly in the project's Tailwind config).
4. (Minor) Add error state display to handleDecision for onSubmitAnswer failures.
5. (Minor) Raise plan injection log level to warning.
6. (Minor) Restore the {question: "..."} backward-compat comment in parseQuestions.

---

🤖 Amber — code review agent | ambient-code/platform

[markturansky]

Amber Review — PR #1589

Summary

Well-structured implementation. The hitl-tools.ts shared module is the right pattern — eliminates three copies of the same normalization function. Backend rename from isAskUserQuestionToolCall → isHITLToolCall is clean. Plan file enrichment in the adapter handles truncation and options formats correctly. Both CodeRabbit findings (runtime type guards, byte-correct truncation) are addressed in commit d7352bb.

Findings

Minor — border-l-3 is not a valid Tailwind CSS class

exit-plan-mode.tsx uses border-l-3 in two places, but Tailwind's border-left width scale only includes border-l (1px), border-l-2 (2px), border-l-4 (4px), border-l-8 (8px). The class is silently ignored — the colored left accent border that visually distinguishes the plan card won't render.

-"rounded-lg border-l-3 pl-3 pr-3 py-2.5",
+"rounded-lg border-l-4 pl-3 pr-3 py-2.5",

(Check what ask-user-question.tsx uses for consistency — if it uses border-l-2, match that.)

Score

7/8 checks clean. LGTM with nit — the visual defect is minor and doesn't affect function.

---

— Amber (code review agent)

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

components/runners/ambient-runner/ambient_runner/bridges/claude/mcp.py (1)

26-49: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

ExitPlanMode missing from DEFAULT_ALLOWED_TOOLS.

The allowlist includes EnterPlanMode (line 42) but not ExitPlanMode. The PR objectives state ExitPlanMode was missing from the runner allowlist, causing indefinite hangs (issue #1583). Backend (isHITLToolCall) and frontend (isExitPlanModeTool) both expect this tool to pass through.

🐛 Proposed fix

     "EnterPlanMode",
+    "ExitPlanMode",
     "EnterWorktree",

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/runners/ambient-runner/ambient_runner/bridges/claude/mcp.py`
around lines 26 - 49, DEFAULT_ALLOWED_TOOLS is missing "ExitPlanMode", which
causes HITL/ExitPlanMode tool calls to be blocked; update the
DEFAULT_ALLOWED_TOOLS list in ambient_runner/bridges/claude/mcp.py to include
"ExitPlanMode" alongside "EnterPlanMode" so that isHITLToolCall and
isExitPlanModeTool pass through correctly.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@components/runners/ambient-runner/ambient_runner/bridges/claude/mcp.py`:
- Around line 26-49: DEFAULT_ALLOWED_TOOLS is missing "ExitPlanMode", which
causes HITL/ExitPlanMode tool calls to be blocked; update the
DEFAULT_ALLOWED_TOOLS list in ambient_runner/bridges/claude/mcp.py to include
"ExitPlanMode" alongside "EnterPlanMode" so that isHITLToolCall and
isExitPlanModeTool pass through correctly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b108ded0-3d88-49a5-aef8-0c63016dd4c3

📥 Commits

Reviewing files that changed from the base of the PR and between bf81358 and 59faa1e.

📒 Files selected for processing (1)

• components/runners/ambient-runner/ambient_runner/bridges/claude/mcp.py