From c752b6d99023f6982d7eaeb247e99b43f97005d1 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Fri, 24 Jul 2026 00:33:47 -0400 Subject: [PATCH 1/2] fix(verify): thread --cli-install mode through Plan and drift-check verify (and the generated cascade-drift-check.yaml, which runs it) always re-planned every generator assuming the default action install mode: Plan never called setInstallMode on any generator, unlike generate-workflow's own command.go, which does so for all eleven generator types before rendering. A repo generated with --cli-install=binary would therefore report every Setup CLI step as spurious drift, forever, since nothing could tell verify which mode to check against. Fixed by mirroring generate-workflow's own pattern exactly: PlanOptions gained a CLIInstall field, parsed once in Plan and applied via setInstallMode to every generator Plan constructs, so a binary-mode repo now verifies clean when told --cli-install=binary. Added the matching --cli-install flag to the verify command itself. Also fixed the other half of the same gap: cascade-drift-check.yaml's own emitted "cascade verify" invocation never passed --cli-install, so even a correctly-generated binary-mode repo's own CI would still report drift at runtime. The generator now emits --cli-install=binary when its own installMode is binary, and omits the flag entirely in the default action mode (byte-identical output for every existing manifest, confirmed by the full byte-identical regression suite still passing unchanged). Verified: two new verify tests prove a binary-mode repo verifies clean when the mode is told, and reports real drift when it isn't (the control, proving the two modes produce genuinely different bytes rather than the test passing by coincidence). Two new drift-check generator tests lock in the flag being present in binary mode and absent in action mode. Full repo test suite, go vet, golangci-lint, and gofmt all pass. --- internal/generate/drift_check.go | 11 +++- internal/generate/drift_check_test.go | 25 ++++++++ internal/generate/plan.go | 35 +++++++++- internal/verify/command.go | 1 + internal/verify/verify.go | 6 ++ internal/verify/verify_test.go | 92 +++++++++++++++++++++++++++ 6 files changed, 166 insertions(+), 4 deletions(-) diff --git a/internal/generate/drift_check.go b/internal/generate/drift_check.go index 05a94f84..196276ae 100644 --- a/internal/generate/drift_check.go +++ b/internal/generate/drift_check.go @@ -153,7 +153,16 @@ func (g *DriftCheckGenerator) writeCheckJob(sb *strings.Builder) { sb.WriteString(" - name: Check for workflow drift\n") sb.WriteString(" run: |\n") sb.WriteString(" set +e\n") - fmt.Fprintf(sb, " cascade verify --config %s > drift-report.txt 2>&1\n", g.getManifestFilePath()) + // --cli-install is omitted in action mode (the default) so existing + // manifests keep byte-identical output; verify's own --cli-install + // default already matches. Binary mode must say so explicitly, or verify + // silently re-plans every file assuming action mode and reports spurious + // drift on every Setup CLI step. + if g.installMode == cliInstallModeBinary { + fmt.Fprintf(sb, " cascade verify --config %s --cli-install=binary > drift-report.txt 2>&1\n", g.getManifestFilePath()) + } else { + fmt.Fprintf(sb, " cascade verify --config %s > drift-report.txt 2>&1\n", g.getManifestFilePath()) + } sb.WriteString(" echo $? > drift-exit.txt\n") sb.WriteString(" set -e\n") sb.WriteString(" cat drift-report.txt\n") diff --git a/internal/generate/drift_check_test.go b/internal/generate/drift_check_test.go index b2b0df79..3df638d9 100644 --- a/internal/generate/drift_check_test.go +++ b/internal/generate/drift_check_test.go @@ -192,3 +192,28 @@ func TestDriftCheckGenerator_Actionlint(t *testing.T) { out, runErr := cmd.CombinedOutput() assert.NoError(t, runErr, "actionlint reported issues:\n%s", string(out)) } + +// TestDriftCheckGenerator_ActionMode_OmitsCLIInstallFlag proves the default +// (action-mode) verify invocation is byte-identical to before --cli-install +// existed: no downstream manifest's committed cascade-drift-check.yaml changes +// just because this generator learned a new flag. +func TestDriftCheckGenerator_ActionMode_OmitsCLIInstallFlag(t *testing.T) { + g := NewDriftCheckGenerator(driftCheckConfig(false), t.TempDir()) + content, err := g.Generate() + require.NoError(t, err) + assert.Contains(t, content, "cascade verify --config .github/manifest.yaml > drift-report.txt") + assert.NotContains(t, content, "--cli-install") +} + +// TestDriftCheckGenerator_BinaryMode_PassesCLIInstallFlag proves a +// binary-mode-generated repo's own drift-check workflow invokes verify with +// the matching --cli-install=binary flag, so verify re-plans the repo in the +// same mode it was generated in instead of silently assuming action mode and +// reporting every Setup CLI step as drift. +func TestDriftCheckGenerator_BinaryMode_PassesCLIInstallFlag(t *testing.T) { + g := NewDriftCheckGenerator(driftCheckConfig(false), t.TempDir()) + g.setInstallMode(cliInstallModeBinary) + content, err := g.Generate() + require.NoError(t, err) + assert.Contains(t, content, "cascade verify --config .github/manifest.yaml --cli-install=binary > drift-report.txt") +} diff --git a/internal/generate/plan.go b/internal/generate/plan.go index 547ebed9..7060135a 100644 --- a/internal/generate/plan.go +++ b/internal/generate/plan.go @@ -41,6 +41,13 @@ type PlanOptions struct { // verify/generate-workflow --own-repo invocation sets this; it is not a // manifest field. OwnRepo bool + // CLIInstall selects how every planned generator emits its "Setup CLI" + // step: "" or "action" (default) for the setup-cli composite action, or + // "binary" for the self-contained install. Mirrors generate-workflow's + // --cli-install flag; verify passes its own --cli-install here so a + // binary-mode-generated repo can be planned back for comparison instead + // of always assuming the action-mode default. + CLIInstall string } // Plan resolves the manifest and returns the complete set of files the generate @@ -65,6 +72,14 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { return nil, fmt.Errorf("parsing config: %w", err) } + // Parsed once and applied to every generator below via setInstallMode, + // mirroring exactly what the generate-workflow command does (command.go) + // so Plan can never disagree with generate-workflow on a binary-mode repo. + installMode, err := parseCLIInstallMode(opts.CLIInstall) + if err != nil { + return nil, fmt.Errorf("parsing --cli-install: %w", err) + } + if opts.PinOverridesPath != "" { if err := ApplyDiskPinOverrides(cfg, opts.PinOverridesPath); err != nil { return nil, fmt.Errorf("applying pin overrides: %w", err) @@ -113,6 +128,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { } var content string for _, t := range orchTargets { + t.Gen.setInstallMode(installMode) content, err = t.Gen.Generate() if err != nil { return nil, fmt.Errorf("generating orchestrate workflow: %w", err) @@ -125,7 +141,9 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // out to one promote-.yaml per component; otherwise a single // promote.yaml, byte-identical to today. if cfg.IsSingleEnvironment() { - content, err = NewReleaseGenerator(cfg, baseDir).Generate() + relGen := NewReleaseGenerator(cfg, baseDir) + relGen.setInstallMode(installMode) + content, err = relGen.Generate() if err != nil { return nil, fmt.Errorf("generating release workflow: %w", err) } @@ -136,6 +154,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { return nil, perr } for _, t := range promoteTargets { + t.Gen.setInstallMode(installMode) content, err = t.Gen.Generate() if err != nil { return nil, fmt.Errorf("generating promote workflow: %w", err) @@ -146,7 +165,9 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // 3. external-update -> .github/workflows/external-update.yaml when primary. if cfg.IsPrimary() { - content, err = NewExternalUpdateGenerator(cfg, baseDir).Generate() + extGen := NewExternalUpdateGenerator(cfg, baseDir) + extGen.setInstallMode(installMode) + content, err = extGen.Generate() if err != nil { return nil, fmt.Errorf("generating external-update workflow: %w", err) } @@ -155,6 +176,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // 4. validate-check -> .github/workflows/cascade-validate.yaml when enabled. if gen := NewValidateCheckGenerator(cfg, baseDir); gen.Enabled() { + gen.setInstallMode(installMode) content, err = gen.Generate() if err != nil { return nil, fmt.Errorf("generating validate-check workflow: %w", err) @@ -164,6 +186,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // 5. merge-queue -> .github/workflows/cascade-merge-queue.yaml when enabled. if gen := NewMergeQueueGenerator(cfg, baseDir); gen.Enabled() { + gen.setInstallMode(installMode) content, err = gen.Generate() if err != nil { return nil, fmt.Errorf("generating merge-queue workflow: %w", err) @@ -179,6 +202,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { return nil, err } for _, t := range hfTargets { + t.Gen.setInstallMode(installMode) content, err = t.Gen.Generate() if err != nil { return nil, fmt.Errorf("generating hotfix workflow: %w", err) @@ -194,6 +218,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { return nil, err } for _, t := range rbTargets { + t.Gen.setInstallMode(installMode) content, err = t.Gen.Generate() if err != nil { return nil, fmt.Errorf("generating rollback workflow: %w", err) @@ -203,7 +228,9 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // 8. pr-preview -> .github/workflows/cascade-pr-preview.yaml when enabled. if cfg.PRPreview.IsEnabled() { - content, err = NewPRPreviewGenerator(cfg, baseDir).Generate() + previewGen := NewPRPreviewGenerator(cfg, baseDir) + previewGen.setInstallMode(installMode) + content, err = previewGen.Generate() if err != nil { return nil, fmt.Errorf("generating pr-preview workflow: %w", err) } @@ -213,6 +240,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // 9. drift-check -> .github/workflows/cascade-drift-check.yaml when enabled, // plus the fork-safe comment companion when drift_check.comment is set. if gen := NewDriftCheckGenerator(cfg, baseDir); gen.Enabled() { + gen.setInstallMode(installMode) content, err = gen.Generate() if err != nil { return nil, fmt.Errorf("generating drift-check workflow: %w", err) @@ -234,6 +262,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // sees the same two files generate writes and reports no drift on a clean // tree. if gen := NewReconcileGenerator(cfg, baseDir); gen.Enabled() { + gen.setInstallMode(installMode) content, err = gen.Generate() if err != nil { return nil, fmt.Errorf("generating reconcile-check workflow: %w", err) diff --git a/internal/verify/command.go b/internal/verify/command.go index 0056842e..f3660f19 100644 --- a/internal/verify/command.go +++ b/internal/verify/command.go @@ -38,6 +38,7 @@ verify is read-only: it never writes files, runs git, or modifies the repo.`, cmd.Flags().BoolVarP(&o.Quiet, "quiet", "q", false, "Suppress the per-file report body; only set the exit code") cmd.Flags().BoolVar(&o.AllowOrphans, "allow-orphans", false, "Do not report cascade-owned workflow files that are no longer in the plan as drift") cmd.Flags().BoolVar(&o.OwnRepo, "own-repo", false, "Verify against cascade's own-repo release-plumbing variant (tag-only manage-release, non-triggering tag-create). Maintainer-only; never used by a downstream manifest.") + cmd.Flags().StringVar(&o.CLIInstall, "cli-install", "action", "How the committed workflows install the cascade CLI: \"action\" (setup-cli composite action, default) or \"binary\" (self-contained install). Must match the mode generate-workflow --cli-install used, or every Setup CLI step reports as spurious drift.") return cmd } diff --git a/internal/verify/verify.go b/internal/verify/verify.go index a65457b3..a10e504e 100644 --- a/internal/verify/verify.go +++ b/internal/verify/verify.go @@ -78,6 +78,11 @@ type Options struct { // the own-repo output; without this, verify would compute the plain variant // and report the deliberate own-repo differences as spurious drift. OwnRepo bool + // CLIInstall mirrors generate-workflow's --cli-install flag ("" / "action" + // or "binary"). It must match the mode the committed files were actually + // generated with, or every planned file whose Setup CLI step differs by + // mode reports as spurious drift. + CLIInstall string } // Run compares every file the manifest would generate against the bytes @@ -103,6 +108,7 @@ func Run(o Options, stdout, stderr io.Writer) error { OutputPath: o.OutputPath, PromoteOutputPath: o.PromoteOutputPath, OwnRepo: o.OwnRepo, + CLIInstall: o.CLIInstall, }) if err != nil { return operational(fmt.Errorf("planning workflows: %w", err)) diff --git a/internal/verify/verify_test.go b/internal/verify/verify_test.go index 2695235b..e7e355cf 100644 --- a/internal/verify/verify_test.go +++ b/internal/verify/verify_test.go @@ -71,27 +71,89 @@ func newRepo(t *testing.T) string { // path is absolute so Plan resolves the manifest, base directory, and emitted // files without consulting the process working directory. func planOpts(dir string) generate.PlanOptions { + return planOptsWithCLIInstall(dir, "") +} + +// planOptsWithCLIInstall is planOpts with an explicit --cli-install mode, for +// tests that need to plan (or re-plan) a repo generated in binary mode. +func planOptsWithCLIInstall(dir, cliInstall string) generate.PlanOptions { return generate.PlanOptions{ ConfigPath: filepath.Join(dir, ".github", "manifest.yaml"), ManifestKey: config.DefaultManifestKey, ActionFolder: "manage-release", OutputPath: filepath.Join(dir, ".github", "workflows", "orchestrate.yaml"), PromoteOutputPath: filepath.Join(dir, ".github", "workflows", "promote.yaml"), + CLIInstall: cliInstall, } } // opts builds the verify options for a repo rooted at dir, mirroring planOpts so // the verify run reads the same absolute paths the plan emitted. func opts(dir string) Options { + return optsWithCLIInstall(dir, "") +} + +// optsWithCLIInstall is opts with an explicit --cli-install mode, mirroring +// planOptsWithCLIInstall so a verify Run reads back what a matching Plan wrote. +func optsWithCLIInstall(dir, cliInstall string) Options { return Options{ ConfigPath: filepath.Join(dir, ".github", "manifest.yaml"), ManifestKey: config.DefaultManifestKey, ActionFolder: "manage-release", OutputPath: filepath.Join(dir, ".github", "workflows", "orchestrate.yaml"), PromoteOutputPath: filepath.Join(dir, ".github", "workflows", "promote.yaml"), + CLIInstall: cliInstall, } } +// newRepoWithCLIInstall mirrors newRepo but plans and materializes the repo +// using the given --cli-install mode, so tests can build a binary-mode +// generated fixture the same way a real adopter's repo would look. +func newRepoWithCLIInstall(t *testing.T, cliInstall string) string { + t.Helper() + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".github", "workflows"), 0o755)) + + stubs := map[string]string{ + ".github/workflows/image-build.yaml": "" + + "name: Image Build\non:\n workflow_call:\n inputs:\n os:\n type: string\n", + ".github/workflows/bundle-build.yaml": "" + + "name: Bundle Build\non:\n workflow_call:\n inputs:\n image:\n type: string\n", + ".github/workflows/deploy.yaml": "" + + "name: Deploy\non:\n workflow_call:\n inputs:\n environment:\n type: string\n", + } + for path, body := range stubs { + require.NoError(t, os.WriteFile(filepath.Join(dir, path), []byte(body), 0o644)) + } + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: config.EnvNames("dev", "staging", "prod"), + Builds: []config.BuildConfig{ + {Name: "image", Workflow: ".github/workflows/image-build.yaml", Triggers: []string{"src/**"}}, + }, + Deploys: []config.DeployConfig{ + {Name: "app", Workflow: ".github/workflows/deploy.yaml", Triggers: []string{"src/**"}, DependsOn: []string{"image"}}, + }, + } + manifest := map[string]any{config.DefaultManifestKey: config.CICDFile{Config: cfg}} + body, err := yaml.Marshal(manifest) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, ".github", "manifest.yaml"), body, 0o644)) + + planned, err := generate.Plan(planOptsWithCLIInstall(dir, cliInstall)) + require.NoError(t, err) + for _, p := range planned { + path := p.Path + if !filepath.IsAbs(path) { + path = filepath.Join(dir, path) + } + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(p.Content), 0o644)) + } + return dir +} + func TestRun_CleanRepo_NoDrift(t *testing.T) { t.Parallel() dir := newRepo(t) @@ -349,3 +411,33 @@ func TestErrDrift_ExitCodeOne(t *testing.T) { require.ErrorAs(t, error(ErrDrift), &ec) require.Equal(t, 1, ec.ExitCode()) } + +// TestRun_CLIInstallBinary_NoDriftWhenModeMatches proves verify can correctly +// check a repo generated with --cli-install=binary: without CLIInstall wired +// through to the underlying Plan, every generator silently defaults to action +// mode internally, so a binary-mode repo would report spurious drift on every +// file whose Setup CLI step differs by mode even though nothing is out of +// sync. +func TestRun_CLIInstallBinary_NoDriftWhenModeMatches(t *testing.T) { + t.Parallel() + dir := newRepoWithCLIInstall(t, "binary") + + var out, errOut bytes.Buffer + err := Run(optsWithCLIInstall(dir, "binary"), &out, &errOut) + require.NoError(t, err, "a clean binary-mode repo must verify with no drift when CLIInstall matches; report:\n%s", errOut.String()) + require.Contains(t, out.String(), "no drift") +} + +// TestRun_CLIInstallBinary_MismatchedModeIsRealDrift is the control for the +// test above: a binary-mode repo checked WITHOUT CLIInstall set (defaulting to +// action) must still report drift, proving the two modes produce genuinely +// different bytes and the prior test isn't passing by coincidence. +func TestRun_CLIInstallBinary_MismatchedModeIsRealDrift(t *testing.T) { + t.Parallel() + dir := newRepoWithCLIInstall(t, "binary") + + var out, errOut bytes.Buffer + err := Run(opts(dir), &out, &errOut) // opts(dir) leaves CLIInstall unset (action) + require.Error(t, err) + require.True(t, errors.Is(err, ErrDrift), "action-mode verify against a binary-mode repo must report drift, got %v", err) +} From e69939ea8ee47b80b2eb934474b0d9763d4917bd Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Fri, 24 Jul 2026 00:41:32 -0400 Subject: [PATCH 2/2] docs: add Unreleased changelog entry for the verify --cli-install fix TestChangelog_UnreleasedCoversUserFacingCommits correctly caught this was missing from the prior commit. --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef487ffa..55b8397b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,16 @@ A `Migration` section is added to any release that bumps `schema_version`. ### Fixed +- **verify:** `cascade verify` (and the generated `cascade-drift-check.yaml`, + which runs it) always re-planned every generator assuming the default + `action` `--cli-install` mode, so a repo generated with + `--cli-install=binary` reported every Setup CLI step as spurious drift, + permanently, with no way to reconcile it. `verify` now accepts its own + `--cli-install` flag, threaded through to every generator `Plan` builds, and + the drift-check generator's emitted `cascade verify` invocation now passes + `--cli-install=binary` when that is the mode it was generated in. Action-mode + output is unchanged. + - **release:** A release cut that materializes a git tag now fails closed when the tag already exists at a different commit, instead of treating the `422 reference already exists` response as an unconditional success. The old