8000 feat(minimal): smart session resolution for `min attach` and bare `min` by 0chroma · Pull Request #875 · gominimal/minimal · GitHub
[go: up one dir, main page]

Skip to content

feat(minimal): smart session resolution for min attach and bare min - #875

Merged
0chroma merged 4 commits into
mainfrom
0chroma/feat-smart-attach-837
Jul 22, 2026
Merged

0chroma merged 4 commits into
mainfrom
0chroma/feat-smart-attach-837

Conversation

@0chroma
@0chroma 0chroma commented Jul 21, 2026 •
Copy link
Copy Markdown
Contributor

Closes #837.

  • min attach with no session arg, and bare min (no subcommand), now resolve a session from the current working directory instead of erroring — auto-attach when unambiguous, interactive fuzzy picker (most-recently-active first, ●/◯ state glyphs) when ambiguous, and a clean error listing candidates in non-interactive contexts (--no-input or no TTY).
  • Bare min activates a new session when none exist, then attaches.
  • Adds project_path + status to the ListSessions wire type so the client can match by cwd and render a state glyph without a per-session GetSessionRecord round-trip.

Test coverage

  • 8 unit tests for the resolution rules and row formatting (crates/minimal/src/attach.rs).
  • 2 integration tests: no-sessions error and ambiguous --no-input error (crates/minimal/tests/cli.rs).
  • The single-session happy path resolves but isn't end-to-end tested because cmd_attach shells out to ssh; resolution logic is covered by unit tests instead.

Not done

  • --latest/--pick flag variants deferred (YAGNI); --no-input covers the script case.

Note

Add smart session resolution to min attach and bare min

  • Running min with no subcommand now attaches to the session matching the current working directory, opens an interactive fuzzy picker when ambiguous, or activates a new session if none exist.
  • min attach without a session ID performs the same smart resolution instead of erroring.
  • The ListSessions RPC now returns project_path and status per session; deserialization is backward compatible with older daemons that omit these fields.
  • A new --no-input global flag disables interactive prompts; ambiguous sessions produce a structured error message listing candidates instead.
  • Risk: bare min invocations that previously errored now trigger a daemon connection and potentially a new session activation.

Changes since #875 opened

  • Added global.no_input flag support to bypass interactive prompts in lib::offer_mfile_scaffold, lib::cmd_activate upload-root confirmation, and lib::cmd_activate loadout configuration handling [eebc6e7]
  • Changed attach::last_activity to compute the maximum of last_stdout and last_stdin timestamps instead of preferring last_stdout [eebc6e7]

Macroscope summarized 75b8d7d.

Summary by CodeRabbit

  • New Features

    • Running min without a subcommand now smart-resolves a session for the current directory and automatically attaches or activates as needed.
    • min attach can auto-select the best matching session (interactive chooser when supported).
    • Session listings now include project path and status.
    • Added --no-input for fully non-interactive, deterministic behavior.
  • Bug Fixes

    • Clear, deterministic errors for zero sessions, ambiguous matches, and interactive cancellation.
  • Tests

    • Expanded coverage for smart attach resolution and the new --no-input behavior.

@coderabbitai
coderabbitai Bot commented Jul 21, 2026 •
Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The CLI now supports bare min and min attach, resolving sessions by the effective working directory, selecting interactively when ambiguous, and reporting deterministic errors without input. Session list RPC metadata includes project paths and statuses with backward-compatible defaults.

Changes

Smart attach and default CLI flow

Layer / File(s) Summary
Session metadata contract
crates/minimald-rpc/src/lib.rs, crates/minimald/src/sessions.rs, crates/minimald/src/rpc.rs
Session list responses include project paths and statuses, with serde defaults for older payloads and updated daemon tests.
Cwd matching and picker behavior
crates/minimal/src/attach.rs
Attach resolution matches sessions to the effective cwd, formats status-aware candidates, supports TTY selection, and reports ambiguous non-interactive choices.
Bare min and optional attach targets
Cargo.toml, crates/minimal/Cargo.toml, crates/minimal/src/lib.rs
The CLI adds the picker dependency, makes command and session arguments optional, adds --no-input, and routes default and attach flows through smart resolution.
CLI integration coverage and deterministic fixtures
crates/minimal/tests/cli.rs, crates/minimal/tests/common/mod.rs, crates/minimal/tests/bug.rs, crates/minimal/src/loadouts.rs
Tests cover empty and ambiguous resolution, with helpers creating sessions at explicit project paths and updated argument fixtures.

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

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant MinimalCLI
  participant Minimald
  participant Inquire
  User->>MinimalCLI: run bare min or min attach
  MinimalCLI->>Minimald: list sessions
  Minimald-->>MinimalCLI: return session metadata
  MinimalCLI->>MinimalCLI: match sessions to effective cwd
  MinimalCLI->>Inquire: pick ambiguous candidates
  Inquire-->>MinimalCLI: return selected session
  MinimalCLI->>Minimald: attach to resolved session
Loading

Possibly related PRs

Suggested reviewers: norrietaylor, twitchyliquid64

Poem

A bunny hops through cwd bright,
Finds one session tucked just right.
If many paths begin to glow,
A picker helps the choice to show.
Bare min now joins the flow! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy #837 by adding cwd-based auto-attach, an interactive picker, bare-min fallback activation, non-interactive handling, and session metadata.
Out of Scope Changes check ✅ Passed No clearly unrelated code changes are evident; the added tests and prompt updates support the smart-attach and --no-input workflow.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title is concise, conventional, and accurately summarizes the main smart-session-resolution change.
Description check ✅ Passed It has a clear summary and testing section, and only the checklist/template headings are missing.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

Comment thread crates/minimal/src/attach.rs Resolved
Comment thread crates/minimal/src/lib.rs Resolved
Comment thread crates/minimald-rpc/src/lib.rs Resolved
@0chroma
0chroma marked this pull request as ready for review July 21, 2026 23:19
@coderabbitai coderabbitai Bot left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
crates/minimal/src/attach.rs (1)

28-40: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

canonicalize runs synchronously inside async callers.

cwd_host_path performs a blocking std::fs::canonicalize call directly (no spawn_blocking). It's called from resolve_smart_attach, an async fn, so a stalled mount (e.g. unresponsive NFS/network path) blocks the tokio worker for the duration of the call. Low practical risk for a one-shot CLI command, but inconsistent with the repo's established pattern of offloading fs work from async contexts.

Based on learnings, "avoid blocking filesystem work in async contexts... run that work in a blocking thread via tokio::task::spawn_blocking."

♻️ Optional refactor
-pub(crate) fn cwd_host_path(
+pub(crate) async fn cwd_host_path(
     global: &crate::GlobalArgs,
 ) -> Result<paths::HostAbsPath, anyhow::Error> {
     let raw = match &global.repo_dir {
         Some(dir) => dir.clone(),
         None => std::env::current_dir().context("determine current working directory")?,
     };
-    let canonical = std::fs::canonicalize(&raw)
-        .with_context(|| format!("cannot resolve path '{}'", raw.display()))?;
+    let canonical = tokio::task::spawn_blocking(move || std::fs::canonicalize(&raw))
+        .await
+        .context("canonicalize task panicked")?
+        .with_context(|| "cannot resolve current working directory")?;
     let utf8 = camino::Utf8PathBuf::from_path_buf(canonical)
         .map_err(|_| anyhow::anyhow!("current working directory is not valid UTF-8"))?;
     paths::HostAbsPath::try_new(utf8).context("invalid current working directory")
 }
🤖 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/minimal/src/attach.rs` around lines 28 - 40, Update the async attach
path around cwd_host_path and its resolve_smart_attach caller so filesystem
resolution, including std::fs::canonicalize, runs inside
tokio::task::spawn_blocking rather than on the Tokio worker. Preserve the
existing path selection, UTF-8 validation, error context, and returned
HostAbsPath behavior, propagating any blocking-task errors appropriately.

Source: Learnings

🤖 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/minimal/src/lib.rs`:
- Around line 602-607: Update the activation prompt guards in cmd_activate to
treat global.no_input as non-interactive for the policy, scaffold, and non-VCS
upload prompts, alongside the existing TTY checks. Ensure --no-input
consistently selects the no-prompt behavior even when a terminal is attached.

---

Nitpick comments:
In `@crates/minimal/src/attach.rs`:
- Around line 28-40: Update the async attach path around cwd_host_path and its
resolve_smart_attach caller so filesystem resolution, including
std::fs::canonicalize, runs inside tokio::task::spawn_blocking rather than on
the Tokio worker. Preserve the existing path selection, UTF-8 validation, error
context, and returned HostAbsPath behavior, propagating any blocking-task errors
appropriately.
🪄 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: ba90c064-671f-4820-b737-fa6ec992a431

📥 Commits

Reviewing files that changed from the base of the PR and between 81011b5 and 6c22ec2.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (11)
  • Cargo.toml
  • crates/minimal/Cargo.toml
  • crates/minimal/src/attach.rs
  • crates/minimal/src/lib.rs
  • crates/minimal/src/loadouts.rs
  • crates/minimal/tests/bug.rs
  • crates/minimal/tests/cli.rs
  • crates/minimal/tests/common/mod.rs
  • crates/minimald-rpc/src/lib.rs
  • crates/minimald/src/rpc.rs
  • crates/minimald/src/sessions.rs

Comment thread crates/minimal/src/lib.rs Resolved
0chroma added 3 commits July 21, 2026 23:44
`min attach` with no session argument and bare `min` (no subcommand)
now resolve a session from the current working directory instead of
erroring. When exactly one session matches the cwd (or only one
exists), attach directly; when ambiguous, open an interactive fuzzy
picker ordered by recent activity; in non-interactive contexts
(--no-input or no TTY), error with a list of candidates. Bare `min`
activates a new session when none exist, then attaches.

Surfaces project_path and status on the ListSessions wire type so the
client can match by cwd and render a state glyph without a
per-session GetSessionRecord round-trip.

Closes #837.
Make ListSessionsEntry.project_path optional with #[serde(default)]
so a new client accepts responses from daemons that predate the
field (entries with None project_path never match cwd but remain
listable and pickable). Propagate --repo-dir (-C) and --no-input
through bare `min`'s activation path so `min -C /path` activates for
/path (not the process cwd) and `min --no-input` skips the policy
prompt. Trim inquire to crossterm+fuzzy features only.
With crossterm in raw mode, Ctrl-C is captured by inquire as
InquireError::OperationInterrupted rather than delivered as SIGINT,
and prompt_skippable only catches Esc (OperationCanceled). Without
explicit handling the user saw "session picker: The operation was
interrupted by the user" instead of a clean abort. Map Ctrl-C to
Ok(None) so it behaves like Esc — the caller surfaces
"session selection cancelled".
@0chroma
0chroma commented Jul 22, 2026
Copy link
Copy Markdown
Contributor Author

@coderabbitai pls stop summarizing my PRs when I've already done that for you

@coderabbitai
coderabbitai Bot commented Jul 22, 2026
Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@0chroma Understood—when you provide a PR summary, I’ll use it as context rather than restating it. I’ll focus review output on actionable findings and only add context where it is necessary to explain one.

🐇


✏️ Learnings added
Learnt from: 0chroma
URL: https://github.com/gominimal/minimal/pull/875

Timestamp: 2026-07-22T06:47:18.031Z
Learning: For PRs in the gominimal/minimal repository, when the author has supplied a PR summary, do not repeat or re-summarize it in CodeRabbit responses; use it as review context and focus output on actionable findings.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

…er ordering

Gate the mfile scaffold, non-VCS upload confirmation, and policy prompt
in cmd_activate on global.no_input so --no-input consistently skips all
interactive prompts, not just the session picker. Also fix
last_activity to pick the max of last_stdout/last_stdin instead of
short-circuiting on whichever field is listed first — a session that
received input after its last output now sorts first in the picker as
advertised.
@0chroma
0chroma commented Jul 22, 2026
Copy link
Copy Markdown
Contributor Author

you too @macroscopeapp don't edit my PR description when I've already given a good summary

@0chroma
0chroma enabled auto-merge (squash) July 22, 2026 06:51
@0chroma
0chroma force-pushed the 0chroma/feat-smart-attach-837 branch from eebc6e7 to 75b8d7d Compare July 22, 2026 07:05
@0chroma
0chroma merged commit 6d239bc into main Jul 22, 2026
29 checks passed
@0chroma
0chroma deleted the 0chroma/feat-smart-attach-837 branch July 22, 2026 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feature: smart min attach / bare min — auto-resolve or interactive session picker

2 participants

0