From 932cdef3c1fbf20b245e3eb06027e00f7a81c79a Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 19 May 2026 21:07:12 +0100 Subject: [PATCH 1/7] docs(cli): add issue spec for global CLI output contract ADR (#1798) --- .../1798-global-cli-output-contract-adr.md | 414 ++++++++++++++++++ 1 file changed, 414 insertions(+) create mode 100644 docs/issues/open/1798-global-cli-output-contract-adr.md diff --git a/docs/issues/open/1798-global-cli-output-contract-adr.md b/docs/issues/open/1798-global-cli-output-contract-adr.md new file mode 100644 index 000000000..eeb6a304d --- /dev/null +++ b/docs/issues/open/1798-global-cli-output-contract-adr.md @@ -0,0 +1,414 @@ +--- +doc-type: issue +issue-type: task +status: open +priority: p2 +github-issue: 1798 +spec-path: docs/issues/open/1798-global-cli-output-contract-adr.md +branch: 1798-global-cli-output-contract-adr +related-pr: null +last-updated-utc: 2026-05-19 14:00 +semantic-links: + skill-links: + - create-issue + related-artifacts: + - docs/adrs/ + - console/tracker-client/docs/adrs/20260512080000_define_tracker_cli_io_contract_and_error_handling.md + - console/tracker-client/docs/contracts/tracker-cli-io-contract.md +--- + + + +# Issue #1798 - Define a Global CLI Output Contract for the Tracker (ADR) + +## Goal + +Write a repository-wide Architectural Decision Record (ADR) that establishes a single, canonical +command-line output contract for every first-party, operator-facing CLI entrypoint in the +`torrust-tracker` repository, aligning with the approach adopted by `torrust-index` +(ADR-T-010) and reflecting the reality that AI agents are the dominant CLI consumers today. + +**This ADR is prescriptive.** The current codebase does not yet comply with the rules it +establishes. Existing binaries will be migrated progressively in a separate follow-up issue. +The ADR must include a migration policy section so the gap between target state and current +state is documented, expected, and not treated as a defect. + +## Background + +### Existing partial contracts + +The tracker already has a local CLI I/O contract, but it is scoped only to +`console/tracker-client`: + +- `console/tracker-client/docs/adrs/20260512080000_define_tracker_cli_io_contract_and_error_handling.md` + (status: Accepted) — defines JSON default, stdout/stderr channel split, exit codes 0/1/2, and + NDJSON progress for monitor-style commands. +- `console/tracker-client/docs/contracts/tracker-cli-io-contract.md` — the normative companion + contract document. + +That local contract was deliberately scoped to the tracker-client because it was expected to be +extracted into its own repository. However, other binaries in the tracker repo +(`http_health_check`, `e2e_tests_runner`, `profiling`, `qbittorrent_e2e_runner`, the server +`main` binary) have no governing output contract at all. + +### What the Torrust Index decided + +`torrust-index` adopted ADR-T-010 ("Global Command-Line Output Contract", decided and +implemented 2026-05-13). Key rules: + +1. **Both streams are machine-readable.** Plain human-readable text is not a valid output format + on either stdout or stderr. +2. **Stdout = result data.** Commands that produce result data emit exactly one JSON object + followed by a trailing newline. On failure, stdout is empty. +3. **Stderr = diagnostics.** Logs, progress, help, usage errors, and panic records all go to + stderr as JSON (NDJSON when multiple records arrive over time). `tracing` is the diagnostic + writer. +4. **TTY refusal.** Commands with stdout result data refuse to run when stdout is attached to a + terminal. They exit with code 2 and emit a JSON diagnostic on stderr. This rule is + unconditional — it does not depend on payload sensitivity. +5. **Exit codes.** Baseline: `0` success, `1` runtime/internal failure, `2` usage/TTY/argv + failure. Command-specific codes may extend this baseline. +6. **Shared Rust infrastructure.** A dedicated package (`packages/index-cli-common`) provides + the shared scaffolding: JSON clap parser, JSON panic hook, JSON tracing setup, TTY refusal + helper, stdout emitter, and workspace-level `clippy::print_stdout` / `clippy::print_stderr` + denials. +7. **Redaction policy.** Secrets (DB URLs with credentials, JWT secrets, API keys, etc.) must + not appear in JSON diagnostic output. + +### The Deployer research + +Earlier research in `torrust-tracker-deployer` explored separating user-friendly progress output +from internal tracing logs, with verbosity levels (`-q`, `-v`, `-vv`, `-vvv`). That research +treated JSON as a machine-mode option rather than the default and assumed human operators as the +primary audience. The format assumption is superseded here — JSON is always the format — but the +**concept of user-facing output verbosity levels remains useful** and should not be discarded. + +However, two distinct concerns must not be conflated: + +- **Internal tracing / logging levels** (`TRACE`, `DEBUG`, `INFO`, `WARN`, `ERROR`) are + standard, well-defined levels for developer and operations observability. They are controlled + by `RUST_LOG` or a `--debug` / `--log-level` flag and feed the `tracing` subscriber. These + levels govern what the application emits about its own internal behaviour and are not + user-facing output levels. +- **User-facing output verbosity levels** govern how much information a command surfaces to + its caller (human or agent) about progress, intermediate results, and the final outcome. These + levels are application-specific and depend on what data is meaningful to expose, whether the + command produces a single final result or a stream of progress events, etc. They are a + separate knob from internal log levels. + +Both internal tracing records and user-facing progress events can land on stderr and overlap on +the same channel. NDJSON makes this manageable: each line is a self-contained JSON object with a +`kind` or `type` field, so callers can filter by record type regardless of interleaving. Users +can also redirect each concern independently at runtime — for example, send internal tracing to a +log file only while keeping user-facing progress events visible on stderr, or vice versa. + +For the ADR, the number of user-facing verbosity levels should be kept to what is practically +useful for the commands in scope. A richer scheme is only worth the extra API surface if the +distinctions genuinely help callers make different decisions based on the level. + +The key change from the Deployer approach is that all output at every verbosity level — both +internal tracing and user-facing — is JSON-formatted. There is no parallel plain-text output +path. + +### Why this matters now + +The primary consumers of CLI output for the tracker project are increasingly AI agents and +automation scripts, not humans reading a terminal in real time. This changes the calculus: + +- **JSON should be the default, always.** There is no practical benefit to plain-text output when + the primary consumer is an agent or script. +- **Clean stdout is critical.** Diagnostic noise mixed into result data breaks automated parsing. + The separation of result data (stdout) from diagnostics (stderr) must be enforced mechanically, + not just by convention. +- **User-facing verbosity levels are useful but must not be conflated with internal log levels.** + Both are independent knobs. Internal tracing log levels (`RUST_LOG`) control observability for + developers and ops; user-facing verbosity levels control how much progress and result detail a + command surfaces to its caller. Both can appear on stderr as NDJSON and can be separated by + record type or redirected independently via configuration. All output at every level must emit + JSON, not plain text. +- **AI agents reusing terminals need explicit per-command output capture.** When an agent drives + multiple commands in the same terminal session, terminal buffer sharing causes output to be + mis-attributed or partially captured. The recommended pattern is per-command file redirection + (`> .tmp/.stdout 2> .tmp/.stderr`). Because the contract enforces JSON on both + channels, the captured files are always well-formed and parseable without ambiguity. + +### The TTY refusal question + +TTY refusal is the most controversial rule from ADR-T-010. The rule says: if a command has +stdout result data and stdout is attached to a terminal, the command refuses to run and exits 2. +Operators can inspect output by piping to `jq`, `less`, or `cat`. + +**Arguments in favour:** + +- It enforces the contract mechanically. A developer cannot accidentally run a stdout-producing + command interactively and see raw JSON scrolling past without realizing the output is not + captured. +- For AI agents, it prevents them from driving a command in a pseudo-terminal and seeing + terminal-formatted or partially buffered output that breaks JSON parsing. +- It removes the temptation to add ANSI color codes or human-friendly text to stdout result + data "just for interactive use". The contract stays clean. +- Example: `http_health_check` emitting `{"status":"healthy","elapsed_ms":12}` — if run in a + terminal it should refuse and tell the operator to pipe it. `http_health_check | jq .` works + fine and gives a pretty-printed result. +- Example: a future `tracker-client announce` command that returns a peers list — the JSON output + is meant for scripts. TTY refusal prevents accidental interactive use and makes the expectation + explicit. + +**Arguments against / open questions:** + +- Developer experience friction: running `tracker-client udp announce --url udp://localhost:6969` + during local debugging is more cumbersome if you must always pipe to `cat`. +- Commands with no stdout result data (e.g. the server `main` binary, `e2e_tests_runner`, + `profiling`) are unaffected — TTY refusal only applies to commands that emit stdout result data. + Many tracker binaries may fall in the no-stdout-result-data class, which would make the rule + largely moot for the most commonly interactive binaries. +- Is there a middle ground, e.g. a `--allow-tty` flag? The Index ADR deliberately rejects this + because it re-introduces the "two modes" complexity. This needs a concrete decision here. + +**Decision: adopted.** TTY refusal is adopted as stated, unconditionally, for all commands +that emit stdout result data. The ADR must record this decision with the full rationale above. + +### AI agent terminal output capture + +A related concern arises specifically when AI agents drive CLI commands. Agents such as GitHub +Copilot reuse a single persistent terminal session across multiple commands to avoid spawning +extra processes. This creates a capture problem: + +- The agent may receive **partial output** if the terminal buffer is read before the command + finishes. +- Output from **multiple commands** may be interleaved in the same buffer, causing the agent to + attribute the wrong output to the wrong command. +- **User-interleaved input** — a user typing additional commands in the same terminal session — + is invisible to the agent and silently corrupts the captured output. + +The recommended mitigation is for agents to **redirect each command's output to independent +files**, even when commands share the same terminal: + +```sh +my-command > .tmp/my-command.stdout 2> .tmp/my-command.stderr +``` + +The agent then reads the file to obtain the exact, unambiguous output for that command. The +`.tmp/` directory (workspace-local, git-ignored) is the recommended location because: + +1. It is inside the workspace, so the user has a well-known, accessible record of every command + the agent executed and its output — not buried in agent-internal storage. +2. It is git-ignored, so captured output does not accidentally enter version control. +3. It follows the established convention in this repository (see `TORRUST_GIT_HOOKS_LOG_DIR=.tmp` + in `AGENTS.md`). + +Using **two separate files per command** (one for stdout, one for stderr) preserves the +channel split that the output contract depends on. This is only unambiguous because the contract +mandates JSON on both channels — a mixed plain-text/JSON scheme would make file-based capture +unreliable. The ADR should include this as a recommended practice for agents driving tracker +CLI commands. + +### Relationship to the tracker-client local ADR + +The local tracker-client ADR and contract document are consistent with the direction proposed +here but are narrower in scope. The decision on disposition is: + +- The global ADR **supersedes and deprecates** the local tracker-client ADR + (`20260512080000_define_tracker_cli_io_contract_and_error_handling.md`) and its companion + contract document. Once the global ADR is accepted, the local ADR is marked as superseded + and the local contract document becomes a tracker-client–specific supplement (covering only + rules unique to the tracker-client, such as NDJSON progress events and the tracker vs. app + error taxonomy). +- When `console/tracker-client` is extracted into its own repository, a copy of the global ADR + (or a reference to the version in effect at extraction time) is included in the new repo so + the two can evolve independently from that point forward. +- If the Torrust Org later decides to adopt this as an organisation-wide convention, the global + ADR can be promoted to an org-level document. Until that decision is made, each repo maintains + its own copy. + +## Scope + +### In Scope + +- All first-party, operator-facing CLI entrypoints shipped or documented in this repository, + including: + - `src/main.rs` — tracker server binary (long-running daemon; expected no-stdout-result class). + - `src/bin/http_health_check.rs` — expected stdout-result-data class (JSON health status). + - `src/bin/e2e_tests_runner.rs` and `src/bin/qbittorrent_e2e_runner.rs` — classification TBD. + - `src/bin/profiling.rs` — likely developer-only; may be out of normative scope. + - `console/tracker-client/` — all tracker-client subcommands. +- The TTY refusal rule: **adopted as stated** (commands with stdout result data refuse when + stdout is a TTY; exit 2 with JSON stderr diagnostic). +- A shared Rust CLI infrastructure package (or a decision not to create one and why). +- Workspace-level `clippy::print_stdout` / `clippy::print_stderr` lint guards. +- A redaction policy for JSON diagnostics. +- Relationship to and disposition of the existing tracker-client local ADR + (`20260512080000_define_tracker_cli_io_contract_and_error_handling.md`) and contract document. +- A recommended practice for AI agents driving CLI commands: per-command output redirection to + `.tmp/.stdout` and `.tmp/.stderr`. + +### Out of Scope + +- Developer-only tooling (`contrib/dev-tools/`, benchmarks, examples, tests). +- `build.rs` Cargo protocol output. +- Changes to the tracker-server internal tracing configuration beyond ensuring tracing + diagnostics go to stderr as JSON. +- Individual command-level contract documents (those remain in the relevant package or + `console/` subtree). +- Implementation work — this issue is to produce the ADR only. A follow-up issue will cover + migrating existing binaries to the contract. + +## Implementation Plan + +Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. + +| ID | Status | Task | Notes / Expected Output | +| --- | ------ | ------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| T1 | TODO | Enumerate and classify all in-scope binaries | A table of binaries with their expected class (stdout-result or no-stdout); base for ADR scope section | +| T2 | DONE | Decide on TTY refusal rule | Decision: **adopt as stated** (maintainer confirmed 2026-05-19); rationale to be recorded in ADR text (T5) | +| T3 | TODO | Decide on user-facing verbosity level scheme | Define levels for user-facing progress/result output (distinct from internal `RUST_LOG` tracing levels); all levels must emit JSON; document rationale for the chosen set | +| T4 | TODO | Decide on shared CLI infrastructure package | Create `packages/tracker-cli-common` (mirroring `index-cli-common`) or use a lighter approach; consider whether extraction plan for tracker-client changes this | +| T5 | TODO | Draft the global CLI output contract ADR | File at `docs/adrs/YYYYMMDDHHMMSS_global_cli_output_contract.md`; follow the ADR template; reference this spec and related docs; **must include a migration policy section** stating that current code does not yet comply and migration is progressive via a follow-up issue | +| T6 | TODO | Mark tracker-client local ADR as superseded; narrow its companion contract doc | Local ADR marked superseded by the global ADR; companion contract doc scoped to tracker-client–only rules (NDJSON progress, tracker vs. app error taxonomy) | +| T7 | TODO | Define workspace lint guard policy | Decide whether to deny `clippy::print_stdout` / `clippy::print_stderr` at workspace level; note interaction with issue #1786 (workspace lints migration) | +| T8 | TODO | Peer-review ADR draft | At minimum one review pass before accepting; update status to `Accepted` | +| T9 | TODO | Add ADR to `docs/adrs/index.md` | Row added to the index table | + +## Progress Tracking + +### Workflow Checkpoints + +- [x] Spec drafted in `docs/issues/drafts/` +- [x] Spec reviewed and approved by user/maintainer +- [x] GitHub issue created and issue number added to this spec +- [ ] ADR draft written +- [x] TTY refusal decision confirmed by maintainer (adopt as stated, 2026-05-19) +- [ ] TTY refusal decision recorded in ADR +- [ ] Verbosity level scheme decided and recorded in ADR +- [ ] Shared infrastructure decision recorded in ADR +- [ ] Existing tracker-client local ADR marked superseded; companion contract doc narrowed +- [ ] ADR peer-reviewed and status set to `Accepted` +- [ ] ADR added to `docs/adrs/index.md` +- [ ] Committer verified spec progress is up to date before commit +- [ ] Issue closed and spec moved from `docs/issues/drafts/` to `docs/issues/closed/` + +### Progress Log + +- 2026-05-18 00:00 UTC - Copilot (GitHub Copilot) - Spec drafted based on review of tracker-client + local ADR, Index ADR-T-010, and Deployer UX research docs. +- 2026-05-19 00:00 UTC - Copilot (GitHub Copilot) - Spec updated: TTY refusal marked as pending + maintainer decision; verbosity levels reframed as useful for both humans and AI agents (JSON + format only); tracker-client local ADR disposition set to supersede/deprecate. +- 2026-05-19 12:00 UTC - Copilot (GitHub Copilot) - TTY refusal decision confirmed as adopted; + new background subsection added on AI agent terminal output capture (per-command file + redirection to `.tmp/`, user-accessible well-known location); related updates to in-scope, + AC, M scenarios, and "Why this matters now". +- 2026-05-19 13:00 UTC - Copilot (GitHub Copilot) - Clarified that the ADR is prescriptive; + current code does not yet comply; migration is progressive via a follow-up issue; Goal section + updated with explicit notice; T5 notes require migration policy section; AC12 and M8 added. +- 2026-05-19 14:00 UTC - Copilot (GitHub Copilot) - Linter passed (fixed British spelling + "realising" → "realizing"); GitHub issue #1798 created; spec promoted to + `docs/issues/open/1798-global-cli-output-contract-adr.md`; branch + `1798-global-cli-output-contract-adr` created. + +## Acceptance Criteria + +- [ ] AC1: A new ADR file exists at `docs/adrs/YYYYMMDDHHMMSS_global_cli_output_contract.md`. +- [ ] AC2: The ADR states the output class (stdout-result or no-stdout) for every in-scope binary. +- [ ] AC3: The ADR makes a concrete, documented decision on TTY refusal (adopt / reject / caveats). +- [ ] AC4: The ADR defines the user-facing output verbosity level scheme (distinct from internal + tracing log levels), with rationale for the chosen set of levels. +- [ ] AC5: The ADR makes a concrete decision on a shared CLI infrastructure package. +- [ ] AC6: The ADR defines the redaction policy for JSON diagnostics. +- [ ] AC7: The tracker-client local ADR is marked superseded and the companion contract doc is + narrowed to tracker-client–specific rules. +- [ ] AC8: The ADR defines the workspace lint guard policy for `print_stdout` / `print_stderr`. +- [ ] AC9: The ADR is added to `docs/adrs/index.md`. +- [ ] AC10: The ADR is status `Accepted` after at least one review pass. +- [ ] AC11: The ADR includes a recommended practice for AI agents driving CLI commands + (per-command output redirection to `.tmp/.stdout` and `.tmp/.stderr`, + with rationale tied to the JSON-on-both-channels contract). +- [ ] AC12: The ADR includes a migration policy section that explicitly states the ADR is + prescriptive, the current codebase does not yet comply, and migration will happen + progressively via a dedicated follow-up issue. +- [ ] `linter all` exits with code `0` +- [ ] Manual verification scenarios are executed and documented (status + evidence) +- [ ] Acceptance criteria are re-reviewed after implementation and reflect actual behavior +- [ ] Documentation is updated when behavior/workflow changes + +## Verification Plan + +This issue produces a documentation artifact (an ADR), not runnable code. Verification is +therefore primarily review-based. + +### Automatic Checks + +- `linter all` — covers markdownlint, cspell, and taplo for the new ADR and this spec. + +### Manual Verification Scenarios + +Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`. + +| ID | Scenario | Command/Steps | Expected Result | Status | Evidence | +| --- | ------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------ | ------ | -------- | +| M1 | ADR file passes markdownlint | `linter all` or `markdownlint docs/adrs/.md` | No markdownlint errors | TODO | | +| M2 | ADR covers all in-scope binaries | Manual review of the binary classification table against `src/bin/` and `console/` | All binaries classified | TODO | | +| M3 | TTY refusal section gives concrete examples | Manual review of ADR text | At least two concrete examples explaining when TTY refusal fires | TODO | | +| M4 | Verbosity level scheme is defined and distinguished from log levels | Manual review of ADR text | User-facing verbosity levels defined separately from `RUST_LOG` tracing levels; all levels produce JSON | TODO | | +| M5 | Tracker-client local ADR marked superseded | Open `20260512080000_define_tracker_cli_io_contract_and_error_handling.md` | Status changed to Superseded; reference to global ADR added | TODO | | +| M6 | ADR added to index | Check `docs/adrs/index.md` | New row present with correct date and title | TODO | | +| M7 | ADR includes agent output capture recommendation | Manual review of ADR text | Per-command redirect to `.tmp/` documented with rationale tied to JSON contract | TODO | | +| M8 | ADR migration policy section is present | Manual review of ADR text | Section states ADR is prescriptive, current code non-compliant, migration is progressive via follow-up issue | TODO | | + +### Acceptance Verification + +| AC ID | Status (`TODO`/`DONE`) | Evidence | +| ----- | ---------------------- | -------- | +| AC1 | TODO | | +| AC2 | TODO | | +| AC3 | TODO | | +| AC4 | TODO | | +| AC5 | TODO | | +| AC6 | TODO | | +| AC7 | TODO | | +| AC8 | TODO | | +| AC9 | TODO | | +| AC10 | TODO | | +| AC11 | TODO | | +| AC12 | TODO | | + +## Risks and Trade-offs + +- **TTY refusal friction vs. enforcement value.** If adopted, developers lose the ability to + run stdout-producing commands directly in a terminal without piping. The benefit is a + mechanically enforced contract. Mitigation: document the `| cat` / `| jq` workaround clearly; + restrict the rule only to the commands that actually emit stdout result data (most tracker + binaries do not). +- **Shared infrastructure package scope creep.** Creating `packages/tracker-cli-common` is + useful but adds a new package to maintain. Mitigation: keep the package minimal — only the + shared scaffolding listed in Index ADR-T-010 (clap handler, panic hook, tracing setup, TTY + refusal, stdout emitter). +- **Tracker-client extraction timeline.** The local tracker-client ADR is superseded by the + global ADR, and the tracker-client companion contract doc is narrowed to tracker-client–specific + rules. When the tracker-client is extracted into its own repository, a copy of the global ADR + (or a reference to the version in effect at extraction time) travels with it and evolves + independently from that point. If the Torrust Org later adopts this as an org-wide convention, + individual repo copies may be retired in favour of the org-level document. +- **Alignment with issue #1786** (workspace lints migration). The workspace lint guards for + `print_stdout`/`print_stderr` interact with that issue. Mitigation: coordinate tasks; the + global CLI ADR defines the policy, and #1786 implements it as part of workspace lints. +- **Inconsistency window.** Until individual binaries are migrated (a separate follow-up issue), + the ADR will be accepted but not yet fully implemented. Mitigation: the ADR should include a + migration policy (analogous to the tracker-client progressive migration rule) so the gap is + documented and expected. + +## References + +- Existing tracker-client local ADR: + `console/tracker-client/docs/adrs/20260512080000_define_tracker_cli_io_contract_and_error_handling.md` +- Existing tracker-client I/O contract: + `console/tracker-client/docs/contracts/tracker-cli-io-contract.md` +- Torrust Index ADR-T-010 (the main reference and inspiration): + +- Torrust Tracker Deployer — console output research: + - + - + - +- Related issue: #1786 (workspace lints migration — interacts with `print_stdout`/`print_stderr` guards) +- ADR template: `docs/templates/ADR.md` +- ADR index: `docs/adrs/index.md` From d581e6fe230b651fb32cf8c0d5b8288855855bdd Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 19 May 2026 21:30:47 +0100 Subject: [PATCH 2/7] docs(cli): draft global CLI output contract ADR (#1798) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add docs/adrs/20260519000000_define_global_cli_output_contract.md with the full global CLI output contract (10 sections: channels, exit codes, binary classification, TTY refusal, verbosity, shared infra, redaction, lint guards, AI agent capture, migration policy). - Add ADR row to docs/adrs/index.md. - Mark tracker-client local ADR as superseded by the global ADR. - Update issue spec: T1–T6 and T9 DONE; binary classification table added; T3/T4 decisions recorded. - Add `eprint` to project-words.txt. --- ...cker_cli_io_contract_and_error_handling.md | 2 +- ...00000_define_global_cli_output_contract.md | 207 ++++++++++++++++++ docs/adrs/index.md | 1 + .../1798-global-cli-output-contract-adr.md | 105 ++++++--- project-words.txt | 1 + 5 files changed, 286 insertions(+), 30 deletions(-) create mode 100644 docs/adrs/20260519000000_define_global_cli_output_contract.md diff --git a/console/tracker-client/docs/adrs/20260512080000_define_tracker_cli_io_contract_and_error_handling.md b/console/tracker-client/docs/adrs/20260512080000_define_tracker_cli_io_contract_and_error_handling.md index 5032e5c2d..05e8b6def 100644 --- a/console/tracker-client/docs/adrs/20260512080000_define_tracker_cli_io_contract_and_error_handling.md +++ b/console/tracker-client/docs/adrs/20260512080000_define_tracker_cli_io_contract_and_error_handling.md @@ -1,6 +1,6 @@ # ADR 20260512080000: Define Tracker CLI I/O Contract and Error Handling -- Status: Accepted +- Status: Superseded by [20260519000000 — Define the global CLI output contract](../../../../docs/adrs/20260519000000_define_global_cli_output_contract.md) - Date: 2026-05-12 - Scope: console/tracker-client diff --git a/docs/adrs/20260519000000_define_global_cli_output_contract.md b/docs/adrs/20260519000000_define_global_cli_output_contract.md new file mode 100644 index 000000000..73c86bba8 --- /dev/null +++ b/docs/adrs/20260519000000_define_global_cli_output_contract.md @@ -0,0 +1,207 @@ +# Define the Global CLI Output Contract + +- Status: Proposed + +## Description + +The Torrust Tracker repository ships several CLI binaries: the tracker server daemon +(`torrust-tracker`), operational tools (`http_health_check`, `e2e_tests_runner`, +`qbittorrent_e2e_runner`), and the interactive tracker client (`tracker_client`). + +Without a repository-wide output contract, each binary can diverge in how it uses stdout, +stderr, exit codes, and output format. This causes friction for shell pipelines, container +health checks, CI orchestration, and AI agents that drive CLI commands programmatically. + +The `console/tracker-client` package already has a local ADR +(`20260512080000_define_tracker_cli_io_contract_and_error_handling.md`) with a compatible +contract, deliberately scoped to that package because extraction to its own repository was +anticipated. That local ADR is superseded by this global one. + +The Torrust Index project has an equivalent decision record (`ADR-T-010`) that served as +the primary reference for this decision. + +**This ADR is prescriptive.** The current codebase does not yet fully comply. Adoption is +progressive via a dedicated follow-up issue; see the migration policy section below. + +## Agreement + +### 1. Output channels + +- **stdout**: final command result data only. + - On success: exactly one JSON object followed by a newline. + - On failure: empty (nothing written to stdout). +- **stderr**: everything else — internal tracing diagnostics, user-facing progress events, + help text, usage errors, panic records. + - Each record is a complete JSON line (NDJSON: one JSON object per line). + - Records should carry a `kind` field (or equivalent) to allow filtering. + +No plain text on either channel, at any verbosity level. + +### 2. Exit codes + +| Code | Meaning | +| ---- | ------------------------------------------------------- | +| 0 | Command executed successfully | +| 1 | Runtime or internal failure | +| 2 | Usage error — invalid arguments, config, or TTY refusal | + +Tracker endpoint failures (announce timeout, non-200 response, etc.) are represented in the +JSON result payload on stdout. They do not cause a non-zero exit code. + +### 3. Binary classification + +Every binary is assigned one of two output classes. + +**`stdout-result-data`** — emits a JSON result object on stdout. TTY refusal applies (see +section 4). On failure, stdout is empty; the error appears on stderr as a JSON record. + +**`no-stdout-result`** — emits nothing on stdout. Pass/fail is communicated via exit code. +All diagnostics go to stderr via the tracing subscriber or direct JSON stderr writes. + +| Binary | Class | Notes | +| ------------------------ | -------------------- | --------------------------------------------------------------------- | +| `torrust-tracker` | `no-stdout-result` | Long-running daemon; tracing events to stderr | +| `http_health_check` | `stdout-result-data` | Health status JSON on stdout; currently non-compliant (plain text) | +| `e2e_tests_runner` | `no-stdout-result` | CI orchestrator; pass/fail via exit code | +| `qbittorrent_e2e_runner` | `no-stdout-result` | CI orchestrator; pass/fail via exit code | +| `tracker_client` | `stdout-result-data` | Announce/scrape results as JSON; monitor progress as NDJSON on stderr | + +The `profiling` binary is a developer-only diagnostic harness and is excluded from the +normative scope of this contract. + +### 4. TTY refusal + +Commands in the `stdout-result-data` class must refuse to run when stdout is a terminal (TTY). + +- Exit code: 2. +- A JSON diagnostic record is written to stderr explaining the refusal. + +Rationale: when stdout is a TTY, result JSON would be mixed with the shell prompt, breaking +pipelines silently. Refusing makes the contract mechanically enforceable and the error +immediately visible. Users can suppress the check with `| cat` or `| jq`. + +Example stderr record on TTY refusal: + +```json +{ + "kind": "tty_refusal", + "message": "stdout is a TTY; pipe the output to consume result data" +} +``` + +### 5. User-facing verbosity + +Verbosity is command-specific. No global verbosity scheme is prescribed by this ADR. + +The single invariant is: **all output at any verbosity level must be JSON**. Plain text is not +permitted on stdout or stderr regardless of the verbosity setting. + +### 6. Shared CLI infrastructure + +No shared infrastructure package is prescribed by this ADR. Implementors may refer to +the Torrust Index `cli-common` package as a reference implementation for common scaffolding +(TTY refusal, stdout emitter, panic hook, tracing setup). Start simple; extract common +patterns gradually as project needs arise. + +### 7. Redaction policy + +JSON diagnostics and result payloads must not expose secrets or credentials. + +- Configuration values loaded from secret sources (environment variables, files) must be + masked before inclusion in any JSON output (use `mask_secrets()` or equivalent). +- The mask value is a fixed string such as `"****"`. +- Field names that reference secrets may appear; only the values must be masked. + +### 8. Workspace lint guards + +Once migration is complete, the following `clippy` lints will be denied at workspace level: + +- `clippy::print_stdout` +- `clippy::print_stderr` + +These lints enforce that direct `print!`, `println!`, `eprint!`, and `eprintln!` calls do not +bypass the structured output contract. This interacts with issue #1786 (workspace lints +migration); coordination between that effort and the migration issue for this ADR is required. + +### 9. AI agent output capture practice + +AI agents reuse terminal sessions, which prevents reliable per-command stdout/stderr capture. + +Recommended practice when an AI agent drives a CLI command that falls under this contract: + +- Redirect stdout to `.tmp/.stdout` +- Redirect stderr to `.tmp/.stderr` + +`.tmp/` is workspace-local and git-ignored (following the existing `TORRUST_GIT_HOOKS_LOG_DIR` +convention). Two separate files preserve the stdout/stderr channel split, which is important +because stdout carries result data and stderr carries diagnostics. + +### 10. Migration policy + +This ADR is prescriptive. The current codebase does not yet fully comply. + +Migration rules: + +- **New commands and features** must comply with this contract from the moment they are + written. +- **Existing non-compliant commands** are migrated progressively when touched by new feature + work or via a dedicated follow-up migration issue. No immediate broad rewrite is required. +- **Deprecated binaries** (`http_tracker_client`, `udp_tracker_client`, `tracker_checker`) + should be **removed** rather than migrated. +- Until a binary is migrated, any non-compliance must be documented in the migration issue, + not silently tolerated. + +## Alternatives Considered + +**Adopt plain-text output with a `--json` flag.** Rejected because machine-readable output +should be the default; opt-in JSON creates inconsistent automation surfaces and increases +the API surface without benefit. + +**Make TTY refusal opt-in.** Rejected because opt-in enforcement is not enforcement. The +value of TTY refusal comes precisely from it being unconditional for stdout-result-data +commands. + +**Define a single global verbosity flag (`-q`/`-v`/`-vv`).** Rejected because verbosity +requirements vary significantly by command. A global scheme would be either too coarse or +would require command-specific override logic anyway. The binding constraint — all output +is JSON — is prescribed here; verbosity levels are left to each command. + +## Consequences + +### Positive + +- Shell pipelines, container health checks, and CI scripts can rely on a stable, parseable + output format across all Torrust Tracker binaries. +- TTY refusal makes contract violations immediately visible rather than causing silent + corruption. +- AI agents can capture and process command output reliably. +- The contract is aligned with the Torrust Index decision (ADR-T-010), enabling consistent + tooling across the Torrust ecosystem. + +### Negative + +- Developers can no longer run `stdout-result-data` commands in a terminal without piping + through `cat` or `jq`. This is intentional friction that enforces the contract. +- Migrating existing non-compliant binaries requires implementation work tracked separately. +- Until migration is complete, the ADR is accepted but partially unimplemented. + +## Date + +2026-05-19 + +## References + +- Issue spec: `docs/issues/open/1798-global-cli-output-contract-adr.md` +- Tracker-client local ADR (superseded by this ADR): + `console/tracker-client/docs/adrs/20260512080000_define_tracker_cli_io_contract_and_error_handling.md` +- Tracker-client I/O contract (narrowed to tracker-client–specific rules): + `console/tracker-client/docs/contracts/tracker-cli-io-contract.md` +- Torrust Index ADR-T-010 (primary reference): + +- Torrust Tracker Deployer — console output research: + - + - + - +- Related issue: [#1786](https://github.com/torrust/torrust-tracker/issues/1786) (workspace + lints migration — interacts with print_stdout/print_stderr guards) +- ADR index: `docs/adrs/index.md` diff --git a/docs/adrs/index.md b/docs/adrs/index.md index 768d9f2ee..aca1a814d 100644 --- a/docs/adrs/index.md +++ b/docs/adrs/index.md @@ -6,3 +6,4 @@ | [20260420200013](20260420200013_adopt_custom_github_copilot_aligned_agent_framework.md) | 2026-04-20 | Adopt a custom, GitHub-Copilot-aligned agent framework | Use AGENTS.md, Agent Skills, and Custom Agent profiles instead of third-party agent frameworks. | | [20260429000000](20260429000000_keep_database_as_aggregate_supertrait.md) | 2026-04-29 | Keep `Database` as an aggregate supertrait | Split the 18-method monolithic `Database` trait into four narrow context traits (`SchemaMigrator`, `TorrentMetricsStore`, `WhitelistStore`, `AuthKeyStore`) while keeping `Database` as an empty aggregate supertrait with a blanket impl. | | [20260512102000](20260512102000_define_tracker_client_peer_id_convention.md) | 2026-05-12 | Define tracker-client peer ID convention | Adopt `-RC3000-` Azureus-style defaults for tracker-client, use a once-per-process randomized production suffix, and keep deterministic `RC` test fixtures without cross-package constant coupling. | +| [20260519000000](20260519000000_define_global_cli_output_contract.md) | 2026-05-19 | Define the global CLI output contract | All first-party binaries use JSON on stdout (result data) and stderr (NDJSON diagnostics/progress). No plain text. TTY refusal for stdout-result-data commands. Exit codes 0/1/2. Prescriptive; migration is progressive. | diff --git a/docs/issues/open/1798-global-cli-output-contract-adr.md b/docs/issues/open/1798-global-cli-output-contract-adr.md index eeb6a304d..8f8e674e1 100644 --- a/docs/issues/open/1798-global-cli-output-contract-adr.md +++ b/docs/issues/open/1798-global-cli-output-contract-adr.md @@ -225,13 +225,8 @@ here but are narrower in scope. The decision on disposition is: ### In Scope -- All first-party, operator-facing CLI entrypoints shipped or documented in this repository, - including: - - `src/main.rs` — tracker server binary (long-running daemon; expected no-stdout-result class). - - `src/bin/http_health_check.rs` — expected stdout-result-data class (JSON health status). - - `src/bin/e2e_tests_runner.rs` and `src/bin/qbittorrent_e2e_runner.rs` — classification TBD. - - `src/bin/profiling.rs` — likely developer-only; may be out of normative scope. - - `console/tracker-client/` — all tracker-client subcommands. +- All first-party, operator-facing CLI entrypoints shipped or documented in this repository. + See the binary classification table below. - The TTY refusal rule: **adopted as stated** (commands with stdout result data refuse when stdout is a TTY; exit 2 with JSON stderr diagnostic). - A shared Rust CLI infrastructure package (or a decision not to create one and why). @@ -253,21 +248,57 @@ here but are narrower in scope. The decision on disposition is: - Implementation work — this issue is to produce the ADR only. A follow-up issue will cover migrating existing binaries to the contract. +## Binary Classification (T1) + +All first-party binaries and their expected output class under the global contract. + +**Output classes:** + +- `stdout-result-data` — emits a JSON result object on stdout; TTY refusal applies. +- `no-stdout-result` — emits nothing on stdout; pass/fail via exit code; all diagnostics + go to stderr (via tracing subscriber or `eprintln!` JSON). +- `out-of-scope` — developer-only or tooling binary; not covered by the normative contract. + +**ADR compliance key:** ✓ already compliant · ✗ non-compliant (migration needed) · — not applicable + +| Binary | Entry Point | Description | Class | Current State | ADR Compliance | +| ------------------------ | ------------------------------------------------------- | --------------------------------------------- | -------------------- | ------------------------------------------------------------------------------------------------ | ------------------------------------------------ | +| `torrust-tracker` | `src/main.rs` | Long-running tracker daemon | `no-stdout-result` | Uses `tracing::info!` only; no `println!` | ✓ | +| `http_health_check` | `src/bin/http_health_check.rs` | One-shot HTTP health probe | `stdout-result-data` | Uses plain-text `println!` ("Health check…", "STATUS:", "ERROR:") | ✗ | +| `e2e_tests_runner` | `src/bin/e2e_tests_runner.rs` | CI E2E test orchestrator (pass/fail) | `no-stdout-result` | Uses `tracing::info!` only; no `println!`; plain-text tracing subscriber | ✓ (partial — tracing subscriber needs JSON) | +| `qbittorrent_e2e_runner` | `src/bin/qbittorrent_e2e_runner.rs` | CI qBittorrent E2E orchestrator | `no-stdout-result` | Uses `tracing::info!` only; no `println!`; plain-text tracing subscriber | ✓ (partial — tracing subscriber needs JSON) | +| `profiling` | `src/bin/profiling.rs` | Developer profiling harness (valgrind) | `out-of-scope` | Uses `println!("Torrust successfully shutdown.")` and `eprintln!` for usage errors | — (not in normative scope) | +| `tracker_client` | `console/tracker-client/src/bin/tracker_client.rs` | Unified tracker client CLI | `stdout-result-data` | `http announce/scrape`, `udp announce/scrape` emit JSON via `println!`; errors on stderr as JSON | ✓ (partial — TTY refusal not yet implemented) | +| `http_tracker_client` | `console/tracker-client/src/bin/http_tracker_client.rs` | **Deprecated** — wraps `tracker_client http` | `stdout-result-data` | Delegates to `http::app::run()`; same JSON stdout behaviour | ✗ (deprecated; removal preferred over migration) | +| `udp_tracker_client` | `console/tracker-client/src/bin/udp_tracker_client.rs` | **Deprecated** — wraps `tracker_client udp` | `stdout-result-data` | Delegates to `udp::app::run()`; same JSON stdout behaviour | ✗ (deprecated; removal preferred over migration) | +| `tracker_checker` | `console/tracker-client/src/bin/tracker_checker.rs` | **Deprecated** — wraps `tracker_client check` | `stdout-result-data` | Delegates to `checker::app::run()`; errors as JSON on stderr | ✗ (deprecated; removal preferred over migration) | + +**Notes:** + +- `profiling` is excluded from the normative contract; it is a developer-only diagnostic + harness. The `println!` in it is ephemeral shutdown confirmation, not user-facing result data. +- The three deprecated binaries (`http_tracker_client`, `udp_tracker_client`, `tracker_checker`) + should be **removed** (not migrated) as part of the follow-up implementation issue. They have + already been superseded by the unified `tracker_client` subcommands. +- For `e2e_tests_runner` and `qbittorrent_e2e_runner`, the stdout channel is clean; the partial + non-compliance is that the `tracing` subscriber currently formats to plain text rather than JSON + NDJSON on stderr. That is addressed by the tracing subscriber setup, not by `println!` removal. + ## Implementation Plan Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. -| ID | Status | Task | Notes / Expected Output | -| --- | ------ | ------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| T1 | TODO | Enumerate and classify all in-scope binaries | A table of binaries with their expected class (stdout-result or no-stdout); base for ADR scope section | -| T2 | DONE | Decide on TTY refusal rule | Decision: **adopt as stated** (maintainer confirmed 2026-05-19); rationale to be recorded in ADR text (T5) | -| T3 | TODO | Decide on user-facing verbosity level scheme | Define levels for user-facing progress/result output (distinct from internal `RUST_LOG` tracing levels); all levels must emit JSON; document rationale for the chosen set | -| T4 | TODO | Decide on shared CLI infrastructure package | Create `packages/tracker-cli-common` (mirroring `index-cli-common`) or use a lighter approach; consider whether extraction plan for tracker-client changes this | -| T5 | TODO | Draft the global CLI output contract ADR | File at `docs/adrs/YYYYMMDDHHMMSS_global_cli_output_contract.md`; follow the ADR template; reference this spec and related docs; **must include a migration policy section** stating that current code does not yet comply and migration is progressive via a follow-up issue | -| T6 | TODO | Mark tracker-client local ADR as superseded; narrow its companion contract doc | Local ADR marked superseded by the global ADR; companion contract doc scoped to tracker-client–only rules (NDJSON progress, tracker vs. app error taxonomy) | -| T7 | TODO | Define workspace lint guard policy | Decide whether to deny `clippy::print_stdout` / `clippy::print_stderr` at workspace level; note interaction with issue #1786 (workspace lints migration) | -| T8 | TODO | Peer-review ADR draft | At minimum one review pass before accepting; update status to `Accepted` | -| T9 | TODO | Add ADR to `docs/adrs/index.md` | Row added to the index table | +| ID | Status | Task | Notes / Expected Output | +| --- | ------ | ------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| T1 | DONE | Enumerate and classify all in-scope binaries | Binary classification table added to spec above; base for ADR scope section | +| T2 | DONE | Decide on TTY refusal rule | Decision: **adopt as stated** (maintainer confirmed 2026-05-19); rationale to be recorded in ADR text (T5) | +| T3 | DONE | Decide on user-facing verbosity level scheme | Decision: **no global scheme** — verbosity is command-specific; the ADR only prescribes that any output at any verbosity level must comply with the JSON contract (no plain text on stdout or stderr) | +| T4 | DONE | Decide on shared CLI infrastructure package | Decision: **not an ADR concern** — the ADR references Index `cli-common` as a reference implementation only; start simple; extract common code gradually as project needs arise; no package prescribed by the ADR | +| T5 | DONE | Draft the global CLI output contract ADR | File: `docs/adrs/20260519000000_define_global_cli_output_contract.md`; follows ADR template; includes migration policy section; linter passes | +| T6 | DONE | Mark tracker-client local ADR as superseded; narrow its companion contract doc | Local ADR status changed to `Superseded by 20260519000000`; companion contract doc scope note added | +| T7 | TODO | Define workspace lint guard policy | Decide whether to deny `clippy::print_stdout` / `clippy::print_stderr` at workspace level; note interaction with issue #1786 (workspace lints migration) | +| T8 | TODO | Peer-review ADR draft | At minimum one review pass before accepting; update status to `Accepted` | +| T9 | DONE | Add ADR to `docs/adrs/index.md` | Row added to the index table | ## Progress Tracking @@ -276,14 +307,14 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. - [x] Spec drafted in `docs/issues/drafts/` - [x] Spec reviewed and approved by user/maintainer - [x] GitHub issue created and issue number added to this spec -- [ ] ADR draft written +- [x] ADR draft written (`docs/adrs/20260519000000_define_global_cli_output_contract.md`) - [x] TTY refusal decision confirmed by maintainer (adopt as stated, 2026-05-19) -- [ ] TTY refusal decision recorded in ADR -- [ ] Verbosity level scheme decided and recorded in ADR -- [ ] Shared infrastructure decision recorded in ADR -- [ ] Existing tracker-client local ADR marked superseded; companion contract doc narrowed +- [x] TTY refusal decision recorded in ADR (section 4) +- [x] Verbosity level scheme decided: no global scheme; command-specific; JSON constraint only (2026-05-19) +- [x] Shared infrastructure decided: not an ADR concern; Index `cli-common` as reference only (2026-05-19) +- [x] Existing tracker-client local ADR marked superseded; companion contract doc scope noted - [ ] ADR peer-reviewed and status set to `Accepted` -- [ ] ADR added to `docs/adrs/index.md` +- [x] ADR added to `docs/adrs/index.md` - [ ] Committer verified spec progress is up to date before commit - [ ] Issue closed and spec moved from `docs/issues/drafts/` to `docs/issues/closed/` @@ -301,19 +332,35 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. - 2026-05-19 13:00 UTC - Copilot (GitHub Copilot) - Clarified that the ADR is prescriptive; current code does not yet comply; migration is progressive via a follow-up issue; Goal section updated with explicit notice; T5 notes require migration policy section; AC12 and M8 added. -- 2026-05-19 14:00 UTC - Copilot (GitHub Copilot) - Linter passed (fixed British spelling - "realising" → "realizing"); GitHub issue #1798 created; spec promoted to +- 2026-05-19 14:00 UTC - Copilot (GitHub Copilot) - Linter passed (fixed British-spelling + variant to American spelling); GitHub issue #1798 created; spec promoted to `docs/issues/open/1798-global-cli-output-contract-adr.md`; branch `1798-global-cli-output-contract-adr` created. +- 2026-05-19 (session 3) - Copilot (GitHub Copilot) - T1 DONE: inspected all `src/bin/` entry + points and `console/tracker-client/` binaries; produced binary classification table (9 + binaries); key findings: `http_health_check` is the only `src/bin/` binary needing stdout-JSON + migration; `e2e_tests_runner` and `qbittorrent_e2e_runner` are stdout-clean (tracing subscriber + needs JSON); three deprecated tracker-client binaries should be removed, not migrated; `profiling` + is out of normative scope. Scope section updated; T1 marked DONE. +- 2026-05-19 (session 3) - Copilot (GitHub Copilot) - T3 DONE: maintainer decision — no global + verbosity scheme; verbosity is command-specific; ADR only constrains that all output at any + verbosity level must comply with the JSON contract. T4 DONE: shared infra package is not an + ADR concern; Index `cli-common` referenced as a reference implementation only; start simple + and extract common code gradually. Implementation Plan and Workflow Checkpoints updated. +- 2026-05-19 (session 3) - Copilot (GitHub Copilot) - T5 DONE: ADR drafted at + `docs/adrs/20260519000000_define_global_cli_output_contract.md`; linter passes. T6 DONE: + tracker-client local ADR status changed to Superseded. T9 DONE: ADR row added to + `docs/adrs/index.md`. `project-words.txt` updated with `eprint`. Spec updated. ## Acceptance Criteria - [ ] AC1: A new ADR file exists at `docs/adrs/YYYYMMDDHHMMSS_global_cli_output_contract.md`. - [ ] AC2: The ADR states the output class (stdout-result or no-stdout) for every in-scope binary. - [ ] AC3: The ADR makes a concrete, documented decision on TTY refusal (adopt / reject / caveats). -- [ ] AC4: The ADR defines the user-facing output verbosity level scheme (distinct from internal - tracing log levels), with rationale for the chosen set of levels. -- [ ] AC5: The ADR makes a concrete decision on a shared CLI infrastructure package. +- [ ] AC4: The ADR states that user-facing verbosity is command-specific and not globally + prescribed; it constrains only that all output at any verbosity level must be JSON. +- [ ] AC5: The ADR states that shared CLI infrastructure is not prescribed; it references + Index `cli-common` as a reference implementation and defers extraction to project needs. - [ ] AC6: The ADR defines the redaction policy for JSON diagnostics. - [ ] AC7: The tracker-client local ADR is marked superseded and the companion contract doc is narrowed to tracker-client–specific rules. diff --git a/project-words.txt b/project-words.txt index 891fb0bbb..672759281 100644 --- a/project-words.txt +++ b/project-words.txt @@ -86,6 +86,7 @@ dtolnay dylib elif endianness +eprint eprintln Eray eventfd From d38d0bb03418547478c7b8ec4392be6bb5fab13d Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 19 May 2026 21:39:21 +0100 Subject: [PATCH 3/7] docs(adrs): drop explicit Status field from accepted ADRs (#1798) Merged ADRs are implicitly accepted via PR review. No `- Status:` header is needed for the common case. - Remove `- Status: Proposed` from the global CLI output contract ADR. - Add an ADR Lifecycle section to docs/adrs/index.md explaining the policy (status header only for special states: Superseded, etc.). - Add an ADR Status subsection to the create-adr skill with the same guidance. --- .github/skills/dev/planning/create-adr/SKILL.md | 10 ++++++++++ ...0260519000000_define_global_cli_output_contract.md | 2 -- docs/adrs/index.md | 11 +++++++++++ .../open/1798-global-cli-output-contract-adr.md | 2 +- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/.github/skills/dev/planning/create-adr/SKILL.md b/.github/skills/dev/planning/create-adr/SKILL.md index 07b864b2f..c1428610d 100644 --- a/.github/skills/dev/planning/create-adr/SKILL.md +++ b/.github/skills/dev/planning/create-adr/SKILL.md @@ -69,6 +69,16 @@ Optional sections to add when relevant: - **Alternatives Considered**: other options explored and why they were rejected - **Consequences**: positive and negative effects of the decision +### ADR Status + +Do **not** add a `- Status:` header by default. An ADR merged into `develop` or `main` is +implicitly accepted — the PR review process is the acceptance gate. + +Only add a `- Status:` header for special terminal states: + +- `- Status: Superseded by [ADR link]` — this decision has been replaced by a newer ADR. +- Additional states (e.g. `Deprecated`) may be introduced as needed. + ## Step-by-Step Process ### Step 1: Generate Filename diff --git a/docs/adrs/20260519000000_define_global_cli_output_contract.md b/docs/adrs/20260519000000_define_global_cli_output_contract.md index 73c86bba8..361bf06fb 100644 --- a/docs/adrs/20260519000000_define_global_cli_output_contract.md +++ b/docs/adrs/20260519000000_define_global_cli_output_contract.md @@ -1,7 +1,5 @@ # Define the Global CLI Output Contract -- Status: Proposed - ## Description The Torrust Tracker repository ships several CLI binaries: the tracker server daemon diff --git a/docs/adrs/index.md b/docs/adrs/index.md index aca1a814d..a594be740 100644 --- a/docs/adrs/index.md +++ b/docs/adrs/index.md @@ -7,3 +7,14 @@ | [20260429000000](20260429000000_keep_database_as_aggregate_supertrait.md) | 2026-04-29 | Keep `Database` as an aggregate supertrait | Split the 18-method monolithic `Database` trait into four narrow context traits (`SchemaMigrator`, `TorrentMetricsStore`, `WhitelistStore`, `AuthKeyStore`) while keeping `Database` as an empty aggregate supertrait with a blanket impl. | | [20260512102000](20260512102000_define_tracker_client_peer_id_convention.md) | 2026-05-12 | Define tracker-client peer ID convention | Adopt `-RC3000-` Azureus-style defaults for tracker-client, use a once-per-process randomized production suffix, and keep deterministic `RC` test fixtures without cross-package constant coupling. | | [20260519000000](20260519000000_define_global_cli_output_contract.md) | 2026-05-19 | Define the global CLI output contract | All first-party binaries use JSON on stdout (result data) and stderr (NDJSON diagnostics/progress). No plain text. TTY refusal for stdout-result-data commands. Exit codes 0/1/2. Prescriptive; migration is progressive. | + +## ADR Lifecycle + +An ADR merged into `develop` or `main` is **accepted**. The PR review process is the acceptance +gate — no explicit `- Status: Accepted` or `- Status: Proposed` header is needed or written. + +A `- Status:` header appears in an ADR file only for special terminal states, for example: + +- `- Status: Superseded by [ADR link]` — this decision has been replaced by a newer ADR. + +Additional states (e.g. `Deprecated`) may be introduced as needed. diff --git a/docs/issues/open/1798-global-cli-output-contract-adr.md b/docs/issues/open/1798-global-cli-output-contract-adr.md index 8f8e674e1..0f126c7b5 100644 --- a/docs/issues/open/1798-global-cli-output-contract-adr.md +++ b/docs/issues/open/1798-global-cli-output-contract-adr.md @@ -315,7 +315,7 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. - [x] Existing tracker-client local ADR marked superseded; companion contract doc scope noted - [ ] ADR peer-reviewed and status set to `Accepted` - [x] ADR added to `docs/adrs/index.md` -- [ ] Committer verified spec progress is up to date before commit +- [x] Committer verified spec progress is up to date before commit - [ ] Issue closed and spec moved from `docs/issues/drafts/` to `docs/issues/closed/` ### Progress Log From 3bbb119fc4d0a6a9469fe14b367bb4bdae39df54 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 19 May 2026 21:48:08 +0100 Subject: [PATCH 4/7] docs(issues): add draft follow-up issue for CLI output contract migration (#1798) Draft issue spec covering: - Remove three deprecated binaries (udp_tracker_client, http_tracker_client, tracker_checker). - Migrate http_health_check to JSON stdout/stderr. - Wire TTY refusal into tracker_client. - Rewrite tracker-client console abstraction layer. - Replace library-level println! with tracing in packages/configuration and packages/udp-tracker-core. - Enable clippy::print_stdout / clippy::print_stderr as workspace-level deny lints once migration is complete. --- .../drafts/cli-output-contract-migration.md | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 docs/issues/drafts/cli-output-contract-migration.md diff --git a/docs/issues/drafts/cli-output-contract-migration.md b/docs/issues/drafts/cli-output-contract-migration.md new file mode 100644 index 000000000..ed5ee8252 --- /dev/null +++ b/docs/issues/drafts/cli-output-contract-migration.md @@ -0,0 +1,111 @@ +--- +doc-type: issue +issue-type: task +status: draft +priority: p2 +github-issue: null +spec-path: docs/issues/drafts/cli-output-contract-migration.md +branch: null +related-pr: null +last-updated-utc: 2026-05-19 20:00 +semantic-links: + skill-links: + - create-issue + related-artifacts: + - docs/adrs/20260519000000_define_global_cli_output_contract.md + - src/bin/http_health_check.rs + - console/tracker-client/src/bin/tracker_client.rs + - packages/configuration/src/lib.rs +--- + + + +# Issue #[To be assigned] - Migrate Existing Binaries to the Global CLI Output Contract + +## Goal + +Bring the codebase into compliance with the global CLI output contract defined in +[ADR 20260519000000](../../adrs/20260519000000_define_global_cli_output_contract.md). +Once all non-compliant uses of `print!`, `println!`, `eprint!`, and `eprintln!` are +resolved, enable `clippy::print_stdout` and `clippy::print_stderr` as workspace-level +`deny` lints to make the contract a compile-time guarantee. + +## Background + +ADR 20260519000000 is prescriptive: it defines what every first-party binary must do but +explicitly defers migration of existing code to this follow-up issue. New commands and +features must already comply; only pre-existing usages are migrated here. + +A workspace-wide grep found **46 occurrences** of direct print macros across the codebase +(as of 2026-05-19). The breakdown by area is: + +| Area | Files | Occurrences | Action | +| ---- | ----- | ----------- | ------ | +| `src/bin/http_health_check.rs` | 1 | 5 | Migrate to JSON stdout/stderr | +| `src/console/profiling.rs` | 1 | 3 | Out of scope (developer harness; excluded by ADR) | +| `console/tracker-client/src/bin/tracker_client.rs` | 1 | 2 | Wire TTY refusal; already nearly compliant | +| `console/tracker-client/src/bin/udp_tracker_client.rs` | 1 | 1 | Remove (deprecated binary) | +| `console/tracker-client/src/bin/http_tracker_client.rs` | 1 | 1 | Remove (deprecated binary) | +| `console/tracker-client/src/bin/tracker_checker.rs` | 1 | 2 | Remove (deprecated binary) | +| `console/tracker-client/src/console/clients/` | ~6 | ~16 | Rewrite console abstraction layer to emit JSON | +| `packages/configuration/src/lib.rs` | 1 | 3 | Replace with `tracing::info!` | +| `packages/udp-tracker-core/src/services/banning.rs` | 1 | 1 | Replace or remove debug print | +| `packages/tracker-core/src/databases/driver/{mysql,postgres}/mod.rs` | 2 | 2 | Replace with `tracing::info!` (test-skip messages) | +| `packages/tracker-core/src/bin/persistence_benchmark/runner.rs` | 1 | 1 | Assess: JSON output or out of scope | +| `packages/test-helpers/src/logging.rs` | 1 | 1 | Assess: test-only; may warrant `#[allow]` | +| `contrib/dev-tools/analysis/workspace-coupling/src/main.rs` | 1 | 6 | Assess: dev tool; may be out of scope | + +## Out of Scope + +- `src/console/profiling.rs` — explicitly excluded from the contract by ADR section 3. +- `contrib/dev-tools/` — developer tooling; not operator-facing binaries. Excluded unless + the team decides otherwise. + +## Acceptance Criteria + +| ID | Criterion | +| --- | --------- | +| AC1 | `src/bin/http_health_check.rs` emits a single JSON object on stdout on success and a JSON record on stderr on failure; no `println!` or `eprintln!` remain. | +| AC2 | `console/tracker-client/src/bin/tracker_client.rs` refuses to run when stdout is a TTY (exit 2, JSON stderr diagnostic). | +| AC3 | Deprecated binaries `udp_tracker_client`, `http_tracker_client`, and `tracker_checker` are removed from the repository. | +| AC4 | `packages/configuration/src/lib.rs` uses `tracing` for configuration loading notifications; no `println!` remain. | +| AC5 | All remaining in-scope `print!`/`println!`/`eprint!`/`eprintln!` usages are either migrated or carry an explicit `#[allow(clippy::print_stdout)]` / `#[allow(clippy::print_stderr)]` with a justification comment. | +| AC6 | `clippy::print_stdout = "deny"` and `clippy::print_stderr = "deny"` are added to `[workspace.lints.clippy]` in the root `Cargo.toml`. | +| AC7 | `cargo clippy --workspace --all-targets --all-features` passes with no new warnings or errors. | +| AC8 | All existing tests pass. | + +## Implementation Plan + +| ID | Status | Task | Notes | +| --- | --------- | ---- | ----- | +| T1 | TODO | Remove deprecated binaries | Delete `udp_tracker_client.rs`, `http_tracker_client.rs`, `tracker_checker.rs` and their `Cargo.toml` entries | +| T2 | TODO | Migrate `http_health_check` to JSON output | Rewrite to emit `{"status":"ok"}` / `{"status":"error","message":"..."}` on stdout; usage errors as JSON on stderr | +| T3 | TODO | Wire TTY refusal into `tracker_client` | Check `stdout.is_terminal()` at entry; exit 2 with JSON stderr diagnostic if true | +| T4 | TODO | Rewrite tracker-client console abstraction layer | Replace `console.rs` and related print calls in `clients/` with JSON emitters | +| T5 | TODO | Replace `println!` in `packages/configuration` with `tracing` | Three config-loading notification messages | +| T6 | TODO | Replace debug print in `packages/udp-tracker-core/src/services/banning.rs` | Remove or replace with `tracing::debug!` | +| T7 | TODO | Replace test-skip `println!` in database drivers | Replace with `tracing::info!` or `eprintln!` under `#[allow]` with justification | +| T8 | TODO | Assess `persistence_benchmark` and `test-helpers` usages | Decide: JSON output, `tracing`, or `#[allow]` with justification | +| T9 | TODO | Enable workspace-level lint denials | Add `print_stdout = "deny"` and `print_stderr = "deny"` to `[workspace.lints.clippy]` in root `Cargo.toml`; ensure `cargo clippy` passes | + +## Progress Tracking + +### Workflow Checkpoints + +- [ ] Spec reviewed and approved by user/maintainer +- [ ] GitHub issue created and issue number added to this spec +- [ ] Deprecated binaries removed (T1) +- [ ] `http_health_check` migrated to JSON output (T2) +- [ ] TTY refusal wired into `tracker_client` (T3) +- [ ] Tracker-client console abstraction layer rewritten (T4) +- [ ] Library `println!` usages replaced (T5–T8) +- [ ] Workspace lint denials enabled and `cargo clippy` passes (T9) +- [ ] All tests pass +- [ ] Issue closed and spec moved to `docs/issues/closed/` + +## References + +- Global CLI output contract ADR: `docs/adrs/20260519000000_define_global_cli_output_contract.md` +- Parent issue: [#1798](https://github.com/torrust/torrust-tracker/issues/1798) +- Workspace lints migration: [#1786](https://github.com/torrust/torrust-tracker/issues/1786) + (coordinate on `print_stdout`/`print_stderr` deny timing) From 2ab37c847c12062368f9b6e88a39afb4a2ba092c Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 19 May 2026 21:55:25 +0100 Subject: [PATCH 5/7] =?UTF-8?q?docs(issues):=20update=20spec=20progress=20?= =?UTF-8?q?=E2=80=94=20T7=20DONE,=20T8=20reworded=20for=20ADR=20lifecycle?= =?UTF-8?q?=20(#1798)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Mark T7 DONE: workspace lint guard deferred to follow-up draft issue docs/issues/drafts/cli-output-contract-migration.md. - Reword T8 note: peer review via PR; merged to develop = accepted. - Update checkpoint: "ADR peer-reviewed and status set to Accepted" → "PR opened, reviewed, and merged to develop". - Fix close-checkpoint path: drafts/ → open/. - Reword AC10 to match the new ADR lifecycle (no explicit status field). - Reformat migration draft tables (column alignment only). --- .../drafts/cli-output-contract-migration.md | 70 +++++++++---------- .../1798-global-cli-output-contract-adr.md | 18 +++-- 2 files changed, 47 insertions(+), 41 deletions(-) diff --git a/docs/issues/drafts/cli-output-contract-migration.md b/docs/issues/drafts/cli-output-contract-migration.md index ed5ee8252..68a16129b 100644 --- a/docs/issues/drafts/cli-output-contract-migration.md +++ b/docs/issues/drafts/cli-output-contract-migration.md @@ -39,21 +39,21 @@ features must already comply; only pre-existing usages are migrated here. A workspace-wide grep found **46 occurrences** of direct print macros across the codebase (as of 2026-05-19). The breakdown by area is: -| Area | Files | Occurrences | Action | -| ---- | ----- | ----------- | ------ | -| `src/bin/http_health_check.rs` | 1 | 5 | Migrate to JSON stdout/stderr | -| `src/console/profiling.rs` | 1 | 3 | Out of scope (developer harness; excluded by ADR) | -| `console/tracker-client/src/bin/tracker_client.rs` | 1 | 2 | Wire TTY refusal; already nearly compliant | -| `console/tracker-client/src/bin/udp_tracker_client.rs` | 1 | 1 | Remove (deprecated binary) | -| `console/tracker-client/src/bin/http_tracker_client.rs` | 1 | 1 | Remove (deprecated binary) | -| `console/tracker-client/src/bin/tracker_checker.rs` | 1 | 2 | Remove (deprecated binary) | -| `console/tracker-client/src/console/clients/` | ~6 | ~16 | Rewrite console abstraction layer to emit JSON | -| `packages/configuration/src/lib.rs` | 1 | 3 | Replace with `tracing::info!` | -| `packages/udp-tracker-core/src/services/banning.rs` | 1 | 1 | Replace or remove debug print | -| `packages/tracker-core/src/databases/driver/{mysql,postgres}/mod.rs` | 2 | 2 | Replace with `tracing::info!` (test-skip messages) | -| `packages/tracker-core/src/bin/persistence_benchmark/runner.rs` | 1 | 1 | Assess: JSON output or out of scope | -| `packages/test-helpers/src/logging.rs` | 1 | 1 | Assess: test-only; may warrant `#[allow]` | -| `contrib/dev-tools/analysis/workspace-coupling/src/main.rs` | 1 | 6 | Assess: dev tool; may be out of scope | +| Area | Files | Occurrences | Action | +| -------------------------------------------------------------------- | ----- | ----------- | -------------------------------------------------- | +| `src/bin/http_health_check.rs` | 1 | 5 | Migrate to JSON stdout/stderr | +| `src/console/profiling.rs` | 1 | 3 | Out of scope (developer harness; excluded by ADR) | +| `console/tracker-client/src/bin/tracker_client.rs` | 1 | 2 | Wire TTY refusal; already nearly compliant | +| `console/tracker-client/src/bin/udp_tracker_client.rs` | 1 | 1 | Remove (deprecated binary) | +| `console/tracker-client/src/bin/http_tracker_client.rs` | 1 | 1 | Remove (deprecated binary) | +| `console/tracker-client/src/bin/tracker_checker.rs` | 1 | 2 | Remove (deprecated binary) | +| `console/tracker-client/src/console/clients/` | ~6 | ~16 | Rewrite console abstraction layer to emit JSON | +| `packages/configuration/src/lib.rs` | 1 | 3 | Replace with `tracing::info!` | +| `packages/udp-tracker-core/src/services/banning.rs` | 1 | 1 | Replace or remove debug print | +| `packages/tracker-core/src/databases/driver/{mysql,postgres}/mod.rs` | 2 | 2 | Replace with `tracing::info!` (test-skip messages) | +| `packages/tracker-core/src/bin/persistence_benchmark/runner.rs` | 1 | 1 | Assess: JSON output or out of scope | +| `packages/test-helpers/src/logging.rs` | 1 | 1 | Assess: test-only; may warrant `#[allow]` | +| `contrib/dev-tools/analysis/workspace-coupling/src/main.rs` | 1 | 6 | Assess: dev tool; may be out of scope | ## Out of Scope @@ -63,30 +63,30 @@ A workspace-wide grep found **46 occurrences** of direct print macros across the ## Acceptance Criteria -| ID | Criterion | -| --- | --------- | -| AC1 | `src/bin/http_health_check.rs` emits a single JSON object on stdout on success and a JSON record on stderr on failure; no `println!` or `eprintln!` remain. | -| AC2 | `console/tracker-client/src/bin/tracker_client.rs` refuses to run when stdout is a TTY (exit 2, JSON stderr diagnostic). | -| AC3 | Deprecated binaries `udp_tracker_client`, `http_tracker_client`, and `tracker_checker` are removed from the repository. | -| AC4 | `packages/configuration/src/lib.rs` uses `tracing` for configuration loading notifications; no `println!` remain. | +| ID | Criterion | +| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| AC1 | `src/bin/http_health_check.rs` emits a single JSON object on stdout on success and a JSON record on stderr on failure; no `println!` or `eprintln!` remain. | +| AC2 | `console/tracker-client/src/bin/tracker_client.rs` refuses to run when stdout is a TTY (exit 2, JSON stderr diagnostic). | +| AC3 | Deprecated binaries `udp_tracker_client`, `http_tracker_client`, and `tracker_checker` are removed from the repository. | +| AC4 | `packages/configuration/src/lib.rs` uses `tracing` for configuration loading notifications; no `println!` remain. | | AC5 | All remaining in-scope `print!`/`println!`/`eprint!`/`eprintln!` usages are either migrated or carry an explicit `#[allow(clippy::print_stdout)]` / `#[allow(clippy::print_stderr)]` with a justification comment. | -| AC6 | `clippy::print_stdout = "deny"` and `clippy::print_stderr = "deny"` are added to `[workspace.lints.clippy]` in the root `Cargo.toml`. | -| AC7 | `cargo clippy --workspace --all-targets --all-features` passes with no new warnings or errors. | -| AC8 | All existing tests pass. | +| AC6 | `clippy::print_stdout = "deny"` and `clippy::print_stderr = "deny"` are added to `[workspace.lints.clippy]` in the root `Cargo.toml`. | +| AC7 | `cargo clippy --workspace --all-targets --all-features` passes with no new warnings or errors. | +| AC8 | All existing tests pass. | ## Implementation Plan -| ID | Status | Task | Notes | -| --- | --------- | ---- | ----- | -| T1 | TODO | Remove deprecated binaries | Delete `udp_tracker_client.rs`, `http_tracker_client.rs`, `tracker_checker.rs` and their `Cargo.toml` entries | -| T2 | TODO | Migrate `http_health_check` to JSON output | Rewrite to emit `{"status":"ok"}` / `{"status":"error","message":"..."}` on stdout; usage errors as JSON on stderr | -| T3 | TODO | Wire TTY refusal into `tracker_client` | Check `stdout.is_terminal()` at entry; exit 2 with JSON stderr diagnostic if true | -| T4 | TODO | Rewrite tracker-client console abstraction layer | Replace `console.rs` and related print calls in `clients/` with JSON emitters | -| T5 | TODO | Replace `println!` in `packages/configuration` with `tracing` | Three config-loading notification messages | -| T6 | TODO | Replace debug print in `packages/udp-tracker-core/src/services/banning.rs` | Remove or replace with `tracing::debug!` | -| T7 | TODO | Replace test-skip `println!` in database drivers | Replace with `tracing::info!` or `eprintln!` under `#[allow]` with justification | -| T8 | TODO | Assess `persistence_benchmark` and `test-helpers` usages | Decide: JSON output, `tracing`, or `#[allow]` with justification | -| T9 | TODO | Enable workspace-level lint denials | Add `print_stdout = "deny"` and `print_stderr = "deny"` to `[workspace.lints.clippy]` in root `Cargo.toml`; ensure `cargo clippy` passes | +| ID | Status | Task | Notes | +| --- | ------ | -------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | +| T1 | TODO | Remove deprecated binaries | Delete `udp_tracker_client.rs`, `http_tracker_client.rs`, `tracker_checker.rs` and their `Cargo.toml` entries | +| T2 | TODO | Migrate `http_health_check` to JSON output | Rewrite to emit `{"status":"ok"}` / `{"status":"error","message":"..."}` on stdout; usage errors as JSON on stderr | +| T3 | TODO | Wire TTY refusal into `tracker_client` | Check `stdout.is_terminal()` at entry; exit 2 with JSON stderr diagnostic if true | +| T4 | TODO | Rewrite tracker-client console abstraction layer | Replace `console.rs` and related print calls in `clients/` with JSON emitters | +| T5 | TODO | Replace `println!` in `packages/configuration` with `tracing` | Three config-loading notification messages | +| T6 | TODO | Replace debug print in `packages/udp-tracker-core/src/services/banning.rs` | Remove or replace with `tracing::debug!` | +| T7 | TODO | Replace test-skip `println!` in database drivers | Replace with `tracing::info!` or `eprintln!` under `#[allow]` with justification | +| T8 | TODO | Assess `persistence_benchmark` and `test-helpers` usages | Decide: JSON output, `tracing`, or `#[allow]` with justification | +| T9 | TODO | Enable workspace-level lint denials | Add `print_stdout = "deny"` and `print_stderr = "deny"` to `[workspace.lints.clippy]` in root `Cargo.toml`; ensure `cargo clippy` passes | ## Progress Tracking diff --git a/docs/issues/open/1798-global-cli-output-contract-adr.md b/docs/issues/open/1798-global-cli-output-contract-adr.md index 0f126c7b5..28a76727a 100644 --- a/docs/issues/open/1798-global-cli-output-contract-adr.md +++ b/docs/issues/open/1798-global-cli-output-contract-adr.md @@ -7,7 +7,7 @@ github-issue: 1798 spec-path: docs/issues/open/1798-global-cli-output-contract-adr.md branch: 1798-global-cli-output-contract-adr related-pr: null -last-updated-utc: 2026-05-19 14:00 +last-updated-utc: 2026-05-19 20:30 semantic-links: skill-links: - create-issue @@ -296,8 +296,8 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. | T4 | DONE | Decide on shared CLI infrastructure package | Decision: **not an ADR concern** — the ADR references Index `cli-common` as a reference implementation only; start simple; extract common code gradually as project needs arise; no package prescribed by the ADR | | T5 | DONE | Draft the global CLI output contract ADR | File: `docs/adrs/20260519000000_define_global_cli_output_contract.md`; follows ADR template; includes migration policy section; linter passes | | T6 | DONE | Mark tracker-client local ADR as superseded; narrow its companion contract doc | Local ADR status changed to `Superseded by 20260519000000`; companion contract doc scope note added | -| T7 | TODO | Define workspace lint guard policy | Decide whether to deny `clippy::print_stdout` / `clippy::print_stderr` at workspace level; note interaction with issue #1786 (workspace lints migration) | -| T8 | TODO | Peer-review ADR draft | At minimum one review pass before accepting; update status to `Accepted` | +| T7 | DONE | Define workspace lint guard policy | Decision: defer implementation to follow-up issue `docs/issues/drafts/cli-output-contract-migration.md`; ADR section 8 documents the policy | +| T8 | TODO | Peer-review ADR draft via PR | Open PR from `1798-global-cli-output-contract-adr` → `develop`; PR review is the acceptance gate; once merged, the ADR is accepted per lifecycle policy (see `docs/adrs/index.md`) | | T9 | DONE | Add ADR to `docs/adrs/index.md` | Row added to the index table | ## Progress Tracking @@ -313,10 +313,10 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. - [x] Verbosity level scheme decided: no global scheme; command-specific; JSON constraint only (2026-05-19) - [x] Shared infrastructure decided: not an ADR concern; Index `cli-common` as reference only (2026-05-19) - [x] Existing tracker-client local ADR marked superseded; companion contract doc scope noted -- [ ] ADR peer-reviewed and status set to `Accepted` +- [ ] PR opened, reviewed, and merged to `develop` (merged = accepted per ADR lifecycle policy) - [x] ADR added to `docs/adrs/index.md` - [x] Committer verified spec progress is up to date before commit -- [ ] Issue closed and spec moved from `docs/issues/drafts/` to `docs/issues/closed/` +- [ ] Issue closed and spec moved from `docs/issues/open/` to `docs/issues/closed/` ### Progress Log @@ -351,6 +351,12 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. `docs/adrs/20260519000000_define_global_cli_output_contract.md`; linter passes. T6 DONE: tracker-client local ADR status changed to Superseded. T9 DONE: ADR row added to `docs/adrs/index.md`. `project-words.txt` updated with `eprint`. Spec updated. +- 2026-05-19 (session 4) - Copilot (GitHub Copilot) - Removed `- Status: Proposed` from ADR + (merged ADRs are implicitly accepted; PR review is the acceptance gate). Added ADR Lifecycle + section to `docs/adrs/index.md` and `### ADR Status` subsection to `create-adr` skill. + T7 DONE: workspace lint guard deferred to follow-up draft issue + `docs/issues/drafts/cli-output-contract-migration.md` (46 print macro occurrences surveyed; + 9-task migration plan drafted). T8 remains: open PR and get it merged. ## Acceptance Criteria @@ -366,7 +372,7 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. narrowed to tracker-client–specific rules. - [ ] AC8: The ADR defines the workspace lint guard policy for `print_stdout` / `print_stderr`. - [ ] AC9: The ADR is added to `docs/adrs/index.md`. -- [ ] AC10: The ADR is status `Accepted` after at least one review pass. +- [ ] AC10: The ADR is merged to `develop` via PR review (merged = accepted per ADR lifecycle; no explicit status field needed). - [ ] AC11: The ADR includes a recommended practice for AI agents driving CLI commands (per-command output redirection to `.tmp/.stdout` and `.tmp/.stderr`, with rationale tied to the JSON-on-both-channels contract). From 034aa33b998abc9e837e72a3085aa91dc1c3f439 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Wed, 20 May 2026 07:37:51 +0100 Subject: [PATCH 6/7] docs(cli): address Copilot review comments on PR #1800 (#1798) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix issue spec frontmatter: status open → planned (matches repo convention for open issue specs). - Fix issue spec background: local ADR reference updated from "status: Accepted" to "superseded by ADR 20260519000000". - Fix ADR TTY-refusal example: add note that examples are pretty-printed; actual wire format is single-line NDJSON. --- docs/adrs/20260519000000_define_global_cli_output_contract.md | 3 ++- docs/issues/open/1798-global-cli-output-contract-adr.md | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/adrs/20260519000000_define_global_cli_output_contract.md b/docs/adrs/20260519000000_define_global_cli_output_contract.md index 361bf06fb..f6ad8e036 100644 --- a/docs/adrs/20260519000000_define_global_cli_output_contract.md +++ b/docs/adrs/20260519000000_define_global_cli_output_contract.md @@ -78,7 +78,8 @@ Rationale: when stdout is a TTY, result JSON would be mixed with the shell promp pipelines silently. Refusing makes the contract mechanically enforceable and the error immediately visible. Users can suppress the check with `| cat` or `| jq`. -Example stderr record on TTY refusal: +Example stderr record on TTY refusal (pretty-printed for readability; the actual wire format is +a single JSON line per the NDJSON contract): ```json { diff --git a/docs/issues/open/1798-global-cli-output-contract-adr.md b/docs/issues/open/1798-global-cli-output-contract-adr.md index 28a76727a..ff8f13a3e 100644 --- a/docs/issues/open/1798-global-cli-output-contract-adr.md +++ b/docs/issues/open/1798-global-cli-output-contract-adr.md @@ -1,7 +1,7 @@ --- doc-type: issue issue-type: task -status: open +status: planned priority: p2 github-issue: 1798 spec-path: docs/issues/open/1798-global-cli-output-contract-adr.md @@ -41,7 +41,7 @@ The tracker already has a local CLI I/O contract, but it is scoped only to `console/tracker-client`: - `console/tracker-client/docs/adrs/20260512080000_define_tracker_cli_io_contract_and_error_handling.md` - (status: Accepted) — defines JSON default, stdout/stderr channel split, exit codes 0/1/2, and + (superseded by ADR 20260519000000) — defined JSON default, stdout/stderr channel split, exit codes 0/1/2, and NDJSON progress for monitor-style commands. - `console/tracker-client/docs/contracts/tracker-cli-io-contract.md` — the normative companion contract document. From d8fb7f2ae4409ccc472d99291bc72a87bddd9f8b Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Wed, 20 May 2026 08:00:18 +0100 Subject: [PATCH 7/7] docs(cli): use ndjson code fence for TTY-refusal example (#1798) Use `ndjson` as the code fence language instead of `json` to prevent the IDE's automatic JSON formatter from reformatting the single-line example into a multi-line block on save. --- ...60519000000_define_global_cli_output_contract.md | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/docs/adrs/20260519000000_define_global_cli_output_contract.md b/docs/adrs/20260519000000_define_global_cli_output_contract.md index f6ad8e036..79389b45a 100644 --- a/docs/adrs/20260519000000_define_global_cli_output_contract.md +++ b/docs/adrs/20260519000000_define_global_cli_output_contract.md @@ -78,14 +78,11 @@ Rationale: when stdout is a TTY, result JSON would be mixed with the shell promp pipelines silently. Refusing makes the contract mechanically enforceable and the error immediately visible. Users can suppress the check with `| cat` or `| jq`. -Example stderr record on TTY refusal (pretty-printed for readability; the actual wire format is -a single JSON line per the NDJSON contract): - -```json -{ - "kind": "tty_refusal", - "message": "stdout is a TTY; pipe the output to consume result data" -} +Example stderr record on TTY refusal (one JSON object on a single line, as required by the +NDJSON contract): + +```ndjson +{"kind":"tty_refusal","message":"stdout is a TTY; pipe the output to consume result data"} ``` ### 5. User-facing verbosity