Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 22 additions & 13 deletions core/app/harness_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
}
}
Expand All @@ -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,
Expand Down
54 changes: 53 additions & 1 deletion core/harness/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down Expand Up @@ -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)
}

Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
37 changes: 37 additions & 0 deletions core/harness/adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
33 changes: 33 additions & 0 deletions core/harness/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down
16 changes: 16 additions & 0 deletions core/harness/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading