feat(discord): connector pilot for the Sage seat, reviewed candidate (#1509)
Zero-dependency Discord connector under packages/discord: binding
validation, REST and gateway clients, pi engine adapter, journal with
append-only inbox, outbox, admissions and notices, and a run.lock
ownership record {pid, start, boot} whose identity is checked three ways
and whose cleanup is gated by STOP. CLI check|run|stop|unlock via
scripts/discord.sh; offline suite scripts/test-discord.sh (28 checks,
87 node tests).
Reviewed by rev-code-02 on #1509 over nine rounds; approved exact tree
4e0feb6758c0a7e4a71483912a8e0d3e3ec95aef at comment 26170. Corrections
(1) to (12) recorded in BUILD-LOG. No listener started, no token read,
no Discord write; the live pilot follows this commit per the brief.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
This commit is contained in:
+114
@@ -2240,3 +2240,117 @@ line 21 whitespace was preserved byte-identically; other whitespace checks pass.
|
||||
QUEUE and CURRENT now name Jason-owned Gate E, Monday 2026-09-14. Issue stays
|
||||
open. No live-seat exchange, deployment, unrelated staging or new queue execution.
|
||||
Receipt: docs/plans/reviews/2026-09-13_webui-publication.md.
|
||||
|
||||
## 2026-09-13 — Discord connector pilot, candidate built (#1509)
|
||||
|
||||
Before: brief `docs/plans/2026-09-13_discord-connector-pilot.md` with Q1-Q27
|
||||
ruled by Jason; no code. Sage's private Shared Signals records (bot, guild,
|
||||
channel ids, token file) exist outside the repository and stay there.
|
||||
|
||||
After: `packages/discord` (plain ESM, zero dependencies, Node 24+): strict
|
||||
binding schema loaded fail-closed from `<dataRoot>/discord/<name>.json` with a
|
||||
0600 token file read at run time only; pure authorize step (guild, author,
|
||||
webhook, bot, user allowlist, channel or thread parent allowlist, mention mode);
|
||||
gateway v10 client with identify, heartbeat jitter, resume on op 7 and op 9,
|
||||
fatal close codes 4004/4010-4014; REST create-message with `enforce_nonce`,
|
||||
empty `allowed_mentions`, reply reference, 429 retry, 5xx as `unknown`;
|
||||
pi 0.85.1 RPC engine with follow-up queueing and per-turn timeout; journal
|
||||
with inbox, outbox intent/receipt, drops, write-once turn records, STOP file
|
||||
and pid; connector loop with duplicate suppression, daily ceiling, chunked
|
||||
delivery under one nonce per chunk, and start-time reconcile that re-sends an
|
||||
unresolved intent only inside Discord's dedupe window. `scripts/discord.sh
|
||||
check|run|stop`, `scripts/test-discord.sh` (28 checks including 74 node tests
|
||||
over fake gateway, REST and pi), `agents/sage/DISCORD-USER.md` (reduced profile,
|
||||
Q14), package README, `docs/TOOLS.md` section, AGENTS.md suites list.
|
||||
|
||||
Verification: `scripts/test-discord.sh` 28/28 (74 node tests); test-config 24, test-auth 15,
|
||||
test-release 14, test-conductor 17, test-foundation 43, seat and ledger node
|
||||
suites all pass. test-task.sh (Docker) not run; nothing in it touches this piece.
|
||||
|
||||
Corrections found by the offline tests before any live traffic: (1) after a
|
||||
timeout or abort the engine attributed the aborted `turn_end` to the next
|
||||
prompt, and a malformed RPC line had the same effect; client-failed turns now
|
||||
stay queued, marked done, until their own `turn_end` arrives. (2) a stale
|
||||
`agent_settled` from a previous run failed a freshly sent prompt; settle now
|
||||
only fails turns whose prompt pi has acknowledged. (3) Q25 named
|
||||
`scripts/verify.sh` as the hook; that script is the Docker hello check, so the
|
||||
suite is its own script instead (brief section 10). (4) found by a code-review
|
||||
subagent before the fleet review: the daily ceiling counted only written turn
|
||||
records, so a burst arriving while turns were still running could queue past
|
||||
the cap; admitted turns now count until their record is written, with a test
|
||||
that fails on the old code. (5) rev-code-02's review (#1509 comment 26114,
|
||||
REQUEST CHANGES) found four more: a retried unknown delivery refreshed the
|
||||
five-minute reconcile window (now measured from the immutable first outbox
|
||||
line for the nonce); context files accepted absolute paths, `..` and symlinks
|
||||
(now repository-relative, symlink-free and realpath-contained); the ceiling
|
||||
lost in-flight turns across a crash (admissions are now appended to
|
||||
`admissions.jsonl` before the engine runs and counted from there); and
|
||||
`run.pid` was a check-then-write with no process identity (now an O_EXCL
|
||||
claim recording the /proc start time, and `stop` signals only a matching live
|
||||
owner). Each has a test that fails on the old code. Also found by the first
|
||||
review and fixed alongside: `run.pid` is cleared if `run` fails before the
|
||||
connector starts.
|
||||
(6) round two (comment 26121) kept F4 open: the O_EXCL claim still had a
|
||||
publication window between create and write, and an empty file read as
|
||||
stale. Ownership is now a `run.lock` directory (atomic mkdir, owner record
|
||||
renamed in, an incomplete claim is busy, a stale one is renamed aside rather
|
||||
than deleted), `run` refuses when its own /proc start marker is unreadable,
|
||||
and a four-process race test proves one winner. Two more from the same
|
||||
round: the daily ceiling notice was memory-only and repeated after a same-day
|
||||
restart (now a `notices.jsonl` line written before the attempt, with a
|
||||
restart test); a duplicate gateway event during the awaited thread lookup
|
||||
was admitted twice (the id is now reserved across the lookup, with a
|
||||
held-lookup test). Suite after round two: 28/28, 80 node tests.
|
||||
(7) round three (comment 26123) reproduced a three-party handoff in the
|
||||
automatic stale reclaim: a delayed reclaimer renamed away a newer live lock
|
||||
and claimed, leaving two live owners. Automatic reclaim is removed. `run`
|
||||
refuses any lock it does not own; a new `unlock` command clears a lock whose
|
||||
owner is verified gone and restores it if the lock changed under it. Tests
|
||||
cover the stale handoff schedule and the changed-under-unlock case. Suite
|
||||
after round three: 28/28, 82 node tests.
|
||||
(8) round four (comment 26132) showed `unlock`'s move-aside-then-restore
|
||||
still lets a claim land in the empty interval and leaves two owners.
|
||||
Compare-then-restore is gone. `unlock` writes `STOP` before inspecting the
|
||||
lock, and every claim re-checks `STOP` after publishing and releases itself
|
||||
if present; `STOP` is the quiescence gate. Tests: the reviewer's four-party
|
||||
schedule, a gated race with no survivor, and the earlier stale handoff. Suite
|
||||
after round four: 28/28, 82 node tests.
|
||||
(9) round five (comment 26150): the owner record lacked a boot identity, so
|
||||
a pid and start-tick pair could recur after a reboot and `stop` could signal
|
||||
an unrelated process; and a live pid with unreadable identity was treated as
|
||||
stale. The record now carries the boot id, identity is three-way (dead or
|
||||
mismatched is stale, unreadable refuses everything, full match is live), and
|
||||
a claimant without readable identity refuses. Also the `unlock` refusal
|
||||
promised an automatic exit on STOP that does not exist; it now says to use
|
||||
`stop`. Tests: different boot id, live pid with unreadable identity. Suite
|
||||
after round five: 28/28, 83 node tests.
|
||||
(10) round six (comment 26158): a live pid whose record lacked start or boot
|
||||
(a round-five record under an upgraded binary) was classed as a positive
|
||||
mismatch, so `unlock` removed the lock under a running connector and a second
|
||||
`run` could claim; and a present but unparseable owner.json read as absent
|
||||
and was cleared the same way. Now alive plus a record without start or boot is
|
||||
"unknown" (never signaled, removed, or claimed over; clears only once the pid
|
||||
is dead), and an unreadable record is "invalid" (refuses everything, names
|
||||
the file). Tests: legacy upgrade with a live child holding {pid, start}
|
||||
(byte-identical lock, no signal target, no second owner, clears after exit)
|
||||
and an invalid-record control across four malformed contents. Suite after
|
||||
round six: 28/28, 85 node tests.
|
||||
(11) round seven (comment 26165): a parseable record whose start or boot was
|
||||
a string of the wrong shape (empty, non-decimal, not a uuid) still compared
|
||||
against /proc and read as a positive mismatch, so `unlock` could remove a
|
||||
lock under a live pid. A start tick is now a decimal digit string and a boot
|
||||
id a lowercase uuid; anything else reads as absent, which under a live pid is
|
||||
"unknown" and refuses everything. The reused-pid test now uses a valid but
|
||||
different decimal start. Tests: six live-child controls for malformed start
|
||||
and boot strings, a syntax table. Suite after round seven: 28/28, 87 node
|
||||
tests.
|
||||
(12) round eight (comment 26168): the start syntax accepted any digit string,
|
||||
so a leading-zero or above-2^64 value the kernel cannot print still compared
|
||||
as a positive mismatch under a live pid. A start tick is now the canonical
|
||||
unsigned decimal /proc prints: no leading zeros, never zero, at most 2^64-1.
|
||||
Tests: boundary controls in the syntax table and three more live-child cases
|
||||
(leading zeros, zero, above range). Suite after round eight: 28/28, 87 node
|
||||
tests.
|
||||
|
||||
Not done: no listener started, no Discord write, no token read, no binding
|
||||
created, no push. Live pilot (brief section 6) follows the reviewed commit.
|
||||
|
||||
Reference in New Issue
Block a user