Skip to content

Commit f4fed64

Browse files
authored
Merge pull request #154 from NDDev-OpenNetwork/codex/shared-harness-projections
fix(harness): support identical shared projections
2 parents 4099536 + a4bfb9b commit f4fed64

5 files changed

Lines changed: 161 additions & 14 deletions

File tree

core/app/harness_sync.go

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (services *Services) ReconcileDeviceHarnesses(
7070
// What each selected harness *wants* to own, independent of what the target
7171
// currently holds. Two selected harnesses that share a skill root collide on
7272
// an empty root too, where no on-disk evidence of the conflict exists yet.
73-
claims := map[string][]string{}
73+
claims := map[string][]harness.AdapterFile{}
7474
for _, id := range harness.CanonicalIDs {
7575
// A selected harness that cannot be rendered is a hard error: the device
7676
// cannot converge on it. An unselected one is skipped instead, so an
@@ -129,21 +129,30 @@ func (services *Services) ReconcileDeviceHarnesses(
129129
// target root. Say so here, while the command is still read-only, rather
130130
// than let the owner discover it half-way through a mutating run.
131131
if wanted[id] {
132-
owned := make([]string, 0, len(inspection.Files))
133-
for _, file := range inspection.Files {
132+
owned := make([]harness.AdapterFile, 0, len(inspection.Files))
133+
candidate, candidateFindings := adapter.Render(request)
134+
if len(candidateFindings) != 0 {
135+
return domain.NewEnvelope(command, classifyFindings(candidateFindings), nil, candidateFindings...)
136+
}
137+
for _, file := range candidate.Files {
134138
if file.Path != lockPath {
135-
owned = append(owned, file.Path)
139+
owned = append(owned, file)
136140
}
137141
}
138142
claims[id] = owned
139-
}
140-
if wanted[id] && !present {
141-
for _, file := range inspection.Files {
142-
if file.Path != lockPath && file.State != "missing" {
143-
collisions = append(collisions, map[string]any{
144-
"harness": id, "path": file.Path,
145-
})
146-
break
143+
if !present {
144+
desiredFiles := map[string]string{}
145+
for _, file := range candidate.Files {
146+
desiredFiles[file.Path] = file.Digest
147+
}
148+
for _, file := range inspection.Files {
149+
if file.Path != lockPath && file.State != "missing" &&
150+
(file.State != "regular" || file.Digest != desiredFiles[file.Path]) {
151+
collisions = append(collisions, map[string]any{
152+
"harness": id, "path": file.Path,
153+
})
154+
break
155+
}
147156
}
148157
}
149158
}
@@ -160,7 +169,7 @@ func (services *Services) ReconcileDeviceHarnesses(
160169
// a path something else already owns; `shared` is two selected harnesses
161170
// wanting the same path, which is true before either is installed and is the
162171
// only one an empty target root can show.
163-
shared := harness.DetectTargetCollisions(claims)
172+
shared := harness.DetectTargetContentCollisions(claims)
164173
if len(collisions) != 0 || len(shared) != 0 {
165174
planFindings = append(planFindings, domain.Finding{
166175
Code: "GDS_HARNESS_TARGET_COLLISION", Severity: domain.SeverityHigh,

core/harness/adapter.go

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,8 @@ func (adapter *profileAdapter) PlanInstall(
320320
return AdapterPlan{Harness: adapter.ID()}, findings
321321
}
322322
for _, file := range inspection.Files {
323-
if file.State != "missing" {
323+
expected := adapterFileMap(candidate.Files)[file.Path]
324+
if file.State != "missing" && (file.State != "regular" || file.Digest != expected.Digest || !ownedByOtherAdapter(targetRoot, adapter.ID(), file.Path, file.Digest)) {
324325
return AdapterPlan{Harness: adapter.ID()}, []domain.Finding{harnessFinding(
325326
"GDS_HARNESS_INSTALL_COLLISION",
326327
"Install refuses to replace an existing managed-path candidate; use update or resolve the collision.",
@@ -386,6 +387,17 @@ func (adapter *profileAdapter) PlanRemove(
386387
if len(findings) != 0 {
387388
return AdapterPlan{Harness: adapter.ID()}, findings
388389
}
390+
keptFiles := make([]AdapterFile, 0, len(candidate.Files))
391+
keptContents := map[string][]byte{}
392+
for _, file := range candidate.Files {
393+
if ownedByOtherAdapter(targetRoot, adapter.ID(), file.Path, file.Digest) {
394+
continue
395+
}
396+
keptFiles = append(keptFiles, file)
397+
keptContents[file.Path] = candidate.contents[file.Path]
398+
}
399+
candidate.Files = keptFiles
400+
candidate.contents = keptContents
389401
return adapter.buildPlan("remove", targetRoot, "", candidate, AdapterCandidate{}, inspection.Fingerprint)
390402
}
391403

@@ -409,6 +421,15 @@ func (adapter *profileAdapter) planTransition(
409421
}
410422
previousPaths := adapterFileMap(previous.Files)
411423
desiredPaths := adapterFileMap(desired.Files)
424+
for _, file := range desired.Files {
425+
if otherDigest, owned := otherAdapterDigest(targetRoot, adapter.ID(), file.Path); owned && otherDigest != file.Digest {
426+
return AdapterPlan{Harness: adapter.ID()}, []domain.Finding{harnessFinding(
427+
"GDS_HARNESS_UPDATE_COLLISION",
428+
"Transition would change a path still owned by another installed adapter.",
429+
map[string]any{"harness": adapter.ID(), "path": file.Path},
430+
)}
431+
}
432+
}
412433
for _, file := range inspection.Files {
413434
if _, wasManaged := previousPaths[file.Path]; wasManaged {
414435
continue
@@ -497,6 +518,37 @@ func adapterFileMap(files []AdapterFile) map[string]AdapterFile {
497518
return result
498519
}
499520

521+
func ownedByOtherAdapter(targetRoot, currentHarness, targetPath, digest string) bool {
522+
otherDigest, found := otherAdapterDigest(targetRoot, currentHarness, targetPath)
523+
return found && otherDigest == digest
524+
}
525+
526+
func otherAdapterDigest(targetRoot, currentHarness, targetPath string) (string, bool) {
527+
entries, err := os.ReadDir(filepath.Join(targetRoot, ".gds", "harness"))
528+
if err != nil {
529+
return "", false
530+
}
531+
for _, entry := range entries {
532+
if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".lock.json") {
533+
continue
534+
}
535+
raw, err := os.ReadFile(filepath.Join(targetRoot, ".gds", "harness", entry.Name()))
536+
if err != nil || len(raw) > maxAdapterSourceBytes {
537+
continue
538+
}
539+
var lock adapterLock
540+
if json.Unmarshal(raw, &lock) != nil || lock.Harness == currentHarness {
541+
continue
542+
}
543+
for _, file := range lock.Files {
544+
if file.Path == targetPath {
545+
return file.Digest, true
546+
}
547+
}
548+
}
549+
return "", false
550+
}
551+
500552
func (adapter *profileAdapter) inspectCandidate(
501553
targetRoot string,
502554
candidate AdapterCandidate,

core/harness/adapter_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,43 @@ func TestAdapterMaterializeVerifyAndRemoveLifecycle(t *testing.T) {
186186
}
187187
}
188188

189+
func TestAdapterInstallAndRemovePreserveIdenticalSharedSkills(t *testing.T) {
190+
root, _ := filepath.Abs(filepath.Join("..", ".."))
191+
schemas, err := validation.NewSchemaSet()
192+
if err != nil {
193+
t.Fatal(err)
194+
}
195+
request := RenderRequest{SkillProfile: "core", Scope: "project"}
196+
target := t.TempDir()
197+
first, findings := NewAdapter(root, "antigravity", schemas)
198+
if len(findings) != 0 {
199+
t.Fatalf("first adapter: %+v", findings)
200+
}
201+
firstCandidate, findings := first.Render(request)
202+
if len(findings) != 0 {
203+
t.Fatalf("first render: %+v", findings)
204+
}
205+
installAdapterTestCandidate(t, target, firstCandidate)
206+
second, findings := NewAdapter(root, "codex", schemas)
207+
if len(findings) != 0 {
208+
t.Fatalf("second adapter: %+v", findings)
209+
}
210+
secondPlan, findings := second.PlanInstall(target, request)
211+
if len(findings) != 0 {
212+
t.Fatalf("shared install refused: %+v", findings)
213+
}
214+
installAdapterTestCandidate(t, target, secondPlan.candidate)
215+
removePlan, findings := second.PlanRemove(target, request)
216+
if len(findings) != 0 {
217+
t.Fatalf("shared remove plan: %+v", findings)
218+
}
219+
for _, file := range removePlan.Files {
220+
if strings.HasPrefix(file.Path, ".agents/skills/") && ownedByOtherAdapter(target, "codex", file.Path, file.Digest) {
221+
t.Fatalf("remove still owns shared file %s", file.Path)
222+
}
223+
}
224+
}
225+
189226
func TestAdapterRemoveBlocksManualDrift(t *testing.T) {
190227
root, _ := filepath.Abs(filepath.Join("..", ".."))
191228
schemas, err := validation.NewSchemaSet()

core/harness/sync.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,39 @@ type TargetCollision struct {
6868
Harnesses []string `json:"harnesses"`
6969
}
7070

71+
// DetectTargetContentCollisions permits two adapters to co-own one canonical
72+
// path only when they render exactly the same bytes. Each adapter still keeps
73+
// its own lock, so presence and lifecycle remain independently observable.
74+
func DetectTargetContentCollisions(claims map[string][]AdapterFile) []TargetCollision {
75+
type claim struct{ harness, digest string }
76+
owners := map[string][]claim{}
77+
for id, files := range claims {
78+
for _, file := range files {
79+
owners[file.Path] = append(owners[file.Path], claim{id, file.Digest})
80+
}
81+
}
82+
collisions := []TargetCollision{}
83+
for target, values := range owners {
84+
if len(values) < 2 {
85+
continue
86+
}
87+
digest := values[0].digest
88+
harnesses := []string{}
89+
for _, value := range values {
90+
harnesses = append(harnesses, value.harness)
91+
if value.digest != digest {
92+
digest = ""
93+
}
94+
}
95+
if digest == "" {
96+
sort.Strings(harnesses)
97+
collisions = append(collisions, TargetCollision{Path: target, Harnesses: harnesses})
98+
}
99+
}
100+
sort.Slice(collisions, func(left, right int) bool { return collisions[left].Path < collisions[right].Path })
101+
return collisions
102+
}
103+
71104
// DetectTargetCollisions reports the target paths that more than one selected
72105
// harness claims.
73106
//

core/harness/sync_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,22 @@ func TestDetectTargetCollisionsFindsTwoSelectedClaimingOnePath(t *testing.T) {
213213
}
214214
}
215215

216+
func TestDetectTargetContentCollisionsPermitsIdenticalCanonicalBytes(t *testing.T) {
217+
shared := AdapterFile{Path: ".agents/skills/review/SKILL.md", Digest: "sha256:same"}
218+
if got := DetectTargetContentCollisions(map[string][]AdapterFile{
219+
"antigravity": {shared}, "codex": {shared},
220+
}); len(got) != 0 {
221+
t.Fatalf("identical canonical bytes must be shareable: %+v", got)
222+
}
223+
changed := shared
224+
changed.Digest = "sha256:different"
225+
if got := DetectTargetContentCollisions(map[string][]AdapterFile{
226+
"antigravity": {shared}, "codex": {changed},
227+
}); len(got) != 1 || got[0].Path != shared.Path {
228+
t.Fatalf("different bytes at one path must collide: %+v", got)
229+
}
230+
}
231+
216232
// Nothing is installed on an empty target root, so a check that only asks
217233
// "is this path already taken" reports no conflict. Desired-state comparison is
218234
// what makes the empty-root case detectable at all.

0 commit comments

Comments
 (0)