diff --git a/core/app/harness_sync.go b/core/app/harness_sync.go index 8f395e1..494bcbc 100644 --- a/core/app/harness_sync.go +++ b/core/app/harness_sync.go @@ -70,7 +70,7 @@ func (services *Services) ReconcileDeviceHarnesses( // What each selected harness *wants* to own, independent of what the target // currently holds. Two selected harnesses that share a skill root collide on // an empty root too, where no on-disk evidence of the conflict exists yet. - claims := map[string][]string{} + claims := map[string][]harness.AdapterFile{} for _, id := range harness.CanonicalIDs { // A selected harness that cannot be rendered is a hard error: the device // cannot converge on it. An unselected one is skipped instead, so an @@ -129,21 +129,30 @@ func (services *Services) ReconcileDeviceHarnesses( // target root. Say so here, while the command is still read-only, rather // than let the owner discover it half-way through a mutating run. if wanted[id] { - owned := make([]string, 0, len(inspection.Files)) - for _, file := range inspection.Files { + owned := make([]harness.AdapterFile, 0, len(inspection.Files)) + candidate, candidateFindings := adapter.Render(request) + if len(candidateFindings) != 0 { + return domain.NewEnvelope(command, classifyFindings(candidateFindings), nil, candidateFindings...) + } + for _, file := range candidate.Files { if file.Path != lockPath { - owned = append(owned, file.Path) + owned = append(owned, file) } } claims[id] = owned - } - if wanted[id] && !present { - for _, file := range inspection.Files { - if file.Path != lockPath && file.State != "missing" { - collisions = append(collisions, map[string]any{ - "harness": id, "path": file.Path, - }) - break + if !present { + desiredFiles := map[string]string{} + for _, file := range candidate.Files { + desiredFiles[file.Path] = file.Digest + } + for _, file := range inspection.Files { + if file.Path != lockPath && file.State != "missing" && + (file.State != "regular" || file.Digest != desiredFiles[file.Path]) { + collisions = append(collisions, map[string]any{ + "harness": id, "path": file.Path, + }) + break + } } } } @@ -160,7 +169,7 @@ func (services *Services) ReconcileDeviceHarnesses( // a path something else already owns; `shared` is two selected harnesses // wanting the same path, which is true before either is installed and is the // only one an empty target root can show. - shared := harness.DetectTargetCollisions(claims) + shared := harness.DetectTargetContentCollisions(claims) if len(collisions) != 0 || len(shared) != 0 { planFindings = append(planFindings, domain.Finding{ Code: "GDS_HARNESS_TARGET_COLLISION", Severity: domain.SeverityHigh, diff --git a/core/harness/adapter.go b/core/harness/adapter.go index f8b565c..73fc01f 100644 --- a/core/harness/adapter.go +++ b/core/harness/adapter.go @@ -320,7 +320,8 @@ func (adapter *profileAdapter) PlanInstall( return AdapterPlan{Harness: adapter.ID()}, findings } for _, file := range inspection.Files { - if file.State != "missing" { + expected := adapterFileMap(candidate.Files)[file.Path] + if file.State != "missing" && (file.State != "regular" || file.Digest != expected.Digest || !ownedByOtherAdapter(targetRoot, adapter.ID(), file.Path, file.Digest)) { return AdapterPlan{Harness: adapter.ID()}, []domain.Finding{harnessFinding( "GDS_HARNESS_INSTALL_COLLISION", "Install refuses to replace an existing managed-path candidate; use update or resolve the collision.", @@ -386,6 +387,17 @@ func (adapter *profileAdapter) PlanRemove( if len(findings) != 0 { return AdapterPlan{Harness: adapter.ID()}, findings } + keptFiles := make([]AdapterFile, 0, len(candidate.Files)) + keptContents := map[string][]byte{} + for _, file := range candidate.Files { + if ownedByOtherAdapter(targetRoot, adapter.ID(), file.Path, file.Digest) { + continue + } + keptFiles = append(keptFiles, file) + keptContents[file.Path] = candidate.contents[file.Path] + } + candidate.Files = keptFiles + candidate.contents = keptContents return adapter.buildPlan("remove", targetRoot, "", candidate, AdapterCandidate{}, inspection.Fingerprint) } @@ -409,6 +421,15 @@ func (adapter *profileAdapter) planTransition( } previousPaths := adapterFileMap(previous.Files) desiredPaths := adapterFileMap(desired.Files) + for _, file := range desired.Files { + if otherDigest, owned := otherAdapterDigest(targetRoot, adapter.ID(), file.Path); owned && otherDigest != file.Digest { + return AdapterPlan{Harness: adapter.ID()}, []domain.Finding{harnessFinding( + "GDS_HARNESS_UPDATE_COLLISION", + "Transition would change a path still owned by another installed adapter.", + map[string]any{"harness": adapter.ID(), "path": file.Path}, + )} + } + } for _, file := range inspection.Files { if _, wasManaged := previousPaths[file.Path]; wasManaged { continue @@ -497,6 +518,37 @@ func adapterFileMap(files []AdapterFile) map[string]AdapterFile { return result } +func ownedByOtherAdapter(targetRoot, currentHarness, targetPath, digest string) bool { + otherDigest, found := otherAdapterDigest(targetRoot, currentHarness, targetPath) + return found && otherDigest == digest +} + +func otherAdapterDigest(targetRoot, currentHarness, targetPath string) (string, bool) { + entries, err := os.ReadDir(filepath.Join(targetRoot, ".gds", "harness")) + if err != nil { + return "", false + } + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".lock.json") { + continue + } + raw, err := os.ReadFile(filepath.Join(targetRoot, ".gds", "harness", entry.Name())) + if err != nil || len(raw) > maxAdapterSourceBytes { + continue + } + var lock adapterLock + if json.Unmarshal(raw, &lock) != nil || lock.Harness == currentHarness { + continue + } + for _, file := range lock.Files { + if file.Path == targetPath { + return file.Digest, true + } + } + } + return "", false +} + func (adapter *profileAdapter) inspectCandidate( targetRoot string, candidate AdapterCandidate, diff --git a/core/harness/adapter_test.go b/core/harness/adapter_test.go index 7942eca..24747ae 100644 --- a/core/harness/adapter_test.go +++ b/core/harness/adapter_test.go @@ -186,6 +186,43 @@ func TestAdapterMaterializeVerifyAndRemoveLifecycle(t *testing.T) { } } +func TestAdapterInstallAndRemovePreserveIdenticalSharedSkills(t *testing.T) { + root, _ := filepath.Abs(filepath.Join("..", "..")) + schemas, err := validation.NewSchemaSet() + if err != nil { + t.Fatal(err) + } + request := RenderRequest{SkillProfile: "core", Scope: "project"} + target := t.TempDir() + first, findings := NewAdapter(root, "antigravity", schemas) + if len(findings) != 0 { + t.Fatalf("first adapter: %+v", findings) + } + firstCandidate, findings := first.Render(request) + if len(findings) != 0 { + t.Fatalf("first render: %+v", findings) + } + installAdapterTestCandidate(t, target, firstCandidate) + second, findings := NewAdapter(root, "codex", schemas) + if len(findings) != 0 { + t.Fatalf("second adapter: %+v", findings) + } + secondPlan, findings := second.PlanInstall(target, request) + if len(findings) != 0 { + t.Fatalf("shared install refused: %+v", findings) + } + installAdapterTestCandidate(t, target, secondPlan.candidate) + removePlan, findings := second.PlanRemove(target, request) + if len(findings) != 0 { + t.Fatalf("shared remove plan: %+v", findings) + } + for _, file := range removePlan.Files { + if strings.HasPrefix(file.Path, ".agents/skills/") && ownedByOtherAdapter(target, "codex", file.Path, file.Digest) { + t.Fatalf("remove still owns shared file %s", file.Path) + } + } +} + func TestAdapterRemoveBlocksManualDrift(t *testing.T) { root, _ := filepath.Abs(filepath.Join("..", "..")) schemas, err := validation.NewSchemaSet() diff --git a/core/harness/sync.go b/core/harness/sync.go index f4905b6..74f4740 100644 --- a/core/harness/sync.go +++ b/core/harness/sync.go @@ -68,6 +68,39 @@ type TargetCollision struct { Harnesses []string `json:"harnesses"` } +// DetectTargetContentCollisions permits two adapters to co-own one canonical +// path only when they render exactly the same bytes. Each adapter still keeps +// its own lock, so presence and lifecycle remain independently observable. +func DetectTargetContentCollisions(claims map[string][]AdapterFile) []TargetCollision { + type claim struct{ harness, digest string } + owners := map[string][]claim{} + for id, files := range claims { + for _, file := range files { + owners[file.Path] = append(owners[file.Path], claim{id, file.Digest}) + } + } + collisions := []TargetCollision{} + for target, values := range owners { + if len(values) < 2 { + continue + } + digest := values[0].digest + harnesses := []string{} + for _, value := range values { + harnesses = append(harnesses, value.harness) + if value.digest != digest { + digest = "" + } + } + if digest == "" { + sort.Strings(harnesses) + collisions = append(collisions, TargetCollision{Path: target, Harnesses: harnesses}) + } + } + sort.Slice(collisions, func(left, right int) bool { return collisions[left].Path < collisions[right].Path }) + return collisions +} + // DetectTargetCollisions reports the target paths that more than one selected // harness claims. // diff --git a/core/harness/sync_test.go b/core/harness/sync_test.go index 47423fb..41fd1bc 100644 --- a/core/harness/sync_test.go +++ b/core/harness/sync_test.go @@ -213,6 +213,22 @@ func TestDetectTargetCollisionsFindsTwoSelectedClaimingOnePath(t *testing.T) { } } +func TestDetectTargetContentCollisionsPermitsIdenticalCanonicalBytes(t *testing.T) { + shared := AdapterFile{Path: ".agents/skills/review/SKILL.md", Digest: "sha256:same"} + if got := DetectTargetContentCollisions(map[string][]AdapterFile{ + "antigravity": {shared}, "codex": {shared}, + }); len(got) != 0 { + t.Fatalf("identical canonical bytes must be shareable: %+v", got) + } + changed := shared + changed.Digest = "sha256:different" + if got := DetectTargetContentCollisions(map[string][]AdapterFile{ + "antigravity": {shared}, "codex": {changed}, + }); len(got) != 1 || got[0].Path != shared.Path { + t.Fatalf("different bytes at one path must collide: %+v", got) + } +} + // Nothing is installed on an empty target root, so a check that only asks // "is this path already taken" reports no conflict. Desired-state comparison is // what makes the empty-root case detectable at all.