From 7109e39e006d8e5dc8a5d0c9672250cd5d02b9f7 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Tue, 13 Oct 2026 17:50:20 -0500 Subject: [PATCH] work: Darkwing row 106 round 1 packet (#1600) Bus host lock follow-ups: release on a fork or send throw, retry a lock that vanishes before the read, the have order test fixed to directory order, the third-racer README sentence and the reviewer wrap. Gate passed on 0069949ff; 9 mutant runs as expected. Co-Authored-By: Claude Opus 5.5 --- agents/darkwing/work/queue-106/r1/build.md | 88 ++++++ agents/darkwing/work/queue-106/r1/build.patch | 254 ++++++++++++++++++ .../queue-106/r1/candidate-manifest.sha256 | 5 + .../darkwing/work/queue-106/r1/gate-run.log | 25 ++ agents/darkwing/work/queue-106/r1/gate.sh | 20 ++ .../work/queue-106/r1/mutants-run.log | 12 + agents/darkwing/work/queue-106/r1/mutants.py | 126 +++++++++ 7 files changed, 530 insertions(+) create mode 100644 agents/darkwing/work/queue-106/r1/build.md create mode 100644 agents/darkwing/work/queue-106/r1/build.patch create mode 100644 agents/darkwing/work/queue-106/r1/candidate-manifest.sha256 create mode 100644 agents/darkwing/work/queue-106/r1/gate-run.log create mode 100644 agents/darkwing/work/queue-106/r1/gate.sh create mode 100644 agents/darkwing/work/queue-106/r1/mutants-run.log create mode 100644 agents/darkwing/work/queue-106/r1/mutants.py diff --git a/agents/darkwing/work/queue-106/r1/build.md b/agents/darkwing/work/queue-106/r1/build.md new file mode 100644 index 000000000..b7fe3f5b7 --- /dev/null +++ b/agents/darkwing/work/queue-106/r1/build.md @@ -0,0 +1,88 @@ +# Row 106 (#1600) round 1: bus host lock follow-ups + +Brief: docs/plans/2026-10-13_landing-follow-ups.md, section "Bus host lock: release on a late throw, a vanishing lock, the third racer" (last changed in aedc5db8f; blob 001cba498 at f00613106). +BASE: 0069949ffc78fa8d4998243a81fd7efe85279442. That is row 104's landed commit. I built on f00613106 and moved the candidate onto BASE before the gate, as Sage asked. The only commit between the two is row 104's, which touches none of the 5 manifest paths, so the diff is byte-identical. The gate and the mutants ran on BASE. Candidate: build.patch against BASE. candidate-manifest.sha256 lists the 5 files, and applying the patch to a fresh worktree at BASE reproduces every digest in it. + +Per Sage, the third-racer sentence is in packages/cli/README.md, and packages/seat/README.md is untouched. + +## What changed + +- packages/cli/src/host.mjs, `startHost` (Sage N1). The broker's fork, its boot send and the wait for its reply now all run in one `try`. Its failure path kills the broker if the fork returned one, waits for it to end, and calls `release()`. Before, the fork and the send ran outside the `try`, so a throw from either left `host.lock` behind until a later start took it over as stale. The wait now starts after the send rather than before it. A send that throws therefore leaves no pending wait whose rejection nobody handles. The reply and the exit event come on a later tick, so the wait still misses neither. The notifier's fork and send are unchanged; the brief covers only the time before the broker's reply. +- packages/cli/src/host.mjs, `readState` (Dewey a). An ENOENT from the read returns null, which `claimHostLock` already treats as "try again" within `CLAIM_TRIES`. Any other read or parse failure still refuses with `bus host state is unreadable: `. A new `found` hook runs between the existence check and the read, for the tests only. `claimHostLock` takes `found` next to `between` and passes it on. `readHostState` and `release()` read through `readState` too: a `host.json` that goes between the check and the read now counts as no host, and `release()` already ignored a lock it couldn't read. +- packages/harness/tests/git-tools.test.mjs, the `have` order test (Dewey b). It makes the two clone directories, reads their order with `readdirSync`, and gives the first-listed clone the older refs. It checks that the order holds once the clones exist, then expects the first-listed clone's 20 refs and then 12 of the other's. The old test took the order from the request, so it passed whatever clone order the code used. +- packages/cli/README.md, step 3 of the host start (Dewey c). One new sentence: of three starts racing one stale lock, two can both claim it, and then the broker's `writer.lock` refuses the second broker. +- roles/reviewer.md (Dewey d). The `nothing-to-fetch` line wraps after "has the commit;" like its neighbours. The words are the same. + +## Tests + +| File | Base | Candidate | New | +|---|---|---|---| +| packages/cli/tests/host.test.mjs | 28 | 31 | `a broker fork that throws leaves no host.lock and no host.json`, `a broker send that throws ...` (one loop), `a host.lock that goes between the check and the read is tried again; one that can't be parsed still refuses as unreadable` | +| packages/harness/tests/git-tools.test.mjs | 4 | 4 | the `have` order test, rewritten | + +The fork and send tests use a test helper, `throwOn`. It hooks the `send` property that node installs on each new child, which `spySends` and `failSends` in the same file already do. Throwing from that install makes `fork` itself throw, after the child has started; throwing from the installed `send` makes the boot send throw. The helper kills every child it saw after the test, the one the throwing fork started included. + +The ENOENT test writes a stale lock, and the `found` hook removes it the first time it runs. The claim then succeeds on the next try, and the hook ran once. The same test then writes `{not json` as the lock and expects the unreadable refusal, with the file left as it was. + +## Mutants + +mutants.py and gate.sh are the scripts I ran, copied here; they point at my scratch worktree, so change `WT`/`G` to rerun them. BASE_TEST in mutants.py reads the test file from the worktree's HEAD, which must be BASE. mutants.py runs each mutant against the named tests only, through `--test-name-pattern`, and counts a pattern that matches no test as a fault. Each file is restored from its `.orig`, and the candidate's diff hashed 60246b4202f67281 (first 16 hex) before and after. Run 22:49:00 to 22:49:03Z on 2026-10-13. + +| Id | Mutant | Test | Result | +|---|---|---|---| +| n01 | the fork runs before the `try` | fork throws | killed | +| n02 | the send runs between two `try`s | send throws | killed | +| n03 | fork and send both before the `try`, as at BASE | fork throws, send throws | killed | +| n04 | no ENOENT case in `readState` | lock goes between check and read | killed | +| n05 | every read failure counts as no lock | lock goes between check and read | killed | +| n06 | `have` takes the clones in reverse directory order | `have` order | killed | +| n07 | `have` sorts the working refs by date across clones | `have` order | killed | +| x01 | control: BASE's `have` test against n06 | `have` order | survived, as expected | +| x02 | control: BASE's `have` test against n07 | `have` order | survived, as expected | + +Every run matched exactly 1 test, and every kill is an assertion failure. In n01 to n03, `host.lock` is still there after the throw. n04 refuses with `bus host state is unreadable` on the lock that went away. n05 is the check that an unparseable lock still refuses: with every read failure as null, the `{not json` claim loops to "kept changing" instead. + +The controls are expected to survive. They correct part of the brief's finding b. The brief says the old test tells per-clone order from a cross-clone sort only when `alpha` lists first. On this ext4, `alpha` lists first, and the old test still passes both n06 and n07. It reads which clone came first from `req.have[0]` and builds its expectation from that, so any order of whole clones passes. The new test fixes the order before the request. + +## Gate + +gate.sh, on BASE 0069949ff, 22:43:03 to 22:48:53Z on 2026-10-13, DOCKER_HOST unset, Docker 29.7.2. + +| Step | Result | Time (UTC) | +|---|---|---| +| pkg-bus | 128 passed, 0 failed | 22:43:03-22:43:06 | +| pkg-business | 67 passed, 0 failed | 22:43:06-22:43:09 | +| pkg-cli | 168 passed, 0 failed | 22:43:09-22:43:33 | +| pkg-control-board | 124 passed, 0 failed | 22:43:33-22:43:34 | +| pkg-conversation | 182 passed, 0 failed | 22:43:34-22:44:24 | +| pkg-discord | 178 passed, 0 failed | 22:44:24-22:44:27 | +| pkg-harness | 73 passed, 0 failed | 22:44:27-22:44:37 | +| pkg-ledger | 78 passed, 0 failed | 22:44:37-22:44:58 | +| pkg-mosaic | 69 passed, 0 failed | 22:44:58-22:45:00 | +| pkg-queue | 156 passed, 0 failed | 22:45:00-22:45:34 | +| pkg-runs | 43 passed, 0 failed | 22:45:34-22:45:35 | +| pkg-seat | 119 passed, 0 failed | 22:45:35-22:45:40 | +| pkg-tasks | 92 passed, 0 failed | 22:45:40-22:45:45 | +| pkg-webui | 108 passed, 0 failed | 22:45:45-22:46:41 | +| script-auth | 15 passed, 0 failed | 22:46:41-22:46:42 | +| script-conductor | 17 passed, 0 failed | 22:46:42-22:46:47 | +| script-config | 24 passed, 0 failed | 22:46:47-22:46:49 | +| script-discord | 66 passed, 0 failed | 22:46:49-22:46:56 | +| script-extension-package | 18 passed, 0 failed | 22:46:56-22:46:57 | +| script-foundation | 44 passed, 0 failed | 22:46:57-22:47:38 | +| script-queue | 28 passed, 0 failed (node 156 passed) | 22:47:38-22:48:12 | +| script-release | 14 passed, 0 failed | 22:48:12-22:48:24 | +| script-task | 98 passed, 0 failed | 22:48:24-22:48:53 | + +The gate printed GATE_PASSED at 22:48:53Z. The Docker steps left a `wt_default` network, created at 22:48:12Z with 0 containers; I removed it. + +## Choices and limits + +- The wait for the boot reply now starts after the send. The other order would need a `.catch` on a wait that a throwing send abandons. Node emits the child's `message` and `exit` events from the event loop, and the wait attaches its listeners in the same synchronous run as the send, so nothing comes in between. +- A fork that throws after node has started the child returns no handle, so `startHost` can't kill that child. The test kills it. I don't know of a real path where node's `fork` throws after the spawn; this is what the hook produces. +- An asynchronous send failure, an `error` event on the broker child, still has no listener in `startHost` and would end the process before `release()`. That was so at BASE, and the brief names only throws. A bounded follow-up if wanted. +- The README sentence goes in step 3, after the takeover it qualifies. The existing sentence after it is unchanged. + +## Going live (decision 109) + +The host change is live only with a new copy, a re-pin and a bus restart, all under Mos's go. Until then the live bus, Console and CLI run host.mjs from the pinned copy at 465bbdfbb. The tests and the README change nothing that runs. The launcher reads a role's contract at each launch, from `MOSAIC_ROLES_DIR` or else the `roles/` of the copy the bus runs from (packages/cli/src/config.mjs, `rolesDir`). So the reviewer wrap reaches a reviewer session only once that copy has it, and it changes the line breaks in the prompt only. diff --git a/agents/darkwing/work/queue-106/r1/build.patch b/agents/darkwing/work/queue-106/r1/build.patch new file mode 100644 index 000000000..0d8b76c06 --- /dev/null +++ b/agents/darkwing/work/queue-106/r1/build.patch @@ -0,0 +1,254 @@ +diff --git a/packages/cli/README.md b/packages/cli/README.md +index 5baebde27..3111fe03d 100644 +--- a/packages/cli/README.md ++++ b/packages/cli/README.md +@@ -181,8 +181,10 @@ mosaic bus status [--json] host state file, socket, writer.lock + (pid )`. A lock whose host no longer runs is taken over + whatever its age: it is moved aside, checked to be the file that was + read, and removed; if another host's claim was moved aside in that race, +- it goes back. Each refusal below removes `host.lock` again, so a host +- that didn't start leaves no file. ++ it goes back. Of three starts racing one stale lock, two can both claim ++ it, and then the broker's `writer.lock` refuses the second broker. Each ++ refusal below removes `host.lock` again, so a host that didn't start ++ leaves no file. + 4. It forks `packages/bus/src/process.mjs` and waits up to 30 s for the boot + reply. If the broker answers `{ok: false, error}`, the host exits 3 and + prints `broker refused to start: `. A timeout or an early exit +diff --git a/packages/cli/src/host.mjs b/packages/cli/src/host.mjs +index 23150d4c1..02b15477d 100644 +--- a/packages/cli/src/host.mjs ++++ b/packages/cli/src/host.mjs +@@ -44,12 +44,16 @@ export function readHostState(dataRoot) { + return readState(hostFile(dataRoot)); + } + +-function readState(file) { ++// A file that goes between the check and the read counts as no file. ++// `found` runs between the two; it is for the tests. ++function readState(file, found = () => {}) { + if (!existsSync(file)) return null; ++ found(); + let state; + try { + state = JSON.parse(readFileSync(file, "utf8")); +- } catch { ++ } catch (e) { ++ if (e.code === "ENOENT") return null; + throw new CliError(`bus host state is unreadable: ${file}`, 3); + } + if (!state || !Number.isSafeInteger(state.pid) || typeof state.startTime !== "string" || typeof state.business !== "string") { +@@ -68,9 +72,11 @@ const same = (a, b) => a?.pid === b?.pid && a?.startTime === b?.startTime && a?. + // moved aside, checked to be the file that was read, and removed; another + // host's claim moved aside by that race goes back. A third host racing the + // same stale lock can still lose a claim there, and then the broker's writer +-// lock refuses the second broker. `between` runs after the stale lock is +-// read; it is for the tests. Exported for the tests. +-export function claimHostLock(dataRoot, state, { between = () => {} } = {}) { ++// lock refuses the second broker. A lock that goes before it is read is ++// tried again, like one that goes before the check. `found` runs once the ++// lock is found and before it is read, `between` after the stale lock is ++// read; both are for the tests. Exported for the tests. ++export function claimHostLock(dataRoot, state, { found = () => {}, between = () => {} } = {}) { + mkdirSync(hostDir(dataRoot), { recursive: true, mode: 0o700 }); + const file = hostLock(dataRoot); + const tag = `${process.pid}.${randomBytes(4).toString("hex")}`; +@@ -85,7 +91,7 @@ export function claimHostLock(dataRoot, state, { between = () => {} } = {}) { + } catch (e) { + if (e.code !== "EEXIST") throw e; + } +- const prior = readState(file); ++ const prior = readState(file, found); + if (!prior) continue; + if (prior.live) throw new CliError(`a bus host already runs for ${prior.business} (pid ${prior.pid})`, 3); + between(); +@@ -177,15 +183,20 @@ export async function startHost({ boot, business, notifier = null, bootTimeoutMs + if (same(held, state)) rmSync(hostLock(dataRoot), { force: true }); + }; + +- const broker = fork(BROKER, [], { stdio: ["ignore", "inherit", "inherit", "ipc"] }); +- const reply = firstReply(broker, bootTimeoutMs, "broker"); +- broker.send({ op: "boot", config: boot }); ++ // A fork or a send that throws gives the lock back too. The wait starts ++ // after the send, so a send that throws leaves no wait behind. The reply ++ // and the exit event come on a later tick, so the wait misses neither. ++ let broker = null; + let ready; + try { +- ready = await reply; ++ broker = fork(BROKER, [], { stdio: ["ignore", "inherit", "inherit", "ipc"] }); ++ broker.send({ op: "boot", config: boot }); ++ ready = await firstReply(broker, bootTimeoutMs, "broker"); + } catch (e) { +- broker.kill("SIGTERM"); +- await ended(broker, 5000); ++ if (broker) { ++ broker.kill("SIGTERM"); ++ await ended(broker, 5000); ++ } + release(); + throw e; + } +diff --git a/packages/cli/tests/host.test.mjs b/packages/cli/tests/host.test.mjs +index 7dc05a7f9..1b55505c6 100644 +--- a/packages/cli/tests/host.test.mjs ++++ b/packages/cli/tests/host.test.mjs +@@ -4,7 +4,7 @@ import { spawn, spawnSync } from "node:child_process"; + import { subscribe, unsubscribe } from "node:diagnostics_channel"; + import { createServer } from "node:http"; + import { once } from "node:events"; +-import { chmodSync, existsSync, readFileSync, readdirSync, statSync, symlinkSync, utimesSync, writeFileSync, mkdirSync } from "node:fs"; ++import { chmodSync, existsSync, readFileSync, readdirSync, rmSync, statSync, symlinkSync, utimesSync, writeFileSync, mkdirSync } from "node:fs"; + import { dirname, join } from "node:path"; + import { fileURLToPath } from "node:url"; + import { Client } from "../../bus/src/client.mjs"; +@@ -108,6 +108,33 @@ function failSends(t, fake) { + }); + } + ++// Makes the next fork throw (`where` "fork") or the new child's sends throw ++// ("send"), through the `send` that node installs on a new child: a fork ++// throws when the install does. Every child it saw is killed after the test, ++// the one a throwing fork started included. ++function throwOn(t, where) { ++ const children = []; ++ const onChild = ({ process: child }) => { ++ children.push(child); ++ let send; ++ Object.defineProperty(child, "send", { ++ configurable: true, ++ get: () => send, ++ set() { ++ if (where === "fork") throw new Error("fork failed"); ++ send = () => { ++ throw new Error("send failed"); ++ }; ++ }, ++ }); ++ }; ++ subscribe("child_process", onChild); ++ t.after(() => { ++ unsubscribe("child_process", onChild); ++ for (const child of children) child.kill("SIGKILL"); ++ }); ++} ++ + const epipe = (child) => { + child.kill("SIGKILL"); + return Object.assign(new Error("write EPIPE"), { code: "EPIPE", errno: -32, syscall: "write" }); +@@ -259,6 +286,27 @@ test("a host takes over a host.lock its dead host left, and puts back a live cla + assert.deepEqual(left(), ["host.lock"]); + }); + ++// Row 106 (#1600, Dewey a): a lock removed after the check and before the ++// read is tried again, within CLAIM_TRIES, and never refused as unreadable. ++test("a host.lock that goes between the check and the read is tried again; one that can't be parsed still refuses as unreadable", (t) => { ++ const dataRoot = tmp(t); ++ mkdirSync(hostDir(dataRoot), { recursive: true, mode: 0o700 }); ++ const stale = { pid: 999999999, startTime: "1", business: "acme", startedAt: "2026-10-13T00:00:00.000Z" }; ++ const mine = { hostVersion: 1, pid: process.pid, startTime: startTimeOf(process.pid), business: "acme", startedAt: "2026-10-13T00:00:01.000Z", notifier: null }; ++ writeFileSync(hostLock(dataRoot), JSON.stringify(stale), { mode: 0o600 }); ++ let found = 0; ++ const gone = () => found++ === 0 && rmSync(hostLock(dataRoot)); ++ claimHostLock(dataRoot, mine, { found: gone }); ++ assert.equal(found, 1, "the lock was found once, and gone before the read"); ++ assert.equal(JSON.parse(readFileSync(hostLock(dataRoot), "utf8")).startedAt, mine.startedAt); ++ assert.deepEqual(readdirSync(hostDir(dataRoot)), ["host.lock"]); ++ ++ writeFileSync(hostLock(dataRoot), "{not json", { mode: 0o600 }); ++ assert.throws(() => claimHostLock(dataRoot, mine), (e) => e.exitCode === 3 && e.message === `bus host state is unreadable: ${hostLock(dataRoot)}`); ++ assert.equal(readFileSync(hostLock(dataRoot), "utf8"), "{not json"); ++ assert.deepEqual(readdirSync(hostDir(dataRoot)), ["host.lock"]); ++}); ++ + test("a live host's lock stays, however old the file is", (t) => { + const dataRoot = tmp(t); + mkdirSync(hostDir(dataRoot), { recursive: true, mode: 0o700 }); +@@ -330,6 +378,22 @@ test("a host started before host.lock existed, holding only host.json, still ref + assert.deepEqual(readdirSync(hostDir(f.dataRoot)), ["host.json"]); + }); + ++// Row 106 (#1600, Sage N1): a throw after the claim and before the boot ++// reply gives host.lock back like a refusal does. ++for (const where of ["fork", "send"]) ++ test(`a broker ${where} that throws leaves no host.lock and no host.json`, async (t) => { ++ const root = tmp(t); ++ const f = fixture(root); ++ makeDeployment(root); ++ const boot = bootConfig({ system: loadSystem({ env: f.env }), businessId: "acme", env: f.env }); ++ throwOn(t, where); ++ const started = startHost({ boot, business: "acme", log: () => {} }); ++ t.after(async () => (await started.catch(() => null))?.close(0)); ++ await assert.rejects(started, (e) => e.message === `${where} failed`); ++ assert.equal(existsSync(hostLock(f.dataRoot)), false); ++ assert.equal(existsSync(hostFile(f.dataRoot)), false); ++ }); ++ + test("a broker that misses its boot wait leaves no host.lock and no host.json", async (t) => { + const root = tmp(t); + const f = fixture(root); +diff --git a/packages/harness/tests/git-tools.test.mjs b/packages/harness/tests/git-tools.test.mjs +index 2880b76f0..0e3bdffe9 100644 +--- a/packages/harness/tests/git-tools.test.mjs ++++ b/packages/harness/tests/git-tools.test.mjs +@@ -5,7 +5,7 @@ + import { test } from "node:test"; + import assert from "node:assert/strict"; + import { spawnSync } from "node:child_process"; +-import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; ++import { existsSync, mkdirSync, readFileSync, readdirSync, writeFileSync } from "node:fs"; + import { createServer } from "node:net"; + import { join } from "node:path"; + import { fetchReview, git as runnerGit, HAVE_MAX } from "../src/git-tools.mjs"; +@@ -82,23 +82,30 @@ test("fetch_review's have lists the working refs before origin's, and no more th + // Past HAVE_MAX the order is per clone: each clone's working refs newest + // first, one clone after another, so an earlier clone's older refs come + // ahead of a later clone's newer ones. The clones' order is the workspace's +-// directory order, so the test takes it from the request. ++// directory order, so the test reads it first and gives the first-listed ++// clone the older refs: one order across clones by date would put the ++// other clone first, whatever order the filesystem lists them in (row 106, ++// #1600, Dewey b). + test("fetch_review's have orders the working refs newest first within each clone, one clone after another", async (t) => { + const dir = scratch(t); + const workspace = join(dir, "ws"); +- // 20 working refs in each clone; beta's are all newer than alpha's. +- // Created in a shuffled order, so the name order isn't the date order. ++ mkdirSync(join(workspace, "alpha"), { recursive: true }); ++ mkdirSync(join(workspace, "beta")); ++ const [early, late] = readdirSync(workspace); ++ // 20 working refs in each clone; the late clone's are all newer than the ++ // early one's. Created in a shuffled order, so the name order isn't the ++ // date order. + const order = [7, 2, 15, 0, 11, 19, 4, 9, 13, 1, 17, 6, 3, 18, 10, 5, 14, 8, 16, 12]; + const refs = (task, base) => Object.fromEntries(order.map((i) => [`refs/remotes/working/work/vikunja-${task}-${i + 1}`, base + i * 60])); +- const alpha = clone(join(workspace, "alpha"), refs(1, 1_700_000_000)); +- const beta = clone(join(workspace, "beta"), refs(2, 1_750_000_000)); ++ const older = clone(join(workspace, early), refs(1, 1_700_000_000)); ++ const newer = clone(join(workspace, late), refs(2, 1_750_000_000)); ++ assert.deepEqual(readdirSync(workspace), [early, late], "the clones kept their directory order"); + const host = await fakeHost(t, dir); + await assert.rejects(fetchReview({ path: host.path, cap: "c".repeat(64), workspace, task_ref: "vikunja:1/2", request: "r1", tmp: dir, env: ENV }), (e) => e.code === "request-not-found"); + const [req] = host.lines; + // Newest first: the ref for i = 19 first, i = 0 last. + const newest = (oids, task) => order.slice().sort((x, y) => y - x).map((i) => oids[`refs/remotes/working/work/vikunja-${task}-${i + 1}`]); +- const [first, second] = new Set(Object.values(alpha)).has(req.have[0]) ? [newest(alpha, 1), newest(beta, 2)] : [newest(beta, 2), newest(alpha, 1)]; +- assert.deepEqual(req.have, [...first, ...second].slice(0, HAVE_MAX)); ++ assert.deepEqual(req.have, [...newest(older, 1), ...newest(newer, 2)].slice(0, HAVE_MAX)); + }); + + // A git on PATH that runs `body` in place of the real one. +diff --git a/roles/reviewer.md b/roles/reviewer.md +index 782aa156e..46211299f 100644 +--- a/roles/reviewer.md ++++ b/roles/reviewer.md +@@ -9,7 +9,8 @@ candidates and record a verdict. You don't change the code you review. + acceptance criteria. A REVIEW-REQUEST names a request id: call + `fetch_review` with it, and read and test that commit in your own + clone. Run the suites yourself when the request says they pass. +-- A `nothing-to-fetch` refusal means a clone of yours has the commit; see "Git socket" in `packages/cli/README.md`. ++- A `nothing-to-fetch` refusal means a clone of yours has the commit; ++ see "Git socket" in `packages/cli/README.md`. + - Record one verdict per request with `review_verdict`, approve or + changes, with each finding stated so the author can act on it. The bus + takes one verdict per request; a changed mind needs a new request. diff --git a/agents/darkwing/work/queue-106/r1/candidate-manifest.sha256 b/agents/darkwing/work/queue-106/r1/candidate-manifest.sha256 new file mode 100644 index 000000000..a41eb6d14 --- /dev/null +++ b/agents/darkwing/work/queue-106/r1/candidate-manifest.sha256 @@ -0,0 +1,5 @@ +622276e54f63171de3a3f761920878492b2eae93c5d0c29502a78c1ecc87e1b3 packages/cli/README.md +501d1e10d64791ecd0560742a6ca19581b8c1f591f16783d04a243db42752c7e packages/cli/src/host.mjs +8190140ea4b7518311ce21bed90a7268e200fd4c4305180689d250b34b35850e packages/cli/tests/host.test.mjs +d857d308205fefe9b4b68e74e1164727b5f5b54678f35ff0e9840300cc36786b packages/harness/tests/git-tools.test.mjs +51f2a795d7b37479c47cb2c7385a3d76e60bc8c9f40377c8f3eb75fc25a123ed roles/reviewer.md diff --git a/agents/darkwing/work/queue-106/r1/gate-run.log b/agents/darkwing/work/queue-106/r1/gate-run.log new file mode 100644 index 000000000..61958e788 --- /dev/null +++ b/agents/darkwing/work/queue-106/r1/gate-run.log @@ -0,0 +1,25 @@ +base 0069949ff DOCKER_HOST=unset docker=29.7.2 start 22:43:03 +pkg-bus rc=0 22:43:03-22:43:06 ℹ pass 128 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 3441.554428 +pkg-business rc=0 22:43:06-22:43:09 ℹ pass 67 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 2641.955905 +pkg-cli rc=0 22:43:09-22:43:33 ℹ pass 168 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 23654.050348 +pkg-control-board rc=0 22:43:33-22:43:34 ℹ pass 124 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 776.406583 +pkg-conversation rc=0 22:43:34-22:44:24 ℹ pass 182 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 50200.857723 +pkg-discord rc=0 22:44:24-22:44:27 ℹ pass 178 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 2724.115957 +pkg-harness rc=0 22:44:27-22:44:37 ℹ pass 73 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 10503.171443 +pkg-ledger rc=0 22:44:37-22:44:58 ℹ pass 78 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 20466.374783 +pkg-mosaic rc=0 22:44:58-22:45:00 ℹ pass 69 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 2551.784669 +pkg-queue rc=0 22:45:00-22:45:34 ℹ pass 156 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 33487.603664 +pkg-runs rc=0 22:45:34-22:45:35 ℹ pass 43 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 792.822006 +pkg-seat rc=0 22:45:35-22:45:40 ℹ pass 119 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 5346.44468 +pkg-tasks rc=0 22:45:40-22:45:45 ℹ pass 92 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 5323.803666 +pkg-webui rc=0 22:45:45-22:46:41 ℹ pass 108 ℹ fail 0 ℹ skipped 0 | ℹ duration_ms 55778.135508 +script-auth rc=0 22:46:41-22:46:42 | selftest: 15 passed, 0 failed +script-conductor rc=0 22:46:42-22:46:47 | selftest: 17 passed, 0 failed +script-config rc=0 22:46:47-22:46:49 | selftest: 24 passed, 0 failed +script-discord rc=0 22:46:49-22:46:56 | discord suite: 66 passed, 0 failed +script-extension-package rc=0 22:46:56-22:46:57 | extension package selftest: 18 passed, 0 failed +script-foundation rc=0 22:46:57-22:47:38 | selftest: 44 passed, 0 failed +script-queue rc=0 22:47:38-22:48:12 ℹ pass 156 ℹ fail 0 | queue suite: 28 passed, 0 failed +script-release rc=0 22:48:12-22:48:24 | selftest: 14 passed, 0 failed +script-task rc=0 22:48:24-22:48:53 | selftest: 98 passed, 0 failed +GATE_PASSED 22:48:53 diff --git a/agents/darkwing/work/queue-106/r1/gate.sh b/agents/darkwing/work/queue-106/r1/gate.sh new file mode 100644 index 000000000..d818433af --- /dev/null +++ b/agents/darkwing/work/queue-106/r1/gate.sh @@ -0,0 +1,20 @@ +#!/bin/bash +set -o pipefail +# Darkwing gate, row 106 (#1600), candidate in the r106 worktree on base 0069949ff (row 104 landed). +# Every package's node suite, then every scripts/test-*.sh, sequential, each +# teed to its own log; stops at the first failure. +G=$HOME/darkwing-scratch/r106/wt +OUT=$HOME/darkwing-scratch/r106/gate +export TMPDIR=$HOME/darkwing-scratch/r106/tmp +mkdir -p "$OUT" "$TMPDIR" +cd "$G" || exit 9 +[ -e "$G/node_modules" ] || { echo "STOP no node_modules in $G"; exit 8; } +echo "base $(git rev-parse --short HEAD) DOCKER_HOST=${DOCKER_HOST:-unset} docker=$(docker info --format '{{.ServerVersion}}' 2>/dev/null || echo none) start $(date -u +%H:%M:%S)" +run() { local name="$1" dir="$2" cmd="$3" t0; t0=$(date -u +%H:%M:%S) + (cd "$G/$dir" && bash -c "$cmd") 2>&1 | tee "$OUT/$name.log" > /dev/null; local rc=$? + echo "$name rc=$rc $t0-$(date -u +%H:%M:%S) $(grep -E '^ℹ (pass|fail|skipped) [0-9]+' "$OUT/$name.log" | tr '\n' ' ') | $(tail -n1 "$OUT/$name.log")" + [ $rc -eq 0 ] || { echo "STOP $name"; exit $rc; }; } +for d in packages/*/; do p=$(basename "$d"); [ -d "$d/tests" ] || continue + run "pkg-$p" "packages/$p" 'node --test tests/*.test.mjs'; done +for s in scripts/test-*.sh; do n=$(basename "$s" .sh); run "script-${n#test-}" . "bash $s"; done +echo "GATE_PASSED $(date -u +%H:%M:%S)" diff --git a/agents/darkwing/work/queue-106/r1/mutants-run.log b/agents/darkwing/work/queue-106/r1/mutants-run.log new file mode 100644 index 000000000..94886a6ea --- /dev/null +++ b/agents/darkwing/work/queue-106/r1/mutants-run.log @@ -0,0 +1,12 @@ +start diff 60246b4202f67281 22:49:00 +n01 the fork runs outside the try: killed (expected killed) [('host.test.mjs', 1)] +n02 the send runs outside the try: killed (expected killed) [('host.test.mjs', 1)] +n03 the fork and send run outside the try, as at the base: killed (expected killed) [('host.test.mjs', 1), ('host.test.mjs', 1)] +n04 a lock gone before the read refuses as unreadable: killed (expected killed) [('host.test.mjs', 1)] +n05 any read failure counts as no lock: killed (expected killed) [('host.test.mjs', 1)] +n06 have takes the clones in reverse directory order: killed (expected killed) [('git-tools.test.mjs', 1)] +n07 have sorts by date across clones: killed (expected killed) [('git-tools.test.mjs', 1)] +x01 control: the base test against n06: survived (expected survived) [('git-tools.test.mjs', 0)] +x02 control: the base test against n07: survived (expected survived) [('git-tools.test.mjs', 0)] +end diff 60246b4202f67281 same 22:49:03 +MUTANTS_OK diff --git a/agents/darkwing/work/queue-106/r1/mutants.py b/agents/darkwing/work/queue-106/r1/mutants.py new file mode 100644 index 000000000..035e002f2 --- /dev/null +++ b/agents/darkwing/work/queue-106/r1/mutants.py @@ -0,0 +1,126 @@ +#!/usr/bin/env python3 +# Row 106 (#1600) mutants. Each takes out or bends one new check; every named +# (test file, name pattern) run must fail on it. A mutant may edit several +# files; each is restored from its .orig after the run, and the candidate's +# diff must hash the same at the end as at the start. The x mutants are +# controls: they run the base's `have` order test against n06 and n07, which +# it passes, since it takes the clone order from the request it checks. +import hashlib, os, signal, subprocess, sys, time + +WT = os.path.expanduser("~/darkwing-scratch/r106/wt") +OUT = os.path.expanduser("~/darkwing-scratch/r106/mutants") +ENV = dict(os.environ, TMPDIR=os.path.expanduser("~/darkwing-scratch/r106/tmp")) +os.makedirs(OUT, exist_ok=True) +os.makedirs(ENV["TMPDIR"], exist_ok=True) + +HO, GT = "packages/cli/src/host.mjs", "packages/harness/src/git-tools.mjs" +T_HO, T_GT = "packages/cli/tests/host.test.mjs", "packages/harness/tests/git-tools.test.mjs" +P_FORK, P_SEND = "^a broker fork that throws", "^a broker send that throws" +P_GONE, P_HAVE = "^a host.lock that goes between", "^fetch_review's have orders" + +TRY = ''' try { + broker = fork(BROKER, [], { stdio: ["ignore", "inherit", "inherit", "ipc"] }); + broker.send({ op: "boot", config: boot }); + ready = await firstReply(broker, bootTimeoutMs, "broker");''' +LOOP = ''' for (const remote of ["refs/remotes/working", "refs/remotes/origin"]) + for (const project of projects) { + if (have.size >= HAVE_MAX) break; + const r = await git(["-C", join(workspace, project), "for-each-ref", "--sort=-committerdate", "--format=%(objectname)", remote], env); + if (r.ok) for (const oid of r.out.split("\\n")) if (OID.test(oid) && have.size < HAVE_MAX) have.add(oid); + }''' +DATE_SORT = (GT, LOOP, ''' for (const remote of ["refs/remotes/working", "refs/remotes/origin"]) { + const all = []; + for (const project of projects) { + const r = await git(["-C", join(workspace, project), "for-each-ref", "--format=%(committerdate:unix) %(objectname)", remote], env); + if (r.ok) for (const l of r.out.split("\\n")) { const [d, oid] = l.split(" "); if (OID.test(oid ?? "")) all.push([Number(d), oid]); } + } + all.sort((x, y) => y[0] - x[0]); + for (const [, oid] of all) if (have.size < HAVE_MAX) have.add(oid); + }''') + +REVERSE = (GT, "const projects = projectsIn(workspace);", "const projects = projectsIn(workspace).reverse();") +BASE_TEST = (T_GT, None, subprocess.run(["git", "-C", WT, "show", "HEAD:" + T_GT], capture_output=True, text=True, check=True).stdout) + +# (name, [(path, old, new)], [(test, pattern)], expect) with old None for a whole-file replacement. +M = [ + ("n01 the fork runs outside the try", [(HO, TRY, ''' broker = fork(BROKER, [], { stdio: ["ignore", "inherit", "inherit", "ipc"] }); + try { + broker.send({ op: "boot", config: boot }); + ready = await firstReply(broker, bootTimeoutMs, "broker");''')], [(T_HO, P_FORK)], "killed"), + ("n02 the send runs outside the try", [(HO, TRY, ''' try { + broker = fork(BROKER, [], { stdio: ["ignore", "inherit", "inherit", "ipc"] }); + } catch (e) { + release(); + throw e; + } + broker.send({ op: "boot", config: boot }); + try { + ready = await firstReply(broker, bootTimeoutMs, "broker");''')], [(T_HO, P_SEND)], "killed"), + ("n03 the fork and send run outside the try, as at the base", [(HO, TRY, ''' broker = fork(BROKER, [], { stdio: ["ignore", "inherit", "inherit", "ipc"] }); + broker.send({ op: "boot", config: boot }); + try { + ready = await firstReply(broker, bootTimeoutMs, "broker");''')], [(T_HO, P_FORK), (T_HO, P_SEND)], "killed"), + ("n04 a lock gone before the read refuses as unreadable", [(HO, ' if (e.code === "ENOENT") return null;\n', "")], [(T_HO, P_GONE)], "killed"), + ("n05 any read failure counts as no lock", [(HO, 'if (e.code === "ENOENT") return null;', "return null;")], [(T_HO, P_GONE)], "killed"), + ("n06 have takes the clones in reverse directory order", [REVERSE], [(T_GT, P_HAVE)], "killed"), + ("n07 have sorts by date across clones", [DATE_SORT], [(T_GT, P_HAVE)], "killed"), + ("x01 control: the base test against n06", [REVERSE, BASE_TEST], [(T_GT, P_HAVE)], "survived"), + ("x02 control: the base test against n07", [DATE_SORT, BASE_TEST], [(T_GT, P_HAVE)], "survived"), +] + +def diffhash(): + d = subprocess.run(["git", "-C", WT, "diff", "--", ".", ":!node_modules"], capture_output=True).stdout + return hashlib.sha256(d).hexdigest() + +def run(test, pattern, log): + pkg = os.path.join(WT, *test.split("/")[:2]) + with open(log, "ab") as f: + f.write(f"== {test} /{pattern}/\n".encode()); f.flush() + p = subprocess.Popen(["node", "--test", f"--test-name-pattern={pattern}", os.path.relpath(os.path.join(WT, test), pkg)], cwd=pkg, env=ENV, + stdout=f, stderr=subprocess.STDOUT, start_new_session=True) + try: + rc = p.wait(timeout=600) + except subprocess.TimeoutExpired: + os.killpg(p.pid, signal.SIGKILL); p.wait(); return "timeout" + # A pattern that matched nothing passes with no tests; count that as a fault. + tail = open(log, encoding="utf8", errors="replace").read().split(f"== {test} /{pattern}/\n")[-1] + ran = sum(int(l.split()[2]) for l in tail.splitlines() if l.startswith(("ℹ pass ", "ℹ fail "))) + return "no-tests" if ran == 0 else rc + +for _, edits, *_ in M: + for path, *_ in edits: + if os.path.exists(os.path.join(WT, path + ".orig")): + sys.exit(f"STOP {path}.orig exists") +start = diffhash() +print(f"start diff {start[:16]} {time.strftime('%H:%M:%S', time.gmtime())}", flush=True) +bad = [] +only = set(sys.argv[1:]) +for name, edits, tests, expect in M: + if only and name.split()[0] not in only: continue + srcs = {} + ok = True + for path, old, new in edits: + full = os.path.join(WT, path) + src = srcs.get(path) or open(full).read() + if old is not None and src.count(old) != 1: + print(f"{name}: ANCHOR {path} {src.count(old)}", flush=True); ok = False; break + srcs[path] = new if old is None else src.replace(old, new) + if not ok: + bad.append(name); continue + for path in srcs: os.rename(os.path.join(WT, path), os.path.join(WT, path + ".orig")) + try: + for path, text in srcs.items(): open(os.path.join(WT, path), "w").write(text) + log = os.path.join(OUT, name.split()[0] + ".log") + open(log, "w").close() + rcs = [run(t, p, log) for t, p in tests] + finally: + for path in srcs: os.replace(os.path.join(WT, path + ".orig"), os.path.join(WT, path)) + if any(rc in ("timeout", "no-tests") for rc in rcs): got = "FAULT" + elif all(rc != 0 for rc in rcs): got = "killed" + elif all(rc == 0 for rc in rcs): got = "survived" + else: got = "MIXED" + print(f"{name}: {got} (expected {expect}) {list(zip([t.split('/')[-1] for t, _ in tests], rcs))}", flush=True) + if got != expect: bad.append(name) +end = diffhash() +print(f"end diff {end[:16]} {'same' if end == start else 'CHANGED'} {time.strftime('%H:%M:%S', time.gmtime())}", flush=True) +print("MUTANTS_OK" if not bad and end == start else f"MUTANTS_BAD {bad}")