Add grok CLI and Qwen Code, and stop our Claude hooks running inert inside grok - #719
Draft
chhhee10 wants to merge 7 commits into
Draft
Add grok CLI and Qwen Code, and stop our Claude hooks running inert inside grok#719chhhee10 wants to merge 7 commits into
chhhee10 wants to merge 7 commits into
Conversation
…e grok grok is the 13th integration and Qwen Code the 14th — both dual-pillar (live hooks + audit), user and project scope. Every contract claim below was verified against the running CLI with a recorder hook on every event plus deny and stop-gate probes, not read off a vendor doc; where the two disagreed, the wire won and the disagreement is recorded. The fix is the part that matters on machines that install neither. grok's hook discovery scans ~/.claude/settings.json, ~/.claude/settings.local.json and <cwd>/.claude/settings.json by default. The last is exactly the file `policies --install --cli claude --scope project` writes, so on any machine with both tools grok was already executing our hooks — passing `--cli claude` while piping its own camelCase payload. tool_name and tool_input arrived undefined, so every builtin that matches a tool name or reads a command or path (block-sudo, block-env-files, block-secrets-write, block-force-push) saw nothing and allowed. A deny would not have landed either: grok ignores Claude's hookSpecificOutput shape, proven by A/B on one live hook, where Claude's shape let the command run and grok's blocked it. Installed, running, costing latency, enforcing nothing — worse than no coverage, because the install reports success. resolveEffectiveCli() detects grok's envelope from the payload shape alone (hookEventName + workspaceRoot with no hook_event_name — a shape Claude never sends) and routes the event onto grok's contract, tool maps and response shape both. Deliberately not an env-var check: GROK_HOOK_EVENT is set by grok's runner but is still just an env var, and misreading a real Claude event would break Claude's own enforcement. Real Claude payloads are untouched. Three things found by probing that no doc states, each of which silently produces enforcement that looks present: - grok's read_file delivers the path as `target_file` (and list_dir as `target_directory`), so without GROK_TOOL_INPUT_MAP a live .env read walks past block-env-files — the identical bug COPILOT_TOOL_INPUT_MAP fixed. - grok fires Stop TWICE, the second at shutdown with the decision parsed and discarded. The Stop branch gates on reason === "end_turn"; blocking on the shutdown fire would record a deny nothing can act on. - grok discovers project hooks only inside a git repo. A trusted non-git dir holding a valid .grok/hooks/*.json logs project_sources=0 and never fires. qwen needs none of that: its payload is pure Claude snake_case and all six of its tools already deliver canonical keys, so it takes no event map, no payload normalization and no tool-input map — only a name map, plus a Stop branch for the one shape that diverges. Two of its behaviours are traps for policy authors and are documented rather than worked around: stop_hook_active is true on the FIRST Stop fire, so it cannot serve as a loop guard, and UserPromptSubmit fires once per model invocation (four times in one observed turn), not once per prompt. Audit adapters read real JSONL for both. qwen's bodies are Gemini-shaped parts[], not Claude content blocks; grok's chat_history.jsonl carries no timestamps at all, so the parser anchors on summary.json's created_at and lays turns out in file order rather than inventing wall-clock times it does not have. Verified end to end against both live CLIs with the built binary: block-sudo and block-env-files each blocked, and for the leak path a grok payload on a --cli claude hook now emits grok's deny shape with the tool canonicalized, while a real Claude payload still emits Claude's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
grok 8 -> 14 events (its entire surface), qwen 12 -> 19. Surveyed before
implementing rather than after, because both CLIs fail quietly here: grok
silently SKIPS hook config keys it does not recognize, so a wrong event name
costs coverage with no error anywhere, and qwen's documented event table turned
out to be incomplete.
What the survey established:
- A live grok accepted all 14 keys (loaded hooks hook_count=14, no unknown-key
warning). Notification and StopFailure were additionally observed firing --
for free, off a quota-exhausted session, since a 429 is exactly the API error
StopFailure fires on. That same run independently validated the end_turn gate
from the previous commit: the failed turn produced StopFailure plus a Stop
with reason "shutdown", and we correctly declined to block it.
- Every qwen event has a real executeHooks() dispatch site in the shipped
bundle. Reading it also surfaced three events qwen dispatches but does not
document: InstructionsLoaded, UserPromptExpansion and PostToolBatch.
Most additions are observation, and on grok they can only ever be: its ACP
handshake advertises blockingEvents ["pre_tool_use","stop","subagent_stop"] and
that is the complete list. So widening grok buys custom-policy surface and audit
signal, not enforcement, and the cost of an event that never fires is zero.
qwen's TodoCreated/TodoCompleted are the exception and the reason this is worth
more than breadth. A new QWEN_EVENT_MAP canonicalizes them onto TaskCreated/
TaskCompleted, and they run in a validation phase where {decision:"block"}
genuinely prevents the write -- verified live by blocking a todo that planned to
skip the tests. That is new enforcement surface, not a bigger log. The block
prevents the whole todo_write rather than the single item; that is upstream's
semantics, not ours.
Three events are deliberately left out, each for a measured reason rather than
caution: MessageDisplay fires per streaming chunk, so subscribing means a hook
process per chunk; PostToolBatch fired 6 times in a task where PostToolUse fired
5, carrying the same tool calls in batch form, which measured +76% hook
invocations against no builtin that reads it; SessionDelete has no canonical
equivalent. Each is one line to add if a custom policy ever wants it.
The 40-builtin count is unchanged at 35 for both -- no builtin subscribes to any
added event. This buys custom-policy surface, audit signal, and one real veto
point on qwen.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by installing the packed tarball and running the real command, which is the only way it could have been found: `policies --install --cli grok` failed with "Missing value(s) for --cli" while every unit test passed. bin/failproofai.mjs carried a THIRD hardcoded CLI list, VALID_CLIS, in three copies -- separate from INTEGRATION_TYPES and separate again from the `--hook --cli` validation that the previous commits updated. grok and qwen reached the other two and not this one, so the hook path worked perfectly while the install path refused the CLI outright. The accompanying usage string was staler still, naming eight CLIs and omitting factory, devin, antigravity and goose as well. Replaced all three copies with one module-scope INSTALLABLE_CLIS, derived the usage string from it, and added a test that reads bin/failproofai.mjs and asserts the list equals INTEGRATION_TYPES -- the same shape of tripwire HARNESS_KEYS already uses, because a hand-maintained duplicate of a list is exactly what drifted here. Verified end to end afterwards against both real CLIs with the globally installed tarball: `policies --install --cli grok --scope user` writes 14 event types, `--cli qwen` writes 19 and preserves the user's model, providers and env block untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four gaps, all reported from a live dashboard: the two badges were indistinguishable, neither CLI appeared in the filter dropdown, neither contributed to the projects list, and the daemon collected nothing from either. Two of those were not code gaps. The filter options and badge colours both derive from KNOWN_CLI_IDS / CLI_ENTRIES, which already listed grok and qwen -- the shipped .next bundle simply predated them, because the tarball had been packed with --ignore-scripts, which skips the Next build. A real build fixes both; there was nothing to add. The other two were real: - lib/projects.ts and the project detail page never aggregated either CLI, so sessions that were sitting on disk had no way to appear. Both now merge, and the projects test mocks them, since a developer machine that has used either CLI would otherwise leak its own sessions into those assertions. - The badges moved off the status palette. The dashboard design system is explicit that green/amber/red carry health meaning and must never be spent on identity, and the first fix reached for yellow, which reads as "degraded". grok is now neutral zinc (matching --color-default, and suiting xAI's monochrome brand) and qwen magenta -- outside the status set, and maximally far apart, which was the actual complaint. The daemon half is two new fpai-collect sources with their main.rs tasks and HARNESS_KEYS entries on both sides. qwen follows the Factory model -- one JSONL per session, real timestamps -- but its bodies are Gemini-shaped parts[] with functionCall/functionResponse, so the transform is its own rather than a clone. grok follows the CURSOR model instead, because its transcript carries no timestamps at all. Per-event times live in a sibling events.jsonl that is not 1:1 with the turns, so rather than mis-pair them, events are stamped from the file mtime plus byte offset: time stays approximately right AND a pure function of the inputs, which is what lets the content-hash dedup collapse a re-read instead of storing it twice. grok needed its own path rules too -- the session id is the parent directory, since every transcript is named chat_history.jsonl; the cwd folder is percent-encoded where everyone else dash-encodes, which at least makes it reversible; and tool_calls[].arguments arrives as a JSON string, parsed here so tool inputs stay queryable like every other source's. Only user lines carrying prompt_index count as operator prompts, because grok writes its environment preamble and its own reminder injections as user lines as well, and surfacing those would make a session read as if the human pasted grok's boilerplate. Verified against real data on this machine: 20 grok and 17 qwen transcripts discovered by the audit adapters with tool names canonicalized and grok's percent-encoded cwd decoded, both CLIs present in the rebuilt dashboard's filter and projects list. Full gate green: 3815 unit, 662 rust, clippy and fmt clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
grok percent-encodes its cwd folder (%2Ftmp%2Ffp-prod), and lib/grok-projects.ts used that folder name as the project's `name`. But `name` IS the URL slug for /project/[name], and a percent-encoded name re-encodes to %252F… inside the link, which the route cannot resolve -- so every grok project row rendered fine and 404'd the moment it was clicked. The slug is now the dash-encoded cwd. That fixes the 404 and a second problem in the same stroke: a name no other CLI can produce merges with nothing, so a directory driven by both grok and qwen showed up as two unrelated rows, one of them dead. Now they land on one row, with a GROK CLI and a QWEN CODE session listed side by side. The decode back to a cwd stays lossy -- `-tmp-fp-prod` decodes to /tmp/fp/prod, not /tmp/fp-prod -- which is exactly why the project page takes its cwd from summary.json's info.cwd and treats the decode as a last resort, the same way the Claude and Factory adapters use their own headers. A test pins that, so nobody "fixes" the lossiness by trusting the decode. Found by opening the page, not by reading the list -- the list rendered correctly the whole time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
These passed on a machine with no failproofaid service and failed on one that had it, which is how they started failing here the moment `failproofai config` installed a real unit. Two distinct bugs, both in the tests. `daemonServiceStatus()` reads the HOST's systemd unit, and no FAILPROOFAI_HOME can sandbox that. So on a machine with a real service, healDaemonFlag() saw it running, failed its end-to-end probe against the temp home's absent socket, and printed its own "cannot evaluate policies" paragraph -- which both cleared daemon.configured and drowned out the version-skew warning these tests are actually about. The daemon-warning block now pins the status to "stopped": the one value healDaemonFlag deliberately ignores, which leaves staleDaemonHint() as the only thing writing lines. That also drops the file from ~12s to ~2s, since the probe is no longer attempted. The second is an assertion that could never have been robust: the warning is hard-wrapped for the terminal, so "denies every tool call" straddles a newline and /denies every tool call/i cannot match it. Both the positive and the negative assertion now normalise whitespace first -- the negative especially, because it would otherwise have passed for the wrong reason the moment the phrase wrapped. Neither the warning text nor fp-reset itself changed; the messages were correct all along. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Marks come from @lobehub/icons-static-svg, which is where the existing twelve came from -- devin.svg still carries that set's signature -- so provenance and drawing style stay consistent rather than me approximating two trademarks by hand. grok's mark is monochrome and the source uses fill="currentColor", which does NOT inherit inside an <img>: it would resolve to the initial colour, black, and disappear against a dark README for roughly half of readers. It ships as an explicit light/dark pair instead, which is what the <picture> elements around it already exist for. Qwen's colour mark is legible on both grounds, so it ships as one file like Claude's and Antigravity's. Both verified rendered on a white and a #0d1117 ground rather than assumed. The grid also moved from 6 columns to 7. That is not cosmetic: the comment above it explains the table exists so columns never re-wrap into ragged orphan rows, and 14 CLIs at 6 columns is exactly the two-cell orphan row it was written to avoid. At 7 it stays two full rows. The 14 translated READMEs under docs/i18n still show twelve; those are generated by scripts/translate-docs and are not hand-edited. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Thanks @chhhee10 for your contribution to Failproof AI! 🙌 We'd love to discuss your PR and welcome you to our community: https://discord.befailproof.ai/ |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization 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:
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 |
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.
Adds grok CLI (xAI) and Qwen Code as the 13th and 14th integrations — live-hook enforcement and audit, user + project scope — and fixes a silent non-enforcement bug that affects people who have installed neither.
Every contract claim here was verified against the running CLI with a recorder hook on every event, plus deny and stop-gate probes. Where a vendor's docs and the wire disagreed, the wire won and the disagreement is recorded in
CLAUDE.md.The fix that matters without either integration
grok's hook discovery scans
~/.claude/settings.json,~/.claude/settings.local.jsonand<cwd>/.claude/settings.jsonby default. That last one is exactly whatpolicies --install --cli claude --scope projectwrites — so on any machine with both tools, grok was already executing our hooks, passing--cli claudewhile piping its own camelCase payload.tool_nameandtool_inputarrivedundefined, so every builtin that matches a tool name or reads a command or path (block-sudo,block-env-files,block-secrets-write,block-force-push) saw nothing and allowed. A deny would not have landed anyway: grok ignores Claude'shookSpecificOutputshape — proven by A/B on one live hook, where Claude's shape let the command run and grok's blocked it.Installed, running, costing latency, enforcing nothing — worse than no coverage, because the install reports success. Same class as the Copilot input-key drift and the Hermes
subagent_stoprow.The handler now detects grok's envelope from the payload shape alone (never an env var, so a real Claude event cannot be misread) and routes it onto grok's contract — tool maps and response shape both. Real Claude payloads are untouched.
Three things only probing found
read_filedelivers the path astarget_file(andlist_dirastarget_directory). Without the input map, a live.envread walks pastblock-env-files.Stoptwice, the second at shutdown with its decision parsed and discarded. Blocking there records enforcement that can never happen, so the gate keys onreason === "end_turn".project_sources=0and never fires — silently.qwen needed none of that: its payload is pure Claude snake_case and all six tools deliver canonical keys. Its traps are documented rather than worked around —
stop_hook_activeistrueon the first Stop fire (unusable as a loop guard), andUserPromptSubmitfires once per model invocation, seven times in one observed turn.Coverage
Both reach 35 of 40 builtin policies enforcing — day-one parity with claude/cursor/devin/factory/antigravity/openclaw. The 5 that don't are the
sanitize-*family onPostToolUse, which only codex and copilot can block; that gap is fleet-wide, not specific to these two.Event surface is each CLI's full useful set: grok 14 of its own 14, qwen 19. Verified before implementing — grok silently skips event keys it does not recognise, and qwen turned out to dispatch three events its own docs never list (
InstructionsLoaded,UserPromptExpansion,PostToolBatch). Three are deliberately left out with measured reasons:MessageDisplayfires per streaming chunk,PostToolBatchmeasured +76% hook invocations while duplicatingPostToolUse, andSessionDeletehas no canonical equivalent.qwen's
TodoCreated/TodoCompletedmap ontoTaskCreated/TaskCompletedand are a real veto point — they run in avalidationphase where a block prevents the write, verified live by blocking a todo that planned to skip the tests.Also in here
fpai-collectsources so their sessions reach the cloud like every other CLI's. qwen follows the Factory model; grok follows the cursor model, because its transcript carries no timestamps at all — events are stamped from file mtime plus byte offset, which keeps time approximately right while staying a pure function of the inputs, as the content-hash dedup requires.policies --install --cli grokwas rejected outright — a third hardcoded CLI list inbin/failproofai.mjs, in three copies, which the unit suite could not see. Now one list plus a tripwire test. Found only by packing and installing the tarball.%252F…. Found by clicking the link, not by reading the page.fp-resettests that passed or failed depending on whether the developer happened to have a daemon installed.Verified end to end
Against both live CLIs with the packed tarball installed globally, and against a real
failproofaidsystemd service:block-sudoandblock-env-filesblocked on each, all response shapes correct, and 197 grok + 207 qwen events delivered to an ingest endpoint — transcripts and hook activity, including sessions started after the daemon was already running.Why it is a draft
observerather thanblock: grokSubagentStop(advertised in its ownblockingEvents, never exercised) and qwenUserPromptSubmit/PostToolUse/SubagentStop. They are probably blocking; nobody has proven it, so nothing claims it.PreToolUsefires for tools called inside a subagent is unverified on both. If it does not, that is an unguarded execution path that currently reads as covered — worth settling before this merges.PostToolUseblock, which would take it to 40/40 and flip the fivesanitize-*rows. One probe would answer it.scripts/translate-docs.Full gate green on the rebased tree: 3864 unit, 320 e2e, 393 rust,
tsc,clippy,fmtand lint clean, andbun run buildpasses.