feat(run): gate comfy run on paid partner nodes via --allow-spend (BE-4326)#591
feat(run): gate comfy run on paid partner nodes via --allow-spend (BE-4326)#591mattmillerai wants to merge 2 commits into
comfy run on paid partner nodes via --allow-spend (BE-4326)#591Conversation
…BE-4326) `comfy run` silently spent Comfy credits when a workflow embedded partner-API (paid) nodes and a credential resolved. `comfy generate` and `comfy run-template` both gate on spend consent; `comfy run` was the hole. Add an `--allow-spend` flag (default False) and a shared `_spend_gate` consent interlock mirroring run-template's gate (BE-4113): - Pretty + TTY: warn naming the nodes, confirm (default No); decline → `spend_consent_required` + exit 1. - JSON / non-TTY: fail closed with `spend_consent_required`, telling the caller to re-run with `--allow-spend`; nothing submitted, nothing spent. The gate fires BEFORE partner-credential resolution / cloud auth so a refusal never triggers a network OAuth refresh. Detection stays fail-open (empty object_info → no gate). Applied to both the local and cloud submit paths (cloud also bills partner nodes server-side). run-template forwards `allow_spend=True` on its handoff to execute() so its already-consented run is not gated a second time. Docs + skill updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
dosubot
Bot
added
size:M
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 6 finding(s).
| Severity | Count |
|---|---|
| 🟠 High | 1 |
| 🟡 Medium | 2 |
| 🟢 Low | 3 |
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
Address cursor-review panel findings on PR #591: - Guard sys.stdin.isatty(): detached/pythonw contexts can leave sys.stdin as None (AttributeError) or closed (ValueError); route those to the fail-closed machine-mode error instead of crashing. - Escape partner class_type names before Rich-markup interpolation so a name like `[bold]` can't raise MarkupError or inject formatting. - Align docs/json-output.md: the cloud spend_consent_required variant carries `where: "cloud"` in details, not host/port. The fail-open catalog-unavailable findings (High/Medium) are a deliberate, load-bearing design choice; deferred to a follow-up for a product decision rather than reversed here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dosubot
Bot
added
size:L
|
🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:
|
1 participant
ELI-5
comfy runcould quietly spend your Comfy credits: if your workflow contained apaid partner-API node (Seedream/Veo/Kling/Gemini/Flux-API/…) and the CLI found a
credential, it submitted and billed you with no warning. Its siblings
comfy generateandcomfy run-templatealready ask first. This adds the same"are you sure?" gate to
comfy run: it stops before spending, tells you whichnodes cost money, and only runs when you say yes (a prompt) or pass
--allow-spend. Ordinary free workflows behave exactly as before.What changed
--allow-spendflag oncomfy run(defaultFalse), help text mirroringrun-template's, threaded throughrun.execute()andrun.execute_cloud()._spend_gateinterlock (copied fromrun-template's gate,BE-4113), inserted between partner-node detection and credential resolution:
typer.confirm(..., default=False);a decline →
spend_consent_required+ exit 1, nothing submitted.spend_consent_required, message tellsthe caller to re-run with
--allow-spend, hint notes free workflows need noflag;
partner_nodesindetails._resolve_partner_credential()/ cloudClient()so arefusal never triggers a network OAuth refresh.
comfy run-templateforwardsallow_spend=Trueonits handoff to
execute()(its own, strictly-stronger gate — it also inspectsgallery signals — has already consented).
spend_consent_requiredrow added to thecomfy runtable indocs/json-output.md;--allow-spenddocumented in the run section ofcomfy_cli/skills/comfy/SKILL.md.Behavior contract
--allow-spendspend_consent_required, nothing submitted/spent{}→ no detection → no gate), same as run-templatepartner_node_requires_credentialstill fires (unchanged)Tests
tests/comfy_cli/command/test_run.py— newTestExecuteSpendGate(machine-modefail-closed,
--allow-spendproceeds, no-partner never gates, TTY declineblocks, TTY accept proceeds) and
TestExecuteCloudSpendGate(cloud machine-modefail-closed +
--allow-spendproceeds). The two existing partner-credentialtests now pass
allow_spend=True(they exercise the post-consent credentialpath).
test_run_template.py— a handoff test assertingallow_spend=Trueisforwarded so
execute()doesn't re-gate. Full suite green (2775 passed, 37skipped);
ruff format+ruff checkclean.Judgment calls
and left cloud conditional ("if cloud submit also bills partner nodes, gate it
the same way"). Cloud does bill partner nodes server-side, so I applied the
same gate in
execute_cloud()for consistency, using the already-loaded cloudobject_infowith the same fail-open posture. This is a behavior change forexisting non-interactive cloud runs of partner workflows (they now need
--allow-spend), but it closes the same silent-spend hole and is covered bytests. Easy to drop if reviewers prefer local-only.
allow_spend=True(per theticket's "simplest correct form"), since run-template's gate is strictly
stronger and has already run.
Negative-claim falsification (per self-review discipline)
This diff adds a fail-closed / deny path, so per the falsification rule: the gate
does not deny a capability — running paid workflows remains fully reachable
via
--allow-spendor an interactive "yes", proven by the passing..._with_allow_spend_proceeds/..._confirm_accepted_proceedstests. It is aconsent checkpoint with a documented escape, not a #581-style dead-end.
Out of scope
comfy generate/run-templategate changes;comfy validatepartner-nodesurfacing (sibling ticket); MCP wrapper changes (follow-up, blocked on this).