-
-
Notifications
You must be signed in to change notification settings - Fork 388
refactor(workflow): move orchestration to the CLI #167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2e3bbe3
5aceb53
debad17
3020b71
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,9 +3,9 @@ import { createRequire } from "node:module"; | |
| import { stdin as input, stdout as output } from "node:process"; | ||
| import { spawn } from "node:child_process"; | ||
| import { mkdtempSync, writeFileSync } from "node:fs"; | ||
| import { readFile } from "node:fs/promises"; | ||
| import { readFile, unlink } from "node:fs/promises"; | ||
| import { tmpdir } from "node:os"; | ||
| import { join, resolve } from "node:path"; | ||
| import { basename, join, resolve } from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import * as prompts from "@clack/prompts"; | ||
| import { getShellConfig } from "@earendil-works/pi-coding-agent"; | ||
|
|
@@ -75,9 +75,10 @@ async function main(argv: string[]): Promise<void> { | |
| runConfigCommand(args); | ||
| return; | ||
| case "agents": | ||
| if (!loadConfig().subagents) { | ||
| const config = loadConfig(); | ||
| if (!config.subagents && !config.workflows) { | ||
| throw new Error( | ||
| "Subagents are disabled. Set DEVSPACE_SUBAGENTS=1 to enable the experimental feature.", | ||
| "Subagents and Dynamic Workflows are disabled. Set DEVSPACE_SUBAGENTS=1 or DEVSPACE_WORKFLOWS=1 to enable agent tooling.", | ||
| ); | ||
| } | ||
| await runAgentsCommand(args); | ||
|
|
@@ -495,27 +496,31 @@ async function runAgentsShow(args: string[]): Promise<void> { | |
|
|
||
| const config = loadConfig(); | ||
| const store = createLocalAgentStore(config); | ||
| let record = store.get(id); | ||
| if (!record) throw new Error(`Unknown subagent id: ${id}`); | ||
| assertAgentInScope(record, resolveCurrentWorkspaceScope(config)); | ||
|
|
||
| const deadline = Date.now() + 15_000; | ||
| while ((record.status === "starting" || record.status === "running") && Date.now() < deadline) { | ||
| await sleep(500); | ||
| record = store.get(id) ?? record; | ||
| } | ||
| try { | ||
| let record = store.get(id); | ||
| if (!record) throw new Error(`Unknown subagent id: ${id}`); | ||
| assertAgentInScope(record, resolveCurrentWorkspaceScope(config)); | ||
|
|
||
| const deadline = Date.now() + 15_000; | ||
| while ((record.status === "starting" || record.status === "running") && Date.now() < deadline) { | ||
| await sleep(500); | ||
| record = store.get(id) ?? record; | ||
| } | ||
|
|
||
| console.log(formatAgentLine(record)); | ||
| if (record.latestResponse) { | ||
| console.log(record.latestResponse); | ||
| return; | ||
| } | ||
| if (record.error) { | ||
| console.log(record.error); | ||
| return; | ||
| } | ||
| if (record.status === "starting" || record.status === "running") { | ||
| console.log(`No final response yet. Call \`devspace agents show ${record.id}\` again later.`); | ||
| console.log(formatAgentLine(record)); | ||
| if (record.latestResponse) { | ||
| console.log(record.latestResponse); | ||
| return; | ||
| } | ||
| if (record.error) { | ||
| console.log(record.error); | ||
| return; | ||
| } | ||
| if (record.status === "starting" || record.status === "running") { | ||
| console.log(`No final response yet. Call \`devspace agents show ${record.id}\` again later.`); | ||
| } | ||
| } finally { | ||
| store.close(); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -527,11 +532,11 @@ async function runAgentsWorker(args: string[]): Promise<void> { | |
|
|
||
| const config = loadConfig(); | ||
| const store = createLocalAgentStore(config); | ||
| const record = store.get(id); | ||
| if (!record) throw new Error(`Unknown subagent id: ${id}`); | ||
|
|
||
| store.update(record.id, { status: "running", error: undefined }); | ||
| try { | ||
| const record = store.get(id); | ||
| if (!record) throw new Error(`Unknown subagent id: ${id}`); | ||
|
|
||
| store.update(record.id, { status: "running", error: undefined }); | ||
| const profiles = await loadLocalAgentProfiles(config, record.workspaceRoot); | ||
|
Comment on lines
+536
to
540
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Enforce workspace scope before starting the worker.
As per coding guidelines: treat every operation as workspace-scoped and use 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| const prompt = await readFile(promptFile, "utf8"); | ||
| const target = resolveLocalAgentExecution({ | ||
|
|
@@ -557,10 +562,18 @@ async function runAgentsWorker(args: string[]): Promise<void> { | |
| error: undefined, | ||
| }); | ||
| } catch (error) { | ||
| store.update(record.id, { | ||
| status: "error", | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| const record = store.get(id); | ||
| if (record) { | ||
| store.update(record.id, { | ||
| status: "error", | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| } | ||
| } finally { | ||
|
Comment on lines
+570
to
+572
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The worker unlinks Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
| if (isGeneratedPromptFile(promptFile)) { | ||
| await unlink(promptFile).catch(() => undefined); | ||
| } | ||
| store.close(); | ||
|
Comment on lines
+573
to
+576
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Remove the generated prompt directory after cleanup.
🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
|
|
||
|
|
@@ -588,6 +601,12 @@ function writeAgentPromptFile(prompt: string): string { | |
| return filePath; | ||
| } | ||
|
|
||
| function isGeneratedPromptFile(filePath: string): boolean { | ||
| const resolvedPath = resolve(filePath); | ||
| const prefix = `${resolve(tmpdir())}${process.platform === "win32" ? "\\" : "/"}devspace-agent-prompt-`; | ||
| return resolvedPath.startsWith(prefix) && basename(resolvedPath) === "prompt.txt"; | ||
| } | ||
|
|
||
| function resolveCurrentWorkspaceRoot(config: ReturnType<typeof loadConfig>): string { | ||
| return resolveCliWorkspaceScope(config.allowedRoots).workspaceRoot; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,9 @@ export function parseLocalAgentRunArgs(args: string[]): ParsedLocalAgentRunArgs | |
| effort = value; | ||
| continue; | ||
| } | ||
| if (part?.startsWith("--")) { | ||
| throw new Error(`Unknown option: ${part}\n${USAGE}`); | ||
| } | ||
|
Comment on lines
+67
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Reject option-like tokens before consuming option values. The new check runs after known options parse their values. Therefore, If model and effort values cannot start with 🤖 Prompt for AI Agents |
||
| promptParts.push(part ?? ""); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,7 +47,6 @@ import { formatPathForPrompt } from "./skills.js"; | |
| import { createWorkspaceStore } from "./workspace-store.js"; | ||
| import { formatAgentsPath, WorkspaceRegistry } from "./workspaces.js"; | ||
| import { buildLocalAgentCatalog } from "./local-agent-catalog.js"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| import { registerWorkflowTools } from "./workflow-tools.js"; | ||
| import { startWorkflowReaper } from "./workflow-lifecycle.js"; | ||
| import { createWorkflowStore } from "./workflow-store.js"; | ||
| import { loadActiveWorkflowSummaries } from "./workflow-ui.js"; | ||
|
|
@@ -1628,10 +1627,6 @@ function createMcpServer( | |
| registerCodexProcessTools(server, config, workspaces, processSessions); | ||
| } | ||
|
|
||
| if (config.workflows) { | ||
| registerWorkflowTools(server, config, workspaces); | ||
| } | ||
|
|
||
| return server; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { resolve } from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import { resolveCliWorkspaceScope } from "./cli-workspace.js"; | ||
| import type { ServerConfig } from "./config.js"; | ||
| import { parseWorkflowArgFlagsResult } from "./workflow-files.js"; | ||
| import { | ||
|
|
@@ -27,6 +28,7 @@ import { | |
| spawnWorkflowWorker, | ||
| spawnWorkflowWorkerFromCli, | ||
| } from "./workflow-worker.js"; | ||
| import { isPathInsideRoot } from "./roots.js"; | ||
|
|
||
| export { runWorkflowWorker, spawnWorkflowWorker, spawnWorkflowWorkerFromCli }; | ||
|
|
||
|
|
@@ -104,6 +106,8 @@ export function printWorkflowHelp(): void { | |
|
|
||
| async function runWorkflowRun(args: string[], config: ServerConfig): Promise<void> { | ||
| const { flags } = splitFlags(args); | ||
| assertKnownFlags(flags, ["follow", "script-path", "file", "name", "resume", "arg"], | ||
| "Usage: devspace workflow run [--file|--script-path <path> | --name <name>] [--resume <runId>] [--arg key=value]... [--follow]"); | ||
| const follow = flags.has("follow"); | ||
| const file = flagValue(flags, "script-path") ?? flagValue(flags, "file"); | ||
| const name = flagValue(flags, "name"); | ||
|
|
@@ -126,10 +130,15 @@ async function runWorkflowRun(args: string[], config: ServerConfig): Promise<voi | |
| }); | ||
| } | ||
|
|
||
| const source = buildCliLaunchSource({ file, name, resumeFrom }); | ||
| const scope = resolveCliWorkspaceScope(config.allowedRoots); | ||
| const source = buildCliLaunchSource({ | ||
| file: file ? resolveWorkflowFilePath(file, scope.workspaceRoot) : undefined, | ||
| name, | ||
| resumeFrom, | ||
| }); | ||
| const store = createWorkflowStore(config); | ||
| try { | ||
| const workspaceRoot = resolve(process.env.DEVSPACE_WORKSPACE_ROOT || process.cwd()); | ||
| const workspaceRoot = scope.workspaceRoot; | ||
| let argsValue = Object.keys(workflowArgs).length ? workflowArgs : undefined; | ||
|
|
||
| // Resume without explicit --arg reuses prior args inside launch; if CLI | ||
|
|
@@ -144,7 +153,7 @@ async function runWorkflowRun(args: string[], config: ServerConfig): Promise<voi | |
| store, | ||
| config, | ||
| workspaceRoot, | ||
| workspaceId: process.env.DEVSPACE_WORKSPACE_ID, | ||
| workspaceId: scope.workspaceId, | ||
| source, | ||
| args: argsValue, | ||
| cliEntry: fileURLToPath(import.meta.url.replace(/workflow-cli\.(ts|js)$/, "cli.$1")), | ||
|
|
@@ -183,8 +192,10 @@ function buildCliLaunchSource(input: { | |
| } | ||
|
|
||
| async function runWorkflowStatus(args: string[], config: ServerConfig): Promise<void> { | ||
| const follow = args.includes("--follow"); | ||
| const runId = args.find((a) => !a.startsWith("-")); | ||
| const { flags, positionals } = splitFlags(args); | ||
| assertKnownFlags(flags, ["follow"], "Usage: devspace workflow status <runId> [--follow]"); | ||
| const follow = flags.has("follow"); | ||
| const runId = positionals[0]; | ||
| if (!runId) { | ||
| throw new InvalidWorkflowInputError({ | ||
| code: "invalid_argument", | ||
|
|
@@ -199,6 +210,7 @@ async function runWorkflowStatus(args: string[], config: ServerConfig): Promise< | |
| if (runResult.isErr()) throw runResult.error; | ||
| const run = runResult.value; | ||
| if (!run) throw new WorkflowNotFoundError(runId); | ||
| assertWorkflowInScope(run, resolveCliWorkspaceScope(config.allowedRoots)); | ||
| console.log(formatRunLine(run)); | ||
| console.log(formatCallSummary(store.listAgentCalls(runId))); | ||
| if (follow) { | ||
|
|
@@ -213,7 +225,9 @@ async function runWorkflowStatus(args: string[], config: ServerConfig): Promise< | |
| } | ||
|
|
||
| async function runWorkflowCancel(args: string[], config: ServerConfig): Promise<void> { | ||
| const runId = args[0]; | ||
| const { flags, positionals } = splitFlags(args); | ||
| assertKnownFlags(flags, [], "Usage: devspace workflow cancel <runId>"); | ||
| const runId = positionals[0]; | ||
| if (!runId) { | ||
| throw new InvalidWorkflowInputError({ | ||
| code: "invalid_argument", | ||
|
|
@@ -223,6 +237,9 @@ async function runWorkflowCancel(args: string[], config: ServerConfig): Promise< | |
| const store = createWorkflowStore(config); | ||
| try { | ||
| reapStaleWorkflows(store); | ||
| const run = store.getRun(runId); | ||
| if (!run) throw new WorkflowNotFoundError(runId); | ||
| assertWorkflowInScope(run, resolveCliWorkspaceScope(config.allowedRoots)); | ||
| console.log(formatRunLine(await cancelWorkflowRun(store, runId))); | ||
| } finally { | ||
| store.close(); | ||
|
|
@@ -233,7 +250,8 @@ async function runWorkflowList(config: ServerConfig): Promise<void> { | |
| const store = createWorkflowStore(config); | ||
| try { | ||
| reapStaleWorkflows(store); | ||
| const runs = store.listRuns(50); | ||
| const scope = resolveCliWorkspaceScope(config.allowedRoots); | ||
| const runs = store.listRunsForWorkspace(scope.workspaceRoot, { limit: 50 }); | ||
| if (runs.length === 0) { | ||
| console.log("No workflow runs."); | ||
| return; | ||
|
|
@@ -245,7 +263,9 @@ async function runWorkflowList(config: ServerConfig): Promise<void> { | |
| } | ||
|
|
||
| async function runWorkflowCalls(args: string[], config: ServerConfig): Promise<void> { | ||
| const runId = args[0]; | ||
| const { flags, positionals } = splitFlags(args); | ||
| assertKnownFlags(flags, [], "Usage: devspace workflow calls <runId>"); | ||
| const runId = positionals[0]; | ||
| if (!runId) { | ||
| throw new InvalidWorkflowInputError({ | ||
| code: "invalid_argument", | ||
|
|
@@ -254,7 +274,9 @@ async function runWorkflowCalls(args: string[], config: ServerConfig): Promise<v | |
| } | ||
| const store = createWorkflowStore(config); | ||
| try { | ||
| if (!store.getRun(runId)) throw new WorkflowNotFoundError(runId); | ||
| const run = store.getRun(runId); | ||
| if (!run) throw new WorkflowNotFoundError(runId); | ||
| assertWorkflowInScope(run, resolveCliWorkspaceScope(config.allowedRoots)); | ||
| const calls = store.listAgentCalls(runId); | ||
| if (calls.length === 0) { | ||
| console.log("No workflow agent calls."); | ||
|
|
@@ -267,8 +289,10 @@ async function runWorkflowCalls(args: string[], config: ServerConfig): Promise<v | |
| } | ||
|
|
||
| async function runWorkflowCall(args: string[], config: ServerConfig): Promise<void> { | ||
| const runId = args[0]; | ||
| const callIndex = Number(args[1]); | ||
| const { flags, positionals } = splitFlags(args); | ||
| assertKnownFlags(flags, [], "Usage: devspace workflow call <runId> <callIndex>"); | ||
| const runId = positionals[0]; | ||
| const callIndex = Number(positionals[1]); | ||
| if (!runId || !Number.isInteger(callIndex) || callIndex < 0) { | ||
| throw new InvalidWorkflowInputError({ | ||
| code: "invalid_argument", | ||
|
|
@@ -277,7 +301,9 @@ async function runWorkflowCall(args: string[], config: ServerConfig): Promise<vo | |
| } | ||
| const store = createWorkflowStore(config); | ||
| try { | ||
| if (!store.getRun(runId)) throw new WorkflowNotFoundError(runId); | ||
| const run = store.getRun(runId); | ||
| if (!run) throw new WorkflowNotFoundError(runId); | ||
| assertWorkflowInScope(run, resolveCliWorkspaceScope(config.allowedRoots)); | ||
| const call = store.getAgentCall(runId, callIndex); | ||
| if (!call) { | ||
| throw new InvalidWorkflowInputError({ | ||
|
|
@@ -294,6 +320,7 @@ async function runWorkflowCall(args: string[], config: ServerConfig): Promise<vo | |
| async function followRun(store: WorkflowStore, runId: string): Promise<void> { | ||
| let sinceSeq = 0; | ||
| for (;;) { | ||
| reapStaleWorkflows(store); | ||
| const page = store.drainEvents(runId, sinceSeq, WORKFLOW_LIMITS.eventDrainDefault); | ||
| for (const event of page.events) printEvent(event); | ||
| sinceSeq = page.nextSeq; | ||
|
|
@@ -440,6 +467,50 @@ function splitFlags(args: string[]): { | |
| return { flags, positionals }; | ||
| } | ||
|
|
||
| function assertKnownFlags( | ||
| flags: Map<string, string | true>, | ||
| allowed: string[], | ||
| usage: string, | ||
| ): void { | ||
| const allowedSet = new Set(allowed); | ||
| const unknown = [...flags.keys()].filter((flag) => !allowedSet.has(flag)); | ||
| if (unknown.length > 0) { | ||
| throw new InvalidWorkflowInputError({ | ||
| code: "invalid_argument", | ||
| message: `${usage}\nUnknown option: --${unknown[0]}`, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| function resolveWorkflowFilePath(path: string, workspaceRoot: string): string { | ||
| const resolvedPath = resolve(workspaceRoot, path); | ||
| if (!isPathInsideRoot(resolvedPath, workspaceRoot)) { | ||
| throw new InvalidWorkflowInputError({ | ||
| code: "invalid_path", | ||
| message: `Workflow file must be inside the workspace: ${workspaceRoot}`, | ||
| }); | ||
| } | ||
| return resolvedPath; | ||
|
Comment on lines
+485
to
+493
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Resolve symlinks before accepting a workflow file.
Based on learnings: enforce lexical and canonical containment for workflow script paths. 🤖 Prompt for AI AgentsSource: Learnings |
||
| } | ||
|
|
||
| function assertWorkflowInScope( | ||
| run: Pick<WorkflowRunRecord, "workspaceRoot" | "workspaceId">, | ||
| scope: { workspaceRoot: string; workspaceId?: string }, | ||
| ): void { | ||
| if (resolve(run.workspaceRoot) !== resolve(scope.workspaceRoot)) { | ||
| throw new InvalidWorkflowInputError({ | ||
| code: "invalid_argument", | ||
| message: `Workflow run belongs to a different workspace: ${scope.workspaceRoot}`, | ||
| }); | ||
| } | ||
| if (scope.workspaceId && run.workspaceId && run.workspaceId !== scope.workspaceId) { | ||
| throw new InvalidWorkflowInputError({ | ||
| code: "invalid_argument", | ||
| message: `Workflow run belongs to a different workspaceId: ${scope.workspaceId}`, | ||
| }); | ||
| } | ||
| } | ||
|
Comment on lines
+496
to
+512
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift Apply When the active scope has a
As per coding guidelines: treat every operation as workspace-scoped and use 📍 Affects 1 file
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| function flagValue(flags: Map<string, string | true>, key: string): string | undefined { | ||
| const value = flags.get(key); | ||
| return typeof value === "string" ? value : undefined; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Scope the
agentscase declaration with braces.Biome reports
noSwitchDeclarationsforconfig. Wrap this case body in braces so its declaration cannot be visible to other switch clauses.Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 Biome (2.5.6)
[error] 78-78: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
Source: Linters/SAST tools