From 36ce7dd747b6e220f01d23b2ee2bf7d1cef55878 Mon Sep 17 00:00:00 2001 From: m3ta-chiron Date: Sat, 22 Aug 2026 21:56:21 +0200 Subject: [PATCH] feat: collision hard errors + rename resolution - flat per-type deployed namespace enforced across own items and all sources - own-vs-vendored and vendored-vs-vendored collisions abort with both parties named - --rename upstream=deployed resolves collisions; on-disk folder always equals deployed name; rename targets are validated against new collisions - checks run before any tree mutation; validate re-checks the namespace offline --- .beads/interactions.jsonl | 1 + internal/cli/vendor.go | 10 ++- internal/e2e/collide_test.go | 121 +++++++++++++++++++++++++ internal/vendor/add.go | 8 ++ internal/vendor/collide.go | 168 +++++++++++++++++++++++++++++++++++ internal/vendor/query.go | 1 + 6 files changed, 308 insertions(+), 1 deletion(-) create mode 100644 internal/e2e/collide_test.go create mode 100644 internal/vendor/collide.go diff --git a/.beads/interactions.jsonl b/.beads/interactions.jsonl index 5f6cb6c..2798eb4 100644 --- a/.beads/interactions.jsonl +++ b/.beads/interactions.jsonl @@ -13,3 +13,4 @@ {"id":"int-eaab83169be77c1e6a853e502b70362f","kind":"field_change","created_at":"2026-06-13T08:17:27.20786458Z","actor":"m3ta-chiron","issue_id":"agent-lib-feu","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Babysitter Deployment Plan exploration completed. Artifact: .a5c/deployment-plan-exploration.md. Run: 01KV00D3F73F6KY5Q8W4TFER5X"}} {"id":"int-4d5bf5dc890a6963a0e1110a8fb71635","kind":"field_change","created_at":"2026-08-22T19:43:28.592284059Z","actor":"m3ta-chiron","issue_id":"agent-lib-j2w.1","extra":{"field":"status","new_value":"closed","old_value":"open"}} {"id":"int-090714c2a2d985183cc965b11b5c53d8","kind":"field_change","created_at":"2026-08-22T19:52:30.546382863Z","actor":"m3ta-chiron","issue_id":"agent-lib-j2w.10","extra":{"field":"status","new_value":"closed","old_value":"in_progress"}} +{"id":"int-c5834aa5e95b421826d076fd129d7a1a","kind":"field_change","created_at":"2026-08-22T19:54:32.168882535Z","actor":"m3ta-chiron","issue_id":"agent-lib-j2w.2","extra":{"field":"status","new_value":"closed","old_value":"in_progress"}} diff --git a/internal/cli/vendor.go b/internal/cli/vendor.go index 3b2fa7b..bf24096 100644 --- a/internal/cli/vendor.go +++ b/internal/cli/vendor.go @@ -23,20 +23,27 @@ func newVendorCmd() *cobra.Command { func newVendorAddCmd() *cobra.Command { var ref string var include []string + var renames []string cmd := &cobra.Command{ Use: "add ", Short: "Vendor a new external git source", Long: "Clones the source (in-process, via go-git), discovers skills, commands,\n" + "agents and MCP fragments, materializes the selection under external//\n" + "and pins URL, ref, revision, selection and renames in the lockfile.\n" + - "Default selection is everything; --include switches to an explicit list.", + "Default selection is everything; --include switches to an explicit list.\n" + + "Collisions abort with a hard error; resolve them with --rename upstream=deployed.", Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { + renameMap, err := vendor.ParseRenames(renames) + if err != nil { + return err + } opts := vendor.AddOptions{ Name: args[0], URL: args[1], Ref: ref, Include: splitList(include), + Renames: renameMap, } if err := vendor.Add(".", opts, cmd.OutOrStdout()); err != nil { return fmt.Errorf("vendor add: %w", err) @@ -46,6 +53,7 @@ func newVendorAddCmd() *cobra.Command { } cmd.Flags().StringVar(&ref, "ref", "", "branch or tag to track (default: the source's default branch)") cmd.Flags().StringSliceVar(&include, "include", nil, "comma-separated item ids to vendor (switches selection to include mode)") + cmd.Flags().StringSliceVar(&renames, "rename", nil, "deployed-name override upstream-id=deployed-name (repeatable)") return cmd } diff --git a/internal/e2e/collide_test.go b/internal/e2e/collide_test.go new file mode 100644 index 0000000..7bf8f7f --- /dev/null +++ b/internal/e2e/collide_test.go @@ -0,0 +1,121 @@ +package e2e + +import ( + "os" + "path/filepath" + "testing" + + "github.com/m3tam3re/agent-lib/internal/lockfile" +) + +func ownSkillFixture(h *harness, t *testing.T) { + t.Helper() + dir := filepath.Join(h.workDir, "skills", "good-skill") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("---\nname: Own Good\n---\n"), 0o644) +} + +func TestCollisionOwnVsVendored(t *testing.T) { + h := newHarness(t) + ownSkillFixture(h, t) + standardFixture(h, t) + + out, err := h.run(t, "vendor", "add", "superpowers", h.upstreamDir) + if err == nil { + t.Fatalf("own-vs-vendored collision must fail:\n%s", out) + } + for _, want := range []string{"collides", "own", "good-skill"} { + if !contains(out, want) { + t.Errorf("error must contain %q:\n%s", want, out) + } + } + if _, err := os.Stat(filepath.Join(h.workDir, "external")); err == nil { + t.Error("collision must abort before any tree mutation") + } + if _, err := os.Stat(filepath.Join(h.workDir, lockfile.FileName)); err == nil { + t.Error("collision must not write a lockfile") + } +} + +func TestCollisionVendoredVsVendored(t *testing.T) { + h := newHarness(t) + standardFixture(h, t) + h.mustRun(t, "vendor", "add", "first", h.upstreamDir) + + out, err := h.run(t, "vendor", "add", "second", h.upstreamDir) + if err == nil { + t.Fatalf("vendored-vs-vendored collision must fail:\n%s", out) + } + for _, want := range []string{"collides", "first"} { + if !contains(out, want) { + t.Errorf("error must name both parties (%q missing):\n%s", want, out) + } + } +} + +func TestCollisionResolvedByRename(t *testing.T) { + h := newHarness(t) + ownSkillFixture(h, t) + standardFixture(h, t) + + out := h.mustRun(t, "vendor", "add", "superpowers", h.upstreamDir, + "--rename", "good-skill=superpowers-good-skill") + if !contains(out, "renamed") && !contains(out, "materialized") { + t.Errorf("rename resolution should report success:\n%s", out) + } + + renamed := filepath.Join(h.workDir, "external/superpowers/skills/superpowers-good-skill/SKILL.md") + if _, err := os.Stat(renamed); err != nil { + t.Fatal("renamed folder must carry the deployed name") + } + original := filepath.Join(h.workDir, "external/superpowers/skills/good-skill") + if _, err := os.Stat(original); err == nil { + t.Error("upstream folder name must not appear after rename") + } + + lock := readFile(t, filepath.Join(h.workDir, lockfile.FileName)) + if !contains(lock, `"good-skill": "superpowers-good-skill"`) { + t.Errorf("rename map must be pinned in the lockfile:\n%s", lock) + } + + if out := h.mustRun(t, "validate"); !contains(out, "valid") { + t.Errorf("renamed state must validate:\n%s", out) + } +} + +func TestCollisionRenameToAnotherCollision(t *testing.T) { + h := newHarness(t) + ownSkillFixture(h, t) + standardFixture(h, t) + + out, err := h.run(t, "vendor", "add", "superpowers", h.upstreamDir, + "--rename", "good-skill=broken-skill") + if err == nil { + t.Fatalf("renaming onto an existing deployed name must fail:\n%s", out) + } + if !contains(out, "collides") { + t.Errorf("error must report the collision:\n%s", out) + } +} + +func TestValidateDetectsHandEditedCollision(t *testing.T) { + h := newHarness(t) + standardFixture(h, t) + h.mustRun(t, "vendor", "add", "superpowers", h.upstreamDir) + h.mustRun(t, "vendor", "add", "second-source", h.upstreamDir, "--include", "good-skill", + "--rename", "good-skill=second-good") + + mutateLockfile(t, h, func(lf *lockfile.Lockfile) { + lf.Sources["second-source"].Renames["good-skill"] = "broken-skill" + }) + + out, err := h.run(t, "validate") + if err == nil { + t.Fatalf("hand-edited rename collision must fail validation:\n%s", out) + } + if !contains(out, "collision") { + t.Errorf("rule must be reported as collision:\n%s", out) + } +} diff --git a/internal/vendor/add.go b/internal/vendor/add.go index a970ca9..76e5456 100644 --- a/internal/vendor/add.go +++ b/internal/vendor/add.go @@ -23,6 +23,7 @@ type AddOptions struct { URL string Ref string Include []string + Renames map[string]string } // Add vendors a new source into the work repository at workDir: it clones, @@ -78,6 +79,10 @@ func Add(workDir string, opts AddOptions, stdout io.Writer) error { return fmt.Errorf("source %q selected zero items; refusing to add an empty source", opts.Name) } + if err := checkCollisions(workDir, lf, opts.Name, selected, opts.Renames); err != nil { + return err + } + src := &lockfile.Source{ URL: normalized, Ref: repo.Ref, @@ -85,6 +90,9 @@ func Add(workDir string, opts AddOptions, stdout io.Writer) error { Selection: sel, Renames: map[string]string{}, } + if len(opts.Renames) > 0 { + src.Renames = opts.Renames + } if cfg != (lockfile.Discovery{}) { src.Discovery = &cfg } diff --git a/internal/vendor/collide.go b/internal/vendor/collide.go new file mode 100644 index 0000000..427ce86 --- /dev/null +++ b/internal/vendor/collide.go @@ -0,0 +1,168 @@ +package vendor + +import ( + "fmt" + "sort" + "strings" + + "github.com/m3tam3re/agent-lib/internal/discovery" + "github.com/m3tam3re/agent-lib/internal/lockfile" +) + +// CollisionError names both parties of a deployed-name clash. +type CollisionError struct { + Type string `json:"type"` + DeployedID string `json:"deployed_id"` + Source string `json:"source"` + OtherOwner string `json:"other_owner"` + OtherID string `json:"other_id"` +} + +func (e *CollisionError) Error() string { + return fmt.Sprintf("source %q: %s %q collides with %s %s %q", + e.Source, e.Type, e.DeployedID, e.OtherOwner, e.Type, e.OtherID) +} + +// deployedIndex maps type -> deployed name -> owner description, built from +// the work repository's own items and every lockfile source except skip. +type deployedIndex map[string]map[string]owner + +type owner struct { + source string + id string +} + +func ownDeployed(workDir string) (deployedIndex, error) { + items, err := discovery.Scan(&discovery.FsTree{Root: workDir}, discovery.Config{}) + if err != nil { + return nil, err + } + idx := deployedIndex{} + for _, it := range items { + idx.add(it.Type, it.UpstreamID, owner{source: "own", id: it.UpstreamID}) + } + return idx, nil +} + +func lockfileDeployed(lf *lockfile.Lockfile, skip string) deployedIndex { + idx := deployedIndex{} + for name, src := range lf.Sources { + if name == skip { + continue + } + for typ, ids := range src.Inventory { + for _, id := range ids { + idx.add(typ, deployedName(id, src.Renames), owner{source: name, id: id}) + } + } + } + return idx +} + +func (d deployedIndex) add(typ, deployed string, o owner) { + if d[typ] == nil { + d[typ] = map[string]owner{} + } + d[typ][deployed] = o +} + +func (d deployedIndex) find(typ, deployed string) (owner, bool) { + o, ok := d[typ][deployed] + return o, ok +} + +func mergeIndex(a, b deployedIndex) deployedIndex { + out := deployedIndex{} + for typ, m := range a { + for dep, o := range m { + out.add(typ, dep, o) + } + } + for typ, m := range b { + for dep, o := range m { + out.add(typ, dep, o) + } + } + return out +} + +// checkCollisions verifies that every selected item of source deploys into +// the flat namespace without clashing — against own items, against other +// sources, and within the source itself (renames included). It performs no +// filesystem mutation. +func checkCollisions(workDir string, lf *lockfile.Lockfile, source string, selected []discovery.Item, renames map[string]string) error { + own, err := ownDeployed(workDir) + if err != nil { + return fmt.Errorf("scanning own items: %w", err) + } + existing := mergeIndex(own, lockfileDeployed(lf, source)) + + seen := map[string]string{} + for _, it := range selected { + deployed := deployedName(it.UpstreamID, renames) + if deployed == "" { + return fmt.Errorf("source %q: rename of %q maps to an empty name", source, it.UpstreamID) + } + if prev, dup := seen[deployed]; dup { + return &CollisionError{ + Type: it.Type, DeployedID: deployed, Source: source, + OtherOwner: "source " + source, OtherID: prev, + } + } + seen[deployed] = it.UpstreamID + if o, clash := existing.find(it.Type, deployed); clash { + return &CollisionError{ + Type: it.Type, DeployedID: deployed, Source: source, + OtherOwner: o.source, OtherID: o.id, + } + } + } + return nil +} + +// validateCollisions re-checks the deployed namespace across own items and +// all lockfile sources — the offline equivalent used by validate. +func validateCollisions(workDir string, lf *lockfile.Lockfile) []ValidationError { + own, err := ownDeployed(workDir) + if err != nil { + return []ValidationError{{Rule: "collision", Message: fmt.Sprintf("scanning own items: %v", err)}} + } + idx := mergeIndex(own, deployedIndex{}) + var errs []ValidationError + for _, name := range sortedSourceNames(lf) { + src := lf.Sources[name] + for typ, ids := range src.Inventory { + for _, id := range ids { + deployed := deployedName(id, src.Renames) + if prev, dup := idx.find(typ, deployed); dup { + errs = append(errs, ValidationError{ + Source: name, + Rule: "collision", + Message: fmt.Sprintf("source %q: %s %q (upstream %q) collides with %s %q", + name, typ, deployed, id, prev.source, prev.id), + }) + continue + } + idx.add(typ, deployed, owner{source: name, id: id}) + } + } + } + sort.Slice(errs, func(i, j int) bool { return errs[i].Message < errs[j].Message }) + return errs +} + +// ParseRenames converts --rename upstream-id=deployed-name flag values. +func ParseRenames(pairs []string) (map[string]string, error) { + if len(pairs) == 0 { + return nil, nil + } + out := map[string]string{} + for _, p := range pairs { + upstream, deployed, ok := strings.Cut(p, "=") + if !ok || strings.TrimSpace(upstream) == "" || strings.TrimSpace(deployed) == "" { + return nil, fmt.Errorf("invalid --rename %q (want upstream-id=deployed-name)", p) + } + out[strings.TrimSpace(upstream)] = strings.TrimSpace(deployed) + } + return out, nil +} diff --git a/internal/vendor/query.go b/internal/vendor/query.go index 7b4e813..ab95764 100644 --- a/internal/vendor/query.go +++ b/internal/vendor/query.go @@ -33,6 +33,7 @@ func Validate(workDir string, lf *lockfile.Lockfile) Validation { v.checkSelection(name, src) v.checkExternalArea(workDir, name, src) } + v.Errors = append(v.Errors, validateCollisions(workDir, lf)...) sort.Slice(v.Errors, func(i, j int) bool { if v.Errors[i].Source != v.Errors[j].Source { return v.Errors[i].Source < v.Errors[j].Source