Cli cleanup - #2710
Merged
Merged
Conversation
`ModeOfOperation` distinguished `Interactive` from `Command`, but both variants carry a command in the sense that matters: the REPL parses the same grammar. The old name therefore said nothing about the axis the enum divides, which is whether the session opens a prompt or executes once and exits. `NonInteractive` names that axis directly and reads as the complement of `Interactive`. The rename reaches every use site in the crate and its tests. It also repairs the intra-doc link on `get_mode_of_operation`, which still pointed at the old variant and would have become a broken rustdoc reference. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Commit d02d6f1 removed a `#[cfg(feature = "nym")]` attribute from `socks5_transmit_failure` and left the blank line that had separated it from the doc-comment above. Clippy reads a doc-comment followed by a blank line as documentation attached to nothing, and warned under `empty_line_after_doc_comments`. Removing the line reattaches the comment to the function it documents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The binary computed the mode of operation a second time, through the public wrapper `is_interactive`, only so it could decide where tracing output goes. That wrapper and `log_file_path` existed for no other caller, so two public functions and forty lines of `main.rs` served one decision the library was already equipped to make. `init_tracing` moves into the library, and both helpers become private. The boundary the crate documents is unchanged: the binary still chooses when process-global state is installed, which matters because `tracing_subscriber::fmt().init()` panics on a second call and a test driving `run_cli` twice would hit it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Moving `init_tracing` into the library removed `is_interactive` and `log_file_path` from the crate's public surface, which is a breaking change the Unreleased section did not state. No consumer in this workspace called either function, but the changelog records the surface, not the workspace. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The diary wrote `indexer-history.tsv` beside the wallet: a plaintext record of when this wallet transmitted, to which indexer, and how each attempt ended. Three guards bounded that at-rest risk. Removing the file retires the risk instead of guarding it, and costs only the history's reach across sessions. `IndexerHistoryHandle` now keeps this session's attempts in a bounded in-memory store and folds each into the session's Health, which is the half the draws actually consult. Everything the file required goes with it: the serializer and its tolerant loader, the compaction pass, the wire tokens on `AttemptRoute`, `AttemptKind`, `FailureKind`, and `FailurePhase`, and the `path` and `recording` state behind `beside_wallet`, `set_recording`, and `is_recording`. `IndexerAttempt::exit` goes too. It was written to the file's exit column and read back by nothing, so with the column gone the field has no reader. Dropping it also removes the ungated `crate::mixnet::ExitNodeId` reference that left `zingolib` uncompilable without the `nym` feature. The `nym-diary` feature still names `beside_wallet` and `set_recording`, so `--features nym-diary` does not build until the following commit deletes the feature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The feature existed to compile in a disk-backed diary handle, and the previous commit removed the disk. What remained was a gate over nothing: a build flag, a `--indexer-diary` session flag, and a runtime opt-in that switched writes no longer performed. All three go, and `network history` improves by losing them. It no longer has two bodies selected by feature, one of which only told the user to rebuild; it renders the attempts this session recorded, in every build that carries the mixnet. The CI job that paired `nym` with `nym-diary` now runs `nym` alone, and ADR 0026's description of that job follows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Commit d02d6f1 replaced twenty-one per-item `#[cfg(feature = "nym")]` attributes with one module-level gate and left the blank lines those attributes had occupied. Clippy reads a doc-comment followed by a blank line as documentation attached to nothing, and the `nym` job lints at `-D warnings`, so `charge_phase`, `mixnet_timing`, and `socks5_transmit_stage` each failed the build. Commit 363f364 closed the fourth instance; these are the rest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A diary is something you keep. The name was earned by the file beside the wallet, and with that file gone the word claims a permanence the thing no longer has, while the module it lives in is already called `indexer_history`. One name for one thing: `MAX_DIARY_ATTEMPTS` becomes `MAX_HISTORY_ATTEMPTS`, the prose in `sweep.rs` and `correspondent.rs` says history, and the Health entry in CONTEXT.md drops the term while recording that it was retired. The string-promotion census needed the same correction, and it carried a fact worth stating plainly. That document recorded the reason the `nym` module was declared in every build: the history's `exit` column had to name an `ExitNodeId`. Removing the column removed the reason, so nothing outside the `nym` feature names that type and the module's own gate is correct again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three preceding commits renamed the types and the accessor but left the old word everywhere a reader meets it first. Forty-seven references to `MixnetMode` survived in doc-comments, in intra-doc links that now resolved to nothing, and in the doc-test examples that `cargo test --doc` compiles, where they named a type and a method that no longer exist. `UnknownMixnetModeToken` follows its subject to `UnknownIndicatorToken`. `communication_mode` becomes `communications` across its thirty-two sites: a binding named for the retired vocabulary holding a value of type `Communications` said the rename had stopped halfway. The changelog records the whole rename set here, including the two public breaks the earlier commits made without an entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`start_interactive` returned nothing, so its caller hardcoded `ExitCode::SUCCESS` for every ending. A prompt that died because the terminal broke printed its error to stdout and then told the shell it had succeeded, which contradicts the contract `run_cli` states: the process exit code the session earned. Only the one-shot half had been given that treatment. It now returns `ExitCode`. A close the user asked for — `quit`, Ctrl-C, or Ctrl-D — earns success; a readline failure earns failure and reports on stderr. Two panics go with it: a prompt that cannot be constructed returns failure rather than unwinding past `main`'s error reporting, and a history entry that cannot be recorded warns and continues, because it costs the user recall and never the command they typed. `start_noninteractive` loses its `Result` for the same reason it never had one to give: no path in it constructs an `Err`. Both dispatch arms now read the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`update_current_price` recorded each quote into the wallet's price list and set `save_required`, so a read-only question dirtied the wallet and provoked a save. Nothing consumed the stored price: the two readers in `wallet/summary.rs` have been commented out, and the price the caller actually uses rides home in `MixnetPriceFetch`. The recorder had exactly one caller and goes with it. The price list stays serialized, so the wallet format is untouched and an existing list survives as loaded. That makes three commands wallet-free that `help` had filed as wallet commands. `info` and `change_server` reach only the indexer, and `current_price` now reaches only the mixnet, so all three move to the section for commands that need no wallet. The two tests covering those sections derive from `requires_wallet` rather than pinning a list, so they followed the change. The sections split on whether a wallet is needed, which leaves the online-versus-offline axis unrepresented: a reader still cannot see from `help` which commands the network bought. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rename of `CommunicationMode` to `Communications` swept only the code the default build compiles. The `offline_only` test module sits behind `#[cfg(not(feature = "nym"))]`, so its two references to the old name survived. Both the Cargo Hack Check job and the nym job then failed on `cargo check -p zingo-cli --no-default-features`, which is the first configuration that compiles that module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An ordinary `cargo check` compiles one feature combination, so a rename or a signature change can leave code behind a `#[cfg(feature = ...)]` gate broken and still look green on a developer's machine. CI catches it in the Cargo Hack Check job, eight minutes after the push. The preceding commit repaired exactly that class of breakage. The `feature-sweep` workbench binary runs CI's own check locally, scoped to the crates the branch touches. It reads the changed files against the merge base with `origin/dev`, maps each to the innermost crate that holds it, skips a virtual workspace manifest, and runs `cargo hack check --feature-powerset --lib --bins --tests` over the result. Pass `--all` for the whole workspace, `--base <ref>` for another comparison point, or crate directories to check those and no others. A missing cargo-hack is refused up front with the one command that installs it. The `makers feature-sweep` task calls the binary, the shared pre-push hook runs it last, and AGENTS.md names it among the Rust rules. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Boot guarantees one proven exit today, because only the Standing Client's birth fails closed; the sweep returns a bool and a session with no reachable indexer opens an indexerless prompt. Everything else pays later, most visibly a first price fetch measured at 30.7 seconds, of which the quote was under two. The ADR records the ruled remedy. Boot acquires the epoch's advertised exits, races four clients through the Sentinel, and assigns each proving exit a role in the order it confirms: IndexerSweep, PriceFetch, IndexerClient, and a spare. The role belongs to the exit rather than the client, so one client persists while four roles do, and the price is fetched during boot and printed rather than stored. Two mechanisms retire with it. The Clutch races four exits and keeps one, buying a real bootstrap hedge by discarding three-quarters of the work it starts; racing four births at boot hedges the same latency and banks every winner. The speed-priority redraw cannot tell a dead exit from a dead cohort, so it convicts an exit on the cohort's evidence. Four costs are recorded as accepted rather than left implicit: the lost indexerless prompt, an epoch of one exit per role and what an observer there can link, the dependency on NYM_EPOCH becoming a real boundary, and the still-undefined trigger for taking up the spare. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Renaming `CommunicationMode` to `Communications` shortened the type enough that rustfmt wants the assertion on one line, and cargo-checkmate lints the whole workspace with `cargo fmt --check`. The format job failed and cancelled its four siblings, so the run's five red jobs were one cause. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ADR 0045 left "starts failing" undefined, which was the one place the decision leaned on judgement it had not supplied. The discrimination it needs already exists and has simply never been pointed at exits: `socks5_transmit_stage` types every SOCKS5 failure by stage, and `charge_phase` turns a stage into a `FailurePhase`, where a `Tunnel` charge is the exit's by construction because the failure landed before the destination was reached. So the trigger mirrors the rule Health already applies to Correspondents, which CONTEXT.md states as "a tunnel-phase failure is the Exit Node's and never charges the Correspondent". A proven exit that takes `UNHEALTHY_FAILURE_THRESHOLD` tunnel-phase charges without an intervening success loses its role to the spare and its proof with it; one success redeems it, so a transient outage costs nothing. The Sentinel keeps the harder rule of one silence condemning. The asymmetry is in the evidence rather than the severity: only the exit can fail to answer the Sentinel, while ordinary traffic has a destination that can fail on its own account. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: