diff --git a/README.md b/README.md index 7954107..f136438 100644 --- a/README.md +++ b/README.md @@ -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 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 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) diff --git a/src/cmd/guide.rs b/src/cmd/guide.rs index 0bbbb96..de55c6f 100644 --- a/src/cmd/guide.rs +++ b/src/cmd/guide.rs @@ -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 --depth N read only N levels below , fetching just + that slice instead of the whole branch hydrate walk 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 diff --git a/src/cmd/scoped.rs b/src/cmd/scoped.rs index 730160a..3b3857e 100644 --- a/src/cmd/scoped.rs +++ b/src/cmd/scoped.rs @@ -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 }, /// Fetch the whole branch and filter locally, for the stated reason. WholeGraph(Fallback), } @@ -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. @@ -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 }, ); } @@ -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 = 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 + } } diff --git a/src/cmd/show.rs b/src/cmd/show.rs index f0d487c..b80d061 100644 --- a/src/cmd/show.rs +++ b/src/cmd/show.rs @@ -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!( "{}", diff --git a/src/cmd/walk.rs b/src/cmd/walk.rs index a13f325..42379f0 100644 --- a/src/cmd/walk.rs +++ b/src/cmd/walk.rs @@ -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)? @@ -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 @@ -226,12 +281,14 @@ fn render_boundary_scoped( mode: OutputMode, ) -> Result { 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; diff --git a/tests/scoped_request.rs b/tests/scoped_request.rs index f3ff88e..7e4cff2 100644 --- a/tests/scoped_request.rs +++ b/tests/scoped_request.rs @@ -43,6 +43,25 @@ fn serve_once(body: &'static str) -> (String, mpsc::Receiver) { (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(); @@ -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}" + ); +}