fix(generate): gate finalize's release/state-write/candidate-dispatch on dry_run - #665
Merged
Merged
Conversation
… on dry_run A dry-run `orchestrate` dispatch (`dry_run: true`) is meant to be a rehearsal that mutates nothing, but the finalize job's real mutations were never gated: - The `Manage Release` step invoked `cascade manage-release` with no `--dry-run` flag, so it cut a real tag and created a real release even though the CLI's own tested `printDryRunPlan` gate (internal/release/command.go) was built to prevent exactly that. The step now forwards the dispatch input as `--dry-run`; the composite `manage-release` action gains a `dry_run` input to carry it. - The `Update Manifest` step committed real state to trunk. It now previews the yq edit and exits before its commit/push loop when `DRY_RUN` is set. - The `Dispatch Release Candidate Build` step fired a real external release workflow run. Its `if:` now also requires `dry_run != 'true'`, mirroring the native-deployment step's existing guard. All three use the null-safe `github.event.inputs.dry_run` accessor so they behave correctly on orchestrate's non-dispatch triggers (push/schedule/ workflow_run), where the inputs context is null. Proven by generation-correctness tests in internal/generate/dry_run_finalize_test.go (red before the fix) and a runtime e2e scenario orchestrate/dry-run-finalize-no-mutations.yaml that dispatches a dry-run orchestrate and asserts no tag was cut and state.dev stayed unwritten. Action-mode (non-dry-run) output is unchanged apart from the additive input. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
cascade generates its own workflows (own-repo mode), so the generator change in the previous commit leaves the committed .github/workflows/orchestrate.yaml and .github/actions/manage-release/action.yaml stale, which the drift check catches. Regenerated with `cascade generate-workflow --own-repo --config .github/manifest.yaml --force`; `cascade verify --own-repo` reports no drift. Additive dry_run wiring only; no other change. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
…ors the input The runtime scenario ran a push-mode orchestrate under workflow_dispatch, but act's --detect-event reruns a workflow that still declares a push: trigger as a push, leaving github.event.inputs empty so the dry_run gate never fired and the rehearsal wrote real state. Switch the manifest to release_trigger: dispatch, which drops the push: trigger, so act runs the workflow_dispatch path and seeds github.event.inputs.dry_run. This mirrors the proven state-advancing dispatch path in 40-release-trigger-dispatch-only.yaml; here the run is a dry-run that must advance nothing. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
Two CI runs confirmed act does not surface github.event.inputs.dry_run to the orchestrate finalize job (the dispatch input the guards read), even under a dispatch-only manifest: the rehearsal still cut the candidate tag and advanced state. This is the same act limitation that already keeps the sibling orchestrate dry-run coverage (dry-run-input-expression.yaml) generation-only. Rather than ship a runtime scenario that cannot exercise the behavior, remove it and the unused OrchestrateStep.dry_run harness knob. The three guards stay proven by the red-able generation-correctness tests in internal/generate/dry_run_finalize_test.go (each turns red when its guard is reverted), and the Manage Release path is additionally backed by the CLI-level dry-run test in internal/release. orchestrate coverage in the registry is already satisfied by 02/03; the runtime dry-run behavior is verified on real GitHub, where github.event.inputs is populated. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
orchestratedispatch (dry_run: true) currently creates a real tag and release and commits real state to trunk; thefinalizejob's mutations were never gated ondry_run.Manage Releasenow forwards--dry-runtocascade manage-release, whose already-testedprintDryRunPlangate (internal/release/command.go) previews the plan instead of mutating; the compositemanage-releaseaction gains adry_runinput to carry it.Update Manifestpreviews the state edit and exits before its commit/push loop;Dispatch Release Candidate Build'sif:now also requiresdry_run != 'true'.github.event.inputs.dry_runaccessor so they behave on orchestrate's non-dispatch triggers (push/schedule/workflow_run), matching the existingwriteNativeDeploymentStepsguard and callback dry_run forwarding.Why / value
dry_runrehearsal is to mutate nothing, yetfinalizecut real tags/releases and wrote real state, so operators could not safely rehearse an orchestrate run.--dry-rungate already existed and was tested (internal/release/command.go:105); it simply was never wired up from the generated workflow side. This connects the two, matching the project convention that--dry-runis gated twice (at the command and at the mutation boundary).DRY_RUNshell guard, mirroring the native-deployment step's existing style).Evidence
Manage Releaseforwardsdry_run; the action threads--dry-runto the CLITestManageReleaseAction_ThreadsDryRunFlag,TestWriteReleaseStep_ForwardsDryRunDispatchInput; runtime dry-run itself is covered by the existinginternal/releaseprintDryRunPlantestsUpdate Manifestpreviews and exits before committing stateTestWriteManifestUpdateStep_GatesMutationOnDryRun(asserts the gate precedes the commit/push loop)Dispatch Release Candidate Buildis suppressed on dry-runTestWriteCandidateDispatchStep_GatedOnDryRunTestByteIdenticalBaseline_SingleComponent(goldens regenerated: additivedry_runonly), fullgo test ./...,go vet,golangci-lint,actionlinton regenerated golden all clean; own-repo workflows regenerated so the drift check passesEach generation-correctness test turns red when its guard is reverted, so a reviewer can falsify it by reverting the behavior alone.
Note on runtime e2e: an earlier revision added a dry-run orchestrate e2e scenario, but two CI runs showed
actdoes not surfacegithub.event.inputs.dry_runto the orchestratefinalizejob (the rehearsal still cut the tag and advanced state), the same limitation that keeps the siblingdry-run-input-expression.yamlgeneration-only. It was removed rather than shipped green-for-the-wrong-reason; the runtime behavior is exercised on real GitHub, wheregithub.event.inputsis populated.