fix(workspace): the findings of #340's review (#272)

changelog.d/272-walk-cap.fixed.md

@@ -1,5 +1,7 @@
-- A workspace mount with more files than one walk takes is now covered by repeated syncs and scans (#272). Each mount sets its cap with `walkCap`, a positive integer no larger than `Number.MAX_SAFE_INTEGER` that defaults to 5000; any other value is refused when the module is created, naming the mount. A bare `sync`, a `sync` of a directory, an agent-action scan, or a watched mount's catch-up scan (at start and when its watcher reattaches) that reaches the cap lists what it didn't reach under `incomplete`, and the next one of the same scope resumes after the last entry it took, then wraps to the start. The place is kept with the mount's disk evidence, so a restart resumes there too. Before, every walk stopped at the same 5000 files, so the files past the cap were never scanned, and a change to one reached the workspace only through a `sync` of its path. A listing (`ls`, `glob`, `grep`) still takes up to the cap from its scope's start.
+- A workspace mount with more files than one walk takes is now covered by repeated syncs (#272). Each mount sets its cap with `walkCap`, a positive integer no larger than `Number.MAX_SAFE_INTEGER` that defaults to 5000; any other value is refused when the module is created, naming the mount. A bare `sync` or a `sync` of a directory that reaches the cap lists what it didn't reach under `incomplete`, and the next one of the same scope resumes after the last entry it took, then wraps to the start. A watched mount's catch-up scan (at start and when its watcher reattaches) takes its turn in the same rotation as the mount's bare syncs. The place is kept with the mount's disk evidence, so a restart resumes there too, unless the machine went down before the store made the latest place durable, which only starts the next walk at an earlier place. Before, every walk stopped at the same 5000 files, so no walk ever reached the files past them. An agent-action scan (`watch: 'on-agent-action'`) still takes up to the cap from the mount's start after every tool batch, and a listing (`ls`, `glob`, `grep`) still takes up to the cap from its scope's start. Both now take entries in the walk's name order, so on a mount over the cap they can reach different files than 0.22's first 5000, which followed the order the platform lists a directory in.
   - A walk takes each directory's entries that aren't directories by name, then its subdirectories by name, so each walk can resume where the last stopped, whatever order the platform lists a directory in. A link, or another entry that isn't a directory, counts against the cap as a file does, since the pass reads each one it takes, through the link on a mount that follows them.
   - Each directory's listing is read whole, so a file deleted on disk is seen wherever the walk lists, past the cap included. Before, a directory holding more files than the cap never proved a deletion.
-  - The `sync` tool's description now says that a sync takes up to the mount's walk cap and that the next sync of the same mount or directory resumes where it stopped, so the first request after the upgrade that carries the workspace's tools misses the prompt cache from its tool block on.
+  - Because a watched mount's catch-up scan takes its turn in the rotation, on a mount over the cap a change made while the process was down reaches the workspace when the rotation comes to it, or when a sync of its path takes it. Before, the catch-up scan at start took a change among the first 5000 files every time, and never one past them.
+  - The `sync` tool's description now says that a sync takes up to the mount's walk cap, and that the next sync of the same mount or directory picks up where the last turn stopped, with a watched mount's catch-up scan taking turns with its whole-mount syncs. So the first request after the upgrade that carries the workspace's tools misses the prompt cache from its tool block on.
+  - On a mount over 5000 entries, the first syncs and catch-up scans after the upgrade catch up on what 0.22's walks never reached: files past the old cap, never scanned, and edits and deletions of tracked files there. Each such pass pushes one event for each kind of change it finds (`workspace:created`, `workspace:modified` or `workspace:deleted`), each listing every path, and a mount whose `wakeOnChange` admits that kind turns each into one message to the agent. Any pass that lists a directory larger than the cap, a listing or an agent-action scan included, also proves deletions there that 0.22 never proved.
   - Going back to 0.22: its walks stop at the same 5000 files as before. It leaves the place alone, so coming forward again resumes there, unless 0.22 wrote a checkpoint meanwhile, which drops every place and starts each scope at its top.

src/modules/workspace/disk-agreement.ts

@@ -15,7 +15,9 @@
  * that isn't that directory any more (an unmounted drive whose mountpoint is
  * now empty, a replaced directory) is an unavailable mount, never a disk on
  * which every file was deleted; only an explicit acceptance takes it, and
- * that starts its evidence afresh.
+ * that starts its evidence afresh. A place, where a walk stopped (`walks`),
+ * binds no root: it only says where the next walk starts, in whatever
+ * directory the root is by then.
  *
  * Every transition is write-ahead (see reconcile.ts): an `intent` entry
  * (`pending`, with the prior value and the expected one) is durable before the
@@ -137,9 +139,10 @@ export class DiskAgreement {
   private readonly mounts = new Map<string, MountEvidence>();
   private configured: Array<{ name: string; root: string }> = [];
   /**
-   * An append (or a rebuild) not yet made durable. Cleared only by a
-   * barrier that succeeds, so a failed barrier stays owed: a later pass with
-   * nothing new to record still settles it before deciding anything.
+   * An append of evidence (or a rebuild) not yet made durable. Cleared only
+   * by a barrier that succeeds, so a failed barrier stays owed: a later pass
+   * with nothing new to record still settles it before deciding anything. A
+   * place owes none (recordWalk).
    */
   private unsynced = false;
 
@@ -279,6 +282,8 @@ export class DiskAgreement {
    * that's lost costs coverage, never a decision: an append that didn't land
    * loses one, and a checkpoint written by a version that doesn't keep them
    * (0.22) loses them all, and each scope's walks start at its top again.
+   * So recording one owes no barrier: a pass that moves only its place makes
+   * no store sync, and the place becomes durable with the next one.
    */
   recordWalk(mount: string, scope: string, after: string | null): void {
     if ((this.walkResumesAfter(mount, scope) ?? null) === after) return;
@@ -319,7 +324,9 @@ export class DiskAgreement {
 
   private write(entry: JournalEntry, opts: { durable?: boolean; afterCommittedState?: boolean }): void {
     this.ensureReconciled();
-    this.unsynced = true; // until proven durable: a failed append may still have landed
+    // Evidence is owed a barrier until proven durable: a failed append may
+    // still have landed. A place isn't (recordWalk).
+    if (entry.t !== 'walk') this.unsynced = true;
     this.journal.append(entry, opts);
     if (opts.durable) this.unsynced = false; // synced after this append, and so everything before it
     const mount = this.mounts.get((entry as { mount: string }).mount);
@@ -366,8 +373,8 @@ function apply(mount: MountEvidence, entry: JournalEntry): void {
 function applyRoot(mount: MountEvidence, entry: Extract<JournalEntry, { t: 'root' }>): void {
   mount.identity = entry.identity;
   if (entry.reset) {
-    // Evidence about another directory, and where its walks stopped, say
-    // nothing about this one.
+    // Evidence about another directory says nothing about this one, and the
+    // accepted root's walks start at its top.
     mount.paths.clear();
     mount.walks.clear();
   }

src/modules/workspace/index.ts

@@ -791,6 +791,7 @@ export class WorkspaceModule implements Module {
       intents: this.intents.get(mount.config.name)!,
       readOnly: mount.config.mode === 'read-only',
       walkCap: mount.config.walkCap ?? WALK_CAP,
+      catchUpScan: (mount.config.watch ?? 'always') === 'always',
     };
   }
 
@@ -994,9 +995,12 @@ export class WorkspaceModule implements Module {
         this.watchers.set(name, watcher);
 
         // Chokidar is started with ignoreInitial:true, so changes made while
-        // the session was down would be invisible. One full pass catches up
-        // under the three-way rule: only real differences produce tree
-        // changes and events.
+        // the session was down would be invisible. A catch-up pass takes them
+        // in under the three-way rule: only real differences produce tree
+        // changes and events. It takes its turn with the mount's bare syncs,
+        // so on a mount over its walkCap it covers the walkCap entries after
+        // where the last turn stopped, and a change elsewhere waits for the
+        // rotation to reach it, or for a sync of its path.
         void this.initialScan(name);
       }
 
@@ -1224,8 +1228,10 @@ export class WorkspaceModule implements Module {
         name: 'sync',
         description: 'Bring disk changes into the workspace. Without a path, every mount (or the given one) is '
           + 'rechecked and nothing pending in the workspace is discarded. A sync takes up to the mount\'s walk '
-          + 'cap of entries, files, links and the like (5000 unless configured); past it, what this one didn\'t reach is listed under '
-          + '`incomplete`, and the next sync of the same mount or directory resumes where it stopped. With a '
+          + 'cap of entries, files, links and the like (5000 unless configured). Past it, what this one didn\'t reach is listed under '
+          + '`incomplete`, and the walk takes turns: the next sync of the same mount or directory picks up where '
+          + 'the last turn stopped, so repeated syncs cover it all. On a mount watched continuously, its own catch-up '
+          + 'scan, at start and when its watcher reattaches, takes turns with its whole-mount syncs. With a '
           + 'path (file or directory), '
           + 'the workspace takes disk\'s state there explicitly: it restores a workspace-deleted file, resolves '
           + 'conflicts toward disk, replaces a workspace draft, and removes a workspace file disk does not have. '
@@ -3004,10 +3010,14 @@ export class WorkspaceModule implements Module {
    * After an agent's completed tool batch, before anything continues its turn:
    * scan every `watch: 'on-agent-action'` mount, so what the batch's tools did
    * on disk (a shell command's files, a deletion) is in the workspace when the
-   * agent next looks. A scan that outlasts the deadline stops holding the
-   * round: the miss is recorded on the mount's status and pushed as a
-   * `workspace:agent-action-scan-incomplete` event, and the scan continues,
-   * deciding on the branch selected when it applies.
+   * agent next looks. On a mount over its walkCap, that holds for the first
+   * walkCap entries in the walk's order, which every batch's scan takes; files
+   * past them come in through syncs and the listings that reach them, while a
+   * read takes in only a file the workspace doesn't hold yet. A scan that
+   * outlasts the deadline stops holding the round: the miss is recorded on
+   * the mount's status and pushed as a `workspace:agent-action-scan-incomplete`
+   * event, and the scan continues, deciding on the branch selected when it
+   * applies.
    */
   async onToolBatchComplete(_agentName?: string): Promise<void> {
     // Every agent's workspace is the store's current branch, so the agent
@@ -3073,7 +3083,11 @@ export class WorkspaceModule implements Module {
     const scan: AgentActionScan = { late: false, settled: Promise.resolve() };
     const pass = this.withMount(mount, () => {
       if (mount.agentActionScanWaiting === scan) mount.agentActionScanWaiting = undefined;
-      return this.passUnlocked(mount, { kind: 'dir', dir: '', recursive: true }, { resume: true });
+      // From the mount's start, as a listing walks, keeping no place: on a
+      // mount over its walkCap, every batch's scan takes the first walkCap
+      // entries in the walk's order, and the place that syncs and catch-up
+      // scans resume from stays theirs.
+      return this.passUnlocked(mount, { kind: 'dir', dir: '', recursive: true });
     });
     scan.settled = pass.then(
       (result) => {

src/modules/workspace/observe.ts

@@ -1059,7 +1059,11 @@ export async function walkScope(
     }
   }
   // The cap stopped the walk only if it left something: a walk whose last
-  // file used up the cap covered its scope.
+  // file used up the cap covered its scope. A directory the walk didn't list
+  // counts as left, even one that holds nothing it would take (empty, or
+  // only ignored names): telling them apart would take another read. Such a
+  // walk keeps its place and reports the directory as not visited, and the
+  // next one lists it on its way to the wrap.
   if (left === 0 && lastTaken !== undefined && (holdingUntaken.size > 0 || unlisted > 0)) walk.stoppedAfter = lastTaken;
   return walk;
 }

src/modules/workspace/reconcile.ts

@@ -154,6 +154,12 @@ export interface MountRuntime {
   readOnly: boolean;
   /** The most entries one walk of the mount takes, files and the other entries that aren't directories (`MountConfig.walkCap`). */
   walkCap: number;
+  /**
+   * Whether the mount runs a catch-up scan (`watch: 'always'`, at start and
+   * when its watcher reattaches), which takes turns with its whole-mount
+   * syncs.
+   */
+  catchUpScan: boolean;
 }
 
 export type Scope =
@@ -168,10 +174,11 @@ export interface PassOptions {
   /** Walk cap for directory scopes; the mount's `walkCap` when absent. */
   cap?: number;
   /**
-   * A recursive directory pass of a sync or scan: its walk resumes after the
-   * last entry the previous one of this scope took, and records where it
-   * stopped, so repeated passes cover a scope larger than the cap. A
-   * listing's pass starts at its scope's start.
+   * A recursive directory pass of a sync or a catch-up scan: its walk resumes
+   * after the last entry the previous one of this scope took, and records
+   * where it stopped, so repeated passes cover a scope larger than the cap.
+   * A listing's pass, and an agent-action scan's, start at their scope's
+   * start and record no place.
    */
   resume?: boolean;
   /**
@@ -818,12 +825,12 @@ export async function reconcilePass(
   // Where the walk stopped, once what it observed is decided: the next pass
   // of this scope resumes after it, or starts afresh once this one covered
   // the scope. A walk that couldn't list its scope's own directory covered
-  // none of it, and leaves the place as it was. Like evidence, a place is
-  // recorded after the root it was gathered under.
+  // none of it, and leaves the place as it was. Unlike evidence, a place
+  // binds no root: it only says where the next walk starts, and that walk
+  // takes its turn after it in whatever directory is the root by then.
   let resumeAfter: string | null | undefined;
   if (opts.resume && walk && scope.kind === 'dir' && scope.recursive && walk.complete.has(scope.dir)) {
     resumeAfter = walk.stoppedAfter ?? null;
-    if (gathered.rootIdentity) agreement.recordRootIdentity(mount.name, gathered.rootIdentity);
     agreement.recordWalk(mount.name, scope.dir, resumeAfter);
   }
 
@@ -851,9 +858,16 @@ export async function reconcilePass(
   if (walk) {
     const tracked = [...result.reports.values()].filter((r) => r.state === 'unverified').map((r) => r.path);
     if (typeof resumeAfter === 'string' && scope.kind === 'dir') {
+      // A watched mount's catch-up scan takes turns with its whole-mount
+      // syncs; nothing else moves a place.
+      const next = scope.dir !== ''
+        ? 'the next sync of this directory'
+        : mount.catchUpScan
+          ? 'the next sync of the mount, or its catch-up scan if that comes first,'
+          : 'the next sync of the mount';
       result.incomplete.push({
         path: scope.dir,
-        reason: `the walk cap (${opts.cap ?? mount.walkCap}) was reached: the next sync or scan of this scope resumes after ${mount.name}/${resumeAfter}`,
+        reason: `the walk cap (${opts.cap ?? mount.walkCap}) was reached: ${next} resumes after ${mount.name}/${resumeAfter}`,
       });
     }
     for (const region of walk.incomplete) {

src/modules/workspace/types.ts

@@ -22,7 +22,9 @@ export interface MountConfig {
    * - 'on-agent-action': after each completed agent tool batch, the mount is
    *   scanned, and the agent's next inference waits for that scan up to
    *   `agentActionScanDeadlineMs`; a scan that takes longer finishes while
-   *   the round goes on
+   *   the round goes on. Each scan walks from the mount's start, so on a
+   *   mount over its `walkCap` it covers the first `walkCap` entries every
+   *   time, and files past them come in as `walkCap` describes
    * - 'never': no background reads; listings and lazy reads still observe
    *   the paths they show
    */
@@ -46,13 +48,23 @@ export interface MountConfig {
    * The most entries one walk of this mount takes (default: 5000), a
    * positive integer no larger than Number.MAX_SAFE_INTEGER: files, and
    * links or other entries that aren't
-   * directories, each of which the pass observes. A sync or scan that
-   * reaches it reports the rest as incomplete, and the next sync or scan of
-   * the same scope resumes after the last entry it took, wrapping to the
-   * start, so repeated ones cover a larger mount; a restart keeps the place.
-   * A listing (`ls`, `glob`, `grep`) takes up to it from its scope's start.
-   * Each directory's listing is read whole, so a deletion is seen wherever
-   * the walk lists, whatever it took.
+   * directories, each of which the pass observes. A sync that reaches it
+   * reports the rest as incomplete, and the next sync of the same scope
+   * resumes after the last entry it took, wrapping to the start, so repeated
+   * syncs cover a larger mount. A watched mount's catch-up scan (at start,
+   * and when its watcher reattaches) takes its turn in the same rotation as
+   * the mount's bare syncs, so on a mount over the cap, a change made while
+   * the process was down waits for the rotation to reach it. The place is
+   * kept with the mount's disk evidence, so a restart resumes there, unless
+   * the machine went down before the store made the latest place durable.
+   * An agent-action scan (`watch: 'on-agent-action'`) and a listing (`ls`,
+   * `glob`, `grep`) take up to it from their scope's start every time. Each
+   * directory's listing is read whole, so a deletion is seen wherever the
+   * walk lists, whatever it took.
+   *
+   * It bounds what a walk observes, not the walk's own work: directories
+   * don't count against it, and each one the walk visits is read whole, so
+   * a tree of many directories that hold nothing is walked in full.
    */
   walkCap?: number;
   /**