Skip to content

Make kitaru setup safe to re-run against Claude Code and custom uv environments - #1004

Merged
htahir1 merged 2 commits into
developfrom
fix/setup-reliability-followups
Sep 9, 2026
Merged

htahir1 merged 2 commits into
developfrom
fix/setup-reliability-followups

Conversation

@htahir1

@htahir1 htahir1 commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #977, which merged before a review of src/kitaru/cli/setup.py turned up four reliability gaps. All four are fixed here, each with a regression test in tests/cli/test_setup.py.

What changes

  1. Claude Code readback verifies the whole launch. claude mcp get output is parsed into a ClaudeEntry (command, args, scope) and must match our command and arguments exactly. Previously only the executable path was checked, so an older entry that kept its old --server and --mode destructive was reported as a successful read-only setup. A readback that fails, or cannot be parsed, is now a failed step rather than a pass.
  2. A failed replacement restores the previous entry. claude mcp add refuses duplicates, so the existing entry has to be removed first. If the add then fails, the removed entry is re-added. Restore only happens when the removed entry was in our scope; a shadowing entry from another scope is never copied into ours. An entry that already matches is left untouched, so re-running is a no-op instead of a remove/add cycle.
  3. UV_PROJECT_ENVIRONMENT is honored for project scope. _find_project_dir used to accept only an environment literally named .venv, so a project with a custom environment was treated as a tool install and Cursor's entry landed in the global ~/.cursor/mcp.json instead of the project file. The project is now found by walking up from the working directory to the pyproject.toml whose uv environment (.venv, or the configured one, relative values resolved against the project) is the running interpreter. This also covers uv workspaces, where the environment lives at the root while the working directory is in a member.
  4. A failed skill swap rolls back. _write_skills retires the old skill directory before renaming the staged one in. If that final rename failed, the active skill vanished into a hidden .old directory. The old version is now moved back on failure.

Reviewer Notes

The interesting code is ClaudeCodeClient.register in src/kitaru/cli/setup.py. Read it top to bottom: get, optional early return, remove, add, restore on failure, readback, parse, compare. The risk is in the ordering. If the restore ran when removed failed, it would add a duplicate; if it ran for an entry from another scope, it would copy that scope's launch into ours. Both guards are on the previous/removed pair, and the two new tests test_claude_failed_add_restores_the_previous_entry and test_claude_failed_add_does_not_restore_an_entry_from_another_scope pin the exact claude call sequence.

_parse_claude_entry reads the Command:, Args:, and Scope: lines that claude mcp get prints. Args are space-joined by the CLI, so an argument with whitespace cannot round-trip; nothing kitaru setup writes contains one. There is no loose fallback: if the output cannot be parsed, the step fails with a pointer to claude mcp get. The line format was checked against a real Claude Code CLI by registering a throwaway stdio server: it prints Command: /bin/echo, Args: hello world (space-joined), and Scope: User config (available in all your projects) / Scope: Project config (shared via .mcp.json) / Scope: Local config (private to you in this project), so the first word of the scope line is what the parser keys on. The same check confirmed that claude mcp add refuses a name that already exists in the target scope (exit 1, already exists in user config), which is why the remove-then-add sequence and its restore path exist.

_find_project_dir changed from "is the prefix's parent a project" to "which ancestor of cwd owns this environment". The cwd-inside-project constraint is preserved by construction. test_resolve_mcp_launch_honors_uv_project_environment covers a relative and an absolute environment value plus the run-from-outside case.

The test fake _fake_claude was rewritten to render a realistic claude mcp get block, since the parser now needs one. It gained knobs for the existing entry's scope, how many adds fail, and whether the readback fails.

Reproduction

With a real Claude Code CLI, from a fresh shell:

claude mcp add --scope user kitaru -- /some/old/kitaru-mcp --server http://old:8000 --mode destructive
kitaru setup --no-skills --mode read-only
claude mcp get kitaru

Before this change the second command reported the Claude step as done while claude mcp get still showed the old server and destructive. After it, setup either replaces the entry (and claude mcp get shows --mode read-only) or reports a failed step naming the mismatch. Run kitaru setup --no-skills --mode read-only a second time: it now makes a single claude mcp get call and leaves the entry alone.

For the uv environment case:

cd some-project-with-pyproject
UV_PROJECT_ENVIRONMENT=$PWD/env uv sync --extra cli --extra mcp
UV_PROJECT_ENVIRONMENT=$PWD/env uv run kitaru setup --no-skills -o json | jq '.item.install, .item.project_dir'

Expect "project" and the project path, and a .cursor/mcp.json inside the project rather than under ~. Running the same command with UV_PROJECT_ENVIRONMENT unset reports "user", which is what the previous _find_project_dir returned in every case, so the two runs show the before and after on one branch.

Local checks run: ruff format, ruff check, ty, typos, and pytest tests/cli (558 passed).

🤖 Generated with Claude Code

https://claude.ai/code/session_01QAUAys8r9ZkczYYGnnJ5ae

Four reliability gaps in `kitaru setup`, found in the review of #977
after it merged:

- The Claude Code readback only checked that the executable appeared in
  `claude mcp get` output, so an older entry keeping its old --server and
  --mode still passed as success. The output is now parsed and must carry
  exactly our command and arguments; a failed or unparseable readback is
  a failure, not a pass.
- The existing Claude entry was removed before the replacement was added,
  with nothing put back when the add failed. The removed entry is now
  restored, and only when it was in our scope, so a shadowing entry from
  another scope is never copied into ours. An entry that already matches
  is left untouched.
- Projects using UV_PROJECT_ENVIRONMENT were treated as tool installs
  because only an environment literally named `.venv` counted, which sent
  Cursor's entry to the global file. The project is now found by walking
  up from the working directory to the pyproject whose uv environment is
  the running interpreter.
- A skill directory swap that failed at the final rename left the active
  skill hidden in a retired `.old` directory. The previous version is now
  moved back on failure.

Each case has a regression test.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QAUAys8r9ZkczYYGnnJ5ae

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@htahir1
htahir1 requested a review from strickvl September 7, 2026 12:17

@strickvl strickvl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Tested this locally on my side as well

@htahir1
htahir1 merged commit efeac21 into develop Sep 9, 2026
32 checks passed
@htahir1
htahir1 deleted the fix/setup-reliability-followups branch September 9, 2026 07:44
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.

2 participants