Skip to content

refactor(minimald): move minimald RPC types to own crate - #420

Merged
twitchyliquid64 merged 1 commit into
mainfrom
tom/minimald
Jun 15, 2026
Merged

refactor(minimald): move minimald RPC types to own crate#420
twitchyliquid64 merged 1 commit into
mainfrom
tom/minimald

Conversation

@twitchyliquid64

@twitchyliquid64 twitchyliquid64 commented Jun 15, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Should help with driving minimald from other crates.

Also removed the duplication in minvmd.

Summary by CodeRabbit

Release Notes

  • Refactor
    • Extracted RPC protocol definitions and message types into a dedicated shared crate, improving code organization and maintainability.
    • Refactored oneshot RPC server-side request handling logic for better separation of concerns and reusability across components.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 10e1c10d-84bc-4f5c-aced-d10ecda8f841

📥 Commits

Reviewing files that changed from the base of the PR and between 0600eb0 and 08f3a85.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (12)
  • Cargo.toml
  • crates/minimald-rpc/Cargo.toml
  • crates/minimald-rpc/src/lib.rs
  • crates/minimald/Cargo.toml
  • crates/minimald/src/connection.rs
  • crates/minimald/src/exec.rs
  • crates/minimald/src/rpc.rs
  • crates/minimald/src/session.rs
  • crates/minimald/src/sftp.rs
  • crates/minimald/src/test_harness.rs
  • crates/minvmd/Cargo.toml
  • crates/minvmd/tests/minimald_session_e2e.rs
📝 Walkthrough

Walkthrough

A new minimald-rpc workspace crate is introduced to hold the shared oneshot SSH RPC wire protocol: the OneshotSshRpc trait, RPC_SUBSYSTEM_PREFIX constant, Errorable<S> response wrapper, and all four concrete RPC message types. The minimald server's rpc.rs is refactored to import from this crate and replace per-RPC handle() methods with standalone serve_* async functions. All downstream import sites and the minvmd e2e test are updated accordingly.

Changes

Extract minimald-rpc shared protocol crate

Layer / File(s) Summary
Workspace and crate manifests
Cargo.toml, crates/minimald-rpc/Cargo.toml, crates/minimald/Cargo.toml, crates/minvmd/Cargo.toml		 Registers minimald-rpc as a workspace member and path dependency, defines the new crate manifest with constcat, serde, and sessions dependencies, and adds minimald-rpc to minimald runtime dependencies and minvmd dev-dependencies.
Shared RPC protocol contract
crates/minimald-rpc/src/lib.rs		 Defines RPC_SUBSYSTEM_PREFIX, the OneshotSshRpc trait, Errorable<S> (with unwrap/ok/err helpers and From<Result> conversion), and all four concrete RPC message types (GetVersion, ListSessions, GetSessionRecord, CreateSession) with their request/response payloads.
Server-side RPC dispatcher refactored
crates/minimald/src/rpc.rs, crates/minimald/src/connection.rs		 Removes local RPC type definitions and handle() methods, introduces the private ServeOneshot glue trait, adds standalone serve_get_version, serve_list_sessions, serve_get_session_record, and serve_create_session async functions, and updates handle_ssh_rpc and the connection subsystem prefix check to use minimald_rpc.
Consumer import updates
crates/minimald/src/exec.rs, crates/minimald/src/session.rs, crates/minimald/src/sftp.rs, crates/minimald/src/test_harness.rs		 Updates import paths in test modules to reference minimald_rpc instead of crate::rpc for CreateSession, CreateSessionRequest, and OneshotSshRpc.
minvmd e2e test updated
crates/minvmd/tests/minimald_session_e2e.rs		 Removes locally defined subsystem constant and payload structs, imports shared types from minimald_rpc, uses CreateSession::NAME for subsystem selection, and decodes responses via Errorable::ok().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • gominimal/minimal#374: Directly relates to the minimald_session_e2e end-to-end session flow that this PR migrates to use minimald_rpc protocol types.
  • gominimal/minimal#381: Corresponds to the CreateSession error-communication changes (mapping "already exists" into Errorable::Err) that this PR centralizes into the new shared crate.

Suggested reviewers

  • norrietaylor
  • evanspearman
  • 0chroma

Poem

🐇 Hopping through the crate tree with glee,
The RPC types now live separately free!
OneshotSshRpc stands tall and bright,
Errorable wraps responses just right.
No more duplication in sight—
One contract to rule them all tonight! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main refactoring: extracting RPC types from minimald into a dedicated minimald-rpc crate.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

@twitchyliquid64
twitchyliquid64 enabled auto-merge (rebase) June 15, 2026 17:02
@twitchyliquid64
twitchyliquid64 merged commit bfc61a8 into main Jun 15, 2026
32 checks passed
@twitchyliquid64
twitchyliquid64 deleted the tom/minimald branch June 15, 2026 17:13
This was referenced Jun 15, 2026
Merged
Merged
Merged
Merged
Merged
Merged
Merged
This was referenced Jun 23, 2026
Merged
Merged
Merged
This was referenced Jul 1, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evanspearman evanspearman evanspearman approved these changes
@0chroma 0chroma Awaiting requested review from 0chroma

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@twitchyliquid64 @evanspearman