Skip to content

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

Open
mattmillerai wants to merge 2 commits into
mainfrom
matt/be-4103-generate-spend-gate
Open

feat(generate): spend-gate — consent before credit spend (BE-4103)#577
mattmillerai wants to merge 2 commits into
mainfrom
matt/be-4103-generate-spend-gate

Conversation

@mattmillerai

Copy link
Copy Markdown
Collaborator

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.

…alls (BE-4103)

A generation call spends Comfy credits on invocation with no confirmation.
Put the proxy call behind a consent interlock, ahead of every network side
effect (auth refresh, asset uploads, the generation request):

- interactive TTY: confirm prompt (default No) before spending
- --yes flag bypasses (automation / evals)
- spend.auto_confirm config persists always-proceed; new
  `comfy generate consent [show|always|ask]` action manages it
- --json / non-TTY with neither: fail closed with
  spend_consent_required (exit 1, nothing spent) — never hang on a
  prompt no machine caller can answer

Ungated: list/schema/refresh/upload/resume/--emit-workflow (spend nothing).
@mattmillerai mattmillerai added the agent-coded PR authored by the agent-work loop label Jul 23, 2026
@mattmillerai
mattmillerai marked this pull request as ready for review July 23, 2026 00:55
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

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.

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Jul 23, 2026
@mattmillerai mattmillerai added the cursor-review Request Cursor bot review label Jul 23, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-coded PR authored by the agent-work loop cursor-review Request Cursor bot review enhancement New feature or request size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants