diff --git a/changelog.d/fix-workspace-file-tools.fixed.md b/changelog.d/fix-workspace-file-tools.fixed.md new file mode 100644 index 000000000..52e51cfea --- /dev/null +++ b/changelog.d/fix-workspace-file-tools.fixed.md @@ -0,0 +1,11 @@ +- The workspace's `edit` writes its replacement as given. It read `$&`, `$$`, `` $` `` and `$'` in `newString` as replacement patterns. It now refuses, changing nothing, four edits it used to report as successes: + - an edit of a file that isn't valid UTF-8, where decoding and writing it back turned every invalid byte sequence in the file into U+FFFD; + - an empty `oldString`, which matched between every character; + - a `replaceAll` whose `oldString` isn't in the file, which stored the file unchanged; + - a lone UTF-16 surrogate in `newString`, which was written as U+FFFD. One in `oldString`, which used to miss, is refused the same way. `write` now refuses one in `content` too, as every tool does in a path. +- Every workspace tool checks its input against its own schema before it does anything: required fields, strings, booleans, and whole numbers, with `read`'s `offset` and `limit` at least 1 and `grep`'s context lines at least 0. An edit missing `newString` wrote the word "undefined", and `replaceAll: "false"` counted as true. A typed field given as `null`, or a number given as a string, is refused too, as `read` already refused them for `offsetChars` and `limitChars`. Before, some handlers read `null` as the field's default and did arithmetic on a numeric string. `read`'s `offset` and `limit`, and `grep`'s `contextBefore` and `contextAfter`, are typed `integer` in their schemas. +- A `write`, `edit` or `delete` that the workspace stored but couldn't finish still answers as an error. That covers one whose push to disk didn't complete, and one that failed after its change was stored. The answer now leads with the fact that the change is stored and mustn't be sent again, then gives the push's reason, which says what disk holds and the way out. All of that is in its text, which is what the model and a `code_execution` script receive of an error. Before, a refused push's answer began "Wrote to Chronicle", "Edited Chronicle" or "Removed from Chronicle", and a failure after the change was stored gave only the error. Either way, an edit sent again could apply twice. +- A refused push's way out names the tool that takes it, `materialize it with force`, where it said `pass force`. For a deletion it adds `applyDeletions`, which materialize needs to unlink anything. A disk copy that can't be verified is told to fix it and materialize again. `force` is named beside that only where a forced push gets past what kept the copy from being verified. A file its permissions keep the workspace from reading is offered force if disk lets the workspace write it, or, for a deletion, unlink it, since a read's refusal doesn't show either. For a deletion, something that isn't a regular file is offered force as it is. Before, force was named for every such copy, though a forced push leaves a symlink the mount doesn't follow, or a FIFO a write meets, as it is. A path whose branch intent can't be read is told to sync the path, which replaces the intent. Where disk's copy can't be verified either, it's told that first, since a sync replaces the intent only once it can see disk's copy. Before, it was told to fix it and materialize again, which is refused the same way until a sync does. `ls`'s description names `applyDeletions` beside `force` for a path the workspace deleted. +- `read` counts the lines a file has: a final line end no longer starts an empty extra line, so "a\nb\n" is two lines, and an empty file has none. `grep` numbers lines the same way. An `offset` past the last line answers empty `content` with the file's `totalLines`, no `fromLine` or `toLine`, and a note saying where the file ends. Before, it claimed a range that ended before it began. Reading an empty file from line 1 answers `totalLines: 0` with no note. +- An `edit` first takes in a change on disk to a file the workspace holds, as a listing does, and applies to that text. Its answer says so when disk's copy had changed since the workspace last agreed with it, when disk had deleted the file, or when disk's copy couldn't be checked and it used the workspace's copy. Before, it edited the older stored copy, and on a mount that pushes, its push then failed as stale. When disk has no copy and nothing records whether it was deleted there, the edit uses the workspace's copy, as before. That copy goes back to disk with the edit's own push on a mount that pushes, or when the path is materialized on one that doesn't, and the edit's answer now says which. Where the path's stored copy is a draft and disk changed too, the edit applies to the draft as before, and its answer names the conflict and the two ways out, as does an edit refused there for what it found. `write` doesn't look at disk first, since it replaces the whole file; as before, it only settles a pending intent. +- `grep`'s refusal of an invalid pattern gives the parser's reason, and the workspace's refusals no longer begin with "Error: ". A push that disk's shape refuses says what's in the way and the way out: a directory at the path, or a file or a symlink where the path needs a directory. Before, it offered `force`, which can't write or unlink there either, and a file in the way named no way out. diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index ae6c172b6..64880421d 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -5,6 +5,7 @@ * auto-sync between real filesystem and Chronicle, and manual materialization. */ +import { isUtf8 } from 'node:buffer'; import { constants as fsConstants } from 'node:fs'; import type { Stats } from 'node:fs'; import { open, readFile, stat, lstat, realpath } from 'node:fs/promises'; @@ -1120,8 +1121,8 @@ export class WorkspaceModule implements Module { type: 'object' as const, properties: { path: { type: 'string', description: 'File path (mount-prefixed, e.g., "project/src/main.ts")' }, - offset: { type: 'number', description: 'Starting line number (1-indexed)' }, - limit: { type: 'number', description: 'Maximum number of lines to return (default 2000)' }, + offset: { type: 'integer', description: 'Starting line number (1-indexed)' }, + limit: { type: 'integer', description: 'Maximum number of lines to return (default 2000)' }, offsetChars: { type: 'integer', description: 'Character paging: zero-based UTF-16 offset (default 0); use nextOffsetChars to continue' }, limitChars: { type: 'integer', description: 'Character paging: code units to return (default 2000), plus at most one to keep a surrogate pair intact. Use the limit suggested by a spill notice.' }, }, @@ -1185,7 +1186,8 @@ export class WorkspaceModule implements Module { + 'still on disk); not-in-branch (on disk, absent from this branch\'s workspace); disk-only (binary or over ' + 'the size limit, never stored — shown with size and image type); conflict, with its kind (disk and the ' + 'workspace both changed; disk holds a different copy and nothing records which is newer; or a write was ' - + 'interrupted) — sync the path to take disk, or materialize that path with force to take the workspace; ' + + 'interrupted) — sync the path to take disk, or materialize that path with force to take the workspace, ' + + 'with applyDeletions too where the workspace deleted it; ' + 'disk-missing-provenance-unknown; unverified (disk could not be checked). `incomplete` names regions ' + 'the listing could not see.', inputSchema: { @@ -1221,8 +1223,8 @@ export class WorkspaceModule implements Module { pattern: { type: 'string', description: 'Regular expression pattern' }, path: { type: 'string', description: 'File or directory to search (mount-prefixed, optional)' }, glob: { type: 'string', description: 'Glob pattern to filter files' }, - contextBefore: { type: 'number', description: 'Context lines before match' }, - contextAfter: { type: 'number', description: 'Context lines after match' }, + contextBefore: { type: 'integer', description: 'Context lines before match' }, + contextAfter: { type: 'integer', description: 'Context lines after match' }, }, required: ['pattern'], }, @@ -1335,6 +1337,13 @@ export class WorkspaceModule implements Module { async handleToolCall(call: ToolCall): Promise { if (!this.running()) return { success: false, isError: true, error: NOT_RUNNING }; try { + // Every handler reads its input as its schema describes it; a call + // that sends something else is refused here, before anything is read. + const schema = this.getTools().find((tool) => tool.name === call.name)?.inputSchema; + const problem = schema ? inputProblem(schema, call.input, INPUT_MINIMUMS[call.name]) : null; + if (problem !== null) { + return { success: false, isError: true, error: `The ${call.name} call did nothing: ${problem}.` }; + } const input = call.input as Record; const generated = typeof input?.path === 'string' ? this.findGeneratedTextFile(input.path) : undefined; if (generated) { @@ -1359,7 +1368,7 @@ export class WorkspaceModule implements Module { return { success: false, error: `Unknown tool: ${call.name}`, isError: true }; } } catch (err) { - return { success: false, error: String(err), isError: true }; + return { success: false, error: err instanceof Error ? err.message : String(err), isError: true }; } } @@ -1608,6 +1617,11 @@ export class WorkspaceModule implements Module { * Parse a mount-prefixed path into (mountName, relativePath). */ private parsePath(path: string): { mount: MountState; relativePath: string } { + // A file's name on disk is UTF-8, which can't hold a lone surrogate: the + // store would keep one name and disk another. + if (LONE_SURROGATE.test(path)) { + throw new Error(`The path ${JSON.stringify(path)} holds a lone UTF-16 surrogate, which UTF-8 can't store, so no file on disk can have that name. Nothing was done.`); + } const slashIdx = path.indexOf('/'); const mountName = slashIdx >= 0 ? path.slice(0, slashIdx) : path; const relativePath = slashIdx >= 0 ? path.slice(slashIdx + 1) : ''; @@ -1951,6 +1965,14 @@ export class WorkspaceModule implements Module { } } + /** + * Whether a tool's change on this mount goes to disk as it commits: the + * push commitToolChange makes, and what an edit's answer says of it. + */ + private pushesOnCommit(mount: MountState): boolean { + return mount.config.autoMaterialize === true && mount.config.mode !== 'read-only'; + } + /** * Commit a tool's write or delete to the workspace tree, with the branch * intent it implies (store origin for a path disk never agreed with; a @@ -1963,7 +1985,8 @@ export class WorkspaceModule implements Module { * Returns the reason when the change reached the workspace but not disk, so * callers surface it in the tool result: the autoMaterialize contract is * that disk is the source of truth for downstream agents. The caller holds - * the mount's turn and has settled the path first (settleBeforeMutation). + * the mount's turn and has settled the path first (settleBeforeMutation, or + * a pass over the path, as an edit makes). */ private async commitToolChange( mount: MountState, @@ -1994,13 +2017,14 @@ export class WorkspaceModule implements Module { }); } else { store.treeRemove(mount.treeStateId, relativePath); + onCommitted?.(); intents.update(relativePath, (cur) => { const next = { ...cur, tombstone: { hash: change.previousHash } }; delete next.origin; return next; }); } - if (!mount.config.autoMaterialize || mount.config.mode === 'read-only') return null; + if (!this.pushesOnCommit(mount)) return null; const pushed = await this.pushUnlocked(mount, [relativePath], { applyDeletions: change.kind === 'remove', @@ -2077,15 +2101,10 @@ export class WorkspaceModule implements Module { const characterPaging = input.offsetChars !== undefined || input.limitChars !== undefined; const offsetChars = input.offsetChars ?? 0; const limitChars = input.limitChars ?? 2000; - if (characterPaging) { - if (input.offset !== undefined || input.limit !== undefined) { - return { success: false, isError: true, error: 'Use either offset/limit (lines) or offsetChars/limitChars (characters), not both.' }; - } - if (!Number.isSafeInteger(offsetChars) || offsetChars < 0 - || !Number.isSafeInteger(limitChars) || limitChars < 1 - || input.offsetChars === null || input.limitChars === null) { - return { success: false, isError: true, error: 'offsetChars must be a non-negative safe integer and limitChars a positive safe integer.' }; - } + // Each number is a whole number of at least its schema's minimum by now + // (inputProblem). + if (characterPaging && (input.offset !== undefined || input.limit !== undefined)) { + return { success: false, isError: true, error: 'Use either offset/limit (lines) or offsetChars/limitChars (characters), not both.' }; } let content = generatedContent; if (content === undefined) { @@ -2124,7 +2143,7 @@ export class WorkspaceModule implements Module { }, }; } - const lines = content.split('\n'); + const lines = linesOf(content); // Apply offset/limit. An unlimited read of a large file would inject the // whole thing into the turn (a 2MB file ≈ 600k tokens → the request blows @@ -2132,6 +2151,22 @@ export class WorkspaceModule implements Module { // default cap; the result reports total/from/to so the agent can page. const DEFAULT_READ_LINE_LIMIT = 2000; const startLine = (input.offset ?? 1) - 1; // Convert to 0-indexed + if (startLine >= lines.length) { + // Nothing to show: an empty file, or an offset past the last line. No + // range is claimed, and the line count says where the file ends. + return { + success: true, + data: { + path: input.path, + totalLines: lines.length, + // Reading an empty file from its start is reading it whole. + ...(startLine > 0 + ? { note: `Line ${startLine + 1} is past the end: the file has ${lines.length} line${lines.length === 1 ? '' : 's'}.` } + : {}), + content: '', + }, + }; + } const effectiveLimit = input.limit ?? DEFAULT_READ_LINE_LIMIT; const endLine = startLine + effectiveLimit; const slice = lines.slice(startLine, endLine); @@ -2230,6 +2265,9 @@ export class WorkspaceModule implements Module { } private async handleWrite(input: WriteInput): Promise { + if (LONE_SURROGATE.test(input.content)) { + return { success: false, isError: true, error: 'The write did nothing: "content" holds a lone UTF-16 surrogate, which UTF-8 can\'t store.' }; + } const { mount, relativePath } = this.parsePath(input.path); if (mount.config.mode === 'read-only') { return { success: false, error: `Mount "${mount.config.name}" is read-only`, isError: true }; @@ -2242,95 +2280,164 @@ export class WorkspaceModule implements Module { } const buffer = Buffer.from(input.content, 'utf-8'); - const materializeError = await this.withMount(mount, async () => { - await this.settleBeforeMutation(mount, relativePath); - const blobHash = store.storeBlob(buffer, 'text/plain'); - return this.commitToolChange(mount, relativePath, { kind: 'set', blobHash, size: buffer.byteLength }); - }); - if (materializeError) { - return { - success: false, - error: `Wrote to Chronicle but failed to materialize "${input.path}" to disk: ${materializeError}. Downstream agents will not see this change until the next materialize call.`, - isError: true, - }; + const receipt = { path: input.path, size: buffer.byteLength, hash: hashContent(input.content) }; + let committed = false; + let refused: string | null; + try { + refused = await this.withMount(mount, async () => { + await this.settleBeforeMutation(mount, relativePath); + const blobHash = store.storeBlob(buffer, 'text/plain'); + return this.commitToolChange(mount, relativePath, { kind: 'set', blobHash, size: buffer.byteLength }, + () => { committed = true; }); + }); + } catch (error) { + if (!committed) throw error; + return storedButUnfinished('write', input.path, { threw: error instanceof Error ? error.message : String(error) }); } + if (refused !== null) return storedButUnfinished('write', input.path, { refused }); - return { - success: true, - data: { - path: input.path, - size: Buffer.byteLength(input.content), - hash: hashContent(input.content), - }, - }; + return { success: true, data: receipt }; } private async handleEdit(input: EditInput): Promise { + // Refused before anything is read: an empty oldString matches between + // every character, and UTF-8 can't store a lone surrogate. + if (input.oldString === '') { + return { success: false, isError: true, error: 'The edit did nothing: "oldString" is empty, and an empty string matches between every character. Give the text to replace.' }; + } + for (const field of ['oldString', 'newString'] as const) { + if (LONE_SURROGATE.test(input[field])) { + return { success: false, isError: true, error: `The edit did nothing: "${field}" holds a lone UTF-16 surrogate, which UTF-8 can't store.` }; + } + } const { mount, relativePath } = this.parsePath(input.path); if (mount.config.mode === 'read-only') { return { success: false, error: `Mount "${mount.config.name}" is read-only`, isError: true }; } const store = this.getStore(); - return this.withMount(mount, async (): Promise => { - const report = await this.ensureSynced(mount, relativePath); - await this.settleBeforeMutation(mount, relativePath); // before the edit reads the entry + let committed = false; + let appliedNote: string | null = null; + try { + return await this.withMount(mount, async (): Promise => { + // The edit applies to the path as disk holds it now: a change on disk + // to a file the workspace holds is taken in first, by the rule a + // listing uses. The pass settles a pending intent on the path too, as + // settleBeforeMutation's would, and the commit enforces the rest + // (commitToolChange). A write passes over its path only to settle a + // pending intent, since it replaces the whole file. + const pass = await this.passUnlocked(mount, { kind: 'paths', paths: [relativePath] }); + const report = pass.reports.get(relativePath); + const took = (op: TreeOp) => pass.ops.some((o) => o.path === relativePath && o.op === op); + // Which copy the edit used, when it isn't the one the workspace last + // agreed on with disk: disk's newer text, or the workspace's copy + // because disk's couldn't be read or disk has none. A file the + // workspace didn't hold has no older copy, so taking it in needs no + // word. + const whichCopy = (verb: string): string | null => took('modified') + ? `Disk's copy had changed since the workspace last agreed with it, so it was taken in first, and the edit ${verb} that text.` + : report?.state === 'unverified' + ? `The path couldn't be checked against disk (${report.note ?? 'not observed'}), so the edit ${verb} the workspace's copy.` + : report?.state === 'disk-missing-provenance-unknown' + ? `Disk has no copy of this file, and nothing records whether it was deleted there, so the edit ${verb} the workspace's copy.` + : null; + // A conflict the pass found is named when the edit is refused for the + // text it found, on either kind of mount, and on a success where the + // mount doesn't push, since such a mount keeps the conflict until + // materialize or sync settles it. A pushing mount's success met none + // (below). A stored edit that couldn't finish leaves it to the push's + // reason, which names the conflict, or to ls, which its answer points to. + const conflict = report?.state === 'conflict' ? report.conflict?.kind ?? 'both-changed' : null; + const conflictNote = (where: string): string | null => conflict === null ? null + : `The path is in conflict with disk (${conflict}): the edit ${where} the workspace's copy. Sync the path to take disk's copy instead, or materialize it with force to write the workspace's over disk's.`; + const suffix = [whichCopy('looked in'), conflictNote('looked in')].filter((n) => n !== null).map((n) => ` ${n}`).join(''); + appliedNote = whichCopy('applied to'); - const entry = store.treeGet(mount.treeStateId, relativePath); - if (!entry) { - return { success: false, error: this.notInWorkspace(input.path, report), isError: true }; - } + const entry = store.treeGet(mount.treeStateId, relativePath); + if (!entry) { + // A pass also drops a copy disk made binary or too large, which + // the disk-only answer describes; any other drop is disk's deletion. + const error = took('deleted') && report?.state !== 'disk-only' + ? `File not found: ${input.path}. Disk's copy was deleted since the workspace last agreed with it, and that was taken in first.` + : this.notInWorkspace(input.path, report); + return { success: false, error, isError: true }; + } - const blob = store.getBlob(entry.blobHash); - if (!blob) { - return { success: false, error: `Blob not found for: ${input.path}`, isError: true }; - } + const blob = store.getBlob(entry.blobHash); + if (!blob) { + return { success: false, error: `Blob not found for: ${input.path}`, isError: true }; + } + // Decoding would turn each invalid sequence into U+FFFD, and writing + // the text back would change bytes the edit never named. + if (!isUtf8(blob)) { + return { + success: false, + isError: true, + error: `The edit did nothing: ${input.path} isn't valid UTF-8, so editing it would change bytes outside the text it replaces. ` + + `Read shows each invalid byte sequence as U+FFFD, and a write replaces the whole file as UTF-8, so writing back what read shows puts U+FFFD in their place.${suffix}`, + }; + } - let content = blob.toString('utf-8'); + let content = blob.toString('utf-8'); - // Validate uniqueness - if (!input.replaceAll) { - const count = content.split(input.oldString).length - 1; + // An edit that would replace nothing is refused, replaceAll or not; + // without replaceAll, the text must occur once, and the refusal says + // how often it does. With replaceAll, whether it occurs is enough. + const count = input.replaceAll + ? Number(content.includes(input.oldString)) + : content.split(input.oldString).length - 1; if (count === 0) { - return { success: false, error: `String not found in ${input.path}`, isError: true }; + return { success: false, error: `String not found in ${input.path}.${suffix}`, isError: true }; } - if (count > 1) { + if (count > 1 && !input.replaceAll) { return { success: false, - error: `String found ${count} times in ${input.path}. Use replaceAll: true or provide more context.`, + error: `String found ${count} times in ${input.path}. Use replaceAll: true or provide more context.${suffix}`, isError: true, }; } - } - content = input.replaceAll - ? content.replaceAll(input.oldString, input.newString) - : content.replace(input.oldString, input.newString); - - const newBuffer = Buffer.from(content, 'utf-8'); - const newBlobHash = store.storeBlob(newBuffer, 'text/plain'); - const materializeError = await this.commitToolChange(mount, relativePath, { - kind: 'set', - blobHash: newBlobHash, - size: newBuffer.byteLength, - mode: entry.mode, - }); - if (materializeError) { + // A replacer function, so that the new text goes in as given: in a + // replacement string, $&, $$, $' and $` are patterns. + content = input.replaceAll + ? content.replaceAll(input.oldString, () => input.newString) + : content.replace(input.oldString, () => input.newString); + + const newBuffer = Buffer.from(content, 'utf-8'); + const newBlobHash = store.storeBlob(newBuffer, 'text/plain'); + const refused = await this.commitToolChange(mount, relativePath, { + kind: 'set', + blobHash: newBlobHash, + size: newBuffer.byteLength, + mode: entry.mode, + }, () => { committed = true; }); + if (refused !== null) return storedButUnfinished('edit', input.path, { refused }, appliedNote); + + // The edit makes the path the workspace's own change (commitToolChange), + // so a file disk lacked goes back with this edit's push, or with the + // next materialize where the mount doesn't push. A push that didn't + // complete answers above, with its reason. One that completed met no + // conflict, since this push, unforced, stops at one: the edit made the + // two copies agree, which settles it, or there was none. So only a + // mount that doesn't push still holds the conflict the pass found. + const pushes = this.pushesOnCommit(mount); + const backOnDisk = report?.state === 'disk-missing-provenance-unknown' + ? `${pushes ? 'Its push put the file back on disk' : 'A materialize of the path puts it back on disk'}; if it was deleted there on purpose, delete it here too.` + : null; + const notes = [appliedNote, backOnDisk, pushes ? null : conflictNote('is in')].filter((n): n is string => n !== null); return { - success: false, - error: `Edited Chronicle but failed to materialize "${input.path}" to disk: ${materializeError}. Downstream agents will not see this change until the next materialize call.`, - isError: true, + success: true, + data: { + path: input.path, + size: newBuffer.byteLength, + ...(notes.length > 0 ? { note: notes.join(' ') } : {}), + }, }; - } - - return { - success: true, - data: { - path: input.path, - size: newBuffer.byteLength, - }, - }; - }); + }); + } catch (error) { + if (!committed) throw error; + return storedButUnfinished('edit', input.path, { threw: error instanceof Error ? error.message : String(error) }, appliedNote); + } } private async handleDelete(input: DeleteInput): Promise { @@ -2340,28 +2447,28 @@ export class WorkspaceModule implements Module { } const store = this.getStore(); - return this.withMount(mount, async (): Promise => { - const report = await this.ensureSynced(mount, relativePath); - await this.settleBeforeMutation(mount, relativePath); - const entry = store.treeGet(mount.treeStateId, relativePath); - if (!entry) { - return { success: false, error: this.notInWorkspace(input.path, report), isError: true }; - } + let committed = false; + try { + return await this.withMount(mount, async (): Promise => { + const report = await this.ensureSynced(mount, relativePath); + await this.settleBeforeMutation(mount, relativePath); + const entry = store.treeGet(mount.treeStateId, relativePath); + if (!entry) { + return { success: false, error: this.notInWorkspace(input.path, report), isError: true }; + } - const materializeError = await this.commitToolChange(mount, relativePath, { kind: 'remove', previousHash: entry.blobHash }); - if (materializeError) { - return { - success: false, - error: `Removed from Chronicle but did not unlink "${input.path}" from disk: ${materializeError}. ` + - 'The disk copy stays (ls shows its state) until materialize with applyDeletions removes it.', - isError: true, - }; - } + const refused = await this.commitToolChange(mount, relativePath, { kind: 'remove', previousHash: entry.blobHash }, + () => { committed = true; }); + if (refused !== null) return storedButUnfinished('delete', input.path, { refused }); - // Its whole receipt is what it changed: kept in the stub if the guard - // withholds the result (agent-framework #277). - return { success: true, data: { path: input.path, deleted: true }, keepWhenWithheld: ['path', 'deleted'] }; - }); + // Its whole receipt is what it changed: kept in the stub if the guard + // withholds the result (agent-framework #277). + return { success: true, data: { path: input.path, deleted: true }, keepWhenWithheld: ['path', 'deleted'] }; + }); + } catch (error) { + if (!committed) throw error; + return storedButUnfinished('delete', input.path, { threw: error instanceof Error ? error.message : String(error) }); + } } private async handleLs(input: LsInput): Promise { @@ -2464,7 +2571,8 @@ export class WorkspaceModule implements Module { try { regex = new RegExp(input.pattern); } catch (e) { - return { success: false, error: `Invalid regex: ${input.pattern}`, isError: true }; + // The parser's reason says where the pattern goes wrong. + return { success: false, error: `The grep did nothing: ${e instanceof Error ? e.message : String(e)}.`, isError: true }; } const fileGlob = input.glob ? globToRegex(input.glob) : null; @@ -2481,7 +2589,7 @@ export class WorkspaceModule implements Module { const incomplete: Array<{ path: string; reason: string }> = []; const search = (content: string): GrepMatch[] => { - const lines = content.split('\n'); + const lines = linesOf(content); const found: GrepMatch[] = []; for (let i = 0; i < lines.length; i++) { if (regex.test(lines[i])) { @@ -3198,6 +3306,99 @@ function stateView(report: PathReport): Record { // Utilities // ========================================================================== +/** A lone UTF-16 surrogate, which UTF-8 can't store: in `u` mode, a pair is one code point. */ +const LONE_SURROGATE = /\p{Cs}/u; + +/** + * The answer to a write, edit or delete that the workspace stored but + * couldn't finish: its push to disk didn't complete, or what follows the tree + * entry threw. It stays an error, since part of the call failed, but it leads + * with what the caller must know before acting on it: the change is stored, + * and sending it again could apply it twice. The push's reason says what disk + * holds and the way out, as materialize takes it. The text carries all of it, + * since an error's data reaches neither the model nor a code_execution + * script. An edit's note on which copy it applied to comes last. + */ +function storedButUnfinished( + tool: 'write' | 'edit' | 'delete', + path: string, + outcome: { refused: string } | { threw: string }, + note: string | null = null, +): ToolResult { + const change = tool === 'delete' ? 'deletion' : tool; + const stored = `The ${change} of "${path}" is stored in the workspace, so don't send it again` + + (tool === 'edit' ? ': a second edit can apply the change twice.' : '.'); + const rest = 'refused' in outcome + ? ` Its push to disk didn't complete: ${outcome.refused}.` + : ` The rest of the call failed: ${outcome.threw}. ls shows the path's state.`; + return { success: false, isError: true, error: stored + rest + (note !== null ? ` ${note}` : '') }; +} + +/** + * A text's lines, as read and grep number them: a final line end ends the + * last line rather than starting an empty one, so "a\nb\n" is two lines, + * and an empty text has none. + */ +function linesOf(content: string): string[] { + const lines = content.split('\n'); + if (lines.at(-1) === '') lines.pop(); + return lines; +} + +/** + * The least each tool's whole-number fields may be, beside their schemas, + * whose type (membrane's ToolParameter) has no `minimum`. + */ +const INPUT_MINIMUMS: Record> = { + read: { offset: 1, limit: 1, offsetChars: 0, limitChars: 1 }, + grep: { contextBefore: 0, contextAfter: 0 }, +}; + +/** + * Why a tool call's input doesn't fit its tool's schema, or null when it does: + * an object, each required field present, and each field its schema calls a + * string, a boolean or an integer of that type, an integer at least its + * minimum. The workspace's schemas use no other types. Without it, an edit + * missing `newString` wrote the word "undefined", and an offset of 1.5 + * numbered lines 1.5, 2.5 and on. A field the schema doesn't describe is left + * to its handler. + */ +function inputProblem( + schema: { properties?: Record; required?: string[] }, + input: unknown, + minimums: Record = {}, +): string | null { + if (input === null || typeof input !== 'object' || Array.isArray(input)) { + return 'its input must be an object of the fields its schema lists'; + } + const fields = input as Record; + for (const name of schema.required ?? []) { + if (fields[name] === undefined) return `"${name}" is required`; + } + for (const [name, value] of Object.entries(fields)) { + const type = (schema.properties?.[name] as { type?: string } | undefined)?.type; + if (value === undefined || type === undefined) continue; + const minimum = minimums[name]; + const fits = type === 'string' ? typeof value === 'string' + : type === 'boolean' ? typeof value === 'boolean' + : type === 'integer' + ? Number.isSafeInteger(value) && (minimum === undefined || (value as number) >= minimum) + : true; + if (!fits) { + const wanted = type === 'string' ? 'a string' + : type === 'boolean' ? 'true or false' + : minimum === undefined ? 'a whole number' : `a whole number of at least ${minimum}`; + const got = value === null ? 'null' + : typeof value === 'number' ? String(value) + : Array.isArray(value) ? 'an array' + : typeof value === 'object' ? 'an object' + : `a ${typeof value}`; + return `"${name}" must be ${wanted}, not ${got}`; + } + } + return null; +} + /** * Convert a glob pattern to a RegExp. Throws, naming the pattern, for a brace * group it can't read: a "{" that never closes, a group inside another, or a diff --git a/src/modules/workspace/reconcile.ts b/src/modules/workspace/reconcile.ts index d2b0a4b13..8f2ab7d79 100644 --- a/src/modules/workspace/reconcile.ts +++ b/src/modules/workspace/reconcile.ts @@ -340,6 +340,47 @@ function settleAdoption(p: Pending, s: { hash: string; size: number } | null): A return p.prior ?? 'forget'; } +/** + * What's in the way, and the way out, when disk's shape keeps a push from a + * path: no push writes a file over a directory or unlinks one, or writes + * below a file or a symlink the walk won't take (`writeContained`, + * `unlinkContained`), forced or not. Null for any other reason, which is + * passed on as it is. + */ +function inTheWay(reason: string | undefined, deletion: boolean): string | null { + if (reason === 'a directory') { + return deletion + ? 'disk has a directory at this path, and no push unlinks one, forced or not — if it should go, move or remove it on disk' + : 'disk has a directory at this path, and no push writes a file over one, forced or not — ' + + 'move or remove that directory on disk and materialize again, or write the file under another path'; + } + // A deletion below a file finds nothing there to unlink, so only a write + // meets this one. + if (reason === 'a parent path is not a directory') { + return 'disk has a file or a symlink where this path needs a directory, and no push writes below one, forced or not — ' + + 'move or remove it on disk and materialize again, or write the file under another path'; + } + return null; +} + +/** + * The forced way out the refusal of a disk copy that couldn't be verified + * names, if any. A forced push skips the verification, so it gets past the + * note only where its effect can go on. A file the module can't read (EACCES) + * is overwritten, or unlinked, where disk lets the workspace write it, or + * unlink it, which a read's EACCES doesn't show: force is named on that + * condition. What isn't a regular file is unlinked by a forced deletion. No + * other note names force: a forced push stops at some (a symlink the mount + * doesn't follow, a FIFO for a write, a parent it can't use); another writer + * still at the file ("the file changed while it was read") is answered by + * materializing again; and the rest aren't known to pass. + */ +function forcedWay(note: string | undefined, deletion: boolean, forced: string): string { + if (note === 'cannot open (EACCES)') return `, or, if disk lets the workspace ${deletion ? 'unlink' : 'write'} the file, ${forced}`; + if (deletion && note === 'not a regular file') return `, or ${forced}`; + return ''; +} + /** * The synchronous precondition of every workspace mutation: evidence owed a * barrier is made durable first, and an adoption left pending on the @@ -1020,6 +1061,10 @@ export async function pushPaths( const p = agreement.get(mount.name, path); const bi = mount.intents.get(path); const v = decide(d, s, p, bi, branchId, false); + // The ways out a refusal names, as materialize takes them: a deletion + // reaches disk only with applyDeletions. + const again = s ? 'materialize again' : 'materialize again with applyDeletions'; + const forced = s ? 'materialize it with force to overwrite it' : 'materialize it with force and applyDeletions to delete it'; // What the effect may find: what this observation saw, unless forced. const expect: Expect = opts.force || d.kind === 'unobserved' || d.kind === 'unhashed' ? { kind: 'any' } @@ -1033,7 +1078,7 @@ export async function pushPaths( result.skipped.push({ path, reason: 'stale copy: disk changed since it last agreed with the workspace, which did not — ' + - 'sync this path to adopt the disk version, or pass force to overwrite it', + `sync this path to adopt the disk version, or ${forced}`, }); continue; } @@ -1065,7 +1110,22 @@ export async function pushPaths( } if (v.state === 'unverified') { if (!opts.force) { - result.skipped.push({ path, reason: `cannot verify disk copy (${v.note ?? 'not observed'}) — fix it and materialize again, or pass force to overwrite it` }); + // An intent that can't be read is the one note fixing doesn't + // answer: nothing is decided over it until a sync of the path + // replaces it (decide()), and a sync replaces it only where it sees + // disk's copy, so a copy it can't see is named first. For the rest, + // fixing what the note names holds, and forcedWay says where force + // does too. + const sync = 'sync this path to adopt the disk version, which replaces the intent'; + result.skipped.push({ + path, + reason: bi?.unreadable !== undefined + ? d.kind === 'absent' || d.kind === 'file' + ? `${v.note} — ${sync}` + : `${v.note}, and disk's copy can't be verified (${d.kind === 'unobserved' ? d.reason : 'not hashed'}) — once it can be, ${sync}` + : inTheWay(v.note, !s) ?? + `cannot verify disk copy (${v.note ?? 'not observed'}) — fix it and ${again}${forcedWay(v.note, !s, forced)}`, + }); continue; } } @@ -1086,13 +1146,13 @@ export async function pushPaths( path, reason: noEvidence ? `disk holds a different copy, and nothing records which is newer (conflict: ${kind}) — ` + - 'sync this path to adopt the disk version, or materialize this path with force to overwrite it' + `sync this path to adopt the disk version, or materialize this path with force ${s ? 'to overwrite it' : 'and applyDeletions to delete it'}` : kind === 'interrupted' ? 'disk holds bytes that match neither what was being written there nor what disk last agreed with ' + '(conflict: interrupted), as a write that failed or was cut off partway leaves — check the file, then ' + - 'pass force to write the workspace copy over it, or sync this path to adopt the disk copy as it is' + `${forced}, or sync this path to adopt the disk copy as it is` : `stale copy: disk changed since it last agreed with the workspace (conflict: ${kind}) — ` + - 'sync this path to adopt the disk version, or pass force to overwrite it', + `sync this path to adopt the disk version, or ${forced}`, }); continue; } @@ -1167,7 +1227,8 @@ export async function pushPaths( // exactly what it was, whatever kind that was. agreement.restore(mount.name, step.path, step.before); recorded = true; - result.skipped.push({ path: step.path, reason: `not ${step.kind === 'write' ? 'written' : 'unlinked'}: ${failure.message}` }); + const why = inTheWay(failure.message, step.kind === 'unlink') ?? failure.message; + result.skipped.push({ path: step.path, reason: `not ${step.kind === 'write' ? 'written' : 'unlinked'}: ${why}` }); } else { // No completion: the intent stays pending, and the next observation // resolves it from whatever disk then holds. diff --git a/test/workspace-file-tools.test.ts b/test/workspace-file-tools.test.ts new file mode 100644 index 000000000..381e6a103 --- /dev/null +++ b/test/workspace-file-tools.test.ts @@ -0,0 +1,632 @@ +// The workspace's file tools write exactly what they're given or refuse, +// say when a change is stored but not on disk, number a file's lines as it +// has them, and edit the text disk holds now. +import { spawnSync } from 'node:child_process'; +import test, { type TestContext } from 'node:test'; +import assert from 'node:assert/strict'; +import { chmodSync, existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { JsStore } from '@animalabs/chronicle'; +import { WorkspaceModule } from '../src/modules/workspace/index.js'; +import type { ToolResult } from '../src/types/events.js'; + +function setup(t: TestContext, opts: { autoMaterialize?: boolean } = {}) { + const root = mkdtempSync(join(tmpdir(), 'workspace-file-tools-')); + const dir = join(root, 'mount'); + mkdirSync(dir); + const store = JsStore.openOrCreate({ path: join(root, 'store') }); + const workspace = new WorkspaceModule({ + mounts: [{ name: 'work', path: dir, mode: 'read-write', watch: 'never', autoMaterialize: opts.autoMaterialize ?? false }], + }); + workspace.initStore(store); + t.after(() => { store.close(); rmSync(root, { recursive: true, force: true }); }); + const call = (name: string, input: unknown): Promise => + workspace.handleToolCall({ id: name, name, input } as Parameters[0]); + const disk = (rel: string): Buffer | null => (existsSync(join(dir, rel)) ? readFileSync(join(dir, rel)) : null); + const stored = async (rel: string): Promise => { + const result = await call('read', { path: `work/${rel}`, offsetChars: 0, limitChars: 100_000 }); + assert.equal(result.success, true, result.error); + return (result.data as { content: string }).content; + }; + return { workspace, store, dir, call, disk, stored }; +} + +const errorOf = (result: ToolResult): string => { + assert.equal(result.success, false, 'the call was refused'); + assert.equal(result.isError, true); + return result.error ?? ''; +}; + +// --------------------------------------------------------------------------- +// Part 1: exactly what was given, or a refusal +// --------------------------------------------------------------------------- + +test('an edit writes its replacement as given, $ patterns and all', async (t) => { + const { dir, call, disk } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'price.txt'), 'price: 10\n'); + const one = await call('edit', { path: 'work/price.txt', oldString: '10', newString: "$$5, $&, $', $`, $1" }); + assert.equal(one.success, true, one.error); + assert.equal(disk('price.txt')!.toString(), "price: $$5, $&, $', $`, $1\n"); + + writeFileSync(join(dir, 'pair.txt'), 'a-a\n'); + const all = await call('edit', { path: 'work/pair.txt', oldString: 'a', newString: '$&$&', replaceAll: true }); + assert.equal(all.success, true, all.error); + assert.equal(disk('pair.txt')!.toString(), '$&$&-$&$&\n'); +}); + +test("an edit of a file that isn't valid UTF-8 is refused and changes nothing, and a write still replaces it", async (t) => { + const { dir, call, disk } = setup(t, { autoMaterialize: true }); + const latin1 = Buffer.from('caf\xe9 au lait\n', 'latin1'); + writeFileSync(join(dir, 'latin1.txt'), latin1); + const refused = errorOf(await call('edit', { path: 'work/latin1.txt', oldString: 'au', newString: 'with' })); + assert.equal(refused, "The edit did nothing: work/latin1.txt isn't valid UTF-8, so editing it would change bytes outside the text it replaces. " + + 'Read shows each invalid byte sequence as U+FFFD, and a write replaces the whole file as UTF-8, so writing back what read shows puts U+FFFD in their place.'); + assert.deepEqual(disk('latin1.txt'), latin1, 'disk keeps every byte'); + + const written = await call('write', { path: 'work/latin1.txt', content: 'café with milk\n' }); + assert.equal(written.success, true, written.error); + assert.equal(disk('latin1.txt')!.toString('utf8'), 'café with milk\n'); +}); + +test('a lone surrogate is refused in a write and in either of an edit\'s strings, and nothing changes', async (t) => { + const { dir, call, disk } = setup(t, { autoMaterialize: true }); + assert.match(errorOf(await call('write', { path: 'work/s.txt', content: 'a\ud800b' })), + /^The write did nothing: "content" holds a lone UTF-16 surrogate/); + assert.equal(disk('s.txt'), null); + assert.match(errorOf(await call('write', { path: 'work/s\ud800.txt', content: 'x' })), + /^The path "work\/s\\ud800\.txt" holds a lone UTF-16 surrogate, which UTF-8 can't store, so no file on disk can have that name\. Nothing was done\.$/); + + writeFileSync(join(dir, 'e.txt'), 'pair \ud83d\ude00 here\n'); + assert.match(errorOf(await call('edit', { path: 'work/e.txt', oldString: 'here', newString: 'x\udc00' })), + /^The edit did nothing: "newString" holds a lone UTF-16 surrogate/); + assert.match(errorOf(await call('edit', { path: 'work/e.txt', oldString: '\ud83d', newString: 'x' })), + /^The edit did nothing: "oldString" holds a lone UTF-16 surrogate/); + assert.equal(disk('e.txt')!.toString(), 'pair \ud83d\ude00 here\n'); + // A pair is one character, not a lone half. + const paired = await call('edit', { path: 'work/e.txt', oldString: 'here', newString: '\ud83c\udf89' }); + assert.equal(paired.success, true, paired.error); +}); + +test('an empty oldString is refused, with replaceAll or without', async (t) => { + const { dir, call, disk } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'e.txt'), 'abc\n'); + for (const replaceAll of [false, true]) { + assert.match(errorOf(await call('edit', { path: 'work/e.txt', oldString: '', newString: 'X', replaceAll })), + /^The edit did nothing: "oldString" is empty/); + } + assert.equal(disk('e.txt')!.toString(), 'abc\n'); +}); + +test("a call whose input doesn't fit its tool's schema does nothing", async (t) => { + const { dir, call, disk } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'u.txt'), 'keep a\n'); + const cases: Array<[string, unknown, RegExp]> = [ + ['edit', { path: 'work/u.txt', oldString: 'a' }, /^The edit call did nothing: "newString" is required\.$/], + ['edit', { path: 'work/u.txt', oldString: 'a', newString: 5 }, /"newString" must be a string, not 5\.$/], + ['edit', { path: 'work/u.txt', oldString: 'a', newString: null }, /"newString" must be a string, not null\.$/], + ['edit', { path: 'work/u.txt', oldString: 'a', newString: 'b', replaceAll: 'false' }, /"replaceAll" must be true or false, not a string\.$/], + ['edit', { path: 'work/u.txt', oldString: 'a', newString: 'b', replaceAll: null }, /"replaceAll" must be true or false, not null\.$/], + ['write', { path: 'work/u.txt', content: ['x'] }, /^The write call did nothing: "content" must be a string, not an array\.$/], + ['write', { path: 3, content: 'x' }, /"path" must be a string, not 3\.$/], + ['read', { path: 'work/u.txt', offset: '1' }, /"offset" must be a whole number of at least 1, not a string\.$/], + ['grep', { pattern: 'a', contextBefore: -1 }, /"contextBefore" must be a whole number of at least 0, not -1\.$/], + ['delete', null, /^The delete call did nothing: its input must be an object/], + ]; + for (const [name, input, expected] of cases) { + assert.match(errorOf(await call(name, input)), expected, `${name} ${JSON.stringify(input)}`); + } + assert.equal(disk('u.txt')!.toString(), 'keep a\n'); +}); + +test('a write, edit or delete that is stored but refused by disk says so, in its text alone', async (t) => { + const { dir, call, disk, stored } = setup(t, { autoMaterialize: true }); + + // A write over a disk change the workspace hasn't seen. + writeFileSync(join(dir, 'w.txt'), 'v1\n'); + assert.equal((await call('read', { path: 'work/w.txt' })).success, true); + writeFileSync(join(dir, 'w.txt'), 'v2 from another writer\n'); + const write = await call('write', { path: 'work/w.txt', content: 'v3 agent\n' }); + assert.match(errorOf(write), /^The write of "work\/w\.txt" is stored in the workspace, so don't send it again\. Its push to disk didn't complete: stale copy: disk changed since it last agreed with the workspace \(conflict: both-changed\) — sync this path to adopt the disk version, or materialize it with force to overwrite it\.$/); + assert.equal(write.data, undefined, "an error's data reaches neither the model nor a script"); + assert.equal(await stored('w.txt'), 'v3 agent\n'); + assert.equal(disk('w.txt')!.toString(), 'v2 from another writer\n'); + + // An edit of a file with another name: the push writes in place, which + // would change the other name's file too. + writeFileSync(join(dir, 'h.txt'), 'shared\n'); + linkSync(join(dir, 'h.txt'), join(dir, 'h2.txt')); + const edit = await call('edit', { path: 'work/h.txt', oldString: 'shared', newString: 'shared, edited' }); + assert.match(errorOf(edit), /^The edit of "work\/h\.txt" is stored in the workspace, so don't send it again: a second edit can apply the change twice\. Its push to disk didn't complete: not written: a file with other hard links/); + assert.equal(edit.data, undefined); + assert.equal(await stored('h.txt'), 'shared, edited\n'); + assert.equal(disk('h2.txt')!.toString(), 'shared\n'); + // Why the answer says not to send it again: the edit's text is still there + // to match, so a second one applies the change twice. + await call('edit', { path: 'work/h.txt', oldString: 'shared', newString: 'shared, edited' }); + assert.equal(await stored('h.txt'), 'shared, edited, edited\n'); + + // A deletion whose disk copy changed since the workspace read it. + writeFileSync(join(dir, 'd.txt'), 'doomed\n'); + assert.equal((await call('read', { path: 'work/d.txt' })).success, true); + writeFileSync(join(dir, 'd.txt'), 'changed on disk\n'); + const del = await call('delete', { path: 'work/d.txt' }); + assert.match(errorOf(del), /^The deletion of "work\/d\.txt" is stored in the workspace, so don't send it again\. Its push to disk didn't complete: stale copy: disk changed since it last agreed with the workspace \(conflict: [a-z-]+\) — sync this path to adopt the disk version, or materialize it with force and applyDeletions to delete it\.$/); + assert.equal(del.data, undefined); + assert.equal(disk('d.txt')!.toString(), 'changed on disk\n'); +}); + +test('a write, edit or delete that throws after its tree entry is set says the change is stored', async (t) => { + const { workspace, dir, call, stored } = setup(t); + const ws = workspace as unknown as { intents: Map unknown }> }; + const intents = ws.intents.get('work')!; + const update = intents.update; + writeFileSync(join(dir, 'e.txt'), 'line\n'); + writeFileSync(join(dir, 'd.txt'), 'doomed\n'); + assert.equal((await call('read', { path: 'work/e.txt' })).success, true); + assert.equal((await call('read', { path: 'work/d.txt' })).success, true); + intents.update = () => { throw new Error('injected intent exception'); }; + try { + const write = await call('write', { path: 'work/w.txt', content: 'new\n' }); + assert.equal(errorOf(write), 'The write of "work/w.txt" is stored in the workspace, so don\'t send it again. The rest of the call failed: injected intent exception. ls shows the path\'s state.'); + assert.equal(write.data, undefined); + const edit = await call('edit', { path: 'work/e.txt', oldString: 'line', newString: 'line, edited' }); + assert.match(errorOf(edit), /^The edit of "work\/e\.txt" is stored in the workspace, so don't send it again: a second edit can apply the change twice\. The rest of the call failed: injected intent exception\. ls shows the path's state\.$/); + assert.equal(edit.data, undefined); + const del = await call('delete', { path: 'work/d.txt' }); + assert.match(errorOf(del), /^The deletion of "work\/d\.txt" is stored in the workspace, so don't send it again\. The rest of the call failed: injected intent exception\. ls shows the path's state\.$/); + assert.equal(del.data, undefined); + } finally { + intents.update = update; + } + assert.equal(await stored('w.txt'), 'new\n'); + assert.equal(await stored('e.txt'), 'line, edited\n'); +}); + +// --------------------------------------------------------------------------- +// Part 2: a read numbers the lines the file has +// --------------------------------------------------------------------------- + +test('a final line end ends the last line, and an empty file reads as empty', async (t) => { + const { dir, call } = setup(t); + writeFileSync(join(dir, 'nl.txt'), 'a\nb\n'); + assert.deepEqual((await call('read', { path: 'work/nl.txt' })).data, { + path: 'work/nl.txt', totalLines: 2, fromLine: 1, toLine: 2, content: ' 1\ta\n 2\tb', + }); + writeFileSync(join(dir, 'blank.txt'), 'a\n\n'); + assert.equal(((await call('read', { path: 'work/blank.txt' })).data as { totalLines: number }).totalLines, 2, + 'an empty line before the final line end is a line'); + writeFileSync(join(dir, 'empty.txt'), ''); + assert.deepEqual((await call('read', { path: 'work/empty.txt' })).data, { path: 'work/empty.txt', totalLines: 0, content: '' }); +}); + +test('grep numbers lines as read does: no match on an empty line after the last', async (t) => { + const { dir, call } = setup(t); + writeFileSync(join(dir, 'g.txt'), 'x\n'); + const result = await call('grep', { pattern: '^$' }); + assert.equal(result.success, true, result.error); + assert.deepEqual((result.data as { results: unknown[] }).results, []); +}); + +test('an offset or limit that is not a whole number of at least 1 is refused', async (t) => { + const { dir, call } = setup(t); + writeFileSync(join(dir, 'nl.txt'), 'a\nb\n'); + for (const input of [{ offset: 0 }, { offset: -1 }, { offset: 1.5 }, { offset: null }, { limit: 0 }, { limit: 2.5 }]) { + assert.match(errorOf(await call('read', { path: 'work/nl.txt', ...input })), + /^The read call did nothing: "(offset|limit)" must be a whole number of at least 1, not /, JSON.stringify(input)); + } +}); + +test('an offset past the last line answers empty, with the line count and no range', async (t) => { + const { dir, call } = setup(t); + writeFileSync(join(dir, 'nl.txt'), 'a\nb\n'); + writeFileSync(join(dir, 'empty.txt'), ''); + assert.deepEqual((await call('read', { path: 'work/empty.txt', offset: 2 })).data, { + path: 'work/empty.txt', totalLines: 0, note: 'Line 2 is past the end: the file has 0 lines.', content: '', + }); + for (const offset of [3, 10]) { + assert.deepEqual((await call('read', { path: 'work/nl.txt', offset })).data, { + path: 'work/nl.txt', totalLines: 2, note: `Line ${offset} is past the end: the file has 2 lines.`, content: '', + }); + } +}); + +// --------------------------------------------------------------------------- +// Part 2: an edit applies to the text disk holds now +// --------------------------------------------------------------------------- + +test("an edit applies to disk's text when disk changed and the store didn't, and says so", async (t) => { + const { dir, call, disk, stored } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 's.txt'), 'v1 line\n'); + assert.equal((await call('read', { path: 'work/s.txt' })).success, true); + writeFileSync(join(dir, 's.txt'), 'v2 line from another writer\n'); + + const edit = await call('edit', { path: 'work/s.txt', oldString: 'line', newString: 'LINE' }); + assert.equal(edit.success, true, edit.error); + assert.match((edit.data as { note: string }).note, /^Disk's copy had changed since the workspace last agreed with it, so it was taken in first, and the edit applied to that text\.$/); + assert.equal(disk('s.txt')!.toString(), 'v2 LINE from another writer\n'); + assert.equal(await stored('s.txt'), 'v2 LINE from another writer\n'); + + // Text only the older copy held isn't there to replace, and the refusal says why. + writeFileSync(join(dir, 's.txt'), 'v3 from another writer\n'); + assert.match(errorOf(await call('edit', { path: 'work/s.txt', oldString: 'LINE', newString: 'x' })), + /^String not found in work\/s\.txt\. Disk's copy had changed since the workspace last agreed with it, so it was taken in first, and the edit looked in that text\.$/); + writeFileSync(join(dir, 's.txt'), 'v4 twice twice\n'); + assert.equal(errorOf(await call('edit', { path: 'work/s.txt', oldString: 'twice', newString: 'x' })), + "String found 2 times in work/s.txt. Use replaceAll: true or provide more context. Disk's copy had changed since the workspace last agreed with it, so it was taken in first, and the edit looked in that text."); +}); + +test('an edit of a draft whose disk copy also changed applies to the draft and names the conflict', async (t) => { + const { dir, call, disk, stored } = setup(t); + writeFileSync(join(dir, 'k.txt'), 'base\n'); + assert.equal((await call('read', { path: 'work/k.txt' })).success, true); + assert.equal((await call('write', { path: 'work/k.txt', content: 'agent draft\n' })).success, true); + writeFileSync(join(dir, 'k.txt'), 'another writer\n'); + + // A miss says so too: the text the agent sought may be disk's. + assert.equal(errorOf(await call('edit', { path: 'work/k.txt', oldString: 'another', newString: 'x' })), + "String not found in work/k.txt. The path is in conflict with disk (both-changed): the edit looked in the workspace's copy. Sync the path to take disk's copy instead, or materialize it with force to write the workspace's over disk's."); + assert.equal(errorOf(await call('edit', { path: 'work/k.txt', oldString: 'a', newString: 'x' })), + "String found 2 times in work/k.txt. Use replaceAll: true or provide more context. The path is in conflict with disk (both-changed): the edit looked in the workspace's copy. Sync the path to take disk's copy instead, or materialize it with force to write the workspace's over disk's."); + const edit = await call('edit', { path: 'work/k.txt', oldString: 'draft', newString: 'draft, edited' }); + assert.equal(edit.success, true, edit.error); + assert.equal((edit.data as { note: string }).note, + "The path is in conflict with disk (both-changed): the edit is in the workspace's copy. Sync the path to take disk's copy instead, or materialize it with force to write the workspace's over disk's."); + assert.equal(await stored('k.txt'), 'agent draft, edited\n'); + assert.equal(disk('k.txt')!.toString(), 'another writer\n'); +}); + +test("an edit refused for a workspace copy that isn't valid UTF-8 names the path's conflict too", async (t) => { + const { store, dir, call } = setup(t); + // An entry from before the evidence, beside a disk copy that differs: + // nothing records which is newer. + const latin1 = Buffer.from('caf\xe9\n', 'latin1'); + store.treeSet('workspace/work/tree', 'c.txt', { blobHash: store.storeBlob(latin1, 'text/plain'), size: latin1.byteLength, mode: 0o644 }); + writeFileSync(join(dir, 'c.txt'), 'disk copy\n'); + assert.match(errorOf(await call('edit', { path: 'work/c.txt', oldString: 'caf', newString: 'tea' })), + /^The edit did nothing: work\/c\.txt isn't valid UTF-8, .* The path is in conflict with disk \(unknown-provenance\): the edit looked in the workspace's copy\. Sync the path to take disk's copy instead, or materialize it with force to write the workspace's over disk's\.$/); +}); + +// --------------------------------------------------------------------------- +// Part 5: smaller texts +// --------------------------------------------------------------------------- + +test("grep's refusal of an invalid pattern gives the parser's reason", async (t) => { + const { dir, call } = setup(t); + writeFileSync(join(dir, 'g.txt'), 'x\n'); + assert.match(errorOf(await call('grep', { pattern: '(' })), /^The grep did nothing: Invalid regular expression: \/\(\/: Unterminated group\.$/); +}); + +test('a refusal raised inside a tool reads without an "Error: " prefix', async (t) => { + const { call } = setup(t); + assert.equal(errorOf(await call('read', { path: 'zzz/a.txt' })), 'Unknown mount: "zzz". Available: work'); +}); + +test("a push that disk's shape refuses names what's in the way and the way out", async (t) => { + const { dir, call } = setup(t, { autoMaterialize: true }); + mkdirSync(join(dir, 'sub')); + assert.match(errorOf(await call('write', { path: 'work/sub', content: 'x\n' })), + /Its push to disk didn't complete: disk has a directory at this path, and no push writes a file over one, forced or not — move or remove that directory on disk and materialize again, or write the file under another path\./); + const forced = await call('materialize', { path: 'work/sub', force: true }); + assert.match(JSON.stringify(forced.data), /not written: disk has a directory at this path, and no push writes a file over one, forced or not/); + + writeFileSync(join(dir, 'f.txt'), 'file\n'); + assert.match(errorOf(await call('write', { path: 'work/f.txt/x.txt', content: 'x\n' })), + /Its push to disk didn't complete: not written: disk has a file or a symlink where this path needs a directory, and no push writes below one, forced or not — move or remove it on disk and materialize again, or write the file under another path\./); +}); + +test('a deletion whose path disk has made a directory is not told to write the file elsewhere', async (t) => { + const { dir, call } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'gone.txt'), 'v1\n'); + assert.equal((await call('read', { path: 'work/gone.txt' })).success, true); + rmSync(join(dir, 'gone.txt')); + mkdirSync(join(dir, 'gone.txt')); + const refused = errorOf(await call('delete', { path: 'work/gone.txt' })); + assert.match(refused, /^The deletion of "work\/gone\.txt" is stored in the workspace/); + assert.doesNotMatch(refused, /write the file under another path/, 'nothing is being written'); + assert.match(refused, /Its push to disk didn't complete: disk has a directory at this path, and no push unlinks one, forced or not — if it should go, move or remove it on disk\.$/); +}); + +// --------------------------------------------------------------------------- +// The fix round of the first review (af-ws-fix) +// --------------------------------------------------------------------------- + +test('an edit that would replace nothing is refused, with replaceAll too', async (t) => { + const { dir, call, disk } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'r.txt'), 'abc\n'); + assert.equal(errorOf(await call('edit', { path: 'work/r.txt', oldString: 'zzz', newString: 'y', replaceAll: true })), 'String not found in work/r.txt.'); + assert.equal(disk('r.txt')!.toString(), 'abc\n'); +}); + +test('an edit of a file disk deleted since the workspace read it says so', async (t) => { + const { dir, call } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'g.txt'), 'here\n'); + assert.equal((await call('read', { path: 'work/g.txt' })).success, true); + rmSync(join(dir, 'g.txt')); + assert.equal(errorOf(await call('edit', { path: 'work/g.txt', oldString: 'here', newString: 'x' })), + "File not found: work/g.txt. Disk's copy was deleted since the workspace last agreed with it, and that was taken in first."); + assert.equal(existsSync(join(dir, 'g.txt')), false, 'the edit put nothing back'); +}); + +test("an edit whose disk copy can't be checked says it used the workspace's copy, stored or not", async (t) => { + for (const autoMaterialize of [false, true]) { + const { dir, call, stored } = setup(t, { autoMaterialize }); + writeFileSync(join(dir, 'u.txt'), 'held\n'); + assert.equal((await call('read', { path: 'work/u.txt' })).success, true); + rmSync(join(dir, 'u.txt')); + mkdirSync(join(dir, 'u.txt')); + const used = "The path couldn't be checked against disk (a directory), so the edit applied to the workspace's copy."; + const edit = await call('edit', { path: 'work/u.txt', oldString: 'held', newString: 'held, edited' }); + if (autoMaterialize) { + // Its push meets the directory, and the stored answer carries the note. + assert.equal(errorOf(edit), 'The edit of "work/u.txt" is stored in the workspace, so don\'t send it again: a second edit can apply the change twice. ' + + "Its push to disk didn't complete: disk has a directory at this path, and no push writes a file over one, forced or not — " + + `move or remove that directory on disk and materialize again, or write the file under another path. ${used}`); + } else { + assert.equal(edit.success, true, edit.error); + assert.equal((edit.data as { note: string }).note, used); + } + assert.equal(await stored('u.txt'), 'held, edited\n'); + assert.equal(errorOf(await call('edit', { path: 'work/u.txt', oldString: 'zzz', newString: 'x' })), + "String not found in work/u.txt. The path couldn't be checked against disk (a directory), so the edit looked in the workspace's copy."); + } +}); + +test('a write, edit or delete that fails before its change is stored answers only the failure', async (t) => { + const { workspace, dir, call, stored } = setup(t); + writeFileSync(join(dir, 'e.txt'), 'line\n'); + assert.equal((await call('read', { path: 'work/e.txt' })).success, true); + // Every write, edit and delete takes the mount's turn before anything else. + const ws = workspace as unknown as { withMount: (...args: unknown[]) => Promise }; + const withMount = ws.withMount; + ws.withMount = async () => { throw new Error('injected before the entry'); }; + try { + const calls: Array<[string, Record]> = [ + ['write', { path: 'work/w.txt', content: 'new\n' }], + ['edit', { path: 'work/e.txt', oldString: 'line', newString: 'x' }], + ['delete', { path: 'work/e.txt' }], + ]; + for (const [name, input] of calls) { + const result = await call(name, input); + assert.equal(errorOf(result), 'injected before the entry', name); + } + } finally { + ws.withMount = withMount; + } + assert.equal(await stored('e.txt'), 'line\n'); + assert.equal((await call('read', { path: 'work/w.txt' })).success, false, 'nothing was stored'); +}); + +test('an edit of a draft whose disk copy also changed, on a mount that pushes, names the conflict its push met', async (t) => { + const { dir, call, disk, stored } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'k.txt'), 'base\n'); + assert.equal((await call('read', { path: 'work/k.txt' })).success, true); + writeFileSync(join(dir, 'k.txt'), 'another writer\n'); + // The write's push is refused, so the workspace keeps it as a draft. + assert.match(errorOf(await call('write', { path: 'work/k.txt', content: 'agent draft\n' })), /^The write of "work\/k\.txt" is stored in the workspace/); + const edit = await call('edit', { path: 'work/k.txt', oldString: 'draft', newString: 'draft, edited' }); + assert.equal(errorOf(edit), 'The edit of "work/k.txt" is stored in the workspace, so don\'t send it again: a second edit can apply the change twice. ' + + "Its push to disk didn't complete: stale copy: disk changed since it last agreed with the workspace (conflict: both-changed) — " + + 'sync this path to adopt the disk version, or materialize it with force to overwrite it.'); + assert.equal(await stored('k.txt'), 'agent draft, edited\n'); + assert.equal(disk('k.txt')!.toString(), 'another writer\n'); + + // An edit that makes the draft agree with disk settles the conflict as its + // push meets it, so its answer names none. + const agreed = await call('edit', { path: 'work/k.txt', oldString: 'agent draft, edited', newString: 'another writer' }); + assert.equal(agreed.success, true, agreed.error); + assert.equal((agreed.data as { note?: string }).note, undefined, 'no conflict remains to name'); + assert.equal(await stored('k.txt'), 'another writer\n'); + assert.equal(disk('k.txt')!.toString(), 'another writer\n'); + const after = await call('materialize', { path: 'work/k.txt' }); + assert.equal(after.success, true, after.error); + assert.doesNotMatch(JSON.stringify(after.data), /conflict/, 'and a materialize meets none'); +}); + +test('an edit that fails after its change is stored, having taken disk\'s change in, says both', async (t) => { + const { workspace, dir, call, stored } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'e.txt'), 'v1 line\n'); + assert.equal((await call('read', { path: 'work/e.txt' })).success, true); + writeFileSync(join(dir, 'e.txt'), 'v2 line from another writer\n'); + const ws = workspace as unknown as { pushUnlocked: (...args: unknown[]) => Promise }; + const push = ws.pushUnlocked; + ws.pushUnlocked = async () => { throw new Error('injected push exception'); }; + try { + const edit = await call('edit', { path: 'work/e.txt', oldString: 'line', newString: 'LINE' }); + assert.equal(errorOf(edit), 'The edit of "work/e.txt" is stored in the workspace, so don\'t send it again: a second edit can apply the change twice. ' + + "The rest of the call failed: injected push exception. ls shows the path's state. " + + "Disk's copy had changed since the workspace last agreed with it, so it was taken in first, and the edit applied to that text."); + } finally { + ws.pushUnlocked = push; + } + assert.equal(await stored('e.txt'), 'v2 LINE from another writer\n'); +}); + +test('a deletion whose disk copy is not a regular file names the materialize that unlinks it', { skip: process.platform === 'win32' ? 'no FIFOs' : false }, async (t) => { + const { dir, call } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'q.txt'), 'v1\n'); + assert.equal((await call('read', { path: 'work/q.txt' })).success, true); + rmSync(join(dir, 'q.txt')); + assert.equal(spawnSync('mkfifo', [join(dir, 'q.txt')]).status, 0, 'mkfifo'); + assert.equal(errorOf(await call('delete', { path: 'work/q.txt' })), 'The deletion of "work/q.txt" is stored in the workspace, so don\'t send it again. ' + + "Its push to disk didn't complete: cannot verify disk copy (not a regular file) — " + + 'fix it and materialize again with applyDeletions, or materialize it with force and applyDeletions to delete it.'); + const forced = await call('materialize', { path: 'work/q.txt', force: true, applyDeletions: true }); + assert.equal(forced.success, true, forced.error); + assert.equal(existsSync(join(dir, 'q.txt')), false, 'the forced materialize unlinked it'); +}); + +// --------------------------------------------------------------------------- +// The fix round of the second review (af-ws-fix-r1) +// --------------------------------------------------------------------------- + +test("an edit of a file disk made binary answers the disk-only text, not a deletion", async (t) => { + const { dir, call } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'b.txt'), 'text\n'); + assert.equal((await call('read', { path: 'work/b.txt' })).success, true); + writeFileSync(join(dir, 'b.txt'), Buffer.from([0, 1, 2, 0, 255, 0])); + assert.equal(errorOf(await call('edit', { path: 'work/b.txt', oldString: 'text', newString: 'x' })), + 'work/b.txt is on disk but not stored in the workspace (binary or over the size limit).'); + assert.deepEqual([...readFileSync(join(dir, 'b.txt'))], [0, 1, 2, 0, 255, 0], 'disk keeps its bytes'); +}); + +test('a deletion of a draft that met a disk file the workspace never saw names applyDeletions', async (t) => { + const { dir, call, disk } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'c.txt'), 'disk text\n'); + // The workspace never read c.txt, so its write is a draft that collides. + assert.match(errorOf(await call('write', { path: 'work/c.txt', content: 'agent text\n' })), /conflict: store-origin-collision/); + assert.equal(errorOf(await call('delete', { path: 'work/c.txt' })), 'The deletion of "work/c.txt" is stored in the workspace, so don\'t send it again. ' + + "Its push to disk didn't complete: disk holds a different copy, and nothing records which is newer (conflict: store-origin-collision) — " + + 'sync this path to adopt the disk version, or materialize this path with force and applyDeletions to delete it.'); + assert.equal(disk('c.txt')!.toString(), 'disk text\n'); +}); + +test("a forced deletion over a directory says no push unlinks one", async (t) => { + const { dir, call } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'gone.txt'), 'v1\n'); + assert.equal((await call('read', { path: 'work/gone.txt' })).success, true); + rmSync(join(dir, 'gone.txt')); + mkdirSync(join(dir, 'gone.txt')); + assert.match(errorOf(await call('delete', { path: 'work/gone.txt' })), /^The deletion of "work\/gone\.txt" is stored in the workspace/); + const forced = await call('materialize', { path: 'work/gone.txt', force: true, applyDeletions: true }); + assert.match(JSON.stringify(forced.data), /not unlinked: disk has a directory at this path, and no push unlinks one, forced or not — if it should go, move or remove it on disk/); + assert.equal(statSync(join(dir, 'gone.txt')).isDirectory(), true, 'the directory stays'); +}); + +test("a write over a FIFO is told to fix it and materialize again, with no force", { skip: process.platform === 'win32' ? 'no FIFOs' : false }, async (t) => { + const { dir, call } = setup(t, { autoMaterialize: true }); + assert.equal(spawnSync('mkfifo', [join(dir, 'p.txt')]).status, 0, 'mkfifo'); + assert.equal(errorOf(await call('write', { path: 'work/p.txt', content: 'x\n' })), 'The write of "work/p.txt" is stored in the workspace, so don\'t send it again. ' + + "Its push to disk didn't complete: cannot verify disk copy (not a regular file) — fix it and materialize again."); + assert.equal(statSync(join(dir, 'p.txt')).isFIFO(), true, 'the FIFO stays'); +}); + +// --------------------------------------------------------------------------- +// The fix round of the third review (af-ws-fix-r2) +// --------------------------------------------------------------------------- + +test("a write over a symlink the mount doesn't follow is told to fix it, with no force", async (t) => { + const { dir, call } = setup(t, { autoMaterialize: true }); + writeFileSync(join(dir, 'target.txt'), 'target\n'); + symlinkSync('target.txt', join(dir, 'l.txt')); + assert.equal(errorOf(await call('write', { path: 'work/l.txt', content: 'x\n' })), 'The write of "work/l.txt" is stored in the workspace, so don\'t send it again. ' + + "Its push to disk didn't complete: cannot verify disk copy (a symlink, which this mount does not follow) — fix it and materialize again."); + assert.equal(lstatSync(join(dir, 'l.txt')).isSymbolicLink(), true, 'the link stays'); + assert.equal(readFileSync(join(dir, 'target.txt'), 'utf8'), 'target\n'); +}); + +test('a disk copy the workspace cannot read is offered force on the condition force needs', async (t) => { + const { dir, call } = setup(t, { autoMaterialize: true }); + for (const name of ['r.txt', 'n.txt', 's.txt']) writeFileSync(join(dir, name), 'v1\n'); + for (const name of ['r.txt', 'n.txt', 's.txt']) assert.equal((await call('read', { path: `work/${name}` })).success, true); + chmodSync(join(dir, 'r.txt'), 0o200); + chmodSync(join(dir, 'n.txt'), 0o000); + chmodSync(join(dir, 's.txt'), 0o200); + t.after(() => { for (const name of ['r.txt', 'n.txt']) if (existsSync(join(dir, name))) chmodSync(join(dir, name), 0o600); }); + try { + readFileSync(join(dir, 'r.txt')); + t.skip('this user can read a mode-0200 file (Windows or root)'); + return; + } catch { /* unreadable, as intended */ } + + const writeRefusal = (path: string): string => `The write of "work/${path}" is stored in the workspace, so don't send it again. ` + + "Its push to disk didn't complete: cannot verify disk copy (cannot open (EACCES)) — fix it and materialize again, " + + 'or, if disk lets the workspace write the file, materialize it with force to overwrite it.'; + assert.equal(errorOf(await call('write', { path: 'work/r.txt', content: 'v2\n' })), writeRefusal('r.txt')); + assert.equal(errorOf(await call('write', { path: 'work/n.txt', content: 'v2\n' })), writeRefusal('n.txt')); + // Force gets past the read where disk lets the file be written, and not + // where it doesn't, which is why the offer carries its condition. + const forced = await call('materialize', { force: true }); + assert.equal(forced.success, true, forced.error); + assert.match(JSON.stringify(forced.data), /n\.txt: not written: [^"]*EACCES/); + chmodSync(join(dir, 'r.txt'), 0o600); + chmodSync(join(dir, 'n.txt'), 0o600); + assert.equal(readFileSync(join(dir, 'r.txt'), 'utf8'), 'v2\n'); + assert.equal(readFileSync(join(dir, 'n.txt'), 'utf8'), 'v1\n'); + + // A deletion's forced unlink needs its directory, not the file's own + // permissions: this one unlinks it. + assert.equal(errorOf(await call('delete', { path: 'work/s.txt' })), 'The deletion of "work/s.txt" is stored in the workspace, so don\'t send it again. ' + + "Its push to disk didn't complete: cannot verify disk copy (cannot open (EACCES)) — fix it and materialize again with applyDeletions, " + + 'or, if disk lets the workspace unlink the file, materialize it with force and applyDeletions to delete it.'); + const unlinked = await call('materialize', { path: 'work/s.txt', force: true, applyDeletions: true }); + assert.equal(unlinked.success, true, unlinked.error); + assert.equal(existsSync(join(dir, 's.txt')), false, 'the forced materialize unlinked it'); +}); + +test("a path whose branch intent can't be read is told to sync the path, which replaces the intent once it sees disk's copy", async (t) => { + const { store, dir, call } = setup(t); + writeFileSync(join(dir, 'u.txt'), 'v1\n'); + assert.equal((await call('read', { path: 'work/u.txt' })).success, true); + const junk = Buffer.from('not an intent'); + store.treeSet('workspace/work/intent', 'u.txt', { blobHash: store.storeBlob(junk, 'text/plain'), size: junk.byteLength, mode: 0o644 }); + const refused = await call('materialize', { path: 'work/u.txt' }); + assert.match(JSON.stringify(refused.data), + /u\.txt: its branch intent can't be read: its record is not a branch intent — sync this path to adopt the disk version, which replaces the intent"/); + const synced = await call('sync', { path: 'work/u.txt' }); + assert.equal(synced.success, true, synced.error); + const again = await call('materialize', { path: 'work/u.txt' }); + assert.equal(again.success, true, again.error); + assert.doesNotMatch(JSON.stringify(again.data), /branch intent/, 'the sync replaced it'); + + // While disk holds what the mount can't see, a sync leaves the intent, so + // the refusal names what disk holds first. + writeFileSync(join(dir, 'w.txt'), 'v1\n'); + assert.equal((await call('read', { path: 'work/w.txt' })).success, true); + store.treeSet('workspace/work/intent', 'w.txt', { blobHash: store.storeBlob(junk, 'text/plain'), size: junk.byteLength, mode: 0o644 }); + rmSync(join(dir, 'w.txt')); + symlinkSync('u.txt', join(dir, 'w.txt')); + const unseen = /w\.txt: its branch intent can't be read: its record is not a branch intent, and disk's copy can't be verified \(a symlink, which this mount does not follow\) — once it can be, sync this path to adopt the disk version, which replaces the intent"/; + assert.match(JSON.stringify((await call('materialize', { path: 'work/w.txt' })).data), unseen); + const blind = await call('sync', { path: 'work/w.txt' }); + assert.equal(blind.success, true, blind.error); + // A sync asks for an adoption, which gets past the intent's check and stops + // at the symlink, before anything is adopted: its note is the symlink's, + // where a pass that adopts nothing answers the intent's. + assert.match(JSON.stringify(blind.data), /"path":"w\.txt","reason":"disk could not be checked: a symlink, which this mount does not follow"/); + assert.match(JSON.stringify((await call('materialize', { path: 'work/w.txt' })).data), unseen, "a sync that can't see disk's copy leaves the intent"); + rmSync(join(dir, 'w.txt')); + // Absent is a copy a sync sees, so the reason names the sync alone. + assert.match(JSON.stringify((await call('materialize', { path: 'work/w.txt' })).data), + /w\.txt: its branch intent can't be read: its record is not a branch intent — sync this path to adopt the disk version, which replaces the intent"/); + const seen = await call('sync', { path: 'work/w.txt' }); + assert.equal(seen.success, true, seen.error); + assert.doesNotMatch(JSON.stringify((await call('materialize', { path: 'work/w.txt' })).data), /branch intent/, 'once it sees disk, the sync replaces it'); +}); + +for (const autoMaterialize of [false, true]) { + test(`an edit of a file disk lacks, with nothing to say whether disk deleted it, says so, ${autoMaterialize ? 'and its push writes it back' : 'and a materialize writes it back'}`, async (t) => { + const { store, call, disk } = setup(t, { autoMaterialize }); + // An entry from before the evidence (as three-way's legacyEntry makes it). + const bytes = Buffer.from('legacy line\n'); + store.treeSet('workspace/work/tree', 'l.txt', { blobHash: store.storeBlob(bytes, 'text/plain'), size: bytes.byteLength, mode: 0o644 }); + const edit = await call('edit', { path: 'work/l.txt', oldString: 'legacy', newString: 'edited' }); + assert.equal(edit.success, true, edit.error); + assert.equal((edit.data as { note?: string }).note, + "Disk has no copy of this file, and nothing records whether it was deleted there, so the edit applied to the workspace's copy. " + + (autoMaterialize ? 'Its push put the file back on disk' : 'A materialize of the path puts it back on disk') + + '; if it was deleted there on purpose, delete it here too.'); + if (autoMaterialize) assert.equal(disk('l.txt')!.toString(), 'edited line\n', 'the push put it back before the answer'); + if (!autoMaterialize) { + assert.equal(disk('l.txt'), null, "a mount that doesn't push leaves disk as it was"); + const materialized = await call('materialize', {}); + assert.equal(materialized.success, true, materialized.error); + } + assert.equal(disk('l.txt')!.toString(), 'edited line\n'); + }); +} + +test("an edit of a file disk lacks whose push is refused doesn't say the file went back", async (t) => { + const { store, dir, call, disk, stored } = setup(t, { autoMaterialize: true }); + // An entry from before the evidence, below a path disk holds as a file, so + // the edit's push can't write it. + const bytes = Buffer.from('legacy line\n'); + store.treeSet('workspace/work/tree', 'f.txt/l.txt', { blobHash: store.storeBlob(bytes, 'text/plain'), size: bytes.byteLength, mode: 0o644 }); + writeFileSync(join(dir, 'f.txt'), 'file\n'); + assert.equal(errorOf(await call('edit', { path: 'work/f.txt/l.txt', oldString: 'legacy', newString: 'edited' })), + 'The edit of "work/f.txt/l.txt" is stored in the workspace, so don\'t send it again: a second edit can apply the change twice. ' + + "Its push to disk didn't complete: not written: disk has a file or a symlink where this path needs a directory, and no push writes below one, forced or not — move or remove it on disk and materialize again, or write the file under another path. " + + "Disk has no copy of this file, and nothing records whether it was deleted there, so the edit applied to the workspace's copy."); + assert.equal(await stored('f.txt/l.txt'), 'edited line\n'); + assert.equal(disk('f.txt')!.toString(), 'file\n', "the refused push left disk's file as it was"); +}); diff --git a/test/workspace-glob-unclosed-brace.test.ts b/test/workspace-glob-unclosed-brace.test.ts index 176d6cae3..84d2ae797 100644 --- a/test/workspace-glob-unclosed-brace.test.ts +++ b/test/workspace-glob-unclosed-brace.test.ts @@ -75,7 +75,7 @@ function assertRefused(name: 'glob' | 'grep', input: Record, pa assert.equal(answer.isError, true); assert.equal( answer.error, - `Error: glob pattern ${JSON.stringify(pattern)} ${problem}. To match a "{" in a name, put "?" in its place.`, + `glob pattern ${JSON.stringify(pattern)} ${problem}. To match a "{" in a name, put "?" in its place.`, ); } diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index 686f3c797..a41f4c216 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -508,7 +508,7 @@ describe('workspace deletes', () => { env.writeDisk('b.txt', 'edited in the shell'); const error = await refused(m, 'delete', { path: 'work/b.txt' }); - assert.match(error, /did not unlink/); + assert.match(error, /^The deletion of "work\/b\.txt" is stored in the workspace, so don't send it again\. Its push to disk didn't complete: /); assert.equal(env.readDisk('b.txt'), 'edited in the shell', 'the shell edit survives'); }); }); @@ -3266,7 +3266,7 @@ describe('the mount boundary', () => { const data = await call(m, 'materialize', {}); const skip = JSON.stringify(data); assert.match(skip, /match neither what was being written there nor what disk last agreed with/); - assert.match(skip, /pass force to write the workspace copy over it/); + assert.match(skip, /check the file, then materialize it with force to overwrite it, or sync this path/); assert.equal(env.readDisk('a.txt'), 'v2, cut off'); await call(m, 'materialize', { force: true }); assert.equal(env.readDisk('a.txt'), 'v2');