A job collected with shell_status is still delivered, so its result arrives twice #2

Activity

1. chigkim commented on Oct 5, 2026

   

   chigkim

   on Oct 5, 2026

   Author

   More actions

   A local patch against 0.4.0 that fixes this. A finished job is held until the agent loop ends (agent_end) and only then delivered, skipping any job shell_status collected in the meantime. When no turn is running it is delivered at once, as before. Jobs adopted after /reload go through the same path.

   @@ -77,6 +77,12 @@
      // deliver through the now-stale pi handle. Each instance has its own copy
      // (reload builds a fresh closure), so this only ever flips once, on the way out.
      let handedOff = false;
   +  // Finished jobs waiting for the agent loop to end before they are delivered.
   +  // Holding them lets a shell_status call later in the same turn collect a job
   +  // and cancel its delivery: deciding at exit time always beats the poll, so
   +  // the result arrived twice (pifydev/shell-background#2).
   +  let busy = false;
   +  const pending: Job[] = [];
    
      const ROOT = join(tmpdir(), "pify-shell-bg");
      const MAX_SESSION_DIR_AGE_MS = 7 * 24 * 60 * 60 * 1000;
   @@ -229,6 +235,17 @@
        );
      }
    
   +  /** Deliver every held job that nothing collected meanwhile, unless a turn is still running. */
   +  function flushPending(): void {
   +    if (busy || handedOff) return;
   +    for (const job of pending.splice(0)) {
   +      if (job.delivered) continue;
   +      job.delivered = true;
   +      registry?.persist(job);
   +      deliver(job);
   +    }
   +  }
   +
      /** Deliver a finished background job into the conversation, once. */
      function scheduleDelivery(job: Job, exit: Promise<unknown>): void {
        exit
   @@ -242,9 +259,8 @@
              return;
            }
            if (job.delivered) return;
   -        job.delivered = true;
   -        registry?.persist(job);
   -        deliver(job);
   +        pending.push(job);
   +        flushPending();
          })
          .catch(() => {
            // A /reload can make captured handles throw; delivery is a convenience,
   @@ -614,10 +630,9 @@
              }
            }
            if (job.status === "running") anyRunning = true;
   -        else if (!job.delivered) {
   -          job.delivered = true;
   -          registry.persist(job);
   -          deliver(job);
   +        else if (!job.delivered && !pending.includes(job)) {
   +          pending.push(job);
   +          flushPending();
            }
          }
          renderWidget(ctx);
   @@ -626,6 +641,19 @@
        timer.unref?.();
      }
    
   +  pi.on("agent_start", async () => {
   +    busy = true;
   +  });
   +
   +  pi.on("agent_end", async () => {
   +    busy = false;
   +    try {
   +      flushPending();
   +    } catch {
   +      // Delivery is a convenience; shell_status still collects the result.
   +    }
   +  });
   +
      pi.on("session_start", async (_event, ctx) => {
        const warnings = loadSettings(ctx.cwd);
        const key = sessionKey(ctx);

   Tested by hand in an interactive session (pi 1.0.2, Windows, Git Bash):

   Case	Before	After
   background: true, then shell_status {id, wait}	result twice	once, from shell_status
   background: true, finishes during the turn, not collected	once	once, after the turn
   auto-backgrounded at 30s, not collected	once	once

   Not done: the package's own test suite was not run, and there is no new test. One unverified edge: pi starts a new run for messages queued from an agent_end handler (agent-session.js, "Messages queued by agent_end handlers require a fresh run"). If that run does not emit agent_start again, a job finishing during it is delivered at once, which is the old behaviour.

   Filed by claude-opus-5-5.
2. chigkim

   added a commit that references this issue on Oct 8, 2026

   fix(shell-background): a job collected with shell_status is no longer…

   ...

   This issue will close once commit e512c60 is merged into the 'main' branch.

   e512c60