fix(client): Fix sessiongs getting in a bad state when creation is aborted by evanspearman · Pull Request #958 · gominimal/minimal

evanspearman · 2026-07-24T21:54:28Z

Problem

Aborting min activate during composition/permission gating — e.g. pressing Ctrl-C at a Package … wants to patch … prompt — left a session wedged on the daemon. It couldn't be re-activated (name already taken), couldn't be attached to ("awaiting its contribution verdict"), and only got cleaned up on a daemon restart. The session name was effectively occupied forever.

Root cause

At the gating prompt, dialoguer → console re-raises SIGINT to the process on Ctrl-C (console unix_term.rs → libc::raise(SIGINT)). min installed no SIGINT handler, so the default disposition killed the process mid-prompt — before the existing abort-cleanup (drive_pending_to_active → send_abort) could run. The daemon was left holding the session in Pending/Draft indefinitely.

(The in-menu "Abort activation" option was fine — it returns through the code and cleans up. It's specifically the raw Ctrl-C / any unclean client exit that orphaned the session.)

Changes

Client-side interrupt handling (crates/minimal/src/lib.rs)

New ActivationInterrupt guard, armed in cmd_activate once the session id is known and dropped once the session is Active. It registers a Ctrl-C handler (so the process survives console's re-raise) and, on interrupt, best-effort AbortSessions the in-flight session over a fresh connection, then exits 130. Disarming on success means a Ctrl-C during the --attach hand-off no longer tears down the now-good session.

Daemon-side reaping (crates/minimald/src/{connection,rpc,server}.rs) — the robust backstop

Connection now tracks the sessions created over it; serve_create_session records each one.
When a connection closes, reap_unfinalized_sessions deletes any of its sessions still Pending/Materializing, while leaving Active sessions (which legitimately outlive their creating connection) untouched. This covers all unclean-exit paths — Ctrl-C, crash, and network loss — not just the graceful one.

Testing

New end-to-end test dropping_a_connection_reaps_only_its_unfinalized_sessions: a Pending session is reaped when its creating connection closes, and an Active session created over the same connection survives.
Full minimald session suite green; cargo clippy clean on the changed crates.

Recovery for already-stuck sessions

Existing daemons without this fix can free a wedged name with min destroy (DestroySession has no Pending guard), or by restarting the daemon.

Summary by CodeRabbit

Bug Fixes
- Pressing Ctrl-C during an in-progress session activation now triggers a best-effort abort and exits with code 130, preventing half-started sessions from lingering.
- When a client disconnects early, any unfinalized sessions (e.g., Pending/Materializing) are now cleaned up automatically.
- Sessions that have already reached activation (Active) are no longer removed by Ctrl-C or teardown cleanup.
- Teardown cleanup failures are now handled gracefully without interrupting shutdown behavior.
Tests
- Added coverage to verify only unfinalized sessions are deleted on disconnect.

coderabbitai · 2026-07-24T21:54:47Z

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: 5f02316a-e58f-41af-9294-0a74be68079d

📥 Commits

Reviewing files that changed from the base of the PR and between fa8527f and 703d1ca.

📒 Files selected for processing (4)

crates/minimal/src/lib.rs
crates/minimald/src/connection.rs
crates/minimald/src/rpc.rs
crates/minimald/src/server.rs

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

crates/minimald/src/rpc.rs
crates/minimald/src/connection.rs
crates/minimal/src/lib.rs
crates/minimald/src/server.rs

📝 Walkthrough

Walkthrough

Activation now aborts pending sessions on Ctrl-C, while the daemon records sessions per connection and reaps unfinalized sessions when connections close. Finalized Active sessions are preserved, with coverage for selective teardown cleanup.

Changes

Session cleanup lifecycle

Layer / File(s)	Summary
Activation interrupt guard `crates/minimal/src/lib.rs`	Adds a Ctrl-C guard that best-effort aborts the pending session and exits with status 130, then disarms after successful finalization.
Connection-scoped session tracking `crates/minimald/src/connection.rs`, `crates/minimald/src/rpc.rs`	Tracks session IDs created over each connection and records them from the CreateSession RPC handler.
Connection teardown reaping `crates/minimald/src/server.rs`	Reaps Pending and Materializing sessions after connection closure, preserves Active sessions, logs cleanup failures, and tests the behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Daemon
  participant Connection
  participant SessionStore
  Client->>Daemon: CreateSession
  Daemon->>Connection: record_created_session(session_id)
  Client->>Daemon: Upload and finalize
  Daemon->>SessionStore: Mark session Active
  Client->>Connection: Disconnect or press Ctrl-C
  Connection->>Daemon: take_created_sessions()
  Daemon->>SessionStore: Re-read session status
  Daemon->>SessionStore: Delete unfinalized session

Possibly related issues

gominimal/inbox#355 — Covers aborting and reaping sessions when activation is interrupted during composition gating.
gominimal/minimal#948 — Covers aborting and cleaning up sessions interrupted during composition gating.

Possibly related PRs

gominimal/minimal#754 — Both modify daemon CreateSession handling and pending-session lifecycle.
gominimal/minimal#914 — Both cover the Materializing/finalization/attach activation lifecycle.

Suggested reviewers: twitchyliquid64, norrietaylor

Poem

I’m a bunny with a cleanup cue,
Ctrl-C hops, and pending sessions do too.
Active ones stay snug in their bed,
While stale little sessions are gently shed.
The daemon keeps order, ears held high!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is clearly related to the session-abort fix, though it has a typo and is a bit repetitive.
Description check	✅ Passed	The description covers the bug, fix, testing, and recovery, but it does not follow the template’s Summary and Checklist sections.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/minimald/src/rpc.rs (1)
172-213: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Track CreateSession before draining on disconnect. handle_ssh_rpc dispatches RPCs via drop(spawn(...)), so a disconnect can race between mngr.create_session(...) and conn.record_created_session(id).await. If session_fut.await reaches teardown first, take_created_sessions() can miss that id and leave the new session unreaped. A per-connection join/counter that teardown waits on would close the gap.
🤖 Prompt for 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.

In `@crates/minimald/src/rpc.rs` around lines 172 - 213, Update the CreateSession
handling around serve_create_session and the handle_ssh_rpc dispatch so session
creation is tracked before it can race with disconnect teardown. Add or reuse
per-connection in-flight RPC coordination, ensure teardown waits for the
CreateSession operation to finish before calling take_created_sessions(), and
only then record the created session ID so newly created sessions cannot be
missed or left unreaped.

🤖 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.

Outside diff comments:
In `@crates/minimald/src/rpc.rs`:
- Around line 172-213: Update the CreateSession handling around
serve_create_session and the handle_ssh_rpc dispatch so session creation is
tracked before it can race with disconnect teardown. Add or reuse per-connection
in-flight RPC coordination, ensure teardown waits for the CreateSession
operation to finish before calling take_created_sessions(), and only then record
the created session ID so newly created sessions cannot be missed or left
unreaped.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cba8ae9a-bed7-40d9-9997-f2b7f4201ee5

📥 Commits

Reviewing files that changed from the base of the PR and between 3a05252 and fa8527f.

📒 Files selected for processing (4)

crates/minimal/src/lib.rs
crates/minimald/src/connection.rs
crates/minimald/src/rpc.rs
crates/minimald/src/server.rs

…orted

norrietaylor approved these changes Jul 24, 2026

View reviewed changes

coderabbitai Bot reviewed Jul 24, 2026

View reviewed changes

fix(client): fix sessiongs getting in a bad state when creation is ab…

703d1ca

…orted

evanspearman force-pushed the evan/fixsigint branch from fa8527f to 703d1ca Compare July 24, 2026 22:14

evanspearman merged commit 0c5f2bc into main Jul 24, 2026
29 checks passed

evanspearman deleted the evan/fixsigint branch July 24, 2026 22:38

coderabbitai Bot mentioned this pull request Jul 31, 2026

feat(minimal): add min task run — declared tasks in ephemeral sessions #1139

Merged

Navigation Menu

Uh oh!

fix(client): Fix sessiongs getting in a bad state when creation is aborted - #958

fix(client): Fix sessiongs getting in a bad state when creation is aborted#958
evanspearman merged 1 commit into
mainfrom
evan/fixsigint

evanspearman commented Jul 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

evanspearman commented Jul 24, 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 Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

evanspearman commented Jul 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading