fix(1175): a run acknowledged after its reply timeout is not an unknown outcome - #1630
Merged
Conversation
There was a problem hiding this comment.
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.
comfyui-mcp-panel#1175. `panel_run({to_node_id:8})` bounds `graph_run` at a
hard-coded 20000 ms. The panel acknowledged the same run 5.3s later, having
successfully queued it; the caller had already been handed outcome-unknown, and
the queue listing the disclosure sends them to was still empty.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…wn outcome
`panel_run({to_node_id:8})` returned OUTCOME UNKNOWN on the 20 000 ms reply
bound. The panel acknowledged that same run 5.3 s later, having queued it. The
caller was sent to `queue (action:"list")`, which came back EMPTY — during the
propagation window an accepted prompt is not visible there yet, so the one check
the disclosure offers reads as "nothing was queued" for a run that was. Only the
#694 retry token eventually recovered it, at the cost of a whole extra agent
round trip to learn something the orchestrator was already holding.
THE BOUND IS NOT WHAT THIS CHANGES, and it is worth saying why, because
"extend the mutation acknowledgement window" is the reporter's own first
suggestion. `graph_run` is bounded at the shared 20 000 ms default, chosen for
commands whose panel-side work is one synchronous canvas edit. A SCOPED run's is
not — measured in the panel, not assumed:
- the handler serialises the prompt once for its unrunnable-type pre-flight
(`app.graphToPrompt()`, comfyui-mcp-panel web/js/comfyui-mcp-panel.js);
- `dispatchScopedRun` serialises it AGAIN for the content hash that attributes
the POST to this run (run-scope-guard.js);
- `queuePromptScopeAttempts` returns FOUR argument shapes, and each attempt
calls `app.queuePrompt`, which serialises again and POSTs /prompt;
- `verifyTimeoutMs` is 5 000 ms and is entered AFTER queuePrompt resolves —
i.e. work that can begin near the end of our window and run past it.
None of that is bounded from out here, so any literal we pick is a number that
happens to fit the rigs we measured: raising it would have made this reporter's
run fit and the next one's not.
So: keep the bound, and stop declaring the outcome unknown while the answer is
still arriving. On a reply-timeout panel_run now waits a bounded grace for the
panel's acknowledgement and, when it lands, hands back THAT reply. This is the
deadline+grace shape `confirm()` already uses for a slow ask_user card, over the
retention #694 already installs for exactly these commands.
- NOTHING IS RE-DISPATCHED. Asserted from the wire, not from the tool's own
account: the scripted panel counts rids and the test requires exactly one.
A reconcile that recovered by asking again would double-render, which is the
whole reason the retry token was acceptable in the first place.
- THE PANEL'S OWN REPLY IS REBUILT, not described. `ok(result)` is byte-for-byte
what ctx.call would have produced on time, so a recovered run takes the
identical path: the same rejection detection, the same prompt-id ticketing,
the same anti-poll guidance. A late `queued:false` therefore stays a refusal
— the bridge retains `ok:true`, which means the executor ANSWERED, not that
it queued anything.
- A RUN THAT NEVER ANSWERS IS REPORTED EXACTLY AS IT IS TODAY, retry token and
all. The grace is paid only on a path that already spent its full bound and
was about to hand back an unknown.
#694 kept only the FACT of a late completion, which is all the retry-token notice
needs; the run's prompt ids live in the body, so the body is retained too — under
a 32 KiB ceiling, because that map is bounded by count and a mutation reply is
small only by convention. Over the ceiling the ENTRY SURVIVES and only the body
is dropped, so the retry-token recovery is never lost to a large payload. The
reconcile PEEKS before it takes for the same reason: takeLateMutation drains by
design, and consuming an entry to answer a question we then could not act on
would delete the recovery the caller still had.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…the grace
Found by mutation, not by review. Deleting `isReplyTimeoutResult(res)` from
reconcileLateRunAck left all seven tests green while making EVERY healthy
panel_run wait out the full grace before returning: the loop peeks a rid nothing
retained, misses, and sleeps to its deadline. The gate has no observable effect
on a success other than the wall clock, so the wall clock is what has to be
asserted.
Measured on performance.now(), not Date.now() — a clock step must not be
readable as the reconcile having fired.
8 mutations now, 8 killed: dropping the reconcile call, never retaining a body,
removing the size ceiling, making peek drain, dropping the RECOVERED note,
dropping the reply-timeout gate, synthesising `{queued:true}` instead of using
the panel's own reply, and dropping the retained-body guard.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
artokun
force-pushed
the
fix/1175-late-run-ack
branch
from
August 16, 2026 18:27
77f303a to
61ad153
Compare
artokun
marked this pull request as ready for review
August 16, 2026 18:42
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Claiming artokun/comfyui-mcp-panel#1175. WIP — analysis and fix in progress.