[v0.1/IPC Review] GridDims の三重検証と ResourceReport Optional 集計の語彙を 1.0 で凍結 #146
Labels
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
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
natsukium/felis#146
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
背景
GridDimsとResourceReportは proto/IPC の core 型だが、1.0 で凍結すると破壊的変更が不可能になる箇所に「二重検証」と「語彙の紛らわしさ」が残っている。1.
GridDimsのadmit/clamp/admit_announced三重検証crates/felis-protocol/src/messages.rs: RequestedDimsはadmit(create:0→daemon default, 範囲外はErr GeometryRejection),clamp(resize: 範囲外は clamp),admit_announced(Size/attach 報告:0は sentinel ではなく error) の 3 パスを持つ。admitとadmit_announcedの違いはrows/colsの0を sentinel と見るか error と見るかの 1 bit のみ。clampは resize 専用だが、InputResizeとSessionCreateの両方がGridDimsを運ぶため、decode 後に「どちらの検証を呼ぶべきか」は family ではなく verb で決まる。proto/felis.protoコメントは「SpawnArgs.dimsとInputResize.dimsは receiver が bounds を apply」と書くが、proto 上は同じGridDims型で区別がない。結果、将来「
0を valid geometry にする」変更は 3 関数すべてに影響し、片方だけ直すと他で 0 sentinel が valid にすり替わる。2.
ResourceReportの Optional 集計フィールドmax_subject_used/per_subject_limit/global_limitは scope により optional で、reference/cli.mdDaemon status 表は scope ごとの denominator を「total_usedagainstglobal_limit,max_subject_usedagainstper_subject_limit」と説明する。問題:
optionalの absent と0の区別が docs に埋もれており、json出力で absent は key 自体が消えるため、consumer が0(値) とnull(absent) を区別しないと「limit 0」 と「limit なし」を誤読する。DAEMONscope でmax_subject_usedが absent になることはreference/ipc.md「Ops (kind=5)」節に散在するが、proto 上はoptionalの理由がコメントだけで型に反映されない。3.
SpawnArgsとEnvBaseの二重 env 語彙SpawnArgsはenv: repeated EnvPair (string key/value)とenv_base: optional EnvBase (bytes key/value)の二層を持つ。前者は CLI--env KEY=VALの user-typed UTF-8、後者は relay が捕捉した host-native bytes(Unix raw bytes / Windows UTF-16 LE)。docs は
REQ-912inherit-then-sanitize とFRLYcarrier block の第一候補を説明するが、proto 上でstringとbytesの使い分け理由がEnvPair/EnvBytesPairのコメントにしかない。1.0 で語彙を凍結するなら「どちらが UTF-8 保証でどちらが platform-native か」をreference/ipc.mdの Session 節の表で凍結すべき。問い
1.0 で以下を決める:
RequestedDimsの 3 関数をadmit_create/admit_resize/admit_announcedに rename し、proto コメントに「同じGridDims型でも検証は verb で決まる」と frozen として明記するか。あるいはSpawnArgs.dimsとInputResize.dimsを別 message 型 (CreateDims/ResizeDims) に分け、proto 上で sentinel 意味を分離するか。ResourceReportの optional 3 field を non-optional +0 = no limitの sentinel に正規化するか、現行のoptionalのままreference/ipc.mdとreference/cli.mdに「absent の意味」表を凍結するか。SpawnArgs.env/env_baseのstringvsbytes使い分けをreference/ipc.md「Session (kind=4)」節の表に凍結として刻むか。提案
RequestedDimsの 3 関数を rename せず、コメントとreference/ipc.md「Session (kind=4)」節に「GridDimsは wire 上同一だがSpawnArgs.dimsはadmit(0 sentinel + refuse),InputResize.dimsはclamp(0→min),GridSizeはadmit_announced(0 error) の三者で検証が異なる」と frozen 表を追加。ResourceReportはoptionalのまま残し、reference/cli.mdDaemon status 表に「max_subject_usedはSUBJECT_SCOPE_DAEMONで absent」行を frozen として追加。SpawnArgs.env/env_baseは proto コメントのまま frozen とし、reference/ipc.mdに「envは UTF-8 の user-typed,env_baseは platform-native bytes」行を追加。GridDimsを分離するならexplanation/architecture/ipc.mdに Revisit trigger を残す:「0sentinel を廃止しoptional GridDimsにするなら major bump」。どちらも 1.0 で docs に刻むことが目的で、wire 型自体は変えない案。
ResourceReportを non-optional に変えるならBREAKING.mdにbase: <sha>が必要になるため、今回は docs 凍結に留める。判定基準
crates/felis-protocol/src/messages.rsのRequestedDims3 関数のコメントとdocs/reference/ipc.md「Session (kind=4)」節の表が 1:1 で対応し、新 contributor が「どの検証を呼ぶか」を再発明しないこと。ResourceReportの 3 optional field がreference/ipc.md「Ops (kind=5)」とreference/cli.mdDaemon status で同じ「absent の意味」表で説明されていること。SpawnArgs.env/env_baseのstringvsbytes区別がreference/ipc.mdの Session 節で凍結として読めること。対象ファイル
crates/felis-protocol/src/messages.rs(RequestedDims,GridDims,SpawnArgs)crates/felis-protocol/proto/felis.proto(GridDims,ResourceReport,SpawnArgs,EnvBase)docs/reference/ipc.md「Session (kind=4)」「Ops (kind=5)」「Semantic limits」docs/reference/cli.md「Daemon status」docs/explanation/architecture/ipc.md「Session lifecycle」「Carrier choices」Parent: #12 および #52 / #16 / #26
Triage (2026-09-05)
Verdict: accept as proposed (docs-only freeze), P1. No wire change: the three
RequestedDimspaths, theoptionalsemantics ofResourceReport, and theenv(UTF-8, user-typed) vsenv_base(platform-native bytes) split get frozen tables inreference/ipc.mdandreference/cli.md, with a Revisit trigger inexplanation/architecture/ipc.mdfor the0-sentinel major bump. The JSON absent-key vs0point forResourceReportalso belongs in thedaemon statussection ofreference/cli.md, since machine consumers read it there. Parent: #52.リリース前なので最小breakingにこだわる必要はない
Triage plan (2026-09-05)
Source-grounded triage against
mainat07a77ae4, reviewed through six rounds of an independent reviewer (piluna then sol); every finding was verified against the source and folded in, and the design decisions below were settled with the maintainer on 2026-09-05. The order that supersedes the tracker's is posted on #12: this issue lands before #29 (schemas, goldens, and reference tables freeze there) and, because it breaks the wire, before #50.GridDimssentinels,ResourceReportoptionals,env/env_baseClaim check
messages.rs:231-275RequestedDims::{admit, admit_announced, clamp}:the only difference between
admitandadmit_announcedis whetherrows/cols
0means "daemon default" (create) or is an error(announced). Pixel
0= unknown in all three.ResourceReport(felis.proto:1619-1640): the three optionals carrypresence deliberately and the comments say when each is unset;
ipc.md:1164-1176repeats it;cli.md:183-220does not say that anabsent field is an omitted JSON key.
EnvPair(string) vsEnvBytesPair/EnvBase(bytes):felis.proto:453-481, comments only.Verdict
accept, with one wire change per the maintainer's note.
**Decision (dims) — settled 2026-09-05: make
SpawnArgs.dimsanoptional GridDimsand drop the rows/cols0sentinel from creates.Absent = the daemon default; present = every axis must be in band
(pixel
0= unknown stays, because a headless create genuinely has nopixel size).
admitandadmit_announcedthen collapse into oneadmit(create and announced), leaving two paths:admit(reject) andclamp(live resize). Wire cost: a message field already has presence inproto3, so
optionalon it does not change encoding; the semanticbreak (a
dims { rows: 0 }create is now refused instead of defaulted)gets the
base:line. Alternative: keep the sentinel and rename thefunctions (
admit_create/admit_resize/admit_announced), docs-only.Rejected because one type with three checks is exactly the trap the
issue names, and the fix is a presence bit the wire already has.
ResourceReport: keepoptional. A0 = no limitsentinel wouldreintroduce the ambiguity the issue objects to elsewhere; presence is the
right encoding. Docs state it.
env/env_base: docs only.Approach
felis.proto:optional GridDims dims = 5inSpawnArgs, commentrewritten;
GridDimscomment states zero validity per carryingfield (create: absent = default, present rows/cols must be in band;
InputResize: any value, clamped; announced: rows/cols0refused;pixel
0= unknown everywhere), not a global rule;just proto.BREAKING.mdbase:line whose prose names the semantic breakbufcannot see: an old client's
rows: 0create is now refused, and a newclient's absent
dimsmakes an old daemon's converter returnMissingField, which ends the connection rather than refusing thecreate — the same rebuild-together posture as #144.
messages.rs:SpawnArgs.dims: Option<RequestedDims>; deleteadmit_announced,admittakes the announced rule (the Kani harnessmessages.rs:627-651is re-pointed atadmit, same property); thedaemon's one create path (
serve.rs:922create_session, shared bySessionMsg::CreateandOpsMsg::Spawn) mapsNone→ default dims; CLIsessions spawn --rows/--cols(verify the flag names) sendsNonewhenneither is given, refuses a partial pair locally (usage, exit 2); the
bridge builds
SpawnArgsin two places (cli_bridge.rs:1168-1183,:1616-1629) withrows/colsindependently optional — both sendNonewhen neither is given and answer a partial pair with a localinvalid_request, tested in the bridge goldens.docs/reference/ipc.md"Session (kind = 4)": a table forGridDimsper carrying field (
SpawnArgs.dims: absent = default, present =admitted;
InputResize.dims: clamped;GridSize/SessionInfo.dims:admitted); "Ops (kind = 5)": absent-vs-
0table for the threeResourceReportoptionals;env(UTF-8, user-typed, applied last) vsenv_base(platform-native bytes, replaces the base) rows.docs/reference/cli.md"Daemon status": "an unset field is an omittedkey in
--format json;0is a measured zero";sessions spawngeometry flags: partial pair is
usage.docs/explanation/architecture/ipc.md: why presence, not sentinels,for both dims and resource ceilings; Revisit trigger for pixel
0.docs/reference/spec.mdREQ-605a (the create sentinel is part of itstext) and its owner,
docs/explanation/architecture/session-lifecycle.md"Geometry bounds";
skills/felis/SKILL.md(sessions spawn/ bridgesessions.spawngeometry params);CHANGELOG.md.None/partial/in-band/out-of-band; CLI partial-pair refusal; the Kaniharness (
messages.rs:627-651) re-pointed atadmit. The existinggeometry tests encode the old sentinel and must change with it:
messages.rs:1030("sentinels survive", an all-zero create admitted)becomes a pixel-only sentinel case, and
crates/felis-protocol/tests/proptest_geometry.rs:59-133—in_boundsdraws zero rows/cols today and the offender classification treats
every zero axis as valid — draws rows/cols only in
MIN..=MAX, keepszero for pixel axes, and classifies offenders per axis.
Dependencies / risk / labels
Before #50 and #29; can share a PR with #144 only if the maintainer wants
one
base:line, otherwise separate. M. Keep labels. Parent #52.