From 63619b90a4fb606a074832eb16f34d6c326d2330 Mon Sep 17 00:00:00 2001 From: m3ta-chiron Date: Sun, 23 Aug 2026 10:36:44 +0200 Subject: [PATCH] feat: three-state sync planner with warning semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Plan is a pure function (inventory, manifest, disk facts, injected path mapping) emitting exactly six actions: deploy, update, skip+warn, leave+warn, remove, keep — deterministic order, no filesystem access - full 3x3 matrix (disk unmodified/user-modified/never-managed x upstream added/changed/removed) table-tested plus missing-disk restore cases, a mixed fixture and a purity/determinism guarantee test - executor applies plans item by item (atomic deploy/remove per item), rewrites the manifest once, appends timestamped SKIP/LEAVE lines with item name and reason to the sync log - sync now plans before mutating: unmanaged occupants block new deploys, upstream deletions remove only unmodified copies, local modifications always win and are warned about --- .beads/interactions.jsonl | 1 + internal/client/executor.go | 163 ++++++++++++++++++++++++ internal/client/planner.go | 145 ++++++++++++++++++++++ internal/client/planner_test.go | 211 ++++++++++++++++++++++++++++++++ internal/client/sync.go | 122 ++++-------------- internal/e2e/warn_test.go | 180 +++++++++++++++++++++++++++ internal/target/opencode.go | 30 +++++ 7 files changed, 751 insertions(+), 101 deletions(-) create mode 100644 internal/client/executor.go create mode 100644 internal/client/planner.go create mode 100644 internal/client/planner_test.go create mode 100644 internal/e2e/warn_test.go diff --git a/.beads/interactions.jsonl b/.beads/interactions.jsonl index 6ffcdc2..b78f282 100644 --- a/.beads/interactions.jsonl +++ b/.beads/interactions.jsonl @@ -16,3 +16,4 @@ {"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"}} {"id":"int-b7efd31fd41bbb5acd082eb3c2f34dbd","kind":"field_change","created_at":"2026-08-22T19:56:21.734777976Z","actor":"m3ta-chiron","issue_id":"agent-lib-j2w.3","extra":{"field":"status","new_value":"closed","old_value":"in_progress"}} {"id":"int-d0f6b2e3946129f47ed8f071902a5c78","kind":"field_change","created_at":"2026-08-22T20:01:15.960626197Z","actor":"m3ta-chiron","issue_id":"agent-lib-j2w.4","extra":{"field":"status","new_value":"closed","old_value":"in_progress"}} +{"id":"int-baf99558287967cfd7c3aeb9c0e6354c","kind":"field_change","created_at":"2026-08-23T08:31:08.602480836Z","actor":"m3ta-chiron","issue_id":"agent-lib-j2w.5","extra":{"field":"status","new_value":"closed","old_value":"in_progress"}} diff --git a/internal/client/executor.go b/internal/client/executor.go new file mode 100644 index 0000000..204fce3 --- /dev/null +++ b/internal/client/executor.go @@ -0,0 +1,163 @@ +package client + +import ( + "fmt" + "io" + "os" + "time" + + "github.com/m3tam3re/agent-lib/internal/deploy" + "github.com/m3tam3re/agent-lib/internal/gitsource" + "github.com/m3tam3re/agent-lib/internal/lockfile" + "github.com/m3tam3re/agent-lib/internal/target" +) + +// SyncReport summarizes one planner-driven sync run. +type SyncReport struct { + Revision string + Deployed int + Updated int + Kept int + Removed int + Skipped int + Left int + Manifest string +} + +// ExecutePlan applies a plan item by item: deploys and removes are atomic +// per item, every skip+warn and leave+warn is appended to the warning log +// with item name and reason, and the manifest is rewritten once at the end. +func ExecutePlan(tree *gitsource.GitTree, plan []PlanEntry, manifest *Manifest, paths StatePaths, rev string, stdout io.Writer) (*SyncReport, error) { + report := &SyncReport{Revision: rev, Manifest: paths.Manifest} + var warnings []string + + for _, e := range plan { + key := manifestKey(e.Type, e.Name) + switch e.Action { + case ActDeploy, ActUpdate: + if err := deployEntry(tree, e); err != nil { + return nil, fmt.Errorf("%s %s/%s: %w", e.Action, e.Type, e.Name, err) + } + manifest.Items[key] = &ManifestItem{ + Name: e.Name, Type: e.Type, Origin: e.Origin, Revision: rev, Hash: e.NewHash, + } + if e.Action == ActDeploy { + report.Deployed++ + } else { + report.Updated++ + } + case ActKeep: + report.Kept++ + case ActRemove: + if err := os.RemoveAll(e.DstPath); err != nil { + return nil, fmt.Errorf("removing %s/%s: %w", e.Type, e.Name, err) + } + delete(manifest.Items, key) + report.Removed++ + case ActSkip: + warnings = append(warnings, fmt.Sprintf("SKIP %s/%s: %s", e.Type, e.Name, e.Reason)) + report.Skipped++ + case ActLeave: + warnings = append(warnings, fmt.Sprintf("LEAVE %s/%s: %s", e.Type, e.Name, e.Reason)) + report.Left++ + } + } + + manifest.Version = ManifestVersion + manifest.Revision = rev + if err := manifest.Save(paths.Manifest); err != nil { + return nil, err + } + if len(warnings) > 0 { + if err := appendWarnings(paths.LogFile, warnings); err != nil { + fmt.Fprintf(stdout, "warning: could not write sync log: %v\n", err) + } + } + + printPlannedSyncReport(stdout, report) + return report, nil +} + +func deployEntry(tree *gitsource.GitTree, e PlanEntry) error { + if e.Type == lockfile.TypeSkill { + return deploy.Dir(tree, e.RelPath, e.DstPath) + } + return deploy.File(tree, e.RelPath, e.DstPath) +} + +// appendWarnings adds timestamped warning lines to the sync log. +func appendWarnings(logPath string, warnings []string) error { + f, err := os.OpenFile(logPath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) + if err != nil { + return err + } + defer f.Close() + stamp := time.Now().UTC().Format(time.RFC3339) + for _, w := range warnings { + if _, err := fmt.Fprintf(f, "%s %s\n", stamp, w); err != nil { + return err + } + } + return nil +} + +// diskFacts observes every deployed path the planner can care about: +// manifest items, current inventory items, and unmanaged occupants of the +// target directories. +func diskFacts(oc *target.OpenCode, manifest *Manifest, items []PlannedItem) map[string]DiskFact { + facts := map[string]DiskFact{} + observe := func(typ, name string) { + key := manifestKey(typ, name) + if _, seen := facts[key]; seen { + return + } + path, ok := oc.DeployPath(typ, name) + if !ok { + return + } + if _, err := os.Stat(path); err != nil { + facts[key] = DiskFact{} + return + } + hash, err := HashDiskPath(typ, path) + if err != nil { + hash = "" + } + facts[key] = DiskFact{Exists: true, Hash: hash} + } + + for _, rec := range manifest.Items { + observe(rec.Type, rec.Name) + } + for _, it := range items { + observe(it.Type, it.Name) + } + for key := range oc.ScanTargetDirs() { + if _, seen := facts[key]; !seen { + facts[key] = DiskFact{Exists: true} + } + } + return facts +} + +func printPlannedSyncReport(w io.Writer, r *SyncReport) { + fmt.Fprintf(w, "synced to revision %s\n", shortRev(r.Revision)) + fmt.Fprintf(w, " deployed: %d\n", r.Deployed) + fmt.Fprintf(w, " updated: %d\n", r.Updated) + fmt.Fprintf(w, " kept: %d\n", r.Kept) + fmt.Fprintf(w, " removed: %d\n", r.Removed) + if r.Skipped > 0 { + fmt.Fprintf(w, " skipped (local modifications kept): %d\n", r.Skipped) + } + if r.Left > 0 { + fmt.Fprintf(w, " unmanaged items left untouched: %d\n", r.Left) + } + fmt.Fprintf(w, " manifest: %s\n", r.Manifest) +} + +func shortRev(rev string) string { + if len(rev) > 12 { + return rev[:12] + } + return rev +} diff --git a/internal/client/planner.go b/internal/client/planner.go new file mode 100644 index 0000000..2062165 --- /dev/null +++ b/internal/client/planner.go @@ -0,0 +1,145 @@ +package client + +import "sort" + +// Action is one step of a sync plan. +type Action string + +// The six sync actions. skip warns about managed items with local +// modifications; leave warns about self-created items agent-lib never +// managed; remove only ever touches unmodified managed copies. +const ( + ActDeploy Action = "deploy" + ActUpdate Action = "update" + ActSkip Action = "skip+warn" + ActLeave Action = "leave+warn" + ActRemove Action = "remove" + ActKeep Action = "keep" +) + +// PlannedItem is one deployable inventory item with its repo-side hash. +type PlannedItem struct { + Type string + Name string + Origin string + RelPath string + Hash string +} + +// DiskFact is the observed on-disk state of one deployed path. +type DiskFact struct { + Exists bool + Hash string +} + +// PlanEntry is one decided action with everything the executor needs. +type PlanEntry struct { + Action Action + Type string + Name string + Origin string + RelPath string + DstPath string + NewHash string + Reason string +} + +// Plan is a pure function from (inventory, manifest, on-disk facts) to an +// action plan. pathFor injects the target path mapping so the planner never +// touches the filesystem or the environment. The plan is sorted by +// (type, name) for deterministic execution and reporting. +func Plan(items []PlannedItem, manifest *Manifest, disk map[string]DiskFact, pathFor func(typ, name string) string) []PlanEntry { + invByKey := map[string]PlannedItem{} + for _, it := range items { + invByKey[manifestKey(it.Type, it.Name)] = it + } + + var plan []PlanEntry + emit := func(a Action, it PlannedItem, reason string) { + e := PlanEntry{Action: a, Reason: reason, NewHash: it.Hash} + e.Type, e.Name, e.Origin, e.RelPath = it.Type, it.Name, it.Origin, it.RelPath + e.DstPath = pathFor(e.Type, e.Name) + plan = append(plan, e) + } + + keys := map[string]bool{} + for key := range invByKey { + keys[key] = true + } + for key := range manifest.Items { + keys[key] = true + } + for key, fact := range disk { + if fact.Exists { + keys[key] = true + } + } + sorted := make([]string, 0, len(keys)) + for key := range keys { + sorted = append(sorted, key) + } + sort.Strings(sorted) + + for _, key := range sorted { + it, inInv := invByKey[key] + rec := manifest.Items[key] + fact := disk[key] + + switch { + case inInv && rec == nil: + if fact.Exists { + emit(ActSkip, it, "path occupied by a local item agent-lib never deployed; deploy blocked") + } else { + emit(ActDeploy, it, "") + } + case inInv && it.Hash == rec.Hash: + decideAgainstDisk(it, rec, fact, emit, "unchanged") + case inInv: + decideAgainstDisk(it, rec, fact, emit, "changed") + case rec != nil && !fact.Exists: + emit(ActRemove, PlannedItem{Type: rec.Type, Name: rec.Name, Origin: rec.Origin}, "already gone from disk; dropping manifest record") + case rec != nil && fact.Hash == rec.Hash: + emit(ActRemove, PlannedItem{Type: rec.Type, Name: rec.Name, Origin: rec.Origin}, "upstream deleted") + case rec != nil: + emit(ActSkip, PlannedItem{Type: rec.Type, Name: rec.Name, Origin: rec.Origin}, + "upstream deleted but the local copy was modified; keeping it") + default: + emit(ActLeave, PlannedItem{Type: typeOfKey(key), Name: nameOfKey(key)}, + "unmanaged local item; left untouched") + } + } + return plan +} + +func decideAgainstDisk(it PlannedItem, rec *ManifestItem, fact DiskFact, emit func(Action, PlannedItem, string), upstreamState string) { + switch { + case !fact.Exists: + emit(ActUpdate, it, "managed item missing on disk; restoring") + case fact.Hash == rec.Hash: + if upstreamState == "unchanged" { + emit(ActKeep, it, "") + } else { + emit(ActUpdate, it, "") + } + default: + emit(ActSkip, it, "locally modified; keeping the local version (upstream "+upstreamState+")") + } +} + +func typeOfKey(key string) string { + for i := len(key) - 1; i >= 0; i-- { + if key[i] == '/' { + return key[:i] + } + } + return key +} + +func nameOfKey(key string) string { + for i := len(key) - 1; i >= 0; i-- { + if key[i] == '/' { + return key[i+1:] + } + } + return key +} diff --git a/internal/client/planner_test.go b/internal/client/planner_test.go new file mode 100644 index 0000000..3e6335e --- /dev/null +++ b/internal/client/planner_test.go @@ -0,0 +1,211 @@ +package client + +import "testing" + +const ( + hashV1 = "sha256:aaa" + hashV2 = "sha256:bbb" + hashMod = "sha256:localmod" +) + +func testPathFor(typ, name string) string { return "/home/u/target/" + typ + "/" + name } + +func manifestWith(entries map[string]*ManifestItem) *Manifest { + m := NewManifest() + for k, v := range entries { + m.Items[k] = v + } + return m +} + +// TestPlanMatrix walks the complete three-state matrix (disk × upstream) +// plus the missing-disk restore cases, asserting the single planned action. +func TestPlanMatrix(t *testing.T) { + type disk string + const ( + diskNone disk = "missing" + diskClean disk = "unmodified" + diskMod disk = "user-modified" + diskForeign disk = "never-managed" + ) + type upstream string + const ( + upAdded upstream = "added" + upChanged upstream = "changed" + upSame upstream = "unchanged" + upGone upstream = "removed" + upAbsent upstream = "not-in-repo" + ) + + cases := []struct { + disk disk + up upstream + want Action + }{ + {diskNone, upAdded, ActDeploy}, + {diskClean, upAdded, ActSkip}, + {diskMod, upAdded, ActSkip}, + {diskForeign, upAdded, ActSkip}, + + {diskNone, upChanged, ActUpdate}, + {diskClean, upChanged, ActUpdate}, + {diskMod, upChanged, ActSkip}, + {diskForeign, upChanged, ActSkip}, + + {diskNone, upSame, ActUpdate}, + {diskClean, upSame, ActKeep}, + {diskMod, upSame, ActSkip}, + {diskForeign, upSame, ActSkip}, + + {diskNone, upGone, ActRemove}, + {diskClean, upGone, ActRemove}, + {diskMod, upGone, ActSkip}, + {diskForeign, upGone, ActSkip}, + + {diskClean, upAbsent, ActLeave}, + {diskNone, upAbsent, ActKeep}, // marker for "no plan entry at all" + } + + for _, tc := range cases { + name := string(tc.disk) + "/" + string(tc.up) + t.Run(name, func(t *testing.T) { + key := manifestKey("skills", "alpha") + var items []PlannedItem + manifest := NewManifest() + diskFacts := map[string]DiskFact{} + + switch tc.up { + case upAdded: + items = append(items, PlannedItem{Type: "skills", Name: "alpha", RelPath: "skills/alpha", Hash: hashV1}) + case upChanged, upSame: + hash := hashV2 + if tc.up == upSame { + hash = hashV1 + } + items = append(items, PlannedItem{Type: "skills", Name: "alpha", RelPath: "skills/alpha", Hash: hash}) + manifest.Items[key] = &ManifestItem{Name: "alpha", Type: "skills", Hash: hashV1} + case upGone: + manifest.Items[key] = &ManifestItem{Name: "alpha", Type: "skills", Hash: hashV1} + case upAbsent: + // neither inventory nor manifest + } + + switch tc.disk { + case diskNone: + case diskClean: + diskFacts[key] = DiskFact{Exists: true, Hash: hashV1} + case diskMod, diskForeign: + diskFacts[key] = DiskFact{Exists: true, Hash: hashMod} + } + + plan := Plan(items, manifest, diskFacts, testPathFor) + if tc.disk == diskNone && tc.up == upAbsent { + if len(plan) != 0 { + t.Fatalf("a key present nowhere must produce no entry, got %+v", plan) + } + return + } + if len(plan) != 1 { + t.Fatalf("want exactly 1 entry, got %d: %+v", len(plan), plan) + } + if plan[0].Action != tc.want { + t.Errorf("action = %s, want %s (reason: %s)", plan[0].Action, tc.want, plan[0].Reason) + } + if plan[0].DstPath != testPathFor("skills", "alpha") { + t.Errorf("DstPath not mapped: %q", plan[0].DstPath) + } + if tc.want == ActSkip || tc.want == ActLeave { + if plan[0].Reason == "" { + t.Error("skip/leave entries must carry a reason") + } + } + }) + } +} + +// TestPlanMixedFixture combines several states in one plan and asserts the +// full deterministic output. +func TestPlanMixedFixture(t *testing.T) { + items := []PlannedItem{ + {Type: "commands", Name: "deploy", RelPath: "commands/deploy.md", Hash: hashV1}, + {Type: "skills", Name: "fresh", RelPath: "skills/fresh", Hash: hashV1}, + {Type: "skills", Name: "tweaked", RelPath: "skills/tweaked", Hash: hashV2}, + {Type: "skills", Name: "keeper", RelPath: "skills/keeper", Hash: hashV1}, + {Type: "agents", Name: "helper", RelPath: "agents/helper.md", Hash: hashV1}, + } + manifest := manifestWith(map[string]*ManifestItem{ + "skills/tweaked": {Name: "tweaked", Type: "skills", Hash: hashV1}, + "skills/goner": {Name: "goner", Type: "skills", Hash: hashV1}, + "skills/keeper": {Name: "keeper", Type: "skills", Hash: hashV1}, + "skills/blocked": {Name: "blocked", Type: "skills", Hash: hashV1}, + "agents/helper": {Name: "helper", Type: "agents", Hash: hashV1}, + }) + disk := map[string]DiskFact{ + "skills/fresh": {Exists: true, Hash: hashMod}, // never-managed × added + "skills/tweaked": {Exists: true, Hash: hashMod}, // user-modified × changed + "skills/goner": {Exists: true, Hash: hashV1}, // unmodified × removed + "skills/keeper": {Exists: true, Hash: hashV1}, // unmodified × unchanged + "skills/blocked": {Exists: true, Hash: hashMod}, // user-modified × removed + "skills/mine": {Exists: true}, // never-managed × absent + "commands/deploy": {}, // missing × added + "agents/helper": {Exists: true, Hash: hashV1}, // unmodified × unchanged + } + + plan := Plan(items, manifest, disk, testPathFor) + got := map[string]Action{} + for _, e := range plan { + got[e.Type+"/"+e.Name] = e.Action + } + want := map[string]Action{ + "commands/deploy": ActDeploy, + "skills/fresh": ActSkip, + "skills/tweaked": ActSkip, + "agents/helper": ActKeep, + "skills/goner": ActRemove, + "skills/keeper": ActKeep, + "skills/blocked": ActSkip, + "skills/mine": ActLeave, + } + if len(got) != len(want) { + t.Fatalf("plan has %d entries, want %d: %+v", len(got), len(want), plan) + } + for key, action := range want { + if got[key] != action { + t.Errorf("%s: action = %s, want %s", key, got[key], action) + } + } + + sorted := true + for i := 1; i < len(plan); i++ { + prev := plan[i-1].Type + "/" + plan[i-1].Name + curr := plan[i].Type + "/" + plan[i].Name + if prev > curr { + sorted = false + } + } + if !sorted { + t.Error("plan must be deterministically sorted by type/name") + } +} + +// TestPlanPureGuarantee compiles the purity contract: planning identical +// inputs twice yields identical plans, and no input is mutated. +func TestPlanPureGuarantee(t *testing.T) { + items := []PlannedItem{{Type: "skills", Name: "a", RelPath: "skills/a", Hash: hashV1}} + manifest := manifestWith(map[string]*ManifestItem{"skills/b": {Name: "b", Type: "skills", Hash: hashV1}}) + disk := map[string]DiskFact{"skills/b": {Exists: true, Hash: hashV1}} + + p1 := Plan(items, manifest, disk, testPathFor) + p2 := Plan(items, manifest, disk, testPathFor) + if len(p1) != len(p2) { + t.Fatal("same inputs must yield same plan length") + } + for i := range p1 { + if p1[i] != p2[i] { + t.Fatalf("plan not deterministic at %d: %+v vs %+v", i, p1[i], p2[i]) + } + } + if manifest.Items["skills/b"] == nil || len(items) != 1 { + t.Error("planner must not mutate its inputs") + } +} diff --git a/internal/client/sync.go b/internal/client/sync.go index 9a00266..4898af5 100644 --- a/internal/client/sync.go +++ b/internal/client/sync.go @@ -1,15 +1,12 @@ package client import ( - "bytes" "fmt" "io" "os" "path/filepath" + "strings" - "github.com/m3tam3re/agent-lib/internal/deploy" - "github.com/m3tam3re/agent-lib/internal/gitsource" - "github.com/m3tam3re/agent-lib/internal/lockfile" "github.com/m3tam3re/agent-lib/internal/target" ) @@ -18,18 +15,10 @@ type SyncOptions struct { ConfigPath string } -// SyncReport summarizes one sync run. -type SyncReport struct { - Revision string - Deployed int - Updated int - Kept int - Manifest string -} - -// Sync pulls the work repository and deploys its content into the OpenCode -// target. Everything is validated and hashed before the first mutation; -// unreachable repositories fail cleanly with the previous state intact. +// Sync pulls the work repository, plans the sync as a pure function over +// (inventory, manifest, disk facts) and executes the plan. Validation and +// hashing happen before the first mutation; unreachable repositories fail +// cleanly with the previous state intact. func Sync(opts SyncOptions, stdout io.Writer) (*SyncReport, error) { cfg, err := LoadClientConfig(opts.ConfigPath) if err != nil { @@ -60,72 +49,30 @@ func Sync(opts SyncOptions, stdout io.Writer) (*SyncReport, error) { return nil, err } - type prepared struct { - item InventoryItem - hash string - dst string - } - var plan []prepared + items := make([]PlannedItem, 0, len(inv.Items)) for _, it := range inv.Deployable() { - dst, ok := oc.DeployPath(it.Type, it.Name) - if !ok { + if _, ok := oc.DeployPath(it.Type, it.Name); !ok { continue } hash, err := HashItem(pull.Tree, it) if err != nil { return nil, fmt.Errorf("hashing %s/%s: %w", it.Type, it.Name, err) } - if !safePath(dst) { - return nil, fmt.Errorf("refusing unsafe deploy path %q", dst) - } - plan = append(plan, prepared{item: it, hash: hash, dst: dst}) + items = append(items, PlannedItem{Type: it.Type, Name: it.Name, Origin: it.Origin, RelPath: it.RelPath, Hash: hash}) } - report := &SyncReport{Revision: pull.Rev, Manifest: paths.Manifest} - for _, p := range plan { - key := manifestKey(p.item.Type, p.item.Name) - record := manifest.Items[key] - if record != nil && record.Hash == p.hash { - if diskHash, err := HashDiskPath(p.item.Type, p.dst); err == nil && diskHash == p.hash { - report.Kept++ - continue - } - } - if err := deployItem(pull.Tree, p.item, p.dst); err != nil { - return nil, fmt.Errorf("deploying %s/%s: %w", p.item.Type, p.item.Name, err) - } - manifest.Items[key] = &ManifestItem{ - Name: p.item.Name, Type: p.item.Type, Origin: p.item.Origin, - Revision: pull.Rev, Hash: p.hash, - } - if record == nil { - report.Deployed++ - } else { - report.Updated++ + pathFor := func(typ, name string) string { + path, _ := oc.DeployPath(typ, name) + return path + } + plan := Plan(items, manifest, diskFacts(oc, manifest, items), pathFor) + for _, e := range plan { + if !safePath(e.DstPath) { + return nil, fmt.Errorf("refusing unsafe deploy path %q", e.DstPath) } } - manifest.Version = ManifestVersion - manifest.Revision = pull.Rev - unchanged, err := manifestUnchanged(manifest, paths.Manifest) - if err != nil { - return nil, err - } - if !unchanged { - if err := manifest.Save(paths.Manifest); err != nil { - return nil, err - } - } - - printSyncReport(stdout, report, len(inv.Items)) - return report, nil -} - -func deployItem(tree *gitsource.GitTree, it InventoryItem, dst string) error { - if it.Type == lockfile.TypeSkill { - return deploy.Dir(tree, it.RelPath, dst) - } - return deploy.File(tree, it.RelPath, dst) + return ExecutePlan(pull.Tree, plan, manifest, paths, pull.Rev, stdout) } // safePath rejects destinations that escape the user's home directory. @@ -134,39 +81,12 @@ func safePath(dst string) bool { if err != nil { return false } + if dst == home { + return false + } rel, err := filepath.Rel(home, dst) if err != nil { return false } - return rel != ".." && !filepath.IsAbs(rel) -} - -func manifestUnchanged(m *Manifest, path string) (bool, error) { - existing, err := os.ReadFile(path) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, err - } - fresh, err := marshalManifest(m) - if err != nil { - return false, err - } - return bytes.Equal(existing, fresh), nil -} - -func printSyncReport(w io.Writer, r *SyncReport, totalItems int) { - fmt.Fprintf(w, "synced to revision %s\n", shortRev(r.Revision)) - fmt.Fprintf(w, " deployed: %d\n", r.Deployed) - fmt.Fprintf(w, " updated: %d\n", r.Updated) - fmt.Fprintf(w, " kept: %d\n", r.Kept) - fmt.Fprintf(w, " manifest: %s\n", r.Manifest) -} - -func shortRev(rev string) string { - if len(rev) > 12 { - return rev[:12] - } - return rev + return rel != ".." && rel != "." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) } diff --git a/internal/e2e/warn_test.go b/internal/e2e/warn_test.go new file mode 100644 index 0000000..92f0d86 --- /dev/null +++ b/internal/e2e/warn_test.go @@ -0,0 +1,180 @@ +package e2e + +import ( + "os" + "path/filepath" + "testing" +) + +func syncOnce(t *testing.T, env *syncEnv) { + t.Helper() + if out, err := env.run(t); err != nil { + t.Fatalf("sync failed: %v\n%s", err, out) + } +} + +func commitWorkRepo(t *testing.T, h *harness, dir, msg string) { + t.Helper() + h.git(t, dir, "add", "-A") + h.git(t, dir, "commit", "-m", msg) +} + +func TestSyncSkipsLocallyModifiedItem(t *testing.T) { + h := newHarness(t) + env := newSyncEnv(t, h) + buildWorkRepo(t, h, env.workRepo) + env.writeConfig(t, env.workRepo, "") + syncOnce(t, env) + + skillPath := filepath.Join(env.home, ".agents/skills/own-skill/SKILL.md") + os.WriteFile(skillPath, []byte("---\nname: Own Skill\n---\n# MY local edits\n"), 0o644) + + p := filepath.Join(env.workRepo, "skills/own-skill/SKILL.md") + os.WriteFile(p, []byte("---\nname: Own Skill\ndescription: changed upstream\n---\n# Upstream v2\n"), 0o644) + commitWorkRepo(t, h, env.workRepo, "change own-skill upstream") + + out, err := env.run(t) + if err != nil { + t.Fatalf("sync with local mods must not fail: %v\n%s", err, out) + } + if !contains(out, "skipped (local modifications kept): 1") { + t.Errorf("modified item must be skipped with a warning:\n%s", out) + } + + kept := readFile(t, skillPath) + if !contains(kept, "MY local edits") { + t.Error("local version must be kept, not overwritten") + } + + log := readFile(t, filepath.Join(env.stateDir, "sync.log")) + if !contains(log, "SKIP skills/own-skill") || !contains(log, "locally modified") { + t.Errorf("warning log must record item and reason:\n%s", log) + } + + manifest := readFile(t, filepath.Join(env.stateDir, "manifest.json")) + if !contains(manifest, "own-skill") { + t.Error("skipped item must keep its manifest record for future syncs") + } +} + +func TestSyncLeavesSelfCreatedItemsUntouched(t *testing.T) { + h := newHarness(t) + env := newSyncEnv(t, h) + buildWorkRepo(t, h, env.workRepo) + env.writeConfig(t, env.workRepo, "") + syncOnce(t, env) + + mineDir := filepath.Join(env.home, ".agents/skills/my-secret-skill") + os.MkdirAll(mineDir, 0o755) + os.WriteFile(filepath.Join(mineDir, "SKILL.md"), []byte("---\nname: Mine\n---\n# private\n"), 0o644) + + out, err := env.run(t) + if err != nil { + t.Fatalf("sync must not fail on unmanaged items: %v\n%s", err, out) + } + if !contains(out, "unmanaged items left untouched: 1") { + t.Errorf("self-created item must be reported as left:\n%s", out) + } + if _, err := os.Stat(filepath.Join(mineDir, "SKILL.md")); err != nil { + t.Error("self-created item must survive untouched") + } + + log := readFile(t, filepath.Join(env.stateDir, "sync.log")) + if !contains(log, "LEAVE skills/my-secret-skill") || !contains(log, "unmanaged local item") { + t.Errorf("leave warning must be logged with reason:\n%s", log) + } + + manifest := readFile(t, filepath.Join(env.stateDir, "manifest.json")) + if contains(manifest, "my-secret-skill") { + t.Error("unmanaged items must never enter the manifest") + } +} + +func TestSyncRemovesUpstreamDeletedItems(t *testing.T) { + h := newHarness(t) + env := newSyncEnv(t, h) + buildWorkRepo(t, h, env.workRepo) + env.writeConfig(t, env.workRepo, "") + syncOnce(t, env) + + os.RemoveAll(filepath.Join(env.workRepo, "skills/own-skill")) + os.Remove(filepath.Join(env.workRepo, "agents/helper.md")) + os.Remove(filepath.Join(env.workRepo, "commands/deploy.md")) + commitWorkRepo(t, h, env.workRepo, "remove everything deployable") + + modifiedAgent := filepath.Join(env.home, ".config/opencode/agents/helper.md") + os.WriteFile(modifiedAgent, []byte("---\nname: Helper\n---\n# my tweaks\n"), 0o644) + + out, err := env.run(t) + if err != nil { + t.Fatalf("sync with removals failed: %v\n%s", err, out) + } + if !contains(out, "removed: 2") { + t.Errorf("unmodified deleted items must be removed:\n%s", out) + } + if _, err := os.Stat(filepath.Join(env.home, ".agents/skills/own-skill")); err == nil { + t.Error("unmodified skill must be removed after upstream deletion") + } + if _, err := os.Stat(modifiedAgent); err != nil { + t.Error("locally modified agent must survive the upstream deletion") + } + if !contains(out, "skipped (local modifications kept): 1") { + t.Errorf("modified+deleted item must be skipped with warning:\n%s", out) + } + + log := readFile(t, filepath.Join(env.stateDir, "sync.log")) + if !contains(log, "upstream deleted but the local copy was modified") { + t.Errorf("log must explain the kept deletion:\n%s", log) + } + + manifest := readFile(t, filepath.Join(env.stateDir, "manifest.json")) + if contains(manifest, "own-skill") || contains(manifest, "deploy") { + t.Errorf("removed items must leave the manifest:\n%s", manifest) + } + if !contains(manifest, "helper") { + t.Error("kept modified item must keep its manifest record") + } +} + +func TestSyncRestoresDeletedManagedItem(t *testing.T) { + h := newHarness(t) + env := newSyncEnv(t, h) + buildWorkRepo(t, h, env.workRepo) + env.writeConfig(t, env.workRepo, "") + syncOnce(t, env) + + os.RemoveAll(filepath.Join(env.home, ".agents/skills/own-skill")) + out, err := env.run(t) + if err != nil { + t.Fatalf("restore sync failed: %v\n%s", err, out) + } + if !contains(out, "updated: 1") { + t.Errorf("locally deleted managed item must be restored:\n%s", out) + } + if _, err := os.Stat(filepath.Join(env.home, ".agents/skills/own-skill/SKILL.md")); err != nil { + t.Error("item must be back on disk") + } +} + +func TestSyncNewUpstreamItemBlockedByUnmanagedOccupant(t *testing.T) { + h := newHarness(t) + env := newSyncEnv(t, h) + buildWorkRepo(t, h, env.workRepo) + + mineDir := filepath.Join(env.home, ".agents/skills/own-skill") + os.MkdirAll(mineDir, 0o755) + os.WriteFile(filepath.Join(mineDir, "SKILL.md"), []byte("---\nname: My Own\n---\n# private version\n"), 0o644) + + env.writeConfig(t, env.workRepo, "") + out, err := env.run(t) + if err != nil { + t.Fatalf("sync must not fail on occupant conflict: %v\n%s", err, out) + } + if !contains(out, "skipped (local modifications kept): 1") { + t.Errorf("new item blocked by unmanaged occupant must be skipped:\n%s", out) + } + kept := readFile(t, filepath.Join(mineDir, "SKILL.md")) + if !contains(kept, "private version") { + t.Error("occupant content must be preserved") + } +} diff --git a/internal/target/opencode.go b/internal/target/opencode.go index 8b7ad2b..d308e82 100644 --- a/internal/target/opencode.go +++ b/internal/target/opencode.go @@ -6,6 +6,7 @@ package target import ( "os" "path/filepath" + "strings" "github.com/m3tam3re/agent-lib/internal/lockfile" ) @@ -39,3 +40,32 @@ func (o *OpenCode) DeployPath(typ, name string) (path string, ok bool) { return "", false } } + +// ScanTargetDirs lists every item currently present in the target dirs, +// keyed "/" — the raw material for detecting unmanaged occupants. +func (o *OpenCode) ScanTargetDirs() map[string]bool { + out := map[string]bool{} + skillsDir := filepath.Join(o.Home, ".agents", "skills") + if entries, err := os.ReadDir(skillsDir); err == nil { + for _, e := range entries { + if e.IsDir() { + out[lockfile.TypeSkill+"/"+e.Name()] = true + } + } + } + for typ, dir := range map[string]string{ + lockfile.TypeCommand: filepath.Join(o.Home, ".config", "opencode", "commands"), + lockfile.TypeAgent: filepath.Join(o.Home, ".config", "opencode", "agents"), + } { + entries, err := os.ReadDir(dir) + if err != nil { + continue + } + for _, e := range entries { + if !e.IsDir() && strings.HasSuffix(e.Name(), ".md") { + out[typ+"/"+strings.TrimSuffix(e.Name(), ".md")] = true + } + } + } + return out +}