Skip to content

refactor(core): one RpcError constructor surface, one management dial, typed transport facts - #4056

Merged
nick-potts merged 7 commits into
mainfrom
t3/core-errors-transport
Nov 10, 2026
Merged

nick-potts merged 7 commits into
mainfrom
t3/core-errors-transport

Conversation

@nick-potts

Copy link
Copy Markdown
Contributor

Why

Batch 1, chunk A of the core untangle (goal: pieces that change together live together, behavior unchanged, so later agent changes read less, break less silently and debug faster).

Four things were spelled many times and could drift apart:

  • RpcError was built as a struct literal at about 360 non-test sites, with local helpers (closed, invalid_argument, internal, ...) re-spelling the same codes.
  • Three management dials (connect_management, dial_tunnel, open_grant_registry) each built their own iroh endpoint, and the close codes 0x50/0x51/0x52 were bare numbers on both sides of the wire.
  • TransportError carried seven booleans that are not mutually exclusive, and ConnectError answered "was it replied, retryable, unreached" through six separate predicates.
  • sdk/prepare.rs read a Build refusal from prose even though the refusal carries a typed details.capability.

DESIGN.md bets touched

  • Bet 4 (partial results are outcomes): T3-F3a. The replied/unreplied distinction is what a bounded command reports. It now lives in one Facts table, and every row equals the answer the old predicates gave. No per-target detail was collapsed.
  • Bet 6 (machine-local authority): T3-F8 and T7-F12. The Management Client label is still what a Machine checks itself. kind() is derived from the label's text and keeps unknown labels as Other. Enforcement refusal is read from the Machine's own refusal capability.
  • Bet 8 (evidence over claims): T3-F3a and T7-F12. Only what the wire said is recorded. The unreached-only marker is now read as unreached, where the old code read it as unanswered. The wire never sends that marker alone, so no answer changes today.

No red flag is crossed. Wire formats are frozen (Stable promise): RPC error JSON, TARGET_UNANSWERED_KEY and the UNANSWERED/UNREACHED status text, the label text and the close code values are unchanged.

What changed

Four commits, one per fix, in order.

  1. T1-F1 RpcError gets one constructor surface in ployz-core/src/rpc.rs: new, one named constructor per RpcErrorCode, forbidden and approval_required over FORBIDDEN_CODE and APPROVAL_REQUIRED_CODE, and the builders with_message, with_details, with_cause, with_causes. caused and unanswered_target are rewritten over new. Every non-test literal in ployz, ployzd, ployz-store, ployz-sdk, ployz-core and ployz-testkit migrates, and the local helpers are deleted. The three struct-update sites use with_details. ployz-store/src/error.rs keeps its helper names; only its private error() body changed.
  2. T7-F12 build/remote.rs returns a Refused carrying by_enforcement, and check_build_capabilities returns CapabilityCheckError. sdk/prepare.rs reads RefusedByEnforcement from the capability and falls back to the BUILD_REFUSED_BY_ENFORCEMENT text for old daemons that send no details.capability. The constants stay as message text.
  3. T3-F8 One management dial (endpoint and dial in connect/management.rs) used by connect_management, dial_tunnel and open_grant_registry. SEGMENTATION_OFFLOAD and the three close codes live in ployz-core/src/domain/management_capability.rs, beside TUNNEL_FAILED, and both sides use them. ManagementClientLabel::kind() replaces the string checks in cluster.rs, handlers/cloud.rs, handlers/server/remove.rs and ployzd/src/machine/local_machine.rs.
  4. T3-F3a TransportError holds origin: Origin { Remote, Local, SessionClosed }, routed: Option<Routed { Unanswered, Unreached }>, budget_expired and the two bools that are not part of the exclusive-state problem. From<tonic::Status> is the single decoder. ConnectError::facts() is one exhaustive table that each consumer reads. The old predicates stay as thin delegates, and as the oracle in a_transport_errors_facts_match_the_flags_they_replaced.

A fifth commit renames the capability check's failure closure, and a sixth drops comments that restated the names they sat on.

Scope

In: the four fix ids below, in core/ only.

Out, on purpose:

  • T1-F1b (test-literal migration and the #[non_exhaustive] flip) is a later batch. Struct-literal construction still compiles.
  • The daemon's test-only "cloud" parse is left alone.
  • unanswered and detailed stay bools on TransportError.
  • The close codes sit beside TUNNEL_FAILED, not in a new ployz-core/src/management.rs.

Decision-log text

T1-F1 RpcError gets one constructor surface and non-test literals migrate.

In ployz-core add RpcError::new(code, message), one named constructor per existing RpcErrorCode (invalid_argument, not_found, ambiguous, unsupported, unavailable, conflict, internal, unauthenticated, confirmation_required), forbidden and approval_required over two consts now in core (FORBIDDEN_CODE, APPROVAL_REQUIRED_CODE) that replace connect.rs FORBIDDEN, approval.rs APPROVAL_REQUIRED and the two Store spellings, and the builders with_details(Value), with_cause(impl Error) (via error_chain::causes) and with_causes(Vec<String>). caused and unanswered_target are rewritten over new. Migrate every non-test literal in ployz, ployzd, ployz-store, ployz-sdk and ployz-testkit, delete the local helpers, and rewrite the three struct-update sites with with_details. ployz-store/src/error.rs keeps its helper names and signatures; only its private error() body changes. Test literals are left for F1b.

T7-F12 Enforcement refusal read by capability.

sdk/prepare.rs matches the *_REFUSED_BY_ENFORCEMENT prose even though the refusal carries a typed details.capability. Read the capability and keep the constants in ployz-core/src/lib.rs as the message text only. Old daemons without details.capability keep matching the prose through the existing constants.

T3-F8 One management dial.

connect/management.rs gets fn endpoint(secret) -> Endpoint and fn dial(endpoint, machine_key, alpn), used by connect_management, dial_tunnel and open_grant_registry. The shared piece is the base builder plus a named segmentation_offload_off const in ployz-core; each side keeps its own timeouts. The grant endpoint gains the offload setting it lacks (a behavior change, noted). The close codes (0x50 refused, 0x51 revoked, 0x52 cleared) become plain u32 consts in ployz-core, used by ployzd and the client. The Management Client label kind is added on ployz_core::ManagementClientLabel as kind(), replacing the string checks. No client-side enum is introduced.

T3-F3a Typed transport facts.

The seven TransportError flags are NOT mutually exclusive, so a single enum Ended is unsound. Replace the bools with TransportError { origin: Origin { Remote, Local, SessionClosed }, routed: Option<Routed { Unanswered, Unreached }>, budget_expired: bool }, whose constructors make the implied pairs impossible to write down wrongly, and keep From<tonic::Status> as the single decoder of the wire marker. ConnectError's retry, is_unreachable, unreached, routed_by_entry, target_unanswered, target_replied and unreplied matrices become one ConnectError::facts() -> Facts table, an exhaustive match in one place, each consumer reading a field. Frozen: TARGET_UNANSWERED_KEY and the UNANSWERED/UNREACHED status text are read unchanged. No new wire marker.

Tradeoffs

  • The facts table is bigger than any one predicate it replaces. The gain is that a new ConnectError variant fails to compile until its row is written.
  • The old predicates stay as delegates so about 60 call sites do not move in this PR. A later cleanup can inline them.
  • T3-F8 deliberately changes one behavior (below) instead of hiding it behind a flag.

Blast Radius

Mostly mechanical: 71 files, 1373 insertions, 1479 deletions, about 209 of the added lines in tests.

Machine behavior: one change. The grant registry endpoint (open_grant_registry) now turns UDP segmentation offload off, as the other two management dials already did. Nothing else alters what a Machine does: no daemon, Docker, WireGuard, ZFS, move or deploy path changed, and the daemon-side edits only swap bare close-code numbers and label string checks for the same values read from shared constants. The decision log asks for verify-server on a cloud-managed dial and a grant registry open. I did not run it. See Verification.

Residual, named once (Cloud security bar): none. Nothing here lets a compromised Cloud cross the bar. The kind() of a label is read on the Machine from the label it already holds.

Out-of-scope note: the comment above CLIENT_REFUSED and CLIENT_CLEARED in ployzd/src/management/mod.rs now repeats the shared constants' docs. I left it because trimming it is outside this chunk.

Verification

Focused local checks only. CI is the gate.

  • cargo clippy -p ployz-core -p ployz-store -p ployz-sdk -p ployz -p ployzd -p ployz-testkit --all-targets is clean at this head.
  • cargo test -p ployz connect:: (77 passed), cargo test -p ployzd management (52 passed) and cargo test -p ployz-core value:: (22 passed), run before the final rebase onto trunk.
  • New tests: the transport oracle, a status-decode table, one test that every other ConnectError keeps its facts, and a_management_client_label_names_its_kind.
  • Not run: verify-server for the grant registry dial. It is a one-setting change on the client's own socket, covered by the unit tests only in that the endpoint builds. A root decision whether it needs real Machines.

RpcError::new, a constructor per code, forbidden and approval_required over
two core constants, and with_message, with_details, with_cause and with_causes.
Non-test struct literals and the one-line helpers that wrapped them now call
these. The serialized form is unchanged.
@coderabbitai

coderabbitai Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: aad7b774-8ab0-4648-9c03-6fb515dca358

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@nick-potts
nick-potts marked this pull request as ready for review November 10, 2026 14:44
@nick-potts
nick-potts added this pull request to the merge queue Nov 10, 2026
RpcError constructors carry the wire sentence, and the struct literals they replaced were never scanned.
@nick-potts
nick-potts removed this pull request from the merge queue due to a manual request Nov 10, 2026
@nick-potts
nick-potts enabled auto-merge November 10, 2026 15:03
@nick-potts
nick-potts added this pull request to the merge queue Nov 10, 2026
Merged via the queue into main with commit a024a11 Nov 10, 2026
21 checks passed
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