fix: route auth hints to comfy cloud login instead of nonexistent comfy auth login#512
fix: route auth hints to comfy cloud login instead of nonexistent comfy auth login#512mattmillerai wants to merge 3 commits into
comfy cloud login instead of nonexistent comfy auth login#512Conversation
…comfy auth login` The `auth` command group manages third-party model-host API tokens (Civitai, Hugging Face) and has no `login` subcommand. Comfy Cloud sign-in is `comfy cloud login`. Seven help/hint/error strings (plus a stale comment) pointed users and agents at the dead `comfy auth login`, including the agent-facing `jobs --where` option help. Correct them all and tighten the signed-out banner tests to guard the hint.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 50 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 (4)
📝 WalkthroughWalkthroughCloud authentication guidance was updated across OAuth errors, jobs and cloud execution messages, run CLI examples, capability output, branding documentation, and related tests. ChangesCloud command guidance
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 1 finding(s).
| Severity | Count |
|---|---|
| 🟡 Medium | 1 |
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
The interactive demo's sign-in step invoked `comfy auth whoami`, which was deliberately removed (test_legacy_auth_whoami_is_gone) — the command now lives at `comfy cloud whoami`. Update the step title, both invocations, and the capabilities summary so the walkthrough runs a command that actually exists. Adjust the step-coverage test needle from "auth" to "whoami" to match. Addresses cursor-review (gemini-3.1-pro edge-case) thread on PR #512. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Status update (automated resolver):
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/comfy_cli/command/test_run_cli.py (1)
28-46: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the exact cloud command, not only the title.
This test would pass if the title said
whoamiwhile invocations still rancomfy auth whoami. Assert the step’s invocation argv/labels containcloud whoamiand excludeauth whoami, keeping the regression net tight and the demo out of the auth weeds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/comfy_cli/command/test_run_cli.py` around lines 28 - 46, Strengthen test_build_steps_contains_full_surface by locating the whoami step and asserting its invocation argv or labels include “cloud whoami” and do not include “auth whoami”; retain the existing title coverage while verifying the exact cloud command.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/comfy_cli/command/test_run_cli.py`:
- Around line 28-46: Strengthen test_build_steps_contains_full_surface by
locating the whoami step and asserting its invocation argv or labels include
“cloud whoami” and do not include “auth whoami”; retain the existing title
coverage while verifying the exact cloud command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2f12adf0-e6e1-4e9f-b6c4-3fd1e5d9e8c6
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
comfy_cli/cloud/oauth.pycomfy_cli/command/jobs.pycomfy_cli/command/run/__init__.pycomfy_cli/command/run_cli.pycomfy_cli/output/branding.pytests/comfy_cli/command/test_run_cli.pytests/comfy_cli/output/test_branding.py
Four cloud error paths (job cancel, workflow HTTP errors, cloud model list/search) still pointed users at `comfy auth whoami`, which does not exist — the `auth` group only has list/set/remove, and sign-in status lives under `comfy cloud whoami`. Same class of bug this PR set out to fix for `auth login`. Also strengthen the run_cli demo test per CodeRabbit: assert the whoami step's argv/labels target `cloud whoami` and never `auth whoami`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed review + self-review in e828256:
Verified the redirect targets are real commands, not dead-ends: Full suite: 2574 passed, 37 skipped. ruff clean. Note: the failing "Windows Specific Commands" check is an unrelated infra failure — ComfyUI's |
|
Resolver root-cause on the failing The Windows This is a Windows file-lock race: This diff cannot cause it: the failing step is All review threads are resolved, Cursor/CodeRabbit findings addressed, local suite green (218 passed), and CI-pinned |
ELI-5
If your Comfy Cloud session expired, several error messages and help texts told you to run
comfy auth loginto sign back in. That command doesn't exist —comfy authonly manages third-party model-host API tokens (Civitai, Hugging Face), and it has nologin. The real sign-in command iscomfy cloud login. This PR fixes all seven bad hints (plus one stale comment) so users — and agents — get sent to a command that actually works.What changed
Corrected every user-facing reference to the nonexistent
comfy auth login→comfy cloud login:comfy_cli/cloud/oauth.py— 4 OAuth error hints (timeout, authorize failure, token-exchange failure, refresh failure)comfy_cli/command/jobs.py— thejobs --whereoption help (agent-facing) and thecloud_unauthorizederror hintcomfy_cli/command/run/__init__.py— thecloud_unauthorizederror hintcomfy_cli/command/run_cli.py— the demo step descriptioncomfy_cli/output/branding.py— a stale code comment ("shown after auth login success")Also tightened the two signed-out-banner tests (
tests/comfy_cli/output/test_branding.py) from a permissive"comfy auth login" in out or "comfy cloud login" in outinto an exactcomfy cloud loginassertion plus a"comfy auth login" not in outguard, so this class of regression is now caught by the suite.Verified
comfy cloud loginis the real command:comfy_cli/cloud/command.py:42registers@app.command("login")under thecloudtyper group (cmdline.py:1637).setup.pyalready used the correct command.Acceptance
git grep "auth login"now returns zero user-facing hits — the only remaining matches are the two negative test assertions that guard against the string reappearing.main(audit snapshot @ a732f5c matched HEAD; edits made by content, not line number).Out of scope — flagged follow-up (
comfy auth whoami)While auditing I found a sibling bug of the same class: 7 references route users to
comfy auth whoami, which was deliberately removed as a command (seetests/comfy_cli/auth/test_auth_command.py:223—test_legacy_auth_whoami_is_gone). The correct command iscomfy cloud whoami. Locations:comfy_cli/command/jobs.py:1097comfy_cli/command/models/search.py:176,:486comfy_cli/command/workflow.py:659comfy_cli/command/run_cli.py:519(step title),:522,:524/:525(invocations + labels) — one of these sits in the same demo step as a line this PR editsI scoped these OUT of this PR because they are a distinct correction (
whoami, notlogin) spanning 5 files, and this ticket was explicitly theauth loginset. Recommend a separate ticket to fix them (same tool-surface audit spirit; therun_cli.pyinvocations would actually error at runtime, though they are markedoptional=True).Testing
ruff check+ruff format --checkon all changed files: cleanpytest tests/comfy_cli/output/test_branding.py tests/comfy_cli/auth/ tests/comfy_cli/command/test_run_cli.py tests/comfy_cli/command/test_run.py: 203 passed