From 4438045b8c41f352b13901bb4da26b4250fb6e76 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 16:33:14 +0100 Subject: [PATCH] A release of nothing held answers released false; the sweep names retention from the seat distribution answers released false for what it does not hold, as the registry seats now define, and takes the removal reason from the generated constant; the sweep reads the answer instead of matching the error's text. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- .../cmd/artifact-store-tools/holder_test.go | 6 ++---- .../cmd/artifact-store-tools/sweepseats.go | 15 ++++++--------- .../cmd/distribution-tools/main.go | 11 ++++++++--- .../internal/holder/holder_test.go | 14 ++++++++++---- modules/distribution/internal/holder/verbs.go | 19 +++++++++++-------- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/modules/artifact-store-tools/cmd/artifact-store-tools/holder_test.go b/modules/artifact-store-tools/cmd/artifact-store-tools/holder_test.go index a0effbb3..37b7d553 100644 --- a/modules/artifact-store-tools/cmd/artifact-store-tools/holder_test.go +++ b/modules/artifact-store-tools/cmd/artifact-store-tools/holder_test.go @@ -124,11 +124,9 @@ func (d *memDoor) seat(key string, body any) (json.RawMessage, error) { d.held[ref] = true return json.Marshal(map[string]any{"held": true}) case "release": - if !d.held[ref] { - return nil, fmt.Errorf("release: %s@%s is not held on %s", in.Name, in.Digest, seat) - } + released := d.held[ref] // what is not held answers released false, as the seat's definition says delete(d.held, ref) - return json.Marshal(map[string]any{"released": true}) + return json.Marshal(map[string]any{"released": released}) case "remove": if d.held[ref] { return nil, fmt.Errorf("remove: refused: %s@%s is held; release it first", in.Name, in.Digest) diff --git a/modules/artifact-store-tools/cmd/artifact-store-tools/sweepseats.go b/modules/artifact-store-tools/cmd/artifact-store-tools/sweepseats.go index 20519cc7..ecb4e373 100644 --- a/modules/artifact-store-tools/cmd/artifact-store-tools/sweepseats.go +++ b/modules/artifact-store-tools/cmd/artifact-store-tools/sweepseats.go @@ -93,19 +93,16 @@ func (h *Holder) holdOnSeat(reference, why string) (bool, error) { } // Retention is the reason of a removal by digest, the sweep's alone (design 51 *Who may call which verb*): the holder -// removes an untagged thing only for it. The seat's definition names no such value yet (a request to mesh-contracts). -const Retention = "retention" - -// notHeld is the holder's refusal of a release of what it does not hold: nothing to let go. -const notHeld = "is not held" +// removes an untagged thing only for it. The seat's definition names it. +const Retention = reg.RemoveInputReasonRetention // removeOnSeat lets reference go: a hold an earlier sweep asked while it was kept is released first (the holder -// refuses to remove a held thing), then it is removed for retention. why goes with the release; the hand act records it. +// refuses to remove a held thing; one not held answers released false), then it is removed for retention. why goes with +// the release; the hand act records it. func (h *Holder) removeOnSeat(reference, why string) error { - err := h.askSeat(reference, reg.VerbRelease, func(name, digest string) any { + if err := h.askSeat(reference, reg.VerbRelease, func(name, digest string) any { return reg.ReleaseInput{Name: name, Digest: digest, Reason: why} - }, ®.ReleaseAnswer{}) - if err != nil && !strings.Contains(err.Error(), notHeld) { + }, ®.ReleaseAnswer{}); err != nil { return err } return h.askSeat(reference, reg.VerbRemove, func(name, digest string) any { diff --git a/modules/distribution/cmd/distribution-tools/main.go b/modules/distribution/cmd/distribution-tools/main.go index b16a18d0..2385b3e9 100644 --- a/modules/distribution/cmd/distribution-tools/main.go +++ b/modules/distribution/cmd/distribution-tools/main.go @@ -350,10 +350,11 @@ func tools(v *holder.Verbs, windows kept[holder.Window], callers func() (holder. if err != nil { return nil, err } - if err := v.Release(seat, stdio.CallerOf(args), holder.HoldRequest{Name: in.Name, Digest: in.Digest, Why: in.Reason}); err != nil { + released, err := v.Release(seat, stdio.CallerOf(args), holder.HoldRequest{Name: in.Name, Digest: in.Digest, Why: in.Reason}) + if err != nil { return nil, err } - return reg.ReleaseAnswer{Released: true}, nil + return reg.ReleaseAnswer{Released: released}, nil }}, ) } @@ -434,7 +435,11 @@ func tools(v *holder.Verbs, windows kept[holder.Window], callers func() (holder. return nil, errors.New("a release by hand is a hand act: say why") } // The caller proved on this call (the operator's session) is carried on; distribution's own never. - return map[string]any{"released": name}, v.Release(str(args, "seat"), stdio.CallerOf(args), holder.HoldRequest{Name: name, Digest: str(args, "digest"), Why: str(args, "why")}) + released, err := v.Release(str(args, "seat"), stdio.CallerOf(args), holder.HoldRequest{Name: name, Digest: str(args, "digest"), Why: str(args, "why")}) + if err == nil && !released { + return map[string]any{"released": false, "note": name + " is not held: nothing was let go"}, nil + } + return map[string]any{"released": name}, err } return v.HeldNow() }}, diff --git a/modules/distribution/internal/holder/holder_test.go b/modules/distribution/internal/holder/holder_test.go index eb589a9c..5e15abfa 100644 --- a/modules/distribution/internal/holder/holder_test.go +++ b/modules/distribution/internal/holder/holder_test.go @@ -281,10 +281,12 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { return b.v.HoldOne(ArtifactSeat, "novox.mesh-build-go", HoldRequest{Name: "m/a", Kind: KindArchive, Digest: part.Digest}) }, "not on the holder's caller list"}, {"a release from a caller not on the list", func(b *bed) error { - return b.v.Release(ArtifactSeat, "novox.mesh-build-go", HoldRequest{Name: "m/a", Digest: part.Digest}) + _, err := b.v.Release(ArtifactSeat, "novox.mesh-build-go", HoldRequest{Name: "m/a", Digest: part.Digest}) + return err }, "not on the holder's caller list"}, {"a release in distribution's own name", func(b *bed) error { - return b.v.Release(ArtifactSeat, "novox.distribution", HoldRequest{Name: "m/a", Digest: part.Digest}) + _, err := b.v.Release(ArtifactSeat, "novox.distribution", HoldRequest{Name: "m/a", Digest: part.Digest}) + return err }, "in its own name"}, {"a mirror by no digest", func(b *bed) error { _, err := b.v.Mirror(ImageSeat, "novox.mesh-build-image", MirrorRequest{Source: "docker.io/library/nats:2"}) @@ -348,8 +350,12 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { if err := b.v.HoldOne(ArtifactSeat, sweep, HoldRequest{Name: "m/b", Kind: KindArchive, Digest: part.Digest}); err != nil { t.Fatal(err) } - if err := b.v.Release(ArtifactSeat, "person.jochen", HoldRequest{Name: "m/b", Digest: part.Digest, Why: "by hand"}); err != nil { - t.Errorf("the operator's session releasing through registry_holds: %v", err) + if released, err := b.v.Release(ArtifactSeat, "person.jochen", HoldRequest{Name: "m/b", Digest: part.Digest, Why: "by hand"}); err != nil || !released { + t.Errorf("the operator's session releasing through registry_holds: %v, %v", released, err) + } + // A release of what is not held answers released false, never an error, and says nothing (the seat's definition). + if released, err := b.v.Release(ArtifactSeat, sweep, HoldRequest{Name: "m/b", Digest: part.Digest, Why: "eligible"}); err != nil || released { + t.Errorf("a release of what is not held: %v, %v; want false and no error", released, err) } var events []string for _, s := range b.said { diff --git a/modules/distribution/internal/holder/verbs.go b/modules/distribution/internal/holder/verbs.go index 18d02d1c..cc90a063 100644 --- a/modules/distribution/internal/holder/verbs.go +++ b/modules/distribution/internal/holder/verbs.go @@ -8,6 +8,8 @@ import ( "slices" "strings" "time" + + reg "git.novox.be/novox/mesh-contracts/go/seats/containerimageregistry" ) // Catalog is what the store answers of what it holds. @@ -109,7 +111,8 @@ type RemoveRequest struct { } // ReasonRetention is the reason of the sweep's removal of an untagged version (ADR 0610 ยง3). -const ReasonRetention = "retention" +// The seat definitions name it (the removal reasons' enum), so the sweep and the holder share one value. +const ReasonRetention = reg.RemoveInputReasonRetention // Remove is `remove`: the builder removes a tagged version; the artifact sweep's holder an untagged bundle, archive or // copied base with the reason retention, which a hold keeps. Every removal says `removed` with its reason. @@ -191,25 +194,25 @@ func (v *Verbs) HoldOne(seat, caller string, r HoldRequest) error { // Release is `release`: the artifact sweep's holder, or the operator's session through registry_holds (a hand act, // recorded by the tool); distribution's own account is never on its list. -func (v *Verbs) Release(seat, caller string, r HoldRequest) error { +func (v *Verbs) Release(seat, caller string, r HoldRequest) (bool, error) { if CallerModule(caller) == Self && !strings.HasPrefix(caller, "person.") { - return fmt.Errorf("release: refused for %s: distribution cannot release in its own name", caller) + return false, fmt.Errorf("release: refused for %s: distribution cannot release in its own name", caller) } if err := v.Callers.Allowed(VerbRelease, caller); err != nil { - return err + return false, err } key := Hold{Seat: seat, Name: r.Name, Digest: r.Digest}.Key() h, held, err := v.Holds.Get(key) if err != nil { - return err + return false, err } if !held { - return fmt.Errorf("release: %s@%s is not held on %s", r.Name, r.Digest, seat) + return false, nil // nothing held: the outcome asked holds already, and nothing is said (the seat's definition) } if err := v.Holds.Delete(key); err != nil { - return err + return false, err } - return v.Say(seat, EventReleased, Event{Name: h.Name, Kind: h.Kind, Digest: h.Digest, Holder: Self, By: caller, Reason: r.Why}) + return true, v.Say(seat, EventReleased, Event{Name: h.Name, Kind: h.Kind, Digest: h.Digest, Holder: Self, By: caller, Reason: r.Why}) } // Tags is `tags` on the image seat: the tags of an image.