docs: reconcile the units RFC with what #204 actually shipped - #212
Merged
Conversation
The primitives section proposed wire that has since landed, and in two
places landed differently. Left as written it now contradicts the
protocol.
- `S2C_LEASE` moves to `0x11`. `0x10` was free when this was written;
#204 shipped `S2C_CREATE_FAILED` there.
- `S2C_CREATE_FAILED` is `[0x10][nonce:2][status:1][detail:N]`, not
`[0x11][nonce:2][reason:1]` — the common status registry rather than
a message-local reason byte, matching what #167's protocol.md had
already allocated. It is also opt-in per request via
`CREATE2_WANT_STATUS`, so a legacy client cannot read a refusal as
PTY zero.
- `max_ptys` kept its `0` default rather than gaining a real one.
#188 landed the env var in the meantime and argued unlimited is
right, and that argument holds: a client that can open a terminal
can already spend the machine from inside it.
- `FEATURE_UNITS` moves to bit 17. 11-13 are reserved for the
extension, channel, and process families; 14-16 shipped with #204.
- The `C2S_KILL` flags arm is `data.len() >= 8`, not `>= 7`. Seven is
the existing message length, so arming there reads a byte that is
not there.
Delivery marks 1-3 shipped and narrows 2 to what is actually left: the
lease family, and the timed `C2S_CLOSE` escalation, which needs `CLOSE`
to hold a "closing" state and tangles with the retention path.
|
Coverage
|
Formatting only, no behaviour change. These fail `prettier --check` on this branch and pass on `main`, which has reformatted them since; lint is red here purely because of that drift, so the RFC change this branch carries cannot go green on its own. They will be replaced wholesale when #94 rebases on main — this is to unblock CI in the meantime, not a claim about the right content.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#204 landed the RFC's primitives (delivery items 1-3, tracked as #181). Two of them landed differently from what the RFC proposed, and the RFC now contradicts the shipped protocol in ways that would mislead whoever implements the unit layer on top.
S2C_LEASE [0x10]0x110x10was free when written; #204 putS2C_CREATE_FAILEDthereS2C_CREATE_FAILED [0x11][nonce:2][reason:1][0x10][nonce:2][status:1][detail:N]protocol.mdmax_ptysgets a real default0FEATURE_UNITSbit 11C2S_KILLflags arm atlen >= 7>= 8Also worth knowing for the layer above: the refusal is opt-in per request. A client sets
CREATE2_WANT_STATUS(bit 3) after seeingFEATURE_CREATE_STATUS(HELLO bit 14), soCREATE,CREATE_AT,CREATE_Nand unflaggedCREATE2keep their success-only contract and a legacy client can't read a refusal as PTY zero.Delivery now marks 1-3 shipped and narrows item 2 to what's actually left: the lease family, and the timed
C2S_CLOSEescalation. That second one needsCLOSEto hold the entry in a "closing" state, which tangles with the retention path #204 added — it was deliberately out of scope for #181 and is still open.No changes to the unit layer itself; how the primitives landed doesn't affect it.