feat(FAFF-382): single-source the worktree-root resolver + make the checked isolation root bind#289
Open
alechill wants to merge 2 commits into
Open
Conversation
…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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.shhook and the preflight) that could drift.This change:
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 thecheckWorktreeIsolationpath.relativepredicate).resolveWorktreeRoot, so they never drift. This aligns a pre-existing default-prefix mismatch (the preflight previously checked~/.faff/worktreeswithout the<repo>suffix) by construction.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--rootback to the main checkout (mainWorktreeRoot) so the assert never false-refuses across the dispatch.Scope (trimmed per the spec): no
BuildDispatchthreading, no concurrency-executor edits — the env-only cross-dispatch case has no real operator (the global~/.faffdefault is the norm), so the deferred placement work (FAFF-400) was cancelled.Acceptance Criteria
Verified: graft Step-3 assert prose (
plugin/skills/faff-graft/SKILL.md);worktree-root --assertexits 1 on an outside path (test/worktree-root.test.mjs— "OUTSIDE … exits 1").faff worktree-rootprints 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 PATHexits 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.
COMMANDS/USAGE/REGION_MAP(factory) /REGION_SELFTEST_ARGV;regions check+worktree-root --selftestpass.Verified:
regions checkPASS,regions selftest(worktree-root factory PASS, 51 members 0 failed).docs/guide/cli.mddocumentsworktree-root;lint-cli-docpasses.Verified:
lint-cli-docPASS (51 subcommands documented).setup-worktree.shresolves viafaff worktree-root(graceful literal-default fallback), not an inline precedence block.Verified: hook diff.
Verified: preflight refactored to
resolveWorktreeRoot; single-source test (linked worktree resolves the same root as the main checkout).Verified:
node --test test/lights-out.test.mjs→ 19/19 pass.git rev-parse --show-toplevelis the source; a missingfaffbinary skips (never hard-fails).Verified: graft SKILL.md prose.
BuildDispatchfield added, neither concurrency executor edited.Verified:
git difftouches onlybin/faff, the hook, graft SKILL, cli.md, and the new test.Verified: no grader
KIND/ eval-case row added.Test evidence
node --test: 1192 pass; the 4 remaining failures (minio/holdout docker-gated + one eval real-spawn) are pre-existing — identical on themainbaseline, unrelated to this change.regions check,regions --selftest,regions selftest,lint-cli-doc(+--selftest),validate-adapters: all PASS.test/worktree-root.test.mjs8/8.Review (
faffter-dark-adversarial-review): pass — Phase-1 structural clean; Phase-2 adversarial findings advisory/non-gating (the onemajor, lexical vs mount-point containment, is a spec-mandated parity choice withcheckWorktreeIsolation).