feat(codex): raise a screen-read dialog as BLOCKED, alert-only

A codex dialog the session now reads off its screen gets exactly what a
Claude `permission_prompt` hook gets, so every surface that already handles
that event needs no second path: `web/screen-blocked.ts` turns the session's
`blockedChanged` edge into the `blocked` wait signal, an Approvals Inbox
item, the `hook:permission_prompt` broadcast (with `source: 'screen'`), web
push and webhooks, and a run-summary row. That lights the red "needs you"
tab alert and desktop notification, the phone overview's NEEDS YOU row and
the TUI. When the dialog leaves the screen the item it opened is resolved,
and `approval:resolved` clears every client.

It is an ALERT and never a keystroke, following #610's shape for DeepSeek:
the item is `answerInTerminal`, push carries no `approvalId` (so no
Approve/Deny), and the answer route refuses it. The dialog was read from a
screen the agent writes most of, so a forged match must cost a false alert
and nothing more. The message every surface shows is built from the CLI's
registry label, never from the dialog's text. The item gets no `capture`
callback, only its frame as context, because the inbox's option parser and
staleness sweep are shaped for Claude's dialogs and the session owns this
item's lifetime. A CLI whose hooks report blocked gets nothing from the
screen, so declaring both could never alert twice.

The answer route now checks `answerInTerminal` BEFORE its hooks gate and
names the CLI: a codex session has no hooks, and "Session mode cannot have
pending approvals" would be false about the item the caller holds.

Wait signals: `blocked` is available where hooks report it OR a CLI declares
`blockedLine` (`blockedSignalAvailable()`); `stop` stays hooks-only, since
nothing on a screen says a turn ended, so `until=stop` on codex is still a
400 (now suggesting `idle, blocked or exit`). A screen-read dialog is a
level the server keeps reading, so a `GET .../wait` that asks for `blocked`
while one is up answers at once; a caller that asked for `idle` alone keeps
getting `idle`. The MCP and `codeman agent` help text say so.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer committed 2026-10-14 05:18:37 +02:00
1 parent beae377122
commit a35ddd82ba
17 files changed
+557 -33

No files matched your search

+54 -1
View File
@@ -13,6 +13,7 @@ import fastifyCookie from '@fastify/cookie';
import { registerApprovalRoutes } from '../../src/web/routes/approval-routes.js';
import { registerHookEventRoutes } from '../../src/web/routes/hook-event-routes.js';
import { approvalInbox } from '../../src/web/approval-inbox.js';
import { raiseScreenBlocked } from '../../src/web/screen-blocked.js';
import { installRouteErrorHandler } from '../../src/web/route-error-handler.js';
import { httpStatusForErrorCode, type ApiErrorCode } from '../../src/types.js';
import { createMockRouteContext, type MockSession } from '../mocks/index.js';
@@ -455,7 +456,6 @@ describe('approval routes', () => {
});
});
describe('approval routes: supervised-hook (DeepSeek) alert-only items', () => {
let harness: RouteTestHarness;
let session: MockSession;
@@ -505,6 +505,59 @@ describe('approval routes: supervised-hook (DeepSeek) alert-only items', () => {
});
});
describe('approval routes: dialogs read off a hook-less screen (codex) are alert-only', () => {
let harness: RouteTestHarness;
let session: MockSession;
const noopDeps = {
broadcast: () => {},
broadcastSessionStateDebounced: () => {},
sendPushNotifications: () => {},
};
beforeEach(async () => {
harness = await createEnvelopeHarness();
session = harness.ctx.sessions.get(SESSION_ID)!;
session.mode = 'codex';
});
afterEach(async () => {
for (const item of approvalInbox.listPending()) {
approvalInbox.resolveForSession(item.sessionId, 'dismissed');
}
approvalInbox.onPending = approvalInbox.onUpdated = approvalInbox.onResolved = undefined;
await harness.app.close();
});
it('lists the item, and the page-load sweep keeps it', async () => {
raiseScreenBlocked(session as never, noopDeps, '› 1. Yes, proceed (y)\n Press enter to confirm or esc to cancel');
const approvals = await listApprovals(harness);
expect(approvals).toHaveLength(1);
expect(approvals[0]).toMatchObject({ sessionId: SESSION_ID, kind: 'permission', answerInTerminal: true });
});
it('refuses an answer with the terminal-only error, not "cannot have pending approvals"', async () => {
// The hooks gate would say a codex session cannot have an item at all, which is false
// about the one the caller holds; the alert-only check runs first.
raiseScreenBlocked(session as never, noopDeps, null);
const [item] = await listApprovals(harness);
for (const payload of [{ action: 'approve' }, { action: 'deny' }, { action: 'option', option: 1 }]) {
const res = await harness.app.inject({ method: 'POST', url: `/api/approvals/${item.id}/answer`, payload });
expect(res.statusCode).toBe(400);
expect(res.json().error).toMatch(/^Codex approvals must be answered in the terminal/);
}
expect(session.writeBuffer).toEqual([]);
expect(await listApprovals(harness)).toHaveLength(1);
});
it('can still be dismissed', async () => {
raiseScreenBlocked(session as never, noopDeps, null);
const [item] = await listApprovals(harness);
const res = await harness.app.inject({ method: 'POST', url: `/api/approvals/${item.id}/dismiss`, payload: {} });
expect(res.statusCode).toBe(200);
expect(await listApprovals(harness)).toHaveLength(0);
});
});
describe('approval routes: multi-user scoping', () => {
const saved: Record<string, string | undefined> = {};
+75
View File
@@ -34,6 +34,7 @@ import { installRouteErrorHandler } from '../../src/web/route-error-handler.js';
import { ApiErrorCode, httpStatusForErrorCode } from '../../src/types.js';
import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js';
import { sessionWaits } from '../../src/web/session-wait-registry.js';
import { approvalInbox } from '../../src/web/approval-inbox.js';
import { MAX_WAIT_MS, MIN_WAIT_MS, MAX_WAITERS_TOTAL, MAX_WAITERS_PER_OWNER } from '../../src/config/agent-wait.js';
// Distinct per file on purpose: the three wait suites share the process-wide
@@ -463,6 +464,80 @@ describe('GET /api/sessions/:id/wait: modes that install no hooks', () => {
});
});
describe('GET /api/sessions/:id/wait: blocked read off a codex screen', () => {
/** The listener deps the screen-blocked raise reaches; nothing else matters here. */
function stubDeps() {
return {
broadcast: vi.fn(),
broadcastSessionStateDebounced: vi.fn(),
sendPushNotifications: vi.fn(),
getRunSummaryTracker: vi.fn(() => undefined),
};
}
afterEach(() => {
approvalInbox.resolveForSession(SESSION_ID, 'session_ended');
});
it('accepts until=blocked for codex, whose dialogs are read off its screen', async () => {
const { app, ctx } = await harness();
ctx.sessions.get(SESSION_ID)!.mode = 'codex';
const res = await app.inject({ method: 'GET', url: `/api/sessions/${SESSION_ID}/wait?until=blocked&timeout=1000` });
expect(res.statusCode).toBe(200);
expect(res.json().data.wait.until).toEqual(['blocked']);
});
it('answers blocked at once for a dialog already on screen, and only to a caller who asked', async () => {
// A hook-raised blocked is an edge; this one is a level the server is still reading,
// so a caller who asks after the dialog appeared does not wait out its timeout.
const { app, ctx } = await harness();
const session = ctx.sessions.get(SESSION_ID)! as unknown as {
mode: string;
status: string;
screenBlocked?: boolean;
};
session.mode = 'codex';
session.status = 'idle';
session.screenBlocked = true;
const asked = await app.inject({ method: 'GET', url: `/api/sessions/${SESSION_ID}/wait?until=idle,blocked` });
expect(asked.json().data.wait).toMatchObject({ signal: 'blocked', immediate: true });
// Such a pane is idle too, and a caller that asked for idle alone keeps getting idle.
const idle = await app.inject({ method: 'GET', url: `/api/sessions/${SESSION_ID}/wait?until=idle` });
expect(idle.json().data.wait).toMatchObject({ signal: 'idle', immediate: true });
});
it('wakes a parked until=blocked waiter when the session raises a screen-read dialog', async () => {
const { app, ctx } = await harness();
const session = ctx.sessions.get(SESSION_ID)!;
session.mode = 'codex';
attachSessionListeners(session as never, createSessionListeners(session as never, stubDeps() as never));
const pending = app.inject({ method: 'GET', url: `/api/sessions/${SESSION_ID}/wait?until=blocked&timeout=600000` });
await new Promise((resolve) => setTimeout(resolve, 20));
expect(sessionWaits.signalWaiterCount(SESSION_ID)).toBe(1);
session.emit('blockedChanged', true, '› 1. Yes, proceed (y)\n Press enter to confirm or esc to cancel');
const body = (await pending).json();
expect(body.data.wait).toMatchObject({ signal: 'blocked', immediate: false, timedOut: false });
expect(approvalInbox.getForSession(SESSION_ID)?.answerInTerminal).toBe(true);
// ...and the dialog leaving the screen resolves the item it opened.
session.emit('blockedChanged', false, null);
expect(approvalInbox.getForSession(SESSION_ID)).toBeUndefined();
});
it('still refuses until=stop for codex: nothing on a screen says a turn ended', async () => {
const { app, ctx } = await harness();
ctx.sessions.get(SESSION_ID)!.mode = 'codex';
const res = await app.inject({ method: 'GET', url: `/api/sessions/${SESSION_ID}/wait?until=stop,blocked` });
expect(res.statusCode).toBe(400);
expect(res.json().error).toContain('Signal(s) stop never fire for codex');
});
});
describe('GET /api/sessions/:id/wait: liveness beats the reported status', () => {
it('answers exit for a session whose PTY is gone, not the idle its status claims', async () => {
// Session parks a dead PTY at status 'idle' and the object survives in the map,
+110
View File
@@ -0,0 +1,110 @@
/**
* What the server raises for a dialog read off a hook-less CLI's screen (codex), and what
* clears it: `web/screen-blocked.ts`. The session side, and the codex captures it reads,
* are in `codex-blocked-detection.test.ts`.
*
* The contract: exactly what a Claude `permission_prompt` hook raises (the `blocked` wait
* signal, an Approvals Inbox item, the `hook:permission_prompt` broadcast, push and
* webhook, a run-summary row), but ALERT-ONLY, since the dialog was read from text the
* agent writes: the item is `answerInTerminal`, push carries no `approvalId`, and the
* inbox never resolves or answers it on its own.
*/
import { describe, expect, it, vi, afterEach } from 'vitest';
import { readFileSync } from 'node:fs';
import { raiseScreenBlocked, clearScreenBlocked, screenBlockedMessage } from '../src/web/screen-blocked.js';
import { approvalInbox } from '../src/web/approval-inbox.js';
import { sessionWaits } from '../src/web/session-wait-registry.js';
import { SseEvent } from '../src/web/sse-events.js';
import type { SessionMode } from '../src/types.js';
const FIXTURES = new URL('./fixtures/codex-dialogs/', import.meta.url);
const fixture = (name: string): string => readFileSync(new URL(name, FIXTURES), 'utf8');
describe('raiseScreenBlocked / clearScreenBlocked', () => {
const SESSION = { id: 'screen-blocked-test', name: 'w9-scratch', mode: 'codex' as SessionMode };
function deps() {
const recordHookEvent = vi.fn();
return {
broadcast: vi.fn(),
broadcastSessionStateDebounced: vi.fn(),
sendPushNotifications: vi.fn(),
getRunSummaryTracker: vi.fn(() => ({ recordHookEvent })),
recordHookEvent,
};
}
afterEach(() => {
approvalInbox.resolveForSession(SESSION.id, 'session_ended');
sessionWaits.cancelAll(SESSION.id);
});
it('raises what a permission_prompt hook raises, alert-only', async () => {
const d = deps();
const waiting = sessionWaits.waitForSignal(SESSION.id, { until: ['blocked'], timeoutMs: 5000 });
const itemId = raiseScreenBlocked(SESSION, d, fixture('exec-approval.txt'));
expect((await waiting).signal).toBe('blocked');
const item = approvalInbox.getForSession(SESSION.id)!;
expect(item.id).toBe(itemId);
expect(item.kind).toBe('permission');
expect(item.answerInTerminal).toBe(true);
expect(item.message).toBe('Codex is waiting for your answer in the terminal');
// The frame is the card's context, and nothing parsed it into answer buttons.
expect(item.context).toContain('Press enter to confirm or esc to cancel');
expect(item.options).toBeUndefined();
expect(d.broadcast).toHaveBeenCalledWith(
SseEvent.HookPermissionPrompt,
expect.objectContaining({ sessionId: SESSION.id, source: 'screen', approvalId: itemId })
);
expect(d.broadcastSessionStateDebounced).toHaveBeenCalledWith(SESSION.id);
// Push and webhook get the message and NO approvalId, so no Approve/Deny buttons.
const [event, push] = d.sendPushNotifications.mock.calls[0] as [string, Record<string, unknown>];
expect(event).toBe(SseEvent.HookPermissionPrompt);
expect(push).toMatchObject({ sessionId: SESSION.id, sessionName: 'w9-scratch', source: 'screen' });
expect(push.message).toBe(screenBlockedMessage('codex'));
expect(push.approvalId).toBeUndefined();
expect(d.recordHookEvent).toHaveBeenCalledWith('permission_prompt', expect.objectContaining({ source: 'screen' }));
});
it('never steers a keystroke: the inbox item cannot be answered', () => {
raiseScreenBlocked(SESSION, deps(), fixture('exec-approval.txt'));
const item = approvalInbox.getForSession(SESSION.id)!;
// No capture callback, so the inbox's Claude-shaped sweep never resolves it either:
// the session owns its lifetime.
expect(approvalInbox.verifyStillAnswerable(item.id)).toBe(true);
approvalInbox.resolveIfDialogGone(SESSION.id);
expect(approvalInbox.getForSession(SESSION.id)?.id).toBe(item.id);
});
it('clears by resolving the item it opened, and only that one', () => {
const resolved: string[] = [];
const prev = approvalInbox.onResolved;
approvalInbox.onResolved = (info) => resolved.push(`${info.id}:${info.resolution}`);
try {
const itemId = raiseScreenBlocked(SESSION, deps(), null);
clearScreenBlocked(SESSION.id, itemId);
expect(approvalInbox.getForSession(SESSION.id)).toBeUndefined();
expect(resolved).toContain(`${itemId}:resolved_in_terminal`);
// An item it did not open (here a newer one) is left alone.
const newer = raiseScreenBlocked(SESSION, deps(), null);
clearScreenBlocked(SESSION.id, itemId);
expect(approvalInbox.getForSession(SESSION.id)?.id).toBe(newer);
clearScreenBlocked(SESSION.id, null);
expect(approvalInbox.getForSession(SESSION.id)?.id).toBe(newer);
} finally {
approvalInbox.onResolved = prev;
}
});
it('raises nothing for a session whose CLI reports blocked through hooks', () => {
const d = deps();
expect(raiseScreenBlocked({ ...SESSION, mode: 'claude' }, d, null)).toBeNull();
expect(d.broadcast).not.toHaveBeenCalled();
expect(d.sendPushNotifications).not.toHaveBeenCalled();
expect(approvalInbox.getForSession(SESSION.id)).toBeUndefined();
});
});
+36 -1
View File
@@ -11,6 +11,7 @@ import {
SessionWaitRegistry,
WaitCapacityError,
hooksAvailableForMode,
blockedSignalAvailable,
parseWaitSignals,
resolveWaitSignals,
signalForStatus,
@@ -100,6 +101,26 @@ describe('signalForStatus', () => {
});
});
describe('blockedSignalAvailable', () => {
it('is true where hooks report it, and for a CLI that declares a blockedLine (codex)', () => {
expect(blockedSignalAvailable('claude')).toBe(true);
expect(blockedSignalAvailable('codex')).toBe(true);
// Not keyed on hooks: codex has none, and its screen-read dialogs are still a source.
expect(hooksAvailableForMode('codex')).toBe(false);
});
it('is false for a CLI with neither (shell, gemini, opencode)', () => {
for (const mode of ['shell', 'gemini', 'opencode'] as const) {
expect(blockedSignalAvailable(mode)).toBe(false);
}
});
it('follows the per-session deepseek bridge, which is its only source', () => {
expect(blockedSignalAvailable('deepseek')).toBe(true);
expect(blockedSignalAvailable('deepseek', { deepSeekStatusReporting: false })).toBe(false);
});
});
describe('hooksAvailableForMode', () => {
it('is true only for claude', () => {
expect(hooksAvailableForMode('claude')).toBe(true);
@@ -144,8 +165,22 @@ describe('resolveWaitSignals', () => {
const result = resolveWaitSignals('stop', codex);
expect(result.until).toEqual([]);
expect(result.error).toContain('codex');
expect(resolveWaitSignals('blocked', codex).error).toBeTruthy();
expect(resolveWaitSignals('idle,stop', codex).error).toBeTruthy();
// gemini declares no blockedLine, so nothing can ever raise `blocked` for it.
expect(resolveWaitSignals('blocked', { mode: 'gemini' as SessionMode }).error).toBeTruthy();
});
it('accepts `blocked` on codex, whose dialogs are read off the screen, but still not `stop`', () => {
// capabilities.workDetect.blockedLine gives codex a source for `blocked`; nothing on a
// screen says a turn ended, so `stop` keeps its 400.
expect(resolveWaitSignals('blocked', codex)).toEqual({ until: ['blocked'], error: null });
expect(resolveWaitSignals('idle,blocked,exit', codex)).toEqual({ until: ['idle', 'blocked', 'exit'], error: null });
const stop = resolveWaitSignals('stop,blocked', codex);
expect(stop.until).toEqual([]);
expect(stop.error).toContain('stop');
expect(stop.error).not.toContain('Signal(s) stop, blocked');
// The suggestion names what does fire for codex.
expect(stop.error).toContain('idle, blocked or exit');
});
it('drops hook-only signals from the DEFAULT set instead of erroring', () => {
+18
View File
@@ -97,6 +97,24 @@ describe('sendPushNotifications -> webhook (the headless case)', () => {
expect(sendNotification).not.toHaveBeenCalled();
});
it('carries a screen-read dialog (codex) with its own message, and a hook message stays out', async () => {
// web/screen-blocked.ts sends the CLI's own label, built server-side, never the
// dialog's text. A Claude hook's `message` keeps the body it always had.
const { send } = await headlessServer({});
await send('hook:permission_prompt', {
sessionId: 's3',
sessionName: 'w3-app',
source: 'screen',
message: 'Codex is waiting for your answer in the terminal',
});
await send('hook:permission_prompt', {
sessionId: 's4',
sessionName: 'w4-app',
message: 'Claude needs your permission to use Bash',
});
expect(got.map((g) => g.body)).toEqual(['[w3-app] Codex is waiting for your answer in the terminal', '[w4-app]']);
});
it('dedupes an immediate repeat of the same event for the same session', async () => {
const { send } = await headlessServer({});
await send('hook:idle_prompt', { sessionId: 's1', sessionName: 'w1-app' });