diff --git a/agents/sage/work/s4-follow-up/trackers-boot.test.mjs b/agents/sage/work/s4-follow-up/trackers-boot.test.mjs new file mode 100644 index 00000000..b7710013 --- /dev/null +++ b/agents/sage/work/s4-follow-up/trackers-boot.test.mjs @@ -0,0 +1,66 @@ +// Sage's row 39 gate check, not part of the candidate: the S4 host boots the +// S3 adapter through the real process.mjs when bootConfig emits trackers. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { chmodSync, mkdirSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { Client } from "../../bus/src/client.mjs"; +import { bootConfig, loadSystem } from "../src/config.mjs"; +import { startHost, startTimeOf } from "../src/host.mjs"; +import { writeJson } from "../../business/tests/helpers.mjs"; +import { FakeVikunja } from "../../tasks/src/fake.mjs"; +import { SCOPES } from "../../tasks/tests/world.mjs"; +import { fixture, tmp } from "./helpers.mjs"; + +test("bootConfig trackers reach the S3 adapter in the real broker child, which goes ready against a fake Vikunja", async (t) => { + const root = tmp(t); + const fake = new FakeVikunja(); + const srv = await fake.listen(); + t.after(() => srv.close()); + const owner = fake.user("svc-acme"); + const bots = {}; + for (const r of ["sync", "pm", "cto", "coder", "reviewer"]) bots[r] = fake.user(`bot-acme-${r}`, { owner }); + const project = fake.project("Acme", owner); + for (const r of ["pm", "cto", "coder", "reviewer"]) fake.share(project, bots[r], 1); + fake.share(project, bots.sync, 0); + fake.install(project); + + const f = fixture(root, "acme", (doc) => { + doc.vars["tracker.baseUrl"] = srv.url; + doc.tracker.sync.botId = bots.sync; + for (const r of Object.keys(doc.roles)) doc.roles[r].tracker.botId = bots[r]; + return doc; + }); + const put = (file, token) => { + writeFileSync(file, token + "\n"); + chmodSync(file, 0o600); + }; + put(f.doc.tracker.sync.credentials.vikunja.file, fake.token(bots.sync, SCOPES.sync)); + for (const r of Object.keys(f.doc.roles)) put(f.doc.roles[r].credentials.vikunja.file, fake.token(bots[r], r === "pm" ? SCOPES.pm : SCOPES.worker)); + writeJson(join(root, "project", ".mosaic", "project.json"), { projectVersion: 1, id: "stack", vars: { "tracker.project": project } }); + + mkdirSync(f.dataRoot, { recursive: true, mode: 0o700 }); + const boot = bootConfig({ system: loadSystem({ env: f.env }), businessId: "acme", env: f.env }); + assert.deepEqual(boot.trackers, { acme: { baseUrl: srv.url, project, pollSeconds: 60, reconcileMinutes: 60 } }); + const host = await startHost({ boot, business: "acme", log: () => {} }); + t.after(() => host.close(0)); + + const launch = await host.bindLaunch({ business: "acme", role: "pm", run: "pm-run", harness: "pi", pid: process.pid, startTime: startTimeOf(process.pid) }); + const pm = new Client({ path: host.path, cap: launch.cap }); + await pm.call("role.claim"); + let code; + const end = Date.now() + 15000; + do { + code = await pm.call("task.close", {}).then(() => "ok", (e) => e.code); + if (code !== "tracker-starting") break; + await new Promise((r) => setTimeout(r, 100)); + } while (Date.now() < end); + console.log(`task.close {} answered: ${code}; fake saw ${fake.requests.length} requests, first ${fake.requests.slice(0, 3).map((r) => `${r.method} ${r.path} ${r.status}`).join(", ")}`); + assert.ok(fake.requests.some((r) => r.path === "/info"), "the adapter called the fake"); + assert.doesNotMatch(code, /^(tracker-|credential-|scope-too-broad)/, "the adapter is ready, so the verb fails on its own arguments"); + const before = fake.requests.length; + const missing = await pm.call("task.close", { task_ref: `vikunja:${project}/999`, verdict: "gate check" }).then(() => "ok", (e) => e.code); + console.log(`task.close on a missing task answered: ${missing}; it made ${fake.requests.slice(before).map((r) => `${r.method} ${r.path} ${r.status}`).join(", ")}`); + assert.ok(fake.requests.length > before, "a well-formed verb reached Vikunja through the adapter"); + assert.equal(await host.close(0), 0); +}); diff --git a/docs/plans/2026-09-26_lead-decisions.md b/docs/plans/2026-09-26_lead-decisions.md index c9980be3..5b782fce 100644 --- a/docs/plans/2026-09-26_lead-decisions.md +++ b/docs/plans/2026-09-26_lead-decisions.md @@ -1396,3 +1396,39 @@ which stay with him. Each item names who decided it and what happened. - Tests for mutants M22 to M24, M26 and M29. - Follow-ups, not round 2: F2 (a refused DM retries every 30 min with no limit) and J4 (loose types in confirmed lines). + +72. **S4 follow-up row and the conversation cohort row (2026-10-09).** + Source: Filbert's and Darkwing's round 1 and round 2 reviews of + #1521, the row 38 and row 39 gate runs, and my probe of the cohort + failures. Brief: `docs/plans/2026-10-09_s4-follow-up-and-cohort.md`. + - The items decision 71 and the reviews held for later go in one + row for Rocko: F2, J4, the N2, N4, N5 and M28 mutants, the reader + capability check, the host's `broker.connected` guard, clearer + journal errors, the `mkdir -m 0700` line in the install message, + the flaky discord lock test, and my trackers boot test. Filbert + and Darkwing review, since the row changes the DM path again. + - F2 ruling. Retrying `unknown` outcomes (network, 5xx or 429) + stays unlimited at the 30-minute cap. A missed blocking decision + still costs more than a duplicate DM. A definite refusal is a + 4xx other than 429. Five of them for one decision stop that DM: + one `gave-up` line, one log line, and the digest says "DM refused, + not retried". The count comes from the journal, so a restart + doesn't start it over. Five is enough to ride out a binding typo + fixed within a few hours. Fewer would give up on a fixable + mistake, and more only lengthens the journal. + - J4 ruling. Opening the journal type-checks confirmed lines and + refuses a bad one with exit 3, the same as a malformed line. + Today's lookups go by string, so the loose types do no harm yet. + But a journal that loads anything stops being evidence. + - The packet's BUILD.md M28 line stays as approved. BUILD-LOG holds + the correction. The bus shim's `spawnSync` timeout (Filbert round + 1, note 8) stays out of scope until a bus row needs it. + - Cohort row. K1, K3 and K10 fail on this host in isolation. All + three use an `ignoreTerm` child under a systemd user scope. That + child survives SIGTERM outside a scope. They passed on 2026-10-05, + the package hasn't changed since 243e153c, and no systemd, kernel + or node upgrade landed after 2026-09-03. Darkwing owns the + diagnosis and fix, and Dewey reviews. Dewey wrote CHAT-03 I1, and + row 40 gates on this suite. If the cause is the host's systemd + configuration, the row reports it and stops. That's Jason's + machine. diff --git a/docs/plans/2026-10-09_s4-follow-up-and-cohort.md b/docs/plans/2026-10-09_s4-follow-up-and-cohort.md new file mode 100644 index 00000000..5342a008 --- /dev/null +++ b/docs/plans/2026-10-09_s4-follow-up-and-cohort.md @@ -0,0 +1,207 @@ +# S4 follow-up and the conversation cohort failures (2026-10-09) + +Status: written by Sage, lead, under lead decision 72. It follows +`docs/plans/BRIEF-TEMPLATE.md` and amends no section of the slice 1 +brief. Two rows, one heading each. + +## S4 follow-up: notifier retry limit, journal types and the round 2 test gaps + +### Problem + +Row 39 (S4, #1521) landed as 2f5303c1 with items the reviews left for +later. They are listed below. + +- **F2.** `packages/cli/src/notifier.mjs` retries a refused DM with + backoff capped at 30 minutes and no attempt limit. It also appends a + journal line on every attempt, so a DM that Discord refuses for good + keeps sending and grows `sent.jsonl` by 48 lines a day. +- **J4.** Confirmed journal lines with loose types load. Examples are a + numeric `decision` and a digest line with no `day` (Filbert round 1, + note 4). +- **Surviving mutants.** N2, N4 and N5 survive (Filbert round 2, notes + 4 to 6): + - N2: the directory check uses `statSync` instead of `lstatSync`. + - N4: `append` opens without `O_NOFOLLOW`. + - N5: `watchChildren`'s early check ignores `signalCode`. +- **M28.** It survived. No test covers the host's "a bus host already + runs" refusal. The BUILD-LOG entry for row 39 records the correction. +- **Reader capability.** host.test checks that the launch capability + stays out of `/proc//cmdline` and `environ`. It doesn't check + the reader capability sent to the notifier (Filbert round 1, note 5). +- **Host shutdown path.** `packages/cli/src/host.mjs:144` and `:150` call + `broker.send({ op: "close" })` without checking `broker.connected`. + Against a broker that already died, that throws `ERR_IPC_CHANNEL_CLOSED`. +- **Test cleanup.** The host test "a notifier that dies" has no + `t.after` close, so a failure leaves children running. +- **Raw errors.** + - A read-only or 0500 journal directory surfaces a raw `EACCES`, not a + `CliError` (Filbert round 2, note 10; Darkwing round 2, note 2). + - A symlinked directory refuses with "must be mode 0700 and owned by + this user". That is true, but it doesn't name the link (Filbert + round 2, note 9). +- **Install message.** `scripts/bus-service.sh` tells the operator to + write `notify.json` but never says to create the directory 0700 + (Darkwing round 2, note 1). An operator who runs a plain `mkdir -p` + gets a host that refuses with exit 3. +- **Flaky discord test.** `packages/discord/tests/journal.test.mjs:120` + publishes pid `2 ** 22 - 7` and expects `ownerState` to be `dead`. On + a host where that pid is live, it reads `unknown`. That was the only + failure in the row 39 M28 mutant run. +- **Trackers boot test.** The combined-tree check that S4's host + boots S3's tracker adapter from config ran from scratch at the row 39 + gate and isn't in the suite. It is at + `agents/sage/work/s4-follow-up/trackers-boot.test.mjs`. Its relative + imports assume `packages/cli/tests/`. + +### Owner and reviewer + +- Owner: rocko. +- Reviewers: filbert, and darkwing because the row changes the DM path + and the discord package's tests. + +### Files owned + +- `packages/cli/src/notifier.mjs`, `packages/cli/src/host.mjs` +- `packages/cli/tests/*`, including the new + `packages/cli/tests/trackers-boot.test.mjs` +- `packages/cli/README.md` +- `packages/discord/tests/journal.test.mjs` +- `scripts/bus-service.sh` +- `docs/TOOLS.md`, only if a command's output changes +- `agents/rocko/work/s4-follow-up/` (the packet) + +### What ships + +1. **F2, under decision 72.** + - A DM refused with an HTTP 4xx other than 429 is a definite + refusal. After 5 definite refusals for one decision, the notifier + stops sending that DM. It appends one `gave-up` line and logs it + once. + - The count comes from the journal, so a restart doesn't reset it. + - The digest lists that decision as "DM refused, not retried". + - An `unknown` outcome (network, 5xx or 429) keeps the capped + backoff with no limit. Under decision 70, a duplicate DM is + cheaper than a missed one. + - The `outcome` comment in `notifier.mjs` and the README name the + new value. +2. **J4.** + - Opening the journal type-checks every complete line: + - `at`: ISO string + - `kind`: `dm` or `digest` + - `decision`: string, required for `dm` + - `day`: `YYYY-MM-DD` string, required for `digest` + - `outcome`: one of the four values + - `messageId`: string or null + - `status`: integer when present + - A line that fails refuses with exit 3, as a malformed line does + today. The decision 71 torn-tail rule is unchanged. +3. **Tests.** + - Tests that kill N2, N4, N5 and M28. + - A reader-capability exposure check in host.test. + - `t.after` cleanup in "a notifier that dies". +4. **Host shutdown.** Guard both `broker.send` calls with + `broker.connected`. +5. **Errors.** + - A journal directory the notifier can't write refuses with a + `CliError`, exit 3, naming the path. + - A symlinked directory refuses with a message that says it is a + link. +6. **Install message.** `bus-service.sh`'s install message adds + `mkdir -m 0700 -p /notify/`. +7. **Discord test.** `journal.test.mjs:120` uses a pid that is + provably not running, for example a reaped child's pid checked with + `kill(pid, 0)`. It must not depend on a fixed number. +8. **Trackers boot test.** Move it into `packages/cli/tests/` as + written, adjusting only its imports if needed. +9. **Suites.** cli, discord, bus and tasks node suites, + `test-discord.sh` and every `test-*.sh` green. Include a mutant + table for N2, N4, N5, M28 and the F2 limit. + +### Out of scope + +- Round 1 note 8, the `spawnSync` timeout in `human-cli`. It lives in + the bus shim. It goes to a bus row if it ever bites. +- The live DM run. That is Sage's, at Jason's go, under decision 70. +- The packet `agents/rocko/work/slice1-s4/BUILD.md`. It stays as + approved, and BUILD-LOG holds the M28 correction. + +### Gate + +- Filbert and Darkwing approve on the row's issue. +- Sage reruns the integration gate on a git worktree with the candidate + applied: every node package suite and every `scripts/test-*.sh`. + Expect zero failures, except the three conversation cases owned by + the next row, and only until that row lands. + +## Conversation cohort: K1, K3 and K10 fail on the scope fixtures + +### Problem + +`packages/conversation/tests/cohort.test.mjs` fails K1, K3 and K10 on +this host. + +- **Where it fails.** It failed at the row 38 and row 39 gates, and + again on the canonical tree run alone on 2026-10-09 at 12:50Z. That + run was `node --test --test-name-pattern='^K(1|3|10):'`: 0 pass, + 3 fail. +- **The assertions.** + - K1: "the escaped child is a listed member". + - K3: "the member ignored TERM". The child is dead after TERM. + - K10: "the member is alive across the crash". +- **What all three share.** Each spawns a tool child with `ignoreTerm` + under a systemd user scope (`packages/conversation/tests/fake-pi.mjs`, + `spawnChild`). +- **Outside a scope.** The same child spawned outside a scope survives + SIGTERM (Sage's probe, 2026-10-09). +- **What passes.** K2 (process-group fallback), K11 to K15 and the + in-process cases pass. +- **History.** All three passed at the row 44 gate on 2026-10-05. The + package hasn't changed since 243e153c (2026-10-04). +- **Not a package upgrade.** The host's last relevant upgrades were + systemd 261.2, kernel 7.2.2 and node 26.8.1, all on 2026-09-03, + before that gate. So something else on the host or in the user + manager changed. +- **Who it blocks.** Row 40 (S5) gates on the conversation suite being + green. + +### Owner and reviewer + +- Owner: darkwing. +- Reviewer: dewey, who wrote CHAT-03 I1 and owns row 40, which needs + this suite green. + +### Files owned + +- `packages/conversation/tests/*` +- `packages/conversation/src/cohort.mjs`, only if the diagnosis shows a + real defect in the cohort proof rather than in the fixture +- `agents/darkwing/work/cohort-k1/` (the packet: diagnosis, probes and + receipts) + +### What ships + +1. **Diagnosis.** Say why an `ignoreTerm` child in the scope dies or + leaves the member list early. Back it with receipts: + - `systemctl --user show` on the scope + - `cgroup.procs` before and after TERM + - the child's exit signal +2. **Fixture defect.** If it is a fixture or environment defect, fix + the fixture so the three cases test what their names say on this + host. Never weaken an assertion to pass. +3. **Code defect.** If it is a defect in `cohort.mjs`, fix it. A + `stopped` proof must stay impossible while a member lives. +4. **Suites.** conversation and webui suites green on this host, and + K1, K3 and K10 green in 3 consecutive isolated runs. + +### Out of scope + +- Changes to the host's systemd configuration or user manager. If the + diagnosis points there, report it and stop; that's Jason's machine. +- Any CHAT-03 behavior beyond the stop and cohort proof. + +### Gate + +- Dewey approves on the row's issue. +- Sage reruns the conversation suite and the three cases in isolation + on a worktree with the candidate applied. Expect 152/152, then 3/3 + three times.