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
This commit is contained in:
@@ -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-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-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-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"}}
|
||||||
|
|||||||
@@ -23,20 +23,27 @@ func newVendorCmd() *cobra.Command {
|
|||||||
func newVendorAddCmd() *cobra.Command {
|
func newVendorAddCmd() *cobra.Command {
|
||||||
var ref string
|
var ref string
|
||||||
var include []string
|
var include []string
|
||||||
|
var renames []string
|
||||||
cmd := &cobra.Command{
|
cmd := &cobra.Command{
|
||||||
Use: "add <name> <url>",
|
Use: "add <name> <url>",
|
||||||
Short: "Vendor a new external git source",
|
Short: "Vendor a new external git source",
|
||||||
Long: "Clones the source (in-process, via go-git), discovers skills, commands,\n" +
|
Long: "Clones the source (in-process, via go-git), discovers skills, commands,\n" +
|
||||||
"agents and MCP fragments, materializes the selection under external/<name>/\n" +
|
"agents and MCP fragments, materializes the selection under external/<name>/\n" +
|
||||||
"and pins URL, ref, revision, selection and renames in the lockfile.\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),
|
Args: cobra.ExactArgs(2),
|
||||||
RunE: func(cmd *cobra.Command, args []string) error {
|
RunE: func(cmd *cobra.Command, args []string) error {
|
||||||
|
renameMap, err := vendor.ParseRenames(renames)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
opts := vendor.AddOptions{
|
opts := vendor.AddOptions{
|
||||||
Name: args[0],
|
Name: args[0],
|
||||||
URL: args[1],
|
URL: args[1],
|
||||||
Ref: ref,
|
Ref: ref,
|
||||||
Include: splitList(include),
|
Include: splitList(include),
|
||||||
|
Renames: renameMap,
|
||||||
}
|
}
|
||||||
if err := vendor.Add(".", opts, cmd.OutOrStdout()); err != nil {
|
if err := vendor.Add(".", opts, cmd.OutOrStdout()); err != nil {
|
||||||
return fmt.Errorf("vendor add: %w", err)
|
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().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(&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
|
return cmd
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
Vendored
+8
@@ -23,6 +23,7 @@ type AddOptions struct {
|
|||||||
URL string
|
URL string
|
||||||
Ref string
|
Ref string
|
||||||
Include []string
|
Include []string
|
||||||
|
Renames map[string]string
|
||||||
}
|
}
|
||||||
|
|
||||||
// Add vendors a new source into the work repository at workDir: it clones,
|
// 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)
|
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{
|
src := &lockfile.Source{
|
||||||
URL: normalized,
|
URL: normalized,
|
||||||
Ref: repo.Ref,
|
Ref: repo.Ref,
|
||||||
@@ -85,6 +90,9 @@ func Add(workDir string, opts AddOptions, stdout io.Writer) error {
|
|||||||
Selection: sel,
|
Selection: sel,
|
||||||
Renames: map[string]string{},
|
Renames: map[string]string{},
|
||||||
}
|
}
|
||||||
|
if len(opts.Renames) > 0 {
|
||||||
|
src.Renames = opts.Renames
|
||||||
|
}
|
||||||
if cfg != (lockfile.Discovery{}) {
|
if cfg != (lockfile.Discovery{}) {
|
||||||
src.Discovery = &cfg
|
src.Discovery = &cfg
|
||||||
}
|
}
|
||||||
|
|||||||
Vendored
+168
@@ -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
|
||||||
|
}
|
||||||
Vendored
+1
@@ -33,6 +33,7 @@ func Validate(workDir string, lf *lockfile.Lockfile) Validation {
|
|||||||
v.checkSelection(name, src)
|
v.checkSelection(name, src)
|
||||||
v.checkExternalArea(workDir, name, src)
|
v.checkExternalArea(workDir, name, src)
|
||||||
}
|
}
|
||||||
|
v.Errors = append(v.Errors, validateCollisions(workDir, lf)...)
|
||||||
sort.Slice(v.Errors, func(i, j int) bool {
|
sort.Slice(v.Errors, func(i, j int) bool {
|
||||||
if v.Errors[i].Source != v.Errors[j].Source {
|
if v.Errors[i].Source != v.Errors[j].Source {
|
||||||
return v.Errors[i].Source < v.Errors[j].Source
|
return v.Errors[i].Source < v.Errors[j].Source
|
||||||
|
|||||||
Reference in New Issue
Block a user