diff --git a/.claude/settings.json b/.claude/settings.json index ac5df3c1f3..2c7b88bf8d 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,9 +1,86 @@ { - "permissions": { - "allow": [ - "Bash(pnpx vitest run --testPathPattern=\"draftCacheV2.property\")", - "Bash(pnpx vitest run \"draftCacheV2.property\")", - "Bash(node -e \"const fc = require\\(''fast-check''\\); console.log\\(Object.keys\\(fc\\).filter\\(k => k.includes\\(''string''\\)\\).join\\('', ''\\)\\)\")" + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "if": "Bash(tsc *)", + "command": "echo 'Use `pnpm typecheck` instead of running tsc directly.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(vue-tsc *)", + "command": "echo 'Use `pnpm typecheck` instead of running vue-tsc directly.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(npx tsc *)", + "command": "echo 'Use `pnpm typecheck` instead of running tsc via npx.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(pnpx tsc *)", + "command": "echo 'Use `pnpm typecheck` instead of running tsc via pnpx.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(pnpm exec tsc *)", + "command": "echo 'Use `pnpm typecheck` instead of `pnpm exec tsc`.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(npx vitest *)", + "command": "echo 'Use `pnpm test:unit` (or `pnpm test:unit -- `) instead of npx vitest.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(pnpx vitest *)", + "command": "echo 'Use `pnpm test:unit` (or `pnpm test:unit -- `) instead of pnpx vitest.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(npx eslint *)", + "command": "echo 'Use `pnpm lint` or `pnpm lint:fix` instead of npx eslint.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(pnpx eslint *)", + "command": "echo 'Use `pnpm lint` or `pnpm lint:fix` instead of pnpx eslint.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(npx prettier *)", + "command": "echo 'This project uses oxfmt, not prettier. Use `pnpm format` or `pnpm format:check`.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(pnpx prettier *)", + "command": "echo 'This project uses oxfmt, not prettier. Use `pnpm format` or `pnpm format:check`.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(npx oxlint *)", + "command": "echo 'Use `pnpm oxlint` instead of npx oxlint.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(npx stylelint *)", + "command": "echo 'Use `pnpm stylelint` instead of npx stylelint.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(npx knip *)", + "command": "echo 'Use `pnpm knip` instead of npx knip.' >&2 && exit 2" + }, + { + "type": "command", + "if": "Bash(pnpx knip *)", + "command": "echo 'Use `pnpm knip` instead of pnpx knip.' >&2 && exit 2" + } + ] + } ] } } diff --git a/.claude/skills/backport-management/SKILL.md b/.claude/skills/backport-management/SKILL.md index d2ad90c072..138eb98a84 100644 --- a/.claude/skills/backport-management/SKILL.md +++ b/.claude/skills/backport-management/SKILL.md @@ -11,10 +11,11 @@ Cherry-pick backport management for Comfy-Org/ComfyUI_frontend stable release br 1. **Discover** — Collect candidates from Slack bot + git log gap (`reference/discovery.md`) 2. **Analyze** — Categorize MUST/SHOULD/SKIP, check deps (`reference/analysis.md`) -3. **Plan** — Order by dependency (leaf fixes first), group into waves per branch -4. **Execute** — Label-driven automation → worktree fallback for conflicts (`reference/execution.md`) -5. **Verify** — After each wave, verify branch integrity before proceeding -6. **Log & Report** — Generate session report with mermaid diagram (`reference/logging.md`) +3. **Human Review** — Present candidates in batches for interactive approval (see Interactive Approval Flow) +4. **Plan** — Order by dependency (leaf fixes first), group into waves per branch +5. **Execute** — Label-driven automation → worktree fallback for conflicts (`reference/execution.md`) +6. **Verify** — After each wave, verify branch integrity before proceeding +7. **Log & Report** — Generate session report (`reference/logging.md`) ## System Context @@ -37,16 +38,29 @@ Cherry-pick backport management for Comfy-Org/ComfyUI_frontend stable release br **Critical: Match PRs to the correct target branches.** -| Branch prefix | Scope | Example | -| ------------- | ------------------------------ | ----------------------------------------- | -| `cloud/*` | Cloud-hosted ComfyUI only | App mode, cloud auth, cloud-specific UI | -| `core/*` | Local/self-hosted ComfyUI only | Core editor, local workflows, node system | +| Branch prefix | Scope | Example | +| ------------- | ------------------------------ | ------------------------------------------------- | +| `cloud/*` | Cloud-hosted ComfyUI only | Team workspaces, cloud queue, cloud-only login | +| `core/*` | Local/self-hosted ComfyUI only | Core editor, local workflows, node system | +| Both | Shared infrastructure | App mode, Firebase auth (API nodes), payment URLs | -**⚠️ NEVER backport cloud-only PRs to `core/*` branches.** Cloud-only changes (app mode, cloud auth, cloud billing UI, cloud-specific API calls) are irrelevant to local users and waste effort. Before backporting any PR to a `core/*` branch, check: +### What Goes Where -- Does the PR title/description mention "app mode", "cloud", or cloud-specific features? -- Does the PR only touch files like `appModeStore.ts`, cloud auth, or cloud-specific components? -- If yes → skip for `core/*` branches (may still apply to `cloud/*` branches) +**Both core + cloud:** + +- **App mode** PRs — app mode is NOT cloud-only +- **Firebase auth** PRs — Firebase auth is on core for API nodes +- **Payment redirect** PRs — payment infrastructure shared +- **Bug fixes** touching shared components + +**Cloud-only (skip for core):** + +- Team workspaces +- Cloud queue virtualization +- Hide API key login +- Cloud-specific UI behind cloud feature flags + +**⚠️ NEVER backport cloud-only PRs to `core/*` branches.** But do NOT assume "app mode" or "Firebase" = cloud-only. Check the actual files changed. ## ⚠️ Gotchas (Learn from Past Sessions) @@ -67,6 +81,32 @@ The `pr-backport.yaml` action reports more conflicts than reality. `git cherry-p 12 or 27 conflicting files can be trivial (snapshots, new files). **Categorize conflicts first**, then decide. See Conflict Triage below. +### Accept-Theirs Can Produce Broken Hybrids + +When a PR **rewrites a component** (e.g., PrimeVue → Reka UI), the accept-theirs regex produces a broken mix of old and new code. The template may reference new APIs while the script still has old imports, or vice versa. + +**Detection:** Content conflicts with 4+ conflict markers in a single `.vue` file, especially when imports change between component libraries. + +**Fix:** Instead of accept-theirs regex, use `git show MERGE_SHA:path/to/file > path/to/file` to get the complete correct version from the merge commit on main. This bypasses the conflict entirely. + +### Cherry-Picks Can Reference Missing Dependencies + +When PR A on main depends on code introduced by PR B (which was merged before A), cherry-picking A brings in code that references B's additions. The cherry-pick succeeds but the branch is broken. + +**Common pattern:** Composables, component files, or type definitions introduced by an earlier PR and used by the cherry-picked PR. + +**Detection:** `pnpm typecheck` fails with "Cannot find module" or "is not defined" errors after cherry-pick. + +**Fix:** Use `git show MERGE_SHA:path/to/missing/file > path/to/missing/file` to bring the missing files from main. Always verify with typecheck. + +### Use `--no-verify` for Worktree Pushes + +Husky hooks fail in worktrees (can't find lint-staged config). Always use `git push --no-verify` and `git commit --no-verify` when working in `/tmp/` worktrees. + +### Automation Success Varies Wildly by Branch + +In the 2026-04-06 session: core/1.42 got 18/26 auto-PRs, cloud/1.42 got only 1/25. The cloud branch has more divergence. **Always plan for manual fallback** — don't assume automation will handle most PRs. + ## Conflict Triage **Always categorize before deciding to skip. High conflict count ≠ hard conflicts.** @@ -77,6 +117,8 @@ The `pr-backport.yaml` action reports more conflicts than reality. `git cherry-p | **Modify/delete (new file)** | PR introduces files not on target | `git add $FILE` — keep the new file | | **Modify/delete (removed)** | Target removed files the PR modifies | `git rm $FILE` — file no longer relevant | | **Content conflicts** | Marker-based (`<<<<<<<`) | Accept theirs via python regex (see below) | +| **Component rewrites** | 4+ markers in `.vue`, library change | Use `git show SHA:path > path` — do NOT accept-theirs | +| **Import-only conflicts** | Only import lines differ | Keep both imports if both used; remove unused after | | **Add/add** | Both sides added same file | Accept theirs, verify no logic conflict | | **Locale/JSON files** | i18n key additions | Accept theirs, validate JSON after | @@ -103,7 +145,7 @@ Skip these without discussion: - **Test-only / lint rule changes** — Not user-facing - **Revert pairs** — If PR A reverted by PR B, skip both. If fixed version (PR C) exists, backport only C. - **Features not on target branch** — e.g., Painter, GLSLShader, appModeStore on core/1.40 -- **Cloud-only PRs on core/\* branches** — App mode, cloud auth, cloud billing. These only affect cloud-hosted ComfyUI. +- **Cloud-only PRs on core/\* branches** — Team workspaces, cloud queue, cloud-only login. (Note: app mode and Firebase auth are NOT cloud-only — see Branch Scope Rules) ## Wave Verification @@ -122,6 +164,18 @@ git worktree remove /tmp/verify-TARGET --force If typecheck or tests fail, stop and investigate before continuing. A broken branch after wave N means all subsequent waves will compound the problem. +### Fix PRs Are Normal + +Expect to create 1 fix PR per branch after verification. Common issues: + +1. **Component rewrite hybrids** — accept-theirs produced broken `.vue` files. Fix: overwrite with correct version from merge commit via `git show SHA:path > path` +2. **Missing dependency files** — cherry-pick brought in code referencing composables/components not on the branch. Fix: add missing files from merge commit +3. **Missing type properties** — cherry-picked code uses interface properties not yet on the branch (e.g., `key` on `ConfirmDialogOptions`). Fix: add the property to the interface +4. **Unused imports** — conflict resolution kept imports that the branch doesn't use. Fix: remove unused imports +5. **Wrong types from conflict resolution** — e.g., `{ top: number; right: number }` vs `{ top: number; left: number }`. Fix: match the return type of the actual function + +Create a fix PR on a branch from the target, verify typecheck passes, then merge with `--squash --admin`. + ### Never Admin-Merge Without CI In a previous bulk session, all 69 backport PRs were merged with `gh pr merge --squash --admin`, bypassing required CI checks. This shipped 3 test failures to a release branch. **Lesson: `--admin` skips all branch protection, including required status checks.** Only use `--admin` after confirming CI has passed (e.g., `gh pr checks $PR` shows all green), or rely on auto-merge (`--auto --squash`) which waits for CI by design. @@ -135,6 +189,43 @@ Large backport sessions (50+ PRs) are expensive and error-prone. Prefer continuo - Reserve session-style bulk backporting for catching up after gaps - When a release branch is created, immediately start the continuous process +## Interactive Approval Flow + +After analysis, present ALL candidates (MUST, SHOULD, and borderline) to the human for interactive review before execution. Do not write a static decisions.md — collect approvals in conversation. + +### Batch Presentation + +Present PRs in batches of 5-10, grouped by theme (visual bugs, interaction bugs, cloud/auth, data correctness, etc.). Use this table format: + +``` + # | PR | Title | Target | Rec | Context +----+--------+------------------------------------------+---------------+------+-------- + 1 | #12345 | fix: broken thing | core+cloud/42 | Y | Description here. Why it matters. Agent reasoning. + 2 | #12346 | fix: another issue | core/42 | N | Only affects removed feature. Not on target branch. +``` + +Each row includes: + +- PR number and title +- Target branches +- Agent recommendation: `Rec: Y` or `Rec: N` with brief reasoning +- 2-3 sentence context: what the PR does, why it matters (or doesn't) + +### Human Response Format + +- `Y` — approve for backport +- `N` — skip +- `?` — investigate (agent shows PR description, files changed, detailed take, then re-asks) +- Any freeform question or comment triggers discussion before moving on +- Bulk responses accepted (e.g. `1 Y, 2 Y, 3 N, 4 ?`) + +### Rules + +- ALL candidates are reviewed, not just MUST items +- When human responds `?`, show the PR description, files changed, and agent's detailed analysis, then re-ask for their decision +- When human asks a question about a PR, answer with context and recommendation, then wait for their decision +- Do not proceed to execution until all batches are reviewed and every candidate has a Y or N + ## Quick Reference ### Label-Driven Automation (default path) @@ -150,13 +241,96 @@ gh api repos/Comfy-Org/ComfyUI_frontend/issues/$PR/labels \ ```bash git worktree add /tmp/backport-$BRANCH origin/$BRANCH cd /tmp/backport-$BRANCH + +# For each PR: +git fetch origin $BRANCH git checkout -b backport-$PR-to-$BRANCH origin/$BRANCH git cherry-pick -m 1 $MERGE_SHA -# Resolve conflicts, push, create PR, merge +# Resolve conflicts (see Conflict Triage) +git push origin backport-$PR-to-$BRANCH --no-verify +gh pr create --base $BRANCH --head backport-$PR-to-$BRANCH \ + --title "[backport $BRANCH] $TITLE (#$PR)" \ + --body "Backport of #$PR. [conflict notes]" +gh pr merge $NEW_PR --squash --admin +sleep 25 ``` +### Efficient Batch: Test-Then-Resolve Pattern + +When many PRs need manual cherry-pick (e.g., cloud branches), test all first: + +```bash +cd /tmp/backport-$BRANCH +for pr in "${ORDER[@]}"; do + git checkout -b test-$pr origin/$BRANCH + if git cherry-pick -m 1 $SHA 2>/dev/null; then + echo "CLEAN: $pr" + else + echo "CONFLICT: $pr" + git cherry-pick --abort + fi + git checkout --detach HEAD + git branch -D test-$pr +done +``` + +Then process clean PRs in a batch loop, conflicts individually. + ### PR Title Convention ``` [backport TARGET_BRANCH] Original Title (#ORIGINAL_PR) ``` + +## Final Deliverables (Slack-Compatible) + +After execution completes, generate two files in `~/temp/backport-session/`. Both must be **Slack-compatible plain text** — no emojis, no markdown tables, no headers (`#`), no bold (`**`), no inline code. Use plain dashes, indentation, and line breaks only. + +### 1. Author Accountability Report + +File: `backport-author-accountability.md` + +Lists all backported PRs grouped by original author (via `gh pr view $PR --json author`). Surfaces who should be self-labeling. + +``` +Backport Session YYYY-MM-DD -- PRs that should have been labeled by authors + +- author-login + - #1234 fix: short title + - #5678 fix: another title +- other-author + - #9012 fix: some other fix +``` + +Authors sorted alphabetically, 4-space indent for nested items. + +### 2. Slack Status Update + +File: `slack-status-update.md` + +A shareable summary of the session. Structure: + +``` +Backport session complete -- YYYY-MM-DD + +[1-sentence summary: N PRs backported to which branches. All pass typecheck.] + +Branches updated: +- core/X.XX: N PRs + N fix PRs (N auto, N manual) +- cloud/X.XX: N PRs + N fix PRs (N auto, N manual) +- ... + +N total PRs created and merged (N backports + N fix PRs). + +Notable fixes included: +- [category]: [list of fixes] +- ... + +Conflict patterns encountered: +- [pattern and how it was resolved] +- ... + +N authors had PRs backported. See author accountability list for details. +``` + +No emojis, no tables, no bold, no headers. Plain text that pastes cleanly into Slack. diff --git a/.claude/skills/backport-management/reference/analysis.md b/.claude/skills/backport-management/reference/analysis.md index d62cd1c935..dd6a41b394 100644 --- a/.claude/skills/backport-management/reference/analysis.md +++ b/.claude/skills/backport-management/reference/analysis.md @@ -23,10 +23,10 @@ For SHOULD items with conflicts: if conflict resolution requires more than trivi **Before categorizing, filter by branch scope:** -| Target branch | Skip if PR is... | -| ------------- | ------------------------------------------------------------------- | -| `core/*` | Cloud-only (app mode, cloud auth, cloud billing, cloud-specific UI) | -| `cloud/*` | Local-only features not present on cloud branch | +| Target branch | Skip if PR is... | +| ------------- | ----------------------------------------------------------------------------------------------------------------- | +| `core/*` | Cloud-only (team workspaces, cloud queue, cloud-only login). Note: app mode and Firebase auth are NOT cloud-only. | +| `cloud/*` | Local-only features not present on cloud branch | Cloud-only PRs backported to `core/*` are wasted effort — `core/*` branches serve local/self-hosted users who never see cloud features. Check PR titles, descriptions, and files changed for cloud-specific indicators. @@ -61,8 +61,6 @@ done ## Human Review Checkpoint -Present decisions.md before execution. Include: +Use the Interactive Approval Flow (see SKILL.md) to review all candidates interactively. Do not write a static decisions.md for the human to edit — instead, present batches of 5-10 PRs with context and recommendations, and collect Y/N/? responses in conversation. -1. All MUST/SHOULD/SKIP categorizations with rationale -2. Questions for human (feature existence, scope, deps) -3. Estimated effort per branch +All candidates must be reviewed (MUST, SHOULD, and borderline items), not just a subset. diff --git a/.claude/skills/backport-management/reference/execution.md b/.claude/skills/backport-management/reference/execution.md index 44de5f66f0..2bc6f9f78a 100644 --- a/.claude/skills/backport-management/reference/execution.md +++ b/.claude/skills/backport-management/reference/execution.md @@ -73,14 +73,22 @@ for PR in ${CONFLICT_PRS[@]}; do git cherry-pick -m 1 $MERGE_SHA # If conflict — NEVER skip based on file count alone! - # Categorize conflicts first: binary PNGs, modify/delete, content, add/add + # Categorize conflicts first: binary PNGs, modify/delete, content, add/add, component rewrites # See SKILL.md Conflict Triage table for resolution per type. + # For component rewrites (4+ markers in a .vue file, library migration): + # DO NOT use accept-theirs regex — it produces broken hybrids. + # Instead, use the complete file from the merge commit: + # git show $MERGE_SHA:path/to/file > path/to/file + + # For simple content conflicts, accept theirs: + # python3 -c "import re; ..." + # Resolve all conflicts, then: git add . GIT_EDITOR=true git cherry-pick --continue - git push origin backport-$PR-to-TARGET + git push origin backport-$PR-to-TARGET --no-verify NEW_PR=$(gh pr create --base TARGET_BRANCH --head backport-$PR-to-TARGET \ --title "[backport TARGET] TITLE (#$PR)" \ --body "Backport of #$PR..." | grep -oP '\d+$') @@ -114,7 +122,30 @@ source ~/.nvm/nvm.sh && nvm use 24 && pnpm install && pnpm typecheck && pnpm tes git worktree remove /tmp/verify-TARGET --force ``` -If verification fails, stop and fix before proceeding to the next wave. Do not compound problems across waves. +If verification fails, **do not skip** — create a fix PR: + +```bash +# Stay in the verify worktree +git checkout -b fix-backport-TARGET origin/TARGET_BRANCH + +# Common fixes: +# 1. Component rewrite hybrids: overwrite with merge commit version +git show MERGE_SHA:path/to/Component.vue > path/to/Component.vue + +# 2. Missing dependency files +git show MERGE_SHA:path/to/missing.ts > path/to/missing.ts + +# 3. Missing type properties: edit the interface +# 4. Unused imports: delete the import lines + +git add -A +git commit --no-verify -m "fix: resolve backport typecheck issues on TARGET" +git push origin fix-backport-TARGET --no-verify +gh pr create --base TARGET --head fix-backport-TARGET --title "fix: resolve backport typecheck issues on TARGET" --body "..." +gh pr merge $PR --squash --admin +``` + +Do not proceed to the next branch until typecheck passes. ## Conflict Resolution Patterns @@ -142,7 +173,35 @@ git rm $FILE git checkout --theirs $FILE && git add $FILE ``` -### 4. Locale Files +### 4. Component Rewrites (DO NOT accept-theirs) + +When a PR completely rewrites a component (e.g., PrimeVue → Reka UI), accept-theirs produces +a broken hybrid with mismatched template/script sections. + +```bash +# Use the complete correct file from the merge commit instead: +git show $MERGE_SHA:src/components/input/MultiSelect.vue > src/components/input/MultiSelect.vue +git show $MERGE_SHA:src/components/input/SingleSelect.vue > src/components/input/SingleSelect.vue +git add src/components/input/MultiSelect.vue src/components/input/SingleSelect.vue +``` + +**Detection:** 4+ conflict markers in a single `.vue` file, imports changing between component +libraries (PrimeVue → Reka UI, etc.), template structure completely different on each side. + +### 5. Missing Dependencies After Cherry-Pick + +Cherry-picks can succeed but leave the branch broken because the PR's code on main +references composables/components introduced by an earlier PR. + +```bash +# Add the missing file from the merge commit: +git show $MERGE_SHA:src/composables/queue/useJobDetailsHover.ts > src/composables/queue/useJobDetailsHover.ts +git show $MERGE_SHA:src/components/builder/BuilderSaveDialogContent.vue > src/components/builder/BuilderSaveDialogContent.vue +``` + +**Detection:** `pnpm typecheck` fails with "Cannot find module" or "X is not defined" after cherry-pick succeeds cleanly. + +### 6. Locale Files Usually adding new i18n keys — accept theirs, validate JSON: @@ -176,8 +235,14 @@ gh pr checks $PR --watch --fail-fast && gh pr merge $PR --squash --admin 8. **Always validate JSON** after resolving locale file conflicts 9. **Dep refresh PRs** — skip on stable branches. Risk of transitive dep regressions outweighs audit cleanup. Cherry-pick individual CVE fixes instead. 10. **Verify after each wave** — run `pnpm typecheck && pnpm test:unit` on the target branch after merging a batch. Catching breakage early prevents compounding errors. -11. **Cloud-only PRs don't belong on core/\* branches** — app mode, cloud auth, and cloud-specific UI changes are irrelevant to local users. Always check PR scope against branch scope before backporting. +11. **App mode and Firebase auth are NOT cloud-only** — they go to both core and cloud branches. Only team workspaces, cloud queue, and cloud-specific login are cloud-only. 12. **Never admin-merge without CI** — `--admin` bypasses all branch protections including required status checks. A bulk session of 69 admin-merges shipped 3 test failures. Always wait for CI to pass first, or use `--auto --squash` which waits by design. +13. **Accept-theirs regex breaks component rewrites** — when a PR migrates between component libraries (PrimeVue → Reka UI), the regex produces a broken hybrid. Use `git show SHA:path > path` to get the complete correct version instead. +14. **Cherry-picks can silently bring in missing-dependency code** — if PR A references a composable introduced by PR B, cherry-picking A succeeds but typecheck fails. Always run typecheck after each wave and add missing files from the merge commit. +15. **Fix PRs are expected** — plan for 1 fix PR per branch to resolve typecheck issues from conflict resolutions. This is normal, not a failure. +16. **Use `--no-verify` in worktrees** — husky hooks fail in `/tmp/` worktrees. Always push/commit with `--no-verify`. +17. **Automation success varies by branch** — core/1.42 got 18/26 auto-PRs (69%), cloud/1.42 got 1/25 (4%). Cloud branches diverge more. Plan for manual fallback. +18. **Test-then-resolve pattern** — for branches with low automation success, run a dry-run loop to classify clean vs conflict PRs before processing. This is much faster than resolving conflicts serially. ## CI Failure Triage diff --git a/.claude/skills/backport-management/reference/logging.md b/.claude/skills/backport-management/reference/logging.md index 4c78ba4b16..77ef0aa1c8 100644 --- a/.claude/skills/backport-management/reference/logging.md +++ b/.claude/skills/backport-management/reference/logging.md @@ -2,26 +2,25 @@ ## During Execution -Maintain `execution-log.md` with per-branch tables: +Maintain `execution-log.md` with per-branch tables (this is internal, markdown tables are fine here): ```markdown -| PR# | Title | CI Status | Status | Backport PR | Notes | -| ----- | ----- | ------------------------------ | --------------------------------- | ----------- | ------- | -| #XXXX | Title | ✅ Pass / ❌ Fail / ⏳ Pending | ✅ Merged / ⏭️ Skip / ⏸️ Deferred | #YYYY | Details | +| PR# | Title | Status | Backport PR | Notes | +| ----- | ----- | ------ | ----------- | ------- | +| #XXXX | Title | merged | #YYYY | Details | ``` ## Wave Verification Log -Track verification results per wave: +Track verification results per wave within execution-log.md: ```markdown -## Wave N Verification — TARGET_BRANCH +Wave N Verification -- TARGET_BRANCH - PRs merged: #A, #B, #C -- Typecheck: ✅ Pass / ❌ Fail -- Unit tests: ✅ Pass / ❌ Fail +- Typecheck: pass / fail +- Fix PR: #YYYY (if needed) - Issues found: (if any) -- Human review needed: (list any non-trivial conflict resolutions) ``` ## Session Report Template @@ -63,40 +62,42 @@ Track verification results per wave: - Feature branches that need tracking for future sessions? ``` -## Final Deliverable: Visual Summary +## Final Deliverables -At session end, generate a **mermaid diagram** showing all backported PRs organized by target branch and category (MUST/SHOULD), plus a summary table. Present this to the user as the final output. +After all branches are complete and verified, generate these files in `~/temp/backport-session/`: -```mermaid -graph TD - subgraph branch1["☁️ cloud/X.XX — N PRs"] - C1["#XXXX title"] - C2["#XXXX title"] - end +### 1. execution-log.md (internal) - subgraph branch2must["🔴 core/X.XX MUST — N PRs"] - M1["#XXXX title"] - end +Per-branch tables with PR#, title, status, backport PR#, notes. Markdown tables are fine — this is for internal tracking, not Slack. - subgraph branch2should["🟡 core/X.XX SHOULD — N PRs"] - S1["#XXXX-#XXXX N auto-merged"] - S2["#XXXX-#XXXX N manual picks"] - end +### 2. backport-author-accountability.md (Slack-compatible) - classDef cloudStyle fill:#1a3a5c,stroke:#4da6ff,color:#e0f0ff - classDef coreStyle fill:#1a4a2e,stroke:#4dff88,color:#e0ffe8 - classDef mustStyle fill:#5c1a1a,stroke:#ff4d4d,color:#ffe0e0 - classDef shouldStyle fill:#4a3a1a,stroke:#ffcc4d,color:#fff5e0 -``` +See SKILL.md "Final Deliverables" section. Plain text, no emojis/tables/headers/bold. Authors sorted alphabetically with PRs nested under each. -Use the `mermaid` tool to render this diagram and present it alongside the summary table as the session's final deliverable. +### 3. slack-status-update.md (Slack-compatible) + +See SKILL.md "Final Deliverables" section. Plain text summary that pastes cleanly into Slack. Includes branch counts, notable fixes, conflict patterns, author count. + +## Slack Formatting Rules + +Both shareable files (author accountability + status update) must follow these rules: + +- No emojis (no checkmarks, no arrows, no icons) +- No markdown tables (use plain lists with dashes) +- No headers (no # or ##) +- No bold (\*_) or italic (_) +- No inline code backticks +- Use -- instead of em dash +- Use plain dashes (-) for lists with 4-space indent for nesting +- Line breaks between sections for readability + +These files should paste directly into a Slack message and look clean. ## Files to Track -- `candidate_list.md` — all candidates per branch -- `decisions.md` — MUST/SHOULD/SKIP with rationale -- `wave-plan.md` — execution order -- `execution-log.md` — real-time status -- `backport-session-report.md` — final summary +All in `~/temp/backport-session/`: -All in `~/temp/backport-session/`. +- `execution-plan.md` -- approved PRs with merge SHAs (input) +- `execution-log.md` -- real-time status with per-branch tables (internal) +- `backport-author-accountability.md` -- PRs grouped by author (Slack-compatible) +- `slack-status-update.md` -- session summary (Slack-compatible) diff --git a/.claude/skills/hardening-flaky-e2e-tests/SKILL.md b/.claude/skills/hardening-flaky-e2e-tests/SKILL.md new file mode 100644 index 0000000000..d45a6f18fc --- /dev/null +++ b/.claude/skills/hardening-flaky-e2e-tests/SKILL.md @@ -0,0 +1,246 @@ +--- +name: hardening-flaky-e2e-tests +description: 'Diagnoses and fixes flaky Playwright e2e tests by replacing race-prone patterns with retry-safe alternatives. Use when triaging CI flakes, hardening spec files, fixing timing races, or asked to stabilize browser tests. Triggers on: flaky, flake, harden, stabilize, race condition in e2e, intermittent failure.' +--- + +# Hardening Flaky E2E Tests + +Fix flaky Playwright specs by identifying race-prone patterns and replacing them with retry-safe alternatives. This skill covers diagnosis, pattern matching, and mechanical transforms — not writing new tests (see `writing-playwright-tests` for that). + +## Workflow + +### 1. Gather CI Evidence + +```bash +gh run list --workflow=ci-test.yaml --limit=5 +gh run download -n playwright-report +``` + +- Open `report.json` and search for `"status": "flaky"` entries. +- Collect file paths, test titles, and error messages. +- Do NOT trust green checks alone — flaky tests that passed on retry still need fixing. +- Use `error-context.md`, traces, and page snapshots before editing code. +- Pull the newest run after each push instead of assuming the flaky set is unchanged. + +### 2. Classify the Flake + +Read the failing assertion and match it against the pattern table. Most flakes fall into one of these categories: + +| # | Pattern | Signature in Code | Fix | +| --- | ------------------------------------- | --------------------------------------------------------- | ---------------------------------------------------------------- | +| 1 | **Snapshot-then-assert** | `expect(await evaluate()).toBe(x)` | `await expect.poll(() => evaluate()).toBe(x)` | +| 2 | **Immediate count** | `const n = await loc.count(); expect(n).toBe(3)` | `await expect(loc).toHaveCount(3)` | +| 3 | **nextFrame after menu click** | `clickMenuItem(x); nextFrame()` | `clickMenuItem(x); contextMenu.waitForHidden()` | +| 4 | **Tight poll timeout** | `expect.poll(..., { timeout: 250 })` | ≥2000 ms; prefer default 5000 ms | +| 5 | **Immediate evaluate after mutation** | `setSetting(k, v); expect(await evaluate()).toBe(x)` | `await expect.poll(() => evaluate()).toBe(x)` | +| 6 | **Screenshot without readiness** | `loadWorkflow(); nextFrame(); toHaveScreenshot()` | `waitForNodes()` or poll state first | +| 7 | **Non-deterministic node order** | `getNodeRefsByType('X')[0]` with >1 match | `getNodeRefById(id)` or guard `toHaveLength(1)` | +| 8 | **Fake readiness helper** | Helper clicks but doesn't assert state | Remove; poll the actual value | +| 9 | **Immediate graph state after drop** | `expect(await getLinkCount()).toBe(1)` | `await expect.poll(() => getLinkCount()).toBe(1)` | +| 10 | **Immediate boundingBox/layout read** | `const box = await loc.boundingBox(); expect(box!.width)` | `await expect.poll(() => loc.boundingBox().then(b => b?.width))` | + +### 3. Apply the Transform + +#### Rule: Choose the Smallest Correct Assertion + +- **Locator state** → use built-in retrying assertions: `toBeVisible()`, `toHaveText()`, `toHaveCount()`, `toHaveClass()` +- **Single async value** → `expect.poll(() => asyncFn()).toBe(expected)` +- **Multiple assertions that must settle together** → `expect(async () => { ... }).toPass()` +- **Never** use `waitForTimeout()` to hide a race. + +```typescript +// ✅ Single value — use expect.poll +await expect + .poll(() => comfyPage.page.evaluate(() => window.app!.graph.links.length)) + .toBe(3) + +// ✅ Locator count — use toHaveCount +await expect(comfyPage.page.locator('.dom-widget')).toHaveCount(2) + +// ✅ Multiple conditions — use toPass +await expect(async () => { + expect(await node1.getValue()).toBe('foo') + expect(await node2.getValue()).toBe('bar') +}).toPass({ timeout: 5000 }) +``` + +#### Rule: Wait for the Real Readiness Boundary + +Visible is not always ready. Prefer user-facing assertions when possible; poll internal state only when there is no UI surface to assert on. + +Common readiness boundaries: + +| After this action... | Wait for... | +| -------------------------------------- | ------------------------------------------------------------ | +| Canvas interaction (drag, click node) | `await comfyPage.nextFrame()` | +| Menu item click | `await contextMenu.waitForHidden()` | +| Workflow load | `await comfyPage.workflow.loadWorkflow(...)` (built-in wait) | +| Settings write | Poll the setting value with `expect.poll()` | +| Node pin/bypass/collapse toggle | `await expect.poll(() => nodeRef.isPinned()).toBe(true)` | +| Graph mutation (add/remove node, link) | Poll link/node count | +| Clipboard write | Poll pasted value | +| Screenshot | Ensure nodes are rendered: `waitForNodes()` or poll state | + +#### Rule: Expose Locators for Retrying Assertions + +When a helper returns a count via `await loc.count()`, callers can't use `toHaveCount()`. Expose the underlying `Locator` as a getter so callers choose between: + +```typescript +// Helper exposes locator +get domWidgets(): Locator { + return this.page.locator('.dom-widget') +} + +// Caller uses retrying assertion +await expect(comfyPage.domWidgets).toHaveCount(2) +``` + +Replace count methods with locator getters so callers can use retrying assertions directly. + +#### Rule: Fix Check-then-Act Races in Helpers + +```typescript +// ❌ Race: count can change between check and waitFor +const count = await locator.count() +if (count > 0) { + await locator.waitFor({ state: 'hidden' }) +} + +// ✅ Direct: waitFor handles both cases +await locator.waitFor({ state: 'hidden' }) +``` + +#### Rule: Remove force:true from Clicks + +`force: true` bypasses actionability checks, hiding real animation/visibility races. Remove it and fix the underlying timing issue. + +```typescript +// ❌ Hides the race +await closeButton.click({ force: true }) + +// ✅ Surfaces the real issue — fix with proper wait +await closeButton.click() +await dialog.waitForHidden() +``` + +#### Rule: Handle Non-deterministic Element Order + +When `getNodeRefsByType` returns multiple nodes, the order is not guaranteed. Don't use index `[0]` blindly. + +```typescript +// ❌ Assumes order +const node = (await comfyPage.nodeOps.getNodeRefsByType('CLIPTextEncode'))[0] + +// ✅ Find by ID or proximity +const nodes = await comfyPage.nodeOps.getNodeRefsByType('CLIPTextEncode') +let target = nodes[0] +for (const n of nodes) { + const pos = await n.getPosition() + if (Math.abs(pos.y - expectedY) < minDist) target = n +} +``` + +Or guard the assumption: + +```typescript +const nodes = await comfyPage.nodeOps.getNodeRefsByType('CLIPTextEncode') +expect(nodes).toHaveLength(1) +const node = nodes[0] +``` + +#### Rule: Use toPass for Timing-sensitive Dismiss Guards + +Some UI elements (e.g. LiteGraph's graphdialog) have built-in dismiss delays. Retry the entire dismiss action: + +```typescript +// ✅ Retry click+assert together +await expect(async () => { + await comfyPage.canvas.click({ position: { x: 10, y: 10 } }) + await expect(dialog).toBeHidden({ timeout: 500 }) +}).toPass({ timeout: 5000 }) +``` + +### 4. Keep Changes Narrow + +- Shared helpers should drive setup to a stable boundary. +- Do not encode one-spec timing assumptions into generic helpers. +- If a race only matters to one spec, prefer a local wait in that spec. +- If a helper fails before the real test begins, remove or relax the brittle precondition and let downstream UI interaction prove readiness. + +### 5. Verify Narrowly + +```bash +# Targeted rerun with repetition +pnpm test:browser:local -- browser_tests/tests/myFile.spec.ts --repeat-each 10 + +# Single test by line number (avoids grep quoting issues on Windows) +pnpm test:browser:local -- browser_tests/tests/myFile.spec.ts:42 +``` + +- Use `--repeat-each 10` for targeted flake verification (use 20 for single test cases). +- Verify with the smallest command that exercises the flaky path. + +### 6. Watch CI E2E Runs + +After pushing, use `gh` to monitor the E2E workflow: + +```bash +# Find the run for the current branch +gh run list --workflow="CI: Tests E2E" --branch=$(git branch --show-current) --limit=1 + +# Watch it live (blocks until complete, streams logs) +gh run watch + +# One-liner: find and watch the latest E2E run for the current branch +gh run list --workflow="CI: Tests E2E" --branch=$(git branch --show-current) --limit=1 --json databaseId --jq ".[0].databaseId" | xargs gh run watch +``` + +On Windows (PowerShell): + +```powershell +# One-liner equivalent +gh run watch (gh run list --workflow="CI: Tests E2E" --branch=$(git branch --show-current) --limit=1 --json databaseId --jq ".[0].databaseId") +``` + +After the run completes: + +```bash +# Download the Playwright report artifact +gh run download -n playwright-report + +# View the run summary in browser +gh run view --web +``` + +Also watch the unit test workflow in parallel if you changed helpers: + +```bash +gh run list --workflow="CI: Tests Unit" --branch=$(git branch --show-current) --limit=1 +``` + +### 7. Pre-merge Checklist + +Before merging a flaky-test fix, confirm: + +- [ ] The latest CI artifact was inspected directly +- [ ] The root cause is stated as a race or readiness mismatch +- [ ] The fix waits on the real readiness boundary +- [ ] The assertion primitive matches the job (poll vs toHaveCount vs toPass) +- [ ] The fix stays local unless a shared helper truly owns the race +- [ ] Local verification uses a targeted rerun +- [ ] No behavioral changes to the test — only timing/retry strategy updated + +## Local Noise — Do Not Fix + +These are local distractions, not CI root causes: + +- Missing local input fixture files required by the test path +- Missing local models directory +- Teardown `EPERM` while restoring the local browser-test user data directory +- Local screenshot baseline differences on Windows + +Rules: + +- First confirm whether it blocks the exact flaky path under investigation. +- Do not commit temporary local assets used only for verification. +- Do not commit local screenshot baselines. diff --git a/.claude/skills/ticket-intake/SKILL.md b/.claude/skills/ticket-intake/SKILL.md deleted file mode 100644 index f01c7f763d..0000000000 --- a/.claude/skills/ticket-intake/SKILL.md +++ /dev/null @@ -1,361 +0,0 @@ ---- -name: ticket-intake -description: 'Parse ticket URL (Notion or GitHub), extract all data, initialize pipeline run. Use when starting work on a new ticket or when asked to pick up a ticket.' ---- - -# Ticket Intake - -Parses a ticket URL from supported sources (Notion or GitHub), extracts all relevant information, and creates a ticket in the pipeline API. - -> **🚨 CRITICAL REQUIREMENT**: This skill MUST register the ticket in the Pipeline API and update the source (Notion/GitHub). If these steps are skipped, the entire pipeline breaks. See [Mandatory API Calls](#mandatory-api-calls-execute-all-three) below. - -## Supported Sources - -| Source | URL Pattern | Provider File | -| ------ | --------------------------------------------------- | --------------------- | -| Notion | `https://notion.so/...` `https://www.notion.so/...` | `providers/notion.md` | -| GitHub | `https://github.com/{owner}/{repo}/issues/{n}` | `providers/github.md` | - -## Quick Start - -When given a ticket URL: - -1. **Detect source type** from URL pattern -2. **Load provider-specific logic** from `providers/` directory -3. Fetch ticket content via appropriate API -4. Extract and normalize properties to common schema -5. **Register ticket in pipeline API** ← MANDATORY -6. **Update source** (Notion status / GitHub comment) ← MANDATORY -7. **Run verification script** to confirm API registration -8. Output summary and handoff to `research-orchestrator` - -## Configuration - -Uses the **production API** by default. No configuration needed for read operations. - -**Defaults (no setup required):** - -- API URL: `https://api-gateway-856475788601.us-central1.run.app` -- Read-only endpoints at `/public/*` require no authentication - -**For write operations** (transitions, creating tickets), set: - -```bash -export PIPELINE_API_KEY="..." # Get from GCP Secret Manager or ask admin -``` - -**Optional (for local working artifacts):** - -```bash -PIPELINE_DIR="${PIPELINE_DIR:-$HOME/repos/ticket-to-pr-pipeline}" -``` - -## Mandatory API Calls (Execute ALL Three) - -**⚠️ These three API calls are the ENTIRE POINT of this skill. Without them, the ticket is invisible to the pipeline, downstream skills will fail, and Notion status won't update.** - -**You MUST make these HTTP requests.** Use `curl` from bash — do not just read this as documentation. - -### Call 1: Create Ticket - -```bash -API_URL="${PIPELINE_API_URL:-https://api-gateway-856475788601.us-central1.run.app}" -API_KEY="${PIPELINE_API_KEY}" - -curl -s -X POST "${API_URL}/v1/tickets" \ - -H "Authorization: Bearer ${API_KEY}" \ - -H "Content-Type: application/json" \ - -H "X-Agent-ID: ${AGENT_ID:-amp-agent}" \ - -d '{ - "notion_page_id": "NOTION_PAGE_UUID_HERE", - "title": "TICKET_TITLE_HERE", - "source": "notion", - "metadata": { - "description": "DESCRIPTION_HERE", - "priority": "High", - "labels": [], - "acceptanceCriteria": [] - } - }' -``` - -Save the returned `id` — you need it for the next two calls. - -### Call 2: Transition to RESEARCH - -```bash -TICKET_ID="id-from-step-1" - -curl -s -X POST "${API_URL}/v1/tickets/${TICKET_ID}/transition" \ - -H "Authorization: Bearer ${API_KEY}" \ - -H "Content-Type: application/json" \ - -H "X-Agent-ID: ${AGENT_ID:-amp-agent}" \ - -d '{ - "to_state": "RESEARCH", - "reason": "Intake complete, starting research" - }' -``` - -### Call 3: Queue Source Update - -```bash -curl -s -X POST "${API_URL}/v1/sync/queue" \ - -H "Authorization: Bearer ${API_KEY}" \ - -H "Content-Type: application/json" \ - -H "X-Agent-ID: ${AGENT_ID:-amp-agent}" \ - -d '{ - "ticket_id": "TICKET_ID_HERE", - "action": "update_status", - "payload": { "status": "In Progress" }, - "priority": "normal" - }' -``` - -> **Note:** The action MUST be `"update_status"` (not `"UPDATE_NOTION_STATUS"`). Valid actions: `update_status`, `update_pr_url`, `mark_done`. - -### TypeScript Equivalent (if using pipeline client) - -```typescript -import { PipelineClient } from '@pipeline/client' - -const client = new PipelineClient({ - apiUrl: - process.env.PIPELINE_API_URL || - 'https://api-gateway-856475788601.us-central1.run.app', - agentId: process.env.AGENT_ID! -}) - -const ticket = await client.createTicket({ - notion_page_id: pageId, - title: ticketTitle, - source: 'notion', - metadata: { description, priority, labels, acceptanceCriteria } -}) - -await client.transitionState( - ticket.id, - 'RESEARCH', - 'Intake complete, starting research' -) - -await client.queueSync(ticket.id, 'update_status', { status: 'In Progress' }) -``` - -## Workflow - -### Step 1: Detect Source Type - -Parse the URL to determine source: - -```javascript -if (url.includes('notion.so')) { - source = 'notion' - // Load providers/notion.md -} else if (url.match(/github\.com\/[^\/]+\/[^\/]+\/issues\/\d+/)) { - source = 'github' - // Load providers/github.md -} else { - // Error: Unsupported URL format -} -``` - -### Step 2: Load Provider and Fetch Data - -Read the appropriate provider file for source-specific instructions: - -- **Notion**: `providers/notion.md` - Uses Notion MCP, handles Slack links -- **GitHub**: `providers/github.md` - Uses `gh` CLI, handles Dosu comments - -Follow the provider's instructions for: - -- Fetching content -- Extracting properties -- **Updating the source** (Notion status → "In Progress", Assignee → pipeline owner) - -### Step 3: Normalize to Common Schema - -All providers must extract normalized ticket data following `schema.md`: - -```json -{ - "id": "abc12345", - "url": "https://...", - "source": "notion | github", - "title": "Ticket title", - "description": "Full description", - "status": "Not Started", - "assignee": "username", - "priority": "High", - "area": "UI", - "labels": ["bug", "frontend"], - "acceptanceCriteria": ["Criterion 1", "Criterion 2"], - "fetchedAt": "2024-01-15T10:30:00Z" -} -``` - -### Step 4: Register Ticket in Pipeline API (MANDATORY — DO NOT SKIP) - -**Execute all three API calls from [Mandatory API Calls](#mandatory-api-calls-execute-all-three) above.** - -This is not optional. This is not documentation. You MUST make these HTTP requests right now. - -1. `createTicket()` → save the returned ticket ID -2. `transitionState(id, 'RESEARCH')` → confirm state changed -3. `queueSync(id, 'update_status', { status: 'In Progress' })` → confirm queued - -**If any call fails**, retry once. If it still fails, report the error prominently — do NOT silently continue. - -### Step 5: Run Verification Script - -After making the API calls, run the verification script to confirm everything worked: - -```bash -bash scripts/verify-intake.sh TICKET_ID_OR_NOTION_PAGE_ID -``` - -**If the script is not available locally**, verify manually via the public API: - -```bash -curl -s "${API_URL}/public/tickets/${TICKET_ID}" | jq '{id, state, title, notion_page_id}' -``` - -Expected output: - -```json -{ - "id": "...", - "state": "RESEARCH", - "title": "...", - "notion_page_id": "..." -} -``` - -**If `state` is not `RESEARCH`, go back to Step 4 and complete the missing calls.** - -### Step 6: Output Summary and Handoff - -Print a clear summary: - -```markdown -## Ticket Intake Complete - -**Source:** Notion | GitHub -**Title:** [Ticket title] -**ID:** abc12345 -**Status:** In Progress (queued) -**Priority:** High -**Area:** UI - -### Description - -[Brief description or first 200 chars] - -### Acceptance Criteria - -- [ ] Criterion 1 -- [ ] Criterion 2 - -### Links - -- **Ticket:** [Original URL] -- **Slack:** [Slack thread content fetched via slackdump] (Notion only) - -### Pipeline - -- **API Ticket ID:** abc12345 -- **State:** RESEARCH -- **Verified:** ✅ (via verify-intake.sh or public API) -``` - -**After printing the summary, immediately handoff** to continue the pipeline. Use the `handoff` tool with all necessary context (ticket ID, source, title, description, slack context if any): - -> **Handoff goal:** "Continue pipeline for ticket {ID} ({title}). Ticket is in RESEARCH state. Load skill: `research-orchestrator` to begin research phase. Ticket data: source={source}, notion_page_id={pageId}, priority={priority}. {slack context summary if available}" - -**Do NOT wait for human approval to proceed.** The intake phase is complete — handoff immediately. - -## Error Handling - -### Unsupported URL - -``` -❌ Unsupported ticket URL format. - -Supported formats: -- Notion: https://notion.so/... or https://www.notion.so/... -- GitHub: https://github.com/{owner}/{repo}/issues/{number} - -Received: [provided URL] -``` - -### Provider-Specific Errors - -See individual provider files for source-specific error handling: - -- `providers/notion.md` - Authentication, page not found -- `providers/github.md` - Auth, rate limits, issue not found - -### Missing Properties - -Continue with available data and note what's missing: - -``` -⚠️ Some properties unavailable: -- Priority: not found (using default: Medium) -- Area: not found - -Proceeding with available data... -``` - -### API Call Failures - -``` -❌ Pipeline API call failed: {method} {endpoint} - Status: {status} - Error: {message} - -Retrying once... - -❌ Retry also failed. INTAKE IS INCOMPLETE. - The ticket was NOT registered in the pipeline. - Downstream skills will not work until this is fixed. -``` - -## Notes - -- This skill focuses ONLY on intake — it does not do research -- Slack thread content is fetched automatically via the `slackdump` skill — no manual copy-paste needed -- ALL API calls (createTicket, transitionState, queueSync) are MANDATORY — never skip them -- The `queueSync` action must be `"update_status"`, NOT `"UPDATE_NOTION_STATUS"` -- Pipeline state is tracked via the API, not local files -- Working artifacts (research-report.md, plan.md) can be saved locally to `$PIPELINE_DIR/runs/{ticket-id}/` -- The `source` field in the ticket determines which research strategies to use - -## API Client Reference - -### Available Methods - -| Method | Description | -| ----------------------------------------------------------- | ------------------------------------------------------------------- | -| `createTicket({ notion_page_id, title, source, metadata })` | Create a new ticket in the API | -| `getTicket(id)` | Retrieve a ticket by ID | -| `findByNotionId(notionPageId)` | Look up a ticket by its Notion page ID | -| `listTickets({ state, agent_id, limit, offset })` | List tickets with optional filters | -| `transitionState(id, state, reason)` | Move ticket to a new state (e.g., `'RESEARCH'`) | -| `setPRCreated(id, prUrl)` | Mark ticket as having a PR created | -| `queueSync(id, action, payload)` | Queue a sync action (`update_status`, `update_pr_url`, `mark_done`) | -| `registerBranch(id, branch, repo)` | Register working branch for automatic PR detection | - -### Error Handling - -```typescript -import { PipelineClient, PipelineAPIError } from '@pipeline/client'; - -try { - await client.createTicket({ ... }); -} catch (error) { - if (error instanceof PipelineAPIError) { - console.error(`API Error ${error.status}: ${error.message}`); - } - throw error; -} -``` diff --git a/.claude/skills/ticket-intake/providers/github.md b/.claude/skills/ticket-intake/providers/github.md deleted file mode 100644 index 7ebaf1082f..0000000000 --- a/.claude/skills/ticket-intake/providers/github.md +++ /dev/null @@ -1,194 +0,0 @@ -# GitHub Provider - Ticket Intake - -Provider-specific logic for ingesting tickets from GitHub Issues. - -## URL Pattern - -``` -https://github.com/{owner}/{repo}/issues/{number} -https://www.github.com/{owner}/{repo}/issues/{number} -``` - -Extract: `owner`, `repo`, `issue_number` from URL. - -## Prerequisites - -- `gh` CLI authenticated (`gh auth status`) -- Access to the repository - -## Fetch Issue Content - -Use `gh` CLI to fetch issue details: - -```bash -# Get issue details in JSON -gh issue view {number} --repo {owner}/{repo} --json title,body,state,labels,assignees,milestone,author,createdAt,comments,linkedPRs - -# Get comments separately if needed -gh issue view {number} --repo {owner}/{repo} --comments -``` - -## Extract Ticket Data - -Map GitHub issue fields to normalized ticket data (stored via API): - -| GitHub Field | ticket.json Field | Notes | -| ------------ | ----------------- | -------------------------- | -| title | title | Direct mapping | -| body | description | Issue body/description | -| state | status | Map: open → "Not Started" | -| labels | labels | Array of label names | -| assignees | assignee | First assignee login | -| author | author | Issue author login | -| milestone | milestone | Milestone title if present | -| comments | comments | Array of comment objects | -| linkedPRs | linkedPRs | PRs linked to this issue | - -### Priority Mapping - -Infer priority from labels: - -- `priority:critical`, `P0` → "Critical" -- `priority:high`, `P1` → "High" -- `priority:medium`, `P2` → "Medium" -- `priority:low`, `P3` → "Low" -- No priority label → "Medium" (default) - -### Area Mapping - -Infer area from labels: - -- `area:ui`, `frontend`, `component:*` → "UI" -- `area:api`, `backend` → "API" -- `area:docs`, `documentation` → "Docs" -- `bug`, `fix` → "Bug" -- `enhancement`, `feature` → "Feature" - -## Update Source - -**For GitHub issues, update is optional but recommended.** - -Add a comment to indicate work has started: - -```bash -gh issue comment {number} --repo {owner}/{repo} --body "🤖 Pipeline started processing this issue." -``` - -Optionally assign to self: - -```bash -gh issue edit {number} --repo {owner}/{repo} --add-assignee @me -``` - -Log any updates via the Pipeline API: - -```typescript -await client.updateTicket(ticketId, { - metadata: { - ...ticket.metadata, - githubWrites: [ - ...(ticket.metadata?.githubWrites || []), - { - action: 'comment', - issueNumber: 123, - at: new Date().toISOString(), - skill: 'ticket-intake', - success: true - } - ] - } -}) -``` - -## GitHub-Specific Ticket Fields - -Store via API using `client.createTicket()`: - -```json -{ - "source": "github", - "githubOwner": "Comfy-Org", - "githubRepo": "ComfyUI_frontend", - "githubIssueNumber": 123, - "githubIssueUrl": "https://github.com/Comfy-Org/ComfyUI_frontend/issues/123", - "labels": ["bug", "area:ui", "priority:high"], - "linkedPRs": [456, 789], - "dosuComment": "..." // Extracted Dosu bot analysis if present -} -``` - -## Dosu Bot Detection - -Many repositories use Dosu bot for automated issue analysis. Check comments for Dosu: - -```bash -gh issue view {number} --repo {owner}/{repo} --comments | grep -A 100 "dosu" -``` - -Look for comments from: - -- `dosu[bot]` -- `dosu-bot` - -Extract Dosu analysis which typically includes: - -- Root cause analysis -- Suggested files to modify -- Related issues/PRs -- Potential solutions - -Store in ticket data via API: - -```json -{ - "dosuComment": { - "found": true, - "analysis": "...", - "suggestedFiles": ["src/file1.ts", "src/file2.ts"], - "relatedIssues": [100, 101] - } -} -``` - -## Extract Linked Issues/PRs - -Parse issue body and comments for references: - -- `#123` → Issue or PR reference -- `fixes #123`, `closes #123` → Linked issue -- `https://github.com/.../issues/123` → Full URL reference - -Store in ticket data via API for research phase: - -```json -{ - "referencedIssues": [100, 101, 102], - "referencedPRs": [200, 201] -} -``` - -## Error Handling - -### Authentication Error - -``` -⚠️ GitHub CLI not authenticated. -Run: gh auth login -``` - -### Issue Not Found - -``` -❌ GitHub issue not found or inaccessible. -- Check the URL is correct -- Ensure you have access to this repository -- Run: gh auth status -``` - -### Rate Limiting - -``` -⚠️ GitHub API rate limited. -Wait a few minutes and try again. -Check status: gh api rate_limit -``` diff --git a/.claude/skills/ticket-intake/providers/notion.md b/.claude/skills/ticket-intake/providers/notion.md deleted file mode 100644 index 42153f9dfe..0000000000 --- a/.claude/skills/ticket-intake/providers/notion.md +++ /dev/null @@ -1,202 +0,0 @@ -# Notion Provider - Ticket Intake - -Provider-specific logic for ingesting tickets from Notion. - -## URL Pattern - -``` -https://www.notion.so/workspace/Page-Title-abc123def456... -https://notion.so/Page-Title-abc123def456... -https://www.notion.so/abc123def456... -``` - -Page ID is the 32-character hex string (with or without hyphens). - -## Prerequisites - -- Notion MCP connected and authenticated -- If not setup: `claude mcp add --transport http notion https://mcp.notion.com/mcp` -- Authenticate via `/mcp` command if prompted - -## Fetch Ticket Content - -Use `Notion:notion-fetch` with the page URL or ID: - -``` -Fetch the full page content including all properties -``` - -## Extract Ticket Data - -Extract these properties (names may vary): - -| Property | Expected Name | Type | -| ------------- | ------------------------- | ------------ | -| Title | Name / Title | Title | -| Status | Status | Select | -| Assignee | Assignee / Assigned To | Person | -| Description | - | Page content | -| Slack Link | Slack Link / Slack Thread | URL | -| GitHub PR | GitHub PR / PR Link | URL | -| Priority | Priority | Select | -| Area | Area / Category | Select | -| Related Tasks | Related Tasks | Relation | - -**If properties are missing**: Note what's unavailable and continue with available data. - -## Update Source (REQUIRED) - -**⚠️ DO NOT SKIP THIS STEP. This is a required action, not optional.** - -**⚠️ Notion Write Safety rules apply (see `$PIPELINE_DIR/docs/notion-write-safety.md` for full reference):** - -- **Whitelist**: Only `Status`, `GitHub PR`, and `Assignee` fields may be written -- **Valid transitions**: Not Started → In Progress, In Progress → In Review, In Review → Done -- **Logging**: Every write attempt MUST be logged with timestamp, field, value, previous value, skill name, and success status - -Use `Notion:notion-update-page` to update the ticket: - -1. **Status**: Set to "In Progress" (only valid from "Not Started") -2. **Assignee**: Assign to pipeline owner (Notion ID: `175d872b-594c-81d4-ba5a-0002911c5966`) - -```json -{ - "page_id": "{page_id_from_ticket}", - "command": "update_properties", - "properties": { - "Status": "In Progress", - "Assignee": "175d872b-594c-81d4-ba5a-0002911c5966" - } -} -``` - -**After the update succeeds**, log the write via the Pipeline API: - -```typescript -await client.updateTicket(ticketId, { - metadata: { - ...ticket.metadata, - notionWrites: [ - ...(ticket.metadata?.notionWrites || []), - { - field: 'Status', - value: 'In Progress', - previousValue: 'Not Started', - at: new Date().toISOString(), - skill: 'ticket-intake', - success: true - } - ] - } -}) -``` - -If update fails, log with `success: false` and continue. - -## Notion-Specific Ticket Fields - -Store via API using `client.createTicket()`: - -```json -{ - "source": "notion", - "notionPageId": "abc123def456...", - "slackLink": "https://slack.com/...", - "relatedTasks": ["page-id-1", "page-id-2"] -} -``` - -## Slack Thread Handling - -If a Slack link exists, use the `slackdump` skill to fetch the thread content programmatically. - -### Slack URL Conversion - -Notion stores Slack links in `slackMessage://` format: - -``` -slackMessage://comfy-organization.slack.com/CHANNEL_ID/THREAD_TS/MESSAGE_TS -``` - -Convert to browser-clickable format: - -``` -https://comfy-organization.slack.com/archives/CHANNEL_ID/pMESSAGE_TS_NO_DOT -``` - -**Example:** - -- Input: `slackMessage://comfy-organization.slack.com/C075ANWQ8KS/1766022478.450909/1764772881.854829` -- Output: `https://comfy-organization.slack.com/archives/C075ANWQ8KS/p1764772881854829` - -(Remove the dot from the last timestamp and prefix with `p`) - -### Fetching Thread Content - -Load the `slackdump` skill and use the **export-thread** workflow: - -```bash -# Export thread by URL -slackdump dump "https://comfy-organization.slack.com/archives/CHANNEL_ID/pMESSAGE_TS" - -# Or by colon notation (channel_id:thread_ts) -slackdump dump CHANNEL_ID:THREAD_TS -``` - -Save the thread content to `$RUN_DIR/slack-context.md` and include it in the ticket metadata. - -> **No manual action required.** The slackdump CLI handles authentication via stored credentials at `~/.cache/slackdump/comfy-organization.bin`. - -## Database Reference: Comfy Tasks - -The "Comfy Tasks" database has these properties (verify via `notion-search`): - -- **Status values**: Not Started, In Progress, In Review, Done -- **Team assignment**: "Frontend Team" for unassigned tickets -- **Filtering note**: Team filtering in Notion may have quirks - handle gracefully - -### Pipeline Owner Details - -When assigning tickets, use these identifiers: - -| Platform | Identifier | -| --------------- | -------------------------------------- | -| Notion User ID | `175d872b-594c-81d4-ba5a-0002911c5966` | -| Notion Name | Christian Byrne | -| Notion Email | cbyrne@comfy.org | -| Slack User ID | U087MJCDHHC | -| GitHub Username | christian-byrne | - -**To update Assignee**, use the Notion User ID (not name): - -``` -properties: {"Assignee": "175d872b-594c-81d4-ba5a-0002911c5966"} -``` - -### Finding Active Tickets - -To list your active tickets: - -``` -Use Notion:notion-search for "Comfy Tasks" -Filter by Assignee = current user OR Team = "Frontend Team" -``` - -## Error Handling - -### Authentication Error - -``` -⚠️ Notion authentication required. -Run: claude mcp add --transport http notion https://mcp.notion.com/mcp -Then authenticate via /mcp command. -``` - -### Page Not Found - -``` -❌ Notion page not found or inaccessible. -- Check the URL is correct -- Ensure you have access to this page -- Try re-authenticating via /mcp -``` diff --git a/.claude/skills/ticket-intake/schema.md b/.claude/skills/ticket-intake/schema.md deleted file mode 100644 index b82a798df1..0000000000 --- a/.claude/skills/ticket-intake/schema.md +++ /dev/null @@ -1,81 +0,0 @@ -# Ticket Schema - -Common schema for normalized ticket data across all sources. This data is stored and retrieved via the Pipeline API, not local files. - -## Ticket Data Schema - -```json -{ - // Required fields (all sources) - "id": "string", // Unique identifier (short form) - "url": "string", // Original URL - "source": "notion | github", // Source type - "title": "string", // Ticket title - "description": "string", // Full description/body - "fetchedAt": "ISO8601", // When ticket was fetched - - // Common optional fields - "status": "string", // Current status - "assignee": "string", // Assigned user - "priority": "string", // Priority level - "area": "string", // Category/area - "labels": ["string"], // Tags/labels - "acceptanceCriteria": ["string"] // List of AC items - - // Source-specific fields (see providers) - // Notion: notionPageId, slackLink, relatedTasks, notionWrites - // GitHub: githubOwner, githubRepo, githubIssueNumber, linkedPRs, dosuComment, referencedIssues -} -``` - -## Ticket State Schema (via API) - -State is managed via the Pipeline API using `client.transitionState()`: - -```json -{ - "ticketId": "string", - "state": "intake | research | planning | implementation | pr_created | done | failed", - "stateChangedAt": "ISO8601", - - // Timestamps tracked by API - "createdAt": "ISO8601", - "updatedAt": "ISO8601" -} -``` - -## Priority Normalization - -All sources should normalize to these values: - -| Normalized | Description | -| ---------- | ------------------------- | -| Critical | Production down, security | -| High | Blocking work, urgent | -| Medium | Normal priority (default) | -| Low | Nice to have, backlog | - -## Status Normalization - -Pipeline tracks these statuses internally: - -| Status | Description | -| -------------- | ---------------------------- | -| research | Gathering context | -| planning | Creating implementation plan | -| implementation | Writing code | -| review | Code review in progress | -| qa | Quality assurance | -| done | PR merged or completed | - -## ID Generation - -IDs are generated by the API when creating tickets. For reference: - -- **Notion**: First 8 characters of page ID -- **GitHub**: `gh-{owner}-{repo}-{issue_number}` (sanitized) - -Examples: - -- Notion: `abc12345` -- GitHub: `gh-comfy-org-frontend-123` diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000000..4a7ea3036a --- /dev/null +++ b/.editorconfig @@ -0,0 +1,12 @@ +root = true + +[*] +indent_style = space +indent_size = 2 +end_of_line = lf +charset = utf-8 +trim_trailing_whitespace = true +insert_final_newline = true + +[*.md] +trim_trailing_whitespace = false diff --git a/.github/actions/find-workflow-run/action.yaml b/.github/actions/find-workflow-run/action.yaml new file mode 100644 index 0000000000..bb18a56f98 --- /dev/null +++ b/.github/actions/find-workflow-run/action.yaml @@ -0,0 +1,65 @@ +name: Find Workflow Run +description: Finds a workflow run for a given commit SHA and outputs its status and run ID. + +inputs: + workflow-id: + description: The workflow filename (e.g., 'ci-size-data.yaml') + required: true + head-sha: + description: The commit SHA to find runs for + required: true + not-found-status: + description: Status to output when no run exists + required: false + default: pending + token: + description: GitHub token for API access + required: true + +outputs: + status: + description: One of 'ready', 'pending', 'failed', or the not-found-status value + value: ${{ steps.find.outputs.status }} + run-id: + description: The workflow run ID (only set when status is 'ready') + value: ${{ steps.find.outputs.run-id }} + +runs: + using: composite + steps: + - name: Find workflow run + id: find + uses: actions/github-script@v8 + env: + WORKFLOW_ID: ${{ inputs.workflow-id }} + HEAD_SHA: ${{ inputs.head-sha }} + NOT_FOUND_STATUS: ${{ inputs.not-found-status }} + with: + github-token: ${{ inputs.token }} + script: | + const { data: runs } = await github.rest.actions.listWorkflowRuns({ + owner: context.repo.owner, + repo: context.repo.repo, + workflow_id: process.env.WORKFLOW_ID, + head_sha: process.env.HEAD_SHA, + per_page: 1, + }); + + const run = runs.workflow_runs[0]; + if (!run) { + core.setOutput('status', process.env.NOT_FOUND_STATUS); + return; + } + + if (run.status !== 'completed') { + core.setOutput('status', 'pending'); + return; + } + + if (run.conclusion !== 'success') { + core.setOutput('status', 'failed'); + return; + } + + core.setOutput('status', 'ready'); + core.setOutput('run-id', String(run.id)); diff --git a/.github/actions/lint-format-verify/action.yml b/.github/actions/lint-format-verify/action.yml new file mode 100644 index 0000000000..1142097123 --- /dev/null +++ b/.github/actions/lint-format-verify/action.yml @@ -0,0 +1,31 @@ +name: 'Lint and format verify' +description: > + Runs the lint/format/knip verification suite plus a conditional + browser-tests typecheck. Shared by ci-lint-format.yaml (PR) and + ci-lint-format-queue.yaml (merge queue) so both paths run the exact + same checks. The caller is responsible for checkout and frontend setup + before invoking this action. + +runs: + using: composite + steps: + - name: Detect browser_tests changes + id: changed-paths + uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 + with: + filters: | + browser_tests: + - 'browser_tests/**' + + - name: Verify lint and format + shell: bash + run: | + pnpm lint + pnpm stylelint + pnpm format:check + pnpm knip + + - name: Typecheck browser tests + if: steps.changed-paths.outputs.browser_tests == 'true' + shell: bash + run: pnpm typecheck:browser diff --git a/.github/actions/resolve-pr-from-workflow-run/action.yaml b/.github/actions/resolve-pr-from-workflow-run/action.yaml new file mode 100644 index 0000000000..6dea5c319e --- /dev/null +++ b/.github/actions/resolve-pr-from-workflow-run/action.yaml @@ -0,0 +1,88 @@ +name: Resolve PR from workflow_run +description: > + Resolves the PR number from a workflow_run event using pull_requests[0] + with a listPullRequestsAssociatedWithCommit fallback. + Skips closed/merged PRs and stale runs (head SHA mismatch). + +inputs: + token: + description: GitHub token for API calls + required: false + default: ${{ github.token }} + +outputs: + skip: + description: "'true' when no open PR was found or the run is stale" + value: ${{ steps.resolve.outputs.skip }} + number: + description: The PR number (empty when skip is true) + value: ${{ steps.resolve.outputs.number }} + base: + description: The PR base branch (empty when skip is true) + value: ${{ steps.resolve.outputs.base }} + head-sha: + description: The PR head SHA (empty when skip is true) + value: ${{ steps.resolve.outputs.head-sha }} + +runs: + using: composite + steps: + - name: Resolve PR + id: resolve + uses: actions/github-script@v8 + with: + github-token: ${{ inputs.token }} + script: | + let pr = context.payload.workflow_run.pull_requests?.[0]; + if (!pr) { + const { data: prs } = await github.rest.repos.listPullRequestsAssociatedWithCommit({ + owner: context.repo.owner, + repo: context.repo.repo, + commit_sha: context.payload.workflow_run.head_sha, + }); + pr = prs.find(p => p.state === 'open'); + } + + // Fork PRs: pull_requests is empty and commit SHA may not be in + // the base repo graph. Fall back to pulls.list with head filter. + if (!pr && context.payload.workflow_run.head_repository?.owner?.login) { + const { data: prs } = await github.rest.pulls.list({ + owner: context.repo.owner, + repo: context.repo.repo, + state: 'open', + head: `${context.payload.workflow_run.head_repository.owner.login}:${context.payload.workflow_run.head_branch}`, + per_page: 1, + }); + pr = prs.find(p => p.head.sha === context.payload.workflow_run.head_sha); + } + + if (!pr) { + core.info('No open PR found for this workflow run — skipping.'); + core.setOutput('skip', 'true'); + return; + } + + const { data: livePr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: pr.number, + }); + + if (livePr.state !== 'open') { + core.info(`PR #${pr.number} is ${livePr.state} — skipping.`); + core.setOutput('skip', 'true'); + return; + } + + if (livePr.head.sha !== context.payload.workflow_run.head_sha) { + core.info( + `Stale run: workflow SHA ${context.payload.workflow_run.head_sha} != PR head ${livePr.head.sha}` + ); + core.setOutput('skip', 'true'); + return; + } + + core.setOutput('base', livePr.base.ref); + core.setOutput('head-sha', livePr.head.sha); + core.setOutput('skip', 'false'); + core.setOutput('number', String(pr.number)); diff --git a/.github/workflows/api-update-registry-api-types.yaml b/.github/workflows/api-update-registry-api-types.yaml deleted file mode 100644 index a35900a725..0000000000 --- a/.github/workflows/api-update-registry-api-types.yaml +++ /dev/null @@ -1,107 +0,0 @@ -# Description: When upstream comfy-api is updated, click dispatch to update the TypeScript type definitions in this repo -name: 'Api: Update Registry API Types' - -on: - # Manual trigger - workflow_dispatch: - - # Triggered from comfy-api repo - repository_dispatch: - types: [comfy-api-updated] - -jobs: - update-registry-types: - runs-on: ubuntu-latest - permissions: - contents: write - pull-requests: write - steps: - - name: Checkout repository - uses: actions/checkout@v6 - - - name: Install pnpm - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 - - - name: Setup Node.js - uses: actions/setup-node@v6 - with: - node-version-file: '.nvmrc' - cache: 'pnpm' - - - name: Install dependencies - run: pnpm install --frozen-lockfile - - - name: Checkout comfy-api repository - uses: actions/checkout@v6 - with: - repository: Comfy-Org/comfy-api - path: comfy-api - token: ${{ secrets.COMFY_API_PAT }} - clean: true - - - name: Get API commit information - id: api-info - run: | - cd comfy-api - API_COMMIT=$(git rev-parse --short HEAD) - echo "commit=${API_COMMIT}" >> $GITHUB_OUTPUT - cd .. - - - name: Generate API types - run: | - echo "Generating TypeScript types from comfy-api@${{ steps.api-info.outputs.commit }}..." - mkdir -p ./packages/registry-types/src - pnpm dlx openapi-typescript ./comfy-api/openapi.yml --output ./packages/registry-types/src/comfyRegistryTypes.ts - - - name: Validate generated types - run: | - if [ ! -f ./packages/registry-types/src/comfyRegistryTypes.ts ]; then - echo "Error: Types file was not generated." - exit 1 - fi - - # Check if file is not empty - if [ ! -s ./packages/registry-types/src/comfyRegistryTypes.ts ]; then - echo "Error: Generated types file is empty." - exit 1 - fi - - - name: Lint generated types - run: | - echo "Linting generated Comfy Registry API types..." - pnpm lint:fix:no-cache -- ./packages/registry-types/src/comfyRegistryTypes.ts - - - name: Check for changes - id: check-changes - run: | - if [[ -z $(git status --porcelain ./packages/registry-types/src/comfyRegistryTypes.ts) ]]; then - echo "No changes to Comfy Registry API types detected." - echo "changed=false" >> $GITHUB_OUTPUT - exit 0 - else - echo "Changes detected in Comfy Registry API types." - echo "changed=true" >> $GITHUB_OUTPUT - fi - - - name: Create Pull Request - if: steps.check-changes.outputs.changed == 'true' - uses: peter-evans/create-pull-request@c0f553fe549906ede9cf27b5156039d195d2ece0 # v8.1.0 - with: - token: ${{ secrets.PR_GH_TOKEN }} - commit-message: '[chore] Update Comfy Registry API types from comfy-api@${{ steps.api-info.outputs.commit }}' - title: '[chore] Update Comfy Registry API types from comfy-api@${{ steps.api-info.outputs.commit }}' - body: | - ## Automated API Type Update - - This PR updates the Comfy Registry API types from the latest comfy-api OpenAPI specification. - - - API commit: ${{ steps.api-info.outputs.commit }} - - Generated on: ${{ github.event.repository.updated_at }} - - These types are automatically generated using openapi-typescript. - branch: update-registry-types-${{ steps.api-info.outputs.commit }} - base: main - labels: CNR - delete-branch: true - add-paths: | - packages/registry-types/src/comfyRegistryTypes.ts diff --git a/.github/workflows/ci-lint-format-queue.yaml b/.github/workflows/ci-lint-format-queue.yaml new file mode 100644 index 0000000000..de064ca885 --- /dev/null +++ b/.github/workflows/ci-lint-format-queue.yaml @@ -0,0 +1,29 @@ +# Description: Lint and format verification for GitHub merge queue runs. +# Paired with ci-lint-format.yaml — workflow name and job name must match +# so branch protection resolves a single required check in both the +# pull_request and merge_group contexts. This file runs verify-only steps +# with a read-only token; auto-fix and PR comments live in the PR workflow. +name: 'CI: Lint Format' + +on: + merge_group: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + lint-and-format: + runs-on: ubuntu-latest + steps: + - name: Checkout merge group ref + uses: actions/checkout@v6 + + - name: Setup frontend + uses: ./.github/actions/setup-frontend + + - name: Verify lint and format + uses: ./.github/actions/lint-format-verify diff --git a/.github/workflows/ci-lint-format.yaml b/.github/workflows/ci-lint-format.yaml index 157bd576d2..6205dbed8c 100644 --- a/.github/workflows/ci-lint-format.yaml +++ b/.github/workflows/ci-lint-format.yaml @@ -1,4 +1,7 @@ -# Description: Linting and code formatting validation for pull requests +# Description: Linting and code formatting validation for pull requests. +# Paired with ci-lint-format-queue.yaml - workflow name and job name must +# match so branch protection resolves a single required check in both the +# pull_request and merge_group contexts. name: 'CI: Lint Format' on: @@ -26,14 +29,6 @@ jobs: - name: Setup frontend uses: ./.github/actions/setup-frontend - - name: Detect browser_tests changes - id: changed-paths - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 - with: - filters: | - browser_tests: - - 'browser_tests/**' - - name: Run ESLint with auto-fix run: pnpm lint:fix @@ -77,16 +72,8 @@ jobs: echo "See CONTRIBUTING.md for more details." exit 1 - - name: Final validation - run: | - pnpm lint - pnpm stylelint - pnpm format:check - pnpm knip - - - name: Typecheck browser tests - if: steps.changed-paths.outputs.browser_tests == 'true' - run: pnpm typecheck:browser + - name: Verify lint and format + uses: ./.github/actions/lint-format-verify - name: Comment on PR about auto-fix if: steps.verify-changed-files.outputs.changed == 'true' && github.event.pull_request.head.repo.full_name == github.repository diff --git a/.github/workflows/ci-perf-report.yaml b/.github/workflows/ci-perf-report.yaml index 3666ecc1f1..b265e0ebb7 100644 --- a/.github/workflows/ci-perf-report.yaml +++ b/.github/workflows/ci-perf-report.yaml @@ -58,21 +58,6 @@ jobs: retention-days: 30 if-no-files-found: warn - - name: Save PR metadata - if: github.event_name == 'pull_request' - run: | - mkdir -p temp/perf-meta - echo "${{ github.event.number }}" > temp/perf-meta/number.txt - echo "${{ github.event.pull_request.base.ref }}" > temp/perf-meta/base.txt - echo "${{ github.event.pull_request.head.sha }}" > temp/perf-meta/head-sha.txt - - - name: Upload PR metadata - if: github.event_name == 'pull_request' - uses: actions/upload-artifact@v6 - with: - name: perf-meta - path: temp/perf-meta/ - - name: Save perf baseline to perf-data branch if: github.event_name == 'push' && github.ref == 'refs/heads/main' && steps.perf.outcome == 'success' continue-on-error: true diff --git a/.github/workflows/ci-size-data.yaml b/.github/workflows/ci-size-data.yaml index 1d28327349..b3e4598fce 100644 --- a/.github/workflows/ci-size-data.yaml +++ b/.github/workflows/ci-size-data.yaml @@ -32,13 +32,6 @@ jobs: - name: Collect size data run: node scripts/size-collect.js - - name: Save PR metadata - if: ${{ github.event_name == 'pull_request' }} - run: | - echo ${{ github.event.number }} > ./temp/size/number.txt - echo ${{ github.base_ref }} > ./temp/size/base.txt - echo ${{ github.event.pull_request.head.sha }} > ./temp/size/head-sha.txt - - name: Upload size data uses: actions/upload-artifact@v6 with: diff --git a/.github/workflows/ci-tests-e2e-coverage.yaml b/.github/workflows/ci-tests-e2e-coverage.yaml new file mode 100644 index 0000000000..7613bbc927 --- /dev/null +++ b/.github/workflows/ci-tests-e2e-coverage.yaml @@ -0,0 +1,147 @@ +name: 'CI: E2E Coverage' + +on: + workflow_run: + workflows: ['CI: Tests E2E'] + types: + - completed + +concurrency: + group: e2e-coverage-${{ github.event.workflow_run.head_sha }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + merge: + if: > + github.repository == 'Comfy-Org/ComfyUI_frontend' && + github.event.workflow_run.conclusion == 'success' + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Setup frontend + uses: ./.github/actions/setup-frontend + + - name: Download all shard coverage data + uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12 + with: + run_id: ${{ github.event.workflow_run.id }} + name: e2e-coverage-shard-.* + name_is_regexp: true + path: temp/coverage-shards + if_no_artifact_found: warn + + - name: Install lcov + run: sudo apt-get install -y -qq lcov + + - name: Merge shard coverage into single LCOV + run: | + mkdir -p coverage/playwright + LCOV_FILES=$(find temp/coverage-shards -name 'coverage.lcov' -type f) + if [ -z "$LCOV_FILES" ]; then + echo "No coverage.lcov files found" + touch coverage/playwright/coverage.lcov + exit 0 + fi + ADD_ARGS="" + for f in $LCOV_FILES; do ADD_ARGS="$ADD_ARGS -a $f"; done + lcov $ADD_ARGS -o coverage/playwright/coverage.lcov + wc -l coverage/playwright/coverage.lcov + + - name: Validate merged coverage + run: | + SHARD_COUNT=$(find temp/coverage-shards -name 'coverage.lcov' -type f | wc -l | tr -d ' ') + if [ "$SHARD_COUNT" -eq 0 ]; then + echo "::error::No shard coverage.lcov files found under temp/coverage-shards" + exit 1 + fi + + MERGED_SF=$(grep -c '^SF:' coverage/playwright/coverage.lcov || echo 0) + MERGED_LH=$(awk -F: '/^LH:/{s+=$2}END{print s+0}' coverage/playwright/coverage.lcov) + MERGED_LF=$(awk -F: '/^LF:/{s+=$2}END{print s+0}' coverage/playwright/coverage.lcov) + echo "### Merged coverage" >> "$GITHUB_STEP_SUMMARY" + echo "- **$MERGED_SF** source files" >> "$GITHUB_STEP_SUMMARY" + echo "- **$MERGED_LH / $MERGED_LF** lines hit" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "| Shard | Files | Lines Hit |" >> "$GITHUB_STEP_SUMMARY" + echo "|-------|-------|-----------|" >> "$GITHUB_STEP_SUMMARY" + for f in $(find temp/coverage-shards -name 'coverage.lcov' -type f | sort); do + SHARD=$(basename "$(dirname "$f")") + SHARD_SF=$(grep -c '^SF:' "$f" || echo 0) + SHARD_LH=$(awk -F: '/^LH:/{s+=$2}END{print s+0}' "$f") + echo "| $SHARD | $SHARD_SF | $SHARD_LH |" >> "$GITHUB_STEP_SUMMARY" + if [ "$MERGED_LH" -lt "$SHARD_LH" ]; then + echo "::error::Merged LH ($MERGED_LH) < shard LH ($SHARD_LH) in $SHARD — possible data loss" + fi + done + + - name: Upload merged coverage data + if: always() + uses: actions/upload-artifact@v6 + with: + name: e2e-coverage + path: coverage/playwright/ + retention-days: 30 + if-no-files-found: warn + + - name: Upload E2E coverage to Codecov + if: always() + uses: codecov/codecov-action@1af58845a975a7985b0beb0cbe6fbbb71a41dbad # v5.5.3 + with: + files: coverage/playwright/coverage.lcov + flags: e2e + token: ${{ secrets.CODECOV_TOKEN }} + fail_ci_if_error: false + + - name: Generate HTML coverage report + run: | + if [ ! -s coverage/playwright/coverage.lcov ]; then + echo "No coverage data; generating placeholder report." + mkdir -p coverage/html + echo '

No E2E coverage data available for this run.

' > coverage/html/index.html + exit 0 + fi + genhtml coverage/playwright/coverage.lcov \ + -o coverage/html \ + --title "ComfyUI E2E Coverage" \ + --no-function-coverage \ + --precision 1 + + - name: Upload HTML report artifact + uses: actions/upload-artifact@v6 + with: + name: e2e-coverage-html + path: coverage/html/ + retention-days: 30 + + deploy: + needs: merge + if: github.event.workflow_run.head_branch == 'main' + runs-on: ubuntu-latest + permissions: + pages: write + id-token: write + environment: + name: github-pages + url: ${{ steps.deployment.outputs.page_url }} + steps: + - name: Download HTML report + uses: actions/download-artifact@v7 + with: + name: e2e-coverage-html + path: coverage/html + + - name: Upload to GitHub Pages + uses: actions/upload-pages-artifact@56afc609e74202658d3ffba0e8f6dda462b719fa # v3.0.1 + with: + path: coverage/html + + - name: Deploy to GitHub Pages + id: deployment + uses: actions/deploy-pages@d6db90164ac5ed86f2b6aed7e0febac5b3c0c03e # v4.0.5 diff --git a/.github/workflows/ci-tests-e2e-forks.yaml b/.github/workflows/ci-tests-e2e-forks.yaml index 3e34c846d0..5e288ef307 100644 --- a/.github/workflows/ci-tests-e2e-forks.yaml +++ b/.github/workflows/ci-tests-e2e-forks.yaml @@ -6,6 +6,10 @@ on: workflows: ['CI: Tests E2E'] types: [requested, completed] +concurrency: + group: ${{ github.workflow }}-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }} + cancel-in-progress: true + jobs: deploy-and-comment-forked-pr: runs-on: ubuntu-latest @@ -30,40 +34,23 @@ jobs: - name: Checkout repository uses: actions/checkout@v6 - - name: Get PR Number + - name: Resolve PR from workflow_run context id: pr - uses: actions/github-script@v8 - with: - script: | - const { data: prs } = await github.rest.pulls.list({ - owner: context.repo.owner, - repo: context.repo.repo, - state: 'open', - }); - - const pr = prs.find(p => p.head.sha === context.payload.workflow_run.head_sha); - - if (!pr) { - console.log('No PR found for SHA:', context.payload.workflow_run.head_sha); - return null; - } - - console.log(`Found PR #${pr.number} from fork: ${context.payload.workflow_run.head_repository.full_name}`); - return pr.number; + uses: ./.github/actions/resolve-pr-from-workflow-run - name: Handle Test Start - if: steps.pr.outputs.result != 'null' && github.event.action == 'requested' + if: steps.pr.outputs.skip != 'true' && github.event.action == 'requested' env: GITHUB_TOKEN: ${{ github.token }} run: | chmod +x scripts/cicd/pr-playwright-deploy-and-comment.sh ./scripts/cicd/pr-playwright-deploy-and-comment.sh \ - "${{ steps.pr.outputs.result }}" \ + "${{ steps.pr.outputs.number }}" \ "${{ github.event.workflow_run.head_branch }}" \ "starting" - name: Download and Deploy Reports - if: steps.pr.outputs.result != 'null' && github.event.action == 'completed' + if: steps.pr.outputs.skip != 'true' && github.event.action == 'completed' uses: actions/download-artifact@v7 with: github-token: ${{ secrets.GITHUB_TOKEN }} @@ -72,7 +59,7 @@ jobs: path: reports - name: Handle Test Completion - if: steps.pr.outputs.result != 'null' && github.event.action == 'completed' + if: steps.pr.outputs.skip != 'true' && github.event.action == 'completed' env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} @@ -85,6 +72,6 @@ jobs: chmod +x scripts/cicd/pr-playwright-deploy-and-comment.sh ./scripts/cicd/pr-playwright-deploy-and-comment.sh \ - "${{ steps.pr.outputs.result }}" \ + "${{ steps.pr.outputs.number }}" \ "${{ github.event.workflow_run.head_branch }}" \ "completed" diff --git a/.github/workflows/ci-tests-e2e.yaml b/.github/workflows/ci-tests-e2e.yaml index f10b8c27e3..59217f742c 100644 --- a/.github/workflows/ci-tests-e2e.yaml +++ b/.github/workflows/ci-tests-e2e.yaml @@ -8,6 +8,7 @@ on: pull_request: branches-ignore: [wip/*, draft/*, temp/*] paths-ignore: ['**/*.md'] + merge_group: workflow_dispatch: concurrency: @@ -86,6 +87,7 @@ jobs: run: pnpm exec playwright test --project=chromium --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} --reporter=blob env: PLAYWRIGHT_BLOB_OUTPUT_DIR: ./blob-report + COLLECT_COVERAGE: 'true' - name: Upload blob report uses: actions/upload-artifact@v6 @@ -95,6 +97,15 @@ jobs: path: blob-report/ retention-days: 1 + - name: Upload shard coverage data + if: always() + uses: actions/upload-artifact@v6 + with: + name: e2e-coverage-shard-${{ matrix.shardIndex }} + path: coverage/playwright/ + retention-days: 1 + if-no-files-found: warn + playwright-tests: # Ideally, each shard runs test in 6 minutes, but allow up to 15 minutes timeout-minutes: 15 diff --git a/.github/workflows/ci-tests-storybook-forks.yaml b/.github/workflows/ci-tests-storybook-forks.yaml index 0d4ab96f11..da8b5bf397 100644 --- a/.github/workflows/ci-tests-storybook-forks.yaml +++ b/.github/workflows/ci-tests-storybook-forks.yaml @@ -6,6 +6,10 @@ on: workflows: ['CI: Tests Storybook'] types: [requested, completed] +concurrency: + group: ${{ github.workflow }}-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }} + cancel-in-progress: true + jobs: deploy-and-comment-forked-pr: runs-on: ubuntu-latest @@ -30,40 +34,23 @@ jobs: - name: Checkout repository uses: actions/checkout@v6 - - name: Get PR Number + - name: Resolve PR from workflow_run context id: pr - uses: actions/github-script@v8 - with: - script: | - const { data: prs } = await github.rest.pulls.list({ - owner: context.repo.owner, - repo: context.repo.repo, - state: 'open', - }); - - const pr = prs.find(p => p.head.sha === context.payload.workflow_run.head_sha); - - if (!pr) { - console.log('No PR found for SHA:', context.payload.workflow_run.head_sha); - return null; - } - - console.log(`Found PR #${pr.number} from fork: ${context.payload.workflow_run.head_repository.full_name}`); - return pr.number; + uses: ./.github/actions/resolve-pr-from-workflow-run - name: Handle Storybook Start - if: steps.pr.outputs.result != 'null' && github.event.action == 'requested' + if: steps.pr.outputs.skip != 'true' && github.event.action == 'requested' env: GITHUB_TOKEN: ${{ github.token }} run: | chmod +x scripts/cicd/pr-storybook-deploy-and-comment.sh ./scripts/cicd/pr-storybook-deploy-and-comment.sh \ - "${{ steps.pr.outputs.result }}" \ + "${{ steps.pr.outputs.number }}" \ "${{ github.event.workflow_run.head_branch }}" \ "starting" - name: Download and Deploy Storybook - if: steps.pr.outputs.result != 'null' && github.event.action == 'completed' && github.event.workflow_run.conclusion == 'success' + if: steps.pr.outputs.skip != 'true' && github.event.action == 'completed' && github.event.workflow_run.conclusion == 'success' uses: actions/download-artifact@v7 with: github-token: ${{ secrets.GITHUB_TOKEN }} @@ -72,7 +59,7 @@ jobs: path: storybook-static - name: Handle Storybook Completion - if: steps.pr.outputs.result != 'null' && github.event.action == 'completed' + if: steps.pr.outputs.skip != 'true' && github.event.action == 'completed' env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} @@ -82,6 +69,6 @@ jobs: run: | chmod +x scripts/cicd/pr-storybook-deploy-and-comment.sh ./scripts/cicd/pr-storybook-deploy-and-comment.sh \ - "${{ steps.pr.outputs.result }}" \ + "${{ steps.pr.outputs.number }}" \ "${{ github.event.workflow_run.head_branch }}" \ "completed" diff --git a/.github/workflows/ci-tests-unit.yaml b/.github/workflows/ci-tests-unit.yaml index 98bb6e70b7..3fc3095bd6 100644 --- a/.github/workflows/ci-tests-unit.yaml +++ b/.github/workflows/ci-tests-unit.yaml @@ -8,6 +8,7 @@ on: pull_request: branches-ignore: [wip/*, draft/*, temp/*] paths-ignore: ['**/*.md'] + merge_group: concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -26,9 +27,20 @@ jobs: - name: Run Vitest tests with coverage run: pnpm test:coverage + - name: Upload unit coverage artifact + if: always() && github.event_name == 'push' + uses: actions/upload-artifact@v6 + with: + name: unit-coverage + path: coverage/lcov.info + retention-days: 30 + if-no-files-found: warn + - name: Upload coverage to Codecov if: always() uses: codecov/codecov-action@1af58845a975a7985b0beb0cbe6fbbb71a41dbad # v5.5.3 with: files: coverage/lcov.info + flags: unit + token: ${{ secrets.CODECOV_TOKEN }} fail_ci_if_error: false diff --git a/.github/workflows/ci-vercel-website-preview.yaml b/.github/workflows/ci-vercel-website-preview.yaml new file mode 100644 index 0000000000..7a26fd178d --- /dev/null +++ b/.github/workflows/ci-vercel-website-preview.yaml @@ -0,0 +1,158 @@ +--- +name: 'CI: Vercel Website Preview' + +on: + pull_request: + types: [opened, synchronize, reopened] + paths: + - 'apps/website/**' + - 'packages/design-system/**' + - 'packages/tailwind-utils/**' + push: + branches: [main] + paths: + - 'apps/website/**' + - 'packages/design-system/**' + - 'packages/tailwind-utils/**' + +env: + VERCEL_ORG_ID: ${{ secrets.VERCEL_WEBSITE_ORG_ID }} + VERCEL_PROJECT_ID: ${{ secrets.VERCEL_WEBSITE_PROJECT_ID }} + VERCEL_TOKEN: ${{ secrets.VERCEL_WEBSITE_TOKEN }} + VERCEL_SCOPE: comfyui + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + deploy-preview: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + permissions: + contents: read + env: + ALIAS_HOST: comfy-website-preview-pr-${{ github.event.pull_request.number }}.vercel.app + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Install pnpm + uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 + + - name: Setup Node + uses: actions/setup-node@v6 + with: + node-version-file: '.nvmrc' + + - name: Install Vercel CLI + run: npm install --global vercel@latest + + - name: Pull Vercel environment information + run: vercel pull --yes --environment=preview + + - name: Build project artifacts + run: vercel build + + - name: Fetch head commit metadata + id: head-commit + uses: actions/github-script@v8 + with: + script: | + const { data } = await github.rest.repos.getCommit({ + owner: context.repo.owner, + repo: context.repo.repo, + ref: context.payload.pull_request.head.sha, + }) + const author = data.author?.login || data.commit.author?.name || '' + const message = (data.commit.message || '').split('\n', 1)[0] + core.setOutput('author', author) + core.setOutput('message', message) + + - name: Deploy project artifacts to Vercel + id: deploy + env: + GIT_COMMIT_REF: ${{ github.event.pull_request.head.ref }} + GIT_COMMIT_SHA: ${{ github.event.pull_request.head.sha }} + GIT_AUTHOR_LOGIN: ${{ steps.head-commit.outputs.author }} + GIT_COMMIT_MESSAGE: ${{ steps.head-commit.outputs.message }} + GIT_PR_ID: ${{ github.event.pull_request.number }} + GIT_REPO: ${{ github.repository }} + run: | + URL=$(vercel deploy --prebuilt \ + --meta githubCommitRef="$GIT_COMMIT_REF" \ + --meta githubCommitSha="$GIT_COMMIT_SHA" \ + --meta githubCommitAuthorLogin="$GIT_AUTHOR_LOGIN" \ + --meta githubCommitMessage="$GIT_COMMIT_MESSAGE" \ + --meta githubPrId="$GIT_PR_ID" \ + --meta githubRepo="$GIT_REPO") + echo "url=$URL" >> "$GITHUB_OUTPUT" + + - name: Alias deployment to stable PR hostname + id: alias-set + continue-on-error: true + env: + DEPLOY_URL: ${{ steps.deploy.outputs.url }} + run: | + vercel alias set "$DEPLOY_URL" "$ALIAS_HOST" --scope="$VERCEL_SCOPE" + + - name: Publish preview outputs + env: + DEPLOY_URL: ${{ steps.deploy.outputs.url }} + ALIAS_OK: ${{ steps.alias-set.outcome == 'success' }} + run: | + if [[ "$ALIAS_OK" == "true" ]]; then + STABLE_URL="https://$ALIAS_HOST" + else + STABLE_URL="$DEPLOY_URL" + fi + mkdir -p temp/vercel-preview + echo "$DEPLOY_URL" > temp/vercel-preview/url.txt + echo "$STABLE_URL" > temp/vercel-preview/stable-url.txt + { + echo "**Preview:** $STABLE_URL" + if [[ "$ALIAS_OK" == "true" ]]; then + echo "**This commit:** $DEPLOY_URL" + else + echo "_Stable alias update failed — URL reflects this commit only._" + fi + } >> "$GITHUB_STEP_SUMMARY" + + - name: Upload preview metadata + uses: actions/upload-artifact@v6 + with: + name: vercel-preview + path: temp/vercel-preview + + deploy-production: + if: github.event_name == 'push' + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Install pnpm + uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 + + - name: Setup Node + uses: actions/setup-node@v6 + with: + node-version-file: '.nvmrc' + + - name: Install Vercel CLI + run: npm install --global vercel@latest + + - name: Pull Vercel environment information + run: vercel pull --yes --environment=production + + - name: Build project artifacts + run: vercel build --prod + + - name: Deploy project artifacts to Vercel + id: deploy + run: | + URL=$(vercel deploy --prebuilt --prod) + echo "url=$URL" >> "$GITHUB_OUTPUT" + + - name: Add deployment URL to summary + run: echo "**Production:** ${{ steps.deploy.outputs.url }}" >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/ci-website-build.yaml b/.github/workflows/ci-website-build.yaml new file mode 100644 index 0000000000..832854c2eb --- /dev/null +++ b/.github/workflows/ci-website-build.yaml @@ -0,0 +1,33 @@ +# Description: Build and validate the marketing website (apps/website) +name: 'CI: Website Build' + +on: + push: + branches: [main, master, website/*] + paths: + - 'apps/website/**' + - 'packages/design-system/**' + - 'pnpm-lock.yaml' + pull_request: + branches-ignore: [wip/*, draft/*, temp/*] + paths: + - 'apps/website/**' + - 'packages/design-system/**' + - 'pnpm-lock.yaml' + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v6 + + - name: Setup frontend + uses: ./.github/actions/setup-frontend + + - name: Build website + run: pnpm --filter @comfyorg/website build diff --git a/.github/workflows/coverage-slack-notify.yaml b/.github/workflows/coverage-slack-notify.yaml new file mode 100644 index 0000000000..7c9896bbc9 --- /dev/null +++ b/.github/workflows/coverage-slack-notify.yaml @@ -0,0 +1,149 @@ +name: 'Coverage: Slack Notification' + +on: + workflow_run: + workflows: ['CI: Tests Unit'] + branches: [main] + types: + - completed + +permissions: + contents: read + actions: read + pull-requests: read + +jobs: + notify: + if: > + github.repository == 'Comfy-Org/ComfyUI_frontend' && + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.event == 'push' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Setup frontend + uses: ./.github/actions/setup-frontend + + - name: Download current unit coverage + uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12 + with: + run_id: ${{ github.event.workflow_run.id }} + name: unit-coverage + path: coverage + + - name: Download previous unit coverage baseline + continue-on-error: true + uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12 + with: + branch: main + workflow: coverage-slack-notify.yaml + name: unit-coverage-baseline + path: temp/coverage-baseline + if_no_artifact_found: warn + + - name: Download latest E2E coverage + continue-on-error: true + uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12 + with: + branch: main + workflow: ci-tests-e2e-coverage.yaml + name: e2e-coverage + path: temp/e2e-coverage + if_no_artifact_found: warn + + - name: Download previous E2E coverage baseline + continue-on-error: true + uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12 + with: + branch: main + workflow: coverage-slack-notify.yaml + name: e2e-coverage-baseline + path: temp/e2e-coverage-baseline + if_no_artifact_found: warn + + - name: Resolve merged PR metadata + id: pr-meta + uses: actions/github-script@v8 + with: + script: | + const sha = context.payload.workflow_run.head_sha; + const { data: commit } = await github.rest.repos.getCommit({ + owner: context.repo.owner, + repo: context.repo.repo, + ref: sha, + }); + const message = commit.commit.message ?? ''; + const firstLine = message.split('\n')[0]; + const match = firstLine.match(/\(#(\d+)\)\s*$/); + if (!match) { + core.setOutput('skip', 'true'); + core.info('No PR number found in commit message — skipping.'); + return; + } + const prNumber = match[1]; + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: Number(prNumber), + }); + core.setOutput('skip', 'false'); + core.setOutput('number', prNumber); + core.setOutput('url', pr.html_url); + core.setOutput('author', pr.user.login); + + - name: Generate Slack notification + if: steps.pr-meta.outputs.skip != 'true' + id: slack-payload + env: + PR_URL: ${{ steps.pr-meta.outputs.url }} + PR_NUMBER: ${{ steps.pr-meta.outputs.number }} + PR_AUTHOR: ${{ steps.pr-meta.outputs.author }} + run: | + PAYLOAD=$(pnpm exec tsx scripts/coverage-slack-notify.ts \ + --pr-url="$PR_URL" \ + --pr-number="$PR_NUMBER" \ + --author="$PR_AUTHOR") + if [ -n "$PAYLOAD" ]; then + echo "has_payload=true" >> "$GITHUB_OUTPUT" + DELIM="SLACK_PAYLOAD_$(date +%s)" + echo "payload<<$DELIM" >> "$GITHUB_OUTPUT" + printf '%s\n' "$PAYLOAD" >> "$GITHUB_OUTPUT" + echo "$DELIM" >> "$GITHUB_OUTPUT" + else + echo "has_payload=false" >> "$GITHUB_OUTPUT" + fi + + - name: Post to Slack + if: steps.slack-payload.outputs.has_payload == 'true' + continue-on-error: true + env: + SLACK_PAYLOAD: ${{ steps.slack-payload.outputs.payload }} + SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} + run: | + # Channel: #p-deprecated-frontend-automated-testing + BODY=$(echo "$SLACK_PAYLOAD" | jq --arg ch "C0AP09LKRDZ" '. + {channel: $ch}') + curl -sf -X POST \ + -H "Authorization: Bearer $SLACK_BOT_TOKEN" \ + -H "Content-Type: application/json" \ + -d "$BODY" \ + -o /dev/null \ + https://slack.com/api/chat.postMessage + + - name: Save unit coverage baseline + if: always() && hashFiles('coverage/lcov.info') != '' + uses: actions/upload-artifact@v6 + with: + name: unit-coverage-baseline + path: coverage/lcov.info + retention-days: 90 + if-no-files-found: warn + + - name: Save E2E coverage baseline + if: always() && hashFiles('temp/e2e-coverage/coverage.lcov') != '' + uses: actions/upload-artifact@v6 + with: + name: e2e-coverage-baseline + path: temp/e2e-coverage/coverage.lcov + retention-days: 90 + if-no-files-found: warn diff --git a/.github/workflows/pr-report.yaml b/.github/workflows/pr-report.yaml index 1fecfdc567..d5c9dbd86f 100644 --- a/.github/workflows/pr-report.yaml +++ b/.github/workflows/pr-report.yaml @@ -2,7 +2,7 @@ name: 'PR: Unified Report' on: workflow_run: - workflows: ['CI: Size Data', 'CI: Performance Report'] + workflows: ['CI: Size Data', 'CI: Performance Report', 'CI: E2E Coverage'] types: - completed @@ -30,110 +30,25 @@ jobs: - name: Resolve PR from workflow_run context id: pr-meta - uses: actions/github-script@v8 - with: - script: | - let pr = context.payload.workflow_run.pull_requests?.[0]; - if (!pr) { - const { data: prs } = await github.rest.repos.listPullRequestsAssociatedWithCommit({ - owner: context.repo.owner, - repo: context.repo.repo, - commit_sha: context.payload.workflow_run.head_sha, - }); - pr = prs.find(p => p.state === 'open'); - } + uses: ./.github/actions/resolve-pr-from-workflow-run - if (!pr) { - core.info('No open PR found for this workflow run — skipping.'); - core.setOutput('skip', 'true'); - return; - } - - // Verify the workflow_run head SHA matches the current PR head - const { data: livePr } = await github.rest.pulls.get({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: pr.number, - }); - - if (livePr.head.sha !== context.payload.workflow_run.head_sha) { - core.info(`Stale run: workflow SHA ${context.payload.workflow_run.head_sha} != PR head ${livePr.head.sha}`); - core.setOutput('skip', 'true'); - return; - } - - core.setOutput('skip', 'false'); - core.setOutput('number', String(pr.number)); - core.setOutput('base', livePr.base.ref); - core.setOutput('head-sha', livePr.head.sha); - - - name: Find size workflow run for this commit + - name: Find size workflow run if: steps.pr-meta.outputs.skip != 'true' id: find-size - uses: actions/github-script@v8 + uses: ./.github/actions/find-workflow-run with: - script: | - const headSha = '${{ steps.pr-meta.outputs.head-sha }}'; - const { data: runs } = await github.rest.actions.listWorkflowRuns({ - owner: context.repo.owner, - repo: context.repo.repo, - workflow_id: 'ci-size-data.yaml', - head_sha: headSha, - per_page: 1, - }); + workflow-id: ci-size-data.yaml + head-sha: ${{ steps.pr-meta.outputs.head-sha }} + token: ${{ secrets.GITHUB_TOKEN }} - const run = runs.workflow_runs[0]; - if (!run) { - core.setOutput('status', 'pending'); - return; - } - - if (run.status !== 'completed') { - core.setOutput('status', 'pending'); - return; - } - - if (run.conclusion !== 'success') { - core.setOutput('status', 'failed'); - return; - } - - core.setOutput('status', 'ready'); - core.setOutput('run-id', String(run.id)); - - - name: Find perf workflow run for this commit + - name: Find perf workflow run if: steps.pr-meta.outputs.skip != 'true' id: find-perf - uses: actions/github-script@v8 + uses: ./.github/actions/find-workflow-run with: - script: | - const headSha = '${{ steps.pr-meta.outputs.head-sha }}'; - const { data: runs } = await github.rest.actions.listWorkflowRuns({ - owner: context.repo.owner, - repo: context.repo.repo, - workflow_id: 'ci-perf-report.yaml', - head_sha: headSha, - per_page: 1, - }); - - const run = runs.workflow_runs[0]; - if (!run) { - core.setOutput('status', 'pending'); - return; - } - - if (run.status !== 'completed') { - core.setOutput('status', 'pending'); - return; - } - - if (run.conclusion !== 'success') { - core.setOutput('status', 'failed'); - return; - } - - core.setOutput('status', 'ready'); - core.setOutput('run-id', String(run.id)); + workflow-id: ci-perf-report.yaml + head-sha: ${{ steps.pr-meta.outputs.head-sha }} + token: ${{ secrets.GITHUB_TOKEN }} - name: Download size data (current) if: steps.pr-meta.outputs.skip != 'true' && steps.find-size.outputs.status == 'ready' @@ -154,6 +69,25 @@ jobs: path: temp/size-prev if_no_artifact_found: warn + - name: Find coverage workflow run + if: steps.pr-meta.outputs.skip != 'true' + id: find-coverage + uses: ./.github/actions/find-workflow-run + with: + workflow-id: ci-tests-e2e-coverage.yaml + head-sha: ${{ steps.pr-meta.outputs.head-sha }} + not-found-status: skip + token: ${{ secrets.GITHUB_TOKEN }} + + - name: Download coverage data + if: steps.pr-meta.outputs.skip != 'true' && steps.find-coverage.outputs.status == 'ready' + uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12 + with: + name: e2e-coverage + run_id: ${{ steps.find-coverage.outputs.run-id }} + path: temp/coverage + if_no_artifact_found: warn + - name: Download perf metrics (current) if: steps.pr-meta.outputs.skip != 'true' && steps.find-perf.outputs.status == 'ready' uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12 @@ -189,9 +123,10 @@ jobs: - name: Generate unified report if: steps.pr-meta.outputs.skip != 'true' run: > - node scripts/unified-report.js + pnpm exec tsx scripts/unified-report.ts --size-status=${{ steps.find-size.outputs.status }} --perf-status=${{ steps.find-perf.outputs.status }} + --coverage-status=${{ steps.find-coverage.outputs.status }} > pr-report.md - name: Remove legacy separate comments diff --git a/.github/workflows/pr-vercel-website-preview.yaml b/.github/workflows/pr-vercel-website-preview.yaml new file mode 100644 index 0000000000..167383ecb0 --- /dev/null +++ b/.github/workflows/pr-vercel-website-preview.yaml @@ -0,0 +1,64 @@ +--- +name: 'PR: Vercel Website Preview' + +on: + workflow_run: + workflows: ['CI: Vercel Website Preview'] + types: + - completed + +permissions: + contents: read + pull-requests: write + actions: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }} + cancel-in-progress: true + +jobs: + comment: + runs-on: ubuntu-latest + if: > + github.repository == 'Comfy-Org/ComfyUI_frontend' && + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.conclusion == 'success' + steps: + - uses: actions/checkout@v6 + + - name: Download preview metadata + uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12 + with: + name: vercel-preview + run_id: ${{ github.event.workflow_run.id }} + path: temp/vercel-preview + + - name: Resolve PR number from workflow_run context + id: pr-meta + uses: ./.github/actions/resolve-pr-from-workflow-run + + - name: Write report + if: steps.pr-meta.outputs.skip != 'true' + env: + DEPLOYED_AT: ${{ github.event.workflow_run.updated_at }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + run: | + STABLE_URL=$(cat temp/vercel-preview/stable-url.txt) + UNIQUE_URL=$(cat temp/vercel-preview/url.txt) + SHORT_SHA="${HEAD_SHA:0:7}" + cat > preview-report.md <This commit: $UNIQUE_URL + + Last updated: $DEPLOYED_AT for \`$SHORT_SHA\` + EOF + + - name: Post PR comment + if: steps.pr-meta.outputs.skip != 'true' + uses: ./.github/actions/post-pr-report-comment + with: + pr-number: ${{ steps.pr-meta.outputs.number }} + report-file: ./preview-report.md + comment-marker: '' + token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/release-biweekly-comfyui.yaml b/.github/workflows/release-biweekly-comfyui.yaml index 88881ee048..8c3f181fdb 100644 --- a/.github/workflows/release-biweekly-comfyui.yaml +++ b/.github/workflows/release-biweekly-comfyui.yaml @@ -1,14 +1,23 @@ -# Automated bi-weekly workflow to bump ComfyUI frontend RC releases -name: 'Release: Bi-weekly ComfyUI' +# Release workflow for ComfyUI frontend: version bump → PyPI publish → ComfyUI PR. +# Runs on a bi-weekly schedule for minor releases, or manually for patch/hotfix releases. +name: 'Release: ComfyUI' on: - # Schedule for Monday at 12:00 PM PST (20:00 UTC) + # Bi-weekly schedule: Monday at 20:00 UTC schedule: - cron: '0 20 * * 1' - # Allow manual triggering (bypasses bi-weekly check) + # Manual trigger for both on-demand minor and patch/hotfix releases workflow_dispatch: inputs: + release_type: + description: 'minor = next minor version (bi-weekly cadence), patch = hotfix for current production version' + required: true + default: 'minor' + type: choice + options: + - minor + - patch comfyui_fork: description: 'ComfyUI fork to use for PR (e.g., Comfy-Org/ComfyUI)' required: false @@ -41,10 +50,11 @@ jobs: - name: Summary run: | - echo "## Bi-weekly Check" >> $GITHUB_STEP_SUMMARY + echo "## Release Check" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY echo "- Is release week: ${{ steps.check.outputs.is_release_week }}" >> $GITHUB_STEP_SUMMARY echo "- Manual trigger: ${{ github.event_name == 'workflow_dispatch' }}" >> $GITHUB_STEP_SUMMARY + echo "- Release type: ${{ inputs.release_type || 'minor (scheduled)' }}" >> $GITHUB_STEP_SUMMARY resolve-version: needs: check-release-week @@ -76,6 +86,8 @@ jobs: - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 + with: + package_json_file: frontend/package.json - name: Setup Node.js uses: actions/setup-node@v6 @@ -89,6 +101,8 @@ jobs: - name: Resolve release information id: resolve working-directory: frontend + env: + RELEASE_TYPE: ${{ inputs.release_type || 'minor' }} run: | set -euo pipefail diff --git a/.github/workflows/release-version-bump.yaml b/.github/workflows/release-version-bump.yaml index 76f624800b..0b7fafb3c3 100644 --- a/.github/workflows/release-version-bump.yaml +++ b/.github/workflows/release-version-bump.yaml @@ -142,6 +142,20 @@ jobs: fi echo "✅ Branch '$BRANCH' exists" + - name: Ensure packageManager field exists + run: | + if ! grep -q '"packageManager"' package.json; then + # Old branches (e.g. core/1.42) predate the packageManager field. + # Inject it so pnpm/action-setup can resolve the version. + node -e " + const fs = require('fs'); + const pkg = JSON.parse(fs.readFileSync('package.json','utf8')); + pkg.packageManager = 'pnpm@10.33.0'; + fs.writeFileSync('package.json', JSON.stringify(pkg, null, 2) + '\n'); + " + echo "Injected packageManager into package.json for legacy branch" + fi + - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 diff --git a/.oxlintrc.json b/.oxlintrc.json index aa8a4c1ccd..19700f80c2 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -64,6 +64,7 @@ ] } ], + "no-unsafe-optional-chaining": "error", "no-self-assign": "allow", "no-unused-expressions": "off", "no-unused-private-class-members": "off", @@ -83,6 +84,7 @@ "typescript/no-unsafe-declaration-merging": "off", "typescript/no-unused-vars": "off", "unicorn/no-empty-file": "off", + "vitest/require-mock-type-parameters": "off", "unicorn/no-new-array": "off", "unicorn/no-single-promise-in-promise-methods": "off", "unicorn/no-useless-fallback-in-spread": "off", @@ -104,8 +106,7 @@ "allowInterfaces": "always" } ], - "vue/no-import-compiler-macros": "error", - "vue/no-dupe-keys": "error" + "vue/no-import-compiler-macros": "error" }, "overrides": [ { @@ -116,13 +117,60 @@ }, { "files": ["browser_tests/**/*.ts"], + "jsPlugins": ["eslint-plugin-playwright"], "rules": { "typescript/no-explicit-any": "error", "no-async-promise-executor": "error", "no-control-regex": "error", "no-useless-rename": "error", "no-unused-private-class-members": "error", - "unicorn/no-empty-file": "error" + "unicorn/no-empty-file": "error", + "playwright/consistent-spacing-between-blocks": "error", + "playwright/expect-expect": [ + "error", + { + "assertFunctionNames": [ + "recordMeasurement", + "logMeasurement", + "builderSaveAs" + ], + "assertFunctionPatterns": [ + "^expect", + "^assert", + "^verify", + "^searchAndExpect", + "waitForOpen", + "waitForClosed", + "waitForRequest" + ] + } + ], + "playwright/max-nested-describe": "error", + "playwright/no-duplicate-hooks": "error", + "playwright/no-element-handle": "error", + "playwright/no-eval": "error", + "playwright/no-focused-test": "error", + "playwright/no-force-option": "error", + "playwright/no-networkidle": "error", + "playwright/no-page-pause": "error", + "playwright/no-skipped-test": "error", + "playwright/no-unsafe-references": "error", + "playwright/no-unused-locators": "error", + "playwright/no-useless-await": "error", + "playwright/no-useless-not": "error", + "playwright/no-wait-for-navigation": "error", + "playwright/no-wait-for-selector": "error", + "playwright/no-wait-for-timeout": "error", + "playwright/prefer-hooks-on-top": "error", + "playwright/prefer-locator": "error", + "playwright/prefer-to-have-count": "error", + "playwright/prefer-to-have-length": "error", + "playwright/prefer-web-first-assertions": "error", + "playwright/prefer-native-locators": "error", + "playwright/require-to-pass-timeout": "error", + "playwright/valid-expect": "error", + "playwright/valid-expect-in-promise": "error", + "playwright/valid-title": "error" } } ] diff --git a/AGENTS.md b/AGENTS.md index 3a5eceb264..a9a77423cb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -44,6 +44,7 @@ This project uses **pnpm**. Always prefer scripts defined in `package.json` (e.g ## Build, Test, and Development Commands - `pnpm dev`: Start Vite dev server. +- `pnpm dev:cloud`: Dev server connected to cloud backend (testcloud.comfy.org) - `pnpm dev:electron`: Dev server with Electron API mocks - `pnpm build`: Type-check then production build to `dist/` - `pnpm preview`: Preview the production build locally @@ -179,6 +180,12 @@ This project uses **pnpm**. Always prefer scripts defined in `package.json` (e.g 24. Do not use function expressions if it's possible to use function declarations instead 25. Watch out for [Code Smells](https://wiki.c2.com/?CodeSmell) and refactor to avoid them +## Design Standards + +Before implementing any user-facing feature, consult the [Comfy Design Standards](https://www.figma.com/design/QreIv5htUaSICNuO2VBHw0/Comfy-Design-Standards) Figma file. Use the Figma MCP to fetch it live — the file is the single source of truth and may be updated by designers at any time. + +See `docs/guidance/design-standards.md` for Figma file keys, section node IDs, and component references. + ## Testing Guidelines See @docs/testing/\*.md for detailed patterns. @@ -226,6 +233,7 @@ See @docs/testing/\*.md for detailed patterns. - shadcn/vue: - Reka UI: - PrimeVue: +- Comfy Design Standards: - ComfyUI: - Electron: - Wiki: @@ -311,6 +319,9 @@ When referencing Comfy-Org repos: - Find existing `!important` classes that are interfering with the styling and propose corrections of those instead. - NEVER use arbitrary percentage values like `w-[80%]` when a Tailwind fraction utility exists - Use `w-4/5` instead of `w-[80%]`, `w-1/2` instead of `w-[50%]`, etc. +- NEVER use font-size classes (`text-xs`, `text-sm`, etc.) to size `icon-[...]` (iconify) icons + - Iconify icons size via `width`/`height: 1.2em`, so font-size produces unpredictable results + - Use `size-*` classes for explicit sizing, or set font-size on the **parent** container and let `1.2em` scale naturally ## Agent-only rules diff --git a/CODEOWNERS b/CODEOWNERS index 3d81c2f5a9..030e550a94 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -69,6 +69,9 @@ /src/renderer/extensions/vueNodes/widgets/composables/usePainterWidget.ts @jtydhr88 /src/lib/litegraph/src/widgets/PainterWidget.ts @jtydhr88 +# GLSL +/src/renderer/glsl/ @jtydhr88 @pythongosssss @christian-byrne + # 3D /src/extensions/core/load3d.ts @jtydhr88 /src/extensions/core/load3dLazy.ts @jtydhr88 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ef7b920948..2bfa098f65 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -62,6 +62,37 @@ python main.py --port 8188 --cpu - Run `pnpm dev:electron` to start the dev server with electron API mocked - Run `pnpm dev:cloud` to start the dev server against the cloud backend (instead of local ComfyUI server) +#### Testing with Cloud & Staging Environments + +Some features — particularly **partner/API nodes** (e.g. BFL, OpenAI, Stability AI) — require a cloud backend for authentication and billing. Running these against a local ComfyUI instance will result in permission errors or logged-out states. There are two ways to connect to a cloud/staging backend: + +**Option 1: Frontend — `pnpm dev:cloud`** + +The simplest approach. This proxies all API requests to the test cloud environment: + +```bash +pnpm dev:cloud +``` + +This sets `DEV_SERVER_COMFYUI_URL` to `https://testcloud.comfy.org/` automatically. You can also set this variable manually in your `.env` file to target a different environment: + +```bash +# .env +DEV_SERVER_COMFYUI_URL=https://stagingcloud.comfy.org/ +``` + +Any `*.comfy.org` URL automatically enables cloud mode, which includes the GCS media proxy needed for viewing generated images and videos. See [.env_example](.env_example) for all available cloud URLs. + +**Option 2: Backend — `--comfy-api-base`** + +Alternatively, launch the ComfyUI backend pointed at the staging API: + +```bash +python main.py --comfy-api-base https://stagingapi.comfy.org --verbose +``` + +Then run `pnpm dev` as usual. This keeps the frontend in local mode but routes backend API calls through staging. + #### Access dev server on touch devices Enable remote access to the dev server by setting `VITE_REMOTE_DEV` in `.env` to `true`. diff --git a/apps/desktop-ui/package.json b/apps/desktop-ui/package.json index d47be69fec..689b9cb72f 100644 --- a/apps/desktop-ui/package.json +++ b/apps/desktop-ui/package.json @@ -5,6 +5,7 @@ "scripts": { "lint": "nx run @comfyorg/desktop-ui:lint", "typecheck": "nx run @comfyorg/desktop-ui:typecheck", + "test:unit": "vitest run --config vitest.config.mts", "storybook": "storybook dev -p 6007", "build-storybook": "storybook build -o dist/storybook" }, diff --git a/apps/desktop-ui/src/components/common/StartupDisplay.test.ts b/apps/desktop-ui/src/components/common/StartupDisplay.test.ts new file mode 100644 index 0000000000..7de8561ed4 --- /dev/null +++ b/apps/desktop-ui/src/components/common/StartupDisplay.test.ts @@ -0,0 +1,97 @@ +import { render, screen } from '@testing-library/vue' +import PrimeVue from 'primevue/config' +import { describe, expect, it } from 'vitest' +import { createI18n } from 'vue-i18n' + +import StartupDisplay from '@/components/common/StartupDisplay.vue' + +const i18n = createI18n({ + legacy: false, + locale: 'en', + messages: { en: { g: { logoAlt: 'ComfyUI' } } } +}) + +const ProgressBarStub = { + props: ['mode', 'value', 'showValue'], + template: + '
' +} + +function renderDisplay( + props: { + progressPercentage?: number + title?: string + statusText?: string + hideProgress?: boolean + fullScreen?: boolean + } = {} +) { + return render(StartupDisplay, { + props, + global: { + plugins: [[PrimeVue, { unstyled: true }], i18n], + stubs: { ProgressBar: ProgressBarStub } + } + }) +} + +describe('StartupDisplay', () => { + describe('progressMode', () => { + it('renders indeterminate mode when progressPercentage is undefined', () => { + renderDisplay() + expect(screen.getByTestId('progress-bar').dataset.mode).toBe( + 'indeterminate' + ) + }) + + it('renders determinate mode when progressPercentage is provided', () => { + renderDisplay({ progressPercentage: 50 }) + expect(screen.getByTestId('progress-bar').dataset.mode).toBe( + 'determinate' + ) + }) + + it('passes progressPercentage as value to the progress bar', () => { + renderDisplay({ progressPercentage: 75 }) + expect(screen.getByTestId('progress-bar').dataset.value).toBe('75') + }) + }) + + describe('hideProgress', () => { + it('hides the progress bar when hideProgress is true', () => { + renderDisplay({ hideProgress: true }) + expect(screen.queryByTestId('progress-bar')).toBeNull() + }) + + it('shows the progress bar by default', () => { + renderDisplay() + expect(screen.getByTestId('progress-bar')).toBeDefined() + }) + }) + + describe('title', () => { + it('renders the title text when provided', () => { + renderDisplay({ title: 'Loading...' }) + expect(screen.getByText('Loading...')).toBeDefined() + }) + + it('does not render h1 when title is not provided', () => { + renderDisplay() + expect(screen.queryByRole('heading', { level: 1 })).toBeNull() + }) + }) + + describe('statusText', () => { + it('renders statusText with data-testid attribute', () => { + renderDisplay({ statusText: 'Starting server' }) + expect(screen.getByTestId('startup-status-text').textContent).toContain( + 'Starting server' + ) + }) + + it('does not render statusText element when not provided', () => { + renderDisplay() + expect(screen.queryByTestId('startup-status-text')).toBeNull() + }) + }) +}) diff --git a/apps/desktop-ui/src/components/common/UrlInput.test.ts b/apps/desktop-ui/src/components/common/UrlInput.test.ts new file mode 100644 index 0000000000..410d8f6d41 --- /dev/null +++ b/apps/desktop-ui/src/components/common/UrlInput.test.ts @@ -0,0 +1,208 @@ +import { render, screen, waitFor } from '@testing-library/vue' +import userEvent from '@testing-library/user-event' +import PrimeVue from 'primevue/config' +import { describe, expect, it, vi, beforeEach } from 'vitest' + +vi.mock('@comfyorg/shared-frontend-utils/networkUtil', () => ({ + checkUrlReachable: vi.fn() +})) + +import { checkUrlReachable } from '@comfyorg/shared-frontend-utils/networkUtil' +import UrlInput from '@/components/common/UrlInput.vue' +import { ValidationState } from '@/utils/validationUtil' + +const InputTextStub = { + props: ['modelValue', 'invalid'], + emits: ['update:modelValue', 'blur'], + template: `` +} + +const InputIconStub = { + template: '' +} + +const IconFieldStub = { + template: '
' +} + +function renderUrlInput( + modelValue = '', + validateUrlFn?: (url: string) => Promise +) { + return render(UrlInput, { + props: { modelValue, ...(validateUrlFn ? { validateUrlFn } : {}) }, + global: { + plugins: [[PrimeVue, { unstyled: true }]], + stubs: { + InputText: InputTextStub, + InputIcon: InputIconStub, + IconField: IconFieldStub + } + } + }) +} + +describe('UrlInput', () => { + beforeEach(() => { + vi.resetAllMocks() + }) + + describe('initial validation on mount', () => { + it('stays IDLE when modelValue is empty on mount', async () => { + renderUrlInput('') + await waitFor(() => { + expect(screen.getByTestId('url-input').dataset.invalid).toBe('false') + }) + }) + + it('sets VALID state when modelValue is a reachable URL on mount', async () => { + vi.mocked(checkUrlReachable).mockResolvedValue(true) + renderUrlInput('https://example.com') + await waitFor(() => { + expect(screen.getByTestId('url-input').dataset.invalid).toBe('false') + }) + }) + + it('sets INVALID state when URL is not reachable on mount', async () => { + vi.mocked(checkUrlReachable).mockResolvedValue(false) + renderUrlInput('https://unreachable.example') + await waitFor(() => { + expect(screen.getByTestId('url-input').dataset.invalid).toBe('true') + }) + }) + }) + + describe('input handling', () => { + it('resets validation state to IDLE on user input', async () => { + vi.mocked(checkUrlReachable).mockResolvedValue(false) + renderUrlInput('https://bad.example') + + await waitFor(() => { + expect(screen.getByTestId('url-input').dataset.invalid).toBe('true') + }) + + const user = userEvent.setup() + await user.type(screen.getByTestId('url-input'), 'x') + expect(screen.getByTestId('url-input').dataset.invalid).toBe('false') + }) + + it('strips whitespace from typed input', async () => { + const onUpdate = vi.fn() + render(UrlInput, { + props: { + modelValue: '', + 'onUpdate:modelValue': onUpdate + }, + global: { + plugins: [[PrimeVue, { unstyled: true }]], + stubs: { + InputText: InputTextStub, + InputIcon: InputIconStub, + IconField: IconFieldStub + } + } + }) + + const user = userEvent.setup() + const input = screen.getByTestId('url-input') + await user.type(input, 'htt ps') + expect((input as HTMLInputElement).value).not.toContain(' ') + }) + }) + + describe('blur handling', () => { + it('emits update:modelValue on blur', async () => { + const onUpdate = vi.fn() + render(UrlInput, { + props: { + modelValue: 'https://example.com', + 'onUpdate:modelValue': onUpdate + }, + global: { + plugins: [[PrimeVue, { unstyled: true }]], + stubs: { + InputText: InputTextStub, + InputIcon: InputIconStub, + IconField: IconFieldStub + } + } + }) + + const user = userEvent.setup() + await user.click(screen.getByTestId('url-input')) + await user.tab() + + expect(onUpdate).toHaveBeenCalled() + }) + + it('normalizes URL on blur', async () => { + const onUpdate = vi.fn() + render(UrlInput, { + props: { + modelValue: 'https://example.com', + 'onUpdate:modelValue': onUpdate + }, + global: { + plugins: [[PrimeVue, { unstyled: true }]], + stubs: { + InputText: InputTextStub, + InputIcon: InputIconStub, + IconField: IconFieldStub + } + } + }) + + const user = userEvent.setup() + await user.click(screen.getByTestId('url-input')) + await user.tab() + + const emittedUrl = onUpdate.mock.calls[0]?.[0] + expect(emittedUrl).toBe('https://example.com/') + }) + }) + + describe('custom validateUrlFn', () => { + it('uses custom validateUrlFn when provided', async () => { + const customValidator = vi.fn().mockResolvedValue(true) + renderUrlInput('https://custom.example', customValidator) + + await waitFor(() => { + expect(customValidator).toHaveBeenCalledWith('https://custom.example') + }) + + expect(checkUrlReachable).not.toHaveBeenCalled() + }) + }) + + describe('state-change emission', () => { + it('emits state-change when validation state changes', async () => { + const onStateChange = vi.fn() + vi.mocked(checkUrlReachable).mockResolvedValue(true) + + render(UrlInput, { + props: { + modelValue: 'https://example.com', + 'onState-change': onStateChange + }, + global: { + plugins: [[PrimeVue, { unstyled: true }]], + stubs: { + InputText: InputTextStub, + InputIcon: InputIconStub, + IconField: IconFieldStub + } + } + }) + + await waitFor(() => { + expect(onStateChange).toHaveBeenCalledWith(ValidationState.VALID) + }) + }) + }) +}) diff --git a/apps/desktop-ui/src/components/install/GpuPicker.test.ts b/apps/desktop-ui/src/components/install/GpuPicker.test.ts new file mode 100644 index 0000000000..c14c26fb1a --- /dev/null +++ b/apps/desktop-ui/src/components/install/GpuPicker.test.ts @@ -0,0 +1,112 @@ +import { render, screen } from '@testing-library/vue' +import PrimeVue from 'primevue/config' +import { describe, expect, it, vi } from 'vitest' +import { createI18n } from 'vue-i18n' + +vi.mock('@/utils/envUtil', () => ({ + electronAPI: vi.fn(() => ({ + getPlatform: vi.fn().mockReturnValue('win32') + })) +})) + +vi.mock('@/i18n', () => ({ + t: (key: string) => key, + te: () => false, + st: (_key: string, fallback: string) => fallback +})) + +import type { TorchDeviceType } from '@comfyorg/comfyui-electron-types' +import GpuPicker from '@/components/install/GpuPicker.vue' + +const i18n = createI18n({ + legacy: false, + locale: 'en', + missingWarn: false, + fallbackWarn: false, + messages: { en: {} } +}) + +const HardwareOptionStub = { + props: ['imagePath', 'placeholderText', 'subtitle', 'selected'], + emits: ['click'], + template: + '' +} + +function renderPicker(device: TorchDeviceType | null = null) { + return render(GpuPicker, { + props: { device }, + global: { + plugins: [[PrimeVue, { unstyled: true }], i18n], + stubs: { + HardwareOption: HardwareOptionStub, + Tag: { + props: ['value'], + template: '{{ value }}' + } + } + } + }) +} + +describe('GpuPicker', () => { + describe('recommended badge', () => { + it('shows recommended badge for nvidia', () => { + renderPicker('nvidia') + expect(screen.getByTestId('recommended-tag')).toBeVisible() + }) + + it('shows recommended badge for amd', () => { + renderPicker('amd') + expect(screen.getByTestId('recommended-tag')).toBeVisible() + }) + + it('does not show recommended badge for cpu', () => { + renderPicker('cpu') + expect(screen.getByTestId('recommended-tag')).not.toBeVisible() + }) + + it('does not show recommended badge for unsupported', () => { + renderPicker('unsupported') + expect(screen.getByTestId('recommended-tag')).not.toBeVisible() + }) + + it('does not show recommended badge when no device is selected', () => { + renderPicker(null) + expect(screen.getByTestId('recommended-tag')).not.toBeVisible() + }) + }) + + describe('selection state', () => { + it('marks nvidia as selected when device is nvidia', () => { + renderPicker('nvidia') + expect(screen.getByTestId('NVIDIA').dataset.selected).toBe('true') + }) + + it('marks cpu as selected when device is cpu', () => { + renderPicker('cpu') + expect(screen.getByTestId('CPU').dataset.selected).toBe('true') + }) + + it('marks unsupported as selected when device is unsupported', () => { + renderPicker('unsupported') + expect(screen.getByTestId('Manual Install').dataset.selected).toBe('true') + }) + + it('no option is selected when device is null', () => { + renderPicker(null) + expect(screen.getByTestId('CPU').dataset.selected).toBe('false') + expect(screen.getByTestId('NVIDIA').dataset.selected).toBe('false') + }) + }) + + describe('gpu options on non-darwin platform', () => { + it('shows NVIDIA, AMD, CPU, and Manual Install options', () => { + renderPicker(null) + expect(screen.getByTestId('NVIDIA')).toBeDefined() + expect(screen.getByTestId('AMD')).toBeDefined() + expect(screen.getByTestId('CPU')).toBeDefined() + expect(screen.getByTestId('Manual Install')).toBeDefined() + }) + }) +}) diff --git a/apps/desktop-ui/src/components/install/MigrationPicker.test.ts b/apps/desktop-ui/src/components/install/MigrationPicker.test.ts new file mode 100644 index 0000000000..91a617b9c2 --- /dev/null +++ b/apps/desktop-ui/src/components/install/MigrationPicker.test.ts @@ -0,0 +1,223 @@ +import { render, screen, waitFor } from '@testing-library/vue' +import userEvent from '@testing-library/user-event' +import PrimeVue from 'primevue/config' +import { describe, expect, it, vi, beforeEach } from 'vitest' +import { createI18n } from 'vue-i18n' + +const mockValidateComfyUISource = vi.fn() +const mockShowDirectoryPicker = vi.fn() + +vi.mock('@/utils/envUtil', () => ({ + electronAPI: vi.fn(() => ({ + validateComfyUISource: mockValidateComfyUISource, + showDirectoryPicker: mockShowDirectoryPicker + })) +})) + +import MigrationPicker from '@/components/install/MigrationPicker.vue' + +const i18n = createI18n({ + legacy: false, + locale: 'en', + messages: { + en: { + install: { + migrationSourcePathDescription: 'Source path description', + migrationOptional: 'Migration is optional', + selectItemsToMigrate: 'Select items to migrate', + pathValidationFailed: 'Validation failed', + failedToSelectDirectory: 'Failed to select directory', + locationPicker: { + migrationPathPlaceholder: 'Enter path' + } + } + } + } +}) + +const InputTextStub = { + props: ['modelValue', 'invalid'], + emits: ['update:modelValue'], + template: `` +} + +const CheckboxStub = { + props: ['modelValue', 'inputId', 'binary'], + emits: ['update:modelValue', 'click'], + template: `` +} + +function renderPicker(sourcePath = '', migrationItemIds: string[] = []) { + return render(MigrationPicker, { + props: { sourcePath, migrationItemIds }, + global: { + plugins: [[PrimeVue, { unstyled: true }], i18n], + stubs: { + InputText: InputTextStub, + Checkbox: CheckboxStub, + Button: { template: '