From b1dcaded724a52bb5741f4e538272d9b9a0519ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Bene=C5=A1?= Date: Wed, 28 Oct 2026 01:20:42 +0000 Subject: [PATCH] Harness spec: one path form, the file tools' texts, the list tree, skills filesystem.md, spec 3.0.0-draft, and the CLI made to match: - Paths: absolute in every answer except a result's bracket (context.md), which keeps the call's path. glob and grep now give full paths. - A missing file is `No such file or directory: {path}` for every tool that needs it, with no hint after it: in a chat a hidden file must read as a missing one. The CLI's "does not exist. Use write to create it." and "Write it first." are gone; an edit of a directory is `Is a directory`, glob over a file `Not a directory`. - write/edit refusals, exactly: "{path} exists and has not been read in this session. Read it first." and "{path} changed since you last read it. Read it again before changing it." (no "on disk"), absolute path. - A listing sorts names in code-point order; `list` with a depth is a tree indented two spaces a level, marks as in a single listing, cut at 400 lines with a line saying so. - Skills: where they are found and in what order on each side (the session's own place first, the person's next, what is shared last), and the name rule ^[a-z0-9][a-z0-9-]{0,63}$ that skill_manage checks. The CLI no longer makes names with _ or .; such skills still load. README's 3.0.0 notes say so; CHANGELOG [Unreleased]. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 8 ++++ harness/README.md | 13 +++++-- harness/filesystem.md | 67 ++++++++++++++++++++++++++++++--- harness/tools/skill_manage.json | 2 +- src/skill/index.ts | 6 ++- src/tool/edit.ts | 2 +- src/tool/multiedit.ts | 2 +- src/tool/read.ts | 5 ++- src/tool/search.ts | 32 +++++++++++----- src/tool/show.ts | 4 +- src/tool/skills.ts | 8 ++-- src/tool/write.ts | 17 +++++---- tests/hermes-layer.test.ts | 4 ++ tests/results.test.ts | 2 +- tests/tools.test.ts | 36 +++++++++++++++++- 15 files changed, 167 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31e6eec..07dba51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,14 @@ instruction file on the way down to where it was started. Harness spec 3.0.0. - **A chat started on a model runs on that model.** When the web UI opens a session on this device with a model the device cannot run, it is refused with the reason instead of being answered on the device's own model. +- **File tools speak the server's words.** A missing file is `No such file or directory: ` + from `read`, `edit`, `multiedit`, `show`, `list`, `glob` and `grep` alike; a file `write` or + `edit` may not change yet says it has not been read, or changed since it was last read, with its + full path. `glob` and `grep` give full paths. `list` sorts names exactly (capitals first) and says + when it stopped at 400 lines. +- **A new skill's name** is lowercase letters, digits and `-`, up to 64 characters, as the Agent + Skills format has it; `_` and `.` are no longer accepted when `skill_manage` makes one. Skills + that already have such a name still load. ### Added diff --git a/harness/README.md b/harness/README.md index 4392f0a..523d3c1 100644 --- a/harness/README.md +++ b/harness/README.md @@ -95,18 +95,23 @@ this design. - **`filesystem.md`** — the file system as a contract: paths, the kinds `dir`, `file`, `mount`, the levels `none`, `ro`, `stage`, `commit`, grants and floors, the exact error texts, and the rule that a granted path under one that is not is reached only by its full path, its ancestor denied - for reading and writing and never listed. Search results and **indexing by tokens** (at most 480 + for reading and writing and never listed. Paths absolute in every answer but a result's bracket; + one `No such file or directory` for every tool, the `write`/`edit` refusals' texts, the `list` + tree; where skills are found and in what order, and the name rule + (`^[a-z0-9][a-z0-9-]{0,63}$`). Search results and **indexing by tokens** (at most 480 tokens a piece, 64 repeated; a piece the embedder refuses is split, never dropped). **Instruction files stack** from the boundary down to the start directory, outermost first, one per directory (`AGENTS.md`, else `CLAUDE.md`), none from a directory the session cannot read — the CLI took only the nearest one before. `blocks/workspace.md` (LLeMbas) tells a chat's model its view. - **`context.md`** — context hygiene, insert-time only, nothing sent ever rewritten: every result - has an id (`#a3f2`); a read carries its lines and content hash; an unchanged re-read, or a range - inside an earlier unchanged read, is a stub; output over 400 lines or 40,000 characters keeps its + has an id (`#0001`: its number in the session, in hex, at least 4 digits); a read carries its + lines and content hash, under the path as the call gave it; an unchanged re-read, or a range + inside an earlier unchanged read, is a stub — naming only a result in the request being built; output over 400 lines or 40,000 characters keeps its head and tail with `[N lines elided, full output #id]`; `recall` shows any stored result; edits answer with their diff; the same listing or search with the same result is a stub; compaction - adds a table of reads and empties the dedupe table; earlier reasoning is left to the chat + adds a table of the reads it covers (those since the previous compaction, in turns still in the + history) and empties the dedupe table; earlier reasoning is left to the chat template. `loop.json` `context` holds the texts and numbers, `blocks/context.md` tells the model. - **`acp.md`**: **link protocol 3** — LLeMbas 2.0 and LLeMbas CLI 2.0 speak it alone, and a 1.x peer and a 2.0 one refuse each other, each saying what to update (discovery's `protocol` is raised to 3 diff --git a/harness/filesystem.md b/harness/filesystem.md index 793d6bd..78f8c63 100644 --- a/harness/filesystem.md +++ b/harness/filesystem.md @@ -24,7 +24,12 @@ one access level for the whole working directory, decided by the CLI's permissio links: a path is what it says. On a device, a link is followed, and where the CLI's rules look at a path they look at where it really is (an `AGENTS.md` committed as a link to a file outside the project is not read). -- Every answer names a path in its **absolute, resolved** form, whatever the call gave. +- Every answer names a path in its **absolute, resolved** form, whatever the call gave — an + error, a refusal, a search's results, a tool's own message. **The one exception is a result's + bracket** (`context.md`): a read's header, an edit's bracket and the compaction table name the + path as the call gave it, so the model recognises what it wrote. What identifies a file, there + too, is its absolute path. A listing names entries by their names under the directory listed + (*A listing*). - Names are case-sensitive, `/`-separated, and never empty, `.` or `..`. ## Node kinds @@ -120,6 +125,22 @@ resolved absolute path: | `mv`'s target, or a file `mkdir` was asked to make | `Already exists: {path}` | | a tool whose `requires` is not met at the time of the call | the tool's `unavailable` text | +A missing file is `No such file or directory` for **every** tool that needs it to exist — `read`, +`edit`, `show`, `list`, `glob`, `grep`, `python`'s `files` — with no hint after it: in a chat a +hidden file must read exactly as a missing one, and a hint ("use write to create it") would say +which. The tool's description says what it needs. + +`write` and `edit` refuse to change an existing file the session has not seen, or one that changed +since it last saw it (by reading it, or by its own change). The texts, exactly: + +| Case | Text | +|---|---| +| the file exists and this session has not read it | `{path} exists and has not been read in this session. Read it first.` | +| it changed since this session last read or changed it | `{path} changed since you last read it. Read it again before changing it.` | + +How a change is noticed is each side's: the content hash on the server, the modification time on a +device. + A device's own errors (a file too large, a binary file, a protected path) are the CLI's and stay as they are; the ones above it says the same way. @@ -127,7 +148,26 @@ they are; the ones above it says the same way. `list` and a directory given to `read`: one entry per line, directories (and mounts) with a trailing `/`, then — in a chat — a mount marked ` [mount]` and an entry the session can read but not change, -in a directory it can, marked ` [read-only]`. Directories first, then by name. +in a directory it can, marked ` [read-only]`. Directories first, then by name in code-point order +(`B` before `a`). An empty directory is `(empty directory)` from `read`, `(empty)` from `list`. + +`list` with a `depth` (default 2, at most 6) is a tree: the entries of the directory listed, and +under each directory the entries of that directory, each level indented by two more spaces, down to +`depth` levels in all (depth 1 is the directory's own entries). The order and the marks are a single +listing's, at every level; a directory's entries come right after it. At most 400 lines, then +`(more entries — list a directory further down)` on its own line. A device leaves out ignored and +build directories (`.git`, `node_modules`, …, and what `.gitignore` says) and shows only +directories that hold a file it would list. + +``` +docs/ + guide/ + index.md +repo/ [mount] + src/ + README.md [read-only] +notes.md +``` ## A search @@ -194,9 +234,26 @@ section's, then a repository's. (Before 3.0.0 the CLI took only the nearest one. A skill is a directory with a `SKILL.md` (frontmatter `name`, `description`), and `skills_list`, `skill_view` and `skill_manage` are its meaning over files on both sides: the index in the prompt (`prompts/blocks/skills.md`), `skill_view` to load one, `skill_manage` to make or change one, which -checks the frontmatter, the name and the size; `edit` works on the file too. In a chat, skills live -in the tree (`/skills//SKILL.md`, and a skills repository mounted there); on a device they are -local files, as before. Neither side syncs skills on its own. +checks the frontmatter, the name and the size; `edit` works on the file too. Neither side syncs +skills on its own. + +**Where they are found**, in order of precedence — a name found in an earlier place wins, and the +later one is not offered: + +| Side | Places, the most specific first | +|---|---| +| chat | the `skills/` directory of every directory from the start directory up to the boundary, the nearest first, **the root's excluded**; then the person's own `/home//skills/`; then the organisation's `/skills/` (a skills repository mounted there, or a plain directory) | +| device | the project's `.agent/skills/` (a trusted project); then the user's own skills directory; then the directories the configuration adds, in its order | + +So on both sides the session's own place comes first, the person's next, what is shared last. Only +what the session can read is a skill there; `skill_manage` makes a new one in the person's own +place. + +**A skill's name**, which `skill_manage` checks when it makes or renames one: lowercase letters, +digits and `-`, starting with a letter or a digit, at most 64 characters — +`^[a-z0-9][a-z0-9-]{0,63}$`, the Agent Skills format's rule. A `SKILL.md` made elsewhere whose name +does not fit is still offered when its name is one line of at most 64 characters; a name is looked +up ignoring case, with `_` and spaces read as `-`. ## Memory diff --git a/harness/tools/skill_manage.json b/harness/tools/skill_manage.json index 4bf9cee..8d20d51 100644 --- a/harness/tools/skill_manage.json +++ b/harness/tools/skill_manage.json @@ -43,7 +43,7 @@ }, "name": { "type": "string", - "description": "The skill (lowercase letters, digits, - _ .; up to 64)" + "description": "The skill (lowercase letters, digits and -, starting with a letter or digit; up to 64)" }, "description": { "description": "create: when to use the skill, one sentence, the trigger first: \"Use when …\"", diff --git a/src/skill/index.ts b/src/skill/index.ts index 05a5bb2..0d4c482 100644 --- a/src/skill/index.ts +++ b/src/skill/index.ts @@ -25,7 +25,11 @@ export interface Skill { export const SUPPORT_DIRS = ["references", "templates", "scripts", "assets"] as const const SKIP = new Set([".git", ".github", ".archive", ".venv", "venv", "node_modules", "site-packages", "__pycache__", ...SUPPORT_DIRS]) -export const NAME = /^[a-z0-9][a-z0-9._-]*$/ +/** A name skill_manage makes (filesystem.md, *Skills*; the Agent Skills format's rule): lowercase + * letters, digits and -, starting with a letter or a digit, at most MAX_NAME. */ +export const NAME = /^[a-z0-9][a-z0-9-]*$/ +/** A category directory's name. */ +export const CATEGORY = /^[a-z0-9][a-z0-9._-]*$/ export const MAX_NAME = 64 export const MAX_DESCRIPTION = 1024 export const INDEX_DESCRIPTION = 60 diff --git a/src/tool/edit.ts b/src/tool/edit.ts index 4b51daf..424dc9e 100644 --- a/src/tool/edit.ts +++ b/src/tool/edit.ts @@ -10,7 +10,7 @@ import { hunks, sha256 } from "../session/results.ts" async function planEdit(a: { path: string; old: string; new: string; all?: boolean }, ctx: ToolContext) { const abs = absPath(a.path, ctx) const display = showPath(abs, ctx) - if (!(await guardExisting(abs, display, ctx))) throw new ToolError(`${display} does not exist. Use write to create it.`) + if (!(await guardExisting(abs, ctx))) throw new ToolError(`No such file or directory: ${abs}`) const raw = await readText(abs) const crlf = raw.includes("\r\n") const before = crlf ? raw.replaceAll("\r\n", "\n") : raw diff --git a/src/tool/multiedit.ts b/src/tool/multiedit.ts index e3df754..12616f8 100644 --- a/src/tool/multiedit.ts +++ b/src/tool/multiedit.ts @@ -10,7 +10,7 @@ import { hunks, sha256 } from "../session/results.ts" async function planEdits(a: { path: string; edits: { old: string; new: string; all?: boolean }[] }, ctx: ToolContext) { const abs = absPath(a.path, ctx) const display = showPath(abs, ctx) - if (!(await guardExisting(abs, display, ctx))) throw new ToolError(`${display} does not exist. Use write to create it.`) + if (!(await guardExisting(abs, ctx))) throw new ToolError(`No such file or directory: ${abs}`) const raw = await readText(abs) const crlf = raw.includes("\r\n") const before = crlf ? raw.replaceAll("\r\n", "\n") : raw diff --git a/src/tool/read.ts b/src/tool/read.ts index fd7b42f..83ee80a 100644 --- a/src/tool/read.ts +++ b/src/tool/read.ts @@ -5,6 +5,7 @@ import { dirname, join } from "node:path" import { z } from "zod" import { absPath, defineTool, showPath, ToolError } from "./tool.ts" import { described } from "../harness.ts" +import { byListing } from "./search.ts" import { sha256 } from "../session/results.ts" const MAX_LINES = 2000 @@ -25,12 +26,12 @@ export const readTool = defineTool({ async run(a, ctx) { const abs = absPath(a.path, ctx) const st = await stat(abs).catch(() => undefined) - if (!st) throw new ToolError(`${a.path} does not exist`) + if (!st) throw new ToolError(`No such file or directory: ${abs}`) if (st.isDirectory()) { const entries = await readdir(abs, { withFileTypes: true }) const lines = entries .map((e) => (e.isDirectory() ? `${e.name}/` : e.name)) - .sort((x, y) => Number(y.endsWith("/")) - Number(x.endsWith("/")) || x.localeCompare(y)) + .sort(byListing) return { output: lines.join("\n") || "(empty directory)", title: `${showPath(abs, ctx)}/ · ${lines.length} entries` } } const file = Bun.file(abs) diff --git a/src/tool/search.ts b/src/tool/search.ts index 4583ca5..93d6c84 100644 --- a/src/tool/search.ts +++ b/src/tool/search.ts @@ -18,8 +18,8 @@ async function listFiles(dir: string, glob: string | undefined, ctx: ToolContext // Spawning in a directory that does not exist fails as "posix_spawn 'rg' ENOENT", which reads // as ripgrep missing. const st = await stat(dir).catch(() => undefined) - if (!st) throw new ToolError(`${showPath(dir, ctx)} does not exist.`) - if (!st.isDirectory()) throw new ToolError(`${showPath(dir, ctx)} is a file, not a directory.`) + if (!st) throw new ToolError(`No such file or directory: ${dir}`) + if (!st.isDirectory()) throw new ToolError(`Not a directory: ${dir}`) if (hasRg) { const args = ["rg", "--files", "--hidden", "--glob", "!.git"] if (glob) args.push("--glob", glob) @@ -51,7 +51,8 @@ export const globTool = defineTool({ const files = await listFiles(dir, a.pattern, ctx) const withTime = await Promise.all(files.map(async (f) => ({ f, t: (await stat(join(dir, f)).catch(() => undefined))?.mtimeMs ?? 0 }))) withTime.sort((x, y) => y.t - x.t) - const shown = withTime.slice(0, 200).map((x) => showPath(join(dir, x.f), ctx)) + // Absolute, as every answer names a path (filesystem.md, *Paths*). + const shown = withTime.slice(0, 200).map((x) => join(dir, x.f)) const more = files.length > 200 ? `\n(${files.length - 200} more — narrow the pattern)` : "" return { output: (shown.join("\n") || "No files found.") + more, title: `${a.pattern} · ${files.length} files`, meta: { hits: files.length } } }, @@ -74,6 +75,7 @@ export const grepTool = defineTool({ async run(a, ctx) { const target = absPath(a.path ?? ".", ctx) const lines: string[] = [] + if (!(await stat(target).catch(() => undefined))) throw new ToolError(`No such file or directory: ${target}`) if (hasRg) { const args = ["rg", "--line-number", "--no-heading", "--color", "never", "--hidden", "--glob", "!.git", "--max-columns", "400", "--max-columns-preview"] if (a.ignore_case) args.push("-i") @@ -82,7 +84,7 @@ export const grepTool = defineTool({ args.push("-e", a.pattern, target) const r = await run(args, ctx.cwd, ctx.signal) if (r.code > 1) throw new ToolError(r.err.trim() || `rg exited ${r.code}`) - for (const l of r.out.split("\n")) if (l) lines.push(l.startsWith(target) ? showPath(target, ctx) + l.slice(target.length) : l) + for (const l of r.out.split("\n")) if (l) lines.push(l) } else { let re: RegExp try { @@ -90,14 +92,13 @@ export const grepTool = defineTool({ } catch (e) { throw new ToolError(`invalid regex: ${(e as Error).message}`) } - const st = await stat(target).catch(() => undefined) - if (!st) throw new ToolError(`${a.path} does not exist`) + const st = await stat(target) const files = st.isDirectory() ? (await listFiles(target, a.include, ctx)).map((f) => join(target, f)) : [target] for (const f of files) { const text = await Bun.file(f).text().catch(() => "") if (text.includes("\0")) continue text.split("\n").forEach((l, i) => { - if (re.test(l)) lines.push(`${showPath(f, ctx)}:${i + 1}:${l.slice(0, 400)}`) + if (re.test(l)) lines.push(`${f}:${i + 1}:${l.slice(0, 400)}`) }) if (lines.length > 1000) break } @@ -109,6 +110,17 @@ export const grepTool = defineTool({ }, }) +/** A listing's order (filesystem.md, *A listing*): directories first, then by name in code-point + * order. */ +export function byListing(x: string, y: string): number { + const dirs = Number(y.endsWith("/")) - Number(x.endsWith("/")) + if (dirs) return dirs + const a = [...x].map((c) => c.codePointAt(0)!) + const b = [...y].map((c) => c.codePointAt(0)!) + for (let i = 0; i < Math.min(a.length, b.length); i++) if (a[i] !== b[i]) return a[i]! - b[i]! + return a.length - b.length +} + export const listTool = defineTool({ ...described("list"), access: "list", @@ -136,7 +148,7 @@ export const listTool = defineTool({ } const out: string[] = [] const walk = (parent: string, indent: string) => { - const kids = [...(tree.get(parent) ?? [])].sort((x, y) => Number(y.endsWith("/")) - Number(x.endsWith("/")) || x.localeCompare(y)) + const kids = [...(tree.get(parent) ?? [])].sort(byListing) for (const k of kids) { if (out.length >= 400) return out.push(indent + k) @@ -144,6 +156,8 @@ export const listTool = defineTool({ } } walk("", "") - return { output: out.join("\n") || "(empty)", title: `${relative(ctx.root, dir) || "."}/ · ${files.length} files`, meta: { hits: files.length } } + // Cut at 400 lines, saying so (filesystem.md, *A listing*). + const more = out.length >= 400 ? "\n(more entries — list a directory further down)" : "" + return { output: (out.join("\n") || "(empty)") + more, title: `${relative(ctx.root, dir) || "."}/ · ${files.length} files`, meta: { hits: files.length } } }, }) diff --git a/src/tool/show.ts b/src/tool/show.ts index 7807d68..b260e12 100644 --- a/src/tool/show.ts +++ b/src/tool/show.ts @@ -59,11 +59,11 @@ export const showTool = defineTool({ let size: number try { const st = statSync(abs) - if (!st.isFile()) throw new ToolError(`${showPath(abs, ctx)} is not a file.`) + if (!st.isFile()) throw new ToolError(st.isDirectory() ? `Is a directory: ${abs}` : `${showPath(abs, ctx)} is not a file.`) size = st.size } catch (e) { if (e instanceof ToolError) throw e - throw new ToolError(`${showPath(abs, ctx)} does not exist. Write it first.`) + throw new ToolError(`No such file or directory: ${abs}`) } if (size > SHOW_MAX_BYTES) throw new ToolError(`${showPath(abs, ctx)} is ${Math.round(size / 1024)} KB; at most 1 MB can be shown. Give the user the path instead.`) const text = await readText(abs) diff --git a/src/tool/skills.ts b/src/tool/skills.ts index 9dedb46..575bb52 100644 --- a/src/tool/skills.ts +++ b/src/tool/skills.ts @@ -5,7 +5,7 @@ import { basename, dirname, join, relative } from "node:path" import { paths } from "../config/paths.ts" import { z } from "zod" import { threatMessage } from "../memory/threats.ts" -import { within, findSkill, globalSkillsDir, INDEX_DESCRIPTION, linkedFiles, loadSkills, MAX_DESCRIPTION, MAX_FILE, MAX_NAME, NAME, type Skill, skillFile, splitFrontmatter, SUPPORT_DIRS } from "../skill/index.ts" +import { CATEGORY, within, findSkill, globalSkillsDir, INDEX_DESCRIPTION, linkedFiles, loadSkills, MAX_DESCRIPTION, MAX_FILE, MAX_NAME, NAME, type Skill, skillFile, splitFrontmatter, SUPPORT_DIRS } from "../skill/index.ts" import { replace } from "./replace-text.ts" import { defineTool, ToolError, type ToolContext } from "./tool.ts" import { described } from "../harness.ts" @@ -144,7 +144,7 @@ export const skillManageTool = defineTool({ const ops = a.operations as Op[] if (ops.some((o) => o.action === "delete" || o.action === "revert") && ops.length > 1) throw new ToolError(`${ops.find((o) => o.action === "delete" || o.action === "revert")!.action} must be the only operation in its call.`) // Every name is checked before anything else: it becomes a directory name below. - for (const o of ops) if (!NAME.test(o.name) || o.name.length > MAX_NAME) throw new ToolError(`"${o.name}" is not a valid skill name: lowercase letters, digits, - _ . — starting with a letter or digit, up to ${MAX_NAME}.`) + for (const o of ops) if (!NAME.test(o.name) || o.name.length > MAX_NAME) throw new ToolError(`"${o.name}" is not a valid skill name: lowercase letters, digits and -, starting with a letter or digit, up to ${MAX_NAME}.`) const skills = skillsOf(ctx) if (ops[0]!.action === "revert") { const name = ops[0]!.name @@ -197,11 +197,11 @@ export const skillManageTool = defineTool({ const fail = (m: string): never => { throw new ToolError(at + m) } - if (!NAME.test(o.name) || o.name.length > MAX_NAME) fail(`"${o.name}" is not a valid skill name: lowercase letters, digits, - _ . — starting with a letter or digit, up to ${MAX_NAME}.`) + if (!NAME.test(o.name) || o.name.length > MAX_NAME) fail(`"${o.name}" is not a valid skill name: lowercase letters, digits and -, starting with a letter or digit, up to ${MAX_NAME}.`) const existing = created.get(o.name) ?? findSkill(skills, o.name) if (o.action === "create") { if (existing) fail(`a skill "${o.name}" already exists (${existing.dir}). Use patch.`) - if (o.category && (!NAME.test(o.category) || o.category.includes("/"))) fail(`"${o.category}" is not a valid category.`) + if (o.category && (!CATEGORY.test(o.category) || o.category.includes("/"))) fail(`"${o.category}" is not a valid category.`) const text = o.content ?? (o.description?.trim() && o.instructions?.trim() ? composeSkill(o.name, o.description, o.instructions) : fail("create needs description and instructions (or content: the whole SKILL.md).")) const bad = validateSkill(text, o.name) ?? threatMessage(text, "The skill") if (bad) fail(bad) diff --git a/src/tool/write.ts b/src/tool/write.ts index 08fcfde..3691ec6 100644 --- a/src/tool/write.ts +++ b/src/tool/write.ts @@ -7,15 +7,16 @@ import { absPath, defineTool, showPath, ToolError, type ToolContext } from "./to import { described } from "../harness.ts" import { hunks, lineCount, sha256 } from "../session/results.ts" -/** Refuse to overwrite a file the model has not read, or one changed on disk since it was read. */ -export async function guardExisting(abs: string, display: string, ctx: ToolContext): Promise { +/** Refuse to overwrite a file the model has not read, or one changed on disk since it was read. The + * texts are the spec's (filesystem.md, *Errors*), with the absolute path. */ +export async function guardExisting(abs: string, ctx: ToolContext): Promise { const st = await stat(abs).catch(() => undefined) if (!st) return false - if (!st.isFile()) throw new ToolError(`${display} is not a regular file`) - if (!ctx.readFiles.has(abs)) throw new ToolError(`${display} exists and has not been read in this session. Read it first.`) + if (st.isDirectory()) throw new ToolError(`Is a directory: ${abs}`) + if (!st.isFile()) throw new ToolError(`${abs} is not a regular file`) + if (!ctx.readFiles.has(abs)) throw new ToolError(`${abs} exists and has not been read in this session. Read it first.`) const seen = ctx.fileStamps.get(abs) - if (seen !== undefined && st.mtimeMs > seen + 1) - throw new ToolError(`${display} changed on disk since you last read it. Read it again before changing it.`) + if (seen !== undefined && st.mtimeMs > seen + 1) throw new ToolError(`${abs} changed since you last read it. Read it again before changing it.`) return true } @@ -43,13 +44,13 @@ export const writeTool = defineTool({ async preview(a, ctx) { const abs = absPath(a.path, ctx) const display = showPath(abs, ctx) - const before = (await guardExisting(abs, display, ctx)) ? await Bun.file(abs).text() : "" + const before = (await guardExisting(abs, ctx)) ? await Bun.file(abs).text() : "" return createTwoFilesPatch(display, display, before, a.content, "", "", { context: 3 }) }, async run(a, ctx) { const abs = absPath(a.path, ctx) const display = showPath(abs, ctx) - const existed = await guardExisting(abs, display, ctx) + const existed = await guardExisting(abs, ctx) const before = existed ? await Bun.file(abs).text() : "" await mkdir(dirname(abs), { recursive: true }) await Bun.write(abs, a.content) diff --git a/tests/hermes-layer.test.ts b/tests/hermes-layer.test.ts index 64a1cd3..498e73f 100644 --- a/tests/hermes-layer.test.ts +++ b/tests/hermes-layer.test.ts @@ -45,6 +45,10 @@ describe("skills", () => { rmSync(join(paths.config, "skills"), { recursive: true, force: true }) const c = ctx() await expect(skillManageTool.run({ operations: [{ action: "create", name: "Bad Name", content: "x" }] }, c)).rejects.toThrow("not a valid skill name") + // The spec's rule (filesystem.md, *Skills*): letters, digits and - only, up to 64. + await expect(skillManageTool.run({ operations: [{ action: "create", name: "snake_case", content: "x" }] }, c)).rejects.toThrow("not a valid skill name") + await expect(skillManageTool.run({ operations: [{ action: "create", name: "v1.2", content: "x" }] }, c)).rejects.toThrow("not a valid skill name") + await expect(skillManageTool.run({ operations: [{ action: "create", name: "a".repeat(65), content: "x" }] }, c)).rejects.toThrow("not a valid skill name") await expect(skillManageTool.run({ operations: [{ action: "create", name: "a", content: skill("b", "wrong name") }] }, c)).rejects.toThrow('must be "a"') const long = "Use when the build breaks on the CI runner and nobody knows why it happens." const r = await skillManageTool.run({ operations: [{ action: "create", name: "ci", category: "devops", content: skill("ci", long) }, { action: "write_file", name: "ci", file_path: "references/runner.md", file_content: "runner notes" }] }, c) diff --git a/tests/results.test.ts b/tests/results.test.ts index 9f589bf..fcaed0a 100644 --- a/tests/results.test.ts +++ b/tests/results.test.ts @@ -45,7 +45,7 @@ test("an unchanged re-read is a stub; after an edit the file is read in full; a expect(r[0]).toMatch(/^\[#0001 a\.txt lines 1-3 of 3 sha ([0-9a-f]{8})\]\n1: one\n2: two\n3: three$/) const sha = /sha ([0-9a-f]{8})/.exec(r[0]!)![1] expect(r[1]).toBe(`[#0002 a.txt lines 2-2 of 3 sha ${sha}] Unchanged since #0001, which already shows lines 1-3.`) - expect(r[2]).toBe("[#0003]\na.txt") + expect(r[2]).toBe(`[#0003]\n${join(dir, "a.txt")}`) expect(r[3]).toBe("[#0004] Same result as #0003.") expect(r[4]).toMatch(/^\[#0005 a\.txt sha [0-9a-f]{8}\]\n@@ -1,3 \+1,3 @@\n one\n-two\n\+2\n three$/) expect(r[5]).toMatch(/^\[#0006 a\.txt lines 1-3 of 3 sha [0-9a-f]{8}\]\n1: one\n2: 2\n3: three$/) diff --git a/tests/tools.test.ts b/tests/tools.test.ts index ca5ed55..a4f09f1 100644 --- a/tests/tools.test.ts +++ b/tests/tools.test.ts @@ -235,7 +235,39 @@ describe("paths the way a model writes them", () => { expect(absPath("~/notes.md", c)).toBe(join(homedir(), "notes.md")) expect(absPath("~", c)).toBe(homedir()) expect(absPath("a/~b", c)).toBe(join(c.root, "a/~b")) - await expect(listTool.run({ path: "nope" }, c)).rejects.toThrow("nope does not exist") - await expect(globTool.run({ pattern: "*", path: "nope" }, c)).rejects.toThrow("nope does not exist") + await expect(listTool.run({ path: "nope" }, c)).rejects.toThrow(`No such file or directory: ${join(c.cwd, "nope")}`) + await expect(globTool.run({ pattern: "*", path: "nope" }, c)).rejects.toThrow(`No such file or directory: ${join(c.cwd, "nope")}`) + }) +}) + +describe("the file system's texts (harness filesystem.md, *Errors*, *A listing*)", () => { + test("a missing file is the one text for every tool, with the absolute path; write and edit refuse in the spec's words", async () => { + const { listTool, globTool, grepTool } = await import("../src/tool/search.ts") + const { writeTool } = await import("../src/tool/write.ts") + const c = ctx() + const gone = join(c.root, "gone.md") + for (const run of [() => readTool.run({ path: "gone.md" }, c), () => editTool.run({ path: "gone.md", old: "a", new: "b" }, c), () => multieditTool.run({ path: "gone.md", edits: [{ old: "a", new: "b" }] }, c), () => listTool.run({ path: "gone.md" }, c), () => globTool.run({ pattern: "*", path: "gone.md" }, c), () => grepTool.run({ pattern: "x", path: "gone.md" }, c)]) + await expect(run()).rejects.toThrow(`No such file or directory: ${gone}`) + writeFileSync(join(c.root, "a.md"), "one\n") + const a = join(c.root, "a.md") + await expect(editTool.run({ path: "a.md", old: "one", new: "two" }, c)).rejects.toThrow(`${a} exists and has not been read in this session. Read it first.`) + await expect(writeTool.run({ path: "a.md", content: "x" }, c)).rejects.toThrow(`${a} exists and has not been read in this session. Read it first.`) + await readTool.run({ path: "a.md" }, c) + c.fileStamps.set(a, 0) + await expect(editTool.run({ path: "a.md", old: "one", new: "two" }, c)).rejects.toThrow(`${a} changed since you last read it. Read it again before changing it.`) + await expect(editTool.run({ path: ".", old: "one", new: "two" }, c)).rejects.toThrow(`Is a directory: ${c.root}`) + await expect(globTool.run({ pattern: "*", path: "a.md" }, c)).rejects.toThrow(`Not a directory: ${a}`) + }) + + test("a listing: directories first, then names in code-point order; a tree two spaces a level; search results absolute", async () => { + const { mkdirSync } = await import("node:fs") + const { listTool, globTool } = await import("../src/tool/search.ts") + const c = ctx() + mkdirSync(join(c.root, "docs/guide"), { recursive: true }) + for (const f of ["b.md", "B.md", "a.md", "docs/index.md", "docs/guide/x.md"]) writeFileSync(join(c.root, f), "x\n") + expect((await readTool.run({ path: "." }, c)).output).toBe("docs/\nB.md\na.md\nb.md") + expect((await listTool.run({}, c)).output).toBe("docs/\n guide/\n index.md\nB.md\na.md\nb.md") + expect((await listTool.run({ depth: 3 }, c)).output).toBe("docs/\n guide/\n x.md\n index.md\nB.md\na.md\nb.md") + expect((await globTool.run({ pattern: "**/index.md" }, c)).output).toBe(join(c.root, "docs/index.md")) }) })