diff --git a/.agents/skills/removing-typescript-suppressions/SKILL.md b/.agents/skills/removing-typescript-suppressions/SKILL.md new file mode 100644 index 00000000000..9f975e376d9 --- /dev/null +++ b/.agents/skills/removing-typescript-suppressions/SKILL.md @@ -0,0 +1,198 @@ +--- +name: removing-typescript-suppressions +description: Replaces @ts-expect-error and @ts-ignore directives with minimal type-safe fixes. Use when removing TypeScript compiler suppressions. +--- + +# Removing TypeScript suppressions + +Replace suppressions by repairing the contract that caused the compiler error. +Do not silence the error elsewhere or widen types beyond the real runtime shape. + +## Workflow + +### 1. Establish the scope + +Read `docs/guidance/typescript.md`, `docs/guidance/engineering.md`, and the +guidance for the affected files. Identify the correct comparison ref and list +only suppressions introduced by the change under review: + +```bash +git diff --unified=2 ...HEAD -- \ + | rg -n '^\+.*@ts-(expect-error|ignore)|^diff --git|^@@' +``` + +Do not expand the task to old suppressions unless the user asks. Preserve a +suppression used deliberately to test a compiler error when that error is the +behavior under test. + +### 2. Read the ownership path + +Read each affected file and trace the value to its source type, runtime owner, +and consumers. Check declarations, store types, generated API types, library +return types, test globals, and the relevant `tsconfig` before choosing a fix. + +Do not accept the suppression comment as the diagnosis. Verify the failing code +path and runtime shape. + +### 3. Expose the real errors + +Remove the in-scope suppressions, then run the narrowest typecheck that owns the +files. Use the compiler diagnostics to group failures by root cause instead of +patching each line independently. + +Common commands in this repository: + +```bash +pnpm typecheck +pnpm typecheck:browser +pnpm typecheck:desktop +pnpm typecheck:scripts +pnpm typecheck:website +``` + +### 4. Repair the contract + +Prefer, in order: + +1. correct types at the source +2. control-flow narrowing +3. an existing domain type or generated API type +4. `unknown` at a genuine compatibility boundary, followed by runtime narrowing + +Avoid `any`, `as any`, a replacement assertion, a broader optional type, or a +new wrapper that merely hides the mismatch. Keep public API types stable and do +not expose internal store types through public facades. + +### 5. Verify behavior and absence + +Run the owning typecheck, focused tests, lint, formatting, and a whitespace +check. Use `pnpm exec vitest run` for Vitest, `pnpm test:browser:local` or +`pnpm test:browser` for Playwright, and the owning repository script for other +test types. Confirm that the diff adds no suppression: + +```bash +pnpm exec eslint +pnpm exec oxfmt --check +git diff --check ...HEAD -- +git diff --unified=0 ...HEAD -- \ + | rg '^\+.*@ts-(expect-error|ignore)' +``` + +The final `rg` command should return no matches. Run broader checks when the fix +changes a shared type, public contract, store, or cross-package boundary. + +## Repair patterns + +### Narrow optional browser globals once + +Copy an optional global to a local and guard it. This preserves narrowing across +callbacks and asynchronous code. + +```typescript +const app = window.app +if (!app) throw new Error('ComfyUI app is not initialized') + +await app.api.getNodeDefs() +``` + +### Use the owner instead of casting a facade + +When a public facade intentionally omits internal collections, import the store +or service that owns those collections. Do not widen the facade or cast through +it for one caller. + +### Narrow value-or-factory unions + +Resolve callbacks before using their values: + +```typescript +const label = + typeof command.label === 'function' ? command.label() : command.label +``` + +Resolve a factory default before passing it to another resolver. Keep untyped +legacy data as `unknown`. Narrow objects before reading properties, functions +before calling them, and returned values before use. + +### Read map identity from `Object.entries` + +If an object's key is the identifier, do not invent an `id` property on its +values: + +```typescript +Object.entries(dialogs).map(([id, dialog]) => ({ id, title: dialog.title })) +``` + +### Handle nullable factories before dereferencing + +Respect library return types such as `LiteGraph.createNode(): LGraphNode | null`: + +```typescript +const node = liteGraph.createNode(nodeName, displayName) +if (!node?.widgets?.length) return {} +``` + +Use optional chaining only when the missing value and the empty value have the +same behavior. Otherwise, use a guard with a useful error. + +### Use named payloads across callback boundaries + +Heterogeneous array arguments often lose positional types in `page.evaluate` +and similar APIs. Pass an object instead of asserting a tuple: + +```typescript +await page.evaluate( + ({ nodeName, displayName, inputNames }) => { + // use the independently typed fields + }, + { nodeName, displayName, inputNames } +) +``` + +### Validate serialization boundaries + +Type values produced by code you own before passing them to +`Object.fromEntries`. If JSON or browser data arrives as `unknown`, validate its +full nested shape before assigning a domain type. A result annotation does not +validate data. + +### Make test preconditions executable + +If a test needs optional output, narrow it with a runtime guard that produces a +clear behavioral failure: + +```typescript +if (!serialized.inputs || !serialized.outputs) { + throw new Error('Expected serialized node labels') +} +``` + +Do not replace the suppression with a non-null assertion. + +### Install minimal test globals without claiming full DOM types + +When Node tests need a small browser shim, add the property to the host object +instead of assigning `{}` to a full `Window` type: + +```typescript +if (typeof window === 'undefined') { + Object.assign(globalThis, { window: {} }) +} +``` + +### Delete documentation-only values + +If strict checking reports an unused constant kept only as documentation, +delete it. Put useful context on the value that the code actually checks. + +## Guardrails + +- Do not hand-declare server response types. Import generated shared types. +- Do not grow `ExtensionManager` or another public API to expose private state. +- Put reusable type guards in leaf modules with runtime-free `import type` + dependencies. +- Do not change runtime behavior while repairing types unless the old code was + demonstrably inconsistent with its runtime contract. Cover such a bug with a + focused test. +- Treat review findings and their proposed fixes as claims. Reproduce the error + against current code, then fix or reject each finding with concrete evidence. diff --git a/.github/workflows/pr-cursor-review.yaml b/.github/workflows/pr-cursor-review.yaml index 6f07b26d341..ac4634458c8 100644 --- a/.github/workflows/pr-cursor-review.yaml +++ b/.github/workflows/pr-cursor-review.yaml @@ -29,7 +29,7 @@ jobs: # SHA-pinned per zizmor `unpinned-uses: hash-pin`. Bump this SHA to pick up # upstream changes; keep `workflows_ref` matching so prompts/scripts load # from the same commit as the workflow definition. - uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@732100bee739ef94fe650016b382a78b377c0af7 # github-workflows main (732100b) + uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@41fb8315f0b81c5fa33dd1edc4bb5004609ba51d # github-workflows main (41fb831) with: # Overriding diff_excludes replaces the reusable default wholesale, so # this restates the generated/vendored defaults and adds this repo's heavy @@ -48,7 +48,7 @@ jobs: :!**/*-snapshots/** :!src/workbench/extensions/manager/types/generatedManagerTypes.ts # Load the prompts/scripts from the same ref as `uses:`. - workflows_ref: 732100bee739ef94fe650016b382a78b377c0af7 + workflows_ref: 41fb8315f0b81c5fa33dd1edc4bb5004609ba51d secrets: CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} # Optional — enables start/complete Slack DMs to the triggerer. diff --git a/.github/workflows/release-recover-tag.yaml b/.github/workflows/release-recover-tag.yaml new file mode 100644 index 00000000000..e6deaa373a9 --- /dev/null +++ b/.github/workflows/release-recover-tag.yaml @@ -0,0 +1,104 @@ +--- +# Creates the release a merged version bump never got. +name: 'Release: Recover Tag' + +on: + workflow_dispatch: + inputs: + branch: + description: 'Branch whose package.json version has no tag (e.g. core/1.48)' + required: true + type: string + +concurrency: + group: release-recover-tag-${{ inputs.branch }} + cancel-in-progress: false + +jobs: + recover: + if: github.repository == 'Comfy-Org/ComfyUI_frontend' + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Checkout branch + uses: actions/checkout@v7 + with: + ref: ${{ inputs.branch }} + fetch-depth: 0 + + - name: Resolve the untagged version + id: version + env: + BRANCH: ${{ inputs.branch }} + run: | + set -euo pipefail + VERSION=$(node -p "require('./package.json').version") + + if git rev-parse -q --verify "refs/tags/v${VERSION}" >/dev/null; then + echo "::error title=Nothing to recover::v${VERSION} already exists, so ${BRANCH} is not missing its tag. Cut a new patch with release-version-bump.yaml instead." + exit 1 + fi + + if [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+- ]]; then + echo "is_prerelease=true" >> "$GITHUB_OUTPUT" + else + echo "is_prerelease=false" >> "$GITHUB_OUTPUT" + fi + + echo "version=$VERSION" >> "$GITHUB_OUTPUT" + echo "Recovering v${VERSION} on ${BRANCH}" + + - name: Install pnpm + uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6.0.9 + + - uses: actions/setup-node@v6 + with: + node-version-file: '.nvmrc' + cache: 'pnpm' + + # Same artifacts release-draft-create.yaml attaches, built the same way: + # a recovered release must be indistinguishable from an on-time one. + - name: Build project + env: + SENTRY_DSN: ${{ secrets.SENTRY_DSN }} + ALGOLIA_APP_ID: ${{ secrets.ALGOLIA_APP_ID }} + ALGOLIA_API_KEY: ${{ secrets.ALGOLIA_API_KEY }} + ENABLE_MINIFY: 'true' + USE_PROD_CONFIG: 'true' + IS_NIGHTLY: ${{ inputs.branch == 'main' }} + run: | + pnpm install --frozen-lockfile + + DISTRIBUTION=desktop pnpm build + pnpm zipdist ./dist ./dist-desktop.zip + + pnpm build + pnpm zipdist + + - name: Create release + uses: softprops/action-gh-release@a06a81a03ee405af7f2048a818ed3f03bbf83c7b # v2.5.0 + with: + token: ${{ secrets.PR_GH_TOKEN }} + files: | + dist.zip + dist-desktop.zip + tag_name: v${{ steps.version.outputs.version }} + target_commitish: ${{ inputs.branch }} + make_latest: >- + ${{ inputs.branch == 'main' && + steps.version.outputs.is_prerelease == 'false' }} + draft: ${{ steps.version.outputs.is_prerelease == 'true' }} + prerelease: >- + ${{ steps.version.outputs.is_prerelease == 'true' }} + generate_release_notes: true + + - name: Summary + run: | + { + echo "## Recovered release" + echo + echo "- Branch: \`${{ inputs.branch }}\`" + echo "- Tag: \`v${{ steps.version.outputs.version }}\`" + echo + echo "Re-run \`release-weekly-comfyui.yaml\` to publish it to PyPI and open the ComfyUI pin PR." + } >> "$GITHUB_STEP_SUMMARY" diff --git a/apps/website/e2e/mcp.spec.ts b/apps/website/e2e/mcp.spec.ts index 0288e20fca8..878a8b7e4c3 100644 --- a/apps/website/e2e/mcp.spec.ts +++ b/apps/website/e2e/mcp.spec.ts @@ -31,11 +31,14 @@ test.describe('MCP page @smoke', () => { } }) - test('Claude Desktop is the default tab and shows only the connector card', async ({ + test('cloud is the default connection with Claude Desktop active', async ({ page }) => { const setup = page.locator('#setup') await setup.scrollIntoViewIfNeeded() + await expect( + setup.getByRole('tab', { name: /Comfy Cloud/ }) + ).toHaveAttribute('data-state', 'active') await expect( setup.getByRole('tab', { name: 'Claude Desktop' }) ).toHaveAttribute('data-state', 'active') @@ -56,7 +59,11 @@ test.describe('MCP page @smoke', () => { }) => { const setup = page.locator('#setup') await setup.scrollIntoViewIfNeeded() - const activePanel = setup.locator('[role="tabpanel"][data-state="active"]') + // Nested tabs: the connection panel and the client panel are both active, + // so target the innermost (client) panel. + const activePanel = setup + .locator('[role="tabpanel"][data-state="active"]') + .last() const agentHeading = setup.getByRole('heading', { name: 'Ask your agent to install Comfy MCP' }) @@ -110,6 +117,59 @@ test.describe('MCP page @smoke', () => { ).toHaveAttribute('href', 'https://github.com/Comfy-Org/comfy-skills') }) + test('local connection tab swaps in the open-source install flow', async ({ + page + }) => { + const setup = page.locator('#setup') + await setup.scrollIntoViewIfNeeded() + + const localTab = setup.getByRole('tab', { name: /Local ComfyUI/ }) + await expect(async () => { + await localTab.click() + await expect(localTab).toHaveAttribute('data-state', 'active', { + timeout: 500 + }) + }).toPass() + + await expect( + setup.getByRole('heading', { name: 'Install the server' }) + ).toBeVisible() + await expect( + setup.getByText('pip install comfy-mcp', { exact: true }) + ).toBeVisible() + + // Claude Code is the default local client and pairs with the agent card. + await expect( + setup.getByText('claude mcp add comfy-mcp -- comfy-mcp', { exact: true }) + ).toBeVisible() + await expect( + setup.getByRole('heading', { + name: 'Ask your agent to install Comfy MCP' + }) + ).toBeVisible() + + // The open-source requirement line replaces the subscription note. + await expect( + setup.getByRole('link', { name: 'open source on GitHub' }) + ).toHaveAttribute('href', 'https://github.com/Comfy-Org/comfy-mcp') + await expect( + setup.getByRole('link', { name: 'subscription of any tier' }) + ).toHaveCount(0) + + // Client tabs inside the local panel swap the per-client instructions. + await selectClientTab(setup, 'Cursor') + await expect( + setup.locator('[role="tabpanel"][data-state="active"]').last() + ).toContainText('.cursor/mcp.json') + + // Switching back to cloud restores the subscription note and endpoint. + await setup.getByRole('tab', { name: /Comfy Cloud/ }).click() + await expect( + setup.getByRole('link', { name: 'subscription of any tier' }) + ).toBeVisible() + await expect(setup.getByText(MCP_ENDPOINT, { exact: true })).toBeVisible() + }) + test('capabilities section shows all six tool cards', async ({ page }) => { for (const title of [ 'Generate anything', @@ -125,6 +185,20 @@ test.describe('MCP page @smoke', () => { } }) + test('production use cases section offers a click-to-play walkthrough', async ({ + page + }) => { + const heading = page.getByRole('heading', { + name: 'Production use cases.' + }) + await heading.scrollIntoViewIfNeeded() + await expect(heading).toBeVisible() + + const video = page.getByLabel(/running production jobs/) + await expect(video).toHaveAttribute('poster', /production-use-cases/) + await expect(video).not.toHaveAttribute('autoplay', /.*/) + }) + test('FAQ lists nine questions and autolinks the server URL', async ({ page }) => { diff --git a/apps/website/src/components/blocks/FeatureRows01.vue b/apps/website/src/components/blocks/FeatureRows01.vue index 246bc08821b..31aec198162 100644 --- a/apps/website/src/components/blocks/FeatureRows01.vue +++ b/apps/website/src/components/blocks/FeatureRows01.vue @@ -49,6 +49,10 @@ const { {{ heading }} +
+ +
+
+ + - diff --git a/apps/website/src/pages/zh-CN/mcp.astro b/apps/website/src/pages/zh-CN/mcp.astro index 8f06345a2a5..f24caeca267 100644 --- a/apps/website/src/pages/zh-CN/mcp.astro +++ b/apps/website/src/pages/zh-CN/mcp.astro @@ -5,6 +5,7 @@ import HeroSection from '../../templates/mcp/HeroSection.vue' import SetupSection from '../../templates/mcp/SetupSection.vue' import WhySection from '../../templates/mcp/WhySection.vue' import ToolsSection from '../../templates/mcp/ToolsSection.vue' +import UseCasesSection from '../../templates/mcp/UseCasesSection.vue' import HowItWorksSection from '../../templates/mcp/HowItWorksSection.vue' import FAQSection from '../../templates/mcp/FAQSection.vue' import { t } from '../../i18n/translations' @@ -16,8 +17,9 @@ import { t } from '../../i18n/translations' > + + - diff --git a/apps/website/src/scripts/posthog.ts b/apps/website/src/scripts/posthog.ts index d3891c2c2b4..44057c35a68 100644 --- a/apps/website/src/scripts/posthog.ts +++ b/apps/website/src/scripts/posthog.ts @@ -50,6 +50,15 @@ export function captureDownloadClick(platform: Platform) { } } +export function captureMcpConnectionTabClick(connection: string) { + if (!initialized) return + try { + posthog.capture('website:mcp_connection_tab_clicked', { connection }) + } catch (error) { + console.error('PostHog MCP connection tab capture failed', error) + } +} + export function captureMcpClientTabClick(client: string) { if (!initialized) return try { diff --git a/apps/website/src/templates/drops/DropsSection.vue b/apps/website/src/templates/drops/DropsSection.vue index 04742b52492..7aae92a28c4 100644 --- a/apps/website/src/templates/drops/DropsSection.vue +++ b/apps/website/src/templates/drops/DropsSection.vue @@ -21,7 +21,7 @@ const items = computed(() => type: drop.media.type, src: drop.media.src, alt: drop.media.alt[locale], - poster: drop.media.poster + poster: drop.media.type === 'video' ? drop.media.poster : undefined }, cta: { label: drop.cta.label[locale], diff --git a/apps/website/src/templates/mcp/SetupSection.test.ts b/apps/website/src/templates/mcp/SetupSection.test.ts new file mode 100644 index 00000000000..d2ff72510ca --- /dev/null +++ b/apps/website/src/templates/mcp/SetupSection.test.ts @@ -0,0 +1,114 @@ +// @vitest-environment happy-dom +import userEvent from '@testing-library/user-event' +import { render, screen } from '@testing-library/vue' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import SetupSection from './SetupSection.vue' + +const { connectionSpy, clientSpy } = vi.hoisted(() => ({ + connectionSpy: vi.fn(), + clientSpy: vi.fn() +})) + +vi.mock('../../scripts/posthog', () => ({ + captureMcpConnectionTabClick: connectionSpy, + captureMcpClientTabClick: clientSpy +})) + +const MCP_ENDPOINT = 'https://cloud.comfy.org/mcp' + +function renderSetup() { + return render(SetupSection, { + props: { locale: 'en' }, + // The walkthrough clip is irrelevant here and