Skip to content

panel_run's duplicate fence refuses a post-reconnect scoped preview, and the override it names is documented as not applying to that caller #1615

Description

@artokun

Refiled from artokun/comfyui-mcp-panel#1127 (label via-panel). Triaged in the panel repo and closed there: not one character of this refusal exists in the panel pack, and the fence never consults the panel. The report is entirely this repo's.

Reporter: Windows 11; ComfyUI 0.29.0; frontend 1.47.11; comfyui-mcp 0.51.16; panel version unknown.

What they hit

After reconnecting to a copied workflow, panel_run(to_node_id: 16) — a partial render to a PreviewImage, i.e. incremental verification of one output branch — was refused. Nothing was queued.

panel_run refused to queue: a render is already in flight … this session cannot confirm it as its own … queueing now would stack a DUPLICATE behind it (#862).

Their scenario (from the report): reconnect while a prior render or a user's own manual render is still running, then ask for a scoped preview.

Where it comes from

  • src/orchestrator/panel-tools.ts — the fence condition and the refusal text (allow_duplicate !== true && !explicitRetry && pre.connected && (pre.running || pre.queueDepth > 0) && !pre.selfAttributedProven).
  • src/services/queue-monitor.ts — snapshot(), markSelfQueued(), isSelfAttributedProven().

Not already fixed. The fence condition, the refusal text, and the allow_duplicate description are byte-identical between v0.51.16 (the reporter's version) and origin/main (v0.51.56) — verified by md5 of each region across both refs. git log v0.51.16..origin/main on both files shows 17 commits, none touching the fence.

Three findings, in the order I'd rank them

1. The remedy exists, and its own documentation tells this caller it does not apply to them

The reporter's second requested fix — "permit an explicit safe partial-output preview when the user has requested it" — is already implemented as allow_duplicate: true, and the message they quoted names it.

But both places it is documented scope it to a use case that is precisely not theirs:

allow_duplicate: "… Pass true only to deliberately queue behind it (a sweep/batch)."

refusal text:   "If you genuinely intend to stack another render behind it
                 (a deliberate sweep/batch), re-call panel_run with allow_duplicate:true."

An agent that wants a one-off scoped preview after a reconnect is not running a sweep or a batch. Reading only … (a sweep/batch), the correct inference is that the override is for someone else. So the caller is handed an escape hatch and simultaneously told it is the wrong one, and stops — which is exactly what the report describes ("The refusal prevents normal incremental verification after reconnect").

The post-reconnect "I checked the queue and this in-flight job is fine to queue behind" case is the most common legitimate use of this override, and it is the one case the wording omits. The fence's own comment already knows this is the expected situation: "after a reconnect this is usually YOUR earlier render still running."

This is the cheapest real fix here and it changes no behaviour: name the post-reconnect verified case in both strings, and keep the sweep/batch case as an example rather than the definition.

2. The refusal blames "an orchestrator restart", but a panel reconnect can drop attribution too

The refusal says the record is lost "after a reconnect/restart (or when an earlier queue reply carried no prompt id)", and the code comments consistently explain it as an orchestrator restart. There is a second path that needs no orchestrator restart at all:

QueueMonitor.start() clears selfQueuedIds and lastSelfQueueTs on any retarget — and its own docstring names the trigger:

"the orchestrator calls stop()+start(newUrl) when ComfyUI is retargeted (e.g. 127.0.0.1→localhost from a panel hello)"

So a panel reconnect whose hello carries a differently-spelled-but-identical ComfyUI URL wipes ownership for renders that are still in flight on the very same server, and every subsequent panel_run is fenced until they drain. That is a strictly wrong outcome, not an accepted cost: the jobs did not become foreign, only their spelling did.

I have not confirmed this is the path the reporter took — I have no log from them, and the ordinary orchestrator-restart path explains their report equally well. It is a real hole in the same mechanism either way, and worth deciding on independently.

3. The panel already holds the answer, and nothing asks it for it

isSelfAttributedProven() can only ever consult this process's in-memory selfQueuedIds. Meanwhile the panel keeps its own registry of the prompt ids it queued:

  • comfyui-mcp-panel/web/js/lib/run-completion.js → createRunCompletionTracker, populated by onQueued(pid), exposed as unsettledPromptIds().
  • The ref is module-scoped on purpose (web/js/comfyui-mcp-panel.js), so it survives a bridge reconnect; it is cleared only by a page reload. That was established in panel#925's triage.
  • It is currently used only to build the reboot marker — it is not exposed over the bridge.

So the reporter's first requested fix ("expose prompt metadata/branch ownership after reconnect") is buildable, but it is a two-repo change and the consuming half is here: the panel would publish owned prompt ids, and the fence would ask before refusing. Filing it as the same-root-cause option, not proposing it as the fix for this ticket — under the current freeze, findings 1 and 2 are the bug-shaped parts.

What I verified, and what I did not

Verified:

  • git grep for refused to queue, stack a DUPLICATE, cannot confirm it as its own, allow_duplicate, selfAttributedProven, #862 across all 508 tracked files of comfyui-mcp-panel → 0 files. The panel cannot emit this message.
  • The fence's inputs are QueueMonitor.snapshot(), fed by the watchdog's own WS/HTTP poll of ComfyUI. The panel is not consulted in the decision.
  • markSelfQueued has exactly one non-test caller: this repo's panel_run handler, from acceptedPromptIds(runReply).
  • The panel's side of that handoff is correct — buildQueueAcceptResult (web/js/lib/queue-rejection.js) emits prompt_id and, for batch_count > 1, prompt_ids, which is exactly what acceptedPromptIds reads. So "the panel returned no prompt id" is not what happened here.
  • The fence regions are unchanged between v0.51.16 and origin/main.

Not verified (no repro run, no reporter logs):

  • Which of the two attribution-loss paths the reporter actually took.
  • Whether their in-flight job was the agent's own earlier render or the user's manual one — the report says the repro covers both.

Related: panel#1127 (this report), panel#925 (same root cause per the reporter; fixed here in 0.50.85), and #862 / #1011, which built and then narrowed this fence.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions