-
Notifications
You must be signed in to change notification settings - Fork 0
feat(sdk): make f.agent real, fix cross-package flow-handle identity, TS quickstart #243
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
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 |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import { | ||
| COMPLETION_REASONS, | ||
| RUN_COMPLETION_REASONS, | ||
| type AgentOptions, | ||
| type AgentResult, | ||
| type CloudHelper, | ||
| type CompletionReason as SurfaceCompletionReason, | ||
|
|
@@ -9,8 +10,13 @@ import { | |
| type Step, | ||
| } from '@relayflows/surface'; | ||
| import type { FlowHandle } from '@relayflows/surface/runtime'; | ||
| import { join } from 'node:path'; | ||
| import { compileSpec, toKernelSpec } from './compile.js'; | ||
| import { getAuthoredFlowDefinition } from './authored-flow.js'; | ||
| import type { GetFlowDefinition } from './authored-flow-loader.js'; | ||
| import { checkAuthoredFlow } from './cli/check.js'; | ||
| import { classifyOutcome, type RunLifecycleOptions } from './cli/run.js'; | ||
| import type { PreflightDiagnostic } from './preflight.js'; | ||
| import { | ||
| AuthoredFlowExecutionError, | ||
| type AuthoredFlowExecutionErrorCode, | ||
|
|
@@ -27,7 +33,7 @@ import type { | |
| RunCompletionReason as ProtocolRunCompletionReason, | ||
| RunOutcome, | ||
| } from './protocol.js'; | ||
| import { SPEC_SCHEMA_VERSION } from './spec.js'; | ||
| import { SPEC_SCHEMA_VERSION, type FlowSpec } from './spec.js'; | ||
|
|
||
| type Assert<T extends true> = T; | ||
| type Equal<A, B> = [A] extends [B] | ||
|
|
@@ -49,6 +55,15 @@ type EveryRunCompletionReasonIsAcceptedByDone = Assert< | |
|
|
||
| export { AuthoredFlowExecutionError, type AuthoredFlowExecutionErrorCode }; | ||
|
|
||
| /** | ||
| * Matches the `"path/glob: readonly"` / `"path/glob: readwrite"` shorthand | ||
| * shown in docs/SURFACE.md and the examples — the only shape a workspace | ||
| * string could plausibly declare a permission in. No parser anywhere in this | ||
| * package turns that annotation into a real restriction, so `lowerAgent` | ||
| * refuses rather than silently accepting and ignoring it. | ||
| */ | ||
| const WORKSPACE_PERMISSION_ANNOTATION = /:\s*(readonly|readwrite)\s*$/i; | ||
|
|
||
| export interface AuthoredFlowJournalStep { | ||
| readonly id: string; | ||
| readonly runId: string; | ||
|
|
@@ -79,12 +94,46 @@ type JournalStepUsesStepCompletionReason = Assert< | |
| * `JournalClient`; values are read back from `step.completed` journal entries. | ||
| * Unsupported headers, verbs, gates, or completion lowering fail closed. | ||
| */ | ||
| export interface ExecuteAuthoredFlowOptions { | ||
| /** | ||
| * Defaults to this package's own static import — correct for every | ||
| * existing (internal, single-instance) caller. A flow loaded from an | ||
| * external path via `loadAuthoredFlow` must pass ITS resolved | ||
| * `getDefinition` instead; see authored-flow-loader.ts's comment on why. | ||
| */ | ||
| readonly getDefinition?: GetFlowDefinition; | ||
| /** | ||
| * The flow file's own path — passed straight to `checkAuthoredFlow` | ||
| * (cli/check.ts) so `f.agent` resolves a CLI the same way a declarative | ||
| * `type: agent` step does: nearest `flows.json`, real auth/model probing, | ||
| * canonicalized path. `checkAuthoredFlow` always does `dirname()` on this, | ||
| * matching `checkFlow`'s real `flows check <file>` contract — pass a FILE | ||
| * path, not a directory, or the search starts one level too high. Defaults | ||
| * to a synthetic `flow.ts` under `process.cwd()` for exactly this reason: | ||
| * `process.cwd()` itself is a directory, and `dirname(process.cwd())` | ||
| * would search cwd's PARENT. | ||
| */ | ||
| readonly flowPath?: string; | ||
| /** Passed straight through to classifyOutcome (cli/run.ts) for f.agent's wait. */ | ||
| readonly signal?: RunLifecycleOptions['signal']; | ||
| readonly onWait?: RunLifecycleOptions['onWait']; | ||
| } | ||
|
|
||
| export async function executeAuthoredFlow<Input = undefined>( | ||
| handle: FlowHandle, | ||
| journal: JournalClient, | ||
| input?: Input, | ||
| options: ExecuteAuthoredFlowOptions = {}, | ||
| ): Promise<AuthoredFlowExecutionResult> { | ||
| const definition = getAuthoredFlowDefinition<Input>(handle); | ||
| const getDefinition = options.getDefinition ?? getAuthoredFlowDefinition; | ||
| const flowPath = options.flowPath ?? join(process.cwd(), 'flow.ts'); | ||
| // Named separately from `options` because `lowerAgent` below has its own, | ||
| // differently-typed `options: AgentOptions` parameter that shadows this one. | ||
| const waitOptions: RunLifecycleOptions = { | ||
| ...(options.signal !== undefined ? { signal: options.signal } : {}), | ||
| ...(options.onWait !== undefined ? { onWait: options.onWait } : {}), | ||
| }; | ||
| const definition = getDefinition<Input>(handle); | ||
| const headerFields = Object.keys(definition.header); | ||
| if (headerFields.length > 0) { | ||
| throw new AuthoredFlowExecutionError( | ||
|
|
@@ -112,6 +161,97 @@ export async function executeAuthoredFlow<Input = undefined>( | |
| return readSuccessfulOutput(journal, outcome, id, journalSteps); | ||
| }; | ||
|
|
||
| // `name` (the first f.agent argument, e.g. "fixer") is not wired to the | ||
| // kernel's `agent` field: that field selects a NAMED declaration from | ||
| // `FlowSpec.agents`, and this executor refuses every non-empty header | ||
| // (see the top of this function) — an authored flow has no way to declare | ||
| // one today. `name` is kept only for step-id readability; CLI selection | ||
| // goes through the project's flows.json default below, same as it does | ||
| // for a bare `type: agent` YAML step with no explicit `cli`. | ||
| const lowerAgent = async ( | ||
| id: string, | ||
| options: AgentOptions, | ||
| ): Promise<AgentResult> => { | ||
| if (options.workspace !== undefined && WORKSPACE_PERMISSION_ANNOTATION.test(options.workspace)) { | ||
| throw new AuthoredFlowExecutionError( | ||
| 'unsupported_workspace_permission', | ||
| `flow "${definition.name}" step "${id}": workspace "${options.workspace}" declares a ` | ||
| + 'permission annotation ("...: readonly" / "...: readwrite"), but nothing enforces it — ' | ||
| + 'no parser anywhere in this package turns that annotation into a real restriction ' | ||
| + '(kernel/DAEMON-LIFECYCLE.md\'s permission model is untouched by f.agent). ' | ||
| + 'Silently accepting and ignoring it would let a flow believe a restriction is in effect ' | ||
| + "when it is not. Declare a bare surface name (no trailing \": readonly\"/\": readwrite\") " | ||
| + 'if you do not need enforcement, or use the declarative spec\'s `permissions` field, which is real.', | ||
| ); | ||
| } | ||
| const authoring: FlowSpec = { | ||
| version: SPEC_SCHEMA_VERSION, | ||
| name: `${definition.name}/${id}`, | ||
| steps: [{ | ||
| id, | ||
| type: 'agent', | ||
| instruction: options.task, | ||
| ...(options.workspace === undefined ? {} : { | ||
| surfaces: { workspace: [{ surface: options.workspace }] }, | ||
| }), | ||
| }], | ||
| }; | ||
| // The kernel never resolves a `cli` on its own — every declarative | ||
| // `flows run`/`flows check` binds it first via this exact function | ||
| // (cli/check.ts), searching for the nearest flows.json from `flowPath` | ||
| // and real-probing auth/model readiness. An authored agent step gets | ||
| // nothing for free just because it was declared in TS instead of YAML. | ||
| const { report, flow: resolved } = checkAuthoredFlow(authoring, flowPath); | ||
| if (!report.ok || resolved === undefined) { | ||
| const refusal = report.diagnostics.find( | ||
| (diagnostic): diagnostic is PreflightDiagnostic & { severity: 'refusal' } => | ||
| diagnostic.severity === 'refusal', | ||
| ); | ||
| throw new AuthoredFlowExecutionError( | ||
| 'agent_cli_unresolved', | ||
| refusal?.message | ||
| ?? `flow "${definition.name}" step "${id}": no CLI could be resolved for f.agent ` | ||
| + `(searched for flows.json from "${flowPath}")`, | ||
| ); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
| const spec = toKernelSpec(resolved); | ||
| const outcome = await journal.runStart(spec); | ||
| // Reuse the declarative CLI's own wait/classification (cli/run.ts) rather | ||
| // than a hand-rolled poll: `step.completed` and the run's own terminal | ||
| // state are appended as two SEPARATE actions (kernel/relayflowd-core/src/machine.rs | ||
| // completion_actions vs complete_run_actions), so a naive read right | ||
| // after runStart can race a real, valid completion — and a genuinely | ||
| // long-running agent has no reason to be bounded by anything other than | ||
| // its own worker's lease, which classifyOutcome already follows | ||
| // (renewing as the lease renews, per docs/SURFACE.md §5's WAITING | ||
| // [worker_lease] contract), never an unrelated fixed deadline. | ||
| const execution = await classifyOutcome(journal, 'run', outcome, report, '', waitOptions); | ||
| if (execution.exitCode === 3) { | ||
| const parked = execution.report.parkedStep; | ||
| throw new AuthoredFlowExecutionError( | ||
| 'agent_parked', | ||
| execution.report.diagnostics.at(-1)?.message | ||
| ?? `flow "${definition.name}" step "${id}" parked` | ||
| + (parked !== undefined ? ` (${parked.type})` : '') | ||
| + ': no worker is attached to run it.', | ||
| undefined, | ||
| outcome.run_id, | ||
| ); | ||
| } | ||
| if (execution.exitCode !== 0) { | ||
| const reason = execution.report.completionReason; | ||
| throw new AuthoredFlowExecutionError( | ||
| 'step_failed', | ||
| execution.report.diagnostics.at(-1)?.message | ||
| ?? `flow "${definition.name}" step "${id}" did not complete successfully ` | ||
| + `(status: ${execution.report.status ?? 'unknown'})`, | ||
| isSurfaceCompletionReason(reason) ? reason : undefined, | ||
| outcome.run_id, | ||
| ); | ||
| } | ||
| return readSuccessfulAgentOutput(journal, outcome.run_id, id, journalSteps); | ||
| }; | ||
|
|
||
| const context: Ctx = { | ||
| run(command) { | ||
| assertOperationAllowed('run', definition.name, requestedCompletion); | ||
|
|
@@ -134,13 +274,15 @@ export async function executeAuthoredFlow<Input = undefined>( | |
| lifecycle, | ||
| )); | ||
| }, | ||
| agent() { | ||
| agent(name, options) { | ||
| assertOperationAllowed('agent', definition.name, requestedCompletion); | ||
| void name; // step-id readability only — see the comment on lowerAgent. | ||
| const id = `agent-${nextStep++}`; | ||
| return trackStep(authoredSteps, unsupportedStep<AgentResult>( | ||
| return trackStep(authoredSteps, new AuthoredFlowOperation( | ||
| id, | ||
| 'agent', | ||
| () => assertOperationAllowed('agent', definition.name, requestedCompletion), | ||
| () => lowerAgent(id, options), | ||
| lifecycle, | ||
| )); | ||
| }, | ||
|
|
@@ -286,42 +428,90 @@ function unsupportedCloud(assertOpen: () => void): CloudHelper { | |
| }) as CloudHelper; | ||
| } | ||
|
|
||
| async function readSuccessfulOutput( | ||
| /** | ||
| * Shared by every `f.*` verb that lowers to one kernel step run in isolation: | ||
| * find its `step.completed` entry, record it, and refuse anything but a | ||
| * clean success before handing the raw `output` back for verb-specific | ||
| * extraction (a plain string for `f.run`, an `AgentResult` for `f.agent`). | ||
| * | ||
| * Callers are responsible for having already established that the RUN | ||
| * reached a terminal, successful state before calling this — `f.run`'s | ||
| * caller relies on `runStart`'s own immediate response (the kernel drives a | ||
| * deterministic step to completion inline, no race); `f.agent`'s caller | ||
| * relies on `classifyOutcome` (cli/run.ts) having already polled to a true | ||
| * terminal state. Given that, a single read from the start of this run's | ||
| * (small, single-step) journal is enough — no polling here, and no run | ||
| * outcome ever needs re-checking. | ||
| */ | ||
| async function readCompletedStepOutput( | ||
| journal: JournalClient, | ||
| outcome: RunOutcome, | ||
| runId: string, | ||
| stepId: string, | ||
| journalSteps: AuthoredFlowJournalStep[], | ||
| ): Promise<string> { | ||
| const entries = (await journal.journalRead(outcome.run_id, 1)).entries; | ||
| ): Promise<unknown> { | ||
| const entries = (await journal.journalRead(runId, 1)).entries; | ||
|
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. P2: When Prompt for AI agents |
||
| const completed = entries.find((entry) => isStepCompleted(entry, stepId)); | ||
| if (!isStepCompleted(completed, stepId)) { | ||
| throw protocolViolation(outcome.run_id, `journal has no step.completed for "${stepId}"`); | ||
| throw protocolViolation(runId, `journal has no step.completed for "${stepId}"`); | ||
| } | ||
|
|
||
| const reason = completed.payload.completionReason; | ||
| journalSteps.push(Object.freeze({ id: stepId, runId: outcome.run_id, completionReason: reason })); | ||
| journalSteps.push(Object.freeze({ id: stepId, runId, completionReason: reason })); | ||
| if (reason !== 'success') { | ||
| throw new AuthoredFlowExecutionError( | ||
| 'step_failed', | ||
| `journal step "${stepId}" completed with ${reason}`, | ||
| reason, | ||
| outcome.run_id, | ||
| ); | ||
| } | ||
| if (outcome.status !== 'completed' || outcome.completion_reason !== 'success') { | ||
| throw protocolViolation( | ||
| outcome.run_id, | ||
| `successful step entry conflicts with run outcome ${outcome.status}/${String(outcome.completion_reason)}`, | ||
| runId, | ||
| ); | ||
| } | ||
| return completed.payload.output; | ||
| } | ||
|
|
||
| const output = completed.payload.output; | ||
| async function readSuccessfulOutput( | ||
| journal: JournalClient, | ||
| outcome: RunOutcome, | ||
| stepId: string, | ||
| journalSteps: AuthoredFlowJournalStep[], | ||
| ): Promise<string> { | ||
| const output = await readCompletedStepOutput(journal, outcome.run_id, stepId, journalSteps); | ||
| if (!isRecord(output) || typeof output['stdout_tail'] !== 'string') { | ||
| throw protocolViolation(outcome.run_id, `step "${stepId}" has no string stdout_tail`); | ||
| } | ||
| return output['stdout_tail']; | ||
| } | ||
|
|
||
| /** | ||
| * The kernel journals an agent step's raw `CliResult` shape | ||
| * (`exit_code`/`stdout_tail`/`stderr_tail`) — same as a deterministic step — | ||
| * UNLESS the CLI's stdout parsed as JSON, in which case `output` is that | ||
| * parsed object directly (worker.ts: `parseJsonOutput(result.stdout_tail) ?? | ||
| * result`). Neither shape carries a real `artifacts` list today: nothing in | ||
| * the kernel or worker enumerates files an agent wrote, and `f.agent`'s | ||
| * `AgentOptions` has no `output` schema parameter for a CLI to target either | ||
| * (unlike the declarative `AgentStepSpec.output` field). `artifacts` is | ||
| * therefore always empty here — honest about what data exists, not a | ||
| * placeholder for something not yet wired. | ||
| */ | ||
| async function readSuccessfulAgentOutput( | ||
| journal: JournalClient, | ||
| runId: string, | ||
| stepId: string, | ||
| journalSteps: AuthoredFlowJournalStep[], | ||
| ): Promise<AgentResult> { | ||
| const output = await readCompletedStepOutput(journal, runId, stepId, journalSteps); | ||
| if (!isRecord(output)) { | ||
| throw protocolViolation(runId, `step "${stepId}" produced a non-object output`); | ||
| } | ||
| if (typeof output['stdout_tail'] === 'string') { | ||
| return { summary: output['stdout_tail'], artifacts: [] }; | ||
| } | ||
| // The CLI's stdout parsed as JSON, so `output` is that value, not a | ||
| // CliResult. Fall back to a stable, inspectable summary rather than | ||
| // refusing a run whose agent step genuinely succeeded. | ||
| return { summary: JSON.stringify(output), artifacts: [] }; | ||
| } | ||
|
|
||
| interface StepCompletedEntry { | ||
| entry_type: 'step.completed'; | ||
| step_id: string; | ||
|
|
||
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.
When a flow performs
await f.run(...)before its firstf.agent(...)and the project CLI is missing, unsupported, or unauthenticated, this lazy call discovers the checkable problem only after the earlier command has already executed and been journaled. That turns a submit-time refusal into a mid-flow failure with potentially irreversible effects, contrary to the RFC's preflight covenant. Resolve and probe the authored flow's required agent CLI before allowing body operations to start.AGENTS.md reference: AGENTS.md:L3-L5
Useful? React with 👍 / 👎.