feat(minimald): implement RenameSession RPC by twitchyliquid64 · Pull Request #436 · gominimal/minimal

twitchyliquid64 · 2026-06-17T01:23:12Z

$> echo "{\"id\": \"019eb7a0-36d9-7d41-8286-265d76e9b3ab\", \"new_name\": \"booooopp\"}" | \
    ssh -s  -o ProxyCommand="socat - UNIX-CONNECT:$XDG_STATE_DIR/minimal/providers/local-0/ssh.sock" \
                -o 'UserKnownHostsFile=/home/xxx/.local/state/minimal/providers/local-0/known_hosts' \
                local-0 minimald-v1-RenameSession
$>

Summary by CodeRabbit

Summary

New Features
- Added an SSH RPC to rename sessions by ID, including a new request/response contract.
- Renames are persisted to disk and immediately reflected in active session records.
Bug Fixes
- Improved consistency when renaming an in-use session so in-memory and stored session data stay in sync.
Behavior Changes
- Renaming a non-existent session returns an error.
- Renaming to an already-used name fails to prevent collisions.
Tests
- Added coverage for successful renames, unknown-session errors, and persistence/uniqueness.

coderabbitai · 2026-06-17T01:23:24Z

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: 29083697-763f-44fd-ab77-396077883c04

📥 Commits

Reviewing files that changed from the base of the PR and between 2d84f9f and 6c6b620.

📒 Files selected for processing (5)

crates/minimald-rpc/src/lib.rs
crates/minimald/src/rpc.rs
crates/minimald/src/session.rs
crates/minimald/src/sessions.rs
crates/sessions/src/store.rs

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

crates/minimald/src/sessions.rs
crates/minimald-rpc/src/lib.rs
crates/minimald/src/rpc.rs
crates/minimald/src/session.rs
crates/sessions/src/store.rs

📝 Walkthrough

Walkthrough

Adds a RenameSession RPC end-to-end: the sessions crate gains disk-level rename with collision detection and index persistence; the minimald session actor gains record-refresh messaging; the sessions manager gains a rename handler that propagates updates to running actors; and the RPC crate gains the RenameSession protocol types, server handler, routing, and tests.

Changes

RenameSession RPC

Layer / File(s)	Summary
Disk store rename foundation `crates/sessions/src/store.rs`	`SessionObject` trait gains `refresh_from_record` method and now requires `Clone`; `Loader` trait gains `rename` method returning `AlreadyExists` on name collision; `DiskSession` derives `Clone`; `flush` helper renamed to `flush_index` and `write_record` helper introduced for atomic `record.json` writes; `DiskLoader::rename` checks for name collisions, updates in-memory name index, writes updated record, and persists index; unit tests validate rename updates on-disk records, remaps name index, persists across reinitialization, rejects duplicate target names, and supports renaming unnamed sessions.
Session actor record refresh `crates/minimald/src/session.rs`	`SessionMessage` enum gains `RefreshRecord(Record)` and test-only `GetRecord` variants; `Session::handle_message` adds matching arms to refresh actor state from updated records and return in-memory record copy under test; `SessionHandle` gains `apply_record` and test-only `record()` to enable manager-side record propagation.
Sessions manager rename orchestration `crates/minimald/src/sessions.rs`	`ManagerMessage` enum gains `RenameSession(SessionId, String, Responder<()>)` variant; manager handler looks up session by ID in store (returns `NotFound` on miss), calls store rename, propagates updated record to running actor via `apply_record`, responds with `Ok(())`; `ManagerHandle::rename_session` public async API sends request to manager and returns `Result<(), SessionsError>`.
RPC contract, server handler, and tests `crates/minimald-rpc/src/lib.rs`, `crates/minimald/src/rpc.rs`	`RenameSessionRequest` (id, new_name), `RenameSessionResponse` (unit), and `RenameSession` with `OneshotSshRpc` impl (subsystem name `minimald-v1-RenameSession`, response type `Errorable<RenameSessionResponse>`) added to protocol crate; `serve_rename_session` handler calls `sessions_manager().rename_session` and maps results to `Errorable::Ok` or `Errorable::Err`; `RenameSession::NAME` routed in `handle_ssh_rpc` match and spawn dispatch; two Tokio tests verify rename updates running session state visible through `GetSessionRecord` and renaming unknown ID returns error containing "no session with ID".

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ServerRPC as RPC Handler
  participant Manager as Sessions Manager
  participant SessionActor as Session Actor
  participant Store as DiskLoader

  Client->>ServerRPC: SSH subsystem minimald-v1-RenameSession
  ServerRPC->>Manager: rename_session(id, new_name)
  Manager->>Store: rename(key, new_name)
  Store-->>Manager: Ok(()) or AlreadyExists
  alt Store Success
    Manager->>SessionActor: apply_record(updated_record)
    SessionActor-->>Manager: success
    Manager-->>ServerRPC: Ok(())
  else Store Failure
    Manager-->>ServerRPC: SessionsError
  end
  ServerRPC-->>Client: Errorable::Ok or Errorable::Err

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

gominimal/minimal#420: Introduced the shared minimald-rpc protocol contract and oneshot SSH RPC dispatch mechanism that this PR's RenameSession RPC extends directly.
gominimal/minimal#381: Both PRs extend the RPC error/response pattern using Errorable for structured error reporting in crates/minimald/src/rpc.rs.

Suggested reviewers

0chroma
evanspearman
norrietaylor

🐇 A session once wandered with no name to claim,
But now it can hop and be given a frame!
The index refreshed, the record rewritten,
Through actor and manager—no rabbit is bitten.
Rename away, little session—you're famous! 🎉

🚥 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 'feat(minimald): implement RenameSession RPC' directly and clearly describes the main change—adding a RenameSession RPC endpoint to minimald. This aligns with the changeset across all modified files.
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.}

coderabbitai

Actionable comments posted: 2

🤖 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/rpc.rs`:
- Line 172: The warning log in the serve_rename_session function is incorrectly
using GetSessionRecord::NAME instead of the proper RPC name for the rename
handler. Replace GetSessionRecord::NAME with the correct constant name that
represents the rename session RPC handler (likely RenameSession::NAME or
similar) to ensure the log message accurately reflects which RPC handler failed
and maintains proper RPC-specific monitoring.

In `@crates/sessions/src/store.rs`:
- Around line 224-229: The write_record function overwrites record.json
directly, which is not crash-safe. If a write fails or the process crashes, the
record can be corrupted or the in-memory name_to_id mapping can become out of
sync. Refactor the function to write to a temporary file first (using a temp
filename in the session_dir), then atomically rename that temporary file to the
final record.json location only after the write succeeds. This ensures that if
the write fails or crashes, the original record.json remains unchanged and the
name_to_id mapping is only updated after a successful write completes.

🪄 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: 6e4f8f2a-cbf3-4834-acc7-822f178a138f

📥 Commits

Reviewing files that changed from the base of the PR and between d4e8b7d and 2d84f9f.

📒 Files selected for processing (5)

crates/minimald-rpc/src/lib.rs
crates/minimald/src/rpc.rs
crates/minimald/src/session.rs
crates/minimald/src/sessions.rs
crates/sessions/src/store.rs

twitchyliquid64 requested review from 0chroma, evanspearman and norrietaylor June 17, 2026 01:23

coderabbitai Bot reviewed Jun 17, 2026

View reviewed changes

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

Comment thread crates/sessions/src/store.rs

feat(minimald): implement RenameSession RPC

6c6b620

twitchyliquid64 force-pushed the tom/rename branch from 2d84f9f to 6c6b620 Compare June 17, 2026 01:42

norrietaylor approved these changes Jun 17, 2026

View reviewed changes

twitchyliquid64 merged commit 01badc9 into main Jun 17, 2026
33 checks passed

twitchyliquid64 deleted the tom/rename branch June 17, 2026 03:31

This was referenced Jun 22, 2026

feat(minimald-rpc,minimald,minimal2): policy types, GetSessionPolicy RPC, and session policy CLI #521

Merged

feat(sessions): update session status #597

Merged

coderabbitai Bot mentioned this pull request Jul 14, 2026

refactor(sessions): Part 1 - store actor & plumbing #744

Merged

gominimal-aw-bot Bot added the needs-human An agent handed off; a human must act, then clear this label. label Jul 31, 2026

gominimal deleted a comment from gominimal-aw-bot Bot Jul 31, 2026

Navigation Menu

Uh oh!

feat(minimald): implement RenameSession RPC - #436

feat(minimald): implement RenameSession RPC#436
twitchyliquid64 merged 1 commit into
mainfrom
tom/rename

twitchyliquid64 commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

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

Uh oh!

Summary by CodeRabbit

Summary

Uh oh!

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

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

twitchyliquid64 commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading