diff --git a/cli/__tests__/enforcement.test.mjs b/cli/__tests__/enforcement.test.mjs index 733565293b..31a338b7d8 100644 --- a/cli/__tests__/enforcement.test.mjs +++ b/cli/__tests__/enforcement.test.mjs @@ -450,6 +450,29 @@ describe('deliverChatReply', () => { expect(post).toHaveBeenCalledWith(messagesPath, { content: 'short answer' }); }); + test('a normal-return refusal is not recorded as a single-message delivery', async () => { + // The runtime route returns HTTP 200 for this policy refusal. The resolved + // promise proves only that the server decided, not that it created a row. + const post = jest.fn().mockResolvedValue({ + success: false, + refused: true, + reason: 'consecutive_run_cap', + consecutive: 3, + guidance: 'Do not retry this message unchanged.', + }); + const res = await deliverChatReply({ client: { post }, podId: 'pod-1', text: 'short answer' }); + + expect(res).toEqual({ + mode: 'refused', + messages: 0, + attemptedMessages: 1, + refused: true, + reason: 'consecutive_run_cap', + consecutive: 3, + guidance: 'Do not retry this message unchanged.', + }); + }); + test('a split-sized reply posts its chunks in order', async () => { const post = jest.fn().mockResolvedValue({}); const text = `${'a'.repeat(390)}\n\n${'b'.repeat(390)}`; @@ -459,6 +482,37 @@ describe('deliverChatReply', () => { expect(post.mock.calls[1][1].content).toBe('b'.repeat(390)); }); + test('a split reply stops at a normal-return refusal and reports only delivered chunks', async () => { + const refusal = { + success: false, + refused: true, + reason: 'consecutive_run_cap', + consecutive: 3, + guidance: 'Wait for someone else to speak.', + }; + const post = jest.fn() + .mockResolvedValueOnce({ success: true }) + .mockResolvedValueOnce({ success: true }) + .mockResolvedValueOnce(refusal); + const text = `${'a'.repeat(390)}\n\n${'b'.repeat(390)}\n\n${'c'.repeat(390)}`; + + const res = await deliverChatReply({ client: { post }, podId: 'pod-1', text }); + + expect(res).toEqual({ + mode: 'refused', + messages: 2, + attemptedMessages: 3, + refused: true, + reason: 'consecutive_run_cap', + consecutive: 3, + guidance: 'Wait for someone else to speak.', + }); + expect(post).toHaveBeenCalledTimes(3); + expect(post.mock.calls.map(([, body]) => body.content)).toEqual([ + 'a'.repeat(390), 'b'.repeat(390), 'c'.repeat(390), + ]); + }); + // Prose overflow is a THREAD now, not an attachment (Sam 57691). // // These pin the rung itself, not just its return value. The old behaviour @@ -507,6 +561,54 @@ describe('deliverChatReply', () => { expect(res.threadRootId).toBe('99'); }); + test('a refused thread headline stops without falling through to an attachment', async () => { + const refusal = { + refused: true, + reason: 'consecutive_run_cap', + guidance: 'Wait for someone else to speak.', + }; + const post = jest.fn().mockResolvedValue(refusal); + const upload = jest.fn(); + + const res = await deliverChatReply({ client: { post, upload }, podId: 'pod-1', text: prose }); + + expect(res).toEqual({ + mode: 'refused', + messages: 0, + attemptedMessages: splitForChat(prose).length, + refused: true, + reason: 'consecutive_run_cap', + guidance: 'Wait for someone else to speak.', + }); + expect(post).toHaveBeenCalledTimes(1); + expect(upload).not.toHaveBeenCalled(); + }); + + test('a refused continuation stops without re-posting the remaining thread as top-level messages', async () => { + const refusal = { + refused: true, + reason: 'consecutive_run_cap', + guidance: 'Wait for someone else to speak.', + }; + const post = jest.fn() + .mockResolvedValueOnce({ message: { id: 'root-1' } }) + .mockResolvedValueOnce({ success: true }) + .mockResolvedValueOnce(refusal); + const upload = jest.fn(); + + const res = await deliverChatReply({ client: { post, upload }, podId: 'pod-1', text: prose }); + + expect(res).toMatchObject({ + mode: 'refused', + messages: 2, + attemptedMessages: splitForChat(prose).length, + reason: 'consecutive_run_cap', + }); + expect(post).toHaveBeenCalledTimes(3); + expect(post.mock.calls.slice(1).every(([, body]) => body.threadRootId === 'root-1')).toBe(true); + expect(upload).not.toHaveBeenCalled(); + }); + test('never guesses a root, and never re-posts the headline it already sent', async () => { // A continuation posted with a missing root becomes another TOP-LEVEL // message, so refusing is the only safe read of an unknown response. @@ -561,6 +663,22 @@ describe('deliverChatReply', () => { expect(post.mock.calls[0][1].content).toContain('Point 0:'); // opening stays the headline }); + test('an attachment-card refusal does not masquerade as an attachment delivery', async () => { + const post = jest.fn().mockResolvedValue({ + refused: true, reason: 'consecutive_run_cap', guidance: 'Do not retry unchanged.', + }); + const upload = jest.fn().mockResolvedValue({ fileName: 'srv.md', originalName: 'reply.md' }); + const text = `\`\`\`js\n${'const x = 1;\n'.repeat(72)}\`\`\``; + + const res = await deliverChatReply({ client: { post, upload }, podId: 'pod-1', text }); + + expect(res).toMatchObject({ + mode: 'refused', messages: 0, attemptedMessages: 1, reason: 'consecutive_run_cap', + }); + expect(post).toHaveBeenCalledTimes(1); + expect(upload).toHaveBeenCalledTimes(1); + }); + test('a document-sized single fence attaches — it cannot ride the single-post branch (msg 53018)', async () => { const post = jest.fn().mockResolvedValue({}); const upload = jest.fn().mockResolvedValue({ @@ -667,6 +785,24 @@ describe('deliverChatReply', () => { expect(post.mock.calls.slice(4).every((c) => c[1].threadRootId === undefined)).toBe(true); expect(log).toHaveBeenCalledWith(expect.stringContaining('posting the remainder top-level')); }); + + test('the attachment fallback also stops at a normal-return refusal', async () => { + const post = jest.fn() + .mockResolvedValueOnce({ success: true }) + .mockResolvedValueOnce({ + refused: true, reason: 'consecutive_run_cap', guidance: 'Wait for a reply first.', + }); + const upload = jest.fn().mockRejectedValue(new Error('older server')); + const fence = `\`\`\`js\n${'const x = 1;\n'.repeat(72)}\`\`\``; + const text = `${'y'.repeat(350)}\n\n${fence}\n\n${'z'.repeat(350)}`; + + const res = await deliverChatReply({ client: { post, upload }, podId: 'pod-1', text }); + + expect(res).toMatchObject({ + mode: 'refused', messages: 1, attemptedMessages: splitForChat(text).length, reason: 'consecutive_run_cap', + }); + expect(post).toHaveBeenCalledTimes(2); + }); }); describe('createClaimHandicap', () => { diff --git a/cli/__tests__/run-loop.test.mjs b/cli/__tests__/run-loop.test.mjs index 0f45b3de4d..6716a99ef3 100644 --- a/cli/__tests__/run-loop.test.mjs +++ b/cli/__tests__/run-loop.test.mjs @@ -125,6 +125,73 @@ describe('performRun', () => { ); }); + test('a normal-return run-cap refusal is acked as a refusal, not a posted reply', async () => { + // The post route deliberately responds 200 with { refused: true }. This + // is terminal guidance — retrying the same event would duplicate the two + // chunks that did land — so the wrapper must expose it locally and ack the + // event as no_action rather than throw into at-least-once redelivery. + const guidance = 'Wait for someone else to speak; do not retry unchanged.'; + let messagePosts = 0; + const mockGet = jest.fn().mockResolvedValue({ events: [makeEvent({ _id: 'evt-run-cap' })] }); + const mockPost = jest.fn(async (route) => { + if (route === '/api/agents/runtime/pods/pod-abc/messages') { + messagePosts += 1; + return messagePosts < 3 + ? { success: true } + : { + success: false, + refused: true, + reason: 'consecutive_run_cap', + consecutive: 3, + guidance, + }; + } + return {}; + }); + createClient.mockReturnValue({ get: mockGet, post: mockPost }); + const onError = jest.fn(); + const text = `${'a'.repeat(390)}\n\n${'b'.repeat(390)}\n\n${'c'.repeat(390)}`; + const spawn = jest.fn(async () => ({ text })); + const adapter = { name: 'stub', detect: stubAdapter.detect, spawn }; + + const { stop } = performRun({ + instanceUrl: 'http://localhost:5000', + token: 'cm_agent_test', + adapter, + agentName: 'my-stub', + onError, + setTimeoutImpl: noopTimeout, + }); + await drainMicrotasks(); + stop(); + + expect(mockPost.mock.calls.filter(([route]) => route === '/api/agents/runtime/pods/pod-abc/messages')) + .toHaveLength(3); + expect(mockPost).toHaveBeenCalledWith( + '/api/agents/runtime/events/evt-run-cap/ack', + { + result: { + outcome: 'no_action', + reason: 'consecutive_run_cap', + details: { + mode: 'refused', + postedMessages: 2, + attemptedMessages: 3, + consecutive: 3, + guidance, + }, + }, + }, + ); + expect(onError).toHaveBeenCalledWith(expect.objectContaining({ + code: 'agent_delivery_refused', + reason: 'consecutive_run_cap', + postedMessages: 2, + attemptedMessages: 3, + })); + expect(onError.mock.calls[0][0].message).toContain(guidance); + }); + test('first_contact event is forwarded to the adapter like a mention', async () => { const events = [makeEvent({ _id: 'evt-first-contact', diff --git a/cli/package.json b/cli/package.json index bf3549222d..b60f2f1227 100644 --- a/cli/package.json +++ b/cli/package.json @@ -1,6 +1,6 @@ { "name": "@commonlyai/cli", - "version": "0.1.19", + "version": "0.1.20", "license": "Apache-2.0", "description": "The Commonly CLI \u2014 connect agents, manage pods, iterate fast", "type": "module", diff --git a/cli/src/commands/agent.js b/cli/src/commands/agent.js index 9f7e10436f..c5406367f1 100644 --- a/cli/src/commands/agent.js +++ b/cli/src/commands/agent.js @@ -1069,6 +1069,7 @@ export const performRun = ({ && /^(HEARTBEAT_OK|HEARTBEAT_NOOP)$/i.test(replyText); const silentReply = !replyText || replyText === 'NO_REPLY' || heartbeatControlReply; let delivered = agentPostedItself; + let deliveryRefusal = null; if (event.type === 'agent.ask') { if (silentReply) { @@ -1154,11 +1155,32 @@ export const performRun = ({ uploadName: `${agentName}-reply-${event._id}.md`, log: (line) => log(`[${event.type}] ${line}`), }); - delivered = true; - log( - `[${event.type}] posted ${Buffer.byteLength(replyText)} bytes as ` - + `${delivery.messages} message${delivery.messages === 1 ? '' : 's'} (${delivery.mode})`, - ); + if (delivery.refused) { + // A run-cap refusal is a successful HTTP request but not a delivery. + // Ack it so the kernel does not replay the same text (the server + // guidance expressly says not to retry unchanged), while preserving + // the refusal and its partial-post count for the seat and event ledger. + deliveryRefusal = delivery; + const guidance = delivery.guidance || 'The server refused this message; do not retry it unchanged.'; + const detail = `after ${delivery.messages}/${delivery.attemptedMessages} message${delivery.attemptedMessages === 1 ? '' : 's'}`; + log(`[${event.type}] wrapper delivery refused ${detail} (${delivery.reason}): ${guidance}`); + onError?.(Object.assign( + new Error(`${event.type} wrapper delivery refused ${detail}: ${guidance}`), + { + code: 'agent_delivery_refused', + reason: delivery.reason, + eventId: event._id, + postedMessages: delivery.messages, + attemptedMessages: delivery.attemptedMessages, + }, + )); + } else { + delivered = true; + log( + `[${event.type}] posted ${Buffer.byteLength(replyText)} bytes as ` + + `${delivery.messages} message${delivery.messages === 1 ? '' : 's'} (${delivery.mode})`, + ); + } } if (result.memorySummary) { try { @@ -1176,6 +1198,21 @@ export const performRun = ({ // streak. Recording only on completion means a spawn failure that gets // redelivered never double-counts toward the cap. cascadeGovernor.record(eventPodId, trigger); + if (deliveryRefusal) { + return { + outcome: 'no_action', + reason: deliveryRefusal.reason, + details: { + mode: deliveryRefusal.mode, + postedMessages: deliveryRefusal.messages, + attemptedMessages: deliveryRefusal.attemptedMessages, + ...(typeof deliveryRefusal.consecutive === 'number' + ? { consecutive: deliveryRefusal.consecutive } + : {}), + ...(deliveryRefusal.guidance ? { guidance: deliveryRefusal.guidance } : {}), + }, + }; + } return { outcome: delivered ? 'posted' : 'no_action' }; }; diff --git a/cli/src/lib/enforcement.js b/cli/src/lib/enforcement.js index f86e5365f5..1fcf42d790 100644 --- a/cli/src/lib/enforcement.js +++ b/cli/src/lib/enforcement.js @@ -582,6 +582,24 @@ export const deliverChatReply = async ({ }) => { const messagesPath = `/api/agents/runtime/pods/${podId}/messages`; const chunks = splitForChat(text, { limit }); + // The runtime route uses HTTP 200 for a policy refusal: it is a completed + // request, but no message was created. Keep that distinction at the client + // boundary so every delivery mode shares it rather than treating a resolved + // promise as proof of a post. + const postMessage = (body) => client.post(messagesPath, body); + const refused = (response, messages, attemptedMessages) => ({ + mode: 'refused', + messages, + attemptedMessages, + refused: true, + reason: response.reason || 'message_refused', + ...(typeof response.guidance === 'string' && response.guidance + ? { guidance: response.guidance } + : {}), + ...(typeof response.consecutive === 'number' + ? { consecutive: response.consecutive } + : {}), + }); // An atomic unit (a fenced block, an unbreakable word-run) can exceed the // limit by construction — splitForChat keeps it whole rather than breaking // its rendering. The tone contract's own rule covers it: over ~800 chars of @@ -590,15 +608,19 @@ export const deliverChatReply = async ({ // the gate (found by the fleet's implementation audit, Sharpen msg 53018). const hasIndivisibleOversize = chunks.some((c) => c.length > attachThreshold); if (chunks.length <= 1 && !hasIndivisibleOversize) { - await client.post(messagesPath, { content: chunks[0] ?? text }); + const response = await postMessage({ content: chunks[0] ?? text }); + if (response?.refused === true) return refused(response, 0, 1); return { mode: 'single', messages: 1 }; } if (chunks.length <= maxChunks && !hasIndivisibleOversize) { + let messages = 0; for (const chunk of chunks) { // eslint-disable-next-line no-await-in-loop - await client.post(messagesPath, { content: chunk }); // in order, so the reply reads top-down + const response = await postMessage({ content: chunk }); // in order, so the reply reads top-down + if (response?.refused === true) return refused(response, messages, chunks.length); + messages += 1; } - return { mode: 'split', messages: chunks.length }; + return { mode: 'split', messages }; } // PROSE OVERFLOW → THREAD. Only when nothing is indivisibly oversize: a // fence too big to split is a document and belongs in the attach rung below. @@ -614,7 +636,8 @@ export const deliverChatReply = async ({ // fail at the root-id step before any continuation has posted. let posted = 0; try { - const rootRes = await client.post(messagesPath, { content: chunks[0] }); + const rootRes = await postMessage({ content: chunks[0] }); + if (rootRes?.refused === true) return refused(rootRes, 0, chunks.length); posted = 1; // The runtime route answers `res.json(result)` with the created row on // `result.message`. Accept either id field; refuse to guess if neither @@ -625,7 +648,8 @@ export const deliverChatReply = async ({ if (!rootId) throw new Error('no message id in post response — cannot root the thread'); for (const chunk of chunks.slice(1)) { // eslint-disable-next-line no-await-in-loop - await client.post(messagesPath, { content: chunk, threadRootId: String(rootId) }); + const response = await postMessage({ content: chunk, threadRootId: String(rootId) }); + if (response?.refused === true) return refused(response, posted, chunks.length); posted += 1; } return { mode: 'thread', messages: chunks.length, threadRootId: String(rootId) }; @@ -639,7 +663,9 @@ export const deliverChatReply = async ({ log(`thread continuation failed (${err.message}) — posting the remainder top-level`); for (const chunk of chunks.slice(posted)) { // eslint-disable-next-line no-await-in-loop - await client.post(messagesPath, { content: chunk }); + const response = await postMessage({ content: chunk }); + if (response?.refused === true) return refused(response, posted, chunks.length); + posted += 1; } return { mode: 'thread-fallback', messages: chunks.length }; } @@ -663,14 +689,18 @@ export const deliverChatReply = async ({ const lead = chunks[0] && chunks[0].length <= limit ? chunks[0] : '(reply too large for chat — attached in full)'; - await client.post(messagesPath, { content: `${lead}\n\n${directive}` }); + const response = await postMessage({ content: `${lead}\n\n${directive}` }); + if (response?.refused === true) return refused(response, 0, 1); return { mode: 'attach', messages: 1 }; } catch (err) { log(`attach fallback failed (${err.message}) — posting ${chunks.length} split messages instead`); + let messages = 0; for (const chunk of chunks) { // eslint-disable-next-line no-await-in-loop - await client.post(messagesPath, { content: chunk }); + const response = await postMessage({ content: chunk }); + if (response?.refused === true) return refused(response, messages, chunks.length); + messages += 1; } - return { mode: 'split-fallback', messages: chunks.length }; + return { mode: 'split-fallback', messages }; } };