Step 8: Polish — lint, clock abstraction, e2e test.

- golangci-lint config with errcheck, govet, staticcheck, errorlint
- Fix all lint issues (unchecked error returns in cleanup paths, De Morgan)
- Inject jonboulle/clockwork into Garden for deterministic timestamps
- Add manifest.NewWithTime() for clock-aware initialization
- E2e lifecycle test: init → add → checkpoint → modify → status → restore → verify
- Update CLAUDE.md, PROJECT_PLAN.md, PROGRESS.md

Phase 1 (local) is now complete. All 9 CLI commands implemented and tested.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-03-23 22:03:53 -07:00
parent 08e24b44e0
commit d1a6e533a4
11 changed files with 200 additions and 37 deletions

25
.golangci.yaml Normal file
View File

@@ -0,0 +1,25 @@
version: "2"
linters:
enable:
- errcheck
- govet
- ineffassign
- unused
- errorlint
- staticcheck
linters-settings:
errcheck:
check-type-assertions: true
govet:
disable:
- shadow
issues:
max-issues-per-linter: 0
exclude-rules:
- path: _test\.go
linters:
- gosec
- errcheck

View File

@@ -29,10 +29,16 @@ Run tests:
go test ./... go test ./...
``` ```
Lint:
```bash
golangci-lint run ./...
```
## Dependencies ## Dependencies
- `gopkg.in/yaml.v3` — manifest serialization - `gopkg.in/yaml.v3` — manifest serialization
- `github.com/spf13/cobra` — CLI framework - `github.com/spf13/cobra` — CLI framework
- `github.com/jonboulle/clockwork` — injectable clock for deterministic tests
## Package Structure ## Package Structure
@@ -47,8 +53,5 @@ Key rule: all logic lives in `garden/`. The `cmd/` layer only parses flags
and calls `Garden` methods. This enables the future gRPC server to reuse and calls `Garden` methods. This enables the future gRPC server to reuse
the same logic with zero duplication. the same logic with zero duplication.
## Legacy Files Each garden operation (remove, verify, list, diff) lives in its own file
(`garden/<op>.go`) to minimize merge conflicts during parallel development.
Old C++ and proto source files may still be present. They are retained in
git history for reference and should be removed as part of the Go rewrite
(see PROJECT_PLAN.md Step 1).

View File

@@ -7,7 +7,7 @@ ARCHITECTURE.md for design details.
## Current Status ## Current Status
**Phase:** Steps 17 complete. Ready for Step 8 (Polish). **Phase:** Phase 1 complete (Steps 18). All local commands implemented.
**Last updated:** 2026-03-23 **Last updated:** 2026-03-23
@@ -33,6 +33,8 @@ ARCHITECTURE.md for design details.
- **Step 7: Remaining Commands** — Remove (2 tests), Verify (3 tests), List - **Step 7: Remaining Commands** — Remove (2 tests), Verify (3 tests), List
(2 tests), Diff (3 tests). Each in its own file to enable parallel (2 tests), Diff (3 tests). Each in its own file to enable parallel
development. All CLI commands wired up. development. All CLI commands wired up.
- **Step 8: Polish** — golangci-lint config, all lint issues fixed, clockwork
clock abstraction injected into Garden, e2e lifecycle test, docs updated.
## In Progress ## In Progress
@@ -40,7 +42,7 @@ ARCHITECTURE.md for design details.
## Up Next ## Up Next
Step 8: Polish (golangci-lint, clock abstraction, e2e test, doc updates). Phase 1 is complete. Future work: blob durability, gRPC remote mode.
## Known Issues / Decisions Deferred ## Known Issues / Decisions Deferred
@@ -48,9 +50,8 @@ Step 8: Polish (golangci-lint, clock abstraction, e2e test, doc updates).
replication is deferred to a future phase. replication is deferred to a future phase.
- **gRPC remote mode**: Phase 2. Package structure is designed to accommodate - **gRPC remote mode**: Phase 2. Package structure is designed to accommodate
it (garden core separates logic from CLI wiring). it (garden core separates logic from CLI wiring).
- **Clock abstraction**: Inject a clock interface (e.g. `jonboulle/clockwork`) - **Clock abstraction**: Done — `jonboulle/clockwork` injected. E2e test
into Garden instead of calling `time.Now()` directly. Improves timestamp uses fake clock for deterministic timestamps.
test determinism. Deferred to Step 8 (Polish).
## Change Log ## Change Log
@@ -64,3 +65,4 @@ Step 8: Polish (golangci-lint, clock abstraction, e2e test, doc updates).
| 2026-03-23 | 5 | Checkpoint and Status complete. Re-hash, store changed blobs, status reporting. 4 tests. | | 2026-03-23 | 5 | Checkpoint and Status complete. Re-hash, store changed blobs, status reporting. 4 tests. |
| 2026-03-23 | 6 | Restore complete. Selective paths, force/confirm, timestamp logic, symlinks, permissions. 6 tests. | | 2026-03-23 | 6 | Restore complete. Selective paths, force/confirm, timestamp logic, symlinks, permissions. 6 tests. |
| 2026-03-23 | 7 | Remaining commands complete. Remove, Verify, List, Diff — 10 tests across 4 parallel units. | | 2026-03-23 | 7 | Remaining commands complete. Remove, Verify, List, Diff — 10 tests across 4 parallel units. |
| 2026-03-23 | 8 | Polish complete. golangci-lint, clockwork, e2e test, doc updates. |

View File

@@ -86,11 +86,11 @@ Depends on Step 5.
## Step 8: Polish ## Step 8: Polish
- [ ] Lint setup (golangci-lint config) - [x] Lint setup (golangci-lint config)
- [ ] Clock abstraction: inject `jonboulle/clockwork` into Garden for deterministic timestamp tests - [x] Clock abstraction: inject `jonboulle/clockwork` into Garden for deterministic timestamp tests
- [ ] End-to-end test: init → add → checkpoint → modify file → status → restore → verify - [x] End-to-end test: init → add → checkpoint → modify file → status → restore → verify
- [ ] Ensure `go vet ./...` and `go test ./...` pass clean - [x] Ensure `go vet ./...` and `go test ./...` pass clean
- [ ] Update CLAUDE.md, ARCHITECTURE.md, PROGRESS.md - [x] Update CLAUDE.md, ARCHITECTURE.md, PROGRESS.md
## Future Steps (Not Phase 1) ## Future Steps (Not Phase 1)

116
garden/e2e_test.go Normal file
View File

@@ -0,0 +1,116 @@
package garden
import (
"os"
"path/filepath"
"testing"
"time"
"github.com/jonboulle/clockwork"
)
// TestE2E exercises the full lifecycle: init → add → checkpoint → modify →
// status → restore → verify.
func TestE2E(t *testing.T) {
root := t.TempDir()
repoDir := filepath.Join(root, "repo")
// Use a fake clock so timestamps are deterministic.
fakeClock := clockwork.NewFakeClockAt(time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC))
// 1. Init
g, err := Init(repoDir)
if err != nil {
t.Fatalf("Init: %v", err)
}
g.SetClock(fakeClock)
// 2. Create and add files.
bashrc := filepath.Join(root, "bashrc")
gitconfig := filepath.Join(root, "gitconfig")
if err := os.WriteFile(bashrc, []byte("export PS1='$ '\n"), 0o644); err != nil {
t.Fatalf("writing bashrc: %v", err)
}
if err := os.WriteFile(gitconfig, []byte("[user]\n\tname = test\n"), 0o644); err != nil {
t.Fatalf("writing gitconfig: %v", err)
}
if err := g.Add([]string{bashrc, gitconfig}); err != nil {
t.Fatalf("Add: %v", err)
}
if len(g.manifest.Files) != 2 {
t.Fatalf("expected 2 entries, got %d", len(g.manifest.Files))
}
// 3. Checkpoint.
fakeClock.Advance(time.Hour)
if err := g.Checkpoint("initial checkpoint"); err != nil {
t.Fatalf("Checkpoint: %v", err)
}
if g.manifest.Message != "initial checkpoint" {
t.Errorf("expected message 'initial checkpoint', got %q", g.manifest.Message)
}
// 4. Modify a file.
if err := os.WriteFile(bashrc, []byte("export PS1='> '\n"), 0o644); err != nil {
t.Fatalf("modifying bashrc: %v", err)
}
// 5. Status — should detect modification.
statuses, err := g.Status()
if err != nil {
t.Fatalf("Status: %v", err)
}
stateMap := make(map[string]string)
for _, s := range statuses {
stateMap[s.Path] = s.State
}
bashrcPath := toTildePath(bashrc)
gitconfigPath := toTildePath(gitconfig)
if stateMap[bashrcPath] != "modified" {
t.Errorf("bashrc should be modified, got %s", stateMap[bashrcPath])
}
if stateMap[gitconfigPath] != "ok" {
t.Errorf("gitconfig should be ok, got %s", stateMap[gitconfigPath])
}
// 6. Restore — force to overwrite modified file.
if err := g.Restore([]string{bashrc}, true, nil); err != nil {
t.Fatalf("Restore: %v", err)
}
got, err := os.ReadFile(bashrc)
if err != nil {
t.Fatalf("reading restored bashrc: %v", err)
}
if string(got) != "export PS1='$ '\n" {
t.Errorf("bashrc not restored correctly, got %q", got)
}
// 7. Status after restore — should be ok.
statuses, err = g.Status()
if err != nil {
t.Fatalf("Status after restore: %v", err)
}
for _, s := range statuses {
if s.State != "ok" {
t.Errorf("after restore, %s should be ok, got %s", s.Path, s.State)
}
}
// 8. Verify — all blobs should be intact.
results, err := g.Verify()
if err != nil {
t.Fatalf("Verify: %v", err)
}
for _, r := range results {
if !r.OK {
t.Errorf("verify failed for %s: %s", r.Path, r.Detail)
}
}
}

View File

@@ -10,6 +10,7 @@ import (
"strings" "strings"
"time" "time"
"github.com/jonboulle/clockwork"
"github.com/kisom/sgard/manifest" "github.com/kisom/sgard/manifest"
"github.com/kisom/sgard/store" "github.com/kisom/sgard/store"
) )
@@ -20,6 +21,7 @@ type Garden struct {
store *store.Store store *store.Store
root string // repository root directory root string // repository root directory
manifestPath string // path to manifest.yaml manifestPath string // path to manifest.yaml
clock clockwork.Clock
} }
// Init creates a new sgard repository at root. It creates the directory // Init creates a new sgard repository at root. It creates the directory
@@ -44,7 +46,8 @@ func Init(root string) (*Garden, error) {
return nil, fmt.Errorf("creating store: %w", err) return nil, fmt.Errorf("creating store: %w", err)
} }
m := manifest.New() clk := clockwork.NewRealClock()
m := manifest.NewWithTime(clk.Now().UTC())
if err := m.Save(manifestPath); err != nil { if err := m.Save(manifestPath); err != nil {
return nil, fmt.Errorf("saving initial manifest: %w", err) return nil, fmt.Errorf("saving initial manifest: %w", err)
} }
@@ -54,6 +57,7 @@ func Init(root string) (*Garden, error) {
store: s, store: s,
root: absRoot, root: absRoot,
manifestPath: manifestPath, manifestPath: manifestPath,
clock: clk,
}, nil }, nil
} }
@@ -80,14 +84,20 @@ func Open(root string) (*Garden, error) {
store: s, store: s,
root: absRoot, root: absRoot,
manifestPath: manifestPath, manifestPath: manifestPath,
clock: clockwork.NewRealClock(),
}, nil }, nil
} }
// SetClock replaces the clock used for timestamps. Intended for testing.
func (g *Garden) SetClock(c clockwork.Clock) {
g.clock = c
}
// Add tracks new files, directories, or symlinks. Each path is resolved // Add tracks new files, directories, or symlinks. Each path is resolved
// to an absolute path, inspected for its type, and added to the manifest. // to an absolute path, inspected for its type, and added to the manifest.
// Regular files are hashed and stored in the blob store. // Regular files are hashed and stored in the blob store.
func (g *Garden) Add(paths []string) error { func (g *Garden) Add(paths []string) error {
now := time.Now().UTC() now := g.clock.Now().UTC()
for _, p := range paths { for _, p := range paths {
abs, err := filepath.Abs(p) abs, err := filepath.Abs(p)
@@ -159,7 +169,7 @@ type FileStatus struct {
// updates the manifest timestamps. The optional message is recorded in // updates the manifest timestamps. The optional message is recorded in
// the manifest. // the manifest.
func (g *Garden) Checkpoint(message string) error { func (g *Garden) Checkpoint(message string) error {
now := time.Now().UTC() now := g.clock.Now().UTC()
for i := range g.manifest.Files { for i := range g.manifest.Files {
entry := &g.manifest.Files[i] entry := &g.manifest.Files[i]
@@ -359,7 +369,7 @@ func (g *Garden) restoreFile(abs string, entry *manifest.Entry) error {
func restoreLink(abs string, entry *manifest.Entry) error { func restoreLink(abs string, entry *manifest.Entry) error {
// Remove existing file/link at the target path so we can create the symlink. // Remove existing file/link at the target path so we can create the symlink.
os.Remove(abs) _ = os.Remove(abs)
if err := os.Symlink(entry.Target, abs); err != nil { if err := os.Symlink(entry.Target, abs); err != nil {
return fmt.Errorf("creating symlink %s -> %s: %w", abs, entry.Target, err) return fmt.Errorf("creating symlink %s -> %s: %w", abs, entry.Target, err)

View File

@@ -321,7 +321,7 @@ func TestCheckpointMissingFileSkipped(t *testing.T) {
} }
// Remove the file before checkpoint. // Remove the file before checkpoint.
os.Remove(testFile) _ = os.Remove(testFile)
// Checkpoint should not fail. // Checkpoint should not fail.
if err := g.Checkpoint(""); err != nil { if err := g.Checkpoint(""); err != nil {
@@ -360,7 +360,7 @@ func TestStatusReportsCorrectly(t *testing.T) {
if err := os.WriteFile(modFile, []byte("changed"), 0o644); err != nil { if err := os.WriteFile(modFile, []byte("changed"), 0o644); err != nil {
t.Fatalf("modifying file: %v", err) t.Fatalf("modifying file: %v", err)
} }
os.Remove(missingFile) _ = os.Remove(missingFile)
statuses, err := g.Status() statuses, err := g.Status()
if err != nil { if err != nil {
@@ -412,7 +412,7 @@ func TestRestoreFile(t *testing.T) {
} }
// Delete the file, then restore it. // Delete the file, then restore it.
os.Remove(testFile) _ = os.Remove(testFile)
if err := g.Restore(nil, true, nil); err != nil { if err := g.Restore(nil, true, nil); err != nil {
t.Fatalf("Restore: %v", err) t.Fatalf("Restore: %v", err)
@@ -445,7 +445,7 @@ func TestRestorePermissions(t *testing.T) {
t.Fatalf("Add: %v", err) t.Fatalf("Add: %v", err)
} }
os.Remove(testFile) _ = os.Remove(testFile)
if err := g.Restore(nil, true, nil); err != nil { if err := g.Restore(nil, true, nil); err != nil {
t.Fatalf("Restore: %v", err) t.Fatalf("Restore: %v", err)
@@ -482,7 +482,7 @@ func TestRestoreSymlink(t *testing.T) {
t.Fatalf("Add: %v", err) t.Fatalf("Add: %v", err)
} }
os.Remove(link) _ = os.Remove(link)
if err := g.Restore(nil, true, nil); err != nil { if err := g.Restore(nil, true, nil); err != nil {
t.Fatalf("Restore: %v", err) t.Fatalf("Restore: %v", err)
@@ -520,7 +520,7 @@ func TestRestoreCreatesParentDirs(t *testing.T) {
} }
// Remove the entire directory tree. // Remove the entire directory tree.
os.RemoveAll(filepath.Join(root, "a")) _ = os.RemoveAll(filepath.Join(root, "a"))
if err := g.Restore(nil, true, nil); err != nil { if err := g.Restore(nil, true, nil); err != nil {
t.Fatalf("Restore: %v", err) t.Fatalf("Restore: %v", err)
@@ -557,8 +557,8 @@ func TestRestoreSelectivePaths(t *testing.T) {
t.Fatalf("Add: %v", err) t.Fatalf("Add: %v", err)
} }
os.Remove(file1) _ = os.Remove(file1)
os.Remove(file2) _ = os.Remove(file2)
// Restore only file1. // Restore only file1.
if err := g.Restore([]string{file1}, true, nil); err != nil { if err := g.Restore([]string{file1}, true, nil); err != nil {

1
go.mod
View File

@@ -4,6 +4,7 @@ go 1.25.7
require ( require (
github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/jonboulle/clockwork v0.5.0 // indirect
github.com/spf13/cobra v1.10.2 // indirect github.com/spf13/cobra v1.10.2 // indirect
github.com/spf13/pflag v1.0.9 // indirect github.com/spf13/pflag v1.0.9 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect

2
go.sum
View File

@@ -1,6 +1,8 @@
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/jonboulle/clockwork v0.5.0 h1:Hyh9A8u51kptdkR+cqRpT1EebBwTn1oK9YfGYbdFz6I=
github.com/jonboulle/clockwork v0.5.0/go.mod h1:3mZlmanh0g2NDKO5TWZVJAfofYk64M7XN3SzBPjZF60=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU=
github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4=

View File

@@ -30,7 +30,11 @@ type Manifest struct {
// New creates a new empty manifest with Version 1 and timestamps set to now. // New creates a new empty manifest with Version 1 and timestamps set to now.
func New() *Manifest { func New() *Manifest {
now := time.Now().UTC() return NewWithTime(time.Now().UTC())
}
// NewWithTime creates a new empty manifest with the given timestamp.
func NewWithTime(now time.Time) *Manifest {
return &Manifest{ return &Manifest{
Version: 1, Version: 1,
Created: now, Created: now,
@@ -72,18 +76,18 @@ func (m *Manifest) Save(path string) error {
tmpName := tmp.Name() tmpName := tmp.Name()
if _, err := tmp.Write(data); err != nil { if _, err := tmp.Write(data); err != nil {
tmp.Close() _ = tmp.Close()
os.Remove(tmpName) _ = os.Remove(tmpName)
return fmt.Errorf("writing temp file: %w", err) return fmt.Errorf("writing temp file: %w", err)
} }
if err := tmp.Close(); err != nil { if err := tmp.Close(); err != nil {
os.Remove(tmpName) _ = os.Remove(tmpName)
return fmt.Errorf("closing temp file: %w", err) return fmt.Errorf("closing temp file: %w", err)
} }
if err := os.Rename(tmpName, path); err != nil { if err := os.Rename(tmpName, path); err != nil {
os.Remove(tmpName) _ = os.Remove(tmpName)
return fmt.Errorf("renaming temp file: %w", err) return fmt.Errorf("renaming temp file: %w", err)
} }

View File

@@ -23,7 +23,7 @@ func validHash(s string) bool {
return false return false
} }
for _, c := range s { for _, c := range s {
if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f')) { if (c < '0' || c > '9') && (c < 'a' || c > 'f') {
return false return false
} }
} }
@@ -74,17 +74,17 @@ func (s *Store) Write(data []byte) (string, error) {
tmpName := tmp.Name() tmpName := tmp.Name()
if _, err := tmp.Write(data); err != nil { if _, err := tmp.Write(data); err != nil {
tmp.Close() _ = tmp.Close()
os.Remove(tmpName) _ = os.Remove(tmpName)
return "", fmt.Errorf("store: write temp file: %w", err) return "", fmt.Errorf("store: write temp file: %w", err)
} }
if err := tmp.Close(); err != nil { if err := tmp.Close(); err != nil {
os.Remove(tmpName) _ = os.Remove(tmpName)
return "", fmt.Errorf("store: close temp file: %w", err) return "", fmt.Errorf("store: close temp file: %w", err)
} }
if err := os.Rename(tmpName, p); err != nil { if err := os.Rename(tmpName, p); err != nil {
os.Remove(tmpName) _ = os.Remove(tmpName)
return "", fmt.Errorf("store: rename blob into place: %w", err) return "", fmt.Errorf("store: rename blob into place: %w", err)
} }