Connect Grok Build as a first-class coding agent - #703
Conversation
Setup knew its agents by name in five places: the BASECAMP_SETUP_AGENT selector arm (`case "claude", "codex":`) and its unknown-value message, the `setup agents` help text, agentBinaryPresent's switch on ids, and the wizard's escape hint. Each was a list to extend by hand, and the registry that already held every agent could have answered for all of them. AgentInfo gains FindBinary, so `setup agents` reads each agent's own lookup rather than a switch that assumes unknown ids have a binary. The selector's default arm asks harness.FindAgent and lists the accepted values from the registry; the help text and the escape hint read the same list. The shared skill's health predicate moves to harness as harness.BaselineSkillInstalled (with AgentSkillPath naming the file) so an agent's health check can be the very same stat that setup and doctor use, rather than a second reading of "installed". Two verdicts tighten in `setup agents`. Connected now means the handler succeeded and the checks pass, the verdict `setup <id>` already reached on its own: a check that passes despite a setup error is a conflict to report, not a connection. And the synthesized "binary not found" remediation applies only when the absence kept the agent from connecting — Claude's plugin is read from installed_plugins.json, so a machine with the plugin installed and no `claude` on PATH is connected, and was warned about anyway. The per-agent command and its summary stop saying "plugin" — `setup <id>` is "Connect <agent> to Basecamp" and answers "connected" / "not connected" — because the next agent to arrive has no plugin, and the Codex picker row's CODEX_HOME lookup becomes agentHomeSkillPath, a home-env-plus-default helper the next row can share.
Register xAI's Grok Build in the agent harness, add `basecamp setup grok`, and include Grok in the skill picker, doctor, `setup agents`, the installers and the docs — a peer of Codex everywhere Codex appears. Grok has no Basecamp plugin. It reads user skills from ~/.grok/skills and from the cross-agent ~/.agents/skills, so the shared ~/.agents/skills/basecamp skill is the whole integration: `setup grok` installs it and confirms it is healthy, and doctor's "Grok Skill" check is skill presence — the same harness.BaselineSkillInstalled predicate setup and doctor already answer from. Nothing is copied into $GROK_HOME/skills; the picker's "Grok (Global)" row is there for someone who chooses that copy, as Codex's is, and refresh keeps it current like every other row. Detection is $GROK_HOME or ~/.grok, or a `grok` binary on PATH, in ~/.local/bin, or in $GROK_HOME/bin, where Grok Build's installers put it (the npm package honors $GROK_HOME/bin). Setup never fabricates the home directory: a machine without Grok gets "Grok not detected" and the shared skill, not a ~/.grok that would make every later detection lie. Rather than a second copy of codex.go, Grok is the first row of harness.SkillAgent — name, id, home env var, home directory, binary — whose methods (Detect, FindBinary, Home, CheckSkill) and registration are written once. Codex is deliberately not a row: it has a native plugin here, so its detection, setup and health stay its own. The command layer builds the setup handler for every row, the selector and doctor's remediation read the table, and the tests run once per row, so the next skill-only agent is a new row plus the prose lists in the installers and docs.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
Windows binary discovery and platform-dependent tests need correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds Grok Build as a first-class, skill-only coding-agent integration.
Changes:
- Registers Grok detection, setup, health checks, and skill locations.
- Derives agent selectors and setup handlers from the registry.
- Updates installers, documentation, CLI surface, and tests.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
skills/basecamp/SKILL.md |
Documents the Grok selector. |
skills/basecamp-doctor/SKILL.md |
Adds Grok remediation guidance. |
scripts/install.sh |
Supports Grok during post-install setup. |
scripts/install.ps1 |
Adds equivalent PowerShell support. |
README.md |
Documents Grok integration and setup. |
internal/harness/skill_agent.go |
Implements shared-skill agent support. |
internal/harness/skill_agent_test.go |
Tests shared-skill agent behavior. |
internal/harness/harness.go |
Centralizes shared-skill health checks. |
internal/harness/codex.go |
Registers Codex binary discovery. |
internal/harness/claude.go |
Registers Claude binary discovery. |
internal/harness/agent.go |
Extends agent metadata with binary lookup. |
internal/harness/agent_test.go |
Tests binary lookup registration. |
internal/commands/wizard.go |
Generates registry-based setup guidance. |
internal/commands/wizard_test.go |
Updates setup command expectations. |
internal/commands/wizard_skill_agent_test.go |
Tests Grok setup workflows. |
internal/commands/wizard_agents.go |
Integrates skill-only agents into setup. |
internal/commands/skill.go |
Adds the Grok global skill target. |
internal/commands/skill_test.go |
Tests agent-specific skill paths. |
internal/commands/setup_agents_test.go |
Covers Grok selectors and outcomes. |
internal/commands/doctor.go |
Adds registry-based Grok remediation. |
internal/commands/doctor_test.go |
Tests Grok doctor breadcrumbs. |
install.md |
Documents Grok installation. |
e2e/setup.bats |
Covers non-terminal Grok setup. |
e2e/installer.bats |
Covers installer compatibility behavior. |
AGENTS.md |
Documents the integration architecture. |
.surface |
Records the new CLI surface. |
Review details
Suppressed comments (1)
internal/harness/skill_agent_test.go:158
- Windows does not enforce Unix directory permission bits, so
Chmod(..., 0o000)does not make this directory unreadable and the assertions below fail. Skip this permission-semantics test on Windows as well as when running as root.
if os.Getuid() == 0 {
t.Skip("root bypasses directory permissions")
}
- Files reviewed: 26/26 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 004ea1e47a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A file or directory merely named grok in ~/.local/bin or the agent's bin was reported as the binary, and made Detect say Grok was present with no home directory. Each candidate now goes through exec.LookPath, which requires an executable regular file on Unix and resolves the PATHEXT extension on Windows, where the official binary is grok.exe. The test stubs take that name on Windows too, so the PATH-lookup test exercises the implementation there instead of failing on the extension.
There was a problem hiding this comment.
🟡 Changes recommended
The new binary-detection test fails on Windows because its Grok stub lacks the required .exe suffix.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 26/26 changed files
- Comments generated: 1
- Review effort level: Balanced
The harness tests took platform-named stubs (grok.exe on Windows) in the previous commit; the `setup grok` test that puts a binary on PATH still wrote the bare name, so it would have failed on a Windows checkout for the same reason. Same naming rule here.
|
Review round addressed at 0be24bc (on top of a merge of Copilot ("Windows binary discovery and platform-dependent tests"): both fixed. Codex: P2 on fallback executability — fixed as above. P2 on |
There was a problem hiding this comment.
🟡 Changes recommended
Binary-remediation logic can misdiagnose skill failures, and several new tests are environment-dependent.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
internal/commands/skill_test.go:281
skillLocationswas initialized before this test runs, so clearingCODEX_HOMEandGROK_HOMEhere cannot change its stored paths. When either variable was set when the test process started, these default-path assertions fail. The helper calls above already verify default and overridden paths; this block should only verify that both picker rows exist.
internal/harness/skill_agent_test.go:30os.UserHomeDirusesUSERPROFILEon Windows, so changing onlyHOMEdoes not isolate these tests there. They can inspect the developer's real.grokand shared skill directories, making the assertions flaky. PointUSERPROFILEat the same temporary directory too.
- Files reviewed: 26/26 changed files
- Comments generated: 3
- Review effort level: Balanced
A shared-skill agent detected by its home directory never runs its binary during setup, so when the shared skill failed to install the synthesized "binary not found" remediation named the wrong cause next to the error that named the right one. The remediation now applies to a plugin agent that is not connected, or to a shared-skill agent absent altogether. emptyHome also isolates what the new tests reach: USERPROFILE, which os.UserHomeDir reads on Windows, and each shared-skill agent's home override such as GROK_HOME, so a developer's own Grok install cannot leak into the setup-agents tests.
There was a problem hiding this comment.
🟡 Changes recommended
Windows test portability and one Grok binary fallback path remain incorrect.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
internal/harness/skill_agent.go:87
- When
GROK_HOMEis set,a.Home()points only at the override, so this drops the official shell installer's default$HOME/.grok/bin/grokfallback. That installer uses$HOME/.grok/binindependently ofGROK_HOME; before the shell reloads PATH, a curl-installed Grok can therefore be reported absent. Probe the default home bin as well as the overridden home bin.
internal/harness/skill_agent_test.go:28 isolatedHomedoes not isolate the Windows home directory:os.UserHomeDirreadsUSERPROFILEthere, so these tests can detect or modify the developer's real.grokand.agentsdirectories. SetUSERPROFILEto the same temporary home, as the command test helper already does.
- Files reviewed: 26/26 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37b946c19c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Origin: basecamp/hey-cli#344 by @shawnyeager — this ports that design (Shawn's) to basecamp-cli.
What it does
Registers xAI's Grok Build in the agent harness and makes it a peer of Codex everywhere Codex appears:
basecamp setup grok(generated from the registry, likesetup claude/setup codex)BASECAMP_SETUP_AGENT=grokforsetup agents; the accepted list (claude, codex, grok, all, or none) is now read from the registry, in the help text, the unknown-value warning and the wizard's escape hintbasecamp setup: Grok detected → its handler runs, warn-and-continue like Codexbasecamp doctor: a "Grok Skill" check, with abasecamp setup grokbreadcrumb on failurebasecamp skillpicker: a "Grok (Global)" row at$GROK_HOME/skills/basecamp/SKILL.md(default~/.grok/…), refreshed in place like Codex's rowscripts/install.sh/install.ps1:grokin the selector docs and the old-binary fallback dispatch.surfaceregenerated (23setup grokentries)Design
Skill-only. basecamp-cli ships no Grok plugin. Grok reads user skills from
~/.grok/skills/and from the cross-agent~/.agents/skills/, so the shared~/.agents/skills/basecamp/SKILL.mdis the whole integration:setup grokinstalls the shared skill and confirms it is healthy, and doctor's check is skill presence. Nothing is copied into$GROK_HOME/skills(the picker row is there for someone who chooses that copy, as Codex's is).One health predicate.
baselineSkillInstalled()moves into harness asharness.BaselineSkillInstalled()/harness.AgentSkillPath();CheckSkillanswers from the same stat, so setup, doctor and the agent's check can never disagree about "installed".harness.SkillAgenttable. Rather than a second copy ofcodex.go, Grok is the first row of a table —{Name, ID, HomeEnv, HomeDir, Binary}— whose methods (Detect,FindBinary,Home,CheckSkill) and registration are written once. The command layer builds askillAgentSetupHandlerper row, the selector and doctor's remediation read the table, and the tests run once per row (forEachSkillAgent). A future skill-only agent is a new row plus the prose lists in the installers and docs.Codex is deliberately not a row. In basecamp-cli, Codex has a native plugin (
.codex-plugin, installed via the 37signals marketplace) with its own detection, setup and version checks — that is the divergence from hey-cli#344, where Codex and Grok are both rows because hey has no Codex plugin. The table's comment says so. No legacy-copy migration is ported either: basecamp-cli never auto-copied the skill into~/.codex/skills(the picker copy is user-chosen), so there is nothing to migrate.Never fabricates the agent.
setup grokgates on detection first; a machine without Grok gets "Grok not detected" plus the shared skill, and no~/.grokis created (a fabricated home would make every later detection — and the command's own verdict — lie).Registry lifting (first commit).
AgentInfogainsFindBinary(set for Claude and Codex too),agentBinaryPresentreads it instead of a switch on ids, the selector'sdefault:arm asksharness.FindAgent, and twosetup agentsverdicts tighten: "connected" now requires the handler to have succeeded and checks to pass (the verdictsetup <id>already reached), and the synthesized "binary not found" warning applies only when the absence actually prevented the connection (a Claude with the plugin already installed and noclaudeon PATH is connected and used to be warned about anyway).setup <id>reads "Connect to Basecamp" and answers "connected" / "not connected" rather than "plugin installed", since Grok has no plugin.Verified Grok facts
grok. Home~/.grok, overridden byGROK_HOME— docs.x.ai/build/settings;xai-dirscrate in xai-org/grok-build.~/.grok/skills/and from the cross-agent~/.agents/skills/— docs.x.ai/build/features/skills-plugins-marketplaces ("Agents.md compatibility").curl -fsSL https://x.ai/cli/install.sh | bash,npm i -g @xai-official/grok) put the binary at~/.grok/bin/grok; the npm package honors$GROK_HOME/bin. HenceFindBinarylooks on PATH, then~/.local/bin, then$GROK_HOME/bin..grok-plugin/plugin.jsonmanifest exists as a fallback behind a rootplugin.json, but basecamp-cli ships no Grok plugin — Grok is skill-only here.Tests
skill_agent_test.go, per row): registration wiring; detect by home dir; detect by binary on PATH;FindBinaryoff PATH in~/.local/bin,<home>/binand$GROK_HOME/bin;Homehonors the env override;CheckSkillfail/pass/warn states, each asserted againstBaselineSkillInstalledso the two cannot drift; missing-home warn.AgentInfo.FindBinarywiring for Claude.wizard_skill_agent_test.go, per row):setup grokexists with the registry Short; handler in the table; not-detected envelope withbasecamp setup grokremediation and no~/.grokfabricated; connected via~/.grok; connected via$GROK_HOMEoverride and via agrokbinary alone; interactive handler warns-and-continues through not-detected, skill-missing and success.setup agents: single-detected Grok connects with no spurious binary warning; explicitgrokselector;allattemptsclaude, codex, grok; not-detected Grok is "attempted", not "connected" (passing skill check ≠ connection); unknown value lists every agent; Claude-already-connected-without-binary no longer warns.agents, claude, codex, grok).setup grokstill runs without a terminal; installer old-binarygrokdegrades toskill install, new-binarygrokdispatches tosetup agents,allnever invokes an unadvertisedsetup grok..agentscreated),~/.grok,$GROK_HOME/bin/grok, the selector,doctor --json,setup --help.Local gate: fmt, vet, lint, lint-actions, test-e2e (467/467), check-naming, check-surface, check-skill-drift, check-bare-groups, check-lint-lockstep, check-smoke-coverage, provenance-check and tidy-check all pass. Eleven unit tests across
appctx,cli,stdinarg,tui/resolveandcommandsfail identically on untouchedmainon this macOS host — they assert on/dev/ptmx-faked or pipe-shaped stdio — and are untouched by this PR.Summary by cubic
Registers xAI's Grok Build as a first-class coding agent, a peer of Claude and Codex in setup, doctor, the skill picker, the installers, and the docs. Grok is skill-only: it reads the shared
~/.agents/skills/basecampskill directly, so no plugin ships.New Features
basecamp setup grokinstalls the shared skill and confirms it's healthy; on a machine without Grok it reports "not detected" and never creates~/.grok.BASECAMP_SETUP_AGENTnow acceptsgrok, and the accepted values are read from the harness registry instead of a hardcoded list.basecamp setup grokremediation breadcrumb; the skill picker gains a "Grok (Global)" row.Refactors
harness.SkillAgenttable; a future skill-only agent is a new row plus prose in the installers and docs.AgentInfogainsFindBinary, and the skill-installed predicate moves to harness so setup, doctor, and the agent's own check share one answer.exec.LookPath, so a non-executable file or directory with the agent's name is not treated as the binary.setup <id>summaries now say "connected" / "not connected" instead of "plugin installed", and the missing-binary warning only fires when the absence actually prevented the connection.Grok is detected by
$GROK_HOME(default~/.grok) or agrokbinary onPATH, in~/.local/bin, or in$GROK_HOME/bin, where its installers put it. Setup never fabricates the home directory, and Codex is deliberately not a row in the table because it has a native plugin. Tests run once per table row, so the Windows test stubs are platform-named (grok.exe) like the harness stubs.Written for commit 37b946c. Summary will update on new commits.