From bfdd8055099e3f2b7d1939637c1f41b62bb836b1 Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 11:30:38 -0400 Subject: [PATCH 01/14] fix(workspace): a nested mount is its directory wherever disk puts it, not only where a path names it The nested-mount checks (#460) read paths and roots lexically, so an outer mount reached an inner mount's directory three ways it couldn't see: - A root configured through a symlink, or beneath one, wasn't nested at all. The outer mount's sync took the inner mount's files in, and a read of `outer/a/b/x.txt` returned the inner's file. That held both for an inner mount configured through a link into the outer one, and for an outer mount configured through a link to the directory holding the inner one. - On a mount that follows symlinks, a link into the inner mount's directory passed every check. A read through it returned the inner's file and stored it as the outer's. A forced push through a directory link wrote a new file into the inner mount, and a forced unlink removed the inner's file. A forced push through a file link overwrote the inner's file. - On a mount that doesn't follow symlinks, #468 refuses every push, unlink and disk read through such a link. A write through it still stored a draft that no push could write. The changes: - Each mount's root is resolved once, at construction (realRoot, realpathSync.native, the same resolution as the async realpath the checks use). A root not made yet takes its nearest existing directory's real path, with the rest as configured. A mount is nested in another where its configured path or its real root lies beneath the other's, and each way it lies there is a spelling of its directory (#525's model), passed over by every walk and push and refused by the tools' lexical check. spellNestedDirs respells each spelling found at construction (`fromRoots`), not only the first. - MountView's nested mounts carry their real root. Every check that resolves a path canonically also asks nestedAtReal: locateDir, parentPlacement, observePath's followed link and openContained in observe.ts; checkOpened and unlinkContained's followed link in effects.ts; and openContainedFile in index.ts. A path whose real location is in a nested mount's directory is refused there, as `"work/l" leads into mount "inner", nested in mount "work": use "inner/sub" in its place`. createDir asks it before each mkdir, so no directory is made where a nested root, made or not, would be (Greptile's G3 on #282). - parseFilePath, on a mount with a mount nested in it, resolves the path (its nearest existing directory's real path, with the rest as given) and refuses one that lands in a nested mount's directory before anything is stored. On a mount that follows links the refusal is NestedMountError's, with "leads into" in place of "lies in". On one that doesn't, it's the link's own refusal (linkWords, as #468's readers and pushes give it), with its code. It runs before the mount's turn, since the checks where disk is acted on are the boundary (Ada-1017, room-203 #145666), and a mount with nothing nested skips it. - readFileFromDisk passes a WorkspaceReadError from parsing through with its own code, rather than reading every parse failure as `traversal`. - The watcher watches and reads its events against the resolved root: a root configured as `./m` matched none of chokidar's normalized paths, so the mount heard nothing. Tests: test/workspace-nested-mount-roots.test.ts, sixteen. They cover the three root configurations; Esther-2584's link into the nested directory, for reads, writes, the disk readers and a named sync; Ada-1017's link to a directory holding the nested mount; a file link into it, through a sync, a read and a forced push; #324's alias, through a forced push and the file tools; the disk reader after a link swapped in past the tools' check; a write through an unfollowed link; the four checks where disk is acted on, called directly (the contained write and unlink are #282's 577258f4, by Ada-1017 at slimepriestess's ask); and a `./m` watcher. Co-Authored-By: Claude Opus 5.5 --- src/modules/workspace/effects.ts | 13 +- src/modules/workspace/index.ts | 153 ++++++--- src/modules/workspace/observe.ts | 60 +++- src/modules/workspace/watcher.ts | 19 +- test/workspace-nested-mount-roots.test.ts | 367 ++++++++++++++++++++++ 5 files changed, 560 insertions(+), 52 deletions(-) create mode 100644 test/workspace-nested-mount-roots.test.ts diff --git a/src/modules/workspace/effects.ts b/src/modules/workspace/effects.ts index bf2a49122..e5a174576 100644 --- a/src/modules/workspace/effects.ts +++ b/src/modules/workspace/effects.ts @@ -53,7 +53,9 @@ import { errno, checkDir, checkRoot, + leadsInto, nameOf, + nestedAtReal, parentOf, } from './observe.js'; @@ -273,7 +275,7 @@ export async function writeContained( let stage = 'write the file'; try { const created = touched; - await checkOpened(view, rootReal, root, parent, target, handle, expect, noFollow, touched); + await checkOpened(view, rootReal, rel, root, parent, target, handle, expect, noFollow, touched); touched = true; // The mode the bytes are written at, and the one the file ends with. let before: number | undefined; @@ -377,6 +379,7 @@ export async function writeContained( async function checkOpened( view: MountView, rootReal: string, + rel: string, root: CheckedDir, parent: CheckedDir, target: string, @@ -391,6 +394,8 @@ async function checkOpened( const real = await realpath(target).catch(() => null); if (real === null) throw new EffectFailed(CHANGED, touched); if (!contained(rootReal, real)) throw new EffectFailed(view.followSymlinks ? 'a symlink that leaves the mount' : OUTSIDE_PARENT, touched); + const holder = nestedAtReal(real, view.nested); + if (holder) throw new EffectFailed(leadsInto(view.name, rel, holder), touched); const named = await stat(real, { bigint: true }).catch(() => null); if (named === null || named.dev !== info.dev || named.ino !== info.ino) throw new EffectFailed(CHANGED, touched); if (!view.followSymlinks) { @@ -459,7 +464,7 @@ export async function chmodContained( let touched = false; let stage = 'set the file\'s mode'; try { - await checkOpened(view, rootReal, root, parent, target, handle, expect, noFollow, false); + await checkOpened(view, rootReal, rel, root, parent, target, handle, expect, noFollow, false); const current = Number((await handle.stat({ bigint: true })).mode & 0o777n); if (from !== undefined && current !== from) throw new EffectFailed(MODE_CHANGED, false); const narrowing = (current & ~mode & 0o777) !== 0; @@ -525,6 +530,10 @@ export async function unlinkContained(view: MountView, rootReal: string, rel: st if (!view.followSymlinks) throw new EffectFailed(NOT_FOLLOWED, false); const real = await realpath(target).catch(() => null); if (real === null || !contained(rootReal, real)) throw new EffectFailed('a symlink that leaves the mount', false); + // One leading into a nested mount's directory is that mount's: the link + // and the file it leads to are left as they are. + const holder = nestedAtReal(real, view.nested); + if (holder) throw new EffectFailed(leadsInto(view.name, rel, holder), false); const through = await stat(real, { bigint: true }).catch(() => null); if (through === null) throw new EffectFailed(CHANGED, false); // A followed link stands for the file it leads to, so it's removed only diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index 420d258a4..cffd0e8fe 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -6,10 +6,10 @@ */ import { isUtf8 } from 'node:buffer'; -import { constants as fsConstants } from 'node:fs'; +import { constants as fsConstants, realpathSync } from 'node:fs'; import type { Stats } from 'node:fs'; import { open, readdir, stat, lstat, realpath } from 'node:fs/promises'; -import { isAbsolute, join, resolve, relative, sep } from 'node:path'; +import { basename, dirname, isAbsolute, join, resolve, relative, sep } from 'node:path'; import type { JsStore } from '@animalabs/chronicle'; import type { Module, ModuleContext, ProcessState, EventResponse } from '../../types/module.js'; import type { ProcessEvent, ToolDefinition, ToolCall, ToolResult } from '../../types/events.js'; @@ -40,7 +40,7 @@ import { MountWatcher, type FsChange } from './watcher.js'; import { hashContent, DEFAULT_MAX_FILE_SIZE } from './sync.js'; import { DiskAgreement, sameRootIdentity } from './disk-agreement.js'; import { BranchIntents } from './branch-intent.js'; -import { IGNORED, NONBLOCK, OTHER_MOUNT, checkDir, diskSpelling, filesystemTrustsCtime, inOtherMount, linkLeads, linkOnTheWay, linkRefusal, linkWords, locateRoot, nestedHolder, passedOverAt, readPrefix, type LinkFound } from './observe.js'; +import { IGNORED, NONBLOCK, OTHER_MOUNT, checkDir, diskSpelling, filesystemTrustsCtime, inOtherMount, leadsInto, linkLeads, linkOnTheWay, linkRefusal, linkWords, locateRoot, nestedAtReal, nestedHolder, passedOverAt, readPrefix, type LinkFound, type NestedMount } from './observe.js'; import { TOOL_BATCH_HOOK_TIMEOUT_MS, ensureStateRegistered } from '../../module-registry.js'; import { type MountRuntime, @@ -166,8 +166,8 @@ export type WorkspaceReadStage = * use there. */ class NestedMountError extends Error { - constructor(path: string, mount: string, holder: string, use: string) { - super(`"${path}" lies in mount "${holder}", nested in mount "${mount}", which neither reads nor writes it: use "${use}".`); + constructor(path: string, mount: string, holder: string, use: string, how: 'lies in' | 'leads into' = 'lies in') { + super(`"${path}" ${how} mount "${holder}", nested in mount "${mount}", which neither reads nor writes it: use "${use}".`); } } @@ -292,6 +292,50 @@ async function linkReadError( return new WorkspaceReadError(code, reason, 'readlink'); } +/** + * `to` beneath `from`, mount-relative and `/`-separated, whatever the + * platform's separator; undefined where it isn't beneath it. Outside, + * `relative` gives a path that climbs with a `..` segment, or an absolute one + * across Windows drives. A name that only begins with two dots, such as + * `..cache`, is inside. + */ +function nestedDir(from: string, to: string): string | undefined { + const rel = relative(from, to); + if (!rel || rel === '..' || rel.startsWith('..' + sep) || isAbsolute(rel)) return undefined; + return rel.split(sep).join('/'); +} + +/** + * A mount root as realpath resolves it, with the links on its way and in it + * taken out: for a root not made yet, its nearest existing directory's real + * path with the rest as configured. Where a directory can't be resolved for + * any other reason, such as a loop or a lack of permission, the path as + * resolved lexically from there. + */ +function realRoot(path: string): string { + const absolute = resolve(path); + try { + return realpathSync.native(absolute); + } catch (err) { + const code = errnoOf(err); + const parent = dirname(absolute); + if ((code !== 'ENOENT' && code !== 'ENOTDIR') || parent === absolute) return absolute; + return join(realRoot(parent), basename(absolute)); + } +} + +/** realRoot for any absolute path, read without holding the process. */ +async function realBeneath(absolute: string): Promise { + try { + return await realpath(absolute); + } catch (err) { + const code = errnoOf(err); + const parent = dirname(absolute); + if ((code !== 'ENOENT' && code !== 'ENOTDIR') || parent === absolute) return absolute; + return join(await realBeneath(parent), basename(absolute)); + } +} + function isContainedPath(root: string, candidate: string): boolean { if (candidate === root) return true; const normalizedRoot = root.endsWith(sep) ? root : root + sep; @@ -733,11 +777,19 @@ export class WorkspaceModule implements Module { private ctimeTrust = new Map(); /** * Each mount's directories that are other mounts nested inside it, with - * each mount's name, in each spelling that names the directory: as - * configured, and as disk spells it where that differs (spellNestedDirs). - * Each spelling is mount-relative and `/`-separated, with its resolved root. + * each mount's name and real root (realRoot), in each spelling that names + * the directory: as configured; as the real roots place it, where that + * differs (`fromRoots`, both found at construction); and as disk spells + * each of those where that differs (spellNestedDirs). Each spelling is + * mount-relative and `/`-separated, with its root beneath the mount's + * resolved root. */ - private otherMounts = new Map }>>(); + private otherMounts = new Map; + spellings: Array<{ dir: string; root: string }>; + }>>(); /** * Whether a file's mode is observed, taken in and applied (MountRuntime's * `modesObserved`): not on Windows, which reports no POSIX modes. A test @@ -766,27 +818,29 @@ export class WorkspaceModule implements Module { // named or not, and in its watcher (OTHER_MOUNT). It's kept apart from // the outer mount's ignore list, which a sync that names a path sets // aside for what it names (#276), so no sync of the outer mount takes in - // the inner one's files. + // the inner one's files. A root configured through a symlink, or beneath + // one, is nested where realpath places it, so each root is resolved + // once, here, before any path is parsed: an inner mount nested only that + // way, and one whose directory the real roots place elsewhere than the + // configured paths do, is passed over at each place. Reading a root that + // stops answering holds the process here, as the watcher's start does + // (#311). + const reals = new Map(config.mounts.map((mount) => [mount.name, realRoot(mount.path)])); for (const outer of config.mounts) { for (const inner of config.mounts) { if (outer === inner) continue; const outerPath = resolve(outer.path); - const innerPath = resolve(inner.path); - const rel = relative(outerPath, innerPath); - // Outside the outer mount, `relative` gives a path that climbs with a - // `..` segment, or an absolute one across Windows drives. A name that - // only begins with two dots, such as `..cache`, is inside it. - if (rel && rel !== '..' && !rel.startsWith('..' + sep) && !isAbsolute(rel)) { - // inner is nested under outer: passed over as exactly that - // directory, read against `/`-separated paths, whatever the - // platform's. - const relPath = rel.split(sep).join('/'); - this.otherMounts.set(outer.name, [...(this.otherMounts.get(outer.name) ?? []), { name: inner.name, spellings: [{ dir: relPath, root: innerPath }] }]); - console.warn( - `[workspace] Mount "${outer.name}" contains mount "${inner.name}" ` + - `at ${relPath}/ — passing over it as another mount's directory`, - ); + const real = reals.get(inner.name)!; + const fromRoots: Array<{ dir: string; root: string }> = []; + for (const dir of [nestedDir(outerPath, resolve(inner.path)), nestedDir(reals.get(outer.name)!, real)]) { + if (dir !== undefined && !fromRoots.some((s) => s.dir === dir)) fromRoots.push({ dir, root: join(outerPath, ...dir.split('/')) }); } + if (fromRoots.length === 0) continue; + this.otherMounts.set(outer.name, [...(this.otherMounts.get(outer.name) ?? []), { name: inner.name, real, fromRoots, spellings: [...fromRoots] }]); + console.warn( + `[workspace] Mount "${outer.name}" contains mount "${inner.name}" ` + + `at ${fromRoots.map((s) => `${s.dir}/`).join(' and ')} — passing over it as another mount's directory`, + ); } } this.config = config; @@ -1857,11 +1911,12 @@ export class WorkspaceModule implements Module { /** * Adds to each nested mount's directory the spelling disk lists it by, - * where that differs from the configuration's. On a disk that folds names, - * the outer mount's walks name the directory as disk does, and so does a - * path parsePath has respelled, while a path given as configured - * still names it as configured (resolveAbsolutePath). A directory not on - * disk keeps its configured spelling alone. Run at each start. + * where that differs from those found at construction (`fromRoots`). On a + * disk that folds names, the outer mount's walks name the directory as + * disk does, and so does a path parsePath has respelled, while a path given + * as configured still names it as configured (resolveAbsolutePath). A + * directory not on disk keeps the spellings found at construction alone. + * Run at each start. */ private async spellNestedDirs(): Promise { for (const [outerName, nested] of this.otherMounts) { @@ -1869,16 +1924,19 @@ export class WorkspaceModule implements Module { if (!outer) continue; const outerRoot = resolve(outer.path); for (const entry of nested) { - const configured = entry.spellings[0]!; - const dir = await diskSpelling(outerRoot, configured.dir); - entry.spellings = dir === configured.dir ? [configured] : [configured, { dir, root: join(outerRoot, ...dir.split('/')) }]; + const spellings = [...entry.fromRoots]; + for (const found of entry.fromRoots) { + const dir = await diskSpelling(outerRoot, found.dir); + if (!spellings.some((s) => s.dir === dir)) spellings.push({ dir, root: join(outerRoot, ...dir.split('/')) }); + } + entry.spellings = spellings; } } } - /** The mounts nested in `mountName`, by directory and name (MountView's `nested`), in each spelling known. */ - private nestedOf(mountName: string): { dir: string; name: string }[] { - return (this.otherMounts.get(mountName) ?? []).flatMap((o) => o.spellings.map((s) => ({ dir: s.dir, name: o.name }))); + /** The mounts nested in `mountName`, by directory, name and real root (MountView's `nested`), in each spelling known. */ + private nestedOf(mountName: string): NestedMount[] { + return (this.otherMounts.get(mountName) ?? []).flatMap((o) => o.spellings.map((s) => ({ dir: s.dir, name: o.name, real: o.real }))); } /** @@ -1921,7 +1979,24 @@ export class WorkspaceModule implements Module { * it, naming the innermost mount that holds it and the path to use there. */ private async parseFilePath(path: string): Promise<{ mount: MountState; relativePath: string; path: string }> { - return this.refuseNested(await this.parsePath(path)); + const parsed = this.refuseNested(await this.parsePath(path)); + // A path can also reach a nested mount's directory through a symlink on + // its way. No push of this mount writes it there and no read reads it + // (nestedAtReal), so it's refused here, before anything is stored, and + // the checks where disk is acted on refuse it again. On a mount that + // doesn't follow the link, its refusal is the link's own (linkWords), as + // the readers and pushes give it. A mount with nothing nested has nothing + // to find, so it pays nothing. + const { mount, relativePath } = parsed; + const nested = this.nestedOf(mount.config.name); + if (nested.length === 0) return parsed; + const holder = nestedAtReal(await realBeneath(resolve(mount.config.path, relativePath)), nested); + if (!holder) return parsed; + if (mount.config.followSymlinks !== true) { + const found = await linkOnTheWay({ root: mount.config.path, followSymlinks: false }, relativePath); + if (found !== null) throw await linkReadError(mount, found, parsed.path, nested); + } + throw new NestedMountError(parsed.path, mount.config.name, holder.name, holder.use, 'leads into'); } /** A parsed path, refused if it lies in a nested mount's directory (parseFilePath). */ @@ -2178,6 +2253,8 @@ export class WorkspaceModule implements Module { if (!isContainedPath(realMountRoot, realFilePath)) { throw new WorkspaceReadError('escape', `Symlink escape detected: ${mountPrefixedPath}`, 'realpath'); } + const holder = nestedAtReal(realFilePath, this.nestedOf(mount.config.name)); + if (holder) throw new WorkspaceReadError('other_mount', leadsInto(mount.config.name, relativePath, holder), 'realpath'); let pathStat: Stats; try { @@ -2238,6 +2315,8 @@ export class WorkspaceModule implements Module { try { ({ mount, relativePath, path: mountPrefixedPath } = await this.parseFilePath(mountPrefixedPath)); } catch (err) { + // A link's refusal (parseFilePath) carries its own code. + if (err instanceof WorkspaceReadError) throw err; const message = err instanceof Error ? err.message : String(err); const code: WorkspaceReadErrorCode = err instanceof NestedMountError ? 'other_mount' : message.startsWith('Unknown mount') ? 'unknown_mount' : 'traversal'; diff --git a/src/modules/workspace/observe.ts b/src/modules/workspace/observe.ts index 772e5a181..0b4f4cce2 100644 --- a/src/modules/workspace/observe.ts +++ b/src/modules/workspace/observe.ts @@ -64,13 +64,23 @@ export interface MountView { */ others: readonly string[]; /** - * The same mounts, each by its directory and its name, for a refusal that - * names the mount a symlink leads into. Without it, that refusal names no - * mount. + * The same mounts, each by its directory, its name and its canonical root, + * for a refusal that names the mount a path leads into: through a symlink + * this mount doesn't follow (linkWords), or through one it follows, or a + * root configured through one (nestedAtReal). Without it, no path is + * refused for where its links lead, and a link's refusal names no mount. */ - nested?: readonly { dir: string; name: string }[]; + nested?: readonly NestedMount[]; } +/** + * A mount nested in another: its directory in one of its spellings, + * mount-relative and `/`-separated, its name, and its root as realpath + * resolves it (the nearest existing directory's, where the root isn't made + * yet), which every spelling shares. + */ +export type NestedMount = { dir: string; name: string; real: string }; + export type StatFingerprint = Omit; /** @@ -290,6 +300,31 @@ export async function linkReason(view: Pick contained(n.real, real)) + .sort((a, b) => b.real.length - a.real.length)[0]; + if (!holder) return undefined; + const rest = relative(holder.real, real).split(sep).join('/'); + return { name: holder.name, use: rest ? `${holder.name}/${rest}` : holder.name }; +} + +/** + * Why a mount-relative path that resolves into a nested mount's directory is + * refused (nestedAtReal): what it leads into, and the path to use in its + * place. Spelled as the tools take paths, from the mount's name. + */ +export function leadsInto(name: string, rel: string, holder: { name: string; use: string }): string { + return `"${rel === '' ? name : `${name}/${rel}`}" leads into mount "${holder.name}", nested in mount "${name}": use "${holder.use}" in its place`; +} + // =========================================================================== // Disk's spelling of a path // =========================================================================== @@ -512,6 +547,11 @@ async function locateDir(view: MountView, rootReal: string, rel: string): Promis if (real !== rootReal) return { kind: 'unobserved', reason: ROOT_UNAVAILABLE }; } else if (!contained(rootReal, real)) { return { kind: 'unobserved', reason: OUTSIDE_PARENT, outside: true }; + } else { + // Through a link this mount follows, a directory can be a nested + // mount's, which the mount neither reads nor writes. + const holder = nestedAtReal(real, view.nested); + if (holder) return { kind: 'unobserved', reason: leadsInto(view.name, rel, holder) }; } const checked = await checkedAt(view, rel, real); if (checked) { @@ -572,6 +612,10 @@ async function createDir(view: MountView, rootReal: string, rel: string): Promis if (base === null) return { kind: 'unobserved', reason: ROOT_UNAVAILABLE }; for (let i = depth; i < parts.length; i++) { const childRel = parts.slice(0, i + 1).join('/'); + // Nothing is made in a nested mount's directory, its root included where + // it isn't made yet, by a path that reaches it through a link. + const holder = nestedAtReal(base.at(parts[i]!), view.nested); + if (holder) return { kind: 'unobserved', reason: leadsInto(view.name, childRel, holder) }; try { await mkdir(base.at(parts[i]!)); } catch (err) { @@ -742,7 +786,9 @@ async function parentPlacement(view: MountView, rootReal: string, relativePath: const parent = await realpath(dirname(join(view.root, relativePath))); const found = await linkRefusal(view, rootReal, relativePath, parent, 'parents'); if (found !== null) return { reason: await linkReason(view, rootReal, found) }; - return contained(rootReal, parent) ? 'inside' : { reason: OUTSIDE_PARENT }; + if (!contained(rootReal, parent)) return { reason: OUTSIDE_PARENT }; + const holder = nestedAtReal(parent, view.nested); + return holder ? { reason: leadsInto(view.name, parentOf(relativePath), holder) } : 'inside'; } catch (err) { const code = errno(err); if (code === 'ENOENT' || code === 'ENOTDIR') return 'gone'; @@ -788,6 +834,8 @@ export async function observePath(view: MountView, rootReal: string, relativePat return { kind: 'unobserved', reason: `a symlink that cannot be resolved (${errno(err) ?? String(err)})` }; } if (!contained(rootReal, real)) return { kind: 'unobserved', reason: 'a symlink that leaves the mount' }; + const holder = nestedAtReal(real, view.nested); + if (holder) return { kind: 'unobserved', reason: leadsInto(view.name, relativePath, holder) }; if (!target.isFile()) { return { kind: 'unobserved', reason: target.isDirectory() ? 'a symlink to a directory, not walked' : 'not a regular file' }; } @@ -851,6 +899,8 @@ async function openContained( if (!contained(rootReal, real)) { return { kind: 'unobserved', reason: view.followSymlinks ? 'a symlink that leaves the mount' : OUTSIDE_PARENT }; } + const holder = nestedAtReal(real, view.nested); + if (holder) return { kind: 'unobserved', reason: leadsInto(view.name, relativePath, holder) }; const named = await stat(real, { bigint: true }).catch(() => null); if (named === null || named.dev !== info.dev || named.ino !== info.ino) { return { kind: 'unobserved', reason: 'the file changed while it was read' }; diff --git a/src/modules/workspace/watcher.ts b/src/modules/workspace/watcher.ts index 36d643bcf..efe8dd25f 100644 --- a/src/modules/workspace/watcher.ts +++ b/src/modules/workspace/watcher.ts @@ -7,7 +7,7 @@ import { watch, type FSWatcher } from 'chokidar'; import { mkdirSync, statSync } from 'node:fs'; -import { basename, sep } from 'node:path'; +import { basename, resolve, sep } from 'node:path'; import { passedOverAt } from './observe.js'; import { type MountConfig } from './types.js'; @@ -63,6 +63,12 @@ export class MountWatcher { private stopped = true; private readonly debounceMs: number; private readonly config: MountConfig; + /** + * The root as watched and as each event's path is read against: resolved, + * since chokidar hands back the paths it reports normalized, and those of + * a root configured as `./m` matched none of them. + */ + private readonly root: string; private readonly onChangeCallback: (changes: FsChange[]) => void; private readonly lifecycle: WatcherLifecycle; @@ -74,6 +80,7 @@ export class MountWatcher { private readonly others: readonly string[] = [], ) { this.config = config; + this.root = resolve(config.path); this.debounceMs = config.watchDebounceMs ?? 300; this.onChangeCallback = onChange; this.lifecycle = lifecycle; @@ -108,7 +115,7 @@ export class MountWatcher { const ignore = this.config.ignore ?? []; - this.watcher = watch(this.config.path, { + this.watcher = watch(this.root, { // What a walk passes over by design, by the path relative to the mount: // another mount's directory, the mount's ignore list and editors' swap // and backup files. chokidar 4 @@ -389,9 +396,7 @@ export class MountWatcher { // Chokidar hands back platform-separated absolute paths; compare with // the platform separator and normalize the result to the '/'-separated // logical form the rest of the workspace uses. No-op on POSIX. - const base = this.config.path.endsWith(sep) - ? this.config.path - : this.config.path + sep; + const base = this.root.endsWith(sep) ? this.root : this.root + sep; if (absolutePath.startsWith(base)) { return absolutePath.slice(base.length).split(sep).join('/'); } @@ -399,9 +404,7 @@ export class MountWatcher { } private isRootPath(absolutePath: string): boolean { - const root = this.config.path.endsWith(sep) - ? this.config.path.slice(0, -1) - : this.config.path; + const root = this.root.endsWith(sep) ? this.root.slice(0, -1) : this.root; const candidate = absolutePath.endsWith(sep) ? absolutePath.slice(0, -1) : absolutePath; diff --git a/test/workspace-nested-mount-roots.test.ts b/test/workspace-nested-mount-roots.test.ts new file mode 100644 index 000000000..ceaa50119 --- /dev/null +++ b/test/workspace-nested-mount-roots.test.ts @@ -0,0 +1,367 @@ +/** + * A mount nested in another is that mount's directory wherever disk puts + * it, not only where the outer mount's paths name it as written. Before, the + * nested-mount checks read paths and roots lexically, so the outer mount + * reached the inner one's directory three ways. A root configured through a + * symlink, or beneath one, wasn't nested at all, and the outer mount took in + * and read the inner mount's files. On a mount that follows symlinks, a + * link into the inner mount's directory let the outer mount read, take in, + * write and unlink the inner mount's files through it. And a write through a + * link the mount doesn't follow stored a draft no push could write. Each + * root is now resolved once, at construction, and nested where it really + * is; every check that resolves a path canonically refuses one in a nested + * mount's directory; and the file tools refuse such a path before anything + * is stored. Separately, a mount configured as `./m` now hears its watcher. + */ + +import { describe, test, type TestContext } from 'node:test'; +import assert from 'node:assert/strict'; +import { existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, renameSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { createRequire, syncBuiltinESMExports } from 'node:module'; +import { tmpdir } from 'node:os'; +import { dirname, join, relative } from 'node:path'; +import { JsStore } from '@animalabs/chronicle'; +import { WorkspaceModule, WorkspaceReadError } from '../src/modules/workspace/index.js'; +import { checkDir, readPath, type MountView } from '../src/modules/workspace/observe.js'; +import { EffectFailed, unlinkContained, writeContained } from '../src/modules/workspace/effects.js'; +import { MountWatcher, type FsChange } from '../src/modules/workspace/watcher.js'; +import type { MountConfig } from '../src/modules/workspace/types.js'; +import type { ModuleContext } from '../src/types/module.js'; + +class Env { + readonly root = mkdtempSync(join(tmpdir(), 'af-ws-nested-roots-')); + readonly store: JsStore; + + constructor(t: TestContext) { + this.store = JsStore.openOrCreate({ path: join(this.root, 'ws.chronicle') }); + t.after(() => { + if (!this.store.isClosed()) this.store.close(); + rmSync(this.root, { recursive: true, force: true }); + }); + } + + at(rel: string): string { + return join(this.root, rel); + } + + write(rel: string, content: string): void { + mkdirSync(dirname(this.at(rel)), { recursive: true }); + writeFileSync(this.at(rel), content); + } + + async open(t: TestContext, mounts: MountConfig[]): Promise { + const m = new WorkspaceModule({ mounts }); + m.initStore(this.store); + await m.start({ isRestart: false, getState: () => null, setState: () => {}, pushEvent: () => {} } as unknown as ModuleContext); + t.after(() => m.stop().catch(() => {})); + return m; + } + + /** The paths a mount's tree holds. */ + tree(mount: string): string[] { + return this.store.treeList(`workspace/${mount}/tree`).map((e: { path: string }) => e.path).sort(); + } +} + +function mount(name: string, path: string, extra: Partial = {}): MountConfig { + return { name, path, mode: 'read-write', watch: 'never', autoMaterialize: false, ...extra }; +} + +async function call(m: WorkspaceModule, name: string, input: Record): Promise<{ success: boolean; error?: string; data?: any }> { + return m.handleToolCall({ id: 't', name, input }) as Promise<{ success: boolean; error?: string; data?: any }>; +} + +/** The refusal a file tool gives a path that reaches a nested mount's directory through a link the mount follows. */ +function leadsInto(path: string, outer: string, inner: string, use: string): string { + return `"${path}" leads into mount "${inner}", nested in mount "${outer}", which neither reads nor writes it: use "${use}".`; +} + +/** The skipped reason for a path in a materialize's or a sync's result. */ +function skippedReason(data: any, path: string): string { + const entry = ((data.skipped ?? []) as Array<{ path?: string; reason: string }>) + .find((s) => s.path === path || s.reason.startsWith(`${path}: `)); + assert.ok(entry, `nothing skipped for ${path}: ${JSON.stringify(data)}`); + return entry.reason; +} + +/** Run `fn` with node:fs/promises' `lstat` replaced by `make(original)`, as the code under test sees it. */ +async function withLstat(make: (original: (...args: any[]) => Promise) => (...args: any[]) => Promise, fn: () => Promise): Promise { + const fsp = createRequire(import.meta.url)('node:fs/promises') as Record Promise>; + const original = fsp.lstat!; + fsp.lstat = make(original); + syncBuiltinESMExports(); + try { + return await fn(); + } finally { + fsp.lstat = original; + syncBuiltinESMExports(); + } +} + +describe('a mount root configured through a symlink', () => { + test('an inner mount configured through a link into the outer mount is nested where the link leads', async (t) => { + const env = new Env(t); + env.write('outer/a/b/x.txt', "the inner mount's\n"); + symlinkSync(env.at('outer/a/b'), env.at('via')); + const m = await env.open(t, [mount('outer', env.at('outer')), mount('inner', env.at('via'))]); + assert.equal((await call(m, 'sync', {})).success, true); + assert.deepEqual(env.tree('outer'), [], "the outer mount took none of the inner mount's files in"); + assert.deepEqual(env.tree('inner'), ['x.txt']); + const read = await call(m, 'read', { path: 'outer/a/b/x.txt' }); + assert.equal(read.success, false); + assert.equal(read.error, '"outer/a/b/x.txt" lies in mount "inner", nested in mount "outer", which neither reads nor writes it: use "inner/x.txt".'); + }); + + test('an inner mount configured through a link inside the outer mount is passed over at both of its directories', async (t) => { + // Configured as outer/via, which leads to outer/a/b: the walk reaches + // the inner mount's files under a/b, the real directory, and a path + // written through via names them too. + const env = new Env(t); + env.write('outer/a/b/x.txt', "the inner mount's\n"); + env.write('outer/top.txt', "the outer mount's\n"); + symlinkSync(env.at('outer/a/b'), env.at('outer/via')); + const m = await env.open(t, [mount('outer', env.at('outer')), mount('inner', env.at('outer/via'))]); + assert.equal((await call(m, 'sync', {})).success, true); + assert.deepEqual(env.tree('outer'), ['top.txt'], "the outer mount took none of the inner mount's files in"); + for (const [path, use] of [['outer/a/b/x.txt', 'inner/x.txt'], ['outer/via/x.txt', 'inner/x.txt']] as const) { + const read = await call(m, 'read', { path }); + assert.equal(read.success, false, path); + assert.equal(read.error, `"${path}" lies in mount "inner", nested in mount "outer", which neither reads nor writes it: use "${use}".`); + } + }); + + test('an outer mount configured through a link holds an inner mount configured at its real path', async (t) => { + const env = new Env(t); + env.write('real-outer/a/b/x.txt', "the inner mount's\n"); + symlinkSync(env.at('real-outer'), env.at('outer-link')); + const m = await env.open(t, [mount('outer', env.at('outer-link')), mount('inner', env.at('real-outer/a/b'))]); + assert.equal((await call(m, 'sync', {})).success, true); + assert.deepEqual(env.tree('outer'), []); + const read = await call(m, 'read', { path: 'outer/a/b/x.txt' }); + assert.equal(read.error, '"outer/a/b/x.txt" lies in mount "inner", nested in mount "outer", which neither reads nor writes it: use "inner/x.txt".'); + }); + + test('an inner root not made yet, beneath a link into the outer mount, is nested where it would be made', async (t) => { + const env = new Env(t); + env.write('outer/a/keep.txt', "the outer mount's\n"); + symlinkSync(env.at('outer/a'), env.at('via')); + const m = await env.open(t, [mount('outer', env.at('outer'), { autoMaterialize: true }), mount('inner', env.at('via/newdir'))]); + const write = await call(m, 'write', { path: 'outer/a/newdir/x.txt', content: 'outer wrote\n' }); + assert.equal(write.success, false); + assert.equal(write.error, '"outer/a/newdir/x.txt" lies in mount "inner", nested in mount "outer", which neither reads nor writes it: use "inner/x.txt".'); + assert.equal(existsSync(env.at('outer/a/newdir')), false, "the inner mount's root wasn't made by the outer mount"); + }); +}); + +describe('a link into a nested mount\'s directory, on a mount that follows symlinks', () => { + test('a read or write through it is refused before anything is stored, naming the path to use (Esther-2584\'s case)', async (t) => { + const env = new Env(t); + env.write('work/nested/sub/x.txt', 'inner file\n'); + symlinkSync('nested/sub', env.at('work/l')); + const m = await env.open(t, [ + mount('work', env.at('work'), { followSymlinks: true, autoMaterialize: true }), + mount('inner', env.at('work/nested'), { autoMaterialize: true }), + ]); + const read = await call(m, 'read', { path: 'work/l/x.txt' }); + assert.equal(read.success, false); + assert.equal(read.error, leadsInto('work/l/x.txt', 'work', 'inner', 'inner/sub/x.txt')); + const write = await call(m, 'write', { path: 'work/l/new.txt', content: 'via work\n' }); + assert.equal(write.success, false); + assert.equal(write.error, leadsInto('work/l/new.txt', 'work', 'inner', 'inner/sub/new.txt')); + assert.equal(existsSync(env.at('work/nested/sub/new.txt')), false); + assert.deepEqual(env.tree('work'), [], 'nothing was stored'); + await assert.rejects(m.readFileFromDisk('work/l/x.txt'), (err: unknown) => { + assert.ok(err instanceof WorkspaceReadError, String(err)); + assert.equal(err.code, 'other_mount'); + assert.equal(err.message, leadsInto('work/l/x.txt', 'work', 'inner', 'inner/sub/x.txt')); + return true; + }); + assert.deepEqual(await m.readBinaryFromDisk('work/l/x.txt'), { error: leadsInto('work/l/x.txt', 'work', 'inner', 'inner/sub/x.txt') }); + }); + + test('a sync naming a path through it takes nothing in, and says where the path leads', async (t) => { + const env = new Env(t); + env.write('work/nested/sub/x.txt', 'inner file\n'); + symlinkSync('nested/sub', env.at('work/l')); + const m = await env.open(t, [mount('work', env.at('work'), { followSymlinks: true }), mount('inner', env.at('work/nested'))]); + const sync = await call(m, 'sync', { path: 'work/l/x.txt' }); + assert.equal(sync.success, true, sync.error); + assert.equal(skippedReason(sync.data, 'l/x.txt'), 'disk could not be checked: "work/l" leads into mount "inner", nested in mount "work": use "inner/sub" in its place'); + assert.deepEqual(env.tree('work'), []); + }); + + test('a path through a link to a directory holding the nested mount is refused where it reaches it, and the rest is the outer mount\'s (Ada-1017\'s case)', async (t) => { + const env = new Env(t); + env.write('work/a/b/x.txt', 'inner file\n'); + env.write('work/a/top.txt', 'outer file under a\n'); + symlinkSync('a', env.at('work/l')); + const m = await env.open(t, [ + mount('work', env.at('work'), { followSymlinks: true, autoMaterialize: true }), + mount('inner', env.at('work/a/b'), { autoMaterialize: true }), + ]); + const read = await call(m, 'read', { path: 'work/l/b/x.txt' }); + assert.equal(read.error, leadsInto('work/l/b/x.txt', 'work', 'inner', 'inner/x.txt')); + const top = await call(m, 'read', { path: 'work/l/top.txt' }); + assert.equal(top.success, true, top.error); + assert.match(top.data.content, /outer file under a/); + for (const [path, use] of [['work/l/b/new.txt', 'inner/new.txt'], ['work/l/b/deep/new.txt', 'inner/deep/new.txt']] as const) { + const write = await call(m, 'write', { path, content: 'via work\n' }); + assert.equal(write.error, leadsInto(path, 'work', 'inner', use)); + } + assert.equal(existsSync(env.at('work/a/b/new.txt')), false); + assert.equal(existsSync(env.at('work/a/b/deep')), false); + }); + + test('a file link into it is taken in by no sync, and a copy held from before is neither read nor pushed through it', async (t) => { + const env = new Env(t); + env.write('outer/a/b/x.txt', "the inner mount's\n"); + env.write('outer/flink.txt', "the outer mount's copy\n"); + const m = await env.open(t, [mount('outer', env.at('outer'), { followSymlinks: true }), mount('inner', env.at('outer/a/b'))]); + assert.equal((await call(m, 'sync', {})).success, true); + rmSync(env.at('outer/flink.txt')); + symlinkSync(env.at('outer/a/b/x.txt'), env.at('outer/flink.txt')); + const sync = await call(m, 'sync', {}); + assert.equal(skippedReason(sync.data, 'flink.txt'), 'disk could not be checked: "outer/flink.txt" leads into mount "inner", nested in mount "outer": use "inner/x.txt" in its place'); + const read = await call(m, 'read', { path: 'outer/flink.txt' }); + assert.equal(read.error, leadsInto('outer/flink.txt', 'outer', 'inner', 'inner/x.txt')); + const materialize = await call(m, 'materialize', { path: 'outer/flink.txt', force: true }); + assert.equal(materialize.data.count, 0); + assert.equal(skippedReason(materialize.data, 'flink.txt'), 'flink.txt: not written: "outer/flink.txt" leads into mount "inner", nested in mount "outer": use "inner/x.txt" in its place'); + assert.equal(readFileSync(env.at('outer/a/b/x.txt'), 'utf8'), "the inner mount's\n"); + }); + + test('a directory replaced by a link into it is the nested mount\'s: a forced push of a copy held from before writes nothing (#324)', async (t) => { + const env = new Env(t); + env.write('outer/alias/x.txt', 'old copy\n'); + env.write('outer/a/b/keep.txt', 'inner\n'); + const m = await env.open(t, [mount('outer', env.at('outer'), { followSymlinks: true }), mount('inner', env.at('outer/a/b'))]); + assert.equal((await call(m, 'read', { path: 'outer/alias/x.txt' })).success, true); + rmSync(env.at('outer/alias'), { recursive: true }); + symlinkSync(env.at('outer/a/b'), env.at('outer/alias')); + env.write('outer/a/b/x.txt', 'inner v2\n'); + const materialize = await call(m, 'materialize', { path: 'outer/alias/x.txt', force: true }); + assert.equal(materialize.data.count, 0); + assert.equal(skippedReason(materialize.data, 'alias/x.txt'), 'alias/x.txt: not written: "outer/alias" leads into mount "inner", nested in mount "outer": use "inner" in its place'); + assert.equal(readFileSync(env.at('outer/a/b/x.txt'), 'utf8'), 'inner v2\n'); + for (const [name, input] of [ + ['edit', { path: 'outer/alias/x.txt', oldString: 'old', newString: 'outer-edited' }], + ['write', { path: 'outer/alias/y.txt', content: 'outer wrote y\n' }], + ['delete', { path: 'outer/alias/x.txt' }], + ] as const) { + const res = await call(m, name, input); + assert.equal(res.error, leadsInto(String(input.path), 'outer', 'inner', `inner/${String(input.path).slice('outer/alias/'.length)}`), name); + } + assert.equal(existsSync(env.at('outer/a/b/y.txt')), false); + assert.equal(readFileSync(env.at('outer/a/b/x.txt'), 'utf8'), 'inner v2\n'); + }); + + test('the disk reader refuses a path whose directory became a link into it after the tools\' check', async (t) => { + const env = new Env(t); + env.write('work/l/x.txt', "the outer mount's\n"); + env.write('work/nested/sub/x.txt', 'inner file\n'); + const m = await env.open(t, [mount('work', env.at('work'), { followSymlinks: true }), mount('inner', env.at('work/nested'))]); + let swapped = false; + await withLstat((lstat) => async (path: string, ...rest: unknown[]) => { + // The reader's lstat of the path comes after the tools' check, which + // found it the outer mount's: the directory becomes a link between. + if (!swapped && String(path) === env.at('work/l/x.txt')) { + swapped = true; + renameSync(env.at('work/l'), env.at('work/l-was')); + symlinkSync('nested/sub', env.at('work/l')); + } + return lstat(path, ...rest); + }, async () => { + await assert.rejects(m.readFileFromDisk('work/l/x.txt'), (err: unknown) => { + assert.ok(err instanceof WorkspaceReadError, String(err)); + assert.equal(err.code, 'other_mount'); + assert.equal(err.message, '"work/l/x.txt" leads into mount "inner", nested in mount "work": use "inner/sub/x.txt" in its place'); + return true; + }); + }); + assert.equal(swapped, true); + }); +}); + +describe('a link into a nested mount\'s directory, on a mount that doesn\'t follow symlinks', () => { + test('a write through it is refused before anything is stored, with the link\'s own refusal', async (t) => { + // The readers and pushes give this refusal already (#468). A write used + // to store a draft that no push could write. + const env = new Env(t); + env.write('work/nested/sub/x.txt', 'inner file\n'); + symlinkSync('nested/sub', env.at('work/l')); + const m = await env.open(t, [mount('work', env.at('work')), mount('inner', env.at('work/nested'))]); + const write = await call(m, 'write', { path: 'work/l/new.txt', content: 'via work\n' }); + assert.equal(write.success, false); + assert.equal(write.error, 'work/l/new.txt: "work/l" is a symlink to "work/nested/sub", which this mount does not follow; it lies in mount "inner", nested in mount "work": use "inner/sub" in place of "work/l"'); + assert.deepEqual(env.tree('work'), [], 'nothing was stored'); + }); +}); + +describe('the checks where disk is acted on, called directly as a push and a read call them', () => { + /** A mount following symlinks, with `inner` nested at `a/b` and a file link `flink.txt` into it. */ + function rig(t: TestContext): { env: Env; view: MountView; rootReal: string } { + const env = new Env(t); + env.write('outer/a/b/x.txt', "the inner mount's"); + symlinkSync(env.at('outer/a/b/x.txt'), env.at('outer/flink.txt')); + // Cast, so the file also compiles against code whose nested mounts carry + // no real root, for the run of these tests on it. + const view = { + root: env.at('outer'), name: 'outer', followSymlinks: true, maxFileSize: 1 << 20, ignore: [], others: ['a/b'], + nested: [{ dir: 'a/b', name: 'inner', real: realpathSync(env.at('outer/a/b')) }], + } as MountView; + return { env, view, rootReal: realpathSync(env.at('outer')) }; + } + const reason = '"outer/flink.txt" leads into mount "inner", nested in mount "outer": use "inner/x.txt" in its place'; + + // These two pin the checks #282 added for slimepriestess's review (its + // 577258f4, by Ada-1017), carried here with the reason this change gives. + test('the contained write refuses a file it opens through a link into a nested mount', async (t) => { + const { env, view, rootReal } = rig(t); + const err = await writeContained(view, rootReal, 'flink.txt', Buffer.from('outer wrote'), { kind: 'any' }).then(() => null, (e) => e); + assert.ok(err instanceof EffectFailed, String(err)); + assert.equal(err.message, reason); + assert.equal(readFileSync(env.at('outer/a/b/x.txt'), 'utf8'), "the inner mount's", "the inner mount's file is as it was"); + }); + + test('the contained unlink refuses a final link into a nested mount, and leaves the link and the file', async (t) => { + const { env, view, rootReal } = rig(t); + const err = await unlinkContained(view, rootReal, 'flink.txt', { kind: 'any' }).then(() => null, (e) => e); + assert.ok(err instanceof EffectFailed, String(err)); + assert.equal(err.message, reason); + assert.ok(lstatSync(env.at('outer/flink.txt')).isSymbolicLink(), 'the link is still there'); + assert.equal(readFileSync(env.at('outer/a/b/x.txt'), 'utf8'), "the inner mount's", "the inner mount's file is as it was"); + }); + + test('the located read refuses a file it opens through a link into a nested mount', async (t) => { + const { view, rootReal } = rig(t); + assert.deepEqual(await readPath(view, rootReal, 'flink.txt', 1 << 20), { kind: 'unobserved', reason }); + }); + + test('a directory made for a push isn\'t made where a link leads into a nested mount\'s root not made yet (G3)', async (t) => { + const { env, view, rootReal } = rig(t); + symlinkSync(env.at('outer/a'), env.at('outer/l')); + const later = { ...view, nested: [{ dir: 'a/c', name: 'later', real: join(realpathSync(env.at('outer/a')), 'c') }] } as MountView; + assert.deepEqual(await checkDir(later, rootReal, 'l/c/d', { create: true }), { + kind: 'unobserved', reason: '"outer/l/c" leads into mount "later", nested in mount "outer": use "later" in its place', + }); + assert.equal(existsSync(env.at('outer/a/c')), false, "the nested mount's root wasn't made"); + }); +}); + +describe('a watcher of a mount configured as a relative path', () => { + test('hears a change beneath a root written with `./`', async (t) => { + const env = new Env(t); + mkdirSync(env.at('m')); + const path = `./${relative(process.cwd(), env.at('m'))}`; + const changes: FsChange[] = []; + let ready = false; + const watcher = new MountWatcher({ name: 'm', path, mode: 'read-write', watch: 'always', watchDebounceMs: 50 }, (batch) => changes.push(...batch), { onReady: () => { ready = true; } }); + watcher.start(); + t.after(() => watcher.stop()); + for (let i = 0; i < 400 && !ready; i++) await new Promise((r) => setTimeout(r, 25)); + assert.equal(ready, true, 'the watcher is ready'); + writeFileSync(env.at('m/x.txt'), 'x\n'); + for (let i = 0; i < 400 && !changes.some((c) => c.path === 'x.txt'); i++) await new Promise((r) => setTimeout(r, 25)); + assert.ok(changes.some((c) => c.path === 'x.txt'), `heard: ${JSON.stringify(changes)}`); + }); +}); From 7c0cfda5744899f68774043ca05dd13342868956 Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 11:34:56 -0400 Subject: [PATCH 02/14] fix(workspace): the nested-roots change's changelog fragment, and a test for the observation of a followed link - changelog.d/workspace-nested-mount-roots.fixed.md says what a host and an agent will notice: a root configured through a symlink is nested where its real path lies, resolved once when the module is constructed; a path through a followed link into a nested mount's directory is refused, with both texts; on a mount that doesn't follow links, a write, edit or delete through such a link is refused before anything is stored; and a `./m` root hears its watcher. - A test pins observePath's check of a followed link, which no test caught when it was taken out: a pass that doesn't rehash, such as a listing's, takes a file whose recorded fingerprint still vouches as unchanged without reading it (learnDisk), so the observation is the only check that sees the link now leads into a mount configured after the copy was taken in. A sync rehashes, so the reader's own check answers there. The test sets the filesystem's ctime trust, as the three-way tests do, and waits out the 2 s racy window before the sync that records the fingerprint. - The test views for the direct calls hold their nested mounts in a variable, so the file compiles against code whose nested mounts carry no real root, for the red-first run (an object literal's extra property is refused even under a cast). - test/workspace-parent-link.test.ts: #468's test of a link into a nested mount's directory wrote `work/l/img.png` before its read_image, for a workspace copy that read_image would look at first. The write is now refused before anything is stored, so the comment says what read_image's refusal now comes from, and the write that stores nothing goes. Co-Authored-By: Claude Opus 5.5 --- .../workspace-nested-mount-roots.fixed.md | 2 + test/workspace-nested-mount-roots.test.ts | 40 +++++++++++++++++-- test/workspace-parent-link.test.ts | 5 +-- 3 files changed, 40 insertions(+), 7 deletions(-) create mode 100644 changelog.d/workspace-nested-mount-roots.fixed.md diff --git a/changelog.d/workspace-nested-mount-roots.fixed.md b/changelog.d/workspace-nested-mount-roots.fixed.md new file mode 100644 index 000000000..e49c8db3a --- /dev/null +++ b/changelog.d/workspace-nested-mount-roots.fixed.md @@ -0,0 +1,2 @@ +- A mount nested in another is now that mount's directory wherever disk puts it, not only where the outer mount's paths name it as written. A root configured through a symlink, or beneath one, is nested where its real path lies; before, it wasn't nested at all, and the outer mount took in and read the inner mount's files. Each mount's root is resolved for this once, synchronously, when the module is constructed. On a mount that follows symlinks, a path that reaches a nested mount's directory through a link is now refused as one written into it: the file tools answer `"work/l/x.txt" leads into mount "inner", nested in mount "work", which neither reads nor writes it: use "inner/sub/x.txt".`, a sync or a materialize leaves it with `"work/l" leads into mount "inner", nested in mount "work": use "inner/sub" in its place`, and `readFileFromDisk` refuses it with the code `other_mount`. Before, the outer mount read, took in, wrote and unlinked the inner mount's files through such a link. On a mount that doesn't follow symlinks, a `write`, `edit` or `delete` through a link into a nested mount's directory is now refused before anything is stored, with the link's own refusal; it used to store a change that no push could write. +- A mount configured with a relative root written with `./`, such as `./m`, now hears its watcher; it heard no change on disk. diff --git a/test/workspace-nested-mount-roots.test.ts b/test/workspace-nested-mount-roots.test.ts index ceaa50119..5aac8a560 100644 --- a/test/workspace-nested-mount-roots.test.ts +++ b/test/workspace-nested-mount-roots.test.ts @@ -16,7 +16,7 @@ import { describe, test, type TestContext } from 'node:test'; import assert from 'node:assert/strict'; -import { existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, renameSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, renameSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs'; import { createRequire, syncBuiltinESMExports } from 'node:module'; import { tmpdir } from 'node:os'; import { dirname, join, relative } from 'node:path'; @@ -230,6 +230,37 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli assert.equal(readFileSync(env.at('outer/a/b/x.txt'), 'utf8'), "the inner mount's\n"); }); + test('a copy taken in through a file link before the nested mount was configured is no longer vouched for by its fingerprint', async (t) => { + // A listing's pass takes a file whose recorded fingerprint still matches + // as unchanged, without reading it, so only the observation of the link + // can say it now leads into another mount's directory. + const env = new Env(t); + env.write('outer/a/b/x.txt', "the inner mount's\n"); + symlinkSync(env.at('outer/a/b/x.txt'), env.at('outer/flink.txt')); + // Each module trusts this filesystem's ctime, whatever it is, so the + // fingerprint vouches, as on Linux's. + const trust = (m: WorkspaceModule) => (m as any).ctimeTrust.set(realpathSync(env.at('outer')), statSync(env.at('outer/a/b/x.txt'), { bigint: true }).dev.toString()); + const before = new WorkspaceModule({ mounts: [mount('outer', env.at('outer'), { followSymlinks: true })] }); + trust(before); + before.initStore(env.store); + await before.start({ isRestart: false, getState: () => null, setState: () => {}, pushEvent: () => {} } as unknown as ModuleContext); + await new Promise((r) => setTimeout(r, 2100)); // past the racy window, so the fingerprint the sync records vouches + assert.equal((await call(before, 'sync', {})).success, true); + await before.stop(); + assert.deepEqual(env.tree('outer'), ['a/b/x.txt', 'flink.txt']); + const m = new WorkspaceModule({ mounts: [mount('outer', env.at('outer'), { followSymlinks: true }), mount('inner', env.at('outer/a/b'))] }); + trust(m); + m.initStore(env.store); + await m.start({ isRestart: false, getState: () => null, setState: () => {}, pushEvent: () => {} } as unknown as ModuleContext); + t.after(() => m.stop().catch(() => {})); + const ls = await call(m, 'ls', { path: 'outer' }); + assert.equal(ls.success, true, ls.error); + const entry = (ls.data.entries as Array<{ name: string; state?: string; note?: string }>).find((e) => e.name === 'flink.txt'); + assert.deepEqual({ state: entry?.state, note: entry?.note }, { + state: 'unverified', note: '"outer/flink.txt" leads into mount "inner", nested in mount "outer": use "inner/x.txt" in its place', + }); + }); + test('a directory replaced by a link into it is the nested mount\'s: a forced push of a copy held from before writes nothing (#324)', async (t) => { const env = new Env(t); env.write('outer/alias/x.txt', 'old copy\n'); @@ -305,9 +336,9 @@ describe('the checks where disk is acted on, called directly as a push and a rea symlinkSync(env.at('outer/a/b/x.txt'), env.at('outer/flink.txt')); // Cast, so the file also compiles against code whose nested mounts carry // no real root, for the run of these tests on it. + const nested = [{ dir: 'a/b', name: 'inner', real: realpathSync(env.at('outer/a/b')) }]; const view = { - root: env.at('outer'), name: 'outer', followSymlinks: true, maxFileSize: 1 << 20, ignore: [], others: ['a/b'], - nested: [{ dir: 'a/b', name: 'inner', real: realpathSync(env.at('outer/a/b')) }], + root: env.at('outer'), name: 'outer', followSymlinks: true, maxFileSize: 1 << 20, ignore: [], others: ['a/b'], nested, } as MountView; return { env, view, rootReal: realpathSync(env.at('outer')) }; } @@ -340,7 +371,8 @@ describe('the checks where disk is acted on, called directly as a push and a rea test('a directory made for a push isn\'t made where a link leads into a nested mount\'s root not made yet (G3)', async (t) => { const { env, view, rootReal } = rig(t); symlinkSync(env.at('outer/a'), env.at('outer/l')); - const later = { ...view, nested: [{ dir: 'a/c', name: 'later', real: join(realpathSync(env.at('outer/a')), 'c') }] } as MountView; + const nested = [{ dir: 'a/c', name: 'later', real: join(realpathSync(env.at('outer/a')), 'c') }]; + const later = { ...view, nested } as MountView; assert.deepEqual(await checkDir(later, rootReal, 'l/c/d', { create: true }), { kind: 'unobserved', reason: '"outer/l/c" leads into mount "later", nested in mount "outer": use "later" in its place', }); diff --git a/test/workspace-parent-link.test.ts b/test/workspace-parent-link.test.ts index 1fd3e5535..b71b8f6fd 100644 --- a/test/workspace-parent-link.test.ts +++ b/test/workspace-parent-link.test.ts @@ -388,9 +388,8 @@ describe('a symlinked parent on a mount that does not follow symlinks', () => { assert.ok(String(res.error).includes(reason), String(res.error)); assert.ok(!String(res.error).includes('followSymlinks'), String(res.error)); assert.equal(existsSync(join(env.dir, 'nested', 'sub', 'new.txt')), false); - // read_image, where the workspace's own copy can't be read as an image, - // gives the refusal, not that copy's failure. - await m.handleToolCall({ id: 't', name: 'write', input: { path: 'work/l/img.png', content: 'not an image\n' } }); + // read_image gives the refusal too: the file tools refuse a path that + // leads into a nested mount before looking for the workspace's copy. const image = await m.handleToolCall({ id: 'i', name: 'read_image', input: { path: 'work/l/img.png' } }); assert.equal(image.success, false); assert.equal(image.error, `work/l/img.png: ${reason}`); From 914570412843eca0c148122c623fe63839149c0e Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 12:11:32 -0400 Subject: [PATCH 03/14] fix(workspace): the nested-roots change's review round: links to places not made yet, a mount at the outer's own root, the watcher's root The internal review's fixes (phoebe-af-mount-roots, at 7c0cfda5): - A link to a place not made yet leads where its target would be. realRoot (a root configured as such a link) and realBeneath (the file tools' check) follow it by readlink, up to the kernel's limit of 40 links, rather than stopping at the link's own place. So a root configured as a link to a directory not made yet inside the outer mount is nested where the link leads (I7), and a write through a dangling link into a nested mount's directory is refused before anything is stored (I3). - A mount whose real root is the outer mount's own, or holds it, isn't nested in it, however its configured path reads: a mount configured as a link to the outer's own root claimed every outer path (I8). - The watcher makes, stats and checks a rename of the root it watches (this.root), not the configured path, so every use agrees (I6). - checkOpened's doc lists its nested-mount check (I16). - Tests: the two links to places not made yet; a mount at the outer's own root; read_image refusing a path that now leads into a nested mount rather than giving the image copy held from before (I5); and each refusal's named path read back through the nested mount (I28). Co-Authored-By: Claude Opus 5.5 --- src/modules/workspace/effects.ts | 9 +-- src/modules/workspace/index.ts | 37 ++++++++---- src/modules/workspace/watcher.ts | 13 +++-- test/workspace-nested-mount-roots.test.ts | 68 +++++++++++++++++++++++ 4 files changed, 107 insertions(+), 20 deletions(-) diff --git a/src/modules/workspace/effects.ts b/src/modules/workspace/effects.ts index e5a174576..de75ec384 100644 --- a/src/modules/workspace/effects.ts +++ b/src/modules/workspace/effects.ts @@ -371,10 +371,11 @@ export async function writeContained( * Before an effect on an opened file: a regular file, and — unless forced — * still holding what was decided on. The hash reads the whole file, so it * comes first, and the checks of where the file is follow it: canonically - * inside the mount, the one this path names (and not a symlink, where the - * mount follows none and the open couldn't refuse one), with no other names, - * in the directory still at its path beneath the root still at its own; and - * nothing wrote it while those ran. + * inside the mount and in no nested mount's directory (nestedAtReal), the + * one this path names (and not a symlink, where the mount follows none and + * the open couldn't refuse one), with no other names, in the directory still + * at its path beneath the root still at its own; and nothing wrote it while + * those ran. */ async function checkOpened( view: MountView, diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index cffd0e8fe..700282a40 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -6,9 +6,9 @@ */ import { isUtf8 } from 'node:buffer'; -import { constants as fsConstants, realpathSync } from 'node:fs'; +import { constants as fsConstants, readlinkSync, realpathSync } from 'node:fs'; import type { Stats } from 'node:fs'; -import { open, readdir, stat, lstat, realpath } from 'node:fs/promises'; +import { open, readdir, readlink, stat, lstat, realpath } from 'node:fs/promises'; import { basename, dirname, isAbsolute, join, resolve, relative, sep } from 'node:path'; import type { JsStore } from '@animalabs/chronicle'; import type { Module, ModuleContext, ProcessState, EventResponse } from '../../types/module.js'; @@ -305,34 +305,47 @@ function nestedDir(from: string, to: string): string | undefined { return rel.split(sep).join('/'); } +/** How many links realRoot and realBeneath follow to a place not made yet before they stop, as the kernel's limit does. */ +const LINKS_FOLLOWED = 40; + /** * A mount root as realpath resolves it, with the links on its way and in it - * taken out: for a root not made yet, its nearest existing directory's real - * path with the rest as configured. Where a directory can't be resolved for + * taken out. Where nothing is there yet, a link leads where its target would + * be, and anything else lies beneath its nearest existing directory's real + * path, with the rest as configured. Where a directory can't be resolved for * any other reason, such as a loop or a lack of permission, the path as * resolved lexically from there. */ -function realRoot(path: string): string { +function realRoot(path: string, links = 0): string { const absolute = resolve(path); try { return realpathSync.native(absolute); } catch (err) { const code = errnoOf(err); + if ((code !== 'ENOENT' && code !== 'ENOTDIR') || links >= LINKS_FOLLOWED) return absolute; + let target: string | null = null; + try { + target = readlinkSync(absolute); + } catch { + // Not a link: what's missing is it or a directory above it. + } + if (target !== null) return realRoot(resolve(dirname(absolute), target), links + 1); const parent = dirname(absolute); - if ((code !== 'ENOENT' && code !== 'ENOTDIR') || parent === absolute) return absolute; - return join(realRoot(parent), basename(absolute)); + return parent === absolute ? absolute : join(realRoot(parent, links), basename(absolute)); } } /** realRoot for any absolute path, read without holding the process. */ -async function realBeneath(absolute: string): Promise { +async function realBeneath(absolute: string, links = 0): Promise { try { return await realpath(absolute); } catch (err) { const code = errnoOf(err); + if ((code !== 'ENOENT' && code !== 'ENOTDIR') || links >= LINKS_FOLLOWED) return absolute; + const target = await readlink(absolute).catch(() => null); + if (target !== null) return realBeneath(resolve(dirname(absolute), target), links + 1); const parent = dirname(absolute); - if ((code !== 'ENOENT' && code !== 'ENOTDIR') || parent === absolute) return absolute; - return join(await realBeneath(parent), basename(absolute)); + return parent === absolute ? absolute : join(await realBeneath(parent, links), basename(absolute)); } } @@ -831,6 +844,10 @@ export class WorkspaceModule implements Module { if (outer === inner) continue; const outerPath = resolve(outer.path); const real = reals.get(inner.name)!; + // A mount whose real root is this one's, or holds it, isn't nested in + // it, however its configured path reads: it would claim this mount's + // every path. + if (isContainedPath(real, reals.get(outer.name)!)) continue; const fromRoots: Array<{ dir: string; root: string }> = []; for (const dir of [nestedDir(outerPath, resolve(inner.path)), nestedDir(reals.get(outer.name)!, real)]) { if (dir !== undefined && !fromRoots.some((s) => s.dir === dir)) fromRoots.push({ dir, root: join(outerPath, ...dir.split('/')) }); diff --git a/src/modules/workspace/watcher.ts b/src/modules/workspace/watcher.ts index efe8dd25f..346d5af27 100644 --- a/src/modules/workspace/watcher.ts +++ b/src/modules/workspace/watcher.ts @@ -64,9 +64,10 @@ export class MountWatcher { private readonly debounceMs: number; private readonly config: MountConfig; /** - * The root as watched and as each event's path is read against: resolved, - * since chokidar hands back the paths it reports normalized, and those of - * a root configured as `./m` matched none of them. + * The root as the watcher makes, watches and checks it, and as each + * event's path is read against: the configured path resolved (a link in it + * kept), since chokidar hands back the paths it reports normalized, and + * those of a root configured as `./m` matched none of them. */ private readonly root: string; private readonly onChangeCallback: (changes: FsChange[]) => void; @@ -107,7 +108,7 @@ export class MountWatcher { // directories a user asked us only to read would be surprising. if (this.config.mode === 'read-write') { try { - mkdirSync(this.config.path, { recursive: true }); + mkdirSync(this.root, { recursive: true }); } catch (err) { this.lifecycle.onError?.(err instanceof Error ? err : new Error(String(err))); } @@ -176,7 +177,7 @@ export class MountWatcher { if (event !== 'rename') return; // A rename on the watched root's path/basename is the one signal that // survives inode reuse on Linux, where unlinkDir/error never fire. - if (this.isRootPath(rawPath) || basename(rawPath) === basename(this.config.path)) { + if (this.isRootPath(rawPath) || basename(rawPath) === basename(this.root)) { void this.checkRootLiveness({ force: true }); } }); @@ -373,7 +374,7 @@ export class MountWatcher { } private readRootIdentity(): RootIdentity { - const stat = statSync(this.config.path); + const stat = statSync(this.root); return { dev: stat.dev, ino: stat.ino, birthtimeMs: stat.birthtimeMs }; } diff --git a/test/workspace-nested-mount-roots.test.ts b/test/workspace-nested-mount-roots.test.ts index 5aac8a560..55e968808 100644 --- a/test/workspace-nested-mount-roots.test.ts +++ b/test/workspace-nested-mount-roots.test.ts @@ -76,6 +76,13 @@ function leadsInto(path: string, outer: string, inner: string, use: string): str return `"${path}" leads into mount "${inner}", nested in mount "${outer}", which neither reads nor writes it: use "${use}".`; } +/** Read `path`, the path a refusal named, and check it holds `text`: the named path reaches the file. */ +async function readsBack(m: WorkspaceModule, path: string, text: string): Promise { + const read = await call(m, 'read', { path }); + assert.equal(read.success, true, `${path}: ${read.error}`); + assert.match(read.data.content, new RegExp(text), path); +} + /** The skipped reason for a path in a materialize's or a sync's result. */ function skippedReason(data: any, path: string): string { const entry = ((data.skipped ?? []) as Array<{ path?: string; reason: string }>) @@ -110,6 +117,31 @@ describe('a mount root configured through a symlink', () => { const read = await call(m, 'read', { path: 'outer/a/b/x.txt' }); assert.equal(read.success, false); assert.equal(read.error, '"outer/a/b/x.txt" lies in mount "inner", nested in mount "outer", which neither reads nor writes it: use "inner/x.txt".'); + await readsBack(m, 'inner/x.txt', "the inner mount's"); + }); + + test('an inner root configured as a link to a directory not made yet inside the outer mount is nested where the link leads', async (t) => { + const env = new Env(t); + env.write('outer/top.txt', "the outer mount's\n"); + symlinkSync(env.at('outer/c/inner-dir'), env.at('via')); + const m = await env.open(t, [mount('outer', env.at('outer')), mount('inner', env.at('via'))]); + // The inner mount's directory is made after the module is constructed. + env.write('outer/c/inner-dir/x.txt', "the inner mount's\n"); + assert.equal((await call(m, 'sync', {})).success, true); + assert.deepEqual(env.tree('outer'), ['top.txt'], "the outer mount took none of the inner mount's files in"); + const read = await call(m, 'read', { path: 'outer/c/inner-dir/x.txt' }); + assert.equal(read.error, '"outer/c/inner-dir/x.txt" lies in mount "inner", nested in mount "outer", which neither reads nor writes it: use "inner/x.txt".'); + await readsBack(m, 'inner/x.txt', "the inner mount's"); + }); + + test('a mount configured as a link to the outer mount\'s own root isn\'t nested in it, and claims none of its paths', async (t) => { + const env = new Env(t); + env.write('outer/x.txt', "the outer mount's\n"); + symlinkSync(env.at('outer'), env.at('outer/self')); + const m = await env.open(t, [mount('outer', env.at('outer')), mount('self', env.at('outer/self'))]); + await readsBack(m, 'outer/x.txt', "the outer mount's"); + const write = await call(m, 'write', { path: 'outer/y.txt', content: 'outer wrote\n' }); + assert.equal(write.success, true, write.error); }); test('an inner mount configured through a link inside the outer mount is passed over at both of its directories', async (t) => { @@ -165,6 +197,7 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli const read = await call(m, 'read', { path: 'work/l/x.txt' }); assert.equal(read.success, false); assert.equal(read.error, leadsInto('work/l/x.txt', 'work', 'inner', 'inner/sub/x.txt')); + await readsBack(m, 'inner/sub/x.txt', 'inner file'); const write = await call(m, 'write', { path: 'work/l/new.txt', content: 'via work\n' }); assert.equal(write.success, false); assert.equal(write.error, leadsInto('work/l/new.txt', 'work', 'inner', 'inner/sub/new.txt')); @@ -201,6 +234,7 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli ]); const read = await call(m, 'read', { path: 'work/l/b/x.txt' }); assert.equal(read.error, leadsInto('work/l/b/x.txt', 'work', 'inner', 'inner/x.txt')); + await readsBack(m, 'inner/x.txt', 'inner file'); const top = await call(m, 'read', { path: 'work/l/top.txt' }); assert.equal(top.success, true, top.error); assert.match(top.data.content, /outer file under a/); @@ -212,6 +246,39 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli assert.equal(existsSync(env.at('work/a/b/deep')), false); }); + test('a link to a place not made yet in it leads there: a write through it is refused before anything is stored', async (t) => { + const env = new Env(t); + env.write('work/nested/keep.txt', 'inner\n'); + symlinkSync('nested/new.txt', env.at('work/flink.txt')); + symlinkSync('nested/sub', env.at('work/l')); + const m = await env.open(t, [ + mount('work', env.at('work'), { followSymlinks: true, autoMaterialize: true }), + mount('inner', env.at('work/nested'), { autoMaterialize: true }), + ]); + for (const [path, use] of [['work/flink.txt', 'inner/new.txt'], ['work/l/x.txt', 'inner/sub/x.txt']] as const) { + const write = await call(m, 'write', { path, content: 'via work\n' }); + assert.equal(write.error, leadsInto(path, 'work', 'inner', use)); + } + assert.deepEqual(env.tree('work'), [], 'nothing was stored'); + assert.equal(existsSync(env.at('work/nested/new.txt')), false); + assert.equal(existsSync(env.at('work/nested/sub')), false); + }); + + test('read_image refuses a path that now leads into it, rather than give the copy held from before', async (t) => { + const env = new Env(t); + mkdirSync(env.at('work/l'), { recursive: true }); + env.write('work/nested/sub/keep.txt', 'inner\n'); + const m = await env.open(t, [mount('work', env.at('work'), { followSymlinks: true }), mount('inner', env.at('work/nested'))]); + const png = Buffer.from('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4////fwAJ+wP9KobjigAAAABJRU5ErkJggg==', 'base64'); + const saved = await m.writeBinary('work/l/img.png', png, 'image/png'); + assert.equal(saved.success, true, saved.error); + rmSync(env.at('work/l'), { recursive: true, force: true }); + symlinkSync('nested/sub', env.at('work/l')); + const image = await call(m, 'read_image', { path: 'work/l/img.png' }); + assert.equal(image.success, false); + assert.equal(image.error, leadsInto('work/l/img.png', 'work', 'inner', 'inner/sub/img.png')); + }); + test('a file link into it is taken in by no sync, and a copy held from before is neither read nor pushed through it', async (t) => { const env = new Env(t); env.write('outer/a/b/x.txt', "the inner mount's\n"); @@ -274,6 +341,7 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli assert.equal(materialize.data.count, 0); assert.equal(skippedReason(materialize.data, 'alias/x.txt'), 'alias/x.txt: not written: "outer/alias" leads into mount "inner", nested in mount "outer": use "inner" in its place'); assert.equal(readFileSync(env.at('outer/a/b/x.txt'), 'utf8'), 'inner v2\n'); + await readsBack(m, 'inner/x.txt', 'inner v2'); for (const [name, input] of [ ['edit', { path: 'outer/alias/x.txt', oldString: 'old', newString: 'outer-edited' }], ['write', { path: 'outer/alias/y.txt', content: 'outer wrote y\n' }], From 6aa434a8afeaa1d0fffa2c5aaae2af3ad7adb83a Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 12:44:56 -0400 Subject: [PATCH 04/14] fix(workspace): place the roots at each start, awaited, and the second review round's fixes Ada-1017's call (room-203 #146358), after Oscar-1808's question (#146326): a synchronous realpath at construction held the whole event loop on a share that stops answering, the class #311 names. - The roots are placed at each start, awaited, before start sets anything (readNesting, where #525's spellNestedDirs read disk's spellings). The constructor nests by configured path, as #460 does, so a call before start has that nesting, as it has #525's spellings only from start. A root moved while the module was stopped is placed again at the next start. realRoot, the synchronous copy, goes: realBeneath places roots and paths. - realBeneath reads a link's target from the link's directory as it is on disk, and resolves nothing lexically on the way, so a `..` in a target is taken from where the links before it lead, as the kernel takes it (phoebe-af-mount-roots-r2 I1). A path it can't place, such as one behind a loop, is resolved lexically from there. - A mount whose real root is the outer mount's own, or holds it, keeps its nesting by configured path and has no real root to claim by (r2 I2): round 1's skip dropped #460's nesting for it too, so a following outer walked into the link and a write through it was stored. - read_image's mapping of other_mount keeps its comment true for the checks this change adds. - Tests: a link with a `..` target under a link (I1); the own-root mount on a following outer, through its link (I2, I4); a mount linked to a directory above the outer (I3); a write through a link to a place not made yet on a mount that doesn't follow links (I8); the dangling root link with a relative target (I10); a root moved while stopped; and, from Langston-2479's read (#146366), read_image with a link swapped in after the tools' check, and a path that is itself a link into a nested mount on a mount that doesn't follow links. The changelog says the last one's code changes from symlink to other_mount. Co-Authored-By: Claude Opus 5.5 --- .../workspace-nested-mount-roots.fixed.md | 2 +- src/modules/workspace/index.ts | 193 ++++++++++-------- src/modules/workspace/observe.ts | 8 +- test/workspace-nested-mount-roots.test.ts | 127 +++++++++++- 4 files changed, 233 insertions(+), 97 deletions(-) diff --git a/changelog.d/workspace-nested-mount-roots.fixed.md b/changelog.d/workspace-nested-mount-roots.fixed.md index e49c8db3a..1ef1496a0 100644 --- a/changelog.d/workspace-nested-mount-roots.fixed.md +++ b/changelog.d/workspace-nested-mount-roots.fixed.md @@ -1,2 +1,2 @@ -- A mount nested in another is now that mount's directory wherever disk puts it, not only where the outer mount's paths name it as written. A root configured through a symlink, or beneath one, is nested where its real path lies; before, it wasn't nested at all, and the outer mount took in and read the inner mount's files. Each mount's root is resolved for this once, synchronously, when the module is constructed. On a mount that follows symlinks, a path that reaches a nested mount's directory through a link is now refused as one written into it: the file tools answer `"work/l/x.txt" leads into mount "inner", nested in mount "work", which neither reads nor writes it: use "inner/sub/x.txt".`, a sync or a materialize leaves it with `"work/l" leads into mount "inner", nested in mount "work": use "inner/sub" in its place`, and `readFileFromDisk` refuses it with the code `other_mount`. Before, the outer mount read, took in, wrote and unlinked the inner mount's files through such a link. On a mount that doesn't follow symlinks, a `write`, `edit` or `delete` through a link into a nested mount's directory is now refused before anything is stored, with the link's own refusal; it used to store a change that no push could write. +- A mount nested in another is now that mount's directory wherever disk puts it, not only where the outer mount's paths name it as written. A root configured through a symlink, or beneath one, is nested where its real path lies; before, it wasn't nested at all, and the outer mount took in and read the inner mount's files. Each start of the module reads each root's real path again, awaited, so a root on a share that stops answering holds that start, not the process; before the first start, mounts are nested by their configured paths alone, as they were. On a mount that follows symlinks, a path that reaches a nested mount's directory through a link is now refused as one written into it: the file tools answer `"work/l/x.txt" leads into mount "inner", nested in mount "work", which neither reads nor writes it: use "inner/sub/x.txt".`, a sync or a materialize leaves it with `"work/l" leads into mount "inner", nested in mount "work": use "inner/sub" in its place`, and `readFileFromDisk` refuses it with the code `other_mount`. Before, the outer mount read, took in, wrote and unlinked the inner mount's files through such a link. On a mount that doesn't follow symlinks, a path through a link into a nested mount's directory is now refused before anything is stored, with the refusal of the link the mount doesn't follow; a `write`, `edit` or `delete` through it used to store a change that no push could write. Where the path is itself a link to a file in a nested mount's directory, `readFileFromDisk` now refuses it with the code `other_mount` and words naming that mount, where it answered `symlink`. - A mount configured with a relative root written with `./`, such as `./m`, now hears its watcher; it heard no change on disk. diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index 700282a40..ff70182f7 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -6,7 +6,7 @@ */ import { isUtf8 } from 'node:buffer'; -import { constants as fsConstants, readlinkSync, realpathSync } from 'node:fs'; +import { constants as fsConstants } from 'node:fs'; import type { Stats } from 'node:fs'; import { open, readdir, readlink, stat, lstat, realpath } from 'node:fs/promises'; import { basename, dirname, isAbsolute, join, resolve, relative, sep } from 'node:path'; @@ -305,47 +305,47 @@ function nestedDir(from: string, to: string): string | undefined { return rel.split(sep).join('/'); } -/** How many links realRoot and realBeneath follow to a place not made yet before they stop, as the kernel's limit does. */ +/** A mount nested in another, as the outer mount's otherMounts holds it. */ +interface NestedMountEntry { + name: string; + /** + * Its real root, as realBeneath places it once a start has read it, and + * its root as resolved until then. None where it's the outer mount's own + * or holds it (NestedMount). + */ + real?: string; + /** Its directory as configured and as the real roots place it. */ + fromRoots: Array<{ dir: string; root: string }>; + /** Those, and each as disk spells it where that differs. */ + spellings: Array<{ dir: string; root: string }>; +} + +/** How many links realBeneath follows to a place not made yet before it stops, as the kernel's limit does. */ const LINKS_FOLLOWED = 40; /** - * A mount root as realpath resolves it, with the links on its way and in it - * taken out. Where nothing is there yet, a link leads where its target would - * be, and anything else lies beneath its nearest existing directory's real - * path, with the rest as configured. Where a directory can't be resolved for - * any other reason, such as a loop or a lack of permission, the path as - * resolved lexically from there. + * An absolute path, a mount root's or one beneath it, as realpath resolves + * it, with the links on its way and in it taken out, read without holding + * the process. Where nothing is there yet, a link leads where its target + * would be, and anything else lies beneath its nearest existing directory's + * real path, with the rest as given. A link's target is read from the + * link's directory as it is on disk, and nothing is resolved lexically on + * the way, so a `..` is taken from where the links before it lead, as the + * kernel takes it. Where a directory can't be resolved for any other + * reason, such as a loop or a lack of permission, the path as resolved + * lexically from there. */ -function realRoot(path: string, links = 0): string { - const absolute = resolve(path); - try { - return realpathSync.native(absolute); - } catch (err) { - const code = errnoOf(err); - if ((code !== 'ENOENT' && code !== 'ENOTDIR') || links >= LINKS_FOLLOWED) return absolute; - let target: string | null = null; - try { - target = readlinkSync(absolute); - } catch { - // Not a link: what's missing is it or a directory above it. - } - if (target !== null) return realRoot(resolve(dirname(absolute), target), links + 1); - const parent = dirname(absolute); - return parent === absolute ? absolute : join(realRoot(parent, links), basename(absolute)); - } -} - -/** realRoot for any absolute path, read without holding the process. */ async function realBeneath(absolute: string, links = 0): Promise { try { return await realpath(absolute); } catch (err) { const code = errnoOf(err); - if ((code !== 'ENOENT' && code !== 'ENOTDIR') || links >= LINKS_FOLLOWED) return absolute; - const target = await readlink(absolute).catch(() => null); - if (target !== null) return realBeneath(resolve(dirname(absolute), target), links + 1); const parent = dirname(absolute); - return parent === absolute ? absolute : join(await realBeneath(parent, links), basename(absolute)); + if ((code !== 'ENOENT' && code !== 'ENOTDIR') || links >= LINKS_FOLLOWED || parent === absolute) return resolve(absolute); + const place = await realBeneath(parent, links); + const target = await readlink(absolute).catch(() => null); + if (target !== null) return realBeneath(isAbsolute(target) ? target : `${place}${sep}${target}`, links + 1); + return join(place, basename(absolute)); } } @@ -790,19 +790,16 @@ export class WorkspaceModule implements Module { private ctimeTrust = new Map(); /** * Each mount's directories that are other mounts nested inside it, with - * each mount's name and real root (realRoot), in each spelling that names - * the directory: as configured; as the real roots place it, where that - * differs (`fromRoots`, both found at construction); and as disk spells - * each of those where that differs (spellNestedDirs). Each spelling is - * mount-relative and `/`-separated, with its root beneath the mount's - * resolved root. + * each mount's name and real root, in each spelling that names the + * directory: as configured; as the real roots place it, where that differs + * (`fromRoots`); and as disk spells each of those where that differs. + * Each spelling is mount-relative and `/`-separated, with its root beneath + * the mount's resolved root. The constructor nests by configured path, + * with each root as resolved standing for its real one, and each start + * reads it all again with the roots placed where disk has them + * (readNesting). */ - private otherMounts = new Map; - spellings: Array<{ dir: string; root: string }>; - }>>(); + private otherMounts = new Map(); /** * Whether a file's mode is observed, taken in and applied (MountRuntime's * `modesObserved`): not on Windows, which reports no POSIX modes. A test @@ -831,32 +828,21 @@ export class WorkspaceModule implements Module { // named or not, and in its watcher (OTHER_MOUNT). It's kept apart from // the outer mount's ignore list, which a sync that names a path sets // aside for what it names (#276), so no sync of the outer mount takes in - // the inner one's files. A root configured through a symlink, or beneath - // one, is nested where realpath places it, so each root is resolved - // once, here, before any path is parsed: an inner mount nested only that - // way, and one whose directory the real roots place elsewhere than the - // configured paths do, is passed over at each place. Reading a root that - // stops answering holds the process here, as the watcher's start does - // (#311). - const reals = new Map(config.mounts.map((mount) => [mount.name, realRoot(mount.path)])); + // the inner one's files. Here by configured path, before any path is + // parsed; each start places the roots where disk has them too + // (readNesting). for (const outer of config.mounts) { for (const inner of config.mounts) { if (outer === inner) continue; const outerPath = resolve(outer.path); - const real = reals.get(inner.name)!; - // A mount whose real root is this one's, or holds it, isn't nested in - // it, however its configured path reads: it would claim this mount's - // every path. - if (isContainedPath(real, reals.get(outer.name)!)) continue; - const fromRoots: Array<{ dir: string; root: string }> = []; - for (const dir of [nestedDir(outerPath, resolve(inner.path)), nestedDir(reals.get(outer.name)!, real)]) { - if (dir !== undefined && !fromRoots.some((s) => s.dir === dir)) fromRoots.push({ dir, root: join(outerPath, ...dir.split('/')) }); - } - if (fromRoots.length === 0) continue; - this.otherMounts.set(outer.name, [...(this.otherMounts.get(outer.name) ?? []), { name: inner.name, real, fromRoots, spellings: [...fromRoots] }]); + const innerPath = resolve(inner.path); + const dir = nestedDir(outerPath, innerPath); + if (dir === undefined) continue; + const configured = [{ dir, root: innerPath }]; + this.otherMounts.set(outer.name, [...(this.otherMounts.get(outer.name) ?? []), { name: inner.name, real: innerPath, fromRoots: configured, spellings: [...configured] }]); console.warn( `[workspace] Mount "${outer.name}" contains mount "${inner.name}" ` + - `at ${fromRoots.map((s) => `${s.dir}/`).join(' and ')} — passing over it as another mount's directory`, + `at ${dir}/ — passing over it as another mount's directory`, ); } } @@ -1033,13 +1019,15 @@ export class WorkspaceModule implements Module { } async start(ctx: ModuleContext): Promise { - // Each nested mount's directory as disk spells it, read before this start - // sets anything, so a restart takes no call and walks or watches nothing - // before the spellings are known. A stop begun meanwhile stands, and this - // start then sets nothing. + // Which mounts are nested where, with the roots placed where disk has + // them and each nested mount's directory as disk spells it, read before + // this start sets anything, so a restart takes no call and walks or + // watches nothing before they're known. A stop begun meanwhile stands, + // and this start then sets nothing. const stops = this.stopsBegun; - await this.spellNestedDirs(); + const nesting = await this.readNesting(); if (this.stopsBegun !== stops) return; + this.otherMounts = nesting; this.ctx = ctx; this.stopped = false; @@ -1731,7 +1719,7 @@ export class WorkspaceModule implements Module { try { // Synchronous, so the path as given: it reaches the same file, and // the nested-mount check matches it as configured or as disk spells - // the nested mount's directory (spellNestedDirs). + // the nested mount's directory (readNesting). const { mount, relativePath } = this.refuseNested({ ...this.parsePathAsGiven(mountPrefixedPath), path: mountPrefixedPath }); return resolve(mount.config.path, relativePath); } catch { @@ -1927,28 +1915,55 @@ export class WorkspaceModule implements Module { } /** - * Adds to each nested mount's directory the spelling disk lists it by, - * where that differs from those found at construction (`fromRoots`). On a + * Which mounts are nested in which, read afresh at each start. A mount is + * nested in another where its configured path, or its real root, lies + * beneath the other's. Each root is placed by realBeneath, awaited, so a + * root on a share that stops answering holds this start and not the + * process, and a root moved while the module was stopped is placed where + * it is now. A mount whose real root is the other's own, or holds it, is + * nested by its configured path alone, with no real root, since through it + * it would claim the other's every path. + * + * Each way it lies there is a spelling of its directory (`fromRoots`), and + * so is the spelling disk lists each of those by, where that differs. On a * disk that folds names, the outer mount's walks name the directory as * disk does, and so does a path parsePath has respelled, while a path given * as configured still names it as configured (resolveAbsolutePath). A - * directory not on disk keeps the spellings found at construction alone. - * Run at each start. + * directory not on disk keeps the spellings the roots give alone. */ - private async spellNestedDirs(): Promise { - for (const [outerName, nested] of this.otherMounts) { - const outer = this.config.mounts.find((m) => m.name === outerName); - if (!outer) continue; - const outerRoot = resolve(outer.path); - for (const entry of nested) { - const spellings = [...entry.fromRoots]; - for (const found of entry.fromRoots) { - const dir = await diskSpelling(outerRoot, found.dir); - if (!spellings.some((s) => s.dir === dir)) spellings.push({ dir, root: join(outerRoot, ...dir.split('/')) }); + private async readNesting(): Promise> { + const reals = new Map(); + for (const mount of this.config.mounts) reals.set(mount.name, await realBeneath(resolve(mount.path))); + const nesting = new Map(); + for (const outer of this.config.mounts) { + const outerPath = resolve(outer.path); + const outerReal = reals.get(outer.name)!; + for (const inner of this.config.mounts) { + if (outer === inner) continue; + const real = reals.get(inner.name)!; + const configured = nestedDir(outerPath, resolve(inner.path)); + const fromRoots: Array<{ dir: string; root: string }> = []; + for (const dir of [configured, nestedDir(outerReal, real)]) { + if (dir !== undefined && !fromRoots.some((s) => s.dir === dir)) fromRoots.push({ dir, root: join(outerPath, ...dir.split('/')) }); + } + if (fromRoots.length === 0) continue; + const spellings = [...fromRoots]; + for (const found of fromRoots) { + const dir = await diskSpelling(outerPath, found.dir); + if (!spellings.some((s) => s.dir === dir)) spellings.push({ dir, root: join(outerPath, ...dir.split('/')) }); + } + const claims = !isContainedPath(real, outerReal); + nesting.set(outer.name, [...(nesting.get(outer.name) ?? []), { name: inner.name, ...(claims ? { real } : {}), fromRoots, spellings }]); + const placed = fromRoots.filter((s) => s.dir !== configured); + if (placed.length > 0) { + console.warn( + `[workspace] Mount "${outer.name}" contains mount "${inner.name}" where its root is on disk, ` + + `at ${placed.map((s) => `${s.dir}/`).join(' and ')} — passing over it as another mount's directory`, + ); } - entry.spellings = spellings; } } + return nesting; } /** The mounts nested in `mountName`, by directory, name and real root (MountView's `nested`), in each spelling known. */ @@ -2382,10 +2397,10 @@ export class WorkspaceModule implements Module { // where it leads, with the setting for a link to a directory. return new WorkspaceImageReadError('parent_symlink', err.message); case 'other_mount': - // The core reader's own words for a link on its way into a nested - // mount's directory: that mount and the path to use there in place of - // the link. (#460's own other_mount, a path in that directory, is - // refused at parse.) + // The core reader's own words for a path a link takes into a nested + // mount's directory: that mount and the path to use there. (#460's + // own other_mount, a path in that directory, is refused at parse, + // and so is a link found there before the read.) return new WorkspaceImageReadError('other_mount', err.message); case 'escape': // The core reader's own words: "Symlink escape detected: " for a diff --git a/src/modules/workspace/observe.ts b/src/modules/workspace/observe.ts index 0b4f4cce2..4ea9e5bab 100644 --- a/src/modules/workspace/observe.ts +++ b/src/modules/workspace/observe.ts @@ -77,9 +77,11 @@ export interface MountView { * A mount nested in another: its directory in one of its spellings, * mount-relative and `/`-separated, its name, and its root as realpath * resolves it (the nearest existing directory's, where the root isn't made - * yet), which every spelling shares. + * yet), which every spelling shares. No real root where it's this mount's + * own or holds it: the mount is nested by its configured path alone, since + * through its real root it would claim this mount's every path. */ -export type NestedMount = { dir: string; name: string; real: string }; +export type NestedMount = { dir: string; name: string; real?: string }; export type StatFingerprint = Omit; @@ -309,7 +311,7 @@ export async function linkReason(view: Pick contained(n.real, real)) + .flatMap((n) => (n.real !== undefined && contained(n.real, real) ? [{ name: n.name, real: n.real }] : [])) .sort((a, b) => b.real.length - a.real.length)[0]; if (!holder) return undefined; const rest = relative(holder.real, real).split(sep).join('/'); diff --git a/test/workspace-nested-mount-roots.test.ts b/test/workspace-nested-mount-roots.test.ts index 55e968808..b01520c35 100644 --- a/test/workspace-nested-mount-roots.test.ts +++ b/test/workspace-nested-mount-roots.test.ts @@ -123,9 +123,9 @@ describe('a mount root configured through a symlink', () => { test('an inner root configured as a link to a directory not made yet inside the outer mount is nested where the link leads', async (t) => { const env = new Env(t); env.write('outer/top.txt', "the outer mount's\n"); - symlinkSync(env.at('outer/c/inner-dir'), env.at('via')); + symlinkSync('outer/c/inner-dir', env.at('via')); const m = await env.open(t, [mount('outer', env.at('outer')), mount('inner', env.at('via'))]); - // The inner mount's directory is made after the module is constructed. + // The inner mount's directory is made after the module starts. env.write('outer/c/inner-dir/x.txt', "the inner mount's\n"); assert.equal((await call(m, 'sync', {})).success, true); assert.deepEqual(env.tree('outer'), ['top.txt'], "the outer mount took none of the inner mount's files in"); @@ -134,14 +134,38 @@ describe('a mount root configured through a symlink', () => { await readsBack(m, 'inner/x.txt', "the inner mount's"); }); - test('a mount configured as a link to the outer mount\'s own root isn\'t nested in it, and claims none of its paths', async (t) => { + test('a mount configured as a link to the outer mount\'s own root is nested by its configured path alone, and claims none of the outer\'s other paths', async (t) => { const env = new Env(t); env.write('outer/x.txt', "the outer mount's\n"); symlinkSync(env.at('outer'), env.at('outer/self')); - const m = await env.open(t, [mount('outer', env.at('outer')), mount('self', env.at('outer/self'))]); + const m = await env.open(t, [mount('outer', env.at('outer'), { followSymlinks: true }), mount('self', env.at('outer/self'))]); + assert.equal((await call(m, 'sync', {})).success, true); + assert.deepEqual(env.tree('outer'), ['x.txt'], 'the outer mount passed over the link'); await readsBack(m, 'outer/x.txt', "the outer mount's"); const write = await call(m, 'write', { path: 'outer/y.txt', content: 'outer wrote\n' }); assert.equal(write.success, true, write.error); + const through = await call(m, 'read', { path: 'outer/self/x.txt' }); + assert.equal(through.error, '"outer/self/x.txt" lies in mount "self", nested in mount "outer", which neither reads nor writes it: use "self/x.txt".'); + await readsBack(m, 'self/x.txt', "the outer mount's"); + }); + + test('a mount configured as a link inside the outer mount to a directory above it is nested by its configured path, and holds the outer mount where it is', async (t) => { + const env = new Env(t); + env.write('top/top.txt', "the upper mount's\n"); + env.write('top/outer/x.txt', "the outer mount's\n"); + symlinkSync(env.at('top'), env.at('top/outer/up')); + const m = await env.open(t, [mount('outer', env.at('top/outer')), mount('up', env.at('top/outer/up'))]); + assert.equal((await call(m, 'sync', {})).success, true); + assert.deepEqual(env.tree('outer'), ['x.txt']); + assert.deepEqual(env.tree('up'), ['top.txt']); + const through = await call(m, 'read', { path: 'outer/up/top.txt' }); + assert.equal(through.error, '"outer/up/top.txt" lies in mount "up", nested in mount "outer", which neither reads nor writes it: use "up/top.txt".'); + const write = await call(m, 'write', { path: 'outer/up/new.txt', content: 'outer wrote\n' }); + assert.equal(write.success, false); + const back = await call(m, 'read', { path: 'up/outer/x.txt' }); + assert.equal(back.error, '"up/outer/x.txt" lies in mount "outer", nested in mount "up", which neither reads nor writes it: use "outer/x.txt".'); + await readsBack(m, 'up/top.txt', "the upper mount's"); + await readsBack(m, 'outer/x.txt', "the outer mount's"); }); test('an inner mount configured through a link inside the outer mount is passed over at both of its directories', async (t) => { @@ -264,6 +288,21 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli assert.equal(existsSync(env.at('work/nested/sub')), false); }); + test('a link to a place not made yet is read from its directory as it is on disk, a `..` in its target included', async (t) => { + // work/alias leads to nested/sub, where flink.txt leads to ../y.txt, not + // made yet. On disk that's nested/y.txt, in the nested mount; read from + // the path as written, it would be work/y.txt. + const env = new Env(t); + env.write('work/nested/sub/keep.txt', 'inner\n'); + symlinkSync('nested/sub', env.at('work/alias')); + symlinkSync('../y.txt', env.at('work/nested/sub/flink.txt')); + const m = await env.open(t, [mount('work', env.at('work'), { followSymlinks: true }), mount('inner', env.at('work/nested'))]); + const write = await call(m, 'write', { path: 'work/alias/flink.txt', content: 'via work\n' }); + assert.equal(write.error, leadsInto('work/alias/flink.txt', 'work', 'inner', 'inner/y.txt')); + assert.deepEqual(env.tree('work'), [], 'nothing was stored'); + assert.equal(existsSync(env.at('work/nested/y.txt')), false); + }); + test('read_image refuses a path that now leads into it, rather than give the copy held from before', async (t) => { const env = new Env(t); mkdirSync(env.at('work/l'), { recursive: true }); @@ -379,9 +418,89 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli }); assert.equal(swapped, true); }); + + test('read_image refuses a path whose directory became a link into it after the tools\' check, over the copy held from before', async (t) => { + // The copy isn't an image, so a disk refusal read as "not found" would + // answer the copy's failure instead (#468's case, which this pins now + // that the tools' check refuses the write that used to store the copy). + const env = new Env(t); + env.write('work/l/img.png', 'not an image\n'); + env.write('work/nested/sub/img.png', 'the inner mount\'s\n'); + const m = await env.open(t, [mount('work', env.at('work'), { followSymlinks: true }), mount('inner', env.at('work/nested'))]); + assert.equal((await call(m, 'sync', {})).success, true); + assert.ok(env.tree('work').includes('l/img.png'), 'the outer mount holds the copy'); + let swapped = false; + await withLstat((lstat) => async (path: string, ...rest: unknown[]) => { + if (!swapped && String(path) === env.at('work/l/img.png')) { + swapped = true; + renameSync(env.at('work/l'), env.at('work/l-was')); + symlinkSync('nested/sub', env.at('work/l')); + } + return lstat(path, ...rest); + }, async () => { + const image = await call(m, 'read_image', { path: 'work/l/img.png' }); + assert.equal(image.success, false); + assert.equal(image.error, '"work/l/img.png" leads into mount "inner", nested in mount "work": use "inner/sub/img.png" in its place'); + }); + assert.equal(swapped, true); + }); +}); + +describe('a root placed at each start', () => { + test('a root moved while the module was stopped is placed where it is at the next start', async (t) => { + const env = new Env(t); + env.write('outer/a/b/x.txt', 'under a/b\n'); + env.write('outer/c/y.txt', 'under c\n'); + symlinkSync(env.at('outer/a/b'), env.at('via')); + const m = await env.open(t, [mount('outer', env.at('outer')), mount('inner', env.at('via'))]); + const ctx = { isRestart: false, getState: () => null, setState: () => {}, pushEvent: () => {} } as unknown as ModuleContext; + assert.equal((await call(m, 'read', { path: 'outer/c/y.txt' })).success, true); + await m.stop(); + rmSync(env.at('via')); + symlinkSync(env.at('outer/c'), env.at('via')); + await m.start({ ...ctx, isRestart: true } as unknown as ModuleContext); + const read = await call(m, 'read', { path: 'outer/c/y.txt' }); + assert.equal(read.error, '"outer/c/y.txt" lies in mount "inner", nested in mount "outer", which neither reads nor writes it: use "inner/y.txt".'); + await readsBack(m, 'outer/a/b/x.txt', 'under a/b'); + }); }); describe('a link into a nested mount\'s directory, on a mount that doesn\'t follow symlinks', () => { + test('a path that is itself a link into it is refused with the mount it leads into, by the tools and the disk reader, and nothing is stored', async (t) => { + // On `main`, readFileFromDisk answered `symlink` ("Symlinks are not + // allowed"), and a write stored a draft that no push could write. + const env = new Env(t); + env.write('work/nested/x.txt', 'inner file\n'); + symlinkSync('nested/x.txt', env.at('work/flink.txt')); + const m = await env.open(t, [mount('work', env.at('work')), mount('inner', env.at('work/nested'))]); + const refusal = 'work/flink.txt: "work/flink.txt" is a symlink to "work/nested/x.txt", which this mount does not follow; it lies in mount "inner", nested in mount "work": use "inner/x.txt" in place of "work/flink.txt"'; + for (const [name, input] of [['read', { path: 'work/flink.txt' }], ['write', { path: 'work/flink.txt', content: 'via work\n' }]] as const) { + const res = await call(m, name, input); + assert.equal(res.error, refusal, name); + } + assert.deepEqual(env.tree('work'), [], 'nothing was stored'); + await assert.rejects(m.readFileFromDisk('work/flink.txt'), (err: unknown) => { + assert.ok(err instanceof WorkspaceReadError, String(err)); + assert.equal(err.code, 'other_mount'); + assert.equal(err.message, refusal); + return true; + }); + await readsBack(m, 'inner/x.txt', 'inner file'); + }); + + test('a write through a link to a place not made yet in it is refused before anything is stored', async (t) => { + const env = new Env(t); + env.write('work/nested/keep.txt', 'inner\n'); + symlinkSync('nested/new.txt', env.at('work/flink.txt')); + const m = await env.open(t, [mount('work', env.at('work')), mount('inner', env.at('work/nested'))]); + // #468's words for a link whose target isn't there to name. + const write = await call(m, 'write', { path: 'work/flink.txt', content: 'via work\n' }); + assert.equal(write.success, false); + assert.equal(write.error, 'work/flink.txt: "work/flink.txt" is a symlink, which this mount does not follow'); + assert.deepEqual(env.tree('work'), [], 'nothing was stored'); + assert.equal(existsSync(env.at('work/nested/new.txt')), false); + }); + test('a write through it is refused before anything is stored, with the link\'s own refusal', async (t) => { // The readers and pushes give this refusal already (#468). A write used // to store a draft that no push could write. From 5b60526aab8e99f12c94b85c44a5aeb09b55828e Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 13:09:19 -0400 Subject: [PATCH 05/14] fix(workspace): a link target written with a trailing separator, and the third review round's fixes - realBeneath drops trailing separators before it places a path: a link to a directory is often written `L2/`, and readlink of a path ending in a separator follows the link it names, so a root configured through such a link to a place not made yet was placed at the link (Ogden-2688, room-203 #146615, J19998; phoebe-af-mount-roots-r3 I18). - readNesting also reads an inner mount's configured path against the outer mount's real root, so an outer configured through a link keeps the configured-path nesting of a mount configured as a link back to its root (r3 I19), as one configured at its real path does. - Docs: NestedMount, MountView.nested and nestedOf say a nested mount's real root is the root as configured until a start reads it, and absent where the mount is nested by configured path alone (r3 I13); realBeneath says a path realpath fails on for another reason is resolved whole, by path.resolve (r3 I15); the test file's header says the roots are read at each start (r3 I9). - Tests: Ogden-2688's three spellings of a root's link target (I18); a dangling root link with an absolute target as well as a relative one (I3); the nesting by configured path before the first start (I4); a target whose `..` follows a link (I8); an outer configured through a link with a mount linked back to its root (I19); the own-root test's title, and the refusal text the above-root test asserts (I6, I16). Co-Authored-By: Claude Opus 5.5 --- src/modules/workspace/index.ts | 21 ++++-- src/modules/workspace/observe.ts | 23 +++--- test/workspace-nested-mount-roots.test.ts | 85 +++++++++++++++++++---- 3 files changed, 98 insertions(+), 31 deletions(-) diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index ff70182f7..c02bb6b9d 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -9,7 +9,7 @@ import { isUtf8 } from 'node:buffer'; import { constants as fsConstants } from 'node:fs'; import type { Stats } from 'node:fs'; import { open, readdir, readlink, stat, lstat, realpath } from 'node:fs/promises'; -import { basename, dirname, isAbsolute, join, resolve, relative, sep } from 'node:path'; +import { basename, dirname, isAbsolute, join, parse, resolve, relative, sep } from 'node:path'; import type { JsStore } from '@animalabs/chronicle'; import type { Module, ModuleContext, ProcessState, EventResponse } from '../../types/module.js'; import type { ProcessEvent, ToolDefinition, ToolCall, ToolResult } from '../../types/events.js'; @@ -331,11 +331,15 @@ const LINKS_FOLLOWED = 40; * real path, with the rest as given. A link's target is read from the * link's directory as it is on disk, and nothing is resolved lexically on * the way, so a `..` is taken from where the links before it lead, as the - * kernel takes it. Where a directory can't be resolved for any other - * reason, such as a loop or a lack of permission, the path as resolved - * lexically from there. + * kernel takes it. Where realpath fails for any other reason, such as a + * loop or a lack of permission, the whole path as path.resolve reads it. */ -async function realBeneath(absolute: string, links = 0): Promise { +async function realBeneath(given: string, links = 0): Promise { + // A trailing separator, as a link to a directory is often written, names + // the same place, but readlink would then follow the link it names. + const { root } = parse(given); + let absolute = given; + while (absolute.length > root.length && (absolute.endsWith('/') || absolute.endsWith(sep))) absolute = absolute.slice(0, -1); try { return await realpath(absolute); } catch (err) { @@ -1943,7 +1947,10 @@ export class WorkspaceModule implements Module { const real = reals.get(inner.name)!; const configured = nestedDir(outerPath, resolve(inner.path)); const fromRoots: Array<{ dir: string; root: string }> = []; - for (const dir of [configured, nestedDir(outerReal, real)]) { + // Its directory as configured, read against this mount's root as + // configured and as it really is, and its real root against this + // mount's. + for (const dir of [configured, nestedDir(outerReal, resolve(inner.path)), nestedDir(outerReal, real)]) { if (dir !== undefined && !fromRoots.some((s) => s.dir === dir)) fromRoots.push({ dir, root: join(outerPath, ...dir.split('/')) }); } if (fromRoots.length === 0) continue; @@ -1966,7 +1973,7 @@ export class WorkspaceModule implements Module { return nesting; } - /** The mounts nested in `mountName`, by directory, name and real root (MountView's `nested`), in each spelling known. */ + /** The mounts nested in `mountName`, by directory, name and real root where it has one (MountView's `nested`), in each spelling known. */ private nestedOf(mountName: string): NestedMount[] { return (this.otherMounts.get(mountName) ?? []).flatMap((o) => o.spellings.map((s) => ({ dir: s.dir, name: o.name, real: o.real }))); } diff --git a/src/modules/workspace/observe.ts b/src/modules/workspace/observe.ts index 4ea9e5bab..e0e8fc841 100644 --- a/src/modules/workspace/observe.ts +++ b/src/modules/workspace/observe.ts @@ -64,22 +64,25 @@ export interface MountView { */ others: readonly string[]; /** - * The same mounts, each by its directory, its name and its canonical root, - * for a refusal that names the mount a path leads into: through a symlink - * this mount doesn't follow (linkWords), or through one it follows, or a - * root configured through one (nestedAtReal). Without it, no path is - * refused for where its links lead, and a link's refusal names no mount. + * The same mounts, each by its directory, its name and, where it has one, + * its real root (NestedMount), for a refusal that names the mount a path + * leads into: through a symlink this mount doesn't follow (linkWords), or + * through one it follows, or a root configured through one (nestedAtReal). + * Without it, no path is refused for where its links lead, and a link's + * refusal names no mount. */ nested?: readonly NestedMount[]; } /** * A mount nested in another: its directory in one of its spellings, - * mount-relative and `/`-separated, its name, and its root as realpath - * resolves it (the nearest existing directory's, where the root isn't made - * yet), which every spelling shares. No real root where it's this mount's - * own or holds it: the mount is nested by its configured path alone, since - * through its real root it would claim this mount's every path. + * mount-relative and `/`-separated, its name, and its real root, which + * every spelling shares. The real root is the root as realpath resolves it + * (the nearest existing directory's, where the root isn't made yet) once + * the module's start has read it, and the root as configured until then. + * There's none where it's this mount's own or holds it: the mount is nested + * by its configured path alone, since through its real root it would claim + * this mount's every path. */ export type NestedMount = { dir: string; name: string; real?: string }; diff --git a/test/workspace-nested-mount-roots.test.ts b/test/workspace-nested-mount-roots.test.ts index b01520c35..3510af814 100644 --- a/test/workspace-nested-mount-roots.test.ts +++ b/test/workspace-nested-mount-roots.test.ts @@ -8,8 +8,8 @@ * link into the inner mount's directory let the outer mount read, take in, * write and unlink the inner mount's files through it. And a write through a * link the mount doesn't follow stored a draft no push could write. Each - * root is now resolved once, at construction, and nested where it really - * is; every check that resolves a path canonically refuses one in a nested + * root is now read at each start, awaited, and nested where it really is; + * every check that resolves a path canonically refuses one in a nested * mount's directory; and the file tools refuse such a path before anything * is stored. Separately, a mount configured as `./m` now hears its watcher. */ @@ -121,20 +121,55 @@ describe('a mount root configured through a symlink', () => { }); test('an inner root configured as a link to a directory not made yet inside the outer mount is nested where the link leads', async (t) => { + for (const absolute of [false, true]) { + const env = new Env(t); + env.write('outer/top.txt', "the outer mount's\n"); + symlinkSync(absolute ? env.at('outer/c/inner-dir') : 'outer/c/inner-dir', env.at('via')); + const m = await env.open(t, [mount('outer', env.at('outer')), mount('inner', env.at('via'))]); + // The inner mount's directory is made after the module starts. + env.write('outer/c/inner-dir/x.txt', "the inner mount's\n"); + assert.equal((await call(m, 'sync', {})).success, true); + assert.deepEqual(env.tree('outer'), ['top.txt'], `the outer mount took none of the inner mount's files in (absolute target: ${absolute})`); + const read = await call(m, 'read', { path: 'outer/c/inner-dir/x.txt' }); + assert.equal(read.error, '"outer/c/inner-dir/x.txt" lies in mount "inner", nested in mount "outer", which neither reads nor writes it: use "inner/x.txt".'); + await readsBack(m, 'inner/x.txt', "the inner mount's"); + } + }); + + test('before its first start, the module nests by configured path alone', async (t) => { + // The roots are read at start, awaited; until then a call has the + // nesting the constructor reads from the configured paths. const env = new Env(t); - env.write('outer/top.txt', "the outer mount's\n"); - symlinkSync('outer/c/inner-dir', env.at('via')); - const m = await env.open(t, [mount('outer', env.at('outer')), mount('inner', env.at('via'))]); - // The inner mount's directory is made after the module starts. - env.write('outer/c/inner-dir/x.txt', "the inner mount's\n"); - assert.equal((await call(m, 'sync', {})).success, true); - assert.deepEqual(env.tree('outer'), ['top.txt'], "the outer mount took none of the inner mount's files in"); - const read = await call(m, 'read', { path: 'outer/c/inner-dir/x.txt' }); - assert.equal(read.error, '"outer/c/inner-dir/x.txt" lies in mount "inner", nested in mount "outer", which neither reads nor writes it: use "inner/x.txt".'); - await readsBack(m, 'inner/x.txt', "the inner mount's"); + env.write('outer/a/b/x.txt', "the linked inner mount's\n"); + env.write('outer/c/y.txt', "the inner mount's\n"); + symlinkSync(env.at('outer/a/b'), env.at('via')); + const m = new WorkspaceModule({ mounts: [mount('outer', env.at('outer')), mount('linked', env.at('via')), mount('inner', env.at('outer/c'))] }); + m.initStore(env.store); + t.after(() => m.stop().catch(() => {})); + assert.equal(m.resolveAbsolutePath('outer/c/y.txt'), null, 'nested by its configured path'); + assert.equal(m.resolveAbsolutePath('outer/a/b/x.txt'), join(env.at('outer'), 'a/b/x.txt'), 'not yet placed where its root is'); + await m.start({ isRestart: false, getState: () => null, setState: () => {}, pushEvent: () => {} } as unknown as ModuleContext); + assert.equal(m.resolveAbsolutePath('outer/a/b/x.txt'), null, 'placed at start'); }); - test('a mount configured as a link to the outer mount\'s own root is nested by its configured path alone, and claims none of the outer\'s other paths', async (t) => { + test('a root configured as a link to a link to a place not made yet is placed where that leads, however the target is written', async (t) => { + // w/L2 leads to made-later, not made yet. The inner mount is configured + // as w/in, a link to L2 written plain, with a trailing separator, and + // with a doubled one beneath it (Ogden-2688's case). + for (const [target, dir] of [['L2', 'made-later'], ['L2/', 'made-later'], ['L2//sub', 'made-later/sub']] as const) { + const env = new Env(t); + mkdirSync(env.at('w'), { recursive: true }); + symlinkSync('made-later', env.at('w/L2')); + symlinkSync(target, env.at('w/in')); + const m = await env.open(t, [mount('w', env.at('w')), mount('in', env.at('w/in'))]); + env.write(`w/${dir}/f.txt`, "the inner mount's\n"); + const read = await call(m, 'read', { path: `w/${dir}/f.txt` }); + assert.equal(read.error, `"w/${dir}/f.txt" lies in mount "in", nested in mount "w", which neither reads nor writes it: use "in/f.txt".`, target); + await readsBack(m, 'in/f.txt', "the inner mount's"); + } + }); + + test('a mount configured as a link to the outer mount\'s own root is nested by its configured path alone, and refuses none of the outer mount\'s other paths', async (t) => { const env = new Env(t); env.write('outer/x.txt', "the outer mount's\n"); symlinkSync(env.at('outer'), env.at('outer/self')); @@ -149,6 +184,20 @@ describe('a mount root configured through a symlink', () => { await readsBack(m, 'self/x.txt', "the outer mount's"); }); + test('an outer mount configured through a link keeps the nesting by configured path of a mount configured as a link back to its root', async (t) => { + // ol leads to o, and o/self leads back to o: self's configured path lies + // beneath the outer mount's root as it really is, not as configured. + const env = new Env(t); + env.write('o/x.txt', "the outer mount's\n"); + symlinkSync(env.at('o'), env.at('ol')); + symlinkSync(env.at('o'), env.at('o/self')); + const m = await env.open(t, [mount('outer', env.at('ol'), { followSymlinks: true }), mount('self', env.at('o/self'))]); + assert.equal((await call(m, 'sync', {})).success, true); + assert.deepEqual(env.tree('outer'), ['x.txt'], 'the outer mount passed over the link'); + const through = await call(m, 'read', { path: 'outer/self/x.txt' }); + assert.equal(through.error, '"outer/self/x.txt" lies in mount "self", nested in mount "outer", which neither reads nor writes it: use "self/x.txt".'); + }); + test('a mount configured as a link inside the outer mount to a directory above it is nested by its configured path, and holds the outer mount where it is', async (t) => { const env = new Env(t); env.write('top/top.txt', "the upper mount's\n"); @@ -161,7 +210,7 @@ describe('a mount root configured through a symlink', () => { const through = await call(m, 'read', { path: 'outer/up/top.txt' }); assert.equal(through.error, '"outer/up/top.txt" lies in mount "up", nested in mount "outer", which neither reads nor writes it: use "up/top.txt".'); const write = await call(m, 'write', { path: 'outer/up/new.txt', content: 'outer wrote\n' }); - assert.equal(write.success, false); + assert.equal(write.error, '"outer/up/new.txt" lies in mount "up", nested in mount "outer", which neither reads nor writes it: use "up/new.txt".'); const back = await call(m, 'read', { path: 'up/outer/x.txt' }); assert.equal(back.error, '"up/outer/x.txt" lies in mount "outer", nested in mount "up", which neither reads nor writes it: use "outer/x.txt".'); await readsBack(m, 'up/top.txt', "the upper mount's"); @@ -299,8 +348,16 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli const m = await env.open(t, [mount('work', env.at('work'), { followSymlinks: true }), mount('inner', env.at('work/nested'))]); const write = await call(m, 'write', { path: 'work/alias/flink.txt', content: 'via work\n' }); assert.equal(write.error, leadsInto('work/alias/flink.txt', 'work', 'inner', 'inner/y.txt')); + // A `..` after a link inside the target is taken from where that link + // leads too: x leads to nested/sub/deep, so x/../z.txt is nested/sub/z.txt. + env.write('work/nested/sub/deep/keep.txt', 'inner\n'); + symlinkSync('nested/sub/deep', env.at('work/x')); + symlinkSync('x/../z.txt', env.at('work/flink2.txt')); + const second = await call(m, 'write', { path: 'work/flink2.txt', content: 'via work\n' }); + assert.equal(second.error, leadsInto('work/flink2.txt', 'work', 'inner', 'inner/sub/z.txt')); assert.deepEqual(env.tree('work'), [], 'nothing was stored'); assert.equal(existsSync(env.at('work/nested/y.txt')), false); + assert.equal(existsSync(env.at('work/nested/sub/z.txt')), false); }); test('read_image refuses a path that now leads into it, rather than give the copy held from before', async (t) => { From cc69cc7b342999fa42f6557f09edbd4cee65347f Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 13:13:50 -0400 Subject: [PATCH 06/14] test(workspace): a write through a file link into a nested mount is refused at the tool The file-link test now also writes through the link once it's in place, the third case slimepriestess's anima-research/agent-framework#530 lists ("link first, then the write ... accepted at the tool" on main), and asserts the tools' refusal. Co-Authored-By: Claude Opus 5.5 --- test/workspace-nested-mount-roots.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/workspace-nested-mount-roots.test.ts b/test/workspace-nested-mount-roots.test.ts index 3510af814..d886de3b9 100644 --- a/test/workspace-nested-mount-roots.test.ts +++ b/test/workspace-nested-mount-roots.test.ts @@ -387,6 +387,10 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli assert.equal(skippedReason(sync.data, 'flink.txt'), 'disk could not be checked: "outer/flink.txt" leads into mount "inner", nested in mount "outer": use "inner/x.txt" in its place'); const read = await call(m, 'read', { path: 'outer/flink.txt' }); assert.equal(read.error, leadsInto('outer/flink.txt', 'outer', 'inner', 'inner/x.txt')); + // A write made now, with the link in place, is refused at the tool + // (anima-research/agent-framework#530's third case). + const write = await call(m, 'write', { path: 'outer/flink.txt', content: 'outer wrote\n' }); + assert.equal(write.error, leadsInto('outer/flink.txt', 'outer', 'inner', 'inner/x.txt')); const materialize = await call(m, 'materialize', { path: 'outer/flink.txt', force: true }); assert.equal(materialize.data.count, 0); assert.equal(skippedReason(materialize.data, 'flink.txt'), 'flink.txt: not written: "outer/flink.txt" leads into mount "inner", nested in mount "outer": use "inner/x.txt" in its place'); From 608eafea46d3d2d9338278f78f03d567ab18793e Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 13:19:04 -0400 Subject: [PATCH 07/14] docs(changelog): the nested-roots entry names #530 The forced write through a file link into a nested mount is what slimepriestess's anima-research/agent-framework#530 reports on main. Co-Authored-By: Claude Opus 5.5 --- changelog.d/workspace-nested-mount-roots.fixed.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/workspace-nested-mount-roots.fixed.md b/changelog.d/workspace-nested-mount-roots.fixed.md index 1ef1496a0..2189c6c21 100644 --- a/changelog.d/workspace-nested-mount-roots.fixed.md +++ b/changelog.d/workspace-nested-mount-roots.fixed.md @@ -1,2 +1,2 @@ -- A mount nested in another is now that mount's directory wherever disk puts it, not only where the outer mount's paths name it as written. A root configured through a symlink, or beneath one, is nested where its real path lies; before, it wasn't nested at all, and the outer mount took in and read the inner mount's files. Each start of the module reads each root's real path again, awaited, so a root on a share that stops answering holds that start, not the process; before the first start, mounts are nested by their configured paths alone, as they were. On a mount that follows symlinks, a path that reaches a nested mount's directory through a link is now refused as one written into it: the file tools answer `"work/l/x.txt" leads into mount "inner", nested in mount "work", which neither reads nor writes it: use "inner/sub/x.txt".`, a sync or a materialize leaves it with `"work/l" leads into mount "inner", nested in mount "work": use "inner/sub" in its place`, and `readFileFromDisk` refuses it with the code `other_mount`. Before, the outer mount read, took in, wrote and unlinked the inner mount's files through such a link. On a mount that doesn't follow symlinks, a path through a link into a nested mount's directory is now refused before anything is stored, with the refusal of the link the mount doesn't follow; a `write`, `edit` or `delete` through it used to store a change that no push could write. Where the path is itself a link to a file in a nested mount's directory, `readFileFromDisk` now refuses it with the code `other_mount` and words naming that mount, where it answered `symlink`. +- A mount nested in another is now that mount's directory wherever disk puts it, not only where the outer mount's paths name it as written. A root configured through a symlink, or beneath one, is nested where its real path lies; before, it wasn't nested at all, and the outer mount took in and read the inner mount's files. Each start of the module reads each root's real path again, awaited, so a root on a share that stops answering holds that start, not the process; before the first start, mounts are nested by their configured paths alone, as they were. On a mount that follows symlinks, a path that reaches a nested mount's directory through a link is now refused as one written into it: the file tools answer `"work/l/x.txt" leads into mount "inner", nested in mount "work", which neither reads nor writes it: use "inner/sub/x.txt".`, a sync or a materialize leaves it with `"work/l" leads into mount "inner", nested in mount "work": use "inner/sub" in its place`, and `readFileFromDisk` refuses it with the code `other_mount`. Before, the outer mount read, took in, wrote and unlinked the inner mount's files through such a link (#530). On a mount that doesn't follow symlinks, a path through a link into a nested mount's directory is now refused before anything is stored, with the refusal of the link the mount doesn't follow; a `write`, `edit` or `delete` through it used to store a change that no push could write. Where the path is itself a link to a file in a nested mount's directory, `readFileFromDisk` now refuses it with the code `other_mount` and words naming that mount, where it answered `symlink`. - A mount configured with a relative root written with `./`, such as `./m`, now hears its watcher; it heard no change on disk. From e433d67422a9a81dfbf0df5a3dd990907cd88f10 Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 13:20:35 -0400 Subject: [PATCH 08/14] fix(workspace): start's nesting warning says where the nested mount lies on disk The warning for a directory beyond the configured one said "where its root is on disk", which doesn't fit a directory found by reading the inner mount's configured path against the outer's real root (Ogden-2688, room-203 #146804): there the root may be the outer mount's own. Co-Authored-By: Claude Opus 5.5 --- src/modules/workspace/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index c02bb6b9d..06f563a84 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -1964,7 +1964,7 @@ export class WorkspaceModule implements Module { const placed = fromRoots.filter((s) => s.dir !== configured); if (placed.length > 0) { console.warn( - `[workspace] Mount "${outer.name}" contains mount "${inner.name}" where its root is on disk, ` + + `[workspace] Mount "${outer.name}" contains mount "${inner.name}" where it lies on disk, ` + `at ${placed.map((s) => `${s.dir}/`).join(' and ')} — passing over it as another mount's directory`, ); } From 5be49ca2e6f458b3e002281ceab5f3d9de4efb72 Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 13:27:32 -0400 Subject: [PATCH 09/14] test(workspace): #530's probe, and its unlink side, as tests - test/workspace-three-way.test.ts ends with slimepriestess's probe from anima-research/agent-framework#530, as the issue gives it, for both autoMaterialize values: a forced materialize through a file link into a nested mount must leave the inner mount's file alone. It fails on main's workspace (697e20d6) both ways. - test/workspace-nested-mount-roots.test.ts: a deletion stored while the path was the outer mount's own file, then a link into the nested mount put in its place, is refused by a forced push with applyDeletions, through a final link and through a directory link; the link stays and the inner mount's file is intact. On main the push unlinks the final link, and through the directory link the inner mount's file. Both fail with the base's workspace sources and pass at this commit (J20054). Co-Authored-By: Claude Opus 5.5 --- test/workspace-nested-mount-roots.test.ts | 23 +++++++++++++++++++++++ test/workspace-three-way.test.ts | 17 +++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/test/workspace-nested-mount-roots.test.ts b/test/workspace-nested-mount-roots.test.ts index d886de3b9..f50d5e736 100644 --- a/test/workspace-nested-mount-roots.test.ts +++ b/test/workspace-nested-mount-roots.test.ts @@ -397,6 +397,29 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli assert.equal(readFileSync(env.at('outer/a/b/x.txt'), 'utf8'), "the inner mount's\n"); }); + test('a deletion stored before a link into it took the path\'s place is refused by a forced push, through a final link and through a directory link', async (t) => { + // anima-research/agent-framework#530's unlink side. On `main` the push + // unlinked the final link, and through the directory link it unlinked + // the inner mount's file. + const env = new Env(t); + env.write('outer/a/b/x.txt', "the inner mount's\n"); + env.write('outer/flink.txt', "the outer mount's\n"); + env.write('outer/alias/x.txt', "the outer mount's\n"); + const m = await env.open(t, [mount('outer', env.at('outer'), { followSymlinks: true }), mount('inner', env.at('outer/a/b'))]); + assert.equal((await call(m, 'sync', {})).success, true); + for (const path of ['outer/flink.txt', 'outer/alias/x.txt']) assert.equal((await call(m, 'delete', { path })).success, true, path); + rmSync(env.at('outer/flink.txt')); + symlinkSync(env.at('outer/a/b/x.txt'), env.at('outer/flink.txt')); + rmSync(env.at('outer/alias'), { recursive: true }); + symlinkSync(env.at('outer/a/b'), env.at('outer/alias')); + const finalLink = await call(m, 'materialize', { path: 'outer/flink.txt', force: true, applyDeletions: true }); + assert.equal(skippedReason(finalLink.data, 'flink.txt'), 'flink.txt: not unlinked: "outer/flink.txt" leads into mount "inner", nested in mount "outer": use "inner/x.txt" in its place'); + const throughDir = await call(m, 'materialize', { path: 'outer/alias/x.txt', force: true, applyDeletions: true }); + assert.equal(skippedReason(throughDir.data, 'alias/x.txt'), 'alias/x.txt: not unlinked: "outer/alias" leads into mount "inner", nested in mount "outer": use "inner" in its place'); + assert.equal(lstatSync(env.at('outer/flink.txt')).isSymbolicLink(), true, 'the link stays'); + assert.equal(readFileSync(env.at('outer/a/b/x.txt'), 'utf8'), "the inner mount's\n"); + }); + test('a copy taken in through a file link before the nested mount was configured is no longer vouched for by its fingerprint', async (t) => { // A listing's pass takes a file whose recorded fingerprint still matches // as unchanged, without reading it, so only the observation of the link diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index 611edbca0..f7ac690aa 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -7451,3 +7451,20 @@ describe('the mount root', () => { assert.match(await refused(m, 'sync', { path: 'work/a.txt', acceptRoot: true }), /whole mount/); }); }); + +// anima-research/agent-framework#530 (slimepriestess): an outer mount's forced +// materialize through a file link wrote into a nested mount's file. Her probe, +// as the issue gives it. +for (const autoMaterialize of [false, true]) test(`a forced materialize through a file link into a nested mount leaves the inner mount's file alone (autoMaterialize ${autoMaterialize})`, async (t) => { + const env = new Env(t); + env.writeDisk('a/b/x.txt', "the inner mount's"); + const m = await env.open({}, { mounts: [ + { name: 'work', path: env.dir, mode: 'read-write', watch: 'never', followSymlinks: true, autoMaterialize }, + { name: 'deep', path: env.disk('a/b'), mode: 'read-write', watch: 'never' }, + ] }); + await m.handleToolCall({ id: 't', name: 'write', input: { path: 'work/flink.txt', content: 'outer wrote' } }); + if (existsSync(env.disk('flink.txt'))) rmSync(env.disk('flink.txt')); + symlinkSync(env.disk('a/b/x.txt'), env.disk('flink.txt')); + await m.handleToolCall({ id: 't', name: 'materialize', input: { path: 'work/flink.txt', force: true } }); + assert.equal(readFileSync(env.disk('a/b/x.txt'), 'utf8'), "the inner mount's"); +}); From b7aa5867fd222c7feb61db61453fef3a4a25d497 Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 13:37:59 -0400 Subject: [PATCH 10/14] test(workspace): #530's first two cases, a push of a draft through a file link A draft made while the path was the outer mount's own file, then a file link into the nested mount put in its place, for both autoMaterialize values. An unforced materialize skips the path with a reason naming the path to use, and a forced one skips it as not written, with the same words; the inner mount's file is left. On main the unforced skip advised a forced push, and the forced push wrote into the inner mount's file (anima-research/agent-framework#530's table, first two rows). Fails with the base's workspace sources and passes here. Co-Authored-By: Claude Opus 5.5 --- test/workspace-nested-mount-roots.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/workspace-nested-mount-roots.test.ts b/test/workspace-nested-mount-roots.test.ts index f50d5e736..6314f3fe4 100644 --- a/test/workspace-nested-mount-roots.test.ts +++ b/test/workspace-nested-mount-roots.test.ts @@ -397,6 +397,25 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli assert.equal(readFileSync(env.at('outer/a/b/x.txt'), 'utf8'), "the inner mount's\n"); }); + for (const autoMaterialize of [false, true]) test(`a draft made before a file link into it took the path's place gets the path to use from a push, unforced and forced (autoMaterialize ${autoMaterialize})`, async (t) => { + // anima-research/agent-framework#530's first two cases. On `main` the + // unforced push skipped the path with advice to force it, and the forced + // push wrote into the inner mount's file. + const env = new Env(t); + env.write('outer/a/b/x.txt', "the inner mount's\n"); + const m = await env.open(t, [mount('outer', env.at('outer'), { followSymlinks: true, autoMaterialize }), mount('inner', env.at('outer/a/b'))]); + assert.equal((await call(m, 'write', { path: 'outer/flink.txt', content: 'outer wrote\n' })).success, true); + if (existsSync(env.at('outer/flink.txt'))) rmSync(env.at('outer/flink.txt')); + symlinkSync(env.at('outer/a/b/x.txt'), env.at('outer/flink.txt')); + const unforced = await call(m, 'materialize', { path: 'outer/flink.txt' }); + assert.equal(unforced.data.count, 0); + assert.equal(skippedReason(unforced.data, 'flink.txt'), 'flink.txt: cannot verify disk copy ("outer/flink.txt" leads into mount "inner", nested in mount "outer": use "inner/x.txt" in its place) — fix it and materialize again'); + const forced = await call(m, 'materialize', { path: 'outer/flink.txt', force: true }); + assert.equal(forced.data.count, 0); + assert.equal(skippedReason(forced.data, 'flink.txt'), 'flink.txt: not written: "outer/flink.txt" leads into mount "inner", nested in mount "outer": use "inner/x.txt" in its place'); + assert.equal(readFileSync(env.at('outer/a/b/x.txt'), 'utf8'), "the inner mount's\n"); + }); + test('a deletion stored before a link into it took the path\'s place is refused by a forced push, through a final link and through a directory link', async (t) => { // anima-research/agent-framework#530's unlink side. On `main` the push // unlinked the final link, and through the directory link it unlinked From 98a5dc524ff09c3b6facf8e9e312d90ef71705e7 Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 14:17:43 -0400 Subject: [PATCH 11/14] fix(workspace): find an outer mount's root on an inner's configured path by where it lies on disk readNesting read an inner mount's configured path, as spelled, against the outer mount's real root, so it matched only where the configured path spelled the way to that root as realpath does. Under macOS's temporary directory, reached through /var, a link to /private/var, it never matched there, and a mount configured as a link back to the outer mount's root, with the outer configured through a link, wasn't nested. Each directory above an inner mount's configured path is now placed by realBeneath at start, and the inner is read beneath the outer mount's root from each one that is that root on disk. That includes the reading the old comparison made, where the path spells the root as it really is. A new test reaches the root through a link above it, so it fails without this on Linux too. Co-Authored-By: Claude Opus 5.5 --- src/modules/workspace/index.ts | 42 ++++++++++++++++------- test/workspace-nested-mount-roots.test.ts | 14 ++++++++ 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index 06f563a84..4e7c9568b 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -1920,13 +1920,16 @@ export class WorkspaceModule implements Module { /** * Which mounts are nested in which, read afresh at each start. A mount is - * nested in another where its configured path, or its real root, lies - * beneath the other's. Each root is placed by realBeneath, awaited, so a - * root on a share that stops answering holds this start and not the - * process, and a root moved while the module was stopped is placed where - * it is now. A mount whose real root is the other's own, or holds it, is - * nested by its configured path alone, with no real root, since through it - * it would claim the other's every path. + * nested in another where its configured path lies beneath the other's + * root, as configured or as it lies on disk, whatever links the path + * reaches that root through, or where its real root lies beneath the + * other's. Each root, and each directory above a configured path, is + * placed by realBeneath, awaited, so a root on a share that stops + * answering holds this start and not the process, and a root moved while + * the module was stopped is placed where it is now. A mount whose real + * root is the other's own, or holds it, is nested by its configured path + * alone, with no real root, since through it it would claim the other's + * every path. * * Each way it lies there is a spelling of its directory (`fromRoots`), and * so is the spelling disk lists each of those by, where that differs. On a @@ -1937,7 +1940,20 @@ export class WorkspaceModule implements Module { */ private async readNesting(): Promise> { const reals = new Map(); - for (const mount of this.config.mounts) reals.set(mount.name, await realBeneath(resolve(mount.path))); + // Each directory above a mount's configured path, with where it lies on + // disk, so a path that reaches another mount's root through a link, as + // one under macOS's /var reaches /private/var, is still read beneath it. + const above = new Map>(); + for (const mount of this.config.mounts) { + const given = resolve(mount.path); + reals.set(mount.name, await realBeneath(given)); + const dirs: Array<{ dir: string; real: string }> = []; + for (let dir = dirname(given); ; dir = dirname(dir)) { + dirs.push({ dir, real: await realBeneath(dir) }); + if (dirname(dir) === dir) break; + } + above.set(mount.name, dirs); + } const nesting = new Map(); for (const outer of this.config.mounts) { const outerPath = resolve(outer.path); @@ -1945,12 +1961,14 @@ export class WorkspaceModule implements Module { for (const inner of this.config.mounts) { if (outer === inner) continue; const real = reals.get(inner.name)!; - const configured = nestedDir(outerPath, resolve(inner.path)); + const innerPath = resolve(inner.path); + const configured = nestedDir(outerPath, innerPath); const fromRoots: Array<{ dir: string; root: string }> = []; // Its directory as configured, read against this mount's root as - // configured and as it really is, and its real root against this - // mount's. - for (const dir of [configured, nestedDir(outerReal, resolve(inner.path)), nestedDir(outerReal, real)]) { + // configured and from each directory above it that is this mount's + // root on disk, and its real root against this mount's. + const throughRoot = above.get(inner.name)!.filter((a) => a.real === outerReal).map((a) => nestedDir(a.dir, innerPath)); + for (const dir of [configured, ...throughRoot, nestedDir(outerReal, real)]) { if (dir !== undefined && !fromRoots.some((s) => s.dir === dir)) fromRoots.push({ dir, root: join(outerPath, ...dir.split('/')) }); } if (fromRoots.length === 0) continue; diff --git a/test/workspace-nested-mount-roots.test.ts b/test/workspace-nested-mount-roots.test.ts index 6314f3fe4..ea5c29783 100644 --- a/test/workspace-nested-mount-roots.test.ts +++ b/test/workspace-nested-mount-roots.test.ts @@ -198,6 +198,20 @@ describe('a mount root configured through a symlink', () => { assert.equal(through.error, '"outer/self/x.txt" lies in mount "self", nested in mount "outer", which neither reads nor writes it: use "self/x.txt".'); }); + test('a mount configured as a link back to the outer mount\'s root keeps the nesting by configured path when its path reaches that root through a link above it', async (t) => { + // alias leads to real, as macOS's /var leads to /private/var, and self's + // configured path reaches the outer mount's root through it. + const env = new Env(t); + env.write('real/o/x.txt', "the outer mount's\n"); + symlinkSync(env.at('real'), env.at('alias')); + symlinkSync(env.at('real/o'), env.at('real/o/self')); + const m = await env.open(t, [mount('outer', env.at('real/o'), { followSymlinks: true }), mount('self', env.at('alias/o/self'))]); + assert.equal((await call(m, 'sync', {})).success, true); + assert.deepEqual(env.tree('outer'), ['x.txt'], 'the outer mount passed over the link'); + const through = await call(m, 'read', { path: 'outer/self/x.txt' }); + assert.equal(through.error, '"outer/self/x.txt" lies in mount "self", nested in mount "outer", which neither reads nor writes it: use "self/x.txt".'); + }); + test('a mount configured as a link inside the outer mount to a directory above it is nested by its configured path, and holds the outer mount where it is', async (t) => { const env = new Env(t); env.write('top/top.txt', "the upper mount's\n"); From 6717153ae26e91e059b1bf1870f6c23f5e7d86bf Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 14:35:11 -0400 Subject: [PATCH 12/14] test(workspace): the nesting of a mount configured through a link that leads out of the outer mount The review of #535 offered placing only an inner mount's configured path's parent on disk, in place of each directory above it. Where the path passes through a link inside the outer mount that leads out of it, the parent lies outside the outer mount's root, and with the outer configured through a link that form loses the nesting a mount configured at its real path keeps. This test pins that case. Co-Authored-By: Claude Opus 5.5 --- test/workspace-nested-mount-roots.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/workspace-nested-mount-roots.test.ts b/test/workspace-nested-mount-roots.test.ts index ea5c29783..710579d70 100644 --- a/test/workspace-nested-mount-roots.test.ts +++ b/test/workspace-nested-mount-roots.test.ts @@ -212,6 +212,22 @@ describe('a mount root configured through a symlink', () => { assert.equal(through.error, '"outer/self/x.txt" lies in mount "self", nested in mount "outer", which neither reads nor writes it: use "self/x.txt".'); }); + test('an outer mount configured through a link keeps the nesting by configured path of a mount configured through a link inside it that leads out of it', async (t) => { + // o/via leads to e, outside the outer mount, and e/self back to o. self's + // configured path reaches the outer mount's root at o, above via, so the + // outer mount configured through ol nests it as one configured at o does. + // Its parent placed on disk is e, which isn't beneath that root. + const env = new Env(t); + env.write('o/x.txt', "the outer mount's\n"); + mkdirSync(env.at('e')); + symlinkSync(env.at('e'), env.at('o/via')); + symlinkSync(env.at('o'), env.at('e/self')); + symlinkSync(env.at('o'), env.at('ol')); + const m = await env.open(t, [mount('outer', env.at('ol'), { followSymlinks: true }), mount('self', env.at('o/via/self'))]); + const through = await call(m, 'read', { path: 'outer/via/self/x.txt' }); + assert.equal(through.error, '"outer/via/self/x.txt" lies in mount "self", nested in mount "outer", which neither reads nor writes it: use "self/x.txt".'); + }); + test('a mount configured as a link inside the outer mount to a directory above it is nested by its configured path, and holds the outer mount where it is', async (t) => { const env = new Env(t); env.write('top/top.txt', "the upper mount's\n"); From e631867f6b126fbc0479631561e4f1cca2b23549 Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 14:35:11 -0400 Subject: [PATCH 13/14] docs(workspace): a redirected create, mkdir or unlink can also reach a nested mount's directory The effects' header said a parent replaced in the window before a create, mkdir or unlink can redirect it outside the mount. With mounts nested, the same window can redirect it into a nested mount's directory, where the nested check behind it refuses the write but the empty entry stays, as the review of #535 found by reading. Co-Authored-By: Claude Opus 5.5 --- src/modules/workspace/effects.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/workspace/effects.ts b/src/modules/workspace/effects.ts index de75ec384..264c655ce 100644 --- a/src/modules/workspace/effects.ts +++ b/src/modules/workspace/effects.ts @@ -24,7 +24,8 @@ * Node has no openat, so a create, mkdir or unlink still resolves its path by * name after the parent was checked: a parent replaced concurrently in that * window can redirect that one operation, leaving an empty file or directory - * outside the mount or unlinking a same-named file there. Nor is there an + * outside the mount or in a nested mount's directory, or unlinking a + * same-named file there. Nor is there an * unlink bound to an inode: an unlink checks the entry's identity again just * before it, and a file replaced at its path after that check is the one * unlinked. Content writes stay bound to the descriptor verified before From ca73162517aaf9ad541a9e9412eb0fe018083716 Mon Sep 17 00:00:00 2001 From: Phoebe-2419 Date: Sun, 18 Oct 2026 14:46:22 -0400 Subject: [PATCH 14/14] fix(workspace): grep searches none of a copy held at a path that leads into a nested mount read refuses a path that reaches a nested mount's directory through a link, but grep searched the workspace's copy of each path its pass reported. So a copy held from before the link, or taken in through it before the mount was nested, came back from grep while read refused it. Greptile's review of #535 found it. grep now asks each path whose disk its pass couldn't check the question read asks (leadsIntoNested, taken out of parseFilePath), and lists such a path under skipped with read's refusal instead of searching it. It asks before it compares the pass's branch with the selected one, so a branch selected while it awaits runs the pass again, rather than having one branch's text labelled with another's states, as Langston-2479 found an earlier draft of this did. The held copy test now greps the mount and the path, and a new test selects a branch while grep asks. Co-Authored-By: Claude Opus 5.5 --- src/modules/workspace/index.ts | 55 ++++++++++++++++++----- test/workspace-nested-mount-roots.test.ts | 52 +++++++++++++++++---- 2 files changed, 87 insertions(+), 20 deletions(-) diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index 4e7c9568b..fdc6f62bc 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -2038,22 +2038,31 @@ export class WorkspaceModule implements Module { private async parseFilePath(path: string): Promise<{ mount: MountState; relativePath: string; path: string }> { const parsed = this.refuseNested(await this.parsePath(path)); // A path can also reach a nested mount's directory through a symlink on - // its way. No push of this mount writes it there and no read reads it - // (nestedAtReal), so it's refused here, before anything is stored, and - // the checks where disk is acted on refuse it again. On a mount that - // doesn't follow the link, its refusal is the link's own (linkWords), as - // the readers and pushes give it. A mount with nothing nested has nothing - // to find, so it pays nothing. - const { mount, relativePath } = parsed; + // its way. No push of this mount writes it there and no read reads it, + // so it's refused here, before anything is stored, and the checks where + // disk is acted on refuse it again. + const refusal = await this.leadsIntoNested(parsed.mount, parsed.relativePath, parsed.path); + if (refusal) throw refusal; + return parsed; + } + + /** + * The refusal for a path of `mount` that reaches a nested mount's directory + * through a symlink on its way (nestedAtReal), or null where it doesn't. On + * a mount that doesn't follow the link, the refusal is the link's own + * (linkWords), as the readers and pushes give it. A mount with nothing + * nested has nothing to find, so it pays nothing. + */ + private async leadsIntoNested(mount: MountState, relativePath: string, path: string): Promise { const nested = this.nestedOf(mount.config.name); - if (nested.length === 0) return parsed; + if (nested.length === 0) return null; const holder = nestedAtReal(await realBeneath(resolve(mount.config.path, relativePath)), nested); - if (!holder) return parsed; + if (!holder) return null; if (mount.config.followSymlinks !== true) { const found = await linkOnTheWay({ root: mount.config.path, followSymlinks: false }, relativePath); - if (found !== null) throw await linkReadError(mount, found, parsed.path, nested); + if (found !== null) return await linkReadError(mount, found, path, nested); } - throw new NestedMountError(parsed.path, mount.config.name, holder.name, holder.use, 'leads into'); + return new NestedMountError(path, mount.config.name, holder.name, holder.use, 'leads into'); } /** A parsed path, refused if it lies in a nested mount's directory (parseFilePath). */ @@ -3237,12 +3246,29 @@ export class WorkspaceModule implements Module { // on: one selected in between would label its content with the other // branch's states. So a pass whose branch is no longer selected runs // again, and a branch that keeps changing leaves the scope unsearched. + // + // A copy held at a path that now leads into a nested mount's directory + // through a link is that mount's file too: read refuses it + // (parseFilePath), so grep searches none of it and says why. The pass + // can't check such a path's disk, so only an unverified one can be + // one. Asking awaits disk, so it's done before the branch is + // compared, and a branch selected meanwhile runs the pass again. let pass: PassResult | null = null; + let refused = new Map(); for (let attempt = 0; attempt < 3 && pass === null; attempt++) { const next = await this.passUnlocked(mount, fileScope ? { kind: 'paths', paths: [relativePath] } : { kind: 'dir', dir: start, recursive: true }); - if (next.branchId === undefined || next.branchId === store.currentBranch().id) pass = next; + const asked = new Map(); + for (const report of next.reports.values()) { + if (report.state !== 'unverified') continue; + const refusal = await this.leadsIntoNested(mount, report.path, `${mount.config.name}/${report.path}`); + if (refusal) asked.set(report.path, refusal.message); + } + if (next.branchId === undefined || next.branchId === store.currentBranch().id) { + pass = next; + refused = asked; + } } if (pass === null) { incomplete.push({ @@ -3272,6 +3298,11 @@ export class WorkspaceModule implements Module { // none of this mount's copy of it, as read refuses it. Beneath a // scope it's passed over by design, like the directory. if (inOtherMount(report.path, this.othersOf(mount.config.name))) continue; + const refusal = refused.get(report.path); + if (refusal !== undefined) { + skipped.push({ file, reason: refusal }); + continue; + } // The workspace's version, labelled with the path's state. const entry = store.treeGet(mount.treeStateId, report.path); const blob = entry ? store.getBlob(entry.blobHash) : null; diff --git a/test/workspace-nested-mount-roots.test.ts b/test/workspace-nested-mount-roots.test.ts index 710579d70..8316db73e 100644 --- a/test/workspace-nested-mount-roots.test.ts +++ b/test/workspace-nested-mount-roots.test.ts @@ -91,16 +91,16 @@ function skippedReason(data: any, path: string): string { return entry.reason; } -/** Run `fn` with node:fs/promises' `lstat` replaced by `make(original)`, as the code under test sees it. */ -async function withLstat(make: (original: (...args: any[]) => Promise) => (...args: any[]) => Promise, fn: () => Promise): Promise { +/** Run `fn` with node:fs/promises' `name` replaced by `make(original)`, as the code under test sees it. */ +async function withFsp(name: 'lstat' | 'realpath', make: (original: (...args: any[]) => Promise) => (...args: any[]) => Promise, fn: () => Promise): Promise { const fsp = createRequire(import.meta.url)('node:fs/promises') as Record Promise>; - const original = fsp.lstat!; - fsp.lstat = make(original); + const original = fsp[name]!; + fsp[name] = make(original); syncBuiltinESMExports(); try { return await fn(); } finally { - fsp.lstat = original; + fsp[name] = original; syncBuiltinESMExports(); } } @@ -405,7 +405,7 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli assert.equal(image.error, leadsInto('work/l/img.png', 'work', 'inner', 'inner/sub/img.png')); }); - test('a file link into it is taken in by no sync, and a copy held from before is neither read nor pushed through it', async (t) => { + test('a file link into it is taken in by no sync, and a copy held from before is neither read, searched nor pushed through it', async (t) => { const env = new Env(t); env.write('outer/a/b/x.txt', "the inner mount's\n"); env.write('outer/flink.txt', "the outer mount's copy\n"); @@ -417,6 +417,11 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli assert.equal(skippedReason(sync.data, 'flink.txt'), 'disk could not be checked: "outer/flink.txt" leads into mount "inner", nested in mount "outer": use "inner/x.txt" in its place'); const read = await call(m, 'read', { path: 'outer/flink.txt' }); assert.equal(read.error, leadsInto('outer/flink.txt', 'outer', 'inner', 'inner/x.txt')); + for (const path of ['outer', 'outer/flink.txt']) { + const grep = await call(m, 'grep', { pattern: 'copy', path }); + assert.deepEqual(grep.data.results, [], `grep of ${path} searched none of the copy held from before`); + assert.deepEqual(grep.data.skipped, [{ file: 'outer/flink.txt', reason: leadsInto('outer/flink.txt', 'outer', 'inner', 'inner/x.txt') }], path); + } // A write made now, with the link in place, is refused at the tool // (anima-research/agent-framework#530's third case). const write = await call(m, 'write', { path: 'outer/flink.txt', content: 'outer wrote\n' }); @@ -427,6 +432,37 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli assert.equal(readFileSync(env.at('outer/a/b/x.txt'), 'utf8'), "the inner mount's\n"); }); + test('grep gives each file with the states of the branch its text comes from, when a branch is selected while it asks about a held copy', async (t) => { + // grep asks whether each path whose disk its pass couldn't check leads + // into a nested mount, which awaits a realpath. A branch selected in that + // await mustn't have its text labelled with the states of the pass. + const env = new Env(t); + env.write('work/l/x.txt', 'outer copy\n'); + env.write('work/nested/sub/keep.txt', 'inner\n'); + env.write('work/z.txt', 'main\n'); + const m = await env.open(t, [mount('work', env.at('work'), { followSymlinks: true }), mount('inner', env.at('work/nested'))]); + assert.equal((await call(m, 'sync', {})).success, true); + const main = env.store.currentBranch().name; + env.store.createBranch('other', main); + env.store.switchBranch('other'); + assert.equal((await call(m, 'write', { path: 'work/z.txt', content: 'OTHER\n' })).success, true); + env.store.switchBranch(main); + rmSync(env.at('work/l'), { recursive: true }); + symlinkSync('nested/sub', env.at('work/l')); + let switched = false; + const grep = await withFsp('realpath', (realpath) => async (path: string, ...rest: unknown[]) => { + if (!switched && String(path) === env.at('work/l/x.txt')) { + switched = true; + env.store.switchBranch('other'); + } + return realpath(path, ...rest); + }, () => call(m, 'grep', { pattern: 'main|OTHER', path: 'work' })); + assert.ok(switched, 'a branch was selected while grep asked'); + const settled = await call(m, 'grep', { pattern: 'main|OTHER', path: 'work' }); + assert.notEqual(settled.data.results[0]?.state, 'synced', "z.txt's state on the other branch isn't main's"); + assert.deepEqual(grep.data.results, settled.data.results, "grep gave the other branch's text with that branch's states"); + }); + for (const autoMaterialize of [false, true]) test(`a draft made before a file link into it took the path's place gets the path to use from a push, unforced and forced (autoMaterialize ${autoMaterialize})`, async (t) => { // anima-research/agent-framework#530's first two cases. On `main` the // unforced push skipped the path with advice to force it, and the forced @@ -532,7 +568,7 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli env.write('work/nested/sub/x.txt', 'inner file\n'); const m = await env.open(t, [mount('work', env.at('work'), { followSymlinks: true }), mount('inner', env.at('work/nested'))]); let swapped = false; - await withLstat((lstat) => async (path: string, ...rest: unknown[]) => { + await withFsp('lstat', (lstat) => async (path: string, ...rest: unknown[]) => { // The reader's lstat of the path comes after the tools' check, which // found it the outer mount's: the directory becomes a link between. if (!swapped && String(path) === env.at('work/l/x.txt')) { @@ -563,7 +599,7 @@ describe('a link into a nested mount\'s directory, on a mount that follows symli assert.equal((await call(m, 'sync', {})).success, true); assert.ok(env.tree('work').includes('l/img.png'), 'the outer mount holds the copy'); let swapped = false; - await withLstat((lstat) => async (path: string, ...rest: unknown[]) => { + await withFsp('lstat', (lstat) => async (path: string, ...rest: unknown[]) => { if (!swapped && String(path) === env.at('work/l/img.png')) { swapped = true; renameSync(env.at('work/l'), env.at('work/l-was'));