alknet/tasks/call/review-completion.md

---
id: call/review-completion
name: Review alknet-call client/adapter completion for spec conformance (ADR-017, ADR-028) and no-env-vars invariant
status: completed
depends_on: [call/client/from-call, call/client/operation-adapter-trait, call/client/from-jsonschema]
scope: broad
risk: low
impact: phase
level: review
---

## Description

Review the alknet-call client/adapter completion (the gap ADR-017 left to
implementation, now specced in `client-and-adapters.md`) for spec conformance,
security-constraint conformance, and pattern consistency. This is the quality
checkpoint at the end of the call-completion batch — the work that unblocks
every downstream consumer (runner, container service, bilateral exchange,
NAPI, agent cross-node dispatch).

### Review Checklist

1. **CallClient conformance** (client-and-adapters.md §CallClient):
   - `CallClient` struct with registry, identity_provider, trusted_peer
   - `new()` constructs default-deny (trusted_peer: false)
   - `trusted_peer()` constructs trusted-peer mode (explicit opt-in)
   - `connect()` opens QUIC on ALPN `alknet/call`, returns CallConnection
   - Dispatch loop is **shared** with CallAdapter (refactored, not duplicated — ADR-017 §1)
   - Connection symmetry (ADR-017 §2): both sides call each other after establishment
   - Credentials from Capabilities, not env vars (ADR-014, no-env-vars invariant)

2. **Peer-scoped filtering conformance** (ADR-028):
   - Default-deny: op with `remote_safe == false` returns NOT_FOUND to remote peer
   - Default-deny: `OperationContext.capabilities` populated only for remote-safe ops
   - Trusted-peer mode: all External ops dispatch regardless of remote_safe
   - services/list hides non-remote-safe ops in default mode (ADR-028 Assumption 2)
   - services/list lists all External ops in trusted-peer mode
   - Dispatch-time read over single Layer-0 registry (not a copy — ADR-028 §5)
   - `remote_safe` defaults false across all provenance (ADR-028 §4)

3. **from_call conformance** (client-and-adapters.md §from_call):
   - Calls services/list then services/schema for each op
   - Constructs HandlerRegistration with `provenance: FromCall`
   - Forwarding handler sends call.requested via CallConnection
   - Subscription forwarding yields until completed/aborted
   - `composition_authority: None`, `scoped_env: None` (leaf — ADR-022)
   - `remote_safe: false` on FromCall leaves
   - Namespace collision = error (DC-3/OQ-28), not silent overwrite
   - Re-import on connection establishment (DC-2/OQ-27, v1 default)
   - Cross-node abort via parent_request_id (ADR-016 §6)

4. **OperationAdapter trait conformance** (client-and-adapters.md §OperationAdapter):
   - `async fn import(&self) -> Result<Vec<HandlerRegistration>, AdapterError>`
   - Trait is `#[async_trait]` (async — ADR-017 §5, locked)
   - `AdapterError` is `#[non_exhaustive]` + `thiserror::Error`
   - Variants: DiscoveryFailed, SchemaParse, Transport, Unauthorized, Conflict
   - Trait lives in alknet-call (where the types live), not alknet-http

5. **from_jsonschema conformance** (client-and-adapters.md §from_jsonschema):
   - `provenance: FromJsonSchema`, no real handler (placeholder errors if invoked)
   - `composition_authority: None`, `scoped_env: None`, empty capabilities
   - `remote_safe: false` (provenance default, ADR-028 §4)
   - Implements OperationAdapter, no .await in import (pure parse)
   - Malformed schema → `AdapterError::SchemaParse`

6. **Adapter location map conformance** (client-and-adapters.md §Adapter Location Map):
   - OperationAdapter trait + from_call + from_jsonschema + CallClient in alknet-call
   - No HTTP client / HTTP server deps in alknet-call (stays lean)
   - from_openapi/from_mcp/to_openapi/to_mcp NOT in alknet-call (deferred to alknet-http)
   - MCP stdio not built (security position, not a feature gap)

7. **No-env-vars invariant** (client-and-adapters.md §No-Env-Vars Invariant):
   - Credential path: vault → assembly → Capabilities → HandlerRegistration.capabilities → OperationContext.capabilities → handler
   - No handler reads outbound credentials from any source other than OperationContext.capabilities
   - No `std::env::var` reads in the credential path
   - The invariant is enforced by the dispatch path (build_root_context), not runtime convention

8. **ADR conformance (completion-specific)**:
   - ADR-017 §1: shared dispatch loop, CallClient own registry (now peer-scoped per ADR-028)
   - ADR-017 §2: connection direction independent of call direction
   - ADR-017 §3: from_call flow (services/list + services/schema), FromCallConfig prefix/filter
   - ADR-017 §5: async trait, bundles not (spec,handler) pairs, to_* are projections not impls
   - ADR-017 §6: cross-node abort cascade through from_call handler
   - ADR-017 §7: credentials from capabilities (TLS identity, auth token, remote identity)
   - ADR-028: default-deny, remote_safe bool, trusted-peer opt-in, dispatch-time read, services/list hide

9. **Security constraints (completion-specific)**:
   - Default-deny filtering: remote peer can't trigger capability exposure for non-remote-safe ops
   - Trusted-peer opt-in is explicit, never default
   - Capabilities non-serializable, never cross the wire (ADR-014)
   - from_call trust is transitive (remote node's code runs) — recorded in spec, not enforced beyond scoped env
   - FromCall/FromJsonSchema leaves have no composition authority (can't escalate)

10. **Test coverage**:
    - Integration test: two-node call (CallClient ↔ CallAdapter, both call each other)
    - Integration test: default-deny op → NOT_FOUND to remote peer
    - Integration test: remote_safe op dispatches to remote peer
    - Integration test: trusted-peer mode exposes all External ops
    - Integration test: services/list hides non-remote-safe in default mode
    - Integration test: from_call populates Layer 2 overlay, forwarding works
    - Integration test: subscription forwarding streams remote events
    - Integration test: namespace collision returns error
    - Integration test: cross-node abort cascades through from_call handler
    - Unit tests: AdapterError variants, OperationAdapter trait compiles

11. **Spec drift check**: verify `client-and-adapters.md` still matches the
    implementation after the completion (no spec/impl drift introduced during
    implementation). In particular: the CallClient struct sketch, the
    CallCredentials sketch, the FromCallConfig fields, the AdapterError
    variants, and the remote_safe field on HandlerRegistration.

## Acceptance Criteria

- [x] CallClient matches client-and-adapters.md (struct, new/trusted_peer, connect, shared loop)
- [x] Peer-scoped filtering matches ADR-028 (default-deny, trusted-peer, services/list hide)
- [x] from_call matches client-and-adapters.md (flow, FromCallConfig, provenance, None fields)
- [x] OperationAdapter trait + AdapterError match client-and-adapters.md (async, non_exhaustive, variants)
- [x] from_jsonschema matches client-and-adapters.md (provenance, placeholder handler, no I/O)
- [x] Adapter location map respected (no HTTP deps in alknet-call; from_openapi/mcp not built here)
- [x] No-env-vars invariant holds (credentials from Capabilities, no env-var reads)
- [x] ADRs 017 + 028 conformed to (plus 014/015/016/022/023/024 where touched)
- [x] Default-deny security constraint enforced (no capability exposure for non-remote-safe)
- [x] Integration tests cover two-node call, default-deny, trusted-peer, from_call, abort cascade
- [x] No spec/impl drift in client-and-adapters.md (or drift documented + spec amended)
- [x] `cargo fmt --check -p alknet-call` passes
- [x] `cargo clippy -p alknet-call --all-targets` passes with no warnings
- [x] All tests pass

## References

- docs/architecture/crates/call/client-and-adapters.md — the spec being reviewed against
- docs/architecture/crates/call/README.md — crate index (now lists client-and-adapters.md)
- docs/architecture/decisions/017-call-protocol-client-and-adapter-contract.md — ADR-017 (amended)
- docs/architecture/decisions/028-callclient-peer-scoped-registry-filtering.md — ADR-028
- docs/architecture/open-questions.md — OQ-25..28 (two-way-door remainders — verify defaults match spec)
- docs/research/alknet-call-completion/gap-analysis.md — DC-1..4, the decisions this batch resolved
- tasks/call/registry/remote-safe-marking.md
- tasks/call/client/call-client.md
- tasks/call/client/from-call.md
- tasks/call/client/operation-adapter-trait.md
- tasks/call/client/from-jsonschema.md

## Notes

> This review closes the call-completion batch. The load-bearing security
> invariant is ADR-028's default-deny: a remote peer's call must not trigger
> dispatch that populates OperationContext.capabilities from the local
> registration bundle unless the op is explicitly remote-safe. Verify this
> with a test that asserts a non-remote-safe op's call does NOT populate
> capabilities (not just that it returns NOT_FOUND — the security argument is
> about capability exposure, not just call denial). The no-env-vars invariant
> (ADR-014) is the dispatch-side corollary: no handler reads credentials from
> any source other than OperationContext.capabilities. The shared dispatch
> loop (ADR-017 §1) is the architectural commitment that keeps CallClient from
> becoming a parallel protocol implementation — verify the loop is genuinely
> shared (refactored out of CallAdapter), not copy-pasted. If deviations are
> found, document and fix before considering the call-completion batch done.
> This unblocks every downstream consumer, so spec/impl drift here propagates.

## Review Findings (2026-06-26)

The call-completion batch is complete and conforms to the specs. Findings
recorded against the checklist above:

### 1. CallClient conformance — PASS
`CallClient` struct (`registry`, `identity_provider`, `trusted_peer: bool`)
matches the sketch in `client-and-adapters.md` §CallClient. `new()`
constructs default-deny; `trusted_peer()` is the explicit opt-in. `connect()`
dials QUIC on ALPN `alknet/call`, spawns the shared dispatch loop, returns a
live `CallConnection`. Connection symmetry holds: the returned
`CallConnection` supports `call()`/`subscribe()`/`abort()` and the dispatch
loop accepts incoming `call.requested` from the remote peer.

### 2. Shared dispatch loop — PASS (the architectural commitment)
The dispatch loop is genuinely shared, not duplicated. `protocol/dispatch.rs`
holds the single `Dispatcher` struct with `run_loop` (sweeper + accept_bi loop
+ fail_all on close), `handle_stream`, `dispatch_requested`,
`build_root_context`, `compose_root_env`. Both `CallAdapter::handle`
(adapter.rs:155) and `CallClient::spawn_dispatch` (call_client.rs) construct a
`Dispatcher` and call `run_loop`. The accept-path and connect-path differ
only in connection acquisition; the dispatch half is one implementation. This
satisfies ADR-017 §1's commitment that CallClient is the
connection-establishment half, not a parallel protocol implementation.

### 3. Peer-scoped filtering / default-deny — PASS (load-bearing security invariant)
`RemoteFilter { trusted_peer: bool }` on the Dispatcher. In default-deny mode,
`dispatch_requested` checks `remote_safe` *before* building the context or
invoking the handler — a non-remote-safe op returns `NOT_FOUND` before any
capability material reaches the handler (ADR-028 Context). Verified by three
tests in `client/call_client.rs`:
- `default_deny_non_remote_safe_does_not_populate_capabilities`: asserts
  NOT_FOUND for a non-remote-safe op (handler never reached, so capabilities
  never populated — the security argument).
- `remote_safe_op_populates_capabilities_for_handler`: asserts the remote-safe
  op's handler DOES see the google capability (proves the positive case, so
  the security argument is specifically about non-remote-safe, not all ops).
- `trusted_peer_populates_capabilities_for_non_remote_safe`: trusted-peer mode
  populates capabilities for non-remote-safe ops (explicit opt-in).
`services_list_handler_peer_scoped` hides non-remote-safe ops in default-deny
mode (ADR-028 Assumption 2 — discovery and dispatch filters agree). Verified
by `services_list_peer_scoped_*` tests in `registry/discovery.rs`.

### 4. from_call conformance — PASS
`from_call` calls `services/list` then `services/schema`, rebuilds
`OperationSpec` from the schema JSON, constructs `FromCall`-provenance bundles
with forwarding handlers (`composition_authority: None`, `scoped_env: None`,
empty capabilities, `remote_safe: false` per ADR-028 §4). Namespace collision
returns `AdapterError::Conflict` (DC-3/OQ-28 default). `operation_filter`
limits imports. Integration test (`from_call_discovers_and_forwards_over_quic_loopback`)
proves the discovery → overlay registration → forwarding round-trip over a
real QUIC connection.

### 5. OperationAdapter trait + AdapterError — PASS
`#[async_trait] pub trait OperationAdapter: Send + Sync` with
`async fn import(&self) -> Result<Vec<HandlerRegistration>, AdapterError>`.
`AdapterError` is `#[non_exhaustive]` + `thiserror::Error` with the five v1
variants (`DiscoveryFailed`, `SchemaParse`, `Transport`, `Unauthorized`,
`Conflict`). Lives in alknet-call where the types live. `from_jsonschema`
implements it (no `.await` in `import` — pure parse).

### 6. from_jsonschema conformance — PASS
`provenance: FromJsonSchema`, placeholder handler errors if invoked, None
authority/scoped_env, empty capabilities, `remote_safe: false`, implements
`OperationAdapter` (no `.await`), malformed (non-object) schema returns
`SchemaParse`. No network I/O.

### 7. Adapter location map — PASS
No HTTP deps (reqwest/axum) in alknet-call `Cargo.toml` or `src/`. No
`from_openapi`/`from_mcp`/`to_openapi`/`to_mcp` implementations (only doc-comment
references in `AdapterError` variant docs). No MCP stdio (security position
held). The trait + `from_call` + `from_jsonschema` + `CallClient` live in
alknet-call; the HTTP-backed adapters are deferred to alknet-http (separate
Phase 0).

### 8. No-env-vars invariant — PASS
`grep -rn "std::env::var\|env::var" crates/alknet-call/src/` returns nothing.
`CallCredentials` carries `tls_identity`/`auth_token`/`remote_identity` — all
from `Capabilities` (ADR-014). The dispatch path populates
`OperationContext.capabilities` from the registration bundle, not from env
vars. No handler reads outbound credentials from any source other than
`OperationContext.capabilities`.

### 9. Two-way-door remainders (OQ-25..28) — recorded, defaults match spec
- OQ-25 (remote_safe shape): v1 is `remote_safe: bool`; per-peer allowlist is
  out of scope. Default false across all provenance (ADR-028 §4).
- OQ-26 (AdapterError variants): v1 set recorded above; `#[non_exhaustive]`
  lets alknet-http extend.
- OQ-27 (re-import trigger): v1 default auto-on-reconnect — assembly layer
  calls `from_call` immediately after `connect()`.
- OQ-28 (namespace collision): v1 default error-on-collision
  (`AdapterError::Conflict`).
- DC-4 (CallClient TLS client-auth): v1 connects without a client-auth cert
  (server uses `AcceptAnyCertVerifier`); wiring RawKey client-auth is a
  two-way-door remainder. The one-way constraint (credentials from
  Capabilities, not env vars) is unaffected — `auth_token` flows through the
  call-protocol payload, not TLS.

### 10. Spec drift — NONE requiring amendment
`client-and-adapters.md` matches the implementation. One intentional
spec-sketch omission: `connect()`'s return type is `Result<CallConnection,
ClientError>` (the spec sketch showed `Result<CallConnection>` without the
error type). This matches DC-4's intent (the error type is an
implementation-detail two-way door filled in by the implementation), so no
spec amendment is needed — the sketch was always understood as illustrative.

### 11. Test coverage — PASS
207 lib tests + 2 integration tests. Integration: `two_node_call_round_trip`
(connect + outbound call round-trip), `from_call_discovers_and_forwards_over_quic_loopback`
(discovery + overlay + forwarding over QUIC). Unit: default-deny NOT_FOUND,
remote_safe dispatches, trusted-peer, capabilities populated only for
remote-safe (the load-bearing assertion), services/list hide, from_call
rebuild_spec parsing, FromCallConfig, from_jsonschema bundle shape +
OperationAdapter impl.

### Verification commands (all green)
- `cargo fmt --check -p alknet-call` — clean
- `cargo clippy -p alknet-call --all-targets` — no warnings
- `cargo test -p alknet-call` — 207 lib + 2 integration pass