fix(edit-history): review round 3 — restored sessions read again, /clear reads the old conversation to its end, bounded memory

- remove() drops the session's slot once the file is gone: a session that comes
  back with the same id (reboot restore) starts afresh instead of answering
  `removed` until the next restart. A run in flight stays on its own slot's store,
  and the routes start a sync in the same tick as their session check, so a
  deletion right after it still stops that sync. A store that is not loaded is
  deleted without being parsed (deleteStore).
- A new conversation (/clear, resume) first reads the previous transcript, next
  to the new one, to its end with its subagents, so edits it got after the last
  read are not lost with its cursors. "Load older" reads the stored conversation
  while the switch is pending.
- At most MAX_LOADED_STORES (3) stores stay in memory; the least recently used one
  without a run is written if it changed (settle) and loads again on demand.
- Tmp files are named per process start, so a reused pid (a container's pid 1)
  no longer keeps a stale one through the startup sweep.
- load() keeps only a plain transcript id (the previous transcript's path is built
  from it) and no cursors without one; a pending Bash call with an id, cwd or
  agent id over 4,096 characters is not kept at parse time either.
- nextRange() is gone: production reads forward through the reader. The store
  tests mirror the reader's range choice, and a new end-to-end test runs the
  real reader, sync and store (/clear cases, 40 seeded random transcripts).
- The wiring guard scans all of src for holders of the sync.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
RandalixandClaude Opus 5.5 committed 2026-10-23 15:35:12 +02:00
1 parent 321268754a
commit 1c87cd2ac7
13 files changed
+581 -90

No files matched your search

+3
View File
@@ -34,6 +34,8 @@ export const MAX_PENDING_BASH = 200;
* this bounds it; an image path past this point of a command is still found in the output.
*/
export const MAX_PENDING_COMMAND_CHARS = 8192;
/** Longest id, cwd or agent id of an unanswered Bash call; a call with a longer one is not kept. */
export const MAX_PENDING_FIELD_CHARS = 4096;
export type EditKind = 'code' | 'image';
/** `edit` = a result with a path and a patch from a tool other than Edit/Write. */
@@ -291,6 +293,7 @@ export function parseClaudeEditLines(text: string, pending: readonly PendingBash
if (typeof block.id !== 'string' || !isRecord(block.input)) continue;
const command = block.input.command;
if (typeof command !== 'string') continue;
if ([block.id, cwd, agentId ?? ''].some((f) => f.length > MAX_PENDING_FIELD_CHARS)) continue;
const entry: PendingBash = { id: block.id, ts, command: command.slice(0, MAX_PENDING_COMMAND_CHARS), cwd };
if (agentId) entry.agentId = agentId;
open.delete(block.id);
+44 -32
View File
@@ -6,11 +6,11 @@
* (`subagents/agent-<id>.jsonl`). Each has a cursor:
* - `offset` — the next byte to read forward; always a line boundary.
* - `floor` — the lowest byte already parsed; > 0 while older lines are unread
* ("ältere laden" reads `[floor − budget, floor)`).
* The store decides WHICH byte range to read (`nextRange`/`olderRange`); the caller reads
* it — locally or over ssh — and hands the bytes to `apply`. Cuts happen only at `\n`,
* which never occurs inside a UTF-8 sequence, so multibyte characters split by a chunk
* boundary are never decoded half.
* ("ältere laden" reads `[floor − budget, floor)`, see `olderRange`).
* The reader (`edit-history-local-source.ts`) reads forward from each `offset` of the
* current conversation, a file without one from `budget` bytes before its end, and hands the
* bytes to `apply`. Cuts happen only at `\n`, which never occurs inside a UTF-8 sequence, so
* multibyte characters split by a chunk boundary are never decoded half.
*
* A first read of a file starts `budget` bytes before its end (transcripts reach 100 MB+),
* drops the cut first line and leaves `floor` there. A line longer than the budget is
@@ -23,8 +23,8 @@
* A new conversation (`/clear`, resume) arrives as a new `transcriptId`: cursors and
* pending Bash calls start over, entries stay. Entries are capped at {@link MAX_ENTRIES}
* and {@link MAX_STORE_BYTES} of diff text; the oldest by time are dropped. Besides them
* the file holds the cursors, at most {@link MAX_PENDING_BASH} unanswered Bash calls and at
* most {@link MAX_AGENTS} subagent labels.
* the file holds one cursor per transcript file of the current conversation, at most
* {@link MAX_PENDING_BASH} unanswered Bash calls and at most {@link MAX_AGENTS} subagent labels.
*
* ⚠️ The file holds secrets: a diff carries whatever the agent wrote, keys and `.env`
* contents included. So, like `intents.json`, it is written 0600 via tmp+rename, bounded,
@@ -40,6 +40,7 @@
* @module edit-history-store
*/
import { randomBytes } from 'node:crypto';
import { mkdir, readdir, readFile, rename, stat, unlink, writeFile } from 'node:fs/promises';
import { join } from 'node:path';
import { dataPath } from './config/instance.js';
@@ -48,6 +49,7 @@ import {
MAX_HUNK_BYTES,
MAX_PENDING_BASH,
MAX_PENDING_COMMAND_CHARS,
MAX_PENDING_FIELD_CHARS,
parseClaudeEditLines,
type EditRecord,
type EditTool,
@@ -72,12 +74,17 @@ export const READ_BUDGET_BYTES = 8 * 1024 * 1024;
* above stays far below it; JSON escaping at most doubles ordinary text.
*/
export const MAX_FILE_BYTES = 32 * 1024 * 1024;
/** Longest id, cwd or agent id of a loaded pending Bash call; a longer one is no real one. */
const MAX_LOADED_FIELD_CHARS = 4096;
export const MAIN_SOURCE = 'main';
const SAVE_DEBOUNCE_MS = 1000;
const FILE_VERSION = 1;
const NL = 0x0a;
/** A transcript id as the reader takes it (`<id>.jsonl`): no path separators. */
const TRANSCRIPT_ID = /^[A-Za-z0-9._-]+$/;
/**
* Names this process's tmp files (`<id>.json.<tag>.tmp`); any other tmp file is left over from
* an earlier run (`sweep`), even one whose pid this process has again (a container's pid 1).
*/
export const TMP_TAG = `${process.pid}-${randomBytes(4).toString('hex')}`;
/** `bash`: an image a Bash call wrote; `sent`: an image claude delivered to the user. */
export type EditHistoryTool = EditTool | 'bash' | 'sent';
@@ -198,7 +205,7 @@ function cursorFrom(raw: unknown): SourceCursor | null {
return cursor;
}
const loadedField = (v: unknown): v is string => typeof v === 'string' && v.length <= MAX_LOADED_FIELD_CHARS;
const loadedField = (v: unknown): v is string => typeof v === 'string' && v.length <= MAX_PENDING_FIELD_CHARS;
function pendingBashFrom(raw: unknown): PendingBash | null {
if (!isObj(raw)) return null;
@@ -279,8 +286,12 @@ export class EditHistoryStore {
if (!isObj(data)) return store;
if (data.version !== FILE_VERSION) return store;
const f = data as unknown as StoreFile;
store.transcriptId = typeof f.transcriptId === 'string' ? f.transcriptId : null;
for (const [k, raw] of Object.entries(isObj(f.cursors) ? f.cursors : {})) {
// The sync builds the previous transcript's path from it (`/clear`), so only a plain id counts.
store.transcriptId =
typeof f.transcriptId === 'string' && TRANSCRIPT_ID.test(f.transcriptId) ? f.transcriptId : null;
// Cursors are offsets into that conversation's files; without it they mean nothing.
const cursors = store.transcriptId !== null && isObj(f.cursors) ? f.cursors : {};
for (const [k, raw] of Object.entries(cursors)) {
const cursor = cursorFrom(raw);
if (cursor) store.cursors.set(k, cursor);
}
@@ -319,9 +330,9 @@ export class EditHistoryStore {
}
let removed = 0;
for (const name of names) {
const tmp = /^(.+)\.json\.(\d+)\.tmp$/.exec(name);
const tmp = /\.json\.[\w-]+\.tmp$/.test(name);
const stale = tmp
? Number(tmp[2]) !== process.pid
? !name.endsWith(`.json.${TMP_TAG}.tmp`)
: name.endsWith('.json') && !keep(name.slice(0, -'.json'.length));
if (!stale) continue;
await unlink(join(dir, name)).catch(() => undefined);
@@ -330,6 +341,11 @@ export class EditHistoryStore {
return removed;
}
/** Delete the stored history of `sessionId` without loading it. */
static async delete(sessionId: string, opts: Pick<EditHistoryStoreOptions, 'dir'> = {}): Promise<void> {
await new EditHistoryStore(sessionId, opts).remove();
}
getTranscriptId(): string | null {
return this.transcriptId;
}
@@ -375,21 +391,6 @@ export class EditHistoryStore {
this.scheduleSave();
}
/**
* The next forward range to read from `source`, or null when nothing is new.
* A conversation other than the stored one starts from scratch (see `apply`).
*/
nextRange(source: string, size: number, transcriptId: string, budget = READ_BUDGET_BYTES): ByteRange | null {
const cursor = transcriptId === this.transcriptId ? this.cursors.get(source) : undefined;
if (cursor && size >= cursor.offset) {
if (size === cursor.offset) return null;
return { offset: cursor.offset, len: Math.min(budget, size - cursor.offset) };
}
// Unknown file, new conversation, or a file that shrank (replaced): read its end.
const offset = Math.max(0, size - budget);
return size > offset ? { offset, len: size - offset } : null;
}
/** The next older range of `source` ("ältere laden"), or null when it is read to its start. */
olderRange(source: string, budget = READ_BUDGET_BYTES): ByteRange | null {
const cursor = this.cursors.get(source);
@@ -418,8 +419,10 @@ export class EditHistoryStore {
}
/**
* Apply a forward chunk read at `nextRange`: a full budget unless it reaches the file's end.
* Returns new entries and Bash image candidates.
* Apply a forward chunk: read from the source's cursor, or, without one, its last `budget`
* bytes; a full budget unless it reaches the file's end. A file that shrank below its cursor
* (replaced) loses the cursor and is read from its end next time. Returns new entries and
* Bash image candidates.
*/
apply(input: ApplyInput): ApplyResult {
if (this.removed) return { added: 0, candidates: [] };
@@ -546,6 +549,15 @@ export class EditHistoryStore {
return added;
}
/**
* Write pending changes, if any, and wait for a write under way (the store is dropped from
* memory and loaded again later). Unlike `flush()`, a store without changes writes nothing.
*/
async settle(): Promise<void> {
if (this.saveTimer) return this.flush();
await this.writing;
}
/** Write now (cancels a pending debounced write). Writes run one after another. */
async flush(): Promise<void> {
if (this.saveTimer) {
@@ -593,7 +605,7 @@ export class EditHistoryStore {
await mkdir(dir, { recursive: true, mode: 0o700 });
// `mode` applies only to a file being created: the fresh tmp file is 0600, and the rename
// makes it the store, so an existing store never keeps wider bits.
const tmp = `${this.file}.${process.pid}.tmp`;
const tmp = `${this.file}.${TMP_TAG}.tmp`;
await writeFile(tmp, JSON.stringify(data), { mode: 0o600 });
if (this.removed) {
await unlink(tmp).catch(() => undefined);
+114 -29
View File
@@ -13,9 +13,18 @@
* for a line longer than the budget) but read in the next pass; up to
* {@link MAX_FORWARD_PASSES} passes per sync, the rest with the next one.
*
* GONE. `remove()` (the session's record is deleted) marks the session gone before it deletes
* the file: a run in flight stops after its current read, its store no longer writes, and a
* later request reads nothing (`unavailable`, reason `removed`) instead of loading a new store.
* NEW CONVERSATION. When the transcript path names another conversation (`/clear`, resume),
* the previous transcript (same directory) is first read to its end, so edits it got after
* the last read are not lost with its cursors; then the new one is read from its end.
* "Load older" reaches back within the current conversation only.
*
* MEMORY. At most {@link MAX_LOADED_STORES} stores stay loaded; past that the least recently
* used one without a run is written (if it changed) and dropped, and loaded again on demand.
*
* GONE. `remove()` (the session's record is deleted) marks the session gone, deletes the file
* and then drops the session's slot: a run in flight stops after its current read and its
* store no longer writes; a request during the removal reads nothing (`unavailable`, reason
* `removed`). A session that comes back with the same id (reboot restore) starts afresh.
* `sweep()` deletes, at startup, the stored history of sessions that no longer exist. A
* subagent transcript that vanished loses its cursor (`dropSource`), not its entries.
*
@@ -33,6 +42,7 @@
* @module web/edit-history-sync
*/
import { basename, dirname, join } from 'node:path';
import type { SessionRemote } from '../types/session.js';
import type { EditSourceReadOptions, EditSourceRequest, EditSources, PathProbe } from '../edit-history-local-source.js';
import type { ImageCandidate } from '../edit-history-parse.js';
@@ -45,6 +55,8 @@ export const IMAGE_MTIME_SLACK_MS = 2000;
export const MAX_FORWARD_PASSES = 3;
/** Sources read back per "Load older" click, a full budget each. */
export const MAX_OLDER_SOURCES = 3;
/** Stores kept in memory; the least recently used one past this is dropped. */
export const MAX_LOADED_STORES = 3;
/** `reason` when the session has no transcript yet (no prompt yet, or not found). */
export const NO_TRANSCRIPT = 'no-transcript';
/** `reason` once the session's history was deleted. */
@@ -79,6 +91,8 @@ export interface EditHistorySyncDeps {
): Promise<EditSources | null>;
probe(paths: readonly string[]): Promise<Array<PathProbe | null>>;
loadStore(sessionId: string): Promise<EditHistoryStore>;
/** Delete a session's stored history without loading it. */
deleteStore(sessionId: string): Promise<void>;
/** Delete the stored history of every session `keep` rejects; returns how many. */
sweepStores(keep: (sessionId: string) => boolean): Promise<number>;
budget?: number;
@@ -151,7 +165,10 @@ interface SessionSlot {
}
export class EditHistorySync {
/** By session, least recently used first. */
private readonly slots = new Map<string, SessionSlot>();
/** Writes of dropped stores; loading that session again waits for its write. */
private readonly unloading = new Map<string, Promise<void>>();
private readonly budget: number;
constructor(private readonly deps: EditHistorySyncDeps) {
@@ -160,15 +177,7 @@ export class EditHistorySync {
/** The session's store, loaded from disk on first use (one load for concurrent callers). */
getStore(sessionId: string): Promise<EditHistoryStore> {
const slot = this.slot(sessionId);
if (!slot.store) {
const loading = this.deps.loadStore(sessionId);
slot.store = loading;
loading.catch(() => {
if (slot.store === loading) slot.store = undefined;
});
}
return slot.store;
return this.storeOf(sessionId, this.slot(sessionId));
}
getStatus(sessionId: string): EditHistoryStatus {
@@ -182,10 +191,10 @@ export class EditHistorySync {
async sync(session: EditHistorySession, opts: SyncOptions = {}): Promise<EditHistoryStatus> {
const why = unsupportedReason(session);
const slot = this.slot(session.id);
if (slot.gone) return this.getStatus(session.id);
if (slot.gone) return { ...slot.status };
if (why) {
slot.status = { state: 'unsupported', reason: why };
return this.getStatus(session.id);
return { ...slot.status };
}
const want: SyncWant = { forward: !opts.older, older: !!opts.older };
if (slot.running) {
@@ -196,33 +205,48 @@ export class EditHistorySync {
await slot.running;
// The queued run starts when the current one ends; wait for it too.
if (slot.running) await slot.running;
return this.getStatus(session.id);
return { ...slot.status };
}
slot.running = this.runLoop(session, slot, want);
try {
await slot.running;
} finally {
slot.running = undefined;
this.unloadIdle(slot);
}
return this.getStatus(session.id);
return { ...slot.status };
}
/** Write every loaded store's pending changes (server shutdown). */
async flushAll(): Promise<void> {
await Promise.all([...this.slots.values()].map(async (slot) => (await slot.store)?.flush()));
await Promise.all([
...[...this.slots.values()].map(async (slot) => (await slot.store?.catch(() => undefined))?.settle()),
...this.unloading.values(),
]);
}
/**
* Delete a session's stored history (its record is gone). The session stays marked, so a
* run in flight or a late request cannot load and write a new store for it.
* Delete a session's stored history (its record is gone). While it runs the session is
* marked, so a run in flight or a request meanwhile cannot load and write a new store for
* it; then its slot goes, and a session that comes back with this id starts afresh.
*/
async remove(sessionId: string): Promise<void> {
const slot = this.slot(sessionId);
slot.gone = true;
slot.again = undefined;
slot.status = { state: 'unavailable', reason: REMOVED };
const store = await this.getStore(sessionId);
await store.remove();
try {
const store = await slot.store?.catch(() => undefined);
if (store) {
await store.remove();
} else {
await this.unloading.get(sessionId);
await this.deps.deleteStore(sessionId);
}
await slot.running;
} finally {
if (this.slots.get(sessionId) === slot) this.slots.delete(sessionId);
}
}
/**
@@ -233,15 +257,53 @@ export class EditHistorySync {
return this.deps.sweepStores((id) => this.slots.has(id) || exists(id));
}
/** The session's slot, now the most recently used. */
private slot(sessionId: string): SessionSlot {
let slot = this.slots.get(sessionId);
if (!slot) {
slot = { status: { state: 'ok' } };
this.slots.set(sessionId, slot);
}
const slot = this.slots.get(sessionId) ?? { status: { state: 'ok' } };
this.slots.delete(sessionId);
this.slots.set(sessionId, slot);
return slot;
}
private storeOf(sessionId: string, slot: SessionSlot): Promise<EditHistoryStore> {
if (!slot.store) {
const loading = (async () => {
await this.unloading.get(sessionId);
return this.deps.loadStore(sessionId);
})();
slot.store = loading;
loading.catch(() => {
if (slot.store === loading) slot.store = undefined;
});
this.unloadIdle(slot);
}
return slot.store;
}
/**
* Past {@link MAX_LOADED_STORES}, drop the least recently used stores no run uses: each is
* written if it changed, and a later request loads it again (after that write).
*/
private unloadIdle(keep?: SessionSlot): void {
let loaded = 0;
for (const slot of this.slots.values()) if (slot.store) loaded++;
for (const [sessionId, slot] of this.slots) {
if (loaded <= MAX_LOADED_STORES) break;
if (!slot.store || slot.running || slot.gone || slot === keep) continue;
this.slots.delete(sessionId);
loaded--;
const writing: Promise<void> = slot.store
.then((store) => store.settle())
.catch((err) => {
console.warn(`[EditHistory] Saving the history of ${sessionId} failed:`, err);
})
.finally(() => {
if (this.unloading.get(sessionId) === writing) this.unloading.delete(sessionId);
});
this.unloading.set(sessionId, writing);
}
}
private async runLoop(session: EditHistorySession, slot: SessionSlot, want: SyncWant): Promise<void> {
let next: SyncWant | undefined = want;
while (next && !slot.gone) {
@@ -253,9 +315,10 @@ export class EditHistorySync {
}
private async runOnce(session: EditHistorySession, slot: SessionSlot, older: boolean): Promise<void> {
const store = await this.getStore(session.id);
let status: EditHistoryStatus;
try {
// This slot's store: a run still in flight after `remove()` cannot reach a later one.
const store = await this.storeOf(session.id, slot);
const path = slot.gone ? null : await this.deps.transcriptPath(session);
let found: boolean;
if (!path) {
@@ -289,11 +352,31 @@ export class EditHistorySync {
/**
* One forward read of all sources. `more` = some file has unread bytes after it, or was cut
* short by the shared budget and left for the next pass. A transcript other than the stored
* one (`/clear`, resume) is read like a first read.
* one (`/clear`, resume) is read like a first read, once the stored one is read to its end.
*/
private async readForward(path: string, store: EditHistoryStore): Promise<{ found: boolean; more: boolean }> {
const own = this.storedTranscript(path, store);
if (own !== path && store.knownSources().length > 0) {
const previous = await this.readFrom(own, store, this.forwardRequests(store));
if (previous.more) return { found: true, more: true };
}
return this.readFrom(path, store, own === path ? this.forwardRequests(store) : []);
}
/**
* The path of the conversation the store holds cursors for: `path` itself, or the previous
* conversation's transcript next to it (`/clear` and resume keep the project directory).
*/
private storedTranscript(path: string, store: EditHistoryStore): string {
const known = store.getTranscriptId();
const requests = known !== null && path.endsWith(`/${known}.jsonl`) ? this.forwardRequests(store) : [];
return known === null || basename(path) === `${known}.jsonl` ? path : join(dirname(path), `${known}.jsonl`);
}
private async readFrom(
path: string,
store: EditHistoryStore,
requests: EditSourceRequest[]
): Promise<{ found: boolean; more: boolean }> {
const res = await this.deps.readSources(path, requests, this.budget);
if (!res) return { found: false, more: false };
this.dropMissing(store, requests, res);
@@ -326,6 +409,8 @@ export class EditHistorySync {
private async readOlder(path: string, store: EditHistoryStore, slot: SessionSlot): Promise<boolean> {
if (!store.backfillTruncated) return true;
// Older lines belong to the stored conversation, even before a forward read moved on.
path = this.storedTranscript(path, store);
const sources = store.sourcesWithOlder().sort();
if (sources.length === 0) return true;
// Take turns: start after the last source the previous click read.
+9 -4
View File
@@ -23,6 +23,7 @@ import {
unsupportedReason,
type EditHistorySession,
type EditHistoryState,
type EditHistoryStatus,
type EditHistorySync,
} from '../edit-history-sync.js';
@@ -104,10 +105,14 @@ export function registerEditHistoryRoutes(app: FastifyInstance, ctx: SessionPort
const { sync: doSync } = req.query as { sync?: string };
const { session, view } = sessionOf(id, req);
const why = unsupportedReason(view);
const store = await sync.getStore(session.id);
if (why) return { success: true, data: listOf(store, session.workingDir, { state: 'unsupported', reason: why }) };
const status = doSync === '1' ? await sync.sync(view) : sync.getStatus(session.id);
return { success: true, data: listOf(store, session.workingDir, status) };
// 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 = why
? { state: 'unsupported', reason: why }
: doSync === '1'
? await sync.sync(view)
: sync.getStatus(session.id);
return { success: true, data: listOf(await sync.getStore(session.id), session.workingDir, status) };
});
app.post('/api/sessions/:id/edits/older', async (req): Promise<ApiResponse<EditHistoryList>> => {
+1
View File
@@ -348,6 +348,7 @@ export class WebServer extends EventEmitter {
readSources: readClaudeEditSources,
probe: probeLocalPaths,
loadStore: (sessionId) => EditHistoryStore.load(sessionId),
deleteStore: (sessionId) => EditHistoryStore.delete(sessionId),
sweepStores: (keep) => EditHistoryStore.sweep(keep),
});
private respawnControllers: Map<string, RespawnController> = new Map();