fix: Unit 3 — register_openable + per-connection ChannelCore (C-02, C-03, C-09)
A channel-open op could not be registered or invoked (C-02):
ChannelCore::register_openable did not exist, and resolve_channel_manager
(C-03) was a stub returning None — the ADR-047 §4 dynamic-resolution
shape (downcast context.env to &dyn ChannelOperationEnv) was unworkable
as written: context.env is a PeerCompositeEnv, not a single concrete
type that can be downcast to a channels-backed env.
The fix is per-connection registration (ADR-047 §4 amendment,
2026-08-13): a ChannelCore is constructed per channels connection (in
the install_channel_zero hook, which already runs per-connection and
already receives the ChannelManager), and register_openable is called on
that connection's overlay OperationRegistry (Layer 2 per ADR-019). The
wrapper closes over the per-connection ChannelCore and uses
ChannelCore::manager() directly — no context.env downcast. This
preserves every invariant ADR-047 §4 was written to protect (layering,
per-connection resolution) without adding as_any() to OperationEnv
(which would close the session/connection overlay patterns from
ADR-024, AGENTS.md §6).
Changes:
- ChannelCore::register_openable wraps the ALPN's OpenHandler with the
ACL→check_open→open_channel→spawn→respond flow (ADR-047 §3). Branches
on spec.op_type: Query/Mutation→Once, Sub→Stream (emits { channel_id }
and completes; data plane on the channel's BiStream), Pub→Sink (stub:
channel:pub_open_not_implemented — requires the channel-adoption path,
C-08/Unit 5). The OpenHandler receives (input, Connection, AuthContext)
and spawns the ALPN's protocol on the channel's BiStream, returning a
JoinHandle for teardown.
- ChannelManager::set_handler_task installs the spawned OpenHandler's
JoinHandle after open_channel (which allocates the channel first to
get the BiStream halves, then the handler is spawned, then the task is
recorded for abort on channel/close / connection drop).
- channel:too_many_channels / channel:allocation_failed error codes
mapped to CallError with details (channel:forbidden is the ACL's
FORBIDDEN, already handled by the registry before the wrapper).
- resolve_channel_manager stub removed (C-03); ChannelOperationEnv trait
and ChannelsSessionEnv retained as a two-way-door implementation detail
for future per-connection routing (not on the open-op path). The
tautology filler test (C-22 env.rs) removed.
- ADR-047 §4 amendment records the per-connection-registration decision
(two-way door: the ADR's door-type section explicitly marks the wrapper
shape as a two-way-door implementation detail; the one-way decisions
— per-ALPN op names, channel_open marker, removal of channel/open —
are unchanged).
Acceptance gate (C-02/C-03): one end-to-end test wires ChannelClient ↔
ChannelsAdapter over a real tokio::io::duplex carrying the channels
8-byte chunk header wire format. The accept side's install_channel_zero
hook builds a per-connection ChannelCore, registers a no-op open op
(channels/tty/sub) via register_openable, and runs the dispatch loop.
The client calls call_open_op("channels/tty/sub") on channel 0; the
wrapper does check_open→open_channel→spawn→respond. Asserts the
response carries a non-zero channel_id and that the per-identity quota
was reserved (policy count for the caller incremented to 1).
Verification: 438 tests pass (was 437; +1 e2e), clippy clean, fmt clean,
doc warnings 2 (was 4; fixed the 2 register_openable broken-link
warnings — C-09; the remaining default_policy and env module/macro
warnings are Unit 6 long-tail items).
This commit is contained in:
1 parent
495e04ed43
commit
8066d08395
5 files changed
+657
-60
No files matched your search
@@ -172,6 +172,7 @@ impl ChannelClient {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::channels::adapter::ChannelsAdapter;
|
||||
use crate::channels::policy::ChannelLifecyclePolicy;
|
||||
use crate::core::auth::{AuthContext, IdentityProvider};
|
||||
use crate::core::types::Connection;
|
||||
use crate::protocol::connection::split_single_stream;
|
||||
@@ -445,4 +446,156 @@ mod tests {
|
||||
"last chunk matches"
|
||||
);
|
||||
}
|
||||
|
||||
/// C-02 / C-03 — the end-to-end acceptance gate for Unit 3
|
||||
/// (`register_openable`, ADR-047 §3 as amended 2026-08-13 —
|
||||
/// per-connection registration). Wires `ChannelClient` (connect
|
||||
/// side) ↔ `ChannelsAdapter` (accept side) over a real
|
||||
/// `tokio::io::duplex` carrying the channels 8-byte chunk header
|
||||
/// wire format. The accept side's `install_channel_zero` hook
|
||||
/// builds a per-connection `ChannelCore` from the adapter-supplied
|
||||
/// `ChannelManager`, registers a no-op open op
|
||||
/// (`channels/tty/sub`) via `register_openable` on a fresh
|
||||
/// per-connection registry, and runs the single-stream dispatch
|
||||
/// loop on it. The client calls `call_open_op("channels/tty/sub")`
|
||||
/// on channel 0; the wrapper does `check_open` → `open_channel` →
|
||||
/// spawn the no-op `OpenHandler` on the channel's `Connection` →
|
||||
/// respond `{ channel_id }`. Asserts the response carries a
|
||||
/// `channel_id` and that the per-identity quota was reserved
|
||||
/// (the policy's count for the caller incremented).
|
||||
///
|
||||
/// This is the gate the review identifies as missing: "register a
|
||||
/// no-op open op via `register_openable`, invoke it end-to-end
|
||||
/// through channel 0, assert `channel_id` is returned and quota is
|
||||
/// reserved." It would have caught C-02 (no way to register an
|
||||
/// open op) and C-03 (`resolve_channel_manager` stub) immediately.
|
||||
#[tokio::test]
|
||||
async fn channel_0_end_to_end_register_openable_returns_channel_id() {
|
||||
use crate::channels::operations::{ChannelCore, OpenHandler};
|
||||
use crate::channels::policy::PerIdentityChannelPolicy;
|
||||
use crate::registry::spec::ChannelOpenSpec;
|
||||
|
||||
let policy: Arc<PerIdentityChannelPolicy> = Arc::new(PerIdentityChannelPolicy::new(256));
|
||||
let policy_for_hook: Arc<dyn ChannelLifecyclePolicy> =
|
||||
Arc::clone(&policy) as Arc<dyn ChannelLifecyclePolicy>;
|
||||
let policy_for_assert = Arc::clone(&policy);
|
||||
|
||||
let open_handler: OpenHandler = Arc::new(|_input, _channel_conn, _auth| {
|
||||
tokio::spawn(async move {
|
||||
// No-op: the channel's BiStream is available via
|
||||
// `_channel_conn.accept_bi()` if the test wanted
|
||||
// to move data on it. For the open-succeeds gate,
|
||||
// just keep the task alive briefly so the handler
|
||||
// task is real (and recorded for teardown).
|
||||
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
|
||||
})
|
||||
});
|
||||
|
||||
let install_hook: crate::channels::adapter::InstallChannelZero =
|
||||
Arc::new(move |manager, channel0_conn, auth| {
|
||||
let open_handler = Arc::clone(&open_handler);
|
||||
let policy = Arc::clone(&policy_for_hook);
|
||||
tokio::spawn(async move {
|
||||
let channel0_bidi = match channel0_conn.accept_bi().await {
|
||||
Ok(s) => s,
|
||||
Err(_) => return,
|
||||
};
|
||||
let (writer, reader) = split_single_stream(channel0_bidi);
|
||||
|
||||
let core = ChannelCore::new(manager, policy);
|
||||
let mut registry = crate::registry::registration::OperationRegistry::new();
|
||||
let spec = OperationSpec::new(
|
||||
"channels/tty/sub",
|
||||
OperationType::Sub,
|
||||
Visibility::External,
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"container": { "type": "string" }
|
||||
},
|
||||
"required": ["container"]
|
||||
}),
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"channel_id": { "type": "integer" }
|
||||
}
|
||||
}),
|
||||
vec![],
|
||||
AccessControl::default(),
|
||||
None,
|
||||
)
|
||||
.with_channel_open(ChannelOpenSpec::new("alknet/tty"));
|
||||
core.register_openable(
|
||||
spec,
|
||||
Arc::clone(&open_handler),
|
||||
&mut registry,
|
||||
auth.clone(),
|
||||
)
|
||||
.expect("register_openable");
|
||||
|
||||
let registry = Arc::new(registry);
|
||||
let provider: Arc<dyn IdentityProvider> = Arc::new(NoopIdProvider);
|
||||
let call_connection = Arc::new(CallConnection::new_single_stream(
|
||||
channel0_conn,
|
||||
Arc::clone(&writer),
|
||||
));
|
||||
let dp = Dispatcher::new(registry, provider);
|
||||
dp.run_loop_single_stream(call_connection, reader, writer)
|
||||
.await;
|
||||
})
|
||||
});
|
||||
|
||||
let (client_end, server_end) = tokio::io::duplex(64 * 1024);
|
||||
let client_conn =
|
||||
Connection::from_bidi(client_end, b"alknet/channels".to_vec(), Some(TEST_ADDR));
|
||||
let server_conn =
|
||||
Connection::from_bidi(server_end, b"alknet/channels".to_vec(), Some(TEST_ADDR));
|
||||
|
||||
let adapter = ChannelsAdapter::new(install_hook);
|
||||
let auth = AuthContext::anonymous(b"alknet/channels");
|
||||
let _server_handle = tokio::spawn(async move {
|
||||
let _ = crate::core::types::ProtocolHandler::handle(&adapter, server_conn, &auth).await;
|
||||
});
|
||||
|
||||
let client = ChannelClient::from_connection(client_conn)
|
||||
.await
|
||||
.expect("channel client init");
|
||||
|
||||
let response = tokio::time::timeout(
|
||||
std::time::Duration::from_secs(10),
|
||||
client.call_open_op(
|
||||
"channels/tty/sub",
|
||||
serde_json::json!({ "container": "abc" }),
|
||||
),
|
||||
)
|
||||
.await
|
||||
.expect("channel-0 open op timed out");
|
||||
|
||||
assert!(
|
||||
response.result.is_ok(),
|
||||
"open op should succeed, got {:?}",
|
||||
response.result
|
||||
);
|
||||
let out = response.result.unwrap();
|
||||
let channel_id = out
|
||||
.get("channel_id")
|
||||
.and_then(|v| v.as_u64())
|
||||
.expect("channel_id in response");
|
||||
assert!(
|
||||
channel_id > 0,
|
||||
"channel_id should be non-zero (channel 0 is the call channel), got {channel_id}"
|
||||
);
|
||||
|
||||
let anonymous = crate::core::auth::Identity {
|
||||
id: "anonymous".to_string(),
|
||||
scopes: vec![],
|
||||
resources: Default::default(),
|
||||
};
|
||||
let count = policy_for_assert.count_for(&anonymous);
|
||||
assert_eq!(
|
||||
count, 1,
|
||||
"quota reserved: the open op incremented the per-identity count"
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user