feat(storage): default database and cache to OS per-user directories#550
feat(storage): default database and cache to OS per-user directories#550SantiagoDePolonia wants to merge 1 commit into
Conversation
When no path is configured and no legacy ./data (or ./.cache) directory exists, the SQLite database and local model cache now land in the platform-conventional locations (XDG dirs on Linux, ~/Library on macOS, %LocalAppData% on Windows) via the new internal/platformdir package. Existing deployments and the Docker image keep their current paths: the presence of ./data or ./.cache pins the historical defaults, so upgrades never move anyone's state. Startup now logs the resolved database path. This makes binary installs (install.sh, Homebrew) work from any working directory without scattering state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe change adds platform-specific data and cache directory helpers, updates SQLite and model-cache defaults to use them, preserves existing local directories, updates configuration wiring and startup logging, and revises related documentation and tests. ChangesPlatform-aware path resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Config
participant Storage
participant PlatformDir
Config->>Storage: request DefaultSQLitePath()
Storage->>Storage: check for ./data
Storage->>PlatformDir: request DataDir() when ./data is absent
PlatformDir-->>Storage: return per-user data directory
Storage-->>Config: return SQLite database path
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@internal/providers/init.go`:
- Around line 222-224: Update the `.cache` check in the provider initialization
flow to return the legacy path only when `os.Stat` succeeds and the resulting
file info from `Stat` satisfies `IsDir()`. If `.cache` is a regular file or
unavailable, preserve the existing fallback behavior.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 526e538a-53bd-46b7-affe-733f45ea37f3
📒 Files selected for processing (12)
.env.templateCLAUDE.mdconfig/config.goconfig/config_test.goconfig/storage.gointernal/app/app.gointernal/platformdir/platformdir.gointernal/platformdir/platformdir_test.gointernal/providers/init.gointernal/storage/default_path_test.gointernal/storage/sqlite.gointernal/storage/storage.go
| if _, err := os.Stat(".cache"); err == nil { | ||
| return ".cache" | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Only preserve .cache when it is a directory.
os.Stat also succeeds when .cache is a regular file. The subsequent filepath.Join(".cache", "models.json") then selects an invalid cache path and can prevent local-cache initialization. Check info.IsDir() before returning the legacy path.
Proposed fix
- if _, err := os.Stat(".cache"); err == nil {
+ if info, err := os.Stat(".cache"); err == nil && info.IsDir() {
return ".cache"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if _, err := os.Stat(".cache"); err == nil { | |
| return ".cache" | |
| } | |
| if info, err := os.Stat(".cache"); err == nil && info.IsDir() { | |
| return ".cache" | |
| } |
🤖 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 `@internal/providers/init.go` around lines 222 - 224, Update the `.cache` check
in the provider initialization flow to return the legacy path only when
`os.Stat` succeeds and the resulting file info from `Stat` satisfies `IsDir()`.
If `.cache` is a regular file or unavailable, preserve the existing fallback
behavior.
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Path is the database file path. Default: ./data/gomodel.db when a | ||
| // ./data directory exists, otherwise the OS per-user data directory | ||
| // (e.g. ~/.local/share/gomodel/gomodel.db). | ||
| Path string `yaml:"path" env:"SQLITE_PATH"` |
There was a problem hiding this comment.
.env.template now tells operators to set GOMODEL_SQLITE_PATH, but this tag still only reads SQLITE_PATH. With no YAML path, Load() ignores the documented variable and keeps the default path, so an explicit database location no longer wins as described by the PR and users can start against the wrong SQLite file.
Context Used: CLAUDE.md (source)
Artifacts
Repro: focused Go test exercising config.Load SQLite env override behavior
- Contains supporting evidence from the run (text/x-go; charset=utf-8).
Repro: verbose go test output showing GOMODEL_SQLITE_PATH ignored and SQLITE_PATH honored
- Keeps the command output available without making the summary code-heavy.
2 participants
Summary
When no path is configured and no legacy
./data(or./.cache) directory exists, the SQLite database and local model cache now default to the platform-conventional per-user locations, resolved by the newinternal/platformdirpackage:$XDG_DATA_HOME/gomodel/gomodel.db(~/.local/share/…)$XDG_CACHE_HOME/gomodel(~/.cache/…)~/Library/Application Support/gomodel/gomodel.db~/Library/Caches/gomodel%LocalAppData%\gomodel\gomodel.db%LocalAppData%\gomodel\cacheBackward compatibility
GOMODEL_SQLITE_PATH/GOMODEL_CACHE_DIR/ YAML paths win, unchanged../data(resp../.cache) pins the historical CWD-relative defaults — existing deployments and the Docker image (which pre-creates both directories, and whose volume mounts create their mount points) never move state on upgrade.storage configured … path=…) so it's always visible where state lives.Motivation
Binary installs (companion PR #549's
install.sh/Homebrew flow) run from arbitrary working directories; CWD-relative defaults would scatter databases wherever the user happened to launch from.Testing
platformdir(per-OS layout, XDG override, data≠cache) andDefaultSQLitePathlegacy-vs-platform resolution (t.Chdir).~/Library/Application Support/gomodel/, cache in~/Library/Caches/gomodel/; both paths logged.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
.cacheanddatadirectories when present.Documentation
Tests