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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ hydrate init Write a pointer to `hydrate guide` into this direct
hydrate projects List the projects on your account (ids for --project)
hydrate fork <name> Fork a working branch from main, bind this directory to it
hydrate branches List your working branches
hydrate show [path] Print a read-only view of a branch's graph
hydrate show [path] [--depth N] Print a read-only view of a branch's graph
hydrate walk <path> Read one node's scoped context (node + neighborhood)
hydrate pull Refresh the local view of the branch's graph
hydrate node add ... Stage a node (behavior or boundary)
Expand Down
3 changes: 3 additions & 0 deletions src/cmd/guide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ Inspecting
hydrate projects list your projects (and the ids for --project)
hydrate branches list the working branches of the selected project
hydrate show [path] read-only view of a branch's graph (optionally a subtree)
hydrate show <path> --depth N read only N levels below <path>, fetching just
that slice instead of the whole branch
hydrate walk <path> read one node's scoped context (node + neighbors);
`--boundary` reads a boundary's children + edges
(it errors on a non-boundary — use the plain walk)

A scriptable agent surface
Every command reads human-friendly on a terminal and machine-readable JSON when
Expand Down
90 changes: 86 additions & 4 deletions src/cmd/scoped.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,12 @@ use crate::state::Index;
/// scoped read — cannot be asserted anywhere.
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum Plan {
/// Read just the slice rooted at this node.
Scoped(Uuid),
/// Read just the slice rooted at this node, with the node's kind as the
/// index recorded it — `None` when the index predates kinds (`node_info`
/// is `#[serde(default)]` for exactly that back-compat) or has no entry.
/// Carried here rather than re-read so the id and the kind come from ONE
/// snapshot; two loads can straddle a concurrent `pull`.
Scoped { id: Uuid, kind: Option<String> },
/// Fetch the whole branch and filter locally, for the stated reason.
WholeGraph(Fallback),
}
Expand Down Expand Up @@ -68,11 +72,28 @@ pub(crate) fn plan(
return Ok(Plan::WholeGraph(Fallback::NoIndex));
};
match index.entries.get(&format!("node:{path}")) {
Some(id) => Ok(Plan::Scoped(*id)),
Some(id) => Ok(Plan::Scoped {
id: *id,
kind: index.node_info.get(id).map(|info| info.kind.clone()),
}),
None => Ok(Plan::WholeGraph(Fallback::PathNotInIndex)),
}
}

/// Kinds this build knows. A token outside this set means the index was
/// written by a newer CLI, so the local check DEFERS rather than rejecting —
/// the same posture [`unaddressable_label`] takes for an unrecognised reason.
/// Guessing here would block a legal request with no way past it.
const KNOWN_KINDS: [&str; 5] = ["behavior", "boundary", "state", "io", "interface"];

/// Whether the index's recorded `kind` is a RECOGNISED non-boundary.
///
/// `false` for a boundary, and `false` for anything this build doesn't know —
/// both mean "don't reject locally; let the server answer".
pub(crate) fn is_known_non_boundary(kind: &str) -> bool {
KNOWN_KINDS.contains(&kind) && kind != "boundary"
}

/// The working-copy root, or `None` when this directory is not one. `show`
/// deliberately works outside a working copy (it takes `--project`), so a
/// missing root is an ordinary state rather than an error.
Expand Down Expand Up @@ -211,7 +232,7 @@ mod tests {
let dir = write_index(&[("node:Api.Rater", id)]);
assert_eq!(
plan(Some(dir.path()), "Api.Rater", true).unwrap(),
Plan::Scoped(id),
Plan::Scoped { id, kind: None },
);
}

Expand Down Expand Up @@ -255,4 +276,65 @@ mod tests {
assert_eq!(sanitize("a\nb"), "a\u{fffd}b");
assert_eq!(sanitize("caf\u{e9}"), "caf\u{e9}");
}

#[test]
fn the_plan_carries_the_kind_from_the_same_snapshot() {
// id and kind must come from ONE read: two loads can straddle a
// concurrent `pull` and combine facts from different snapshots.
let id = Uuid::from_u128(3);
let dir = write_index_with_kind(&[("node:Api", id)], id, "behavior");
match plan(Some(dir.path()), "Api", true).unwrap() {
Plan::Scoped { id: got, kind } => {
assert_eq!(got, id);
assert_eq!(kind.as_deref(), Some("behavior"));
}
other => panic!("expected Scoped, got {other:?}"),
}
}

#[test]
fn an_index_without_node_info_yields_no_kind() {
// `node_info` is #[serde(default)] for back-compat with older pulls.
let id = Uuid::from_u128(4);
let dir = write_index(&[("node:Api", id)]);
match plan(Some(dir.path()), "Api", true).unwrap() {
Plan::Scoped { kind, .. } => assert_eq!(kind, None),
other => panic!("expected Scoped, got {other:?}"),
}
}

#[test]
fn only_recognised_non_boundary_kinds_are_rejected_locally() {
for k in ["behavior", "state", "io", "interface"] {
assert!(is_known_non_boundary(k), "{k} should reject locally");
}
assert!(!is_known_non_boundary("boundary"));
// A kind this build predates must DEFER, not reject — the same posture
// `unaddressable_label` takes for an unrecognised reason. Rejecting
// would block a legal request with no way past it.
assert!(!is_known_non_boundary("hyperboundary"));
assert!(!is_known_non_boundary(""));
}

fn write_index_with_kind(entries: &[(&str, Uuid)], id: Uuid, kind: &str) -> tempfile::TempDir {
let dir = tempfile::tempdir().unwrap();
let hy = dir.path().join(".hydrate");
std::fs::create_dir_all(&hy).unwrap();
let map: std::collections::BTreeMap<String, Uuid> = entries
.iter()
.map(|(k, v)| ((*k).to_string(), *v))
.collect();
let index = serde_json::json!({
"version": 2,
"entries": map,
"node_info": {
id.to_string(): {
"kind": kind, "inputs": [], "outputs": [], "config": []
}
},
"edges": {},
});
std::fs::write(hy.join("index.json"), index.to_string()).unwrap();
dir
}
}
2 changes: 1 addition & 1 deletion src/cmd/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub fn run(
if let (Some(depth), Some(path)) = (args.depth, args.path.as_deref()) {
let on_bound_branch = bound == Some(branch_id);
match scoped::plan(scoped::base_dir().as_deref(), path, on_bound_branch)? {
scoped::Plan::Scoped(node_id) => {
scoped::Plan::Scoped { id: node_id, .. } => {
let subtree = client.fetch_branch_subtree(branch_id, node_id, depth)?;
println!(
"{}",
Expand Down
69 changes: 63 additions & 6 deletions src/cmd/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,30 @@ pub fn run(args: crate::cli::WalkArgs, mode: OutputMode) -> Result<(), CliError>
// pulled index supplies; `walk` always reads the branch it is bound to, so
// the index (which records no branch identity of its own) applies.
match scoped::plan(Some(&base), &args.path, true)? {
scoped::Plan::Scoped(node_id) => {
scoped::Plan::Scoped { id: node_id, kind } => {
// Reject a recognised non-boundary BEFORE the request. The server
// 404s such an id, so the check inside the renderer never runs —
// it is the /boundary route's contract that a non-boundary is not
// found, not a generic property. An UNRECOGNISED kind defers: the
// index may have been written by a newer CLI.
if args.boundary {
if let Some(kind) = kind.as_deref() {
if scoped::is_known_non_boundary(kind) {
return Err(not_a_boundary(&args.path, kind));
}
} else {
// The index resolved the path but carries no kind — an
// index pulled before kinds were recorded. Say so, rather
// than letting the request 404 with no explanation of why
// the local check didn't help.
eprintln!(
"note: this working copy's index has no kind for \
'{}', so --boundary could not be checked locally; \
run `hydrate pull` to refresh it.",
args.path,
);
}
}
let out = if args.boundary {
let cell = client.fetch_branch_boundary(binding.branch_id, node_id)?;
render_boundary_scoped(&cell, &args.path, mode)?
Expand Down Expand Up @@ -118,6 +141,38 @@ fn label_of(
})
}

/// The "you asked for a boundary and this isn't one" error.
///
/// One builder for all three sites (the local preflight, and both renderers'
/// server-data checks) so the guidance cannot drift between the scoped and
/// whole-graph paths — which is the divergence this whole line of work exists
/// to remove.
///
/// `from_index` hedges the claim. The preflight reads a SNAPSHOT: a node's
/// kind is mutable over the wire, so a node that was a behavior at pull time
/// may be a boundary now. Stating it as present-tense fact would refuse a
/// legitimate read while asserting something false, with no remedy named.
fn not_a_boundary_msg(path: &str, kind: &str, from_index: bool) -> String {
let kind = scoped::sanitize(kind);
if from_index {
format!(
"'{path}' is not a boundary — this working copy's index has it as \
a {kind}. Run `hydrate walk {path}` for its neighborhood, or \
`hydrate pull` if the index is behind."
)
} else {
format!(
"'{path}' is not a boundary (it is a {kind}); run \
`hydrate walk {path}` for its neighborhood"
)
}
}

/// The preflight variant: the claim comes from the local index.
fn not_a_boundary(path: &str, kind: &str) -> CliError {
CliError::InvalidArgument(not_a_boundary_msg(path, kind, true))
}

/// The `unaddressable` map as something a consumer can act on.
///
/// The server keys it by node id, but this CLI does not surface ids — and a
Expand Down Expand Up @@ -226,12 +281,14 @@ fn render_boundary_scoped(
mode: OutputMode,
) -> Result<String, CliError> {
if cell.boundary.kind != models::wire_node::Kind::Boundary {
// Same guidance the whole-graph path gives. Without this the quality of
// the error depends on whether an index happens to exist locally.
return Err(CliError::InvalidArgument(format!(
"'{path}' is not a boundary (it is a {}); run `hydrate walk {path}` \
for its neighborhood",
// Defence in depth, and normally unreachable: the /boundary route 404s
// a non-boundary id, so a 200 body should always be one. Kept because
// it is the only check that does not depend on a local index — if the
// route's contract ever widens, this is what still catches a mismatch.
return Err(CliError::InvalidArgument(not_a_boundary_msg(
path,
view::kind_str(cell.boundary.kind),
false,
)));
}
let paths = &cell.paths;
Expand Down
137 changes: 137 additions & 0 deletions tests/scoped_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,25 @@ fn serve_once(body: &'static str) -> (String, mpsc::Receiver<String>) {
(addr, rx)
}

/// A working copy whose index also records the node's KIND, as a real pull does.
fn workdir_with_kind(kind: &str) -> tempfile::TempDir {
let dir = workdir();
std::fs::write(
dir.path().join(".hydrate").join("index.json"),
serde_json::json!({
"version": 2,
"entries": { "node:Api": NODE_ID },
"node_info": {
NODE_ID: { "kind": kind, "inputs": [], "outputs": [], "config": [] }
},
"edges": {},
})
.to_string(),
)
.unwrap();
dir
}

/// A working copy bound to BOUND_BRANCH whose index knows `Api` -> NODE_ID.
fn workdir() -> tempfile::TempDir {
let dir = tempfile::tempdir().unwrap();
Expand Down Expand Up @@ -159,3 +178,121 @@ fn without_an_index_walk_falls_back_to_the_whole_graph_read() {
"with no index the whole-graph read is correct, got: {request}"
);
}

#[test]
fn walk_boundary_on_a_behavior_fails_before_making_a_request() {
// The server 404s a non-boundary id, so a check that runs on the RESPONSE
// can never fire — the user just gets `service error (404)`. The guard has
// to preempt the request, and the way to prove it did is that no request
// ever arrives.
let (addr, rx) = serve_once("{}");
let dir = workdir_with_kind("behavior");
let out = Command::new(env!("CARGO_BIN_EXE_hydrate"))
.args(["walk", "Api", "--boundary"])
.current_dir(dir.path())
.env("HYD_BASE_URL", &addr)
.env("HYD_API_KEY", "test-key-not-a-real-credential")
.output()
.expect("binary should run");

// Pin the CONTRACT, not the phrasing: it must name the problem, the
// remedy, and — because this verdict comes from a snapshot — that the
// index may be behind.
let stderr = String::from_utf8_lossy(&out.stderr);
assert!(stderr.contains("is not a boundary"), "got: {stderr}");
assert!(
stderr.contains("hydrate walk Api"),
"must name the remedy: {stderr}"
);
assert!(
stderr.contains("hydrate pull"),
"must admit the index may be behind: {stderr}"
);
assert_eq!(
out.status.code(),
Some(1),
"stable exit code for a bad argument"
);
assert!(
out.stdout.is_empty(),
"the error belongs on stderr so piped stdout stays parseable"
);
assert!(
rx.recv_timeout(std::time::Duration::from_millis(750))
.is_err(),
"the guard must fire BEFORE the request, but one was sent"
);
}

#[test]
fn walk_boundary_on_a_real_boundary_still_requests() {
let body = r#"{"version":"v1","project_id":"33333333-3333-3333-3333-333333333333",
"branch":{"id":"11111111-1111-1111-1111-111111111111","version":1},
"boundary":{"id":"22222222-2222-2222-2222-222222222222","kind":"boundary",
"parent_id":null,"position":{"x":0,"y":0},
"data":{"name":"Api","description":"","status":"idle",
"is_test_node":false,"is_external":false}},
"children":[],"edges":[],
"paths":{"22222222-2222-2222-2222-222222222222":"Api"},
"unaddressable":{}}"#;
let (addr, rx) = serve_once(body);
let dir = workdir_with_kind("boundary");
run_walk(&dir, &addr, &["walk", "Api", "--boundary"]);
let request = rx.recv_timeout(std::time::Duration::from_secs(10)).unwrap();
assert!(request.contains("/boundary/"), "got: {request}");
}

#[test]
fn an_index_without_kinds_defers_to_the_server() {
// `node_info` is #[serde(default)] so an index pulled by an older CLI
// still loads. It resolves the path but knows no kind — the request must
// still go out, and the user must be told why the local check was skipped.
let body = r#"{"version":"v1","project_id":"33333333-3333-3333-3333-333333333333",
"branch":{"id":"11111111-1111-1111-1111-111111111111","version":1},
"boundary":{"id":"22222222-2222-2222-2222-222222222222","kind":"boundary",
"parent_id":null,"position":{"x":0,"y":0},
"data":{"name":"Api","description":"","status":"idle",
"is_test_node":false,"is_external":false}},
"children":[],"edges":[],
"paths":{"22222222-2222-2222-2222-222222222222":"Api"},
"unaddressable":{}}"#;
let (addr, rx) = serve_once(body);
let dir = workdir(); // index has `entries` but an empty `node_info`
let out = Command::new(env!("CARGO_BIN_EXE_hydrate"))
.args(["walk", "Api", "--boundary"])
.current_dir(dir.path())
.env("HYD_BASE_URL", &addr)
.env("HYD_API_KEY", "test-key-not-a-real-credential")
.output()
.expect("binary should run");

let request = rx.recv_timeout(std::time::Duration::from_secs(10)).unwrap();
assert!(request.contains("/boundary/"), "must defer, got: {request}");
let stderr = String::from_utf8_lossy(&out.stderr);
assert!(
stderr.contains("no kind for") && stderr.contains("hydrate pull"),
"the skipped local check must be reported: {stderr}"
);
}

#[test]
fn an_unrecognised_kind_defers_rather_than_rejecting() {
// An index written by a NEWER CLI can carry a kind this build predates.
// Rejecting it would block a legal request with no way past; the server
// is the authority on what its own route accepts.
let (addr, rx) = serve_once("{}");
let dir = workdir_with_kind("hyperboundary");
Command::new(env!("CARGO_BIN_EXE_hydrate"))
.args(["walk", "Api", "--boundary"])
.current_dir(dir.path())
.env("HYD_BASE_URL", &addr)
.env("HYD_API_KEY", "test-key-not-a-real-credential")
.output()
.expect("binary should run");

let request = rx.recv_timeout(std::time::Duration::from_secs(10)).unwrap();
assert!(
request.contains("/boundary/"),
"an unknown kind must defer to the server, got: {request}"
);
}