feat(cigen): per-phase secret scoping + migration --format json/--env

cigen/analyze.go

@@ -101,15 +101,31 @@ func Analyze(configs []string, opts Options) (*CIPlan, error) {
 	if primaryConfigPath == "" {
 		primaryConfigPath = relativizeConfigPath(primaryPath)
 	}
+
+	// Per-phase secret scoping (multi-phase only). The prereq phase is scoped to
+	// the secrets ITS config references; the deploy (last) phase keeps the
+	// primary union (which already includes the migration DBEnv via deriveSecrets).
+	var prereqSecrets []SecretRef
+	scoped := false
 	phaseConfigPath := opts.PhaseConfig
 	if phaseConfigPath != "" {
 		if opts.PhaseConfigAlias != "" {
 			phaseConfigPath = opts.PhaseConfigAlias
 		} else {
-			phaseConfigPath = relativizeConfigPath(phaseConfigPath)
+			phaseConfigPath = relativizeConfigPath(opts.PhaseConfig)
+		}
+		if pcfg, perr := config.LoadFromFile(opts.PhaseConfig); perr == nil {
+			// deriveMigrations is primary-only: the migrating phase is the LAST
+			// phase, so the prereq never gets a migration DBEnv (pass nil).
+			prereqSecrets = deriveSecrets(pcfg, nil)
+			scoped = true
+		} else {
+			plan.Warnings = append(plan.Warnings,
+				fmt.Sprintf("per-phase secret scoping unavailable: phase config %q not loadable (%v); using union", opts.PhaseConfig, perr))
 		}
 	}
-	plan.Phases = derivePhases(primaryConfigPath, phaseConfigPath)
+
+	plan.Phases = derivePhases(primaryConfigPath, phaseConfigPath, plan.Secrets, prereqSecrets, scoped)
 
 	return plan, nil
 }
@@ -223,6 +239,12 @@ func deriveMigrations(cfg *config.WorkflowConfig) *MigrationsSpec {
 	if spec.DBEnv == "" {
 		return nil
 	}
+	// Derive --env only when exactly one environment is declared (unambiguous).
+	if len(m.Environments) == 1 {
+		for envName := range m.Environments {
+			spec.Env = envName
+		}
+	}
 	return spec
 }
 
@@ -390,18 +412,28 @@ func extractPrimaryDomain(cfg map[string]any) string {
 }
 
 // derivePhases builds the ordered list of deploy phases.
-func derivePhases(primaryPath, phaseConfig string) []DeployPhase {
+func derivePhases(primaryPath, phaseConfig string, primarySecrets, prereqSecrets []SecretRef, scoped bool) []DeployPhase {
 	var phases []DeployPhase
 	if phaseConfig != "" {
 		phases = append(phases, DeployPhase{
 			Name:       "prereq",
 			ConfigPath: phaseConfig,
+			Secrets:    prereqSecrets,
+			Scoped:     scoped,
 		})
 	}
-	phases = append(phases, DeployPhase{
+	deploy := DeployPhase{
 		Name:       "deploy",
 		ConfigPath: primaryPath,
-	})
+	}
+	// The deploy phase is scoped to the primary union only in the multi-phase
+	// case (scoped==true means the prereq loaded). Single-phase stays unscoped:
+	// the union IS its scope, so the renderer's union fallback applies unchanged.
+	if scoped {
+		deploy.Secrets = primarySecrets
+		deploy.Scoped = true
+	}
+	phases = append(phases, deploy)
 	return phases
 }
 
@@ -428,5 +460,13 @@ func deriveWarnings(cfg *config.WorkflowConfig, migrations *MigrationsSpec, secr
 		}
 	}
 
+	// (c) migrations environment ambiguity: ≥2 declared → --env omitted.
+	if cfg.CI != nil && len(cfg.CI.Migrations) > 0 {
+		if n := len(cfg.CI.Migrations[0].Environments); n >= 2 {
+			warnings = append(warnings,
+				fmt.Sprintf("migrations environment ambiguous (%d declared); --env omitted — set it in the generated workflow", n))
+		}
+	}
+
 	return warnings
 }

cigen/analyze_phase_test.go

@@ -0,0 +1,114 @@
+package cigen
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/GoCodeAlone/workflow/config"
+)
+
+func TestAnalyze_PerPhaseScoping_PrereqExcludesDeploySecret(t *testing.T) {
+	plan, err := Analyze([]string{"testdata/multisite/deploy.yaml"}, Options{
+		PhaseConfig: "testdata/multisite/deploy.prereq.yaml",
+	})
+	if err != nil {
+		t.Fatalf("Analyze: %v", err)
+	}
+	if len(plan.Phases) != 2 {
+		t.Fatalf("expected 2 phases, got %d", len(plan.Phases))
+	}
+	prereq, deploy := plan.Phases[0], plan.Phases[1]
+	if !prereq.Scoped || !deploy.Scoped {
+		t.Fatalf("expected both phases Scoped, got prereq=%v deploy=%v", prereq.Scoped, deploy.Scoped)
+	}
+	if hasSecret(prereq.Secrets, "MULTISITE_DB_URL") {
+		t.Errorf("prereq phase must NOT carry the deploy-only migration secret MULTISITE_DB_URL; got %v", names(prereq.Secrets))
+	}
+	if !hasSecret(deploy.Secrets, "MULTISITE_DB_URL") {
+		t.Errorf("deploy (last) phase must carry MULTISITE_DB_URL; got %v", names(deploy.Secrets))
+	}
+	// prereq genuinely needs the provider token — sanity that scoping isn't empty.
+	if !hasSecret(prereq.Secrets, "DIGITALOCEAN_TOKEN") {
+		t.Errorf("prereq phase should carry DIGITALOCEAN_TOKEN; got %v", names(prereq.Secrets))
+	}
+}
+
+func TestAnalyze_SinglePhase_NotScoped(t *testing.T) {
+	plan, err := Analyze([]string{"testdata/multisite/deploy.yaml"}, Options{})
+	if err != nil {
+		t.Fatalf("Analyze: %v", err)
+	}
+	if len(plan.Phases) != 1 {
+		t.Fatalf("expected 1 phase, got %d", len(plan.Phases))
+	}
+	if plan.Phases[0].Scoped {
+		t.Errorf("single-phase deploy must not be Scoped (union is its scope)")
+	}
+}
+
+func TestAnalyze_PhaseConfigAliasOnly_FallsBackToUnion(t *testing.T) {
+	plan, err := Analyze([]string{"testdata/multisite/deploy.yaml"}, Options{
+		PhaseConfig:      "/nonexistent/deploy.prereq.yaml",
+		PhaseConfigAlias: "deploy.prereq.yaml",
+	})
+	if err != nil {
+		t.Fatalf("Analyze: %v", err)
+	}
+	if plan.Phases[0].Scoped {
+		t.Errorf("alias-only/unloadable phase config must fall back (Scoped=false)")
+	}
+	if !containsSubstr(plan.Warnings, "per-phase secret scoping unavailable") {
+		t.Errorf("expected an unscopable warning; got %v", plan.Warnings)
+	}
+}
+
+func TestDeriveMigrations_SingleEnvDerived(t *testing.T) {
+	cfg, err := config.LoadFromFile("testdata/migrations-one-env.yaml")
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	m := deriveMigrations(cfg)
+	if m == nil || m.Env != "prod" {
+		t.Fatalf("expected Env=prod, got %+v", m)
+	}
+}
+
+func TestDeriveMigrations_TwoEnvsAmbiguous(t *testing.T) {
+	cfg, err := config.LoadFromFile("testdata/migrations-two-envs.yaml")
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	m := deriveMigrations(cfg)
+	if m == nil || m.Env != "" {
+		t.Fatalf("expected Env empty (ambiguous), got %+v", m)
+	}
+	w := deriveWarnings(cfg, m, deriveSecrets(cfg, m))
+	if !containsSubstr(w, "migrations environment ambiguous") {
+		t.Errorf("expected ambiguity warning; got %v", w)
+	}
+}
+
+// test helpers
+func hasSecret(s []SecretRef, name string) bool {
+	for _, r := range s {
+		if r.Name == name {
+			return true
+		}
+	}
+	return false
+}
+func names(s []SecretRef) []string {
+	out := make([]string, 0, len(s))
+	for _, r := range s {
+		out = append(out, r.Name)
+	}
+	return out
+}
+func containsSubstr(ss []string, sub string) bool {
+	for _, s := range ss {
+		if strings.Contains(s, sub) {
+			return true
+		}
+	}
+	return false
+}

cigen/multisite_evidence_test.go

@@ -0,0 +1,43 @@
+package cigen
+
+import (
+	"os"
+	"strings"
+	"testing"
+
+	"gopkg.in/yaml.v3"
+)
+
+// TestMultisiteEvidence_Honest locks the committed demonstration-fidelity
+// artifact. It reads the on-disk generated-infra.yml (NOT a freshly rendered
+// plan) and asserts exactly the honest claims in GAP.md: apply-prereq is
+// scoped (no deploy-only DB secret), apply-deploy keeps it, and the migrations
+// step carries --format json but NOT --env (multisite declares no environments).
+func TestMultisiteEvidence_Honest(t *testing.T) {
+	b, err := os.ReadFile("testdata/multisite/generated-infra.yml")
+	if err != nil {
+		t.Fatalf("read committed evidence: %v", err)
+	}
+	yml := string(b)
+
+	var doc struct {
+		Jobs map[string]struct {
+			Env map[string]any `yaml:"env"`
+		} `yaml:"jobs"`
+	}
+	if err := yaml.Unmarshal(b, &doc); err != nil {
+		t.Fatalf("committed evidence is not valid YAML: %v", err)
+	}
+	if _, ok := doc.Jobs["apply-prereq"].Env["MULTISITE_DB_URL"]; ok {
+		t.Errorf("apply-prereq must NOT carry the deploy-only MULTISITE_DB_URL (scoping gap #3)")
+	}
+	if _, ok := doc.Jobs["apply-deploy"].Env["MULTISITE_DB_URL"]; !ok {
+		t.Errorf("apply-deploy must carry MULTISITE_DB_URL")
+	}
+	if !strings.Contains(yml, "wfctl migrations up --config 'deploy.yaml' --format json") {
+		t.Errorf("migrations step must emit --format json (gap #4)")
+	}
+	if strings.Contains(yml, "migrations up") && strings.Contains(yml, "--env ") {
+		t.Errorf("multisite declares no ci.migrations.environments → --env must NOT be emitted (honest, design C1)")
+	}
+}

cigen/plan.go

@@ -55,6 +55,14 @@ type DeployPhase struct {
 	ConfigPath string `json:"config_path"`
 	// Include is an optional list of module names to include in this phase.
 	Include []string `json:"include,omitempty"`
+	// Secrets is the set of secrets this phase's apply job needs. Populated
+	// only when Scoped is true; otherwise the renderer uses CIPlan.Secrets.
+	Secrets []SecretRef `json:"secrets,omitempty"`
+	// Scoped is true when per-phase secret derivation ran against a real,
+	// loaded config for this phase. The renderer branches its env: source on
+	// this flag — NOT on len(Secrets) — so a genuinely zero-secret scoped
+	// phase emits no union, while an unscoped phase falls back to the union.
+	Scoped bool `json:"scoped,omitempty"`
 }
 
 // MigrationsSpec describes the database migration step.

cigen/plan_test.go

@@ -0,0 +1,31 @@
+package cigen
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+func TestDeployPhase_ScopedSecretsJSON(t *testing.T) {
+	p := DeployPhase{
+		Name:       "prereq",
+		ConfigPath: "deploy.prereq.yaml",
+		Secrets:    []SecretRef{{Name: "DIGITALOCEAN_TOKEN"}},
+		Scoped:     true,
+	}
+	b, err := json.Marshal(p)
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+	var got DeployPhase
+	if err := json.Unmarshal(b, &got); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if !got.Scoped || len(got.Secrets) != 1 || got.Secrets[0].Name != "DIGITALOCEAN_TOKEN" {
+		t.Fatalf("round-trip lost fields: %+v", got)
+	}
+	// Unscoped phase with no secrets must omit both in JSON (additive, back-compat).
+	b2, _ := json.Marshal(DeployPhase{Name: "deploy", ConfigPath: "deploy.yaml"})
+	if string(b2) != `{"name":"deploy","config_path":"deploy.yaml"}` {
+		t.Fatalf("unexpected JSON for unscoped phase: %s", b2)
+	}
+}

cigen/render_gha.go

@@ -174,11 +174,16 @@ func writeApplyJob(b *strings.Builder, jobName string, phase DeployPhase, needs
 	}
 	fmt.Fprintf(b, "    runs-on: %s\n", runner)
 
-	// Secrets env block
-	if len(p.Secrets) > 0 {
+	// Secrets env block. Branch the SOURCE on phase.Scoped (NOT len): a scoped
+	// phase uses its own subset (possibly empty → no env block); an unscoped
+	// phase falls back to the plan-wide union.
+	secrets := p.Secrets
+	if phase.Scoped {
+		secrets = phase.Secrets
+	}
+	if len(secrets) > 0 {
 		b.WriteString("    env:\n")
-		for _, s := range p.Secrets {
-			// Use ${{ secrets.NAME }} — use raw string to avoid template interpretation
+		for _, s := range secrets {
 			fmt.Fprintf(b, "      %s: ${{ secrets.%s }}\n", s.Name, s.Name)
 		}
 	}
@@ -209,8 +214,7 @@ func writeApplyJob(b *strings.Builder, jobName string, phase DeployPhase, needs
 	// the real migration runner — `wfctl ci run --phase migrate` is NOT a valid
 	// phase (ci run only accepts build|test|deploy) and would fail at runtime.
 	// No step-level `env:` is needed: deriveSecrets always adds the migrations
-	// DBEnv to the secrets union, so it is already in the apply job's job-level
-	// `env:` block above; re-declaring it here would be redundant.
+	// DBEnv to the last phase's env block above; re-declaring it here would be redundant.
 	isLastPhase := phase.Name == p.Phases[len(p.Phases)-1].Name
 	if isLastPhase && p.Migrations != nil {
 		b.WriteString("      - name: Run migrations\n")
@@ -221,14 +225,15 @@ func writeApplyJob(b *strings.Builder, jobName string, phase DeployPhase, needs
 	fmt.Fprintf(b, "        run: wfctl infra apply --config '%s' --auto-approve\n", phase.ConfigPath)
 }
 
-// migrationsUpCommand builds the `wfctl migrations up` invocation. The deploy
-// env is included as `--env <env>` only when MigrationsSpec.Env is set;
-// otherwise it is omitted (the command defaults to the empty environment).
+// migrationsUpCommand builds the `wfctl migrations up` invocation. `--env <env>`
+// is included only when MigrationsSpec.Env is set; `--format json` is always
+// appended (machine-readable output; matches the deployed multisite workflow).
 func migrationsUpCommand(configPath, env string) string {
 	cmd := fmt.Sprintf("wfctl migrations up --config '%s'", configPath)
 	if env != "" {
 		cmd += fmt.Sprintf(" --env %s", env)
 	}
+	cmd += " --format json"
 	return cmd
 }