diff --git a/docs/CLOUD.md b/docs/CLOUD.md index eda945f1..bad7c5ac 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/package-lock.json b/packages/sdk/package-lock.json index c29011a8..12899f59 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.4", "@types/node": "^22.7.0", "typescript": "^5.6.0", "vitest": "^2.1.0" } }, + "node_modules/@agent-relay/cli-surface": { + "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" + } + }, "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 1787d13b..65392d59 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": [ @@ -55,6 +59,7 @@ "yaml": "^2.5.1" }, "devDependencies": { + "@agent-relay/cli-surface": "^12.2.4", "@types/node": "^22.7.0", "typescript": "^5.6.0", "vitest": "^2.1.0" diff --git a/packages/sdk/src/cli-commands.ts b/packages/sdk/src/cli-commands.ts new file mode 100644 index 00000000..121b7972 --- /dev/null +++ b/packages/sdk/src/cli-commands.ts @@ -0,0 +1,339 @@ +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', +}; + +/** + * 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, + 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: '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', + 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; not valid with --verify' }, + { 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' }, + NO_CONNECT_OPTION, + 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`', + }, + { + 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', + options: [ + DATA_DIR_OPTION, + { 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'], + }, + { + 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: '--dry-run', description: 'Print the patch and apply nothing' }, + { 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'], + }, + { + 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[]; + +/** + * 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 c2559f9b..2fe2a6db 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 4f89fe8c..f5f5b9ed 100644 --- a/packages/sdk/src/cli.ts +++ b/packages/sdk/src/cli.ts @@ -36,6 +36,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, @@ -50,14 +51,19 @@ 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 | 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; noConnect: 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 } @@ -92,7 +98,7 @@ const USAGE = [ 'flows run [--json] [--no-spawn] [--no-observer-link] [--data-dir ] [--local-agent] [--reuse-from ] ', 'flows run --cloud [--json] [--wait] [--sync-code] [--no-connect] ', 'flows run --cloud [--json] [--wait] [--sync-code] [--no-connect] --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] ', @@ -117,9 +123,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); @@ -135,9 +180,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); @@ -159,7 +208,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. @@ -174,45 +225,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 @@ -458,6 +491,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)); @@ -718,9 +756,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) { @@ -730,6 +772,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; @@ -741,7 +788,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 { @@ -956,3 +1003,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/build.ts b/packages/sdk/src/cli/build.ts index 72e07578..30ffa498 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-run.ts b/packages/sdk/src/cli/cloud-run.ts index b3fc0d26..4b930143 100644 --- a/packages/sdk/src/cli/cloud-run.ts +++ b/packages/sdk/src/cli/cloud-run.ts @@ -11,11 +11,13 @@ export async function runCloudCli( value: string; json: boolean; wait: boolean; input: string | undefined; syncCode: boolean; noConnect?: 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 @@ -23,7 +25,7 @@ export async function runCloudCli( let submitting = false; let harnesses: readonly string[] = []; 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`. @@ -40,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 @@ -62,16 +64,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.' @@ -83,8 +85,5 @@ export async function runCloudCli( && (['configuration', 'unsupported_source', 'invalid_input', 'unsupported_storage_backend', 'sync_too_large', 'sync_unsupported', 'integration_not_connected'].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/cloud-sync.ts b/packages/sdk/src/cli/cloud-sync.ts index f890c1ea..03246305 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,50 @@ 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, {}); + // 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; } - 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 +64,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/cli/deploy.ts b/packages/sdk/src/cli/deploy.ts index f72dff0a..0af2c050 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 3f156f76..ef059042 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'); @@ -215,6 +219,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 +241,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/cloud-sync.ts b/packages/sdk/src/cloud-sync.ts index f48425a9..1781f54f 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 0e1115f9..9b63d977 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 { scheduleInCloud, listCloudSchedules, unscheduleInCloud, everyToCron, declaredScheduleCron, diff --git a/packages/sdk/src/relay-cli.ts b/packages/sdk/src/relay-cli.ts new file mode 100644 index 00000000..93b0ef68 --- /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/bundle.test.ts b/packages/sdk/tests/bundle.test.ts index 832c8528..1c517eea 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 54c25ecd..4dd04817 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' }); @@ -372,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'); @@ -387,3 +422,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/deploy.test.ts b/packages/sdk/tests/deploy.test.ts index 23ec14c7..6df4c384 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-live.test.ts b/packages/sdk/tests/relay-cli-surface-live.test.ts new file mode 100644 index 00000000..2cfbbd83 --- /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 00000000..67809909 --- /dev/null +++ b/packages/sdk/tests/relay-cli-surface.test.ts @@ -0,0 +1,476 @@ +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 CliCommandSpec, type CliOptionSpec, 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 BUNDLE_DIR = `dist/flows/${DIGEST}`; +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: '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. + { 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', + '--no-connect', '--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', '--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'], + 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', '--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' }, + { 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. */ +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 + // 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 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('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. + 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 + // 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/tests/webhook.test.ts b/packages/sdk/tests/webhook.test.ts index ba2d9072..1f15b677 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', ''], diff --git a/packages/sdk/tsconfig.tests.json b/packages/sdk/tsconfig.tests.json index aebe9706..c0d95083 100644 --- a/packages/sdk/tsconfig.tests.json +++ b/packages/sdk/tsconfig.tests.json @@ -41,7 +41,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"] }