From 1b246c823f504788cf9f3f67a163951d1b6a80d3 Mon Sep 17 00:00:00 2001 From: Sebastian Blank Date: Thu, 30 Apr 2026 14:55:13 +0200 Subject: [PATCH 1/2] fix(paths): findMarkdownFilesRecursive follows symlinks (closes #1501) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Dirent.isFile()` returns false for symlinks, so symlinked `.md` files in the search root were silently skipped. This made team-shared command directories that get exposed to Archon via symlinks (a `~/.archon/commands/foo.md` → `/path/to/team-repo/...` pattern) unusable — workflow nodes referencing those commands failed with `Command prompt not found` even though the file was present and readable. Workflow discovery in `loadWorkflowsFromDir` already handles this correctly by stat()-ing each entry; this brings command discovery in line. Implementation: when a `Dirent` is a symlink, follow it with `stat()` and use the target's `isFile()` / `isDirectory()` for the branch decision. Broken symlinks are skipped silently (matches the existing tolerance for missing search dirs at the top of the function). Tests cover: symlinked file in the search root, mixed regular + symlinked entries in the same dir, symlinked subdirectory, broken symlink. All four exercise the new branch. --- packages/paths/src/archon-paths.test.ts | 68 +++++++++++++++++++++++++ packages/paths/src/archon-paths.ts | 27 ++++++++-- 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/packages/paths/src/archon-paths.test.ts b/packages/paths/src/archon-paths.test.ts index a4303c7957..409987c137 100644 --- a/packages/paths/src/archon-paths.test.ts +++ b/packages/paths/src/archon-paths.test.ts @@ -36,7 +36,9 @@ import { resolveProjectRootFromCwd, ensureProjectStructure, createProjectSourceSymlink, + findMarkdownFilesRecursive, } from './archon-paths'; +import { symlink as fsSymlink } from 'fs/promises'; /** All env vars that path functions depend on */ const ENV_VARS = ['WORKSPACE_PATH', 'WORKTREE_BASE', 'ARCHON_HOME', 'ARCHON_DOCKER', 'HOME']; @@ -776,3 +778,69 @@ 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']); + }); +}); diff --git a/packages/paths/src/archon-paths.ts b/packages/paths/src/archon-paths.ts index 9a5d30aae4..dfdc28070e 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, rm, stat } from 'fs/promises'; import { createLogger } from './logger'; /** Lazy-initialized logger (deferred so test mocks can intercept createLogger) */ @@ -238,7 +238,28 @@ 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/`). @@ -249,7 +270,7 @@ export async function findMarkdownFilesRecursive( options ); 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), From 98d0d2f380666093ae87f75a96bbab8b83035f69 Mon Sep 17 00:00:00 2001 From: Sebastian Blank Date: Mon, 1 Jun 2026 08:39:00 +0200 Subject: [PATCH 2/2] fix(paths): guard findMarkdownFilesRecursive against symlink cycles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit review feedback on #1503: now that symlinked directories are followed, a symlink pointing back at an ancestor (e.g. `dir/loop → dir`) would recurse forever and hang command/workflow discovery at startup. Carries a `Set` of visited real paths through the recursion. On the first entry the search root's real path is added; before descending into any child directory, its real path is resolved with `realpath()` and checked against the set — already-visited targets are skipped instead of re-entered. Two new regression tests cover the self-referential case (`root/loop → root`) and a multi-level back-edge (`root/a/back → root`). While touching the test file, also consolidated the duplicated `fs/promises` import (second CodeRabbit note). Drive-by: `validator.test.ts` now isolates `ARCHON_HOME` to a temp dir in its top-level `beforeEach`. Without this, the symlink-following fix causes the "no commands found" assertions to fail on developer machines where `~/.archon/commands/` contains personal command symlinks — the old silent-skip-symlinks behaviour was hiding the lack of isolation. Co-Authored-By: Claude Opus 4.7 --- packages/paths/src/archon-paths.test.ts | 24 ++++++++++++-- packages/paths/src/archon-paths.ts | 41 ++++++++++++++++++++++-- packages/workflows/src/validator.test.ts | 23 +++++++++++++ 3 files changed, 83 insertions(+), 5 deletions(-) diff --git a/packages/paths/src/archon-paths.test.ts b/packages/paths/src/archon-paths.test.ts index 409987c137..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'; @@ -38,7 +38,6 @@ import { createProjectSourceSymlink, findMarkdownFilesRecursive, } from './archon-paths'; -import { symlink as fsSymlink } from 'fs/promises'; /** All env vars that path functions depend on */ const ENV_VARS = ['WORKSPACE_PATH', 'WORKTREE_BASE', 'ARCHON_HOME', 'ARCHON_DOCKER', 'HOME']; @@ -843,4 +842,25 @@ describe.skipIf(isWindows)('findMarkdownFilesRecursive — symlinks', () => { 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 dfdc28070e..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, stat } 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 }); @@ -264,10 +285,24 @@ export async function findMarkdownFilesRecursive( // 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 (isFile && entry.name.endsWith('.md')) { 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 {