diff --git a/CHANGELOG.md b/CHANGELOG.md index e10b30b..29d37c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,14 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - **Disclose the global settings merge before it happens (ME-22).** `forge init` now prints the global `~/.claude/settings.json` merge disclosure before the merge mutates the file, not after. +- **Hooks install in exec form (ME-23).** Hooks and the statusline now use Claude Code's exec + form (`"command": "bash", "args": [...]`) instead of a shell string, so a guard path containing + spaces works with no quoting — removing the shell-quoting hazard the earlier RA-12 fix worked + around. Upgrades are seamless: an existing install written in the old shell-string form (or the + `${CLAUDE_PLUGIN_ROOT}` plugin spelling) is deduped by guard identity and healed in place to + exec form on the next merge, and ownership tracking / uninstall are form-agnostic — so a + re-install never duplicates a hook and uninstall still reverses exactly Forge's own additions. + - **Honest positioning (ME-24).** README no longer claims "one brain for every AI coding agent", "enforced guardrails", or that every task passes a deterministic gate: automatic hooks are Claude Code-specific (advisory by default, enforcement opt-in via diff --git a/global/settings.template.json b/global/settings.template.json index 1deeb8c..5f73996 100644 --- a/global/settings.template.json +++ b/global/settings.template.json @@ -5,7 +5,8 @@ "cleanupPeriodDays": 30, "statusLine": { "type": "command", - "command": "bash ~/.forge/statusline.sh" + "command": "bash", + "args": ["~/.forge/statusline.sh"] }, "permissions": { "defaultMode": "default", @@ -76,11 +77,13 @@ "hooks": [ { "type": "command", - "command": "bash ~/.forge/guards/cortex.sh prompt" + "command": "bash", + "args": ["~/.forge/guards/cortex.sh", "prompt"] }, { "type": "command", - "command": "bash ~/.forge/guards/cortex.sh preflight" + "command": "bash", + "args": ["~/.forge/guards/cortex.sh", "preflight"] } ] } @@ -91,11 +94,13 @@ "hooks": [ { "type": "command", - "command": "bash ~/.forge/guards/recall-load.sh" + "command": "bash", + "args": ["~/.forge/guards/recall-load.sh"] }, { "type": "command", - "command": "bash ~/.forge/guards/cortex.sh session-start" + "command": "bash", + "args": ["~/.forge/guards/cortex.sh", "session-start"] } ] } @@ -106,15 +111,18 @@ "hooks": [ { "type": "command", - "command": "bash ~/.forge/guards/protect-paths.sh" + "command": "bash", + "args": ["~/.forge/guards/protect-paths.sh"] }, { "type": "command", - "command": "bash ~/.forge/guards/cost-budget.sh" + "command": "bash", + "args": ["~/.forge/guards/cost-budget.sh"] }, { "type": "command", - "command": "bash ~/.forge/guards/doom-loop.sh" + "command": "bash", + "args": ["~/.forge/guards/doom-loop.sh"] } ] }, @@ -123,7 +131,8 @@ "hooks": [ { "type": "command", - "command": "bash ~/.forge/guards/cortex.sh pre-edit" + "command": "bash", + "args": ["~/.forge/guards/cortex.sh", "pre-edit"] } ] } @@ -134,11 +143,13 @@ "hooks": [ { "type": "command", - "command": "bash ~/.forge/guards/format-on-edit.sh" + "command": "bash", + "args": ["~/.forge/guards/format-on-edit.sh"] }, { "type": "command", - "command": "bash ~/.forge/guards/cortex.sh capture" + "command": "bash", + "args": ["~/.forge/guards/cortex.sh", "capture"] } ] }, @@ -147,11 +158,13 @@ "hooks": [ { "type": "command", - "command": "bash ~/.forge/guards/secret-redact.sh" + "command": "bash", + "args": ["~/.forge/guards/secret-redact.sh"] }, { "type": "command", - "command": "bash ~/.forge/guards/cortex.sh capture" + "command": "bash", + "args": ["~/.forge/guards/cortex.sh", "capture"] } ] } @@ -161,19 +174,23 @@ "hooks": [ { "type": "command", - "command": "bash ~/.forge/guards/completion-gate.sh" + "command": "bash", + "args": ["~/.forge/guards/completion-gate.sh"] }, { "type": "command", - "command": "bash ~/.forge/guards/lean-guard.sh" + "command": "bash", + "args": ["~/.forge/guards/lean-guard.sh"] }, { "type": "command", - "command": "bash ~/.forge/guards/session-learner.sh" + "command": "bash", + "args": ["~/.forge/guards/session-learner.sh"] }, { "type": "command", - "command": "bash ~/.forge/guards/cortex.sh stop" + "command": "bash", + "args": ["~/.forge/guards/cortex.sh", "stop"] } ] } diff --git a/hooks/hooks.json b/hooks/hooks.json index fe5c973..f2b9379 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -6,11 +6,16 @@ "hooks": [ { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/recall-load.sh" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/recall-load.sh"] }, { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/cortex.sh session-start" + "command": "bash", + "args": [ + "${CLAUDE_PLUGIN_ROOT}/global/guards/cortex.sh", + "session-start" + ] } ] } @@ -20,11 +25,16 @@ "hooks": [ { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/cortex.sh prompt" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/cortex.sh", "prompt"] }, { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/cortex.sh preflight" + "command": "bash", + "args": [ + "${CLAUDE_PLUGIN_ROOT}/global/guards/cortex.sh", + "preflight" + ] } ] } @@ -35,15 +45,18 @@ "hooks": [ { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/protect-paths.sh" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/protect-paths.sh"] }, { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/cost-budget.sh" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/cost-budget.sh"] }, { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/doom-loop.sh" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/doom-loop.sh"] } ] }, @@ -52,7 +65,11 @@ "hooks": [ { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/cortex.sh pre-edit" + "command": "bash", + "args": [ + "${CLAUDE_PLUGIN_ROOT}/global/guards/cortex.sh", + "pre-edit" + ] } ] } @@ -63,11 +80,13 @@ "hooks": [ { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/format-on-edit.sh" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/format-on-edit.sh"] }, { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/cortex.sh capture" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/cortex.sh", "capture"] } ] }, @@ -76,11 +95,13 @@ "hooks": [ { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/secret-redact.sh" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/secret-redact.sh"] }, { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/cortex.sh capture" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/cortex.sh", "capture"] } ] } @@ -90,19 +111,23 @@ "hooks": [ { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/completion-gate.sh" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/completion-gate.sh"] }, { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/lean-guard.sh" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/lean-guard.sh"] }, { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/session-learner.sh" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/session-learner.sh"] }, { "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}\"/global/guards/cortex.sh stop" + "command": "bash", + "args": ["${CLAUDE_PLUGIN_ROOT}/global/guards/cortex.sh", "stop"] } ] } diff --git a/src/doctor.js b/src/doctor.js index c703691..37e6321 100644 --- a/src/doctor.js +++ b/src/doctor.js @@ -46,18 +46,20 @@ const readJsonSafe = (p) => { }; // The REQUIRED Forge hook guard identities, derived from the settings template — the SAME -// keys `mergeSettings` installs. `guardKey` normalizes every command (quoting + the legacy -// `~/.forge/` prefix) down to `basename.sh args`, so the template's `~/.forge/guards/cortex.sh -// prompt` and an INSTALLED, path-resolved `bash '/x/global/guards/cortex.sh' prompt` both -// reduce to `cortex.sh prompt` — the raw template compares cleanly against a resolved install -// without re-running resolveManagedPaths. A failed template load returns [] (fail-open). +// keys `mergeSettings` installs. `guardKey` reduces every hook — exec form (`command`+`args`, +// ME-23) or legacy shell string — down to `basename.sh args`, so the template's exec-form +// `{command:"bash", args:["~/.forge/guards/cortex.sh","prompt"]}` and an INSTALLED, path-resolved +// hook (exec form OR a pre-ME-23 `bash '/x/global/guards/cortex.sh' prompt`) both reduce to +// `cortex.sh prompt` — the raw template compares cleanly against a resolved install without +// re-running resolveManagedPaths. A failed template load returns [] (fail-open). function templateGuardKeys() { try { const tpl = readJson(join(BRAND.root, "global", "settings.template.json")); const keys = new Set(); for (const entries of Object.values(tpl.hooks || {})) { for (const entry of Array.isArray(entries) ? entries : []) { - for (const h of entry?.hooks || []) if (h?.command) keys.add(guardKey(h.command)); + for (const h of entry?.hooks || []) + if (h?.command || Array.isArray(h?.args)) keys.add(guardKey(h)); } } return [...keys]; @@ -73,7 +75,7 @@ function installedGuardKeys(hooks) { for (const entries of Object.values(hooks)) { for (const entry of Array.isArray(entries) ? entries : []) { for (const h of entry?.hooks || []) - if (typeof h?.command === "string") keys.add(guardKey(h.command)); + if (typeof h?.command === "string" || Array.isArray(h?.args)) keys.add(guardKey(h)); } } } @@ -261,13 +263,26 @@ function checkLayers(out) { } } -function commandScriptFromPluginRoot(command) { - const marker = '"$' + '{CLAUDE_PLUGIN_ROOT}"/'; - const i = command.indexOf(marker); - if (i === -1) return null; - const rest = command.slice(i + marker.length); - const script = rest.split(/\s+/)[0]?.replace(/^['"]|['"]$/g, ""); - return script || null; +// The guard script a plugin hook references, relative to CLAUDE_PLUGIN_ROOT. Reads BOTH forms: +// an exec-form hook (path in an `args[]` element, ME-23) and a legacy shell-string `command`. +// Quotes are stripped first so `"${CLAUDE_PLUGIN_ROOT}"/…` and the unquoted `${CLAUDE_PLUGIN_ROOT}/…` +// both resolve. +function commandScriptFromPluginRoot(hook) { + // Concatenated so biome's noTemplateCurlyInString doesn't flag this literal search string. + const marker = "$" + "{CLAUDE_PLUGIN_ROOT}/"; + const candidates = []; + if (hook && Array.isArray(hook.args)) for (const a of hook.args) candidates.push(String(a)); + const cmd = typeof hook === "string" ? hook : hook?.command; + if (typeof cmd === "string") candidates.push(cmd); + for (const raw of candidates) { + const c = raw.replace(/["']/g, ""); + const i = c.indexOf(marker); + if (i === -1) continue; + const rest = c.slice(i + marker.length); + const script = rest.split(/\s+/)[0]; + if (script) return script; + } + return null; } // Plugin/hook compatibility: Forge should be additive and self-contained. Claude Code @@ -286,12 +301,11 @@ function checkPluginCompatibility(out) { const commands = Object.values(hooks) .flatMap((entries) => (Array.isArray(entries) ? entries : [])) .flatMap((entry) => (Array.isArray(entry.hooks) ? entry.hooks : [])) - .map((h) => h.command) .filter(Boolean); const missing = []; const notExec = []; - for (const command of commands) { - const rel = commandScriptFromPluginRoot(command); + for (const hook of commands) { + const rel = commandScriptFromPluginRoot(hook); if (!rel) continue; const abs = join(BRAND.root, rel); if (!existsSync(abs)) { diff --git a/src/init.js b/src/init.js index 6b3fef1..22593ff 100644 --- a/src/init.js +++ b/src/init.js @@ -67,39 +67,52 @@ const FORGE_OWNED_KEY = "_forgeOwned"; // ownership/removal purposes (HI-05). const FORGE_GLOBAL = join(BRAND.root, "global"); -/** True iff `command` is a Forge-managed hook/statusline command — i.e. it resolves to a - * path under the installed assets root (`~/.forge/…`, the resolved `/global/…`, quoted - * or not) or the plugin root (`${CLAUDE_PLUGIN_ROOT}/global/…`). A user's same-basename hook - * at a different absolute path is NOT Forge-owned, so it is never blocked on merge or removed - * on uninstall. Path-aware on purpose: `guardKey` (basename+args) is for dedup only. */ -function isForgeCommand(command) { - const c = normalizeCommand(command); +/** True iff `hook` is a Forge-managed hook/statusline entry — i.e. it resolves to a path + * under the installed assets root (`~/.forge/…`, the resolved `/global/…`, quoted or + * not) or the plugin root (`${CLAUDE_PLUGIN_ROOT}/global/…`). Works on BOTH forms: a legacy + * shell-string `command`, and an exec-form entry whose path lives in `args[]`. A user's + * same-basename hook at a different absolute path is NOT Forge-owned, so it is never blocked + * on merge or removed on uninstall. Path-aware on purpose: `guardKey` (basename+args) is for + * dedup only. */ +function isForgeCommand(hook) { + const c = canonicalCommand(hook); return c.includes(`${FORGE_GLOBAL}/`) || c.includes("${CLAUDE_PLUGIN_ROOT}/global/"); } -/** Single-quote a path for safe embedding in a shell command (RA-12): an install - * prefix containing spaces (or any shell metacharacter) must not split into words. - * Embedded single quotes use the standard `'\''` escape. */ +/** Single-quote a path for safe embedding in a shell command: an install prefix containing + * spaces (or any shell metacharacter) must not split into words. Embedded single quotes use + * the standard `'\''` escape. ME-23 moved hooks to exec form (`command`+`args`, spawned with + * NO shell), which removes the need to quote the guard path at all — this helper survives only + * for the defensive legacy shell-string path in `resolveManagedPaths` and for tests that model + * a pre-ME-23 install. */ export function shellQuote(path) { return `'${String(path).replaceAll("'", `'\\''`)}'`; } -/** Rewrite the template's `~/.forge/…` hook + statusline commands to the ACTUAL installed +/** Rewrite the template's `~/.forge/…` hook + statusline entries to the ACTUAL installed * package location. The npm global-install path never creates `~/.forge`, so a literal * `~/.forge/guards/*.sh` reference points at nothing (P0-02). `~/.forge` is the `global/` - * dir (that's what install.sh symlinks), so `~/.forge/X` resolves to `/global/X` — - * single-quoted, so an install path with spaces still runs (RA-12). Trailing args stay - * outside the quotes: `bash '/guards/cortex.sh' prompt`. */ + * dir (that's what install.sh symlinks), so `~/.forge/X` resolves to `/global/X`. + * In exec form (ME-23) the path is its OWN `args[]` element and is rewritten UNQUOTED — the + * hook is spawned directly with no shell, so an install path containing spaces just works + * with no quoting. A legacy shell-string `command` (should no longer appear in the template, + * but handled defensively) keeps the single-quoting a real shell would still need. */ function resolveManagedPaths(template) { const base = join(BRAND.root, "global"); - const fix = (cmd) => + const fixStr = (cmd) => typeof cmd === "string" ? cmd.replace(/~\/\.forge\/(\S+)/g, (_, rest) => shellQuote(join(base, rest))) : cmd; - if (template.statusLine?.command) template.statusLine.command = fix(template.statusLine.command); + const fixArg = (a) => + typeof a === "string" ? a.replace(/^~\/\.forge\/(.+)$/, (_, rest) => join(base, rest)) : a; + const fixEntry = (h) => { + if (h && Array.isArray(h.args)) h.args = h.args.map(fixArg); + else if (h && typeof h.command === "string") h.command = fixStr(h.command); + }; + if (template.statusLine) fixEntry(template.statusLine); for (const entries of Object.values(template.hooks || {})) { for (const entry of entries) { - for (const h of entry.hooks || []) h.command = fix(h.command); + for (const h of entry.hooks || []) fixEntry(h); } } return template; @@ -117,6 +130,21 @@ function normalizeCommand(command) { .replaceAll("~/.forge/", `${join(BRAND.root, "global")}/`); } +/** Reduce ANY hook/statusLine entry to one comparable command string, quote-normalized and + * with the legacy `~/.forge/` prefix resolved, so the SAME guard compares equal across forms: + * - legacy shell-string: `{command:"bash /x/guards/cortex.sh prompt"}` + * - exec form (ME-23): `{command:"bash", args:["/x/guards/cortex.sh","prompt"]}` + * In exec form the `args[]` are joined onto `command` before normalizing. A bare string is + * accepted too. Used for ownership/removal matching (identity of the WHOLE command, not just + * the basename — that is `guardKey`'s job). */ +function canonicalCommand(hook) { + if (hook && typeof hook === "object") { + if (Array.isArray(hook.args)) return normalizeCommand([hook.command, ...hook.args].join(" ")); + return normalizeCommand(hook.command); + } + return normalizeCommand(hook); +} + function loadTemplate() { const path = join(BRAND.root, "global", "settings.template.json"); return resolveManagedPaths(JSON.parse(readFileSync(path, "utf8"))); @@ -156,58 +184,87 @@ function unionStrings(a = [], b = []) { return [...set]; } -/** Extract guard identity (basename + trailing args) from a hook command for dedup. - * Quote-normalized (RA-12), so all of `bash ~/.forge/guards/cortex.sh prompt`, - * `bash '/install path/global/guards/cortex.sh' prompt`, and - * `"${CLAUDE_PLUGIN_ROOT}"/global/guards/cortex.sh prompt` → `cortex.sh prompt` — - * old unquoted installed entries and new quoted template entries must dedupe, or - * every existing install would grow duplicate hooks on re-merge. Exported for tests. */ -export function guardKey(command) { - const cmd = normalizeCommand(command); +/** Extract guard identity (`basename.sh` + trailing args) from a hook for dedup/ownership. + * THE CRUX of ME-23: it must derive the SAME key from BOTH forms of the same guard — + * - legacy shell-string: `bash /x/guards/cortex.sh prompt` → `cortex.sh prompt` + * - exec form: `{command:"bash", args:["/x/guards/cortex.sh","prompt"]}` → `cortex.sh prompt` + * so a user carrying the OLD shell-string install and a re-merge with the NEW exec-form + * template dedupe to ONE entry instead of growing a duplicate. For the legacy string it is + * quote-normalized (RA-12), collapsing quoted / unquoted / `~/.forge` / plugin-root spellings. + * For exec form it takes the basename of the `args[]` element that holds the `.sh` path and + * appends the remaining args. Accepts a whole hook object (preferred) or a bare command string + * (back-compat). Exported for tests + doctor. */ +export function guardKey(hook) { + // Exec form: the script path is an `args[]` element; the elements after it are its arguments. + if (hook && typeof hook === "object" && Array.isArray(hook.args)) { + const args = hook.args.map((a) => String(a)); + const i = args.findIndex((a) => a.includes(".sh")); + if (i >= 0) { + const base = args[i].split(/[/\\]/).pop(); + const rest = args + .slice(i + 1) + .join(" ") + .trim(); + return rest ? `${base} ${rest}` : base; + } + return canonicalCommand(hook); // no script arg — fall back to the whole invocation + } + // Legacy shell-string, or an object carrying only `.command`. + const cmd = normalizeCommand(hook && typeof hook === "object" ? hook.command : hook); const m = cmd.match(/([^/\\]+\.sh)\s*(.*)/); return m ? `${m[1]} ${m[2]}`.trim() : cmd; } /** Merge Forge hook entries into existing hook arrays, matching by guard identity to avoid - * duplicates. An existing entry that is the SAME resolved command as the template's — just - * spelled without quotes (a pre-RA-12 install) — is upgraded in place to the quoted form. - * A template hook counts as "already installed" only when an existing FORGE-owned entry (some - * path spelling of it) shares its guard key: a user's same-basename hook at a DIFFERENT path - * must NOT block Forge's own install (HI-05). Returns the merged tree plus the guard identities - * actually added this merge (`added: [{event, key, command}]`) so the ownership manifest can - * reverse exactly them later. */ + * duplicates. An existing entry that is the SAME resolved command as the template's — spelled + * as a legacy shell string (a pre-ME-23 install) or differently quoted — is upgraded in place + * to the exec form (`command`+`args`). A template hook counts as "already installed" only when + * an existing FORGE-owned entry (some path/form spelling of it) shares its guard key: a user's + * same-basename hook at a DIFFERENT path must NOT block Forge's own install (HI-05). Returns the + * merged tree plus the guard identities actually added this merge (`added: [{event, key, + * command, args?}]`) so the ownership manifest can reverse exactly them later. */ function mergeHooks(existing = {}, template = {}) { const merged = { ...existing }; - /** @type {{event:string, key:string, command:string}[]} */ + /** @type {{event:string, key:string, command:string, args?:string[]}[]} */ const added = []; for (const [event, entries] of Object.entries(template)) { const existingEntries = merged[event] || []; - // guardKey → template command, to heal old unquoted spellings of the same command. + // guardKey → template hook, to heal legacy shell-string spellings of the same guard. const templateByKey = new Map(); for (const entry of entries) { - for (const h of entry.hooks || []) { - if (h.command) templateByKey.set(guardKey(h.command), h.command); - } + for (const h of entry.hooks || []) templateByKey.set(guardKey(h), h); } + // Upgrade an existing FORGE-owned entry that is the SAME resolved command as the template's + // — a legacy shell string (pre-ME-23) or a differently-quoted spelling — in place to the exec + // form. A user's same-basename hook at a DIFFERENT path has a different canonical command, so + // it is never touched. for (const entry of existingEntries) { for (const h of entry.hooks || []) { - if (!h.command) continue; - const tpl = templateByKey.get(guardKey(h.command)); - if (tpl && tpl !== h.command && normalizeCommand(h.command) === normalizeCommand(tpl)) { - h.command = tpl; // same command, pre-quoting spelling → upgrade to the quoted form + if (h.command == null && !Array.isArray(h.args)) continue; + const tpl = templateByKey.get(guardKey(h)); + if (tpl && canonicalCommand(h) === canonicalCommand(tpl)) { + h.command = tpl.command; + if (Array.isArray(tpl.args)) h.args = [...tpl.args]; + else delete h.args; } } } const presentForgeKeys = new Set( existingEntries - .flatMap((e) => (e.hooks || []).map((h) => h.command)) - .filter((c) => typeof c === "string" && isForgeCommand(c)) - .map(guardKey), + .flatMap((e) => e.hooks || []) + .filter((h) => isForgeCommand(h)) + .map((h) => guardKey(h)), ); const newEntries = []; for (const entry of entries) { - const hooks = (entry.hooks || []).filter((h) => !presentForgeKeys.has(guardKey(h.command))); - for (const h of hooks) added.push({ event, key: guardKey(h.command), command: h.command }); + const hooks = (entry.hooks || []).filter((h) => !presentForgeKeys.has(guardKey(h))); + for (const h of hooks) + added.push({ + event, + key: guardKey(h), + command: h.command, + args: Array.isArray(h.args) ? [...h.args] : undefined, + }); if (hooks.length) { newEntries.push({ ...entry, hooks }); } @@ -227,7 +284,7 @@ function mergeHooks(existing = {}, template = {}) { function legacyOwnedScan(settings, template) { const owned = { permissions: { allow: [], ask: [], deny: [] }, - /** @type {{event:string, key:string, command:string}[]} */ + /** @type {{event:string, key:string, command:string, args?:string[]}[]} */ hooks: [], statusLine: false, schema: false, @@ -243,29 +300,30 @@ function legacyOwnedScan(settings, template) { } const templateKeys = new Set(); for (const entries of Object.values(template.hooks || {})) - for (const entry of entries) - for (const h of entry.hooks || []) if (h.command) templateKeys.add(guardKey(h.command)); + for (const entry of entries) for (const h of entry.hooks || []) templateKeys.add(guardKey(h)); if (settings.hooks && typeof settings.hooks === "object") { for (const [event, entries] of Object.entries(settings.hooks)) { if (!Array.isArray(entries)) continue; for (const entry of entries) for (const h of entry?.hooks || []) if ( - typeof h?.command === "string" && - isForgeCommand(h.command) && - templateKeys.has(guardKey(h.command)) + h && + (typeof h.command === "string" || Array.isArray(h.args)) && + isForgeCommand(h) && + templateKeys.has(guardKey(h)) ) owned.hooks.push({ event, - key: guardKey(h.command), + key: guardKey(h), command: h.command, + args: Array.isArray(h.args) ? [...h.args] : undefined, }); } } if ( settings.statusLine?.command && template.statusLine?.command && - normalizeCommand(settings.statusLine.command) === normalizeCommand(template.statusLine.command) + canonicalCommand(settings.statusLine) === canonicalCommand(template.statusLine) ) owned.statusLine = true; if (settings.$schema && settings.$schema === template.$schema) owned.schema = true; @@ -327,9 +385,10 @@ export function mergeSettings({ settingsPath, noSettings, onNotice } = {}) { const beforeHooks = JSON.stringify(existing.hooks || {}); const { merged, added } = mergeHooks(existing.hooks, template.hooks); existing.hooks = merged; + // Dedup by event+key: in exec form `command` is always "bash", so a hook's identity is its + // guard key (basename+args), not the command string. for (const a of added) - if (!ownedHooks.some((o) => o.event === a.event && o.command === a.command)) - ownedHooks.push(a); + if (!ownedHooks.some((o) => o.event === a.event && o.key === a.key)) ownedHooks.push(a); if (JSON.stringify(existing.hooks) !== beforeHooks) report.added.push("hooks"); else report.unchanged.push("hooks"); } @@ -469,11 +528,11 @@ export function removeForgeSettings({ settingsPath } = {}) { ask: new Set(owned.permissions?.ask || []), deny: new Set(owned.permissions?.deny || []), }; - /** @type {Map>} event → normalized commands Forge owns */ + /** @type {Map>} event → canonical commands Forge owns (either form) */ const ownedHookCmds = new Map(); for (const h of owned.hooks || []) { if (!ownedHookCmds.has(h.event)) ownedHookCmds.set(h.event, new Set()); - ownedHookCmds.get(h.event).add(normalizeCommand(h.command)); + ownedHookCmds.get(h.event).add(canonicalCommand(h)); } // Drop the statusLine/$schema only if Forge set it AND it is still the template's value — // if the user replaced it after install, it is theirs now (leave it). @@ -481,11 +540,12 @@ export function removeForgeSettings({ settingsPath } = {}) { Boolean(owned.statusLine) && Boolean(settings.statusLine?.command) && Boolean(template.statusLine?.command) && - normalizeCommand(settings.statusLine.command) === normalizeCommand(template.statusLine.command); + canonicalCommand(settings.statusLine) === canonicalCommand(template.statusLine); const dropSchema = Boolean(owned.schema) && Boolean(settings.$schema) && settings.$schema === template.$schema; - // Hooks: drop only the exact commands Forge added (matched quote-normalized within the event). + // Hooks: drop only the exact commands Forge added (matched canonically within the event, so a + // manifest entry recorded in either form removes the installed hook in either form). if (settings.hooks && typeof settings.hooks === "object") { for (const [event, entries] of Object.entries(settings.hooks)) { if (!Array.isArray(entries)) continue; @@ -498,9 +558,7 @@ export function removeForgeSettings({ settingsPath } = {}) { kept.push(entry); continue; } - const hooks = entry.hooks.filter( - (h) => !(typeof h?.command === "string" && ownedCmds.has(normalizeCommand(h.command))), - ); + const hooks = entry.hooks.filter((h) => !(h && ownedCmds.has(canonicalCommand(h)))); if (hooks.length !== entry.hooks.length) changed = true; if (hooks.length) kept.push({ ...entry, hooks }); } diff --git a/test/init.test.js b/test/init.test.js index 2e76199..75ffe13 100644 --- a/test/init.test.js +++ b/test/init.test.js @@ -25,6 +25,11 @@ import { } from "../src/init.js"; import { GITATTRIBUTES_RULE } from "../src/ledger_store.js"; +/** The effective shell command a hook/statusLine entry runs, across BOTH forms: exec form + * (`command`+`args`, ME-23) joins the args onto the command; a legacy shell string is returned + * as-is. Tests assert on this so they are form-agnostic. */ +const effectiveCmd = (h) => (Array.isArray(h.args) ? [h.command, ...h.args].join(" ") : h.command); + test("init emits the shared config for a fresh repo in one call", () => { const root = mkdtempSync(join(tmpdir(), "forge-init-")); init({ targetRoot: root }); @@ -71,7 +76,7 @@ test("mergeSettings deduplicates plugin-style and settings-style hooks", () => { mergeSettings({ settingsPath }); const result = JSON.parse(readFileSync(settingsPath, "utf8")); const promptHooks = result.hooks.UserPromptSubmit.flatMap((e) => - (e.hooks || []).map((h) => h.command), + (e.hooks || []).map(effectiveCmd), ); const cortexPromptCount = promptHooks.filter((c) => c.includes("cortex.sh prompt")).length; assert.equal( @@ -79,7 +84,7 @@ test("mergeSettings deduplicates plugin-style and settings-style hooks", () => { 1, "cortex.sh prompt must not duplicate across plugin + settings paths", ); - const stopHooks = result.hooks.Stop.flatMap((e) => (e.hooks || []).map((h) => h.command)); + const stopHooks = result.hooks.Stop.flatMap((e) => (e.hooks || []).map(effectiveCmd)); const leanCount = stopHooks.filter((c) => c.includes("lean-guard.sh")).length; assert.equal(leanCount, 1, "lean-guard.sh must not duplicate"); }); @@ -187,7 +192,7 @@ test("mergeSettings backs up an existing valid file and resolves guard paths abs assert.ok(result.backup && existsSync(result.backup), "a timestamped backup was written"); const merged = JSON.parse(readFileSync(settingsPath, "utf8")); const cmds = (merged.hooks?.UserPromptSubmit || []).flatMap((e) => - (e.hooks || []).map((h) => h.command), + (e.hooks || []).map(effectiveCmd), ); assert.ok( cmds.some((c) => c.includes("/global/guards/") && !c.includes("~/.forge/")), @@ -284,48 +289,104 @@ test("catalog indexes tools (with a why), crew, and guards", () => { }); // --------------------------------------------------------------------------- -// RA-12 — quoted hook paths +// ME-23 — exec-form hooks (command + args, no shell quoting) + legacy dedup // --------------------------------------------------------------------------- test("shellQuote single-quotes paths (spaces safe) and escapes embedded quotes", () => { + // Retained for the defensive legacy shell-string path; exec form no longer needs it. assert.equal(shellQuote("/a b/guards/x.sh"), "'/a b/guards/x.sh'"); assert.equal(shellQuote("/o'brien/x.sh"), "'/o'\\''brien/x.sh'"); }); -test("guardKey normalizes quoting: quoted, unquoted, tilde, and plugin-root spellings all match", () => { +test("guardKey derives ONE identity from legacy shell-string AND exec-form spellings (ME-23)", () => { const base = join(BRAND.root, "global"); - const key = guardKey(`bash ~/.forge/guards/cortex.sh prompt`); - assert.equal(key, "cortex.sh prompt"); + const key = "cortex.sh prompt"; + // Legacy shell-string spellings (quoted, unquoted, tilde, plugin-root). + assert.equal(guardKey(`bash ~/.forge/guards/cortex.sh prompt`), key); assert.equal(guardKey(`bash '${base}/guards/cortex.sh' prompt`), key); assert.equal(guardKey(`bash ${base}/guards/cortex.sh prompt`), key); assert.equal(guardKey(`bash '/a b/with space/guards/cortex.sh' prompt`), key); assert.equal(guardKey('"${CLAUDE_PLUGIN_ROOT}"/global/guards/cortex.sh prompt'), key); + // Exec-form objects with the same basename+args reduce to the SAME key. + assert.equal( + guardKey({ + command: "bash", + args: ["~/.forge/guards/cortex.sh", "prompt"], + }), + key, + ); + assert.equal(guardKey({ command: "bash", args: [`${base}/guards/cortex.sh`, "prompt"] }), key); + assert.equal( + guardKey({ + command: "bash", + args: ["/a b/with space/guards/cortex.sh", "prompt"], + }), + key, + ); + assert.equal( + guardKey({ + command: "bash", + args: ["${CLAUDE_PLUGIN_ROOT}/global/guards/cortex.sh", "prompt"], + }), + key, + ); + // A hook object carrying only a legacy `.command` string is accepted too. + assert.equal(guardKey({ command: `bash ${base}/guards/cortex.sh prompt` }), key); // Args are part of identity: `cortex.sh prompt` and `cortex.sh stop` differ. - assert.notEqual(guardKey("bash ~/.forge/guards/cortex.sh stop"), key); + assert.notEqual(guardKey({ command: "bash", args: ["~/.forge/guards/cortex.sh", "stop"] }), key); }); -test("mergeSettings resolves hook paths QUOTED so an install path with spaces survives", () => { - const tmp = mkdtempSync(join(tmpdir(), "forge-quoted-")); +test("mergeSettings writes hooks in EXEC form — path in args[], resolved, UNQUOTED (ME-23)", () => { + const tmp = mkdtempSync(join(tmpdir(), "forge-execform-")); const settingsPath = join(tmp, "settings.json"); mergeSettings({ settingsPath }); const merged = JSON.parse(readFileSync(settingsPath, "utf8")); const base = join(BRAND.root, "global"); - const cmds = Object.values(merged.hooks) + const hooks = Object.values(merged.hooks) .flat() - .flatMap((e) => (e.hooks || []).map((h) => h.command)); - assert.ok(cmds.length > 0, "hooks merged"); - for (const c of cmds) { - assert.match(c, /'[^']*\.sh'/, `hook path is single-quoted: ${c}`); - assert.ok(c.includes(`'${base}/`), `resolved to the quoted install base: ${c}`); + .flatMap((e) => e.hooks || []); + assert.ok(hooks.length > 0, "hooks merged"); + for (const h of hooks) { + assert.equal(h.command, "bash", "command is the bare interpreter, no path baked in"); + assert.ok(Array.isArray(h.args) && h.args.length > 0, "path lives in args[]"); + const path = h.args[0]; + // Resolved to the real install base, NOT the unmaterialized ~/.forge, and — the whole point + // of exec form — carrying NO surrounding quote characters (spaces would just work). + assert.ok(path.startsWith(`${base}/`), `args path resolved to the install base: ${path}`); + assert.ok(!path.includes("~/.forge/"), "no unresolved ~/.forge in args path"); + assert.doesNotMatch(path, /['"]/, `exec-form args path is unquoted: ${path}`); } - assert.match(merged.statusLine.command, /'[^']*\.sh'/, "statusline path quoted too"); + // The statusLine migrated to exec form too. + assert.equal(merged.statusLine.command, "bash"); + assert.equal(merged.statusLine.args[0], join(base, "statusline.sh")); + assert.doesNotMatch(merged.statusLine.args[0], /['"]/, "statusline args path unquoted"); +}); + +test("resolveManagedPaths: a base path WITH A SPACE lands literally in args, no quoting (ME-23)", () => { + // The exec-form guarantee: the args element is the LITERAL path even with a space — nothing + // to escape because the hook is spawned directly with no shell. We assert the shape a merge + // produces (args[0] === the exact join()ed path) so a space in BRAND.root could never break it. + const tmp = mkdtempSync(join(tmpdir(), "forge-space-")); + const settingsPath = join(tmp, "settings.json"); + mergeSettings({ settingsPath }); + const merged = JSON.parse(readFileSync(settingsPath, "utf8")); + const base = join(BRAND.root, "global"); + const cortexPrompt = merged.hooks.UserPromptSubmit.flatMap((e) => e.hooks || []).find( + (h) => guardKey(h) === "cortex.sh prompt", + ); + assert.ok(cortexPrompt, "cortex prompt hook present"); + assert.deepEqual( + cortexPrompt.args, + [join(base, "guards", "cortex.sh"), "prompt"], + "args are the literal resolved path element + trailing arg, verbatim and unquoted", + ); }); -test("old UNQUOTED installed hook entries dedupe against the new quoted template and are healed", () => { - const tmp = mkdtempSync(join(tmpdir(), "forge-requote-")); +test("an OLD shell-string install dedupes against the exec-form template and is upgraded (ME-23)", () => { + const tmp = mkdtempSync(join(tmpdir(), "forge-legacy-dedup-")); const settingsPath = join(tmp, "settings.json"); const base = join(BRAND.root, "global"); - // What a pre-RA-12 install wrote: resolved absolute path, no quotes. + // What a pre-ME-23 install wrote: one resolved shell-string command (RA-12 quoted form). writeFileSync( settingsPath, JSON.stringify({ @@ -335,7 +396,7 @@ test("old UNQUOTED installed hook entries dedupe against the new quoted template hooks: [ { type: "command", - command: `bash ${base}/guards/cortex.sh prompt`, + command: `bash ${shellQuote(`${base}/guards/cortex.sh`)} prompt`, }, ], }, @@ -345,15 +406,17 @@ test("old UNQUOTED installed hook entries dedupe against the new quoted template ); mergeSettings({ settingsPath }); const merged = JSON.parse(readFileSync(settingsPath, "utf8")); - const promptCmds = merged.hooks.UserPromptSubmit.flatMap((e) => - (e.hooks || []).map((h) => h.command), - ).filter((c) => c.includes("cortex.sh") && c.endsWith("prompt")); - assert.equal(promptCmds.length, 1, "no duplicate on re-merge with the quoted template"); + const promptHooks = merged.hooks.UserPromptSubmit.flatMap((e) => e.hooks || []).filter( + (h) => guardKey(h) === "cortex.sh prompt", + ); assert.equal( - promptCmds[0], - `bash ${shellQuote(`${base}/guards/cortex.sh`)} prompt`, - "the old unquoted entry is upgraded to the quoted spelling", + promptHooks.length, + 1, + "no duplicate — the legacy hook deduped against the template", ); + // ...and it was upgraded IN PLACE to exec form (command + resolved, unquoted args). + assert.equal(promptHooks[0].command, "bash"); + assert.deepEqual(promptHooks[0].args, [`${base}/guards/cortex.sh`, "prompt"]); }); // --------------------------------------------------------------------------- @@ -523,7 +586,7 @@ test("HI-05 ownership round-trip: user-owned entries that collide with the templ assert.equal(afterMerge._forgeOwned.added.schema, false, "Forge did not set $schema"); // Forge DID add its own cortex.sh prompt hook even though the user has a same-name one. const promptCmds = afterMerge.hooks.UserPromptSubmit.flatMap((e) => - (e.hooks || []).map((h) => h.command), + (e.hooks || []).map(effectiveCmd), ).filter((c) => c.includes("cortex.sh") && c.endsWith("prompt")); assert.equal(promptCmds.length, 2, "user's and Forge's cortex hooks coexist (different paths)"); @@ -568,7 +631,7 @@ test("HI-05 re-merge is idempotent: the manifest and entries are stable, no dupl const afterSecond = JSON.parse(readFileSync(settingsPath, "utf8")); assert.deepEqual(afterSecond._forgeOwned, first._forgeOwned, "manifest is byte-stable"); const promptCmds = afterSecond.hooks.UserPromptSubmit.flatMap((e) => - (e.hooks || []).map((h) => h.command), + (e.hooks || []).map(effectiveCmd), ); assert.equal( promptCmds.filter((c) => c.includes("cortex.sh") && c.endsWith("prompt")).length, diff --git a/test/settings_template.test.js b/test/settings_template.test.js index d0a76de..fa1c852 100644 --- a/test/settings_template.test.js +++ b/test/settings_template.test.js @@ -68,11 +68,14 @@ const pluginHooks = JSON.parse( readFileSync(new URL("../hooks/hooks.json", import.meta.url), "utf8"), ); +/** The effective command a hook runs — exec form (`command`+`args`, ME-23) or legacy string. */ +const hookText = (h) => [h.command ?? "", ...(h.args ?? [])].join(" "); + /** Matcher of the first hook group under `event` whose command mentions `guard`. */ function matcherFor(manifest, event, guard) { const groups = manifest.hooks?.[event] ?? []; for (const group of groups) { - if ((group.hooks ?? []).some((h) => (h.command ?? "").includes(guard))) { + if ((group.hooks ?? []).some((h) => hookText(h).includes(guard))) { return group.matcher ?? ""; } } diff --git a/test/stop_gate.test.js b/test/stop_gate.test.js index 6f5a527..c193957 100644 --- a/test/stop_gate.test.js +++ b/test/stop_gate.test.js @@ -214,7 +214,10 @@ test("both hook manifests register the completion gate under Stop (lockstep)", ( const repo = join(dirname(fileURLToPath(import.meta.url)), ".."); const plugin = JSON.parse(readFileSync(join(repo, "hooks", "hooks.json"), "utf8")); const template = JSON.parse(readFileSync(join(repo, "global", "settings.template.json"), "utf8")); - const flat = (m) => (m.hooks.Stop || []).flatMap((e) => e.hooks.map((h) => h.command)).join("\n"); + const flat = (m) => + (m.hooks.Stop || []) + .flatMap((e) => e.hooks.map((h) => [h.command, ...(h.args ?? [])].join(" "))) + .join("\n"); assert.match(flat(plugin), /completion-gate\.sh/, "plugin manifest wires the gate"); assert.match(flat(template), /completion-gate\.sh/, "init template wires the gate"); });