From ff087a554b260305941c7d9987b2e1f1929cfd0f Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Wed, 30 Sep 2026 22:56:27 +0200 Subject: [PATCH] Watchdog notice reader: pin timeout, spawn-error, 5xx/429 and guard behaviour; say what non-2xx is Tests only, plus wording. No change to what the reader or bridge does. - A spawn timeout and a spawn error (ENOBUFS from a bridge that floods stdout) record unknown, are never re-posted, and block the closure. WATCHDOG_NOTICE_BRIDGE_TIMEOUT_MS overrides the 60 s timeout so the test reaches that branch; the default is unchanged. - A 500/502/503/429 answer to the message POST exits 75 from the bridge, and through the reader is retried with the same nonce until it completes. - allowed_mentions {parse: []} is asserted in the posted body; status exits non-zero while an entry is unknown and zero when none is. - The bridge comment and docs/watchdog-notice.md no longer call a non-2xx answer "definite": it is retried with the same nonce, relying on enforce_nonce inside Discord's window, and may double-send if Discord acted on a 5xx outside it. Builds. Two tests fail on Windows (SIGKILL status), as before; they pass on Yggdrasil. Co-Authored-By: Claude Sonnet 5.5 --- docs/watchdog-notice.md | 12 ++++--- tests/watchdog-notice.test.mjs | 64 ++++++++++++++++++++++++++++++---- tools/bifrost-bridge.mjs | 9 +++-- tools/watchdog-notice.mjs | 10 +++--- 4 files changed, 77 insertions(+), 18 deletions(-) diff --git a/docs/watchdog-notice.md b/docs/watchdog-notice.md index a1c7004..610f46c 100644 --- a/docs/watchdog-notice.md +++ b/docs/watchdog-notice.md @@ -33,10 +33,14 @@ node tools/watchdog-notice.mjs status --journal-store J start becomes `unknown` and is never sent again. A `failed` entry is retried on later runs with the same nonce, up to five attempts. The journal is never pruned: what Bifrost has sent is Bifrost's memory, whatever P holds. -- The bridge (`bifrost-bridge.mjs discord-dm`) exits 75 only when it knows no - message was sent: argument or token errors, a failure opening the DM channel, - or a non-2xx answer to the message POST. The reader records `failed` for - exit 75 only. Any other non-success without a message id (another exit code, +- The bridge (`bifrost-bridge.mjs discord-dm`) exits 75 for argument or token + errors, a failure opening the DM channel, or any non-2xx answer to the message + POST. A non-2xx is retried, not proven unsent: the retry carries the same nonce + and relies on Discord's `enforce_nonce` inside its window, so a 5xx that Discord + answered after creating the message can be delivered twice if the retry falls + outside that window. A 5xx stays retryable because parking it as `unknown` + would lose the notice on every routine Discord outage. The reader records + `failed` for exit 75 only. Any other non-success without a message id (another exit code, a signal, the spawn timeout, unreadable output) is `unknown`: never sent again, counted by the exit status. - The exit status is non-zero while any entry is `unknown` or out of attempts, or diff --git a/tests/watchdog-notice.test.mjs b/tests/watchdog-notice.test.mjs index 21a9223..ff7c6ae 100644 --- a/tests/watchdog-notice.test.mjs +++ b/tests/watchdog-notice.test.mjs @@ -32,8 +32,9 @@ const cache = createRequire(resolve(cult, "packages", "cultcache-ts", "package.j const msgpack = createRequire(resolve(cult, "packages", "cultnet-ts", "package.json"))("@msgpack/msgpack"); // A bridge double that logs each call (with the content it was handed) and -// behaves as the `mode` file says: ok; fail (exit 75, known not sent); die -// (exit 1 after "sending"); signal (killed after "sending"); or crash (kills its +// behaves as the `mode` file says: ok; fail (exit 75, retryable); die +// (exit 1 after "sending"); signal (killed after "sending"); hang (never answers after +// "sending"); flood (answers with more stdout than spawnSync buffers); or crash (kills its // parent, the reader, after logging, as a power cut between send and journal would). const FAKE_BRIDGE = ` import { appendFileSync, readFileSync } from "node:fs"; @@ -49,6 +50,9 @@ const mode = readFileSync(resolve(here, "mode"), "utf8").trim(); if (mode === "crash") { process.kill(process.ppid, "SIGKILL"); process.exit(0); } if (mode === "fail" || mode === "die") { process.stderr.write("boom " + opt("recipient-id")); process.stdout.write("boom " + opt("recipient-id")); process.exit(mode === "fail" ? 75 : 1); } if (mode === "signal") process.kill(process.pid, "SIGKILL"); +if (mode === "hang") setTimeout(() => {}, 30000); +else if (mode === "flood") process.stdout.write("x".repeat(2 * 1024 * 1024)); +else process.stdout.write(JSON.stringify({ action: "discord-dm", ok: true, messageId: "m" + (n + 1) })); `; @@ -172,7 +176,7 @@ test("a running entry becomes unknown and is never re-posted", async () => { assert.equal((await w.posts()).length, posts.length); }); -test("only a known-unsent bridge exit is retried, with the same nonce, up to five attempts", async () => { +test("only a bridge exit 75 is retried, with the same nonce, up to five attempts", async () => { const w = await world("wn-retry-", { mode: "fail" }); const exits = []; for (let run = 0; run < 5; run += 1) exits.push(w.run().status); @@ -265,6 +269,7 @@ test("a bridge that dies after sending is unknown and never re-posted", async () const first = w.run(); assert.notEqual(first.status, 0); assert.equal(JSON.parse(first.stdout).unknown, 2); + assert.notEqual((await w.status()).status, 0, "status exits non-zero while an entry is unknown"); await w.setMode("ok"); assert.notEqual(w.run().status, 0); assert.notEqual(w.run().status, 0); @@ -276,6 +281,22 @@ test("a bridge that dies after sending is unknown and never re-posted", async () } }); +test("a spawn timeout and a spawn error are unknown, never failed, never re-posted", async () => { + for (const mode of ["hang", "flood"]) { + const w = await world(`wn-${mode}-`, { mode }); + const first = w.run({ env: { WATCHDOG_NOTICE_BRIDGE_TIMEOUT_MS: "1500" } }); + assert.notEqual(first.status, 0, mode); + assert.equal(JSON.parse(first.stdout).unknown, 2, mode); + await w.setMode("ok"); + assert.notEqual(w.run().status, 0); + assert.notEqual(w.run().status, 0); + const posts = await w.posts(); + assert.equal(posts.length, 2, `${mode}: one opening post per incident, no retry, no closure`); + assert.ok(posts.every((post) => notice(post) === "opened")); + assert.deepEqual((await w.status()).rows.map((row) => `${row.notice}:${row.status}:${row.attempts}:${row.lastError}`), ["opened:unknown:1:bridge-exit-unknown", "opened:unknown:1:bridge-exit-unknown"], mode); + } +}); + test("errors name a code and never echo a path or an input", async () => { const w = await world("wn-canary-", { incidents: null }); await mkdir(resolve(w.dir, "CANARYDIR")); @@ -299,7 +320,8 @@ globalThis.fetch = async (url, init) => { const channels = String(url).endsWith("/users/@me/channels"); const behavior = process.env.STUB_BEHAVIOR; if (channels && behavior === "dm-channel-500") return new Response("{}", { status: 500 }); - if (!channels && behavior === "message-400") return new Response("{}", { status: 400 }); + const rejected = /^message-(\\d{3})$/.exec(behavior ?? ""); + if (!channels && rejected) return new Response("{}", { status: Number(rejected[1]) }); if (!channels && behavior === "message-network-error") throw new TypeError("fetch failed"); return new Response(JSON.stringify(channels ? { id: "dm-channel" } : { id: "message-1" }), { status: 200 }); }; @@ -319,6 +341,7 @@ test("discord-dm carries nonce and enforce_nonce when asked", async () => { let posted = (await fetches(stub.log)).filter((entry) => entry.url.endsWith("/channels/dm-channel/messages")); assert.equal(posted.at(-1).body.nonce, "abc123"); assert.equal(posted.at(-1).body.enforce_nonce, true); + assert.deepEqual(posted.at(-1).body.allowed_mentions, { parse: [] }, "no mention is ever parsed"); const without = dm(); assert.equal(without.status, 0, without.stderr); @@ -347,14 +370,40 @@ test("discord-dm exits 75 only before delivery", async () => { assert.equal(dm(undefined, { recipient: "" }).status, 75, "missing argument"); assert.equal(dm("dm-channel-500").status, 75, "DM channel failure"); assert.equal(await messagePosts(), 0, "nothing was posted by any of those"); - assert.equal(dm("message-400").status, 75, "definite non-2xx on the message POST"); + assert.equal(dm("message-400").status, 75, "a 4xx answer to the message POST"); assert.equal(await messagePosts(), 1); + for (const status of [500, 502, 503, 429]) { + const before = await messagePosts(); + assert.equal(dm(`message-${status}`).status, 75, `a ${status} answer is retryable, not unknown`); + assert.equal(await messagePosts(), before + 1); + } const lost = dm("message-network-error"); - assert.equal(lost.status, 1, "a network error after the message POST was sent is not known-unsent"); - assert.equal(await messagePosts(), 2); + assert.equal(lost.status, 1, "a network error after the message POST was sent is not exit 75"); + assert.equal(await messagePosts(), 6); assert.equal(dm().status, 0); }); +test("a 5xx or 429 on the message POST is retried with the same nonce", async () => { + for (const status of [502, 429]) { + const w = await world(`wn-${status}-`); + const stub = await fetchStub(w.dir); + const env = { NODE_OPTIONS: stub.options, BIFROST_DISCORD_BOT_TOKEN: "test-token-not-real" }; + const messages = async () => (await fetches(stub.log)).filter((entry) => entry.url.endsWith("/messages")); + for (let tick = 1; tick <= 2; tick += 1) { + const result = w.run({ bridge: BRIDGE, env: { ...env, STUB_BEHAVIOR: `message-${status}` } }); + assert.equal(result.status, 0, result.stderr); + assert.deepEqual((await w.status()).rows.map((row) => `${row.status}:${row.attempts}`), [`failed:${tick}`, `failed:${tick}`]); + } + const sent = await messages(); + assert.equal(sent.length, 4, "two openings, each tried twice"); + const nonces = Map.groupBy(sent, (entry) => entry.body.content); + assert.equal(nonces.size, 2); + for (const group of nonces.values()) assert.equal(new Set(group.map((entry) => entry.body.nonce)).size, 1, "a retry reuses its nonce"); + assert.equal(w.run({ bridge: BRIDGE, env }).status, 0); + assert.deepEqual((await w.status()).rows.map((row) => row.status), ["completed", "completed", "completed"]); + } +}); + test("every subject Idunn accepts is delivered, inside backticks", async () => { const good = ["gamecult/odin", "odin:store", "_odin", "a".repeat(256), "Odin.v2-x"]; const bad = ["a".repeat(257), "has space", "back`tick", "new\nline", "", "ün"]; @@ -406,5 +455,6 @@ test("the reader posts through the real bridge with its nonce", async () => { } assert.ok(existsSync(w.receipts), "the bridge wrote its crossing receipts to the reader's receipt store"); assert.deepEqual((await w.status()).rows.map((row) => row.status), ["completed", "completed", "completed"]); + assert.equal((await w.status()).status, 0, "status exits zero when nothing is unknown or exhausted"); }); diff --git a/tools/bifrost-bridge.mjs b/tools/bifrost-bridge.mjs index 3d9e7db..e51b2f0 100644 --- a/tools/bifrost-bridge.mjs +++ b/tools/bifrost-bridge.mjs @@ -471,9 +471,12 @@ async function postDiscordMessage(options) { } // discord-dm exits NOT_SENT_EXIT only when it knows no message was delivered: -// anything before the message POST, or a definite non-2xx answer to it. A -// failure after the POST was sent (network error, receipt completion, output) -// keeps exit 1, so the caller treats the outcome as unknown. +// anything before the message POST, or a non-2xx answer to it. A 5xx or 429 +// is not proof of non-delivery (Discord may have created the message before +// answering): the caller retries with the same nonce and relies on +// enforce_nonce inside Discord's window. A failure after the POST was sent +// (network error, receipt completion, output) keeps exit 1, so the caller +// treats the outcome as unknown. const NOT_SENT_EXIT = 75; class NotSentError extends Error {} class DiscordRejection extends Error {} diff --git a/tools/watchdog-notice.mjs b/tools/watchdog-notice.mjs index 9fbee59..485a59d 100644 --- a/tools/watchdog-notice.mjs +++ b/tools/watchdog-notice.mjs @@ -28,15 +28,17 @@ const INCIDENT_SCHEMA = "idunn.operator_incident.v1"; const JOURNAL_TYPE = "bifrost.watchdog_notice_execution"; const JOURNAL_SCHEMA = "bifrost.watchdog_notice_execution.v1"; const MAX_ATTEMPTS = 5; -const BRIDGE_TIMEOUT_MS = 60_000; +// The override exists so a test can reach the timeout branch without a 60 s wait. +const BRIDGE_TIMEOUT_MS = Number(process.env.WATCHDOG_NOTICE_BRIDGE_TIMEOUT_MS) || 60_000; const STATUSES = ["running", "completed", "failed", "unknown"]; const NAME = /^[a-z][a-z-]{0,63}$/; // Idunn's require_id (GameCult/Idunn src/control_plane.rs:9122-9131 at 7f7528b): // 1-256 bytes of [A-Za-z0-9-_.:/]. The notice puts the subject in inline code. const SUBJECT = /^[A-Za-z0-9_.:/-]{1,256}$/; const MAX_DATE_MS = 8.64e15; -// bifrost-bridge.mjs discord-dm exits 75 (EX_TEMPFAIL) only when it knows the -// message was not sent. Every other non-success leaves the outcome unknown. +// bifrost-bridge.mjs discord-dm exits 75 (EX_TEMPFAIL) for failures before +// the message POST and for any non-2xx answer to it: retried with the same nonce. +// Every other non-success leaves the outcome unknown. const BRIDGE_NOT_SENT = 75; class NoticeError extends Error {} @@ -175,7 +177,7 @@ function noticeText(record, notice) { } // The one send primitive. The outcome is completed with a message id; failed -// only when the bridge reports exit 75 (known not sent); otherwise unknown. The +// only when the bridge reports exit 75 (retryable, same nonce); otherwise unknown. The // error is a fixed code: bridge stdout and stderr never reach the journal. async function sendNotice({ bridgeCli, recipientId, receiptStore, key, nonce, record, content }) { const dir = await mkdtemp(resolve(tmpdir(), "watchdog-notice-"));