[v0.1/CLI Review] Point/Stream 例外と ErrorKind 二重定義を 1.0 で整理 #145

Closed
opened 2026-09-05 11:57:42 +09:00 by natsukium · 2 comments
Owner

背景

docs/reference/cli.md の Machine output 節と crates/felis-cli/src/cli_output.rsErrorKind::exit_code / from_stream_reason は 1.0 で frozen するが、現行の CLI に「Point/Stream 例外」と「ErrorKind 二重定義」が残っている。

1. Point/Stream 例外の散在

reference/cli.md は全 verb を Point / Stream / Exempt に三分するが、例外が 3 箇所ある:

  • felis config check / show-effective: Point だが exit 1 が「document errors」を意味し、他 Point の 1 (no_match/ambiguous/refused) と意味が違う。cli.md Exit codes 表に「Diagnostic exceptions」として脚注があるのみ。
  • felis doctor: Point だが daemon unreachable を exit 2 ではなく warn + exit 1/0 にする例外(cli.md Doctor 節)。
  • felis daemon status: Point だが ResourceReportworker_threadsresources[] 外の単独フィールドで、他 Point の {"v":1, ...} と shape が異なる。

いずれも reference/cli.md 分類表では同じ Point に置かれ、例外が表から読めない。

2. ErrorKind の二重定義と exit_code の重複

crates/felis-cli/src/cli_output.rs: ErrorKind (20 variants) と crates/felis-protocol/src/messages.rs: StreamErrorReason (4 variants) / AttachFailure / RefusalReason が別オントロジーで、map が ErrorKind::from_stream_reason と各 verb の match に散在する:

  • StreamErrorReason::TooManyStreams → AtCapacitycli_output.rs で一箇所だけ map。
  • AttachFailure::SessionLimitReached → SpawnFailed downgrade は convert/session.rs で ledger 行5として処理。
  • RefusalReason::AtCapacity → daemon unreachable の preface レベルの map は connector.rs

docs/reference/cli.md Exit codes 表はこれらを「Kinds mapping to exit 1/2」として一覧するが、ErrorKind::exit_codematch と手で同期する必要があり、ずれると exit 契約が破れる。skills/felis の scripting 例もこの表を参照する。

3. --format の verb クラスごとの拒否が clap value_parser に依存

PointFormat::parse_point_formatjsonl を拒否し、StreamFormat::parse_stream_formatjson を拒否する。拒否は clap の value_parser = fn で行われ、exit 2 + human message になるが、reference/cli.md Machine output 節の「Selecting an incompatible format exits 2」と同じ文言が cli_output.rs の Err 文字列にも重複する。二箇所を同期しないと help と docs がずれる。

問い

1.0 で以下を決める:

  1. config check / show-effective / doctor の exit 例外を Point のサブクラス (Point-diagnostic) として分類表に明示するか、現行の脚注のまま frozen にするか。
  2. ErrorKind を CLI surface の single source にし、StreamErrorReason / AttachFailure / RefusalReason との map を cli_output.rs の一箇所 (from_wire_reason / from_attach_failure) に集約するか。
  3. daemon statusworker_threadsresources[] に含めるか、単独フィールドのまま docs に「resource row ではない」理由を frozen として残すか。

提案

  • reference/cli.md「Machine output」分類表に「Point-diagnostic」行を追加し、config check / show-effective / doctor をそこに移動。Exit codes 表の「Diagnostic exceptions」脚注を削除し、表本体に config check: 1 = document errors を行として追加。cli_output.rsPointFormat / StreamFormat は現行の value_parser のまま frozen と明記。
  • ErrorKind::from_wire を新設し、StreamErrorReason / AttachFailure / RefusalReason からの map を一箇所に集約。reference/cli.md Exit codes 表はこの関数の match から生成される golden test (tests/cli_output.rs) で同期を担保。
  • daemon statusworker_threads は単独フィールドのまま残すが、reference/cli.md Daemon status 節に「worker_threads は resource row ではなく runtime identity」と frozen コメントを追加し、OpsStatusReply の proto コメントと一致させる。

判定基準

  • reference/cli.md「Classification by verb」表を読んだだけで config check が exit 1 で document error を意味することが分かり、sessions list1 (no_match) と混同しないこと。
  • ErrorKind から exit 1/2 への map が cli_output.rs の一箇所で定義され、reference/cli.md Exit codes 表と cargo test -p felis-cli --test cli_output の golden が同期していること。
  • felis daemon status --format jsonworker_threadsresources[] 外にあることが docs と proto コメントの両方で同じ理由で説明されていること。

対象ファイル

  • docs/reference/cli.md「Machine output」「Exit codes」「Daemon status」「Doctor」
  • crates/felis-cli/src/cli_output.rs (ErrorKind, from_stream_reason, exit_code)
  • crates/felis-cli/src/cli_sessions.rs / cli_config.rs / cli_daemon.rs (各 verb の ErrorKind 選択)
  • crates/felis-protocol/proto/felis.proto (OpsStatusReply.worker_threads コメント)
  • tests/ (cli_output golden, skills/felis の scripting 例)

Parent: #12 および #23 / #29 / #55

## 背景 `docs/reference/cli.md` の Machine output 節と `crates/felis-cli/src/cli_output.rs` の `ErrorKind::exit_code` / `from_stream_reason` は 1.0 で frozen するが、現行の CLI に「Point/Stream 例外」と「ErrorKind 二重定義」が残っている。 ### 1. Point/Stream 例外の散在 `reference/cli.md` は全 verb を Point / Stream / Exempt に三分するが、例外が 3 箇所ある: - `felis config check` / `show-effective`: Point だが exit `1` が「document errors」を意味し、他 Point の `1` (no_match/ambiguous/refused) と意味が違う。`cli.md` Exit codes 表に「Diagnostic exceptions」として脚注があるのみ。 - `felis doctor`: Point だが daemon unreachable を exit `2` ではなく `warn` + exit `1/0` にする例外(`cli.md` Doctor 節)。 - `felis daemon status`: Point だが `ResourceReport` の `worker_threads` が `resources[]` 外の単独フィールドで、他 Point の `{"v":1, ...}` と shape が異なる。 いずれも `reference/cli.md` 分類表では同じ Point に置かれ、例外が表から読めない。 ### 2. ErrorKind の二重定義と `exit_code` の重複 `crates/felis-cli/src/cli_output.rs: ErrorKind` (20 variants) と `crates/felis-protocol/src/messages.rs: StreamErrorReason` (4 variants) / `AttachFailure` / `RefusalReason` が別オントロジーで、map が `ErrorKind::from_stream_reason` と各 verb の `match` に散在する: - `StreamErrorReason::TooManyStreams → AtCapacity` は `cli_output.rs` で一箇所だけ map。 - `AttachFailure::SessionLimitReached → SpawnFailed` downgrade は `convert/session.rs` で ledger 行5として処理。 - `RefusalReason::AtCapacity → daemon unreachable` の preface レベルの map は `connector.rs`。 `docs/reference/cli.md` Exit codes 表はこれらを「Kinds mapping to exit 1/2」として一覧するが、`ErrorKind::exit_code` の `match` と手で同期する必要があり、ずれると exit 契約が破れる。`skills/felis` の scripting 例もこの表を参照する。 ### 3. `--format` の verb クラスごとの拒否が clap value_parser に依存 `PointFormat::parse_point_format` は `jsonl` を拒否し、`StreamFormat::parse_stream_format` は `json` を拒否する。拒否は clap の `value_parser = fn` で行われ、exit `2` + human message になるが、`reference/cli.md` Machine output 節の「Selecting an incompatible format exits 2」と同じ文言が `cli_output.rs` の Err 文字列にも重複する。二箇所を同期しないと help と docs がずれる。 ## 問い 1.0 で以下を決める: 1. `config check` / `show-effective` / `doctor` の exit 例外を Point のサブクラス (`Point-diagnostic`) として分類表に明示するか、現行の脚注のまま frozen にするか。 2. `ErrorKind` を CLI surface の single source にし、`StreamErrorReason` / `AttachFailure` / `RefusalReason` との map を `cli_output.rs` の一箇所 (`from_wire_reason` / `from_attach_failure`) に集約するか。 3. `daemon status` の `worker_threads` を `resources[]` に含めるか、単独フィールドのまま docs に「resource row ではない」理由を frozen として残すか。 ## 提案 - `reference/cli.md`「Machine output」分類表に「Point-diagnostic」行を追加し、`config check` / `show-effective` / `doctor` をそこに移動。Exit codes 表の「Diagnostic exceptions」脚注を削除し、表本体に `config check: 1 = document errors` を行として追加。`cli_output.rs` の `PointFormat` / `StreamFormat` は現行の value_parser のまま frozen と明記。 - `ErrorKind::from_wire` を新設し、`StreamErrorReason` / `AttachFailure` / `RefusalReason` からの map を一箇所に集約。`reference/cli.md` Exit codes 表はこの関数の `match` から生成される golden test (`tests/cli_output.rs`) で同期を担保。 - `daemon status` の `worker_threads` は単独フィールドのまま残すが、`reference/cli.md` Daemon status 節に「`worker_threads` は resource row ではなく runtime identity」と frozen コメントを追加し、`OpsStatusReply` の proto コメントと一致させる。 ## 判定基準 - `reference/cli.md`「Classification by verb」表を読んだだけで `config check` が exit `1` で document error を意味することが分かり、`sessions list` の `1` (no_match) と混同しないこと。 - `ErrorKind` から exit `1/2` への map が `cli_output.rs` の一箇所で定義され、`reference/cli.md` Exit codes 表と `cargo test -p felis-cli --test cli_output` の golden が同期していること。 - `felis daemon status --format json` の `worker_threads` が `resources[]` 外にあることが docs と proto コメントの両方で同じ理由で説明されていること。 ## 対象ファイル - `docs/reference/cli.md`「Machine output」「Exit codes」「Daemon status」「Doctor」 - `crates/felis-cli/src/cli_output.rs` (`ErrorKind`, `from_stream_reason`, `exit_code`) - `crates/felis-cli/src/cli_sessions.rs` / `cli_config.rs` / `cli_daemon.rs` (各 verb の ErrorKind 選択) - `crates/felis-protocol/proto/felis.proto` (`OpsStatusReply.worker_threads` コメント) - `tests/` (`cli_output` golden, `skills/felis` の scripting 例) Parent: #12 および #23 / #29 / #55
Author
Owner

Triage (2026-09-05)

Verdict: accept with narrowing, P1. The exit-code and classification tables freeze at v0.1.0, so the doc half must land before #29 records the goldens; the code half is a refactor that only matters because it feeds the golden.

  1. Add a Point-diagnostic row to the classification table for config check / show-effective / doctor, and move the "Diagnostic exceptions" footnote into the exit-code table body. No exit code changes.
  2. One ErrorKind::from_wire (stream reason / attach failure / refusal) in cli_output.rs, with a golden test that #29 freezes; the reference table is then derived, not hand-synced.
  3. Keep worker_threads outside resources[] and state the reason (runtime identity, not a resource row) in both the reference and the proto comment.

Order: before #29. Parent: #55.

## Triage (2026-09-05) **Verdict: accept with narrowing, P1.** The exit-code and classification tables freeze at v0.1.0, so the doc half must land before #29 records the goldens; the code half is a refactor that only matters because it feeds the golden. 1. Add a `Point-diagnostic` row to the classification table for `config check` / `show-effective` / `doctor`, and move the "Diagnostic exceptions" footnote into the exit-code table body. No exit code changes. 2. One `ErrorKind::from_wire` (stream reason / attach failure / refusal) in `cli_output.rs`, with a golden test that #29 freezes; the reference table is then derived, not hand-synced. 3. Keep `worker_threads` outside `resources[]` and state the reason (runtime identity, not a resource row) in both the reference and the proto comment. Order: before #29. Parent: #55.
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).

Point-diagnostic class, one wire → ErrorKind map, worker_threads

Claim check

  • docs/reference/cli.md:41-51 lists the diagnostic exceptions as prose
    after the exit-code table; :66-73 puts config check,
    show-effective, doctor in the plain Point row.
  • cli_output.rs:150-254: ErrorKind (21 variants), exit_code, and
    from_stream_reason are already the single stream map; the attach map
    is cli_sessions.rs:2322-2333 create_refusal, and the preface
    refusal map is conn.rs:90-104 (AtCapacity vs DaemonUnreachable).
    Three sites, as the issue says.
  • worker_threads: felis.proto:1607-1611 already states "identity, not a
    resource row"; cli.md:183-204 does not.

Verdict

accept with narrowing, as triaged. No exit-code change.

Approach

  1. cli.md: add a Point-diagnostic row to "Classification by verb"
    (config check, config show-effective, doctor) with the meaning of
    1 in the row; move the "Diagnostic exceptions" prose into the table
    body. Bridge's own-pipe exit 1 stays a sentence under felis bridge.
  2. cli_output.rs: ErrorKind::from_attach_failure and
    ErrorKind::from_refusal next to from_stream_reason, with
    create_refusal (cli_sessions.rs:2322-2333), the bridge's own
    inline arm (cli_bridge.rs:1648-1658, the same two-way split
    duplicated), the conn.rs arm, and the bridge's dial path
    (cli_bridge.rs:2877-2895, at_capacity_detail) all calling them. One golden test
    (crates/felis-cli/tests/ or the existing cli_output tests) that
    renders the full kind → exit table and the three wire maps as a fixture
    #29 will freeze; the reference table is then checked against the
    fixture, not hand-synced.
  3. cli.md "Daemon status": one sentence for worker_threads mirroring
    the proto comment.

Dependencies / risk / labels

Before #29. S. Keep labels. Parent #55.

## 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). **Point-diagnostic class, one wire → `ErrorKind` map, `worker_threads`** ### Claim check - `docs/reference/cli.md:41-51` lists the diagnostic exceptions as prose after the exit-code table; `:66-73` puts `config check`, `show-effective`, `doctor` in the plain Point row. - `cli_output.rs:150-254`: `ErrorKind` (21 variants), `exit_code`, and `from_stream_reason` are already the single stream map; the attach map is `cli_sessions.rs:2322-2333` `create_refusal`, and the preface refusal map is `conn.rs:90-104` (`AtCapacity` vs `DaemonUnreachable`). Three sites, as the issue says. - `worker_threads`: `felis.proto:1607-1611` already states "identity, not a resource row"; `cli.md:183-204` does not. ### Verdict **accept with narrowing, as triaged.** No exit-code change. ### Approach 1. `cli.md`: add a `Point-diagnostic` row to "Classification by verb" (`config check`, `config show-effective`, `doctor`) with the meaning of `1` in the row; move the "Diagnostic exceptions" prose into the table body. Bridge's own-pipe exit `1` stays a sentence under `felis bridge`. 2. `cli_output.rs`: `ErrorKind::from_attach_failure` and `ErrorKind::from_refusal` next to `from_stream_reason`, with `create_refusal` (`cli_sessions.rs:2322-2333`), the bridge's own inline arm (`cli_bridge.rs:1648-1658`, the same two-way split duplicated), the `conn.rs` arm, and the bridge's dial path (`cli_bridge.rs:2877-2895`, `at_capacity_detail`) all calling them. One golden test (`crates/felis-cli/tests/` or the existing `cli_output` tests) that renders the full kind → exit table and the three wire maps as a fixture #29 will freeze; the reference table is then checked against the fixture, not hand-synced. 3. `cli.md` "Daemon status": one sentence for `worker_threads` mirroring the proto comment. ### Dependencies / risk / labels Before #29. **S.** Keep labels. Parent #55.
Sign in to join this conversation.
No description provided.