feat!: coalesce default dir computation across crates by twitchyliquid64 · Pull Request #619 · gominimal/minimal

twitchyliquid64 · 2026-07-02T17:06:14Z

Minimal cache dir: Use XDG_CACHE_DIR/minimal fallback ~/.cache/minimal
Minimal state dir: Use XDG_STATE_DIR/minimal fallback ~/.local/state/minimal

This is technically a behavior change for legacy mip, which would use ~/.cache/minimal unconditionally, but now respects XDG_CACHE_DIR if set. But given we are doing a pivot to Minimal One, now is the right time for that.

Summary by CodeRabbit

New Features
- Added standardized default locations for the daemon’s state and cache under dedicated “minimal” directories, improving consistent setup across platforms.
- Updated both the daemon and client to use these shared defaults for socket and directory resolution.
Bug Fixes
- Improved robustness when platform directory discovery fails by relying on consistent fallback behavior.
- Enhanced handling of user-supplied directory paths with clearer error messages instead of less descriptive failures.

coderabbitai · 2026-07-02T17:06:35Z

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: 2dc5f8fd-41bf-4803-9352-48783d4d99fc

📥 Commits

Reviewing files that changed from the base of the PR and between fbb4af7 and d0eb6d6.

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (7)

crates/mctx/Cargo.toml
crates/mctx/src/config.rs
crates/minimal/src/client.rs
crates/minimald/Cargo.toml
crates/minimald/src/main.rs
crates/paths/Cargo.toml
crates/paths/src/lib.rs

💤 Files with no reviewable changes (1)

crates/minimald/Cargo.toml

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

crates/paths/Cargo.toml
crates/minimald/src/main.rs
crates/paths/src/lib.rs
crates/minimal/src/client.rs
crates/mctx/src/config.rs
crates/mctx/Cargo.toml

📝 Walkthrough

Walkthrough

Adds shared helpers for minimal state and cache directories in paths, then updates mctx, minimal, and minimald to use them and adjust their dependency manifests.

Changes

Minimal directory defaults centralization

Layer / File(s)	Summary
Add minimal_cache_dir/minimal_state_dir helpers `crates/paths/src/lib.rs`, `crates/paths/Cargo.toml`	New public functions return `<cache>/minimal` and `<state>/minimal` through a shared `default_dir` helper using `dirs` with a home-dir fallback.
mctx ConfigBuilder uses paths helpers `crates/mctx/src/config.rs`, `crates/mctx/Cargo.toml`	`ConfigBuilder::build` now derives minimal state/cache defaults from `paths::minimal_state_dir()` and `paths::minimal_cache_dir()`, and the manifest swaps `dirs` for `paths`.
minimal client socket path resolution `crates/minimal/src/client.rs`	`resolve_socket_path` now builds the native socket path from `paths::minimal_state_dir()` instead of inline `dirs` fallback logic.
minimald CLI directory resolution `crates/minimald/src/main.rs`, `crates/minimald/Cargo.toml`	`Cli::minimal_state_dir()` and `Cli::minimal_cache_dir()` now use `paths` defaults, switch `.unwrap()` to `.expect()`, remove `Utf8PathBuf`, and update dependencies.
Estimated code review effort: 2 (Simple)	~15 minutes

Possibly related PRs

gominimal/minimal#292: Also updates mctx::ConfigBuilder state/cache directory configuration in crates/mctx/src/config.rs.
gominimal/minimal#338: Also threads minimal_cache_dir/minimal_state_dir through crates/minimald session logic.
gominimal/minimal#400: Also modifies crates/minimald/src/main.rs minimal state/cache directory computation.

Suggested reviewers: norrietaylor, evanspearman

Poem

A rabbit hopped through paths anew,
No more dirs fallback askew,
One helper now for state and cache,
Across three crates, a tidy stash,
Thump thump — the directories align! 🐇📁

🚥 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 accurately summarizes the main change: centralizing default directory computation across crates.
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

Actionable comments posted: 3

🧹 Nitpick comments (1)

crates/mctx/src/config.rs (1)
156-167: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicated path-conversion chain.

.as_utf8_path().as_std_path().to_path_buf() is repeated here and in crates/minimal/src/client.rs. Consider a small convenience method (e.g. DaemonAbsPath::into_path_buf()) in the paths crate to remove the repetition.
🤖 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/mctx/src/config.rs` around lines 156 - 167, The path conversion chain
is duplicated in the `Config` fallback logic and in `minimal::client`, so
extract it into a shared convenience on `DaemonAbsPath` in the `paths` crate
(for example an `into_path_buf()` helper). Update the `minimal_state_dir` and
`minimal_cache_dir` initializers in `Config` to call that helper, and similarly
switch the `crates/minimal/src/client.rs` call site to use the shared method
instead of repeating `.as_utf8_path().as_std_path().to_path_buf()`.

🤖 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/mctx/src/config.rs`:
- Around line 156-167: The `ConfigBuilder::build` fallback for
`minimal_state_dir` and `minimal_cache_dir` currently calls
`paths::minimal_state_dir()` / `paths::minimal_cache_dir()`, which can panic and
bypass the `Result<Config, ConfigError>` contract. Update `build()` to avoid any
panicking path by resolving these directories through a fallible flow and
propagating failures as `ConfigError` instead; use the existing `ConfigBuilder`
and `ConfigError` types so callers still get an error instead of a crash.

In `@crates/minimal/src/client.rs`:
- Around line 165-171: `resolve_socket_path` in `client.rs` should preserve its
`std::io::Result<PathBuf>` contract instead of panicking through
`paths::minimal_state_dir()`. Restore the previous error propagation by handling
the case where neither the state dir nor home dir can be resolved and returning
a `NotFound`-style `Err` from `resolve_socket_path`, so callers can handle the
failure instead of crashing; use the existing `resolve_socket_path` and
`paths::minimal_state_dir`/`default_dir` paths to locate the change.

In `@crates/paths/src/lib.rs`:
- Around line 147-161: default_dir() currently panics on home-dir lookup and
path validation, which breaks the Result-based flow used by minimal_cache_dir(),
minimal_state_dir(), resolve_socket_path(), and ConfigBuilder::build(). Update
default_dir() to return a Result using the crate’s Error type instead of calling
expect, and propagate failures from dirs::home_dir(),
Utf8PathBuf::from_path_buf(), and DaemonAbsPath::try_new(). Then adjust the
minimal_* helpers to bubble that Result up so callers like client.rs and
config.rs can handle the error, while any CLI boundary that wants to abort can
still unwrap/expect there.

---

Nitpick comments:
In `@crates/mctx/src/config.rs`:
- Around line 156-167: The path conversion chain is duplicated in the `Config`
fallback logic and in `minimal::client`, so extract it into a shared convenience
on `DaemonAbsPath` in the `paths` crate (for example an `into_path_buf()`
helper). Update the `minimal_state_dir` and `minimal_cache_dir` initializers in
`Config` to call that helper, and similarly switch the
`crates/minimal/src/client.rs` call site to use the shared method instead of
repeating `.as_utf8_path().as_std_path().to_path_buf()`.

🪄 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: 627f98f8-8e6d-4847-8328-bad4b69122ba

📥 Commits

Reviewing files that changed from the base of the PR and between fb618f9 and fbb4af7.

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (7)

crates/mctx/Cargo.toml
crates/mctx/src/config.rs
crates/minimal/src/client.rs
crates/minimald/Cargo.toml
crates/minimald/src/main.rs
crates/paths/Cargo.toml
crates/paths/src/lib.rs

💤 Files with no reviewable changes (1)

crates/minimald/Cargo.toml

twitchyliquid64 requested review from 0chroma, bryan-minimal, evanspearman, msample and norrietaylor July 2, 2026 17:06

coderabbitai Bot reviewed Jul 2, 2026

View reviewed changes

Comment thread crates/mctx/src/config.rs

Comment thread crates/minimal/src/client.rs

Comment thread crates/paths/src/lib.rs

twitchyliquid64 enabled auto-merge (squash) July 2, 2026 17:26

feat: coalesce default dir computation across crates

d0eb6d6

twitchyliquid64 force-pushed the tom/mip branch from fbb4af7 to d0eb6d6 Compare July 2, 2026 17:29

twitchyliquid64 changed the title ~~feat: coalesce default dir computation across crates~~ feat!: coalesce default dir computation across crates Jul 2, 2026

msample approved these changes Jul 2, 2026

View reviewed changes

twitchyliquid64 merged commit d6a4e49 into main Jul 2, 2026
28 checks passed

twitchyliquid64 deleted the tom/mip branch July 2, 2026 23:22

gominimal-aw-bot Bot mentioned this pull request Jul 31, 2026

fix: correct --minimal-dir help to name the state dir and per-platform defaults #1136

Merged

Navigation Menu

Uh oh!

feat!: coalesce default dir computation across crates - #619

feat!: coalesce default dir computation across crates#619
twitchyliquid64 merged 1 commit into
mainfrom
tom/mip

twitchyliquid64 commented Jul 2, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

twitchyliquid64 commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

twitchyliquid64 commented Jul 2, 2026 •

edited

Loading

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading