Skip to content

chore(rpc): guard against todo!() in dispatched RPC handlers (S02-2) - #9

Open
orrinfrazier wants to merge 1 commit into
mainfrom
fix/S02-2-todo-guard
Open

chore(rpc): guard against todo!() in dispatched RPC handlers (S02-2)#9
orrinfrazier wants to merge 1 commit into
mainfrom
fix/S02-2-todo-guard

Conversation

@orrinfrazier

@orrinfrazier orrinfrazier commented May 24, 2026

Copy link
Copy Markdown
Owner

Closes #19

Summary

  • Adds a syn-based #[cfg(test)] guard so a live-dispatched cuprated RPC handler cannot contain a reachable todo!()/unimplemented!() — preventing re-introduction of Cuprate Crash Starting RPC Server Cuprate/cuprate#522-class panics as the RPC surface is completed toward v1.0.0.
  • Today every todo!()-bearing handler except one is dead (dispatch routes it to not_available()); the guard locks that invariant in.
  • Implements Option A (an automated, CI-enforced test) from the S02-2 proposal rather than Option B (a human convention).

How it works

dispatched_todo_violations() parses each map_request match, collects the handlers that are actually dispatched (arms calling a handler fn other than not_available()), follows the call graph — including shared::/helper:: delegation — to a fixpoint taint set, and asserts the violation set exactly equals an ALLOWLIST.

The allowlist currently holds exactly ("json_rpc", "get_output_distribution") — the one real live-dispatched todo!() (it delegates to shared::get_output_distribution, tracked separately). Exact-equality is a two-way ratchet:

  • a newly-dispatched todo!() fails the test (regression guard), and
  • removing the last todo!() from an allowlisted handler also fails it (forces allowlist cleanup — so this test will go red when the get_output_distribution fix lands; delete the entry then).

Changes

  • binaries/cuprated/src/rpc/handlers/todo_guard.rs — new, test-only (#[cfg(test)]): the scanner + 11 tests.
  • binaries/cuprated/src/rpc/handlers.rs+ #[cfg(test)] mod todo_guard;
  • Cargo.toml / binaries/cuprated/Cargo.tomlsyn as a workspace dev-dependency (already in the lockfile → no new crates, cargo-deny unaffected).
  • No production handler code is modified.

Testing

  • 11 unit + integration tests, all green; full cuprated suite green (22 lib tests, 0 failures).
  • cargo clippy --all-targets --all-features -- -D warnings, cargo fmt --check, and cargo deny check bans licenses sources all clean.
  • Mutation test: temporarily wiring GetInfo to its todo!()-bearing get_info handler makes the guard fail with {(json_rpc, get_info), (json_rpc, get_output_distribution)} and an actionable message — confirming the guard catches the regression it exists to prevent.

Review notes

Reviewed via 3-way cross-family consensus (Claude + Gemini + Codex), which caught and fixed three latent false-negative gaps before merge: direct cross-module dispatch (shared::foo in an arm), helper.rs not being scanned, and the dispatch-match selection assuming the first match.

Add a `syn`-based `#[cfg(test)]` guard (S02-2) that scans the cuprated RPC
dispatch tables (`map_request` in handlers/{json_rpc,other_json,bin}.rs) and
fails if any *live-dispatched* handler transitively contains a reachable
`todo!()`/`unimplemented!()`. Today these todos are dead (dispatch routes them
to `not_available()`), but the guard prevents re-introducing Cuprate#522-class panics
as handlers are wired in toward v1.0.0.

The current allowlist holds exactly the one known live-dispatched todo,
`json_rpc::get_output_distribution` (tracked separately). Exact-equality makes
the allowlist a two-way ratchet: a newly-dispatched todo fails the test, and
removing the last todo from an allowlisted handler also fails it.

Test-only change: adds `syn` as a workspace dev-dependency (already in the
lockfile, no new crates) and `#[cfg(test)] mod todo_guard;`.
@orrinfrazier
orrinfrazier force-pushed the fix/S02-2-todo-guard branch from f827d83 to 72802d6 Compare July 28, 2026 23:17
@github-actions github-actions Bot added A-binaries A-workspace Area: Changes to a root workspace file or general repo file. A-dependency labels Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-binaries A-dependency A-workspace Area: Changes to a root workspace file or general repo file.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cuprated: guard against todo!() in dispatched RPC handlers

1 participant