feat: Add new deployment check option

[tylbees]

No description provided.

[linear]

SERV-52 Add deployment confirmation prompt via `--check` or `FAL_DEPLOY_CHECK=true` or admin config

[efiop]

@claude review

[efiop]

@claude review

[efiop]

@claude also pls double check that we are not breaking backward compat for public API in ‎projects/fal/src/fal/api . Like deploy() etc.

[tylbees]

@claude review

[tylbees]

@claude review

[efiop]

@claude review

[claude]

Claude finished @efiop's task in 2m 8s —— View job

---

Code Review

• Read CLAUDE.md for repo-specific guidelines
• Read changed files and git diff
• Compare against previous review findings
• Analyze code for bugs, security issues, and best practices
• Post review feedback

---

Fixed since previous review

• ✅ DeploymentCheckSummary.source is now rendered — _render_deployment_check_summary now emits "Triggered by: {source}" (deploy_check.py:463), and test_render_deployment_check_summary_includes_triggered_by confirms it. The last review incorrectly marked this as still open — this was fixed.

---

Nit: source values are displayed as raw literals

deploy_check.py:463:

console.print(f"[bold]Triggered by:[/bold] {summary.source}")

This renders as Triggered by: flag, Triggered by: env, or Triggered by: admin. The type values are fine for developers but could be more readable for end users:

Current	Suggested
flag	--check flag
env	FAL_DEPLOY_CHECK env var
admin	admin policy

A small display-name mapping would fix this without touching the type.

---

Minor: _effective_scale_config double computation (still open since Apr 27)

deploy_check.py:270-271:

desired_config = _desired_scale_config(prepared)
effective_config = _effective_scale_config(prepared, production_alias)

_effective_scale_config (deploy_check.py:376) internally calls _desired_scale_config(prepared) itself, so _desired_scale_config runs twice per _build_deployment_check_summary call. Harmless, but _effective_scale_config could accept a pre-computed desired_config arg to skip the redundancy.

---

Minor: "After Deploy" column misleading for --reset-scale-required rows (still open since Apr 21)

deploy_check.py:310-321: When field in INHERITED_SCALE_FIELDS and reset_scale=False and desired_value != production_value, the row shows desired_value (the code value) in the "After Deploy" column, colored yellow, with the note "Code value will not apply without --reset-scale". The value that will actually be in production after deploy is production_value, not desired_value. The column header "After Deploy" implies the opposite. The yellow style and note mitigate this, but the label is still misleading — the column for these rows is displaying the code value, not the deployed value.

---

Nit: _admin_requires_deploy_check fail-open/fail-closed inconsistency

deploy_check.py:164-180:

• Network exceptions / parse errors → return True (fail-closed ✅, fixed in last round)
• Non-OK HTTP status (e.g. 500) → return False (fail-open)

A transient 500 from the server would silently skip the admin-required check. Consider treating 5xx responses the same as network exceptions (return True), while keeping return False only for 4xx (which suggest the user has no admin-level policy rather than a server fault).

---

Nit: Double get_app_data_from_toml lookup in non-check path (still open since Apr 21)

When app_ref is an app name, _resolve_team_and_app_ref (cli/deploy.py:49) reads the toml once for team resolution, then _resolve_deployment_reference (api/deploy.py:136-145) reads it again. Harmless but redundant.

---

Summary of open items

Issue	Severity	Status
source displayed as raw literal in "Triggered by:"	Nit	🆕 New
_admin_requires_deploy_check inconsistent fail behavior for 5xx	Nit	🆕 New
_effective_scale_config double computation	Minor	Still open (since Apr 27)
"After Deploy" column misleading for --reset-scale-required rows	Minor	Still open (since Apr 21)
Double get_app_data_from_toml lookup (non-check path)	Nit	Still open (since Apr 21)

The last medium issue ("source not rendered") is now closed. Remaining items are all minor/nit — this is otherwise ready to merge.