Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions docs/SURFACE.md
Original file line number Diff line number Diff line change
Expand Up @@ -1022,6 +1022,19 @@ existing run from its journal. The data directory defaults to `.relayflowd`.
`--json` writes one report-shaped object to stdout while diagnostics remain on
stderr.

`check`'s `REQUIRES` line names the harnesses a flow needs; a spec with `agent`
steps also needs a *worker attached for step type `agent`*, and without one the
run parks at the first such step rather than failing. `flows check` cannot see
whether one is attached — it is daemon-free by construction — so it warns
`agent_worker_unresolved` instead of guessing, naming the steps and pointing at
`flows run --local-agent`. The warning is emitted in the `REQUIRES` position,
because it is a footnote to that line. It never refuses, and it is scoped to
`agent` steps: `--local-agent` attaches no `llm` worker, so naming it for an
`llm` step would be false. Only `flows check` opts in. `run` knows the answer,
`build` and `deploy` check a spec that will run elsewhere, and an authored
`.flow.ts` is checked through its header without compiling step bodies, so it
has no agent steps to count.

`flows check --watch` checks once, then watches the target, its reachable
relative `use:` imports, and the nearest `flows.json` walking up from the
flow directory. Saves are debounced for 150 ms; a change during a check
Expand Down Expand Up @@ -1350,6 +1363,28 @@ asserts on `AgentResult.artifacts` — which is how the data-dir case was caught
at all, by `packages/sdk/tests/agent-transcript-live.test.ts` failing its own
`artifacts.length !== 0` check.

That exclusion cuts both ways. A gate reads the journaled list and nothing else,
so an `artifact_exists` path inside a prefix the walk skips — a segment named
exactly `.git`, `.relayflowd` or `node_modules` — is a path the
scan can never contribute, however faithfully the agent writes the file.
Preflight **warns** `gate_path_unscanned`, naming the excluded prefix. Segments
are compared exactly: `node_modules-copy/out.md` and `reports/v1.2/review.md`
are fine. The rule lives in `packages/sdk/src/artifact-scan-policy.ts` and the
walk itself consumes it, so the warning cannot drift from the scan it
describes, and it is collected before any environment probe can return early,
so it is not hidden behind an unresolved CLI for a pass or two.

It warns rather than refuses because the scan is not the only writer of that
list, which makes such a gate unproven rather than unsatisfiable. The *bundled*
worker journals object-shaped JSON stdout — and a completed Relay task's output
— verbatim in place of the scanned wrapper, so an agent that answers with its
own `{"artifacts": [...]}` puts exactly what it names in the list, an unscanned
path included, and the gate passes. `step.complete` accepts any `output`, so a custom
worker may do the same. Which route a step takes is a run-time fact, so
preflight reports the scan's limitation and leaves the verdict to the run. The
warning retires with the exclusion: when the scan stops skipping those prefixes,
the kind and `packages/sdk/src/named-gate-preflight.ts` are deleted together.

- Are YAML helper verbs (`slack:`, `mcp:`) core spec vocabulary or compile-time expansion into `run`/effect steps? Leaning: expansion — the kernel spec stays seven words; helpers stay a surface concern.
- Helper generation cadence: generated from relayfile adapter manifests at build time vs published per-adapter packages. Leaning: generated, with hand-tuned verb names for the top providers.
- `on` inside a running body (subscribe after start, buffer events while a step runs, end on `idle`/`deadline`). Proposed in [`docs/EVENT-AWAIT.md`](EVENT-AWAIT.md); motivating case is a flow that babysits the PR it opened.
Expand Down
12 changes: 2 additions & 10 deletions packages/sdk/src/agent-artifacts.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,12 @@
import { createHash } from 'node:crypto';
import { readFile, readdir, stat } from 'node:fs/promises';
import { join, relative, sep } from 'node:path';
import { isUnscannedEntryName } from './artifact-scan-policy.js';

function isEnoent(error: unknown): boolean {
return error instanceof Error && (error as NodeJS.ErrnoException).code === 'ENOENT';
}

/**
* Entry names never walked and never reported, matched exactly, at any depth,
* before the entry's type is consulted — so a `.git` *file* (a worktree's
* gitdir pointer) is skipped as surely as a `.git` directory. Exact names, not
* prefixes: `.github`, `.relayflowd-notes` and every other author-chosen name
* that merely starts the same way stays eligible.
*/
const SKIPPED_ENTRY_NAMES = new Set(['.git', '.relayflowd', 'node_modules']);

/**
* A recursive snapshot of every regular file under `dir`, keyed by its
* `dir`-relative POSIX path, valued by a content signature (size + sha256).
Expand Down Expand Up @@ -55,7 +47,7 @@ async function walk(root: string, current: string, out: Map<string, string>): Pr
throw error;
}
for (const entry of entries) {
if (SKIPPED_ENTRY_NAMES.has(entry.name)) continue;
if (isUnscannedEntryName(entry.name)) continue;
const path = join(current, entry.name);
if (entry.isDirectory()) {
await walk(root, path, out);
Expand Down
51 changes: 51 additions & 0 deletions packages/sdk/src/artifact-scan-policy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* The bundled agent worker's artifact-scan policy, as a pure predicate.
*
* `agent-artifacts.ts` walks an agent step's cwd and records every regular
* file it finds, except entries named exactly `.git`, `.relayflowd` or
* `node_modules` — files as well as directories, in both cases. Dotfiles and
* dot-directories ARE eligible: `.workflow-artifacts/` is the conventional
* place a flow tells its agents to write. What that walk records is one
* source of the journaled `output.artifacts` list, which is the only thing an
* `artifact_exists` gate reads (`named-gate-lowering.ts` tests exact
* membership in that list and never touches disk).
*
* Static analysis needs the same rule without importing filesystem traversal,
* so the rule lives here and both consume it. It bounds the *scan*, not the
* list: `worker.ts` journals object-shaped JSON stdout and completed Relay
* task output verbatim instead of the scanned wrapper, and
* `JournalClient.stepComplete` accepts any `output` from any worker — so a
* path this predicate excludes can still reach the list by those routes
* (`named-gate-preflight.ts` warns rather than refusing for exactly that
* reason).
*/

/**
* Entry names the walk never descends into or records, matched exactly, at
* any depth, before the entry's type is consulted — so a `.git` *file* (a
* worktree's gitdir pointer) is skipped as surely as a `.git` directory.
* Exact names, not prefixes: `.github`, `.relayflowd-notes` and every other
* author-chosen name that merely starts the same way stays eligible.
*/
const SKIPPED_ENTRY_NAMES = new Set(['.git', '.relayflowd', 'node_modules']);

export function isUnscannedEntryName(name: string): boolean {
return SKIPPED_ENTRY_NAMES.has(name);
}

/**
* The shortest `/`-prefix of a relative POSIX `path` the scan excludes, or
* `undefined` when every segment is scanned.
*
* Segments are compared exactly, with no normalization: the gate matches the
* author's literal string against the worker's literal list, so `node_modules`
* is excluded and `node_modules-copy` is not, and a backslash is an ordinary
* filename character rather than a separator. The whole prefix is returned,
* not the offending segment alone, because that prefix is the directory an
* author has to move the artifact out of.
*/
export function unscannedArtifactPrefix(path: string): string | undefined {
const segments = path.split('/');
const index = segments.findIndex(isUnscannedEntryName);
return index === -1 ? undefined : segments.slice(0, index + 1).join('/');
}
24 changes: 22 additions & 2 deletions packages/sdk/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,13 @@ export async function runCli(
// refuses `--data-dir` on `check`, so there is no data dir to attach to.
// `flows check` keeps working with no daemon, no relayflowd binary and no
// data directory at all -- a property worth keeping, not an omission.
// Only this invocation opts into `agent_worker_unresolved`: `flows check`
// attaches no worker and, being daemon-free, cannot see one attached
// elsewhere. The authored `.flow.ts` path checks header declarations
// without compiling steps, so it has no agent steps to count.
const checked = /\.(?:[cm]?[jt]s)$/.test(parsed.value)
? await checkAuthoredFlowComposed(parsed.value) : checkFlow(parsed.value);
? await checkAuthoredFlowComposed(parsed.value)
: checkFlow(parsed.value, { warnUnresolvedAgentWorker: true });
emitCheckReport(checked.report, parsed.json, io);
return checked.report.ok ? 0 : 2;
}
Expand Down Expand Up @@ -941,6 +946,18 @@ function parseTickArgs(rest: readonly string[]): ParsedArgs | undefined {
};
}

/**
* `agent_worker_unresolved` reads as a footnote to `REQUIRES codex (step
* "implement"), …`: that line already names the steps that need an agent
* worker, and this says what has to be true for one to be attached. So the
* plain-text pass holds it back and emits it in that position, exactly once —
* the leading batch below skips it rather than printing it twice. JSON mode
* returns before any of this and keeps the single ordered diagnostics array.
*/
function isWorkerSurfaceWarning(diagnostic: CheckReport['diagnostics'][number]): boolean {
return diagnostic.kind === 'agent_worker_unresolved';
}

const MODEL_PROVENANCE: Readonly<Record<CliModelSource, string>> = {
step: 'step',
named: 'named agent',
Expand All @@ -953,11 +970,13 @@ function modelProvenance(source: CliModelSource | undefined): string {
}

function emitCheckReport(report: CheckReport, json: boolean, io: CliIo): void {
emitDiagnostics(report.diagnostics, io);
if (json) {
emitDiagnostics(report.diagnostics, io);
io.stdout(JSON.stringify(report));
return;
}
const deferred = report.diagnostics.filter(isWorkerSurfaceWarning);
emitDiagnostics(report.diagnostics.filter((diagnostic) => !isWorkerSurfaceWarning(diagnostic)), io);
for (const gate of report.gates) {
// A gate that accepts every output is legal, but it must not read like a
// gate that judges something.
Expand Down Expand Up @@ -1001,6 +1020,7 @@ function emitCheckReport(report: CheckReport, json: boolean, io: CliIo): void {
// the hosted verbs check the same list against Cloud before submitting.
const requires = report.requirements === undefined ? '' : describeFlowRequirements(report.requirements);
if (requires) io.stdout(`REQUIRES ${requires}`);
emitDiagnostics(deferred, io);
if (report.ok) io.stdout(`CHECK PASSED ${report.path ?? ''}`.trimEnd());
}

Expand Down
66 changes: 66 additions & 0 deletions packages/sdk/src/cli/check-worker-surface.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { compileSpec } from '../compile.js';
import type { FlowSpec } from '../spec.js';
import type { CheckWarningDiagnostic } from './check.js';

/** Enough ids to recognise the flow, not enough to bury the remedy. */
const NAMED_STEP_LIMIT = 3;

/**
* `agent` steps need a worker attached for step type `agent`, and nothing in a
* spec says whether one will be. `flows run` attaches the SDK's `AgentWorker`
* only under `--local-agent` (`cli/run.ts`); with no worker the run parks at
* the first agent step with `no worker is attached for step type "agent"`.
* `flows check` already walks these steps to print its `REQUIRES` line, so the
* fact is in hand — this spends it.
*
* Warning, never a refusal, and worded as a requirement rather than a
* prediction: worker attachment is *unknown* here, not absent. A worker can be
* attached out of band (an SDK `AgentWorker` against the same daemon, Cloud's
* workers for a hosted run), and `flows check` is daemon-free by construction,
* so it cannot see one either way.
*
* Scoped to `agent` steps. `--local-agent` attaches no `llm` worker on the
* YAML path, so naming it for `llm` steps would be false; those steps are not
* counted and not mentioned.
*
* Blind spot, shared with `permissions_unenforced`: an authored `.flow.ts`
* flow is checked through `checkMcpHeader`, which preflights a synthetic
* one-step header spec without executing the body. There are no compiled
* `agent` steps to count, so this diagnostic is silent for `.flow.ts`.
*/
export function agentWorkerDiagnostics(authoring: FlowSpec): CheckWarningDiagnostic[] {
const steps = agentStepIds(authoring);
if (steps.length === 0) return [];
return [{ severity: 'warning', kind: 'agent_worker_unresolved', message: message(steps) }];
}

/**
* Compiled `agent` steps, by id. Compilation is the only walk that sees them
* all: a YAML helper step (`slack: { post: … }`) is an `agent` step carrying a
* helper envelope and needs the same SDK agent worker, and only the compiler
* lowers it into one. `requirements.harnesses` is the wrong source — it
* deduplicates by harness, counts `llm` use, and omits helper steps entirely.
*
* A spec that does not compile yields no diagnostic rather than a thrown
* check: the compile refusal is owned by the existing preflight path and is
* the whole report in that case.
*/
function agentStepIds(authoring: FlowSpec): string[] {
try {
return compileSpec(authoring).steps.filter((step) => step.type === 'agent').map((step) => step.id);
} catch {
return [];
}
}

function message(steps: readonly string[]): string {
const named = steps.slice(0, NAMED_STEP_LIMIT).map((id) => `"${id}"`).join(', ');
const rest = steps.length - NAMED_STEP_LIMIT;
const list = rest > 0 ? `${named} and ${rest} more` : named;
const subject = steps.length === 1
? `1 agent step (${list}) requires`
: `${steps.length} agent steps (${list}) require`;
return `${subject} an attached worker. For a local run, use \`flows run --local-agent <flow>\`, `
+ 'unless you already attach an agent worker for this daemon. `flows check` does not verify worker '
+ 'attachment: with no worker attached the run parks at the first agent step.';
}
39 changes: 35 additions & 4 deletions packages/sdk/src/cli/check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { dirname, isAbsolute, join, parse as parsePath, resolve } from 'node:pat
import { spawnSync } from 'node:child_process';
import { parse as parseYaml } from 'yaml';
import { CompileError, compileSpec, kernelToAuthoring } from '../compile.js';
import { agentWorkerDiagnostics } from './check-worker-surface.js';
import { helperReady } from '../yaml-helper-effect.js';
import { flowRequirements, type FlowRequirements } from '../flow-requirements.js';
import {
Expand Down Expand Up @@ -117,14 +118,32 @@ export interface CheckExecution {
flow?: FlowSpec;
}

/**
* Facts about the *caller*, not about the spec, that change which diagnostics
* apply. Preflight stays a pure function of the spec plus environment probes;
* anything that depends on how the flow is about to be invoked opts in here.
*/
export interface CheckInvocation {
/**
* Report `agent_worker_unresolved` when the spec has `agent` steps.
*
* Only `flows check` sets this. `flows run` attaches its own worker under
* `--local-agent` and knows the answer, `flows build` and `flows deploy`
* check a spec that will run elsewhere, and an SDK caller that reaches
* `checkAuthoredFlow` directly is generally running a worker already —
* warning any of them would be noise about a question they have answered.
*/
warnUnresolvedAgentWorker?: boolean;
}

export class CheckFailure extends Error {
constructor(readonly kind: CheckFailureKind, message: string) {
super(message);
}
}

/** Validate and preflight one working-tree spec without starting a run. */
export function checkFlow(path: string): CheckExecution {
export function checkFlow(path: string, invocation: CheckInvocation = {}): CheckExecution {
const absolutePath = resolve(path);
try {
const source = readFlowSource(absolutePath);
Expand All @@ -134,7 +153,7 @@ export function checkFlow(path: string): CheckExecution {
: [];
let execution: CheckExecution;
try {
execution = checkAuthoredFlow(readFlow(source, absolutePath), path);
execution = checkAuthoredFlow(readFlow(source, absolutePath), path, undefined, invocation);
} catch (error) {
if (!(error instanceof CheckFailure)) throw error;
execution = { report: inputFailureReport(error, path) };
Expand Down Expand Up @@ -164,7 +183,12 @@ function safeRequirements(authoring: FlowSpec, projectCli: string | undefined):
}

/** Preflight a validated authored flow through the same path as YAML/JSON. */
export function checkAuthoredFlow(authoring: FlowSpec, path: string, projectConfig?: ProjectConfig): CheckExecution {
export function checkAuthoredFlow(
authoring: FlowSpec,
path: string,
projectConfig?: ProjectConfig,
invocation: CheckInvocation = {},
): CheckExecution {
const absolutePath = resolve(path);
try {
const config = projectConfig ?? readProjectConfig(dirname(absolutePath));
Expand Down Expand Up @@ -195,14 +219,21 @@ export function checkAuthoredFlow(authoring: FlowSpec, path: string, projectConf
result.diagnostics.push({ severity: 'warning', kind: 'budget_unmetered',
message: 'Managed communication sessions do not report token or dollar usage. Budget ceilings cannot bound their spend; communication.timeoutMs bounds their duration.' });
}
// Reported whatever preflight concluded. An environment refusal (a missing
// CLI, an unknown model) is fixed and rerun; the worker question is still
// open on the next pass, and staying silent about it here is what made an
// author meet it one dead run at a time.
const workerSurface = invocation.warnUnresolvedAgentWorker === true
? agentWorkerDiagnostics(authoring)
: [];
return {
report: {
ok: result.ok,
path,
...(config.path !== undefined ? { projectConfigPath: config.path } : {}),
gates: result.gates,
resolutions: result.resolutions,
diagnostics: result.diagnostics,
diagnostics: [...result.diagnostics, ...workerSurface],
requirements: safeRequirements(authoring, config.cli),
},
...(result.ok && flow !== undefined ? { flow } : {}),
Expand Down
23 changes: 21 additions & 2 deletions packages/sdk/src/failure-kinds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ export const CHECK_FAILURE_KINDS = [
* (#442). Warning-only by design: the declaration stays legal and the flow
* still runs, but an author who wrote one must not be left believing it
* sandboxes the step.
*
* `gate_path_unscanned` names an `artifact_exists` path inside a prefix the
* bundled agent worker's artifact scan excludes, so the scan can never put it
* in the journaled `output.artifacts` the gate reads. It warns rather than
* refuses because the scan is not the only writer of that list — the same
* worker promotes JSON stdout and completed Relay task output verbatim, and a
* custom worker may journal anything — so such a gate is unproven, not
* unsatisfiable. Temporary: it describes #513, and retires with
* `named-gate-preflight.ts` when the scan stops excluding those paths.
*/
export const PREFLIGHT_WARNING_KINDS = [
'unprovable_effects',
Expand All @@ -83,6 +92,7 @@ export const PREFLIGHT_WARNING_KINDS = [
'vacuous_gate',
'budget_unmetered',
'permissions_unenforced',
'gate_path_unscanned',
] as const;

/**
Expand Down Expand Up @@ -136,8 +146,17 @@ export const RUN_WARNING_KINDS = [
'connection_file_stale',
] as const;

/** File-level editor hints emitted by flows check, outside pure preflight. */
export const CHECK_WARNING_KINDS = ['editor_schema_missing'] as const;
/**
* Warnings emitted by flows check, outside pure preflight.
*
* `editor_schema_missing` is a file-level editor hint. `agent_worker_unresolved`
* is a property of the *invocation*, not of the spec: a spec with `agent`
* steps needs a worker attached for step type `agent`, and only the caller
* knows whether it attaches one. `flows check` attaches none and, being
* daemon-free, can see none either — so it opts in, while `flows run`, `flows
* build` and SDK submissions do not (cli/check-worker-surface.ts).
*/
export const CHECK_WARNING_KINDS = ['editor_schema_missing', 'agent_worker_unresolved'] as const;
export type CheckWarningKind = (typeof CHECK_WARNING_KINDS)[number];

export type PreflightFailureKind = (typeof PREFLIGHT_FAILURE_KINDS)[number];
Expand Down
Loading
Loading