feat(addons): make an operation added by an upgrade pending on its own

An upgrade adds operations to an entry that already holds others. With one
operation ID per entry, the whole entry went pending until the upgrade
completed, so the running old version lost its grants for that time, and a
crash removed all of them.

Give an operation its own optional operation ID. Only that operation waits
for its lifecycle operation, and reconciliation removes only what an
unfinished upgrade added.

Assisted-by: Claude:claude-sonnet-5-5 go

core/services/addons/hostgrants/document.go

@@ -89,6 +89,11 @@ type OpGrant struct {
 	// BlanketRevision names the blanket switch revision that produced a
 	// grant with Source blanket, so the group can be told apart and revoked.
 	BlanketRevision uint64 `json:"blanketRevision,omitempty"`
+	// OperationID is set when the operation was added to an entry that already
+	// existed, by the lifecycle operation of an upgrade. It takes effect only
+	// once that operation completed, while the other operations of the entry
+	// keep working. Empty means the operation of the entry counts.
+	OperationID string `json:"operationId,omitempty"`
 }
 
 // AddonGrants is the UI-made grant entry of one add-on.
@@ -343,6 +348,9 @@ func validate(s Snapshot) error {
 				return invalid("operation %q of %q", op.Name, id)
 			}
 			seen[op.Name] = true
+			if op.OperationID != "" && !validOperationID(op.OperationID) {
+				return invalid("operation ID of %q for %q", op.Name, id)
+			}
 			if !validText(op.By) || !validText(op.Version) || !validTime(op.At) {
 				return invalid("metadata of %q for %q", op.Name, id)
 			}

core/services/addons/hostgrants/policy.go

@@ -119,12 +119,22 @@ func (f Floor) Decide(doc Snapshot, c Context, addonID, op string) Decision {
 	if c.Identity == "" || entry.Identity != c.Identity {
 		return Decision{Reason: ReasonIdentityMismatch}
 	}
-	if c.Completed != nil && !c.Completed(entry.OperationID) {
+	if c.Completed != nil && !c.Completed(EffectiveOperationID(entry, entry.Ops[i])) {
 		return Decision{Reason: ReasonNotGranted}
 	}
 	return Decision{Allowed: true, Source: entry.Ops[i].Source}
 }
 
+// EffectiveOperationID is the lifecycle operation whose completion makes one
+// operation of an entry effective: its own, when an upgrade added it to an
+// existing entry, else the one that created the entry.
+func EffectiveOperationID(entry AddonGrants, op OpGrant) string {
+	if op.OperationID != "" {
+		return op.OperationID
+	}
+	return entry.OperationID
+}
+
 // OpStatus is a Decision for one named operation.
 type OpStatus struct {
 	Op string

core/services/addons/hostgrants/reconcile.go

@@ -51,8 +51,30 @@ func PlanReconcile(snap Snapshot, in ReconcileInput) (Patch, []Removal) {
 			reason = RemovedNotInstalled
 		case identity != entry.Identity:
 			reason = RemovedIdentityMismatch
-		case in.OperationCompleted != nil && !in.OperationCompleted(entry.OperationID):
-			reason = RemovedOperationIncomplete
+		}
+		if reason == "" && in.OperationCompleted != nil {
+			// An operation added by an upgrade is pending on its own; the
+			// operations the entry held before keep working.
+			var kept []OpGrant
+			var dropped []string
+			for _, op := range entry.Ops {
+				if in.OperationCompleted(EffectiveOperationID(entry, op)) {
+					kept = append(kept, op)
+				} else {
+					dropped = append(dropped, op.Name)
+				}
+			}
+			switch {
+			case len(dropped) == 0:
+			case len(kept) == 0:
+				reason = RemovedOperationIncomplete
+			default:
+				trimmed := entry
+				trimmed.Ops = kept
+				patch.Addons[id] = &trimmed
+				removals = append(removals, Removal{AddonID: id, Reason: RemovedOperationIncomplete, Ops: dropped})
+				continue
+			}
 		}
 		if reason == "" {
 			continue

core/services/addons/hostgrants/reconcile_test.go

@@ -161,3 +161,40 @@ func TestAuditAppendsFsyncedLines(t *testing.T) {
 		t.Error("a symlink must be refused")
 	}
 }
+
+func TestUpgradeAddedOperationIsPendingOnItsOwn(t *testing.T) {
+	entry := *grant("install-op", "workspace.materialize")
+	entry.Ops = append(entry.Ops, OpGrant{Name: "egress.check", Source: SourceUpgrade, OperationID: "upgrade-op"})
+	snap := emptySnapshot()
+	snap.Grants["a"] = entry
+	completed := map[string]bool{"install-op": true}
+	ctx := Context{Identity: "id-1", Completed: func(id string) bool { return completed[id] }}
+
+	// Before the upgrade completes, the old operation works and the new one does not.
+	var f Floor
+	if !f.Decide(snap, ctx, "a", "workspace.materialize").Allowed {
+		t.Fatal("the operation held before the upgrade stopped working")
+	}
+	if d := f.Decide(snap, ctx, "a", "egress.check"); d.Allowed || d.Reason != ReasonNotGranted {
+		t.Fatalf("%+v", d)
+	}
+	completed["upgrade-op"] = true
+	if !f.Decide(snap, ctx, "a", "egress.check").Allowed {
+		t.Fatal("the operation of a completed upgrade is not effective")
+	}
+
+	// A crash before the upgrade completed removes only what it added.
+	delete(completed, "upgrade-op")
+	in := ReconcileInput{
+		Installed:          func(string) (string, bool) { return "id-1", true },
+		OperationCompleted: func(id string) bool { return completed[id] },
+	}
+	patch, removals := PlanReconcile(snap, in)
+	if len(removals) != 1 || !reflect.DeepEqual(removals[0].Ops, []string{"egress.check"}) || removals[0].Reason != RemovedOperationIncomplete {
+		t.Fatalf("%+v", removals)
+	}
+	kept := patch.Addons["a"]
+	if kept == nil || len(kept.Ops) != 1 || kept.Ops[0].Name != "workspace.materialize" {
+		t.Fatalf("%+v", kept)
+	}
+}