fix(cli): unify settings.json + star-cache.json under ~/.config/tokscale on macOS#468
Conversation
…ale on macOS `Settings::config_path()` and `star_cache_path()` called `dirs::config_dir()`, which returns `~/Library/Application Support/` on macOS. That left settings.json and star-cache.json under Application Support while auth.rs, cursor.rs, and antigravity.rs (and all four READMEs) hardcoded `~/.config/tokscale/` — so macOS users editing the documented path silently changed a file tokscale never read. New `paths::get_config_dir()` helper resolves env override → macOS/Linux `~/.config/tokscale` → Windows platform default → `.tokscale` fallback. Both call sites switched to it. `Settings::load()` reads the new path first, then falls back to the legacy `~/Library/Application Support/tokscale/settings.json` on macOS only, so existing users keep their theme / scanner / defaultClients preferences after upgrading. `save()` always writes the new path. `TOKSCALE_CONFIG_DIR` env var added (mirrors `TOKSCALE_HEADLESS_DIR` semantics) and documented in all four README variants. Windows-Specific Configuration sections corrected: settings.json lands at `%APPDATA%\\tokscale\\` (the actual platform default), not `%USERPROFILE%\\.config\\tokscale\\`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
junhoyeo
left a comment
There was a problem hiding this comment.
Thanks for digging into this @yousiki! Real bug — the new defaultClients setting we just shipped in #464 was silently no-op on macOS for anyone who followed the README. Surgical fix, hermetic tests, graceful one-way migration that preserves existing macOS users' theme/scanner/defaultClients across the upgrade. Approving and squash merging into v2.1.0.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8f5b9e1f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
5 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="README.md">
<violation number="1" location="README.md:518">
P3: README overstates `TOKSCALE_CONFIG_DIR` as absolute-only, but code accepts any string value verbatim (including relative paths).</violation>
</file>
<file name="README.ja.md">
<violation number="1" location="README.ja.md:501">
P2: `TOKSCALE_CONFIG_DIR` documentation is incomplete: it affects shared config files (including `star-cache.json`), not only `settings.json`.</violation>
</file>
<file name="crates/tokscale-cli/src/paths.rs">
<violation number="1" location="crates/tokscale-cli/src/paths.rs:28">
P2: Linux config resolution now ignores `XDG_CONFIG_HOME` by hardcoding `$HOME/.config`, which can break existing setups using non-default XDG config paths.</violation>
</file>
<file name="README.zh-cn.md">
<violation number="1" location="README.zh-cn.md:501">
P3: 文档将 `TOKSCALE_CONFIG_DIR` 描述为“绝对路径”是错误的;实现会原样接受环境变量值(包含相对路径),应改为“支持任意有效路径”或“建议使用绝对路径”。</violation>
</file>
<file name="crates/tokscale-cli/src/tui/settings.rs">
<violation number="1" location="crates/tokscale-cli/src/tui/settings.rs:148">
P2: `Settings::load()` falls back to legacy macOS settings even when `TOKSCALE_CONFIG_DIR` is explicitly set, which can override an intentional custom config directory with stale legacy data.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Thanks for merging my PR! I really appreciate this community project, and I’m honored to have been able to contribute. |
Summary
Settings::config_path()(tui/settings.rs) andstar_cache_path()(main.rs) calleddirs::config_dir(), which returns~/Library/Application Support/on macOS. That left two pieces of TUI state stranded under Application Support whileauth.rs,cursor.rs,antigravity.rs, and all four README variants documented~/.config/tokscale/. macOS users following the docs edited a settings.json that tokscale never read.This PR unifies the path on macOS + Linux and adds an env override.
crates/tokscale-cli/src/paths.rs::get_config_dir(): env override → macOS/Linux$HOME/.config/tokscale→ Windowsdirs::config_dir().join("tokscale")→.tokscalelast-ditch fallback.Settings::load()reads the new path first, then falls back to the legacy~/Library/Application Support/tokscale/settings.jsonon macOS only — so existing macOS users keep their theme / scanner /defaultClientsafter upgrade.save()always writes the new path; the next save makes the legacy file irrelevant.TOKSCALE_CONFIG_DIRenv var added (mirrorsTOKSCALE_HEADLESS_DIRsemantics — anyOkvalue wins, no empty-string special case).TOKSCALE_CONFIG_DIRrow to env-var tables, and corrected the Windows-Specific Configuration section — settings.json lives at%APPDATA%\tokscale\(the actualdirs::config_dir()default on Windows), not%USERPROFILE%\.config\tokscale\. Added the missingcredentials.jsonrow.Out of scope (per the originating issue):
auth.rs,cursor.rs,antigravity.rsalready hardcode~/.config/tokscale/correctly and were left alone. Cache dirs and headless paths were also left alone.Test plan
cargo build -p tokscale-clicleancargo clippy -p tokscale-cli --bin tokscale --testsclean (only pre-existing antigravity warnings)cargo test -p tokscale-cli --bin tokscale paths::tests::— 2 passedcargo test -p tokscale-cli --bin tokscale tui::settings::— 10 passed (incl. new macOS-gatedload_falls_back_to_legacy_macos_path_when_new_path_missing)cargo test -p tokscale-cli— only the pre-existingtest_data_loader_keeps_synthetic_gateway_messages_under_original_clientfailure (unrelated, reproduces onmain)bunx tokscale@latest tuicreates~/.config/tokscale/settings.json, not~/Library/Application Support/TOKSCALE_CONFIG_DIR=/tmp/tk bunx tokscale@latest tuiwrites/tmp/tk/settings.json~/Library/Application Support/tokscale/settings.jsonsees their prior settings on first launch after upgrading%APPDATA%\tokscale\settings.json)🤖 Generated with Claude Code
Summary by cubic
Unifies the TUI config and star cache on macOS under
~/.config/tokscale, matching Linux and the docs. AddsTOKSCALE_CONFIG_DIRto override the location and preserves existing macOS settings on upgrade.Bug Fixes
~/.config/tokscale/settings.json; no more~/Library/Application Support/....Settings::load()falls back to the legacy macOS path once, so existing theme/scanner/defaultClients are kept.star-cache.jsonnow lives in the same config dir; Windows behavior is unchanged.New Features
paths::get_config_dir()to centralize config-dir resolution across platforms.TOKSCALE_CONFIG_DIRto override wheresettings.jsonis stored.TOKSCALE_CONFIG_DIR, corrected Windows path to%APPDATA%\tokscale\, and added the missingcredentials.jsonrow.Written for commit d8f5b9e. Summary will update on new commits.