From f8dad9dbc82500faa578e234bc75780d5544ad22 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Sun, 6 Sep 2026 19:26:51 +0000 Subject: [PATCH] feat(review 006 Unit 2): additive OperationSpec.description disclosed via discovery (E-02) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - OperationSpec gains description: Option (builder with_description, defaults None; no struct-literal construction sites exist, so additive by construction) - spec_to_json_pub emits description when set; rebuild_spec_for parses it back — the field survives from_call discovery and op/register announcement (same round-trip pattern as resource_id_path / publish_schema) - services/list and the local-ops half of services/list-peers emit description when set; output-schema docs on both listing specs and operation_spec_schema advertise the field - Tests: builder/default, emit/omit, listing emission, schema disclosure, schema-doc presence, round-trip + absent-stays-absent (8 new; 616 total) - Docs: review 006 Unit 2 marked IMPLEMENTED; ADR-047 §6 amendment records the E-02 discovery decision (listing enrichment lands, the channel/resources/subscribe half stays deferred); OQ-40 gains the load-bearing note; operation-registry.md struct + listing docs; CHANGELOG Verification: cargo test (616 pass), clippy -D warnings (host + wasm32), fmt --check, cargo doc --no-deps, wasm32 check — all clean --- CHANGELOG.md | 10 ++ .../047-openable-alpns-are-operations.md | 35 ++++- docs/architecture/open-questions.md | 2 +- docs/architecture/operation-registry.md | 9 ++ ...6-channel-open-establishment-gap-review.md | 16 ++- src/client/from_call.rs | 62 +++++++++ src/registry/discovery.rs | 130 +++++++++++++++++- src/registry/spec.rs | 38 +++++ 8 files changed, 294 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7566bb..e9986ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,16 @@ N-1). Additive establishment machinery; one breaking change (the ### Added +- **`OperationSpec.description` (review 006 E-02).** Additive + `Option` op description (builder: `with_description`), + disclosed by `services/list` and `services/list-peers` (local + listings) when set, carried in the `services/schema` wire shape + (`spec_to_json_pub` emit / `rebuild_spec_for` parse — the field + survives `from_call` discovery and `op/register` announcement). + Describes the op, not the produced resource set (ADR-047 §6 + amendment: the live resource-enumeration half stays deferred, + OQ-40). Absent on the wire when unset — additive for all consumers. + - **Channel-open establishment phase (ADR-049 — review 006 E-01).** `ChannelCore::register_openable_with_establisher` registers a per-ALPN open op with an establisher hook (`OpenEstablisher`): an diff --git a/docs/architecture/decisions/047-openable-alpns-are-operations.md b/docs/architecture/decisions/047-openable-alpns-are-operations.md index fd21487..5678466 100644 --- a/docs/architecture/decisions/047-openable-alpns-are-operations.md +++ b/docs/architecture/decisions/047-openable-alpns-are-operations.md @@ -8,7 +8,40 @@ Accepted (amends ADR-037; refines ADR-044, ADR-046; §4 amended registration, 2026-08-13)"; amendment #2 (2026-09-03) — the per-connection registration mechanism is the **per-session fork of the base registry installed as the session's dispatch registry**, not the -connection overlay — see "Amendment (§4 mechanism, 2026-09-03)" below) +connection overlay — see "Amendment (§4 mechanism, 2026-09-03)" below; +§6 amended 2026-09-06 — the static half of discovery gains an additive +per-op `description` on the listing, the dynamic half stays deferred +— see "Amendment (§6 listing enrichment, 2026-09-06)" below) + +## Amendment (§6 listing enrichment, 2026-09-06) + +Review 006 E-02 (from the alktunnels Phase 0 sweep) made §6's dynamic +half load-bearing for the first time and asked for a decision on the +static half. Decision: **both halves of the "static per-op" split gain +what is cheap today; the dynamic half stays deferred** (OQ-40, now +with a "load-bearing for alktunnels discovery UI" note in +`open-questions.md`; alktunnels v1 uses config-known op names): + +- `OperationSpec` gains `description: Option` — a human- + readable op description, set via `with_description` at registration. + Additive (defaults `None`; no struct-literal construction sites + exist — all sites use `OperationSpec::new`). +- `services/list` (and the local-ops half of `services/list-peers`) + emit `description` when set — one round-trip answers "which ops + exist and what are they for" without the N+1 `services/schema` + sweep. The output-schema docs on both listing specs and on + `services/schema`'s `operation_spec_schema` advertise the field. +- `spec_to_json_pub` emits it when set; `rebuild_spec_for` parses it + back — the description survives discovery and peer announcement + (`from_call`, `op/register`) like every other additive spec field + (`resource_id_path`, `publish_schema` round-trip the same way). + +Scope note from E-02 stands: the listing field describes **the op**, +not **the produced resource set** (a tunnel producer registers one op +and N resources). "Which tunnel resources may I open, live" remains +`channel/resources/subscribe`'s job (§6's dynamic half, ADR-037 § +`channel/resources/subscribe`) — deferred until a consumer needs live +resource discovery. ## Amendment (§4 mechanism, 2026-09-03) diff --git a/docs/architecture/open-questions.md b/docs/architecture/open-questions.md index db26c85..99d4852 100644 --- a/docs/architecture/open-questions.md +++ b/docs/architecture/open-questions.md @@ -83,7 +83,7 @@ is the load-bearing piece the broker composes on. | OQ-37 | `from_call` relay wrapper for marked ops | open | medium | ADR-047 §1 names it as a consumer (hub) concern; alkcall's `from_call` reconstructs the marker (Gap F resolved) so the consumer can branch on it | | OQ-38 | ALPN→path-segment mapping | resolved | low | ADR-047 §"Negative" — strip the `alk/` prefix; ALPNs without that prefix use the full ALPN string (rare, two-way-door) | | OQ-39 | `channel/control` control-handle surface | open | medium | The `channel/control` handler currently returns `channel:control_not_implemented`. The control-handle surface (per-channel control callbacks registered by ALPN crates, routing `message` to the handler's control handle for `channel_id`) is real design work — each ALPN crate needs a way to register a control callback, and the channels layer needs a control-handle registry keyed by `channel_id`. Deferred until an ALPN crate (TTY, tunnel) needs out-of-band control. | -| OQ-40 | `channel/resources/subscribe` live subscription | open | medium | The `channel/resources/subscribe` handler currently returns `channel:resources_not_implemented`. The live subscription aggregated from ALPN-crate resource enumerators (ADR-047 §6) requires each ALPN crate to provide a resource enumerator, and the channels layer to aggregate them into a live `Stream` that emits on any change. The current stub is a one-shot error; the real implementation is deferred until a consumer (hub, dashboard) needs live resource discovery. | +| OQ-40 | `channel/resources/subscribe` live subscription | open | medium | The `channel/resources/subscribe` handler currently returns `channel:resources_not_implemented`. The live subscription aggregated from ALPN-crate resource enumerators (ADR-047 §6) requires each ALPN crate to provide a resource enumerator, and the channels layer to aggregate them into a live `Stream` that emits on any change. The current stub is a one-shot error; the real implementation is deferred until a consumer (hub, dashboard) needs live resource discovery. **Load-bearing for alktunnels' discovery UI** (review 006 E-02): a consumer cannot distinguish produced tunnel resources by op name alone. The static half is covered (0.5.0: `OperationSpec.description` on `services/list` — describes the op, not the live resource set); the dynamic half stays deferred — alktunnels v1 uses config-known op names. | | OQ-41 | QUIC-native multi-stream substrate | open | medium | Only the in-line substrate mode is implemented (single bidi stream, header-demuxed N channels). The QUIC-native multi-stream substrate (accept remaining bidi streams, read headers off each — ADR-034 §substrate modes) is deferred to the downstream alknet crate. The wire format and demux loop are correct for both substrates; only the outer `accept_bi()` loop is missing. The alknet crate owns the QUIC dial/accept loop and is the natural place for the multi-stream accept loop. This crate stays transport-agnostic (no QUIC dependency, WASM-compatible). | ## Core Types diff --git a/docs/architecture/operation-registry.md b/docs/architecture/operation-registry.md index 3c29d85..3d3b044 100644 --- a/docs/architecture/operation-registry.md +++ b/docs/architecture/operation-registry.md @@ -39,6 +39,9 @@ pub struct OperationSpec { pub output_schema: Value, // JSON Schema for output pub error_schemas: Vec, // Declared domain errors (ADR-016) pub access_control: AccessControl, + /// Human-readable op description (review 006 E-02). Disclosed by + /// `services/list` when set; `None` when the op declares none. + pub description: Option, /// JSON pointer into the input for the resource ID, when /// `access_control.resource_type` is set and the operation targets a /// specific runtime-spawned resource (ADR-011). e.g., `"$.containerId"` @@ -829,6 +832,12 @@ These are read-only — no admin operations are exposed through the call protoco } ``` +Each listing entry also carries `description` (review 006 E-02) when +the op's spec declares one (`OperationSpec.description`, set via +`with_description`) — the field is additive and absent otherwise. It +describes the op, not the produced resource set: live resource +discovery stays with `channel/resources/subscribe` (ADR-047 §6, OQ-40). + `services/schema` accepts `{ "name": "fs/readFile" }` (no leading slash — registry form, same as `OperationSpec.name`) and returns the full `OperationSpec` including input/output JSON Schemas and declared diff --git a/docs/reviews/006-channel-open-establishment-gap-review.md b/docs/reviews/006-channel-open-establishment-gap-review.md index 32aaec5..3b560cb 100644 --- a/docs/reviews/006-channel-open-establishment-gap-review.md +++ b/docs/reviews/006-channel-open-establishment-gap-review.md @@ -27,7 +27,13 @@ gates landed as tests; two implementation-shape notes recorded in ADR-049's amendment: the establisher does not receive the channel `Connection` (yield-once BiStream belongs to the pump handler), and the bound is the earlier of dispatch deadline and per-registration -timeout). The original remediation sketch below is superseded by the +timeout). **Unit 2 (E-02) is implemented** in alkcall 0.5.0 +(`OperationSpec.description: Option` + +`with_description`, `spec_to_json_pub` emits it when set, +`rebuild_spec_for` parses it (shared by `from_call` and `op/register`), +`services/list` and `services/list-peers` local listings emit it when +set; the ADR-047 §6 amendment below records the discovery decision). +The original remediation sketch below is superseded by the "Remediation plan (post-verification)" section; the original sketch is retained for the record. @@ -360,6 +366,10 @@ transports, two contracts). alkhttp unaffected (no openable ops). small ADR amendment: recommend (3) — subscribe for live resource sets (the decided shape, now load-bearing), plus an additive `description` field on the listing (cheap, immediately useful). +**Implemented 2026-09-06 (alkcall 0.5.0):** the listing half landed +(`OperationSpec.description`, four touchpoints as the appendix +corrected); the subscribe half stays deferred (OQ-40, now with the +load-bearing note). **Unit 3 — E-03/E-04.** E-03: log-or-comment; E-04: doc note. Trivial. @@ -407,7 +417,9 @@ correction): struct field + `spec_to_json_pub` emit + `services/list` emits it when set. OQ-40 stays deferred but gains a "load-bearing for alktunnels discovery UI" note; the `channel/resources/subscribe` half stays deferred (alktunnels v1 uses -config-known op names). +config-known op names). **Status: IMPLEMENTED (2026-09-06)** — the +discovery decision (3: enriched listing now, subscribe deferred) is +recorded as an amendment to ADR-047 §6. **Unit 3 — E-03/E-04/N-2 (trivial batch, same PR series as Unit 1).** E-03: debug log (or pinning comment) on the discarded `UnknownChannel` diff --git a/src/client/from_call.rs b/src/client/from_call.rs index c08fa96..81c95c3 100644 --- a/src/client/from_call.rs +++ b/src/client/from_call.rs @@ -285,6 +285,10 @@ pub(crate) fn rebuild_spec_for( } } + if let Some(description) = schema_json.get("description").and_then(|v| v.as_str()) { + spec = spec.with_description(description); + } + Ok(spec) } @@ -768,6 +772,64 @@ mod tests { assert_eq!(rebuilt.publish_schema.as_ref(), Some(&publish_schema)); } + /// E-02 gate (review 006): `description` survives the spec wire + /// round-trip — `spec_to_json_pub` serializes it when set, + /// `rebuild_spec_for` parses it back (the `op/register` announced-spec + /// path and the `from_call` import path both parse through here). + #[test] + fn spec_round_trips_description() { + use crate::registry::discovery::spec_to_json_pub; + + let spec = OperationSpec::new( + "channels/tty/sub", + OperationType::Sub, + Visibility::External, + json!({}), + json!({}), + vec![], + crate::registry::spec::AccessControl::default(), + None, + ) + .with_description("Interactive TTY sessions"); + let wire = spec_to_json_pub(&spec); + assert_eq!( + wire.get("description").and_then(|v| v.as_str()), + Some("Interactive TTY sessions"), + "description serialized" + ); + + let rebuilt = rebuild_spec_for(&wire, "channels/tty/sub", &None).expect("rebuild"); + assert_eq!( + rebuilt.description.as_deref(), + Some("Interactive TTY sessions"), + "description survives the round-trip" + ); + } + + /// E-02 companion: a spec without `description` serializes no + /// `description` key and rebuilds with `None` (additive optional + /// field — absent stays absent, old producers stay parseable). + #[test] + fn spec_without_description_stays_absent_through_round_trip() { + use crate::registry::discovery::spec_to_json_pub; + + let spec = OperationSpec::new( + "fs/readFile", + OperationType::Query, + Visibility::External, + json!({}), + json!({}), + vec![], + crate::registry::spec::AccessControl::default(), + None, + ); + let wire = spec_to_json_pub(&spec); + assert!(wire.get("description").is_none()); + + let rebuilt = rebuild_spec_for(&wire, "fs/readFile", &None).expect("rebuild"); + assert_eq!(rebuilt.description, None); + } + #[test] fn derive_alpn_from_op_name_strips_channels_prefix() { assert_eq!( diff --git a/src/registry/discovery.rs b/src/registry/discovery.rs index 210977e..9604794 100644 --- a/src/registry/discovery.rs +++ b/src/registry/discovery.rs @@ -33,6 +33,10 @@ pub fn services_list_spec() -> OperationSpec { "op_type": { "type": "string", "enum": ["query", "mutation", "sub", "pub"] + }, + "description": { + "type": "string", + "description": "Human-readable op description (review 006 E-02). Absent when the producer declares none." } } } @@ -87,6 +91,10 @@ pub fn services_list_peers_spec() -> OperationSpec { "op_type": { "type": "string", "enum": ["query", "mutation", "sub", "pub"] + }, + "description": { + "type": "string", + "description": "Human-readable op description. Absent when the op declares none." } } } @@ -116,6 +124,9 @@ fn operation_spec_schema() -> Value { "type": "string", "enum": ["external", "internal"] }, + "description": { + "description": "Human-readable op description (review 006 E-02). Absent when the op declares none; disclosed verbatim in `services/list` when set." + }, "input_schema": {}, "output_schema": {}, "error_schemas": { @@ -227,6 +238,9 @@ pub fn spec_to_json_pub(spec: &OperationSpec) -> Value { if let Some(resource_id_path) = &spec.resource_id_path { json["resource_id_path"] = json!(resource_id_path); } + if let Some(description) = &spec.description { + json["description"] = json!(description); + } if spec.channel_open.is_some() { json["channel_open"] = json!(true); } @@ -259,11 +273,15 @@ pub fn services_list_handler(registry: Arc) -> Handler { .is_allowed() }) .map(|s| { - json!({ + let mut listing = json!({ "name": s.name, "namespace": s.namespace, "op_type": op_type_str(s.op_type), - }) + }); + if let Some(description) = &s.description { + listing["description"] = json!(description); + } + listing }) .collect(); ResponseEnvelope::ok(ctx.request_id, json!({ "operations": ops })) @@ -333,11 +351,15 @@ pub fn services_list_peers_handler(registry: Arc) -> Handler .is_allowed() }) .map(|s| { - json!({ + let mut listing = json!({ "name": s.name, "namespace": s.namespace, "op_type": op_type_str(s.op_type), - }) + }); + if let Some(description) = &s.description { + listing["description"] = json!(description); + } + listing }) .collect(); let mut peers: Vec = Vec::new(); @@ -1057,6 +1079,106 @@ mod tests { ); } + // --- review 006 E-02: `description` on OperationSpec ------------------- + + #[test] + fn spec_to_json_emits_description_when_set() { + let spec = external_spec("fs/readFile").with_description("Read a file"); + let json_val = spec_to_json(&spec); + assert_eq!(json_val.get("description"), Some(&json!("Read a file"))); + } + + #[test] + fn spec_to_json_omits_description_when_absent() { + let spec = external_spec("fs/readFile"); + let json_val = spec_to_json(&spec); + assert!( + json_val.get("description").is_none(), + "description must be absent when not set (additive optional field)" + ); + } + + #[tokio::test] + async fn services_list_emits_description_when_set() { + let registry = Arc::new(OperationRegistry::new()); + registry + .register(HandlerRegistration::new( + external_spec("channels/tty/sub").with_description("Interactive TTY sessions"), + HandlerKind::Once(echo_handler()), + OperationProvenance::Local, + None, + None, + Capabilities::new(), + )) + .unwrap(); + registry + .register(HandlerRegistration::new( + external_spec("fs/readFile"), + HandlerKind::Once(echo_handler()), + OperationProvenance::Local, + None, + None, + Capabilities::new(), + )) + .unwrap(); + let handler = services_list_handler(Arc::clone(®istry)); + let response = handler(json!({}), root_context("req-e02-1")).await; + let output = response.result.expect("ok response"); + let ops = output + .get("operations") + .and_then(|v| v.as_array()) + .expect("operations array") + .iter() + .map(|o| { + ( + o.get("name").and_then(|n| n.as_str()).unwrap_or(""), + o.get("description").and_then(|d| d.as_str()), + ) + }) + .collect::>(); + assert!( + ops.contains(&("channels/tty/sub", Some("Interactive TTY sessions"))), + "described op carries its description: {ops:?}" + ); + assert!( + ops.contains(&("fs/readFile", None)), + "undescribed op omits the description key: {ops:?}" + ); + } + + #[tokio::test] + async fn services_schema_discloses_description() { + let registry = registry_with_ops(); + let handler = services_schema_handler(Arc::clone(®istry)); + let described = external_spec("fs/readFile").with_description("Read a file"); + registry + .register(HandlerRegistration::new( + described, + HandlerKind::Once(echo_handler()), + OperationProvenance::Local, + None, + None, + Capabilities::new(), + )) + .unwrap(); + let response = handler(json!({ "name": "fs/readFile" }), root_context("req-e02-2")).await; + let spec = response.result.expect("ok response"); + assert_eq!(spec.get("description"), Some(&json!("Read a file"))); + } + + #[test] + fn operation_spec_schema_documents_description_property() { + let schema = operation_spec_schema(); + let props = schema + .get("properties") + .and_then(|v| v.as_object()) + .expect("properties object"); + assert!( + props.contains_key("description"), + "operation_spec_schema must advertise description" + ); + } + #[tokio::test] async fn services_list_filters_by_access_control_authorized_peer() { let registry = registry_with_access_controlled_ops(); diff --git a/src/registry/spec.rs b/src/registry/spec.rs index ff3ec78..a8485e8 100644 --- a/src/registry/spec.rs +++ b/src/registry/spec.rs @@ -181,6 +181,13 @@ pub struct OperationSpec { pub output_schema: Value, pub error_schemas: Vec, pub access_control: AccessControl, + /// Human-readable op description, disclosed by `services/list` when + /// set and by `services/schema` (`spec_to_json_pub`) — review 006 + /// E-02. `None` (absent on the wire) for ops that declare none; the + /// field is additive and registry-side (it describes the op, not the + /// produced resource set — ADR-047 §6 keeps the live half of + /// discovery in `channel/resources/subscribe`). + pub description: Option, /// JSON pointer into the input for the resource ID, when /// `access_control.resource_type` is set and the operation targets a /// specific runtime-spawned resource (ADR-011). e.g. `"$.containerId"` @@ -234,12 +241,22 @@ impl OperationSpec { output_schema, error_schemas, access_control, + description: None, resource_id_path, publish_schema: None, channel_open: None, } } + /// Set the op's `description` (review 006 E-02). Disclosed by + /// `services/list` when set; carried in the `services/schema` wire + /// shape (`spec_to_json_pub` / `rebuild_spec_for`). Builder-style; + /// returns `self` for chaining at registration sites. + pub fn with_description(mut self, description: impl Into) -> Self { + self.description = Some(description.into()); + self + } + /// Set the `publish_schema` (Pub ops only, ADR-046). Validates each /// `call.published` chunk's `input`. Builder-style; returns `self` /// for chaining at registration sites. @@ -359,6 +376,27 @@ mod tests { assert_eq!(spec.channel_open, None); } + #[test] + fn description_defaults_to_none_and_builder_sets_it() { + let spec = OperationSpec::new( + "channels/tty/sub", + OperationType::Sub, + Visibility::External, + serde_json::json!({}), + serde_json::json!({}), + vec![], + AccessControl::default(), + None, + ); + assert_eq!(spec.description, None); + + let described = spec.with_description("Interactive TTY sessions"); + assert_eq!( + described.description.as_deref(), + Some("Interactive TTY sessions") + ); + } + #[test] fn with_channel_open_sets_marker() { let spec = OperationSpec::new(