fix(grok): skills, reasoning, plan mode, usage, and stalled turns - #7638
fix(grok): skills, reasoning, plan mode, usage, and stalled turns#76381xpixi wants to merge 24 commits into
Conversation
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.
Grok 4.6 often omits ACP allow_always. T3 still shows Always allow this session, then mapped a missing option to cancelled. Fall back to allow_once and remember the choice for the rest of the session. Fixes pingdotgg#6502
T3 only shows approval if Grok sends session/request_permission. A local [ui] permission_mode = always-approve made Supervised a no-op. Start Supervised sessions with grok --permission-mode default so the thread mode wins. Full access still passes --always-approve.
Grok's ACP CLI resends the ENTIRE accumulated terminal output on every tool_call_update notification (~10/sec, 145 KB+ each) instead of a delta, so a single redrawing terminal flooded runtime event ingestion and head-of-line-blocked every other thread. Bound tool call output to an 8 KB tail in the shared ACP parser (tool call state, rawOutput and the raw payload alike, leaving non-text content entries such as diffs untouched) and coalesce cumulative resends that did not grow meaningfully, while terminal (completed/failed) updates always emit immediately so the final tool call state stays correct.
|
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:
Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces multiple user-visible Grok capabilities and changes turn settlement, permission enforcement, shared ACP event parsing, and usage-contract behavior across server and clients. The breadth and runtime blast radius require human review despite focused tests and backward-compatible handling for older usage summaries. You can add or adjust custom eligibility rules. Learn more. |
Display-text extraction trims before joining, so oversized whitespace-only (or whitespace-padded) content entries skipped the 8 KB tail cap and were persisted/emitted unbounded on toolCall.data.content and rawPayload. Bound or normalize each text entry before those early returns, including the empty-chunks path.
|
Pushed
|
Keep live command output flowing by coalescing content/rawOutput growth even when detail stays the command, and emit the first tool_call even when it has no detail. Distribute oversized text tails across the original entries so content around images/diffs keeps its order. Accept empty plan.md writes, require ~/.grok/sessions paths to live under the user home directory, and only set planModeActive after a successful enter_plan_mode (clear it on exit).
|
Pushed
Live command output never reaching the UI ( Retained tail reordered around non-text entries ( Empty
Workspace Tests: |
Signal the turn liveness watchdog after decrementing livenessUpdatesInFlight so a pause-consumed wake cannot leave it blocked forever. Emit pending-to-inProgress tool updates even when detail and output are unchanged, and index retained text tails by original entry instead of scanning.
|
Pushed Liveness watchdog deadlock (
O(n²) tail mapping (
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
isGrokPlanMarkdownPath now matches spawn HOME, GROK_HOME, the ACP mock home, and canonical /home|/Users layouts, while still rejecting repo-local .grok/sessions paths. Empty rawInput.content no longer hides a following plan.md diff: prefer non-empty markdown, and only cache empty when the write is actually a clear.
|
Pushed Plan path matcher ignored non-home sessions Empty rawInput hid diff markdown
|
The canonical-home fallback ran against a lowercased haystack on Windows, so /Users/ never matched. Make that regex case-insensitive.
|
Pushed |
isGrokPlanMarkdownPath treated /home/other/.grok/sessions/../../project/plan.md as a session plan because the canonical fallback used unrestricted .+ . Reject .. segments before matching and require real path segments under sessions/.
|
Pushed |
Scan ~/.grok/sessions/**/updates.jsonl so Grok turns show up next to Claude and Codex. Contract v5 still merges v4 summaries.
1xpixi
changed the title
|
The Usage page only scanned Claude Code and Codex transcripts, so Grok work never showed up even when Source. Parser. Contract. UI. Web and mobile Usage charts/tables get a Grok Build series. Day-table empty-state This is the Grok slice of #5706. It avoids the scope and parser bugs in #5704, #5830, #6051, and #6057 (wrong tick scale, Verified with |
There was a problem hiding this comment.
One convention issue found: new host-runtime reads in apps/server/src/provider/acp/XAiAcpExtension.ts bypass the injected HostProcess* references that this repo requires in Effect code.
Posted via Macroscope — Effect Service Conventions
isGrokPlanMarkdownPath and extractGrokPlanMarkdownFromToolCallData were reading process.platform and process.env directly, the only unsuppressed t3code/no-global-process-runtime hits in the tree. Take platform and environment as arguments; GrokAdapter resolves them from HostProcessPlatform and HostProcessEnvironment.
|
Pushed Host runtime reads in plan path matching (
|
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.
|
Note 🤖 GPT-5.6 Sol responding on behalf of Theo Closing this PR after an automated pass over open pull requests. Combines skills, reasoning, plan mode, usage, and stalled-turn changes in one broad provider rewrite. |
|
reopening because this feels like a good faith attempt to make the Grok integration more reliable. I'll give this a look later. Not sure when just yet. If you can keep it up to date, I'd appreciate that a lot. |
…ffort - docs(readme): remove Grok reliability fork banner from PR branch so upstream README no longer claims to be a fork of itself after merge (Cursor 585a2287) - fix(grok): do not treat omitted reasoningEffort as explicit clear in applyGrokAcpModelSelection (Cursor 8eb1e158) Session start and later turns send the selected model without a reasoning option until the user touches the control. Omitting the option now preserves the CLI-advertised default (e.g. Extra High) instead of calling session/set_model with no _meta and clearing it. GrokAdapter keeps currentReasoningEffort when the turn/start input does not provide reasoningEffort, so the composer snapshot stays consistent with the thread's actual effort.
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 afd24b4. Configure here.
| if (trimmed.length === 0) { | ||
| ctx.lastKnownProposedPlanMarkdown = ""; | ||
| ctx.lastKnownProposedPlanTurnId = turnId; | ||
| return; |
There was a problem hiding this comment.
Empty plan writes wipe fallback
Medium Severity
An empty plan.md tool update still calls emitProposedPlanCompleted, which stores "" on lastKnownProposedPlanMarkdown and skips emitting. extractGrokPlanMarkdownFromToolCallData returns "" for a recognized plan write with empty body, so a later status-only or blank rewrite erases a previously captured plan. extractXAiExitPlanMarkdown then falls back to that empty string and emits the empty-state card, replacing the real proposal when exit_plan_mode omits planContent.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit afd24b4. Configure here.
7 participants
What Changed
Grok in T3 Code is missing several things the CLI already does: skills, advertised reasoning levels, plan-mode approval, Supervised prompts, usage-limit errors, and turn settlement when the ACP stream dies. The Usage page also ignored Grok even when
~/.grok/sessionsalready had per-turn token and cost data. One busy Grok tool call can also flood the shared ACP parser and stall every other thread.This PR stays on the Grok adapter boundary, plus Usage scanning of Grok
updates.jsonl. It does not add a new provider, a new picker, or a client rewrite. The usage contract goes to v5 and still merges v4 summaries. Web, desktop, and mobile already render skills, traits, proposed plans, approvals, and failed turns. They were empty or stuck because the Grok snapshot and ACP adapter never filled those existing fields.$picker always said "No skills found"grok inspect --jsonafter a successful version probe and attachskillsto the provider snapshot. Discovery is best-effort: timeout, old CLI, or bad JSON yields[]and never degrades the probe._meta.reasoningEffort/reasoningEffortsonto the existingreasoningEffortselect. Collapse duplicate Default badges. Send{ reasoningEffort }onsession/set_model, including same-model changes and explicit clear.plan.md, then hung on_x.ai/exit_plan_modewith no cardx.ai/exit_plan_mode/_x.ai/exit_plan_mode, emitturn.proposed.completed, replyabandonedso the native gate unblocks without auto-implementing (same shape as Claude ExitPlanMode). Promote in-progressplan.mdwrites while plan mode is active.~/.grokalways-approve skipped the bar. Always allow cancelled the turn--permission-mode defaultand Full access with--always-approve. If Grok omitsallow_always, map Always allow toallow_onceand remember it for the rest of the session.rate_limit/errorprompt completions as failed ACP requests, emit one failed turn, restore ready, keep the selected model.tool_call_update(~10/s)completed/failedalways emit. Cursor shares this parser; payloads under the cap stay byte-identical.turn_completedusage from$GROK_HOME/sessions/**/updates.jsonl(default~/.grok). ConvertcostUsdTicksat 10^10 ticks per USD, emit per-model records, pro-rate leftover aggregate cost, and skip large chat/events logs. Web and mobile show Grok Build. Contract v5 still merges v4 summaries so mixed-version environments keep Claude/Codex totals.Follow-up on the tool-output cap:
extractTextContentFromToolCallContentused to return oversized whitespace-only entries unchanged (chunks.length === 0and joined-under-cap early returns), sotoolCall.data.contentandrawPayloadcould still persist unbounded terminal buffers. Each text entry is now bounded or normalized before those returns.No new client UI. The
$picker, Reasoning traits control, proposed-plan card, approval bar, and Usage page are the ones already in the app. Usage gets a Grok Build series, label, and icon in the existing chart and tables.Why
T3 already has provider-agnostic surfaces for these. Claude and Codex fill them. Grok did not, so Grok threads looked empty, hung, or silently finished. Asking the CLI (
inspect, ACP model_meta, xAI extension methods) is smaller than reimplementing Grok's skill scan, effort catalog, or plan TUI.This is one Grok-gap concern, not a product expansion. Earlier attempts for skills (#5960 disk scan, #5503 slash-as-skill, #5131 shared ACP
availableCommands) are superseded here bygrok inspect --json, matching #7587.Usage reporting had its own pile of overlapping PRs (#5704 Grok+OpenCode+filters, #5830 Grok-only, #6051 Grok-only, #6057 Cursor+Grok+OpenCode). Those were too wide, used the wrong log (
unified.jsonl), converted ticks off by 10x, invented colliding dedupe keys, or dropped provider cost on mixed-model turns. This keeps Grok-only, scansupdates.jsonlonly, uses 10^10 ticks/USD, readsGROK_HOMEfromHostProcessEnvironmentwith empty-value fallback, and does not synthesize a dedupe key withoutprompt_id.House style prefers one commit-sized concern per PR. These patches are the Grok adapter slice that has to land together to make a Grok thread usable. Most of the line count is tests and the ACP mock. I can split if that is the review shape you want.
UI Changes
No new components. Existing surfaces start receiving Grok data. The Usage page now also includes Grok Build.
$on a Grok thread showed "No skills found". Grok 4.6 had no Reasoning control. Plan mode wrote a file and then froze with no plan card. Supervised never asked if the local CLI was always-approve. Always allow stopped the turn. A usage-limit turn completed empty. A dead ACP stream stayed on Working. Usage only listed Codex and Claude Code.$lists the CLI's skill catalog. Reasoning shows Extra High / High / Medium / Low when ACP advertises them. Plan mode updates the proposed-plan card and the turn can finish. Supervised asks; Always allow continues the session. Usage limits fail the turn with a visible message. Silent turns fail instead of hanging. Usage reports Grok Build from persisted session updates.Live verified on Windows with a packaged desktop build of this branch (T3 Code Alpha 0.0.33, unsigned local NSIS) against a real Grok CLI.
After — Grok Build on the Usage page
After — composer with advertised reasoning and full access
There is no new motion or timing to record.
Verification
Focused, no spawn:
vp test run apps/server/src/provider/acp/GrokAcpSupport.test.ts— 15 passed (effort tokens,session/set_modelmetadata, spawn args, Supervised argv)vp test run apps/server/src/provider/Layers/GrokProvider.test.ts— capability mapping 8/8 passed; snapshot disabled/pending/missing-binary passedvp test run apps/server/src/provider/Drivers/GrokSkills.test.ts— 5 passedvp test run apps/server/src/provider/acp/XAiAcpExtension.test.ts— 14 passedvp test run apps/server/src/provider/acp/AcpRuntimeModel.test.ts— 21 passed (8 KB tail, interleaved non-text entries, whitespace-only / padded early-return paths)Live Grok adapter tests that write
#!/bin/shwrappers fail on this Windows host withspawn EFTYPE. That is a pre-existing mock shape, not a regression in these patches. Linux CI owns those.Packaged Windows NSIS from this branch built and ran: skills picker, reasoning control, plan card, Supervised / Always allow, turn settlement, and Grok Build on the Usage page all worked against a real Grok CLI (
$16.20, 39 sessions from~/.grok).Usage scan (57 passed):
vp test run apps/server/src/usage/usageTranscripts.test.ts apps/server/src/usage/usageScanCache.test.ts packages/shared/src/usageMerge.test.ts apps/web/src/components/usage/UsageProviderChart.test.tsNot run: full monorepo suite (CI owns it).
Skip
Vercel – t3code-marketing: fork PRs cannot authorize Ping Labs marketing deploys. This branch does not touchapps/marketing.Checklist
Related
Fixes #4109 (skills catalog; does not add
/slash commands).Fixes #5706 (Grok on the Usage page; OpenCode left to a later PR).
Fixes #7210.
Fixes #6556.
Fixes #6502.
Sources, with original authorship preserved on the commits:
inspectModel: Grok 4.6. Harness: Grok Build TUI.
Note
Medium Risk
Changes Grok turn settlement, permission spawning, and shared ACP tool-update parsing (used beyond Grok), plus usage contract/provider scanning—incorrect timeouts or coalescing could fail good turns or hide live tool output.
Overview
Grok adapter and provider snapshot now align T3 with what the Grok CLI already exposes: skills from
grok inspect --json(best-effort, servercwd), reasoning effort from ACP model_metaapplied viasession/set_model, and spawn argv mapped from T3RuntimeMode(e.g. Supervised →--permission-mode default, full access →--always-approve).Plan mode and approvals add handling for
enter_plan_mode, sessionplan.mdwrites, and xAIexit_plan_modereverse-requests asturn.proposed.completedwith an abandoned response so turns unblock. Permission UI maps Always allow toallow_oncewhen Grok omitsallow_always, and remembers session-wide auto-approve afteracceptForSession. xAIrate_limit/errorprompt completions surface as failed turns with a usage-limit message while keeping the selected model.Turn liveness adds a watchdog that only starts counting after observable ACP progress, fails silent streams after configurable inactivity (longer while tools run), and pauses during approvals, user input, and in-flight updates; steering and plan updates refresh the timer.
Shared ACP parsing caps and coalesces oversized or repetitive
tool_call_updatetext (including whitespace-only paths) and improves in-progress emission for command tools. Usage scans Grokupdates.jsonlunder$GROK_HOME/~/.grokand mobile adds Grok Build to the usage chart series. Tests andacp-mock-agentenv flags expand coverage for these flows.Reviewed by Cursor Bugbot for commit afd24b4. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add Grok skills, reasoning, plan mode, usage tracking, and stall detection
grok inspect --jsonand injects them into provider snapshots; probe runs with the server's configured cwd_meta, and applyGrokAcpModelSelection sends effort viasession/set_modelmetadata without clearing defaults on same-model reselectionenter_plan_mode, surfacesplan.mdwrites and xAIexit_plan_modereverse-requests asturn.proposed.completedevents, and unblocks the turnDEFAULT_GROK_TURN_INACTIVITY_TIMEOUT_MS, active tool:DEFAULT_GROK_ACTIVE_TOOL_INACTIVITY_TIMEOUT_MS) that fails and cancels stalled turns after ACP progress begins; pauses during approvals and in-flight updatesupdates.jsonlvia parseGrokLine with per-model splitting and cost-tick-to-USD conversion; mapsRuntimeModeto Grok CLI permission flags; improves tool-call content bounding and emission coalescing in AcpRuntimeModel.tsUSAGE_CONTRACT_VERSIONfrom 4 to 5 in usage.ts; clients on version <4 (USAGE_MERGE_COMPATIBLE_SINCE) are now excluded from merges and marked stale.grokAcpSpawnArgsin GrokAcpSupport.ts changes Grok CLI argv based onRuntimeMode, soapproval-requirednow passes--permission-mode defaultto override config-level--always-approveMacroscope summarized afd24b4.