fix(review): add Codex last-response test coverage + minor hardening (PR #152)

- Add test/routes/session-routes-codex-last-response.test.ts (app.inject +
  temp CODEX_HOME fixture rollouts): originator match beats cwd fallback when
  two panes share a dir, cwd fallback excludes sibling-claimed/foreign-cwd
  rollouts, resume-uuid filename match, history.jsonl pin outranks originator,
  event_msg/legacy user-turn dedup keeps old-codex turns, injected-context
  filtering, image placeholder, envelope shape ({success:true,data:{text,
  timestamp[,messages]}}), and a Claude-mode regression guard (codex reader
  never consulted for claude sessions)
- Replace clear-at-cap Map caches (codexHistoryPinCache, codexRolloutMetaCache)
  with the repo-standard LRUMap so a full cache wipe can't thrash hot entries
  on large rollout collections
- Join multi-block assistant/user text with a blank line instead of no
  separator (extractCodexBlockText)
- Re-enable the terminal-buffer eye fallback for shell sessions (they have no
  transcript source at all); TUI modes keep the clear placeholder

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-07-12 17:42:39 +02:00
parent 116db81002
commit 895edfedb0
3 changed files with 391 additions and 8 deletions
+4 -3
View File
@@ -1613,11 +1613,12 @@ class CodemanApp {
let lastResponse = data.text || '';
// Source 2: Terminal buffer fallback — strip ANSI, drop Claude CLI chrome.
// Claude-only: _cleanTerminalBuffer knows Claude CLI's output; for TUI
// modes (codex/opencode/gemini) it yields repaint garbage, so a clear
// Claude + shell only: _cleanTerminalBuffer knows Claude CLI's output, and
// shell sessions have no transcript source at all; for TUI modes
// (codex/opencode/gemini) it yields repaint garbage, so a clear
// placeholder beats a messy screen dump there.
const sessionMode = this.sessions.get(this.activeSessionId)?.mode || 'claude';
if (!lastResponse && sessionMode === 'claude') {
if (!lastResponse && (sessionMode === 'claude' || sessionMode === 'shell')) {
const termRes = await fetch(`/api/sessions/${this.activeSessionId}/terminal`);
const termData = (await termRes.json())?.data ?? {};
if (termData.terminalBuffer) {
+4 -5
View File
@@ -62,6 +62,7 @@ import { RunSummaryTracker } from '../../run-summary.js';
import { MAX_INPUT_LENGTH, MAX_SESSION_NAME_LENGTH } from '../../config/terminal-limits.js';
import { MAX_PASTE_IMAGE_BYTES } from '../../config/buffer-limits.js';
import { dataPath } from '../../config/instance.js';
import { LRUMap } from '../../utils/lru-map.js';
// Path to linked-cases registry (same file used by case-routes resolveCasePath)
const LINKED_CASES_FILE = dataPath('linked-cases.json');
@@ -1030,7 +1031,7 @@ export function registerSessionRoutes(
// codex TUI itself. An entry is credited to this pane only when its Enter is
// the closest among all codex panes, so a menu keystroke in another pane
// can't steal the attribution.
const codexHistoryPinCache = new Map<string, { submitAt: number; threadId: string }>();
const codexHistoryPinCache = new LRUMap<string, { submitAt: number; threadId: string }>({ maxSize: 1024 });
async function resolveCodexThreadFromHistory(
session: { id: string; codexLastSubmitAt?: number },
codexHome: string
@@ -1071,7 +1072,6 @@ export function registerSessionRoutes(
if (!best || dist < best.dist) best = { threadId: e.session_id, dist };
}
if (!best) return null;
if (codexHistoryPinCache.size >= 1024) codexHistoryPinCache.clear();
codexHistoryPinCache.set(session.id, { submitAt, threadId: best.threadId });
return best.threadId;
}
@@ -1168,7 +1168,7 @@ export function registerSessionRoutes(
// rewritten (verified: resume appends without touching it), so the parsed
// identity of a given path can be cached forever. This turns the per-request
// scan into stat calls plus head reads for new files only.
const codexRolloutMetaCache = new Map<string, { cwd?: string; originator?: string }>();
const codexRolloutMetaCache = new LRUMap<string, { cwd?: string; originator?: string }>({ maxSize: 4096 });
async function readCodexRolloutMetaCached(
filePath: string,
headBuf: Buffer
@@ -1181,7 +1181,6 @@ export function registerSessionRoutes(
// Don't cache a still-incomplete head: a rollout being created may not
// have flushed session_meta/turn_context yet.
if (!meta.cwd && !meta.originator) return meta;
if (codexRolloutMetaCache.size >= 4096) codexRolloutMetaCache.clear();
codexRolloutMetaCache.set(filePath, meta);
return meta;
}
@@ -1198,7 +1197,7 @@ export function registerSessionRoutes(
typeof (b as { text?: string }).text === 'string'
)
.map((b) => b.text)
.join('');
.join('\n\n');
}
// Single pass over a Codex rollout: track the last assistant message (for the
@@ -0,0 +1,383 @@
/**
* @fileoverview Tests for the Codex branch of GET /api/sessions/:id/last-response (PR #152).
*
* Uses app.inject() — no real HTTP ports needed.
* Port: N/A (app.inject doesn't open ports)
*
* Fixture rollouts live in a per-test temp CODEX_HOME (the route resolves
* `process.env.CODEX_HOME || ~/.codex` at request time), exercising the real
* locator/parser code paths against real files:
* - originator match beats the cwd+mtime fallback when two panes share a dir
* - resume-uuid filename match (resumed rollouts keep foreign session_meta)
* - history.jsonl pin outranks the originator match
* - event_msg vs legacy response_item user-turn dedup keeps old-codex turns
* - injected-context rows (AGENTS.md, environment_context, …) are filtered
* - response envelope shape; Claude-mode behavior unchanged (regression guard)
*/
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import Fastify, { type FastifyInstance } from 'fastify';
import fastifyCookie from '@fastify/cookie';
import { mkdtempSync, mkdirSync, writeFileSync, utimesSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { createMockRouteContext, createMockSession, type MockRouteContext } from '../mocks/index.js';
import { installRouteErrorHandler } from '../../src/web/route-error-handler.js';
import { ApiErrorCode, httpStatusForErrorCode } from '../../src/types.js';
import { registerSessionRoutes } from '../../src/web/routes/session-routes.js';
interface LocalHarness {
app: FastifyInstance;
ctx: MockRouteContext;
}
/**
* Mirror of the production uniform-envelope hook (server.ts) — same local
* harness idiom as session-routes.test.ts, so assertions match the wire format.
*/
async function createEnvelopeHarness(
registerFn: (app: FastifyInstance, ctx: MockRouteContext) => void
): Promise<LocalHarness> {
const app = Fastify({ logger: false });
await app.register(fastifyCookie);
const ctx = createMockRouteContext();
registerFn(app, ctx);
app.addHook('preSerialization', (req, reply, payload: unknown, done) => {
if (!req.url.startsWith('/api')) return done(null, payload);
if (payload === null || typeof payload !== 'object') return done(null, payload);
if (Buffer.isBuffer(payload) || typeof (payload as { pipe?: unknown }).pipe === 'function') {
return done(null, payload);
}
const p = payload as { success?: unknown; errorCode?: unknown };
if (p.success === false) {
if (reply.statusCode === 200 && typeof p.errorCode === 'string') {
reply.code(httpStatusForErrorCode(p.errorCode as ApiErrorCode));
}
return done(null, payload);
}
if (p.success === true) return done(null, payload);
return done(null, { success: true, data: payload });
});
installRouteErrorHandler(app);
await app.ready();
return { app, ctx };
}
// ── Rollout fixture helpers (shapes observed on codex-cli 0.144) ──────────────
const sessionMeta = (cwd: string, originator?: string) => ({
type: 'session_meta',
payload: { cwd, originator },
});
const assistantMsg = (text: string, timestamp = '2026-07-01T00:00:00Z') => ({
timestamp,
type: 'response_item',
payload: { type: 'message', role: 'assistant', content: [{ type: 'output_text', text }] },
});
const legacyUserMsg = (text: string, timestamp = '2026-07-01T00:00:00Z') => ({
timestamp,
type: 'response_item',
payload: { type: 'message', role: 'user', content: [{ type: 'input_text', text }] },
});
const eventUserMsg = (message: string, timestamp = '2026-07-01T00:00:00Z') => ({
timestamp,
type: 'event_msg',
payload: { type: 'user_message', message },
});
const UUID_A = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa';
const UUID_B = 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb';
const UUID_C = 'cccccccc-cccc-4ccc-8ccc-cccccccccccc';
// Fixed epoch (seconds) for deterministic mtime ordering.
const BASE_MTIME = 1_750_000_000;
describe('GET /api/sessions/:id/last-response (codex)', () => {
let harness: LocalHarness;
let codexHome: string;
let prevCodexHome: string | undefined;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let session: any; // MockSession, loosened for codex-only fields (codexConfig, codexLastSubmitAt)
let workdir: string;
/** Write a rollout under CODEX_HOME/sessions/<date>/ with a controlled mtime. */
function writeRollout(name: string, entries: unknown[], mtimeSec: number): string {
const dir = join(codexHome, 'sessions', '2026', '07', '01');
mkdirSync(dir, { recursive: true });
const filePath = join(dir, name);
let content = entries.map((e) => JSON.stringify(e)).join('\n') + '\n';
// The locator skips files under 100 bytes (blank padding lines are ignored by the parser).
while (content.length < 100) content += '\n';
writeFileSync(filePath, content);
utimesSync(filePath, mtimeSec, mtimeSec);
return filePath;
}
function writeHistory(entries: Array<{ session_id: string; ts: number }>): void {
writeFileSync(join(codexHome, 'history.jsonl'), entries.map((e) => JSON.stringify(e)).join('\n') + '\n');
}
async function getLastResponse(id: string, full = false) {
const res = await harness.app.inject({
method: 'GET',
url: `/api/sessions/${id}/last-response${full ? '?context=full' : ''}`,
});
return { res, body: JSON.parse(res.body) };
}
beforeEach(async () => {
codexHome = mkdtempSync(join(tmpdir(), 'codeman-codex-rv-'));
prevCodexHome = process.env.CODEX_HOME;
process.env.CODEX_HOME = codexHome;
harness = await createEnvelopeHarness(registerSessionRoutes);
session = harness.ctx._session;
session.mode = 'codex';
workdir = join(codexHome, 'workdir');
session.workingDir = workdir;
});
afterEach(async () => {
if (prevCodexHome === undefined) delete process.env.CODEX_HOME;
else process.env.CODEX_HOME = prevCodexHome;
rmSync(codexHome, { recursive: true, force: true });
await harness.app.close();
});
// ── Locator: originator vs cwd fallback ─────────────────────────────────
it('originator match beats the cwd+mtime fallback when two panes share a dir', async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const paneB: any = createMockSession('codex-b');
paneB.mode = 'codex';
paneB.workingDir = workdir;
harness.ctx.sessions.set('codex-b', paneB);
// Pane B's rollout is NEWER — the naive cwd+mtime heuristic would show it for pane A.
writeRollout(
`rollout-2026-07-01T00-00-00-${UUID_A}.jsonl`,
[sessionMeta(workdir, `codeman_${session.id}`), assistantMsg('answer A')],
BASE_MTIME
);
writeRollout(
`rollout-2026-07-01T00-01-00-${UUID_B}.jsonl`,
[sessionMeta(workdir, 'codeman_codex-b'), assistantMsg('answer B')],
BASE_MTIME + 100
);
const a = await getLastResponse(session.id);
expect(a.res.statusCode).toBe(200);
expect(a.body.data.text).toBe('answer A');
const b = await getLastResponse('codex-b');
expect(b.body.data.text).toBe('answer B');
});
it('cwd fallback excludes rollouts claimed by other panes and skips foreign cwds', async () => {
// Pane has no originator-stamped rollout (pre-existing pane). Newest same-cwd
// rollout belongs to another codeman pane → must fall through to the unclaimed one.
writeRollout(
`rollout-2026-07-01T00-00-00-${UUID_A}.jsonl`,
[sessionMeta(workdir), assistantMsg('unclaimed answer')],
BASE_MTIME
);
writeRollout(
`rollout-2026-07-01T00-01-00-${UUID_B}.jsonl`,
[sessionMeta(workdir, 'codeman_some-other-pane'), assistantMsg('sibling answer')],
BASE_MTIME + 100
);
writeRollout(
`rollout-2026-07-01T00-02-00-${UUID_C}.jsonl`,
[sessionMeta('/elsewhere/entirely'), assistantMsg('foreign-cwd answer')],
BASE_MTIME + 200
);
const { body } = await getLastResponse(session.id);
expect(body.data.text).toBe('unclaimed answer');
});
// ── Locator: resume-uuid match ───────────────────────────────────────────
it('resolves a resumed pane via the rollout filename uuid despite foreign session_meta', async () => {
session.codexConfig = { resumeSessionId: UUID_A };
// Resumed rollouts keep the ORIGINAL session_meta (foreign originator + launch cwd),
// so neither originator nor cwd matching can find them — only the filename uuid.
writeRollout(
`rollout-2026-06-30T12-00-00-${UUID_A}.jsonl`,
[sessionMeta('/original/launch/dir', 'codex_cli_rs'), assistantMsg('resumed answer')],
BASE_MTIME
);
// A newer same-cwd decoy must NOT win over the uuid match.
writeRollout(
`rollout-2026-07-01T00-00-00-${UUID_B}.jsonl`,
[sessionMeta(workdir), assistantMsg('decoy answer')],
BASE_MTIME + 100
);
const { body } = await getLastResponse(session.id);
expect(body.data.text).toBe('resumed answer');
});
// ── Locator: history.jsonl pin ───────────────────────────────────────────
it('history.jsonl pin (pane last-submit correlation) outranks the originator match', async () => {
const submitAtSec = BASE_MTIME + 500;
session.codexLastSubmitAt = submitAtSec * 1000;
writeHistory([{ session_id: UUID_B, ts: submitAtSec }]);
// Originator-stamped rollout exists and is NEWER, but the pane /resume'd onto
// UUID_B inside the TUI — the history pin must follow it there.
writeRollout(
`rollout-2026-07-01T00-00-00-${UUID_A}.jsonl`,
[sessionMeta(workdir, `codeman_${session.id}`), assistantMsg('originator answer')],
BASE_MTIME + 600
);
writeRollout(
`rollout-2026-06-30T12-00-00-${UUID_B}.jsonl`,
[sessionMeta('/original/launch/dir', 'codex_cli_rs'), assistantMsg('history answer')],
BASE_MTIME
);
const { body } = await getLastResponse(session.id);
expect(body.data.text).toBe('history answer');
});
// ── Reader: dedup + filtering ────────────────────────────────────────────
it('event_msg/legacy dedup keeps old-codex turns and drops event twins (mixed-version rollout)', async () => {
writeRollout(
`rollout-2026-07-01T00-00-00-${UUID_A}.jsonl`,
[
sessionMeta(workdir, `codeman_${session.id}`),
// Old-codex turn: response_item only, no event_msg twin — must survive.
legacyUserMsg('old prompt'),
assistantMsg('old answer'),
// Modern turn: event_msg + duplicate response_item row — one user row only.
eventUserMsg('new prompt'),
legacyUserMsg('new prompt'),
assistantMsg('new answer'),
],
BASE_MTIME
);
const { body } = await getLastResponse(session.id, true);
expect(body.data.text).toBe('new answer');
expect(body.data.messages.map((m: { role: string; text: string }) => [m.role, m.text])).toEqual([
['user', 'old prompt'],
['assistant', 'old answer'],
['user', 'new prompt'],
['assistant', 'new answer'],
]);
});
it('filters injected-context rows from the full thread', async () => {
writeRollout(
`rollout-2026-07-01T00-00-00-${UUID_A}.jsonl`,
[
sessionMeta(workdir, `codeman_${session.id}`),
legacyUserMsg('# AGENTS.md instructions for the workspace'),
legacyUserMsg('<environment_context>\n<cwd>/somewhere</cwd>'),
eventUserMsg('<user_instructions>be nice</user_instructions>'),
legacyUserMsg('real question'),
assistantMsg('real answer'),
],
BASE_MTIME
);
const { body } = await getLastResponse(session.id, true);
expect(body.data.messages).toEqual([
{ role: 'user', text: 'real question', timestamp: '2026-07-01T00:00:00Z' },
{ role: 'assistant', text: 'real answer', timestamp: '2026-07-01T00:00:00Z' },
]);
});
it('renders an image placeholder for image-only event_msg inputs', async () => {
writeRollout(
`rollout-2026-07-01T00-00-00-${UUID_A}.jsonl`,
[
sessionMeta(workdir, `codeman_${session.id}`),
{ timestamp: '2026-07-01T00:00:00Z', type: 'event_msg', payload: { type: 'user_message', images: ['a', 'b'] } },
assistantMsg('looked at the images'),
],
BASE_MTIME
);
const { body } = await getLastResponse(session.id, true);
expect(body.data.messages[0]).toEqual({
role: 'user',
text: '*[image ×2]*',
timestamp: '2026-07-01T00:00:00Z',
});
});
// ── Envelope shape + Claude-mode regression guard ────────────────────────
it('returns the {success:true,data:{text,timestamp}} envelope; messages only with ?context=full', async () => {
writeRollout(
`rollout-2026-07-01T00-00-00-${UUID_A}.jsonl`,
[sessionMeta(workdir, `codeman_${session.id}`), assistantMsg('the answer', '2026-07-01T01:02:03Z')],
BASE_MTIME
);
const brief = await getLastResponse(session.id);
expect(brief.res.statusCode).toBe(200);
expect(brief.body).toEqual({
success: true,
data: { text: 'the answer', timestamp: '2026-07-01T01:02:03Z' },
});
const full = await getLastResponse(session.id, true);
expect(full.body.success).toBe(true);
expect(Array.isArray(full.body.data.messages)).toBe(true);
});
it('returns an empty envelope (not an error) when no rollout matches', async () => {
const brief = await getLastResponse(session.id);
expect(brief.res.statusCode).toBe(200);
expect(brief.body).toEqual({ success: true, data: { text: '', timestamp: '' } });
const full = await getLastResponse(session.id, true);
expect(full.body.data.messages).toEqual([]);
});
it('leaves Claude-mode sessions on the ~/.claude/projects reader (regression guard)', async () => {
const fakeHome = mkdtempSync(join(tmpdir(), 'codeman-claude-home-'));
const prevHome = process.env.HOME;
process.env.HOME = fakeHome;
try {
session.mode = 'claude';
const projDir = join(fakeHome, '.claude', 'projects', 'proj1');
mkdirSync(projDir, { recursive: true });
writeFileSync(
join(projDir, `${session.id}.jsonl`),
JSON.stringify({
type: 'assistant',
timestamp: '2026-07-01T00:00:00Z',
message: { content: [{ type: 'text', text: 'claude answer' }] },
}) + '\n'
);
// A codex rollout for the same session id must NOT be consulted in claude mode.
writeRollout(
`rollout-2026-07-01T00-00-00-${UUID_A}.jsonl`,
[sessionMeta(workdir, `codeman_${session.id}`), assistantMsg('codex answer')],
BASE_MTIME
);
const { res, body } = await getLastResponse(session.id);
expect(res.statusCode).toBe(200);
expect(body.data.text).toBe('claude answer');
expect(body.data.timestamp).toBe('2026-07-01T00:00:00Z');
} finally {
process.env.HOME = prevHome;
rmSync(fakeHome, { recursive: true, force: true });
}
});
});