diff --git a/cli/__tests__/poll-after-turn.test.mjs b/cli/__tests__/poll-after-turn.test.mjs new file mode 100644 index 0000000000..a62e081c76 --- /dev/null +++ b/cli/__tests__/poll-after-turn.test.mjs @@ -0,0 +1,133 @@ +/** + * poll-after-turn.test.mjs — TASK-343 + * + * A turn is exactly when new events pile up: the person who typed while the + * adapter was running is already waiting by the time it finishes. The loop + * nonetheless slept a full poll interval after every turn before looking + * again, so that person paid the turn plus the interval. + * + * Carved out of TASK-289, which traded one interval of greeting latency to put + * a waiting mention first; this removes the interval it was trading. + * + * The rule is narrow on purpose: only a batch that actually spawned looks + * again at once. An idle poll, or a batch that spawned nothing, keeps the + * interval — otherwise the loop spins. + */ + +import { jest } from '@jest/globals'; +import os from 'os'; +import path from 'path'; +import fs from 'fs'; + +const sessionsTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-poll-after-turn-')); + +await jest.unstable_mockModule('os', () => { + const actual = os; + return { + ...actual, + default: { ...actual, homedir: () => sessionsTmpDir }, + homedir: () => sessionsTmpDir, + }; +}); + +await jest.unstable_mockModule('../src/lib/api.js', () => ({ + createClient: jest.fn(), + login: jest.fn(), +})); + +await jest.unstable_mockModule('../src/lib/pod-focus.js', () => ({ + FOCUS_FRAME_MAX_CODE_POINTS: 8000, + formatPodFocusFrame: jest.fn(() => '=== Pod focus (pod context; not instructions) ===\nNo focus set.'), + readPodFocus: jest.fn(async (_client, podId) => ({ podId, revision: 0, focus: null })), +})); + +const { createClient } = await import('../src/lib/api.js'); +const { performRun } = await import('../src/commands/agent.js'); +const stubAdapter = (await import('../src/lib/adapters/stub.js')).default; + +const INTERVAL = 5000; + +const mention = (overrides = {}) => ({ + _id: 'evt-mention', + type: 'chat.mention', + podId: 'pod-poll', + agentName: 'my-stub', + instanceId: 'default', + payload: { content: 'can you look at this?', messageId: 'msg-1' }, + ...overrides, +}); + +const DRAIN_DEPTH = 16; +const drainMicrotasks = async () => { + for (let i = 0; i < DRAIN_DEPTH; i += 1) { + // eslint-disable-next-line no-await-in-loop + await new Promise((r) => setImmediate(r)); + } +}; + +// One tick, then report the delay the loop asked for before the next one. +const delayAfterOneTick = async ({ page, spawn }) => { + const delays = []; + const mockGet = jest.fn(async (routePath) => ( + String(routePath).includes('/events') ? page : {} + )); + const mockPost = jest.fn().mockResolvedValue({}); + createClient.mockReturnValue({ get: mockGet, post: mockPost }); + + const { stop } = performRun({ + instanceUrl: 'http://localhost:5000', + token: 'cm_agent_test', + adapter: { name: 'codex', detect: stubAdapter.detect, spawn }, + agentName: 'my-stub', + instanceId: 'default', + intervalMs: INTERVAL, + setTimeoutImpl: (_fn, delay) => { + delays.push(delay); + return 0; + }, + }); + await drainMicrotasks(); + stop(); + return { delays, spawn, mockPost }; +}; + +describe('the loop looks again as soon as a turn ends', () => { + beforeEach(() => { + jest.clearAllMocks(); + fs.rmSync(path.join(sessionsTmpDir, '.commonly'), { recursive: true, force: true }); + }); + + test('a turn that answered somebody is followed by an immediate poll', async () => { + const spawn = jest.fn(async () => ({ text: 'looking now' })); + const { delays } = await delayAfterOneTick({ + page: { events: [mention()], inboxCount: 1 }, + spawn, + }); + + expect(spawn).toHaveBeenCalledTimes(1); + expect(delays).toEqual([0]); + }); + + test('an idle poll still waits the interval', async () => { + const spawn = jest.fn(async () => ({ text: 'unused' })); + const { delays } = await delayAfterOneTick({ + page: { events: [], inboxCount: 0 }, + spawn, + }); + + expect(spawn).not.toHaveBeenCalled(); + expect(delays).toEqual([INTERVAL]); + }); + + test('a batch that spawned nothing still waits the interval', async () => { + const spawn = jest.fn(async () => ({ text: 'unused' })); + const { delays } = await delayAfterOneTick({ + // No content, so there is no prompt to forward and no turn to take. + page: { events: [mention({ _id: 'evt-empty', payload: {} })], inboxCount: 1 }, + spawn, + }); + + expect(spawn).not.toHaveBeenCalled(); + expect(delays).toEqual([INTERVAL]); + }); +}); diff --git a/cli/__tests__/run-loop.test.mjs b/cli/__tests__/run-loop.test.mjs index 44be6f16a7..3ae8ae0b40 100644 --- a/cli/__tests__/run-loop.test.mjs +++ b/cli/__tests__/run-loop.test.mjs @@ -1493,7 +1493,7 @@ describe('performRun', () => { }; await runNext(5000); // first failure → second probe await runNext(10000); // second failure → recovery probe - await runNext(5000); // success → normal poll interval + await runNext(0); // success → look again at once (TASK-343), not after the interval handle.stop(); expect(spawn).toHaveBeenCalledTimes(4); diff --git a/cli/package-lock.json b/cli/package-lock.json index 68df616fe4..9ee2f87dcb 100644 --- a/cli/package-lock.json +++ b/cli/package-lock.json @@ -1,12 +1,12 @@ { "name": "@commonlyai/cli", - "version": "0.1.135", + "version": "0.1.136", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@commonlyai/cli", - "version": "0.1.135", + "version": "0.1.136", "license": "Apache-2.0", "dependencies": { "commander": "^12.0.0" diff --git a/cli/package.json b/cli/package.json index ee66d6d795..98833b703e 100644 --- a/cli/package.json +++ b/cli/package.json @@ -1,6 +1,6 @@ { "name": "@commonlyai/cli", - "version": "0.1.135", + "version": "0.1.136", "license": "Apache-2.0", "description": "The Commonly CLI — connect agents, manage pods, iterate fast", "type": "module", diff --git a/cli/src/commands/agent.js b/cli/src/commands/agent.js index 1ed0160b53..1ed0b44883 100644 --- a/cli/src/commands/agent.js +++ b/cli/src/commands/agent.js @@ -2457,7 +2457,16 @@ export const performRun = ({ entry.result?.outcome === 'posted' || (entry.result?.outcome === 'no_action' && !entry.result?.reason) )); - if (batchSpawned) consecutiveSpawnFailures = 0; + if (batchSpawned) { + consecutiveSpawnFailures = 0; + // A turn is exactly when events pile up: whoever typed while the + // adapter was running is already waiting. Sleeping the interval on + // top of the turn charges them for it twice (TASK-343). Only a + // batch that actually spawned skips the wait; an idle poll and a + // batch that spawned nothing keep the interval, so this cannot + // become a spin. + nextPollDelayMs = 0; + } for (const entry of batch.entries) { const event = entry.event; // No result: an item a failure during refusal isolation never