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
59 changes: 59 additions & 0 deletions docs/SURFACE.md
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,65 @@ flow-wide `FlowHeader.workspace` / `tools.fs` scopes. The chief harness above
remains an aspirational example; this option does not make that entire harness
executable today.

### Per-agent working directory

`AgentOptions.cwd` — and the `cwd:` key on a declarative `type: agent` step —
names the directory the agent's CLI is spawned in. It is how one flow drives
agents in several checkouts:

```ts
const api = await f.agent("api", {
task: "Apply the rename in the API checkout.",
cwd: "checkouts/service-a",
});
const web = await f.agent("web", {
task: "Apply the matching rename in the web checkout.",
cwd: "checkouts/service-b",
});
```

**The path is relative to the run root** — the working directory `flows run`
was invoked from, which is also the directory an agent runs in when `cwd` is
absent, and the uploaded tree on the Cloud path (docs/CLOUD.md, "Code sync").
A relative declaration is the same declaration on every host; an absolute one
names a place a different machine does not have, so absolute paths are refused
rather than resolved. The authored runner makes one exception at its own
edge: an absolute `options.cwd` that names a directory inside this process's
run root is lowered to the equivalent relative declaration, and one that
escapes the root is refused — the spec never carries an absolute path either
way.

Two checks answer two different questions:

- **The declaration** is checked lexically, with no filesystem access, by
`flows check`, by the authored runner, and by the kernel — the same rule in
all three, so a spec that passes the first is not refused by the last. A
`cwd` must be a nonempty string, must not be absolute or URI-like, must not
carry surrounding whitespace or a NUL, and must have no empty, `.` or `..`
components. `cwd: null` is a refusal, not "no directory". `cwd` is an agent
field: a `deterministic` or `llm` step that declares it is `invalid_spec`.
- **The target** is resolved at dispatch by the worker that spawns the CLI,
the one process that provably shares the agent's filesystem. It must be an
existing directory whose symlink-free path lies inside the symlink-free run
root. A symlink out of the tree, and a sibling whose name merely starts with
the run root's, are both outside it. A step whose directory does not resolve
completes `worker_error` with the reason journaled, and no CLI is spawned.

`AgentResult.artifacts` are reported relative to this directory, so each agent
above reports paths within its own checkout.

`cwd` is **not supported with `transport: "relay"`** and is refused when both
are declared: the relay agent runs on another host, where this process can
neither resolve the directory nor hold it inside the run root.

This is a declaration of where an agent starts, not a sandbox. Nothing stops a
CLI from reading or writing outside the directory it was spawned in. Per-step
scoping is `permissions`, which is recorded and not enforced (gate 8 / #442).

Workspace surfaces are a different thing and are not a directory selector:
`surfaces.workspace` names the revisions a step pins and writes back to, not a
path on the host running the CLI.

### Supported TypeScript LLM calls

The local authored executor supports these signatures:
Expand Down
84 changes: 66 additions & 18 deletions kernel/relayflowd-core/src/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,15 +163,15 @@ impl RunSpec {
if cli.as_ref().is_some_and(|value| value.trim().is_empty()) {
return Err(SpecError::EmptyStepCli(step.id.clone()));
}
if let StepKind::Agent { cwd: Some(cwd), .. } = &step.kind
&& !cwd.starts_with('/')
{
return Err(SpecError::RelativeStepCwd {
step: step.id.clone(),
cwd: cwd.clone(),
});
}
if let StepKind::Agent { surfaces, .. } = &step.kind {
if let StepKind::Agent { surfaces, cwd, .. } = &step.kind {
if let Some(cwd) = cwd
&& !is_run_root_relative_path(cwd)
{
return Err(SpecError::InvalidAgentCwd {
step: step.id.clone(),
cwd: cwd.clone(),
});
}
for workspace in &surfaces.workspace {
if path_surface_identity(&workspace.surface).is_none() {
return Err(SpecError::InvalidWorkspaceSurface {
Expand Down Expand Up @@ -272,6 +272,28 @@ pub(crate) fn path_surface_identity(path: &str) -> Option<(String, Vec<String>)>
.then_some((namespace, components))
}

/// Whether a declared agent `cwd` names a place inside the run's own tree.
///
/// The same lexical canonical rule surfaces get — no whitespace aliases,
/// empty components, `.` or `..` — narrowed to relative paths only, because
/// the declaration is read against the run root the worker executes under.
/// An absolute path or a URI-like mount identity names a place the run root
/// does not contain, and a `..` component walks out of it; both are refused
/// here rather than discovered on the host. NUL cannot occur in a host path
/// component, so a declaration carrying one can only mislead a reader.
fn is_run_root_relative_path(path: &str) -> bool {
// U+FEFF is named explicitly because JavaScript's `\s` includes it and
// `char::is_whitespace` does not; the SDK names U+0085 for the mirror
// reason. Both sides then refuse `White_Space ∪ {U+FEFF}` at the edges,
// so a padded declaration cannot pass `flows check` and be refused here.
let padding = |c: char| c.is_whitespace() || c == '\u{feff}';
!path.contains('\0')
&& !path.starts_with(padding)
&& !path.ends_with(padding)
&& matches!(path_surface_identity(path), Some((namespace, components))
if namespace.is_empty() && !components.is_empty())
}

pub fn is_canonical_external_surface(path: &str) -> bool {
path_surface_identity(path).is_some()
}
Expand Down Expand Up @@ -321,8 +343,8 @@ const STEP_AGENT_FIELDS: &[&str] = &[
"instruction",
"cli",
"model",
"transport",
"cwd",
"transport",
"recovery_mode",
"surfaces",
"permissions",
Expand All @@ -339,7 +361,25 @@ fn reject_unknown_step_fields(value: &Value) -> Result<(), SpecError> {
let kind_fields = match object.get("type").and_then(Value::as_str) {
Some("deterministic") => STEP_DETERMINISTIC_FIELDS,
Some("llm") => STEP_LLM_FIELDS,
Some("agent") => STEP_AGENT_FIELDS,
Some("agent") => {
// `cwd` deserializes into `Option<String>`, where serde reads
// an explicit null as absence — the step would then run in the
// default directory under a spec that declared otherwise. The
// shape is checked before serde so a null, a number or an
// object is refused instead of silently defaulted.
if let Some(cwd) = object.get("cwd")
&& !cwd.is_string()
{
return Err(SpecError::Malformed(format!(
"step {}: cwd must be a string",
object
.get("id")
.and_then(Value::as_str)
.unwrap_or("?")
)));
}
STEP_AGENT_FIELDS
}
// Missing/unknown type is rejected by serde's tagged-enum error.
_ => continue,
};
Expand Down Expand Up @@ -461,16 +501,22 @@ pub enum StepKind {
/// then handed to the worker, which surfaces it to the CLI.
#[serde(default, skip_serializing_if = "Option::is_none")]
model: Option<String>,
/// Directory the attached worker spawns the declared CLI in, relative
/// to the run root the worker executes under. The kernel performs no
/// I/O: it checks the declaration lexically (the same canonical-path
/// rule surfaces get, plus "relative, so it names a place inside the
/// run's tree") and carries it verbatim, so the choice is part of the
/// run's record rather than ambient host state. Resolving it against a
/// real filesystem — existence, symlinks, containment — belongs to the
/// worker that spawns the CLI, the one process that shares that
/// filesystem. Absent means the run root itself.
#[serde(default, skip_serializing_if = "Option::is_none")]
cwd: Option<String>,
/// How the attached worker invokes the declared CLI. The kernel does
/// not implement either transport; it journals and dispatches the
/// choice so the worker can honor it deterministically.
#[serde(default, skip_serializing_if = "Option::is_none")]
transport: Option<AgentTransport>,
/// Absolute working directory the worker starts the CLI in. Carried
/// and dispatched like `model`: the kernel never enters it, and
/// omitting it serializes the step exactly as before.
#[serde(default, skip_serializing_if = "Option::is_none")]
cwd: Option<String>,
#[serde(default)]
recovery_mode: RecoveryMode,
/// Declared mutable surfaces (RFC Appendix A rule 1) — names only.
Expand Down Expand Up @@ -781,8 +827,10 @@ pub enum SpecError {
EmptyStepId,
#[error("step {0} cli cannot be empty")]
EmptyStepCli(String),
#[error("agent step {step} cwd must be an absolute path, got {cwd:?}")]
RelativeStepCwd { step: String, cwd: String },
#[error(
"agent step {step} declares working directory {cwd:?}, which is not a run-root-relative path (no absolute paths, empty components, \".\" or \"..\")"
)]
InvalidAgentCwd { step: String, cwd: String },
#[error("agent step {step} declares non-canonical external surface {path:?}")]
InvalidExternalSurface { step: String, path: String },
#[error("agent step {step} declares non-canonical workspace surface {surface:?}")]
Expand Down
18 changes: 9 additions & 9 deletions kernel/relayflowd-core/src/spec/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,33 +417,33 @@ fn the_default_policy_is_absent_from_the_serialized_boundary_spec() {
}

#[test]
fn agent_cwd_is_carried_and_must_be_absolute() {
fn agent_cwd_is_carried_and_must_be_run_root_relative() {
let with_cwd = RunSpec::parse(&json!({
"steps": [{"id": "a", "type": "agent", "instruction": "i", "cwd": "/repo/.wt/a"}]
"steps": [{"id": "a", "type": "agent", "instruction": "i", "cwd": ".wt/a"}]
}))
.unwrap();
assert!(with_cwd.validate().is_ok());
assert_eq!(
serde_json::to_value(&with_cwd.steps[0]).unwrap()["cwd"],
json!("/repo/.wt/a")
json!(".wt/a")
);

let relative = RunSpec::parse(&json!({
"steps": [{"id": "a", "type": "agent", "instruction": "i", "cwd": "wt/a"}]
let absolute = RunSpec::parse(&json!({
"steps": [{"id": "a", "type": "agent", "instruction": "i", "cwd": "/repo/.wt/a"}]
}))
.unwrap();
assert_eq!(
relative.validate(),
Err(SpecError::RelativeStepCwd {
absolute.validate(),
Err(SpecError::InvalidAgentCwd {
step: "a".to_owned(),
cwd: "wt/a".to_owned()
cwd: "/repo/.wt/a".to_owned()
})
);

// `cwd` is agent-only: an llm step still refuses it as an unknown field.
assert!(matches!(
RunSpec::parse(&json!({
"steps": [{"id": "a", "type": "llm", "prompt": "p", "cwd": "/repo"}]
"steps": [{"id": "a", "type": "llm", "prompt": "p", "cwd": "repo"}]
})),
Err(SpecError::UnknownField { .. })
));
Expand Down
89 changes: 89 additions & 0 deletions kernel/relayflowd-core/tests/spec_parity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,92 @@ fn placement_declaration_acceptance_matches_the_sdk_corpus() {
}
}
}

/// flows#357: `cwd` reached the daemon as `unknown field "cwd" at steps[0]`
/// after `flows check` had already passed, because the SDK lowered a field the
/// kernel's closed agent schema did not name. The canonical bytes and hash of a
/// flow that declares it — on two steps and not on a third — are now pinned on
/// both sides of the boundary.
#[test]
fn agent_working_directories_have_identical_canonical_bytes_and_hash() {
assert_parity(
include_str!("../../../testdata/agent-cwd.spec.canonical.json"),
include_str!("../../../testdata/agent-cwd.spec.sha256"),
);
}

#[test]
fn agent_cwd_declaration_acceptance_matches_the_sdk_corpus() {
let cases: Vec<Value> =
serde_json::from_str(include_str!("../../../testdata/agent-cwd-cases.json")).unwrap();
for case in cases {
let spec = serde_json::json!({"steps":[{
"id":"s","type":"agent","instruction":"work","cwd":case["cwd"],
}]});
let accepted = RunSpec::parse(&spec)
.and_then(|spec| spec.validate())
.is_ok();
assert_eq!(
accepted,
case["valid"].as_bool().unwrap(),
"{}",
case["name"]
);
}
}

/// An absent `cwd` is absent in the re-serialized spec, not `"cwd":null`: every
/// fixture committed before this field existed keeps its bytes and its hash.
#[test]
fn an_undeclared_agent_cwd_is_not_serialized() {
let value = serde_json::json!({
"steps": [{"id": "agent", "type": "agent", "instruction": "work"}],
});
let parsed = RunSpec::parse(&value).expect("an agent step without cwd must parse");
parsed.validate().expect("and must validate");
assert!(
serde_json::to_value(parsed).unwrap()["steps"][0]
.get("cwd")
.is_none()
);
}

/// `Option<String>` reads an explicit null as absence, which would run the step
/// in the default directory under a spec that declared otherwise. The shape is
/// checked before serde so every non-string spelling fails closed.
#[test]
fn a_non_string_agent_cwd_fails_closed_rather_than_defaulting() {
for cwd in [
serde_json::json!(null),
serde_json::json!(7),
serde_json::json!(["checkouts/service-a"]),
serde_json::json!({"path": "checkouts/service-a"}),
serde_json::json!(true),
] {
let value = serde_json::json!({
"steps": [{"id": "agent", "type": "agent", "instruction": "work", "cwd": cwd}],
});
let error = RunSpec::parse(&value).expect_err("a non-string cwd must fail closed");
assert!(
error.to_string().contains("cwd must be a string"),
"{cwd}: {error}"
);
}
}

/// `cwd` is agent-only, and the near-miss spelling is still an unknown field:
/// widening one verb's schema must not quietly widen the others or the name.
#[test]
fn agent_cwd_is_not_accepted_on_other_verbs_or_under_another_name() {
for step in [
serde_json::json!({"id":"s","type":"deterministic","command":"true","cwd":"checkouts/a"}),
serde_json::json!({"id":"s","type":"llm","prompt":"work","cwd":"checkouts/a"}),
serde_json::json!({"id":"s","type":"agent","instruction":"work","cwdd":"checkouts/a"}),
serde_json::json!({"id":"s","type":"agent","instruction":"work","worker_cwd":"checkouts/a"}),
] {
assert!(
RunSpec::parse(&serde_json::json!({"steps": [step.clone()]})).is_err(),
"{step} must fail closed"
);
}
}
4 changes: 2 additions & 2 deletions packages/schema/flows.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1415,7 +1415,7 @@
},
"cwd": {
"title": "cwd",
"description": "Working directory for the CLI subprocess; defaults to the flow-runner's cwd.",
"description": "Directory the declared CLI is spawned in, as a path relative to the run\nroot — the flow-runner's working directory, which is also where the CLI\nruns when this is absent. Absolute paths, `.`, `..` and empty components\nare refused lexically by `flows check` and by the kernel; the worker that\nspawns the CLI additionally requires the symlink-free directory to exist\ninside the symlink-free run root. A declaration, not a sandbox: nothing\nstops a CLI from writing outside it. Not supported with\n`transport: 'relay'`, where the agent runs on a host this process cannot\nresolve. See docs/SURFACE.md.",
"type": "string"
},
"transport": {
Expand Down Expand Up @@ -2434,7 +2434,7 @@
},
"cwd": {
"title": "cwd",
"description": "Working directory for the CLI subprocess; kernel passes through untouched.",
"description": "Run-root-relative directory the attached worker spawns the CLI in. The\nkernel checks the shape and does no I/O: existence and containment are\ndecided by the worker, on the host that shares the agent's filesystem.",
"type": "string"
},
"transport": {
Expand Down
Loading
Loading