diff --git a/packages/paths/src/archon-paths.test.ts b/packages/paths/src/archon-paths.test.ts index a4303c7957..a04c8cba1c 100644 --- a/packages/paths/src/archon-paths.test.ts +++ b/packages/paths/src/archon-paths.test.ts @@ -2,7 +2,7 @@ import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; import { homedir, tmpdir } from 'os'; import { join } from 'path'; import { existsSync } from 'fs'; -import { mkdir, rm, writeFile, lstat, readlink } from 'fs/promises'; +import { mkdir, rm, writeFile, lstat, readlink, symlink as fsSymlink } from 'fs/promises'; const isWindows = process.platform === 'win32'; @@ -36,6 +36,7 @@ import { resolveProjectRootFromCwd, ensureProjectStructure, createProjectSourceSymlink, + findMarkdownFilesRecursive, } from './archon-paths'; /** All env vars that path functions depend on */ @@ -776,3 +777,90 @@ describe('createProjectSourceSymlink', () => { expect(stats.isSymbolicLink()).toBe(true); }); }); + +// ───────────────────────────────────────────────────────────────────────── +// findMarkdownFilesRecursive — symlink handling +// +// Regression tests for #1501: symlinked .md files (e.g. team-shared +// commands made available via `~/.archon/commands/foo.md` → repo path) +// must be discovered, not silently ignored. +// ───────────────────────────────────────────────────────────────────────── + +describe.skipIf(isWindows)('findMarkdownFilesRecursive — symlinks', () => { + let tempRoot: string; + let realRepo: string; + + beforeEach(async () => { + const ts = Date.now() + Math.random(); + tempRoot = join(tmpdir(), `archon-find-md-${ts}`); + realRepo = join(tmpdir(), `archon-find-md-source-${ts}`); + await mkdir(tempRoot, { recursive: true }); + await mkdir(realRepo, { recursive: true }); + }); + + afterEach(async () => { + await rm(tempRoot, { recursive: true, force: true }); + await rm(realRepo, { recursive: true, force: true }); + }); + + test('finds .md file reached via symlink in the search root', async () => { + // Real source file in a "team repo"-shaped location + await writeFile(join(realRepo, 'team-cmd.md'), '# team command'); + + // Symlink it into the search root (same shape as sync-to-archon.sh) + await fsSymlink(join(realRepo, 'team-cmd.md'), join(tempRoot, 'team-cmd.md')); + + const found = await findMarkdownFilesRecursive(tempRoot); + expect(found.map(e => e.commandName)).toContain('team-cmd'); + }); + + test('mixes regular files and symlinks in the same directory', async () => { + await writeFile(join(tempRoot, 'plain.md'), '# plain'); + await writeFile(join(realRepo, 'linked.md'), '# linked'); + await fsSymlink(join(realRepo, 'linked.md'), join(tempRoot, 'linked.md')); + + const found = await findMarkdownFilesRecursive(tempRoot); + const names = found.map(e => e.commandName).sort(); + expect(names).toEqual(['linked', 'plain']); + }); + + test('descends into a symlinked directory of .md files', async () => { + const realSubdir = join(realRepo, 'group'); + await mkdir(realSubdir, { recursive: true }); + await writeFile(join(realSubdir, 'inside.md'), '# inside'); + + await fsSymlink(realSubdir, join(tempRoot, 'group')); + + const found = await findMarkdownFilesRecursive(tempRoot); + expect(found.map(e => e.commandName)).toContain('inside'); + }); + + test('skips broken symlinks silently', async () => { + await writeFile(join(tempRoot, 'real.md'), '# real'); + await fsSymlink(join(realRepo, 'gone.md'), join(tempRoot, 'broken.md')); + + const found = await findMarkdownFilesRecursive(tempRoot); + expect(found.map(e => e.commandName)).toEqual(['real']); + }); + + test('does not recurse infinitely on a self-referential symlink cycle', async () => { + // Symlink that points at the search root itself — the classic infinite + // loop trap. Walk must terminate and report only the real file. + await writeFile(join(tempRoot, 'top.md'), '# top'); + await fsSymlink(tempRoot, join(tempRoot, 'loop')); + + const found = await findMarkdownFilesRecursive(tempRoot); + expect(found.map(e => e.commandName)).toEqual(['top']); + }); + + test('does not recurse infinitely on a multi-level symlink cycle', async () => { + // tempRoot/a/back → tempRoot (loop via a sibling subdirectory). + const subA = join(tempRoot, 'a'); + await mkdir(subA, { recursive: true }); + await writeFile(join(subA, 'inside.md'), '# inside'); + await fsSymlink(tempRoot, join(subA, 'back')); + + const found = await findMarkdownFilesRecursive(tempRoot); + expect(found.map(e => e.commandName).sort()).toEqual(['inside']); + }); +}); diff --git a/packages/paths/src/archon-paths.ts b/packages/paths/src/archon-paths.ts index 9a5d30aae4..199388730c 100644 --- a/packages/paths/src/archon-paths.ts +++ b/packages/paths/src/archon-paths.ts @@ -16,7 +16,7 @@ import { join, dirname, normalize, basename } from 'path'; import { homedir } from 'os'; -import { access, mkdir, symlink, lstat, readdir, readlink, rm } from 'fs/promises'; +import { access, mkdir, symlink, lstat, readdir, readlink, realpath, rm, stat } from 'fs/promises'; import { createLogger } from './logger'; /** Lazy-initialized logger (deferred so test mocks can intercept createLogger) */ @@ -218,12 +218,33 @@ export async function findMarkdownFilesRecursive( rootPath: string, relativePath = '', options?: { maxDepth?: number } +): Promise<{ commandName: string; relativePath: string }[]> { + return findMarkdownFilesRecursiveImpl(rootPath, relativePath, options, new Set()); +} + +async function findMarkdownFilesRecursiveImpl( + rootPath: string, + relativePath: string, + options: { maxDepth?: number } | undefined, + visitedRealPaths: Set ): Promise<{ commandName: string; relativePath: string }[]> { const maxDepth = options?.maxDepth ?? Infinity; const currentDepth = relativePath ? relativePath.split(/[/\\]/).filter(Boolean).length : 0; const results: { commandName: string; relativePath: string }[] = []; const fullPath = join(rootPath, relativePath); + // Seed visited-set on first entry so a symlink that points back at the + // search root itself is detected as a cycle on the first descent. + if (visitedRealPaths.size === 0) { + try { + visitedRealPaths.add(await realpath(fullPath)); + } catch (e) { + const err = e as NodeJS.ErrnoException; + if (err.code === 'ENOENT') return results; + throw err; + } + } + let entries; try { entries = await readdir(fullPath, { withFileTypes: true }); @@ -238,18 +259,53 @@ export async function findMarkdownFilesRecursive( continue; } - if (entry.isDirectory()) { + // Resolve the entry's actual type, following symlinks so that team-shared + // command/workflow files made available via symlink (e.g. `~/.archon/commands/foo.md` + // → `/path/to/team-repo/commands/foo.md`) are picked up. `Dirent.isFile()` / + // `Dirent.isDirectory()` both return false for symlinks, so without the + // stat() they would be silently skipped. + let isDir: boolean; + let isFile: boolean; + if (entry.isSymbolicLink()) { + try { + const targetStat = await stat(join(fullPath, entry.name)); + isDir = targetStat.isDirectory(); + isFile = targetStat.isFile(); + } catch { + // Broken symlink — skip silently + continue; + } + } else { + isDir = entry.isDirectory(); + isFile = entry.isFile(); + } + + if (isDir) { // Skip descending if we're already at the depth cap — files at deeper // levels are silently ignored (matches the convention that `.archon/*/` // folders support one level of grouping like `defaults/`). if (currentDepth >= maxDepth) continue; - const subResults = await findMarkdownFilesRecursive( + + // Resolve the child directory's real path before descending. A symlink + // pointing back at an ancestor (or any already-visited target) would + // otherwise recurse forever. + let realChild: string; + try { + realChild = await realpath(join(fullPath, entry.name)); + } catch { + continue; + } + if (visitedRealPaths.has(realChild)) continue; + visitedRealPaths.add(realChild); + + const subResults = await findMarkdownFilesRecursiveImpl( rootPath, join(relativePath, entry.name), - options + options, + visitedRealPaths ); results.push(...subResults); - } else if (entry.isFile() && entry.name.endsWith('.md')) { + } else if (isFile && entry.name.endsWith('.md')) { results.push({ commandName: basename(entry.name, '.md'), relativePath: join(relativePath, entry.name), diff --git a/packages/workflows/src/validator.test.ts b/packages/workflows/src/validator.test.ts index 67b288b0e2..f6a6833902 100644 --- a/packages/workflows/src/validator.test.ts +++ b/packages/workflows/src/validator.test.ts @@ -22,13 +22,36 @@ import type { WorkflowDefinition, DagNode } from './schemas'; // ============================================================================= let tmpDir: string; +// Isolated ARCHON_HOME so home-scoped command/workflow discovery never sees +// the developer's real ~/.archon/. Without this, tests that assert "no +// commands found" fail on machines where the user has personal commands in +// their home dir. +let tmpHomeDir: string; +let originalArchonHome: string | undefined; +let originalArchonDocker: string | undefined; beforeEach(async () => { tmpDir = await mkdtemp(join(tmpdir(), 'validator-test-')); + tmpHomeDir = await mkdtemp(join(tmpdir(), 'validator-home-')); + originalArchonHome = process.env.ARCHON_HOME; + originalArchonDocker = process.env.ARCHON_DOCKER; + process.env.ARCHON_HOME = tmpHomeDir; + delete process.env.ARCHON_DOCKER; }); afterEach(async () => { await rm(tmpDir, { recursive: true, force: true }); + await rm(tmpHomeDir, { recursive: true, force: true }); + if (originalArchonHome === undefined) { + delete process.env.ARCHON_HOME; + } else { + process.env.ARCHON_HOME = originalArchonHome; + } + if (originalArchonDocker === undefined) { + delete process.env.ARCHON_DOCKER; + } else { + process.env.ARCHON_DOCKER = originalArchonDocker; + } }); function makeWorkflow(name: string, nodes: DagNode[], provider?: string): WorkflowDefinition {