feat(#5631)!: make --preview accept alias for real Wrangler preview upload

[fullsend-ai-coder]

Summary

Makes mint deploy --platform=cloudflare --preview=<alias> perform a real Wrangler preview upload instead of a production deploy. The --preview flag changes from a bare boolean to a string that accepts a preview alias, which is passed through to Wrangler as --preview-alias.

Related Issue

Closes #5631

Changes

• --preview flag: Changed from BoolVar to StringVar. Value is the preview alias (e.g. --preview=bt-run-42)
• Preview deploy path: Uses wrangler versions upload --preview-alias=<alias> instead of wrangler deploy, so the durable Worker script is not affected
• Preview URL: Deterministic from alias + worker name: https://<alias>-<worker-name>.workers.dev — callers can compute this without scraping Wrangler output
• Preview teardown: Abandons the preview alias (no-op) instead of deleting the durable Worker script via wrangler delete
• WranglerRunner.Deploy interface: preview bool → previewAlias string (empty = durable, non-empty = preview)
• Validation: Added ValidatePreviewAlias() with same rules as Worker names (2-63 lowercase alphanumeric/hyphens)
• Help text: Documents the alias → preview URL derivation for callers (BT, operators)
• Backward compat: Legacy bare-preview teardown (DeployPreview without alias) still falls back to wrangler delete

Testing

• All existing CF dispatch and CLI mint tests pass with race detector
• New tests for preview-alias deploy, preview-alias teardown (no delete), alias validation, invalid alias rejection, dry-run preview output
• go vet passes on changed packages

Note: pre-commit could not run in sandbox due to network restrictions (git fetch blocked). The post-script runs authoritative pre-commit on the runner.

---

Closes #5631

Post-script verification

• Branch is not main/master (agent/5631-cf-preview-alias)
• Secret scan passed (gitleaks — 6d3cb1f2fe81fe4c3a315d5234f0624b19281620..HEAD)
• PR body secret scan passed (gitleaks — no-git)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:37 AM UTC · Completed 11:52 AM UTC
Commit: f050104 · View workflow run →

[codecov]

Codecov Report

❌ Patch coverage is 94.44444% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/dispatch/cf/provisioner.go	85.71%	3 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

All prior findings from the previous review have been addressed:

• [logic-error] (medium) — CLI help text and doc comment URL pattern now correctly omit .<subdomain> ✓
• [logic-error] (low) — Legacy bare-preview teardown path removed entirely; validate() now enforces PreviewAlias is non-empty for DeployPreview ✓
• [edge-case] (low) — parseWorkerURL removed from deployPreview; now uses only deterministic URL pattern ✓

Findings

Low

• [logic-error] internal/dispatch/cf/provisioner.go:240 — validate() guards DeployPreview with empty PreviewAlias but not the inverse: DeployDurable with non-empty PreviewAlias. Provision routes on PreviewAlias (non-empty → deployPreview) while Teardown routes on DeployMode, so this mismatch would cause a preview deploy with a non-teardownable config. Not reachable from the CLI, but the public Config API permits it. Consider adding: if p.cfg.DeployMode != DeployPreview && p.cfg.PreviewAlias != "" { return error }.

• [logic-error] internal/dispatch/cf/provisioner.go:468 — deployPreview always uses the deterministic URL pattern https://<alias>-<worker>.workers.dev without parsing wrangler output, while deployDurable parses the actual URL first. If the Cloudflare account has a configured workers.dev subdomain, the preview URL would omit it. Pre-existing pattern — the durable deploy fallback uses the same no-subdomain format — and explicitly documented as a design choice to avoid parseWorkerURL false positives.

Previous run

Review

All prior findings from the previous review have been addressed:

• [breaking-change-unmarked] (critical) — PR title now includes ! suffix ✓
• [outdated-flag-syntax] (high) — docs/cli/mint.md updated with correct --preview=<alias> syntax ✓
• [secret-exposure] (medium) — deployPreview() now passes --keep-vars ✓
• [logic-error] (low) — validate() now guards against DeployPreview with empty PreviewAlias ✓
• [missing-test] (low) — CLI-level fake now captures and asserts previewAlias ✓

Findings

Medium

• [logic-error] internal/cli/mint.go:414 — The CLI help text states the preview URL pattern is https://<alias>-<worker-name>.<subdomain>.workers.dev (with a <subdomain> component), but the actual URL constructed by the code uses https://<alias>-<worker-name>.workers.dev (no subdomain). This mismatch appears in two places: the CLI long help (line 414) and the Config.PreviewAlias doc comment (provisioner.go:95). The code (dry-run output at line 646, fallback URL at provisioner.go:480), the --preview flag description, and docs/cli/mint.md all correctly omit the subdomain. Users relying on the help text to compute preview URLs will construct incorrect URLs.
  Remediation: Remove .<subdomain> from the URL pattern in: (1) CLI help text at mint.go:414, (2) Config.PreviewAlias doc comment at provisioner.go:95. If Cloudflare actually uses a subdomain component, update the code and docs to include it consistently.

Low

• [logic-error] internal/dispatch/cf/provisioner.go:229 — The Teardown legacy path (DeployPreview with empty PreviewAlias → wrangler delete) is unreachable through normal Provisioner lifecycle: validate() at line 248 now rejects DeployPreview with empty PreviewAlias. The test TestProvisioner_Teardown_PreviewLegacyBareFlag exercises this path only by bypassing validate() (since Teardown does not call it). This is a design inconsistency — either remove the legacy path to match validate() constraint, or document why it must remain for backward compatibility with Provisioners created before this change.

• [edge-case] internal/dispatch/cf/provisioner.go:478 — parseWorkerURL in deployPreview matches the first line containing workers.dev and https://, which could return the production Worker URL instead of the preview URL if wrangler versions upload outputs both. The deterministic fallback URL (line 480) is correct, but a parseWorkerURL false-positive match would prevent it from being used.

Previous run (2)

Review

Findings

Critical

• [breaking-change-unmarked] PR title — The --preview flag changed from BoolVar (bare --preview was valid) to StringVar (now requires --preview=<alias>). This is a breaking change per COMMITS.md: previously accepted input (--preview without a value) is now rejected. The PR title must carry the ! suffix so GoReleaser surfaces the break in release notes. COMMITS.md states this omission "has caused real incidents."
  Remediation: Change PR title to feat(#5631)!: make --preview accept alias for real Wrangler preview upload and add a BREAKING CHANGE: trailer to the commit message explaining the migration path (e.g., "Replace bare --preview with --preview=<alias> where <alias> is a 2-63 char lowercase alphanumeric/hyphen string").

High

• [outdated-flag-syntax] docs/cli/mint.md:59,77 — The standalone CLI docs still describe --preview as a boolean flag. Line 59 says "Use --preview for ephemeral test deploys" and the flags table at line 77 shows default false. Both are now incorrect — --preview requires an alias value and defaults to empty string. A user following these docs will get a parse error.
  Remediation: Update line 59 to document --preview=<alias> syntax and explain it runs wrangler versions upload. Update the flags table: change default from false to "" and description to match the new behavior.

Medium

• [secret-exposure] internal/dispatch/cf/provisioner.go:444 — The new deployPreview() method omits --keep-vars from the wrangler versions upload command, while deployDurable() explicitly passes it to preserve Worker secrets (PEM keys stored via StoreAgentPEM). Since preview-alias deploys target the same Worker script as production, if wrangler versions upload shares the same binding-overwrite behavior as wrangler deploy, this could wipe PEM secrets on preview deploys.
  Remediation: Verify whether wrangler versions upload overwrites bindings without --keep-vars. If it does, add --keep-vars. If it does not (versions upload is additive to bindings), add a code comment explaining why --keep-vars is intentionally omitted.

Low

• [logic-error] internal/dispatch/cf/provisioner.go:184 — Provision() now routes deploy behavior based on PreviewAlias (empty → durable, non-empty → preview), while Teardown() routes based on DeployMode. A Config{DeployMode: DeployPreview, PreviewAlias: ""} would cause Provision to do a durable deploy but Teardown to delete the Worker. This state is unreachable through the CLI (which gates DeployPreview on non-empty alias), but a future direct consumer of NewProvisioner could hit it.
  Remediation: Add a defensive validation: if p.cfg.DeployMode == DeployPreview && p.cfg.PreviewAlias == "" { return error }. This future-proofs the API.

• [missing-test] internal/cli/mint_test.go:55 — The CLI-level fake (fakeCFWranglerRunner.Deploy) discards the previewAlias parameter (_ string). TestMintDeployCmd_CloudflarePreviewDeploy passes --preview=bt-run-42 but never asserts the alias reached the runner. The provisioner-level tests do cover this, but the CLI integration path has a gap.
  Remediation: Capture previewAlias in fakeCFDeployCall and assert fake.deployCalls[0].previewAlias == "bt-run-42" in the preview deploy test.

---

Labels: PR modifies mint deploy CLI and Cloudflare dispatch provisioner (Go code)

[ifireball]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 12:01 PM UTC · Completed 12:11 PM UTC
Commit: f050104 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed all 5 review findings: updated docs for new --preview= syntax, added --keep-vars to preview deploys for secret safety, added defensive validation against DeployPreview without alias, and captured previewAlias in CLI test fake with assertion. PR title ! suffix requires manual update (sandbox limitation).

Fixed (5):

1. [breaking-change-unmarked] PR title missing ! suffix for breaking change: Added BREAKING CHANGE: trailer to the commit message explaining the migration path. PR title update to add ! suffix requires manual action (sandbox blocks GraphQL mutations).
2. [outdated-flag-syntax] docs/cli/mint.md still describes --preview as boolean (docs/cli/mint.md): Updated line 59 to document --preview= syntax with wrangler versions upload explanation. Updated flags table: changed default from false to "" and description to match new behavior.
3. [secret-exposure] deployPreview() omits --keep-vars (internal/dispatch/cf/provisioner.go): Added --keep-vars to deployPreview() for parity with deployDurable(). Since preview-alias deploys target the same Worker script as production, omitting it could wipe PEM secrets stored via StoreAgentPEM.
4. [logic-error] Provision routes on PreviewAlias but Teardown routes on DeployMode (internal/dispatch/cf/provisioner.go): Added defensive validation in validate(): DeployPreview now requires a non-empty PreviewAlias. This prevents the mismatch where Provision would do a durable deploy but Teardown would delete the Worker. Legacy bare-preview teardown path is preserved since Teardown does not call validate().
5. [missing-test] CLI fake discards previewAlias parameter (internal/cli/mint_test.go): Captured previewAlias in fakeCFDeployCall struct and Deploy method. Updated TestMintDeployCmd_CloudflarePreviewDeploy to assert fake.deployCalls[0].previewAlias == "bt-run-42".

Tests: passed

Decision points

• PR title update requires ! suffix but sandbox blocks gh pr edit (GraphQL mutations) (alternatives: Update PR title via gh pr edit, Document for manual update; rationale: Sandbox network policy blocks GraphQL API. Added BREAKING CHANGE trailer to commit message; PR title update noted for manual action.)

_{Updated by fullsend fix agent}

[github-actions]

Site preview

Preview: https://f623cff2-site.fullsend-ai.workers.dev

Commit: 9cdf235bf9b7749c11f5a7095b9a44f914d014d7

[ifireball]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:58 PM UTC · Completed 8:14 PM UTC
Commit: 7f143a5 · View workflow run →

Superseded by updated review

[ifireball]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 8:53 AM UTC · Completed 9:03 AM UTC
Commit: 7f143a5 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 2 (human-triggered)

Addressed all 3 review findings: fixed URL pattern mismatch by removing . from help text and doc comment, removed unreachable legacy Teardown path and its tests, and eliminated parseWorkerURL false-positive risk in deployPreview by using deterministic URL directly.

Fixed (3):

1. [logic-error] CLI help text and doc comment have in preview URL pattern (internal/cli/mint.go): Removed . from the URL pattern in CLI help text (line 414) and Config.PreviewAlias doc comment (provisioner.go:95) to match the actual URL constructed by the code: https://-.workers.dev
2. [logic-error] Teardown legacy path unreachable due to validate() guard (internal/dispatch/cf/provisioner.go): Removed the unreachable legacy Teardown path (DeployPreview with empty PreviewAlias -> wrangler delete) since validate() now enforces non-empty PreviewAlias for DeployPreview. Updated Teardown doc comment to note the validate() constraint. Removed two tests (TestProvisioner_Teardown_PreviewLegacyBareFlag, TestProvisioner_Teardown_LegacyDeleteError) that bypassed validate() to exercise the now-removed dead code.
3. [edge-case] parseWorkerURL in deployPreview could match production URL (internal/dispatch/cf/provisioner.go): Replaced the parseWorkerURL-with-fallback pattern in deployPreview() with the deterministic URL directly (https://-.workers.dev). parseWorkerURL could match the production Worker URL from wrangler output as a false positive, preventing the correct deterministic fallback from being used.

Tests: passed

Decision points

• Chose to remove the legacy Teardown path entirely rather than just documenting the inconsistency (alternatives: Document why legacy path must remain for backward compatibility, Remove the validate() guard to keep the legacy path reachable; rationale: validate() already makes the legacy path unreachable through normal Provisioner lifecycle, making it dead code. Removing it eliminates the design inconsistency the reviewer flagged. The validate() guard is the correct constraint — DeployPreview without PreviewAlias would cause a durable deploy followed by a destructive teardown.)

_{Updated by fullsend fix agent}

[ifireball]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:51 AM UTC · Completed 10:12 AM UTC
Commit: 553cfe2 · View workflow run →

[ifireball]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 10:41 AM UTC · Completed 10:49 AM UTC
Commit: 553cfe2 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 3 (human-triggered)

Fixed 1 of 2 review findings. Added symmetric validate() guard for DeployDurable with non-empty PreviewAlias (prevents unteardownable preview configs). Disagreed with 1 finding (deterministic URL pattern is a documented design choice acknowledged by the reviewer).

Fixed (1):

1. validate() missing guard for DeployDurable with non-empty PreviewAlias (internal/dispatch/cf/provisioner.go): Added symmetric validation guard in validate(): reject Config where DeployMode is not DeployPreview but PreviewAlias is non-empty. This prevents Provision from routing to deployPreview (via non-empty PreviewAlias) while Teardown rejects the operation (via DeployDurable mode), creating an unteardownable preview config. Added corresponding test TestProvisioner_Provision_DurableWithPreviewAliasRejected.

Disagreed (1):

1. deployPreview uses deterministic URL pattern without parsing wrangler output: The review itself acknowledges this is a pre-existing pattern and explicitly documented as a design choice to avoid parseWorkerURL false positives. The durable deploy fallback uses the same no-subdomain format. No code change needed.

Tests: passed

_{Updated by fullsend fix agent}

[rh-hemartin]

Makes sense to me so we can kind of control where it goes and decide some prefixes for the environments.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 6:07 AM UTC · Completed 7:04 AM UTC
Commit: 9cdf235 · View workflow run →

[fullsend-ai-retro]

Retro: PR #5639 — feat(#5631)!: make --preview accept alias for real Wrangler preview upload

Workflow shape: Triage → Code → Review1 → Fix1 → Review2 → Fix2 → Review3 → Fix3 → Human approvals → Merge

Timeline

Time (UTC)	Event
Jul 27, 10:09	Issue #5631 created by ifireball
Jul 27, 10:10–10:15	Triage agent ran successfully
Jul 27, 11:19–11:35	Code agent created PR #5639 (16 min)
Jul 27, 11:36–11:52	Review agent iteration 1: 5 findings (1 critical, 1 high, 1 medium, 2 low). Submitted DISMISSED review
Jul 27, 11:52	Auto-fix run 30263592039 failed — misidentified bot-authored PR as human-authored
Jul 27, 11:59	ifireball manually triggered /fs-fix
Jul 27, 11:59–12:12	Fix agent iteration 1: addressed all 5 findings
Jul 27, 19:57	ifireball manually triggered /fs-review (7.75-hour gap)
Jul 27, 19:57–20:14	Review agent iteration 2: confirmed 5 prior findings addressed, raised 3 new (1 medium, 2 low). Submitted COMMENTED review
Jul 28, 08:51	ifireball manually triggered /fs-fix
Jul 28, 08:51–09:03	Fix agent iteration 2: addressed 3 findings
Jul 28, 08:53	ifireball APPROVED
Jul 28, 09:49	ifireball manually triggered /fs-review
Jul 28, 09:49–10:12	Review agent iteration 3: confirmed 3 prior addressed, raised 2 low findings. APPROVED
Jul 28, 10:39	ifireball manually triggered /fs-fix for remaining low finding
Jul 28, 10:39–10:49	Fix agent iteration 3: fixed 1, correctly disagreed with 1
Jul 28, 11:16	ifireball APPROVED, enabled auto-merge
Jul 28, 11:30	rh-hemartin APPROVED
Aug 3, 05:56	ifireball manually added to merge queue (6-day gap — see proposal)
Aug 3, 06:05	Merged

What went well

Review agent quality was excellent. Across 3 iterations, the agent raised 10 findings: 9 true positives (90%) and 0 false positives. The single borderline finding (deterministic URL pattern) was self-acknowledged as a documented design choice, and the fix agent correctly disagreed. High-value catches included a medium-severity secret-exposure risk (--keep-vars omission that could wipe PEM secrets) and a critical breaking-change-unmarked PR title. Severity calibration was appropriate throughout.

Fix agent showed good judgment. Addressed 9 of 9 actionable findings reliably and correctly pushed back on 1 finding with sound reasoning. All fix iterations passed tests including the race detector.

Human review delta was zero. Neither human reviewer (ifireball, rh-hemartin) identified issues the review agent missed. All 10 substantive code review findings came from the agent.

Evidence for existing open issues

• #5536 / #1569 (bot detection): Auto-fix run 30263592039 failed because gh pr view --json author returned app/fullsend-ai-coder (no [bot] suffix) and the eligibility regex [bot]$ didn't match, treating the bot-authored PR as human-authored. This required ifireball to manually trigger /fs-fix for every iteration.
• #5666 (auto re-review after fix): There was a 7.75-hour gap between fix iteration 1 completing (Jul 27 12:12 UTC) and ifireball manually triggering /fs-review (Jul 27 19:57 UTC). Automated re-review dispatch after fix pushes would have eliminated this idle time.
• agents#421 / agents#563 (breaking-change detection): The code agent produced a breaking change (CLI flag type change) without the ! suffix in the PR title, despite AGENTS.md and COMMITS.md explicitly requiring it. This was the review agent's critical finding.
• #1088 (code agent self-review loop): 3 of 5 initial findings were avoidable with context the repo already provides — the breaking-change marker, the outdated docs, and the missing test assertion. A pre-PR self-review would likely have reduced iterations from 3 to 1.
• agents#543 (cascading findings): Fix iteration 1 added a validate() guard, which made the legacy teardown path dead code — caught in iteration 2. This cascading pattern is inherent to iterative fixing.
• agents#380 (symmetric defensive patterns): The code agent created deployPreview() as a sibling of deployDurable() but dropped the --keep-vars safety flag. The sibling method's argument list was visible in the same file.
• #1582 (first-pass completeness): Review iteration 1 missed the URL pattern mismatch and parseWorkerURL risk that were present in the original commit; these were only found in iteration 2.

Proposals filed

• Auto-merge cannot enqueue PRs on merge-queue-enabled branches with strict status checks (in fullsend-ai/fullsend)