mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 10:39:32 +01:00
fix(sessions): leave a pending idle alert to the user through a keep-alive turn
Only a human spends an idle alert, but the keep-alive turn resolved an unseen one three ways: its working transition, its stop hook, and its own idle prompt replacing the item with a pre-acknowledged one. While the turn runs, a pending idle item is now left exactly as found, the page's stop handler returns on the keep-alive reason before clearing hooks or raising Response Complete, and session:working/session:idle carry keepAlive so the stuck-detection timer neither restarts nor re-arms. The window that marks the turn closes on its own idle prompt or any other submit instead of a fixed three minutes, which a slow turn's idle prompt (sent a minute after the turn ends) could outlast. The service keeps the default and cap in memory, read through a strict reader so a mid-write read is retried rather than kept, and the cap is clamped before the strict settings save.
This commit is contained in:
1 parent
71a51df2e2
commit
5db6c633d3
22 files changed
+441
-41
No files matched your search
@@ -0,0 +1,126 @@
|
||||
// Port: none (handlers sliced out of the shipped frontend files and run against stubs).
|
||||
//
|
||||
// The prompt-cache keep-alive turn (prompt-cache-keepwarm.ts) is Codeman's, not the user's, so
|
||||
// on the page it must spend nothing: an idle alert still pending stays (only a human spends
|
||||
// one), no "Response Complete" is raised, and the stuck-detection timer neither restarts nor
|
||||
// re-arms ("Session Idle 10+ minutes" would otherwise follow every nudge). The server marks the
|
||||
// turn's `hook:stop` with `acknowledgedReason` and its `session:working`/`session:idle` with
|
||||
// `keepAlive`; these tests run the real handlers on those payloads.
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
const PUBLIC = resolve(import.meta.dirname, '../src/web/public');
|
||||
const SESSION = 'keep-alive-session';
|
||||
const THRESHOLD = 600_000;
|
||||
|
||||
/** The body of a method as shipped, compiled with `data` as its one parameter. */
|
||||
function method(file: string, signature: string): (this: unknown, data: Record<string, unknown>) => void {
|
||||
const source = readFileSync(resolve(PUBLIC, file), 'utf8');
|
||||
const start = source.indexOf(` ${signature} {`);
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
const bodyStart = source.indexOf('{', start) + 1;
|
||||
const end = source.indexOf('\n }', bodyStart);
|
||||
return new Function('data', source.slice(bodyStart, end)) as (data: Record<string, unknown>) => void;
|
||||
}
|
||||
|
||||
function makeApp() {
|
||||
const notifications: string[] = [];
|
||||
const app = {
|
||||
sessions: new Map([[SESSION, { status: 'idle' }]]),
|
||||
activeSessionId: 'another-session',
|
||||
respawnStatus: {},
|
||||
notificationManager: { preferences: { stuckThresholdMs: THRESHOLD } },
|
||||
idleTimers: new Map<string, unknown>(),
|
||||
pendingHooks: new Map<string, Set<string>>(),
|
||||
tabAlerts: new Map<string, string>(),
|
||||
notifications,
|
||||
renderSessionTabs() {},
|
||||
sendPendingCtrlL() {},
|
||||
_updateLocalEchoState() {},
|
||||
_notifySession: (_id: string, _level: string, kind: string) => notifications.push(kind),
|
||||
clearPendingHooks(sessionId: string) {
|
||||
app.pendingHooks.delete(sessionId);
|
||||
app.tabAlerts.delete(sessionId);
|
||||
},
|
||||
onIdle: method('app.js', '_onSessionIdle(data)'),
|
||||
onWorking: method('app.js', '_onSessionWorking(data)'),
|
||||
onStop: method('settings-ui.js', '_onHookStop(data)'),
|
||||
};
|
||||
return app;
|
||||
}
|
||||
|
||||
describe('the keep-alive turn spends nothing on the page', () => {
|
||||
beforeEach(() => vi.useFakeTimers());
|
||||
afterEach(() => vi.useRealTimers());
|
||||
|
||||
it('keeps the stuck timer of the last real turn: not cleared, not re-armed', () => {
|
||||
const app = makeApp();
|
||||
app.onIdle({ id: SESSION });
|
||||
const realTimer = app.idleTimers.get(SESSION);
|
||||
expect(realTimer).toBeDefined();
|
||||
vi.advanceTimersByTime(THRESHOLD / 2);
|
||||
|
||||
app.onWorking({ id: SESSION, keepAlive: true });
|
||||
expect(app.idleTimers.get(SESSION)).toBe(realTimer);
|
||||
app.onIdle({ id: SESSION, keepAlive: true });
|
||||
expect(app.idleTimers.get(SESSION)).toBe(realTimer);
|
||||
|
||||
// Fires once, on the real turn's schedule, and nothing follows the keep-alive.
|
||||
vi.advanceTimersByTime(THRESHOLD / 2);
|
||||
expect(app.notifications).toEqual(['session-stuck']);
|
||||
vi.advanceTimersByTime(THRESHOLD * 2);
|
||||
expect(app.notifications).toEqual(['session-stuck']);
|
||||
});
|
||||
|
||||
it('a real turn still restarts the stuck timer', () => {
|
||||
const app = makeApp();
|
||||
app.onIdle({ id: SESSION });
|
||||
app.onWorking({ id: SESSION });
|
||||
expect(app.idleTimers.has(SESSION)).toBe(false);
|
||||
app.onIdle({ id: SESSION });
|
||||
expect(app.idleTimers.has(SESSION)).toBe(true);
|
||||
});
|
||||
|
||||
it("its stop leaves a pending idle alert and raises no 'Response Complete'", () => {
|
||||
const app = makeApp();
|
||||
app.pendingHooks.set(SESSION, new Set(['idle_prompt']));
|
||||
app.tabAlerts.set(SESSION, 'idle');
|
||||
app.onStop({ sessionId: SESSION, acknowledgedReason: 'cache keep-alive' });
|
||||
expect([...(app.pendingHooks.get(SESSION) ?? [])]).toEqual(['idle_prompt']);
|
||||
expect(app.tabAlerts.get(SESSION)).toBe('idle');
|
||||
// _onSessionWorking deletes the tab alert only with no pending hook, so it stays too.
|
||||
app.onWorking({ id: SESSION, keepAlive: true });
|
||||
expect(app.tabAlerts.get(SESSION)).toBe('idle');
|
||||
expect(app.notifications).toEqual([]);
|
||||
});
|
||||
|
||||
it('a real stop still clears and notifies', () => {
|
||||
const app = makeApp();
|
||||
app.pendingHooks.set(SESSION, new Set(['idle_prompt']));
|
||||
app.onStop({ sessionId: SESSION });
|
||||
expect(app.pendingHooks.has(SESSION)).toBe(false);
|
||||
expect(app.notifications).toEqual(['hook-stop']);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* `promptCacheKeepWarmMaxHours` is clamped to the settings schema's range (0.5 to 24) before the
|
||||
* save: the schema is strict, so one value outside it would reject every other change in the body.
|
||||
*/
|
||||
describe('the keep-warm cap is clamped before the save', () => {
|
||||
it.each([
|
||||
['0.25', 0.5],
|
||||
['30', 24],
|
||||
['', 2],
|
||||
['abc', 2],
|
||||
['3.5', 3.5],
|
||||
])('%j saves as %d', (input, expected) => {
|
||||
const source = readFileSync(resolve(PUBLIC, 'settings-ui.js'), 'utf8');
|
||||
const match = source.match(/promptCacheKeepWarmMaxHours:\s*\n?((?:\s*\/\/.*\n)*)\s*([^\n]+),\n/);
|
||||
expect(match).not.toBeNull();
|
||||
const document = { getElementById: () => ({ value: input }) };
|
||||
const value = new Function('document', `return ${match![2]};`)(document);
|
||||
expect(value).toBe(expected);
|
||||
});
|
||||
});
|
||||
@@ -15,7 +15,7 @@ import { tmpdir } from 'node:os';
|
||||
import { dirname, join, resolve } from 'node:path';
|
||||
import { describe, it, expect, afterEach } from 'vitest';
|
||||
import { decideIdleCompactionOptOut } from '../src/tmux-manager.js';
|
||||
import { readKeepWarmSettings } from '../src/hooks-config.js';
|
||||
import { readKeepWarmSettings, readKeepWarmSettingsOrThrow } from '../src/hooks-config.js';
|
||||
import { SETTINGS_PATH } from '../src/web/route-helpers.js';
|
||||
import { DEFAULT_KEEP_WARM_MAX_HOURS } from '../src/prompt-cache-keepwarm.js';
|
||||
|
||||
@@ -155,4 +155,20 @@ describe('readKeepWarmSettings', () => {
|
||||
await writeFile(SETTINGS_PATH, JSON.stringify({ promptCacheKeepWarm: 'yes', promptCacheKeepWarmMaxHours: -1 }));
|
||||
expect(await readKeepWarmSettings()).toEqual({ keepWarmDefault: false, maxHours: 2 });
|
||||
});
|
||||
|
||||
/**
|
||||
* The service keeps what it reads until the next keep-warm settings change, so its reader must
|
||||
* never hand back a fallback for a file it could not read: settings.json is rewritten in place
|
||||
* by several routes, and defaults kept from a mid-write read would hold keep-warm off.
|
||||
*/
|
||||
it('the service reader rejects an unreadable file instead of falling back, and the server wires it', async () => {
|
||||
expect(await readKeepWarmSettingsOrThrow()).toEqual({ keepWarmDefault: false, maxHours: 2 });
|
||||
await mkdir(dirname(SETTINGS_PATH), { recursive: true });
|
||||
await writeFile(SETTINGS_PATH, JSON.stringify({ promptCacheKeepWarm: true, promptCacheKeepWarmMaxHours: 3.5 }));
|
||||
expect(await readKeepWarmSettingsOrThrow()).toEqual({ keepWarmDefault: true, maxHours: 3.5 });
|
||||
await writeFile(SETTINGS_PATH, '{"promptCacheKeepWarm": tr');
|
||||
await expect(readKeepWarmSettingsOrThrow()).rejects.toThrow();
|
||||
expect(await readKeepWarmSettings()).toEqual({ keepWarmDefault: false, maxHours: 2 });
|
||||
expect(src('web/server.ts')).toContain('readSettings: readKeepWarmSettingsOrThrow,');
|
||||
});
|
||||
});
|
||||
@@ -56,6 +56,9 @@ export class MockSession extends EventEmitter {
|
||||
}
|
||||
/** Mirrors the prompt-cache keep-warm surface on Session (prompt-cache-keepwarm.ts). */
|
||||
keepAliveTurnActive = false;
|
||||
endKeepAliveTurn(): void {
|
||||
this.keepAliveTurnActive = false;
|
||||
}
|
||||
keepWarmEnabled: boolean | undefined = undefined;
|
||||
setKeepWarmEnabled(enabled: boolean | undefined): void {
|
||||
this.keepWarmEnabled = enabled;
|
||||
|
||||
@@ -228,6 +228,28 @@ describe('the two submit clocks on a Session', () => {
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* The window is a ceiling, not the turn's length: a slow keep-alive turn's idle prompt lands
|
||||
* about a minute after it ends, so the window closes on that prompt (the route calls
|
||||
* endKeepAliveTurn) or on any other submit, never by a short clock alone.
|
||||
*/
|
||||
it('the keep-alive window survives the nudge and a bare Enter, and ends on any other submit', () => {
|
||||
const session = new Session({ workingDir: '/tmp' });
|
||||
session.beginKeepAliveTurn(KEEP_WARM_TURN_WINDOW_MS);
|
||||
session.write(`${KEEP_WARM_NUDGE}\r`);
|
||||
session.write('\r');
|
||||
expect(session.keepAliveTurnActive).toBe(true);
|
||||
session.write('/compact\r');
|
||||
expect(session.keepAliveTurnActive).toBe(false);
|
||||
|
||||
session.beginKeepAliveTurn(KEEP_WARM_TURN_WINDOW_MS);
|
||||
session.write('typed but not sent');
|
||||
expect(session.keepAliveTurnActive).toBe(true);
|
||||
session.endKeepAliveTurn();
|
||||
expect(session.keepAliveTurnActive).toBe(false);
|
||||
expect(KEEP_WARM_TURN_WINDOW_MS).toBeGreaterThanOrEqual(10 * 60_000);
|
||||
});
|
||||
|
||||
it('persists the real-prompt clock and the choice, and restores both', () => {
|
||||
const session = new Session({ workingDir: '/tmp', lastRealPromptAt: NOW, keepWarmEnabled: true });
|
||||
expect(session.toState()).toMatchObject({ lastRealPromptAt: NOW, keepWarmEnabled: true });
|
||||
@@ -529,16 +551,48 @@ describe('PromptCacheKeepWarmService', () => {
|
||||
// Second evaluation: not due, held in its settings read.
|
||||
session.promptCache = warmCache(NOW + HOUR);
|
||||
releaseRead = () => {};
|
||||
const second = service.schedule(session.id);
|
||||
// rescheduleAll() drops the kept settings, so this evaluation really waits in its read.
|
||||
service.rescheduleAll();
|
||||
await settle();
|
||||
releaseWrite(true);
|
||||
await first;
|
||||
releaseRead();
|
||||
await second;
|
||||
await settle();
|
||||
expect(published).toEqual([expect.objectContaining({ state: 'armed' })]);
|
||||
service.stop();
|
||||
});
|
||||
|
||||
/**
|
||||
* Every statusline report schedules an evaluation, so the settings are read once and kept
|
||||
* rather than read from disk on each report, the feature off included; the settings PUT calls
|
||||
* rescheduleAll() when either key changes, which is what makes the next read fresh.
|
||||
*/
|
||||
it('reads the settings once, again after rescheduleAll(), and again after a failed read', async () => {
|
||||
const session = new FakeSession();
|
||||
session.promptCache = warmCache(NOW + HOUR);
|
||||
let reads = 0;
|
||||
let fail = false;
|
||||
const { ports } = makePorts(session, {
|
||||
readSettings: async () => {
|
||||
reads += 1;
|
||||
if (fail) throw new Error('unreadable');
|
||||
return { keepWarmDefault: false, maxHours: 2 };
|
||||
},
|
||||
});
|
||||
const service = new PromptCacheKeepWarmService(ports);
|
||||
await service.schedule(session.id);
|
||||
await service.schedule(session.id);
|
||||
expect(reads).toBe(1);
|
||||
fail = true;
|
||||
service.rescheduleAll();
|
||||
await settle();
|
||||
expect(reads).toBe(2);
|
||||
fail = false;
|
||||
await service.schedule(session.id);
|
||||
expect(reads).toBe(3);
|
||||
service.stop();
|
||||
});
|
||||
|
||||
it('remove() forgets the nudge count, so an id seen again starts from zero', async () => {
|
||||
const session = new FakeSession();
|
||||
session.keepWarmEnabled = true;
|
||||
|
||||
@@ -172,6 +172,15 @@ describe('Keep-warm line on the Status row', () => {
|
||||
`Keep-alive: next ${clock(T)} · idle compaction on until relaunch`
|
||||
);
|
||||
});
|
||||
|
||||
it('says idle compaction stays off when keep-warm was switched off on a launch that carries the opt-out', () => {
|
||||
const { formatKeepWarm } = loadApi();
|
||||
const off = { enabled: false, state: 'off', reason: 'off', nudges: 0 };
|
||||
expect(formatKeepWarm({ ...off, idleCompactionOff: false }, NOW, 'injected')).toBe('');
|
||||
expect(formatKeepWarm({ ...off, idleCompactionOff: true }, NOW, 'injected')).toBe(
|
||||
'Keep-alive: off · idle compaction off until relaunch'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
|
||||
@@ -127,6 +127,75 @@ describe('hook-event-routes', () => {
|
||||
expect(item?.acknowledgedAt).toBe(item?.createdAt);
|
||||
});
|
||||
|
||||
/**
|
||||
* Only a human spends an idle alert (CLAUDE.md, Approvals Inbox). A keep-alive turn that
|
||||
* starts while an idle item is pending must leave that item exactly as it found it: its stop
|
||||
* does not resolve it, and its idle prompt does not replace it, so an unseen alert stays
|
||||
* unseen (and pushes nothing a second time) and one a person looked at keeps saying so.
|
||||
*/
|
||||
it.each([
|
||||
{ seen: false, label: 'an unseen' },
|
||||
{ seen: true, label: 'a seen' },
|
||||
])('leaves $label idle item pending through a keep-alive turn', async ({ seen }) => {
|
||||
const sessionId = harness.ctx._sessionId;
|
||||
const hook = (event: string) =>
|
||||
harness.app.inject({ method: 'POST', url: '/api/hook-event', payload: { event, sessionId, data: null } });
|
||||
await hook('idle_prompt');
|
||||
const before = approvalInbox.getForSession(sessionId);
|
||||
expect(before?.acknowledgedAt).toBeUndefined();
|
||||
if (seen) approvalInbox.acknowledge(sessionId);
|
||||
const acknowledgedAt = before?.acknowledgedAt;
|
||||
harness.ctx.sendPushNotifications.mockClear();
|
||||
harness.ctx.broadcast.mockClear();
|
||||
|
||||
harness.ctx._session.keepAliveTurnActive = true;
|
||||
await hook('stop');
|
||||
await hook('idle_prompt');
|
||||
|
||||
const after = approvalInbox.getForSession(sessionId);
|
||||
approvalInbox.resolveForSession(sessionId, 'dismissed');
|
||||
expect(after?.id).toBe(before?.id);
|
||||
expect(after?.acknowledgedAt).toBe(acknowledgedAt);
|
||||
expect(after?.acknowledgedReason).toBeUndefined();
|
||||
expect(harness.ctx.sendPushNotifications).not.toHaveBeenCalled();
|
||||
// The page's _onHookStop and _onHookIdlePrompt return on the reason, leaving the tab alert.
|
||||
for (const event of ['stop', 'idle_prompt']) {
|
||||
expect(harness.ctx.broadcast).toHaveBeenCalledWith(
|
||||
`hook:${event}`,
|
||||
expect.objectContaining({ acknowledgedReason: 'cache keep-alive' })
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("a keep-alive turn's idle prompt ends the keep-alive window, its stop does not", async () => {
|
||||
const sessionId = harness.ctx._sessionId;
|
||||
const hook = (event: string) =>
|
||||
harness.app.inject({ method: 'POST', url: '/api/hook-event', payload: { event, sessionId, data: null } });
|
||||
harness.ctx._session.keepAliveTurnActive = true;
|
||||
await hook('stop');
|
||||
expect(harness.ctx._session.keepAliveTurnActive).toBe(true);
|
||||
await hook('idle_prompt');
|
||||
approvalInbox.resolveForSession(sessionId, 'dismissed');
|
||||
expect(harness.ctx._session.keepAliveTurnActive).toBe(false);
|
||||
});
|
||||
|
||||
it("a keep-alive turn's stop still resolves a pending permission dialog", async () => {
|
||||
const sessionId = harness.ctx._sessionId;
|
||||
await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/hook-event',
|
||||
payload: { event: 'permission_prompt', sessionId, data: null },
|
||||
});
|
||||
expect(approvalInbox.getForSession(sessionId)?.kind).toBe('permission');
|
||||
harness.ctx._session.keepAliveTurnActive = true;
|
||||
await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/hook-event',
|
||||
payload: { event: 'stop', sessionId, data: null },
|
||||
});
|
||||
expect(approvalInbox.getForSession(sessionId)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('sends push notifications for hook events', async () => {
|
||||
const res = await harness.app.inject({
|
||||
method: 'POST',
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
detachSessionListeners,
|
||||
} from '../src/web/session-listener-wiring.js';
|
||||
import { SseEvent } from '../src/web/sse-events.js';
|
||||
import { approvalInbox } from '../src/web/approval-inbox.js';
|
||||
|
||||
describe('session listener wiring', () => {
|
||||
it('forwards the attachment request source through registerAttachment', async () => {
|
||||
@@ -62,6 +63,37 @@ describe('session listener wiring', () => {
|
||||
expect(session.listenerCount('displayModelChanged')).toBe(0);
|
||||
});
|
||||
|
||||
/**
|
||||
* A working transition resolves a pending idle item (input arrived), except during Codeman's
|
||||
* own keep-alive turn: only a human spends an idle alert. Both transitions of that turn are
|
||||
* flagged `keepAlive`, which is what keeps the page's stuck-detection timer out of it.
|
||||
*/
|
||||
it.each([
|
||||
{ keepAlive: false, resolved: true },
|
||||
{ keepAlive: true, resolved: false },
|
||||
])('working and idle with keepAlive=$keepAlive', ({ keepAlive, resolved }) => {
|
||||
const session = new Session({ id: `wiring-keepalive-${keepAlive}`, workingDir: '/tmp', mode: 'claude' });
|
||||
const broadcast = vi.fn();
|
||||
const deps = {
|
||||
broadcast,
|
||||
broadcastSessionStateDebounced: vi.fn(),
|
||||
getRunSummaryTracker: vi.fn(() => undefined),
|
||||
} as unknown as Parameters<typeof createSessionListeners>[1];
|
||||
const refs = createSessionListeners(session, deps);
|
||||
approvalInbox.notePrompt({ sessionId: session.id, sessionName: 'w1', kind: 'idle' });
|
||||
if (keepAlive) session.beginKeepAliveTurn(60_000);
|
||||
|
||||
refs.working();
|
||||
refs.idle();
|
||||
|
||||
const pending = approvalInbox.getForSession(session.id);
|
||||
approvalInbox.resolveForSession(session.id, 'dismissed');
|
||||
expect(pending === undefined).toBe(resolved);
|
||||
const flag = keepAlive ? { keepAlive: true } : {};
|
||||
expect(broadcast).toHaveBeenCalledWith(SseEvent.SessionWorking, { id: session.id, ...flag });
|
||||
expect(broadcast).toHaveBeenCalledWith(SseEvent.SessionIdle, { id: session.id, ...flag });
|
||||
});
|
||||
|
||||
/** The listener reads the setting asynchronously; let its promise chain settle. */
|
||||
const flush = () => new Promise((resolve) => setTimeout(resolve, 5));
|
||||
|
||||
|
||||
Reference in new issue
Block a user