feat(agent): add native grok agent backend#314
Draft
falkoro wants to merge 6 commits into
Draft
Conversation
One-shot CLI backend for the standalone grok CLI (Grok Build), modeled on the claude/codex/pi pattern: prompt via temp file (--prompt-file), plain text output, schema contract inlined in the prompt (like pi), gitSafeEnv, classifyTransient retries, context cancellation during streaming. - internal/agent/grok.go + grok_test.go - types: AgentGrok; agent.go: factory case + error message - config: agent_args_override allowlist + reserved flags (incl. --effort, which the grok API rejects and must never be injected) - cmd/fakeagent: grok handler for the e2e harness Verified: gofmt, make lint, go test -race ./... (one pre-existing TestInitTracksCommandTelemetry failure also fails on clean upstream), make e2e green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- stream grok stdout with a bufio.Reader (incremental ReadString) instead of a bufio.Scanner with a 256MB max-token buffer, avoiding a large up-front allocation / OOM risk on a single very long line - correct buildGrokArgs comment: managed flags are appended last and therefore take precedence (and are reserved in config); the prior comment claimed the opposite - drop developer-specific /home/falk/.grok/bin/grok paths from grok_test.go in favour of a neutral /path/to/grok Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add `agent: grok`, a native one-shot CLI backend for the standalone grok CLI (Grok Build), modeled on the existing claude/codex/pi pattern. Today the only way to route grok through no-mistakes is via opencode with an xAI model, which adds a server dependency and breaks in two known ways (`opencode serve` rejects `--model` from agent_args_override, and xAI's 200-tool cap kills startup when global MCP servers are configured). A native backend removes that chain. - internal/agent/grok.go: temp-file prompt (`--prompt-file`) so large review prompts never hit argv limits; plain-text output via `--output-format plain`; schema contract inlined in the prompt (grok has no structured-output flag); `gitSafeEnv`; `classifyTransient` retries; incremental `bufio.Reader` streaming with no fixed token cap; and `shellenv.ConfigureShellCommand` so a cancelled context reaps the grok process group, not just the direct child. - internal/types, internal/agent, internal/cli: `AgentGrok`; factory case in `NewWithOptions`; default binary; doctor listing. - internal/config: `agent_args_override` allowlist entry plus reserved flags, including `--effort`, which the grok API rejects (400) and must never be injected. - cmd/fakeagent: grok handler for the e2e harness. - Unit tests for construction, arg ordering (user extraArgs take precedence), schema prompt contract, and the absence of `--effort`. Co-authored-by: Claude <noreply@anthropic.com>
Owner
|
Thanks @falkoro - this grok backend is properly raised via no-mistakes. It's still marked draft, so whenever you're happy with it, take it out of draft and give me a ping - I'll review and merge. No rush. 🙏 |
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
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.
Intent
The developer asked for a security vulnerability review of a committed change that adds Grok as a new agent type in the no-mistakes pipeline. The change wires the grok CLI into agent spawning, configuration validation, doctor diagnostics, types, and a fakeagent test stub, following the same patterns as existing agents such as Claude and Pi. They provided the list of changed files and the unified diff so every new code path could be inspected. They explicitly requested investigation using the assistant's security review methodology and a findings list of any vulnerabilities discovered. The review was scoped to the supplied diff, with implicit focus on subprocess invocation, reserved CLI flags, temp prompt files, permission modes, and whether user-supplied agent arguments could bypass managed invocation controls.
What Changed
grokagent backend that writes prompts to a temp file, invokes the grok CLI with pipeline-managed flags (--prompt-file,--cwd,--permission-mode,--output-format), streams plain-text stdout with a 256MB per-line cap, and retries transient failures.grokas a selectable agent type, reject reserved grok CLI flags (including--singleand--prompt-json) inagent_args_override, and include grok inno-mistakes doctoravailability checks.fakeagent grokstub for pipeline and integration tests.Risk Assessment
✅ Low: Grok integration mirrors established native-agent patterns with reserved managed flags, forced bypassPermissions, 256MB-bounded stdout scanning, process-group cleanup, and global-only agent_args_override, and the prior review hardening addressed the only substantive gaps.
Testing
Exercised Grok unit and integration tests (including a new run-once test that records exact child argv), all eight grok reserved-flag rejection cases in config validation, a fakeagent grok end-to-end invocation that logged managed flags and returned fenced JSON, and doctor output confirming grok is listed when present on PATH. No security regressions or test failures were observed.
Evidence: Fakeagent grok invocation log (managed argv + prompt)
{"agent":"grok","args":["--model","grok-3","--prompt-file","/tmp/nm-grok-manual-QCeE.md","--cwd","/home/falk/.nm-grok/worktrees/d250dc19c698/01KVR3V1D2X5EPMKPEQKNGJHXX","--permission-mode","bypassPermissions","--output-format","plain"]}Evidence: Fakeagent grok stdout (fenced JSON response)
Evidence: Doctor output listing grok agent
✓ grok /tmp/no-mistakes-evidence/01KVR3V1D2X5EPMKPEQKNGJHXX/grokEvidence: Grok agent integration test (argv + fenced JSON parsing)
Evidence: Reserved grok flags rejection tests
Evidence: Security review summary
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
🔧 **Rebase** - 1 issue found → auto-fixed ✅
internal/agent/grok.go- merge conflict rebasing onto refs/remotes/no-mistakes-push/feat/grok-agent🔧 Fix applied.
✅ Re-checked - no issues remain.
🔧 **Review** - 3 issues found → auto-fixed ✅
internal/config/config.go:389- reservedAgentArgs for grok blocks -p/--prompt-file but not the CLI's actual alternate prompt flags --single and --prompt-json. A maintainer who adds either to agent_args_override will get a hard grok CLI error at runtime (the CLI mutually excludes these with --prompt-file), but config load will not catch it early.internal/agent/grok.go:87- Stdout is accumulated in a strings.Builder via unbounded ReadString('\n') with no total-size cap. A single very long line (e.g. minified JSON) can grow memory without limit and OOM the daemon; pi caps scanner output at 256MB.internal/agent/grok.go:137- buildGrokArgs comment claims grok treats the last duplicate flag as authoritative, but the real grok CLI rejects duplicate flags (--model, --permission-mode, --prompt-file) with a hard error. The reserved-arg validation is what actually protects managed flags, not last-wins semantics.🔧 Fix: Harden grok reserved flags and stdout bounds
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
go test ./internal/agent -run 'Grok' -count=1 -vgo test ./internal/config -run 'ReservedArgsRejected' -count=1 -vgo build -buildvcs=false -o /tmp/no-mistakes-evidence/01KVR3V1D2X5EPMKPEQKNGJHXX/fakeagent ./cmd/fakeagentthen fakeagent grok symlink invocation with--prompt-file,--cwd,--permission-mode,--output-formatgo build -buildvcs=false -o /tmp/no-mistakes-evidence/01KVR3V1D2X5EPMKPEQKNGJHXX/no-mistakes ./cmd/no-mistakesthenno-mistakes doctorwith grok on PATH✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.