diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7cc37cc72..74ef9078b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -131,6 +131,7 @@ jobs: key: ${{ runner.os }}-bun-${{ hashFiles('bun.lock') }} restore-keys: ${{ runner.os }}-bun- - run: bun install --frozen-lockfile + - run: bun test packages/spec-graph - run: bun run build:binary - run: bun run smoke:binary diff --git a/packages/spec-graph/SPEC.md b/packages/spec-graph/SPEC.md index dabf05e8c..33bf5e330 100644 --- a/packages/spec-graph/SPEC.md +++ b/packages/spec-graph/SPEC.md @@ -19,7 +19,8 @@ It manages the frontmatter schema the repo's specs use — a file is a spec once `id` and `type`; the schema itself is documented in the skill. Frontmatter is handled with the `yaml` library, link/metadata lists inline. `spec_create` writes new frontmatter in a canonical field order; `spec_update` edits an existing file **in place** — it preserves the file's own field order and any -comments / nested fields, and writes back the original line ending (LF or CRLF). +comments / nested fields, rewrites the frontmatter block alone (in the line ending that block already +used), and leaves every byte of the body untouched. ## Boundary @@ -45,22 +46,24 @@ own boundary and leaves. ## Derived read index The filesystem is the source of truth; the model the tools read is **derived, in-memory, read-only, and -revalidated on demand**. Each read re-globs the spec set (ignoring `node_modules`/`.git`/`dist`/`build`) -and revalidates every file by `(mtimeMs, size)`: unchanged files reuse their cached parse, changed/new -files are re-parsed, vanished files are evicted, and the derived graph is rebuilt only when the spec set -actually changed. So specs added, deleted, or edited from any source — including pi's normal `write`/`edit` -— are always current, while redundant re-parse/rebuild is skipped when nothing moved. One `SpecIndex` is -reused per root (keyed by cwd) so the cache pays off across an agent's calls. The one theoretical miss is -an edit landing within the same mtime tick *and* keeping byte length identical (negligible; a content hash -is the sanctioned escalation). +revalidated on demand**. Each read re-runs the **glob** — a name-sorted traversal that ignores +`node_modules`/`.git`/`dist`/`build` — and revalidates every file by `(mtimeMs, size)`: unchanged files +reuse their cached parse, changed/new files are re-parsed, vanished files are evicted, and the derived +graph is rebuilt only when the spec set actually changed. So specs added, deleted, or edited from any +source — including pi's normal `write`/`edit` — are always current, while redundant re-parse/rebuild is +skipped when nothing moved. One `SpecIndex` is reused per root (keyed by cwd) so the cache pays off +across an agent's calls. The one theoretical miss is an edit landing within the same mtime tick *and* +keeping byte length identical (negligible; a content hash is the sanctioned escalation). ## Tools Read — `spec_grep` (content search, narrowable by metadata), `spec_get` (a node's frontmatter, resolved links, and path — no body), `spec_graph` (a bounded subtree/ancestors/neighbors slice). Manage — -`spec_create`, `spec_update` (frontmatter only), `spec_delete`, `spec_validate`. Per-tool usage lives in -the skill and in each tool's `description`; **none edit prose** — prose is written/edited with pi's normal -`read`/`write`/`edit`. +`spec_create` (which refuses any path the index could never see — see `resolveSpecPath` in +`submodule-spec-graph-core` — and any parameters that would write a file born a non-spec), +`spec_update` (frontmatter only), `spec_delete`, `spec_validate`. Per-tool +usage lives in the skill and in each tool's `description`; **none edit prose** — prose is written/edited +with pi's normal `read`/`write`/`edit`. ## Knowledge delivery diff --git a/packages/spec-graph/core/SPEC.md b/packages/spec-graph/core/SPEC.md index 61f3fcedf..b7ba6ffb7 100644 --- a/packages/spec-graph/core/SPEC.md +++ b/packages/spec-graph/core/SPEC.md @@ -20,7 +20,10 @@ grep with metadata filters, bounded graph slices, and structural validation. Imp - **Owns:** everything above. The filesystem is the source of truth; the model is derived, in-memory, and read-only — the revalidation mechanism lives in `module-spec-graph` (*Derived read index*). - **Public surface:** the `index.ts` **barrel**. `tools/` imports the model only through it, never a leaf - file directly. + file directly. The barrel binds *consumers*; the module's own test (`core/core.test.ts`) may import a + leaf directly to reach helpers that are deliberately not public — the pure path guards, segment + resolver, and glob order comparator in `store.ts`, which pin `resolveSpecPath` and the glob order + without touching a filesystem. - **Allowed deps:** `yaml`; Node built-ins. - **Forbidden:** any `@earendil-works/*` (this is what keeps `core/` isolated and unit-testable) and any `@thinkrail/*` package. @@ -36,14 +39,75 @@ Acyclic and one-way: `parse` is the root, `graph` builds on it, and `query`/`val | `graph.ts` | files → nodes + edges (parent tree, DAG + reverse); duplicate-id tracking | `parse` | | `query.ts` | content grep with metadata filters; bounded graph slices | `parse`, `graph` | | `validate.ts` | dangling links, duplicate ids, parent cycles | `parse`, `graph` | -| `store.ts` | `SpecIndex`: the on-demand fs glob + per-file parse cache + memoized graph (the `core/index` module) | `parse`, `graph`, `query` | +| `store.ts` | `SpecIndex`: the on-demand fs glob + per-file parse cache + memoized graph (the `core/index` module); the indexable-path rule (`resolveSpecPath`) | `parse`, `graph`, `query` | ## Invariants - No `@earendil-works/*` import anywhere under `core/`. - `buildGraph` is pure (same input → same output); the index revalidates each file by `(mtimeMs, size)`, memoizes the graph, and never serves a stale one. -- On a duplicate `id`, the first file seen wins the node slot; the duplicate set is recorded for `validate`. +- The glob filters each directory's entries down to the traversal candidates **first** — directories that + are not ignored, plus `.md` files — and only then normalizes and sorts them, so a directory full of + irrelevant entries costs the per-read scan nothing beyond the pass it discards them in. +- That order is **total**: candidates compare by their **NFC-normalized** name and tie-break on the raw + name by code unit. So the spec order — and therefore which file wins a duplicate `id` — is the same on + every filesystem: not whatever order `readdir` happened to return, not a different answer on a + filesystem that hands back decomposed (NFD) names, and not `readdir`'s order again for two canonically + equivalent spellings (precomposed `é` vs `e\u0301`), which share one NFC key and would otherwise be + left wherever a stable sort found them. + Directories and `.md` files stay in **one** candidate list, so a subdirectory is descended in its place + among its sibling files rather than before or after all of them, and the spec sequence is name-ordered + end to end. The filter-then-sort rewrite **preserves** that, it did not introduce it; the test exists + because two separate lists are the obvious shape to reach for and would silently move the + duplicate-`id` winner. +- On a duplicate `id`, the first file in that order wins the node slot; the duplicate set is recorded for + `validate`. +- `resolveSpecPath` is the single answer to "could the index ever see this path?" and it answers with the + **canonical relative path**, so a caller can never report an identity the index will not produce. + It requires: root-relative, inside the root, `.md`, outside the ignored dirs, an existing root, and no + symlink at any component beneath the root — checked with `lstat` per component, dangling links included, + because the glob never descends a symlink. On Windows, colon-bearing paths are refused before + normalization: this closes both drive-relative paths (`C:..\\outside.md`, which `isAbsolute` does not + recognize) and NTFS alternate data streams (`file:SPEC.md`, which `readdir` cannot see). Every `..` + segment that survives normalization is also refused, and the canonical absolute result must pass a + final `relative(root, target)` containment check. These overlapping gates are deliberate: path syntax, + normalization, and component-wise canonicalization must not be able to undermine each other. Portable + `win32` arithmetic tests pin the escape on every host; the Windows CI lane also runs the package tests + natively, including the `resolveSpecPath` and `spec_create` integrations. The symlink rule is what a + string check and a `realpath` comparison both miss: a link is rejected whether it + leaves the project, lands in an ignored directory, or points back at an indexed one, since in every case + the file it creates is invisible to every other spec tool. +- `resolveSpecPath` canonicalizes each component to its **on-disk spelling** before it judges or reports + it. A component whose bytes already match an entry of its parent directory is taken as written; a + component that resolves on this filesystem *without* matching any entry byte-for-byte — a + case-insensitive or Unicode-folding filesystem — becomes the one parent entry equal to it under + `normalize("NFC").toLowerCase()`. Zero or two such entries is an **error**: it fails closed rather than + guess an identity. Canonicalization stops at the first component that does not resolve, and the rest + keeps the caller's spelling, because on a case-sensitive filesystem a new `Docs/` beside an existing + `docs/` really is a new directory the glob will see. `rel` and `abs` are assembled from those canonical + segments, never from the lexical spelling. A lexical `rel` is what let `NODE_MODULES/SPEC.md` report an + identity the glob would never produce on a case-insensitive filesystem, where the write landed in + `node_modules/`. The `.md` rule is judged twice, on the caller's string and again on the canonical + `rel`, because a leaf that canonicalizes to `SPEC.MD` is a file the byte-exact glob never indexes. + Existence is probed with `lstat`, so a dangling symlink counts as resolving and still reaches the + symlink rejection above. + Canonicalization reads the **parent's** listing, so every parent that exists is listed — not only the + ones whose component exists — and a parent that exists but cannot be listed is an **error**, never an + empty listing. That `readdir` is exactly the call the glob makes there: a directory the resolver cannot + list is one the glob abandons, so a spec written under it would be invisible for the same reason a + `node_modules` one is. +- **The write path over-refuses; the read path stays exact.** `resolveSpecPath` matches `IGNORED_DIRS` + on the `normalize("NFC").toLowerCase()` fold of each component, so it refuses `NODE_MODULES/SPEC.md` + in every spelling; the glob keeps matching the byte-exact `readdir` name. The two mistakes are not the + same size. A resolver that refuses too much answers the caller with a reason it can act on; a glob that + skips too much drops a directory a person really named `Build/` out of the index with no sign at all. + Canonicalization alone did not reach the write path, because it only speaks for a component that + already exists: on a project whose `node_modules` was not installed yet, `NODE_MODULES/SPEC.md` + resolved, created the real `node_modules` on a case-insensitive filesystem, and handed the glob every + dependency under it to index. Folding the glob as well bought nothing against that — an installed + `node_modules` is lowercase on disk and the exact check already skips it — and it cost the `Build/` + case, so the glob stays exact. What the fold does not reach at all: a spec written into `NODE_MODULES/` + by pi's own `write` never passes through `resolveSpecPath`. - `SpecNode.type` stays `string`: the read model indexes whatever is on disk, so it tolerates any `type`; the `SPEC_TYPES` vocabulary constrains only the `spec_create` authoring surface, never the graph. - Finite vocabularies (`SPEC_TYPES`, `SPEC_STATUSES`, `SLICE_DIRECTIONS`, `LINK_KINDS`, `IDENTITY_FIELDS`) @@ -52,6 +116,14 @@ Acyclic and one-way: `parse` is the root, `graph` builds on it, and `query`/`val - Reads coerce frontmatter to a scalar/string-array dialect (lossy — nested maps and comments are dropped), which is fine for the derived model. The write path (`updateFrontmatterText`) is **lossless**: it mutates a live `yaml` Document in place, so untouched fields keep their order and any comments / - nested values survive, and it writes the file back in its original line ending (LF or CRLF). Field - order is **preserved, never re-sorted** — `FIELD_ORDER` is only the order `spec_create` builds *new* - frontmatter in. The `\r`-strip on the fence-interior lines is what makes CRLF-authored files parse. + nested values survive. Field order is **preserved, never re-sorted** — `FIELD_ORDER` is only the order + `spec_create` builds *new* frontmatter in. The `\r`-strip on the fence-interior lines is what makes + CRLF-authored files parse. +- The write path rewrites **only the frontmatter block**: the body is spliced back byte-for-byte, a leading + BOM is put back, and the line ending applied to the rewritten block is read from the frontmatter's own + first break (LF or CRLF). Nothing is inferred from the body — a file whose prose happens to mix endings + keeps every prose byte it had, which is what makes "`spec_update` never edits prose" true of the bytes + and not just of the fields. +- The read paths carry the same obligation the other way: `grepSpecs` splits on `\n`, drops a trailing + `\r` and a leading BOM before matching, so an anchored pattern behaves identically on an LF spec, a + CRLF spec, and a BOM-prefixed one — the BOM would otherwise hide line 1 from every `^` pattern. diff --git a/packages/spec-graph/core/core.test.ts b/packages/spec-graph/core/core.test.ts index 110cb3db3..dad3ba716 100644 --- a/packages/spec-graph/core/core.test.ts +++ b/packages/spec-graph/core/core.test.ts @@ -1,9 +1,19 @@ import { expect, test } from "bun:test"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, win32 } from "node:path"; import { buildGraph, + DEFAULT_GREP_LIMIT, FIELD_ORDER, FIELDS, graphSlice, @@ -15,8 +25,10 @@ import { LIST_LINK_FIELDS, parseFile, REQUIRED_FIELDS, + resolveSpecPath, SINGLE_LINK_FIELDS, SLICE_DIRECTIONS, + SPEC_FILE_EXTENSION, SPEC_STATUSES, SPEC_TYPES, SpecIndex, @@ -24,6 +36,63 @@ import { updateFrontmatterText, validateGraph, } from "./index.ts"; +import { + compareWalkEntries, + hasWindowsNamespaceSyntax, + isPathInsideRoot, + resolvePathSegment, + type SegmentResolution, + toWalkEntry, +} from "./store.ts"; + +function probeFilesystem(): { + foldsCase: boolean; + foldsUnicode: boolean; + deniesListing: boolean; +} { + const probe = mkdtempSync(join(tmpdir(), "spec-probe-")); + try { + for (const name of ["CaseProbe", "caf\u00e9", "cafe\u0301"]) { + mkdirSync(join(probe, name), { recursive: true }); + } + const foldsCase = existsSync(join(probe, "caseprobe")); + const foldsUnicode = readdirSync(probe).length < 3; + return { + foldsCase, + foldsUnicode, + deniesListing: withUnlistableDirectory(probe, refusesListing), + }; + } finally { + rmSync(probe, { recursive: true, force: true }); + } +} + +function withUnlistableDirectory(parent: string, fn: (dir: string) => T): T { + const dir = join(parent, "unlistable"); + mkdirSync(dir, { recursive: true }); + chmodSync(dir, 0o311); + try { + return fn(dir); + } finally { + chmodSync(dir, 0o755); + } +} + +function refusesListing(dir: string): boolean { + try { + readdirSync(dir); + return false; + } catch { + return true; + } +} + +const { foldsCase, foldsUnicode, deniesListing } = probeFilesystem(); +const caseFolding = test.skipIf(!foldsCase); +const spellingPreserving = test.skipIf(foldsUnicode); +const listingDenied = test.skipIf(!deniesListing); +const windowsOnly = test.skipIf(process.platform !== "win32"); +const nonWindows = test.skipIf(process.platform === "win32"); test("the finite-vocabulary tuples carry exactly their members", () => { expect([...IDENTITY_FIELDS]).toEqual(["id", "type"]); @@ -203,6 +272,30 @@ test("updateFrontmatterText writes the file back in its original CRLF line endin expect(/(? { + const body = "# Body\nplain line\r\nanother\n"; + const res = updateFrontmatterText(`---\nid: a\ntype: module-design\ntitle: T\n---\n${body}`, { + set: { title: "T2" }, + }) as { content: string }; + expect(res.content).toBe(`---\nid: a\ntype: module-design\ntitle: T2\n---\n${body}`); +}); + +test("updateFrontmatterText reads the line ending from the frontmatter, not from any body line", () => { + const lfWithOneCrlfInProse = "---\nid: a\ntype: module-design\ntitle: T\n---\nprose\r\n"; + const res = updateFrontmatterText(lfWithOneCrlfInProse, { set: { title: "T2" } }) as { + content: string; + }; + expect(res.content.startsWith("---\nid: a\n")).toBe(true); + expect(res.content.endsWith("prose\r\n")).toBe(true); +}); + +test("updateFrontmatterText keeps a leading BOM the split step strips off", () => { + const file = "\ufeff---\nid: a\ntype: module-design\ntitle: T\n---\nbody\n"; + const res = updateFrontmatterText(file, { set: { title: "T2" } }) as { content: string }; + expect(res.content.startsWith("\ufeff")).toBe(true); + expect(parseFile(res.content).frontmatter?.title).toBe("T2"); +}); + test("updateFrontmatterText rejects set on a list field (use addList/removeList instead)", () => { const res = updateFrontmatterText("---\nid: a\ntype: t\n---\nbody\n", { set: { tags: "a, b" } }); expect("error" in res).toBe(true); @@ -317,6 +410,30 @@ test("grepSpecs marks truncated only when a match exists beyond the limit", () = expect(cut.truncated).toBe(true); }); +test("grepSpecs strips the CR of a CRLF spec so anchored patterns still match", () => { + const entries = [ + { + path: "a.md", + content: "---\r\nid: a\r\ntype: t\r\n---\r\nhello world\r\n", + frontmatter: { id: "a", type: "t" }, + }, + ]; + expect(grepSpecs(entries, { pattern: "world$", regex: true }).matches).toHaveLength(1); + expect(grepSpecs(entries, { pattern: "^hello", regex: true }).matches[0]?.snippet).toBe( + "hello world", + ); +}); + +test("grepSpecs falls back to the default limit rather than reporting a silent truncation", () => { + const content = [{ path: "a.md", content: "x\nx", frontmatter: { id: "a", type: "t" } }]; + for (const limit of [0, -5, 0.5]) { + const res = grepSpecs(content, { pattern: "x", limit }); + expect(res.matches).toHaveLength(2); + expect(res.truncated).toBe(false); + } + expect(DEFAULT_GREP_LIMIT).toBe(200); +}); + test("validateGraph flags dangling links, duplicate ids, and parent cycles", () => { const g = buildGraph([ { path: "a.md", frontmatter: { id: "a", type: "t", parent: "b", "depends-on": ["ghost"] } }, @@ -439,3 +556,287 @@ test("SpecIndex tracks a file entering and leaving spec-hood via its frontmatter expect([...index.graph().nodes.keys()]).toEqual([]); }); }); + +function withProject(fn: (root: string, outer: string) => void): void { + withIndexRoot((outer) => { + const root = join(outer, "project"); + mkdirSync(root, { recursive: true }); + fn(root, outer); + }); +} + +const ok = (r: ReturnType, root: string): string => { + if ("error" in r) throw new Error(`expected a resolved path, got: ${r.error}`); + expect(r.abs).toBe(join(root, ...r.rel.split("/"))); + return r.rel; +}; + +test("resolveSpecPath returns the canonical relative path the index would report", () => { + withProject((root) => { + expect(ok(resolveSpecPath(root, "SPEC.md"), root)).toBe("SPEC.md"); + expect(ok(resolveSpecPath(root, "packages/core/SPEC.md"), root)).toBe("packages/core/SPEC.md"); + expect(ok(resolveSpecPath(root, "./packages/core/SPEC.md"), root)).toBe( + "packages/core/SPEC.md", + ); + expect(ok(resolveSpecPath(root, "pkg/./sub/../SPEC.md"), root)).toBe("pkg/SPEC.md"); + expect(SPEC_FILE_EXTENSION).toBe(".md"); + }); +}); + +test("resolveSpecPath rejects every path the index could never see", () => { + withProject((root) => { + for (const path of [ + "", + "../outside.md", + "pkg/../../outside.md", + "/etc/outside.md", + "notes/spec.txt", + "node_modules/dep/SPEC.md", + "pkg/dist/SPEC.md", + ]) { + expect(resolveSpecPath(root, path)).toHaveProperty("error"); + } + }); +}); + +test("Windows drive and stream syntax is never a root-relative spec path", () => { + for (const path of [ + "C:SPEC.md", + "C:..\\..\\outside.md", + "notes.txt:SPEC.md", + "dir\\notes.txt:SPEC.md", + "\\rooted\\SPEC.md", + "\\\\server\\share\\SPEC.md", + ]) { + expect(hasWindowsNamespaceSyntax(path)).toBe(true); + } + for (const path of ["SPEC.md", "dir\\SPEC.md"]) { + expect(hasWindowsNamespaceSyntax(path)).toBe(false); + } +}); + +test("the final containment gate catches Windows drive-relative traversal after joining", () => { + const root = "C:\\repo"; + const attack = "C:..\\..\\..\\outside.md"; + let target = root; + for (const segment of win32.normalize(attack).split(win32.sep)) { + target = win32.join(target, segment); + } + + expect(win32.isAbsolute(attack)).toBe(false); + expect(target).toBe("C:\\outside.md"); + expect(isPathInsideRoot(root, "C:\\repo\\docs\\SPEC.md", win32)).toBe(true); + expect(isPathInsideRoot(root, target, win32)).toBe(false); + expect(isPathInsideRoot(root, "D:\\outside.md", win32)).toBe(false); +}); + +windowsOnly("resolveSpecPath rejects Windows drive-relative and stream paths", () => { + withProject((root) => { + for (const path of ["C:..\\..\\outside.md", "notes.txt:SPEC.md"]) { + expect(resolveSpecPath(root, path)).toHaveProperty("error"); + } + }); +}); + +nonWindows("Windows-only syntax keeps its literal index identity on POSIX", () => { + withProject((root) => { + const names = ["notes.txt:SPEC.md", "dir\\SPEC.md"]; + for (const [index, path] of names.entries()) { + const resolved = resolveSpecPath(root, path); + if ("error" in resolved) throw new Error(`expected a resolved path, got: ${resolved.error}`); + writeFileSync( + resolved.abs, + `---\nid: literal-${index}\ntype: module-design\ntitle: Literal\n---\n`, + ); + } + + const graph = new SpecIndex(root).graph(); + expect(graph.nodes.get("literal-0")?.path).toBe(names[0]); + expect(graph.nodes.get("literal-1")?.path).toBe(names[1]); + }); +}); + +test("resolveSpecPath rejects a symlinked directory even when it points back inside the root", () => { + withProject((root, outer) => { + mkdirSync(join(root, "real"), { recursive: true }); + mkdirSync(join(root, "node_modules", "hidden"), { recursive: true }); + mkdirSync(join(outer, "elsewhere"), { recursive: true }); + symlinkSync(join(outer, "elsewhere"), join(root, "away"), "dir"); + symlinkSync(join(outer, "never-created"), join(root, "gone"), "dir"); + symlinkSync(join(root, "node_modules", "hidden"), join(root, "docs"), "dir"); + symlinkSync(join(root, "real"), join(root, "alias"), "dir"); + + expect(resolveSpecPath(root, "away/evil.md")).toHaveProperty("error"); + expect(resolveSpecPath(root, "gone/evil.md")).toHaveProperty("error"); + expect(resolveSpecPath(root, "docs/ghost.md")).toHaveProperty("error"); + expect(resolveSpecPath(root, "alias/SPEC.md")).toHaveProperty("error"); + expect(ok(resolveSpecPath(root, "real/SPEC.md"), root)).toBe("real/SPEC.md"); + }); +}); + +test("resolveSpecPath rejects a symlink at the leaf, dangling or not", () => { + withProject((root, outer) => { + symlinkSync(join(outer, "never-created.md"), join(root, "dangling.md")); + writeFileSync(join(outer, "real-outside.md"), "outside\n"); + symlinkSync(join(outer, "real-outside.md"), join(root, "live.md")); + + expect(resolveSpecPath(root, "dangling.md")).toHaveProperty("error"); + expect(resolveSpecPath(root, "live.md")).toHaveProperty("error"); + }); +}); + +const resolvedName = (r: SegmentResolution): string => { + if ("error" in r) throw new Error(`expected a resolved segment, got: ${r.error}`); + return r.name; +}; + +test("resolvePathSegment canonicalizes to the on-disk spelling and never guesses", () => { + expect(resolvedName(resolvePathSegment(["docs", "pkg"], "docs", true))).toBe("docs"); + expect(resolvedName(resolvePathSegment(["Docs", "pkg"], "docs", true))).toBe("Docs"); + expect(resolvedName(resolvePathSegment(["caf\u00e9"], "cafe\u0301", true))).toBe("caf\u00e9"); + expect(resolvedName(resolvePathSegment(["docs"], "Docs", false))).toBe("Docs"); + expect(resolvedName(resolvePathSegment(["docs"], "SPEC.md", false))).toBe("SPEC.md"); + + expect(resolvePathSegment(["Docs", "docs"], "DOCS", true)).toHaveProperty("error"); + expect(resolvePathSegment(["pkg"], "docs", true)).toHaveProperty("error"); +}); + +test("resolvePathSegment refuses an ignored directory in any spelling, existing or not", () => { + expect(resolvePathSegment(["node_modules"], "node_modules", true)).toHaveProperty("error"); + expect(resolvePathSegment(["node_modules"], "NODE_MODULES", true)).toHaveProperty("error"); + for (const spelling of [ + "node_modules", + "NODE_MODULES", + "Node_Modules", + "DIST", + "BUILD", + ".GIT", + ]) { + expect(resolvePathSegment([], spelling, false)).toHaveProperty("error"); + } +}); + +caseFolding("resolveSpecPath resolves a case alias to the spelling the index will walk", () => { + withProject((root) => { + mkdirSync(join(root, "node_modules"), { recursive: true }); + mkdirSync(join(root, "docs"), { recursive: true }); + + expect(resolveSpecPath(root, "NODE_MODULES/SPEC.md")).toHaveProperty("error"); + expect(ok(resolveSpecPath(root, "Docs/SPEC.md"), root)).toBe("docs/SPEC.md"); + expect(ok(resolveSpecPath(root, "Docs/Nested/SPEC.md"), root)).toBe("docs/Nested/SPEC.md"); + + writeFileSync( + join(root, "docs", "SPEC.MD"), + "---\nid: shouty\ntype: module-design\ntitle: S\n---\n", + ); + expect(resolveSpecPath(root, "docs/spec.md")).toHaveProperty("error"); + }); +}); + +test("resolveSpecPath fails closed when the root does not exist", () => { + expect(resolveSpecPath(join(tmpdir(), "spec-index-definitely-absent"), "SPEC.md")).toHaveProperty( + "error", + ); +}); + +listingDenied( + "resolveSpecPath fails closed when a parent directory exists but cannot be listed", + () => { + withProject((root) => { + withUnlistableDirectory(root, () => { + expect(resolveSpecPath(root, "unlistable/SPEC.md")).toHaveProperty("error"); + expect(resolveSpecPath(root, "unlistable/nested/SPEC.md")).toHaveProperty("error"); + }); + expect(ok(resolveSpecPath(root, "unlistable/SPEC.md"), root)).toBe("unlistable/SPEC.md"); + }); + }, +); + +test("SpecIndex walks in a stable order, so a duplicate id resolves the same everywhere", () => { + withIndexRoot((root) => { + for (const name of ["z-later", "a-earlier", "m-middle"]) { + mkdirSync(join(root, name), { recursive: true }); + writeFileSync( + join(root, name, "SPEC.md"), + "---\nid: dup\ntype: module-design\ntitle: Dup\n---\n", + ); + } + const graph = new SpecIndex(root).graph(); + expect(graph.nodes.get("dup")?.path).toBe("a-earlier/SPEC.md"); + expect(graph.duplicateIds.get("dup")).toEqual([ + "a-earlier/SPEC.md", + "m-middle/SPEC.md", + "z-later/SPEC.md", + ]); + }); +}); + +test("SpecIndex walks a directory in its place among its sibling files, not before or after them", () => { + withIndexRoot((root) => { + const spec = "---\nid: dup\ntype: module-design\ntitle: Dup\n---\n"; + mkdirSync(join(root, "b-dir"), { recursive: true }); + writeFileSync(join(root, "a.md"), spec); + writeFileSync(join(root, "b-dir", "SPEC.md"), spec); + writeFileSync(join(root, "c.md"), spec); + + const graph = new SpecIndex(root).graph(); + expect(graph.duplicateIds.get("dup")).toEqual(["a.md", "b-dir/SPEC.md", "c.md"]); + expect(graph.nodes.get("dup")?.path).toBe("a.md"); + }); +}); + +test("the glob keeps the byte-exact ignored rule the resolver deliberately over-refuses", () => { + withIndexRoot((root) => { + mkdirSync(join(root, "NODE_MODULES"), { recursive: true }); + writeFileSync( + join(root, "NODE_MODULES", "SPEC.md"), + "---\nid: shouted\ntype: module-design\ntitle: S\n---\n", + ); + + expect(resolveSpecPath(root, "NODE_MODULES/SPEC.md")).toHaveProperty("error"); + expect(new SpecIndex(root).graph().nodes.has("shouted")).toBe(true); + }); +}); + +const walkOrder = (names: readonly string[]): string[] => + names + .map((name) => toWalkEntry(name, false)) + .sort(compareWalkEntries) + .map((entry) => entry.name); + +test("the walk order compares NFC-normalized names, not raw code units", () => { + expect(walkOrder(["A\u0308pfel", "Banana"])).toEqual(["Banana", "A\u0308pfel"]); + expect(walkOrder(["Banana", "A\u0308pfel"])).toEqual(["Banana", "A\u0308pfel"]); + expect(walkOrder(["\u00c4pfel", "Banana"])).toEqual(["Banana", "\u00c4pfel"]); +}); + +test("the walk order is total, so canonically equivalent names sort the same either way round", () => { + const names = ["caf\u00e9", "cafe\u0301", "zebra", "\u00c4pfel", "A\u0308pfel", "apple"]; + expect(walkOrder(names)).toEqual(walkOrder([...names].reverse())); + expect(walkOrder(["zebra", "caf\u00e9", "apple", "cafe\u0301"])).toEqual([ + "apple", + "cafe\u0301", + "caf\u00e9", + "zebra", + ]); +}); + +spellingPreserving( + "SpecIndex resolves a duplicate id the same way for canonically equivalent directory names", + () => { + withIndexRoot((root) => { + const spellings = ["caf\u00e9", "cafe\u0301"]; + for (const name of spellings) { + mkdirSync(join(root, name), { recursive: true }); + writeFileSync( + join(root, name, "SPEC.md"), + "---\nid: dup\ntype: module-design\ntitle: Dup\n---\n", + ); + } + const expected = ["cafe\u0301/SPEC.md", "caf\u00e9/SPEC.md"]; + const graph = new SpecIndex(root).graph(); + expect(graph.duplicateIds.get("dup")).toEqual(expected); + expect(graph.nodes.get("dup")?.path).toBe(expected[0]); + }); + }, +); diff --git a/packages/spec-graph/core/index.ts b/packages/spec-graph/core/index.ts index 33769524a..53cc4ffba 100644 --- a/packages/spec-graph/core/index.ts +++ b/packages/spec-graph/core/index.ts @@ -34,6 +34,7 @@ export { } from "./parse.ts"; export { + DEFAULT_GREP_LIMIT, type GraphSlice, type GrepMatch, type GrepOptions, @@ -46,7 +47,13 @@ export { type SpecContentEntry, type SpecFilters, } from "./query.ts"; -export { type SpecFileRecord, SpecIndex } from "./store.ts"; +export { + resolveSpecPath, + SPEC_FILE_EXTENSION, + type SpecFileRecord, + SpecIndex, + type SpecPathResolution, +} from "./store.ts"; export { type DanglingLink, type DuplicateId, diff --git a/packages/spec-graph/core/parse.ts b/packages/spec-graph/core/parse.ts index 9c49e049f..a38b19682 100644 --- a/packages/spec-graph/core/parse.ts +++ b/packages/spec-graph/core/parse.ts @@ -53,8 +53,15 @@ export const FIELD_ORDER = [ export type LinkKind = (typeof SINGLE_LINK_FIELDS)[number] | (typeof LIST_LINK_FIELDS)[number]; const FENCE = "---"; +const BOM = "\ufeff"; +const CRLF = "\r\n"; const TO_STRING = { lineWidth: 0, flowCollectionPadding: false } as const; +function lineEnding(text: string): string { + const first = text.indexOf("\n"); + return first > 0 && text[first - 1] === "\r" ? CRLF : "\n"; +} + export interface ParsedFile { frontmatter: Frontmatter | null; body: string; @@ -76,7 +83,7 @@ function toFrontmatter(loaded: unknown): Frontmatter | null { } function splitFrontmatter(content: string): { fmText: string | null; body: string } { - const normalized = content.startsWith("\ufeff") ? content.slice(1) : content; + const normalized = content.startsWith(BOM) ? content.slice(1) : content; const lines = normalized.split("\n"); if (lines[0]?.trim() !== FENCE) return { fmText: null, body: content }; let end = -1; @@ -215,6 +222,7 @@ export function updateFrontmatterText( } inlineLists(doc); - const out = `${FENCE}\n${doc.toString(TO_STRING)}${FENCE}\n${body}`; - return { content: fileText.includes("\r\n") ? out.replace(/\r?\n/g, "\r\n") : out }; + const block = `${FENCE}\n${doc.toString(TO_STRING)}${FENCE}\n`; + const rewritten = lineEnding(fileText) === CRLF ? block.replace(/\n/g, CRLF) : block; + return { content: `${fileText.startsWith(BOM) ? BOM : ""}${rewritten}${body}` }; } diff --git a/packages/spec-graph/core/query.ts b/packages/spec-graph/core/query.ts index f58c17605..b78da28e1 100644 --- a/packages/spec-graph/core/query.ts +++ b/packages/spec-graph/core/query.ts @@ -32,6 +32,8 @@ export interface GrepResult { truncated: boolean; } +export const DEFAULT_GREP_LIMIT = 200; + function matchesFilters(fm: Frontmatter, filters: SpecFilters): boolean { if (filters.type !== undefined && scalar(fm, FIELDS.type) !== filters.type) return false; if (filters.parent !== undefined && scalar(fm, FIELDS.parent) !== filters.parent) return false; @@ -56,14 +58,16 @@ function buildMatcher(opts: GrepOptions): (line: string) => boolean { } export function grepSpecs(entries: SpecContentEntry[], opts: GrepOptions): GrepResult { - const limit = opts.limit ?? 200; + const requested = Math.trunc(opts.limit ?? DEFAULT_GREP_LIMIT); + const limit = requested > 0 ? requested : DEFAULT_GREP_LIMIT; const matcher = buildMatcher(opts); const matches: GrepMatch[] = []; for (const entry of entries) { if (!matchesFilters(entry.frontmatter, opts)) continue; const lines = entry.content.split("\n"); for (let i = 0; i < lines.length; i++) { - const line = lines[i] ?? ""; + const raw = (i === 0 ? (lines[0]?.replace(/^\ufeff/, "") ?? "") : (lines[i] ?? "")) as string; + const line = raw.endsWith("\r") ? raw.slice(0, -1) : raw; if (matcher(line)) { if (matches.length >= limit) return { matches, truncated: true }; matches.push({ path: entry.path, line: i + 1, snippet: line.trim() }); diff --git a/packages/spec-graph/core/store.ts b/packages/spec-graph/core/store.ts index dee4bf4f8..9224dd78a 100644 --- a/packages/spec-graph/core/store.ts +++ b/packages/spec-graph/core/store.ts @@ -1,11 +1,165 @@ -import { readdirSync, readFileSync, statSync } from "node:fs"; -import { join, relative, sep } from "node:path"; +import { existsSync, lstatSync, readdirSync, readFileSync, statSync } from "node:fs"; +import { isAbsolute, join, normalize, relative, sep, win32 } from "node:path"; import { buildGraph, type SpecGraph } from "./graph.ts"; import { FIELDS, type Frontmatter, isSpec, parseFile, scalar } from "./parse.ts"; import type { SpecContentEntry } from "./query.ts"; const IGNORED_DIRS = new Set(["node_modules", ".git", "dist", "build"]); +export const SPEC_FILE_EXTENSION = ".md"; + +interface PathSemantics { + isAbsolute(path: string): boolean; + relative(from: string, to: string): string; + sep: string; +} + +const NATIVE_PATH_SEMANTICS: PathSemantics = { isAbsolute, relative, sep }; + +export function hasWindowsNamespaceSyntax(path: string): boolean { + return win32.parse(path).root !== "" || path.includes(":"); +} + +export function isPathInsideRoot( + root: string, + target: string, + paths: PathSemantics = NATIVE_PATH_SEMANTICS, +): boolean { + const rel = paths.relative(root, target); + return rel !== ".." && !rel.startsWith(`..${paths.sep}`) && !paths.isAbsolute(rel); +} + +function isSymlink(target: string): boolean { + try { + return lstatSync(target).isSymbolicLink(); + } catch { + return false; + } +} + +function resolves(target: string): boolean { + try { + lstatSync(target); + return true; + } catch { + return false; + } +} + +function directoryEntries(dir: string): string[] | null { + try { + return readdirSync(dir); + } catch { + return null; + } +} + +function fold(name: string): string { + return name.normalize("NFC").toLowerCase(); +} + +function isIgnoredName(name: string): boolean { + return IGNORED_DIRS.has(fold(name)); +} + +export type SegmentResolution = { name: string } | { error: string }; + +export function resolvePathSegment( + entries: readonly string[], + segment: string, + exists: boolean, +): SegmentResolution { + let name = segment; + if (exists && !entries.includes(segment)) { + const folded = fold(segment); + const [only, ...rest] = entries.filter((entry) => fold(entry) === folded); + if (only === undefined) { + return { + error: `Path component "${segment}" resolves to no entry its parent directory lists`, + }; + } + if (rest.length > 0) { + return { + error: `Path component "${segment}" matches more than one entry on this filesystem ("${only}", "${rest.join('", "')}")`, + }; + } + name = only; + } + if (isIgnoredName(name)) { + return { error: `Path is inside an ignored directory ("${name}") and would not be indexed` }; + } + return { name }; +} + +export type SpecPathResolution = { rel: string; abs: string } | { error: string }; + +export function resolveSpecPath(root: string, path: string): SpecPathResolution { + if (path.trim() === "") return { error: "Path must not be empty." }; + if (isAbsolute(path)) return { error: `Path must be root-relative, not absolute: ${path}` }; + if (process.platform === "win32" && hasWindowsNamespaceSyntax(path)) { + return { error: `Path must not use Windows drive or stream syntax: ${path}` }; + } + if (!path.endsWith(SPEC_FILE_EXTENSION)) { + return { error: `Spec files must end in ${SPEC_FILE_EXTENSION}: ${path}` }; + } + + const segments = normalize(path).split(sep); + if (segments.includes("..")) { + return { error: `Path must stay inside the project root: ${path}` }; + } + if (!existsSync(root)) return { error: `Project root does not exist: ${root}` }; + + let walked = root; + let walkedExists = true; + const canonical: string[] = []; + for (const segment of segments) { + let entries: readonly string[] = []; + if (walkedExists) { + const listed = directoryEntries(walked); + if (listed === null) { + return { + error: `Path passes through a directory the index cannot list: ${path}`, + }; + } + entries = listed; + } + const exists: boolean = walkedExists && resolves(join(walked, segment)); + const resolution = resolvePathSegment(entries, segment, exists); + if ("error" in resolution) return { error: `${resolution.error}: ${path}` }; + walked = join(walked, resolution.name); + if (isSymlink(walked)) { + return { error: `Path passes through a symlink, which the index never follows: ${path}` }; + } + canonical.push(resolution.name); + walkedExists = exists; + } + + const rel = canonical.join("/"); + if (!rel.endsWith(SPEC_FILE_EXTENSION)) { + return { error: `Spec files must end in ${SPEC_FILE_EXTENSION}: ${path}` }; + } + if (!isPathInsideRoot(root, walked)) { + return { error: `Path must stay inside the project root: ${path}` }; + } + return { rel, abs: walked }; +} + +export interface WalkEntry { + readonly name: string; + readonly key: string; + readonly directory: boolean; +} + +export function toWalkEntry(name: string, directory: boolean): WalkEntry { + return { name, key: name.normalize("NFC"), directory }; +} + +export function compareWalkEntries(a: WalkEntry, b: WalkEntry): number { + if (a.key !== b.key) return a.key < b.key ? -1 : 1; + if (a.name !== b.name) return a.name < b.name ? -1 : 1; + return 0; +} + export interface SpecFileRecord { abs: string; rel: string; @@ -45,15 +199,20 @@ export class SpecIndex { } catch { return; } + const candidates: WalkEntry[] = []; for (const dirent of dirents) { - const abs = join(dir, dirent.name); if (dirent.isDirectory()) { - if (IGNORED_DIRS.has(dirent.name)) continue; - yield* this.walk(abs); - } else if (dirent.isFile() && dirent.name.endsWith(".md")) { - yield abs; + if (!IGNORED_DIRS.has(dirent.name)) candidates.push(toWalkEntry(dirent.name, true)); + } else if (dirent.isFile() && dirent.name.endsWith(SPEC_FILE_EXTENSION)) { + candidates.push(toWalkEntry(dirent.name, false)); } } + candidates.sort(compareWalkEntries); + for (const candidate of candidates) { + const abs = join(dir, candidate.name); + if (candidate.directory) yield* this.walk(abs); + else yield abs; + } } private scan(): SpecFileRecord[] { diff --git a/packages/spec-graph/tools/SPEC.md b/packages/spec-graph/tools/SPEC.md index c9d495f17..8549eedbe 100644 --- a/packages/spec-graph/tools/SPEC.md +++ b/packages/spec-graph/tools/SPEC.md @@ -46,6 +46,11 @@ file; the other four are read-only. `spec_create.status` ← `SPEC_STATUSES`, `spec_graph.direction` ← `SLICE_DIRECTIONS`, `spec_graph.edge` ← `LINK_KINDS`) — never re-typed literals, so a `core` rename flows here with no edit (pinned by `tools/tools.test.ts`). +- `spec_create` writes only what the index can later read back, and it checks that in three places: the + target path goes through `core`'s `resolveSpecPath` (which also yields the canonical relative path the + tool reports, so its `Created ` never disagrees with what `spec_get` will say), the assembled + bytes must parse back as a spec before anything is written, and the write itself is exclusive + (`flag: "wx"`) so the existence check cannot be raced or satisfied by a link. - The spec root is `ctx.cwd`; one `SpecIndex` is reused per root (freshness handled in `core/` — see `module-spec-graph`). `spec_update` reads via `recordForId` to reuse the scan's cached read; write tools just write, and the next read picks the change up. diff --git a/packages/spec-graph/tools/create.ts b/packages/spec-graph/tools/create.ts index 2d5e1d53f..bc8532424 100644 --- a/packages/spec-graph/tools/create.ts +++ b/packages/spec-graph/tools/create.ts @@ -6,6 +6,9 @@ import { Type } from "typebox"; import { FIELDS, type Frontmatter, + isSpec, + parseFile, + resolveSpecPath, SPEC_STATUSES, SPEC_TYPES, serializeFrontmatter, @@ -14,7 +17,8 @@ import { errorResult, getIndex, scaffoldBody, textResult } from "./shared.ts"; const parameters = Type.Object({ path: Type.String({ - description: "Root-relative path for the new spec file (e.g. src/foo/SPEC.md).", + description: + "Root-relative path for the new spec file, ending in .md (e.g. src/foo/SPEC.md). Must stay inside the project root.", }), id: Type.String({ description: "Unique spec id." }), type: StringEnum(SPEC_TYPES, { @@ -40,14 +44,17 @@ export function registerSpecCreate(pi: ExtensionAPI): void { name: "spec_create", label: "Spec Create", description: - "Create a new spec file with scaffolded frontmatter (id, type, title, an optional status, and any links) and a heading-only body stub chosen by type. Fails if the file already exists or the id is already in use. Edit prose afterward with the write/edit tools.", + "Create a new spec file with scaffolded frontmatter (id, type, title, an optional status, and any links) and a heading-only body stub chosen by type. Fails if the file already exists, the id is already in use, or the path is not an indexable root-relative .md path. Edit prose afterward with the write/edit tools.", promptSnippet: "spec_create — create a new spec file with scaffolded frontmatter (id/type/title/links) and heading stubs.", parameters, async execute(_callId, params, _signal, _onUpdate, ctx) { + const resolved = resolveSpecPath(ctx.cwd, params.path); + if ("error" in resolved) return errorResult(resolved.error); + const { rel, abs } = resolved; + const index = getIndex(ctx.cwd); - const abs = index.absPath(params.path); - if (existsSync(abs)) return errorResult(`File already exists: ${params.path}`); + if (existsSync(abs)) return errorResult(`File already exists: ${rel}`); if (index.graph().nodes.has(params.id)) { return errorResult(`Spec id "${params.id}" is already in use.`); } @@ -63,16 +70,18 @@ export function registerSpecCreate(pi: ExtensionAPI): void { if (params.tags?.length) fm[FIELDS.tags] = params.tags; const content = `${serializeFrontmatter(fm)}\n${scaffoldBody(params.type)}`; + if (!isSpec(parseFile(content).frontmatter)) { + return errorResult( + `Refusing to write ${rel}: the frontmatter would not be a spec (id and type must be non-empty).`, + ); + } try { mkdirSync(dirname(abs), { recursive: true }); - writeFileSync(abs, content, "utf8"); + writeFileSync(abs, content, { encoding: "utf8", flag: "wx" }); } catch (err) { - return errorResult(`Failed to write ${params.path}: ${(err as Error).message}`); + return errorResult(`Failed to write ${rel}: ${(err as Error).message}`); } - return textResult(`Created ${params.path} (id: ${params.id}).`, { - path: params.path, - id: params.id, - }); + return textResult(`Created ${rel} (id: ${params.id}).`, { path: rel, id: params.id }); }, }); } diff --git a/packages/spec-graph/tools/grep.ts b/packages/spec-graph/tools/grep.ts index 50ddaf34b..dbf44990a 100644 --- a/packages/spec-graph/tools/grep.ts +++ b/packages/spec-graph/tools/grep.ts @@ -1,6 +1,6 @@ import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; import { Type } from "typebox"; -import { type GrepResult, grepSpecs } from "../core/index.ts"; +import { DEFAULT_GREP_LIMIT, type GrepResult, grepSpecs } from "../core/index.ts"; import { errorResult, getIndex, textResult } from "./shared.ts"; const parameters = Type.Object({ @@ -19,7 +19,9 @@ const parameters = Type.Object({ dependsOn: Type.Optional( Type.String({ description: "Only search specs that depend-on this id." }), ), - limit: Type.Optional(Type.Number({ description: "Max matches to return (default: 200)." })), + limit: Type.Optional( + Type.Number({ description: `Max matches to return (default: ${DEFAULT_GREP_LIMIT}).` }), + ), }); export function registerSpecGrep(pi: ExtensionAPI): void { diff --git a/packages/spec-graph/tools/tools.test.ts b/packages/spec-graph/tools/tools.test.ts index e148c63cd..0244bfa05 100644 --- a/packages/spec-graph/tools/tools.test.ts +++ b/packages/spec-graph/tools/tools.test.ts @@ -1,5 +1,13 @@ import { expect, test } from "bun:test"; -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import type { @@ -8,9 +16,28 @@ import type { ExtensionContext, ToolDefinition, } from "@earendil-works/pi-coding-agent"; -import { LINK_KINDS, SLICE_DIRECTIONS, SPEC_STATUSES, SPEC_TYPES } from "../core/index.ts"; +import { + LINK_KINDS, + resolveSpecPath, + SLICE_DIRECTIONS, + SPEC_STATUSES, + SPEC_TYPES, +} from "../core/index.ts"; import { registerSpecTools } from "./index.ts"; +function foldsCase(): boolean { + const probe = mkdtempSync(join(tmpdir(), "spec-probe-")); + try { + mkdirSync(join(probe, "CaseProbe"), { recursive: true }); + return existsSync(join(probe, "caseprobe")); + } finally { + rmSync(probe, { recursive: true, force: true }); + } +} + +const caseFolding = test.skipIf(!foldsCase()); +const windowsOnly = test.skipIf(process.platform !== "win32"); + const tools = new Map(); registerSpecTools({ registerTool(tool: ToolDefinition) { @@ -176,6 +203,115 @@ test("spec_create rejects a duplicate id and an existing path", async () => { }); }); +test("spec_create refuses a path the index could never see, and writes nothing", async () => { + await withRoot(async (outer) => { + const root = join(outer, "project"); + mkdirSync(root, { recursive: true }); + const rejected = [ + "../escape.md", + "pkg/../../escape.md", + "/tmp/escape.md", + "notes/spec.txt", + "node_modules/dep/SPEC.md", + "", + ]; + let n = 0; + for (const path of rejected) { + const res = await run( + "spec_create", + { path, id: `id-${n++}`, type: "module-design", title: "T" }, + root, + ); + expect(isError(res)).toBe(true); + const expected = resolveSpecPath(root, path); + expect(text(res)).toContain((expected as { error: string }).error); + } + expect(existsSync(join(outer, "escape.md"))).toBe(false); + expect(existsSync(join(root, "notes/spec.txt"))).toBe(false); + }); +}); + +windowsOnly("spec_create refuses Windows drive-relative and stream paths", async () => { + await withRoot(async (root) => { + let n = 0; + for (const path of ["C:..\\..\\outside.md", "notes.txt:SPEC.md"]) { + const result = await run( + "spec_create", + { path, id: `windows-${n++}`, type: "task-spec", title: "Windows" }, + root, + ); + expect(isError(result)).toBe(true); + expect(text(result)).toContain("Windows drive or stream syntax"); + } + }); +}); + +caseFolding( + "spec_create refuses a case alias of an ignored directory, and writes nothing", + async () => { + await withRoot(async (root) => { + mkdirSync(join(root, "node_modules"), { recursive: true }); + + const res = await run( + "spec_create", + { path: "NODE_MODULES/SPEC.md", id: "aliased", type: "module-design", title: "A" }, + root, + ); + expect(isError(res)).toBe(true); + expect(existsSync(join(root, "node_modules", "SPEC.md"))).toBe(false); + }); + }, +); + +test("spec_create writes nothing through a symlink, dangling leaf included", async () => { + await withRoot(async (outer) => { + const root = join(outer, "project"); + mkdirSync(join(outer, "elsewhere"), { recursive: true }); + mkdirSync(root, { recursive: true }); + symlinkSync(join(outer, "elsewhere"), join(root, "link"), "dir"); + symlinkSync(join(outer, "leaf.md"), join(root, "leaf.md")); + + for (const path of ["link/evil.md", "leaf.md"]) { + const res = await run( + "spec_create", + { path, id: `evil-${path}`, type: "module-design", title: "E" }, + root, + ); + expect(isError(res)).toBe(true); + } + expect(existsSync(join(outer, "elsewhere", "evil.md"))).toBe(false); + expect(existsSync(join(outer, "leaf.md"))).toBe(false); + }); +}); + +test("spec_create reports the canonical path the index will produce, not the caller's spelling", async () => { + await withRoot(async (root) => { + const res = await run( + "spec_create", + { path: "./pkg/sub/../SPEC.md", id: "pkg", type: "module-design", title: "P" }, + root, + ); + expect(isError(res)).toBe(false); + expect(res.details).toMatchObject({ path: "pkg/SPEC.md" }); + expect(text(res)).toContain("pkg/SPEC.md"); + + const got = await run("spec_get", { id: "pkg" }, root); + expect(text(got)).toContain("path: pkg/SPEC.md"); + }); +}); + +test("spec_create refuses params that would write a file born a non-spec", async () => { + await withRoot(async (root) => { + const res = await run( + "spec_create", + { path: "blank.md", id: "", type: "module-design", title: "T" }, + root, + ); + expect(isError(res)).toBe(true); + expect(existsSync(join(root, "blank.md"))).toBe(false); + }); +}); + test("spec_graph returns a bounded subtree; spec_grep searches within specs + metadata filter", async () => { await withRoot(async (root) => { await run(