-
Notifications
You must be signed in to change notification settings - Fork 1
SPEC-010: conformance audit — reverse sweep + folded recognition in global status #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| package agent | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "sort" | ||
| "strings" | ||
| ) | ||
|
|
||
| // audit.go implements the SPEC-010 Phase 1 reverse sweep: enumerate | ||
| // what actually occupies the managed subdirs of each per-tool | ||
| // directory and classify every entry the forward pass (computeStatus) | ||
| // did not already claim. | ||
| // | ||
| // The sweep is read-only and bounded by toolSweepDirs — the tool root | ||
| // itself is application state (credentials, sessions, caches) and is | ||
| // NEVER enumerated. See SPEC-010 §Hard boundary. | ||
|
|
||
| const ( | ||
| // StateForeign: an entry in a managed subdir that no .agents/ | ||
| // artifact claims and that does not point into the .agents/ tree. | ||
| // Hands-off by rule 2 of SPEC-010 — reported, never mutated. | ||
| StateForeign = "foreign" | ||
|
|
||
| // StateOrphaned: a symlink pointing into the .agents/ tree that no | ||
| // current artifact claims — typically left behind by an artifact | ||
| // removal or rename. Prune candidate. | ||
| StateOrphaned = "orphaned" | ||
|
|
||
| // StateFolded: the expected destination resolves to the canonical | ||
| // artifact through an ancestor symlink (e.g. a dir-level | ||
| // .claude/skills/<name> -> .agents/skills/<name> link). Conformant | ||
| // at a coarser granularity than a fresh sync would create. | ||
| StateFolded = "folded" | ||
| ) | ||
|
|
||
| // toolSweepDirs returns the managed artifact subdirs the audit may | ||
| // enumerate for one tool at global scope. Anything not listed here is | ||
| // out of bounds — in particular the tool root, which holds the tool's | ||
| // own application state. | ||
| // | ||
| // Copilot and Codex have no per-artifact dirs (single concat file, | ||
| // already covered by concat states), so they sweep nothing. | ||
| func toolSweepDirs(tool Tool, parent string) []string { | ||
| switch tool.ID { | ||
| case "claude": | ||
| base := filepath.Join(parent, ".claude") | ||
| var dirs []string | ||
| for _, d := range []string{"skills", "commands", "rules", "agents", "plans", "specs", "adrs"} { | ||
| dirs = append(dirs, filepath.Join(base, d)) | ||
| } | ||
| return dirs | ||
| case "codeium": | ||
| return []string{filepath.Join(parent, ".codeium", "windsurf", "global_workflows")} | ||
| case "cursor": | ||
| return []string{filepath.Join(parent, ".cursor", "rules")} | ||
| default: | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| // sweepUnmanaged enumerates the managed subdirs of each tool and | ||
| // returns one StatusEntry per entry the forward pass did not claim, | ||
| // classified foreign or orphaned. | ||
| // | ||
| // expected is the set of destination paths the forward pass computed | ||
| // (symlink-strategy destinations only). A directory entry is claimed | ||
| // either directly (its own path is expected) or as skill scaffolding | ||
| // (its <path>/SKILL.md is expected — Claude skills link the SKILL.md | ||
| // inside a per-skill dir). | ||
| // | ||
| // agentsRoot is the canonical tree; a symlink resolving inside it but | ||
| // absent from expected is orphaned rather than foreign. | ||
| // | ||
| // The sweep is one level deep by design: a foreign directory is one | ||
| // row, not a recursive listing — the audit sizes the mess, it doesn't | ||
| // inventory other tools' internals. | ||
| func sweepUnmanaged(tools []Tool, parent, agentsRoot string, expected map[string]bool) []StatusEntry { | ||
| var rows []StatusEntry | ||
| agentsRootAbs, err := filepath.Abs(agentsRoot) | ||
| if err != nil { | ||
| agentsRootAbs = agentsRoot | ||
| } | ||
|
|
||
| for _, tool := range tools { | ||
| for _, dir := range toolSweepDirs(tool, parent) { | ||
| entries, err := os.ReadDir(dir) | ||
| if err != nil { | ||
| continue // absent dir: nothing to audit | ||
| } | ||
| for _, e := range entries { | ||
| full := filepath.Join(dir, e.Name()) | ||
| if expected[full] || expected[filepath.Join(full, "SKILL.md")] { | ||
| continue // claimed — the forward pass reports it | ||
| } | ||
| rows = append(rows, classifyUnclaimed(tool.ID, full, agentsRootAbs)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| sort.Slice(rows, func(i, j int) bool { | ||
| if rows[i].Tool != rows[j].Tool { | ||
| return rows[i].Tool < rows[j].Tool | ||
| } | ||
| return rows[i].DestinationPath < rows[j].DestinationPath | ||
| }) | ||
| return rows | ||
| } | ||
|
|
||
| // classifyUnclaimed decides foreign vs orphaned for one unclaimed | ||
| // entry. Only a symlink whose target lies inside the .agents/ tree is | ||
| // orphaned; everything else — real files, real dirs, links elsewhere — | ||
| // is foreign and hands-off. | ||
| func classifyUnclaimed(toolID, path, agentsRootAbs string) StatusEntry { | ||
| row := StatusEntry{ | ||
| Tool: toolID, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 suggestion: In |
||
| ArtifactName: filepath.Base(path), | ||
| DestinationPath: path, | ||
| State: StateForeign, | ||
| Detail: "untracked — left alone", | ||
| } | ||
|
|
||
| fi, err := os.Lstat(path) | ||
| if err != nil || fi.Mode()&os.ModeSymlink == 0 { | ||
| return row | ||
| } | ||
| target, err := os.Readlink(path) | ||
| if err != nil { | ||
| return row | ||
| } | ||
| if !filepath.IsAbs(target) { | ||
| target = filepath.Join(filepath.Dir(path), target) | ||
| } | ||
| target = filepath.Clean(target) | ||
| if target == agentsRootAbs || strings.HasPrefix(target, agentsRootAbs+string(filepath.Separator)) { | ||
| row.State = StateOrphaned | ||
| row.Detail = fmt.Sprintf("points at %s but no artifact claims it", target) | ||
| } | ||
| return row | ||
| } | ||
|
|
||
| // foldedResolves reports whether destPath, after resolving every | ||
| // symlink along it, lands on the same file as wantTarget — i.e. an | ||
| // ancestor symlink (a fold) already routes the destination to the | ||
| // canonical artifact even though destPath itself is not a link. A | ||
| // plain real file at destPath resolves to itself and can never equal | ||
| // a distinct wantTarget, so conflicts are not misclassified. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 warning: |
||
| func foldedResolves(destPath, wantTarget string) bool { | ||
| rd, err := filepath.EvalSymlinks(destPath) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| rw, err := filepath.EvalSymlinks(wantTarget) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| return rd == rw | ||
| } | ||
|
|
||
| // auditSummary renders the one-line count summary for the full row | ||
| // set (forward pass + sweep), keyed and ordered by state. | ||
| func auditSummary(rows []StatusEntry) string { | ||
| counts := map[string]int{} | ||
| for _, r := range rows { | ||
| counts[r.State]++ | ||
| } | ||
| states := make([]string, 0, len(counts)) | ||
| for s := range counts { | ||
| states = append(states, s) | ||
| } | ||
| sort.Strings(states) | ||
| parts := make([]string, 0, len(states)) | ||
| for _, s := range states { | ||
| parts = append(parts, fmt.Sprintf("%d %s", counts[s], s)) | ||
| } | ||
| return "audit: " + strings.Join(parts, ", ") | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| package agent | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
| "testing" | ||
| ) | ||
|
|
||
| // audit_test.go covers the SPEC-010 Phase 1 acceptance criteria: the | ||
| // reverse sweep (foreign / orphaned) and folded-ancestor recognition. | ||
|
|
||
| // AC-3: an entry in a managed subdir that no artifact claims and that | ||
| // doesn't point into .agents/ reports foreign — and stays untouched. | ||
| func TestGlobalStatus_ForeignReported(t *testing.T) { | ||
| a, root, stdout := newStatusTestApp(t) | ||
| seedRule(t, a.ResolveGlobalRoot(), "managed", "body\n") | ||
|
|
||
| foreign := filepath.Join(root, ".claude", "skills", "hundred-million-offers") | ||
| if err := os.MkdirAll(foreign, 0o755); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| marker := filepath.Join(foreign, "SKILL.md") | ||
| if err := os.WriteFile(marker, []byte("# not ours\n"), 0o644); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| if err := a.CmdGlobalStatus(GlobalStatusOpts{}); err != nil { | ||
| t.Fatalf("status: %v", err) | ||
| } | ||
| out := stdout.String() | ||
| if !strings.Contains(out, "[foreign] claude/hundred-million-offers") { | ||
| t.Errorf("expected foreign row for hundred-million-offers:\n%s", out) | ||
| } | ||
| // Untouched: read-only audit. | ||
| if _, err := os.Stat(marker); err != nil { | ||
| t.Errorf("audit mutated a foreign entry: %v", err) | ||
| } | ||
| } | ||
|
|
||
| // AC-4: a symlink into the .agents/ tree with no claiming artifact | ||
| // reports orphaned, not foreign. | ||
| func TestGlobalStatus_OrphanedSymlink(t *testing.T) { | ||
| a, root, stdout := newStatusTestApp(t) | ||
| seedRule(t, a.ResolveGlobalRoot(), "managed", "body\n") | ||
|
|
||
| commandsDir := filepath.Join(root, ".claude", "commands") | ||
| if err := os.MkdirAll(commandsDir, 0o755); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| // Points into .agents/ but the artifact is gone. | ||
| gone := filepath.Join(a.ResolveGlobalRoot(), "workflows", "deleted.md") | ||
| if err := os.Symlink(gone, filepath.Join(commandsDir, "deleted.md")); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| if err := a.CmdGlobalStatus(GlobalStatusOpts{}); err != nil { | ||
| t.Fatalf("status: %v", err) | ||
| } | ||
| out := stdout.String() | ||
| if !strings.Contains(out, "[orphaned] claude/deleted.md") { | ||
| t.Errorf("expected orphaned row for deleted.md:\n%s", out) | ||
| } | ||
| } | ||
|
|
||
| // AC-2: a dir-level symlink into .agents/ that resolves the expected | ||
| // artifact reports folded — and the sweep does not double-report it. | ||
| func TestGlobalStatus_FoldedSkillRecognized(t *testing.T) { | ||
| a, root, stdout := newStatusTestApp(t) | ||
| seedSkill(t, a.ResolveGlobalRoot(), "decision-helper", | ||
| "---\nname: decision-helper\n---\nbody\n", nil) | ||
|
|
||
| skillsDir := filepath.Join(root, ".claude", "skills") | ||
| if err := os.MkdirAll(skillsDir, 0o755); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| // The fold: dir-level link instead of the per-SKILL.md link a | ||
| // fresh sync would create. | ||
| src := filepath.Join(a.ResolveGlobalRoot(), "skills", "decision-helper") | ||
| if err := os.Symlink(src, filepath.Join(skillsDir, "decision-helper")); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| if err := a.CmdGlobalStatus(GlobalStatusOpts{}); err != nil { | ||
| t.Fatalf("status: %v", err) | ||
| } | ||
| out := stdout.String() | ||
| if !strings.Contains(out, "[folded] claude/skill/decision-helper") { | ||
| t.Errorf("expected folded row for decision-helper:\n%s", out) | ||
| } | ||
| if strings.Contains(out, "[foreign] claude/decision-helper") || | ||
| strings.Contains(out, "[orphaned] claude/decision-helper") { | ||
| t.Errorf("folded skill double-reported by the sweep:\n%s", out) | ||
| } | ||
| if strings.Contains(out, "[not-a-symlink] claude/skill/decision-helper") { | ||
| t.Errorf("folded skill misclassified as conflict:\n%s", out) | ||
| } | ||
| } | ||
|
|
||
| // AC-1: a real dir shadowing a claimed artifact is still a conflict | ||
| // (not-a-symlink), never folded/foreign. | ||
| func TestGlobalStatus_ConflictStillReported(t *testing.T) { | ||
| a, root, stdout := newStatusTestApp(t) | ||
| seedSkill(t, a.ResolveGlobalRoot(), "find-skills", | ||
| "---\nname: find-skills\n---\nbody\n", nil) | ||
|
|
||
| shadow := filepath.Join(root, ".claude", "skills", "find-skills") | ||
| if err := os.MkdirAll(shadow, 0o755); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if err := os.WriteFile(filepath.Join(shadow, "SKILL.md"), []byte("# local fork\n"), 0o644); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| if err := a.CmdGlobalStatus(GlobalStatusOpts{}); err != nil { | ||
| t.Fatalf("status: %v", err) | ||
| } | ||
| out := stdout.String() | ||
| if !strings.Contains(out, "[not-a-symlink] claude/skill/find-skills") { | ||
| t.Errorf("expected conflict row for find-skills:\n%s", out) | ||
| } | ||
| } | ||
|
|
||
| // AC-5: tool app state (files outside the managed subdirs) never | ||
| // appears in output. | ||
| func TestGlobalStatus_AppStateNeverSwept(t *testing.T) { | ||
| a, root, stdout := newStatusTestApp(t) | ||
| seedRule(t, a.ResolveGlobalRoot(), "managed", "body\n") | ||
|
|
||
| claude := filepath.Join(root, ".claude") | ||
| if err := os.MkdirAll(filepath.Join(claude, "sessions"), 0o755); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| for _, f := range []string{".credentials.json", "history.jsonl", "settings.json"} { | ||
| if err := os.WriteFile(filepath.Join(claude, f), []byte("secret"), 0o600); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| } | ||
|
|
||
| if err := a.CmdGlobalStatus(GlobalStatusOpts{}); err != nil { | ||
| t.Fatalf("status: %v", err) | ||
| } | ||
| out := stdout.String() | ||
| for _, banned := range []string{".credentials.json", "history.jsonl", "sessions", "settings.json"} { | ||
| if strings.Contains(out, banned) { | ||
| t.Errorf("app-state entry %q leaked into audit output:\n%s", banned, out) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // The summary line counts every state across forward pass + sweep. | ||
| func TestGlobalStatus_SummaryLine(t *testing.T) { | ||
| a, root, stdout := newStatusTestApp(t) | ||
| seedRule(t, a.ResolveGlobalRoot(), "managed", "body\n") | ||
| if err := os.MkdirAll(filepath.Join(root, ".claude", "skills", "stray"), 0o755); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| if err := a.CmdGlobalStatus(GlobalStatusOpts{}); err != nil { | ||
| t.Fatalf("status: %v", err) | ||
| } | ||
| out := stdout.String() | ||
| if !strings.Contains(out, "audit:") { | ||
| t.Errorf("expected audit summary line:\n%s", out) | ||
| } | ||
| if !strings.Contains(out, "1 foreign") { | ||
| t.Errorf("expected '1 foreign' in summary:\n%s", out) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 suggestion: In
-inlinesweepUnmanaged, the checkexpected[full] || expected[filepath.Join(full, "SKILL.md")]only handles the case where a skill's SKILL.md is expected. However, the forward pass may also claim the skill directory itself (e.g., for folded skills). If a skill directory is claimed via a fold, it should be added toexpectedto prevent double-reporting. Consider adding the directory path toexpectedwhen a fold is detected.