Skip to content

feat(paths,workflows): unify ~/.archon/{workflows,commands,scripts} + drop globalSearchPath (closes #1136)#1315

Merged
Wirasm merged 3 commits intodevfrom
feat/home-scoped-workflows-commands
Apr 20, 2026
Merged

feat(paths,workflows): unify ~/.archon/{workflows,commands,scripts} + drop globalSearchPath (closes #1136)#1315
Wirasm merged 3 commits intodevfrom
feat/home-scoped-workflows-commands

Conversation

@Wirasm
Copy link
Copy Markdown
Collaborator

@Wirasm Wirasm commented Apr 20, 2026

Summary

  • Problem: Archon's home-scoped workflows lived at ~/.archon/.archon/workflows/ — an awkward double-nested path that was the outlier among ~/.archon/'s siblings (workspaces/, archon.db, config.yaml are all direct children). Worse, discovery relied on an opt-in globalSearchPath parameter that most callers forgot, so @archon /workflow run my-global-workflow from a GitHub webhook returned "Workflow not found" (fix: pass globalSearchPath to command handler workflow discovery #1136). The bug was the primitive, not the individual call sites.
  • Why it matters: As the surface grew (chat command handler, Web UI picker, orchestrator resolver), each new site had to remember the param. Five of six call sites on dev drop it. The fix @jonasvanderhaegen submitted in fix: pass globalSearchPath to command handler workflow discovery #1136 would plug this hole for now but the same class of bug recurs on every new consumer.
  • What changed:
    1. ~/.archon/workflows/, ~/.archon/commands/, ~/.archon/scripts/ as direct children of ~/.archon/ — user-level, applies to every project.
    2. Home-scoped commands and scripts are new capability (previously only workflows had a home-scope, and even that was buried one folder deeper than necessary).
    3. Discovery is automaticdiscoverWorkflowsWithConfig(cwd, loadConfig) and discoverScriptsForCwd(cwd) read home paths internally. globalSearchPath is removed. No caller can forget it because the option no longer exists.
    4. Commands are now resolvable by basename across 1 level of subfolders (latent bug fix — before, subfolder commands appeared in discoverAvailableCommands but resolveCommand couldn't load them).
    5. All three types (workflows, commands, scripts) enforce a uniform 1-level subfolder convention, matching the existing defaults/ pattern. Deeper nesting is silently skipped.
    6. WorkflowSource gains 'global' — home-scoped workflows/commands now render under a distinct Web UI badge instead of being coerced to 'project'.
  • What did not change (scope boundary): Repo-scoped discovery (<cwd>/.archon/workflows/ etc.) is unchanged. Bundled defaults (binary-embedded) are unchanged. The defaults/ subfolder convention still works (it's a 1-level subfolder). No config surface added — no new .archon/config.yaml keys.

UX Journey

Before

  User                  ~/.archon/                          Discovery                     Result
  ────                  ──────────                          ─────────                     ──────
  my-workflow.yaml ─▶   ~/.archon/.archon/workflows/        discoverWorkflowsWithConfig
                        (the weird double)                  { globalSearchPath? }
                                                            → opt-in; 5 of 6 callers forget
  @archon /workflow ────────────────────────────────────▶   [X] "Workflow not found"
  run my-workflow                                            (webhook, Web UI, resolver)

  Personal commands ─▶  (no home scope today — repo only or bundled)
  Personal scripts  ─▶  (no home scope today — repo only)

  Subfolder in commands ─▶ listed by discoverAvailableCommands but [X] not resolvable
                                                                    by bare name

After

  User                  ~/.archon/                          Discovery                     Result
  ────                  ──────────                          ─────────                     ──────
  my-workflow.yaml ─▶   ~/.archon/workflows/                discoverWorkflowsWithConfig
  my-cmd.md        ─▶   ~/.archon/commands/                 (reads home unconditionally)
  my-script.ts     ─▶   ~/.archon/scripts/                  discoverScriptsForCwd
                        (direct child — same as             (reads home unconditionally)
                         workspaces/, archon.db, etc.)

  @archon /workflow ────────────────────────────────────▶   ✅ resolves from every surface
  run my-workflow
  (chat, webhook, CLI, Web UI — all inherit home-scope for free)

  Subfolder at 1 level ─▶ resolvable by bare name (matches `defaults/` convention)
  Subfolder at 2+ deep ─▶ ignored silently (consistent 1-level cap)

  Legacy ~/.archon/.archon/workflows/ ─▶ detected on first use; one WARN per process with
                                          exact `mv` command; NOT read (clean cut)

Architecture Diagram

Before

discoverWorkflows(cwd, options?)
  └─ options.globalSearchPath ← ← forgotten by most callers
     → join(globalSearchPath, '.archon/workflows')
                               ↓
                        ~/.archon/.archon/workflows/   [outlier path]

resolveCommand(name, cwd) → join(dir, `${name}.md`)    [direct lookup, no recursion]
loadCommand(name, cwd)    → join(dir, `${name}.md`)    [direct lookup, no recursion]

discoverScripts(scriptsDir) ← repo-only, hardcoded in dag-executor:1321

After

discoverWorkflows(cwd)                              [+] no options arg
  ├─ loadWorkflowsFromDir(bundled)                  source: 'bundled'
  ├─ loadWorkflowsFromDir(~/.archon/workflows)      source: 'global'   [+]
  └─ loadWorkflowsFromDir(cwd/.archon/workflows)    source: 'project'
     (all capped at MAX_DISCOVERY_DEPTH = 1)                            [+]

resolveCommand(name, cwd)                           walks 1 level in each scope
loadCommand(name, cwd)                              basename-matches entries       [+]

discoverScriptsForCwd(cwd)                          repo-scope + home-scope merge  [+]
  ├─ discoverScripts(~/.archon/scripts)             source: 'global' (implicit)
  └─ discoverScripts(cwd/.archon/scripts)           repo wins on collision

maybeWarnLegacyHomePath()                           one-time WARN per process     [+]
                                                    with actionable mv command

Connection inventory:

From To Status Notes
@archon/paths getHomeWorkflowsPath(), getHomeCommandsPath(), getHomeScriptsPath() new Direct children of ~/.archon/
@archon/paths getLegacyHomeWorkflowsPath() new Detection-only — for migration warning
@archon/paths.findMarkdownFilesRecursive accepts maxDepth? modified Callers pass 1; default unlimited for backcompat
@archon/workflows.discoverWorkflowsWithConfig { globalSearchPath? } option removed Home scope read automatically
@archon/workflows.discoverScriptsForCwd new public function new Consumer of getHomeScriptsPath()
WorkflowSource union 'global' variant new 'bundled' | 'project' | 'global'
WorkflowProvider.scriptsDir (dag-executor:1321 hardcoded) discoverScriptsForCwd(cwd) modified Repo + home merge
resolveCommand / loadCommand direct join(dir, name.md) modified Now walks 1 level; subfolder commands resolvable

Label Snapshot

  • Risk: risk: medium (changes path semantics; migration path provided; affected users = those with global workflows at the old path)
  • Size: size: M
  • Scope: paths, workflows, core, server, web, cli, docs
  • Module: paths:home-paths, workflows:discovery, workflows:validator, workflows:script-discovery, workflows:executor-shared, core:orchestrator-agent, core:handlers/command-handler, server:routes/api, web:workflows/NodePalette, cli:workflow-cmd

Change Metadata

  • Change type: feature
  • Primary scope: multi

Linked Issue

Validation Evidence (required)

bun run validate   # ✅ check:bundled + type-check + lint + format + tests across all 10 packages

New test coverage:

  • @archon/paths/archon-paths.test.tsgetHomeWorkflowsPath, getHomeCommandsPath, getHomeScriptsPath, getLegacyHomeWorkflowsPath under default HOME, ARCHON_HOME override, Docker. Regression guard for the double-.archon path.
  • @archon/workflows/loader.test.ts — 7 new tests: home-scoped workflows load, source: 'global' classification, repo overrides home, home-missing silent skip, 1-level subfolder works, 2-level subfolder rejected, legacy-path deprecation warning fires exactly once per process + does NOT load from legacy path.
  • @archon/workflows/validator.test.ts — 3 new tests: home-scoped commands discovered, resolveCommand finds home-only commands, repo overrides home.
  • @archon/workflows/script-discovery.test.ts — 5 new tests: 1-level subfolder supported, 2-level rejected, repo + home merge, repo wins on collision, home-missing tolerance.

E2E smoke (run locally, cleaned up):

✅ e2e-home-scope (at ~/.archon/workflows/e2e-home-scope.yaml) — discovered + run from a scratch git repo in /tmp
✅ e2e-home-command-script — home-scoped script at ~/.archon/scripts/*.ts resolves inside a script: node ("PASS: home-scoped script resolved")
✅ ~/.archon/workflows/triage/grouped-smoke.yaml (1-level subfolder) listed in `archon workflow list`
✅ workflow.legacy_home_path_detected WARN fires with exact `mv` command (my real user home has a legacy file)
✅ legacy-path workflow does NOT appear in the list (clean cut)

Security Impact (required)

  • New permissions/capabilities? No — reads existing user files under ~/.archon/
  • New external network calls? No
  • Secrets/tokens handling changed? No
  • File system access scope changed? Yes — narrow. Discovery now reads ~/.archon/workflows/, ~/.archon/commands/, ~/.archon/scripts/ on every call (previously opt-in via globalSearchPath). All three are under the user's ~/.archon/ root; no paths outside the user's archon directory are introduced. No automatic file mutation — users must migrate from the legacy path manually.

Compatibility / Migration

  • Backward compatible? Mostly yes. Repos without home-scoped content behave identically. Users with pre-refactor global workflows at ~/.archon/.archon/workflows/ see a one-time WARN per process with the exact mv command and must migrate manually. The legacy path is not read — this is the "clean cut" design you approved during planning.
  • Config/env changes? No
  • Database migration needed? No

Human Verification (required)

  • Verified scenarios: bun run validate passes locally. E2E smokes (listed above) run end-to-end.
  • Edge cases checked: deprecation warning fires exactly once per process; ENOENT on home dirs is silent (expected for new users); script home-scope survives discoverScriptsForCwd when repo has no .archon/scripts/; commands resolve by basename from any 1-depth subfolder.
  • What was not verified: regenerating packages/web/src/lib/api.generated.d.ts via bun --filter @archon/web generate:types (requires dev server running) — manually updated the single-line WorkflowSource union in the generated file to add 'global', will re-regen next time someone touches the OpenAPI surface.

Side Effects / Blast Radius (required)

  • Affected subsystems: @archon/paths (new helpers), @archon/workflows (discovery + validator + script-discovery + executor-shared), @archon/core (orchestrator-agent, handlers/command-handler), @archon/server (routes/api workflow + command listings), @archon/web (workflow builder palette badges), @archon/cli (workflow command). Every existing consumer of workflow/command/script discovery touches the new code paths, which is the point.
  • Potential unintended effects: Users with the legacy ~/.archon/.archon/workflows/ directory stop seeing those workflows load until they run the mv command. Documented; WARN is self-explaining.
  • Guardrails: existing workflow.* structured log events plus a new workflow.legacy_home_path_detected WARN for operator visibility.

Rollback Plan (required)

  • Fast rollback: git revert <commit> — single commit, cohesive. Users who migrated to ~/.archon/workflows/ under the new code would need to move back to the legacy location manually after revert (documented in CHANGELOG).
  • Feature flags: N/A.
  • Observable failure symptoms: Home-scoped workflows stop loading; users see their moved files "disappear" from archon workflow list until the revert (then they need to move them back to the legacy path).

Risks and Mitigations

  • Risk: Users with existing global workflows at ~/.archon/.archon/workflows/ see them vanish from archon workflow list on first run after upgrade.
    • Mitigation: One-time WARN per process with the exact mv command. mv is the standard migration — no bespoke tooling needed.
  • Risk: A user's home-scoped command conflicts with a repo-scoped one of the same name.
    • Mitigation: Documented precedence (repo > home > bundled). Existing behavior for name collisions — repo wins silently. Same rule applies to scripts.
  • Risk: Two files in different home-scope subfolders share a basename (e.g. ~/.archon/commands/triage/review.md and ~/.archon/commands/team/review.md).
    • Mitigation: Deterministic walk order — first match wins. Documented as user error in the Global Workflows guide; within-scope duplicates of the same basename should be avoided.

Credit to @jonasvanderhaegen for #1136 surfacing the bug class. Commit carries a Co-authored-by: trailer.

Summary by CodeRabbit

Release Notes

  • New Features

    • Home-scoped workflows, commands, and scripts now auto-discover from ~/.archon/ across all projects.
    • Global commands integrated into command palette with distinct branding.
    • Single-level subfolder organization supported for resource grouping.
  • Improvements

    • Resource precedence clarified: project resources override home-scoped, which override bundled defaults.
    • UI now distinguishes home-scoped ("global") resources in workflow and command displays.
    • Migration guidance provided for legacy configuration paths.

… drop globalSearchPath

Collapses the awkward `~/.archon/.archon/workflows/` convention to a direct
`~/.archon/workflows/` child (matching `workspaces/`, `archon.db`, etc.), adds
home-scoped commands and scripts with the same loading story, and kills the
opt-in `globalSearchPath` parameter so every call site gets home-scope for free.

Closes #1136 (supersedes @jonasvanderhaegen's tactical fix — the bug was the
primitive itself: an easy-to-forget parameter that five of six call sites on
dev dropped).

Primitive changes:

- Home paths are direct children of `~/.archon/`. New helpers in `@archon/paths`:
  `getHomeWorkflowsPath()`, `getHomeCommandsPath()`, `getHomeScriptsPath()`,
  and `getLegacyHomeWorkflowsPath()` (detection-only for migration).
- `discoverWorkflowsWithConfig(cwd, loadConfig)` reads home-scope internally.
  The old `{ globalSearchPath }` option is removed. Chat command handler, Web
  UI workflow picker, orchestrator resolve path — all inherit home-scope for
  free without maintainer patches at every new site.
- `discoverScriptsForCwd(cwd)` merges home + repo scripts (repo wins on name
  collision). dag-executor and validator use it; the hardcoded
  `resolve(cwd, '.archon', 'scripts')` single-scope path is gone.
- Command resolution is now walked-by-basename in each scope. `loadCommand`
  and `resolveCommand` walk 1 subfolder deep and match by `.md` basename, so
  `.archon/commands/triage/review.md` resolves as `review` — closes the
  latent bug where subfolder commands were listed but unresolvable.
- All three (`workflows/`, `commands/`, `scripts/`) enforce a 1-level
  subfolder cap (matches the existing `defaults/` convention). Deeper
  nesting is silently skipped.
- `WorkflowSource` gains `'global'` alongside `'bundled'` and `'project'`.
  Web UI node palette shows a dedicated "Global (~/.archon/commands/)"
  section; badges updated.

Migration (clean cut — no fallback read):

- First use after upgrade: if `~/.archon/.archon/workflows/` exists, Archon
  logs a one-time WARN per process with the exact `mv` command:
  `mv ~/.archon/.archon/workflows ~/.archon/workflows && rmdir ~/.archon/.archon`
  The legacy path is NOT read — users migrate manually. Rollback caveat
  noted in CHANGELOG.

Tests:

- `@archon/paths/archon-paths.test.ts`: new helper tests (default HOME,
  ARCHON_HOME override, Docker), plus regression guards for the double-`.archon/`
  path.
- `@archon/workflows/loader.test.ts`: home-scoped workflows, precedence,
  subfolder 1-depth cap, legacy-path deprecation warning fires exactly once
  per process.
- `@archon/workflows/validator.test.ts`: home-scoped commands + subfolder
  resolution.
- `@archon/workflows/script-discovery.test.ts`: depth cap + merge semantics
  (repo wins, home-missing tolerance).
- Existing CLI + orchestrator tests updated to drop `globalSearchPath`
  assertions.

E2E smoke (verified locally, before cleanup):

- `.archon/workflows/e2e-home-scope.yaml` + scratch repo at /tmp
- Home-scoped workflow discovered from an unrelated git repo
- Home-scoped script (`~/.archon/scripts/*.ts`) executes inside a script node
- 1-level subfolder workflow (`~/.archon/workflows/triage/*.yaml`) listed
- Legacy path warning fires with actionable `mv` command; workflows there
  are NOT loaded

Docs: `CLAUDE.md`, `docs-web/guides/global-workflows.md` (full rewrite for
three-type scope + subfolder convention + migration), `docs-web/reference/
configuration.md` (directory tree), `docs-web/reference/cli.md`,
`docs-web/guides/authoring-workflows.md`.

Co-authored-by: Jonas Vanderhaegen <7755555+jonasvanderhaegen@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 20, 2026

📝 Walkthrough

Walkthrough

This PR introduces a home-scope layer for Archon resources under ~/.archon/, including workflows, commands, and scripts directories. It removes the explicit globalSearchPath option from workflow discovery functions (making home-scope discovery automatic), updates workflow/command sources to include a 'global' variant, implements depth-capped discovery for one-level subfolder grouping, and refactors environment loading and command/script resolution to follow a three-tier precedence model: bundled < home/global < project.

Changes

Cohort / File(s) Summary
Path helpers
packages/paths/src/archon-paths.ts, packages/paths/src/archon-paths.test.ts, packages/paths/src/index.ts
Added four new exported path helpers (getHomeWorkflowsPath, getHomeCommandsPath, getHomeScriptsPath, getLegacyHomeWorkflowsPath) and updated findMarkdownFilesRecursive to accept optional maxDepth parameter for depth-capped directory traversal.
Workflow discovery
packages/workflows/src/workflow-discovery.ts, packages/workflows/src/loader.test.ts
Removed globalSearchPath option from discovery signatures; home-scope workflows now discovered automatically from ~/.archon/workflows/. Added one-time legacy-path deprecation warning for ~/.archon/.archon/workflows/ and depth capping at 1 level. Updated precedence to bundled < global < project.
Script discovery and resolution
packages/workflows/src/script-discovery.ts, packages/workflows/src/script-discovery.test.ts, packages/workflows/src/dag-executor.ts
Added discoverScriptsForCwd(cwd) to merge repo and home scripts with repo precedence. Updated scanScriptDir with depth limiting and refactored script node execution to use merged discovery.
Command loading and validation
packages/workflows/src/executor-shared.ts, packages/workflows/src/validator.ts, packages/workflows/src/validator.test.ts, packages/workflows/src/load-command-prompt.test.ts
Updated command loading to discover from home scope with maxDepth: 1; added loadCommandPrompt test suite validating home/repo precedence. Refactored resolveCommand to enforce repo → home → bundled precedence via directory enumeration rather than direct file checks.
Schema and type updates
packages/server/src/routes/schemas/workflow.schemas.ts, packages/workflows/src/schemas/workflow.ts, packages/web/src/lib/api.generated.d.ts
Extended WorkflowSource union from 'project' | 'bundled' to include 'global', reflecting home-scope resources in API contracts and UI type definitions.
Server API endpoints
packages/server/src/routes/api.ts
Updated /api/commands to discover and merge commands from bundled, global (home), and project scopes; apply maxDepth: 1 to all filesystem walks; and label home commands with source: 'global'.
Web UI components
packages/web/src/components/workflows/CommandPicker.tsx, packages/web/src/components/workflows/NodePalette.tsx
Updated command source badge styling to differentiate project/global/bundled sources. Added "Global (~/.archon/commands/)" section to NodePalette when global commands exist.
CLI and core workflow discovery
packages/cli/src/commands/workflow.ts, packages/cli/src/commands/workflow.test.ts, packages/core/src/orchestrator/orchestrator-agent.ts, packages/core/src/orchestrator/orchestrator.test.ts
Removed globalSearchPath plumbing from workflow discovery calls; home-scope is now handled internally. Updated test assertions to reflect removal of third globalSearchPath argument.
Core utilities
packages/core/src/utils/commands.ts
Updated findMarkdownFilesRecursive to accept optional options?: { maxDepth?: number } and enforce depth-capped traversal.
Documentation
CHANGELOG.md, CLAUDE.md, packages/docs-web/src/content/docs/guides/authoring-workflows.md, packages/docs-web/src/content/docs/guides/global-workflows.md, packages/docs-web/src/content/docs/reference/cli.md, packages/docs-web/src/content/docs/reference/configuration.md
Updated documentation to reflect ~/.archon/workflows/, ~/.archon/commands/, ~/.archon/scripts/ locations; added migration guidance for legacy ~/.archon/.archon/workflows/ path; documented bundled < global < project precedence and one-level subfolder support.
Test configuration
packages/workflows/package.json
Added load-command-prompt.test.ts to test script execution chain.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client/CLI/Server
    participant Discovery as Workflow Discovery
    participant Bundled as Bundled Workflows
    participant Home as Home Scope (~/.archon)
    participant Project as Project Scope (.archon)

    Client->>Discovery: discoverWorkflows(cwd)
    Discovery->>Bundled: Load bundled defaults
    Bundled-->>Discovery: bundled workflows (source: 'bundled')
    
    Discovery->>Home: Load ~/.archon/workflows/
    Home-->>Discovery: home workflows (source: 'global')
    
    Discovery->>Project: Load .archon/workflows/
    Project-->>Discovery: project workflows (source: 'project')
    
    Discovery->>Discovery: Merge: bundled < global < project<br/>(repo wins on collision)
    Discovery-->>Client: Merged workflow map with precedence
Loading
sequenceDiagram
    participant Client as Command Resolver
    participant Bundled as Bundled Commands
    participant Home as Home Commands (~/.archon)
    participant Project as Project Commands (.archon)

    Client->>Bundled: Discover /{appDefaults}/commands<br/>(maxDepth: 1)
    Bundled-->>Client: bundled commands (source: 'bundled')
    
    Client->>Home: Discover ~/.archon/commands<br/>(maxDepth: 1, no error if missing)
    Home-->>Client: home commands (source: 'global')
    
    Client->>Project: Discover .archon/commands<br/>(maxDepth: 1)
    Project-->>Client: project commands (source: 'project')
    
    Client->>Client: Merge with precedence:<br/>bundled < global < project
    Client-->>Client: Return resolved command map
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related issues

  • Global workflows invisible to webhook commands and web UI #1138: This PR directly addresses the retrieved issue by removing ad-hoc globalSearchPath plumbing from workflow discovery function signatures and making home-scoped workflow discovery automatic, with corresponding updates to all call sites and schema changes to reflect the 'global' source variant.

Possibly related PRs

Poem

🐰 Home, then project, bundled last—
A trio of scopes at long last cast!
Workflows and commands, one level deep,
Precedence rules the helpers keep.
No more globalSearchPath to thread,
Discovery flows where config is read! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly describes the main change: moving home-scoped artifacts to unified ~/.archon/ paths and removing the optional globalSearchPath parameter that made home discovery automatic.
Description check ✅ Passed Description is comprehensive and well-structured, covering problem statement, UX journey before/after, architecture diagrams, change metadata, validation evidence, security impact, compatibility, and rollback plans—aligning with the template sections.
Linked Issues check ✅ Passed PR closes #1136 by eliminating the root cause (opt-in globalSearchPath) rather than patching call sites, making home-scope discovery automatic across all consumers—meeting the primary objective of ensuring workflows in user home are discovered by the command handler.
Out of Scope Changes check ✅ Passed All changes are directly in scope: path helpers, discovery internals, precedence logic, schema updates for 'global' source, validator/executor adaptations, documentation, and tests—all tied to unifying ~/.archon/{workflows,commands,scripts} and removing globalSearchPath.
Docstring Coverage ✅ Passed Docstring coverage is 81.25% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/home-scoped-workflows-commands

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
packages/cli/src/commands/workflow.test.ts (1)

313-324: ⚠️ Potential issue | 🟡 Minor

Clear the discovery mock before asserting this call shape.

Line 324 can pass against earlier calls because the mocked module is process-global and this test does not clear discoverWorkflowsWithConfig before invoking workflowListCommand.

🧪 Proposed test isolation fix
   it('calls discoverWorkflowsWithConfig with (cwd, loadConfig) — home scope is internal', async () => {
     const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery');
-    (discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockResolvedValueOnce({
+    const discoverMock = discoverWorkflowsWithConfig as ReturnType<typeof mock>;
+    discoverMock.mockClear();
+    discoverMock.mockResolvedValueOnce({
       workflows: [],
       errors: [],
     });
 
     await workflowListCommand('/test/path');
 
     // After the globalSearchPath refactor, discovery reads ~/.archon/workflows/
     // on every call with no option — every caller inherits home-scope for free.
-    expect(discoverWorkflowsWithConfig).toHaveBeenCalledWith('/test/path', expect.any(Function));
+    expect(discoverMock.mock.calls).toEqual([['/test/path', expect.any(Function)]]);
   });

As per coding guidelines, **/*.test.ts: Bun's mock.module() is process-global and irreversible.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/workflow.test.ts` around lines 313 - 324, The test
is asserting a call shape against a process-global mock that may have prior
invocations; clear the mock before invoking workflowListCommand by calling
mockClear()/mockReset() on discoverWorkflowsWithConfig (e.g.
(discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockClear() ) so only
calls made during this test are considered; then call await
workflowListCommand('/test/path') and assert
expect(discoverWorkflowsWithConfig).toHaveBeenCalledWith('/test/path',
expect.any(Function)).
packages/docs-web/src/content/docs/reference/configuration.md (1)

32-42: ⚠️ Potential issue | 🟡 Minor

Add repo-local scripts to the directory diagram.

Line 32 says repo-local scripts/ override home-scoped scripts, but the repository-level tree only shows commands/, workflows/, and config.yaml.

📝 Proposed docs update
 .archon/
 ├── commands/       # Custom commands
 │   └── plan.md
 ├── workflows/      # Workflow definitions (YAML files)
+├── scripts/        # Named workflow scripts
 └── config.yaml     # Repo-specific configuration (optional)

Based on learnings, workflow scripts support named script files in .archon/scripts/.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/docs-web/src/content/docs/reference/configuration.md` around lines
32 - 42, Update the repository-level tree under the "Repository-Level
(.archon/)" section to include the missing scripts/ directory (e.g., show
.archon/scripts/ alongside commands/ and workflows/) so the docs reflect that
repo-local scripts override home-scoped scripts; modify the code block that
currently lists .archon/ ├── commands/ ├── workflows/ └── config.yaml to include
a scripts/ entry (and optionally an example file like build.sh or deploy.md) to
match the text describing repo-local `scripts/`.
packages/server/src/routes/api.ts (2)

2129-2183: ⚠️ Potential issue | 🟠 Major

Fetch home-scoped workflows in the detail route.

GET /api/workflows can now list source: 'global', but GET /api/workflows/:name still falls through from project directly to bundled defaults. A global workflow can therefore 404 in the Web UI, or be masked by a same-named bundled workflow instead of taking precedence.

Add a home-scoped lookup before bundled defaults, or reuse the same discovery path as the list endpoint so precedence stays bundled < global < project.

As per coding guidelines, **/workflows/src/**/*.ts: Home-scoped workflows, commands, and scripts use source: 'global'; load priority is bundled < global < project.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server/src/routes/api.ts` around lines 2129 - 2183, The detail route
currently checks project then bundled defaults but omits home-scoped (global)
workflows, causing global workflows to 404 or be shadowed; add a home-scoped
lookup between the project block and the bundled-defaults block (same precedence
used by GET /api/workflows: bundled < global < project). Specifically, after the
workingDir try/catch and before the Object.hasOwn(BUNDLED_WORKFLOWS, name)
check, attempt to read and parse the home/global workflow file (use the same
discovery helper used by the list endpoint — e.g., getHomeWorkflowsPath() or the
function that list route uses), call parseWorkflow and return apiError on parse
error or c.json with source: 'global' on success; on read errors only continue
when ENOENT, and log other errors via getLog().error similar to other blocks
(use parseWorkflow, apiError, getLog(), name, filename symbols to match existing
patterns).

2208-2239: ⚠️ Potential issue | 🟠 Major

Avoid writing fallback workflows into the retired nested path.

When no cwd or registered codebase exists, these routes set workingDir = getArchonHome() and then append the repo-relative .archon/workflows, producing ~/.archon/.archon/workflows. That path is now legacy/detection-only, so saves become invisible to discovery and deletes target the wrong location.

🛠️ Proposed home-path handling
-    if (!workingDir) {
-      workingDir = getArchonHome();
-    }
+    const isHomeScopedWorkflow = !workingDir;
     try {
       const [workflowFolder] = getWorkflowFolderSearchPaths();
-      const dirPath = join(workingDir, workflowFolder);
+      const dirPath = isHomeScopedWorkflow
+        ? getHomeWorkflowsPath()
+        : join(workingDir, workflowFolder);
       await mkdir(dirPath, { recursive: true });
       const filePath = join(dirPath, `${name}.yaml`);
       await writeFile(filePath, yamlContent, 'utf-8');
       return c.json({
         workflow: parsed.workflow,
         filename: `${name}.yaml`,
-        source: 'project' as WorkflowSource,
+        source: (isHomeScopedWorkflow ? 'global' : 'project') as WorkflowSource,
       });
-    if (!workingDir) {
-      workingDir = getArchonHome();
-    }
+    const isHomeScopedWorkflow = !workingDir;
 
     const [workflowFolder] = getWorkflowFolderSearchPaths();
-    const filePath = join(workingDir, workflowFolder, `${name}.yaml`);
+    const filePath = join(
+      isHomeScopedWorkflow ? getHomeWorkflowsPath() : join(workingDir, workflowFolder),
+      `${name}.yaml`
+    );

This also requires importing getHomeWorkflowsPath from @archon/paths.

As per coding guidelines, migration from ~/.archon/.archon/workflows/ should warn and not load from the old location.

Also applies to: 2269-2277

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server/src/routes/api.ts` around lines 2208 - 2239, When falling
back to getArchonHome() (the workingDir assignment around save workflow
handling), do not append the repo-relative workflow folder from
getWorkflowFolderSearchPaths() because that produces the legacy
~/.archon/.archon/workflows path; instead import and use getHomeWorkflowsPath()
to compute the home workflows directory when workingDir === getArchonHome(), so
set dirPath from getHomeWorkflowsPath() before calling mkdir/writeFile. Also add
a check for the old nested path (join(getArchonHome(), workflowFolder)) and if
it exists, emit a warning via getLog().warn but do not write into that legacy
location. Apply the same change to the other save/delete block referenced
(similar code around lines 2269-2277), and ensure getHomeWorkflowsPath is
imported from `@archon/paths`.
packages/workflows/src/workflow-discovery.ts (1)

286-305: ⚠️ Potential issue | 🟠 Major

Don’t preserve bundled for project files that only share a default basename.

This branch treats any repo workflow whose basename matches a bundled default as source: 'bundled'. Since loadWorkflowsFromDir() flattens subfolders to basename keys, a project file like .archon/workflows/archon-implement.yaml can override bundled content but still render/classify as bundled. Preserve the bundled label only when you can prove this is the repo defaults/ rediscovery case; otherwise project overrides should be source: 'project'. As per coding guidelines, home-scoped workflows use load priority bundled < global < project.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/workflow-discovery.ts` around lines 286 - 305, The
loop currently preserves source:'bundled' whenever a repo workflow basename
matches an existing bundled entry; instead, only preserve bundled when this repo
discovery is specifically the repo "defaults/" rediscovery case—i.e., check an
explicit indicator on repoResult (for example a flag/property such as
repoResult.isDefaults or repoResult.sourceDir === 'defaults' that your loader
sets when calling loadWorkflowsFromDir for the defaults/ subdirectory) before
keeping existing?.source === 'bundled'. Update the logic in the
repoResult.workflows iteration (the block referencing workflowsByFile, filename,
existing, and source values) to require that repoResult indicates it came from
the defaults/ dir in addition to existing?.source === 'bundled'; otherwise treat
the repo file as source:'project'.
🧹 Nitpick comments (4)
packages/core/src/utils/commands.ts (1)

15-18: Add the explicit type for the defaulted parameter.

relativePath is inferred from the default value, but this changed signature should keep parameter annotations explicit.

♻️ Proposed type annotation
 export async function findMarkdownFilesRecursive(
   rootPath: string,
-  relativePath = '',
+  relativePath: string = '',
   options?: { maxDepth?: number }
 ): Promise<{ commandName: string; relativePath: string }[]> {

As per coding guidelines, “Strict TypeScript configuration enforced; all functions must have complete type annotations; no any types without explicit justification”.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/utils/commands.ts` around lines 15 - 18, The function
findMarkdownFilesRecursive has an implicitly typed parameter relativePath set to
''; update its signature to explicitly annotate that parameter as a string
(e.g., relativePath: string = ''), keeping options as-is, so the function has
full TypeScript parameter annotations and satisfies the strict typing guideline.
packages/workflows/src/script-discovery.ts (1)

108-113: Broaden the duplicate-script wording.

With one-level subfolders, duplicates can be triage/foo.ts and release/foo.ts; the current message only mentions “across extensions.”

📝 Proposed wording update
- * Throws if duplicate script names are found across different extensions within the directory.
+ * Throws if duplicate script basenames are found within the directory, including subfolders.
       throw new Error(
         `Duplicate script name "${name}": found "${existing.path}" and "${entryPath}". ` +
-          'Script names must be unique across extensions.'
+          'Script names must be unique within the directory, including subfolders.'
       );

Also applies to: 121-125

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/script-discovery.ts` around lines 108 - 113, The
duplicate-script error message in the script discovery logic is too narrow;
update the thrown Error where you check const existing = scripts.get(name) (and
the similar block later) to broaden the wording so it explains that script names
must be unique across extensions and one-level subfolders (e.g., "triage/foo.ts"
vs "release/foo.ts"), and include the conflicting paths (existing.path and
entryPath) in the message; modify the Error thrown in these checks to reflect
this broader scope while still showing the two conflicting file paths.
packages/workflows/src/executor-shared.ts (1)

162-167: Detect duplicate command basenames within a scope.

With 1-level subfolders, triage/review.md and team/review.md both resolve as review; entries.find(...) silently picks one based on directory iteration order. Since the docs call duplicate basenames a user error, return a clear duplicate error instead of loading an arbitrary prompt.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/executor-shared.ts` around lines 162 - 167, The code
silently picks the first matching entry when resolving commandName across
resolvedSearchPaths using archonPaths.findMarkdownFilesRecursive and
entries.find, which hides duplicate basenames (e.g., triage/review.md vs
team/review.md); modify the lookup in the loop that computes filePath so that
after collecting matches for a given commandName you detect if multiple distinct
entries have the same basename and throw/return a clear duplicate error
(including the conflicting relativePath or dir names) instead of
proceeding—i.e., replace the single entries.find(...) resolution with logic that
filters all entries where e.commandName === commandName, if matches.length > 1
raise a duplicate-command error referencing commandName and the conflicting
entries, otherwise use the single match to set filePath.
packages/workflows/src/validator.ts (1)

185-198: Surface duplicate command basenames instead of picking the first match.

resolveCommandInDir() silently chooses one file when multiple 1-level subfolders contain the same basename. That makes validation pass while runtime behavior depends on traversal order; detect duplicates and return a clear validation error instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/validator.ts` around lines 185 - 198,
resolveCommandInDir currently picks the first matching entry and hides duplicate
basenames; change it to detect all entries with e.commandName === commandName
(use entries.filter) and if more than one match is found throw a clear Error
listing the conflicting match.relativePath values (joined with the rootDir for
full paths) so validation surface duplicates instead of returning a single path;
otherwise continue to return the single matched path as before. Ensure you
reference resolveCommandInDir, findMarkdownFilesRecursive, entries, commandName,
and relativePath when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CLAUDE.md`:
- Line 726: The API docs mention the workflow endpoint GET /api/workflows/:name
as returning source: 'project' | 'bundled' but the spec now includes a new
global source value; update the endpoint description to list source: 'project' |
'bundled' | 'global' (or the enum/union that represents sources used elsewhere),
and ensure any examples or schema blocks for GET /api/workflows/:name (and
related commands/workflows response schemas) reflect the new "global" value so
Web/API consumers see the added enum member.

In `@packages/cli/src/commands/workflow.ts`:
- Line 13: The failure hint currently assumes only a repo-scoped workflows path;
update the error handling around discoverWorkflowsWithConfig to include the home
workflow directory as well (i.e., when constructing the permission-failure hint,
mention both the repo workflows path and the home workflows path). Locate the
catch/logging that wraps permission errors for discoverWorkflowsWithConfig (also
the similar logic around lines 122-129) and expand the hint text to reference
the home workflow directory name/path used by the CLI (so permission errors
originating from home-scoped workflows are not mischaracterized as repo-only).

In `@packages/core/src/orchestrator/orchestrator-agent.ts`:
- Around line 391-393: The call to
discoverWorkflowsWithConfig(getArchonWorkspacesPath(), loadConfig) causes
home-scoped workflows to be discovered twice (once here and again during repo
discovery), producing duplicate entries in allErrors; update the logic so home
workflows are only discovered once for scoped conversations — either by adding
an option/flag to discoverWorkflowsWithConfig to exclude home-scoped workflows
when performing project-scoped discovery (and use that flag here or in the later
repo discovery), or by de-duplicating error entries before returning (e.g.,
collapse errors by unique workflow identifier/path and error message). Locate
and modify the usage of discoverWorkflowsWithConfig and the allErrors
aggregation in orchestrator-agent (references: discoverWorkflowsWithConfig,
getArchonWorkspacesPath, allErrors) to implement one of these fixes and ensure
duplicate error reporting is eliminated.

In `@packages/docs-web/src/content/docs/guides/global-workflows.md`:
- Around line 16-48: The fenced code blocks that show filesystem paths and the
tree under "Subfolders (1 level deep)" lack a language tag and trigger
markdownlint MD040; update each triple-backtick fence in
packages/docs-web/src/content/docs/guides/global-workflows.md that contains the
path listings (the three short path lists near the top and the $ARCHON_HOME
variant, plus the tree example under "Subfolders (1 level deep)") to use the
text language tag (i.e., change ``` to ```text) so the path/tree blocks are
treated as plain text and satisfy linting.

In `@packages/paths/src/archon-paths.test.ts`:
- Around line 229-288: The tests for
getHomeWorkflowsPath/getHomeCommandsPath/getLegacyHomeWorkflowsPath leave
WORKSPACE_PATH set which can force Docker mode via getArchonHome(); clear
process.env.WORKSPACE_PATH in each affected test setup where you currently
delete ARCHON_HOME and ARCHON_DOCKER so the assertions are deterministic.
Specifically, update the tests that call getHomeWorkflowsPath,
getHomeCommandsPath and getLegacyHomeWorkflowsPath to also delete or unset
process.env.WORKSPACE_PATH alongside ARCHON_HOME and ARCHON_DOCKER before
calling the helper functions.

In `@packages/server/src/routes/api.ts`:
- Around line 2326-2331: The home command discovery is using
findMarkdownFilesRecursive(homeCommandsPath) with no depth limit, allowing paths
deeper than one level (e.g., ~/.archon/commands/a/b/foo.md); change the call
that discovers home-scoped commands (around getHomeCommandsPath() usage and the
loop that calls commandMap.set) to restrict recursion to one level only by
invoking the finder with a maxDepth/limit of 1 (or replace it with the one-level
helper), so only files in ~/.archon/commands and its immediate subfolders are
returned and deeper nesting is silently ignored.
- Line 143: Replace the hard-coded union type WorkflowSource with a type derived
from the Zod schema to avoid drift: remove "type WorkflowSource = 'project' |
'bundled' | 'global';" and instead declare "type WorkflowSource = z.infer<typeof
workflowSourceSchema>;" (referencing workflowSourceSchema and the z import
already used in the file).

In `@packages/workflows/src/executor-shared.ts`:
- Around line 152-160: The resolvedSearchPaths ordering puts project (cwd) paths
before the home commands path, causing repo defaults to override global/home
commands; update the construction of resolvedSearchPaths so
archonPaths.getHomeCommandsPath() appears before the repo-mapped paths (i.e.
build resolvedSearchPaths with archonPaths.getHomeCommandsPath() first, then
...searchPaths.map(folder => join(cwd, folder))), keeping use of
getCommandFolderSearchPaths(configuredFolder), cwd, and
archonPaths.getHomeCommandsPath() intact.

In `@packages/workflows/src/validator.ts`:
- Around line 219-228: Validation currently iterates the full list from
getCommandFolderSearchPaths() (which includes .archon/commands/defaults) before
checking home commands, causing repo defaults to win over home commands; change
validator.ts so resolveCommandInDir is applied in runtime/documented precedence:
first check project repo command folders only (filtering out the defaults path
returned by getCommandFolderSearchPaths()), then call
resolveCommandInDir(getHomeCommandsPath(), commandName) to check home-scoped
commands, and finally fall back to the bundled/default commands path (the
defaults entry from getCommandFolderSearchPaths()) if neither project nor home
resolve; use the existing symbols getCommandFolderSearchPaths,
resolveCommandInDir, getHomeCommandsPath, commandName and cwd to implement the
reordered checks.

In `@packages/workflows/src/workflow-discovery.ts`:
- Around line 53-72: The migration warning in maybeWarnLegacyHomePath builds a
naive moveCommand that will nest the legacy folder if newPath already exists;
update the function to probe newPath (e.g., await access(newPath)) and branch:
if newPath does not exist, keep the current mv command, otherwise emit a
merge-safe command (for example use mv -T "<legacyPath>" "<newPath>" or an
rsync/ditto style merge command that preserves/merges contents and removes the
source), and include that alternative command in the getLog().warn payload
(still setting hasWarnedLegacyHomePath=true afterwards); ensure the change
message matches the corresponding CHANGELOG.md entry.

---

Outside diff comments:
In `@packages/cli/src/commands/workflow.test.ts`:
- Around line 313-324: The test is asserting a call shape against a
process-global mock that may have prior invocations; clear the mock before
invoking workflowListCommand by calling mockClear()/mockReset() on
discoverWorkflowsWithConfig (e.g. (discoverWorkflowsWithConfig as
ReturnType<typeof mock>).mockClear() ) so only calls made during this test are
considered; then call await workflowListCommand('/test/path') and assert
expect(discoverWorkflowsWithConfig).toHaveBeenCalledWith('/test/path',
expect.any(Function)).

In `@packages/docs-web/src/content/docs/reference/configuration.md`:
- Around line 32-42: Update the repository-level tree under the
"Repository-Level (.archon/)" section to include the missing scripts/ directory
(e.g., show .archon/scripts/ alongside commands/ and workflows/) so the docs
reflect that repo-local scripts override home-scoped scripts; modify the code
block that currently lists .archon/ ├── commands/ ├── workflows/ └── config.yaml
to include a scripts/ entry (and optionally an example file like build.sh or
deploy.md) to match the text describing repo-local `scripts/`.

In `@packages/server/src/routes/api.ts`:
- Around line 2129-2183: The detail route currently checks project then bundled
defaults but omits home-scoped (global) workflows, causing global workflows to
404 or be shadowed; add a home-scoped lookup between the project block and the
bundled-defaults block (same precedence used by GET /api/workflows: bundled <
global < project). Specifically, after the workingDir try/catch and before the
Object.hasOwn(BUNDLED_WORKFLOWS, name) check, attempt to read and parse the
home/global workflow file (use the same discovery helper used by the list
endpoint — e.g., getHomeWorkflowsPath() or the function that list route uses),
call parseWorkflow and return apiError on parse error or c.json with source:
'global' on success; on read errors only continue when ENOENT, and log other
errors via getLog().error similar to other blocks (use parseWorkflow, apiError,
getLog(), name, filename symbols to match existing patterns).
- Around line 2208-2239: When falling back to getArchonHome() (the workingDir
assignment around save workflow handling), do not append the repo-relative
workflow folder from getWorkflowFolderSearchPaths() because that produces the
legacy ~/.archon/.archon/workflows path; instead import and use
getHomeWorkflowsPath() to compute the home workflows directory when workingDir
=== getArchonHome(), so set dirPath from getHomeWorkflowsPath() before calling
mkdir/writeFile. Also add a check for the old nested path (join(getArchonHome(),
workflowFolder)) and if it exists, emit a warning via getLog().warn but do not
write into that legacy location. Apply the same change to the other save/delete
block referenced (similar code around lines 2269-2277), and ensure
getHomeWorkflowsPath is imported from `@archon/paths`.

In `@packages/workflows/src/workflow-discovery.ts`:
- Around line 286-305: The loop currently preserves source:'bundled' whenever a
repo workflow basename matches an existing bundled entry; instead, only preserve
bundled when this repo discovery is specifically the repo "defaults/"
rediscovery case—i.e., check an explicit indicator on repoResult (for example a
flag/property such as repoResult.isDefaults or repoResult.sourceDir ===
'defaults' that your loader sets when calling loadWorkflowsFromDir for the
defaults/ subdirectory) before keeping existing?.source === 'bundled'. Update
the logic in the repoResult.workflows iteration (the block referencing
workflowsByFile, filename, existing, and source values) to require that
repoResult indicates it came from the defaults/ dir in addition to
existing?.source === 'bundled'; otherwise treat the repo file as
source:'project'.

---

Nitpick comments:
In `@packages/core/src/utils/commands.ts`:
- Around line 15-18: The function findMarkdownFilesRecursive has an implicitly
typed parameter relativePath set to ''; update its signature to explicitly
annotate that parameter as a string (e.g., relativePath: string = ''), keeping
options as-is, so the function has full TypeScript parameter annotations and
satisfies the strict typing guideline.

In `@packages/workflows/src/executor-shared.ts`:
- Around line 162-167: The code silently picks the first matching entry when
resolving commandName across resolvedSearchPaths using
archonPaths.findMarkdownFilesRecursive and entries.find, which hides duplicate
basenames (e.g., triage/review.md vs team/review.md); modify the lookup in the
loop that computes filePath so that after collecting matches for a given
commandName you detect if multiple distinct entries have the same basename and
throw/return a clear duplicate error (including the conflicting relativePath or
dir names) instead of proceeding—i.e., replace the single entries.find(...)
resolution with logic that filters all entries where e.commandName ===
commandName, if matches.length > 1 raise a duplicate-command error referencing
commandName and the conflicting entries, otherwise use the single match to set
filePath.

In `@packages/workflows/src/script-discovery.ts`:
- Around line 108-113: The duplicate-script error message in the script
discovery logic is too narrow; update the thrown Error where you check const
existing = scripts.get(name) (and the similar block later) to broaden the
wording so it explains that script names must be unique across extensions and
one-level subfolders (e.g., "triage/foo.ts" vs "release/foo.ts"), and include
the conflicting paths (existing.path and entryPath) in the message; modify the
Error thrown in these checks to reflect this broader scope while still showing
the two conflicting file paths.

In `@packages/workflows/src/validator.ts`:
- Around line 185-198: resolveCommandInDir currently picks the first matching
entry and hides duplicate basenames; change it to detect all entries with
e.commandName === commandName (use entries.filter) and if more than one match is
found throw a clear Error listing the conflicting match.relativePath values
(joined with the rootDir for full paths) so validation surface duplicates
instead of returning a single path; otherwise continue to return the single
matched path as before. Ensure you reference resolveCommandInDir,
findMarkdownFilesRecursive, entries, commandName, and relativePath when making
the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b15e1701-ece3-40a4-98bd-992810b3cf30

📥 Commits

Reviewing files that changed from the base of the PR and between 235a8ce and e2d318f.

📒 Files selected for processing (29)
  • .archon/workflows/e2e-home-scope.yaml
  • CHANGELOG.md
  • CLAUDE.md
  • packages/cli/src/commands/workflow.test.ts
  • packages/cli/src/commands/workflow.ts
  • packages/core/src/orchestrator/orchestrator-agent.ts
  • packages/core/src/orchestrator/orchestrator.test.ts
  • packages/core/src/utils/commands.ts
  • packages/docs-web/src/content/docs/guides/authoring-workflows.md
  • packages/docs-web/src/content/docs/guides/global-workflows.md
  • packages/docs-web/src/content/docs/reference/cli.md
  • packages/docs-web/src/content/docs/reference/configuration.md
  • packages/paths/src/archon-paths.test.ts
  • packages/paths/src/archon-paths.ts
  • packages/paths/src/index.ts
  • packages/server/src/routes/api.ts
  • packages/server/src/routes/schemas/workflow.schemas.ts
  • packages/web/src/components/workflows/CommandPicker.tsx
  • packages/web/src/components/workflows/NodePalette.tsx
  • packages/web/src/lib/api.generated.d.ts
  • packages/workflows/src/dag-executor.ts
  • packages/workflows/src/executor-shared.ts
  • packages/workflows/src/loader.test.ts
  • packages/workflows/src/schemas/workflow.ts
  • packages/workflows/src/script-discovery.test.ts
  • packages/workflows/src/script-discovery.ts
  • packages/workflows/src/validator.test.ts
  • packages/workflows/src/validator.ts
  • packages/workflows/src/workflow-discovery.ts

Comment thread CLAUDE.md
- Workflows: `~/.archon/workflows/` (or `$ARCHON_HOME/workflows/`)
- Commands: `~/.archon/commands/` (or `$ARCHON_HOME/commands/`)
- Scripts: `~/.archon/scripts/` (or `$ARCHON_HOME/scripts/`)
- Source label: `source: 'global'` on workflows and commands (scripts don't have a source label)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update the API docs to include source: 'global'.

This section now documents the new global source label, but the workflow endpoint docs later still describe GET /api/workflows/:name as returning only source: 'project' | 'bundled'. Please update that API line too so Web/API consumers see the new enum value.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` at line 726, The API docs mention the workflow endpoint GET
/api/workflows/:name as returning source: 'project' | 'bundled' but the spec now
includes a new global source value; update the endpoint description to list
source: 'project' | 'bundled' | 'global' (or the enum/union that represents
sources used elsewhere), and ensure any examples or schema blocks for GET
/api/workflows/:name (and related commands/workflows response schemas) reflect
the new "global" value so Web/API consumers see the added enum member.

import { WORKFLOW_EVENT_TYPES, type WorkflowEventType } from '@archon/workflows/store';
import { configureIsolation, getIsolationProvider } from '@archon/isolation';
import { createLogger, getArchonHome } from '@archon/paths';
import { createLogger } from '@archon/paths';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Include the home workflow directory in the failure hint.

discoverWorkflowsWithConfig now reads home-scoped workflows internally, so permission failures from the home path are currently wrapped with a repo-only hint.

💬 Proposed clearer hint
-import { createLogger } from '@archon/paths';
+import { createLogger, getHomeWorkflowsPath } from '@archon/paths';
@@
     throw new Error(
-      `Error loading workflows: ${err.message}\nHint: Check permissions on .archon/workflows/ directory.`
+      `Error loading workflows: ${err.message}\nHint: Check permissions on .archon/workflows/ and ${getHomeWorkflowsPath()} directories.`
     );

Also applies to: 122-129

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/workflow.ts` at line 13, The failure hint currently
assumes only a repo-scoped workflows path; update the error handling around
discoverWorkflowsWithConfig to include the home workflow directory as well
(i.e., when constructing the permission-failure hint, mention both the repo
workflows path and the home workflows path). Locate the catch/logging that wraps
permission errors for discoverWorkflowsWithConfig (also the similar logic around
lines 122-129) and expand the hint text to reference the home workflow directory
name/path used by the CLI (so permission errors originating from home-scoped
workflows are not mischaracterized as repo-only).

Comment on lines +391 to +393
// Home-scoped workflows at ~/.archon/workflows/ are discovered automatically
// by discoverWorkflowsWithConfig — no option needed.
const result = await discoverWorkflowsWithConfig(getArchonWorkspacesPath(), loadConfig);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid discovering home workflows twice for project-scoped conversations.

This call now includes home-scoped workflows internally, and the later repo discovery in this function does too. A broken home workflow can therefore be reported twice in allErrors; either discover once for scoped conversations or de-duplicate errors before returning.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/orchestrator/orchestrator-agent.ts` around lines 391 - 393,
The call to discoverWorkflowsWithConfig(getArchonWorkspacesPath(), loadConfig)
causes home-scoped workflows to be discovered twice (once here and again during
repo discovery), producing duplicate entries in allErrors; update the logic so
home workflows are only discovered once for scoped conversations — either by
adding an option/flag to discoverWorkflowsWithConfig to exclude home-scoped
workflows when performing project-scoped discovery (and use that flag here or in
the later repo discovery), or by de-duplicating error entries before returning
(e.g., collapse errors by unique workflow identifier/path and error message).
Locate and modify the usage of discoverWorkflowsWithConfig and the allErrors
aggregation in orchestrator-agent (references: discoverWorkflowsWithConfig,
getArchonWorkspacesPath, allErrors) to implement one of these fixes and ensure
duplicate error reporting is eliminated.

Comment on lines 16 to +48
```
~/.archon/.archon/workflows/
~/.archon/workflows/
~/.archon/commands/
~/.archon/scripts/
```

Or, if you have set `ARCHON_HOME`:

```
$ARCHON_HOME/.archon/workflows/
$ARCHON_HOME/workflows/
$ARCHON_HOME/commands/
$ARCHON_HOME/scripts/
```

Create the directory if it does not exist:
Create the directories if they do not exist:

```bash
mkdir -p ~/.archon/.archon/workflows
mkdir -p ~/.archon/workflows ~/.archon/commands ~/.archon/scripts
```

> **Note on location.** These are direct children of `~/.archon/` -- same level as `workspaces/`, `archon.db`, and `config.yaml`. Earlier Archon versions stored global workflows at `~/.archon/.archon/workflows/`; see [Migrating from the old path](#migrating-from-the-old-path) below.

## Subfolders (1 level deep)

Each directory supports one level of subfolders for grouping, matching the existing `defaults/` convention. Deeper nesting is ignored silently.

```
~/.archon/workflows/
├── my-review.yaml # ✅ top-level file
├── triage/ # ✅ 1-level subfolder (grouping)
│ └── weekly-cleanup.yaml # ✅ resolvable as `weekly-cleanup`
└── team/personal/too-deep.yaml # ❌ ignored — 2 levels down
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add languages to these fenced blocks.

markdownlint flags the path/tree fences here (MD040). Use text for the path listings/tree to keep docs validation clean.

Proposed docs fix
-```
+```text
 ~/.archon/workflows/
 ~/.archon/commands/
 ~/.archon/scripts/

@@
- +text
$ARCHON_HOME/workflows/
$ARCHON_HOME/commands/
$ARCHON_HOME/scripts/

@@
-```
+```text
~/.archon/workflows/
├── my-review.yaml              # ✅ top-level file
├── triage/                     # ✅ 1-level subfolder (grouping)
│   └── weekly-cleanup.yaml     # ✅ resolvable as `weekly-cleanup`
└── team/personal/too-deep.yaml # ❌ ignored — 2 levels down
</details>

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>

[warning] 16-16: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

[warning] 24-24: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

[warning] 42-42: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @packages/docs-web/src/content/docs/guides/global-workflows.md around lines
16 - 48, The fenced code blocks that show filesystem paths and the tree under
"Subfolders (1 level deep)" lack a language tag and trigger markdownlint MD040;
update each triple-backtick fence in
packages/docs-web/src/content/docs/guides/global-workflows.md that contains the
path listings (the three short path lists near the top and the $ARCHON_HOME
variant, plus the tree example under "Subfolders (1 level deep)") to use the
text language tag (i.e., change totext) so the path/tree blocks are
treated as plain text and satisfy linting.


</details>

<!-- fingerprinting:phantom:medusa:ibis -->

<!-- This is an auto-generated comment by CodeRabbit -->

Comment on lines +229 to +288
describe('getHomeWorkflowsPath', () => {
test('returns ~/.archon/workflows by default (direct child of ~/.archon/)', () => {
delete process.env.ARCHON_HOME;
delete process.env.ARCHON_DOCKER;
expect(getHomeWorkflowsPath()).toBe(join(homedir(), '.archon', 'workflows'));
});

test('returns /.archon/workflows in Docker', () => {
process.env.ARCHON_DOCKER = 'true';
expect(getHomeWorkflowsPath()).toBe(join('/', '.archon', 'workflows'));
});

test('uses ARCHON_HOME when set', () => {
delete process.env.ARCHON_DOCKER;
process.env.ARCHON_HOME = '/custom/archon';
expect(getHomeWorkflowsPath()).toBe(join('/custom/archon', 'workflows'));
});

test('no double `.archon/` nesting — must sit next to workspaces/ and worktrees/', () => {
// Regression guard: the old location was ~/.archon/.archon/workflows/.
// New location must NOT reintroduce the double-nested path.
delete process.env.ARCHON_HOME;
delete process.env.ARCHON_DOCKER;
expect(getHomeWorkflowsPath()).not.toContain(join('.archon', '.archon'));
});
});

describe('getHomeCommandsPath', () => {
test('returns ~/.archon/commands by default', () => {
delete process.env.ARCHON_HOME;
delete process.env.ARCHON_DOCKER;
expect(getHomeCommandsPath()).toBe(join(homedir(), '.archon', 'commands'));
});

test('returns /.archon/commands in Docker', () => {
process.env.ARCHON_DOCKER = 'true';
expect(getHomeCommandsPath()).toBe(join('/', '.archon', 'commands'));
});

test('uses ARCHON_HOME when set', () => {
delete process.env.ARCHON_DOCKER;
process.env.ARCHON_HOME = '/custom/archon';
expect(getHomeCommandsPath()).toBe(join('/custom/archon', 'commands'));
});
});

describe('getLegacyHomeWorkflowsPath', () => {
// This helper only exists so discovery can DETECT files at the old location
// and emit a deprecation warning. It is not a fallback read path.
test('returns ~/.archon/.archon/workflows (the retired location)', () => {
delete process.env.ARCHON_HOME;
delete process.env.ARCHON_DOCKER;
expect(getLegacyHomeWorkflowsPath()).toBe(join(homedir(), '.archon', '.archon', 'workflows'));
});

test('honors ARCHON_HOME so migration detection works in custom setups', () => {
delete process.env.ARCHON_DOCKER;
process.env.ARCHON_HOME = '/custom/archon';
expect(getLegacyHomeWorkflowsPath()).toBe(join('/custom/archon', '.archon', 'workflows'));
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clear WORKSPACE_PATH in the new local-path tests.

These tests clear ARCHON_DOCKER, but getArchonHome() also enters Docker mode when WORKSPACE_PATH is set. That makes the new default and ARCHON_HOME assertions environment-dependent.

🧪 Proposed deterministic setup
     test('returns ~/.archon/workflows by default (direct child of ~/.archon/)', () => {
+      delete process.env.WORKSPACE_PATH;
       delete process.env.ARCHON_HOME;
       delete process.env.ARCHON_DOCKER;
       expect(getHomeWorkflowsPath()).toBe(join(homedir(), '.archon', 'workflows'));
     });
     test('uses ARCHON_HOME when set', () => {
+      delete process.env.WORKSPACE_PATH;
       delete process.env.ARCHON_DOCKER;
       process.env.ARCHON_HOME = '/custom/archon';
       expect(getHomeWorkflowsPath()).toBe(join('/custom/archon', 'workflows'));
     });
     test('returns ~/.archon/commands by default', () => {
+      delete process.env.WORKSPACE_PATH;
       delete process.env.ARCHON_HOME;
       delete process.env.ARCHON_DOCKER;
       expect(getHomeCommandsPath()).toBe(join(homedir(), '.archon', 'commands'));
     });
     test('uses ARCHON_HOME when set', () => {
+      delete process.env.WORKSPACE_PATH;
       delete process.env.ARCHON_DOCKER;
       process.env.ARCHON_HOME = '/custom/archon';
       expect(getHomeCommandsPath()).toBe(join('/custom/archon', 'commands'));
     });
     test('returns ~/.archon/.archon/workflows (the retired location)', () => {
+      delete process.env.WORKSPACE_PATH;
       delete process.env.ARCHON_HOME;
       delete process.env.ARCHON_DOCKER;
       expect(getLegacyHomeWorkflowsPath()).toBe(join(homedir(), '.archon', '.archon', 'workflows'));
     });
     test('honors ARCHON_HOME so migration detection works in custom setups', () => {
+      delete process.env.WORKSPACE_PATH;
       delete process.env.ARCHON_DOCKER;
       process.env.ARCHON_HOME = '/custom/archon';
       expect(getLegacyHomeWorkflowsPath()).toBe(join('/custom/archon', '.archon', 'workflows'));
     });

As per coding guidelines, **/*.test.ts: keep tests deterministic.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/paths/src/archon-paths.test.ts` around lines 229 - 288, The tests
for getHomeWorkflowsPath/getHomeCommandsPath/getLegacyHomeWorkflowsPath leave
WORKSPACE_PATH set which can force Docker mode via getArchonHome(); clear
process.env.WORKSPACE_PATH in each affected test setup where you currently
delete ARCHON_HOME and ARCHON_DOCKER so the assertions are deterministic.
Specifically, update the tests that call getHomeWorkflowsPath,
getHomeCommandsPath and getLegacyHomeWorkflowsPath to also delete or unset
process.env.WORKSPACE_PATH alongside ARCHON_HOME and ARCHON_DOCKER before
calling the helper functions.

}

type WorkflowSource = 'project' | 'bundled';
type WorkflowSource = 'project' | 'bundled' | 'global';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Derive WorkflowSource from the schema.

Line 143 duplicates workflowSourceSchema; the next enum change could drift between runtime validation and local typing.

♻️ Proposed type derivation
 import {
+  workflowSourceSchema,
   workflowListResponseSchema,
   validateWorkflowBodySchema,
   validateWorkflowResponseSchema,
   getWorkflowResponseSchema,
-type WorkflowSource = 'project' | 'bundled' | 'global';
+type WorkflowSource = z.infer<typeof workflowSourceSchema>;

As per coding guidelines, **/*.ts: Always use z.infer<typeof schema> for type derivation from Zod schemas.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server/src/routes/api.ts` at line 143, Replace the hard-coded union
type WorkflowSource with a type derived from the Zod schema to avoid drift:
remove "type WorkflowSource = 'project' | 'bundled' | 'global';" and instead
declare "type WorkflowSource = z.infer<typeof workflowSourceSchema>;"
(referencing workflowSourceSchema and the z import already used in the file).

Comment thread packages/server/src/routes/api.ts
Comment on lines +152 to +160
// Use command folder paths with optional configured folder.
// Each scope is walked 1 subfolder deep so `triage/review.md` resolves as
// `review` — matching the workflows/scripts convention. Resolution
// precedence: repo > home (~/.archon/commands/) > bundled/app defaults.
const searchPaths = archonPaths.getCommandFolderSearchPaths(configuredFolder);
const resolvedSearchPaths: string[] = [
...searchPaths.map(folder => join(cwd, folder)),
archonPaths.getHomeCommandsPath(),
];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep home commands ahead of repo defaults.

getCommandFolderSearchPaths() includes .archon/commands/defaults, so appending getHomeCommandsPath() after all searchPaths makes copied/default repo commands win over home-scoped commands. That violates the documented bundled < global < project priority when defaults exist on disk.

Proposed ordering fix
-  const searchPaths = archonPaths.getCommandFolderSearchPaths(configuredFolder);
+  const searchPaths = archonPaths.getCommandFolderSearchPaths(configuredFolder);
+  const repoDefaultPaths = searchPaths.filter(folder => folder === '.archon/commands/defaults');
+  const repoProjectPaths = searchPaths.filter(folder => folder !== '.archon/commands/defaults');
   const resolvedSearchPaths: string[] = [
-    ...searchPaths.map(folder => join(cwd, folder)),
+    ...repoProjectPaths.map(folder => join(cwd, folder)),
     archonPaths.getHomeCommandsPath(),
+    ...repoDefaultPaths.map(folder => join(cwd, folder)),
   ];

Based on learnings, home-scoped workflows, commands, and scripts use load priority: bundled < global < project.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Use command folder paths with optional configured folder.
// Each scope is walked 1 subfolder deep so `triage/review.md` resolves as
// `review` — matching the workflows/scripts convention. Resolution
// precedence: repo > home (~/.archon/commands/) > bundled/app defaults.
const searchPaths = archonPaths.getCommandFolderSearchPaths(configuredFolder);
const resolvedSearchPaths: string[] = [
...searchPaths.map(folder => join(cwd, folder)),
archonPaths.getHomeCommandsPath(),
];
// Use command folder paths with optional configured folder.
// Each scope is walked 1 subfolder deep so `triage/review.md` resolves as
// `review` — matching the workflows/scripts convention. Resolution
// precedence: repo > home (~/.archon/commands/) > bundled/app defaults.
const searchPaths = archonPaths.getCommandFolderSearchPaths(configuredFolder);
const repoDefaultPaths = searchPaths.filter(folder => folder === '.archon/commands/defaults');
const repoProjectPaths = searchPaths.filter(folder => folder !== '.archon/commands/defaults');
const resolvedSearchPaths: string[] = [
...repoProjectPaths.map(folder => join(cwd, folder)),
archonPaths.getHomeCommandsPath(),
...repoDefaultPaths.map(folder => join(cwd, folder)),
];
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/executor-shared.ts` around lines 152 - 160, The
resolvedSearchPaths ordering puts project (cwd) paths before the home commands
path, causing repo defaults to override global/home commands; update the
construction of resolvedSearchPaths so archonPaths.getHomeCommandsPath() appears
before the repo-mapped paths (i.e. build resolvedSearchPaths with
archonPaths.getHomeCommandsPath() first, then ...searchPaths.map(folder =>
join(cwd, folder))), keeping use of
getCommandFolderSearchPaths(configuredFolder), cwd, and
archonPaths.getHomeCommandsPath() intact.

Comment thread packages/workflows/src/validator.ts Outdated
Comment on lines +219 to +228
// 1. Repo search paths
const searchPaths = getCommandFolderSearchPaths(config?.commandFolder);
for (const folder of searchPaths) {
const filePath = join(cwd, folder, `${commandName}.md`);
if (await fileExists(filePath)) {
return filePath;
}
const resolved = await resolveCommandInDir(join(cwd, folder), commandName);
if (resolved) return resolved;
}

// Bundled defaults
// 2. Home-scoped commands (~/.archon/commands/).
const homeResolved = await resolveCommandInDir(getHomeCommandsPath(), commandName);
if (homeResolved) return homeResolved;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Apply the same command precedence in validation.

Because getCommandFolderSearchPaths() includes .archon/commands/defaults, this resolves repo defaults before home commands. Validation should mirror runtime/documented precedence: project commands first, then home, then bundled/default commands.

Proposed ordering fix
   // 1. Repo search paths
   const searchPaths = getCommandFolderSearchPaths(config?.commandFolder);
-  for (const folder of searchPaths) {
+  const repoDefaultPaths = searchPaths.filter(folder => folder === '.archon/commands/defaults');
+  const repoProjectPaths = searchPaths.filter(folder => folder !== '.archon/commands/defaults');
+
+  for (const folder of repoProjectPaths) {
     const resolved = await resolveCommandInDir(join(cwd, folder), commandName);
     if (resolved) return resolved;
   }
 
   // 2. Home-scoped commands (~/.archon/commands/).
   const homeResolved = await resolveCommandInDir(getHomeCommandsPath(), commandName);
   if (homeResolved) return homeResolved;
+
+  for (const folder of repoDefaultPaths) {
+    const resolved = await resolveCommandInDir(join(cwd, folder), commandName);
+    if (resolved) return resolved;
+  }

Based on learnings, home-scoped workflows, commands, and scripts use load priority: bundled < global < project.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/validator.ts` around lines 219 - 228, Validation
currently iterates the full list from getCommandFolderSearchPaths() (which
includes .archon/commands/defaults) before checking home commands, causing repo
defaults to win over home commands; change validator.ts so resolveCommandInDir
is applied in runtime/documented precedence: first check project repo command
folders only (filtering out the defaults path returned by
getCommandFolderSearchPaths()), then call
resolveCommandInDir(getHomeCommandsPath(), commandName) to check home-scoped
commands, and finally fall back to the bundled/default commands path (the
defaults entry from getCommandFolderSearchPaths()) if neither project nor home
resolve; use the existing symbols getCommandFolderSearchPaths,
resolveCommandInDir, getHomeCommandsPath, commandName and cwd to implement the
reordered checks.

Comment on lines +53 to +72
async function maybeWarnLegacyHomePath(): Promise<void> {
if (hasWarnedLegacyHomePath) return;
const legacyPath = archonPaths.getLegacyHomeWorkflowsPath();
const newPath = archonPaths.getHomeWorkflowsPath();
try {
await access(legacyPath);
} catch (error) {
// ENOENT is the happy path — the legacy location isn't in use.
const err = error as NodeJS.ErrnoException;
if (err.code === 'ENOENT') {
hasWarnedLegacyHomePath = true; // no work to do on subsequent calls
return;
}
getLog().debug({ err, legacyPath }, 'workflow.legacy_home_path_probe_error');
return;
}
// Legacy directory exists — surface an actionable migration hint exactly once.
const moveCommand = `mv "${legacyPath}" "${newPath}" && rmdir "${join(archonPaths.getArchonHome(), '.archon')}"`;
getLog().warn({ legacyPath, newPath, moveCommand }, 'workflow.legacy_home_path_detected');
hasWarnedLegacyHomePath = true;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Guard the migration command when the new directory already exists.

Line 70 emits mv "$legacyPath" "$newPath", but if ~/.archon/workflows already exists, mv nests the legacy directory under it instead of merging contents. Users with any new-scope workflows could end up with ~/.archon/workflows/workflows/...; legacy one-level subfolders would then become too deep to discover. Please emit a merge-safe command or branch the warning based on whether newPath already exists, and keep CHANGELOG.md Line 37 in sync.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/workflow-discovery.ts` around lines 53 - 72, The
migration warning in maybeWarnLegacyHomePath builds a naive moveCommand that
will nest the legacy folder if newPath already exists; update the function to
probe newPath (e.g., await access(newPath)) and branch: if newPath does not
exist, keep the current mv command, otherwise emit a merge-safe command (for
example use mv -T "<legacyPath>" "<newPath>" or an rsync/ditto style merge
command that preserves/merges contents and removes the source), and include that
alternative command in the getLog().warn payload (still setting
hasWarnedLegacyHomePath=true afterwards); ensure the change message matches the
corresponding CHANGELOG.md entry.

@Wirasm
Copy link
Copy Markdown
Collaborator Author

Wirasm commented Apr 20, 2026

PR Review Summary — Multi-Agent Pass

Six specialist agents reviewed this PR (code-reviewer, docs-impact, pr-test-analyzer, silent-failure-hunter, type-design-analyzer, code-simplifier). Convergent findings from 2+ agents are flagged with [N agents].

Critical Issues (3)

Agent(s) Issue Location Fix
[5 agents] code-reviewer / pr-test-analyzer / silent-failure-hunter / type-design / code-simplifier GET /api/commands calls findMarkdownFilesRecursive without { maxDepth: 1 }, so the Web UI palette lists commands from arbitrarily deep subfolders that loadCommandPrompt/resolveCommand (both capped at depth 1) cannot resolve — phantom entries that fail at execution. packages/server/src/routes/api.ts:2313, 2329 (new home-commands block), 2331 Pass '', { maxDepth: 1 } to all three calls.
silent-failure-hunter discoverAvailableScripts broad-catches the actionable Duplicate script name error from scanScriptDir and silently returns []. Users with two same-basename scripts see the entire script list disappear from validator output and the Web UI with no diagnostic. packages/workflows/src/validator.ts:616-624 Re-throw on 'Duplicate script name'; only swallow ENOENT/EACCES. Or just propagate.
[2 agents] code-reviewer / silent-failure-hunter discoverScriptsForCwd calls discoverScripts(getHomeScriptsPath()) before the repo path. A duplicate-name error in ~/.archon/scripts/ aborts the whole call — repo-scoped scripts never load and a workflow with a script: node referencing a repo-local script fails with a cryptic error pointing at the user's home dir. packages/workflows/src/script-discovery.ts:146-159 Wrap home-scope discoverScripts in a try/catch; log + fall back to empty Map so home failures cannot kill repo-scope discovery.

Important Issues (5)

Agent(s) Issue Location Fix
[2 agents] code-reviewer / silent-failure-hunter maybeWarnLegacyHomePath non-ENOENT branch: (a) does not set hasWarnedLegacyHomePath = true, so the probe re-runs on every discovery call for the process lifetime; (b) on EACCES (legacy dir present but unreadable) the user never sees the migration WARN — they have stranded workflows and no instruction. packages/workflows/src/workflow-discovery.ts:59-67 Set the flag in both branches; promote non-ENOENT to warn level so EACCES on legacy dir surfaces the migration message.
silent-failure-hunter findMarkdownFilesRecursive propagates EACCES from a single subfolder up through every non-wrapping caller (validator.ts:152/159/175/195, executor-shared.ts:163). A chmod 700 subdir under ~/.archon/commands/ crashes command discovery rather than skipping. maxDepth: 1 reduces but does not eliminate the surface. packages/paths/src/archon-paths.ts:217-224 Catch EACCES on the recursive subfolder call, log+continue; let other errors propagate.
silent-failure-hunter discoverAvailableCommands home-commands findMarkdownFilesRecursive call is unwrapped — inconsistent with the equivalent block in api.ts step 3, which discriminates ENOENT vs other errors. packages/workflows/src/validator.ts:159-164 Wrap with try/catch, tolerate ENOENT, log other errors.
[2 agents] type-design / code-reviewer (PR body acknowledges) packages/web/src/lib/api.generated.d.ts is hand-edited to add 'global' to WorkflowSource. Next bun --filter @archon/web generate:types overwrites the edit — frontend type errors will appear on regen. packages/web/src/lib/api.generated.d.ts:2351 Run bun run dev:server && bun --filter @archon/web generate:types before merge so the file reflects the actual OpenAPI spec (Zod already has 'global').
[2 agents] type-design / code-simplifier findMarkdownFilesRecursive is implemented twice with byte-identical bodies (paths copy has the new maxDepth + ENOENT guard; core copy has it too). packages/paths/src/archon-paths.ts:207 and packages/core/src/utils/commands.ts:15 Re-export from @archon/paths in packages/core/src/utils/commands.ts (@archon/server already depends on @archon/core which depends on @archon/paths).

Test Gaps (pr-test-analyzer)

Rating Gap Suggested Test
8/10 No test asserts a 2-level-deep command does NOT appear in GET /api/commands. Would catch the Critical #1 above. api.workflows.test.ts — place file at <homeDir>/commands/a/b/too-deep.md, assert it's absent; add same at a/ok.md, assert it's present.
7/10 loadCommandPrompt home-scope behavior is not tested at all in executor-shared.test.ts. resolveCommand (path lookup) is covered; loadCommandPrompt (content load) is not. The walk-then-read idiom is new. Add suite: home-only command resolves; repo wins over same-named home; ENOENT on home dir falls through to bundled silently; 1-level subfolder resolves by basename.
6/10 loader.test.ts:779 asserts moveCommand contains 'mv'. It does not assert the && rmdir "<archonHome>/.archon" clause is correct. If the rmdir target ever points at the new path, users following the printed command would delete their migrated workflows. Add expect(...).toMatch(/rmdir.*\.archon["']/) and not.toMatch(/rmdir.*workflows/).
5/10 getHomeScriptsPath is missing the Docker / ARCHON_HOME / default-HOME triplet that getHomeWorkflowsPath and getHomeCommandsPath have. Cheap regression guard against the original double-.archon bug. Mirror existing tests in archon-paths.test.ts.
5/10 2-level-deep silent-skip is tested for workflows + scripts but not for commands. One unit test in validator.test.ts.

Documentation Gaps (docs-impact)

File Issue Fix
packages/docs-web/src/content/docs/reference/archon-directories.md:27-43 The ~/.archon/ tree omits the new direct children (workflows/, commands/, scripts/). This doc was missed by the PR. Add the three lines + corresponding Purpose entries.
packages/docs-web/src/content/docs/reference/api.md:218, 311 GET /api/workflows/:name and GET /api/commands document source as a 2-variant union. After this PR it's 3-variant ('project' | 'bundled' | 'global'). Add 'global' to both.
CLAUDE.md:548-562 The Archon Directory Structure tree diagram does not include workflows/, commands/, scripts/ — only the prose section below (lines 723-731) does. The two are now inconsistent within the same file. Add the three entries to the tree.
packages/docs-web/src/content/docs/getting-started/concepts.md:36-47 Pre-existing gap made more visible by this PR: node-types table omits script:. With home-scoped scripts now first-class in global-workflows.md, this omission is more likely to confuse. Add a script: row.

Suggestions (code-simplifier)

Suggestion Location Notes
Extract CommandSection helper component — three structurally identical JSX blocks (project / global / bundled) now meet the rule-of-three. packages/web/src/components/workflows/NodePalette.tsx:72-133 Cosmetic; preserves exact behavior.

Strengths

  • Removing globalSearchPath is the right structural fix — eliminates the bug class by making the only correct behavior the only available behavior. (fix: pass globalSearchPath to command handler workflow discovery #1136 is properly closed by this approach, not patched at one of six sites.)
  • getLegacyHomeWorkflowsPath is deliberately narrow — exported only for detection, never for loading. Correct intent boundary.
  • scanScriptDir correctly re-throws non-ENOENT and the duplicate-name error message is actionable. (The bug is downstream — discoverAvailableScripts swallows it.)
  • Repo-workflow folder access failure in workflow-discovery.ts:331-338 correctly re-throws non-ENOENT — the project/home asymmetry is intentional and right.
  • The "warns exactly once per process" test is correctly constructed with module-scoped flag reset between tests.
  • WorkflowSource reused for commands rather than introducing a parallel CommandSource for identical semantics.

Type Design Ratings

Dimension Score
Encapsulation 9/10
Invariant Expression 8/10
Invariant Usefulness 8/10
Invariant Enforcement 7/10 (generated-file hand-edit is the gap)

Verdict: NEEDS FIXES

The structural design is sound — the agents converged on this. The blockers are correctness issues introduced by the new code, not design choices:

  1. api.ts missing maxDepth: 1 — palette/executor mismatch (5/6 agents flagged it independently)
  2. discoverAvailableScripts swallows duplicate-name error
  3. discoverScriptsForCwd home failure aborts repo discovery
  4. Regenerate api.generated.d.ts before merge

Recommended Action Order

  1. Fix the three Critical issues (api.ts depth, validator's swallowed duplicate-name, script-discovery home isolation) — small targeted edits, no design change.
  2. Tighten maybeWarnLegacyHomePath (set flag + promote non-ENOENT to warn).
  3. Wrap findMarkdownFilesRecursive EACCES at the recursive call site.
  4. Run bun run dev:server + bun --filter @archon/web generate:types and re-commit api.generated.d.ts.
  5. Add the high-value tests (Critical Model stucked at response stream text #1 regression test + loadCommandPrompt home-scope suite + moveCommand rmdir assertion).
  6. Update the four doc files (archon-directories.md, api.md, CLAUDE.md tree, concepts.md).
  7. Optional polish: collapse findMarkdownFilesRecursive duplicate (re-export from @archon/paths); extract CommandSection.

The 4 new tests in `scanScriptDir depth cap` and `discoverScriptsForCwd —
merge repo + home with repo winning` compared incoming mock paths with
hardcoded forward-slash strings (`if (path === '/scripts/triage')`). On
Windows, `path.join('/scripts', 'triage')` produces `\scripts\triage`, so
those branches never matched, readdir returned `[]`, and the tests failed.

Added a `norm()` helper at module scope and wrapped the incoming `path`
argument in every `mockImplementation` before comparing. Stored paths go
through `normalizeSep()` in production code, so the existing equality
assertions on `script.path` remain OS-independent.

Fixes Windows CI job `test (windows-latest)` on PR #1315.
@Wirasm
Copy link
Copy Markdown
Collaborator Author

Wirasm commented Apr 20, 2026

PR Review Summary — 6 specialist agents

CI Fix Pushed

  • 02b8fb66 — normalizes path separators in script-discovery.test.ts mock comparators; the 4 Windows failures at lines 186/223/237/251 came from path.join producing \scripts\triage on Windows while mocks compared against hardcoded '/scripts/triage'. Production paths already use normalizeSep(), so only the mocks needed fixing.

Critical Issues (2)

# Agent(s) Issue Location
1 code-reviewer, silent-failure-hunter, type-design-analyzer findMarkdownFilesRecursive called without maxDepth: 1 for bundled / home / project command listings. Defaults to Infinity. UI palette will surface commands from subfolders >1 level deep that the executor (capped at 1) cannot resolve → silent "command not found" at runtime. packages/server/src/routes/api.ts (3 call sites — ~2314, ~2329, ~2346)
2 silent-failure-hunter EACCES/EPERM on ~/.archon/commands/ propagates uncaught through discoverAvailableCommands and resolveCommandInDir. Validator crashes with raw filesystem error; no user-actionable message. Only ENOENT is handled. packages/workflows/src/validator.ts (home-commands block + resolveCommandInDir)

Important Issues (4)

# Agent Issue Location
3 silent-failure-hunter maybeWarnLegacyHomePath catches non-ENOENT errors at DEBUG and returns without setting hasWarnedLegacyHomePath=true. Result: EACCES on legacy path re-probes every discovery call, user never sees the permission issue. packages/workflows/src/workflow-discovery.ts
4 silent-failure-hunter discoverScriptsForCwd throws on EACCES (via scanScriptDir), but callers in dag-executor.ts (executeScriptNode) have no try/catch → unhandled crash bypasses safeSendMessage/logNodeError. Same for validateScript / validateWorkflowResources. packages/workflows/src/dag-executor.ts, validator.ts
5 code-reviewer maybeWarnLegacyHomePath sets hasWarnedLegacyHomePath=true after await access(). Concurrent callers (server startup with parallel codebase resolution) can both pass the guard → double-warn. Set the flag eagerly before the await. packages/workflows/src/workflow-discovery.ts
6 pr-test-analyzer (rating 8/10) loadCommandPrompt is the runtime command resolution path and received a material code change (added home-scope + findMarkdownFilesRecursive walk). Zero unit tests exist for it. Coverage for resolveCommand (validator) doesn't exercise the executor path. packages/workflows/src/executor-shared.ts

Suggestions (4)

# Agent Suggestion Location
7 code-reviewer .archon/workflows/e2e-home-scope.yaml is a one-off smoke test — ships permanently in everyone's workflow list. Same coverage exists in loader.test.ts. Remove. .archon/workflows/e2e-home-scope.yaml
8 comment-analyzer MAX_DISCOVERY_DEPTH comment leads with "0 = only files at the root" while the constant is 1 — causes a reader double-take. Reorder to lead with the actual value. packages/workflows/src/workflow-discovery.ts
9 comment-analyzer MAX_SCRIPT_DISCOVERY_DEPTH comment lacks the "why not deeper" rationale that MAX_DISCOVERY_DEPTH has ("adds no organizational value, just routing ambiguity"). Copy the sentence across. packages/workflows/src/script-discovery.ts
10 pr-test-analyzer (rating 5/10) getHomeScriptsPath has no unit test; getHomeWorkflowsPath, getHomeCommandsPath, and getLegacyHomeWorkflowsPath all do. Asymmetric — add one for consistency. packages/paths/src/archon-paths.test.ts

Strengths

  • WorkflowSource / CommandEntry.source correctly defined once in the engine schema and propagated to frontend via OpenAPI — no drift (type-design-analyzer 8/10 usefulness).
  • getLegacyHomeWorkflowsPath docstring ("signal, not a source") is load-bearing and well-written.
  • maybeWarnLegacyHomePath comment block documents the process-scoped invariant and test escape hatch cleanly.
  • discoverScriptsForCwd JSDoc makes cross-scope vs within-scope duplicate distinction explicit.
  • Legacy-path deprecation tests cover all four cases (warns, doesn't load, fires once, silent when absent).
  • Repo-over-home precedence tested for all three types (workflows, commands, scripts).

Type Design Ratings

Dimension Score
Encapsulation 7/10
Invariant expression 6/10
Usefulness 8/10
Enforcement 5/10 (primarily issue #1 — depth cap is opt-in and 3 call sites forget it)

Verdict: NEEDS FIXES

Blocking:

Recommended before merge:

Nice-to-have:

  • Remove the e2e smoke workflow, tidy the two comment issues, add the missing getHomeScriptsPath unit test.

Critical fixes:
- api.ts: add `maxDepth: 1` to all 3 findMarkdownFilesRecursive calls in
  GET /api/commands (bundled/home/project). Without this the UI palette
  surfaced commands from deep subfolders that the executor (capped at 1)
  could not resolve — silent "command not found" at runtime.
- validator.ts: wrap home-scope findMarkdownFilesRecursive and
  resolveCommandInDir calls in try/catch so EACCES/EPERM on
  ~/.archon/commands/ doesn't crash the validator with a raw filesystem
  error. ENOENT still returns [] via the underlying helper.

Error handling fixes:
- workflow-discovery.ts: maybeWarnLegacyHomePath now sets the
  "warned-once" flag eagerly before `await access()`, so concurrent
  discovery calls (server startup with parallel codebase resolution)
  can't double-warn. Non-ENOENT probe errors (EACCES/EPERM) now log at
  WARN instead of DEBUG so permission issues on the legacy dir are
  visible in default operation.
- dag-executor.ts: wrap discoverScriptsForCwd in its own try/catch so
  an EACCES on ~/.archon/scripts/ routes through safeSendMessage /
  logNodeError with a dedicated "failed to discover scripts" message
  instead of being mis-attributed by the outer catch's
  "permission denied (check cwd permissions)" branch.

Tests:
- load-command-prompt.test.ts (new): 6 tests covering the executor's
  command resolution hot path — home-scope resolves when repo misses,
  repo shadows home, 1-level subfolder resolvable by basename, 2-level
  rejected, not-found, empty-file. Runs in its own bun test batch.
- archon-paths.test.ts: add getHomeScriptsPath describe block to match
  the existing getHomeCommandsPath / getHomeWorkflowsPath coverage.

Comment clarity:
- workflow-discovery.ts: MAX_DISCOVERY_DEPTH comment now leads with the
  actual value (1) before describing what 0 would mean.
- script-discovery.ts: copy the "routing ambiguity" rationale from
  MAX_DISCOVERY_DEPTH to MAX_SCRIPT_DISCOVERY_DEPTH.

Cleanup:
- Remove .archon/workflows/e2e-home-scope.yaml — one-off smoke test that
  would ship permanently in every project's workflow list. Equivalent
  coverage exists in loader.test.ts.

Addresses all blocking and important feedback from the multi-agent
review on PR #1315.
@Wirasm
Copy link
Copy Markdown
Collaborator Author

Wirasm commented Apr 20, 2026

Review feedback addressed — pushed in 987f186a

Blocking:

  • api.ts — added maxDepth: 1 to all 3 findMarkdownFilesRecursive calls in GET /api/commands; UI palette now matches executor resolution.
  • validator.ts — wrapped home-scope calls in discoverAvailableCommands and resolveCommand with try/catch (logs commands.home_discovery_failed / commands.home_resolve_failed); EACCES/EPERM on ~/.archon/commands/ no longer crashes the validator.

Important:

  • workflow-discovery.ts maybeWarnLegacyHomePath — sets hasWarnedLegacyHomePath = true eagerly (before await access) to close the concurrent-double-warn race; upgraded non-ENOENT log from DEBUG to WARN so permission issues on the legacy dir are visible.
  • dag-executor.ts — wrapped discoverScriptsForCwd in its own try/catch that routes through safeSendMessage / logNodeError with a dedicated "failed to discover scripts" message. Previously, EACCES would be misattributed by the outer catch's "permission denied (check cwd permissions)" branch.
  • Added packages/workflows/src/load-command-prompt.test.ts — 6 tests for the executor's command resolution hot path: home resolves when repo misses, repo shadows home, 1-level subfolder resolves by basename, 2-level rejected, not-found, empty-file. Runs in its own bun test batch.

Cleanup:

  • Removed .archon/workflows/e2e-home-scope.yaml — one-off smoke that would have shipped permanently; equivalent coverage is in loader.test.ts.
  • MAX_DISCOVERY_DEPTH comment now leads with the actual value 1 (not 0).
  • Copied "routing ambiguity" rationale from MAX_DISCOVERY_DEPTH to MAX_SCRIPT_DISCOVERY_DEPTH.
  • Added getHomeScriptsPath unit tests to archon-paths.test.ts (matches sibling coverage).

bun run validate passes (check:bundled + type-check + lint + format:check + all tests across 10 packages).

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/server/src/routes/api.ts (1)

2129-2183: ⚠️ Potential issue | 🟠 Major

Add global workflow fallback to single-workflow fetch route.

GET /api/workflows can list workflows with source: 'global' via discoverWorkflowsWithConfig(), but GET /api/workflows/:name doesn't check the home/global path before falling back to bundled defaults. A global-only workflow shown in the UI will 404 when opened by name, breaking editor and detail flows.

Import getHomeWorkflowsPath and add a fallback between project and bundled lookups:

Proposed fix
 import {
   createLogger,
   getWorkflowFolderSearchPaths,
   getCommandFolderSearchPaths,
   getDefaultCommandsPath,
   getDefaultWorkflowsPath,
   getArchonWorkspacesPath,
   getHomeCommandsPath,
+  getHomeWorkflowsPath,
   getRunArtifactsPath,
-      // 2. Fall back to bundled defaults (binary: embedded map; dev: also check filesystem)
+      // 2. Fall back to home-scoped workflows
+      try {
+        const homeFilePath = join(getHomeWorkflowsPath(), filename);
+        const content = await readFile(homeFilePath, 'utf-8');
+        const result = parseWorkflow(content, filename);
+        if (result.error) {
+          return apiError(c, 500, `Home workflow file is invalid: ${result.error.error}`);
+        }
+        return c.json({
+          workflow: result.workflow,
+          filename,
+          source: 'global' as WorkflowSource,
+        });
+      } catch (err) {
+        if ((err as NodeJS.ErrnoException).code !== 'ENOENT') {
+          getLog().error({ err, name }, 'workflow.fetch_home_failed');
+          return apiError(c, 500, 'Failed to read home workflow');
+        }
+      }
+
+      // 3. Fall back to bundled defaults (binary: embedded map; dev: also check filesystem)
       if (Object.hasOwn(BUNDLED_WORKFLOWS, name)) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server/src/routes/api.ts` around lines 2129 - 2183, Add a
home/global fallback check between the project and bundled lookups: import
getHomeWorkflowsPath, then after the project lookup block (the try/catch using
getWorkflowFolderSearchPaths and readFile) attempt to read
join(getHomeWorkflowsPath(), filename), parse it with parseWorkflow and return
the workflow like the other branches (source: 'global' or similar), handling
parse errors via apiError and treating ENOENT as non-fatal while logging other
errors with getLog().error (use a distinct log name such as
'workflow.fetch_home_failed'); keep the existing bundled and default fallback
logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/workflows/src/validator.ts`:
- Around line 624-635: The catch in discoverAvailableScripts currently swallows
all errors; modify it so only filesystem-related errors are handled and other
application errors (like duplicate-name errors thrown in discoverScriptsForCwd /
script-discovery.ts line that detects duplicate script names) propagate: in
discoverAvailableScripts, catch the error, inspect it (e.g., err.code) and if it
is one of filesystem codes (ENOENT, EACCES, EPERM) log the warning via
getLog().warn({ err, cwd }, 'script_discovery_failed') and return [], otherwise
rethrow the error so duplicate-name and other logic errors bubble up.

---

Outside diff comments:
In `@packages/server/src/routes/api.ts`:
- Around line 2129-2183: Add a home/global fallback check between the project
and bundled lookups: import getHomeWorkflowsPath, then after the project lookup
block (the try/catch using getWorkflowFolderSearchPaths and readFile) attempt to
read join(getHomeWorkflowsPath(), filename), parse it with parseWorkflow and
return the workflow like the other branches (source: 'global' or similar),
handling parse errors via apiError and treating ENOENT as non-fatal while
logging other errors with getLog().error (use a distinct log name such as
'workflow.fetch_home_failed'); keep the existing bundled and default fallback
logic unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fffe2a71-cd8d-44df-ba20-ad932840bbc2

📥 Commits

Reviewing files that changed from the base of the PR and between 02b8fb6 and 987f186.

📒 Files selected for processing (8)
  • packages/paths/src/archon-paths.test.ts
  • packages/server/src/routes/api.ts
  • packages/workflows/package.json
  • packages/workflows/src/dag-executor.ts
  • packages/workflows/src/load-command-prompt.test.ts
  • packages/workflows/src/script-discovery.ts
  • packages/workflows/src/validator.ts
  • packages/workflows/src/workflow-discovery.ts
✅ Files skipped from review due to trivial changes (2)
  • packages/workflows/package.json
  • packages/paths/src/archon-paths.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/workflows/src/script-discovery.ts
  • packages/workflows/src/workflow-discovery.ts

Comment on lines 624 to 635
export async function discoverAvailableScripts(
cwd: string
): Promise<{ name: string; path: string; runtime: ScriptRuntime }[]> {
const scriptsDir = resolve(cwd, '.archon', 'scripts');
try {
const scripts = await discoverScripts(scriptsDir);
const scripts = await discoverScriptsForCwd(cwd);
return [...scripts.values()].map(s => ({ name: s.name, path: s.path, runtime: s.runtime }));
} catch (error) {
const err = error as Error;
getLog().warn({ err, scriptsDir }, 'script_discovery_failed');
getLog().warn({ err, cwd }, 'script_discovery_failed');
return [];
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm discoverScriptsForCwd actually throws on duplicates (vs. silently deduping),
# and see exactly what non-ENOENT errors it surfaces.
fd -t f script-discovery.ts packages/workflows
rg -nP -C3 '(throw new Error|Duplicate script|EACCES|ENOENT)' --type=ts -g 'script-discovery*.ts'

Repository: coleam00/Archon

Length of output: 3706


🏁 Script executed:

cat -n packages/workflows/src/script-discovery.ts | head -120

Repository: coleam00/Archon

Length of output: 4761


🏁 Script executed:

# Check the exact current state of discoverAvailableScripts in validator.ts
sed -n '620,640p' packages/workflows/src/validator.ts

Repository: coleam00/Archon

Length of output: 794


🏁 Script executed:

# Search for all error handling patterns in script-discovery to see all possible thrown errors
rg -n 'throw|EACCEs|EPERM' packages/workflows/src/script-discovery.ts

Repository: coleam00/Archon

Length of output: 256


🏁 Script executed:

# Find the full implementation of discoverScriptsForCwd
rg -n 'async function discoverScriptsForCwd' packages/workflows/src/ -A 30

Repository: coleam00/Archon

Length of output: 1880


🏁 Script executed:

# Check if EACCES/EPERM can realistically occur from readdir/stat
# and verify all error paths in script-discovery
rg -n 'readdir|stat' packages/workflows/src/script-discovery.ts

Repository: coleam00/Archon

Length of output: 260


🏁 Script executed:

# Find the discoverScripts function (called by discoverScriptsForCwd)
rg -n 'async function discoverScripts\(' packages/workflows/src/script-discovery.ts -A 15

Repository: coleam00/Archon

Length of output: 813


🏁 Script executed:

# Verify what errors can escape from scanScriptDir up through discoverScripts to discoverScriptsForCwd
sed -n '125,162p' packages/workflows/src/script-discovery.ts

Repository: coleam00/Archon

Length of output: 1714


Narrow the catch to allow duplicate-name errors and other application-logic errors to propagate.

The current broad catch (error) swallows every error and returns [], which contradicts the PR objective and test expectations (line 121 of script-discovery.test.ts expects duplicates to throw). The underlying discovery chain throws on duplicate script names (script-discovery.ts line 113) and directory read failures; only filesystem-specific errors (ENOENT, EACCES, EPERM) should be caught and silently ignored.

Narrow to:

 export async function discoverAvailableScripts(
   cwd: string
 ): Promise<{ name: string; path: string; runtime: ScriptRuntime }[]> {
   try {
     const scripts = await discoverScriptsForCwd(cwd);
     return [...scripts.values()].map(s => ({ name: s.name, path: s.path, runtime: s.runtime }));
   } catch (error) {
-    const err = error as Error;
-    getLog().warn({ err, cwd }, 'script_discovery_failed');
-    return [];
+    const err = error as NodeJS.ErrnoException;
+    if (err.code === 'ENOENT' || err.code === 'EACCES' || err.code === 'EPERM') {
+      getLog().warn({ err, cwd }, 'script_discovery_fs_error');
+      return [];
+    }
+    // Duplicate-name (and other programmatic) errors are actionable — surface them.
+    throw err;
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/validator.ts` around lines 624 - 635, The catch in
discoverAvailableScripts currently swallows all errors; modify it so only
filesystem-related errors are handled and other application errors (like
duplicate-name errors thrown in discoverScriptsForCwd / script-discovery.ts line
that detects duplicate script names) propagate: in discoverAvailableScripts,
catch the error, inspect it (e.g., err.code) and if it is one of filesystem
codes (ENOENT, EACCES, EPERM) log the warning via getLog().warn({ err, cwd },
'script_discovery_failed') and return [], otherwise rethrow the error so
duplicate-name and other logic errors bubble up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant