Add Grok Build CLI integration - #4225
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR integrates Grok: registers the agent and task-manager detection, extends hook definitions/config resolution, adds Grok sanitizer policy, implements notification summarization and persisted status, generalizes PID inference and binding resolution, updates resume/session models, and adds tests, docs, and localization. ChangesGrok Build CLI Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@CLI/cmux.swift`:
- Line 19912: The user-facing error currently prints a provider-specific name
via def.displayName; update the print call that references def.configDir and
def.displayName to use cmux-centric, minimal copy (e.g. mention only the config
directory and "Install cmux first" or similar) so no upstream/provider names are
exposed—locate the print statement that uses def.configDir and def.displayName
and replace the message to reference only def.configDir and the product name
"cmux".
In `@CLI/CMUXCLI`+AgentHookDefinitions.swift:
- Around line 49-57: resolvedConfigDir() currently treats whitespace-only env
override values as valid because envValue isn't trimmed; trim envValue before
validating and using it so blank/whitespace-only values fall through to the HOME
fallback. Specifically, in the branch that reads configDirEnvOverride and
envValue, create a trimmedEnv = envValue.trimmingCharacters(in:
.whitespacesAndNewlines), check trimmedEnv.isEmpty (and use trimmedEnv for
expanding tildes and building the URL), and continue to keep the existing
trimming/check for configDirEnvOverrideSubpath when appending the subpath.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: dcb90da4-f7d3-469e-9926-670ae98d4a48
📒 Files selected for processing (19)
CLI/CMUXCLI+AgentHookDefinitions.swiftCLI/cmux.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchEnvironmentPolicy.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizer.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizerPrimaryPolicies.swiftResources/Localizable.xcstringsSources/RestorableAgentSession.swiftSources/RestorableAgentTypes.swiftSources/SessionAgentPresentation.swiftSources/SessionIndexModels.swiftSources/SessionIndexStore.swiftSources/SessionIndexView.swiftSources/TaskManagerTypes.swiftcmuxTests/CLIGenericHookPersistenceTests.swiftcmuxTests/RestorableAgentHookProviderResumeTests.swiftcmuxTests/RestorableAgentNonInteractiveTests.swiftcmuxTests/SessionIndexViewTests.swiftcmuxTests/SessionPersistenceTests.swiftcmuxTests/TaskManagerResourcesTests.swift
Greptile SummaryThis PR adds Grok Build CLI as a first-class agent: hook installer (
Confidence Score: 5/5Safe to merge; the Grok integration is self-contained and all previously-identified issues have been addressed. All items raised in previous review threads are resolved: single-call resolveAgentHookTarget, persisted lastNotificationStatus, shell-trace logging gated to #if DEBUG, publishesStopNotification: false eliminating double-notify, and symmetric legacy-hook file pruning. No new defects found. No files require special attention. Important Files Changed
Reviews (31): Last reviewed commit: "fix: add grok stop completion fallback" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 19 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizerPrimaryPolicies.swift">
<violation number="1" location="Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizerPrimaryPolicies.swift:183">
P2: `--worktree`/`-w` are dropped but not marked as value options in `grokPolicy`, so spaced forms leave an orphan path argument and can truncate subsequent preserved options.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic
Stale bot review: all requested CodeRabbit inline threads were addressed, CodeRabbit marked them resolved, and the CodeRabbit status check is passing on the latest commit.
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="CLI/cmux.swift">
<violation number="1" location="CLI/cmux.swift:21140">
P2: Fallback notification restoration reuses body/subtitle but keeps stale status, which can incorrectly set the agent to Idle.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
CLI/cmux.swift (1)
18923-19006:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSanitize hook text before showing it in notifications.
This path forwards raw hook
message/body/text/errorfields into user-visible notifications and also injectsdef.displayNameinto fallback/status copy. That leaks upstream/vendor-specific details into alerts, which this repo explicitly forbids. Please map these events to cmux-owned copy and keep raw payload text only in sanitized logs/telemetry.As per coding guidelines “Do not introduce/alter user-facing errors, alerts, recovery copy, or command output that expose implementation details… vendor/service names… raw upstream error messages… [or] request/session IDs…”
Also applies to: 21344-21345, 21411-21429
🤖 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 `@CLI/cmux.swift`:
- Around line 18995-19000: The current fallback branch in
AgentHookNotificationSummary creation collapses any unrecognized non-empty
notification into status: .idle which triggers an Idle publish and can
incorrectly clear active work; update the logic (the
AgentHookNotificationSummary construction and the code path that calls
set_status/publishes the status) to use a neutral/no-status value (e.g., nil or
a dedicated .unknown/.none) instead of .idle for unclassified messages and
ensure set_status is only invoked when classification confidence is high —
adjust the fallback in the AgentHookNotificationSummary creation and the
downstream switch/publish that sends Idle so that unrecognized notifications do
not automatically call set_status(.idle).
- Around line 20045-20048: The current check uses fm.fileExists(atPath:
configDir) which returns true for regular files too; update the logic to call
fm.fileExists(atPath:isDirectory:) and inspect the isDirectory Bool: if the path
exists but isDirectory == false, return/throw the same sanitized error/guidance
as the missing-dir branch (so callers know to fix GROK_HOME or the override),
and only call fm.createDirectory(atPath:configDir, withIntermediateDirectories:
true) when the path either does not exist or exists and isDirectory == true;
reference configDir, def.createConfigDirIfMissing,
fm.fileExists(atPath:isDirectory:), and
fm.createDirectory(atPath:withIntermediateDirectories:) when making the change.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 1eb9ddff-a356-4b8d-acbe-5eacc05a4070
📒 Files selected for processing (6)
CLI/cmux.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizer.swiftREADME.mdResources/Localizable.xcstringscmuxTests/RestorableAgentHookProviderResumeTests.swiftdocs/cli-contract.md
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This branch was successfully deployed
2 participants
Summary
Fixes #4220.
Verification
Note
Medium Risk
Moderate risk: adds a new agent integration and significantly changes hook targeting, notification delivery/clearing semantics, and socket connection behavior, which can affect session restore and user-visible notifications.
Overview
Adds Grok as a supported generic agent hook integration (new
grokAgentHookDef, env override viaGROK_HOME+ subpath, optional config dir auto-create, legacy hook pruning, and Grok-specific hook shell wrapper/pinned socket behavior).Extends the hook runtime to handle a new
notificationaction (plusnotifyalias), persist last notification/status/runtime state per session, dedupe certain Grok notifications, improve completion/waiting classification, and strengthen target resolution (direct args/env/PID-based binding) so hooks can still map to the correct workspace/surface when env is stripped.Updates surrounding infrastructure: adds Grok to restorable agent kinds/session UI/task-manager detection and resume command generation; adds Grok launch argument sanitizer policy and preserves
GROK_*env vars; introduces per-panelclear_notifications --panel, non-coalescingnotify_target_async, notification queue/store instrumentation + surface-scoped clears, socket connect retries, safer socket-path override rules for tagged dev builds, and minor title-sync/notification suppression tweaks.Reviewed by Cursor Bugbot for commit 8559c88. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds Grok Build CLI as a first-class agent with installer, resume (-r), task-manager detection, a Feed PreToolUse bridge, and durable per-panel notifications. Also tightens the notification waiting/needs-input classifier and hardens delivery, sockets, and auto title sync, aligning with Linear #4220.
New Features
cmux-session.jsonto~/.grok/hooks/(respectsGROK_HOME+hooks), auto-creates the dir, errors if a file blocks the path, prunes legacy hooks..grokkind withgrok -r <id>; sanitizer drops optional--resume/-rand--worktree/-w; preservesGROK_HOME/GROK_SANDBOX; adds Grok task-manager detection and a Feed PreToolUse bridge.notification/notify; parses title/subtitle/body; persists last message and status (idle/needsInput/error) with a neutral fallback; suppresses placeholder/internal stop events.Bug Fixes
clear_notifications --panel=<id>; structured-agent panels suppress raw terminal notifications;notify_target_asyncno longer coalesces repeats; restores Grok stop-notification de-duplication; adds a stop-completion fallback when stop events are missing.CMUX_SOCKET_PATHunless opted-in; dev CLI shim execsCMUX_BUNDLED_CLI_PATHwhen set.minisign).Written for commit 8559c88. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
notifyaccepted as synonym ofnotification.Improvements
Documentation
Tests