docs: reference claims behavior the code does not implement (attach prefix validation, Kitty placement X/Y) #349

Closed

opened 2026-09-22 20:16:52 +09:00 by natsukium · 1 comment

natsukium commented

2026-09-22 20:16:52 +09:00

Owner

Context

Pre-release review of the frozen surfaces (round 2). Two normative statements in docs/reference/ describe behavior the code does not have. Each needs a decision: fix the reference, or implement the claim before the freeze. A third and fourth case are mechanical wording errors and go in a separate doc commit.

1. `SessionAttach.id_prefix` validation

docs/reference/ipc.md (the Attach bullet under "Session (kind = 4)") says a target whose id_prefix "is not 1-32 hex digits" is "refused at decode", and that an attach, "unlike the Ops verbs' id_prefix, cannot answer a malformed prefix with NoMatch".

The implementation does the opposite: crates/felis-protocol/src/convert/session.rs passes the prefix through unvalidated, and crates/felis-daemon/src/serve.rs (take_attach_handle) resolves it with felis_protocol::session_prefix::resolve_session_prefix, which returns ResolvedId::NoMatch for an empty, overlong, or non-hex prefix (crates/felis-protocol/src/session_prefix.rs). So an attach with a malformed prefix is answered AttachFailure::NoMatch, exactly like an Ops verb.

Decision: either restore the decode-time refusal (fail closed, as the reference argues) or update the reference to the implemented NoMatch behavior. The implemented behavior is consistent with every other prefix field and with resolve_session_prefix's own comment, so updating the reference looks right; but the reference's rationale was deliberate, so this is a decision, not a typo.

2. Kitty placement `X`, `Y`

docs/reference/protocols/kitty-graphics.md "Placement parameters" lists "X, Y: Pixel offset within destination cell", and the support matrix marks a=p as supported. The dispatcher never reads them for a placement: upsert_placement (crates/felis-daemon/src/graphics.rs) reads c, r, z, C, q, p, and the x,y,w,h source rectangle, and felis_grid::images::Placement (crates/felis-grid/src/images.rs:644) has no destination-offset field. X/Y are read only for a=c (frame composition), where they name the source offset.

So a producer that sends a=p with X=/Y= gets an image at the cell origin, silently. Decision: implement destination pixel offsets (a wire field plus renderer work, so an additive post-freeze change) or drop the X/Y line and state that destination pixel offsets are not applied. The honest pre-freeze state is the latter; the former can land later as an additive minor.

Acceptance criteria

Each claim matches the code, or the code matches the claim.
The support matrix and the reference agree.
The grep sweep for the changed terms runs across docs/.

## Context Pre-release review of the frozen surfaces (round 2). Two normative statements in `docs/reference/` describe behavior the code does not have. Each needs a decision: fix the reference, or implement the claim before the freeze. A third and fourth case are mechanical wording errors and go in a separate doc commit. ## 1. `SessionAttach.id_prefix` validation `docs/reference/ipc.md` (the `Attach` bullet under "Session (kind = 4)") says a target whose `id_prefix` "is not 1-32 hex digits" is "refused at decode", and that an attach, "unlike the `Ops` verbs' `id_prefix`, cannot answer a malformed prefix with `NoMatch`". The implementation does the opposite: `crates/felis-protocol/src/convert/session.rs` passes the prefix through unvalidated, and `crates/felis-daemon/src/serve.rs` (`take_attach_handle`) resolves it with `felis_protocol::session_prefix::resolve_session_prefix`, which returns `ResolvedId::NoMatch` for an empty, overlong, or non-hex prefix (`crates/felis-protocol/src/session_prefix.rs`). So an attach with a malformed prefix is answered `AttachFailure::NoMatch`, exactly like an `Ops` verb. Decision: either restore the decode-time refusal (fail closed, as the reference argues) or update the reference to the implemented `NoMatch` behavior. The implemented behavior is consistent with every other prefix field and with `resolve_session_prefix`'s own comment, so updating the reference looks right; but the reference's rationale was deliberate, so this is a decision, not a typo. ## 2. Kitty placement `X`, `Y` `docs/reference/protocols/kitty-graphics.md` "Placement parameters" lists "`X`, `Y`: Pixel offset within destination cell", and the support matrix marks `a=p` as supported. The dispatcher never reads them for a placement: `upsert_placement` (`crates/felis-daemon/src/graphics.rs`) reads `c`, `r`, `z`, `C`, `q`, `p`, and the `x,y,w,h` source rectangle, and `felis_grid::images::Placement` (`crates/felis-grid/src/images.rs:644`) has no destination-offset field. `X`/`Y` are read only for `a=c` (frame composition), where they name the source offset. So a producer that sends `a=p` with `X=`/`Y=` gets an image at the cell origin, silently. Decision: implement destination pixel offsets (a wire field plus renderer work, so an additive post-freeze change) or drop the `X`/`Y` line and state that destination pixel offsets are not applied. The honest pre-freeze state is the latter; the former can land later as an additive minor. ## Acceptance criteria - [ ] Each claim matches the code, or the code matches the claim. - [ ] The support matrix and the reference agree. - [ ] The grep sweep for the changed terms runs across `docs/`.

natsukium added the

priority/P1

release/v0.1.0

type/bug

labels

2026-09-22 20:17:01 +09:00

natsukium referenced this issue

2026-09-22 20:19:51 +09:00

docs: align the IPC reference with the code it describes #350

natsukium referenced this issue from a commit

2026-09-22 20:34:54 +09:00

docs: align the IPC reference with the code it describes (#350)

natsukium referenced this issue

2026-09-22 20:38:08 +09:00

protocol, cli: pre-release review of frozen surfaces — six cheap-before-freeze gaps #347

natsukium commented

2026-09-22 20:38:08 +09:00

Author

Owner

Triage (2026-09-22)

Adding a third claim of the same kind, from #347 finding C: OSC 1 sets only the daemon-side icon name (felis-grid/src/sink.rs), which no GridMsg arm carries, so the client never applies it. The support matrix's check mark overstates what the sequence does. Same treatment as the other two: state what is implemented.

Recommended resolution for the first two, so the PR need not reopen them: item 1 updates the reference to the implemented NoMatch behavior (consistent with every other prefix field and with resolve_session_prefix); item 2 drops the X/Y line and states that destination pixel offsets are not applied. Both are doc-only.

## Triage (2026-09-22) Adding a third claim of the same kind, from #347 finding C: OSC 1 sets only the daemon-side icon name (`felis-grid/src/sink.rs`), which no `GridMsg` arm carries, so the client never applies it. The support matrix's check mark overstates what the sequence does. Same treatment as the other two: state what is implemented. Recommended resolution for the first two, so the PR need not reopen them: item 1 updates the reference to the implemented `NoMatch` behavior (consistent with every other prefix field and with `resolve_session_prefix`); item 2 drops the `X`/`Y` line and states that destination pixel offsets are not applied. Both are doc-only.

natsukium referenced this issue from a commit

2026-09-22 21:15:18 +09:00

docs: state what the attach, placement, and icon-name code actually does

natsukium referenced this issue from a pull request that will close it,

2026-09-22 21:15:36 +09:00

docs: state what the attach, placement, and icon-name code actually does #353

natsukium closed this issue

2026-09-22 21:48:25 +09:00

Commenting is not possible because the repository is archived.