Skip to content

Commit f5443b4

Browse files
committed
Merge branch 'main' into matt/fe-1103-harden-assets-cursor-pagination-in-flight-tracking
Resolves a semantic conflict that no textual conflict surfaced: main taught the flat-output pager to treat an empty-string next_cursor as a real cursor (after: '') rather than falling back to offset paging, while this branch had rewritten the refresh path around `page.next_cursor || undefined`, which collapses '' back to undefined. Adopts main's semantics in the refresh path so the cursor it seeds survives into the subsequent loadMore, keeping main's "threads an empty-string cursor into after instead of falling back to offset" coverage green in the merged result.
2 parents 666f8dd + c814b8e commit f5443b4

3,374 files changed

Lines changed: 416775 additions & 80730 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.agents/checks/adr-compliance.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,15 @@ Flag:
3333
- **New circular entity dependencies** — New circular imports between `LGraph``Subgraph`, `LGraphNode``LGraphCanvas`, or similar entity classes.
3434
- **Direct `graph._version++`** — Mutating the private version counter directly instead of through a public API. Extensions already depend on this side-channel; it must become a proper API.
3535

36-
### Centralized Registries and ECS-Style Access
36+
### Dedicated Stores and Data/Behavior Separation
3737

38-
All entity data access should move toward centralized query patterns, not instance property access.
38+
Entity data lives in dedicated Pinia stores keyed by string IDs (`widgetValueStore`, `domWidgetStore`, `layoutStore`, `nodeOutputStore`, `subgraphNavigationStore`, `previewExposureStore`), not on entity instances.
3939

4040
Flag:
4141

42-
- **New instance method/property patterns** — Adding `node.someProperty` or `node.someMethod()` for data that should be a component in the World, queried via `world.getComponent(entityId, ComponentType)`.
42+
- **New instance method/property patterns** — Adding `node.someProperty` or `node.someMethod()` for data that belongs in a dedicated store (e.g. widget values → `widgetValueStore` keyed by `WidgetId`).
4343
- **OOP inheritance for entity modeling** — Extending entity classes with new subclasses instead of composing behavior through components and systems.
44-
- **Scattered state**New entity state stored in multiple locations (class properties, stores, local variables) instead of being consolidated in the World or in a single store.
44+
- **Duplicated authority**Storing the same entity state in both a class property and a store, or across two stores, so ownership becomes ambiguous. Each piece of state should have one owning store.
4545

4646
### Extension Ecosystem Impact
4747

.agents/checks/pattern-compliance.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Steps:
2222
- Explicit return types on exported functions
2323
- Use `es-toolkit` for utility functions, NOT lodash. Flag any new `import ... from 'lodash'` or `import ... from 'lodash/*'`
2424
- Never use `z.any()` in Zod schemas — use `z.unknown()` and narrow
25+
- Never hand-declare or duplicate server/API response types locally — flag any local interface/type that reimplements a type already generated from an OpenAPI spec (e.g. `@comfyorg/ingest-types`, `@comfyorg/registry-types` in `packages/`) instead of importing it. This caused real bugs in PR #14771 (see `docs/guidance/typescript.md`)
2526

2627
### Vue (if applicable)
2728

.agents/checks/playwright-e2e.md

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,20 @@ tools: [Read, Grep]
77

88
You are reviewing Playwright E2E test code in `browser_tests/`. Focus on issues a **reviewer** would catch that an author might miss — flakiness risks, fixture misuse, test isolation problems, and convention violations.
99

10+
**Rulebook — single source of truth:** `browser_tests/README.md` is the canonical
11+
browser-test guide. Every rule below is enforced against it; when in doubt, read
12+
that file. The checks in this profile are the reviewer-facing subset of that
13+
guide — they do not add new conventions of their own.
14+
1015
Reference docs (read if you need full context):
1116

12-
- `browser_tests/README.md` — setup, patterns, screenshot workflow
13-
- `browser_tests/AGENTS.md` — directory structure, fixture overview
14-
- `docs/guidance/playwright.md` — type assertion rules, test tags, forbidden patterns
15-
- `.claude/skills/writing-playwright-tests/SKILL.md` — anti-patterns, retry patterns, Vue Nodes vs LiteGraph decision guide
17+
- **`browser_tests/README.md`** — canonical guide: setup, directory structure,
18+
writing conventions, typed mocks, flake prevention, screenshot workflow
19+
- `.claude/skills/writing-playwright-tests/SKILL.md` — authoring anti-patterns,
20+
retry patterns, Vue Nodes vs LiteGraph decision guide
21+
- `.claude/skills/hardening-flaky-e2e-tests/SKILL.md` — flake transforms
22+
- `browser_tests/AGENTS.md` and `docs/guidance/playwright.md` are stubs that
23+
redirect to the canonical guide
1624

1725
## Checks
1826

@@ -47,9 +55,9 @@ Reference docs (read if you need full context):
4755

4856
### Convention Violations (Minor)
4957

50-
11. **Missing test tags**Every `test.describe` should have `tag` with at least one of: `@smoke`, `@slow`, `@screenshot`, `@canvas`, `@node`, `@widget`, `@mobile`, `@2x`. See `.claude/skills/writing-playwright-tests/SKILL.md` for when to use each.
58+
11. **Missing project-routing tags**Project-routing tags are load-bearing: `playwright.config.ts` selects which project/run a test lands in by grepping `@mobile`, `@2x`, `@0.5x`, `@perf`, `@audit`, `@cloud`, `@oss`. A test that must run in one of those projects but lacks the tag silently won't run there. Organizational tags (`@smoke`, `@slow`, `@screenshot`, `@canvas`, `@node`, `@widget`, `@vue-nodes`, `@subgraph`, `@ui`) are for `--grep` filtering and are encouraged but not mandatory. See `browser_tests/README.md` → Test Tags.
5159

52-
12. **`as any` type assertions** — Forbidden in E2E tests. Use specific type assertions or test-local type helpers. See `docs/guidance/playwright.md` for acceptable patterns.
60+
12. **`as any` type assertions** — Forbidden in E2E tests. Use specific type assertions or test-local type helpers. See `browser_tests/README.md` → Type safety for acceptable patterns.
5361

5462
13. **Screenshot tests without masking dynamic content** — Timestamps, version numbers, or other non-deterministic content in screenshots will cause flakes. Use `mask` option.
5563

@@ -65,10 +73,22 @@ Reference docs (read if you need full context):
6573

6674
18. **Vue Nodes / LiteGraph mismatch** — If testing Vue-rendered node UI (DOM widgets, CSS states), should use `comfyPage.vueNodes.*`. If testing canvas interactions/connections, should use `comfyPage.nodeOps.*`. Mixing both in one test is a smell.
6775

76+
### Structure, Types & Regressions (Medium)
77+
78+
19. **Non-test code inline in the spec** — Flag free-standing functions/constants at the top of a spec that do setup, wire locators, or drive reusable dialog interactions (e.g. a "close the templates dialog" helper). These belong in `browser_tests/fixtures/components/` (page objects), `fixtures/helpers/`, or a Playwright fixture. Do NOT flag top-level Playwright hooks or configuration — `test.use()`, `test.describe.configure()`, and file-level `test.beforeEach()`/`test.afterEach()` legitimately live in the spec. The target is free-standing helpers/constants/locator wiring, not Playwright's own API surface. See `browser_tests/README.md` → Test structure.
79+
80+
20. **Inline-declared types in specs/mocks** — Never hand-write `interface`/`type` shapes for API payloads, node definitions, or store data inside a spec or mock file. Must import the real type — generated packages (`@comfyorg/ingest-types`, `@comfyorg/registry-types`, `generatedManagerTypes.ts`) or `src/` Zod schemas. This requirement also applies to typed mock/data factories under `browser_tests/fixtures/data/` — the imported-type rule is enforced there despite the general `browser_tests/fixtures/` exclusion below. See `browser_tests/README.md` → Type safety and Test Data & Typed Mocks.
81+
82+
21. **Spec not nested in a feature folder** — New specs must live in a `tests/` subfolder mirroring the feature under test, not dumped at the top level of `tests/`. Flag new top-level `browser_tests/tests/*.spec.ts` additions. See `browser_tests/README.md` → Spec File Placement.
83+
84+
22. **Stale spec filename** — A spec's filename must describe the coverage it actually holds. Flag renamed/rescoped tests whose filename no longer matches their content. See `browser_tests/README.md` → Spec File Placement.
85+
86+
23. **Test edit that may undo a prior fix** — When a diff removes or weakens an existing assertion, wait, or timeout in a test, confirm the change isn't silently undoing a deliberate regression/flake fix. The author should have run a `git blame` regression-detection pass; a reviewer flags removals of load-bearing guards whose history shows they fixed a specific race or bug. See `browser_tests/README.md` → Before changing an existing test.
87+
6888
## Rules
6989

7090
- Only review `.spec.ts` files and supporting code in `browser_tests/`
71-
- Do NOT flag patterns in fixture/helper code (`browser_tests/fixtures/`) — those are shared infrastructure with different rules
72-
- "Major" for flakiness risks (items 1-7), "medium" for fixture misuse (8-10), "minor" for convention violations (11-15), "nitpick" for test design (16-18)
91+
- Do NOT flag patterns in fixture/helper code (`browser_tests/fixtures/`) — those are shared infrastructure with different rules (exception: rule 20's imported-type requirement still applies to typed mock/data factories under `browser_tests/fixtures/data/`)
92+
- "Major" for flakiness risks (items 1-7), "medium" for fixture misuse (8-10), "minor" for convention violations (11-15), "nitpick" for test design (16-18), "medium" for structure/type/regression rules (19-23)
7393
- When flagging missing fixture usage (item 8), confirm the helper exists by checking the fixture code — don't assume
7494
- Existing tests that predate conventions are acceptable to modify but not required to fix

.agents/checks/test-quality.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ Rules:
3131

3232
- Tests use **Vitest** (not Jest) — run with `pnpm test:unit`
3333
- Test files are **colocated**: `MyComponent.test.ts` next to `MyComponent.vue`
34-
- Use `@vue/test-utils` for component testing, `@pinia/testing` (`createTestingPinia`) for store tests
34+
- Use `@testing-library/vue` with `@testing-library/user-event` for new
35+
component tests, and `@pinia/testing` (`createTestingPinia`) for store tests
3536
- Browser/E2E tests use **Playwright** in `browser_tests/` — run with `pnpm test:browser:local`
3637
- Mock composables using the singleton factory pattern inside `vi.mock()` — see `docs/testing/unit-testing.md` for the pattern
3738
- Never use `any` in test code either — proper typing applies to tests too

.agents/resume

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#!/usr/bin/env bash
2+
set -euo pipefail
3+
4+
repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
5+
node_major="$(tr -d '[:space:]' < "$repo_root/.nvmrc")"
6+
node_home="$HOME/.local/share/comfyui-frontend-node-v$node_major"
7+
export PATH="$node_home/bin:$PATH"
8+
9+
[[ -x "$node_home/bin/node" && -x "$node_home/bin/pnpm" ]]
10+
expected_pnpm="$("$node_home/bin/node" -e "console.log(require(process.argv[1]).packageManager.split('@')[1])" "$repo_root/package.json")"
11+
actual_node="$("$node_home/bin/node" --version)"
12+
actual_pnpm="$("$node_home/bin/pnpm" --version)"
13+
14+
[[ "$actual_node" == v"$node_major".* ]]
15+
[[ "$actual_pnpm" == "$expected_pnpm" ]]
16+
17+
echo "Orb toolchain ready: Node.js $actual_node, pnpm $actual_pnpm"

.agents/setup

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
#!/usr/bin/env bash
2+
set -euo pipefail
3+
4+
repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
5+
cd "$repo_root"
6+
7+
if [[ "${AMP_ORB:-}" == 1 ]]; then
8+
gh auth setup-git
9+
10+
if ! command -v docker >/dev/null 2>&1 ||
11+
! command -v dockerd >/dev/null 2>&1; then
12+
echo "Installing Docker"
13+
sudo install -m 0755 -d /etc/apt/keyrings
14+
sudo curl -fsSL https://download.docker.com/linux/debian/gpg \
15+
-o /etc/apt/keyrings/docker.asc
16+
sudo chmod a+r /etc/apt/keyrings/docker.asc
17+
echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.asc] https://download.docker.com/linux/debian $(. /etc/os-release && echo "$VERSION_CODENAME") stable" |
18+
sudo tee /etc/apt/sources.list.d/docker.list >/dev/null
19+
sudo apt-get update
20+
sudo apt-get install -y \
21+
docker-ce \
22+
docker-ce-cli \
23+
containerd.io \
24+
docker-buildx-plugin \
25+
docker-compose-plugin
26+
fi
27+
fi
28+
29+
node_major="$(tr -d '[:space:]' < "$repo_root/.nvmrc")"
30+
node_home="$HOME/.local/share/comfyui-frontend-node-v$node_major"
31+
32+
install_node() {
33+
local architecture
34+
case "$(uname -m)" in
35+
x86_64) architecture=x64 ;;
36+
aarch64 | arm64) architecture=arm64 ;;
37+
*)
38+
echo "Unsupported architecture: $(uname -m)" >&2
39+
return 1
40+
;;
41+
esac
42+
43+
local release_base="https://nodejs.org/dist/latest-v${node_major}.x"
44+
local archive
45+
archive="$(curl -fsSL "$release_base/SHASUMS256.txt" | awk -v architecture="$architecture" '$2 == "node-v" substr($2, 7, index($2, "-linux-") - 7) "-linux-" architecture ".tar.xz" { print $2 }')"
46+
if [[ -z "$archive" ]]; then
47+
echo "Could not resolve the latest Node.js v${node_major} release" >&2
48+
return 1
49+
fi
50+
51+
local temporary_directory
52+
temporary_directory="$(mktemp -d)"
53+
(
54+
trap 'rm -rf "$temporary_directory"' EXIT
55+
echo "Installing Node.js v${node_major}"
56+
curl -fsSL "$release_base/$archive" -o "$temporary_directory/$archive"
57+
curl -fsSL "$release_base/SHASUMS256.txt" -o "$temporary_directory/SHASUMS256.txt"
58+
(
59+
cd "$temporary_directory"
60+
grep " $archive$" SHASUMS256.txt | sha256sum --check --status
61+
)
62+
rm -rf "$node_home"
63+
mkdir -p "$node_home"
64+
tar -xJf "$temporary_directory/$archive" --strip-components=1 -C "$node_home"
65+
)
66+
}
67+
68+
if [[ ! -x "$node_home/bin/node" ]] ||
69+
[[ "$($node_home/bin/node --version)" != v"$node_major".* ]]; then
70+
install_node
71+
fi
72+
73+
export PATH="$node_home/bin:$PATH"
74+
75+
pnpm_version="$(node -p "require('$repo_root/package.json').packageManager.split('@')[1]")"
76+
if [[ ! -x "$node_home/bin/pnpm" ]] ||
77+
[[ "$(pnpm --version 2>/dev/null || true)" != "$pnpm_version" ]]; then
78+
echo "Installing pnpm $pnpm_version"
79+
npm install --global --prefix "$node_home" "pnpm@$pnpm_version"
80+
fi
81+
82+
mkdir -p "$HOME/.local/bin"
83+
for executable in node npm npx pnpm pnpx; do
84+
ln -sfn "$node_home/bin/$executable" "$HOME/.local/bin/$executable"
85+
done
86+
87+
echo "Installing workspace dependencies"
88+
pnpm install --frozen-lockfile
89+
90+
echo "Installing Playwright Chromium and system dependencies"
91+
pnpm exec playwright install --with-deps chromium
92+
93+
echo "Orb setup complete"
94+
node --version
95+
pnpm --version
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
---
2+
name: consolidating-test-setup
3+
description: Consolidates repeated test setup and teardown into the narrowest shared lifecycle owner. Use when test files repeat initialization, cleanup, environment management, fixtures, or equivalent test doubles.
4+
---
5+
6+
# Consolidating test setup
7+
8+
Move repeated test plumbing to the narrowest shared owner that can provide it
9+
reliably. Keep test files focused on behavior.
10+
11+
## Principles
12+
13+
### Prove the code is equivalent
14+
15+
Repetition is evidence, not proof. Similar code may use different defaults,
16+
ordering, or lifetimes. Group candidates by behavior before treating them as
17+
duplicates.
18+
19+
### Centralize invariants, not scenarios
20+
21+
Shared setup should establish conditions that hold for every test in its scope.
22+
State that explains one scenario belongs with that scenario.
23+
24+
### Use the narrowest owner
25+
26+
Prefer the mechanism that already owns the lifecycle. Use runner configuration
27+
before hooks, scoped fixtures before global setup, and local setup for local
28+
requirements. Wider reuse is not better reuse.
29+
30+
### Treat setup and teardown as one contract
31+
32+
Define who creates state, how long it lives, and who releases it. Cleanup must
33+
finish even when a test fails. Tests must not depend on execution order or on
34+
another test's cleanup.
35+
36+
A shared default should be deterministic and easy to override. Local exceptions
37+
must not weaken isolation elsewhere.
38+
39+
### Subtract before abstracting
40+
41+
Remove suspected cargo-cult setup and run the affected tests. A targeted pass
42+
only marks it provisionally unnecessary. Delete it after the complete relevant
43+
suite and runner-specific lifecycle checks pass. If a test fails, use the
44+
failure to identify the contract before designing shared setup.
45+
46+
A helper must own a policy or lifecycle, or remove meaningful reader effort.
47+
Moving the same lines behind a new name is not an improvement.
48+
49+
### Preserve test intent
50+
51+
Do not hide state that matters to the behavior under test. Check history before
52+
removing synchronization or cleanup because it may guard a past regression.
53+
Preserve that guarantee even if the implementation changes.
54+
55+
## Runner-specific guidance
56+
57+
Load only the reference for the runner in use:
58+
59+
- [`reference/vitest.md`](reference/vitest.md) for Vitest and similar
60+
in-process unit-test runners
61+
- [`reference/playwright.md`](reference/playwright.md) for Playwright and
62+
similar browser-test runners
63+
64+
These references supplement the repository's testing guide. They do not replace
65+
it. Load both only when the task spans both runners.
66+
67+
## Workflow
68+
69+
### 1. Discover
70+
71+
Read the runner configuration, setup files, fixtures, helpers, and test guidance.
72+
Count repeated lifecycle behavior and note the suites that use it.
73+
74+
### 2. Classify
75+
76+
For each candidate, record:
77+
78+
- the state it owns
79+
- its lifetime and scope
80+
- whether it is an invariant or scenario detail
81+
- intentional differences between suites
82+
- behavior already supplied by the runner
83+
84+
### 3. Challenge
85+
86+
Remove the candidate and run every affected suite. Classify it as unnecessary,
87+
universal, common with exceptions, or suite-specific.
88+
89+
Subagents may evaluate disjoint groups of suites. Require evidence and exception
90+
reports from each. Keep the design and integration decision in the parent task.
91+
92+
### 4. Place
93+
94+
Choose the required lifetime and scope first. Then use the simplest owner at
95+
that exact boundary:
96+
97+
- native runner behavior that supports the required scope
98+
- existing shared setup, fixture, or hook
99+
- a focused helper or test double
100+
- the individual suite
101+
102+
Do not move state to a broader owner because its mechanism appears earlier in
103+
the list.
104+
105+
### 5. Migrate
106+
107+
Move one responsibility at a time. Add the shared owner, then remove only the
108+
local code it replaces. Keep intentional exceptions and scenario setup visible.
109+
Avoid unrelated test rewrites.
110+
111+
### 6. Prove
112+
113+
Run affected tests, tests of the shared lifecycle, repository static checks, and
114+
the complete relevant suite. Measure before and after when the new behavior runs
115+
for every test. Use enough comparable samples to separate a real change from
116+
normal variance.
117+
118+
## Report
119+
120+
State the repeated contract, its new owner, retained exceptions, rejected
121+
alternatives, duplication removed, and validation results. Include performance
122+
evidence when shared setup could affect suite runtime.
123+
124+
## Guardrails
125+
126+
- Do not replace native runner behavior with a custom abstraction.
127+
- Do not widen setup scope without evidence.
128+
- Do not add a helper merely to move code.
129+
- Do not hide global mutable state behind helper indirection.
130+
- Do not trade visible test intent for shorter files.
131+
- Do not trust targeted tests alone after changing shared lifecycle behavior.

0 commit comments

Comments
 (0)