mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 02:29:33 +01:00
fix(edit-history): colons in file:// paths, yield on forward reads, no store for unsupported
Follow-up from an own review pass of de436576:
- PATH_TOKEN_RE branch 1 stops at the next `file://` instead of at `:`,
so `file:///tmp/plot-12:30.png` is found again; still linear.
- Forward reads yield between files like "Load older"; a store removed
meanwhile ends the read and its printed paths are never probed.
- POST /edits/older and GET /edits/:entryId no longer load a store for
remote, Docker or non-claude sessions (shared unsupportedList()).
- Time budgets widened (1 s per line, 3 s per chunk; ~3 s / 6.6 s before
the fix) so the throttled suite does not flake.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
de43657676
commit
109fd98038
8 files changed
+68
-21
No files matched your search
@@ -104,9 +104,10 @@ const TOKEN_CHAR = '[^\\s\'"`<>|;&()=,:*?]';
|
||||
const PATH_TOKEN_RE = new RegExp(
|
||||
[
|
||||
// 1. file:// URL (Codex: `Saved to: file:///…`) — may contain spaces up to the extension.
|
||||
// No `:` inside: it keeps the lazy scan from running to the line end from every
|
||||
// `file://` of a line that repeats it (quadratic otherwise).
|
||||
`file://(/[^\\n:]*?\\.${IMAGE_EXT})${EXT_END}`,
|
||||
// The lazy scan stops at the next `file://`: otherwise it runs to the line end from
|
||||
// every `file://` of a line that repeats it (quadratic). `:` stays allowed, so a
|
||||
// timestamped name (`plot-12:30.png`) is still found.
|
||||
`file://(/(?:(?!file://)[^\\n])*?\\.${IMAGE_EXT})${EXT_END}`,
|
||||
// 2./3. Quoted paths, spaces allowed.
|
||||
`"([^"\\n]*\\.${IMAGE_EXT})"`,
|
||||
`'([^'\\n]*\\.${IMAGE_EXT})'`,
|
||||
|
||||
@@ -573,6 +573,11 @@ export class EditHistoryStore {
|
||||
* Delete the persisted file (session deleted) and forget the content. The store takes no
|
||||
* changes afterwards, so a read still in flight neither writes nor refills it.
|
||||
*/
|
||||
/** True once {@link remove} ran: the store ignores every later write. */
|
||||
isRemoved(): boolean {
|
||||
return this.removed;
|
||||
}
|
||||
|
||||
async remove(): Promise<void> {
|
||||
this.removed = true;
|
||||
if (this.saveTimer) {
|
||||
|
||||
@@ -110,6 +110,11 @@ export function unsupportedReason(session: EditHistorySession): string | undefin
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/** One turn of the event loop, between synchronous parses of up to one budget each. */
|
||||
function breathe(): Promise<void> {
|
||||
return new Promise((resolve) => setImmediate(resolve));
|
||||
}
|
||||
|
||||
/**
|
||||
* Bash candidates whose probed file is a regular file with an mtime inside the call's window
|
||||
* (files claude sent to the user skip the window: they exist and were shown on purpose).
|
||||
@@ -386,13 +391,18 @@ export class EditHistorySync {
|
||||
const requested = new Set(requests.map((r) => r.relPath));
|
||||
let more = false;
|
||||
const candidates: ImageCandidate[] = [];
|
||||
let parsed = false;
|
||||
for (const f of res.files) {
|
||||
// Each file is up to one budget (8 MiB) of synchronous parsing: yield between them.
|
||||
if (parsed) await breathe();
|
||||
if (store.isRemoved()) break;
|
||||
// A requested file reads on from its cursor, a new one its tail.
|
||||
const available = requested.has(f.relPath) ? f.size - f.startOffset : f.size;
|
||||
if (f.chunk.length < Math.min(this.budget, available)) {
|
||||
more = true;
|
||||
continue;
|
||||
}
|
||||
parsed = true;
|
||||
const r = store.apply({
|
||||
transcriptId: res.transcriptId,
|
||||
source: f.relPath,
|
||||
@@ -439,11 +449,10 @@ export class EditHistorySync {
|
||||
let parsed = false;
|
||||
for (const f of res.files) {
|
||||
if (!wanted.has(f.relPath)) continue;
|
||||
// Each file is up to one budget (8 MiB) of synchronous parsing: let the server breathe
|
||||
// between them. A store removed meanwhile ignores the rest (it is inert once removed).
|
||||
if (parsed) await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
// Each file is up to one budget (8 MiB) of synchronous parsing: yield between them.
|
||||
if (parsed) await breathe();
|
||||
parsed = true;
|
||||
if (slot.gone) break;
|
||||
if (slot.gone) return true;
|
||||
const r = store.applyOlder({
|
||||
transcriptId: res.transcriptId,
|
||||
source: f.relPath,
|
||||
@@ -465,7 +474,8 @@ export class EditHistorySync {
|
||||
|
||||
private async addImages(store: EditHistoryStore, candidates: readonly ImageCandidate[]): Promise<void> {
|
||||
const paths = [...new Set(candidates.flatMap((c) => c.paths))];
|
||||
if (paths.length === 0) return;
|
||||
// A removed store keeps nothing: do not probe agent-printed paths for it.
|
||||
if (paths.length === 0 || store.isRemoved()) return;
|
||||
let probes: Array<PathProbe | null>;
|
||||
try {
|
||||
probes = await this.deps.probe(paths);
|
||||
|
||||
@@ -86,6 +86,11 @@ function listOf(
|
||||
return list;
|
||||
}
|
||||
|
||||
/** The answer for a session that is never read (remote, Docker, non-claude): no store involved. */
|
||||
function unsupportedList(reason: string): EditHistoryList {
|
||||
return { state: 'unsupported', reason, backfillTruncated: false, agents: {}, entries: [] };
|
||||
}
|
||||
|
||||
export function registerEditHistoryRoutes(app: FastifyInstance, ctx: SessionPort, sync: EditHistorySync): void {
|
||||
const sessionOf = (id: string, req: Parameters<typeof findSessionOrFail>[2]) => {
|
||||
const session = findSessionOrFail(ctx, id, req);
|
||||
@@ -106,12 +111,7 @@ export function registerEditHistoryRoutes(app: FastifyInstance, ctx: SessionPort
|
||||
const { session, view } = sessionOf(id, req);
|
||||
const why = unsupportedReason(view);
|
||||
// Never read: answer an empty list without loading (or creating) a store for it.
|
||||
if (why) {
|
||||
return {
|
||||
success: true,
|
||||
data: { state: 'unsupported', reason: why, backfillTruncated: false, agents: {}, entries: [] },
|
||||
};
|
||||
}
|
||||
if (why) return { success: true, data: unsupportedList(why) };
|
||||
// The sync starts in the same tick as the session check: a deletion right after it marks
|
||||
// the running sync gone (EditHistorySync.remove), so nothing writes a deleted session.
|
||||
const status: EditHistoryStatus = doSync === '1' ? await sync.sync(view) : sync.getStatus(session.id);
|
||||
@@ -121,14 +121,16 @@ export function registerEditHistoryRoutes(app: FastifyInstance, ctx: SessionPort
|
||||
app.post('/api/sessions/:id/edits/older', async (req): Promise<ApiResponse<EditHistoryList>> => {
|
||||
const { id } = req.params as { id: string };
|
||||
const { session, view } = sessionOf(id, req);
|
||||
const why = unsupportedReason(view);
|
||||
if (why) return { success: true, data: unsupportedList(why) };
|
||||
const status = await sync.sync(view, { older: true });
|
||||
return { success: true, data: listOf(await sync.getStore(session.id), session.workingDir, status) };
|
||||
});
|
||||
|
||||
app.get('/api/sessions/:id/edits/:entryId', async (req, reply): Promise<ApiResponse<EditHistoryDiff>> => {
|
||||
const { id, entryId } = req.params as { id: string; entryId: string };
|
||||
const { session } = sessionOf(id, req);
|
||||
const entry = (await sync.getStore(session.id)).get(entryId);
|
||||
const { session, view } = sessionOf(id, req);
|
||||
const entry = unsupportedReason(view) ? undefined : (await sync.getStore(session.id)).get(entryId);
|
||||
if (!entry) {
|
||||
reply.code(404);
|
||||
return createErrorResponse(ApiErrorCode.NOT_FOUND, 'No such edit in this session');
|
||||
|
||||
Reference in new issue
Block a user