app: a cancel that did nothing, and said nothing

/interrupt only consults the RUNNING half of ComfyUI's queue. A job waiting behind another one is not there, so cancelling it returned 200, logged "not currently running, skipping interrupt", and let the generation run in full when its turn came -- the user asked for it to stop, the UI showed it stopping, and the GPU spent the time anyway. Two tabs against one ComfyUI is all it takes, and one tab per generation is now the normal arrangement. ComfyUI has a route for exactly this: /api/jobs/<id>/cancel interrupts a running job and dequeues a pending one, atomically, and answers {"cancelled": false} for an id that already finished. An older ComfyUI without it falls back to the interrupt, which is right for the case people hit most. Reading that answer matters: a dequeued job never runs, so no execution_interrupted is ever sent, and the run would have sat at "Queued" forever after a cancel that worked. A run that IS executing is still left to the socket to declare stopped -- an interrupt is a request, and claiming it early shows "cancelled" over a model still finishing its step. The one test touching cancel drove execution_start first, so it only ever exercised the running case -- which is the half that already worked.
2026-08-30 04:17:29 +03:00
parent 4bc6a1e400
commit 6a588d3143
2 changed files with 125 additions and 13 deletions
@@ -75,7 +75,7 @@ export function createRun({
    // A cancel asked for while the id was unknown was never sent, because a
    // null prompt_id is a GLOBAL interrupt at ComfyUI: it stops whatever any
    // client is running. Now that the run has a name, it can be stopped by it.
-    if (cancelWanted) sendInterrupt();
+    if (cancelWanted) sendCancel();
  }

  function handle(message) {
@@ -157,12 +157,47 @@ export function createRun({
  // A cancel asked for before the run had a name.
  let cancelWanted = false;

-  async function sendInterrupt() {
+  async function sendCancel() {
    cancelWanted = false;
+    const id = state.promptId;
+
+    // ComfyUI's own state-agnostic cancel: it interrupts a job that is running
+    // and dequeues one that is only pending, atomically, and answers
+    // {"cancelled": false} for an id that has already finished.
+    //
+    // /interrupt alone was not enough and looked like it was. It only consults
+    // the RUNNING half of the queue, so cancelling a job waiting behind another
+    // one returned 200, logged "not currently running, skipping interrupt", and
+    // let the job run in full when its turn came -- a cancel that did nothing
+    // and said nothing, which is worse than a cancel that fails.
+    let response;
+    try {
+      response = await doFetch(`${base}/api/jobs/${encodeURIComponent(id)}/cancel`, {
+        method: "POST",
+        headers: { "Content-Type": "application/json" },
+      });
+    } catch {
+      response = null;
+    }
+    if (response && response.ok) {
+      // A job that was only PENDING is dequeued and never runs, so no
+      // execution_interrupted is ever sent -- nothing would arrive to move the
+      // UI off "Queued", and the run would look stuck forever after a cancel
+      // that worked. A RUNNING job is left to the socket, which says when it
+      // actually stopped rather than when it was asked to.
+      const body = await response.json().catch(() => ({}));
+      if (body && body.cancelled && state.phase === QUEUED) {
+        emit({ phase: CANCELLED, progress: null, node: null });
+      }
+      return response;
+    }
+
+    // An older ComfyUI without that route. Interrupting is all it can do, and
+    // it is right for the case people hit most: a run that is under way.
    return doFetch(`${base}/interrupt`, {
      method: "POST",
      headers: { "Content-Type": "application/json" },
-      body: JSON.stringify({ prompt_id: state.promptId }),
+      body: JSON.stringify({ prompt_id: id }),
    });
  }

@@ -249,7 +284,7 @@ export function createRun({
        cancelWanted = true;
        return true;
      }
-      await sendInterrupt();
+      await sendCancel();
      // Not marked cancelled here: the server says when it actually stopped,
      // and claiming it early is how a UI ends up showing "cancelled" over a
      // run that is still burning GPU time.
@@ -134,8 +134,10 @@ test("cancelling asks the server and waits for it to say it stopped", async () =
  run.handle(JSON.parse(message("execution_start", { prompt_id: "p-1" }).data));

  await run.cancel();
-  assert.equal(sent[1].url, "/interrupt");
-  assert.equal(sent[1].body.prompt_id, "p-1");
+  // The state-agnostic route, not /interrupt: a job waiting behind another one
+  // is not in the running half of the queue, so /interrupt answers 200 and does
+  // nothing at all.
+  assert.equal(sent[1].url, "/api/jobs/p-1/cancel");
  // Claiming it early is how a UI shows "cancelled" over a run still burning
  // GPU time. The server says when it actually stopped.
  assert.equal(run.state.phase, RUNNING);
@@ -254,15 +256,15 @@ test("cancelling before the run has a name never interrupts anything else", asyn
  const started = run.start({});

  assert.equal(await run.cancel(), true, "the cancel was refused rather than remembered");
-  assert.deepEqual(sent.filter((s) => s.url.endsWith("/interrupt")), [],
-    "an interrupt went out with no prompt id, which is a global interrupt");
+  assert.deepEqual(sent.filter((s) => s.url.includes("cancel") || s.url.endsWith("/interrupt")), [],
+    "a cancel went out with no prompt id, which ComfyUI reads as a global interrupt");

  release();
  await started;

-  const interrupts = sent.filter((s) => s.url.endsWith("/interrupt"));
-  assert.equal(interrupts.length, 1, "the cancel the user asked for never happened");
-  assert.equal(interrupts[0].body.prompt_id, "mine");
+  const cancels = sent.filter((s) => s.url.includes("/cancel"));
+  assert.equal(cancels.length, 1, "the cancel the user asked for never happened");
+  assert.equal(cancels[0].url, "/api/jobs/mine/cancel");
 });

 test("a cancel is never sent with a null prompt id, whatever the phase", async () => {
@@ -318,8 +320,7 @@ test("an adopted run can be cancelled by its own id", async () => {
  const { run, sent } = runner(ok({}));
  run.adopt("was-running");
  await run.cancel();
-  assert.equal(sent[0].url, "/interrupt");
-  assert.equal(sent[0].body.prompt_id, "was-running");
+  assert.equal(sent[0].url, "/api/jobs/was-running/cancel");
 });

 // --- a run that finishes while the page is loading ---------------------------
@@ -365,3 +366,79 @@ test("what was seen while idle does not become somebody else's run", () => {
  run.adopt("mine");
  assert.equal(run.state.phase, RUNNING, "a stranger's finish was replayed onto this run");
 });
+
+test("a run waiting behind another one can still be cancelled", async () => {
+  // The case /interrupt cannot serve. ComfyUI only consults the RUNNING half of
+  // its queue there, so a job that is merely pending gets a 200, a log line
+  // saying it was skipped, and a full generation when its turn arrives.
+  const { run, sent } = runner(ok({ prompt_id: "waiting" }));
+  await run.start({});
+  assert.equal(run.state.phase, QUEUED, "this test needs a run that has not started");
+
+  await run.cancel();
+  assert.equal(sent[1].url, "/api/jobs/waiting/cancel",
+    "a queued run was cancelled with a call that only stops a running one");
+});
+
+test("an older ComfyUI without the cancel route still interrupts", async () => {
+  const sent = [];
+  const fetchImpl = async (url, init) => {
+    sent.push({ url, body: init && init.body ? JSON.parse(init.body) : null });
+    if (url.includes("/api/jobs/")) return { ok: false, status: 404, json: async () => ({}) };
+    return { ok: true, status: 200, json: async () => ({ prompt_id: "p-1" }) };
+  };
+  const run = createRun({ fetch: fetchImpl, clientId: "c1" });
+  await run.start({});
+  run.handle({ type: "execution_start", data: { prompt_id: "p-1" } });
+
+  await run.cancel();
+  const urls = sent.map((s) => s.url);
+  assert.ok(urls.includes("/api/jobs/p-1/cancel"));
+  assert.ok(urls.includes("/interrupt"), "nothing was tried after the route was absent");
+  assert.equal(sent[sent.length - 1].body.prompt_id, "p-1",
+    "the fallback interrupted globally rather than this run");
+});
+
+test("a cancelled pending run does not sit at Queued forever", async () => {
+  // Dequeuing a job that never ran sends nothing on the socket -- there is no
+  // execution to interrupt and so no execution_interrupted. Without reading the
+  // answer, a cancel that worked leaves the UI saying Queued indefinitely.
+  const sent = [];
+  const fetchImpl = async (url, init) => {
+    sent.push(url);
+    if (url.includes("/cancel")) return { ok: true, status: 200, json: async () => ({ cancelled: true }) };
+    return { ok: true, status: 200, json: async () => ({ prompt_id: "waiting" }) };
+  };
+  const run = createRun({ fetch: fetchImpl, clientId: "c1" });
+  await run.start({});
+  await run.cancel();
+  assert.equal(run.state.phase, CANCELLED);
+});
+
+test("a running run is still left to the socket to declare stopped", async () => {
+  const fetchImpl = async (url) => ({
+    ok: true, status: 200,
+    json: async () => (url.includes("/cancel") ? { cancelled: true } : { prompt_id: "p-1" }),
+  });
+  const run = createRun({ fetch: fetchImpl, clientId: "c1" });
+  await run.start({});
+  run.handle({ type: "execution_start", data: { prompt_id: "p-1" } });
+
+  await run.cancel();
+  // Claiming it early is how a UI shows "cancelled" over a run still burning
+  // GPU time: the interrupt is a request, and the model finishes its step.
+  assert.equal(run.state.phase, RUNNING);
+  run.handle({ type: "execution_interrupted", data: { prompt_id: "p-1" } });
+  assert.equal(run.state.phase, CANCELLED);
+});
+
+test("a cancel the server says did nothing does not claim it did", async () => {
+  const fetchImpl = async (url) => ({
+    ok: true, status: 200,
+    json: async () => (url.includes("/cancel") ? { cancelled: false } : { prompt_id: "gone" }),
+  });
+  const run = createRun({ fetch: fetchImpl, clientId: "c1" });
+  await run.start({});
+  await run.cancel();
+  assert.equal(run.state.phase, QUEUED, "the run was declared cancelled on the server's word that it was not");
+});