feat(cli): stdin json protocol and strict exit code semantics #74

Merged

yuris merged 1 commit from feat/issue-32-protocol into main 2026-09-23 04:05:52 +00:00

Conversation 1 Commits 1 Files changed 9 +1350 -367

yuris commented 2026-09-23 03:59:34 +00:00

Owner

Copy link

Motivation

Agents drive the Life service through life … --json in a child process, so the
process boundary is the interface. Three things made it unsafe:

• a structured body could be passed in the argument list — a task title, a
  private reference, a credential — where every process on the host can read it
  and shell history retains it;
• the exit code was computed from the envelope status alone, so a 202 or a lost
  answer could not be told from success, and an error envelope whose code the map
  did not carry collapsed to exit 1;
• the exit-code map did not cover every code the service emits:
  clarification_required, operation_unknown, needs_reconciliation,
  partial_apply, context_pending and limit_exceeded all fell through to
  exit 1, so an agent could not branch on them.

Issue #32 asks for the documented transport contract to be real rather than
aspirational: --input - for every body-taking command, one envelope on stdout
with diagnostics on stderr, and exit codes that mean what
docs/technical/cli-api.md says.

Changes

Stdin is the structured-input path (src/cli.rs)

• read_input is bounded: stdin or --input FILE is read up to 1 MiB and
  refused beyond it with invalid_input, exit 2, before anything is dispatched.
• one shared reader pair — read_body (a body is required) and
  read_optional_body (an absent body is {}) — replaces the ten hand-rolled
  copies. A body that is empty, malformed or not a JSON object is exit 2 with
  nothing dispatched, uniformly.
• every state-changing command routes through one operation_key helper: a
  caller's --operation-id is used verbatim, and an absent one is generated
  (high entropy) before transmission.

No payload and no credential in argv

• --token and LIFE_TOKEN are removed. The bearer token is read only from the
  secret file the client configuration names, which is what the skill and
  operations-security.md already documented; Endpoint::resolve no longer
  accepts a token at all.
• task add / event add no longer accept --title / --collection-id.
  Those were undocumented shortcuts that put private payload fields in the
  argument list; the body is the only way, and passing them is a usage error
  (exit 2).

Exit codes: semantic outcome before transport (src/client.rs)

• exit_code_for(envelope, http_status) decides on the envelope's own outcome
  first: partial → 6 even when a step carried etag_conflict,
  needs_clarification → 3, ok arriving as 202 → 6. Only an error envelope
  whose code the map does not carry falls back to the documented HTTP mapping,
  and an error envelope is never exit 0.
• the map is total over every ApiFailure constructor and every code a route
  writes into an envelope.
• a transmitted write that times out — or whose answer cannot be read as an
  envelope — is status: partial / error.code: operation_unknown with exit 6,
  the operation ID the client already knows, and the client-generated request ID
  the request carried as X-Request-ID. A server request ID is never invented.
  request_pending is gone: it was not in the documented vocabulary.

Channel separation

• in the human (non---json) path, warnings and the incomplete-coverage notice
  moved from stdout to stderr, so stdout carries the result and stderr the
  diagnostics in both modes.

The skill stays true (skills/life/references/)

• errors.md: limit_exceeded is exit 2 (it was documented as a known gap in
  the client's map), new rows for configuration_invalid,
  clarification_required and needs_reconciliation, and a timeout now reports
  operation_unknown.
• commands.md: no --token; the 1 MiB bound and the JSON-object rule;
  life event cancel moved to the not-implemented table — it parses but has no
  route in this client and exits 5, which the Writes table previously
  contradicted.

Verification

• cargo test — 714 tests across every suite, all green (rebased on
  main at the append-only timeline corrections merge).
• cargo clippy --all-targets -- -D warnings — clean; cargo fmt --check —
  clean.
• New tests/cli_protocol_tests.rs (10 tests, all through the real binary):
  • the exit map checked against every ApiFailure constructor twice — through
    the code map and through the envelope at its own HTTP status — plus the
    route-level codes;
  • semantic-outcome-over-HTTP cases (partial + etag_conflict at 412 → 6,
    ok at 202 → 6, unmapped code at 200 → 1, never 0);
  • all 15 body-taking commands read stdin, refuse a malformed body with exit 2
    and nothing dispatched, and read the same body from a file;
  • a body over 1 MiB is refused with exit 2 and never dispatched, while a body
    just under it is dispatched;
  • one envelope on stdout for twelve outcome classes, and the human path keeps
    warnings and the partial-coverage notice on stderr;
  • the running process's argument list, read from the process table, contains
    neither the secret-file token nor the stdin body;
  • Idempotency-Key is sent verbatim by 13 state-changing commands and by none
    of the read-only POSTs (resolve, next, context search).
• Updated expectations that pinned the old behaviour:
  tests/cli_creation_tests.rs (timeout envelope),
  tests/crash_recovery_tests.rs (read timeout envelope),
  tests/update_tests.rs / tests/skill_package_tests.rs (the resolver now
  takes the HTTP status).

Related Issues

Closes #32

## Motivation Agents drive the Life service through `life … --json` in a child process, so the process boundary *is* the interface. Three things made it unsafe: - a structured body could be passed in the argument list — a task title, a private reference, a credential — where every process on the host can read it and shell history retains it; - the exit code was computed from the envelope status alone, so a 202 or a lost answer could not be told from success, and an error envelope whose code the map did not carry collapsed to exit 1; - the exit-code map did not cover every code the service emits: `clarification_required`, `operation_unknown`, `needs_reconciliation`, `partial_apply`, `context_pending` and `limit_exceeded` all fell through to exit 1, so an agent could not branch on them. Issue #32 asks for the documented transport contract to be real rather than aspirational: `--input -` for every body-taking command, one envelope on stdout with diagnostics on stderr, and exit codes that mean what `docs/technical/cli-api.md` says. ## Changes ### Stdin is the structured-input path (`src/cli.rs`) - `read_input` is bounded: stdin or `--input FILE` is read up to 1 MiB and refused beyond it with `invalid_input`, exit 2, before anything is dispatched. - one shared reader pair — `read_body` (a body is required) and `read_optional_body` (an absent body is `{}`) — replaces the ten hand-rolled copies. A body that is empty, malformed or not a JSON object is exit 2 with nothing dispatched, uniformly. - every state-changing command routes through one `operation_key` helper: a caller's `--operation-id` is used verbatim, and an absent one is generated (high entropy) before transmission. ### No payload and no credential in argv - `--token` and `LIFE_TOKEN` are removed. The bearer token is read only from the secret file the client configuration names, which is what the skill and `operations-security.md` already documented; `Endpoint::resolve` no longer accepts a token at all. - `task add` / `event add` no longer accept `--title` / `--collection-id`. Those were undocumented shortcuts that put private payload fields in the argument list; the body is the only way, and passing them is a usage error (exit 2). ### Exit codes: semantic outcome before transport (`src/client.rs`) - `exit_code_for(envelope, http_status)` decides on the envelope's own outcome first: `partial` → 6 even when a step carried `etag_conflict`, `needs_clarification` → 3, `ok` arriving as 202 → 6. Only an `error` envelope whose code the map does not carry falls back to the documented HTTP mapping, and an error envelope is never exit 0. - the map is total over every `ApiFailure` constructor and every code a route writes into an envelope. - a transmitted write that times out — or whose answer cannot be read as an envelope — is `status: partial` / `error.code: operation_unknown` with exit 6, the operation ID the client already knows, and the client-generated request ID the request carried as `X-Request-ID`. A server request ID is never invented. `request_pending` is gone: it was not in the documented vocabulary. ### Channel separation - in the human (non-`--json`) path, warnings and the incomplete-coverage notice moved from stdout to stderr, so stdout carries the result and stderr the diagnostics in both modes. ### The skill stays true (`skills/life/references/`) - `errors.md`: `limit_exceeded` is exit 2 (it was documented as a known gap in the client's map), new rows for `configuration_invalid`, `clarification_required` and `needs_reconciliation`, and a timeout now reports `operation_unknown`. - `commands.md`: no `--token`; the 1 MiB bound and the JSON-object rule; `life event cancel` moved to the not-implemented table — it parses but has no route in this client and exits 5, which the Writes table previously contradicted. ## Verification - `cargo test` — 714 tests across every suite, all green (rebased on `main` at the append-only timeline corrections merge). - `cargo clippy --all-targets -- -D warnings` — clean; `cargo fmt --check` — clean. - New `tests/cli_protocol_tests.rs` (10 tests, all through the real binary): - the exit map checked against every `ApiFailure` constructor twice — through the code map and through the envelope at its own HTTP status — plus the route-level codes; - semantic-outcome-over-HTTP cases (partial + `etag_conflict` at 412 → 6, `ok` at 202 → 6, unmapped code at 200 → 1, never 0); - all 15 body-taking commands read stdin, refuse a malformed body with exit 2 and nothing dispatched, and read the same body from a file; - a body over 1 MiB is refused with exit 2 and never dispatched, while a body just under it is dispatched; - one envelope on stdout for twelve outcome classes, and the human path keeps warnings and the partial-coverage notice on stderr; - the running process's argument list, read from the process table, contains neither the secret-file token nor the stdin body; - `Idempotency-Key` is sent verbatim by 13 state-changing commands and by none of the read-only POSTs (`resolve`, `next`, `context search`). - Updated expectations that pinned the old behaviour: `tests/cli_creation_tests.rs` (timeout envelope), `tests/crash_recovery_tests.rs` (read timeout envelope), `tests/update_tests.rs` / `tests/skill_package_tests.rs` (the resolver now takes the HTTP status). ## Related Issues Closes #32

yuris added 1 commit 2026-09-23 03:59:34 +00:00

feat(cli): stdin json protocol and strict exit code semantics 6c28e00ed6

yuris force-pushed feat/issue-32-protocol from 6c28e00ed6 to 64875503c7 2026-09-23 04:03:41 +00:00 Compare

yuris commented 2026-09-23 04:05:50 +00:00

Author

Owner

Copy link

Coordinator review — APPROVED

Verified 6487550 myself: cargo test 714 passed / 0 failed (10 new protocol tests), cargo clippy --all-targets -- -D warnings 0 errors, cargo fmt --check clean, and --token/LIFE_TOKEN are genuinely gone from the CLI (grep -c → 0).

This was the right issue to run as an audit, and the findings are real defects rather than cosmetic ones. The three that mattered most:

1. Exit-code gaps. clarification_required, operation_unknown, needs_reconciliation, partial_apply, context_pending and limit_exceeded all fell through to exit 1. A caller branching on exit codes — which is exactly what the shipped skill tells an agent to do — would have read a clarification as a generic failure and a partial apply as a plain error. The map is now total over every ApiFailure constructor plus the route-level codes, and the_exit_code_map_covers_every_code_the_service_can_emit enforces totality rather than sampling.
2. The timeout envelope was non-normative. It returned status: error with a code (request_pending) the contract does not define. It now returns status: partial + operation_unknown + data{operation_id, operation_state: unknown} at exit 6 — matching the normative envelope in verification.md — and a write whose answer is not an envelope is an unknown outcome rather than exit 1. That is the difference between an agent stopping to inspect an operation and an agent assuming a failure.
3. Credentials could reach argv/env. Removing --token/LIFE_TOKEN so the token comes only from the configured secret file closes a leak the log-redaction work in #35 could not cover, since ps is not a log. Dropping the undocumented --title/--collection-id argv payloads is the same class of fix.

Also verified as correct: exit_code_for is now status-aware (ok @202 → 6, and an unmapped error falls back to the documented HTTP map rather than 0), the human path no longer prints warnings to stdout, stdin bodies are bounded at 1 MiB, ten hand-rolled body readers and eight operation-key generators collapsed into shared helpers, and the skill's incorrect limit_exceeded/request_pending/event cancel entries were corrected in the same commit.

One gap you correctly documented instead of hiding: life event cancel has no EventSubcommands::Cancel and no POST /v1/events/{id}/cancel route, although cli-api.md's command-to-route table lists it. Moving it to the skill's not-implemented table is honest, but the contract still advertises it — I am filing that as a follow-up so it gets implemented rather than silently dropped from the docs.

Merging.

## Coordinator review — APPROVED Verified `6487550` myself: `cargo test` **714 passed / 0 failed** (10 new protocol tests), `cargo clippy --all-targets -- -D warnings` 0 errors, `cargo fmt --check` clean, and `--token`/`LIFE_TOKEN` are genuinely gone from the CLI (`grep -c` → 0). This was the right issue to run as an **audit**, and the findings are real defects rather than cosmetic ones. The three that mattered most: 1. **Exit-code gaps.** `clarification_required`, `operation_unknown`, `needs_reconciliation`, `partial_apply`, `context_pending` and `limit_exceeded` all fell through to exit 1. A caller branching on exit codes — which is exactly what the shipped skill tells an agent to do — would have read a *clarification* as a generic failure and a *partial apply* as a plain error. The map is now total over every `ApiFailure` constructor plus the route-level codes, and `the_exit_code_map_covers_every_code_the_service_can_emit` enforces totality rather than sampling. 2. **The timeout envelope was non-normative.** It returned `status: error` with a code (`request_pending`) the contract does not define. It now returns `status: partial` + `operation_unknown` + `data{operation_id, operation_state: unknown}` at exit 6 — matching the normative envelope in `verification.md` — and a write whose answer is not an envelope is an unknown outcome rather than exit 1. That is the difference between an agent stopping to inspect an operation and an agent assuming a failure. 3. **Credentials could reach `argv`/env.** Removing `--token`/`LIFE_TOKEN` so the token comes only from the configured secret file closes a leak the log-redaction work in #35 could not cover, since `ps` is not a log. Dropping the undocumented `--title`/`--collection-id` argv payloads is the same class of fix. Also verified as correct: `exit_code_for` is now status-aware (`ok` @202 → 6, and an unmapped error falls back to the documented HTTP map rather than 0), the human path no longer prints warnings to stdout, stdin bodies are bounded at 1 MiB, ten hand-rolled body readers and eight operation-key generators collapsed into shared helpers, and the skill's incorrect `limit_exceeded`/`request_pending`/`event cancel` entries were corrected in the same commit. **One gap you correctly documented instead of hiding:** `life event cancel` has no `EventSubcommands::Cancel` and no `POST /v1/events/{id}/cancel` route, although `cli-api.md`'s command-to-route table lists it. Moving it to the skill's not-implemented table is honest, but the contract still advertises it — I am filing that as a follow-up so it gets implemented rather than silently dropped from the docs. Merging.

yuris referenced this pull request 2026-09-23 04:05:50 +00:00

Implement life event cancel / POST /v1/events/{id}/cancel #75

yuris merged commit 763b2d3f44 into main 2026-09-23 04:05:52 +00:00

yuris referenced this pull request from a commit 2026-09-23 04:05:52 +00:00

feat(cli): stdin json protocol and strict exit code semantics (#74)

yuris changed title from feat(cli): stdin json protocol and strict exit code semantics (#32) to feat(cli): stdin json protocol and strict exit code semantics 2026-09-23 05:11:58 +00:00

yuris referenced this pull request from a commit 2026-09-23 11:54:43 +00:00

feat(cli): stdin json protocol and strict exit code semantics (#74)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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

yuris/life-service!74

No description provided.