Repository navigation
feat(minimal): smart session resolution for min attach and bare min - #875
Conversation
📝 WalkthroughWalkthroughThe CLI now supports bare ChangesSmart attach and default CLI flow
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
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/minimal/src/attach.rs (1)
28-40: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
canonicalizeruns synchronously inside async callers.
cwd_host_pathperforms a blockingstd::fs::canonicalizecall directly (nospawn_blocking). It's called fromresolve_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
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
Cargo.tomlcrates/minimal/Cargo.tomlcrates/minimal/src/attach.rscrates/minimal/src/lib.rscrates/minimal/src/loadouts.rscrates/minimal/tests/bug.rscrates/minimal/tests/cli.rscrates/minimal/tests/common/mod.rscrates/minimald-rpc/src/lib.rscrates/minimald/src/rpc.rscrates/minimald/src/sessions.rs
`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".
|
@coderabbitai pls stop summarizing my PRs when I've already done that for you |
|
Tip For best results, initiate chat on the files or code changes.
🐇 ✏️ Learnings added
|
…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.
|
you too @macroscopeapp don't edit my PR description when I've already given a good summary |
eebc6e7 to
75b8d7d
Compare
Closes #837.
min attachwith no session arg, and baremin(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-inputor no TTY).minactivates a new session when none exist, then attaches.project_path+statusto theListSessionswire type so the client can match by cwd and render a state glyph without a per-sessionGetSessionRecordround-trip.Test coverage
crates/minimal/src/attach.rs).--no-inputerror (crates/minimal/tests/cli.rs).cmd_attachshells out tossh; resolution logic is covered by unit tests instead.Not done
--latest/--pickflag variants deferred (YAGNI);--no-inputcovers the script case.Note
Add smart session resolution to
min attachand bareminminwith 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 attachwithout a session ID performs the same smart resolution instead of erroring.ListSessionsRPC now returnsproject_pathandstatusper session; deserialization is backward compatible with older daemons that omit these fields.--no-inputglobal flag disables interactive prompts; ambiguous sessions produce a structured error message listing candidates instead.mininvocations that previously errored now trigger a daemon connection and potentially a new session activation.Changes since #875 opened
global.no_inputflag support to bypass interactive prompts inlib::offer_mfile_scaffold,lib::cmd_activateupload-root confirmation, andlib::cmd_activateloadout configuration handling [eebc6e7]attach::last_activityto compute the maximum oflast_stdoutandlast_stdintimestamps instead of preferringlast_stdout[eebc6e7]Macroscope summarized 75b8d7d.
Summary by CodeRabbit
New Features
minwithout a subcommand now smart-resolves a session for the current directory and automatically attaches or activates as needed.min attachcan auto-select the best matching session (interactive chooser when supported).--no-inputfor fully non-interactive, deterministic behavior.Bug Fixes
Tests
--no-inputbehavior.