Skip to content

feat(FAFF-382): single-source the worktree-root resolver + make the checked isolation root bind#289

Open
alechill wants to merge 2 commits into
mainfrom
faff-382-grafts-honour-the-checked-lights-out-worktree-root
Open

feat(FAFF-382): single-source the worktree-root resolver + make the checked isolation root bind#289
alechill wants to merge 2 commits into
mainfrom
faff-382-grafts-honour-the-checked-lights-out-worktree-root

Conversation

@alechill

@alechill alechill commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What & why

Closes FAFF-382. FAFF-379 made the L4 lights-out preflight verify the resolved worktree root is outside the repo tree and creatable — but nothing downstream consumed that property, and the env→config→default precedence was inlined in two places (the setup-worktree.sh hook and the preflight) that could drift.

This change:

  1. Adds faff worktree-root [--assert PATH] [--root DIR] [--json] [--selftest] — the single canonical resolver of the precedence (FAFF_WORKTREE_ROOT.faffrc worktree_root~/.faff/worktrees/<basename(repoRoot)>; env/config verbatim, only the default appends <repo>) plus a segment-aware --assert (strictly-under, reusing the checkWorktreeIsolation path.relative predicate).
  2. Refactors the two inlined copies onto it — the hook and the FAFF-379 preflight now call the shared resolveWorktreeRoot, so they never drift. This aligns a pre-existing default-prefix mismatch (the preflight previously checked ~/.faff/worktrees without the <repo> suffix) by construction.
  3. Adds a graft Step-3 assert — after EnterWorktree, graft asserts the created worktree is under the resolved root: autonomous refuses fail-closed, interactive warns. This makes the FAFF-379-verified property bind. The resolver maps a linked worktree's --root back to the main checkout (mainWorktreeRoot) so the assert never false-refuses across the dispatch.

Scope (trimmed per the spec): no BuildDispatch threading, no concurrency-executor edits — the env-only cross-dispatch case has no real operator (the global ~/.faff default is the norm), so the deferred placement work (FAFF-400) was cancelled.

Acceptance Criteria

  • A worktree outside the resolved root no longer builds silently — autonomous refuses pre-build, interactive warns.
    Verified: graft Step-3 assert prose (plugin/skills/faff-graft/SKILL.md); worktree-root --assert exits 1 on an outside path (test/worktree-root.test.mjs — "OUTSIDE … exits 1").
  • faff worktree-root prints the resolved root with precedence env→config→default; env/config verbatim, only default appends <repo>.
    Verified: test/worktree-root.test.mjs — default / env-verbatim / config-verbatim cases + --selftest.
  • faff worktree-root --assert PATH exits 0 iff PATH is strictly under the resolved root, else exit 1 with a reason naming PATH + the root.
    Verified: under (exit 0) / outside (exit 1, reason) / root-itself (exit 1) tests.
  • Registered in COMMANDS / USAGE / REGION_MAP (factory) / REGION_SELFTEST_ARGV; regions check + worktree-root --selftest pass.
    Verified: regions check PASS, regions selftest (worktree-root factory PASS, 51 members 0 failed).
  • docs/guide/cli.md documents worktree-root; lint-cli-doc passes.
    Verified: lint-cli-doc PASS (51 subcommands documented).
  • setup-worktree.sh resolves via faff worktree-root (graceful literal-default fallback), not an inline precedence block.
    Verified: hook diff.
  • The FAFF-379 preflight resolves via the same shared resolver; hook / preflight / CLI return identical paths.
    Verified: preflight refactored to resolveWorktreeRoot; single-source test (linked worktree resolves the same root as the main checkout).
  • The FAFF-379 lights-out tests stay green unchanged after the default alignment.
    Verified: node --test test/lights-out.test.mjs → 19/19 pass.
  • Graft Step 3 gains the post-entry assert; autonomous refuses fail-closed (logged), interactive warns; git rev-parse --show-toplevel is the source; a missing faff binary skips (never hard-fails).
    Verified: graft SKILL.md prose.
  • Scope guard: no BuildDispatch field added, neither concurrency executor edited.
    Verified: git diff touches only bin/faff, the hook, graft SKILL, cli.md, and the new test.
  • No LLM-judgement seam introduced (resolver + assert are deterministic).
    Verified: no grader KIND / eval-case row added.

Test evidence

  • Full node --test: 1192 pass; the 4 remaining failures (minio/holdout docker-gated + one eval real-spawn) are pre-existing — identical on the main baseline, unrelated to this change.
  • regions check, regions --selftest, regions selftest, lint-cli-doc (+ --selftest), validate-adapters: all PASS.
  • FAFF-379 tests 19/19; new test/worktree-root.test.mjs 8/8.

Review (faffter-dark-adversarial-review): pass — Phase-1 structural clean; Phase-2 adversarial findings advisory/non-gating (the one major, lexical vs mount-point containment, is a spec-mandated parity choice with checkWorktreeIsolation).

alechill added 2 commits July 7, 2026 16:48
…hecked isolation root bind

Add `faff worktree-root [--assert]` as the one canonical resolver of the
env -> .faffrc worktree_root -> ~/.faff/worktrees/<repo> precedence, and refactor
the two inlined copies (setup-worktree.sh hook + the FAFF-379 lights-out preflight)
onto it so they never drift. The default is now the repo-suffixed placement root,
aligning the preflight's prior parent-dir check by construction.

Graft Step 3 gains a post-entry assert (`worktree-root --assert`, local resolution)
that makes the FAFF-379-verified isolation property bind: a worktree landing outside
the resolved root refuses fail-closed (autonomous) / warns (interactive). The resolver
maps a linked worktree's --root back to the main checkout so the assert never
false-refuses across the dispatch.

No BuildDispatch/concurrency-executor changes (env-only cross-dispatch case is out of
scope; FAFF-400 cancelled).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant