Per-target north star (98a): atomic gate+doctor+persona flip, reading committed target state

[yihanzhu]

Per-target north star (98a) — the atomic gate + doctor + persona flip, reading COMMITTED state

Closes #98. Manager-debate reached PROCEED (Faber⇄Codex consensus). This activates the dormant #99 resolver atomically, so the consensus gate's source and Faber's approval source flip together and read committed target state — the recurring inconsistency #99 was reset to avoid.

The 5 build items (all in this one PR)

1. manager-review.sh → resolver, COMMITTED read, pinned commit. Resolves the north star for the target via ns_resolve, then reads its committed content with git show "$head_commit:.fabrica/north-star.md" — pinned to the same commit the detached review worktree is materialized at (head_commit, captured once), never a free-floating later HEAD. A star that exists only as an uncommitted worktree edit does NOT authorize (→ UNSET → FAIL before any Codex verdict). A LOCAL star still carrying the fabrica-shipped-default marker → FAIL (un-replaced placeholder). FABRICA_SELF is exempt from the placeholder-FAIL (its root NORTH_STAR.md is Fabrica's real approved goal).
2. doctor.sh check (h) → resolver (target). Reads the target's star via the resolver, consistent with the gate. UNSET → WARN (not FAIL) — doctor only diagnoses. Marker/staleness semantics + || true guards preserved; diagnostic reads the working-tree copy and notes when it differs from HEAD.
3. Faber approval/logging source flips in the SAME PR. templates/faber-command.md + manager/CLAUDE.md now reference the resolved target (committed) .fabrica/north-star.md as the operator-approval + logging source, not {{FABRICA_ROOT}}/NORTH_STAR.md (the Fabrica-self exception is kept). Gate source ≡ approval source — stated explicitly so they never diverge. The template was edited (not the generated output).
4. setup-target-repo.sh seeding re-added (reverted in Per-target north star (1/3): dormant resolver foundation (lib + adopter template + tests); consumers switch atomically in #98/98a #99's reset; recovered from history and re-applied): seeds <target>/.fabrica/north-star.md from the template if absent — idempotent (never overwrites), guarded by cwd-slug == target-slug via ns_slug_eq (env -u GH_REPO for the slug), and --check reports a missing star as drift.
5. templates/.fabrica/north-star.md → present tense — drops the "dormant / Per-target north star (98a): atomic gate+doctor+persona flip, reading committed target state #98 pending" hedge, states that setup/doctor/manager-review use it; keeps the fabrica-shipped-default marker + the "replace, then remove the marker, then commit" nudge.

How the committed read is pinned to a single commit

head_commit="$(git rev-parse HEAD)" is captured once. The review worktree is added at that exact commit (git worktree add --detach "$worktree" "$head_commit"), and the north-star content is read from the same commit (git show "${head_commit}:…"). One pin, no re-lookup — so the gate's goal is exactly the committed tree Codex reviews, immune to the dirty working tree.

Note on the resolver: ns_resolve classifies the source via its identity logic (Fabrica-self vs. target), but its LOCAL-vs-UNSET result is working-tree-based, so the gate does not use it to authorize. For a normal target the authoritative authorize test is whether .fabrica/north-star.md exists at head_commit — which is what makes a committed-but-worktree-deleted star still authorize.

Tests (scripts/test/north-star-gate.test.sh, 30 asserts, CI-wired)

Runs the real scripts end-to-end against throwaway real git repos with gh/codex faked on PATH (hermetic, offline). Highlights:

• approval-source == gate-source (source identity — pinned by inspecting the shipped files, since operator approval isn't machine-readable).
• committed vs uncommitted — BOTH directions: (a) a worktree-only star does NOT authorize (gate FAILs); (b) a HEAD-committed star still authorizes even if the working-tree copy is deleted or modified to a placeholder.
• LOCAL committed → debates; LOCAL+marker → FAIL; UNSET → FAIL; doctor UNSET → WARN, doctor LOCAL committed → pass, doctor marker → WARN; setup seeds only when cwd-slug==target, --check flags missing star as drift, seed idempotency.

The #99 resolver-lib suite stays green (33 asserts). Added the new test to ci/required-files.txt and .github/workflows/ci.yml.

CI green locally

• structure check (under bash) — ok
• shellcheck -x -S style — clean across all *.sh
• bash -n on every changed script — ok
• north-star resolver tests — 33 passed
• north-star gate/consumer tests — 30 passed

⚠️ Self-modification note

The scripts take effect on merge. The live /faber command changes only after re-running scripts/install.sh from the merged clone (this PR edits the template, not the generated ~/.claude/commands/faber.md). The persona (manager/CLAUDE.md) takes effect the next time Faber reads it. This is the safety-critical atomic flip — gate source ≡ approval source, committed-only — the rail being preserved, not weakened. Every other rail is intact (reviewer read-only, CI merge gate, rounds cap, needs-human, merge SHA-pin).

Out of scope → 98b: docs sweep (README/QUICKSTART/RESTORE/root CLAUDE.md/reviewer-doc/repo-setup), greenfield tie-in, MapleFolio history, migration guidance.

🤖 Generated with Claude Code

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: b33b444
Reviewed-base: d13b5b7

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

The manager-review gate rejects valid seeded north-star files because it scans the whole file for the placeholder marker, and setup can also shadow Fabrica-self's root north star. These issues break the core per-target north-star workflow introduced by the patch.

Full review comments:

• [P1] Scope placeholder-marker check to the active heading — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Dzs6h4TjC8/scripts/manager-review.sh:208-208
  When an adopter follows the seeded template and removes <!-- fabrica-shipped-default --> only from the active heading, the committed file still contains that exact string in the explanatory paragraph at the top. This whole-file grep therefore exits before Codex for a valid north star, while doctor.sh already scopes the same check to the active line. Please check the active status: active heading, or otherwise exclude prose, instead of the entire file.

• [P2] Skip target-local seeding for Fabrica self — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Dzs6h4TjC8/scripts/setup-target-repo.sh:266-266
  When the current target is the Fabrica repo itself, cwd_is_target is true, so this auto-setup path writes .fabrica/north-star.md into the control-plane checkout. The resolver checks LOCAL before FABRICA_SELF, so that seeded placeholder shadows the intended root NORTH_STAR.md and subsequent manager-review.sh runs for Fabrica self fail instead of using the self north star. Exempt Fabrica-self from the .fabrica drift/seed path.

• [P2] Guard resolver sourcing in doctor — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Dzs6h4TjC8/scripts/doctor.sh:125-125
  If scripts/lib/north-star.sh is missing or not restored, this unconditional source aborts the script before check (f) can read ci/required-files.txt and report the missing restore-critical file. That regresses doctor’s purpose as a restore self-check; guard the source or move it after the manifest check so a missing resolver is reported as a fail: line instead of a shell crash.

[cloudflare-workers-and-pages]

Deploying fabrica with    Cloudflare Pages

Latest commit:	71eae3c
Status:	✅  Deploy successful!
Preview URL:	https://3c335278.fabrica-6yx.pages.dev
Branch Preview URL:	https://issue-98a-atomic-flip.fabrica-6yx.pages.dev

View logs

[yihanzhu]

round-1 — 6 authorization fixes (head 16e59ff)

Two adversarial passes on this safety-critical gate flip — the Codex review and a separate attack sweep — found 6 distinct authorization bugs. All fixed here, each with a regression assert. Both suites green; no rail weakened (these strengthen committed-only, gate≡approval source, placeholder blocks, per-target isolation).

Two came from the adversarial attack sweep specifically: the P1 slug-spoof (B) and the placeholder-bypass false-PASS (A).

The 6 fixes

• A — Placeholder check (gate + doctor), two bugs together. The marker check is now (1) scoped to the active-entry region — a whole-file grep tripped on the marker's mentions in the template prose, wrongly FAILing a correctly-replaced star; and (2) whitespace/case-insensitive — a byte-exact grep -F let <!--fabrica-shipped-default--> / padded / UPPERCASE / tab / reflow-split variants bypass and wrongly AUTHORIZE. New shared helper ns_has_shipped_default_marker / ns_active_region in scripts/lib/north-star.sh, used identically by manager-review.sh and doctor.sh (h) so the gate and self-check never disagree.
• B — [P1 SECURITY] Resolver Fabrica-self identity is PATH-only. Dropped the slug-based FABRICA_SELF fallback: the slug comes from the attacker-settable git remote URL, so a target pointing origin at Fabrica's slug was authorized against Fabrica's root star (bypassing its own star and the placeholder-FAIL). Kept only the gh-free path check (toplevel == ns_fabrica_root), and gave it precedence over the Order-1 LOCAL check so a stray/committed .fabrica/north-star.md in the control-plane checkout can't shadow the root star.
• C — Exempt Fabrica-self from seeding (setup-target-repo.sh): never seed .fabrica/north-star.md into the control-plane repo itself (path-based detection, same as B); skips with a note.
• D — doctor resolver source guard (doctor.sh): a missing scripts/lib/north-star.sh is now reported as a fail: line and the summary still prints — no crash before check (f).
• E — doctor (h) reads the COMMITTED source (doctor.sh): the verdict is driven off git cat-file -e HEAD:… + git show HEAD:…, matching what the gate authorizes on; the working-tree copy is a supplementary note. UNSET still WARNs.
• F — [confused-deputy] Gate rejects nested/embedded repos (manager-review.sh): refuses to authorize off a north star in a separate git repo nested inside another work tree. Detected via parent-in-work-tree + a different --git-common-dir, so a legitimate linked worktree (same repo) is not rejected.

New regression tests

Resolver suite (north-star-resolver.test.sh) — now 43 passed:

• (m) shared marker matcher: no-space / UPPERCASE / padded / tab / reflow-split variants on the active heading → match; marker-only-in-prose (correctly-replaced) → no match; non-active heading → no match; shipped template → match.
• (c-spoof) [P1] slug spoofs Fabrica but path differs → NOT FABRICA_SELF (UNSET).
• (c-prec) stray committed .fabrica/north-star.md in Fabrica-self → still FABRICA_SELF/root (precedence).
• (c)/(c')/(a)/(b)/(e) updated to the PATH-only identity model.

Gate suite (north-star-gate.test.sh) — now 48 passed:

• (3d-i) marker variants (no-space+UPPERCASE / reflow-split / tab) → gate FAIL.
• (3d-ii) correctly-replaced star (marker only in prose) → gate PROCEEDs (no false FAIL).
• (6a) nested/embedded repo → gate FAIL; (6b) linked worktree → PROCEEDs (not rejected).
• (4d)/(4d') doctor (h) reads committed star even when worktree copy is deleted / modified.
• (4e) doctor with missing lib → summary prints + missing lib reported.
• (5d) setup on Fabrica-self → does not seed .fabrica/north-star.md.

CI green locally

• structure check (bash) — ok
• shellcheck -x -S style — clean
• bash -n on all changed scripts — clean
• resolver tests: 43 passed / 0 failed
• gate tests: 48 passed / 0 failed

Prompt-sync: manager-review.sh / doctor.sh / setup-target-repo.sh take effect on next run; scripts/lib/north-star.sh when they re-source it. No /faber regeneration needed (install.sh unchanged).

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 16e59ff
Reviewed-base: d13b5b7

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

The patch adds the intended per-target north-star flow, but it has edge cases that can block valid north stars: self runs can ignore a committed root star if the worktree file is absent, and marker detection can false-positive on prose mentions. These are functional gate failures rather than documentation nits.

Full review comments:

• [P2] Resolve self north star from HEAD — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.ih8TALqP7U/scripts/manager-review.sh:225-228
  For a Fabrica-self run where NORTH_STAR.md is still committed but the working-tree copy has been deleted, ns_resolve reports UNSET because it checks the filesystem before returning FABRICA_SELF; this branch is skipped and the script then looks for HEAD:.fabrica/north-star.md, failing before Codex despite HEAD:NORTH_STAR.md being the intended committed source. This violates the new committed-state guarantee for self runs, so the self/normal source decision should not depend on the worktree copy existing.

• [P2] Require the marker comment when matching placeholders — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.ih8TALqP7U/scripts/lib/north-star.sh:228-230
  When a valid active north-star entry mentions fabrica-shipped-default as prose inside the active region after the <!-- ... --> marker was removed, this still returns true because it strips whitespace and searches for the bare token. That makes manager-review.sh and doctor.sh keep treating the star as the shipped placeholder even though the actual marker comment is gone; match the HTML comment form flexibly instead of any token occurrence.

[yihanzhu]

Round-2 — 4 residual authorization fixes (Codex re-review + second attack workflow)

Head: e86dd71. All 4 preserve the authorization rails; each ships a regression test. Prior gate + resolver asserts stay green.

FIX 1 — FABRICA_SELF off COMMITTED state (ns_resolve, scripts/lib/north-star.sh). The Order-1 self branch now keys on committed existence — git -C "$toplevel" cat-file -e HEAD:NORTH_STAR.md — instead of a working-tree [ -f "$root_star" ] stat. A control plane whose NORTH_STAR.md is committed but whose worktree copy is deleted now still resolves FABRICA_SELF, agreeing with the gate's git show HEAD:NORTH_STAR.md (pre-fix it read UNSET/LOCAL and the gate's committed read disagreed).

FIX 2 — placeholder marker = COMMENT FORM, not a bare token (ns_has_shipped_default_marker, shared by gate + doctor). After strip-whitespace + lowercase, the matcher now requires the HTML-comment form via grep -Eq "<!--[^>]*fabrica-shipped-default[^<]*-->". The [^>]* before the token can't cross a preceding --> and [^<]* after it can't cross into a following <!--, so the token must sit inside one <!-- … --> comment. Genuine variants still match (no-space, padded, UPPERCASE, tab, reflow-split, and comments with extra interior text like <!-- fabrica-shipped-default: keep -->); a delimiter-free prose mention in the active region (e.g. "remove the fabrica-shipped-default marker") no longer false-FAILs a valid star — the round-1 regression this closes.

FIX 3 — case-canonicalize the Fabrica-self path compare (round-1 regression; north-star.sh + setup-target-repo.sh). toplevel is git-canonical (git rev-parse --show-toplevel) but fabrica_root was pwd -P (case-preserving), so a case-variant control-plane path on a case-insensitive FS made toplevel == fabrica_root falsely differ → Fabrica-self's own debate FAILed / setup polluted the control plane. ns_fabrica_root now canonicalizes its physical root through git (git -C <root> rev-parse --show-toplevel, falling back to pwd -P when not a work tree), so both operands are produced the same way. Both consumers share ns_fabrica_root, so one change fixes both compares.

FIX 4 — reject a committed SYMLINK north star (new shared helper ns_committed_is_regular_file; gate LOCAL + FABRICA_SELF branches + doctor (h)). Before the marker check, assert git ls-tree <commit> -- <path> mode is 100644/100755, not 120000. A committed symlink makes git show return the link's target-path string (verified: returns ../real-goal.md), bypassing the marker check. The gate now FAILs with "committed north star must be a regular file, not a symlink"; doctor (h) WARNs symmetrically.

Tests (one per fix; all prior asserts stay green)

• Resolver suite (north-star-resolver.test.sh): 50 passed, 0 failed — adds (c-committed) FIX 1, (n) FIX 3 case-canonical invariant (case-variant probe on case-insensitive FS; skip-pass on case-sensitive CI), and (m) FIX 2 prose/interior/decoy cases.
• Gate suite (north-star-gate.test.sh): 58 passed, 0 failed — adds (2c) FIX 1 self committed+worktree-deleted → PROCEED, (3d-iii) FIX 2 prose-in-active → PROCEED, (7a)/(7b) FIX 4 LOCAL/self symlink → FAIL, (4f) doctor symlink → WARN.

CI green locally

• structure check (ci/required-files.txt, bash) — ok
• shellcheck -x -S style (all .sh) — clean
• bash -n (all touched scripts) — ok
• both test suites — green (counts above)

Deferred

The pin-anchor hardening (pin the gate to origin/HEAD/default-branch rather than raw local HEAD) is out of scope here and tracked in #102 — it has offline/greenfield tradeoffs (both central to North Star B) that need their own debate. This PR keeps the existing head_commit=$(git rev-parse HEAD) pin; the committed-vs-uncommitted core guarantee of 98a stands.

Self-modification note: these change scripts/lib/north-star.sh and the gate/doctor/setup consumers — live behavior changes only when Faber re-reads the synced artifacts (the resolver is sourced at run time). No /faber regeneration needed for the coder path. Do NOT merge / do NOT approve — round-2 review.

[yihanzhu]

CI-red fix: shellcheck 0.9.0 vs local 0.11.0 skew (head 050cb94)

CI runs shellcheck 0.9.0 (ubuntu-latest preinstalled); locally we have 0.11.0, which does not surface these — that version gap was the whole bug. Reproduced the 4 CI findings by running shellcheck 0.9.0 directly (-x -S style, the exact CI invocation) and confirmed they're now gone.

Fixed:

• SC2015 ×2 (scripts/manager-review.sh:195,196) — restructured: moved || true outside the command substitution onto the assignment (inner_common="$( cd … && cd … && pwd -P )" || true), removing the A && B || C shape inside the subshell entirely. Still errexit-safe.
• SC2317 ×2 (scripts/test/north-star-resolver.test.sh:186,188) — the intentionally-unreachable slug stubs. Made the disable version-robust: # shellcheck disable=SC2317,SC2329 (0.9.0 emits SC2317, 0.11.0 emits SC2329 for the same "appears unreachable").

Verified: full 0.9.0 sweep across all *.sh is clean (exit 0) — no other 0.9.0-only findings; 0.11.0 also still clean; resolver 50/50 + gate 58/58 pass; structure check ok.

Note for coders: local shellcheck 0.11.0 disagrees with CI's 0.9.0 — verify against 0.9.0 (or write version-robust disables covering both code numbers) before declaring shellcheck green.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 050cb94
Reviewed-base: d13b5b7

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

The new symlink guard breaks documented subdirectory invocations of manager-review.sh, and the active-region scanner can miss the real active placeholder marker when earlier prose contains the same status phrase. These are functional gate issues that should be fixed before considering the patch correct.

Full review comments:

• [P2] Check committed paths from the repository root — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.dFWzSBx4Ze/scripts/lib/north-star.sh:202-202
  When manager-review.sh is run from a subdirectory, which the script documents as supported, it calls this helper with $PWD and a root-relative path such as .fabrica/north-star.md. git ls-tree ... -- "$relpath" interprets the pathspec relative to -C "$dir", so from any subdirectory the mode lookup returns empty for a regular committed north star and the new symlink guard rejects the run as if the file were not regular. Resolve dir to the git top-level here, or pass the captured top-level, before running ls-tree.

• [P2] Require the active marker scan to start on a heading — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.dFWzSBx4Ze/scripts/lib/north-star.sh:248-248
  This starts the active region on any line that contains status: active, not just a Markdown heading as the function contract says. If a north-star file has prose or front matter before the real active heading that mentions status: active, the region ends at the next heading before it ever sees the shipped-default marker on the actual placeholder heading, so manager-review.sh can proceed against an unreplaced template. Anchor this condition to heading lines before accepting it as the active entry.

[yihanzhu]

Round-3: 2 precision fixes from the final Codex review, both in scripts/lib/north-star.sh (head 0429652).

FIX 1 [P2] — symlink guard resolves the pathspec from the git top-level (ns_committed_is_regular_file).
The helper ran git -C "$dir" ls-tree "$commit" -- "$relpath" with dir=$PWD and a root-relative relpath. When the gate is invoked from a subdirectory of the target (documented as supported — the companion git show <commit>:<relpath> reads root-relative regardless of cwd), ls-tree interpreted the pathspec relative to the subdir → the mode lookup returned empty for a valid regular committed file → the round-2 symlink guard falsely rejected the run. Fix: resolve <dir> to its git top-level (git -C "$dir" rev-parse --show-toplevel) and run ls-tree there, so the root-relative pathspec is correct from any subdirectory. An empty top (not a work tree) short-circuits to rc 1 — we never git -C "" ls-tree, which git treats as staying in the current dir.

FIX 2 [P2] — active-region scan starts on a heading line (ns_active_region).
The region opened on any line containing status: active. A north-star file with prose/front-matter mentioning status: active before the real active heading would start the region there and end it at the next heading — so the shipped-default marker on the actual placeholder heading was never scanned → the gate could proceed against an un-replaced template (placeholder bypass). Fix: anchor the region-start to a Markdown heading line (^#{1,6}[[:space:]]) that also carries the status: … active marker. Verified against the shipped files — the real active entries in NORTH_STAR.md / .fabrica/north-star.md / the template are headings (### … · status: **active** · …), so this matches the real format.

Tests (one regression test per fix, plus gate integration):

• Resolver suite: (o) ns_committed_is_regular_file recognizes a regular committed star from a subdir (rc 0) and still rejects a committed symlink from a subdir; (p) ns_active_region/marker matcher — a prose status: active before the marked active heading is still detected as a placeholder, and a normal single-heading entry still works. 50 → 57 asserts.
• Gate suite: (8a/8b) real gate from a subdirectory proceeds on a regular committed star and still FAILs on a committed symlink; (9a/9b) prose-status: active-before-heading still FAILs (no bypass), single-heading active proceeds. Two round-1 placeholder fixtures moved to the heading-form marker (matching the real shipped template). 58 → 66 asserts.

All 66 gate + 57 resolver asserts green. These preserve the authorization rails (symlink guard, committed read, placeholder-FAIL, PATH-only Fabrica-self identity all intact).

Local verification: shellcheck 0.9.0 (CI’s version, downloaded static binary) clean over all *.sh with -x -S style; structure check ok under bash; bash -n clean. No prompt/behavior sync needed — this is gate tooling, not a synced prompt.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 0429652
Reviewed-base: d13b5b7

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

The resolver can misclassify a Fabrica-self checkout as a normal target when the root north star is missing but a stray target-local star exists, allowing the gate to use the wrong source.

Review comment:

• [P2] Keep Fabrica-self from falling through to local star — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.P92oqYYaGp/scripts/lib/north-star.sh:376-379
  When the cwd is the control-plane checkout but NORTH_STAR.md is not committed, this condition falls through to the .fabrica/north-star.md branch below. If a stray local star exists, manager-review.sh can authorize a Fabrica-self run against that file instead of failing on the missing committed root north star, contradicting the documented precedence that .fabrica/north-star.md must not shadow Fabrica-self. Return FABRICA_SELF on the path identity match and let callers reject the missing root file.

[yihanzhu]

Round-3 [P2] — Fabrica-self path identity classifies as FABRICA_SELF unconditionally (head db7ba36)

FABRICA_SELF is now classified purely by PATH identity; a missing committed root FAILs at the gate instead of falling through to .fabrica/north-star.md. Classification (which source applies) is the resolver's job; authorization (whether that source carries a real committed star) is the gate's job.

• scripts/lib/north-star.sh (ns_resolve): on a PATH-identity match (toplevel == fabrica_root, both git-canonical), return FABRICA_SELF <root>/NORTH_STAR.md unconditionally. Removed the round-2 git cat-file -e HEAD:NORTH_STAR.md existence gate — that gate let a control-plane cwd whose root star was not committed fall through to the LOCAL branch, where a stray committed .fabrica/north-star.md would be authorized as if it were Fabrica's own star. .fabrica can now never shadow Fabrica-self.
• scripts/manager-review.sh (FABRICA_SELF branch): authorization still requires a committed root (git show HEAD:NORTH_STAR.md). Reordered so the committed read runs before the symlink guard: a missing committed root now FAILs cleanly with the accurate "NORTH_STAR.md is not committed at HEAD" message (previously the symlink-guard-first ordering mis-reported an absent root as a symlink). It does not fall back to .fabrica. The FAIL path is set -e-safe (condition context).
• scripts/doctor.sh / scripts/setup-target-repo.sh: unchanged — neither ever gated classification on committed existence. doctor's FABRICA_SELF-uncommitted-root WARN is now correctly reachable.

Tests (all existing asserts kept green):

• Resolver: 57 → 59 — new (c-no-committed-root): control-plane with NO committed NORTH_STAR.md + a stray committed .fabrica/north-star.md → resolves FABRICA_SELF (not LOCAL), path is the root star.
• Gate: 66 → 69 — new (2d): same fixture → gate FAILs ("NORTH_STAR.md is not committed at HEAD"), and does not authorize/PROCEED off the stray .fabrica star.
• Regressions green: committed root present → FABRICA_SELF + PROCEED; committed-but-worktree-deleted root → still PROCEED; symlink root → still FAILs with the symlink message.

Verification: shellcheck 0.9.0 (CI's version) -x -S style over all 9 *.sh → clean (exit 0); bash -n clean; structure check ok. GitHub CI expected green.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: db7ba36
Reviewed-base: d13b5b7

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

The target-local north-star gate is mostly implemented, but the migration leaves users pointed at stale setup documentation and doctor can still miss uncommitted north-star changes for Fabrica-self runs.

Full review comments:

• [P2] Update referenced north-star setup docs — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.XGHQsHiam2/scripts/manager-review.sh:311-311
  When a target has no committed .fabrica/north-star.md, this new failure sends users to reviewer/manager-review.md, but that document still instructs them to use the control-plane NORTH_STAR.md (and scripts/install.sh still prints the same next step). A fresh adopter following the referenced docs will edit/approve the wrong file and keep hitting this gate; update the referenced docs/install output alongside this new target-local requirement.

• [P2] Warn on dirty Fabrica-self north star — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.XGHQsHiam2/scripts/doctor.sh:359-360
  When doctor.sh is run in the Fabrica control-plane checkout, committed_relpath is NORTH_STAR.md, so this .fabrica-only diff check suppresses the warning for uncommitted root north-star edits. In that scenario manager-review.sh reads HEAD:NORTH_STAR.md and ignores the working-tree change, but doctor reports a clean pass; compare against $committed_relpath so Fabrica-self gets the same “gate reads committed version” warning.

[yihanzhu]

Round-3 [P2] consistency fixes applied (head 4bf14fd):

• FIX 1 — doctor dirty-warning (scripts/doctor.sh): the head-vs-worktree "differs from HEAD — the gate reads the committed version" note now drives off $committed_relpath (the exact path the gate reads) instead of a hardcoded .fabrica/north-star.md guard. So a Fabrica-self checkout (committed_relpath = NORTH_STAR.md) now gets the same warning on an uncommitted root north-star edit — previously suppressed, which read as a misleading silent pass while manager-review.sh authorizes off HEAD:NORTH_STAR.md. Added a doctor test (4d'') — a Fabrica-self clone with a dirty root star; it FAILs against the old hardcoded check and passes now.

• FIX 2 — reviewer/manager-review.md (the doc the gate's FAIL message points at): rewritten to the target-local model. New "north star" section; corrected the step-0 gate check, the how-it-runs step 1 (was factually reading the control-plane NORTH_STAR.md), the override-worthy-drop log location, and the prompt — all now: the debate is judged against the target's committed .fabrica/north-star.md (read at HEAD), with the control-plane root NORTH_STAR.md now being only Fabrica-self's own target file.

• FIX 3 — scripts/install.sh next-step output: the printed guidance now tells adopters to set + commit + approve .fabrica/north-star.md per target (Fabrica-self uses its own root NORTH_STAR.md), matching the shipped gate. Only the north-star text changed; install logic untouched.

The broader narrative docs sweep (README.md, QUICKSTART.md, RESTORE.md, root CLAUDE.md, templates/repo-setup.md) plus greenfield tie-in and the MapleFolio history log remain 98b — out of scope here.

Verification: gate suite 71 passed / 0 failed (was 69; +2 from the new drift asserts), resolver suite 59 passed / 0 failed, bash -n clean on all changed scripts, structure check ok, and shellcheck 0.9.0 (CI's version, -x -S style over all *.sh) clean (exit 0).

Prompt/doc sync note: these are a doc (reviewer/manager-review.md) + a script's printed output (install.sh) — they take effect immediately; no /faber regeneration needed for these two.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 4bf14fd
Reviewed-base: d13b5b7

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

The per-target north-star switch misses linked worktrees of the Fabrica repo itself, causing self-runs from those worktrees to be treated as external targets and potentially seeded with the wrong north-star file.

Review comment:

• [P2] Recognize same-repo worktrees as Fabrica-self — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.VyykXulwJm/scripts/lib/north-star.sh:369-369
  When the target is a linked worktree of the Fabrica control-plane repo, git rev-parse --show-toplevel returns the linked worktree path while ns_fabrica_root returns the main checkout path, so this strict equality fails. The resolver then treats the Fabrica worktree as an external target and manager-review.sh/doctor won't read the committed root NORTH_STAR.md; the mirrored check in setup-target-repo.sh can also seed .fabrica/north-star.md into that Fabrica worktree. Compare the shared git common dir or otherwise include same-repo worktrees in the self check.

[yihanzhu]

Fabrica-self identity now recognizes linked worktrees via shared git common-dir (consistent with the nested-repo guard).

• What: the self-identity check was strict top-level path equality (toplevel == fabrica_root). Faber runs from a linked worktree (.claude/worktrees/*), whose git rev-parse --show-toplevel is the worktree path while ns_fabrica_root returns the main checkout — so the compare failed and the Fabrica worktree was misclassified as an external target (manager-review/doctor skipped the root NORTH_STAR.md; setup could seed .fabrica/north-star.md into it).
• Fix: new ns_git_common_dir helper canonicalizes git rev-parse --git-common-dir to an absolute physical path; ns_resolve (and setup-target-repo.sh's cwd_is_fabrica_self guard) classify Fabrica-self iff the target's common-dir equals Fabrica's own — the same signal round-1's nested-repo guard (FIX F) uses. The top-level path match is kept as one accepted case. A linked worktree shares its parent's common-dir → self; a genuinely separate repo (different common-dir) → not self.
• Tests: +2 resolver cases (linked worktree → FABRICA_SELF; separate repo → not falsely self) and +1 gate case (setup from a Fabrica worktree skips the seed). Resolver 62 pass, gate 73 pass — all prior asserts green. shellcheck 0.9.0 clean (-x -S style); structure check + bash -n green.
• Safety rails preserved (slug still never used for identity; classification vs authorization split intact). This is shipped-tooling behavior (scripts/lib + scripts/*.sh) — live on merge, no install.sh re-run needed.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 7ebaa62
Reviewed-base: d13b5b7

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

The production changes appear coherent, but the newly added gate test can hang during local interactive execution because its fake codex blocks on stdin for doctor probes.

Review comment:

• [P2] Guard fake codex stdin drain — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.h0Qq16AFd4/scripts/test/north-star-gate.test.sh:113-113
  When this suite reaches the doctor checks, doctor.sh invokes codex login --help without piping stdin. The fake codex here always runs cat, so a normal local run of bash scripts/test/north-star-gate.test.sh from an interactive terminal blocks waiting for input; CI may mask this if stdin is closed. Drain stdin only for the codex exec ... - path.

[yihanzhu]

Fake codex stub now drains stdin only for the real gate call (codex exec … -), so the suite no longer blocks on stdin during doctor probes (codex login --help / codex login status are invoked without piped stdin). Non-exec probes exit 0 without reading stdin, preserving their observable behavior. Test-only — no production script changed.

Verified: both suites green (gate 73/73, resolver 62/62), completing in ~1s even with a never-closing FIFO on stdin (proving no stdin block). Shellcheck 0.9.0 clean (-x -S style over all *.sh), bash -n OK, structure check ok.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 11d335b
Reviewed-base: d13b5b7

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

The per-target north-star wiring mostly follows the intended committed-source model, but setup's new worktree-only missing-file checks leak into doctor as a hard failure and can seed a placeholder over an already committed star that is merely deleted locally.

Full review comments:

• [P2] Keep north-star drift out of label checks — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.6lncze4kTZ/scripts/setup-target-repo.sh:242-244
  When doctor.sh <repo> is run from the target checkout after labels are correct but .fabrica/north-star.md is absent, this new drift path makes setup-target-repo.sh --check exit 1; doctor check (g) treats that as a hard fail: even though check (h) and the surrounding docs classify unset/missing north stars as warn-only so user-directed work can proceed. This turns a north-star warning into a blocking doctor failure.

• [P2] Do not seed over a committed north star — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.6lncze4kTZ/scripts/setup-target-repo.sh:309-314
  When the target already has a real .fabrica/north-star.md committed but the working-tree copy is deleted, this worktree-only -f check falls through and copies the shipped placeholder. Since setup runs as benign bootstrap while the gate/doctor intentionally read HEAD in this scenario, it can dirty the checkout with a placeholder over an existing committed north star and risk that placeholder being committed later.

[yihanzhu]

Seeding removed from 98a (head 3ab6fae).

Reverted scripts/setup-target-repo.sh to origin/main — placeholder-seeding of a target's .fabrica/north-star.md is adoption scope, so it moves to 98b. This drops all of the seeding additions in one revert (the north-star seed, the cwd-slug/common-dir seed-guard, the Fabrica-self seed-exempt, and the --check north-star-missing drift). --check returns to label-only.

This removes the cause of the two P2s (rather than patching around them):

• doctor (g) no longer hard-fails on a missing north star — check (g) delegates to setup-target-repo.sh --check, now label-only, so a missing star can no longer make the label check hard-fail.
• No seed-over-committed risk — setup no longer writes .fabrica/north-star.md at all.

The atomic flip core is unchanged: the resolver (incl. ns_git_common_dir and the PATH/common-dir Fabrica-self identity — still used by the gate/doctor), the manager-review.sh committed-read gate + placeholder-FAIL + nested-repo guard, and doctor.sh check (h) all stay. All resolver-identity asserts stay.

Test changes: removed the section-(5) seeding asserts from north-star-gate.test.sh (5a/5b/5c/5d/5e) + their runner invocations, and synced the suite/CI comments. north-star-resolver.test.sh is unchanged (it never drove setup; all its slug/ns_slug_eq cases are pure resolver-identity).

Verification (local):

• git diff origin/main -- scripts/setup-target-repo.sh is empty (== main); setup-target-repo.sh absent from git diff --stat origin/main.
• Gate suite: 62 passed, 0 failed (was 73 asserts; 11 seeding asserts removed).
• Resolver suite: 62 passed, 0 failed (unchanged).
• Structure check ok under bash; bash -n clean; shellcheck 0.9.0 clean (-x -S style, all 9 *.sh).

Do not merge / do not approve — for review.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 3ab6fae
Reviewed-base: d13b5b7

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

The patch has a real gate-bypass bug in marker detection for large north-star files, and its new unset-star remediation points users to a command that does not create the required file. These should be fixed before the change is considered correct.

Full review comments:

• [P1] Don't let large marked north-star files bypass the gate — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.xQ10jP2jFa/scripts/lib/north-star.sh:299-299
  When callers run printf '%s' "$north_star" | ns_has_shipped_default_marker - under set -o pipefail, this exit closes stdin as soon as the next heading/rule is seen. If a committed .fabrica/north-star.md still has the placeholder marker in the active entry and has enough content after the next section, the upstream printf exits with SIGPIPE 141, so the if condition is false even though the marker matched; manager-review.sh can then proceed against the shipped placeholder and doctor.sh can miss the warning. The awk should drain stdin, or the callers should avoid piping into a function that exits early.

• [P2] Don't suggest a setup command that never seeds the star — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.xQ10jP2jFa/scripts/manager-review.sh:311-312
  In the no-committed-star path, this message tells users to run setup-target-repo.sh to seed .fabrica/north-star.md, but that script is still label-only in this patch, so following the instruction leaves the file absent and rerunning the gate fails the same way. Either implement the seeding here/there, or change this remediation (and the matching template text) to tell users to create/copy and commit .fabrica/north-star.md directly.

[yihanzhu]

Round-3 Codex findings addressed (head aacd040)

[P1] ns_has_shipped_default_marker no longer SIGPIPE-bypasses the placeholder-FAIL under pipefail.
ns_active_region's awk used to exit as soon as the active region ended (next heading/rule). Callers run printf '%s' "$north_star" | ns_has_shipped_default_marker - under set -o pipefail. For a large committed .fabrica/north-star.md (placeholder marker in the active entry + tens of KB of body after the next section — enough to fill the ~64KB pipe buffer), that early exit closed awk's stdin while the upstream printf still had bytes to write → printf died with SIGPIPE (141) → pipefail made 141 the whole pipeline's status → the if ns_has_shipped_default_marker condition read FALSE even though the marker matched → the gate proceeded against a shipped placeholder and doctor.sh missed the WARN (a placeholder-FAIL bypass).

The awk now drains stdin to EOF — it never exits. When the region ends it latches (in_region=0; done=1) and keeps reading every remaining line, emitting nothing further; a !done guard keeps it to the FIRST active region. Emitted bytes are byte-identical to the old exit behaviour, so the marker verdict is unchanged for every existing case — only the stdin-draining differs, which is what closes the bypass.

New regression test (q) in scripts/test/north-star-resolver.test.sh: a ~250KB placeholder star still reports marker present under pipefail (reproduced rc=141 / "marker MISSED" against the pre-fix awk; passes after), and a large correctly-replaced star still returns a clean no-match (rc 1, no SIGPIPE).

[P2] Remediation/template/docs updated to "operator creates + commits .fabrica/north-star.md" (setup no longer seeds).
Seeding was stripped from setup-target-repo.sh (label-only now), so every message telling the operator setup will seed the star was stale. Now they all say: copy templates/.fabrica/north-star.md into the target, replace the placeholder, remove the fabrica-shipped-default marker, then commit + approve it — and note explicitly that setup-target-repo.sh only creates the loop labels. Fixed in:

• scripts/manager-review.sh — the no-committed-star FAIL message.
• templates/.fabrica/north-star.md — the "Fabrica's tooling uses this file" paragraph.
• scripts/install.sh — next-step 2.
• scripts/setup-target-repo.sh — manual-follow-up step 6 (also corrected the stale NORTH_STAR.md in your fabrica clone pointer to the per-target .fabrica/north-star.md).
• scripts/lib/north-star.sh — a stale "setup would seed" rationale comment.

Every authorization rail is preserved (this fixes a bypass). No auto-seed added — accurate to this PR's behaviour (greenfield/98b may automate later).

Verification: shellcheck 0.9.0 clean (version: 0.9.0) on all scripts with -x -S style; bash -n clean; structure check ok; resolver suite 64 passed / 0 failed, gate suite 62 passed / 0 failed. GitHub CI expected green.

Not merging, not approving.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: aacd040
Reviewed-base: d13b5b7

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

The manager-review gate can still proceed on a malformed committed north-star file that has no active entry, which undermines the new per-target authorization check. This should be fixed before the patch is considered correct.

Review comment:

• [P2] Reject north-star files without an active entry — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.vLf1uXXRZa/scripts/manager-review.sh:294-300
  When a target commits .fabrica/north-star.md but the file has no valid status: active heading (for example a typo while editing the shipped template), ns_has_shipped_default_marker returns false and this branch proceeds to run Codex anyway. Since proactive work is authorized only by an approved active north star and doctor.sh only warns on this condition, the gate should fail before debating when ns_active_region is empty.

[yihanzhu]

[P2] fixed — the manager-debate gate now FAILs when the committed north star has no active entry.

Previously a committed .fabrica/north-star.md (or the control plane's NORTH_STAR.md) with content but no valid status: active heading — e.g. the status: active marker was mistyped or deleted while editing the template — slipped through: ns_has_shipped_default_marker is false (no active region → no marker to find) and the file exists (so it's not UNSET), so the gate proceeded to run Codex against a goalless file. Proactive work is authorized only by an approved active north star.

Fix (scripts/manager-review.sh): after reading the committed north star on an authorized path (FABRICA_SELF root NORTH_STAR.md or LOCAL .fabrica/north-star.md), require a non-empty active region via the shared ns_active_region helper — using the SAME committed content already read (pinned commit), fed over stdin (-), identical to doctor's (h) check and the placeholder scoping. If the active region is empty it FAILs before debating with an actionable message ("no active 'status: active' north-star entry … set an active entry, commit it, and approve it"). This sits alongside the existing UNSET-FAIL and placeholder-FAIL, closing the goalless-debate gap between them, and covers both the LOCAL and the FABRICA_SELF branches.

doctor.sh continues to WARN on this condition (it already did — "has no 'status: active' entry"): user-directed work stays valid; only the proactive gate FAILs.

Tests: new gate regression (10) — a committed star with no status: active heading FAILs and does not reach the Codex verdict; a normal active entry still PROCEEDs, placeholder still FAILs, UNSET still FAILs. Four existing PROCEED tests that used plain-prose stars now carry a status: active heading (their intent is "a real committed star authorizes"). Suites: gate 65 passed / 0 failed (was 62), resolver 64 passed / 0 failed (unchanged). shellcheck 0.9.0 clean (-x -S style), structure check ok, bash -n clean.

No safety rail weakened — this closes an authorization gap (goalless debate).

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 13d8981
Reviewed-base: d13b5b7

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

The implementation adds a new committed per-target north-star requirement, but several user-facing setup documents still direct users to the old control-plane file, leaving documented onboarding paths broken.

Review comment:

• [P2] Update remaining onboarding docs for target north stars — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Sw08e8cLCF/templates/faber-command.md:10-10
  This line flips the live /faber instructions to the target repo's .fabrica/north-star.md, but the files this command still treats as onboarding/source-of-truth (README.md, QUICKSTART.md, and templates/repo-setup.md) still tell users to edit the control-plane NORTH_STAR.md. A user following those documented setup paths will not commit .fabrica/north-star.md, so the new manager-review.sh gate fails before any proactive debate. Please update the remaining setup docs in the same change so every path points to the committed target north star.

[yihanzhu]

Swept the remaining onboarding/setup docs to the per-target `.fabrica/north-star.md` model so every documented path matches the already-flipped gate (Codex final [P2]). Head now `71eae3c`. Docs-only — no code/logic change.

Files swept:

• QUICKSTART.md — step 4 "Set your own north star" now points to the target repo's `.fabrica/north-star.md` (copy the `templates/.fabrica/north-star.md` template into the target, replace placeholder, drop the `fabrica-shipped-default` marker, commit); notes setup does not auto-seed it and a missing/placeholder/no-active-entry star FAILs the gate; Fabrica-self exception (root `NORTH_STAR.md`) called out.
• README.md — "Judgment lives at the direction" bullet now says you approve each target's committed `.fabrica/north-star.md` (root `NORTH_STAR.md` only when the target is this control-plane repo). Layout listing: reframed `NORTH_STAR.md` as Fabrica-self's own target file and added the restore-critical `scripts/lib/north-star.sh` resolver + `templates/.fabrica/north-star.md` template entries.
• templates/repo-setup.md — §6 rewritten: the north star lives in the target repo at `.fabrica/north-star.md` (create+commit yourself; `setup-target-repo.sh` only makes labels), gate reads the committed file, Fabrica-self exception noted.

No changes needed in RESTORE.md or the root CLAUDE.md — their only north-star mentions are the location-agnostic decision-altitude / approval-path sense ("north-star / goal drift", "approved the active north star"), which stay correct under the flip. No literal `NORTH_STAR.md` file-location onboarding refs remain there.

Verification:

• Grep across README/QUICKSTART/RESTORE/CLAUDE.md/repo-setup.md confirms no remaining stale control-plane-`NORTH_STAR.md` onboarding guidance — every literal ref now points to the per-target `.fabrica/north-star.md`, with root `NORTH_STAR.md` framed only as Fabrica-self's target file.
• Docs-only diff (no `*.sh` touched); shellcheck unaffected.
• Structure check (`ci/required-files.txt` manifest) green; resolver tests 64/64, gate tests 65/65 — unchanged.
• CI expected green.

Greenfield tie-in (bootstrap creates the target's `.fabrica/north-star.md`) and the MapleFolio history log remain 98b / #100.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 71eae3c
Reviewed-base: d13b5b7

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

No actionable correctness issues were identified in the changed scripts, tests, or documentation. The north-star resolution and gate changes appear internally consistent and covered by the added test suite.