From 5fc755c0bf308db646d26b748a8f13dd66211749 Mon Sep 17 00:00:00 2001 From: bussyjd Date: Thu, 16 Jul 2026 15:26:09 +0400 Subject: [PATCH] fix(serviceoffer): scope ERC-8004 agentId to the offer's own chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every site that fed ServiceOfferStatus.AgentID trusted RegistrationRequest.Status.AgentID (or the equally chain-blind status.AgentID/offer.Status.AgentID fallbacks) even though that subresource is never chain-tagged and is never reset when spec.payment.network changes. Switching an offer from Base Sepolia to Base retained the Sepolia numeric id, skipped the on-chain ownership check (erc8004 FindRegistrationByOwnerAndURI/TxHash), and durably wrote it into the shared AgentIdentity CR under "base" — even though that Base token may belong to a different wallet. Disabling registration left the stale id in status/CLI output. Fixes all four sites to derive AgentID solely via AgentIdentityAgentIDForChain(identity.Status, ): reconcileRegistrationStatus, reconcileRegistrationActive (now correctly forces on-chain verification after a chain switch), reconcileRegistrationTombstone, and applySharedRegistrationStatus (also drops the stale bare-Phase Registered=True path for the same reason). Disable now clears status.AgentID/RegistrationTxHash. Adds RemoveAgentIdentityRegistration plus `obol sell identity forget ` as the operator remediation path for already-poisoned records. Addresses a Canary402 field report. Claude-Session: https://claude.ai/code/session_014YjPMViNrZ7zBVgUQzwEKk --- cmd/obol/sell_identity.go | 64 ++++++++++ internal/monetizeapi/agentidentity_test.go | 33 +++++ internal/monetizeapi/types.go | 22 ++++ internal/serviceoffercontroller/controller.go | 93 ++++++++------ .../serviceoffercontroller/controller_test.go | 47 +++++++- .../identity_controller.go | 8 -- .../identity_controller_test.go | 114 ++++++++++++++++-- 7 files changed, 322 insertions(+), 59 deletions(-) diff --git a/cmd/obol/sell_identity.go b/cmd/obol/sell_identity.go index 81812c92..c8f4b9e1 100644 --- a/cmd/obol/sell_identity.go +++ b/cmd/obol/sell_identity.go @@ -291,6 +291,70 @@ func sellIdentityCommand(cfg *config.Config) *cli.Command { Usage: "Manage the durable ERC-8004 AgentIdentity record", Commands: []*cli.Command{ sellIdentityImportCommand(cfg), + sellIdentityForgetCommand(cfg), + }, + } +} + +func sellIdentityForgetCommand(cfg *config.Config) *cli.Command { + return &cli.Command{ + Name: "forget", + Usage: "Remove a chain's registration from the AgentIdentity record", + ArgsUsage: "", + Description: `Removes the recorded agentId for from the AgentIdentity CR. + +Use this to correct a registration that was recorded under the wrong +chain (e.g. a wrong-chain agentId written by a bug, or an offer that +switched networks before an on-chain id was verified). This does not +touch the on-chain NFT itself; it only clears the local record so the +controller re-derives the id from scratch on the next reconcile.`, + Action: func(ctx context.Context, cmd *cli.Command) error { + if cmd.NArg() != 1 { + return fmt.Errorf("chain required: obol sell identity forget ") + } + chain := strings.TrimSpace(cmd.Args().First()) + + ns := monetizeapi.AgentIdentityDefaultNamespace + name := monetizeapi.AgentIdentityDefaultName + rec, err := loadAgentIdentity(cfg, ns, name) + if err != nil { + return err + } + if rec == nil { + return fmt.Errorf("AgentIdentity %s/%s not found", ns, name) + } + existing := monetizeapi.AgentIdentityAgentIDForChain(rec.Status, chain) + if existing == "" { + getUI(cmd).Printf("AgentIdentity %s/%s has no registration on %s; nothing to do.", ns, name, chain) + return nil + } + rec.Status = monetizeapi.RemoveAgentIdentityRegistration(rec.Status, chain) + // Patch registrations explicitly rather than via + // patchAgentIdentityStatus: that helper marshals with + // `omitempty`, so dropping the last entry (the common case — + // the poisoned record is usually the only one) would omit + // the field from the JSON merge patch and leave the stale + // array untouched server-side instead of clearing it. + registrations := rec.Status.Registrations + if registrations == nil { + registrations = []monetizeapi.AgentIdentityRegistration{} + } + patch, err := json.Marshal(map[string]any{"status": map[string]any{"registrations": registrations}}) + if err != nil { + return err + } + if err := kubectlRun( + cfg, + "patch", "agentidentities.obol.org", name, + "-n", ns, + "--subresource=status", + "--type=merge", + "-p", string(patch), + ); err != nil { + return err + } + getUI(cmd).Successf("Removed agent %s on %s from AgentIdentity %s/%s.", existing, chain, ns, name) + return nil }, } } diff --git a/internal/monetizeapi/agentidentity_test.go b/internal/monetizeapi/agentidentity_test.go index a619d9cf..cfeba962 100644 --- a/internal/monetizeapi/agentidentity_test.go +++ b/internal/monetizeapi/agentidentity_test.go @@ -33,3 +33,36 @@ func TestUpsertAgentIdentityRegistration_DedupesChain(t *testing.T) { t.Fatalf("registrations = %+v, want deduped base + base-sepolia", status.Registrations) } } + +func TestRemoveAgentIdentityRegistration(t *testing.T) { + status := AgentIdentityStatus{} + status = UpsertAgentIdentityRegistration(status, "base-sepolia", "99") + status = UpsertAgentIdentityRegistration(status, "base", "42") + + status = RemoveAgentIdentityRegistration(status, "base") + + if got := AgentIdentityAgentIDForChain(status, "base"); got != "" { + t.Errorf("base agentId = %q, want empty after remove", got) + } + if got := AgentIdentityAgentIDForChain(status, "base-sepolia"); got != "99" { + t.Errorf("base-sepolia agentId = %q, want 99 (untouched)", got) + } + if len(status.Registrations) != 1 { + t.Fatalf("registrations = %+v, want only base-sepolia left", status.Registrations) + } +} + +func TestRemoveAgentIdentityRegistration_UnknownChainAndEmptyChainAreNoops(t *testing.T) { + status := AgentIdentityStatus{} + status = UpsertAgentIdentityRegistration(status, "base", "42") + + status = RemoveAgentIdentityRegistration(status, "polygon") + if got := AgentIdentityAgentIDForChain(status, "base"); got != "42" { + t.Errorf("base agentId = %q, want 42 unchanged by removing an unrelated chain", got) + } + + status = RemoveAgentIdentityRegistration(status, "") + if got := AgentIdentityAgentIDForChain(status, "base"); got != "42" { + t.Errorf("base agentId = %q, want 42 unchanged by an empty-chain no-op", got) + } +} diff --git a/internal/monetizeapi/types.go b/internal/monetizeapi/types.go index d9bdd874..90ced6a5 100644 --- a/internal/monetizeapi/types.go +++ b/internal/monetizeapi/types.go @@ -1151,6 +1151,28 @@ func UpsertAgentIdentityRegistration(status AgentIdentityStatus, chain, agentID return status } +// RemoveAgentIdentityRegistration drops chain's registration, if any. It is +// the counterpart to UpsertAgentIdentityRegistration and exists as the +// operator remediation path for a registration that was written under the +// wrong chain (e.g. `obol sell identity forget `) — there is no +// automatic removal anywhere else, since the durable-identity/tombstone +// design otherwise treats a recorded registration as permanent. +func RemoveAgentIdentityRegistration(status AgentIdentityStatus, chain string) AgentIdentityStatus { + chain = strings.TrimSpace(chain) + if chain == "" { + return status + } + out := status.Registrations[:0] + for _, registration := range status.Registrations { + if strings.EqualFold(strings.TrimSpace(registration.Chain), chain) { + continue + } + out = append(out, registration) + } + status.Registrations = out + return status +} + func HasAgentIdentityRegistrations(status AgentIdentityStatus) bool { for _, registration := range status.Registrations { if strings.TrimSpace(registration.Chain) != "" && strings.TrimSpace(registration.AgentID) != "" { diff --git a/internal/serviceoffercontroller/controller.go b/internal/serviceoffercontroller/controller.go index d7ffd571..f7c06c93 100644 --- a/internal/serviceoffercontroller/controller.go +++ b/internal/serviceoffercontroller/controller.go @@ -864,6 +864,11 @@ func (c *Controller) reconcileRegistrationStatus(ctx context.Context, status *mo if err := c.deleteRegistrationRequest(ctx, offer.Namespace, offer.Name); err != nil { return err } + // Disabling registration must stop reporting whatever agentId was + // last recorded — otherwise a disabled offer keeps showing a + // (possibly stale, wrong-chain) id in status/CLI output. + status.AgentID = "" + status.RegistrationTxHash = "" setCondition(status, "Registered", "True", "Disabled", "Registration disabled") return nil } @@ -905,8 +910,7 @@ func (c *Controller) reconcileRegistrationStatus(ctx context.Context, status *mo if err != nil { return err } - request.Status = registrationRequestStatusWithIdentity(request, identity) - applySharedRegistrationStatus(status, offer, owner, request) + applySharedRegistrationStatus(status, offer, owner, identity, request) return nil } @@ -926,14 +930,7 @@ func (c *Controller) reconcileRegistrationStatus(ctx context.Context, status *mo return err } - status.AgentID = request.Status.AgentID - status.RegistrationTxHash = request.Status.RegistrationTxHash - if agentID := monetizeapi.AgentIdentityAgentIDForChain(identity.Status, offer.Spec.Payment.Network); agentID != "" { - status.AgentID = agentID - request.Status = registrationRequestStatusWithIdentity(request, identity) - } - - applySharedRegistrationStatus(status, offer, owner, request) + applySharedRegistrationStatus(status, offer, owner, identity, request) return nil } @@ -1072,7 +1069,12 @@ func (c *Controller) reconcileRegistrationActive(ctx context.Context, raw *unstr return c.updateRegistrationStatus(ctx, raw, status) } registrationChain := firstNonEmpty(request.Spec.Chain, offer.Spec.Payment.Network) - agentID := firstNonEmpty(monetizeapi.AgentIdentityAgentIDForChain(identity.Status, registrationChain), status.AgentID, offer.Status.AgentID) + // Chain-scoped only: status.AgentID/offer.Status.AgentID are not + // chain-tagged, so falling back to them here would reuse a + // wrong-chain id after a network switch and skip the on-chain + // ownership verification below. AgentIdentityAgentIDForChain + // correctly returns "" until this chain has a verified registration. + agentID := monetizeapi.AgentIdentityAgentIDForChain(identity.Status, registrationChain) txHash := firstNonEmpty(status.RegistrationTxHash, offer.Status.RegistrationTxHash) offers, err := c.registrationOffersForIdentity(defaultAgentIdentityKey(), "", "") @@ -1215,21 +1217,18 @@ func (c *Controller) recoverRegistration(ctx context.Context, client *erc8004.Cl func (c *Controller) reconcileRegistrationTombstone(ctx context.Context, raw *unstructured.Unstructured, request *monetizeapi.RegistrationRequest, offer *monetizeapi.ServiceOffer, baseURL string) error { status := request.Status - identityRaw, identity, err := c.ensureAgentIdentityForOffer(ctx, offer) + _, identity, err := c.ensureAgentIdentityForOffer(ctx, offer) if err != nil { status.Phase = registrationPhaseAwaitingExternal status.Message = truncateMessage(fmt.Sprintf("Waiting for AgentIdentity tombstone: %v", err)) return c.updateRegistrationStatus(ctx, raw, status) } + // Chain-scoped only — see reconcileRegistrationActive for why the + // status.AgentID/offer.Status.AgentID fallbacks are dropped. Since + // agentID is now always exactly what AgentIdentityAgentIDForChain + // already returns, there is nothing left to backfill into identity. registrationChain := firstNonEmpty(request.Spec.Chain, offer.Spec.Payment.Network) - agentID := firstNonEmpty(monetizeapi.AgentIdentityAgentIDForChain(identity.Status, registrationChain), status.AgentID, offer.Status.AgentID) - - if agentID != "" && monetizeapi.AgentIdentityAgentIDForChain(identity.Status, registrationChain) == "" { - identity.Status = agentIdentityStatusFromRegistration(identity, registrationChain, agentID) - if err := c.updateAgentIdentityStatus(ctx, identityRaw, identity.Status); err != nil { - return err - } - } + agentID := monetizeapi.AgentIdentityAgentIDForChain(identity.Status, registrationChain) document := BuildIdentityRegistrationDocument(IdentityRegistrationView{ Identity: identity, @@ -1434,34 +1433,50 @@ func selectRegistrationOwner(offers []*monetizeapi.ServiceOffer) *monetizeapi.Se return offers[0] } -func applySharedRegistrationStatus(status *monetizeapi.ServiceOfferStatus, offer, owner *monetizeapi.ServiceOffer, request *monetizeapi.RegistrationRequest) { - status.AgentID = request.Status.AgentID - status.RegistrationTxHash = request.Status.RegistrationTxHash +// applySharedRegistrationStatus copies a shared RegistrationRequest's phase +// onto offer's status, but the agentId itself is never trusted straight off +// request.Status: that subresource belongs to the registration owner and is +// never chain-tagged, so it can't tell whether it was recorded under +// offer's own Spec.Payment.Network or a different chain the owner (or offer +// itself, before a network switch) used previously. The agentId is instead +// always re-derived from the chain-scoped AgentIdentity record — if the +// identity has no registration for offer's chain, agentId stays empty and +// Registered does not flip True on the strength of a foreign chain's id. +func applySharedRegistrationStatus(status *monetizeapi.ServiceOfferStatus, offer, owner *monetizeapi.ServiceOffer, identity *monetizeapi.AgentIdentity, request *monetizeapi.RegistrationRequest) { + agentID := monetizeapi.AgentIdentityAgentIDForChain(identity.Status, offer.Spec.Payment.Network) + status.AgentID = agentID + // The tx hash lives on the same chain-blind request.Status subresource + // the agentId was retired from, so it is only mirrored once the offer's + // own chain has a recorded registration for it to describe. + status.RegistrationTxHash = "" + if agentID != "" { + status.RegistrationTxHash = request.Status.RegistrationTxHash + } if !isConditionTrue(*status, "RoutePublished") { setCondition(status, "Registered", "False", "WaitingForRoute", "Waiting for route publication before shared registration") return } - // Prefer a completed RegistrationRequest phase. Also treat a non-empty - // agentId (including one filled from AgentIdentity after CLI register) - // as Registered=True so non-owner offers and post-hoc enables leave - // Pending once the on-chain id is known — even when RegistrationTxHash - // is still empty (CLI path does not always populate the request tx). - if requestPhaseReady(request.Status.Phase) || strings.TrimSpace(request.Status.AgentID) != "" { - message := defaultString(request.Status.Message, "Registration reconciled") - if request.Status.AgentID != "" { - message = defaultString(request.Status.Message, fmt.Sprintf("Recorded agent %s", request.Status.AgentID)) - } + // Registered only ever flips True on the strength of a chain-scoped + // agentId. requestPhaseReady(request.Status.Phase) alone is NOT a + // sufficient condition: Phase is set by whoever last reconciled this + // RegistrationRequest for whatever chain it targeted at that time, and + // (like the AgentID field) is never chain-tagged — it can read stale + // "Registered" from a chain the offer (or, when shared, the owner) has + // since moved off. Every legitimate Phase=Registered transition sets + // agentID for that same chain in the same reconcile pass, so requiring + // agentID != "" here costs no real case while closing the stale-phase + // gap (this is also what tempers the request.Status.AgentID-based + // widening 84dcbf83 added — that source is gone, but a bare Phase + // check has the identical staleness problem). + if agentID != "" { + message := defaultString(request.Status.Message, fmt.Sprintf("Recorded agent %s", agentID)) if owner != nil && (owner.Namespace != offer.Namespace || owner.Name != offer.Name) { - if request.Status.AgentID != "" { - message = fmt.Sprintf("Shared registration via %s/%s recorded agent %s", owner.Namespace, owner.Name, request.Status.AgentID) - } else { - message = fmt.Sprintf("Shared registration via %s/%s is active", owner.Namespace, owner.Name) - } + message = fmt.Sprintf("Shared registration via %s/%s recorded agent %s", owner.Namespace, owner.Name, agentID) } reason := defaultString(request.Status.Phase, "Active") - if !requestPhaseReady(request.Status.Phase) && request.Status.AgentID != "" { + if !requestPhaseReady(request.Status.Phase) { reason = "Active" } setCondition(status, "Registered", "True", reason, message) diff --git a/internal/serviceoffercontroller/controller_test.go b/internal/serviceoffercontroller/controller_test.go index a5ee6558..2f59be64 100644 --- a/internal/serviceoffercontroller/controller_test.go +++ b/internal/serviceoffercontroller/controller_test.go @@ -130,6 +130,9 @@ func TestApplySharedRegistrationStatus_NonOwnerUsesSharedAgent(t *testing.T) { } owner := &monetizeapi.ServiceOffer{ObjectMeta: metav1.ObjectMeta{Name: "alpha", Namespace: "demo"}} offer := &monetizeapi.ServiceOffer{ObjectMeta: metav1.ObjectMeta{Name: "beta", Namespace: "demo"}} + offer.Spec.Payment.Network = "base" + identity := &monetizeapi.AgentIdentity{} + identity.Status = monetizeapi.UpsertAgentIdentityRegistration(identity.Status, "base", "42") request := &monetizeapi.RegistrationRequest{ Status: monetizeapi.RegistrationRequestStatus{ Phase: registrationPhaseRegistered, @@ -138,7 +141,7 @@ func TestApplySharedRegistrationStatus_NonOwnerUsesSharedAgent(t *testing.T) { }, } - applySharedRegistrationStatus(status, offer, owner, request) + applySharedRegistrationStatus(status, offer, owner, identity, request) if status.AgentID != "42" || status.RegistrationTxHash != "0xtx" { t.Fatalf("shared registration identifiers not copied: %+v", status) @@ -151,6 +154,9 @@ func TestApplySharedRegistrationStatus_NonOwnerUsesSharedAgent(t *testing.T) { func TestApplySharedRegistrationStatus_WaitsForRoute(t *testing.T) { status := &monetizeapi.ServiceOfferStatus{} owner := &monetizeapi.ServiceOffer{ObjectMeta: metav1.ObjectMeta{Name: "alpha", Namespace: "demo"}} + owner.Spec.Payment.Network = "base" + identity := &monetizeapi.AgentIdentity{} + identity.Status = monetizeapi.UpsertAgentIdentityRegistration(identity.Status, "base", "7") request := &monetizeapi.RegistrationRequest{ Status: monetizeapi.RegistrationRequestStatus{ Phase: registrationPhaseRegistered, @@ -158,7 +164,7 @@ func TestApplySharedRegistrationStatus_WaitsForRoute(t *testing.T) { }, } - applySharedRegistrationStatus(status, owner, owner, request) + applySharedRegistrationStatus(status, owner, owner, identity, request) if isConditionTrue(*status, "Registered") { t.Fatalf("registered should remain false until route is published: %+v", status.Conditions) @@ -174,6 +180,9 @@ func TestApplySharedRegistrationStatus_AgentIDWithoutPhase(t *testing.T) { } owner := &monetizeapi.ServiceOffer{ObjectMeta: metav1.ObjectMeta{Name: "alpha", Namespace: "demo"}} offer := &monetizeapi.ServiceOffer{ObjectMeta: metav1.ObjectMeta{Name: "beta", Namespace: "other"}} + offer.Spec.Payment.Network = "base" + identity := &monetizeapi.AgentIdentity{} + identity.Status = monetizeapi.UpsertAgentIdentityRegistration(identity.Status, "base", "8104") request := &monetizeapi.RegistrationRequest{ Status: monetizeapi.RegistrationRequestStatus{ AgentID: "8104", @@ -181,7 +190,7 @@ func TestApplySharedRegistrationStatus_AgentIDWithoutPhase(t *testing.T) { }, } - applySharedRegistrationStatus(status, offer, owner, request) + applySharedRegistrationStatus(status, offer, owner, identity, request) if status.AgentID != "8104" { t.Fatalf("AgentID = %q, want 8104", status.AgentID) @@ -190,3 +199,35 @@ func TestApplySharedRegistrationStatus_AgentIDWithoutPhase(t *testing.T) { t.Fatalf("Registered should be True when agentId is known: %+v", status.Conditions) } } + +// Chain-switch regression: offer B borrows a shared registration from owner +// A, but B's own Spec.Payment.Network has no verified registration in the +// AgentIdentity (e.g. B just switched networks, or was never registered on +// this chain). Even though the owner's RegistrationRequest is Phase=Registered +// with a non-empty (different-chain) AgentID, B's status must not adopt it +// and Registered must not flip True on the strength of a foreign chain's id. +func TestApplySharedRegistrationStatus_ChainMismatchDoesNotAdoptForeignAgentID(t *testing.T) { + status := &monetizeapi.ServiceOfferStatus{ + Conditions: []monetizeapi.Condition{{Type: "RoutePublished", Status: "True"}}, + } + owner := &monetizeapi.ServiceOffer{ObjectMeta: metav1.ObjectMeta{Name: "alpha", Namespace: "demo"}} + offer := &monetizeapi.ServiceOffer{ObjectMeta: metav1.ObjectMeta{Name: "beta", Namespace: "demo"}} + offer.Spec.Payment.Network = "base" // switched away from base-sepolia + identity := &monetizeapi.AgentIdentity{} + identity.Status = monetizeapi.UpsertAgentIdentityRegistration(identity.Status, "base-sepolia", "42") + request := &monetizeapi.RegistrationRequest{ + Status: monetizeapi.RegistrationRequestStatus{ + Phase: registrationPhaseRegistered, + AgentID: "42", // owner's base-sepolia id — must not leak onto offer's base status + }, + } + + applySharedRegistrationStatus(status, offer, owner, identity, request) + + if status.AgentID != "" { + t.Fatalf("AgentID = %q, want empty (no verified registration on offer's own chain)", status.AgentID) + } + if isConditionTrue(*status, "Registered") { + t.Fatalf("Registered should not flip True from a different chain's agentId: %+v", status.Conditions) + } +} diff --git a/internal/serviceoffercontroller/identity_controller.go b/internal/serviceoffercontroller/identity_controller.go index 92fcd03e..fefbffb6 100644 --- a/internal/serviceoffercontroller/identity_controller.go +++ b/internal/serviceoffercontroller/identity_controller.go @@ -436,11 +436,3 @@ func agentIdentityStatusFromRegistration(identity *monetizeapi.AgentIdentity, ch return status } -func registrationRequestStatusWithIdentity(request *monetizeapi.RegistrationRequest, identity *monetizeapi.AgentIdentity) monetizeapi.RegistrationRequestStatus { - status := request.Status - if identity == nil { - return status - } - status.AgentID = firstNonEmpty(monetizeapi.AgentIdentityAgentIDForChain(identity.Status, request.Spec.Chain), status.AgentID) - return status -} diff --git a/internal/serviceoffercontroller/identity_controller_test.go b/internal/serviceoffercontroller/identity_controller_test.go index cf9bf43e..0b9d0f0c 100644 --- a/internal/serviceoffercontroller/identity_controller_test.go +++ b/internal/serviceoffercontroller/identity_controller_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "testing" + "github.com/ObolNetwork/obol-stack/internal/erc8004" "github.com/ObolNetwork/obol-stack/internal/monetizeapi" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -71,17 +72,112 @@ func TestReconcileRegistrationTombstone_PublishesIdentityDocument(t *testing.T) } } -func TestRecreatedServiceOffer_MirrorsAgentIdentityAgentID(t *testing.T) { - identity := defaultIdentity("777") +// Chain-switch regression (Canary402 field report): identity already has a +// verified base-sepolia registration, but the offer (and its +// RegistrationRequest, re-applied with the new spec.chain on every apply) +// has switched to base. request.Status.AgentID / offer.Status.AgentID still +// carry the old base-sepolia numeric id — that subresource is never reset +// by a chain switch. reconcileRegistrationActive must not adopt the stale +// id for base, must not upsert it into the AgentIdentity CR, and must not +// publish it in the registration document either. +func TestReconcileRegistrationActive_ChainSwitchDoesNotLeakWrongChainAgentID(t *testing.T) { + identity := defaultIdentity("777") // base-sepolia -> 777 + identity.APIVersion = monetizeapi.Group + "/" + monetizeapi.Version + identity.Kind = monetizeapi.AgentIdentityKind + identity.UID = types.UID("identity-uid") + request := &monetizeapi.RegistrationRequest{} - request.Spec.Chain = "base-sepolia" - request.Status.RegistrationTxHash = "0xabc" - status := registrationRequestStatusWithIdentity(request, identity) - if status.AgentID != "777" { - t.Fatalf("AgentID = %q, want 777", status.AgentID) + request.Namespace = "demo" + request.Name = registrationRequestName("svc") + request.Spec.ServiceOfferName = "svc" + request.Spec.ServiceOfferNamespace = "demo" + request.Spec.Chain = "base" // switched + request.Status.AgentID = "777" + request.Status.RegistrationTxHash = "0xsepolia" + + offer := readyOffer("svc") + offer.Namespace = "demo" + offer.Spec.Payment.Network = "base" // switched + offer.Spec.Payment.PayTo = "0xNewOwner" + offer.Status.AgentID = "777" // stale, mirrors pre-fix ServiceOffer.status + + rawRequest := registrationRequestToUnstructured(t, request) + dynClient := fake.NewSimpleDynamicClientWithCustomListKinds( + runtime.NewScheme(), + identityListKinds(), + agentIdentityToUnstructured(identity), + rawRequest, + ) + c := controllerForIdentityTest(dynClient) + + if err := c.reconcileRegistrationActive(context.Background(), rawRequest, request, offer, "https://seller.test"); err != nil { + t.Fatalf("reconcileRegistrationActive: %v", err) + } + + identityRaw, err := c.agentIdentities.Namespace(identity.Namespace).Get(context.Background(), identity.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("get AgentIdentity: %v", err) + } + persisted, err := decodeAgentIdentity(identityRaw) + if err != nil { + t.Fatalf("decode AgentIdentity: %v", err) + } + if got := monetizeapi.AgentIdentityAgentIDForChain(persisted.Status, "base"); got != "" { + t.Fatalf("AgentIdentity gained a base registration %q sourced from the stale base-sepolia id", got) + } + if got := monetizeapi.AgentIdentityAgentIDForChain(persisted.Status, "base-sepolia"); got != "777" { + t.Fatalf("base-sepolia registration = %q, want untouched 777", got) + } + + cm, err := c.configMaps.Namespace(identity.Namespace).Get(context.Background(), agentIdentityRegistrationName(identity), metav1.GetOptions{}) + if err != nil { + t.Fatalf("get identity ConfigMap: %v", err) + } + data, _, _ := unstructured.NestedStringMap(cm.Object, "data") + var doc map[string]any + if err := json.Unmarshal([]byte(data["agent-registration.json"]), &doc); err != nil { + t.Fatalf("unmarshal document: %v\n%s", err, data["agent-registration.json"]) + } + regs, _ := doc["registrations"].([]any) + if len(regs) != 1 { + t.Fatalf("registrations = %#v, want only the base-sepolia entry", doc["registrations"]) + } + reg0, _ := regs[0].(map[string]any) + if reg0["agentId"] != float64(777) { + t.Fatalf("registrations[0].agentId = %#v, want 777", reg0["agentId"]) + } + if reg0["agentRegistry"] != erc8004.BaseSepolia.CAIP10Registry() { + t.Fatalf("registrations[0].agentRegistry = %#v, want base-sepolia's registry, not base's", reg0["agentRegistry"]) + } +} + +// Disable must stop reporting whatever agentId was last recorded, including +// a stale/wrong-chain one — otherwise a disabled offer keeps showing it in +// ServiceOffer.status (and `obol sell status` output). +func TestReconcileRegistrationStatus_DisableClearsStaleAgentID(t *testing.T) { + status := &monetizeapi.ServiceOfferStatus{ + AgentID: "stale-sepolia-id", + RegistrationTxHash: "0xstale", + } + offer := readyOffer("svc") + offer.Namespace = "demo" + offer.Spec.Registration.Enabled = false + + dynClient := fake.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), identityListKinds()) + c := controllerForIdentityTest(dynClient) + + if err := c.reconcileRegistrationStatus(context.Background(), status, offer); err != nil { + t.Fatalf("reconcileRegistrationStatus: %v", err) + } + + if status.AgentID != "" { + t.Fatalf("AgentID = %q, want cleared on disable", status.AgentID) + } + if status.RegistrationTxHash != "" { + t.Fatalf("RegistrationTxHash = %q, want cleared on disable", status.RegistrationTxHash) } - if status.RegistrationTxHash != "0xabc" { - t.Fatalf("RegistrationTxHash = %q, want 0xabc", status.RegistrationTxHash) + if !isConditionTrue(*status, "Registered") { + t.Fatalf("Registered condition not set: %+v", status.Conditions) } }