fix(grok): surface plan mode approval in proposed plan cards - #5409
fix(grok): surface plan mode approval in proposed plan cards#5409ahmed-besic wants to merge 4 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
ApprovabilityVerdict: Skipped Macroscope did not run approvability analysis for this PR. Macroscope could not determine whether this PR modifies its approvability configuration, so the PR was not approved automatically. A PR that may change the rules that govern approval is never approved automatically. |
|
I know you have disabled plan mode, and its fine, i dont use it either, the issue is sometimes grok will enter plan mode by itself and the thread will die. |
|
I just ran into this issue. I didn't use Is something wrong with Plan mode? I personally really like it when I want the agent to think without making changes and get my agreement before going forward. |
Grok intercepts exit_plan_mode and reverse-RPCs the client for approval. Without a handler the turn hung and plan.md was invisible in T3. Capture plan.md writes and x.ai/exit_plan_mode into turn.proposed.completed, then abandon the native gate so the existing plan card can finish the flow.
Clear lastKnownProposedPlanMarkdown on new turns and settlement so an empty exit_plan_mode cannot re-surface a previous plan. Restrict plan.md detection to paths under .grok/sessions so workspace plan.md files are not promoted to proposed plans.
Scope proposed-plan dedupe to the current turn so re-proposing the same text later still emits a card. Use a fresh event stamp when promoting plan.md writes (avoid shared eventIds with tool lifecycle events). Only promote session plan.md while plan mode is active after enter_plan_mode.
d0959e3 to
7f0f88a
Compare
|
I use plan mode all the time. Sometimes Grok also enters plan mode on its own, and that currently makes the thread broken in T3 Code: the plan stays empty / never appears, and I have to leave T3 and resume/approve in the Grok CLI to continue. Please merge this — it is a real daily-driver issue, not an edge case. |
Port pingdotgg#5409 onto current main. Grok intercepts exit_plan_mode and reverse-RPCs the client for approval; without a handler the turn hung and plan.md was invisible. Capture plan.md writes and x.ai/exit_plan_mode into turn.proposed.completed, then abandon the native gate so the existing plan card can finish the flow. Made-with: Grok 4.6
Intent: Stop Grok 4.6 threads from hanging after plan mode. When Grok finishes planning it reverse-RPCs x.ai/exit_plan_mode; an unanswered client looks disconnected, the plan never appears, and plan mode stays stuck. Behavior: - Handle both x.ai/exit_plan_mode and _x.ai/exit_plan_mode, including wrapped payloads and null planContent. - Emit the existing proposed-plan card from planContent. If Grok races the plan-file write, fall back to the last session plan.md body captured this turn. - Reply abandoned with a capture message so Grok unblocks without implementing in the same turn. - Detect enter_plan_mode and, while plan mode is active, promote writes to ~/.grok/sessions/.../plan.md onto the same card so it updates before exit. - Ignore workspace files named plan.md. Dedupe identical markdown per turn; clear fallback state on turn settle and on a new non-steer turn. - Leave implement / request-changes to the existing proposed-plan follow-up, matching Claude ExitPlanMode capture. Design constraints: - Do not auto-approve even in full-access; Grok's own always-approve still requires plan review. - Do not add a second Grok-only approval prompt. Reuse turn.proposed.completed and the existing plan card. - Keep the dialect at the Grok adapter boundary. Do not change contracts or other providers. Integration: - Grok adapter, xAI ACP extension helpers, ACP mock, and internals provider docs. - Web, desktop, and mobile consume the existing proposed-plan event with no client changes. Verification: - vp test run apps/server/src/provider/acp/XAiAcpExtension.test.ts apps/server/src/provider/Layers/GrokAdapter.test.ts — 37 passed. Rebase notes: - Conflict hotspots are GrokAdapter startSession extension handlers, ToolCallUpdated, sendTurn settlement, and XAiAcpExtension.ts beside ask_user_question. - Upstream pingdotgg#4514 is still open; pingdotgg#5409 / pingdotgg#6431 / pingdotgg#7638 implement this capture-and-abandon shape plus live plan.md promotion. Drop this patch only when main handles both method spellings, shows the proposed-plan card during plan.md writes and on exit, falls back when planContent is empty, and unblocks without auto-implementing.
Port pingdotgg#5409 onto current main. Grok intercepts exit_plan_mode and reverse-RPCs the client for approval; without a handler the turn hung and plan.md was invisible. Capture plan.md writes and x.ai/exit_plan_mode into turn.proposed.completed, then abandon the native gate so the existing plan card can finish the flow. Made-with: Grok 4.6
|
Merged current main. Plan-mode tests and server typecheck are passing. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d6e12ef. Configure here.
| return newText.trim(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Stale plan from rawInput preference
Medium Severity
extractGrokPlanMarkdownFromToolCallData returns rawInput.content before checking diff newText. Merged ToolCallUpdated state keeps earlier rawInput when a later update only refreshes content, so mid-plan promotion can keep emitting the first draft. Turn-scoped dedupe then blocks the real plan, and an exit_plan_mode with empty planContent can fall back to that stale markdown.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d6e12ef. Configure here.
Port pingdotgg#5409 onto current main. Grok intercepts exit_plan_mode and reverse-RPCs the client for approval; without a handler the turn hung and plan.md was invisible. Capture plan.md writes and x.ai/exit_plan_mode into turn.proposed.completed, then abandon the native gate so the existing plan card can finish the flow. Made-with: Grok 4.6
Port pingdotgg#5409 onto current main. Grok intercepts exit_plan_mode and reverse-RPCs the client for approval; without a handler the turn hung and plan.md was invisible. Capture plan.md writes and x.ai/exit_plan_mode into turn.proposed.completed, then abandon the native gate so the existing plan card can finish the flow. Made-with: Grok 4.6
Port pingdotgg#5409 onto current main. Grok intercepts exit_plan_mode and reverse-RPCs the client for approval; without a handler the turn hung and plan.md was invisible. Capture plan.md writes and x.ai/exit_plan_mode into turn.proposed.completed, then abandon the native gate so the existing plan card can finish the flow. Made-with: Grok 4.6
What Changed
When Grok Build enters plan mode, it writes
plan.mdand then callsexit_plan_mode. That tool is intercepted and reverse-RPCs the client over ACP as_x.ai/exit_plan_mode/x.ai/exit_plan_modewithplanContent, expecting the TUI plan approval window (approve / request changes / abandon).T3 never handled that extension method, so the turn stalled with no UI, and the plan content never showed up as a proposed plan. Users had to open the same session in the Grok terminal and approve there to continue.
This:
x.ai/exit_plan_modeand_x.ai/exit_plan_modeturn.proposed.completedwith the plan markdown so the existing proposed-plan card appearsoutcome: "abandoned"plus a client-capture message so the native gate unblocks without auto-implementing (same idea as Claude's ExitPlanMode capture)plan.mdwrites (tool_call / tool_call_update targeting…/plan.md) into the same proposed-plan path so the card updates while plan mode is still activeWhy
Grok threads that entered plan mode looked dead: no output, no approval UI, and no way to continue without leaving T3. Claude already maps ExitPlanMode into proposed plans; Cursor maps
cursor/create_plan. Grok needed the same product path for its xAI extension method.Live probe against
grok agent stdio0.2.118 confirmed the wire shape:{ sessionId, toolCallId, planContent }and a successful unblock with{ outcome: "abandoned", feedback }.UI Changes
Uses the existing proposed plan card — no new UI components.
plan.mdis written, the plan card appears and updates. Onexit_plan_mode, the turn finishes and the card stays actionable (implement / follow-up) like Claude/Cursor plans.Validation
vp test run src/provider/acp/XAiAcpExtension.test.ts src/provider/Layers/GrokAdapter.test.ts— 35 passed_x.ai/exit_plan_modepayload + abandoned response unblocks the promptChecklist
Made by Grok Build via the Grok Build harness while working on T3 Code.
Note
Medium Risk
Touches Grok ACP session/turn lifecycle and a new reverse-RPC handler that auto-abandons Grok’s plan gate. Incorrect handling could stall turns or emit the wrong plan markdown.
Overview
Grok plan mode no longer stalls waiting on the native TUI approval window. The adapter now captures the plan into the existing proposed-plan card (same product path as Claude
ExitPlanMode).Handlers for
x.ai/exit_plan_mode/_x.ai/exit_plan_modeemitturn.proposed.completedand reply withoutcome: "abandoned"so the prompt unblocks without auto-implementing. Whileenter_plan_modeis active, writes to Grok’s session…/.grok/sessions/…/plan.mdalso update that card; workspaceplan.mdfiles are ignored.Plan markdown is deduped per turn and cleared when a turn ends or a new one starts, so a later empty
exit_plan_modecannot resurrect the previous plan. Mock ACP coverage plus unit tests cover both the exit RPC and mid-plan writes.Reviewed by Cursor Bugbot for commit d6e12ef. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Surface Grok plan mode approval as proposed plan cards in
GrokAdapterGrokSessionContext(fieldslastKnownProposedPlanMarkdown,lastKnownProposedPlanTurnId,planModeActive) so the adapter can detect and dedupe proposed-plan emissions per turn_x.ai/exit_plan_modeext requests in GrokAdapter.ts, extracts plan markdown via XAiAcpExtension.ts, emits aturn.proposed.completedevent, and responds with anabandonedoutcome so the turn does not hang~/.grok/sessions/.../plan.md(detected byisGrokPlanMarkdownPath) intoturn.proposed.completedevents with per-turn dedupeT3_ACP_EMIT_XAI_EXIT_PLAN_MODEandT3_ACP_EMIT_XAI_PLAN_MD_WRITEin acp-mock-agent.ts for testing both flowsclearProposedPlanFallbackresets plan state at turn boundaries and on turn completion;plan.mdwrites outside~/.grok/sessions/paths are intentionally excluded byisGrokPlanMarkdownPathMacroscope summarized d6e12ef.