fix(delete): apply must rebuild the retirement evidence the approver signed

.gds/bundle.lock.yaml

@@ -5,14 +5,14 @@ bundle:
   version: "0.9.0-dev"
   release_sequence: 0
   channel: "development"
-  source_tree_digest: "sha256:6108aa7953daaa83e6f0497ce49ad328faa73f3b5711db3ab808ca982997f1f7"
-  digest: "sha256:76519a731f68615c821b7b79d2d4aa4dc98735a1fc8349e7b8e1fff27337bd34"
+  source_tree_digest: "sha256:e88904c3b4b3654f40b30843ea28f1ab33bb4ee45db6d6b0f0f89076857ba54f"
+  digest: "sha256:1e6b426bb6796d36de00d07037fbfc2591f2f6af30d63f2bd65ae091dbf82973"
 
 projection:
-  input_digest: "sha256:3ac5dff25cf43ff0c55f7ab2f13538402b8401f09da4a1b816f5ff898c17df03"
-  output_digest: "sha256:b975910bc9ac02a54efa6316051bf5f9c2909ff0d6ddf60a4e759f5b3faf98b7"
+  input_digest: "sha256:c939fde7254664217bc73f2b990ce7e73db38359ae62bcc8ca690ab6ca066b92"
+  output_digest: "sha256:0f5697325f6623916cb575aa8e1b2e0a3422635de596a1fd048d2776deda131c"
   files:
     - path: ".gds/compiled-policy.json"
       digest: "sha256:7ba2962e8afdf99eaf32582cd866f394abff2e24728b0644f013ec67d478b521"
     - path: ".github/workflows/gds-ci.yml"
-      digest: "sha256:01d2bb480a1d52e044ded2a2ee8a07b136e612ca030b81da4eab73398e57ebfd"
+      digest: "sha256:a6a19aa0ac91644efe450663924f229a7f474d016e58764be2b561608b4bd8b4"

.github/workflows/gds-ci.yml

@@ -1,8 +1,8 @@
 # GENERATED FILE - DO NOT EDIT DIRECTLY
 # generator: gds
 # bundle: 0.9.0-dev
-# source-tree-digest: sha256:6108aa7953daaa83e6f0497ce49ad328faa73f3b5711db3ab808ca982997f1f7
-# input-digest: sha256:3ac5dff25cf43ff0c55f7ab2f13538402b8401f09da4a1b816f5ff898c17df03
+# source-tree-digest: sha256:e88904c3b4b3654f40b30843ea28f1ab33bb4ee45db6d6b0f0f89076857ba54f
+# input-digest: sha256:c939fde7254664217bc73f2b990ce7e73db38359ae62bcc8ca690ab6ca066b92
 # output-digest: sha256:8c045e745cc69b731bc695a4a9d58a48c10f1ab7dd85b7354db7bfd0e072711c
 # edit-source:
 #   - .gds/repository.yaml

core/app/repository_delete.go

@@ -89,6 +89,31 @@ type repositoryDeleteObserver struct {
 	transition      repositoryworkflow.ProviderTransition
 }
 
+// newRepositoryDeleteObserver is the single construction site for the delete
+// precondition observer. Planning and apply both go through it because they
+// drifted apart once already: the apply path was built without the retirement
+// reader and without the preserved set, so every remote collection read as
+// unknown and every accepted loss read as blocking. `Retirable` was then false
+// forever, which the engine surfaced as GDS_STALE_PLAN with an empty mismatch
+// list — a failed observation wearing the name of a state change. Both inputs
+// now come from the transition, which the plan stores and the approval signs.
+func (services *Services) newRepositoryDeleteObserver(
+	root string,
+	transition repositoryworkflow.ProviderTransition,
+	options RepositoryDeleteOptions,
+	reader repositoryworkflow.ProviderReader,
+) repositoryDeleteObserver {
+	return repositoryDeleteObserver{
+		services: services, root: root, inventoryRoot: transition.AnalysisRoot,
+		maxDepth: options.MaxDepth, maxRepositories: options.MaxRepositories,
+		concurrency: options.Concurrency,
+		reader:      reader,
+		retirement:  retirementReaderFor(reader),
+		preserve:    transition.PreservedIdentities,
+		transition:  transition,
+	}
+}
+
 func (observer repositoryDeleteObserver) Observe(
 	ctx context.Context,
 	repositoryID string,
@@ -229,15 +254,10 @@ func (services *Services) PlanRepositoryDelete(
 		return domain.InternalError(command, err)
 	}
 	engine := operations.NewDefaultEngine(
-		store, services.Schemas, repositoryDeleteObserver{
-			services: services, root: current.root, inventoryRoot: current.transition.AnalysisRoot,
-			maxDepth: options.MaxDepth, maxRepositories: options.MaxRepositories,
-			concurrency: options.Concurrency,
-			reader:      providerRuntime.readers[current.transition.CurrentInstallation],
-			retirement:  retirementReaderFor(providerRuntime.readers[current.transition.CurrentInstallation]),
-			preserve:    options.PreserveIdentities,
-			transition:  current.transition,
-		}, nil, options.DeviceID, options.SessionID)
+		store, services.Schemas, services.newRepositoryDeleteObserver(
+			current.root, current.transition, options,
+			providerRuntime.readers[current.transition.CurrentInstallation],
+		), nil, options.DeviceID, options.SessionID)
 	engine.Now = services.Now
 	if err := engine.PutPlan(ctx, plan); err != nil {
 		return operationFailureEnvelope(command, err)
@@ -336,12 +356,10 @@ func (services *Services) applyRepositoryDelete(
 		return githubMutationRuntimeError(command, err)
 	}
 	handler := &repositoryworkflow.ProviderHandler{Readers: providerRuntime.readers, Writer: writer}
-	observer := repositoryDeleteObserver{
-		services: services, root: current.root, inventoryRoot: transition.AnalysisRoot,
-		maxDepth: options.MaxDepth, maxRepositories: options.MaxRepositories,
-		concurrency: options.Concurrency,
-		reader:      providerRuntime.readers[transition.CurrentInstallation], transition: transition,
-	}
+	observer := services.newRepositoryDeleteObserver(
+		current.root, transition, options,
+		providerRuntime.readers[transition.CurrentInstallation],
+	)
 	engine := operations.NewDefaultEngine(
 		store, services.Schemas, observer,
 		map[string]operations.ActionHandler{repositoryworkflow.ProviderLifecycleAction: handler},
@@ -446,6 +464,7 @@ func (services *Services) repositoryDeleteContext(
 		return repositoryDeleteContext{}, []domain.Finding{*finding}
 	}
 	transition.AnalysisRoot = physicalInventoryRoot
+	transition.PreservedIdentities = append([]string(nil), options.PreserveIdentities...)
 	index, indexFindings := services.completeRelationshipIndex(ctx, DiscoveryOptions{
 		Root: physicalInventoryRoot, MaxDepth: options.MaxDepth,
 		MaxRepositories: options.MaxRepositories, Concurrency: options.Concurrency,

core/app/repository_delete_observer_test.go

@@ -0,0 +1,39 @@
+package app
+
+import (
+	"testing"
+
+	repositoryworkflow "github.com/NDDev-OpenNetwork/github-device-sync/core/repository"
+)
+
+// The delete precondition observer is built twice — once when the plan is
+// stored and once when it is applied — and the two sites drifted apart, which
+// made every approved deletion fail as GDS_STALE_PLAN. Planning and apply must
+// derive the same observer from the same stored transition.
+func TestDeleteObserverCarriesTheApprovedRetirementDeclaration(t *testing.T) {
+	services := &Services{}
+	transition := repositoryworkflow.ProviderTransition{
+		Operation: repositoryworkflow.DeleteOperation, RepositoryID: "repo_01ABCDEFGHJKMNPQRSTVWXYZ01",
+		ProviderRepositoryID: 42, CurrentInstallation: "installation:github-personal",
+		CurrentOwner: "example-owner", CurrentName: "example", CurrentLifecycle: "archived",
+		TargetInstallation: "installation:github-personal",
+		TargetOwner:        "example-owner", TargetName: "example", TargetLifecycle: "tombstoned",
+		AnalysisRoot:        "/verified-estate",
+		PreservedIdentities: []string{"commits:unpushed", "ref:refs/tags/v0.6.10"},
+	}
+	options := RepositoryDeleteOptions{MaxDepth: 8, MaxRepositories: 2000, Concurrency: 4}
+
+	observer := services.newRepositoryDeleteObserver("/verified-estate/example", transition, options, nil)
+
+	if len(observer.preserve) != len(transition.PreservedIdentities) {
+		t.Fatalf("preserve=%#v transition=%#v", observer.preserve, transition.PreservedIdentities)
+	}
+	for index, identity := range transition.PreservedIdentities {
+		if observer.preserve[index] != identity {
+			t.Fatalf("preserve[%d]=%q expected %q", index, observer.preserve[index], identity)
+		}
+	}
+	if observer.inventoryRoot != transition.AnalysisRoot {
+		t.Fatalf("inventoryRoot=%q expected %q", observer.inventoryRoot, transition.AnalysisRoot)
+	}
+}

core/cli/root.go

@@ -2269,10 +2269,11 @@ func (executor *executor) repositoryDeleteCommand() *cobra.Command {
 					})
 				}
 				if !plan && (child.Flags().Changed("inventory-root") ||
-					child.Flags().Changed("confirm-repository-id") || child.Flags().Changed("confirm-provider-id")) {
+					child.Flags().Changed("confirm-repository-id") ||
+					child.Flags().Changed("confirm-provider-id") || child.Flags().Changed("preserve")) {
 					return domain.NewEnvelope("gds repository delete", domain.ExitInput, nil, domain.Finding{
 						Code: "GDS_REPOSITORY_DELETE_INPUT_CONFLICT", Severity: domain.SeverityHigh,
-						Message: "Inventory and confirmation values are planning inputs and cannot alter a stored plan.",
+						Message: "Inventory, confirmation and preservation values are planning inputs and cannot alter a stored plan.",
 					})
 				}
 				switch {

core/repository/lifecycle.go

@@ -36,6 +36,13 @@ type ProviderTransition struct {
 	TargetName             string `json:"target_name"`
 	TargetLifecycle        string `json:"target_lifecycle"`
 	AnalysisRoot           string `json:"analysis_root,omitempty"`
+	// PreservedIdentities is the exact set of retirement identities the
+	// operator accepted losing when the plan was built. It lives on the
+	// transition, and therefore inside the plan parameters and the plan
+	// digest, because the apply path has to rebuild the same retirement
+	// evidence the approver saw. Recomputing it from apply-time flags would
+	// let an approved plan be applied against a different declaration.
+	PreservedIdentities []string `json:"preserved_identities,omitempty"`
 }
 
 func ValidateDelete(current domain.RepositoryAnchor) (ProviderTransition, []domain.Finding) {
@@ -76,6 +83,13 @@ func Parameters(transition ProviderTransition) map[string]any {
 	if transition.AnalysisRoot != "" {
 		parameters["analysis_root"] = transition.AnalysisRoot
 	}
+	if len(transition.PreservedIdentities) != 0 {
+		identities := make([]any, 0, len(transition.PreservedIdentities))
+		for _, identity := range transition.PreservedIdentities {
+			identities = append(identities, identity)
+		}
+		parameters["preserved_identities"] = identities
+	}
 	return map[string]any{"repository_provider": parameters}
 }
 
@@ -102,6 +116,15 @@ func StepTransition(step operations.Step) (ProviderTransition, error) {
 	result.TargetName, _ = raw["target_name"].(string)
 	result.TargetLifecycle, _ = raw["target_lifecycle"].(string)
 	result.AnalysisRoot, _ = raw["analysis_root"].(string)
+	if identities, ok := raw["preserved_identities"].([]any); ok {
+		for _, identity := range identities {
+			value, valid := identity.(string)
+			if !valid || value == "" {
+				return ProviderTransition{}, errors.New("repository provider parameters are invalid")
+			}
+			result.PreservedIdentities = append(result.PreservedIdentities, value)
+		}
+	}
 	if (result.Operation != RenameOperation && result.Operation != TransferOperation &&
 		result.Operation != ArchiveOperation && result.Operation != DeleteOperation) ||
 		result.ProviderRepositoryID < 1 || result.CurrentInstallation == "" || result.CurrentOwner == "" ||

core/repository/lifecycle_test.go

@@ -115,3 +115,46 @@ func assertTransitionFinding(t *testing.T, findings []domain.Finding, code strin
 	}
 	t.Fatalf("missing %s in %#v", code, findings)
 }
+
+func TestDeletePlanCarriesTheAcceptedLossesThroughItsParameters(t *testing.T) {
+	current := lifecycleAnchor()
+	current.Repository.Lifecycle = "archived"
+	transition, findings := ValidateDelete(current)
+	if len(findings) != 0 {
+		t.Fatalf("findings=%#v", findings)
+	}
+	// What the operator accepted losing is part of what the approver signs, so
+	// it has to survive into the stored plan: the apply path rebuilds the
+	// retirement evidence from it, and an empty set makes every accepted loss
+	// read as blocking again.
+	transition.AnalysisRoot = "/verified-estate"
+	transition.PreservedIdentities = []string{"commits:unpushed", "ref:refs/tags/v0.6.10"}
+	decoded, err := StepTransition(operations.Step{
+		RepositoryID: transition.RepositoryID, Action: ProviderLifecycleAction,
+		Parameters: Parameters(transition),
+	})
+	if err != nil || !reflect.DeepEqual(decoded, transition) {
+		t.Fatalf("decoded=%#v transition=%#v err=%v", decoded, transition, err)
+	}
+}
+
+func TestDeleteParametersRejectANonStringAcceptedLoss(t *testing.T) {
+	current := lifecycleAnchor()
+	current.Repository.Lifecycle = "archived"
+	transition, findings := ValidateDelete(current)
+	if len(findings) != 0 {
+		t.Fatalf("findings=%#v", findings)
+	}
+	parameters := Parameters(transition)
+	provider, ok := parameters["repository_provider"].(map[string]any)
+	if !ok {
+		t.Fatalf("parameters=%#v", parameters)
+	}
+	provider["preserved_identities"] = []any{"commits:unpushed", 7}
+	if _, err := StepTransition(operations.Step{
+		RepositoryID: transition.RepositoryID, Action: ProviderLifecycleAction,
+		Parameters: parameters,
+	}); err == nil {
+		t.Fatal("a non-string accepted loss must not decode into a preservation declaration")
+	}
+}