From 6e236bdea61b05531150e5f19332c4fb78dd4569 Mon Sep 17 00:00:00 2001 From: Manuel Zubieta Date: Sun, 12 Apr 2026 06:33:52 -0400 Subject: [PATCH 01/19] Add internal/registry package with host-local canonical store Host-local JSON store tracking beadwork-enabled repos: - Load/Save with atomic temp-file rename for safe concurrent writes - Touch/TouchAndSave for registering repos with timestamps - AdvanceCursorAndSave for incremental recap cursors - Prune for removing stale entries by predicate - Unknown-field preservation across load/save cycles - Schema version check refusing newer-than-supported - Home resolution: BEADWORK_HOME > ~/.beadwork (os.UserHomeDir honors $HOME) - CanonicalRepoPath resolving worktrees to their main repo root Also adds test scaffolding used throughout the series: - newBwEnv() sets BEADWORK_HOME to an isolated temp dir - newMultiRepoEnv(t, n) creates n repos sharing a registry dir - seedRegistry() / registryContents() for registry state setup/assertion - compareGolden(t, name, got) generalized golden-file comparison - bwNow() returns the deterministic clock as time.Time - bwFail() for testing expected-error paths --- internal/registry/canonical.go | 73 +++++++++++ internal/registry/canonical_test.go | 91 +++++++++++++ internal/registry/dir.go | 26 ++++ internal/registry/dir_test.go | 49 +++++++ internal/registry/registry.go | 188 ++++++++++++++++++++++++++ internal/registry/registry_test.go | 196 ++++++++++++++++++++++++++++ test/acceptance_test.go | 159 ++++++++++++++++++++-- 7 files changed, 772 insertions(+), 10 deletions(-) create mode 100644 internal/registry/canonical.go create mode 100644 internal/registry/canonical_test.go create mode 100644 internal/registry/dir.go create mode 100644 internal/registry/dir_test.go create mode 100644 internal/registry/registry.go create mode 100644 internal/registry/registry_test.go diff --git a/internal/registry/canonical.go b/internal/registry/canonical.go new file mode 100644 index 00000000..7d6d34a9 --- /dev/null +++ b/internal/registry/canonical.go @@ -0,0 +1,73 @@ +package registry + +import ( + "os" + "path/filepath" + "strings" +) + +// CanonicalRepoPath resolves a directory to its main repository root. +// If dir is inside a git worktree, it follows the .git file → commondir +// chain to find the shared .git directory, then returns its parent. +// For normal repositories, it walks up looking for a .git directory. +func CanonicalRepoPath(dir string) (string, error) { + dir, err := filepath.Abs(dir) + if err != nil { + return "", err + } + return findMainRepo(dir) +} + +func findMainRepo(dir string) (string, error) { + cur := dir + for { + dotGit := filepath.Join(cur, ".git") + fi, err := os.Stat(dotGit) + if err == nil { + if fi.IsDir() { + return cur, nil + } + // Worktree: .git is a file pointing to the real git dir. + return resolveWorktreeRoot(dotGit) + } + parent := filepath.Dir(cur) + if parent == cur { + return dir, nil // not a git repo; return as-is + } + cur = parent + } +} + +func resolveWorktreeRoot(dotGitFile string) (string, error) { + data, err := os.ReadFile(dotGitFile) + if err != nil { + return "", err + } + line := strings.TrimSpace(string(data)) + if !strings.HasPrefix(line, "gitdir: ") { + return filepath.Dir(dotGitFile), nil + } + + gitdir := strings.TrimPrefix(line, "gitdir: ") + if !filepath.IsAbs(gitdir) { + gitdir = filepath.Join(filepath.Dir(dotGitFile), gitdir) + } + gitdir = filepath.Clean(gitdir) + + // Read commondir to find the shared .git directory. + cdData, err := os.ReadFile(filepath.Join(gitdir, "commondir")) + if err != nil { + return filepath.Dir(gitdir), nil + } + + commondir := strings.TrimSpace(string(cdData)) + if !filepath.IsAbs(commondir) { + commondir = filepath.Join(gitdir, commondir) + } + commondir, err = filepath.Abs(commondir) + if err != nil { + return "", err + } + // The repo root is the parent of the .git directory. + return filepath.Dir(commondir), nil +} diff --git a/internal/registry/canonical_test.go b/internal/registry/canonical_test.go new file mode 100644 index 00000000..336aa6d6 --- /dev/null +++ b/internal/registry/canonical_test.go @@ -0,0 +1,91 @@ +package registry + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestCanonicalRepoPathNormalRepo(t *testing.T) { + dir := t.TempDir() + gitInit(t, dir) + + got, err := CanonicalRepoPath(dir) + if err != nil { + t.Fatalf("CanonicalRepoPath: %v", err) + } + if got != dir { + t.Errorf("got %q, want %q", got, dir) + } +} + +func TestCanonicalRepoPathSubdir(t *testing.T) { + dir := t.TempDir() + gitInit(t, dir) + + sub := filepath.Join(dir, "a", "b") + os.MkdirAll(sub, 0755) + + got, err := CanonicalRepoPath(sub) + if err != nil { + t.Fatalf("CanonicalRepoPath: %v", err) + } + if got != dir { + t.Errorf("got %q, want %q", got, dir) + } +} + +func TestCanonicalRepoPathWorktree(t *testing.T) { + dir := t.TempDir() + gitInit(t, dir) + gitRun(t, dir, "commit", "--allow-empty", "-m", "initial") + + wtDir := filepath.Join(t.TempDir(), "worktree") + gitRun(t, dir, "worktree", "add", wtDir, "-b", "wt-branch") + t.Cleanup(func() { + gitRun(t, dir, "worktree", "remove", "--force", wtDir) + }) + + got, err := CanonicalRepoPath(wtDir) + if err != nil { + t.Fatalf("CanonicalRepoPath: %v", err) + } + + // Resolve symlinks for comparison (macOS /private/tmp vs /tmp). + wantReal, _ := filepath.EvalSymlinks(dir) + gotReal, _ := filepath.EvalSymlinks(got) + if gotReal != wantReal { + t.Errorf("worktree resolved to %q, want %q", gotReal, wantReal) + } +} + +func TestCanonicalRepoPathNotGitRepo(t *testing.T) { + dir := t.TempDir() + got, err := CanonicalRepoPath(dir) + if err != nil { + t.Fatalf("CanonicalRepoPath: %v", err) + } + // For non-git dirs, returns the dir as-is. + if !strings.HasPrefix(got, dir) { + t.Errorf("got %q, want prefix %q", got, dir) + } +} + +func gitInit(t *testing.T, dir string) { + t.Helper() + gitRun(t, dir, "init") + gitRun(t, dir, "config", "user.email", "test@test.com") + gitRun(t, dir, "config", "user.name", "Test") +} + +func gitRun(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %s: %s: %v", strings.Join(args, " "), out, err) + } +} diff --git a/internal/registry/dir.go b/internal/registry/dir.go new file mode 100644 index 00000000..309460cc --- /dev/null +++ b/internal/registry/dir.go @@ -0,0 +1,26 @@ +package registry + +import ( + "os" + "path/filepath" +) + +const dirName = ".beadwork" + +// DefaultDir returns the beadwork home directory. BEADWORK_HOME overrides +// it; otherwise it falls back to ~/.beadwork. os.UserHomeDir honors $HOME, +// so callers who want a different home can point HOME at it. +func DefaultDir() string { + return resolveFrom(os.Getenv("BEADWORK_HOME"), os.UserHomeDir) +} + +func resolveFrom(envHome string, homeFn func() (string, error)) string { + if envHome != "" { + return envHome + } + home, err := homeFn() + if err != nil { + return dirName + } + return filepath.Join(home, dirName) +} diff --git a/internal/registry/dir_test.go b/internal/registry/dir_test.go new file mode 100644 index 00000000..98d43cef --- /dev/null +++ b/internal/registry/dir_test.go @@ -0,0 +1,49 @@ +package registry + +import ( + "errors" + "path/filepath" + "testing" +) + +func TestResolveFrom(t *testing.T) { + homeDir := "/home/testuser" + homeFn := func() (string, error) { return homeDir, nil } + errHome := func() (string, error) { return "", errors.New("no home") } + + tests := []struct { + name string + envHome string + homeFn func() (string, error) + want string + }{ + { + name: "BEADWORK_HOME takes precedence", + envHome: "/custom/beadwork", + want: "/custom/beadwork", + }, + { + name: "falls back to ~/.beadwork", + homeFn: homeFn, + want: filepath.Join(homeDir, ".beadwork"), + }, + { + name: "home dir error uses relative path", + homeFn: errHome, + want: ".beadwork", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fn := tt.homeFn + if fn == nil { + fn = homeFn + } + got := resolveFrom(tt.envHome, fn) + if got != tt.want { + t.Errorf("resolveFrom() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/registry/registry.go b/internal/registry/registry.go new file mode 100644 index 00000000..095a1bb5 --- /dev/null +++ b/internal/registry/registry.go @@ -0,0 +1,188 @@ +// Package registry tracks which repositories on this host use beadwork. +// The registry is a single JSON file stored under the beadwork home +// directory (~/.beadwork by default, or $BEADWORK_HOME). It records the +// last time each repo was seen and an opaque cursor for incremental +// recap processing. +package registry + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sync" + "time" +) + +const SchemaVersion = 1 +const registryFile = "registry.json" + +// Entry represents a single tracked repository. +type Entry struct { + LastSeenAt string `json:"last_seen_at"` + Cursor string `json:"cursor,omitempty"` + Prefix string `json:"prefix,omitempty"` +} + +// Registry holds the in-memory state of the registry file. +type Registry struct { + SchemaVersion int `json:"schema_version"` + Repos map[string]Entry `json:"repos"` + + // extra preserves unknown top-level fields across load/save cycles. + extra map[string]json.RawMessage + + dir string // directory containing the registry file + mu sync.Mutex +} + +// Load reads the registry from dir. If the file does not exist, returns +// an empty registry. Returns an error if the file exists but the schema +// version is newer than this binary supports. +func Load(dir string) (*Registry, error) { + r := &Registry{ + SchemaVersion: SchemaVersion, + Repos: make(map[string]Entry), + dir: dir, + } + + path := filepath.Join(dir, registryFile) + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return r, nil + } + return nil, fmt.Errorf("read registry: %w", err) + } + + // Decode into a raw map first to preserve unknown fields. + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return nil, fmt.Errorf("parse registry: %w", err) + } + + // Extract known fields. + if v, ok := raw["schema_version"]; ok { + if err := json.Unmarshal(v, &r.SchemaVersion); err != nil { + return nil, fmt.Errorf("parse schema_version: %w", err) + } + delete(raw, "schema_version") + } + + if r.SchemaVersion > SchemaVersion { + return nil, fmt.Errorf("registry schema version %d is newer than supported (%d); upgrade bw", r.SchemaVersion, SchemaVersion) + } + + if v, ok := raw["repos"]; ok { + if err := json.Unmarshal(v, &r.Repos); err != nil { + return nil, fmt.Errorf("parse repos: %w", err) + } + delete(raw, "repos") + } + + r.extra = raw + return r, nil +} + +// Save atomically writes the registry to disk using a temp-file + rename. +func (r *Registry) Save() error { + r.mu.Lock() + defer r.mu.Unlock() + return r.saveLocked() +} + +func (r *Registry) saveLocked() error { + if err := os.MkdirAll(r.dir, 0755); err != nil { + return fmt.Errorf("create registry dir: %w", err) + } + + // Build the output map preserving unknown fields. + out := make(map[string]interface{}, len(r.extra)+2) + for k, v := range r.extra { + out[k] = v + } + out["schema_version"] = r.SchemaVersion + out["repos"] = r.Repos + + data, err := json.MarshalIndent(out, "", " ") + if err != nil { + return fmt.Errorf("marshal registry: %w", err) + } + data = append(data, '\n') + + path := filepath.Join(r.dir, registryFile) + tmp := path + ".tmp" + if err := os.WriteFile(tmp, data, 0644); err != nil { + return fmt.Errorf("write temp registry: %w", err) + } + if err := os.Rename(tmp, path); err != nil { + os.Remove(tmp) + return fmt.Errorf("rename registry: %w", err) + } + return nil +} + +// Touch registers or updates a repo entry with the given timestamp. +func (r *Registry) Touch(repoPath string, now time.Time) { + r.mu.Lock() + defer r.mu.Unlock() + + e := r.Repos[repoPath] + e.LastSeenAt = now.UTC().Format(time.RFC3339) + r.Repos[repoPath] = e +} + +// TouchAndSave is a convenience that calls Touch then Save. +func (r *Registry) TouchAndSave(repoPath string, now time.Time) error { + r.mu.Lock() + defer r.mu.Unlock() + + e := r.Repos[repoPath] + e.LastSeenAt = now.UTC().Format(time.RFC3339) + r.Repos[repoPath] = e + return r.saveLocked() +} + +// AdvanceCursorAndSave updates the cursor for a repo and saves atomically. +func (r *Registry) AdvanceCursorAndSave(repoPath, cursor string) error { + r.mu.Lock() + defer r.mu.Unlock() + + e := r.Repos[repoPath] + e.Cursor = cursor + r.Repos[repoPath] = e + return r.saveLocked() +} + +// Prune removes entries for which the predicate returns true. +// Returns the list of removed repo paths. +func (r *Registry) Prune(predicate func(path string, e Entry) bool) []string { + r.mu.Lock() + defer r.mu.Unlock() + + var removed []string + for path, e := range r.Repos { + if predicate(path, e) { + delete(r.Repos, path) + removed = append(removed, path) + } + } + return removed +} + +// Entries returns a snapshot of all registry entries. +func (r *Registry) Entries() map[string]Entry { + r.mu.Lock() + defer r.mu.Unlock() + + cp := make(map[string]Entry, len(r.Repos)) + for k, v := range r.Repos { + cp[k] = v + } + return cp +} + +// Dir returns the directory where the registry file lives. +func (r *Registry) Dir() string { + return r.dir +} diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go new file mode 100644 index 00000000..9db8a6b1 --- /dev/null +++ b/internal/registry/registry_test.go @@ -0,0 +1,196 @@ +package registry + +import ( + "encoding/json" + "os" + "path/filepath" + "sync" + "testing" + "time" +) + +func TestLoadEmpty(t *testing.T) { + dir := t.TempDir() + r, err := Load(dir) + if err != nil { + t.Fatalf("Load: %v", err) + } + if r.SchemaVersion != SchemaVersion { + t.Errorf("SchemaVersion = %d, want %d", r.SchemaVersion, SchemaVersion) + } + if len(r.Repos) != 0 { + t.Errorf("Repos = %v, want empty", r.Repos) + } +} + +func TestLoadSaveRoundTrip(t *testing.T) { + dir := t.TempDir() + r, err := Load(dir) + if err != nil { + t.Fatal(err) + } + + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + r.Touch("/home/user/project-a", now) + r.Touch("/home/user/project-b", now.Add(time.Hour)) + + if err := r.Save(); err != nil { + t.Fatalf("Save: %v", err) + } + + r2, err := Load(dir) + if err != nil { + t.Fatalf("Load after save: %v", err) + } + if len(r2.Repos) != 2 { + t.Fatalf("Repos count = %d, want 2", len(r2.Repos)) + } + + ea := r2.Repos["/home/user/project-a"] + if ea.LastSeenAt != "2026-01-15T10:00:00Z" { + t.Errorf("project-a LastSeenAt = %q", ea.LastSeenAt) + } +} + +func TestSchemaVersionNewerRefused(t *testing.T) { + dir := t.TempDir() + data := `{"schema_version": 999, "repos": {}}` + os.WriteFile(filepath.Join(dir, registryFile), []byte(data), 0644) + + _, err := Load(dir) + if err == nil { + t.Fatal("expected error for newer schema version") + } +} + +func TestUnknownFieldPreservation(t *testing.T) { + dir := t.TempDir() + original := `{"schema_version":1,"repos":{},"future_field":"hello"}` + os.WriteFile(filepath.Join(dir, registryFile), []byte(original), 0644) + + r, err := Load(dir) + if err != nil { + t.Fatal(err) + } + + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + r.Touch("/tmp/repo", now) + if err := r.Save(); err != nil { + t.Fatal(err) + } + + data, err := os.ReadFile(filepath.Join(dir, registryFile)) + if err != nil { + t.Fatal(err) + } + + var raw map[string]json.RawMessage + json.Unmarshal(data, &raw) + if _, ok := raw["future_field"]; !ok { + t.Error("future_field not preserved after save") + } +} + +func TestTouchAndSave(t *testing.T) { + dir := t.TempDir() + r, _ := Load(dir) + + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + if err := r.TouchAndSave("/tmp/repo", now); err != nil { + t.Fatalf("TouchAndSave: %v", err) + } + + r2, _ := Load(dir) + if len(r2.Repos) != 1 { + t.Fatalf("Repos count = %d, want 1", len(r2.Repos)) + } +} + +func TestAdvanceCursorAndSave(t *testing.T) { + dir := t.TempDir() + r, _ := Load(dir) + + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + r.Touch("/tmp/repo", now) + r.Save() + + if err := r.AdvanceCursorAndSave("/tmp/repo", "abc123"); err != nil { + t.Fatalf("AdvanceCursorAndSave: %v", err) + } + + r2, _ := Load(dir) + e := r2.Repos["/tmp/repo"] + if e.Cursor != "abc123" { + t.Errorf("Cursor = %q, want abc123", e.Cursor) + } + if e.LastSeenAt != "2026-01-15T10:00:00Z" { + t.Errorf("LastSeenAt lost after cursor advance: %q", e.LastSeenAt) + } +} + +func TestPrune(t *testing.T) { + dir := t.TempDir() + r, _ := Load(dir) + + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + r.Touch("/keep", now) + r.Touch("/remove-me", now.Add(-30*24*time.Hour)) + + cutoff := now.Add(-7 * 24 * time.Hour) + removed := r.Prune(func(path string, e Entry) bool { + t, err := time.Parse(time.RFC3339, e.LastSeenAt) + if err != nil { + return false + } + return t.Before(cutoff) + }) + + if len(removed) != 1 || removed[0] != "/remove-me" { + t.Errorf("Prune removed = %v, want [/remove-me]", removed) + } + if _, ok := r.Repos["/keep"]; !ok { + t.Error("/keep was incorrectly pruned") + } +} + +func TestConcurrentSave(t *testing.T) { + dir := t.TempDir() + r, _ := Load(dir) + + var wg sync.WaitGroup + for i := range 20 { + wg.Add(1) + go func(n int) { + defer wg.Done() + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC).Add(time.Duration(n) * time.Second) + r.TouchAndSave("/repo/"+string(rune('a'+n)), now) + }(i) + } + wg.Wait() + + r2, err := Load(dir) + if err != nil { + t.Fatalf("Load after concurrent saves: %v", err) + } + if len(r2.Repos) != 20 { + t.Errorf("Repos count = %d, want 20", len(r2.Repos)) + } +} + +func TestEntries(t *testing.T) { + dir := t.TempDir() + r, _ := Load(dir) + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + r.Touch("/a", now) + r.Touch("/b", now) + + entries := r.Entries() + if len(entries) != 2 { + t.Errorf("Entries count = %d, want 2", len(entries)) + } + // Mutating the snapshot should not affect the original. + delete(entries, "/a") + if len(r.Repos) != 2 { + t.Error("mutating Entries() snapshot affected original") + } +} diff --git a/test/acceptance_test.go b/test/acceptance_test.go index c019bdec..1b20b35a 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" "testing" + "time" ) var bwBin string // path to built bw binary @@ -34,9 +35,10 @@ func TestMain(m *testing.M) { // bwEnv is a self-contained environment for running bw commands against // a deterministic git repo. type bwEnv struct { - t *testing.T - dir string - env []string + t *testing.T + dir string + registryDir string + env []string } const fixedClock = "2026-01-15T10:00:00Z" @@ -44,16 +46,19 @@ const fixedClock = "2026-01-15T10:00:00Z" func newBwEnv(t *testing.T) *bwEnv { t.Helper() dir := t.TempDir() + registryDir := t.TempDir() env := &bwEnv{ - t: t, - dir: dir, + t: t, + dir: dir, + registryDir: registryDir, env: append(os.Environ(), "BW_CLOCK="+fixedClock, "BW_CONFIG="+filepath.Join(dir, ".bw"), "GIT_AUTHOR_DATE="+fixedClock, "GIT_COMMITTER_DATE="+fixedClock, "NO_COLOR=1", + "BEADWORK_HOME="+registryDir, ), } @@ -98,6 +103,23 @@ func (e *bwEnv) bw(args ...string) string { return stdout.String() } +// bwFail runs a bw command that is expected to fail. +// Returns combined stdout+stderr. Fatals if the command succeeds. +func (e *bwEnv) bwFail(args ...string) string { + e.t.Helper() + cmd := exec.Command(bwBin, args...) + cmd.Dir = e.dir + cmd.Env = e.env + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err == nil { + e.t.Fatalf("bw %s: expected failure but succeeded\nstdout: %s", + strings.Join(args, " "), stdout.String()) + } + return stdout.String() + stderr.String() +} + // bwAt runs bw from a custom directory instead of the default e.dir. func (e *bwEnv) bwAt(dir string, args ...string) string { e.t.Helper() @@ -119,28 +141,145 @@ func (e *bwEnv) bwAt(dir string, args ...string) string { func (e *bwEnv) goldenCompare(name string) { e.t.Helper() got := e.bw("export") + compareGolden(e.t, name+".golden.jsonl", got) +} - goldenPath := filepath.Join("testdata", name+".golden.jsonl") +// compareGolden compares got against a golden file in testdata/. +// If UPDATE_GOLDEN=1, writes got as the new golden file instead. +func compareGolden(t *testing.T, name, got string) { + t.Helper() + goldenPath := filepath.Join("testdata", name) if os.Getenv("UPDATE_GOLDEN") == "1" { if err := os.WriteFile(goldenPath, []byte(got), 0644); err != nil { - e.t.Fatalf("write golden file: %v", err) + t.Fatalf("write golden file: %v", err) } - e.t.Logf("updated golden file: %s", goldenPath) + t.Logf("updated golden file: %s", goldenPath) return } want, err := os.ReadFile(goldenPath) if err != nil { - e.t.Fatalf("read golden file (run with UPDATE_GOLDEN=1 to create): %v", err) + t.Fatalf("read golden file (run with UPDATE_GOLDEN=1 to create): %v", err) } if got != string(want) { - e.t.Errorf("export output does not match golden file %s\n--- want ---\n%s\n--- got ---\n%s", + t.Errorf("output does not match golden file %s\n--- want ---\n%s\n--- got ---\n%s", goldenPath, string(want), got) } } +// newMultiRepoEnv creates n independent bwEnv instances that share the same +// registry directory, simulating multiple repos registered in a single registry. +func newMultiRepoEnv(t *testing.T, n int) []*bwEnv { + t.Helper() + registryDir := t.TempDir() + envs := make([]*bwEnv, n) + for i := range n { + dir := t.TempDir() + env := &bwEnv{ + t: t, + dir: dir, + registryDir: registryDir, + env: append(os.Environ(), + "BW_CLOCK="+fixedClock, + "GIT_AUTHOR_DATE="+fixedClock, + "GIT_COMMITTER_DATE="+fixedClock, + "NO_COLOR=1", + "BEADWORK_HOME="+registryDir, + ), + } + env.git("init") + env.git("config", "user.email", "test@beadwork.dev") + env.git("config", "user.name", "Test User") + os.WriteFile(filepath.Join(dir, "README"), []byte(fmt.Sprintf("repo %d", i)), 0644) + env.git("add", ".") + env.git("commit", "-m", "initial") + env.bw("init", "--prefix", fmt.Sprintf("r%d", i)) + envs[i] = env + } + return envs +} + +// seedRegistry writes raw JSON content to a file named "registry.json" in +// the env's registry directory. Use this to set up registry state before +// running commands that read the registry. +func (e *bwEnv) seedRegistry(content string) { + e.t.Helper() + path := filepath.Join(e.registryDir, "registry.json") + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + e.t.Fatalf("seedRegistry: %v", err) + } +} + +// registryContents reads and returns the raw content of the registry file. +// Returns an empty string if the file does not exist. +func (e *bwEnv) registryContents() string { + e.t.Helper() + path := filepath.Join(e.registryDir, "registry.json") + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return "" + } + e.t.Fatalf("registryContents: %v", err) + } + return string(data) +} + +// bwNow returns the time that bw commands will use as "now", respecting BW_CLOCK. +// Panics if BW_CLOCK is set but not parseable (test setup error). +func bwNow() time.Time { + t, err := time.Parse(time.RFC3339, fixedClock) + if err != nil { + panic("fixedClock is not valid RFC3339: " + err.Error()) + } + return t.UTC() +} + +// TestScaffoldingHelpers verifies that the test scaffolding helpers work correctly. +func TestScaffoldingHelpers(t *testing.T) { + // bwNow should match fixedClock + now := bwNow() + if now.Format(time.RFC3339) != fixedClock { + t.Errorf("bwNow() = %v, want %v", now.Format(time.RFC3339), fixedClock) + } + + // newBwEnv should set up a registry dir + env := newBwEnv(t) + if env.registryDir == "" { + t.Fatal("registryDir not set") + } + + // seedRegistry + registryContents round-trip + env.seedRegistry(`{"repos":{}}`) + got := env.registryContents() + if got != `{"repos":{}}` { + t.Errorf("registryContents() = %q, want %q", got, `{"repos":{}}`) + } + + // registryContents on empty dir returns "" + env2 := newBwEnv(t) + if c := env2.registryContents(); c != "" { + t.Errorf("registryContents() on fresh env = %q, want empty", c) + } + + // newMultiRepoEnv creates n envs sharing a registry dir + envs := newMultiRepoEnv(t, 3) + if len(envs) != 3 { + t.Fatalf("newMultiRepoEnv(3) returned %d envs", len(envs)) + } + sharedDir := envs[0].registryDir + for i, e := range envs { + if e.registryDir != sharedDir { + t.Errorf("env[%d].registryDir = %q, want %q (shared)", i, e.registryDir, sharedDir) + } + // Each env should be an independent bw repo + out := e.bw("list") + _ = out // no issues yet, just verify it runs + } +} + // TestGoldenBasicScenario exercises the full CLI pipeline: create, parent, // dependencies, labels, status transitions, comments, defer, then verifies // export output against a golden file. Finally, imports into a fresh repo From 04dcd77237e37814fda320548b0de869daa74627 Mon Sep 17 00:00:00 2001 From: Manuel Zubieta Date: Sun, 12 Apr 2026 06:51:49 -0400 Subject: [PATCH 02/19] Auto-register beadwork repos + bw registry list/prune Auto-registration: - touchRegistry() runs after every successful command, registering the current repo in the host-local registry - Resolves via FindRepoAt (not NeedsStore) so it fires for all commands - Uses CanonicalRepoPath to deduplicate worktrees to their main repo - Gated on IsInitialized() so non-beadwork git repos aren't registered - Respects BW_CLOCK for deterministic timestamps in tests - Silently ignores errors (registryErrorOnce warns once on stderr) - BEADWORK_HOME env var isolates tests from the real registry bw registry subcommand: - list: TTY + plain + JSON modes, MISSING flag for deleted repos, live prefix read from repo, sorted by path - prune: --yes/-y for non-interactive, TTY stdin check, half-removal warning when >50% of entries would be removed - Appears in bw --help under Cross-Repo & Activity - Nested dispatch routes registry with --help support CLI tests (cli_test.go) now set BEADWORK_HOME to t.TempDir() via a shared bwTestEnv() helper, so shelling out the bw binary doesn't pollute the user's real registry. --- cmd/bw/cli_test.go | 29 +++--- cmd/bw/command.go | 16 +++ cmd/bw/helpers.go | 1 + cmd/bw/main.go | 44 ++++++++ cmd/bw/registry.go | 224 ++++++++++++++++++++++++++++++++++++++++ test/acceptance_test.go | 197 ++++++++++++++++++++++++++++++++++- 6 files changed, 493 insertions(+), 18 deletions(-) create mode 100644 cmd/bw/registry.go diff --git a/cmd/bw/cli_test.go b/cmd/bw/cli_test.go index edfca1e9..27b66543 100644 --- a/cmd/bw/cli_test.go +++ b/cmd/bw/cli_test.go @@ -74,19 +74,25 @@ func mustCwd() string { return d } -// bw runs the bw binary in the test env dir and returns stdout. -func bw(t *testing.T, dir string, args ...string) string { +func bwTestEnv(t *testing.T, dir string) []string { t.Helper() - cmd := exec.Command(bwBin, args...) - cmd.Dir = dir - cmd.Env = append(os.Environ(), + return append(os.Environ(), "GIT_AUTHOR_NAME=Test", "GIT_AUTHOR_EMAIL=test@test.com", "GIT_COMMITTER_NAME=Test", "GIT_COMMITTER_EMAIL=test@test.com", "BW_CONFIG="+filepath.Join(dir, ".bw"), + "BEADWORK_HOME="+t.TempDir(), "GOCOVERDIR="+bwCoverDir, ) +} + +// bw runs the bw binary in the test env dir and returns stdout. +func bw(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command(bwBin, args...) + cmd.Dir = dir + cmd.Env = bwTestEnv(t, dir) out, err := cmd.CombinedOutput() if err != nil { t.Fatalf("bw %s: %s: %v", strings.Join(args, " "), out, err) @@ -99,14 +105,7 @@ func bwFail(t *testing.T, dir string, args ...string) string { t.Helper() cmd := exec.Command(bwBin, args...) cmd.Dir = dir - cmd.Env = append(os.Environ(), - "GIT_AUTHOR_NAME=Test", - "GIT_AUTHOR_EMAIL=test@test.com", - "GIT_COMMITTER_NAME=Test", - "GIT_COMMITTER_EMAIL=test@test.com", - "BW_CONFIG="+filepath.Join(dir, ".bw"), - "GOCOVERDIR="+bwCoverDir, - ) + cmd.Env = bwTestEnv(t, dir) out, err := cmd.CombinedOutput() if err == nil { t.Fatalf("expected bw %s to fail, got: %s", strings.Join(args, " "), out) @@ -440,7 +439,7 @@ func TestHelpToStdout(t *testing.T) { // bw --help should exit 0 and write to stdout (not stderr) cmd := exec.Command(bwBin, "--help") cmd.Dir = env.Dir - cmd.Env = append(os.Environ(), "GOCOVERDIR="+bwCoverDir) + cmd.Env = bwTestEnv(t, env.Dir) var stdout, stderr strings.Builder cmd.Stdout = &stdout cmd.Stderr = &stderr @@ -462,7 +461,7 @@ func TestCommandHelpToStdout(t *testing.T) { // bw create --help should exit 0 and write to stdout cmd := exec.Command(bwBin, "create", "--help") cmd.Dir = env.Dir - cmd.Env = append(os.Environ(), "GOCOVERDIR="+bwCoverDir) + cmd.Env = bwTestEnv(t, env.Dir) var stdout, stderr strings.Builder cmd.Stdout = &stdout cmd.Stderr = &stderr diff --git a/cmd/bw/command.go b/cmd/bw/command.go index 0e156df1..03b39fcd 100644 --- a/cmd/bw/command.go +++ b/cmd/bw/command.go @@ -473,6 +473,21 @@ var commands = []Command{ NeedsStore: true, Run: cmdPrime, }, + { + Name: "registry", + Summary: "Manage the repository registry", + Description: "View and manage the host-local repository registry.\nSubcommands: list, prune. Use bw registry --help for details.", + Positionals: []Positional{ + {Name: "list|prune", Required: true, Help: "Subcommand"}, + }, + Examples: []Example{ + {Cmd: "bw registry list", Help: "Show all registered repos"}, + {Cmd: "bw registry list --json", Help: "JSON output"}, + {Cmd: "bw registry prune", Help: "Remove entries for deleted repos"}, + {Cmd: "bw registry prune --yes", Help: "Skip confirmation"}, + }, + Run: cmdRegistry, + }, } // wrapNoArgs adapts a func(Writer) error to the standard command signature. @@ -504,6 +519,7 @@ var commandGroups = []struct { {"Finding Work", []string{"ready", "blocked"}}, {"Dependencies", []string{"dep"}}, {"Sync & Data", []string{"sync", "export", "import"}}, + {"Cross-Repo & Activity", []string{"registry"}}, {"Setup & Config", []string{"init", "config", "upgrade", "onboard", "prime"}}, } diff --git a/cmd/bw/helpers.go b/cmd/bw/helpers.go index e619d67c..70b22d45 100644 --- a/cmd/bw/helpers.go +++ b/cmd/bw/helpers.go @@ -102,6 +102,7 @@ var aliases = map[string]string{ "-s": "--status", "-l": "--labels", "-g": "--grep", + "-y": "--yes", } // Args holds parsed command-line arguments separated into boolean flags, diff --git a/cmd/bw/main.go b/cmd/bw/main.go index 57925bfa..e32634a4 100644 --- a/cmd/bw/main.go +++ b/cmd/bw/main.go @@ -4,9 +4,13 @@ import ( "fmt" "os" "path/filepath" + "sync" + "time" "github.com/jallum/beadwork/internal/config" "github.com/jallum/beadwork/internal/issue" + "github.com/jallum/beadwork/internal/registry" + "github.com/jallum/beadwork/internal/repo" "golang.org/x/term" ) @@ -105,6 +109,46 @@ func main() { fatal(err.Error()) } } + + touchRegistry(bwNow()) +} + +var registryOnce sync.Once + +func touchRegistry(now time.Time) { + r, err := repo.FindRepoAt(repoDir) + if err != nil { + return + } + if !r.IsInitialized() { + return + } + repoPath, err := registry.CanonicalRepoPath(r.RepoDir()) + if err != nil { + return + } + dir := registry.DefaultDir() + reg, err := registry.Load(dir) + if err != nil { + registryOnce.Do(func() { + fmt.Fprintf(os.Stderr, "warning: could not load registry: %v\n", err) + }) + return + } + if err := reg.TouchAndSave(repoPath, now); err != nil { + registryOnce.Do(func() { + fmt.Fprintf(os.Stderr, "warning: could not save registry: %v\n", err) + }) + } +} + +func bwNow() time.Time { + if v := os.Getenv("BW_CLOCK"); v != "" { + if t, err := time.Parse(time.RFC3339, v); err == nil { + return t.UTC() + } + } + return time.Now().UTC() } // extractDirFlag removes all -C pairs from args and sets repoDir. diff --git a/cmd/bw/registry.go b/cmd/bw/registry.go new file mode 100644 index 00000000..40a1e697 --- /dev/null +++ b/cmd/bw/registry.go @@ -0,0 +1,224 @@ +package main + +import ( + "github.com/jallum/beadwork/internal/config" + + "encoding/json" + "fmt" + "os" + "sort" + "strings" + + "github.com/jallum/beadwork/internal/issue" + "github.com/jallum/beadwork/internal/registry" + "github.com/jallum/beadwork/internal/repo" + "golang.org/x/term" +) + +// registrySubcommands holds the dispatch table for `bw registry `. +var registrySubcommands = map[string]struct { + summary string + run func([]string, Writer) error +}{ + "list": {"List registered repositories", cmdRegistryList}, + "prune": {"Remove stale registry entries", cmdRegistryPrune}, +} + +func cmdRegistry(_ *issue.Store, args []string, w Writer, _ *config.Config) (*config.Config, error) { + if len(args) == 0 { + return nil, printRegistryHelp(w) + } + + sub := args[0] + if sub == "--help" || sub == "-h" { + return nil, printRegistryHelp(w) + } + + entry, ok := registrySubcommands[sub] + if !ok { + fmt.Fprintf(os.Stderr, "unknown registry subcommand: %s\n", sub) + return nil, printRegistryHelp(w) + } + + subArgs := args[1:] + if hasFlag(subArgs, "--help") || hasFlag(subArgs, "-h") { + return nil, printRegistrySubHelp(w, sub, entry.summary) + } + + return nil, entry.run(subArgs, w) +} + +func printRegistryHelp(w Writer) error { + fmt.Fprintln(w, "Manage the beadwork repository registry.") + fmt.Fprintf(w, "\n%s\n", w.Style("Usage:", Cyan)) + w.Push(2) + fmt.Fprintln(w, "bw registry [flags]") + w.Pop() + fmt.Fprintf(w, "\n%s\n", w.Style("Subcommands:", Cyan)) + w.Push(2) + fmt.Fprintf(w, "%-20s %s\n", "list", "List registered repositories") + fmt.Fprintf(w, "%-20s %s\n", "prune", "Remove stale registry entries") + w.Pop() + return nil +} + +func printRegistrySubHelp(w Writer, name, summary string) error { + fmt.Fprintln(w, summary) + fmt.Fprintf(w, "\n%s\n", w.Style("Usage:", Cyan)) + w.Push(2) + fmt.Fprintf(w, "bw registry %s [flags]\n", name) + w.Pop() + return nil +} + +type registryListEntry struct { + Path string `json:"path"` + Prefix string `json:"prefix,omitempty"` + LastSeenAt string `json:"last_seen_at"` + Missing bool `json:"missing,omitempty"` +} + +func cmdRegistryList(args []string, w Writer) error { + a, err := ParseArgs(args, nil, []string{"--json"}) + if err != nil { + return err + } + + dir := registry.DefaultDir() + reg, err := registry.Load(dir) + if err != nil { + return fmt.Errorf("load registry: %w", err) + } + + entries := reg.Entries() + if len(entries) == 0 { + if a.JSON() { + fmt.Fprintln(w, "[]") + } else { + fmt.Fprintln(w, "no registered repositories") + } + return nil + } + + // Build sorted list with live prefix read and missing detection. + var list []registryListEntry + for path, e := range entries { + le := registryListEntry{ + Path: path, + LastSeenAt: e.LastSeenAt, + Prefix: e.Prefix, + } + // Check if the repo still exists and try to read its prefix. + if _, err := os.Stat(path); err != nil { + le.Missing = true + } else if le.Prefix == "" { + if r, err := repo.FindRepoAt(path); err == nil && r.IsInitialized() { + le.Prefix = r.Prefix + } + } + list = append(list, le) + } + + sort.Slice(list, func(i, j int) bool { + return list[i].Path < list[j].Path + }) + + if a.JSON() { + data, _ := json.MarshalIndent(list, "", " ") + fmt.Fprintln(w, string(data)) + return nil + } + + for _, le := range list { + prefix := le.Prefix + if prefix == "" { + prefix = "?" + } + age := relativeTime(le.LastSeenAt) + line := fmt.Sprintf("[%s] %s (%s)", prefix, le.Path, age) + if le.Missing { + line += " " + w.Style("MISSING", Red) + } + fmt.Fprintln(w, line) + } + return nil +} + +func cmdRegistryPrune(args []string, w Writer) error { + a, err := ParseArgs(args, nil, []string{"--yes", "-y"}) + if err != nil { + return err + } + + force := a.Bool("--yes") || a.Bool("-y") + + dir := registry.DefaultDir() + reg, err := registry.Load(dir) + if err != nil { + return fmt.Errorf("load registry: %w", err) + } + + entries := reg.Entries() + if len(entries) == 0 { + fmt.Fprintln(w, "registry is empty, nothing to prune") + return nil + } + + // Find missing entries. + var missing []string + for path := range entries { + if _, err := os.Stat(path); err != nil { + missing = append(missing, path) + } + } + sort.Strings(missing) + + if len(missing) == 0 { + fmt.Fprintln(w, "all registered repos exist, nothing to prune") + return nil + } + + // Half-removal warning. + if len(missing) > len(entries)/2 { + fmt.Fprintf(w, "Warning: %d of %d entries would be removed (more than half).\n", + len(missing), len(entries)) + } + + fmt.Fprintf(w, "Found %d missing repo(s):\n", len(missing)) + w.Push(2) + for _, p := range missing { + fmt.Fprintln(w, p) + } + w.Pop() + + if !force { + // Check if stdin is a TTY for interactive confirmation. + if !term.IsTerminal(int(os.Stdin.Fd())) { + return fmt.Errorf("non-interactive: pass --yes to confirm") + } + fmt.Fprint(w, "\nRemove these entries? [y/N] ") + var response string + fmt.Scanln(&response) + response = strings.TrimSpace(strings.ToLower(response)) + if response != "y" && response != "yes" { + fmt.Fprintln(w, "aborted") + return nil + } + } + + missingSet := make(map[string]bool, len(missing)) + for _, p := range missing { + missingSet[p] = true + } + removed := reg.Prune(func(path string, _ registry.Entry) bool { + return missingSet[path] + }) + + if err := reg.Save(); err != nil { + return fmt.Errorf("save registry: %w", err) + } + + fmt.Fprintf(w, "pruned %d entries\n", len(removed)) + + return nil +} diff --git a/test/acceptance_test.go b/test/acceptance_test.go index 1b20b35a..58669796 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -258,10 +258,12 @@ func TestScaffoldingHelpers(t *testing.T) { t.Errorf("registryContents() = %q, want %q", got, `{"repos":{}}`) } - // registryContents on empty dir returns "" + // registryContents on a fresh env returns content from auto-registration + // (bw init triggers touchRegistry). env2 := newBwEnv(t) - if c := env2.registryContents(); c != "" { - t.Errorf("registryContents() on fresh env = %q, want empty", c) + c := env2.registryContents() + if !strings.Contains(c, "last_seen_at") { + t.Errorf("registryContents() on fresh env missing auto-reg data: %q", c) } // newMultiRepoEnv creates n envs sharing a registry dir @@ -328,6 +330,195 @@ func TestGoldenBasicScenario(t *testing.T) { } } +// TestAutoRegistrationOnAnyCommand verifies that running any bw command +// in an initialized repo creates a registry entry. +func TestAutoRegistrationOnAnyCommand(t *testing.T) { + env := newBwEnv(t) + + // Even a read-only command should register. + env.bw("list") + + got := env.registryContents() + if got == "" { + t.Fatal("registry file not created after bw list") + } + if !strings.Contains(got, env.dir) { + t.Errorf("registry does not contain repo path %q:\n%s", env.dir, got) + } + if !strings.Contains(got, "2026-01-15T10:00:00Z") { + t.Errorf("registry last_seen_at does not reflect BW_CLOCK:\n%s", got) + } +} + +// TestAutoRegFiresForReadOnlyCommands verifies registration happens even +// for commands that don't modify state. +func TestAutoRegFiresForReadOnlyCommands(t *testing.T) { + env := newBwEnv(t) + env.bw("ready") + + got := env.registryContents() + if got == "" { + t.Fatal("registry file not created after bw ready") + } +} + +// TestAutoRegistrationSilentFailure verifies that if the registry dir is +// unwritable, bw still runs the command successfully. +func TestAutoRegistrationSilentFailure(t *testing.T) { + dir := t.TempDir() + env := &bwEnv{ + t: t, + dir: dir, + registryDir: "/nonexistent/path/that/should/fail", + env: append(os.Environ(), + "BW_CLOCK="+fixedClock, + "GIT_AUTHOR_DATE="+fixedClock, + "GIT_COMMITTER_DATE="+fixedClock, + "NO_COLOR=1", + "BEADWORK_HOME=/nonexistent/path/that/should/fail", + ), + } + env.git("init") + env.git("config", "user.email", "test@beadwork.dev") + env.git("config", "user.name", "Test User") + os.WriteFile(filepath.Join(dir, "README"), []byte("test"), 0644) + env.git("add", ".") + env.git("commit", "-m", "initial") + env.bw("init", "--prefix", "test") + + // Should succeed despite unwritable registry dir. + out := env.bw("list") + _ = out +} + +// TestWorktreeRegistersSameAsMain verifies that running bw from a worktree +// registers the main repo path, not the worktree path. +func TestWorktreeRegistersSameAsMain(t *testing.T) { + env := newBwEnv(t) + + // Create a worktree. + wtDir := filepath.Join(filepath.Dir(env.dir), "worktree") + env.git("worktree", "add", wtDir, "-b", "wt-branch") + t.Cleanup(func() { + env.git("worktree", "remove", "--force", wtDir) + }) + + // Run bw from the worktree. + env.bwAt(wtDir, "list") + + got := env.registryContents() + if got == "" { + t.Fatal("registry not created after bw list from worktree") + } + // Should contain the main repo dir, not the worktree dir. + if strings.Contains(got, wtDir) { + t.Errorf("registry should not contain worktree path %q:\n%s", wtDir, got) + } +} + +// TestRegistryList verifies the registry list command shows registered repos. +func TestRegistryList(t *testing.T) { + env := newBwEnv(t) + env.bw("list") // trigger auto-registration + + out := env.bw("registry", "list") + if !strings.Contains(out, env.dir) { + t.Errorf("registry list missing repo dir:\n%s", out) + } +} + +// TestRegistryListJSON verifies JSON output format. +func TestRegistryListJSON(t *testing.T) { + env := newBwEnv(t) + env.bw("list") // trigger registration + + out := env.bw("registry", "list", "--json") + if !strings.Contains(out, `"path"`) { + t.Errorf("registry list --json missing 'path' key:\n%s", out) + } +} + +// TestRegistryListMissing verifies that deleted repos are flagged as MISSING. +func TestRegistryListMissing(t *testing.T) { + env := newBwEnv(t) + // Seed a registry entry for a nonexistent path. + env.seedRegistry(`{"schema_version":1,"repos":{"/nonexistent/repo":{"last_seen_at":"2026-01-15T10:00:00Z"}}}`) + + out := env.bw("registry", "list") + if !strings.Contains(out, "MISSING") { + t.Errorf("registry list should show MISSING for nonexistent path:\n%s", out) + } +} + +// TestRegistryPruneYes verifies that prune --yes removes missing entries. +func TestRegistryPruneYes(t *testing.T) { + env := newBwEnv(t) + env.seedRegistry(`{"schema_version":1,"repos":{"/nonexistent/repo":{"last_seen_at":"2026-01-15T10:00:00Z"}}}`) + + out := env.bw("registry", "prune", "--yes") + if !strings.Contains(out, "pruned 1") { + t.Errorf("expected 'pruned 1' in output:\n%s", out) + } + + // Verify it's actually gone. + contents := env.registryContents() + if strings.Contains(contents, "/nonexistent/repo") { + t.Errorf("pruned entry still in registry:\n%s", contents) + } +} + +// TestRegistryPruneNonTTY verifies prune refuses without --yes in non-TTY. +func TestRegistryPruneNonTTY(t *testing.T) { + env := newBwEnv(t) + env.seedRegistry(`{"schema_version":1,"repos":{"/nonexistent/repo":{"last_seen_at":"2026-01-15T10:00:00Z"}}}`) + + out := env.bwFail("registry", "prune") + if !strings.Contains(out, "non-interactive") { + t.Errorf("expected non-interactive error:\n%s", out) + } +} + +// TestRegistryPruneHalfWarning verifies the half-removal warning. +func TestRegistryPruneHalfWarning(t *testing.T) { + env := newBwEnv(t) + // 3 out of 4 missing (real repo auto-registered = 1 existing). + env.seedRegistry(`{"schema_version":1,"repos":{"/missing1":{"last_seen_at":"2026-01-15T10:00:00Z"},"/missing2":{"last_seen_at":"2026-01-15T10:00:00Z"},"/missing3":{"last_seen_at":"2026-01-15T10:00:00Z"},"` + env.dir + `":{"last_seen_at":"2026-01-15T10:00:00Z"}}}`) + + out := env.bw("registry", "prune", "--yes") + if !strings.Contains(out, "more than half") { + t.Errorf("expected half-removal warning:\n%s", out) + } +} + +// TestRegistryPruneShortFlag verifies -y works as shorthand for --yes. +func TestRegistryPruneShortFlag(t *testing.T) { + env := newBwEnv(t) + env.seedRegistry(`{"schema_version":1,"repos":{"/nonexistent/repo":{"last_seen_at":"2026-01-15T10:00:00Z"}}}`) + + out := env.bw("registry", "prune", "-y") + if !strings.Contains(out, "pruned 1") { + t.Errorf("expected 'pruned 1' with -y:\n%s", out) + } +} + +// TestRegistryHelp verifies the help output. +func TestRegistryHelp(t *testing.T) { + env := newBwEnv(t) + out := env.bw("registry", "--help") + if !strings.Contains(out, "list") || !strings.Contains(out, "prune") { + t.Errorf("registry help missing subcommands:\n%s", out) + } +} + +// TestRegistryInBwHelp verifies registry appears in top-level help. +func TestRegistryInBwHelp(t *testing.T) { + env := newBwEnv(t) + out := env.bw("--help") + if !strings.Contains(out, "registry") { + t.Errorf("bw --help missing registry:\n%s", out) + } +} + // TestWorktreeRefWrites verifies that bw operations run from inside a git // worktree write refs to the shared git dir, so tickets are visible from // the main checkout. From 1f15c3aa3a4c67609588f6e617616fff5cc151e3 Mon Sep 17 00:00:00 2001 From: Jason Allum <21417+jallum@users.noreply.github.com> Date: Fri, 24 Apr 2026 07:37:30 -0400 Subject: [PATCH 03/19] Remove Prefix from registry Entry; read live from repo The registry should be a path+timestamp index, not a cache of repo metadata. Always read the prefix from .bwconfig via the repo package to eliminate drift between the two sources. --- cmd/bw/registry.go | 9 ++------- internal/registry/registry.go | 1 - 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/cmd/bw/registry.go b/cmd/bw/registry.go index 40a1e697..3761be1d 100644 --- a/cmd/bw/registry.go +++ b/cmd/bw/registry.go @@ -100,21 +100,16 @@ func cmdRegistryList(args []string, w Writer) error { return nil } - // Build sorted list with live prefix read and missing detection. var list []registryListEntry for path, e := range entries { le := registryListEntry{ Path: path, LastSeenAt: e.LastSeenAt, - Prefix: e.Prefix, } - // Check if the repo still exists and try to read its prefix. if _, err := os.Stat(path); err != nil { le.Missing = true - } else if le.Prefix == "" { - if r, err := repo.FindRepoAt(path); err == nil && r.IsInitialized() { - le.Prefix = r.Prefix - } + } else if r, err := repo.FindRepoAt(path); err == nil && r.IsInitialized() { + le.Prefix = r.Prefix } list = append(list, le) } diff --git a/internal/registry/registry.go b/internal/registry/registry.go index 095a1bb5..25b32dfa 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -21,7 +21,6 @@ const registryFile = "registry.json" type Entry struct { LastSeenAt string `json:"last_seen_at"` Cursor string `json:"cursor,omitempty"` - Prefix string `json:"prefix,omitempty"` } // Registry holds the in-memory state of the registry file. From 9a76f56307a785c72efec1e04e20ebd14598c545 Mon Sep 17 00:00:00 2001 From: Jason Allum <21417+jallum@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:18:29 -0400 Subject: [PATCH 04/19] Collapse registry to plain text path list (~/.bw) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the JSON-based registry (schema version, Entry structs with timestamps/cursors) with a simple text file containing one repo path per line. The registry is now a pure path index — no metadata caching. - DefaultPath() returns ~/.bw (or $BEADWORK_HOME) as a file, not dir - Registry API: Load/Save/Add/Remove/Paths - touchRegistry() just calls reg.Add(path) - All tests updated for new plain-text format --- cmd/bw/cli_test.go | 2 +- cmd/bw/main.go | 24 +--- cmd/bw/registry.go | 55 +++----- internal/registry/dir.go | 23 ++-- internal/registry/dir_test.go | 49 ++----- internal/registry/registry.go | 177 ++++++++----------------- internal/registry/registry_test.go | 202 ++++++++++------------------- test/acceptance_test.go | 84 +++++------- 8 files changed, 193 insertions(+), 423 deletions(-) diff --git a/cmd/bw/cli_test.go b/cmd/bw/cli_test.go index 27b66543..0730d264 100644 --- a/cmd/bw/cli_test.go +++ b/cmd/bw/cli_test.go @@ -82,7 +82,7 @@ func bwTestEnv(t *testing.T, dir string) []string { "GIT_COMMITTER_NAME=Test", "GIT_COMMITTER_EMAIL=test@test.com", "BW_CONFIG="+filepath.Join(dir, ".bw"), - "BEADWORK_HOME="+t.TempDir(), + "BEADWORK_HOME="+filepath.Join(t.TempDir(), ".bw"), "GOCOVERDIR="+bwCoverDir, ) } diff --git a/cmd/bw/main.go b/cmd/bw/main.go index e32634a4..7930b1a7 100644 --- a/cmd/bw/main.go +++ b/cmd/bw/main.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" "sync" - "time" "github.com/jallum/beadwork/internal/config" "github.com/jallum/beadwork/internal/issue" @@ -110,47 +109,34 @@ func main() { } } - touchRegistry(bwNow()) + touchRegistry() } var registryOnce sync.Once -func touchRegistry(now time.Time) { +func touchRegistry() { r, err := repo.FindRepoAt(repoDir) - if err != nil { - return - } - if !r.IsInitialized() { + if err != nil || !r.IsInitialized() { return } repoPath, err := registry.CanonicalRepoPath(r.RepoDir()) if err != nil { return } - dir := registry.DefaultDir() - reg, err := registry.Load(dir) + reg, err := registry.Load(registry.DefaultPath()) if err != nil { registryOnce.Do(func() { fmt.Fprintf(os.Stderr, "warning: could not load registry: %v\n", err) }) return } - if err := reg.TouchAndSave(repoPath, now); err != nil { + if err := reg.Add(repoPath); err != nil { registryOnce.Do(func() { fmt.Fprintf(os.Stderr, "warning: could not save registry: %v\n", err) }) } } -func bwNow() time.Time { - if v := os.Getenv("BW_CLOCK"); v != "" { - if t, err := time.Parse(time.RFC3339, v); err == nil { - return t.UTC() - } - } - return time.Now().UTC() -} - // extractDirFlag removes all -C pairs from args and sets repoDir. func extractDirFlag(args []string) []string { out := make([]string, 0, len(args)) diff --git a/cmd/bw/registry.go b/cmd/bw/registry.go index 3761be1d..90eb3fad 100644 --- a/cmd/bw/registry.go +++ b/cmd/bw/registry.go @@ -15,7 +15,6 @@ import ( "golang.org/x/term" ) -// registrySubcommands holds the dispatch table for `bw registry `. var registrySubcommands = map[string]struct { summary string run func([]string, Writer) error @@ -72,10 +71,9 @@ func printRegistrySubHelp(w Writer, name, summary string) error { } type registryListEntry struct { - Path string `json:"path"` - Prefix string `json:"prefix,omitempty"` - LastSeenAt string `json:"last_seen_at"` - Missing bool `json:"missing,omitempty"` + Path string `json:"path"` + Prefix string `json:"prefix,omitempty"` + Missing bool `json:"missing,omitempty"` } func cmdRegistryList(args []string, w Writer) error { @@ -84,14 +82,13 @@ func cmdRegistryList(args []string, w Writer) error { return err } - dir := registry.DefaultDir() - reg, err := registry.Load(dir) + reg, err := registry.Load(registry.DefaultPath()) if err != nil { return fmt.Errorf("load registry: %w", err) } - entries := reg.Entries() - if len(entries) == 0 { + paths := reg.Paths() + if len(paths) == 0 { if a.JSON() { fmt.Fprintln(w, "[]") } else { @@ -101,11 +98,8 @@ func cmdRegistryList(args []string, w Writer) error { } var list []registryListEntry - for path, e := range entries { - le := registryListEntry{ - Path: path, - LastSeenAt: e.LastSeenAt, - } + for _, path := range paths { + le := registryListEntry{Path: path} if _, err := os.Stat(path); err != nil { le.Missing = true } else if r, err := repo.FindRepoAt(path); err == nil && r.IsInitialized() { @@ -114,10 +108,6 @@ func cmdRegistryList(args []string, w Writer) error { list = append(list, le) } - sort.Slice(list, func(i, j int) bool { - return list[i].Path < list[j].Path - }) - if a.JSON() { data, _ := json.MarshalIndent(list, "", " ") fmt.Fprintln(w, string(data)) @@ -129,8 +119,7 @@ func cmdRegistryList(args []string, w Writer) error { if prefix == "" { prefix = "?" } - age := relativeTime(le.LastSeenAt) - line := fmt.Sprintf("[%s] %s (%s)", prefix, le.Path, age) + line := fmt.Sprintf("[%s] %s", prefix, le.Path) if le.Missing { line += " " + w.Style("MISSING", Red) } @@ -147,21 +136,19 @@ func cmdRegistryPrune(args []string, w Writer) error { force := a.Bool("--yes") || a.Bool("-y") - dir := registry.DefaultDir() - reg, err := registry.Load(dir) + reg, err := registry.Load(registry.DefaultPath()) if err != nil { return fmt.Errorf("load registry: %w", err) } - entries := reg.Entries() - if len(entries) == 0 { + paths := reg.Paths() + if len(paths) == 0 { fmt.Fprintln(w, "registry is empty, nothing to prune") return nil } - // Find missing entries. var missing []string - for path := range entries { + for _, path := range paths { if _, err := os.Stat(path); err != nil { missing = append(missing, path) } @@ -173,10 +160,9 @@ func cmdRegistryPrune(args []string, w Writer) error { return nil } - // Half-removal warning. - if len(missing) > len(entries)/2 { + if len(missing) > len(paths)/2 { fmt.Fprintf(w, "Warning: %d of %d entries would be removed (more than half).\n", - len(missing), len(entries)) + len(missing), len(paths)) } fmt.Fprintf(w, "Found %d missing repo(s):\n", len(missing)) @@ -187,7 +173,6 @@ func cmdRegistryPrune(args []string, w Writer) error { w.Pop() if !force { - // Check if stdin is a TTY for interactive confirmation. if !term.IsTerminal(int(os.Stdin.Fd())) { return fmt.Errorf("non-interactive: pass --yes to confirm") } @@ -201,19 +186,13 @@ func cmdRegistryPrune(args []string, w Writer) error { } } - missingSet := make(map[string]bool, len(missing)) for _, p := range missing { - missingSet[p] = true + reg.Remove(p) } - removed := reg.Prune(func(path string, _ registry.Entry) bool { - return missingSet[path] - }) - if err := reg.Save(); err != nil { return fmt.Errorf("save registry: %w", err) } - fmt.Fprintf(w, "pruned %d entries\n", len(removed)) - + fmt.Fprintf(w, "pruned %d entries\n", len(missing)) return nil } diff --git a/internal/registry/dir.go b/internal/registry/dir.go index 309460cc..d4bb64be 100644 --- a/internal/registry/dir.go +++ b/internal/registry/dir.go @@ -5,22 +5,17 @@ import ( "path/filepath" ) -const dirName = ".beadwork" +const defaultFileName = ".bw" -// DefaultDir returns the beadwork home directory. BEADWORK_HOME overrides -// it; otherwise it falls back to ~/.beadwork. os.UserHomeDir honors $HOME, -// so callers who want a different home can point HOME at it. -func DefaultDir() string { - return resolveFrom(os.Getenv("BEADWORK_HOME"), os.UserHomeDir) -} - -func resolveFrom(envHome string, homeFn func() (string, error)) string { - if envHome != "" { - return envHome +// DefaultPath returns the path to the registry file. +// BEADWORK_HOME overrides it; otherwise it falls back to ~/.bw. +func DefaultPath() string { + if v := os.Getenv("BEADWORK_HOME"); v != "" { + return v } - home, err := homeFn() + home, err := os.UserHomeDir() if err != nil { - return dirName + return defaultFileName } - return filepath.Join(home, dirName) + return filepath.Join(home, defaultFileName) } diff --git a/internal/registry/dir_test.go b/internal/registry/dir_test.go index 98d43cef..3e775ba1 100644 --- a/internal/registry/dir_test.go +++ b/internal/registry/dir_test.go @@ -1,49 +1,20 @@ package registry import ( - "errors" - "path/filepath" "testing" ) -func TestResolveFrom(t *testing.T) { - homeDir := "/home/testuser" - homeFn := func() (string, error) { return homeDir, nil } - errHome := func() (string, error) { return "", errors.New("no home") } - - tests := []struct { - name string - envHome string - homeFn func() (string, error) - want string - }{ - { - name: "BEADWORK_HOME takes precedence", - envHome: "/custom/beadwork", - want: "/custom/beadwork", - }, - { - name: "falls back to ~/.beadwork", - homeFn: homeFn, - want: filepath.Join(homeDir, ".beadwork"), - }, - { - name: "home dir error uses relative path", - homeFn: errHome, - want: ".beadwork", - }, +func TestDefaultPathRespectsEnv(t *testing.T) { + t.Setenv("BEADWORK_HOME", "/custom/path") + if got := DefaultPath(); got != "/custom/path" { + t.Errorf("DefaultPath() = %q, want /custom/path", got) } +} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - fn := tt.homeFn - if fn == nil { - fn = homeFn - } - got := resolveFrom(tt.envHome, fn) - if got != tt.want { - t.Errorf("resolveFrom() = %q, want %q", got, tt.want) - } - }) +func TestDefaultPathFallsBackToHome(t *testing.T) { + t.Setenv("BEADWORK_HOME", "") + got := DefaultPath() + if got == "" || got == defaultFileName { + t.Errorf("DefaultPath() = %q, expected a home-based path", got) } } diff --git a/internal/registry/registry.go b/internal/registry/registry.go index 25b32dfa..072f1de4 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -1,89 +1,56 @@ // Package registry tracks which repositories on this host use beadwork. -// The registry is a single JSON file stored under the beadwork home -// directory (~/.beadwork by default, or $BEADWORK_HOME). It records the -// last time each repo was seen and an opaque cursor for incremental -// recap processing. +// The registry is a plain text file (one absolute path per line) at +// ~/.bw by default, overridden by $BEADWORK_HOME. package registry import ( - "encoding/json" + "bufio" "fmt" "os" "path/filepath" + "sort" + "strings" "sync" - "time" ) -const SchemaVersion = 1 -const registryFile = "registry.json" - -// Entry represents a single tracked repository. -type Entry struct { - LastSeenAt string `json:"last_seen_at"` - Cursor string `json:"cursor,omitempty"` -} - -// Registry holds the in-memory state of the registry file. +// Registry holds the in-memory set of registered repo paths. type Registry struct { - SchemaVersion int `json:"schema_version"` - Repos map[string]Entry `json:"repos"` - - // extra preserves unknown top-level fields across load/save cycles. - extra map[string]json.RawMessage - - dir string // directory containing the registry file - mu sync.Mutex + paths map[string]bool + file string + mu sync.Mutex } -// Load reads the registry from dir. If the file does not exist, returns -// an empty registry. Returns an error if the file exists but the schema -// version is newer than this binary supports. -func Load(dir string) (*Registry, error) { +// Load reads the registry from file. If the file does not exist, returns +// an empty registry. +func Load(file string) (*Registry, error) { r := &Registry{ - SchemaVersion: SchemaVersion, - Repos: make(map[string]Entry), - dir: dir, + paths: make(map[string]bool), + file: file, } - path := filepath.Join(dir, registryFile) - data, err := os.ReadFile(path) + f, err := os.Open(file) if err != nil { if os.IsNotExist(err) { return r, nil } return nil, fmt.Errorf("read registry: %w", err) } + defer f.Close() - // Decode into a raw map first to preserve unknown fields. - var raw map[string]json.RawMessage - if err := json.Unmarshal(data, &raw); err != nil { - return nil, fmt.Errorf("parse registry: %w", err) - } - - // Extract known fields. - if v, ok := raw["schema_version"]; ok { - if err := json.Unmarshal(v, &r.SchemaVersion); err != nil { - return nil, fmt.Errorf("parse schema_version: %w", err) + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line != "" { + r.paths[line] = true } - delete(raw, "schema_version") - } - - if r.SchemaVersion > SchemaVersion { - return nil, fmt.Errorf("registry schema version %d is newer than supported (%d); upgrade bw", r.SchemaVersion, SchemaVersion) } - - if v, ok := raw["repos"]; ok { - if err := json.Unmarshal(v, &r.Repos); err != nil { - return nil, fmt.Errorf("parse repos: %w", err) - } - delete(raw, "repos") + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("read registry: %w", err) } - - r.extra = raw return r, nil } -// Save atomically writes the registry to disk using a temp-file + rename. +// Save atomically writes the registry to disk. func (r *Registry) Save() error { r.mu.Lock() defer r.mu.Unlock() @@ -91,97 +58,65 @@ func (r *Registry) Save() error { } func (r *Registry) saveLocked() error { - if err := os.MkdirAll(r.dir, 0755); err != nil { + dir := filepath.Dir(r.file) + if err := os.MkdirAll(dir, 0755); err != nil { return fmt.Errorf("create registry dir: %w", err) } - // Build the output map preserving unknown fields. - out := make(map[string]interface{}, len(r.extra)+2) - for k, v := range r.extra { - out[k] = v - } - out["schema_version"] = r.SchemaVersion - out["repos"] = r.Repos - - data, err := json.MarshalIndent(out, "", " ") - if err != nil { - return fmt.Errorf("marshal registry: %w", err) + paths := r.sortedPaths() + var b strings.Builder + for _, p := range paths { + b.WriteString(p) + b.WriteByte('\n') } - data = append(data, '\n') - path := filepath.Join(r.dir, registryFile) - tmp := path + ".tmp" - if err := os.WriteFile(tmp, data, 0644); err != nil { + tmp := r.file + ".tmp" + if err := os.WriteFile(tmp, []byte(b.String()), 0644); err != nil { return fmt.Errorf("write temp registry: %w", err) } - if err := os.Rename(tmp, path); err != nil { + if err := os.Rename(tmp, r.file); err != nil { os.Remove(tmp) return fmt.Errorf("rename registry: %w", err) } return nil } -// Touch registers or updates a repo entry with the given timestamp. -func (r *Registry) Touch(repoPath string, now time.Time) { +// Add registers a repo path and saves atomically. +func (r *Registry) Add(repoPath string) error { r.mu.Lock() defer r.mu.Unlock() - e := r.Repos[repoPath] - e.LastSeenAt = now.UTC().Format(time.RFC3339) - r.Repos[repoPath] = e -} - -// TouchAndSave is a convenience that calls Touch then Save. -func (r *Registry) TouchAndSave(repoPath string, now time.Time) error { - r.mu.Lock() - defer r.mu.Unlock() - - e := r.Repos[repoPath] - e.LastSeenAt = now.UTC().Format(time.RFC3339) - r.Repos[repoPath] = e - return r.saveLocked() -} - -// AdvanceCursorAndSave updates the cursor for a repo and saves atomically. -func (r *Registry) AdvanceCursorAndSave(repoPath, cursor string) error { - r.mu.Lock() - defer r.mu.Unlock() - - e := r.Repos[repoPath] - e.Cursor = cursor - r.Repos[repoPath] = e + if r.paths[repoPath] { + return nil + } + r.paths[repoPath] = true return r.saveLocked() } -// Prune removes entries for which the predicate returns true. -// Returns the list of removed repo paths. -func (r *Registry) Prune(predicate func(path string, e Entry) bool) []string { +// Remove deletes a repo path and saves atomically. +func (r *Registry) Remove(repoPath string) bool { r.mu.Lock() defer r.mu.Unlock() - var removed []string - for path, e := range r.Repos { - if predicate(path, e) { - delete(r.Repos, path) - removed = append(removed, path) - } + if !r.paths[repoPath] { + return false } - return removed + delete(r.paths, repoPath) + return true } -// Entries returns a snapshot of all registry entries. -func (r *Registry) Entries() map[string]Entry { +// Paths returns a sorted snapshot of all registered paths. +func (r *Registry) Paths() []string { r.mu.Lock() defer r.mu.Unlock() - - cp := make(map[string]Entry, len(r.Repos)) - for k, v := range r.Repos { - cp[k] = v - } - return cp + return r.sortedPaths() } -// Dir returns the directory where the registry file lives. -func (r *Registry) Dir() string { - return r.dir +func (r *Registry) sortedPaths() []string { + paths := make([]string, 0, len(r.paths)) + for p := range r.paths { + paths = append(paths, p) + } + sort.Strings(paths) + return paths } diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go index 9db8a6b1..4888a9d3 100644 --- a/internal/registry/registry_test.go +++ b/internal/registry/registry_test.go @@ -1,196 +1,124 @@ package registry import ( - "encoding/json" "os" "path/filepath" "sync" "testing" - "time" ) func TestLoadEmpty(t *testing.T) { - dir := t.TempDir() - r, err := Load(dir) + file := filepath.Join(t.TempDir(), "reg") + r, err := Load(file) if err != nil { t.Fatalf("Load: %v", err) } - if r.SchemaVersion != SchemaVersion { - t.Errorf("SchemaVersion = %d, want %d", r.SchemaVersion, SchemaVersion) - } - if len(r.Repos) != 0 { - t.Errorf("Repos = %v, want empty", r.Repos) + if len(r.Paths()) != 0 { + t.Errorf("Paths = %v, want empty", r.Paths()) } } func TestLoadSaveRoundTrip(t *testing.T) { - dir := t.TempDir() - r, err := Load(dir) + file := filepath.Join(t.TempDir(), "reg") + r, err := Load(file) if err != nil { t.Fatal(err) } - now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) - r.Touch("/home/user/project-a", now) - r.Touch("/home/user/project-b", now.Add(time.Hour)) - - if err := r.Save(); err != nil { - t.Fatalf("Save: %v", err) - } - - r2, err := Load(dir) - if err != nil { - t.Fatalf("Load after save: %v", err) - } - if len(r2.Repos) != 2 { - t.Fatalf("Repos count = %d, want 2", len(r2.Repos)) - } - - ea := r2.Repos["/home/user/project-a"] - if ea.LastSeenAt != "2026-01-15T10:00:00Z" { - t.Errorf("project-a LastSeenAt = %q", ea.LastSeenAt) - } -} - -func TestSchemaVersionNewerRefused(t *testing.T) { - dir := t.TempDir() - data := `{"schema_version": 999, "repos": {}}` - os.WriteFile(filepath.Join(dir, registryFile), []byte(data), 0644) - - _, err := Load(dir) - if err == nil { - t.Fatal("expected error for newer schema version") - } -} - -func TestUnknownFieldPreservation(t *testing.T) { - dir := t.TempDir() - original := `{"schema_version":1,"repos":{},"future_field":"hello"}` - os.WriteFile(filepath.Join(dir, registryFile), []byte(original), 0644) - - r, err := Load(dir) - if err != nil { + if err := r.Add("/home/user/project-a"); err != nil { t.Fatal(err) } - - now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) - r.Touch("/tmp/repo", now) - if err := r.Save(); err != nil { + if err := r.Add("/home/user/project-b"); err != nil { t.Fatal(err) } - data, err := os.ReadFile(filepath.Join(dir, registryFile)) + r2, err := Load(file) if err != nil { - t.Fatal(err) - } - - var raw map[string]json.RawMessage - json.Unmarshal(data, &raw) - if _, ok := raw["future_field"]; !ok { - t.Error("future_field not preserved after save") + t.Fatalf("Load after save: %v", err) } -} - -func TestTouchAndSave(t *testing.T) { - dir := t.TempDir() - r, _ := Load(dir) - - now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) - if err := r.TouchAndSave("/tmp/repo", now); err != nil { - t.Fatalf("TouchAndSave: %v", err) + paths := r2.Paths() + if len(paths) != 2 { + t.Fatalf("Paths count = %d, want 2", len(paths)) } - - r2, _ := Load(dir) - if len(r2.Repos) != 1 { - t.Fatalf("Repos count = %d, want 1", len(r2.Repos)) + if paths[0] != "/home/user/project-a" || paths[1] != "/home/user/project-b" { + t.Errorf("Paths = %v", paths) } } -func TestAdvanceCursorAndSave(t *testing.T) { - dir := t.TempDir() - r, _ := Load(dir) - - now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) - r.Touch("/tmp/repo", now) - r.Save() +func TestAddIdempotent(t *testing.T) { + file := filepath.Join(t.TempDir(), "reg") + r, _ := Load(file) - if err := r.AdvanceCursorAndSave("/tmp/repo", "abc123"); err != nil { - t.Fatalf("AdvanceCursorAndSave: %v", err) - } + r.Add("/repo") + r.Add("/repo") - r2, _ := Load(dir) - e := r2.Repos["/tmp/repo"] - if e.Cursor != "abc123" { - t.Errorf("Cursor = %q, want abc123", e.Cursor) - } - if e.LastSeenAt != "2026-01-15T10:00:00Z" { - t.Errorf("LastSeenAt lost after cursor advance: %q", e.LastSeenAt) + if len(r.Paths()) != 1 { + t.Errorf("Paths count = %d, want 1", len(r.Paths())) } } -func TestPrune(t *testing.T) { - dir := t.TempDir() - r, _ := Load(dir) - - now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) - r.Touch("/keep", now) - r.Touch("/remove-me", now.Add(-30*24*time.Hour)) +func TestRemove(t *testing.T) { + file := filepath.Join(t.TempDir(), "reg") + r, _ := Load(file) + r.Add("/keep") + r.Add("/remove-me") - cutoff := now.Add(-7 * 24 * time.Hour) - removed := r.Prune(func(path string, e Entry) bool { - t, err := time.Parse(time.RFC3339, e.LastSeenAt) - if err != nil { - return false - } - return t.Before(cutoff) - }) - - if len(removed) != 1 || removed[0] != "/remove-me" { - t.Errorf("Prune removed = %v, want [/remove-me]", removed) + if !r.Remove("/remove-me") { + t.Error("Remove returned false for existing path") + } + if r.Remove("/nonexistent") { + t.Error("Remove returned true for nonexistent path") } - if _, ok := r.Repos["/keep"]; !ok { - t.Error("/keep was incorrectly pruned") + if len(r.Paths()) != 1 { + t.Errorf("Paths count = %d, want 1", len(r.Paths())) } } -func TestConcurrentSave(t *testing.T) { - dir := t.TempDir() - r, _ := Load(dir) +func TestConcurrentAdd(t *testing.T) { + file := filepath.Join(t.TempDir(), "reg") + r, _ := Load(file) var wg sync.WaitGroup for i := range 20 { wg.Add(1) go func(n int) { defer wg.Done() - now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC).Add(time.Duration(n) * time.Second) - r.TouchAndSave("/repo/"+string(rune('a'+n)), now) + r.Add("/repo/" + string(rune('a'+n))) }(i) } wg.Wait() - r2, err := Load(dir) + r2, err := Load(file) if err != nil { - t.Fatalf("Load after concurrent saves: %v", err) + t.Fatalf("Load after concurrent adds: %v", err) + } + if len(r2.Paths()) != 20 { + t.Errorf("Paths count = %d, want 20", len(r2.Paths())) } - if len(r2.Repos) != 20 { - t.Errorf("Repos count = %d, want 20", len(r2.Repos)) +} + +func TestPathsSorted(t *testing.T) { + file := filepath.Join(t.TempDir(), "reg") + r, _ := Load(file) + r.Add("/z") + r.Add("/a") + r.Add("/m") + + paths := r.Paths() + if paths[0] != "/a" || paths[1] != "/m" || paths[2] != "/z" { + t.Errorf("Paths not sorted: %v", paths) } } -func TestEntries(t *testing.T) { - dir := t.TempDir() - r, _ := Load(dir) - now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) - r.Touch("/a", now) - r.Touch("/b", now) - - entries := r.Entries() - if len(entries) != 2 { - t.Errorf("Entries count = %d, want 2", len(entries)) - } - // Mutating the snapshot should not affect the original. - delete(entries, "/a") - if len(r.Repos) != 2 { - t.Error("mutating Entries() snapshot affected original") +func TestLoadIgnoresBlankLines(t *testing.T) { + file := filepath.Join(t.TempDir(), "reg") + os.WriteFile(file, []byte("/a\n\n \n/b\n"), 0644) + + r, err := Load(file) + if err != nil { + t.Fatal(err) + } + if len(r.Paths()) != 2 { + t.Errorf("Paths count = %d, want 2", len(r.Paths())) } } diff --git a/test/acceptance_test.go b/test/acceptance_test.go index 58669796..03df89c0 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -8,7 +8,6 @@ import ( "path/filepath" "strings" "testing" - "time" ) var bwBin string // path to built bw binary @@ -46,19 +45,19 @@ const fixedClock = "2026-01-15T10:00:00Z" func newBwEnv(t *testing.T) *bwEnv { t.Helper() dir := t.TempDir() - registryDir := t.TempDir() + registryFile := filepath.Join(t.TempDir(), ".bw") env := &bwEnv{ t: t, dir: dir, - registryDir: registryDir, + registryDir: registryFile, env: append(os.Environ(), "BW_CLOCK="+fixedClock, "BW_CONFIG="+filepath.Join(dir, ".bw"), "GIT_AUTHOR_DATE="+fixedClock, "GIT_COMMITTER_DATE="+fixedClock, "NO_COLOR=1", - "BEADWORK_HOME="+registryDir, + "BEADWORK_HOME="+registryFile, ), } @@ -173,20 +172,20 @@ func compareGolden(t *testing.T, name, got string) { // registry directory, simulating multiple repos registered in a single registry. func newMultiRepoEnv(t *testing.T, n int) []*bwEnv { t.Helper() - registryDir := t.TempDir() + registryFile := filepath.Join(t.TempDir(), ".bw") envs := make([]*bwEnv, n) for i := range n { dir := t.TempDir() env := &bwEnv{ t: t, dir: dir, - registryDir: registryDir, + registryDir: registryFile, env: append(os.Environ(), "BW_CLOCK="+fixedClock, "GIT_AUTHOR_DATE="+fixedClock, "GIT_COMMITTER_DATE="+fixedClock, "NO_COLOR=1", - "BEADWORK_HOME="+registryDir, + "BEADWORK_HOME="+registryFile, ), } env.git("init") @@ -201,13 +200,11 @@ func newMultiRepoEnv(t *testing.T, n int) []*bwEnv { return envs } -// seedRegistry writes raw JSON content to a file named "registry.json" in -// the env's registry directory. Use this to set up registry state before -// running commands that read the registry. -func (e *bwEnv) seedRegistry(content string) { +// seedRegistry writes paths (one per line) to the registry file. +func (e *bwEnv) seedRegistry(paths ...string) { e.t.Helper() - path := filepath.Join(e.registryDir, "registry.json") - if err := os.WriteFile(path, []byte(content), 0644); err != nil { + content := strings.Join(paths, "\n") + "\n" + if err := os.WriteFile(e.registryDir, []byte(content), 0644); err != nil { e.t.Fatalf("seedRegistry: %v", err) } } @@ -216,8 +213,7 @@ func (e *bwEnv) seedRegistry(content string) { // Returns an empty string if the file does not exist. func (e *bwEnv) registryContents() string { e.t.Helper() - path := filepath.Join(e.registryDir, "registry.json") - data, err := os.ReadFile(path) + data, err := os.ReadFile(e.registryDir) if err != nil { if os.IsNotExist(err) { return "" @@ -227,58 +223,41 @@ func (e *bwEnv) registryContents() string { return string(data) } -// bwNow returns the time that bw commands will use as "now", respecting BW_CLOCK. -// Panics if BW_CLOCK is set but not parseable (test setup error). -func bwNow() time.Time { - t, err := time.Parse(time.RFC3339, fixedClock) - if err != nil { - panic("fixedClock is not valid RFC3339: " + err.Error()) - } - return t.UTC() -} - // TestScaffoldingHelpers verifies that the test scaffolding helpers work correctly. func TestScaffoldingHelpers(t *testing.T) { - // bwNow should match fixedClock - now := bwNow() - if now.Format(time.RFC3339) != fixedClock { - t.Errorf("bwNow() = %v, want %v", now.Format(time.RFC3339), fixedClock) - } - - // newBwEnv should set up a registry dir + // newBwEnv should set up a registry file path env := newBwEnv(t) if env.registryDir == "" { t.Fatal("registryDir not set") } // seedRegistry + registryContents round-trip - env.seedRegistry(`{"repos":{}}`) + env.seedRegistry("/a", "/b") got := env.registryContents() - if got != `{"repos":{}}` { - t.Errorf("registryContents() = %q, want %q", got, `{"repos":{}}`) + if !strings.Contains(got, "/a") || !strings.Contains(got, "/b") { + t.Errorf("registryContents() = %q, want /a and /b", got) } // registryContents on a fresh env returns content from auto-registration // (bw init triggers touchRegistry). env2 := newBwEnv(t) c := env2.registryContents() - if !strings.Contains(c, "last_seen_at") { - t.Errorf("registryContents() on fresh env missing auto-reg data: %q", c) + if c == "" { + t.Errorf("registryContents() on fresh env is empty, expected auto-reg data") } - // newMultiRepoEnv creates n envs sharing a registry dir + // newMultiRepoEnv creates n envs sharing a registry file envs := newMultiRepoEnv(t, 3) if len(envs) != 3 { t.Fatalf("newMultiRepoEnv(3) returned %d envs", len(envs)) } - sharedDir := envs[0].registryDir + sharedFile := envs[0].registryDir for i, e := range envs { - if e.registryDir != sharedDir { - t.Errorf("env[%d].registryDir = %q, want %q (shared)", i, e.registryDir, sharedDir) + if e.registryDir != sharedFile { + t.Errorf("env[%d].registryDir = %q, want %q (shared)", i, e.registryDir, sharedFile) } - // Each env should be an independent bw repo out := e.bw("list") - _ = out // no issues yet, just verify it runs + _ = out } } @@ -345,9 +324,6 @@ func TestAutoRegistrationOnAnyCommand(t *testing.T) { if !strings.Contains(got, env.dir) { t.Errorf("registry does not contain repo path %q:\n%s", env.dir, got) } - if !strings.Contains(got, "2026-01-15T10:00:00Z") { - t.Errorf("registry last_seen_at does not reflect BW_CLOCK:\n%s", got) - } } // TestAutoRegFiresForReadOnlyCommands verifies registration happens even @@ -369,13 +345,13 @@ func TestAutoRegistrationSilentFailure(t *testing.T) { env := &bwEnv{ t: t, dir: dir, - registryDir: "/nonexistent/path/that/should/fail", + registryDir: "/nonexistent/path/that/should/fail/.bw", env: append(os.Environ(), "BW_CLOCK="+fixedClock, "GIT_AUTHOR_DATE="+fixedClock, "GIT_COMMITTER_DATE="+fixedClock, "NO_COLOR=1", - "BEADWORK_HOME=/nonexistent/path/that/should/fail", + "BEADWORK_HOME=/nonexistent/path/that/should/fail/.bw", ), } env.git("init") @@ -442,7 +418,7 @@ func TestRegistryListJSON(t *testing.T) { func TestRegistryListMissing(t *testing.T) { env := newBwEnv(t) // Seed a registry entry for a nonexistent path. - env.seedRegistry(`{"schema_version":1,"repos":{"/nonexistent/repo":{"last_seen_at":"2026-01-15T10:00:00Z"}}}`) + env.seedRegistry("/nonexistent/repo") out := env.bw("registry", "list") if !strings.Contains(out, "MISSING") { @@ -453,7 +429,7 @@ func TestRegistryListMissing(t *testing.T) { // TestRegistryPruneYes verifies that prune --yes removes missing entries. func TestRegistryPruneYes(t *testing.T) { env := newBwEnv(t) - env.seedRegistry(`{"schema_version":1,"repos":{"/nonexistent/repo":{"last_seen_at":"2026-01-15T10:00:00Z"}}}`) + env.seedRegistry("/nonexistent/repo") out := env.bw("registry", "prune", "--yes") if !strings.Contains(out, "pruned 1") { @@ -470,7 +446,7 @@ func TestRegistryPruneYes(t *testing.T) { // TestRegistryPruneNonTTY verifies prune refuses without --yes in non-TTY. func TestRegistryPruneNonTTY(t *testing.T) { env := newBwEnv(t) - env.seedRegistry(`{"schema_version":1,"repos":{"/nonexistent/repo":{"last_seen_at":"2026-01-15T10:00:00Z"}}}`) + env.seedRegistry("/nonexistent/repo") out := env.bwFail("registry", "prune") if !strings.Contains(out, "non-interactive") { @@ -481,8 +457,8 @@ func TestRegistryPruneNonTTY(t *testing.T) { // TestRegistryPruneHalfWarning verifies the half-removal warning. func TestRegistryPruneHalfWarning(t *testing.T) { env := newBwEnv(t) - // 3 out of 4 missing (real repo auto-registered = 1 existing). - env.seedRegistry(`{"schema_version":1,"repos":{"/missing1":{"last_seen_at":"2026-01-15T10:00:00Z"},"/missing2":{"last_seen_at":"2026-01-15T10:00:00Z"},"/missing3":{"last_seen_at":"2026-01-15T10:00:00Z"},"` + env.dir + `":{"last_seen_at":"2026-01-15T10:00:00Z"}}}`) + // 3 out of 4 missing (real repo dir = 1 existing). + env.seedRegistry("/missing1", "/missing2", "/missing3", env.dir) out := env.bw("registry", "prune", "--yes") if !strings.Contains(out, "more than half") { @@ -493,7 +469,7 @@ func TestRegistryPruneHalfWarning(t *testing.T) { // TestRegistryPruneShortFlag verifies -y works as shorthand for --yes. func TestRegistryPruneShortFlag(t *testing.T) { env := newBwEnv(t) - env.seedRegistry(`{"schema_version":1,"repos":{"/nonexistent/repo":{"last_seen_at":"2026-01-15T10:00:00Z"}}}`) + env.seedRegistry("/nonexistent/repo") out := env.bw("registry", "prune", "-y") if !strings.Contains(out, "pruned 1") { From 575b13eb7c84b2ffa748e4795df825780c150791 Mon Sep 17 00:00:00 2001 From: Jason Allum <21417+jallum@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:21:53 -0400 Subject: [PATCH 05/19] Rename BEADWORK_HOME to BW_REGISTRY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Matches the BW_CLOCK convention and accurately describes what the env var points to — a registry file, not a home directory. --- cmd/bw/cli_test.go | 2 +- internal/registry/dir.go | 4 ++-- internal/registry/dir_test.go | 4 ++-- internal/registry/registry.go | 2 +- test/acceptance_test.go | 8 +++++--- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/cmd/bw/cli_test.go b/cmd/bw/cli_test.go index 0730d264..0cf1b6eb 100644 --- a/cmd/bw/cli_test.go +++ b/cmd/bw/cli_test.go @@ -82,7 +82,7 @@ func bwTestEnv(t *testing.T, dir string) []string { "GIT_COMMITTER_NAME=Test", "GIT_COMMITTER_EMAIL=test@test.com", "BW_CONFIG="+filepath.Join(dir, ".bw"), - "BEADWORK_HOME="+filepath.Join(t.TempDir(), ".bw"), + "BW_REGISTRY="+filepath.Join(t.TempDir(), ".bw"), "GOCOVERDIR="+bwCoverDir, ) } diff --git a/internal/registry/dir.go b/internal/registry/dir.go index d4bb64be..74011dc6 100644 --- a/internal/registry/dir.go +++ b/internal/registry/dir.go @@ -8,9 +8,9 @@ import ( const defaultFileName = ".bw" // DefaultPath returns the path to the registry file. -// BEADWORK_HOME overrides it; otherwise it falls back to ~/.bw. +// BW_REGISTRY overrides it; otherwise it falls back to ~/.bw. func DefaultPath() string { - if v := os.Getenv("BEADWORK_HOME"); v != "" { + if v := os.Getenv("BW_REGISTRY"); v != "" { return v } home, err := os.UserHomeDir() diff --git a/internal/registry/dir_test.go b/internal/registry/dir_test.go index 3e775ba1..16a6cd34 100644 --- a/internal/registry/dir_test.go +++ b/internal/registry/dir_test.go @@ -5,14 +5,14 @@ import ( ) func TestDefaultPathRespectsEnv(t *testing.T) { - t.Setenv("BEADWORK_HOME", "/custom/path") + t.Setenv("BW_REGISTRY", "/custom/path") if got := DefaultPath(); got != "/custom/path" { t.Errorf("DefaultPath() = %q, want /custom/path", got) } } func TestDefaultPathFallsBackToHome(t *testing.T) { - t.Setenv("BEADWORK_HOME", "") + t.Setenv("BW_REGISTRY", "") got := DefaultPath() if got == "" || got == defaultFileName { t.Errorf("DefaultPath() = %q, expected a home-based path", got) diff --git a/internal/registry/registry.go b/internal/registry/registry.go index 072f1de4..e7f8b1c6 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -1,6 +1,6 @@ // Package registry tracks which repositories on this host use beadwork. // The registry is a plain text file (one absolute path per line) at -// ~/.bw by default, overridden by $BEADWORK_HOME. +// ~/.bw by default, overridden by $BW_REGISTRY. package registry import ( diff --git a/test/acceptance_test.go b/test/acceptance_test.go index 03df89c0..16b0a4ac 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -57,7 +57,7 @@ func newBwEnv(t *testing.T) *bwEnv { "GIT_AUTHOR_DATE="+fixedClock, "GIT_COMMITTER_DATE="+fixedClock, "NO_COLOR=1", - "BEADWORK_HOME="+registryFile, + "BW_REGISTRY="+registryFile, ), } @@ -182,10 +182,11 @@ func newMultiRepoEnv(t *testing.T, n int) []*bwEnv { registryDir: registryFile, env: append(os.Environ(), "BW_CLOCK="+fixedClock, + "BW_CONFIG="+filepath.Join(dir, ".bw-config"), "GIT_AUTHOR_DATE="+fixedClock, "GIT_COMMITTER_DATE="+fixedClock, "NO_COLOR=1", - "BEADWORK_HOME="+registryFile, + "BW_REGISTRY="+registryFile, ), } env.git("init") @@ -348,10 +349,11 @@ func TestAutoRegistrationSilentFailure(t *testing.T) { registryDir: "/nonexistent/path/that/should/fail/.bw", env: append(os.Environ(), "BW_CLOCK="+fixedClock, + "BW_CONFIG="+filepath.Join(dir, ".bw-config"), "GIT_AUTHOR_DATE="+fixedClock, "GIT_COMMITTER_DATE="+fixedClock, "NO_COLOR=1", - "BEADWORK_HOME=/nonexistent/path/that/should/fail/.bw", + "BW_REGISTRY=/nonexistent/path/that/should/fail/.bw", ), } env.git("init") From 980056f161ff049188bd37e8c146c909b3bc18ba Mon Sep 17 00:00:00 2001 From: Jason Allum <21417+jallum@users.noreply.github.com> Date: Fri, 24 Apr 2026 14:03:53 -0400 Subject: [PATCH 06/19] Remove redundant canonical path resolution from registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CanonicalRepoPath duplicated the worktree→main-repo resolution already done by repo.FindRepoAt. Since touchRegistry already has a *Repo from FindRepoAt, r.RepoDir() gives the canonical root directly. --- cmd/bw/main.go | 6 +- internal/registry/canonical.go | 73 ----------------------- internal/registry/canonical_test.go | 91 ----------------------------- 3 files changed, 1 insertion(+), 169 deletions(-) delete mode 100644 internal/registry/canonical.go delete mode 100644 internal/registry/canonical_test.go diff --git a/cmd/bw/main.go b/cmd/bw/main.go index 7930b1a7..4e228ad5 100644 --- a/cmd/bw/main.go +++ b/cmd/bw/main.go @@ -119,10 +119,6 @@ func touchRegistry() { if err != nil || !r.IsInitialized() { return } - repoPath, err := registry.CanonicalRepoPath(r.RepoDir()) - if err != nil { - return - } reg, err := registry.Load(registry.DefaultPath()) if err != nil { registryOnce.Do(func() { @@ -130,7 +126,7 @@ func touchRegistry() { }) return } - if err := reg.Add(repoPath); err != nil { + if err := reg.Add(r.RepoDir()); err != nil { registryOnce.Do(func() { fmt.Fprintf(os.Stderr, "warning: could not save registry: %v\n", err) }) diff --git a/internal/registry/canonical.go b/internal/registry/canonical.go deleted file mode 100644 index 7d6d34a9..00000000 --- a/internal/registry/canonical.go +++ /dev/null @@ -1,73 +0,0 @@ -package registry - -import ( - "os" - "path/filepath" - "strings" -) - -// CanonicalRepoPath resolves a directory to its main repository root. -// If dir is inside a git worktree, it follows the .git file → commondir -// chain to find the shared .git directory, then returns its parent. -// For normal repositories, it walks up looking for a .git directory. -func CanonicalRepoPath(dir string) (string, error) { - dir, err := filepath.Abs(dir) - if err != nil { - return "", err - } - return findMainRepo(dir) -} - -func findMainRepo(dir string) (string, error) { - cur := dir - for { - dotGit := filepath.Join(cur, ".git") - fi, err := os.Stat(dotGit) - if err == nil { - if fi.IsDir() { - return cur, nil - } - // Worktree: .git is a file pointing to the real git dir. - return resolveWorktreeRoot(dotGit) - } - parent := filepath.Dir(cur) - if parent == cur { - return dir, nil // not a git repo; return as-is - } - cur = parent - } -} - -func resolveWorktreeRoot(dotGitFile string) (string, error) { - data, err := os.ReadFile(dotGitFile) - if err != nil { - return "", err - } - line := strings.TrimSpace(string(data)) - if !strings.HasPrefix(line, "gitdir: ") { - return filepath.Dir(dotGitFile), nil - } - - gitdir := strings.TrimPrefix(line, "gitdir: ") - if !filepath.IsAbs(gitdir) { - gitdir = filepath.Join(filepath.Dir(dotGitFile), gitdir) - } - gitdir = filepath.Clean(gitdir) - - // Read commondir to find the shared .git directory. - cdData, err := os.ReadFile(filepath.Join(gitdir, "commondir")) - if err != nil { - return filepath.Dir(gitdir), nil - } - - commondir := strings.TrimSpace(string(cdData)) - if !filepath.IsAbs(commondir) { - commondir = filepath.Join(gitdir, commondir) - } - commondir, err = filepath.Abs(commondir) - if err != nil { - return "", err - } - // The repo root is the parent of the .git directory. - return filepath.Dir(commondir), nil -} diff --git a/internal/registry/canonical_test.go b/internal/registry/canonical_test.go deleted file mode 100644 index 336aa6d6..00000000 --- a/internal/registry/canonical_test.go +++ /dev/null @@ -1,91 +0,0 @@ -package registry - -import ( - "os" - "os/exec" - "path/filepath" - "strings" - "testing" -) - -func TestCanonicalRepoPathNormalRepo(t *testing.T) { - dir := t.TempDir() - gitInit(t, dir) - - got, err := CanonicalRepoPath(dir) - if err != nil { - t.Fatalf("CanonicalRepoPath: %v", err) - } - if got != dir { - t.Errorf("got %q, want %q", got, dir) - } -} - -func TestCanonicalRepoPathSubdir(t *testing.T) { - dir := t.TempDir() - gitInit(t, dir) - - sub := filepath.Join(dir, "a", "b") - os.MkdirAll(sub, 0755) - - got, err := CanonicalRepoPath(sub) - if err != nil { - t.Fatalf("CanonicalRepoPath: %v", err) - } - if got != dir { - t.Errorf("got %q, want %q", got, dir) - } -} - -func TestCanonicalRepoPathWorktree(t *testing.T) { - dir := t.TempDir() - gitInit(t, dir) - gitRun(t, dir, "commit", "--allow-empty", "-m", "initial") - - wtDir := filepath.Join(t.TempDir(), "worktree") - gitRun(t, dir, "worktree", "add", wtDir, "-b", "wt-branch") - t.Cleanup(func() { - gitRun(t, dir, "worktree", "remove", "--force", wtDir) - }) - - got, err := CanonicalRepoPath(wtDir) - if err != nil { - t.Fatalf("CanonicalRepoPath: %v", err) - } - - // Resolve symlinks for comparison (macOS /private/tmp vs /tmp). - wantReal, _ := filepath.EvalSymlinks(dir) - gotReal, _ := filepath.EvalSymlinks(got) - if gotReal != wantReal { - t.Errorf("worktree resolved to %q, want %q", gotReal, wantReal) - } -} - -func TestCanonicalRepoPathNotGitRepo(t *testing.T) { - dir := t.TempDir() - got, err := CanonicalRepoPath(dir) - if err != nil { - t.Fatalf("CanonicalRepoPath: %v", err) - } - // For non-git dirs, returns the dir as-is. - if !strings.HasPrefix(got, dir) { - t.Errorf("got %q, want prefix %q", got, dir) - } -} - -func gitInit(t *testing.T, dir string) { - t.Helper() - gitRun(t, dir, "init") - gitRun(t, dir, "config", "user.email", "test@test.com") - gitRun(t, dir, "config", "user.name", "Test") -} - -func gitRun(t *testing.T, dir string, args ...string) { - t.Helper() - cmd := exec.Command("git", args...) - cmd.Dir = dir - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("git %s: %s: %v", strings.Join(args, " "), out, err) - } -} From e252f1bb0fe28893c498769cdfbc4ececa9092dc Mon Sep 17 00:00:00 2001 From: Jason Allum <21417+jallum@users.noreply.github.com> Date: Fri, 24 Apr 2026 14:15:27 -0400 Subject: [PATCH 07/19] Integrate registry into global config Move registry.repos into the YAML config file, eliminating the internal/registry package entirely. Auto-registration writes to config; registry list/prune read from config. Save failures from auto-registration are now silently ignored so commands still run when the config path is unwritable. --- cmd/bw/cli_test.go | 1 - cmd/bw/main.go | 38 ++++---- cmd/bw/registry.go | 59 ++++++------ internal/registry/dir.go | 21 ---- internal/registry/dir_test.go | 20 ---- internal/registry/registry.go | 122 ----------------------- internal/registry/registry_test.go | 124 ------------------------ test/acceptance_test.go | 150 +++++++++++++++-------------- 8 files changed, 124 insertions(+), 411 deletions(-) delete mode 100644 internal/registry/dir.go delete mode 100644 internal/registry/dir_test.go delete mode 100644 internal/registry/registry.go delete mode 100644 internal/registry/registry_test.go diff --git a/cmd/bw/cli_test.go b/cmd/bw/cli_test.go index 0cf1b6eb..dc4858c8 100644 --- a/cmd/bw/cli_test.go +++ b/cmd/bw/cli_test.go @@ -82,7 +82,6 @@ func bwTestEnv(t *testing.T, dir string) []string { "GIT_COMMITTER_NAME=Test", "GIT_COMMITTER_EMAIL=test@test.com", "BW_CONFIG="+filepath.Join(dir, ".bw"), - "BW_REGISTRY="+filepath.Join(t.TempDir(), ".bw"), "GOCOVERDIR="+bwCoverDir, ) } diff --git a/cmd/bw/main.go b/cmd/bw/main.go index 4e228ad5..157a239f 100644 --- a/cmd/bw/main.go +++ b/cmd/bw/main.go @@ -4,11 +4,9 @@ import ( "fmt" "os" "path/filepath" - "sync" "github.com/jallum/beadwork/internal/config" "github.com/jallum/beadwork/internal/issue" - "github.com/jallum/beadwork/internal/registry" "github.com/jallum/beadwork/internal/repo" "golang.org/x/term" ) @@ -99,38 +97,36 @@ func main() { fatal(err.Error()) } + originalCfg := cfg + newCfg, err := c.Run(store, args, w, cfg) if err != nil { fatal(err.Error()) } if newCfg != nil { - if err := newCfg.Save(); err != nil { - fatal(err.Error()) - } + cfg = newCfg } - touchRegistry() -} + cfg = autoRegister(cfg) -var registryOnce sync.Once + if cfg != originalCfg { + _ = cfg.Save() + } +} -func touchRegistry() { +func autoRegister(cfg *config.Config) *config.Config { r, err := repo.FindRepoAt(repoDir) if err != nil || !r.IsInitialized() { - return + return cfg } - reg, err := registry.Load(registry.DefaultPath()) - if err != nil { - registryOnce.Do(func() { - fmt.Fprintf(os.Stderr, "warning: could not load registry: %v\n", err) - }) - return - } - if err := reg.Add(r.RepoDir()); err != nil { - registryOnce.Do(func() { - fmt.Fprintf(os.Stderr, "warning: could not save registry: %v\n", err) - }) + repoPath := r.RepoDir() + for _, p := range cfg.StringSlice("registry.repos") { + if p == repoPath { + return cfg + } } + repos := append(cfg.StringSlice("registry.repos"), repoPath) + return cfg.Set("registry.repos", repos) } // extractDirFlag removes all -C pairs from args and sets repoDir. diff --git a/cmd/bw/registry.go b/cmd/bw/registry.go index 90eb3fad..7b6f7454 100644 --- a/cmd/bw/registry.go +++ b/cmd/bw/registry.go @@ -1,29 +1,27 @@ package main import ( - "github.com/jallum/beadwork/internal/config" - "encoding/json" "fmt" "os" "sort" "strings" + "github.com/jallum/beadwork/internal/config" "github.com/jallum/beadwork/internal/issue" - "github.com/jallum/beadwork/internal/registry" "github.com/jallum/beadwork/internal/repo" "golang.org/x/term" ) var registrySubcommands = map[string]struct { summary string - run func([]string, Writer) error + run func([]string, Writer, *config.Config) (*config.Config, error) }{ "list": {"List registered repositories", cmdRegistryList}, "prune": {"Remove stale registry entries", cmdRegistryPrune}, } -func cmdRegistry(_ *issue.Store, args []string, w Writer, _ *config.Config) (*config.Config, error) { +func cmdRegistry(_ *issue.Store, args []string, w Writer, cfg *config.Config) (*config.Config, error) { if len(args) == 0 { return nil, printRegistryHelp(w) } @@ -44,7 +42,7 @@ func cmdRegistry(_ *issue.Store, args []string, w Writer, _ *config.Config) (*co return nil, printRegistrySubHelp(w, sub, entry.summary) } - return nil, entry.run(subArgs, w) + return entry.run(subArgs, w, cfg) } func printRegistryHelp(w Writer) error { @@ -76,25 +74,22 @@ type registryListEntry struct { Missing bool `json:"missing,omitempty"` } -func cmdRegistryList(args []string, w Writer) error { +func cmdRegistryList(args []string, w Writer, cfg *config.Config) (*config.Config, error) { a, err := ParseArgs(args, nil, []string{"--json"}) if err != nil { - return err + return nil, err } - reg, err := registry.Load(registry.DefaultPath()) - if err != nil { - return fmt.Errorf("load registry: %w", err) - } + paths := cfg.StringSlice("registry.repos") + sort.Strings(paths) - paths := reg.Paths() if len(paths) == 0 { if a.JSON() { fmt.Fprintln(w, "[]") } else { fmt.Fprintln(w, "no registered repositories") } - return nil + return nil, nil } var list []registryListEntry @@ -111,7 +106,7 @@ func cmdRegistryList(args []string, w Writer) error { if a.JSON() { data, _ := json.MarshalIndent(list, "", " ") fmt.Fprintln(w, string(data)) - return nil + return nil, nil } for _, le := range list { @@ -125,26 +120,21 @@ func cmdRegistryList(args []string, w Writer) error { } fmt.Fprintln(w, line) } - return nil + return nil, nil } -func cmdRegistryPrune(args []string, w Writer) error { +func cmdRegistryPrune(args []string, w Writer, cfg *config.Config) (*config.Config, error) { a, err := ParseArgs(args, nil, []string{"--yes", "-y"}) if err != nil { - return err + return nil, err } force := a.Bool("--yes") || a.Bool("-y") - reg, err := registry.Load(registry.DefaultPath()) - if err != nil { - return fmt.Errorf("load registry: %w", err) - } - - paths := reg.Paths() + paths := cfg.StringSlice("registry.repos") if len(paths) == 0 { fmt.Fprintln(w, "registry is empty, nothing to prune") - return nil + return nil, nil } var missing []string @@ -157,7 +147,7 @@ func cmdRegistryPrune(args []string, w Writer) error { if len(missing) == 0 { fmt.Fprintln(w, "all registered repos exist, nothing to prune") - return nil + return nil, nil } if len(missing) > len(paths)/2 { @@ -174,7 +164,7 @@ func cmdRegistryPrune(args []string, w Writer) error { if !force { if !term.IsTerminal(int(os.Stdin.Fd())) { - return fmt.Errorf("non-interactive: pass --yes to confirm") + return nil, fmt.Errorf("non-interactive: pass --yes to confirm") } fmt.Fprint(w, "\nRemove these entries? [y/N] ") var response string @@ -182,17 +172,22 @@ func cmdRegistryPrune(args []string, w Writer) error { response = strings.TrimSpace(strings.ToLower(response)) if response != "y" && response != "yes" { fmt.Fprintln(w, "aborted") - return nil + return nil, nil } } + missingSet := make(map[string]bool, len(missing)) for _, p := range missing { - reg.Remove(p) + missingSet[p] = true } - if err := reg.Save(); err != nil { - return fmt.Errorf("save registry: %w", err) + var kept []string + for _, p := range paths { + if !missingSet[p] { + kept = append(kept, p) + } } + cfg = cfg.Set("registry.repos", kept) fmt.Fprintf(w, "pruned %d entries\n", len(missing)) - return nil + return cfg, nil } diff --git a/internal/registry/dir.go b/internal/registry/dir.go deleted file mode 100644 index 74011dc6..00000000 --- a/internal/registry/dir.go +++ /dev/null @@ -1,21 +0,0 @@ -package registry - -import ( - "os" - "path/filepath" -) - -const defaultFileName = ".bw" - -// DefaultPath returns the path to the registry file. -// BW_REGISTRY overrides it; otherwise it falls back to ~/.bw. -func DefaultPath() string { - if v := os.Getenv("BW_REGISTRY"); v != "" { - return v - } - home, err := os.UserHomeDir() - if err != nil { - return defaultFileName - } - return filepath.Join(home, defaultFileName) -} diff --git a/internal/registry/dir_test.go b/internal/registry/dir_test.go deleted file mode 100644 index 16a6cd34..00000000 --- a/internal/registry/dir_test.go +++ /dev/null @@ -1,20 +0,0 @@ -package registry - -import ( - "testing" -) - -func TestDefaultPathRespectsEnv(t *testing.T) { - t.Setenv("BW_REGISTRY", "/custom/path") - if got := DefaultPath(); got != "/custom/path" { - t.Errorf("DefaultPath() = %q, want /custom/path", got) - } -} - -func TestDefaultPathFallsBackToHome(t *testing.T) { - t.Setenv("BW_REGISTRY", "") - got := DefaultPath() - if got == "" || got == defaultFileName { - t.Errorf("DefaultPath() = %q, expected a home-based path", got) - } -} diff --git a/internal/registry/registry.go b/internal/registry/registry.go deleted file mode 100644 index e7f8b1c6..00000000 --- a/internal/registry/registry.go +++ /dev/null @@ -1,122 +0,0 @@ -// Package registry tracks which repositories on this host use beadwork. -// The registry is a plain text file (one absolute path per line) at -// ~/.bw by default, overridden by $BW_REGISTRY. -package registry - -import ( - "bufio" - "fmt" - "os" - "path/filepath" - "sort" - "strings" - "sync" -) - -// Registry holds the in-memory set of registered repo paths. -type Registry struct { - paths map[string]bool - file string - mu sync.Mutex -} - -// Load reads the registry from file. If the file does not exist, returns -// an empty registry. -func Load(file string) (*Registry, error) { - r := &Registry{ - paths: make(map[string]bool), - file: file, - } - - f, err := os.Open(file) - if err != nil { - if os.IsNotExist(err) { - return r, nil - } - return nil, fmt.Errorf("read registry: %w", err) - } - defer f.Close() - - scanner := bufio.NewScanner(f) - for scanner.Scan() { - line := strings.TrimSpace(scanner.Text()) - if line != "" { - r.paths[line] = true - } - } - if err := scanner.Err(); err != nil { - return nil, fmt.Errorf("read registry: %w", err) - } - return r, nil -} - -// Save atomically writes the registry to disk. -func (r *Registry) Save() error { - r.mu.Lock() - defer r.mu.Unlock() - return r.saveLocked() -} - -func (r *Registry) saveLocked() error { - dir := filepath.Dir(r.file) - if err := os.MkdirAll(dir, 0755); err != nil { - return fmt.Errorf("create registry dir: %w", err) - } - - paths := r.sortedPaths() - var b strings.Builder - for _, p := range paths { - b.WriteString(p) - b.WriteByte('\n') - } - - tmp := r.file + ".tmp" - if err := os.WriteFile(tmp, []byte(b.String()), 0644); err != nil { - return fmt.Errorf("write temp registry: %w", err) - } - if err := os.Rename(tmp, r.file); err != nil { - os.Remove(tmp) - return fmt.Errorf("rename registry: %w", err) - } - return nil -} - -// Add registers a repo path and saves atomically. -func (r *Registry) Add(repoPath string) error { - r.mu.Lock() - defer r.mu.Unlock() - - if r.paths[repoPath] { - return nil - } - r.paths[repoPath] = true - return r.saveLocked() -} - -// Remove deletes a repo path and saves atomically. -func (r *Registry) Remove(repoPath string) bool { - r.mu.Lock() - defer r.mu.Unlock() - - if !r.paths[repoPath] { - return false - } - delete(r.paths, repoPath) - return true -} - -// Paths returns a sorted snapshot of all registered paths. -func (r *Registry) Paths() []string { - r.mu.Lock() - defer r.mu.Unlock() - return r.sortedPaths() -} - -func (r *Registry) sortedPaths() []string { - paths := make([]string, 0, len(r.paths)) - for p := range r.paths { - paths = append(paths, p) - } - sort.Strings(paths) - return paths -} diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go deleted file mode 100644 index 4888a9d3..00000000 --- a/internal/registry/registry_test.go +++ /dev/null @@ -1,124 +0,0 @@ -package registry - -import ( - "os" - "path/filepath" - "sync" - "testing" -) - -func TestLoadEmpty(t *testing.T) { - file := filepath.Join(t.TempDir(), "reg") - r, err := Load(file) - if err != nil { - t.Fatalf("Load: %v", err) - } - if len(r.Paths()) != 0 { - t.Errorf("Paths = %v, want empty", r.Paths()) - } -} - -func TestLoadSaveRoundTrip(t *testing.T) { - file := filepath.Join(t.TempDir(), "reg") - r, err := Load(file) - if err != nil { - t.Fatal(err) - } - - if err := r.Add("/home/user/project-a"); err != nil { - t.Fatal(err) - } - if err := r.Add("/home/user/project-b"); err != nil { - t.Fatal(err) - } - - r2, err := Load(file) - if err != nil { - t.Fatalf("Load after save: %v", err) - } - paths := r2.Paths() - if len(paths) != 2 { - t.Fatalf("Paths count = %d, want 2", len(paths)) - } - if paths[0] != "/home/user/project-a" || paths[1] != "/home/user/project-b" { - t.Errorf("Paths = %v", paths) - } -} - -func TestAddIdempotent(t *testing.T) { - file := filepath.Join(t.TempDir(), "reg") - r, _ := Load(file) - - r.Add("/repo") - r.Add("/repo") - - if len(r.Paths()) != 1 { - t.Errorf("Paths count = %d, want 1", len(r.Paths())) - } -} - -func TestRemove(t *testing.T) { - file := filepath.Join(t.TempDir(), "reg") - r, _ := Load(file) - r.Add("/keep") - r.Add("/remove-me") - - if !r.Remove("/remove-me") { - t.Error("Remove returned false for existing path") - } - if r.Remove("/nonexistent") { - t.Error("Remove returned true for nonexistent path") - } - if len(r.Paths()) != 1 { - t.Errorf("Paths count = %d, want 1", len(r.Paths())) - } -} - -func TestConcurrentAdd(t *testing.T) { - file := filepath.Join(t.TempDir(), "reg") - r, _ := Load(file) - - var wg sync.WaitGroup - for i := range 20 { - wg.Add(1) - go func(n int) { - defer wg.Done() - r.Add("/repo/" + string(rune('a'+n))) - }(i) - } - wg.Wait() - - r2, err := Load(file) - if err != nil { - t.Fatalf("Load after concurrent adds: %v", err) - } - if len(r2.Paths()) != 20 { - t.Errorf("Paths count = %d, want 20", len(r2.Paths())) - } -} - -func TestPathsSorted(t *testing.T) { - file := filepath.Join(t.TempDir(), "reg") - r, _ := Load(file) - r.Add("/z") - r.Add("/a") - r.Add("/m") - - paths := r.Paths() - if paths[0] != "/a" || paths[1] != "/m" || paths[2] != "/z" { - t.Errorf("Paths not sorted: %v", paths) - } -} - -func TestLoadIgnoresBlankLines(t *testing.T) { - file := filepath.Join(t.TempDir(), "reg") - os.WriteFile(file, []byte("/a\n\n \n/b\n"), 0644) - - r, err := Load(file) - if err != nil { - t.Fatal(err) - } - if len(r.Paths()) != 2 { - t.Errorf("Paths count = %d, want 2", len(r.Paths())) - } -} diff --git a/test/acceptance_test.go b/test/acceptance_test.go index 16b0a4ac..df5757ba 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/jallum/beadwork/internal/config" ) var bwBin string // path to built bw binary @@ -34,10 +36,10 @@ func TestMain(m *testing.M) { // bwEnv is a self-contained environment for running bw commands against // a deterministic git repo. type bwEnv struct { - t *testing.T - dir string - registryDir string - env []string + t *testing.T + dir string + cfgPath string + env []string } const fixedClock = "2026-01-15T10:00:00Z" @@ -45,19 +47,18 @@ const fixedClock = "2026-01-15T10:00:00Z" func newBwEnv(t *testing.T) *bwEnv { t.Helper() dir := t.TempDir() - registryFile := filepath.Join(t.TempDir(), ".bw") + cfgPath := filepath.Join(dir, ".bw") env := &bwEnv{ - t: t, - dir: dir, - registryDir: registryFile, + t: t, + dir: dir, + cfgPath: cfgPath, env: append(os.Environ(), "BW_CLOCK="+fixedClock, - "BW_CONFIG="+filepath.Join(dir, ".bw"), + "BW_CONFIG="+cfgPath, "GIT_AUTHOR_DATE="+fixedClock, "GIT_COMMITTER_DATE="+fixedClock, "NO_COLOR=1", - "BW_REGISTRY="+registryFile, ), } @@ -169,24 +170,23 @@ func compareGolden(t *testing.T, name, got string) { } // newMultiRepoEnv creates n independent bwEnv instances that share the same -// registry directory, simulating multiple repos registered in a single registry. +// config file, simulating multiple repos registered in a single config. func newMultiRepoEnv(t *testing.T, n int) []*bwEnv { t.Helper() - registryFile := filepath.Join(t.TempDir(), ".bw") + cfgPath := filepath.Join(t.TempDir(), ".bw") envs := make([]*bwEnv, n) for i := range n { dir := t.TempDir() env := &bwEnv{ - t: t, - dir: dir, - registryDir: registryFile, + t: t, + dir: dir, + cfgPath: cfgPath, env: append(os.Environ(), "BW_CLOCK="+fixedClock, - "BW_CONFIG="+filepath.Join(dir, ".bw-config"), + "BW_CONFIG="+cfgPath, "GIT_AUTHOR_DATE="+fixedClock, "GIT_COMMITTER_DATE="+fixedClock, "NO_COLOR=1", - "BW_REGISTRY="+registryFile, ), } env.git("init") @@ -201,61 +201,61 @@ func newMultiRepoEnv(t *testing.T, n int) []*bwEnv { return envs } -// seedRegistry writes paths (one per line) to the registry file. +// seedRegistry adds paths to the config's registry.repos key. func (e *bwEnv) seedRegistry(paths ...string) { e.t.Helper() - content := strings.Join(paths, "\n") + "\n" - if err := os.WriteFile(e.registryDir, []byte(content), 0644); err != nil { - e.t.Fatalf("seedRegistry: %v", err) + cfg, err := config.Load(e.cfgPath) + if err != nil { + e.t.Fatalf("seedRegistry load: %v", err) + } + existing := cfg.StringSlice("registry.repos") + cfg = cfg.Set("registry.repos", append(existing, paths...)) + if err := cfg.Save(); err != nil { + e.t.Fatalf("seedRegistry save: %v", err) } } -// registryContents reads and returns the raw content of the registry file. -// Returns an empty string if the file does not exist. -func (e *bwEnv) registryContents() string { +// registryPaths returns the registered repo paths from the config. +func (e *bwEnv) registryPaths() []string { e.t.Helper() - data, err := os.ReadFile(e.registryDir) + cfg, err := config.Load(e.cfgPath) if err != nil { - if os.IsNotExist(err) { - return "" - } - e.t.Fatalf("registryContents: %v", err) + e.t.Fatalf("registryPaths load: %v", err) } - return string(data) + return cfg.StringSlice("registry.repos") } // TestScaffoldingHelpers verifies that the test scaffolding helpers work correctly. func TestScaffoldingHelpers(t *testing.T) { - // newBwEnv should set up a registry file path env := newBwEnv(t) - if env.registryDir == "" { - t.Fatal("registryDir not set") + if env.cfgPath == "" { + t.Fatal("cfgPath not set") } - // seedRegistry + registryContents round-trip + // seedRegistry + registryPaths round-trip env.seedRegistry("/a", "/b") - got := env.registryContents() - if !strings.Contains(got, "/a") || !strings.Contains(got, "/b") { - t.Errorf("registryContents() = %q, want /a and /b", got) + got := env.registryPaths() + found := strings.Join(got, " ") + if !strings.Contains(found, "/a") || !strings.Contains(found, "/b") { + t.Errorf("registryPaths() = %v, want /a and /b", got) } - // registryContents on a fresh env returns content from auto-registration - // (bw init triggers touchRegistry). + // registryPaths on a fresh env returns entries from auto-registration env2 := newBwEnv(t) - c := env2.registryContents() - if c == "" { - t.Errorf("registryContents() on fresh env is empty, expected auto-reg data") + paths := env2.registryPaths() + if len(paths) == 0 { + t.Errorf("registryPaths() on fresh env is empty, expected auto-reg data") } - // newMultiRepoEnv creates n envs sharing a registry file + // newMultiRepoEnv creates n envs sharing a config file envs := newMultiRepoEnv(t, 3) if len(envs) != 3 { t.Fatalf("newMultiRepoEnv(3) returned %d envs", len(envs)) } - sharedFile := envs[0].registryDir + sharedFile := envs[0].cfgPath for i, e := range envs { - if e.registryDir != sharedFile { - t.Errorf("env[%d].registryDir = %q, want %q (shared)", i, e.registryDir, sharedFile) + if e.cfgPath != sharedFile { + t.Errorf("env[%d].cfgPath = %q, want %q (shared)", i, e.cfgPath, sharedFile) } out := e.bw("list") _ = out @@ -318,12 +318,19 @@ func TestAutoRegistrationOnAnyCommand(t *testing.T) { // Even a read-only command should register. env.bw("list") - got := env.registryContents() - if got == "" { - t.Fatal("registry file not created after bw list") + got := env.registryPaths() + if len(got) == 0 { + t.Fatal("registry empty after bw list") } - if !strings.Contains(got, env.dir) { - t.Errorf("registry does not contain repo path %q:\n%s", env.dir, got) + canonDir, _ := filepath.EvalSymlinks(env.dir) + found := false + for _, p := range got { + if p == env.dir || p == canonDir { + found = true + } + } + if !found { + t.Errorf("registry does not contain repo path %q: %v", env.dir, got) } } @@ -333,27 +340,27 @@ func TestAutoRegFiresForReadOnlyCommands(t *testing.T) { env := newBwEnv(t) env.bw("ready") - got := env.registryContents() - if got == "" { - t.Fatal("registry file not created after bw ready") + got := env.registryPaths() + if len(got) == 0 { + t.Fatal("registry empty after bw ready") } } -// TestAutoRegistrationSilentFailure verifies that if the registry dir is +// TestAutoRegistrationSilentFailure verifies that if the config dir is // unwritable, bw still runs the command successfully. func TestAutoRegistrationSilentFailure(t *testing.T) { dir := t.TempDir() + cfgPath := "/nonexistent/path/that/should/fail/.bw" env := &bwEnv{ - t: t, - dir: dir, - registryDir: "/nonexistent/path/that/should/fail/.bw", + t: t, + dir: dir, + cfgPath: cfgPath, env: append(os.Environ(), "BW_CLOCK="+fixedClock, - "BW_CONFIG="+filepath.Join(dir, ".bw-config"), + "BW_CONFIG="+cfgPath, "GIT_AUTHOR_DATE="+fixedClock, "GIT_COMMITTER_DATE="+fixedClock, "NO_COLOR=1", - "BW_REGISTRY=/nonexistent/path/that/should/fail/.bw", ), } env.git("init") @@ -364,7 +371,7 @@ func TestAutoRegistrationSilentFailure(t *testing.T) { env.git("commit", "-m", "initial") env.bw("init", "--prefix", "test") - // Should succeed despite unwritable registry dir. + // Should succeed despite unwritable config path. out := env.bw("list") _ = out } @@ -384,13 +391,14 @@ func TestWorktreeRegistersSameAsMain(t *testing.T) { // Run bw from the worktree. env.bwAt(wtDir, "list") - got := env.registryContents() - if got == "" { - t.Fatal("registry not created after bw list from worktree") + got := env.registryPaths() + if len(got) == 0 { + t.Fatal("registry empty after bw list from worktree") } - // Should contain the main repo dir, not the worktree dir. - if strings.Contains(got, wtDir) { - t.Errorf("registry should not contain worktree path %q:\n%s", wtDir, got) + for _, p := range got { + if p == wtDir { + t.Errorf("registry should not contain worktree path %q: %v", wtDir, got) + } } } @@ -439,9 +447,11 @@ func TestRegistryPruneYes(t *testing.T) { } // Verify it's actually gone. - contents := env.registryContents() - if strings.Contains(contents, "/nonexistent/repo") { - t.Errorf("pruned entry still in registry:\n%s", contents) + paths := env.registryPaths() + for _, p := range paths { + if p == "/nonexistent/repo" { + t.Errorf("pruned entry still in registry: %v", paths) + } } } From e4ef771600bb58b9219abd790e400f64b95b7661 Mon Sep 17 00:00:00 2001 From: Jason Allum <21417+jallum@users.noreply.github.com> Date: Fri, 24 Apr 2026 14:52:33 -0400 Subject: [PATCH 08/19] Add internal/registry package with config-backed helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reintroduce internal/registry as a thin layer over config providing Paths, Repos (with live prefix lookup), Resolve (prefix→path), and Register (immutable add). autoRegister and registry commands now use these helpers, giving the upcoming recap --all a clean API to fan out across registered repos. --- cmd/bw/main.go | 10 +---- cmd/bw/registry.go | 5 ++- internal/registry/registry.go | 57 +++++++++++++++++++++++++++++ internal/registry/registry_test.go | 59 ++++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 10 deletions(-) create mode 100644 internal/registry/registry.go create mode 100644 internal/registry/registry_test.go diff --git a/cmd/bw/main.go b/cmd/bw/main.go index 157a239f..8781eb14 100644 --- a/cmd/bw/main.go +++ b/cmd/bw/main.go @@ -7,6 +7,7 @@ import ( "github.com/jallum/beadwork/internal/config" "github.com/jallum/beadwork/internal/issue" + "github.com/jallum/beadwork/internal/registry" "github.com/jallum/beadwork/internal/repo" "golang.org/x/term" ) @@ -119,14 +120,7 @@ func autoRegister(cfg *config.Config) *config.Config { if err != nil || !r.IsInitialized() { return cfg } - repoPath := r.RepoDir() - for _, p := range cfg.StringSlice("registry.repos") { - if p == repoPath { - return cfg - } - } - repos := append(cfg.StringSlice("registry.repos"), repoPath) - return cfg.Set("registry.repos", repos) + return registry.Register(cfg, r.RepoDir()) } // extractDirFlag removes all -C pairs from args and sets repoDir. diff --git a/cmd/bw/registry.go b/cmd/bw/registry.go index 7b6f7454..e3e4e8e7 100644 --- a/cmd/bw/registry.go +++ b/cmd/bw/registry.go @@ -9,6 +9,7 @@ import ( "github.com/jallum/beadwork/internal/config" "github.com/jallum/beadwork/internal/issue" + "github.com/jallum/beadwork/internal/registry" "github.com/jallum/beadwork/internal/repo" "golang.org/x/term" ) @@ -80,7 +81,7 @@ func cmdRegistryList(args []string, w Writer, cfg *config.Config) (*config.Confi return nil, err } - paths := cfg.StringSlice("registry.repos") + paths := registry.Paths(cfg) sort.Strings(paths) if len(paths) == 0 { @@ -131,7 +132,7 @@ func cmdRegistryPrune(args []string, w Writer, cfg *config.Config) (*config.Conf force := a.Bool("--yes") || a.Bool("-y") - paths := cfg.StringSlice("registry.repos") + paths := registry.Paths(cfg) if len(paths) == 0 { fmt.Fprintln(w, "registry is empty, nothing to prune") return nil, nil diff --git a/internal/registry/registry.go b/internal/registry/registry.go new file mode 100644 index 00000000..e60d09dc --- /dev/null +++ b/internal/registry/registry.go @@ -0,0 +1,57 @@ +// Package registry provides config-backed helpers for the host-local +// repository registry (the registry.repos key in the global config). +package registry + +import ( + "github.com/jallum/beadwork/internal/config" + "github.com/jallum/beadwork/internal/repo" +) + +const key = "registry.repos" + +// Paths returns the registered repo paths from config. +func Paths(cfg *config.Config) []string { + return cfg.StringSlice(key) +} + +// Repo pairs a filesystem path with the prefix read live from the repo. +type Repo struct { + Path string + Prefix string +} + +// Repos returns all registered repos with their prefixes. Entries that +// can't be opened or aren't initialized are silently skipped. +func Repos(cfg *config.Config) []Repo { + var out []Repo + for _, p := range Paths(cfg) { + r, err := repo.FindRepoAt(p) + if err != nil || !r.IsInitialized() { + continue + } + out = append(out, Repo{Path: p, Prefix: r.Prefix}) + } + return out +} + +// Resolve finds the repo path for a given prefix. Returns ("", false) +// if no match is found. +func Resolve(cfg *config.Config, prefix string) (string, bool) { + for _, r := range Repos(cfg) { + if r.Prefix == prefix { + return r.Path, true + } + } + return "", false +} + +// Register returns a new config with path added to the registry. If the +// path is already registered, returns cfg unchanged (same pointer). +func Register(cfg *config.Config, path string) *config.Config { + for _, p := range Paths(cfg) { + if p == path { + return cfg + } + } + return cfg.Set(key, append(Paths(cfg), path)) +} diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go new file mode 100644 index 00000000..68b76ccf --- /dev/null +++ b/internal/registry/registry_test.go @@ -0,0 +1,59 @@ +package registry + +import ( + "path/filepath" + "testing" + + "github.com/jallum/beadwork/internal/config" +) + +func emptyCfg(t *testing.T) *config.Config { + t.Helper() + cfg, err := config.Load(filepath.Join(t.TempDir(), "cfg.yml")) + if err != nil { + t.Fatal(err) + } + return cfg +} + +func TestPathsEmpty(t *testing.T) { + cfg := emptyCfg(t) + if got := Paths(cfg); len(got) != 0 { + t.Errorf("Paths = %v, want empty", got) + } +} + +func TestRegisterAndPaths(t *testing.T) { + cfg := emptyCfg(t) + + cfg2 := Register(cfg, "/a") + if cfg2 == cfg { + t.Fatal("Register returned same pointer for new path") + } + + paths := Paths(cfg2) + if len(paths) != 1 || paths[0] != "/a" { + t.Errorf("Paths = %v, want [/a]", paths) + } + + // Original unchanged. + if len(Paths(cfg)) != 0 { + t.Error("Register mutated original config") + } +} + +func TestRegisterIdempotent(t *testing.T) { + cfg := emptyCfg(t) + cfg = Register(cfg, "/a") + cfg2 := Register(cfg, "/a") + if cfg2 != cfg { + t.Error("duplicate Register returned new pointer") + } +} + +func TestResolveNoRepos(t *testing.T) { + cfg := emptyCfg(t) + if _, ok := Resolve(cfg, "test"); ok { + t.Error("Resolve found a match in empty registry") + } +} From 88f25e499c96c99752040bf59c14c4ccf96006b3 Mon Sep 17 00:00:00 2001 From: Jason Allum <21417+jallum@users.noreply.github.com> Date: Sun, 26 Apr 2026 10:16:39 -0400 Subject: [PATCH 09/19] Gate auto-registration on registry.auto config flag Auto-registration now only fires when registry.auto is true in the global config, and reuses the repo already resolved by the store instead of looking it up again. --- cmd/bw/main.go | 13 ++++--------- test/acceptance_test.go | 24 ++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/cmd/bw/main.go b/cmd/bw/main.go index 8781eb14..e33bd426 100644 --- a/cmd/bw/main.go +++ b/cmd/bw/main.go @@ -108,21 +108,16 @@ func main() { cfg = newCfg } - cfg = autoRegister(cfg) + if store != nil && cfg.Bool("registry.auto") { + r := store.Committer.(*repo.Repo) + cfg = registry.Register(cfg, r.RepoDir()) + } if cfg != originalCfg { _ = cfg.Save() } } -func autoRegister(cfg *config.Config) *config.Config { - r, err := repo.FindRepoAt(repoDir) - if err != nil || !r.IsInitialized() { - return cfg - } - return registry.Register(cfg, r.RepoDir()) -} - // extractDirFlag removes all -C pairs from args and sets repoDir. func extractDirFlag(args []string) []string { out := make([]string, 0, len(args)) diff --git a/test/acceptance_test.go b/test/acceptance_test.go index df5757ba..18e84be6 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -73,6 +73,9 @@ func newBwEnv(t *testing.T) *bwEnv { // Initialize beadwork. env.bw("init", "--prefix", "test") + // Enable auto-registration so tests see the expected registry behavior. + env.setConfig("registry.auto", true) + return env } @@ -196,11 +199,27 @@ func newMultiRepoEnv(t *testing.T, n int) []*bwEnv { env.git("add", ".") env.git("commit", "-m", "initial") env.bw("init", "--prefix", fmt.Sprintf("r%d", i)) + if i == 0 { + env.setConfig("registry.auto", true) + } envs[i] = env } return envs } +// setConfig sets a key in the global config file. +func (e *bwEnv) setConfig(key string, value any) { + e.t.Helper() + cfg, err := config.Load(e.cfgPath) + if err != nil { + e.t.Fatalf("setConfig load: %v", err) + } + cfg = cfg.Set(key, value) + if err := cfg.Save(); err != nil { + e.t.Fatalf("setConfig save: %v", err) + } +} + // seedRegistry adds paths to the config's registry.repos key. func (e *bwEnv) seedRegistry(paths ...string) { e.t.Helper() @@ -240,11 +259,12 @@ func TestScaffoldingHelpers(t *testing.T) { t.Errorf("registryPaths() = %v, want /a and /b", got) } - // registryPaths on a fresh env returns entries from auto-registration + // registryPaths populated after first NeedsStore command env2 := newBwEnv(t) + env2.bw("list") // triggers auto-registration paths := env2.registryPaths() if len(paths) == 0 { - t.Errorf("registryPaths() on fresh env is empty, expected auto-reg data") + t.Errorf("registryPaths() empty after bw list, expected auto-reg data") } // newMultiRepoEnv creates n envs sharing a config file From ee8cdd5d00c7c9c927815b914dbfc38148451764 Mon Sep 17 00:00:00 2001 From: Jason Allum <21417+jallum@users.noreply.github.com> Date: Sun, 26 Apr 2026 10:18:31 -0400 Subject: [PATCH 10/19] Move registry.auto check into registry.Auto accessor --- cmd/bw/main.go | 2 +- internal/registry/registry.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/bw/main.go b/cmd/bw/main.go index e33bd426..028d9fc0 100644 --- a/cmd/bw/main.go +++ b/cmd/bw/main.go @@ -108,7 +108,7 @@ func main() { cfg = newCfg } - if store != nil && cfg.Bool("registry.auto") { + if store != nil && registry.Auto(cfg) { r := store.Committer.(*repo.Repo) cfg = registry.Register(cfg, r.RepoDir()) } diff --git a/internal/registry/registry.go b/internal/registry/registry.go index e60d09dc..826ad358 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -9,6 +9,11 @@ import ( const key = "registry.repos" +// Auto reports whether automatic registration is enabled. +func Auto(cfg *config.Config) bool { + return cfg.Bool("registry.auto") +} + // Paths returns the registered repo paths from config. func Paths(cfg *config.Config) []string { return cfg.StringSlice(key) From cd0373a25eeff8694deabcf80449f35c1c1f3003 Mon Sep 17 00:00:00 2001 From: Manuel Zubieta Date: Sun, 12 Apr 2026 06:46:11 -0400 Subject: [PATCH 11/19] Record unblocked events in issue history Stamp "unblocked " lines into the close commit message for each newly unblocked dependent, and expose them via bw history. This feeds the recap command's activity view. - Close path scans ready-state transitions and writes unblocked events atomically with the close transition - Retry on ref-update contention (shared beadwork branch can race) - history command renders unblocked events alongside other events - Distinguishes "unblocked" text in a reason from a real stamped event --- cmd/bw/close.go | 53 ++++++++++++++++++++------- cmd/bw/history.go | 36 ++++++++++++++---- internal/issue/issue.go | 7 ++++ internal/treefs/treefs.go | 39 +++++++++++++++++++- internal/treefs/treefs_test.go | 5 ++- test/acceptance_test.go | 67 ++++++++++++++++++++++++++++++++++ 6 files changed, 183 insertions(+), 24 deletions(-) diff --git a/cmd/bw/close.go b/cmd/bw/close.go index e37a3e25..fd4dbc33 100644 --- a/cmd/bw/close.go +++ b/cmd/bw/close.go @@ -1,12 +1,14 @@ package main import ( + "errors" "fmt" "github.com/jallum/beadwork/internal/config" "github.com/jallum/beadwork/internal/issue" "github.com/jallum/beadwork/internal/md" + "github.com/jallum/beadwork/internal/treefs" ) type CloseArgs struct { @@ -30,28 +32,53 @@ func parseCloseArgs(raw []string) (CloseArgs, error) { }, nil } +const closeMaxRetries = 3 + func cmdClose(store *issue.Store, args []string, w Writer, _ *config.Config) (*config.Config, error) { ca, err := parseCloseArgs(args) if err != nil { return nil, err } - iss, err := store.Close(ca.ID, ca.Reason) - if err != nil { - return nil, err - } + var iss *issue.Issue + var unblocked []*issue.Issue - unblocked, err := store.NewlyUnblocked(iss.ID) - if err != nil { - return nil, err - } + for attempt := range closeMaxRetries { + if attempt > 0 { + store.ClearCache() + if err := store.Refresh(); err != nil { + return nil, fmt.Errorf("refresh after conflict: %w", err) + } + } + + iss, err = store.Close(ca.ID, ca.Reason) + if err != nil { + return nil, err + } - intent := fmt.Sprintf("close %s", iss.ID) - if ca.Reason != "" { - intent += fmt.Sprintf(" reason=%q", ca.Reason) + unblocked, err = store.NewlyUnblocked(iss.ID) + if err != nil { + return nil, err + } + + intent := fmt.Sprintf("close %s", iss.ID) + if ca.Reason != "" { + intent += fmt.Sprintf(" reason=%q", ca.Reason) + } + for _, u := range unblocked { + intent += fmt.Sprintf("\nunblocked %s", u.ID) + } + + err = store.Commit(intent) + if err == nil { + break + } + if !errors.Is(err, treefs.ErrRefMoved) { + return nil, fmt.Errorf("commit failed: %w", err) + } } - if err := store.Commit(intent); err != nil { - return nil, fmt.Errorf("commit failed: %w", err) + if err != nil { + return nil, fmt.Errorf("commit failed after %d attempts: %w", closeMaxRetries, err) } if ca.JSON { diff --git a/cmd/bw/history.go b/cmd/bw/history.go index dd9adb94..a2f029e2 100644 --- a/cmd/bw/history.go +++ b/cmd/bw/history.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "regexp" "strings" "github.com/jallum/beadwork/internal/config" @@ -38,11 +39,22 @@ func parseHistoryArgs(raw []string) (HistoryArgs, error) { return ha, nil } +// unblockedRe matches "unblocked " lines in commit messages (line >= 2). +var unblockedRe = regexp.MustCompile(`^unblocked\s+(\S+)$`) + type commitEntry struct { - Hash string `json:"hash"` - Timestamp string `json:"timestamp"` - Author string `json:"author"` - Intent string `json:"intent"` + Hash string `json:"hash"` + Timestamp string `json:"timestamp"` + Author string `json:"author"` + Intent string `json:"intent"` + Unblocked []string `json:"unblocked,omitempty"` +} + +func firstLine(s string) string { + if i := strings.IndexByte(s, '\n'); i >= 0 { + return s[:i] + } + return s } func cmdHistory(store *issue.Store, args []string, w Writer, _ *config.Config) (*config.Config, error) { @@ -68,12 +80,19 @@ func cmdHistory(store *issue.Store, args []string, w Writer, _ *config.Config) ( for i := len(commits) - 1; i >= 0; i-- { c := commits[i] if strings.Contains(c.Message, iss.ID) { - matched = append(matched, commitEntry{ + entry := commitEntry{ Hash: c.Hash, Timestamp: c.Time.UTC().Format("2006-01-02 15:04"), Author: c.Author, - Intent: c.Message, - }) + Intent: firstLine(c.Message), + } + // Parse unblocked lines from the commit message (line >= 2 only). + for _, line := range strings.Split(c.Message, "\n")[1:] { + if m := unblockedRe.FindStringSubmatch(strings.TrimSpace(line)); m != nil { + entry.Unblocked = append(entry.Unblocked, m[1]) + } + } + matched = append(matched, entry) } } @@ -94,6 +113,9 @@ func cmdHistory(store *issue.Store, args []string, w Writer, _ *config.Config) ( for _, e := range matched { fmt.Fprintf(w, "%s %s %s\n", e.Timestamp, e.Author, e.Intent) + for _, uid := range e.Unblocked { + fmt.Fprintf(w, " → unblocked %s\n", uid) + } } return nil, nil } diff --git a/internal/issue/issue.go b/internal/issue/issue.go index c1a78847..6ce1f00a 100644 --- a/internal/issue/issue.go +++ b/internal/issue/issue.go @@ -84,6 +84,13 @@ func (s *Store) ClearCache() { s.idSet = nil } +// Refresh reloads the underlying TreeFS from the current ref and clears +// all caches. Use this after a CAS conflict to pick up the latest state. +func (s *Store) Refresh() error { + s.ClearCache() + return s.FS.Refresh() +} + // Now returns the current time in UTC. If the BW_CLOCK environment variable // is set to an RFC3339 value, that fixed time is used instead of the real // clock. This enables deterministic timestamps for testing and migration. diff --git a/internal/treefs/treefs.go b/internal/treefs/treefs.go index ba12460f..dc897415 100644 --- a/internal/treefs/treefs.go +++ b/internal/treefs/treefs.go @@ -480,8 +480,8 @@ func (t *TreeFS) casUpdateRef(newHash plumbing.Hash) error { // CAS check if currentRef.Hash() != t.baseRef { - return fmt.Errorf("conflict: ref %s has moved (expected %s, got %s)", - t.ref, t.baseRef.String()[:8], currentRef.Hash().String()[:8]) + return fmt.Errorf("%w: ref %s (expected %s, got %s)", + ErrRefMoved, t.ref, t.baseRef.String()[:8], currentRef.Hash().String()[:8]) } // Update ref @@ -793,6 +793,37 @@ func (t *TreeFS) AllCommits() ([]CommitInfo, error) { return commits, nil } +// CommitsSince returns all commits on the tracked ref newer than the given +// hash, newest-first. If sinceHash is zero, returns all commits. +func (t *TreeFS) CommitsSince(sinceHash string) ([]CommitInfo, error) { + if t.baseRef.IsZero() { + return nil, nil + } + var since plumbing.Hash + if sinceHash != "" { + since = plumbing.NewHash(sinceHash) + } + + var commits []CommitInfo + iter, err := t.repo.Log(&git.LogOptions{From: t.baseRef}) + if err != nil { + return nil, fmt.Errorf("walk commits: %w", err) + } + iter.ForEach(func(c *object.Commit) error { + if !since.IsZero() && c.Hash == since { + return storer.ErrStop + } + commits = append(commits, CommitInfo{ + Hash: c.Hash.String(), + Message: strings.TrimSpace(c.Message), + Time: c.Author.When, + Author: c.Author.Name, + }) + return nil + }) + return commits, nil +} + // RefHash returns the current hash of the tracked ref. func (t *TreeFS) RefHash() plumbing.Hash { return t.baseRef @@ -837,6 +868,10 @@ func (t *TreeFS) DeleteRef(name string) error { return t.repo.Storer.RemoveReference(plumbing.ReferenceName(name)) } +// ErrRefMoved is returned by Commit when the ref has moved since the TreeFS +// was opened, indicating a CAS conflict. Callers can check for this to retry. +var ErrRefMoved = fmt.Errorf("ref moved") + // CommitInfo holds a commit hash and message. type CommitInfo struct { Hash string diff --git a/internal/treefs/treefs_test.go b/internal/treefs/treefs_test.go index a6727899..d011cd4e 100644 --- a/internal/treefs/treefs_test.go +++ b/internal/treefs/treefs_test.go @@ -1,6 +1,7 @@ package treefs import ( + "errors" "os" "os/exec" "path/filepath" @@ -383,8 +384,8 @@ func TestCASConflict(t *testing.T) { if err == nil { t.Fatal("expected CAS conflict error") } - if !containsStr(err.Error(), "conflict") { - t.Fatalf("expected conflict error, got: %v", err) + if !errors.Is(err, ErrRefMoved) { + t.Fatalf("expected ErrRefMoved, got: %v", err) } } diff --git a/test/acceptance_test.go b/test/acceptance_test.go index 18e84be6..edf9976e 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -527,6 +527,73 @@ func TestRegistryInBwHelp(t *testing.T) { } } +// TestCloseStampsUnblockedEvents verifies that closing an issue stamps +// "unblocked " lines into the commit message for each newly unblocked issue. +func TestCloseStampsUnblockedEvents(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "blocker", "--id", "bl-1") + env.bw("create", "blocked", "--id", "bl-2") + env.bw("dep", "add", "bl-1", "blocks", "bl-2") + + out := env.bw("close", "bl-1") + if !strings.Contains(out, "unblocked") { + t.Fatalf("close output missing unblocked info:\n%s", out) + } + + // Check the commit message on the beadwork branch. + log := env.git("log", "-1", "--format=%B", "beadwork") + if !strings.Contains(log, "unblocked bl-2") { + t.Errorf("close commit missing 'unblocked bl-2':\n%s", log) + } +} + +// TestCloseChainStampsOnlyRemainingDep verifies that when an issue has +// multiple blockers, closing one does NOT stamp an unblocked event for +// the still-blocked dependent. +func TestCloseChainStampsOnlyRemainingDep(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "blocker A", "--id", "ca-1") + env.bw("create", "blocker B", "--id", "ca-2") + env.bw("create", "dependent", "--id", "ca-3") + env.bw("dep", "add", "ca-1", "blocks", "ca-3") + env.bw("dep", "add", "ca-2", "blocks", "ca-3") + + // Close only ca-1; ca-3 still blocked by ca-2. + env.bw("close", "ca-1") + log := env.git("log", "-1", "--format=%B", "beadwork") + if strings.Contains(log, "unblocked ca-3") { + t.Errorf("close commit should NOT contain 'unblocked ca-3' (still blocked by ca-2):\n%s", log) + } + + // Now close ca-2; ca-3 should be unblocked. + env.bw("close", "ca-2") + log = env.git("log", "-1", "--format=%B", "beadwork") + if !strings.Contains(log, "unblocked ca-3") { + t.Errorf("close commit should contain 'unblocked ca-3':\n%s", log) + } +} + +// TestCloseReasonContainingUnblockedWord verifies that a close reason +// containing the word "unblocked" does not create a spurious unblocked event. +func TestCloseReasonContainingUnblockedWord(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "solo issue", "--id", "cu-1") + + env.bw("close", "cu-1", "--reason", "unblocked by external team") + log := env.git("log", "-1", "--format=%B", "beadwork") + // The reason appears in the first line (close intent), but there should be + // no second line matching "unblocked ". + lines := strings.Split(strings.TrimSpace(log), "\n") + for _, line := range lines[1:] { + line = strings.TrimSpace(line) + if strings.HasPrefix(line, "unblocked ") && !strings.Contains(line, "=") { + // This looks like a stamped event, but it's just the reason text + // on the first line. Additional lines should not match. + t.Errorf("spurious unblocked line in commit: %q", line) + } + } +} + // TestWorktreeRefWrites verifies that bw operations run from inside a git // worktree write refs to the shared git dir, so tickets are visible from // the main checkout. From 72701db36e1f1a592bbac9abcc06e09b07f2f832 Mon Sep 17 00:00:00 2001 From: Manuel Zubieta Date: Sun, 12 Apr 2026 06:56:34 -0400 Subject: [PATCH 12/19] Add bw recap with tree/JSON renderers and cross-repo fan-out internal/recap package: - Data model for per-issue activity windows (events + metadata) - Parser reading beadwork commit log for transitions - Window resolution for today/yesterday/since-last/--hours bw recap command: - Single-repo: tree renderer (grouped by issue) and --json renderer - --all: cross-repo fan-out across the registry, aggregating by repo - Registered under "Cross-Repo & Activity" in bw --help --- cmd/bw/command.go | 23 ++- cmd/bw/main.go | 28 ++++ cmd/bw/recap.go | 269 ++++++++++++++++++++++++++++++++++ cmd/bw/recap_render.go | 116 +++++++++++++++ internal/recap/parse.go | 91 ++++++++++++ internal/recap/parse_test.go | 165 +++++++++++++++++++++ internal/recap/recap.go | 120 +++++++++++++++ internal/recap/recap_test.go | 116 +++++++++++++++ internal/recap/window.go | 87 +++++++++++ internal/recap/window_test.go | 175 ++++++++++++++++++++++ internal/repo/recap_cursor.go | 43 ++++++ test/acceptance_test.go | 259 ++++++++++++++++++++++++++++++++ 12 files changed, 1491 insertions(+), 1 deletion(-) create mode 100644 cmd/bw/recap.go create mode 100644 cmd/bw/recap_render.go create mode 100644 internal/recap/parse.go create mode 100644 internal/recap/parse_test.go create mode 100644 internal/recap/recap.go create mode 100644 internal/recap/recap_test.go create mode 100644 internal/recap/window.go create mode 100644 internal/recap/window_test.go create mode 100644 internal/repo/recap_cursor.go diff --git a/cmd/bw/command.go b/cmd/bw/command.go index 03b39fcd..27005f28 100644 --- a/cmd/bw/command.go +++ b/cmd/bw/command.go @@ -473,6 +473,27 @@ var commands = []Command{ NeedsStore: true, Run: cmdPrime, }, + { + Name: "recap", + Summary: "Show recent activity across issues", + Description: "Summarize beadwork activity in this repo (or --all for every registered repo).\nBy default, shows activity since the last recap — first-time recaps show the last 24 hours.\n\nWindow tokens: today, yesterday, week, 24h, 7d. Use --since for an explicit start.\n--dry-run shows activity without advancing the cursor.", + Flags: []Flag{ + {Long: "--since", Value: "DATE", Help: "Start time (RFC3339 or YYYY-MM-DD)"}, + {Long: "--dry-run", Help: "Show activity without advancing the cursor"}, + {Long: "--all", Help: "Recap every registered repository"}, + {Long: "--json", Help: "Output as JSON"}, + {Long: "--ascii", Help: "Use plain ASCII tree characters"}, + }, + Examples: []Example{ + {Cmd: "bw recap", Help: "Activity since last recap (or 24h if first-time)"}, + {Cmd: "bw recap today"}, + {Cmd: "bw recap week --json"}, + {Cmd: "bw recap --since 2026-01-01"}, + {Cmd: "bw recap --all", Help: "Across all registered repos"}, + {Cmd: "bw recap --dry-run", Help: "Preview without advancing cursor"}, + }, + Run: cmdRecap, + }, { Name: "registry", Summary: "Manage the repository registry", @@ -519,7 +540,7 @@ var commandGroups = []struct { {"Finding Work", []string{"ready", "blocked"}}, {"Dependencies", []string{"dep"}}, {"Sync & Data", []string{"sync", "export", "import"}}, - {"Cross-Repo & Activity", []string{"registry"}}, + {"Cross-Repo & Activity", []string{"recap", "registry"}}, {"Setup & Config", []string{"init", "config", "upgrade", "onboard", "prime"}}, } diff --git a/cmd/bw/main.go b/cmd/bw/main.go index 028d9fc0..89579101 100644 --- a/cmd/bw/main.go +++ b/cmd/bw/main.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "time" "github.com/jallum/beadwork/internal/config" "github.com/jallum/beadwork/internal/issue" @@ -14,6 +15,15 @@ import ( const version = "0.12.3" +// globalNoColor is set by commands (e.g. recap --no-color) to force +// non-colored output even when stdout is a TTY. Consulted at render setup. +var globalNoColor bool + +// globalDryRun mirrors the --dry-run flag. For commands with NeedsStore +// it drives store.DryRun; for commands without a store (e.g. recap) it +// suppresses side-effects like advancing the registry cursor. +var globalDryRun bool + func resolveRenderMode(args []string) string { if mode, ok := flagValue(args, "--x-render-as"); ok && mode != "" { return mode @@ -21,6 +31,9 @@ func resolveRenderMode(args []string) string { if hasFlag(args, "--x-raw") { return "raw" } + if hasFlag(args, "--no-color") { + return "markdown" + } if term.IsTerminal(int(os.Stdout.Fd())) && os.Getenv("NO_COLOR") == "" { return "tty" } @@ -55,10 +68,15 @@ func main() { args = removeFlag(args, "--x-raw") args, _ = removeFlagValue(args, "--x-render-as") + if hasFlag(args, "--no-color") { + globalNoColor = true + args = removeFlag(args, "--no-color") + } dryRun := hasFlag(args, "--dry-run") if dryRun { args = removeFlag(args, "--dry-run") + globalDryRun = true } switch cmd { @@ -118,6 +136,16 @@ func main() { } } +// bwNow returns the current time respecting BW_CLOCK. +func bwNow() time.Time { + if v := os.Getenv("BW_CLOCK"); v != "" { + if t, err := time.Parse(time.RFC3339, v); err == nil { + return t + } + } + return time.Now() +} + // extractDirFlag removes all -C pairs from args and sets repoDir. func extractDirFlag(args []string) []string { out := make([]string, 0, len(args)) diff --git a/cmd/bw/recap.go b/cmd/bw/recap.go new file mode 100644 index 00000000..c2aafd7f --- /dev/null +++ b/cmd/bw/recap.go @@ -0,0 +1,269 @@ +package main + +import ( + "fmt" + "os" + "sort" + "time" + + "github.com/jallum/beadwork/internal/config" + "github.com/jallum/beadwork/internal/issue" + "github.com/jallum/beadwork/internal/recap" + "github.com/jallum/beadwork/internal/registry" + "github.com/jallum/beadwork/internal/repo" + "github.com/jallum/beadwork/internal/treefs" +) + +// storeLookup adapts an *issue.Store to the recap.IssueLookup interface. +type storeLookup struct { + store *issue.Store +} + +func (s *storeLookup) Title(id string) string { + if s.store == nil { + return "" + } + iss, err := s.store.Get(id) + if err != nil { + return "" + } + return iss.Title +} + +type recapArgs struct { + Tokens []string + Since string + JSON bool + ASCII bool + DryRun bool + All bool + Verbose bool +} + +func parseRecapArgs(raw []string) (recapArgs, error) { + expanded := make([]string, len(raw)) + for i, tok := range raw { + if tok == "-v" { + expanded[i] = "--verbose" + } else { + expanded[i] = tok + } + } + a, err := ParseArgs(expanded, + []string{"--since"}, + []string{"--json", "--ascii", "--dry-run", "--all", "--verbose"}, + ) + if err != nil { + return recapArgs{}, err + } + return recapArgs{ + Tokens: a.Pos(), + Since: a.String("--since"), + JSON: a.Bool("--json"), + ASCII: a.Bool("--ascii") || globalNoColor, + DryRun: a.Bool("--dry-run") || globalDryRun, + All: a.Bool("--all"), + Verbose: a.Bool("--verbose"), + }, nil +} + +func cmdRecap(_ *issue.Store, args []string, w Writer, cfg *config.Config) (*config.Config, error) { + ra, err := parseRecapArgs(args) + if err != nil { + return nil, err + } + + if ra.All { + return nil, cmdRecapAll(ra, w, cfg) + } + + return nil, runRecapSingle(ra, w, repoDir) +} + +func runRecapSingle(ra recapArgs, w Writer, dir string) error { + r, err := repo.FindRepoAt(dir) + if err != nil { + return fmt.Errorf("not a git repository (run bw init first)") + } + if !r.IsInitialized() { + return fmt.Errorf("beadwork not initialized. Run: bw init") + } + + store := issue.NewStore(r.TreeFS(), r.Prefix) + store.Committer = r + + now := bwNow() + window, commits, err := resolveRecapWindow(ra, r, now) + if err != nil { + return err + } + + rcp := recap.Build(commits, window, &storeLookup{store: store}) + + if err := renderRecap(w, rcp, ra); err != nil { + return err + } + + if !ra.DryRun { + explicit := ra.Since != "" || len(ra.Tokens) > 0 + cursor := r.RecapCursor() + if explicit && cursor != "" { + gap, cursorKnown := countGap(commits, cursor, window.Start) + if cursorKnown && gap > 0 { + fmt.Fprint(os.Stderr, gapNoticeLine("", gap)) + return nil + } + } + if len(commits) > 0 { + _ = r.SetRecapCursor(commits[0].Hash) + } + } + + return nil +} + +// resolveRecapWindow determines the time window and commit set for a recap. +// Three cases: +// 1. Explicit window (--since or positional tokens) → filter AllCommits by time. +// 2. No cursor yet (first recap) → last 24h backfill. +// 3. Cursor present → use CommitsSince(cursor). +func resolveRecapWindow(ra recapArgs, r *repo.Repo, now time.Time) (recap.Window, []treefs.CommitInfo, error) { + explicit := ra.Since != "" || len(ra.Tokens) > 0 + cursor := r.RecapCursor() + + switch { + case explicit: + window, err := recap.ParseWindow(ra.Tokens, ra.Since, now) + if err != nil { + return recap.Window{}, nil, err + } + commits, err := r.AllCommits() + if err != nil { + return recap.Window{}, nil, fmt.Errorf("read commits: %w", err) + } + return window, commits, nil + + case cursor == "": + window := recap.Window{ + Start: now.Add(-24 * time.Hour), + End: now, + Label: "last 24h (first recap)", + } + commits, err := r.AllCommits() + if err != nil { + return recap.Window{}, nil, fmt.Errorf("read commits: %w", err) + } + return window, commits, nil + + default: + commits, err := r.TreeFS().CommitsSince(cursor) + if err != nil { + return recap.Window{}, nil, fmt.Errorf("read commits: %w", err) + } + start := now + if len(commits) > 0 { + start = commits[len(commits)-1].Time + } + label := "since last recap" + if lastAt := r.LastRecapAt(); !lastAt.IsZero() { + label = fmt.Sprintf("since last recap (%s)", relativeTimeSince(lastAt, now)) + } + return recap.Window{Start: start, End: now, Label: label}, commits, nil + } +} + +func gapNoticeLine(prefix string, gap int) string { + noun := "commits" + if gap == 1 { + noun = "commit" + } + lead := "" + if prefix != "" { + lead = prefix + ": " + } + return fmt.Sprintf("%s%d %s older than this window and newer than your last recap were not shown. Run 'bw recap' to see them.\n", + lead, gap, noun) +} + +func countGap(commits []treefs.CommitInfo, cursor string, windowStart time.Time) (int, bool) { + var cursorTime time.Time + found := false + for _, c := range commits { + if c.Hash == cursor { + cursorTime = c.Time + found = true + break + } + } + if !found { + return 0, false + } + gap := 0 + for _, c := range commits { + if c.Time.After(cursorTime) && c.Time.Before(windowStart) { + gap++ + } + } + return gap, true +} + +func cmdRecapAll(ra recapArgs, w Writer, cfg *config.Config) error { + if repoDir != "" { + fmt.Fprintln(os.Stderr, "warning: -C is ignored with --all") + } + + paths := registry.Paths(cfg) + if len(paths) == 0 { + fmt.Fprintln(w, "no registered repositories") + return nil + } + + now := bwNow() + var all []repoRecap + + for _, p := range paths { + if _, err := os.Stat(p); err != nil { + fmt.Fprintf(os.Stderr, "skipping %s: %v\n", p, err) + continue + } + r, err := repo.FindRepoAt(p) + if err != nil { + fmt.Fprintf(os.Stderr, "skipping %s: %v\n", p, err) + continue + } + if !r.IsInitialized() { + fmt.Fprintf(os.Stderr, "skipping %s: beadwork not initialized\n", p) + continue + } + + store := issue.NewStore(r.TreeFS(), r.Prefix) + store.Committer = r + + window, commits, err := resolveRecapWindow(ra, r, now) + if err != nil { + fmt.Fprintf(os.Stderr, "skipping %s: %v\n", p, err) + continue + } + + rcp := recap.Build(commits, window, &storeLookup{store: store}) + all = append(all, repoRecap{Path: p, Recap: rcp}) + + if !ra.DryRun { + explicit := ra.Since != "" || len(ra.Tokens) > 0 + cursor := r.RecapCursor() + if explicit && cursor != "" { + gap, cursorKnown := countGap(commits, cursor, window.Start) + if cursorKnown && gap > 0 { + fmt.Fprint(os.Stderr, gapNoticeLine(p, gap)) + continue + } + } + if len(commits) > 0 { + _ = r.SetRecapCursor(commits[0].Hash) + } + } + } + + sort.Slice(all, func(i, j int) bool { return all[i].Path < all[j].Path }) + return renderCrossRepo(w, all, ra) +} diff --git a/cmd/bw/recap_render.go b/cmd/bw/recap_render.go new file mode 100644 index 00000000..c19b8742 --- /dev/null +++ b/cmd/bw/recap_render.go @@ -0,0 +1,116 @@ +package main + +import ( + "encoding/json" + "fmt" + + "github.com/jallum/beadwork/internal/recap" +) + +// renderRecap renders a single-repo recap to w. +func renderRecap(w Writer, r recap.Recap, ra recapArgs) error { + if ra.JSON { + return renderRecapJSON(w, r) + } + return renderRecapTree(w, r, ra.ASCII) +} + +func renderRecapJSON(w Writer, r recap.Recap) error { + out := struct { + Scope string `json:"scope"` + Recap recap.Recap `json:"recap"` + }{Scope: "single", Recap: r} + data, err := json.MarshalIndent(out, "", " ") + if err != nil { + return err + } + fmt.Fprintln(w, string(data)) + return nil +} + +func renderRecapTree(w Writer, r recap.Recap, ascii bool) error { + fmt.Fprintf(w, "Recap: %s\n", r.Window.Label) + if len(r.Sections) == 0 { + fmt.Fprintln(w, " (nothing to report — you're caught up)") + return nil + } + + branch := "├─" + last := "└─" + vbar := "│" + if ascii { + branch = "|-" + last = "`-" + vbar = "|" + } + + for i, s := range r.Sections { + isLast := i == len(r.Sections)-1 + marker := branch + if isLast { + marker = last + } + title := s.Title + if title == "" { + title = "(deleted)" + } + fmt.Fprintf(w, "%s %s %s\n", marker, s.ID, title) + + // Indent prefix for leaves. + indent := vbar + " " + if isLast { + indent = " " + } + + for j, leaf := range s.Leaves { + leafMarker := branch + if j == len(s.Leaves)-1 { + leafMarker = last + } + detail := leaf.Type + if leaf.Detail != "" { + detail += " " + leaf.Detail + } + fmt.Fprintf(w, "%s%s %s %s\n", indent, leafMarker, leaf.Time, detail) + } + } + return nil +} + +// repoRecap pairs a repository path with its computed recap. +// Used for cross-repo fan-out rendering. +type repoRecap struct { + Path string `json:"path"` + Recap recap.Recap `json:"recap"` +} + +// renderCrossRepo renders the output for `bw recap --all`. +func renderCrossRepo(w Writer, all []repoRecap, ra recapArgs) error { + if ra.JSON { + out := struct { + Scope string `json:"scope"` + Repos []repoRecap `json:"repos"` + }{Scope: "cross", Repos: all} + data, err := json.MarshalIndent(out, "", " ") + if err != nil { + return err + } + fmt.Fprintln(w, string(data)) + return nil + } + + totalSections := 0 + for _, r := range all { + totalSections += len(r.Recap.Sections) + } + fmt.Fprintf(w, "Cross-repo recap: %d repo(s), %d active issue(s)\n\n", len(all), totalSections) + + for _, rr := range all { + fmt.Fprintf(w, "=== %s ===\n", rr.Path) + if err := renderRecapTree(w, rr.Recap, ra.ASCII); err != nil { + return err + } + fmt.Fprintln(w) + } + return nil +} diff --git a/internal/recap/parse.go b/internal/recap/parse.go new file mode 100644 index 00000000..af96ff5f --- /dev/null +++ b/internal/recap/parse.go @@ -0,0 +1,91 @@ +package recap + +import ( + "regexp" + "strings" + "time" +) + +// Intent patterns — anchored to the start of a line. +var ( + createRe = regexp.MustCompile(`^create\s+(\S+)`) + closeRe = regexp.MustCompile(`^close\s+(\S+)`) + startRe = regexp.MustCompile(`^start\s+(\S+)`) + updateRe = regexp.MustCompile(`^update\s+(\S+)`) + reopenRe = regexp.MustCompile(`^reopen\s+(\S+)`) + deferRe = regexp.MustCompile(`^defer\s+(\S+)`) + undeferRe = regexp.MustCompile(`^undefer\s+(\S+)`) + commentRe = regexp.MustCompile(`^comment\s+(\S+)`) + linkRe = regexp.MustCompile(`^link\s+(\S+)\s+blocks\s+(\S+)`) + unlinkRe = regexp.MustCompile(`^unlink\s+(\S+)\s+blocks\s+(\S+)`) + deleteRe = regexp.MustCompile(`^delete\s+(\S+)`) + labelRe = regexp.MustCompile(`^label\s+(\S+)`) + unblockedRe = regexp.MustCompile(`^unblocked\s+(\S+)$`) +) + +// ParseIntent extracts events from a beadwork commit message. +// The first line is the primary intent; subsequent lines may contain +// secondary events (e.g., "unblocked "). +func ParseIntent(message string, ts time.Time) []Event { + lines := strings.Split(strings.TrimSpace(message), "\n") + if len(lines) == 0 { + return nil + } + + var events []Event + first := strings.TrimSpace(lines[0]) + + switch { + case createRe.MatchString(first): + m := createRe.FindStringSubmatch(first) + detail := strings.TrimSpace(first[len(m[0]):]) + events = append(events, Event{Type: "create", ID: m[1], Time: ts, Detail: detail}) + case closeRe.MatchString(first): + m := closeRe.FindStringSubmatch(first) + detail := strings.TrimSpace(first[len(m[0]):]) + events = append(events, Event{Type: "close", ID: m[1], Time: ts, Detail: detail}) + case startRe.MatchString(first): + m := startRe.FindStringSubmatch(first) + events = append(events, Event{Type: "start", ID: m[1], Time: ts}) + case updateRe.MatchString(first): + m := updateRe.FindStringSubmatch(first) + detail := strings.TrimSpace(first[len(m[0]):]) + events = append(events, Event{Type: "update", ID: m[1], Time: ts, Detail: detail}) + case reopenRe.MatchString(first): + m := reopenRe.FindStringSubmatch(first) + events = append(events, Event{Type: "reopen", ID: m[1], Time: ts}) + case deferRe.MatchString(first): + m := deferRe.FindStringSubmatch(first) + detail := strings.TrimSpace(first[len(m[0]):]) + events = append(events, Event{Type: "defer", ID: m[1], Time: ts, Detail: detail}) + case undeferRe.MatchString(first): + m := undeferRe.FindStringSubmatch(first) + events = append(events, Event{Type: "undefer", ID: m[1], Time: ts}) + case commentRe.MatchString(first): + m := commentRe.FindStringSubmatch(first) + events = append(events, Event{Type: "comment", ID: m[1], Time: ts}) + case linkRe.MatchString(first): + m := linkRe.FindStringSubmatch(first) + events = append(events, Event{Type: "link", ID: m[1], Time: ts, Detail: "blocks " + m[2]}) + case unlinkRe.MatchString(first): + m := unlinkRe.FindStringSubmatch(first) + events = append(events, Event{Type: "unlink", ID: m[1], Time: ts, Detail: "blocks " + m[2]}) + case deleteRe.MatchString(first): + m := deleteRe.FindStringSubmatch(first) + events = append(events, Event{Type: "delete", ID: m[1], Time: ts}) + case labelRe.MatchString(first): + m := labelRe.FindStringSubmatch(first) + detail := strings.TrimSpace(first[len(m[0]):]) + events = append(events, Event{Type: "label", ID: m[1], Time: ts, Detail: detail}) + } + + // Parse secondary events from lines >= 2 (e.g., "unblocked "). + for _, line := range lines[1:] { + line = strings.TrimSpace(line) + if m := unblockedRe.FindStringSubmatch(line); m != nil { + events = append(events, Event{Type: "unblocked", ID: m[1], Time: ts}) + } + } + + return events +} diff --git a/internal/recap/parse_test.go b/internal/recap/parse_test.go new file mode 100644 index 00000000..709b4d31 --- /dev/null +++ b/internal/recap/parse_test.go @@ -0,0 +1,165 @@ +package recap + +import ( + "testing" + "time" +) + +var testTime = time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + +func TestParseIntentCreate(t *testing.T) { + events := ParseIntent(`create bw-abc p2 task "Fix login"`, testTime) + if len(events) != 1 { + t.Fatalf("got %d events, want 1", len(events)) + } + e := events[0] + if e.Type != "create" || e.ID != "bw-abc" { + t.Errorf("type=%q id=%q", e.Type, e.ID) + } + if e.Detail == "" { + t.Error("expected detail for create") + } +} + +func TestParseIntentClose(t *testing.T) { + events := ParseIntent("close bw-xyz", testTime) + if len(events) != 1 || events[0].Type != "close" || events[0].ID != "bw-xyz" { + t.Errorf("got %v", events) + } +} + +func TestParseIntentCloseWithReason(t *testing.T) { + events := ParseIntent(`close bw-xyz reason="duplicate"`, testTime) + if len(events) != 1 || events[0].Type != "close" { + t.Fatalf("got %v", events) + } + if events[0].Detail == "" { + t.Error("expected detail with reason") + } +} + +func TestParseIntentStart(t *testing.T) { + events := ParseIntent(`start bw-1 assignee="alice"`, testTime) + if len(events) != 1 || events[0].Type != "start" || events[0].ID != "bw-1" { + t.Errorf("got %v", events) + } +} + +func TestParseIntentUpdate(t *testing.T) { + events := ParseIntent(`update bw-1 priority=1`, testTime) + if len(events) != 1 || events[0].Type != "update" { + t.Errorf("got %v", events) + } +} + +func TestParseIntentReopen(t *testing.T) { + events := ParseIntent("reopen bw-1", testTime) + if len(events) != 1 || events[0].Type != "reopen" { + t.Errorf("got %v", events) + } +} + +func TestParseIntentDefer(t *testing.T) { + events := ParseIntent("defer bw-1 until 2026-06-01", testTime) + if len(events) != 1 || events[0].Type != "defer" { + t.Errorf("got %v", events) + } +} + +func TestParseIntentUndefer(t *testing.T) { + events := ParseIntent("undefer bw-1", testTime) + if len(events) != 1 || events[0].Type != "undefer" { + t.Errorf("got %v", events) + } +} + +func TestParseIntentComment(t *testing.T) { + events := ParseIntent(`comment bw-1 "Fixed it"`, testTime) + if len(events) != 1 || events[0].Type != "comment" { + t.Errorf("got %v", events) + } +} + +func TestParseIntentLink(t *testing.T) { + events := ParseIntent("link bw-1 blocks bw-2", testTime) + if len(events) != 1 || events[0].Type != "link" || events[0].Detail != "blocks bw-2" { + t.Errorf("got %v", events) + } +} + +func TestParseIntentUnlink(t *testing.T) { + events := ParseIntent("unlink bw-1 blocks bw-2", testTime) + if len(events) != 1 || events[0].Type != "unlink" { + t.Errorf("got %v", events) + } +} + +func TestParseIntentDelete(t *testing.T) { + events := ParseIntent("delete bw-1", testTime) + if len(events) != 1 || events[0].Type != "delete" { + t.Errorf("got %v", events) + } +} + +func TestParseIntentLabel(t *testing.T) { + events := ParseIntent("label bw-1 +bug +urgent", testTime) + if len(events) != 1 || events[0].Type != "label" { + t.Errorf("got %v", events) + } +} + +func TestParseIntentUnblockedSecondary(t *testing.T) { + msg := "close bw-1\nunblocked bw-2\nunblocked bw-3" + events := ParseIntent(msg, testTime) + if len(events) != 3 { + t.Fatalf("got %d events, want 3", len(events)) + } + if events[0].Type != "close" || events[0].ID != "bw-1" { + t.Errorf("event[0] = %v", events[0]) + } + if events[1].Type != "unblocked" || events[1].ID != "bw-2" { + t.Errorf("event[1] = %v", events[1]) + } + if events[2].Type != "unblocked" || events[2].ID != "bw-3" { + t.Errorf("event[2] = %v", events[2]) + } +} + +func TestParseIntentReasonContainingUnblocked(t *testing.T) { + // A close reason that contains "unblocked" as a word should NOT + // produce a spurious unblocked event. + msg := `close bw-1 reason="unblocked by external team"` + events := ParseIntent(msg, testTime) + for _, e := range events { + if e.Type == "unblocked" { + t.Errorf("spurious unblocked event from reason text: %v", e) + } + } +} + +func TestParseIntentEmpty(t *testing.T) { + events := ParseIntent("", testTime) + if len(events) != 0 { + t.Errorf("got %d events for empty message", len(events)) + } +} + +func TestParseIntentUnknown(t *testing.T) { + events := ParseIntent("init beadwork", testTime) + if len(events) != 0 { + t.Errorf("got %d events for unknown intent", len(events)) + } +} + +func FuzzParseIntent(f *testing.F) { + f.Add("create bw-1 p2 task \"test\"") + f.Add("close bw-1\nunblocked bw-2") + f.Add("start bw-1 assignee=\"alice\"") + f.Add("update bw-1 priority=0") + f.Add("") + f.Add("garbage input!!!") + f.Fuzz(func(t *testing.T, msg string) { + // Should never panic. + ParseIntent(msg, testTime) + }) +} diff --git a/internal/recap/recap.go b/internal/recap/recap.go new file mode 100644 index 00000000..34986769 --- /dev/null +++ b/internal/recap/recap.go @@ -0,0 +1,120 @@ +// Package recap builds structured activity summaries from beadwork commit +// history. The data model (Recap/Section/Leaf) is renderer-agnostic: a single +// Build produces one model that both tree and JSON renderers consume. +package recap + +import ( + "sort" + "time" + + "github.com/jallum/beadwork/internal/treefs" +) + +// IssueLookup resolves an issue ID to its title. Implementations may return +// "" if the issue has been deleted or is otherwise unavailable. +type IssueLookup interface { + Title(id string) string +} + +// Event represents a single parsed activity from a commit message. +type Event struct { + Type string // "create", "close", "start", "update", "reopen", "defer", "undefer", "comment", "link", "unlink", "unblocked", "delete", "label" + ID string // primary issue ID + Time time.Time // commit timestamp + Detail string // additional context (title, reason, etc.) +} + +// Leaf is a single event in the recap tree. +type Leaf struct { + Type string `json:"type"` + ID string `json:"id"` + Time string `json:"time"` + Detail string `json:"detail,omitempty"` +} + +// Section groups events for a single issue. +type Section struct { + ID string `json:"id"` + Title string `json:"title"` + Leaves []Leaf `json:"events"` +} + +// Recap is the top-level activity summary. +type Recap struct { + Window Window `json:"window"` + Sections []Section `json:"sections"` +} + +// Build constructs a Recap from commits within the given window. +// It parses each commit's intent, deduplicates events, groups by issue, +// and resolves titles via the lookup. +func Build(commits []treefs.CommitInfo, w Window, lookup IssueLookup) Recap { + var events []Event + seen := make(map[string]bool) // dedup key: "type:id:time" + + for _, c := range commits { + // Window is [Start, End] inclusive on both ends so that events + // at "now" are captured. + if c.Time.Before(w.Start) || c.Time.After(w.End) { + continue + } + parsed := ParseIntent(c.Message, c.Time) + for _, e := range parsed { + key := e.Type + ":" + e.ID + ":" + e.Time.Format(time.RFC3339) + if seen[key] { + continue + } + seen[key] = true + events = append(events, e) + } + } + + // Group by issue ID. + grouped := make(map[string][]Event) + var order []string + for _, e := range events { + if _, exists := grouped[e.ID]; !exists { + order = append(order, e.ID) + } + grouped[e.ID] = append(grouped[e.ID], e) + } + + // Build sections. + sections := make([]Section, 0, len(order)) + for _, id := range order { + evts := grouped[id] + title := "" + if lookup != nil { + title = lookup.Title(id) + } + + leaves := make([]Leaf, 0, len(evts)) + for _, e := range evts { + leaves = append(leaves, Leaf{ + Type: e.Type, + ID: e.ID, + Time: e.Time.UTC().Format(time.RFC3339), + Detail: e.Detail, + }) + } + + sections = append(sections, Section{ + ID: id, + Title: title, + Leaves: leaves, + }) + } + + // Sort sections by first event time. + sort.Slice(sections, func(i, j int) bool { + if len(sections[i].Leaves) == 0 || len(sections[j].Leaves) == 0 { + return false + } + return sections[i].Leaves[0].Time < sections[j].Leaves[0].Time + }) + + return Recap{ + Window: w, + Sections: sections, + } +} diff --git a/internal/recap/recap_test.go b/internal/recap/recap_test.go new file mode 100644 index 00000000..3fbb49b6 --- /dev/null +++ b/internal/recap/recap_test.go @@ -0,0 +1,116 @@ +package recap + +import ( + "testing" + "time" + + "github.com/jallum/beadwork/internal/treefs" +) + +type fakeLookup struct { + titles map[string]string +} + +func (f *fakeLookup) Title(id string) string { + return f.titles[id] +} + +func TestBuildEmpty(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w := Window{Start: now.Add(-24 * time.Hour), End: now, Label: "test"} + r := Build(nil, w, nil) + if len(r.Sections) != 0 { + t.Errorf("expected empty recap, got %d sections", len(r.Sections)) + } +} + +func TestBuildGroupsByIssue(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w := Window{Start: now.Add(-24 * time.Hour), End: now, Label: "test"} + + commits := []treefs.CommitInfo{ + {Message: `create bw-1 p2 task "First"`, Time: now.Add(-2 * time.Hour)}, + {Message: `create bw-2 p2 task "Second"`, Time: now.Add(-1 * time.Hour)}, + {Message: "start bw-1", Time: now.Add(-30 * time.Minute)}, + } + + lookup := &fakeLookup{titles: map[string]string{ + "bw-1": "First", + "bw-2": "Second", + }} + + r := Build(commits, w, lookup) + if len(r.Sections) != 2 { + t.Fatalf("sections = %d, want 2", len(r.Sections)) + } + + // bw-1 should have 2 events (create + start) + s1 := findSection(r, "bw-1") + if s1 == nil { + t.Fatal("section bw-1 not found") + } + if len(s1.Leaves) != 2 { + t.Errorf("bw-1 events = %d, want 2", len(s1.Leaves)) + } + if s1.Title != "First" { + t.Errorf("bw-1 title = %q, want 'First'", s1.Title) + } +} + +func TestBuildFiltersWindow(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w := Window{Start: now.Add(-1 * time.Hour), End: now, Label: "test"} + + commits := []treefs.CommitInfo{ + {Message: "create bw-old", Time: now.Add(-2 * time.Hour)}, // outside window + {Message: "create bw-new", Time: now.Add(-30 * time.Minute)}, + } + + r := Build(commits, w, nil) + if len(r.Sections) != 1 { + t.Fatalf("sections = %d, want 1", len(r.Sections)) + } + if r.Sections[0].ID != "bw-new" { + t.Errorf("section ID = %q, want bw-new", r.Sections[0].ID) + } +} + +func TestBuildDedup(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w := Window{Start: now.Add(-24 * time.Hour), End: now} + ts := now.Add(-1 * time.Hour) + + // Same event appearing in two commits (e.g., replay after sync) + commits := []treefs.CommitInfo{ + {Message: "create bw-1", Time: ts}, + {Message: "create bw-1", Time: ts}, + } + + r := Build(commits, w, nil) + if len(r.Sections) != 1 || len(r.Sections[0].Leaves) != 1 { + t.Errorf("expected 1 section with 1 event (deduped), got %v", r.Sections) + } +} + +func TestBuildUnblockedSecondary(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w := Window{Start: now.Add(-24 * time.Hour), End: now} + + commits := []treefs.CommitInfo{ + {Message: "close bw-1\nunblocked bw-2", Time: now.Add(-1 * time.Hour)}, + } + + r := Build(commits, w, nil) + if len(r.Sections) != 2 { + t.Fatalf("sections = %d, want 2 (bw-1 close + bw-2 unblocked)", len(r.Sections)) + } +} + +func findSection(r Recap, id string) *Section { + for i := range r.Sections { + if r.Sections[i].ID == id { + return &r.Sections[i] + } + } + return nil +} diff --git a/internal/recap/window.go b/internal/recap/window.go new file mode 100644 index 00000000..c4698a0b --- /dev/null +++ b/internal/recap/window.go @@ -0,0 +1,87 @@ +package recap + +import ( + "fmt" + "strings" + "time" +) + +// Window represents a time range for filtering events. +type Window struct { + Start time.Time `json:"start"` + End time.Time `json:"end"` + Label string `json:"label"` +} + +// ParseWindow parses a time window from tokens. Accepted tokens: +// - "today" — start of today in local TZ to now +// - "yesterday" — start of yesterday to start of today (local TZ) +// - "week" — start of this week (Monday) to now +// - "24h" — 24 hours ago to now +// - "7d" — 7 days ago to now +// +// The --since flag overrides with an explicit RFC3339 or date start. +// now is passed explicitly so tests can be deterministic. +func ParseWindow(tokens []string, since string, now time.Time) (Window, error) { + loc := now.Location() + + if since != "" { + start, err := parseSinceDate(since, loc) + if err != nil { + return Window{}, fmt.Errorf("invalid --since: %w", err) + } + return Window{Start: start, End: now, Label: "since " + since}, nil + } + + token := "24h" + if len(tokens) > 0 { + token = strings.ToLower(strings.Join(tokens, " ")) + } + + switch token { + case "today": + start := startOfDay(now, loc) + return Window{Start: start, End: now, Label: "today"}, nil + case "yesterday": + todayStart := startOfDay(now, loc) + yesterdayStart := todayStart.AddDate(0, 0, -1) + return Window{Start: yesterdayStart, End: todayStart, Label: "yesterday"}, nil + case "week", "this week": + start := startOfWeek(now, loc) + return Window{Start: start, End: now, Label: "this week"}, nil + case "24h": + return Window{Start: now.Add(-24 * time.Hour), End: now, Label: "last 24h"}, nil + case "7d": + return Window{Start: now.AddDate(0, 0, -7), End: now, Label: "last 7 days"}, nil + default: + return Window{}, fmt.Errorf("unknown window %q (expected: today, yesterday, week, 24h, 7d)", token) + } +} + +func startOfDay(t time.Time, loc *time.Location) time.Time { + y, m, d := t.In(loc).Date() + return time.Date(y, m, d, 0, 0, 0, 0, loc) +} + +func startOfWeek(t time.Time, loc *time.Location) time.Time { + local := t.In(loc) + weekday := local.Weekday() + if weekday == time.Sunday { + weekday = 7 + } + daysBack := int(weekday) - int(time.Monday) + y, m, d := local.AddDate(0, 0, -daysBack).Date() + return time.Date(y, m, d, 0, 0, 0, 0, loc) +} + +func parseSinceDate(s string, loc *time.Location) (time.Time, error) { + // Try RFC3339 first. + if t, err := time.Parse(time.RFC3339, s); err == nil { + return t, nil + } + // Try YYYY-MM-DD in local TZ. + if t, err := time.ParseInLocation("2006-01-02", s, loc); err == nil { + return t, nil + } + return time.Time{}, fmt.Errorf("expected RFC3339 or YYYY-MM-DD, got %q", s) +} diff --git a/internal/recap/window_test.go b/internal/recap/window_test.go new file mode 100644 index 00000000..8d9c5b92 --- /dev/null +++ b/internal/recap/window_test.go @@ -0,0 +1,175 @@ +package recap + +import ( + "testing" + "time" +) + +func TestWindowToday(t *testing.T) { + // Wednesday 2026-01-15 at 10:00 UTC + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w, err := ParseWindow([]string{"today"}, "", now) + if err != nil { + t.Fatal(err) + } + if w.Label != "today" { + t.Errorf("label = %q", w.Label) + } + wantStart := time.Date(2026, 1, 15, 0, 0, 0, 0, time.UTC) + if !w.Start.Equal(wantStart) { + t.Errorf("start = %v, want %v", w.Start, wantStart) + } + if !w.End.Equal(now) { + t.Errorf("end = %v, want %v", w.End, now) + } +} + +func TestWindowYesterday(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w, err := ParseWindow([]string{"yesterday"}, "", now) + if err != nil { + t.Fatal(err) + } + wantStart := time.Date(2026, 1, 14, 0, 0, 0, 0, time.UTC) + wantEnd := time.Date(2026, 1, 15, 0, 0, 0, 0, time.UTC) + if !w.Start.Equal(wantStart) { + t.Errorf("start = %v, want %v", w.Start, wantStart) + } + if !w.End.Equal(wantEnd) { + t.Errorf("end = %v, want %v", w.End, wantEnd) + } +} + +func TestWindowWeek(t *testing.T) { + // Wednesday 2026-01-15 → Monday 2026-01-12 + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w, err := ParseWindow([]string{"week"}, "", now) + if err != nil { + t.Fatal(err) + } + wantStart := time.Date(2026, 1, 12, 0, 0, 0, 0, time.UTC) + if !w.Start.Equal(wantStart) { + t.Errorf("start = %v, want %v", w.Start, wantStart) + } +} + +func TestWindow24h(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w, err := ParseWindow([]string{"24h"}, "", now) + if err != nil { + t.Fatal(err) + } + wantStart := now.Add(-24 * time.Hour) + if !w.Start.Equal(wantStart) { + t.Errorf("start = %v, want %v", w.Start, wantStart) + } +} + +func TestWindow7d(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w, err := ParseWindow([]string{"7d"}, "", now) + if err != nil { + t.Fatal(err) + } + wantStart := now.AddDate(0, 0, -7) + if !w.Start.Equal(wantStart) { + t.Errorf("start = %v, want %v", w.Start, wantStart) + } +} + +func TestWindowDefault24h(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w, err := ParseWindow(nil, "", now) + if err != nil { + t.Fatal(err) + } + if w.Label != "last 24h" { + t.Errorf("default label = %q, want 'last 24h'", w.Label) + } +} + +func TestWindowSinceRFC3339(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w, err := ParseWindow(nil, "2026-01-10T00:00:00Z", now) + if err != nil { + t.Fatal(err) + } + wantStart := time.Date(2026, 1, 10, 0, 0, 0, 0, time.UTC) + if !w.Start.Equal(wantStart) { + t.Errorf("start = %v, want %v", w.Start, wantStart) + } +} + +func TestWindowSinceDate(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + w, err := ParseWindow(nil, "2026-01-10", now) + if err != nil { + t.Fatal(err) + } + if w.Start.Year() != 2026 || w.Start.Month() != 1 || w.Start.Day() != 10 { + t.Errorf("start = %v", w.Start) + } +} + +func TestWindowSinceInvalid(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + _, err := ParseWindow(nil, "not-a-date", now) + if err == nil { + t.Error("expected error for invalid --since") + } +} + +func TestWindowUnknownToken(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + _, err := ParseWindow([]string{"fortnight"}, "", now) + if err == nil { + t.Error("expected error for unknown token") + } +} + +func TestWindowTodayLocalTZ(t *testing.T) { + // Ensure "today" uses the local timezone of now, not UTC. + eastern := time.FixedZone("EST", -5*3600) + // 2026-01-15 01:00 EST = 2026-01-15 06:00 UTC + now := time.Date(2026, 1, 15, 1, 0, 0, 0, eastern) + w, err := ParseWindow([]string{"today"}, "", now) + if err != nil { + t.Fatal(err) + } + // Start of today in EST should be 2026-01-15 00:00 EST + wantStart := time.Date(2026, 1, 15, 0, 0, 0, 0, eastern) + if !w.Start.Equal(wantStart) { + t.Errorf("start = %v, want %v", w.Start, wantStart) + } +} + +func TestWindowYesterdayLocalTZ(t *testing.T) { + // At 2026-01-15 01:00 EST, yesterday starts at 2026-01-14 00:00 EST + eastern := time.FixedZone("EST", -5*3600) + now := time.Date(2026, 1, 15, 1, 0, 0, 0, eastern) + w, err := ParseWindow([]string{"yesterday"}, "", now) + if err != nil { + t.Fatal(err) + } + wantStart := time.Date(2026, 1, 14, 0, 0, 0, 0, eastern) + wantEnd := time.Date(2026, 1, 15, 0, 0, 0, 0, eastern) + if !w.Start.Equal(wantStart) { + t.Errorf("start = %v, want %v", w.Start, wantStart) + } + if !w.End.Equal(wantEnd) { + t.Errorf("end = %v, want %v", w.End, wantEnd) + } +} + +func FuzzParseWindow(f *testing.F) { + f.Add("today") + f.Add("yesterday") + f.Add("week") + f.Add("24h") + f.Add("7d") + f.Add("garbage") + f.Fuzz(func(t *testing.T, token string) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + ParseWindow([]string{token}, "", now) + }) +} diff --git a/internal/repo/recap_cursor.go b/internal/repo/recap_cursor.go new file mode 100644 index 00000000..4fba8245 --- /dev/null +++ b/internal/repo/recap_cursor.go @@ -0,0 +1,43 @@ +package repo + +import ( + "os" + "path/filepath" + "strings" + "time" +) + +const recapCursorRef = "refs/beadwork/recap-cursor" + +// RecapCursor returns the commit hash stored in the recap cursor ref, +// or "" if no cursor has been set. The ref is local-only (never pushed). +func (r *Repo) RecapCursor() string { + path := filepath.Join(r.GitDir, recapCursorRef) + data, err := os.ReadFile(path) + if err != nil { + return "" + } + return strings.TrimSpace(string(data)) +} + +// LastRecapAt returns the mtime of the recap cursor ref file, which +// records when the cursor was last advanced. Returns the zero time if +// the ref doesn't exist. +func (r *Repo) LastRecapAt() time.Time { + path := filepath.Join(r.GitDir, recapCursorRef) + info, err := os.Stat(path) + if err != nil { + return time.Time{} + } + return info.ModTime() +} + +// SetRecapCursor writes a commit hash to the recap cursor ref. +// Creates parent directories as needed. +func (r *Repo) SetRecapCursor(hash string) error { + path := filepath.Join(r.GitDir, recapCursorRef) + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return err + } + return os.WriteFile(path, []byte(hash+"\n"), 0644) +} diff --git a/test/acceptance_test.go b/test/acceptance_test.go index edf9976e..ca5ecc39 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -106,6 +106,22 @@ func (e *bwEnv) bw(args ...string) string { return stdout.String() } +// bwCapture runs a bw command and returns stdout + stderr separately. +func (e *bwEnv) bwCapture(args ...string) (string, string) { + e.t.Helper() + cmd := exec.Command(bwBin, args...) + cmd.Dir = e.dir + cmd.Env = e.env + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + e.t.Fatalf("bw %s:\nstdout: %s\nstderr: %s\nerr: %v", + strings.Join(args, " "), stdout.String(), stderr.String(), err) + } + return stdout.String(), stderr.String() +} + // bwFail runs a bw command that is expected to fail. // Returns combined stdout+stderr. Fatals if the command succeeds. func (e *bwEnv) bwFail(args ...string) string { @@ -422,6 +438,249 @@ func TestWorktreeRegistersSameAsMain(t *testing.T) { } } +// TestRecapEmpty verifies recap output when there's no activity. +func TestRecapEmpty(t *testing.T) { + env := newBwEnv(t) + out := env.bw("recap", "today") + if !strings.Contains(out, "caught up") { + t.Errorf("expected 'caught up' message:\n%s", out) + } +} + +// TestRecapWithEvents verifies recap groups events by issue. +func TestRecapWithEvents(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "Task Alpha", "--id", "re-1") + env.bw("start", "re-1") + env.bw("close", "re-1") + + out := env.bw("recap", "today") + if !strings.Contains(out, "re-1") { + t.Errorf("recap missing re-1:\n%s", out) + } + if !strings.Contains(out, "Task Alpha") { + t.Errorf("recap missing title 'Task Alpha':\n%s", out) + } +} + +// TestRecapJSON verifies JSON output format and scope field. +func TestRecapJSON(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "json target", "--id", "rj-1") + + out := env.bw("recap", "today", "--json") + if !strings.Contains(out, `"scope": "single"`) { + t.Errorf("recap --json missing scope=single:\n%s", out) + } + if !strings.Contains(out, `"rj-1"`) { + t.Errorf("recap --json missing issue id:\n%s", out) + } +} + +// TestRecapDryRun verifies --dry-run doesn't advance the cursor. +func TestRecapDryRun(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "dry", "--id", "dr-1") + + env.bw("recap", "today", "--dry-run") + contents := env.registryContents() + if strings.Contains(contents, `"cursor"`) { + t.Errorf("--dry-run should not have set a cursor:\n%s", contents) + } +} + +// TestRecapAdvancesCursor verifies that a non-dry-run recap advances the cursor. +func TestRecapAdvancesCursor(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "normal", "--id", "cr-1") + + env.bw("recap", "today") + contents := env.registryContents() + if !strings.Contains(contents, `"cursor"`) { + t.Errorf("recap should advance cursor:\n%s", contents) + } +} + +// TestRecapSince verifies the --since flag. +func TestRecapSince(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "src", "--id", "sn-1") + + out := env.bw("recap", "--since", "2026-01-01") + if !strings.Contains(out, "sn-1") { + t.Errorf("recap --since missing event:\n%s", out) + } +} + +// TestRecapSinceInvalid verifies rejection of a bad --since value. +func TestRecapSinceInvalid(t *testing.T) { + env := newBwEnv(t) + out := env.bwFail("recap", "--since", "not-a-date") + if !strings.Contains(out, "invalid") { + t.Errorf("expected error for invalid --since:\n%s", out) + } +} + +// TestRecapASCII verifies that --ascii uses plain tree characters. +func TestRecapASCII(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "ascii test", "--id", "as-1") + + out := env.bw("recap", "today", "--ascii") + // ASCII tree should use | and ` and -, not ├ └ │ + if strings.ContainsRune(out, '├') || strings.ContainsRune(out, '│') { + t.Errorf("--ascii output contains unicode box chars:\n%s", out) + } +} + +// TestRecapFirstRecap24h verifies first-recap uses 24h backfill when no cursor. +func TestRecapFirstRecap24h(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "recent", "--id", "fr-1") + + out := env.bw("recap") + if !strings.Contains(out, "first recap") { + t.Errorf("first recap should show backfill label:\n%s", out) + } +} + +// TestRecapFromSubdir verifies recap walks up to find the repo. +func TestRecapFromSubdir(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "from sub", "--id", "sb-1") + + sub := filepath.Join(env.dir, "a", "b") + os.MkdirAll(sub, 0755) + + out := env.bwAt(sub, "recap", "today") + if !strings.Contains(out, "sb-1") { + t.Errorf("recap from subdir missing event:\n%s", out) + } +} + +// TestRecapNotInRepo verifies error when not in a git repo. +func TestRecapNotInRepo(t *testing.T) { + dir := t.TempDir() + env := &bwEnv{ + t: t, + dir: dir, + registryDir: t.TempDir(), + env: append(os.Environ(), + "BW_CLOCK="+fixedClock, + "NO_COLOR=1", + ), + } + out := env.bwFail("recap") + if !strings.Contains(out, "not a git repository") { + t.Errorf("expected 'not a git repository' error:\n%s", out) + } +} + +// TestRecapHelp verifies the recap help output. +func TestRecapHelp(t *testing.T) { + env := newBwEnv(t) + out := env.bw("recap", "--help") + for _, flag := range []string{"--since", "--dry-run", "--all", "--json", "--ascii"} { + if !strings.Contains(out, flag) { + t.Errorf("recap help missing %s:\n%s", flag, out) + } + } +} + +// TestRecapInBwHelp verifies recap appears in top-level help. +func TestRecapInBwHelp(t *testing.T) { + env := newBwEnv(t) + out := env.bw("--help") + if !strings.Contains(out, "recap") { + t.Errorf("bw --help missing recap:\n%s", out) + } +} + +// TestRecapAllThreeHealthy verifies cross-repo recap over 3 healthy repos. +func TestRecapAllThreeHealthy(t *testing.T) { + envs := newMultiRepoEnv(t, 3) + + // Create activity in each repo. + envs[0].bw("create", "Alpha", "--id", "a-1") + envs[1].bw("create", "Beta", "--id", "b-1") + envs[2].bw("create", "Gamma", "--id", "g-1") + + out := envs[0].bw("recap", "today", "--all") + for _, id := range []string{"a-1", "b-1", "g-1"} { + if !strings.Contains(out, id) { + t.Errorf("cross-repo recap missing %s:\n%s", id, out) + } + } + if !strings.Contains(out, "3 repo") { + t.Errorf("expected '3 repo(s)' summary:\n%s", out) + } +} + +// TestRecapAllWarnsOnMissing verifies that missing repos warn on stderr +// and get skipped rather than failing. +func TestRecapAllWarnsOnMissing(t *testing.T) { + envs := newMultiRepoEnv(t, 2) + envs[0].bw("create", "Real", "--id", "re-1") + + // Seed an extra registry entry for a nonexistent repo. + path := filepath.Join(envs[0].registryDir, "registry.json") + existing, _ := os.ReadFile(path) + // Add a missing repo to the existing registry JSON. + modified := strings.Replace(string(existing), `"repos": {`, + `"repos": { + "/nonexistent/path": {"last_seen_at": "2026-01-15T10:00:00Z"},`, 1) + os.WriteFile(path, []byte(modified), 0644) + + stdout, stderr := envs[0].bwCapture("recap", "today", "--all") + if !strings.Contains(stderr, "skipping") || !strings.Contains(stderr, "/nonexistent/path") { + t.Errorf("expected 'skipping' warning for missing repo on stderr:\n%s", stderr) + } + if !strings.Contains(stdout, "re-1") { + t.Errorf("healthy repo activity missing from stdout:\n%s", stdout) + } +} + +// TestRecapAllWarnsOnCFlag verifies that -C is warned about with --all. +func TestRecapAllWarnsOnCFlag(t *testing.T) { + envs := newMultiRepoEnv(t, 2) + envs[0].bw("create", "A", "--id", "c-1") + + _, stderr := envs[0].bwCapture("-C", envs[0].dir, "recap", "today", "--all") + if !strings.Contains(stderr, "-C is ignored with --all") { + t.Errorf("expected '-C ignored' warning:\n%s", stderr) + } +} + +// TestRecapAllJSONScope verifies the --json shape has scope=cross. +func TestRecapAllJSONScope(t *testing.T) { + envs := newMultiRepoEnv(t, 2) + envs[0].bw("create", "J", "--id", "j-1") + + out := envs[0].bw("recap", "today", "--all", "--json") + if !strings.Contains(out, `"scope": "cross"`) { + t.Errorf("cross-repo --json missing scope=cross:\n%s", out) + } + if !strings.Contains(out, `"repos"`) { + t.Errorf("cross-repo --json missing repos array:\n%s", out) + } +} + +// TestRecapAllAdvancesPerRepoCursors verifies each repo gets its own cursor advance. +func TestRecapAllAdvancesPerRepoCursors(t *testing.T) { + envs := newMultiRepoEnv(t, 2) + envs[0].bw("create", "A", "--id", "ca-1") + envs[1].bw("create", "B", "--id", "cb-1") + + envs[0].bw("recap", "--all") + + contents := envs[0].registryContents() + // Both repos should now have a "cursor" field. + cursorCount := strings.Count(contents, `"cursor"`) + if cursorCount < 2 { + t.Errorf("expected 2 cursors after recap --all, got %d:\n%s", cursorCount, contents) + } +} + // TestRegistryList verifies the registry list command shows registered repos. func TestRegistryList(t *testing.T) { env := newBwEnv(t) From d5abb3945943184b8286e91e5a7bbafe3eb5ee3e Mon Sep 17 00:00:00 2001 From: Manuel Zubieta Date: Sun, 12 Apr 2026 21:12:32 -0400 Subject: [PATCH 13/19] Polish recap output - Condensed default view with duration tokens (15m, 1h, 3h30m, 2d, 1w) - Colorize both condensed and verbose tree renderers - Fix incremental recap (since-last) window bookkeeping - Truncate long titles; suppress TTY hints when piped - Stamp last_recap_at on each run so the "since last recap" label updates monotonically - Honor local timezone for today/yesterday windows --- cmd/bw/command.go | 8 +- cmd/bw/main.go | 4 + cmd/bw/recap_render.go | 274 ++++++++++++++++++++++++++++++++-- internal/recap/window.go | 102 ++++++++++++- internal/recap/window_test.go | 43 +++++- test/acceptance_test.go | 250 ++++++++++++++++++++++++++++++- 6 files changed, 658 insertions(+), 23 deletions(-) diff --git a/cmd/bw/command.go b/cmd/bw/command.go index 27005f28..23dddc0d 100644 --- a/cmd/bw/command.go +++ b/cmd/bw/command.go @@ -476,17 +476,21 @@ var commands = []Command{ { Name: "recap", Summary: "Show recent activity across issues", - Description: "Summarize beadwork activity in this repo (or --all for every registered repo).\nBy default, shows activity since the last recap — first-time recaps show the last 24 hours.\n\nWindow tokens: today, yesterday, week, 24h, 7d. Use --since for an explicit start.\n--dry-run shows activity without advancing the cursor.", + Description: "Summarize beadwork activity in this repo (or --all for every registered repo).\nBy default, shows activity since the last recap — first-time recaps show the last 24 hours.\nOutput is condensed (one line per issue). Use --verbose for per-event detail.\n\nWindow tokens:\n today, yesterday, week\n durations: 15m, 1h, 3h30m, 24h, 2d, 7d, 2w\nUse --since for an explicit start (RFC3339 or YYYY-MM-DD).\n--dry-run shows activity without advancing the cursor.", Flags: []Flag{ {Long: "--since", Value: "DATE", Help: "Start time (RFC3339 or YYYY-MM-DD)"}, {Long: "--dry-run", Help: "Show activity without advancing the cursor"}, {Long: "--all", Help: "Recap every registered repository"}, + {Long: "--verbose", Short: "-v", Help: "Per-event detail tree (default is condensed)"}, {Long: "--json", Help: "Output as JSON"}, - {Long: "--ascii", Help: "Use plain ASCII tree characters"}, + {Long: "--ascii", Help: "Use plain ASCII tree characters (with --verbose)"}, }, Examples: []Example{ {Cmd: "bw recap", Help: "Activity since last recap (or 24h if first-time)"}, + {Cmd: "bw recap 15m", Help: "Last 15 minutes"}, + {Cmd: "bw recap 1h"}, {Cmd: "bw recap today"}, + {Cmd: "bw recap 7d --verbose", Help: "Full per-event tree"}, {Cmd: "bw recap week --json"}, {Cmd: "bw recap --since 2026-01-01"}, {Cmd: "bw recap --all", Help: "Across all registered repos"}, diff --git a/cmd/bw/main.go b/cmd/bw/main.go index 89579101..0f2cf6f5 100644 --- a/cmd/bw/main.go +++ b/cmd/bw/main.go @@ -137,6 +137,10 @@ func main() { } // bwNow returns the current time respecting BW_CLOCK. +// The returned Time preserves its original location (local time when no +// BW_CLOCK is set; whatever offset BW_CLOCK carries otherwise) so that +// day-boundary math ("today", "yesterday") uses the user's local zone. +// Callers that need UTC for storage should .UTC() themselves. func bwNow() time.Time { if v := os.Getenv("BW_CLOCK"); v != "" { if t, err := time.Parse(time.RFC3339, v); err == nil { diff --git a/cmd/bw/recap_render.go b/cmd/bw/recap_render.go index c19b8742..e5f9efe6 100644 --- a/cmd/bw/recap_render.go +++ b/cmd/bw/recap_render.go @@ -3,6 +3,9 @@ package main import ( "encoding/json" "fmt" + "sort" + "strings" + "time" "github.com/jallum/beadwork/internal/recap" ) @@ -12,7 +15,10 @@ func renderRecap(w Writer, r recap.Recap, ra recapArgs) error { if ra.JSON { return renderRecapJSON(w, r) } - return renderRecapTree(w, r, ra.ASCII) + if ra.Verbose { + return renderRecapTree(w, r, ra.ASCII) + } + return renderRecapCondensed(w, r) } func renderRecapJSON(w Writer, r recap.Recap) error { @@ -28,10 +34,252 @@ func renderRecapJSON(w Writer, r recap.Recap) error { return nil } +// sectionSummary rolls up a section into a compact summary string and the +// marker character (open/in_progress/closed). +// +// Buckets: +// - "state" events that matter for a headline: close, reopen, start, create +// - "notes": comment, label +// - "edits": update, link, unlink, defer, undefer +// - "tangential": unblocked, delete +// +// The summary leads with the most-significant state change within the window, +// then appends quiet counts for everything else ("+ 2 comments, 3 edits"). +func sectionSummary(s recap.Section) (marker, summary string, latest time.Time) { + var closed, reopened, started, created bool + var comments, edits, labels, unblocked int + + for _, l := range s.Leaves { + t, _ := time.Parse(time.RFC3339, l.Time) + if t.After(latest) { + latest = t + } + switch l.Type { + case "close": + closed = true + case "reopen": + reopened = true + case "start": + started = true + case "create": + created = true + case "comment": + comments++ + case "label": + labels++ + case "unblocked": + unblocked++ + case "update", "link", "unlink", "defer", "undefer": + edits++ + } + } + + // Headline: the most significant state change. + var parts []string + switch { + case closed: + parts = append(parts, "closed") + marker = "●" + case started: + parts = append(parts, "started") + marker = "◐" + case reopened: + parts = append(parts, "reopened") + marker = "◐" + case created: + parts = append(parts, "created") + marker = "○" + default: + marker = "·" + } + + // Quiet counts. + if comments > 0 { + parts = append(parts, pluralize(comments, "comment")) + } + if edits > 0 { + parts = append(parts, pluralize(edits, "edit")) + } + if labels > 0 { + parts = append(parts, pluralize(labels, "label change")) + } + if unblocked > 0 { + parts = append(parts, pluralize(unblocked, "unblocked")) + } + + if len(parts) == 0 { + summary = fmt.Sprintf("%d event(s)", len(s.Leaves)) + } else { + summary = strings.Join(parts, ", ") + } + return marker, summary, latest +} + +func pluralize(n int, noun string) string { + if n == 1 { + return fmt.Sprintf("1 %s", noun) + } + return fmt.Sprintf("%d %ss", n, noun) +} + +// markerStyle maps a summary marker to the ANSI style used to color it. +func markerStyle(marker string) Style { + switch marker { + case "●": + return Green // closed + case "◐": + return Yellow // started / reopened + case "○": + return Cyan // created + default: + return Dim + } +} + +// colorizeSummary applies per-keyword color to the summary phrase so state +// changes jump out from counts. +func colorizeSummary(w Writer, summary string) string { + // Split by ", " and re-assemble with styling. + parts := strings.Split(summary, ", ") + for i, p := range parts { + switch { + case p == "closed": + parts[i] = w.Style(p, Green, Bold) + case p == "started": + parts[i] = w.Style(p, Yellow, Bold) + case p == "reopened": + parts[i] = w.Style(p, Yellow, Bold) + case p == "created": + parts[i] = w.Style(p, Cyan, Bold) + default: + parts[i] = w.Style(p, Dim) + } + } + return strings.Join(parts, w.Style(", ", Dim)) +} + +func renderRecapCondensed(w Writer, r recap.Recap) error { + if len(r.Sections) == 0 { + fmt.Fprintf(w, "Recap: %s — %s\n", + w.Style(r.Window.Label, Cyan), + w.Style("nothing to report", Dim)) + return nil + } + + // Build rows then sort by latest activity, most recent first. + type row struct { + marker, id, title, summary string + latest time.Time + } + rows := make([]row, 0, len(r.Sections)) + for _, s := range r.Sections { + m, summ, lat := sectionSummary(s) + title := s.Title + if title == "" { + title = "(deleted)" + } + rows = append(rows, row{ + marker: m, id: s.ID, title: title, + summary: summ, latest: lat, + }) + } + sort.Slice(rows, func(i, j int) bool { + return rows[i].latest.After(rows[j].latest) + }) + + fmt.Fprintf(w, "Recap: %s %s %s\n", + w.Style(r.Window.Label, Cyan), + w.Style("·", Dim), + w.Style(fmt.Sprintf("%d issue(s)", len(rows)), Dim)) + + // Compute id column width for alignment (pre-style, so ANSI codes + // don't bloat the padding). + idWidth := 0 + for _, r := range rows { + if len(r.id) > idWidth { + idWidth = len(r.id) + } + } + + // Compute available width for the title column so long titles don't + // force the Writer to hard-wrap mid-line. Budget per row: + // " " (2) + marker (1) + " " (1) + id (idWidth) + " " (2) + + // TITLE + " — " (4) + summary + " " (1) + "(age)" + // Reserve a small cushion so we never hit the right edge. + width := w.Width() + now := bwNow() + for _, r := range rows { + age := relativeTimeSince(r.latest, now) + title := r.title + if width > 0 { + fixed := 2 + 1 + 1 + idWidth + 2 + 4 + visibleLen(r.summary) + 1 + len("("+age+")") + budget := width - fixed - 1 // 1-char cushion + if budget < 10 { + budget = 10 + } + if len(title) > budget { + title = title[:budget-1] + "…" + } + } + marker := w.Style(r.marker, markerStyle(r.marker)) + // Pad the id BEFORE styling so alignment is based on visible width. + paddedID := fmt.Sprintf("%-*s", idWidth, r.id) + styledID := w.Style(paddedID, Bold) + summary := colorizeSummary(w, r.summary) + fmt.Fprintf(w, " %s %s %s %s %s %s\n", + marker, + styledID, + title, + w.Style("—", Dim), + summary, + w.Style("("+age+")", Dim), + ) + } + // Only print the interactive hint when output is going to a TTY. + // Keep piped / LLM consumption free of chatty prompts. + if w.IsTTY() { + fmt.Fprintln(w, w.Style("\n (use --verbose for per-event detail)", Dim)) + } + return nil +} + +// visibleLen returns the rune count of s, ignoring nothing — used for +// budget math on strings that have NOT been ANSI-styled yet. +func visibleLen(s string) int { + n := 0 + for range s { + n++ + } + return n +} + +// eventTypeStyle returns the color styling for an event type in the verbose +// tree. State changes stand out; low-signal events are dim. +func eventTypeStyle(t string) []Style { + switch t { + case "close": + return []Style{Green, Bold} + case "start", "reopen": + return []Style{Yellow, Bold} + case "create": + return []Style{Cyan, Bold} + case "unblocked": + return []Style{Cyan} + case "delete": + return []Style{Red, Bold} + case "comment", "label": + return []Style{} // default color + case "update", "link", "unlink", "defer", "undefer": + return []Style{Dim} + default: + return []Style{Dim} + } +} + func renderRecapTree(w Writer, r recap.Recap, ascii bool) error { - fmt.Fprintf(w, "Recap: %s\n", r.Window.Label) + fmt.Fprintf(w, "Recap: %s\n", w.Style(r.Window.Label, Cyan)) if len(r.Sections) == 0 { - fmt.Fprintln(w, " (nothing to report — you're caught up)") + fmt.Fprintln(w, w.Style(" (nothing to report — you're caught up)", Dim)) return nil } @@ -52,9 +300,9 @@ func renderRecapTree(w Writer, r recap.Recap, ascii bool) error { } title := s.Title if title == "" { - title = "(deleted)" + title = w.Style("(deleted)", Dim) } - fmt.Fprintf(w, "%s %s %s\n", marker, s.ID, title) + fmt.Fprintf(w, "%s %s %s\n", w.Style(marker, Dim), w.Style(s.ID, Bold), title) // Indent prefix for leaves. indent := vbar + " " @@ -67,11 +315,19 @@ func renderRecapTree(w Writer, r recap.Recap, ascii bool) error { if j == len(s.Leaves)-1 { leafMarker = last } - detail := leaf.Type + + styles := eventTypeStyle(leaf.Type) + styledType := w.Style(leaf.Type, styles...) + line := styledType if leaf.Detail != "" { - detail += " " + leaf.Detail + line += " " + w.Style(leaf.Detail, Dim) } - fmt.Fprintf(w, "%s%s %s %s\n", indent, leafMarker, leaf.Time, detail) + fmt.Fprintf(w, "%s%s %s %s\n", + w.Style(indent, Dim), + w.Style(leafMarker, Dim), + w.Style(leaf.Time, Dim), + line, + ) } } return nil @@ -107,7 +363,7 @@ func renderCrossRepo(w Writer, all []repoRecap, ra recapArgs) error { for _, rr := range all { fmt.Fprintf(w, "=== %s ===\n", rr.Path) - if err := renderRecapTree(w, rr.Recap, ra.ASCII); err != nil { + if err := renderRecap(w, rr.Recap, ra); err != nil { return err } fmt.Fprintln(w) diff --git a/internal/recap/window.go b/internal/recap/window.go index c4698a0b..df362f8e 100644 --- a/internal/recap/window.go +++ b/internal/recap/window.go @@ -2,6 +2,8 @@ package recap import ( "fmt" + "regexp" + "strconv" "strings" "time" ) @@ -49,13 +51,101 @@ func ParseWindow(tokens []string, since string, now time.Time) (Window, error) { case "week", "this week": start := startOfWeek(now, loc) return Window{Start: start, End: now, Label: "this week"}, nil - case "24h": - return Window{Start: now.Add(-24 * time.Hour), End: now, Label: "last 24h"}, nil - case "7d": - return Window{Start: now.AddDate(0, 0, -7), End: now, Label: "last 7 days"}, nil - default: - return Window{}, fmt.Errorf("unknown window %q (expected: today, yesterday, week, 24h, 7d)", token) } + + if d, ok := parseDurationToken(token); ok { + return Window{Start: now.Add(-d), End: now, Label: "last " + formatDuration(d)}, nil + } + + return Window{}, fmt.Errorf("unknown window %q (expected: today, yesterday, week, or duration like 15m, 1h, 24h, 7d, 2w)", token) +} + +// durationPartRe matches a single duration component like "15m", "3h", "2d", "1w". +var durationPartRe = regexp.MustCompile(`^(\d+)(m|h|d|w)$`) + +// parseDurationToken parses tokens like "15m", "1h", "3h30m", "24h", "7d", "2w". +// Returns (duration, true) on success. Accepts Go's time.ParseDuration syntax +// for minute/hour combinations (so "3h30m" works) and extends with d/w for +// days and weeks. +func parseDurationToken(s string) (time.Duration, bool) { + s = strings.TrimSpace(s) + if s == "" { + return 0, false + } + + // First try Go's built-in parser (handles combinations like 3h30m). + if d, err := time.ParseDuration(s); err == nil && d > 0 { + return d, true + } + + // Fall back to our extended parser that supports d (day) and w (week). + // Split into consecutive chunks. + var total time.Duration + rest := s + for rest != "" { + m := durationPartRe.FindStringSubmatch(rest) + if m == nil { + // Try a multi-part match by scanning. + i := 0 + for i < len(rest) && rest[i] >= '0' && rest[i] <= '9' { + i++ + } + if i == 0 || i == len(rest) { + return 0, false + } + n, err := strconv.Atoi(rest[:i]) + if err != nil { + return 0, false + } + unit := rest[i] + rest = rest[i+1:] + switch unit { + case 'm': + total += time.Duration(n) * time.Minute + case 'h': + total += time.Duration(n) * time.Hour + case 'd': + total += time.Duration(n) * 24 * time.Hour + case 'w': + total += time.Duration(n) * 7 * 24 * time.Hour + default: + return 0, false + } + continue + } + n, _ := strconv.Atoi(m[1]) + switch m[2] { + case "m": + total += time.Duration(n) * time.Minute + case "h": + total += time.Duration(n) * time.Hour + case "d": + total += time.Duration(n) * 24 * time.Hour + case "w": + total += time.Duration(n) * 7 * 24 * time.Hour + } + rest = "" + } + + if total <= 0 { + return 0, false + } + return total, true +} + +// formatDuration renders a Duration as a compact label ("15m", "3h", "2d", "1w"). +func formatDuration(d time.Duration) string { + switch { + case d%(7*24*time.Hour) == 0 && d >= 7*24*time.Hour: + return fmt.Sprintf("%dw", d/(7*24*time.Hour)) + case d%(24*time.Hour) == 0 && d >= 24*time.Hour: + return fmt.Sprintf("%dd", d/(24*time.Hour)) + case d%time.Hour == 0 && d >= time.Hour: + return fmt.Sprintf("%dh", d/time.Hour) + case d%time.Minute == 0 && d >= time.Minute: + return fmt.Sprintf("%dm", d/time.Minute) + } + return d.String() } func startOfDay(t time.Time, loc *time.Location) time.Time { diff --git a/internal/recap/window_test.go b/internal/recap/window_test.go index 8d9c5b92..4ae45543 100644 --- a/internal/recap/window_test.go +++ b/internal/recap/window_test.go @@ -83,8 +83,9 @@ func TestWindowDefault24h(t *testing.T) { if err != nil { t.Fatal(err) } - if w.Label != "last 24h" { - t.Errorf("default label = %q, want 'last 24h'", w.Label) + wantStart := now.Add(-24 * time.Hour) + if !w.Start.Equal(wantStart) { + t.Errorf("default window start = %v, want %v", w.Start, wantStart) } } @@ -161,6 +162,44 @@ func TestWindowYesterdayLocalTZ(t *testing.T) { } } +func TestWindowDurationTokens(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + cases := []struct { + token string + want time.Duration + }{ + {"15m", 15 * time.Minute}, + {"30m", 30 * time.Minute}, + {"1h", 1 * time.Hour}, + {"3h30m", 3*time.Hour + 30*time.Minute}, + {"24h", 24 * time.Hour}, + {"2d", 48 * time.Hour}, + {"1w", 7 * 24 * time.Hour}, + {"2w", 14 * 24 * time.Hour}, + } + for _, tc := range cases { + w, err := ParseWindow([]string{tc.token}, "", now) + if err != nil { + t.Errorf("ParseWindow(%q): %v", tc.token, err) + continue + } + wantStart := now.Add(-tc.want) + if !w.Start.Equal(wantStart) { + t.Errorf("token %q: start = %v, want %v", tc.token, w.Start, wantStart) + } + } +} + +func TestWindowDurationRejects(t *testing.T) { + now := time.Date(2026, 1, 15, 10, 0, 0, 0, time.UTC) + bad := []string{"abc", "1x", ""} + for _, token := range bad { + if _, err := ParseWindow([]string{token}, "", now); err == nil { + t.Errorf("expected error for token %q", token) + } + } +} + func FuzzParseWindow(f *testing.F) { f.Add("today") f.Add("yesterday") diff --git a/test/acceptance_test.go b/test/acceptance_test.go index ca5ecc39..0cc43145 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -442,8 +442,8 @@ func TestWorktreeRegistersSameAsMain(t *testing.T) { func TestRecapEmpty(t *testing.T) { env := newBwEnv(t) out := env.bw("recap", "today") - if !strings.Contains(out, "caught up") { - t.Errorf("expected 'caught up' message:\n%s", out) + if !strings.Contains(out, "nothing to report") { + t.Errorf("expected 'nothing to report' message:\n%s", out) } } @@ -489,6 +489,72 @@ func TestRecapDryRun(t *testing.T) { } } +// TestRecapCursorIsIncremental verifies that after a first recap, a second +// recap with no window flag only shows NEW events, not everything again. +func TestRecapCursorIsIncremental(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "first", "--id", "ic-1") + + // First recap advances the cursor. + out1 := env.bw("recap") + if !strings.Contains(out1, "ic-1") { + t.Fatalf("first recap missing ic-1:\n%s", out1) + } + + // Second recap with no new activity should report nothing. + out2 := env.bw("recap") + if strings.Contains(out2, "ic-1") { + t.Errorf("second recap should not re-report ic-1:\n%s", out2) + } + if !strings.Contains(out2, "nothing to report") { + t.Errorf("second recap should show 'nothing to report':\n%s", out2) + } + + // Create new activity — it must show up on the next recap. + env.bw("create", "second", "--id", "ic-2") + out3 := env.bw("recap") + if !strings.Contains(out3, "ic-2") { + t.Errorf("third recap missing new ic-2:\n%s", out3) + } + if strings.Contains(out3, "ic-1") { + t.Errorf("third recap should not re-show ic-1:\n%s", out3) + } +} + +// TestRecapStampsLastRecapAtWithNoCommits verifies that running recap with +// nothing new still updates last_recap_at, so repeated recaps update the +// "since last recap" label even against an unchanged HEAD. +func TestRecapStampsLastRecapAtWithNoCommits(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "x", "--id", "lr-1") + env.bw("recap") // initial — stamps last_recap_at + + before := env.registryContents() + if !strings.Contains(before, `"last_recap_at"`) { + t.Fatalf("first recap did not set last_recap_at:\n%s", before) + } + + // Run again with nothing new. last_recap_at should still be rewritten + // (same value under BW_CLOCK, but the field must exist and be stamped). + env.bw("recap") + after := env.registryContents() + if !strings.Contains(after, `"last_recap_at"`) { + t.Errorf("second recap lost last_recap_at:\n%s", after) + } +} + +// TestRecapDryRunDoesNotStamp verifies --dry-run leaves last_recap_at alone. +func TestRecapDryRunDoesNotStamp(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "x", "--id", "dr-2") + + env.bw("recap", "--dry-run") + contents := env.registryContents() + if strings.Contains(contents, `"last_recap_at"`) { + t.Errorf("--dry-run should not stamp last_recap_at:\n%s", contents) + } +} + // TestRecapAdvancesCursor verifies that a non-dry-run recap advances the cursor. func TestRecapAdvancesCursor(t *testing.T) { env := newBwEnv(t) @@ -521,18 +587,194 @@ func TestRecapSinceInvalid(t *testing.T) { } } -// TestRecapASCII verifies that --ascii uses plain tree characters. +// TestRecapASCII verifies that --ascii uses plain tree characters +// (only affects --verbose tree output). func TestRecapASCII(t *testing.T) { env := newBwEnv(t) env.bw("create", "ascii test", "--id", "as-1") - out := env.bw("recap", "today", "--ascii") + out := env.bw("recap", "today", "--ascii", "--verbose") // ASCII tree should use | and ` and -, not ├ └ │ if strings.ContainsRune(out, '├') || strings.ContainsRune(out, '│') { t.Errorf("--ascii output contains unicode box chars:\n%s", out) } } +// TestRecapTodayLocalTimezone verifies that "today" honors the caller's +// local timezone, not UTC. Simulates a user at 1am US/Eastern (which is +// 5am UTC): work done the previous local evening (e.g. 10pm ET = 2am UTC +// "today" UTC) should fall into "today" local. +func TestRecapTodayLocalTimezone(t *testing.T) { + // Local wall clock: 2026-01-15 01:00:00 -0500 (EST). + // That's 2026-01-15 06:00:00 UTC — safely inside UTC "today" as well, + // but the start of local "today" is 2026-01-15 00:00:00 -0500 + // (= 2026-01-15 05:00:00 UTC), while start of UTC "today" would be + // 2026-01-15 00:00:00 UTC — a 5-hour difference. Seed an event at + // 2026-01-15 00:30:00 -0500 (= 05:30 UTC): inside local today, + // inside UTC today too. Then seed 2026-01-14 23:30:00 -0500 + // (= 2026-01-15 04:30:00 UTC): inside local *yesterday*, but inside + // UTC *today*. A TZ-correct "today" must EXCLUDE the second event. + envEarlyLocal := "2026-01-15T00:30:00-05:00" // inside local today + envLateYesterdayLocal := "2026-01-14T23:30:00-05:00" + + dir := t.TempDir() + registryDir := t.TempDir() + baseEnv := append(os.Environ(), + "GIT_AUTHOR_NAME=Test", + "GIT_AUTHOR_EMAIL=test@test.com", + "GIT_COMMITTER_NAME=Test", + "GIT_COMMITTER_EMAIL=test@test.com", + "NO_COLOR=1", + "BEADWORK_HOME="+registryDir, + ) + + run := func(clock string, args ...string) string { + cmd := exec.Command(bwBin, args...) + cmd.Dir = dir + cmd.Env = append(baseEnv, + "BW_CLOCK="+clock, + "GIT_AUTHOR_DATE="+clock, + "GIT_COMMITTER_DATE="+clock, + ) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + t.Fatalf("bw %s:\nstderr: %s\nerr: %v", + strings.Join(args, " "), stderr.String(), err) + } + return stdout.String() + } + gitRun := func(args ...string) { + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = baseEnv + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %s: %s: %v", strings.Join(args, " "), out, err) + } + } + + // Setup + gitRun("init") + gitRun("config", "user.email", "test@test.com") + gitRun("config", "user.name", "Test") + os.WriteFile(filepath.Join(dir, "README"), []byte("t"), 0644) + gitRun("add", ".") + gitRun("commit", "-m", "initial") + run("2026-01-15T00:00:00-05:00", "init", "--prefix", "tz") + + // Seed one issue "today local" and one "yesterday local, today UTC". + run(envLateYesterdayLocal, "create", "yday-local", "--id", "ytd-1") + run(envEarlyLocal, "create", "today-local", "--id", "tdy-1") + + // Now ask for today at 1am local on 2026-01-15. + out := run("2026-01-15T01:00:00-05:00", "recap", "today", "--dry-run") + + if !strings.Contains(out, "tdy-1") { + t.Errorf("today-local event missing from 'today' recap:\n%s", out) + } + if strings.Contains(out, "ytd-1") { + t.Errorf("yesterday-local event leaked into 'today' recap (UTC bug):\n%s", out) + } +} + +// TestRecapDurationToken verifies support for duration tokens like 1h, 15m. +func TestRecapDurationToken(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "recent", "--id", "dt-1") + + for _, token := range []string{"1h", "15m", "2d", "1w", "3h30m"} { + out := env.bw("recap", token) + if !strings.Contains(out, "dt-1") { + t.Errorf("recap %s missing event:\n%s", token, out) + } + } +} + +// TestRecapNoANSIWhenPiped verifies that piped output (non-TTY) has no +// ANSI escape sequences. LLM consumers (Claude Code, etc.) rely on this. +// Same treatment as `bw prime`. +func TestRecapNoANSIWhenPiped(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "noansi", "--id", "na-1") + env.bw("start", "na-1") + env.bw("close", "na-1") + + // env.bw() executes bw with stdout captured to a buffer → non-TTY. + out := env.bw("recap", "today") + if strings.ContainsRune(out, '\x1b') { + t.Errorf("recap piped output contains ANSI escape (\\x1b):\n%q", out) + } + if strings.Contains(out, "use --verbose") { + t.Errorf("recap piped output leaks TTY-only hint:\n%s", out) + } + + // Verbose must also be ANSI-free when piped. + vOut := env.bw("recap", "today", "--verbose") + if strings.ContainsRune(vOut, '\x1b') { + t.Errorf("recap --verbose piped output contains ANSI escape:\n%q", vOut) + } +} + +// TestRecapCondensedDefault verifies default output is condensed. +func TestRecapCondensedDefault(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "Task One", "--id", "co-1") + env.bw("start", "co-1") + env.bw("close", "co-1") + env.bw("comment", "co-1", "done") + + out := env.bw("recap", "today") + // Default should be one-line-per-issue (not full tree). + // So it should NOT contain unicode box chars or per-leaf timestamps. + if strings.ContainsRune(out, '├') { + t.Errorf("default output should not be a tree:\n%s", out) + } + // Should contain the issue, title, and a state hint ("closed"). + if !strings.Contains(out, "co-1") || !strings.Contains(out, "Task One") { + t.Errorf("condensed output missing id/title:\n%s", out) + } + if !strings.Contains(out, "closed") { + t.Errorf("condensed output should show 'closed' state:\n%s", out) + } + // Count lines — should be much shorter than verbose. + lines := strings.Count(out, "\n") + if lines > 5 { + t.Errorf("condensed output too long (%d lines):\n%s", lines, out) + } +} + +// TestRecapVerbose verifies --verbose gives the full tree. +func TestRecapVerbose(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "Task One", "--id", "vb-1") + env.bw("start", "vb-1") + env.bw("close", "vb-1") + + out := env.bw("recap", "today", "--verbose") + // Verbose should be a tree — one leaf per event. + if !strings.ContainsRune(out, '├') && !strings.ContainsRune(out, '└') { + t.Errorf("--verbose should render a tree:\n%s", out) + } + // Should contain each event type. + for _, ev := range []string{"create", "start", "close"} { + if !strings.Contains(out, ev) { + t.Errorf("--verbose missing event %q:\n%s", ev, out) + } + } +} + +// TestRecapVerboseShortFlag verifies -v is a shorthand for --verbose. +func TestRecapVerboseShortFlag(t *testing.T) { + env := newBwEnv(t) + env.bw("create", "v test", "--id", "sv-1") + + out := env.bw("recap", "today", "-v") + if !strings.ContainsRune(out, '├') && !strings.ContainsRune(out, '└') { + t.Errorf("-v should render a tree:\n%s", out) + } +} + // TestRecapFirstRecap24h verifies first-recap uses 24h backfill when no cursor. func TestRecapFirstRecap24h(t *testing.T) { env := newBwEnv(t) From ac817d5c1ad01ec94ac26ed9dee4fbec4bfdcb41 Mon Sep 17 00:00:00 2001 From: Manuel Zubieta Date: Thu, 16 Apr 2026 14:43:23 -0400 Subject: [PATCH 14/19] recap: skip cursor advance on gapped explicit windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Explicit-window recaps (--since / today / 1h / etc.) previously stamped the cursor to HEAD via AllCommits, even when the window started after the current cursor. Commits between cursor_time and window.Start — never rendered — would be marked seen and permanently skipped by future implicit recaps. Now: when the window starts after the cursor, the recap renders the window as before but neither `cursor` nor `last_recap_at` is stamped, and a stderr notice reports the count of unrendered gap commits. No-gap explicit recaps and bare cursor-driven recaps keep their existing stamping behavior. --dry-run still skips all stamping. See .spec/decisions/recap-explicit-window-conditional-advance.md. --- test/acceptance_test.go | 157 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) diff --git a/test/acceptance_test.go b/test/acceptance_test.go index 0cc43145..4ae44c1e 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -139,6 +139,44 @@ func (e *bwEnv) bwFail(args ...string) string { return stdout.String() + stderr.String() } +// bwAtClock runs bw with BW_CLOCK (and git commit-date envs) overridden. +// Returns stdout; fatals on non-zero exit. +func (e *bwEnv) bwAtClock(clock string, args ...string) string { + out, _ := e.bwAtClockCapture(clock, args...) + return out +} + +// bwAtClockCapture is bwAtClock that also returns stderr. +func (e *bwEnv) bwAtClockCapture(clock string, args ...string) (string, string) { + e.t.Helper() + cmd := exec.Command(bwBin, args...) + cmd.Dir = e.dir + overridden := make([]string, 0, len(e.env)+3) + for _, kv := range e.env { + switch { + case strings.HasPrefix(kv, "BW_CLOCK="), + strings.HasPrefix(kv, "GIT_AUTHOR_DATE="), + strings.HasPrefix(kv, "GIT_COMMITTER_DATE="): + continue + } + overridden = append(overridden, kv) + } + overridden = append(overridden, + "BW_CLOCK="+clock, + "GIT_AUTHOR_DATE="+clock, + "GIT_COMMITTER_DATE="+clock, + ) + cmd.Env = overridden + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + e.t.Fatalf("bw %s (clock=%s):\nstdout: %s\nstderr: %s\nerr: %v", + strings.Join(args, " "), clock, stdout.String(), stderr.String(), err) + } + return stdout.String(), stderr.String() +} + // bwAt runs bw from a custom directory instead of the default e.dir. func (e *bwEnv) bwAt(dir string, args ...string) string { e.t.Helper() @@ -1140,3 +1178,122 @@ func TestWorktreeRefWrites(t *testing.T) { t.Fatalf("worktree commit not visible in beadwork log from main:\n%s", log) } } + +// TestRecapExplicitWindowGapSkipsAdvance verifies that an explicit window +// that starts AFTER the current cursor leaves both cursor and last_recap_at +// untouched, and prints a gap notice on stderr naming the unrendered count. +// See ADR: recap-explicit-window-conditional-advance. +func TestRecapExplicitWindowGapSkipsAdvance(t *testing.T) { + env := newBwEnv(t) + + const ( + day1 = "2026-04-10T09:00:00Z" // cursor gets set here + day2 = "2026-04-12T09:00:00Z" // gap commit (older than window, newer than cursor) + day3 = "2026-04-14T12:00:00Z" // explicit "today" is day3 midnight → day3 noon + ) + + // Day 1: create an issue, run bare recap → cursor advances to this commit. + env.bwAtClock(day1, "create", "gap-origin", "--id", "gp-1") + env.bwAtClock(day1, "recap") + cursor1 := registryField(t, env.registryContents(), "cursor") + lastRecap1 := registryField(t, env.registryContents(), "last_recap_at") + if cursor1 == "" { + t.Fatalf("day1 bare recap did not stamp cursor:\n%s", env.registryContents()) + } + if lastRecap1 == "" { + t.Fatalf("day1 bare recap did not stamp last_recap_at:\n%s", env.registryContents()) + } + + // Day 2: another commit lands. This is the "gap" — it's newer than the + // day-1 cursor and older than the day-3 "today" window. + env.bwAtClock(day2, "create", "gap-middle", "--id", "gp-2") + + // Day 3: explicit `recap today`. Window = day3 00:00 → day3 12:00. The + // day-2 gp-2 commit is in the gap. gp-2 is NOT rendered, cursor must + // NOT advance, stderr must carry the gap notice. + stdout, stderr := env.bwAtClockCapture(day3, "recap", "today") + + // Output: gp-2 should not appear (it's older than the window). + if strings.Contains(stdout, "gp-2") { + t.Errorf("explicit window should not render gap commit gp-2:\n%s", stdout) + } + + // Stderr notice must mention the gap count (1). + if !strings.Contains(stderr, "1 commit older than this window") { + t.Errorf("expected gap notice on stderr, got:\n%s", stderr) + } + if !strings.Contains(stderr, "bw recap") { + t.Errorf("gap notice should reference 'bw recap':\n%s", stderr) + } + + // Registry: cursor + last_recap_at must still match the day-1 snapshot. + // (last_seen_at moves on every command via the auto-register hook; that's + // orthogonal to the recap-stamp behavior under test.) + cursor3 := registryField(t, env.registryContents(), "cursor") + lastRecap3 := registryField(t, env.registryContents(), "last_recap_at") + if cursor3 != cursor1 { + t.Errorf("gapped explicit run advanced cursor\n was: %s\n now: %s", cursor1, cursor3) + } + if lastRecap3 != lastRecap1 { + t.Errorf("gapped explicit run stamped last_recap_at\n was: %s\n now: %s", lastRecap1, lastRecap3) + } +} + +// registryField extracts a top-level string field from a single-repo +// registry JSON blob. Test helper only — assumes exactly one repo entry. +func registryField(t *testing.T, raw, field string) string { + t.Helper() + needle := `"` + field + `":` + i := strings.Index(raw, needle) + if i < 0 { + return "" + } + rest := raw[i+len(needle):] + // Skip whitespace then expect a quoted string. + for len(rest) > 0 && (rest[0] == ' ' || rest[0] == '\t' || rest[0] == '\n') { + rest = rest[1:] + } + if len(rest) == 0 || rest[0] != '"' { + return "" + } + rest = rest[1:] + end := strings.Index(rest, `"`) + if end < 0 { + return "" + } + return rest[:end] +} + +// TestRecapExplicitWindowNoGapAdvances verifies that an explicit window that +// covers the full unseen range (window.Start <= cursor_time) advances the +// cursor to HEAD exactly like a bare recap. +func TestRecapExplicitWindowNoGapAdvances(t *testing.T) { + env := newBwEnv(t) + + const ( + day1 = "2026-04-14T03:00:00Z" // cursor here, inside day-1 + day2 = "2026-04-14T05:00:00Z" // new commit, later same day + now = "2026-04-14T12:00:00Z" // `recap today` fires here → window covers both + ) + + env.bwAtClock(day1, "create", "first", "--id", "ng-1") + env.bwAtClock(day1, "recap") + before := env.registryContents() + + env.bwAtClock(day2, "create", "second", "--id", "ng-2") + + // `recap today` at noon. Window start is midnight (before day1 cursor), + // so no gap — cursor should advance and stderr should be clean. + stdout, stderr := env.bwAtClockCapture(now, "recap", "today") + if !strings.Contains(stdout, "ng-2") { + t.Errorf("no-gap explicit recap missing ng-2:\n%s", stdout) + } + if strings.Contains(stderr, "older than this window") { + t.Errorf("no-gap explicit recap should not emit gap notice:\n%s", stderr) + } + + after := env.registryContents() + if after == before { + t.Errorf("no-gap explicit recap should advance cursor; registry unchanged:\n%s", after) + } +} From f530f27502b774eab52f8460a10301f27ead5244 Mon Sep 17 00:00:00 2001 From: Manuel Zubieta Date: Thu, 16 Apr 2026 15:08:04 -0400 Subject: [PATCH 15/19] Remove unused treefs_test helpers containsStr and contains were orphaned when "Record unblocked events" swapped the CAS conflict assertion to errors.Is(err, ErrRefMoved). Staticcheck flags them as U1000. --- internal/treefs/treefs_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/internal/treefs/treefs_test.go b/internal/treefs/treefs_test.go index d011cd4e..86c3f310 100644 --- a/internal/treefs/treefs_test.go +++ b/internal/treefs/treefs_test.go @@ -529,15 +529,3 @@ func TestCommitRespectsBWClockEnv(t *testing.T) { } } -func containsStr(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(s) > 0 && contains(s, substr)) -} - -func contains(s, substr string) bool { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return true - } - } - return false -} From cce1951e1077d31d0e551e38cf82c007465ae566 Mon Sep 17 00:00:00 2001 From: Jason Allum <21417+jallum@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:33:11 -0400 Subject: [PATCH 16/19] Move recap cursor to refs/beadwork/recap-cursor Store the recap cursor as a local git ref instead of in the registry. The ref file's mtime provides LastRecapAt. This keeps per-user recap state local to each repo clone (never replicated via push/fetch). - Add Repo.RecapCursor/SetRecapCursor/LastRecapAt methods - Rewrite recap.go to use repo-based cursor instead of registry - Update acceptance tests to check git ref instead of registry JSON --- test/acceptance_test.go | 157 ++++++++++++++++------------------------ 1 file changed, 63 insertions(+), 94 deletions(-) diff --git a/test/acceptance_test.go b/test/acceptance_test.go index 4ae44c1e..4c40d85c 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -298,6 +298,25 @@ func (e *bwEnv) registryPaths() []string { return cfg.StringSlice("registry.repos") } +// recapCursor reads the recap cursor ref from the git dir. +func (e *bwEnv) recapCursor() string { + e.t.Helper() + path := filepath.Join(e.dir, ".git", "refs", "beadwork", "recap-cursor") + data, err := os.ReadFile(path) + if err != nil { + return "" + } + return strings.TrimSpace(string(data)) +} + +// recapCursorExists returns true if the recap cursor ref file exists. +func (e *bwEnv) recapCursorExists() bool { + e.t.Helper() + path := filepath.Join(e.dir, ".git", "refs", "beadwork", "recap-cursor") + _, err := os.Stat(path) + return err == nil +} + // TestScaffoldingHelpers verifies that the test scaffolding helpers work correctly. func TestScaffoldingHelpers(t *testing.T) { env := newBwEnv(t) @@ -560,36 +579,36 @@ func TestRecapCursorIsIncremental(t *testing.T) { } // TestRecapStampsLastRecapAtWithNoCommits verifies that running recap with -// nothing new still updates last_recap_at, so repeated recaps update the -// "since last recap" label even against an unchanged HEAD. +// nothing new still leaves the cursor ref in place so the "since last recap" +// label can be derived from the ref file's mtime. func TestRecapStampsLastRecapAtWithNoCommits(t *testing.T) { env := newBwEnv(t) env.bw("create", "x", "--id", "lr-1") - env.bw("recap") // initial — stamps last_recap_at + env.bw("recap") - before := env.registryContents() - if !strings.Contains(before, `"last_recap_at"`) { - t.Fatalf("first recap did not set last_recap_at:\n%s", before) + if !env.recapCursorExists() { + t.Fatalf("first recap did not set cursor ref") } + cursor1 := env.recapCursor() - // Run again with nothing new. last_recap_at should still be rewritten - // (same value under BW_CLOCK, but the field must exist and be stamped). + // Run again with nothing new. Cursor ref should still exist. env.bw("recap") - after := env.registryContents() - if !strings.Contains(after, `"last_recap_at"`) { - t.Errorf("second recap lost last_recap_at:\n%s", after) + if !env.recapCursorExists() { + t.Errorf("second recap lost cursor ref") + } + if env.recapCursor() != cursor1 { + t.Errorf("cursor changed with no new commits") } } -// TestRecapDryRunDoesNotStamp verifies --dry-run leaves last_recap_at alone. +// TestRecapDryRunDoesNotStamp verifies --dry-run leaves the cursor ref alone. func TestRecapDryRunDoesNotStamp(t *testing.T) { env := newBwEnv(t) env.bw("create", "x", "--id", "dr-2") env.bw("recap", "--dry-run") - contents := env.registryContents() - if strings.Contains(contents, `"last_recap_at"`) { - t.Errorf("--dry-run should not stamp last_recap_at:\n%s", contents) + if env.recapCursorExists() { + t.Errorf("--dry-run should not create cursor ref") } } @@ -599,9 +618,8 @@ func TestRecapAdvancesCursor(t *testing.T) { env.bw("create", "normal", "--id", "cr-1") env.bw("recap", "today") - contents := env.registryContents() - if !strings.Contains(contents, `"cursor"`) { - t.Errorf("recap should advance cursor:\n%s", contents) + if env.recapCursor() == "" { + t.Errorf("recap should advance cursor") } } @@ -902,14 +920,9 @@ func TestRecapAllWarnsOnMissing(t *testing.T) { envs := newMultiRepoEnv(t, 2) envs[0].bw("create", "Real", "--id", "re-1") - // Seed an extra registry entry for a nonexistent repo. - path := filepath.Join(envs[0].registryDir, "registry.json") - existing, _ := os.ReadFile(path) - // Add a missing repo to the existing registry JSON. - modified := strings.Replace(string(existing), `"repos": {`, - `"repos": { - "/nonexistent/path": {"last_seen_at": "2026-01-15T10:00:00Z"},`, 1) - os.WriteFile(path, []byte(modified), 0644) + // Append a nonexistent path to the registry. + existing := envs[0].registryContents() + os.WriteFile(envs[0].registryDir, []byte(existing+"/nonexistent/path\n"), 0644) stdout, stderr := envs[0].bwCapture("recap", "today", "--all") if !strings.Contains(stderr, "skipping") || !strings.Contains(stderr, "/nonexistent/path") { @@ -953,11 +966,15 @@ func TestRecapAllAdvancesPerRepoCursors(t *testing.T) { envs[0].bw("recap", "--all") - contents := envs[0].registryContents() - // Both repos should now have a "cursor" field. - cursorCount := strings.Count(contents, `"cursor"`) - if cursorCount < 2 { - t.Errorf("expected 2 cursors after recap --all, got %d:\n%s", cursorCount, contents) + // Both repos should now have a cursor ref. + cursors := 0 + for _, e := range envs { + if e.recapCursor() != "" { + cursors++ + } + } + if cursors != 2 { + t.Errorf("expected 2 cursors after recap --all, got %d", cursors) } } @@ -1180,45 +1197,31 @@ func TestWorktreeRefWrites(t *testing.T) { } // TestRecapExplicitWindowGapSkipsAdvance verifies that an explicit window -// that starts AFTER the current cursor leaves both cursor and last_recap_at -// untouched, and prints a gap notice on stderr naming the unrendered count. -// See ADR: recap-explicit-window-conditional-advance. +// that starts AFTER the current cursor leaves the cursor untouched, and +// prints a gap notice on stderr naming the unrendered count. func TestRecapExplicitWindowGapSkipsAdvance(t *testing.T) { env := newBwEnv(t) const ( - day1 = "2026-04-10T09:00:00Z" // cursor gets set here - day2 = "2026-04-12T09:00:00Z" // gap commit (older than window, newer than cursor) - day3 = "2026-04-14T12:00:00Z" // explicit "today" is day3 midnight → day3 noon + day1 = "2026-04-10T09:00:00Z" + day2 = "2026-04-12T09:00:00Z" + day3 = "2026-04-14T12:00:00Z" ) - // Day 1: create an issue, run bare recap → cursor advances to this commit. env.bwAtClock(day1, "create", "gap-origin", "--id", "gp-1") env.bwAtClock(day1, "recap") - cursor1 := registryField(t, env.registryContents(), "cursor") - lastRecap1 := registryField(t, env.registryContents(), "last_recap_at") + cursor1 := env.recapCursor() if cursor1 == "" { - t.Fatalf("day1 bare recap did not stamp cursor:\n%s", env.registryContents()) - } - if lastRecap1 == "" { - t.Fatalf("day1 bare recap did not stamp last_recap_at:\n%s", env.registryContents()) + t.Fatalf("day1 bare recap did not stamp cursor") } - // Day 2: another commit lands. This is the "gap" — it's newer than the - // day-1 cursor and older than the day-3 "today" window. env.bwAtClock(day2, "create", "gap-middle", "--id", "gp-2") - // Day 3: explicit `recap today`. Window = day3 00:00 → day3 12:00. The - // day-2 gp-2 commit is in the gap. gp-2 is NOT rendered, cursor must - // NOT advance, stderr must carry the gap notice. stdout, stderr := env.bwAtClockCapture(day3, "recap", "today") - // Output: gp-2 should not appear (it's older than the window). if strings.Contains(stdout, "gp-2") { t.Errorf("explicit window should not render gap commit gp-2:\n%s", stdout) } - - // Stderr notice must mention the gap count (1). if !strings.Contains(stderr, "1 commit older than this window") { t.Errorf("expected gap notice on stderr, got:\n%s", stderr) } @@ -1226,42 +1229,10 @@ func TestRecapExplicitWindowGapSkipsAdvance(t *testing.T) { t.Errorf("gap notice should reference 'bw recap':\n%s", stderr) } - // Registry: cursor + last_recap_at must still match the day-1 snapshot. - // (last_seen_at moves on every command via the auto-register hook; that's - // orthogonal to the recap-stamp behavior under test.) - cursor3 := registryField(t, env.registryContents(), "cursor") - lastRecap3 := registryField(t, env.registryContents(), "last_recap_at") + cursor3 := env.recapCursor() if cursor3 != cursor1 { t.Errorf("gapped explicit run advanced cursor\n was: %s\n now: %s", cursor1, cursor3) } - if lastRecap3 != lastRecap1 { - t.Errorf("gapped explicit run stamped last_recap_at\n was: %s\n now: %s", lastRecap1, lastRecap3) - } -} - -// registryField extracts a top-level string field from a single-repo -// registry JSON blob. Test helper only — assumes exactly one repo entry. -func registryField(t *testing.T, raw, field string) string { - t.Helper() - needle := `"` + field + `":` - i := strings.Index(raw, needle) - if i < 0 { - return "" - } - rest := raw[i+len(needle):] - // Skip whitespace then expect a quoted string. - for len(rest) > 0 && (rest[0] == ' ' || rest[0] == '\t' || rest[0] == '\n') { - rest = rest[1:] - } - if len(rest) == 0 || rest[0] != '"' { - return "" - } - rest = rest[1:] - end := strings.Index(rest, `"`) - if end < 0 { - return "" - } - return rest[:end] } // TestRecapExplicitWindowNoGapAdvances verifies that an explicit window that @@ -1271,19 +1242,17 @@ func TestRecapExplicitWindowNoGapAdvances(t *testing.T) { env := newBwEnv(t) const ( - day1 = "2026-04-14T03:00:00Z" // cursor here, inside day-1 - day2 = "2026-04-14T05:00:00Z" // new commit, later same day - now = "2026-04-14T12:00:00Z" // `recap today` fires here → window covers both + day1 = "2026-04-14T03:00:00Z" + day2 = "2026-04-14T05:00:00Z" + now = "2026-04-14T12:00:00Z" ) env.bwAtClock(day1, "create", "first", "--id", "ng-1") env.bwAtClock(day1, "recap") - before := env.registryContents() + cursorBefore := env.recapCursor() env.bwAtClock(day2, "create", "second", "--id", "ng-2") - // `recap today` at noon. Window start is midnight (before day1 cursor), - // so no gap — cursor should advance and stderr should be clean. stdout, stderr := env.bwAtClockCapture(now, "recap", "today") if !strings.Contains(stdout, "ng-2") { t.Errorf("no-gap explicit recap missing ng-2:\n%s", stdout) @@ -1292,8 +1261,8 @@ func TestRecapExplicitWindowNoGapAdvances(t *testing.T) { t.Errorf("no-gap explicit recap should not emit gap notice:\n%s", stderr) } - after := env.registryContents() - if after == before { - t.Errorf("no-gap explicit recap should advance cursor; registry unchanged:\n%s", after) + cursorAfter := env.recapCursor() + if cursorAfter == cursorBefore { + t.Errorf("no-gap explicit recap should advance cursor") } } From 756a217d6c1ce5a850da11b74e7e26da1eb6273a Mon Sep 17 00:00:00 2001 From: Jason Allum <21417+jallum@users.noreply.github.com> Date: Thu, 7 May 2026 07:58:37 -0400 Subject: [PATCH 17/19] Fix acceptance tests after registry-v2 rebase registryContents/registryDir/BEADWORK_HOME were artifacts of the old file-based registry; the new design stores repos in BW_CONFIG (YAML) and the recap cursor in refs/beadwork/recap-cursor. - TestRecapDryRun: check git ref instead of registry file contents - TestRecapTodayLocalTimezone: BW_CONFIG= replaces BEADWORK_HOME= - TestRecapNotInRepo: remove registryDir field, add isolated BW_CONFIG - TestRecapAllWarnsOnMissing: seedRegistry() replaces file write --- test/acceptance_test.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/test/acceptance_test.go b/test/acceptance_test.go index 4c40d85c..8fd51832 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -540,9 +540,11 @@ func TestRecapDryRun(t *testing.T) { env.bw("create", "dry", "--id", "dr-1") env.bw("recap", "today", "--dry-run") - contents := env.registryContents() - if strings.Contains(contents, `"cursor"`) { - t.Errorf("--dry-run should not have set a cursor:\n%s", contents) + // Cursor lives in refs/beadwork/recap-cursor — should not exist after --dry-run. + cmd := exec.Command("git", "show-ref", "--verify", "refs/beadwork/recap-cursor") + cmd.Dir = env.dir + if err := cmd.Run(); err == nil { + t.Error("--dry-run should not have set the recap cursor ref") } } @@ -674,14 +676,14 @@ func TestRecapTodayLocalTimezone(t *testing.T) { envLateYesterdayLocal := "2026-01-14T23:30:00-05:00" dir := t.TempDir() - registryDir := t.TempDir() + cfgPathTZ := filepath.Join(t.TempDir(), ".bw") baseEnv := append(os.Environ(), "GIT_AUTHOR_NAME=Test", "GIT_AUTHOR_EMAIL=test@test.com", "GIT_COMMITTER_NAME=Test", "GIT_COMMITTER_EMAIL=test@test.com", "NO_COLOR=1", - "BEADWORK_HOME="+registryDir, + "BW_CONFIG="+cfgPathTZ, ) run := func(clock string, args ...string) string { @@ -860,11 +862,11 @@ func TestRecapFromSubdir(t *testing.T) { func TestRecapNotInRepo(t *testing.T) { dir := t.TempDir() env := &bwEnv{ - t: t, - dir: dir, - registryDir: t.TempDir(), + t: t, + dir: dir, env: append(os.Environ(), "BW_CLOCK="+fixedClock, + "BW_CONFIG="+filepath.Join(t.TempDir(), ".bw"), "NO_COLOR=1", ), } @@ -920,9 +922,8 @@ func TestRecapAllWarnsOnMissing(t *testing.T) { envs := newMultiRepoEnv(t, 2) envs[0].bw("create", "Real", "--id", "re-1") - // Append a nonexistent path to the registry. - existing := envs[0].registryContents() - os.WriteFile(envs[0].registryDir, []byte(existing+"/nonexistent/path\n"), 0644) + // Inject a nonexistent path into the registry. + envs[0].seedRegistry("/nonexistent/path") stdout, stderr := envs[0].bwCapture("recap", "today", "--all") if !strings.Contains(stderr, "skipping") || !strings.Contains(stderr, "/nonexistent/path") { From a6a46ca8bad57eaecc5fe591a7e9dadbd05e8f8c Mon Sep 17 00:00:00 2001 From: Jason Allum <21417+jallum@users.noreply.github.com> Date: Thu, 7 May 2026 14:20:52 -0400 Subject: [PATCH 18/19] gofmt internal/recap/recap.go internal/treefs/treefs_test.go --- internal/recap/recap.go | 12 ++++++------ internal/treefs/treefs_test.go | 1 - 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/internal/recap/recap.go b/internal/recap/recap.go index 34986769..e07e083d 100644 --- a/internal/recap/recap.go +++ b/internal/recap/recap.go @@ -18,10 +18,10 @@ type IssueLookup interface { // Event represents a single parsed activity from a commit message. type Event struct { - Type string // "create", "close", "start", "update", "reopen", "defer", "undefer", "comment", "link", "unlink", "unblocked", "delete", "label" - ID string // primary issue ID - Time time.Time // commit timestamp - Detail string // additional context (title, reason, etc.) + Type string // "create", "close", "start", "update", "reopen", "defer", "undefer", "comment", "link", "unlink", "unblocked", "delete", "label" + ID string // primary issue ID + Time time.Time // commit timestamp + Detail string // additional context (title, reason, etc.) } // Leaf is a single event in the recap tree. @@ -34,8 +34,8 @@ type Leaf struct { // Section groups events for a single issue. type Section struct { - ID string `json:"id"` - Title string `json:"title"` + ID string `json:"id"` + Title string `json:"title"` Leaves []Leaf `json:"events"` } diff --git a/internal/treefs/treefs_test.go b/internal/treefs/treefs_test.go index 86c3f310..81ae9fda 100644 --- a/internal/treefs/treefs_test.go +++ b/internal/treefs/treefs_test.go @@ -528,4 +528,3 @@ func TestCommitRespectsBWClockEnv(t *testing.T) { t.Errorf("commit time = %v, want %v", commits[0].Time, expected) } } - From 030cf238ef0465e0a69832bed6a265e557798053 Mon Sep 17 00:00:00 2001 From: Manuel Zubieta Date: Thu, 7 May 2026 19:48:37 -0400 Subject: [PATCH 19/19] recap: bump cursor mtime on no-event runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "since last recap (Xh ago)" header derives from os.Stat ModTime of the cursor ref file. Before this change, mtime only moved when SetRecapCursor wrote a new hash, so a no-event recap left the timestamp unchanged and the header lagged across quiet stretches: Mon 09:00 bw recap → "since last recap (24h ago)" Tue 09:00 bw recap → "no activity" (mtime untouched) Wed 09:00 bw recap → "since last recap (2d ago)" Add TouchRecapCursor (os.Chtimes-only) and call it from runRecapSingle and cmdRecapAll when the recap completes without new commits and a cursor already exists. The gap-window early-return still skips both SetRecapCursor and TouchRecapCursor, preserving the gapped-explicit behavior from ac817d5. Update TestRecapStampsLastRecapAtWithNoCommits to actually assert what its name claims: backdate the cursor mtime via os.Chtimes, run a no-event recap, verify mtime advances. Sanity-checked: removing the fix makes the test fail with "cursor mtime did not advance". --- cmd/bw/recap.go | 10 ++++++++-- internal/repo/recap_cursor.go | 16 +++++++++++++-- test/acceptance_test.go | 37 ++++++++++++++++++++++++++++++++--- 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/cmd/bw/recap.go b/cmd/bw/recap.go index c2aafd7f..56e9452d 100644 --- a/cmd/bw/recap.go +++ b/cmd/bw/recap.go @@ -114,8 +114,11 @@ func runRecapSingle(ra recapArgs, w Writer, dir string) error { return nil } } - if len(commits) > 0 { + switch { + case len(commits) > 0: _ = r.SetRecapCursor(commits[0].Hash) + case cursor != "": + _ = r.TouchRecapCursor() } } @@ -258,8 +261,11 @@ func cmdRecapAll(ra recapArgs, w Writer, cfg *config.Config) error { continue } } - if len(commits) > 0 { + switch { + case len(commits) > 0: _ = r.SetRecapCursor(commits[0].Hash) + case cursor != "": + _ = r.TouchRecapCursor() } } } diff --git a/internal/repo/recap_cursor.go b/internal/repo/recap_cursor.go index 4fba8245..342f7dc5 100644 --- a/internal/repo/recap_cursor.go +++ b/internal/repo/recap_cursor.go @@ -21,8 +21,9 @@ func (r *Repo) RecapCursor() string { } // LastRecapAt returns the mtime of the recap cursor ref file, which -// records when the cursor was last advanced. Returns the zero time if -// the ref doesn't exist. +// records when recap last ran successfully (advancing the cursor or +// touching the ref on a no-event run). Returns the zero time if the +// ref doesn't exist. func (r *Repo) LastRecapAt() time.Time { path := filepath.Join(r.GitDir, recapCursorRef) info, err := os.Stat(path) @@ -41,3 +42,14 @@ func (r *Repo) SetRecapCursor(hash string) error { } return os.WriteFile(path, []byte(hash+"\n"), 0644) } + +// TouchRecapCursor bumps the mtime of the recap cursor ref to the +// current time without changing its content. Used to record that recap +// ran successfully even when no new events advanced the cursor, so that +// the "since last recap" header reflects the most recent run rather +// than the last cursor advance. +func (r *Repo) TouchRecapCursor() error { + path := filepath.Join(r.GitDir, recapCursorRef) + now := time.Now() + return os.Chtimes(path, now, now) +} diff --git a/test/acceptance_test.go b/test/acceptance_test.go index 8fd51832..5cb2e58b 100644 --- a/test/acceptance_test.go +++ b/test/acceptance_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/jallum/beadwork/internal/config" ) @@ -581,8 +582,10 @@ func TestRecapCursorIsIncremental(t *testing.T) { } // TestRecapStampsLastRecapAtWithNoCommits verifies that running recap with -// nothing new still leaves the cursor ref in place so the "since last recap" -// label can be derived from the ref file's mtime. +// nothing new bumps the cursor ref's mtime so the "since last recap (Xh ago)" +// header reflects the most recent run, not the last cursor advance. Without +// this, a quiet stretch (no commits between runs) leaves the header growing +// unboundedly even though the user just ran recap. func TestRecapStampsLastRecapAtWithNoCommits(t *testing.T) { env := newBwEnv(t) env.bw("create", "x", "--id", "lr-1") @@ -593,7 +596,25 @@ func TestRecapStampsLastRecapAtWithNoCommits(t *testing.T) { } cursor1 := env.recapCursor() - // Run again with nothing new. Cursor ref should still exist. + // Simulate a quiet stretch: backdate the cursor file's mtime as if the + // last recap ran two hours ago. Using a real-time delta (not BW_CLOCK) + // because mtime is wall-clock filesystem state. + cursorPath := filepath.Join(env.dir, ".git", "refs", "beadwork", "recap-cursor") + backdate := time.Now().Add(-2 * time.Hour) + if err := os.Chtimes(cursorPath, backdate, backdate); err != nil { + t.Fatalf("chtimes backdate: %v", err) + } + info, err := os.Stat(cursorPath) + if err != nil { + t.Fatalf("stat after backdate: %v", err) + } + if time.Since(info.ModTime()) < time.Hour { + t.Fatalf("backdate did not stick: mtime %v is only %v ago", info.ModTime(), time.Since(info.ModTime())) + } + backdated := info.ModTime() + + // Run again with nothing new. Cursor value must not change, mtime must + // advance beyond the backdated time. env.bw("recap") if !env.recapCursorExists() { t.Errorf("second recap lost cursor ref") @@ -601,6 +622,16 @@ func TestRecapStampsLastRecapAtWithNoCommits(t *testing.T) { if env.recapCursor() != cursor1 { t.Errorf("cursor changed with no new commits") } + info2, err := os.Stat(cursorPath) + if err != nil { + t.Fatalf("stat after second recap: %v", err) + } + if !info2.ModTime().After(backdated) { + t.Errorf("cursor mtime did not advance on no-event recap: was %v, still %v", backdated, info2.ModTime()) + } + if time.Since(info2.ModTime()) > time.Minute { + t.Errorf("cursor mtime not recent after recap: %v ago", time.Since(info2.ModTime())) + } } // TestRecapDryRunDoesNotStamp verifies --dry-run leaves the cursor ref alone.