feat(generate): spend-gate — consent before credit spend (BE-4103)

[mattmillerai]

ELI-5

comfy generate flux-pro --prompt "a cat" costs real money (Comfy credits) the moment you run it — and until now it never asked. This PR makes it ask first. At a terminal you get a "this spends credits — proceed?" prompt; scripts and agents must pass --yes (or the user can persist "always proceed" once with comfy generate consent always); and a machine call (--json or no terminal) with neither errors out cleanly instead of spending silently or hanging on a question nobody can answer.

What

The money interlock for comfy generate (launch blocker for exposing generation through the local MCP, parent BE-4102):

• Interactive confirm — TTY runs prompt (default No) before any credit-spending proxy call.
• --yes — explicit bypass for automation/evals (new meta flag, stripped like --async/--json).
• spend.auto_confirm config — persistent always-proceed, managed by a new comfy generate consent [show|always|ask] action; stored in config.ini via ConfigManager (see judgment calls).
• Fail-closed non-interactive — --json or no stdin TTY with neither consent source → error spend_consent_required (registered in error_codes.REGISTRY), exit 1, nothing spent, never a hanging prompt.

The gate sits in _generate before every network side effect — before OAuth refresh (resolve_api_key), before asset-upload transforms, before the generation request. Ungated because they spend nothing: list / schema / refresh / upload / resume (polls an already-paid job) / --emit-workflow (local artifact, no proxy call).

Consent failures emit generate:error with error_kind="consent"; the consent action emits generate:consent. Docs updated: top help, per-model schema help, and the agent-facing skills/comfy/SKILL.md (explicitly tells agents not to reflexively add --yes — it must represent real human approval).

Acceptance criteria

• comfy generate <model> --json with no consent → errors, spends nothing ✅ (test + live CLI run: exits 1 with spend_consent_required before any network call)
• --yes or spend.auto_confirm=true → proceeds ✅ (tests for both, plus config-persistence-to-disk check)
• Interactive TTY → prompts before spending ✅ (accept / decline / bare-Enter-defaults-to-No tests)

Judgment calls

• spend.auto_confirm lives in config.ini (ConfigManager), not secrets.json. The ticket says "same persistence that backs comfy cloud set-key", which literally is the auth-record store (secrets.json, provider→key schema). A boolean preference doesn't fit that schema; config.ini is the CLI's canonical settings persistence (same durability, same directory). Easy to move if the literal store was intended.
• Added comfy generate consent [show|always|ask] — the ticket asks for the config but no setter; without one, users would hand-edit config.ini. Small (~40 lines), makes the acceptance path actually usable, and gives the fail-closed error a concrete remediation to name.
• Gate before auth resolution: an unauthenticated non-consenting machine caller gets the consent error, not the auth error, and no OAuth token refresh happens pre-consent. Everything before the gate is local-only.
• TTY check is stdin-only (prompt reads stdin). stdin=TTY, stdout=redirected will prompt into the redirect — consistent with every other prompt in this CLI (typer.confirm defaults); the no-input-TTY hang case fails closed.
• Breaking change by design: previously-working unattended comfy generate invocations now require --yes — that is the ticket. Fallback for garbage spend.auto_confirm values is fail-closed.
• Deny-path falsification: the fail-closed error is a consent interlock, not a capability denial — every deny message names the working paths (--yes, consent always), and each of those paths is empirically proven by tests + a live CLI run.

Tests

• New tests/comfy_cli/command/generate/test_spend_gate.py — 19 tests: fail-closed (json / pretty-non-TTY / async / pre-auth), bypasses (flag, config true/false/garbage), interactive prompt (accept/decline/default), ungated paths (emit-workflow, schema errors), lifecycle telemetry, and the consent action incl. on-disk persistence.
• New package conftest.py autouse fixture pre-authorizes the gate for the existing 177 generate tests (they exercise post-gate behavior; CliRunner has no TTY), overridden by name in the gate's own tests.
• Full suite: 2554 passed; the only failures are the 2 known test_validate_command reds from main (BE-3357×BE-3359, fix in test: fix repo-wide pytest failure — reconcile validate-command fixtures with the no-outputs check (BE-4097) #576). Ruff format clean; the 15 local ruff check UP038 hits are the known ruff-0.12.7-vs-CI-0.15.15 false flags in untouched files.

Safety note (from the ticket): human review required before merge — this is the money interlock. Do not auto-merge.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a582ef18-eaa9-4df8-837c-4f66a3cab839

📥 Commits

Reviewing files that changed from the base of the PR and between 18314a2 and 8a9dd4e.

📒 Files selected for processing (7)

• comfy_cli/command/generate/app.py
• comfy_cli/command/generate/schema.py
• comfy_cli/constants.py
• comfy_cli/error_codes.py
• comfy_cli/skills/comfy/SKILL.md
• tests/comfy_cli/command/generate/conftest.py
• tests/comfy_cli/command/generate/test_spend_gate.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-4103-generate-spend-gate

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-4103-generate-spend-gate

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

⚠️ Review failed

Judge call failed (status=parse_error): Could not parse JSON findings from output. First 500 chars:
Shell execution is blocked here, but the file contents plus static analysis let me adjudicate confidently. Key verifications:

- **Interpolation crash (real):** `ConfigManager.get_bool` → `configparser.getboolean`, which applies interpolation before boolean conversion. A `%` in the value raises `configparser.InterpolationError` (subclass of `configparser.Error`, **not** `ValueError`), so `except ValueError` misses it. Confirmed real; 3 reviewers across labs/types.
- **`--yes` fail-open (real):**

Re-trigger by removing and re-adding the cursor-review label.

[bigcat88]

Reviewed as the money interlock deserves — code, tests, and live CLI runs (none of which spent anything). Approving:

• Fail-closed verified live: comfy generate seedance --prompt x --json with no consent → exit 1 with spend_consent_required — and crucially the consent error, not the auth error, proving the gate sits before resolve_api_key and any network side effect.
• --yes verified live: passes the gate and dies at the auth step on an unauthenticated box — gate passage without spend.
• Persistence verified live: consent always → spend.auto_confirm = true in config.ini → the same json call then reaches auth; consent ask reverts. show is a clean read.
• Interactive default-No verified on a real TTY (via script): prompt renders, bare Enter → "Canceled — no credits were spent."
• Fail-closed details hold up: get_bool returns None when unset (→ False) and raises ValueError on garbage (→ caught → False); closed-stdin ValueError from isatty → no prompt possible → fail closed.
• The autouse conftest fixture is safe: the suite-wide _isolate_config_path keeps the cm.set("true") writes in tmp dirs, and the teardown handles the singleton's in-memory state.
• Mutation check: neutralizing _confirm_spend fails 11 of the 19 spend-gate tests — the interlock is pinned from every direction.
• Full unit suite: 2562 passed. Two test_generate_binary_response_* failures I hit turned out to be my own environment: a fresh ~/.comfy/openapi-cache.yml from live-testing #575 leaks into the test suite (_USER_CACHE has no test isolation — pre-existing, reproduces on main, disappears with the cache removed). Worth a small follow-up ticket: isolate the spec user-cache path in tests the way config/jobs-state already are.

Per the sweep protocol the merge decision stays with you — but this has had its human-directed empirical review.