Skip to content

refactor(hooks): migrate hooks to exec form (command+args) — ME-23#89

Merged
CodeWithJuber merged 2 commits into
masterfrom
claude/parallel-solving-3dc0ji
Jul 19, 2026
Merged

refactor(hooks): migrate hooks to exec form (command+args) — ME-23#89
CodeWithJuber merged 2 commits into
masterfrom
claude/parallel-solving-3dc0ji

Conversation

@CodeWithJuber

Copy link
Copy Markdown
Owner

What & why

Closes ME-23, the last code-addressable finding from the v0.22.1 re-audit (it was deferred out of PR #88 as needing its own careful pass).

Forge's hooks and statusline were installed in Claude Code's shell-string form ("command": "bash ~/.forge/guards/X.sh arg"), which is why the earlier RA-12 fix had to shell-quote the guard path to survive install prefixes containing spaces. This PR migrates them to the exec form ("command": "bash", "args": ["<path>", "arg"]), which Claude Code spawns directly with no shell — so paths with spaces or metacharacters just work, with no quoting needed.

Contract verified first. Before touching anything I confirmed against the official Claude Code hooks reference that the exec-form args array is part of the real hook schema (when args is present, command is spawned directly, no shell) — a migration to a non-existent field would have silently disabled every guard while JSON-shape tests stayed green. It's supported, and the docs specifically recommend exec form for path placeholders.

The hard part — backward compatibility. An existing install carries the OLD shell-string hooks in the user's settings.json. The dedup/ownership machinery (guardKey, canonicalCommand, isForgeCommand, the _forgeOwned manifest from HI-05) is now form-agnostic: both {command:"bash", args:["/x/guards/cortex.sh","prompt"]} and the legacy bash '/x/guards/cortex.sh' prompt (and the ${CLAUDE_PLUGIN_ROOT} plugin spelling) collapse to the same identity cortex.sh prompt. So on the next merge an old install is deduped and healed in place to exec form — never duplicated — and uninstall still reverses exactly Forge's own additions while leaving a user's same-basename hook at a different path byte-identical.

Files: global/settings.template.json, hooks/hooks.json (both manifests → exec form), src/init.js (guardKey/canonicalCommand/isForgeCommand/resolveManagedPaths/mergeHooks/removeForgeSettings), src/doctor.js (form-agnostic guard-key checks), plus tests.

Integrated branch: 1057 tests / 0 failures, Biome + tsc + forge docs check green, forge verify PASS.

Checklist

  • npm test passes (1057 tests, 0 fail)
  • npm run check passes (Biome)
  • New public functions have a test (guardKey across both forms; legacy→exec dedup; ownership round-trip)
  • Conventional commit message (refactor:)
  • CHANGELOG.md updated under ## [Unreleased]
  • No new runtime dependency
  • Substrate/docs updated (forge docs check green)

Risk & rollback

  • Risk level: medium — touches the hook merge/ownership logic from HI-05/RA-12. Mitigated by the form-agnostic identity being unit-tested and the legacy-install dedup being an explicit regression test (old shell-string + ${CLAUDE_PLUGIN_ROOT} spelling both dedup to count 1). Behavior-preserving: same guards, same matchers, just exec form.
  • Rollback plan: revert the single refactor commit.

Extra checks (tick if applicable)

  • npm run typecheck passes
  • Input validated at boundaries; errors handled
  • Logs contain no secrets/PII
  • If AI-assisted: I understand it, verified the package API (against the official hooks reference), and it has tests

🤖 Generated with Claude Code

https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6


Generated by Claude Code

claude added 2 commits July 19, 2026 13:31
…s), dedup across legacy shell-string installs (ME-23)

Move Forge's hooks and statusline from shell-string form
("command": "bash ~/.forge/guards/X.sh arg") to exec form
("command": "bash", "args": ["~/.forge/guards/X.sh", "arg"]) in both
manifests (global/settings.template.json and hooks/hooks.json). When args
is present Claude Code spawns the command directly with no shell, so the
guard path needs no quoting — this eliminates the shell-quoting hazard the
RA-12 fix worked around (a path with spaces now just works).

The merge/ownership machinery in src/init.js is made form-agnostic and
backward-compatible:
- guardKey(h) derives ONE identity (basename.sh + trailing args) from BOTH
  a legacy shell-string command and an exec-form {command,args} hook, so an
  old install and the new template dedupe to a single entry.
- canonicalCommand(h) reduces either form to one comparable command string
  for ownership/removal matching; isForgeCommand(h) is path-aware across
  forms so a user's different-path hook is never claimed.
- mergeHooks upgrades an existing Forge-owned legacy shell-string hook in
  place to exec form and never duplicates; the _forgeOwned manifest records
  exec-form identities and removeForgeSettings reverses exactly them.
- resolveManagedPaths rewrites ~/.forge in the args element unquoted (keeps
  the defensive quoted path only for a stray legacy string command).

doctor.js (checkSettings guard keys, plugin-compat script extraction) reads
both forms. shellQuote is retained for the legacy path + tests. Tests updated
to the exec-form shape with added coverage for cross-form guardKey identity,
unquoted args, and legacy-install dedup/upgrade.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
@CodeWithJuber
CodeWithJuber marked this pull request as ready for review July 19, 2026 13:44
@CodeWithJuber
CodeWithJuber merged commit 52c61c8 into master Jul 19, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants