natsukium/felis

Watch

[v0.1/Config Review] client overlay の schema/runtime 乖離と additionalProperties 凍結 #140

New issue

Closed

opened 2026-09-05 11:38:37 +09:00 by natsukium · 2 comments

natsukium commented

2026-09-05 11:38:37 +09:00

Owner

背景

EffectiveConfig は #[serde(default)] による lenient parsing を永続契約とする（docs/explanation/architecture/control-surfaces.md「Lenient config parsing」: 共有文書なので vocabulary を1消費者が所有しない）。具体的には:

crates/felis-client-core/src/config.rs の EffectiveConfig と全 sub-struct は #[serde(default)]。
未知キーは serde_ignored で warning に回し、parse error にしない。
client: BTreeMap<String, toml::Value> は他 client セクションを raw TOML のまま保持し、この client の schema で検証しない。

一方 JSON Schema (crates/felis-client-core/felis-config.schema.json, just schema 生成) は:

#[cfg_attr(feature = "schema", schemars(with = "BTreeMap<String, EffectiveConfig>"))]
pub client: BTreeMap<String, toml::Value>,

と schemars(with = ...) で上書きし、他 client セクションを EffectiveConfig として厳密に検証する形で出力する。runtime は toml::Value で lenient、schema は EffectiveConfig で strict という乖離がある。

さらに runtime が BTreeMap<String, toml::Value> であるため、トップレベルに additionalProperties: false を置けない理由が「単一文書に複数 client の vocabulary が同居する」ことだけで、1.0 で凍結するとサードパーティ frontend が独自キーを追加した際の editor 上の typo 検出と forward-compat の両立が permanent に lenient に固定される。#4「単一文書 + オーバレイ vs 複数ファイル」で config.d/<client>.toml の drop-in 案が提示されたが、additionalProperties の strict 化可否という schema 観点の決定が未分離。

これは config の「キー名は今しか変えられないが、追加は additive で読める」という契約の核心で、1.0 でキー集合を凍結する際に schema/runtime の乖離を残すと、後から strict 化は breaking（既存の lenient ファイルが error になる）。

問い

1.0 で以下を決める:

schema と runtime の client 型を一致させるか — schemars(with = BTreeMap<String, EffectiveConfig>) をやめて BTreeMap<String, toml::Value> 相当の permissive schema にするか、逆に runtime も BTreeMap<String, EffectiveConfig> に寄せて「他 client セクションも警告対象」に昇格するか。
単一文書の additionalProperties を non-strict のまま凍結するか — 現行の「unknown key は warn」契約を 1.0 で規範化し、felis-config.schema.json のトップレベル additionalProperties: false を永遠に置かない決定として reference/config.md に明記するか。config.d/ に移行するなら今が最後のチャンスで、#4 と合わせて判断が必要。
他 client セクションの inert 性をどこまで保証するか — client.other の中身が壊れた TOML でも current client の起動を妨げないことはテストされているか。diagnostics が他 client の unknown_key を report しないことは 1.0 の契約として固定するか。

提案

短期: schema の client 型を runtime に揃える — schemars(with = BTreeMap<String, toml::Value>) 相当の open map にし、「他 client セクションは inert、検証しない」ことを reference/config.md「Per-client overrides」に frozen として追記。additionalProperties: false を置かない理由を felis-config.schema.json のトップコメントと config.md に残す。
長期トリガ: explanation/architecture/control-surfaces.md に Revisit trigger を残す —「全 client が schema を共有できた時、または additionalProperties: false で typo 検出を厳密にしたい要望が継続したら config.d/ への移行を検討」。#4 とリンク。
just schema の snapshot と felis config check の diagnostics カバレッジに「他 client の壊れた TOML が current client を壊さない」ケースを追加。

判定基準

felis-config.schema.json を VS Code に当てた時に、自 client セクションの typo は波線、他 client セクションの独自キーは波線にならない（inert）ことが editor 上で確認できること。
cargo test -p felis-client-core --features schema の schema snapshot が runtime の EffectiveConfig と乖離しないこと（schemars の with 乖離が無い）。
reference/config.md「Per-client overrides」「Behavior on missing / malformed values」表に「他 client セクションは inert」が 1.0 凍結として明記されていること。
explanation/architecture/control-surfaces.md に strict 化の Revisit trigger が記録されていること。

対象ファイル

crates/felis-client-core/src/config.rs (EffectiveConfig.client 型、schemars attribute)
crates/felis-client-core/felis-config.schema.json (生成物、トップコメント)
docs/reference/config.md「Per-client overrides」「Behavior on missing / malformed values」
docs/explanation/architecture/control-surfaces.md「Lenient config parsing」
tests/ (config diagnostics の cross-crate guard)

Parent: #12 および #11 / #4

## 背景 `EffectiveConfig` は `#[serde(default)]` による lenient parsing を永続契約とする（`docs/explanation/architecture/control-surfaces.md`「Lenient config parsing」: 共有文書なので vocabulary を1消費者が所有しない）。具体的には: - `crates/felis-client-core/src/config.rs` の `EffectiveConfig` と全 sub-struct は `#[serde(default)]`。 - 未知キーは `serde_ignored` で warning に回し、parse error にしない。 - `client: BTreeMap<String, toml::Value>` は他 client セクションを raw TOML のまま保持し、この client の schema で検証しない。一方 JSON Schema (`crates/felis-client-core/felis-config.schema.json`, `just schema` 生成) は: ```rust #[cfg_attr(feature = "schema", schemars(with = "BTreeMap<String, EffectiveConfig>"))] pub client: BTreeMap<String, toml::Value>, ``` と `schemars(with = ...)` で上書きし、他 client セクションを `EffectiveConfig` として厳密に検証する形で出力する。runtime は `toml::Value` で lenient、schema は `EffectiveConfig` で strict という乖離がある。さらに runtime が `BTreeMap<String, toml::Value>` であるため、トップレベルに `additionalProperties: false` を置けない理由が「単一文書に複数 client の vocabulary が同居する」ことだけで、1.0 で凍結するとサードパーティ frontend が独自キーを追加した際の editor 上の typo 検出と forward-compat の両立が permanent に lenient に固定される。`#4`「単一文書 + オーバレイ vs 複数ファイル」で `config.d/<client>.toml` の drop-in 案が提示されたが、`additionalProperties` の strict 化可否という schema 観点の決定が未分離。これは config の「キー名は今しか変えられないが、追加は additive で読める」という契約の核心で、1.0 でキー集合を凍結する際に schema/runtime の乖離を残すと、後から strict 化は breaking（既存の lenient ファイルが error になる）。 ## 問い 1.0 で以下を決める: 1. **schema と runtime の `client` 型を一致させるか** — `schemars(with = BTreeMap<String, EffectiveConfig>)` をやめて `BTreeMap<String, toml::Value>` 相当の permissive schema にするか、逆に runtime も `BTreeMap<String, EffectiveConfig>` に寄せて「他 client セクションも警告対象」に昇格するか。 2. **単一文書の `additionalProperties` を non-strict のまま凍結するか** — 現行の「unknown key は warn」契約を 1.0 で規範化し、`felis-config.schema.json` のトップレベル `additionalProperties: false` を永遠に置かない決定として `reference/config.md` に明記するか。`config.d/` に移行するなら今が最後のチャンスで、`#4` と合わせて判断が必要。 3. **他 client セクションの inert 性をどこまで保証するか** — `client.other` の中身が壊れた TOML でも current client の起動を妨げないことはテストされているか。`diagnostics` が他 client の unknown_key を report しないことは 1.0 の契約として固定するか。 ## 提案 - **短期**: schema の `client` 型を runtime に揃える — `schemars(with = BTreeMap<String, toml::Value>)` 相当の open map にし、「他 client セクションは inert、検証しない」ことを `reference/config.md`「Per-client overrides」に frozen として追記。`additionalProperties: false` を置かない理由を `felis-config.schema.json` のトップコメントと `config.md` に残す。 - **長期トリガ**: `explanation/architecture/control-surfaces.md` に Revisit trigger を残す —「全 client が schema を共有できた時、または `additionalProperties: false` で typo 検出を厳密にしたい要望が継続したら `config.d/` への移行を検討」。`#4` とリンク。 - `just schema` の snapshot と `felis config check` の diagnostics カバレッジに「他 client の壊れた TOML が current client を壊さない」ケースを追加。 ## 判定基準 - `felis-config.schema.json` を VS Code に当てた時に、自 client セクションの typo は波線、他 client セクションの独自キーは波線にならない（inert）ことが editor 上で確認できること。 - `cargo test -p felis-client-core --features schema` の schema snapshot が runtime の `EffectiveConfig` と乖離しないこと（`schemars` の `with` 乖離が無い）。 - `reference/config.md`「Per-client overrides」「Behavior on missing / malformed values」表に「他 client セクションは inert」が 1.0 凍結として明記されていること。 - `explanation/architecture/control-surfaces.md` に strict 化の Revisit trigger が記録されていること。 ## 対象ファイル - `crates/felis-client-core/src/config.rs` (`EffectiveConfig.client` 型、`schemars` attribute) - `crates/felis-client-core/felis-config.schema.json` (生成物、トップコメント) - `docs/reference/config.md`「Per-client overrides」「Behavior on missing / malformed values」 - `docs/explanation/architecture/control-surfaces.md`「Lenient config parsing」 - `tests/` (config diagnostics の cross-crate guard) Parent: #12 および #11 / #4

natsukium added the

priority/P1

release/v0.1.0

labels

2026-09-05 12:11:14 +09:00

natsukium commented

2026-09-05 12:11:18 +09:00

Author

Owner

Triage (2026-09-05)

Verdict: accept, P1. Verified: config.rs:73 overrides the schema type of client to BTreeMap<String, EffectiveConfig> while runtime holds toml::Value. #29 publishes this schema, so the mismatch must be gone before then. Take the short-term proposal: an open map in the schema matching the runtime, "other client sections are inert and unvalidated" frozen in reference/config.md, the "no top-level additionalProperties: false" reason in the schema header, and a Revisit trigger (config.d/ drop-ins) in explanation/architecture/control-surfaces.md. Add the "broken TOML under another client section does not stop this client" case to the diagnostics tests. Order: before #29.

## Triage (2026-09-05) **Verdict: accept, P1.** Verified: `config.rs:73` overrides the schema type of `client` to `BTreeMap<String, EffectiveConfig>` while runtime holds `toml::Value`. #29 publishes this schema, so the mismatch must be gone before then. Take the short-term proposal: an open map in the schema matching the runtime, "other client sections are inert and unvalidated" frozen in `reference/config.md`, the "no top-level `additionalProperties: false`" reason in the schema header, and a Revisit trigger (`config.d/` drop-ins) in `explanation/architecture/control-surfaces.md`. Add the "broken TOML under another client section does not stop this client" case to the diagnostics tests. Order: before #29.

natsukium referenced this issue

2026-09-05 12:11:19 +09:00

[v0.1/Tracker] Close release blockers before v0.1.0 #12

natsukium commented

2026-09-05 21:25:59 +09:00

Author

Owner

Triage plan (2026-09-05)

Source-grounded triage against main at 07a77ae4, reviewed through six rounds of an independent reviewer (pi luna 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).

Config schema client map: match the runtime

Claim check

crates/felis-client-core/src/config.rs:70-75: runtime client: BTreeMap<String, toml::Value>, schema overridden to
BTreeMap<String, EffectiveConfig>, which in the generated
felis-config.schema.json:7-16 becomes additionalProperties: {"$ref": "#"}.
So an editor validates every [client.<name>] section against this
client's vocabulary, while the runtime validates none but its own
(document.rs:137-165, serde_ignored after the overlay merge).
The root object has no additionalProperties: false (the 23 occurrences
are nested closed shapes such as keymap entries), so today an editor
flags neither own-section typos nor foreign-section keys; it flags only
type mismatches in any section. The issue's acceptance criterion
("own-client typos squiggle") is therefore not satisfiable by the
client change alone; it needs a decision on root strictness.

Verdict

accept. Root strictness is not open: docs/explanation/architecture/control-surfaces.md:796-812
already records "the generated JSON schema is deliberately non-strict"
(editor completion and hover; the runtime parser is the authority;
Revisit: none). So the root and every section stay open, and the only
change is the client map: name the known client id explicitly,
properties: { "felis": {"$ref": "#"} }, additionalProperties: true.
That validates the GUI's own overlay with its own schema and leaves every
other client's section open, which is the runtime split. The issue's
"own-client typos squiggle" criterion is therefore out of scope (it would
need the strictness that decision rejects); say so in the plan comment.

Approach

config.rs:70-75: replace schemars(with = …) by schemars(schema_with = "client_overlay_schema") producing the shape above (the client id
comes from the same constant the loader matches on; verify its name in
document.rs).
just schema; update the schema header text to state the two reasons
(open map: other clients' sections are inert; no root
additionalProperties: false: shared document, unknown keys warn).
docs/reference/config.md "Per-client overrides": "Other client
sections are inert" becomes a frozen 1.0 statement, plus one sentence
that the published schema validates only the felis overlay.
docs/explanation/architecture/control-surfaces.md: the "Lenient
config parsing" decision keeps its Revisit: none. The
config.d/<client>.toml drop-in idea belongs to the one-document
decision it already sits under (:789), so add the Revisit trigger
there, with a concrete condition: a second first-party client ships
whose top-level vocabulary diverges from the GUI's (a section one
client owns and the other must not read), linking #4.
Tests: (1) schema snapshot test in config.rs (:2503) asserts the
client shape; (2) diagnostics test: a [client.other] section holding
a wrong-typed known key and an unknown key yields no diagnostic and does
not stop this client's load.

Dependencies / risk / labels

Before #29 (publishes the schema). S. Keep labels. CHANGELOG.md:
the schema's overlay validation changes (a config surface).

## Triage plan (2026-09-05) Source-grounded triage against `main` at `07a77ae4`, reviewed through six rounds of an independent reviewer (`pi` luna 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). **Config schema `client` map: match the runtime** ### Claim check - `crates/felis-client-core/src/config.rs:70-75`: runtime `client: BTreeMap<String, toml::Value>`, schema overridden to `BTreeMap<String, EffectiveConfig>`, which in the generated `felis-config.schema.json:7-16` becomes `additionalProperties: {"$ref": "#"}`. So an editor validates *every* `[client.<name>]` section against this client's vocabulary, while the runtime validates none but its own (`document.rs:137-165`, `serde_ignored` after the overlay merge). - The root object has no `additionalProperties: false` (the 23 occurrences are nested closed shapes such as keymap entries), so today an editor flags neither own-section typos nor foreign-section keys; it flags only *type* mismatches in any section. The issue's acceptance criterion ("own-client typos squiggle") is therefore not satisfiable by the `client` change alone; it needs a decision on root strictness. ### Verdict **accept.** Root strictness is not open: `docs/explanation/architecture/control-surfaces.md:796-812` already records "the generated JSON schema is deliberately non-strict" (editor completion and hover; the runtime parser is the authority; Revisit: none). So the root and every section stay open, and the only change is the `client` map: name the known client id explicitly, `properties: { "felis": {"$ref": "#"} }`, `additionalProperties: true`. That validates the GUI's own overlay with its own schema and leaves every other client's section open, which is the runtime split. The issue's "own-client typos squiggle" criterion is therefore out of scope (it would need the strictness that decision rejects); say so in the plan comment. ### Approach - `config.rs:70-75`: replace `schemars(with = …)` by `schemars(schema_with = "client_overlay_schema")` producing the shape above (the client id comes from the same constant the loader matches on; verify its name in `document.rs`). - `just schema`; update the schema header text to state the two reasons (open map: other clients' sections are inert; no root `additionalProperties: false`: shared document, unknown keys warn). - `docs/reference/config.md` "Per-client overrides": "Other client sections are inert" becomes a frozen 1.0 statement, plus one sentence that the published schema validates only the `felis` overlay. - `docs/explanation/architecture/control-surfaces.md`: the "Lenient config parsing" decision keeps its `Revisit: none`. The `config.d/<client>.toml` drop-in idea belongs to the *one-document* decision it already sits under (`:789`), so add the Revisit trigger there, with a concrete condition: a second first-party client ships whose top-level vocabulary diverges from the GUI's (a section one client owns and the other must not read), linking #4. - Tests: (1) schema snapshot test in `config.rs` (`:2503`) asserts the `client` shape; (2) diagnostics test: a `[client.other]` section holding a wrong-typed known key and an unknown key yields no diagnostic and does not stop this client's load. ### Dependencies / risk / labels Before #29 (publishes the schema). **S.** Keep labels. `CHANGELOG.md`: the schema's overlay validation changes (a config surface).

natsukium added the

status/planned

label

2026-09-05 21:25:59 +09:00

natsukium referenced this issue

2026-09-05 21:26:13 +09:00

[v0.1/Tracker] Close release blockers before v0.1.0 #12

natsukium referenced this issue from a commit

2026-09-06 03:38:16 +09:00

config: validate only this client's overlay in the JSON schema

natsukium referenced this issue from a commit

2026-09-06 03:38:16 +09:00