Skip to content

Follow-up: install_rsc_agent_guardrails registers a PostToolUse hook with an unsupported "args" array — installed guardrail likely never runs #4753

Description

@justin808

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions