chore(grok): reduce run-grok.sh to setup-only, move its tests to the adapter - #785
Merged
Conversation
…adapter With every surface dispatching through run-harness (#784), run-grok.sh's run path had no callers: ~260 lines of model/permission flags, MCP allows, run-shaping knobs, grok invocation and envelope normalization - a second copy of adapters/grok.sh. A duplicate that drifts is how grok's MCP folder-trust gate stayed broken on two surfaces for two releases, so it goes. What remains is the CLI version pin and the GROK_CREDENTIALS restore + rotating-refresh-token persistence (§2b), which the workflows call as `run-grok.sh setup`. A stale caller using the old contract (prompt on stdin) now exits 2 with a pointer to run-harness. Staging grok and exiting 0 would hand it empty stdout, which reads as "the model returned nothing" - the silent no-op class this whole consolidation was about. The flag/envelope/MCP coverage is MOVED, not deleted. scripts/tests/ test_harness_adapter_grok.sh is the first test suite for harness-adapter and asserts against the code that now implements those behaviours: the --trust regression guard, the thought firewall, per-server MCPTool allows, model-id forwarding, GROK_* knob plumbing, and abnormal-stop handling. Verified the guard actually bites by reverting --trust in the adapter and watching it fail. Also corrects comments across skill_mode.sh, llm-gateway.sh, aeon.yml and the dashboard that still described run-grok.sh as the thing running grok. Note: `skill_mode.sh grok-args` is now orphaned - run-grok.sh was its only consumer, and the adapter maps capability mode differently (bypassPermissions + the dispatcher's OS sandbox). Left in place and labelled rather than deleted, because removing it means deciding what grok read-only enforcement should be; docs/CAPABILITIES.md still documents that allowlist as the mechanism.
aaronjmars
added a commit
that referenced
this pull request
Jul 27, 2026
…788) The docs described a guard that does not exist. docs/CAPABILITIES.md and docs/harnesses.md both said a mode: read-only skill is confined on grok by an explicit tool allowlist under grok's own --sandbox read-only, generated by scripts/skill_mode.sh grok-args. Every part of that is wrong: * adapters/grok.sh carries no allowlist, deliberately. Headless grok aborts the entire turn on a denied tool (stopReason=Cancelled) instead of degrading, and skills are authored for Claude Code, so they reach for tools no allowlist predicted. It runs --permission-mode bypassPermissions with no --allow and no --deny rules. * grok's own --sandbox read-only is silently ignored on grok 0.2.101 (writes still land) and nest-conflicts with the wrapper sandbox. * grok-args itself had no callers once run-grok.sh became setup-only (#785). So an operator reading either doc would conclude a read-only grok skill was tool-confined when nothing was confining it. What actually enforces read-only, on all six harnesses, is the dispatcher's OS sandbox in harness-adapter/lib/sandbox.sh plus the workflow's post-run revert. Changes: * scripts/skill_mode.sh - grok-args removed, not rewired: the real mechanism is not expressible in this file. A tombstone comment records why it should not come back in that shape. The header no longer claims allowedTools is the enforcement. * docs/CAPABILITIES.md - replaces the wrong paragraph with the three enforcement layers and which one is load-bearing, plus why native harness sandboxes are not relied on (codex's kills the network, grok's is ignored, pi/vibe/kimi have none). Also disambiguates capabilities: [read_only] (documentation, unenforced) from mode: read-only (enforced). * docs/harnesses.md - rewrites the capability-mode section for grok. * harness-adapter/lib/sandbox.sh - header said "five harnesses" and named opencode, which is not one of the six. * harness-adapter/docs/aeon-integration.md - marks its three-branch shape as superseded; aeon routes every harness through run-harness now. * scripts/tests/test_skill_mode.sh - the three grok-args cases asserted a guard that never worked. Replaced with a guard that the subcommand stays gone and that the usage line no longer advertises it. All 17 capability-mode tests pass, plus notify, skill-requires, harness-adapter grok and run-grok suites; okf-validate clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Final piece of the run-path consolidation (#779 → #781 → #782 → #784).
What goes
With every surface dispatching through
run-harness(#784),scripts/run-grok.sh's run path had no callers — ~260 lines of model/permission flags, MCP allows, run-shaping knobs, grok invocation and envelope normalization. It was a second copy ofadapters/grok.sh, and a duplicate that drifts is precisely how grok's MCP folder-trust gate stayed broken on two surfaces for two releases.474 → 206lines. What remains is the CLI version pin and theGROK_CREDENTIALSrestore + rotating-refresh-token persistence (§2b), which the workflows call asrun-grok.sh setup.A stale caller now fails loudly. Using the old contract (prompt on stdin) exits 2 with a pointer to
run-harness:Staging grok and exiting 0 would have handed it empty stdout, which reads as "the model returned nothing" — the silent no-op class this whole thread was about.
The tests move, they don't disappear
harness-adapterhad no test suite at all, so deletingtest_run_grok.sh's flag/envelope/MCP cases would have dropped the regression guard for the exact bug this thread started with. Newscripts/tests/test_harness_adapter_grok.sh— the first tests forharness-adapter— asserts against the code that now implements those behaviours:--trustis passed for a repo-local.mcp.json, and does not leak into a non-MCP run--allow MCPTool(<srv>__*)per declared server.result--modelforwarded for a real grok id, omitted for a leftoverclaude-*idGROK_MAX_TURNSreaches the adapterI checked the guard actually bites rather than trusting a green run: reverting
ARGS+=(--trust)in the adapter turns it red.test_run_grok.shkeeps auth gating and all five OAuth refresh/persist cases, and gains three for the new subcommand contract. Both are wired intoci-tests.yml.Also
Comments in
skill_mode.sh,llm-gateway.sh,aeon.ymlandapps/dashboard/lib/constants.tsstill describedrun-grok.shas the thing that runs grok, passes--no-subagents, supplies the compat--rules. Corrected to point at the adapter.One thing deliberately left
skill_mode.sh grok-argsis now orphaned.run-grok.shwas its only consumer, andadapters/grok.shmaps capability mode differently —--permission-mode bypassPermissionsplus the dispatcher's OS sandbox, because a denied tool aborts grok's whole turn.I labelled it rather than deleting it, because removing it means settling a question I shouldn't answer unilaterally:
docs/CAPABILITIES.mdstill documents that allowlist (plus--sandbox read-only) as how grok enforces read-only, and that is not what runs today. Worth a look — grok read-only currently rests on the post-run stray-write revert, sinceaeon.ymlpasses--no-sandboxfor grok. Deciding whether that is the intended posture is a separate call from this cleanup.