fix(1126): a combo write is decided by whether the panel could READ the option list

[artokun]

Refs #1126. Related, same area: #507, #240, #347, #477. Cross-feature interaction with #1223 (v0.14.39) and #716.

This does NOT close #1126. It closes a real, adjacent defect that the investigation of #1126 uncovered. The reported symptom survives — measured, not assumed. See "What this does not fix" below.

The defect this fixes

A dynamic combo's option list comes from options.values(widget) — the node's own callback. It can mutate the widget and it can fail. When it failed, the panel had compared the value to nothing, yet answered with a refusal that reached the user as a verdict about their value:

Value "F:\Downloads\Scarlet1.0.fbx" is not a valid option for combo widget "fbx_file"

Nothing was ever compared. A node whose runtime handler takes an absolute path had the path refused as though the path were wrong.

The two outcomes are now answered differently, and both from what the panel observed:

• The list WAS read and the value is not in it — still refused, unchanged. A typo'd model name is still caught before a run fails deep in model loading (Combo widget set by panel_set_widget silently changes to another enum after run #240/panel_set_widget can never write a dynamic combo whose server-side option list is empty (StarNodes Ollama "model") #507 intact).
• The list could NOT be read at all (callback threw / returned a non-list / no callback) — the valid set is not knowable here, exactly like panel_set_widget can never write a dynamic combo whose server-side option list is empty (StarNodes Ollama "model") #507's empty list, and Combo widget set by panel_set_widget silently changes to another enum after run #240's reason for strict membership (a number silently reinterpreted as an index into a real list) has no list to index into. Accepted as a last resort, after the authoritative /object_info refresh and the upload probe, and only when a LIVE /object_info declares this input's list empty too — a failed live read is not licence to ignore a list the server publishes.

Every message now says which of the two happened, on the refusal and on the success, and the success carries the observed reason rather than a plausible default (there are three distinct reasons; picking one would be wrong two-thirds of the time).

Guards deliberately kept

• Non-empty string only. Combo widget set by panel_set_widget silently changes to another enum after run #240 — a real option list exists on the widget, we simply cannot read it, so a NUMBER could still be reinterpreted as an index into it. panel_restart_comfyui hangs until 300 second tool timeout #347 — "" clears the widget, and an unreadable list must not become a new door to that.
• Its own marker (unreadableAcceptanceUsed), not emptyAcceptanceUsed. It arms the panel_set_widget can never write a dynamic combo whose server-side option list is empty (StarNodes Ollama "model") #507 promoted-write rail cross-check for the same reason panel_set_widget can never write a dynamic combo whose server-side option list is empty (StarNodes Ollama "model") #507 does — a promoted write also mutates the parent's serializing rail — but it can never adopt a rail's numeric label. Adoption is justified by "the inner list admitted any scalar", true of an empty list and false of one that exists and could not be read; adopting there would write a rail's NUMBER onto a widget whose real, unread list may not hold it, and would silently replace the value the caller sent. Unreadable ⇒ verify or refuse, never substitute.
• The cross-check's refusals no longer describe an unread list as empty.
• The panel_set_widget refused … frame is kept on the last-resort attempt's own refusal; anything that is not a validation refusal propagates unchanged.
• The disclosure reaches the user-visible reply: the activity summary renders the unvalidated write with a warning icon and a scoped NOT-validated line, read as DATA off the result rather than matched out of translated prose.

No new tool argument, so nothing here depends on an orchestrator schema change and no capability needs advertising — the panel decides from what it can see.

Review round 2 — live evidence, nested chains, honest disclosure

1. A SNAPSHOT-sourced empty list no longer authorizes a blind write (#1223 × #1126)

A cross-feature hole opened by two changes that are each correct alone. #1223 (v0.14.39) lets getFreshObjectInfo answer from objectInfoSnapshot.authorize(…).defs — the last-observed schema — when both live /object_info probes go silent. This fallback's second condition read that answer as "the SERVER declares this input's option list empty", but a snapshot is what the server said before it went quiet, retained across exactly the window in which nobody can re-fetch. Options change without a reconnect (a model downloaded while the node's own callback keeps failing), so a stale [] would newly authorize an unvalidated write against a list that is no longer empty.

The panel now threads the provenance it already knows rather than letting the lib infer it, as a function read at the moment of decision — which branch answered is only known after the oracle runs, and the lib may re-ask. UNKNOWN provenance — a probe that throws — fails closed too: nothing established must not read as live. The refusal names which of the two facts is missing, because "refresh and retry" is only actionable if the caller knows it is the schema, not their value, that could not be established.

Direct and single-hop promoted writes on a live schema are unchanged. (Rounds 3 and 4 below widen this from a boolean to a five-state provenance — a snapshot is not the only stale source — and establish that a snapshot cannot answer this question at all.)

2. A NESTED promotion is refused instead of blind-written

The rejection this fallback answers describes the immediate promoted projection. On a nested chain (outer → intermediate subgraph → concrete node) the value is ultimately driven into a deeper concrete widget this path never read, whose own client-populated list may be perfectly readable — so the fallback's premise, "the valid set is not knowable from here", is not established, and an off-list value would bypass a live list that was available all along.

Refusing is chosen over validating the concrete widget, deliberately. Validating it would mean running membership at another level on a last-resort path, against a dynamic source carrying the same never-read-twice and stateful-callback hazards the sibling cross-check already documents — new blind-write surface in the one place least able to carry it. A refusal is recoverable and names the shape (it points at panel_enter_subgraph and at the node's own option callback); a wrong blind write into a nested chain lands on a serializing rail. Direct and single-hop promoted writes, where the widget that was read is the one the value drives, are unaffected.

3. The disclosure no longer over-claims

For a promoted write whose parent rail has a readable non-empty list, the sibling cross-check does compare the value and proceeds only on membership — yet the reply note and the activity summary both said nothing checked it. That is false in exactly the case where the most checking happened, and on a change whose whole value is telling the truth about what was and was not validated, an over-claimed disclosure is worse than a missing one.

applyWidgetWrite now emits promoted_rail_validated when the rail's real list vouched for the value; the note and the summary scope their claim to this widget's own list and say so. Emitted only when true, so a reader that does not know the field sees exactly what it saw before. Both are proven by driving the real code — the note through the real runSetWidget ladder on a real single-hop promotion, the summary through the real extracted describeCommand — not by scanning source.

Review round 3 — cache provenance, an empty final read, proxy-vs-rail

4. A CACHE-HIT empty list does not authorize a blind write either (#716 × #1126)

The provenance gate above closed the snapshot hole but not the ordinary cache one, one layer down. #716's burst cache answers /object_info for up to 1.5s, so writes 2..N of a burst never reach the server at all — schemaFromSnapshot() correctly returned false, and the branch read a cached [] as current. "Not from a snapshot" is not the same as "live." If the backend's options change externally inside that TTL, the same off-list write lands.

The threaded fact is therefore no longer a boolean but a provenance — "live" / "cache" / "snapshot" / "none" — keyed on the same observedAtEpoch that already gates #1223's objectInfoSnapshot.record: captured inside the cache loader, which runs only on a miss, so a cache hit or a joined in-flight read leaves it null. The fact was already there to be reported; nothing new had to be invented.

And the fallback re-asks rather than merely refusing. Failing closed on every cache hit would refuse writes 2..N of an ordinary burst — precisely the multi-widget case this PR exists to serve — so it drops the burst cache (invalidateObjectInfoCache) and re-calls the same oracle once, which is a cheap trade on a last-resort path. Then:

• live answer still declares the list empty ⇒ the write proceeds, as before;
• live answer publishes a real list ⇒ refused — this is the hole the re-ask exists to find, and good provenance must not become licence to write past the very answer just fetched to check it;
• no live answer at all ⇒ refused, naming the layer that withheld it (snapshot ⇒ reconnect; cache ⇒ retry in a moment; unknown ⇒ nothing was established).

Nested promotions are now decided before the re-ask: the shape is already known, the answer cannot change, and re-fetching a whole /object_info for a write about to be refused anyway is pure cost.

5. An empty FINAL read is #507's valid transition, not a refusal

options.values is a callback and answers per call. If it threw on the initial read and the post-refresh retry but returned [] on the final one, only acceptUnreadableComboOptions was enabled — so coercion fell through to #507's empty-list branch, raised a retryable emptyOptions, and refused with "the server's option list may simply be stale — refreshing it before deciding" at the end of a ladder that had already refreshed it. By that line the live server schema has already confirmed the list empty for this input, which is #507's own precondition, so this was a valid transition being rejected with a message that contradicted the ladder it sat at the end of.

The final write now enables both acceptances. Which one admitted the value is still decided at coercion time and reported from there, so the disclosure names what happened rather than what the call hoped for: an empty final read reports empty_option_list (and correctly carries #507's rail label-adoption rule, because an empty list really does admit any scalar), an unreadable one reports option_list_unreadable. applyWidgetWrite now names the empty acceptance on its own result, so one outcome has one name from either ladder branch.

6. Only the AUTHORITATIVE rail counts as validation

#477: a single host input can reference two identity-authenticated widgets — the serializing rail (_widget) and a parent-facing display proxy (input.widget), a read-only mirror. The cross-check walks both, and crediting any match meant a dual-projection promotion whose rail list is empty but whose proxy list holds the value still emitted promoted_rail_validated — the reply and the activity summary both claiming the serializing rail vouched for a value it never listed.

Now only a match on parentWidget sets it. A proxy match stays silent and the write is reported as fully unvalidated, which is what it is with respect to what actually gets queued. The write itself is unchanged — both projections are still synced — only the claim is narrowed. On a change whose whole value is telling the truth about what was and was not validated, an overstated disclosure is worse than a missing one.

Review round 4 — a reconnect-spanning schema, and a branch that could never run

7. A response the backend reconnected underneath is not "live" (P1)

object-info-cache intentionally still hands a retired response to its original waiter, and objectInfoSnapshot.record refuses to file it because observed !== current. The provenance above labelled that same response "live" purely because the loader had run. If the old process declared an empty option list and the replacement publishes a real one, the fallback blind-writes an off-list value — and a restart is precisely the event that changes what the server publishes.

The provenance now applies the same test record already applies (observedAtEpoch === backendReconnectEpoch) rather than inventing a second rule, and reports "reconnected" with a refusal that names the replaced process. The two can no longer disagree about whether an answer is usable as backend evidence.

8. The snapshot-specific path was UNREACHABLE — dead code, and the test hid it

objectInfoSnapshot.authorize returns a detached map of type names: every value is the shared frozen EMPTY_DEF, with no input at all. That detachment is #1223's defence against beforeRegisterNodeDef mutating defs in place (and it keeps ~5.4MB off each connection) — the snapshot module's own comments name serverDeclaresEmptyComboOptions as one of the helpers that returns false on it.

So the shape test is always false against a snapshot, for every input on every node. readSchemaProvenance() never got to observe "snapshot": the re-ask and its diagnostic were dead code. The round-3 test passed only because it hand-built a full starObjectInfo([]) instead of the real shape — the third time on this branch that a test chose an input shape production never sends.

Fixed on both sides:

• The fixture is now the real thing. It is built by driving record + authorize, never hand-rolled, and a separate test pins the premise directly — membership survives, .input is undefined, the shape test returns false. A future change that re-attached payloads to the snapshot fails there, with a reason, instead of silently reviving a branch that was reasoned about as unreachable.
• Chosen behaviour: a snapshot can NEVER authorize this fallback. Not "sometimes, if it happens to say empty" — it structurally cannot say empty, so there is no case to gate. The alternative, teaching the snapshot to retain option lists, was rejected outright: that trades a real integrity guarantee for a convenience on the rarest path in the ladder. What it gets instead is its own reachable refusal, because falling through to the generic end-of-ladder message would tell the caller only that their combo could not be read — sending them to look at their value while the actual cause is a silent backend, which is the exact misattribution this whole change exists to stop.

The fallback is also restructured so evidence is established before the shape test: the three stale layers (cache, reconnected, snapshot) are indistinguishable to a shape test, and one of them cannot answer it at all. The stale-vs-live disagreement report is now reached through reAsked && staleDeclaredEmpty, which is a fact captured before authDefs is overwritten rather than re-derived from it.

Review round 5 — stop re-deriving liveness; let the cache answer

9. A response an invalidate() retired mid-flight is not live either (P1)

registerComfyNodeDefs drops the burst cache on a refresh, a pack install, or a download completing. That bumps the cache generation without moving the reconnect epoch — and object-info-cache deliberately still hands the retired response to its original waiter. The classifier called it "live" because the loader had run, so an old schema declaring [] could authorize a blind write after the refresh installed a real option list.

That was the fourth distinct way a response turned out not to be live:

round	"not live" because	how the classifier missed it
1	it came from #1223's snapshot	—
2	#716's cache served it from the TTL	the loader had not run
3	the backend reconnected mid-flight	the epoch moved after the loader ran
4	an invalidate() retired it mid-flight	the generation moved, and the epoch did not

Four rounds, four patches, each correct and each to the same reconstructed test. So the fix is not a fifth condition. The panel was inferring liveness from a proxy (did my loader body run?) while the component that actually knows tracks it precisely: object-info-cache.js owns the generation counter whose entire purpose is retiring requests, and it decides whether each read is served, joined, or issued.

It now answers the question itself:

const { value, provenance } = await objectInfoCache.readWithProvenance(loader, {
  stamp: () => backendReconnectEpoch,
});

provenance is "live" | "cache" | "reconnected" | "retired" | "unknown", decided after the request settles from state the cache owns. The reconnect epoch is the one fact the cache cannot know, so it is handed in as an opaque stamp captured at issuance and re-checked at delivery — the cache never learns what a reconnect is, only that a caller-owned fact must not have moved. read() keeps its old contract (payload only), so no other consumer changed.

What it costs, stated plainly:

• One new method on a shared, heavily-guarded component. The request-construction body was lifted out of the returned object literal into a shared readInternal; the load-bearing invariants (yield-first before the slot assignment, the requestId captured before the promise, the finally releasing only its own slot) are unchanged and still covered by that file's existing tests.
• read() now awaits internally rather than returning the in-flight promise directly. Behaviourally identical for an async function, and no test depended on promise identity — but it is a real change to a hot path.
• The cache gained an optional notion of a caller stamp. It is opaque and ignored when absent, so this is additive.
• objectInfoSnapshot.record is now gated on the same verdict in both routes that file into it. record's own epoch test would still have accepted a generation-retired response, so the snapshot could retain a schema the panel itself had just superseded. record's checks stay as defence in depth for callers with no verdict to offer.

What a fifth would look like — and this is the payoff: a new retirement mechanism added to object-info-cache.js is classified by that file, at the point it is introduced, instead of reading as "live" at every call site until someone notices. The one thing that would still escape is a staleness source outside the cache, and there the honest answer is that no single authority can help: "snapshot" is exactly that case, which is why the panel still reports it itself.

I also fixed the same latent hole in graph_remove_widget, which files into the same snapshot and had the same observedAtEpoch !== null gate. Not in the findings; found while removing the pattern.

10. A nested promotion is refused before paying for a re-fetch (P2)

The chain shape is already known and no schema can make the write succeed, so on a cache/reconnected/retired/snapshot provenance the ladder was dropping the burst cache and downloading a multi-megabyte /object_info purely to reach a refusal it had already decided on. The nested check now runs first. The test asserts the cost directly (invalidated === 0, one fetch total), not just the message.

11. A post-write failure is no longer reported as a refusal (P2)

Every panel_set_widget refused … message asserts more than the text it wraps: that this was a refusal, so nothing was applied. Exactly one WidgetWriteError breaks that — the one raised after the graph was mutated when the rollback failed to restore it ("the graph may be in a partial state"). All four reframing sites were wrapping it anyway, telling the caller nothing happened about a graph that had changed.

That error now carries partialWrite: true, and a single refusalFrame() helper — the one place a WidgetWriteError becomes a refusal message — propagates it verbatim instead. Telling a caller "nothing was applied" when something was is the exact defect this PR family exists to avoid.

Review round 6 — a verdict is about a moment, and it expires

12. Revalidate liveness after the recovery awaits (P1)

Rounds 1–4 all asked what kind of response is this, and the round-5 refactor gave that one authority. This is a different question: when was that true. The initial /object_info read can be genuinely "live" — and then definitions change while refreshCombos and tryUploadAssetAccept are awaited. The stored string still said "live", so the blind-write path authorized from a schema that had since been superseded, bypassing a list that may have become non-empty in exactly that window.

The thorough fix was the cheap one, because I already own the file: the verdict now carries the ability to re-ask itself.

const { value, provenance, provenanceNow } = await cache.readWithProvenance(loader, { stamp });

provenance is the classification at delivery — a historical fact that does not mutate. provenanceNow() recomputes from scratch on every call. The panel stores the question, not the answer, and the lib's existing schemaProvenance() call — which already happens after those awaits — now re-asks and gets the truth about then.

This is the same lesson #1223 records for its snapshot: provenance describes the connection, not the bytes, so it has to be re-asked rather than remembered.

13. Coalesce the cache-bypass rereads (P2)

The risk I flagged two rounds ago arrived. invalidate() + re-read is global: two cache-sourced unreadable writes reaching the fallback together each invalidated, and the second retired the first's just-issued request — so a perfectly valid write was handed "retired" and refused. One caller breaking another. And nothing coalesced, so a burst paid for one multi-megabyte /object_info per caller — the exact symptom #716 exists to prevent, reintroduced on the recovery path.

readFresh() replaces it: it bypasses only the stored entry, never touching the generation or the ordinary in-flight slot, and keeps its own slot that concurrent forced rereads join. A joiner gets "live" rather than "cache" because the request it rides was issued by a forced read — it bypassed the TTL by construction, which is precisely what a joiner normally cannot vouch for. Each joiner still captures its own stamp at join and re-checks at delivery, so a reconnect landing on one caller is reported to that caller alone. An invalidate() still retires a forced reread, because a reread issued before an invalidation is exactly as superseded as an ordinary one.

The lib's capability is now named for the outcome it needs (refetchObjectInfoLive) rather than the mechanism, so the cache decides how to get a fresh answer without disturbing anyone else.

14. A refusal that reported an option list it never observed

The stale-vs-live disagreement refusal said "re-asking the server produced one that DOES publish a list for it." What was actually observed is a negative: serverDeclaresEmptyComboOptions returned false. That is equally true when the server publishes a real list and when the re-read no longer describes this input — or this type — at all. Asserting the first as fact is the same over-claim this PR exists to remove, committed by the message written to explain it. It now states the observation and names the possibilities.

Review round 7 — the joiner's stamp belonged to the request, not the caller

15. Preserve the request stamp when coalescing forced reads (P1)

This is the risk the previous round flagged in its own handover, and it was wrong in the direction that round predicted — the permissive one.

readFresh reported "live" to a joiner on the reasoning that the request it rides bypassed the TTL by construction. That is true of the payload's age, and irrelevant to the connection. The stamp was captured per caller rather than stored on the in-flight record, so:

1. a forced read is issued on epoch A;
2. the backend process is replaced, advancing the stamp to B;
3. a second caller reaches readFresh, reads the current epoch as its own B, joins;
4. delivery compares B to B and reports "live" — for a response the previous process produced.

No invalidate() is needed to reach it: when the reconnect-triggered node-def refresh coalesces with one already running, the generation never moves. The unreadable-combo fallback would then blind-write an off-list value against a schema published by a backend that no longer exists — the exact class of write this PR exists to prevent, on the replacement backend.

The issuance facts now live on the record (issuedStamp, readStamp, stampUnreadable), and every joiner is classified against those. A joiner that asks for reconnect detection on a request issued without a stamp gets "unknown" rather than "live": the issuance epoch was never recorded and cannot be reconstructed, and nothing established must never read as live.

It is the same distinction provenanceNow() was built for, one level down — there it was delivery-time versus now, here it is which moment the comparison is anchored at. A request cannot vouch for a connection that changed after it was issued, however fresh its payload.

And labelling it is not enough. A reconnect-spanning response is now also refused a place in the cache, on both paths — the ordinary one had the same hole, since a reconnect can advance the stamp without bumping the generation. Storing it would serve the dead process's schema to every later reader as "cache" for the rest of the TTL, turning one badly-timed response into a second and a half of them. The response is still returned to the caller that awaited it, correctly labelled; it is only denied authority and storage.

Reviewer note: read object-info-cache.js first

This file is the highest-risk part of the PR and should be reviewed as such. It has now been modified in three consecutive review rounds, and its own header documents three separate review-found bugs in roughly sixty lines of request logic — a synchronously-throwing loader corrupting the in-flight slot, an outcome wrapper being cached as a success, and a joined read reporting as issued. Rounds 5, 6 and 7 each added to that count: the delivered-verdict refactor, the coalescing forced-reread slot, and the joiner stamp above.

The concentration is not accidental. This is where "how did we come to believe this schema" is decided, and every review round has found that the previous answer was under-specified in the permissive direction. Concretely, the parts most worth an independent check:

• the two in-flight slots (inflight and freshInflight) and their interaction with invalidate(), which must retire both;
• each path's finally releasing only its own slot;
• the classification anchoring — verdict() is called with the record's issuance facts on the forced path and the call's own on the ordinary one, because on the ordinary path the issuer is the only caller that can be live.

What this does NOT fix, and why #1126 stays open

#1126's tell is Valid options (1): empty — a list that read perfectly well and held one placeholder. I drove the real runSetWidget against that literal shape:

case	result
A) live ["empty"], server ["empty"]	REFUSED — "Its option list WAS read successfully and holds 1 …"
B) live ["empty"], server []	REFUSED — same
C) options.values() throws, server [] fetched live	WRITTEN (option_list_unreadable)

Only C is what this PR fixes. The reporter's combo is case A, so their symptom survives and their workaround (copying the file into ComfyUI's input directory) is still required.

That is not a gap to be papered over here. Nothing observable from the panel separates "a placeholder the node's JS inserted" from "real options the server could not enumerate" — not the option names (None is a real, meaningful option on plenty of nodes: a LoRA slot's "no lora"), and not the server's declaration, which also covers the StarNodes combo whose own JS populates a list of REAL models where an off-list value is a typo worth refusing. Inferring it was tried and broke three #507 invariants that are correct.

Case A needs the caller, because the caller is the one who knows the node accepts values its dropdown cannot list.

Follow-up that does close #1126

Branch: fix/1126-assertion-approach-dr1133

A per-call allow_unlisted assertion on panel_set_widget that converts a would-be combo refusal into an unvalidated, disclosed write. It closes case A. It is complete and verified — full suite green, 8/8 mutations killed, including the #240 regression (a rail listing the NUMBER 4444 must not replace the caller's string "4444") and #347's clear-to-"" rule.

It is not landing yet because it is inert: panel_set_widget's argument schema lives in comfyui-mcp, and src/orchestrator/panel-tools.ts currently neither declares allow_unlisted in the zod schema (the tool is additionalProperties: false, so a client that sends it gets -32602 Unrecognized key) nor forwards it into ctx.call({ cmd: "graph_set_widget", … }). Both are required upstream before the panel half can ever fire.

Sequencing rule: the panel-side half lands now; the caller-side half lands when the caller can actually send it. Shipping a route that cannot fire is a trap this repo has now hit three times (#1223 v1 recorded a snapshot on a path that never runs at startup; #757 v1 tested an object shape while production sends a JSON string; and round 4 of this PR, where the snapshot branch could never be entered because the test hand-built a schema shape production never produces). Each looked correct, each was fully tested, each was dead.

Verification

• npm run test:unit — 4455 tests, 4454 pass, 0 fail, 1 todo (includes scripts/i18n-check, i18n-render-check, check-tool-vocabulary, and check-panel-scope — every name in the panel bundle resolves)

• npm run typecheck — clean

• Merge state against main: CLEAN. origin/main (through v0.14.41 / #1478) is merged in; the only shared file is comfyui-mcp-panel.js and it auto-merged, with both sides re-verified afterwards.

  Caveat, recorded rather than hidden: manager-install.test.mjs #671 verifyInstalled … is a wall-clock budget test that intermittently fails under full-suite parallel load on a Windows dev box (observed 24–53s against its budget). It is pre-existing and unrelated — it fails identically on this branch tip with all panel_set_widget cannot set an absolute path on a dynamic file combo even though the node accepts it at runtime #1126 changes stashed, and passes in isolation on this exact tree. It did not fire on the runs recorded above. Flagged as a possible real Windows-perf signal worth its own issue, not addressed here.

Mutation tested — 36 applied to this design, 36 killed. Restored from an in-memory snapshot (never git checkout), with every anchor asserted to occur exactly once first, because this checkout is CRLF and a silently-missed LF anchor reads exactly like a kill.

#	mutation	killed by
T-1	the panel stores the delivered string again — a verdict that expires mid-ladder goes unnoticed	graph_set_widget THREADS the schema provenance into runSetWidget
T-2	provenanceNow replays the delivered verdict instead of recomputing it	a verdict EXPIRES — provenanceNow re-answers, it does not replay
T-3	the lib reads the provenance once, before the recovery awaits, and trusts it after	graph_set_widget THREADS the schema provenance into runSetWidget
K-1	readFresh bumps the generation — one caller's reread retires another's request	two concurrent readFresh callers COALESCE and neither retires the other
K-2	concurrent forced rereads do not coalesce — a burst pays per caller	two concurrent readFresh callers COALESCE and neither retires the other
K-3	readFresh is satisfied by the stored entry — the bypass does not bypass	two concurrent readFresh callers COALESCE and neither retires the other
K-5	the lib reaches for the global invalidate() again instead of the coalescing reread	graph_set_widget THREADS the schema provenance into runSetWidget
O-1	the re-read refusal asserts the server does publish a list — a negative reported as a positive	a re-ask that finds a REAL list refuses instead of writing blind
A-1	the cache never reports a retired response — an invalidate() mid-flight reads as live	an invalidate() DURING the request retires it — 'retired', not live
A-2	the cache never reports reconnected — a stamp that moved reads as live	a stamp that MOVES mid-flight is 'reconnected', and outranks a retirement
A-3	a served cache hit is reported live	a SERVED cache hit and a JOINED read are both 'cache'
A-4	a joined read is reported live — riding another call's request reads as asking	a SERVED cache hit and a JOINED read are both 'cache'
A-5	a throwing stamp fails OPEN in the cache	a THROWING stamp establishes nothing — 'unknown', never live
A-6	the stamp is captured after the await, so a mid-flight reconnect can never be seen	an ISSUED request that nothing retired is live
A-7	the panel stops handing the cache its reconnect stamp	graph_set_widget THREADS the schema provenance into runSetWidget
A-8	the snapshot files a non-live response again — a retired schema becomes last-observed	graph_set_widget THREADS the schema provenance into runSetWidget
F1-c4	"retired" drops out of the recognizer — a superseded response reads as unknown	a RETIRED response is not live
W-1	a partial write is reworded into a "refused" frame — the caller is told nothing landed	a PARTIAL write is never reworded into a 'refused' frame
W-2	the post-mutation error stops declaring itself partial	a PARTIAL write is never reworded into a 'refused' frame
F1-a	drop the provenance gate entirely — any schema authorizes the blind write	a RECONNECT-SPANNING response is not live
F1-b	invert the gate — refuse on LIVE, authorize from stale evidence	an UNREADABLE option list writes the path and discloses that nothing checked it
F1-c	UNKNOWN provenance fails OPEN — a throwing probe reads as live	UNKNOWN provenance fails closed — a throwing probe is not a live one
F1-c2	ignore the provenance answer — every schema reads as live	a SNAPSHOT can never authorize the blind write, and says why
F1-c3	"reconnected" drops out of the recognizer — a replaced process reads as unknown	a RECONNECT-SPANNING response is not live
F1-d	the panel stops threading provenance — the lib can only infer it	graph_set_widget THREADS the schema provenance into runSetWidget
X-1	a reconnect-spanning response is relabelled live — the replaced process authorizes	graph_set_widget THREADS the schema provenance into runSetWidget
S-1	delete the snapshot branch — a name-only map falls through to the generic refusal	a SNAPSHOT can never authorize the blind write, and says why
S-2	undo the snapshot's detachment — EMPTY_DEF carries the real def through	the real snapshot map holds NAMES ONLY — it cannot answer an option-list question
C-1	treat a #716 cache hit as live — "not from a snapshot" read as "the server answered"	a CACHE-HIT empty list does NOT authorize a blind write either
C-2	never re-ask — a cache hit can only fail closed, refusing writes 2..N of a burst	a cache hit is RE-ASKED, and a live empty answer authorizes the write
C-3	the panel marks a cache hit as live — the epoch gating #1223's record is ignored	graph_set_widget THREADS the schema provenance into runSetWidget
C-4	drop the stale-vs-live disagreement report — the actionable refusal goes generic	a re-ask that finds a REAL list refuses instead of writing blind
C-5	never drop the cache, so the re-ask can be answered by the stale entry itself	graph_set_widget THREADS the schema provenance into runSetWidget
E-1	drop the empty acceptance from the final write — a stateful [] answer is refused	a stateful callback that finally answers [] takes #507's acceptance, not a refusal
E-2	applyWidgetWrite stops naming the empty acceptance, so the reply cannot report it	a stateful callback that finally answers [] takes #507's acceptance, not a refusal
F2-a	blind-write a NESTED promotion instead of refusing it	the unreadable fallback REFUSES a nested promotion rather than writing blind
F2-b	nestedPromotion is recorded but never becomes true — a dead flag	the unreadable fallback REFUSES a nested promotion rather than writing blind
F2-c	invert it — refuse the SINGLE-HOP promotion, blind-write the nested one	an UNREADABLE option list writes the path and discloses that nothing checked it
F3-a	the reply note reverts to the unqualified "nothing checked it"	the reply note does not claim 'nothing checked it' when the RAIL did
F3-b	applyWidgetWrite never emits promoted_rail_validated	the reply note does not claim 'nothing checked it' when the RAIL did
F3-c	claim rail validation ALWAYS — over-claim in the other direction	a rail that VALIDATED the value is reported as having done so
F3-d	the activity summary ignores the rail-checked variant	a value the parent RAIL validated is NOT reported as wholly unchecked
F3-e	credit ANY sibling — a #477 display proxy match claims the rail validated it	a DISPLAY PROXY match does not claim the serializing RAIL validated the value
R-1	regression: drop the serverDeclaresEmptyComboOptions requirement on the unreadable path	a failed live read is NOT licence to ignore a list the SERVER publishes
R-2	regression: let the unreadable path ADOPT a rail's numeric label (#240/#667)	the unreadable acceptance ARMS the rail cross-check but NEVER adopts a rail label

F1-d is the one that mattered most: it survived the first harness run. Nothing proved the panel actually hands the lib the provenance, so stubbing the wiring to a constant left every behavioural test green while production authorized blind writes from stale schemas again — the exact dead-code shape this PR's own body warns about. A source-level wiring assertion (like #718's fence) now pins it, and C-3, C-5 and X-1 ride on the same test: every branch of the oracle must record its provenance, "live" must stay gated on both facts objectInfoSnapshot.record is gated on (an epoch captured inside the cache loader, and that epoch still matching), and the lib must be able to drop the cache before re-asking.

S-2 is the guard against the round-4 defect recurring: it mutates object-info-snapshot.js itself, so if the snapshot ever stopped detaching, the fixture test fails with a reason instead of the #1126 branch quietly changing shape underneath a suite that would still be green.

One mutation is recorded as an equivalent mutant rather than a hole. An earlier form of F1-c2 (return "live" || false || p === …) survived, and correctly so: it left the ternary condition truthy, so the reader still returned p unchanged. It is noted in the harness so a later run does not re-chase it, and replaced with a form that actually short-circuits the reader. The round-3 mutations C-3/X-1 were retired outright — they anchored the hand-rolled panel classifier that round 5 deleted, and their subject matter is now covered by A-1…A-6 inside the cache.

Draft on purpose — review and merge are the maintainer's.

[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.

[artokun]

Not mergeable as it stands — holding

Two independent reasons, both established rather than suspected.

1. The escape cannot be reached by any current caller

Measured against the live orchestrator (comfyui-mcp 0.51.16):

panel_set_widget params: node_id, widget, value, clear, retry_of
additionalProperties: false
{"allow_unlisted": true}  ->  MCP error -32602: Unrecognized key: "allow_unlisted"

panel_set_widget's argument schema lives in comfyui-mcp, a different package. Until it declares the option, this branch is inert — and the refusal was originally telling agents to "retry with allow_unlisted:true", i.e. pointing at a recovery that returns a schema error. That is #932's circularity in a new form. The message now states it as a capability requirement and says outright that Unrecognized key means stop retrying.

2. The implementation has confirmed defects, including one that falsifies its own safety claim

• The #240 STRING-only guarantee is false. Reproduced on HEAD: an inner combo options:{values:["empty"]} promoted to a rail whose list holds the NUMBER 4444. The acceptance sets emptyAcceptanceUsed to arm the panel_set_widget can never write a dynamic combo whose server-side option list is empty (StarNodes Ollama "model") #507 sibling cross-check, and that check's panel_set_widget cannot select a combo option whose label is numeric (e.g. ProRes profile "4444") #667 numeric-label adoption then replaces the admitted string with the rail's number, writing a non-string to the widget. The safety addition introduced the hazard it was added to prevent.
• typeof "" === "string", so the flag admits an empty string to a combo — reopening panel_restart_comfyui hangs until 300 second tool timeout #347's rule that clearing a combo to "" must be refused, through a different door.
• The reply renderer has no knowledge of off_list_value_accepted, so the disclosure that justified permitting an unvalidated write never reaches the user-visible reply.
• No hello-frame advertisement, so an orchestrator could not discover whether the connected panel honours the option.
• A truthy non-boolean ("true") is silently ignored while the refusal keeps advertising the flag.

Also recorded: two rejected alternatives

Inferring the case from serverDeclaresEmptyComboOptions — broke three #507 invariants that are correct, because that condition also covers a combo whose own JS populates REAL options, where an off-list value is a typo worth refusing.

Accepting any off-list string by default (no opt-in) — broke three further independent guards: #347's clear-to-"" rule, "an invalid value is REJECTED, not coerced to another enum" on a lllite_name FILE combo (the common case, where a wrong filename fails deep in model loading), and the dynamic-list equivalent. The combo guard is not friction; it is doing real work in the majority case.

What would make this shippable

1. allow_unlisted declared in comfyui-mcp's panel_set_widget schema.
2. The acceptance must NOT arm the sibling cross-check, and must have its own promoted-write rule that cannot adopt a rail's numeric label.
3. A non-empty string test, not typeof.
4. Disclosure wired into the reply renderer, and the capability advertised on hello.

Leaving this as a draft rather than merging an inert escape with confirmed defects. #1126 stays open with the workaround that does work today.

[artokun]

Review finished — one root cause behind nearly all of it

The full verify pass landed (29 agents, 11 refuted / 4 plausible / 13 confirmed). Its synthesis is sharper than my hold note above, and worth recording precisely: the escape is verified working only for a direct, non-promoted combo with a real closed list, and broken in most other shapes — almost all traceable to one decision, reusing emptyAcceptanceUsed instead of giving this path its own marker.

That decision was mine, and I made it as a safety fix two commits in. It caused two confirmed defects at once:

1. It routes the accepted string into the #667 rail label-adoption, so a NUMBER lands on the widget. Verified on the committed snapshot: rail {values:[4444]}, caller sends the string "4444" with the flag, and the NUMBER 4444 is written to both the rail and the inner widget whose live list is ["empty"]. That is exactly #240's index-drift hazard, which my own comment claims survives "by construction — STRING only". It also writes a value the caller never sent while off_list_note asserts "the graph now holds a value no option list vouches for", which is false for an adopted option. Before this branch, adoption could only fire when the inner list was empty — which was its entire justification.

2. It makes the flag unusable inside a subgraph, with a diagnosis that states a falsehood. The #1126 node is very plausibly promoted, and then the parent rail mirrors the placeholder list. Verified: the write is refused with … is not a valid option for the parent subgraph's combo widget "fbx_alias" (1 options) … the inner widget's option list is empty, but this one is not. The inner list is ["empty"] — non-empty. And my comment's stated remedy, "the caller can address the rail directly", is not reachable: the outer widget is this path. The user is left with the copy-into-input-dir workaround this PR exists to remove, plus a message sending the agent to look for an empty list that does not exist.

Two further confirmed gaps in the ladder:

3. Gated on offList alone, so the dynamic-empty-combo family ignores the flag entirely and terminates on an internal "refreshing before deciding" retry message — i.e. the case closest to the report is the one the flag does not reach.

4. A non-string with the flag set re-throws the raw WidgetWriteError out of runSetWidget unframed, so the retry the refusal instructed yields Value 640 is not a valid option… with no node id, node type, or tool name attached.

Plus the lower-severity set already noted: the panel's activity summary renders an admittedly-unvalidated write as an ordinary success; a non-boolean allow_unlisted is dropped with no diagnosis while the refusal keeps advertising it; and the parameter has no orchestrator counterpart or capability signal, so the branch is inert in production regardless.

Conclusion

Holding stands, and the fix list is now concrete rather than a guess:

1. Give this path its own marker; do not arm emptyAcceptanceUsed. Write a promoted-write rule for it that cannot adopt a rail's numeric label.
2. Non-empty string test, not typeof.
3. Extend the trigger past offList to the dynamic-empty-combo family.
4. Keep the panel_set_widget refused … frame on every path, including the flagged retry.
5. Wire the disclosure into the reply renderer, advertise the capability on hello, and diagnose a non-boolean instead of dropping it.
6. Declare allow_unlisted in comfyui-mcp's panel_set_widget schema — without which none of the above is reachable.

Worth being explicit that the review earned its keep here twice over: my "safety" commit introduced the two worst defects, and the test suite had gone green over all of it.

[artokun]

Reworked against the hold — the decision no longer comes from the caller

The fix list was concrete, but items 1–5 were all downstream of item 6: allow_unlisted is an assertion the panel cannot verify, so it had to be laundered through emptyAcceptanceUsed to stay safe, and it could never be reached anyway without a comfyui-mcp schema change. Rather than repair five symptoms of that, this replaces the assertion with the panel's own observation.

options.values(widget) is the node's callback. Either the panel read the list or it did not:

• could not read it (threw / returned a non-list / absent) — nothing was compared to anything, so the old refusal was making a claim about the value it had no basis for. Accepted as a last resort, after the refresh and the upload probe, and only when /object_info declares this input's list empty too.
• read it, value absent — refused, unchanged. not-installed:70b against a real list still fails here.

What that does to the fix list:

1. Own marker, no emptyAcceptanceUsed. ✅ It arms the rail cross-check (a promoted write still mutates the rail) but is excluded from panel_set_widget cannot select a combo option whose label is numeric (e.g. ProRes profile "4444") #667's label adoption — that rule's justification is "the inner list was empty, so any scalar was admissible", which is false of a list that exists and could not be read. The 4444 case you reproduced is now a refusal, and the cross-check no longer says "the inner widget's option list is empty" about a list nobody read.
2. Non-empty string test. ✅ "" refused.
3. Trigger reaches the dynamic family. ✅ It is now keyed on the unreadable observation directly, not on an off-list miss.
4. Frame kept. ✅ The last-resort attempt's own refusal is framed with tool/widget/node; non-validation failures propagate unchanged.
5. Disclosure + capability. ✅ Wired into the activity summary as data, not prose. No hello advertisement needed — there is no caller-facing option left to discover.
6. comfyui-mcp schema. ✅ Moot. No new tool argument, nothing cross-repo, not inert.

Two things I want visible rather than buried:

• This does not make the reporter's literal case writable. Valid options (1): empty reads successfully and holds one option, so it refuses by design. Your review already killed both ways of guessing at that (the name denylist and the server-declaration inference), and I did not find a third. panel_set_widget cannot set an absolute path on a dynamic file combo even though the node accepts it at runtime #1126 should stay open.
• When the live read fails but the server publishes a real list, the write is refused rather than validated against the server's list — the combo refresh deliberately never clobbers a function-valued options.values, so the widget never adopts it. Same as main; pinned by a test in both directions so it is a choice, not an accident.

Verification: 18 tests both directions, 5 mutations applied and each killed by a named test (unconditional adoption / cross-check skipped / frame dropped / server gate removed / summary disclosure deleted). npm run test:unit is 4411 pass, 1 fail — manager-install #671, a wall-clock budget assertion that fails identically on a pristine origin/main under the same parallel run and passes in isolation.

Left as a draft: the shape of the decision changed, so this wants a fresh read rather than a diff against the held version.