Promote dev to main: five-branch CI, fail-closed selection, OIDC trust fix

[Atom-oh]

Promotes PR #20.

🤖 Generated with Claude Code

[github-actions]

🤖 AI Code Review (Claude Fable 5 chair · lens×model matrix)

_Cells (model/lens): codex/L2 codex/L3 codex/L4 codex/L5 _

Status: BLOCKED — CRITICAL/MAJOR issues must be resolved before merge

🛑 Forced FAIL due to coverage collapse: at most one vendor survived, so cross-verification across the lens x model matrix cannot happen — fail-closed regardless of the chair's judgment.

⚠️ Coverage degraded: model(s) [kiro-opus, kiro-gpt] had no response across every lens (invalid flag/missing binary/auth failure, etc.) — the review below was synthesized without them.

I verified the panel claims against the base workflows (terraform.yml, deploy-agentcore.yml) and the diff. Here is the synthesized review.

---

PR #21 Review — Promote dev to main: five-branch CI, fail-closed selection, OIDC trust fix

Summary

This PR replaces the expr && A || B ternary stack-selection pattern in three workflows with explicit per-branch case blocks that hard-fail on unknown branches or missing config, retires deploy-preview.yml in favor of three standing user branches deploying via deploy-web.yml, and corrects the documented OIDC trust model (jobs with environment: present the environment:<name> sub, not a branch ref — the base's docs had the dev deployer pinned to refs/heads/dev, which would have failed every environment-gated deploy). The fail-closed selection is a genuine improvement: previously, dispatching deploy-agentcore.yml or terraform.yml apply from an arbitrary branch would silently fall through to the dev stack's secrets; now it errors. No CRITICAL or MAJOR finding survived verification against the base.

Panel note: one model (codex) per lens, so no cross-model agreement signal is available; each finding was verified directly against the diff and base instead.

Issues per lens

L2 — Code correctness

Panel (codex): no findings. Chair concurs. I additionally verified the edge cases the new case blocks create:

• terraform.yml push/PR triggers remain branches: [main, dev] (unchanged by the diff), so the plan job's *) exit 1 arm is unreachable for user branches via push — user-branch PRs into dev resolve TARGET=dev correctly via github.base_ref.
• secrets[format('TF_BACKEND_HCL_PREVIEW_{0}', github.ref_name)] with lowercase branch names works — GitHub's secrets-context lookup is case-insensitive (same pattern the deleted preview workflow relied on).
• The relaxed deploy gate (github.ref_name != 'main') is safe because push triggers are limited to the five listed branches, and any other-branch workflow_dispatch hits the fail-closed *) arm.

L3 — Security / AWS mutation safety

Panel (codex) raised two MAJORs; chair downgrades both to MINOR after verification:

• MINOR (downgraded from panel MAJOR) — All stacks' backend secrets injected as job-level env before selection (deploy-web.yml, terraform.yml apply, deploy-agentcore.yml). Factually accurate: MAIN_BACKEND_B64/MAIN_TFVARS_B64 are present in dev/user-branch jobs' process env, visible to every step including third-party actions. However, this is not a privilege-boundary break: these are repo-level secrets already readable by any branch's workflow in this repo (the base design explicitly documents that they cannot be environment-scoped because ungated builds need them), and the actual security boundary is the OIDC role — a dev/user job holds only the dev-tier role, so possessing the prod backend.hcl/tfvars (config, not credentials) grants no prod mutation. It is a defense-in-depth/supply-chain-hygiene regression relative to the base's one-pair-per-job env, not a MAJOR.
• MINOR (downgraded from panel MAJOR) — User branches share the dev-tier deployer role spanning dev + all user stacks. Accurate and a real least-privilege widening versus the retired preview role (which was scoped to preview-only resources). But it is deliberate, documented in dev-repo-setup.md §2 ("dev + user stacks' ECS/ECR-pin/apply — never production"), confined to the dev tier, and grants nothing new in practice: the same writers can push to dev (the default branch; only main carries the protect ruleset) and assume the same role. Cross-user-stack interference among three trusted collaborators is an accepted tradeoff, not a gate-worthy defect. Production isolation (environment production sub + main-only branch policy) is unchanged.
• No ADR-005 concern: this PR mutates nothing in the app's AWS-autonomy surface; CI deploy roles are out of that ADR's scope. No hardcoded secrets introduced.

L4 — Observability / data-integration correctness

Panel (codex): no findings. Chair concurs — the diff touches no Steampipe, compliance, or AgentCore diagnosis logic. (deploy-agentcore.yml changes are credential/selection plumbing only; the make migrate → make agentcore ordering required by CLAUDE.md is preserved.)

• MINOR — Reproducibility gap in deploy-agentcore.yml (chair-added, adjacent to the diff): the new fail-closed selection correctly limits agentcore dispatch to main/dev, which means user stacks have no CI path for migrations/agent provisioning — this is fine, but it interacts with the L5 doc gap below.

L5 — Docs / ADR consistency

• MINOR (downgraded from panel MAJOR) — "Push = auto-deploy" overstates user-branch coverage. Verified: deploy-web.yml triggers only on web/** + CHANGELOG.md, and the new fail-closed arms in terraform.yml apply and deploy-agentcore.yml explicitly reject user branches — so user-stack infra/agent changes are local-make-only. The runbook's table does name deploy-web.yml as the trigger, and the provisioning steps are already local, so this is under-specification rather than a materially misleading claim. Worth one clarifying sentence ("auto-deploy covers the web image only; infra/agentcore for user stacks is applied locally"), not a gate.
• MINOR (panel agrees) — branch-strategy.md still titles the section "Per-user preview stacks" and the secrets keep the _PREVIEW_ naming while the preview tier/role/workflow is retired. The secret names staying put is pragmatic (avoids re-registration), but the heading should say "standing user stacks."
• Positive consistency note: the rewritten trust matrix in dev-repo-setup.md now matches the actual workflows (deployer roles trust environment subs; build roles trust branch refs; the retired preview role is explicitly called out) — this fixes a base-docs error rather than introducing one.

Suggestions

1. Scope the base64 secret env vars to the "Restore backend" step (step-level env:) or select them in a dedicated step that emits only the chosen pair, so third-party actions (buildx, ECR login, build-push) never see the other stacks' backend config in their environment.
2. Add one sentence to branch-strategy.md stating that user-branch auto-deploy covers the web image only, and that terraform apply / agentcore for user stacks are run locally (make deploy/make agentcore), since CI now hard-rejects those branches for both.
3. Rename the "Per-user preview stacks" heading to reflect the standing-user-stack model; optionally note that the TF_*_PREVIEW_<USER> secret names are retained for continuity.
4. Longer term, if user-stack isolation matters more than convenience, per-user deployer roles (trusting per-branch subs or per-user environments) would restore the least-privilege property the preview role had — cheap to add later since selection is already an explicit case.

Verdict

The fail-closed selection and OIDC trust correction are real fixes to silent-fallthrough and would-be-broken-deploy behavior in the base. The panel's two L3 MAJORs are factually grounded but overstate severity: neither crosses a privilege boundary that wasn't already crossable, both are documented deliberate tradeoffs confined to the dev tier, and production isolation is strengthened, not weakened. The L5 MAJOR is a doc under-specification, not an inconsistency that misleads into breakage. All retained findings are MINOR.

---

Triggered by commit 2c0f70a9127b339176a5b243084f6b57e621f11e · workflow: .github/workflows/pr-review.yml