From b7af403cc076cdfc50596d5b91c667bc736ee253 Mon Sep 17 00:00:00 2001 From: Nathaniel Parson Koroso Date: Wed, 8 Jul 2026 19:15:58 -0700 Subject: [PATCH] docs(detection-proof): sync rows 8-10 to CI-step delivery (not forks) and all-live commit design --- .../tests/customNodes/DETECTION_PROOF.md | 75 ++++++++++--------- 1 file changed, 41 insertions(+), 34 deletions(-) diff --git a/browser_tests/tests/customNodes/DETECTION_PROOF.md b/browser_tests/tests/customNodes/DETECTION_PROOF.md index 0580dac739..2d06bab8a3 100644 --- a/browser_tests/tests/customNodes/DETECTION_PROOF.md +++ b/browser_tests/tests/customNodes/DETECTION_PROOF.md @@ -44,14 +44,16 @@ The gate protects against two distinct things, and the proof covers both: is the primary thing the gate guards on every frontend PR. These breaks live in `src/`. - **Pack-bug** - a pack itself ships a bug (or a pinned pack is bumped to a - broken version). The gate catches these too. On CI these breaks are - delivered the only way a frontend-repo commit can deliver them: through the - manifest (`browser_tests/fixtures/data/customNodeManifest.json`), by - pointing the pack's `repo`/`pin` at a fork commit that carries the bug - - which is exactly the pinned-bump scenario the mode describes. CI clones - every pack fresh at its pin, so editing pack files in the frontend repo - does nothing there; direct pack-file edits only work against a local - backend (which is how the exact reds below were captured). + broken version). The gate catches these too. CI clones every pack fresh at + its pin, so editing pack files in the frontend repo does nothing - the clone + overwrites them. Two ways deliver a pack break on CI: (a) point the manifest + (`browser_tests/fixtures/data/customNodeManifest.json`) `repo`/`pin` at a + broken fork, which is exactly the pinned-bump scenario and the most + production-faithful; or (b) a self-contained CI step that patches each cloned + pack in place right after install. The proof PR uses (b) - no external repos, + and each patch asserts it landed (`grep`, fails the job otherwise) so a silent + no-op cannot fake a pass. Both reproduce the same edits captured against a + local backend (which is how the exact reds below were captured). Each row below is labelled with its mode. @@ -65,18 +67,18 @@ change WHICH pair or node the message names without weakening the catch - the promise is the tier and the failure class, not byte-identical offender text across pin changes. Sections refer to [ARCHITECTURE.md](ARCHITECTURE.md). -| # | Surface (ARCH section) | Mode | Real regression it recreates | The one-file break | CI check that catches it | Exact red | -| --- | ------------------------------------------------ | ---- | ------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------ | ----------------------------------------------------------------------------------------------------------- | -| 1 | Mount completeness, canvas / v1 (s1, s5) | FE | A change dropping declared parts on the canvas renderer (class; no single ticket - the v2 wave below shows how this family presents) | `src/services/litegraphService.ts` `addInputs`: stop materializing the last declared input | Tests Custom Nodes / mount tier | `BatchCount+: instance is missing declared input "batch" (litegraph)` | -| 2 | Mount completeness, DOM / v2 (s1, s5) | FE | Widgets missing under Nodes 2.0 (FE-627/FE-634 iTools buttons; FE-841 is the adjacent wrong-style class, present but unproven caught) | `src/renderer/extensions/vueNodes/widgets/registry/widgetRegistry.ts`: drop the `int` widget component mapping | Tests Custom Nodes / mount tier (Vue pass) | `Ideogram4PromptBuilderKJ: Vue mounts 9 of 15 widgets` | -| 3 | Persistence, save/reload (s1, s8) | FE | Widgets reverting to socket-only on reload: the defaultInput migration regression that PR #12279 (open) exists to fix | `src/lib/litegraph/src/LGraphNode.ts` `configure`: off-by-one drops the last `widgets_values` entry | Tests Custom Nodes / persistence tier | `Seed (rgthree): widgets_values [1,"fixed"] -> [1,"randomize"] on set-values reload` | -| 4 | Wiring - type compatibility (s5, s6) | FE | A frontend change narrowing connectable types (class; no single verified ticket) | `src/lib/litegraph/src/LiteGraphGlobal.ts` `isValidConnection`: reject IMAGE links | Tests Custom Nodes / connectivity sweep | `AddLabel.IMAGE -> FastPreviewBatch.input: CONNECT_REJECTED` (full pair list) | -| 5 | Wiring - drop resolution (s5) | FE | Drag/slot resolution family (nearest reported symptoms: FE-625/FE-632 EditUtils connections shift after drag) | `src/lib/litegraph/src/canvas/measureSlots.ts` `getNodeInputOnPos`: return undefined | Tests Custom Nodes / connectivity drag | `EmptyImage.IMAGE -> ImageBatch.image2 with VueNodes=false` | -| 6 | Execution - frontend prompt serialization (s7) | FE | A prompt-serialization change corrupting inputs (class; no single verified ticket) | `src/utils/executionUtil.ts`: drop numeric widget values from the API prompt | Tests Custom Nodes / curated run (T1) | `Prompt outputs failed validation; ImpactInt: value; ImpactFloat: value` | -| 7 | Zero-visible-errors / load hook (s1) | FE | An extension hook crashing on graph load, the mechanism packs hook (FE-751 class; the break is in a core extension, hence FE mode) | `src/composables/node/useNodeBadge.ts` `afterConfigureGraph`: throw | Tests Custom Nodes / curated run (T1) | `Error calling extension 'Comfy.NodeBadge' method 'afterConfigureGraph' ...` | -| 8 | Console / pageerror ledger (s10) | Pack | An uncaught pack-JS error during save/reload (the betterCombos.js `typeof null` bug this suite found) | manifest pin swap: point ComfyUI-Custom-Scripts at a fork commit whose `showText.js` logs a `console.error` in `onConfigure` (captured locally by editing the installed pack directly) | Tests Custom Nodes / curated run (T1) | `console errors during curated run` + the exact text + script URL | -| 9 | Execution - runtime (s7) | Pack | A pack node raising at execution (WAS Text Find/Replace infinite loop; KJ ImageGridtoBatch min violation) | manifest pin swap: point was-node-suite at a fork commit whose `return_constant_number` raises (captured locally by editing the installed pack directly) | Tests Custom Nodes / auto-run tier | `Constant Number: EXECUTION_ERROR (Constant Number: ValueError) - not in cannotRunAlone; a regression, ...` | -| 10 | Registration / expectedNodes sentinels (s5, s10) | Pack | A pinned pack bump renaming a node key | manifest pin swap: point ComfyUI-Impact-Pack at a fork commit that renames the `ImpactInt` mapping key (captured locally by editing the installed pack directly) | Tests Custom Nodes / zero-skip gate | job goes red on `skipped != 0` (T0 + T1 skip; the workflow's "Forbid skipped tests" step fails) | +| # | Surface (ARCH section) | Mode | Real regression it recreates | The one-file break | CI check that catches it | Exact red | +| --- | ------------------------------------------------ | ---- | ------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------ | ----------------------------------------------------------------------------------------------------------- | +| 1 | Mount completeness, canvas / v1 (s1, s5) | FE | A change dropping declared parts on the canvas renderer (class; no single ticket - the v2 wave below shows how this family presents) | `src/services/litegraphService.ts` `addInputs`: stop materializing the last declared input | Tests Custom Nodes / mount tier | `BatchCount+: instance is missing declared input "batch" (litegraph)` | +| 2 | Mount completeness, DOM / v2 (s1, s5) | FE | Widgets missing under Nodes 2.0 (FE-627/FE-634 iTools buttons; FE-841 is the adjacent wrong-style class, present but unproven caught) | `src/renderer/extensions/vueNodes/widgets/registry/widgetRegistry.ts`: drop the `int` widget component mapping | Tests Custom Nodes / mount tier (Vue pass) | `Ideogram4PromptBuilderKJ: Vue mounts 9 of 15 widgets` | +| 3 | Persistence, save/reload (s1, s8) | FE | Widgets reverting to socket-only on reload: the defaultInput migration regression that PR #12279 (open) exists to fix | `src/lib/litegraph/src/LGraphNode.ts` `configure`: off-by-one drops the last `widgets_values` entry | Tests Custom Nodes / persistence tier | `Seed (rgthree): widgets_values [1,"fixed"] -> [1,"randomize"] on set-values reload` | +| 4 | Wiring - type compatibility (s5, s6) | FE | A frontend change narrowing connectable types (class; no single verified ticket) | `src/lib/litegraph/src/LiteGraphGlobal.ts` `isValidConnection`: reject IMAGE links | Tests Custom Nodes / connectivity sweep | `AddLabel.IMAGE -> FastPreviewBatch.input: CONNECT_REJECTED` (full pair list) | +| 5 | Wiring - drop resolution (s5) | FE | Drag/slot resolution family (nearest reported symptoms: FE-625/FE-632 EditUtils connections shift after drag) | `src/lib/litegraph/src/canvas/measureSlots.ts` `getNodeInputOnPos`: return undefined | Tests Custom Nodes / connectivity drag | `EmptyImage.IMAGE -> ImageBatch.image2 with VueNodes=false` | +| 6 | Execution - frontend prompt serialization (s7) | FE | A prompt-serialization change corrupting inputs (class; no single verified ticket) | `src/utils/executionUtil.ts`: drop numeric widget values from the API prompt | Tests Custom Nodes / curated run (T1) | `Prompt outputs failed validation; ImpactInt: value; ImpactFloat: value` | +| 7 | Zero-visible-errors / load hook (s1) | FE | An extension hook crashing on graph load, the mechanism packs hook (FE-751 class; the break is in a core extension, hence FE mode) | `src/composables/node/useNodeBadge.ts` `afterConfigureGraph`: throw | Tests Custom Nodes / curated run (T1) | `Error calling extension 'Comfy.NodeBadge' method 'afterConfigureGraph' ...` | +| 8 | Console / pageerror ledger (s10) | Pack | An uncaught pack-JS error during save/reload (the betterCombos.js `typeof null` bug this suite found) | CI step patches the cloned ComfyUI-Custom-Scripts `showText.js` to log a `console.error` in `onConfigure` (captured locally by editing the installed pack directly) | Tests Custom Nodes / curated run (T1) | `console errors during curated run` + the exact text + script URL | +| 9 | Execution - runtime (s7) | Pack | A pack node raising at execution (WAS Text Find/Replace infinite loop; KJ ImageGridtoBatch min violation) | CI step patches the cloned was-node-suite `return_constant_number` to raise on entry (captured locally by editing the installed pack directly) | Tests Custom Nodes / auto-run tier | `Constant Number: EXECUTION_ERROR (Constant Number: ValueError) - not in cannotRunAlone; a regression, ...` | +| 10 | Registration / expectedNodes sentinels (s5, s10) | Pack | A pinned pack bump renaming a node key | CI step patches the cloned ComfyUI-Impact-Pack `__init__.py` to rename the `ImpactInt` mapping key (captured locally by editing the installed pack directly) | Tests Custom Nodes / zero-skip gate | job goes red on `skipped != 0` (T0 + T1 skip; the workflow's "Forbid skipped tests" step fails) | ### Links of various types (surface 4/5 expanded) @@ -155,23 +157,28 @@ against an environment that changed under it, not a suite defect. Therefore: ## Building the proof PR 1. Branch off the suite branch: `git checkout -b nathaniel/detection-proof nathaniel/custom-node-e2e-suite`. -2. For the three Pack-mode rows, first publish the broken fork commits (one - fork per pack, each a one-line change matching its row); the demo commit in - this repo is then the manifest `repo`/`pin` edit pointing at that fork. The - FE-mode rows need no forks - their break is a direct `src/` edit. -3. One commit per matrix row, each a single-file break. FE-mode rows carry an - inline comment in the changed `src/` file: - `// DETECTION PROOF: recreates - . Expected: custom-nodes check red at .` - Pack-mode rows change only the JSON manifest, which cannot carry a comment - (a `//` line would break both the CI jq install loop and the loader's - JSON.parse), so their provenance lives in the commit message per step 4. -4. Commit message names the surface and the reference, e.g. - `demo(detection): break Vue widget mount (recreates FE-627/FE-634) - expect mount tier red`. +2. One commit per matrix row, each breaking one surface, stacked so all breaks + are live at HEAD at once (not reverted between commits - the goal is to see + every surface broken together, and the `Tests Custom Nodes` job reds across + every tier in one run). FE-mode rows (1-7) are a direct `src/` edit carrying + an inline comment in the changed file: + `// DETECTION PROOF (row N, surface): recreates . Expected: red .` +3. Pack-mode rows (8-10) are delivered by one CI step + (`DETECTION PROOF - break packs`, on this branch only) that patches each + cloned pack in place right after install. Each patch asserts it landed + (`grep`, fails the job otherwise) so a silent no-op cannot fake a pass. The + step is fenced to this never-merge branch and must never be ported to a real + suite branch. +4. Commit message names the surface, e.g. + `detection-proof: break mount (v2 Vue renderer) - drops the int widget mapping`. 5. Open the PR against the suite branch (not main) with the correlation matrix as the description and a bold header: **This PR must never merge. Every commit is a deliberate break; green would mean the gate missed a regression.** -6. Let CI run. Each commit turns its named check red at the named tier. The - reviewer bisects commit-by-commit to correlate break -> catch. +6. Let CI run on HEAD. With every break live, the `Tests Custom Nodes` job reds + across every tier in one run. Attribute a red to its cause via the labelled + comment on the matching `src/` file (rows 1-7) or in the CI break step + (rows 8-10); checking out commit N (which contains breaks 1..N) narrows it + further. ## References