Skip to content

Quick wins batch#35

Merged
maxholman merged 11 commits intomainfrom
chore/quick-wins
Feb 27, 2026
Merged

Quick wins batch#35
maxholman merged 11 commits intomainfrom
chore/quick-wins

Conversation

@maxholman
Copy link
Copy Markdown
Contributor

Quick wins batch

Scope

A grab-bag of small, self-contained improvements from TODO.md — code quality,
website copy, and minor UX fixes. Each item is independent.

1. Metrics field visibility

Make the seven AtomicU64 fields in crates/core/src/control/metrics.rs
private. The existing inc_*/dec_* methods are the correct API surface.

Impl notes: Only two call sites read fields directly — both in
crates/core/src/control/handler.rs: handle_stats() (line ~101) and
metrics() (line ~286). Add a snapshot/read method to Metrics and use it
from those two call sites.

2. TryFrom<ProtoNodeRole> error type

Replace type Error = String in crates/core/src/types.rs (line ~28) with a
proper NodeRoleError enum. Only one error case exists: RoleUnknown. A
design is already sketched in docs/tasks/10-type-system-improvements.md.

3. ExitNodeResponse construction boilerplate

Add a constructor to ExitNodeResponse (protobuf-generated, defined in
crates/wire/proto/data.proto lines 136-146). The pair: Some(…), response: Some(…) pattern is repeated ~20 times across crates/core/src/exit/orchestrator.rs
and crates/exit-adapter/src/adapter.rs. A simple fn new(pair: Option<SocketAddressPair>, response: Response) -> Self that wraps response
in Some(…) covers all cases. One test uses ..Default::default() — leave
that alone.

4. Website stale benchmark claim

website/src/content/docs/index.mdoc line ~30 claims "2.4 Gbps" but the
actual benchmark table in benchmarks.mdoc shows TCP maxing at ~1.5 Gbps.
Replace with "multi-gigabit" or similar. Also check the packet-loss claim on
line ~32 ("7.7 Mbps" / "17 Mbps") against the benchmark data.

6. Fix "ping the daemon" description

Two locations:

  • crates/cli/src/repl.rs line ~196: "ping [<peer>]\tPing the daemon or a peer"
  • crates/cli/src/cli.rs line ~31: /// Ping the daemon or a peer.

The daemon-ping path (ipc.rs line ~215) returns status info (uptime, version,
role) — that's the info command, not ping. Peer ping returns "not yet
implemented". Fix the help text to say "Ping a peer". No-arg behavior should
auto-select the sole connected peer (the protobuf PingRequest.peer field
already supports empty = auto-select). If zero or multiple peers are connected,
return a clear error.

7. Audit #[allow(clippy::...)] suppressions

33 instances total. Categories:

  • Auto-generated code (5): wire/src/{control,data,management}.rs,
    cli/src/version.rs, daemon/src/lib.rs — all suppress doc_markdown/
    must_use_candidate/needless_raw_string_hashes on include!() blocks.
    Already commented or self-evident. Leave as-is.
  • too_many_lines (7): All marked // refactor candidate. Leave as-is
    (out of scope for this task).
  • cast_possible_truncation (6): Port u32→u16 casts from protobuf,
    millis as u64, ICMP ident. All safe in context. Add brief comments.
  • cast_precision_loss (2): usize as f64 for throughput display. Safe.
    Add brief comments.
  • missing_errors_doc (2): Crate-level in core and exit-adapter.
    Core already has a comment. Add comment to exit-adapter.
  • module_inception (2): client/mod.rs and server/mod.rs. Intentional
    structure. Add brief comments.
  • result_large_err (2): client/ws/mod.rs. Add comments explaining
    the error type.
  • needless_pass_by_value (1): entry-stack constructor. Already
    commented. Leave as-is.
  • struct_excessive_bools (1): CLI flags struct. Already commented.
    Leave as-is.
  • socket_set.rs crate-level cast_possible_truncation (1): Blanket
    suppression. Consider narrowing to specific sites or add a module-level
    comment.

8. Parameter naming consistency

12 renames across 4 files where minority impls diverge from the established
convention:

pairset (ExitAdapter trait / mock):

  • crates/core/src/exit/net/adapter.rs:116tcp_listen(pair → set
  • crates/core/tests/mock_exit.rs:27tcp_close(pair → set
  • crates/core/tests/mock_exit.rs:54tcp_listen(_pair → _set
  • crates/core/tests/mock_exit.rs:68udp_send(_pair → _set
  • crates/core/tests/mock_exit.rs:77icmp_session(_pair → _set
  • crates/core/tests/mock_exit.rs:85udp_recv_session(_pair → _set
  • crates/core/tests/mock_exit.rs:92tcp_recv_session(_pair → _set

bufdata (ExitAdapter::tcp_send trait def):

  • crates/core/src/exit/net/adapter.rs:103buf → data

dst_addrdest (RxSession::send impls):

  • crates/exit-adapter/src/sessions/tcp.rs:31_dst_addr → _dest
  • crates/exit-adapter/src/sessions/icmp.rs:88dst_addr → dest

argsconfig (Client::try_new QUIC impl):

  • crates/core/src/client/quic/mod.rs:70args → config

_ocsp_ocsp_response (ServerCertVerifier):

  • crates/core/src/tls/verifiers.rs:23_ocsp → _ocsp_response

Out of scope

  • Anything architectural (ConnectionManager decomposition, arc-swap, etc.)
  • New features (DNS transport, shell command, etc.)
  • Performance work (buffer pooling, lock contention)
  • Anything requiring new dependencies

Why

Low-risk improvements that tighten the type system, reduce boilerplate, fix
stale docs, and clean up suppressions. All quick to implement and review.

Notes

  • Items are independent — can be done in any order
  • Each item should be its own commit on the branch
  • Run just check after each item

maxholman and others added 11 commits February 27, 2026 18:05
Add REPL issues section to TODO, fix ping help text ("Ping a peer"
instead of "Ping the daemon or a peer"), replace specific throughput
numbers with general multi-gigabit language on the website, and
fix stale task reference in async-correctness doc.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Align with the src/dst naming convention used throughout the codebase.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Document the reasoning behind each #[allow(clippy::...)] so future
readers understand why the lint is suppressed. Also rename _ocsp →
_ocsp_response in TLS verifiers for clarity.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace String error in TryFrom<ProtoNodeRole> with a dedicated
NodeRoleError enum for better error handling at call sites.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The parameter is a configuration struct, not command-line arguments.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rename SocketPair/pair variables to SocketSet/set across the exit
adapter trait, orchestrator, and response types. Add ExitNodeResponse::new()
helper to reduce boilerplate in response construction. Also rename
buf → data in tcp_send for semantic clarity.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Match the established src/dst convention used throughout wire,
socket_set, entry-stack, and manager (42 existing usages). Renames
14 sites across 5 files.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The guard form clippy suggests would require cloning resp (moved into
send), so suppress with an explanatory comment.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@maxholman maxholman merged commit c797613 into main Feb 27, 2026
4 checks passed
@maxholman maxholman deleted the chore/quick-wins branch February 27, 2026 12:12
maxholman added a commit that referenced this pull request May 6, 2026
Sweep of website/ deps to latest within ranges, plus a vite downgrade
from 8 -> 7 to match astro's transitive vite (7.3.2) and avoid a
rolldown regression with @tailwindcss/vite 4.2.4.

Closes alerts #28 #29 #30 #31 #33 #34 #35 #36 #37 #38 #39 #40 #44 #48
covering vite, picomatch, postcss, yaml, astro, smol-toml.

- vite ^8.0.1 -> ^7.3.2 (drops the now-redundant vite 8 lineage; astro
  pulls 7.3.2 transitively, which is the patched version)
- astro 6.0.6 -> 6.2.2 (#44)
- @tailwindcss/vite 4.2.2 -> 4.2.4
- smol-toml: lockfile bump to 1.6.1 (#28)
- postcss: lockfile bump to 8.5.14 (#48)
- picomatch: lockfile bumps to 2.3.2 + 4.0.4 (#29 #30 #39 #40)
- yaml is now omitted entirely (it was an optional vite peer)

Verified: pnpm build succeeds; no @tailwindcss/vite peer-dep warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant