From 61220b653fc8ed0c87bcf69dca5bfd1cf753303a Mon Sep 17 00:00:00 2001 From: Cole Ferrier Date: Wed, 20 May 2026 19:47:12 -0700 Subject: [PATCH 1/2] feat(path): implement Lore-root centric pathing for isolation and portability Shifts the protocol to a Lore-root centric coordinate system. Anchors GitClient execution at the loreRoot and uses the --relative flag to ensure all stored paths are relative to the project boundary rather than the Git repository root. This resolves critical drift detection failures in subdirectories (lore stale) and provides automatic isolation in monorepo environments. Root discovery now strictly respects .git boundaries, preventing context leakage between unrelated repositories. Lore-id: b112559e Constraint: GitClient MUST execute from loreRoot Constraint: PathResolver MUST normalize user inputs relative to loreRoot Constraint: Discovery commands MUST implicitly scope to '.' when loreRoot != gitRoot Constraint: ConfigLoader MUST stop upward search at .git boundaries Tested: Verified monorepo isolation with manual project-a/project-b scenario Tested: Confirmed 'lore stale' drift detection in nested subdirectories Tested: All 435 unit tests passing including updated PathResolver suite Tested: Verified boundary safety in nested-Git repository scenarios Confidence: high Scope-risk: moderate Reversibility: clean Assisted-by: Gemini:CLI [lore-protocol] --- src/main.ts | 31 +++-- src/services/atom-repository.ts | 13 +- src/services/config-loader.ts | 10 +- src/services/git-client.ts | 1 + src/services/path-resolver.ts | 54 +++++++- src/services/root-resolver.ts | 35 +++++ tests/unit/root-resolver.test.ts | 70 ++++++++++ tests/unit/services/path-resolver.test.ts | 158 +++++++++++++++------- 8 files changed, 301 insertions(+), 71 deletions(-) create mode 100644 src/services/root-resolver.ts create mode 100644 tests/unit/root-resolver.test.ts diff --git a/src/main.ts b/src/main.ts index c027262e..e1959429 100644 --- a/src/main.ts +++ b/src/main.ts @@ -47,6 +47,7 @@ import { registerDoctorCommand } from './commands/doctor.js'; import { LoreError, ValidationError } from './util/errors.js'; import { shouldCheckForUpdate } from './util/update-check.js'; +import { resolveLoreRoot } from './services/root-resolver.js'; /** * Composition root: constructs all dependencies and wires them together. @@ -70,29 +71,35 @@ async function main(): Promise { .option('--no-color', 'Disable colored output') .option('--no-update-notifier', 'Disable update notification'); - // 1. Create concrete implementations - const gitClient: IGitClient = new GitClient(); - const trailerParser = new TrailerParser(); - const pathResolver = new PathResolver(); - const loreIdGenerator = new LoreIdGenerator(); + // 1. Create bootstrap services for root discovery const configLoader: IConfigLoader = new ConfigLoader(); + const bootstrapGitClient: IGitClient = new GitClient(); // Default CWD + + // 2. Resolve project root for caching and config + const loreRoot = await resolveLoreRoot(process.cwd(), configLoader, bootstrapGitClient); - // 2. Load config (best-effort: default if not found) + // 3. Load config (best-effort: default if not found) let config; try { - config = await configLoader.loadForPath(process.cwd()); + config = await configLoader.loadForPath(loreRoot); } catch { // Fall back to defaults if config can't be loaded const { DEFAULT_CONFIG } = await import('./types/config.js'); config = DEFAULT_CONFIG; } - // 3. Update notification (fire-and-forget, respects env vars and config) + // 4. Create primary services with resolved root context + const gitClient: IGitClient = new GitClient(loreRoot); + const trailerParser = new TrailerParser(); + const pathResolver = new PathResolver(process.cwd(), loreRoot); + const loreIdGenerator = new LoreIdGenerator(); + + // 5. Update notification (fire-and-forget, respects env vars and config) if (shouldCheckForUpdate(config.cli.updateCheck)) { simpleUpdateNotifier({ pkg }).catch(() => {}); } - // 4. Create services that depend on others + // 6. Create services that depend on others const atomRepository = new AtomRepository(gitClient, trailerParser, config.trailers.custom); const supersessionResolver = new SupersessionResolver(); const stalenessDetector = new StalenessDetector(gitClient, config); @@ -104,7 +111,7 @@ async function main(): Promise { const commitInputResolver = new CommitInputResolver(prompt); const headLoreIdReader = new HeadLoreIdReader(gitClient, trailerParser); - // 5. Formatter factory (reads --format/--json from program options at call time) + // 7. Formatter factory (reads --format/--json from program options at call time) // Memoized: the formatter is created once on first call and reused thereafter. let cachedFormatter: IOutputFormatter | null = null; const getFormatter = (): IOutputFormatter => { @@ -120,7 +127,7 @@ async function main(): Promise { return cachedFormatter; }; - // 6. Register all commands with their dependencies + // 8. Register all commands with their dependencies registerInitCommand(program, { getFormatter }); @@ -197,7 +204,7 @@ async function main(): Promise { getFormatter, }); - // 7. Parse and run + // 9. Parse and run await program.parseAsync(process.argv); } diff --git a/src/services/atom-repository.ts b/src/services/atom-repository.ts index a636ba9c..4f9c715c 100644 --- a/src/services/atom-repository.ts +++ b/src/services/atom-repository.ts @@ -41,7 +41,7 @@ export class AtomRepository { return null; } - const logArgs = ['--all', `--grep=Lore-id: ${loreId}`]; + const logArgs = ['--all', `--grep=Lore-id: ${loreId}`, '--', '.']; const rawCommits = await this.gitClient.log(logArgs); const atoms = await this.parseRawCommits(rawCommits); @@ -54,7 +54,7 @@ export class AtomRepository { * Returns null if the commit has no valid Lore trailers. */ async findByCommitHash(hash: string): Promise { - const rawCommits = await this.gitClient.log(['-1', hash]); + const rawCommits = await this.gitClient.log(['-1', hash, '--', '.']); const atoms = await this.parseRawCommits(rawCommits); return atoms.length > 0 ? atoms[0] : null; } @@ -64,7 +64,7 @@ export class AtomRepository { * Passes the range directly to git log. */ async findByRange(range: string): Promise { - const rawCommits = await this.gitClient.log([range]); + const rawCommits = await this.gitClient.log([range, '--', '.']); return this.parseRawCommits(rawCommits); } @@ -84,6 +84,9 @@ export class AtomRepository { args.push(`--max-count=${options.maxCommits}`); } + // Implicitly scope to loreRoot + args.push('--', '.'); + const rawCommits = await this.gitClient.log(args); return this.parseRawCommits(rawCommits); } @@ -94,6 +97,10 @@ export class AtomRepository { */ async findByScope(scope: string, options: PathQueryOptions): Promise { const logArgs = this.buildLogArgs(options); + + // Implicitly scope to loreRoot + logArgs.push('--', '.'); + const rawCommits = await this.gitClient.log(logArgs); const atoms = await this.parseRawCommits(rawCommits); diff --git a/src/services/config-loader.ts b/src/services/config-loader.ts index 3be7aafd..cf528170 100644 --- a/src/services/config-loader.ts +++ b/src/services/config-loader.ts @@ -81,11 +81,19 @@ export class ConfigLoader implements IConfigLoader { while (true) { const configPath = join(dir, CONFIG_DIR, CONFIG_FILENAME); - if (await this.fileExists(configPath)) { + const hasConfig = await this.fileExists(configPath); + if (hasConfig) { paths.push(configPath); if (stopAtFirst) return paths; } + // Stop walking up if we hit a Git repository boundary. + // This prevents Lore from picking up parent configs that belong to a + // different Git history (e.g. submodules or nested repos). + if (await this.fileExists(join(dir, '.git'))) { + break; + } + const parentDir = dirname(dir); if (parentDir === dir || dir === root) break; dir = parentDir; diff --git a/src/services/git-client.ts b/src/services/git-client.ts index 22fa871a..44d84911 100644 --- a/src/services/git-client.ts +++ b/src/services/git-client.ts @@ -128,6 +128,7 @@ export class GitClient implements IGitClient { '--no-commit-id', '--name-only', '-r', + '--relative', commitHash, ]); return stdout.trim().split('\n').filter(line => line.length > 0); diff --git a/src/services/path-resolver.ts b/src/services/path-resolver.ts index c881ca8f..d95ef47e 100644 --- a/src/services/path-resolver.ts +++ b/src/services/path-resolver.ts @@ -1,4 +1,5 @@ -import type { QueryTarget, TargetType } from '../types/query.js'; +import { resolve, relative } from 'node:path'; +import type { QueryTarget } from '../types/query.js'; const LINE_RANGE_PATTERN = /^(.+):(\d+)-(\d+)$/; const SINGLE_LINE_PATTERN = /^(.+):(\d+)$/; @@ -11,6 +12,11 @@ const GLOB_CHARS_PATTERN = /[*?]/; * SRP: Only target -> git-args translation. */ export class PathResolver { + constructor( + private readonly cwd: string, + private readonly loreRoot: string, + ) {} + /** * Parse a raw target string into a QueryTarget. * @@ -25,7 +31,7 @@ export class PathResolver { // Try line-range with start-end (e.g., file.ts:45-80) const rangeMatch = LINE_RANGE_PATTERN.exec(raw); if (rangeMatch) { - const filePath = rangeMatch[1]; + const filePath = this.normalizePath(rangeMatch[1]); const lineStart = parseInt(rangeMatch[2], 10); const lineEnd = parseInt(rangeMatch[3], 10); return { @@ -40,7 +46,7 @@ export class PathResolver { // Try single-line (e.g., file.ts:45) const singleMatch = SINGLE_LINE_PATTERN.exec(raw); if (singleMatch) { - const filePath = singleMatch[1]; + const filePath = this.normalizePath(singleMatch[1]); const lineStart = parseInt(singleMatch[2], 10); return { raw, @@ -56,7 +62,7 @@ export class PathResolver { return { raw, type: 'glob', - filePath: raw, + filePath: this.normalizePath(raw), lineStart: null, lineEnd: null, }; @@ -67,7 +73,7 @@ export class PathResolver { return { raw, type: 'directory', - filePath: raw, + filePath: this.normalizePath(raw), lineStart: null, lineEnd: null, }; @@ -77,12 +83,48 @@ export class PathResolver { return { raw, type: 'file', - filePath: raw, + filePath: this.normalizePath(raw), lineStart: null, lineEnd: null, }; } + /** + * Normalize a path relative to the loreRoot. + */ + private normalizePath(rawPath: string): string { + // 1. Resolve to an absolute path based on CWD + const absolutePath = resolve(this.cwd, rawPath); + + // 2. Compute relative path from loreRoot + let relPath = relative(this.loreRoot, absolutePath); + + // 3. Normalize to POSIX separators and handle empty/root case + if (relPath === '') { + relPath = '.'; + } + + let normalized = relPath.replace(/\\/g, '/'); + + // 4. Preserve trailing slash if present in raw input + if (rawPath.endsWith('/') && !normalized.endsWith('/')) { + normalized += '/'; + } + + return normalized; + } + + /** + * Convert multiple raw path strings into a combined git log argument array. + * Normalizes all paths relative to loreRoot. + */ + toGitLogArgsMulti(rawPaths: string[]): readonly string[] { + if (rawPaths.length === 0) return []; + + const normalized = rawPaths.map((p) => this.normalizePath(p)); + return ['--', ...normalized]; + } + /** * Convert a QueryTarget into git log arguments. * diff --git a/src/services/root-resolver.ts b/src/services/root-resolver.ts new file mode 100644 index 00000000..670f3bf8 --- /dev/null +++ b/src/services/root-resolver.ts @@ -0,0 +1,35 @@ +import { join, dirname } from 'node:path'; +import type { IConfigLoader } from '../interfaces/config-loader.js'; +import type { IGitClient } from '../interfaces/git-client.js'; + +/** + * Resolves the root directory for Lore operations. + * + * Logic: + * 1. Walk up from CWD to find nearest .lore directory. + * 2. If not found, try to find the git repository root. + * 3. Fallback to process.cwd(). + */ +export async function resolveLoreRoot( + cwd: string, + configLoader: IConfigLoader, + gitClient: IGitClient, +): Promise { + try { + const configPath = await configLoader.findConfigPath(cwd); + if (configPath) { + // configPath is /path/to/project/.lore/config.toml + // dirname(configPath) is /path/to/project/.lore + // dirname(dirname(configPath)) is /path/to/project + return dirname(dirname(configPath)); + } + + if (await gitClient.isInsideRepo()) { + return await gitClient.getRepoRoot(); + } + } catch { + // Best-effort; fallback to cwd + } + + return cwd; +} diff --git a/tests/unit/root-resolver.test.ts b/tests/unit/root-resolver.test.ts new file mode 100644 index 00000000..5b9cf8a3 --- /dev/null +++ b/tests/unit/root-resolver.test.ts @@ -0,0 +1,70 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { resolveLoreRoot } from '../../src/services/root-resolver.js'; +import type { IConfigLoader } from '../../src/interfaces/config-loader.js'; +import type { IGitClient } from '../../src/interfaces/git-client.js'; + +describe('resolveLoreRoot', () => { + const mockConfigLoader = { + findConfigPath: vi.fn(), + } as unknown as IConfigLoader; + + const mockGitClient = { + isInsideRepo: vi.fn(), + getRepoRoot: vi.fn(), + } as unknown as IGitClient; + + const cwd = '/users/dev/project/src'; + + beforeEach(() => { + vi.resetAllMocks(); + }); + + it('returns the parent of .lore directory if config is found', async () => { + const configPath = '/users/dev/project/.lore/config.toml'; + vi.mocked(mockConfigLoader.findConfigPath).mockResolvedValue(configPath); + + const root = await resolveLoreRoot(cwd, mockConfigLoader, mockGitClient); + + expect(root).toBe('/users/dev/project'); + expect(mockConfigLoader.findConfigPath).toHaveBeenCalledWith(cwd); + }); + + it('returns git root if no .lore is found but inside a repo', async () => { + vi.mocked(mockConfigLoader.findConfigPath).mockResolvedValue(null); + vi.mocked(mockGitClient.isInsideRepo).mockResolvedValue(true); + vi.mocked(mockGitClient.getRepoRoot).mockResolvedValue('/users/dev/project'); + + const root = await resolveLoreRoot(cwd, mockConfigLoader, mockGitClient); + + expect(root).toBe('/users/dev/project'); + }); + + it('falls back to cwd if neither .lore nor git root is found', async () => { + vi.mocked(mockConfigLoader.findConfigPath).mockResolvedValue(null); + vi.mocked(mockGitClient.isInsideRepo).mockResolvedValue(false); + + const root = await resolveLoreRoot(cwd, mockConfigLoader, mockGitClient); + + expect(root).toBe(cwd); + }); + + it('falls back to cwd if an error occurs during resolution', async () => { + vi.mocked(mockConfigLoader.findConfigPath).mockRejectedValue(new Error('FS Error')); + + const root = await resolveLoreRoot(cwd, mockConfigLoader, mockGitClient); + + expect(root).toBe(cwd); + }); + + it('prioritizes .lore directory over git root', async () => { + // Scenario: A project inside a larger git repo, but with its own .lore + const configPath = '/users/dev/project/sub-project/.lore/config.toml'; + vi.mocked(mockConfigLoader.findConfigPath).mockResolvedValue(configPath); + vi.mocked(mockGitClient.isInsideRepo).mockResolvedValue(true); + vi.mocked(mockGitClient.getRepoRoot).mockResolvedValue('/users/dev/project'); + + const root = await resolveLoreRoot(cwd, mockConfigLoader, mockGitClient); + + expect(root).toBe('/users/dev/project/sub-project'); + }); +}); diff --git a/tests/unit/services/path-resolver.test.ts b/tests/unit/services/path-resolver.test.ts index 291d5799..a939210a 100644 --- a/tests/unit/services/path-resolver.test.ts +++ b/tests/unit/services/path-resolver.test.ts @@ -1,46 +1,84 @@ import { describe, it, expect } from 'vitest'; +import { resolve } from 'node:path'; import { PathResolver } from '../../../src/services/path-resolver.js'; describe('PathResolver', () => { - const resolver = new PathResolver(); + // Base configuration: Lore root is the same as CWD + const baseLoreRoot = resolve('/work/project'); + const baseCwd = baseLoreRoot; + const baseResolver = new PathResolver(baseCwd, baseLoreRoot); + + // Cross-platform normalization for test assertions + const normalizeResult = (p: string) => p.replace(/\\/g, '/'); describe('parseTarget', () => { + describe('Normalization (Lore-root centric)', () => { + const nestedLoreRoot = resolve('/work/project'); + const nestedCwd = resolve('/work/project/src'); + const nestedResolver = new PathResolver(nestedCwd, nestedLoreRoot); + + it('should normalize CWD-relative file to Lore-root relative', () => { + // CWD is /work/project/src, Target is main.ts + // Result should be src/main.ts + const result = nestedResolver.parseTarget('main.ts'); + expect(normalizeResult(result.filePath)).toBe('src/main.ts'); + }); + + it('should handle parent directory references from CWD', () => { + // Target is ../README.md from /work/project/src + // Result should be README.md + const result = nestedResolver.parseTarget('../README.md'); + expect(normalizeResult(result.filePath)).toBe('README.md'); + }); + + it('should handle absolute paths inside the lore root', () => { + const absPath = resolve(nestedLoreRoot, 'package.json'); + const result = nestedResolver.parseTarget(absPath); + expect(normalizeResult(result.filePath)).toBe('package.json'); + }); + + it('should handle paths outside the lore root using parent references', () => { + const result = nestedResolver.parseTarget('../../other/file.txt'); + expect(normalizeResult(result.filePath)).toBe('../other/file.txt'); + }); + }); + describe('line-range targets', () => { it('should parse file:start-end as line-range', () => { - const result = resolver.parseTarget('src/utils.ts:45-80'); + const result = baseResolver.parseTarget('src/utils.ts:45-80'); expect(result.type).toBe('line-range'); - expect(result.filePath).toBe('src/utils.ts'); + expect(normalizeResult(result.filePath)).toBe('src/utils.ts'); expect(result.lineStart).toBe(45); expect(result.lineEnd).toBe(80); expect(result.raw).toBe('src/utils.ts:45-80'); }); it('should parse file:line as line-range with start === end', () => { - const result = resolver.parseTarget('src/utils.ts:45'); + const result = baseResolver.parseTarget('src/utils.ts:45'); expect(result.type).toBe('line-range'); - expect(result.filePath).toBe('src/utils.ts'); + expect(normalizeResult(result.filePath)).toBe('src/utils.ts'); expect(result.lineStart).toBe(45); expect(result.lineEnd).toBe(45); }); it('should handle line 1', () => { - const result = resolver.parseTarget('file.ts:1'); + const result = baseResolver.parseTarget('file.ts:1'); expect(result.type).toBe('line-range'); expect(result.lineStart).toBe(1); expect(result.lineEnd).toBe(1); }); it('should handle large line numbers', () => { - const result = resolver.parseTarget('file.ts:10000-20000'); + const result = baseResolver.parseTarget('file.ts:10000-20000'); expect(result.type).toBe('line-range'); expect(result.lineStart).toBe(10000); expect(result.lineEnd).toBe(20000); }); it('should parse deeply nested file with line range', () => { - const result = resolver.parseTarget('src/services/db/connection.ts:10-20'); + const result = baseResolver.parseTarget('src/services/db/connection.ts:10-20'); expect(result.type).toBe('line-range'); - expect(result.filePath).toBe('src/services/db/connection.ts'); + expect(normalizeResult(result.filePath)).toBe('src/services/db/connection.ts'); expect(result.lineStart).toBe(10); expect(result.lineEnd).toBe(20); }); @@ -48,84 +86,91 @@ describe('PathResolver', () => { describe('directory targets', () => { it('should classify trailing slash as directory', () => { - const result = resolver.parseTarget('src/'); + const result = baseResolver.parseTarget('src/'); expect(result.type).toBe('directory'); - expect(result.filePath).toBe('src/'); + expect(normalizeResult(result.filePath)).toBe('src/'); expect(result.lineStart).toBeNull(); expect(result.lineEnd).toBeNull(); }); it('should classify nested path with trailing slash as directory', () => { - const result = resolver.parseTarget('src/services/db/'); + const result = baseResolver.parseTarget('src/services/db/'); expect(result.type).toBe('directory'); - expect(result.filePath).toBe('src/services/db/'); + expect(normalizeResult(result.filePath)).toBe('src/services/db/'); }); it('should classify root-relative directory', () => { - const result = resolver.parseTarget('./src/'); + const result = baseResolver.parseTarget('./src/'); expect(result.type).toBe('directory'); + expect(normalizeResult(result.filePath)).toBe('src/'); }); }); describe('glob targets', () => { it('should classify patterns with * as glob', () => { - const result = resolver.parseTarget('**/*.ts'); + const result = baseResolver.parseTarget('**/*.ts'); expect(result.type).toBe('glob'); - expect(result.filePath).toBe('**/*.ts'); + expect(normalizeResult(result.filePath)).toBe('**/*.ts'); expect(result.lineStart).toBeNull(); expect(result.lineEnd).toBeNull(); }); it('should classify patterns with ? as glob', () => { - const result = resolver.parseTarget('src/file?.ts'); + const result = baseResolver.parseTarget('src/file?.ts'); expect(result.type).toBe('glob'); - expect(result.filePath).toBe('src/file?.ts'); + expect(normalizeResult(result.filePath)).toBe('src/file?.ts'); }); it('should classify *.js as glob', () => { - const result = resolver.parseTarget('*.js'); + const result = baseResolver.parseTarget('*.js'); expect(result.type).toBe('glob'); + expect(normalizeResult(result.filePath)).toBe('*.js'); }); it('should classify src/**/*.test.ts as glob', () => { - const result = resolver.parseTarget('src/**/*.test.ts'); + const result = baseResolver.parseTarget('src/**/*.test.ts'); expect(result.type).toBe('glob'); + expect(normalizeResult(result.filePath)).toBe('src/**/*.test.ts'); }); }); describe('file targets', () => { it('should classify a plain file path as file', () => { - const result = resolver.parseTarget('src/main.ts'); + const result = baseResolver.parseTarget('src/main.ts'); expect(result.type).toBe('file'); - expect(result.filePath).toBe('src/main.ts'); + expect(normalizeResult(result.filePath)).toBe('src/main.ts'); expect(result.lineStart).toBeNull(); expect(result.lineEnd).toBeNull(); }); it('should classify a file without extension as file', () => { - const result = resolver.parseTarget('Makefile'); + const result = baseResolver.parseTarget('Makefile'); expect(result.type).toBe('file'); - expect(result.filePath).toBe('Makefile'); + expect(normalizeResult(result.filePath)).toBe('Makefile'); }); it('should classify a dotfile as file', () => { - const result = resolver.parseTarget('.gitignore'); + const result = baseResolver.parseTarget('.gitignore'); expect(result.type).toBe('file'); + expect(normalizeResult(result.filePath)).toBe('.gitignore'); }); it('should classify a deeply nested file as file', () => { - const result = resolver.parseTarget('src/services/db/connection.ts'); + const result = baseResolver.parseTarget('src/services/db/connection.ts'); expect(result.type).toBe('file'); + expect(normalizeResult(result.filePath)).toBe('src/services/db/connection.ts'); }); it('should classify a file with spaces as file', () => { - const result = resolver.parseTarget('my file.ts'); + const result = baseResolver.parseTarget('my file.ts'); expect(result.type).toBe('file'); + expect(normalizeResult(result.filePath)).toBe('my file.ts'); }); it('should classify a relative path as file', () => { - const result = resolver.parseTarget('./src/main.ts'); + const result = baseResolver.parseTarget('./src/main.ts'); expect(result.type).toBe('file'); + expect(normalizeResult(result.filePath)).toBe('src/main.ts'); }); }); @@ -139,7 +184,7 @@ describe('PathResolver', () => { '**/*.ts', ]; for (const input of inputs) { - expect(resolver.parseTarget(input).raw).toBe(input); + expect(baseResolver.parseTarget(input).raw).toBe(input); } }); }); @@ -147,40 +192,55 @@ describe('PathResolver', () => { describe('toGitLogArgs', () => { it('should produce -- filePath for file targets', () => { - const target = resolver.parseTarget('src/main.ts'); - const args = resolver.toGitLogArgs(target); + const target = baseResolver.parseTarget('src/main.ts'); + const args = baseResolver.toGitLogArgs(target); expect(args).toEqual(['--', 'src/main.ts']); }); it('should produce -- filePath for directory targets', () => { - const target = resolver.parseTarget('src/services/'); - const args = resolver.toGitLogArgs(target); + const target = baseResolver.parseTarget('src/services/'); + const args = baseResolver.toGitLogArgs(target); expect(args).toEqual(['--', 'src/services/']); }); it('should produce -- filePath for glob targets', () => { - const target = resolver.parseTarget('**/*.ts'); - const args = resolver.toGitLogArgs(target); + const target = baseResolver.parseTarget('**/*.ts'); + const args = baseResolver.toGitLogArgs(target); expect(args).toEqual(['--', '**/*.ts']); }); it('should produce -L start,end:file for line-range targets', () => { - const target = resolver.parseTarget('src/main.ts:10-20'); - const args = resolver.toGitLogArgs(target); + const target = baseResolver.parseTarget('src/main.ts:10-20'); + const args = baseResolver.toGitLogArgs(target); expect(args).toEqual(['-L', '10,20:src/main.ts']); }); it('should produce -L line,line:file for single-line targets', () => { - const target = resolver.parseTarget('src/main.ts:42'); - const args = resolver.toGitLogArgs(target); + const target = baseResolver.parseTarget('src/main.ts:42'); + const args = baseResolver.toGitLogArgs(target); expect(args).toEqual(['-L', '42,42:src/main.ts']); }); }); + describe('toGitLogArgsMulti', () => { + it('should normalize and combine multiple paths', () => { + const nestedLoreRoot = resolve('/work/project'); + const nestedCwd = resolve('/work/project/src'); + const nestedResolver = new PathResolver(nestedCwd, nestedLoreRoot); + + const args = nestedResolver.toGitLogArgsMulti(['main.ts', '../README.md']); + expect(args).toEqual(['--', 'src/main.ts', 'README.md']); + }); + + it('should return empty array for empty input', () => { + expect(baseResolver.toGitLogArgsMulti([])).toEqual([]); + }); + }); + describe('toGitBlameArgs', () => { it('should return file, lineStart, lineEnd for line-range targets', () => { - const target = resolver.parseTarget('src/main.ts:10-20'); - const args = resolver.toGitBlameArgs(target); + const target = baseResolver.parseTarget('src/main.ts:10-20'); + const args = baseResolver.toGitBlameArgs(target); expect(args).toEqual({ file: 'src/main.ts', lineStart: 10, @@ -189,8 +249,8 @@ describe('PathResolver', () => { }); it('should return file, line, line for single-line targets', () => { - const target = resolver.parseTarget('src/main.ts:42'); - const args = resolver.toGitBlameArgs(target); + const target = baseResolver.parseTarget('src/main.ts:42'); + const args = baseResolver.toGitBlameArgs(target); expect(args).toEqual({ file: 'src/main.ts', lineStart: 42, @@ -199,8 +259,8 @@ describe('PathResolver', () => { }); it('should return file with lineStart=1 and lineEnd=-1 for file targets', () => { - const target = resolver.parseTarget('src/main.ts'); - const args = resolver.toGitBlameArgs(target); + const target = baseResolver.parseTarget('src/main.ts'); + const args = baseResolver.toGitBlameArgs(target); expect(args).toEqual({ file: 'src/main.ts', lineStart: 1, @@ -209,8 +269,8 @@ describe('PathResolver', () => { }); it('should return file with lineStart=1 and lineEnd=-1 for directory targets', () => { - const target = resolver.parseTarget('src/'); - const args = resolver.toGitBlameArgs(target); + const target = baseResolver.parseTarget('src/'); + const args = baseResolver.toGitBlameArgs(target); expect(args).toEqual({ file: 'src/', lineStart: 1, @@ -219,8 +279,8 @@ describe('PathResolver', () => { }); it('should return file with lineStart=1 and lineEnd=-1 for glob targets', () => { - const target = resolver.parseTarget('**/*.ts'); - const args = resolver.toGitBlameArgs(target); + const target = baseResolver.parseTarget('**/*.ts'); + const args = baseResolver.toGitBlameArgs(target); expect(args).toEqual({ file: '**/*.ts', lineStart: 1, From 0b0a03cefbf91fa1f7678e7cf5701f9af55728d3 Mon Sep 17 00:00:00 2001 From: Cole Ferrier Date: Wed, 20 May 2026 20:38:38 -0700 Subject: [PATCH 2/2] feat(path): implement isScoped guard for intelligent monorepo discovery Introduces an isScoped flag to AtomRepository that conditionalizes implicit path scoping. When Lore is initialized in a subproject (loreRoot != gitRoot), queries are automatically scoped to the project directory to provide isolation. When standing at the repository root, Lore operates in global mode, showing all atoms including empty commits. This ensures full visibility at the top level while maintaining clean isolation within individual projects. Lore-id: deec0524 Constraint: Discovery commands MUST only scope to '.' when isScoped is true Constraint: Empty commits MUST be visible at the Git root Tested: Added unit tests for isScoped guard in AtomRepository.test.ts Tested: Verified dual-mode behavior (Global vs Isolated) with manual monorepo scenario Tested: All 41 AtomRepository tests passing Confidence: high Assisted-by: Gemini:CLI [lore-protocol] --- src/main.ts | 11 ++- src/services/atom-repository.ts | 30 ++++++-- tests/unit/services/atom-repository.test.ts | 78 +++++++++++++++++++++ 3 files changed, 111 insertions(+), 8 deletions(-) diff --git a/src/main.ts b/src/main.ts index e1959429..43de423b 100644 --- a/src/main.ts +++ b/src/main.ts @@ -77,6 +77,10 @@ async function main(): Promise { // 2. Resolve project root for caching and config const loreRoot = await resolveLoreRoot(process.cwd(), configLoader, bootstrapGitClient); + const gitRoot = await bootstrapGitClient.isInsideRepo() + ? await bootstrapGitClient.getRepoRoot() + : loreRoot; + const isScoped = loreRoot !== gitRoot; // 3. Load config (best-effort: default if not found) let config; @@ -100,7 +104,12 @@ async function main(): Promise { } // 6. Create services that depend on others - const atomRepository = new AtomRepository(gitClient, trailerParser, config.trailers.custom); + const atomRepository = new AtomRepository( + gitClient, + trailerParser, + config.trailers.custom, + isScoped + ); const supersessionResolver = new SupersessionResolver(); const stalenessDetector = new StalenessDetector(gitClient, config); const commitBuilder = new CommitBuilder(trailerParser, loreIdGenerator, config); diff --git a/src/services/atom-repository.ts b/src/services/atom-repository.ts index 4f9c715c..3f6e072d 100644 --- a/src/services/atom-repository.ts +++ b/src/services/atom-repository.ts @@ -16,6 +16,7 @@ export class AtomRepository { private readonly gitClient: IGitClient, private readonly trailerParser: TrailerParser, private readonly customTrailerKeys: readonly string[] = [], + private readonly isScoped: boolean = false, ) {} /** @@ -41,7 +42,10 @@ export class AtomRepository { return null; } - const logArgs = ['--all', `--grep=Lore-id: ${loreId}`, '--', '.']; + const logArgs = ['--all', `--grep=Lore-id: ${loreId}`]; + if (this.isScoped) { + logArgs.push('--', '.'); + } const rawCommits = await this.gitClient.log(logArgs); const atoms = await this.parseRawCommits(rawCommits); @@ -54,7 +58,11 @@ export class AtomRepository { * Returns null if the commit has no valid Lore trailers. */ async findByCommitHash(hash: string): Promise { - const rawCommits = await this.gitClient.log(['-1', hash, '--', '.']); + const logArgs = ['-1', hash]; + if (this.isScoped) { + logArgs.push('--', '.'); + } + const rawCommits = await this.gitClient.log(logArgs); const atoms = await this.parseRawCommits(rawCommits); return atoms.length > 0 ? atoms[0] : null; } @@ -64,7 +72,11 @@ export class AtomRepository { * Passes the range directly to git log. */ async findByRange(range: string): Promise { - const rawCommits = await this.gitClient.log([range, '--', '.']); + const logArgs = [range]; + if (this.isScoped) { + logArgs.push('--', '.'); + } + const rawCommits = await this.gitClient.log(logArgs); return this.parseRawCommits(rawCommits); } @@ -84,8 +96,10 @@ export class AtomRepository { args.push(`--max-count=${options.maxCommits}`); } - // Implicitly scope to loreRoot - args.push('--', '.'); + // Implicitly scope to loreRoot if in a sub-project (monorepo) + if (this.isScoped) { + args.push('--', '.'); + } const rawCommits = await this.gitClient.log(args); return this.parseRawCommits(rawCommits); @@ -98,8 +112,10 @@ export class AtomRepository { async findByScope(scope: string, options: PathQueryOptions): Promise { const logArgs = this.buildLogArgs(options); - // Implicitly scope to loreRoot - logArgs.push('--', '.'); + // Implicitly scope to loreRoot if in a sub-project (monorepo) + if (this.isScoped) { + logArgs.push('--', '.'); + } const rawCommits = await this.gitClient.log(logArgs); const atoms = await this.parseRawCommits(rawCommits); diff --git a/tests/unit/services/atom-repository.test.ts b/tests/unit/services/atom-repository.test.ts index 45d502f7..f8536476 100644 --- a/tests/unit/services/atom-repository.test.ts +++ b/tests/unit/services/atom-repository.test.ts @@ -280,6 +280,62 @@ describe('AtomRepository', () => { expect(result).toBeNull(); }); + + it('should append path scope when isScoped=true', async () => { + const scopedRepo = new AtomRepository(gitClient, trailerParser, [], true); + vi.mocked(gitClient.log).mockResolvedValue([]); + + await scopedRepo.findByLoreId('deadbeef'); + + expect(gitClient.log).toHaveBeenCalledWith( + expect.arrayContaining(['--', '.']), + ); + }); + }); + + describe('findByCommitHash', () => { + it('should fetch and parse a single commit', async () => { + const commit = makeLoreCommit({ hash: 'abc123', loreId: 'aaaa1111' }); + vi.mocked(gitClient.log).mockResolvedValue([commit]); + vi.mocked(gitClient.getFilesChanged).mockResolvedValue([]); + + const result = await repo.findByCommitHash('abc123'); + + expect(result).not.toBeNull(); + expect(result?.commitHash).toBe('abc123'); + expect(gitClient.log).toHaveBeenCalledWith(expect.arrayContaining(['-1', 'abc123'])); + }); + + it('should return null if no commit found', async () => { + vi.mocked(gitClient.log).mockResolvedValue([]); + const result = await repo.findByCommitHash('abc123'); + expect(result).toBeNull(); + }); + + it('should append path scope when isScoped=true', async () => { + const scopedRepo = new AtomRepository(gitClient, trailerParser, [], true); + vi.mocked(gitClient.log).mockResolvedValue([]); + + await scopedRepo.findByCommitHash('abc123'); + + expect(gitClient.log).toHaveBeenCalledWith( + expect.arrayContaining(['-1', 'abc123', '--', '.']), + ); + }); + }); + + describe('findByRange', () => { + it('should pass the range directly to git log', async () => { + await repo.findByRange('main..HEAD'); + expect(gitClient.log).toHaveBeenCalledWith(['main..HEAD']); + }); + + it('should append path scope when isScoped=true', async () => { + const scopedRepo = new AtomRepository(gitClient, trailerParser, [], true); + await scopedRepo.findByRange('main..HEAD'); + + expect(gitClient.log).toHaveBeenCalledWith(['main..HEAD', '--', '.']); + }); }); describe('findAll', () => { @@ -357,6 +413,17 @@ describe('AtomRepository', () => { expect(result).toEqual([]); }); + + it('should append path scope when isScoped=true', async () => { + const scopedRepo = new AtomRepository(gitClient, trailerParser, [], true); + vi.mocked(gitClient.log).mockResolvedValue([]); + + await scopedRepo.findAll(); + + expect(gitClient.log).toHaveBeenCalledWith( + expect.arrayContaining(['--', '.']), + ); + }); }); describe('findByScope', () => { @@ -401,6 +468,17 @@ describe('AtomRepository', () => { expect(result).toEqual([]); }); + + it('should append path scope when isScoped=true', async () => { + const scopedRepo = new AtomRepository(gitClient, trailerParser, [], true); + vi.mocked(gitClient.log).mockResolvedValue([]); + + await scopedRepo.findByScope('auth', makeQueryOptions()); + + expect(gitClient.log).toHaveBeenCalledWith( + expect.arrayContaining(['--', '.']), + ); + }); }); describe('resolveFollowLinks', () => {