From 888e5dbe46fc459649a56e8db3d8d46337c07c57 Mon Sep 17 00:00:00 2001 From: Gladys-2413 Date: Wed, 14 Oct 2026 07:38:16 -0400 Subject: [PATCH 1/7] fix(workspace): a per-mount walk cap, and syncs and scans that resume past it (#272) A walk stopped at 5000 files, in the same order each time, so on a mount larger than that the same files were never scanned: a disk change past the cap reached the workspace only through a sync of its path (antra-tess's field report on #248: a 12,000-file mount, a text file appended to for two weeks, bare syncs reporting totalSynced: 0). Ada-1017's calls (room-203 #93219): a per-mount cap, no mount without one, resume for directory syncs too, the listing change below in scope, and the place kept across restarts. - MountConfig.walkCap: a positive integer, default 5000 (WALK_CAP). Any other value is refused when the WorkspaceModule is created, naming the mount. Every walk of the mount takes up to it. - The walk's order: each directory's files by name (UTF-16 code units), then its subdirectories by name, each whole (compareInWalk). The walk sorts each listing itself so it visits files in the order it compares positions by; Node's readdir returns UTF-8 byte order on Linux (checked on the VPS), which differs for names such as U+FF21 and U+1F600. - Resume: a recursive pass of a sync or scan (a bare sync, a directory sync, an agent-action scan, a watched mount's first scan) walks after the last file the previous one of its scope took, then wraps to the scope's start, so every pass takes up to a full cap's worth and repeated passes cover the scope. Where it stopped (Walk.stoppedAfter) is recorded once the pass has decided, keyed by (mount, scope directory). A listing's pass (ls, glob, grep) starts at its scope's start and records nothing. - The place is kept in the disk-agreement journal, with the evidence: a new { t: 'walk', mount, scope, after } entry, a walks map per mount, and the checkpoint's walks. An accepted new root, or a mount whose root changed, starts with none. The entry has no `path` field: 0.22 replays an entry it doesn't know as a forget of entry.path, and so leaves evidence alone (checked against the published 0.22.1, below). The replay now applies only the kinds it knows (apply()). - Each directory's listing is read whole, past the cap included, and every name recorded: files the walk didn't take go to Walk.seen (present, unobserved this pass), and provenAbsent proves a name the listing lacks absent whichever files were taken. Without this, a resumed walk whose boundaries repeat (10,000 files, cap 5000) would never take a straddling directory whole, and a deletion there would never be proven. Once the cap is reached, no further directory is listed. - A capped pass reports, under `incomplete`, each directory holding files it didn't take and each directory it didn't list, with "the file cap (N) was reached", and for a resuming pass "; the next sync or scan here resumes after /". - The sync tool's description says a sync takes up to the mount's walk cap and the next resumes where it stopped (a one-time prompt-cache miss on upgrade; the changelog says so). Tests (workspace-three-way, 176): "a capped walk deletes nothing it did not reach" now asserts what the whole listing makes true: c.txt, missing from the root's listing, is deleted; d.txt, past the cap, stays unverified and kept. New, under "a mount larger than its walk cap": three bare syncs at walkCap 2 take five files written out of order, in the walk's order, the third wrapping to the start and taking a disk edit there; eight files written scrambled are taken three at a time by name; names that sort apart by UTF-8 byte and UTF-16 code unit are each taken once; a disk edit past the cap reaches the workspace on a later sync; a deletion in a directory larger than the cap is proven on the first sync; a listing takes the same files every time and records no place; a directory sync resumes for its own scope; the place survives a restart; walkCap values 0, -1, 1.5, Infinity, NaN and '5000' are refused. On the VPS: tsc clean; workspace-three-way 176/176 on Node 24.21.0 and 20.20.2, and every workspace test file passes on both. Removals (rel272/removals-404e5ba.txt, and the sort and record removals rerun at this tree): the sort, the whole-listing completeness, the seen check in provenAbsent, the wrap, the stretch's strict "after", the walk's stopping place, the cap regions for untaken files, reading the place, recording it, the mount's walkCap, replaying the place at open, the walkCap validation, and the bare sync's and the directory sync's resume: each, removed, fails the tests aimed at it. Going back (rel272/probe.mjs): the published 0.22.1's DiskAgreement replays a journal holding two walk entries and keeps both paths' evidence; this code then finds the evidence and both places. --- changelog.d/272-walk-cap.fixed.md | 5 + src/modules/workspace/disk-agreement.ts | 73 +++++++-- src/modules/workspace/index.ts | 21 ++- src/modules/workspace/observe.ts | 207 ++++++++++++++++++++---- src/modules/workspace/reconcile.ts | 27 +++- src/modules/workspace/types.ts | 10 ++ test/workspace-three-way.test.ts | 130 ++++++++++++++- 7 files changed, 413 insertions(+), 60 deletions(-) create mode 100644 changelog.d/272-walk-cap.fixed.md diff --git a/changelog.d/272-walk-cap.fixed.md b/changelog.d/272-walk-cap.fixed.md new file mode 100644 index 000000000..b2edcff76 --- /dev/null +++ b/changelog.d/272-walk-cap.fixed.md @@ -0,0 +1,5 @@ +- 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 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 first scan that reaches the cap lists what it didn't reach under `incomplete`, and the next one of the same scope resumes after the last file 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 5000 files in `readdir` order, 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 walk takes each directory's files by name, then its subdirectories by name, so the files past the cap are the same from one walk to the next. + - 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 says this, so the first request after the upgrade that carries the workspace's tools misses the prompt cache from its tool block on. + - Going back to 0.22: the place is kept in an entry 0.22 doesn't know and leaves alone, so its walks start at the top again. diff --git a/src/modules/workspace/disk-agreement.ts b/src/modules/workspace/disk-agreement.ts index ea9c7aceb..43471d1aa 100644 --- a/src/modules/workspace/disk-agreement.ts +++ b/src/modules/workspace/disk-agreement.ts @@ -92,16 +92,30 @@ type JournalEntry = /** The root's identity; with `reset`, an accepted new root, whose evidence starts empty. */ | { t: 'root'; mount: string; identity: RootIdentity; reset?: true } | { t: 'set'; mount: string; path: string; value: Physical } - | { t: 'forget'; mount: string; path: string }; + | { t: 'forget'; mount: string; path: string } + /** + * Where a capped walk of `scope` stopped (the last file it took), or null + * once a walk covered the scope. It carries no `path` field: agent-framework + * 0.22 replays an entry it doesn't know as a `forget` of `entry.path`, and + * so leaves the evidence alone when there is none. + */ + | { t: 'walk'; mount: string; scope: string; after: string | null }; interface MountEvidence { root: string; identity?: RootIdentity; paths: Map; + /** By scope ('' for the mount root): where the next walk of it resumes. */ + walks: Map; } interface Snapshot { - mounts: Record }>; + mounts: Record; + walks?: Record; + }>; } export const DISK_AGREEMENT_RECORD_TYPE = 'workspace/disk-agreement'; @@ -183,19 +197,19 @@ export class DiskAgreement { root: mount.root, ...(mount.identity ? { identity: mount.identity } : {}), paths: new Map(Object.entries(mount.paths)), + walks: new Map(Object.entries(mount.walks ?? {})), }); } for (const { entry } of entries) { if (entry.t === 'mount') { const existing = replayed.get(entry.mount); - if (!existing || !sameRootPath(existing.root, entry.root)) replayed.set(entry.mount, { root: entry.root, paths: new Map() }); + if (!existing || !sameRootPath(existing.root, entry.root)) { + replayed.set(entry.mount, { root: entry.root, paths: new Map(), walks: new Map() }); + } continue; } const mount = replayed.get(entry.mount); - if (!mount) continue; - if (entry.t === 'root') applyRoot(mount, entry); - else if (entry.t === 'set') mount.paths.set(entry.path, entry.value); - else mount.paths.delete(entry.path); + if (mount) apply(mount, entry); } this.mounts.clear(); for (const { name, root } of this.configured) { @@ -203,7 +217,7 @@ export class DiskAgreement { if (found && sameRootPath(found.root, root)) { this.mounts.set(name, found); } else { - this.mounts.set(name, { root, paths: new Map() }); + this.mounts.set(name, { root, paths: new Map(), walks: new Map() }); this.journal.append({ t: 'mount', mount: name, root }); } } @@ -250,6 +264,25 @@ export class DiskAgreement { return out; } + /** + * Where the next walk of `scope` ('' for the mount root) resumes: the last + * file a walk of it took before its file cap stopped it, if one did. + */ + walkResumesAfter(mount: string, scope: string): string | undefined { + this.ensureReconciled(); + return this.mounts.get(mount)?.walks.get(scope); + } + + /** + * Record where a walk of `scope` stopped (`after`), or that one covered it + * (null). Kept with the evidence so a restart resumes there too; a lost + * entry only costs the walk its place. + */ + recordWalk(mount: string, scope: string, after: string | null): void { + if ((this.walkResumesAfter(mount, scope) ?? null) === after) return; + this.write({ t: 'walk', mount, scope, after }, {}); + } + /** * Record an intent. `durable` makes it (and every append before it) stable * before the caller performs the effect; batches pass it on their last one. @@ -288,10 +321,7 @@ export class DiskAgreement { 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); - if (!mount) return; - if (entry.t === 'root') applyRoot(mount, entry); - else if (entry.t === 'set') mount.paths.set(entry.path, entry.value); - else if (entry.t === 'forget') mount.paths.delete(entry.path); + if (mount) apply(mount, entry); } /** Checkpoint when the tail has outgrown the live evidence. */ @@ -307,6 +337,7 @@ export class DiskAgreement { root: mount.root, ...(mount.identity ? { identity: mount.identity } : {}), paths: Object.fromEntries(mount.paths), + ...(mount.walks.size > 0 ? { walks: Object.fromEntries(mount.walks) } : {}), }; } this.journal.checkpoint(snapshot); @@ -319,7 +350,23 @@ export class DiskAgreement { } } +/** Apply one entry to a mount's evidence; one of a kind this version doesn't know changes nothing. */ +function apply(mount: MountEvidence, entry: JournalEntry): void { + if (entry.t === 'root') applyRoot(mount, entry); + else if (entry.t === 'set') mount.paths.set(entry.path, entry.value); + else if (entry.t === 'forget') mount.paths.delete(entry.path); + else if (entry.t === 'walk') { + if (entry.after === null) mount.walks.delete(entry.scope); + else mount.walks.set(entry.scope, entry.after); + } +} + function applyRoot(mount: MountEvidence, entry: Extract): void { mount.identity = entry.identity; - if (entry.reset) mount.paths.clear(); + if (entry.reset) { + // Evidence about another directory, and where its walks stopped, say + // nothing about this one. + mount.paths.clear(); + mount.walks.clear(); + } } diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index ab7c5692e..2ffba9fbc 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -50,6 +50,7 @@ import { type PushResult, type Scope, type TreeOp, + WALK_CAP, pushPaths, reconcilePass, settleAdoptionBeforeMutation, @@ -651,6 +652,12 @@ export class WorkspaceModule implements Module { private savedState: WorkspaceModuleState | null = null; constructor(config: WorkspaceConfig) { + for (const mount of config.mounts) { + const cap: unknown = mount.walkCap; + if (cap !== undefined && !(Number.isSafeInteger(cap) && (cap as number) > 0)) { + throw new Error(`[workspace] Mount "${mount.name}": walkCap must be a positive integer, not ${JSON.stringify(cap) ?? String(cap)}`); + } + } // Detect overlapping mount paths: if mount A contains mount B, // auto-add an ignore rule on A for B's path to prevent syncing // the sub-mount's directory through the super-mount. @@ -779,6 +786,7 @@ export class WorkspaceModule implements Module { treeStateId: mount.treeStateId, intents: this.intents.get(mount.config.name)!, readOnly: mount.config.mode === 'read-only', + walkCap: mount.config.walkCap ?? WALK_CAP, }; } @@ -1004,7 +1012,7 @@ export class WorkspaceModule implements Module { try { // A full pass under the three-way rule: deletions on disk are confirmed // where nothing newer is pending, and nothing unvisited is touched. - await this.withMount(mount, () => this.passUnlocked(mount, { kind: 'dir', dir: '', recursive: true })); + await this.withMount(mount, () => this.passUnlocked(mount, { kind: 'dir', dir: '', recursive: true }, { resume: true })); } catch (err) { const msg = err instanceof Error ? err.message : String(err); this.ctx?.pushEvent({ @@ -1211,7 +1219,9 @@ 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 in full and nothing pending in the workspace is discarded. With a path (file or directory), ' + + 'rechecked and nothing pending in the workspace is discarded. A sync takes up to the mount\'s walk ' + + 'cap of files (5000 unless configured); past it, what this one didn\'t reach is listed under ' + + '`incomplete`, and the next sync resumes where it stopped. 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. ' + 'Each workspace change it gives up is listed under `discarded`, with the state it had and, as `op`, ' @@ -2851,7 +2861,8 @@ export class WorkspaceModule implements Module { if (input.acceptRoot && await this.acceptRootUnlocked(mount)) rootsAccepted.push(name); if (!relativePath) { // A full recheck: every file rehashed, nothing pending discarded. - return this.passUnlocked(mount, { kind: 'dir', dir: '', recursive: true }, { rehash: true }); + // Past the mount's walkCap, it resumes where the last one stopped. + return this.passUnlocked(mount, { kind: 'dir', dir: '', recursive: true }, { rehash: true, resume: true }); } // An explicit path: the workspace takes disk's state there. const prefix = relativePath + '/'; @@ -2864,7 +2875,7 @@ export class WorkspaceModule implements Module { // beneath it (a directory scope's walk; a path scope doesn't walk). const opts: PassOptions = { rehash: true, adopt: () => true, branchId: askedOn, named: true }; if (!isDir) return this.passUnlocked(mount, { kind: 'paths', paths: [relativePath] }, opts); - const beneath = await this.passUnlocked(mount, { kind: 'dir', dir: relativePath, recursive: true }, opts); + const beneath = await this.passUnlocked(mount, { kind: 'dir', dir: relativePath, recursive: true }, { ...opts, resume: true }); // Something at the path itself is no part of a directory scope: a file // disk has where the workspace holds a directory, or a workspace file // where disk has a directory. Its own pass takes disk's state there @@ -3057,7 +3068,7 @@ 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 }); + return this.passUnlocked(mount, { kind: 'dir', dir: '', recursive: true }, { resume: true }); }); scan.settled = pass.then( (result) => { diff --git a/src/modules/workspace/observe.ts b/src/modules/workspace/observe.ts index 95776d90d..7d763eb58 100644 --- a/src/modules/workspace/observe.ts +++ b/src/modules/workspace/observe.ts @@ -672,13 +672,23 @@ export function sniffImageMime(head: Buffer): string | undefined { // =========================================================================== export interface Walk { - /** Regular files seen. */ + /** Regular files the walk took: the pass observes each one. */ files: Set; + /** + * Regular files a listing showed that the walk didn't take, past its file + * cap or outside the stretch a resumed walk covered: present, and not + * observed by this pass. + */ + seen: Set; /** Directories seen. */ dirs: Set; /** Names seen that are neither (symlinks, sockets…), or ignored: present, unproven. */ others: Map; - /** Directories listed completely. */ + /** + * Directories whose listing the walk read whole, and that were still the + * directory at their path once it had: every name in one is in `files`, + * `seen`, `dirs` or `others`, so a name in none of them is absent. + */ complete: Set; /** Directories that don't exist (everything beneath them is absent). */ missing: Set; @@ -690,6 +700,59 @@ export interface Walk { * never walked; the pass reports the link itself by path. */ incomplete: Array<{ path: string; reason: string; kind: 'error' | 'cap' | 'ignored' | 'symlink' }>; + /** + * When the file cap stopped the walk, the last file it took. A walk that + * resumes after it takes the files after it first, in the walk's order + * (`compareInWalk`), then wraps to the start of its scope. + */ + stoppedAfter?: string; +} + +/** Names compare by UTF-16 code unit, the same on every platform and filesystem. */ +function compareNames(a: string, b: string): number { + return a < b ? -1 : a > b ? 1 : 0; +} + +/** + * Where file `a` falls against file `b` in the walk's order: in each + * directory, its files by name, then its subdirectories by name, each with + * everything beneath it. A resumed walk relies on visiting files in the + * order it compares positions by, so the walk sorts each listing itself + * rather than take `readdir`'s order, which is the platform's (Node returns + * UTF-8 byte order on Linux, which differs from this one for some names). + */ +function compareInWalk(a: string, b: string): number { + const x = a.split('/'); + const y = b.split('/'); + for (let i = 0; ; i++) { + const lastX = i === x.length - 1; + const lastY = i === y.length - 1; + if (x[i] !== y[i]) { + if (lastX && lastY) return compareNames(x[i]!, y[i]!); + if (lastX) return -1; // a directory's files come before its subdirectories + if (lastY) return 1; + return compareNames(x[i]!, y[i]!); + } + if (lastX && lastY) return 0; + if (lastX) return -1; + if (lastY) return 1; + } +} + +/** + * Where the subtree of directory `dir` falls against file `at` in the walk's + * order: wholly before it, holding it (an ancestor of it), or wholly after it. + */ +function subtreeAgainst(dir: string, at: string): 'before' | 'holds' | 'after' { + if (dir === '') return 'holds'; + const d = dir.split('/'); + const p = at.split('/'); + for (let i = 0; i < d.length; i++) { + // `at` is a file in the directory above d[i], whose files come first. + if (i === p.length - 1) return 'after'; + if (d[i] !== p[i]) return compareNames(d[i]!, p[i]!) < 0 ? 'before' : 'after'; + } + return 'holds'; } /** @@ -740,17 +803,30 @@ async function checkChild(view: MountView, rootReal: string, parent: CheckedDir, /** * List a directory scope ('' for the mount root), descending when recursive, - * up to `cap` files in total. A scope that resolves outside the mount is not - * listed at all; beneath it, the walk only descends real directories, never - * symlinks. Each directory's listing counts only if, once listed, it is still - * the directory at its path: a directory moved or replaced meanwhile is an - * incomplete region, contributing no names and proving nothing absent. + * taking up to `cap` files in the walk's order (`compareInWalk`). A scope + * that resolves outside the mount is not listed at all; beneath it, the walk + * only descends real directories, never symlinks. Each directory's listing + * counts only if, once listed, it is still the directory at its path: a + * directory moved or replaced meanwhile is an incomplete region, contributing + * no names and proving nothing absent. + * + * Each listing is read whole and every name in it recorded, so the files the + * cap leaves are known present (`seen`) and a name the listing lacks is + * proven absent, whichever files were taken. Once the cap is reached, no + * further directory is listed. + * + * With `resumeAfter` (a file in the scope, from an earlier walk's + * `stoppedAfter`), a recursive walk takes the files after it first, then + * wraps to the start of the scope, so repeated walks cover a scope larger + * than the cap. */ export async function walkScope( view: MountView, rootReal: string, scope: string, recursive: boolean, cap: number, - opts: { named?: boolean } = {}, + opts: { named?: boolean; resumeAfter?: string } = {}, ): Promise { - const walk: Walk = { files: new Set(), dirs: new Set(), others: new Map(), complete: new Set(), missing: new Set(), incomplete: [] }; + const walk: Walk = { + files: new Set(), seen: new Set(), dirs: new Set(), others: new Map(), complete: new Set(), missing: new Set(), incomplete: [], + }; const top = await checkDir(view, rootReal, scope); if (top.kind === 'absent') { walk.missing.add(scope); @@ -760,20 +836,37 @@ export async function walkScope( walk.incomplete.push({ path: scope, reason: top.outside ? 'the directory resolves outside the mount' : top.reason, kind: 'error' }); return walk; } - let capped = false; // A scope a path sync names is walked even where the ignore list covers // it: a pattern covering the scope itself, or a directory above it, doesn't // apply beneath it. So a named directory's contents are synced, as a named // file is (agent-framework #276). A listing keeps the ignore: its pass // would otherwise start tracking what it lists. const ignore = opts.named ? view.ignore.filter((pattern) => !coveredByIgnore(scope, [pattern])) : view.ignore; + // A position outside the scope, or for a walk that doesn't descend, is no + // position in this walk's order. + const after = recursive && opts.resumeAfter !== undefined && (scope === '' || opts.resumeAfter.startsWith(scope + '/')) + ? opts.resumeAfter + : undefined; + + type Listing = { files: string[]; subdirs: string[] }; + /** Each directory is listed once: a resumed walk visits the ones holding its position on both sides of it. */ + const listings = new Map(); + const visited = new Set(); + /** Paths already reported as errors, so a second visit doesn't report them again. */ + const failed = new Set(); + const fail = (path: string, reason: string): void => { + if (failed.has(path)) return; + failed.add(path); + walk.incomplete.push({ path, reason, kind: 'error' }); + }; + let left = cap; + let lastTaken: string | undefined; - const visit = async (dir: CheckedDir): Promise => { + const list = async (dir: CheckedDir): Promise => { const rel = dir.rel; - if (capped) { - walk.incomplete.push({ path: rel, reason: `not visited: the file cap (${cap}) was reached`, kind: 'cap' }); - return; - } + const known = listings.get(rel); + if (known !== undefined) return known; + listings.set(rel, null); let entries; try { entries = await readdir(dir.real, { withFileTypes: true }); @@ -782,18 +875,18 @@ export async function walkScope( if (code === 'ENOENT' || code === 'ENOTDIR') { // Gone since it was checked: absent beneath only if that can be shown. const proof = rel === '' ? ROOT_UNAVAILABLE : await confirmAbsent(view, rootReal, rel, 'beneath'); - if (typeof proof === 'string') walk.incomplete.push({ path: rel, reason: proof, kind: 'error' }); + if (typeof proof === 'string') fail(rel, proof); else walk.missing.add(rel); - return; + return null; } - walk.incomplete.push({ path: rel, reason: `cannot list (${code ?? String(err)})`, kind: 'error' }); - return; + fail(rel, `cannot list (${code ?? String(err)})`); + return null; } + entries.sort((a, b) => compareNames(a.name, b.name)); const files: string[] = []; const subdirs: string[] = []; const others: Array<[string, string]> = []; const regions: Walk['incomplete'] = []; - let reachedCap = false; for (const entry of entries) { const childRel = rel ? `${rel}/${entry.name}` : entry.name; if (isIgnored(childRel, entry.name, ignore)) { @@ -804,12 +897,6 @@ export async function walkScope( if (entry.isDirectory()) { subdirs.push(childRel); } else if (entry.isFile()) { - if (walk.files.size + files.length >= cap) { - capped = true; - reachedCap = true; - regions.push({ path: rel, reason: `the file cap (${cap}) was reached`, kind: 'cap' }); - break; - } files.push(childRel); } else { const symlink = entry.isSymbolicLink(); @@ -820,25 +907,75 @@ export async function walkScope( // A listing of a directory that was moved or replaced meanwhile says // nothing about this path: neither its names nor its absences count. if (!(await dir.stillHere())) { - walk.incomplete.push({ path: rel, reason: CHANGED_WALK, kind: 'error' }); - return; + fail(rel, CHANGED_WALK); + return null; } - for (const f of files) walk.files.add(f); + for (const f of files) walk.seen.add(f); for (const d of subdirs) walk.dirs.add(d); for (const [p, why] of others) walk.others.set(p, why); walk.incomplete.push(...regions); - if (reachedCap) return; walk.complete.add(rel); + const listing = { files, subdirs }; + listings.set(rel, listing); + return listing; + }; + + type Stretch = 'all' | 'after' | 'upto'; + const fileIn = (stretch: Stretch, file: string): boolean => + stretch === 'all' || (compareInWalk(file, after!) > 0) === (stretch === 'after'); + const dirIn = (stretch: Stretch, dir: string): boolean => { + if (stretch === 'all') return true; + const at = subtreeAgainst(dir, after!); + return at === 'holds' || at === (stretch === 'after' ? 'after' : 'before'); + }; + + const visit = async (dir: CheckedDir, stretch: Stretch): Promise => { + visited.add(dir.rel); + const listing = await list(dir); + if (!listing) return; + for (const f of listing.files) { + if (left === 0) break; + if (!fileIn(stretch, f)) continue; + left--; + walk.seen.delete(f); + walk.files.add(f); + lastTaken = f; + } if (!recursive) return; - for (const sub of subdirs) { + for (const sub of listing.subdirs) { + // Once the cap is reached, no further directory is listed. + if (left === 0) break; + if (!dirIn(stretch, sub)) continue; const child = await checkChild(view, rootReal, dir, sub); - if (child.kind === 'checked') await visit(child.dir); + if (child.kind === 'checked') await visit(child.dir, stretch); else if (child.kind === 'absent') walk.missing.add(sub); - else walk.incomplete.push({ path: sub, reason: child.reason, kind: 'error' }); + else fail(sub, child.reason); } }; - await visit(top.dir); + if (after === undefined) { + await visit(top.dir, 'all'); + } else { + await visit(top.dir, 'after'); + if (left > 0) await visit(top.dir, 'upto'); + } + if (left === 0 && lastTaken !== undefined) walk.stoppedAfter = lastTaken; + + // What the cap left: directories holding files not taken, and directories + // a listing showed that weren't listed. A walk that doesn't descend leaves + // its subdirectories by design. + const capped = `the file cap (${cap}) was reached`; + const holdingUntaken = new Set(); + for (const f of walk.seen) holdingUntaken.add(parentOf(f)); + for (const d of holdingUntaken) { + walk.incomplete.push({ path: d, reason: `${capped} before every file here was taken`, kind: 'cap' }); + } + if (recursive) { + for (const d of walk.dirs) { + if (visited.has(d) || walk.missing.has(d) || failed.has(d)) continue; + walk.incomplete.push({ path: d, reason: `not visited: ${capped}`, kind: 'cap' }); + } + } return walk; } @@ -855,7 +992,7 @@ export function provenAbsent(walk: Walk, path: string): boolean { if (walk.missing.has(dir)) return true; const child = dir ? `${dir}/${parts[i]}` : parts[i]!; if (walk.complete.has(dir)) { - if (walk.others.has(child)) return false; // present, but unproven + if (walk.others.has(child) || walk.seen.has(child)) return false; // present, but unproven if (i === parts.length - 1) return !walk.files.has(child) && !walk.dirs.has(child); if (!walk.dirs.has(child)) return true; // no directory there: nothing beneath can exist } diff --git a/src/modules/workspace/reconcile.ts b/src/modules/workspace/reconcile.ts index 08082a1ec..227e221d7 100644 --- a/src/modules/workspace/reconcile.ts +++ b/src/modules/workspace/reconcile.ts @@ -152,6 +152,8 @@ export interface MountRuntime { treeStateId: string; intents: BranchIntents; readOnly: boolean; + /** The most files one walk of the mount takes (`MountConfig.walkCap`). */ + walkCap: number; } export type Scope = @@ -163,8 +165,15 @@ export interface PassOptions { rehash?: boolean; /** Paths whose disk state is adopted explicitly (a path sync). */ adopt?: (path: string) => boolean; - /** Walk cap for directory scopes. */ + /** 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 file 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. + */ + resume?: boolean; /** * The scope was named by a path sync: ignore patterns that cover it don't * apply beneath it (see walkScope). A listing's scope keeps them. @@ -618,7 +627,11 @@ async function gather(store: JsStore, agreement: DiskAgreement, mount: MountRunt for (const [p] of agreement.paths(mount.name, scope.dir)) if (inScope(p, scope)) candidates.add(p); for (const [p] of mount.intents.list(scope.dir)) if (inScope(p, scope)) candidates.add(p); if (rootUnavailable === null) { - walk = await walkScope(mount.view, mount.rootReal, scope.dir, scope.recursive, opts.cap ?? WALK_CAP, { named: opts.named }); + const resumeAfter = opts.resume ? agreement.walkResumesAfter(mount.name, scope.dir) : undefined; + walk = await walkScope(mount.view, mount.rootReal, scope.dir, scope.recursive, opts.cap ?? mount.walkCap, { + named: opts.named, + ...(resumeAfter !== undefined ? { resumeAfter } : {}), + }); for (const f of walk.files) if (inScope(f, scope)) candidates.add(f); // Ignored names are listed only when something tracks them (above). for (const [p, why] of walk.others) if (why !== 'ignored by the mount' && inScope(p, scope)) candidates.add(p); @@ -793,6 +806,12 @@ export async function reconcilePass( agreement.set(mount.name, x.path, next, { afterCommittedState: first }); first = false; } + // 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. + const resumeAfter = opts.resume && walk && scope.kind === 'dir' && scope.recursive ? walk.stoppedAfter ?? null : undefined; + if (resumeAfter !== undefined && scope.kind === 'dir') agreement.recordWalk(mount.name, scope.dir, resumeAfter); + // 5. Durable before returning. An agreement advance (D = S) has no pending // intent to recover from, so losing its append would leave an older P that // another branch would misread as a disk edit. @@ -816,9 +835,11 @@ export async function reconcilePass( } if (walk) { const tracked = [...result.reports.values()].filter((r) => r.state === 'unverified').map((r) => r.path); + const resumes = typeof resumeAfter === 'string' ? `; the next sync or scan here resumes after ${mount.name}/${resumeAfter}` : ''; for (const region of walk.incomplete) { const hides = tracked.some((p) => p === region.path || p.startsWith(region.path + '/')); - if (region.kind === 'error' || region.kind === 'cap' || hides) result.incomplete.push({ path: region.path, reason: region.reason }); + if (region.kind === 'cap') result.incomplete.push({ path: region.path, reason: region.reason + resumes }); + else if (region.kind === 'error' || hides) result.incomplete.push({ path: region.path, reason: region.reason }); // An ignored name that hides nothing tracked is reported nowhere else: // counted, so a sync that says 0 also says what it didn't look at // (agent-framework #276). A symlink region isn't counted: the link diff --git a/src/modules/workspace/types.ts b/src/modules/workspace/types.ts index 3517a8b96..a909144ce 100644 --- a/src/modules/workspace/types.ts +++ b/src/modules/workspace/types.ts @@ -42,6 +42,16 @@ export interface MountConfig { followSymlinks?: boolean; /** Maximum file size in bytes (default: 5MB) */ maxFileSize?: number; + /** + * The most files one walk of this mount takes (default: 5000), a positive + * integer. 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 file + * 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 files it took. + */ + walkCap?: number; /** * If set, filesystem changes on this mount request inference. * - `true` — wake on any op (created | modified | deleted) diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index 85082063f..9eb798196 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -2032,7 +2032,7 @@ describe('incomplete walks', () => { assert.notEqual(env.store.treeGet(TREE, 'cache/x.txt'), null); }); - test('a capped walk deletes nothing it did not reach', async (t) => { + test('a capped walk proves absent what a listing lacks, and leaves unverified what it did not take', async (t) => { const env = new Env(t); const m = await env.open(); for (const f of ['a.txt', 'b.txt', 'c.txt', 'd.txt']) await seedSynced(env, m, f, f); @@ -2040,9 +2040,131 @@ describe('incomplete walks', () => { const mount = (m as any).mounts.get('work'); const pass = await (m as any).withMount(mount, () => (m as any).passUnlocked(mount, { kind: 'dir', dir: '', recursive: true }, { cap: 2 })); - assert.equal(pass.reports.get('c.txt')?.state, 'unverified'); - assert.ok(pass.incomplete.some((r: { reason: string }) => /file cap/.test(r.reason))); - assert.notEqual(env.store.treeGet(TREE, 'c.txt'), null); + // The root's listing, read whole, has no c.txt: a deletion is seen + // wherever the walk lists, whichever files it took. + assert.equal(env.store.treeGet(TREE, 'c.txt'), null); + // d.txt is there, past the cap: unverified by this pass, and kept. + assert.equal(pass.reports.get('d.txt')?.state, 'unverified'); + assert.notEqual(env.store.treeGet(TREE, 'd.txt'), null); + assert.ok(pass.incomplete.some((r: { reason: string }) => /file cap \(2\) was reached/.test(r.reason))); + }); +}); + +describe('a mount larger than its walk cap (agent-framework #272)', () => { + const syncedNames = (env: Env) => env.store.treeList(TREE).map((e) => e.path).sort(); + + test('repeated bare syncs resume where the last stopped, so together they take the whole mount', async (t) => { + const env = new Env(t); + // Written out of order: the walk takes each directory's files by name, + // then its subdirectories by name. + for (const f of ['b/y.txt', 'z.txt', 'a.txt', 'b/x.txt', 'm.txt']) env.writeDisk(f, f); + const m = await env.open({ walkCap: 2 }); + + const first = await call(m, 'sync', {}); + assert.deepEqual(syncedNames(env), ['a.txt', 'm.txt'], 'the first two in the walk order'); + assert.ok(first.incomplete.some((r: { reason: string }) => /file cap \(2\) was reached.*resumes after work\/m\.txt/.test(r.reason)), + JSON.stringify(first.incomplete)); + + await call(m, 'sync', {}); + assert.deepEqual(syncedNames(env), ['a.txt', 'b/x.txt', 'm.txt', 'z.txt'], 'the next two, after m.txt'); + + // The last one, then the walk wraps to the start for the second: the + // mount is covered, and a.txt is looked at again. + env.writeDisk('a.txt', 'edited before the wrap'); + await call(m, 'sync', {}); + assert.deepEqual(syncedNames(env), ['a.txt', 'b/x.txt', 'b/y.txt', 'm.txt', 'z.txt']); + assert.equal(await contentOf(m, 'a.txt'), 'edited before the wrap', 'the third sync wrapped to a.txt'); + }); + + test('the walk takes files by name, whatever order they were written in', async (t) => { + const env = new Env(t); + // Neither the order written nor its reverse starts with the three + // smallest names, so a walk taking them in either order would differ. + for (const f of ['g.txt', 'c.txt', 'h.txt', 'a.txt', 'f.txt', 'b.txt', 'e.txt', 'd.txt']) env.writeDisk(f, f); + const m = await env.open({ walkCap: 3 }); + await call(m, 'sync', {}); + assert.deepEqual(syncedNames(env), ['a.txt', 'b.txt', 'c.txt']); + await call(m, 'sync', {}); + assert.deepEqual(syncedNames(env), ['a.txt', 'b.txt', 'c.txt', 'd.txt', 'e.txt', 'f.txt']); + }); + + test('names that sort apart by UTF-8 byte and by UTF-16 code unit are each taken once: the walk visits files in the order it resumes by', async (t) => { + const env = new Env(t); + // U+FF21 comes before U+1F600 by UTF-8 byte, the order Node's readdir + // returns on Linux, and after it by UTF-16 code unit, the walk's order. + for (const f of ['b.txt', '\uFF21.txt', '\u{1F600}.txt']) env.writeDisk(f, f); + const m = await env.open({ walkCap: 1 }); + const taken: string[] = []; + for (let i = 0; i < 3; i++) { + const before = new Set(syncedNames(env)); + await call(m, 'sync', {}); + taken.push(...syncedNames(env).filter((n) => !before.has(n))); + } + assert.deepEqual(taken, ['b.txt', '\u{1F600}.txt', '\uFF21.txt']); + }); + + test('a disk edit past the cap reaches the workspace on a later bare sync', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 2 }); + for (const f of ['a.txt', 'b.txt', 'c.txt']) await seedSynced(env, m, f, f); + env.writeDisk('c.txt', 'edited past the cap'); + await call(m, 'sync', {}); + await call(m, 'sync', {}); + assert.equal(await contentOf(m, 'c.txt'), 'edited past the cap'); + }); + + test('a deletion in a directory larger than the cap is proven by a listing read whole', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 2 }); + for (const f of ['big/a.txt', 'big/b.txt', 'big/c.txt', 'big/d.txt', 'big/e.txt']) await seedSynced(env, m, f, f); + env.rmDisk('big/e.txt'); + // The first bare sync takes a.txt and b.txt; big's listing lacks e.txt. + await call(m, 'sync', {}); + assert.equal(env.store.treeGet(TREE, 'big/e.txt'), null); + assert.notEqual(env.store.treeGet(TREE, 'big/d.txt'), null); + }); + + test('a listing starts at its scope every time, and leaves no place behind', async (t) => { + const env = new Env(t); + for (const f of ['a.txt', 'b.txt', 'c.txt']) env.writeDisk(f, f); + const m = await env.open({ walkCap: 2 }); + const listed = async () => [...(await listing(m)).keys()].sort(); + assert.deepEqual(await listed(), ['a.txt', 'b.txt']); + assert.deepEqual(await listed(), ['a.txt', 'b.txt'], 'the same files again'); + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', ''), undefined); + }); + + test('a directory sync resumes for its own scope', async (t) => { + const env = new Env(t); + for (const f of ['d/a.txt', 'd/b.txt', 'd/c.txt', 'top.txt']) env.writeDisk(f, f); + const m = await env.open({ walkCap: 2 }); + await call(m, 'sync', { path: 'work/d' }); + assert.deepEqual(syncedNames(env), ['d/a.txt', 'd/b.txt']); + await call(m, 'sync', { path: 'work/d' }); + assert.deepEqual(syncedNames(env), ['d/a.txt', 'd/b.txt', 'd/c.txt']); + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', ''), undefined, 'the mount root keeps its own place'); + }); + + test('the place survives a restart: the next bare sync resumes after it', async (t) => { + const env = new Env(t); + for (const f of ['a.txt', 'b.txt', 'c.txt', 'd.txt']) env.writeDisk(f, f); + let m = await env.open({ walkCap: 2 }); + await call(m, 'sync', {}); + assert.deepEqual(syncedNames(env), ['a.txt', 'b.txt']); + m = await env.restart(m, { walkCap: 2 }); + await call(m, 'sync', {}); + assert.deepEqual(syncedNames(env), ['a.txt', 'b.txt', 'c.txt', 'd.txt'], 'c.txt and d.txt, not a.txt and b.txt again'); + }); + + test('walkCap must be a positive integer', () => { + for (const walkCap of [0, -1, 1.5, Number.POSITIVE_INFINITY, Number.NaN, '5000']) { + assert.throws( + () => new WorkspaceModule({ mounts: [{ name: 'work', path: '/tmp', mode: 'read-write', walkCap: walkCap as number }] }), + /Mount "work": walkCap must be a positive integer/, + `walkCap ${String(walkCap)}`, + ); + } + assert.doesNotThrow(() => new WorkspaceModule({ mounts: [{ name: 'work', path: '/tmp', mode: 'read-write', walkCap: 12_000 }] })); }); }); From 968a48f14ea2e6802bd214bd60c122efddf7a4cf Mon Sep 17 00:00:00 2001 From: Gladys-2413 Date: Wed, 14 Oct 2026 08:44:32 -0400 Subject: [PATCH 2/7] fix(workspace): #272's review round (sw-af272): a replaced directory's listing, places that stand, nearest reasons The review of 888e5dbe (sw-af272, wf-1090: 69 findings in 29 issues; verdicts recorded) found these, each fixed here: - I2: a resumed walk visits the directories holding its place twice, and reused the first listing without checking the directory was still the one it was read from. The cached listing now keeps the directory's dev and ino; on the second visit a different directory drops it (not complete, a "the directory changed while the walk ran" region), so a replaced directory's old names prove nothing absent. - I4: a pass whose walk couldn't list its scope's own directory recorded a null place, erasing it. A place is now recorded only when the walk listed the scope's directory (walk.complete has it). - I14: a place could be recorded before the mount's root identity, which acceptRoot's first-identity branch then wouldn't reset. A pass records the root identity before its place, as it does before evidence. - I17: a walk whose last file used up the cap recorded a place though it covered its scope. stoppedAfter is set only if the cap left an untaken file or an unlisted directory. - I13: an unreached path took the first region holding it, so a root-level cap region ('' matches everything) named the wrong reason. It now takes the nearest (longest) region holding it. - I28: every cap region carried the resume suffix. The pass now adds one entry at its scope: "the file cap (N) was reached: the next sync or scan of this scope resumes after /"; the regions keep their short reasons. - I3: the walkCap refusal showed NaN and Infinity as null (JSON) and threw on a BigInt. Numbers show as JavaScript writes them, a BigInt with n, anything else as JSON, and the message says the integer is at most Number.MAX_SAFE_INTEGER. - I15: the sync description says the next sync of the same mount or directory resumes. - I23: recordWalk's comment says a 0.22 checkpoint drops every place. - I7, I26, I27, I29: the changelog fragment's wording (the order lets each walk resume; what the description says; catch-up scans at start and reattach; what going back to 0.22 does, and what a 0.22 checkpoint does). New tests, under "a mount larger than its walk cap" (I1, I5, I6, I8-I11, I13, I14, I16, I17, I19, I21 and the fixes above): an accepted new root starts from its top; places go through a checkpoint and a null clears one; agent-action and catch-up scans resume; directories on both sides of the place; a directory past the cap isn't listed, so its deletion waits; one resume entry and nearest-region reasons; a directory replaced between the stretches (readdir hook); a place recorded after its root by a pass with no evidence (files over maxFileSize); a covered scope (last file uses up the cap; a resumed pass taking every file) leaves no place and nothing incomplete; glob and grep leave no place; a directory sync leaves the root's place; an unlistable scope (chmod 000) keeps its place; the walk entry has no path and an unknown entry kind leaves evidence alone; each refused walkCap is named in the message. The order test's comment says Node's readdir already sorts its names on Linux. Not changed (verdicts): I12 intended (the place advances over taken files, observed or not), I18 intended, I22 not-real (a trailing-slash scope 'd/' walks 'd//...' paths, so its place matches; the double slash itself is on main already), I24 not-real (the wrap ends at the place inclusive because that file is the cycle's last), I25 not-worth. On the VPS: tsc clean; workspace-three-way 191/191 on Node 24.21.0 (190/190 on 24.21.0 and 20.20.2 before the root-identity test was added). --- changelog.d/272-walk-cap.fixed.md | 8 +- src/modules/workspace/disk-agreement.ts | 6 +- src/modules/workspace/index.ts | 9 +- src/modules/workspace/observe.ts | 28 ++- src/modules/workspace/reconcile.ts | 31 ++- test/workspace-three-way.test.ts | 244 +++++++++++++++++++++++- 6 files changed, 300 insertions(+), 26 deletions(-) diff --git a/changelog.d/272-walk-cap.fixed.md b/changelog.d/272-walk-cap.fixed.md index b2edcff76..493ca8e92 100644 --- a/changelog.d/272-walk-cap.fixed.md +++ b/changelog.d/272-walk-cap.fixed.md @@ -1,5 +1,5 @@ -- 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 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 first scan that reaches the cap lists what it didn't reach under `incomplete`, and the next one of the same scope resumes after the last file 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 5000 files in `readdir` order, 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 walk takes each directory's files by name, then its subdirectories by name, so the files past the cap are the same from one walk to the next. +- 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 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 file 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 walk takes each directory's files by name, then its subdirectories by name, so each walk can resume where the last stopped, whatever order the platform lists a directory in. - 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 says this, so the first request after the upgrade that carries the workspace's tools misses the prompt cache from its tool block on. - - Going back to 0.22: the place is kept in an entry 0.22 doesn't know and leaves alone, so its walks start at the top again. + - 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. + - 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. diff --git a/src/modules/workspace/disk-agreement.ts b/src/modules/workspace/disk-agreement.ts index 43471d1aa..686c58e9b 100644 --- a/src/modules/workspace/disk-agreement.ts +++ b/src/modules/workspace/disk-agreement.ts @@ -275,8 +275,10 @@ export class DiskAgreement { /** * Record where a walk of `scope` stopped (`after`), or that one covered it - * (null). Kept with the evidence so a restart resumes there too; a lost - * entry only costs the walk its place. + * (null). Kept with the evidence so a restart resumes there too. A place + * 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. */ recordWalk(mount: string, scope: string, after: string | null): void { if ((this.walkResumesAfter(mount, scope) ?? null) === after) return; diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index 2ffba9fbc..1c6884224 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -655,7 +655,11 @@ export class WorkspaceModule implements Module { for (const mount of config.mounts) { const cap: unknown = mount.walkCap; if (cap !== undefined && !(Number.isSafeInteger(cap) && (cap as number) > 0)) { - throw new Error(`[workspace] Mount "${mount.name}": walkCap must be a positive integer, not ${JSON.stringify(cap) ?? String(cap)}`); + // Numbers as JavaScript writes them (JSON would say null for NaN), anything else as JSON. + const shown = typeof cap === 'number' ? String(cap) : typeof cap === 'bigint' ? `${cap}n` : JSON.stringify(cap) ?? String(cap); + throw new Error( + `[workspace] Mount "${mount.name}": walkCap must be a positive integer no larger than ${Number.MAX_SAFE_INTEGER}, not ${shown}`, + ); } } // Detect overlapping mount paths: if mount A contains mount B, @@ -1221,7 +1225,8 @@ export class WorkspaceModule implements Module { 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 files (5000 unless configured); past it, what this one didn\'t reach is listed under ' - + '`incomplete`, and the next sync resumes where it stopped. With a path (file or directory), ' + + '`incomplete`, and the next sync of the same mount or directory resumes where it stopped. 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. ' + 'Each workspace change it gives up is listed under `discarded`, with the state it had and, as `op`, ' diff --git a/src/modules/workspace/observe.ts b/src/modules/workspace/observe.ts index 7d763eb58..9c811f2c8 100644 --- a/src/modules/workspace/observe.ts +++ b/src/modules/workspace/observe.ts @@ -848,8 +848,12 @@ export async function walkScope( ? opts.resumeAfter : undefined; - type Listing = { files: string[]; subdirs: string[] }; - /** Each directory is listed once: a resumed walk visits the ones holding its position on both sides of it. */ + type Listing = { files: string[]; subdirs: string[]; dev: string; ino: string }; + /** + * Each directory is listed once: a resumed walk visits the ones holding its + * position on both sides of it, and the second visit reuses the listing + * while the directory is still the one it was read from. + */ const listings = new Map(); const visited = new Set(); /** Paths already reported as errors, so a second visit doesn't report them again. */ @@ -865,7 +869,17 @@ export async function walkScope( const list = async (dir: CheckedDir): Promise => { const rel = dir.rel; const known = listings.get(rel); - if (known !== undefined) return known; + if (known === null) return null; + if (known !== undefined) { + const now = dir.identity(); + if (known.dev === now.dev && known.ino === now.ino) return known; + // Replaced since its listing was read: that listing says nothing about + // the directory there now, so it proves nothing absent. + walk.complete.delete(rel); + listings.set(rel, null); + fail(rel, CHANGED_WALK); + return null; + } listings.set(rel, null); let entries; try { @@ -915,7 +929,7 @@ export async function walkScope( for (const [p, why] of others) walk.others.set(p, why); walk.incomplete.push(...regions); walk.complete.add(rel); - const listing = { files, subdirs }; + const listing = { files, subdirs, ...dir.identity() }; listings.set(rel, listing); return listing; }; @@ -959,7 +973,6 @@ export async function walkScope( await visit(top.dir, 'after'); if (left > 0) await visit(top.dir, 'upto'); } - if (left === 0 && lastTaken !== undefined) walk.stoppedAfter = lastTaken; // What the cap left: directories holding files not taken, and directories // a listing showed that weren't listed. A walk that doesn't descend leaves @@ -970,12 +983,17 @@ export async function walkScope( for (const d of holdingUntaken) { walk.incomplete.push({ path: d, reason: `${capped} before every file here was taken`, kind: 'cap' }); } + let unlisted = 0; if (recursive) { for (const d of walk.dirs) { if (visited.has(d) || walk.missing.has(d) || failed.has(d)) continue; + unlisted++; walk.incomplete.push({ path: d, reason: `not visited: ${capped}`, kind: 'cap' }); } } + // The cap stopped the walk only if it left something: a walk whose last + // file used up the cap covered its scope. + if (left === 0 && lastTaken !== undefined && (holdingUntaken.size > 0 || unlisted > 0)) walk.stoppedAfter = lastTaken; return walk; } diff --git a/src/modules/workspace/reconcile.ts b/src/modules/workspace/reconcile.ts index 227e221d7..f9944da64 100644 --- a/src/modules/workspace/reconcile.ts +++ b/src/modules/workspace/reconcile.ts @@ -651,8 +651,13 @@ async function gather(store: JsStore, agreement: DiskAgreement, mount: MountRunt continue; } if (walk && !walk.files.has(path) && !walk.others.has(path)) { - // Beneath a region the walk didn't reach: unproven either way. - const region = walk.incomplete.find((r) => r.path === '' || path === r.path || path.startsWith(r.path + '/')); + // Beneath a region the walk didn't reach: unproven either way. The + // nearest region holding it says why. + let region: Walk['incomplete'][number] | undefined; + for (const r of walk.incomplete) { + if (r.path !== '' && path !== r.path && !path.startsWith(r.path + '/')) continue; + if (region === undefined || r.path.length > region.path.length) region = r; + } gathered.facts.set(path, { kind: 'unobserved', reason: region ? region.reason : 'not reached by the walk' }); continue; } @@ -808,9 +813,15 @@ 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. - const resumeAfter = opts.resume && walk && scope.kind === 'dir' && scope.recursive ? walk.stoppedAfter ?? null : undefined; - if (resumeAfter !== undefined && scope.kind === 'dir') agreement.recordWalk(mount.name, scope.dir, resumeAfter); + // 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. + 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); + } // 5. Durable before returning. An agreement advance (D = S) has no pending // intent to recover from, so losing its append would leave an older P that @@ -835,11 +846,15 @@ export async function reconcilePass( } if (walk) { const tracked = [...result.reports.values()].filter((r) => r.state === 'unverified').map((r) => r.path); - const resumes = typeof resumeAfter === 'string' ? `; the next sync or scan here resumes after ${mount.name}/${resumeAfter}` : ''; + if (typeof resumeAfter === 'string' && scope.kind === 'dir') { + result.incomplete.push({ + path: scope.dir, + reason: `the file cap (${opts.cap ?? mount.walkCap}) was reached: the next sync or scan of this scope resumes after ${mount.name}/${resumeAfter}`, + }); + } for (const region of walk.incomplete) { const hides = tracked.some((p) => p === region.path || p.startsWith(region.path + '/')); - if (region.kind === 'cap') result.incomplete.push({ path: region.path, reason: region.reason + resumes }); - else if (region.kind === 'error' || hides) result.incomplete.push({ path: region.path, reason: region.reason }); + if (region.kind === 'error' || region.kind === 'cap' || hides) result.incomplete.push({ path: region.path, reason: region.reason }); // An ignored name that hides nothing tracked is reported nowhere else: // counted, so a sync that says 0 also says what it didn't look at // (agent-framework #276). A symlink region isn't counted: the link diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index 9eb798196..dd0ec6f01 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -21,7 +21,8 @@ import { tmpdir } from 'node:os'; import { basename, dirname, join } from 'node:path'; import { JsStore } from '@animalabs/chronicle'; import { WorkspaceModule } from '../src/modules/workspace/index.js'; -import { DiskAgreement } from '../src/modules/workspace/disk-agreement.js'; +import { DISK_AGREEMENT_RECORD_TYPE, DiskAgreement } from '../src/modules/workspace/disk-agreement.js'; +import { RecordJournal } from '../src/record-journal.js'; import { confirmAbsent, fingerprintVouches, filesystemTrustsCtime, fsTypeTrustsCtime, observePath, readPath, type MountView } from '../src/modules/workspace/observe.js'; import { EffectFailed, syncDirectories, unlinkContained, writeContained } from '../src/modules/workspace/effects.js'; import { pushPaths, settleAdoptionBeforeMutation } from '../src/modules/workspace/reconcile.js'; @@ -2079,7 +2080,9 @@ describe('a mount larger than its walk cap (agent-framework #272)', () => { test('the walk takes files by name, whatever order they were written in', async (t) => { const env = new Env(t); // Neither the order written nor its reverse starts with the three - // smallest names, so a walk taking them in either order would differ. + // smallest names. Node's readdir already returns these names sorted on + // Linux, so this pins the order where readdir doesn't sort them; the next + // test is the one that fails on Linux if the walk stops sorting. for (const f of ['g.txt', 'c.txt', 'h.txt', 'a.txt', 'f.txt', 'b.txt', 'e.txt', 'd.txt']) env.writeDisk(f, f); const m = await env.open({ walkCap: 3 }); await call(m, 'sync', {}); @@ -2156,16 +2159,247 @@ describe('a mount larger than its walk cap (agent-framework #272)', () => { assert.deepEqual(syncedNames(env), ['a.txt', 'b.txt', 'c.txt', 'd.txt'], 'c.txt and d.txt, not a.txt and b.txt again'); }); - test('walkCap must be a positive integer', () => { - for (const walkCap of [0, -1, 1.5, Number.POSITIVE_INFINITY, Number.NaN, '5000']) { + test('walkCap must be a positive integer, and the refusal names the value it was given', () => { + const refused: Array<[unknown, string]> = [ + [0, '0'], [-1, '-1'], [1.5, '1.5'], [Number.POSITIVE_INFINITY, 'Infinity'], [Number.NaN, 'NaN'], + [2 ** 53, '9007199254740992'], ['5000', '"5000"'], [10n, '10n'], [null, 'null'], + ]; + for (const [walkCap, shown] of refused) { assert.throws( () => new WorkspaceModule({ mounts: [{ name: 'work', path: '/tmp', mode: 'read-write', walkCap: walkCap as number }] }), - /Mount "work": walkCap must be a positive integer/, + (err: Error) => err.message.startsWith('[workspace] Mount "work": walkCap must be a positive integer') + && err.message.endsWith(`, not ${shown}`), `walkCap ${String(walkCap)}`, ); } assert.doesNotThrow(() => new WorkspaceModule({ mounts: [{ name: 'work', path: '/tmp', mode: 'read-write', walkCap: 12_000 }] })); }); + + test('a walk whose last file uses up the cap covered its scope: no place, nothing past the cap', async (t) => { + const env = new Env(t); + for (const f of ['a.txt', 'b.txt']) env.writeDisk(f, f); + const m = await env.open({ walkCap: 2 }); + const synced = await call(m, 'sync', {}); + assert.equal(synced.incomplete, undefined, JSON.stringify(synced.incomplete)); + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', ''), undefined); + }); + + test('a resumed pass that takes every file reports nothing past the cap and clears its place', async (t) => { + const env = new Env(t); + for (const f of ['a.txt', 'b.txt', 'c.txt']) env.writeDisk(f, f); + let m = await env.open({ walkCap: 2 }); + await call(m, 'sync', {}); + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', ''), 'b.txt'); + // A larger cap: the pass resumes after b.txt, takes c.txt, wraps and + // takes a.txt and b.txt, and stops short of the cap. + m = await env.restart(m, { walkCap: 5 }); + const synced = await call(m, 'sync', {}); + assert.equal(synced.incomplete, undefined, JSON.stringify(synced.incomplete)); + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', ''), undefined); + }); + + test('directories before and after the place: the after stretch takes a later sibling, the wrap an earlier one', async (t) => { + const env = new Env(t); + for (const f of ['a/1.txt', 'a/2.txt', 'a/3.txt', 'm/1.txt', 'm/2.txt']) env.writeDisk(f, f); + const m = await env.open({ walkCap: 2 }); + await call(m, 'sync', {}); + assert.deepEqual(syncedNames(env), ['a/1.txt', 'a/2.txt']); + // After a/2.txt: a/3.txt, then m/, the sibling after a/. + await call(m, 'sync', {}); + assert.deepEqual(syncedNames(env), ['a/1.txt', 'a/2.txt', 'a/3.txt', 'm/1.txt']); + // After m/1.txt: m/2.txt, then the wrap reaches a/, which lies before the place. + env.writeDisk('a/1.txt', 'edited before the wrap'); + await call(m, 'sync', {}); + assert.deepEqual(syncedNames(env), ['a/1.txt', 'a/2.txt', 'a/3.txt', 'm/1.txt', 'm/2.txt']); + assert.equal(await contentOf(m, 'a/1.txt'), 'edited before the wrap'); + }); + + test('once the cap is reached no further directory is listed, so a deletion there waits for a pass that lists it', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 1 }); + await seedSynced(env, m, 'z/q.txt', 'q'); + env.rmDisk('z/q.txt'); + for (const f of ['a.txt', 'b.txt']) env.writeDisk(f, f); + const synced = await call(m, 'sync', {}); + assert.notEqual(env.store.treeGet(TREE, 'z/q.txt'), null, 'z/ was not listed: nothing proven'); + assert.ok(synced.incomplete.some((r: { mount: string; path: string; reason: string }) => r.mount === 'work' && r.path === 'z' && /not visited/.test(r.reason)), + JSON.stringify(synced.incomplete)); + }); + + test('the capped pass says once where the next resumes, and each unreached path gets the reason of the region nearest it', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 1 }); + await seedSynced(env, m, 'z/q.txt', 'q'); + for (const f of ['a.txt', 'b.txt']) env.writeDisk(f, f); + const synced = await call(m, 'sync', {}); + const resumes = synced.incomplete.filter((r: { reason: string }) => /resumes after/.test(r.reason)); + assert.deepEqual(resumes.map((r: { mount: string; path: string; reason: string }) => [r.mount, r.path, r.reason]), + [['work', '', 'the file cap (1) was reached: the next sync or scan of this scope resumes after work/a.txt']]); + // z/q.txt sits beneath z/, which wasn't listed: its reason is z/'s, not + // that of the root, which holds an untaken b.txt. + const skipped = synced.skipped?.find((x: { path: string }) => x.path === 'z/q.txt'); + assert.match(skipped?.reason ?? '', /not visited: the file cap \(1\) was reached/, JSON.stringify(synced.skipped)); + }); + + test('agent-action scans resume where the last stopped', async (t) => { + const env = new Env(t); + for (const f of ['a.txt', 'b.txt', 'c.txt', 'd.txt']) env.writeDisk(f, f); + const m = await env.open({ watch: 'on-agent-action', walkCap: 2 }); + await m.onToolBatchComplete('agent'); + assert.deepEqual(syncedNames(env), ['a.txt', 'b.txt']); + await m.onToolBatchComplete('agent'); + assert.deepEqual(syncedNames(env), ['a.txt', 'b.txt', 'c.txt', 'd.txt']); + }); + + test("a watched mount's catch-up scan resumes where the last stopped", async (t) => { + const env = new Env(t); + for (const f of ['a.txt', 'b.txt', 'c.txt', 'd.txt']) env.writeDisk(f, f); + const m = await env.open({ walkCap: 2 }); + await (m as any).initialScan('work'); + assert.deepEqual(syncedNames(env), ['a.txt', 'b.txt']); + await (m as any).initialScan('work'); + assert.deepEqual(syncedNames(env), ['a.txt', 'b.txt', 'c.txt', 'd.txt']); + }); + + test('glob and grep take the same files every time and leave no place', async (t) => { + const env = new Env(t); + for (const f of ['a.txt', 'b.txt', 'c.txt']) env.writeDisk(f, 'v'); + const m = await env.open({ walkCap: 2 }); + const globbed = async () => (await call(m, 'glob', { pattern: '**' })).matches.map((x: { path: string }) => x.path).sort(); + const first = await globbed(); + assert.deepEqual(await globbed(), first); + const grepped = async () => JSON.stringify(await call(m, 'grep', { pattern: 'v' })); + const once = await grepped(); + assert.equal(await grepped(), once); + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', ''), undefined); + }); + + test("a directory sync leaves the mount root's place as it was", async (t) => { + const env = new Env(t); + for (const f of ['a.txt', 'b.txt', 'c.txt', 'd/x.txt', 'd/y.txt', 'd/z.txt']) env.writeDisk(f, f); + const m = await env.open({ walkCap: 2 }); + await call(m, 'sync', {}); + const agreement = (m as any).agreementOrThrow(); + assert.equal(agreement.walkResumesAfter('work', ''), 'b.txt'); + await call(m, 'sync', { path: 'work/d' }); + await call(m, 'sync', { path: 'work/d' }); + assert.equal(agreement.walkResumesAfter('work', ''), 'b.txt'); + }); + + test("a pass that can't list its scope's directory leaves the place as it was", { skip: IS_ROOT ? 'root reads every directory' : false }, async (t) => { + const env = new Env(t); + for (const f of ['d/a.txt', 'd/b.txt', 'd/c.txt']) env.writeDisk(f, f); + const m = await env.open({ walkCap: 2 }); + await call(m, 'sync', { path: 'work/d' }); + const agreement = (m as any).agreementOrThrow(); + assert.equal(agreement.walkResumesAfter('work', 'd'), 'd/b.txt'); + chmodSync(env.disk('d'), 0o000); + try { + const blocked = await call(m, 'sync', { path: 'work/d' }); + assert.ok(blocked.incomplete?.some((r: { reason: string }) => /cannot list/.test(r.reason)), JSON.stringify(blocked)); + assert.equal(agreement.walkResumesAfter('work', 'd'), 'd/b.txt', 'the place stands'); + } finally { + chmodSync(env.disk('d'), 0o755); + } + await call(m, 'sync', { path: 'work/d' }); + assert.ok(syncedNames(env).includes('d/c.txt'), 'the next pass resumed after d/b.txt'); + }); + + test('a directory replaced between the two stretches of a resumed walk proves nothing absent from its old listing', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 3 }); + for (const f of ['s/a.txt', 's/b.txt', 's/c.txt', 's/z.txt', 't/x.txt']) await seedSynced(env, m, f, f); + await call(m, 'sync', {}); // takes s/a, s/b, s/c: the place is s/c.txt + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', ''), 's/c.txt'); + env.rmDisk('s/z.txt'); + // The next pass lists s/ (without z.txt) on the after stretch, then t/. + // While t/ is listed, s/ is replaced by a directory that holds z.txt, so + // the wrap's second visit to s/ meets another directory. + let swapped = false; + await withFsPromises('readdir', (readdir) => async (path: string, ...rest: unknown[]) => { + if (!swapped && String(path).endsWith(`${'/'}t`)) { + swapped = true; + renameSync(env.disk('s'), join(env.root, 's-away')); + for (const f of ['a.txt', 'b.txt', 'c.txt', 'z.txt']) env.writeDisk(`s/${f}`, `s/${f}`); + } + return readdir(path, ...rest); + }, async () => { + const synced = await call(m, 'sync', {}); + assert.ok(synced.incomplete?.some((r: { mount: string; path: string; reason: string }) => r.mount === 'work' && r.path === 's' && /changed while the walk ran/.test(r.reason)), + JSON.stringify(synced.incomplete)); + }); + assert.ok(swapped); + assert.notEqual(env.store.treeGet(TREE, 's/z.txt'), null, 's/z.txt is on disk in the new s/: not proven absent'); + }); + + test('an accepted new root starts with no place', async (t) => { + const env = new Env(t); + for (const f of ['a.txt', 'b.txt', 'c.txt']) env.writeDisk(f, f); + const m = await env.open({ walkCap: 2 }); + await call(m, 'sync', {}); + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', ''), 'b.txt'); + // Another directory takes the root's place, holding the same files. + renameSync(env.dir, `${env.dir}-away`); + mkdirSync(env.dir); + for (const f of ['a.txt', 'b.txt', 'c.txt']) env.writeDisk(f, f); + await call(m, 'sync', { mount: 'work', acceptRoot: true }); + // From the new root's start: a.txt and b.txt. Resuming after the old + // place would have taken c.txt, then a.txt. + assert.equal(env.store.treeGet(TREE, 'c.txt'), null, 'c.txt not taken'); + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', ''), 'b.txt'); + }); + + test('a place is recorded after the root it was gathered under, even by a pass that records no evidence', async (t) => { + const env = new Env(t); + // Files too large to take in: the pass records no evidence for them. + for (const f of ['a.txt', 'b.txt']) env.writeDisk(f, 'more than one byte'); + const m = await env.open({ walkCap: 1, maxFileSize: 1 }); + await call(m, 'sync', {}); + const agreement = (m as any).agreementOrThrow(); + assert.equal(agreement.walkResumesAfter('work', ''), 'a.txt'); + const st = statSync(env.dir, { bigint: true }); + assert.deepEqual(agreement.rootIdentity('work'), { dev: String(st.dev), ino: String(st.ino) }); + }); + + test('places go through a checkpoint, and a scope a walk covered stays cleared', (t) => { + const env = new Env(t); + const agreement = new DiskAgreement(env.store); + agreement.open([{ name: 'work', root: env.dir }]); + agreement.recordWalk('work', '', 'x.txt'); + agreement.recordWalk('work', 'd', 'd/y.txt'); + agreement.recordWalk('work', 'e', 'e/z.txt'); + agreement.recordWalk('work', 'e', null); + for (let i = 0; i < 1100; i++) agreement.set('work', `f${i}.txt`, { kind: 'absent' }); + agreement.maybeCheckpoint(); + assert.equal((agreement as any).journal.entriesSinceCheckpoint, 0, 'a checkpoint was written'); + agreement.barrier(); + const reopened = new DiskAgreement(env.store); + reopened.open([{ name: 'work', root: env.dir }]); + assert.equal(reopened.walkResumesAfter('work', ''), 'x.txt'); + assert.equal(reopened.walkResumesAfter('work', 'd'), 'd/y.txt'); + assert.equal(reopened.walkResumesAfter('work', 'e'), undefined); + }); + + test("a place's journal entry carries no path, and an entry of a kind this version doesn't know changes nothing", (t) => { + const env = new Env(t); + const agreement = new DiskAgreement(env.store); + agreement.open([{ name: 'work', root: env.dir }]); + agreement.set('work', 'a.txt', { kind: 'absent' }); + agreement.recordWalk('work', '', 'a.txt'); + agreement.barrier(); + // agent-framework 0.22 replays an entry it doesn't know as a forget of + // entry.path: a place with a path would forget that path's evidence. + const journal = new RecordJournal, unknown>(env.store, { type: DISK_AGREEMENT_RECORD_TYPE }); + const walks = journal.load().entries.map(({ entry }) => entry).filter((e) => e.t === 'walk'); + assert.equal(walks.length, 1); + assert.equal('path' in walks[0]!, false, JSON.stringify(walks[0])); + // A later version's entry this one doesn't know leaves the evidence alone. + journal.append({ t: 'a-later-kind', mount: 'work', path: 'a.txt' }); + const reopened = new DiskAgreement(env.store); + reopened.open([{ name: 'work', root: env.dir }]); + assert.deepEqual(reopened.get('work', 'a.txt'), { kind: 'absent' }); + assert.equal(reopened.walkResumesAfter('work', ''), 'a.txt'); + }); }); describe('the mount boundary', () => { From b75beb87cd062a3de91794568f9bd62cb23c504f Mon Sep 17 00:00:00 2001 From: Gladys-2413 Date: Wed, 14 Oct 2026 09:30:31 -0400 Subject: [PATCH 3/7] fix(workspace): a resumed walk rechecks its scope's own directory on the wrap, and drops what it read of a replaced subtree (#272) Beth-2389's reads of 968a48f1 (room-203 #94382, probes J8230, J8234, J8235) and of the first answer, 66adb358 (#94649, probes J8269): - Should-fix: both stretches visit the scope through the same top CheckedDir, whose identity() is the one taken when it was checked, so the revisit's identity comparison could never differ for the scope's own directory. A directory sync whose scope was replaced between the stretches reused the old listing and proved a file absent that the new directory holds. The cached branch now also requires dir.stillHere(), which checks the path again; a subdirectory's revisit already had a fresh check from checkChild, and the mount root has gather's root check too. - A replaced directory's drop now takes back everything the walk read at or beneath it, since all of it describes the directory that was there: every listing there stops counting as complete; their untaken files leave Walk.seen and their subdirectories leave Walk.dirs, so a pass that never reached its cap reports nothing as the cap's ("the file cap (N) was reached" for the directory, or "not visited" for a subdirectory before the place); and a subdirectory proven missing beneath it meanwhile leaves Walk.missing, so it proves nothing absent about what's there now. The directory itself stays in its parent's names. Beth's two lines for the subdirectories and the missing ones, and her probes as tests. Tests (workspace-three-way, 195): - a directory sync's scope replaced while the after stretch lists d/t keeps d/z.txt and leaves the place at d/c.txt; - the replaced-subdirectory test also asserts that a pass that took one file of its three, t/x.txt, reports nothing as the cap's; - with d/ replaced after d/t/ was listed beneath it, d/t/k.txt stays unproven; - (Beth's probe A) a replaced scope's subdirectory before the place, d/a, isn't reported as the cap's; - (Beth's probe B) d/u/, removed while d/k/ is listed and so proven missing, then back when d/ is replaced, keeps d/u/q.txt. On the VPS: tsc clean; workspace-three-way 195/195 on Node 24.21.0 and 20.20.2. --- src/modules/workspace/observe.ts | 27 ++++++-- test/workspace-three-way.test.ts | 102 +++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 6 deletions(-) diff --git a/src/modules/workspace/observe.ts b/src/modules/workspace/observe.ts index 9c811f2c8..72b2877c6 100644 --- a/src/modules/workspace/observe.ts +++ b/src/modules/workspace/observe.ts @@ -852,7 +852,7 @@ export async function walkScope( /** * Each directory is listed once: a resumed walk visits the ones holding its * position on both sides of it, and the second visit reuses the listing - * while the directory is still the one it was read from. + * while the directory at its path is still the one it was read from. */ const listings = new Map(); const visited = new Set(); @@ -871,12 +871,27 @@ export async function walkScope( const known = listings.get(rel); if (known === null) return null; if (known !== undefined) { + // The scope's own directory comes back as the same CheckedDir, so its + // identity is checked against the path again, as a subdirectory's is + // by the fresh check that brought it here. const now = dir.identity(); - if (known.dev === now.dev && known.ino === now.ino) return known; - // Replaced since its listing was read: that listing says nothing about - // the directory there now, so it proves nothing absent. - walk.complete.delete(rel); - listings.set(rel, null); + if (known.dev === now.dev && known.ino === now.ino && await dir.stillHere()) return known; + // Replaced since its listing was read: what the walk read of it and + // beneath it describes the directory that was there, so none of it + // proves anything absent, and neither its untaken files nor its + // unlisted subdirectories are the cap's to report. A subdirectory + // proven missing beneath it meanwhile proves nothing about what's + // there now either. + for (const [path, listing] of listings) { + if (rel !== '' && path !== rel && !path.startsWith(rel + '/')) continue; + if (listing) { + for (const f of listing.files) walk.seen.delete(f); + for (const sub of listing.subdirs) walk.dirs.delete(sub); + } + walk.complete.delete(path); + listings.set(path, null); + } + for (const path of walk.missing) if (rel === '' || path.startsWith(rel + '/')) walk.missing.delete(path); fail(rel, CHANGED_WALK); return null; } diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index dd0ec6f01..3a5acc702 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -2327,11 +2327,113 @@ describe('a mount larger than its walk cap (agent-framework #272)', () => { const synced = await call(m, 'sync', {}); assert.ok(synced.incomplete?.some((r: { mount: string; path: string; reason: string }) => r.mount === 'work' && r.path === 's' && /changed while the walk ran/.test(r.reason)), JSON.stringify(synced.incomplete)); + // The pass took one file of its three, t/x.txt: nothing it left is the cap's. + assert.ok(!synced.incomplete.some((r: { reason: string }) => /file cap/.test(r.reason)), JSON.stringify(synced.incomplete)); }); assert.ok(swapped); assert.notEqual(env.store.treeGet(TREE, 's/z.txt'), null, 's/z.txt is on disk in the new s/: not proven absent'); }); + test("a directory sync's scope replaced between the two stretches proves nothing absent from its old listing", async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 3 }); + for (const f of ['d/a.txt', 'd/b.txt', 'd/c.txt', 'd/z.txt', 'd/t/x.txt']) await seedSynced(env, m, f, f); + await call(m, 'sync', { path: 'work/d' }); // takes d/a, d/b, d/c: the place is d/c.txt + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', 'd'), 'd/c.txt'); + env.rmDisk('d/z.txt'); + // The scope's own directory is replaced while the after stretch lists + // d/t/, so the wrap comes back to another d/, which holds z.txt. + let swapped = false; + await withFsPromises('readdir', (readdir) => async (path: string, ...rest: unknown[]) => { + if (!swapped && String(path).endsWith(`${'/'}d${'/'}t`)) { + swapped = true; + renameSync(env.disk('d'), join(env.root, 'd-away')); + for (const f of ['a.txt', 'b.txt', 'c.txt', 'z.txt', 't/x.txt']) env.writeDisk(`d/${f}`, `d/${f}`); + } + return readdir(path, ...rest); + }, async () => { + const synced = await call(m, 'sync', { path: 'work/d' }); + assert.ok(synced.incomplete?.some((r: { path: string; reason: string }) => /changed while the walk ran/.test(r.reason) && /(^|\/)d$/.test(r.path)), + JSON.stringify(synced.incomplete)); + }); + assert.ok(swapped); + assert.notEqual(env.store.treeGet(TREE, 'd/z.txt'), null, 'd/z.txt is on disk in the new d/: not proven absent'); + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', 'd'), 'd/c.txt', 'a pass whose scope changed under it leaves the place'); + }); + + test('when a directory is replaced, the listings read beneath it before stop proving anything too', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 3 }); + for (const f of ['d/a.txt', 'd/b.txt', 'd/c.txt', 'd/t/k.txt', 'd/u/x.txt']) await seedSynced(env, m, f, f); + await call(m, 'sync', { path: 'work/d' }); // takes d/a, d/b, d/c: the place is d/c.txt + env.rmDisk('d/t/k.txt'); + // The after stretch lists d/t/ (without k.txt), then d/ is replaced while + // d/u/ is listed; the new d/t/ holds k.txt. + let swapped = false; + await withFsPromises('readdir', (readdir) => async (path: string, ...rest: unknown[]) => { + if (!swapped && String(path).endsWith(`${'/'}d${'/'}u`)) { + swapped = true; + renameSync(env.disk('d'), join(env.root, 'd-away')); + for (const f of ['a.txt', 'b.txt', 'c.txt', 't/k.txt', 'u/x.txt']) env.writeDisk(`d/${f}`, `d/${f}`); + } + return readdir(path, ...rest); + }, () => call(m, 'sync', { path: 'work/d' })); + assert.ok(swapped); + assert.notEqual(env.store.treeGet(TREE, 'd/t/k.txt'), null, "d/t/'s listing came from the replaced d/: it proves nothing absent"); + }); + + test("a replaced scope's subdirectory before the place isn't reported as the cap's", async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 3 }); + const files = ['d/a/f.txt', 'd/m/m1.txt', 'd/m/m2.txt', 'd/m/m3.txt', 'd/m/m4.txt', 'd/z/q.txt']; + for (const f of files) await seedSynced(env, m, f, f); + await call(m, 'sync', { path: 'work/d' }); // takes d/a/f, d/m/m1, d/m/m2: the place is d/m/m2.txt + // The after stretch takes d/m/m3 and d/m/m4 (two of three), then lists + // d/z/, while d/ is replaced; the wrap then meets another d/. + let swapped = false; + await withFsPromises('readdir', (readdir) => async (path: string, ...rest: unknown[]) => { + if (!swapped && String(path).endsWith(`${'/'}d${'/'}z`)) { + swapped = true; + renameSync(env.disk('d'), join(env.root, 'd-away')); + for (const f of files) env.writeDisk(f, f); + } + return readdir(path, ...rest); + }, async () => { + const synced = await call(m, 'sync', { path: 'work/d' }); + assert.ok(synced.incomplete?.some((r: { path: string; reason: string }) => /changed while the walk ran/.test(r.reason) && /(^|\/)d$/.test(r.path)), + JSON.stringify(synced.incomplete)); + assert.ok(!synced.incomplete.some((r: { reason: string }) => /file cap/.test(r.reason)), JSON.stringify(synced.incomplete)); + }); + assert.ok(swapped); + }); + + test('a subdirectory proven missing beneath a scope that is then replaced proves nothing absent', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 3 }); + const files = ['d/a.txt', 'd/b.txt', 'd/c.txt', 'd/k/x.txt', 'd/u/q.txt', 'd/z/y.txt']; + for (const f of files) await seedSynced(env, m, f, f); + await call(m, 'sync', { path: 'work/d' }); // takes d/a, d/b, d/c: the place is d/c.txt + // While the after stretch lists d/k/, d/u/ is removed, so the walk proves + // it missing; while it lists d/z/, d/ is replaced by a directory holding + // every file again, u/q.txt included (as a delete-and-recreate would). + let removed = false; + let swapped = false; + await withFsPromises('readdir', (readdir) => async (path: string, ...rest: unknown[]) => { + if (!removed && String(path).endsWith(`${'/'}d${'/'}k`)) { + removed = true; + rmSync(env.disk('d/u'), { recursive: true, force: true }); + } + if (!swapped && String(path).endsWith(`${'/'}d${'/'}z`)) { + swapped = true; + renameSync(env.disk('d'), join(env.root, 'd-away')); + for (const f of files) env.writeDisk(f, f); + } + return readdir(path, ...rest); + }, () => call(m, 'sync', { path: 'work/d' })); + assert.ok(removed && swapped); + assert.notEqual(env.store.treeGet(TREE, 'd/u/q.txt'), null, 'd/u/q.txt is on disk in the new d/: not proven absent'); + }); + test('an accepted new root starts with no place', async (t) => { const env = new Env(t); for (const f of ['a.txt', 'b.txt', 'c.txt']) env.writeDisk(f, f); From b295e9629ef8bbeea4bc8ad632fa3b400ae36fd5 Mon Sep 17 00:00:00 2001 From: Gladys-2413 Date: Wed, 14 Oct 2026 10:37:22 -0400 Subject: [PATCH 4/7] fix(workspace): a failed check on the wrap drops its listing, a directory found again isn't missing, and links count against the cap (#272) Greptile's review of b75beb87 on #340 found three issues: - A directory listed on the after stretch can fail its check on the wrap, for instance when it has become a symlink. Its listing stayed complete, so a tracked file the new target holds was proven absent. A failed or absent check now drops what the walk read at and beneath that directory, through dropBeneath, the helper the replaced-directory drop now uses too. - A directory proven missing on the after stretch and found again on the wrap kept its missing proof, which provenAbsent checks first, so the files the wrap took from it were proven absent. A directory found again now clears the missing proofs at and beneath it. - Whole listings recorded every link, and the pass observed each one, so on a mount that follows links a pass could read through any number of them past the cap. A link, or another entry that isn't a directory, now counts against the cap in the walk's order as a file does (Listing.leaves). The pass observes only the ones it took (Walk.othersTaken). An untaken one is unverified with the cap's reason, as an untaken file is, and the pass doesn't report it by path. The reason text says "the walk cap", and the walkCap doc, the docs of walkScope and walkResumesAfter, the sync description and the changelog say what it counts. Tests in workspace-three-way (198): - a directory listed on the after stretch that is a symlink on the wrap proves nothing absent; - a directory proven missing on the after stretch and back by the wrap proves nothing absent; - on a mount following links at walkCap 2, the first pass takes a.txt and l1, and the second l2 and l3. Untaken links get the cap's region, the pass doesn't report untracked ones by path, and it doesn't read through a tracked link it didn't take. Six tests' "file cap" patterns now read "walk cap". On the VPS, tsc is clean and workspace-three-way passes 198/198 on Node 24.21.0 and 20.20.2. Each of the six new pieces, removed on its own, fails its test: the drop on a failed check, clearing a found directory's missing proof, counting links, their cap region, and observing only taken links, as candidates and as tracked paths. --- changelog.d/272-walk-cap.fixed.md | 2 +- src/modules/workspace/disk-agreement.ts | 2 +- src/modules/workspace/index.ts | 2 +- src/modules/workspace/observe.ts | 115 +++++++++++++++--------- src/modules/workspace/reconcile.ts | 22 ++--- src/modules/workspace/types.ts | 16 ++-- test/workspace-three-way.test.ts | 85 ++++++++++++++++-- 7 files changed, 178 insertions(+), 66 deletions(-) diff --git a/changelog.d/272-walk-cap.fixed.md b/changelog.d/272-walk-cap.fixed.md index 493ca8e92..3b54052be 100644 --- a/changelog.d/272-walk-cap.fixed.md +++ b/changelog.d/272-walk-cap.fixed.md @@ -1,5 +1,5 @@ - 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 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 file 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 walk takes each directory's files 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 walk takes each directory's files 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. - 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. diff --git a/src/modules/workspace/disk-agreement.ts b/src/modules/workspace/disk-agreement.ts index 686c58e9b..373a28fdd 100644 --- a/src/modules/workspace/disk-agreement.ts +++ b/src/modules/workspace/disk-agreement.ts @@ -266,7 +266,7 @@ export class DiskAgreement { /** * Where the next walk of `scope` ('' for the mount root) resumes: the last - * file a walk of it took before its file cap stopped it, if one did. + * entry a walk of it took before its cap stopped it, if one did. */ walkResumesAfter(mount: string, scope: string): string | undefined { this.ensureReconciled(); diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index 1c6884224..18667b426 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -1224,7 +1224,7 @@ 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 files (5000 unless configured); past it, what this one didn\'t reach is listed under ' + + 'cap of files and links (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 ' + 'path (file or directory), ' + 'the workspace takes disk\'s state there explicitly: it restores a workspace-deleted file, resolves ' diff --git a/src/modules/workspace/observe.ts b/src/modules/workspace/observe.ts index 72b2877c6..1c846b9fe 100644 --- a/src/modules/workspace/observe.ts +++ b/src/modules/workspace/observe.ts @@ -19,7 +19,7 @@ * identity, and what they showed — a listing, an absence — is accepted only * if each is still the directory its logical path names afterwards. A walk records which * directories it listed completely, and only those prove a file absent: an - * unreadable directory, an ignored subtree, the file cap, a symlink the walk + * unreadable directory, an ignored subtree, the walk cap, a symlink the walk * doesn't descend, or a directory that changed while it was listed leave * everything beneath them unproven. Outside a walk, absence is proven the * same way: by a directory inside the mount, still at its path, that lacks @@ -675,11 +675,17 @@ export interface Walk { /** Regular files the walk took: the pass observes each one. */ files: Set; /** - * Regular files a listing showed that the walk didn't take, past its file - * cap or outside the stretch a resumed walk covered: present, and not + * Regular files a listing showed that the walk didn't take, past its cap + * or outside the stretch a resumed walk covered: present, and not * observed by this pass. */ seen: Set; + /** + * The names in `others` that aren't ignored and that the walk took, as it + * takes files, against its cap: the pass observes these by path. The rest + * are present, and not observed by this pass. + */ + othersTaken: Set; /** Directories seen. */ dirs: Set; /** Names seen that are neither (symlinks, sockets…), or ignored: present, unproven. */ @@ -701,8 +707,8 @@ export interface Walk { */ incomplete: Array<{ path: string; reason: string; kind: 'error' | 'cap' | 'ignored' | 'symlink' }>; /** - * When the file cap stopped the walk, the last file it took. A walk that - * resumes after it takes the files after it first, in the walk's order + * When the walk cap stopped the walk, the last entry it took. A walk that + * resumes after it takes the entries after it first, in the walk's order * (`compareInWalk`), then wraps to the start of its scope. */ stoppedAfter?: string; @@ -803,7 +809,8 @@ async function checkChild(view: MountView, rootReal: string, parent: CheckedDir, /** * List a directory scope ('' for the mount root), descending when recursive, - * taking up to `cap` files in the walk's order (`compareInWalk`). A scope + * taking up to `cap` entries in the walk's order (`compareInWalk`): files, + * and links or other entries that aren't directories. A scope * that resolves outside the mount is not listed at all; beneath it, the walk * only descends real directories, never symlinks. Each directory's listing * counts only if, once listed, it is still the directory at its path: a @@ -825,7 +832,8 @@ export async function walkScope( opts: { named?: boolean; resumeAfter?: string } = {}, ): Promise { const walk: Walk = { - files: new Set(), seen: new Set(), dirs: new Set(), others: new Map(), complete: new Set(), missing: new Set(), incomplete: [], + files: new Set(), seen: new Set(), othersTaken: new Set(), dirs: new Set(), others: new Map(), complete: new Set(), missing: new Set(), + incomplete: [], }; const top = await checkDir(view, rootReal, scope); if (top.kind === 'absent') { @@ -848,7 +856,8 @@ export async function walkScope( ? opts.resumeAfter : undefined; - type Listing = { files: string[]; subdirs: string[]; dev: string; ino: string }; + /** A listing's entries that aren't directories or ignored, in the walk's order, each a regular file or not. */ + type Listing = { leaves: Array<[string, boolean]>; subdirs: string[]; dev: string; ino: string }; /** * Each directory is listed once: a resumed walk visits the ones holding its * position on both sides of it, and the second visit reuses the listing @@ -866,6 +875,30 @@ export async function walkScope( let left = cap; let lastTaken: string | undefined; + /** + * Take back what the walk read at or beneath `rel`, a directory replaced + * or gone since it was listed: it describes the directory that was there, + * so no listing there proves anything absent, the entries and + * subdirectories they left untaken aren't the cap's to report, and a + * subdirectory proven missing there proves nothing about what's there + * now. What was taken stays taken: the pass observes it by path. + */ + const dropBeneath = (rel: string): void => { + for (const [path, listing] of listings) { + if (rel !== '' && path !== rel && !path.startsWith(rel + '/')) continue; + if (listing) { + for (const [leaf, isFile] of listing.leaves) { + if (isFile) walk.seen.delete(leaf); + else if (!walk.othersTaken.has(leaf)) walk.others.delete(leaf); + } + for (const sub of listing.subdirs) walk.dirs.delete(sub); + } + walk.complete.delete(path); + listings.set(path, null); + } + for (const path of walk.missing) if (rel === '' || path.startsWith(rel + '/')) walk.missing.delete(path); + }; + const list = async (dir: CheckedDir): Promise => { const rel = dir.rel; const known = listings.get(rel); @@ -876,22 +909,7 @@ export async function walkScope( // by the fresh check that brought it here. const now = dir.identity(); if (known.dev === now.dev && known.ino === now.ino && await dir.stillHere()) return known; - // Replaced since its listing was read: what the walk read of it and - // beneath it describes the directory that was there, so none of it - // proves anything absent, and neither its untaken files nor its - // unlisted subdirectories are the cap's to report. A subdirectory - // proven missing beneath it meanwhile proves nothing about what's - // there now either. - for (const [path, listing] of listings) { - if (rel !== '' && path !== rel && !path.startsWith(rel + '/')) continue; - if (listing) { - for (const f of listing.files) walk.seen.delete(f); - for (const sub of listing.subdirs) walk.dirs.delete(sub); - } - walk.complete.delete(path); - listings.set(path, null); - } - for (const path of walk.missing) if (rel === '' || path.startsWith(rel + '/')) walk.missing.delete(path); + dropBeneath(rel); fail(rel, CHANGED_WALK); return null; } @@ -912,7 +930,7 @@ export async function walkScope( return null; } entries.sort((a, b) => compareNames(a.name, b.name)); - const files: string[] = []; + const leaves: Array<[string, boolean]> = []; const subdirs: string[] = []; const others: Array<[string, string]> = []; const regions: Walk['incomplete'] = []; @@ -926,9 +944,10 @@ export async function walkScope( if (entry.isDirectory()) { subdirs.push(childRel); } else if (entry.isFile()) { - files.push(childRel); + leaves.push([childRel, true]); } else { const symlink = entry.isSymbolicLink(); + leaves.push([childRel, false]); others.push([childRel, symlink ? 'a symlink' : 'not a regular file']); if (symlink) regions.push({ path: childRel, reason: 'a symlink, not walked', kind: 'symlink' }); } @@ -939,12 +958,12 @@ export async function walkScope( fail(rel, CHANGED_WALK); return null; } - for (const f of files) walk.seen.add(f); + for (const [p, isFile] of leaves) if (isFile) walk.seen.add(p); for (const d of subdirs) walk.dirs.add(d); for (const [p, why] of others) walk.others.set(p, why); walk.incomplete.push(...regions); walk.complete.add(rel); - const listing = { files, subdirs, ...dir.identity() }; + const listing = { leaves, subdirs, ...dir.identity() }; listings.set(rel, listing); return listing; }; @@ -962,13 +981,20 @@ export async function walkScope( visited.add(dir.rel); const listing = await list(dir); if (!listing) return; - for (const f of listing.files) { + // A link or other entry that isn't a directory is taken as a file is, + // against the cap: the pass observes each one it takes, and following a + // link can read as much as a file. + for (const [leaf, isFile] of listing.leaves) { if (left === 0) break; - if (!fileIn(stretch, f)) continue; + if (!fileIn(stretch, leaf)) continue; left--; - walk.seen.delete(f); - walk.files.add(f); - lastTaken = f; + if (isFile) { + walk.seen.delete(leaf); + walk.files.add(leaf); + } else { + walk.othersTaken.add(leaf); + } + lastTaken = leaf; } if (!recursive) return; for (const sub of listing.subdirs) { @@ -976,8 +1002,16 @@ export async function walkScope( if (left === 0) break; if (!dirIn(stretch, sub)) continue; const child = await checkChild(view, rootReal, dir, sub); - if (child.kind === 'checked') await visit(child.dir, stretch); - else if (child.kind === 'absent') walk.missing.add(sub); + if (child.kind === 'checked') { + // A directory found again, on the wrap, after it was proven missing. + for (const p of walk.missing) if (p === sub || p.startsWith(sub + '/')) walk.missing.delete(p); + await visit(child.dir, stretch); + continue; + } + // On the wrap, a directory listed on the first stretch that isn't the + // directory there now: what the walk read of it describes another. + dropBeneath(sub); + if (child.kind === 'absent') walk.missing.add(sub); else fail(sub, child.reason); } }; @@ -989,14 +1023,15 @@ export async function walkScope( if (left > 0) await visit(top.dir, 'upto'); } - // What the cap left: directories holding files not taken, and directories - // a listing showed that weren't listed. A walk that doesn't descend leaves - // its subdirectories by design. - const capped = `the file cap (${cap}) was reached`; + // What the cap left: directories holding entries not taken, and + // directories a listing showed that weren't listed. A walk that doesn't + // descend leaves its subdirectories by design. + const capped = `the walk cap (${cap}) was reached`; const holdingUntaken = new Set(); for (const f of walk.seen) holdingUntaken.add(parentOf(f)); + for (const [p, why] of walk.others) if (why !== 'ignored by the mount' && !walk.othersTaken.has(p)) holdingUntaken.add(parentOf(p)); for (const d of holdingUntaken) { - walk.incomplete.push({ path: d, reason: `${capped} before every file here was taken`, kind: 'cap' }); + walk.incomplete.push({ path: d, reason: `${capped} before everything here was taken`, kind: 'cap' }); } let unlisted = 0; if (recursive) { diff --git a/src/modules/workspace/reconcile.ts b/src/modules/workspace/reconcile.ts index f9944da64..34e9fd58d 100644 --- a/src/modules/workspace/reconcile.ts +++ b/src/modules/workspace/reconcile.ts @@ -152,7 +152,7 @@ export interface MountRuntime { treeStateId: string; intents: BranchIntents; readOnly: boolean; - /** The most files one walk of the mount takes (`MountConfig.walkCap`). */ + /** The most entries one walk of the mount takes, files and the other entries that aren't directories (`MountConfig.walkCap`). */ walkCap: number; } @@ -634,7 +634,9 @@ async function gather(store: JsStore, agreement: DiskAgreement, mount: MountRunt }); for (const f of walk.files) if (inScope(f, scope)) candidates.add(f); // Ignored names are listed only when something tracks them (above). - for (const [p, why] of walk.others) if (why !== 'ignored by the mount' && inScope(p, scope)) candidates.add(p); + // The other entries it took (a link, a socket…): ignored names are + // listed only when something tracks them (above). + for (const p of walk.othersTaken) if (inScope(p, scope)) candidates.add(p); for (const d of walk.dirs) if (inScope(d, { kind: 'dir', dir: scope.dir, recursive: false })) gathered.dirs.push(d); } } @@ -650,9 +652,13 @@ async function gather(store: JsStore, agreement: DiskAgreement, mount: MountRunt gathered.facts.set(path, { kind: 'absent' }); continue; } - if (walk && !walk.files.has(path) && !walk.others.has(path)) { - // Beneath a region the walk didn't reach: unproven either way. The - // nearest region holding it says why. + if (walk?.others.get(path) === 'ignored by the mount') { + gathered.facts.set(path, { kind: 'unobserved', reason: 'ignored by the mount' }); + continue; + } + if (walk && !walk.files.has(path) && !walk.othersTaken.has(path)) { + // Not taken by the walk, or beneath a region it didn't reach: unproven + // either way. The nearest region holding it says why. let region: Walk['incomplete'][number] | undefined; for (const r of walk.incomplete) { if (r.path !== '' && path !== r.path && !path.startsWith(r.path + '/')) continue; @@ -661,10 +667,6 @@ async function gather(store: JsStore, agreement: DiskAgreement, mount: MountRunt gathered.facts.set(path, { kind: 'unobserved', reason: region ? region.reason : 'not reached by the walk' }); continue; } - if (walk?.others.get(path) === 'ignored by the mount') { - gathered.facts.set(path, { kind: 'unobserved', reason: 'ignored by the mount' }); - continue; - } const p = agreement.get(mount.name, path); const hasStore = store.treeGet(mount.treeStateId, path) !== null; const hasIntent = mount.intents.get(path) !== null; @@ -849,7 +851,7 @@ export async function reconcilePass( if (typeof resumeAfter === 'string' && scope.kind === 'dir') { result.incomplete.push({ path: scope.dir, - reason: `the file 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: the next sync or scan of this scope resumes after ${mount.name}/${resumeAfter}`, }); } for (const region of walk.incomplete) { diff --git a/src/modules/workspace/types.ts b/src/modules/workspace/types.ts index a909144ce..8dcfcb4a2 100644 --- a/src/modules/workspace/types.ts +++ b/src/modules/workspace/types.ts @@ -43,13 +43,15 @@ export interface MountConfig { /** Maximum file size in bytes (default: 5MB) */ maxFileSize?: number; /** - * The most files one walk of this mount takes (default: 5000), a positive - * integer. 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 file - * 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 files it took. + * The most entries one walk of this mount takes (default: 5000), a + * positive 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. */ walkCap?: number; /** diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index 3a5acc702..ce2c7df7c 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -2047,7 +2047,7 @@ describe('incomplete walks', () => { // d.txt is there, past the cap: unverified by this pass, and kept. assert.equal(pass.reports.get('d.txt')?.state, 'unverified'); assert.notEqual(env.store.treeGet(TREE, 'd.txt'), null); - assert.ok(pass.incomplete.some((r: { reason: string }) => /file cap \(2\) was reached/.test(r.reason))); + assert.ok(pass.incomplete.some((r: { reason: string }) => /walk cap \(2\) was reached/.test(r.reason))); }); }); @@ -2063,7 +2063,7 @@ describe('a mount larger than its walk cap (agent-framework #272)', () => { const first = await call(m, 'sync', {}); assert.deepEqual(syncedNames(env), ['a.txt', 'm.txt'], 'the first two in the walk order'); - assert.ok(first.incomplete.some((r: { reason: string }) => /file cap \(2\) was reached.*resumes after work\/m\.txt/.test(r.reason)), + assert.ok(first.incomplete.some((r: { reason: string }) => /walk cap \(2\) was reached.*resumes after work\/m\.txt/.test(r.reason)), JSON.stringify(first.incomplete)); await call(m, 'sync', {}); @@ -2234,11 +2234,11 @@ describe('a mount larger than its walk cap (agent-framework #272)', () => { const synced = await call(m, 'sync', {}); const resumes = synced.incomplete.filter((r: { reason: string }) => /resumes after/.test(r.reason)); assert.deepEqual(resumes.map((r: { mount: string; path: string; reason: string }) => [r.mount, r.path, r.reason]), - [['work', '', 'the file cap (1) was reached: the next sync or scan of this scope resumes after work/a.txt']]); + [['work', '', 'the walk cap (1) was reached: the next sync or scan of this scope resumes after work/a.txt']]); // z/q.txt sits beneath z/, which wasn't listed: its reason is z/'s, not // that of the root, which holds an untaken b.txt. const skipped = synced.skipped?.find((x: { path: string }) => x.path === 'z/q.txt'); - assert.match(skipped?.reason ?? '', /not visited: the file cap \(1\) was reached/, JSON.stringify(synced.skipped)); + assert.match(skipped?.reason ?? '', /not visited: the walk cap \(1\) was reached/, JSON.stringify(synced.skipped)); }); test('agent-action scans resume where the last stopped', async (t) => { @@ -2328,7 +2328,7 @@ describe('a mount larger than its walk cap (agent-framework #272)', () => { assert.ok(synced.incomplete?.some((r: { mount: string; path: string; reason: string }) => r.mount === 'work' && r.path === 's' && /changed while the walk ran/.test(r.reason)), JSON.stringify(synced.incomplete)); // The pass took one file of its three, t/x.txt: nothing it left is the cap's. - assert.ok(!synced.incomplete.some((r: { reason: string }) => /file cap/.test(r.reason)), JSON.stringify(synced.incomplete)); + assert.ok(!synced.incomplete.some((r: { reason: string }) => /walk cap/.test(r.reason)), JSON.stringify(synced.incomplete)); }); assert.ok(swapped); assert.notEqual(env.store.treeGet(TREE, 's/z.txt'), null, 's/z.txt is on disk in the new s/: not proven absent'); @@ -2402,7 +2402,7 @@ describe('a mount larger than its walk cap (agent-framework #272)', () => { const synced = await call(m, 'sync', { path: 'work/d' }); assert.ok(synced.incomplete?.some((r: { path: string; reason: string }) => /changed while the walk ran/.test(r.reason) && /(^|\/)d$/.test(r.path)), JSON.stringify(synced.incomplete)); - assert.ok(!synced.incomplete.some((r: { reason: string }) => /file cap/.test(r.reason)), JSON.stringify(synced.incomplete)); + assert.ok(!synced.incomplete.some((r: { reason: string }) => /walk cap/.test(r.reason)), JSON.stringify(synced.incomplete)); }); assert.ok(swapped); }); @@ -2434,6 +2434,79 @@ describe('a mount larger than its walk cap (agent-framework #272)', () => { assert.notEqual(env.store.treeGet(TREE, 'd/u/q.txt'), null, 'd/u/q.txt is on disk in the new d/: not proven absent'); }); + test('a directory listed on the after stretch that is a symlink on the wrap proves nothing absent from its old listing', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 3 }); + for (const f of ['s/a.txt', 's/b.txt', 's/c.txt', 's/z.txt', 't/x.txt']) await seedSynced(env, m, f, f); + await call(m, 'sync', {}); // takes s/a, s/b, s/c: the place is s/c.txt + env.rmDisk('s/z.txt'); + // While t/ is listed, s/ becomes a link to a directory that holds z.txt. + // The wrap's check of s/ fails, so its listing from the after stretch + // describes a directory that isn't there. + let swapped = false; + await withFsPromises('readdir', (readdir) => async (path: string, ...rest: unknown[]) => { + if (!swapped && String(path).endsWith(`${'/'}t`)) { + swapped = true; + renameSync(env.disk('s'), join(env.root, 's-away')); + writeFileSync(join(env.root, 's-away', 'z.txt'), 's/z.txt'); + symlinkSync(join(env.root, 's-away'), env.disk('s')); + } + return readdir(path, ...rest); + }, () => call(m, 'sync', {})); + assert.ok(swapped); + assert.notEqual(env.store.treeGet(TREE, 's/z.txt'), null, "s/'s old listing proves nothing once s/ isn't that directory"); + }); + + test('a directory proven missing on the after stretch and back by the wrap proves nothing absent', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 2 }); + for (const f of ['m/1.txt', 'm/2.txt', 'm/3.txt', 'z/q.txt']) await seedSynced(env, m, f, f); + await call(m, 'sync', {}); // takes m/1, m/2: the place is m/2.txt + // The root's listing shows m/, which is gone by the time the after + // stretch checks it, so the walk proves m/ missing; while it lists z/, + // m/ comes back. + let removed = false; + let restored = false; + await withFsPromises('readdir', (readdir) => async (path: string, ...rest: unknown[]) => { + if (!removed && String(path) === realpathSync(env.dir)) { + const listed = await readdir(path, ...rest); + removed = true; + rmSync(env.disk('m'), { recursive: true }); + return listed; + } + if (!restored && String(path).endsWith(`${'/'}z`)) { + restored = true; + for (const f of ['m/1.txt', 'm/2.txt', 'm/3.txt']) env.writeDisk(f, f); + } + return readdir(path, ...rest); + }, () => call(m, 'sync', {})); + assert.ok(removed && restored); + // The wrap lists the m/ there now and takes m/1.txt from it. + for (const f of ['m/1.txt', 'm/2.txt', 'm/3.txt']) assert.notEqual(env.store.treeGet(TREE, f), null, `${f} is on disk`); + }); + + test('a link takes a place under the cap as a file does, so a followed link past the cap is read on a later pass', async (t) => { + const env = new Env(t); + for (const f of ['zz/t1.txt', 'zz/t2.txt', 'zz/t3.txt']) env.writeDisk(f, `target ${f}`); + env.writeDisk('a.txt', 'a'); + for (const n of ['1', '2', '3']) symlinkSync(env.disk(`zz/t${n}.txt`), env.disk(`l${n}`)); + const m = await env.open({ walkCap: 2, followSymlinks: true }); + // The root's entries in order: a.txt, l1, l2, l3; the cap takes a.txt and l1. + const first = await call(m, 'sync', {}); + assert.deepEqual(syncedNames(env), ['a.txt', 'l1'], JSON.stringify(first)); + assert.ok(first.incomplete.some((r: { path: string; reason: string }) => r.path === '' && /before everything here was taken/.test(r.reason)), + JSON.stringify(first.incomplete)); + // Like a file past the cap, an untracked link the pass didn't take isn't reported by path. + assert.ok(!(first.skipped ?? []).some((x: { path: string }) => x.path === 'l2' || x.path === 'l3'), JSON.stringify(first.skipped)); + await call(m, 'sync', {}); // l2 and l3: the place is l3 + assert.deepEqual(syncedNames(env), ['a.txt', 'l1', 'l2', 'l3']); + // A tracked link the next pass doesn't take isn't read through either: + // the third pass takes zz/t1.txt and zz/t2.txt, not l3. + env.writeDisk('zz/t3.txt', 'changed behind l3'); + await call(m, 'sync', {}); + assert.equal(await contentOf(m, 'l3'), 'target zz/t3.txt'); + }); + test('an accepted new root starts with no place', async (t) => { const env = new Env(t); for (const f of ['a.txt', 'b.txt', 'c.txt']) env.writeDisk(f, f); From 9ab96788af17d3afc38587b4c1cf43b7504ba244 Mon Sep 17 00:00:00 2001 From: Gladys-2413 Date: Wed, 14 Oct 2026 11:17:06 -0400 Subject: [PATCH 5/7] fix(workspace): each check of a directory replaces its own missing proof, and a tracked link the cap leaves is the cap's (#272) Beth-2389 read b295e962, found three cases with probes, and wrote the lines that fix them: - A check of a subdirectory cleared the missing proofs at and beneath it only when it found a directory. A directory proven missing on the after stretch that the wrap found as a link kept its own proof, so the pass proved the files through the link absent and removed them. This predates b295e962. A subdirectory proven missing beneath a directory that is still the same one lost its proof when the walk checked the parent again, so the pass reported it as not visited for the cap and kept a deleted file. This one is new in b295e962. Each check now deletes the proof of the directory it checks, before its outcome, and nothing beneath it. What the walk proved beneath a directory that is still the same one stands, and list()'s identity check drops it from one that isn't. - A tracked link the cap left took the reason of its own symlink region, "a symlink, not walked", because that region was the nearest one holding it, and the region listed the link under incomplete. A symlink region is about what lies beneath the link. The nearest-region lookup now skips a symlink region at the path itself, so the link gets its directory's cap reason as a tracked file does, and a symlink region hides only paths beneath it. A tracked link the pass did take, on a mount that doesn't follow links, now appears under skipped alone, and no longer under incomplete as well. - The place is an entry, not a file, in the doc of disk-agreement's walk entry, walkScope's doc, PassOptions.resume and the changelog. gather's comment on candidates no longer says its sentence about ignored names twice. Tests in workspace-three-way (201), Beth's three probes: - a directory proven missing on the after stretch that is an in-mount link on the wrap proves nothing absent; - a subdirectory proven missing on the after stretch stays missing when its parent is the same directory on the wrap, so the pass reports nothing for the cap and the deleted file goes; - a tracked link the cap leaves is unverified for the cap, as a tracked file is, and incomplete lists it only by its directory. On the VPS, tsc is clean and workspace-three-way passes 201/201 on Node 24.21.0 and 20.20.2. Without the per-check delete, two tests fail. With the old clearing beneath a found directory, two fail. Without the symlink skip in the nearest region, or the symlink exclusion in what a region hides, the link test fails. --- changelog.d/272-walk-cap.fixed.md | 2 +- src/modules/workspace/disk-agreement.ts | 2 +- src/modules/workspace/observe.ts | 11 ++-- src/modules/workspace/reconcile.ts | 21 +++++--- test/workspace-three-way.test.ts | 69 +++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 14 deletions(-) diff --git a/changelog.d/272-walk-cap.fixed.md b/changelog.d/272-walk-cap.fixed.md index 3b54052be..0e5f6f12e 100644 --- a/changelog.d/272-walk-cap.fixed.md +++ b/changelog.d/272-walk-cap.fixed.md @@ -1,4 +1,4 @@ -- 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 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 file 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 and scans (#272). Each mount sets its cap with `walkCap`, a positive 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 walk takes each directory's files 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. diff --git a/src/modules/workspace/disk-agreement.ts b/src/modules/workspace/disk-agreement.ts index 373a28fdd..3ae6034e9 100644 --- a/src/modules/workspace/disk-agreement.ts +++ b/src/modules/workspace/disk-agreement.ts @@ -94,7 +94,7 @@ type JournalEntry = | { t: 'set'; mount: string; path: string; value: Physical } | { t: 'forget'; mount: string; path: string } /** - * Where a capped walk of `scope` stopped (the last file it took), or null + * Where a capped walk of `scope` stopped (the last entry it took), or null * once a walk covered the scope. It carries no `path` field: agent-framework * 0.22 replays an entry it doesn't know as a `forget` of `entry.path`, and * so leaves the evidence alone when there is none. diff --git a/src/modules/workspace/observe.ts b/src/modules/workspace/observe.ts index 1c846b9fe..bda91a888 100644 --- a/src/modules/workspace/observe.ts +++ b/src/modules/workspace/observe.ts @@ -822,8 +822,8 @@ async function checkChild(view: MountView, rootReal: string, parent: CheckedDir, * proven absent, whichever files were taken. Once the cap is reached, no * further directory is listed. * - * With `resumeAfter` (a file in the scope, from an earlier walk's - * `stoppedAfter`), a recursive walk takes the files after it first, then + * With `resumeAfter` (an entry in the scope, from an earlier walk's + * `stoppedAfter`), a recursive walk takes the entries after it first, then * wraps to the start of the scope, so repeated walks cover a scope larger * than the cap. */ @@ -1002,9 +1002,12 @@ export async function walkScope( if (left === 0) break; if (!dirIn(stretch, sub)) continue; const child = await checkChild(view, rootReal, dir, sub); + // This check replaces what an earlier one proved of `sub` itself, such + // as missing on the after stretch and found again on the wrap. What was + // proven beneath a directory that is still the same one stands, and + // list() drops it from one that isn't. + walk.missing.delete(sub); if (child.kind === 'checked') { - // A directory found again, on the wrap, after it was proven missing. - for (const p of walk.missing) if (p === sub || p.startsWith(sub + '/')) walk.missing.delete(p); await visit(child.dir, stretch); continue; } diff --git a/src/modules/workspace/reconcile.ts b/src/modules/workspace/reconcile.ts index 34e9fd58d..639e923d6 100644 --- a/src/modules/workspace/reconcile.ts +++ b/src/modules/workspace/reconcile.ts @@ -169,7 +169,7 @@ export interface PassOptions { cap?: number; /** * A recursive directory pass of a sync or scan: its walk resumes after the - * last file the previous one of this scope took, and records where it + * 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. */ @@ -633,9 +633,9 @@ async function gather(store: JsStore, agreement: DiskAgreement, mount: MountRunt ...(resumeAfter !== undefined ? { resumeAfter } : {}), }); for (const f of walk.files) if (inScope(f, scope)) candidates.add(f); - // Ignored names are listed only when something tracks them (above). - // The other entries it took (a link, a socket…): ignored names are - // listed only when something tracks them (above). + // The other entries it took (a link, a socket…). Ignored names, and the + // entries the cap left, are listed only when something tracks them + // (above). for (const p of walk.othersTaken) if (inScope(p, scope)) candidates.add(p); for (const d of walk.dirs) if (inScope(d, { kind: 'dir', dir: scope.dir, recursive: false })) gathered.dirs.push(d); } @@ -662,6 +662,8 @@ async function gather(store: JsStore, agreement: DiskAgreement, mount: MountRunt let region: Walk['incomplete'][number] | undefined; for (const r of walk.incomplete) { if (r.path !== '' && path !== r.path && !path.startsWith(r.path + '/')) continue; + // A link's own region is about what lies beneath it, not the link. + if (r.kind === 'symlink' && r.path === path) continue; if (region === undefined || r.path.length > region.path.length) region = r; } gathered.facts.set(path, { kind: 'unobserved', reason: region ? region.reason : 'not reached by the walk' }); @@ -855,14 +857,17 @@ export async function reconcilePass( }); } for (const region of walk.incomplete) { - const hides = tracked.some((p) => p === region.path || p.startsWith(region.path + '/')); + // A link's own region hides only what lies beneath it: the link itself + // is reported by path, with its own reason. + const hides = tracked.some((p) => (region.kind !== 'symlink' && p === region.path) || p.startsWith(region.path + '/')); if (region.kind === 'error' || region.kind === 'cap' || hides) result.incomplete.push({ path: region.path, reason: region.reason }); // An ignored name that hides nothing tracked is reported nowhere else: // counted, so a sync that says 0 also says what it didn't look at - // (agent-framework #276). A symlink region isn't counted: the link - // itself is a candidate the pass reports by path, taken through the + // (agent-framework #276). A symlink region isn't counted: a link the + // walk took is a candidate the pass reports by path, taken through the // link when it's a file on a mount that follows symlinks, and - // otherwise listed under `skipped`. + // otherwise listed under `skipped`; a tracked link the cap left is + // unverified for the cap. else if (region.kind === 'ignored') result.passedOver.set(region.reason, (result.passedOver.get(region.reason) ?? 0) + 1); } } diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index ce2c7df7c..22561ec16 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -2507,6 +2507,75 @@ describe('a mount larger than its walk cap (agent-framework #272)', () => { assert.equal(await contentOf(m, 'l3'), 'target zz/t3.txt'); }); + test('a directory proven missing on the after stretch that is an in-mount link on the wrap proves nothing absent', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 2 }); + for (const f of ['m/1.txt', 'm/2.txt', 'm/3.txt', 'z/q.txt']) await seedSynced(env, m, f, f); + await call(m, 'sync', {}); // takes m/1, m/2: the place is m/2.txt + // After the root is listed, m/ moves to m-away/ (in the mount), so the + // after stretch proves m/ missing; while z/ is listed, m becomes a link + // to m-away/, which holds all three files. + let moved = false; + let linked = false; + await withFsPromises('readdir', (readdir) => async (path: string, ...rest: unknown[]) => { + if (!moved && String(path) === realpathSync(env.dir)) { + const listed = await readdir(path, ...rest); + moved = true; + renameSync(env.disk('m'), env.disk('m-away')); + return listed; + } + if (!linked && String(path).endsWith(`${'/'}z`)) { + linked = true; + symlinkSync(env.disk('m-away'), env.disk('m')); + } + return readdir(path, ...rest); + }, () => call(m, 'sync', {})); + assert.ok(moved && linked); + for (const f of ['m/1.txt', 'm/2.txt', 'm/3.txt']) assert.notEqual(env.store.treeGet(TREE, f), null, `${f} is on disk through the link m: not proven absent`); + }); + + test('a subdirectory proven missing on the after stretch stays missing when its parent is the same directory on the wrap', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 2 }); + for (const f of ['d/a.txt', 'd/b.txt', 'd/q/x.txt']) await seedSynced(env, m, f, f); + await call(m, 'sync', {}); // takes d/a, d/b: the place is d/b.txt; d/q is left + env.rmDisk('d/a.txt'); // so the next pass takes one of its two + // d/ is listed with q/ in it; q/ is gone by the time the after stretch + // checks it, so the walk proves d/q missing. d/ itself is untouched. + let removed = false; + let synced: any; + await withFsPromises('readdir', (readdir) => async (path: string, ...rest: unknown[]) => { + if (!removed && String(path).endsWith(`${'/'}d`)) { + const listed = await readdir(path, ...rest); + removed = true; + rmSync(env.disk('d/q'), { recursive: true }); + return listed; + } + return readdir(path, ...rest); + }, async () => { + synced = await call(m, 'sync', {}); + }); + assert.ok(removed); + assert.ok(!(synced.incomplete ?? []).some((r: { reason: string }) => /walk cap/.test(r.reason)), `the pass took one of its two: ${JSON.stringify(synced.incomplete)}`); + assert.equal(env.store.treeGet(TREE, 'd/q/x.txt'), null, 'd/q was proven missing: d/q/x.txt is gone from disk'); + }); + + test('a tracked link the cap leaves is unverified for the cap, as a tracked file is, and listed under incomplete only by its directory', async (t) => { + const env = new Env(t); + for (const f of ['zz/t1.txt', 'zz/t2.txt', 'zz/t3.txt']) env.writeDisk(f, `target ${f}`); + env.writeDisk('a.txt', 'a'); + for (const n of ['1', '2', '3']) symlinkSync(env.disk(`zz/t${n}.txt`), env.disk(`l${n}`)); + const m = await env.open({ walkCap: 2, followSymlinks: true }); + await call(m, 'sync', {}); // a.txt, l1 + await call(m, 'sync', {}); // l2, l3: the place is l3 + const third = await call(m, 'sync', {}); // zz/t1.txt, zz/t2.txt + for (const p of ['a.txt', 'l1', 'l2', 'l3']) { + const reason = (third.skipped ?? []).find((x: { path: string }) => x.path === p)?.reason; + assert.match(reason ?? '', /walk cap \(2\) was reached/, `${p}: ${reason}`); + } + assert.ok(!(third.incomplete ?? []).some((r: { path: string }) => ['l1', 'l2', 'l3'].includes(r.path)), JSON.stringify(third.incomplete)); + }); + test('an accepted new root starts with no place', async (t) => { const env = new Env(t); for (const f of ['a.txt', 'b.txt', 'c.txt']) env.writeDisk(f, f); From 5222beaef9955e47c27244e259016a00f3227202 Mon Sep 17 00:00:00 2001 From: Gladys-2413 Date: Wed, 14 Oct 2026 11:31:19 -0400 Subject: [PATCH 6/7] fix(workspace): a replaced directory's names and regions go with it, and a directory gone when listed is listed afresh on the wrap (#272) An internal review of b295e962 found these. This message leaves out its findings that 9ab96788 already fixed. - dropBeneath took back a replaced directory's untaken files and subdirectories. It took back its untaken links and other entries only through the listing's leaves, and kept its ignored names and the symlink and ignored regions its listings gave rise to. So a stale untaken link counted as the cap's, and a stale ignored name as passed over. It now drops every untaken name beneath the directory from Walk.others, and every symlink or ignored region beneath it. - A directory gone when the walk read its listing (ENOENT, proven missing) kept a null listing, so the wrap, finding it again, never listed it. The walk now removes its cache entry with the proof, so a later check lists it afresh. - The sync description says the cap counts "entries, files, links and the like". The changelog says a walk takes each directory's entries that aren't directories by name before its subdirectories. The walkCap doc and the changelog say the integer is at most Number.MAX_SAFE_INTEGER. - The link test asserts that its third pass took zz/t1.txt and zz/t2.txt. One finding is left as it is. A directory recreated with a recycled inode passes the identity check here, as it does for every listing in a walk, and the walk's absence proofs rest on that identity throughout. Tests in workspace-three-way (203): - a replaced directory's untaken link and ignored name no longer count as the cap's or as passed over; - a directory gone when the after stretch lists it, and back by the wrap, is listed afresh there, and the pass takes its edit. On the VPS, tsc is clean and workspace-three-way passes 203/203 on Node 24.21.0. Each of the three new pieces, removed on its own, fails its test. --- changelog.d/272-walk-cap.fixed.md | 4 +-- src/modules/workspace/index.ts | 2 +- src/modules/workspace/observe.ts | 24 +++++++++---- src/modules/workspace/types.ts | 3 +- test/workspace-three-way.test.ts | 56 +++++++++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 11 deletions(-) diff --git a/changelog.d/272-walk-cap.fixed.md b/changelog.d/272-walk-cap.fixed.md index 0e5f6f12e..f2c99e57b 100644 --- a/changelog.d/272-walk-cap.fixed.md +++ b/changelog.d/272-walk-cap.fixed.md @@ -1,5 +1,5 @@ -- 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 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 walk takes each directory's files 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. +- 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 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. - 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. diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index 18667b426..3be26091f 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -1224,7 +1224,7 @@ 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 files and links (5000 unless configured); past it, what this one didn\'t reach is listed under ' + + '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 ' + 'path (file or directory), ' + 'the workspace takes disk\'s state there explicitly: it restores a workspace-deleted file, resolves ' diff --git a/src/modules/workspace/observe.ts b/src/modules/workspace/observe.ts index bda91a888..3f2485ba5 100644 --- a/src/modules/workspace/observe.ts +++ b/src/modules/workspace/observe.ts @@ -884,19 +884,24 @@ export async function walkScope( * now. What was taken stays taken: the pass observes it by path. */ const dropBeneath = (rel: string): void => { + const beneath = (path: string): boolean => rel === '' || path.startsWith(rel + '/'); for (const [path, listing] of listings) { - if (rel !== '' && path !== rel && !path.startsWith(rel + '/')) continue; + if (path !== rel && !beneath(path)) continue; if (listing) { - for (const [leaf, isFile] of listing.leaves) { - if (isFile) walk.seen.delete(leaf); - else if (!walk.othersTaken.has(leaf)) walk.others.delete(leaf); - } + for (const [leaf, isFile] of listing.leaves) if (isFile) walk.seen.delete(leaf); for (const sub of listing.subdirs) walk.dirs.delete(sub); } walk.complete.delete(path); listings.set(path, null); } - for (const path of walk.missing) if (rel === '' || path.startsWith(rel + '/')) walk.missing.delete(path); + // The links, ignored names and other entries those listings showed, and + // the regions they gave rise to, described the directory that was there. + for (const [path] of walk.others) if (beneath(path) && !walk.othersTaken.has(path)) walk.others.delete(path); + for (let i = walk.incomplete.length - 1; i >= 0; i--) { + const region = walk.incomplete[i]!; + if ((region.kind === 'symlink' || region.kind === 'ignored') && beneath(region.path)) walk.incomplete.splice(i, 1); + } + for (const path of walk.missing) if (beneath(path)) walk.missing.delete(path); }; const list = async (dir: CheckedDir): Promise => { @@ -923,7 +928,12 @@ export async function walkScope( // Gone since it was checked: absent beneath only if that can be shown. const proof = rel === '' ? ROOT_UNAVAILABLE : await confirmAbsent(view, rootReal, rel, 'beneath'); if (typeof proof === 'string') fail(rel, proof); - else walk.missing.add(rel); + else { + walk.missing.add(rel); + // Nothing was listed, so a later check that finds it again, on the + // wrap, lists it afresh. + listings.delete(rel); + } return null; } fail(rel, `cannot list (${code ?? String(err)})`); diff --git a/src/modules/workspace/types.ts b/src/modules/workspace/types.ts index 8dcfcb4a2..369ba3c40 100644 --- a/src/modules/workspace/types.ts +++ b/src/modules/workspace/types.ts @@ -44,7 +44,8 @@ export interface MountConfig { maxFileSize?: number; /** * The most entries one walk of this mount takes (default: 5000), a - * positive integer: files, and links or other entries that aren't + * 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 diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index 22561ec16..8010039d4 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -2504,6 +2504,7 @@ describe('a mount larger than its walk cap (agent-framework #272)', () => { // the third pass takes zz/t1.txt and zz/t2.txt, not l3. env.writeDisk('zz/t3.txt', 'changed behind l3'); await call(m, 'sync', {}); + assert.deepEqual(syncedNames(env), ['a.txt', 'l1', 'l2', 'l3', 'zz/t1.txt', 'zz/t2.txt'], 'the third pass took zz/t1.txt and zz/t2.txt'); assert.equal(await contentOf(m, 'l3'), 'target zz/t3.txt'); }); @@ -2576,6 +2577,61 @@ describe('a mount larger than its walk cap (agent-framework #272)', () => { assert.ok(!(third.incomplete ?? []).some((r: { path: string }) => ['l1', 'l2', 'l3'].includes(r.path)), JSON.stringify(third.incomplete)); }); + test("a replaced directory's untaken links and ignored names are no longer counted, as the cap's or as passed over", async (t) => { + const env = new Env(t); + for (const f of ['s/a.txt', 's/b.txt', 's/c.txt', 's/x.log', 't/x.txt']) env.writeDisk(f, f); + symlinkSync(env.disk('t/x.txt'), env.disk('s/aa-link')); + const m = await env.open({ walkCap: 3, ignore: ['*.log'] }); + await call(m, 'sync', {}); // s/a.txt, s/aa-link, s/b.txt: the place is s/b.txt + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', ''), 's/b.txt'); + // The after stretch takes s/c.txt and lists t/, while s/ is replaced; the + // wrap then meets another s/, so the old listing's a.txt, aa-link and + // x.log (ignored) describe nothing there. + let swapped = false; + let synced: any; + await withFsPromises('readdir', (readdir) => async (path: string, ...rest: unknown[]) => { + if (!swapped && String(path).endsWith(`${'/'}t`)) { + swapped = true; + renameSync(env.disk('s'), join(env.root, 's-away')); + for (const f of ['s/a.txt', 's/b.txt', 's/c.txt']) env.writeDisk(f, f); + } + return readdir(path, ...rest); + }, async () => { + synced = await call(m, 'sync', {}); + }); + assert.ok(swapped); + assert.ok(synced.incomplete.some((r: { path: string; reason: string }) => r.path === 's' && /changed while the walk ran/.test(r.reason)), JSON.stringify(synced.incomplete)); + // The pass took two entries of its three: nothing is the cap's. + assert.ok(!synced.incomplete.some((r: { reason: string }) => /walk cap/.test(r.reason)), JSON.stringify(synced.incomplete)); + assert.equal(synced.passedOver, undefined, JSON.stringify(synced.passedOver)); + }); + + test('a directory gone when the after stretch lists it, and back by the wrap, is listed afresh there', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 2 }); + for (const f of ['m/1.txt', 'm/2.txt', 'm/3.txt', 'z/q.txt']) await seedSynced(env, m, f, f); + await call(m, 'sync', {}); // takes m/1, m/2: the place is m/2.txt + // m/ is there when it's checked and gone when it's listed, so the after + // stretch proves it missing; while z/ is listed, it comes back, with an + // edit to m/1.txt the wrap takes. + let removed = false; + let restored = false; + await withFsPromises('readdir', (readdir) => async (path: string, ...rest: unknown[]) => { + if (!removed && String(path).endsWith(`${'/'}m`)) { + removed = true; + rmSync(env.disk('m'), { recursive: true }); + } + if (!restored && String(path).endsWith(`${'/'}z`)) { + restored = true; + env.writeDisk('m/1.txt', 'm/1.txt edited while it was gone'); + for (const f of ['m/2.txt', 'm/3.txt']) env.writeDisk(f, f); + } + return readdir(path, ...rest); + }, () => call(m, 'sync', {})); + assert.ok(removed && restored); + assert.equal(await contentOf(m, 'm/1.txt'), 'm/1.txt edited while it was gone'); + }); + test('an accepted new root starts with no place', async (t) => { const env = new Env(t); for (const f of ['a.txt', 'b.txt', 'c.txt']) env.writeDisk(f, f); From 036077969a0d684e6c90d6c50d85a9f282e943f4 Mon Sep 17 00:00:00 2001 From: Gladys-2413 Date: Wed, 14 Oct 2026 11:52:43 -0400 Subject: [PATCH 7/7] fix(workspace): reading a directory replaces an earlier proof that it was missing (#272) Beth-2389 read 5222beae, found this with a probe, and wrote the line that fixes it. Since 5222beae, the wrap reads afresh a directory that the after stretch proved missing when its listing hit ENOENT. A subdirectory gets a check first, and that check's delete drops the proof. A directory sync's own directory gets no check on the wrap, so its missing proof stood. When another directory had taken its place, the fresh read recorded "the directory changed while the walk ran", and provenAbsent still proved every tracked file beneath it absent, so the pass removed them. list() now deletes the directory's missing proof as it starts a read. A read is a check of the directory too, and its outcome, listed, changed or missing again, is what counts. For a subdirectory it repeats the check's own delete. Test in workspace-three-way (204), Beth's probe: a directory sync whose own directory is gone when the after stretch lists it, and another by the wrap, proves nothing absent and reports the directory changed. On the VPS, tsc is clean and workspace-three-way passes 204/204 on Node 24.21.0 and 20.20.2. Without the new delete, the test fails. --- src/modules/workspace/observe.ts | 4 ++++ test/workspace-three-way.test.ts | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/modules/workspace/observe.ts b/src/modules/workspace/observe.ts index 3f2485ba5..050d1bbb3 100644 --- a/src/modules/workspace/observe.ts +++ b/src/modules/workspace/observe.ts @@ -919,6 +919,10 @@ export async function walkScope( return null; } listings.set(rel, null); + // Reading a directory is a check of it too, and replaces an earlier proof + // that it was missing: the scope's own directory, read afresh on the + // wrap, has no check of its own. + walk.missing.delete(rel); let entries; try { entries = await readdir(dir.real, { withFileTypes: true }); diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index 8010039d4..35d8a5ca0 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -2632,6 +2632,32 @@ describe('a mount larger than its walk cap (agent-framework #272)', () => { assert.equal(await contentOf(m, 'm/1.txt'), 'm/1.txt edited while it was gone'); }); + test('a directory sync whose own directory is gone when the after stretch lists it, and another by the wrap, proves nothing absent', async (t) => { + const env = new Env(t); + const m = await env.open({ walkCap: 2 }); + for (const f of ['d/1.txt', 'd/2.txt', 'd/3.txt']) await seedSynced(env, m, f, f); + await call(m, 'sync', { path: 'work/d' }); // takes d/1, d/2: the place is d/2.txt + assert.equal((m as any).agreementOrThrow().walkResumesAfter('work', 'd'), 'd/2.txt'); + // d/ is there when the walk checks it and gone when the after stretch + // lists it, so the walk proves it missing; by the wrap's listing another + // d/ holds the same three files. + let calls = 0; + let synced: any; + await withFsPromises('readdir', (readdir) => async (path: string, ...rest: unknown[]) => { + if (String(path).endsWith(`${'/'}d`)) { + calls++; + if (calls === 1) renameSync(env.disk('d'), join(env.root, 'd-away')); + if (calls === 2) for (const f of ['d/1.txt', 'd/2.txt', 'd/3.txt']) env.writeDisk(f, f); + } + return readdir(path, ...rest); + }, async () => { + synced = await call(m, 'sync', { path: 'work/d' }); + }); + assert.ok(calls >= 2, 'the wrap listed d/ again'); + for (const f of ['d/1.txt', 'd/2.txt', 'd/3.txt']) assert.notEqual(env.store.treeGet(TREE, f), null, `${f} is on disk in the new d/: not proven absent`); + assert.ok(synced.incomplete.some((r: { path: string; reason: string }) => r.path === 'd' && /changed while the walk ran/.test(r.reason)), JSON.stringify(synced.incomplete)); + }); + test('an accepted new root starts with no place', async (t) => { const env = new Env(t); for (const f of ['a.txt', 'b.txt', 'c.txt']) env.writeDisk(f, f);