feat: deploy frontend preview to Cloudflare Pages with ConnectionPanel#11118
Open
snomiao wants to merge 51 commits into
Open
feat: deploy frontend preview to Cloudflare Pages with ConnectionPanel#11118snomiao wants to merge 51 commits into
snomiao wants to merge 51 commits into
Conversation
dosubot
Bot
added
the
size:XL
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a GitHub Actions preview/deploy workflow and deploy script for Cloudflare Pages, injects CI build-time constants, introduces a Connection Panel view with router and tests, and updates ComfyApi to honor a localStorage backend override. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Frontend as Frontend App
participant ConnView as ConnectionPanelView
participant HTTP as HTTP Endpoint
participant WS as WebSocket Endpoint
participant Storage as localStorage
User->>Frontend: Navigate to /connect
Frontend->>ConnView: Mount
ConnView->>Storage: Read comfyui-preview-backend-url
ConnView->>ConnView: Initialize URL
User->>ConnView: Enter URL and click Test
ConnView->>Storage: Save normalized URL
par HTTP Test
ConnView->>HTTP: GET /api/system_stats (5s)
HTTP-->>ConnView: Response / Timeout
and WS Test
ConnView->>WS: Open ws(s)://.../ws (5s)
WS-->>ConnView: Open / Error
end
ConnView->>User: Show httpStatus & wsStatus indicators
ConnView->>Frontend: If both ok -> Connect & Go (reload to /)
sequenceDiagram
participant GitHub as GitHub
participant GHA as GitHub Actions
participant BuildJob as Build
participant Script as Deploy Script
participant Wrangler as Wrangler CLI
participant Cloudflare as Cloudflare Pages
participant GHApi as GitHub API
GitHub->>GHA: PR opened / push to main
GHA->>GHA: comment-on-pr-start job (PRs)
GHA->>Script: run status="starting"
Script->>GHApi: post "building" comment
GHA->>BuildJob: run pnpm build
BuildJob-->>GHA: upload dist artifact
GHA->>Script: run status="completed" (PRs)
Script->>Wrangler: deploy dist/ to Pages (retries)
Wrangler-->>Cloudflare: create preview
Cloudflare-->>Script: return preview URL
Script->>GHApi: update PR comment with preview link
GHA->>Cloudflare: on main push -> deploy-production job uses Wrangler to deploy main branch
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🌐 Frontend Preview: ✅ Built — 🌐 Open Preview |
🎭 Playwright: ✅ 1431 passed, 0 failed · 2 flaky📊 Browser Reports
|
🎨 Storybook: ✅ Built — View Storybook |
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
📦 Bundle: 5.25 MB gzip 🔴 +12.6 kBDetailsSummary
Category Glance App Entry Points — 24.4 kB (baseline 22.5 kB) • 🔴 +1.82 kBMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.24 MB (baseline 1.24 MB) • 🔴 +160 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 104 kB (baseline 77.7 kB) • 🔴 +26.3 kBTop-level views, pages, and routed surfaces
Status: 12 added / 11 removed Panels & Settings — 490 kB (baseline 488 kB) • 🔴 +1.76 kBConfiguration panels, inspectors, and settings screens
Status: 22 added / 21 removed User & Accounts — 17.6 kB (baseline 17.5 kB) • 🔴 +140 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed Editors & Dialogs — 113 kB (baseline 113 kB) • 🔴 +210 BModals, dialogs, drawers, and in-app editors
Status: 4 added / 4 removed UI Components — 64.1 kB (baseline 61 kB) • 🔴 +3.05 kBReusable component library chunks
Status: 13 added / 13 removed Data & Services — 3.04 MB (baseline 3.04 MB) • 🔴 +2.93 kBStores, services, APIs, and repositories
Status: 17 added / 16 removed / 1 unchanged Utilities & Hooks — 364 kB (baseline 364 kB) • 🟢 -659 BHelpers, composables, and utility bundles
Status: 31 added / 31 removed Vendor & Third-Party — 9.88 MB (baseline 9.88 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Status: 7 added / 7 removed / 9 unchanged Other — 8.84 MB (baseline 8.83 MB) • 🔴 +8.04 kBBundles that do not match a named category
Status: 131 added / 131 removed / 4 unchanged ⚡ Performance
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a Cloudflare Pages deployment pipeline for a ComfyUI frontend preview site and introduces a /connect ConnectionPanel page that helps users configure/test connectivity to a ComfyUI backend, including displaying CI build metadata.
Changes:
- Add GitHub Actions workflow to build and deploy Cloudflare Pages previews (with auto PR comment updates) and deploy
mainto the production Pages branch. - Add
ConnectionPanelViewat/connectwith backend URL persistence, HTTP+WS connectivity tests, and build info footer. - Inject CI metadata into the Vite build (
__CI_*__) and register the corresponding TS/ESLint globals and i18n strings.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci-deploy-preview.yaml |
New workflow to build and deploy preview/production to Cloudflare Pages and comment on PRs. |
scripts/cicd/pr-preview-deploy-and-comment.sh |
New deployment + PR comment helper script for Cloudflare Pages previews. |
src/views/ConnectionPanelView.vue |
New /connect view to configure/test backend connectivity and show build metadata. |
src/views/ConnectionPanelView.test.ts |
Unit tests for the new ConnectionPanel view behaviors. |
src/router.ts |
Adds the /connect route pointing to ConnectionPanelView. |
src/locales/en/main.json |
Adds connectionPanel.* i18n strings used by the new view. |
vite.config.mts |
Injects CI env vars as build-time globals for build metadata display. |
global.d.ts |
Declares new __CI_*__ globals for TypeScript. |
eslint.config.ts |
Registers new __CI_*__ globals for linting. |
TODO.md |
Tracks the implementation plan and checklist for the preview deployment + connection panel. |
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #11118 +/- ##
===========================================
- Coverage 67.68% 54.62% -13.06%
===========================================
Files 1477 1379 -98
Lines 83266 70661 -12605
Branches 22925 18781 -4144
===========================================
- Hits 56355 38601 -17754
- Misses 25970 31510 +5540
+ Partials 941 550 -391
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1043 files with indirect coverage changes 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci-deploy-preview.yaml:
- Around line 15-17: The deploy-and-comment job can race with
comment-on-pr-start and overwrite its marker; add a dependency so
deploy-and-comment waits for comment-on-pr-start (e.g., add needs: [build,
comment-on-pr-start] to the deploy-and-comment job) and apply the same change to
the other deploy/comment block referenced (lines 77-80) so any job that updates
the shared marker includes the comment-on-pr-* job in its needs list.
- Around line 18-22: The job "comment-on-pr-start" currently sets job-level
permissions to only pull-requests: write which causes checkout to fail because
unspecified permissions default to none; update the permissions block for the
comment-on-pr-start job to include contents: read alongside pull-requests: write
so actions/checkout@v6 can access the repository (i.e., add contents: read to
the permissions mapping for that job).
In `@scripts/cicd/pr-preview-deploy-and-comment.sh`:
- Around line 129-139: The current branch-normalization logic that sets
cloudflare_branch from BRANCH_NAME can collapse distinct branches and cause
preview collisions; instead generate and use a PR-scoped alias (e.g.,
"pr-$PR_NUMBER") for preview deployments and pass that alias into deploy_preview
rather than cloudflare_branch. Update the code that defines cloudflare_branch to
create preview_alias="pr-$PR_NUMBER" (or similar), replace usages of
cloudflare_branch when calling deploy_preview and any related deploy metadata,
and ensure deploy_preview (the call site) receives this PR-scoped alias so
previews are unique per PR.
- Around line 48-56: The wrangler install block runs unconditionally and uses a
top-level return (invalid); move the installation logic so it only runs in the
deployment path (i.e., when STATUS="completed" before calling deploy_preview()),
replace the top-level return with exit 1 (or wrap the logic inside a function)
so failures terminate correctly, and ensure deploy_preview() is the only caller
that triggers npm install -g wrangler@^4.0.0 to avoid attempting installation
for non-deploy status values.
In `@src/router.ts`:
- Around line 86-89: The router base is being derived from
window.location.pathname causing a hard refresh on /connect to set base to
'/connect' and make the /connect route resolve as root; update getBasePath()
(used when creating the router in src/router.ts) so it only returns a deployment
subpath determined from build-time configuration or a known static prefix (e.g.,
process.env.BASE_URL or a hardcoded '/ComfyUI/') and never the full current
pathname—alternatively sanitize the pathname by stripping any recognized route
segments before returning the base; ensure createRouter (or wherever
getBasePath() is consumed) uses this corrected base so direct loads to
'/connect' resolve correctly.
In `@src/views/ConnectionPanelView.test.ts`:
- Around line 54-57: Tests are leaving a global stubbed via vi.stubGlobal (e.g.,
fetch stub at vi.stubGlobal in ConnectionPanelView.test), but the existing
beforeEach uses vi.restoreAllMocks() which does not remove stubbed globals; add
vi.unstubAllGlobals() (or call it in the same beforeEach/afterEach alongside
vi.restoreAllMocks()) to ensure any vi.stubGlobal() stubs (like the rejected
fetch at line ~104) are removed between tests so subsequent tests don't inherit
them.
In `@src/views/ConnectionPanelView.vue`:
- Around line 97-104: Replace the hardcoded CORS value "*" in the code block
with the current preview origin: add a computed property (e.g., previewOrigin =
window.location.origin and corsCommand or getCorsCommand) that returns `python
main.py --enable-cors-header="${previewOrigin}"`, then bind that computed value
into the template's code block instead of the literal string; ensure the
computed name you create (e.g., corsCommand) is referenced where the template
currently renders the static command so the shown command is origin-scoped and
not a global wildcard.
- Around line 40-88: Make the status block a screen-reader live region so async
test results are announced: add aria-live="polite" and aria-atomic="true" (and
optionally role="status") to the <section> that renders the
httpStatus/wsStatus/connectionError messages (the container controlled by
v-if="httpStatus !== null || wsStatus !== null"), so updates to connectionError,
httpStatus, or wsStatus are announced to assistive tech.
- Around line 240-252: Replace hardcoded English strings in the computed
properties buildLabel and buildTooltip with vue-i18n calls: import/use the i18n
translator (t) inside these computed functions, map the cases to localized keys
(e.g., connectionPanel.build.pr with { prNumber },
connectionPanel.build.versionShort with { version }, and for tooltip entries
connectionPanel.build.version, .commit, .branch, .runId, .jobId), and pass the
sliced commit or other values as params; keep the existing branching logic
(prNumber -> branch -> version) and the parts.join('\n') behavior but substitute
t(...) for each literal label/prefix so all user-facing strings are localized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b4921688-56d8-412c-8733-c83526d48268
📒 Files selected for processing (10)
.github/workflows/ci-deploy-preview.yamlTODO.mdeslint.config.tsglobal.d.tsscripts/cicd/pr-preview-deploy-and-comment.shsrc/locales/en/main.jsonsrc/router.tssrc/views/ConnectionPanelView.test.tssrc/views/ConnectionPanelView.vuevite.config.mts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/views/ConnectionPanelView.vue (3)
42-45:⚠️ Potential issue | 🟡 MinorAnnounce async connection results to assistive tech.
This status block appears after the user clicks Test, but it is not a live region, so screen-reader users won't hear when the HTTP/WS result arrives. Add
role="status",aria-live="polite", andaria-atomic="true"on the section container.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/ConnectionPanelView.vue` around lines 42 - 45, The status container that shows test results (the <section> rendered when httpStatus or wsStatus changes) is not announced to assistive tech; update that section element in ConnectionPanelView.vue (the block using v-if="httpStatus !== null || wsStatus !== null") to include role="status", aria-live="polite", and aria-atomic="true" so screen readers will politely announce the async HTTP/WS results when httpStatus or wsStatus updates.
311-323:⚠️ Potential issue | 🟡 MinorLocalize the build metadata labels.
PR #,v,Version:,Commit:,Branch:,Run ID:, andJob ID:are still hardcoded English strings insidebuildLabelandbuildTooltip. These should go throughconnectionPanel.*keys like the rest of this view. As per coding guidelines, "Use vue-i18n for ALL user-facing strings, configured in src/locales/en/main.json".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/ConnectionPanelView.vue` around lines 311 - 323, The buildLabel and buildTooltip computed properties currently use hardcoded English strings; update buildLabel and buildTooltip to use vue-i18n keys under connectionPanel (e.g., connectionPanel.prLabel, connectionPanel.versionPrefix, connectionPanel.versionTooltip, connectionPanel.commitLabel, connectionPanel.branchLabel, connectionPanel.runIdLabel, connectionPanel.jobIdLabel) instead of literals; obtain the translation function (e.g., via useI18n or this.$t) in this component and replace `PR #${prNumber}`, `v${version}`, and the tooltip parts (`Version:`, `Commit:`, `Branch:`, `Run ID:`, `Job ID:`) with t('connectionPanel.xxx') calls, formatting/interpolating the variables (prNumber, version, commit.slice(0,8), branch, runId, jobId) into the translated strings so all user-facing text uses vue-i18n keys.
139-143:⚠️ Potential issue | 🟠 MajorScope the CORS command to this preview origin.
Both snippets still tell users to run
--enable-cors-header="*". That opens their local ComfyUI instance to every origin, even though this page already knows the exact origin it needs. Renderwindow.location.originhere, or clearly mark*as disposable local-only debugging guidance.Also applies to: 163-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/ConnectionPanelView.vue` around lines 139 - 143, The displayed CORS command currently uses a wildcard (*) which opens ComfyUI to all origins; update the ConnectionPanelView.vue template to render the actual preview origin instead of "*" by injecting window.location.origin into the command string (e.g., compute a template property like previewOrigin = window.location.origin and use it in the <code> block) or, if you must keep "*", add an explicit local-only warning next to the command; modify the code block that renders the launch command (the <code> element in ConnectionPanelView.vue) to use that previewOrigin value (and apply the same change to the other snippet later in the file).src/views/ConnectionPanelView.test.ts (1)
43-43:⚠️ Potential issue | 🟡 MinorUnstub globals between test cases.
vi.restoreAllMocks()does not remove globals installed viavi.stubGlobal(), so the rejectedfetchfrom Line 115 can leak into later cases and fail them for the wrong reason.Suggested fix
-import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -vi.stubGlobal('localStorage', mockLocalStorage) - const i18n = createI18n({ @@ describe('ConnectionPanelView', () => { beforeEach(() => { mockLocalStorage.clear() vi.restoreAllMocks() + vi.stubGlobal('localStorage', mockLocalStorage) }) + + afterEach(() => { + vi.unstubAllGlobals() + })In Vitest, does vi.restoreAllMocks() revert globals installed with vi.stubGlobal(), or should tests call vi.unstubAllGlobals() between cases?Also applies to: 60-63, 115-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/ConnectionPanelView.test.ts` at line 43, Tests use vi.stubGlobal('localStorage', mockLocalStorage) (and other globals) but only call vi.restoreAllMocks(), which does not remove globals; add vi.unstubAllGlobals() in the test teardown (e.g., an afterEach hook) so globals stubbed with vi.stubGlobal are removed between cases to prevent the rejected fetch stub (and other global stubs) from leaking into subsequent tests; keep existing vi.restoreAllMocks()/vi.resetAllMocks() calls as well to clear spies/mocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/scripts/api.ts`:
- Around line 374-378: The code is storing an absolute URL into
ComfyApi.api_base which breaks how createSocket constructs websocket URLs;
instead, when parsing the saved remote backend URL, split and store origin and
path separately: keep this.api_host (or add this.api_origin) set to url.origin
(scheme+host+port) and set this.api_base to only the trimmed path
(url.pathname.replace(/\/+$/, '')), and then update createSocket to build the WS
URL from the same origin + path (or switch scheme to ws/wss based on origin) so
host, path, and ws/wss stay aligned.
- Around line 374-378: The code currently lets a user-configured preview backend
(set via remoteBackend and applied to this.api_host / this.api_base) receive
Cloud credentials; update the logic so that when a preview backend is configured
you do NOT forward Cloud auth by default—either disable forwarding entirely or
only allow it when the preview origin is explicitly trusted. Concretely, add a
check around the auth-forwarding paths (createSocket() and fetchApi()) that
returns false if a preview backend is active (remoteBackend present /
this.api_base differs from the built-in origin) unless the origin appears in an
allowlist of trusted origins, and use that check before appending the Cloud
token or setting Cloud auth headers.
---
Duplicate comments:
In `@src/views/ConnectionPanelView.test.ts`:
- Line 43: Tests use vi.stubGlobal('localStorage', mockLocalStorage) (and other
globals) but only call vi.restoreAllMocks(), which does not remove globals; add
vi.unstubAllGlobals() in the test teardown (e.g., an afterEach hook) so globals
stubbed with vi.stubGlobal are removed between cases to prevent the rejected
fetch stub (and other global stubs) from leaking into subsequent tests; keep
existing vi.restoreAllMocks()/vi.resetAllMocks() calls as well to clear
spies/mocks.
In `@src/views/ConnectionPanelView.vue`:
- Around line 42-45: The status container that shows test results (the <section>
rendered when httpStatus or wsStatus changes) is not announced to assistive
tech; update that section element in ConnectionPanelView.vue (the block using
v-if="httpStatus !== null || wsStatus !== null") to include role="status",
aria-live="polite", and aria-atomic="true" so screen readers will politely
announce the async HTTP/WS results when httpStatus or wsStatus updates.
- Around line 311-323: The buildLabel and buildTooltip computed properties
currently use hardcoded English strings; update buildLabel and buildTooltip to
use vue-i18n keys under connectionPanel (e.g., connectionPanel.prLabel,
connectionPanel.versionPrefix, connectionPanel.versionTooltip,
connectionPanel.commitLabel, connectionPanel.branchLabel,
connectionPanel.runIdLabel, connectionPanel.jobIdLabel) instead of literals;
obtain the translation function (e.g., via useI18n or this.$t) in this component
and replace `PR #${prNumber}`, `v${version}`, and the tooltip parts (`Version:`,
`Commit:`, `Branch:`, `Run ID:`, `Job ID:`) with t('connectionPanel.xxx') calls,
formatting/interpolating the variables (prNumber, version, commit.slice(0,8),
branch, runId, jobId) into the translated strings so all user-facing text uses
vue-i18n keys.
- Around line 139-143: The displayed CORS command currently uses a wildcard (*)
which opens ComfyUI to all origins; update the ConnectionPanelView.vue template
to render the actual preview origin instead of "*" by injecting
window.location.origin into the command string (e.g., compute a template
property like previewOrigin = window.location.origin and use it in the <code>
block) or, if you must keep "*", add an explicit local-only warning next to the
command; modify the code block that renders the launch command (the <code>
element in ConnectionPanelView.vue) to use that previewOrigin value (and apply
the same change to the other snippet later in the file).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b51c0f34-1670-4e93-b47f-b322f41f1e49
📒 Files selected for processing (5)
src/locales/en/main.jsonsrc/router.tssrc/scripts/api.tssrc/views/ConnectionPanelView.test.tssrc/views/ConnectionPanelView.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- src/router.ts
- src/locales/en/main.json
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/views/ConnectionPanelView.test.ts (1)
44-45:⚠️ Potential issue | 🟡 MinorUnstubbed globals can leak across tests.
Line 44 and Line 111 use
vi.stubGlobal(...), but Line 61–64 only callsvi.restoreAllMocks(), which won’t remove stubbed globals. Please unstub globals after each test and restublocalStorageinbeforeEach.As per coding guidelines, “Keep module mocks contained; do not use global mutable state within the test file; use `vi.hoisted()` if necessary for per-test mock manipulation.”Suggested fix
-import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -vi.stubGlobal('localStorage', mockLocalStorage) - const i18n = createI18n({ @@ describe('ConnectionPanelView', () => { beforeEach(() => { + vi.stubGlobal('localStorage', mockLocalStorage) mockLocalStorage.clear() vi.restoreAllMocks() }) + + afterEach(() => { + vi.unstubAllGlobals() + })Also applies to: 61-64, 111-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/ConnectionPanelView.test.ts` around lines 44 - 45, Tests currently stub global localStorage with vi.stubGlobal('localStorage', mockLocalStorage) but only call vi.restoreAllMocks(), which doesn't remove global stubs; update the test setup to stub localStorage in beforeEach and call vi.unstubAllGlobals() in afterEach (or call vi.unstubAllGlobals() alongside vi.restoreAllMocks()) so globals are cleaned between tests; ensure references to vi.stubGlobal, vi.unstubAllGlobals, vi.restoreAllMocks, beforeEach and afterEach are used to restub and unstub localStorage per test to avoid leaking global state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/views/ConnectionPanelView.test.ts`:
- Around line 44-45: Tests currently stub global localStorage with
vi.stubGlobal('localStorage', mockLocalStorage) but only call
vi.restoreAllMocks(), which doesn't remove global stubs; update the test setup
to stub localStorage in beforeEach and call vi.unstubAllGlobals() in afterEach
(or call vi.unstubAllGlobals() alongside vi.restoreAllMocks()) so globals are
cleaned between tests; ensure references to vi.stubGlobal, vi.unstubAllGlobals,
vi.restoreAllMocks, beforeEach and afterEach are used to restub and unstub
localStorage per test to avoid leaking global state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9f7b2fe0-662a-4ac9-8fae-e0d6150699a4
📒 Files selected for processing (3)
eslint.config.tssrc/views/ConnectionPanelView.test.tsvite.config.mts
✅ Files skipped from review due to trivial changes (2)
- vite.config.mts
- eslint.config.ts
dosubot
Bot
added
size:XXL
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
comfy install bundles ComfyUI-Manager by default. Surface that in the quick start, plus an explanatory aside on why Manager smooths the UX (one-click missing-model/custom-node install). For the manual python alt path, add the git clone step explicitly.
The @/utils/tailwindUtil shim was removed from main in #11453; CI merge-commit builds fail for this branch. Switch the import to the package that replaced it. Both paths resolve locally today, so the change is a drop-in.
The deploy script uses `pnpm dlx wrangler` when wrangler isn't on PATH, but the deploy-and-comment job runs on a bare ubuntu image with no pnpm — all three retry attempts fail with `pnpm: command not found`. Set up pnpm + Node via the same actions setup-frontend uses.
…ll into one step uv pip install puts comfy on PATH immediately, so both commands chain with && in the same shell. Collapses 4 quick-start steps to 3 and keeps the shell flow linear for copy-paste.
Aligns frontend preview with storybook/playwright deploy scripts which already key the cloudflare branch alias on the sanitized git branch name. Falls back to pr-$PR_NUMBER when BRANCH_NAME is unset.
…ts argv Parse `--comfy-api-base` out of `/system_stats` `system.argv` and surface the backend's Comfy Cloud target in the connection panel. When it disagrees with the frontend's build-time `getComfyApiBaseUrl()`, show an amber warning so users can spot the mismatch before sign-in tokens get rejected. Records the cleaner long-term path (add `comfy_api_base` to the backend's `/features` endpoint) in docs/backend-cloud-api-base-feature-flag.md.
…target When the backend reports a known Comfy Cloud API base (prod or staging via `--comfy-api-base`), surface a link to the matching platform's API-keys page (platform.comfy.org or stagingplatform.comfy.org). Custom/unknown bases hide the link rather than guess a URL.
Adds a prominent amber aside under the panel title showing: - This is an in-flight PR build, UI may change rapidly, not for production - Build provenance: PR number, commit hash, author (all linked to GitHub) - Trust warning: do not connect a backend you care about unless you trust the PR author, since a malicious frontend can read or modify any workflow / model / output on the connected backend. Wires `CI_PR_AUTHOR` from `github.event.pull_request.user.login` through the build env into a new `__CI_PR_AUTHOR__` define.
… local-network guidance - Add CopyCodeBlock component with copy-to-clipboard and 2s "copied" checkmark feedback - Replace all static <code> blocks in ConnectionPanelView with CopyCodeBlock - Pre-compute launch command strings in script to avoid HTML entity escaping in template - Expand localAccess section with --listen command for multi-device LAN testing - Add corsOriginNote explaining why the exact origin matters (not *) - Add i18n keys: corsOriginNote, localAccessListenDescription, localAccessListenNote Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ctly When the backend exposes `system.comfy_api_base` (added in companion backend PR), use it directly instead of parsing it out of `argv`. Falls back to argv parsing for older backends that don't yet include the field. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add prPreviewBadges extension (active only when CI_PR_NUMBER is set): - Topbar warning badge showing "PR #XXXX", tooltip with author/commit/backend URL - About panel badges: clickable PR#, author (@handle), commit hash links - Declare __CI_PR_NUMBER__, __CI_PR_AUTHOR__, __CI_BRANCH__, __CI_RUN_ID__, __CI_JOB_ID__, __COMFYUI_FRONTEND_COMMIT__ as global TS constants in vite-env.d.ts - Extract resolveBackendCloudBase() to shared util at src/platform/connectionPanel/resolveBackendCloudBase.ts - Add comfy_api_base as optional field in zSystemStats schema (companion to backend PR Comfy-Org/ComfyUI#13571) - Add prPreview i18n keys Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
snomiao
force-pushed
the
sno-frontend-preview
branch
from
e2492a4 to
d840020
Compare
Comment on lines
+1
to
+6
| # 案 B: バックエンドの `/features` に `comfy_api_base` を追加する | ||
|
|
||
| ## 背景 | ||
|
|
||
| ComfyUI バックエンドは `--comfy-api-base` CLI フラグで Comfy Cloud の API ベース URL（prod / staging / カスタム）を選択する。 | ||
| フロントエンドは `__USE_PROD_CONFIG__` ビルド時定数で同じ値を選ぶ。 |
There was a problem hiding this comment.
This doc is written entirely in Japanese, while the rest of the repo’s docs appear to be in English (e.g. docs/FEATURE_FLAGS.md). To keep documentation accessible/consistent, please translate this file to English (or move it to a locale-specific docs area if one exists).
Comment on lines
+128
to
+131
| # Falls back to pr-$PR_NUMBER if BRANCH_NAME is unset | ||
| if [ -n "$BRANCH_NAME" ]; then | ||
| cloudflare_branch=$(echo "$BRANCH_NAME" | tr '[:upper:]' '[:lower:]' | \ | ||
| sed 's/[^a-z0-9-]/-/g' | sed 's/--*/-/g' | sed 's/^-\|-$//g') |
There was a problem hiding this comment.
cloudflare_branch can become an empty string after sanitization (e.g. branch name contains only characters removed/replaced by the sed pipeline). That would produce an invalid Pages branch/URL like https://.comfy-ui.pages.dev and likely fail deployment. Add a post-sanitization fallback (e.g. if empty, set to pr-$PR_NUMBER or a fixed placeholder).
Suggested change
| # Falls back to pr-$PR_NUMBER if BRANCH_NAME is unset | |
| if [ -n "$BRANCH_NAME" ]; then | |
| cloudflare_branch=$(echo "$BRANCH_NAME" | tr '[:upper:]' '[:lower:]' | \ | |
| sed 's/[^a-z0-9-]/-/g' | sed 's/--*/-/g' | sed 's/^-\|-$//g') | |
| # Falls back to pr-$PR_NUMBER if BRANCH_NAME is unset or sanitization produces an empty value | |
| if [ -n "$BRANCH_NAME" ]; then | |
| cloudflare_branch=$(echo "$BRANCH_NAME" | tr '[:upper:]' '[:lower:]' | \ | |
| sed 's/[^a-z0-9-]/-/g' | sed 's/--*/-/g' | sed 's/^-\|-$//g') | |
| if [ -z "$cloudflare_branch" ]; then | |
| cloudflare_branch="pr-$PR_NUMBER" | |
| fi |
| resolve(true) | ||
| }) | ||
| ws.addEventListener('error', () => { | ||
| clearTimeout(timeout) |
There was a problem hiding this comment.
testWs() resolves false on the error event but doesn’t close the WebSocket. Closing it in the error handler avoids leaving a failed socket around (and matches the timeout/open paths that explicitly close).
Suggested change
| clearTimeout(timeout) | |
| clearTimeout(timeout) | |
| ws.close() |
Comment on lines
+7
to
+14
| <button | ||
| :title="copied ? t('clipboard.successMessage') : t('g.copyToClipboard')" | ||
| :aria-label=" | ||
| copied ? t('clipboard.successMessage') : t('g.copyToClipboard') | ||
| " | ||
| class="absolute top-2 right-2 rounded-sm p-1 text-neutral-500 transition-colors hover:text-neutral-100" | ||
| @click="copy(text)" | ||
| > |
There was a problem hiding this comment.
This <button> is missing an explicit type="button". If this component is ever rendered inside a <form>, the default type="submit" can cause unintended form submissions. Set type="button" to make the behavior explicit (consistent with other components like src/components/chip/Tag.vue).
Comment on lines
+116
to
+118
| {{ t('connectionPanel.http') }} | ||
| {{ httpStatus === true ? '✓' : httpStatus === false ? '✗' : '—' }} | ||
| </span> |
There was a problem hiding this comment.
The connection status line uses hardcoded glyphs ('✓', '✗', '—'). If these are intended to be user-facing, consider replacing them with an icon (with appropriate aria-label) or moving them into i18n so they’re consistent with the rest of the localized UI.
…links Extends TopbarBadge with a popoverLinks field that renders as <a> tags in the popover. The PR preview badge now shows clickable links to the GitHub PR page, author profile, commit, and a "Configure backend →" link that re-opens /connect to change the backend URL at any time. Also fixes prPreviewBadges backendUrl to read from localStorage directly (shows '—' when no remote backend configured, instead of the CF Pages domain itself). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Badge now shows "PR #11118 · abc12345" so reviewers can see the exact commit at a glance without opening the popover. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… set Full mode only had v-tooltip (plain text on hover). When popoverLinks is present, the badge now shows a clickable Popover with links, matching the behavior of icon-only and compact modes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The tooltip string (with HTML-escaped URLs and dot-separated fields) was showing inside the popover alongside the cleaner popoverLinks section. Remove it — the links already convey all the same info without the HTML entity artifacts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview builds were using stagingapi.comfy.org (USE_PROD_CONFIG=false default), causing a constant cloud environment mismatch warning for every user who followed the quick-start guide — their local ComfyUI backend defaults to api.comfy.org (production). Setting USE_PROD_CONFIG=true aligns the preview with the default backend behavior so the mismatch warning only fires when the user deliberately passes --comfy-api-base=staging. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Users can now enter their Comfy API key on /connect before clicking "Connect & Open ComfyUI". The key is saved to localStorage (comfy_api_key) so it's picked up automatically after connecting — no need to re-enter it in Settings → API Key. Existing key is pre-filled from localStorage if already set. Empty field on connect leaves any existing stored key unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add Test button for Comfy API Key input (validates against /customers endpoint) - Show ✓/✗ status after test - Update hint text: "Only needed for cloud-API nodes" - Hide API key section when backend reports --disable-api-nodes in argv - Show notice when API nodes are disabled Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contextually show the API key field only after backend is tested and cloud API is detected, co-located with the cloud API info and the "Generate an API key" link. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ML escaping escapeParameter: true in i18n.ts caused slashes in URLs to render as / in the popover tooltip. Concatenate the label and URL directly instead of passing the URL through t() interpolation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Deploy the ComfyUI frontend to `comfy-ui.pages.dev` via Cloudflare Pages. A ConnectionPanel at `/connect` auto-opens when no local backend is detected, guiding users through installing and launching ComfyUI.
Live preview: https://sno-frontend-preview.comfy-ui.pages.dev
(URL is the sanitized git branch name — stable across redeploys)
Screenshots
Connection Panel (no backend detected, auto-redirected from `/`):
GraphView (after connecting to local backend):
Changes
ConnectionPanel UX flow
```sh
comfy launch -- --enable-cors-header="https://sno-frontend-preview.comfy-ui.pages.dev"
```
Companion backend PR
Comfy-Org/ComfyUI#13571 — adds `system.comfy_api_base` to `/api/system_stats` so the frontend can read the cloud API target directly instead of parsing argv. One-line change; fully backwards-compatible.
Review Focus
Testing