diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index e4da7b5..3f38b46 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -13,7 +13,7 @@ "name": "kensai", "description": "Multi-agent code review pipeline with adversarial falsification, orchestrated by a stateful MCP server", "source": "./claude/kensai", - "version": "0.1.7", + "version": "0.1.8", "license": "MIT", "keywords": [ "code-review", diff --git a/claude/kensai/.claude-plugin/plugin.json b/claude/kensai/.claude-plugin/plugin.json index a82b6ad..0a536ec 100644 --- a/claude/kensai/.claude-plugin/plugin.json +++ b/claude/kensai/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "kensai", - "version": "0.1.7", + "version": "0.1.8", "description": "Multi-agent code review pipeline with adversarial falsification, orchestrated by a stateful MCP server", "author": { "name": "xobotyi", diff --git a/claude/kensai/mcp/review/CLAUDE.md b/claude/kensai/mcp/review/CLAUDE.md index 0db7406..89c2599 100644 --- a/claude/kensai/mcp/review/CLAUDE.md +++ b/claude/kensai/mcp/review/CLAUDE.md @@ -37,7 +37,7 @@ src/ │ └── CLAUDE.md ├── repofs/ — scoped filesystem facade composing all sub-modules │ ├── CLAUDE.md -│ ├── git/ — thin git CLI abstraction with diff annotation +│ ├── git/ — thin git CLI abstraction with diff annotation; gitignore/ matcher submodule │ ├── rg/ — ripgrep CLI abstraction for content search │ ├── pathindex/ — tree-structured file index with fuzzy/glob search │ └── lineiter/ — streaming line iterator with pluggable binary detection diff --git a/claude/kensai/mcp/review/package.json b/claude/kensai/mcp/review/package.json index b440ead..63df3e6 100644 --- a/claude/kensai/mcp/review/package.json +++ b/claude/kensai/mcp/review/package.json @@ -1,6 +1,6 @@ { "name": "@gaijin/kensai-review-mcp", - "version": "0.1.7", + "version": "0.1.8", "description": "Stateful MCP server for the kensai multi-agent code review pipeline", "license": "MIT", "author": { diff --git a/claude/kensai/mcp/review/src/repofs/CLAUDE.md b/claude/kensai/mcp/review/src/repofs/CLAUDE.md index e4d6694..7911fc2 100644 --- a/claude/kensai/mcp/review/src/repofs/CLAUDE.md +++ b/claude/kensai/mcp/review/src/repofs/CLAUDE.md @@ -6,22 +6,21 @@ Single entry point for all filesystem and search operations within a project roo ## API -| Export | Purpose | -| -------------------------------- | ---------------------------------------------------------------- | -| `RepoFs.open(root, signal?)` | Async factory. Builds file index and validates git in parallel | -| `.root` | Absolute path to repository root | -| `.git` | `Repo` instance for git operations | -| `.index` | `PathIndex` instance — O(1) lookup, fuzzy search, glob filter | -| `.resolve(path)` | Normalize any path to POSIX relative from root. Throws on escape | -| `.fileExists(path)` | Check file existence via index | -| `.dirExists(path)` | Check directory existence via index | -| `.readFile(path, opts?)` | Read text file with binary detection, line windowing, byte cap | -| `.listDir(path, opts?)` | List directory entries from index tree with depth control | -| `.findFiles(pattern, opts?)` | Fuzzy search file paths via PathIndex | -| `.globFiles(opts?)` | Glob-filter indexed paths via PathIndex | -| `.grep(pattern, opts?, signal?)` | Content search via ripgrep scoped to root | -| `.flushReadPaths()` | Return and clear paths from successful readFile calls | -| `RepoFsError` | Error class for scoped filesystem failures | +| Export | Purpose | +| -------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- | +| `RepoFs.open(root, signal?)` | Async factory. Builds gitignore-aware index (pure filesystem walk) and validates git in parallel | +| `.root` | Absolute path to repository root | +| `.git` | `Repo` instance for git operations | +| `.index` | `PathIndex` instance — O(1) lookup, fuzzy search, glob filter. Gitignore-filtered walk; review layer reconciles changed files via `add()` | +| `.resolve(path)` | Normalize any path to POSIX relative from root. Throws on escape | +| `.fileExists(path)` | Check file existence via index | +| `.dirExists(path)` | Check directory existence via index | +| `.readFile(path)` | Validate via index, return raw bytes. Lazily fills the entry's size; lstat fallback for unindexed paths | +| `.findFiles(pattern, opts?)` | Fuzzy search file paths via PathIndex | +| `.globFiles(opts?)` | Glob-filter indexed paths via PathIndex | +| `.grep(pattern, opts?, signal?)` | Content search via ripgrep scoped to root | +| `.flushReadPaths()` | Return and clear paths from successful readFile calls | +| `RepoFsError` | Error class for scoped filesystem failures | ## Path Resolution @@ -34,61 +33,35 @@ Single entry point for all filesystem and search operations within a project roo 5. Returns the POSIX relative path from root (index-compatible) All index lookups, error messages, and read tracking use the resolved relative path. -The only place that needs the absolute path is `createReadStream` in `readFile`. +The only place that needs the absolute path is `fsp.readFile` (and the fallback `lstat`) in `readFile`. ## File Reading -Validation chain (all index-based, no stat calls): +`readFile` validates and returns raw bytes — line windowing, binary probing, and length caps live in +the `read_file` tool (`src/toolsets/fs/read_file.ts`): 1. `resolve()` — containment -2. `index.get()` — existence, rejects dirs and symlinks by entry type -3. Binary extension check -4. Byte limit enforcement (when no explicit `lineLimit`) -5. Streaming read via `LineIter` with binary probe and per-line byte cap +2. `index.get()` — rejects non-file entries (dirs, symlinks) by type +3. Unindexed paths fall back to the filesystem: first segment must not be in `WALK_EXCLUDE_DIRS`, + `lstat` must report a regular file (symlinks rejected) +4. `fsp.readFile` — fills the index entry's lazy `size` from the buffer length +5. Records the read path for instruction resolution via `flushReadPaths()` -Returns `ReadResult` with `lines` (num, content, fullLen), `truncated`, `empty`, `pastEof`. -Records successful reads for future instruction resolution via `flushReadPaths()`. - -### ReadFileOptions - -| Option | Default | Purpose | -| ----------------- | ------- | ------------------------------------------------- | -| `lineOffset` | 1 | Start line (1-based) | -| `lineLimit` | 2000 | Max lines returned | -| `lineLengthCap` | 1024 | Per-line byte cap before truncation | -| `binaryProbeSize` | 512 | Bytes scanned for null byte detection | -| `byteLimit` | 256 KB | Reject files over this without explicit lineLimit | -| `signal` | — | AbortSignal for cancellation | - -## Directory Listing - -Traverses the PathIndex tree — no filesystem I/O at list time. -Index is built once at `open()` and reflects the state at that point. - -Returns `ListResult` with `entries` (path, name, isDir, isSymlink, size) and `truncated`. -Entries sorted by name within each level. Excluded directories are listed but not descended into. - -### ListDirOptions - -| Option | Default | Purpose | -| -------------- | ---------------------------------------------------- | ----------------------------------------- | -| `maxDepth` | 1 | Recursion depth. 1 = direct children only | -| `maxEntries` | 1000 | Maximum entries before truncation | -| `skipDotfiles` | false | Omit entries starting with "." | -| `excludeDirs` | vendor, node_modules, .git, dist, build, **pycache** | Skip descent into these directories | +Directory listing has no RepoFs method — the `list_dir` tool (`src/toolsets/fs/list_dir.ts`) +traverses `.index.dir()` directly. ## Containment `resolve()` prevents path escape via textual containment: `path.relative(root, abs)` must not start with `..`. Symlinks are safe because PathIndex does not traverse them — symlinked entries -appear in the index with `type: "symlink"` and `readFile` rejects them by type. +appear in the index with `type: "symlink"` and `readFile` rejects them by type. The `readFile` +fallback for unindexed paths uses `lstat` and rejects non-regular files, so a gitignored symlink +cannot bypass containment. ## Dependencies -- `node:fs` (createReadStream) -- `node:fs/promises` (stat — only in `open()`) -- `node:path` (resolve, relative, posix.normalize, posix.basename, posix.dirname, posix.extname) +- `node:fs/promises` (stat in `open()`, readFile + fallback lstat in `readFile`) +- `node:path` (resolve, relative, posix.normalize) - `../git/git.ts` (Repo) -- `../lineiter/lineiter.ts` (LineIter, ErrBinaryContent) -- `../pathindex/pathindex.ts` (PathIndex, FilterOptions, IndexEntryDir) +- `../pathindex/pathindex.ts` (PathIndex, WALK_EXCLUDE_DIRS, FilterOptions) - `../rg/rg.ts` (grep, GrepOptions, GrepResult) diff --git a/claude/kensai/mcp/review/src/repofs/git/CLAUDE.md b/claude/kensai/mcp/review/src/repofs/git/CLAUDE.md index 62d7c29..5ab12a0 100644 --- a/claude/kensai/mcp/review/src/repofs/git/CLAUDE.md +++ b/claude/kensai/mcp/review/src/repofs/git/CLAUDE.md @@ -2,6 +2,7 @@ Thin abstraction over the local git CLI. Ported from Go `git/repo.go` and `gittoolset/annotate.go`. Foundation for `git-diff`, `git-changed-files`, `git-log` tools. +Contains the `gitignore/` submodule (pattern matching for walk-time filtering) — see its `CLAUDE.md`. ## API diff --git a/claude/kensai/mcp/review/src/repofs/git/gitignore/CLAUDE.md b/claude/kensai/mcp/review/src/repofs/git/gitignore/CLAUDE.md new file mode 100644 index 0000000..f410b09 --- /dev/null +++ b/claude/kensai/mcp/review/src/repofs/git/gitignore/CLAUDE.md @@ -0,0 +1,37 @@ +# gitignore + +Gitignore pattern matching ported from the Go reference implementation. Parses `.gitignore` syntax into compiled +matchers and chains them hierarchically for walk-time filtering. + +## API + +| Export | Purpose | +| ------------------------- | ---------------------------------------------------------------------------------------------------------- | +| `parse(content, dir)` | Parse a `.gitignore` body into a `Matcher`. `dir` is the file's directory relative to root (`""` for root) | +| `parsePattern(line, dir)` | Compile one gitignore line. Returns `null` for blanks, comments, invalid patterns | +| `Matcher` | Immutable pattern list with optional parent chain | +| `.match(relPath, isDir)` | `true` if the path is ignored. Last-match-wins; own patterns checked before parent chain | +| `.append(...patterns)` | New `Matcher` layered on top of this one | +| `.withParent(parent)` | Rechain onto a new parent; collapses to `parent` when this matcher is empty | +| `MatchResult`, `Pattern` | Pattern interface and per-pattern match result (`no-match` / `exclude` / `include`) | + +## Semantics + +- Full gitignore syntax: negation (`!`), dir-only trailing `/`, anchoring (leading or interior `/`), `**` globs, + character classes, escaped `#` / `!` / trailing space +- `[!abc]` character classes normalized to picomatch's `[^abc]` +- Globs match dotfiles; braces and extglobs are literal — gitignore has no brace expansion +- Basename patterns (no slash) match at any depth below the defining directory; patterns with a slash are anchored to + it (compiled with a `dir/` prefix) +- Nested `.gitignore` scoping is by construction: consumers consult a directory's matcher chain only for paths under + that directory + +## Consumers + +- `pathindex` — walk-time filtering: per-directory `.gitignore` chained onto the parent matcher, `.git/info/exclude` + seeds the root chain + +## Dependencies + +- `picomatch` — glob compilation +- `node:path/posix` (basename) diff --git a/claude/kensai/mcp/review/src/repofs/git/gitignore/gitignore.test.ts b/claude/kensai/mcp/review/src/repofs/git/gitignore/gitignore.test.ts new file mode 100644 index 0000000..2d4666e --- /dev/null +++ b/claude/kensai/mcp/review/src/repofs/git/gitignore/gitignore.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { Matcher, parse, parsePattern } from "./gitignore.ts"; + +describe("parse", () => { + it("parses mixed content", () => { + const content = ["# comment", "", "*.log", "build/", "!important.log", " ", "doc/**/*.pdf"].join("\n"); + + const m = parse(content, ""); + + expect(m.match("debug.log", false)).toBe(true); + expect(m.match("build", true)).toBe(true); + expect(m.match("build", false)).toBe(false); + expect(m.match("important.log", false)).toBe(false); + expect(m.match("doc/ref/manual.pdf", false)).toBe(true); + expect(m.match("main.go", false)).toBe(false); + }); + + it("parses with dir scoping", () => { + const content = "build/output.log\n**/test\n"; + const m = parse(content, "src/pkg"); + + expect(m.match("src/pkg/build/output.log", false)).toBe(true); + expect(m.match("build/output.log", false)).toBe(false); + expect(m.match("src/pkg/test", true)).toBe(true); + expect(m.match("src/pkg/a/b/test", true)).toBe(true); + }); +}); + +describe("Matcher — last match wins", () => { + it("negation overrides earlier exclude", () => { + const p1 = parsePattern("*.log", "")!; + const p2 = parsePattern("!important.log", "")!; + const m = new Matcher([p1, p2]); + + expect(m.match("important.log", false)).toBe(false); + expect(m.match("debug.log", false)).toBe(true); + expect(m.match("main.go", false)).toBe(false); + }); +}); + +describe("Matcher — append", () => { + it("child overrides parent immutably", () => { + const p1 = parsePattern("*.log", "")!; + const parent = new Matcher([p1]); + + const p2 = parsePattern("!important.log", "")!; + const child = parent.append(p2); + + expect(parent.match("important.log", false)).toBe(true); + expect(child.match("important.log", false)).toBe(false); + }); + + it("append on empty returns new matcher", () => { + const p1 = parsePattern("*.log", "")!; + const m = new Matcher([]); + const result = m.append(p1); + + expect(result).not.toBe(m); + expect(result.match("debug.log", false)).toBe(true); + }); + + it("append with no patterns returns same instance", () => { + const p1 = parsePattern("*.log", "")!; + const m = new Matcher([p1]); + expect(m.append()).toBe(m); + }); +}); + +describe("Matcher — withParent", () => { + it("child overrides parent", () => { + const parent = parse("*.log\n", ""); + const child = parse("!important.log\n", ""); + const chained = child.withParent(parent); + + expect(chained!.match("important.log", false)).toBe(false); + expect(chained!.match("debug.log", false)).toBe(true); + }); + + it("empty child returns parent", () => { + const parent = parse("*.log\n", ""); + const child = new Matcher([]); + const result = child.withParent(parent); + + expect(result).toBe(parent); + }); + + it("child with null parent works", () => { + const child = parse("*.log\n", ""); + const result = child.withParent(null); + + expect(result).not.toBeNull(); + expect(result!.match("debug.log", false)).toBe(true); + }); +}); + +describe("Matcher — empty", () => { + it("empty matcher matches nothing", () => { + const m = new Matcher([]); + expect(m.match("anything", false)).toBe(false); + }); +}); diff --git a/claude/kensai/mcp/review/src/repofs/git/gitignore/gitignore.ts b/claude/kensai/mcp/review/src/repofs/git/gitignore/gitignore.ts new file mode 100644 index 0000000..a349eb6 --- /dev/null +++ b/claude/kensai/mcp/review/src/repofs/git/gitignore/gitignore.ts @@ -0,0 +1,55 @@ +import path from "node:path/posix"; + +import { type MatchResult, type Pattern, parsePattern } from "./pattern.ts"; + +export type { MatchResult, Pattern }; +export { parsePattern }; + +export class Matcher { + readonly #patterns: readonly Pattern[]; + readonly #parent: Matcher | null; + + constructor(patterns: Pattern[], parent: Matcher | null = null) { + this.#patterns = patterns; + this.#parent = parent; + } + + match(relPath: string, isDir: boolean): boolean { + return Matcher.#walk(this, relPath, isDir); + } + + static #walk(start: Matcher, relPath: string, isDir: boolean): boolean { + const name = path.basename(relPath); + + for (let cur: Matcher | null = start; cur != null; cur = cur.#parent) { + for (let i = cur.#patterns.length - 1; i >= 0; i--) { + const result = cur.#patterns[i]!.match(relPath, name, isDir); + if (result === "exclude") return true; + if (result === "include") return false; + } + } + + return false; + } + + append(...patterns: Pattern[]): Matcher { + if (patterns.length === 0) return this; + return new Matcher(patterns, this); + } + + withParent(parent: Matcher | null): Matcher | null { + if (this.#patterns.length === 0) return parent; + return new Matcher([...this.#patterns], parent); + } +} + +export function parse(content: string, dir: string): Matcher { + const patterns: Pattern[] = []; + + for (const line of content.split("\n")) { + const p = parsePattern(line, dir); + if (p) patterns.push(p); + } + + return new Matcher(patterns); +} diff --git a/claude/kensai/mcp/review/src/repofs/git/gitignore/pattern.test.ts b/claude/kensai/mcp/review/src/repofs/git/gitignore/pattern.test.ts new file mode 100644 index 0000000..ba85125 --- /dev/null +++ b/claude/kensai/mcp/review/src/repofs/git/gitignore/pattern.test.ts @@ -0,0 +1,257 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { parsePattern } from "./pattern.ts"; + +describe("parsePattern — blank lines", () => { + it.each(["", " ", "\t"])("rejects %j", (line) => { + expect(parsePattern(line, "")).toBeNull(); + }); +}); + +describe("parsePattern — comments", () => { + it("rejects comment", () => { + expect(parsePattern("# this is a comment", "")).toBeNull(); + }); +}); + +describe("parsePattern — escaped hash", () => { + it("matches literal #", () => { + const p = parsePattern("\\#readme", "")!; + expect(p).not.toBeNull(); + expect(p.match("#readme", "#readme", false)).toBe("exclude"); + }); +}); + +describe("parsePattern — trailing spaces", () => { + it("strips unescaped trailing spaces", () => { + const p = parsePattern("*.log ", "")!; + expect(p).not.toBeNull(); + expect(p.match("debug.log", "debug.log", false)).toBe("exclude"); + }); + + it("preserves escaped trailing space", () => { + const p = parsePattern("pattern\\ ", "")!; + expect(p).not.toBeNull(); + expect(p.match("pattern ", "pattern ", false)).toBe("exclude"); + expect(p.match("pattern", "pattern", false)).toBe("no-match"); + }); + + it("interior spaces preserved with escaped trailing space", () => { + const p = parsePattern("pattern \\ ", "")!; + expect(p).not.toBeNull(); + expect(p.match("pattern ", "pattern ", false)).toBe("exclude"); + expect(p.match("pattern ", "pattern ", false)).toBe("no-match"); + }); + + it("even backslash count — trailing space is unescaped and stripped", () => { + const p = parsePattern("pattern\\\\ ", "")!; + expect(p).not.toBeNull(); + expect(p.match("pattern\\", "pattern\\", false)).toBe("exclude"); + expect(p.match("pattern\\ ", "pattern\\ ", false)).toBe("no-match"); + }); + + it("triple backslash — odd count preserves trailing space", () => { + const p = parsePattern("pattern\\\\\\ ", "")!; + expect(p).not.toBeNull(); + expect(p.match("pattern\\ ", "pattern\\ ", false)).toBe("exclude"); + }); +}); + +describe("parsePattern — negation", () => { + it("negated pattern returns include", () => { + const p = parsePattern("!important.log", "")!; + expect(p).not.toBeNull(); + expect(p.match("important.log", "important.log", false)).toBe("include"); + }); + + it("escaped bang is literal", () => { + const p = parsePattern("\\!important", "")!; + expect(p).not.toBeNull(); + expect(p.match("!important", "!important", false)).toBe("exclude"); + }); + + it("negation then empty is invalid", () => { + expect(parsePattern("!/", "")).toBeNull(); + }); +}); + +describe("parsePattern — dirOnly", () => { + it("matches directory", () => { + const p = parsePattern("build/", "")!; + expect(p).not.toBeNull(); + expect(p.match("build", "build", true)).toBe("exclude"); + }); + + it("skips file", () => { + const p = parsePattern("build/", "")!; + expect(p).not.toBeNull(); + expect(p.match("build", "build", false)).toBe("no-match"); + }); + + it("dirOnly path pattern", () => { + const p = parsePattern("build/cache/", "")!; + expect(p).not.toBeNull(); + expect(p.match("build/cache", "cache", true)).toBe("exclude"); + expect(p.match("build/cache", "cache", false)).toBe("no-match"); + }); +}); + +describe("parsePattern — wildcards", () => { + it("star glob", () => { + const p = parsePattern("*.log", "")!; + expect(p).not.toBeNull(); + expect(p.match("output.log", "output.log", false)).toBe("exclude"); + expect(p.match("main.go", "main.go", false)).toBe("no-match"); + }); + + it("question mark", () => { + const p = parsePattern("?.log", "")!; + expect(p).not.toBeNull(); + expect(p.match("a.log", "a.log", false)).toBe("exclude"); + expect(p.match("ab.log", "ab.log", false)).toBe("no-match"); + }); + + it("character class", () => { + const p = parsePattern("*.[oa]", "")!; + expect(p).not.toBeNull(); + expect(p.match("foo.o", "foo.o", false)).toBe("exclude"); + expect(p.match("foo.a", "foo.a", false)).toBe("exclude"); + expect(p.match("foo.c", "foo.c", false)).toBe("no-match"); + }); + + it("negated character class", () => { + const p = parsePattern("[!abc].txt", "")!; + expect(p).not.toBeNull(); + expect(p.match("d.txt", "d.txt", false)).toBe("exclude"); + expect(p.match("a.txt", "a.txt", false)).toBe("no-match"); + }); + + it("star matches dotfiles", () => { + const p = parsePattern("*.log", "")!; + expect(p).not.toBeNull(); + expect(p.match(".secret.log", ".secret.log", false)).toBe("exclude"); + }); + + it("braces are literal, not expanded", () => { + const p = parsePattern("file.{js,ts}", "")!; + expect(p).not.toBeNull(); + expect(p.match("file.js", "file.js", false)).toBe("no-match"); + expect(p.match("file.{js,ts}", "file.{js,ts}", false)).toBe("exclude"); + }); +}); + +describe("parsePattern — exact name", () => { + it("matches exact basename", () => { + const p = parsePattern("node_modules", "")!; + expect(p).not.toBeNull(); + expect(p.match("node_modules", "node_modules", false)).toBe("exclude"); + expect(p.match("vendor", "vendor", false)).toBe("no-match"); + }); +}); + +describe("parsePattern — basename matches any depth", () => { + it("basename pattern matches at any depth", () => { + const p = parsePattern("*.log", "")!; + expect(p).not.toBeNull(); + expect(p.match("debug.log", "debug.log", false)).toBe("exclude"); + expect(p.match("src/debug.log", "debug.log", false)).toBe("exclude"); + expect(p.match("a/b/c/debug.log", "debug.log", false)).toBe("exclude"); + }); +}); + +describe("parsePattern — path patterns", () => { + it("path pattern is anchored", () => { + const p = parsePattern("build/output.log", "")!; + expect(p).not.toBeNull(); + expect(p.match("build/output.log", "output.log", false)).toBe("exclude"); + expect(p.match("src/build/output.log", "output.log", false)).toBe("no-match"); + }); + + it("leading slash anchors to root", () => { + const p = parsePattern("/TODO", "")!; + expect(p).not.toBeNull(); + expect(p.match("TODO", "TODO", false)).toBe("exclude"); + expect(p.match("src/TODO", "TODO", false)).toBe("no-match"); + }); + + it("leading slash with dir", () => { + const p = parsePattern("/TODO", "src")!; + expect(p).not.toBeNull(); + expect(p.match("src/TODO", "TODO", false)).toBe("exclude"); + expect(p.match("TODO", "TODO", false)).toBe("no-match"); + }); +}); + +describe("parsePattern — double star", () => { + it("leading /** with leading slash at root", () => { + const p = parsePattern("/**/foo", "")!; + expect(p).not.toBeNull(); + expect(p.match("foo", "foo", false)).toBe("exclude"); + expect(p.match("a/b/foo", "foo", false)).toBe("exclude"); + expect(p.match("bar", "bar", false)).toBe("no-match"); + }); + + it("leading /** with leading slash and dir", () => { + const p = parsePattern("/**/foo", "src")!; + expect(p).not.toBeNull(); + expect(p.match("src/foo", "foo", false)).toBe("exclude"); + expect(p.match("src/a/b/foo", "foo", false)).toBe("exclude"); + expect(p.match("other/foo", "foo", false)).toBe("no-match"); + }); + + it("leading ** as basename", () => { + const p = parsePattern("**/foo", "")!; + expect(p).not.toBeNull(); + expect(p.match("foo", "foo", false)).toBe("exclude"); + expect(p.match("a/foo", "foo", false)).toBe("exclude"); + expect(p.match("a/b/c/foo", "foo", false)).toBe("exclude"); + }); + + it("leading ** with extension", () => { + const p = parsePattern("**/*.log", "")!; + expect(p).not.toBeNull(); + expect(p.match("output.log", "output.log", false)).toBe("exclude"); + expect(p.match("a/output.log", "output.log", false)).toBe("exclude"); + expect(p.match("a/b/c/output.log", "output.log", false)).toBe("exclude"); + }); + + it("leading ** with dir", () => { + const p = parsePattern("**/foo", "src")!; + expect(p).not.toBeNull(); + expect(p.match("src/foo", "foo", false)).toBe("exclude"); + expect(p.match("src/a/b/foo", "foo", false)).toBe("exclude"); + }); + + it("leading ** path fallback", () => { + const p = parsePattern("**/test/*.go", "")!; + expect(p).not.toBeNull(); + expect(p.match("test/foo.go", "foo.go", false)).toBe("exclude"); + expect(p.match("a/test/foo.go", "foo.go", false)).toBe("exclude"); + expect(p.match("a/b/test/foo.go", "foo.go", false)).toBe("exclude"); + }); + + it("trailing **", () => { + const p = parsePattern("doc/**", "")!; + expect(p).not.toBeNull(); + expect(p.match("doc/a.txt", "a.txt", false)).toBe("exclude"); + expect(p.match("doc/x/y/a.txt", "a.txt", false)).toBe("exclude"); + }); + + it("middle **", () => { + const p = parsePattern("a/**/b", "")!; + expect(p).not.toBeNull(); + expect(p.match("a/b", "b", false)).toBe("exclude"); + expect(p.match("a/x/b", "b", false)).toBe("exclude"); + expect(p.match("a/x/y/b", "b", false)).toBe("exclude"); + }); +}); + +describe("parsePattern — dir scoping", () => { + it("path pattern scoped to dir", () => { + const p = parsePattern("build/output.log", "src/pkg")!; + expect(p).not.toBeNull(); + expect(p.match("src/pkg/build/output.log", "output.log", false)).toBe("exclude"); + expect(p.match("build/output.log", "output.log", false)).toBe("no-match"); + expect(p.match("other/build/output.log", "output.log", false)).toBe("no-match"); + }); +}); diff --git a/claude/kensai/mcp/review/src/repofs/git/gitignore/pattern.ts b/claude/kensai/mcp/review/src/repofs/git/gitignore/pattern.ts new file mode 100644 index 0000000..bbc2e00 --- /dev/null +++ b/claude/kensai/mcp/review/src/repofs/git/gitignore/pattern.ts @@ -0,0 +1,143 @@ +import picomatch from "picomatch"; + +export type MatchResult = "no-match" | "exclude" | "include"; + +export interface Pattern { + match(relPath: string, name: string, isDir: boolean): MatchResult; +} + +export function parsePattern(line: string, dir: string): Pattern | null { + const parsed = parseLine(line); + if (!parsed) return null; + + const { negated, dirOnly, isPath, cleanLine } = parsed; + const matcher = compileMatcher(isPath, cleanLine, dir); + if (!matcher) return null; + + return { + match(relPath: string, name: string, isDir: boolean): MatchResult { + if (dirOnly && !isDir) return "no-match"; + + const target = isPath ? relPath : name; + if (matcher(target)) return negated ? "include" : "exclude"; + + return "no-match"; + }, + }; +} + +interface ParsedLine { + negated: boolean; + dirOnly: boolean; + isPath: boolean; + cleanLine: string; +} + +function parseLine(line: string): ParsedLine | null { + line = trimTrailingWhitespace(line); + + if (line === "" || line.startsWith("#")) return null; + + const flags = parseFlags(line); + if (flags.cleanLine === "") return null; + + return flags; +} + +function parseFlags(line: string): ParsedLine { + let negated = false; + let dirOnly = false; + + if (line.startsWith("!")) { + negated = true; + line = line.slice(1); + } + + if (line.startsWith("\\#") || line.startsWith("\\!")) { + line = line.slice(1); + } + + if (line.endsWith("/")) { + dirOnly = true; + while (line.endsWith("/")) line = line.slice(0, -1); + } + + const hasLeadingSlash = line.startsWith("/"); + if (hasLeadingSlash) { + line = line.slice(1); + } + + if (line.startsWith("**/")) { + const rest = line.slice(3); + if (!rest.includes("/") && !hasLeadingSlash) { + return { negated, dirOnly, isPath: false, cleanLine: rest }; + } + } + + const isPath = hasLeadingSlash || line.includes("/"); + + return { negated, dirOnly, isPath, cleanLine: line }; +} + +/** + * Strips unescaped trailing spaces and tabs. + * A trailing `\ ` (backslash-space) preserves one space. Even + * backslash count (`\\ `) means the backslash itself is escaped and + * the trailing space is unescaped — it gets stripped. + */ +function trimTrailingWhitespace(line: string): string { + let end = line.length; + while (end > 0 && (line[end - 1] === " " || line[end - 1] === "\t")) end--; + + if (end === line.length) return line; + + const trimmed = line.slice(0, end); + + let backslashes = 0; + for (let i = trimmed.length - 1; i >= 0 && trimmed[i] === "\\"; i--) { + backslashes++; + } + + if (backslashes % 2 === 1) return trimmed + " "; + + return trimmed; +} + +// picomatch uses [^abc] for negated character classes; gitignore uses [!abc]. +function normalizeCharClasses(pattern: string): string { + return pattern.replaceAll("[!", "[^"); +} + +// Gitignore globs match dotfiles and have no brace expansion or extglob syntax. +// windows:false keeps backslashes as escape characters on all platforms — picomatch's +// Windows auto-detection would convert them to path separators, breaking \-escapes. +// Patterns and matched paths are always POSIX here. +const PICOMATCH_OPTIONS: picomatch.PicomatchOptions = { + dot: true, + nobrace: true, + noextglob: true, + windows: false, +}; + +function compileMatcher(isPath: boolean, line: string, dir: string): picomatch.Matcher | null { + line = normalizeCharClasses(line); + + if (!isPath) { + try { + return picomatch(line, PICOMATCH_OPTIONS); + } catch { + return null; + } + } + + if (dir === ".") dir = ""; + + let full = line; + if (dir) full = `${dir}/${line}`; + + try { + return picomatch(full, PICOMATCH_OPTIONS); + } catch { + return null; + } +} diff --git a/claude/kensai/mcp/review/src/repofs/lineiter/CLAUDE.md b/claude/kensai/mcp/review/src/repofs/lineiter/CLAUDE.md index 62875f5..a68681b 100644 --- a/claude/kensai/mcp/review/src/repofs/lineiter/CLAUDE.md +++ b/claude/kensai/mcp/review/src/repofs/lineiter/CLAUDE.md @@ -2,7 +2,7 @@ Streaming pull-style line iterator with pluggable binary detection and per-line byte cap. Ported from Go `fstoolset/lineiter`. Consumes an `AsyncIterable` — no internal buffering. -Foundation for `fs-file-read`. +Consumed by `countFileLines` (pathindex) for on-demand line metrics and binary detection. ## API diff --git a/claude/kensai/mcp/review/src/repofs/pathindex/CLAUDE.md b/claude/kensai/mcp/review/src/repofs/pathindex/CLAUDE.md index d3bf06d..0fa2316 100644 --- a/claude/kensai/mcp/review/src/repofs/pathindex/CLAUDE.md +++ b/claude/kensai/mcp/review/src/repofs/pathindex/CLAUDE.md @@ -1,25 +1,27 @@ # pathindex -Tree-structured file index with fuzzy and glob search. Built once at session start, immutable after construction. +Tree-structured file index with fuzzy and glob search. Built once at session start; afterwards entries are only +enriched — lazy size fills, `add()` reconciliation — never removed or restructured. Replaces per-call filesystem operations for file lookup, directory listing, and path suggestions. ## API -| Method | Purpose | -| ------------------------------ | -------------------------------------------------------- | -| `PathIndex.new(root, signal?)` | Async build — parallel walk + stat, symlink detection | -| `PathIndex.from(root, paths)` | Sync build from path strings (trailing `/` = directory) | -| `get(path)` | O(1) entry lookup by relative path | -| `dir(path)` | O(1) directory node lookup | -| `globSearch(opts?)` | Filter paths by include/exclude globs | -| `fuzzySearch(pattern, opts?)` | Multi-word fuzzy search with optional glob pre-filtering | -| `validateGlobs(patterns)` | Check glob patterns are compilable | +| Method | Purpose | +| ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `PathIndex.new(root, signal?)` | Async build — gitignore-aware stat-free parallel walk, symlink resolution. Works on any directory tree, git or not | +| `PathIndex.from(root, paths)` | Sync build from path strings (trailing `/` = directory) | +| `add(relPaths)` | Insert paths the walk pruned (e.g. changed tracked-but-ignored files, reconciled by the review layer) — lstat-classified, size filled, already-indexed and nonexistent paths skipped | +| `get(path)` | O(1) entry lookup by relative path | +| `dir(path)` | O(1) directory node lookup | +| `globSearch(opts?)` | Filter paths by include/exclude globs | +| `fuzzySearch(pattern, opts?)` | Multi-word fuzzy search with optional glob pre-filtering | +| `validateGlobs(patterns)` | Check glob patterns are compilable | ## Entry Types Discriminated union on `type`: -- **`file`** — `name`, `size` (bytes), `lineCount` (0 for binary), `maxLineLen` (longest line in bytes, 0 for binary/empty), `isBinary` (null byte in leading 512 bytes) +- **`file`** — `name`, `size` (bytes; null at build time — lazily filled by `RepoFs.readFile` or manifest stat), `isBinary` (extension-based hint; content probe runs at read/scan time). Line metrics are not stored — `countFileLines` computes them on demand - **`dir`** — `name`, `children: IndexEntry[]` - **`symlink`** — `name`, `target` (raw readlink), `targetType`, `size` (target's size if file) @@ -46,13 +48,20 @@ Powered by `picomatch`. Pattern without `/` uses `matchBase` (matches basename a ## Walker - Async recursive with parallel child dispatch (`Promise.all` per directory) -- Symlinks indexed with target info but not followed for recursion -- Per file: `stat` (size) and `LineIter` body-less scan (line count + binary detection) run in parallel -- Binary detection via null byte probe on first 512 bytes — binary files get `lineCount: 0, isBinary: true` +- Gitignore-aware: each directory's `.gitignore` is parsed and chained onto the parent matcher; + `.git/info/exclude` seeds the root chain +- Ignored files are omitted; ignored directories are pruned without descent — `!` re-inclusion + inside a pruned directory is impossible, matching git. Tracked-but-ignored files are absent + from the index (still readable via the `RepoFs.readFile` fallback) +- Symlinks indexed with target info (readlink + stat) but not followed for recursion; ignore rules match them as files +- Files are never statted at build — entries are created from the dirent alone. Size is lazily + filled (`RepoFs.readFile`, manifest stat); line metrics and content-based binary detection are + computed on demand by `countFileLines`, never stored on the entry - Paths stored as forward-slash POSIX relative to root ## Dependencies - `fuzzysort` — SublimeText-style fuzzy scoring - `picomatch` — glob compilation and matching -- `lineiter` — body-less line counting and binary detection during walk +- `../git/gitignore` — walk-time ignore filtering +- `lineiter` — on-demand line counting and binary detection (`countFileLines`) diff --git a/claude/kensai/mcp/review/src/repofs/pathindex/pathindex.test.ts b/claude/kensai/mcp/review/src/repofs/pathindex/pathindex.test.ts index c8a96a8..635b6cd 100644 --- a/claude/kensai/mcp/review/src/repofs/pathindex/pathindex.test.ts +++ b/claude/kensai/mcp/review/src/repofs/pathindex/pathindex.test.ts @@ -310,26 +310,14 @@ describe("PathIndex.new", () => { expect(ix.paths).toEqual(["a/b/x.go", "a/y.go", "z.go"]); }); - it("tracks file sizes", async () => { + it("does not stat files at build — size is null until lazily filled", async () => { await writeFile(join(tempDir, "small.txt"), "hi"); - await writeFile(join(tempDir, "big.txt"), "a".repeat(1000)); const ix = await PathIndex.new(tempDir); - const small = ix.get("small.txt") as IndexEntryFile; - const big = ix.get("big.txt") as IndexEntryFile; - expect(small.size).toBe(2); - expect(big.size).toBe(1000); + const entry = ix.get("small.txt") as IndexEntryFile; + expect(entry.size).toBeNull(); expect(ix.get("nonexistent")).toBeUndefined(); }); - it("index is stat-only — lineCount and maxLineLen are zero", async () => { - await writeFile(join(tempDir, "three.txt"), "one\ntwo\nthree\n"); - const ix = await PathIndex.new(tempDir); - const entry = ix.get("three.txt") as IndexEntryFile; - expect(entry.lineCount).toBe(0); - expect(entry.maxLineLen).toBe(0); - expect(entry.isBinary).toBe(false); - }); - it("detects binary by extension", async () => { await writeFile(join(tempDir, "image.png"), Buffer.from([0x89, 0x50, 0x4e, 0x47])); await writeFile(join(tempDir, "code.ts"), "const x = 1;\n"); @@ -342,8 +330,7 @@ describe("PathIndex.new", () => { await writeFile(join(tempDir, "empty.txt"), ""); const ix = await PathIndex.new(tempDir); const entry = ix.get("empty.txt") as IndexEntryFile; - expect(entry.lineCount).toBe(0); - expect(entry.maxLineLen).toBe(0); + expect(entry.size).toBeNull(); expect(entry.isBinary).toBe(false); }); @@ -391,6 +378,100 @@ describe("PathIndex.new", () => { }); }); +describe("PathIndex.new — gitignore", () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "pathindex-gi-")); + }); + + afterEach(async () => { + await rm(tempDir, { recursive: true, maxRetries: 3 }); + }); + + it("filters files matched by root .gitignore", async () => { + await createFiles(tempDir, ["main.go", "debug.log"]); + await writeFile(join(tempDir, ".gitignore"), "*.log\n"); + const ix = await PathIndex.new(tempDir); + expect(ix.paths).toEqual([".gitignore", "main.go"]); + }); + + it("prunes ignored directories without descent", async () => { + await createFiles(tempDir, ["src/main.go", "node_modules/lib/index.js", "node_modules/lib/deep/x.js"]); + await writeFile(join(tempDir, ".gitignore"), "node_modules/\n"); + const ix = await PathIndex.new(tempDir); + expect(ix.paths.filter((p) => p.startsWith("node_modules"))).toEqual([]); + expect(ix.dir("node_modules")).toBeUndefined(); + expect(ix.paths).toContain("src/main.go"); + }); + + it("nested .gitignore applies only under its directory", async () => { + await createFiles(tempDir, ["a.tmp", "sub/b.tmp", "sub/keep.go"]); + await writeFile(join(tempDir, "sub/.gitignore"), "*.tmp\n"); + const ix = await PathIndex.new(tempDir); + expect(ix.paths).toContain("a.tmp"); + expect(ix.paths).not.toContain("sub/b.tmp"); + expect(ix.paths).toContain("sub/keep.go"); + }); + + it("negation re-includes a previously excluded file", async () => { + await createFiles(tempDir, ["debug.log", "keep.log"]); + await writeFile(join(tempDir, ".gitignore"), "*.log\n!keep.log\n"); + const ix = await PathIndex.new(tempDir); + expect(ix.paths).not.toContain("debug.log"); + expect(ix.paths).toContain("keep.log"); + }); + + it("child .gitignore overrides parent patterns", async () => { + await createFiles(tempDir, ["top.log", "sub/inner.log"]); + await writeFile(join(tempDir, ".gitignore"), "*.log\n"); + await writeFile(join(tempDir, "sub/.gitignore"), "!inner.log\n"); + const ix = await PathIndex.new(tempDir); + expect(ix.paths).not.toContain("top.log"); + expect(ix.paths).toContain("sub/inner.log"); + }); + + it("cannot re-include inside a pruned directory", async () => { + await createFiles(tempDir, ["build/app.js", "main.go"]); + await writeFile(join(tempDir, ".gitignore"), "build/\n!build/app.js\n"); + const ix = await PathIndex.new(tempDir); + expect(ix.paths.filter((p) => p.startsWith("build"))).toEqual([]); + }); + + it("anchored pattern matches only at its level", async () => { + await createFiles(tempDir, ["top.log", "sub/top.log"]); + await writeFile(join(tempDir, ".gitignore"), "/top.log\n"); + const ix = await PathIndex.new(tempDir); + expect(ix.paths).not.toContain("top.log"); + expect(ix.paths).toContain("sub/top.log"); + }); + + it("dir-only pattern does not match a file with the same name", async () => { + await createFiles(tempDir, ["build", "src/build/out.js"]); + await writeFile(join(tempDir, ".gitignore"), "build/\n"); + const ix = await PathIndex.new(tempDir); + expect(ix.paths).toContain("build"); + expect(ix.paths.filter((p) => p.startsWith("src/build"))).toEqual([]); + }); + + it("respects .git/info/exclude", async () => { + await createFiles(tempDir, ["main.go", "scratch.txt", ".git/info/exclude"]); + await writeFile(join(tempDir, ".git/info/exclude"), "scratch.txt\n"); + const ix = await PathIndex.new(tempDir); + expect(ix.paths).not.toContain("scratch.txt"); + expect(ix.paths).toContain("main.go"); + }); + + it("ignored symlinks are skipped", async () => { + await writeFile(join(tempDir, "real.txt"), "content"); + await symlink(join(tempDir, "real.txt"), join(tempDir, "link.txt")); + await writeFile(join(tempDir, ".gitignore"), "link.txt\n"); + const ix = await PathIndex.new(tempDir); + expect(ix.paths).not.toContain("link.txt"); + expect(ix.paths).toContain("real.txt"); + }); +}); + describe("countFileLines", () => { let tempDir: string; @@ -428,3 +509,77 @@ describe("countFileLines", () => { expect(result.lineCount).toBe(2); }); }); + +describe("PathIndex.add", () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "pathindex-add-")); + }); + + afterEach(async () => { + await rm(tempDir, { recursive: true, maxRetries: 3 }); + }); + + it("inserts files pruned by the walk, with size from lstat", async () => { + await createFiles(tempDir, ["main.go", "build/out.js"]); + await writeFile(join(tempDir, ".gitignore"), "build/\n"); + const ix = await PathIndex.new(tempDir); + expect(ix.get("build/out.js")).toBeUndefined(); + + await ix.add(["build/out.js"]); + const entry = ix.get("build/out.js") as IndexEntryFile; + expect(entry.type).toBe("file"); + expect(entry.size).toBe(1); + expect(ix.dir("build")).toBeDefined(); + }); + + it("keeps paths sorted after insertion", async () => { + await createFiles(tempDir, ["m.go", "z.go", "a.log"]); + await writeFile(join(tempDir, ".gitignore"), "*.log\n"); + const ix = await PathIndex.new(tempDir); + + await ix.add(["a.log"]); + expect(ix.paths).toEqual([".gitignore", "a.log", "m.go", "z.go"]); + }); + + it("skips already-indexed paths", async () => { + await createFiles(tempDir, ["main.go"]); + const ix = await PathIndex.new(tempDir); + + await ix.add(["main.go"]); + expect(ix.paths.filter((p) => p === "main.go")).toHaveLength(1); + }); + + it("skips nonexistent paths and directories", async () => { + await createFiles(tempDir, ["sub/file.go"]); + const ix = await PathIndex.new(tempDir); + const before = [...ix.paths]; + + await ix.add(["missing.go", "sub"]); + expect(ix.paths).toEqual(before); + }); + + it("classifies symlinks via lstat", async () => { + await writeFile(join(tempDir, "real.txt"), "content"); + await symlink(join(tempDir, "real.txt"), join(tempDir, "link.txt")); + await writeFile(join(tempDir, ".gitignore"), "link.txt\n"); + const ix = await PathIndex.new(tempDir); + expect(ix.get("link.txt")).toBeUndefined(); + + await ix.add(["link.txt"]); + const entry = ix.get("link.txt") as IndexEntrySymlink; + expect(entry.type).toBe("symlink"); + expect(entry.targetType).toBe("file"); + expect(entry.size).toBe(7); + }); + + it("sets binary hint from extension", async () => { + await createFiles(tempDir, ["asset.png"]); + await writeFile(join(tempDir, ".gitignore"), "*.png\n"); + const ix = await PathIndex.new(tempDir); + + await ix.add(["asset.png"]); + expect((ix.get("asset.png") as IndexEntryFile).isBinary).toBe(true); + }); +}); diff --git a/claude/kensai/mcp/review/src/repofs/pathindex/pathindex.ts b/claude/kensai/mcp/review/src/repofs/pathindex/pathindex.ts index 451ec51..fd56593 100644 --- a/claude/kensai/mcp/review/src/repofs/pathindex/pathindex.ts +++ b/claude/kensai/mcp/review/src/repofs/pathindex/pathindex.ts @@ -6,11 +6,15 @@ import path from "node:path/posix"; import fuzzysort from "fuzzysort"; import picomatch from "picomatch"; +import type { Matcher } from "../git/gitignore/gitignore.ts"; +import { parse as parseGitignore } from "../git/gitignore/gitignore.ts"; import { ErrBinaryContent, LineIter } from "../lineiter/lineiter.ts"; /** Directory names skipped during filesystem walk. Never descended into. */ export const WALK_EXCLUDE_DIRS: ReadonlySet = new Set([".git"]); +const GITIGNORE_FILE = ".gitignore"; + /** Extensions treated as binary without opening the file. Stat-only — no line counting, no fd consumed. */ const BINARY_EXTENSIONS: ReadonlySet = new Set([ // Images @@ -147,11 +151,8 @@ const BINARY_EXTENSIONS: ReadonlySet = new Set([ export type IndexEntryFile = { type: "file"; name: string; - size: number; - /** Total number of lines. 0 for binary files. */ - lineCount: number; - /** Byte length of the longest line (excluding `\n`). 0 for binary/empty files. */ - maxLineLen: number; + /** Byte size. Null until lazily filled — by a full read ({@link RepoFs.readFile}) or a manifest stat. */ + size: number | null; /** Extension-based hint. Content-based detection (null byte probe) runs at read/scan time. */ isBinary: boolean; }; @@ -202,54 +203,36 @@ export class PathIndex { const ix = new PathIndex(rootPath); for (let p of paths.toSorted()) { - const segments = p.split("/"); - let file: string | undefined; - if (p.endsWith("/")) { - segments.pop(); p = p.slice(0, -1); - } else { - file = segments.pop(); - } - - let parent = ix.root; - for (const [i, name] of segments.entries()) { - if (!name) continue; - const relPath = segments.slice(0, i + 1).join("/"); - let dir = ix.#entries.get(relPath) as IndexEntryDir | undefined; - - if (!dir) { - dir = { type: "dir", name, children: [] }; - parent.children.push(dir); - ix.#entries.set(relPath, dir); - } - - parent = dir; - } - - if (file) { - const entry: IndexEntryFile = { - type: "file", - name: file, - size: 0, - lineCount: 0, - maxLineLen: 0, - isBinary: false, - }; - parent.children.push(entry); - ix.#entries.set(p, entry); + ix.#ensureParents(p.split("/")); + ix.paths.push(p); + continue; } - ix.paths.push(p); + ix.#insert(p, { + type: "file", + name: path.basename(p), + size: null, + isBinary: false, + }); } return ix; } - /** Walk the filesystem tree rooted at `rootPath` and return a tree-structured index. */ + /** + * Walk the filesystem tree rooted at `rootPath` and return a tree-structured index. + * + * Gitignore-aware: `.gitignore` files are loaded per directory and chained hierarchically, + * `.git/info/exclude` seeds the root chain. Ignored files are omitted; ignored directories + * are pruned without descent (re-inclusion via `!` inside a pruned directory is impossible, + * matching git). + */ static async new(rootPath: string, signal?: AbortSignal): Promise { const ix = new PathIndex(rootPath); - ix.root.children = (await ix.#walkDir("", signal)).children; + const exclude = await readIgnoreFile(nativePath.join(ix.absRoot, ".git", "info", "exclude"), "", null); + ix.root.children = (await ix.#walkDir("", exclude, signal)).children; ix.paths.sort(); return ix; } @@ -294,20 +277,101 @@ export class PathIndex { return entry?.type === "dir" ? entry : undefined; } - /** Parallel recursive walk — all children (subdirs + file stats) dispatched via Promise.all per directory. */ - async #walkDir(dir: string, signal?: AbortSignal): Promise { + /** + * Inserts paths the walk did not produce — e.g. tracked-but-ignored files from `git ls-files`. + * Each path is lstat-classified: regular files and symlinks are inserted (with size filled + * from the stat), anything else — missing, directories — is skipped, as are paths already + * in the index. + */ + async add(relPaths: readonly string[]): Promise { + const additions = await Promise.all( + relPaths.map(async (p): Promise => { + if (this.#entries.has(p)) return null; + + const absPath = nativePath.join(this.absRoot, p); + const stat = await fs.lstat(absPath).catch(() => null); + if (!stat) return null; + + if (stat.isSymbolicLink()) { + const entry = await this.#resolveSymlink(absPath, path.basename(p)); + return entry ? [p, entry] : null; + } + + if (!stat.isFile()) return null; + + return [ + p, + { + type: "file", + name: path.basename(p), + size: stat.size, + isBinary: BINARY_EXTENSIONS.has(path.extname(p).toLowerCase()), + }, + ]; + }), + ); + + let inserted = false; + for (const a of additions) { + if (!a) continue; + this.#insert(a[0], a[1]); + inserted = true; + } + + if (inserted) this.paths.sort(); + } + + /** Creates missing directory nodes along the segment chain. Returns the innermost directory. */ + #ensureParents(segments: string[]): IndexEntryDir { + let parent = this.root; + + for (const [i, name] of segments.entries()) { + if (!name) continue; + const relPath = segments.slice(0, i + 1).join("/"); + let dir = this.#entries.get(relPath) as IndexEntryDir | undefined; + + if (!dir) { + dir = { type: "dir", name, children: [] }; + parent.children.push(dir); + this.#entries.set(relPath, dir); + } + + parent = dir; + } + + return parent; + } + + /** Inserts a leaf entry at relPath, creating parent directories as needed. */ + #insert(relPath: string, entry: IndexEntryFile | IndexEntrySymlink): void { + const segments = relPath.split("/"); + segments.pop(); + this.#ensureParents(segments).children.push(entry); + this.#entries.set(relPath, entry); + this.paths.push(relPath); + } + + /** Parallel recursive walk — subdirs and symlink resolution dispatched via Promise.all per directory. File entries are created synchronously without stat. */ + async #walkDir(dir: string, ignore: Matcher | null, signal?: AbortSignal): Promise { signal?.throwIfAborted(); const dirEntries = await fs.readdir(nativePath.resolve(this.absRoot, dir), { withFileTypes: true }); const dirEntry: IndexEntryDir = { type: "dir", name: path.basename(dir), children: [] }; const work: Promise[] = []; + if (dirEntries.some((d) => d.name === GITIGNORE_FILE && d.isFile())) { + ignore = await readIgnoreFile(nativePath.join(this.absRoot, dir, GITIGNORE_FILE), dir, ignore); + } + for (const dirent of dirEntries) { if (WALK_EXCLUDE_DIRS.has(dirent.name)) continue; const relPath = path.join(dir, dirent.name); const absPath = nativePath.join(this.absRoot, relPath); + // Symlinks match as files (isDir false), like git. + if (ignore?.match(relPath, dirent.isDirectory())) continue; + if (dirent.isSymbolicLink()) { work.push( this.#resolveSymlink(absPath, dirent.name).then((entry) => { @@ -323,7 +387,7 @@ export class PathIndex { if (dirent.isDirectory()) { work.push( - this.#walkDir(relPath, signal).then((subtree) => { + this.#walkDir(relPath, ignore, signal).then((subtree) => { dirEntry.children.push(subtree); this.#entries.set(relPath, subtree); }), @@ -331,21 +395,15 @@ export class PathIndex { continue; } - work.push( - fs.stat(absPath).then((s) => { - const entry: IndexEntryFile = { - type: "file", - name: dirent.name, - size: s.size, - lineCount: 0, - maxLineLen: 0, - isBinary: BINARY_EXTENSIONS.has(path.extname(dirent.name).toLowerCase()), - }; - dirEntry.children.push(entry); - this.paths.push(relPath); - this.#entries.set(relPath, entry); - }), - ); + const entry: IndexEntryFile = { + type: "file", + name: dirent.name, + size: null, + isBinary: BINARY_EXTENSIONS.has(path.extname(dirent.name).toLowerCase()), + }; + dirEntry.children.push(entry); + this.paths.push(relPath); + this.#entries.set(relPath, entry); } await Promise.all(work); @@ -376,6 +434,13 @@ export class PathIndex { } } +/** Parse an ignore file into a Matcher chained onto `parent`. Absent or unreadable file returns `parent` as-is. */ +async function readIgnoreFile(absPath: string, dir: string, parent: Matcher | null): Promise { + const content = await fs.readFile(absPath, "utf8").catch(() => null); + if (content == null) return parent; + return parseGitignore(content, dir).withParent(parent); +} + /** Count lines, detect binary, and measure max line length for a single file. */ export async function countFileLines( absPath: string, diff --git a/claude/kensai/mcp/review/src/repofs/repofs.test.ts b/claude/kensai/mcp/review/src/repofs/repofs.test.ts index 150ea93..a7d199e 100644 --- a/claude/kensai/mcp/review/src/repofs/repofs.test.ts +++ b/claude/kensai/mcp/review/src/repofs/repofs.test.ts @@ -172,6 +172,14 @@ describe("readFile", () => { } }); + it("lazily fills index entry size", async () => { + const entry = rfs.index.get("src/main.ts"); + expect(entry?.type).toBe("file"); + expect((entry as { size: number | null }).size).toBeNull(); + const buf = await rfs.readFile("src/main.ts"); + expect((entry as { size: number | null }).size).toBe(buf.length); + }); + it("records read paths for flush", async () => { rfs.flushReadPaths(); await rfs.readFile("README.md"); @@ -228,3 +236,53 @@ describe("flushReadPaths", () => { expect(rfs.flushReadPaths()).toEqual([]); }); }); + +describe("gitignore filtering and read fallback", () => { + let dir: string; + let gfs: RepoFs; + + beforeAll(async () => { + dir = await mkdtemp(join(tmpdir(), "repofs-gitview-")); + await writeFile(join(dir, "tracked.txt"), "a"); + await writeFile(join(dir, "tracked-ignored.log"), "b"); + await git(["init"], dir); + await git(["-c", "user.name=test", "-c", "user.email=test@test.com", "add", "."], dir); + await git(["-c", "user.name=test", "-c", "user.email=test@test.com", "commit", "-m", "init"], dir); + + await writeFile(join(dir, ".gitignore"), "*.log\nignored-link\n"); + await writeFile(join(dir, "untracked.txt"), "c"); + await writeFile(join(dir, "untracked-ignored.log"), "d"); + await symlink("/etc/hosts", join(dir, "ignored-link")); + + gfs = await RepoFs.open(dir); + }); + + afterAll(async () => { + await rm(dir, { recursive: true, force: true, maxRetries: 3 }); + }); + + it("prunes tracked-but-ignored files from the index, still readable via fallback", async () => { + expect(gfs.fileExists("tracked-ignored.log")).toBe(false); + const buf = await gfs.readFile("tracked-ignored.log"); + expect(buf.toString("utf-8")).toBe("b"); + }); + + it("excludes ignored untracked files", () => { + expect(gfs.fileExists("untracked-ignored.log")).toBe(false); + }); + + it("includes untracked non-ignored files", () => { + expect(gfs.fileExists("untracked.txt")).toBe(true); + expect(gfs.fileExists(".gitignore")).toBe(true); + }); + + it("readFile fallback rejects unindexed symlinks", async () => { + expect(gfs.index.get("ignored-link")).toBeUndefined(); + await expect(gfs.readFile("ignored-link")).rejects.toThrow("file not found"); + }); + + it("readFile fallback still reads ignored regular files", async () => { + const buf = await gfs.readFile("untracked-ignored.log"); + expect(buf.toString("utf-8")).toBe("d"); + }); +}); diff --git a/claude/kensai/mcp/review/src/repofs/repofs.ts b/claude/kensai/mcp/review/src/repofs/repofs.ts index de87d10..31cd56d 100644 --- a/claude/kensai/mcp/review/src/repofs/repofs.ts +++ b/claude/kensai/mcp/review/src/repofs/repofs.ts @@ -31,7 +31,7 @@ export class RepoFs { this.#git = git; } - /** Builds the file index and validates git in parallel. */ + /** Builds the file index (gitignore-aware walk) and validates git in parallel. */ static async open(root: string, signal?: AbortSignal): Promise { const absRoot = path.resolve(root); @@ -96,14 +96,18 @@ export class RepoFs { if (firstSegment && WALK_EXCLUDE_DIRS.has(firstSegment)) { throw new RepoFsError(`file not found: ${rel}`); } - const stat = await fsp.stat(absPath).catch(() => null); + // lstat, not stat — a symlink outside the index must not be readable through + // the fallback, or it would bypass path containment. + const stat = await fsp.lstat(absPath).catch(() => null); if (!stat?.isFile()) { throw new RepoFsError(`file not found: ${rel}`); } } this.#pendingReadPaths.push(rel); - return fsp.readFile(absPath); + const buf = await fsp.readFile(absPath); + if (entry) entry.size = buf.length; + return buf; } /** Fuzzy search via {@link PathIndex.fuzzySearch}. */ diff --git a/claude/kensai/mcp/review/src/session/CLAUDE.md b/claude/kensai/mcp/review/src/session/CLAUDE.md index 9ad8380..0ad2dd6 100644 --- a/claude/kensai/mcp/review/src/session/CLAUDE.md +++ b/claude/kensai/mcp/review/src/session/CLAUDE.md @@ -9,28 +9,28 @@ Stateful domain logic for a review session. `Session` (fat factory + data holder Fat `start()` factory — validates git, builds PathIndex, collects diffs and metadata in parallel. All data is lossless; tools perform lossy transformations (collapsing, capping, formatting). -| Export | Purpose | -| --------------------------- | ------------------------------------------------------------------------- | -| `Session.start(root, mode)` | Async factory — returns fully populated session | -| `.id` | 40-char SHA1 hex, unique per session | -| `.root` | Absolute repo path | -| `.mode` | `"committed" \| "uncommitted" \| "all"` | -| `.phase` | Current `SessionPhase` — starts at `GROUNDING` | -| `.startedAt` | Construction timestamp | -| `.rfs` | `RepoFs` instance | -| `.commit` | `GitLogEntry \| null` — HEAD commit; null for uncommitted mode | -| `.changedFiles` | All changed files with status and +/- stats | -| `.manifest` | Per-file shape metrics (bytes, lines, maxLineLen, binary) from PathIndex | -| `.diffs` | Per-file unified diffs (3-line context) for reviewable files | -| `.instructions` | Discovered instruction files (CLAUDE.md, AGENTS.md) from directory chains | -| `.findings` | `FindingsStorage` instance | -| `.grounding` | `GroundingStorage` instance | -| `.advance(to)` | Transition to the given phase — throws `SessionError` if invalid | -| `SessionPhase` | `"GROUNDING" \| "SURFACING" \| "PROVING" \| "FILING" \| "COMPLETE"` | -| `ReviewMode` | `"committed" \| "uncommitted" \| "all"` | -| `FileDiff` | `{ path, content }` | -| `ManifestEntry` | `{ path, bytes, lines, maxLineLen, binary }` | -| `SessionError` | Error class for session failures | +| Export | Purpose | +| --------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Session.start(root, mode)` | Async factory — returns fully populated session | +| `.id` | 40-char SHA1 hex, unique per session | +| `.root` | Absolute repo path | +| `.mode` | `"committed" \| "uncommitted" \| "all"` | +| `.phase` | Current `SessionPhase` — starts at `GROUNDING` | +| `.startedAt` | Construction timestamp | +| `.rfs` | `RepoFs` instance | +| `.commit` | `GitLogEntry \| null` — HEAD commit; null for uncommitted mode | +| `.changedFiles` | All changed files with status and +/- stats | +| `.manifest` | Per-file shape metrics (bytes, lines, maxLineLen, binary) — stats changed files, filling lazy index sizes; reconciles changed files missing from the gitignore-filtered index (tracked-but-ignored) via `index.add()` | +| `.diffs` | Per-file unified diffs (3-line context) for reviewable files | +| `.instructions` | Discovered instruction files (CLAUDE.md, AGENTS.md) from directory chains | +| `.findings` | `FindingsStorage` instance | +| `.grounding` | `GroundingStorage` instance | +| `.advance(to)` | Transition to the given phase — throws `SessionError` if invalid | +| `SessionPhase` | `"GROUNDING" \| "SURFACING" \| "PROVING" \| "FILING" \| "COMPLETE"` | +| `ReviewMode` | `"committed" \| "uncommitted" \| "all"` | +| `FileDiff` | `{ path, content }` | +| `ManifestEntry` | `{ path, bytes, lines, maxLineLen, binary }` | +| `SessionError` | Error class for session failures | ### Phase state machine @@ -146,6 +146,8 @@ Resolved at `Session.start()` in parallel with diff fetching. Delivered as block ## Dependencies - `node:crypto` (Session ID generation) +- `node:fs/promises` (manifest stat) - `../repofs/repofs.ts` (RepoFs) - `../repofs/git/git.ts` (GitFileStat, GitLogEntry) +- `../repofs/pathindex/pathindex.ts` (countFileLines) - `./instructions.ts` (resolveInstructions) diff --git a/claude/kensai/mcp/review/src/session/session.test.ts b/claude/kensai/mcp/review/src/session/session.test.ts index ce55191..0442f22 100644 --- a/claude/kensai/mcp/review/src/session/session.test.ts +++ b/claude/kensai/mcp/review/src/session/session.test.ts @@ -176,6 +176,14 @@ describe("Session", () => { expect(hello!.binary).toBe(false); }); + it("manifest stat fills lazy index sizes", async () => { + const session = await Session.start(repoDir, "committed"); + const hello = session.manifest.find((m) => m.path === "hello.txt"); + const entry = session.rfs.index.get("hello.txt"); + expect(entry?.type).toBe("file"); + expect((entry as { size: number | null }).size).toBe(hello!.bytes); + }); + it("creates fresh storages", async () => { const session = await Session.start(repoDir, "committed"); expect(session.findings.list()).toHaveLength(0); @@ -360,3 +368,38 @@ describe("Session", () => { }); }); }); + +describe("manifest reconciliation", () => { + let repoDir: string; + + beforeAll(async () => { + repoDir = await initTestRepo(); + + await writeFile(join(repoDir, "app.log"), "v1\n"); + await writeFile(join(repoDir, "main.ts"), "const x = 1;\n"); + await git(repoDir, "add", "."); + await git(repoDir, "commit", "-m", "c1"); + + await writeFile(join(repoDir, ".gitignore"), "*.log\n"); + await writeFile(join(repoDir, "app.log"), "v1\nv2\n"); + await git(repoDir, "add", "app.log", ".gitignore"); + await git(repoDir, "commit", "-m", "c2"); + }); + + afterAll(async () => { + await rm(repoDir, { recursive: true, force: true, maxRetries: 3 }); + }); + + it("changed tracked-but-ignored file gets a manifest entry and becomes visible in the index", async () => { + const session = await Session.start(repoDir, "committed"); + + expect(session.changedFiles.map((f) => f.path)).toContain("app.log"); + + const entry = session.manifest.find((m) => m.path === "app.log"); + expect(entry).toBeDefined(); + expect(entry!.bytes).toBeGreaterThan(0); + expect(entry!.lines).toBe(2); + + expect(session.rfs.fileExists("app.log")).toBe(true); + }); +}); diff --git a/claude/kensai/mcp/review/src/session/session.ts b/claude/kensai/mcp/review/src/session/session.ts index b4de340..9e791c7 100644 --- a/claude/kensai/mcp/review/src/session/session.ts +++ b/claude/kensai/mcp/review/src/session/session.ts @@ -1,4 +1,5 @@ import { createHash } from "node:crypto"; +import fsp from "node:fs/promises"; import path from "node:path"; import type { GitFileStat, GitLogEntry } from "../repofs/git/git.ts"; @@ -181,9 +182,16 @@ export function filterReviewableFiles(files: readonly GitFileStat[]): GitFileSta return files.filter((f) => f.status !== "deleted" && !shouldSkipDiff(f.path)); } -/** Builds per-file shape metrics for all non-deleted changed files. Counts lines on demand (only for changed files). */ +/** + * Builds per-file shape metrics for all non-deleted changed files. The index is stat-free at + * build, so this stats each candidate (filling the entry's lazy size) and counts lines on + * demand — only for changed files. Changed files absent from the gitignore-filtered index + * (tracked-but-ignored) are reconciled into it first — the diff is the source of truth. + */ async function buildManifest(rfs: RepoFs, files: readonly GitFileStat[]): Promise { - const candidates: Array<{ path: string; entry: { size: number; isBinary: boolean } }> = []; + await rfs.index.add(files.filter((f) => f.status !== "deleted" && !rfs.index.get(f.path)).map((f) => f.path)); + + const candidates: Array<{ path: string; entry: { size: number | null; isBinary: boolean } }> = []; for (const f of files) { if (f.status === "deleted") continue; @@ -194,20 +202,20 @@ async function buildManifest(rfs: RepoFs, files: readonly GitFileStat[]): Promis const results = await Promise.all( candidates.map(async ({ path: filePath, entry }) => { + const absPath = path.resolve(rfs.root, filePath); + + const stat = await fsp.stat(absPath).catch(() => null); + if (stat) entry.size = stat.size; + const bytes = entry.size ?? 0; + if (entry.isBinary) { - return { path: filePath, bytes: entry.size, lines: 0, maxLineLen: 0, binary: true }; + return { path: filePath, bytes, lines: 0, maxLineLen: 0, binary: true }; } try { - const lc = await countFileLines(path.resolve(rfs.root, filePath)); - return { - path: filePath, - bytes: entry.size, - lines: lc.lineCount, - maxLineLen: lc.maxLineLen, - binary: lc.isBinary, - }; + const lc = await countFileLines(absPath); + return { path: filePath, bytes, lines: lc.lineCount, maxLineLen: lc.maxLineLen, binary: lc.isBinary }; } catch { - return { path: filePath, bytes: entry.size, lines: 0, maxLineLen: 0, binary: false }; + return { path: filePath, bytes, lines: 0, maxLineLen: 0, binary: false }; } }), ); diff --git a/claude/kensai/mcp/review/src/toolsets/fs/list_dir.ts b/claude/kensai/mcp/review/src/toolsets/fs/list_dir.ts index 4c7db24..d9cdf9f 100644 --- a/claude/kensai/mcp/review/src/toolsets/fs/list_dir.ts +++ b/claude/kensai/mcp/review/src/toolsets/fs/list_dir.ts @@ -85,7 +85,7 @@ function collect( collect(child.children, rel, maxDepth, currentDepth + 1, skipDotfiles, maxEntries, out, state); } } else if (child.type === "file") { - out.push(`${rel} (${child.size} bytes)\n`); + out.push(child.size != null ? `${rel} (${child.size} bytes)\n` : `${rel}\n`); } else { out.push(`${rel} -> ${child.target}\n`); } @@ -111,7 +111,7 @@ export function listDirTool(ctx: ToolContext): ToolRegistrar { ` - Total entries capped at ${DEFAULT_MAX_ENTRIES}. Truncation marker names the cap that fired.`, ' - Dotfiles shown by default; skip_dotfiles=true omits "."-prefixed entries.', ` - Descent suppressed for: ${EXCLUDE_DIRS.join(", ")}. The entry itself is still listed.`, - ' - Output: one entry per line. Directories as "path/", files as "path (N bytes)". Sorted alphabetically.', + ' - Output: one entry per line. Directories as "path/", files as "path", with "(N bytes)" when size is known. Sorted alphabetically.', " - Empty directory -> [directory is empty] marker, not a tool error.", ].join("\n"), inputSchema,