fix(edit-history): review round 1 — record-bound deletion, load-time bounds, older reads per source

- The history is deleted when the session's persisted record goes (forgetEditHistory):
  kill via cleanupSession (a pinned record is only demoted and keeps it), the
  persisted-only DELETE, and stale cleanup at startup. After remove() neither a sync in
  flight nor a later request writes the file again; writes are serialized.
- load() re-bounds a damaged file: cursors, pending Bash calls (MAX_PENDING_BASH) and
  subagent labels (MAX_AGENTS, oldest unused dropped first).
- Load older reads one block of every transcript, budget split between them, and only
  the files it asked for; a block short of budget reads its end. A line longer than the
  budget is skipped going back as well (floorMidLine).
- Concurrent first callers share one store; a forward request folded with an older one
  still reads forward.
- Images outside the workspace get no thumbnail (one attachment id per render); a click
  opens the shared preview, which registers once with notify: false.
- Docs: per-edit diff cap, deletion rule, load-time bounds, thumbnails, follow triggers.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/web/edit-history-sync.ts

@@ -5,8 +5,15 @@
  *
  * WHEN. Only when the panel asks (`GET …/edits?sync=1` on open and while it follows the
  * session, `POST …/edits/older`): no timer, no hook, no background read for sessions nobody
- * looks at. One sync per session runs at a time; a request during a run is folded into one
- * more run afterwards.
+ * looks at. One sync per session runs at a time; requests during a run are folded into one
+ * more forward read and/or one more older read afterwards.
+ *
+ * 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.
+ *
+ * OLDER. "Load older" splits the budget evenly over the sources with older lines and reads
+ * only those blocks, so every source moves back with each click.
  *
  * WHICH SESSIONS. Local claude sessions. Remote, Docker and non-claude sessions answer
  * `unsupported` and are never read.
@@ -20,7 +27,7 @@
  */
 
 import type { SessionRemote } from '../types/session.js';
-import type { EditSourceRequest, EditSources, PathProbe } from '../edit-history-local-source.js';
+import type { EditSourceReadOptions, EditSourceRequest, EditSources, PathProbe } from '../edit-history-local-source.js';
 import type { ImageCandidate } from '../edit-history-parse.js';
 import { EditHistoryStore, READ_BUDGET_BYTES, type ConfirmedImage } from '../edit-history-store.js';
 import { getCli } from '../config/cli-registry/registry.js';
@@ -31,6 +38,8 @@ export const IMAGE_MTIME_SLACK_MS = 2000;
 export const MAX_FORWARD_PASSES = 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. */
+export const REMOVED = 'removed';
 
 export type EditHistoryState = 'ok' | 'unavailable' | 'unsupported';
 
@@ -53,7 +62,12 @@ export interface EditHistorySession {
 export interface EditHistorySyncDeps {
   /** The session's main transcript path, or null when it has none yet. */
   transcriptPath(session: EditHistorySession): Promise<string | null>;
-  readSources(mainPath: string, requests: readonly EditSourceRequest[], budget: number): Promise<EditSources | null>;
+  readSources(
+    mainPath: string,
+    requests: readonly EditSourceRequest[],
+    budget: number,
+    opts?: EditSourceReadOptions
+  ): Promise<EditSources | null>;
   probe(paths: readonly string[]): Promise<Array<PathProbe | null>>;
   loadStore(sessionId: string): Promise<EditHistoryStore>;
   budget?: number;
@@ -109,11 +123,18 @@ export function confirmImages(
   return out;
 }
 
+interface SyncWant {
+  forward: boolean;
+  older: boolean;
+}
+
 interface SessionSlot {
-  store?: EditHistoryStore;
+  store?: Promise<EditHistoryStore>;
   status: EditHistoryStatus;
   running?: Promise<void>;
-  again?: { older: boolean };
+  again?: SyncWant;
+  /** The history was deleted: nothing reads for this session any more. */
+  gone?: boolean;
 }
 
 export class EditHistorySync {
@@ -124,10 +145,16 @@ export class EditHistorySync {
     this.budget = deps.budget ?? READ_BUDGET_BYTES;
   }
 
-  /** The session's store, loaded from disk on first use. */
-  async getStore(sessionId: string): Promise<EditHistoryStore> {
+  /** 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) slot.store = await this.deps.loadStore(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;
   }
 
@@ -142,18 +169,23 @@ 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 (why) {
       slot.status = { state: 'unsupported', reason: why };
       return this.getStatus(session.id);
     }
+    const want: SyncWant = { forward: !opts.older, older: !!opts.older };
     if (slot.running) {
-      slot.again = { older: (slot.again?.older ?? false) || !!opts.older };
+      slot.again = {
+        forward: (slot.again?.forward ?? false) || want.forward,
+        older: (slot.again?.older ?? false) || want.older,
+      };
       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);
     }
-    slot.running = this.runLoop(session, !!opts.older);
+    slot.running = this.runLoop(session, slot, want);
     try {
       await slot.running;
     } finally {
@@ -164,13 +196,19 @@ export class EditHistorySync {
 
   /** Write every loaded store's pending changes (server shutdown). */
   async flushAll(): Promise<void> {
-    await Promise.all([...this.slots.values()].map((slot) => slot.store?.flush()));
+    await Promise.all([...this.slots.values()].map(async (slot) => (await slot.store)?.flush()));
   }
 
-  /** Drop a deleted session: forget it and delete its stored history. */
+  /**
+   * 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.
+   */
   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);
-    this.slots.delete(sessionId);
     await store.remove();
   }
 
@@ -183,21 +221,21 @@ export class EditHistorySync {
     return slot;
   }
 
-  private async runLoop(session: EditHistorySession, older: boolean): Promise<void> {
-    const slot = this.slot(session.id);
-    let next: boolean | null = older;
-    while (next !== null) {
-      await this.runOnce(session, next);
-      next = slot.again ? slot.again.older : null;
+  private async runLoop(session: EditHistorySession, slot: SessionSlot, want: SyncWant): Promise<void> {
+    let next: SyncWant | undefined = want;
+    while (next && !slot.gone) {
+      if (next.forward) await this.runOnce(session, slot, false);
+      if (next.older && !slot.gone) await this.runOnce(session, slot, true);
+      next = slot.again;
       slot.again = undefined;
     }
   }
 
-  private async runOnce(session: EditHistorySession, older: boolean): Promise<void> {
-    const slot = this.slot(session.id);
+  private async runOnce(session: EditHistorySession, slot: SessionSlot, older: boolean): Promise<void> {
     const store = await this.getStore(session.id);
+    let status: EditHistoryStatus;
     try {
-      const path = await this.deps.transcriptPath(session);
+      const path = slot.gone ? null : await this.deps.transcriptPath(session);
       let found: boolean;
       if (!path) {
         found = false;
@@ -205,16 +243,17 @@ export class EditHistorySync {
         found = await this.readOlder(path, store);
       } else {
         found = true;
-        for (let pass = 0; pass < MAX_FORWARD_PASSES && found; pass++) {
+        for (let pass = 0; pass < MAX_FORWARD_PASSES && found && !slot.gone; pass++) {
           const r = await this.readForward(path, store);
           found = r.found;
           if (!r.more) break;
         }
       }
-      slot.status = found ? { state: 'ok' } : { state: 'unavailable', reason: NO_TRANSCRIPT };
+      status = found ? { state: 'ok' } : { state: 'unavailable', reason: NO_TRANSCRIPT };
     } catch (err) {
-      slot.status = { state: 'unavailable', reason: err instanceof Error ? err.message : String(err) };
+      status = { state: 'unavailable', reason: err instanceof Error ? err.message : String(err) };
     }
+    if (!slot.gone) slot.status = status;
   }
 
   private forwardRequests(store: EditHistoryStore): EditSourceRequest[] {
@@ -257,13 +296,16 @@ export class EditHistorySync {
 
   private async readOlder(path: string, store: EditHistoryStore): Promise<boolean> {
     if (!store.backfillTruncated) return true;
+    const sources = store.sourcesWithOlder();
+    if (sources.length === 0) return true;
+    const share = Math.max(1, Math.floor(this.budget / sources.length));
     const requests: EditSourceRequest[] = [];
-    for (const relPath of store.sourcesWithOlder()) {
-      const range = store.olderRange(relPath, this.budget);
+    for (const relPath of sources) {
+      const range = store.olderRange(relPath, share);
       if (range) requests.push({ relPath, offset: range.offset, len: range.len });
     }
     if (requests.length === 0) return true;
-    const res = await this.deps.readSources(path, requests, this.budget);
+    const res = await this.deps.readSources(path, requests, this.budget, { onlyRequested: true });
     if (!res) return false;
     if (res.transcriptId !== store.getTranscriptId()) return true;
     const wanted = new Set(requests.map((r) => r.relPath));

src/web/ports/session-port.ts

@@ -19,6 +19,11 @@ export interface SessionPort {
    */
   addSession(session: Session, placement?: SessionPlacementHint): Promise<TabLayout>;
   cleanupSession(sessionId: string, killMux?: boolean, reason?: string): Promise<void>;
+  /**
+   * Delete a session's stored edit history once its record is gone. `cleanupSession` does it
+   * itself; this is for removing a record without a live session (DELETE route).
+   */
+  forgetEditHistory(sessionId: string): void;
   setupSessionListeners(session: Session): Promise<void>;
   persistSessionState(session: Session): void;
   persistSessionStateNow(session: Session): void;

src/web/public/edit-history-ui.js

@@ -13,11 +13,12 @@
  *
  * - A code entry opens its diff inline (`GET /api/sessions/:id/edits/:entryId`), drawn with the Git
  *   panel's diff classes (`git-diff`, `git-diff-line--hunk|add|del`).
- * - An image entry shows a thumbnail and opens the shared file preview (panels-ui.js
- *   `openFilePreview`), as does a click on any path. Both go through the existing file routes only:
- *   inside the workspace the confined `file-raw`, outside it an id minted by the attachment route
- *   with `notify: false` (`_registerExternalPreview`, which enforces the attachment guard and its
- *   extension allowlist). A path from the transcript is never served by a route of its own.
+ * - An image entry opens the shared file preview (panels-ui.js `openFilePreview`), as does a click
+ *   on any path; inside the workspace it also shows a thumbnail. Both go through the existing file
+ *   routes only: the thumbnail is the confined `file-raw`, and the preview of a file outside the
+ *   workspace registers it on click with `notify: false` (`_registerExternalPreview`, which
+ *   enforces the attachment guard and its extension allowlist). A path from the transcript is
+ *   never served by a route of its own.
  * - `backfillTruncated` offers "Load older" (`POST /api/sessions/:id/edits/older`).
  *
  * Paths, agent descriptions and diffs are untrusted text: they are only ever written with
@@ -26,7 +27,7 @@
  * @mixin Extends CodemanApp.prototype via Object.assign
  * @dependency app.js (this.activeSessionId, this.sessions, this.loadAppSettingsFromStorage, this.getDefaultSettings, this.$)
  * @dependency api-client.js (this._api, this._apiJson; resolved at call time)
- * @dependency panels-ui.js (openFilePreview, _isExternalPreviewPath, _registerExternalPreview)
+ * @dependency panels-ui.js (openFilePreview, _isExternalPreviewPath)
  * @dependency i18n.js (window.codemanT)
  * @loadorder 12.58 of 16, after git-status-ui.js, before entrance-animations.js
  */
@@ -302,32 +303,16 @@ Object.assign(CodemanApp.prototype, {
   },
 
   /**
-   * Thumbnail source for an image entry, through the routes the file preview uses: inside the
-   * workspace the confined `file-raw` route (a string), outside it the attachment `/raw` route by
-   * an id the attachment route mints with `notify: false` (a promise, one per path and panel
-   * state, resolving to the URL or null when the attachment guard refuses the file). SVGs get none
-   * (both routes serve them as a download); clicking still opens the preview.
+   * Thumbnail URL for an image entry inside the workspace: the confined `file-raw` route the file
+   * preview uses. An image outside the workspace gets none: showing it needs an attachment id
+   * (`POST …/attachments`, `notify: false`), and minting one per rendered row would push the
+   * session's real attachments out of its capped registry. Clicking it opens the file preview,
+   * which registers it like any other external file. SVGs get none either (`file-raw` serves
+   * them as a download).
    */
-  _editHistoryThumbSrc(path, sid, h) {
-    if (!path || /\.svg$/i.test(path)) return null;
-    if (!this._isExternalPreviewPath?.(path, sid)) {
-      return this._editHistoryUrl(`/api/sessions/${encodeURIComponent(sid)}/file-raw?path=${encodeURIComponent(path)}`);
-    }
-    if (typeof this._registerExternalPreview !== 'function') return null;
-    if (!h.thumbs) h.thumbs = new Map();
-    if (!h.thumbs.has(path)) {
-      h.thumbs.set(
-        path,
-        this._registerExternalPreview(path, sid).then((r) =>
-          r?.attachmentId
-            ? this._editHistoryUrl(
-                `/api/sessions/${encodeURIComponent(sid)}/attachments/${encodeURIComponent(r.attachmentId)}/raw`
-              )
-            : null
-        )
-      );
-    }
-    return h.thumbs.get(path);
+  _editHistoryThumbSrc(path, sid) {
+    if (!path || /\.svg$/i.test(path) || this._isExternalPreviewPath?.(path, sid) !== false) return null;
+    return this._editHistoryUrl(`/api/sessions/${encodeURIComponent(sid)}/file-raw?path=${encodeURIComponent(path)}`);
   },
 
   _renderEditHistoryPanel() {
@@ -528,7 +513,7 @@ Object.assign(CodemanApp.prototype, {
           open();
         }
       });
-      const src = this._editHistoryThumbSrc(e.path, h.sessionId, h);
+      const src = this._editHistoryThumbSrc(e.path, h.sessionId);
       if (src) {
         const img = el('img', 'edit-history-thumb');
         img.loading = 'lazy';
@@ -538,15 +523,7 @@ Object.assign(CodemanApp.prototype, {
           open();
         });
         img.addEventListener('error', () => img.remove());
-        if (typeof src === 'string') {
-          img.src = src;
-        } else {
-          // A re-render meanwhile replaced this element; the new one reads the same promise.
-          src.then((url) => {
-            if (url) img.src = url;
-            else img.remove();
-          });
-        }
+        img.src = src;
         out.push(img);
       }
       return out;

src/web/routes/session-routes.ts

@@ -1722,7 +1722,8 @@ export function registerSessionRoutes(
       // Called for its existence/ownership 404 side effect only — demoteOrRemoveSession
       // below re-looks-up the record by id, so the returned SessionState is unused here.
       findPersistedSessionOrFail(ctx.store, id, req);
-      ctx.store.demoteOrRemoveSession(id);
+      // The edit history goes with the record (same rule as `cleanupSession`).
+      if (ctx.store.demoteOrRemoveSession(id) === 'removed') ctx.forgetEditHistory(id);
       // Mirrors the broadcast at the tail of the live-session cleanup path
       // (_doCleanupSession in server.ts) — without it, other open tabs keep
       // showing the retired row until their next unrelated fetch.

src/web/server.ts

@@ -841,6 +841,7 @@ export class WebServer extends EventEmitter {
       addSession: this.registerSessionWithLayout.bind(this),
       tabLayouts: this.tabLayouts,
       cleanupSession: this.cleanupSession.bind(this),
+      forgetEditHistory: this.forgetEditHistory.bind(this),
       setupSessionListeners: this.setupSessionListeners.bind(this),
       persistSessionState: this.persistSessionState.bind(this),
       persistSessionStateNow: this._persistSessionStateNow.bind(this),
@@ -1576,13 +1577,6 @@ export class WebServer extends EventEmitter {
     // Stop transcript watcher
     this.stopTranscriptWatcher(sessionId);
 
-    // Edit history: a killed session takes its stored history (diffs can hold secrets) with it
-    if (killMux) {
-      void this.editHistorySync.remove(sessionId).catch((err) => {
-        console.warn(`[EditHistory] Removing history of ${sessionId} failed:`, getErrorMessage(err));
-      });
-    }
-
     // Stop and remove run summary tracker
     const summaryTracker = this.runSummaryTrackers.get(sessionId);
     if (summaryTracker) {
@@ -1703,7 +1697,8 @@ export class WebServer extends EventEmitter {
       // Only remove from state.json if we're also killing the mux session.
       // When killMux=false (server shutdown), preserve state for recovery.
       if (killMux) {
-        this.store.demoteOrRemoveSession(sessionId);
+        // The edit history goes with the record; a pinned session's demoted record keeps it.
+        if (this.store.demoteOrRemoveSession(sessionId) !== 'preserved') this.forgetEditHistory(sessionId);
       }
     }
 
@@ -2509,10 +2504,21 @@ export class WebServer extends EventEmitter {
     const lifecycleLog = getLifecycleLog();
     for (const s of result.cleaned) {
       lifecycleLog.log({ event: 'stale_cleaned', sessionId: s.id, name: s.name });
+      this.forgetEditHistory(s.id);
     }
     return result.count;
   }
 
+  /**
+   * Delete a session's edit history (diffs can hold secrets) once its record is gone: deleted
+   * live (`cleanupSession`), deleted as a record only (DELETE route), or cleaned as stale.
+   */
+  private forgetEditHistory(sessionId: string): void {
+    void this.editHistorySync.remove(sessionId).catch((err) => {
+      console.warn(`[EditHistory] Removing history of ${sessionId} failed:`, getErrorMessage(err));
+    });
+  }
+
   /**
    * Get lightweight state for SSE init - excludes full terminal buffers
    * to prevent browser freezes. Terminal buffers are fetched on-demand.