publish: a retry after publish-timeout pushes once; the bus holds the decision until its push ends and records a late push (row 242, #1748)

agents/megavolt/work/queue-242/BUILD.md

@@ -0,0 +1,105 @@
+# Row 242 build: a retry after publish-timeout pushes once
+
+Row 242, #1748, P19 440. Brief: docs/plans/2026-10-20_agent-credentials.md, "Brief: row 237
+follow-up N1, a retry after publish-timeout pushes once" (9d165be8c). Base: origin/refactor
+8fede5fc6 (row 210 landed f5753c0e8).
+
+## The defect
+
+The git socket polls publishResult until publishDeadlineMs, then refuses publish-timeout
+(packages/cli/src/git-socket.mjs:433) while the bus process's push (runtime publishPush) can
+still be running. The socket then calls publishRefused, which went straight to
+broker.publishRefused: it filed the refusal and deleted the ticket. checkPublish no longer saw an
+open ticket on the decision, so a retry got a new ticket and pushed a second ref while the first
+push was still on its way to the fork.
+
+## Fix chosen: hold the decision until the push ends, then record a push that reached the fork
+
+packages/bus/src/runtime.mjs, `publishRefused` and `settle`:
+
+- If the bus has no push for the ticket, publishRefused goes to the broker as before.
+- If the push is still running, the refusal code is held on the entry and the ticket stays open.
+  checkPublish refuses another publish on the decision with publish-busy (row 237's check),
+  the same as when the socket is still polling.
+- When the push ends, `settle` runs:
+  - A push that reached the fork is recorded with broker.recordPublish, which spends the
+    decision. The bus logs `pushed after the git socket refused <code>; recorded`.
+  - A push that failed is filed as publishRefused with the socket's code.
+  - If recordPublish refuses with a BusError, that code is filed instead.
+- If the push has already ended when the refusal arrives, it's settled at once. The sage-1440 test
+  relies on this: its push fails fast, so publish-timeout is filed once, immediately, as before.
+
+Why hold rather than kill. Killing the git client doesn't undo a push the fork has already
+received: receive-pack can update the ref after the client dies. After a kill the bus would have
+to ls-remote the fork to learn what landed, and the result could still race the server. Holding
+waits for the push's own outcome. Each git run in publishRemote already has its own limit
+(publishRemote defaults to checkout.mjs GIT_LIMITS), so a hold is bounded. Holding also reuses the existing publish-busy
+state, and the refusal text is unchanged.
+
+Why record and not only hold. If the hold just filed publish-timeout once the push ended, the
+ticket would close and the decision would be free again with a ref on the fork. A retry would then
+push a second ref. Recording the late push spends the decision, so the retry gets
+decision-consumed.
+
+The publish-timeout refusal text is unchanged (brief condition).
+
+## Test
+
+packages/cli/tests/publish-socket.test.mjs:302, "row 242: a retry after publish-timeout refuses
+publish-busy while the bus process's push runs; the late push is recorded and the decision pushes
+once".
+
+- publishDeadlineMs 150 and a store token. The fork's pre-receive hook writes `held`, then loops
+  until the test writes `release`.
+- The first publish gives publish-timeout. The test waits for `held`.
+- A retry to branch `other` gives publish-busy (:322). Refused events: ["publish-busy"]. No
+  git.push.remote allowed event.
+- The test writes `release` and waits until there is exactly one allowed event, with ref
+  refs/heads/coder/feat and the work commit.
+- The next retry gives decision-consumed (:330).
+- Exactly one git-receive-pack request reached the fake Gitea, and the fork has only
+  refs/heads/coder/feat. Refused events: ["publish-busy", "decision-consumed"]. No TOKEN appears in
+  events or logs.
+
+Fail fast (sage-1681): the body is wrapped in try/finally, and the finally writes `release`. Under
+the first mutant run, the assert at the retry failed and the hook was never released. setup's
+teardown waits for the fake Gitea, which waited for the held push, so the file hung until I stopped
+it. Now a failed assert ends the hook and the run. Each wait is bounded at 10 s with an assert.fail.
+
+## Mutants
+
+Script: /mnt/storage/scratch/megavolt-r242/mutants.sh. Each mutant: save, one-line sed, run
+publish-socket.test.mjs whole, restore, cmp.
+
+| Mutant | Change | Expected kill |
+|---|---|---|
+| M1 | export `publishRefused: (ticket, args) => broker.publishRefused(ticket, args)` again (removes the fix) | :302 at :322: the retry gets a new ticket and its own push, so publish-timeout, not publish-busy (seen in the stopped 03:06Z run) |
+| M2 | `settle` files publishRefused without trying recordPublish (hold only) | :302 at the wait for one allowed event, then :330 |
+
+Results (Sage's slot, sage-1686, 2026-10-21T03:35:00Z to 03:35:15Z): both killed, both restored
+(cmp ok).
+
+- M1: publish-socket 8/1, the row 242 test fails at :322 in 0.5 s.
+- M2: publish-socket 8/1, the row 242 test fails at :327 (the bounded wait at :311 for one allowed
+  event) in 10.3 s.
+
+## Suites
+
+On Sage's slot (sage-1686), on base 8fede5fc6 plus this row:
+
+- bus `node --test tests/*.test.mjs`: 499/499, 03:35:20Z to 03:35:34Z. This is one fewer than
+  Sage's 241 gate (500) because the base predates row 241's land.test case.
+- cli `node --test tests/*.test.mjs`: 566/566, 03:35:34Z to 03:43:47Z. That's 565 plus the
+  row 242 test.
+
+Singles before the slot: publish-socket.test.mjs 9/9, bus publish.test.mjs 13/13. The seat suite
+and scripts/test-*.sh are left for Sage's landing gate.
+
+## Residuals
+
+- The agent was told publish-timeout, but the push is recorded later and the run isn't
+  notified. The allowed event and the bus log line are the only trace. A later publish on the
+  decision gets decision-consumed, which tells the agent the decision was used. Whether the run
+  should get a RESULT for a late record is a follow-up question for Sage.
+- The hold lives in the bus process's memory, like the broker's open tickets. A bus restart during
+  a hold drops both, the same as today for any publish in flight.

packages/bus/src/runtime.mjs

@@ -257,9 +257,42 @@ export async function startBroker({
           if (!(e instanceof BusError)) process.stderr.write(`bus: publish git-failed: ${JSON.stringify({ name: e?.name, code: e?.code })}\n`);
           Object.assign(entry, { state: 'refused', code: e instanceof BusError ? e.code : 'git-failed' });
         },
-      );
+      ).then(() => {
+        if (entry.held) settle(ticket, entry, entry.held);
+      });
       return { state: 'running' };
     }
+    // Row 242 (#1748): the git socket gave up on a publish (publish-timeout) whose push it never
+    // collected. Its publishRefused waits for the push: until that ends the ticket stays open, so
+    // checkPublish refuses another publish on the decision with publish-busy (row 237). A push that
+    // reached the fork is then recorded, which spends the decision; one that didn't is filed with
+    // the socket's code. Either way the decision can't push a second ref.
+    function settle(ticket, entry, code) {
+      remotes.delete(ticket);
+      try {
+        if (entry.state === 'pushed') {
+          try {
+            broker.recordPublish(ticket, { ref: entry.ref, commit: entry.commit, previous: entry.previous });
+            process.stderr.write(`bus: publish ${ticket}: pushed after the git socket refused ${code}; recorded\n`);
+            return;
+          } catch (e) {
+            if (!(e instanceof BusError)) throw e;
+            code = e.code;
+          }
+        }
+        broker.publishRefused(ticket, { code });
+      } catch (e) {
+        process.stderr.write(`bus: publish ${ticket}: not settled: ${JSON.stringify({ name: e?.name, code: e?.code })}\n`);
+      }
+    }
+    function publishRefused(ticket, args) {
+      const entry = remotes.get(ticket);
+      if (!entry) return broker.publishRefused(ticket, args);
+      if (!/^[a-z-]{1,64}$/.test(args?.code ?? '')) throw new BusError('invalid-request');
+      if (entry.state === 'running') entry.held = args.code;
+      else settle(ticket, entry, args.code);
+      return args.code;
+    }
     function publishResult(ticket) {
       const entry = remotes.get(ticket);
       if (!entry) throw new BusError('publish-unknown');
@@ -296,7 +329,7 @@ export async function startBroker({
       publishPush,
       publishResult,
       recordPublish: (ticket, pushed) => broker.recordPublish(ticket, pushed),
-      publishRefused: (ticket, args) => broker.publishRefused(ticket, args),
+      publishRefused,
       landHeld: (record) => broker.landHeld(record),
       // Row 156 (#1651): the launcher's landing pass.
       landHolders: (business) => broker.landHolders(business),

packages/cli/tests/publish-socket.test.mjs

@@ -5,7 +5,7 @@
 import { test } from "node:test";
 import assert from "node:assert/strict";
 import { spawnSync } from "node:child_process";
-import { chmodSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { chmodSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
 import { connect } from "node:net";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
@@ -298,3 +298,41 @@ test("sage-1440: the publishResult poll is bounded: publish-timeout, filed once,
   assert.equal(w.events("action.allowed").filter((e) => e.body.action === "git.push.remote" && e.body.operation !== "decision.raise").length, 0);
   assert.ok(w.logs.some((l) => l.includes("publish-timeout")));
 });
+
+test("row 242: a retry after publish-timeout refuses publish-busy while the bus process's push runs; the late push is recorded and the decision pushes once", async (t) => {
+  const w = await setup(t, { publishDeadlineMs: 150 });
+  w.bus.credentials.issue("demo/coder", "gitea", TOKEN, "2099-01-01");
+  // The fork holds the push in pre-receive until the test writes `release`; `held` says it's there.
+  const held = join(w.root, "held");
+  const release = join(w.root, "release");
+  writeFileSync(join(w.forkRepo, "hooks", "pre-receive"), `#!/bin/sh\n: > '${held}'\nwhile [ ! -e '${release}' ]; do sleep 0.05; done\ncat > /dev/null\n`, { mode: 0o755 });
+  const until = async (what, ok) => {
+    for (const end = Date.now() + 10_000; !ok(); ) {
+      if (Date.now() > end) assert.fail(`timed out waiting for ${what}`);
+      await new Promise((r) => setTimeout(r, 20));
+    }
+  };
+  const allowed = () => w.events("action.allowed").filter((e) => e.body.action === "git.push.remote" && e.body.operation !== "decision.raise");
+  // A failed assert must still let the hook end: teardown waits for the fake Gitea, which waits for the push.
+  try {
+    const d = w.approve();
+    assert.deepEqual(await w.publish(d), { ok: false, error: "publish-timeout" });
+    await until("the push to reach pre-receive", () => existsSync(held));
+    // The socket gave up, but the push still runs: the decision is held, and nothing is filed yet.
+    assert.deepEqual(await w.publish(d, { branch: "other" }), { ok: false, error: "publish-busy" });
+    assert.deepEqual(w.events("action.refused").map((e) => e.body.code), ["publish-busy"]);
+    assert.equal(allowed().length, 0);
+    writeFileSync(release, "");
+    // The push reached the fork, so it's recorded, which spends the decision.
+    await until("the late push to be recorded", () => allowed().length === 1);
+    assert.equal(allowed()[0].body.ref, "refs/heads/coder/feat");
+    assert.equal(allowed()[0].body.commit, w.work);
+    assert.deepEqual(await w.publish(d, { branch: "other" }), { ok: false, error: "decision-consumed" });
+    assert.equal(w.gitea.requests.filter((r) => r.path.endsWith("/git-receive-pack")).length, 1);
+    assert.equal(git(w.root, `--git-dir=${w.forkRepo}`, "for-each-ref", "--format=%(refname)", "refs/heads/coder/"), "refs/heads/coder/feat");
+    assert.deepEqual(w.events("action.refused").map((e) => e.body.code), ["publish-busy", "decision-consumed"]);
+  } finally {
+    writeFileSync(release, "");
+  }
+  assert.equal(JSON.stringify([w.events("action.allowed"), w.events("action.refused"), w.logs]).includes(TOKEN), false);
+});