Summary
react_on_rails:install_rsc_agent_guardrails (shipped in #4606) registers its PostToolUse hook using a "command" + "args" array shape that is not part of Claude Code's hook schema. Claude Code hook entries take a single command string. As written, the installed RSC advisory guardrail very likely never runs in a user's app.
Flagged in review on #4606 (thread) with an explicit "please double-check this against Claude Code's actual hook schema before merging" — it merged before that verification happened. My fault as batch coordinator; filing so it isn't lost.
The bug
react_on_rails/lib/react_on_rails/agent_guardrails.rb:
HOOK_COMMAND = "ruby"
HOOK_ARGS = ["${CLAUDE_PROJECT_DIR}/.claude/hooks/rsc-app-safety-check.rb"].freeze
...
(entry["hooks"] ||= []) << { "type" => "command", "command" => HOOK_COMMAND, "args" => HOOK_ARGS }
This writes into the app's .claude/settings.json:
{ "type": "command", "command": "ruby", "args": ["${CLAUDE_PROJECT_DIR}/.claude/hooks/rsc-app-safety-check.rb"] }
With args ignored by the hook runner, the registered command is bare ruby — no script — so the safety check never executes and the guardrail is silently inert.
Corroborating evidence in this repo: every hook in our own .claude/settings.json uses a single command string, including the one #4599 just shipped:
{ "type": "command", "command": "${CLAUDE_PROJECT_DIR}/.claude/hooks/rsc-guardrails-check.sh" }
No hook anywhere in the repo uses an args array.
Why CI didn't catch it
react_on_rails/spec/react_on_rails/agent_guardrails_spec.rb pins the buggy shape (lines ~667, ~700, ~717-718, ~731 assert "args" => described_class::HOOK_ARGS). The specs encode the wrong schema, so they pass. Full hosted CI was green on the merged head.
Proposed fix
The template already starts with #!/usr/bin/env ruby and copy_file chmods it 0o755, so it is directly invocable — no ruby prefix needed. Match the repo's own convention:
HOOK_COMMAND = "${CLAUDE_PROJECT_DIR}/.claude/hooks/rsc-app-safety-check.rb"
# drop HOOK_ARGS entirely
...
(entry["hooks"] ||= []) << { "type" => "command", "command" => HOOK_COMMAND }
Then update registered_hook? / managed_hook? (they compare against HOOK_ARGS) and the specs that pin args. A spec asserting the emitted entry has no args key would prevent regression.
Worth adding an end-to-end check that the generated settings.json hook entry actually executes the script, since a schema mismatch here is invisible to unit specs.
Also noted in the same review (minor, same file)
copy_file uses plain File.write while register_hook uses the atomic_write helper; a partial write could leave a corrupt-but-executable hook script. Use atomic_write for consistency.
add_hook strips managed hooks from every PostToolUse entry regardless of matcher, potentially leaving entries with an empty "hooks": [] array behind after an upgrade (cosmetic).
- The hook is registered against the
Edit|Write matcher, so it spawns a fresh ruby process on every edit (JS, CSS, docs, specs), paying interpreter boot cost even though it exits early for irrelevant paths. Consider whether the per-edit tax is acceptable.
Source PR: #4606
Summary
react_on_rails:install_rsc_agent_guardrails(shipped in #4606) registers itsPostToolUsehook using a"command"+"args"array shape that is not part of Claude Code's hook schema. Claude Code hook entries take a singlecommandstring. As written, the installed RSC advisory guardrail very likely never runs in a user's app.Flagged in review on #4606 (thread) with an explicit "please double-check this against Claude Code's actual hook schema before merging" — it merged before that verification happened. My fault as batch coordinator; filing so it isn't lost.
The bug
react_on_rails/lib/react_on_rails/agent_guardrails.rb:This writes into the app's
.claude/settings.json:{ "type": "command", "command": "ruby", "args": ["${CLAUDE_PROJECT_DIR}/.claude/hooks/rsc-app-safety-check.rb"] }With
argsignored by the hook runner, the registered command is bareruby— no script — so the safety check never executes and the guardrail is silently inert.Corroborating evidence in this repo: every hook in our own
.claude/settings.jsonuses a single command string, including the one #4599 just shipped:{ "type": "command", "command": "${CLAUDE_PROJECT_DIR}/.claude/hooks/rsc-guardrails-check.sh" }No hook anywhere in the repo uses an
argsarray.Why CI didn't catch it
react_on_rails/spec/react_on_rails/agent_guardrails_spec.rbpins the buggy shape (lines ~667, ~700, ~717-718, ~731 assert"args" => described_class::HOOK_ARGS). The specs encode the wrong schema, so they pass. Full hosted CI was green on the merged head.Proposed fix
The template already starts with
#!/usr/bin/env rubyandcopy_filechmods it0o755, so it is directly invocable — norubyprefix needed. Match the repo's own convention:Then update
registered_hook?/managed_hook?(they compare againstHOOK_ARGS) and the specs that pinargs. A spec asserting the emitted entry has noargskey would prevent regression.Worth adding an end-to-end check that the generated
settings.jsonhook entry actually executes the script, since a schema mismatch here is invisible to unit specs.Also noted in the same review (minor, same file)
copy_fileuses plainFile.writewhileregister_hookuses theatomic_writehelper; a partial write could leave a corrupt-but-executable hook script. Useatomic_writefor consistency.add_hookstrips managed hooks from everyPostToolUseentry regardless of matcher, potentially leaving entries with an empty"hooks": []array behind after an upgrade (cosmetic).Edit|Writematcher, so it spawns a freshrubyprocess on every edit (JS, CSS, docs, specs), paying interpreter boot cost even though it exits early for irrelevant paths. Consider whether the per-edit tax is acceptable.Source PR: #4606