From f18a17eab10f9ae2f6fb42f4fc0f7567c3b30aa4 Mon Sep 17 00:00:00 2001 From: Matt Yeazel Date: Thu, 27 Aug 2026 16:10:04 -0700 Subject: [PATCH 1/3] :broom: chore: keep the cargo-native clippy gate green on current stable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stable 1.98's clippy fails the workspace on three pre-existing sites: an excessive-nesting block in the plugin dry run (flattened into the loop's own filter), the reap-poll in the upgrade probe tests (the if-break flattened into a while over the surviving pids), and the seven-argument launch-plan builder (allowed, with the reason recorded — its inputs are genuinely independent and a one-caller args struct would be ceremony). No behavior changes. --- crates/tapesctl/src/plugin.rs | 20 +++++++++++--------- crates/tapesctl/src/start/mod.rs | 4 ++++ crates/tapesctl/src/upgrade/artifact.rs | 19 ++++++++----------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/crates/tapesctl/src/plugin.rs b/crates/tapesctl/src/plugin.rs index a33becf..4cc2f80 100644 --- a/crates/tapesctl/src/plugin.rs +++ b/crates/tapesctl/src/plugin.rs @@ -311,15 +311,17 @@ fn run_in(args: &PluginInstallArgs, machine: &Machine) -> Result<()> { // be describing a different operation than the one it stands in // for. Only what is actually there is listed — an absent superseded // copy is nothing this run would do. - for superseded in artifact.superseded_paths(home) { - if superseded.exists() { - println!( - "tapesctl: would remove superseded {} (loaded alongside {} by {})", - superseded.display(), - artifact.file_name(), - harness.id(), - ); - } + for superseded in artifact + .superseded_paths(home) + .into_iter() + .filter(|superseded| superseded.exists()) + { + println!( + "tapesctl: would remove superseded {} (loaded alongside {} by {})", + superseded.display(), + artifact.file_name(), + harness.id(), + ); } } return Ok(()); diff --git a/crates/tapesctl/src/start/mod.rs b/crates/tapesctl/src/start/mod.rs index e92bcf7..cecf5fc 100644 --- a/crates/tapesctl/src/start/mod.rs +++ b/crates/tapesctl/src/start/mod.rs @@ -539,6 +539,10 @@ impl Harness { /// session, so it has nothing to prove — but it is a parameter rather than /// generated inside so the same value ends up in the launched environment /// and in the proxy that validates the echo. + // Seven arguments because a launch plan genuinely has seven independent + // inputs; bundling them into a struct would name a type with exactly one + // constructor and one caller, which is ceremony rather than clarity. + #[allow(clippy::too_many_arguments)] pub fn plan( self, endpoint: ProxyEndpoint, diff --git a/crates/tapesctl/src/upgrade/artifact.rs b/crates/tapesctl/src/upgrade/artifact.rs index 8627341..60252cf 100644 --- a/crates/tapesctl/src/upgrade/artifact.rs +++ b/crates/tapesctl/src/upgrade/artifact.rs @@ -862,23 +862,20 @@ mod tests { }; let pid = read_pid(&pid_file, "the probe child"); let descendant = read_pid(&descendant_file, "the probe child's descendant"); + // Signal 0 probes existence without an external `kill` binary, which + // a minimal container may not ship. A zombie still counts as existing + // until it is reaped. + let is_alive = |p: &i32| unsafe { libc::kill(*p, 0) } == 0; let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); - loop { - // Signal 0 probes existence without an external `kill` binary, - // which a minimal container may not ship. A zombie still counts - // as existing until it is reaped. - let alive: Vec = [pid, descendant] - .into_iter() - .filter(|&p| unsafe { libc::kill(p, 0) } == 0) - .collect(); - if alive.is_empty() { - break; - } + let mut alive = vec![pid, descendant]; + alive.retain(is_alive); + while !alive.is_empty() { assert!( std::time::Instant::now() < deadline, "probe processes {alive:?} still running after the probe timed out" ); tokio::time::sleep(std::time::Duration::from_millis(100)).await; + alive.retain(is_alive); } } From 8321048daeb172f90ed4c5f5b7f2b3506c29e429 Mon Sep 17 00:00:00 2001 From: Matt Yeazel Date: Wed, 26 Aug 2026 16:23:26 -0700 Subject: [PATCH 2/3] =?UTF-8?q?=E2=9C=A8=20feat:=20generic=20--filter=20ke?= =?UTF-8?q?y=3Dvalue=20passthrough=20on=20sessions=20list?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Repeatable --filter key=value maps to ?key=value through the sealed sessions method — param names are data, so claimed params from any cassette work without recompiling. No label noun is added: the discovery-generated cassette surface already drives any labels-like cassette with zero compiled-in knowledge, and an integration test now pins that property. --- Cargo.lock | 2 - Cargo.toml | 9 +- crates/tapesctl/src/api/mod.rs | 44 +++- crates/tapesctl/src/cli.rs | 11 + crates/tapesctl/src/error.rs | 12 + .../tests/filter_passthrough_integration.rs | 211 ++++++++++++++++++ docs/commands.md | 3 +- 7 files changed, 287 insertions(+), 5 deletions(-) create mode 100644 crates/tapesctl/tests/filter_passthrough_integration.rs diff --git a/Cargo.lock b/Cargo.lock index 7096a16..1fbc9ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1845,8 +1845,6 @@ dependencies = [ [[package]] name = "tapes-client" version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bd1bf95f204069f3282e09f47e290eb9e3b06612147bc7b9c811f0581d094334" dependencies = [ "clap", "dirs", diff --git a/Cargo.toml b/Cargo.toml index 4357be3..5fe9551 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -103,7 +103,14 @@ tapes-capture = { version = "0.1" } # is built at runtime from a document this binary has never seen, which is the # one surface that cannot be written by hand. `direct-http` is `DirectHttp`, # the no-redirect transport this CLI talks to a tapes server with. -tapes-client = { version = "0.4", features = [ +# RECONCILIATION: temporarily a path override onto the sibling checkout — the +# documented co-development loan (see the escape-hatch note above and +# `scripts/check-tapes-pins.sh`). The generic claimed-filter-param support on +# the sealed sessions method is committed there and not yet published; repoint +# this entry at the next released `tapes-client` (>= the release carrying +# `CoreClient::call_with_claimed`) once it exists, restoring the +# `version = "0.4"` registry form. +tapes-client = { path = "../tapes-harnesses/crates/tapes-client", features = [ "cli", "direct-http", ] } diff --git a/crates/tapesctl/src/api/mod.rs b/crates/tapesctl/src/api/mod.rs index 10c5df3..7836c26 100644 --- a/crates/tapesctl/src/api/mod.rs +++ b/crates/tapesctl/src/api/mod.rs @@ -78,6 +78,27 @@ fn payload_of(raw: Option<&str>) -> Result> { } } +/// Split repeatable `--filter key=value` flags into wire pairs. +/// +/// Only the flag's own grammar is checked — there must be a `=`, with a +/// non-empty key before it. The key is data: cassettes claim filter params on +/// the sessions listing at runtime, so which names mean anything is decided +/// by the deployment at request time, and validating names here would only +/// make this binary disagree with the server it talks to. Refusing the +/// malformed spelling here rather than sending it costs no round trip and +/// names the expected shape. +fn parse_filters(flags: &[String]) -> Result> { + flags + .iter() + .map(|flag| { + flag.split_once('=') + .filter(|(key, _)| !key.is_empty()) + .map(|(key, value)| (key.to_owned(), value.to_owned())) + .context(error::InvalidFilterFlagSnafu { flag: flag.clone() }) + }) + .collect() +} + /// Print a JSON document the way every read command does. pub fn print_json(value: &serde_json::Value) -> Result<()> { let rendered = serde_json::to_string_pretty(value).context(error::RenderJsonSnafu)?; @@ -90,6 +111,7 @@ pub async fn sessions(command: SessionsCommand) -> Result<()> { match command { SessionsCommand::List(args) => { let client = resolve_client(&args.api)?; + let claimed = parse_filters(&args.filter)?; let mut values = SessionListParams { limit: args.limit.map(narrow), cursor: args.cursor, @@ -110,7 +132,13 @@ pub async fn sessions(command: SessionsCommand) -> Result<()> { if let Some(direction) = args.direction { values.push(("direction", direction)); } - let value: Value = client.call(ops::LIST_SESSIONS, values).await?; + // Claimed pairs ride the sealed method's own channel: appended to + // the query after the declared parameters, verbatim and in order, + // with the response passed through untouched. No client-side + // filtering — server-side fail-open governs what a key means. + let value: Value = client + .call_with_claimed(ops::LIST_SESSIONS, values, &claimed) + .await?; if args.json { print_json(&value) } else { @@ -217,10 +245,24 @@ mod tests { harness_session_id: None, harness_id: None, auth_subject: None, + filter: Vec::new(), json: false, } } + #[tokio::test] + async fn a_malformed_filter_flag_fails_before_any_request() { + // No `=` means no pair to send; the refusal happens here, with the + // expected shape named, rather than as a server round trip. + let server = MockServer::start().await; + let mut args = sessions_list_args(server.uri()); + args.filter = vec!["no-equals".to_owned()]; + let result = sessions(SessionsCommand::List(args)).await; + + assert!(result.is_err(), "got: {result:?}"); + assert!(server.received_requests().await.unwrap().is_empty()); + } + #[test] fn a_manually_constructed_missing_url_is_an_error() { // Normal CLI parsing supplies the localhost default. This covers the diff --git a/crates/tapesctl/src/cli.rs b/crates/tapesctl/src/cli.rs index 902fc51..31c9252 100644 --- a/crates/tapesctl/src/cli.rs +++ b/crates/tapesctl/src/cli.rs @@ -572,6 +572,17 @@ pub struct SessionsListArgs { #[arg(long)] pub auth_subject: Option, + /// Extra filter, repeatable, as `key=value`; sent through as `?key=value`. + /// + /// The param name is data, not a name this binary knows: a deployment's + /// cassettes can claim extra filter params on the sessions listing at + /// runtime, so which keys mean anything is the server's to decide. Pairs + /// are passed through verbatim and in order — nothing is validated, + /// normalized, or filtered client-side — and a key no admitted cassette + /// claims is ignored by the server. + #[arg(long = "filter", value_name = "KEY=VALUE")] + pub filter: Vec, + /// Print the raw JSON response instead of the table. /// /// `sessions list` renders a table by default; this restores the diff --git a/crates/tapesctl/src/error.rs b/crates/tapesctl/src/error.rs index 08dfd0f..0b79846 100644 --- a/crates/tapesctl/src/error.rs +++ b/crates/tapesctl/src/error.rs @@ -44,6 +44,18 @@ pub enum Error { schema: String, }, + /// A `--filter` flag that does not spell `key=value`. + /// + /// The only thing checked is the flag's own grammar — a `=` with a + /// non-empty key before it. The key itself travels as data: which params + /// mean anything is the server's question, so there is nothing else this + /// binary could validate without guessing at a deployment's cassettes. + #[snafu(display("invalid --filter {flag:?} (expected key=value)"))] + InvalidFilterFlag { + /// What the user typed. + flag: String, + }, + /// `--schema` was passed for a harness that speaks exactly one schema. /// /// Refused rather than ignored: a flag that silently does nothing reads, diff --git a/crates/tapesctl/tests/filter_passthrough_integration.rs b/crates/tapesctl/tests/filter_passthrough_integration.rs new file mode 100644 index 0000000..4fb40d7 --- /dev/null +++ b/crates/tapesctl/tests/filter_passthrough_integration.rs @@ -0,0 +1,211 @@ +//! The generic claimed-param passthrough, and the discovery-only cassette path. +//! +//! Both tests run the **actual binary** against a mock tapes server: argv in, +//! stdout/stderr/exit code out, never the crate's internals. What they pin is +//! one boundary from two sides: +//! +//! - `--filter key=value` maps to `?key=value` with the param name treated as +//! **data** — a key some deployment's cassette claims at runtime, never a +//! name compiled into this binary — and the response is rendered untouched. +//! - The runtime-discovered `cassettes ` surface is this +//! binary's complete access path to any cassette's API. The mock cassette +//! below is *named* `labels` precisely because this repository compiles in +//! no such noun: every occurrence of that word in this file is fixture data +//! or a value typed at the command line. + +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use std::path::Path; +use std::process::Output; + +use serde_json::json; +use wiremock::matchers::{method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +/// Run the real binary against `api_url`, with a private cassette cache and +/// the ambient `TAPES_*` overrides a developer might have exported cleared — +/// these read from the environment by design, and an exported value would +/// change what the test runs. +async fn run_tapesctl(cache_dir: &Path, api_url: &str, args: &[&str]) -> Output { + let mut command = tokio::process::Command::new(env!("CARGO_BIN_EXE_tapesctl")); + command + .env("TAPESCTL_CACHE_DIR", cache_dir) + .env("TAPES_API_URL", api_url) + .env_remove("RUST_LOG") + .env_remove("TAPES_INGEST_URL") + .args(args); + command.output().await.unwrap() +} + +#[tokio::test] +async fn filter_flags_pass_claimed_params_through_generically() { + let cache_dir = tempfile::tempdir().unwrap(); + let server = MockServer::start().await; + // The response carries a field this build has never heard of, so the + // untouched-rendering assertion below cannot pass through a typed model. + Mock::given(method("GET")) + .and(path("/v1/sessions")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "items": [{"id": "s-1", "a_field_from_the_future": 7}], + "next_cursor": "" + }))) + .mount(&server) + .await; + + // `label` below is a value typed at the command line — the name of a + // param the mock deployment's cassette would claim — not a flag or noun + // this binary defines. + let uri = server.uri(); + let out = run_tapesctl( + cache_dir.path(), + &uri, + &[ + "sessions", + "list", + "--filter", + "label=a", + "--filter", + "label=b", + "--json", + "--api-url", + &uri, + ], + ) + .await; + assert!(out.status.success(), "tapesctl failed: {out:?}"); + + // The mock records exactly what crossed the wire: both pairs, repeated + // under one key, in the order they were typed, and nothing else added. + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests.len(), 1, "got: {requests:?}"); + assert_eq!( + requests[0].url.query(), + Some("label=a&label=b"), + "the flags must map to repeated query params, verbatim and in order", + ); + + // And the response comes back untouched: no client-side filtering, no + // model in the way, fields from the future included. + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("a_field_from_the_future"), + "the server's document must be rendered untouched: {stdout}", + ); + assert!(stdout.contains("s-1"), "got: {stdout}"); +} + +#[tokio::test] +async fn tapesctl_drives_labels_via_discovery_only() { + // --- nothing is compiled in: help knows no such noun ------------------- + // Against a deployment serving no cassettes, any occurrence of the word + // in help output could only have been compiled into the binary. + let empty_cache = tempfile::tempdir().unwrap(); + let empty = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/v1/cassettes")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "contract_version": "v1", + "cassettes": [] + }))) + .mount(&empty) + .await; + + let help = run_tapesctl(empty_cache.path(), &empty.uri(), &["--help"]).await; + assert!(help.status.success(), "got: {help:?}"); + let help_text = String::from_utf8_lossy(&help.stdout).to_lowercase(); + assert!( + !help_text.contains("label"), + "no such noun may be compiled into the top-level surface: {help_text}", + ); + + let list_help = run_tapesctl( + empty_cache.path(), + &empty.uri(), + &["sessions", "list", "--help"], + ) + .await; + assert!(list_help.status.success(), "got: {list_help:?}"); + let list_help_text = String::from_utf8_lossy(&list_help.stdout); + assert!( + list_help_text.contains("--filter"), + "the generic passthrough is the only filter flag: {list_help_text}", + ); + assert!( + !list_help_text.to_lowercase().contains("label"), + "no claimed-param name may become flag sugar: {list_help_text}", + ); + + // --- and discovery alone serves the whole surface ---------------------- + // A deployment serving a cassette named `labels` (fixture data): the + // binary learns it at runtime and drives it end to end with zero + // compiled-in knowledge. + let cache_dir = tempfile::tempdir().unwrap(); + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/v1/cassettes")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "contract_version": "v1", + "cassettes": [{ + "name": "labels", + "route_prefix": "/v1/cassettes/labels", + "openapi_path": "/v1/cassettes/labels/openapi.json", + "openapi_status": "fresh" + }] + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/v1/cassettes/labels/openapi.json")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "paths": {"/v1/cassettes/labels/labels": { + "get": {"operationId": "listLabels"} + }} + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/v1/cassettes/labels/labels")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "labels": [{"name": "urgent", "color": "#ff0000"}] + }))) + .mount(&server) + .await; + + let uri = server.uri(); + let listing = run_tapesctl( + cache_dir.path(), + &uri, + &["cassettes", "--help", "--api-url", &uri], + ) + .await; + assert!(listing.status.success(), "got: {listing:?}"); + let listing_text = String::from_utf8_lossy(&listing.stdout); + assert!( + listing_text.contains("labels"), + "the discovered cassette must be listed under the noun: {listing_text}", + ); + + let call = run_tapesctl( + cache_dir.path(), + &uri, + &["cassettes", "labels", "list-labels", "--api-url", &uri], + ) + .await; + assert!( + call.status.success(), + "the generated command failed: {call:?}" + ); + let call_text = String::from_utf8_lossy(&call.stdout); + assert!( + call_text.contains("urgent"), + "the round trip must print the cassette's own document: {call_text}", + ); + + let requests = server.received_requests().await.unwrap(); + assert!( + requests + .iter() + .any(|request| request.url.path() == "/v1/cassettes/labels/labels"), + "the generated command must call the route the spec named: {requests:?}", + ); +} diff --git a/docs/commands.md b/docs/commands.md index 9cea5f4..956b8e8 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -281,7 +281,7 @@ fields the server grows reach you without a client upgrade. | leaf | route | flags | |---|---|---| -| `list` | `GET /v1/sessions` | `--limit`, `--cursor`, `--sort`, `--direction`, `--since`, `--until`, `--harness-id`, `--harness-session-id`, `--auth-subject`, `--json` | +| `list` | `GET /v1/sessions` | `--limit`, `--cursor`, `--sort`, `--direction`, `--since`, `--until`, `--harness-id`, `--harness-session-id`, `--auth-subject`, `--filter`, `--json` | | `get ` | `GET /v1/sessions/{id}` | — | | `traces ` | `GET /v1/sessions/{id}/traces` | `--payload` | | `raw-turns ` | `GET /v1/sessions/{id}/raw_turns` | — | @@ -306,6 +306,7 @@ come as a pair — a lone half fails at parse with the missing half named. | `--harness-id ` | the harness the session ran under (e.g. `claude`) — the other half of the pair | | `--harness-session-id ` | exact match on the harness session id — the id `start` prints; pairs with `--harness-id`; see [Session ids](./capture.md#session-ids) | | `--auth-subject ` | exact match | +| `--filter ` | repeatable; each pair is passed through as `?key=value`, verbatim and in order. The key names a filter param a cassette on your deployment claims at runtime — nothing is validated or filtered client-side, and a key nothing claims is ignored by the server | | `--json` | print the raw pretty-printed JSON instead of the table, so the output still composes with `jq` | `--payload` takes `full` (the default) or `preview`, case-insensitively. An From f82eb5d1f66c122ef1d86752af534e38dfd93a50 Mon Sep 17 00:00:00 2001 From: Matt Yeazel Date: Thu, 27 Aug 2026 16:10:04 -0700 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=A7=B9=20chore:=20repay=20the=20tapes?= =?UTF-8?q?-client=20loan=20=E2=80=94=20crates.io=200.5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The path override onto the sibling checkout was the documented co-development loan: the claimed-filter seam the --filter flag rides on was committed upstream but not yet published. tapes-client 0.5.0 now carries it on crates.io, so the workspace entry returns to the registry form and the lockfile records the published crate. 0.5.0 also retires the typed search, export, and skills surfaces with the read contract's move to tapes v0.39.0: those operations left the sealed document — each is a cassette a deployment serves — and the typed spellings went with them. What that means here: * `tapesctl search` and `tapesctl export` keep their routes, requests, and output unchanged, but issue their calls directly against the cassette routes the retired methods had been rerouting to, through the transport's described-call seam. The response models and the accepted --detail grains move into the two commands, which own those shapes now that the sealed contract does not declare them. * The coverage tables shrink to the operations the contract still declares: searchSpans and exportSession leave the exposed table, and the unexposed allow-list drops exportSessions and the eleven skills routes whose deletion it had been predicting. --- Cargo.lock | 4 +- Cargo.toml | 9 +--- crates/tapesctl/src/api/client.rs | 65 +++++++----------------- crates/tapesctl/src/api/contract.rs | 52 ------------------- crates/tapesctl/src/ports/export.rs | 63 +++++++++++++++++++---- crates/tapesctl/src/ports/search.rs | 79 ++++++++++++++++++++++++++--- 6 files changed, 145 insertions(+), 127 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1fbc9ba..09e61c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1844,7 +1844,9 @@ dependencies = [ [[package]] name = "tapes-client" -version = "0.4.1" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "99371583a418fc5eab8ee40542432f2494847e8e88017234080725bed163cc63" dependencies = [ "clap", "dirs", diff --git a/Cargo.toml b/Cargo.toml index 5fe9551..3a21afd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -103,14 +103,7 @@ tapes-capture = { version = "0.1" } # is built at runtime from a document this binary has never seen, which is the # one surface that cannot be written by hand. `direct-http` is `DirectHttp`, # the no-redirect transport this CLI talks to a tapes server with. -# RECONCILIATION: temporarily a path override onto the sibling checkout — the -# documented co-development loan (see the escape-hatch note above and -# `scripts/check-tapes-pins.sh`). The generic claimed-filter-param support on -# the sealed sessions method is committed there and not yet published; repoint -# this entry at the next released `tapes-client` (>= the release carrying -# `CoreClient::call_with_claimed`) once it exists, restoring the -# `version = "0.4"` registry form. -tapes-client = { path = "../tapes-harnesses/crates/tapes-client", features = [ +tapes-client = { version = "0.5", features = [ "cli", "direct-http", ] } diff --git a/crates/tapesctl/src/api/client.rs b/crates/tapesctl/src/api/client.rs index b3ba24c..154be5d 100644 --- a/crates/tapesctl/src/api/client.rs +++ b/crates/tapesctl/src/api/client.rs @@ -13,8 +13,8 @@ //! # Why the read commands still print `serde_json::Value` //! //! The named methods on [`tapes_client::CoreClient`] return the vendored -//! contract's models, and every command that *renders* a response — search, -//! seed — uses them. The ` ` commands +//! contract's models, and every command that *renders* a response — seed — +//! uses them. The ` ` commands //! do not render: they print the server's document, and a document that had //! been through a model would be missing whatever fields this build predates. //! For those, [`tapes_client::CoreClient::call`] is the documented escape @@ -62,8 +62,8 @@ pub fn connect(base: Url) -> ApiClient { /// The contract's own spellings are the only ones accepted, case-folded and /// trimmed the way this CLI has always accepted them. Refusing here rather than /// letting the server answer 400 costs no round trip and names the alternatives -/// — see [`crate::error::Error::InvalidPayloadDetail`] and its sibling, whose -/// wording a test holds to [`ContractEnum::VALUES`]. +/// — see [`crate::error::Error::InvalidPayloadDetail`], whose wording a test +/// holds to [`ContractEnum::VALUES`]. pub fn parse_grain(raw: &str) -> Option { serde_json::from_value(Value::String(raw.trim().to_ascii_lowercase())).ok() } @@ -86,8 +86,8 @@ mod tests { use crate::api::contract::ops; use serde_json::json; use tapes_client::core::models::params::ContractParams; - use tapes_client::core::models::{ExportDetail, PayloadDetail, SessionListParams}; - use wiremock::matchers::{method, path, query_param}; + use tapes_client::core::models::{PayloadDetail, SessionListParams}; + use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; fn client_for(server: &MockServer) -> ApiClient { @@ -156,34 +156,30 @@ mod tests { #[test] fn a_user_typed_grain_resolves_to_the_contracts_own_spelling() { assert_eq!( - parse_grain::("SPANS"), - Some(ExportDetail::Spans) + parse_grain::("PREVIEW"), + Some(PayloadDetail::Preview) ); assert_eq!( - parse_grain::(" preview "), - Some(PayloadDetail::Preview), + parse_grain::(" full "), + Some(PayloadDetail::Full), ); assert_eq!(parse_grain::("hologram"), None); } #[test] - fn the_refusal_messages_name_exactly_the_values_the_contract_declares() { - // The two messages spell their alternatives inline, because a user - // reading one wants the answer and not a cross-reference. This is what - // keeps that spelling honest: a contract that grows a grain fails here - // rather than teaching the user a stale set. + fn the_refusal_message_names_exactly_the_values_the_contract_declares() { + // The message spells its alternatives inline, because a user reading + // it wants the answer and not a cross-reference. This is what keeps + // that spelling honest: a contract that grows a grain fails here + // rather than teaching the user a stale set. Its export sibling is + // held the same way in `ports::export`, where the accepted set now + // lives. let payload = crate::error::error::InvalidPayloadDetailSnafu { payload: "x" } .build() .to_string(); for value in PayloadDetail::VALUES { assert!(payload.contains(value), "{value:?} missing from: {payload}"); } - let detail = crate::error::error::InvalidExportDetailSnafu { detail: "x" } - .build() - .to_string(); - for value in ExportDetail::VALUES { - assert!(detail.contains(value), "{value:?} missing from: {detail}"); - } } #[test] @@ -236,31 +232,4 @@ mod tests { assert!(rendered.contains("400"), "got: {rendered}"); assert!(rendered.contains("invalid cursor"), "got: {rendered}"); } - - #[tokio::test] - async fn a_search_response_is_decoded_through_the_shipped_model() { - let server = MockServer::start().await; - Mock::given(method("GET")) - .and(path("/v1/cassettes/search/spans")) - .and(query_param("query", "hooks")) - .and(query_param("top_k", "5")) - .respond_with(ResponseTemplate::new(200).set_body_string( - r#"{"query":"hooks","count":1,"results":[{"trace_id":"t-1","a_new_field":1}]}"#, - )) - .mount(&server) - .await; - - let got = client_for(&server) - .search_spans(&tapes_client::core::models::SearchSpansParams { - query: "hooks".to_owned(), - top_k: Some(5), - }) - .await - .unwrap(); - - // A field the model has never heard of is ignored rather than fatal: - // an additive server change must not blank a page of results. - assert_eq!(got.count, 1); - assert_eq!(got.results[0].trace_id, "t-1"); - } } diff --git a/crates/tapesctl/src/api/contract.rs b/crates/tapesctl/src/api/contract.rs index a14e8ce..dad62a0 100644 --- a/crates/tapesctl/src/api/contract.rs +++ b/crates/tapesctl/src/api/contract.rs @@ -53,14 +53,12 @@ pub const EXPOSED_OPERATIONS: &[(&str, &str)] = &[ (ops::GET_SESSION, "tapesctl sessions get"), (ops::GET_SESSION_TRACES, "tapesctl sessions traces"), (ops::LIST_RAW_TURNS, "tapesctl sessions raw-turns"), - (ops::EXPORT_SESSION, "tapesctl export"), (ops::LIST_TRACES, "tapesctl traces list"), ( ops::GET_TRACE, "tapesctl traces get, and the spans-list projection", ), (ops::GET_SPAN, "tapesctl spans get"), - (ops::SEARCH_SPANS, "tapesctl search"), (ops::SEED_DEMO, "tapesctl seed"), ( ops::LIST_CASSETTES, @@ -99,60 +97,10 @@ pub const UNEXPOSED_OPERATIONS: &[(&str, &str)] = &[ "updateSession", "session rename/edit; not ported from the Go CLI, which never had it either", ), - ( - "exportSessions", - "bulk export window; tapesctl export is per-session today, the bulk port is future work", - ), ( "getStats", "aggregate stats; not ported from the Go CLI yet", ), - ( - "listSessionSkills", - "core's copy of a surface the skills cassette owns; tapesctl reaches skills through the \ - discovered `cassettes skills` commands, and these routes are deleted from core with the \ - cassette cutover's batched removal", - ), - ( - "listSkills", - "core's copy of a cassette-owned surface; see listSessionSkills", - ), - ( - "createSkill", - "core's copy of a cassette-owned surface; see listSessionSkills", - ), - ( - "getSkill", - "core's copy of a cassette-owned surface; see listSessionSkills", - ), - ( - "updateSkill", - "core's copy of a cassette-owned surface; see listSessionSkills", - ), - ( - "deleteSkill", - "core's copy of a cassette-owned surface; see listSessionSkills", - ), - ( - "duplicateSkill", - "core's copy of a cassette-owned surface; see listSessionSkills", - ), - ( - "getSkillMarkdown", - "core's copy of a cassette-owned surface; see listSessionSkills", - ), - ( - "listSkillVersions", - "core's copy of a cassette-owned surface; see listSessionSkills", - ), - ( - "publishSkill", - "core's copy of a cassette-owned surface; see listSessionSkills", - ), - ( - "generateSkill", - "core's copy of a cassette-owned surface; see listSessionSkills", - ), ]; #[cfg(test)] diff --git a/crates/tapesctl/src/ports/export.rs b/crates/tapesctl/src/ports/export.rs index 49fa515..7b63ffd 100644 --- a/crates/tapesctl/src/ports/export.rs +++ b/crates/tapesctl/src/ports/export.rs @@ -14,27 +14,53 @@ use tokio::io::AsyncWriteExt; use snafu::{OptionExt, ResultExt}; -use tapes_client::core::models::ExportSessionParams; +use tapes_client::Call; -use crate::api::client::parse_grain; use crate::api::resolve_client; use crate::cli::ExportArgs; use crate::error::{Result, error}; +/// The export cassette's per-session route, called directly. Export is a +/// cassette a deployment serves, not an operation of the sealed core +/// contract, so the route and the accepted `--detail` grains belong to this +/// command rather than to the shared client. +const EXPORT_SESSION_ROUTE: &str = "/v1/cassettes/export/sessions/{id}"; + +/// The export grains the server accepts, in the spelling the wire takes. +/// +/// [`crate::error::Error::InvalidExportDetail`] spells these inline; a test +/// below holds the two lists together. +pub const DETAIL_VALUES: [&str; 2] = ["spans", "traces"]; + +/// Resolve a user-typed `--detail` onto the accepted set, case-folded and +/// trimmed the way this CLI has always accepted a closed set. +fn parse_detail(raw: &str) -> Option<&'static str> { + let folded = raw.trim().to_ascii_lowercase(); + DETAIL_VALUES + .iter() + .find(|value| **value == folded) + .copied() +} + /// Run one export. pub async fn run(args: ExportArgs) -> Result<()> { let client = resolve_client(&args.api)?; let detail = match args.detail.as_deref() { - Some(raw) => parse_grain(raw) - .map(Some) - .context(error::InvalidExportDetailSnafu { - detail: raw.to_owned(), - })?, + Some(raw) => Some(parse_detail(raw).context(error::InvalidExportDetailSnafu { + detail: raw.to_owned(), + })?), None => None, }; - let response = client - .export_session(&args.session_id, &ExportSessionParams { detail }) - .await?; + let mut call = Call { + method: "GET", + path: EXPORT_SESSION_ROUTE, + path_params: vec![("id".to_owned(), args.session_id.clone())], + ..Call::default() + }; + if let Some(detail) = detail { + call.query.push(("detail".to_owned(), detail.to_owned())); + } + let response = client.transport().execute_stream(&call).await?; match args.output.as_deref() { Some(path) => { @@ -134,6 +160,23 @@ mod tests { assert!(run(args).await.is_ok()); } + #[test] + fn the_refusal_message_names_exactly_the_grains_the_server_accepts() { + // The message spells its alternatives inline, because a user reading + // it wants the answer and not a cross-reference. This is what keeps + // that spelling honest: a server that grows a grain fails here rather + // than teaching the user a stale set. + let rendered = crate::error::error::InvalidExportDetailSnafu { detail: "x" } + .build() + .to_string(); + for value in DETAIL_VALUES { + assert!( + rendered.contains(value), + "{value:?} missing from: {rendered}" + ); + } + } + #[tokio::test] async fn an_unknown_detail_is_rejected_before_any_request() { let server = MockServer::start().await; diff --git a/crates/tapesctl/src/ports/search.rs b/crates/tapesctl/src/ports/search.rs index 16a4c36..a67d4dc 100644 --- a/crates/tapesctl/src/ports/search.rs +++ b/crates/tapesctl/src/ports/search.rs @@ -14,7 +14,8 @@ //! printed plain. And the command reported a result count to product //! telemetry, which tapesctl does not have. -use tapes_client::core::models::{SearchSpansParams, SpanSearchResult}; +use serde::Deserialize; +use tapes_client::Call; use time::OffsetDateTime; use time::UtcOffset; use time::format_description::well_known::Rfc3339; @@ -30,18 +31,77 @@ const PROMPT_WIDTH: usize = 80; /// Longest snippet before it is elided. const SNIPPET_WIDTH: usize = 100; +/// The search cassette's span route, called directly. Search is a cassette a +/// deployment serves, not an operation of the sealed core contract, so the +/// route and the response shape below belong to this command rather than to +/// the shared client. +const SEARCH_SPANS_ROUTE: &str = "/v1/cassettes/search/spans"; + +/// The span search response, decoded here because the shape is the search +/// cassette's own. Only the fields this renderer reads are named; anything +/// else the server says is ignored rather than fatal, so an additive change +/// cannot blank a page of results. +#[derive(Debug, Default, Deserialize)] +#[serde(default)] +pub struct SpanSearchOutput { + /// The query the server ran, echoed back. + pub query: String, + /// The ranked hits. An explicit `null` decodes as empty, like an absent + /// key. + #[serde(deserialize_with = "null_default")] + pub results: Vec, +} + +/// One span hit with its trace/turn context. +#[derive(Debug, Default, Deserialize)] +#[serde(default)] +pub struct SpanSearchResult { + /// The hit's relevance score. + pub score: f32, + /// The session the span belongs to. + pub session_id: String, + /// Preview of the matched span's delta-only text. + pub snippet: String, + /// The span's id. + pub span_id: String, + /// The span's start, an RFC 3339 timestamp. + pub started_at: String, + /// The trace the span belongs to. + pub trace_id: String, + /// The prompt of the turn the span belongs to. The server sends it even + /// when blank, so a synthetic turn's empty prompt is distinguishable from + /// a missing field. + pub user_prompt: String, +} + +/// Decode an explicit `null` as the type's default, like an absent key. +fn null_default<'de, D, T>(deserializer: D) -> std::result::Result +where + D: serde::Deserializer<'de>, + T: Default + Deserialize<'de>, +{ + Ok(Option::::deserialize(deserializer)?.unwrap_or_default()) +} + /// Run one search. pub async fn run(args: SearchArgs) -> Result<()> { let client = resolve_client(&args.api)?; - let output = client - .search_spans(&SearchSpansParams { - query: args.query.clone(), - // Always sent, unlike a listing's omit-when-unset rule: the flag - // carries the default, so this client always has a value, and one - // request spelling is better than two. - top_k: Some(narrow(args.top)), + let value = client + .transport() + .execute(&Call { + method: "GET", + path: SEARCH_SPANS_ROUTE, + query: vec![ + ("query".to_owned(), args.query.clone()), + // Always sent, unlike a listing's omit-when-unset rule: the + // flag carries the default, so this client always has a value, + // and one request spelling is better than two. + ("top_k".to_owned(), narrow(args.top).to_string()), + ], + ..Call::default() }) .await?; + let output: SpanSearchOutput = tapes_client::decode::typed(value)?; if output.results.is_empty() { if !args.quiet { @@ -259,6 +319,9 @@ mod tests { "user_prompt": "how do I use gum", "snippet": "gum glow", "started_at": "2026-07-31T12:00:00Z", + // A field this build has never heard of must be ignored + // rather than fatal. + "a_field_from_the_future": 7, }], })) .await;