diff --git a/.gds/bundle.lock.yaml b/.gds/bundle.lock.yaml index e62254d..b38287f 100644 --- a/.gds/bundle.lock.yaml +++ b/.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" diff --git a/.github/workflows/gds-ci.yml b/.github/workflows/gds-ci.yml index a55164a..c150c79 100644 --- a/.github/workflows/gds-ci.yml +++ b/.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 diff --git a/core/app/repository_delete.go b/core/app/repository_delete.go index f47a7f3..7a1e365 100644 --- a/core/app/repository_delete.go +++ b/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, diff --git a/core/app/repository_delete_observer_test.go b/core/app/repository_delete_observer_test.go new file mode 100644 index 0000000..ba944fd --- /dev/null +++ b/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) + } +} diff --git a/core/cli/root.go b/core/cli/root.go index 284873b..cc508e9 100644 --- a/core/cli/root.go +++ b/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 { diff --git a/core/repository/lifecycle.go b/core/repository/lifecycle.go index 1eb2a89..1ba48db 100644 --- a/core/repository/lifecycle.go +++ b/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 == "" || diff --git a/core/repository/lifecycle_test.go b/core/repository/lifecycle_test.go index 39e7d16..2a97bbe 100644 --- a/core/repository/lifecycle_test.go +++ b/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") + } +}