From 2ee610bbeee9ed3178fe6d17c6d192eecf71fb5f Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 09:50:45 -0400 Subject: [PATCH 01/11] Add design spec: Install an alias for the docket binary --- ...l-an-alias-for-the-docket-binary-design.md | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-06-install-an-alias-for-the-docket-binary-design.md diff --git a/docs/superpowers/specs/2026-10-06-install-an-alias-for-the-docket-binary-design.md b/docs/superpowers/specs/2026-10-06-install-an-alias-for-the-docket-binary-design.md new file mode 100644 index 000000000..6076d86de --- /dev/null +++ b/docs/superpowers/specs/2026-10-06-install-an-alias-for-the-docket-binary-design.md @@ -0,0 +1,67 @@ +# Install an alias for the docket binary: design + +Change 0534 — Install an alias for the docket binary + +Change #534, groomed interactively on 2026-10-05, split out of #532 in the private-visibility series (#529–#533 plus this change and the private-instructions change). It is independent and can land first. + +## Summary + +Every install also places `dckt`, a symlink to the `docket` binary, in the same bin directory. The private-instructions change in this series needs it: its user-level Claude hook and Codex/OpenCode pointer must not contain the word "docket", so they invoke `dckt instructions`. Nothing else switches to the alias. Skills keep resolving `docket` from the capability catalog. + +## Evidence gathered at grooming + +- **Two install paths place the binary.** + - **Public release install** (v1.0.0-alpha.1 already uses it): `internal/release/downloader/install.sh` is rendered into every release bundle by `internal/release/render.go` and published as a release asset. It: + - downloads the archive and verifies its SHA-256 before extraction; + - stages the binary in the bin directory (`.docket-stage.XXXXXX`) and runs `"$stage" install …`; + - `mv -f`s it to `$bin_dir/docket`; + - writes its own ownership record at `${XDG_STATE_HOME:-~/.local/state}/docket/release-binary.record`. + + Its contract comment pins the runtime dependencies (`/bin/sh`, `curl`, `tar`, one SHA-256 tool), the refusal to replace a binary it does not own, and "no --force path". `tests/test_release_downloader.sh`, `tests/test_release_downloader_refusals.sh`, `tests/test_release_downloader_converge.sh`, and `scripts/release-smoke.sh` depend on its spellings and PATH sandbox. + - **Development install:** the repository-root `install.sh` is a thin bootstrapper that delegates to `docket development install` (`internal/install/devmode.go`, `binaryName = "docket"`, `resolveBinDir` defaulting to `UserRoots.BinDir` = `${XDG_BIN_HOME:-~/.local/bin}`). That command builds the binary and places it inside a journaled install transaction. +- **Uninstall never removes the binary.** `internal/install/uninstall.go`: "The installation's own binary and every unattributed target remain recorded and untouched." +- **Skills build every invocation from the capability catalog,** whose argv spells `docket`. The binary's commands do not depend on the name it was invoked by. The plan verifies this against the CLI root. + +## Decisions (settled with the human) + +1. The alias is `dckt`, a symlink to the installed `docket`, in the same bin directory. +2. Both install paths create it. The release downloader is the path users take from v1.0, and the development installer is the path behind the repository-root `install.sh`. +3. The alias follows the binary: same placement, same ownership posture, left in place by `uninstall`. +4. A foreign `dckt` is never overwritten, and there is no force path. +5. Sparse documentation: installer output and the downloader's usage text only, no `docs/` pages. + +## Design + +### Release downloader + +After the verified binary is moved into `$bin_dir/docket`, the downloader ensures `$bin_dir/dckt`: + +- **Absent:** create a symlink to `docket`, using a relative target so moving the bin directory keeps it valid. +- **Already a symlink resolving to `$bin_dir/docket`:** leave it (converge). +- **Anything else:** leave it untouched, print a one-line warning naming the path and the remedy, and finish the install successfully. The binary install is never failed by the alias. + +The ownership record gains the alias path, so a later run can tell its own alias from a foreign one. The contract comment, usage text, and tests are updated: `ln -s` is admitted by the spelling ban and the PATH sandbox, and the tests cover create, converge, and foreign refusal. `scripts/release-smoke.sh` asserts that the alias exists after a smoke install. + +### Development installer + +`docket development install` applies the same three-way rule beside the binary it places, inside its journaled transaction. A rollback of a failed install removes an alias it created in that transaction and leaves any pre-existing one alone. The repository-root `install.sh` changes only its header comment, which lists what an install produces. + +### Install check and uninstall + +- `docket install check` reports a missing or foreign `dckt` as a finding with its remedy. It is a finding, not a refusal. +- `docket uninstall` leaves the alias in place, exactly as it leaves the binary. + +## Acceptance criteria + +1. A fresh release install (downloader against a local release fixture) leaves `$bin_dir/dckt` resolving to `$bin_dir/docket`. A second run converges with no change. +2. A fresh development install does the same. +3. A pre-existing foreign `dckt` is untouched by both installers, both installs succeed with a warning, and `install check` reports it. +4. `dckt version --json` and `docket version --json` report identical output. +5. `uninstall` leaves both binary and alias in place. +6. The downloader's spelling-ban and PATH-sandbox tests pass with the added `ln -s`. Removing the alias step turns the new downloader test red (mutation-tested). + +## Out of scope + +- Renaming the binary or the catalog spelling. +- Shell aliases, completions, and package formulas. +- The user-level surfaces that invoke `dckt` (the private-instructions change). From 36ceb29b1863d4140c3d6a691a7cbdec12146eca Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 10:06:03 -0400 Subject: [PATCH 02/11] feat(install): classify the dckt alias beside an installed binary --- internal/install/alias.go | 78 ++++++++++++++++++ internal/install/alias_test.go | 143 +++++++++++++++++++++++++++++++++ internal/install/state.go | 4 +- internal/install/state_test.go | 17 ++++ 4 files changed, 240 insertions(+), 2 deletions(-) create mode 100644 internal/install/alias.go create mode 100644 internal/install/alias_test.go diff --git a/internal/install/alias.go b/internal/install/alias.go new file mode 100644 index 000000000..693190c8e --- /dev/null +++ b/internal/install/alias.go @@ -0,0 +1,78 @@ +package install + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" +) + +// Every install also places a second, shorter name for the binary beside it: +// AliasName, a symlink to the installed docket. It follows the binary — same +// directory, same ownership posture, left in place by uninstall — and a dckt +// docket did not create is never overwritten. A missing or foreign alias is a +// finding, never a failure: the binary is installed either way. +const ( + AliasName = "dckt" + // roleBinaryAlias marks the development install's alias in the installed + // state. Like roleBinary it belongs to the installation, never a harness. + roleBinaryAlias = "binary-alias" +) + +// The alias finding kinds. +const ( + AliasMissing = "missing" + AliasForeign = "foreign" +) + +// AliasFinding is one alias that is not what an install would leave: absent, +// or occupied by something that does not resolve to the binary. +type AliasFinding struct { + Kind string + Path string + Binary string + Remedy string +} + +const ( + remedyAliasMissing = "re-run the installer that placed docket here (the release install.sh, or docket development install) to create it" + remedyAliasForeign = "docket did not create what is at this path and will not replace it; move or delete it, then re-run the installer to get the dckt alias" +) + +// AliasPathFor is where the alias for binary lives: beside it. +func AliasPathFor(binary string) string { + return filepath.Join(filepath.Dir(binary), AliasName) +} + +// InspectBinaryAlias classifies the alias beside binary. A nil finding means a +// symlink that resolves — every hop canonicalised — to binary. Absence is +// AliasMissing; anything else at the path (a link elsewhere, a dangling link, +// a file, a directory) is AliasForeign. It only reads. An error means the +// probe itself failed, which is never reported as a clean absence. +func InspectBinaryAlias(binary string) (*AliasFinding, error) { + path := AliasPathFor(binary) + info, err := os.Lstat(path) + switch { + case errors.Is(err, fs.ErrNotExist): + return &AliasFinding{Kind: AliasMissing, Path: path, Binary: binary, Remedy: remedyAliasMissing}, nil + case err != nil: + return nil, fmt.Errorf("install: inspecting %s: %w", path, err) + } + if info.Mode()&fs.ModeSymlink != 0 { + have, err := linkDestination(path) + if err != nil { + return nil, err + } + want, err := canonicalPath(binary) + if err != nil { + return nil, err + } + if have == want { + if _, err := os.Stat(path); err == nil { + return nil, nil + } + } + } + return &AliasFinding{Kind: AliasForeign, Path: path, Binary: binary, Remedy: remedyAliasForeign}, nil +} diff --git a/internal/install/alias_test.go b/internal/install/alias_test.go new file mode 100644 index 000000000..300d23143 --- /dev/null +++ b/internal/install/alias_test.go @@ -0,0 +1,143 @@ +package install + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/danielhanold/docket/internal/testsupport" +) + +// aliasBin makes a canonical bin dir holding a docket binary and returns both. +func aliasBin(t *testing.T) (string, string) { + t.Helper() + dir, err := filepath.EvalSymlinks(testsupport.TempDir(t)) + if err != nil { + t.Fatal(err) + } + bin := filepath.Join(dir, "bin") + if err := os.MkdirAll(bin, 0o755); err != nil { + t.Fatal(err) + } + binary := filepath.Join(bin, "docket") + if err := os.WriteFile(binary, []byte("binary\n"), 0o755); err != nil { + t.Fatal(err) + } + return bin, binary +} + +func TestInspectBinaryAlias(t *testing.T) { + t.Run("absent is missing", func(t *testing.T) { + _, binary := aliasBin(t) + f, err := InspectBinaryAlias(binary) + if err != nil || f == nil || f.Kind != AliasMissing || f.Path != AliasPathFor(binary) || f.Remedy == "" { + t.Fatalf("finding = %+v, err %v; want missing with a remedy", f, err) + } + }) + t.Run("relative link (downloader spelling) is healthy", func(t *testing.T) { + bin, binary := aliasBin(t) + if err := os.Symlink("docket", filepath.Join(bin, AliasName)); err != nil { + t.Fatal(err) + } + if f, err := InspectBinaryAlias(binary); err != nil || f != nil { + t.Fatalf("finding = %+v, err %v; want healthy", f, err) + } + }) + t.Run("absolute link (development spelling) is healthy", func(t *testing.T) { + bin, binary := aliasBin(t) + if err := os.Symlink(binary, filepath.Join(bin, AliasName)); err != nil { + t.Fatal(err) + } + if f, err := InspectBinaryAlias(binary); err != nil || f != nil { + t.Fatalf("finding = %+v, err %v; want healthy", f, err) + } + }) + t.Run("alias reached through a symlinked directory is healthy", func(t *testing.T) { + bin, binary := aliasBin(t) + via := filepath.Join(filepath.Dir(bin), "via") + if err := os.Symlink(bin, via); err != nil { + t.Fatal(err) + } + // The link spells the binary through the symlinked directory; the + // identity check must canonicalise every hop and still call it ours. + if err := os.Symlink(filepath.Join(via, "docket"), filepath.Join(bin, AliasName)); err != nil { + t.Fatal(err) + } + if f, err := InspectBinaryAlias(filepath.Join(via, "docket")); err != nil || f != nil { + t.Fatalf("finding = %+v, err %v; want healthy", f, err) + } + // The same alias probed through the canonical spelling of the binary + // differs from the link's text; only a canonical comparison calls it ours. + if f, err := InspectBinaryAlias(binary); err != nil || f != nil { + t.Fatalf("canonical spelling: finding = %+v, err %v; want healthy", f, err) + } + }) + t.Run("link elsewhere is foreign", func(t *testing.T) { + bin, binary := aliasBin(t) + other := filepath.Join(bin, "other") + if err := os.WriteFile(other, []byte("x"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(other, filepath.Join(bin, AliasName)); err != nil { + t.Fatal(err) + } + f, err := InspectBinaryAlias(binary) + if err != nil || f == nil || f.Kind != AliasForeign { + t.Fatalf("finding = %+v, err %v; want foreign", f, err) + } + }) + t.Run("dangling link is foreign", func(t *testing.T) { + bin, binary := aliasBin(t) + if err := os.Symlink(filepath.Join(bin, "gone"), filepath.Join(bin, AliasName)); err != nil { + t.Fatal(err) + } + if f, err := InspectBinaryAlias(binary); err != nil || f == nil || f.Kind != AliasForeign { + t.Fatalf("finding = %+v, err %v; want foreign", f, err) + } + }) + t.Run("regular file is foreign", func(t *testing.T) { + bin, binary := aliasBin(t) + if err := os.WriteFile(filepath.Join(bin, AliasName), []byte("mine\n"), 0o755); err != nil { + t.Fatal(err) + } + if f, err := InspectBinaryAlias(binary); err != nil || f == nil || f.Kind != AliasForeign { + t.Fatalf("finding = %+v, err %v; want foreign", f, err) + } + }) + t.Run("directory is foreign", func(t *testing.T) { + bin, binary := aliasBin(t) + if err := os.Mkdir(filepath.Join(bin, AliasName), 0o755); err != nil { + t.Fatal(err) + } + if f, err := InspectBinaryAlias(binary); err != nil || f == nil || f.Kind != AliasForeign { + t.Fatalf("finding = %+v, err %v; want foreign", f, err) + } + }) + t.Run("probe error is an error, not missing", func(t *testing.T) { + if runtime.GOOS == "windows" || os.Geteuid() == 0 { + t.Skip("permission bits do not deny root") + } + bin, binary := aliasBin(t) + if err := os.Chmod(bin, 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(bin, 0o755) }) + f, err := InspectBinaryAlias(binary) + if err == nil { + t.Fatalf("finding = %+v with nil error; an unreadable bin dir must not read as a clean absence", f) + } + }) + t.Run("link to a vanished binary is foreign", func(t *testing.T) { + bin, binary := aliasBin(t) + if err := os.Symlink("docket", filepath.Join(bin, AliasName)); err != nil { + t.Fatal(err) + } + if err := os.Remove(binary); err != nil { + t.Fatal(err) + } + if f, err := InspectBinaryAlias(binary); err != nil || f == nil || f.Kind != AliasForeign { + t.Fatalf("finding = %+v, err %v; want foreign (the alias resolves to nothing)", f, err) + } + }) +} diff --git a/internal/install/state.go b/internal/install/state.go index 4f130359f..d592b0a8d 100644 --- a/internal/install/state.go +++ b/internal/install/state.go @@ -162,8 +162,8 @@ func ValidateState(s *State) error { if err := validateTarget(target); err != nil { return fmt.Errorf("target %q: %w", target.Path, err) } - if target.Role == roleBinary && target.Harness != "" { - return fmt.Errorf("binary target %q is attributed to harness %q", target.Path, target.Harness) + if (target.Role == roleBinary || target.Role == roleBinaryAlias) && target.Harness != "" { + return fmt.Errorf("installation target %q (role %s) is attributed to harness %q", target.Path, target.Role, target.Harness) } if target.Harness != "" { if !harnesses[target.Harness] { diff --git a/internal/install/state_test.go b/internal/install/state_test.go index d5543631f..bcb74f084 100644 --- a/internal/install/state_test.go +++ b/internal/install/state_test.go @@ -288,3 +288,20 @@ func TestWriteStateAtomicNoTorn(t *testing.T) { } } } + +func TestStateRejectsHarnessAttributedAlias(t *testing.T) { + s := sampleState() + unattributed := TargetRecord{Path: "/home/u/.local/bin/dckt", Kind: KindSymlink, + LinkTarget: "/home/u/.local/bin/docket", Role: roleBinaryAlias} + s.Targets = append(s.Targets, unattributed) + if err := ValidateState(s); err != nil { + t.Fatalf("an unattributed alias record was rejected: %v", err) + } + s = sampleState() + attributed := unattributed + attributed.Harness = s.Harnesses[0] + s.Targets = append(s.Targets, attributed) + if err := ValidateState(s); err == nil { + t.Fatal("a harness-attributed alias record validated") + } +} From 72f0eae328b66148c5bd6d4b6c30b42f8d942d50 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 10:10:16 -0400 Subject: [PATCH 03/11] feat(install): development install places the dckt alias beside the binary --- internal/install/alias.go | 20 +++ internal/install/devmode.go | 17 +++ internal/install/devmode_test.go | 216 +++++++++++++++++++++++++++++ internal/install/service.go | 8 +- internal/install/uninstall_test.go | 22 ++- 5 files changed, 279 insertions(+), 4 deletions(-) diff --git a/internal/install/alias.go b/internal/install/alias.go index 693190c8e..757943ed3 100644 --- a/internal/install/alias.go +++ b/internal/install/alias.go @@ -76,3 +76,23 @@ func InspectBinaryAlias(binary string) (*AliasFinding, error) { } return &AliasFinding{Kind: AliasForeign, Path: path, Binary: binary, Remedy: remedyAliasForeign}, nil } + +// planBinaryAlias is the development install's alias step: the symlink target +// it would own beside binary, or — when what is there is not provably docket's +// — no target and a foreign finding. It inspects with the same ownership proofs +// every other target gets, so "already owned" means exactly what it means for a +// harness link: a link already resolving to binary, or one the prior state +// recorded and that still matches its record. A conflict is reported rather +// than returned as a refusal: the binary install must never be failed by the +// alias. +func planBinaryAlias(binary string, prior *State) (*Target, *AliasFinding, error) { + t := Target{Path: AliasPathFor(binary), Kind: KindSymlink, LinkTarget: binary, Role: roleBinaryAlias} + inspection, err := InspectTarget(t, prior, nil) + if err != nil { + return nil, nil, err + } + if inspection.Disposition == DispositionConflict { + return nil, &AliasFinding{Kind: AliasForeign, Path: t.Path, Binary: binary, Remedy: remedyAliasForeign}, nil + } + return &t, nil, nil +} diff --git a/internal/install/devmode.go b/internal/install/devmode.go index d4390187f..0de968ba4 100644 --- a/internal/install/devmode.go +++ b/internal/install/devmode.go @@ -282,6 +282,20 @@ func developmentInstallCandidate(o DevOptions) Outcome { Mode: binaryMode, Role: roleBinary, }) + // The alias rides the same transaction as the binary it names, so a + // rollback removes an alias this run created and restores nothing it did + // not. A foreign dckt is left out of the plan and reported, never refused. + prior, err := LoadState(o.Roots.StatePath()) + if err != nil { + return fail(out, ReasonStateInvalid, err) + } + aliasTarget, aliasFinding, err := planBinaryAlias(installedBinary, prior) + if err != nil { + return fail(out, ReasonFilesystemFailed, err) + } + if aliasTarget != nil { + targets = append(targets, *aliasTarget) + } // The candidate re-resolved the repository phase itself (the parent passed // --repo-dir through verbatim), so machine and repository writes ride one @@ -296,6 +310,9 @@ func developmentInstallCandidate(o DevOptions) Outcome { sourceRoot: ds.source, sourceDigest: ds.digest, }, o.RepoPhase, out) + if aliasFinding != nil { + out.AliasFindings = append(out.AliasFindings, *aliasFinding) + } return collectPostCommitLocked(out, lock, func() CollectionOutcome { return collectLocked(CollectOptions{Roots: o.Roots, FS: o.FS}, lock) }) diff --git a/internal/install/devmode_test.go b/internal/install/devmode_test.go index c423d1110..adc4a73c8 100644 --- a/internal/install/devmode_test.go +++ b/internal/install/devmode_test.go @@ -808,3 +808,219 @@ func TestDevCandidateRefusesDrift(t *testing.T) { } assertUnchanged(t, homeBefore, snapshot(t, w.home), "drifted candidate source") } + +// --------------------------------------------------------------------------- +// The dckt alias +// --------------------------------------------------------------------------- + +// The development install places dckt beside the binary it installs, as a +// recorded, journaled target attributed to no harness. +func TestDevInstallPlacesTheAlias(t *testing.T) { + w := newWorld(t) + mkdirAll(t, w.path(".toy")) + src := newSource(t) + bin := filepath.Join(w.home, "bin") + + out := install.DevelopmentInstall(w.devCandidate(t, src, bin)) + if out.Err != nil { + t.Fatalf("DevelopmentInstall: %v (reason %q)", out.Err, out.Reason) + } + alias := filepath.Join(bin, install.AliasName) + binary := filepath.Join(bin, "docket") + dest, err := os.Readlink(alias) + if err != nil { + t.Fatalf("alias not created: %v", err) + } + if dest != binary { + t.Errorf("alias -> %s, want %s", dest, binary) + } + if _, ok := findAction(out, install.OpCreate, alias); !ok { + t.Errorf("no create action reported for %s: %v", alias, out.Actions) + } + if len(out.AliasFindings) != 0 { + t.Errorf("a fresh install reported alias findings: %+v", out.AliasFindings) + } + var rec *install.TargetRecord + for _, r := range loadState(t, w.roots).Targets { + if r.Path == alias { + r := r + rec = &r + } + } + if rec == nil || rec.Kind != install.KindSymlink || rec.Harness != "" { + t.Fatalf("alias record = %+v, want an unattributed symlink record", rec) + } + + // A second run converges: nothing applied, nothing changed on disk. + before := snapshot(t, w.home) + again := install.DevelopmentInstall(w.devCandidate(t, src, bin)) + if again.Err != nil || again.Applied || len(again.AliasFindings) != 0 { + t.Fatalf("second run = %+v", again) + } + assertUnchanged(t, before, snapshot(t, w.home), "second development install") +} + +// A dckt the release downloader left (a RELATIVE link to docket) is already +// ours: the development install leaves it, reporting no finding. +func TestDevInstallAcceptsTheDownloadersRelativeAlias(t *testing.T) { + w := newWorld(t) + mkdirAll(t, w.path(".toy")) + bin := filepath.Join(w.home, "bin") + mkdirAll(t, bin) + alias := filepath.Join(bin, install.AliasName) + if err := os.Symlink("docket", alias); err != nil { + t.Fatal(err) + } + out := install.DevelopmentInstall(w.devCandidate(t, newSource(t), bin)) + if out.Err != nil || len(out.AliasFindings) != 0 { + t.Fatalf("outcome = %+v", out) + } + if dest, _ := os.Readlink(alias); dest != "docket" { + t.Errorf("the downloader's relative alias was rewritten to %q", dest) + } +} + +// A dckt docket does not own is never touched; the install still succeeds and +// reports the foreign alias as a finding, and the alias is not recorded. +func TestDevInstallPreservesAForeignAlias(t *testing.T) { + for _, tc := range []struct { + name string + plant func(t *testing.T, alias string) + }{ + {"file", func(t *testing.T, alias string) { writeFile(t, alias, "#!/bin/sh\necho mine\n") }}, + {"link elsewhere", func(t *testing.T, alias string) { + if err := os.Symlink("/usr/bin/true", alias); err != nil { + t.Fatal(err) + } + }}, + {"dangling link", func(t *testing.T, alias string) { + if err := os.Symlink(filepath.Join(filepath.Dir(alias), "gone"), alias); err != nil { + t.Fatal(err) + } + }}, + {"directory", func(t *testing.T, alias string) { + writeFile(t, filepath.Join(alias, "keep"), "mine\n") + }}, + } { + t.Run(tc.name, func(t *testing.T) { + w := newWorld(t) + mkdirAll(t, w.path(".toy")) + bin := filepath.Join(w.home, "bin") + mkdirAll(t, bin) + alias := filepath.Join(bin, install.AliasName) + tc.plant(t, alias) + aliasBefore := snapshot(t, alias) + + out := install.DevelopmentInstall(w.devCandidate(t, newSource(t), bin)) + if out.Err != nil || !out.Applied { + t.Fatalf("a foreign alias failed the install: %+v", out) + } + if len(out.AliasFindings) != 1 || out.AliasFindings[0].Kind != install.AliasForeign || + out.AliasFindings[0].Path != alias || out.AliasFindings[0].Remedy == "" { + t.Fatalf("alias findings = %+v, want one foreign finding with a remedy", out.AliasFindings) + } + assertUnchanged(t, aliasBefore, snapshot(t, alias), "foreign alias") + if _, err := os.Stat(filepath.Join(bin, "docket")); err != nil { + t.Errorf("the binary was not installed: %v", err) + } + for _, rec := range loadState(t, w.roots).Targets { + if rec.Path == alias { + t.Errorf("a foreign alias was recorded as docket's: %+v", rec) + } + } + }) + } + + // An alias docket recorded once and the user has since replaced is no + // longer provably docket's: the record is no licence to overwrite it. + t.Run("recorded alias replaced by the user", func(t *testing.T) { + w := newWorld(t) + mkdirAll(t, w.path(".toy")) + src := newSource(t) + bin := filepath.Join(w.home, "bin") + if out := install.DevelopmentInstall(w.devCandidate(t, src, bin)); out.Err != nil { + t.Fatalf("first DevelopmentInstall: %v (reason %q)", out.Err, out.Reason) + } + alias := filepath.Join(bin, install.AliasName) + if err := os.Remove(alias); err != nil { + t.Fatal(err) + } + writeFile(t, alias, "#!/bin/sh\necho mine\n") + aliasBefore := snapshot(t, alias) + + out := install.DevelopmentInstall(w.devCandidate(t, src, bin)) + if out.Err != nil { + t.Fatalf("a replaced alias failed the install: %v (reason %q)", out.Err, out.Reason) + } + if len(out.AliasFindings) != 1 || out.AliasFindings[0].Kind != install.AliasForeign { + t.Fatalf("alias findings = %+v, want one foreign finding", out.AliasFindings) + } + assertUnchanged(t, aliasBefore, snapshot(t, alias), "replaced alias") + }) +} + +// A failed transaction rolls back an alias it created, and leaves a +// pre-existing one exactly as it was. +func TestDevInstallRollbackAndTheAlias(t *testing.T) { + const injected = "injected: state commit fails" + failCommit := func(w *world) install.FSOps { + return &failingFS{inner: install.RealFS{}, fail: func(op, path string) error { + if op == "Rename" && path == w.roots.StatePath() { + return errors.New(injected) + } + return nil + }} + } + t.Run("created alias is removed", func(t *testing.T) { + w := newWorld(t) + mkdirAll(t, w.path(".toy")) + bin := filepath.Join(w.home, "bin") + o := w.devCandidate(t, newSource(t), bin) + o.FS = failCommit(w) + out := install.DevelopmentInstall(o) + if out.Err == nil || !strings.Contains(out.Err.Error(), injected) { + t.Fatalf("err = %v, want the injected commit failure", out.Err) + } + if _, err := os.Lstat(filepath.Join(bin, install.AliasName)); !os.IsNotExist(err) { + t.Fatalf("rollback left the alias it created: %v", err) + } + if _, err := os.Lstat(filepath.Join(bin, "docket")); !os.IsNotExist(err) { + t.Fatalf("rollback left the binary it created: %v", err) + } + }) + t.Run("pre-existing alias survives", func(t *testing.T) { + w := newWorld(t) + mkdirAll(t, w.path(".toy")) + bin := filepath.Join(w.home, "bin") + mkdirAll(t, bin) + alias := filepath.Join(bin, install.AliasName) + if err := os.Symlink("docket", alias); err != nil { + t.Fatal(err) + } + o := w.devCandidate(t, newSource(t), bin) + o.FS = failCommit(w) + out := install.DevelopmentInstall(o) + if out.Err == nil || !strings.Contains(out.Err.Error(), injected) { + t.Fatalf("err = %v, want the injected commit failure", out.Err) + } + if dest, err := os.Readlink(alias); err != nil || dest != "docket" { + t.Fatalf("rollback disturbed the pre-existing alias: %q, %v", dest, err) + } + }) + t.Run("foreign alias survives", func(t *testing.T) { + w := newWorld(t) + mkdirAll(t, w.path(".toy")) + bin := filepath.Join(w.home, "bin") + alias := filepath.Join(bin, install.AliasName) + writeFile(t, alias, "#!/bin/sh\necho mine\n") + o := w.devCandidate(t, newSource(t), bin) + o.FS = failCommit(w) + out := install.DevelopmentInstall(o) + if out.Err == nil || !strings.Contains(out.Err.Error(), injected) { + t.Fatalf("err = %v, want the injected commit failure", out.Err) + } + if got := readFile(t, alias); got != "#!/bin/sh\necho mine\n" { + t.Fatalf("rollback disturbed the foreign alias: %q", got) + } + }) +} diff --git a/internal/install/service.go b/internal/install/service.go index d51609867..84ce064ab 100644 --- a/internal/install/service.go +++ b/internal/install/service.go @@ -176,8 +176,12 @@ type Outcome struct { // out of Err and Reason: a published installation remains successful even // when reclaiming an old version tree needs a later retry. Collection CollectionOutcome - Reason string - Err error + // AliasFindings are dckt aliases an operation found missing or foreign. + // They are findings, never failures: they leave Reason and Err alone, and + // the app layer renders them as warnings. + AliasFindings []AliasFinding + Reason string + Err error // Relayed marks a development-install PARENT outcome whose candidate has // already printed the sole result document to the shared stdout. The parent // itself planned nothing and wrote nothing; the CLI presenter emits no diff --git a/internal/install/uninstall_test.go b/internal/install/uninstall_test.go index b69d9a6a6..1ff3827d5 100644 --- a/internal/install/uninstall_test.go +++ b/internal/install/uninstall_test.go @@ -32,7 +32,7 @@ func newUninstallFixture(t *testing.T) uninstallFixture { t.Fatal(err) } paths := map[string]string{} - records := make([]TargetRecord, 0, 5) + records := make([]TargetRecord, 0, 6) for _, harness := range uninstallHarnesses { path := filepath.Join(roots.Home, "."+harness, "agents", "docket.md") body := harness + "\n" @@ -43,6 +43,11 @@ func newUninstallFixture(t *testing.T) uninstallFixture { binary := filepath.Join(roots.BinDir, "docket") writeFileOrDie(t, binary, "binary\n") records = append(records, TargetRecord{Path: binary, Kind: KindFile, SHA256: hashBytes([]byte("binary\n")), Role: roleBinary}) + alias := filepath.Join(roots.BinDir, AliasName) + if err := os.Symlink(binary, alias); err != nil { + t.Fatal(err) + } + records = append(records, TargetRecord{Path: alias, Kind: KindSymlink, LinkTarget: binary, Role: roleBinaryAlias}) state := &State{FormatVersion: StateFormatVersion, ProductVersion: "v-test", AssetProtocol: 1, AssetSetID: manifest.AssetSetID, Mode: ModeRelease, Harnesses: append([]string(nil), uninstallHarnesses...), AgentDigest: "sha256:agents", Targets: records} @@ -79,10 +84,23 @@ func TestUninstallAllAndScopedHarnesses(t *testing.T) { if got := readOrDie(t, binary); got != "binary\n" { t.Fatalf("unattributed binary = %q", got) } + alias := filepath.Join(f.roots.BinDir, AliasName) + if dest, err := os.Readlink(alias); err != nil || dest != binary { + t.Fatalf("uninstall disturbed the alias: %q, %v", dest, err) + } state, err := LoadState(f.roots.StatePath()) - if err != nil || len(state.Harnesses) != 0 || state.AssetSetID != "" || len(state.Targets) != 1 { + if err != nil || len(state.Harnesses) != 0 || state.AssetSetID != "" || len(state.Targets) != 2 { t.Fatalf("empty published state = %#v, %v", state, err) } + // The installation's own binary and its alias are the two records an + // uninstall keeps: it retires harness material, never the installation. + kept := map[string]string{} + for _, rec := range state.Targets { + kept[rec.Path] = rec.Role + } + if kept[binary] != roleBinary || kept[alias] != roleBinaryAlias { + t.Fatalf("kept records = %v, want the binary and its alias", kept) + } if _, err := os.Lstat(filepath.Dir(f.roots.VersionDir(f.state.AssetSetID))); !errors.Is(err, fs.ErrNotExist) { t.Fatalf("unreferenced version tree remains: %v", err) } From 053d815ee4d8f1e3b7073d7730140188eaa6f95a Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 10:12:00 -0400 Subject: [PATCH 04/11] feat(install): install check reports a missing or foreign dckt as a finding --- internal/install/alias.go | 75 ++++++++++++++++++++++++++++++++ internal/install/alias_test.go | 46 ++++++++++++++++++++ internal/install/devmode_test.go | 38 ++++++++++++++++ internal/install/roots.go | 14 ++++++ internal/install/roots_test.go | 22 ++++++++++ internal/install/service.go | 10 +++++ internal/install/service_test.go | 36 +++++++++++++++ 7 files changed, 241 insertions(+) diff --git a/internal/install/alias.go b/internal/install/alias.go index 757943ed3..ee62483c8 100644 --- a/internal/install/alias.go +++ b/internal/install/alias.go @@ -1,11 +1,13 @@ package install import ( + "bufio" "errors" "fmt" "io/fs" "os" "path/filepath" + "strings" ) // Every install also places a second, shorter name for the binary beside it: @@ -96,3 +98,76 @@ func planBinaryAlias(binary string, prior *State) (*Target, *AliasFinding, error } return &t, nil, nil } + +// readReleaseBinaryPath returns the absolute binary path the release +// downloader's record names, or "" when there is no record or it names no +// usable absolute path — the downloader itself grants no ownership to such a +// record. A record that exists but cannot be read is an error, not an absence. +func readReleaseBinaryPath(recordPath string) (string, error) { + if recordPath == "" { + return "", nil + } + f, err := os.Open(recordPath) + switch { + case errors.Is(err, fs.ErrNotExist): + return "", nil + case err != nil: + return "", fmt.Errorf("install: reading %s: %w", recordPath, err) + } + defer f.Close() + sc := bufio.NewScanner(f) + for sc.Scan() { + if v, ok := strings.CutPrefix(sc.Text(), "path="); ok { + if filepath.IsAbs(v) { + return filepath.Clean(v), nil + } + return "", nil + } + } + if err := sc.Err(); err != nil { + return "", fmt.Errorf("install: reading %s: %w", recordPath, err) + } + return "", nil +} + +// checkBinaryAliases classifies the alias beside every binary an installer +// owns: the development installation's recorded binary, and the binary the +// release downloader's record names. It only reads. +func checkBinaryAliases(state *State, roots UserRoots) ([]AliasFinding, error) { + var binaries []string + seen := map[string]bool{} + add := func(p string) { + if p == "" { + return + } + p = filepath.Clean(p) + if !seen[p] { + seen[p] = true + binaries = append(binaries, p) + } + } + if state != nil { + for _, rec := range state.Targets { + if rec.Role == roleBinary { + add(rec.Path) + } + } + } + release, err := readReleaseBinaryPath(roots.ReleaseBinaryRecordPath()) + if err != nil { + return nil, err + } + add(release) + + var findings []AliasFinding + for _, binary := range binaries { + f, err := InspectBinaryAlias(binary) + if err != nil { + return nil, err + } + if f != nil { + findings = append(findings, *f) + } + } + return findings, nil +} diff --git a/internal/install/alias_test.go b/internal/install/alias_test.go index 300d23143..0620292ff 100644 --- a/internal/install/alias_test.go +++ b/internal/install/alias_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" "github.com/danielhanold/docket/internal/testsupport" @@ -141,3 +142,48 @@ func TestInspectBinaryAlias(t *testing.T) { } }) } + +func TestReadReleaseBinaryPath(t *testing.T) { + dir := testsupport.TempDir(t) + rec := filepath.Join(dir, "release-binary.record") + if got, err := readReleaseBinaryPath(rec); err != nil || got != "" { + t.Fatalf("absent record = %q, %v; want empty, nil", got, err) + } + write := func(body string) { + if err := os.WriteFile(rec, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + write("path=/abs/bin/docket\nversion=v1.0.0\nsha256=abc\nalias=/abs/bin/dckt\n") + if got, err := readReleaseBinaryPath(rec); err != nil || got != "/abs/bin/docket" { + t.Fatalf("record path = %q, %v", got, err) + } + write("version=v1.0.0\n") + if got, err := readReleaseBinaryPath(rec); err != nil || got != "" { + t.Fatalf("record without path= = %q, %v; want empty", got, err) + } + write("path=relative/docket\n") + if got, err := readReleaseBinaryPath(rec); err != nil || got != "" { + t.Fatalf("relative path= = %q, %v; want empty", got, err) + } +} + +// The Go reader and the POSIX writer spell the record location and its path= +// key independently; this ties the two so a respelling on either side reddens. +func TestReleaseRecordSpellingMatchesDownloader(t *testing.T) { + src, err := os.ReadFile(filepath.Join("..", "release", "downloader", "install.sh")) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + `record="${XDG_STATE_HOME:-$HOME/.local/state}/docket/release-binary.record"`, + `printf 'path=%s\n' "$dest"`, + } { + if !strings.Contains(string(src), want) { + t.Errorf("downloader no longer contains %q; ReleaseBinaryRecordPath/readReleaseBinaryPath must move with it", want) + } + } + if got := (UserRoots{StateHome: "/s"}).ReleaseBinaryRecordPath(); got != "/s/docket/release-binary.record" { + t.Errorf("ReleaseBinaryRecordPath = %q", got) + } +} diff --git a/internal/install/devmode_test.go b/internal/install/devmode_test.go index adc4a73c8..3019172f2 100644 --- a/internal/install/devmode_test.go +++ b/internal/install/devmode_test.go @@ -658,9 +658,47 @@ func TestCheckVerifiesTheDevelopmentBinary(t *testing.T) { if _, ok := findAction(out, install.OpDrift, binary); ok { t.Errorf("an intact binary was reported as drift") } + if len(out.AliasFindings) != 0 { + t.Errorf("a healthy development install reported alias findings: %+v", out.AliasFindings) + } assertUnchanged(t, before, snapshot(t, w.home), "check over a healthy development install") }) + t.Run("alias deleted is a finding, not drift", func(t *testing.T) { + w, o, binary := setup(t) + alias := filepath.Join(filepath.Dir(binary), install.AliasName) + if err := os.Remove(alias); err != nil { + t.Fatal(err) + } + before := snapshot(t, w.home) + out := install.Check(o) + if out.Reason != "" || out.Err != nil { + t.Fatalf("a missing alias failed the check: reason %q err %v", out.Reason, out.Err) + } + if len(out.AliasFindings) != 1 || out.AliasFindings[0].Kind != install.AliasMissing || out.AliasFindings[0].Path != alias { + t.Fatalf("alias findings = %+v", out.AliasFindings) + } + assertUnchanged(t, before, snapshot(t, w.home), "check over a missing alias") + }) + + t.Run("alias foreign is a finding, not drift", func(t *testing.T) { + w, o, binary := setup(t) + alias := filepath.Join(filepath.Dir(binary), install.AliasName) + if err := os.Remove(alias); err != nil { + t.Fatal(err) + } + writeFile(t, alias, "mine\n") + before := snapshot(t, w.home) + out := install.Check(o) + if out.Reason != "" || out.Err != nil { + t.Fatalf("a foreign alias failed the check: reason %q err %v", out.Reason, out.Err) + } + if len(out.AliasFindings) != 1 || out.AliasFindings[0].Kind != install.AliasForeign { + t.Fatalf("alias findings = %+v", out.AliasFindings) + } + assertUnchanged(t, before, snapshot(t, w.home), "check over a foreign alias") + }) + t.Run("replaced", func(t *testing.T) { w, o, binary := setup(t) if err := os.WriteFile(binary, []byte("someone else's binary\n"), 0o755); err != nil { diff --git a/internal/install/roots.go b/internal/install/roots.go index b66534e4a..8e4afd051 100644 --- a/internal/install/roots.go +++ b/internal/install/roots.go @@ -20,6 +20,7 @@ type UserRoots struct { DataRoot string // /docket ConfigHome string // XDG_CONFIG_HOME or ~/.config (for opencode) BinDir string // XDG_BIN_HOME or ~/.local/bin (development mode) + StateHome string // XDG_STATE_HOME or ~/.local/state (the release downloader's record) } // ResolveRoots reads the environment through the injected getenv func and the @@ -56,6 +57,7 @@ func ResolveRoots(homeFn func() (string, error), getenv func(string) string) (Us DataRoot: filepath.Join(xdgOr(getenv, "XDG_DATA_HOME", filepath.Join(home, ".local", "share")), "docket"), ConfigHome: xdgOr(getenv, "XDG_CONFIG_HOME", filepath.Join(home, ".config")), BinDir: xdgOr(getenv, "XDG_BIN_HOME", filepath.Join(home, ".local", "bin")), + StateHome: xdgOr(getenv, "XDG_STATE_HOME", filepath.Join(home, ".local", "state")), } for _, p := range []string{roots.Home, roots.DataRoot, roots.ConfigHome, roots.BinDir} { @@ -119,6 +121,18 @@ func (r UserRoots) CollectionQuarantineDir() string { // StatePath is the published ownership manifest. func (r UserRoots) StatePath() string { return filepath.Join(r.DataRoot, "state", "install.json") } +// ReleaseBinaryRecordPath is the release downloader's ownership record: where +// it says the release binary it installed lives. It is the downloader's file, +// spelled independently in internal/release/downloader/install.sh and tied to +// this spelling by TestReleaseRecordSpellingMatchesDownloader. Empty roots +// (StateHome unset) name no record. +func (r UserRoots) ReleaseBinaryRecordPath() string { + if r.StateHome == "" { + return "" + } + return filepath.Join(r.StateHome, "docket", "release-binary.record") +} + // LockPath is the file carrying the exclusive installation lock. It sits at the // root of the data tree because it serializes every mutation under it; see // lock.go for why the lock is an flock rather than the file's existence. diff --git a/internal/install/roots_test.go b/internal/install/roots_test.go index 0dab08083..572e4a32c 100644 --- a/internal/install/roots_test.go +++ b/internal/install/roots_test.go @@ -273,3 +273,25 @@ func TestVersionDirSanitizesID(t *testing.T) { } } } + +func TestResolveRootsStateHome(t *testing.T) { + home := cleanTempDir(t) + roots, err := ResolveRoots(fixedHome(home), fakeEnv(nil)) + if err != nil { + t.Fatal(err) + } + if got, want := roots.ReleaseBinaryRecordPath(), filepath.Join(home, ".local", "state", "docket", "release-binary.record"); got != want { + t.Errorf("default ReleaseBinaryRecordPath = %q, want %q", got, want) + } + state := filepath.Join(home, "xdg-state") + roots, err = ResolveRoots(fixedHome(home), fakeEnv(map[string]string{"XDG_STATE_HOME": state})) + if err != nil { + t.Fatal(err) + } + if got, want := roots.ReleaseBinaryRecordPath(), filepath.Join(state, "docket", "release-binary.record"); got != want { + t.Errorf("XDG ReleaseBinaryRecordPath = %q, want %q", got, want) + } + if got := (UserRoots{}).ReleaseBinaryRecordPath(); got != "" { + t.Errorf("zero roots ReleaseBinaryRecordPath = %q, want empty", got) + } +} diff --git a/internal/install/service.go b/internal/install/service.go index 84ce064ab..9c3ced956 100644 --- a/internal/install/service.go +++ b/internal/install/service.go @@ -442,6 +442,16 @@ func Check(o Options) Outcome { } drift = append(drift, binaryDrift...) + // The dckt alias beside each installed binary is a finding, never drift: + // the binary works without it, and a check that failed on it would fail + // the release downloader's closing `install check` for a user who merely + // owns a dckt of their own. + aliasFindings, err := checkBinaryAliases(state, o.Roots) + if err != nil { + return fail(out, ReasonFilesystemFailed, err) + } + out.AliasFindings = aliasFindings + prunes, err := PruneCandidates(scopedTo(state, plannerNames(selected)), targets) if err != nil { return fail(out, ReasonStateInvalid, err) diff --git a/internal/install/service_test.go b/internal/install/service_test.go index 7a0898dba..78bda0955 100644 --- a/internal/install/service_test.go +++ b/internal/install/service_test.go @@ -1451,6 +1451,42 @@ func TestCheckNamesTheDriftedTarget(t *testing.T) { } } +// A release install records no binary: the downloader placed it and its own +// record says where. Check reads that record and classifies the alias beside +// the binary it names, reporting a missing one as a finding, never drift. +func TestCheckReportsTheReleaseAlias(t *testing.T) { + w := newWorld(t, ".claude") + o := w.options(nil) + o.Harnesses = []string{"claude"} + if out := install.Install(o); out.Err != nil { + t.Fatalf("Install: %v (reason %q)", out.Err, out.Reason) + } + bin := filepath.Join(w.home, ".local", "bin") + mkdirAll(t, bin) + binary := filepath.Join(bin, "docket") + writeFile(t, binary, "release binary\n") + writeFile(t, filepath.Join(w.home, ".local", "state", "docket", "release-binary.record"), + "path="+binary+"\nversion=v1.0.0\nsha256=x\n") + + check := o + check.FS = panicFS{} + out := install.Check(check) + if out.Reason != "" || out.Err != nil { + t.Fatalf("check: reason %q err %v", out.Reason, out.Err) + } + if len(out.AliasFindings) != 1 || out.AliasFindings[0].Kind != install.AliasMissing || + out.AliasFindings[0].Path != filepath.Join(bin, install.AliasName) { + t.Fatalf("alias findings = %+v, want one missing", out.AliasFindings) + } + + if err := os.Symlink("docket", filepath.Join(bin, install.AliasName)); err != nil { + t.Fatal(err) + } + if out := install.Check(check); out.Err != nil || len(out.AliasFindings) != 0 { + t.Fatalf("a healthy release alias: err %v findings %+v", out.Err, out.AliasFindings) + } +} + func TestCheckDetectsMissingVersionTree(t *testing.T) { w := newWorld(t, ".claude") o := w.options(nil) From c7340a1c69630637a929d43c154563105a5c9ed7 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 10:13:37 -0400 Subject: [PATCH 05/11] feat(app): surface dckt alias findings as install warnings --- internal/app/finding_codes.go | 9 +++++++++ internal/app/install.go | 18 ++++++++++++++++++ internal/app/install_test.go | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/internal/app/finding_codes.go b/internal/app/finding_codes.go index 111427c8f..d2955cb6d 100644 --- a/internal/app/finding_codes.go +++ b/internal/app/finding_codes.go @@ -148,6 +148,13 @@ const ( // collection surfaces when a version tree could not be reclaimed; the primary // install/uninstall result is unaffected. FCCollectionPending FindingCode = "collection-pending" + + // binary-alias-missing / binary-alias-foreign are the warning codes an + // install check or development install surfaces for the dckt alias beside + // an installed binary. They are warnings only: the primary result is + // unaffected. + FCBinaryAliasForeign FindingCode = "binary-alias-foreign" + FCBinaryAliasMissing FindingCode = "binary-alias-missing" ) // AllFindingCodes is the authoritative, sorted, deduplicated vocabulary of every @@ -189,6 +196,8 @@ var AllFindingCodes = []FindingCode{ FindingCode(ReasonAttachPathOccupied), FCArtifactRenderFailed, FCAuthoredInputTooLarge, + FCBinaryAliasForeign, + FCBinaryAliasMissing, FCBranchMalformed, FindingCode("branch-still-exists"), FCCollectionPending, diff --git a/internal/app/install.go b/internal/app/install.go index bc84fe21c..628759571 100644 --- a/internal/app/install.go +++ b/internal/app/install.go @@ -235,9 +235,27 @@ func NewInstallResult(operation string, out install.Outcome) InstallResult { if collectionNeedsAttention(out.Collection) { r.Warnings = append(r.Warnings, collectionWarning(out.Collection)) } + r.Warnings = append(r.Warnings, aliasWarnings(out.AliasFindings)...) return r } +// aliasWarnings renders the dckt alias findings as warnings. The alias is a +// convenience beside the binary, so its absence or a foreign occupant is +// reported with its remedy and never reclassifies the operation. +func aliasWarnings(findings []install.AliasFinding) []InstallWarning { + var out []InstallWarning + for _, f := range findings { + code, msg := FCBinaryAliasMissing, "the dckt alias for "+f.Binary+" is missing" + if f.Kind == install.AliasForeign { + code, msg = FCBinaryAliasForeign, "dckt here is not docket's alias for "+f.Binary+"; left untouched" + } + out = append(out, InstallWarning{Diagnostic: config.Diagnostic{ + Code: string(code), Severity: config.SeverityWarning, Path: f.Path, Message: msg, Remedy: f.Remedy, + }}) + } + return out +} + // classifyInstall maps the service's stable reason to the protocol result. // // The default is deliberately internal-error rather than a plausible-looking diff --git a/internal/app/install_test.go b/internal/app/install_test.go index 85339d4ec..224bdf72c 100644 --- a/internal/app/install_test.go +++ b/internal/app/install_test.go @@ -474,3 +474,37 @@ func TestInstallResultNoWarningsOmitsField(t *testing.T) { t.Fatalf("human text has a phantom warning line:\n%s", r.HumanText()) } } + +// A dckt finding is a warning: it never changes the result class, so a check +// that only finds an alias problem still exits clean. +func TestInstallResultRendersAliasFindingsAsWarnings(t *testing.T) { + out := install.Outcome{ + Mode: install.ModeRelease, + AliasFindings: []install.AliasFinding{ + {Kind: install.AliasMissing, Path: "/b/dckt", Binary: "/b/docket", Remedy: "re-run"}, + {Kind: install.AliasForeign, Path: "/c/dckt", Binary: "/c/docket", Remedy: "move it"}, + }, + } + r := NewInstallResult(OperationInstallCheck, out) + if r.Result != ResultNoOp { + t.Fatalf("result = %q, want %q: an alias finding must not change the class", r.Result, ResultNoOp) + } + if len(r.Warnings) != 2 { + t.Fatalf("warnings = %+v", r.Warnings) + } + want := []struct{ code, path, remedy string }{ + {string(FCBinaryAliasMissing), "/b/dckt", "re-run"}, + {string(FCBinaryAliasForeign), "/c/dckt", "move it"}, + } + for i, w := range want { + got := r.Warnings[i] + if got.Code != w.code || got.Path != w.path || got.Remedy != w.remedy || + got.Severity != config.SeverityWarning || got.Message == "" { + t.Errorf("warning[%d] = %+v, want code %s path %s remedy %s", i, got, w.code, w.path, w.remedy) + } + } + human := r.HumanText() + if !strings.Contains(human, "warning: /c/dckt") || !strings.Contains(human, "move it") { + t.Errorf("human text does not show the foreign alias warning with its remedy:\n%s", human) + } +} From 109e154b8151b51f3f2975bf40eadc0504722b72 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 10:16:31 -0400 Subject: [PATCH 06/11] feat(release): downloader places and records the dckt alias --- internal/release/downloader/install.sh | 26 ++++++++ tests/test_release_downloader.sh | 78 +++++++++++++++++++++-- tests/test_release_downloader_converge.sh | 5 +- tests/test_release_downloader_refusals.sh | 5 +- 4 files changed, 106 insertions(+), 8 deletions(-) diff --git a/internal/release/downloader/install.sh b/internal/release/downloader/install.sh index eb9b94110..75b3eca09 100755 --- a/internal/release/downloader/install.sh +++ b/internal/release/downloader/install.sh @@ -12,6 +12,11 @@ # ever moved into the bin dir or run. Unverified bytes are never executed or installed. # - It keeps its own ownership record and REFUSES to replace a binary it does not own. There is # no --force path. +# - It also places dckt, a RELATIVE symlink to docket, beside the binary (ln -s is its one tool +# beyond the list above). An absent dckt is created; a dckt already resolving to the binary is +# left as is; anything else is left UNTOUCHED with a warning, and the install still succeeds — +# the binary install is never failed by the alias. The record gains alias= only when the +# alias is in place and ours. # - Exit codes: 0 success; 2 usage error (bad flag / non-absolute bin dir / bad version); 1 # every other failure. Every failure prints a one-line actionable diagnostic to stderr. set -u @@ -23,6 +28,7 @@ DOCKET_DEFAULT_VERSION="@DOCKET_DEFAULT_VERSION@" # --- Diagnostics -------------------------------------------------------------------------------- die() { printf '%s\n' "install.sh: $1" >&2; exit "${2:-1}"; } +warn() { printf '%s\n' "install.sh: warning: $1" >&2; } usage() { printf '%s\n' \ @@ -31,6 +37,8 @@ usage() { "Downloads the docket release binary, verifies its SHA-256 against the release" \ "checksums.txt BEFORE extraction, installs it, and records ownership so a later run" \ "will only ever replace a binary it installed." \ +"Also places dckt, a symlink to docket, in the same directory, unless a dckt" \ +"it did not create is already there (then it warns and leaves it alone)." \ "" \ " --version release version to install (default: the stamped bundle version)" \ " --bin-dir absolute install directory (default: \${XDG_BIN_HOME:-\$HOME/.local/bin})" \ @@ -271,6 +279,23 @@ chmod 755 "$stage" || die "cannot set mode 755 on the staged binary" # (3) Only after the asset transaction succeeds, move the staged binary into place. mv -f "$stage" "$dest" || die "cannot move the staged binary into $dest" +# (3b) The dckt alias. Identity is decided by -ef (device + inode, every symlink hop followed), never +# by a link's spelling: the development installer writes an absolute link, this script a relative +# one, and both are ours. A dangling or foreign link fails -ef and is left alone. +alias_path="$bin_dir/dckt" +alias_owned=no +if [ -L "$alias_path" ] && [ "$alias_path" -ef "$dest" ]; then + alias_owned=yes +elif [ ! -e "$alias_path" ] && [ ! -L "$alias_path" ]; then + if ln -s docket "$alias_path"; then + alias_owned=yes + else + warn "could not create the dckt alias at $alias_path; docket is installed, re-run to retry" + fi +else + warn "$alias_path exists and is not docket's alias; left untouched. Move or delete it, then re-run to get dckt." +fi + # (4) Publish the ownership record atomically: write beside it, then mv -f into place. record_dir=$(dirname "$record") mkdir -p "$record_dir" || die "cannot create state dir $record_dir" @@ -278,6 +303,7 @@ record_tmp="$record.tmp.$$" { printf 'path=%s\n' "$dest" printf 'version=%s\n' "$version" + if [ "$alias_owned" = yes ]; then printf 'alias=%s\n' "$alias_path"; fi printf 'sha256=%s\n' "$bin_sha" } > "$record_tmp" || { rm -f "$record_tmp"; die "cannot write the ownership record"; } mv -f "$record_tmp" "$record" || { rm -f "$record_tmp"; die "cannot publish the ownership record"; } diff --git a/tests/test_release_downloader.sh b/tests/test_release_downloader.sh index b1412e47c..c051089b3 100644 --- a/tests/test_release_downloader.sh +++ b/tests/test_release_downloader.sh @@ -8,8 +8,10 @@ # forbidden-interpreter spelling. Sections B–F are the Task 7 hermetic behavior tests — a genuinely # fresh /bin/sh runs the downloader under a locked-down PATH sandbox against a file:// "release" # built inline, proving the checksum-verified install path with BOTH hash providers (sha256sum and -# OpenSSL), under umask 077, with an argument-capturing fake docket. Tasks 8–9 add the refusal and -# convergence sibling FILES. +# OpenSSL), under umask 077, with an argument-capturing fake docket. Section G covers the dckt +# alias: created relative, converged on rerun and on the development installer's absolute link, +# and never taken from a foreign owner (a file, a link elsewhere, a dangling link). Section F, the +# mutation pass, stays last. Tasks 8–9 add the refusal and convergence sibling FILES. # # THE SPELLING BAN (Section A) MATCHES SPELLINGS, NOT THE PROPERTY. Section A's final assertion greps # install.sh for the whole words bash|python|perl|shasum|jq|eval. That is a byte-pattern check: it @@ -149,8 +151,9 @@ chmod 755 "$TRIPWIRE_SRC" # it is not symlinked; dirname IS used by the record path and is required). gzip is required for # tar's `-z`: GNU tar (Linux) execs the external gzip binary, so its absence from the sandbox reddens # `tar -tzf`/`tar -xzf`; BSD tar (macOS) links libz internally and needs none, which is why omitting -# it only fails on Linux. The banned set gets a tripwire each. -DL_REAL_TOOLS='curl tar gzip uname mktemp mkdir mv cp chmod rm grep sed cat dirname' +# it only fails on Linux. ln is the dckt alias step's one new tool (ln -s). The banned set gets a +# tripwire each. +DL_REAL_TOOLS='curl tar gzip uname mktemp mkdir mv cp chmod rm grep sed cat dirname ln' DL_BANNED='bash python python3 perl shasum jq' # Build $SANDBOX/bin: real symlinks for the needed tools + a tripwire for each banned tool. $1 @@ -338,6 +341,73 @@ else ok "sha256sum absent on host — idempotent-rerun case skipped" fi +# --- (G) the dckt alias: created, converged, and never taken from its owner ---------------------- +# Uses the host's readlink in the OUTER shell only (test scaffolding); the downloader itself +# decides with `[ -L ]` / `[ -ef ]` builtins and creates with `ln -s`. +if have sha256sum; then PROV_G=sha256sum; elif have openssl; then PROV_G=openssl; else PROV_G=''; fi +if [ -n "$PROV_G" ]; then + ALIAS_ERR="$WORK/alias.err" + + # (G1) fresh install creates a RELATIVE dckt -> docket and records it. + dl_build_sandbox "$PROV_G" || nok "G1: sandbox build failed" + dl_case; VER=v0.1.0; dl_mk_release "$RELEASES" "$VER" + ALIAS="$RUN_BIN/dckt" + dl_run --version "$VER" --harness claude 2>"$ALIAS_ERR"; rc=$? + if [ "$rc" = 0 ]; then ok "alias: fresh install exits 0"; else nok "alias: fresh install exit $rc"; fi + if [ -L "$ALIAS" ] && [ "$(readlink "$ALIAS")" = docket ]; then ok "alias: dckt is a relative symlink to docket"; else nok "alias: dckt missing or not -> docket ($(readlink "$ALIAS" 2>/dev/null))"; fi + if [ "$ALIAS" -ef "$DEST" ]; then ok "alias: dckt resolves to the installed binary"; else nok "alias: dckt does not resolve to $DEST"; fi + if grep -qxF "alias=$ALIAS" "$RECORD"; then ok "alias: ownership record names the alias"; else nok "alias: record lacks alias=: $(tr '\n' '|' < "$RECORD" 2>/dev/null)"; fi + if grep -qF warning "$ALIAS_ERR"; then nok "alias: fresh install warned: $(cat "$ALIAS_ERR")"; else ok "alias: fresh install printed no warning"; fi + if [ -s "$TRIP_LOG" ]; then nok "alias: a banned tool was invoked"; else ok "alias: tripwire log empty"; fi + + # (G2) rerun converges: the link and the record are unchanged, no warning. + link_before=$(readlink "$ALIAS"); rec_before=$(cat "$RECORD") + : > "$FAKE_DOCKET_LOG" + dl_run --version "$VER" --harness claude 2>"$ALIAS_ERR"; rc=$? + if [ "$rc" = 0 ] && [ "$(readlink "$ALIAS")" = "$link_before" ] && [ "$(cat "$RECORD")" = "$rec_before" ]; then + ok "alias: rerun converges (exit 0, link and record unchanged)" + else + nok "alias: rerun did not converge (exit $rc)" + fi + if grep -qF warning "$ALIAS_ERR"; then nok "alias: converging rerun warned"; else ok "alias: converging rerun printed no warning"; fi + + # (G3) a foreign regular file named dckt: untouched, install succeeds with a warning, not recorded. + dl_build_sandbox "$PROV_G" || nok "G3: sandbox build failed" + dl_case; dl_mk_release "$RELEASES" "$VER" + ALIAS="$RUN_BIN/dckt" + printf '#!/bin/sh\necho mine\n' > "$ALIAS"; foreign_sha=$(dl_sha "$ALIAS") + dl_run --version "$VER" --harness claude 2>"$ALIAS_ERR"; rc=$? + if [ "$rc" = 0 ]; then ok "alias-foreign-file: install exits 0"; else nok "alias-foreign-file: exit $rc"; fi + if [ ! -L "$ALIAS" ] && [ "$(dl_sha "$ALIAS")" = "$foreign_sha" ]; then ok "alias-foreign-file: foreign dckt byte-identical"; else nok "alias-foreign-file: foreign dckt was modified"; fi + if grep -qF "warning" "$ALIAS_ERR" && grep -qF -- "$ALIAS" "$ALIAS_ERR"; then ok "alias-foreign-file: warning names the path"; else nok "alias-foreign-file: no warning naming $ALIAS (got: $(tr '\n' ' ' < "$ALIAS_ERR"))"; fi + if grep -q '^alias=' "$RECORD"; then nok "alias-foreign-file: a foreign dckt was recorded as owned"; else ok "alias-foreign-file: record carries no alias="; fi + if [ -f "$DEST" ]; then ok "alias-foreign-file: the binary was still installed"; else nok "alias-foreign-file: binary missing"; fi + + # (G4) an ABSOLUTE link to the binary (the development installer's spelling) converges silently. + dl_build_sandbox "$PROV_G" || nok "G4: sandbox build failed" + dl_case; dl_mk_release "$RELEASES" "$VER" + ALIAS="$RUN_BIN/dckt" + ln -s "$DEST" "$ALIAS" + dl_run --version "$VER" --harness claude 2>"$ALIAS_ERR"; rc=$? + if [ "$rc" = 0 ] && [ "$(readlink "$ALIAS")" = "$DEST" ]; then ok "alias-absolute: absolute link to docket left as is"; else nok "alias-absolute: exit $rc, link now $(readlink "$ALIAS" 2>/dev/null)"; fi + if grep -qF warning "$ALIAS_ERR"; then nok "alias-absolute: an owned absolute alias was warned about"; else ok "alias-absolute: no warning"; fi + if grep -qxF "alias=$ALIAS" "$RECORD"; then ok "alias-absolute: record names the alias"; else nok "alias-absolute: record lacks alias="; fi + + # (G5) a symlink elsewhere, and a dangling one: both foreign, both untouched. + for _kind in elsewhere dangling; do + dl_build_sandbox "$PROV_G" || nok "G5: sandbox build failed" + dl_case; dl_mk_release "$RELEASES" "$VER" + ALIAS="$RUN_BIN/dckt" + case $_kind in elsewhere) _tgt="$SANDBOX/bin/cat" ;; dangling) _tgt="$RUN_BIN/gone" ;; esac + ln -s "$_tgt" "$ALIAS" + dl_run --version "$VER" --harness claude 2>"$ALIAS_ERR"; rc=$? + if [ "$rc" = 0 ] && [ "$(readlink "$ALIAS")" = "$_tgt" ]; then ok "alias-$_kind: install exits 0 and the link is untouched"; else nok "alias-$_kind: exit $rc, link now $(readlink "$ALIAS" 2>/dev/null)"; fi + if grep -qF warning "$ALIAS_ERR"; then ok "alias-$_kind: warned"; else nok "alias-$_kind: no warning"; fi + done +else + ok "no hash provider on host — alias cases skipped" +fi + # --- (F) mutation pass: the tripwire assert is non-vacuous, and verification precedes install ---- # (F1) Directly invoke a banned-tool tripwire once. TRIP_LOG must become non-empty and the tool must # exit 9 — so every "tripwire log empty" assert above is a real guard, able to redden, not decoration. diff --git a/tests/test_release_downloader_converge.sh b/tests/test_release_downloader_converge.sh index dadc4ed29..08a39643f 100755 --- a/tests/test_release_downloader_converge.sh +++ b/tests/test_release_downloader_converge.sh @@ -100,8 +100,9 @@ chmod 755 "$TRIPWIRE_SRC" # it is not symlinked; dirname IS used by the record path and is required). gzip is required for # tar's `-z`: GNU tar (Linux) execs the external gzip binary, so its absence from the sandbox reddens # `tar -tzf`/`tar -xzf`; BSD tar (macOS) links libz internally and needs none, which is why omitting -# it only fails on Linux. The banned set gets a tripwire each. -DL_REAL_TOOLS='curl tar gzip uname mktemp mkdir mv cp chmod rm grep sed cat dirname' +# it only fails on Linux. ln is the dckt alias step's one new tool (ln -s). The banned set gets a +# tripwire each. +DL_REAL_TOOLS='curl tar gzip uname mktemp mkdir mv cp chmod rm grep sed cat dirname ln' DL_BANNED='bash python python3 perl shasum jq' # Build $SANDBOX/bin: real symlinks for the needed tools + a tripwire for each banned tool. $1 diff --git a/tests/test_release_downloader_refusals.sh b/tests/test_release_downloader_refusals.sh index fb76e978d..e10843c75 100644 --- a/tests/test_release_downloader_refusals.sh +++ b/tests/test_release_downloader_refusals.sh @@ -93,8 +93,9 @@ chmod 755 "$TRIPWIRE_SRC" # it is not symlinked; dirname IS used by the record path and is required). gzip is required for # tar's `-z`: GNU tar (Linux) execs the external gzip binary, so its absence from the sandbox reddens # `tar -tzf`/`tar -xzf`; BSD tar (macOS) links libz internally and needs none, which is why omitting -# it only fails on Linux. The banned set gets a tripwire each. -DL_REAL_TOOLS='curl tar gzip uname mktemp mkdir mv cp chmod rm grep sed cat dirname' +# it only fails on Linux. ln is the dckt alias step's one new tool (ln -s). The banned set gets a +# tripwire each. +DL_REAL_TOOLS='curl tar gzip uname mktemp mkdir mv cp chmod rm grep sed cat dirname ln' DL_BANNED='bash python python3 perl shasum jq' # Build $SANDBOX/bin: real symlinks for the needed tools + a tripwire for each banned tool. $1 From 654f0c990a379e7ee97de3c640bcb297437253aa Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 10:17:42 -0400 Subject: [PATCH 07/11] test(release): dckt answers like docket; smoke asserts the alias --- install.sh | 2 ++ internal/bashupgrade/alias_invocation_test.go | 34 +++++++++++++++++++ scripts/release-smoke.md | 4 +-- scripts/release-smoke.sh | 13 +++++++ 4 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 internal/bashupgrade/alias_invocation_test.go diff --git a/install.sh b/install.sh index 0e84a9089..f01d3523f 100755 --- a/install.sh +++ b/install.sh @@ -14,6 +14,8 @@ # # The heavy lifting (building the binary, linking harnesses, adopting legacy artifacts, # the journaled install transaction) lives in the Go engine reached through that command. +# An install leaves the docket binary plus dckt, a symlink to it, in the bin directory; a dckt +# docket did not create is never replaced (the install warns and leaves it alone). # # DOCKET_BOOTSTRAP_DRY_RUN=1 prints the resolved command to stdout and exits 0 without # running it — the test seam this file's own tests key on. diff --git a/internal/bashupgrade/alias_invocation_test.go b/internal/bashupgrade/alias_invocation_test.go new file mode 100644 index 000000000..7d45413b8 --- /dev/null +++ b/internal/bashupgrade/alias_invocation_test.go @@ -0,0 +1,34 @@ +package bashupgrade + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/danielhanold/docket/internal/install" + "github.com/danielhanold/docket/internal/testsupport" +) + +// The alias is only useful if the binary behaves the same whatever name it was +// invoked by. This uses the real built binary (shared with the upgrade tests via +// buildDocket's sync.Once, so it costs no extra build) behind a dckt symlink. +func TestAliasInvocationMatchesDocket(t *testing.T) { + bin := buildDocket(t) + dir := testsupport.TempDir(t) + alias := filepath.Join(dir, install.AliasName) + if err := os.Symlink(bin, alias); err != nil { + t.Fatal(err) + } + run := func(path string) string { + t.Helper() + out, err := exec.Command(path, "version", "--json").Output() + if err != nil { + t.Fatalf("%s version --json: %v", path, err) + } + return string(out) + } + if got, want := run(alias), run(bin); got != want { + t.Fatalf("dckt version --json = %q, docket version --json = %q", got, want) + } +} diff --git a/scripts/release-smoke.md b/scripts/release-smoke.md index 7240363f3..82459700c 100644 --- a/scripts/release-smoke.md +++ b/scripts/release-smoke.md @@ -60,8 +60,8 @@ fresh *session* (its own home + state + bin). The downloader is fed the bundle o | A — host tuple | `uname -s`/`-m` map to one of the four approved tuples; anything else fails here. | | B — verify + extract | The host archive has **exactly one** `checksums.txt` line, its SHA-256 matches, its tar listing is exactly `docket`, and it extracts to one regular file. | | C — identity | The extracted binary's `version --json` reports the exact expected version; `diagnostic runtime --json` reports the host `go_os`/`go_arch` and `supported_target: true`. | -| D — install | The bundle downloader installs across `--harness claude --harness codex --harness cursor --harness opencode`; a binary and an ownership record naming the version land. | -| E — check | `install check --json` is clean (`applied` or `no-op`, exit 0). | +| D — install | The bundle downloader installs across `--harness claude --harness codex --harness cursor --harness opencode`; a binary and an ownership record naming the version land; a `dckt` symlink resolving to the binary lands beside it, the record names it (`alias=`), and `dckt version --json` equals `docket version --json`. | +| E — check | `install check --json` is clean (`applied` or `no-op`, exit 0); it carries no `binary-alias-*` warning. | | F — idempotence | A same-version rerun leaves the dest binary and the record **byte-identical** (hash before == after). | | G — upgrade | *(only with `--base-bundle`)* Install base, plant a **foreign** sentinel under a harness dir, upgrade to head: the binary and record become head's, the upgrade is non-vacuous (base and head bytes differ), and the sentinel is byte-identical after — the downloader replaces only bytes it owns. | | H — convergence | A **doctored copy** of the downloader with the single `mv -f "$stage" "$dest"` line replaced by `exit 97` interrupts after the asset install and before the rename; dest is never created and no stage file is left. A rerun of the **real** downloader then converges to a real installed binary and publishes the record. The doctoring is asserted to change exactly one line, so the case cannot pass vacuously (learning `assert-detects-removal-not-replacement`). | diff --git a/scripts/release-smoke.sh b/scripts/release-smoke.sh index 3d9ec949c..7a9abb869 100755 --- a/scripts/release-smoke.sh +++ b/scripts/release-smoke.sh @@ -195,6 +195,16 @@ fi [ -f "$S_RECORD" ] || die "install: no ownership record after a successful install" grep -qxF "version=$VERSION" "$S_RECORD" \ || die "install: ownership record does not name the installed version: $(tr '\n' '|' < "$S_RECORD")" +S_ALIAS="$S_BIN/dckt" +[ -L "$S_ALIAS" ] && [ "$S_ALIAS" -ef "$S_DEST" ] \ + || die "install: no dckt alias resolving to the installed binary at $S_ALIAS" +grep -qxF "alias=$S_ALIAS" "$S_RECORD" \ + || die "install: ownership record does not name the dckt alias: $(tr '\n' '|' < "$S_RECORD")" +alias_ver=$(env HOME="$S_HOME" XDG_STATE_HOME="$S_STATE" XDG_BIN_HOME="$S_BIN" \ + XDG_DATA_HOME="$S_DATA" XDG_CONFIG_HOME="$S_CONFIG" TMPDIR="$S_TMP" "$S_ALIAS" version --json) \ + || die "install: '$S_ALIAS version --json' exited non-zero" +[ "$alias_ver" = "$(run_docket version --json)" ] \ + || die "install: dckt version --json differs from docket version --json" # --- Block E: read-only install check must be clean ---------------------------------------------- step "block E: install check --json" @@ -205,6 +215,9 @@ case $check_result in applied|no-op) ;; *) die "check: install check result '$check_result' is not clean (expected applied or no-op)" ;; esac +case $check_json in + *'"binary-alias-'*) die "check: install check reports a dckt alias warning after a clean install" ;; +esac # --- Block F: same-version rerun is idempotent --------------------------------------------------- step "block F: same-version rerun idempotence" From b6940b93738c022d14cb7df634903876e52684fa Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 10:25:36 -0400 Subject: [PATCH 08/11] test(bashupgrade): the guide's install mirror places the dckt alias like install.sh installHandoff mirrors internal/release/downloader/install.sh's fresh-install path but predated its alias step, so install check correctly reported the missing dckt and confirm-install's no-warning promise failed. Mirror the downloader: a relative dckt -> docket symlink beside the binary and alias= in the ownership record. --- internal/bashupgrade/registry_test.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/internal/bashupgrade/registry_test.go b/internal/bashupgrade/registry_test.go index 70481dcf2..44f31b1cc 100644 --- a/internal/bashupgrade/registry_test.go +++ b/internal/bashupgrade/registry_test.go @@ -426,13 +426,17 @@ func mirrorInstallBinary(t *testing.T, c *upgradeCase, st *runState, body string // installHandoff mirrors internal/release/downloader/install.sh from the point it // has a verified binary: stage it beside the destination, run the staged binary's -// `install --harness claude`, and only on success move it into place, write the -// ownership record, and run `docket install check`. +// `install --harness claude`, and only on success move it into place, place the +// relative dckt alias beside it, write the ownership record (naming the alias), and +// run `docket install check`. func installHandoff(t *testing.T, c *upgradeCase, st *runState) { t.Helper() dest := filepath.Join(c.BinDir, "docket") - if _, err := os.Lstat(dest); err == nil { - t.Fatalf("%s already exists; the mirror covers only the downloader's fresh-install path", dest) + alias := filepath.Join(c.BinDir, "dckt") + for _, p := range []string{dest, alias} { + if _, err := os.Lstat(p); err == nil { + t.Fatalf("%s already exists; the mirror covers only the downloader's fresh-install path", p) + } } if err := os.MkdirAll(c.BinDir, 0o755); err != nil { t.Fatal(err) @@ -506,12 +510,16 @@ func installHandoff(t *testing.T, c *upgradeCase, st *runState) { if err := os.Rename(stage, dest); err != nil { t.Fatal(err) } + // The downloader's fresh-install alias step: `ln -s docket "$bin_dir/dckt"`. + if err := os.Symlink("docket", alias); err != nil { + t.Fatal(err) + } sum := sha256.Sum256(bin) recDir := filepath.Join(c.Home, ".local", "state", "docket") if err := os.MkdirAll(recDir, 0o755); err != nil { t.Fatal(err) } - rec := "path=" + dest + "\nversion=development\nsha256=" + hex.EncodeToString(sum[:]) + "\n" + rec := "path=" + dest + "\nversion=development\nalias=" + alias + "\nsha256=" + hex.EncodeToString(sum[:]) + "\n" if err := os.WriteFile(filepath.Join(recDir, "release-binary.record"), []byte(rec), 0o644); err != nil { t.Fatal(err) } From 07d1b63a956c678c71a8d43ffeff5679fe2c93ea Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 10:36:19 -0400 Subject: [PATCH 09/11] fix(install): an unresolvable dckt is a foreign alias, never a failed install Review blocker: a looping dckt, a link through a regular file or into an unreadable directory, or an unreadable regular-file dckt made InspectBinaryAlias and planBinaryAlias return the resolution error, failing development install (filesystem-failed) and install check. Once the alias path is known to exist, any failure resolving or reading it is now an AliasForeign finding; only the Lstat error and failures canonicalising the binary path remain errors. --- internal/install/alias.go | 34 ++++++++++++++++++------ internal/install/alias_test.go | 20 ++++++++++++++ internal/install/devmode_test.go | 45 ++++++++++++++++++++++++++++++++ internal/install/service_test.go | 13 +++++++++ 4 files changed, 104 insertions(+), 8 deletions(-) diff --git a/internal/install/alias.go b/internal/install/alias.go index ee62483c8..dfc2869a5 100644 --- a/internal/install/alias.go +++ b/internal/install/alias.go @@ -50,8 +50,10 @@ func AliasPathFor(binary string) string { // InspectBinaryAlias classifies the alias beside binary. A nil finding means a // symlink that resolves — every hop canonicalised — to binary. Absence is // AliasMissing; anything else at the path (a link elsewhere, a dangling link, -// a file, a directory) is AliasForeign. It only reads. An error means the -// probe itself failed, which is never reported as a clean absence. +// a file, a directory, a link that cannot even be resolved) is AliasForeign. It +// only reads. An error means the probe itself failed — the alias path cannot be +// examined, or the binary's own path cannot be canonicalised — which is never +// reported as a clean absence. func InspectBinaryAlias(binary string) (*AliasFinding, error) { path := AliasPathFor(binary) info, err := os.Lstat(path) @@ -62,15 +64,14 @@ func InspectBinaryAlias(binary string) (*AliasFinding, error) { return nil, fmt.Errorf("install: inspecting %s: %w", path, err) } if info.Mode()&fs.ModeSymlink != 0 { - have, err := linkDestination(path) - if err != nil { - return nil, err - } want, err := canonicalPath(binary) if err != nil { return nil, err } - if have == want { + // Something is at the path; a link docket cannot resolve (a loop, a + // hop through a file, an unreadable directory) is not provably + // docket's, so it is foreign rather than a failed probe. + if have, err := linkDestination(path); err == nil && have == want { if _, err := os.Stat(path); err == nil { return nil, nil } @@ -87,14 +88,31 @@ func InspectBinaryAlias(binary string) (*AliasFinding, error) { // recorded and that still matches its record. A conflict is reported rather // than returned as a refusal: the binary install must never be failed by the // alias. +// +// Only a failure to examine the alias path, or to canonicalise the binary's +// own path, is an error. Once something is known to be at the path, any +// failure resolving or reading it (a looping link, a hop through a file, an +// unreadable directory or file) is a foreign finding: what docket cannot +// resolve it cannot prove it owns. func planBinaryAlias(binary string, prior *State) (*Target, *AliasFinding, error) { t := Target{Path: AliasPathFor(binary), Kind: KindSymlink, LinkTarget: binary, Role: roleBinaryAlias} + foreign := &AliasFinding{Kind: AliasForeign, Path: t.Path, Binary: binary, Remedy: remedyAliasForeign} + _, lerr := os.Lstat(t.Path) + if lerr != nil && !errors.Is(lerr, fs.ErrNotExist) { + return nil, nil, fmt.Errorf("install: inspecting %s: %w", t.Path, lerr) + } + if _, err := canonicalPath(binary); err != nil { + return nil, nil, err + } inspection, err := InspectTarget(t, prior, nil) if err != nil { + if lerr == nil { + return nil, foreign, nil + } return nil, nil, err } if inspection.Disposition == DispositionConflict { - return nil, &AliasFinding{Kind: AliasForeign, Path: t.Path, Binary: binary, Remedy: remedyAliasForeign}, nil + return nil, foreign, nil } return &t, nil, nil } diff --git a/internal/install/alias_test.go b/internal/install/alias_test.go index 0620292ff..87edd9c97 100644 --- a/internal/install/alias_test.go +++ b/internal/install/alias_test.go @@ -115,6 +115,26 @@ func TestInspectBinaryAlias(t *testing.T) { t.Fatalf("finding = %+v, err %v; want foreign", f, err) } }) + // An alias that cannot even be resolved is still somebody else's entry: + // a resolution failure is a foreign finding, never a failed probe. + t.Run("self-looping link is foreign", func(t *testing.T) { + bin, binary := aliasBin(t) + if err := os.Symlink(AliasName, filepath.Join(bin, AliasName)); err != nil { + t.Fatal(err) + } + if f, err := InspectBinaryAlias(binary); err != nil || f == nil || f.Kind != AliasForeign { + t.Fatalf("finding = %+v, err %v; want foreign", f, err) + } + }) + t.Run("link through a regular file is foreign", func(t *testing.T) { + bin, binary := aliasBin(t) + if err := os.Symlink(filepath.Join(binary, "x"), filepath.Join(bin, AliasName)); err != nil { + t.Fatal(err) + } + if f, err := InspectBinaryAlias(binary); err != nil || f == nil || f.Kind != AliasForeign { + t.Fatalf("finding = %+v, err %v; want foreign", f, err) + } + }) t.Run("probe error is an error, not missing", func(t *testing.T) { if runtime.GOOS == "windows" || os.Geteuid() == 0 { t.Skip("permission bits do not deny root") diff --git a/internal/install/devmode_test.go b/internal/install/devmode_test.go index 3019172f2..ad02014e8 100644 --- a/internal/install/devmode_test.go +++ b/internal/install/devmode_test.go @@ -4,6 +4,7 @@ import ( "errors" "os" "path/filepath" + "runtime" "strings" "testing" @@ -939,6 +940,18 @@ func TestDevInstallPreservesAForeignAlias(t *testing.T) { {"directory", func(t *testing.T, alias string) { writeFile(t, filepath.Join(alias, "keep"), "mine\n") }}, + {"self-looping link", func(t *testing.T, alias string) { + if err := os.Symlink(install.AliasName, alias); err != nil { + t.Fatal(err) + } + }}, + {"link through a regular file", func(t *testing.T, alias string) { + plain := filepath.Join(filepath.Dir(filepath.Dir(alias)), "plain") + writeFile(t, plain, "mine\n") + if err := os.Symlink(filepath.Join(plain, "x"), alias); err != nil { + t.Fatal(err) + } + }}, } { t.Run(tc.name, func(t *testing.T) { w := newWorld(t) @@ -969,6 +982,38 @@ func TestDevInstallPreservesAForeignAlias(t *testing.T) { }) } + // A regular-file dckt the install cannot even read is still foreign: the + // unreadable bytes are no reason to fail the binary install. + t.Run("unreadable file", func(t *testing.T) { + if runtime.GOOS == "windows" || os.Geteuid() == 0 { + t.Skip("permission bits do not deny root") + } + w := newWorld(t) + mkdirAll(t, w.path(".toy")) + bin := filepath.Join(w.home, "bin") + mkdirAll(t, bin) + alias := filepath.Join(bin, install.AliasName) + writeFile(t, alias, "#!/bin/sh\necho mine\n") + if err := os.Chmod(alias, 0o755); err != nil { + t.Fatal(err) + } + aliasBefore := snapshot(t, alias) + if err := os.Chmod(alias, 0o000); err != nil { + t.Fatal(err) + } + out := install.DevelopmentInstall(w.devCandidate(t, newSource(t), bin)) + if err := os.Chmod(alias, 0o755); err != nil { + t.Fatal(err) + } + if out.Err != nil || !out.Applied { + t.Fatalf("an unreadable alias failed the install: %+v", out) + } + if len(out.AliasFindings) != 1 || out.AliasFindings[0].Kind != install.AliasForeign { + t.Fatalf("alias findings = %+v, want one foreign finding", out.AliasFindings) + } + assertUnchanged(t, aliasBefore, snapshot(t, alias), "unreadable alias") + }) + // An alias docket recorded once and the user has since replaced is no // longer provably docket's: the record is no licence to overwrite it. t.Run("recorded alias replaced by the user", func(t *testing.T) { diff --git a/internal/install/service_test.go b/internal/install/service_test.go index 78bda0955..2e03d1a15 100644 --- a/internal/install/service_test.go +++ b/internal/install/service_test.go @@ -1485,6 +1485,19 @@ func TestCheckReportsTheReleaseAlias(t *testing.T) { if out := install.Check(check); out.Err != nil || len(out.AliasFindings) != 0 { t.Fatalf("a healthy release alias: err %v findings %+v", out.Err, out.AliasFindings) } + + // An alias that cannot be resolved is a foreign finding, never a failed + // check: the downloader's final `install check` must still exit clean. + if err := os.Remove(filepath.Join(bin, install.AliasName)); err != nil { + t.Fatal(err) + } + if err := os.Symlink(install.AliasName, filepath.Join(bin, install.AliasName)); err != nil { + t.Fatal(err) + } + if out := install.Check(check); out.Err != nil || out.Reason != "" || + len(out.AliasFindings) != 1 || out.AliasFindings[0].Kind != install.AliasForeign { + t.Fatalf("a self-looping release alias: reason %q err %v findings %+v", out.Reason, out.Err, out.AliasFindings) + } } func TestCheckDetectsMissingVersionTree(t *testing.T) { From 582a5d02833748bb40a3ac10283fa02b4e335a69 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 10:38:40 -0400 Subject: [PATCH 10/11] fix(install): drop a replaced alias's stale record; a vanished binary is not a foreign alias Two review findings, batched: - A dckt docket recorded earlier and the user later replaced was reported foreign but its roleBinaryAlias record was carried forward forever (records with no harness are always carried). The development install now passes the foreign alias path to desiredState, which drops any prior roleBinaryAlias record for it, so the published state stops claiming a path docket no longer owns. The record never granted ownership; this keeps state truthful. - When the docket binary itself had vanished, check classified docket's own dckt -> docket link as AliasForeign with the "docket did not create what is at this path" remedy, contradicting the binary-drift report. checkBinaryAliases now skips the alias probe for an absent binary; binary drift (or a re-install) covers it. InspectBinaryAlias's primitive classification is unchanged, and an unresolvable alias beside a present binary is still foreign, never an error. --- internal/install/alias.go | 11 +++++++- internal/install/alias_test.go | 34 ++++++++++++++++++++++- internal/install/devmode.go | 21 +++++++++------ internal/install/devmode_test.go | 46 ++++++++++++++++++++++++++++++++ internal/install/service.go | 11 ++++++++ 5 files changed, 113 insertions(+), 10 deletions(-) diff --git a/internal/install/alias.go b/internal/install/alias.go index dfc2869a5..47d60887b 100644 --- a/internal/install/alias.go +++ b/internal/install/alias.go @@ -150,7 +150,8 @@ func readReleaseBinaryPath(recordPath string) (string, error) { // checkBinaryAliases classifies the alias beside every binary an installer // owns: the development installation's recorded binary, and the binary the -// release downloader's record names. It only reads. +// release downloader's record names. A binary that is absent is skipped: its +// alias cannot be judged against nothing. It only reads. func checkBinaryAliases(state *State, roots UserRoots) ([]AliasFinding, error) { var binaries []string seen := map[string]bool{} @@ -179,6 +180,14 @@ func checkBinaryAliases(state *State, roots UserRoots) ([]AliasFinding, error) { var findings []AliasFinding for _, binary := range binaries { + // A binary that has vanished is binary drift (or a re-install) to + // report; docket's own link to it resolving to nothing is no evidence + // that someone else put it there, so the alias is not probed at all. + if _, err := os.Lstat(binary); errors.Is(err, fs.ErrNotExist) { + continue + } else if err != nil { + return nil, fmt.Errorf("install: inspecting %s: %w", binary, err) + } f, err := InspectBinaryAlias(binary) if err != nil { return nil, err diff --git a/internal/install/alias_test.go b/internal/install/alias_test.go index 87edd9c97..2cee78388 100644 --- a/internal/install/alias_test.go +++ b/internal/install/alias_test.go @@ -149,7 +149,10 @@ func TestInspectBinaryAlias(t *testing.T) { t.Fatalf("finding = %+v with nil error; an unreadable bin dir must not read as a clean absence", f) } }) - t.Run("link to a vanished binary is foreign", func(t *testing.T) { + // The primitive cannot prove a link to nothing is docket's; it is + // checkBinaryAliases that keeps this from surfacing as a foreign finding + // (see TestCheckBinaryAliasesSkipsAVanishedBinary). + t.Run("link to a vanished binary is foreign to the primitive", func(t *testing.T) { bin, binary := aliasBin(t) if err := os.Symlink("docket", filepath.Join(bin, AliasName)); err != nil { t.Fatal(err) @@ -163,6 +166,35 @@ func TestInspectBinaryAlias(t *testing.T) { }) } +// A binary that has vanished is binary drift (or a re-install) to report, not +// an alias to classify: docket's own link to it must not read as foreign — +// whether the binary was the development record's or the release downloader's. +func TestCheckBinaryAliasesSkipsAVanishedBinary(t *testing.T) { + bin, binary := aliasBin(t) + if err := os.Symlink("docket", filepath.Join(bin, AliasName)); err != nil { + t.Fatal(err) + } + if err := os.Remove(binary); err != nil { + t.Fatal(err) + } + state := &State{Targets: []TargetRecord{{Path: binary, Kind: KindFile, Role: roleBinary}}} + if got, err := checkBinaryAliases(state, UserRoots{StateHome: testsupport.TempDir(t)}); err != nil || len(got) != 0 { + t.Fatalf("recorded binary: findings %+v, err %v; want none", got, err) + } + + stateHome := testsupport.TempDir(t) + rec := (UserRoots{StateHome: stateHome}).ReleaseBinaryRecordPath() + if err := os.MkdirAll(filepath.Dir(rec), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(rec, []byte("path="+binary+"\n"), 0o644); err != nil { + t.Fatal(err) + } + if got, err := checkBinaryAliases(nil, UserRoots{StateHome: stateHome}); err != nil || len(got) != 0 { + t.Fatalf("release binary: findings %+v, err %v; want none", got, err) + } +} + func TestReadReleaseBinaryPath(t *testing.T) { dir := testsupport.TempDir(t) rec := filepath.Join(dir, "release-binary.record") diff --git a/internal/install/devmode.go b/internal/install/devmode.go index 0de968ba4..5f57fb684 100644 --- a/internal/install/devmode.go +++ b/internal/install/devmode.go @@ -296,19 +296,24 @@ func developmentInstallCandidate(o DevOptions) Outcome { if aliasTarget != nil { targets = append(targets, *aliasTarget) } + var foreignAliases []string + if aliasFinding != nil && aliasFinding.Kind == AliasForeign { + foreignAliases = append(foreignAliases, aliasFinding.Path) + } // The candidate re-resolved the repository phase itself (the parent passed // --repo-dir through verbatim), so machine and repository writes ride one // transaction here exactly as a release install does. out = applyPlan(o.Options, plannedInstallation{ - mode: ModeDevelopment, - harnesses: out.Harnesses, - targets: targets, - owner: owner, - assetSetID: ds.digest, - assetProtocol: ds.manifest.AssetProtocol, - sourceRoot: ds.source, - sourceDigest: ds.digest, + mode: ModeDevelopment, + harnesses: out.Harnesses, + targets: targets, + owner: owner, + assetSetID: ds.digest, + assetProtocol: ds.manifest.AssetProtocol, + sourceRoot: ds.source, + sourceDigest: ds.digest, + foreignAliases: foreignAliases, }, o.RepoPhase, out) if aliasFinding != nil { out.AliasFindings = append(out.AliasFindings, *aliasFinding) diff --git a/internal/install/devmode_test.go b/internal/install/devmode_test.go index ad02014e8..381ea5251 100644 --- a/internal/install/devmode_test.go +++ b/internal/install/devmode_test.go @@ -1039,9 +1039,55 @@ func TestDevInstallPreservesAForeignAlias(t *testing.T) { t.Fatalf("alias findings = %+v, want one foreign finding", out.AliasFindings) } assertUnchanged(t, aliasBefore, snapshot(t, alias), "replaced alias") + // The old record grants nothing any more, so the state this run + // publishes must not keep claiming the path as docket's alias. + for _, rec := range loadState(t, w.roots).Targets { + if rec.Path == alias { + t.Errorf("the replaced alias's stale record was carried forward: %+v", rec) + } + } }) } +// When the installed binary itself is gone, docket's own alias beside it +// resolves to nothing — but it is still docket's. Check reports the missing +// binary as drift and says nothing about the alias, rather than calling it +// something docket did not create. +func TestCheckVanishedBinaryIsDriftNotAForeignAlias(t *testing.T) { + w := newWorld(t) + mkdirAll(t, w.path(".toy")) + bin := filepath.Join(w.home, "bin") + dev := w.devCandidate(t, newSource(t), bin) + if out := install.DevelopmentInstall(dev); out.Err != nil { + t.Fatalf("DevelopmentInstall: %v (reason %q)", out.Err, out.Reason) + } + binary := filepath.Join(bin, "docket") + alias := filepath.Join(bin, install.AliasName) + // The downloader's relative spelling: docket's own link either way. + if err := os.Remove(alias); err != nil { + t.Fatal(err) + } + if err := os.Symlink("docket", alias); err != nil { + t.Fatal(err) + } + if err := os.Remove(binary); err != nil { + t.Fatal(err) + } + + check := dev.Options + check.FS = panicFS{} + out := install.Check(check) + if out.Err != nil && out.Reason == "" { + t.Fatalf("check failed outright: %v", out.Err) + } + if _, ok := findAction(out, install.OpDrift, binary); !ok { + t.Errorf("the vanished binary was not reported as drift: %v", out.Actions) + } + if len(out.AliasFindings) != 0 { + t.Errorf("alias findings = %+v; a link to a vanished binary is binary drift, not a foreign alias", out.AliasFindings) + } +} + // A failed transaction rolls back an alias it created, and leaves a // pre-existing one exactly as it was. func TestDevInstallRollbackAndTheAlias(t *testing.T) { diff --git a/internal/install/service.go b/internal/install/service.go index 9c3ced956..4592ecccb 100644 --- a/internal/install/service.go +++ b/internal/install/service.go @@ -514,6 +514,10 @@ type plannedInstallation struct { assetProtocol int sourceRoot string sourceDigest string + // foreignAliases are alias paths this run found occupied by something + // docket cannot prove it owns. A prior roleBinaryAlias record for one of + // them grants nothing, so it is not carried into the published state. + foreignAliases []string } // applyPlan is the shared tail of Install and DevelopmentInstall: classify, @@ -824,8 +828,15 @@ func desiredState(o Options, p plannedInstallation, prior *State) (*State, error rec.Path = path records = append(records, rec) } + foreignAlias := make(map[string]bool, len(p.foreignAliases)) + for _, path := range p.foreignAliases { + foreignAlias[filepath.Clean(path)] = true + } if prior != nil { for _, rec := range prior.Targets { + if rec.Role == roleBinaryAlias && foreignAlias[filepath.Clean(rec.Path)] { + continue // replaced by the user: no longer docket's to record. + } if !selected[rec.Harness] { carry(rec) } From 1b77b8390146ffad6503a869bf0b475fdeda8b18 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 10:39:33 -0400 Subject: [PATCH 11/11] docs(release): the downloader's alias= record line is informational only --- internal/release/downloader/install.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/release/downloader/install.sh b/internal/release/downloader/install.sh index 75b3eca09..b3681d582 100755 --- a/internal/release/downloader/install.sh +++ b/internal/release/downloader/install.sh @@ -16,7 +16,8 @@ # beyond the list above). An absent dckt is created; a dckt already resolving to the binary is # left as is; anything else is left UNTOUCHED with a warning, and the install still succeeds — # the binary install is never failed by the alias. The record gains alias= only when the -# alias is in place and ours. +# alias is in place and ours, but that line is informational only: nothing reads it. Alias +# ownership is decided by the link resolving to the installed binary, never by the record. # - Exit codes: 0 success; 2 usage error (bad flag / non-absolute bin dir / bad version); 1 # every other failure. Every failure prints a one-line actionable diagnostic to stderr. set -u @@ -303,6 +304,7 @@ record_tmp="$record.tmp.$$" { printf 'path=%s\n' "$dest" printf 'version=%s\n' "$version" + # alias= is informational only; ownership is decided by the link resolving to the binary. if [ "$alias_owned" = yes ]; then printf 'alias=%s\n' "$alias_path"; fi printf 'sha256=%s\n' "$bin_sha" } > "$record_tmp" || { rm -f "$record_tmp"; die "cannot write the ownership record"; }