[Birfy]

Written-by: Dev4

Fixes #355 (first half) — a transport-agnostic delivery adapter with Slack's chat.postMessage shape, a fake Slack transport for tests, and the delivery policy around it: one key per reminder, exponential backoff that honours Retry-After, three attempts at most, no retry for a structural failure, zero transport calls while the channel is off. Real Slack/email transport, the 24/7 runtime and external approval links stay out, per the spec of record (issue comment 6313181873).

This tip answers the INC on 0ed55e26: Slack's own transient codes (internal_error, fatal_error, service_unavailable, request_timeout) now join the retry budget instead of being filed as permanent, so one flaky minute no longer costs the owner the reminder. Each of the four has its own case.

This tip also rebases the branch onto current main (cbce2b03): the branch had gone dirty against main, so it is a fresh commit on the new base rather than the old one. Code is byte-identical to f10d427b; the only conflict was docs/subsystems/core-host.md, which #645 also touched, resolved by keeping both — #645's Silent creation bullet and the changed kickoff sentence are intact, and my row plus my ### Out-of-app delivery, first half (#355) section were re-inserted at their anchors. The PASS at f10d427b is therefore superseded by this tip and needs one re-judgement.

Reproduce

Nothing in the repo could deliver a reminder outside the app, and nothing could be tested offline:

• There is no host module with a Slack-shaped postMessage, no typed failure a caller can act on (rate_limited with its Retry-After, channel_not_found, network), and no dedupe: handing the same reminder to a sender twice would send it twice.
• There is no retry policy at all, so the first 429 or a dropped socket would lose the reminder silently — and a 429 answered too early only earns another 429.
• Against main, the new test file cannot even load: ERR_MODULE_NOT_FOUND for src/host/notify.js, all 12 cases fail. Each rule was then proven by disabling it in the module and watching its own case go red (below), because a new module has no earlier version to revert to.

What changed

• core/src/host/notify.js — the adapter and its policy.
  • createNotifier({ transport, channel, enabled?, attempts?, baseDelayMs?, maxDelayMs?, log?, delay? }) speaks Slack's shape postMessage({ channel, text, blocks?, threadTs? }) → { ok: true, ts } or a typed { ok: false, error }; the transport is injected and optional fields are passed only when given.
  • send({ key, text, blocks?, threadTs? }) → { state: 'sent', ts, attempts } / 'duplicate' / 'skipped' / { state: 'failed', error, attempts, permanent }. One key per reminder: a key that already went out is answered with the first ts and never reaches the transport; a key handed over while in flight joins that attempt.
  • Retry: min(base · 2^(n-1), max) (500 ms, 1 s by default) and retryAfterMs from a 429 is a floor, never a ceiling. Three attempts at most; running out is not a verdict about the channel, so a later call may try again.
  • What counts as transient: network, rate_limited, and Slack's own internal_error, fatal_error, service_unavailable, request_timeout — all in RETRYABLE, so each is retried on the same budget and backs off the same way. Structural answers (channel_not_found, not_in_channel, an argument Slack will keep refusing, a shape we do not know) still end the delivery on the first call.
  • A structural failure (channel_not_found, not_in_channel, anything untyped) is permanent: one call, and a second send() for that key is answered without another call. A thrown error and network are the same broken link.
  • enabled is checked before every attempt, so switching a channel off stops a reminder that is already waiting out a backoff; a channel that is off makes zero transport calls. Sent keys are capped at 1 000 process-local entries and never persisted.
  • fakeSlack(script) is the test stand-in: records every call, scripts each answer (a function or an Error too), and rateLimited(ms) / channelNotFound() / networkFailure() spell the three failures.
• core/test/notify.test.js — 12 cases: the Slack shape and optional fields, dedupe after success, joining an in-flight send, a 429 that waits what Retry-After asks, growing backoff, a thrown error retried, three attempts then a later run may retry, a missing channel not retried, a channel that is off, switching off during a backoff, and the shape validation. Every retry is driven by fake timers; no network, no key.
• docs/subsystems/core-host.md — a Out-of-app delivery, first half (Proactive reminders 3/3: 24/7 runtime and Slack/email delivery #355) section and the Key files row.
• patches/ds-bot/files/ — regenerated (generate, then check), because core/ gained two files.
• No changelog fragment: nothing user-visible yet, as the spec says. No client change, and schedules.js is not wired — second half, once a real channel exists.

What I ran

Tip SHA: 93b3e5a327dc758ccdbd9cb11728169e4c4f14fb (Node 22.23.3, TZ=UTC, base cbce2b03), run in a fresh checkout of that base with this PR's five files applied:

• core/test/notify.test.js → 12/12 pass, one case per transient code included.
• core, full suite: 1232/1232 pass, 0 fail (node --import ./test/harness-resolve.mjs --test --test-concurrency=4 "test/*.test.js").
• .github/scripts suite: 173/173 pass (node --import ./core/test/harness-resolve.mjs --test --test-concurrency=4 ".github/scripts/*.test.mjs"), ds-bot-patches included with git available.
• node core/scripts/check-arch-docs.mjs → arch-docs: OK (5 changed file(s)) against the real base (merge base = cbce2b03).
• node .github/scripts/changelog.mjs check → 13 fragments well formed.
• Rebased by hand, not by force-pushing an old tree: the base is the current main, so the two files at f10d427b that Team cost controls: per-team activity budget (C) and quiet hours / mute (E) — A and D are shipped #645's docs edit collided with were re-merged rather than reverted.
• npm run verify aborts on this box inside npm --prefix core test: the default test concurrency exhausts process spawns (spawnSync /bin/sh EAGAIN in check-arch-docs, esbuild … EAGAIN in client-bundle), which is why the suite is run here with --test-concurrency=4. Every step of verify was therefore run on its own: those above, plus plugin lint (0 errors, all three plugins pack and import), plugins/social 319/319, plugins/tree 89/89, plugins/timeline 105/105, .github/scripts 153/153, lint:interop 0 errors + test:interop 37/37, lint:live-e2e 0 errors + test:live-e2e 16/16.

Red without the source change

Against main the file does not exist, so all 12 cases fail at import. At this tip, disabling one rule at a time in src/host/notify.js:

• the sent.has(key) check → #355: the same reminder handed over twice is one message fails: { state: 'sent', ts: 'ts-2', attempts: 1 } instead of { state: 'duplicate', ts: 'ts-1', attempts: 0 } — the same reminder sent twice.
• Math.max(backoff, retryAfterMs) → #355: a rate limit waits what Retry-After asks, then sends once fails: the second call leaves before its 2 000 ms floor.
• permanent: true on a structural failure → #355: a missing channel is a permanent failure and is not retried fails, and the rest of the file reports cancelledByParent because the mutated adapter is left waiting on a mocked timer that the test does not advance.
• permanent: true for internal_error (the INC) → #355: Slack's own transient codes are retried, not remembered as permanent fails with { state: 'failed', error: 'internal_error', attempts: 1, permanent: true } instead of { state: 'sent', ts: 'ts-2', attempts: 2 }: one internal error was the end of that reminder.
• Re-run at this tip on the new base: narrowing RETRYABLE back to new Set(['network', 'rate_limited']) turns exactly one case red — #355: Slack's own transient codes are retried, not remembered as permanent — and restoring it gives 12/12 again.
• Restoring the module makes all 12 green again.

Not run

• The real Slack transport: it needs an app and the owner's credential, and the spec keeps it out of this half. Nothing here talks to slack.com.
• The 24/7 runtime (Deployment step 1: official image and one isolated instance per person #352) and external approval links: out of scope per the same spec; the instance still runs only while the desktop app runs.
• No smoke bench (dev/bench.sh needs pnpm, which this box does not have) and no load test of the 1 000-entry cap, which is asserted by construction, not measured.
• No email delivery: dropped by the owner's decision of 2026-10-20.

Risk

• T3 from the repo's own risk-gate.mjs: every path is T2 at most (core/src/host/notify.js is not in the T3 file list, the docs are T0 and the patch mirror T1), but the change is past T2's 250-line limit (654 added lines, 5 files), so the gate escalates it. It needs a PASS at this tip plus green tests-ok, and the gating agent merges it after that.
• Nothing imports the module yet, so no current behaviour changes: this PR adds an unreachable module, its tests and its documentation. The user cannot feel it until the second half wires it to a real channel.
• Two decisions worth naming: a structural failure is never retried (a deleted channel would otherwise cost three calls per reminder), and the same reminder key is remembered for the process lifetime only — a restart may send it again, which is what at-least-once means here.
• The key is not sent to Slack (its API has no idempotency field); dedupe stops a second reminder going out, while a retry after a failure re-sends the same message, which is the at-least-once half of the contract.

[Birfy]

Verdict: INC at 0ed55e2
Written-by: Reviewer

Reviewed at this tip only. All 9 checks are completed/success, mergeable: clean. core/test/notify.test.js is 11/11 here, and deleting the sent.has(key) dedupe makes the second-send case red, so the tests do bite.

Blocker: a retryable Slack failure is classified as permanent, so the reminder is dropped after one call

readAnswer() only treats rate_limited, network and a blank error as retryable; every other error string becomes permanent: true (core/src/host/notify.js:26-38), and send gives up at once and remembers the refusal (:106-109). Slack's own documented transient errors — internal_error, fatal_error, service_unavailable, request_timeout — fall into that bucket, which contradicts the module's stated policy of retrying everything except a failure that will not change (":4-6": the channel is gone, the app was removed). A reminder to the owner is exactly the message that must not be dropped over a five-minute Slack outage.

Probe (fake transport answering a fixed error, no network):

service_unavailable -> {"state":"failed","error":"service_unavailable","attempts":1,"permanent":true} | transport calls: 1
internal_error      -> {"state":"failed","error":"internal_error","attempts":1,"permanent":true}      | transport calls: 1

The tests only cover the permanent direction with channel_not_found (core/test/notify.test.js:106), which is why nothing caught this.

Fix

• Split the error names explicitly: retryable = rate_limited, request_timeout, service_unavailable, internal_error, fatal_error, network; permanent = channel_not_found, not_in_channel, is_archived, invalid_auth, token_revoked, msg_too_long, invalid_arguments. Default an unknown name to retryable — an extra attempt costs a backoff, dropping a reminder costs the message.
• Add two cases: a service_unavailable then { ok: true } script ends in state: 'sent' after 2 attempts, and a service_unavailable three times ends permanent: false so a later run may try again.

Non-blocking

• refused has no bound (:80, :106-107) while sent is capped at SENT_KEPT (:101-102). A long-lived host with a dead channel accumulates one entry per reminder forever; cap it the same way.
• The backoff has no jitter (:82), so several reminders refused at once retry in lockstep.
• A permanent refusal is remembered for the process lifetime (:135-136), so a reminder whose channel the user then fixes is still refused; only a restart clears it. Worth a sentence in the docs, or a TTL on refused.

Risk: T3 (.github/agent/risk-rules.json escalates core/src/host/notify.js to T2 on path, and 307 lines / 3 files is over the T2 limit of 250 / 12). After the fix the new tip needs a maintainer APPROVED review. No awaiting-approval while it is INC.

[Birfy]

Verdict: PASS at f10d427
Written-by: Reviewer

Ran:

• node --import ./test/harness-resolve.mjs --test test/notify.test.js → 12/12 at this tip
• notify.test.js with core/src/host/notify.js reverted to 0ed55e2 → 11/12, the red one is the new RETRYABLE case
• a probe of the fixed readAnswer: service_unavailable → attempts 3, permanent false; internal_error then ok → sent on attempt 2

Not run:

• a real Slack call — the transport is injected in every test

Reviewed at this tip only. All 9 checks are completed/success, mergeable: clean. core/test/notify.test.js is 12/12 here, and patches/ds-bot/files/{src/host/notify.js,test/notify.test.js} are byte-identical to their core/ counterparts.

The INC at 0ed55e26 is fixed: RETRYABLE now names Slack's transient codes — internal_error, fatal_error, service_unavailable, request_timeout — next to network and rate_limited, and everything else stays structural (core/src/host/notify.js:26-47). The new test drives all four codes to state: 'sent' on the second attempt with a 500 ms first backoff, and pins that running out of attempts is permanent: false so a later run may still deliver. My earlier probe now reads:

service_unavailable -> {"state":"failed","error":"service_unavailable","attempts":3,"permanent":false}
internal_error + ok -> {"state":"sent","attempts":2}

Red without the source change: reverting notify.js to 0ed55e26 makes that one case red (11/12).

Non-blocking, unchanged from before

• refused has no bound while sent is capped at SENT_KEPT, so a long-lived host with a dead channel grows one entry per reminder.
• The backoff has no jitter.
• A permanent refusal is remembered for the process lifetime, so a reminder whose channel the user then fixes stays refused until a restart. A TTL on refused would fix it.
• The list is closed: a new transient Slack code would land in permanent until someone adds it. The comment says so; defaulting an unknown name to retryable would invert the risk.

Risk: T3 (core/src/host/notify.js is T2 on path and 333 lines / 3 files is over the T2 limit), so this needs a maintainer APPROVED review at this tip. Added awaiting-approval; not enqueued.

[Birfy]

Verdict: PASS at 93b3e5a
Written-by: Reviewer

The tip moved (rebase onto cbce2b03), so the PASS at f10d427b is void and this judges the new head. The rebase is content-neutral for the change: core/src/host/notify.js, core/test/notify.test.js and both patches/ds-bot/files/ copies are byte-identical to the reviewed tip, and the only difference is docs/subsystems/core-host.md, where this PR's section now sits next to #681's and #642's without overlap.

The adapter is sound on its own terms: one key per reminder with dedupe, an in-flight join, at most three attempts, exponential backoff floored by Retry-After, no retry for a structural failure, and an enabled check before every call so a channel switched off while a reminder waits out a backoff makes no transport call. Nothing wires it up yet (that is #355's second half, schedules.js), and the docs say so.

Ran: node --import ./test/harness-resolve.mjs --test --test-timeout=90000 --test-concurrency=6 "test/*.test.js" (in core/, TZ=UTC DSH_HARNESS=/opt/flybot)

• tip 93b3e5a3: 1076 tests, 1047 pass, 29 fail.
• main cbce2b03, same command: 1064 tests, 1035 pass, 29 fail. The failing sets are identical (diff clean), so the 12 new tests all pass and nothing else regressed; the 29 are the local esbuild/git misses this machine always has.

Ran: node --import ./test/harness-resolve.mjs --test test/notify.test.js

• 12 tests, 12 pass, 0 fail.

Ran: GET /repos/ThinkFlowLab/FlyBot/compare/cbce2b03...93b3e5a

• 1 commit ahead, 0 behind, 5 files, +654/−0; mergeable: true, mergeable_state: clean.

Ran: GET /repos/ThinkFlowLab/FlyBot/commits/93b3e5a3/check-runs

• 9 checks, all completed/success (tests-ok, merge-gate, package, plugins (social|timeline|tree), ubuntu-latest, reviewed tip, risk tier).

Ran (red without the source change), each on a copy of this tip with the tests kept:

• deleted core/src/host/notify.js → notify.test.js cannot import it and fails.
• removed the sent.has(key) dedupe and the inFlight.has(key) join from send() → 9 of the 12 red (the two dedupe cases first, then the neighbours cancelled by their hanging transports).
• replaced the Retry-After floor (Math.max(backoff(attempt), answer.retryAfterMs ?? 0)) with the plain backoff → exactly 1 red, a rate limit waits what Retry-After asks, then sends once.

Not run:

• The real Slack transport does not exist in this half, so nothing here touched a network or a credential.
• scripts/build-client.mjs --check and the patch-series check need esbuild/git, which this machine lacks; the package job covers both at this tip, including "ds-bot/ matches the pin, patches apply, core/ is ds-bot/ + patches".

Not blocking, one line each:

• refused grows without bound, unlike sent (capped at 1000): a long-lived host keeps one entry per permanently failed key forever, and such a key is never retried even if the channel is later repaired under the same notifier. A cap, or a note that the notifier is rebuilt when the channel changes, would say which of the two is intended.
• A transport that never settles parks that key for good: inFlight is cleared only in .finally, and setEnabled(false) cannot stop a call already awaiting the transport. The real transport is where a timeout belongs, but the adapter could defend with one, because a hung HTTP call is exactly how a dead link looks.
• readAnswer treats { ok: true } without a ts as a network failure and retries it three times. That is right for Slack, which always returns ts, but a future transport that answers ok without one looks like an outage; the comment above readAnswer is a good place to say so.