From b293806ac5bec954b9406f585610941d5b152849 Mon Sep 17 00:00:00 2001 From: Doug Brown Date: Tue, 11 Aug 2026 18:03:02 +0000 Subject: [PATCH] fix: reflect manual runs in status --- cmd/git-tend/run.go | 47 ++++++++++++- cmd/git-tend/run_test.go | 108 ++++++++++++++++++++++++++++++ internal/daemon/daemon.go | 9 ++- internal/paths/paths.go | 8 ++- internal/paths/paths_test.go | 20 ++++++ internal/status/status.go | 118 ++++++++++++++++++++++++++++----- internal/status/status_test.go | 31 +++++++++ internal/sync/sync.go | 16 ++++- internal/sync/sync_test.go | 14 ++++ 9 files changed, 347 insertions(+), 24 deletions(-) create mode 100644 cmd/git-tend/run_test.go create mode 100644 internal/paths/paths_test.go diff --git a/cmd/git-tend/run.go b/cmd/git-tend/run.go index 64293d3..4208079 100644 --- a/cmd/git-tend/run.go +++ b/cmd/git-tend/run.go @@ -4,11 +4,13 @@ import ( "context" "fmt" "path/filepath" + "time" "github.com/spf13/cobra" "github.com/sdougbrown/git-tend/internal/config" "github.com/sdougbrown/git-tend/internal/paths" + "github.com/sdougbrown/git-tend/internal/status" "github.com/sdougbrown/git-tend/internal/sync" ) @@ -34,7 +36,12 @@ func runRepo(cmd *cobra.Command, args []string) error { stateDir := paths.StateDir() ctx := context.Background() - result := sync.Sync(ctx, repoPath, cfg, stateDir) + result := sync.SyncManual(ctx, repoPath, cfg, stateDir) + if result.State != "skipped" { + if err := recordRunStatus(filepath.Join(stateDir, "status.json"), repoPath, cfg.Mode, result); err != nil { + return fmt.Errorf("recording status: %w", err) + } + } fmt.Printf("state: %s\n", result.State) if result.Error != "" { @@ -45,3 +52,41 @@ func runRepo(cmd *cobra.Command, args []string) error { } return fmt.Errorf("sync failed (%s): %s", result.State, result.Error) } + +func recordRunStatus(statusPath, repoPath, mode string, result sync.SyncResult) error { + now := time.Now().UTC().Format(time.RFC3339Nano) + return status.UpdateRepo(statusPath, repoPath, func(rs status.RepoStatus) status.RepoStatus { + rs.Mode = mode + rs.UpdatedAt = now + + switch result.State { + case "ok": + rs.PriorState = rs.CurrentState + rs.CurrentState = "ok" + rs.LastSyncAt = now + rs.LastError = "" + rs.Ahead = result.Ahead + rs.Behind = result.Behind + rs.StuckSince = "" + rs.SnoozedUntil = "" + rs.OfflineSince = "" + rs.ConsecutiveOfflineFailures = 0 + case "offline": + rs.PriorState = rs.CurrentState + rs.CurrentState = "offline" + rs.LastError = result.Error + if rs.OfflineSince == "" { + rs.OfflineSince = now + } + rs.ConsecutiveOfflineFailures++ + case "stuck": + rs.PriorState = rs.CurrentState + rs.CurrentState = "stuck" + rs.LastError = result.Error + if rs.StuckSince == "" { + rs.StuckSince = now + } + } + return rs + }) +} diff --git a/cmd/git-tend/run_test.go b/cmd/git-tend/run_test.go new file mode 100644 index 0000000..acf5207 --- /dev/null +++ b/cmd/git-tend/run_test.go @@ -0,0 +1,108 @@ +package main + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/spf13/cobra" + + "github.com/sdougbrown/git-tend/internal/paths" + "github.com/sdougbrown/git-tend/internal/status" +) + +func runGit(t *testing.T, dir string, args ...string) { + t.Helper() + out, err := exec.Command("git", append([]string{"-C", dir}, args...)...).CombinedOutput() + if err != nil { + t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out) + } +} + +func setupRunRepo(t *testing.T) string { + t.Helper() + remote := filepath.Join(t.TempDir(), "remote.git") + if out, err := exec.Command("git", "init", "--bare", "--initial-branch=main", remote).CombinedOutput(); err != nil { + t.Fatalf("creating bare remote: %v\n%s", err, out) + } + + repo := filepath.Join(t.TempDir(), "repo") + if err := os.MkdirAll(repo, 0755); err != nil { + t.Fatal(err) + } + runGit(t, repo, "init", "--initial-branch=main") + runGit(t, repo, "config", "user.email", "test@gittend.local") + runGit(t, repo, "config", "user.name", "git-tend test") + runGit(t, repo, "remote", "add", "origin", remote) + if err := os.WriteFile(filepath.Join(repo, ".gittend"), []byte("mode = \"read-write\"\nsync_branch = \"main\"\ndebounce = \"1h\"\n"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, "work.txt"), []byte("before"), 0644); err != nil { + t.Fatal(err) + } + runGit(t, repo, "add", ".gittend", "work.txt") + runGit(t, repo, "commit", "-m", "initial") + runGit(t, repo, "push", "-u", "origin", "main") + if err := os.WriteFile(filepath.Join(repo, "work.txt"), []byte("manual change"), 0644); err != nil { + t.Fatal(err) + } + return repo +} + +func TestRunAfterUnstickBypassesDebounceAndPreservesStatus(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("XDG_STATE_HOME", t.TempDir()) + repo := setupRunRepo(t) + stateDir := paths.StateDir() + if err := os.MkdirAll(stateDir, 0755); err != nil { + t.Fatal(err) + } + statusPath := filepath.Join(stateDir, "status.json") + stale := status.RepoStatus{ + Mode: "read-write", + CurrentState: "stuck", + UpdatedAt: time.Now().Add(-time.Minute).UTC().Format(time.RFC3339Nano), + } + if err := status.Write(statusPath, &status.StatusFile{Repos: map[string]status.RepoStatus{repo: stale}}); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(repo, ".gittend.stuck"), []byte("stuck"), 0644); err != nil { + t.Fatal(err) + } + if err := removeStuckFlag(repo); err != nil { + t.Fatalf("unstick: %v", err) + } + t.Chdir(repo) + if err := runRepo(&cobra.Command{}, []string{"."}); err != nil { + t.Fatalf("run: %v", err) + } + + got := status.Read(statusPath).Repos[repo] + if got.CurrentState != "ok" { + t.Fatalf("status after manual run = %q, want ok (error: %s)", got.CurrentState, got.LastError) + } + if got.LastSyncAt == "" { + t.Fatal("manual run did not record last sync time") + } + + // This models a daemon tick that began before the manual run and writes its + // stale in-memory snapshot afterwards. + if err := status.MergeAndWrite(statusPath, &status.StatusFile{Repos: map[string]status.RepoStatus{repo: stale}}); err != nil { + t.Fatal(err) + } + if got := status.Read(statusPath).Repos[repo].CurrentState; got != "ok" { + t.Errorf("stale daemon write replaced manual status with %q, want ok", got) + } + + out, err := exec.Command("git", "-C", repo, "status", "--porcelain").Output() + if err != nil { + t.Fatal(err) + } + if strings.TrimSpace(string(out)) != "" { + t.Errorf("manual change was not committed: %s", out) + } +} diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index aaf3223..09a5971 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -185,7 +185,7 @@ func (d *Daemon) rescanRoots() { if _, exists := d.repoStatus[r.Path]; exists { continue } - rs := status.RepoStatus{CurrentState: "pending"} + rs := status.RepoStatus{CurrentState: "pending", UpdatedAt: time.Now().UTC().Format(time.RFC3339Nano)} if r.Config != nil { rs.Mode = r.Config.Mode } @@ -218,6 +218,7 @@ func (d *Daemon) tick(ctx context.Context) { rs.PriorState = rs.CurrentState rs.CurrentState = "snoozed" rs.SnoozedUntil = snoozedUntil.UTC().Format(time.RFC3339) + rs.UpdatedAt = time.Now().UTC().Format(time.RFC3339Nano) d.repoStatus[repo.Path] = rs d.mu.Unlock() continue @@ -239,6 +240,7 @@ func (d *Daemon) tick(ctx context.Context) { if rs.StuckSince == "" { rs.StuckSince = now.UTC().Format(time.RFC3339) } + rs.UpdatedAt = now.UTC().Format(time.RFC3339Nano) } d.repoStatus[repo.Path] = rs d.mu.Unlock() @@ -331,6 +333,9 @@ func (d *Daemon) tick(ctx context.Context) { d.logger.Debug("repo skipped", "repo", repo.Path, "reason", result.Error) } + if result.State != "skipped" { + rs.UpdatedAt = now.UTC().Format(time.RFC3339Nano) + } d.repoStatus[repo.Path] = rs d.mu.Unlock() @@ -355,7 +360,7 @@ func (d *Daemon) writeStatus() { Repos: d.repoStatus, } - if err := status.Write(filepath.Join(d.stateDir, "status.json"), sf); err != nil { + if err := status.MergeAndWrite(filepath.Join(d.stateDir, "status.json"), sf); err != nil { d.logger.Error("writing status", "error", err) } } diff --git a/internal/paths/paths.go b/internal/paths/paths.go index 84fd14c..f5e69eb 100644 --- a/internal/paths/paths.go +++ b/internal/paths/paths.go @@ -52,7 +52,13 @@ func ExpandPath(path string) string { path = strings.Replace(path, "$HOME", home, 1) } } - return path + + // Managed-repo state is keyed by paths found during scanning, which are + // absolute. Normalize CLI paths as well so `run .` updates that same entry. + if absolute, err := filepath.Abs(path); err == nil { + return filepath.Clean(absolute) + } + return filepath.Clean(path) } func appSupportDir() string { diff --git a/internal/paths/paths_test.go b/internal/paths/paths_test.go new file mode 100644 index 0000000..c4a2335 --- /dev/null +++ b/internal/paths/paths_test.go @@ -0,0 +1,20 @@ +package paths + +import ( + "path/filepath" + "testing" +) + +func TestExpandPathMakesRelativePathsAbsolute(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + + got := ExpandPath(".") + want, err := filepath.Abs(dir) + if err != nil { + t.Fatal(err) + } + if got != want { + t.Errorf("ExpandPath(\".\") = %q, want %q", got, want) + } +} diff --git a/internal/status/status.go b/internal/status/status.go index 4d54cff..2b0733e 100644 --- a/internal/status/status.go +++ b/internal/status/status.go @@ -3,29 +3,33 @@ package status import ( "encoding/json" "os" + "path/filepath" "time" + + "golang.org/x/sys/unix" ) type StatusFile struct { - Version int `json:"version"` - DaemonStartedAt string `json:"daemon_started_at"` - LastTickAt string `json:"last_tick_at"` - IntervalSeconds int `json:"interval_seconds"` + Version int `json:"version"` + DaemonStartedAt string `json:"daemon_started_at"` + LastTickAt string `json:"last_tick_at"` + IntervalSeconds int `json:"interval_seconds"` Repos map[string]RepoStatus `json:"repos"` } type RepoStatus struct { - Mode string `json:"mode"` - CurrentState string `json:"current_state"` - PriorState string `json:"prior_state"` - LastSyncAt string `json:"last_sync_at"` - LastError string `json:"last_error"` - Ahead int `json:"ahead"` - Behind int `json:"behind"` - StuckSince string `json:"stuck_since"` - SnoozedUntil string `json:"snoozed_until"` - OfflineSince string `json:"offline_since"` - ConsecutiveOfflineFailures int `json:"consecutive_offline_failures"` + Mode string `json:"mode"` + CurrentState string `json:"current_state"` + PriorState string `json:"prior_state"` + UpdatedAt string `json:"updated_at"` + LastSyncAt string `json:"last_sync_at"` + LastError string `json:"last_error"` + Ahead int `json:"ahead"` + Behind int `json:"behind"` + StuckSince string `json:"stuck_since"` + SnoozedUntil string `json:"snoozed_until"` + OfflineSince string `json:"offline_since"` + ConsecutiveOfflineFailures int `json:"consecutive_offline_failures"` } func Read(path string) *StatusFile { @@ -49,16 +53,96 @@ func Read(path string) *StatusFile { return &sf } +// UpdateRepo atomically updates one repo's status. It is used by foreground +// commands, which can run concurrently with the daemon. +func UpdateRepo(path, repoPath string, update func(RepoStatus) RepoStatus) error { + return withLock(path, func() error { + sf := read(path) + if sf == nil { + sf = &StatusFile{Version: 1, Repos: make(map[string]RepoStatus)} + } + if sf.Repos == nil { + sf.Repos = make(map[string]RepoStatus) + } + sf.Repos[repoPath] = update(sf.Repos[repoPath]) + return write(path, sf) + }) +} + +// MergeAndWrite writes the daemon's snapshot without replacing a newer +// foreground-command result. Both writers share the sidecar lock. +func MergeAndWrite(path string, sf *StatusFile) error { + return withLock(path, func() error { + current := read(path) + if current != nil { + for repoPath, candidate := range sf.Repos { + if existing, ok := current.Repos[repoPath]; ok && newerOrEqual(existing.UpdatedAt, candidate.UpdatedAt) { + sf.Repos[repoPath] = existing + } + } + } + return write(path, sf) + }) +} + func Write(path string, sf *StatusFile) error { + return withLock(path, func() error { return write(path, sf) }) +} + +func read(path string) *StatusFile { + data, err := os.ReadFile(path) + if err != nil { + return nil + } + var sf StatusFile + if json.Unmarshal(data, &sf) != nil { + return nil + } + return &sf +} + +func write(path string, sf *StatusFile) error { data, err := json.MarshalIndent(sf, "", " ") if err != nil { return err } - tmpPath := path + ".tmp" - if err := os.WriteFile(tmpPath, data, 0644); err != nil { + tmp, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+".tmp-") + if err != nil { return err } + tmpPath := tmp.Name() + defer os.Remove(tmpPath) + if err := tmp.Chmod(0644); err != nil { + tmp.Close() + return err + } + if _, err := tmp.Write(data); err != nil { + tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } return os.Rename(tmpPath, path) } + +func withLock(path string, fn func() error) error { + lock, err := os.OpenFile(path+".lock", os.O_CREATE|os.O_RDWR, 0644) + if err != nil { + return err + } + defer lock.Close() + if err := unix.Flock(int(lock.Fd()), unix.LOCK_EX); err != nil { + return err + } + defer unix.Flock(int(lock.Fd()), unix.LOCK_UN) + return fn() +} + +func newerOrEqual(a, b string) bool { + at, errA := time.Parse(time.RFC3339, a) + bt, errB := time.Parse(time.RFC3339, b) + return errA == nil && (errB != nil || !at.Before(bt)) +} diff --git a/internal/status/status_test.go b/internal/status/status_test.go index 5cea66b..d01a8ea 100644 --- a/internal/status/status_test.go +++ b/internal/status/status_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "testing" + "time" ) func TestReadMissing(t *testing.T) { @@ -42,6 +43,36 @@ func TestWriteReadRoundtrip(t *testing.T) { } } +func TestMergeAndWritePreservesNewerForegroundResult(t *testing.T) { + path := filepath.Join(t.TempDir(), "status.json") + repo := "/test/repo" + old := time.Now().Add(-time.Minute).UTC().Format(time.RFC3339Nano) + manual := time.Now().UTC().Format(time.RFC3339Nano) + + if err := Write(path, &StatusFile{Repos: map[string]RepoStatus{ + repo: {CurrentState: "stuck", UpdatedAt: old}, + }}); err != nil { + t.Fatal(err) + } + if err := UpdateRepo(path, repo, func(rs RepoStatus) RepoStatus { + rs.CurrentState = "ok" + rs.UpdatedAt = manual + return rs + }); err != nil { + t.Fatal(err) + } + + if err := MergeAndWrite(path, &StatusFile{Repos: map[string]RepoStatus{ + repo: {CurrentState: "stuck", UpdatedAt: old}, + }}); err != nil { + t.Fatal(err) + } + + if got := Read(path).Repos[repo].CurrentState; got != "ok" { + t.Errorf("state after stale daemon write = %q, want ok", got) + } +} + func TestAtomicWriteNoTmpFile(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "status.json") diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 97ce5f7..63a2b53 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -27,6 +27,16 @@ type SyncResult struct { } func Sync(ctx context.Context, repoPath string, cfg *config.Config, stateDir string) SyncResult { + return sync(ctx, repoPath, cfg, stateDir, false) +} + +// SyncManual runs an explicit user-requested sync. Unlike periodic daemon +// syncs, it does not defer read-write work for the debounce interval. +func SyncManual(ctx context.Context, repoPath string, cfg *config.Config, stateDir string) SyncResult { + return sync(ctx, repoPath, cfg, stateDir, true) +} + +func sync(ctx context.Context, repoPath string, cfg *config.Config, stateDir string, bypassDebounce bool) SyncResult { hash := sha256.Sum256([]byte(repoPath)) lockDir := filepath.Join(stateDir, "locks") lockPath := filepath.Join(lockDir, hex.EncodeToString(hash[:])+".lock") @@ -68,7 +78,7 @@ func Sync(ctx context.Context, repoPath string, cfg *config.Config, stateDir str return syncReadOnly(ctx, repoPath, timeout) } - return syncReadWrite(ctx, repoPath, cfg, timeout) + return syncReadWrite(ctx, repoPath, cfg, timeout, bypassDebounce) } func parseIntervalTimeout(interval string) time.Duration { @@ -108,7 +118,7 @@ func syncReadOnly(ctx context.Context, repoPath string, timeout time.Duration) S return SyncResult{State: "ok"} } -func syncReadWrite(ctx context.Context, repoPath string, cfg *config.Config, timeout time.Duration) SyncResult { +func syncReadWrite(ctx context.Context, repoPath string, cfg *config.Config, timeout time.Duration, bypassDebounce bool) SyncResult { debounceDur := parseDebounce(cfg.Debounce) files, err := git.ListTrackedFiles(repoPath) @@ -130,7 +140,7 @@ func syncReadWrite(ctx context.Context, repoPath string, cfg *config.Config, tim maxMtime = fi.ModTime() } } - if !maxMtime.IsZero() && now.Sub(maxMtime) < debounceDur { + if !bypassDebounce && !maxMtime.IsZero() && now.Sub(maxMtime) < debounceDur { return SyncResult{State: "skipped", Error: "debounce"} } diff --git a/internal/sync/sync_test.go b/internal/sync/sync_test.go index e6361be..0f19383 100644 --- a/internal/sync/sync_test.go +++ b/internal/sync/sync_test.go @@ -442,6 +442,20 @@ func TestSyncWithDebounce(t *testing.T) { if result.State != "skipped" { t.Fatalf("expected skipped, got %s: %s", result.State, result.Error) } + + manual := SyncManual(context.Background(), repo, cfg, stateDir) + if manual.State != "ok" { + t.Fatalf("manual sync should bypass debounce, got %s: %s", manual.State, manual.Error) + } + clone2 := gitClone(t, remote) + defer os.RemoveAll(clone2) + contents, err := os.ReadFile(filepath.Join(clone2, "recent.txt")) + if err != nil { + t.Fatal(err) + } + if got := string(contents); got != "just modified" { + t.Errorf("remote recent.txt = %q, want committed manual change", got) + } } func TestSyncLockContentionSkips(t *testing.T) {