From 519e3398c9254edc171ad1416e3bcc7bf953c748 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 14 Aug 2026 08:02:18 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20bug-hunt=20round=208=20=E2=80=94=20g?= =?UTF-8?q?uided-apply=20domain=20consent,=20truthful=20capture=20summary,?= =?UTF-8?q?=20local-overlay=20terminal=20compare,=20deps=20error=20diagnos?= =?UTF-8?q?tics,=20snapshot=20containment=20guard,=20exact=20release=20ver?= =?UTF-8?q?sion=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six code/infra fixes, each adversarially confirmed before fixing: - guided apply groups risky changes per FileDomain in registry order; a two-name literal switch had bucketed keybindings/emacs/iterm2-profiles under "dotfiles", so the bulk consent named the wrong domain and one "yes" spanned a mixed bucket - capture counts a brew/npm re-dump only when the manifest bytes changed (brew bundle dump --force rewrites unconditionally, so every clean run claimed "wrote 1 change(s)" over an empty git status); the npm list is written via backup.AtomicWrite and left untouched when identical - capture compares a terminal preference domain against the local-wins source (terminalRepoStatusSource), as apply/status/diff already do; a domain captured to [l]ocal was re-offered as drifted forever - the deps install/dump rails include the package manager's combined output in their errors, matching the uninstall rail, so a failed apply --deps names the failing package instead of "exit status 1" - snapshotCurrent runs the resolved-parent containment guard, covering the exported Snapshot entry point work receive uses; a parent symlink out of $HOME let the pre-receive snapshot persist out-of-home content - the release version-stamp gate compares exactly against "ferry " in release.yml and release.sh; the dev line is next-release-plus--dev, so the substring grep passed an unstamped binary on the modal release Docs: the README/tutorial "one privileged step" claim covers the apt uninstall rail (restore --packages runs apt-get as root); the compatibility contract names pack, receive, and take-back as claim-file writers; the commands reference gains the agents parent-noun row; make preflight enters the build-and-check lists; the consistency lint's private-tier advice mandates amending committed history. Assisted-by: Claude:claude-fable-5 --- .abcd/work/DECISIONS.md | 1 + .github/workflows/release.yml | 9 ++- AGENTS.md | 1 + CHANGELOG.md | 50 ++++++++++++++++ CONTRIBUTING.md | 1 + README.md | 7 ++- cmd/capture.go | 34 ++++++++--- cmd/guided_apply.go | 41 +++++++------ cmd/terminal_status_source_test.go | 62 +++++++++++++++++++ docs/reference/commands.md | 1 + docs/reference/compatibility.md | 2 +- docs/reference/configuration.md | 4 +- docs/tutorials/getting-started.md | 5 +- internal/backup/backup_test.go | 38 ++++++++++++ internal/backup/snapshot.go | 19 ++++++ internal/deps/deps_test.go | 96 ++++++++++++++++++++++++++++-- internal/deps/dump.go | 38 ++++++++---- internal/deps/install.go | 8 +-- internal/deps/npm.go | 31 ++++++---- internal/deps/npm_status_test.go | 15 ++++- scripts/consistency-lint.sh | 2 +- scripts/release.sh | 7 ++- 22 files changed, 400 insertions(+), 72 deletions(-) create mode 100644 cmd/terminal_status_source_test.go diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index c25464e..95fcc33 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -28,3 +28,4 @@ a `.abcd/development/decisions/` ADR when it shapes architecture or is expensive - 2026-08-06: Bug-hunt round 5 (fresh hunt, 4 dimensions, 20 adversarial refuters over ~28 candidates): fixed five code defects — sync's pre-commit secret gate read porcelain -z through the TrimSpace helper (a leading-space first entry — the normal capture-then-sync state — parsed to a corrupt path and that file was silently never scanned; raw gitSync now, plus sorted deterministic clobber aborts); machine config written via temp+rename (in-place O_TRUNC destroyed it on crash/ENOSPC, and the leaf symlink was followed); work receive/restore surface held/failed locks per the shared contract (prune's retained count real); doctor reports plan-time containment refusals as [fail] (its ssh/containment invariants were vacuous over the filtered plan); agents locally-drifted guidance names ferry capture (capture-back shipped long ago; conflicts keep the repo-copy remedy). Release path: verify-macos runs vet/unit/race (a hand-pushed tag ran no macOS unit leg anywhere); auto-release runs check-plan-shipped BEFORE cutting the immutable tag and bounds the re-release retry at three failed runs (counted where called-workflow runs actually live: failed auto-release runs since the tag, plus standalone release.yml runs for the tagged commit); release.yml refuses non-semver tag names (backticks in a tag reached a make-spawned shell) and publishes the tag's CHANGELOG section as the release body; gen-checksums enumerates bin/ferry-* by the same glob the upload/attestation use (three unlinked target lists); smoke-e2e fails closed without a hash tool; install.sh renames the staged binary into place. Docs: README quickstart reaches a working push (init --github + sync; the old flow pushed a repo missing the capture), tutorial Fresh path gains remote add, wizard answers-file schema documented in the configuration reference, apply --deps wording names both dependency domains, deps/README drops the nonexistent "pinned post-install steps" mechanism, generated CLI pages show --version and init/restore positionals, in-place-use of a local worktree source stated, private companion project name redacted from this log (history retains it — owner-only to rewrite). Refuted as non-defects: interactive yes-on-conflict exiting 0 (documented attended/unattended split), undated 0.1.0/0.2.0 CHANGELOG headings (never tagged; dates would be fabricated), British CLI output (house majority), smoke-e2e single-yes (one risky group = one prompt), repoIgnoresCase via -C merged config (matches git's own semantics). Carried forward: CONTEXT.md lacks the v0.10.0 work-store invariants (mirrors AGENTS.md Boundaries — owner call), sync refuses a bare-init repo whose init.defaultBranch is not main (new lead, unadjudicated). - 2026-08-08: Bug-hunt round 6 (fresh hunt, 4 dimensions, 4 hunters; 29 candidates, one independent adversarial refuter each — 9 confirmed, 20 refuted). Fixed five code/infra defects: a fresh config repo is pinned to the managed branch (bare `git init` honours the machine's `init.defaultBranch`, git's own default being `master`, while sync integrates and pushes `main` and refuses anything else — so `init --github` succeeded and every later `ferry sync` failed; the eval fixtures forced `-b main` instead of driving `ferry init`, so no test could see it); `init`/`bundle import` carry the machine-scoped fields across config.toml's wholesale rewrite (a plain `ferry init` re-run dropped `managed` and erased the `[work]` cargo store, silently un-configuring sync and every work verb — `[work]` now survives every route, `managed` only when the run reuses that repo); the pre-commit secret gate consumes a rename/copy origin field as parseStatusZ does (a 4-byte-or-longer origin was sliced as a status entry, so a staged `git mv src/dotfiles …` read as `/dotfiles`, hit EISDIR against the repo's own `dotfiles/` and fail-closed the whole sync on a path the user never touched); `appendLineOnce` normalises a missing trailing newline (an append onto an unterminated `.git/info/exclude` fused the two patterns — destroying the user's last ignore rule AND silently voiding ferry's — while scaffold printed success), and reports its close error; release.yml passes `--prerelease` for a suffixed tag (the tag gate deliberately admits one and both scripts skip cleanly so the run goes green, but the publish marked it a full release, making a rehearsal tag the repo's `latest` — the exact URL install.sh serves). Hardening: the two boundary-refusal message bodies move to internal/dotfile as `RefusalSSHBody`/`RefusalEscapeBody`, so the five plan packages and doctor's invariant matcher share one definition, plus a consistency-lint rule that fails on any re-inlined copy (a reword in one package silently turned a security-invariant [fail] into a [pass], with no compile error and nothing red). Docs: the tutorial's fresh flow stages with `add -A` before committing (`commit -am` never staged capture's new files, so the push published a repo missing the capture); the agents drift guidance names `ferry capture` first and drops the false "backed up, reversible" claim for `--force` (the write-once baseline holds the pre-ferry file, so restore cannot return a force-overwritten live edit). Decision log: four 2026-07-06 entries gained the `- ` marker they lacked, which made them CommonMark lazy continuations absorbed into the preceding bullet. Refuted as non-defects: the tutorial's "capture can push / apply can pull" purpose sentence (the same section attributes both to `ferry sync` 30 lines later, and the prerequisites table states the split up front); `.abcd/.work.local/` ignored via `.git/info/exclude` rather than `.gitignore` (ADR 0002 chooses this verbatim — "never `.gitignore`" — and release.sh returns early when NEXT.md is absent, so the claimed dirt chain cannot form); smoke-e2e.sh being CI-unwired (both assertions it claims as unique are covered more strictly by CI-run evals — restore byte-identity WITH mode in restore_test.go, and the ~/.ssh tripwire across the whole surface in safety_test.go plus a strace no-open gate); the auto-release three-failure bound counting transient failures (documented in cutting-a-release.md, recoverable by re-running, and strictly better than unbounded retry on an immutable tag); AGENTS.md:58 and CONTEXT.md:24 "repo-authoritative" (every clause is literally true — apply still skips drift — and configuration.md's emacs entry proves the formula carries no claim about capture, needing a separate "no capture pass" sentence); the plans' frontmatter divergence (check-plan-shipped.sh greps `Status:` and matches both forms; the README says "for example"); `ABCDevelopment` in six archival lines (AGENTS.md's rule is scoped to naming the AI assistant, the sibling `abcd-cli` repo is public, and scrubbing archival records is an owner call); five code nitpicks (restore's package count, Prune's partial report, the status.go errors.Is marker, config.toml's 0644, and .gitignore's create-only perm) and nine infra/docs nitpicks (auto-merge timeout — reusable-workflow calls cannot take one either, so the premise was false; the actions:read comment; status=failure under-counting, which loosens not tightens; the START_SHA tripwire, which is documented fail-closed; the public/private comments; the CLI-page naming column, whose two-level pattern reads correctly; the drift page's link blurb; the .work.local link, deliberate and self-describing; ADR 0003's heading depth, which MADR permits). - 2026-08-13: Bug-hunt round 7 (fresh hunt, 4 dimensions, 4 hunters; 23 unique candidates, one independent adversarial refuter each — 19 confirmed, 4 refuted). Fixed nine substantive defects: every work verb refuses a cargo store that is or resolves into `~/.ssh` before any read/write on the path (the store was the one configurable, ferry-written path with no ssh guard — `work pack` created a world-writable directory inside `~/.ssh` and wrote bundles there, while the guarded repo/`--out` paths prove hand-edited config is in the threat model); the pre-commit secret gate walks a collapsed untracked directory (`?? dir/` at default -unormal EISDIR'd the fail-closed read, wedging every sync after a capture created a new repo subdirectory, with advice a directory can never satisfy — the sibling backup pass already walked exactly this shape, and the round-3 test dodged it with a staged `add -A`); `ferry init ` wiring the already-configured repo carries `managed` (the round-6 carry keyed on the no-arg route, so `ferry init .` from inside the configured repo dropped the flag with no supported way back — `init --github` refuses while a repo is configured; the carry is gated on the route having WIRED a pre-existing worktree, because a clone or fresh seed can land on the recorded path after the old repo was deleted and must never inherit the flag — the final dual review caught exactly that hole in this round's first cut, both reviewers NO-SHIP, fixed in the single remediation round with an eval pinning the recreated-at-same-path case); `bundle import` pins HEAD to the managed branch before its initial commit (the same unpinned `git init` round 6 fixed for the fresh path, left on the import route); `init --github`'s success banner and existing-repo hint name `ferry sync` as the publish verb (the flow's final instruction claimed capture pushes and apply pulls — capture never commits or pushes, apply never touches the network, and the banner is the last thing the flow prints); drift/conflict guidance is keyed on the registry's Captures() flag, so status stops steering terminals/keybindings/emacs/iterm2-profiles at `ferry capture` and iterm2-profiles stops falling through apply/diff's hand-maintained domain list to the dotfiles wording (an existing eval forbade exactly this wording for diff but never ran status); `apply --deps` states when a failed installed-set snapshot suppressed the restore-record (the fail-closed suppression printed the same "installed 0 package(s)" as a benign re-run, and restore --packages silently lost the run); release.yml asserts the built binary reports the tag before checksumming (Go's linker silently drops -X for a moved symbol, so a rename would publish -dev-reporting binaries with valid checksums and provenance, nothing red) and verifies the published assets against the published checksums.txt after the release (the pairing install.sh depends on; runs on private repos where the attestation check is gated off); consistency-lint invariant 5 fails if `.abcd/.work.local/` is ever tracked, and `make preflight` runs the lint so the pre-push hook catches it before publication (the exclude rule is per-clone and absent in a fresh checkout, so `git add -A` staged the private tier with every gate green — ADR 0002's exclude-not-gitignore choice honoured, enforcement added where none existed). Docs: the compatibility contract covers the four work-domain versioned files (including the machine-crossing cargo manifest) and states their unversioned-is-refused rule plus the no-retention policy for journal runs and snapshots; README/tutorial scope the "never requires sudo" claim to name the apt `apply --deps` exception; doctor's synopsis says git (required) vs zsh/package manager (recommended); install.sh documents FERRY_VERSION and its pipe-placement footgun; evals README lists init_preserve_test.go; the drift page's remedy bullet covers all four repo-authoritative domains; the stale lint step labels name all the invariants; prune-releases refuses to prune when the just-published release is missing from a listing whose fetch failure the empty-grep guard would otherwise swallow. Refuted as non-defects: Brewfile inline-comment rejection (CHANGELOG documents the fail-closed trade-off verbatim, and comment-stripping would regress a legal `#` inside an option value); the release verify 20-minute timeout (measured runs finish in ~2 minutes — 9x headroom — and the burnt-version consequence is documented and one-click recoverable); the refusal-constant concatenation (vet folds constant concatenation and the const-ness is compile-enforced); FERRY_NO_NETWORK's absent header doc (a contract-optional test hook that installs nothing). Carried forward: capture's own completion hint never names `ferry sync` for a managed repo; installApt/installBrew swallow the package manager's stderr (exit-status-only errors); the round-6 `carryMachineScoped` sites now have eval coverage but the unit seam stays untested; work-domain state growth has no pruner (documented this round as deliberate for snapshots; journal runs are inert dead bytes a future keep-last-N sweep could clear); guided_apply buckets the repo-authoritative domains under a "dotfiles" label. +- 2026-08-14: Bug-hunt round 8 (fresh hunt, 4 dimensions, 4 hunters; 22 unique candidates, one independent adversarial refuter each — 12 confirmed, 12 refuted, 2 hunter-self-refuted). Fixed six code defects: guided apply's risky walkthrough groups every FileDomain under its own name in registry order (a two-name literal switch predating fn-5 dropped keybindings/emacs/iterm2-profiles into the "dotfiles" bucket, so the group header and the bulk consent named a domain the user was not reviewing and one "yes" spanned the mixed bucket); capture's brew/npm re-dumps count a change only when the manifest's bytes actually changed (`brew bundle dump --force` rewrites unconditionally, so every clean re-run claimed "wrote 1 change(s)" over an empty `git status` and "nothing has drifted" was unreachable on any machine managing either domain), with the npm list written via backup.AtomicWrite and left untouched when identical; capture's terminal preference-domain compare resolves the local overlay first via terminalRepoStatusSource, exactly as apply/status/diff do (a domain captured to [l]ocal was re-offered as drifted forever, and re-accepting it shared would promote machine-divergent settings); the deps install/dump rails carry the package manager's combined output in their errors as the uninstall rail already did (a failed apply --deps surfaced as a bare "exit status 1"); backup.Engine snapshots run the resolved-parent containment guard inside snapshotCurrent so the exported Snapshot used by work receive is guarded identically to restore (a parent symlink out of $HOME let the pre-receive snapshot read and persist out-of-home content the write boundary then refused); the release version-stamp gate compares exactly against "ferry " in release.yml and release.sh (the dev line is next-release-plus--dev, so the substring grep passed an unstamped binary on precisely the modal release). Docs: the README/tutorial "one privileged step" claim covers restore --packages (the apt uninstall rail runs apt-get as root, per the code's own comments, and no user-facing page said so); the compatibility contract's claim-file row names pack, receive, and take-back as writers (receive.AppendClaim writes claim..json); the commands reference gains the agents parent-noun row; make preflight enters AGENTS.md/CONTRIBUTING.md; the consistency lint's private-tier advice mandates amending committed history (git rm --cached alone greens the gate while the push still publishes). Refuted as non-defects: the terminal-capture placeholder plaintext leak (placeholders exist only for content GateValue already rates High, and the re-export re-trips the gate); the hand-over sudo mkdir (round-1 adjudication stands — a host-OS setup example, not a ferry operation); the release how-to's five-step summary (explicitly non-exhaustive by its own structure); +build-metadata tags becoming latest (SemVer-correct and deliberate per the workflow's own comment); the attestation docs' missing visibility caveat (true of everything a reader can download); the retention-policy scoping, [work]-in-ferry.toml, topic-plan lifecycle, behavioral-in-evals-README, "works today" framing, attribution-rule carve-out (rule already scoped to credit; amending owner policy is an owner call), the pre-push hook's absent in-repo installation (dispatcher is deliberately external; hazard population equals hook population), and a range-based invariant-5 check (breaks CI's detached checkout — the advice-text fix shipped instead). Carried forward: capture's completion hint still never names ferry sync for a managed repo (adjudicated a nitpick, unfixed this round); the managed carry stays path-keyed (an origin-URL comparison would close it); guided-apply's per-domain groups have no direct interactive eval; the darwin-only defaults export has no injectable seam, so the terminal-capture fix is pinned at the terminalRepoStatusSource seam rather than end-to-end. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 836b67a..70abd5a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -334,12 +334,15 @@ jobs: # version — with valid checksums and provenance over the wrong artifact — # and nothing else in this workflow would go red. Same assertion as the # recovery driver (scripts/release.sh); the runner is x64 Linux, so the - # linux-amd64 binary is directly executable. + # linux-amd64 binary is directly executable. The compare is EXACT, not a + # substring: the dev line is the next release plus -dev (v0.11.0-dev), so + # an unstamped binary's output CONTAINS the very tag being released and a + # substring match would green on exactly the failure this step guards. run: | set -euo pipefail reported="$(./bin/ferry-linux-amd64 version)" - if ! printf '%s' "$reported" | grep -qF "$TAG"; then - echo "release: built binary reports '$reported', not '$TAG' — the version stamp is broken" >&2 + if [ "$reported" != "ferry $TAG" ]; then + echo "release: built binary reports '$reported', not 'ferry $TAG' — the version stamp is broken" >&2 exit 1 fi diff --git a/AGENTS.md b/AGENTS.md index 7704447..e92730d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,6 +18,7 @@ go vet ./... # static checks go test ./... # full unit + eval suite (evals skip when FERRY_BIN is unset) go test ./internal/agents/ # a single package go test -run TestResolve ./internal/agents/ # a single test +make preflight # the pre-push gate: build, vet, unit + race tests, consistency lint ``` The evals drive the real binary, so they need `FERRY_BIN` pointing at this diff --git a/CHANGELOG.md b/CHANGELOG.md index 3015182..d7064d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -369,6 +369,56 @@ called out in a **Breaking** section. See cannot bring a force-overwritten live edit back. The guidance now names `ferry capture` first, as `status` and `apply` already do, and states plainly what `--force` discards. +- **Guided apply names the domain it asks consent for.** The interactive + walkthrough grouped risky changes by a hand-maintained two-name list, so + keybindings, emacs, and iTerm2 Dynamic Profile changes fell into the + `dotfiles` bucket: the group header and the bulk "Apply all N change(s) in + dotfiles?" prompt named a domain the user was not reviewing, and one "yes" + covered the mixed bucket. Every domain now gets its own group under its own + name, in registry order. +- **`ferry capture` reports deps drift truthfully.** The Homebrew and npm + manifest re-dumps counted every successful dump as a captured change — + `brew bundle dump --force` rewrites unconditionally — so a clean re-run + printed "wrote 1 change(s) into the repo. Review with `git status`…" over an + empty `git status`, and "nothing has drifted" was unreachable on any machine + managing either domain. Both re-dumps now compare bytes and count only real + changes; the npm list is also written crash-safely (temp + rename) instead + of truncate-in-place, and is left untouched when it already matches. +- **A captured-to-local terminal domain reads as clean.** Capture compared the + live iTerm2/Apple Terminal export only against the shared repo plist, while + `apply`, `status`, and `diff` all resolve the per-machine local overlay + first. After routing a terminal capture to `[l]ocal`, every later capture + re-offered the identical domain as drifted (and re-accepting it as shared + would promote machine-divergent settings into the shared repo) while + `status` reported it clean. Capture now compares against the same + local-wins source the other commands use. +- **Failed dependency installs name the failure.** The brew/apt/npm install + and dump rails discarded the package manager's combined output on error, so + a failed `apply --deps` aborted with a bare `exit status 1` and no way to + tell which package or tap failed. The manager's own diagnostics now ride the + error, as the uninstall rail's errors already did. +- **Pre-receive snapshots are containment-guarded.** The work domain's + per-receive snapshot read its target paths before any guard ran, so a parent + directory symlinked out of `$HOME` let the snapshot read and persist + out-of-home content into ferry's snapshot store — content the write boundary + downstream refuses to touch. The snapshot now runs the same resolved-parent + containment guard as restore, refusing before anything is read or written. +- **The release version-stamp gate is an exact compare.** Both the release + workflow and `scripts/release.sh` checked the built binary's reported + version with a substring match; the development line is the next release + plus `-dev` (`v0.11.0-dev` contains `v0.11.0`), so an unstamped binary + passed the gate on exactly the modal release it guards. Both now require + the output to equal `ferry ` verbatim. +- **Docs corrections.** The "one privileged step" claim in the README and + tutorial now covers both halves of the apt rail — `restore --packages` + uninstalls as root exactly as `apply --deps` installs — and the apt + reference section says so; the compatibility contract states that claim + files are written by pack, receive, and take-back alike (each account only + its own), not by pack alone; the commands reference gains the missing + `agents` parent-noun row; `make preflight` appears in the build-and-check + lists; and the consistency lint's private-tier advice now says committed + history must be amended, since `git rm --cached` alone greens the gate + while the push still publishes the file. ## [0.10.0] - 2026-07-19 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e6fda85..811b7bb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,6 +13,7 @@ make build # cross-compiles bin/ferry-- (there is no plain gofmt -l . # format gate: any output names a file needing `gofmt -w` go vet ./... # static checks go test ./... # full unit + eval suite (evals skip when FERRY_BIN is unset) +make preflight # the pre-push gate: build, vet, unit + race tests, consistency lint ``` The eval suite drives the real binary, so it needs `FERRY_BIN` pointing at this diff --git a/README.md b/README.md index 69c3691..b3d1232 100644 --- a/README.md +++ b/README.md @@ -98,8 +98,9 @@ The documentation follows [Diátaxis](https://diataxis.fr/); the map is in blank one without `--force`, so your config is never silently erased. - **No admin assumed**: ferry installs to `~/.local/bin` and needs no `sudo` or root for its own operation, so it works on any account, including locked-down or managed - machines. The one exception is the opt-in `apply --deps` step on apt-based Linux, - which delegates to your package manager and must itself run under `sudo`. ferry never - edits your shell on its own. + machines. The one exception is the opt-in apt rail on Debian-style Linux — `apply + --deps` (installs) and `restore --packages` (uninstalls what `--deps` recorded) — + which delegates to your package manager and must itself run under `sudo`. ferry + never edits your shell on its own. - **Safe with secrets**: ferry never touches `~/.ssh/`. SSH keys and other secrets are handled out-of-band and never committed. See [SSH](docs/explanation/ssh.md). diff --git a/cmd/capture.go b/cmd/capture.go index cb2d3ed..3a2fddc 100644 --- a/cmd/capture.go +++ b/cmd/capture.go @@ -214,7 +214,10 @@ func runCapture(c *cobra.Command, _ []string) error { // ran only after a non-deps change was accepted, so a deps-only machine reported // "nothing has drifted" and never re-dumped.) reDumpDeps reports the file it // wrote and counts as both an offered AND a captured change so the summary below - // reflects it. A missing/out-of-scope manager is a clean skip (no offer). + // reflects it — but ONLY when the dump actually changed the manifest's bytes: + // `brew bundle dump --force` rewrites unconditionally, so counting every + // successful dump made a clean re-run claim "wrote 1 change(s)" over an empty + // `git status`. A missing/out-of-scope manager is a clean skip (no offer). if reDumpDeps(ctx, out) { offered++ captured++ @@ -1064,10 +1067,18 @@ func captureTerminalDomain(cc captureCtx, domain string) (wrote bool, offered bo return false, false, err } - // Only offer when the live export actually DIFFERS from the committed repo copy - // (don't offer a no-op). An absent repo copy is itself a difference (capture - // would create it). - repoBytes, _ := os.ReadFile(repoDest) + // Only offer when the live export actually DIFFERS from the bytes apply would + // deploy (don't offer a no-op). Compare against the LOCAL-WINS source exactly + // as apply and status resolve it (terminalRepoStatusSource): a domain captured + // to the [l]ocal overlay must read as clean on the next run — comparing only + // the shared copy re-offered an already-captured domain forever, while status + // reported it clean. An absent repo copy is itself a difference (capture would + // create it). + compareSrc := terminalRepoStatusSource(cc.repoPath, domain, prefID) + if _, err := safeRepoPath(cc.repoPath, compareSrc); err != nil { + return false, false, err + } + repoBytes, _ := os.ReadFile(compareSrc) if domain == "iterm2" { // Compare LIKE-FOR-LIKE: filter the repo side to the same allowlist so a repo // plist that happens to carry stale volatile keys never registers as drift. @@ -1516,12 +1527,17 @@ func reDumpDeps(ctx *cmdContext, out io.Writer) bool { fmt.Fprintf(out, "deps: skipped manifest re-dump (%v)\n", err) return false } - path, err := deps.ReDumpManifest(depsDir, deps.ExecRunner{}) + path, changed, err := deps.ReDumpManifest(depsDir, deps.ExecRunner{}) if err != nil { // No manager / unsupported dump: report briefly, never fail capture. fmt.Fprintf(out, "deps: skipped manifest re-dump (%v)\n", err) return false } + if !changed { + // The dump ran but produced the bytes already committed: no drift, so + // nothing is counted and the summary can truthfully say nothing changed. + return false + } fmt.Fprintf(out, "deps: re-dumped manifest %s\n", relTo(ctx.RepoPath, path)) return true } @@ -1550,11 +1566,15 @@ func reDumpNpmGlobals(ctx *cmdContext, out io.Writer) bool { fmt.Fprintf(out, "npm-globals: skipped manifest re-dump (%v)\n", err) return false } - path, err := deps.ReDumpNpmGlobals(depsDir, deps.ExecRunner{}) + path, changed, err := deps.ReDumpNpmGlobals(depsDir, deps.ExecRunner{}) if err != nil { fmt.Fprintf(out, "npm-globals: skipped manifest re-dump (%v)\n", err) return false } + if !changed { + // Committed list already matches this machine: no drift, nothing counted. + return false + } fmt.Fprintf(out, "npm-globals: re-dumped %s\n", relTo(ctx.RepoPath, path)) return true } diff --git a/cmd/guided_apply.go b/cmd/guided_apply.go index 7d65270..15e18e1 100644 --- a/cmd/guided_apply.go +++ b/cmd/guided_apply.go @@ -33,6 +33,7 @@ import ( "io" "os" "path/filepath" + "slices" "sort" "strings" @@ -264,29 +265,33 @@ type riskyGroup struct { items []planItem } -// groupRisky buckets risky items by domain (dotfiles, then agents, then -// terminals) so the walkthrough confirms a domain wholesale or drills into it. +// groupRisky buckets risky items by FileDomain, in registry order, so the +// walkthrough confirms a domain wholesale or drills into it. Every domain gets +// its own group under its own name: a hand-maintained name list here once let +// keybindings, emacs, and iterm2-profiles fall into the "dotfiles" bucket, so +// the group header and the bulk "yes" consent named a domain the user was not +// reviewing. func groupRisky(risky []planItem) []riskyGroup { - var dot, ag, term []planItem + byDomain := make(map[string][]planItem) + var order []string + for _, fd := range buildRegistry(nil).FileDomains { + order = append(order, fd.Name()) + } for _, it := range risky { - switch it.fileDomain { - case "agents": - ag = append(ag, it) - case "terminals": - term = append(term, it) - default: - dot = append(dot, it) + name := it.fileDomain + if name == "" { + name = "dotfiles" + } + if _, seen := byDomain[name]; !seen && !slices.Contains(order, name) { + order = append(order, name) } + byDomain[name] = append(byDomain[name], it) } var groups []riskyGroup - if len(dot) > 0 { - groups = append(groups, riskyGroup{"dotfiles", dot}) - } - if len(ag) > 0 { - groups = append(groups, riskyGroup{"agents", ag}) - } - if len(term) > 0 { - groups = append(groups, riskyGroup{"terminals", term}) + for _, name := range order { + if items := byDomain[name]; len(items) > 0 { + groups = append(groups, riskyGroup{name, items}) + } } return groups } diff --git a/cmd/terminal_status_source_test.go b/cmd/terminal_status_source_test.go new file mode 100644 index 0000000..eaf5d32 --- /dev/null +++ b/cmd/terminal_status_source_test.go @@ -0,0 +1,62 @@ +package cmd + +import ( + "os" + "path/filepath" + "testing" +) + +// terminalRepoStatusSource is the single local-wins resolution both status AND +// capture compare the live preference-domain export against (apply's +// terminalExportBlob mirrors it). Capture once compared only the shared copy, +// so a domain captured to the [l]ocal overlay was re-offered as drifted on +// every later run while status reported it clean — these tests pin the shared +// seam that fix relies on. +func TestTerminalRepoStatusSourcePrefersLocalOverlay(t *testing.T) { + repo := t.TempDir() + local := filepath.Join(repo, "local", "iterm2", "com.googlecode.iterm2.plist") + if err := os.MkdirAll(filepath.Dir(local), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(local, []byte("overlay"), 0o644); err != nil { + t.Fatal(err) + } + got := terminalRepoStatusSource(repo, "iterm2", "com.googlecode.iterm2") + if got != local { + t.Errorf("with a local overlay present, source = %q, want the overlay %q", got, local) + } +} + +func TestTerminalRepoStatusSourceFallsBackToShared(t *testing.T) { + repo := t.TempDir() + for domain, shared := range map[string]string{ + "iterm2": filepath.Join(repo, "iterm2", "com.googlecode.iterm2.plist"), + "terminal": filepath.Join(repo, "terminal", "com.apple.Terminal.plist"), + } { + id := filepath.Base(shared) + id = id[:len(id)-len(".plist")] + if got := terminalRepoStatusSource(repo, domain, id); got != shared { + t.Errorf("%s: with no local overlay, source = %q, want shared %q", domain, got, shared) + } + } +} + +func TestTerminalRepoStatusSourceIgnoresSymlinkedOverlay(t *testing.T) { + repo := t.TempDir() + outside := filepath.Join(t.TempDir(), "elsewhere.plist") + if err := os.WriteFile(outside, []byte("outside"), 0o644); err != nil { + t.Fatal(err) + } + localDir := filepath.Join(repo, "local", "iterm2") + if err := os.MkdirAll(localDir, 0o755); err != nil { + t.Fatal(err) + } + link := filepath.Join(localDir, "com.googlecode.iterm2.plist") + if err := os.Symlink(outside, link); err != nil { + t.Fatal(err) + } + shared := filepath.Join(repo, "iterm2", "com.googlecode.iterm2.plist") + if got := terminalRepoStatusSource(repo, "iterm2", "com.googlecode.iterm2"); got != shared { + t.Errorf("symlinked overlay must be refused, source = %q, want shared %q", got, shared) + } +} diff --git a/docs/reference/commands.md b/docs/reference/commands.md index 6212ea0..6539583 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -27,6 +27,7 @@ Every command is run as `ferry ` (e.g. `ferry init`). | `work status []` | Show the project's cargo, both accounts' claims, handover-marker freshness ("handed over, not modified since" vs modified after), drift against this account's baseline, store size, and other projects present in the store. Read-only. | | `work prune []` | Apply keep-last-N retention now (`--keep `, default from the `[work]` table, else 5), or remove one bundle exactly with `--bundle `. | | `work restore ` | Revert exactly the last `work receive` on this account from its per-receive snapshot. (`ferry restore work` instead reverts **all** work-verb writes to the pre-ferry baseline.) | +| `agents` | Parent noun for onboarding project repos and migrating agent-instruction setups. Its subcommands are `agents scaffold` and `agents adopt`. | | `agents scaffold` | Set up a project repo for AI-agent work from the templates in the config repo's `agents/` area: an `AGENTS.md` router, `CLAUDE.md`/`GEMINI.md` bridges, and committed `.abcd/work/` memory files. `--private` instead creates a `.abcd/.work.local/` layer hidden via `.git/info/exclude` — for repos you don't own, it leaves zero tracked trace. `--attribution` (mutually exclusive with `--private`) instead installs a `prepare-commit-msg` hook that appends a kernel-style `Assisted-by:` trailer to agent-authored commits, for repos that require AI disclosure. Idempotent; never overwrites or repoints anything it didn't create. Works in linked worktrees and submodules. | | `agents adopt` | One-time migration of an existing symlink-based instruction setup into the config repo: imports the source files (never modifying the source directory), then swaps each `$HOME` bridge symlink for a ferry-managed copy in a single journalled transaction — any failure rolls back and the symlinks return. Refuses directory-level bridges with exact instructions rather than writing through them. | | `version` | Print the version; `--verbose` adds the Go version and platform. | diff --git a/docs/reference/compatibility.md b/docs/reference/compatibility.md index c4aceee..14c9a16 100644 --- a/docs/reference/compatibility.md +++ b/docs/reference/compatibility.md @@ -54,7 +54,7 @@ The versioned files are: | `agents-targets.json` | Every `$HOME` destination the agents domain has applied on this machine, so `ferry restore agents` reverts what was actually applied. | | `journal//manifest.json` | One apply run's record of prior states and actions, used to roll back an interrupted run. | | `work/.json` | Per-project work-domain state on this machine: the last pack and receive this account performed. | -| The cargo bundle's `ferry-work.json` manifest and the store's `claim..json` files | Written by `ferry work pack` into the configured cargo store (the manifest as a member of each `.ferrywork` bundle) and read by `ferry work receive` — often by a *different* account or machine, which makes their version envelope the most compatibility-relevant in ferry. | +| The cargo bundle's `ferry-work.json` manifest and the store's `claim..json` files | The manifest is written by `ferry work pack` (as a member of each `.ferrywork` bundle) and read by `ferry work receive`; the claim files are written by pack, receive, *and* take-back alike — each account appends only to its own `claim..json`, merged on read by receive and `work status`. These files routinely cross to a *different* account or machine, which makes their version envelope the most compatibility-relevant in ferry. | | `.abcd/.work.local/.ferry-handover.json` | The project-side handover marker pack records (never a cargo-store file); receive consumes and removes it. | The immutable **baseline** (ferry's record of true pre-ferry state, which diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 384f03a..b9f706c 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -504,7 +504,9 @@ a plain Debian package name before anything runs: letters, digits and `+ - . : ~` only, starting with a letter or digit, and a trailing `-` is refused (apt reads it as the remove modifier). Like Homebrew, this is install/reconcile-only — `apply --deps` never uninstalls a package the manifest -omits. +omits. The apt uninstall rail, `restore --packages`, removes only the packages +ferry recorded as self-installed and, like the install, runs `apt-get` as root — +invoke it under `sudo` on apt machines. ### npm globals diff --git a/docs/tutorials/getting-started.md b/docs/tutorials/getting-started.md index feeefba..e6d1b72 100644 --- a/docs/tutorials/getting-started.md +++ b/docs/tutorials/getting-started.md @@ -18,8 +18,9 @@ the work it deliberately does **not** reimplement: You do **not** need admin/root, and you do not need to pre-install anything ferry manages: that's ferry's job. The above are the host tools ferry stands on. (The -one privileged step is `apply --deps` on apt-based Linux, which delegates to -`apt-get` and must itself run under `sudo`; Homebrew needs none.) +one privileged surface is the apt rail on apt-based Linux — `apply --deps` to +install, and `restore --packages` to uninstall what `--deps` recorded — which +delegates to `apt-get` and must itself run under `sudo`; Homebrew needs none.) > **Linux scope.** The core (dotfiles, dependencies, backup/restore) is cross-platform > and CI-tested on Linux, as are the config-file terminal emulators (Alacritty, kitty, diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index 9f1819f..5827570 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -1211,6 +1211,44 @@ func asErrLockHeld(err error, target **ErrLockHeld) bool { return false } +// TestSnapshotRefusesParentSymlinkEscapingHome: the exported Snapshot entry +// point (used by `ferry work receive`) must run the same resolved-containment +// guard restorePaths applies BEFORE the snapshot reads anything. A parent +// symlink under $HOME that resolves outside it would otherwise let the +// snapshot read and persist out-of-home content into the snapshot store — +// content the downstream write-boundary guard refuses to touch. +func TestSnapshotRefusesParentSymlinkEscapingHome(t *testing.T) { + e, home := homeEngine(t) + outside := t.TempDir() + + // ~/synced -> , with a real file behind the link. + if err := os.MkdirAll(filepath.Join(outside, "proj"), 0o755); err != nil { + t.Fatal(err) + } + secret := filepath.Join(outside, "proj", "memory.json") + mustWrite(t, secret, []byte("OUT-OF-HOME"), 0o600) + if err := os.Symlink(outside, filepath.Join(home, "synced")); err != nil { + t.Fatal(err) + } + + target := filepath.Join(home, "synced", "proj", "memory.json") + if _, err := e.Snapshot([]string{target}); err == nil { + t.Fatal("Snapshot through a home-escaping parent symlink: want refusal, got nil") + } + + // Nothing may have been persisted: the snapshot store must hold no blobs. + var blobs []string + _ = filepath.WalkDir(e.snapshotDir, func(p string, d os.DirEntry, err error) error { + if err == nil && !d.IsDir() { + blobs = append(blobs, p) + } + return nil + }) + if len(blobs) != 0 { + t.Errorf("refused snapshot persisted files: %v", blobs) + } +} + func TestSnapshotIsDirectlyReversible(t *testing.T) { e, home := newEngine(t) present := filepath.Join(home, "present") diff --git a/internal/backup/snapshot.go b/internal/backup/snapshot.go index 8b6e978..19aee81 100644 --- a/internal/backup/snapshot.go +++ b/internal/backup/snapshot.go @@ -25,7 +25,26 @@ type snapshot struct { // snapshotCurrent captures the current state of each path into a new snapshot // and returns its ID. Empty input yields an (empty) snapshot — restore always // produces a recoverable point even when there is nothing to revert. +// +// Every REAL path's resolved parent chain is containment-guarded BEFORE +// anything is read or created: captureForSnapshot -> captureState uses +// os.Lstat+os.ReadFile, which FOLLOW intermediate parent symlinks, so an +// unguarded snapshot would read and persist content from outside $HOME (or +// under ~/.ssh) that the write-boundary guard downstream would refuse to +// touch. Guarding HERE covers both entry points identically — restorePaths +// (which pre-filters refusals into its skip list, so its paths re-pass) and +// the exported Snapshot used by `ferry work receive`, which previously reached +// this read with no guard at all. Resource paths are synthetic (no filesystem +// chain) and are captured via their hooks. func (e *Engine) snapshotCurrent(absPaths []string) (string, error) { + for _, p := range absPaths { + if isResourcePath(p) { + continue + } + if err := guardResolvedContainment(p); err != nil { + return "", fmt.Errorf("snapshot refused for %s: %w", p, err) + } + } id := newRunID() dir := filepath.Join(e.snapshotDir, id) if err := os.MkdirAll(dir, dirPerm); err != nil { diff --git a/internal/deps/deps_test.go b/internal/deps/deps_test.go index 891ac85..6ac88f4 100644 --- a/internal/deps/deps_test.go +++ b/internal/deps/deps_test.go @@ -220,6 +220,40 @@ func TestInstallBrew_RecordsInstalledSet(t *testing.T) { } } +// diagRunner fails any call whose joined args contain failOn, returning the +// manager's diagnostic output ALONGSIDE the error — the shape CombinedOutput +// produces for a real failed brew/apt/npm run. +type diagRunner struct { + failOn string + diag string +} + +func (d *diagRunner) Run(args ...string) (string, error) { + if strings.Contains(strings.Join(args, " "), d.failOn) { + return d.diag, errors.New("exit status 1") + } + return "", nil +} + +// TestInstallBrew_FailureSurfacesManagerOutput: when the bundle fails, the +// returned error must carry brew's own diagnostics — a bare "exit status 1" +// leaves the user with no way to know which package or tap failed. +func TestInstallBrew_FailureSurfacesManagerOutput(t *testing.T) { + dir := t.TempDir() + shared := filepath.Join(dir, "Brewfile.darwin") + writeFile(t, shared, "brew \"nonexistent-formula\"\n") + m := Manifest{Manager: platform.ManagerBrew, GOOS: "darwin", Shared: shared} + + const diag = "Error: No available formula with the name \"nonexistent-formula\"" + _, err := install(m, &diagRunner{failOn: "bundle", diag: diag + "\n"}) + if err == nil { + t.Fatal("install with failing bundle: want error") + } + if !strings.Contains(err.Error(), diag) { + t.Errorf("install error must surface brew's diagnostics, got: %v", err) + } +} + // growingRunner returns `before` for brew list until a bundle runs, then `after`. type growingRunner struct { before, after string @@ -584,7 +618,7 @@ func TestReDump_Brew_TargetsOnlyDetectedFile(t *testing.T) { Local: filepath.Join(depsDir, "Brewfile.darwin.local"), } r := newFakeRunner() - got, err := reDump(m, r) + got, _, err := reDump(m, r) if err != nil { t.Fatalf("reDump brew: %v", err) } @@ -625,7 +659,7 @@ func TestReDump_Brew_RefusesSymlinkTarget(t *testing.T) { m := Manifest{Manager: platform.ManagerBrew, GOOS: "darwin", Shared: target} r := newFakeRunner() - if _, err := reDump(m, r); err == nil { + if _, _, err := reDump(m, r); err == nil { t.Fatal("reDump with symlink target: want refusal error, got nil") } if r.invoked("bundle dump") || r.invoked("dump") { @@ -649,7 +683,7 @@ func TestReDump_Brew_AbsentTargetDumps(t *testing.T) { target := filepath.Join(depsDir, "Brewfile.darwin") m := Manifest{Manager: platform.ManagerBrew, GOOS: "darwin", Shared: target} r := newFakeRunner() - got, err := reDump(m, r) + got, _, err := reDump(m, r) if err != nil { t.Fatalf("reDump absent target: %v", err) } @@ -661,10 +695,62 @@ func TestReDump_Brew_AbsentTargetDumps(t *testing.T) { } } +// brewWritingRunner fakes `brew bundle dump` by writing body to the --file= +// target, so the before/after change detection sees what brew would produce. +type brewWritingRunner struct { + body string + calls [][]string +} + +func (r *brewWritingRunner) Run(args ...string) (string, error) { + r.calls = append(r.calls, args) + for _, a := range args { + if strings.HasPrefix(a, "--file=") { + if err := os.WriteFile(strings.TrimPrefix(a, "--file="), []byte(r.body), 0o644); err != nil { + return "", err + } + } + } + return "", nil +} + +// TestReDump_Brew_ChangeDetection: a dump whose bytes differ from the committed +// manifest reports changed=true; a dump that reproduces the committed bytes +// reports changed=false, so capture never counts a clean re-dump as a change. +func TestReDump_Brew_ChangeDetection(t *testing.T) { + dir := t.TempDir() + depsDir := filepath.Join(dir, "deps") + target := filepath.Join(depsDir, "Brewfile.darwin") + m := Manifest{Manager: platform.ManagerBrew, GOOS: "darwin", Shared: target} + + // Fresh dump over no manifest: changed. + r := &brewWritingRunner{body: "brew \"jq\"\n"} + if _, changed, err := reDump(m, r); err != nil { + t.Fatalf("reDump fresh: %v", err) + } else if !changed { + t.Errorf("fresh dump over no manifest must report changed=true") + } + + // Identical re-dump: unchanged. + if _, changed, err := reDump(m, r); err != nil { + t.Fatalf("reDump identical: %v", err) + } else if changed { + t.Errorf("identical re-dump must report changed=false") + } + + // A dump with different bytes (a new package appeared): changed. + r.body = "brew \"jq\"\nbrew \"ripgrep\"\n" + if _, changed, err := reDump(m, r); err != nil { + t.Fatalf("reDump drifted: %v", err) + } else if !changed { + t.Errorf("drifted dump must report changed=true") + } +} + func TestReDump_NoManager_Reports(t *testing.T) { m := Manifest{Manager: platform.ManagerNone} r := newFakeRunner() - if _, err := reDump(m, r); !errors.Is(err, ErrNoPackageManager) { + if _, _, err := reDump(m, r); !errors.Is(err, ErrNoPackageManager) { t.Errorf("reDump no manager: err=%v want ErrNoPackageManager", err) } if len(r.calls) != 0 { @@ -675,7 +761,7 @@ func TestReDump_NoManager_Reports(t *testing.T) { func TestReDump_AptUnsupported(t *testing.T) { m := Manifest{Manager: platform.ManagerApt, GOOS: "linux", Shared: "/repo/deps/apt.txt"} r := newFakeRunner() - _, err := reDump(m, r) + _, _, err := reDump(m, r) if err == nil { t.Fatal("reDump apt: want unsupported error, got nil") } diff --git a/internal/deps/dump.go b/internal/deps/dump.go index 1a9bc5c..b5d922b 100644 --- a/internal/deps/dump.go +++ b/internal/deps/dump.go @@ -1,9 +1,11 @@ package deps import ( + "bytes" "fmt" "os" "path/filepath" + "strings" "github.com/REPPL/ferry/internal/platform" ) @@ -19,11 +21,15 @@ import ( // unsupported rather than guessing — the shared apt.txt stays hand-curated. // - none: ErrNoPackageManager (the caller reports it; never bootstraps a PM). // -// Returns the absolute path of the file that was (re)written. -func ReDumpManifest(depsDir string, runner CommandRunner) (string, error) { +// Returns the absolute path of the file that was (re)written, plus whether the +// dump actually CHANGED its bytes — `brew bundle dump --force` truncates and +// rewrites unconditionally, so without the before/after compare a clean +// re-dump would be indistinguishable from real drift and capture's summary +// would claim a change it never made. +func ReDumpManifest(depsDir string, runner CommandRunner) (string, bool, error) { m, err := SelectManifest(depsDir) if err != nil { - return "", err + return "", false, err } return reDump(m, runner) } @@ -31,9 +37,9 @@ func ReDumpManifest(depsDir string, runner CommandRunner) (string, error) { // reDump is the testable core: the manifest is pre-selected, so a test asserts // the dump targets ONLY the detected manager's file (Brewfile.) and never // another OS's. -func reDump(m Manifest, runner CommandRunner) (string, error) { +func reDump(m Manifest, runner CommandRunner) (string, bool, error) { if runner == nil { - return "", fmt.Errorf("deps: nil CommandRunner") + return "", false, fmt.Errorf("deps: nil CommandRunner") } switch m.Manager { case platform.ManagerBrew: @@ -45,20 +51,28 @@ func reDump(m Manifest, runner CommandRunner) (string, error) { // files here, so a symlink component is always illegitimate. This Lstats the // deps-side path only; it never reads ~/.ssh. if err := refuseSymlinkTarget(m.Shared); err != nil { - return "", err + return "", false, err } // Ensure the deps/ directory exists so brew bundle dump can write into it. if err := os.MkdirAll(filepath.Dir(m.Shared), 0o755); err != nil { - return "", fmt.Errorf("deps: create deps dir for %s: %w", m.Shared, err) + return "", false, fmt.Errorf("deps: create deps dir for %s: %w", m.Shared, err) } - if _, err := runner.Run(brewBin, "bundle", "dump", "--force", "--file="+m.Shared); err != nil { - return "", fmt.Errorf("deps: brew bundle dump --file=%s: %w", m.Shared, err) + // brew writes the file itself, so change detection is a before/after byte + // compare. Either read tolerates a missing file as nil bytes: a fresh dump + // over no manifest compares nil-vs-content (changed), and an unreadable + // after-file compares equal (unchanged) rather than failing a dump brew + // itself reported as successful. The symlink guard above already refused a + // non-regular target. + before, _ := os.ReadFile(m.Shared) + if out, err := runner.Run(brewBin, "bundle", "dump", "--force", "--file="+m.Shared); err != nil { + return "", false, fmt.Errorf("deps: brew bundle dump --file=%s: %w (%s)", m.Shared, err, strings.TrimSpace(out)) } - return m.Shared, nil + after, _ := os.ReadFile(m.Shared) + return m.Shared, !bytes.Equal(before, after), nil case platform.ManagerApt: - return "", fmt.Errorf("deps: apt has no clean installed-set dump; %s stays hand-curated (capture is brew-only)", m.Shared) + return "", false, fmt.Errorf("deps: apt has no clean installed-set dump; %s stays hand-curated (capture is brew-only)", m.Shared) default: - return "", ErrNoPackageManager + return "", false, ErrNoPackageManager } } diff --git a/internal/deps/install.go b/internal/deps/install.go index 5ec7e92..51afa9f 100644 --- a/internal/deps/install.go +++ b/internal/deps/install.go @@ -126,8 +126,8 @@ func installBrew(m Manifest, runner CommandRunner) (InstallResult, error) { // layered after shared). brew bundle is idempotent: already-present formulae // are left alone, so re-running is safe. for _, file := range bundleFiles { - if _, err := runner.Run(brewBin, "bundle", "--file="+file); err != nil { - return res, fmt.Errorf("deps: brew bundle --file=%s: %w", file, err) + if out, err := runner.Run(brewBin, "bundle", "--file="+file); err != nil { + return res, fmt.Errorf("deps: brew bundle --file=%s: %w (%s)", file, err, strings.TrimSpace(out)) } } @@ -224,8 +224,8 @@ func installApt(m Manifest, runner CommandRunner) (InstallResult, error) { // `-oDPkg::Pre-Invoke::=touch /tmp/x` from being read as an apt option and // executed as root under `sudo ferry apply --deps`. args := append([]string{aptGetBin, "install", "-y", "--"}, pkgs...) - if _, err := runner.Run(args...); err != nil { - return res, fmt.Errorf("deps: %s: %w", joinArgs(args), err) + if out, err := runner.Run(args...); err != nil { + return res, fmt.Errorf("deps: %s: %w (%s)", joinArgs(args), err, strings.TrimSpace(out)) } after, afterOK := aptInstalledSet(runner, pkgs) diff --git a/internal/deps/npm.go b/internal/deps/npm.go index 3d77715..4f1614e 100644 --- a/internal/deps/npm.go +++ b/internal/deps/npm.go @@ -1,6 +1,7 @@ package deps import ( + "bytes" "encoding/json" "fmt" "os" @@ -8,6 +9,8 @@ import ( "regexp" "sort" "strings" + + "github.com/REPPL/ferry/internal/backup" ) // npmBin is the npm executable name, resolved through PATH by the runner (npm is @@ -75,26 +78,32 @@ func DumpNpmGlobals(runner CommandRunner) ([]string, error) { // npm analogue of ReDumpManifest for brew: capture calls it. The target is // symlink-guarded BEFORE the write (ferry only writes regular files under deps/, // so a symlinked npm-globals.txt or a symlinked deps/ directory is refused, never -// written through). Returns the absolute path written. -func ReDumpNpmGlobals(depsDir string, runner CommandRunner) (string, error) { +// written through). Returns the absolute path plus whether the list's bytes +// changed; an already-matching manifest is left untouched so capture's summary +// never counts a clean re-dump as a captured change. +func ReDumpNpmGlobals(depsDir string, runner CommandRunner) (string, bool, error) { if depsDir == "" { - return "", fmt.Errorf("deps: empty deps directory") + return "", false, fmt.Errorf("deps: empty deps directory") } names, err := DumpNpmGlobals(runner) if err != nil { - return "", err + return "", false, err } target := NpmGlobalsFile(depsDir) if err := refuseSymlinkTarget(target); err != nil { - return "", err + return "", false, err + } + rendered := []byte(renderNpmGlobals(names)) + if existing, err := os.ReadFile(target); err == nil && bytes.Equal(existing, rendered) { + return target, false, nil } if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { - return "", fmt.Errorf("deps: create deps dir for %s: %w", target, err) + return "", false, fmt.Errorf("deps: create deps dir for %s: %w", target, err) } - if err := os.WriteFile(target, []byte(renderNpmGlobals(names)), 0o644); err != nil { - return "", fmt.Errorf("deps: write %s: %w", target, err) + if err := backup.AtomicWrite(target, rendered, 0o644); err != nil { + return "", false, fmt.Errorf("deps: write %s: %w", target, err) } - return target, nil + return target, true, nil } // renderNpmGlobals renders names as a deterministic file body: one name per line, @@ -178,8 +187,8 @@ func InstallNpmGlobals(depsDir string, runner CommandRunner) ([]string, error) { return nil, nil } args := append([]string{npmBin, "i", "-g", "--"}, names...) - if _, err := runner.Run(args...); err != nil { - return nil, fmt.Errorf("deps: %s: %w", joinArgs(args), err) + if out, err := runner.Run(args...); err != nil { + return nil, fmt.Errorf("deps: %s: %w (%s)", joinArgs(args), err, strings.TrimSpace(out)) } return names, nil } diff --git a/internal/deps/npm_status_test.go b/internal/deps/npm_status_test.go index ff47cd0..d966d0e 100644 --- a/internal/deps/npm_status_test.go +++ b/internal/deps/npm_status_test.go @@ -155,10 +155,13 @@ func TestReDumpNpmGlobals_WritesSortedList(t *testing.T) { r := newFakeRunner() r.replies["ls -g"] = `{"dependencies":{"zed":{"version":"1"},"apple":{"version":"2"},"npm":{"version":"3"}}}` - path, err := ReDumpNpmGlobals(depsDir, r) + path, changed, err := ReDumpNpmGlobals(depsDir, r) if err != nil { t.Fatalf("ReDumpNpmGlobals: %v", err) } + if !changed { + t.Errorf("first dump into an empty deps dir must report changed=true") + } got, err := os.ReadFile(path) if err != nil { t.Fatalf("read written list: %v", err) @@ -166,6 +169,14 @@ func TestReDumpNpmGlobals_WritesSortedList(t *testing.T) { if want := "apple\nzed\n"; string(got) != want { t.Errorf("npm-globals.txt = %q, want %q", got, want) } + + // A second identical dump must be a no-op: same bytes, changed=false, so + // capture never counts a clean re-dump as a captured change. + if _, changed, err := ReDumpNpmGlobals(depsDir, r); err != nil { + t.Fatalf("ReDumpNpmGlobals (re-run): %v", err) + } else if changed { + t.Errorf("identical re-dump must report changed=false") + } } func TestReDumpNpmGlobals_RefusesSymlinkTarget(t *testing.T) { @@ -177,7 +188,7 @@ func TestReDumpNpmGlobals_RefusesSymlinkTarget(t *testing.T) { } r := newFakeRunner() r.replies["ls -g"] = `{"dependencies":{"a":{"version":"1"}}}` - if _, err := ReDumpNpmGlobals(depsDir, r); err == nil { + if _, _, err := ReDumpNpmGlobals(depsDir, r); err == nil { t.Errorf("ReDumpNpmGlobals wrote through a symlink target — must refuse") } } diff --git a/scripts/consistency-lint.sh b/scripts/consistency-lint.sh index 9bd2e49..bf829a3 100755 --- a/scripts/consistency-lint.sh +++ b/scripts/consistency-lint.sh @@ -83,7 +83,7 @@ done # nothing upstream of this check stops `git add -A` staging the handoff. tracked_local=$(git ls-files -- '.abcd/.work.local/' || true) if [ -n "$tracked_local" ]; then - echo "consistency-lint: .abcd/.work.local/ is the private tier (ADR 0002) and must never be committed — untrack with \`git rm --cached\`:" >&2 + echo "consistency-lint: .abcd/.work.local/ is the private tier (ADR 0002) and must never be committed — remove it from the INDEX AND from every unpushed commit that carries it (\`git rm --cached\` plus an amend/rebase of those commits; \`git rm --cached\` alone greens this gate while the pushed history still publishes the file):" >&2 echo "$tracked_local" | sed 's/^/ /' >&2 fail=1 fi diff --git a/scripts/release.sh b/scripts/release.sh index a83f840..f9472c3 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -154,8 +154,11 @@ main() { [ -x "$bin" ] || die "expected built binary $bin is missing" local reported reported="$("$bin" version)" - if ! printf '%s' "$reported" | grep -qF "$VERSION"; then - die "built binary reports '$reported', not '$VERSION'" + # Exact compare: the dev line is the next release plus -dev, so an unstamped + # binary's "ferry vX.Y.Z-dev" CONTAINS vX.Y.Z and a substring match would pass + # on exactly the stamp failure this gate exists to catch. + if [ "$reported" != "ferry $VERSION" ]; then + die "built binary reports '$reported', not 'ferry $VERSION'" fi echo "release: rehearsal — $bin reports: $reported" From 32b3844c933a3de1e5e13fe243f3568d0381ed65 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 14 Aug 2026 08:17:07 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20round-8=20merge-gate=20remediation?= =?UTF-8?q?=20=E2=80=94=20commands.md=20group=20list,=20fifth=20deps=20err?= =?UTF-8?q?or=20site,=20empty-manifest=20change=20detection,=20groupRisky?= =?UTF-8?q?=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the final dual review of PR #37: - docs/reference/commands.md's apply row still enumerated the guided walkthrough groups as dotfiles/agents/terminals — the exact list the groupRisky fix deleted — and now names all six domains (the reviewer's one blocking finding); the twin stale comments in cmd/apply.go updated - internal/deps/status.go's read-only drift dump was a fifth site discarding the manager's output on error; now consistent with the other rails, and the CHANGELOG wording is accurate - a brew dump that CREATES an empty manifest now counts as a change: nil-vs-empty bytes compare equal, so reDump checks existence too - groupRisky gains the unit tests it shipped without (per-domain names, registry order, no lumping into dotfiles, preference-item fallback) - the dump.go change-detection comment and capture's "exactly as apply" claim corrected; CHANGELOG narrates the npm 0644 normalisation and receive's abort-first refusal ordering Assisted-by: Claude:claude-fable-5 --- .abcd/work/DECISIONS.md | 2 +- CHANGELOG.md | 16 ++++++---- cmd/apply.go | 9 +++--- cmd/capture.go | 12 ++++---- cmd/guided_apply.go | 4 ++- cmd/guided_apply_group_test.go | 55 ++++++++++++++++++++++++++++++++++ docs/reference/commands.md | 2 +- internal/deps/deps_test.go | 11 +++++++ internal/deps/dump.go | 15 ++++++---- internal/deps/status.go | 2 +- 10 files changed, 103 insertions(+), 25 deletions(-) create mode 100644 cmd/guided_apply_group_test.go diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 95fcc33..d7f6095 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -28,4 +28,4 @@ a `.abcd/development/decisions/` ADR when it shapes architecture or is expensive - 2026-08-06: Bug-hunt round 5 (fresh hunt, 4 dimensions, 20 adversarial refuters over ~28 candidates): fixed five code defects — sync's pre-commit secret gate read porcelain -z through the TrimSpace helper (a leading-space first entry — the normal capture-then-sync state — parsed to a corrupt path and that file was silently never scanned; raw gitSync now, plus sorted deterministic clobber aborts); machine config written via temp+rename (in-place O_TRUNC destroyed it on crash/ENOSPC, and the leaf symlink was followed); work receive/restore surface held/failed locks per the shared contract (prune's retained count real); doctor reports plan-time containment refusals as [fail] (its ssh/containment invariants were vacuous over the filtered plan); agents locally-drifted guidance names ferry capture (capture-back shipped long ago; conflicts keep the repo-copy remedy). Release path: verify-macos runs vet/unit/race (a hand-pushed tag ran no macOS unit leg anywhere); auto-release runs check-plan-shipped BEFORE cutting the immutable tag and bounds the re-release retry at three failed runs (counted where called-workflow runs actually live: failed auto-release runs since the tag, plus standalone release.yml runs for the tagged commit); release.yml refuses non-semver tag names (backticks in a tag reached a make-spawned shell) and publishes the tag's CHANGELOG section as the release body; gen-checksums enumerates bin/ferry-* by the same glob the upload/attestation use (three unlinked target lists); smoke-e2e fails closed without a hash tool; install.sh renames the staged binary into place. Docs: README quickstart reaches a working push (init --github + sync; the old flow pushed a repo missing the capture), tutorial Fresh path gains remote add, wizard answers-file schema documented in the configuration reference, apply --deps wording names both dependency domains, deps/README drops the nonexistent "pinned post-install steps" mechanism, generated CLI pages show --version and init/restore positionals, in-place-use of a local worktree source stated, private companion project name redacted from this log (history retains it — owner-only to rewrite). Refuted as non-defects: interactive yes-on-conflict exiting 0 (documented attended/unattended split), undated 0.1.0/0.2.0 CHANGELOG headings (never tagged; dates would be fabricated), British CLI output (house majority), smoke-e2e single-yes (one risky group = one prompt), repoIgnoresCase via -C merged config (matches git's own semantics). Carried forward: CONTEXT.md lacks the v0.10.0 work-store invariants (mirrors AGENTS.md Boundaries — owner call), sync refuses a bare-init repo whose init.defaultBranch is not main (new lead, unadjudicated). - 2026-08-08: Bug-hunt round 6 (fresh hunt, 4 dimensions, 4 hunters; 29 candidates, one independent adversarial refuter each — 9 confirmed, 20 refuted). Fixed five code/infra defects: a fresh config repo is pinned to the managed branch (bare `git init` honours the machine's `init.defaultBranch`, git's own default being `master`, while sync integrates and pushes `main` and refuses anything else — so `init --github` succeeded and every later `ferry sync` failed; the eval fixtures forced `-b main` instead of driving `ferry init`, so no test could see it); `init`/`bundle import` carry the machine-scoped fields across config.toml's wholesale rewrite (a plain `ferry init` re-run dropped `managed` and erased the `[work]` cargo store, silently un-configuring sync and every work verb — `[work]` now survives every route, `managed` only when the run reuses that repo); the pre-commit secret gate consumes a rename/copy origin field as parseStatusZ does (a 4-byte-or-longer origin was sliced as a status entry, so a staged `git mv src/dotfiles …` read as `/dotfiles`, hit EISDIR against the repo's own `dotfiles/` and fail-closed the whole sync on a path the user never touched); `appendLineOnce` normalises a missing trailing newline (an append onto an unterminated `.git/info/exclude` fused the two patterns — destroying the user's last ignore rule AND silently voiding ferry's — while scaffold printed success), and reports its close error; release.yml passes `--prerelease` for a suffixed tag (the tag gate deliberately admits one and both scripts skip cleanly so the run goes green, but the publish marked it a full release, making a rehearsal tag the repo's `latest` — the exact URL install.sh serves). Hardening: the two boundary-refusal message bodies move to internal/dotfile as `RefusalSSHBody`/`RefusalEscapeBody`, so the five plan packages and doctor's invariant matcher share one definition, plus a consistency-lint rule that fails on any re-inlined copy (a reword in one package silently turned a security-invariant [fail] into a [pass], with no compile error and nothing red). Docs: the tutorial's fresh flow stages with `add -A` before committing (`commit -am` never staged capture's new files, so the push published a repo missing the capture); the agents drift guidance names `ferry capture` first and drops the false "backed up, reversible" claim for `--force` (the write-once baseline holds the pre-ferry file, so restore cannot return a force-overwritten live edit). Decision log: four 2026-07-06 entries gained the `- ` marker they lacked, which made them CommonMark lazy continuations absorbed into the preceding bullet. Refuted as non-defects: the tutorial's "capture can push / apply can pull" purpose sentence (the same section attributes both to `ferry sync` 30 lines later, and the prerequisites table states the split up front); `.abcd/.work.local/` ignored via `.git/info/exclude` rather than `.gitignore` (ADR 0002 chooses this verbatim — "never `.gitignore`" — and release.sh returns early when NEXT.md is absent, so the claimed dirt chain cannot form); smoke-e2e.sh being CI-unwired (both assertions it claims as unique are covered more strictly by CI-run evals — restore byte-identity WITH mode in restore_test.go, and the ~/.ssh tripwire across the whole surface in safety_test.go plus a strace no-open gate); the auto-release three-failure bound counting transient failures (documented in cutting-a-release.md, recoverable by re-running, and strictly better than unbounded retry on an immutable tag); AGENTS.md:58 and CONTEXT.md:24 "repo-authoritative" (every clause is literally true — apply still skips drift — and configuration.md's emacs entry proves the formula carries no claim about capture, needing a separate "no capture pass" sentence); the plans' frontmatter divergence (check-plan-shipped.sh greps `Status:` and matches both forms; the README says "for example"); `ABCDevelopment` in six archival lines (AGENTS.md's rule is scoped to naming the AI assistant, the sibling `abcd-cli` repo is public, and scrubbing archival records is an owner call); five code nitpicks (restore's package count, Prune's partial report, the status.go errors.Is marker, config.toml's 0644, and .gitignore's create-only perm) and nine infra/docs nitpicks (auto-merge timeout — reusable-workflow calls cannot take one either, so the premise was false; the actions:read comment; status=failure under-counting, which loosens not tightens; the START_SHA tripwire, which is documented fail-closed; the public/private comments; the CLI-page naming column, whose two-level pattern reads correctly; the drift page's link blurb; the .work.local link, deliberate and self-describing; ADR 0003's heading depth, which MADR permits). - 2026-08-13: Bug-hunt round 7 (fresh hunt, 4 dimensions, 4 hunters; 23 unique candidates, one independent adversarial refuter each — 19 confirmed, 4 refuted). Fixed nine substantive defects: every work verb refuses a cargo store that is or resolves into `~/.ssh` before any read/write on the path (the store was the one configurable, ferry-written path with no ssh guard — `work pack` created a world-writable directory inside `~/.ssh` and wrote bundles there, while the guarded repo/`--out` paths prove hand-edited config is in the threat model); the pre-commit secret gate walks a collapsed untracked directory (`?? dir/` at default -unormal EISDIR'd the fail-closed read, wedging every sync after a capture created a new repo subdirectory, with advice a directory can never satisfy — the sibling backup pass already walked exactly this shape, and the round-3 test dodged it with a staged `add -A`); `ferry init ` wiring the already-configured repo carries `managed` (the round-6 carry keyed on the no-arg route, so `ferry init .` from inside the configured repo dropped the flag with no supported way back — `init --github` refuses while a repo is configured; the carry is gated on the route having WIRED a pre-existing worktree, because a clone or fresh seed can land on the recorded path after the old repo was deleted and must never inherit the flag — the final dual review caught exactly that hole in this round's first cut, both reviewers NO-SHIP, fixed in the single remediation round with an eval pinning the recreated-at-same-path case); `bundle import` pins HEAD to the managed branch before its initial commit (the same unpinned `git init` round 6 fixed for the fresh path, left on the import route); `init --github`'s success banner and existing-repo hint name `ferry sync` as the publish verb (the flow's final instruction claimed capture pushes and apply pulls — capture never commits or pushes, apply never touches the network, and the banner is the last thing the flow prints); drift/conflict guidance is keyed on the registry's Captures() flag, so status stops steering terminals/keybindings/emacs/iterm2-profiles at `ferry capture` and iterm2-profiles stops falling through apply/diff's hand-maintained domain list to the dotfiles wording (an existing eval forbade exactly this wording for diff but never ran status); `apply --deps` states when a failed installed-set snapshot suppressed the restore-record (the fail-closed suppression printed the same "installed 0 package(s)" as a benign re-run, and restore --packages silently lost the run); release.yml asserts the built binary reports the tag before checksumming (Go's linker silently drops -X for a moved symbol, so a rename would publish -dev-reporting binaries with valid checksums and provenance, nothing red) and verifies the published assets against the published checksums.txt after the release (the pairing install.sh depends on; runs on private repos where the attestation check is gated off); consistency-lint invariant 5 fails if `.abcd/.work.local/` is ever tracked, and `make preflight` runs the lint so the pre-push hook catches it before publication (the exclude rule is per-clone and absent in a fresh checkout, so `git add -A` staged the private tier with every gate green — ADR 0002's exclude-not-gitignore choice honoured, enforcement added where none existed). Docs: the compatibility contract covers the four work-domain versioned files (including the machine-crossing cargo manifest) and states their unversioned-is-refused rule plus the no-retention policy for journal runs and snapshots; README/tutorial scope the "never requires sudo" claim to name the apt `apply --deps` exception; doctor's synopsis says git (required) vs zsh/package manager (recommended); install.sh documents FERRY_VERSION and its pipe-placement footgun; evals README lists init_preserve_test.go; the drift page's remedy bullet covers all four repo-authoritative domains; the stale lint step labels name all the invariants; prune-releases refuses to prune when the just-published release is missing from a listing whose fetch failure the empty-grep guard would otherwise swallow. Refuted as non-defects: Brewfile inline-comment rejection (CHANGELOG documents the fail-closed trade-off verbatim, and comment-stripping would regress a legal `#` inside an option value); the release verify 20-minute timeout (measured runs finish in ~2 minutes — 9x headroom — and the burnt-version consequence is documented and one-click recoverable); the refusal-constant concatenation (vet folds constant concatenation and the const-ness is compile-enforced); FERRY_NO_NETWORK's absent header doc (a contract-optional test hook that installs nothing). Carried forward: capture's own completion hint never names `ferry sync` for a managed repo; installApt/installBrew swallow the package manager's stderr (exit-status-only errors); the round-6 `carryMachineScoped` sites now have eval coverage but the unit seam stays untested; work-domain state growth has no pruner (documented this round as deliberate for snapshots; journal runs are inert dead bytes a future keep-last-N sweep could clear); guided_apply buckets the repo-authoritative domains under a "dotfiles" label. -- 2026-08-14: Bug-hunt round 8 (fresh hunt, 4 dimensions, 4 hunters; 22 unique candidates, one independent adversarial refuter each — 12 confirmed, 12 refuted, 2 hunter-self-refuted). Fixed six code defects: guided apply's risky walkthrough groups every FileDomain under its own name in registry order (a two-name literal switch predating fn-5 dropped keybindings/emacs/iterm2-profiles into the "dotfiles" bucket, so the group header and the bulk consent named a domain the user was not reviewing and one "yes" spanned the mixed bucket); capture's brew/npm re-dumps count a change only when the manifest's bytes actually changed (`brew bundle dump --force` rewrites unconditionally, so every clean re-run claimed "wrote 1 change(s)" over an empty `git status` and "nothing has drifted" was unreachable on any machine managing either domain), with the npm list written via backup.AtomicWrite and left untouched when identical; capture's terminal preference-domain compare resolves the local overlay first via terminalRepoStatusSource, exactly as apply/status/diff do (a domain captured to [l]ocal was re-offered as drifted forever, and re-accepting it shared would promote machine-divergent settings); the deps install/dump rails carry the package manager's combined output in their errors as the uninstall rail already did (a failed apply --deps surfaced as a bare "exit status 1"); backup.Engine snapshots run the resolved-parent containment guard inside snapshotCurrent so the exported Snapshot used by work receive is guarded identically to restore (a parent symlink out of $HOME let the pre-receive snapshot read and persist out-of-home content the write boundary then refused); the release version-stamp gate compares exactly against "ferry " in release.yml and release.sh (the dev line is next-release-plus--dev, so the substring grep passed an unstamped binary on precisely the modal release). Docs: the README/tutorial "one privileged step" claim covers restore --packages (the apt uninstall rail runs apt-get as root, per the code's own comments, and no user-facing page said so); the compatibility contract's claim-file row names pack, receive, and take-back as writers (receive.AppendClaim writes claim..json); the commands reference gains the agents parent-noun row; make preflight enters AGENTS.md/CONTRIBUTING.md; the consistency lint's private-tier advice mandates amending committed history (git rm --cached alone greens the gate while the push still publishes). Refuted as non-defects: the terminal-capture placeholder plaintext leak (placeholders exist only for content GateValue already rates High, and the re-export re-trips the gate); the hand-over sudo mkdir (round-1 adjudication stands — a host-OS setup example, not a ferry operation); the release how-to's five-step summary (explicitly non-exhaustive by its own structure); +build-metadata tags becoming latest (SemVer-correct and deliberate per the workflow's own comment); the attestation docs' missing visibility caveat (true of everything a reader can download); the retention-policy scoping, [work]-in-ferry.toml, topic-plan lifecycle, behavioral-in-evals-README, "works today" framing, attribution-rule carve-out (rule already scoped to credit; amending owner policy is an owner call), the pre-push hook's absent in-repo installation (dispatcher is deliberately external; hazard population equals hook population), and a range-based invariant-5 check (breaks CI's detached checkout — the advice-text fix shipped instead). Carried forward: capture's completion hint still never names ferry sync for a managed repo (adjudicated a nitpick, unfixed this round); the managed carry stays path-keyed (an origin-URL comparison would close it); guided-apply's per-domain groups have no direct interactive eval; the darwin-only defaults export has no injectable seam, so the terminal-capture fix is pinned at the terminalRepoStatusSource seam rather than end-to-end. +- 2026-08-14: Bug-hunt round 8 (fresh hunt, 4 dimensions, 4 hunters; 22 unique candidates, one independent adversarial refuter each — 12 confirmed, 12 refuted, 2 hunter-self-refuted). Fixed six code defects: guided apply's risky walkthrough groups every FileDomain under its own name in registry order (a two-name literal switch predating fn-5 dropped keybindings/emacs/iterm2-profiles into the "dotfiles" bucket, so the group header and the bulk consent named a domain the user was not reviewing and one "yes" spanned the mixed bucket); capture's brew/npm re-dumps count a change only when the manifest's bytes actually changed (`brew bundle dump --force` rewrites unconditionally, so every clean re-run claimed "wrote 1 change(s)" over an empty `git status` and "nothing has drifted" was unreachable on any machine managing either domain), with the npm list written via backup.AtomicWrite and left untouched when identical; capture's terminal preference-domain compare resolves the local overlay first via terminalRepoStatusSource, exactly as apply/status/diff do (a domain captured to [l]ocal was re-offered as drifted forever, and re-accepting it shared would promote machine-divergent settings); the deps install/dump rails carry the package manager's combined output in their errors as the uninstall rail already did (a failed apply --deps surfaced as a bare "exit status 1"); backup.Engine snapshots run the resolved-parent containment guard inside snapshotCurrent so the exported Snapshot used by work receive is guarded identically to restore (a parent symlink out of $HOME let the pre-receive snapshot read and persist out-of-home content the write boundary then refused); the release version-stamp gate compares exactly against "ferry " in release.yml and release.sh (the dev line is next-release-plus--dev, so the substring grep passed an unstamped binary on precisely the modal release). Docs: the README/tutorial "one privileged step" claim covers restore --packages (the apt uninstall rail runs apt-get as root, per the code's own comments, and no user-facing page said so); the compatibility contract's claim-file row names pack, receive, and take-back as writers (receive.AppendClaim writes claim..json); the commands reference gains the agents parent-noun row; make preflight enters AGENTS.md/CONTRIBUTING.md; the consistency lint's private-tier advice mandates amending committed history (git rm --cached alone greens the gate while the push still publishes). Refuted as non-defects: the terminal-capture placeholder plaintext leak (placeholders exist only for content GateValue already rates High, and the re-export re-trips the gate); the hand-over sudo mkdir (round-1 adjudication stands — a host-OS setup example, not a ferry operation); the release how-to's five-step summary (explicitly non-exhaustive by its own structure); +build-metadata tags becoming latest (SemVer-correct and deliberate per the workflow's own comment); the attestation docs' missing visibility caveat (true of everything a reader can download); the retention-policy scoping, [work]-in-ferry.toml, topic-plan lifecycle, behavioral-in-evals-README, "works today" framing, attribution-rule carve-out (rule already scoped to credit; amending owner policy is an owner call), the pre-push hook's absent in-repo installation (dispatcher is deliberately external; hazard population equals hook population), and a range-based invariant-5 check (breaks CI's detached checkout — the advice-text fix shipped instead). Merge-gate remediation (one round, regressions reviewer's blocker): commands.md's apply row still enumerated the walkthrough groups as "dotfiles / agents / terminals" — the exact list the code fix deleted — now naming all six domains; folded in the reviewers' safe notes: the status drift dump was a fifth output-discarding site (internal/deps/status.go), a dump that creates an empty manifest now counts as a change (nil-vs-empty bytes compare equal, so existence is checked), groupRisky gained the unit tests it shipped without, the dump.go comment and capture's "exactly as apply" claim were corrected, and the CHANGELOG narrates the npm 0644 normalisation and receive's abort-first failure mode. Carried forward: capture's completion hint still never names ferry sync for a managed repo (adjudicated a nitpick, unfixed this round); the managed carry stays path-keyed (an origin-URL comparison would close it); terminalRepoStatusSource ignores an extensionless local overlay that apply's terminalExportBlob honours, and falls back to shared behind a poisoned overlay where apply fails closed (pre-existing, inherited by capture's compare); guided-apply's per-domain groups have no direct interactive eval; the darwin-only defaults export has no injectable seam, so the terminal-capture fix is pinned at the terminalRepoStatusSource seam rather than end-to-end. diff --git a/CHANGELOG.md b/CHANGELOG.md index d7064d0..5b582c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -383,7 +383,8 @@ called out in a **Breaking** section. See empty `git status`, and "nothing has drifted" was unreachable on any machine managing either domain. Both re-dumps now compare bytes and count only real changes; the npm list is also written crash-safely (temp + rename) instead - of truncate-in-place, and is left untouched when it already matches. + of truncate-in-place — normalising its mode to 0644 — and is left untouched + when it already matches. - **A captured-to-local terminal domain reads as clean.** Capture compared the live iTerm2/Apple Terminal export only against the shared repo plist, while `apply`, `status`, and `diff` all resolve the per-machine local overlay @@ -393,16 +394,19 @@ called out in a **Breaking** section. See `status` reported it clean. Capture now compares against the same local-wins source the other commands use. - **Failed dependency installs name the failure.** The brew/apt/npm install - and dump rails discarded the package manager's combined output on error, so - a failed `apply --deps` aborted with a bare `exit status 1` and no way to - tell which package or tap failed. The manager's own diagnostics now ride the - error, as the uninstall rail's errors already did. + and dump rails — including the read-only drift dump behind `ferry status` — + discarded the package manager's combined output on error, so a failed + `apply --deps` aborted with a bare `exit status 1` and no way to tell which + package or tap failed. The manager's own diagnostics now ride the error, as + the uninstall rail's errors already did. - **Pre-receive snapshots are containment-guarded.** The work domain's per-receive snapshot read its target paths before any guard ran, so a parent directory symlinked out of `$HOME` let the snapshot read and persist out-of-home content into ferry's snapshot store — content the write boundary downstream refuses to touch. The snapshot now runs the same resolved-parent - containment guard as restore, refusing before anything is read or written. + containment guard as restore, refusing before anything is read or written — + so a refused `work receive` now aborts up front, where it previously aborted + at the write boundary after earlier items had already landed. - **The release version-stamp gate is an exact compare.** Both the release workflow and `scripts/release.sh` checked the built binary's reported version with a substring match; the development line is the next release diff --git a/cmd/apply.go b/cmd/apply.go index 98dac55..3ee9aee 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -55,8 +55,9 @@ const ( // resolved, secret-rendered content so diff and apply share identical planning. type planItem struct { kind planKind - // fileDomain is the owning FileDomain's scope name ("dotfiles", "agents", - // "terminals") for a kindFile item. It selects the per-domain report wording + // fileDomain is the owning FileDomain's scope name (a registry name: + // "dotfiles", "agents", "terminals", "keybindings", "emacs", + // "iterm2-profiles") for a kindFile item. It selects the per-domain report wording // (capture guidance vs repo-authoritative vs repo-source) and the // agents-target recording that used to be keyed on the collapsed kinds. Empty // for kindPreference. @@ -221,8 +222,8 @@ func buildPlanWithEngine(ctx *cmdContext, eng *backup.Engine) (items []planItem, // FileDomain fan-out (fn-5): plan each in-scope FileDomain through the // converged registry, in the registry's LOAD-BEARING order (dotfiles, agents, - // terminals), so item AND warning ORDERING matches the pre-fn-5 dispatch - // byte-for-byte. Each managed domain's rich planItems — secrets rendered, + // terminals, keybindings, emacs, iterm2-profiles), so item AND warning + // ORDERING matches the pre-fn-5 dispatch byte-for-byte. Each managed domain's rich planItems — secrets rendered, // three-way state classified, risk assessed — is produced by its per-domain // planner via the filePlanner upcast (the frozen domains.FileItem cannot carry // state/skip/risk). An out-of-scope domain emits its own de-scope warnings diff --git a/cmd/capture.go b/cmd/capture.go index 3a2fddc..59d4ef3 100644 --- a/cmd/capture.go +++ b/cmd/capture.go @@ -1069,11 +1069,13 @@ func captureTerminalDomain(cc captureCtx, domain string) (wrote bool, offered bo // Only offer when the live export actually DIFFERS from the bytes apply would // deploy (don't offer a no-op). Compare against the LOCAL-WINS source exactly - // as apply and status resolve it (terminalRepoStatusSource): a domain captured - // to the [l]ocal overlay must read as clean on the next run — comparing only - // the shared copy re-offered an already-captured domain forever, while status - // reported it clean. An absent repo copy is itself a difference (capture would - // create it). + // as status resolves it (terminalRepoStatusSource, which closely mirrors + // apply's terminalExportBlob — apply additionally accepts an extensionless + // overlay and fails closed on a poisoned one): a domain captured to the + // [l]ocal overlay must read as clean on the next run — comparing only the + // shared copy re-offered an already-captured domain forever, while status + // reported it clean. An absent repo copy is itself a difference (capture + // would create it). compareSrc := terminalRepoStatusSource(cc.repoPath, domain, prefID) if _, err := safeRepoPath(cc.repoPath, compareSrc); err != nil { return false, false, err diff --git a/cmd/guided_apply.go b/cmd/guided_apply.go index 15e18e1..82e3025 100644 --- a/cmd/guided_apply.go +++ b/cmd/guided_apply.go @@ -280,9 +280,11 @@ func groupRisky(risky []planItem) []riskyGroup { for _, it := range risky { name := it.fileDomain if name == "" { + // Non-FileDomain items (terminal preference domains) carry no + // fileDomain; they keep their pre-existing dotfiles-bucket home. name = "dotfiles" } - if _, seen := byDomain[name]; !seen && !slices.Contains(order, name) { + if !slices.Contains(order, name) { order = append(order, name) } byDomain[name] = append(byDomain[name], it) diff --git a/cmd/guided_apply_group_test.go b/cmd/guided_apply_group_test.go new file mode 100644 index 0000000..5ad7db6 --- /dev/null +++ b/cmd/guided_apply_group_test.go @@ -0,0 +1,55 @@ +package cmd + +import "testing" + +// groupRisky once bucketed every FileDomain it did not name into "dotfiles", +// so the walkthrough's group header and bulk consent named a domain the user +// was not reviewing. These tests pin the fix: every domain groups under its +// own name, in registry order, and only non-FileDomain items (which carry no +// fileDomain) fall back to the dotfiles bucket. +func TestGroupRiskyGroupsEveryDomainUnderItsOwnName(t *testing.T) { + risky := []planItem{ + {domain: "iterm2-profiles:p.json", fileDomain: "iterm2-profiles"}, + {domain: "emacs:init.el", fileDomain: "emacs"}, + {domain: "dotfiles:.zshrc", fileDomain: "dotfiles"}, + {domain: "keybindings:DefaultKeyBinding.dict", fileDomain: "keybindings"}, + {domain: "agents:AGENTS.md", fileDomain: "agents"}, + {domain: "terminals:alacritty.toml", fileDomain: "terminals"}, + } + groups := groupRisky(risky) + + wantOrder := []string{"dotfiles", "agents", "terminals", "keybindings", "emacs", "iterm2-profiles"} + if len(groups) != len(wantOrder) { + t.Fatalf("got %d groups, want %d: %+v", len(groups), len(wantOrder), groups) + } + for i, want := range wantOrder { + if groups[i].name != want { + t.Errorf("group[%d] = %q, want %q (registry order)", i, groups[i].name, want) + } + if len(groups[i].items) != 1 { + t.Errorf("group %q has %d items, want 1", groups[i].name, len(groups[i].items)) + } + } +} + +func TestGroupRiskyNeverLumpsOtherDomainsIntoDotfiles(t *testing.T) { + risky := []planItem{ + {domain: "emacs:init.el", fileDomain: "emacs"}, + {domain: "keybindings:DefaultKeyBinding.dict", fileDomain: "keybindings"}, + } + for _, g := range groupRisky(risky) { + if g.name == "dotfiles" { + t.Fatalf("emacs/keybindings items grouped under %q — the bulk consent would name a domain the user is not reviewing", g.name) + } + } +} + +func TestGroupRiskyFileDomainlessItemsKeepDotfilesBucket(t *testing.T) { + risky := []planItem{ + {domain: "iterm2 preference domain", fileDomain: ""}, + } + groups := groupRisky(risky) + if len(groups) != 1 || groups[0].name != "dotfiles" { + t.Fatalf("fileDomain-less item grouped as %+v, want the pre-existing dotfiles bucket", groups) + } +} diff --git a/docs/reference/commands.md b/docs/reference/commands.md index 6539583..26d6884 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -9,7 +9,7 @@ Every command is run as `ferry ` (e.g. `ferry init`). | `init --wizard=off\|interactive\|answers:` | Choose the first-run wizard mode. `off` skips it (the non-interactive adopt-and-extract fallback); `interactive` forces the TUI (needs a tty pair); `answers:` drives every wizard decision from a TOML answers file (same gates, preview, backup, and confirm, no tty needed; schema in [Configuration](configuration.md#the-wizard-answers-file)). Default (unset): interactive on a real tty pair, else `off`. | | `init --repair` | Opt into the wizard's repair review: hardcoded `/Users/` paths to `$HOME`, duplicate `PATH` exports, dead `source` lines: each fix is accepted or declined individually. Needs a running wizard, so it conflicts with `--wizard=off` and (in interactive mode) a non-tty run — unless `--wizard=answers:` drives it, which satisfies the consent requirement. | | `init --github [name]` | Create a **new private** GitHub repo via the `gh` CLI's existing auth and manage it as ferry's HTTPS remote. Needs `gh` authenticated; ferry stores no token. Always private, never reuses an existing repo, and won't push a file that looks like a secret: the wizard (or the non-interactive fallback) extracts detected secrets to the local store first, so only placeholders are committed and pushed. Add `--yes` to assent to the create-confirm (needed non-interactively). | -| `apply` | Reconcile this machine to the repo (deploy dotfiles, terminal settings). On a run that has changes it walks the pending work **grouped by domain** (dotfiles / agents / terminals), staying **quiet when safe** and **stopping when risky**: a safe change (creating a file where none exists, or updating a target whose live content still matches what ferry last deployed) applies automatically, while a *risky* change — overwriting a file that differs from the last-deployed baseline, adopting a pre-existing file, or deploying a value from the secret store — halts for confirmation. In the walkthrough you confirm a domain wholesale, drill into it to see each change's full diff, apply or skip a change this run, or skip it *always* (remembered per machine in the gitignored `.local` layer). A clean, in-sync apply prints one line. Non-interactively — or with `--skip-wizard` — nothing risky is applied unattended: risky changes **fail closed** (listed, refused, non-zero exit) while the safe subset still applies. Idempotent; safe to re-run. Dependencies install behind `apply --deps`, and only for the dependency domains declared under `[manage]`: `brew = true` for the Homebrew/apt install, `npm-globals = true` for global npm packages. | +| `apply` | Reconcile this machine to the repo (deploy dotfiles, terminal settings). On a run that has changes it walks the pending work **grouped by domain** (each managed domain — dotfiles, agents, terminals, keybindings, emacs, iTerm2 profiles — under its own name), staying **quiet when safe** and **stopping when risky**: a safe change (creating a file where none exists, or updating a target whose live content still matches what ferry last deployed) applies automatically, while a *risky* change — overwriting a file that differs from the last-deployed baseline, adopting a pre-existing file, or deploying a value from the secret store — halts for confirmation. In the walkthrough you confirm a domain wholesale, drill into it to see each change's full diff, apply or skip a change this run, or skip it *always* (remembered per machine in the gitignored `.local` layer). A clean, in-sync apply prints one line. Non-interactively — or with `--skip-wizard` — nothing risky is applied unattended: risky changes **fail closed** (listed, refused, non-zero exit) while the safe subset still applies. Idempotent; safe to re-run. Dependencies install behind `apply --deps`, and only for the dependency domains declared under `[manage]`: `brew = true` for the Homebrew/apt install, `npm-globals = true` for global npm packages. | | `apply --skip-wizard` | Skip the guided walkthrough (for experts and scripts): safe changes still auto-apply, but risky changes are refused rather than prompted — they never happen unattended. | | `apply --force` | Treat every risky change as confirmed (an explicit override) and overwrite uncaptured local edits on a conflict; the downstream data-loss guards still apply and warn. | | `capture` | Pull local changes back into the repo. Interactive: approve each change, route it *shared* (synced everywhere) or *local* (this machine only). For sources that reference stored secrets, capture compares against the rendered content and splices your edits back around the placeholders, so stored values never re-enter the repo and a store-routed secret never blocks its own round-trip. It also captures edits to deployed agent files (routing to their shared source or a `local/agents/` overlay, refusing a true divergence with a diff) and offers to adopt new agent-shaped files it finds; see [The agents domain](../explanation/agents.md). | diff --git a/internal/deps/deps_test.go b/internal/deps/deps_test.go index 6ac88f4..3e376de 100644 --- a/internal/deps/deps_test.go +++ b/internal/deps/deps_test.go @@ -745,6 +745,17 @@ func TestReDump_Brew_ChangeDetection(t *testing.T) { } else if !changed { t.Errorf("drifted dump must report changed=true") } + + // A dump that CREATES an empty manifest (zero formulae installed) is still a + // change — nil-vs-empty byte compares equal, so the existence check must + // carry it. + m2 := Manifest{Manager: platform.ManagerBrew, GOOS: "darwin", + Shared: filepath.Join(dir, "deps2", "Brewfile.darwin")} + if _, changed, err := reDump(m2, &brewWritingRunner{body: ""}); err != nil { + t.Fatalf("reDump empty-fresh: %v", err) + } else if !changed { + t.Errorf("creating an empty manifest must report changed=true") + } } func TestReDump_NoManager_Reports(t *testing.T) { diff --git a/internal/deps/dump.go b/internal/deps/dump.go index b5d922b..a47432c 100644 --- a/internal/deps/dump.go +++ b/internal/deps/dump.go @@ -58,17 +58,20 @@ func reDump(m Manifest, runner CommandRunner) (string, bool, error) { return "", false, fmt.Errorf("deps: create deps dir for %s: %w", m.Shared, err) } // brew writes the file itself, so change detection is a before/after byte - // compare. Either read tolerates a missing file as nil bytes: a fresh dump - // over no manifest compares nil-vs-content (changed), and an unreadable - // after-file compares equal (unchanged) rather than failing a dump brew - // itself reported as successful. The symlink guard above already refused a - // non-regular target. + // compare plus an existence check: a dump that CREATES the manifest is a + // change even when both reads yield empty bytes (a machine with zero + // formulae dumps an empty file, and nil-vs-empty compares equal). Reads + // tolerate errors as nil bytes — a dump brew itself reported successful is + // never failed here, at worst it is miscounted for the summary. The + // symlink guard above already refused a non-regular target. + _, statErr := os.Lstat(m.Shared) + existedBefore := statErr == nil before, _ := os.ReadFile(m.Shared) if out, err := runner.Run(brewBin, "bundle", "dump", "--force", "--file="+m.Shared); err != nil { return "", false, fmt.Errorf("deps: brew bundle dump --file=%s: %w (%s)", m.Shared, err, strings.TrimSpace(out)) } after, _ := os.ReadFile(m.Shared) - return m.Shared, !bytes.Equal(before, after), nil + return m.Shared, !existedBefore || !bytes.Equal(before, after), nil case platform.ManagerApt: return "", false, fmt.Errorf("deps: apt has no clean installed-set dump; %s stays hand-curated (capture is brew-only)", m.Shared) default: diff --git a/internal/deps/status.go b/internal/deps/status.go index 0f9a5d9..882f19f 100644 --- a/internal/deps/status.go +++ b/internal/deps/status.go @@ -74,7 +74,7 @@ func brewDrift(m Manifest, runner CommandRunner) (Drift, bool, error) { // rather than through the fail-closed install allow-list. out, err := runner.Run(brewBin, "bundle", "dump", "--file=-") if err != nil { - return Drift{}, true, fmt.Errorf("deps: brew bundle dump --file=-: %w", err) + return Drift{}, true, fmt.Errorf("deps: brew bundle dump --file=-: %w (%s)", err, strings.TrimSpace(out)) } liveKeys := brewKeySetLenient(out)