From 16a639266726d7296b5a1e6e93eac6691791d532 Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Wed, 7 Oct 2026 05:17:23 -0400 Subject: [PATCH 01/17] fix(surgery): return once marks are scheduled, not after Discord accepts them Field incident (brosefo's household, 2026-10-07): a web-UI rollback removed 918 Discord messages from a resident's context. rollbackToMessage awaited syncDiscordAwarenessMarkers() inside reserveStoreForSurgery, so the serial reaction drain (~1/s at Discord's pace) held the store-wide reservation and the operator's surgery-result for its whole length; the web client gave up after 60 s while the branch had already switched. A composed fixture against the shipped 0.21.0 source reproduced it: the rollback promise resolved only after the last add_reaction, and channel traffic sent during the drain was answered only after it. Now both live surgeries split body from marks: - The reservation covers fork/switch (or redaction), durable batch activation, and any source restoration on error. After the body change lands, activateSurgeryMarkers() records the batch active and returns a SurgeryMarkerReceipt: none | queued (count, batchId) | not-scheduled. - deliverDiscordAwarenessInBackground() calls syncDiscordAwarenessMarkers() without awaiting it, before release(). The MCPL data-plane gate is installed synchronously inside it, so MCPL-borne wakes stay behind the gate exactly as before; only the response and the store reservation stop waiting on remote reactions. Failures are logged and raised as an ops alert ('discord-awareness-delivery'); the gate's own failure handling (connection recycle) is unchanged. - A ledger failure in activate() after the body change no longer fails an applied rollback (previously a raw throw after the switch, logged as an error) or undoes a fully redacted suppression (previously activate() sat in the redaction try block, so it retired the batch and restored the source). The batch is retired so the reconciliation that delivery runs next cannot promote it behind a "not scheduled" receipt; if retiring fails too, the receipt names the batch left prepared. - `markers` is returned by both surgeries, by message-granular host/command undo, recorded in operator-actions.jsonl, and printed on the [operator] stderr line. It is a scheduling statement, never a delivery claim. Agreed seam: Kit-1008 and Agnes-1009 (shelf-380 body contract, room-203 #41462/#41558) stack their safe-boundary lease on this split; room-225's marks contract (opt-in scope, one-shot, background delivery that does not gate MCPL traffic) is a separate, pending change. Tests: test/surgery-marks-receipt.test.ts (6) with a fake Discord MCPL server that withholds reaction replies. Red on origin/main 64c480b: 0/6 (rollback still pending after 3 s with reactions held; activation failures throw; suppression restored; no markers field). Green: 6/6. Related files (operator-surgery, discord-awareness-outbox, mcpl-awareness-barrier, offline-recovery-branch, host-quiesce, puppet-turn-reservation): 87/87. Co-Authored-By: Claude Opus 5.5 --- .../fix-surgery-marks-receipt.fixed.md | 13 + src/framework.ts | 133 +++++++- src/index.ts | 7 +- src/operator-log.ts | 21 ++ test/fixtures/surgery-marks-mcpl-server.mjs | 113 +++++++ test/surgery-marks-receipt.test.ts | 290 ++++++++++++++++++ 6 files changed, 568 insertions(+), 9 deletions(-) create mode 100644 changelog.d/fix-surgery-marks-receipt.fixed.md create mode 100644 test/fixtures/surgery-marks-mcpl-server.mjs create mode 100644 test/surgery-marks-receipt.test.ts diff --git a/changelog.d/fix-surgery-marks-receipt.fixed.md b/changelog.d/fix-surgery-marks-receipt.fixed.md new file mode 100644 index 000000000..ae2c47d1e --- /dev/null +++ b/changelog.d/fix-surgery-marks-receipt.fixed.md @@ -0,0 +1,13 @@ +- Live `rollbackToMessage` and `suppressMessages` (and message-granular + `host/command` undo, which rides on rollback) return once the body change + has landed and the Discord awareness batch is durably active, instead of + after every 💤 reaction has been delivered. Previously the store stayed + reserved, and the operator got no answer, for the whole serial drain: about + 15 minutes for a rollback that removed 918 channel messages. Delivery now + continues in the background, behind the same MCPL data-plane gate as + before. The result's new `markers` field (`none` | `queued` | + `not-scheduled`) says whether marks were scheduled, never that Discord + accepted them. If the ledger write after the switch fails, the result + reports `not-scheduled` and retires the batch rather than failing a + rollback that already applied, and a suppression whose redactions all + committed is no longer undone by that failure. diff --git a/src/framework.ts b/src/framework.ts index c527c488f..3f100dcce 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -406,12 +406,14 @@ import { DiscordAwarenessOutbox, defaultDiscordAwarenessOutboxPath, extractDiscordAwarenessRefs, + type DiscordAwarenessBatch, } from './recovery/discord-awareness-outbox.js'; import { OperatorActionError, OperatorLog, capIds, defaultOperatorLogPath, + type SurgeryMarkerReceipt, type OperatorLogEntry, type OperatorLogInput, type OperatorRequester, @@ -717,6 +719,14 @@ function bodyGroupRun( }; } +function describeMarkers(m: SurgeryMarkerReceipt): string { + if (m.status === 'queued') return `queued:${m.queued}`; + if (m.status === 'not-scheduled') { + return m.batchId ? `not-scheduled(batch ${m.batchId} left prepared)` : 'not-scheduled(batch retired)'; + } + return 'none'; +} + function describeRequester(r: OperatorRequester | undefined): string { if (!r) return 'unknown'; return `${r.name ?? r.id ?? 'unknown'} (${r.via})`; @@ -5068,6 +5078,9 @@ export class AgentFramework { /** Discord addresses removed by message-granular undo. The durable outbox * owns eventual delivery; this is also returned for immediate surfaces. */ removedRefs?: Array<{ serverId: string; channelId: string; messageId: string }>; + /** For message-granular undo: whether awareness marks were scheduled + * (not whether Discord has accepted them). */ + markers?: SurgeryMarkerReceipt; hidden?: number; /** For `hide`: the Discord (channelId, messageId) of each removed message * that carried one — so the surface can mark them with a reaction. */ @@ -5311,6 +5324,7 @@ export class AgentFramework { ok: true, messagesRemoved: r.messagesRemoved, removedRefs: r.removedRefs, + markers: r.markers, lastVisible: r.lastVisible, }; } catch (error) { @@ -5942,6 +5956,11 @@ export class AgentFramework { * Discord messages that left the live context get awareness markers via the * durable outbox, exactly as message-granular `undo` does. * + * Returns once the switch has landed and the marker batch is durably active + * (`markers`), without waiting for Discord to accept the reactions: delivery + * runs after the store reservation is released, behind the MCPL data-plane + * gate, and its per-message outcomes stay in the outbox ledger. + * * Throws `OperatorActionError` (`agent-busy`, `unknown-message`, …) — the * agent must be idle; nothing is queued. */ @@ -5964,6 +5983,8 @@ export class AgentFramework { * the group's last shard so the sharded message stays whole. */ tailMessageId: string; removedRefs: Array<{ serverId: string; channelId: string; messageId: string }>; + /** Whether awareness marks were scheduled; never a delivery claim. */ + markers: SurgeryMarkerReceipt; lastVisible: { participant?: string; role?: string; preview?: string } | null; }> { const logBase: OperatorLogInput = { @@ -6001,9 +6022,11 @@ export class AgentFramework { } // Gate + reserve the whole store (see reserveStoreForSurgery); held - // until the switch has landed or been rolled back. + // until the switch has landed and its marker batch is durably active, + // or the switch has been rolled back. Remote delivery is not waited on. const release = this.reserveStoreForSurgery('roll back', agentName); let branchName: string; + let markers: SurgeryMarkerReceipt; try { // Prepare the external side effect before switching Chronicle. If the // process dies after the switch but before activate(), startup @@ -6034,8 +6057,13 @@ export class AgentFramework { { cause: error }, ); } - if (markerBatch) this.discordAwarenessOutbox!.activate(markerBatch.id); - await this.syncDiscordAwarenessMarkers(); + // The body change has landed. Nothing after this point turns the + // rollback into a failure: marks are an outbox outcome, reported + // apart from the body (`markers`). + markers = this.activateSurgeryMarkers(markerBatch, 'rollback', agentName); + // Installs the MCPL data-plane gate synchronously, before the store is + // released; the drain itself runs after this call returns. + this.deliverDiscordAwarenessInBackground('rollback', agentName); this.materializeConfigMountAfterBranchSwitch(); } finally { release(); @@ -6044,7 +6072,8 @@ export class AgentFramework { console.error( `[operator] rollback agent=${agentName} to=${tailMessageId}` + `${tailMessageId !== opts.messageId ? ` (requested ${opts.messageId}, snapped to end of body group)` : ''}` + - ` removed=${messagesRemoved} branch=${branchName} by=${describeRequester(opts.requester)}`, + ` removed=${messagesRemoved} branch=${branchName} marks=${describeMarkers(markers)}` + + ` by=${describeRequester(opts.requester)}`, ); this.recordOperatorAction({ ...logBase, @@ -6054,6 +6083,7 @@ export class AgentFramework { tailMessageId, messagesRemoved, discordRefs: removedRefs.length, + markers, }, }); return { @@ -6063,6 +6093,7 @@ export class AgentFramework { messagesRemoved, tailMessageId, removedRefs, + markers, lastVisible: await this.lastVisiblePreview(agentName), }; } catch (error) { @@ -6078,6 +6109,8 @@ export class AgentFramework { * removed together. Discord originals get awareness markers via the outbox; * the batch is `explicit`-activated only after every removal succeeded, and * a crash mid-way is finished at next boot (resumePreparedDiscordSuppressions). + * Like rollbackToMessage, it returns once the body change and the batch's + * activation are durable (`markers`); delivery runs in the background. * * Not retroactive over derived state: a message already folded into an * autobiographical summary stays in that summary — roll back to before it @@ -6099,6 +6132,8 @@ export class AgentFramework { messagesRemoved: number; removedIds: string[]; removedRefs: Array<{ serverId: string; channelId: string; messageId: string }>; + /** Whether awareness marks were scheduled; never a delivery claim. */ + markers: SurgeryMarkerReceipt; lastVisible: { participant?: string; role?: string; preview?: string } | null; }> { const requestedIds = [...new Set(opts.messageIds.map(String))]; @@ -6151,9 +6186,11 @@ export class AgentFramework { throw new OperatorActionError('invalid', 'Suppression branch name must differ from the active branch'); } // Gate + reserve the whole store (see reserveStoreForSurgery); held - // until the fork is fully redacted or the source is restored. + // until the fork is fully redacted and its marker batch durably + // active, or the source is restored. Remote delivery is not waited on. const release = this.reserveStoreForSurgery('suppress', agentName); let createdBranch: string; + let markers: SurgeryMarkerReceipt; try { const markerBatch = this.discordAwarenessOutbox?.prepare({ agentName, @@ -6194,7 +6231,6 @@ export class AgentFramework { if (iv.group || iv.fromId !== iv.toId) cm.removeMessages(iv.fromId as MessageId, iv.toId as MessageId); else cm.removeMessage(iv.fromId as MessageId); } - if (markerBatch) this.discordAwarenessOutbox!.activate(markerBatch.id); } catch (error) { // A partially suppressed branch is not safe to serve from. Keep it // for diagnosis, put the agent back on the untouched source — and @@ -6207,7 +6243,16 @@ export class AgentFramework { { cause: error }, ); } - await this.syncDiscordAwarenessMarkers(); + // Every redaction committed: the body change has landed and the fork + // is safe to serve from. Activating the marker batch is bookkeeping + // about a world-side effect; if it fails, the suppression still + // stands and the receipt says the marks were not scheduled. (No + // interval needs resuming any more, so retiring the batch there + // loses nothing.) + markers = this.activateSurgeryMarkers(markerBatch, 'suppress', agentName); + // Installs the MCPL data-plane gate synchronously, before the store is + // released; the drain itself runs after this call returns. + this.deliverDiscordAwarenessInBackground('suppress', agentName); this.materializeConfigMountAfterBranchSwitch(); } finally { release(); @@ -6215,7 +6260,7 @@ export class AgentFramework { console.error( `[operator] suppress agent=${agentName} removed=${messagesRemoved} branch=${createdBranch} ` + - `by=${describeRequester(opts.requester)}`, + `marks=${describeMarkers(markers)} by=${describeRequester(opts.requester)}`, ); const loggedIds = capIds(removedIds); this.recordOperatorAction({ @@ -6227,6 +6272,7 @@ export class AgentFramework { removedIds: loggedIds.ids, ...(loggedIds.truncated ? { removedIdsTruncated: true } : {}), discordRefs: removedRefs.length, + markers, }, }); return { @@ -6236,6 +6282,7 @@ export class AgentFramework { messagesRemoved, removedIds, removedRefs, + markers, lastVisible: await this.lastVisiblePreview(agentName), }; } catch (error) { @@ -6244,6 +6291,76 @@ export class AgentFramework { } } + /** + * Record a surgery's prepared marker batch as active once its body change + * has landed, and say what happened. Never throws: the body is already + * applied, so a ledger failure here is reported as `not-scheduled` rather + * than as a failed surgery an operator might retry. A batch that could not + * be activated is retired, so that the reconciliation which delivery runs + * next cannot promote it behind a receipt saying it was not scheduled; only + * when retiring fails too does it stay prepared (named by `batchId`). A + * batch with no refs (a suppression journal with nothing addressable) is + * still activated, which closes its resume journal; it schedules no marks. + */ + private activateSurgeryMarkers( + batch: DiscordAwarenessBatch | null, + verb: 'rollback' | 'suppress', + agentName: string, + ): SurgeryMarkerReceipt { + if (!batch) return { status: 'none', queued: 0 }; + try { + this.discordAwarenessOutbox!.activate(batch.id); + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + let retired = false; + let retireDetail = ''; + try { + retired = this.discordAwarenessOutbox!.discard(batch.id); + } catch (discardError) { + retireDetail = discardError instanceof Error ? discardError.message : String(discardError); + } + console.error( + `[discord-awareness] ${verb} agent=${agentName}: body applied, but marker batch ${batch.id} ` + + `could not be activated (${detail}); ` + + (retired ? 'batch retired' : `batch left prepared${retireDetail ? ` (retire failed: ${retireDetail})` : ''}`), + ); + if (batch.refs.length === 0) return { status: 'none', queued: 0 }; + this.opsAlert( + 'discord-awareness-not-scheduled', + agentName, + `${verb} applied, but its ${batch.refs.length} awareness mark(s) were not scheduled: ${detail}`, + { data: { batchId: batch.id, retired } }, + ); + return retired + ? { status: 'not-scheduled', queued: 0, error: detail } + : { status: 'not-scheduled', queued: 0, error: detail, batchId: batch.id }; + } + if (batch.refs.length === 0) return { status: 'none', queued: 0 }; + return { status: 'queued', queued: batch.refs.length, batchId: batch.id }; + } + + /** + * Hand awareness delivery to the background, without holding the caller. + * `syncDiscordAwarenessMarkers` installs the MCPL data-plane gate + * synchronously, before its first await, so calling this while a surgery + * still holds the store keeps MCPL-borne wakes behind the gate exactly as + * before. Only the surgery's response and its store reservation stop + * waiting on remote reactions. A failure is accounted by the gate itself + * (connections recycled); here it is logged and raised as an ops alert. + */ + private deliverDiscordAwarenessInBackground(verb: 'rollback' | 'suppress', agentName: string): void { + if (!this.discordAwarenessOutbox) return; + this.syncDiscordAwarenessMarkers().catch((error) => { + const detail = error instanceof Error ? error.message : String(error); + console.error(`[discord-awareness] delivery after ${verb} agent=${agentName} failed: ${detail}`); + this.opsAlert( + 'discord-awareness-delivery', + agentName, + `Awareness mark delivery after ${verb} failed: ${detail}`, + ); + }); + } + /** Re-materialize config files from the (new) active branch — fire and * forget; the gate picks changes up via mtime. Mirrors undoLastTurn. */ private materializeConfigMountAfterBranchSwitch(): void { diff --git a/src/index.ts b/src/index.ts index bd07d3547..939a4ac56 100644 --- a/src/index.ts +++ b/src/index.ts @@ -142,7 +142,12 @@ export type { } from './recovery/discord-awareness-outbox.js'; // Live operator surgery (rollback / suppress) and its durable action log export { OperatorLog, OperatorActionError, defaultOperatorLogPath } from './operator-log.js'; -export type { OperatorLogEntry, OperatorLogInput, OperatorRequester } from './operator-log.js'; +export type { + OperatorLogEntry, + OperatorLogInput, + OperatorRequester, + SurgeryMarkerReceipt, +} from './operator-log.js'; export { createOfflineRecoveryBranch } from './recovery/offline-branch.js'; export type { OfflineRecoveryBranchOptions, diff --git a/src/operator-log.ts b/src/operator-log.ts index 710e6b505..48de210f0 100644 --- a/src/operator-log.ts +++ b/src/operator-log.ts @@ -118,6 +118,27 @@ export function capIds(ids: string[]): { ids: string[]; count: number; truncated : { ids, count: ids.length }; } +/** + * What a live surgery did about Discord awareness marks, reported apart from + * the body change itself. A surgery that returns has applied its body change; + * this says only whether marks were scheduled: + * + * - `none` — the change removed no addressable Discord message. + * - `queued` — the outbox batch is durably active and its delivery has been + * handed to the background drain. It is not a delivery claim: per-message + * outcomes stay in the outbox ledger. + * - `not-scheduled` — the body change landed, but recording the batch as + * active failed, so no mark will be delivered for it: the batch is retired + * from the ledger. Only if retiring it failed as well is `batchId` present: + * that batch stays prepared, and a later reconciliation (or, for a + * suppression, the next startup) that can write the ledger may still + * promote it. + */ +export type SurgeryMarkerReceipt = + | { status: 'none'; queued: 0 } + | { status: 'queued'; queued: number; batchId: string } + | { status: 'not-scheduled'; queued: 0; error: string; batchId?: string }; + /** Thrown by live surgery methods when a request cannot be honored. `code` * lets surfaces distinguish "agent busy — quiesce first" from bad input. */ export class OperatorActionError extends Error { diff --git a/test/fixtures/surgery-marks-mcpl-server.mjs b/test/fixtures/surgery-marks-mcpl-server.mjs new file mode 100644 index 000000000..9c5314901 --- /dev/null +++ b/test/fixtures/surgery-marks-mcpl-server.mjs @@ -0,0 +1,113 @@ +// Fake Discord MCPL server for test/surgery-marks-receipt.test.ts. +// +// Every tools/call is appended to CALLS_PATH. While HOLD_PATH is set and that +// file does not exist, replies are withheld, standing in for Discord pacing a +// long reaction drain. When PROBE_PATH appears, the server sends one +// inference-bearing channels/incoming request and appends to EVENTS_PATH when +// the host answers it, so a test can see whether the MCPL data plane was held. +import { appendFileSync, existsSync } from 'node:fs'; + +const callsPath = process.env.CALLS_PATH; +const eventsPath = process.env.EVENTS_PATH; +const holdPath = process.env.HOLD_PATH; +const probePath = process.env.PROBE_PATH; +const channelId = process.env.CHANNEL_ID ?? 'discord:g1:c1'; + +const send = (message) => process.stdout.write(JSON.stringify(message) + '\n'); +const reply = (id, result) => send({ jsonrpc: '2.0', id, result }); +const request = (id, method, params) => send({ jsonrpc: '2.0', id, method, params }); +const event = (name, extra = {}) => { + if (eventsPath) appendFileSync(eventsPath, JSON.stringify({ event: name, at: Date.now(), ...extra }) + '\n'); +}; + +const held = []; +let probed = false; +let buf = ''; + +function answer(message) { + event('reaction-answered', { messageId: message.params?.arguments?.messageId }); + reply(message.id, { content: [{ type: 'text', text: 'Reaction applied' }] }); +} + +function handle(message) { + if (message.method === 'initialize') { + reply(message.id, { + capabilities: { + experimental: { + mcpl: { + version: '0.5', + pushEvents: true, + channels: { register: true, incoming: true }, + featureSets: { chat: { description: 'chat', uses: ['pushEvents'] } }, + }, + }, + }, + }); + return; + } + if (message.method === 'featureSets/update') { + if (message.id !== undefined && message.id !== null) reply(message.id, { accepted: true }); + return; + } + if (message.method === 'notifications/initialized') { + request(100, 'channels/register', { + channels: [{ id: channelId, type: 'discord', label: 'busy', direction: 'bidirectional' }], + }); + return; + } + if (message.method === 'tools/list') { + reply(message.id, { + tools: [ + { name: 'add_reaction', description: 'add', inputSchema: { type: 'object' } }, + { name: 'remove_reaction', description: 'remove', inputSchema: { type: 'object' } }, + ], + }); + return; + } + if (message.method === 'tools/call') { + if (callsPath) { + appendFileSync(callsPath, JSON.stringify({ at: Date.now(), name: message.params?.name, args: message.params?.arguments }) + '\n'); + } + if (holdPath && !existsSync(holdPath)) held.push(message); + else answer(message); + return; + } + if (message.id === 900) { + event('probe-answered', { error: message.error ?? null }); + } +} + +process.stdin.on('data', (chunk) => { + buf += chunk.toString('utf8'); + let newline; + while ((newline = buf.indexOf('\n')) >= 0) { + const line = buf.slice(0, newline); + buf = buf.slice(newline + 1); + if (!line.trim()) continue; + try { + handle(JSON.parse(line)); + } catch (error) { + event('server-error', { message: error instanceof Error ? error.message : String(error) }); + } + } +}); + +const timer = setInterval(() => { + if (held.length > 0 && holdPath && existsSync(holdPath)) { + for (const message of held.splice(0)) answer(message); + } + if (!probed && probePath && existsSync(probePath)) { + probed = true; + event('probe-sent'); + request(900, 'channels/incoming', { + messages: [{ + channelId, + messageId: 'probe-during-delivery', + author: { id: 'bystander', name: 'Bystander' }, + timestamp: new Date().toISOString(), + content: [{ type: 'text', text: 'ordinary traffic while marks are delivered' }], + }], + }); + } +}, 5); +timer.unref(); diff --git a/test/surgery-marks-receipt.test.ts b/test/surgery-marks-receipt.test.ts new file mode 100644 index 000000000..13e8793dd --- /dev/null +++ b/test/surgery-marks-receipt.test.ts @@ -0,0 +1,290 @@ +/** + * Live surgery reports its body change apart from the Discord awareness marks + * it schedules, and does not wait on Discord to accept them. + * + * Field incident (2026-10-07): a web-UI rollback removed 918 Discord messages + * from a resident's context. rollbackToMessage awaited the serial reaction + * drain inside its store reservation, so the operator's surgery-result never + * arrived within the client's 60 s, the store stayed reserved, and every MCPL + * data plane stayed paused for the whole drain. These tests hold the fake + * server's reaction replies to model that drain. + * + * The receipt is a scheduling statement (`markers`), never a delivery claim, + * and a ledger failure after the body change landed is reported as + * `not-scheduled` instead of a failed surgery an operator might retry. + */ +import { describe, it, beforeEach, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { fileURLToPath } from 'node:url'; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { AgentFramework, DiscordAwarenessOutbox } from '../src/index.js'; +import type { OperatorLogEntry } from '../src/index.js'; +import { MockMembrane } from './helpers/mock-membrane.js'; + +const FIXTURE = fileURLToPath(new URL('./fixtures/surgery-marks-mcpl-server.mjs', import.meta.url)); + +function jsonl>(path: string): T[] { + if (!existsSync(path)) return []; + return readFileSync(path, 'utf8').split('\n').filter(Boolean).map((line) => JSON.parse(line) as T); +} + +async function waitFor(description: string, predicate: () => boolean, timeoutMs = 5_000): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (predicate()) return; + await new Promise((resolve) => setTimeout(resolve, 10)); + } + assert.fail(`timed out waiting for ${description}`); +} + +/** Resolve with the promise's value, or fail if it is still pending after ms. */ +async function settlesWithin(promise: Promise, ms: number, what: string): Promise { + let timer: ReturnType | undefined; + try { + return await Promise.race([ + promise, + new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`${what} still pending after ${ms} ms`)), ms); + }), + ]); + } finally { + if (timer) clearTimeout(timer); + } +} + +describe('surgery marker receipt', () => { + let dir: string; + let storePath: string; + let callsPath: string; + let eventsPath: string; + let holdPath: string; + let probePath: string; + let framework: AgentFramework; + + const cm = () => framework.getAgent('resident')!.getContextManager(); + const outboxPath = () => join(storePath, 'recovery', 'discord-awareness-outbox.json'); + const ledger = () => new DiscordAwarenessOutbox(outboxPath()).batches(); + + async function start(withDiscord: boolean): Promise { + framework = await AgentFramework.create({ + storePath, + membrane: new MockMembrane().asMembrane(), + agents: [{ name: 'resident', model: 'test-model', systemPrompt: 'You are a resident.' }], + modules: [], + maintenanceIntervalMs: 0, + ...(withDiscord ? { + mcplServers: [{ + id: 'discord', + command: process.execPath, + args: [FIXTURE], + env: { + CALLS_PATH: callsPath, + EVENTS_PATH: eventsPath, + HOLD_PATH: holdPath, + PROBE_PATH: probePath, + }, + enabledFeatureSets: ['chat'], + }], + } : {}), + }); + } + + /** A kept message, the rollback target, then `n` addressable messages. */ + function seed(n: number): { tail: string; removed: string[] } { + cm().addMessage('Operator', [{ type: 'text', text: 'kept' }], { + serverId: 'discord', channelId: 'discord:g1:dm', messageId: 'kept', + }); + const tail = String(cm().addMessage('Operator', [{ type: 'text', text: 'last good' }], { + serverId: 'discord', channelId: 'discord:g1:dm', messageId: 'tail', + })); + const removed: string[] = []; + for (let i = 0; i < n; i++) { + removed.push(String(cm().addMessage(`Member${i}`, [{ type: 'text', text: `ambient ${i}` }], { + serverId: 'discord', channelId: 'discord:g1:c1', messageId: `amb-${i}`, + }))); + } + return { tail, removed }; + } + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'surgery-marks-')); + storePath = join(dir, 'store'); + callsPath = join(dir, 'calls.jsonl'); + eventsPath = join(dir, 'events.jsonl'); + holdPath = join(dir, 'release-reactions'); + probePath = join(dir, 'send-probe'); + }); + + afterEach(async () => { + writeFileSync(holdPath, '1'); + await framework?.stop(); + rmSync(dir, { recursive: true, force: true }); + }); + + it('rollback returns once the batch is active, before Discord has accepted any reaction', async () => { + await start(true); + const { tail } = seed(3); + + const result = await settlesWithin( + framework.rollbackToMessage('resident', { messageId: tail }), + 3_000, + 'rollbackToMessage with reactions held', + ); + + assert.equal(result.messagesRemoved, 3); + assert.equal(result.markers.status, 'queued'); + assert.equal(result.markers.queued, 3); + const [batch] = ledger(); + assert.equal(batch.status, 'active', 'the batch is durably active before the call returns'); + assert.equal(result.markers.status === 'queued' && result.markers.batchId, batch.id); + assert.equal( + jsonl(eventsPath).filter((e) => e.event === 'reaction-answered').length, + 0, + 'no reaction had been accepted when the rollback returned', + ); + + // Delivery continues in the background and completes once Discord answers. + writeFileSync(holdPath, '1'); + await waitFor('all three reactions answered', () => + jsonl(eventsPath).filter((e) => e.event === 'reaction-answered').length === 3); + await framework.syncDiscordAwarenessMarkers(); + const entries = ledger()[0].refs; + assert.deepEqual(entries.map((ref) => ref.deliveryStatus), ['applied', 'applied', 'applied']); + assert.deepEqual( + jsonl<{ name: string; args: { emoji: string } }>(callsPath).map((call) => `${call.name}:${call.args.emoji}`), + ['add_reaction:💤', 'add_reaction:💤', 'add_reaction:💤'], + ); + + const logged = new Map( + framework.getOperatorLog({ limit: 10 }).map((entry) => [entry.kind, entry]), + ); + assert.deepEqual(logged.get('rollback')?.result?.markers, result.markers); + }); + + it('releases the store while delivery runs, but keeps MCPL traffic behind the delivery gate', async () => { + await start(true); + const { removed } = seed(4); + + await settlesWithin( + framework.rollbackToMessage('resident', { messageId: removed[1] }), + 3_000, + 'rollbackToMessage with reactions held', + ); + await waitFor('the first reaction call to reach the server', () => jsonl(callsPath).length >= 1); + + // The store reservation is released: a second surgery is admitted while + // the first one's reactions are still unanswered. + const second = await settlesWithin( + framework.suppressMessages('resident', { messageIds: [removed[0]] }), + 3_000, + 'a second surgery during delivery', + ); + assert.equal(second.messagesRemoved, 1); + assert.equal(second.markers.status, 'queued'); + + // The gate was installed before release: ordinary channel traffic sent + // during delivery is not answered until the reactions are. + writeFileSync(probePath, '1'); + await waitFor('the probe to be sent', () => jsonl(eventsPath).some((e) => e.event === 'probe-sent')); + await new Promise((resolve) => setTimeout(resolve, 200)); + assert.equal( + jsonl(eventsPath).some((e) => e.event === 'probe-answered'), + false, + 'channel traffic waits behind the delivery gate', + ); + + writeFileSync(holdPath, '1'); + await waitFor('the probe to be answered after delivery', () => + jsonl(eventsPath).some((e) => e.event === 'probe-answered')); + const events = jsonl<{ event: string; at: number }>(eventsPath); + const lastReaction = Math.max(...events.filter((e) => e.event === 'reaction-answered').map((e) => e.at)); + const probe = events.find((e) => e.event === 'probe-answered')!; + assert.ok(probe.at >= lastReaction, 'the probe is answered only after the held reactions'); + }); + + it('a rollback whose marker bookkeeping fails after the switch reports the body as applied', async () => { + await start(false); + const { tail } = seed(2); + const original = DiscordAwarenessOutbox.prototype.activate; + DiscordAwarenessOutbox.prototype.activate = function () { + throw new Error('injected ledger failure'); + }; + let result: Awaited>; + try { + result = await framework.rollbackToMessage('resident', { messageId: tail }); + } finally { + DiscordAwarenessOutbox.prototype.activate = original; + } + + assert.equal(cm().currentBranch().name, result.targetBranch, 'the rollback stands'); + assert.equal(result.messagesRemoved, 2); + assert.equal(result.markers.status, 'not-scheduled'); + assert.match(result.markers.status === 'not-scheduled' ? result.markers.error : '', /injected ledger failure/); + assert.equal(result.markers.status === 'not-scheduled' && result.markers.batchId, undefined); + // The unactivated batch is retired, so the reconciliation that delivery + // runs cannot promote it behind a receipt that said "not scheduled". + await framework.syncDiscordAwarenessMarkers(); + assert.deepEqual(ledger(), []); + + const entry = framework.getOperatorLog({ limit: 10 }).find((e) => e.kind === 'rollback'); + assert.equal(entry?.error, undefined, 'logged as an applied rollback, not a failure'); + assert.equal((entry?.result?.markers as { status?: string } | undefined)?.status, 'not-scheduled'); + }); + + it('names a batch it could neither activate nor retire, which stays prepared', async () => { + await start(false); + const { tail } = seed(2); + const activate = DiscordAwarenessOutbox.prototype.activate; + const discard = DiscordAwarenessOutbox.prototype.discard; + DiscordAwarenessOutbox.prototype.activate = function () { + throw new Error('injected ledger failure'); + }; + DiscordAwarenessOutbox.prototype.discard = function () { + throw new Error('injected retire failure'); + }; + let result: Awaited>; + try { + result = await framework.rollbackToMessage('resident', { messageId: tail }); + } finally { + DiscordAwarenessOutbox.prototype.activate = activate; + DiscordAwarenessOutbox.prototype.discard = discard; + } + assert.equal(cm().currentBranch().name, result.targetBranch); + assert.equal(result.markers.status, 'not-scheduled'); + const batches = ledger(); + assert.equal(batches.length, 1); + assert.equal(result.markers.status === 'not-scheduled' && result.markers.batchId, batches[0].id); + }); + + it('a suppression whose marker bookkeeping fails after the last redaction stands', async () => { + await start(false); + const { removed } = seed(3); + const original = DiscordAwarenessOutbox.prototype.activate; + DiscordAwarenessOutbox.prototype.activate = function () { + throw new Error('injected ledger failure'); + }; + let result: Awaited>; + try { + result = await framework.suppressMessages('resident', { messageIds: [removed[1]] }); + } finally { + DiscordAwarenessOutbox.prototype.activate = original; + } + + assert.equal(cm().currentBranch().name, result.targetBranch, 'the suppression is not undone'); + assert.deepEqual(result.removedIds, [removed[1]]); + assert.ok(!cm().getAllMessages().some((m) => String(m.id) === removed[1])); + assert.equal(result.markers.status, 'not-scheduled'); + assert.deepEqual(ledger(), [], 'every interval committed, so the retired journal loses nothing'); + }); + + it('a surgery that removes nothing addressable schedules no marks', async () => { + await start(false); + const keep = String(cm().addMessage('Operator', [{ type: 'text', text: 'kept' }], {})); + cm().addMessage('resident', [{ type: 'text', text: 'local only' }], {}); + const result = await framework.rollbackToMessage('resident', { messageId: keep }); + assert.deepEqual(result.markers, { status: 'none', queued: 0 }); + assert.equal(ledger().length, 0); + }); +}); From 82a267a81203308418136c7582d70cb34dfe6484 Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Wed, 7 Oct 2026 05:36:12 -0400 Subject: [PATCH 02/17] fix(surgery): report unresolved marker bookkeeping as unresolved Review (June-1016, room-225 #41840): when both activate() and discard() failed, the receipt said `not-scheduled` while the batch stayed prepared, and the reconciliation that the surgery's own background delivery runs promoted it at once: June's probe saw a `not-scheduled, queued: 0` receipt followed by an active ledger batch and two real add_reaction calls. The fourth test's title claimed "stays prepared" without asserting it. SurgeryMarkerReceipt now has four outcomes, each with a guarantee that holds on every later path: - `not-scheduled`: the batch was retired from the ledger, so no reconciliation or restart can deliver it (no batchId is offered). - `unresolved`: the ledger could record neither activation nor retirement; the receipt names the batch and promises nothing about it. The ops alert is 'discord-awareness-unresolved' and says it may still be delivered. - `queued` / `none` unchanged. Tests: the not-scheduled case now runs against the fake Discord server and asserts no reaction is sent across the delivery reconciliation and a full restart on the same store; the double-failure case asserts `unresolved`, names the batch still in the ledger, and makes no prepared/held claim. Focused + related files: 87/87 plus 6/6. Co-Authored-By: Claude Opus 5.5 --- .../fix-surgery-marks-receipt.fixed.md | 11 ++++--- src/framework.ts | 31 +++++++++++-------- src/operator-log.ts | 14 +++++---- test/surgery-marks-receipt.test.ts | 30 +++++++++++------- 4 files changed, 51 insertions(+), 35 deletions(-) diff --git a/changelog.d/fix-surgery-marks-receipt.fixed.md b/changelog.d/fix-surgery-marks-receipt.fixed.md index ae2c47d1e..caef410eb 100644 --- a/changelog.d/fix-surgery-marks-receipt.fixed.md +++ b/changelog.d/fix-surgery-marks-receipt.fixed.md @@ -6,8 +6,9 @@ 15 minutes for a rollback that removed 918 channel messages. Delivery now continues in the background, behind the same MCPL data-plane gate as before. The result's new `markers` field (`none` | `queued` | - `not-scheduled`) says whether marks were scheduled, never that Discord - accepted them. If the ledger write after the switch fails, the result - reports `not-scheduled` and retires the batch rather than failing a - rollback that already applied, and a suppression whose redactions all - committed is no longer undone by that failure. + `not-scheduled` | `unresolved`) says whether marks were scheduled, never + that Discord accepted them. If the ledger write after the switch fails, the + rollback that already applied is no longer reported as failed: the batch is + retired (`not-scheduled`), or, if even that write fails, the result says + the batch may still be delivered (`unresolved`). A suppression whose + redactions all committed is no longer undone by that failure. diff --git a/src/framework.ts b/src/framework.ts index 3f100dcce..f296c4ebe 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -721,9 +721,8 @@ function bodyGroupRun( function describeMarkers(m: SurgeryMarkerReceipt): string { if (m.status === 'queued') return `queued:${m.queued}`; - if (m.status === 'not-scheduled') { - return m.batchId ? `not-scheduled(batch ${m.batchId} left prepared)` : 'not-scheduled(batch retired)'; - } + if (m.status === 'not-scheduled') return 'not-scheduled(batch retired)'; + if (m.status === 'unresolved') return `unresolved(batch ${m.batchId} may still be delivered)`; return 'none'; } @@ -6294,13 +6293,15 @@ export class AgentFramework { /** * Record a surgery's prepared marker batch as active once its body change * has landed, and say what happened. Never throws: the body is already - * applied, so a ledger failure here is reported as `not-scheduled` rather - * than as a failed surgery an operator might retry. A batch that could not - * be activated is retired, so that the reconciliation which delivery runs - * next cannot promote it behind a receipt saying it was not scheduled; only - * when retiring fails too does it stay prepared (named by `batchId`). A - * batch with no refs (a suppression journal with nothing addressable) is - * still activated, which closes its resume journal; it schedules no marks. + * applied, so a ledger failure here is reported in the receipt rather than + * as a failed surgery an operator might retry. A batch that could not be + * activated is retired, so no later reconciliation can deliver it behind a + * receipt saying it was not scheduled (`not-scheduled`). If retiring fails + * too, the receipt says only that the outcome is `unresolved`: the batch is + * still prepared, and the reconciliation that delivery runs next may well + * promote it. A batch with no refs (a suppression journal with nothing + * addressable) is still activated, which closes its resume journal; it + * schedules no marks. */ private activateSurgeryMarkers( batch: DiscordAwarenessBatch | null, @@ -6326,14 +6327,18 @@ export class AgentFramework { ); if (batch.refs.length === 0) return { status: 'none', queued: 0 }; this.opsAlert( - 'discord-awareness-not-scheduled', + retired ? 'discord-awareness-not-scheduled' : 'discord-awareness-unresolved', agentName, - `${verb} applied, but its ${batch.refs.length} awareness mark(s) were not scheduled: ${detail}`, + retired + ? `${verb} applied, but its ${batch.refs.length} awareness mark(s) were not scheduled ` + + `(batch ${batch.id} retired): ${detail}` + : `${verb} applied, but awareness batch ${batch.id} (${batch.refs.length} mark(s)) could be ` + + `neither activated nor retired; it may still be delivered: ${detail}`, { data: { batchId: batch.id, retired } }, ); return retired ? { status: 'not-scheduled', queued: 0, error: detail } - : { status: 'not-scheduled', queued: 0, error: detail, batchId: batch.id }; + : { status: 'unresolved', queued: 0, batchId: batch.id, error: detail }; } if (batch.refs.length === 0) return { status: 'none', queued: 0 }; return { status: 'queued', queued: batch.refs.length, batchId: batch.id }; diff --git a/src/operator-log.ts b/src/operator-log.ts index 48de210f0..2a50106c5 100644 --- a/src/operator-log.ts +++ b/src/operator-log.ts @@ -128,16 +128,18 @@ export function capIds(ids: string[]): { ids: string[]; count: number; truncated * handed to the background drain. It is not a delivery claim: per-message * outcomes stay in the outbox ledger. * - `not-scheduled` — the body change landed, but recording the batch as - * active failed, so no mark will be delivered for it: the batch is retired - * from the ledger. Only if retiring it failed as well is `batchId` present: - * that batch stays prepared, and a later reconciliation (or, for a - * suppression, the next startup) that can write the ledger may still - * promote it. + * active failed, so it was retired from the ledger: none of its marks will + * be delivered. + * - `unresolved` — the ledger could record neither the activation nor the + * retirement. The batch (`batchId`) is still in the ledger in its prepared + * state, and any later reconciliation or restart that can read it may + * promote and deliver it; nothing here promises otherwise. */ export type SurgeryMarkerReceipt = | { status: 'none'; queued: 0 } | { status: 'queued'; queued: number; batchId: string } - | { status: 'not-scheduled'; queued: 0; error: string; batchId?: string }; + | { status: 'not-scheduled'; queued: 0; error: string } + | { status: 'unresolved'; queued: 0; batchId: string; error: string }; /** Thrown by live surgery methods when a request cannot be honored. `code` * lets surfaces distinguish "agent busy — quiesce first" from bad input. */ diff --git a/test/surgery-marks-receipt.test.ts b/test/surgery-marks-receipt.test.ts index 13e8793dd..c71ac0f31 100644 --- a/test/surgery-marks-receipt.test.ts +++ b/test/surgery-marks-receipt.test.ts @@ -205,7 +205,7 @@ describe('surgery marker receipt', () => { }); it('a rollback whose marker bookkeeping fails after the switch reports the body as applied', async () => { - await start(false); + await start(true); const { tail } = seed(2); const original = DiscordAwarenessOutbox.prototype.activate; DiscordAwarenessOutbox.prototype.activate = function () { @@ -222,18 +222,23 @@ describe('surgery marker receipt', () => { assert.equal(result.messagesRemoved, 2); assert.equal(result.markers.status, 'not-scheduled'); assert.match(result.markers.status === 'not-scheduled' ? result.markers.error : '', /injected ledger failure/); - assert.equal(result.markers.status === 'not-scheduled' && result.markers.batchId, undefined); - // The unactivated batch is retired, so the reconciliation that delivery - // runs cannot promote it behind a receipt that said "not scheduled". - await framework.syncDiscordAwarenessMarkers(); - assert.deepEqual(ledger(), []); - const entry = framework.getOperatorLog({ limit: 10 }).find((e) => e.kind === 'rollback'); assert.equal(entry?.error, undefined, 'logged as an applied rollback, not a failure'); assert.equal((entry?.result?.markers as { status?: string } | undefined)?.status, 'not-scheduled'); + + // "Not scheduled" holds across the reconciliation delivery runs and + // across a restart: the batch was retired, so nothing can promote it. + writeFileSync(holdPath, '1'); + await framework.syncDiscordAwarenessMarkers(); + assert.deepEqual(ledger(), []); + await framework.stop(); + await start(true); + await framework.syncDiscordAwarenessMarkers(); + assert.deepEqual(ledger(), []); + assert.equal(jsonl(callsPath).length, 0, 'no reaction was ever sent'); }); - it('names a batch it could neither activate nor retire, which stays prepared', async () => { + it('reports unresolved bookkeeping when the batch can be neither activated nor retired', async () => { await start(false); const { tail } = seed(2); const activate = DiscordAwarenessOutbox.prototype.activate; @@ -251,11 +256,14 @@ describe('surgery marker receipt', () => { DiscordAwarenessOutbox.prototype.activate = activate; DiscordAwarenessOutbox.prototype.discard = discard; } - assert.equal(cm().currentBranch().name, result.targetBranch); - assert.equal(result.markers.status, 'not-scheduled'); + assert.equal(cm().currentBranch().name, result.targetBranch, 'the rollback stands'); + // The receipt promises nothing about this batch: it is still in the + // ledger, and a reconciliation may promote and deliver it. + assert.equal(result.markers.status, 'unresolved'); const batches = ledger(); assert.equal(batches.length, 1); - assert.equal(result.markers.status === 'not-scheduled' && result.markers.batchId, batches[0].id); + assert.equal(result.markers.status === 'unresolved' && result.markers.batchId, batches[0].id); + assert.match(result.markers.status === 'unresolved' ? result.markers.error : '', /injected ledger failure/); }); it('a suppression whose marker bookkeeping fails after the last redaction stands', async () => { From 0254342bd8f63f7907af517f82d9bdeb2cab3cd4 Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Wed, 7 Oct 2026 05:41:04 -0400 Subject: [PATCH 03/17] docs(surgery): describe delivery as started without awaiting, not run after return Review (June-1016, room-225 #41877): syncDiscordAwarenessMarkers() starts the drain synchronously through its first awaited tools/call, so the first reaction request can be written before release() and before the caller gets its receipt. Comments that said the drain runs after the call returns, or after the reservation is released, overstated the ordering. They now say delivery is started (behind the gate) and continues without the caller or the store reservation waiting on it. Comment-only change. Co-Authored-By: Claude Opus 5.5 --- src/framework.ts | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/framework.ts b/src/framework.ts index f296c4ebe..8385cc033 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -5957,8 +5957,9 @@ export class AgentFramework { * * Returns once the switch has landed and the marker batch is durably active * (`markers`), without waiting for Discord to accept the reactions: delivery - * runs after the store reservation is released, behind the MCPL data-plane - * gate, and its per-message outcomes stay in the outbox ledger. + * is started, behind the MCPL data-plane gate, and continues without the + * caller or the store reservation waiting on it; its per-message outcomes + * stay in the outbox ledger. * * Throws `OperatorActionError` (`agent-busy`, `unknown-message`, …) — the * agent must be idle; nothing is queued. @@ -6061,7 +6062,8 @@ export class AgentFramework { // apart from the body (`markers`). markers = this.activateSurgeryMarkers(markerBatch, 'rollback', agentName); // Installs the MCPL data-plane gate synchronously, before the store is - // released; the drain itself runs after this call returns. + // released, and starts delivery without awaiting it (the first + // reaction request may already be on the wire when this returns). this.deliverDiscordAwarenessInBackground('rollback', agentName); this.materializeConfigMountAfterBranchSwitch(); } finally { @@ -6109,7 +6111,8 @@ export class AgentFramework { * the batch is `explicit`-activated only after every removal succeeded, and * a crash mid-way is finished at next boot (resumePreparedDiscordSuppressions). * Like rollbackToMessage, it returns once the body change and the batch's - * activation are durable (`markers`); delivery runs in the background. + * activation are durable (`markers`); delivery continues without the caller + * waiting on it. * * Not retroactive over derived state: a message already folded into an * autobiographical summary stays in that summary — roll back to before it @@ -6250,7 +6253,8 @@ export class AgentFramework { // loses nothing.) markers = this.activateSurgeryMarkers(markerBatch, 'suppress', agentName); // Installs the MCPL data-plane gate synchronously, before the store is - // released; the drain itself runs after this call returns. + // released, and starts delivery without awaiting it (the first + // reaction request may already be on the wire when this returns). this.deliverDiscordAwarenessInBackground('suppress', agentName); this.materializeConfigMountAfterBranchSwitch(); } finally { @@ -6345,13 +6349,15 @@ export class AgentFramework { } /** - * Hand awareness delivery to the background, without holding the caller. + * Start awareness delivery without the caller awaiting it. * `syncDiscordAwarenessMarkers` installs the MCPL data-plane gate * synchronously, before its first await, so calling this while a surgery * still holds the store keeps MCPL-borne wakes behind the gate exactly as - * before. Only the surgery's response and its store reservation stop - * waiting on remote reactions. A failure is accounted by the gate itself - * (connections recycled); here it is logged and raised as an ops alert. + * before. The drain starts in the same synchronous run (the first reaction + * request can be written before this returns) and continues independently; + * only the surgery's response and its store reservation stop waiting on + * remote reactions. A failure is accounted by the gate itself (connections + * recycled); here it is logged and raised as an ops alert. */ private deliverDiscordAwarenessInBackground(verb: 'rollback' | 'suppress', agentName: string): void { if (!this.discordAwarenessOutbox) return; From 287eef2d695cfaad10e73787e2907d446bab00f4 Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Wed, 7 Oct 2026 05:45:45 -0400 Subject: [PATCH 04/17] docs(surgery): say what every receipt outcome guarantees, not 'durably active' Review (June-1016, room-225 #41978): the SurgeryMarkerReceipt doc said an unresolved batch is 'in its prepared state', and the surgery docs and reservation comments said the call returns once the batch is durably active. Neither holds for every outcome: June's real-server probe saw an unresolved batch already promoted and delivering. The docs now say the body is applied with a marker-scheduling receipt, the reservation is held until marker scheduling has been recorded or reported, and an unresolved receipt names a retained batch whose scheduling outcome is unresolved. The changelog fragment's opening sentence is corrected the same way. Comments only. Co-Authored-By: Claude Opus 5.5 --- .../fix-surgery-marks-receipt.fixed.md | 4 +-- src/framework.ts | 26 ++++++++++--------- src/operator-log.ts | 6 ++--- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/changelog.d/fix-surgery-marks-receipt.fixed.md b/changelog.d/fix-surgery-marks-receipt.fixed.md index caef410eb..85e715a28 100644 --- a/changelog.d/fix-surgery-marks-receipt.fixed.md +++ b/changelog.d/fix-surgery-marks-receipt.fixed.md @@ -1,7 +1,7 @@ - Live `rollbackToMessage` and `suppressMessages` (and message-granular `host/command` undo, which rides on rollback) return once the body change - has landed and the Discord awareness batch is durably active, instead of - after every 💤 reaction has been delivered. Previously the store stayed + has landed, with a marker-scheduling receipt, instead of after every 💤 + reaction has been delivered. Previously the store stayed reserved, and the operator got no answer, for the whole serial drain: about 15 minutes for a rollback that removed 918 channel messages. Delivery now continues in the background, behind the same MCPL data-plane gate as diff --git a/src/framework.ts b/src/framework.ts index 8385cc033..13551a5ce 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -5955,11 +5955,11 @@ export class AgentFramework { * Discord messages that left the live context get awareness markers via the * durable outbox, exactly as message-granular `undo` does. * - * Returns once the switch has landed and the marker batch is durably active - * (`markers`), without waiting for Discord to accept the reactions: delivery - * is started, behind the MCPL data-plane gate, and continues without the - * caller or the store reservation waiting on it; its per-message outcomes - * stay in the outbox ledger. + * Returns once the switch has landed: the body is applied, with a + * marker-scheduling receipt (`markers`). It does not wait for Discord to + * accept the reactions: delivery is started, behind the MCPL data-plane + * gate, and continues without the caller or the store reservation waiting + * on it; its per-message outcomes stay in the outbox ledger. * * Throws `OperatorActionError` (`agent-busy`, `unknown-message`, …) — the * agent must be idle; nothing is queued. @@ -6022,8 +6022,9 @@ export class AgentFramework { } // Gate + reserve the whole store (see reserveStoreForSurgery); held - // until the switch has landed and its marker batch is durably active, - // or the switch has been rolled back. Remote delivery is not waited on. + // until the switch has landed and its marker scheduling has been + // recorded or reported, or the switch has been rolled back. Remote + // delivery is not waited on. const release = this.reserveStoreForSurgery('roll back', agentName); let branchName: string; let markers: SurgeryMarkerReceipt; @@ -6110,9 +6111,9 @@ export class AgentFramework { * removed together. Discord originals get awareness markers via the outbox; * the batch is `explicit`-activated only after every removal succeeded, and * a crash mid-way is finished at next boot (resumePreparedDiscordSuppressions). - * Like rollbackToMessage, it returns once the body change and the batch's - * activation are durable (`markers`); delivery continues without the caller - * waiting on it. + * Like rollbackToMessage, it returns once the body change has landed, with + * a marker-scheduling receipt (`markers`); delivery continues without the + * caller waiting on it. * * Not retroactive over derived state: a message already folded into an * autobiographical summary stays in that summary — roll back to before it @@ -6188,8 +6189,9 @@ export class AgentFramework { throw new OperatorActionError('invalid', 'Suppression branch name must differ from the active branch'); } // Gate + reserve the whole store (see reserveStoreForSurgery); held - // until the fork is fully redacted and its marker batch durably - // active, or the source is restored. Remote delivery is not waited on. + // until the fork is fully redacted and its marker scheduling has been + // recorded or reported, or the source is restored. Remote delivery is + // not waited on. const release = this.reserveStoreForSurgery('suppress', agentName); let createdBranch: string; let markers: SurgeryMarkerReceipt; diff --git a/src/operator-log.ts b/src/operator-log.ts index 2a50106c5..23f3dbaff 100644 --- a/src/operator-log.ts +++ b/src/operator-log.ts @@ -131,9 +131,9 @@ export function capIds(ids: string[]): { ids: string[]; count: number; truncated * active failed, so it was retired from the ledger: none of its marks will * be delivered. * - `unresolved` — the ledger could record neither the activation nor the - * retirement. The batch (`batchId`) is still in the ledger in its prepared - * state, and any later reconciliation or restart that can read it may - * promote and deliver it; nothing here promises otherwise. + * retirement. `batchId` names a retained batch whose scheduling outcome is + * unresolved: any later reconciliation or restart that can read it may + * promote and deliver it, and nothing here promises otherwise. */ export type SurgeryMarkerReceipt = | { status: 'none'; queued: 0 } From 80c74c813a75252deb874c9c61bb29eaca4eb2a1 Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Wed, 7 Oct 2026 06:02:34 -0400 Subject: [PATCH 05/17] docs(changelog): state the incident's drain duration as projected, not observed Review (June-1016, room-225 #42149): 'about 15 minutes for a rollback that removed 918 channel messages' read as an observed duration. The household stopped the host after 225 deliveries; fifteen minutes is the projection at the reported pace. Replaced with June's endorsed wording. Changelog only. Co-Authored-By: Claude Opus 5.5 --- changelog.d/fix-surgery-marks-receipt.fixed.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/changelog.d/fix-surgery-marks-receipt.fixed.md b/changelog.d/fix-surgery-marks-receipt.fixed.md index 85e715a28..dcaf321ea 100644 --- a/changelog.d/fix-surgery-marks-receipt.fixed.md +++ b/changelog.d/fix-surgery-marks-receipt.fixed.md @@ -1,9 +1,10 @@ - Live `rollbackToMessage` and `suppressMessages` (and message-granular `host/command` undo, which rides on rollback) return once the body change has landed, with a marker-scheduling receipt, instead of after every 💤 - reaction has been delivered. Previously the store stayed - reserved, and the operator got no answer, for the whole serial drain: about - 15 minutes for a rollback that removed 918 channel messages. Delivery now + reaction has been delivered. Previously the store stayed reserved, and the + operator got no answer, for the whole serial drain. At the reported pace + of about one reaction per second, 918 queued marks would take roughly 15 + minutes; the household's client timed out after 60 seconds. Delivery now continues in the background, behind the same MCPL data-plane gate as before. The result's new `markers` field (`none` | `queued` | `not-scheduled` | `unresolved`) says whether marks were scheduled, never From acd95b6b19f04eef2b9ded6be93df021b1987073 Mon Sep 17 00:00:00 2001 From: Fern-1209 Date: Wed, 7 Oct 2026 07:40:54 -0400 Subject: [PATCH 06/17] fix(awareness): new gates wait for running drains, not only pending marks Greptile's P1 on #246 (discussion_r4205867371). Now that rollbackToMessage returns while its marks are still being delivered, an operator can restore the source branch (api/server.ts cmdBranchSwitch: store.switchBranch, then syncDiscordAwarenessMarkers) while the first add_reaction still waits for its reply. Reconciliation makes the rollback's marks undesired, and the held add is not present yet, so pending() is empty. The new generation was then a no-work gate: releaseMcplDataPlaneGate completed it at once and it replaced the generation that was waiting on the running drain, opening every MCPL data plane while that drain went on sending its saved adds and then the removals they need. A tools/list_changed after such a restore reached the same hole. On main the same switch could only land as a concurrent request during a rollback's awaited drain (cmdBranchSwitch never consults surgeryHold); #246 makes it the ordinary sequence, so the fix belongs here. beginDiscordAwarenessBarrier now accounts for the server ids of every running drain (discordAwarenessDrains) as well as those with pending operations, and joins those drains; drainDiscordAwarenessOutbox already returns a running drain, which picks up the removals in its next pass. Every gate installer (startup, sync, quiesce resume, connect, reconnect, list change) goes through it. Only the case of a running drain with nothing pending changes: while a drain runs, its in-flight operation is otherwise still pending in the ledger. Out of scope, deliberately: the drain still sends the adds it saved before the restore and then removes them. That is how it already behaves, and the same as a restore on main after the rollback finished; the stacked marks contract (retract supersedes undispatched adds) replaces this drain. Test: restoring the source while delivery runs keeps traffic gated until the running drain is done. Red on 80c74c8 (the probe was answered while the first reaction was still held); green here. Focused file 7/7; awareness/surgery/quiesce/reconnect files 96/96. --- .../fix-surgery-marks-receipt.fixed.md | 18 ++++--- src/framework.ts | 29 ++++++++--- test/surgery-marks-receipt.test.ts | 48 +++++++++++++++++++ 3 files changed, 81 insertions(+), 14 deletions(-) diff --git a/changelog.d/fix-surgery-marks-receipt.fixed.md b/changelog.d/fix-surgery-marks-receipt.fixed.md index dcaf321ea..20abfe420 100644 --- a/changelog.d/fix-surgery-marks-receipt.fixed.md +++ b/changelog.d/fix-surgery-marks-receipt.fixed.md @@ -6,10 +6,14 @@ of about one reaction per second, 918 queued marks would take roughly 15 minutes; the household's client timed out after 60 seconds. Delivery now continues in the background, behind the same MCPL data-plane gate as - before. The result's new `markers` field (`none` | `queued` | - `not-scheduled` | `unresolved`) says whether marks were scheduled, never - that Discord accepted them. If the ledger write after the switch fails, the - rollback that already applied is no longer reported as failed: the batch is - retired (`not-scheduled`), or, if even that write fails, the result says - the batch may still be delivered (`unresolved`). A suppression whose - redactions all committed is no longer undone by that failure. + before. A gate installed while a drain is still running (restoring the + source branch mid-delivery, say) now waits for that drain even when the + switch leaves nothing pending, so traffic stays held through the drain's + remaining reactions and the removals they need. The result's new + `markers` field (`none` | `queued` | `not-scheduled` | `unresolved`) says + whether marks were scheduled, never that Discord accepted them. If the + ledger write after the switch fails, the rollback that already applied is + no longer reported as failed: the batch is retired (`not-scheduled`), or, + if even that write fails, the result says the batch may still be delivered + (`unresolved`). A suppression whose redactions all committed is no longer + undone by that failure. diff --git a/src/framework.ts b/src/framework.ts index 13551a5ce..9b80c3985 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -13295,7 +13295,10 @@ export class AgentFramework { await this.refreshMcplTools(); } - /** Reconcile the durable ledger with Chronicle, then deliver every server's work. */ + /** + * Reconcile the durable ledger with Chronicle, then deliver every server's + * work, joining any drain still running. + */ async syncDiscordAwarenessMarkers(_onlyServerId?: string): Promise { if (!this.discordAwarenessOutbox) return; // A targeted retry may discover pending work for other Discord servers @@ -13510,13 +13513,25 @@ export class AgentFramework { throw new DiscordAwarenessAccountingError('branch reconciliation', error); } const pending = this.readDiscordAwarenessPending(); - if (pending.length === 0) { + // A drain that is still running holds operations it read before this + // reconciliation, and it sends them whatever the ledger now says. A branch + // move can hide them from `pending`: restoring a rollback's source makes + // its marks undesired, and an add whose reply has not arrived is not yet + // present, so the ledger shows nothing to do while the drain goes on to + // send its saved adds and then the removals they need. So a generation + // accounts for every running drain as well as for every pending + // operation, and joins those drains instead of releasing the data planes + // beside them. + const serverIds = [...new Set([ + ...pending.map((operation) => operation.ref.serverId), + ...this.discordAwarenessDrains.keys(), + ])]; + if (serverIds.length === 0) { return { requiresBarrier: false, promise: Promise.resolve({ status: 'delivered', delivered: 0, failed: 0 }), }; } - const serverIds = [...new Set(pending.map((operation) => operation.ref.serverId))]; const promise = Promise.all( serverIds.map((serverId) => this.drainDiscordAwarenessOutbox(serverId)), ).then((outcomes): DiscordAwarenessDrainOutcome => { @@ -13546,10 +13561,10 @@ export class AgentFramework { /** * Install one framework-global Discord-awareness generation before releasing - * any inference-bearing event. With pending work, every MCPL data plane is - * paused while all control planes remain live. With no pending work, ready() - * runs synchronously so request responders preserve their historical - * same-stack behavior. + * any inference-bearing event. With pending work or a drain still running, + * every MCPL data plane is paused while all control planes remain live. + * With neither, ready() runs synchronously so request responders preserve + * their historical same-stack behavior. */ private installMcplDataPlaneGate(): DiscordAwarenessBarrier { for (const connection of this.mcplServerRegistry?.getAllServers() ?? []) { diff --git a/test/surgery-marks-receipt.test.ts b/test/surgery-marks-receipt.test.ts index c71ac0f31..cff83288b 100644 --- a/test/surgery-marks-receipt.test.ts +++ b/test/surgery-marks-receipt.test.ts @@ -204,6 +204,54 @@ describe('surgery marker receipt', () => { assert.ok(probe.at >= lastReaction, 'the probe is answered only after the held reactions'); }); + it('restoring the source while delivery runs keeps traffic gated until the running drain is done', async () => { + await start(true); + const { tail } = seed(3); + + const rollback = await settlesWithin( + framework.rollbackToMessage('resident', { messageId: tail }), + 3_000, + 'rollbackToMessage with reactions held', + ); + await waitFor('the first reaction call to reach the server', () => jsonl(callsPath).length >= 1); + + // The operator restores the source the way the branch-switch API does + // (store switch, then synchronization) while the first add_reaction still + // waits for its reply. Reconciliation makes every mark undesired, and the + // held one is not present yet, so the ledger shows nothing pending: only + // the running drain knows it will still send its saved adds. + framework.getStore().switchBranch(rollback.sourceBranch); + const restored = framework.syncDiscordAwarenessMarkers(); + + writeFileSync(probePath, '1'); + await waitFor('the probe to be sent', () => jsonl(eventsPath).some((e) => e.event === 'probe-sent')); + await new Promise((resolve) => setTimeout(resolve, 200)); + assert.equal( + jsonl(eventsPath).some((e) => e.event === 'probe-answered'), + false, + 'channel traffic waits for the running drain even though nothing appears pending', + ); + + writeFileSync(holdPath, '1'); + await settlesWithin(restored, 5_000, "the restore's synchronization"); + await waitFor('the probe to be answered after delivery', () => + jsonl(eventsPath).some((e) => e.event === 'probe-answered')); + + // Every reaction request the drain made, including the removals the + // restore needs, was answered before channel traffic was released, and + // the restore leaves no mark behind. + const events = jsonl<{ event: string }>(eventsPath); + const probeIndex = events.findIndex((e) => e.event === 'probe-answered'); + const answered = (from: number, to?: number) => + events.slice(from, to).filter((e) => e.event === 'reaction-answered').length; + assert.equal(answered(0, probeIndex), jsonl(callsPath).length, 'every reaction call answered before the probe'); + assert.equal(answered(probeIndex), 0, 'no reaction answered after the probe'); + assert.deepEqual( + ledger()[0].refs.map((ref) => [ref.messageId, ref.desired, ref.markerPresent, ref.deliveryStatus]), + [['amb-0', false, false, 'applied'], ['amb-1', false, false, 'applied'], ['amb-2', false, false, 'applied']], + ); + }); + it('a rollback whose marker bookkeeping fails after the switch reports the body as applied', async () => { await start(true); const { tail } = seed(2); From 8b22d1fbed4c1472b4018373b28e8fc4cd00925d Mon Sep 17 00:00:00 2001 From: Fern-1209 Date: Wed, 7 Oct 2026 07:42:08 -0400 Subject: [PATCH 07/17] docs(surgery): `none` also covers a disabled awareness outbox Greptile's P2 on #246 (discussion_r4205867381). The receipt documented `none` as "the change removed no addressable Discord message", but a framework without an awareness outbox (no discordAwarenessOutboxPath and no storePath, which the config doc calls disabled) has no batch to prepare, so activateSurgeryMarkers(null) returns `none` even when the surgery removed addressable Discord messages. A caller could read that as proof that none were affected. Broaden the documented meaning to what the code guarantees, no marks were scheduled, name both reasons, and point to the surgery's removedRefs (discordRefs in the operator log) as what says whether any addressable message was removed. The four statuses stay as they are: removedRefs already sits beside every receipt and tells the two `none` cases apart. Comment-only change. --- src/operator-log.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/operator-log.ts b/src/operator-log.ts index 23f3dbaff..726ee93ed 100644 --- a/src/operator-log.ts +++ b/src/operator-log.ts @@ -123,7 +123,11 @@ export function capIds(ids: string[]): { ids: string[]; count: number; truncated * the body change itself. A surgery that returns has applied its body change; * this says only whether marks were scheduled: * - * - `none` — the change removed no addressable Discord message. + * - `none` — no marks were scheduled: the change removed no addressable + * Discord message, or the awareness outbox is disabled (no + * `discordAwarenessOutboxPath` and no `storePath`). It is not proof that + * no addressable Discord message was removed: the surgery's `removedRefs` + * (`discordRefs` in the operator log) say whether any was. * - `queued` — the outbox batch is durably active and its delivery has been * handed to the background drain. It is not a delivery claim: per-message * outcomes stay in the outbox ledger. From 8b530ee4f40e3a20da7c973678998fdd2f5c0cfc Mon Sep 17 00:00:00 2001 From: Fern-1209 Date: Wed, 7 Oct 2026 07:47:06 -0400 Subject: [PATCH 08/17] test(surgery): require both surgeries' reactions before traffic, by event order Greptile's P2 on #246 (discussion_r4205867396). The second-batch test compared the probe's timestamp only with reactions that happened to be answered, with >=, so it passed if the second surgery's amb-0 were never delivered, and equal milliseconds let a probe answered first pass too. Assert instead, by the fake server's own event order, that exactly amb-0, amb-2 and amb-3 were answered before probe-answered (greptile's suggestion, with a comment saying why). Mutation check: with the drain cut to a single pass (a `break` after the first for loop in the built drainDiscordAwarenessOutbox), the second batch is never delivered (delivered=2). The old assertion still passed under it; the new one fails with actual [amb-2, amb-3]. Without the mutation it passes. Focused file 7/7; awareness/surgery/quiesce/reconnect files 96/96 after a clean rebuild. --- test/surgery-marks-receipt.test.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/test/surgery-marks-receipt.test.ts b/test/surgery-marks-receipt.test.ts index cff83288b..f2a1b4c1b 100644 --- a/test/surgery-marks-receipt.test.ts +++ b/test/surgery-marks-receipt.test.ts @@ -198,10 +198,21 @@ describe('surgery marker receipt', () => { writeFileSync(holdPath, '1'); await waitFor('the probe to be answered after delivery', () => jsonl(eventsPath).some((e) => e.event === 'probe-answered')); - const events = jsonl<{ event: string; at: number }>(eventsPath); - const lastReaction = Math.max(...events.filter((e) => e.event === 'reaction-answered').map((e) => e.at)); - const probe = events.find((e) => e.event === 'probe-answered')!; - assert.ok(probe.at >= lastReaction, 'the probe is answered only after the held reactions'); + // The server's own event order, not timestamps (two events can share a + // millisecond), and every expected reaction by id: one compared only with + // the reactions that happened to be answered would pass if the second + // surgery's batch were never delivered. + const events = jsonl<{ event: string; messageId?: string }>(eventsPath); + const probeIndex = events.findIndex((e) => e.event === 'probe-answered'); + const answeredBeforeProbe = events.slice(0, probeIndex) + .filter((e) => e.event === 'reaction-answered') + .map((e) => e.messageId) + .sort(); + assert.deepEqual( + answeredBeforeProbe, + ['amb-0', 'amb-2', 'amb-3'], + 'both surgeries\' reactions are answered before channel traffic is released', + ); }); it('restoring the source while delivery runs keeps traffic gated until the running drain is done', async () => { From 30dd6f1d77554f22c34c0d2279d29ae77dc9407b Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Sun, 11 Oct 2026 11:15:14 -0400 Subject: [PATCH 09/17] fix(awareness): no operator command waits for Discord; a gate waits for every mark pending when it was armed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The haiku sweep of #246 (sw-af246, Oscar-1808's merge procedure; verdicts room-416 #70789) found 29 issues at 8b530ee; 19 are fix-now, all here. Code: - I2: startDiscordAwarenessDelivery(after, agentName) is public (it was the private deliverDiscordAwarenessInBackground), and every branch move uses it: rollback and suppress as before, and now the web API's branch.switch, undo and redo (api/server.ts) and host/command turn undo. Each returns once its branch has moved; delivery starts behind the data-plane gate (installed synchronously, before the first await); a failure to start is an ops alert ('discord-awareness-delivery'), not a failed command. This changes the web switch's contract, which said it is acknowledged only after its marks were attempted; the comment says so. One wait stays, as the PR text says: the host-command listener waits out the gate before running a command (pre-existing gating). - I13: the reconnect gate completes only after both the awareness drain and the server's re-registration have finished (Promise.all). The gate is global, so another server's drain could settle first and ready the reconnected server's data plane before its grant existed; its data was rejected fail-closed. - I18, I19: drainForGeneration: a generation that finds a drain running waits for it, then drains again with a fresh pass (two generations share the pass that follows one drain). A running drain never re-sends a key it attempted, so an add re-pended mid-drain (rollback, restore, redo) was skipped, and it may have made its last ledger read before a batch activated since. The across-server combination is combineDrainOutcomes, used for both. - I28: awaitDiscordAwarenessBarriers: the push, inference, incoming and host-command handlers wait until no generation is current or the current one is one they already waited for, not only for the one they first saw. Text: - I3, I21: activateSurgeryMarkers' doc: a rollback's batch (target-branch) may be promoted by the very next reconciliation, a suppression's (explicit) waits for the next start; a zero-ref batch reports none whatever the ledger does, and a journal left prepared closes at the next start. The suppression comment says retiring gives up only the marks. - I8: the `queued` receipt says delivery was started in the background, and what happens if that start fails. - Fragment: every awareness reaction (💤 by default) (I29); the other branch moves and the web switch's changed contract (I2); a turn without channel traffic may run during delivery (I14); the [operator] line's marks= and the operator-actions entry's markers (I23); the fresh pass, the handlers and the reconnect gate. Tests: - surgery-marks-receipt: the gate is up when the store is released (I15); a web switch, undo and redo and a host turn undo each return while every reaction reply is held, and the gate stays up (I2); a rollback, its restore and a redo during one drain send the redo's adds again before the probe is answered (I18); a disabled outbox (app-owned store) schedules none and puts up no gate (I5); a delivery whose reconciliation throws raises the alert with no unhandled rejection (I6); a zero-ref suppression reports none whether its journal activates or is left prepared (I11); a suppression's double failure is unresolved, and the suppress log carries its markers (I4). Existing tests gain a positive gate check before each probe (I10), nothing pending before the restore probe (I9), probe replies without error (I22), and a body change in the nothing-addressable test (I25). - surgery fixture: ANSWER_LIMIT_PATH caps how many calls are answered in all, and held calls are answered in order. - mcpl-awareness-barrier: a reconnect while another server's drain finishes and the reconnecting server's policy reply is held: its data waits for the grant and is accepted (I13; fixture POLICY_HOLD_PATH); every inbound handler waits out a generation armed while it waits (I28). - discord-awareness-outbox: the message-granular undo's result carries its receipt (I4). Mutations of the compiled framework and API server, each killed: the web switch, undo and redo and the host turn undo each awaiting delivery, the reconnect gate completing on the drain alone, a generation joining the running drain, handlers waiting only for the first generation, the background catch removed, the gate installed after the store is released, no disabled-outbox guard, each zero-ref return removed, and the host undo dropping its receipt. surgery-marks-receipt 14/14, mcpl-awareness-barrier 27/27, discord-awareness-outbox 6/6, operator-surgery 18/18; tsc clean. Co-Authored-By: Claude Opus 5.5 --- .../fix-surgery-marks-receipt.fixed.md | 50 ++-- src/api/server.ts | 15 +- src/framework.ts | 161 +++++++---- src/operator-log.ts | 7 +- test/discord-awareness-outbox.test.ts | 3 + .../awareness-barrier-mcpl-server.mjs | 16 ++ test/fixtures/surgery-marks-mcpl-server.mjs | 31 ++- test/mcpl-awareness-barrier.test.ts | 107 ++++++++ test/surgery-marks-receipt.test.ts | 253 +++++++++++++++++- 9 files changed, 550 insertions(+), 93 deletions(-) diff --git a/changelog.d/fix-surgery-marks-receipt.fixed.md b/changelog.d/fix-surgery-marks-receipt.fixed.md index 20abfe420..e4411371a 100644 --- a/changelog.d/fix-surgery-marks-receipt.fixed.md +++ b/changelog.d/fix-surgery-marks-receipt.fixed.md @@ -1,19 +1,35 @@ - Live `rollbackToMessage` and `suppressMessages` (and message-granular `host/command` undo, which rides on rollback) return once the body change - has landed, with a marker-scheduling receipt, instead of after every 💤 - reaction has been delivered. Previously the store stayed reserved, and the - operator got no answer, for the whole serial drain. At the reported pace - of about one reaction per second, 918 queued marks would take roughly 15 - minutes; the household's client timed out after 60 seconds. Delivery now - continues in the background, behind the same MCPL data-plane gate as - before. A gate installed while a drain is still running (restoring the - source branch mid-delivery, say) now waits for that drain even when the - switch leaves nothing pending, so traffic stays held through the drain's - remaining reactions and the removals they need. The result's new - `markers` field (`none` | `queued` | `not-scheduled` | `unresolved`) says - whether marks were scheduled, never that Discord accepted them. If the - ledger write after the switch fails, the rollback that already applied is - no longer reported as failed: the batch is retired (`not-scheduled`), or, - if even that write fails, the result says the batch may still be delivered - (`unresolved`). A suppression whose redactions all committed is no longer - undone by that failure. + has landed, with a marker-scheduling receipt, instead of after every + awareness reaction (💤 by default) has been delivered. Previously the store + stayed reserved, and the operator got no answer, for the whole serial + drain. At the reported pace of about one reaction per second, 918 queued + marks would take roughly 15 minutes; the household's client timed out + after 60 seconds. Delivery now continues in the background, behind the + same MCPL data-plane gate as before, which holds channel traffic until the + marks are attempted. A turn that carries no channel traffic, such as a + scheduler timer or a provider admission, may run during delivery. +- The other branch moves no longer wait for Discord either: the web API's + `branch.switch`, `undo` and `redo`, and `host/command` turn undo, return + once the branch has moved, and their marks follow behind the same gate. A + delivery that fails to start raises a `discord-awareness-delivery` ops + alert instead of failing the command, and the next sync, reconnect or + start delivers what is still pending. (The web switch used to be + acknowledged only after its marks were attempted.) +- A gate installed while a drain is still running (restoring the source + branch mid-delivery, say) waits for that drain and then drains again from + the ledger, so every mark pending when the gate was armed is attempted + before traffic is released: the drain's remaining reactions, the removals + they need, and an add re-pended during the drain (a rollback, its undo, + then a redo). Inbound MCPL handlers wait out a generation installed while + they wait, not only the one they first saw, and a reconnect's gate opens + only once both its drain and the server's re-registration have finished. +- The result's new `markers` field (`none` | `queued` | `not-scheduled` | + `unresolved`) says whether marks were scheduled, never that Discord + accepted them. The `[operator]` stderr line gains `marks=…` and the + operator-actions log entry gains `markers`. If the ledger write after the + switch fails, the rollback that already applied is no longer reported as + failed: the batch is retired (`not-scheduled`), or, if even that write + fails, the result says the batch may still be delivered (`unresolved`). A + suppression whose redactions all committed is no longer undone by that + failure. diff --git a/src/api/server.ts b/src/api/server.ts index 58664705b..ec4ddb7fe 100644 --- a/src/api/server.ts +++ b/src/api/server.ts @@ -543,8 +543,13 @@ export class ApiServer { this.currentBranch = params.name; // Reversible Discord awareness markers are part of the branch projection. - // Do not acknowledge the switch until add/remove operations were attempted. - await this.framework.syncDiscordAwarenessMarkers(); + // The switch is acknowledged once it has landed: delivery of the marks it + // adds and removes starts behind the MCPL data-plane gate, which holds + // channel traffic until they are attempted, and a failure is an ops alert + // rather than a failed switch. (It used to wait for every add and remove + // to be attempted, which after a large rollback took the drain's whole + // length.) + this.framework.startDiscordAwarenessDelivery('branch switch'); // Materialize config files from the new branch const ws = this.framework.getModule('workspace'); @@ -598,7 +603,8 @@ export class ApiServer { if (result.undone) { this.currentBranch = result.toBranch!; - await this.framework.syncDiscordAwarenessMarkers(); + // Marks follow behind the data-plane gate; the undo doesn't wait for Discord. + this.framework.startDiscordAwarenessDelivery('undo', params.agentName); this.broadcast('branch:switched', { from: result.fromBranch, to: result.toBranch, @@ -618,7 +624,8 @@ export class ApiServer { if (result.redone) { this.currentBranch = result.toBranch!; - await this.framework.syncDiscordAwarenessMarkers(); + // Marks follow behind the data-plane gate; the redo doesn't wait for Discord. + this.framework.startDiscordAwarenessDelivery('redo', params.agentName); this.broadcast('branch:switched', { from: result.fromBranch, to: result.toBranch, diff --git a/src/framework.ts b/src/framework.ts index 9b80c3985..b61e63a07 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -513,6 +513,21 @@ interface DiscordAwarenessBarrier { promise: Promise; } +/** Several drains' outcomes as one: unavailable only if every one was. */ +function combineDrainOutcomes(outcomes: DiscordAwarenessDrainOutcome[]): DiscordAwarenessDrainOutcome { + if (outcomes.every((outcome) => outcome.status === 'unavailable')) { + return { + status: 'unavailable', + accounted: outcomes.reduce((count, outcome) => count + (outcome.status === 'unavailable' ? outcome.accounted : 0), 0), + }; + } + return { + status: 'delivered', + delivered: outcomes.reduce((count, outcome) => count + (outcome.status === 'delivered' ? outcome.delivered : 0), 0), + failed: outcomes.reduce((count, outcome) => count + (outcome.status === 'delivered' ? outcome.failed : 0), 0), + }; +} + function normalizeDiscordAwarenessDeadline(value: number | undefined): number { if (value === undefined || !Number.isFinite(value) || value <= 0) { return DEFAULT_DISCORD_AWARENESS_DEADLINE_MS; @@ -5360,7 +5375,9 @@ export class AgentFramework { return { ok: true, undone: 0, requested, lastVisible: null }; } - await this.syncDiscordAwarenessMarkers(); + // The branch has moved: delivery of the marks it changes starts behind + // the data-plane gate, and the command doesn't wait for Discord. + this.startDiscordAwarenessDelivery('turn undo', agentName); return { ok: true, undone, requested, lastVisible: await this.lastVisiblePreview(agentName) }; } @@ -6065,7 +6082,7 @@ export class AgentFramework { // Installs the MCPL data-plane gate synchronously, before the store is // released, and starts delivery without awaiting it (the first // reaction request may already be on the wire when this returns). - this.deliverDiscordAwarenessInBackground('rollback', agentName); + this.startDiscordAwarenessDelivery('rollback', agentName); this.materializeConfigMountAfterBranchSwitch(); } finally { release(); @@ -6250,14 +6267,15 @@ export class AgentFramework { // Every redaction committed: the body change has landed and the fork // is safe to serve from. Activating the marker batch is bookkeeping // about a world-side effect; if it fails, the suppression still - // stands and the receipt says the marks were not scheduled. (No - // interval needs resuming any more, so retiring the batch there - // loses nothing.) + // stands and the receipt says the marks were not scheduled. (The + // batch's resume journal has nothing left to do, since every interval + // has committed, so retiring the batch gives up only the marks, and + // the receipt says so.) markers = this.activateSurgeryMarkers(markerBatch, 'suppress', agentName); // Installs the MCPL data-plane gate synchronously, before the store is // released, and starts delivery without awaiting it (the first // reaction request may already be on the wire when this returns). - this.deliverDiscordAwarenessInBackground('suppress', agentName); + this.startDiscordAwarenessDelivery('suppress', agentName); this.materializeConfigMountAfterBranchSwitch(); } finally { release(); @@ -6304,10 +6322,17 @@ export class AgentFramework { * activated is retired, so no later reconciliation can deliver it behind a * receipt saying it was not scheduled (`not-scheduled`). If retiring fails * too, the receipt says only that the outcome is `unresolved`: the batch is - * still prepared, and the reconciliation that delivery runs next may well - * promote it. A batch with no refs (a suppression journal with nothing - * addressable) is still activated, which closes its resume journal; it - * schedules no marks. + * still prepared, and it may still be delivered. A rollback's batch + * (activated by target branch) may be promoted by the very next + * reconciliation, the one the delivery it starts runs; a suppression's + * (activated explicitly) waits for the next start. + * + * A batch with no refs (a suppression journal with nothing addressable) is + * still activated, which closes its resume journal. It schedules no marks, + * so it reports `none` whatever the ledger does. If it can be neither + * activated nor retired, its journal is left prepared and closes at the + * next start: every interval it names has already committed, so the resume + * skips them all and activates it. */ private activateSurgeryMarkers( batch: DiscordAwarenessBatch | null, @@ -6351,25 +6376,35 @@ export class AgentFramework { } /** - * Start awareness delivery without the caller awaiting it. - * `syncDiscordAwarenessMarkers` installs the MCPL data-plane gate - * synchronously, before its first await, so calling this while a surgery - * still holds the store keeps MCPL-borne wakes behind the gate exactly as - * before. The drain starts in the same synchronous run (the first reaction - * request can be written before this returns) and continues independently; - * only the surgery's response and its store reservation stop waiting on - * remote reactions. A failure is accounted by the gate itself (connections - * recycled); here it is logged and raised as an ops alert. + * Start awareness delivery after a branch move (a surgery, a switch, an + * undo or redo) without the caller awaiting it. No operator response waits + * for Discord to accept marks: the MCPL data-plane gate alone holds channel + * traffic until they are attempted. `syncDiscordAwarenessMarkers` installs + * that gate synchronously, before its first await, so calling this while a + * surgery still holds the store keeps MCPL-borne wakes behind the gate + * exactly as before. The drain starts in the same synchronous run (the + * first reaction request can be written before this returns) and continues + * independently; the caller's response and any store reservation stop + * waiting on remote reactions. A wake that carries no channel traffic (a + * scheduler timer, a provider admission) may run during delivery. A failure + * is accounted by the gate itself (connections recycled); here it is + * logged and raised as an ops alert, and the next sync, reconnect or start + * delivers what is still pending. + * + * @param after What moved the branch, for the log and the alert + * ("rollback", "branch switch", "undo" …). + * @param agentName The agent the move was for, or "framework" for a + * branch switch that names none. */ - private deliverDiscordAwarenessInBackground(verb: 'rollback' | 'suppress', agentName: string): void { + startDiscordAwarenessDelivery(after: string, agentName = 'framework'): void { if (!this.discordAwarenessOutbox) return; this.syncDiscordAwarenessMarkers().catch((error) => { const detail = error instanceof Error ? error.message : String(error); - console.error(`[discord-awareness] delivery after ${verb} agent=${agentName} failed: ${detail}`); + console.error(`[discord-awareness] delivery after ${after} agent=${agentName} failed: ${detail}`); this.opsAlert( 'discord-awareness-delivery', agentName, - `Awareness mark delivery after ${verb} failed: ${detail}`, + `Awareness mark delivery after ${after} failed: ${detail}`, ); }); } @@ -13520,8 +13555,8 @@ export class AgentFramework { // present, so the ledger shows nothing to do while the drain goes on to // send its saved adds and then the removals they need. So a generation // accounts for every running drain as well as for every pending - // operation, and joins those drains instead of releasing the data planes - // beside them. + // operation, and waits for those drains instead of releasing the data + // planes beside them (drainForGeneration). const serverIds = [...new Set([ ...pending.map((operation) => operation.ref.serverId), ...this.discordAwarenessDrains.keys(), @@ -13533,32 +13568,44 @@ export class AgentFramework { }; } const promise = Promise.all( - serverIds.map((serverId) => this.drainDiscordAwarenessOutbox(serverId)), - ).then((outcomes): DiscordAwarenessDrainOutcome => { - if (outcomes.every((outcome) => outcome.status === 'unavailable')) { - return { - status: 'unavailable', - accounted: outcomes.reduce( - (count, outcome) => count + (outcome.status === 'unavailable' ? outcome.accounted : 0), - 0, - ), - }; - } - return { - status: 'delivered', - delivered: outcomes.reduce( - (count, outcome) => count + (outcome.status === 'delivered' ? outcome.delivered : 0), - 0, - ), - failed: outcomes.reduce( - (count, outcome) => count + (outcome.status === 'delivered' ? outcome.failed : 0), - 0, - ), - }; - }); + serverIds.map((serverId) => this.drainForGeneration(serverId)), + ).then(combineDrainOutcomes); return { requiresBarrier: true, promise }; } + /** + * One server's drain for a new generation: a gate completes only after + * every operation pending when it was armed has been attempted. A drain + * already running can't promise that. It never re-sends a key it has + * attempted, so an operation re-pended while it runs (a rollback, its undo, + * then a redo, all mid-drain) stays pending, and it may already have made + * its last read of the ledger, missing a batch activated since. So a + * generation that finds a drain running waits for it, then drains again + * with a fresh pass that reads the ledger anew. Two generations waiting on + * the same drain share the fresh pass that follows it. + */ + private drainForGeneration(serverId: string): Promise { + const running = this.discordAwarenessDrains.get(serverId); + if (!running) return this.drainDiscordAwarenessOutbox(serverId); + return running.then((first) => this.drainDiscordAwarenessOutbox(serverId) + .then((fresh) => combineDrainOutcomes([first, fresh]))); + } + + /** + * Wait out the awareness gate for an inbound event: the generation current + * now, and any generation installed while waiting, until none is current + * or the current one is one already waited for. A handler that waited only + * for the generation it first saw would go on behind a newer one armed + * meanwhile. Rejects as the generation it waited for rejects. + */ + private async awaitDiscordAwarenessBarriers(): Promise { + let waited: DiscordAwarenessBarrier | null = null; + for (let barrier = this.discordAwarenessBarrier; barrier && barrier !== waited; barrier = this.discordAwarenessBarrier) { + waited = barrier; + await barrier.promise; + } + } + /** * Install one framework-global Discord-awareness generation before releasing * any inference-bearing event. With pending work or a drain still running, @@ -14229,8 +14276,7 @@ export class AgentFramework { params: PushEventParams, responder?: { respond: (result: unknown) => void; respondError: (code: number, message: string) => void }, ) => { - const barrier = this.discordAwarenessBarrier; - if (barrier) await barrier.promise; + await this.awaitDiscordAwarenessBarriers(); await this.pushHandler?.handlePushEvent(connection.id, params, responder as never); }); @@ -14245,8 +14291,7 @@ export class AgentFramework { return; } if (this.inferenceRouter && responder) { - const barrier = this.discordAwarenessBarrier; - if (barrier) await barrier.promise; + await this.awaitDiscordAwarenessBarriers(); await this.inferenceRouter.handleInferenceRequest(connection.id, params, { respond: responder.respond, respondError: responder.respondError, @@ -14332,8 +14377,7 @@ export class AgentFramework { params: ChannelsIncomingParams, responder?: { respond: (result: unknown) => void }, ) => { - const barrier = this.discordAwarenessBarrier; - if (barrier) await barrier.promise; + await this.awaitDiscordAwarenessBarriers(); await this.channelRegistry?.handleIncoming(connection.id, params, responder as never); }); @@ -14344,8 +14388,7 @@ export class AgentFramework { ) => { if (!responder) return; try { - const barrier = this.discordAwarenessBarrier; - if (barrier) await barrier.promise; + await this.awaitDiscordAwarenessBarriers(); const result = await this.handleHostCommand(connection.id, params ?? {}); responder.respond(result); } catch (error) { @@ -14387,7 +14430,7 @@ export class AgentFramework { // grant) before the data-plane barrier releases — otherwise the first // post-reconnect events land while the grant is still empty and are // rejected fail-closed instead of delivered. - void (async () => { + const reregistered = (async () => { try { const config = this.mcplServerConfigs.get(connection.id); if (config) { @@ -14402,7 +14445,11 @@ export class AgentFramework { this.releaseMcplDataPlaneGate(awarenessBarrier); this.handleToolsListChanged(connection.id); })(); - void awarenessBarrier.promise.then(() => { + // The gate completes, and the data planes go ready, only once both the + // awareness drain and the re-registration have finished. A drain can + // settle first (one already running, or little to send), and the grant + // must exist before the first data-plane event flows. + void Promise.all([awarenessBarrier.promise, reregistered]).then(() => { if ( awarenessBarrier.requiresBarrier && !this.completeMcplDataPlaneGate(awarenessBarrier) diff --git a/src/operator-log.ts b/src/operator-log.ts index 726ee93ed..b441e04ab 100644 --- a/src/operator-log.ts +++ b/src/operator-log.ts @@ -128,8 +128,11 @@ export function capIds(ids: string[]): { ids: string[]; count: number; truncated * `discordAwarenessOutboxPath` and no `storePath`). It is not proof that * no addressable Discord message was removed: the surgery's `removedRefs` * (`discordRefs` in the operator log) say whether any was. - * - `queued` — the outbox batch is durably active and its delivery has been - * handed to the background drain. It is not a delivery claim: per-message + * - `queued` — the outbox batch is durably active, and its delivery was + * started in the background. If starting it fails (reconciliation reading + * or writing the ledger), the batch stays durably active, the + * `discord-awareness-delivery` ops alert says so, and the next sync, + * reconnect or start delivers it. It is not a delivery claim: per-message * outcomes stay in the outbox ledger. * - `not-scheduled` — the body change landed, but recording the batch as * active failed, so it was retired from the ledger: none of its marks will diff --git a/test/discord-awareness-outbox.test.ts b/test/discord-awareness-outbox.test.ts index 4a35a21f5..fb0f3bed1 100644 --- a/test/discord-awareness-outbox.test.ts +++ b/test/discord-awareness-outbox.test.ts @@ -270,6 +270,9 @@ test('message-granular undo prepares markers before switching branches', async ( }); assert.equal(result.ok, true); assert.deepEqual(result.removedRefs.map((ref: { messageId: string }) => ref.messageId), ['m1', 'm2']); + // The command's receipt says the two marks were scheduled, under the batch the ledger holds. + const [batch] = framework.discordAwarenessOutbox.batches(); + assert.deepEqual(result.markers, { status: 'queued', queued: 2, batchId: batch.id }); assert.deepEqual(calls.map((call) => call.split('/')[0]), ['branch:undo-msgs', 'switch:undo-msgs']); assert.deepEqual( framework.discordAwarenessOutbox.pending('discord').map((operation: { ref: { messageId: string } }) => operation.ref.messageId), diff --git a/test/fixtures/awareness-barrier-mcpl-server.mjs b/test/fixtures/awareness-barrier-mcpl-server.mjs index a16b342b3..f8789d22d 100644 --- a/test/fixtures/awareness-barrier-mcpl-server.mjs +++ b/test/fixtures/awareness-barrier-mcpl-server.mjs @@ -14,6 +14,10 @@ const dataMethod = process.env.DATA_METHOD ?? 'push/event'; const serverId = process.env.SERVER_ID ?? 'discord'; const arrivalPath = process.env.ARRIVAL_PATH; const waitForArrivalPath = process.env.WAIT_FOR_ARRIVAL_PATH; +// After a reconnect (generation 2 on), the host's initial policy Request +// (featureSets/update) is answered only once this file exists, so a test can +// hold the server's re-registration open. +const policyHoldPath = process.env.POLICY_HOLD_PATH; let generation = 1; if (generationPath) { @@ -52,6 +56,7 @@ let reactionQueued = false; let pendingControl = false; let listChangeTriggered = false; let reactionOrdinal = 0; +let heldPolicy = null; let buf = ''; function ledgerDeliveryStatus() { @@ -195,6 +200,11 @@ function handle(message) { // host times out (15s), the grant stays empty, and every push/channel // in these tests is rejected fail-closed. if (message.id !== undefined && message.id !== null) { + if (policyHoldPath && generation >= 2 && !existsSync(policyHoldPath)) { + heldPolicy = message; + log('policy-held'); + return; + } reply(message.id, { accepted: true }); } return; @@ -257,6 +267,12 @@ process.stdin.on('data', (chunk) => { }); const timer = setInterval(() => { + if (heldPolicy && policyHoldPath && existsSync(policyHoldPath)) { + const held = heldPolicy; + heldPolicy = null; + log('policy-response'); + reply(held.id, { accepted: true }); + } if (pendingToolRequest && !registered && waitForArrivalPath && existsSync(waitForArrivalPath)) { serviceReaction(pendingToolRequest); maybeFinishRegistration(); diff --git a/test/fixtures/surgery-marks-mcpl-server.mjs b/test/fixtures/surgery-marks-mcpl-server.mjs index 9c5314901..4d2a0a117 100644 --- a/test/fixtures/surgery-marks-mcpl-server.mjs +++ b/test/fixtures/surgery-marks-mcpl-server.mjs @@ -2,15 +2,18 @@ // // Every tools/call is appended to CALLS_PATH. While HOLD_PATH is set and that // file does not exist, replies are withheld, standing in for Discord pacing a -// long reaction drain. When PROBE_PATH appears, the server sends one -// inference-bearing channels/incoming request and appends to EVENTS_PATH when -// the host answers it, so a test can see whether the MCPL data plane was held. -import { appendFileSync, existsSync } from 'node:fs'; +// long reaction drain. ANSWER_LIMIT_PATH, when that file exists, caps how many +// calls are answered in all: later calls wait until the number in it grows. +// When PROBE_PATH appears, the server sends one inference-bearing +// channels/incoming request and appends to EVENTS_PATH when the host answers +// it, so a test can see whether the MCPL data plane was held. +import { appendFileSync, existsSync, readFileSync } from 'node:fs'; const callsPath = process.env.CALLS_PATH; const eventsPath = process.env.EVENTS_PATH; const holdPath = process.env.HOLD_PATH; const probePath = process.env.PROBE_PATH; +const answerLimitPath = process.env.ANSWER_LIMIT_PATH; const channelId = process.env.CHANNEL_ID ?? 'discord:g1:c1'; const send = (message) => process.stdout.write(JSON.stringify(message) + '\n'); @@ -21,11 +24,23 @@ const event = (name, extra = {}) => { }; const held = []; +let answered = 0; let probed = false; let buf = ''; +function answerLimit() { + if (!answerLimitPath || !existsSync(answerLimitPath)) return Infinity; + const limit = Number(readFileSync(answerLimitPath, 'utf8').trim()); + return Number.isFinite(limit) ? limit : Infinity; +} + +function mayAnswer() { + return (!holdPath || existsSync(holdPath)) && answered < answerLimit(); +} + function answer(message) { - event('reaction-answered', { messageId: message.params?.arguments?.messageId }); + answered++; + event('reaction-answered', { messageId: message.params?.arguments?.messageId, name: message.params?.name }); reply(message.id, { content: [{ type: 'text', text: 'Reaction applied' }] }); } @@ -68,7 +83,7 @@ function handle(message) { if (callsPath) { appendFileSync(callsPath, JSON.stringify({ at: Date.now(), name: message.params?.name, args: message.params?.arguments }) + '\n'); } - if (holdPath && !existsSync(holdPath)) held.push(message); + if (held.length > 0 || !mayAnswer()) held.push(message); else answer(message); return; } @@ -93,9 +108,7 @@ process.stdin.on('data', (chunk) => { }); const timer = setInterval(() => { - if (held.length > 0 && holdPath && existsSync(holdPath)) { - for (const message of held.splice(0)) answer(message); - } + while (held.length > 0 && mayAnswer()) answer(held.shift()); if (!probed && probePath && existsSync(probePath)) { probed = true; event('probe-sent'); diff --git a/test/mcpl-awareness-barrier.test.ts b/test/mcpl-awareness-barrier.test.ts index 9cb27c5d3..b5827ed1d 100644 --- a/test/mcpl-awareness-barrier.test.ts +++ b/test/mcpl-awareness-barrier.test.ts @@ -9,6 +9,7 @@ import { writeFileSync, } from 'node:fs'; import { join } from 'node:path'; +import { EventEmitter } from 'node:events'; import { AgentFramework } from '../src/framework.js'; import { McplServerConnection } from '../src/mcpl/server-connection.js'; @@ -533,6 +534,82 @@ test('reconnect pauses fresh inbound data but permits registration and awareness } }); +test('a reconnect gate opens only after re-registration, even when the awareness drain settles first', async () => { + // The gate is global: marks pending for one server keep every data plane + // paused. Here the reconnecting server's re-registration (its §5.3 policy + // reply) is held while another server's drain finishes. The reconnecting + // server's data must wait for its grant, not only for the drain. + mkdirSync(TEST_ROOT, { recursive: true }); + const dir = mkdtempSync(join(TEST_ROOT, 'awareness-reconnect-grant-')); + const outboxPath = join(dir, 'awareness.json'); + const statusPath = join(dir, 'status.jsonl'); + const crashPath = join(dir, 'crash'); + const policyHoldPath = join(dir, 'release-policy'); + const server = (id: 'discord' | 'heartbeat', env: Record, reconnect = false): McplServerConfig => ({ + id, + command: process.execPath, + args: [FIXTURE], + requestTimeoutMs: 0, + env: { STATUS_PATH: statusPath, LEDGER_PATH: outboxPath, SERVER_ID: id, ...env }, + enabledFeatureSets: ['chat'], + ...(reconnect ? { reconnect: true, reconnectIntervalMs: 20, reconnectMaxIntervalMs: 20 } : {}), + }); + const outbox = new DiscordAwarenessOutbox(outboxPath); + let framework: AgentFramework | undefined; + try { + framework = await AgentFramework.create({ + storePath: join(dir, 'store'), + discordAwarenessOutboxPath: outboxPath, + discordAwarenessDeadlineMs: 1_000, + membrane: new MockMembrane().asMembrane(), + agents: [{ name: 'assistant', model: 'test', systemPrompt: 'test' }], + modules: [], + mcplServers: [ + server('discord', {}), + server('heartbeat', { + CRASH_PATH: crashPath, + GENERATION_PATH: join(dir, 'heartbeat-generation'), + POLICY_HOLD_PATH: policyHoldPath, + }, true), + ], + }); + await waitFor('both servers\' startup data answered', () => ['discord', 'heartbeat'].every((id) => + records(statusPath).some((r) => r.serverId === id && r.generation === 1 && r.event === 'push-response'))); + + // A mark pending for discord, then the heartbeat server drops and comes back. + const batch = outbox.prepare({ + agentName: 'assistant', + sourceBranch: 'source', + targetBranch: 'main', + activationPolicy: 'explicit', + refs: [{ serverId: 'discord', channelId: 'discord:guild:startup', messageId: 'message-3' }], + }); + assert.ok(batch); + outbox.activate(batch.id); + writeFileSync(crashPath, 'crash'); + + await waitFor('the reconnect generation\'s policy held', () => + records(statusPath).some((r) => r.serverId === 'heartbeat' && r.generation === 2 && r.event === 'policy-held')); + await waitFor('discord\'s mark delivered while re-registration is held', () => + outbox.batches()[0]?.refs[0]?.deliveryStatus === 'applied'); + await new Promise((resolve) => setTimeout(resolve, 200)); + assert.equal( + records(statusPath).some((r) => r.serverId === 'heartbeat' && r.generation === 2 && r.event === 'push-response'), + false, + 'the reconnected server\'s data waits for its grant, though the drain has settled', + ); + + writeFileSync(policyHoldPath, 'release'); + await waitFor('the reconnected server\'s data after its grant', () => + records(statusPath).some((r) => r.serverId === 'heartbeat' && r.generation === 2 && r.event === 'push-response')); + const push = records(statusPath).find((r) => r.serverId === 'heartbeat' && r.generation === 2 && r.event === 'push-response'); + assert.equal(push?.accepted, true, 'delivered under the re-established grant, not rejected fail-closed'); + } finally { + await framework?.stop(); + rmSync(dir, { recursive: true, force: true }); + } +}); + test('startup fails explicitly when awareness reconciliation throws', async () => { mkdirSync(TEST_ROOT, { recursive: true }); const dir = mkdtempSync(join(TEST_ROOT, 'awareness-reconcile-failure-')); @@ -772,6 +849,36 @@ test('overlapping reconnect/list-change generations release only the newest barr assert.equal(readyCalls, 1); }); +test('inbound handlers wait out a generation installed while they wait', async () => { + const framework = Object.create(AgentFramework.prototype) as any; + const first = deferred(); + const second = deferred(); + framework.discordAwarenessBarrier = { generation: 1, requiresBarrier: true, promise: first.promise }; + const handled: string[] = []; + framework.pushHandler = { handlePushEvent: async () => { handled.push('push-event'); } }; + framework.inferenceRouter = { handleInferenceRequest: async () => { handled.push('inference-request'); } }; + framework.channelRegistry = { handleIncoming: async () => { handled.push('channels-incoming'); } }; + framework.handleHostCommand = async () => { handled.push('host-command'); return { ok: true }; }; + const connection = new EventEmitter() as any; + connection.id = 'discord'; + framework.wireMcplEvents(connection); + const responder = { id: 1, respond: () => {}, respondError: () => {} }; + for (const event of ['push-event', 'inference-request', 'channels-incoming', 'host-command']) { + connection.emit(event, {}, responder); + } + + // A newer generation is armed while the handlers wait on the first. + framework.discordAwarenessBarrier = { generation: 2, requiresBarrier: true, promise: second.promise }; + first.resolve({ status: 'delivered', delivered: 0, failed: 0 }); + await new Promise((resolve) => setTimeout(resolve, 20)); + assert.deepEqual(handled, [], 'no handler runs behind the newer generation'); + + framework.discordAwarenessBarrier = null; + second.resolve({ status: 'delivered', delivered: 0, failed: 0 }); + await new Promise((resolve) => setTimeout(resolve, 20)); + assert.deepEqual(handled.sort(), ['channels-incoming', 'host-command', 'inference-request', 'push-event']); +}); + test('synchronous zero-pending flush rechecks the gate after a nested list change', () => { const connection = new (McplServerConnection as any)( 'discord', null, null, diff --git a/test/surgery-marks-receipt.test.ts b/test/surgery-marks-receipt.test.ts index f2a1b4c1b..65f18a70e 100644 --- a/test/surgery-marks-receipt.test.ts +++ b/test/surgery-marks-receipt.test.ts @@ -19,7 +19,7 @@ import { fileURLToPath } from 'node:url'; import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { AgentFramework, DiscordAwarenessOutbox } from '../src/index.js'; +import { AgentFramework, ApiServer, DiscordAwarenessOutbox } from '../src/index.js'; import type { OperatorLogEntry } from '../src/index.js'; import { MockMembrane } from './helpers/mock-membrane.js'; @@ -60,16 +60,32 @@ describe('surgery marker receipt', () => { let callsPath: string; let eventsPath: string; let holdPath: string; + let answerLimitPath: string; let probePath: string; let framework: AgentFramework; const cm = () => framework.getAgent('resident')!.getContextManager(); const outboxPath = () => join(storePath, 'recovery', 'discord-awareness-outbox.json'); const ledger = () => new DiscordAwarenessOutbox(outboxPath()).batches(); - - async function start(withDiscord: boolean): Promise { + type Internals = { + discordAwarenessBarrier: { requiresBarrier: boolean } | null; + mcplServerRegistry: { getServer(id: string): { dataPlaneReady: boolean } | undefined } | null; + reserveStoreForSurgery(verb: string, agentName: string): () => void; + recordTurnCheckpoint(agentName: string): void; + handleHostCommand(serverId: string, params: Record): Promise>; + }; + const internals = () => framework as unknown as Internals; + /** The delivery gate is up: a generation that requires it, and discord's data plane paused. */ + const assertGateUp = (when: string) => { + assert.equal(internals().discordAwarenessBarrier?.requiresBarrier, true, `${when}: an awareness generation requires the gate`); + assert.equal(internals().mcplServerRegistry?.getServer('discord')?.dataPlaneReady, false, `${when}: discord's data plane is paused`); + }; + const probeAnswer = () => jsonl<{ event: string; error?: unknown }>(eventsPath).find((e) => e.event === 'probe-answered'); + + /** With `appStore`, the framework gets an app-owned store and no storePath, so its outbox is disabled. */ + async function start(withDiscord: boolean, appStore?: unknown): Promise { framework = await AgentFramework.create({ - storePath, + ...(appStore ? { store: appStore as never } : { storePath }), membrane: new MockMembrane().asMembrane(), agents: [{ name: 'resident', model: 'test-model', systemPrompt: 'You are a resident.' }], modules: [], @@ -83,6 +99,7 @@ describe('surgery marker receipt', () => { CALLS_PATH: callsPath, EVENTS_PATH: eventsPath, HOLD_PATH: holdPath, + ANSWER_LIMIT_PATH: answerLimitPath, PROBE_PATH: probePath, }, enabledFeatureSets: ['chat'], @@ -114,6 +131,7 @@ describe('surgery marker receipt', () => { callsPath = join(dir, 'calls.jsonl'); eventsPath = join(dir, 'events.jsonl'); holdPath = join(dir, 'release-reactions'); + answerLimitPath = join(dir, 'answer-limit'); probePath = join(dir, 'send-probe'); }); @@ -186,6 +204,7 @@ describe('surgery marker receipt', () => { // The gate was installed before release: ordinary channel traffic sent // during delivery is not answered until the reactions are. + assertGateUp('during the first surgery\'s delivery'); writeFileSync(probePath, '1'); await waitFor('the probe to be sent', () => jsonl(eventsPath).some((e) => e.event === 'probe-sent')); await new Promise((resolve) => setTimeout(resolve, 200)); @@ -198,6 +217,7 @@ describe('surgery marker receipt', () => { writeFileSync(holdPath, '1'); await waitFor('the probe to be answered after delivery', () => jsonl(eventsPath).some((e) => e.event === 'probe-answered')); + assert.equal(probeAnswer()?.error, null, 'the probe was answered, not refused'); // The server's own event order, not timestamps (two events can share a // millisecond), and every expected reaction by id: one compared only with // the reactions that happened to be answered would pass if the second @@ -233,6 +253,9 @@ describe('surgery marker receipt', () => { // the running drain knows it will still send its saved adds. framework.getStore().switchBranch(rollback.sourceBranch); const restored = framework.syncDiscordAwarenessMarkers(); + // Nothing pending for discord: only the running drain keeps the gate up. + assert.equal(new DiscordAwarenessOutbox(outboxPath()).pending('discord').length, 0, 'the ledger shows nothing pending'); + assertGateUp('after the restore'); writeFileSync(probePath, '1'); await waitFor('the probe to be sent', () => jsonl(eventsPath).some((e) => e.event === 'probe-sent')); @@ -247,6 +270,7 @@ describe('surgery marker receipt', () => { await settlesWithin(restored, 5_000, "the restore's synchronization"); await waitFor('the probe to be answered after delivery', () => jsonl(eventsPath).some((e) => e.event === 'probe-answered')); + assert.equal(probeAnswer()?.error, null, 'the probe was answered, not refused'); // Every reaction request the drain made, including the removals the // restore needs, was answered before channel traffic was released, and @@ -344,6 +368,226 @@ describe('surgery marker receipt', () => { assert.ok(!cm().getAllMessages().some((m) => String(m.id) === removed[1])); assert.equal(result.markers.status, 'not-scheduled'); assert.deepEqual(ledger(), [], 'every interval committed, so the retired journal loses nothing'); + const entry = framework.getOperatorLog({ limit: 10 }).find((e) => e.kind === 'suppress'); + assert.equal(entry?.error, undefined, 'logged as an applied suppression'); + assert.deepEqual(entry?.result?.markers, result.markers); + }); + + it('installs the delivery gate before the surgery releases the store', async () => { + await start(true); + const { tail } = seed(2); + const atRelease: Array<{ installed: boolean; requires?: boolean }> = []; + const reserve = internals().reserveStoreForSurgery.bind(framework); + internals().reserveStoreForSurgery = (verb, agentName) => { + const release = reserve(verb, agentName); + return () => { + const barrier = internals().discordAwarenessBarrier; + atRelease.push({ installed: barrier !== null, requires: barrier?.requiresBarrier }); + release(); + }; + }; + await settlesWithin(framework.rollbackToMessage('resident', { messageId: tail }), 3_000, 'rollback with reactions held'); + assert.deepEqual(atRelease, [{ installed: true, requires: true }]); + }); + + it('a switch, an undo, a redo and a turn undo during a running drain return while replies are held', async () => { + await start(true); + internals().recordTurnCheckpoint('resident'); + const { tail } = seed(3); + const rollback = await settlesWithin( + framework.rollbackToMessage('resident', { messageId: tail }), + 3_000, + 'rollbackToMessage with reactions held', + ); + await waitFor('the first reaction call to reach the server', () => jsonl(callsPath).length >= 1); + + // The web API's commands, called as its WebSocket dispatch calls them. + const server = new ApiServer(framework, { port: 0 }) as unknown as { + cmdBranchSwitch(params: { name: string }): Promise<{ switched: boolean }>; + cmdUndo(params: { agentName: string }): Promise<{ undone?: boolean }>; + cmdRedo(params: { agentName: string }): Promise<{ redone?: boolean }>; + }; + const switched = await settlesWithin(server.cmdBranchSwitch({ name: rollback.sourceBranch }), 3_000, 'web branch switch'); + assert.equal(switched.switched, true); + assertGateUp('after the switch'); + const undone = await settlesWithin(server.cmdUndo({ agentName: 'resident' }), 3_000, 'web undo'); + assert.equal(undone.undone, true); + const redone = await settlesWithin(server.cmdRedo({ agentName: 'resident' }), 3_000, 'web redo'); + assert.equal(redone.redone, true); + const turnUndo = await settlesWithin( + internals().handleHostCommand('discord', { command: 'undo', agentName: 'resident', turns: 1 }), + 3_000, + 'host/command turn undo', + ); + assert.equal(turnUndo.undone, 1); + + // None of them waited for Discord: no reaction has been answered, and the + // gate still holds traffic for the marks they moved. + assert.equal(jsonl(eventsPath).filter((e) => e.event === 'reaction-answered').length, 0); + assertGateUp('after every command returned'); + writeFileSync(holdPath, '1'); + await framework.syncDiscordAwarenessMarkers(); + assert.equal(new DiscordAwarenessOutbox(outboxPath()).pending().length, 0, 'every mark the moves needed was delivered'); + }); + + it('a rollback, its restore and a redo during one drain: every mark is sent before traffic is released', async () => { + await start(true); + const { tail } = seed(2); + const rollback = await settlesWithin( + framework.rollbackToMessage('resident', { messageId: tail }), + 3_000, + 'rollbackToMessage with reactions held', + ); + await waitFor('the first add to reach the server', () => jsonl(callsPath).length >= 1); + + // The source is restored while the first add waits for its reply. + framework.getStore().switchBranch(rollback.sourceBranch); + const restored = framework.syncDiscordAwarenessMarkers(); + // The drain sends both adds, then the removals the restore needs. The + // first removal is held. + writeFileSync(answerLimitPath, '2'); + writeFileSync(holdPath, '1'); + await waitFor('the first removal to reach the server', () => + jsonl<{ name: string }>(callsPath).some((call) => call.name === 'remove_reaction')); + // The rollback is redone while that removal is in flight. The drain has + // already attempted both adds, so it won't send them again itself. + framework.getStore().switchBranch(rollback.targetBranch); + const redone = framework.syncDiscordAwarenessMarkers(); + writeFileSync(probePath, '1'); + await waitFor('the probe to be sent', () => jsonl(eventsPath).some((e) => e.event === 'probe-sent')); + assertGateUp('after the redo'); + + writeFileSync(answerLimitPath, '99'); + await settlesWithin(Promise.all([restored, redone]), 5_000, 'the restore and redo synchronizations'); + await waitFor('the probe to be answered after delivery', () => probeAnswer() !== undefined); + assert.equal(probeAnswer()?.error, null, 'the probe was answered, not refused'); + + assert.deepEqual( + ledger()[0].refs.map((ref) => [ref.messageId, ref.desired, ref.markerPresent]), + [['amb-0', true, true], ['amb-1', true, true]], + 'the redo\'s marks are on Discord', + ); + const events = jsonl<{ event: string; name?: string; messageId?: string }>(eventsPath); + const probeIndex = events.findIndex((e) => e.event === 'probe-answered'); + assert.deepEqual( + events.slice(0, probeIndex).filter((e) => e.event === 'reaction-answered').map((e) => `${e.name}:${e.messageId}`), + [ + 'add_reaction:amb-0', 'add_reaction:amb-1', + 'remove_reaction:amb-0', 'remove_reaction:amb-1', + 'add_reaction:amb-0', 'add_reaction:amb-1', + ], + 'the redo\'s adds were sent again, before channel traffic was released', + ); + }); + + it('with the awareness outbox disabled, a rollback schedules no marks and puts up no gate', async () => { + const { JsStore } = await import('@animalabs/chronicle'); + await start(true, JsStore.openOrCreate({ path: join(dir, 'app-store') })); + const { tail } = seed(2); + const result = await settlesWithin( + framework.rollbackToMessage('resident', { messageId: tail }), + 3_000, + 'rollbackToMessage without an outbox', + ); + assert.equal(result.messagesRemoved, 2); + assert.equal(result.removedRefs.length, 2, 'it removed addressable Discord messages'); + assert.deepEqual(result.markers, { status: 'none', queued: 0 }); + assert.equal(internals().discordAwarenessBarrier, null, 'no gate'); + + // Reactions are still held, and none is needed: traffic flows at once. + writeFileSync(probePath, '1'); + await waitFor('the probe to be answered with no gate', () => probeAnswer() !== undefined); + assert.equal(probeAnswer()?.error, null); + assert.equal(jsonl(callsPath).length, 0, 'no reaction was sent'); + }); + + it('a delivery that fails to start raises an ops alert and leaves no unhandled rejection', async () => { + await start(true); + const { tail } = seed(2); + const alerts: Array<{ kind: string; message: string }> = []; + framework.onTrace((event) => { + if (event.type === 'ops:alert') alerts.push(event as unknown as { kind: string; message: string }); + }); + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown) => { unhandled.push(reason); }; + process.on('unhandledRejection', onUnhandled); + const reconcile = DiscordAwarenessOutbox.prototype.reconcileForBranch; + DiscordAwarenessOutbox.prototype.reconcileForBranch = function () { + throw new Error('injected ledger read failure'); + }; + let result: Awaited>; + try { + result = await settlesWithin( + framework.rollbackToMessage('resident', { messageId: tail }), + 3_000, + 'rollbackToMessage whose delivery fails to start', + ); + await waitFor('the delivery alert', () => alerts.some((alert) => alert.kind === 'discord-awareness-delivery')); + await new Promise((resolve) => setTimeout(resolve, 50)); + } finally { + DiscordAwarenessOutbox.prototype.reconcileForBranch = reconcile; + process.off('unhandledRejection', onUnhandled); + } + assert.deepEqual(unhandled, [], 'the background failure is handled'); + assert.match( + alerts.find((alert) => alert.kind === 'discord-awareness-delivery')!.message, + /after rollback failed: .*injected ledger read failure/, + ); + // Scheduling succeeded: the batch stays durably active for the next sync. + assert.equal(result.markers.status, 'queued'); + assert.equal(ledger()[0].status, 'active'); + }); + + it('a suppression with nothing addressable reports none, whether or not its journal can be closed', async () => { + await start(false); + const local = String(cm().addMessage('resident', [{ type: 'text', text: 'local only' }], {})); + const other = String(cm().addMessage('resident', [{ type: 'text', text: 'also local' }], {})); + cm().addMessage('resident', [{ type: 'text', text: 'after' }], {}); + + const closed = await framework.suppressMessages('resident', { messageIds: [local] }); + assert.deepEqual(closed.markers, { status: 'none', queued: 0 }); + const [journal] = ledger(); + assert.equal(journal?.status, 'active', 'activated, which closes the resume journal'); + assert.deepEqual(journal.refs, []); + + // When the journal can be neither activated nor retired, it still + // schedules nothing, and the receipt says none, not unresolved. + const activate = DiscordAwarenessOutbox.prototype.activate; + const discard = DiscordAwarenessOutbox.prototype.discard; + DiscordAwarenessOutbox.prototype.activate = function () { throw new Error('injected ledger failure'); }; + DiscordAwarenessOutbox.prototype.discard = function () { throw new Error('injected retire failure'); }; + let stuck: Awaited>; + try { + stuck = await framework.suppressMessages('resident', { messageIds: [other] }); + } finally { + DiscordAwarenessOutbox.prototype.activate = activate; + DiscordAwarenessOutbox.prototype.discard = discard; + } + assert.deepEqual(stuck.markers, { status: 'none', queued: 0 }); + assert.equal(ledger().filter((batch) => batch.status === 'prepared').length, 1, 'its journal is left prepared'); + }); + + it('a suppression whose batch can be neither activated nor retired reports unresolved', async () => { + await start(false); + const { removed } = seed(3); + const activate = DiscordAwarenessOutbox.prototype.activate; + const discard = DiscordAwarenessOutbox.prototype.discard; + DiscordAwarenessOutbox.prototype.activate = function () { throw new Error('injected ledger failure'); }; + DiscordAwarenessOutbox.prototype.discard = function () { throw new Error('injected retire failure'); }; + let result: Awaited>; + try { + result = await framework.suppressMessages('resident', { messageIds: [removed[1]] }); + } finally { + DiscordAwarenessOutbox.prototype.activate = activate; + DiscordAwarenessOutbox.prototype.discard = discard; + } + assert.equal(cm().currentBranch().name, result.targetBranch, 'the suppression stands'); + assert.equal(result.markers.status, 'unresolved'); + const [batch] = ledger(); + assert.equal(batch.status, 'prepared'); + assert.equal(result.markers.status === 'unresolved' && result.markers.batchId, batch.id); + const entry = framework.getOperatorLog({ limit: 10 }).find((e) => e.kind === 'suppress'); + assert.deepEqual(entry?.result?.markers, result.markers); }); it('a surgery that removes nothing addressable schedules no marks', async () => { @@ -351,6 +595,7 @@ describe('surgery marker receipt', () => { const keep = String(cm().addMessage('Operator', [{ type: 'text', text: 'kept' }], {})); cm().addMessage('resident', [{ type: 'text', text: 'local only' }], {}); const result = await framework.rollbackToMessage('resident', { messageId: keep }); + assert.equal(result.messagesRemoved, 1, 'the rollback did change the body'); assert.deepEqual(result.markers, { status: 'none', queued: 0 }); assert.equal(ledger().length, 0); }); From 0fb707e105332333b18cff6c32c4d9cfd3abfd7e Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Sun, 11 Oct 2026 11:24:34 -0400 Subject: [PATCH 10/17] fix(awareness): with no gate up, an inbound event is still handled in the turn it arrives The baseline for 30dd6f1 (this branch merged onto main 02adf27, the full suite on the Anima VPS) failed two of main's coalescer tests on Node 20.20.2 and 24.21.0, and nothing else: mcpl-coalescing R11 ("a retraction during the render-start commit wait is final; the batch is not restored") and R13 ("a retraction during the audience lookup is final; the stale batch is not frozen"), both 'noted' where 'retracted' was expected. Main passes both. They reproduce locally on the merged tree, every run. The cause is 30dd6f1's I28 change. `await this.awaitDiscordAwarenessBarriers()` yields a microtask even when no generation is current, where the old `if (barrier) await barrier.promise` didn't await at all. So a push was handed to the coalescer a turn later than it arrived, after a retraction that followed it on the wire. The four handlers now wait only when a generation is current: `if (this.discordAwarenessBarrier) await this.awaitDiscordAwarenessBarriers();`. With none, an event is handled in the turn it arrives, as it always was. With R11 and R13 rerun against the compiled merged tree, the guard makes both pass. Test (mcpl-awareness-barrier, 28): 'with no generation current, an inbound event is handled in the turn it arrives' emits all four events on a wired connection with no gate up and checks that every handler ran before emit returned. With the guard removed from the compiled framework it fails; the generation test (I28) still passes either way. surgery-marks-receipt 14/14. Co-Authored-By: Claude Opus 5.5 --- src/framework.ts | 11 +++++++---- test/mcpl-awareness-barrier.test.ts | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/framework.ts b/src/framework.ts index b61e63a07..682af78c7 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -14276,7 +14276,10 @@ export class AgentFramework { params: PushEventParams, responder?: { respond: (result: unknown) => void; respondError: (code: number, message: string) => void }, ) => { - await this.awaitDiscordAwarenessBarriers(); + // Only when a generation is current: with none, the event is handled in + // the turn it arrives, as it always was (the push coalescer's ordering of + // a push against its retraction depends on that). + if (this.discordAwarenessBarrier) await this.awaitDiscordAwarenessBarriers(); await this.pushHandler?.handlePushEvent(connection.id, params, responder as never); }); @@ -14291,7 +14294,7 @@ export class AgentFramework { return; } if (this.inferenceRouter && responder) { - await this.awaitDiscordAwarenessBarriers(); + if (this.discordAwarenessBarrier) await this.awaitDiscordAwarenessBarriers(); await this.inferenceRouter.handleInferenceRequest(connection.id, params, { respond: responder.respond, respondError: responder.respondError, @@ -14377,7 +14380,7 @@ export class AgentFramework { params: ChannelsIncomingParams, responder?: { respond: (result: unknown) => void }, ) => { - await this.awaitDiscordAwarenessBarriers(); + if (this.discordAwarenessBarrier) await this.awaitDiscordAwarenessBarriers(); await this.channelRegistry?.handleIncoming(connection.id, params, responder as never); }); @@ -14388,7 +14391,7 @@ export class AgentFramework { ) => { if (!responder) return; try { - await this.awaitDiscordAwarenessBarriers(); + if (this.discordAwarenessBarrier) await this.awaitDiscordAwarenessBarriers(); const result = await this.handleHostCommand(connection.id, params ?? {}); responder.respond(result); } catch (error) { diff --git a/test/mcpl-awareness-barrier.test.ts b/test/mcpl-awareness-barrier.test.ts index b5827ed1d..95e26b658 100644 --- a/test/mcpl-awareness-barrier.test.ts +++ b/test/mcpl-awareness-barrier.test.ts @@ -879,6 +879,26 @@ test('inbound handlers wait out a generation installed while they wait', async ( assert.deepEqual(handled.sort(), ['channels-incoming', 'host-command', 'inference-request', 'push-event']); }); +test('with no generation current, an inbound event is handled in the turn it arrives', () => { + // The push coalescer orders a push against its retraction by arrival, so a + // handler with no gate to wait out must not yield first. + const framework = Object.create(AgentFramework.prototype) as any; + framework.discordAwarenessBarrier = null; + const handled: string[] = []; + framework.pushHandler = { handlePushEvent: async () => { handled.push('push-event'); } }; + framework.inferenceRouter = { handleInferenceRequest: async () => { handled.push('inference-request'); } }; + framework.channelRegistry = { handleIncoming: async () => { handled.push('channels-incoming'); } }; + framework.handleHostCommand = async () => { handled.push('host-command'); return { ok: true }; }; + const connection = new EventEmitter() as any; + connection.id = 'discord'; + framework.wireMcplEvents(connection); + const responder = { id: 1, respond: () => {}, respondError: () => {} }; + for (const event of ['push-event', 'inference-request', 'channels-incoming', 'host-command']) { + connection.emit(event, {}, responder); + } + assert.deepEqual(handled.sort(), ['channels-incoming', 'host-command', 'inference-request', 'push-event']); +}); + test('synchronous zero-pending flush rechecks the gate after a nested list change', () => { const connection = new (McplServerConnection as any)( 'discord', null, null, From b099ed1a1b7ca349571f9ffaf6862fae633df1a4 Mon Sep 17 00:00:00 2001 From: Kestrel-2409 Date: Tue, 13 Oct 2026 09:45:43 -0400 Subject: [PATCH 11/17] test(awareness): pin each branch move's delivery start, the bookkeeping alerts and a failed generation's refusals An internal review of this branch's last round and its merge with main found pieces that no test failed without. Each was checked by removing it from the compiled framework or API server: - The web API's switch, undo and redo, and host/command turn undo, could each drop its startDiscordAwarenessDelivery() call and the commands case still passed: it ended with an explicit sync, and the gate it checked was the rollback's. The case now checks that each command arms a newer generation before it returns, and ends without a sync, so the deliveries the commands started must finish on their own. - The discord-awareness-not-scheduled and -unresolved ops alerts were never asserted. The four bookkeeping-failure cases now assert theirs, and the zero-ref suppression asserts that it raises none. - A handler that swallowed a failed generation's rejection passed. New case: a handler waiting on a generation that fails refuses its request (-32603) and never runs it, for all four handlers. - The reconnect case's absence window now first asserts that the reconnecting server has sent the data its grant must hold. Text, with no behaviour change: - startDiscordAwarenessDelivery's comment: with a drain running, this generation's pass follows it rather than starting at once. - The suppression's comment gives both outcomes of a failed activation: not scheduled once the batch is retired, unresolved if it can't be. - operator-log's `queued` doc: the delivery alert reports the failure; the batch staying active is a separate fact. - The changelog: a mark whose server isn't connected is deferred until it is, as before. Co-Authored-By: Claude Opus 5.5 --- .../fix-surgery-marks-receipt.fixed.md | 3 +- src/framework.ts | 27 ++++++----- src/operator-log.ts | 4 +- test/mcpl-awareness-barrier.test.ts | 38 ++++++++++++++++ test/surgery-marks-receipt.test.ts | 45 ++++++++++++++++--- 5 files changed, 95 insertions(+), 22 deletions(-) diff --git a/changelog.d/fix-surgery-marks-receipt.fixed.md b/changelog.d/fix-surgery-marks-receipt.fixed.md index e4411371a..170dfebc3 100644 --- a/changelog.d/fix-surgery-marks-receipt.fixed.md +++ b/changelog.d/fix-surgery-marks-receipt.fixed.md @@ -21,7 +21,8 @@ the ledger, so every mark pending when the gate was armed is attempted before traffic is released: the drain's remaining reactions, the removals they need, and an add re-pended during the drain (a rollback, its undo, - then a redo). Inbound MCPL handlers wait out a generation installed while + then a redo). A mark whose server isn't connected is deferred until it + is, as before. Inbound MCPL handlers wait out a generation installed while they wait, not only the one they first saw, and a reconnect's gate opens only once both its drain and the server's re-registration have finished. - The result's new `markers` field (`none` | `queued` | `not-scheduled` | diff --git a/src/framework.ts b/src/framework.ts index 4263a89b8..d25592b09 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -6608,10 +6608,11 @@ export class AgentFramework { // Every redaction committed: the body change has landed and the fork // is safe to serve from. Activating the marker batch is bookkeeping // about a world-side effect; if it fails, the suppression still - // stands and the receipt says the marks were not scheduled. (The - // batch's resume journal has nothing left to do, since every interval - // has committed, so retiring the batch gives up only the marks, and - // the receipt says so.) + // stands, and the receipt says what became of the marks: not + // scheduled once the batch is retired, or unresolved if it can't be. + // (The batch's resume journal has nothing left to do, since every + // interval has committed, so retiring the batch gives up only the + // marks.) markers = this.activateSurgeryMarkers(markerBatch, 'suppress', agentName); // Installs the MCPL data-plane gate synchronously, before the store is // released, and starts delivery without awaiting it (the first @@ -6723,14 +6724,16 @@ export class AgentFramework { * traffic until they are attempted. `syncDiscordAwarenessMarkers` installs * that gate synchronously, before its first await, so calling this while a * surgery still holds the store keeps MCPL-borne wakes behind the gate - * exactly as before. The drain starts in the same synchronous run (the - * first reaction request can be written before this returns) and continues - * independently; the caller's response and any store reservation stop - * waiting on remote reactions. A wake that carries no channel traffic (a - * scheduler timer, a provider admission) may run during delivery. A failure - * is accounted by the gate itself (connections recycled); here it is - * logged and raised as an ops alert, and the next sync, reconnect or start - * delivers what is still pending. + * exactly as before. With no drain running, the drain starts in the same + * synchronous run (the first reaction request can be written before this + * returns); with one running, this generation's pass follows it + * (drainForGeneration). Either way delivery continues independently: the + * caller's response and any store reservation stop waiting on remote + * reactions. A wake that carries no channel traffic (a scheduler timer, a + * provider admission) may run during delivery. A failure is accounted by + * the gate itself (connections recycled); here it is logged and raised as + * an ops alert, and the next sync, reconnect or start delivers what is + * still pending. * * @param after What moved the branch, for the log and the alert * ("rollback", "branch switch", "undo" …). diff --git a/src/operator-log.ts b/src/operator-log.ts index b441e04ab..b0d322e73 100644 --- a/src/operator-log.ts +++ b/src/operator-log.ts @@ -130,8 +130,8 @@ export function capIds(ids: string[]): { ids: string[]; count: number; truncated * (`discordRefs` in the operator log) say whether any was. * - `queued` — the outbox batch is durably active, and its delivery was * started in the background. If starting it fails (reconciliation reading - * or writing the ledger), the batch stays durably active, the - * `discord-awareness-delivery` ops alert says so, and the next sync, + * or writing the ledger), a `discord-awareness-delivery` ops alert reports + * the failure, the batch stays durably active, and the next sync, * reconnect or start delivers it. It is not a delivery claim: per-message * outcomes stay in the outbox ledger. * - `not-scheduled` — the body change landed, but recording the batch as diff --git a/test/mcpl-awareness-barrier.test.ts b/test/mcpl-awareness-barrier.test.ts index 95e26b658..35f018902 100644 --- a/test/mcpl-awareness-barrier.test.ts +++ b/test/mcpl-awareness-barrier.test.ts @@ -590,6 +590,10 @@ test('a reconnect gate opens only after re-registration, even when the awareness await waitFor('the reconnect generation\'s policy held', () => records(statusPath).some((r) => r.serverId === 'heartbeat' && r.generation === 2 && r.event === 'policy-held')); + assert.ok( + records(statusPath).some((r) => r.serverId === 'heartbeat' && r.generation === 2 && r.event === 'data-request-sent'), + 'the reconnected server has already sent the data its grant must hold', + ); await waitFor('discord\'s mark delivered while re-registration is held', () => outbox.batches()[0]?.refs[0]?.deliveryStatus === 'applied'); await new Promise((resolve) => setTimeout(resolve, 200)); @@ -879,6 +883,40 @@ test('inbound handlers wait out a generation installed while they wait', async ( assert.deepEqual(handled.sort(), ['channels-incoming', 'host-command', 'inference-request', 'push-event']); }); +test('an inbound handler waiting on a generation that fails refuses its request and never runs it', async () => { + // A generation fails when the awareness ledger can't be read or written, so + // its marks were not all attempted: nothing that waited behind it may run, + // and each request is answered with the failure rather than left waiting. + const framework = Object.create(AgentFramework.prototype) as any; + const failing = deferred(); + framework.discordAwarenessBarrier = { generation: 1, requiresBarrier: true, promise: failing.promise }; + const handled: string[] = []; + framework.pushHandler = { handlePushEvent: async () => { handled.push('push-event'); } }; + framework.inferenceRouter = { handleInferenceRequest: async () => { handled.push('inference-request'); } }; + framework.channelRegistry = { handleIncoming: async () => { handled.push('channels-incoming'); } }; + framework.handleHostCommand = async () => { handled.push('host-command'); return { ok: true }; }; + const connection = new EventEmitter() as any; + connection.id = 'discord'; + framework.wireMcplEvents(connection); + const answers: Array<{ event: string; code?: number; result?: unknown }> = []; + for (const event of ['push-event', 'inference-request', 'channels-incoming', 'host-command']) { + connection.emit(event, {}, { + id: 1, + respond: (result: unknown) => { answers.push({ event, result }); }, + respondError: (code: number) => { answers.push({ event, code }); }, + }); + } + + failing.reject(new Error('injected accounting failure')); + await new Promise((resolve) => setTimeout(resolve, 20)); + assert.deepEqual(handled, [], 'nothing runs behind a failed generation'); + assert.deepEqual( + answers.sort((a, b) => a.event.localeCompare(b.event)), + ['channels-incoming', 'host-command', 'inference-request', 'push-event'].map((event) => ({ event, code: -32603 })), + 'each request is refused with the failure', + ); +}); + test('with no generation current, an inbound event is handled in the turn it arrives', () => { // The push coalescer orders a push against its retraction by arrival, so a // handler with no gate to wait out must not yield first. diff --git a/test/surgery-marks-receipt.test.ts b/test/surgery-marks-receipt.test.ts index 65f18a70e..c87038402 100644 --- a/test/surgery-marks-receipt.test.ts +++ b/test/surgery-marks-receipt.test.ts @@ -68,7 +68,7 @@ describe('surgery marker receipt', () => { const outboxPath = () => join(storePath, 'recovery', 'discord-awareness-outbox.json'); const ledger = () => new DiscordAwarenessOutbox(outboxPath()).batches(); type Internals = { - discordAwarenessBarrier: { requiresBarrier: boolean } | null; + discordAwarenessBarrier: { generation: number; requiresBarrier: boolean } | null; mcplServerRegistry: { getServer(id: string): { dataPlaneReady: boolean } | undefined } | null; reserveStoreForSurgery(verb: string, agentName: string): () => void; recordTurnCheckpoint(agentName: string): void; @@ -81,6 +81,15 @@ describe('surgery marker receipt', () => { assert.equal(internals().mcplServerRegistry?.getServer('discord')?.dataPlaneReady, false, `${when}: discord's data plane is paused`); }; const probeAnswer = () => jsonl<{ event: string; error?: unknown }>(eventsPath).find((e) => e.event === 'probe-answered'); + /** The kinds of the Discord-awareness ops alerts raised from now on, in order. */ + const awarenessAlerts = (): string[] => { + const kinds: string[] = []; + framework.onTrace((event) => { + const alert = event as unknown as { type: string; kind?: string }; + if (alert.type === 'ops:alert' && alert.kind?.startsWith('discord-awareness')) kinds.push(alert.kind); + }); + return kinds; + }; /** With `appStore`, the framework gets an app-owned store and no storePath, so its outbox is disabled. */ async function start(withDiscord: boolean, appStore?: unknown): Promise { @@ -290,6 +299,7 @@ describe('surgery marker receipt', () => { it('a rollback whose marker bookkeeping fails after the switch reports the body as applied', async () => { await start(true); const { tail } = seed(2); + const alerts = awarenessAlerts(); const original = DiscordAwarenessOutbox.prototype.activate; DiscordAwarenessOutbox.prototype.activate = function () { throw new Error('injected ledger failure'); @@ -308,6 +318,7 @@ describe('surgery marker receipt', () => { const entry = framework.getOperatorLog({ limit: 10 }).find((e) => e.kind === 'rollback'); assert.equal(entry?.error, undefined, 'logged as an applied rollback, not a failure'); assert.equal((entry?.result?.markers as { status?: string } | undefined)?.status, 'not-scheduled'); + assert.deepEqual(alerts, ['discord-awareness-not-scheduled'], 'the operator is told its marks were not scheduled'); // "Not scheduled" holds across the reconciliation delivery runs and // across a restart: the batch was retired, so nothing can promote it. @@ -324,6 +335,7 @@ describe('surgery marker receipt', () => { it('reports unresolved bookkeeping when the batch can be neither activated nor retired', async () => { await start(false); const { tail } = seed(2); + const alerts = awarenessAlerts(); const activate = DiscordAwarenessOutbox.prototype.activate; const discard = DiscordAwarenessOutbox.prototype.discard; DiscordAwarenessOutbox.prototype.activate = function () { @@ -347,11 +359,13 @@ describe('surgery marker receipt', () => { assert.equal(batches.length, 1); assert.equal(result.markers.status === 'unresolved' && result.markers.batchId, batches[0].id); assert.match(result.markers.status === 'unresolved' ? result.markers.error : '', /injected ledger failure/); + assert.deepEqual(alerts, ['discord-awareness-unresolved'], 'the operator is told the batch may still be delivered'); }); it('a suppression whose marker bookkeeping fails after the last redaction stands', async () => { await start(false); const { removed } = seed(3); + const alerts = awarenessAlerts(); const original = DiscordAwarenessOutbox.prototype.activate; DiscordAwarenessOutbox.prototype.activate = function () { throw new Error('injected ledger failure'); @@ -371,6 +385,7 @@ describe('surgery marker receipt', () => { const entry = framework.getOperatorLog({ limit: 10 }).find((e) => e.kind === 'suppress'); assert.equal(entry?.error, undefined, 'logged as an applied suppression'); assert.deepEqual(entry?.result?.markers, result.markers); + assert.deepEqual(alerts, ['discord-awareness-not-scheduled']); }); it('installs the delivery gate before the surgery releases the store', async () => { @@ -390,7 +405,7 @@ describe('surgery marker receipt', () => { assert.deepEqual(atRelease, [{ installed: true, requires: true }]); }); - it('a switch, an undo, a redo and a turn undo during a running drain return while replies are held', async () => { + it('a switch, an undo, a redo and a turn undo during a running drain each start delivery and return while replies are held', async () => { await start(true); internals().recordTurnCheckpoint('resident'); const { tail } = seed(3); @@ -407,26 +422,38 @@ describe('surgery marker receipt', () => { cmdUndo(params: { agentName: string }): Promise<{ undone?: boolean }>; cmdRedo(params: { agentName: string }): Promise<{ redone?: boolean }>; }; + // Each command starts delivery of the marks it moved before it returns: it + // arms a newer generation, which holds traffic, and doesn't wait for it. + let generation = internals().discordAwarenessBarrier!.generation; + const startedDelivery = (when: string) => { + assertGateUp(when); + const current = internals().discordAwarenessBarrier!.generation; + assert.ok(current > generation, `${when}: a newer generation, the delivery the command started`); + generation = current; + }; const switched = await settlesWithin(server.cmdBranchSwitch({ name: rollback.sourceBranch }), 3_000, 'web branch switch'); assert.equal(switched.switched, true); - assertGateUp('after the switch'); + startedDelivery('after the switch'); const undone = await settlesWithin(server.cmdUndo({ agentName: 'resident' }), 3_000, 'web undo'); assert.equal(undone.undone, true); + startedDelivery('after the undo'); const redone = await settlesWithin(server.cmdRedo({ agentName: 'resident' }), 3_000, 'web redo'); assert.equal(redone.redone, true); + startedDelivery('after the redo'); const turnUndo = await settlesWithin( internals().handleHostCommand('discord', { command: 'undo', agentName: 'resident', turns: 1 }), 3_000, 'host/command turn undo', ); assert.equal(turnUndo.undone, 1); + startedDelivery('after the turn undo'); - // None of them waited for Discord: no reaction has been answered, and the - // gate still holds traffic for the marks they moved. + // None of them waited for Discord: no reaction has been answered. assert.equal(jsonl(eventsPath).filter((e) => e.event === 'reaction-answered').length, 0); - assertGateUp('after every command returned'); + // Once Discord answers, the deliveries the commands started finish on + // their own, with no further sync, and leave nothing pending. writeFileSync(holdPath, '1'); - await framework.syncDiscordAwarenessMarkers(); + await waitFor('the deliveries the commands started to finish', () => internals().discordAwarenessBarrier === null); assert.equal(new DiscordAwarenessOutbox(outboxPath()).pending().length, 0, 'every mark the moves needed was delivered'); }); @@ -540,6 +567,7 @@ describe('surgery marker receipt', () => { it('a suppression with nothing addressable reports none, whether or not its journal can be closed', async () => { await start(false); + const alerts = awarenessAlerts(); const local = String(cm().addMessage('resident', [{ type: 'text', text: 'local only' }], {})); const other = String(cm().addMessage('resident', [{ type: 'text', text: 'also local' }], {})); cm().addMessage('resident', [{ type: 'text', text: 'after' }], {}); @@ -565,11 +593,13 @@ describe('surgery marker receipt', () => { } assert.deepEqual(stuck.markers, { status: 'none', queued: 0 }); assert.equal(ledger().filter((batch) => batch.status === 'prepared').length, 1, 'its journal is left prepared'); + assert.deepEqual(alerts, [], 'no marks were at stake, so no alert'); }); it('a suppression whose batch can be neither activated nor retired reports unresolved', async () => { await start(false); const { removed } = seed(3); + const alerts = awarenessAlerts(); const activate = DiscordAwarenessOutbox.prototype.activate; const discard = DiscordAwarenessOutbox.prototype.discard; DiscordAwarenessOutbox.prototype.activate = function () { throw new Error('injected ledger failure'); }; @@ -588,6 +618,7 @@ describe('surgery marker receipt', () => { assert.equal(result.markers.status === 'unresolved' && result.markers.batchId, batch.id); const entry = framework.getOperatorLog({ limit: 10 }).find((e) => e.kind === 'suppress'); assert.deepEqual(entry?.result?.markers, result.markers); + assert.deepEqual(alerts, ['discord-awareness-unresolved']); }); it('a surgery that removes nothing addressable schedules no marks', async () => { From e1f8e61007867acc890146cf21e280d5af27caac Mon Sep 17 00:00:00 2001 From: Kestrel-2409 Date: Tue, 13 Oct 2026 10:12:04 -0400 Subject: [PATCH 12/17] docs(awareness): say where a mark is deferred rather than attempted; wait for the refusals by event The changelog now says, where it first makes the claim, that the gate holds channel traffic until the marks are attempted or, when their server isn't connected, deferred. Before, it said every pending mark is attempted and only a later sentence carved out a disconnected server. startDiscordAwarenessDelivery's comment now says which failure recycles the connections: the current generation's. A superseded generation's failure leaves that to the newer generation, whose own pass meets the same ledger. The failed-generation case in mcpl-awareness-barrier now waits for its four answers instead of a fixed 20 ms before checking that nothing ran. Co-Authored-By: Claude Opus 5.5 --- .../fix-surgery-marks-receipt.fixed.md | 19 ++++++++++--------- src/framework.ts | 8 +++++--- test/mcpl-awareness-barrier.test.ts | 2 +- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/changelog.d/fix-surgery-marks-receipt.fixed.md b/changelog.d/fix-surgery-marks-receipt.fixed.md index 170dfebc3..f824d30e2 100644 --- a/changelog.d/fix-surgery-marks-receipt.fixed.md +++ b/changelog.d/fix-surgery-marks-receipt.fixed.md @@ -7,8 +7,9 @@ marks would take roughly 15 minutes; the household's client timed out after 60 seconds. Delivery now continues in the background, behind the same MCPL data-plane gate as before, which holds channel traffic until the - marks are attempted. A turn that carries no channel traffic, such as a - scheduler timer or a provider admission, may run during delivery. + marks are attempted, or deferred when their server isn't connected. A + turn that carries no channel traffic, such as a scheduler timer or a + provider admission, may run during delivery. - The other branch moves no longer wait for Discord either: the web API's `branch.switch`, `undo` and `redo`, and `host/command` turn undo, return once the branch has moved, and their marks follow behind the same gate. A @@ -18,13 +19,13 @@ acknowledged only after its marks were attempted.) - A gate installed while a drain is still running (restoring the source branch mid-delivery, say) waits for that drain and then drains again from - the ledger, so every mark pending when the gate was armed is attempted - before traffic is released: the drain's remaining reactions, the removals - they need, and an add re-pended during the drain (a rollback, its undo, - then a redo). A mark whose server isn't connected is deferred until it - is, as before. Inbound MCPL handlers wait out a generation installed while - they wait, not only the one they first saw, and a reconnect's gate opens - only once both its drain and the server's re-registration have finished. + the ledger. So before traffic is released, every mark pending when the + gate was armed is attempted, or deferred until its server is connected, + as before: the drain's remaining reactions, the removals they need, and an + add re-pended during the drain (a rollback, its undo, then a redo). + Inbound MCPL handlers wait out a generation installed while they wait, not + only the one they first saw, and a reconnect's gate opens only once both + its drain and the server's re-registration have finished. - The result's new `markers` field (`none` | `queued` | `not-scheduled` | `unresolved`) says whether marks were scheduled, never that Discord accepted them. The `[operator]` stderr line gains `marks=…` and the diff --git a/src/framework.ts b/src/framework.ts index d25592b09..d918a334a 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -6731,9 +6731,11 @@ export class AgentFramework { * caller's response and any store reservation stop waiting on remote * reactions. A wake that carries no channel traffic (a scheduler timer, a * provider admission) may run during delivery. A failure is accounted by - * the gate itself (connections recycled); here it is logged and raised as - * an ops alert, and the next sync, reconnect or start delivers what is - * still pending. + * the gate itself: the current generation's failure recycles the + * connections, and a superseded one leaves that to the newer generation, + * whose own pass meets the same ledger. Here it is logged and raised as an + * ops alert, and the next sync, reconnect or start delivers what is still + * pending. * * @param after What moved the branch, for the log and the alert * ("rollback", "branch switch", "undo" …). diff --git a/test/mcpl-awareness-barrier.test.ts b/test/mcpl-awareness-barrier.test.ts index 35f018902..6530bee6a 100644 --- a/test/mcpl-awareness-barrier.test.ts +++ b/test/mcpl-awareness-barrier.test.ts @@ -908,7 +908,7 @@ test('an inbound handler waiting on a generation that fails refuses its request } failing.reject(new Error('injected accounting failure')); - await new Promise((resolve) => setTimeout(resolve, 20)); + await waitFor('every request to be answered', () => answers.length === 4); assert.deepEqual(handled, [], 'nothing runs behind a failed generation'); assert.deepEqual( answers.sort((a, b) => a.event.localeCompare(b.event)), From 1f60e57e32cb62552eb5b96e88908c42f2f7b84b Mon Sep 17 00:00:00 2001 From: Kestrel-2409 Date: Tue, 13 Oct 2026 10:39:13 -0400 Subject: [PATCH 13/17] docs(awareness): channel traffic admitted before the gate may now be answered during delivery The store reservation used to last through the whole drain, so a channel message admitted before the surgery's gate went up (queued when the surgery began, or arriving during its switch or fork) started its turn only after every mark had been attempted. The reservation now ends once the gate is up, and the gate holds only traffic that arrives after it, so such a turn can answer before the marks show. A reader's probe showed it with this PR's own fixture. The changelog and startDiscordAwarenessDelivery's doc now say so, and no longer say that only turns without channel traffic may run during delivery. Also in the changelog: - a reconnect's mcpl:server-reconnected and module:added traces now come once the server is re-registered, where they came at once when no marks were pending; - the names a caller needs: the exported SurgeryMarkerReceipt type, the discord-awareness-not-scheduled and discord-awareness-unresolved ops alerts, and the now-public startDiscordAwarenessDelivery(after, agentName?). syncDiscordAwarenessMarkers' doc said it joins a running drain; since the gate's generations were reworked, it waits for that drain and then runs a fresh pass. Co-Authored-By: Claude Opus 5.5 --- .../fix-surgery-marks-receipt.fixed.md | 44 ++++++++++++------- src/framework.ts | 36 ++++++++------- 2 files changed, 48 insertions(+), 32 deletions(-) diff --git a/changelog.d/fix-surgery-marks-receipt.fixed.md b/changelog.d/fix-surgery-marks-receipt.fixed.md index f824d30e2..524477363 100644 --- a/changelog.d/fix-surgery-marks-receipt.fixed.md +++ b/changelog.d/fix-surgery-marks-receipt.fixed.md @@ -6,17 +6,24 @@ drain. At the reported pace of about one reaction per second, 918 queued marks would take roughly 15 minutes; the household's client timed out after 60 seconds. Delivery now continues in the background, behind the - same MCPL data-plane gate as before, which holds channel traffic until the - marks are attempted, or deferred when their server isn't connected. A - turn that carries no channel traffic, such as a scheduler timer or a - provider admission, may run during delivery. + same MCPL data-plane gate as before, which holds channel traffic that + arrives after it until the marks are attempted, or deferred when their + server isn't connected. A turn may now run during delivery when it carries + no channel traffic, such as a scheduler timer or a provider admission, and + also when its channel traffic was admitted before the gate went up (queued + when the surgery began, or arriving during its switch or fork). Such a + turn used to wait for the whole drain, since the store stayed reserved; + now it can answer before the marks show. - The other branch moves no longer wait for Discord either: the web API's `branch.switch`, `undo` and `redo`, and `host/command` turn undo, return - once the branch has moved, and their marks follow behind the same gate. A - delivery that fails to start raises a `discord-awareness-delivery` ops - alert instead of failing the command, and the next sync, reconnect or - start delivers what is still pending. (The web switch used to be - acknowledged only after its marks were attempted.) + once the branch has moved, and their marks follow behind the same gate. + Each starts its delivery through + `startDiscordAwarenessDelivery(after, agentName?)`, now public, which a + host can call after its own branch moves. A delivery that fails to start + raises a `discord-awareness-delivery` ops alert instead of failing the + command, and the next sync, reconnect or start delivers what is still + pending. (The web switch used to be acknowledged only after its marks were + attempted.) - A gate installed while a drain is still running (restoring the source branch mid-delivery, say) waits for that drain and then drains again from the ledger. So before traffic is released, every mark pending when the @@ -25,13 +32,18 @@ add re-pended during the drain (a rollback, its undo, then a redo). Inbound MCPL handlers wait out a generation installed while they wait, not only the one they first saw, and a reconnect's gate opens only once both - its drain and the server's re-registration have finished. + its drain and the server's re-registration have finished. So a + reconnect's `mcpl:server-reconnected` and `module:added` traces now come + once the server is re-registered, where they came at once when no marks + were pending. - The result's new `markers` field (`none` | `queued` | `not-scheduled` | - `unresolved`) says whether marks were scheduled, never that Discord - accepted them. The `[operator]` stderr line gains `marks=…` and the - operator-actions log entry gains `markers`. If the ledger write after the - switch fails, the rollback that already applied is no longer reported as - failed: the batch is retired (`not-scheduled`), or, if even that write - fails, the result says the batch may still be delivered (`unresolved`). A + `unresolved`, the exported `SurgeryMarkerReceipt` type) says whether marks + were scheduled, never that Discord accepted them. The `[operator]` stderr + line gains `marks=…` and the operator-actions log entry gains `markers`. + If the ledger write after the switch fails, the rollback that already + applied is no longer reported as failed: the batch is retired + (`not-scheduled`, with a `discord-awareness-not-scheduled` ops alert), or, + if even that write fails, the result says the batch may still be + delivered (`unresolved`, with a `discord-awareness-unresolved` ops alert). A suppression whose redactions all committed is no longer undone by that failure. diff --git a/src/framework.ts b/src/framework.ts index d918a334a..278a43414 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -6721,21 +6721,24 @@ export class AgentFramework { * Start awareness delivery after a branch move (a surgery, a switch, an * undo or redo) without the caller awaiting it. No operator response waits * for Discord to accept marks: the MCPL data-plane gate alone holds channel - * traffic until they are attempted. `syncDiscordAwarenessMarkers` installs - * that gate synchronously, before its first await, so calling this while a - * surgery still holds the store keeps MCPL-borne wakes behind the gate - * exactly as before. With no drain running, the drain starts in the same - * synchronous run (the first reaction request can be written before this - * returns); with one running, this generation's pass follows it - * (drainForGeneration). Either way delivery continues independently: the - * caller's response and any store reservation stop waiting on remote - * reactions. A wake that carries no channel traffic (a scheduler timer, a - * provider admission) may run during delivery. A failure is accounted by - * the gate itself: the current generation's failure recycles the - * connections, and a superseded one leaves that to the newer generation, - * whose own pass meets the same ledger. Here it is logged and raised as an - * ops alert, and the next sync, reconnect or start delivers what is still - * pending. + * traffic that arrives after it until they are attempted. + * `syncDiscordAwarenessMarkers` installs that gate synchronously, before + * its first await, so calling this while a surgery still holds the store + * puts the gate up before the store is released. With no drain running, the + * drain starts in the same synchronous run (the first reaction request can + * be written before this returns); with one running, this generation's pass + * follows it (drainForGeneration). Either way delivery continues + * independently: the caller's response and any store reservation stop + * waiting on remote reactions. A turn may run during delivery when its wake + * carries no channel traffic (a scheduler timer, a provider admission), and + * also when its channel traffic was admitted before the gate went up + * (queued when a surgery began, or arriving during its switch or fork): the + * surgery's store reservation, which used to hold such a turn through the + * drain, now ends once the gate is up. A failure is accounted by the gate + * itself: the current generation's failure recycles the connections, and a + * superseded one leaves that to the newer generation, whose own pass meets + * the same ledger. Here it is logged and raised as an ops alert, and the + * next sync, reconnect or start delivers what is still pending. * * @param after What moved the branch, for the log and the alert * ("rollback", "branch switch", "undo" …). @@ -14059,7 +14062,8 @@ export class AgentFramework { /** * Reconcile the durable ledger with Chronicle, then deliver every server's - * work, joining any drain still running. + * work. A drain still running is waited for and then followed by a fresh + * pass (drainForGeneration). */ async syncDiscordAwarenessMarkers(_onlyServerId?: string): Promise { if (!this.discordAwarenessOutbox) return; From 0de75ec25462b6188e03d078c83dbce81ad713b8 Mon Sep 17 00:00:00 2001 From: Kestrel-2409 Date: Tue, 13 Oct 2026 10:42:59 -0400 Subject: [PATCH 14/17] test(surgery): pin that a turn for channel traffic admitted before the gate may start before the drain completes This is the accepted behaviour the previous commit documents. The surgery's store reservation ends once the gate is up, and the gate holds only traffic that arrives after it. So a channel message admitted earlier, here during the rollback's switch, wakes a turn while the drain still runs: the first reaction request is on the wire, every reply is still held, and the gate is still up. With the rollback holding its reservation through the drain, as it did before this branch, the case fails. start() takes an optional MockMembrane, so the case can see the turn's request. Co-Authored-By: Claude Opus 5.5 --- test/surgery-marks-receipt.test.ts | 38 ++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/test/surgery-marks-receipt.test.ts b/test/surgery-marks-receipt.test.ts index c87038402..74f693528 100644 --- a/test/surgery-marks-receipt.test.ts +++ b/test/surgery-marks-receipt.test.ts @@ -92,10 +92,10 @@ describe('surgery marker receipt', () => { }; /** With `appStore`, the framework gets an app-owned store and no storePath, so its outbox is disabled. */ - async function start(withDiscord: boolean, appStore?: unknown): Promise { + async function start(withDiscord: boolean, appStore?: unknown, membrane = new MockMembrane()): Promise { framework = await AgentFramework.create({ ...(appStore ? { store: appStore as never } : { storePath }), - membrane: new MockMembrane().asMembrane(), + membrane: membrane.asMembrane(), agents: [{ name: 'resident', model: 'test-model', systemPrompt: 'You are a resident.' }], modules: [], maintenanceIntervalMs: 0, @@ -507,6 +507,40 @@ describe('surgery marker receipt', () => { ); }); + it('a turn for channel traffic admitted before the gate may start before the drain completes', async () => { + // Accepted, and stated in the changelog: the surgery's store reservation + // ends once the gate is up, and the gate holds only traffic that arrives + // after it. Before this change the reservation held such a turn until the + // whole drain had finished. + const membrane = new MockMembrane(); + await start(true, undefined, membrane); + framework.start(); + const { tail } = seed(3); + // The switch the rollback awaits goes on only once one ordinary channel + // message has been admitted and answered, before any gate is up. + const switchBranch = cm().switchBranch.bind(cm()); + (cm() as unknown as { switchBranch: (name: string) => Promise }).switchBranch = async (name: string) => { + writeFileSync(probePath, '1'); + await waitFor('the channel message answered during the switch', () => probeAnswer() !== undefined); + return switchBranch(name); + }; + const turnsBefore = membrane.calls.length; + + await settlesWithin(framework.rollbackToMessage('resident', { messageId: tail }), 3_000, 'rollbackToMessage with reactions held'); + assert.equal(probeAnswer()?.error, null, 'the channel message was admitted'); + await waitFor('a turn for the channel message', () => membrane.calls.length > turnsBefore); + + // The turn started while the drain still runs: the first reaction request + // is on the wire, but every reply is still held and the gate is still up. + await waitFor('the first reaction request to reach the server', () => jsonl(callsPath).length >= 1); + assert.equal(jsonl(eventsPath).filter((e) => e.event === 'reaction-answered').length, 0, 'no reaction reply has been answered'); + assertGateUp('while that turn runs'); + assert.ok( + JSON.stringify(membrane.calls[turnsBefore]).includes('ordinary traffic while marks are delivered'), + 'the turn carries the channel message', + ); + }); + it('with the awareness outbox disabled, a rollback schedules no marks and puts up no gate', async () => { const { JsStore } = await import('@animalabs/chronicle'); await start(true, JsStore.openOrCreate({ path: join(dir, 'app-store') })); From 4c65258632483d719ef7b901a97cef286c5a1dda Mon Sep 17 00:00:00 2001 From: Kestrel-2409 Date: Tue, 13 Oct 2026 10:45:08 -0400 Subject: [PATCH 15/17] docs(awareness): a reconnect's traces follow its re-registration attempt, granted or not; an unresolved batch is retained The registration step in the reconnect listener catches its own failure and settles either way, so the changelog now says the reconnect traces come once that attempt has settled, granted or not, rather than once the server is re-registered. activateSurgeryMarkers knows only that it could not retire the batch, so its doc and its log line now say the batch is retained, not that it is still prepared. The pre-gate test's comment says that it pins an accepted boundary, and that holding pre-gate work by other means would be a deliberate change to it, made together with the text. Co-Authored-By: Claude Opus 5.5 --- changelog.d/fix-surgery-marks-receipt.fixed.md | 6 +++--- src/framework.ts | 4 ++-- test/surgery-marks-receipt.test.ts | 8 +++++--- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/changelog.d/fix-surgery-marks-receipt.fixed.md b/changelog.d/fix-surgery-marks-receipt.fixed.md index 524477363..9e813e7d7 100644 --- a/changelog.d/fix-surgery-marks-receipt.fixed.md +++ b/changelog.d/fix-surgery-marks-receipt.fixed.md @@ -32,10 +32,10 @@ add re-pended during the drain (a rollback, its undo, then a redo). Inbound MCPL handlers wait out a generation installed while they wait, not only the one they first saw, and a reconnect's gate opens only once both - its drain and the server's re-registration have finished. So a + its drain and the server's re-registration attempt have settled. So a reconnect's `mcpl:server-reconnected` and `module:added` traces now come - once the server is re-registered, where they came at once when no marks - were pending. + once that attempt has settled, granted or not, where they came at once + when no marks were pending. - The result's new `markers` field (`none` | `queued` | `not-scheduled` | `unresolved`, the exported `SurgeryMarkerReceipt` type) says whether marks were scheduled, never that Discord accepted them. The `[operator]` stderr diff --git a/src/framework.ts b/src/framework.ts index 278a43414..118b3c64e 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -6664,7 +6664,7 @@ export class AgentFramework { * activated is retired, so no later reconciliation can deliver it behind a * receipt saying it was not scheduled (`not-scheduled`). If retiring fails * too, the receipt says only that the outcome is `unresolved`: the batch is - * still prepared, and it may still be delivered. A rollback's batch + * retained, and it may still be delivered. A rollback's batch * (activated by target branch) may be promoted by the very next * reconciliation, the one the delivery it starts runs; a suppression's * (activated explicitly) waits for the next start. @@ -6696,7 +6696,7 @@ export class AgentFramework { console.error( `[discord-awareness] ${verb} agent=${agentName}: body applied, but marker batch ${batch.id} ` + `could not be activated (${detail}); ` + - (retired ? 'batch retired' : `batch left prepared${retireDetail ? ` (retire failed: ${retireDetail})` : ''}`), + (retired ? 'batch retired' : `batch retained${retireDetail ? ` (retire failed: ${retireDetail})` : ''}`), ); if (batch.refs.length === 0) return { status: 'none', queued: 0 }; this.opsAlert( diff --git a/test/surgery-marks-receipt.test.ts b/test/surgery-marks-receipt.test.ts index 74f693528..8be05a8e7 100644 --- a/test/surgery-marks-receipt.test.ts +++ b/test/surgery-marks-receipt.test.ts @@ -509,9 +509,11 @@ describe('surgery marker receipt', () => { it('a turn for channel traffic admitted before the gate may start before the drain completes', async () => { // Accepted, and stated in the changelog: the surgery's store reservation - // ends once the gate is up, and the gate holds only traffic that arrives - // after it. Before this change the reservation held such a turn until the - // whole drain had finished. + // ends once the gate is up, so no operator command waits on Discord, and + // the gate holds only traffic that arrives after it. Before this change + // the reservation held such a turn until the whole drain had finished. + // This pins that accepted boundary: holding pre-gate work by some other + // means would be a deliberate change to it, made together with the text. const membrane = new MockMembrane(); await start(true, undefined, membrane); framework.start(); From 181fd99eeca3372ebe4dd4f2a017bd47dc497bd5 Mon Sep 17 00:00:00 2001 From: Kestrel-2409 Date: Tue, 13 Oct 2026 10:45:48 -0400 Subject: [PATCH 16/17] docs(changelog): startDiscordAwarenessDelivery is a new public method, not a newly public one Against main it is new: its private predecessor existed only on this branch, so "now public" would send a release-notes reader looking for an old private method. Co-Authored-By: Claude Opus 5.5 --- changelog.d/fix-surgery-marks-receipt.fixed.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/changelog.d/fix-surgery-marks-receipt.fixed.md b/changelog.d/fix-surgery-marks-receipt.fixed.md index 9e813e7d7..cb6d479a6 100644 --- a/changelog.d/fix-surgery-marks-receipt.fixed.md +++ b/changelog.d/fix-surgery-marks-receipt.fixed.md @@ -18,10 +18,10 @@ `branch.switch`, `undo` and `redo`, and `host/command` turn undo, return once the branch has moved, and their marks follow behind the same gate. Each starts its delivery through - `startDiscordAwarenessDelivery(after, agentName?)`, now public, which a - host can call after its own branch moves. A delivery that fails to start - raises a `discord-awareness-delivery` ops alert instead of failing the - command, and the next sync, reconnect or start delivers what is still + `startDiscordAwarenessDelivery(after, agentName?)`, a new public method + that a host can call after its own branch moves. A delivery that fails to + start raises a `discord-awareness-delivery` ops alert instead of failing + the command, and the next sync, reconnect or start delivers what is still pending. (The web switch used to be acknowledged only after its marks were attempted.) - A gate installed while a drain is still running (restoring the source From d81c7da6a199d1e2228bf6fd6ccf16d17c950765 Mon Sep 17 00:00:00 2001 From: Kestrel-2409 Date: Tue, 13 Oct 2026 10:53:22 -0400 Subject: [PATCH 17/17] docs(api): the web switch used to await the awareness sync, not every mark On main, cmdBranchSwitch awaited syncDiscordAwarenessMarkers. With nothing pending for the switch itself, as after a restore during a rollback's drain, that sync returned at once while the earlier drain still ran. So the switch was not always acknowledged only after its marks were attempted, as the changelog and the switch's comment said. Both now say it awaited the awareness sync. The comment also says the gate holds traffic that arrives after it, and that a mark is deferred while its server is disconnected. Co-Authored-By: Claude Opus 5.5 --- changelog.d/fix-surgery-marks-receipt.fixed.md | 4 ++-- src/api/server.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/changelog.d/fix-surgery-marks-receipt.fixed.md b/changelog.d/fix-surgery-marks-receipt.fixed.md index cb6d479a6..7aeb18faf 100644 --- a/changelog.d/fix-surgery-marks-receipt.fixed.md +++ b/changelog.d/fix-surgery-marks-receipt.fixed.md @@ -22,8 +22,8 @@ that a host can call after its own branch moves. A delivery that fails to start raises a `discord-awareness-delivery` ops alert instead of failing the command, and the next sync, reconnect or start delivers what is still - pending. (The web switch used to be acknowledged only after its marks were - attempted.) + pending. (The web switch used to await the awareness sync before + answering.) - A gate installed while a drain is still running (restoring the source branch mid-delivery, say) waits for that drain and then drains again from the ledger. So before traffic is released, every mark pending when the diff --git a/src/api/server.ts b/src/api/server.ts index ec4ddb7fe..21e648915 100644 --- a/src/api/server.ts +++ b/src/api/server.ts @@ -545,10 +545,10 @@ export class ApiServer { // Reversible Discord awareness markers are part of the branch projection. // The switch is acknowledged once it has landed: delivery of the marks it // adds and removes starts behind the MCPL data-plane gate, which holds - // channel traffic until they are attempted, and a failure is an ops alert - // rather than a failed switch. (It used to wait for every add and remove - // to be attempted, which after a large rollback took the drain's whole - // length.) + // channel traffic that arrives after it until they are attempted, or + // deferred while their server is disconnected, and a failure is an ops + // alert rather than a failed switch. (It used to await the awareness + // sync, which after a large rollback took the drain's whole length.) this.framework.startDiscordAwarenessDelivery('branch switch'); // Materialize config files from the new branch