feat: run SkillFlows from the CLI, SDK and scheduler - #90
Conversation
SkillFlow definitions lived in core but the executor lived in
@open-gitagent/voice, so the CLI, SDK and scheduler could create and
list flows without ever being able to run one — while core's system
prompt told every agent that "@flow_name" worked.
Move the step loop into core as flow-runner.ts with its side effects
injected, so each front-end supplies only what is genuinely specific
to it:
executeFlow(flow, input, { runStep, onProgress, requestApproval })
- CLI: "@flow-name [input]" trigger and a /flows command. The trigger
is anchored to the start of the line so an address like
bob@daily-report.com cannot fire a flow. Gates prompt on the
existing readline instance.
- Schedules: new optional `flow` field, so a schedule can run a
SkillFlow instead of a single prompt. `prompt` becomes the flow's
input. A flow schedule with no executor fails loudly.
- SDK: runFlow() wraps executeFlow with the obvious defaults (run each
step as an isolated query against the agent dir) and reports
per-step token usage, so callers don't hand-write a runStep.
Approval gates now deny when no approval handler is supplied. A gate
guards a risky step; with nobody to ask, continuing silently removes
the protection it exists to provide. Unattended callers opt in with
`approve: "auto"`.
Per-step usage counts cache reads and writes, which the session
totals omit — a cache-heavy step otherwise reports near-zero input.
Verified end to end: CLI approve/deny, SDK with a custom runStep,
runFlow across all approval policies, and a scheduled flow. 21 new
tests, none requiring an API key.
shreyas-lyzr
left a comment
shreyas-lyzr
left a comment
There was a problem hiding this comment.
Three issues to address before merge — one is a correctness bug that affects real-world inputs, one is a correctness inconsistency between the CLI and SDK paths, and one is a cosmetic/waste issue worth cleaning up. The overall structure is solid: the loop, injection model, and approval gate semantics are well-designed, and the test coverage is thorough.
Security pass: no new dependencies added, no secrets in the diff, no obvious injection or authz gaps. The approval-gate-denies-by-default change is the right call.
Review follow-ups on open-gitagent#89. The @flow trigger required only a word boundary after the name, so "@daily-report.com" fired the flow with input ".com" — a "." counts as a boundary. Require whitespace or end-of-line instead, which rejects it outright. The same line existed in the voice server; fixed there too. resolveFlow matched by name regardless of type, then errored if the result turned out to be a reference workflow. With both notes.md and notes.yaml present, readdir order decided which one won, so a runnable flow could be reported as "not runnable". Prefer a runnable match and fall back to any match, keeping the more useful error when only a markdown workflow exists. Not changed: step 0 passing the user's input both through {input} and the context block. Emptying the context block drops the input entirely for flows whose first step doesn't template {input}, which is most of them.
shreyas-lyzr
left a comment
shreyas-lyzr
left a comment
There was a problem hiding this comment.
The follow-up commit addresses all three issues cleanly.
Regex anchor: (?=\s|$) replaces the word boundary — @daily-report.com no longer matches. The fix is also applied in the voice server.
resolveFlow: prefers a type === "flow" match before falling back to any match. The index.ts REPL path already filtered by type and is consistent.
Context doubling: the author made a deliberate call not to change the initial runningContext = userContext — correctly noted that emptying it would drop the input for flows whose first step doesn't template {input}. That's a reasonable trade-off and within scope for the author to own.
Security pass: no new dependencies, no secrets, no injection or authz gaps.
Looks good to merge.
2 participants
Closes #89
Problem
Core owns the SkillFlow format — discoverWorkflows, loadFlowDefinition, saveFlowDefinition, deleteFlowDefinition, and the workflows block injected into every system prompt. But the executor lived only in @open-gitagent/voice. So the CLI, SDK and scheduler could create, list and delete flows without ever being able to run one — while workflows.ts:156 told every agent, including the CLI's, that @flow_name worked.
Approach
Move the step loop into core as src/flow-runner.ts, with its side effects injected:
executeFlow(flow, input, { runStep, onProgress, requestApproval })
The loop owns ordering, prompt construction, {input} substitution, context threading and gate semantics. Each caller supplies only what's genuinely specific to it — voice passes its WebSocket broadcaster and Telegram/WhatsApp approver, the CLI passes console.log and a readline prompt, the scheduler passes a logger. The visual builder stays in voice; only the loop moved.
What's included
src/flow-runner.ts — the loop, plus a typed event stream (flow_start, step_start, step_done, approval_requested, approval_resolved, flow_done, flow_aborted) that front-ends render however they like.
src/run-flow.ts — runFlow(), a wrapper supplying the obvious default (each step is an isolated query() against the agent dir) so callers don't hand-write a runStep, plus per-step token usage:
const r = await runFlow({ agentDir, flow: "daily-report", input: "..." });
CLI — @flow-name [input] trigger and a /flows command. The trigger is anchored to line start, so an address like bob@daily-report.com can't fire a flow. Gates prompt on the existing readline instance. Both re-discover workflows rather than using the startup snapshot, matching how /skills calls refreshSkills.
Schedules — new optional flow field, so a schedule can run a SkillFlow instead of a single prompt (prompt becomes the flow's input). A flow schedule with no executor fails loudly rather than silently doing nothing.
Behaviour change worth reviewing
Approval gates now deny when no handler is supplied. A gate guards a risky step; with nobody to ask, continuing silently removes the protection it exists to provide. Unattended callers opt in explicitly with approve: "auto".
This changes voice's current behaviour, which auto-approves when no channel is connected — addressed in the companion voice PR.
Testing
21 new tests, none requiring an API key — gate-only flows resolve before any model call. Also verified end to end against a real agent:
CLI: /flows, @flow, gate approved and denied, unknown flow, and the anchored trigger correctly ignoring bob@daily-report.com
SDK: executeFlow with a custom runStep; runFlow across deny / "auto" / a human approver
A scheduled flow firing on cron and stopping at its gate
Per-step token accounting reconciled against billing to the last digit