Skip to content

Fix/270 claude install#309

Open
kavish-100403 wants to merge 9 commits into
darnitdevorg:mainfrom
kavish-100403:fix/270-claude-install
Open

Fix/270 claude install#309
kavish-100403 wants to merge 9 commits into
darnitdevorg:mainfrom
kavish-100403:fix/270-claude-install

Conversation

@kavish-100403

Copy link
Copy Markdown
Contributor

Summary

Fixes darnit install writing MCP config to the wrong file for Claude Code.

  • Default client is now claude-code (was ambiguous claude)
  • Claude Code (global): writes ~/.claude.json (was ~/.claude/settings.json, which is Claude Desktop)
  • Claude Code (--project): also writes .mcp.json at the repo root (MCP was global-only before)
  • Claude Desktop: --client claude-desktop keeps ~/.claude/settings.json
  • Deprecation: --client claude still works but logs a warning; use claude-code or claude-desktop

Type of Change

  • Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Breaking change (fix or feature causing existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)

Framework Changes Checklist

If this PR modifies the darnit framework (packages/darnit/):

  • Updated framework spec (docs/architecture/framework-design.md) if behavior changed
  • Ran uv run python scripts/validate_sync.py --verbose and it passes

Control/TOML Changes Checklist

If this PR modifies controls or TOML configuration:

  • Control metadata defined in TOML (not Python code)
  • SARIF fields (description, severity, help_url) included where appropriate
  • Ran validation to confirm TOML schema compliance

Testing

  • Tests pass locally (uv run pytest tests/ -v)
  • Added tests for new functionality (if applicable)
  • Linting passes (uv run ruff check .)

Additional Notes

Files changed:

  • packages/darnit/src/darnit/cli.py — path logic, argparse choices, deprecation warning
  • tests/darnit/test_cli.py — tests for claude-code, claude-desktop, --project, deprecation
  • docs/getting-started/using-skills.md — install commands and config file table
  • docs/install/from-source.mddarnit install is now the recommended path for Claude Code

@Marc-cn

Marc-cn commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Thanks @kavish-100403 . I reviewed and tested this merged onto current main: CLI tests pass, validate_sync.py passes, and I tested every install path. I also verified end-to-end that claude mcp list picks up the written config (Claude Code 2.1.197 on Windows and 2.1.204 on Linux). The Claude Code side is correct and works as described:

  • ~/.claude.json for user scope and .mcp.json for --project match Claude Code's documented MCP locations
  • --client claude deprecation warning fires and still writes the right file
  • merge logic preserves existing config keys, and the .bak backup is a nice touch

One blocking issue before this can go in:

--client claude-desktop targets the wrong file. ~/.claude/settings.json is Claude Code's settings file (permissions, hooks, env), not Claude Desktop's config. Claude Desktop reads claude_desktop_config.json at platform-specific locations:

  • macOS: ~/Library/Application Support/Claude/claude_desktop_config.json
  • Windows: %APPDATA%\Claude\claude_desktop_config.json
  • Linux: ~/.config/Claude/claude_desktop_config.json

As written, --client claude-desktop writes a file that neither product reads, and the table in docs/getting-started/using-skills.md documents that as working. Two ways to resolve, either is fine with me:

  1. Make it platform-aware, e.g.:
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 os/sys imports if missing, plus a test with monkeypatched APPDATA/platform, and fix the docs table)

  1. Or drop claude-desktop from this PR entirely and keep it scoped to the Claude Code fix, which stands on its own. It can come back in a follow-up with the right path

Thanks!

@mlieberman85 mlieberman85 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. 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 survive git blame better and makes the redesign PR's diff cleaner too.
  2. Keep a test that exercises the default client. test_install_claude_creates_settings used to call main(["install"]) and now passes --client claude-code explicitly. Since the whole point of this fix is that the default was wrong, please keep at least one test that calls main(["install"]) with no --client to pin the new default.
  3. --project silently ignored for non-claude-code clients. Today Claude Code is the only client with a project-scoped MCP config path (.mcp.json), so --project is effectively a no-op for claude-desktop and cursordarnit install --client claude-desktop --project accepts 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.

@mlieberman85

Copy link
Copy Markdown
Contributor

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.

@kavish-100403

Copy link
Copy Markdown
Contributor Author

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.

@kavish-100403

Copy link
Copy Markdown
Contributor Author

Sorry again for the delay. I've addressed the review feedback:

Marc-cn

  • Fixed --client claude-desktop to write the platform-specific claude_desktop_config.json path (macOS / Windows %APPDATA% / Linux), with an updated test and docs.

mlieberman85

  1. Removed unrelated formatter churn from cli.py so the diff is install-focused.
  2. Restored a default-client test via main(["install"]) (no --client).
  3. --project with non–claude-code clients now logs a warning instead of failing silently.

All tests/darnit/test_cli.py tests pass locally (24). Happy to adjust further if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants