Add scripts/install.sh: reproducibly generate the /faber command with a repo-derived path

[yihanzhu]

Closes #10

What changed

• scripts/install.sh (new) — idempotent installer that generates ~/.claude/commands/faber.md. It resolves the control-plane repo root from the script's own location (symlink-aware: follows readlink then cd … && pwd -P), renders the new template by substituting the resolved absolute path for {{FABRICA_ROOT}}, creates ~/.claude/commands/ if missing, and prints next steps in the same style as setup-target-repo.sh.
• templates/faber-command.md (new) — the /faber command body, now version-controlled, with a {{FABRICA_ROOT}} placeholder. It is faithful to the current live command; only the hardcoded ~/git/fabrica paths were replaced with the placeholder (verified byte-identical when substituted back).
• RESTORE.md — section 1 ("Recreate Faber") now recreates the /faber command by running scripts/install.sh instead of by hand. (The runbook previously didn't cover the slash command at all — the exact gap the issue flags.)
• README.md — Layout block lists scripts/install.sh and templates/faber-command.md, one line each.

Why

The /faber command was made by hand and hardcoded ~/git/fabrica in ~6 places — not reproducible (reusable-by-anyone) and not restorable (full-backup). Deriving the path from the clone's own location fixes both.

Idempotency / safety

• Re-running with identical content is a no-op ("Already up to date").
• An existing faber.md that differs is backed up to faber.md.bak before overwriting — never clobbered silently.
• No ~/git/fabrica literal and no {{FABRICA_ROOT}} placeholder in the generated output.

How tested (sandboxed HOME — the real ~/.claude/commands/faber.md was never touched)

tmphome="$(mktemp -d)"
HOME="$tmphome" bash scripts/install.sh   # run 1 → Created
HOME="$tmphome" bash scripts/install.sh   # run 2 → Already up to date (idempotent)

Evidence:

• Run 1: Created: $tmphome/.claude/commands/faber.md, repo path derived to /Users/<me>/git/fabrica-i10.
• Run 2: Already up to date — idempotent, no .bak written when content is identical.
• Generated file assertions: ✅ no {{FABRICA_ROOT}} placeholder · ✅ no ~/git/fabrica literal · ✅ resolved repo path present.
• Separate test against a differing existing faber.md: ✅ old content backed up to faber.md.bak, new file carries the resolved path.
• Confirmed the real ~/.claude/commands/faber.md was not modified and no real .bak was created. Temp dirs cleaned up.

CI

• shellcheck clean across install.sh, setup-target-repo.sh, codex-review.sh.
• Structure check (required-files list) passes — no required files removed.

🤖 Generated with Claude Code

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The installer can generate invalid output or fail for valid clone paths containing sed metacharacters, which undermines the repo-derived path behavior this patch adds.

Review comment:

• [P2] Escape the repo path before substituting it with sed — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.0LF6vy0GkP/scripts/install.sh:35-35
  When the clone path contains sed replacement metacharacters such as & or the # delimiter, this line does not insert the literal path: & expands back to {{FABRICA_ROOT}}, and # terminates the sed expression, so valid locations like /Users/me/R&D#fabrica produce an unusable command or make install fail. Escape the replacement, or render without sed replacement parsing, before writing faber.md.

[yihanzhu]

Round 1 — addressed Codex [P2]

Finding: placeholder substitution used sed with a # delimiter, which breaks for valid clone paths containing sed metacharacters — & re-expands to the matched text and # terminates the expression (e.g. /Users/me/R&D#fabrica).

Fix: dropped sed in favor of bash literal string replacement:

template_contents="$(cat "$template")"
rendered="${template_contents//'{{FABRICA_ROOT}}'/$repo_root}"

Bash ${var//pattern/replacement} treats the replacement literally, so &, #, /, and spaces all substitute correctly. The {{FABRICA_ROOT}} placeholder name is unchanged.

Behavior preserved: symlink-aware repo path, template sourced from templates/faber-command.md, idempotent re-runs, .bak backup on diff, next-steps output, no ~/git/fabrica literal, executable bit intact.

Verification (sandboxed HOME, real ~/.claude/commands/faber.md untouched):

• Ran install twice under a temp HOME → Created then Already up to date, no spurious .bak.
• Ran install from a dir whose resolved path is …/R&D#fab rica (contains &, #, /, space) → the literal path landed correctly in faber.md, no leftover placeholder.
• shellcheck scripts/install.sh clean; CI structure check passes.

Commit: ae9b050

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The new installer/template works for simple paths but emits broken shell commands when the derived repo path contains whitespace, despite explicitly aiming to support arbitrary clone locations.

Review comment:

• [P2] Quote generated command paths — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.dcnQRx6oSX/templates/faber-command.md:16-16
  If the control-plane clone is under a path containing spaces, the generated /faber command tells the manager to run the reviewer with an unquoted absolute path like /Users/me/My Projects/fabrica/scripts/codex-review.sh <PR#>, which the shell splits before invocation. This breaks the installer’s stated support for clones living anywhere; the runnable command snippets should quote or shell-escape the substituted path, including the setup command below.

[yihanzhu]

Round 2 — addressed Codex [P2] (quote generated command paths)

Finding: the generated /faber command embedded the substituted {{FABRICA_ROOT}} path unquoted in runnable shell snippets, so a clone under a path with spaces (e.g. /Users/me/My Projects/fabrica) word-splits and the command breaks — defeating the installer's support for clones living anywhere.

Fix: double-quoted the substituted path in every runnable command snippet:

• templates/faber-command.md — the Codex reviewer invocation "{{FABRICA_ROOT}}/scripts/codex-review.sh" <PR#> and the setup command "{{FABRICA_ROOT}}/scripts/setup-target-repo.sh".
• scripts/install.sh — the printed next-steps setup command "$repo_root/scripts/setup-target-repo.sh" <owner>/<repo>.

Plain prose references to files that are read (e.g. the templates/ pointer) are left unquoted — they are not shell-executed.

Behavior preserved: path still derived from script location (symlink-aware), template-sourced, idempotent, .bak on differ, no ~/git/fabrica literal, no leftover {{FABRICA_ROOT}}, executable bit intact. Diff is 3 lines changed (quoting only).

Verification (sandboxed HOME, real ~/.claude/commands/faber.md untouched):

• Installed from a clone path containing a space (…/My Projects/fab rica) under a temp HOME.
• Generated faber.md: both runnable command lines are double-quoted with the full space-containing path intact; full path present 6×; no leftover placeholder.
• shellcheck scripts/install.sh clean; CI structure check passes; temp dirs cleaned up.

Commit: eb109de

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The generated /faber command includes invalid setup guidance for unbootstrapped target repos because it omits the required repository argument to the setup script. This should be corrected before the patch is considered complete.

Review comment:

• [P2] Pass the target repo to setup-target-repo — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.k0SrBedYaU/templates/faber-command.md:21-21
  When /faber is invoked in a repo that has not been bootstrapped, this instruction tells Faber to ask the user to run setup-target-repo.sh with no arguments. That script requires <owner>/<repo> and exits with usage when it is omitted, so the generated setup guidance cannot complete the label bootstrap unless the target repo argument is included here.

[yihanzhu]

Faber — escalating to human (round-3 cap)

CI green every round. Reached the ~3-round cap, so flagging needs-human per the loop. Unlike a thrashing standoff, all three findings were valid and productive — the loop did its job.

Round history:

• round-0 (initial PR): [P2] install.sh used sed for path substitution → broke on paths with sed metacharacters (&, #). FIXED (ae9b050) — switched to bash literal replacement ${var//.../...}.
• round-1: [P2] generated /faber command embedded the path unquoted in runnable snippets → broke on paths with spaces. FIXED (eb109de) — double-quoted all runnable command paths.
• round-2: [P2] the generated setup guidance runs setup-target-repo.sh with no <owner>/<repo> argument, but that script requires it (exits with usage). OPEN.

Note on the round-2 finding: this is a pre-existing content bug inherited verbatim from the current hand-made ~/.claude/commands/faber.md — i.e. your live /faber command already gives this broken setup instruction. Issue #10's stated scope is "parameterize the path," so fixing this command-content bug is arguably a separate concern.

Recommendation (your call at the gate):

• (a) I apply a one-line fix tomorrow on your OK — make the template say "{{FABRICA_ROOT}}/scripts/setup-target-repo.sh" <owner>/<repo> — then merge. Cleanest; closes everything. (I lean this.)
• (b) Merge Add scripts/install.sh: reproducibly generate the /faber command with a repo-derived path #12 as-is now (path-parameterization works, CI green) and fix the inherited setup-command bug in a separate tiny issue.

Substantive #10 goal is met: scripts/install.sh + templates/faber-command.md reproducibly generate /faber with a repo-derived path (no ~/git/fabrica literal), idempotent with .bak backup; RESTORE.md + README wired. CI green.

[yihanzhu]

Round 3 — addressed Codex [P2] (pass <owner>/<repo> to setup-target-repo)

Finding: the generated /faber setup guidance invoked setup-target-repo.sh with no argument, but that script requires <owner>/<repo> (usage: $0 <owner>/<repo>) and exits non-zero when omitted — so the label-bootstrap instruction couldn't complete.

Fix (1 line, templates/faber-command.md:21): appended the target-repo argument to the runnable command, keeping the existing double-quoting around the substituted path:
"{{FABRICA_ROOT}}/scripts/setup-target-repo.sh" <owner>/<repo> — phrased so the user runs it for this repo.

Other references checked for consistency:

• scripts/install.sh printed next-step — already includes <owner>/<repo>; unchanged.
• templates/repo-setup.md — already includes <owner>/<repo>; unchanged.
• README.md — prose description of the script (not a runnable command); left as-is.

No behavior change to install.sh logic — text/template correctness only.

Verification (sandboxed HOME; real ~/.claude/commands/faber.md untouched):

• HOME="$(mktemp -d)" bash scripts/install.sh → generated faber.md line 21 now reads the setup command double-quoted with the full substituted path and <owner>/<repo> appended; no leftover {{FABRICA_ROOT}}. Temp dir cleaned up; real faber.md confirmed unmodified.
• shellcheck scripts/install.sh clean (all *.sh clean, CI parity); CI structure check passes; install.sh executable bit intact.

Commit: 71733d9

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The added installer, template, and documentation changes are internally consistent and do not introduce any clearly actionable correctness issues.

[yihanzhu]

Faber — confirming round clean ✅

Applied the setup-target-repo.sh <owner>/<repo> fix (71733d9). Codex confirming re-review: no actionable issues. CI green. All three findings across the loop (sed metacharacters · unquoted paths/spaces · missing setup arg) are resolved.

Removing needs-human — ready for your merge.