Fix/270 claude install#309
Conversation
git merge upstream/main
|
Thanks @kavish-100403 . I reviewed and tested this merged onto current main: CLI tests pass,
One blocking issue before this can go in:
As written,
elif args.client == "claude-desktop":
if sys.platform == "darwin":
base = Path.home() / "Library" / "Application Support" / "Claude"
elif sys.platform == "win32":
base = Path(os.environ.get("APPDATA", str(Path.home() / "AppData" / "Roaming"))) / "Claude"
else:
base = Path.home() / ".config" / "Claude"
settings_path = base / "claude_desktop_config.json"(add
Thanks! |
mlieberman85
left a comment
There was a problem hiding this comment.
Thanks for tackling #270 — this fixes a real bug that's been biting users since the Feature 014 dry-run.
Heads-up on direction: I'm planning a broader redesign of darnit install soon, along the lines of speckit's model — an --integration abstraction with a per-integration registry (one file per client), auto-detection with an interactive picker when ambiguous, and a project-scoped default. The core fix here (right file for Claude Code, .mcp.json for --project) is exactly what the redesign needs to encode, so this PR is a stepping stone rather than throwaway work. I'd rather ship it now than leave #270 open while the redesign lands. Please don't feel pressure to over-invest in cleanup that the redesign will touch anyway.
Substantive items I'd like addressed before merge:
- Drop the unrelated formatter churn in
cli.py. A large chunk of the diff is reflow (argparse arg formatting, joined multi-line strings, blank-line additions) unrelated to the fix. Please revert those hunks so the bug fix is the only content — it'll survivegit blamebetter and makes the redesign PR's diff cleaner too. - Keep a test that exercises the default client.
test_install_claude_creates_settingsused to callmain(["install"])and now passes--client claude-codeexplicitly. Since the whole point of this fix is that the default was wrong, please keep at least one test that callsmain(["install"])with no--clientto pin the new default. --projectsilently ignored for non-claude-code clients. Today Claude Code is the only client with a project-scoped MCP config path (.mcp.json), so--projectis effectively a no-op forclaude-desktopandcursor—darnit install --client claude-desktop --projectaccepts the flag and just writes the global path. Not urgent right now (only Claude Code supports project scope), but as we add more clients some will support it and some won't. Please warn or reject at argparse time so the behavior is explicit rather than silent, and future contributors adding new integrations don't inherit a silent-drop pattern.
Smaller nits (take or leave):
- Deprecation message could steer users to `claude-code` more directly ("defaulting to `claude-code`; pass `--client claude-desktop` explicitly if you meant Claude Desktop").
- Move `"claude"` last in the argparse `choices` list so `--help` foregrounds the recommended values.
- Worth a follow-up: `~/.claude.json` now stores substantial user state (project histories, permission entries). Read-modify-write from `darnit install` while Claude Code is running is racy. Not blocking — good candidate for an atomic write in the redesign.
Once 1–3 are addressed, this is good to merge.
|
Hi @kavish-100403 -- friendly ping. There are two review rounds with blocking items on this PR (from @Marc-cn and me, both from the week of 2026-07-08 through 2026-07-11) and no follow-up push since. Are you still working on this? Happy to help unblock if you're stuck on any of the asks -- just let us know. |
|
Sorry for the late reply — I'll have a look at the review feedback and I'm still working on this. Thanks for the ping and for the detailed reviews. |
|
Sorry again for the delay. I've addressed the review feedback: Marc-cn
mlieberman85
All |
Summary
Fixes
darnit installwriting MCP config to the wrong file for Claude Code.claude-code(was ambiguousclaude)~/.claude.json(was~/.claude/settings.json, which is Claude Desktop)--project): also writes.mcp.jsonat the repo root (MCP was global-only before)--client claude-desktopkeeps~/.claude/settings.json--client claudestill works but logs a warning; useclaude-codeorclaude-desktopType of Change
Framework Changes Checklist
If this PR modifies the darnit framework (
packages/darnit/):docs/architecture/framework-design.md) if behavior changeduv run python scripts/validate_sync.py --verboseand it passesControl/TOML Changes Checklist
If this PR modifies controls or TOML configuration:
Testing
uv run pytest tests/ -v)uv run ruff check .)Additional Notes
Files changed:
packages/darnit/src/darnit/cli.py— path logic, argparse choices, deprecation warningtests/darnit/test_cli.py— tests for claude-code, claude-desktop,--project, deprecationdocs/getting-started/using-skills.md— install commands and config file tabledocs/install/from-source.md—darnit installis now the recommended path for Claude Code