feat(protocol): make correlation an exclusive identity #74

Merged

natsukium merged 15 commits from refactor/exclusive-correlation-48 into main 2026-09-04 17:46:19 +09:00

Conversation 0 Commits 15 Files changed 24 +2241 -562

natsukium commented 2026-09-04 17:07:53 +09:00

Owner

Copy link

Fixes #48.

Correlation carried two independent scalar fields, so the wire could encode
an envelope naming both a request and a stream, and every handler recovered its
mandatory id ad hoc. This makes correlation an exclusive identity and moves the
per-arm rule into the shared layer.

• felis-protocol: Correlation becomes a protobuf oneof; the domain type is
  Correlation::Request(RequestId) | Correlation::Stream(StreamId). Neither id,
  both ids, and unknown arms are malformed at decode. Prost output regenerated.
• Each message arm declares its correlation class — uncorrelated, request opener,
  request reply, stream opener, or stream item — and the arm table is applied to
  every frame, Push included.
• felis-transport: ConnectionDriver::decode validates the declaration before
  returning a deliverable message, so correlation is read off the delivery
  instead of being re-derived per family.
• felis-daemon / felis-cli / felis-client-core: handlers read the id off
  the driver's delivery; the CLI sends a request under the lock that issued its
  id, and a request-scoped refusal is admitted through the driver.

The break is acknowledged in crates/felis-protocol/proto/BREAKING.md (pre-release,
rides the current major), so proto-compat passes.

Doc cascade: docs/reference/spec.md sources the exclusive envelope and the arm
table; docs/reference/ipc.md and docs/explanation/architecture/ipc.md split
what a mode admits from what the daemon sends and argue the rejected alternatives;
docs/reference/glossary.md and docs/reference/testing.md follow the renamed
terms; CHANGELOG.md records the wire change.

Verified: just check green (fmt, clippy, nextest, deny); reviewed by plan/correctness/docs lenses and pi sol

Fixes #48. `Correlation` carried two independent scalar fields, so the wire could encode an envelope naming both a request and a stream, and every handler recovered its mandatory id ad hoc. This makes correlation an exclusive identity and moves the per-arm rule into the shared layer. - `felis-protocol`: `Correlation` becomes a protobuf `oneof`; the domain type is `Correlation::Request(RequestId) | Correlation::Stream(StreamId)`. Neither id, both ids, and unknown arms are malformed at decode. Prost output regenerated. - Each message arm declares its correlation class — uncorrelated, request opener, request reply, stream opener, or stream item — and the arm table is applied to every frame, `Push` included. - `felis-transport`: `ConnectionDriver::decode` validates the declaration before returning a deliverable message, so correlation is read off the delivery instead of being re-derived per family. - `felis-daemon` / `felis-cli` / `felis-client-core`: handlers read the id off the driver's delivery; the CLI sends a request under the lock that issued its id, and a request-scoped refusal is admitted through the driver. The break is acknowledged in `crates/felis-protocol/proto/BREAKING.md` (pre-release, rides the current major), so `proto-compat` passes. Doc cascade: `docs/reference/spec.md` sources the exclusive envelope and the arm table; `docs/reference/ipc.md` and `docs/explanation/architecture/ipc.md` split what a mode admits from what the daemon sends and argue the rejected alternatives; `docs/reference/glossary.md` and `docs/reference/testing.md` follow the renamed terms; `CHANGELOG.md` records the wire change. Verified: just check green (fmt, clippy, nextest, deny); reviewed by plan/correctness/docs lenses and pi sol

natsukium added 15 commits 2026-09-04 17:07:53 +09:00

feat(protocol): make correlation an exclusive identity ... c0d3fc4d91

`Correlation` was two independent optional scalars, so the schema could
encode states the request/stream model has no reading for: neither id
set was rejected at the conversion, both set was accepted, and what
both meant was decided by whichever call site looked at which field
first. That is an optional-field bag rather than an identity, and a
non-Rust peer would have had to reproduce felis's call-site order to
interoperate.

The envelope becomes a `oneof` and the domain type an exclusive enum.
Tags 1 and 2 stay, so every honest frame is byte-identical — a oneof of
scalars encodes exactly as the bare scalar did — and the two
independent sequences and the ABA argument stay as they were. `buf
breaking` still calls the move a break (FIELD_SAME_ONEOF), acknowledged
in BREAKING.md for the pre-release major; the local pre-commit hook
cannot read that file, so this commit is the intended --no-verify.

prost resolves a oneof last-field-wins, so a body setting both tags
would decode as a stream envelope rather than being refused. The peek
therefore reads field 100 as two plain scalars — a private mirror, not
the generated type — and refuses a body that set both, which is what
makes "the old both-ids encoding is malformed" true of peers as well as
of this build.

Refs #48

Assisted-by: Claude Code

refactor(transport): validate the envelope against the arm's class ... e72891c328

`account_for` looked only at `stream_id`: a request id on an arm that
should carry none passed silently, `match_reply` accepted a reply that
also named a live stream, and every mandatory request id was recovered
after the fact by a handler. So "this arm requires a request id" was
stated once per call site and enforced nowhere a peer could read.

The arm's `correlation` class is now the whole rule, checked in
`decode`, and a delivered message carries the identity that check
validated — so a handler reads `delivered.request()` instead of
re-deriving one that cannot be `None` for its class. The class check
runs after the direction column and before the mode and phase ones: a
mode-denied arm is refused through its own envelope and the connection
lives on, so its id has to be accounted for or the next request would
look like a skip.

The daemon now holds the request sequence the way it already held the
stream one, which is what catches a skipped or reused id where it was
sent. It keeps no set of unanswered requests: the sequence is strictly
increasing, so an id is either the next one or already malformed, and
such a set would have to be retired on every reply-writing path or grow
for the connection's life.

Refs #48

Assisted-by: Claude Code

refactor: read correlation off the driver's delivery ... b8380af8a3

`ops_request_of`, `stream_of` and the inline `Region::Request` branch
each re-derived an id the driver had already seen, and the client
called `match_reply` beside `decode` — two statements of one rule, in
the two places most likely to drift from it. The driver now validates
the envelope against the arm's class, so every one of them is either
deleted or reads the identity back off the delivery.

Two lookups survive, and only because they need an id where no
delivery is in hand: the `Notify` subscriber writes its ack while the
payload is still borrowed, and an `Ops` arm the decode denied is
refused through an envelope the decode never returned.

Refs #48

Assisted-by: Claude Code

docs: record the exclusive correlation identity ... b9f2a5f84d

The reference stated the envelope as two scalars and left "which id
does this arm carry" to prose per family, which is the shape the
schema no longer has. It now spells the oneof, tabulates the envelope
each correlation class demands, and says the daemon holds the request
sequence as well as the stream one.

The explanation gains the decision itself: why two optional scalars
were an optional-field bag a non-Rust peer had to reproduce by
call-site behavior, why a future arm needing both concepts earns a
named field rather than a widened envelope, why the peek keeps two
plain scalars (prost's last-field-wins), and why the daemon tracks no
unanswered-request set.

Refs #48

Assisted-by: Claude Code

fix(cli): send a request under the lock that issued its id ... 2e5ed1a54b

A request id is as positional on the wire as a stream id: the daemon
accepts a verb only when its `request_id` is the next unissued one and
ends the connection otherwise. The bridge runs every op on its own
task against one shared link, so two verbs allocate from the same
driver concurrently; allocating outside the writer lock left the send
order free to disagree with the allocation order, which would fail
every operation on that link, not just the one that overtook.

`open_stream` already takes the writer lock first for exactly this
reason. `request` now does the same, and checks the link for loss
before the allocation rather than after, so no id is burned on a frame
that is never sent.

Refs #48

Assisted-by: Claude Code

fix(daemon): read the subscribe stream id off its delivery ... 225eac6748

The `Notify` subscriber was the last handler recovering a mandatory
correlation id from the raw payload, even though the driver had
already validated that id against the arm's class and handed it back
on the `Delivered` value the match discarded. Binding it instead
removes the second, unenforced statement of "a `Subscribe` carries a
stream id" and retires `stream_of` with it; `request_of` stays,
because refusing an `Ops` arm whose decode was denied returns before
any delivery exists.

Refs #48

Assisted-by: Claude Code

fix(transport): apply the arm table to every frame, Push included ... 558148237b

Three gaps between the table and what the driver enforced.

The envelope was peeked only where `MessageKind::is_correlated` said a
wrapper declares the slot, so the `uncorrelated` class went unchecked
on `Input`, `Grid`, `Image`, `Push` and `Conn`: a hand-built body can
set field 100 whatever the schema reserves it for, prost skipped it,
and the frame was delivered. An envelope no arm claims is
unattributable correlation, which ends the connection (REQ-114). The
peek now runs on every body and its result goes through the class
check, `Conn` included. A body that is not protobuf at all is still
reported as its family failing to decode rather than as a correlation
fault, so a corrupt frame does not send its reader hunting an id bug.

The kind-level mode and phase columns were folded into one test
guarded by a known mode. A frame refused purely on phase was reported
as a mode denial, and the phase filter never ran during the handshake —
the one phase it exists for, since the daemon learns its mode from the
`Hello` it is filtering. They are refused separately now, and the
handshake's hand-written "only `Conn` here" case is the table's fold
instead of a second policy beside it, which is what leaves
`MessageKind::admissible` with no caller.

`Push::Reattach`, `SessionExited` and `RetargetHost` go back to the
attach-capable modes. Narrowing them to `window` is a receive-side
wire change — an `Ops` attach that is sent one would end its
connection — and that is a decision to take on its own evidence, not a
side effect of writing the routing table down. Which subscribers the
daemon *sends* them to is unchanged and still window-only.

Refs #48

Assisted-by: Claude Code

docs: source the exclusive envelope and the arm table in spec.md ... caa6f5f37e

REQ-113 still described correlation as two independent ids and said
nothing about the exclusive `oneof`, the three malformed encodings, or
the daemon holding the request sequence, and the arm-routing table had
no requirement at all despite `reference/ipc.md` carrying a normative
section and `reference/testing.md` a gate for it. A requirement doc
that trails its reference and explanation twins is the one place a
reader cannot check what the wire promises.

Refs #48

Assisted-by: Claude Code

docs: separate what a mode admits from what the daemon sends ... 16288f8f78

The mode table read as one projection of the arm table's `modes`
column, but half its rows say which subscribers the daemon writes to,
which is narrower than what the mode admits on receipt and may narrow
further without a wire break. Split into an `admits` table and an
`is sent` one, the `Push` rows stop contradicting the arm table now
that every push arm names both attach-capable modes.

The changelog gains the receive-side half of the arm table: enforcing
each arm's row in the shared driver leaves the admitted set unchanged
but moves the refusal, so a third-party client that receives an arm
its mode does not admit ends the connection instead of ignoring the
frame.

The explanation's rejected alternative is stated in the present tense
rather than as a change narrative, and the driver test count leaves
`reference/testing.md`: a pinned total goes stale on every test added
and says nothing the file itself does not.

Refs #48

Assisted-by: Claude Code

chore(protocol): regenerate the prost output for the Push arms ... ec11d1f3d5

The three attach-capable Push arms got their `modes=window,ops`
declaration in the schema when the arm table started admitting them on
an Ops connection, but the committed generated file was never
regenerated, so its `// arm:` comments still read `modes=window`. The
arm-table test reads `felis.proto` directly and never the generated
file, which leaves only the buf-generate pre-commit hook to catch the
drift; a reader of the generated crate would meanwhile see a table that
contradicts the one the driver enforces.

Refs #48

fix(protocol): refuse a both-ids envelope on tag presence ... f5e8d54472

The peek mirrored field 100 as two plain scalars, so it decided
"both ids" on the values that survived the parse. prost resolves those
scalars last-field-wins like the oneof it stands in for, so a body
writing request_id=7, stream_id=1, request_id=0 left the mirror holding
a lone stream id: the ambiguity check never fired, and the frame was
admitted as a stream envelope. A canonical protobuf reader resolves the
same bytes to request_id=0, which is malformed, so felis accepted a
frame no other implementation of this schema can read that way, which
is precisely what the exclusive envelope exists to prevent.

The peek now reads the envelope submessage a tag at a time and refuses
a body in which both tags appear at all, whatever value each last held,
and refuses an id tag encoded as anything but a varint. Repeated
occurrences of field 100 are concatenated first, protobuf's own merge
rule, so the two ids cannot be split across two occurrences either.

Refs #48

fix(client-core): admit a request-scoped refusal through the driver ... 8121ecd77a

`is_reply_to` decoded a `Conn` frame itself and the caller then retired
the request id with a bare `match_reply`, so a request-scoped
`Conn::Error` never reached `ConnectionDriver::classify`. That path
skips the envelope, direction and arm columns: every `Conn` arm is
uncorrelated, so a refusal that also carried a field-100 envelope is
unattributable correlation that ends the connection, yet the connector
took it as a legitimate typed refusal of the outstanding request and
kept reading. Draining a family is not an exemption from the arm table,
and neither is interpreting one frame ahead of the driver.

The peek now only reports that the frame is a request-scoped refusal;
the driver decides which id it names and whether the frame is
admissible, and its own `match_reply` retires the id exactly once.

Refs #48

docs: complete the admits table and argue the rejected alternatives ... 17a49348e0

The mode table now claims to be the arm table's `modes` column
projected onto the three modes, but it listed eight of the ten frame
kinds: `Grid` and `Image` were absent though every arm of both declares
the attach-capable modes, so a third-party `Ops` client built from the
page would conclude those frames are illegal on its connection and end
it on the rehydrate burst the daemon really sends. The projection gains
both families, and the `is sent` table gains the row for what an `Ops`
attach receives.

The prose fixes follow the doc-prose norms: the reference page drops a
sentence that argued the arm table's purpose rather than stating a fact
(the argument already lives in the explanation twin), the explanation's
rejected alternatives argue the alternative instead of narrating what
the daemon used to do, and the em dashes this page carried none of
before are gone.

Refs #48

refactor(transport): reconcile the arm table with the landed schema ... f95b75bbae

The first three commits of this series reached main in revised form,
and their revision differs from what the rest of the series was written
against: the driver grew `admit_discarded` / `admit_parked` for the
frames a reader drops or holds back, and the window-management pushes
(`Push::Reattach`, `SessionExited`, `RetargetHost`) narrowed to
`ModeSet::WINDOW`. Both are kept, expressed against the exclusive
envelope this series introduces.

`admit_parked` survives because the guard is real: a `Push::Reattach`
requeued unread on an `Ops` connection would let a verb report success
over a routing violation (REQ-114). It cannot use `admit_drained`,
which accounts, since the parked frame is read again — so the class
check splits out of `account_for` as the stateless half, and the
envelope peek splits out of `classify` so a raw frame can reach it.
`admit_discarded` does not survive: draining accounts in this series,
and a stream item naming a stream the client never opened is a
violation only the stream table can see.

The narrowed Push arms make the family's arm table two rows wide, so it
leaves `arms_are_uniform` and pays the full decode on the drain path;
`Grid` and `Image`, which carry the traffic that made the fast path
worth having, are unaffected.

docs: say the asides straight instead of in em-dashes ... 3ff8517e19

The prose norm this repo checks mechanically rejects the em-dash aside:
a dash that stands for "that is" or "because" hides which one it meant,
and a paired dash buries a clause the reader has to hold open. Each
site here states the relation it was standing in for, or drops the
aside where it added nothing.

Refs #48

natsukium force-pushed refactor/exclusive-correlation-48 from 3ff8517e19 to 90a3cab4ca 2026-09-04 17:24:34 +09:00 Compare

natsukium merged commit 90a3cab4ca into main 2026-09-04 17:46:19 +09:00

natsukium deleted branch refactor/exclusive-correlation-48 2026-09-04 17:46:19 +09:00

natsukium referenced this pull request 2026-09-07 16:09:12 +09:00

[v0.1/Proto Review] Freeze protobuf and IPC contracts (7 issues) #52

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

priority/P0

Must be resolved before the v0.1.0 tag

  

priority/P1

Must be settled before the first public contract freezes

  

priority/P2

Explicitly deferred until after v0.1.0

  

release/v0.1.0

Work tracked for the first tagged release

  

status/blocked

Cannot start now: waits on an upstream release, another issue, or a recorded revisit trigger

  

status/planned

Triaged with a source-grounded plan and a passed pi review; ready to implement

  

type/bug

Shipped behavior that contradicts the docs or the code's own intent; needs a product code change

  

type/design

An open scope or trade-off question; needs a recorded decision in the owning explanation doc before code

  

type/test-gap

Documented or reviewed behavior that no test pins; usually split from a review finding, needs no product code change

  

type/tracker

Parent issue that tracks child work

No labels

priority/P0

priority/P1

priority/P2

release/v0.1.0

status/blocked

status/planned

type/bug

type/design

type/test-gap

type/tracker

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

natsukium/felis!74

No description provided.