From d1b49209d31a961734518992a2da40354f9629fc Mon Sep 17 00:00:00 2001 From: agentrelaybot Date: Thu, 17 Sep 2026 14:29:04 -0700 Subject: [PATCH 1/8] feat(sdk): expose the flows CLI as a mountable relay CLI surface Adds `@relayflows/sdk/relay-cli`, the surface `agent-relay` mounts as `agent-relay flows`. It wraps the CLI this package already ships rather than reimplementing any of it: `run` delegates to `runCli`, and `packages/relayflows/bin/flows.js` keeps calling `runCli` on the same path, unchanged. Single source for the command tree. `src/cli-commands.ts` holds one `CLI_VERBS` table that both gates `parseArgs`' verb dispatch and is projected into `surface.commands`, so the declared tree and the dispatched tree cannot describe different things. A compile-time assertion ties the table's declared variants to `ParsedArgs['command']` in both directions: adding a variant to the union without claiming it in the table fails `tsc`, not just the test. Contract audit. `runCli` already returned an exit code and wrote only through the injected `CliIo`; the one real violation was global signal handlers, installed in five places (hn-monitor, tick, cloud-run, serve-webhook, check --watch). Those are now threaded through one `withInterrupt` helper and an optional `RunCliOptions.signal`. Passing a signal installs nothing, which is what the surface does; passing none keeps the standalone binary owning SIGINT exactly as before, and a test proves that half with a real emitted signal. The contract package is linked dev-only through tsconfig `paths` and a vitest alias instead of a `file:` devDependency: npm records a `file:` path in package-lock.json, and `checkInstalledVersions` in bundle-typescript.ts rejects non-portable lockfile entries, which would break every standalone TS bundle build. The tests still run the real `assertSurfaceConforms`/`walkCommands`, and `dist/relay-cli.d.ts` stays self-contained, so the published package gains no dependency on relay. Tests: conformance against the real contract, a drift test covering both directions, and an E2E that runs a declarative flow and an authored .flow.ts on the real relayflowd kernel through `surface.run`, asserting the exit code, the progress written to the injected io, and the file the run actually wrote. Co-Authored-By: Claude Opus 5 (1M context) Session-Id: 318c0079-049c-468e-872e-2878cedbd37a --- packages/sdk/package.json | 4 + packages/sdk/src/cli-commands.ts | 275 ++++++++++++++++++ packages/sdk/src/cli-watch.ts | 17 +- packages/sdk/src/cli.ts | 130 ++++++--- packages/sdk/src/cli/cloud-run.ts | 21 +- packages/sdk/src/cli/serve-webhook.ts | 15 +- packages/sdk/src/relay-cli.ts | 117 ++++++++ .../sdk/tests/relay-cli-surface-live.test.ts | 128 ++++++++ packages/sdk/tests/relay-cli-surface.test.ts | 244 ++++++++++++++++ packages/sdk/tsconfig.tests.json | 16 +- packages/sdk/vitest.config.ts | 19 ++ 11 files changed, 918 insertions(+), 68 deletions(-) create mode 100644 packages/sdk/src/cli-commands.ts create mode 100644 packages/sdk/src/relay-cli.ts create mode 100644 packages/sdk/tests/relay-cli-surface-live.test.ts create mode 100644 packages/sdk/tests/relay-cli-surface.test.ts diff --git a/packages/sdk/package.json b/packages/sdk/package.json index 37dbf9b39..333453c15 100644 --- a/packages/sdk/package.json +++ b/packages/sdk/package.json @@ -24,6 +24,10 @@ "./cli": { "types": "./dist/cli.d.ts", "import": "./dist/cli.js" + }, + "./relay-cli": { + "types": "./dist/relay-cli.d.ts", + "import": "./dist/relay-cli.js" } }, "files": [ diff --git a/packages/sdk/src/cli-commands.ts b/packages/sdk/src/cli-commands.ts new file mode 100644 index 000000000..4434f7317 --- /dev/null +++ b/packages/sdk/src/cli-commands.ts @@ -0,0 +1,275 @@ +import { DEFAULT_DATA_DIR } from './daemon-connection.js'; +import type { ParsedArgs } from './cli.js'; + +/** + * The one declaration of the `flows` command surface. + * + * Two consumers read this table and nothing else: + * + * 1. `parseArgs` in `cli.ts` gates its verb dispatch on {@link CLI_VERB_NAMES}, + * so a token that is not in this table can never reach a parser. + * 2. `createRelayCliSurface` in `relay-cli.ts` projects it into the + * `RelayCliSurface.commands` tree the `agent-relay` host mounts. + * + * Because both sides derive from one array, `commands` and `run` cannot + * describe different trees. The drift test proves the remaining direction -- + * that each declared command actually parses -- and {@link CLI_VERBS} carries a + * compile-time assertion that every `ParsedArgs` variant is claimed by some + * verb, so extending the union without extending this table fails `tsc`. + * + * The types here are deliberately local rather than imported from + * `@agent-relay/cli-surface`: they are structurally identical, and keeping the + * import out means `dist/relay-cli.d.ts` stands alone, so a consumer of + * `@relayflows/sdk` never needs the contract package resolvable. Assignability + * to the real contract is asserted in `tests/relay-cli-surface.test.ts`. + */ + +/** A positional argument. Mirrors `RelayCliArgSpec`. */ +export interface CliArgSpec { + name: string; + description: string; + required: boolean; + variadic?: boolean; +} + +/** A flag, in commander's flag-string form. Mirrors `RelayCliOptionSpec`. */ +export interface CliOptionSpec { + flags: string; + description: string; + defaultValue?: string | boolean | number; +} + +/** One node of the command tree. Mirrors `RelayCliCommandSpec`. */ +export interface CliCommandSpec { + name: string; + description: string; + aliases?: readonly string[]; + args?: readonly CliArgSpec[]; + options?: readonly CliOptionSpec[]; + subcommands?: readonly CliCommandSpec[]; +} + +/** A top-level verb, plus the `ParsedArgs` variants it can produce. */ +export interface CliVerbSpec extends CliCommandSpec { + /** + * Every `ParsedArgs.command` this verb can parse to. + * + * Usually one, but the verb set and the variant set are not one-to-one: + * `deploy` produces `deploy` for a digest reference and `cloud-deploy` for + * authored source, and `run` produces `cloud-run` under `--cloud`. + */ + variants: readonly ParsedArgs['command'][]; +} + +const DATA_DIR_OPTION: CliOptionSpec = { + flags: '--data-dir ', + description: 'Daemon data directory', + defaultValue: DEFAULT_DATA_DIR, +}; + +const JSON_OPTION: CliOptionSpec = { + flags: '--json', + description: 'Emit one machine-readable JSON object instead of text', +}; + +/** Flags shared by the two verbs that execute a flow locally. */ +const LOCAL_EXECUTION_OPTIONS = [ + JSON_OPTION, + DATA_DIR_OPTION, + { flags: '--local-agent', description: 'Run agent steps in this process instead of a worker' }, + { flags: '--no-spawn', description: 'Require a running relayflowd rather than starting one' }, + { flags: '--no-observer-link', description: 'Do not mint an observer link for this run' }, + { + flags: '--allow-human-influenced', + description: 'Proceed even though the run carries human-influenced state', + }, +] as const satisfies readonly CliOptionSpec[]; + +/** + * `as const satisfies` rather than a type annotation on purpose: an annotation + * would widen every `variants` entry to `ParsedArgs['command']` and the + * compile-time exhaustiveness assertion below would pass vacuously. + */ +export const CLI_VERBS = [ + { + name: 'add', + description: 'Install a helper plugin into this project', + args: [{ name: 'helper', description: 'Helper name or @flows/', required: true }], + variants: ['add'], + }, + { + name: 'build', + description: 'Compile a flow into a sealed, content-addressed bundle', + args: [{ name: 'source', description: 'flow.yaml, flow.ts, or a bundle directory with --verify', required: true }], + options: [ + { flags: '--out ', description: 'Directory to write the bundle into' }, + { flags: '--verify', description: 'Verify an existing bundle directory instead of building' }, + JSON_OPTION, + ], + variants: ['build'], + }, + { + name: 'check', + description: 'Compile and preflight a flow without running it, or opening a daemon socket', + args: [{ name: 'source', description: 'flow.ts, flow.yaml, or spec.json', required: true }], + options: [JSON_OPTION, { flags: '--watch', description: 'Re-check on every change to the flow and its imports' }], + variants: ['check'], + }, + { + name: 'deploy', + description: 'Copy a sealed bundle into a file bucket, or deploy a hosted trigger listener', + args: [{ name: 'flow', description: 'flow.ts for a hosted listener, or @sha256: for a bundle', required: true }], + options: [ + { flags: '--to ', description: 'Destination file bucket for a sealed bundle' }, + { flags: '--repo ', description: 'Repository the hosted listener watches' }, + { flags: '--on ', description: 'Trigger source, as [:key=value,...]; repeatable' }, + { flags: '--approver ', description: 'Handle delivered to every launched run as input.approver' }, + { flags: '--agents ', description: 'Agent harnesses to allow, as claude[,codex]' }, + { flags: '--name ', description: 'Name for the hosted listener' }, + { flags: '--draft', description: 'Create the listener without activating it' }, + JSON_OPTION, + ], + variants: ['deploy', 'cloud-deploy'], + }, + { + name: 'deployments', + description: 'List this workspace’s hosted trigger listeners', + options: [JSON_OPTION], + variants: ['deployments'], + }, + { + name: 'hn-monitor', + description: 'Hacker News monitor: poll for matching stories and launch a flow per hit', + subcommands: [ + { + name: 'start', + description: 'Start polling in the foreground', + args: [{ name: 'spec', description: 'Monitor spec.json', required: true }], + options: [ + DATA_DIR_OPTION, + { flags: '--poll-interval-ms ', description: 'Milliseconds between polls' }, + ], + }, + ], + variants: ['hn-monitor'], + }, + { + name: 'observer', + description: 'Mint a read-only observer link without running a flow', + options: [DATA_DIR_OPTION], + variants: ['observer'], + }, + { + name: 'replay', + description: 'Replay a finished run from its local journal', + args: [{ name: 'run-id', description: 'Run id to replay', required: true }], + options: [ + JSON_OPTION, + DATA_DIR_OPTION, + { flags: '--at ', description: 'Replay up to this step' }, + { + flags: '--allow-human-influenced', + description: 'Proceed even though the run carries human-influenced state', + }, + ], + variants: ['replay'], + }, + { + name: 'resume', + description: 'Resume an interrupted local run from where its journal left off', + args: [{ name: 'run-id', description: 'Run id to resume', required: true }], + options: LOCAL_EXECUTION_OPTIONS, + variants: ['resume'], + }, + { + name: 'run', + description: 'Run a flow locally, or submit it to Cloud with --cloud', + args: [{ name: 'flow', description: 'flow.yaml, flow.ts, spec.json, or @sha256:', required: true }], + options: [ + ...LOCAL_EXECUTION_OPTIONS, + { flags: '--input ', description: 'Input for an authored .flow.ts, inline JSON or a file path' }, + { flags: '--bucket ', description: 'File bucket to fetch a sealed bundle from' }, + { flags: '--reuse-from ', description: 'Reuse memoized step outputs from an earlier run' }, + { flags: '--cloud', description: 'Submit to Agent Relay Cloud instead of running locally' }, + { flags: '--wait', description: 'With --cloud, poll until the hosted run reaches a terminal state' }, + { + flags: '--sync-code', + description: 'With --cloud, upload the working directory as the run’s tree; pull results back with `flows sync`', + }, + ], + variants: ['run', 'cloud-run'], + }, + { + name: 'serve-webhook', + description: 'Run the local webhook receiver that writes provider deliveries into the trigger inbox', + options: [ + DATA_DIR_OPTION, + { flags: '--port ', description: 'Port to listen on, bound to 127.0.0.1' }, + { flags: '--allow ', description: 'Comma-separated flow names this receiver admits' }, + ], + variants: ['serve-webhook'], + }, + { + name: 'sync', + description: 'Apply a hosted run’s code changes to a local tree (replaces `agent-relay cloud sync`)', + args: [{ name: 'run-id', description: 'Hosted run id whose patch to apply', required: true }], + options: [ + JSON_OPTION, + { flags: '--dir ', description: 'Tree to apply the patch to', defaultValue: '.' }, + ], + variants: ['sync'], + }, + { + name: 'tick', + description: 'Interval scheduler: launch a flow on a fixed local cadence', + subcommands: [ + { + name: 'start', + description: 'Start ticking in the foreground', + args: [{ name: 'spec', description: 'Flow spec.json to launch each tick', required: true }], + options: [ + DATA_DIR_OPTION, + { flags: '--schedule-id ', description: 'Stable id identifying this schedule' }, + { flags: '--interval-ms ', description: 'Milliseconds between ticks' }, + { flags: '--epoch-ms ', description: 'Epoch the tick grid is aligned to' }, + { flags: '--max-catch-up ', description: 'Most missed ticks to replay after a gap' }, + { flags: '--poll-interval-ms ', description: 'Milliseconds between schedule polls' }, + ], + }, + ], + variants: ['tick'], + }, + { + name: 'undeploy', + description: 'Remove a hosted trigger listener', + args: [{ name: 'deployment-id', description: 'Deployment id to remove', required: true }], + options: [JSON_OPTION], + variants: ['undeploy'], + }, +] as const satisfies readonly CliVerbSpec[]; + +/** + * Every `ParsedArgs` variant claimed by some verb in {@link CLI_VERBS}. + * + * Widened from the table rather than written down, so it tracks the table. + */ +type DeclaredVariant = (typeof CLI_VERBS)[number]['variants'][number]; + +/** + * Compile-time drift guard, the half a runtime test cannot cover. + * + * Adding a variant to `ParsedArgs` without giving some verb a claim on it + * leaves `Exclude` non-`never`, and + * this assignment stops compiling. The reverse direction catches a table entry + * naming a variant that no longer exists. + */ +type AssertNever = T; +export type _EveryVariantIsDeclared = AssertNever>; +export type _EveryDeclaredVariantExists = AssertNever>; + +/** + * The verbs `parseArgs` accepts. A token outside this set is refused before any + * per-verb parser sees it, which is what keeps dispatch and {@link CLI_VERBS} + * from drifting apart. + */ +export const CLI_VERB_NAMES: ReadonlySet = new Set(CLI_VERBS.map((verb) => verb.name)); diff --git a/packages/sdk/src/cli-watch.ts b/packages/sdk/src/cli-watch.ts index c2559f9ba..2fe2a6dbb 100644 --- a/packages/sdk/src/cli-watch.ts +++ b/packages/sdk/src/cli-watch.ts @@ -9,24 +9,23 @@ const DEBOUNCE_MS = 150; type ExitCode = 0 | 1 | 2 | 3; /** A runner around the ordinary CLI, with a fresh module cache for every check. */ -export async function watchCheck(path: string, json: boolean, io: CliIo): Promise { - const controller = new AbortController(); - const stop = (): void => controller.abort(); - process.once('SIGINT', stop); - process.once('SIGTERM', stop); +export async function watchCheck( + path: string, + json: boolean, + io: CliIo, + /** Cancellation, owned by the caller; this function installs no signal handler. */ + signal: AbortSignal, +): Promise { try { return await watchChecks({ path, - signal: controller.signal, + signal, check: () => checkOnce(path, json, io), clear: () => { if (!json) io.stdout('\x1b[2J\x1b[H'); }, }); } catch (error) { io.stderr(`REFUSED [input_unreadable] Could not watch "${path}": ${error instanceof Error ? error.message : String(error)}`); return 2; - } finally { - process.off('SIGINT', stop); - process.off('SIGTERM', stop); } } diff --git a/packages/sdk/src/cli.ts b/packages/sdk/src/cli.ts index 2dfad5df1..82f43cfe9 100644 --- a/packages/sdk/src/cli.ts +++ b/packages/sdk/src/cli.ts @@ -33,6 +33,7 @@ import { parseBuildArgs, runBuild, type BuildArgs } from './cli/build.js'; import { runHnMonitor } from './cli/hn-monitor.js'; import { runTickRunner } from './cli/tick-runner.js'; import { DEFAULT_DATA_DIR } from './daemon-connection.js'; +import { CLI_VERB_NAMES } from './cli-commands.js'; import { mintObserverUrl, resolveObserverLinkEnv, @@ -47,7 +48,12 @@ export interface CliIo { } type CliExitCode = 0 | 1 | 2 | 3; -type ParsedArgs = + +/** + * Every shape `parseArgs` can produce. Exported for `cli-commands.ts`, whose + * table must claim each variant or fail to compile. + */ +export type ParsedArgs = | { command: 'add'; value: string } | ReplayArgs | BuildArgs @@ -106,9 +112,48 @@ const PROCESS_IO: CliIo = { stderr: (line) => process.stderr.write(`${line}\n`), }; +/** Optional knobs for an embedded caller. `bin/flows.js` passes none. */ +export interface RunCliOptions { + /** + * Cancellation for the long-running verbs (`run --cloud`, `check --watch`, + * `serve-webhook`, `hn-monitor start`, `tick start`). + * + * Supply one and `runCli` installs **no** process signal handlers -- required + * of a CLI surface mounted into another host, which owns SIGINT itself. + * Omit it and the standalone `flows` binary keeps today's behaviour exactly: + * SIGINT/SIGTERM are handled here, for the duration of that verb only. + */ + signal?: AbortSignal; +} + +/** + * Run one long-running verb under a cancellation signal. + * + * With a caller-supplied signal this installs nothing. Without one it owns + * SIGINT/SIGTERM for the duration of `body` and removes the handlers after -- + * the pre-existing standalone behaviour, unchanged. + */ +async function withInterrupt( + provided: AbortSignal | undefined, + body: (signal: AbortSignal) => Promise, +): Promise { + if (provided !== undefined) return body(provided); + const controller = new AbortController(); + const onSignal = (): void => controller.abort(); + process.once('SIGINT', onSignal); + process.once('SIGTERM', onSignal); + try { + return await body(controller.signal); + } finally { + process.off('SIGINT', onSignal); + process.off('SIGTERM', onSignal); + } +} + export async function runCli( args: readonly string[], io: CliIo = PROCESS_IO, + options: RunCliOptions = {}, ): Promise { if (args.length === 1 && (args[0] === '--help' || args[0] === '-h')) { io.stdout(USAGE); @@ -124,9 +169,13 @@ export async function runCli( if (parsed.command === 'add') return addPlugin(parsed.value, io); - if (parsed.command === 'serve-webhook') return runServeWebhook(parsed, io); + if (parsed.command === 'serve-webhook') { + return withInterrupt(options.signal, (signal) => runServeWebhook(parsed, io, signal)); + } - if (parsed.command === 'cloud-run') return runCloudCli(parsed, io); + if (parsed.command === 'cloud-run') { + return withInterrupt(options.signal, (signal) => runCloudCli(parsed, io, signal)); + } if (parsed.command === 'sync') return runCloudSyncCli(parsed, io); if (parsed.command === 'cloud-deploy') return runCloudDeployCli(parsed, io); if (parsed.command === 'deployments') return runCloudDeploymentsCli(parsed, io); @@ -136,7 +185,9 @@ export async function runCli( if (parsed.command === 'deploy') return runDeploy(parsed, io); if (parsed.command === 'check') { - if (parsed.watch) return watchCheck(parsed.value, parsed.json, io); + if (parsed.watch) { + return withInterrupt(options.signal, (signal) => watchCheck(parsed.value, parsed.json, io, signal)); + } // Deliberately daemon-free (kernel/DAEMON-LIFECYCLE.md §4). `checkFlow` is // a compile-and-preflight that opens no daemon socket, and the parser // refuses `--data-dir` on `check`, so there is no data dir to attach to. @@ -151,45 +202,27 @@ export async function runCli( if (parsed.command === 'observer') return runObserverCommand(io); if (parsed.command === 'hn-monitor') { - const controller = new AbortController(); - const onSignal = (): void => controller.abort(); - process.once('SIGINT', onSignal); - process.once('SIGTERM', onSignal); - try { - return await runHnMonitor({ - dataDir: parsed.dataDir, - specPath: parsed.specPath, - pollIntervalMs: parsed.pollIntervalMs, - signal: controller.signal, - }, io); - } finally { - process.off('SIGINT', onSignal); - process.off('SIGTERM', onSignal); - } + return withInterrupt(options.signal, (signal) => runHnMonitor({ + dataDir: parsed.dataDir, + specPath: parsed.specPath, + pollIntervalMs: parsed.pollIntervalMs, + signal, + }, io)); } if (parsed.command === 'tick') { - const controller = new AbortController(); - const onSignal = (): void => controller.abort(); - process.once('SIGINT', onSignal); - process.once('SIGTERM', onSignal); - try { - return await runTickRunner({ - dataDir: parsed.dataDir, - specPath: parsed.specPath, - schedule: { - scheduleId: parsed.scheduleId, - intervalMs: parsed.intervalMs, - ...(parsed.epochMs === undefined ? {} : { epochMs: parsed.epochMs }), - ...(parsed.maxCatchUp === undefined ? {} : { maxCatchUp: parsed.maxCatchUp }), - }, - pollIntervalMs: parsed.pollIntervalMs, - signal: controller.signal, - }, io) as CliExitCode; - } finally { - process.off('SIGINT', onSignal); - process.off('SIGTERM', onSignal); - } + return withInterrupt(options.signal, async (signal) => await runTickRunner({ + dataDir: parsed.dataDir, + specPath: parsed.specPath, + schedule: { + scheduleId: parsed.scheduleId, + intervalMs: parsed.intervalMs, + ...(parsed.epochMs === undefined ? {} : { epochMs: parsed.epochMs }), + ...(parsed.maxCatchUp === undefined ? {} : { maxCatchUp: parsed.maxCatchUp }), + }, + pollIntervalMs: parsed.pollIntervalMs, + signal, + }, io) as CliExitCode); } // Attach-or-spawn runs inside `runFlow`/`resumeFlow`/`runDirectFlow`, at the @@ -431,6 +464,11 @@ function emitWait( function parseArgs(args: readonly string[]): ParsedArgs | undefined { const command = args[0]; + // The verb set lives in exactly one place -- `CLI_VERBS` in cli-commands.ts -- + // which is also what `createRelayCliSurface` projects into `commands`. Gating + // dispatch on it means a token the surface does not declare can never reach a + // parser, so the declared tree and the dispatched tree cannot drift apart. + if (command === undefined || !CLI_VERB_NAMES.has(command)) return undefined; if (command === 'add') return args.length === 2 ? { command: 'add', value: args[1]! } : undefined; if (command === 'replay') return parseReplayArgs(args.slice(1)); if (command === 'build') return parseBuildArgs(args.slice(1)); @@ -831,3 +869,15 @@ if (isDirectInvocation(process.argv[1])) { process.exitCode = exitCode; }); } + +/** + * The argv parser, exported for the CLI-surface drift test. + * + * The drift test must prove that every command `cli-commands.ts` declares + * actually routes to a `ParsedArgs` variant, and that every variant is + * reachable from some declared command. Observing that through `runCli` would + * mean executing the commands. Not part of the package's public API -- + * `@relayflows/sdk/cli` exports `runCli`, and `@relayflows/sdk/relay-cli` + * exports the surface. + */ +export { parseArgs as parseCliArgs }; diff --git a/packages/sdk/src/cli/cloud-run.ts b/packages/sdk/src/cli/cloud-run.ts index 6c7d38949..328448d91 100644 --- a/packages/sdk/src/cli/cloud-run.ts +++ b/packages/sdk/src/cli/cloud-run.ts @@ -10,18 +10,20 @@ export async function runCloudCli( value: string; json: boolean; wait: boolean; input: string | undefined; syncCode: boolean; }, io: CliIo, + /** + * Cancellation, owned by the caller. `runCli` supplies either the embedder's + * signal or one it drives from SIGINT/SIGTERM itself, so no process-wide + * handler is installed here. + */ + signal: AbortSignal, ): Promise<0 | 1 | 2> { - const controller = new AbortController(); - const abort = (): void => controller.abort(); - process.once('SIGINT', abort); - process.once('SIGTERM', abort); let runId: string | undefined; // Flips at the run submission. An interruption before it — during prepare, // packing or upload — admitted nothing and is safe to retry; only an // interrupted submission has unknown admission. let submitting = false; try { - const options: RunInCloudOptions = { signal: controller.signal, onSubmit: () => { submitting = true; } }; + const options: RunInCloudOptions = { signal, onSubmit: () => { submitting = true; } }; if (isAuthoredFlowPath(path)) { // Same parse as a local direct run, so a file-or-inline argument means // the same thing on both sides of `--cloud`. @@ -51,16 +53,16 @@ export async function runCloudCli( if (json) io.stdout(JSON.stringify({ ok: true, ...receipt })); return 0; } - const run = await waitForCloudFlowRun(receipt.runId, { signal: controller.signal }); + const run = await waitForCloudFlowRun(receipt.runId, { signal }); const ok = run.status === 'completed'; if (json) io.stdout(JSON.stringify({ ok, ...receipt, ...run })); else io.stdout(`${run.status.toUpperCase()} ${run.runId} completionReason: ${'completionReason' in run ? run.completionReason : 'unavailable'}`); return ok ? 0 : 1; } catch (error) { - const code = controller.signal.aborted + const code = signal.aborted ? runId ? 'observation_aborted' : submitting ? 'admission_unknown' : 'submission_aborted' : error instanceof CloudFlowError ? error.code : 'cloud_run_failed'; - const message = controller.signal.aborted + const message = signal.aborted ? runId ? 'Stopped observing; the hosted run has not been cancelled.' : submitting ? 'Submission interrupted before a receipt was received. Admission is unknown; Cloud may have started the run. Do not resubmit blindly.' @@ -71,8 +73,5 @@ export async function runCloudCli( return error instanceof CloudFlowError && (['configuration', 'unsupported_source', 'invalid_input', 'unsupported_storage_backend', 'sync_too_large', 'sync_unsupported'].includes(error.code) || (runId === undefined && error.code === 'http_error' && [401, 403].includes(error.status ?? 0))) ? 2 : 1; - } finally { - process.off('SIGINT', abort); - process.off('SIGTERM', abort); } } diff --git a/packages/sdk/src/cli/serve-webhook.ts b/packages/sdk/src/cli/serve-webhook.ts index 3f156f765..0c6e75163 100644 --- a/packages/sdk/src/cli/serve-webhook.ts +++ b/packages/sdk/src/cli/serve-webhook.ts @@ -215,6 +215,12 @@ async function directory(path: string): Promise { export async function runServeWebhook( options: { dataDir: string; port: number; admitted?: readonly string[] }, io: CliIo, + /** + * Shutdown, owned by the caller. `runCli` supplies either the embedder's + * signal or one it drives from SIGINT/SIGTERM, so the receiver installs no + * process-wide handler of its own. + */ + signal: AbortSignal, ): Promise<0 | 1> { try { const admittedNames = options.admitted === undefined ? undefined : new Set(options.admitted); @@ -231,12 +237,9 @@ export async function runServeWebhook( server.closeAllConnections(); }; server.once('error', reject); - process.once('SIGINT', stop); - process.once('SIGTERM', stop); - server.once('close', () => { - process.off('SIGINT', stop); - process.off('SIGTERM', stop); - }); + if (signal.aborted) { stop(); return; } + signal.addEventListener('abort', stop, { once: true }); + server.once('close', () => signal.removeEventListener('abort', stop)); }); return 0; } catch (error) { diff --git a/packages/sdk/src/relay-cli.ts b/packages/sdk/src/relay-cli.ts new file mode 100644 index 000000000..93b0ef68b --- /dev/null +++ b/packages/sdk/src/relay-cli.ts @@ -0,0 +1,117 @@ +import { readFileSync } from 'node:fs'; + +import { runCli } from './cli.js'; +import { CLI_VERBS, CLI_VERB_NAMES, type CliCommandSpec } from './cli-commands.js'; + +/** + * The `@relayflows/sdk/relay-cli` entrypoint: a mountable CLI surface. + * + * `agent-relay` mounts this as `agent-relay flows`. The surface is a thin + * projection over the CLI this package already ships -- `commands` comes from + * the same `CLI_VERBS` table `parseArgs` dispatches on, and `run` delegates + * straight to `runCli`. No command is reimplemented here, and + * `packages/relayflows/bin/flows.js` keeps calling `runCli` on the same path. + * + * Structurally typed against `@agent-relay/cli-surface` without importing it, + * so `dist/relay-cli.d.ts` has no dependency on the contract package and this + * package gains no runtime dependency on relay. `tests/relay-cli-surface.test.ts` + * asserts the assignability and runs the contract's own conformance checks. + */ + +/** Host-supplied output sink. Mirrors `RelayCliIo`. */ +export interface RelayCliIo { + stdout(chunk: string): void; + stderr(chunk: string): void; +} + +/** A mountable product CLI. Mirrors `RelayCliSurface`. */ +export interface RelayCliSurface { + id: string; + version: string; + contract: 1; + commands: readonly CliCommandSpec[]; + run(argv: readonly string[], io: RelayCliIo): Promise; +} + +/** Options for {@link createRelayCliSurface}. */ +export interface CreateRelayCliSurfaceOptions { + /** + * Cancellation for the long-running verbs, owned by the host. + * + * A surface must install no global signal handlers, so one is always passed + * to `runCli` -- a never-aborting signal when the host supplies none. Pass a + * real one to get graceful cancellation (for example, the + * "Stopped observing; the hosted run has not been cancelled" path on + * `run --cloud --wait`) instead of the host's SIGINT killing the process. + */ + signal?: AbortSignal; +} + +/** Exit code for an argv the surface cannot route, per the contract. */ +const EXIT_UNKNOWN_COMMAND = 2; + +/** Drop `variants` -- the routing detail the host has no use for. */ +function toCommandSpec(verb: CliCommandSpec & { variants?: unknown }): CliCommandSpec { + const { variants: _variants, ...spec } = verb; + return spec; +} + +/** + * Read this package's version from its own manifest. + * + * Resolved from `import.meta.url` rather than imported, because `package.json` + * sits outside `rootDir` and a hardcoded literal would silently go stale at the + * next release. `src/` and `dist/` are both one level below the manifest, so + * the same relative path is correct before and after a build. + */ +function packageVersion(): string { + try { + const manifest: unknown = JSON.parse( + readFileSync(new URL('../package.json', import.meta.url), 'utf8'), + ); + const version = (manifest as { version?: unknown }).version; + return typeof version === 'string' && version.length > 0 ? version : '0.0.0'; + } catch { + // A surface that cannot read its own manifest is still perfectly runnable; + // refusing to mount over a cosmetic field would be the worse failure. + return '0.0.0'; + } +} + +/** + * Build the relayflows CLI surface. + * + * @param options - Host-supplied cancellation. + * @returns A surface whose `run` resolves to an exit code, writes only through + * the supplied io, and installs no process signal handlers. + */ +export function createRelayCliSurface( + options: CreateRelayCliSurfaceOptions = {}, +): RelayCliSurface { + return { + id: 'relayflows', + version: packageVersion(), + contract: 1, + commands: CLI_VERBS.map(toCommandSpec), + async run(argv: readonly string[], io: RelayCliIo): Promise { + const verb = argv[0]; + // `runCli` would refuse this too, but with the full usage block. Naming + // the offending token is the more useful answer when the host has just + // routed `agent-relay flows ` here. + if (verb !== undefined && !verb.startsWith('-') && !CLI_VERB_NAMES.has(verb)) { + io.stderr(`error: '${verb}' is not a command of relayflows\n`); + io.stderr("Run 'agent-relay flows --help' for the available commands.\n"); + return EXIT_UNKNOWN_COMMAND; + } + return runCli( + argv, + // `CliIo` is line-oriented and the contract's io is chunk-oriented, so + // the terminator is added here rather than by every call site. + { stdout: (line) => io.stdout(`${line}\n`), stderr: (line) => io.stderr(`${line}\n`) }, + // Always pass a signal: that is what keeps `runCli` from installing the + // SIGINT/SIGTERM handlers the standalone binary still relies on. + { signal: options.signal ?? new AbortController().signal }, + ); + }, + }; +} diff --git a/packages/sdk/tests/relay-cli-surface-live.test.ts b/packages/sdk/tests/relay-cli-surface-live.test.ts new file mode 100644 index 000000000..2cfbbd83f --- /dev/null +++ b/packages/sdk/tests/relay-cli-surface-live.test.ts @@ -0,0 +1,128 @@ +import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterEach, expect, it } from 'vitest'; +import type { RelayCliIo } from '@agent-relay/cli-surface'; + +import { createRelayCliSurface } from '../src/relay-cli.js'; + +/** + * The end-to-end bar for the mounted surface: real flows, executed by the real + * relayflowd kernel, reached only through `surface.run(...)`. + * + * Nothing is stubbed and nothing is mocked. The surface spawns the daemon, the + * kernel journals the run, and the assertions are on what the host would + * actually see -- the exit code, the output the surface wrote to its injected + * io, and (for the authored flow) the side effect the run left on disk. + */ + +const HERE = dirname(fileURLToPath(import.meta.url)); +const ROOT = join(HERE, '..', '..', '..'); +const YAML_FLOW = join(ROOT, 'testdata', 'hello-deterministic.flow.yaml'); +const AUTHORED_FLOW = join(HERE, 'fixtures', 'direct-input.flow.ts'); + +const KERNEL = process.env['RELAYFLOWD_BIN']; +/** Opt in with the same real-kernel override the other SDK live tests use. */ +const noKernel = !KERNEL || !existsSync(KERNEL); + +const temporaryDirectories: string[] = []; + +afterEach(() => { + for (const directory of temporaryDirectories.splice(0)) { + // Stop the daemon this run spawned before removing its data directory. + try { + const connection: { pid?: number } = JSON.parse( + readFileSync(join(directory, 'connection.json'), 'utf8'), + ); + if (typeof connection.pid === 'number') process.kill(connection.pid, 'SIGTERM'); + } catch { + // No daemon to stop when preflight refused before spawning one. + } + rmSync(directory, { recursive: true, force: true }); + } +}); + +function dataDirectory(prefix: string): string { + const directory = mkdtempSync(join(tmpdir(), prefix)); + temporaryDirectories.push(directory); + return directory; +} + +function capture(): RelayCliIo & { out: string; err: string } { + const sink = { + out: '', + err: '', + stdout(chunk: string) { sink.out += chunk; }, + stderr(chunk: string) { sink.err += chunk; }, + }; + return sink; +} + +it.skipIf(noKernel)( + 'runs a declarative flow end to end on the real kernel through surface.run', + async () => { + const dataDir = dataDirectory('flows-surface-e2e-'); + const io = capture(); + + const code = await createRelayCliSurface().run( + ['run', '--data-dir', dataDir, '--no-observer-link', YAML_FLOW], + io, + ); + + expect(code, io.err || io.out).toBe(0); + // The run summary the host would show the user, on the injected stdout. + expect(io.out).toMatch(/^RUN \S+ completed \(2 steps\) completionReason: success$/m); + // Preflight diagnostics go to the injected stderr, not the process's own. + expect(io.err).toContain('WARNING [unprovable_effects]'); + }, + 60_000, +); + +it.skipIf(noKernel)( + 'reports the same declarative run as one JSON object under --json', + async () => { + const dataDir = dataDirectory('flows-surface-e2e-json-'); + const io = capture(); + + const code = await createRelayCliSurface().run( + ['run', '--json', '--data-dir', dataDir, '--no-observer-link', YAML_FLOW], + io, + ); + + expect(code, io.err || io.out).toBe(0); + const report: unknown = JSON.parse(io.out); + expect(report).toMatchObject({ completionReason: 'success', completedSteps: 2 }); + }, + 60_000, +); + +it.skipIf(noKernel)( + 'streams authored step progress through the injected io, and leaves the run’s side effect on disk', + async () => { + // An authored .flow.ts, because per-step progress is emitted by the + // authored executor (`observeStep`); a declarative YAML flow is stepped by + // the kernel and reports only its summary. Asserting progress on the YAML + // run would be asserting something the runtime never emits. + const dataDir = dataDirectory('flows-surface-e2e-authored-'); + const written = join(dataDir, 'written.txt'); + const io = capture(); + + const code = await createRelayCliSurface().run( + [ + 'run', '--data-dir', dataDir, '--no-observer-link', AUTHORED_FLOW, + '--input', JSON.stringify({ output: written, value: 'hello-from-surface' }), + ], + io, + ); + + expect(code, io.err || io.out).toBe(0); + // Progress, rendered by `renderProgress` and routed to the host's stderr. + expect(io.err).toMatch(/○ run-1 \(deterministic\)/); + expect(io.err).toMatch(/✓ run-1 \(deterministic\).*completionReason: success/); + expect(io.out).toMatch(/^RUN \S+ completed \(2 steps\) completionReason: success$/m); + // The step really executed: its shell command wrote this file. + expect(readFileSync(written, 'utf8')).toBe('hello-from-surface'); + }, + 60_000, +); diff --git a/packages/sdk/tests/relay-cli-surface.test.ts b/packages/sdk/tests/relay-cli-surface.test.ts new file mode 100644 index 000000000..621cf7e57 --- /dev/null +++ b/packages/sdk/tests/relay-cli-surface.test.ts @@ -0,0 +1,244 @@ +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { + assertSurfaceConforms, + findSurfaceViolations, + walkCommands, + type RelayCliIo, + type RelayCliSurface, +} from '@agent-relay/cli-surface'; + +import { createRelayCliSurface } from '../src/relay-cli.js'; +import { CLI_VERBS, type CliVerbSpec } from '../src/cli-commands.js'; +import { parseCliArgs, runCli, type ParsedArgs } from '../src/cli.js'; + +const temporaryDirectories: string[] = []; + +afterEach(() => { + for (const directory of temporaryDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } +}); + +function temporaryDirectory(prefix: string): string { + const directory = mkdtempSync(join(tmpdir(), prefix)); + temporaryDirectories.push(directory); + return directory; +} + +function capture(): RelayCliIo & { out: string; err: string } { + const sink = { + out: '', + err: '', + stdout(chunk: string) { sink.out += chunk; }, + stderr(chunk: string) { sink.err += chunk; }, + }; + return sink; +} + +const DIGEST = `hello@sha256:${'a'.repeat(64)}`; +const RUN_ID = '01JABCDEFGHJKMNPQRSTVWXYZ0'; + +/** + * One realistic invocation per declared command, and the `ParsedArgs` variant + * it must produce. + * + * This is the only hand-written list in the drift check, and the test below + * pins it to `CLI_VERBS` in both directions, so a verb added to the table + * without a sample here fails rather than going silently unexercised. + */ +const INVOCATIONS: readonly { verb: string; argv: readonly string[]; variant: ParsedArgs['command'] }[] = [ + { verb: 'add', argv: ['add', 'my-helper'], variant: 'add' }, + { verb: 'build', argv: ['build', 'flow.yaml'], variant: 'build' }, + { verb: 'build', argv: ['build', '--out', 'dist', 'flow.yaml'], variant: 'build' }, + { verb: 'check', argv: ['check', 'flow.yaml'], variant: 'check' }, + { verb: 'check', argv: ['check', '--watch', '--json', 'flow.yaml'], variant: 'check' }, + // Both `deploy` forms: the positional decides which variant the verb produces. + { verb: 'deploy', argv: ['deploy', DIGEST, '--to', 'file:///tmp/bucket'], variant: 'deploy' }, + { + verb: 'deploy', + argv: ['deploy', 'review.flow.ts', '--repo', 'owner/name', '--on', 'github', '--approver', 'someone'], + variant: 'cloud-deploy', + }, + { verb: 'deployments', argv: ['deployments', '--json'], variant: 'deployments' }, + { verb: 'hn-monitor', argv: ['hn-monitor', 'start', 'spec.json'], variant: 'hn-monitor' }, + { verb: 'observer', argv: ['observer'], variant: 'observer' }, + { verb: 'replay', argv: ['replay', RUN_ID], variant: 'replay' }, + { verb: 'resume', argv: ['resume', RUN_ID], variant: 'resume' }, + { verb: 'run', argv: ['run', 'flow.yaml'], variant: 'run' }, + // `--cloud` is the second variant of the same verb. + { verb: 'run', argv: ['run', '--cloud', '--wait', 'flow.yaml'], variant: 'cloud-run' }, + { + verb: 'serve-webhook', + argv: ['serve-webhook', '--data-dir', '/tmp/inbox', '--port', '8080'], + variant: 'serve-webhook', + }, + { verb: 'sync', argv: ['sync', RUN_ID], variant: 'sync' }, + { + verb: 'tick', + argv: ['tick', 'start', '--schedule-id', 'nightly', '--interval-ms', '60000', 'spec.json'], + variant: 'tick', + }, + { verb: 'undeploy', argv: ['undeploy', 'dep_123'], variant: 'undeploy' }, +]; + +describe('relay-cli surface: contract conformance', () => { + it('satisfies the structural contract from @agent-relay/cli-surface', () => { + // Typed assignment, not a cast: this is where the locally-declared surface + // shape in src/relay-cli.ts is proven to BE a RelayCliSurface. If the two + // drift, this file stops compiling under `npm run typecheck:tests`. + const surface: RelayCliSurface = createRelayCliSurface(); + + expect(findSurfaceViolations(surface)).toEqual([]); + expect(() => assertSurfaceConforms(surface)).not.toThrow(); + }); + + it('identifies itself as relayflows on contract v1 with this package version', async () => { + const surface = createRelayCliSurface(); + const manifest: { version: string } = JSON.parse( + await import('node:fs/promises').then((fs) => + fs.readFile(new URL('../package.json', import.meta.url), 'utf8')), + ); + + expect(surface.id).toBe('relayflows'); + expect(surface.contract).toBe(1); + // Read from the manifest, so a release bump cannot leave a stale literal. + expect(surface.version).toBe(manifest.version); + expect(surface.version).not.toBe('0.0.0'); + }); + + it('declares no routing internals to the host', () => { + // `variants` is how the table ties itself to ParsedArgs; it is not part of + // the contract and must not leak into the mounted tree. + for (const { command } of walkCommands(createRelayCliSurface().commands)) { + expect(command).not.toHaveProperty('variants'); + } + }); +}); + +describe('relay-cli surface: drift between `commands` and `run`', () => { + const surface = createRelayCliSurface(); + const declaredTopLevel = surface.commands.map((command) => command.name); + + it('declares exactly the verbs the parser dispatches', () => { + // Both sides read CLI_VERBS, so this pins the projection rather than a + // second list: `commands` must lose nothing on its way to the host. + expect(declaredTopLevel).toEqual(CLI_VERBS.map((verb) => verb.name)); + }); + + it('exercises every declared command, and only declared commands', () => { + // Closes the loop on INVOCATIONS: a verb added to CLI_VERBS with no sample + // below fails here instead of quietly skipping the routing assertion. + expect(new Set(INVOCATIONS.map((invocation) => invocation.verb))) + .toEqual(new Set(declaredTopLevel)); + }); + + it.each(INVOCATIONS)('routes `flows $verb` to the $variant variant', ({ verb, argv, variant }) => { + const parsed = parseCliArgs(argv); + + expect(parsed, `\`${argv.join(' ')}\` did not parse`).toBeDefined(); + expect(parsed!.command).toBe(variant); + + // And the variant it produced must be one the table says this verb yields. + const declared = CLI_VERBS.find((entry) => entry.name === verb); + expect(declared, `${verb} is declared`).toBeDefined(); + expect(declared!.variants as readonly string[]).toContain(parsed!.command); + }); + + it('reaches every ParsedArgs variant from a declared command', () => { + // The other direction of the drift check. The compile-time assertion in + // cli-commands.ts proves every variant is *claimed* by a verb; this proves + // each claim is actually reachable through the parser. + const claimed = new Set(CLI_VERBS.flatMap((verb) => verb.variants as readonly string[])); + const reached = new Set(INVOCATIONS.map((invocation) => invocation.variant)); + + expect(reached).toEqual(claimed); + }); + + it('declares each subcommand under the name the parser actually requires', () => { + // `hn-monitor` and `tick` route on a second token. The tree must name that + // token exactly, or the host's help sends users to an invocation that + // refuses. (Probing the no-subcommand verbs the same way would prove + // nothing: `flows add start` legitimately parses, with `start` as the + // helper name.) + for (const verb of CLI_VERBS as readonly CliVerbSpec[]) { + if (!verb.subcommands?.length) continue; + + // Every declared subcommand is the leading token of a routable invocation. + for (const sub of verb.subcommands) { + const sample = INVOCATIONS.find( + (invocation) => invocation.verb === verb.name && invocation.argv[1] === sub.name, + ); + expect(sample, `${verb.name} ${sub.name} has a routable invocation`).toBeDefined(); + } + + // And the token is required: the same argv without it is refused. + const routable = INVOCATIONS.find((invocation) => invocation.verb === verb.name)!; + const withoutSubcommand = [routable.argv[0]!, ...routable.argv.slice(2)]; + expect( + parseCliArgs(withoutSubcommand), + `${verb.name} without its subcommand is refused`, + ).toBeUndefined(); + } + }); +}); + +describe('relay-cli surface: run() behaviour', () => { + it('refuses an unknown command with exit 2 and names it on stderr', async () => { + const io = capture(); + + await expect(createRelayCliSurface().run(['bogus'], io)).resolves.toBe(2); + + expect(io.err).toContain("'bogus' is not a command of relayflows"); + expect(io.out).toBe(''); + }); + + it('writes only through the supplied io', async () => { + const io = capture(); + + await expect(createRelayCliSurface().run(['--help'], io)).resolves.toBe(0); + + expect(io.out).toContain('flows run'); + // Line-oriented CliIo, chunk-oriented RelayCliIo: the adapter must add the + // terminator, or the host renders every command on one line. + expect(io.out.endsWith('\n')).toBe(true); + }); + + it('installs no process signal handlers, unlike the standalone binary', async () => { + const before = { + int: process.listenerCount('SIGINT'), + term: process.listenerCount('SIGTERM'), + }; + const io = capture(); + const aborted = AbortSignal.abort(); + + // serve-webhook is one of the five verbs that used to install handlers. It + // really starts the receiver, then stops on the signal rather than a SIGINT. + const code = await createRelayCliSurface({ signal: aborted }) + .run(['serve-webhook', '--data-dir', temporaryDirectory('flows-surface-'), '--port', '0'], io); + + expect(code).toBe(0); + expect(io.out).toContain('WEBHOOK http://127.0.0.1:'); + expect(process.listenerCount('SIGINT')).toBe(before.int); + expect(process.listenerCount('SIGTERM')).toBe(before.term); + }); + + it('leaves the standalone runCli path handling SIGINT as it always did', async () => { + // The other half of the same change: `bin/flows.js` passes no signal, so + // runCli must still own SIGINT. Proven by the real mechanism -- emitting + // the signal is what stops the receiver. + const io = { stdout: () => {}, stderr: () => {} }; + const started = runCli( + ['serve-webhook', '--data-dir', temporaryDirectory('flows-standalone-'), '--port', '0'], + io, + ); + + // Yield until the handler is installed, then interrupt as a user would. + while (process.listenerCount('SIGINT') === 0) await new Promise((r) => setImmediate(r)); + process.emit('SIGINT'); + + await expect(started).resolves.toBe(0); + }); +}); diff --git a/packages/sdk/tsconfig.tests.json b/packages/sdk/tsconfig.tests.json index 8eb1254be..59e4e9308 100644 --- a/packages/sdk/tsconfig.tests.json +++ b/packages/sdk/tsconfig.tests.json @@ -7,7 +7,17 @@ "noEmit": true, "allowImportingTsExtensions": true, "rootDir": ".", - "types": ["node", "vitest"] + "types": ["node", "vitest"], + // The relay CLI-surface contract, resolved dev-only. Deliberately NOT a + // package.json devDependency: npm would write a `file:`/`..` entry into + // package-lock.json, and `checkInstalledVersions` in bundle-typescript.ts + // rejects exactly that, so every standalone TS bundle would stop building. + // A path mapping gives the tests the real contract types with no lockfile + // entry, and the published package keeps no dependency on it at all. + // Mirrored by `resolve.alias` in vitest.config.ts for the runtime helpers. + "paths": { + "@agent-relay/cli-surface": ["../../../relay/packages/cli-surface/dist/index.d.ts"] + } }, "include": [ "src/**/*.ts", @@ -40,7 +50,9 @@ "tests/webhook-hardening.test.ts", "tests/adapters/claude.test.ts", "tests/adapters/codex.test.ts", - "tests/adapters/registry.test.ts" + "tests/adapters/registry.test.ts", + "tests/relay-cli-surface.test.ts", + "tests/relay-cli-surface-live.test.ts" ], "exclude": ["node_modules", "dist"] } diff --git a/packages/sdk/vitest.config.ts b/packages/sdk/vitest.config.ts index efa710b61..dad36a5b9 100644 --- a/packages/sdk/vitest.config.ts +++ b/packages/sdk/vitest.config.ts @@ -1,6 +1,25 @@ +import { fileURLToPath } from 'node:url'; import { defineConfig } from 'vitest/config'; +/** + * The relay CLI-surface contract package, linked dev-only for the surface + * conformance and drift tests. + * + * Not a package.json devDependency on purpose: npm records a `file:` / `..` + * location in package-lock.json, which `checkInstalledVersions` in + * bundle-typescript.ts refuses, breaking every standalone TS bundle build. + * Aliasing it here (and via `paths` in tsconfig.tests.json) gives the tests the + * real `assertSurfaceConforms` / `walkCommands` implementations while leaving + * both the lockfile and the published package untouched. + */ +const CLI_SURFACE = fileURLToPath( + new URL('../../../relay/packages/cli-surface/dist/index.js', import.meta.url), +); + export default defineConfig({ + resolve: { + alias: { '@agent-relay/cli-surface': CLI_SURFACE }, + }, test: { environment: 'node', include: ['tests/**/*.test.ts'], From 3b342c1c03e8c3a17deaa407c78b0848b534ee12 Mon Sep 17 00:00:00 2001 From: agentrelaybot Date: Thu, 17 Sep 2026 15:35:29 -0700 Subject: [PATCH 2/8] feat(sdk): make `flows sync` a drop-in for `agent-relay cloud sync` `agent-relay cloud sync` is to delegate here, and `flows sync` already claims to replace it. It was not a drop-in: three behaviours lived only in relay, and one of them is a safety property, so v2 users were exposed too. Path exclusions. The sandbox commits its baseline before the run, so the agent runtime's own bookkeeping inside a synced tree -- `.agent-bin/**`, `.relayfile.acl`, `.relayfile-mount-state.json` and its `.tmp-*` temporaries, `.trajectories/**`, `.workflow-context/**` -- shows up in the post-run diff. Applying that verbatim drags trajectory records and agent binaries into the user's checkout and overwrites the mount state of the tree being synced into. `CLOUD_SYNC_PATCH_EXCLUDES` is now the single home for the list, and `applyCloudPatch` takes an `exclude` option that defaults to it. Both `git apply` invocations carry the identical arguments: a `--check` without them answers a different question than the apply, passing on an excluded hunk that is never written or failing on one and refusing a patch whose applied part was clean. `applyCloudPatch` now returns what it wrote and what it dropped, so `flows sync` can report both instead of listing paths it did not touch. Dry run. `flows sync --dry-run` prints the patch and applies nothing. Under `--json` the diff travels in the payload's `patch` field rather than loose on stdout beside it, so a consumer still parses one object. Multi-path patches. The `/patch` route branches on the run's `paths`, not on `relayflowVersion`, so a v2 `--sync-code` run that submits several paths gets `{ patches: { : ... } }` too -- this is not a v1 shape. `CloudPatchSet` and `downloadCloudPatchSet` model both; `downloadCloudPatch` keeps refusing the multi-path one unchanged. `flows sync --dry-run` shows each path's patch; applying stays refused, because they target different repositories and no single `--dir` is the right destination. Tests are real: patches touching every excluded pattern are applied by actual `git apply` and the files are asserted absent from disk, the matcher backing the report is pinned to `git apply --numstat` for the same patterns, and the excluded-conflict case proves the check and the apply agree. The surface drift test grew a flag check -- every declared boolean option must be accepted by at least one declared invocation of its verb -- so a switch advertised to the host that the parser refuses now fails there. Co-Authored-By: Claude Opus 5 (1M context) Session-Id: b45ad9f9-470a-495c-a8bf-e9346524097b --- docs/CLOUD.md | 30 +- packages/sdk/src/cli-commands.ts | 1 + packages/sdk/src/cli.ts | 17 +- packages/sdk/src/cli/cloud-sync.ts | 87 ++++- packages/sdk/src/cloud-sync.ts | 175 +++++++++- packages/sdk/src/index.ts | 6 +- packages/sdk/tests/cloud-sync.test.ts | 319 ++++++++++++++++++- packages/sdk/tests/relay-cli-surface.test.ts | 46 +++ 8 files changed, 651 insertions(+), 30 deletions(-) diff --git a/docs/CLOUD.md b/docs/CLOUD.md index 3f26353bf..401c6b649 100644 --- a/docs/CLOUD.md +++ b/docs/CLOUD.md @@ -89,9 +89,33 @@ pointing at a tree Cloud does not hold. and every touched path is listed, deletions included: what the run changed — it is your own flow's output, but it is agent output — is reviewed with `git diff` before any of it is kept, the same contract v1's `cloud sync` had. -Runs that declared several mounted paths carry one patch per path and are -refused here (`sync_unsupported`). `--dir ` targets a checkout other -than the current directory. +`--dir ` targets a checkout other than the current directory. + +The agent runtime's own bookkeeping inside the synced tree is never written. +The sandbox commits its baseline before the run, so `.agent-bin/**`, +`.relayfile.acl`, `.relayfile-mount-state.json` and its `.tmp-*` temporaries, +`.trajectories/**` and `.workflow-context/**` all show up in the post-run diff; +applying them verbatim would drag trajectory records and mount state into your +checkout, and overwrite the mount state of the tree being synced into. They are +dropped with `git apply --exclude`, listed as `SKIPPED` (`excluded` under +`--json`), and — because the exclusions are a property of the patch that lands — +the `--check` pass carries the identical arguments: a conflict in a hunk that is +never applied is not a refusal. The patterns are anchored at the patch root, so +a vendored `packages/x/.agent-bin/tool` belongs to a different tree and rides +along. `CLOUD_SYNC_PATCH_EXCLUDES` is the list's single home; `applyCloudPatch` +takes an `exclude` option, and `[]` applies a patch whole. + +`--dry-run` prints the patch and applies nothing, reporting which paths it would +write and which it would skip. Under `--json` the diff travels in the payload's +`patch` field rather than loose on stdout beside it, so one object still parses. + +A run that declared several mounted paths carries one patch per path, keyed by +path name. `flows sync --dry-run` shows each of them; applying is refused +(`sync_unsupported`, exit 2), because they target different repositories and no +single `--dir` is the right destination — inspect them, then apply each in its +own repository. This is not a v1 shape: the `/patch` route branches on the run's +`paths`, not on `relayflowVersion`, so a v2 `--sync-code` run that submits +several paths answers the same way. A synced run and a Cloud repository grant are mutually exclusive on the server: `--sync-code` is the local-driven development loop, and diff --git a/packages/sdk/src/cli-commands.ts b/packages/sdk/src/cli-commands.ts index 4434f7317..0cb9713de 100644 --- a/packages/sdk/src/cli-commands.ts +++ b/packages/sdk/src/cli-commands.ts @@ -215,6 +215,7 @@ export const CLI_VERBS = [ args: [{ name: 'run-id', description: 'Hosted run id whose patch to apply', required: true }], options: [ JSON_OPTION, + { flags: '--dry-run', description: 'Print the patch and apply nothing' }, { flags: '--dir ', description: 'Tree to apply the patch to', defaultValue: '.' }, ], variants: ['sync'], diff --git a/packages/sdk/src/cli.ts b/packages/sdk/src/cli.ts index 82f43cfe9..b05693de6 100644 --- a/packages/sdk/src/cli.ts +++ b/packages/sdk/src/cli.ts @@ -60,7 +60,7 @@ export type ParsedArgs = | DeployArgs | { command: 'serve-webhook'; dataDir: string; port: number; admitted?: readonly string[] } | { command: 'cloud-run'; value: string; json: boolean; wait: boolean; input: string | undefined; syncCode: boolean } - | { command: 'sync'; runId: string; json: boolean; root: string } + | { command: 'sync'; runId: string; json: boolean; root: string; dryRun: boolean } | CloudDeployArgs | { command: 'deployments'; json: boolean } | { command: 'undeploy'; agentId: string; json: boolean } @@ -88,7 +88,7 @@ const USAGE = [ 'flows run [--json] [--no-spawn] [--no-observer-link] [--data-dir ] [--local-agent] [--reuse-from ] ', 'flows run --cloud [--json] [--wait] [--sync-code] ', 'flows run --cloud [--json] [--wait] [--sync-code] --input ', - 'flows sync [--json] [--dir ] ', + 'flows sync [--json] [--dry-run] [--dir ] ', 'flows run [--json] [--no-spawn] [--no-observer-link] [--data-dir ] [--local-agent] --input ', 'flows tick start --schedule-id --interval-ms [--epoch-ms ] [--max-catch-up ] [--poll-interval-ms ] [--data-dir ] ', 'flows resume [--allow-human-influenced] [--json] [--no-spawn] [--no-observer-link] [--data-dir ] [--local-agent] ', @@ -653,9 +653,13 @@ function parseHnMonitorArgs(rest: readonly string[]): ParsedArgs | undefined { * directory at all -- the mint is a pure Relaycast API round-trip. No * positional argument, no other flags. */ -/** `flows sync [--json] [--dir ] `: apply a hosted run's patch to a local tree. */ +/** + * `flows sync [--json] [--dry-run] [--dir ] `: apply a hosted + * run's patch to a local tree, or with `--dry-run` print it and apply nothing. + */ function parseSyncArgs(args: readonly string[]): ParsedArgs | undefined { let json = false; + let dryRun = false; let root: string | undefined; const positionals: string[] = []; for (let index = 0; index < args.length; index += 1) { @@ -665,6 +669,11 @@ function parseSyncArgs(args: readonly string[]): ParsedArgs | undefined { json = true; continue; } + if (argument === '--dry-run') { + if (dryRun) return undefined; + dryRun = true; + continue; + } if (argument === '--dir') { const value = args[index + 1]; if (root !== undefined || value === undefined || value.startsWith('-')) return undefined; @@ -676,7 +685,7 @@ function parseSyncArgs(args: readonly string[]): ParsedArgs | undefined { positionals.push(argument); } if (positionals.length !== 1) return undefined; - return { command: 'sync', runId: positionals[0]!, json, root: root ?? '.' }; + return { command: 'sync', runId: positionals[0]!, json, dryRun, root: root ?? '.' }; } function parseObserverArgs(rest: readonly string[]): ParsedArgs | undefined { diff --git a/packages/sdk/src/cli/cloud-sync.ts b/packages/sdk/src/cli/cloud-sync.ts index f890c1ea4..b7b5c5616 100644 --- a/packages/sdk/src/cli/cloud-sync.ts +++ b/packages/sdk/src/cli/cloud-sync.ts @@ -1,5 +1,7 @@ import { CloudFlowError } from '../cloud-http.js'; -import { applyCloudPatch, downloadCloudPatch, patchedPaths } from '../cloud-sync.js'; +import { + applyCloudPatch, downloadCloudPatchSet, excludedPatchPaths, patchedPaths, type CloudPathPatch, +} from '../cloud-sync.js'; import type { CliIo } from '../cli.js'; /** @@ -7,23 +9,44 @@ import type { CliIo } from '../cli.js'; * and apply it here. The patch is the sandbox's own `git diff` against the * uploaded baseline, so applying it reproduces exactly what the run's steps * wrote — no re-execution, no re-upload. + * + * The agent runtime's own bookkeeping inside that tree is dropped on the way + * in (`CLOUD_SYNC_PATCH_EXCLUDES`); `--dry-run` prints the patch instead of + * applying it, and a run that produced one patch per mounted path is shown but + * never applied, because no single `--dir` is the right destination for all of + * them. */ export async function runCloudSyncCli( - { runId, json, root }: { runId: string; json: boolean; root: string }, + { runId, json, root, dryRun }: { runId: string; json: boolean; root: string; dryRun: boolean }, io: CliIo, ): Promise<0 | 1 | 2> { try { - const { patch, hasChanges } = await downloadCloudPatch(runId, {}); - if (!hasChanges || !patch.trim()) { + const set = await downloadCloudPatchSet(runId, {}); + if (!set.hasChanges) { if (json) io.stdout(JSON.stringify({ ok: true, runId, hasChanges: false, applied: false })); else io.stdout(`NO CHANGES ${runId}`); return 0; } - applyCloudPatch(root, patch); - const files = patchedPaths(patch); - if (json) io.stdout(JSON.stringify({ ok: true, runId, hasChanges: true, applied: true, files })); + if (set.kind === 'multi-path') { + const changed = set.patches.filter(entry => entry.hasChanges && entry.patch.trim() !== ''); + if (!dryRun) { + throw new CloudFlowError('sync_unsupported', + `Run ${runId} produced ${changed.length} path-scoped patch${changed.length === 1 ? '' : 'es'} ` + + `(${changed.map(entry => entry.name).join(', ')}); flows sync applies single-tree runs only. ` + + 'Inspect them with --dry-run, then apply each in its own repository.'); + } + reportMultiPathDryRun(runId, changed, json, io); + return 0; + } + if (dryRun) { + reportDryRun(runId, set.patch, json, io); + return 0; + } + const { files, excluded } = applyCloudPatch(root, set.patch); + if (json) io.stdout(JSON.stringify({ ok: true, runId, hasChanges: true, applied: true, files, excluded })); else { - io.stdout(`APPLIED ${runId}: ${files.length} file${files.length === 1 ? '' : 's'}${files.length ? `\n ${files.join('\n ')}` : ''}`); + io.stdout(`APPLIED ${runId}: ${files.length} file${files.length === 1 ? '' : 's'}${indented(files)}`); + if (excluded.length) io.stdout(`SKIPPED agent runtime paths: ${excluded.length}${indented(excluded)}`); io.stdout('Applied to the working tree, uncommitted: review with git diff before keeping it.'); } return 0; @@ -35,3 +58,51 @@ export async function runCloudSyncCli( return error instanceof CloudFlowError && ['configuration', 'sync_unsupported', 'patch_conflict'].includes(error.code) ? 2 : 1; } } + +/** A single-tree patch, shown rather than applied. */ +function reportDryRun(runId: string, patch: string, json: boolean, io: CliIo): void { + const excluded = excludedPatchPaths(patch); + const files = patchedPaths(patch).filter(path => !excluded.includes(path)); + if (json) { + // The patch travels in the payload, not on stdout beside it: a --json + // consumer parses one object, and a diff printed alongside it is not JSON. + io.stdout(JSON.stringify({ + ok: true, runId, hasChanges: true, applied: false, dryRun: true, files, excluded, patch, + })); + return; + } + io.stdout(`DRY RUN ${runId}: ${files.length} file${files.length === 1 ? '' : 's'} would be written${indented(files)}`); + if (excluded.length) io.stdout(`Would skip agent runtime paths: ${excluded.length}${indented(excluded)}`); + emitPatch(patch, io); +} + +/** Every changed path-scoped patch, shown; applying them is not this command's call. */ +function reportMultiPathDryRun( + runId: string, changed: readonly CloudPathPatch[], json: boolean, io: CliIo, +): void { + if (json) { + io.stdout(JSON.stringify({ + ok: true, runId, hasChanges: true, applied: false, dryRun: true, multiPath: true, + patches: changed.map(({ name, patch }) => { + const excluded = excludedPatchPaths(patch); + return { name, files: patchedPaths(patch).filter(path => !excluded.includes(path)), excluded, patch }; + }), + })); + return; + } + io.stdout(`DRY RUN ${runId}: ${changed.length} path-scoped patch${changed.length === 1 ? '' : 'es'}`); + io.stdout('Each targets its own repository; apply them there, not with --dir.'); + for (const { name, patch } of changed) { + io.stdout(`--- patch for path "${name}" ---`); + emitPatch(patch, io); + } +} + +/** Write a diff through the line-oriented io without gaining or losing a newline. */ +function emitPatch(patch: string, io: CliIo): void { + for (const line of patch.replace(/\n$/u, '').split('\n')) io.stdout(line); +} + +function indented(paths: readonly string[]): string { + return paths.length ? `\n ${paths.join('\n ')}` : ''; +} diff --git a/packages/sdk/src/cloud-sync.ts b/packages/sdk/src/cloud-sync.ts index f48425a91..1781f54f3 100644 --- a/packages/sdk/src/cloud-sync.ts +++ b/packages/sdk/src/cloud-sync.ts @@ -272,37 +272,188 @@ export interface CloudPatch { hasChanges: boolean; } -/** The sandbox's post-run diff. Multi-path runs carry several patches and are refused here. */ -export async function downloadCloudPatch(runId: string, options: CloudConnectionOptions): Promise { +/** One entry of a multi-path run's patch map: the mounted path's name and its diff. */ +export interface CloudPathPatch extends CloudPatch { + name: string; +} + +/** + * What `/patch` answered, in both shapes the endpoint can produce. + * + * A run that declared mounted `paths` gets one `changes-.patch` per path + * and the route answers `{ patches: { : { patch, hasChanges } } }`; every + * other run gets `changes.patch` and the flat `{ patch, hasChanges }`. The + * multi-path shape is not a v1 relic -- `paths` is orthogonal to + * `relayflowVersion`, so a v2 run that submits several paths returns it too. + */ +export type CloudPatchSet = + | { kind: 'single'; patch: string; hasChanges: boolean } + | { kind: 'multi-path'; patches: readonly CloudPathPatch[]; hasChanges: boolean }; + +/** + * The sandbox's post-run diff, in whichever shape the run produced. + * + * Both shapes are modelled rather than one refused at the transport, so a + * caller can show a multi-path run's patches (`flows sync --dry-run`) before + * deciding what to do with them. Applying them is still the caller's refusal + * to make: they target different repositories and no single tree is the right + * destination. + */ +export async function downloadCloudPatchSet( + runId: string, options: CloudConnectionOptions, +): Promise { const payload = await cloudRequest(`/api/v1/workflows/runs/${encodeURIComponent(cloudRunId(runId))}/patch`, options); if (!isCloudRecord(payload)) throw new CloudFlowError('invalid_response', 'Cloud patch response was not an object.'); if (isCloudRecord(payload.patches)) { - const names = Object.keys(payload.patches); - throw new CloudFlowError('sync_unsupported', - `Run ${runId} produced ${names.length} path-scoped patches (${names.join(', ')}); flows sync applies single-tree runs only.`); + const patches: CloudPathPatch[] = []; + for (const [name, entry] of Object.entries(payload.patches)) { + if (!isCloudRecord(entry) || typeof entry.patch !== 'string' || typeof entry.hasChanges !== 'boolean') { + throw new CloudFlowError('invalid_response', `Cloud patch response has an unusable entry for path "${name}".`); + } + patches.push({ name, patch: entry.patch, hasChanges: entry.hasChanges }); + } + return { kind: 'multi-path', patches, hasChanges: patches.some(entry => entry.hasChanges && entry.patch.trim() !== '') }; } if (typeof payload.patch !== 'string' || typeof payload.hasChanges !== 'boolean') { throw new CloudFlowError('invalid_response', 'Cloud patch response is missing patch or hasChanges.'); } - return { patch: payload.patch, hasChanges: payload.hasChanges }; + return { kind: 'single', patch: payload.patch, hasChanges: payload.hasChanges }; +} + +/** The sandbox's post-run diff. Multi-path runs carry several patches and are refused here. */ +export async function downloadCloudPatch(runId: string, options: CloudConnectionOptions): Promise { + const set = await downloadCloudPatchSet(runId, options); + if (set.kind === 'multi-path') { + const names = set.patches.map(entry => entry.name); + throw new CloudFlowError('sync_unsupported', + `Run ${runId} produced ${names.length} path-scoped patches (${names.join(', ')}); flows sync applies single-tree runs only.`); + } + return { patch: set.patch, hasChanges: set.hasChanges }; +} + +/** + * Paths a synced patch must never write, the single home for the list. + * + * These are the agent runtime's own bookkeeping inside a synced tree: helper + * binaries staged for the sandbox, the relayfile mount's ACL and state files + * (including the temporaries a mid-write state leaves behind), trajectory + * records and workflow context. The sandbox commits its baseline before the + * run, so every one of them shows up in the post-run diff as a creation or a + * modification -- applying that diff verbatim drags the run's own plumbing into + * the user's checkout, where at best it is noise in `git diff` and at worst it + * overwrites the mount state of the tree being synced into. + * + * `git apply --exclude` matches these with wildmatch, anchored at the patch + * root and with `*` stopping at a `/`: `.agent-bin/**` drops + * `.agent-bin/nested/tool` but deliberately not `packages/x/.agent-bin/tool`, + * which belongs to a different tree than the one being synced. + */ +export const CLOUD_SYNC_PATCH_EXCLUDES = [ + '.agent-bin/**', + '.relayfile.acl', + '.relayfile-mount-state.json', + '.relayfile-mount-state.json.tmp-*', + '.trajectories/**', + '.workflow-context/**', +] as const; + +/** The `a/` and `b/` sides of every `diff --git` header, in file order. */ +function patchHeaders(patch: string): { old: string; new: string }[] { + const headers: { old: string; new: string }[] = []; + for (const match of patch.matchAll(/^diff --git a\/(.+?) b\/(.+)$/gmu)) { + headers.push({ old: match[1]!, new: match[2]! }); + } + return headers; } /** Every path a unified diff touches, deletions included, in order of first appearance. */ export function patchedPaths(patch: string): string[] { const paths: string[] = []; - for (const match of patch.matchAll(/^diff --git a\/(.+?) b\/(.+)$/gmu)) { - for (const path of [match[1]!, match[2]!]) if (!paths.includes(path)) paths.push(path); + for (const header of patchHeaders(patch)) { + for (const path of [header.old, header.new]) if (!paths.includes(path)) paths.push(path); } return paths; } +/** + * `git apply --exclude`'s wildmatch, as a matcher over a patch's own paths. + * + * Anchored at the patch root, `**` crosses `/` and `*`/`?` do not -- the subset + * of wildmatch {@link CLOUD_SYNC_PATCH_EXCLUDES} uses. Kept honest by a test + * that runs the same patterns through `git apply --numstat` and requires the + * two answers to agree, so a divergence fails here rather than silently + * reporting a path as dropped that git actually wrote. + */ +function matchesExclude(path: string, pattern: string): boolean { + let expression = '^'; + for (let index = 0; index < pattern.length; index += 1) { + const character = pattern[index]!; + if (character === '*') { + if (pattern[index + 1] === '*') { expression += '.*'; index += 1; continue; } + expression += '[^/]*'; + continue; + } + expression += character === '?' ? '[^/]' : character.replace(/[.+^${}()|[\]\\]/gu, '\\$&'); + } + return new RegExp(`${expression}$`, 'u').test(path); +} + +/** + * The subset of a patch's paths `exclude` drops, in order of first appearance. + * + * Decided per `diff --git` header on its `b/` side, which is the name `git + * apply` itself tests -- so a rename is dropped or kept whole, never half. A + * deletion names the same path on both sides, so it is covered by the same rule. + */ +export function excludedPatchPaths( + patch: string, exclude: readonly string[] = CLOUD_SYNC_PATCH_EXCLUDES, +): string[] { + const paths: string[] = []; + for (const header of patchHeaders(patch)) { + if (!exclude.some(pattern => matchesExclude(header.new, pattern))) continue; + for (const path of [header.old, header.new]) if (!paths.includes(path)) paths.push(path); + } + return paths; +} + +/** Options for {@link applyCloudPatch}. */ +export interface ApplyCloudPatchOptions { + /** + * Path patterns to drop, defaulting to {@link CLOUD_SYNC_PATCH_EXCLUDES}. + * Pass `[]` to apply a patch whole -- including the runtime artifacts the + * default list exists to keep out of a working tree. + */ + exclude?: readonly string[]; +} + +/** What {@link applyCloudPatch} wrote, and what it dropped on the way. */ +export interface AppliedCloudPatch { + /** Paths the apply wrote, in order of first appearance in the patch. */ + files: string[]; + /** Paths `exclude` dropped, in order of first appearance in the patch. */ + excluded: string[]; +} + /** * `git apply --check` then `git apply`; a conflict leaves the tree untouched. * The patch lands in the working tree uncommitted, so what the run changed is - * reviewed with `git diff` before anything is kept — the same contract as v1. + * reviewed with `git diff` before anything is kept -- the same contract as v1. + * + * Both invocations carry the identical `--exclude` arguments. A check run + * without them is a different question than the apply answers: it can pass on + * an excluded hunk that the apply then never writes, or fail on one and refuse + * a patch whose applied part was clean. The exclusions are a property of the + * patch that lands, so they belong to both halves or neither. + * + * A patch whose every path is excluded is a no-op, not a failure: `git apply` + * exits 0 having written nothing, and the returned `files` is empty. */ -export function applyCloudPatch(root: string, patch: string): void { - const args = ['-C', resolve(root), 'apply', '--whitespace=nowarn']; +export function applyCloudPatch( + root: string, patch: string, options: ApplyCloudPatchOptions = {}, +): AppliedCloudPatch { + const exclude = options.exclude ?? CLOUD_SYNC_PATCH_EXCLUDES; + const args = ['-C', resolve(root), 'apply', '--whitespace=nowarn', + ...exclude.map(pattern => `--exclude=${pattern}`)]; const check = spawnSync('git', [...args, '--check'], { input: patch, encoding: 'utf8' }); if (check.status !== 0) { throw new CloudFlowError('patch_conflict', @@ -312,4 +463,6 @@ export function applyCloudPatch(root: string, patch: string): void { if (apply.status !== 0) { throw new CloudFlowError('patch_conflict', `git apply failed:\n${apply.stderr.trim()}`); } + const excluded = excludedPatchPaths(patch, exclude); + return { files: patchedPaths(patch).filter(path => !excluded.includes(path)), excluded }; } diff --git a/packages/sdk/src/index.ts b/packages/sdk/src/index.ts index c36942182..b6b6d702c 100644 --- a/packages/sdk/src/index.ts +++ b/packages/sdk/src/index.ts @@ -64,8 +64,10 @@ export { type CloudFlowSource, type RunInCloudOptions, type CloudRunReceipt, type CloudRunState, } from './cloud-run.js'; export { - downloadCloudPatch, applyCloudPatch, packWorkingTree, patchedPaths, MAX_SYNC_BYTES, - type CloudPatch, type PackedTree, + downloadCloudPatch, downloadCloudPatchSet, applyCloudPatch, packWorkingTree, patchedPaths, + excludedPatchPaths, CLOUD_SYNC_PATCH_EXCLUDES, MAX_SYNC_BYTES, + type CloudPatch, type CloudPathPatch, type CloudPatchSet, type PackedTree, + type ApplyCloudPatchOptions, type AppliedCloudPatch, } from './cloud-sync.js'; export { deployToCloud, listCloudDeployments, undeployFromCloud, parseRepository, parseTriggerSource, FLOW_TRIGGER_PROVIDERS, diff --git a/packages/sdk/tests/cloud-sync.test.ts b/packages/sdk/tests/cloud-sync.test.ts index 54c25ecdf..df55160dd 100644 --- a/packages/sdk/tests/cloud-sync.test.ts +++ b/packages/sdk/tests/cloud-sync.test.ts @@ -8,7 +8,10 @@ import { gunzipSync } from 'node:zlib'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { runCli } from '../src/cli.js'; import { runInCloud } from '../src/cloud-run.js'; -import { applyCloudPatch, packWorkingTree, patchedPaths, prepareCloudSync } from '../src/cloud-sync.js'; +import { + CLOUD_SYNC_PATCH_EXCLUDES, applyCloudPatch, excludedPatchPaths, packWorkingTree, patchedPaths, + prepareCloudSync, +} from '../src/cloud-sync.js'; const dirs: string[] = []; afterEach(async () => { @@ -316,6 +319,7 @@ describe('flows run --cloud --sync-code / flows sync', () => { ['run', '--cloud', '--sync-code', '--sync-code', 'flow.yaml'], ['run', '--cloud', '--input', '{}', 'flow.yaml'], ['sync'], ['sync', 'a', 'b'], ['sync', '--dir'], ['sync', '--json', '--json', 'r'], + ['sync', '--dry-run', '--dry-run', 'r'], ['sync', '--dry', 'r'], ])('refuses argv %j before any request', async (...args) => { const fetch = vi.spyOn(globalThis, 'fetch'); expect(await runCli(args, { stdout: () => {}, stderr: () => {} })).toBe(2); @@ -337,7 +341,8 @@ describe('flows run --cloud --sync-code / flows sync', () => { cloud({ '/api/v1/workflows/runs/done-run/patch': () => ({ patch, hasChanges: true }) }); const output: string[] = []; expect(await runCli(['sync', '--json', '--dir', root, 'done-run'], { stdout: line => output.push(line), stderr: line => output.push(line) })).toBe(0); - expect(JSON.parse(output[0]!)).toEqual({ ok: true, runId: 'done-run', hasChanges: true, applied: true, files: ['a.txt', 'b.txt', 'gone.txt'] }); + expect(JSON.parse(output[0]!)).toEqual({ ok: true, runId: 'done-run', hasChanges: true, applied: true, + files: ['a.txt', 'b.txt', 'gone.txt'], excluded: [] }); expect(await readFile(join(root, 'a.txt'), 'utf8')).toBe('one\ntwo\n'); expect(await readFile(join(root, 'b.txt'), 'utf8')).toBe('fresh\n'); await expect(stat(join(root, 'gone.txt'))).rejects.toMatchObject({ code: 'ENOENT' }); @@ -387,3 +392,313 @@ describe('flows run --cloud --sync-code / flows sync', () => { expect(await readFile(join(root, 'a.txt'), 'utf8')).toBe('one\n'); }); }); + +/** + * The patch a real synced run produces: source changes the user wants, and the + * agent runtime's own bookkeeping, which they do not. Every excluded pattern + * appears, plus the two near misses that must survive -- a nested + * `.agent-bin` belonging to a different tree, and a state file whose name only + * resembles the temporaries. + */ +const RUNTIME_ARTEFACT_PATCH = [ + 'diff --git a/src/keep.ts b/src/keep.ts', + '--- a/src/keep.ts', + '+++ b/src/keep.ts', + '@@ -1 +1 @@', + '-export const kept = 1;', + '+export const kept = 2;', + 'diff --git a/.trajectories/run.jsonl b/.trajectories/run.jsonl', + 'new file mode 100644', + '--- /dev/null', + '+++ b/.trajectories/run.jsonl', + '@@ -0,0 +1 @@', + '+{"step":"leaked"}', + 'diff --git a/.agent-bin/nested/helper b/.agent-bin/nested/helper', + 'new file mode 100644', + '--- /dev/null', + '+++ b/.agent-bin/nested/helper', + '@@ -0,0 +1 @@', + '+#!/bin/sh', + 'diff --git a/.relayfile.acl b/.relayfile.acl', + '--- a/.relayfile.acl', + '+++ b/.relayfile.acl', + '@@ -1 +1 @@', + '-allow: nobody', + '+allow: everybody', + 'diff --git a/.relayfile-mount-state.json b/.relayfile-mount-state.json', + '--- a/.relayfile-mount-state.json', + '+++ b/.relayfile-mount-state.json', + '@@ -1 +1 @@', + '-{"mounted":false}', + '+{"mounted":true}', + 'diff --git a/.relayfile-mount-state.json.tmp-9911 b/.relayfile-mount-state.json.tmp-9911', + 'new file mode 100644', + '--- /dev/null', + '+++ b/.relayfile-mount-state.json.tmp-9911', + '@@ -0,0 +1 @@', + '+half-written', + 'diff --git a/.workflow-context/ctx.json b/.workflow-context/ctx.json', + 'new file mode 100644', + '--- /dev/null', + '+++ b/.workflow-context/ctx.json', + '@@ -0,0 +1 @@', + '+{"run":"x"}', + 'diff --git a/packages/x/.agent-bin/tool b/packages/x/.agent-bin/tool', + '--- a/packages/x/.agent-bin/tool', + '+++ b/packages/x/.agent-bin/tool', + '@@ -1 +1 @@', + '-vendored', + '+vendored twice', + 'diff --git a/.relayfile-mount-state.json.backup b/.relayfile-mount-state.json.backup', + '--- a/.relayfile-mount-state.json.backup', + '+++ b/.relayfile-mount-state.json.backup', + '@@ -1 +1 @@', + '-old', + '+new', + '', +].join('\n'); + +/** The tree that patch was cut against, committed so `git apply` has its baseline. */ +async function artefactCheckout(): Promise { + const root = await tempDir('cloud-sync-exclude-'); + git(root, 'init', '-q'); + await mkdir(join(root, 'src')); + await mkdir(join(root, 'packages/x/.agent-bin'), { recursive: true }); + await writeFile(join(root, 'src/keep.ts'), 'export const kept = 1;\n'); + await writeFile(join(root, '.relayfile.acl'), 'allow: nobody\n'); + await writeFile(join(root, '.relayfile-mount-state.json'), '{"mounted":false}\n'); + await writeFile(join(root, '.relayfile-mount-state.json.backup'), 'old\n'); + await writeFile(join(root, 'packages/x/.agent-bin/tool'), 'vendored\n'); + git(root, 'add', '-A'); + git(root, 'commit', '-q', '-m', 'baseline'); + return root; +} + +const KEPT_PATHS = [ + 'src/keep.ts', 'packages/x/.agent-bin/tool', '.relayfile-mount-state.json.backup', +]; +const DROPPED_PATHS = [ + '.trajectories/run.jsonl', '.agent-bin/nested/helper', '.relayfile.acl', + '.relayfile-mount-state.json', '.relayfile-mount-state.json.tmp-9911', '.workflow-context/ctx.json', +]; + +describe('applyCloudPatch path exclusions', () => { + it('writes the run’s source changes and never the agent runtime artefacts it touched', async () => { + const root = await artefactCheckout(); + + const { files, excluded } = applyCloudPatch(root, RUNTIME_ARTEFACT_PATCH); + + expect(files).toEqual(KEPT_PATHS); + expect(excluded).toEqual(DROPPED_PATHS); + // The point of the whole exercise: the files are not on disk. + expect(await readFile(join(root, 'src/keep.ts'), 'utf8')).toBe('export const kept = 2;\n'); + expect(await readFile(join(root, 'packages/x/.agent-bin/tool'), 'utf8')).toBe('vendored twice\n'); + expect(await readFile(join(root, '.relayfile-mount-state.json.backup'), 'utf8')).toBe('new\n'); + await expect(stat(join(root, '.trajectories'))).rejects.toMatchObject({ code: 'ENOENT' }); + await expect(stat(join(root, '.agent-bin'))).rejects.toMatchObject({ code: 'ENOENT' }); + await expect(stat(join(root, '.workflow-context'))).rejects.toMatchObject({ code: 'ENOENT' }); + await expect(stat(join(root, '.relayfile-mount-state.json.tmp-9911'))).rejects.toMatchObject({ code: 'ENOENT' }); + // Modifications to tracked runtime files are dropped too, not just creations. + expect(await readFile(join(root, '.relayfile.acl'), 'utf8')).toBe('allow: nobody\n'); + expect(await readFile(join(root, '.relayfile-mount-state.json'), 'utf8')).toBe('{"mounted":false}\n'); + }); + + it('reports exactly the paths git itself drops for the same patterns', async () => { + const root = await artefactCheckout(); + + // `git apply --numstat` lists what an apply with these excludes would + // write. The SDK's own matcher has to agree with it, or every `files` and + // `excluded` it reports is fiction. + const numstat = execFileSync('git', [ + '-C', root, 'apply', '--numstat', + ...CLOUD_SYNC_PATCH_EXCLUDES.map(pattern => `--exclude=${pattern}`), + ], { input: RUNTIME_ARTEFACT_PATCH, encoding: 'utf8' }); + const gitWould = numstat.trim().split('\n').filter(Boolean).map(line => line.split('\t')[2]!); + + expect(gitWould.sort()).toEqual([...KEPT_PATHS].sort()); + expect(excludedPatchPaths(RUNTIME_ARTEFACT_PATCH).sort()).toEqual([...DROPPED_PATHS].sort()); + }); + + it('checks with the same excludes it applies with, so an excluded conflict is not a refusal', async () => { + const root = await artefactCheckout(); + // The excluded hunk's context does not match the tree at all. With the + // excludes on `--check` this is irrelevant -- the hunk is never applied. + // Without them, `--check` fails and the clean source change is lost. + const patch = 'diff --git a/src/keep.ts b/src/keep.ts\n--- a/src/keep.ts\n+++ b/src/keep.ts\n' + + '@@ -1 +1 @@\n-export const kept = 1;\n+export const kept = 3;\n' + + 'diff --git a/.trajectories/run.jsonl b/.trajectories/run.jsonl\n' + + '--- a/.trajectories/run.jsonl\n+++ b/.trajectories/run.jsonl\n' + + '@@ -1 +1 @@\n-a line this tree has never held\n+replaced\n'; + + expect(applyCloudPatch(root, patch)).toEqual({ files: ['src/keep.ts'], excluded: ['.trajectories/run.jsonl'] }); + expect(await readFile(join(root, 'src/keep.ts'), 'utf8')).toBe('export const kept = 3;\n'); + }); + + it('applies the patch whole when the caller opts out, and is a clean no-op when everything is excluded', async () => { + const root = await artefactCheckout(); + + const applied = applyCloudPatch(root, RUNTIME_ARTEFACT_PATCH, { exclude: [] }); + expect(applied.excluded).toEqual([]); + expect(applied.files).toEqual([...KEPT_PATHS.slice(0, 1), ...DROPPED_PATHS, ...KEPT_PATHS.slice(1)]); + expect(await readFile(join(root, '.trajectories/run.jsonl'), 'utf8')).toBe('{"step":"leaked"}\n'); + + // A patch that touches nothing but excluded paths is not a conflict. + const only = await artefactCheckout(); + const runtimeOnly = 'diff --git a/.trajectories/run.jsonl b/.trajectories/run.jsonl\nnew file mode 100644\n' + + '--- /dev/null\n+++ b/.trajectories/run.jsonl\n@@ -0,0 +1 @@\n+{"step":"leaked"}\n'; + expect(applyCloudPatch(only, runtimeOnly)).toEqual({ files: [], excluded: ['.trajectories/run.jsonl'] }); + await expect(stat(join(only, '.trajectories'))).rejects.toMatchObject({ code: 'ENOENT' }); + }); +}); + +describe('flows sync --dry-run', () => { + it('prints the patch, names what it would skip, and writes nothing', async () => { + const root = await artefactCheckout(); + cloud({ '/api/v1/workflows/runs/dry/patch': () => ({ patch: RUNTIME_ARTEFACT_PATCH, hasChanges: true }) }); + const output: string[] = []; + + const code = await runCli(['sync', '--dry-run', '--dir', root, 'dry'], + { stdout: line => output.push(line), stderr: line => output.push(line) }); + + expect(code, output.join('\n')).toBe(0); + const text = output.join('\n'); + expect(output[0]).toMatch(/^DRY RUN dry: 3 files would be written\n {2}src\/keep\.ts\n/u); + expect(text).toContain('src/keep.ts'); + expect(text).toContain('Would skip agent runtime paths: 6'); + // The whole diff is on stdout, verbatim enough to pipe into `git apply`. + expect(text).toContain('diff --git a/src/keep.ts b/src/keep.ts'); + expect(text).toContain('+export const kept = 2;'); + // And the tree is untouched. + expect(await readFile(join(root, 'src/keep.ts'), 'utf8')).toBe('export const kept = 1;\n'); + await expect(stat(join(root, '.trajectories'))).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('carries the patch in the JSON payload rather than beside it', async () => { + const root = await artefactCheckout(); + cloud({ '/api/v1/workflows/runs/dry/patch': () => ({ patch: RUNTIME_ARTEFACT_PATCH, hasChanges: true }) }); + const output: string[] = []; + + expect(await runCli(['sync', '--dry-run', '--json', '--dir', root, 'dry'], + { stdout: line => output.push(line), stderr: line => output.push(line) })).toBe(0); + + // Exactly one line, and it parses: the diff is a field, not loose stdout. + expect(output).toHaveLength(1); + expect(JSON.parse(output[0]!)).toEqual({ + ok: true, runId: 'dry', hasChanges: true, applied: false, dryRun: true, + files: KEPT_PATHS, excluded: DROPPED_PATHS, patch: RUNTIME_ARTEFACT_PATCH, + }); + expect(await readFile(join(root, 'src/keep.ts'), 'utf8')).toBe('export const kept = 1;\n'); + }); + + it('shows every path-scoped patch of a multi-path run without applying any of them', async () => { + const root = await artefactCheckout(); + const api = 'diff --git a/api/handler.ts b/api/handler.ts\n--- a/api/handler.ts\n+++ b/api/handler.ts\n' + + '@@ -1 +1 @@\n-old\n+new\n'; + const web = 'diff --git a/web/page.tsx b/web/page.tsx\n--- a/web/page.tsx\n+++ b/web/page.tsx\n' + + '@@ -1 +1 @@\n-old\n+new\n'; + cloud({ '/api/v1/workflows/runs/multi/patch': () => ({ patches: { + api: { patch: api, hasChanges: true }, + web: { patch: web, hasChanges: true }, + docs: { patch: '', hasChanges: false }, + } }) }); + const output: string[] = []; + + expect(await runCli(['sync', '--dry-run', '--dir', root, 'multi'], + { stdout: line => output.push(line), stderr: line => output.push(line) })).toBe(0); + + const text = output.join('\n'); + expect(output[0]).toBe('DRY RUN multi: 2 path-scoped patches'); + expect(text).toContain('--- patch for path "api" ---'); + expect(text).toContain('--- patch for path "web" ---'); + // The path with no changes is not presented as something to apply. + expect(text).not.toContain('"docs"'); + expect(text).toContain('+++ b/api/handler.ts'); + expect(text).toContain('+++ b/web/page.tsx'); + await expect(stat(join(root, 'api'))).rejects.toMatchObject({ code: 'ENOENT' }); + await expect(stat(join(root, 'web'))).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('keys a multi-path --json dry run by path name', async () => { + const root = await artefactCheckout(); + const api = 'diff --git a/api/handler.ts b/api/handler.ts\n--- a/api/handler.ts\n+++ b/api/handler.ts\n' + + '@@ -1 +1 @@\n-old\n+new\n'; + cloud({ '/api/v1/workflows/runs/multi/patch': () => ({ patches: { api: { patch: api, hasChanges: true } } }) }); + const output: string[] = []; + + expect(await runCli(['sync', '--dry-run', '--json', '--dir', root, 'multi'], + { stdout: line => output.push(line), stderr: () => {} })).toBe(0); + + expect(JSON.parse(output[0]!)).toEqual({ + ok: true, runId: 'multi', hasChanges: true, applied: false, dryRun: true, multiPath: true, + patches: [{ name: 'api', files: ['api/handler.ts'], excluded: [], patch: api }], + }); + }); + + it('refuses to apply a multi-path run and points at --dry-run', async () => { + const root = await artefactCheckout(); + cloud({ '/api/v1/workflows/runs/multi/patch': () => ({ patches: { + api: { patch: 'diff --git a/api/x b/api/x\n', hasChanges: true }, + web: { patch: 'diff --git a/web/y b/web/y\n', hasChanges: true }, + } }) }); + const errors: string[] = []; + + expect(await runCli(['sync', '--dir', root, 'multi'], + { stdout: () => {}, stderr: line => errors.push(line) })).toBe(2); + + expect(errors[0]).toContain('sync_unsupported'); + expect(errors[0]).toContain('2 path-scoped patches (api, web)'); + expect(errors[0]).toContain('--dry-run'); + }); + + it('reports no changes for a multi-path run whose every path is quiet', async () => { + const root = await artefactCheckout(); + cloud({ '/api/v1/workflows/runs/quiet/patch': () => ({ patches: { + api: { patch: '', hasChanges: false }, web: { patch: '', hasChanges: false }, + } }) }); + const output: string[] = []; + + expect(await runCli(['sync', '--dir', root, 'quiet'], + { stdout: line => output.push(line), stderr: line => output.push(line) })).toBe(0); + expect(output).toEqual(['NO CHANGES quiet']); + }); + + it('refuses a patch map whose entry is not a patch', async () => { + cloud({ '/api/v1/workflows/runs/bad/patch': () => ({ patches: { api: { patch: 7, hasChanges: true } } }) }); + const errors: string[] = []; + + expect(await runCli(['sync', 'bad'], { stdout: () => {}, stderr: line => errors.push(line) })).toBe(1); + expect(errors[0]).toContain('invalid_response'); + expect(errors[0]).toContain('"api"'); + }); +}); + +describe('flows sync reporting', () => { + it('names the runtime artefacts it dropped alongside what it applied', async () => { + const root = await artefactCheckout(); + cloud({ '/api/v1/workflows/runs/real/patch': () => ({ patch: RUNTIME_ARTEFACT_PATCH, hasChanges: true }) }); + const output: string[] = []; + + expect(await runCli(['sync', '--json', '--dir', root, 'real'], + { stdout: line => output.push(line), stderr: line => output.push(line) })).toBe(0); + + expect(JSON.parse(output[0]!)).toEqual({ + ok: true, runId: 'real', hasChanges: true, applied: true, + files: KEPT_PATHS, excluded: DROPPED_PATHS, + }); + await expect(stat(join(root, '.trajectories'))).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('says so on the human path too', async () => { + const root = await artefactCheckout(); + cloud({ '/api/v1/workflows/runs/real/patch': () => ({ patch: RUNTIME_ARTEFACT_PATCH, hasChanges: true }) }); + const output: string[] = []; + + expect(await runCli(['sync', '--dir', root, 'real'], + { stdout: line => output.push(line), stderr: line => output.push(line) })).toBe(0); + + expect(output[0]).toMatch(/^APPLIED real: 3 files\n {2}src\/keep\.ts\n/u); + expect(output.join('\n')).toContain('SKIPPED agent runtime paths: 6'); + expect(output.join('\n')).toContain('.trajectories/run.jsonl'); + expect(output.at(-1)).toContain('review with git diff'); + }); +}); diff --git a/packages/sdk/tests/relay-cli-surface.test.ts b/packages/sdk/tests/relay-cli-surface.test.ts index 621cf7e57..cda017138 100644 --- a/packages/sdk/tests/relay-cli-surface.test.ts +++ b/packages/sdk/tests/relay-cli-surface.test.ts @@ -76,6 +76,7 @@ const INVOCATIONS: readonly { verb: string; argv: readonly string[]; variant: Pa variant: 'serve-webhook', }, { verb: 'sync', argv: ['sync', RUN_ID], variant: 'sync' }, + { verb: 'sync', argv: ['sync', '--dry-run', '--json', RUN_ID], variant: 'sync' }, { verb: 'tick', argv: ['tick', 'start', '--schedule-id', 'nightly', '--interval-ms', '60000', 'spec.json'], @@ -157,6 +158,51 @@ describe('relay-cli surface: drift between `commands` and `run`', () => { expect(reached).toEqual(claimed); }); + it('declares no flag the parser refuses everywhere', () => { + // The other half of command drift: a verb can be declared correctly and + // still advertise a switch nothing accepts. Every declared boolean option + // has to be accepted by at least one of that verb's sample invocations -- + // "at least one", because some flags are only valid in combination + // (`run --wait` needs `--cloud`, `deploy --draft` only the hosted form). + // Value-taking options are left out: their placeholder is not an argv token. + const declaredFlags = (CLI_VERBS as readonly CliVerbSpec[]).flatMap((verb) => [ + ...(verb.options ?? []).map((option) => option.flags), + ...(verb.subcommands ?? []).flatMap((sub) => (sub.options ?? []).map((option) => option.flags)), + ].filter((flags) => !flags.includes('<')).map((flags) => ({ verb: verb.name, flags }))); + + // A real table always has some; an empty list would make this test vacuous. + expect(declaredFlags.length).toBeGreaterThan(0); + + for (const { verb, flags } of declaredFlags) { + const samples = INVOCATIONS.filter((invocation) => invocation.verb === verb); + expect(samples.length, `${verb} has a sample invocation`).toBeGreaterThan(0); + + const accepted = samples.some(({ argv }) => { + // A sample that already carries the flag has answered the question; + // adding it a second time is a duplicate, which every verb refuses. + if (argv.includes(flags)) return parseCliArgs(argv) !== undefined; + // Otherwise: after the verb, and after its subcommand token where there is one. + const at = (CLI_VERBS as readonly CliVerbSpec[]) + .find((entry) => entry.name === verb)!.subcommands?.length ? 2 : 1; + return parseCliArgs([...argv.slice(0, at), flags, ...argv.slice(at)]) !== undefined; + }); + + expect(accepted, `\`flows ${verb} ${flags}\` is accepted by the parser`).toBe(true); + } + }); + + it('refuses a flag it does not declare', () => { + // And the converse, spot-checked where the table is most likely to go + // stale: an undeclared switch is a parse failure, not a silent no-op. + const declared = new Set( + (CLI_VERBS.find((verb) => verb.name === 'sync')!.options ?? []).map((option) => option.flags), + ); + + expect(declared).toEqual(new Set(['--json', '--dry-run', '--dir '])); + expect(parseCliArgs(['sync', '--dry', RUN_ID])).toBeUndefined(); + expect(parseCliArgs(['sync', '--exclude', '.trajectories/**', RUN_ID])).toBeUndefined(); + }); + it('declares each subcommand under the name the parser actually requires', () => { // `hn-monitor` and `tick` route on a second token. The tree must name that // token exactly, or the host's help sends users to an invocation that From 4b3802e07fb99f6a84cb87ba7bbb56de4e8b1565 Mon Sep 17 00:00:00 2001 From: agentrelaybot Date: Thu, 17 Sep 2026 16:01:58 -0700 Subject: [PATCH 3/8] chore(sdk): declare the @agent-relay/cli-surface devDependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tests/relay-cli-surface.test.ts` imports the contract package to assert the surface satisfies it, but nothing declared the dependency — it resolved only where a sibling checkout happened to provide it, and CI could not load the test. Now pinned to the published 12.2.2. devDependency only: `src/relay-cli.ts` remains structurally typed, so the published package still has no runtime dependency on relay. Co-Authored-By: Claude Opus 5 (1M context) Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916 --- packages/sdk/package-lock.json | 10 ++++++++++ packages/sdk/package.json | 1 + 2 files changed, 11 insertions(+) diff --git a/packages/sdk/package-lock.json b/packages/sdk/package-lock.json index c7a98cb8e..af4e19b52 100644 --- a/packages/sdk/package-lock.json +++ b/packages/sdk/package-lock.json @@ -25,11 +25,21 @@ "flows": "dist/cli.js" }, "devDependencies": { + "@agent-relay/cli-surface": "^12.2.2", "@types/node": "^22.7.0", "typescript": "^5.6.0", "vitest": "^2.1.0" } }, + "node_modules/@agent-relay/cli-surface": { + "version": "12.2.2", + "resolved": "https://registry.npmjs.org/@agent-relay/cli-surface/-/cli-surface-12.2.2.tgz", + "integrity": "sha512-xOIx+a3AjFn3P2MZp1GwqwVS7qZiBKu4uMXV+mK/gb0RsH//PD6zcG+855DdF/Zep/yVv9JSEcyfWl/wh0eJhw==", + "dev": true, + "engines": { + "node": ">=22.0.0" + } + }, "node_modules/@esbuild/aix-ppc64": { "version": "0.21.5", "resolved": "https://registry.npmjs.org/@esbuild/aix-ppc64/-/aix-ppc64-0.21.5.tgz", diff --git a/packages/sdk/package.json b/packages/sdk/package.json index 333453c15..0a31a005b 100644 --- a/packages/sdk/package.json +++ b/packages/sdk/package.json @@ -59,6 +59,7 @@ "yaml": "^2.5.1" }, "devDependencies": { + "@agent-relay/cli-surface": "^12.2.2", "@types/node": "^22.7.0", "typescript": "^5.6.0", "vitest": "^2.1.0" From 7b610ea899110ad24681c746e5a89962b1fd3154 Mon Sep 17 00:00:00 2001 From: agentrelaybot Date: Thu, 17 Sep 2026 16:44:57 -0700 Subject: [PATCH 4/8] fix(sdk): make the flows parsers accept what the command surface advertises Four behavioural findings from the #451 review. Three are one class: the declared spec advertises flags the parser refuses, and `agent-relay` renders help FROM that spec, so each one is a user following help into an exit 2. In every case the parser was the wrong half -- the combination help promises is the better UX -- so the parser moved and the spec only narrowed where a combination genuinely describes nothing. serve-webhook. `DATA_DIR_OPTION` carries `.relayflowd` as its default and every other verb honours it; `parseWebhookArgs` alone required the flag. It now defaults like the rest. `--port` stays required and the spec says so. build. `--verify` was a leading mode token accepting exactly one positional and never `--json`, so `build --verify` and `build --verify --json ` -- both implied by a spec that lists the three as peers -- refused. It is now an ordinary flag. `--out` with `--verify` is the one pair still refused (a verify writes nothing, so a destination would be silently ignored) and the option's description says so rather than leaving the reader to find out at exit 2. deploy. `--json` is declared on the verb both forms share; only the hosted listener parser accepted it. The bundle form accepts it now and emits one object per outcome, refusals included. cloud sync. `runCloudSyncCli` took a single-tree result's `hasChanges` at face value. The multi-path branch discounts an empty body and so did `agent-relay cloud sync`, which this command replaces; without the guard an empty patch reached `git apply` and failed as `patch_conflict` instead of NO CHANGES. The drift test grows three assertions so the class cannot recur: every declared option -- value-taking ones included -- must be carried by an invocation that parses, every option declaring a default must be omittable from one, and every command must parse exactly its declared positional arity with each required positional actually required. Reverting any of the four fixes fails a test. Co-Authored-By: Claude Opus 5 (1M context) Session-Id: 0156272a-dc1d-44ab-99cf-720666eb754e --- packages/sdk/src/cli-commands.ts | 4 +- packages/sdk/src/cli/build.ts | 26 ++- packages/sdk/src/cli/cloud-sync.ts | 8 +- packages/sdk/src/cli/deploy.ts | 32 +++- packages/sdk/src/cli/serve-webhook.ts | 8 +- packages/sdk/tests/bundle.test.ts | 23 +++ packages/sdk/tests/cloud-sync.test.ts | 30 ++++ packages/sdk/tests/deploy.test.ts | 27 ++- packages/sdk/tests/relay-cli-surface.test.ts | 171 ++++++++++++++++++- packages/sdk/tests/webhook.test.ts | 7 +- 10 files changed, 314 insertions(+), 22 deletions(-) diff --git a/packages/sdk/src/cli-commands.ts b/packages/sdk/src/cli-commands.ts index 0cb9713de..b1594e4ee 100644 --- a/packages/sdk/src/cli-commands.ts +++ b/packages/sdk/src/cli-commands.ts @@ -102,7 +102,7 @@ export const CLI_VERBS = [ description: 'Compile a flow into a sealed, content-addressed bundle', args: [{ name: 'source', description: 'flow.yaml, flow.ts, or a bundle directory with --verify', required: true }], options: [ - { flags: '--out ', description: 'Directory to write the bundle into' }, + { flags: '--out ', description: 'Directory to write the bundle into; not valid with --verify' }, { flags: '--verify', description: 'Verify an existing bundle directory instead of building' }, JSON_OPTION, ], @@ -204,7 +204,7 @@ export const CLI_VERBS = [ description: 'Run the local webhook receiver that writes provider deliveries into the trigger inbox', options: [ DATA_DIR_OPTION, - { flags: '--port ', description: 'Port to listen on, bound to 127.0.0.1' }, + { flags: '--port ', description: 'Port to listen on, bound to 127.0.0.1; required' }, { flags: '--allow ', description: 'Comma-separated flow names this receiver admits' }, ], variants: ['serve-webhook'], diff --git a/packages/sdk/src/cli/build.ts b/packages/sdk/src/cli/build.ts index 7a3700434..bc1aae1d4 100644 --- a/packages/sdk/src/cli/build.ts +++ b/packages/sdk/src/cli/build.ts @@ -12,14 +12,20 @@ import type { FlowSpec } from '../spec.js'; export interface BuildArgs { command: 'build'; value: string; out?: string; verify: boolean; json: boolean } +/** + * `--verify` is an ordinary flag, not a leading mode token: the command surface + * lists it beside `--out` and `--json`, so every order the help implies has to + * parse (`build --verify --json ` and `build --verify` alike). + * + * `--out` is the one combination genuinely refused: it names where a build + * writes, and a verify builds nothing, so accepting the pair would silently + * ignore the destination. The surface says so in the option's description. + */ export function parseBuildArgs(args: readonly string[]): BuildArgs | undefined { - if (args[0] === '--verify') { - return args.length === 2 && !args[1]!.startsWith('-') - ? { command: 'build', value: args[1]!, verify: true, json: false } : undefined; - } let out: string | undefined; let value: string | undefined; let json = false; + let verify = false; for (let i = 0; i < args.length; i++) { const arg = args[i]!; if (arg === '--out') { @@ -28,10 +34,14 @@ export function parseBuildArgs(args: readonly string[]): BuildArgs | undefined { } else if (arg === '--json') { if (json) return undefined; json = true; + } else if (arg === '--verify') { + if (verify) return undefined; + verify = true; } else if (arg.startsWith('-') || value !== undefined) return undefined; else value = arg; } - return value === undefined ? undefined : { command: 'build', value, out, verify: false, json }; + if (value === undefined || (verify && out !== undefined)) return undefined; + return { command: 'build', value, out, verify, json }; } /** @@ -49,7 +59,11 @@ function emitBuildCheckReport(report: CheckReport, json: boolean, io: CliIo): vo export async function runBuild(args: BuildArgs, io: CliIo): Promise<0 | 2> { try { if (args.verify) { - io.stdout(`VERIFIED sha256:${await verifyBundle(args.value)}`); + const digest = `sha256:${await verifyBundle(args.value)}`; + // `--json` is declared on the verb, not on one of its forms: a verify + // under it emits the same single object a `--json` consumer parses. + if (args.json) io.stdout(JSON.stringify({ ok: true, verified: true, bundle: args.value, digest })); + else io.stdout(`VERIFIED ${digest}`); return 0; } // Gate the build on the same preflight pipeline `flows check` uses. diff --git a/packages/sdk/src/cli/cloud-sync.ts b/packages/sdk/src/cli/cloud-sync.ts index b7b5c5616..03246305d 100644 --- a/packages/sdk/src/cli/cloud-sync.ts +++ b/packages/sdk/src/cli/cloud-sync.ts @@ -22,7 +22,13 @@ export async function runCloudSyncCli( ): Promise<0 | 1 | 2> { try { const set = await downloadCloudPatchSet(runId, {}); - if (!set.hasChanges) { + // A single-tree run can answer `hasChanges: true` with an empty body. The + // multi-path branch already discounts those (`patch.trim() !== ''` in + // `downloadCloudPatchSet`), and so did `agent-relay cloud sync`, which this + // command replaces. Without the same guard an empty patch reaches `git + // apply`, which fails as `patch_conflict` instead of reporting NO CHANGES. + const hasChanges = set.kind === 'single' ? set.hasChanges && set.patch.trim() !== '' : set.hasChanges; + if (!hasChanges) { if (json) io.stdout(JSON.stringify({ ok: true, runId, hasChanges: false, applied: false })); else io.stdout(`NO CHANGES ${runId}`); return 0; diff --git a/packages/sdk/src/cli/deploy.ts b/packages/sdk/src/cli/deploy.ts index f72dff0a0..0af2c0505 100644 --- a/packages/sdk/src/cli/deploy.ts +++ b/packages/sdk/src/cli/deploy.ts @@ -4,19 +4,35 @@ import { BundleFailure, bucketDirectory, copyBundle, exists, parseDigestReferenc verifyDigest, writableBucket } from '../bundle-transport.js'; import { checkRunnableBundle } from './bundle-preflight.js'; -export interface DeployArgs { command: 'deploy'; value: string; to: string } +export interface DeployArgs { command: 'deploy'; value: string; to: string; json: boolean } + +/** + * `--json` is declared on the `deploy` verb, which both forms share, so the + * bundle form has to accept it too -- the hosted-listener parser already did, + * and help cannot tell a reader the flag belongs to only one of them. + */ export function parseDeployArgs(args: readonly string[]): DeployArgs | undefined { let value: string | undefined; let to: string | undefined; + let json = false; for (let i = 0; i < args.length; i++) { const arg = args[i]!; if (arg === '--to') { if (to !== undefined || !args[i + 1] || args[i + 1]!.startsWith('-')) return undefined; to = args[++i]; + } else if (arg === '--json') { + if (json) return undefined; + json = true; } else if (arg.startsWith('-') || value !== undefined) return undefined; else value = arg; } - return value && to && parseDigestReference(value) ? { command: 'deploy', value, to } : undefined; + return value && to && parseDigestReference(value) ? { command: 'deploy', value, to, json } : undefined; +} + +/** One machine-readable object per outcome, or the text line it mirrors. */ +function emitDeployOutcome(status: 'deployed' | 'already-present', args: DeployArgs, io: CliIo): void { + if (args.json) io.stdout(JSON.stringify({ ok: true, bundle: args.value, to: args.to, status })); + else io.stdout(`${status === 'deployed' ? 'DEPLOYED' : 'SKIPPED (already-present)'} ${args.value} ${args.to}`); } export async function runDeploy(args: DeployArgs, io: CliIo): Promise<0 | 1 | 2> { let started = false; @@ -29,6 +45,10 @@ export async function runDeploy(args: DeployArgs, io: CliIo): Promise<0 | 1 | 2> const checked = await checkRunnableBundle(source, ref.name); if (!checked.report.ok) { for (const diagnostic of checked.report.diagnostics) io.stderr(`${diagnostic.severity.toUpperCase()} [${diagnostic.kind}] ${diagnostic.message}`); + if (args.json) { + io.stdout(JSON.stringify({ ok: false, code: 'bundle_unrunnable', bundle: args.value, to: args.to, + diagnostics: checked.report.diagnostics })); + } return 2; } // `ok` may still carry warnings (e.g. `budget_unmetered`); report them. @@ -39,18 +59,20 @@ export async function runDeploy(args: DeployArgs, io: CliIo): Promise<0 | 1 | 2> if (await exists(target)) { await verifyDigest(target, ref.digest); io.stderr(`deploy_noop: ${args.value}`); - io.stdout(`SKIPPED (already-present) ${args.value} ${args.to}`); + emitDeployOutcome('already-present', args, io); return 0; } await writableBucket(target); started = true; const copied = await copyBundle(source, target, ref.digest); if (!copied) io.stderr(`deploy_noop: ${args.value}`); - io.stdout(`${copied ? 'DEPLOYED' : 'SKIPPED (already-present)'} ${args.value} ${args.to}`); + emitDeployOutcome(copied ? 'deployed' : 'already-present', args, io); return 0; } catch (error) { const kind = started ? 'deploy_partial' : error instanceof BundleFailure ? error.kind : 'bucket_unreachable'; - io.stderr(`${started ? 'FAILED' : 'REFUSED'} [${kind}] ${error instanceof Error ? error.message : String(error)}`); + const message = error instanceof Error ? error.message : String(error); + io.stderr(`${started ? 'FAILED' : 'REFUSED'} [${kind}] ${message}`); + if (args.json) io.stdout(JSON.stringify({ ok: false, code: kind, message, bundle: args.value, to: args.to })); return started ? 1 : 2; } } diff --git a/packages/sdk/src/cli/serve-webhook.ts b/packages/sdk/src/cli/serve-webhook.ts index 0c6e75163..ef0590428 100644 --- a/packages/sdk/src/cli/serve-webhook.ts +++ b/packages/sdk/src/cli/serve-webhook.ts @@ -4,6 +4,7 @@ import { createServer, type Server, type ServerResponse } from 'node:http'; import { join, resolve } from 'node:path'; import { TextDecoder } from 'node:util'; import type { CliIo } from '../cli.js'; +import { DEFAULT_DATA_DIR } from '../daemon-connection.js'; import { providerInboxEvent } from '../trigger-executor.js'; import { verifySignature, schemeFor } from '../webhook-signature.js'; import { TokenBucketLimiter, keyFor, type RateLimitConfig } from '../webhook-rate-limit.js'; @@ -24,9 +25,12 @@ export function parseWebhookArgs(args: readonly string[]): { || !value || value.startsWith('-')) return undefined; values.set(flag, value); } - const dataDir = values.get('--data-dir'); + // `--data-dir` is optional here as it is on every other verb, and defaults to + // the same directory: the command surface advertises that default, so a + // receiver started without the flag has to run rather than exit 2. + const dataDir = values.get('--data-dir') ?? DEFAULT_DATA_DIR; const portText = values.get('--port'); - if (!dataDir || !portText || !/^\d+$/.test(portText)) return undefined; + if (!portText || !/^\d+$/.test(portText)) return undefined; const port = Number(portText); if (!Number.isInteger(port) || port < 0 || port > 65535) return undefined; const allow = values.get('--allow'); diff --git a/packages/sdk/tests/bundle.test.ts b/packages/sdk/tests/bundle.test.ts index 832c85287..1c517eea7 100644 --- a/packages/sdk/tests/bundle.test.ts +++ b/packages/sdk/tests/bundle.test.ts @@ -76,6 +76,29 @@ describe('immutable bundles', () => { await expect(seal(out)).rejects.toThrow('spec.canonical.json'); }); + it('verifies with --verify in any position and answers --json with one object', async () => { + // The surface lists --verify beside --out and --json, so every order help + // implies has to parse: --verify was a leading mode token that refused + // both `build --verify` and `build --verify --json ` (#451). + const bundle = await seal(await temp()); + const digest = `sha256:${await verifyBundle(bundle)}`; + + const trailing = invoke([bundle, '--verify']); + expect(trailing.status, trailing.stderr).toBe(0); + expect(trailing.stdout.trim()).toBe(`VERIFIED ${digest}`); + + const json = invoke(['--verify', '--json', bundle]); + expect(json.status, json.stderr).toBe(0); + expect(JSON.parse(json.stdout)).toEqual({ ok: true, verified: true, bundle, digest }); + }); + + it('refuses --out with --verify rather than ignoring the destination', async () => { + // The one combination the surface narrows instead: a verify builds nothing, + // so a destination for its output would describe nothing. + const bundle = await seal(await temp()); + expect(invoke(['--verify', '--out', 'dist', bundle]).status).toBe(2); + }); + it.each(['identity.json', 'manifest.json'])('detects tampered %s', async file => { const bundle = await seal(await temp()); await writeFile(join(bundle, file), '{}'); diff --git a/packages/sdk/tests/cloud-sync.test.ts b/packages/sdk/tests/cloud-sync.test.ts index df55160dd..4dd04817c 100644 --- a/packages/sdk/tests/cloud-sync.test.ts +++ b/packages/sdk/tests/cloud-sync.test.ts @@ -377,6 +377,36 @@ describe('flows run --cloud --sync-code / flows sync', () => { expect(output).toEqual(['NO CHANGES quiet']); }); + it('reports no changes for an empty single-tree patch the server flags as changed', async () => { + // A single-tree run can answer hasChanges: true with an empty body. The + // multi-path branch discounts those, and so did `agent-relay cloud sync`; + // without the same guard the empty patch reaches `git apply` and fails as + // patch_conflict instead of reporting NO CHANGES. + const root = await tempDir('cloud-sync-empty-'); + git(root, 'init', '-q'); + cloud({ '/api/v1/workflows/runs/empty/patch': () => ({ patch: ' \n', hasChanges: true }) }); + const output: string[] = []; + const errors: string[] = []; + expect(await runCli(['sync', '--dir', root, 'empty'], + { stdout: line => output.push(line), stderr: line => errors.push(line) })).toBe(0); + expect(output).toEqual(['NO CHANGES empty']); + expect(errors).toEqual([]); + }); + + it('reports an empty single-tree patch as no changes under --json and --dry-run', async () => { + const root = await tempDir('cloud-sync-empty-json-'); + cloud({ '/api/v1/workflows/runs/empty/patch': () => ({ patch: '', hasChanges: true }) }); + const output: string[] = []; + const io = { stdout: (line: string) => output.push(line), stderr: () => {} }; + + expect(await runCli(['sync', '--json', '--dir', root, 'empty'], io)).toBe(0); + expect(JSON.parse(output[0]!)).toEqual({ ok: true, runId: 'empty', hasChanges: false, applied: false }); + + output.length = 0; + expect(await runCli(['sync', '--dry-run', '--dir', root, 'empty'], io)).toBe(0); + expect(output).toEqual(['NO CHANGES empty']); + }); + it('refuses multi-path patches and conflicting patches without partial application', async () => { const root = await tempDir('cloud-sync-conflict-'); git(root, 'init', '-q'); diff --git a/packages/sdk/tests/deploy.test.ts b/packages/sdk/tests/deploy.test.ts index 23ec14c74..6df4c3848 100644 --- a/packages/sdk/tests/deploy.test.ts +++ b/packages/sdk/tests/deploy.test.ts @@ -38,7 +38,7 @@ describe('flows deploy file buckets', () => { return copy(source, target, digest); }); const stdout: string[] = []; const stderr: string[] = []; - expect(await runDeploy({ command: 'deploy', value: f.reference, to: f.bucket }, { + expect(await runDeploy({ command: 'deploy', value: f.reference, to: f.bucket, json: false }, { stdout: line => stdout.push(line), stderr: line => stderr.push(line), })).toBe(0); expect(await verifyBundle(f.target, f.digest)).toBe(f.digest); @@ -46,6 +46,31 @@ describe('flows deploy file buckets', () => { expect(stdout.join('\n')).not.toContain('DEPLOYED'); expect(stderr.join('\n')).toContain('deploy_noop'); }); + it('answers --json with one object per outcome', async () => { + // `--json` is declared on the shared `deploy` verb; only the hosted-listener + // parser accepted it, so help promised JSON the bundle form refused (#451). + const f = await setup(); + const first = f.invoke(['deploy', f.reference, '--to', f.bucket, '--json']); + expect(first.status, first.stderr).toBe(0); + expect(JSON.parse(first.stdout)) + .toEqual({ ok: true, bundle: f.reference, to: f.bucket, status: 'deployed' }); + + const second = f.invoke(['deploy', '--json', f.reference, '--to', f.bucket]); + expect(second.status, second.stderr).toBe(0); + expect(JSON.parse(second.stdout)) + .toEqual({ ok: true, bundle: f.reference, to: f.bucket, status: 'already-present' }); + }); + + it('reports a refusal as JSON under --json', async () => { + const f = await setup(); + await rm(f.bundle, { recursive: true }); + const result = f.invoke(['deploy', f.reference, '--to', f.bucket, '--json']); + expect(result.status).toBe(2); + expect(JSON.parse(result.stdout)).toMatchObject({ + ok: false, code: 'bundle_missing_locally', bundle: f.reference, to: f.bucket, + }); + }); + it('refuses a missing local bundle before creating the bucket', async () => { const f = await setup(); await rm(f.bundle, { recursive: true }); diff --git a/packages/sdk/tests/relay-cli-surface.test.ts b/packages/sdk/tests/relay-cli-surface.test.ts index cda017138..99990a386 100644 --- a/packages/sdk/tests/relay-cli-surface.test.ts +++ b/packages/sdk/tests/relay-cli-surface.test.ts @@ -11,7 +11,7 @@ import { } from '@agent-relay/cli-surface'; import { createRelayCliSurface } from '../src/relay-cli.js'; -import { CLI_VERBS, type CliVerbSpec } from '../src/cli-commands.js'; +import { CLI_VERBS, type CliCommandSpec, type CliOptionSpec, type CliVerbSpec } from '../src/cli-commands.js'; import { parseCliArgs, runCli, type ParsedArgs } from '../src/cli.js'; const temporaryDirectories: string[] = []; @@ -39,6 +39,7 @@ function capture(): RelayCliIo & { out: string; err: string } { } const DIGEST = `hello@sha256:${'a'.repeat(64)}`; +const BUNDLE_DIR = `dist/flows/${DIGEST}`; const RUN_ID = '01JABCDEFGHJKMNPQRSTVWXYZ0'; /** @@ -52,39 +53,122 @@ const RUN_ID = '01JABCDEFGHJKMNPQRSTVWXYZ0'; const INVOCATIONS: readonly { verb: string; argv: readonly string[]; variant: ParsedArgs['command'] }[] = [ { verb: 'add', argv: ['add', 'my-helper'], variant: 'add' }, { verb: 'build', argv: ['build', 'flow.yaml'], variant: 'build' }, - { verb: 'build', argv: ['build', '--out', 'dist', 'flow.yaml'], variant: 'build' }, + { verb: 'build', argv: ['build', '--out', 'dist', '--json', 'flow.yaml'], variant: 'build' }, + // `--verify` is a flag like any other: both orders help implies must parse. + { verb: 'build', argv: ['build', '--verify', '--json', BUNDLE_DIR], variant: 'build' }, + { verb: 'build', argv: ['build', BUNDLE_DIR, '--verify'], variant: 'build' }, { verb: 'check', argv: ['check', 'flow.yaml'], variant: 'check' }, { verb: 'check', argv: ['check', '--watch', '--json', 'flow.yaml'], variant: 'check' }, // Both `deploy` forms: the positional decides which variant the verb produces. { verb: 'deploy', argv: ['deploy', DIGEST, '--to', 'file:///tmp/bucket'], variant: 'deploy' }, + { verb: 'deploy', argv: ['deploy', DIGEST, '--to', 'file:///tmp/bucket', '--json'], variant: 'deploy' }, { verb: 'deploy', argv: ['deploy', 'review.flow.ts', '--repo', 'owner/name', '--on', 'github', '--approver', 'someone'], variant: 'cloud-deploy', }, + { + verb: 'deploy', + argv: ['deploy', 'review.flow.ts', '--repo', 'owner/name', '--on', 'github:label=review', + '--approver', 'someone', '--name', 'review-listener', '--agents', 'claude,codex', '--draft', '--json'], + variant: 'cloud-deploy', + }, { verb: 'deployments', argv: ['deployments', '--json'], variant: 'deployments' }, { verb: 'hn-monitor', argv: ['hn-monitor', 'start', 'spec.json'], variant: 'hn-monitor' }, + { + verb: 'hn-monitor', + argv: ['hn-monitor', 'start', '--data-dir', '.relayflowd', '--poll-interval-ms', '1000', 'spec.json'], + variant: 'hn-monitor', + }, { verb: 'observer', argv: ['observer'], variant: 'observer' }, + { verb: 'observer', argv: ['observer', '--data-dir', '.relayflowd'], variant: 'observer' }, { verb: 'replay', argv: ['replay', RUN_ID], variant: 'replay' }, + { + verb: 'replay', + argv: ['replay', '--json', '--data-dir', '.relayflowd', '--at', 'step-1', + '--allow-human-influenced', RUN_ID], + variant: 'replay', + }, { verb: 'resume', argv: ['resume', RUN_ID], variant: 'resume' }, + { + verb: 'resume', + argv: ['resume', '--json', '--data-dir', '.relayflowd', '--local-agent', '--no-spawn', + '--no-observer-link', '--allow-human-influenced', RUN_ID], + variant: 'resume', + }, { verb: 'run', argv: ['run', 'flow.yaml'], variant: 'run' }, + { + verb: 'run', + argv: ['run', '--json', '--data-dir', '.relayflowd', '--local-agent', '--no-spawn', + '--no-observer-link', '--allow-human-influenced', '--input', '{"a":1}', 'review.flow.ts'], + variant: 'run', + }, + // `--input` is the authored body's argument and `--reuse-from` memoizes a + // declarative run, so they belong to different sources rather than one argv. + { verb: 'run', argv: ['run', '--reuse-from', RUN_ID, 'flow.yaml'], variant: 'run' }, + { verb: 'run', argv: ['run', '--bucket', 'file:///tmp/bucket', DIGEST], variant: 'run' }, // `--cloud` is the second variant of the same verb. { verb: 'run', argv: ['run', '--cloud', '--wait', 'flow.yaml'], variant: 'cloud-run' }, + { verb: 'run', argv: ['run', '--cloud', '--sync-code', 'review.flow.ts'], variant: 'cloud-run' }, { verb: 'serve-webhook', - argv: ['serve-webhook', '--data-dir', '/tmp/inbox', '--port', '8080'], + argv: ['serve-webhook', '--data-dir', '/tmp/inbox', '--port', '8080', '--allow', 'review,triage'], variant: 'serve-webhook', }, + // `--data-dir` carries a default in the tree, so the receiver has to start without it. + { verb: 'serve-webhook', argv: ['serve-webhook', '--port', '8080'], variant: 'serve-webhook' }, { verb: 'sync', argv: ['sync', RUN_ID], variant: 'sync' }, - { verb: 'sync', argv: ['sync', '--dry-run', '--json', RUN_ID], variant: 'sync' }, + { verb: 'sync', argv: ['sync', '--dry-run', '--json', '--dir', '.', RUN_ID], variant: 'sync' }, { verb: 'tick', argv: ['tick', 'start', '--schedule-id', 'nightly', '--interval-ms', '60000', 'spec.json'], variant: 'tick', }, + { + verb: 'tick', + argv: ['tick', 'start', '--data-dir', '.relayflowd', '--schedule-id', 'nightly', + '--interval-ms', '60000', '--epoch-ms', '0', '--max-catch-up', '3', + '--poll-interval-ms', '1000', 'spec.json'], + variant: 'tick', + }, { verb: 'undeploy', argv: ['undeploy', 'dep_123'], variant: 'undeploy' }, + { verb: 'undeploy', argv: ['undeploy', '--json', 'dep_123'], variant: 'undeploy' }, ]; +/** Every command in the declared tree, with the argv tokens that reach it. */ +function declaredCommands(): { command: CliCommandSpec; path: readonly string[] }[] { + return (CLI_VERBS as readonly CliVerbSpec[]).flatMap((verb) => [ + { command: verb as CliCommandSpec, path: [verb.name] }, + ...(verb.subcommands ?? []).map((sub) => ({ command: sub, path: [verb.name, sub.name] })), + ]); +} + +/** `'--out '` -> `'--out'`: the token a user actually types. */ +function flagToken(option: CliOptionSpec): string { + return option.flags.split(' ')[0]!; +} + +function reaches(argv: readonly string[], path: readonly string[]): boolean { + return path.every((token, index) => argv[index] === token); +} + +/** + * Where the positionals are in an invocation of `command`, reading the declared + * options to know which tokens are flag values rather than arguments. + */ +function positionalIndices(command: CliCommandSpec, argv: readonly string[], start: number): number[] { + const takesValue = new Set( + (command.options ?? []).filter((option) => option.flags.includes('<')).map(flagToken), + ); + const indices: number[] = []; + for (let index = start; index < argv.length; index++) { + const token = argv[index]!; + if (!token.startsWith('-')) indices.push(index); + else if (takesValue.has(token)) index += 1; + } + return indices; +} + describe('relay-cli surface: contract conformance', () => { it('satisfies the structural contract from @agent-relay/cli-surface', () => { // Typed assignment, not a cast: this is where the locally-declared surface @@ -191,6 +275,85 @@ describe('relay-cli surface: drift between `commands` and `run`', () => { } }); + it('carries every declared flag on an invocation that parses', () => { + // The stronger half of the same guard. Inserting a flag into one sample + // only proves it is tolerated in that position; this proves every declared + // option -- value-taking ones included, which the insertion check has to + // skip -- is carried by a real invocation of its own command, and the + // routing test above proves each of those invocations parses. + // + // This is what `build --json`, `build --verify` and `deploy --json` + // failed: help listed them, no invocation could carry them. + const uncarried: string[] = []; + + for (const { command, path } of declaredCommands()) { + for (const option of command.options ?? []) { + const flag = flagToken(option); + const carried = INVOCATIONS.some( + ({ argv }) => reaches(argv, path) && argv.includes(flag) && parseCliArgs(argv) !== undefined, + ); + if (!carried) uncarried.push(`flows ${path.join(' ')} ${flag}`); + } + } + + expect(uncarried).toEqual([]); + }); + + it('lets every option that declares a default be omitted', () => { + // A `defaultValue` in the tree is help telling the reader the flag is + // optional. `serve-webhook` advertised `--data-dir` with `.relayflowd` and + // then exited 2 without it; a default the parser does not honour is the + // same drift as a flag it refuses. + const undefaulted: string[] = []; + + for (const { command, path } of declaredCommands()) { + for (const option of command.options ?? []) { + if (option.defaultValue === undefined) continue; + const flag = flagToken(option); + const omitted = INVOCATIONS.some( + ({ argv }) => reaches(argv, path) && !argv.includes(flag) && parseCliArgs(argv) !== undefined, + ); + if (!omitted) undefaulted.push(`flows ${path.join(' ')} without ${flag}`); + } + } + + expect(undefaulted).toEqual([]); + }); + + it('parses exactly the positional arity each command declares', () => { + // The third way help can lie: the right flags on the wrong number of + // arguments. Every sample must carry exactly the declared positionals, and + // a positional declared `required: true` must actually be required. + for (const { command, path } of declaredCommands()) { + // A verb that routes on a subcommand declares no positionals of its own; + // its arguments belong to the subcommand entry, checked on its own turn. + if (command.subcommands?.length) continue; + + const required = (command.args ?? []).filter((arg) => arg.required).length; + const variadic = (command.args ?? []).some((arg) => arg.variadic); + const samples = INVOCATIONS.filter(({ argv }) => reaches(argv, path)); + + expect(samples.length, `flows ${path.join(' ')} has a sample invocation`).toBeGreaterThan(0); + + for (const { argv } of samples) { + const positionals = positionalIndices(command, argv, path.length); + const rendered = `flows ${argv.join(' ')}`; + if (variadic) expect(positionals.length, rendered).toBeGreaterThanOrEqual(required); + else expect(positionals.length, rendered).toBe(required); + } + + if (required === 0) continue; + + // And dropping the last declared positional is refused, so `required` + // means required rather than "documented". + const { argv } = samples[0]!; + const last = positionalIndices(command, argv, path.length).at(-1)!; + const short = [...argv.slice(0, last), ...argv.slice(last + 1)]; + + expect(parseCliArgs(short), `\`flows ${short.join(' ')}\` is refused`).toBeUndefined(); + } + }); + it('refuses a flag it does not declare', () => { // And the converse, spot-checked where the table is most likely to go // stale: an undeclared switch is a parse failure, not a silent no-op. diff --git a/packages/sdk/tests/webhook.test.ts b/packages/sdk/tests/webhook.test.ts index ba2d9072b..1f15b677b 100644 --- a/packages/sdk/tests/webhook.test.ts +++ b/packages/sdk/tests/webhook.test.ts @@ -4,6 +4,7 @@ import { join, resolve } from 'node:path'; import { tmpdir } from 'node:os'; import type { Server } from 'node:http'; import { startWebhookServer, parseWebhookArgs } from '../src/cli/serve-webhook.js'; +import { DEFAULT_DATA_DIR } from '../src/daemon-connection.js'; import { preflightWebhookTriggers } from '../src/preflight.js'; import { webhook, slack, github } from '@relayflows/surface'; import { webhookTriggerSpec } from '../src/trigger-executor.js'; @@ -135,9 +136,13 @@ describe('webhook ingress', () => { }); it('parses CLI options strictly', () => { expect(parseWebhookArgs(['--data-dir', 'data', '--port', '0'])).toEqual({ command: 'serve-webhook', dataDir: 'data', port: 0 }); + // The command surface advertises `--data-dir` with a default, so omitting + // it starts the receiver on that default rather than exiting 2 (#451). + expect(parseWebhookArgs(['--port', '0'])) + .toEqual({ command: 'serve-webhook', dataDir: DEFAULT_DATA_DIR, port: 0 }); expect(parseWebhookArgs(['--data-dir', 'data', '--port', '0', '--allow', 'release,push'])) .toEqual({ command: 'serve-webhook', dataDir: 'data', port: 0, admitted: ['release', 'push'] }); - for (const args of [[], ['--port', '80'], ['--data-dir', 'x', '--port', '65536'], + for (const args of [[], ['--data-dir', 'x'], ['--data-dir', 'x', '--port', '65536'], ['--data-dir', 'x', '--port', '1', '--port', '2'], ['--data-dir', 'x', '--port', '3.1'], // #303 admission: empty --allow list and invalid names refuse at parse time ['--data-dir', 'x', '--port', '0', '--allow', ''], From b4082f2679d6cda4bb00517b75e67c889fab20df Mon Sep 17 00:00:00 2001 From: agentrelaybot Date: Thu, 17 Sep 2026 23:03:12 -0700 Subject: [PATCH 5/8] chore(sdk): resolve @agent-relay/cli-surface from the registry `tests/relay-cli-surface.test.ts` imports the contract package but nothing declared it, so it resolved only where a sibling checkout happened to provide one. Relay's 12.2.4 release publishes a working tarball (the earlier 12.2.2 held only package.json), so this pins ^12.2.4 and records it in the lockfile. devDependency only: src/relay-cli.ts stays structurally typed, so the published package still carries no runtime dependency on relay. npm ci clean; 90 tests passing. Co-Authored-By: Claude Opus 5 (1M context) Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916 --- packages/sdk/package-lock.json | 8 ++++---- packages/sdk/package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/sdk/package-lock.json b/packages/sdk/package-lock.json index af4e19b52..8fe43fe26 100644 --- a/packages/sdk/package-lock.json +++ b/packages/sdk/package-lock.json @@ -25,16 +25,16 @@ "flows": "dist/cli.js" }, "devDependencies": { - "@agent-relay/cli-surface": "^12.2.2", + "@agent-relay/cli-surface": "^12.2.4", "@types/node": "^22.7.0", "typescript": "^5.6.0", "vitest": "^2.1.0" } }, "node_modules/@agent-relay/cli-surface": { - "version": "12.2.2", - "resolved": "https://registry.npmjs.org/@agent-relay/cli-surface/-/cli-surface-12.2.2.tgz", - "integrity": "sha512-xOIx+a3AjFn3P2MZp1GwqwVS7qZiBKu4uMXV+mK/gb0RsH//PD6zcG+855DdF/Zep/yVv9JSEcyfWl/wh0eJhw==", + "version": "12.2.4", + "resolved": "https://registry.npmjs.org/@agent-relay/cli-surface/-/cli-surface-12.2.4.tgz", + "integrity": "sha512-DqJXout26UOLNXi+yTgAD53ek9TXGoSw+5LX8wRwqWeiItiBdw5oHE54UeIzjdbVxNZyk58Dw1494goHN/+AUg==", "dev": true, "engines": { "node": ">=22.0.0" diff --git a/packages/sdk/package.json b/packages/sdk/package.json index 0a31a005b..25383c53b 100644 --- a/packages/sdk/package.json +++ b/packages/sdk/package.json @@ -59,7 +59,7 @@ "yaml": "^2.5.1" }, "devDependencies": { - "@agent-relay/cli-surface": "^12.2.2", + "@agent-relay/cli-surface": "^12.2.4", "@types/node": "^22.7.0", "typescript": "^5.6.0", "vitest": "^2.1.0" From 5cad9dba4d837536690ed22a21f15a31ca92f6e6 Mon Sep 17 00:00:00 2001 From: agentrelaybot Date: Fri, 18 Sep 2026 06:38:02 -0700 Subject: [PATCH 6/8] fix(sdk): drop the sibling-checkout alias for @agent-relay/cli-surface `vitest.config.ts` aliased `@agent-relay/cli-surface` to `../../../relay/packages/cli-surface/dist/index.js` -- a sibling clone of the relay repo -- and `tsconfig.tests.json` mirrored it in `paths`. Both predate the package becoming a real registry devDependency, and neither was removed when it did. The alias is unconditional, so vitest rewrites the bare specifier to that absolute path before any node_modules lookup. On a machine that happens to have the relay repo checked out next door it resolves and the suite is green; on a GitHub runner the path does not exist, vite's alias plugin fails to resolve it, and vite-node reports the failure against the original id: Cannot find module '@agent-relay/cli-surface' imported from packages/sdk/tests/relay-cli-surface.test.ts The installed devDependency was never consulted. The tsconfig `paths` entry was harmless only because tsc falls back to node resolution when a mapping misses, which is why typecheck:tests stayed green while vitest did not. Removing both makes the tests resolve the published 12.2.4 devDependency the lockfile already pins, which is what the registry move was for. Co-Authored-By: Claude Opus 5 (1M context) Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916 --- packages/sdk/tsconfig.tests.json | 12 +----------- packages/sdk/vitest.config.ts | 19 ------------------- 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/packages/sdk/tsconfig.tests.json b/packages/sdk/tsconfig.tests.json index 59e4e9308..a48420dde 100644 --- a/packages/sdk/tsconfig.tests.json +++ b/packages/sdk/tsconfig.tests.json @@ -7,17 +7,7 @@ "noEmit": true, "allowImportingTsExtensions": true, "rootDir": ".", - "types": ["node", "vitest"], - // The relay CLI-surface contract, resolved dev-only. Deliberately NOT a - // package.json devDependency: npm would write a `file:`/`..` entry into - // package-lock.json, and `checkInstalledVersions` in bundle-typescript.ts - // rejects exactly that, so every standalone TS bundle would stop building. - // A path mapping gives the tests the real contract types with no lockfile - // entry, and the published package keeps no dependency on it at all. - // Mirrored by `resolve.alias` in vitest.config.ts for the runtime helpers. - "paths": { - "@agent-relay/cli-surface": ["../../../relay/packages/cli-surface/dist/index.d.ts"] - } + "types": ["node", "vitest"] }, "include": [ "src/**/*.ts", diff --git a/packages/sdk/vitest.config.ts b/packages/sdk/vitest.config.ts index dad36a5b9..efa710b61 100644 --- a/packages/sdk/vitest.config.ts +++ b/packages/sdk/vitest.config.ts @@ -1,25 +1,6 @@ -import { fileURLToPath } from 'node:url'; import { defineConfig } from 'vitest/config'; -/** - * The relay CLI-surface contract package, linked dev-only for the surface - * conformance and drift tests. - * - * Not a package.json devDependency on purpose: npm records a `file:` / `..` - * location in package-lock.json, which `checkInstalledVersions` in - * bundle-typescript.ts refuses, breaking every standalone TS bundle build. - * Aliasing it here (and via `paths` in tsconfig.tests.json) gives the tests the - * real `assertSurfaceConforms` / `walkCommands` implementations while leaving - * both the lockfile and the published package untouched. - */ -const CLI_SURFACE = fileURLToPath( - new URL('../../../relay/packages/cli-surface/dist/index.js', import.meta.url), -); - export default defineConfig({ - resolve: { - alias: { '@agent-relay/cli-surface': CLI_SURFACE }, - }, test: { environment: 'node', include: ['tests/**/*.test.ts'], From 61a58daf79ed6424fcdd59ac302f369934bf33fa Mon Sep 17 00:00:00 2001 From: agentrelaybot Date: Fri, 18 Sep 2026 07:27:23 -0700 Subject: [PATCH 7/8] fix(sdk): pass the caller's signal to cloud-run's connect preflight `ensureFlowConnections` was handed `controller.signal`, a name that does not exist on this path -- the local AbortController it came from belongs to `runCli`, not to `runCloudCli`, which receives the already-derived `signal`. A `run --cloud` that reached the connect preflight died with a ReferenceError instead of submitting, and an aborted submission surfaced as that error rather than as the `submission_aborted` the classifier below it names. Co-Authored-By: Claude Opus 5 (1M context) Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916 Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916 --- packages/sdk/src/cli/cloud-run.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sdk/src/cli/cloud-run.ts b/packages/sdk/src/cli/cloud-run.ts index 3b8310661..4b930143f 100644 --- a/packages/sdk/src/cli/cloud-run.ts +++ b/packages/sdk/src/cli/cloud-run.ts @@ -42,7 +42,7 @@ export async function runCloudCli( // arguments must not first open a browser. const connections = await ensureFlowConnections({ path, prompt: cliConnectPrompt(io, { noConnect, json }), - }, { signal: controller.signal }); + }, { signal }); harnesses = connections?.requirements.harnesses ?? []; for (const provider of connections?.outcome.connected ?? []) if (!json) io.stdout(`CONNECTED ${provider}`); // The tree is the invoking directory, as with v1: the flow path is where From d5f987ca7e209ceada5fd1203b1deb60ad75281e Mon Sep 17 00:00:00 2001 From: agentrelaybot Date: Fri, 18 Sep 2026 07:27:44 -0700 Subject: [PATCH 8/8] feat(sdk): declare answer, schedule, schedules and unschedule on the surface The rebase brought four verbs onto `parseArgs` that `CLI_VERBS` did not claim, so `_EveryVariantIsDeclared` stopped compiling -- the table's whole point. Declare them, with the help text `agent-relay flows --help` will render: - `answer yes|no` records a person's decision on a parked `f.human`; it continues nothing itself, so the description names the `flows resume` that does. `--note`, `--by`, `--data-dir` and `--no-spawn` are the parser's, and `--by` documents its OS-user default. - `schedule ` registers a Cloud cron or `--every` cadence; with neither flag the flow's own `schedule.*` handler supplies the cron, which is the part of the contract a user cannot guess, so `--cron` says it. - `schedules` and `unschedule ` mirror the wording of their `deployments` / `undeploy` neighbours. `--no-connect` is declared too, on the three verbs that hand a flow to Cloud (`deploy`, `run --cloud`, `schedule`). The parser has accepted it since the integration-connect prompts landed; only help had not caught up, which is the same drift in the direction the type assertion cannot see. Every new verb gets sample invocations in the drift test rather than any narrowing of it: the suite still requires each declared verb to be routable, each declared flag to be carried by an invocation that parses, each default to be omittable, and each declared positional to be genuinely required. Co-Authored-By: Claude Opus 5 (1M context) Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916 Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916 --- packages/sdk/src/cli-commands.ts | 63 ++++++++++++++++++++ packages/sdk/tests/relay-cli-surface.test.ts | 27 ++++++++- 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/packages/sdk/src/cli-commands.ts b/packages/sdk/src/cli-commands.ts index b1594e4ee..121b7972a 100644 --- a/packages/sdk/src/cli-commands.ts +++ b/packages/sdk/src/cli-commands.ts @@ -72,6 +72,16 @@ const JSON_OPTION: CliOptionSpec = { description: 'Emit one machine-readable JSON object instead of text', }; +/** + * Shared by the verbs that hand a flow to Cloud. Each of them preflights what + * the source needs connected and offers to connect it; this refuses instead, + * which is what a non-interactive caller wants. + */ +const NO_CONNECT_OPTION: CliOptionSpec = { + flags: '--no-connect', + description: 'Refuse a missing integration instead of offering to connect it', +}; + /** Flags shared by the two verbs that execute a flow locally. */ const LOCAL_EXECUTION_OPTIONS = [ JSON_OPTION, @@ -97,6 +107,23 @@ export const CLI_VERBS = [ args: [{ name: 'helper', description: 'Helper name or @flows/', required: true }], variants: ['add'], }, + { + name: 'answer', + description: 'Answer a run’s parked f.human question; `flows resume` then continues the body', + args: [ + { name: 'run-id', description: 'Run parked on the question', required: true }, + { name: 'wait-id', description: 'Which question to answer, named human- in the order the body asked', required: true }, + { name: 'answer', description: 'The decision, as yes or no (also true or false)', required: true }, + ], + options: [ + JSON_OPTION, + DATA_DIR_OPTION, + { flags: '--no-spawn', description: 'Require a running relayflowd rather than starting one' }, + { flags: '--note ', description: 'Reason recorded on the journal alongside the answer' }, + { flags: '--by ', description: 'Who answered, when relaying a person’s decision; defaults to the OS user' }, + ], + variants: ['answer'], + }, { name: 'build', description: 'Compile a flow into a sealed, content-addressed bundle', @@ -127,6 +154,7 @@ export const CLI_VERBS = [ { flags: '--agents ', description: 'Agent harnesses to allow, as claude[,codex]' }, { flags: '--name ', description: 'Name for the hosted listener' }, { flags: '--draft', description: 'Create the listener without activating it' }, + NO_CONNECT_OPTION, JSON_OPTION, ], variants: ['deploy', 'cloud-deploy'], @@ -196,9 +224,37 @@ export const CLI_VERBS = [ flags: '--sync-code', description: 'With --cloud, upload the working directory as the run’s tree; pull results back with `flows sync`', }, + { + flags: '--no-connect', + description: 'With --cloud, refuse a missing integration instead of offering to connect it', + }, ], variants: ['run', 'cloud-run'], }, + { + name: 'schedule', + description: 'Register a flow to run in Cloud on a cron or interval, or on the one it declares', + args: [{ name: 'flow', description: 'flow.yaml or flow.ts submitted on every fire', required: true }], + options: [ + { + flags: '--cron ', + description: 'Cron expression to fire on; with neither this nor --every, the flow’s own schedule.* handler supplies it', + }, + { flags: '--every ', description: 'Fixed cadence, as ; not valid with --cron' }, + { flags: '--tz ', description: 'IANA timezone the cron is read in' }, + { flags: '--input ', description: 'Input for an authored .flow.ts, inline JSON or a file path' }, + { flags: '--name ', description: 'Name for the schedule' }, + NO_CONNECT_OPTION, + JSON_OPTION, + ], + variants: ['schedule'], + }, + { + name: 'schedules', + description: 'List this workspace’s Cloud schedules', + options: [JSON_OPTION], + variants: ['schedules'], + }, { name: 'serve-webhook', description: 'Run the local webhook receiver that writes provider deliveries into the trigger inbox', @@ -247,6 +303,13 @@ export const CLI_VERBS = [ options: [JSON_OPTION], variants: ['undeploy'], }, + { + name: 'unschedule', + description: 'Remove a Cloud schedule, so it stops firing', + args: [{ name: 'schedule-id', description: 'Schedule id to remove', required: true }], + options: [JSON_OPTION], + variants: ['unschedule'], + }, ] as const satisfies readonly CliVerbSpec[]; /** diff --git a/packages/sdk/tests/relay-cli-surface.test.ts b/packages/sdk/tests/relay-cli-surface.test.ts index 99990a386..678099094 100644 --- a/packages/sdk/tests/relay-cli-surface.test.ts +++ b/packages/sdk/tests/relay-cli-surface.test.ts @@ -52,6 +52,13 @@ const RUN_ID = '01JABCDEFGHJKMNPQRSTVWXYZ0'; */ const INVOCATIONS: readonly { verb: string; argv: readonly string[]; variant: ParsedArgs['command'] }[] = [ { verb: 'add', argv: ['add', 'my-helper'], variant: 'add' }, + { verb: 'answer', argv: ['answer', RUN_ID, 'human-1', 'yes'], variant: 'answer' }, + { + verb: 'answer', + argv: ['answer', '--json', '--no-spawn', '--data-dir', '.relayflowd', + '--note', 'approved on the call', '--by', 'someone', RUN_ID, 'human-1', 'no'], + variant: 'answer', + }, { verb: 'build', argv: ['build', 'flow.yaml'], variant: 'build' }, { verb: 'build', argv: ['build', '--out', 'dist', '--json', 'flow.yaml'], variant: 'build' }, // `--verify` is a flag like any other: both orders help implies must parse. @@ -70,7 +77,8 @@ const INVOCATIONS: readonly { verb: string; argv: readonly string[]; variant: Pa { verb: 'deploy', argv: ['deploy', 'review.flow.ts', '--repo', 'owner/name', '--on', 'github:label=review', - '--approver', 'someone', '--name', 'review-listener', '--agents', 'claude,codex', '--draft', '--json'], + '--approver', 'someone', '--name', 'review-listener', '--agents', 'claude,codex', '--draft', + '--no-connect', '--json'], variant: 'cloud-deploy', }, { verb: 'deployments', argv: ['deployments', '--json'], variant: 'deployments' }, @@ -109,7 +117,20 @@ const INVOCATIONS: readonly { verb: string; argv: readonly string[]; variant: Pa { verb: 'run', argv: ['run', '--bucket', 'file:///tmp/bucket', DIGEST], variant: 'run' }, // `--cloud` is the second variant of the same verb. { verb: 'run', argv: ['run', '--cloud', '--wait', 'flow.yaml'], variant: 'cloud-run' }, - { verb: 'run', argv: ['run', '--cloud', '--sync-code', 'review.flow.ts'], variant: 'cloud-run' }, + { verb: 'run', argv: ['run', '--cloud', '--sync-code', '--no-connect', 'review.flow.ts'], variant: 'cloud-run' }, + // With neither --cron nor --every the flow's own `schedule.*` handler supplies + // the cron, so the bare form has to parse. + { verb: 'schedule', argv: ['schedule', 'review.flow.ts'], variant: 'schedule' }, + { verb: 'schedule', argv: ['schedule', '--cron', '0 9 * * *', 'flow.yaml'], variant: 'schedule' }, + { + verb: 'schedule', + // `--input` is the authored body's argument, so it travels with a .flow.ts. + argv: ['schedule', '--every', '15m', '--tz', 'America/New_York', '--name', 'nightly-review', + '--input', '{"a":1}', '--no-connect', '--json', 'review.flow.ts'], + variant: 'schedule', + }, + { verb: 'schedules', argv: ['schedules'], variant: 'schedules' }, + { verb: 'schedules', argv: ['schedules', '--json'], variant: 'schedules' }, { verb: 'serve-webhook', argv: ['serve-webhook', '--data-dir', '/tmp/inbox', '--port', '8080', '--allow', 'review,triage'], @@ -133,6 +154,8 @@ const INVOCATIONS: readonly { verb: string; argv: readonly string[]; variant: Pa }, { verb: 'undeploy', argv: ['undeploy', 'dep_123'], variant: 'undeploy' }, { verb: 'undeploy', argv: ['undeploy', '--json', 'dep_123'], variant: 'undeploy' }, + { verb: 'unschedule', argv: ['unschedule', 'sched_123'], variant: 'unschedule' }, + { verb: 'unschedule', argv: ['unschedule', '--json', 'sched_123'], variant: 'unschedule' }, ]; /** Every command in the declared tree, with the argv tokens that reach it. */