diff --git a/.trail/learnings/2026-11-11-explicit-repository-mounts-need-boundary-checks-atif.md b/.trail/learnings/2026-11-11-explicit-repository-mounts-need-boundary-checks-atif.md new file mode 100644 index 000000000..cf1b75afc --- /dev/null +++ b/.trail/learnings/2026-11-11-explicit-repository-mounts-need-boundary-checks-atif.md @@ -0,0 +1,25 @@ +--- +title: Explicit repository mounts need boundary checks and retarget detection +author: Atif +date: 2026-11-11 +branch: atif_symlink +id: l-467468cd +scope: repo +status: active +src: agent +touches: + - src/ingest/fs.ts + - src/graph/fingerprint.ts + - test/graph-follow-nested-repos.test.ts +--- + +## Decided +Review of atif_symlink at a18b1bc found two reproducible gaps before merging. Preserve main's independent submodule and nested-repository opt-ins when admitting explicit mounts. Capture physical repository targets during the existing file walk and persist them in the fingerprint. A changed target makes freshness unknown, forcing one rebuild that hashes source bytes and saves the new identity. This avoids trusting identical file stats from a different checkout without adding another enumeration or hashing every file on every query. Keep older fingerprints readable so their saved filters survive the one-time identity-aware rebuild. + +## Tried and ruled out +The 37 existing ingest, nested-repo, only-dir and context-only-dir tests all passed, but do not establish these edge cases. A temporary Git fixture with a mode-160000 entry returned no files with followNestedRepos=true and followSubmodules=false; adding that path to includeIgnoredRepos returned the child's source. A second fixture built a linked repo then retargeted the link to another repo with different equal-length source and equal mtimes. Default probeDrift reported clean; GRAFT_REFRESH=hash detected the changed file. Therefore file-list parity and ordinary edit-refresh tests alone are insufficient. + +## Watch out +Keep logical mount paths for graph IDs and physical checkout paths for cycle/target identity. Do not replace recursion-stack cycle detection with global realpath deduplication without deciding how multiple mount aliases should behave. The stat shortcut predates this branch; symlink retargeting gives it a new trigger. Explicit mount admission must not change a gitlink into a nested clone: classify first, then apply the independent traversal gate. Record target identity before invoking Git so filesystem fallback also retains it. + +In the code: src/ingest/fs.ts explicit entries and followChild; src/graph/fingerprint.ts probeDrift and statUnchanged; test/graph-follow-nested-repos.test.ts. Run the four targeted test files with node --import tsx --test and test-local Git signing disabled. diff --git a/README.md b/README.md index 44f79c59b..322887499 100644 --- a/README.md +++ b/README.md @@ -699,9 +699,16 @@ trail build -e .ts .py # only index these extensions (also limits trail build --no-reuse # re-parse every file instead of replaying unchanged ones from cache trail build --follow-submodules # include initialized submodules; persist the choice for builds + MCP refresh trail build --no-follow-submodules # exclude submodules again and persist that choice (the default) -trail build --follow-nested-repos # include nested git clones the index doesn't track; persist the choice +trail build --follow-nested-repos # include nested git clones and symlinked repos; persist the choice trail build --no-follow-nested-repos # exclude nested clones again and persist that choice (the default) +# Index normal project source plus selected repo mounts (child ignores still apply). +trail build --follow-nested-repos \ + --include-repo source-material/historical-repos/widget \ + --include-repo source-material/historical-repos/databraid-api +trail build --all-dirs # clear a saved --only-dir restriction; keep selected repo mounts +trail build --no-include-ignored-repos # clear the saved ignored-repository mounts + trail status [dir] # the graft map, learnings and tokens saved, on one screen trail upgrade # npm install -g the latest published version # a new version is announced automatically (checked once a day); @@ -791,12 +798,17 @@ The graft map supports these layouts: refreshes behave the same way. Run `trail build --no-follow-submodules` to restore and persist the default boundary. - **A git repo with other repos cloned inside it** (no gitlink, no index entry): the shape multi-repo manifest tools like `west`, `repo`, `gclient` and `tsrc` check dependencies out into, and the shape you get by cloning an upstream into the tree to patch it locally. `--follow-submodules` cannot reach these: they have no `160000` index entry to follow. Run `trail build --follow-nested-repos` to fold them into one graph, prefixing child paths (for example, `external/parser/src/index.ts`) while honoring each clone's own Git ignore rules. A clone at a git-ignored path stays absent, since Git never reports it. The choice is saved in `.graft/config.json` and is independent of `--follow-submodules`; neither flag implies the other. Run `trail build --no-follow-nested-repos` to restore and persist the default boundary. Prefer this over the multi-repo split below when the nested repos import from each other and you want those edges in one graph; prefer the split when you want each repo scored and refreshed on its own. +- **A Git repo containing symlinks to other repos** (for example, a review folder with `repos/api` and `repos/frontend` pointing to separate checkouts) — run `trail build --follow-nested-repos` at the review folder's root. Git-visible links to directories with their own `.git` are followed, including linked worktrees. The single graph stays at the review folder's `graft/`, with paths such as `repos/api/src/index.ts`, so agents and queries opened at that root use the same index. Each linked repo keeps its own ignore rules; broken links, cycles, file links, and links to ordinary directories are skipped. The saved option also applies to later builds and automatic refreshes. - **A folder of separate git repos** (no `.git` at the top): `trail build` auto-splits: each child gets its own (git-ignored) `graft/`, and the parent gets a `graft/workspace.json` index. Queries from the parent federate across every child, always labeled `/`. Run `trail build` inside a child to work on just that repo. +To index the normal project source alongside selected linked repositories, use `--follow-nested-repos` and repeat `--include-repo ` for each mount, including mounts the parent deliberately Git-ignores. Every selected path must be a repository under the build root. Graft saves these explicit mounts in local `.graft/config.json`, preserves the parent's `.gitignore`, and continues to honor each child's ignore rules. Plain rebuilds and automatic refreshes retain this setup. `.ignore` controls ripgrep and does not override Git's visibility for Graft. + +`--only-dir` restricts the entire index to its paths, including project source. If an earlier build saved such a restriction, pass `--all-dirs` once to restore the normal file scope while retaining selected repository mounts. The older `--include-ignored-repos --only-dir ...` form remains available for an index limited to those repositories. + In every layout, narrow to one sub-project with `trail ask "" --in /` once you know where you're working. diff --git a/src/cli.ts b/src/cli.ts index 9f06d115c..88a47b549 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -12,7 +12,7 @@ import { spawnSync } from "node:child_process"; import "dotenv/config"; import { Command, Option } from "commander"; import { existsSync, readdirSync, readFileSync, rmSync, statSync } from "node:fs"; -import { basename, dirname, join, relative, resolve, sep } from "node:path"; +import { basename, dirname, join, relative, resolve, isAbsolute, sep } from "node:path"; import { fileURLToPath } from "node:url"; import { Graft } from "./engine.js"; import { resolveConfig, type EngineConfig } from "./ai/providers.js"; @@ -58,7 +58,7 @@ import { createInterface } from "node:readline"; import { createHash } from "node:crypto"; import { formatUpgradeReport, formatVersionReport, getNpmViewVersion, readCurrentVersion, readPackageName, runUpgrade } from "./cli-meta.js"; import { nativeFallbackNotice } from "./graph/native.js"; -import { lockHeldByOther, patchBuildConfig, type BuildConfig } from "./util/state.js"; +import { lockHeldByOther, patchBuildConfig, readFollowNestedRepos, type BuildConfig } from "./util/state.js"; import { NEVER_INCLUDE_DIRS } from "./ingest/fs.js"; import { normalizePathPrefix } from "./util/paths.js"; import { latestSession, formatSessionStats, sessionInputRate } from "./claude/session-metrics.js"; @@ -551,12 +551,26 @@ grouped(program.command("build"), GROUP.setup) ) .option( "--follow-nested-repos", - "include nested Git clones the index does not track (a multi-repo manifest checkout, or any repo cloned into the tree) " + + "include nested Git clones and directory symlinks to Git repos " + "as ONE graph, so imports across them resolve; persisted for later builds and automatic refreshes", ) .option( "--no-follow-nested-repos", - "exclude untracked nested Git clones; persisted for later builds and automatic refreshes (default)", + "exclude nested Git clones and directory symlinks to Git repos; persisted for later builds and automatic refreshes (default)", + ) + .option( + "--include-repo ", + "with --follow-nested-repos: add this repository to the normal project index, even if its mount is Git-ignored; repeatable and persisted; child ignores still apply", + (val: string, prev: string[]) => [...prev, val], + [] as string[], + ) + .option( + "--include-ignored-repos", + "with --follow-nested-repos and --only-dir: include explicitly selected Git repos even if the parent ignores their mounts; child ignores still apply; persisted", + ) + .option( + "--no-include-ignored-repos", + "clear the saved ignored-repository mounts", ) .option( "--include-dir ", @@ -574,6 +588,7 @@ grouped(program.command("build"), GROUP.setup) (val: string, prev: string[]) => [...prev, val], [] as string[], ) + .option("--all-dirs", "clear any saved --only-dir restriction and restore the normal project file scope") .option("--no-gitignore", "skip writing graft/ into .gitignore (same as GRAFT_NO_GITIGNORE=1)") .option("--no-ignore", "skip writing .ignore for ripgrep re-admit (same as GRAFT_NO_IGNORE=1)") .action(async ( @@ -591,9 +606,12 @@ grouped(program.command("build"), GROUP.setup) synthConcurrency?: string; synthBatchChars?: string; includeDir?: string[]; + includeRepo?: string[]; onlyDir?: string[]; + allDirs?: boolean; followSubmodules?: boolean; followNestedRepos?: boolean; + includeIgnoredRepos?: boolean; gitignore?: boolean; ignore?: boolean; }, @@ -657,7 +675,11 @@ grouped(program.command("build"), GROUP.setup) // The whitelist is NOT persisted to `.graft/config.json`: it belongs with the // graph (the fingerprint records it at build time), never in the source repo, // so a `--only-dir` build leaves no trace under the repo being indexed. - let onlyDirs: string[] | undefined; + if (opts.allDirs && opts.onlyDir?.length) { + console.error("✗ --all-dirs and --only-dir cannot be used together"); + process.exit(1); + } + let onlyDirs: string[] | undefined = opts.allDirs ? [] : undefined; if (opts.onlyDir && opts.onlyDir.length > 0) { // --only-dir takes a repo-relative path prefix, normalized to the same // posix, no-`./`, no-trailing-slash form `--in` uses, so the prefix match @@ -679,6 +701,40 @@ grouped(program.command("build"), GROUP.setup) if (followNestedReposWasExplicit && typeof opts.followNestedRepos === "boolean") { buildConfigPatch.followNestedRepos = opts.followNestedRepos; } + const explicitIgnoredRepos = command.getOptionValueSource("includeIgnoredRepos") === "cli"; + if (opts.includeRepo?.length && explicitIgnoredRepos) { + console.error("✗ --include-repo cannot be combined with --include-ignored-repos or --no-include-ignored-repos"); + process.exit(1); + } + if (opts.includeRepo?.length || explicitIgnoredRepos) { + if (explicitIgnoredRepos && opts.includeIgnoredRepos === false) { + buildConfigPatch.includeIgnoredRepos = []; + } else { + const flag = opts.includeRepo?.length ? "--include-repo" : "--include-ignored-repos"; + const selectedRepos = opts.includeRepo?.length + ? [...new Set(opts.includeRepo.map((path) => normalizePathPrefix(path)))] + : onlyDirs; + const root = resolve(dir); + const followsNested = buildConfigPatch.followNestedRepos ?? readFollowNestedRepos(root); + if (!followsNested || !selectedRepos?.length || !existsSync(join(root, ".git"))) { + console.error(`✗ ${flag} requires --follow-nested-repos and ${flag === "--include-repo" ? "repository paths" : "--only-dir paths to repositories"} under a Git root`); + process.exit(1); + } + for (const path of selectedRepos) { + const abs = resolve(root, path); + const rel = relative(root, abs); + let valid = rel !== "" && rel !== ".." && !rel.startsWith(`..${sep}`) && !isAbsolute(rel); + try { + valid = valid && statSync(abs).isDirectory() && existsSync(join(abs, ".git")); + } catch { valid = false; } + if (!valid) { + console.error(`✗ ${flag}: "${path}" must name a Git repository inside the build root`); + process.exit(1); + } + } + buildConfigPatch.includeIgnoredRepos = selectedRepos; + } + } if (Object.keys(buildConfigPatch).length > 0) { patchBuildConfig(resolve(dir), buildConfigPatch); } diff --git a/src/context/build.ts b/src/context/build.ts index fc3e6a5d8..0f86c4e6c 100644 --- a/src/context/build.ts +++ b/src/context/build.ts @@ -21,7 +21,7 @@ import { readFingerprint } from "../graph/fingerprint.js"; import { contentHash } from "../util/id.js"; import { relPosix } from "../util/paths.js"; import { readSourceFile } from "../util/source.js"; -import { readFollowNestedRepos, readFollowSubmodules, readIncludeDirs } from "../util/state.js"; +import { readFollowNestedRepos, readFollowSubmodules, readIncludeDirs, readIncludeIgnoredRepos } from "../util/state.js"; import type { Summarizer } from "../ai/summarize.js"; import { LlmFailureGate } from "../ai/failure.js"; import type { FileSummary, SynthNode, Synthesizer } from "../ai/synthesize.js"; @@ -122,7 +122,7 @@ export interface BuildResult { * fingerprint — the same source `checkGraph` / `probeDrift` use, so a later * `--deep` without the flag still skips files the wiring pass excluded. */ function resolveOnlyDirs(outDir: string, explicit?: readonly string[]): Set | undefined { - const list = explicit && explicit.length > 0 ? explicit : (readFingerprint(outDir)?.onlyDirs ?? []); + const list = explicit ?? readFingerprint(outDir)?.onlyDirs ?? []; return list.length > 0 ? new Set(list) : undefined; } @@ -141,6 +141,7 @@ export function listContextFiles( const walked = walkDir(root, readIncludeDirs(root), { followSubmodules: readFollowSubmodules(root), followNestedRepos: readFollowNestedRepos(root), + includeIgnoredRepos: readIncludeIgnoredRepos(root), }) .filter((f) => exts.some((e) => f.toLowerCase().endsWith(e))) .filter((f) => !f.startsWith(outDir)); diff --git a/src/graph/build.ts b/src/graph/build.ts index ac007870c..03d6498b9 100644 --- a/src/graph/build.ts +++ b/src/graph/build.ts @@ -25,14 +25,14 @@ import type { NativeFallback } from "./native.js"; import { contentHash } from "../util/id.js"; import { relPosix } from "../util/paths.js"; import { readSourceFile } from "../util/source.js"; -import { readFollowNestedRepos, readFollowSubmodules, readIncludeDirs } from "../util/state.js"; +import { readFollowNestedRepos, readFollowSubmodules, readIncludeDirs, readIncludeIgnoredRepos } from "../util/state.js"; import { emptyExtractCache, readExtractCache, writeExtractCache, type ExtractEntry, } from "./extract-cache.js"; -import { writeFingerprint } from "./fingerprint.js"; +import { readFingerprint, writeFingerprint } from "./fingerprint.js"; import { seedGraph, type SeedResult } from "./seed.js"; import { filterByOnlyDirs, listSourceStats, normalizeExtensions } from "./source-files.js"; import { resolveEdges, type GoModule } from "./resolve.js"; @@ -223,11 +223,15 @@ export async function buildGraph( // Enumerate once: source extraction, scope discovery, and Go module // resolution must agree on the same Git-ignore-aware working-tree view — // including the repo's persisted directory and submodule choices. + const repositoryTargets: Record = Object.create(null); const walked = walkDir(root, readIncludeDirs(root), { followSubmodules: readFollowSubmodules(root), followNestedRepos: readFollowNestedRepos(root), + includeIgnoredRepos: readIncludeIgnoredRepos(root), + onRepository: (mount, target) => { repositoryTargets[mount] = target; }, }); - const onlyDirs = opts.onlyDirs && opts.onlyDirs.length > 0 ? new Set(opts.onlyDirs) : undefined; + const selectedDirs = opts.onlyDirs ?? readFingerprint(outDir)?.onlyDirs; + const onlyDirs = selectedDirs?.length ? new Set(selectedDirs) : undefined; const repoFiles = filterByOnlyDirs(walked, root, onlyDirs); const extensions = normalizeExtensions(opts.extensions); const files = listSourceStats(root, outDir, repoFiles, undefined, extensions); @@ -436,7 +440,7 @@ export async function buildGraph( // these source bytes." Nothing about the projections below — which is why it is // safe to write here, and why `graphOnly` builds (the query path, which stops // right after this line) are still recorded as fresh. - writeFingerprint(outDir, entries, opts.onlyDirs, extensions); + writeFingerprint(outDir, entries, selectedDirs, extensions, repositoryTargets); // Tier-2 passive surface: project the nodes into per-file markdown cards, and // refresh the INDEX roster. Pure projection — no LLM, no network. diff --git a/src/graph/fingerprint.ts b/src/graph/fingerprint.ts index 012960800..dfd60399e 100644 --- a/src/graph/fingerprint.ts +++ b/src/graph/fingerprint.ts @@ -18,11 +18,12 @@ * degrade safely: no fingerprint → "unknown, rebuild"; no parse cache → the * rebuild is just cold. */ +import { realpathSync } from "node:fs"; import { join } from "node:path"; import { CACHE_DIR } from "../context/node-file.js"; import { contentHash } from "../util/id.js"; import { readSourceFile } from "../util/source.js"; -import { readJson, writeJsonAtomic } from "../util/state.js"; +import { readFollowNestedRepos, readJson, writeJsonAtomic } from "../util/state.js"; import { extractorStamp, pruneSidecars, type ExtractEntry } from "./extract-cache.js"; import { listSourceStats } from "./source-files.js"; @@ -56,6 +57,9 @@ export interface Fingerprint { onlyDirs?: string[]; /** Normalized source extension selection (`--extensions`); absent = all supported. */ extensions?: string[]; + /** Logical repository mounts → physical checkout paths, observed during the + * build's walk. Missing on older fingerprints; an empty object means no mounts. */ + repositoryTargets?: Record; } /** What moved since the last build. Empty in all three arrays = nothing to do. */ @@ -91,11 +95,12 @@ export function writeFingerprint( entries: Record, onlyDirs?: string[], extensions?: string[], + repositoryTargets: Record = {}, ): boolean { const files: Record = {}; for (const [rel, e] of Object.entries(entries)) files[rel] = [e.size, e.mtimeMs, e.hash]; try { - const record: Fingerprint = { version: FINGERPRINT_VERSION, extractor: stamp(), files }; + const record: Fingerprint = { version: FINGERPRINT_VERSION, extractor: stamp(), files, repositoryTargets }; if (onlyDirs && onlyDirs.length > 0) record.onlyDirs = onlyDirs; if (extensions && extensions.length > 0) record.extensions = extensions; writeJsonAtomic(fingerprintPath(outDir), record, true); @@ -155,6 +160,18 @@ export function probeDrift(root: string, outDir: string): Drift | null { const fp = readFingerprint(outDir); if (!fp) return null; + // A different checkout may preserve every filename, size and mtime. Rebuild + // once to verify its bytes and save its identity rather than trusting the old + // checkout's stat shortcut. Keep legacy fingerprints readable for their filters. + if (!fp.repositoryTargets && readFollowNestedRepos(root)) return null; + for (const [mount, target] of Object.entries(fp.repositoryTargets ?? {})) { + try { + if (realpathSync(join(root, mount)) !== target) return null; + } catch { + // A removed/broken mount is handled by the ordinary removed-file diff below. + } + } + const drift: Drift = { changed: [], added: [], removed: [] }; const seen = new Set(); diff --git a/src/graph/scopes.ts b/src/graph/scopes.ts index 7e7648bfd..b0d8ed2d9 100644 --- a/src/graph/scopes.ts +++ b/src/graph/scopes.ts @@ -25,7 +25,7 @@ import { existsSync, readdirSync, readFileSync, realpathSync, statSync, type Dir import { join, resolve, sep } from "node:path"; import { shouldSkipDir, walkDir } from "../ingest/fs.js"; import { relPosix } from "../util/paths.js"; -import { readFollowNestedRepos, readFollowSubmodules, readIncludeDirs } from "../util/state.js"; +import { readFollowNestedRepos, readFollowSubmodules, readIncludeDirs, readIncludeIgnoredRepos } from "../util/state.js"; import type { GraphV1, ScopeV1 } from "./types.js"; /** Project-marker files, checked in this order (also the order `markers` is built in). */ @@ -135,6 +135,7 @@ export function discoverScopes( repoFiles: string[] = walkDir(root, readIncludeDirs(resolve(root)), { followSubmodules: readFollowSubmodules(resolve(root)), followNestedRepos: readFollowNestedRepos(resolve(root)), + includeIgnoredRepos: readIncludeIgnoredRepos(resolve(root)), }), ): ScopeV1[] { const absRoot = resolve(root); diff --git a/src/graph/source-files.ts b/src/graph/source-files.ts index 777579b40..69d977cbd 100644 --- a/src/graph/source-files.ts +++ b/src/graph/source-files.ts @@ -10,7 +10,7 @@ import { statSync } from "node:fs"; import { extname, resolve } from "node:path"; import { walkDir } from "../ingest/fs.js"; import { relPosix } from "../util/paths.js"; -import { readFollowNestedRepos, readFollowSubmodules, readIncludeDirs } from "../util/state.js"; +import { readFollowNestedRepos, readFollowSubmodules, readIncludeDirs, readIncludeIgnoredRepos } from "../util/state.js"; import { languageOf, depthExtensions } from "./extract.js"; import { genericLangOf, genericExtensions } from "./generic.js"; import { containerLangOf, containerExtensions } from "./container.js"; @@ -72,6 +72,7 @@ export function listSourceFiles( repoFiles: string[] = walkDir(root, readIncludeDirs(resolve(root)), { followSubmodules: readFollowSubmodules(resolve(root)), followNestedRepos: readFollowNestedRepos(resolve(root)), + includeIgnoredRepos: readIncludeIgnoredRepos(resolve(root)), }), onlyDirs?: ReadonlySet, extensions?: readonly string[], diff --git a/src/ingest/fs.ts b/src/ingest/fs.ts index 1932bf2d4..33f79db0c 100644 --- a/src/ingest/fs.ts +++ b/src/ingest/fs.ts @@ -4,6 +4,7 @@ import { spawnSync } from "node:child_process"; import { existsSync, lstatSync, readdirSync, realpathSync, statSync } from "node:fs"; import { isAbsolute, join, relative, resolve, sep } from "node:path"; +import { relPosix } from "../util/paths.js"; /** Directories that are dependency/build output, never source. */ export const SKIP_DIRS = new Set([ @@ -78,21 +79,27 @@ export function shouldSkipDir(name: string, includes?: ReadonlySet): boo * * - `followSubmodules` follows a GITLINK — an index entry with mode `160000`, * a commit the superproject versions. - * - `followNestedRepos` follows a nested clone the parent's index knows - * nothing about. Manifest-driven multi-repo tools (west, repo, gclient, + * - `followNestedRepos` follows a nested clone or a directory symlink to a + * repo. Manifest-driven multi-repo tools (west, repo, gclient, * tsrc, mr, …) all check dependencies out this way, and so does anyone who * clones an upstream into the tree to patch it locally. * * Both default to false, preserving the historical superproject boundary. Each * child keeps its own ignore rules; uninitialized submodules and gitignored - * nested clones remain absent either way (`--exclude-standard` never emits - * them, so there is nothing to follow). + * nested repos remain absent either way (`--exclude-standard` never emits + * them, so there is nothing to follow), unless explicitly named through + * `includeIgnoredRepos`; that exception bypasses only the parent's ignores. */ export interface WalkOptions { /** Include initialized Git submodules (gitlinks) recursively. Default false. */ followSubmodules?: boolean; - /** Include nested Git clones the parent index does not track. Default false. */ + /** Include nested Git clones and directory symlinks to repos. Default false. */ followNestedRepos?: boolean; + /** Explicit parent-relative repo mounts, bypassing only the parent's ignores. */ + includeIgnoredRepos?: ReadonlySet; + /** Observe followed repository mounts during the same walk. Paths are relative + * to the walk root; physical targets identify checkouts, not graph node IDs. */ + onRepository?: (mount: string, target: string) => void; } /** @@ -157,8 +164,10 @@ function remapWalkPaths(requested: string, canonical: string, files: string[]): * root* is unwrapped once ({@link canonicalWalkRoot}); emitted paths are remapped * onto the caller-facing root so `relPosix` stays `src/foo.ts` rather than a * `../` escape through the physical path (macOS `/tmp` → `/private/tmp`). - * Symlinks *inside* the tree are not followed: git still `lstat`s each listed - * path, and the filesystem walk still requires `Dirent.isFile` / `isDirectory`. + * Symlinks *inside* the tree are skipped by default. `followNestedRepos` also + * follows Git-visible directory links with their own `.git`, retaining the + * mounted paths and using physical repository identities to stop cycles. + * Other internal links and links in the filesystem fallback remain skipped. */ export function walkDir( dir: string, @@ -189,9 +198,18 @@ function gitVisibleFiles( } const state = traversal ?? { topRoot: root, activeRoots: new Set() }; - const rootKey = process.platform === "win32" ? root.toLowerCase() : root; + let realRoot: string; + try { + realRoot = realpathSync(root); + } catch { + realRoot = root; // Let Git's existing failure/fallback path handle it. + } + const rootKey = process.platform === "win32" ? realRoot.toLowerCase() : realRoot; if (state.activeRoots.has(rootKey)) return []; state.activeRoots.add(rootKey); + // Record before Git runs: a child using the filesystem fallback still needs + // target identity for freshness, just like a successfully enumerated checkout. + if (traversal) opts.onRepository?.(relPosix(state.topRoot, root), realRoot); // Following uses `-t --stage` so one process can distinguish gitlinks from // tracked and untracked files. `-z` keeps either form safe for unusual paths. @@ -249,6 +267,18 @@ function gitVisibleFiles( } const out = new Set(); + if (!traversal && opts.followNestedRepos === true) { + for (const rel of opts.includeIgnoredRepos ?? []) { + const abs = resolve(root, rel); + // Persisted settings must never reach above the requested root or turn + // an ordinary ignored source directory into an implicit repository. + if (abs === root || escapedCanonical(root, abs) || !existsSync(join(abs, ".git"))) continue; + const mount = relative(root, abs).replace(/\\/g, "/"); + const prior = entries.get(mount); + const gitlink = prior?.gitlink === true; + entries.set(mount, { gitlink, nested: !gitlink }); + } + } for (const [rel, entry] of entries) { const abs = resolve(root, rel); // Filter against the original superproject path. Otherwise a submodule @@ -256,9 +286,28 @@ function gitVisibleFiles( // recursion resets its relative root. if (skippedPath(relative(state.topRoot, abs), includes)) continue; - const followChild = - (entry.gitlink && opts.followSubmodules === true) || - (entry.nested && opts.followNestedRepos === true); + let stat: ReturnType; + try { + stat = lstatSync(abs); + } catch { + // Git can still list tracked paths deleted from the working tree. + continue; + } + let linkedRepo = false; + if (!entry.gitlink && opts.followNestedRepos === true && stat.isSymbolicLink()) { + try { + const target = realpathSync(abs); + // Do not descend through a link to this repo or one of its ancestors. + if (target === realRoot || realRoot.startsWith(target + sep)) continue; + linkedRepo = statSync(target).isDirectory() && existsSync(join(abs, ".git")); + } catch { + continue; // A dangling or cyclic link is not a repository to follow. + } + } + + const followChild = entry.gitlink + ? opts.followSubmodules === true + : (entry.nested || linkedRepo) && opts.followNestedRepos === true; if (followChild) { // A deinitialized gitlink can resolve upward to the superproject when Git // runs inside it, and a nested-looking directory may have no repo at all. @@ -281,14 +330,7 @@ function gitVisibleFiles( continue; } - try { - const stat = lstatSync(abs); - if (!stat.isFile() || stat.size > MAX_FILE_BYTES) continue; - } catch { - // A tracked file deleted from the working tree is still printed by - // `--cached`; absence means it is not part of the current source set. - continue; - } + if (!stat.isFile() || stat.size > MAX_FILE_BYTES) continue; out.add(abs); } diff --git a/src/util/state.ts b/src/util/state.ts index c189ac248..54bd0fc2d 100644 --- a/src/util/state.ts +++ b/src/util/state.ts @@ -122,6 +122,9 @@ export interface BuildConfig { * checkout someone parked in the tree. Absent/false keeps the historical * boundary. */ followNestedRepos?: boolean; + /** Explicit repo-relative repository mounts to follow even when the parent + * ignores them. Child ignore rules still apply; requires followNestedRepos. */ + includeIgnoredRepos?: string[]; } /** Local, Git-ignored repository configuration. Kept outside generated @@ -180,6 +183,11 @@ export function readFollowSubmodules(d: string): boolean { export function readFollowNestedRepos(d: string): boolean { return readBuildConfig(d)?.followNestedRepos === true; } + +export function readIncludeIgnoredRepos(d: string): Set | undefined { + const repos = readBuildConfig(d)?.includeIgnoredRepos; + return repos?.length ? new Set(repos) : undefined; +} // Best-effort read-modify-write; not atomic across concurrent processes, but acceptable // for episodic hook writes (worst case is a lost update, not corruption). export function patchStats(d: string, patch: Partial): Stats { diff --git a/test/context-only-dir.test.ts b/test/context-only-dir.test.ts index b9c29229c..77d826814 100644 --- a/test/context-only-dir.test.ts +++ b/test/context-only-dir.test.ts @@ -68,6 +68,22 @@ function manifestPaths(dir: string): string[] { return manifest.files.map((f) => f.path).sort(); } +test("an explicit empty selection restores the full concept scope after a restricted build", async () => { + const dir = repoWithOutOfScope(); + const rec = recording(); + try { + await buildGraph(dir, { onlyDirs: ["src"] }); + await buildContext(dir, { + model: "fake", summarizer: rec.summarizer, synthesizer: rec.synthesizer, + onlyDirs: [], + }); + assert.deepEqual([...rec.summarized].sort(), ["docs/c.ts", "src/a.ts", "tools/b.ts"]); + assert.deepEqual(manifestPaths(dir), ["docs/c.ts", "src/a.ts", "tools/b.ts"]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("#252: buildContext --only-dir src does not summarize or synthesize out-of-scope files", async () => { const dir = repoWithOutOfScope(); const rec = recording(); diff --git a/test/graph-follow-nested-repos.test.ts b/test/graph-follow-nested-repos.test.ts index b9eb52d3c..4a9f88c80 100644 --- a/test/graph-follow-nested-repos.test.ts +++ b/test/graph-follow-nested-repos.test.ts @@ -1,13 +1,14 @@ import { execFileSync } from "node:child_process"; import { test } from "node:test"; import assert from "node:assert/strict"; -import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, readFileSync, rmSync, symlinkSync, unlinkSync, utimesSync, writeFileSync } from "node:fs"; import { join } from "node:path"; -import { isClean, probeDrift } from "../src/graph/fingerprint.js"; +import { fingerprintPath, isClean, probeDrift, readFingerprint } from "../src/graph/fingerprint.js"; import { readGraph, wiringPath } from "../src/graph/write.js"; import { readBuildConfig } from "../src/util/state.js"; import type { GraphV1 } from "../src/graph/types.js"; import { runCli, tmpRepo } from "./helpers.js"; +import { ensureFreshGraph } from "../src/graph/refresh.js"; function gitRun(root: string, args: string[]): void { execFileSync("git", args, { cwd: root, stdio: "ignore" }); @@ -186,8 +187,196 @@ test("follow-nested-repos and follow-submodules are independent switches", () => const both = graphOf(parent); assert.ok(both.nodes.some((n) => n.id === "modules/sub/src/sub.ts#subValue")); assert.ok(both.nodes.some((n) => n.id === "external/dep/src/dep.ts#depValue")); + + // Explicit admission bypasses parent ignores, never the submodule gate. + expectCliOk(["build", parent, "--include-repo", "modules/sub", "--no-follow-submodules"]); + assert.ok(!graphOf(parent).nodes.some((n) => n.path.startsWith("modules/sub/"))); + expectCliOk(["build", parent]); + assert.ok(!graphOf(parent).nodes.some((n) => n.path.startsWith("modules/sub/"))); + expectCliOk(["build", parent, "--follow-submodules"]); + assert.ok(graphOf(parent).nodes.some((n) => n.id === "modules/sub/src/sub.ts#subValue")); } finally { rmSync(parent, { recursive: true, force: true }); rmSync(sub, { recursive: true, force: true }); } }); + +test("linked repos build and refresh one queryable graph at the parent, including a linked worktree", async () => { + const parent = tmpRepo("linked-graph-parent"); + const library = tmpRepo("linked-graph-library"); + const checkout = tmpRepo("linked-graph-checkout"); + const worktree = join(checkout, "worktree"); + try { + gitRun(parent, ["init", "-q"]); + gitRun(library, ["init", "-q"]); + write(library, "src/api.ts", "export function linkedValue(): number { return 1; }\n"); + commitAll(library, "library fixture"); + gitRun(library, ["worktree", "add", "--detach", worktree]); + write(worktree, "src/consumer.ts", 'import { linkedValue } from "../../library/src/api.js";\nexport function linkedConsumer() { return linkedValue(); }\n'); + mkdirSync(join(parent, "repos")); + const linkType = process.platform === "win32" ? "junction" : "dir"; + symlinkSync(library, join(parent, "repos", "library"), linkType); + symlinkSync(worktree, join(parent, "repos", "consumer"), linkType); + + expectCliOk(["build", parent, "--follow-nested-repos"]); + const graph = graphOf(parent); + assert.ok(graph.nodes.some((n) => n.id === "repos/library/src/api.ts#linkedValue")); + assert.ok(graph.nodes.some((n) => n.id === "repos/consumer/src/consumer.ts#linkedConsumer")); + assert.ok(graph.edges.some((e) => e.relation === "imports" && e.source === "repos/consumer/src/consumer.ts" && e.target === "repos/library/src/api.ts")); + assert.equal(existsSync(join(parent, "graft", "workspace.json")), false, "the parent owns a single graph"); + assert.equal(existsSync(join(library, "graft")), false, "the linked target needs no separate index"); + assert.deepEqual(readBuildConfig(parent), { followNestedRepos: true }); + assert.ok(isClean(probeDrift(parent, join(parent, "graft"))!)); + + const query = runCli(["ask", "linkedValue", parent, "--source"]); + assert.equal(query.status, 0, query.describe()); + assert.match(query.stdout, /repos\/library\/src\/api\.ts/); + write(library, "src/added.ts", "export function addedLinkedValue() { return 2; }\n"); + assert.ok(probeDrift(parent, join(parent, "graft"))!.added.includes("repos/library/src/added.ts")); + assert.equal((await ensureFreshGraph(parent)).refreshed, true, "automatic refresh follows the saved option"); + assert.ok(graphOf(parent).nodes.some((n) => n.id === "repos/library/src/added.ts#addedLinkedValue")); + + expectCliOk(["build", parent]); + assert.ok(graphOf(parent).nodes.some((n) => n.id === "repos/library/src/added.ts#addedLinkedValue")); + expectCliOk(["build", parent, "--no-follow-nested-repos"]); + assert.equal(graphOf(parent).nodes.length, 0); + } finally { + rmSync(parent, { recursive: true, force: true }); + rmSync(checkout, { recursive: true, force: true }); + rmSync(library, { recursive: true, force: true }); + } +}); + +test("explicit ignored repository mounts preserve parent and child ignores and refresh only selected repos", async () => { + const parent = tmpRepo("ignored-links-parent"); + const targets = [tmpRepo("ignored-widget"), tmpRepo("ignored-api"), tmpRepo("ignored-other")]; + const names = ["widget", "databraid-api", "other"]; + const selected = names.slice(0, 2).map((name) => `source-material/historical-repos/${name}`); + const whitelist = selected.flatMap((path) => ["--only-dir", path]); + const ignore = "/source-material/\n/graft/\n/.graft/\n"; + try { + gitRun(parent, ["init", "-q"]); + write(parent, ".gitignore", ignore); + write(parent, "root.ts", "export const excludedRoot = 1;\n"); + mkdirSync(join(parent, "source-material", "historical-repos"), { recursive: true }); + for (const [i, target] of targets.entries()) { + gitRun(target, ["init", "-q"]); + write(target, ".gitignore", "*.generated.ts\n"); + write(target, "src/value.ts", `export function value${i}() { return ${i}; }\n`); + write(target, "src/ignored.generated.ts", "export const ignoredValue = 1;\n"); + symlinkSync(target, join(parent, "source-material", "historical-repos", names[i]), process.platform === "win32" ? "junction" : "dir"); + } + expectCliOk(["build", parent, "--follow-nested-repos", ...whitelist]); + assert.equal(graphOf(parent).nodes.length, 0, "parent ignores remain authoritative without an explicit override"); + expectCliOk(["build", parent, "--include-ignored-repos", ...whitelist]); + const graph = graphOf(parent); + assert.ok(graph.nodes.some((n) => n.id === `${selected[0]}/src/value.ts#value0`)); + assert.ok(graph.nodes.some((n) => n.id === `${selected[1]}/src/value.ts#value1`)); + assert.ok(graph.nodes.every((n) => selected.some((path) => n.path.startsWith(`${path}/`)))); + assert.ok(!graph.nodes.some((n) => n.path.endsWith("ignored.generated.ts")), "the child ignore rules still apply"); + assert.equal(readFileSync(join(parent, ".gitignore"), "utf8"), ignore); + assert.deepEqual(readBuildConfig(parent), { followNestedRepos: true, includeIgnoredRepos: selected }); + assert.ok(isClean(probeDrift(parent, join(parent, "graft"))!)); + + write(targets[0], "src/added.ts", "export function addedIgnoredMountValue() { return 5; }\n"); + assert.equal((await ensureFreshGraph(parent)).refreshed, true); + assert.ok(graphOf(parent).nodes.some((n) => n.id === `${selected[0]}/src/added.ts#addedIgnoredMountValue`)); + assert.ok(graphOf(parent).nodes.every((n) => selected.some((path) => n.path.startsWith(`${path}/`)))); + + expectCliOk(["build", parent]); + assert.ok(graphOf(parent).nodes.some((n) => n.id === `${selected[0]}/src/added.ts#addedIgnoredMountValue`)); + assert.ok(graphOf(parent).nodes.every((n) => selected.some((path) => n.path.startsWith(`${path}/`))), "a plain rebuild keeps only the selected repos"); + expectCliOk(["build", parent, "--no-follow-nested-repos", ...whitelist]); + assert.equal(graphOf(parent).nodes.length, 0); + expectCliOk(["build", parent, "--follow-nested-repos", "--no-include-ignored-repos", ...whitelist]); + assert.equal(graphOf(parent).nodes.length, 0); + assert.deepEqual(readBuildConfig(parent)?.includeIgnoredRepos, []); + + const repoFlags = selected.flatMap((path) => ["--include-repo", path]); + expectCliOk(["build", parent, "--all-dirs", ...repoFlags]); + const combined = graphOf(parent); + assert.ok(combined.nodes.some((n) => n.path === "root.ts"), "normal project source is indexed alongside selected repos"); + assert.ok(combined.nodes.some((n) => n.path === `${selected[0]}/src/value.ts`)); + assert.ok(combined.nodes.some((n) => n.path === `${selected[1]}/src/value.ts`)); + assert.ok(!combined.nodes.some((n) => n.path.includes("historical-repos/other/"))); + assert.ok(!combined.nodes.some((n) => n.path.endsWith("ignored.generated.ts"))); + write(parent, "src/added.ts", "export const newProjectValue = 10;\n"); + write(targets[1], "src/added.ts", "export const newReferenceValue = 20;\n"); + assert.equal((await ensureFreshGraph(parent)).refreshed, true); + assert.ok(graphOf(parent).nodes.some((n) => n.path === "src/added.ts")); + assert.ok(graphOf(parent).nodes.some((n) => n.path === `${selected[1]}/src/added.ts`)); + expectCliOk(["build", parent]); + assert.ok(graphOf(parent).nodes.some((n) => n.path === "root.ts")); + assert.ok(graphOf(parent).nodes.some((n) => n.path === `${selected[0]}/src/value.ts`)); + + for (const args of [ + ["build", parent, "--include-ignored-repos"], + ["build", parent, "--include-ignored-repos", "--only-dir", "source-material"], + ["build", parent, "--include-ignored-repos", "--only-dir", `../${targets[0].split(/[\\/]/).pop()}`], + ]) { + const result = runCli(args); + assert.equal(result.status, 1, result.describe()); + assert.match(result.stderr, /include-ignored-repos/); + } + for (const args of [ + ["build", parent, "--all-dirs", "--only-dir", "src"], + ["build", parent, "--include-repo", "source-material"], + ["build", parent, "--include-repo", selected[0], "--no-include-ignored-repos"], + ["build", parent, "--include-repo", selected[0], "--no-follow-nested-repos"], + ]) { + const result = runCli(args); + assert.equal(result.status, 1, result.describe()); + } + assert.equal(readFileSync(join(parent, ".gitignore"), "utf8"), ignore); + } finally { + rmSync(parent, { recursive: true, force: true }); + for (const target of targets) rmSync(target, { recursive: true, force: true }); + } +}); + +for (const ignored of [false, true]) { + test(`refresh detects retargeted ${ignored ? "explicit ignored" : "visible"} repository links with identical file stats`, async () => { + const parent = tmpRepo("retarget-parent"); + const targets = [tmpRepo("retarget-before"), tmpRepo("retarget-after")]; + const mount = join(parent, "refs", "library"); + const linkType = process.platform === "win32" ? "junction" : "dir"; + try { + gitRun(parent, ["init", "-q"]); + write(parent, "root.ts", "export const rootValue = 1;\n"); + if (ignored) write(parent, ".gitignore", "/refs/\n"); + for (const [i, target] of targets.entries()) { + gitRun(target, ["init", "-q"]); + write(target, "value.ts", `export function ${i ? "after" : "prior"}() { return 1; }\n`); + utimesSync(join(target, "value.ts"), 1_700_000_000, 1_700_000_000); + } + mkdirSync(join(parent, "refs")); + symlinkSync(targets[0], mount, linkType); + expectCliOk(["build", parent, "--follow-nested-repos", ...(ignored ? ["--include-repo", "refs/library"] : [])]); + assert.ok(graphOf(parent).nodes.some((n) => n.id === "refs/library/value.ts#prior")); + assert.ok(isClean(probeDrift(parent, join(parent, "graft"))!)); + assert.equal((await ensureFreshGraph(parent)).refreshed, false); + + unlinkSync(mount); + symlinkSync(targets[1], mount, linkType); + const drift = probeDrift(parent, join(parent, "graft")); + assert.ok(drift === null || !isClean(drift), "a new checkout cannot inherit the old stat fast path"); + assert.equal((await ensureFreshGraph(parent)).refreshed, true); + const graph = graphOf(parent); + assert.ok(graph.nodes.some((n) => n.id === "refs/library/value.ts#after")); + assert.ok(!graph.nodes.some((n) => n.id === "refs/library/value.ts#prior")); + assert.ok(graph.nodes.some((n) => n.path === "root.ts")); + assert.equal((await ensureFreshGraph(parent)).refreshed, false, "the new identity is persisted"); + + // Old fingerprints still retain their filters but need one identity-aware rebuild. + const outDir = join(parent, "graft"); + const legacy = readFingerprint(outDir)!; + delete legacy.repositoryTargets; + writeFileSync(fingerprintPath(outDir), JSON.stringify(legacy)); + assert.equal((await ensureFreshGraph(parent)).refreshed, true); + assert.equal((await ensureFreshGraph(parent)).refreshed, false); + } finally { + rmSync(parent, { recursive: true, force: true }); + for (const target of targets) rmSync(target, { recursive: true, force: true }); + } + }); +} diff --git a/test/graph-only-dir.test.ts b/test/graph-only-dir.test.ts index 153e804ca..bcec60449 100644 --- a/test/graph-only-dir.test.ts +++ b/test/graph-only-dir.test.ts @@ -66,6 +66,17 @@ test("--only-dir limits the walk, records the whitelist in the fingerprint, and // excluded src/b and top.ts are NOT reported as phantom "added" drift. const drift = probeDrift(d, join(d, "graft")); assert.ok(drift && isClean(drift), "excluded files must not read as drift"); + + runCli(["build", d]); + assert.ok(graphOf(d)!.nodes.every((n) => n.path.startsWith("src/a/")), "a plain rebuild retains the saved selection"); + assert.deepEqual(readFingerprint(join(d, "graft"))?.onlyDirs, ["src/a"]); + + runCli(["build", d, "--all-dirs"]); + assert.ok(graphOf(d)!.nodes.some((n) => n.path === "src/b/b.ts")); + assert.ok(graphOf(d)!.nodes.some((n) => n.path === "top.ts")); + assert.equal(readFingerprint(join(d, "graft"))?.onlyDirs, undefined); + runCli(["build", d]); + assert.ok(graphOf(d)!.nodes.some((n) => n.path === "top.ts")); } finally { rmSync(d, { recursive: true, force: true }); } diff --git a/test/ingest-fs.test.ts b/test/ingest-fs.test.ts index d4a16981f..cfa3a9d82 100644 --- a/test/ingest-fs.test.ts +++ b/test/ingest-fs.test.ts @@ -571,3 +571,75 @@ test("walkDir on an ordinary directory is unchanged when siblings are symlink fi rmSync(dir, { recursive: true, force: true }); } }); + +test("followNestedRepos follows visible directory links to repos, honoring child ignores and parent skips", () => { + const parent = fixture("linked-parent"); + const child = fixture("linked-child"); + const plain = mkdtempSync(join(tmpdir(), "graft-linked-plain-")); + try { + write(parent, "src/root.ts"); + write(parent, ".gitignore", "repos/ignored\n"); + write(child, ".gitignore", "*.generated.ts\n"); + write(child, "src/tracked.ts"); + write(child, "src/forced.generated.ts"); + write(child, "build/handwritten.ts"); + commitAll(child, "linked child", ["src/forced.generated.ts"]); + write(child, "src/untracked.ts"); + write(child, "src/ignored.generated.ts"); + write(plain, "src/escape.ts"); + mkdirSync(join(parent, "repos")); + symlinkDirectory(child, join(parent, "repos", "tracked")); + runGit(parent, ["add", "repos/tracked"]); + symlinkDirectory(child, join(parent, "repos", "untracked")); + symlinkDirectory(child, join(parent, "repos", "ignored")); + symlinkDirectory(child, join(parent, "vendor")); + symlinkDirectory(child, join(parent, ".hidden")); + symlinkDirectory(plain, join(parent, "plain-link")); + + assert.deepEqual(walked(parent), ["src/root.ts"]); + assert.deepEqual(walked(parent, undefined, true), ["src/root.ts"], "following submodules alone does not follow linked repos"); + const follow = (includes?: ReadonlySet) => + walkDir(parent, includes, { followNestedRepos: true }) + .map((p) => relative(parent, p).replace(/\\/g, "/")).sort(); + assert.deepEqual(follow(), [ + "repos/tracked/src/forced.generated.ts", + "repos/tracked/src/tracked.ts", + "repos/tracked/src/untracked.ts", + "repos/untracked/src/forced.generated.ts", + "repos/untracked/src/tracked.ts", + "repos/untracked/src/untracked.ts", + "src/root.ts", + ]); + assert.ok(follow(new Set(["vendor", "build"])).includes("vendor/build/handwritten.ts")); + } finally { + rmSync(parent, { recursive: true, force: true }); + rmSync(child, { recursive: true, force: true }); + rmSync(plain, { recursive: true, force: true }); + } +}); + +test("followNestedRepos stops symlink cycles between repos and skips broken and file links", { timeout: 5_000 }, (t) => { + const parent = fixture("linked-cycle-parent"); + const child = fixture("linked-cycle-child"); + try { + write(parent, "src/root.ts"); + write(child, "src/child.ts"); + symlinkDirectory(child, join(parent, "child")); + symlinkDirectory(parent, join(child, "back")); + symlinkDirectory(parent, join(parent, "self")); + try { + symlinkDirectory(join(parent, "missing"), join(parent, "broken")); + symlinkSync(join(child, "src", "child.ts"), join(parent, "file.ts")); + } catch (err) { + t.diagnostic(`cannot create dangling/file links: ${err instanceof Error ? err.message : err}`); + } + assert.deepEqual( + walkDir(parent, undefined, { followNestedRepos: true }) + .map((p) => relative(parent, p).replace(/\\/g, "/")).sort(), + ["child/src/child.ts", "src/root.ts"], + ); + } finally { + rmSync(parent, { recursive: true, force: true }); + rmSync(child, { recursive: true, force: true }); + } +});