fix(git): status-before-close by construction — the split-halves seam duplex (w4-git-exit-close)
The wave-3 gate's booked finding 1: the git row's exit-report order was honored by scheduling, not by construction — into_stream()'s ChannelCloseOnDrop spawned a CHANNEL_CLOSE send racing the exit_status_request send for a queue position on russh's bounded message queue (event_buffer_size, default 10); a full buffer at the serve tail could silently no-op the report. - the seam now rides a split-halves duplex built from russh 0.65's public API (its ChannelRx/ChannelTx are private): the reader is git_map's ChannelHalveReader (owned read half, the private rx's observation semantics repinned, no Drop path ever messages the wire) and the writer the write half's ChannelTx via make_writer_ext (poll_shutdown still sends EOF; the Rust 2024 capture phantom erased through a trait object) - the report's send is now the sole close-contracting sender, ordered ahead of the explicit close by sequential awaited sends on the one handle — saturated buffer or not - new wire row backlogged_seam_tail_still_reports_exit_status_before_close: a volume flood freezes the armed client loop (the new harness DataGate + the client data hook russh awaits inline) so the server's flush pends and the bounded message queue saturates at the serve tail; the gate's release drives the tail through the unwinding contention — the row pins exit-status(0) before close under the saturation and falsifies the old shape deterministically (15/15 red under a temporary into_stream() revert, 10/10 green with the fix) - doubles gain a race-safe per-repo serve gate (install_gate, the double-check pattern over list_refs); harness gains drive_with_data_gate (the drive* bodies factored into mount_and_connect, existing signatures unchanged) - all existing git wire rows unchanged and green (wire order identical when the race does not fire); spawn-and-return intact; tty rows untouched; event_buffer_size stays russh's default (pinned in the row) Verification: cargo test (default), --all-features, and six mixed per-feature builds green; clippy x2 -D warnings; fmt -check; cargo doc --no-deps 0 warnings; taskgraph validate (35 tasks).
This commit is contained in:
1 parent
dba64d65d3
commit
9cf5bfbaf3
5 files changed
+614
-15
No files matched your search
@@ -1,7 +1,7 @@
|
||||
---
|
||||
id: w4-git-exit-close
|
||||
name: Wave 4 — git row hardening (status-before-close by construction — wave-3 gate booked item 1)
|
||||
status: pending
|
||||
status: completed
|
||||
depends_on: [w3-git-mapping]
|
||||
scope: narrow
|
||||
risk: medium
|
||||
@@ -71,8 +71,114 @@ the by-construction property proven on the wire, not by reasoning.
|
||||
|
||||
## Notes
|
||||
|
||||
> To be filled by implementation agent
|
||||
### The shape decided (2026-10-29)
|
||||
|
||||
Went with the first named option — **split the channel halves in
|
||||
`serve_git_row` and pass them into the seam** — refined by one russh
|
||||
0.65 reality: `russh::channels::io::{ChannelRx, ChannelTx}` are
|
||||
*private* at this pin, so the split-halves duplex is rebuilt from the
|
||||
public API in `src/git_map.rs`:
|
||||
|
||||
- **The reader** is `git_map`'s own
|
||||
[`ChannelHalveReader`]: the owned `ChannelReadHalf`
|
||||
(`channel.split()`), a `poll_read` state machine whose observation
|
||||
semantics repin russh's private `ChannelRx` *without* the
|
||||
`ChannelCloseOnDrop` wrapper — `Data` fills (partial reads
|
||||
buffered), `ExtendedData`/other non-data messages swallowed and
|
||||
re-read (the `_`-arm parity), `Eof` ends the stream, the peer's
|
||||
`Close` leaves the reader to the channel's sender-death EOF
|
||||
(`wait()` → `None`). The `wait()` access is *future-owned*: each
|
||||
read round-trips the half through a boxed future that parks it
|
||||
inside `ChannelReadHalf::wait` and hands it back with the observed
|
||||
message — fully owned (`'static`-usable, the seam's `T` bound)
|
||||
across every park/buffer/drop state, no extra task, and **no
|
||||
`Drop` path ever messages the wire**.
|
||||
- **The writer** is the write half's `ChannelTx` via
|
||||
`make_writer_ext(None)`. Its value is `'static`-free of the half
|
||||
(clones only) but the Rust 2024 capture rules give the opaque a
|
||||
phantom of the half's borrow; a trait-object erasure
|
||||
(`Box<dyn AsyncWrite + Unpin + Send>`) drops the phantom, leaving
|
||||
the seam's `'static` bound satisfiable without parking the half.
|
||||
`poll_shutdown` still sends EOF (exactly `into_stream`'s).
|
||||
|
||||
The result: **nothing the seam drops ever messages the wire** — the
|
||||
report's `exit_status_request` send is the sole close-contracting
|
||||
sender, and its queue position ahead of the explicit close holds by
|
||||
construction (sequential awaited sends on the one `Handle`),
|
||||
saturated buffer or not. The hazard's premise (russh's
|
||||
`ChannelCloseOnDrop` `Drop` spawning a `CHANNEL_CLOSE` send that can
|
||||
win a queue position on the bounded `Sender<Msg>` —
|
||||
`event_buffer_size`, default 10 — before the report's) is *removed*,
|
||||
not raced.
|
||||
|
||||
### The wire row (the by-construction property pinned)
|
||||
|
||||
`tests/git_mapping.rs::backlogged_seam_tail_still_reports_exit_status_before_close`
|
||||
— the finding's failure scenario made real on the wire: the serve's
|
||||
answer volume (300 one-pkt-line ref answers) exceeds the entire
|
||||
outbound pipeline via the new client-direction data gate
|
||||
(`tests/common/harness.rs::DataGate` + the harness handler's `data`
|
||||
hook, which russh's client loop awaits inline): the armed gate freezes
|
||||
the client's loop → the transport duplex fills (`DUPLEX_CAPACITY`) →
|
||||
the server's flush pends → russh's bounded message queue fills → the
|
||||
serve's remaining answer writes pend (the serve tail blocked). The
|
||||
gate's release then drives the serve tail through the just-unwinding
|
||||
contention, and the row pins: the whole backlogged answer drains
|
||||
**through the same queue the report rides**, then exit-status(0), then
|
||||
close — order asserted, exit code asserted.
|
||||
|
||||
**The row falsifies the old shape.** With a temporary one-line revert
|
||||
of `serve_git_row` to `channel.into_stream()` the row FAILED
|
||||
**15/15** runs — the implicit close won the frozen-drain queue race
|
||||
("expected exit-status(0), got Some(Close)"), the silent report loss
|
||||
the gate booked. With the split-halves shape: **10/10 green**. So the
|
||||
row is a two-sided pin: deterministic red under the hazard, green by
|
||||
construction without it.
|
||||
|
||||
Test-vehicle additions (test surface, freely versioned): the doubles'
|
||||
per-repo serve gate (`tests/common/git_doubles.rs`: `install_gate` —
|
||||
race-safe double-check `MemGate`, gating `GitRefs::list_refs`, so the
|
||||
row schedules the serve's stages against prepared wire state) and the
|
||||
harness's `DataGate` + `drive_with_data_gate` (the `drive*` bodies
|
||||
factored into the shared `mount_and_connect` core; existing wrappers'
|
||||
signatures unchanged).
|
||||
|
||||
### Constraints check
|
||||
|
||||
- **Every existing git wire row green** — the full git suite
|
||||
(`tests/git_mapping.rs`, 12 rows) and every other loopback suite:
|
||||
no behavior change on the happy path (the wire order the rows pin —
|
||||
data, exit-status, close — is identical under both shapes when the
|
||||
race doesn't fire).
|
||||
- **Spawn-and-return intact** (ADR-006 decision 4): the reporting
|
||||
still rides the spawned serving task through the cloned
|
||||
`Session::handle()`; `serve.rs`'s mechanics untouched.
|
||||
- **The tty rows untouched** (not in this class — the gate verified).
|
||||
- **No API/limits change**: `event_buffer_size` stays russh's default
|
||||
posture (the adapter docs' recorded default posture preserved); the
|
||||
row pins the default's value with a note that a russh default
|
||||
change re-reads the row's freeze-settle yields, not its assertions.
|
||||
|
||||
## Summary
|
||||
|
||||
> To be filled on completion
|
||||
**Completed.** The git row's status-before-close now holds **by
|
||||
construction**: `serve_git_row` no longer hands russh's
|
||||
close-on-drop `ChannelStream` (`into_stream()`) to the seam — the
|
||||
seam rides a split-halves duplex (`git_channel_duplex` →
|
||||
`ChannelHalveReader` over the owned read half + the write half's
|
||||
`ChannelTx` via the capture-erased trait object) where **no drop
|
||||
path ever messages the wire**, making the report's send the sole
|
||||
close-contracting sender and the report-before-close order
|
||||
unconditional (russh 0.65's private `ChannelRx`/`ChannelTx` noted;
|
||||
the semantics repinned from the public API). New wire row pins the
|
||||
by-construction property on a genuinely saturated tail (volume flood
|
||||
→ frozen client loop → duplex → bounded message queue; gate release
|
||||
→ the tail through the unwinding contention) — and falsifies the old
|
||||
shape deterministically (15/15 red under a temporary
|
||||
`into_stream()` revert, 10/10 green with the fix). Doubles gained a
|
||||
race-safe per-repo serve gate; the harness gained the client-side
|
||||
data gate + `drive_with_data_gate` (the `drive*` cores factored,
|
||||
signatures unchanged). Verification: `cargo test` (default),
|
||||
`--all-features`, and 6 mixed per-feature builds all green; clippy ×2
|
||||
`-D warnings` clean; fmt clean; `cargo doc --no-deps` warning-free;
|
||||
`taskgraph validate` clean (35 tasks).
|
||||
Reference in new issue
Block a user