mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-15 10:59:32 +01:00
fix(story-mode): the reader cannot stall the server, and reads only real transcripts
From review round 7 (security): - Eviction past the per-transcript caps dropped entries one shift() at a time after the whole tail was parsed: quadratic and never yielding (a 2 s event-loop stall on a 70 MB long turn, 51 s from 300k entries). It is one splice, done at every 1 MB yield, so the peak stays near the caps; and a window that saw a turn start which was then trimmed away no longer widens to a bigger tail that would be trimmed the same. - A FIFO or a device in place of a transcript or a helper's .meta.json (stat said size 0, then open() waited for a writer) pinned a libuv worker per request; four of them hang every fs, dns and crypto call on the server. Files are now opened non-blocking, checked with the handle's own stat (a regular file only) and read through that handle. - The cache held 6 transcripts for everyone, so a seventh open view made every poll a full re-read, and a 304 was decided only after reading. An unchanged file is now a 304 from a stat alone, and the cache holds up to 24 transcripts within 30 million characters. - Story Mode is the first route to send a transcript's content to the browser, so the hook-reported path is used only when it really lies (symlinks resolved) in a Claude projects folder: the session's relocated CLAUDE_CONFIG_DIR (now also used for the lookup by id) or the server's. Anything else falls back to the lookup by conversation id. Tests move their transcripts into the (temp) projects folder and cover a path outside it, a symlink out of it, a FIFO transcript, a FIFO meta file and a 100,000-entry tail. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
7aa7106cb4
commit
250a77d945
6 files changed
+258
-44
No files matched your search
@@ -2,13 +2,15 @@
|
||||
* @fileoverview GET /api/sessions/:id/story (Story Mode) against invented transcripts in a temp dir,
|
||||
* plus the reader's incremental tail read. Port: N/A (app.inject()).
|
||||
*/
|
||||
import { appendFileSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { appendFileSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { createRouteTestHarness } from './_route-test-utils.js';
|
||||
import { registerStoryRoutes } from '../../src/web/routes/story-routes.js';
|
||||
import { clearStoryReaderCache, readStoryEntries } from '../../src/story-reader.js';
|
||||
import { claudeProjectsDir } from '../../src/utils/claude-transcript.js';
|
||||
import { bash, demoTurn, edit, end, prompt, read, resetClock, result, said, tool } from '../story-fixtures.js';
|
||||
|
||||
const URL = '/api/sessions/test-session-1/story';
|
||||
@@ -30,13 +32,72 @@ async function setup(opts: { hookPath?: string | null; mode?: string; remote?: u
|
||||
beforeEach(() => {
|
||||
resetClock();
|
||||
clearStoryReaderCache();
|
||||
dir = mkdtempSync(join(tmpdir(), 'story-route-'));
|
||||
// Where Claude Code writes transcripts (under the test's temp HOME): the only place a hook path is read.
|
||||
mkdirSync(claudeProjectsDir(), { recursive: true });
|
||||
dir = mkdtempSync(join(claudeProjectsDir(), 'story-route-'));
|
||||
transcript = join(dir, 'conversation.jsonl');
|
||||
});
|
||||
afterEach(() => {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('GET /api/sessions/:id/story: which file is read', () => {
|
||||
it('a hook path outside the Claude projects folder is never read, nor a symlink out of it', async () => {
|
||||
const outside = mkdtempSync(join(tmpdir(), 'story-outside-'));
|
||||
try {
|
||||
const file = join(outside, 'conversation.jsonl');
|
||||
writeFileSync(file, jsonl([prompt('secret job'), said('Done.'), end(2000)]));
|
||||
let { app } = await setup({ hookPath: file });
|
||||
let body = (await app.inject({ method: 'GET', url: URL })).json();
|
||||
expect(body.data).toMatchObject({ supported: true, turns: [], page: null });
|
||||
const link = join(dir, 'link.jsonl');
|
||||
symlinkSync(file, link);
|
||||
({ app } = await setup({ hookPath: link }));
|
||||
body = (await app.inject({ method: 'GET', url: URL })).json();
|
||||
expect(body.data).toMatchObject({ supported: true, turns: [], page: null });
|
||||
} finally {
|
||||
rmSync(outside, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("a FIFO in place of a helper's meta file costs nothing: the helper still draws", async () => {
|
||||
writeFileSync(transcript, jsonl([prompt('go'), end(1000)]));
|
||||
const sub = join(dir, 'conversation', 'subagents');
|
||||
mkdirSync(sub, { recursive: true });
|
||||
writeFileSync(join(sub, 'agent-h1.jsonl'), jsonl([prompt('helper task'), said('Working.')]));
|
||||
execFileSync('mkfifo', [join(sub, 'agent-h1.meta.json')]);
|
||||
const { app } = await setup();
|
||||
const started = Date.now();
|
||||
const res = await app.inject({ method: 'GET', url: `${URL}/helpers/h1` });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.json().data.helper).toEqual({ agentId: 'h1' });
|
||||
expect(Date.now() - started).toBeLessThan(2000);
|
||||
});
|
||||
|
||||
it('a transcript with far more entries than are kept reads in one pass and keeps the newest', async () => {
|
||||
const lines = [prompt('one long turn')];
|
||||
for (let i = 0; i < 100_000; i++) lines.push(said(`step ${i}`));
|
||||
writeFileSync(transcript, jsonl(lines));
|
||||
const started = Date.now();
|
||||
const read = await readStoryEntries(transcript);
|
||||
// Trimmed with one splice per megabyte, never one shift() per entry (seconds at this size).
|
||||
expect(Date.now() - started).toBeLessThan(4000);
|
||||
expect(read.entries).toHaveLength(20_000);
|
||||
expect(read.partial).toBe(true);
|
||||
});
|
||||
|
||||
it('a FIFO in place of the transcript fails at once, never waiting for a writer', async () => {
|
||||
const fifo = join(dir, 'fifo.jsonl');
|
||||
execFileSync('mkfifo', [fifo]);
|
||||
const { app } = await setup({ hookPath: fifo });
|
||||
const started = Date.now();
|
||||
const res = await app.inject({ method: 'GET', url: URL });
|
||||
expect(res.statusCode).toBe(500);
|
||||
expect(res.json().errorCode).toBe('OPERATION_FAILED');
|
||||
expect(Date.now() - started).toBeLessThan(2000);
|
||||
});
|
||||
});
|
||||
|
||||
describe('GET /api/sessions/:id/story', () => {
|
||||
it('draws the newest turn of the hook-reported transcript', async () => {
|
||||
writeFileSync(transcript, jsonl([prompt('first job'), said('Done.'), end(2000), ...demoTurn()]));
|
||||
@@ -103,11 +164,11 @@ describe('GET /api/sessions/:id/story', () => {
|
||||
expect(res.json().data).toEqual({ supported: true, turns: [], page: null });
|
||||
});
|
||||
|
||||
it('fails with OPERATION_FAILED when the transcript cannot be read', async () => {
|
||||
it('a hook path that is gone falls back to the lookup by conversation id: nothing yet, not an error', async () => {
|
||||
const { app } = await setup({ hookPath: join(dir, 'missing.jsonl') });
|
||||
const res = await app.inject({ method: 'GET', url: URL });
|
||||
expect(res.statusCode).toBe(500);
|
||||
expect(res.json()).toMatchObject({ success: false, errorCode: 'OPERATION_FAILED' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.json().data).toMatchObject({ supported: true, turns: [], page: null });
|
||||
});
|
||||
|
||||
it('404s an unknown session', async () => {
|
||||
|
||||
@@ -3,12 +3,13 @@
|
||||
* synthetic transcript (test/story-fixtures.ts) handed to the session the way Claude's hooks do it,
|
||||
* then grown while the overlay is open so the live redraw runs off real SSE events.
|
||||
*/
|
||||
import { appendFileSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { appendFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { homedir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
|
||||
import { chromium, type Browser, type Page } from 'playwright';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
import { claudeProjectsDir } from '../src/utils/claude-transcript.js';
|
||||
import { demoTurn, resetClock } from './story-fixtures.js';
|
||||
|
||||
const jsonl = (lines: unknown[]) => lines.map((l) => JSON.stringify(l)).join('\n') + '\n';
|
||||
@@ -18,6 +19,7 @@ describe('Story Mode in a real browser', () => {
|
||||
let browser: Browser;
|
||||
let page: Page;
|
||||
let root: string;
|
||||
let transcriptDir: string;
|
||||
let transcript: string;
|
||||
let sessionId: string;
|
||||
let rest: unknown[];
|
||||
@@ -39,7 +41,10 @@ describe('Story Mode in a real browser', () => {
|
||||
beforeAll(async () => {
|
||||
resetClock();
|
||||
root = mkdtempSync(join(homedir(), 'story-ui-'));
|
||||
transcript = join(root, 'conversation.jsonl');
|
||||
// Where Claude Code writes transcripts (under the test's temp HOME): the only place a hook path is read.
|
||||
mkdirSync(claudeProjectsDir(), { recursive: true });
|
||||
transcriptDir = mkdtempSync(join(claudeProjectsDir(), 'story-ui-'));
|
||||
transcript = join(transcriptDir, 'conversation.jsonl');
|
||||
const turn = demoTurn();
|
||||
// The turn up to the first edit's result is "running"; the rest arrives during the test.
|
||||
writeFileSync(transcript, jsonl(turn.slice(0, 14)));
|
||||
@@ -71,6 +76,7 @@ describe('Story Mode in a real browser', () => {
|
||||
if (browser) await browser.close();
|
||||
if (server) await server.stop();
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
rmSync(transcriptDir, { recursive: true, force: true });
|
||||
}, 60000);
|
||||
|
||||
it('is off by default: no header button', async () => {
|
||||
|
||||
Reference in new issue
Block a user