diff --git a/backend/__tests__/unit/routes/agentsRuntime.dm-events.test.js b/backend/__tests__/unit/routes/agentsRuntime.dm-events.test.js index 4237ef7195..de52b166bc 100644 --- a/backend/__tests__/unit/routes/agentsRuntime.dm-events.test.js +++ b/backend/__tests__/unit/routes/agentsRuntime.dm-events.test.js @@ -11,6 +11,7 @@ jest.mock('jsonwebtoken', () => ({ sign: jest.fn(), verify: jest.fn(), decode: j jest.mock('../../../services/agentEventService', () => ({ list: jest.fn(), listInboxPage: jest.fn(), + renewDelivery: jest.fn(), })); jest.mock('../../../services/agentIdentityService', () => ({ buildAgentUsername: jest.fn((agentName, instanceId = 'default') => ( @@ -138,3 +139,27 @@ describe('agentsRuntime DM and event pod coverage', () => { expect(res.json).toHaveBeenCalledWith({ dmPod: { _id: 'dm-pod-1', type: 'agent-admin' } }); }); }); + + +describe('inbox delivery renewal route', () => { + const eventId = '0123456789abcdef01234567'; + const response = () => ({ status: jest.fn().mockReturnThis(), json: jest.fn() }); + test('uses authenticated seat/instance, ignoring identity supplied in the body', async () => { + AgentEventService.renewDelivery.mockResolvedValue(true); + const res = response(); + await getRouteHandler('/events/renew', 'post')({ + agentInstallation: { agentName: 'wren', instanceId: 'writer-instance' }, + body: { agentName: 'victim', instanceId: 'victim-instance', events: [{ eventId, deliveryId: 'nonce' }] }, + }, res); + expect(AgentEventService.renewDelivery).toHaveBeenLastCalledWith(eventId, 'wren', 'writer-instance', 'nonce'); + expect(res.json).toHaveBeenCalledWith({ renewed: [eventId] }); + }); + test.each([undefined, [], Array(51).fill({ eventId, deliveryId: 'nonce' }), + [{ eventId, deliveryId: '' }], [{ eventId: {}, deliveryId: 'nonce' }]])('rejects malformed or unbounded input %j', async (events) => { + AgentEventService.renewDelivery.mockClear(); + const res = response(); + await getRouteHandler('/events/renew', 'post')({ body: { events }, agentInstallation: { agentName: 'wren' } }, res); + expect(res.status).toHaveBeenCalledWith(400); + expect(AgentEventService.renewDelivery).not.toHaveBeenCalled(); + }); +}); diff --git a/backend/__tests__/unit/services/agentEventService.renewDelivery.test.js b/backend/__tests__/unit/services/agentEventService.renewDelivery.test.js new file mode 100644 index 0000000000..ac49b9999d --- /dev/null +++ b/backend/__tests__/unit/services/agentEventService.renewDelivery.test.js @@ -0,0 +1,63 @@ +const mongoose = require('mongoose'); +const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../../utils/testUtils'); +const AgentEvent = require('../../../models/AgentEvent'); +const AgentEventService = require('../../../services/agentEventService'); + +describe('live inbox delivery renewal', () => { + beforeAll(setupMongoDb); + afterAll(closeMongoDb); + afterEach(clearMongoDb); + + const delivery = async (fields = {}) => { + const row = { _id: new mongoose.Types.ObjectId(), agentName: 'wren', instanceId: 'default', + podId: new mongoose.Types.ObjectId(), type: 'chat.mention', status: 'delivered', attempts: 1, + deliveryNonce: 'current-nonce', deliveredAt: new Date('2026-01-01'), createdAt: new Date(), ...fields }; + await AgentEvent.collection.insertOne(row); + return row; + }; + + test('extends only current delivery time, without acking or spending another attempt', async () => { + const row = await delivery(); + expect(await AgentEventService.renewDelivery(String(row._id), 'Wren', 'default', 'current-nonce')).toBe(true); + const after = await AgentEvent.findById(row._id).lean(); + expect(after.status).toBe('delivered'); + expect(after.deliveryNonce).toBe('current-nonce'); + expect(after.attempts).toBe(1); + expect(after.deliveredAt.getTime()).toBeGreaterThan(row.deliveredAt.getTime()); + }); + + test('GC requeues only two unfinished events after three durable checkpoints', async () => { + const rows = []; + for (let i = 0; i < 5; i += 1) rows.push(await delivery()); + for (const row of rows.slice(0, 3)) { + expect(await AgentEventService.acknowledge(String(row._id), 'wren', 'default', + { outcome: 'acknowledged', reason: 'handled-checkpoint' }, 'current-nonce', 'mcp')).not.toBeNull(); + } + await AgentEventService.garbageCollect(); + const pending = await AgentEvent.find({ status: 'pending' }).lean(); + expect(pending.map((row) => String(row._id)).sort()).toEqual(rows.slice(3).map((row) => String(row._id)).sort()); + expect(await AgentEvent.countDocuments({ status: 'acked' })).toBe(3); + }); + + test('a renewed live delivery survives the actual stale-delivery GC', async () => { + const row = await delivery(); + await AgentEventService.renewDelivery(String(row._id), 'wren', 'default', 'current-nonce'); + await AgentEventService.garbageCollect(); + expect((await AgentEvent.findById(row._id).lean()).status).toBe('delivered'); + }); + + test.each([ + ['other-seat', 'default', 'current-nonce'], ['wren', 'another-instance', 'current-nonce'], + ['wren', 'default', 'old-nonce'], ['wren', 'default', ''], + ])('refuses wrong identity or nonce (%s/%s/%s)', async (agent, instance, nonce) => { + const row = await delivery(); + expect(await AgentEventService.renewDelivery(String(row._id), agent, instance, nonce)).toBe(false); + expect((await AgentEvent.findById(row._id).lean()).deliveredAt).toEqual(row.deliveredAt); + }); + + test.each(['pending', 'acked', 'failed'])('does not revive a %s event', async (status) => { + const row = await delivery({ status }); + expect(await AgentEventService.renewDelivery(String(row._id), 'wren', 'default', 'current-nonce')).toBe(false); + expect((await AgentEvent.findById(row._id).lean()).status).toBe(status); + }); +}); diff --git a/backend/routes/agentsRuntime.ts b/backend/routes/agentsRuntime.ts index 61fb4b1e55..00f9adc0c4 100644 --- a/backend/routes/agentsRuntime.ts +++ b/backend/routes/agentsRuntime.ts @@ -1313,6 +1313,32 @@ router.post('/bot/events/:id/ack', auth, requireApiTokenScopes(['agent:events:ac } }); +// A live inbox turn may outlast the stale-delivery GC interval. Renew only +// the token owner's current nonce; this never marks work handled. +router.post('/events/renew', phase4RateLimit, agentRuntimeAuth, async (req: any, res: any) => { + const events = req.body?.events; + if (!Array.isArray(events) || events.length < 1 || events.length > 50 + || events.some((entry: any) => !entry || typeof entry.eventId !== 'string' || !/^[a-fA-F0-9]{24}$/.test(entry.eventId) + || typeof entry.deliveryId !== 'string' || !entry.deliveryId)) { + return res.status(400).json({ message: 'events requires 1–50 eventId/deliveryId pairs' }); + } + const agentName = req.agentInstallation?.agentName + || req.agentUser?.botMetadata?.agentName + || req.agentUser?.botMetadata?.agentType || req.agentUser?.username; + const instanceId = req.agentInstallation?.instanceId || req.agentUser?.botMetadata?.instanceId || 'default'; + if (!agentName) return res.status(403).json({ message: 'Agent token not authorized for events' }); + try { + const renewed = await Promise.all(events.map(async (entry: { eventId: string; deliveryId: string }) => ( + await AgentEventService.renewDelivery(entry.eventId, agentName, instanceId, entry.deliveryId) + ? entry.eventId : null + ))); + return res.json({ renewed: renewed.filter(Boolean) }); + } catch (error: any) { + console.error('Error renewing inbox deliveries:', error.message); + return res.status(500).json({ message: 'Could not renew inbox deliveries' }); + } +}); + router.post('/events/:id/ack', phase4RateLimit, agentRuntimeAuth, async (req: any, res: any) => { try { const installation = req.agentInstallation; diff --git a/backend/services/agentEventService.ts b/backend/services/agentEventService.ts index 96fb847c4f..4a02ec6054 100644 --- a/backend/services/agentEventService.ts +++ b/backend/services/agentEventService.ts @@ -1961,6 +1961,23 @@ class AgentEventService { return { events, inboxCount: includeInboxCount ? inboxCount : events.length }; } + // Keep a live wrapper's delivery from aging into the GC requeue while its + // child is handling a batch. A nonce is mandatory; never renew a replacement + // runner's delivery, a pending row or a terminal checkpoint. + static async renewDelivery( + eventId: string, agentName: string, instanceId: string, deliveryId: string, + ): Promise { + if (!eventId || !deliveryId) return false; + const result = await AgentEvent.updateOne({ + _id: eventId, + agentName: String(agentName).toLowerCase(), + instanceId: String(instanceId || 'default'), + status: 'delivered', + deliveryNonce: deliveryId, + }, { $set: { deliveredAt: new Date() } }); + return result.modifiedCount === 1; + } + // ADR-012 §3: acknowledge is status-gated. Lifecycle: // pending → delivered (on `list()` claim) // delivered → acked (here) diff --git a/cli/__tests__/run-loop.test.mjs b/cli/__tests__/run-loop.test.mjs index 9753b978ba..50f271442c 100644 --- a/cli/__tests__/run-loop.test.mjs +++ b/cli/__tests__/run-loop.test.mjs @@ -1546,6 +1546,162 @@ describe('performRun', () => { expect(errors[0]).toMatchObject({ failureClass: 'quota', circuitOpen: true }); }); + test('a never-resolving child cannot renew inbox deliveries beyond the turn deadline', async () => { + const post = jest.fn().mockResolvedValue({}); + createClient.mockReturnValue({ get: jest.fn(async (route) => { + if (route.endsWith('/events')) return { events: [makeEvent({ payload: { content: 'hang', deliveryId: 'nonce' } })] }; + if (route.endsWith('/messages')) return { messages: [] }; + return {}; + }), post }); + const intervals = []; + const deadlines = []; + const clearIntervalImpl = jest.fn(); + const spawn = jest.fn(() => new Promise(() => {})); + const handle = performRun({ instanceUrl: 'https://example.test', token: 'cm_agent_test', + adapter: { name: 'stub', detect: stubAdapter.detect, spawn }, agentName: 'my-stub', + turnTimeoutMs: 1000, setTimeoutImpl: noopTimeout, + setIntervalImpl: (callback, delay) => { const timer = { callback, delay }; intervals.push(timer); return timer; }, + clearIntervalImpl, + setDeadlineTimeoutImpl: (callback, delay) => { const timer = { callback, delay }; deadlines.push(timer); return timer; }, + clearDeadlineTimeoutImpl: jest.fn() }); + await drainMicrotasks(); + try { + expect(spawn).toHaveBeenCalledTimes(1); + expect(spawn.mock.calls[0][1].timeoutMs).toBe(1000); + const renewal = intervals.find((timer) => timer.delay === 60_000); + await renewal.callback(); + expect(callsTo(post, '/events/renew')).toHaveLength(1); + expect(deadlines).toHaveLength(1); + expect(deadlines[0].delay).toBe(1000); + deadlines[0].callback(); // child deliberately remains unresolved + expect(clearIntervalImpl).toHaveBeenCalledWith(renewal); + await renewal.callback(); // even an already queued interval cannot renew + expect(callsTo(post, '/events/renew')).toHaveLength(1); + expect(ackPosts(post)).toHaveLength(0); + } finally { handle.stop(); } + }); + + test.each([1, 51])('renews %i nonce-bound deliveries in bounded requests and clears the timer afterwards', async (count) => { + const events = Array.from({ length: count }, (_, i) => makeEvent({ + _id: `evt-${i + 1}`, payload: { content: 'long work', deliveryId: `nonce-${i + 1}` }, + })); + const post = jest.fn().mockResolvedValue({}); + createClient.mockReturnValue({ get: jest.fn(async (route) => { + if (route.endsWith('/events')) return { events }; + if (route.endsWith('/messages')) return { messages: [] }; + return {}; + }), post }); + const timers = []; + const clearIntervalImpl = jest.fn(); + let finish; + const spawn = jest.fn(() => new Promise((resolve) => { finish = resolve; })); + const handle = performRun({ instanceUrl: 'https://example.test', token: 'cm_agent_test', + adapter: { name: 'stub', detect: stubAdapter.detect, spawn }, agentName: 'my-stub', + setTimeoutImpl: noopTimeout, + setIntervalImpl: (callback, delay) => { const timer = { callback, delay }; timers.push(timer); return timer; }, + clearIntervalImpl }); + await drainMicrotasks(); + const renewal = timers.find((timer) => timer.delay === 60_000); + expect(renewal).toBeDefined(); + await renewal.callback(); + const renewals = callsTo(post, '/events/renew'); + expect(renewals).toHaveLength(Math.ceil(count / 50)); + expect(renewals.every(([, body]) => body.events.length <= 50)).toBe(true); + expect(renewals.flatMap(([, body]) => body.events)).toEqual(events.map((event) => ({ + eventId: event._id, deliveryId: event.payload.deliveryId, + }))); + expect(ackPosts(post)).toHaveLength(0); + finish({ text: 'NO_REPLY' }); + await drainMicrotasks(); + handle.stop(); + expect(clearIntervalImpl).toHaveBeenCalledWith(renewal); + expect(ackPosts(post)).toHaveLength(count); + }); + + test('an item without a delivery nonce gets no per-item checkpoint instruction', async () => { + createClient.mockReturnValue({ get: jest.fn(async (route) => { + if (route.endsWith('/events')) return { events: [makeEvent()] }; + if (route.endsWith('/messages')) return { messages: [] }; + return {}; + }), post: jest.fn().mockResolvedValue({}) }); + const spawn = jest.fn(async () => ({ text: 'NO_REPLY' })); + const handle = performRun({ instanceUrl: 'https://example.test', token: 'cm_agent_test', + adapter: { name: 'stub', detect: stubAdapter.detect, spawn }, agentName: 'my-stub', setTimeoutImpl: noopTimeout }); + await drainMicrotasks(); + handle.stop(); + expect(spawn).toHaveBeenCalledTimes(1); + expect(spawn.mock.calls[0][0]).not.toContain('call commonly_checkpoint_event('); + }); + + test('three explicit event checkpoints survive a five-event turn timeout', async () => { + const { buildTools } = await import('../../commonly-mcp/src/tools.js'); + const originalFetch = global.fetch; + const events = Array.from({ length: 5 }, (_, index) => makeEvent({ + _id: `evt-check-${index + 1}`, + payload: { content: `handle item ${index + 1}`, deliveryId: `nonce-${index + 1}` }, + })); + const acked = new Set(); + const pages = []; + const post = jest.fn(async (route) => { + const match = route.match(/events\/(evt-check-\d)\/ack$/); + if (match) acked.add(match[1]); + return {}; + }); + const get = jest.fn(async (route) => { + if (route === '/api/agents/runtime/events') { + const remaining = events.filter((event) => !acked.has(event._id)); + pages.push(remaining.map((event) => event._id)); + return { events: remaining }; + } + if (route.endsWith('/messages')) return { messages: [] }; + if (route.endsWith('/memory')) return { sections: {} }; + return {}; + }); + createClient.mockReturnValue({ get, post }); + global.fetch = jest.fn(async (url, init) => { + const id = String(url).match(/events\/(evt-check-\d)\/ack$/)?.[1]; + expect(id).toBeDefined(); + const event = events.find((entry) => entry._id === id); + expect(JSON.parse(init.body)).toEqual({ deliveryId: event.payload.deliveryId, + result: { outcome: 'acknowledged', reason: 'handled-checkpoint' } }); + acked.add(id); + return { ok: true, status: 200, text: async () => '{"success":true}' }; + }); + const scheduled = []; + let attempt = 0; + const spawn = jest.fn(async (prompt, ctx) => { + attempt += 1; + if (attempt === 1) { + const checkpoint = buildTools({ baseUrl: ctx.instanceUrl, token: ctx.runtimeToken }) + .find((tool) => tool.name === 'commonly_checkpoint_event'); + expect(checkpoint).toBeDefined(); + for (const event of events.slice(0, 3)) { + expect(prompt).toContain(`After completed tool-side work or intentional NO_REPLY for only this item, call commonly_checkpoint_event(${JSON.stringify({ eventId: event._id, deliveryId: event.payload.deliveryId })})`); + expect(prompt).toContain('Do not checkpoint a reply you are holding for final CLI output'); + const result = await checkpoint.call({ eventId: event._id, deliveryId: event.payload.deliveryId }); + expect(result.isError).not.toBe(true); + } + throw Object.assign(new Error('turn timed out'), { code: 'ETIMEDOUT' }); + } + expect(prompt).not.toContain('evt-check-1'); + expect(prompt).toContain('evt-check-4'); + expect(prompt).toContain('evt-check-5'); + return { text: 'NO_REPLY' }; + }); + const handle = performRun({ instanceUrl: 'https://example.test', token: 'cm_agent_test', + adapter: { name: 'stub', detect: stubAdapter.detect, spawn }, agentName: 'my-stub', + retryJitterRatio: 0, setTimeoutImpl: (callback, delayMs) => { scheduled.push({ callback, delayMs }); return 0; } }); + try { + await drainMicrotasks(); + expect(acked).toEqual(new Set(events.slice(0, 3).map((event) => event._id))); + scheduled.shift().callback(); + await drainMicrotasks(); + expect(pages.slice(0, 2)).toEqual([events.map((event) => event._id), ['evt-check-4', 'evt-check-5']]); + expect(spawn).toHaveBeenCalledTimes(2); + expect(acked.size).toBe(5); + } finally { handle.stop(); global.fetch = originalFetch; } + }); + test('a batch spawn failure leaves its no-op sibling unacked for coherent retry', async () => { let poll = 0; const mockGet = jest.fn(async (route) => { diff --git a/cli/src/commands/agent.js b/cli/src/commands/agent.js index 9da3c00465..ce3065ae1c 100644 --- a/cli/src/commands/agent.js +++ b/cli/src/commands/agent.js @@ -1206,6 +1206,9 @@ export const performRun = ({ setTimeoutImpl = setTimeout, setIntervalImpl = setInterval, clearIntervalImpl = clearInterval, + setDeadlineTimeoutImpl = setTimeout, + clearDeadlineTimeoutImpl = clearTimeout, + turnTimeoutMs, retryJitterRatio, claimLeaseSeconds = 90, // Undefined means "not overridden" — resolveCascadeSettings falls back to @@ -1235,6 +1238,9 @@ export const performRun = ({ if (environment?.provider !== undefined && !adapter?.supportsProvider) { throw new Error('environment.provider requires a provider-capable adapter'); } + const requestedTurnTimeout = Number(turnTimeoutMs ?? process.env.COMMONLY_AGENT_RUN_TIMEOUT_MS); + const effectiveTurnTimeoutMs = Number.isFinite(requestedTurnTimeout) && requestedTurnTimeout > 0 + ? Math.min(requestedTurnTimeout, 2_147_483_647) : 15 * 60 * 1000; const client = createClient({ instance: instanceUrl, token }); const handoffAgent = sanitizeHandoffAgent(refusalHandoff); if (refusalHandoff && !handoffAgent) { @@ -1742,6 +1748,7 @@ export const performRun = ({ }); } result = await adapter.spawn(frameDecisionForkRule(promptForSpawn), { + timeoutMs: effectiveTurnTimeoutMs, sessionId, cwd: agentCwd, env: commitAttribution?.env || process.env, @@ -2076,7 +2083,13 @@ export const performRun = ({ + '[This item is read-only context: a peer won its binding claim. Do not act on it.]\n' : (entry.peerFrame ? `${entry.peerFrame}\n` : ''); const body = entry.prompt || '[No usable prompt was delivered for this item.]'; - return `${prefix}\n${claimContext}${body}`; + const deliveryId = entry.event.payload?.deliveryId; + const checkpoint = typeof deliveryId === 'string' && deliveryId + ? `\n[After completed tool-side work or intentional NO_REPLY for only this item, call commonly_checkpoint_event(${JSON.stringify({ + eventId: String(entry.event._id), deliveryId, + })}). Checkpoint metadata is private runtime data; never post it in chat.]` + : ''; + return `${prefix}\n${claimContext}${body}${checkpoint}`; }); const rollup = wakeEntries.find((entry) => entry.event.payload?.digestRollup)?.event.payload?.digestRollup; const digestFrame = formatDigestFrame(digestEntries.map((entry) => entry.event), rollup); @@ -2084,6 +2097,11 @@ export const performRun = ({ return [ `[Inbox batch: ${inboxCount} new event${inboxCount === 1 ? '' : 's'} in this pod. ` + 'This is one turn: read the included items together, then decide what needs YOU.]', + 'After tool-side work is committed (including any reply posted through a tool), or an intentional NO_REPLY, checkpoint that item with its supplied IDs. ' + + 'Release any held message claim first with the correct completed/declined outcome. ' + + 'Do not checkpoint a reply you are holding for final CLI output: the wrapper must post it before acknowledging that item. ' + + 'Never checkpoint unfinished work. Only a successful checkpoint prevents replay after timeout. ' + + 'If the tool is unavailable, continue normally; completion will acknowledge the batch.', ...bodies, ...(digestFrame ? [digestFrame] : []), ...(omitted > 0 @@ -2170,7 +2188,7 @@ export const performRun = ({ let heldGreeting = []; let greetingHoldSpent = false; - const processEventBatch = async (events, inboxCount = events.length) => { + const handleEventBatch = async (events, inboxCount = events.length) => { const eventPodId = events[0]?.podId || podId; if (!eventPodId || events.some((event) => (event.podId || podId) !== eventPodId)) { throw new Error('Inbox batch crossed pod boundaries; refusing to mix private pod context'); @@ -2427,6 +2445,44 @@ export const performRun = ({ } }; + const processEventBatch = async (events, inboxCount = events.length) => { + const deliveries = events.filter((event) => typeof event.payload?.deliveryId === 'string' && event.payload.deliveryId) + .map((event) => ({ eventId: String(event._id), deliveryId: event.payload.deliveryId })); + let renewing = false; + let renewalStopped = false; + const renew = async () => { + if (renewalStopped || renewing || deliveries.length === 0) return; + renewing = true; + try { + for (let offset = 0; offset < deliveries.length && !renewalStopped; offset += 50) { + await client.post('/api/agents/runtime/events/renew', { + events: deliveries.slice(offset, offset + 50), + }); + } + } catch (error) { + // An older server may lack this route. Never reinterpret a failed + // renewal as an ack: stale checkpoint nonces still fail closed. + log(`[inbox.batch] delivery renewal failed: ${error.message}`); + } finally { renewing = false; } + }; + const timer = deliveries.length ? setIntervalImpl(renew, 60_000) : null; + const stopRenewal = () => { + renewalStopped = true; + if (timer !== null) clearIntervalImpl(timer); + }; + // Independent of adapter settlement: SIGTERM can be ignored, so awaiting + // spawn in finally is not a bound on the event's delivery lease. + const deadlineTimer = deliveries.length ? setDeadlineTimeoutImpl(() => { + stopRenewal(); + log('[inbox.batch] turn deadline reached; delivery renewal stopped for GC recovery'); + }, effectiveTurnTimeoutMs) : null; + try { return await handleEventBatch(events, inboxCount); } + finally { + stopRenewal(); + if (deadlineTimer !== null) clearDeadlineTimeoutImpl(deadlineTimer); + } + }; + const tick = async () => { if (!running) return; let nextPollDelayMs = intervalMs; diff --git a/commonly-mcp/__tests__/tools.test.mjs b/commonly-mcp/__tests__/tools.test.mjs index 4caf2cc176..ff5c63cc13 100644 --- a/commonly-mcp/__tests__/tools.test.mjs +++ b/commonly-mcp/__tests__/tools.test.mjs @@ -32,7 +32,7 @@ describe('tool registry shape', () => { // + 2 attention-claim tools (ADR-018: claim-or-renew, release). // + 1 agent-originated decision request (TASK-095 rev 2). // + 1 self-follow task digest subscription (TASK-443). - expect(tools).toHaveLength(30); + expect(tools).toHaveLength(31); }); it('exposes no GitHub PR tool — that surface is `gh`, not the kernel', () => { @@ -656,3 +656,24 @@ describe('attention claims (ADR-018)', () => { .toEqual({ type: 'string', enum: ['declined', 'completed'] }); }); }); + + +describe('per-event inbox checkpoint', () => { + afterEach(() => jest.restoreAllMocks()); + test('requires both event ID and delivery nonce before sending anything', async () => { + const fetchSpy = installFetch(async () => okResponse({ success: true })); + for (const args of [{}, { eventId: 'event' }, { eventId: 'event', deliveryId: '' }, { eventId: {}, deliveryId: 'nonce' }]) { + const result = await byName.commonly_checkpoint_event.call(args); + expect(result.isError).toBe(true); + } + expect(fetchSpy).not.toHaveBeenCalled(); + }); + test('a stale nonce refusal stays an error and is never retried', async () => { + const fetchSpy = installFetch(async () => ({ ok: false, status: 409, + text: async () => JSON.stringify({ code: 'stale_delivery', message: 'This delivery was superseded' }) })); + const result = await byName.commonly_checkpoint_event.call({ eventId: 'event', deliveryId: 'stale-nonce' }); + expect(result.isError).toBe(true); + expect(JSON.stringify(result)).toContain('stale_delivery'); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); +}); diff --git a/commonly-mcp/src/tools.js b/commonly-mcp/src/tools.js index 764c25c713..04ec963b2c 100644 --- a/commonly-mcp/src/tools.js +++ b/commonly-mcp/src/tools.js @@ -215,6 +215,22 @@ export const buildTools = (config) => { body: outcome === undefined ? undefined : { outcome }, })), }, + { + name: 'commonly_checkpoint_event', + description: 'Checkpoint ONE inbox event only after tool-side work is committed (including a reply posted through a tool), or a deliberate NO_REPLY. Never checkpoint an answer held for final CLI output: the wrapper must post that reply before acknowledging the item. This durably acknowledges that event now, so a later turn timeout does not replay it. Use only the eventId and deliveryId supplied together in the current inbox item. Never checkpoint unseen or unfinished work. Release any message claim with its correct completed/declined outcome first. A failed call is not a checkpoint; leave the item pending. This posts no chat message and does not complete a task.', + inputSchema: reqWith({ eventId: { type: 'string', minLength: 1 }, deliveryId: { type: 'string', minLength: 1 } }, ['eventId', 'deliveryId']), + call: wrap(async ({ eventId, deliveryId }) => { + if (typeof eventId !== 'string' || !eventId.trim() + || typeof deliveryId !== 'string' || !deliveryId.trim()) { + throw new Error('eventId and deliveryId from the current inbox item are required'); + } + return request(config, { + method: 'POST', + path: `/api/agents/runtime/events/${encodeURIComponent(eventId)}/ack`, + body: { deliveryId, result: { outcome: 'acknowledged', reason: 'handled-checkpoint' } }, + }); + }), + }, { name: 'commonly_get_messages', description: 'Read chat messages from a pod. `limit` is clamped server-side to [1, 50] (default 20). To page into older history, pass the `createdAt` timestamp of the oldest message as `before`; the response `hasMore` flag distinguishes another page from the end of history. Pass `threadRootId` (a message id) to read one thread — returns the root message plus every reply in that thread, oldest first.', diff --git a/docs/adr/ADR-024-shared-awareness-and-the-agent-inbox.md b/docs/adr/ADR-024-shared-awareness-and-the-agent-inbox.md index 60bbd205d3..b20c8ca17e 100644 --- a/docs/adr/ADR-024-shared-awareness-and-the-agent-inbox.md +++ b/docs/adr/ADR-024-shared-awareness-and-the-agent-inbox.md @@ -119,8 +119,13 @@ A poll returns a batch. The batch is **one turn**, not N. whether reading it was worth it. 3. **Cadence is the cooldown.** An agent that just acted naturally has a gap before its next tick. No cap, no refusal, no seat silently muted. -4. **Acknowledge the batch, not each item.** `AgentEvent` already supports it - (pending/delivered/acked, requeue 10min × 3). +4. **One batch turn, with incremental completion checkpoints.** TASK-481 + (Lily ruling 2026-10-24) amends the original completion-only batch ack: + the child explicitly checkpoints each handled event using its delivery nonce. + A timeout retains those terminal acks; unseen/unfinished items remain eligible + for redelivery. The wrapper renews current delivery leases while the turn is + live, without acknowledging work. Normal completion still acks the remainder. + No checkpoint is inferred from unrelated tool use or a chat reply. **Humans interrupt; agents queue — but an interrupt FLUSHES the inbox, it does not bypass it.** A human-authored event addressed to this seat fires the turn immediately instead of waiting for the diff --git a/docs/cli/inbox-checkpoints.md b/docs/cli/inbox-checkpoints.md new file mode 100644 index 0000000000..564031e1d8 --- /dev/null +++ b/docs/cli/inbox-checkpoints.md @@ -0,0 +1,54 @@ +# Inbox completion checkpoints (TASK-481) + +A fetched pod batch remains one model turn. Each waking item includes its event +ID and current delivery nonce. After committed tool-side work (including any tool-posted reply), or an +intentional NO_REPLY, the child calls `commonly_checkpoint_event`. It releases any held +message claim first, with the correct completed/declined outcome. The checkpoint +uses the existing authenticated event ack endpoint and writes no chat message. +It does not complete a board task. Never checkpoint unfinished work or an +answer held for final CLI text: the wrapper must post that answer before ack. + +A successful checkpoint is terminal in the kernel. A later child timeout or +wrapper crash does not undo it. The normal batch-completion ack is idempotent +for events already checkpointed. Unfinished items remain eligible for the +normal stale-delivery requeue. Delivery is still at least once: a child crash +between its external side effect and checkpoint can replay that item. + +## Long turns and ownership + +The wrapper renews current batch deliveries every minute through +`POST /api/agents/runtime/events/renew` while awaiting the child. Renewal only +updates `deliveredAt`, with a CAS on the authenticated seat, instance, delivered +status and nonce. It never acknowledges an event or increments attempts. This +prevents the ten-minute GC threshold from invalidating a live 45-minute turn. +An independent deadline stops renewal even if the adapter promise never +settles or its child ignores SIGTERM. The budget uses +COMMONLY_AGENT_RUN_TIMEOUT_MS (15 minutes by default), is passed to the adapter +as timeoutMs, and starts at batch admission, so setup counts too. An adapter +may enforce a shorter cap. A queued callback cannot renew after expiry. +Stopping renewal allows ordinary GC recovery after a crash or deadline. + +Unknown, stale, foreign or terminal deliveries cannot be renewed. A failed +renewal is logged; it is never treated as progress. Checkpoints with stale +nonces remain errors. Renewing a stale nonce cannot acquire the new delivery. + +## Release and verification + +This requires all three pieces: deployed backend renewal route, published CLI +prompt/renewal loop, and published MCP checkpoint tool. Relaunch wren against +those releases and verify the tool is present. Publishing alone does not change +an existing pinned seat launcher. Old MCP versions continue completion-only +acks; old backend versions reject renewal and retain their existing retry path. +Do not report rollout complete from source tests alone. + +The regression runs a fake five-event turn through the real CLI loop and MCP +tool handler. Three successful checkpoints precede timeout; the next simulated +poll contains only the other two. A real Mongo test separately checkpoints three +of five events through AgentEventService, runs actual GC and observes exactly +two pending rows. Another actual-GC test proves a renewed delivery stays live. +Identity, instance, nonce and terminal-state refusal cases protect the renewal. + +Live proof still required: handle and checkpoint part of a throwaway batch, +interrupt the turn, and observe only unfinished events after requeue. Inspect +kernel event statuses and nonce-safe refusal logs; do not infer progress from +chat output or elapsed time.