diff --git a/internal/state/aliases.go b/internal/state/aliases.go index f3b94c2..0719f2b 100644 --- a/internal/state/aliases.go +++ b/internal/state/aliases.go @@ -67,7 +67,7 @@ func (store *Store) UpsertAliasBaseline(root, home string, baseline AliasBaselin if err != nil { return AliasBaseline{}, err } - now := formatTimestamp(store.clock.Now()) + now := formatTimestamp(store.now()) transaction, err := store.database.conn.Begin() if err != nil { return AliasBaseline{}, err diff --git a/internal/state/aliases_read.go b/internal/state/aliases_read.go index 6d83513..a2b9437 100644 --- a/internal/state/aliases_read.go +++ b/internal/state/aliases_read.go @@ -54,7 +54,7 @@ func (store *Store) setAliasStatus(key aliasBaselineKey, statement string) (Alia if !IsSlashRelative(key.alias) { return AliasBaseline{}, fmt.Errorf("state: alias path %q is not a slash-relative path", key.alias) } - now := formatTimestamp(store.clock.Now()) + now := formatTimestamp(store.now()) transaction, err := store.database.conn.Begin() if err != nil { return AliasBaseline{}, err diff --git a/internal/state/aliases_test.go b/internal/state/aliases_test.go index c6a4ead..656d29d 100644 --- a/internal/state/aliases_test.go +++ b/internal/state/aliases_test.go @@ -53,7 +53,7 @@ func seedAlias(t *testing.T, store *Store, spec aliasSpec) { func testAliasUpsertCreatesRow(t *testing.T) { clock := &pinnedClock{now: time.Date(2026, 3, 4, 5, 6, 7, 0, time.UTC)} - store := openStore(t, Dependencies{StateHome: t.TempDir(), Clock: clock}) + store := openStore(t, Dependencies{StateHome: t.TempDir(), Now: clock.Now}) root := t.TempDir() home := t.TempDir() baseline := aliasBaseline(".config/app", "canonical/.config/app", "conf") diff --git a/internal/state/files.go b/internal/state/files.go index 78f7c8b..0b95837 100644 --- a/internal/state/files.go +++ b/internal/state/files.go @@ -92,7 +92,7 @@ func (store *Store) UpsertFileBaseline(root, home string, baseline FileBaseline) if err != nil { return FileBaseline{}, err } - now := formatTimestamp(store.clock.Now()) + now := formatTimestamp(store.now()) transaction, err := store.database.conn.Begin() if err != nil { return FileBaseline{}, err @@ -128,7 +128,7 @@ func (store *Store) setFileStatus(key fileBaselineKey, statement string) (FileBa if !IsSlashRelative(key.target) { return FileBaseline{}, fmt.Errorf("state: file target %q is not a slash-relative path", key.target) } - now := formatTimestamp(store.clock.Now()) + now := formatTimestamp(store.now()) transaction, err := store.database.conn.Begin() if err != nil { return FileBaseline{}, err diff --git a/internal/state/files_test.go b/internal/state/files_test.go index cf6aab1..724b804 100644 --- a/internal/state/files_test.go +++ b/internal/state/files_test.go @@ -77,7 +77,7 @@ func seedOrdinary(t *testing.T, store *Store, spec seedSpec) { func testFileUpsertCreatesRow(t *testing.T) { clock := &pinnedClock{now: time.Date(2026, 3, 4, 5, 6, 7, 0, time.UTC)} - store := openStore(t, Dependencies{StateHome: t.TempDir(), Clock: clock}) + store := openStore(t, Dependencies{StateHome: t.TempDir(), Now: clock.Now}) root := t.TempDir() home := t.TempDir() baseline := ordinaryBaseline(".config/app/config", "", 0x11) diff --git a/internal/state/lock.go b/internal/state/lock.go index 7eba854..c10069b 100644 --- a/internal/state/lock.go +++ b/internal/state/lock.go @@ -34,6 +34,10 @@ func (lock *Lock) Path() string { // holds it. After acquisition it writes the current PID to the lock file for // diagnostics. func (lock *Lock) Acquire() error { + return lock.acquire(writeProcessID) +} + +func (lock *Lock) acquire(writePID func(string) error) error { if err := preparePrivateFile(lock.path); err != nil { return err } @@ -45,7 +49,11 @@ func (lock *Lock) Acquire() error { if !locked { return errLockHeld(lock.path) } - return writeProcessID(lock.path) + if err := writePID(lock.path); err != nil { + _ = lock.Release() + return err + } + return nil } // Release releases the advisory lock. It is idempotent: calling Release on a diff --git a/internal/state/lock_test.go b/internal/state/lock_test.go index 5b1cdae..b5b846a 100644 --- a/internal/state/lock_test.go +++ b/internal/state/lock_test.go @@ -1,6 +1,7 @@ package state import ( + "errors" "os" "path/filepath" "strconv" @@ -19,12 +20,32 @@ func TestStateLock(t *testing.T) { {"rejects non-regular lock file", testRejectsNonRegularLock}, {"rejects wrong lock mode", testRejectsWrongLockMode}, {"release is idempotent", testReleaseIsIdempotent}, + {"PID diagnostic failure releases lock", testPIDDiagnosticFailureReleasesLock}, } for _, scenario := range scenarios { t.Run(scenario.name, scenario.run) } } +func testPIDDiagnosticFailureReleasesLock(t *testing.T) { + path := tempLockPath(t) + lock := NewLock(path) + want := errors.New("diagnostic write failed") + if err := lock.acquire(func(string) error { return want }); !errors.Is(err, want) { + t.Fatalf("Acquire error = %v, want %v", err, want) + } + if err := lock.Release(); err != nil { + t.Fatalf("Release after failed Acquire: %v", err) + } + probe := NewLock(path) + if err := probe.Acquire(); err != nil { + t.Fatalf("lock remained held after failed Acquire: %v", err) + } + if err := probe.Release(); err != nil { + t.Fatalf("probe Release: %v", err) + } +} + func testLockAcquireRelease(t *testing.T) { lock := NewLock(tempLockPath(t)) if err := lock.Acquire(); err != nil { diff --git a/internal/state/repositories.go b/internal/state/repositories.go index 5023817..8ae758c 100644 --- a/internal/state/repositories.go +++ b/internal/state/repositories.go @@ -55,7 +55,7 @@ func (store *Store) RegisterRepository(root, home string) (Repository, error) { if err != nil { return Repository{}, err } - now := formatTimestamp(store.clock.Now()) + now := formatTimestamp(store.now()) transaction, err := store.database.conn.Begin() if err != nil { return Repository{}, err @@ -77,7 +77,7 @@ func (store *Store) SetDefaultRepository(root, home string) (Repository, error) if err != nil { return Repository{}, err } - now := formatTimestamp(store.clock.Now()) + now := formatTimestamp(store.now()) transaction, err := store.database.conn.Begin() if err != nil { return Repository{}, err diff --git a/internal/state/repositories_test.go b/internal/state/repositories_test.go index 403dd09..c483488 100644 --- a/internal/state/repositories_test.go +++ b/internal/state/repositories_test.go @@ -30,7 +30,7 @@ func TestRepositoryRows(t *testing.T) { func testRepositoryRegisters(t *testing.T) { clock := &pinnedClock{now: time.Date(2026, 3, 4, 5, 6, 7, 0, time.UTC)} - store := openStore(t, Dependencies{StateHome: t.TempDir(), Clock: clock}) + store := openStore(t, Dependencies{StateHome: t.TempDir(), Now: clock.Now}) root := t.TempDir() home := t.TempDir() first, err := store.RegisterRepository(root, home) @@ -48,7 +48,7 @@ func testRepositoryRegisters(t *testing.T) { func testRepositoryReRegistration(t *testing.T) { clock := &pinnedClock{now: time.Date(2026, 3, 4, 5, 6, 7, 0, time.UTC)} - store := openStore(t, Dependencies{StateHome: t.TempDir(), Clock: clock}) + store := openStore(t, Dependencies{StateHome: t.TempDir(), Now: clock.Now}) root := t.TempDir() home := t.TempDir() first, err := store.RegisterRepository(root, home) diff --git a/internal/state/store.go b/internal/state/store.go index 73a289f..d9d0fd6 100644 --- a/internal/state/store.go +++ b/internal/state/store.go @@ -3,15 +3,16 @@ package state import ( "context" "path/filepath" + "time" ) // Dependencies bundles the injectable seams of a Store. StateHome, when set, // overrides the XDG-derived state directory; when empty, resolution reads -// XDG_STATE_HOME and rejects a relative value. Clock supplies timestamps and -// defaults to SystemClock when nil. +// XDG_STATE_HOME and rejects a relative value. Now supplies timestamps and +// defaults to the wall clock when nil. type Dependencies struct { StateHome string - Clock Clock + Now func() time.Time } // Store coordinates the state lifecycle: path resolution, advisory locking, @@ -20,7 +21,7 @@ type Dependencies struct { // reverses it. type Store struct { stateHome string - clock Clock + now func() time.Time lock *Lock database *Database } @@ -29,16 +30,11 @@ type Store struct { // no SQLite connection, acquires no lock, creates no path, and inspects no // repository: every effect begins inside Acquire. func NewStore(deps Dependencies) *Store { - clock := deps.Clock - if clock == nil { - clock = SystemClock{} + now := deps.Now + if now == nil { + now = time.Now } - return &Store{stateHome: deps.StateHome, clock: clock} -} - -// Clock returns the clock the store records timestamps with. It is never nil. -func (store *Store) Clock() Clock { - return store.clock + return &Store{stateHome: deps.StateHome, now: now} } // Database returns the opened database handle, or nil before Acquire succeeds. diff --git a/internal/state/store_test.go b/internal/state/store_test.go index 53e07d4..dbd7d93 100644 --- a/internal/state/store_test.go +++ b/internal/state/store_test.go @@ -33,9 +33,6 @@ func testStoreConstructionClean(t *testing.T) { if store.Database() != nil { t.Fatal("Database non-nil before Acquire") } - if store.Clock() == nil { - t.Fatal("Clock nil") - } if _, err := os.Stat(catteryDirFor(t, deps)); !os.IsNotExist(err) { t.Fatalf("NewStore created the cattery directory before Acquire: %v", err) } @@ -143,7 +140,7 @@ func testStoreMigrateFailure(t *testing.T) { func tempDependencies(t *testing.T) Dependencies { t.Helper() - return Dependencies{StateHome: t.TempDir(), Clock: SystemClock{}} + return Dependencies{StateHome: t.TempDir()} } func catteryDirFor(t *testing.T, deps Dependencies) string { diff --git a/internal/state/transitions.go b/internal/state/transitions.go index 534f873..564647d 100644 --- a/internal/state/transitions.go +++ b/internal/state/transitions.go @@ -79,7 +79,7 @@ func (store *Store) transitionToAlias(transaction *sql.Tx, transition aliasTrans } func (store *Store) applyAliasTransition(transaction *sql.Tx, transition aliasTransition) error { - now := formatTimestamp(store.clock.Now()) + now := formatTimestamp(store.now()) if err := execIn(transaction, aliasUpsertByRepositorySQL, transition.repositoryID, transition.baseline.AliasPath, transition.baseline.CanonicalTargetPath, transition.baseline.GroupName, string(transition.baseline.Layer), now); err != nil { @@ -125,7 +125,7 @@ func (store *Store) transitionToFile(transaction *sql.Tx, transition fileTransit } func (store *Store) applyFileTransition(transaction *sql.Tx, transition fileTransition) error { - now := formatTimestamp(store.clock.Now()) + now := formatTimestamp(store.now()) if err := execIn(transaction, fileUpsertByRepositorySQL, transition.repositoryID, transition.baseline.TargetPath, transition.baseline.GroupName, transition.baseline.SourcePath, string(transition.baseline.SourceKind), string(transition.baseline.Layer), diff --git a/internal/state/types.go b/internal/state/types.go index ce7390d..96ca207 100644 --- a/internal/state/types.go +++ b/internal/state/types.go @@ -2,7 +2,7 @@ // successfully deployed representation of every managed target. This file holds // the data-transfer objects and enum validators only: no SQL, XDG, or file-lock // type crosses the package boundary here. Provider and store seams live in -// later files; the sole interface declared here is the narrow Clock. +// later files. package state import ( @@ -119,20 +119,6 @@ type AliasBaseline struct { RetiredAt *time.Time } -// Clock reports the current time so callers can inject a deterministic clock. -// This is the only interface declared in this package and stays at one method -// so it remains a narrow value seam, not a provider contract. -type Clock interface { - Now() time.Time -} - -// SystemClock reads the wall clock. It is the default Clock injected when a -// caller does not supply one. -type SystemClock struct{} - -// Now returns the current local time. -func (SystemClock) Now() time.Time { return time.Now() } - // IsSlashRelative reports whether path is a non-empty relative path expressed // with forward slashes only: it must not be absolute and must not contain a // backslash. State rows store target and source paths in this form so the diff --git a/internal/state/types_test.go b/internal/state/types_test.go index 0abcd7a..6130aaf 100644 --- a/internal/state/types_test.go +++ b/internal/state/types_test.go @@ -1,7 +1,6 @@ package state import ( - "reflect" "testing" "time" @@ -17,7 +16,6 @@ func TestStateContract(t *testing.T) { {"digest width", testDigestWidth}, {"defensive copies", testDefensiveCopy}, {"slash-relative path forms", testPathForms}, - {"clock is the only narrow seam", testClockSeam}, } for _, scenario := range scenarios { t.Run(scenario.name, scenario.run) @@ -93,17 +91,3 @@ func testPathForms(t *testing.T) { } } } - -func testClockSeam(t *testing.T) { - var clock Clock = SystemClock{} - if clock.Now().IsZero() { - t.Fatal("SystemClock.Now returned the zero time") - } - clockType := reflect.TypeOf((*Clock)(nil)).Elem() - if clockType.NumMethod() != 1 { - t.Fatalf("Clock declares %d methods, want 1", clockType.NumMethod()) - } - if clockType.Method(0).Name != "Now" { - t.Fatalf("Clock method = %s, want Now", clockType.Method(0).Name) - } -} diff --git a/internal/subprocess/run.go b/internal/subprocess/run.go index 0973261..121060d 100644 --- a/internal/subprocess/run.go +++ b/internal/subprocess/run.go @@ -12,6 +12,7 @@ import ( "io" "os" "os/exec" + "syscall" "time" ) @@ -148,5 +149,5 @@ func launchError(err error) *LaunchError { func isPathENOENT(err error) bool { var pathErr *os.PathError - return errors.As(err, &pathErr) + return errors.As(err, &pathErr) && errors.Is(pathErr.Err, syscall.ENOENT) } diff --git a/internal/subprocess/run_test.go b/internal/subprocess/run_test.go index 04aee15..5f20134 100644 --- a/internal/subprocess/run_test.go +++ b/internal/subprocess/run_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "strings" + "syscall" "testing" ) @@ -18,12 +19,24 @@ func TestProcessRun(t *testing.T) { {"normal", testNormalRun}, {"nonzero", testNonzeroRun}, {"missing", testMissingExecutable}, + {"non-missing path error", testNonMissingPathError}, } for _, scenario := range scenarios { t.Run(scenario.name, scenario.run) } } +func testNonMissingPathError(t *testing.T) { + cause := &os.PathError{Op: "start", Path: "/not-executable", Err: syscall.EACCES} + launchErr := launchError(cause) + if launchErr.NotFound { + t.Fatal("NotFound = true for a non-ENOENT path error") + } + if !errors.Is(launchErr, cause) { + t.Fatal("LaunchError did not preserve the original cause") + } +} + func testNormalRun(t *testing.T) { var stdout bytes.Buffer request := Request{ diff --git a/internal/testfixture/database/store.go b/internal/testfixture/database/store.go index f4905e0..b7a3ae7 100644 --- a/internal/testfixture/database/store.go +++ b/internal/testfixture/database/store.go @@ -30,8 +30,8 @@ func fixtureOrigin() time.Time { return time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) } -// Clock is a deterministic state.Clock pinned to a fixed instant. Advance -// moves the instant so tests can produce stable, ordered timestamps. +// Clock is a deterministic clock pinned to a fixed instant. Advance moves the +// instant so tests can produce stable, ordered timestamps. type Clock struct { now time.Time } @@ -75,7 +75,7 @@ func New(t *testing.T) *Fixture { } stateHome := filepath.Join(root, "state") clock := NewClock(fixtureOrigin()) - store := state.NewStore(state.Dependencies{StateHome: stateHome, Clock: clock}) + store := state.NewStore(state.Dependencies{StateHome: stateHome, Now: clock.Now}) if err := store.Acquire(context.Background()); err != nil { t.Fatalf("fixture store acquire: %v", err) } diff --git a/internal/testfixture/database/store_test.go b/internal/testfixture/database/store_test.go index f10cc93..040e625 100644 --- a/internal/testfixture/database/store_test.go +++ b/internal/testfixture/database/store_test.go @@ -44,9 +44,6 @@ func testFixtureStoreOpen(t *testing.T) { if fixture.Store.Database() == nil { t.Fatal("store database nil after New") } - if fixture.Store.Clock() != fixture.Clock { - t.Fatal("store does not use the fixture clock") - } assertMode(t, fixture.Directory(), 0o700) assertMode(t, fixture.DatabasePath(), 0o600) assertMode(t, fixture.LockPath(), 0o600) @@ -61,7 +58,7 @@ func testFixtureConnectionsIndependent(t *testing.T) { if err := first.Store.Close(); err != nil { t.Fatalf("first close: %v", err) } - reopen := state.NewStore(state.Dependencies{StateHome: first.StateHome, Clock: first.Clock}) + reopen := state.NewStore(state.Dependencies{StateHome: first.StateHome, Now: first.Clock.Now}) if err := reopen.Acquire(context.Background()); err != nil { t.Fatalf("first state home not reopenable: %v", err) }