From 78001d2c014b631a4c723bd85b4d59ba023ea465 Mon Sep 17 00:00:00 2001 From: gastown/crew/george Date: Wed, 21 Jan 2026 10:51:42 -0800 Subject: [PATCH] fix: update patrol_check tests and add cross-filesystem clone support - Update patrol_check tests to expect StatusOK instead of StatusWarning for missing templates (embedded templates fill the gap) - Add moveDir helper with cross-filesystem fallback for git clones - Remove accidentally committed events.jsonl file Co-Authored-By: Claude Opus 4.5 --- internal/.events.jsonl | 5 -- internal/doctor/patrol_check_test.go | 80 ++++++++++++++---------- internal/git/git.go | 92 +++++++++++++++++++++++++--- 3 files changed, 132 insertions(+), 45 deletions(-) delete mode 100644 internal/.events.jsonl diff --git a/internal/.events.jsonl b/internal/.events.jsonl deleted file mode 100644 index daccf82e..00000000 --- a/internal/.events.jsonl +++ /dev/null @@ -1,5 +0,0 @@ -{"ts":"2026-01-18T09:52:29Z","source":"gt","type":"session_death","actor":"gt-gastown-witness","payload":{"agent":"unknown","caller":"gt doctor","reason":"zombie cleanup","session":"gt-gastown-witness"},"visibility":"feed"} -{"ts":"2026-01-18T10:00:24Z","source":"gt","type":"session_death","actor":"gt-gastown-witness","payload":{"agent":"unknown","caller":"gt doctor","reason":"zombie cleanup","session":"gt-gastown-witness"},"visibility":"feed"} -{"ts":"2026-01-18T10:06:48Z","source":"gt","type":"session_death","actor":"gt-gastown-witness","payload":{"agent":"unknown","caller":"gt doctor","reason":"zombie cleanup","session":"gt-gastown-witness"},"visibility":"feed"} -{"ts":"2026-01-18T14:26:38Z","source":"gt","type":"session_death","actor":"gt-gastown-witness","payload":{"agent":"unknown","caller":"gt doctor","reason":"zombie cleanup","session":"gt-gastown-witness"},"visibility":"feed"} -{"ts":"2026-01-18T14:30:02Z","source":"gt","type":"session_death","actor":"gt-gastown-witness","payload":{"agent":"unknown","caller":"gt doctor","reason":"zombie cleanup","session":"gt-gastown-witness"},"visibility":"feed"} diff --git a/internal/doctor/patrol_check_test.go b/internal/doctor/patrol_check_test.go index ee0f5de9..a90ad756 100644 --- a/internal/doctor/patrol_check_test.go +++ b/internal/doctor/patrol_check_test.go @@ -75,14 +75,12 @@ func TestPatrolRolesHavePromptsCheck_NoTemplatesDir(t *testing.T) { result := check.Run(ctx) - if result.Status != StatusWarning { - t.Errorf("Status = %v, want Warning", result.Status) + // Rigs without templates directory use embedded templates - this is OK + if result.Status != StatusOK { + t.Errorf("Status = %v, want OK (using embedded templates)", result.Status) } - if len(check.missingByRig) != 1 { - t.Errorf("missingByRig count = %d, want 1", len(check.missingByRig)) - } - if len(check.missingByRig["myproject"]) != 3 { - t.Errorf("missing templates for myproject = %d, want 3", len(check.missingByRig["myproject"])) + if len(check.missingByRig) != 0 { + t.Errorf("missingByRig count = %d, want 0 (rig skipped)", len(check.missingByRig)) } } @@ -100,8 +98,9 @@ func TestPatrolRolesHavePromptsCheck_SomeTemplatesMissing(t *testing.T) { result := check.Run(ctx) - if result.Status != StatusWarning { - t.Errorf("Status = %v, want Warning", result.Status) + // Missing templates in custom override dir is OK - embedded templates fill the gap + if result.Status != StatusOK { + t.Errorf("Status = %v, want OK (embedded templates fill gaps)", result.Status) } if len(check.missingByRig["myproject"]) != 2 { t.Errorf("missing templates = %d, want 2 (witness, refinery)", len(check.missingByRig["myproject"])) @@ -135,21 +134,25 @@ func TestPatrolRolesHavePromptsCheck_AllTemplatesExist(t *testing.T) { func TestPatrolRolesHavePromptsCheck_Fix(t *testing.T) { tmpDir := t.TempDir() setupRigConfig(t, tmpDir, []string{"myproject"}) + // Create templates dir so rig is checked (not skipped) + templatesDir := setupRigTemplatesDir(t, tmpDir, "myproject") check := NewPatrolRolesHavePromptsCheck() ctx := &CheckContext{TownRoot: tmpDir} result := check.Run(ctx) - if result.Status != StatusWarning { - t.Fatalf("Initial Status = %v, want Warning", result.Status) + // Status is OK (embedded templates fill gaps) but missingByRig is populated + if result.Status != StatusOK { + t.Fatalf("Initial Status = %v, want OK", result.Status) + } + if len(check.missingByRig["myproject"]) != 3 { + t.Fatalf("missingByRig = %d, want 3", len(check.missingByRig["myproject"])) } err := check.Fix(ctx) if err != nil { t.Fatalf("Fix() error = %v", err) } - - templatesDir := filepath.Join(tmpDir, "myproject", "mayor", "rig", "internal", "templates", "roles") for _, tmpl := range requiredRolePrompts { path := filepath.Join(templatesDir, tmpl) info, err := os.Stat(path) @@ -182,8 +185,9 @@ func TestPatrolRolesHavePromptsCheck_FixPartial(t *testing.T) { ctx := &CheckContext{TownRoot: tmpDir} result := check.Run(ctx) - if result.Status != StatusWarning { - t.Fatalf("Initial Status = %v, want Warning", result.Status) + // Status is OK (embedded templates fill gaps) but missingByRig is populated + if result.Status != StatusOK { + t.Fatalf("Initial Status = %v, want OK", result.Status) } if len(check.missingByRig["myproject"]) != 2 { t.Fatalf("missing = %d, want 2", len(check.missingByRig["myproject"])) @@ -214,6 +218,7 @@ func TestPatrolRolesHavePromptsCheck_MultipleRigs(t *testing.T) { tmpDir := t.TempDir() setupRigConfig(t, tmpDir, []string{"project1", "project2"}) + // project1 has templates dir with all templates templatesDir1 := setupRigTemplatesDir(t, tmpDir, "project1") for _, tmpl := range requiredRolePrompts { if err := os.WriteFile(filepath.Join(templatesDir1, tmpl), []byte("test"), 0644); err != nil { @@ -221,13 +226,17 @@ func TestPatrolRolesHavePromptsCheck_MultipleRigs(t *testing.T) { } } + // project2 has templates dir but no files (missing all) + setupRigTemplatesDir(t, tmpDir, "project2") + check := NewPatrolRolesHavePromptsCheck() ctx := &CheckContext{TownRoot: tmpDir} result := check.Run(ctx) - if result.Status != StatusWarning { - t.Errorf("Status = %v, want Warning (project2 missing)", result.Status) + // Status is OK (embedded templates fill gaps) + if result.Status != StatusOK { + t.Errorf("Status = %v, want OK", result.Status) } if _, ok := check.missingByRig["project1"]; ok { t.Error("project1 should not be in missingByRig") @@ -240,17 +249,21 @@ func TestPatrolRolesHavePromptsCheck_MultipleRigs(t *testing.T) { func TestPatrolRolesHavePromptsCheck_FixHint(t *testing.T) { tmpDir := t.TempDir() setupRigConfig(t, tmpDir, []string{"myproject"}) + // Create templates dir so rig is checked (not skipped) + setupRigTemplatesDir(t, tmpDir, "myproject") check := NewPatrolRolesHavePromptsCheck() ctx := &CheckContext{TownRoot: tmpDir} result := check.Run(ctx) - if result.FixHint == "" { - t.Error("FixHint should not be empty for warning status") + // Status is now OK (embedded templates are fine), so no FixHint + if result.Status != StatusOK { + t.Errorf("Status = %v, want OK", result.Status) } - if result.FixHint != "Run 'gt doctor --fix' to copy embedded templates to rig repos" { - t.Errorf("FixHint = %q, unexpected value", result.FixHint) + // FixHint is empty for OK status since nothing is broken + if result.FixHint != "" { + t.Errorf("FixHint = %q, want empty for OK status", result.FixHint) } } @@ -258,6 +271,7 @@ func TestPatrolRolesHavePromptsCheck_FixMultipleRigs(t *testing.T) { tmpDir := t.TempDir() setupRigConfig(t, tmpDir, []string{"project1", "project2", "project3"}) + // project1 has all templates templatesDir1 := setupRigTemplatesDir(t, tmpDir, "project1") for _, tmpl := range requiredRolePrompts { if err := os.WriteFile(filepath.Join(templatesDir1, tmpl), []byte("existing"), 0644); err != nil { @@ -265,12 +279,17 @@ func TestPatrolRolesHavePromptsCheck_FixMultipleRigs(t *testing.T) { } } + // project2 and project3 have templates dir but no files + setupRigTemplatesDir(t, tmpDir, "project2") + setupRigTemplatesDir(t, tmpDir, "project3") + check := NewPatrolRolesHavePromptsCheck() ctx := &CheckContext{TownRoot: tmpDir} result := check.Run(ctx) - if result.Status != StatusWarning { - t.Fatalf("Initial Status = %v, want Warning", result.Status) + // Status is OK (embedded templates fill gaps) but missingByRig is populated + if result.Status != StatusOK { + t.Fatalf("Initial Status = %v, want OK", result.Status) } if len(check.missingByRig) != 2 { t.Fatalf("missingByRig count = %d, want 2 (project2, project3)", len(check.missingByRig)) @@ -300,20 +319,17 @@ func TestPatrolRolesHavePromptsCheck_FixMultipleRigs(t *testing.T) { func TestPatrolRolesHavePromptsCheck_DetailsFormat(t *testing.T) { tmpDir := t.TempDir() setupRigConfig(t, tmpDir, []string{"myproject"}) + // Create templates dir so rig is checked (not skipped) + setupRigTemplatesDir(t, tmpDir, "myproject") check := NewPatrolRolesHavePromptsCheck() ctx := &CheckContext{TownRoot: tmpDir} - result := check.Run(ctx) + _ = check.Run(ctx) - if len(result.Details) != 3 { - t.Fatalf("Details count = %d, want 3", len(result.Details)) - } - - for _, detail := range result.Details { - if detail[:10] != "myproject:" { - t.Errorf("Detail %q should be prefixed with 'myproject:'", detail) - } + // Status is OK now, but missingByRig should be populated with 3 templates + if len(check.missingByRig["myproject"]) != 3 { + t.Fatalf("missingByRig count = %d, want 3", len(check.missingByRig["myproject"])) } } diff --git a/internal/git/git.go b/internal/git/git.go index 8a0775b1..4e79ef12 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -4,6 +4,7 @@ package git import ( "bytes" "fmt" + "io" "os" "os/exec" "path/filepath" @@ -33,6 +34,81 @@ func (e *GitError) Unwrap() error { return e.Err } +// moveDir moves a directory from src to dest. It first tries os.Rename for +// efficiency, but falls back to copy+delete if src and dest are on different +// filesystems (which causes EXDEV error on rename). +func moveDir(src, dest string) error { + // Try rename first - works if same filesystem + if err := os.Rename(src, dest); err == nil { + return nil + } + + // Rename failed, try copy+delete as fallback for cross-filesystem moves + if err := copyDir(src, dest); err != nil { + return fmt.Errorf("copying directory: %w", err) + } + if err := os.RemoveAll(src); err != nil { + return fmt.Errorf("removing source after copy: %w", err) + } + return nil +} + +// copyDir recursively copies a directory from src to dest. +func copyDir(src, dest string) error { + srcInfo, err := os.Stat(src) + if err != nil { + return err + } + + if err := os.MkdirAll(dest, srcInfo.Mode()); err != nil { + return err + } + + entries, err := os.ReadDir(src) + if err != nil { + return err + } + + for _, entry := range entries { + srcPath := filepath.Join(src, entry.Name()) + destPath := filepath.Join(dest, entry.Name()) + + if entry.IsDir() { + if err := copyDir(srcPath, destPath); err != nil { + return err + } + } else { + if err := copyFile(srcPath, destPath); err != nil { + return err + } + } + } + return nil +} + +// copyFile copies a single file from src to dest, preserving permissions. +func copyFile(src, dest string) error { + srcFile, err := os.Open(src) + if err != nil { + return err + } + defer srcFile.Close() + + srcInfo, err := srcFile.Stat() + if err != nil { + return err + } + + destFile, err := os.OpenFile(dest, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, srcInfo.Mode()) + if err != nil { + return err + } + defer destFile.Close() + + _, err = io.Copy(destFile, srcFile) + return err +} + // Git wraps git operations for a working directory. type Git struct { workDir string @@ -140,8 +216,8 @@ func (g *Git) Clone(url, dest string) error { return g.wrapError(err, stdout.String(), stderr.String(), []string{"clone", url}) } - // Move to final destination - if err := os.Rename(tmpDest, dest); err != nil { + // Move to final destination (handles cross-filesystem moves) + if err := moveDir(tmpDest, dest); err != nil { return fmt.Errorf("moving clone to destination: %w", err) } @@ -180,8 +256,8 @@ func (g *Git) CloneWithReference(url, dest, reference string) error { return g.wrapError(err, stdout.String(), stderr.String(), []string{"clone", "--reference-if-able", url}) } - // Move to final destination - if err := os.Rename(tmpDest, dest); err != nil { + // Move to final destination (handles cross-filesystem moves) + if err := moveDir(tmpDest, dest); err != nil { return fmt.Errorf("moving clone to destination: %w", err) } @@ -220,8 +296,8 @@ func (g *Git) CloneBare(url, dest string) error { return g.wrapError(err, stdout.String(), stderr.String(), []string{"clone", "--bare", url}) } - // Move to final destination - if err := os.Rename(tmpDest, dest); err != nil { + // Move to final destination (handles cross-filesystem moves) + if err := moveDir(tmpDest, dest); err != nil { return fmt.Errorf("moving clone to destination: %w", err) } @@ -302,8 +378,8 @@ func (g *Git) CloneBareWithReference(url, dest, reference string) error { return g.wrapError(err, stdout.String(), stderr.String(), []string{"clone", "--bare", "--reference-if-able", url}) } - // Move to final destination - if err := os.Rename(tmpDest, dest); err != nil { + // Move to final destination (handles cross-filesystem moves) + if err := moveDir(tmpDest, dest); err != nil { return fmt.Errorf("moving clone to destination: %w", err) }