Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 64 additions & 17 deletions packages/server/src/routes/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
getDefaultWorkflowsPath,
getArchonWorkspacesPath,
getHomeCommandsPath,
getHomeWorkflowsPath,
getRunArtifactsPath,
getArchonHome,
isDocker,
Expand Down Expand Up @@ -2260,7 +2261,34 @@ export function registerApiRoutes(
}
}

// 2. Fall back to bundled defaults (binary: embedded map; dev: also check filesystem)
// 2. Home-scoped (`~/.archon/workflows/`) — applies to every project.
// Mirrors the discovery layer's priority documented in CLAUDE.md
// (`bundled < global < project`); the list endpoint via
// `discoverWorkflowsWithConfig` reads this path unconditionally, so
// skipping it here would make the workflow visible in the dashboard
// but unfetchable on the detail page.
{
const homeFilePath = join(getHomeWorkflowsPath(), filename);
try {
const content = await readFile(homeFilePath, 'utf-8');
const result = parseWorkflow(content, filename);
if (result.error) {
return apiError(c, 500, `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-scoped workflow');
}
}
}

// 3. Fall back to bundled defaults (binary: embedded map; dev: also check filesystem)
if (Object.hasOwn(BUNDLED_WORKFLOWS, name)) {
const bundledContent = BUNDLED_WORKFLOWS[name];
const result = parseWorkflow(bundledContent, filename);
Expand All @@ -2270,6 +2298,7 @@ export function registerApiRoutes(
return c.json({ workflow: result.workflow, filename, source: 'bundled' as WorkflowSource });
}

// 4. Source-build fallback: filesystem-backed defaults
if (!isBinaryBuild()) {
const defaultFilePath = join(getDefaultWorkflowsPath(), filename);
try {
Expand Down Expand Up @@ -2306,18 +2335,21 @@ export function registerApiRoutes(
return apiError(c, 400, 'Invalid workflow name');
}

// Resolve where to write. If `cwd` is provided (or a codebase exists)
// we write into that project's `.archon/workflows/`. With no project at
// all, fall back to the home-scoped layer (`~/.archon/workflows/`) —
// NOT to `<archonHome>/.archon/workflows/`, which is the legacy path
// the migration warning in `workflow-discovery.ts` explicitly tells
// users to abandon.
const cwd = c.req.query('cwd');
let workingDir = cwd;
let projectDir: string | undefined = cwd;
if (cwd) {
if (!(await validateCwd(cwd))) {
return apiError(c, 400, 'Invalid cwd: must match a registered codebase path');
}
} else {
const codebases = await codebaseDb.listCodebases();
if (codebases.length > 0) workingDir = codebases[0].default_cwd;
}
if (!workingDir) {
workingDir = getArchonHome();
if (codebases.length > 0) projectDir = codebases[0].default_cwd;
}

const { definition } = getValidatedBody(c, saveWorkflowBodySchema);
Expand All @@ -2337,20 +2369,29 @@ export function registerApiRoutes(
return apiError(c, 400, 'Workflow definition is invalid', parsed.error.error);
}

try {
let dirPath: string;
let savedSource: WorkflowSource;
if (projectDir) {
const [workflowFolder] = getWorkflowFolderSearchPaths();
const dirPath = join(workingDir, workflowFolder);
dirPath = join(projectDir, workflowFolder);
savedSource = 'project';
} else {
dirPath = getHomeWorkflowsPath();
savedSource = 'global';
}

try {
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: savedSource,
});
} catch (error) {
const err = error instanceof Error ? error : new Error(String(error));
getLog().error({ err, name }, 'workflow.save_failed');
getLog().error({ err, name, dirPath }, 'workflow.save_failed');
return apiError(c, 500, 'Failed to save workflow');
}
});
Expand All @@ -2367,22 +2408,28 @@ export function registerApiRoutes(
return apiError(c, 400, `Cannot delete bundled default workflow: ${name}`);
}

// Mirror PUT's resolution: project-scoped if a cwd or registered codebase
// exists, home-scoped (`~/.archon/workflows/`) otherwise. Same rationale
// for steering away from the legacy `<archonHome>/.archon/workflows/`
// path applies.
const cwd = c.req.query('cwd');
let workingDir = cwd;
let projectDir: string | undefined = cwd;
if (cwd) {
if (!(await validateCwd(cwd))) {
return apiError(c, 400, 'Invalid cwd: must match a registered codebase path');
}
} else {
const codebases = await codebaseDb.listCodebases();
if (codebases.length > 0) workingDir = codebases[0].default_cwd;
}
if (!workingDir) {
workingDir = getArchonHome();
if (codebases.length > 0) projectDir = codebases[0].default_cwd;
}

const [workflowFolder] = getWorkflowFolderSearchPaths();
const filePath = join(workingDir, workflowFolder, `${name}.yaml`);
let filePath: string;
if (projectDir) {
const [workflowFolder] = getWorkflowFolderSearchPaths();
filePath = join(projectDir, workflowFolder, `${name}.yaml`);
} else {
filePath = join(getHomeWorkflowsPath(), `${name}.yaml`);
}

try {
await unlink(filePath);
Expand Down
126 changes: 124 additions & 2 deletions packages/server/src/routes/api.workflows.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,86 @@ describe('GET /api/workflows/:name', () => {
}
});

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 });
}
});
Comment on lines +256 to +293
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 | ⚡ Quick win

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.


test('project-scoped workflow shadows a same-named home-scoped one (priority: project > global > bundled)', async () => {
const testArchonHome = join(tmpdir(), `archon-home-shadow-${Date.now()}`);
const homeWorkflowDir = join(testArchonHome, 'workflows');
await mkdir(homeWorkflowDir, { recursive: true });
await writeFile(
join(homeWorkflowDir, 'shared.yaml'),
'name: shared\ndescription: home version\nnodes: []\n'
);

const projectDir = join(tmpdir(), `archon-proj-shadow-${Date.now()}`);
const projectWorkflowDir = join(projectDir, '.archon', 'workflows');
await mkdir(projectWorkflowDir, { recursive: true });
await writeFile(
join(projectWorkflowDir, 'shared.yaml'),
'name: shared\ndescription: project version\nnodes: []\n'
);

process.env.ARCHON_HOME = testArchonHome;

try {
const app = createTestApp();
registerApiRoutes(app, {} as WebAdapter, {} as ConversationLockManager);

mockListCodebases.mockImplementationOnce(async () => [{ default_cwd: projectDir }]);
mockParseWorkflow.mockReturnValueOnce({
workflow: makeTestWorkflow({ name: 'shared', description: 'project version' }),
error: null,
});

const response = await app.request(`/api/workflows/shared?cwd=${projectDir}`);
expect(response.status).toBe(200);
const body = (await response.json()) as { source: string; workflow: { description: string } };
expect(body.source).toBe('project');
expect(body.workflow.description).toBe('project version');
} finally {
delete process.env.ARCHON_HOME;
await rm(testArchonHome, { recursive: true, force: true });
await rm(projectDir, { recursive: true, force: true });
}
});

test('returns WorkflowDefinition shape with expected top-level fields', async () => {
const app = createTestApp();
registerApiRoutes(app, {} as WebAdapter, {} as ConversationLockManager);
Expand Down Expand Up @@ -314,7 +394,10 @@ describe('PUT /api/workflows/:name', () => {
expect(body.error).toContain('definition');
});

test('falls back to getArchonHome() when no cwd and no codebases registered', async () => {
test('falls back to home-scoped (~/.archon/workflows/) when no cwd and no codebases registered', async () => {
// Regression: previously this fell through to
// <archonHome>/.archon/workflows/, which is the legacy pre-0.x path the
// migration warning explicitly tells users to abandon. Closes #1524.
const testArchonHome = join(tmpdir(), `archon-home-test-${Date.now()}`);
process.env.ARCHON_HOME = testArchonHome;

Expand All @@ -341,7 +424,12 @@ describe('PUT /api/workflows/:name', () => {
});
expect(response.status).toBe(200);
const body = (await response.json()) as { workflow: object; source: string };
expect(body.source).toBe('project');
// The save must land in ~/.archon/workflows/ (source 'global'), NOT
// ~/.archon/.archon/workflows/ (source 'project' on a legacy path).
expect(body.source).toBe('global');
const savedPath = join(testArchonHome, 'workflows', 'my-workflow.yaml');
const stat = await import('fs/promises').then(m => m.stat(savedPath));
expect(stat.isFile()).toBe(true);
} finally {
delete process.env.ARCHON_HOME;
await rm(testArchonHome, { recursive: true, force: true });
Expand Down Expand Up @@ -446,6 +534,40 @@ describe('DELETE /api/workflows/:name', () => {
expect(body.error).toContain('nonexistent-no-cwd-test');
});

test('removes home-scoped workflow when no cwd / no codebases registered', async () => {
// Regression for #1524: DELETE used to fall through to
// <archonHome>/.archon/workflows/, the legacy path, leaving real
// home-scoped files untouched. Now it deletes from ~/.archon/workflows/.
const testArchonHome = join(tmpdir(), `archon-home-del-${Date.now()}`);
const homeWorkflowDir = join(testArchonHome, 'workflows');
await mkdir(homeWorkflowDir, { recursive: true });
await writeFile(
join(homeWorkflowDir, 'home-only.yaml'),
'name: home-only\ndescription: x\nnodes: []\n'
);
process.env.ARCHON_HOME = testArchonHome;

try {
const app = createTestApp();
registerApiRoutes(app, {} as WebAdapter, {} as ConversationLockManager);

mockListCodebases.mockImplementationOnce(async () => []);

const response = await app.request('/api/workflows/home-only', { method: 'DELETE' });
expect(response.status).toBe(200);
const body = (await response.json()) as { deleted: boolean; name: string };
expect(body.deleted).toBe(true);
expect(body.name).toBe('home-only');

// Verify the file is actually gone from the home-scoped location.
const fs = await import('fs/promises');
await expect(fs.stat(join(homeWorkflowDir, 'home-only.yaml'))).rejects.toThrow();
} finally {
delete process.env.ARCHON_HOME;
await rm(testArchonHome, { recursive: true, force: true });
}
});

test('removes existing workflow file and returns deleted:true', async () => {
const testDir = join(tmpdir(), `wf-del-test-${Date.now()}`);
const workflowDir = join(testDir, '.archon', 'workflows');
Expand Down