feat(sessions): plumbing for create session changes by evanspearman · Pull Request #600 · gominimal/minimal

evanspearman · 2026-06-30T15:16:06Z

Plumbing for the unified CreateSession flow. The wire shape now
carries both the out-of-band session config and the client's Phase 1
contribution; the existing empty-contribution fast path is preserved
for internal callers (sftp, exec, session-recovery).

New SessionConfig { name, project_path, network, policy, attrs }
in minimald-rpc. username deliberately omitted — sourced from
the SSH connection context on the daemon side.
CreateSessionRequest becomes { config, contribution };
contribution is #[serde(default)] so internal callers don't
need to know about it.
CreateSessionResponse becomes an enum Ready { id } | Pending { id, response }. Only Ready is reachable today; Pending is
wire-defined so adding Phase 2 routing later isn't a wire change.
Manager actor reshapes ManagerMessage::CreateSession to carry
config + username + contribution + responder. Empty-contribution
path assembles a Record, validates policy, persists Active,
returns Ready { id } — byte-equivalent to the old behavior.
Dispatch reads ssh_username from the connection lock and threads
it to serve_create_session.
14 caller sites migrated. Test helpers (create_session_req,
unwrap_ready) centralized in test_harness.
SessionStatus gets a forward-compat rustdoc note about the
upload-driven state-machine evolution.

Summary by CodeRabbit

New Features
- Session creation now uses a more expressive request (session config plus contribution) and returns an explicit readiness state (Ready vs Pending), improving clarity for clients and examples.
Bug Fixes
- Session creation flows now correctly handle readiness: only Ready is accepted (treating Pending as an error).
- Attach flows now reliably use the newly created session ID.
Documentation
- Updated session composition documentation with the new request/response behavior and added forward-compatibility guidance for session status handling.

coderabbitai · 2026-06-30T15:16:46Z

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: 9ed41781-8c1a-4a42-bb56-9e50af57c0d1

📥 Commits

Reviewing files that changed from the base of the PR and between 79e68ad and 8dd0d66.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (13)

crates/minimal2/src/main.rs
crates/minimald-rpc/Cargo.toml
crates/minimald-rpc/src/lib.rs
crates/minimald/src/exec.rs
crates/minimald/src/rpc.rs
crates/minimald/src/session.rs
crates/minimald/src/sessions.rs
crates/minimald/src/sftp.rs
crates/minimald/src/test_harness.rs
crates/minvmd/examples/exec.rs
crates/minvmd/tests/minimald_session_e2e.rs
crates/sessions/docs/COMPOSITION.md
crates/sessions/src/lib.rs

✅ Files skipped from review due to trivial changes (2)

crates/sessions/docs/COMPOSITION.md
crates/sessions/src/lib.rs

🚧 Files skipped from review as they are similar to previous changes (10)

crates/minimald-rpc/Cargo.toml
crates/minvmd/tests/minimald_session_e2e.rs
crates/minimald/src/exec.rs
crates/minimald/src/sftp.rs
crates/minimald/src/test_harness.rs
crates/minimald/src/session.rs
crates/minimal2/src/main.rs
crates/minvmd/examples/exec.rs
crates/minimald-rpc/src/lib.rs
crates/minimald/src/rpc.rs

📝 Walkthrough

Walkthrough

The CreateSession RPC contract now uses SessionConfig plus WireContribution, and responses are tagged as Ready or Pending. The session manager, SSH RPC path, client callers, examples, tests, and documentation were updated to use the new shapes.

Changes

CreateSession RPC refactor

Layer / File(s)	Summary
RPC wire contract: SessionConfig and Ready/Pending response `crates/minimald-rpc/Cargo.toml`, `crates/minimald-rpc/src/lib.rs`	Adds `SessionConfig`, changes `CreateSessionRequest` to carry `config` plus `WireContribution`, changes `CreateSessionResponse` to `Ready`/`Pending`, adds the `paths` workspace dependency, and adds round-trip tests for the new request/response shapes.
Session manager actor: config-driven CreateSession handler `crates/minimald/src/sessions.rs`	Introduces `CreateSessionMsg`, updates `ManagerMessage::CreateSession`, builds `sessions::Record` from config and username, validates policy and contribution, persists the record, returns `Ready { id }`, and updates related tests.
SSH RPC handler: thread ssh_username into serve_create_session `crates/minimald/src/rpc.rs`	Reads `ssh_username` with the channel lock, threads it through `handle_ssh_rpc`, and passes it to `serve_create_session`, which forwards the new request fields to the session manager.
Test harness helpers: create_session_req and unwrap_ready `crates/minimald/src/test_harness.rs`	Adds shared helpers to build `CreateSessionRequest` values and extract `SessionId` from `CreateSessionResponse::Ready`.
Callers, examples, and docs updated for the new session flow `crates/minimal2/src/main.rs`, `crates/minimald/src/exec.rs`, `crates/minimald/src/session.rs`, `crates/minimald/src/sftp.rs`, `crates/minvmd/examples/exec.rs`, `crates/minvmd/tests/minimald_session_e2e.rs`, `crates/sessions/docs/COMPOSITION.md`, `crates/sessions/src/lib.rs`	Updates CLI and test callers to use SessionConfig-based requests and Ready/Pending matching, and updates composition and status documentation for the new session flow.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

gominimal/minimal#381: Updates around CreateSession response handling and error mapping in crates/minimald/src/rpc.rs are directly related to the RPC path refactor here.
gominimal/minimal#443: This PR extends the session composition flow by embedding WireContribution into CreateSessionRequest and adding Pending semantics.
gominimal/minimal#597: The session manager now constructs sessions::Record with explicit status handling, which this PR depends on when creating Active sessions.

Suggested reviewers

0chroma
norrietaylor

Poem

🐇 I hopped through config fields tonight,
With Ready shining soft and bright.
Pending waits in a future haze,
While session IDs now take new ways.

🚥 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 clearly describes the main change: plumbing updates for the create-session flow in sessions.
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.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/minimald/src/sessions.rs`:
- Around line 271-277: The CreateSessionMsg handling in sessions.rs is
discarding contribution unconditionally, which lets non-empty phase-1 data be
silently ignored while still returning Ready. Update the CreateSessionMsg /
session creation flow to validate contribution before persisting the session: if
contribution is not the default/empty value, return InvalidInput instead of
proceeding, and only allow it to pass once the Phase 2 path in the session
creation logic is ready to consume it.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef2d7e82-16e5-40ef-9584-85947f4e10ab

📥 Commits

Reviewing files that changed from the base of the PR and between d15539a and 79e68ad.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (13)

crates/minimal2/src/main.rs
crates/minimald-rpc/Cargo.toml
crates/minimald-rpc/src/lib.rs
crates/minimald/src/exec.rs
crates/minimald/src/rpc.rs
crates/minimald/src/session.rs
crates/minimald/src/sessions.rs
crates/minimald/src/sftp.rs
crates/minimald/src/test_harness.rs
crates/minvmd/examples/exec.rs
crates/minvmd/tests/minimald_session_e2e.rs
crates/sessions/docs/COMPOSITION.md
crates/sessions/src/lib.rs

feat(sessions): plumbing for create session changes

8dd0d66

coderabbitai Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread crates/minimald/src/sessions.rs Outdated

evanspearman force-pushed the evan/createsession-wire branch from 79e68ad to 8dd0d66 Compare June 30, 2026 15:47

evanspearman mentioned this pull request Jun 30, 2026

feat(sessions): tighten daemon composer pending/ready routing #602

Merged

twitchyliquid64 approved these changes Jun 30, 2026

View reviewed changes

evanspearman merged commit cacf329 into main Jun 30, 2026
42 checks passed

evanspearman deleted the evan/createsession-wire branch June 30, 2026 21:15

This was referenced Jul 10, 2026

feat(minimald): rip out arbitrary session exec #709

Merged

refactor: move all session logic into session actor, init loadouts in own RPC #754

Merged

Navigation Menu

Uh oh!

feat(sessions): plumbing for create session changes - #600

feat(sessions): plumbing for create session changes#600
evanspearman merged 1 commit into
mainfrom
evan/createsession-wire

evanspearman commented Jun 30, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 30, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

evanspearman commented Jun 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

evanspearman commented Jun 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 30, 2026 •

edited

Loading