spec(1187): two fixes built and rejected — why the tag cannot be trusted (and the leg that argument stood on, corrected)

[artokun]

Refs #1187. Ships no behaviour change — the two attempted fixes are reverted and the runtime is back to main's behaviour. What it now ships is the record: docs/design/graph-binding-tag-vs-tracker.md, plus a pointer comment at the expression a third attempt would edit.

Two fixes were built, reviewed and rejected. This records why, so the third attempt does not repeat them.

The bug is real

web/js/lib/graph-binding.js, resolveGraphBindingVerdict (currently 2183-2185):

const rootShapeMismatch =
  contentDiffers &&
  !(structureMatches && graphRootWorkflowUuidMatches({ rootGraph, activeWorkflowUuid }));

The positive tag is consulted only as a conjunct of structureMatches. A hand edit differs structurally by definition, so structureMatches is false, the term collapses, and the workflow's own identity stamp can never rescue the read. Meanwhile ComfyUI's ChangeTracker captures on user-input events, so there is a window where activeState reports the old node count, _nodes reports the new one, and isModified has not flipped — so the dirty-tab escape hatch does not fire either. Every read and every mutation refuses. It self-clears once the tracker captures, which is why it presents as intermittent.

Attempt 1 — let the tag outrank the content comparison. Rejected, P0.

contentDiffers && !tagMatches — i.e. delete the structureMatches conjunct.

graph-binding.js:1117 says of that identity+structure pair that "the relaxation is deliberately NARROW, and both conjuncts are load-bearing", and the STRUCTURE clause (1128-1133) states what its half buys: a different node set, ids, types, links, groups, reroutes, subgraphs, definitions or content-bearing extra "all remain refusals, so #349 is untouched". Deleting it removes the half that keeps #349 closed.

Every stale-tag mitigation the file already ships — staleTagOnEmptyCanvas, contentProvesActiveWorkflow — demands content proof. None trusts the tag by itself. The attempt inverted the file's own rule.

Also surfaced: the containment claimed for it was caller-dependent. graphCommandBindingBar sets includeBaselineReadGuard: false for every classified read (graph-binding.js:2091), which disables both midPopulation and baselineReadDesync, so reads like graph_serialize had no backstop at all.

Correction on re-verification (2026-08-14) — read before re-deriving the P0

The original rejection cited graph-binding.js:1757-1777 (#565/#817: some builds do not reset graph.extra in configure(), so a reused app.graph carries the previous workflow's tag). That citation stops fifteen lines too early. Lines 1779-1794 of the same comment record a measurement taken 2026-08-09 on ComfyUI frontend 1.48.7:

configure(payload with extra:{})      -> tag gone, nodes 10 -> 1
configure(payload with NO extra key)  -> tag gone, nodes  1 -> 2
clear()                               -> tag gone, nodes    -> 0

On that build the tag does not survive a content change, and the file concludes the #565/#817 premise "is therefore frontend-specific, and the clauses may simply not be reachable on current builds". #1187's reporter is on frontend 1.48.6, the same generation.

So the configure() mechanism alone does not carry the P0 on current frontends, and a reader who followed the old citation would have found what looks like a refutation. The rejection still stands, on three legs that do not depend on it:

1. The measurement is one frontend and says so: "Verified on one frontend, which is exactly why neither statement should be universal." It licenses no universal trust in the tag in either direction.
2. Binding guard throws a false "live graph is out of sync" on every NEW blank workflow (stale root extra.comfyui_mcp.workflow_uuid) #565 and panel_graph_outline rejects active workflow after tab switch with root-workflow-uuid-mismatch #817 are real reports from builds where the tag did survive, and panel_graph_outline rejects active workflow after tab switch with root-workflow-uuid-mismatch #817 is still open. The asymmetry matters: the existing stale-tag clauses are safe to leave in place when unreachable because they admit — an unreachable widening costs nothing. A fix that relies on the tag being fresh does the opposite: it fails open on exactly the builds that reported the problem.
3. Build-independent: graph-binding.js:1145-1161 records as a known, deliberate gap that sealProvenRootBinding will stamp A's tag onto an untagged root serializing equal to A with no other open workflow matching it — which a closed duplicate's stranded canvas satisfies, since the exclusivity sweep can only see open tabs. The structure conjunct is what bounds that gap to "still structurally A". Attempt 1 deletes exactly that bound.

Leg 3 involves no frontend behaviour at all and is the one to reason from.

Attempt 2 — settle the lagging tracker, then re-ask. Rejected, same P0, and worse.

On root-shape-mismatch + a matching tag + a clean tab: call captureCanvasIntoTracker (web/js/comfyui-mcp-panel.js:2343), then re-run the same resolver, unchanged.

It rested on one sentence never verified — "after the capture the content still has to agree." It does not. The capture flips isModified, and isModified === true makes graphRootMismatchesActiveWorkflow return false immediately (graph-binding.js:1017). The comparison is not re-run, it is suppressed. The capture does not validate the canvas; it silences the check that would have caught it. Same P0 by a longer route.

Two costs attempt 1 did not have:

• it fires on merely reading a wrongly-mounted canvas, not only after a real edit;
• a capture that sees a change "pushes an undo entry and clears the redo queue" (comfyui-mcp-panel.js:2397-2398), recording canvas B as workflow A's change. A user could lose a redo by reading.

And the test written to prove it safe was vacuous. It forced rootUuidMismatch: true — the case where the tag is provably foreign. The dangerous case is a stale tag that still matches, where that flag is false. It asserted the safe case and labelled it the dangerous one.

Why this is hard

Three facts that cannot be combined into a proof using the tracker alone:

1. The tag can be stale → identity is not proof.
2. The tracker lags → content is not currently accurate.
3. Making the tracker current flips isModified → which disables the content check outright.

The root reason: the ChangeTracker follows whatever canvas is mounted. It cannot distinguish "the user edited A" from "this is B wearing A's tag", because in both cases it faithfully reports the canvas in front of it.

What a third attempt probably needs

Evidence the tracker does not carry — the workflow's own persisted/serialized content, which is what rootContentProvesActiveWorkflow and contentProofExclusiveAmongOpen already reason about. Compare the live root against workflow A's own stored state, not against a snapshot of whatever is mounted.

That likely depends on #1014 (a saved workflow loses its identity across a reconnect — still open) landing first, since it is the case where no published UUID exists to anchor any of this.

Verification performed

Both attempts were carried to completion before being rejected — full unit suite green, and mutation-verified (6 mutations on attempt 2, all confirmed red) — which is precisely why they are worth recording. A green suite and a clean mutation run did not make either one safe. Attempt 2's suite passed 143/143 while the P0 path was untested, and the shared fence harness does not inject the settle path's dependencies, so those tests silently exercised a degraded no-op.

Re-verified on this branch after merging main (2026-08-14): every file, function and line reference above re-checked against the merged tree; npm run test:unit green (4399 tests, 0 fail).

🤖 Generated with Claude Code

[Copilot]

Copilot wasn't able to review any files in this pull request.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.