fix(serviceoffer): scope ERC-8004 agentId to the offer's own chain

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: "<chain>",
+		Description: `Removes the recorded agentId for <chain> 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>")
+			}
+			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
 		},
 	}
 }

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)
+	}
+}

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 <chain>`) — 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) != "" {

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)

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,14 +154,17 @@ 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,
 			AgentID: "7",
 		},
 	}
 
-	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,14 +180,17 @@ 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",
 			// Phase empty / Pending — common after CLI-only registration path
 		},
 	}
 
-	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)
+	}
+}

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
-}