natsukium/felis
Watch
1
Code Issues 27 Pull requests 1 Projects Releases Packages Wiki Activity Actions

[v0.1/P1] Simplify the CLI machine contract before freeze #23

New issue
Closed
opened 2026-09-03 16:18:04 +09:00 by natsukium · 2 comments
natsukium commented 2026-09-03 16:18:04 +09:00
Owner
Copy link

Parent: #12 (P1.4). Supersedes the actionable machine-contract scope in #3 and #9.

Why

The current contract freezes avoidable exceptions: a bounded roster is modeled as a stream, machine format does not own usage errors, exit statuses conflate typed refusal with transport failure, and identifiers/destination flags have context-dependent meanings.

Scope

  • Make sessions list a point result: {"v":1,"sessions":[...]}.
  • Once a valid machine format is recognized, encode later usage errors in that format on stderr.
  • Use exit 1 for typed daemon refusals and exit 2 for usage/transport/protocol failures.
  • Make error.kind additive; unknown values must degrade to generic failure.
  • Keep short_id only where roster uniqueness was established.
  • Rename retarget destinations to --to-host/--to-socket and remove felis ssh.
  • Correct notification help to match its human default.

Acceptance criteria

  • Every verb is classified as Point, Stream, or Exempt with one terminal rule.
  • Machine-mode usage, refusal, transport, and protocol failures have golden outputs and exit codes.
  • Point replies never imply unchecked short_id uniqueness.
  • Help, completions, man pages, CLI reference, schema, skills/felis, and changelog are regenerated together.
Parent: #12 (P1.4). Supersedes the actionable machine-contract scope in #3 and #9. ## Why The current contract freezes avoidable exceptions: a bounded roster is modeled as a stream, machine format does not own usage errors, exit statuses conflate typed refusal with transport failure, and identifiers/destination flags have context-dependent meanings. ## Scope - Make `sessions list` a point result: `{"v":1,"sessions":[...]}`. - Once a valid machine format is recognized, encode later usage errors in that format on stderr. - Use exit 1 for typed daemon refusals and exit 2 for usage/transport/protocol failures. - Make `error.kind` additive; unknown values must degrade to generic failure. - Keep `short_id` only where roster uniqueness was established. - Rename retarget destinations to `--to-host`/`--to-socket` and remove `felis ssh`. - Correct notification help to match its human default. ## Acceptance criteria - [ ] Every verb is classified as Point, Stream, or Exempt with one terminal rule. - [ ] Machine-mode usage, refusal, transport, and protocol failures have golden outputs and exit codes. - [ ] Point replies never imply unchecked `short_id` uniqueness. - [ ] Help, completions, man pages, CLI reference, schema, `skills/felis`, and changelog are regenerated together.
natsukium commented 2026-09-03 16:46:07 +09:00
Author
Owner
Copy link

CLI args focused review (#55) で、実装時に golden matrix へ含めたい追加確認が3点あった。

  1. felis sessions spawn --format json --json を cold socket で実行すると、stderr は tracing の INFO 行 + JSON error の2行になる。現行の「point failure は exactly one JSON object on stderr」を破るため、machine channel と console log を分離する必要がある。
  2. --format と capture --source は custom value_parser = fn なので clap metadata に possible values がなく、help/generated completion が値候補を列挙できない。contract を確定する際に finite vocabulary を clap にも公開したい。
  3. window retarget --session 1a -- htop、SSH carrier なしの --ssh-arg、payload なしの send は parse 後の usage error。issue 本文の「valid machine format 認識後は同 format」をこの全経路で pin する必要がある。

spawn の command separator 自体は #53 に分離した。

CLI args focused review (#55) で、実装時に golden matrix へ含めたい追加確認が3点あった。 1. `felis sessions spawn --format json --json` を cold socket で実行すると、stderr は tracing の INFO 行 + JSON error の2行になる。現行の「point failure は exactly one JSON object on stderr」を破るため、machine channel と console log を分離する必要がある。 2. `--format` と `capture --source` は custom `value_parser = fn` なので clap metadata に possible values がなく、help/generated completion が値候補を列挙できない。contract を確定する際に finite vocabulary を clap にも公開したい。 3. `window retarget --session 1a -- htop`、SSH carrier なしの `--ssh-arg`、payload なしの `send` は parse 後の usage error。issue 本文の「valid machine format 認識後は同 format」をこの全経路で pin する必要がある。 `spawn` の command separator 自体は #53 に分離した。
natsukium commented 2026-09-03 20:35:13 +09:00
Author
Owner
Copy link

Triage plan (2026-09-03)

Source-grounded triage against main at 69076d42, reviewed through seven rounds of an independent reviewer (pi sol/luna) until it passed with no findings. The dependency order that supersedes the tracker's is posted on #12.

Claim check

Every claim in the body holds at HEAD (69076d42); the three post-snapshot commits changed only the spawn separator and --trace-perf.

  • sessions list is modeled as a stream. SessionOp::List { output: StreamFormat, .. } at crates/felis-cli/src/cli_sessions.rs:129-131; cmd_list emits one SessionObject per item then out.end(count, None) (cli_sessions.rs:940-952). docs/reference/cli.md:77 lists it under Stream; docs/explanation/architecture/control-surfaces.md:311-313 argues "list counts sessions". The bridge already answers sessions.list as a point {"sessions":[…]} (cli.md:710-712), so the CLI is the odd framing of the same roster.
  • Usage errors after a valid machine format are human. Post-parse refusals all eprintln! regardless of --format: reject_global_carrier (cli_sessions.rs:828-838, used from main.rs:373-380, 463-469, 503-509), send with no payload (cli_sessions.rs:577-592), staged_retarget's --ssh-arg without host and --session + -- <cmd> (cli_sessions.rs:868-881). cli.md:44-50 freezes this as "always a human message on stderr".
  • Exit status conflates typed refusal with transport failure. create_refusal maps every non-capacity daemon refusal (invalid_request) to exit 2 (cli_sessions.rs:2247-2259); a daemon Refused terminal on capture/search is exit 2 whatever the reason (cli_sessions.rs:2111, 2383; cli.md:574-578 says an uncompilable --regex is 2); daemon status on an old minor is unsupported at 2 (cli_daemon.rs:61-63). Meanwhile at_capacity is 1 and notifications subscribe splits InvalidRequest to 1 (cli_notifications.rs:71-80). There is no single kind→code table; each site picks.
  • error.kind is documented as closed. cli.md:107-113 ("a closed set a consumer branches on"), cli_output.rs:122-124, and skills/felis/SKILL.md inherit the wording.
  • short_id on rosterless replies. SessionRef::new uses short_id_alone (cli_output.rs:516-525), as do TagResult, SwitchResult (from_short_id/to_short_id), RetargetResult, and NotificationObject.session_short_id (cli_output.rs:527-551, 685-705). cli.md:150-160 documents that these are "not checked against anything". Roster-shortened short_id exists only in SessionObject::new (cli_output.rs:446-449, list/info).
  • Destination flags and felis ssh. RetargetCarrierFlags { host, to_socket } (cli_sessions.rs:735-745) already mixes --host with --to-socket on one verb; the root --host (main.rs:308-309) means "which daemon this invocation dials". Cmd::Ssh (main.rs:145-165) is the alias, promised as "permanent — never removed within a schema major" in main.rs:149-151, cli.md:422-429, control-surfaces.md:437-445, CHANGELOG.md:86. Pre-v0.1.0 the promise is revocable, but the docs record it as a decision, so the explanation twin must carry the reversal.
  • Notification help. Cmd::Notifications help reads "Stream desktop notifications from sessions as JSON lines" (main.rs:83) while the default is human (StreamFormat default cli_output.rs:85-91; cli.md:617-620).

#55's four acceptance gaps, verified:

  1. Log lines on the machine stderr channel. main.rs:357 initializes logging::init(Console::Stderr, None, "info,felis=debug") before any verb; the autospawn path logs info!("daemon socket unreachable; auto-spawning felis-daemon") (crates/felis-client-core/src/spawn.rs:36) and a spawn failure then reaches Dial::open → out.fail(2, DaemonUnreachable) as a JSON object (conn.rs:80-90). So sessions spawn --format json against a socket whose daemon cannot be spawned writes an INFO line and then the error object on stderr, contradicting cli.md:68 ("exactly one typed error object"). dial.rs:292 (warn!) is another emitter. The --json in #55's example is now a clap error after #53 (tests.rs:360), so the reproduction is sessions spawn --format json --socket <path> with felis-daemon unreachable.
  2. No possible values in clap metadata. --format uses value_parser = parse_point_format / parse_stream_format (cli_output.rs:71-77, 85-91); capture --source uses parse_region_source (cli_sessions.rs:265, 469-480). Neither is a ValueEnum, so --help, clap_complete, and clap_mangen show no candidate list.
  3. Post-parse usage paths: enumerated above; all bypass the Reporter.
  4. send --raw with no payload is accepted. raw only selects KeyBytes over Paste for the text payload (cli_sessions.rs:1192); with text: None and --wait/--key the flag is inert (cli_sessions.rs:577-580).

Nothing in the issue is already fixed. One correction: the issue says "remove felis ssh"; the closed #37/#39 comments confirm the decision (rename to --to-host, drop the alias), so this issue owns that cascade, including the reversal of the "permanent alias" record.

Verdict

accept-with-changes. The scope stands; the approach should differ in three places:

  1. Fold as many post-parse usage errors as possible into clap rather than teaching every hand-rolled refusal to speak JSON. Once felis ssh is gone, --ssh-arg can carry requires = "to_host" (the comment at cli_sessions.rs:868-871 names the alias as the only reason it cannot today), --session can conflicts_with = "command", text can be required_unless_present_any = ["wait", "key"], and --raw can requires = "text". That leaves exactly one post-parse usage error (reject_global_carrier, a root-flag/subcommand interaction clap cannot state), and the contract rule becomes one sentence: a refusal clap can state is a clap error (human, exit 2, no framing chosen yet); every refusal after a successful parse is emitted through the verb's class framing.
  2. Make the exit code a function of ErrorKind (one table in cli_output.rs) rather than a per-site literal; that is the "one terminal rule" the acceptance criterion asks for.
  3. Add a usage kind (additive) for the surviving post-parse refusal instead of overloading invalid_request, which the daemon also produces.

Principle check: no capability is added; the change removes ambiguity and silent no-ops. Principle 1 ("no dedicated tool does it better") and 4 ("Explicit over heuristic") pass. The felis ssh removal does not violate the frozen-surface promise because nothing is frozen before the first tag (#12 "Contract freeze boundary").

Approach

A. sessions list becomes a point verb

  • cli_sessions.rs:129-131: output: PointFormat; cmd_list (940-952) builds ListResult { sessions: Vec<SessionObject> } and calls out.result. New serde type in cli_output.rs beside SessionObject; short_id stays roster-shortened against the unfiltered roster (--tag semantics unchanged).
  • plan() (cli_sessions.rs:549-553) uses point_out.
  • Bridge: sessions.list already returns {"sessions":[…]}; make it reuse the same ListResult body via body_value so the two cannot drift.
  • Cascade: cli.md:76-77, 167, 559-561, 710-712; control-surfaces.md:311-313 (the "list counts sessions" argument becomes "list is a bounded roster; a stream implies an unbounded or incremental source; Revisit if the roster ever streams incrementally"); ipc.md:1497, 1552; docs/how-to/reap-sessions.md:28-49 and docs/explanation/input.md:248 (jq 'select(.event | not)' idioms become .sessions[]); skills/felis/SKILL.md:29, 153, 166; tests.rs:264-310 matrix (move list to the point loop); e2e list_prints_one_session_and_exits_zero, a_stream_that_never_opens_still_emits_its_error_terminal (tests/cli_sessions.rs:213, 326 — the latter must move to capture/search).

B. Usage errors in the verb's framing

  • Clap-level moves (all in cli_sessions.rs): --raw requires = "text"; text required_unless_present_any = ["wait", "key"] (delete the hand check at 577-592); RetargetFlags.ssh_arg requires = "to_host"; RetargetFlags.session conflicts_with = "command" (delete both checks in staged_retarget, 868-881, keeping the function for require_switch_from). retarget-all flattens the same structs so it inherits them.
  • Surviving refusal: reject_global_carrier (cli_sessions.rs:828-838) takes a &Reporter and emits out.fail(2, ErrorKind::Usage, …). Call sites: config (main.rs:373-380, point reporter from op's PointFormat), window retarget (main.rs:503-509, flags.output.format), frontend (main.rs:463-469, Exempt → stays human). This needs Cmd::Config/ConfigOp to expose its format before run; a small fn format(&self) -> Format on ConfigOp and RetargetFlags suffices.
  • New ErrorKind::Usage ("usage") in cli_output.rs:125-200; add to the token test at 789-817.
  • Log separation (#55 gap 1): after Cli::parse() in main.rs:342, compute cli.machine_format() (walk the subcommand for a PointFormat/StreamFormat with a machine value) and pass a "off" fallback directive to logging::init when machine; RUST_LOG set explicitly still wins (felis-transport/src/logging.rs:47-49), which is the documented opt-in. Document in cli.md "Machine output": machine mode emits no console log lines unless RUST_LOG is set.
  • Golden matrix e2e (tests/cli_sessions.rs): for each of {clap usage, post-parse usage on window retarget --format json with a global carrier, daemon refusal invalid_request on spawn --rows 9999 --format json, transport daemon_unreachable on a cold socket, protocol (unsupported via a minor-4 fixture daemon if the harness can pin one, else skipped with a note)} assert: exit code, stdout empty or one object, stderr exactly one parseable object (lines().count() == 1). For the autospawn INFO case: copy CARGO_BIN_EXE_felis into a temp dir (sibling lookup misses, spawn.rs:48-53) and run with PATH="" so spawn_daemon_child fails immediately with NotFound; assert stderr is exactly one JSON object.

C. One exit-code table

  • impl ErrorKind { const fn exit_code(self) -> i32 } in cli_output.rs: 1 for no_match, ambiguous, timeout, no_input_owner, no_such_attachment, not_accepted, session_ended, invalid_request, at_capacity, refused; 2 for usage, malformed_request, unsupported, protocol_mismatch, daemon_unreachable, daemon_lost, protocol, canceled, input_failed, internal. Reporter::fail(kind, message) drops the code parameter and returns kind.exit_code(); every out.fail(2|1, …) call (about 60 sites, list from grep -n "\.fail(" crates/felis-cli/src/*.rs) loses its literal.
  • Behavior changes this forces (record in CHANGELOG): spawn invalid_request 2→1 (create_refusal, cli_sessions.rs:2258); capture/search daemon-Refused terminals 2→1 (2111, 2383; cli.md:574-578 rewritten); daemon status unsupported stays 2 (the comment at cli_daemon.rs:61-62 is the rationale; unsupported is a peer-capability class). The grep-shaped exceptions (search no match, notifications --once) and the config/doctor exceptions stay as documented in cli.md:19-42.
  • Bridge: no exit codes per request, but BridgeError should use the same ErrorKind (it already does via as_str); nothing to change beyond the new kind.

D. Additive error.kind

  • cli.md:107-113: replace "closed set" with "a vocabulary that grows additively within an epoch; a consumer treats an unknown kind as a generic failure and reads the exit code". Same sentence in skills/felis/SKILL.md and the bridge paragraph (cli.md:703). control-surfaces.md records the decision (rejected: a closed set, because every new daemon refusal would then need a v bump).

E. short_id only with a roster

  • Remove short_id from SessionRef, TagResult, SwitchResult (from_short_id, to_short_id), RetargetResult, and session_short_id from NotificationObject (cli_output.rs:503-551, 670-705); short_id_alone becomes a human-output helper only (main.rs:605, cli_notifications.rs:66). Keep short_id on SessionObject.
  • Cascade: every {"id":"<hex>","short_id":"…"} shape in cli.md:169-178, 205-221, 332-355, 476; cli.md:150-160 collapses to "only list and info carry short_id"; ipc.md:1489-1500; skills/felis/SKILL.md:62-71, 106, 192; cli_notifications.rs:26-28 help; tests spawn_json_emits_id_object (tests/cli_sessions.rs:1516-1520), tag_json…, kill_and_evict_json…, send_wait_json…, bridge tests asserting short_id, unit test a_rosterless_result_shows_the_floor_length_prefix (cli_output.rs:837-843).

F. --to-host / --to-socket, no felis ssh

  • RetargetCarrierFlags.host → to_host (#[arg(long = "to-host", value_name = "user@host")], conflicts_with = "to_host" on to_socket) at cli_sessions.rs:735-745; delete over_ssh; RetargetArgs.host → to_host (789-820); retarget_from_args (897-919).
  • Delete Cmd::Ssh (main.rs:145-165) and its arm (424-432); retarget() loses the verb parameter's second caller. Root --host (main.rs:301-309) keeps its name and stays root-before-command (not global = true); document that placement as frozen (#55's open question), with the rationale that a post-verb --host would collide in reading with --to-host on the retarget verbs.
  • Completions (cli_completions.rs): the ssh positional and --host-as-destination branches in both retarget target helpers (154-199, 285-327) and the _hosts substitutions (365-366, 402-404) move to --to-host; after #42 the target helper only handles --to-socket, so this shrinks to a rename. Tests 658-705 (the_ssh_alias_*) are deleted; the_ssh_alias_pushes_the_same_retarget_as_window_retarget (tests/cli_sessions.rs:1980) is deleted; zsh_overlay_routes_ssh_destination_and_host_to_hosts_completer retargets to --to-host.
  • Man pages: felis-ssh.1 disappears from __mangen output automatically; the Nix package's install list must not reference it (check nix/ for an explicit page list).
  • Docs: cli.md:401-486 (grammar block, alias paragraph, carrier bullets), cli.md:358, docs/reference/control-surfaces.md retarget row, control-surfaces.md:437-450 (replace the "permanent alias" record with: rejected felis ssh — one destination spelled three ways; Revisit if a launcher-shaped remote verb is ever earned), spec.md:36 (REQ-006b "aliased felis ssh <dest>"), session-lifecycle.md:712, scrollback.md:522, keybindings.md:152, docs/how-to/attach-over-ssh.md:94-137, README.md:39, skills/felis/SKILL.md:356-384, 528, crates/felis-protocol comments mentioning the alias (messages.rs, felis.proto — comments only, no wire change, so no just proto regeneration beyond comment sync), CHANGELOG.md (one "Removed" entry; the old entries at 86, 891 stay as history).

G. Notification help

  • main.rs:83: "Stream desktop notifications from sessions (human lines by default, --format jsonl for objects)". Also cli_notifications.rs:23-34 already says the right thing.

H. Finite vocabularies in clap

  • Replace parse_point_format/parse_stream_format with two ValueEnums (PointFormatValue { Human, Json }, StreamFormatValue { Human, Jsonl }) converting into Format; replace parse_region_source with a ValueEnum SourceArg mapping onto RegionSource. The clap error then lists possible values, which satisfies cli.md:80-83 "naming the value the verb does take"; the unit test at cli_output.rs:723-736 changes to assert the ValueEnum's variants. completions and __mangen pick the values up automatically; add a test that the generated zsh script contains (human json)/(human jsonl) and the four sources.

I. Classification table and generated surfaces

  • cli.md:60-78 gains the terminal rule per class in one column (Point: one object per channel; Stream: exactly one terminal on stdout; Exempt: none) and the sentence "a refusal clap can state is human; a refusal after a successful parse is in the class framing". #35 (post-v0.1) stays the process rule for future verbs.
  • Regenerate together: just check (help snapshots are not stored — consider adding a tests.rs test that asserts Cli::command().render_long_help() contains no felis ssh and the --to-host spelling), felis completions {fish,zsh,bash} (covered by unit tests), __mangen (a test that the page set equals the visible subcommand set), skills/felis, CHANGELOG.md. The config JSON schema is untouched (no config key moves).

Dependencies

  • Landed: #53, #54 (both in HEAD; the --json example in #55 gap 1 must be restated as above).
  • #42 first (S): it deletes the remote branch of the completion overlays, which this issue would otherwise rewrite twice.
  • #24 (retarget op ids) changes RetargetResult/SwitchResult shape; decide its fields before E removes short_id from them, or land #24 first and let this issue apply the short_id rule to its final shape.
  • #20 (spawn to correlated Ops) rewrites the spawn failure path (create_refusal); C's kind→code table should be applied on top of #20's shape, not before it, or the mapping moves twice. Tracker order (#20 in step 5, #23 in step 7) still holds.
  • #17 touches main.rs:347-351 (--version exclusivity); independent but same file — sequence to avoid conflicts.
  • #29 depends on this issue (schemas freeze the shapes this issue changes).

Risk/effort

L. Wide but mechanical: ~60 fail sites, two overlay scripts, and a doc cascade across 15+ files. Main risks: (1) the exit-code retargeting of capture/search/spawn breaks the existing e2e assertions and any external consumer of the pre-release surface (acceptable pre-v0.1.0, but every changed code must land in CHANGELOG.md); (2) the log-suppression in machine mode hides a real diagnostic when autospawn fails — mitigated by keeping RUST_LOG as the override and putting the spawn failure detail into error.message (conn.rs:76-78 already renders the chain); (3) the zsh overlay's substitution anchors (cli_completions.rs:512-535) silently no-op if value names change — the tripwire test at 536-549 must be extended for --to-host's value name.

Labels

Keep priority/P1, release/v0.1.0. It sits inside #12's freeze boundary ("CLI command/option meanings, exit-status classes, and machine JSON epoch 1"); none of it can be done after the tag without a v bump.

## Triage plan (2026-09-03) Source-grounded triage against `main` at `69076d42`, reviewed through seven rounds of an independent reviewer (`pi` sol/luna) until it passed with no findings. The dependency order that supersedes the tracker's is posted on #12. ## Claim check Every claim in the body holds at HEAD (69076d42); the three post-snapshot commits changed only the spawn separator and `--trace-perf`. - **`sessions list` is modeled as a stream.** `SessionOp::List { output: StreamFormat, .. }` at `crates/felis-cli/src/cli_sessions.rs:129-131`; `cmd_list` emits one `SessionObject` per item then `out.end(count, None)` (`cli_sessions.rs:940-952`). `docs/reference/cli.md:77` lists it under Stream; `docs/explanation/architecture/control-surfaces.md:311-313` argues "`list` counts sessions". The bridge already answers `sessions.list` as a point `{"sessions":[…]}` (`cli.md:710-712`), so the CLI is the odd framing of the same roster. - **Usage errors after a valid machine format are human.** Post-parse refusals all `eprintln!` regardless of `--format`: `reject_global_carrier` (`cli_sessions.rs:828-838`, used from `main.rs:373-380, 463-469, 503-509`), `send` with no payload (`cli_sessions.rs:577-592`), `staged_retarget`'s `--ssh-arg` without host and `--session` + `-- <cmd>` (`cli_sessions.rs:868-881`). `cli.md:44-50` freezes this as "always a human message on stderr". - **Exit status conflates typed refusal with transport failure.** `create_refusal` maps every non-capacity daemon refusal (`invalid_request`) to exit 2 (`cli_sessions.rs:2247-2259`); a daemon `Refused` terminal on `capture`/`search` is exit 2 whatever the reason (`cli_sessions.rs:2111, 2383`; `cli.md:574-578` says an uncompilable `--regex` is 2); `daemon status` on an old minor is `unsupported` at 2 (`cli_daemon.rs:61-63`). Meanwhile `at_capacity` is 1 and `notifications subscribe` splits `InvalidRequest` to 1 (`cli_notifications.rs:71-80`). There is no single kind→code table; each site picks. - **`error.kind` is documented as closed.** `cli.md:107-113` ("a closed set a consumer branches on"), `cli_output.rs:122-124`, and `skills/felis/SKILL.md` inherit the wording. - **`short_id` on rosterless replies.** `SessionRef::new` uses `short_id_alone` (`cli_output.rs:516-525`), as do `TagResult`, `SwitchResult` (`from_short_id`/`to_short_id`), `RetargetResult`, and `NotificationObject.session_short_id` (`cli_output.rs:527-551, 685-705`). `cli.md:150-160` documents that these are "not checked against anything". Roster-shortened `short_id` exists only in `SessionObject::new` (`cli_output.rs:446-449`, `list`/`info`). - **Destination flags and `felis ssh`.** `RetargetCarrierFlags { host, to_socket }` (`cli_sessions.rs:735-745`) already mixes `--host` with `--to-socket` on one verb; the root `--host` (`main.rs:308-309`) means "which daemon this invocation dials". `Cmd::Ssh` (`main.rs:145-165`) is the alias, promised as "permanent — never removed within a schema major" in `main.rs:149-151`, `cli.md:422-429`, `control-surfaces.md:437-445`, `CHANGELOG.md:86`. Pre-v0.1.0 the promise is revocable, but the docs record it as a decision, so the explanation twin must carry the reversal. - **Notification help.** `Cmd::Notifications` help reads "Stream desktop notifications from sessions as JSON lines" (`main.rs:83`) while the default is `human` (`StreamFormat` default `cli_output.rs:85-91`; `cli.md:617-620`). #55's four acceptance gaps, verified: 1. **Log lines on the machine stderr channel.** `main.rs:357` initializes `logging::init(Console::Stderr, None, "info,felis=debug")` before any verb; the autospawn path logs `info!("daemon socket unreachable; auto-spawning felis-daemon")` (`crates/felis-client-core/src/spawn.rs:36`) and a spawn failure then reaches `Dial::open` → `out.fail(2, DaemonUnreachable)` as a JSON object (`conn.rs:80-90`). So `sessions spawn --format json` against a socket whose daemon cannot be spawned writes an INFO line and then the error object on stderr, contradicting `cli.md:68` ("exactly one typed error object"). `dial.rs:292` (`warn!`) is another emitter. The `--json` in #55's example is now a clap error after #53 (`tests.rs:360`), so the reproduction is `sessions spawn --format json --socket <path>` with `felis-daemon` unreachable. 2. **No possible values in clap metadata.** `--format` uses `value_parser = parse_point_format` / `parse_stream_format` (`cli_output.rs:71-77, 85-91`); `capture --source` uses `parse_region_source` (`cli_sessions.rs:265, 469-480`). Neither is a `ValueEnum`, so `--help`, `clap_complete`, and `clap_mangen` show no candidate list. 3. **Post-parse usage paths**: enumerated above; all bypass the `Reporter`. 4. **`send --raw` with no payload is accepted.** `raw` only selects `KeyBytes` over `Paste` for the text payload (`cli_sessions.rs:1192`); with `text: None` and `--wait`/`--key` the flag is inert (`cli_sessions.rs:577-580`). Nothing in the issue is already fixed. One correction: the issue says "remove `felis ssh`"; the closed #37/#39 comments confirm the decision (rename to `--to-host`, drop the alias), so this issue owns that cascade, including the reversal of the "permanent alias" record. ## Verdict **accept-with-changes.** The scope stands; the approach should differ in three places: 1. Fold as many post-parse usage errors as possible *into clap* rather than teaching every hand-rolled refusal to speak JSON. Once `felis ssh` is gone, `--ssh-arg` can carry `requires = "to_host"` (the comment at `cli_sessions.rs:868-871` names the alias as the only reason it cannot today), `--session` can `conflicts_with = "command"`, `text` can be `required_unless_present_any = ["wait", "key"]`, and `--raw` can `requires = "text"`. That leaves exactly one post-parse usage error (`reject_global_carrier`, a root-flag/subcommand interaction clap cannot state), and the contract rule becomes one sentence: *a refusal clap can state is a clap error (human, exit 2, no framing chosen yet); every refusal after a successful parse is emitted through the verb's class framing.* 2. Make the exit code a function of `ErrorKind` (one table in `cli_output.rs`) rather than a per-site literal; that is the "one terminal rule" the acceptance criterion asks for. 3. Add a `usage` kind (additive) for the surviving post-parse refusal instead of overloading `invalid_request`, which the daemon also produces. Principle check: no capability is added; the change removes ambiguity and silent no-ops. Principle 1 ("no dedicated tool does it better") and 4 ("Explicit over heuristic") pass. The `felis ssh` removal does not violate the frozen-surface promise because nothing is frozen before the first tag (#12 "Contract freeze boundary"). ## Approach ### A. `sessions list` becomes a point verb - `cli_sessions.rs:129-131`: `output: PointFormat`; `cmd_list` (`940-952`) builds `ListResult { sessions: Vec<SessionObject> }` and calls `out.result`. New serde type in `cli_output.rs` beside `SessionObject`; `short_id` stays roster-shortened against the unfiltered roster (`--tag` semantics unchanged). - `plan()` (`cli_sessions.rs:549-553`) uses `point_out`. - Bridge: `sessions.list` already returns `{"sessions":[…]}`; make it reuse the same `ListResult` body via `body_value` so the two cannot drift. - Cascade: `cli.md:76-77, 167, 559-561, 710-712`; `control-surfaces.md:311-313` (the "`list` counts sessions" argument becomes "list is a bounded roster; a stream implies an unbounded or incremental source; *Revisit if* the roster ever streams incrementally"); `ipc.md:1497, 1552`; `docs/how-to/reap-sessions.md:28-49` and `docs/explanation/input.md:248` (`jq 'select(.event | not)'` idioms become `.sessions[]`); `skills/felis/SKILL.md:29, 153, 166`; `tests.rs:264-310` matrix (move `list` to the point loop); e2e `list_prints_one_session_and_exits_zero`, `a_stream_that_never_opens_still_emits_its_error_terminal` (`tests/cli_sessions.rs:213, 326` — the latter must move to `capture`/`search`). ### B. Usage errors in the verb's framing - Clap-level moves (all in `cli_sessions.rs`): `--raw` `requires = "text"`; `text` `required_unless_present_any = ["wait", "key"]` (delete the hand check at `577-592`); `RetargetFlags.ssh_arg` `requires = "to_host"`; `RetargetFlags.session` `conflicts_with = "command"` (delete both checks in `staged_retarget`, `868-881`, keeping the function for `require_switch_from`). `retarget-all` flattens the same structs so it inherits them. - Surviving refusal: `reject_global_carrier` (`cli_sessions.rs:828-838`) takes a `&Reporter` and emits `out.fail(2, ErrorKind::Usage, …)`. Call sites: `config` (`main.rs:373-380`, point reporter from `op`'s `PointFormat`), `window retarget` (`main.rs:503-509`, `flags.output.format`), `frontend` (`main.rs:463-469`, Exempt → stays human). This needs `Cmd::Config`/`ConfigOp` to expose its format before `run`; a small `fn format(&self) -> Format` on `ConfigOp` and `RetargetFlags` suffices. - New `ErrorKind::Usage` (`"usage"`) in `cli_output.rs:125-200`; add to the token test at `789-817`. - Log separation (#55 gap 1): after `Cli::parse()` in `main.rs:342`, compute `cli.machine_format()` (walk the subcommand for a `PointFormat`/`StreamFormat` with a machine value) and pass a `"off"` fallback directive to `logging::init` when machine; `RUST_LOG` set explicitly still wins (`felis-transport/src/logging.rs:47-49`), which is the documented opt-in. Document in `cli.md` "Machine output": *machine mode emits no console log lines unless `RUST_LOG` is set.* - Golden matrix e2e (`tests/cli_sessions.rs`): for each of {clap usage, post-parse usage on `window retarget --format json` with a global carrier, daemon refusal `invalid_request` on `spawn --rows 9999 --format json`, transport `daemon_unreachable` on a cold socket, protocol (`unsupported` via a minor-4 fixture daemon if the harness can pin one, else skipped with a note)} assert: exit code, stdout empty or one object, stderr exactly one parseable object (`lines().count() == 1`). For the autospawn INFO case: copy `CARGO_BIN_EXE_felis` into a temp dir (sibling lookup misses, `spawn.rs:48-53`) and run with `PATH=""` so `spawn_daemon_child` fails immediately with NotFound; assert stderr is exactly one JSON object. ### C. One exit-code table - `impl ErrorKind { const fn exit_code(self) -> i32 }` in `cli_output.rs`: 1 for `no_match, ambiguous, timeout, no_input_owner, no_such_attachment, not_accepted, session_ended, invalid_request, at_capacity, refused`; 2 for `usage, malformed_request, unsupported, protocol_mismatch, daemon_unreachable, daemon_lost, protocol, canceled, input_failed, internal`. `Reporter::fail(kind, message)` drops the `code` parameter and returns `kind.exit_code()`; every `out.fail(2|1, …)` call (about 60 sites, list from `grep -n "\.fail(" crates/felis-cli/src/*.rs`) loses its literal. - Behavior changes this forces (record in CHANGELOG): `spawn` `invalid_request` 2→1 (`create_refusal`, `cli_sessions.rs:2258`); `capture`/`search` daemon-`Refused` terminals 2→1 (`2111, 2383`; `cli.md:574-578` rewritten); `daemon status` `unsupported` stays 2 (the comment at `cli_daemon.rs:61-62` is the rationale; `unsupported` is a peer-capability class). The grep-shaped exceptions (`search` no match, `notifications --once`) and the `config`/`doctor` exceptions stay as documented in `cli.md:19-42`. - Bridge: no exit codes per request, but `BridgeError` should use the same `ErrorKind` (it already does via `as_str`); nothing to change beyond the new kind. ### D. Additive `error.kind` - `cli.md:107-113`: replace "closed set" with "a vocabulary that grows additively within an epoch; a consumer treats an unknown `kind` as a generic failure and reads the exit code". Same sentence in `skills/felis/SKILL.md` and the bridge paragraph (`cli.md:703`). `control-surfaces.md` records the decision (rejected: a closed set, because every new daemon refusal would then need a `v` bump). ### E. `short_id` only with a roster - Remove `short_id` from `SessionRef`, `TagResult`, `SwitchResult` (`from_short_id`, `to_short_id`), `RetargetResult`, and `session_short_id` from `NotificationObject` (`cli_output.rs:503-551, 670-705`); `short_id_alone` becomes a human-output helper only (`main.rs:605`, `cli_notifications.rs:66`). Keep `short_id` on `SessionObject`. - Cascade: every `{"id":"<hex>","short_id":"…"}` shape in `cli.md:169-178, 205-221, 332-355, 476`; `cli.md:150-160` collapses to "only `list` and `info` carry `short_id`"; `ipc.md:1489-1500`; `skills/felis/SKILL.md:62-71, 106, 192`; `cli_notifications.rs:26-28` help; tests `spawn_json_emits_id_object` (`tests/cli_sessions.rs:1516-1520`), `tag_json…`, `kill_and_evict_json…`, `send_wait_json…`, bridge tests asserting `short_id`, unit test `a_rosterless_result_shows_the_floor_length_prefix` (`cli_output.rs:837-843`). ### F. `--to-host` / `--to-socket`, no `felis ssh` - `RetargetCarrierFlags.host` → `to_host` (`#[arg(long = "to-host", value_name = "user@host")]`, `conflicts_with = "to_host"` on `to_socket`) at `cli_sessions.rs:735-745`; delete `over_ssh`; `RetargetArgs.host` → `to_host` (`789-820`); `retarget_from_args` (`897-919`). - Delete `Cmd::Ssh` (`main.rs:145-165`) and its arm (`424-432`); `retarget()` loses the `verb` parameter's second caller. Root `--host` (`main.rs:301-309`) keeps its name and stays root-before-command (not `global = true`); document that placement as frozen (#55's open question), with the rationale that a post-verb `--host` would collide in reading with `--to-host` on the retarget verbs. - Completions (`cli_completions.rs`): the `ssh` positional and `--host`-as-destination branches in both retarget target helpers (`154-199, 285-327`) and the `_hosts` substitutions (`365-366`, `402-404`) move to `--to-host`; after #42 the target helper only handles `--to-socket`, so this shrinks to a rename. Tests `658-705` (`the_ssh_alias_*`) are deleted; `the_ssh_alias_pushes_the_same_retarget_as_window_retarget` (`tests/cli_sessions.rs:1980`) is deleted; `zsh_overlay_routes_ssh_destination_and_host_to_hosts_completer` retargets to `--to-host`. - Man pages: `felis-ssh.1` disappears from `__mangen` output automatically; the Nix package's install list must not reference it (check `nix/` for an explicit page list). - Docs: `cli.md:401-486` (grammar block, alias paragraph, carrier bullets), `cli.md:358`, `docs/reference/control-surfaces.md` retarget row, `control-surfaces.md:437-450` (replace the "permanent alias" record with: rejected `felis ssh` — one destination spelled three ways; *Revisit if* a launcher-shaped remote verb is ever earned), `spec.md:36` (REQ-006b "aliased `felis ssh <dest>`"), `session-lifecycle.md:712`, `scrollback.md:522`, `keybindings.md:152`, `docs/how-to/attach-over-ssh.md:94-137`, `README.md:39`, `skills/felis/SKILL.md:356-384, 528`, `crates/felis-protocol` comments mentioning the alias (`messages.rs`, `felis.proto` — comments only, no wire change, so no `just proto` regeneration beyond comment sync), `CHANGELOG.md` (one "Removed" entry; the old entries at 86, 891 stay as history). ### G. Notification help - `main.rs:83`: "Stream desktop notifications from sessions (human lines by default, `--format jsonl` for objects)". Also `cli_notifications.rs:23-34` already says the right thing. ### H. Finite vocabularies in clap - Replace `parse_point_format`/`parse_stream_format` with two `ValueEnum`s (`PointFormatValue { Human, Json }`, `StreamFormatValue { Human, Jsonl }`) converting into `Format`; replace `parse_region_source` with a `ValueEnum` `SourceArg` mapping onto `RegionSource`. The clap error then lists possible values, which satisfies `cli.md:80-83` "naming the value the verb does take"; the unit test at `cli_output.rs:723-736` changes to assert the ValueEnum's variants. `completions` and `__mangen` pick the values up automatically; add a test that the generated zsh script contains `(human json)`/`(human jsonl)` and the four sources. ### I. Classification table and generated surfaces - `cli.md:60-78` gains the terminal rule per class in one column (Point: one object per channel; Stream: exactly one terminal on stdout; Exempt: none) and the sentence "a refusal clap can state is human; a refusal after a successful parse is in the class framing". `#35` (post-v0.1) stays the process rule for future verbs. - Regenerate together: `just check` (help snapshots are not stored — consider adding a `tests.rs` test that asserts `Cli::command().render_long_help()` contains no `felis ssh` and the `--to-host` spelling), `felis completions {fish,zsh,bash}` (covered by unit tests), `__mangen` (a test that the page set equals the visible subcommand set), `skills/felis`, `CHANGELOG.md`. The config JSON schema is untouched (no config key moves). ## Dependencies - Landed: #53, #54 (both in HEAD; the `--json` example in #55 gap 1 must be restated as above). - **#42 first** (S): it deletes the remote branch of the completion overlays, which this issue would otherwise rewrite twice. - **#24** (retarget op ids) changes `RetargetResult`/`SwitchResult` shape; decide its fields before E removes `short_id` from them, or land #24 first and let this issue apply the `short_id` rule to its final shape. - **#20** (spawn to correlated Ops) rewrites the spawn failure path (`create_refusal`); C's kind→code table should be applied on top of #20's shape, not before it, or the mapping moves twice. Tracker order (#20 in step 5, #23 in step 7) still holds. - **#17** touches `main.rs:347-351` (`--version` exclusivity); independent but same file — sequence to avoid conflicts. - **#29** depends on this issue (schemas freeze the shapes this issue changes). ## Risk/effort **L.** Wide but mechanical: ~60 `fail` sites, two overlay scripts, and a doc cascade across 15+ files. Main risks: (1) the exit-code retargeting of `capture`/`search`/`spawn` breaks the existing e2e assertions and any external consumer of the pre-release surface (acceptable pre-v0.1.0, but every changed code must land in `CHANGELOG.md`); (2) the log-suppression in machine mode hides a real diagnostic when autospawn fails — mitigated by keeping `RUST_LOG` as the override and putting the spawn failure detail into `error.message` (`conn.rs:76-78` already renders the chain); (3) the zsh overlay's substitution anchors (`cli_completions.rs:512-535`) silently no-op if value names change — the tripwire test at `536-549` must be extended for `--to-host`'s value name. ## Labels Keep **priority/P1**, **release/v0.1.0**. It sits inside #12's freeze boundary ("CLI command/option meanings, exit-status classes, and machine JSON epoch 1"); none of it can be done after the tag without a `v` bump.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
ci/darwin-smoke
No results found.
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
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#23
No description provided.