fix(server): wire ~/.archon/workflows/ into single-workflow REST endpoints#1525
fix(server): wire ~/.archon/workflows/ into single-workflow REST endpoints#1525blankse wants to merge 1 commit intocoleam00:devfrom
Conversation
…ingle-workflow REST endpoints
Background
`GET /api/workflows` (list) reads home-scoped workflows correctly via
`discoverWorkflowsWithConfig` — files placed under `~/.archon/workflows/`
appear in the dashboard, run successfully, and persist across projects
exactly as documented in CLAUDE.md (`bundled < global < project`).
The single-workflow REST endpoints (`GET`, `PUT`, `DELETE
/api/workflows/{name}`) never got the memo. Their lookup ladder skips
straight from project-scoped to bundled, so any home-scoped workflow
returns 404 from `GET`. The Web UI's run-detail page calls `getWorkflow`
to render the DAG graph; the 404 leaves it stuck on "Loading graph…"
forever.
`PUT` and `DELETE` had a related but distinct bug: when no codebase is
registered they fell back to `workingDir = getArchonHome()` and joined
the workflow folder as `<archonHome>/.archon/workflows/` — which is the
legacy pre-0.x location whose only remaining purpose is to emit the
migration warning. New workflows created from the dashboard with no
project landed there silently and stayed invisible to discovery.
Closes coleam00#1524.
Change
1. `GET /api/workflows/{name}` gains a step 2 between project-scoped and
bundled: read `getHomeWorkflowsPath()/<name>.yaml` and return
`source: 'global'` on hit. Mirrors the discovery layer's documented
priority. ENOENT falls through; any other error surfaces as 500.
2. `PUT /api/workflows/{name}`: when no `cwd` and no codebase exists,
write to `getHomeWorkflowsPath()` directly (`source: 'global'`)
instead of joining `getArchonHome()` with the project workflow
folder. Project-scoped saves are unchanged.
3. `DELETE /api/workflows/{name}`: same correction as PUT — operate on
`getHomeWorkflowsPath()` for the no-project case.
The `WorkflowSource` type already includes `'global'`; the discovery
code already labels home-scoped workflows that way. The endpoints now
agree with the rest of the system.
Tests
- New: GET returns `source: 'global'` for a home-scoped file.
- New: GET project-shadows-home (regression cover for the documented
priority).
- New: DELETE removes a real home-scoped file when no project is
registered, and verifies the file actually disappears from
`~/.archon/workflows/`.
- Updated: the existing "fall back to getArchonHome() when no cwd"
PUT test now asserts `source: 'global'` and verifies the file lands
at `~/.archon/workflows/<name>.yaml`, not at the legacy
`<archonHome>/.archon/workflows/...` path.
- Tests pull `ARCHON_HOME` via `process.env`, matching the existing
pattern in this file.
Out of scope
`WorkflowExecution.tsx` shows "Loading graph…" without a timeout or
error path when the GET returns anything other than 200 — flagged in
coleam00#1524 as a separate Web UI follow-up. Not touched here.
📝 WalkthroughWalkthroughThe workflow API endpoints now implement tiered workflow discovery: GET reads from project-scoped, then home-scoped (~/.archon/workflows/), returning Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/server/src/routes/api.workflows.test.ts`:
- Around line 256-293: The test mutates process.env.ARCHON_HOME and the finally
block unconditionally deletes it; capture the original value before setting
ARCHON_HOME (e.g., const prevArchonHome = process.env.ARCHON_HOME), set
process.env.ARCHON_HOME = testArchonHome for the test, and in the finally block
restore it (process.env.ARCHON_HOME = prevArchonHome or delete if prevArchonHome
is undefined) instead of always deleting; apply the same pattern to the other
affected tests (the blocks around lines 295-334, 397-437, 537-569) to avoid
leaking environment state.
🪄 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: d109f173-1b47-4e93-b7d3-8e8bbd72cb68
📒 Files selected for processing (2)
packages/server/src/routes/api.tspackages/server/src/routes/api.workflows.test.ts
| test('returns home-scoped workflow with source:global when only ~/.archon/workflows/ has it', async () => { | ||
| // Regression for #1524: home-scoped workflows are listed by the | ||
| // dashboard but were unfetchable via GET, breaking the run detail page. | ||
| const testArchonHome = join(tmpdir(), `archon-home-get-${Date.now()}`); | ||
| const homeWorkflowDir = join(testArchonHome, 'workflows'); | ||
| await mkdir(homeWorkflowDir, { recursive: true }); | ||
| await writeFile( | ||
| join(homeWorkflowDir, 'team-shared.yaml'), | ||
| 'name: team-shared\ndescription: shared\nnodes:\n - id: n\n command: c\n' | ||
| ); | ||
| process.env.ARCHON_HOME = testArchonHome; | ||
|
|
||
| try { | ||
| const app = createTestApp(); | ||
| registerApiRoutes(app, {} as WebAdapter, {} as ConversationLockManager); | ||
|
|
||
| // No project-scoped match; falls through to home-scoped. | ||
| mockListCodebases.mockImplementationOnce(async () => []); | ||
| mockParseWorkflow.mockReturnValueOnce({ | ||
| workflow: makeTestWorkflow({ name: 'team-shared', description: 'shared' }), | ||
| error: null, | ||
| }); | ||
|
|
||
| const response = await app.request('/api/workflows/team-shared'); | ||
| expect(response.status).toBe(200); | ||
| const body = (await response.json()) as { | ||
| source: string; | ||
| filename: string; | ||
| workflow: { name: string }; | ||
| }; | ||
| expect(body.source).toBe('global'); | ||
| expect(body.filename).toBe('team-shared.yaml'); | ||
| expect(body.workflow.name).toBe('team-shared'); | ||
| } finally { | ||
| delete process.env.ARCHON_HOME; | ||
| await rm(testArchonHome, { recursive: true, force: true }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Restore any pre-existing ARCHON_HOME instead of always deleting it.
These tests mutate a process-global env var, but finally always deletes it. If the runner already had ARCHON_HOME set, later tests inherit the wrong environment and the suite becomes order-dependent.
Suggested fix
- process.env.ARCHON_HOME = testArchonHome;
+ const previousArchonHome = process.env.ARCHON_HOME;
+ process.env.ARCHON_HOME = testArchonHome;
try {
// ...
} finally {
- delete process.env.ARCHON_HOME;
+ if (previousArchonHome === undefined) {
+ delete process.env.ARCHON_HOME;
+ } else {
+ process.env.ARCHON_HOME = previousArchonHome;
+ }
await rm(testArchonHome, { recursive: true, force: true });
}As per coding guidelines, **/*.test.ts: "keep tests deterministic — no flaky timing or network dependence without guardrails".
Also applies to: 295-334, 397-437, 537-569
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/api.workflows.test.ts` around lines 256 - 293, The
test mutates process.env.ARCHON_HOME and the finally block unconditionally
deletes it; capture the original value before setting ARCHON_HOME (e.g., const
prevArchonHome = process.env.ARCHON_HOME), set process.env.ARCHON_HOME =
testArchonHome for the test, and in the finally block restore it
(process.env.ARCHON_HOME = prevArchonHome or delete if prevArchonHome is
undefined) instead of always deleting; apply the same pattern to the other
affected tests (the blocks around lines 295-334, 397-437, 537-569) to avoid
leaking environment state.
Summary
GET,PUT, andDELETE /api/workflows/{name}all skip the home-scoped layer (~/.archon/workflows/), even though the list endpoint advertises those workflows in the dashboard. End-result: a home-scoped workflow shows up in the list, can be run from the CLI, but the run-detail page is stuck on "Loading graph…" forever because the GET that fetches the DAG topology returns 404. Saving from the dashboard with no project selected lands the YAML in~/.archon/.archon/workflows/(the legacy pre-0.x path the migration warning explicitly tells users to abandon).~/.archon/workflows/. Discovery sees them, the dashboard lists them, runs work — only the detail page hangs. CLAUDE.md documents the prioritybundled < global < project; the single-workflow endpoints just walkproject → bundled, missing the middle layer entirely.GETnow readsgetHomeWorkflowsPath()/<name>.yamlbetween the project and bundled tiers, returningsource: 'global'.PUT/DELETEnow operate on~/.archon/workflows/(not~/.archon/.archon/workflows/) when no project is registered. TheWorkflowSourcetype already had'global'; the discovery layer already used it.WorkflowExecution.tsx's "Loading graph…" state with no error path. Even after the API fix, an actual 404 / network error still hangs the UI forever — flagged in bug(server): GET/PUT/DELETE /api/workflows/{name} miss home-scoped (~/.archon/workflows/) — workflow detail page hangs #1524 as a separate Web UI follow-up.UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
getWorkflowRoute→<cwd>/.archon/workflows/getWorkflowRoute→~/.archon/workflows/getWorkflowRoute→BUNDLED_WORKFLOWSsaveWorkflowRoute→<cwd>/.archon/workflows/saveWorkflowRoute→~/.archon/workflows/~/.archon/.archon/workflows/(legacy)deleteWorkflowRoute→~/.archon/workflows/discoverWorkflowsWithConfig) →~/.archon/workflows/Label Snapshot
risk: lowsize: Sserverserver:api-routesChange Metadata
bugserverLinked Issue
Validation Evidence
api.workflows.test.tsadds three regression tests covering the GET home-scoped hit, the project-shadows-home priority, and DELETE on a real home-scoped file. The existing PUT "no cwd / no codebases" test was updated to verify the YAML lands in~/.archon/workflows/(not the legacy path) and is reported withsource: 'global'. Pre-existingloader.test.ts/dag-executor.test.tsfailures ondevreproduce on a clean checkout in this WSL2 environment without my changes — unrelated.bun run lintwithout raised heap OOMs in WSL2 ondev(pre-existing). Ran withNODE_OPTIONS='--max-old-space-size=8192'and got clean.curl http://localhost:3000/api/workflows/df-implement-with-preview-fastagainst a fresh server before the fix → 404 (workflow at~/.archon/workflows/...); after the fix returns 200 withsource: 'global'and the run-detail page renders the DAG.Security Impact
~/.archon/workflows/was already read by the discovery code path; this PR adds read access for the same path from the GET handler and redirects PUT/DELETE writes from the legacy~/.archon/.archon/workflows/to~/.archon/workflows/. No new directory the server didn't already touch via discovery.Compatibility / Migration
PUT/DELETEchange where no-project saves land: previously~/.archon/.archon/workflows/, now~/.archon/workflows/. Anyone with workflows already saved at the legacy path continues to be told byworkflow-discovery.tsto migrate via the documentedmvcommand (warning is unchanged). New saves go to the correct location automatically.Human Verification
source: 'global'(real fixture under tmpdir +process.env.ARCHON_HOME).~/.archon/workflows/<name>.yamland returnssource: 'global'. Verified the file is on disk at the expected path.~/.archon/workflows/and returnsdeleted: true. Verified the file is actually gone.cwdand home have the same name.cwdset unchanged — project paths still go throughgetWorkflowFolderSearchPaths().Side Effects / Blast Radius
packages/server/src/routes/api.ts(3 endpoints).workflow.fetch_failed,workflow.fetch_home_failed,workflow.save_failed,workflow.delete_failedkeep the existing shape; a regression in the home tier surfaces asworkflow.fetch_home_failed.Rollback Plan
dev. No state migration. Existing files at~/.archon/workflows/written by the new PUT path will be re-discovered via the LIST endpoint regardless (that part already worked).api.workflows.test.tstests.Risks and Mitigations
readFileadds one extra filesystem stat on the GET path for every project-scoped miss. Negligible; it's a single ENOENT-or-hit on a fixed path. Mitigation: none required.WorkflowSourcecould re-surface the literal string in the JSON response. Mitigation: tests assertsource === 'global'directly so any drift fails CI.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests