mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 10:39:32 +01:00
fix(edit-history): bounded image probe, linear path scan (review of #693)
- probeLocalPaths: every agent-printed candidate goes through probePathKind() first (no pastCap, bulk scan), sequentially; only a path that answered 'file' gets realpath + stat. A path on a mount that stopped answering no longer hangs the sync, a queued sync or remove(). - PATH_TOKEN_RE: branch 4 starts only at a token boundary (`:` excluded so URLs stay ignored), branch 1 stops at `:`. Linear in the line length (30,000 `#` took ~3 s before). - Load older yields to the event loop between the files it parses. - GET /edits answers unsupported without loading a store. - CLAUDE.md paragraph trimmed to the rules; probe and regex rules added to architecture-invariants#edit-history-panel. Tests: stall test through the real sync and probe with a stat that never settles; time-budget tests with 30,000-character inputs and a 139-result base64 chunk; route test asserts no store load for unsupported sessions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
bf2a705705
commit
de43657676
10 files changed
+229
-35
No files matched your search
@@ -141,12 +141,25 @@ describe('readClaudeEditSources', () => {
|
||||
});
|
||||
|
||||
describe('probeLocalPaths', () => {
|
||||
it('reports kind, size and mtime, expands ~/ against home, null for missing', async () => {
|
||||
it('reports size and mtime of files, expands ~/ against home, null for missing and directories', async () => {
|
||||
put('home/img.png', 'PNG!', 1_700_000_000);
|
||||
mkdirSync(join(root, 'dir'));
|
||||
const res = await probeLocalPaths(['~/img.png', join(root, 'dir'), join(root, 'nope.png')], join(root, 'home'));
|
||||
expect(res[0]).toEqual({ realPath: join(root, 'home/img.png'), kind: 'file', size: 4, mtimeMs: 1_700_000_000_000 });
|
||||
expect(res[1]).toMatchObject({ kind: 'directory', size: 0 });
|
||||
expect(res[1]).toBeNull();
|
||||
expect(res[2]).toBeNull();
|
||||
});
|
||||
|
||||
it('touches a path only after the bounded probe saw a file there', async () => {
|
||||
put('a.png', 'PNG!', 1_700_000_000);
|
||||
put('b.png', 'PNG!', 1_700_000_000);
|
||||
const asked: string[] = [];
|
||||
const res = await probeLocalPaths([join(root, 'a.png'), join(root, 'b.png'), 'rel.png'], root, async (p) => {
|
||||
asked.push(p);
|
||||
return p.endsWith('a.png') ? 'unknown' : 'file';
|
||||
});
|
||||
// a.png exists, but its filesystem did not answer the bounded probe: no unbounded stat.
|
||||
expect(res.map((r) => r?.realPath ?? null)).toEqual([null, join(root, 'b.png'), null]);
|
||||
expect(asked).toEqual([join(root, 'a.png'), join(root, 'b.png')]); // relative: never probed
|
||||
});
|
||||
});
|
||||
@@ -349,6 +349,50 @@ describe('extractImagePathCandidates', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('extractImagePathCandidates: linear in the line length (#693 review)', () => {
|
||||
// Claude Code keeps up to 30,000 characters of Bash output, all of it agent-printed, and the
|
||||
// scan runs synchronously in the request. Before the fix one such line took ~2.7 s.
|
||||
const N = 30_000;
|
||||
const BUDGET_MS = 250;
|
||||
const timed = (text: string) => {
|
||||
const t = performance.now();
|
||||
const out = extractImagePathCandidates('render', text, '', CWD);
|
||||
return { out, ms: performance.now() - t };
|
||||
};
|
||||
const base64 = Buffer.from(Array.from({ length: (N * 3) / 4 }, (_, i) => (i * 131 + 7) & 255)).toString('base64');
|
||||
|
||||
it.each([
|
||||
...'#+%@![{^:/a.~-$'.split('').map((c) => [`30,000 × ${c}`, c.repeat(N)] as const),
|
||||
['a base64 blob', base64] as const,
|
||||
['repeated file:///', 'file:///'.repeat(N / 8)] as const,
|
||||
['repeated file:///x', 'file:///x'.repeat(N / 9)] as const,
|
||||
['an unclosed quote', '"' + 'a'.repeat(N)] as const,
|
||||
['repeated =a', '=a'.repeat(N / 2)] as const,
|
||||
])('%s stays under the time budget and still finds a path after it', (_label, line) => {
|
||||
const { out, ms } = timed(`${line}\nsaved out/plot.png`);
|
||||
expect(ms).toBeLessThan(BUDGET_MS);
|
||||
expect(out).toContain('/work/repo/out/plot.png');
|
||||
});
|
||||
|
||||
it('parses a chunk of 139 Bash results with a 30,000-character base64 blob each under budget', () => {
|
||||
const lines: string[] = [];
|
||||
for (let i = 0; i < 139; i++) {
|
||||
lines.push(bashUseLine(`toolu_${i}`, '2026-10-15T20:00:00.000Z', 'cat img.b64'));
|
||||
lines.push(
|
||||
toolResultLine(`toolu_${i}`, '2026-10-15T20:00:01.000Z', {
|
||||
stdout: base64,
|
||||
stderr: '',
|
||||
interrupted: false,
|
||||
isImage: false,
|
||||
})
|
||||
);
|
||||
}
|
||||
const t = performance.now();
|
||||
parseClaudeEditLines(lines.join('\n'));
|
||||
expect(performance.now() - t).toBeLessThan(1500);
|
||||
});
|
||||
});
|
||||
|
||||
describe('classifyEditPath', () => {
|
||||
it.each([
|
||||
['/a/b.png', 'image'],
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
/**
|
||||
* Edit history image probe against a filesystem that stopped answering (#693 review): a Bash
|
||||
* call printed a path on a dead hard mount, whose stat() never settles. The real sync with the
|
||||
* real probe (`probeLocalPaths` → bounded `probePathKind`) must still finish, the next queued
|
||||
* sync too, and `remove()` must not wait on it. Own file: the stalled probe stays recorded
|
||||
* process-wide for the rest of the run.
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
const DEAD = '/mnt/dead-nas';
|
||||
|
||||
vi.hoisted(() => {
|
||||
process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS = '100';
|
||||
});
|
||||
|
||||
vi.mock('node:fs/promises', async (importOriginal) => {
|
||||
const real = await importOriginal<typeof import('node:fs/promises')>();
|
||||
const hangs = (p: unknown) => String(p).startsWith('/mnt/dead-nas');
|
||||
const stat = ((p: Parameters<typeof real.stat>[0], ...rest: unknown[]) =>
|
||||
hangs(p) ? new Promise(() => {}) : (real.stat as (...a: unknown[]) => unknown)(p, ...rest)) as typeof real.stat;
|
||||
const realpath = ((p: Parameters<typeof real.realpath>[0], ...rest: unknown[]) =>
|
||||
hangs(p)
|
||||
? new Promise(() => {})
|
||||
: (real.realpath as (...a: unknown[]) => unknown)(p, ...rest)) as typeof real.realpath;
|
||||
return { ...real, stat, realpath, default: { ...real, stat, realpath } };
|
||||
});
|
||||
|
||||
import { mkdtempSync, rmSync, utimesSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { EditHistoryStore } from '../src/edit-history-store.js';
|
||||
import { probeLocalPaths, type EditSources } from '../src/edit-history-local-source.js';
|
||||
import { EditHistorySync, type EditHistorySession } from '../src/web/edit-history-sync.js';
|
||||
|
||||
const T0 = Date.parse('2026-10-15T20:00:00.000Z');
|
||||
const session: EditHistorySession = { id: 's1', mode: 'claude', remote: undefined, claudeSessionId: 'conv1' };
|
||||
|
||||
function bashPair(id: string, start: number, end: number, stdout: string): string {
|
||||
const use = {
|
||||
type: 'assistant',
|
||||
timestamp: new Date(start).toISOString(),
|
||||
cwd: '/w',
|
||||
message: { role: 'assistant', content: [{ type: 'tool_use', id, name: 'Bash', input: { command: 'render' } }] },
|
||||
};
|
||||
const res = {
|
||||
type: 'user',
|
||||
timestamp: new Date(end).toISOString(),
|
||||
cwd: '/w',
|
||||
message: { role: 'user', content: [{ tool_use_id: id, type: 'tool_result', content: stdout }] },
|
||||
toolUseResult: { stdout, stderr: '', interrupted: false, isImage: false },
|
||||
};
|
||||
return JSON.stringify(use) + '\n' + JSON.stringify(res) + '\n';
|
||||
}
|
||||
|
||||
/** Rejects after `ms`, so a hang fails the test instead of timing it out silently. */
|
||||
function within<T>(p: Promise<T>, ms: number, what: string): Promise<T> {
|
||||
return Promise.race([
|
||||
p,
|
||||
new Promise<T>((_, reject) => setTimeout(() => reject(new Error(`${what} still pending after ${ms} ms`)), ms)),
|
||||
]);
|
||||
}
|
||||
|
||||
let dir: string;
|
||||
let stores: string;
|
||||
beforeEach(() => {
|
||||
dir = mkdtempSync(join(tmpdir(), 'edit-stall-'));
|
||||
stores = join(dir, 'stores');
|
||||
});
|
||||
afterEach(() => {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('edit history image probe on an unresponsive mount', () => {
|
||||
it('finishes sync, a queued sync and remove() while one printed path never answers', async () => {
|
||||
const ok = join(dir, 'ok.png');
|
||||
writeFileSync(ok, 'PNG!');
|
||||
utimesSync(ok, new Date(T0 + 2000), new Date(T0 + 2000));
|
||||
let main = Buffer.from(bashPair('b1', T0, T0 + 5000, `${DEAD}/plots/a.png\nsaved ${ok}\n`));
|
||||
|
||||
const sync = new EditHistorySync({
|
||||
transcriptPath: async () => '/p/conv1.jsonl',
|
||||
readSources: async (_path, requests, budget): Promise<EditSources> => {
|
||||
const req = requests.find((r) => r.relPath === 'main');
|
||||
const start = Math.min(req?.offset ?? 0, main.length);
|
||||
const len = Math.min(budget, main.length - start);
|
||||
return {
|
||||
transcriptId: 'conv1',
|
||||
files: [{ relPath: 'main', size: main.length, startOffset: start, chunk: main.subarray(start, start + len) }],
|
||||
metas: {},
|
||||
};
|
||||
},
|
||||
probe: (paths) => probeLocalPaths(paths),
|
||||
loadStore: (id) => EditHistoryStore.load(id, { dir: stores, debounceMs: 0 }),
|
||||
deleteStore: (id) => EditHistoryStore.delete(id, { dir: stores }),
|
||||
sweepStores: async () => 0,
|
||||
});
|
||||
|
||||
const first = sync.sync(session);
|
||||
main = Buffer.concat([main, Buffer.from(bashPair('b2', T0 + 6000, T0 + 7000, `${DEAD}/plots/b.png\n`))]);
|
||||
const second = sync.sync(session);
|
||||
expect(await within(first, 1500, 'first sync')).toEqual({ state: 'ok' });
|
||||
expect(await within(second, 1500, 'queued sync')).toEqual({ state: 'ok' });
|
||||
expect((await sync.getStore('s1')).list().map((e) => e.path)).toEqual([ok]);
|
||||
await within(sync.remove('s1'), 1500, 'remove()');
|
||||
});
|
||||
});
|
||||
@@ -186,12 +186,16 @@ describe('GET /api/sessions/:id/edits', () => {
|
||||
['remote', { remote: { host: 'huff' } }],
|
||||
['docker', { docker: { image: 'x' } }],
|
||||
['mode', { mode: 'shell' }],
|
||||
])('answers unsupported (%s) without reading', async (reason, patch) => {
|
||||
const { app, read } = await setup();
|
||||
])('answers unsupported (%s) without reading or loading a store', async (reason, patch) => {
|
||||
const loadStore = vi.fn(async (id: string) => EditHistoryStore.load(id, { dir, debounceMs: 0 }));
|
||||
const { app, read } = await setup({ loadStore });
|
||||
Object.assign(session, patch);
|
||||
const data = (await app.inject({ method: 'GET', url: `${URL}?sync=1` })).json().data;
|
||||
expect(data).toMatchObject({ state: 'unsupported', reason });
|
||||
for (const url of [URL, `${URL}?sync=1`]) {
|
||||
const data = (await app.inject({ method: 'GET', url })).json().data;
|
||||
expect(data).toEqual({ state: 'unsupported', reason, backfillTruncated: false, agents: {}, entries: [] });
|
||||
}
|
||||
expect(read).not.toHaveBeenCalled();
|
||||
expect(loadStore).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('404s an unknown session', async () => {
|
||||
|
||||
Reference in new issue
Block a user