Added-node error scan leaks when a node is removed and re-added: a stale finish() evicts the newer scan set

## Problem / Goal

Remove a node while its added-node error scan is still running, then add the **same node object**
back, and `installErrorClearingHooks` **permanently loses track of the second scan**. It can never be
cancelled and it is never released on dispose, so the graph keeps a "scan in progress" flag set
forever and `hasPendingAddedNodeErrorScan()` stays `true` for that execution ID for the lifetime of
the page.

The cause is that `finish()` is **not idempotent** and **does not check identity** before deleting its
map key. It runs twice for a cancelled scan — once synchronously from `cancel()`, once again when the
in-flight promise settles — and the second run deletes a `Map` entry that by then belongs to a
different, live scan.

Reproduced and fixed by execution at `origin/main` `296fc5cd07`, using the file's existing test
harness. Fails before, passes after, all 94 pre-existing tests in the surrounding suites still green.

## Provenance

* **Found by:** listener/lifecycle audit of `useErrorClearingHooks` · re-derived against `origin/main`
`296fc5cd07` on 2026-08-23
* **How:** read `scheduleAddedNodeScan` in full; built a probe on the existing
`useErrorClearingHooks.test.ts` deferred-verification pattern that adds a node, removes it, re-adds
the same object, settles the first scan's deferred verification, then disposes; asserted the store's
pending-scan flag. Applied the fix and re-ran.
* **Confidence:** verified by execution
* **Not a duplicate of #15697**, which is a different mechanism (`replaceWithMapping` dispatching
`node:added` without `node:before-removed`) and is scoped to `feature/ecs-migration`. This one
reproduces on `main` with no branch code involved.

## Evidence

All anchors opened at `origin/main` `296fc5cd07`, in
`src/composables/graph/useErrorClearingHooks.ts`.

**The non-idempotent, non-identity-checked release**, `:442-451`:

```ts
function finish() {
finishPendingScan() // :443
scansForNode.delete(control) // :444
if (scansForNode.size === 0) pendingScans.delete(node) // :445
}

function cancel() {
abortController.abort() // :449
finish() // :450
}
```

`scansForNode` is captured at `:437` and is **the Set instance that existed when this scan was
scheduled**. `:445` deletes `pendingScans[node]` whenever *that captured Set* is empty, without
checking whether the map still holds it.

**`finish` is reachable twice for one scan.** `:455-459`:

```ts
void runAddedNodeScan(rootGraph, node, abortController.signal)
.catch(...)
.finally(finish) // :459
```

`cancel()` calls `finish()` synchronously, and the promise's `.finally(finish)` calls it again when
the scan settles. `runAddedNodeScan`'s `finally` block (`:409`) awaits
`Promise.allSettled(pendingVerifications)`, so that second call can land arbitrarily late — after an
asset verification round trip.

**The re-add path creates a fresh Set under the same key**, `:436-440`:

```ts
const existingScans = pendingScans.get(node)
const scansForNode = existingScans ?? new Set()
if (!existingScans) {
pendingScans.set(node, scansForNode)
}
```

**The two consumers that then see nothing**, `:599` and `:634-636`:

```ts
for (const scan of pendingScans.get(node) ?? []) scan.cancel() // :599, onNodeRemoved
```

```ts
for (const scans of pendingScans.values()) { // :634, dispose
for (const scan of scans) scan.finish()
}
```

### The trace

1. `graph.add(node)` → scan A scheduled. `SetX = {A}`, `pendingScans[node] = SetX`. Store count 1.
2. `graph.remove(node)` → `:599` → `A.cancel()` → abort, `finish()` → `SetX` empty → `:445` deletes
`pendingScans[node]`. Store count 0. Correct so far.
3. `graph.add(node)` with the **same node object** → scan B scheduled. `SetY = {B}`,
`pendingScans[node] = SetY`. Store count 1.
4. Scan A's awaited verification settles → `.finally(finish)` fires A's `finish` a **second** time.
`SetX.delete(A)` is a no-op, `SetX.size === 0` is still true, and `:445` deletes
`pendingScans[node]` — **which now holds `SetY`.** B is orphaned.
5. Any later `graph.remove(node)` finds nothing at `:599`: **B's `AbortController` is never aborted**,
so its scan keeps running against a removed node. `dispose()` finds nothing at `:634`: B's
`finishPendingScan` is never called, so `hasPendingAddedNodeErrorScan(rootGraph, executionId)`
stays `true` forever.

Re-adding the same node object is not exotic: undo of a delete, drag in and out of a subgraph, and
node-replacement flows all move an existing `LGraphNode` instance between graphs. The map is keyed by
the node **object**, so the identity survives.

### Executed at `origin/main` `296fc5cd07`

`src/composables/graph/useErrorClearingHooks.test.ts` already has everything needed: `app.rootGraph`
is stubbed with `vi.spyOn(app, 'rootGraph', 'get')`, `missingModelScan.scanNodeModelCandidates` and
`missingMediaScan.scanNodeMediaCandidates` are spied, and
`missingModelScan.verifyAssetSupportedCandidates` is mocked with a deferred resolver — see the
existing `releases a pending added-node scan when hooks are disposed` test and its neighbours.

The probe used a resolver **array** so the two scans can be settled independently:

```ts
const resolvers: (() => void)[] = []
vi.spyOn(missingModelScan, 'verifyAssetSupportedCandidates').mockImplementation(
async () => { await new Promise((resolve) => resolvers.push(resolve)) }
)

const cleanup = installErrorClearingHooks(graph)
const store = useExecutionErrorStore()
const node = new LGraphNode('CheckpointLoaderSimple')
node.id = toNodeId(1)
const executionId = createNodeExecutionId([node.id])

graph.add(node) // scan A
await vi.waitFor(() => expect(resolvers.length).toBe(1))
expect(store.hasPendingAddedNodeErrorScan(graph, executionId)).toBe(true)

graph.remove(node) // A cancelled, key deleted
expect(store.hasPendingAddedNodeErrorScan(graph, executionId)).toBe(false)

graph.add(node) // scan B, fresh Set, same key
await vi.waitFor(() => expect(resolvers.length).toBe(2))
expect(store.hasPendingAddedNodeErrorScan(graph, executionId)).toBe(true)

resolvers[0]() // A's stale finish() runs
await new Promise((r) => setTimeout(r, 0))

cleanup() // should release B
expect(store.hasPendingAddedNodeErrorScan(graph, executionId)).toBe(false)
```

Before the fix — every intermediate assertion passes, only the last one fails, which is what makes
this a targeted detector rather than a broken harness:

```
AssertionError: expected true to be false // Object.is equality
❯ src/composables/graph/__probe.test.ts:71:68
Test Files 1 failed (1)
Tests 1 failed (1)
```

The fix, replacing `:442-446`:

```ts
let finished = false
function finish() {
if (finished) return
finished = true
finishPendingScan()
scansForNode.delete(control)
if (scansForNode.size === 0 && pendingScans.get(node) === scansForNode)
pendingScans.delete(node)
}
```

After the fix:

| Run | Result |
| --- | ------ |
| probe + `useErrorClearingHooks.test.ts` + `useErrorClearingHooks.promotion.test.ts` + `executionErrorStore.test.ts` | `Test Files 4 passed (4)` · `Tests 94 passed (94)` |

Note that `beginAddedNodeErrorScan`'s own returned callback is **already** idempotent
(`src/stores/executionErrorStore.ts:78-87` sets a `finished` flag), so the store's count is not
double-decremented today. That is why this shows up as a *stuck* pending scan rather than as a
negative count, and it is why the guard belongs in `useErrorClearingHooks` as well.

## Proposed Solution

Make `finish()` idempotent and identity-checked, as above. Two independent bugs, one edit:

* `if (finished) return` stops the second invocation from touching the map at all.
* `pendingScans.get(node) === scansForNode` stops any future `finish` from deleting a key it no
longer owns.

Keep both. The identity check alone would still let a stale `finish` call `scansForNode.delete` and
re-run `finishPendingScan`; the idempotence check alone would not protect the case where two scans
for the same node interleave in a different order.

## Acceptance Criteria

- [ ] `finish()` in `scheduleAddedNodeScan` cannot run twice, and cannot delete a `pendingScans`
entry it does not own
- [ ] A regression test in `src/composables/graph/useErrorClearingHooks.test.ts` following the probe
above: add, remove, re-add the same node object, settle the first scan's deferred verification,
dispose, and assert `hasPendingAddedNodeErrorScan` is `false`
- [ ] A second assertion that the re-added node's scan **is** cancelled by a later
`graph.remove(node)` — that is the other half of the bug and the dispose assertion alone does
not cover it
- [ ] Mutation-verified: revert the fix, confirm the new test(s) go red, restore, confirm green.
Report the before/after test counts in the PR
- [ ] `vitest run src/composables/graph src/stores/executionErrorStore.test.ts` is green, and
`NODE_OPTIONS=--max-old-space-size=8192 vue-tsc --noEmit` exits 0

## Out of scope

* **`useErrorClearingHooks`'s size.** #11215 proposes splitting this file. Do not refactor it here.
* **#15697** and anything on `feature/ecs-migration`. This issue is about `main` only.
* **The promotion-error path** (`promotionErrors.*`) and the missing-media/missing-model stores. Not
implicated.
* **Changing `beginAddedNodeErrorScan`.** Its idempotence is correct and load-bearing; leave it.

## Working notes for whoever picks this up

Traps in this repo that will otherwise cost you a cycle:

* **This is a pnpm workspace with eight `node_modules` directories** — the root plus
`apps/desktop-ui`, `apps/website`, and
`packages/{design-system,object-info-parser,tailwind-utils,shared-frontend-utils,ingest-types}`.
With only the root linked, `vitest` dies at `Failed to resolve import "clsx"` **at every commit**,
which makes any comparison meaningless.
* **Run a positive control before trusting any red result.**
`vitest run src/lib/litegraph/src/LLink.test.ts` should report `Tests 3 passed (3)`. Note the path:
it is `src/lib/litegraph/src/`, not `src/lib/litegraph/test/` — the latter does not exist and
vitest exits **0** with `No test files found`, which reads as a pass.
* **Vitest 4 has no `basic` reporter.** `--reporter=basic` exits 1 with
`Failed to load custom Reporter from basic` before running anything. Omit the flag.
* **There is no root `vitest.config.ts`.** The config is `vite.config.mts`, read by default.
* **`vue-tsc --noEmit` OOMs at Node's 2GB default heap and exits 134**, and a grep filtered for
`error TS` then returns zero — byte-identical to a clean run. Always pass
`NODE_OPTIONS=--max-old-space-size=8192` and always report the exit code.
* **Ordering matters in the probe.** Settle scan A's verification only *after* scan B has been
scheduled; if you resolve too early the stale `finish` lands before B exists and the bug does not
reproduce. `vi.waitFor(() => expect(resolvers.length).toBe(n))` is the reliable gate.
* **`oxlint`'s exit code is ambiguous in both directions.** Most rules here run at warning level, so
exit 0 does not mean no findings; it also exits 1 when every input path is ignored
(`No files found to lint`). Read the output, not just `$?`.