glm-5.3-flash
e2fa32b3c7
Adopt alkcall 0.5.0's channel-open establishment phase (ADR-049 — review 006 E-01 + N-1) and migrate the channels-path semantic failures per its §5 sequencing (alktty ADR-010). - `register_openable` registers `channels/tty/sub` with an establisher (`register_openable_with_establisher`): full `NegotiateRequest` parse of schema-valid `input`, `carriage == "raw"`, non-empty `cmd`, backend lookup, and the ADR-050 ownership check run before the open reply; rejections are `channel:open_failed` with `details.reason` (`unknown_resource` / `handler_error` / `timeout`) — no phantom channel (the SSH contract holds consumer-visibly) - `backend.allocate` deliberately stays in the pump handler: `Establishment` is payloadless so the `TtyHandle` cannot cross the establisher→handler boundary, and re-allocating would violate ADR-005's kill-on-Drop contract — `allocate_failed` remains the one in-band failure class on the channels path (pinned by test) - `TtySessionError::ChannelsOpen` carries alkcall's typed `ChannelOpenError` (`#[from]`) instead of a flattened `String` — the N-1 fix at alktty's layer (breaking) - channels-path semantic failures change shape from `NegotiationRejected` in-band frames to `channel:open_failed` call errors (breaking); the direct-ALPN path is unchanged - `tty_open_spec()` gains a `description` (review 006 E-02) and an ErrorDefinition for `channel:open_failed` (ADR-016 — disclosed via services/schema) - alkcall = "0.5.0"; version 0.2.0; ADR-010 + ADR-009 amendment + tty-adapter.md + CHANGELOG Verification: cargo test (112 lib + integration), cargo test --all-features (136), clippy --all-targets -D warnings (host + wasm), fmt --check, cargo doc --no-deps clean; wasm32-unknown-unknown check confirms the default crate stays wasm-clean.
218 lines
11 KiB
Markdown
218 lines
11 KiB
Markdown
# ADR-009: The Channels Open Op's `input` Is the Negotiation
|
|
|
|
## Status
|
|
|
|
Accepted (2026-09-05). Resolves review #001 L1. Prerequisites: alkcall
|
|
0.4.0 (call-time `input_schema` enforcement) and alkcall 0.4.1
|
|
(early-arrival parking for un-adopted channels).
|
|
|
|
Amended 2026-09-05 (review #002 R4): a `NegotiateRequest` parse failure
|
|
of the schema-validated `input` is now a client-visible
|
|
`malformed_negotiation` error frame, not a silent teardown — see
|
|
§"Parse-failure error frame (R4 amendment, 2026-09-05)".
|
|
|
|
Amended 2026-09-06 (ADR-010): with alkcall 0.5.0 (ADR-049 — review 006
|
|
E-01), the semantic-failure classes the R4 amendment reported in-band
|
|
(malformed negotiation, unknown backend, ownership denial) move into
|
|
the producer's establisher and resolve as `channel:open_failed` call
|
|
errors — no phantom channel. The in-band error-frame path on this
|
|
channel shrinks to one class: `allocate_failed` (the establisher
|
|
cannot carry the allocated handle across — see ADR-010 §2). The R4
|
|
tests that asserted the in-band frames for those classes now assert
|
|
the call-error shape.
|
|
|
|
## Context
|
|
|
|
Before this ADR, the channels path carried the negotiation twice. The
|
|
consumer opened a channel via the `channels/tty/sub` open op, passing
|
|
the `NegotiateRequest` params as the op's `input`; the producer-side
|
|
open handler then ignored that `input` (`let _ = input;`) and read a
|
|
second, full negotiation frame from the channel's data stream — the
|
|
same frame the direct-ALPN path reads. The registry validated `input`
|
|
against the op's schema, but the backend selection actually came from
|
|
the unvalidated wire frame. Two independent gates (the registry's
|
|
schema check on `input` and `drive_session`'s wire-frame parse) could
|
|
disagree: a client could pass schema-valid `input` and write a
|
|
divergent frame.
|
|
|
|
Review #001 (L1) flagged this as a latent inconsistency and asked for
|
|
a publisher decision: either drop the `input` parameter or pass it
|
|
through as the authoritative negotiation. Investigation of alkcall
|
|
found the review's premise wrong in an important way — alkcall (≤0.3.1)
|
|
never enforced `input_schema` on any dispatch path at all: the schema
|
|
was advertise-only (metadata in discovery). The registry gate this
|
|
design leaned on did not exist.
|
|
|
|
Two upstream changes landed to make the design sound:
|
|
|
|
1. **alkcall 0.4.0** — `OperationSpec.input_schema` is enforced at
|
|
call time by all three registry dispatch entry points (`invoke`,
|
|
`invoke_streaming`, `invoke_sink`). The schema is compiled once at
|
|
registration (fail-closed, the same rule as `publish_schema` /
|
|
CF-003). Violations return `INVALID_INPUT` before the handler runs.
|
|
2. **alkcall 0.4.1** — the demux parks early-arrival chunks for a
|
|
not-yet-adopted channel (bounded FIFO, 64 payloads per channel) and
|
|
`adopt_channel` drains them into the new receiver. Before this, the
|
|
open-op-response / producer's-first-write race silently dropped the
|
|
first chunks of any push-first producer. This bug was found by the
|
|
L3 end-to-end test (an immediately-resolving backend's stdout
|
|
sentinel + exit chunk raced the consumer's `adopt_channel`; the
|
|
session never resolved).
|
|
|
|
## Decision
|
|
|
|
The open op's registry-validated `input` **is** the negotiation
|
|
(design 2 of the L1 decision). The channels path carries **no second
|
|
negotiation frame** on the channel's data stream — the stream starts
|
|
directly in raw-chunk mode (TTY's 5-byte format, ADR-001/ADR-008).
|
|
|
|
### Producer side
|
|
|
|
`make_tty_open_handler` parses the open op's `input` into a
|
|
`NegotiateRequest` and drives the new `drive_session_pre_negotiated`
|
|
(`src/adapter.rs`) — the same three-pump session driver as the direct
|
|
path, minus the wire-frame negotiation phase. Validation is factored
|
|
into `validate_and_allocate`, shared by both paths: `carriage ==
|
|
"raw"`, non-empty `cmd`, backend lookup, the ADR-050 ownership check,
|
|
and `backend.allocate`. On the channels path the `tty:open` scope gate
|
|
is NOT re-checked in the handler — the registry's `AccessControl`
|
|
enforced it before the handler ran (the identity the registry checked,
|
|
resolved from the connection, is the authoritative one; the
|
|
handler-side identity is only the ownership-check subject).
|
|
|
|
Post-open failures (unknown backend, `allocate_failed`, ownership
|
|
denial) still go to the client as a `0x00`-prefixed negotiation error
|
|
frame on the channel stream — the same framing the direct path uses —
|
|
so the consumer's ADR-001 §5 disambiguation read applies unchanged.
|
|
|
|
A `NegotiateRequest` parse failure of the schema-validated `input`
|
|
(the schema is deliberately partial — `carriage`/`backend`/`cmd`
|
|
required, `tty`/`cwd`/`env`/backend-params free-form so the opaque
|
|
ADR-053 params pass through) was originally a handler-side failure
|
|
logged and returned without an error frame. The R4 amendment below
|
|
makes it a client-visible error frame like every other post-open
|
|
failure.
|
|
|
|
### Consumer side
|
|
|
|
`TtySession::open_via_channels` (`src/session.rs`) parses `params` as
|
|
a `NegotiateRequest` locally first (fail-fast before a channel is
|
|
allocated), opens the channel via `ChannelClient::open_channel`,
|
|
builds the channel `Connection`, and starts raw-chunk mode directly
|
|
(`from_halves_raw` — no negotiation frame is written). The
|
|
`0x00`-error-frame peek is retained: the first byte of the producer's
|
|
response disambiguates a post-open negotiation error frame (length
|
|
prefix starting `0x00`) from a raw chunk (`stream_type` in `{1, 2, 4}`).
|
|
|
|
### Scope of the change
|
|
|
|
- `tty_open_spec()`'s input schema becomes the partial
|
|
`NegotiateRequest` shape (required: `carriage`, `backend`, `cmd`).
|
|
- `drive_session_pre_negotiated` is public API (a new producer entry
|
|
point alongside `drive_session`); `validate_and_allocate` is
|
|
private, shared by both drivers.
|
|
- The direct-ALPN path is unchanged: it still reads the wire-frame
|
|
negotiation (ADR-001) and enforces the scope gate itself.
|
|
- The 5-byte chunk format, the error-frame layout, and the
|
|
`NegotiateRequest` JSON shape are unchanged (ADR-001, ADR-006).
|
|
This ADR removes a frame from the channels data stream; it does not
|
|
change any frame that remains.
|
|
|
|
### Parse-failure error frame (R4 amendment, 2026-09-05)
|
|
|
|
All three post-open failure classes on the channels path are now
|
|
client-visible through the same `0x00` error-frame peek
|
|
(`from_halves_raw`):
|
|
|
|
1. **Semantic validation failures** (`carriage != "raw"`, empty `cmd`,
|
|
unknown backend, `allocate_failed`, ownership denial) — error
|
|
frames from `validate_and_allocate` (unchanged).
|
|
2. **`NegotiateRequest` parse failure of schema-valid `input`** — the
|
|
open handler accepts the channel's `BiStream` and writes a
|
|
`malformed_negotiation` frame
|
|
(`{"error":"malformed_negotiation","message":"..."}`) via the shared
|
|
`crate::adapter::send_negotiation_error`, then returns. Reachable
|
|
despite the registry's schema check because the schema is
|
|
deliberately partial: e.g. `cwd` typed as a number passes the schema
|
|
(unknown-key/type fields pass through for the opaque ADR-053
|
|
params) but fails the typed parse.
|
|
3. **Schema-invalid `input`** — rejected at dispatch by the registry
|
|
(alkcall 0.4) before any handler runs: a `CallError` on the open
|
|
op, no channel allocated, no error frame (unchanged).
|
|
|
|
This replaces the original behavior (log + return, channel teardown,
|
|
consumer observes `NoExitChunk` — indistinguishable from a crashed
|
|
producer). The error-frame layout is unchanged (ADR-001's wire-stable
|
|
contract); no new frame type, no new stream type. Tests: the
|
|
`make_tty_open_handler` seam test (hand-built `input` bypassing the
|
|
schema, `channels.rs::tests`) and the real-registry end-to-end test
|
|
(`testing.rs`); both assert the consumer-side `0x00` peek observes
|
|
`malformed_negotiation`.
|
|
|
|
## Consequences
|
|
|
|
**Positive:**
|
|
|
|
- **One negotiation per session.** The registry's schema check and the
|
|
semantic validation run on the same value — the two-gate
|
|
disagreement is structurally impossible.
|
|
- **Fewer bytes and one less round trip** on the channels path: no
|
|
4-byte length prefix + JSON frame on the data stream, and the
|
|
consumer needs no negotiation write before its first read.
|
|
- **The consumer's `open_via_channels` fails fast** on malformed
|
|
params before a channel is allocated.
|
|
- **`open_via_channels` and `from_bidi_stream_via` are covered
|
|
end-to-end** against the real producer path (review #001 L3): 5
|
|
session tests + a pre-negotiated adapter test + the shared
|
|
`crate::testing` harness (moved out of `channels.rs::tests` so the
|
|
session tests share it).
|
|
|
|
**Negative:**
|
|
|
|
- **A second producer entry point.** `drive_session` (direct) and
|
|
`drive_session_pre_negotiated` (channels) must stay in sync on
|
|
validation semantics; the shared `validate_and_allocate` keeps the
|
|
validated logic in one place, but the scope-gate policy differs by
|
|
path (`enforce_scope` flag) and is documented at both call sites.
|
|
- **The schema is deliberately partial.** Raw JSON Schema is
|
|
permissive on unknown keys, so the opaque backend params pass
|
|
through unchecked by the registry; semantic validation stays in
|
|
`validate_and_allocate` and the backend's `allocate`. Duplicating
|
|
full `NegotiateRequest` schemas in the spec would create a second
|
|
definition to drift against `negotiation.rs`.
|
|
- **Consumer/producer version skew.** A consumer that writes the old
|
|
second negotiation frame to a producer built on this ADR will have
|
|
that frame parsed as a raw chunk (stream_type byte = first JSON
|
|
byte, garbage length) — the session fails confusingly. Acceptable
|
|
pre-1.0 (no released channels-path consumers); the direct path is
|
|
unaffected.
|
|
|
|
## Door type
|
|
|
|
**Two-way for now, one-way once a released consumer exists.** The
|
|
channels-path negotiation location (open-op `input` vs. a wire frame)
|
|
is reversible pre-1.0 — the `NegotiateRequest` shape, the 5-byte chunk
|
|
format, and the error-frame layout (the wire-stable contracts) are
|
|
untouched. Once a released consumer depends on "open op, then raw
|
|
chunks," removing or re-adding the wire-frame negotiation on this path
|
|
becomes a peer-breaking change. The `drive_session_pre_negotiated`
|
|
public API is additive; `drive_session` is unchanged.
|
|
|
|
## References
|
|
|
|
- [ADR-001](001-wire-format-and-two-carriage.md) — the two-carriage
|
|
model (JSON negotiation + raw chunks) and the §5 framing
|
|
disambiguation this ADR reuses for the post-open error frame
|
|
- [ADR-002](002-ttybackend-trait-and-ttyhandle.md) — the backend
|
|
selection and allocation this ADR's validation runs
|
|
- [ADR-006](006-negotiation-framing-self-contained.md) — the
|
|
negotiation frame layout (unchanged; the direct path still uses it)
|
|
- [ADR-008](008-channels-pure-channel-multiplexing.md) — the channels
|
|
layer strips its 8-byte header transparently; TTY owns its framing
|
|
(this ADR decides what TTY's framing on the channels path contains)
|
|
- `docs/reviews/001-code-review.md` §"Resolution (2026-09-05, L1 + L3)"
|
|
- `src/adapter.rs` — `drive_session`, `drive_session_pre_negotiated`,
|
|
`validate_and_allocate`
|
|
- `src/channels.rs` — `make_tty_open_handler`, `tty_open_spec`
|
|
- `src/session.rs` — `open_via_channels`, `from_halves_raw`
|
|
- `src/testing.rs` — the shared producer/consumer channels harness |