feat: allocation moves into the channels establisher (alkcall 0.6.0 / review 007 R-01) — bump to 0.3.0
- Bump alkcall 0.5.0 -> 0.6.0 (Establishment plan payload, ADR-049 amendment 2); alktty 0.2.0 -> 0.3.0 (breaking channels-path allocation-failure shape) - make_tty_establisher runs backend.allocate; failure maps to EstablishmentError::DialFailed (channel:open_failed reason dial_failed); the TtyHandle crosses to the pump handler via a private per-open AllocatedHandle one-shot slot (the handle is not Sync — it cannot be the ChannelPlan directly) - make_tty_open_handler gains the Option<ChannelPlan> parameter; a plan drives the new drive_session_pre_allocated (pumps only), a None plan falls back to drive_session_pre_negotiated (inline validate-and-allocate — defense-in-depth for no-establisher registrations) - tty_open_spec's channel:open_failed ErrorDefinition declares dial_failed (four reachable reasons) - Pinned in-band allocate test flipped: allocate_failure_fails_open_as_dial_failed (end-to-end, no channel survives); establisher unit gate for the plan slot + failure mapping - ADR-010 amended (§2A; §2 kept as historical record), tty-adapter.md, session/channels/adapter docs, CHANGELOG, AGENTS.md alkcall pin updated Verification: cargo test (113) + --all-features (137), clippy -D warnings (native + wasm32, default + all-features), fmt, doc, wasm32 check — all clean
This commit is contained in:
1 parent
3352a02dd5
commit
af9dbaedea
10 files changed
+434
-147
No files matched your search
+187
-60
@@ -18,13 +18,13 @@
|
||||
//! `channel:open_failed` on rejection — no phantom channel. The
|
||||
//! `TtyOpenHandler` receives the channel's `Connection` (data-plane
|
||||
//! ALPN `alk/tty`), calls `accept_bi()` to get the channel's
|
||||
//! `BiStream`, and runs [`crate::adapter::drive_session_pre_negotiated`]
|
||||
//! on it — the same three-pump session driver as the direct-ALPN path,
|
||||
//! minus the wire-frame negotiation phase (the open op's `input` is
|
||||
//! the negotiation, ADR-009). Backend allocation stays in the pump
|
||||
//! handler (`allocate_failed` still arrives in-band as a negotiation
|
||||
//! error frame — the establisher cannot carry the allocated handle
|
||||
//! across to the handler; ADR-010).
|
||||
//! `BiStream`, and runs the session pumps on it — the same three-pump
|
||||
//! session driver as the direct-ALPN path, minus the wire-frame
|
||||
//! negotiation phase (the open op's `input` is the negotiation,
|
||||
//! ADR-009). Backend allocation runs in the establisher (alkcall 0.6
|
||||
//! R-01: the allocated `TtyHandle` crosses via the `Establishment`
|
||||
//! plan payload); no negotiation error frame is ever written on the
|
||||
//! channels path by a registered producer.
|
||||
//!
|
||||
//! The access control (scope-gate + ownership) is wired into the
|
||||
//! `OperationSpec`'s `AccessControl` and enforced by the registry's
|
||||
@@ -37,10 +37,10 @@
|
||||
//! [`ChannelCore`]: alkcall::channels::operations::ChannelCore
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
use alkcall::channels::operations::{
|
||||
ChannelCore, Establishment, EstablishmentError, OpenEstablisher, OpenHandler,
|
||||
ChannelCore, ChannelPlan, Establishment, EstablishmentError, OpenEstablisher, OpenHandler,
|
||||
};
|
||||
use alkcall::core::auth::AuthContext;
|
||||
use alkcall::core::ownership::OwnershipProvider;
|
||||
@@ -148,9 +148,10 @@ pub fn register_openable(
|
||||
/// here would create a second definition to drift. The registry check
|
||||
/// catches structurally-broken opens (missing command, wrong types);
|
||||
/// the full parse and semantic validation (`carriage == "raw"`,
|
||||
/// backend lookup, ownership) runs in the establisher (ADR-010 —
|
||||
/// failures are `channel:open_failed`); `backend.allocate` runs in the
|
||||
/// pump handler.
|
||||
/// backend lookup, ownership) and backend allocation run in the
|
||||
/// establisher (ADR-010, as amended for alkcall 0.6 — failures are
|
||||
/// `channel:open_failed`, including `allocate_failed` as
|
||||
/// `dial_failed`).
|
||||
///
|
||||
/// The op declares an [`ErrorDefinition`] for `channel:open_failed`
|
||||
/// (ADR-016 — establishment failures are disclosed so `services/schema`
|
||||
@@ -164,7 +165,7 @@ pub fn tty_open_spec() -> OperationSpec {
|
||||
"properties": {
|
||||
"reason": {
|
||||
"type": "string",
|
||||
"enum": ["unknown_resource", "handler_error", "timeout"]
|
||||
"enum": ["dial_failed", "unknown_resource", "handler_error", "timeout"]
|
||||
},
|
||||
"message": { "type": "string" }
|
||||
},
|
||||
@@ -196,8 +197,9 @@ pub fn tty_open_spec() -> OperationSpec {
|
||||
code: "channel:open_failed".to_string(),
|
||||
description: "Establishment failed after channel allocation: the establisher \
|
||||
rejected the negotiation (malformed negotiation, unknown backend, \
|
||||
ownership denial) or the establishment deadline expired. Details: \
|
||||
{ reason, message }. No channel_id is returned."
|
||||
ownership denial), the backend allocation failed (dial_failed), or \
|
||||
the establishment deadline expired. Details: { reason, message }. \
|
||||
No channel_id is returned."
|
||||
.to_string(),
|
||||
schema: open_failed_schema,
|
||||
http_status: None,
|
||||
@@ -226,11 +228,15 @@ pub fn tty_open_spec() -> OperationSpec {
|
||||
/// full `NegotiateRequest` parse of the registry-schema-validated
|
||||
/// `input` (the schema is deliberately partial so the opaque ADR-053
|
||||
/// backend params pass through), `carriage == "raw"`, non-empty
|
||||
/// `cmd`, the backend lookup, and the ADR-050 ownership check. Any
|
||||
/// rejection resolves `Err(EstablishmentError)` and the wrapper
|
||||
/// replies `channel:open_failed` with `details.reason` — the SSH
|
||||
/// contract: the channel never exists opener-side, no `channel_id` is
|
||||
/// ever returned.
|
||||
/// `cmd`, the backend lookup, the ADR-050 ownership check, and —
|
||||
/// since alkcall 0.6 (ADR-049 amendment 2, review 007 R-01) gave
|
||||
/// `Establishment` its plan payload — the backend allocation
|
||||
/// (ADR-010 as amended: the allocated `TtyHandle` crosses to the pump
|
||||
/// handler via the plan). Any rejection resolves
|
||||
/// `Err(EstablishmentError)` and the wrapper replies
|
||||
/// `channel:open_failed` with `details.reason` — the SSH contract:
|
||||
/// the channel never exists opener-side, no `channel_id` is ever
|
||||
/// returned.
|
||||
///
|
||||
/// The `tty:open` scope gate is NOT checked here — the registry's
|
||||
/// `AccessControl` enforced it before the establisher ran (the
|
||||
@@ -249,14 +255,23 @@ pub fn tty_open_spec() -> OperationSpec {
|
||||
/// - Ownership denial → [`EstablishmentError::HandlerError`] (an ACL
|
||||
/// outcome, not a dial outcome; the registry's scope gate stays the
|
||||
/// pre-allocation `FORBIDDEN` path).
|
||||
/// - `backend.allocate` failure (ADR-005's `AllocFailed`: PTY
|
||||
/// couldn't be allocated, docker exec failed to start, SSH channel
|
||||
/// request rejected) → [`EstablishmentError::DialFailed`] — the
|
||||
/// target refused or the backend lacked capacity; the review-007
|
||||
/// verification gate's "allocate in the establisher →
|
||||
/// `allocate_failed` is a call error, no in-band frame" shape.
|
||||
///
|
||||
/// Backend allocation deliberately does NOT happen here: the
|
||||
/// establisher has no way to hand the allocated `TtyHandle` to the
|
||||
/// pump handler (`Establishment` is payloadless — reserved for a
|
||||
/// channel plan), and a second `allocate` for the same open would
|
||||
/// violate the kill-on-`Drop` contract (ADR-005). `allocate_failed`
|
||||
/// therefore stays a post-open in-band negotiation error frame
|
||||
/// (ADR-010).
|
||||
/// The allocation is handed to the pump handler through the
|
||||
/// establishment plan: a private `AllocatedHandle` slot (the
|
||||
/// `TtyHandle` itself is not `Sync` — its fields are boxed dyn
|
||||
/// streams — so it cannot be the `ChannelPlan` directly). The slot is
|
||||
/// created per establisher invocation (per open), so the
|
||||
/// shared-slot same-resource race alkcall's R-01 test pins is
|
||||
/// unreachable by construction; the wrapper moves the plan to this
|
||||
/// open's handler and the slot is `take`n exactly once. The
|
||||
/// establisher-before-handler ordering is alkcall's wrapper
|
||||
/// guarantee (ADR-049 §1).
|
||||
fn make_tty_establisher(
|
||||
backends: Arc<HashMap<String, Arc<dyn TtyBackend>>>,
|
||||
ownership: Option<Arc<dyn OwnershipProvider>>,
|
||||
@@ -311,44 +326,77 @@ fn make_tty_establisher(
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(Establishment {})
|
||||
let handle = match backend.allocate(¶ms).await {
|
||||
Ok(h) => h,
|
||||
Err(e) => {
|
||||
debug!("tty: channels establisher: allocation failed: {e}");
|
||||
return Err(EstablishmentError::DialFailed {
|
||||
message: e.to_string(),
|
||||
});
|
||||
}
|
||||
};
|
||||
Ok(Establishment::new(Arc::new(AllocatedHandle {
|
||||
handle: Mutex::new(Some(handle)),
|
||||
}) as ChannelPlan))
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
/// The establishment plan payload for the channels path: the
|
||||
/// `TtyHandle` the establisher allocated, behind a one-shot slot (the
|
||||
/// handle's boxed dyn fields make the struct non-`Sync`, so it cannot
|
||||
/// be the `ChannelPlan` directly). Created per open; `take`n by the
|
||||
/// pump handler.
|
||||
struct AllocatedHandle {
|
||||
handle: Mutex<Option<crate::backend::TtyHandle>>,
|
||||
}
|
||||
|
||||
impl AllocatedHandle {
|
||||
fn take(&self) -> Option<crate::backend::TtyHandle> {
|
||||
self.handle.lock().unwrap_or_else(|e| e.into_inner()).take()
|
||||
}
|
||||
}
|
||||
|
||||
/// Build the [`OpenHandler`] for `channels/tty/sub`.
|
||||
///
|
||||
/// The handler receives the open op's `input` — the `NegotiateRequest`
|
||||
/// params, validated by the establisher before the handler was spawned
|
||||
/// (alkcall ADR-049 §1: the establisher is awaited bounded, before the
|
||||
/// reply and before the pump handler) — re-parses it, calls
|
||||
/// params, validated (and allocated against) by the establisher before
|
||||
/// the handler was spawned (alkcall ADR-049 §1: the establisher is
|
||||
/// awaited bounded, before the reply and before the pump handler) —
|
||||
/// plus the establisher's `plan`: the [`AllocatedHandle`] slot
|
||||
/// carrying the backend-allocated `TtyHandle` (alkcall 0.6 ADR-049
|
||||
/// amendment 2, review 007 R-01). It re-parses `input`, calls
|
||||
/// `accept_bi()` to get the channel's [`BiStream`], splits it into
|
||||
/// read/write halves (the stdlib `tokio::io::split` idiom — the same
|
||||
/// split the direct-ALPN `TtyAdapter::handle` does), and runs
|
||||
/// [`crate::adapter::drive_session_pre_negotiated`] on them — the same
|
||||
/// three-pump session driver as the direct-ALPN path. The establisher
|
||||
/// already rejected the semantic-failure classes (malformed params,
|
||||
/// `carriage != "raw"`, unknown backend, ownership denial) as
|
||||
/// `channel:open_failed`, so the driver's `validate_and_allocate` pass
|
||||
/// on this path only re-runs the allocation — its negotiation error
|
||||
/// frames remain reachable only for `allocate_failed` (the establisher
|
||||
/// cannot carry the handle across — ADR-010).
|
||||
/// split the direct-ALPN `TtyAdapter::handle` does), and runs the
|
||||
/// pre-allocated session driver on them — the same three-pump session
|
||||
/// driver as the direct-ALPN path.
|
||||
///
|
||||
/// With a plan the handler never allocates: the establisher already
|
||||
/// ran `backend.allocate` (ADR-010 as amended), so no negotiation
|
||||
/// error frame is reachable on the registered path. With `plan: None`
|
||||
/// (a no-establisher registration — defense-in-depth, the ADR-010
|
||||
/// pre-0.3.0 shape) the handler re-runs the full inline
|
||||
/// validate-and-allocate, whose negotiation error frames
|
||||
/// (`malformed_negotiation`, `allocate_failed`, ...) remain the only
|
||||
/// failure surface for that registration shape.
|
||||
///
|
||||
/// The returned task awaits the session driver inline (R-02 lifetime
|
||||
/// contract): the `JoinHandle`'s completion is the wrapper's teardown
|
||||
/// trigger, so the pumps must finish before the task returns — never
|
||||
/// spawn-and-forget.
|
||||
///
|
||||
/// The `tty:open` scope gate is enforced by the registry's `AccessControl`
|
||||
/// before this handler runs — the handler does not re-check it.
|
||||
///
|
||||
/// The handler's `JoinHandle` is recorded by the channels wrapper for
|
||||
/// teardown (abort on `channel/close` / connection drop). When the
|
||||
/// session ends (exit chunk sent, stream closed, or stream reset),
|
||||
/// `drive_session_pre_negotiated` returns and the spawned task
|
||||
/// completes; the wrapper's teardown task then calls
|
||||
/// `manager.teardown_channel` and `policy.on_close` (ADR-047 §7).
|
||||
fn make_tty_open_handler(
|
||||
backends: Arc<HashMap<String, Arc<dyn TtyBackend>>>,
|
||||
identity: Option<alkcall::core::auth::Identity>,
|
||||
) -> OpenHandler {
|
||||
Arc::new(
|
||||
move |input: Value, channel_conn: Connection, auth: AuthContext| {
|
||||
move |input: Value,
|
||||
plan: Option<ChannelPlan>,
|
||||
channel_conn: Connection,
|
||||
auth: AuthContext| {
|
||||
let backends = Arc::clone(&backends);
|
||||
let identity = identity.clone().or_else(|| auth.identity.clone());
|
||||
|
||||
@@ -382,15 +430,31 @@ fn make_tty_open_handler(
|
||||
}
|
||||
};
|
||||
let (client_read, client_write) = tokio::io::split(stream);
|
||||
drive_session_pre_negotiated(
|
||||
client_write,
|
||||
client_read,
|
||||
req,
|
||||
backends,
|
||||
None,
|
||||
identity,
|
||||
)
|
||||
.await;
|
||||
let allocated = plan
|
||||
.as_ref()
|
||||
.and_then(|p| p.downcast_ref::<AllocatedHandle>())
|
||||
.and_then(|slot| slot.take());
|
||||
match allocated {
|
||||
Some(handle) => {
|
||||
crate::adapter::drive_session_pre_allocated(
|
||||
client_write,
|
||||
client_read,
|
||||
handle,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
None => {
|
||||
drive_session_pre_negotiated(
|
||||
client_write,
|
||||
client_read,
|
||||
req,
|
||||
backends,
|
||||
None,
|
||||
identity,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
})
|
||||
},
|
||||
)
|
||||
@@ -468,6 +532,7 @@ mod tests {
|
||||
"cmd": ["true"],
|
||||
"cwd": 42
|
||||
}),
|
||||
None,
|
||||
channel_conn,
|
||||
AuthContext::anonymous(b"test"),
|
||||
);
|
||||
@@ -652,6 +717,62 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// Allocation runs in the establisher (alkcall 0.6 — the
|
||||
/// `TtyHandle` crosses to the pump handler via the `Establishment`
|
||||
/// plan payload, ADR-010 as amended). Its failure maps to
|
||||
/// `DialFailed` (`details.reason == "dial_failed"` — the target
|
||||
/// refused or the backend lacked capacity), and a successful
|
||||
/// allocation lands in the plan slot the handler takes.
|
||||
#[tokio::test]
|
||||
async fn establisher_allocates_and_maps_failure_to_dial_failed() {
|
||||
struct AllocFailBackend;
|
||||
#[async_trait::async_trait]
|
||||
impl TtyBackend for AllocFailBackend {
|
||||
async fn allocate(
|
||||
&self,
|
||||
_params: &crate::backend::TtyParams,
|
||||
) -> Result<crate::backend::TtyHandle, crate::backend::TtyError> {
|
||||
Err(crate::backend::TtyError::AllocFailed {
|
||||
message: "out of ptys".to_string(),
|
||||
})
|
||||
}
|
||||
}
|
||||
let mut failing: HashMap<String, Arc<dyn TtyBackend>> = HashMap::new();
|
||||
failing.insert("mock".to_string(), Arc::new(AllocFailBackend));
|
||||
let result = establisher_result(
|
||||
Arc::new(failing),
|
||||
None,
|
||||
json!({ "carriage": "raw", "backend": "mock", "cmd": ["true"] }),
|
||||
)
|
||||
.await;
|
||||
match result {
|
||||
Err(e) => {
|
||||
assert_eq!(e.reason(), "dial_failed");
|
||||
assert!(e.message().contains("out of ptys"));
|
||||
}
|
||||
Ok(_) => panic!("allocation failure must be rejected by the establisher"),
|
||||
}
|
||||
|
||||
let result = establisher_result(
|
||||
mock_backends_one(),
|
||||
None,
|
||||
json!({ "carriage": "raw", "backend": "mock", "cmd": ["true"] }),
|
||||
)
|
||||
.await;
|
||||
let establishment = result.expect("successful allocate carries the handle");
|
||||
let slot = establishment
|
||||
.plan
|
||||
.as_ref()
|
||||
.and_then(|p| p.downcast_ref::<AllocatedHandle>())
|
||||
.expect("plan is the AllocatedHandle slot")
|
||||
.take()
|
||||
.expect("the allocated handle is in the slot");
|
||||
// The handle is live: its exit_code resolves (MockBackend
|
||||
// resolves immediately with the configured code).
|
||||
let code = slot.exit_code.await.expect("exit_code resolves");
|
||||
assert_eq!(code, 0);
|
||||
}
|
||||
|
||||
/// E-02: the spec carries a description (disclosed by
|
||||
/// `services/list`) and declares the `channel:open_failed` error
|
||||
/// schema (ADR-016 — the establishment-failure contract is
|
||||
@@ -672,11 +793,17 @@ mod tests {
|
||||
.as_array()
|
||||
.expect("reason enum");
|
||||
let reasons: Vec<&str> = enum_values.iter().filter_map(|v| v.as_str()).collect();
|
||||
// The establisher produces only these three (dial_failed and
|
||||
// resource_shortage have no TTY producer path — ADR-010).
|
||||
// The establisher produces these four (resource_shortage has
|
||||
// no TTY producer path — ADR-010; allocation failure maps to
|
||||
// dial_failed, the amended mapping).
|
||||
assert_eq!(
|
||||
reasons,
|
||||
vec!["unknown_resource", "handler_error", "timeout"]
|
||||
vec![
|
||||
"dial_failed",
|
||||
"unknown_resource",
|
||||
"handler_error",
|
||||
"timeout"
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user