Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 198 additions & 0 deletions .agents/skills/removing-typescript-suppressions/SKILL.md
Original file line number Diff line number Diff line change
@@ -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 <base-ref>...HEAD -- <paths> \
| 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 <changed-files>
pnpm exec oxfmt --check <changed-files>
git diff --check <base-ref>...HEAD -- <paths>
git diff --unified=0 <base-ref>...HEAD -- <paths> \
| 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.
4 changes: 2 additions & 2 deletions .github/workflows/pr-cursor-review.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
104 changes: 104 additions & 0 deletions .github/workflows/release-recover-tag.yaml
Original file line number Diff line number Diff line change
@@ -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"
Loading
Loading