[pull] main from Comfy-Org:main - #32
Merged
Merged
Conversation
## Summary Adds a reusable agent skill for finding repeated test setup and teardown and moving it to the narrowest correct lifecycle owner. ## Changes - **What**: Adds framework-neutral consolidation principles and workflow, with progressively loaded Vitest and Playwright references. ## Review Focus - Whether the main skill remains framework-neutral and principle-driven - Whether the runner-specific references capture the important lifecycle boundaries without duplicating canonical testing guidance --------- Co-authored-by: Amp <amp@ampcode.com>
## Summary Use the Playwright CI ComfyUI container for local development and Amp orbs without running cloud and local frontends together. ## Changes - **What**: Use `GH_TOKEN` plus a username derived through `gh api user`, with `COMFY_CI_CONTAINER_*` overrides and explicit fallback messages. - **What**: Fail CI when workflow and launcher container versions drift. - **What**: Document local setup and Amp project credentials. ## Review Focus Check mode selection in `.amp/services.yaml`, credential-gated image pulls, the public-source fallback, and exact container-version drift validation. ## Testing - `pnpm test:browser:local browserTabTitle.spec.ts --project=chromium` (3 passed) - Mocked credential/no-credential launcher paths and confirmed no private pull occurs without both values - Verified cloud mode skips Docker and the local backend; invalid modes fail clearly - Verified concurrent startup cannot stop the active backend - ShellCheck, formatter, positive/negative drift checks, and service health checks --------- Co-authored-by: Amp <amp@ampcode.com>
*PR Created by the Glary-Bot Agent*
---
Markdown Note headers render with noticeably more space before the next
line in Vue nodes than in classic litegraph.
The two renderers use entirely separate markdown pipelines with
unrelated heading styles:
| | classic litegraph (UI 2.0 off) | Vue nodes (UI 2.0 on) |
|---|---|---|
| pipeline | Tiptap/ProseMirror DOM widget (`useMarkdownWidget.ts`) |
`marked` + DOMPurify + `v-html` (`WidgetMarkdown.vue`) |
| container | `.comfy-markdown .tiptap` | `.comfy-markdown-content` |
| heading rules | none — UA defaults (`0.67em` margins) | `margin-top:
2rem; margin-bottom: 1rem` + `line-height: 1.6` |
Those fixed `rem` margins never tracked `--comfy-textarea-font-size`, so
at the default 10px widget font a heading carried a **16px** gap under a
**15.7px** heading, and `line-height: 1.6` added another ~4.7px of
half-leading below the heading text.
## Change
Both renderers now share one em-relative heading rhythm, so margins
scale with the heading instead of the root font size:
```css
.comfy-markdown .tiptap h1, /* …h2–h6 */
.comfy-markdown-content h1, /* …h2–h6 */ {
margin-block: 0.67em;
line-height: 1.2;
}
```
`0.67em` / `1.2` are exactly what classic litegraph already resolved to,
so classic h1 is pixel-identical. The per-level `margin-top: 2rem;
margin-bottom: 1rem` declarations and their now-redundant `:first-child`
overrides are removed from `.comfy-markdown-content` (the existing
`*:first-child { margin-top: 0 }` still wins on specificity and keeps a
leading heading flush).
## Measured (default 10px widget font, 100% zoom)
| | before | after |
|---|---|---|
| Vue h1 `margin-bottom` | 16px (fixed) | 10.53px (0.67em) |
| Vue h1 line box | 25.14px | 18.86px |
| Vue h1 top → paragraph top | 41.1px | 29.4px |
| Vue h2 `margin-top` / `margin-bottom` | 32px / 16px | 8.61px / 8.61px
|
| classic h1 `margin-bottom`, line box | 13.4px, 24px | **unchanged** |
Margin-to-heading ratio is now 0.67 at every level in both renderers
(h1/h2/h3 verified). Margins also scale correctly now: at
`Comfy.TextareaWidget.FontSize: 20` the Vue h1 margin becomes 21.06px
instead of remaining stuck at 16px.
Classic h2–h6 tighten slightly (e.g. h2 `margin-block` 12.45px →
10.05px) because they previously followed the UA's inverted scale, where
*smaller* headings got *larger* margins. h1 — the reported case — is
untouched.
## Verification
- Reproduced live against a running backend + dev server, toggling
`Comfy.VueNodes.Enabled` and measuring `getComputedStyle` in both
states, at 100% and 220% canvas zoom.
- `pnpm lint` (stylelint + oxlint + eslint), `pnpm format:check` pass.
- `WidgetMarkdown`, `useMarkdownWidget`, `markdownRendererUtil` unit
tests pass (36 passed, 4 skipped).
- No test added — per `AGENTS.md`, tests should not depend on
non-behavioral features like styles.
## Notes
- `.comfy-markdown-content` is also used by `WidgetTextPreview.vue`,
which picks up the same tightened rhythm.
- The node help panel (`NodeHelpContent.vue`) keeps its own duplicate
`2rem`/`1rem` heading rules — a different surface, left alone
deliberately.
- Remaining absolute difference between the two renderers comes from
their different heading font-size scales (classic uses UA
`2em`/`1.5em`/`1.17em`; Vue uses `22/14`, `18/14`, `16/14` of the base)
and body `line-height` (`1.6` vs `normal`). Those are design decisions,
not spacing bugs, so they are out of scope here.
Screenshots below are the same Markdown Note at 220% zoom.
## Screenshots
---------
Co-authored-by: glary-bot <bot@glary.dev>
## Summary Split the existing user-facing and correctness repairs out of #15062 so the framework migration can be reviewed independently. ## Changes - Applies the locale-data repairs already present in #15062 onto current `main`, preserving translations that were independently updated on `main` in the meantime. - Repairs the stale, corrupted, missing, and leftover locale values already included in #15062. - Fixes the custom-node locale workflow copy path (`temp/diff`, not `ComfyUI_frontend/temp/diff`). - Contains no i18n framework or dependency migration and no newly authored translations. ## Validation - Semantic three-way comparison against #15062 original repair patch for all 26 affected locale files. - JSON parse and format validation passed. - Pre-commit checks, including `knip`, passed. Extracted mechanically from #15062; no repair content was reauthored.
## Summary Enrolls ComfyUI_frontend in the shared advisory PR risk grader with a repo-specific risk map and a controlled bot runbook that proves the full R0-R3 range before merge. ## Changes - **What**: Adds the pinned reusable grader caller, maps actual frontend risk surfaces (auth, billing, workflow serialization, extension/API contracts, rollout/build/core infrastructure, and low-risk docs/i18n/tests/storybook), and registers the `comfy-pr-bot` R0 demo lane. Grading is deterministic: `grade = worst(path floor, provenance, reversibility)`. It syncs exactly one advisory `risk:*` label; `risk-dispute` remains the human override path. - **Breaking**: None. Labels are advisory and do not gate, route, or merge PRs. - **Dependencies**: None. The workflow pins `Comfy-Org/github-workflows` at `e4a8f7cd4a073da082b03950136530d4df50738f`. ## Verification - `risk:R0`: #15034, bot-authored runbook test change after all checks became green. - `risk:R1`: #14890, human-authored low-risk docs change. - `risk:R2`: #14891 and #14892, store changes. - `risk:R3`: #14893 and this PR, grader-owned configuration or workflow changes. ## Review Focus - The risk map and runbook registry are read from the base ref, so a PR cannot lower its own grade. - `default_tier` remains `R0`; human provenance floors ordinary PRs at `R1`. - R0 requires all three axes to be R0. Pending or failed checks raise reversibility to R2. - Workflow dispatch supports enrollment backfill and manual re-grading; the `RISK_CONFIG` repository variable remains the kill switch. Created by Codex --------- Co-authored-by: huang47 <157390+huang47@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…outes (IR-105) (#15063) ## Summary Two durable fixes for the IR-105 splash-screen hang incident. ### Fix 1 — Bootstrap auth wait timeout (`bootstrapStore.ts`) **Root cause:** `until(isInitialized).toBe(true)` and `until(isAuthenticated).toBe(true)` in `startStoreBootstrap` have no timeout. A stale Firebase session or broken auth response hangs bootstrap silently and indefinitely — no error, no log, just a forever splash screen. **Fix:** Extract `waitForCloudAuth()` with: - 16 s timeout (matching `router.ts:176` and `WorkspaceAuthGate.vue:78` — existing convention) - One retry after 3 s backoff - On second timeout: `captureException` to Sentry + **continue bootstrap** rather than blocking This means the worst case is now 35 s to a usable (logged-out) state instead of infinity. ### Fix 2 — Catch-all route for unknown paths (`router.ts`) **Root cause:** A request to e.g. `cloud.comfy.org/woiadawd` gets a 200 + app shell from the static host, Vue boots, finds **no matching route**, and the global auth guard's `until(isInitialized)` then hangs (see Fix 1). Even with Fix 1, a no-match route leaves the app on the splash screen. **Fix:** Add `{ path: '/:pathMatch(.*)*', redirect: '/' }` at the end of the route list. Unknown paths redirect to root; the global auth guard then sends unauthenticated users to `/cloud/login` as normal. ## Red-Green Verification | Commit | Purpose | |--------|---------| | `test: add failing tests for unbounded auth wait in startStoreBootstrap` | 🔴 Proves test catches the bug — the "gives up" test times out in 5 s against the unpatched code | | `fix: bound auth wait in startStoreBootstrap and add catch-all route` | 🟢 Both new tests pass, all 18 tests in the two affected files pass | ## Relation to open PRs - Supersedes / closes the scope of #15032 (`fix/bootstrap-auth-wait-timeout`) — same fix, same tests. If reviewers prefer to land #15032 first this PR can drop Fix 1. - Companion to #15022 (marketing-site redirect) and #15026 (`/login` → `cloud-login` route, already on main). ## Test Plan - [x] `pnpm vitest run src/stores/bootstrapStore.test.ts` — 7/7 pass (including 2 new timeout tests) - [x] `pnpm vitest run src/platform/cloud/onboarding/onboardingCloudRoutes.test.ts` — 11/11 pass - [x] `pnpm typecheck` — clean - [x] `pnpm exec eslint src/stores/bootstrapStore.ts src/router.ts` — clean - [ ] CI green (Playwright, Storybook, Codecov) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: GitHub Action <action@github.com>
Pressing spacebar while dragging vue nodes will now initiate a pan operation. Followup to #8998 --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: Amp <amp@ampcode.com>
…5062) ## Summary Replace the `@lobehub/i18n-cli` translation step with an in-house pipeline (`scripts/i18n/`) so changed English source strings are retranslated, removed keys are pruned, and protected syntax survives translation byte-for-byte. The user-facing and correctness repairs were split into #15137 (merged). This PR is strictly the framework migration. ## Changes - `pnpm locale` now runs `scripts/i18n/update-locales.ts`. It records the English sources of the last successful run as git blob hashes in `src/locales/.source-manifest.json`, diffs them against the current `src/locales/en/*.json` to invalidate changed and deleted keys, and rebuilds every output locale from the English structure. - Validates interpolation placeholders and protected literals (`<Picture N>`, `17k+5`, quoted `match` and `max`), retries corrupted translations, and aborts without writing if corruption persists. - `pnpm locale:check` reports pending work offline. - Normalizes English entry files to the committed serialization before hashing so manifest hashes match workflow commits regardless of `collect-i18n` and formatting order. - Removes `@lobehub/i18n-cli` and `.i18nrc.cjs`; configuration now lives in `scripts/i18n/config.ts`. - Translation requests go through the official `openai` SDK (the one new dependency, dev-only with no transitive dependencies), which owns transport retries, timeouts, and typed completion parsing; the pipeline keeps only domain-level retries for truncated or malformed completions. - Uses a blobless full-history checkout in the i18n workflows so recorded English source blobs can be fetched on demand. ## Review Focus Manifest advancement only after full validation; rebuild-from-source semantics; translation retry and validation behavior; and source-provenance handling. Depends on #15137 (merged). Supersedes #15024. [FE-1499](https://linear.app/comfyorg/issue/FE-1499/fix-i18n-release-workflow-changed-source-strings-stay-stale-and)
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )