Skip to content

Add kitaru setup to wire skills and the MCP server into detected coding agents - #977

Merged
htahir1 merged 12 commits into
developfrom
feat/kitaru-setup
Sep 7, 2026
Merged

htahir1 merged 12 commits into
developfrom
feat/kitaru-setup

Conversation

@htahir1

@htahir1 htahir1 commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #953. The one-line installer (#952) detects coding agents in bash and runs once, so anyone who installs Claude Code, Cursor, or Codex the day after installing Kitaru gets no skills and no MCP server until they re-run the installer. kitaru setup moves that wiring into the CLI so it can be re-run any time, and the installer now calls it instead of carrying its own detection.

What changed

  • src/kitaru/cli/setup.py (new): the command. Skills come from the zenml-io/kitaru-skills tarball, read member by member (no extractall, no symlinks, no path escapes) into ~/.agents/skills, plus ~/.claude/skills and ~/.codex/skills when that CLI is on PATH or its home dir exists. MCP registration per client: Claude Code (claude mcp get / remove / add --scope user|project), Codex (codex mcp add, which overwrites), Cursor (.cursor/mcp.json, in the repo for a project install, in ~ otherwise), Windsurf (~/.codeium/windsurf/mcp_config.json). JSON clients are merged, keeping other servers and replacing only kitaru. With no client found it prints the snippet to paste.
  • Launch resolution: inside a project (sys.prefix is a .venv under a pyproject.toml, cwd inside it) the entry is uv run --directory <project> kitaru-mcp with Claude Code project scope, matching what the installer already wrote. Otherwise the absolute kitaru-mcp next to the interpreter, so a client that does not share the user's PATH still finds it. A missing kitaru-mcp is an invalid_configuration error with the install hint.
  • Options: --mode read-only|standard|destructive (default standard), --no-skills, --no-mcp; the global --server picks the target URL, defaulting to the CLI's selected server, then http://localhost:8000.
  • Output: a rich steps table on a TTY (src/kitaru/cli/output.py _emit_setup), the usual JSON envelope otherwise, with steps, skills, and mcp_snippet. One failing client is a warning, not a failure; the command only fails when every step failed.
  • install.sh: after installing the package it probes kitaru schema setup (offline) and, if present, runs kitaru setup with the same --server, --mode, --no-skills, --no-mcp it was given. Releases before this command fall through to the previous bash implementation, unchanged, so --version 0.24.0 keeps working. Closing message gains a "New editor? kitaru setup" line.
  • Docs: installation.md steps 2 and 3 now say the installer runs kitaru setup and name Cursor and Windsurf; the by-hand block uses uv run kitaru setup instead of npx skills add; agent-native/setup.md hint explains re-running after a new editor. README step 1 and CHANGELOG updated.
  • tests/cli/test_setup.py: 12 tests covering skills install and re-run cleanup, tarball path filtering, Claude and Codex command sequences, JSON merge idempotency, project vs user launch resolution, stored-server default, invalid server, download failure. test_schema.py adds setup to the top-level command set.

What reviewers should focus on

  • Client detection heuristics. Cursor and Windsurf are detected by directory existence, not a CLI. A machine with a stale ~/.cursor gets a config written. I think that is the right trade (harmless file, and the skills dirs already use the same rule) but it is a judgement call.
  • Claude Code scope handling. claude mcp get kitaru succeeding in another scope than the one we register in makes the remove fail; we ignore that and let add decide. If add fails because the name is taken elsewhere, that surfaces as a warning with the CLI's last output line.
  • resolve_mcp_launch requires cwd inside the project for project mode. Running kitaru setup from a tool install while standing in a repo stays user mode; the next_actions line tells the user to add Kitaru to the project and re-run.
  • The installer probes schema setup rather than a version compare, so the handoff is keyed on capability. Tested against 0.25.0 from PyPI (probe exits 2, bash fallback runs) and a locally built 0.26.0.dev0 (handoff runs).

Validation

uv run ruff format --check .   # clean
uv run ruff check .            # 3 findings, all in the untracked examples/end_to_end/ on this machine, not in the diff
uv run ty check                # 11 diagnostics, same untracked directory
uv run pytest tests/cli tests/mcp -q   # 675 passed, 1 skipped, 13 xfailed
uvx typos                      # clean
shellcheck -s bash install.sh  # two pre-existing infos/warnings, none new

Docker, Ubuntu 24.04, from the built wheel:

  • uv tool install then kitaru setup: 6 skills downloaded from GitHub into ~/.agents/skills, no client, snippet printed; with ~/.cursor present and --server http://localhost:9000 --mode read-only: ~/.cursor/mcp.json written; kitaru doctor reports the 6 skills; --mode yolo rejected as invalid_arguments.
  • uv init + uv add the wheel + fake claude on PATH: uv run kitaru setup ran claude mcp add --scope project kitaru -- /root/.local/bin/uv run --directory /work/agent kitaru-mcp --server ... --mode standard, wrote .cursor/mcp.json in the repo, copied skills into ~/.claude/skills too.
  • install.sh end to end with UV_FIND_LINKS pointing at a 0.26.0.dev0 build: handoff step runs kitaru setup, same results; --quiet --no-mcp exits 0 with skills installed. Rich table checked on a pty.

Not exercised: real claude / codex binaries (faked), Windows paths.

Follow-ups

  • Once this ships in a release, the bash skills/MCP sections in install.sh can be deleted after the last supported pre-setup version drops out of use.
  • kitaru doctor could list registered MCP clients; today it only reports skills.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UrD7eCgTu11F6qsYMS5JMP

…ng agents

The one-line installer detected coding agents in bash and ran once, so a
client installed after Kitaru got no skills and no MCP server until the
installer was re-run. `kitaru setup` owns that now: it installs the skills
from the zenml-io/kitaru-skills tarball into ~/.agents/skills (plus
~/.claude/skills and ~/.codex/skills when present) and registers kitaru-mcp
with Claude Code and Codex through their CLIs, and with Cursor and Windsurf
through their JSON files. A project install launches through
`uv run --directory <project>` and uses Claude Code's project scope; a tool
install points at the absolute kitaru-mcp path. Every write replaces the
previous entry, so re-running updates instead of duplicating. The installer
hands off to it when the installed version has the command and keeps the bash
path for older releases.

Closes #953

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrD7eCgTu11F6qsYMS5JMP
@htahir1
htahir1 requested a review from strickvl September 3, 2026 21:49

@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 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

@strickvl this is #953, the kitaru setup command we talked about when the installer landed. The installer now hands off to it (with a bash fallback for releases that predate the command), so "installed Cursor yesterday, how do I wire it up" becomes one command. Tested in Docker in both project and tool-install modes, details in the body. The client-detection heuristics for Cursor and Windsurf are the part I would most like your eyes on, since importers and adapters aside, you have the most feel for how people actually have these editors set up.

htahir1 and others added 5 commits September 4, 2026 07:24
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrD7eCgTu11F6qsYMS5JMP
…tor wording

A fresh install in a shell whose PATH lacks ~/.local/bin printed plain
`kitaru login --local`, which fails until a new terminal is opened. `uvx
kitaru` reuses the environment the installer just made, so print that
prefix in that case. Also fix the "Kitaru is needs attention" headline and
point doctor's missing-skills hint at `kitaru setup`.

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

htahir1 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Two small additions from a colleague's fresh install today (ee6f367):

  • The installer printed plain kitaru login --local in a shell whose PATH did not yet have ~/.local/bin, so the next step failed until a new terminal. When kitaru does not resolve in the current shell, the closing message now prints uvx kitaru ... (uv reuses the tool environment it just installed) and says a new terminal will have plain kitaru. Verified in a container with the tool dir deliberately off PATH: uvx kitaru doctor runs from the fresh install.
  • kitaru doctor printed "Kitaru is needs attention." Fixed the headline, and its missing-skills hint now says kitaru setup instead of npx skills add.

@devin-ai-integration

Copy link
Copy Markdown

Review: reliability / bug risks

Reviewed the diff and ran the new command for real (project install, HOME pointed at a scratch dir, plus fault-injected probes). uv run pytest tests/cli/test_setup.py -q → 12 passed. I did not run the full just check / just test, and I did not exercise real claude/codex binaries or Windows. Overall the shape is good — reading tar members explicitly instead of extractall, os.replace for the JSON writes, and the post-copy SKILL.md completeness check are all the right calls. The issues below are about what happens when a step goes wrong.

1. --no-mcp still fails hard when kitaru-mcp is missing

setup() resolves the launch before it knows whether MCP is wanted:

launch = resolve_mcp_launch(current, user_home)   # src/kitaru/cli/setup.py:105
...
if register_mcp:                                   # :136

resolve_mcp_launch raises invalid_configuration when there is no sibling kitaru-mcp (:206-213), so on a kitaru[cli]-only install kitaru setup --no-mcp aborts without installing skills — the one thing it was asked to do. Repro: a venv with kitaru[cli] but not [mcp], kitaru setup --no-mcp → kitaru-mcp is not installed next to this kitaru. Suggest resolving the launch (and building mcp_snippet) only under if register_mcp:, or degrading to a warning step when register_mcp is false.

Same shape, lower stakes: if MCP resolution fails while skills would have succeeded, the whole command fails. A missing MCP extra is arguably a warning + failed MCP step, not an aborted run.

2. "every detected client failed" reports success

        if registered == 0:
            steps.append(_step("mcp", "manual", "skipped", "No configurable MCP client detected. ..."))  # :145-155
...
    if steps and all(step["status"] == "failed" for step in steps):   # :157
        raise CLIError(...)

registered == 0 conflates no client found with every client failed, so the message ("No configurable MCP client detected") is wrong in the second case, and the skipped step it appends also breaks the all(... == "failed") guard — the command exits 0. Repro: with ~/.cursor present, make .cursor/mcp.json unreadable/invalid JSON → single failed step + warning + a "no client detected" step, exit code 0. Since install.sh only surfaces a warning on nonzero exit, a user whose only MCP client failed to be configured gets a green install. Suggest tracking detected separately from registered, appending the manual step only when detected == 0, and failing (or at least a distinct message) when detected > 0 and registered == 0.

3. JSON config rewrite drops the original file mode

        temporary = self.path.with_name(self.path.name + ".tmp")   # :440
        temporary.write_text(...)
        os.replace(temporary, self.path)

The temp file gets fresh default-umask permissions, so an existing mcp.json at 0600 comes back 0644 (verified locally: 600 → 644). These files can contain other servers' tokens/env, so this is a real, if quiet, regression on re-run. os.chmod(temporary, self.path.stat().st_mode) before the replace (when the file existed) fixes it. Two other nits in the same block: the .tmp name is fixed, so two concurrent runs can clobber each other, and a crash between write_text and os.replace leaves a stray mcp.json.tmp next to the config.

4. Skill install can leave destinations half-updated

_install_skills (:276-291) rmtrees and rewrites each <destination>/<name> in place, in sequence. The first OSError — anywhere, including the third file of the second skill in the second destination — raises, and setup() then marks all destinations failed (:117-118) even though earlier ones were written fine, while the destination that blew up is left with a deleted or partially written skill. Ctrl-C mid-run has the same effect. Extracting into a staging dir and os.replace-ing each skill dir into place (or at minimum reporting per-destination status instead of one all-or-nothing step) would make a re-run recoverable and stop a failure from destroying a previously working skill.

Minor, same function: destinations: Iterable[Path] is iterated three times, so passing a generator would silently skip the write/verify loops — take a Sequence.

5. --server resolution diverges from every other command

_resolve_server_url (:243-248) checks the explicit value then client.config.get_server_url(), falling back to http://localhost:8000. Commands that talk to a server go through resolve_target() (cli/config.py:125-141), which also honors KITARU_API_URL. So KITARU_API_URL=https://kitaru.example.com kitaru setup registers the MCP server against localhost:8000 while kitaru doctor in the same shell talks to the remote server — silent, and only visible later as MCP tools hitting the wrong backend. Related: install.sh honors KITARU_LOCAL_URL in the legacy fallback (MCP_SERVER_URL="${KITARU_SERVER:-$KITARU_LOCAL_URL}") but the new kitaru setup path ignores it, so that env var quietly stops working for anyone on a new release. Suggest reusing the standard resolution and passing --server through explicitly.

6. Claude entry can be shadowed by another scope but reported done

ClaudeCodeClient.register (:374-400) runs claude mcp get kitaru (scope-agnostic), and if that succeeds, removes only from self.scope. When the existing entry lives in a higher-precedence scope (local/project beating the user entry we just added), the remove fails, the comment says "adding still succeeds when the name is free in ours", and the step reports done — but Claude Code keeps launching the old command. This is exactly the re-run-after-reinstall case the PR is meant to fix. Worth either re-reading claude mcp get after the add and comparing the command, or surfacing a warning when the pre-existing entry could not be removed.

Smaller things

  • _extract_skills silently drops any member over _MAX_SKILL_FILE_BYTES (:328); if that member is SKILL.md the skill vanishes from the result with no diagnostic, and otherwise the skill installs incomplete. A skipped-file warning would help.
  • _MAX_ARCHIVE_BYTES is checked after the full body is already in memory (:313), and the decompressed size is unbounded — fine for a trusted repo, but the surrounding code is otherwise carefully defensive.
  • kitaru setup --no-skills --no-mcp exits 0 having done nothing at all; install.sh guards against that combination itself, so the CLI's silence is only reachable by hand.
  • Re-running blows away local edits under ~/.agents/skills/<name> by design (rmtree + rewrite). Intentional and tested, but worth a line in the docs since the skill dirs look user-editable.
  • mode is validated in app.py but setup() accepts any str at runtime (hence the # type: ignore[arg-type]); a mode not in MCP_MODES check in setup() would make the public function safe for the installer path too.
  • CHANGELOG.md currently conflicts with develop — merge conflict shows on the PR.

htahir1 and others added 2 commits September 4, 2026 15:38
- Resolve the MCP launch only when MCP registration is wanted, and turn a
  missing kitaru-mcp into a failed step instead of aborting before skills.
- Track detected clients separately from registered ones: "no client
  detected" only when none was found; every detected client failing exits 1.
- JSON client configs are rewritten through a uniquely named temp file that
  keeps the original mode, so a 0600 mcp.json stays 0600 and no .tmp is left.
- Skills install per destination through a staging dir and atomic rename;
  a failure leaves the previous skill in place and reports that destination
  only. Destinations are a Sequence. Oversized archive members are skipped
  and named; the archive size limit is enforced while streaming.
- Server resolution goes through resolve_target (KITARU_API_URL, stored
  server), then KITARU_LOCAL_URL as the installer always honored.
- Claude Code registration reads the entry back after the add and fails
  with a hint when another scope still shadows it.
- --mode validated inside setup() too; --no-skills --no-mcp warns.

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

htahir1 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review, all six were real. Addressed in the latest commit:

  1. --no-mcp without kitaru-mcp: the launch is now resolved only under register_mcp. When it is wanted but kitaru-mcp is missing, that is a failed mcp step with the install hint and exit 1, and skills still install.
  2. "every client failed" vs "none detected": detected and registered are tracked separately. The manual snippet step only appears when nothing was detected; detected-but-all-failed sets exit 1 with a warning to fix and re-run, so install.sh surfaces it.
  3. JSON config mode: rewritten via tempfile.mkstemp in the same directory, os.chmod to the original mode before os.replace, temp removed on any failure. Test asserts a 0600 mcp.json stays 0600 and no .tmp is left.
  4. Half-updated skills: each skill is written to a staging dir next to its target and swapped in with a rename, so a failure or Ctrl-C leaves the previous version intact. Outcomes are reported per destination; a write failure in ~/.codex/skills no longer marks ~/.agents/skills failed (test covers it). destinations is a Sequence. Oversized members are skipped and named in the step detail; the archive limit is enforced while streaming rather than after buffering.
  5. Server resolution: resolve_target() like every other command (--server, KITARU_API_URL, stored server), then KITARU_LOCAL_URL as the installer honored, then the default. Test covers all three.
  6. Shadowed Claude entry: after claude mcp add, claude mcp get kitaru is read back; if the winning entry does not carry our command, the step fails with a claude mcp remove hint instead of reporting done.

Smaller: --mode is validated inside setup() as well (no more type: ignore), --no-skills --no-mcp warns "nothing to do", and the docs note that a re-run overwrites local edits under ~/.agents/skills/kitaru-*. 544 CLI tests pass.

@devin-ai-integration

Copy link
Copy Markdown

Re-reviewed at 1919919. All six numbered findings are addressed, and I verified each one against the new code rather than just reading it (fault-injected probes plus uv run pytest tests/cli/test_setup.py tests/cli/test_schema.py -q → 26 passed; ruff check/ruff format --check/ty check clean on the changed files):

  1. --no-mcp with no kitaru-mcp → exit 0, skills installed; with MCP wanted → skills still installed and a mcp/kitaru-mcp: failed step, exit 1. ✔
  2. Cursor with invalid mcp.json → one failed step, no bogus "no client detected" step, exit 1. ✔
  3. 0600 mcp.json stays 0600, other servers preserved, no .tmp left behind. ✔
  4. Per-destination isolation works: an unwritable destination fails alone while the other is written, and the staging + rename means a previous skill is never left half-copied. ✔
  5. KITARU_API_URL → honored, KITARU_LOCAL_URL → honored, neither → http://localhost:8000. ✔
  6. Fake claude whose post-add mcp get reports a different command → failed with the remove-the-other-scope hint; matching command → done. ✔

Three small things the rewrite introduced, none blocking:

  • Crash-leftover staging dirs are visible to skill discovery. _write_skills stages at <destination>/.<name>.XXXX.staging, so a kill between the write and the rename leaves a complete skill copy in the skills dir, and skill_discovery uses iterdir(), which does not skip dotted names. Verified: a leftover .kitaru-dev.abc.staging/SKILL.md is reported as an installed skill at that path by get_kitaru_skill_status. Staging one level down (<destination>/.kitaru-staging/<name>/) keeps the atomic rename and stays invisible to the scan, and clearing that dir at the start of a run would also stop leftovers accumulating.
  • Skill directories are now 0700. They inherit mkdtemp's mode instead of the previous mkdir + umask 0755 (verified: 0o700). Fine for a single-user machine, but an agent running as another user (devcontainer, shared image) can no longer read ~/.agents/skills/<name>. A chmod(staging, 0o755 & ~umask) — or just os.chmod(staging, 0o777 & ~current_umask) before the rename — restores the old behavior.
  • --no-mcp output still leads with the MCP server line, rendering Kitaru MCP server: - in standard mode (project install). because server_url is None now. Worth skipping that line (and mode) when MCP registration was not requested.

Micro-nit, take it or leave it: in the target.is_dir() branch there is a window between os.replace(target, retired) and os.replace(staging, target) where an interrupt loses the skill and leaves a .<name>.XXXX.old directory; the except only cleans staging. Directory renames within one directory realistically don't fail, so this is only reachable via a signal.

Also worth updating before merge: the PR description still documents the old contract ("One failing client is a warning, not a failure; the command only fails when every step failed", and --server "defaulting to the CLI's selected server, then http://localhost:8000"), which is now the opposite of how it behaves.

I have not run the full just check / just test, and have not exercised real claude/codex binaries or Windows. The CHANGELOG.md conflict is resolved and the PR shows mergeable.

@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.

I found four actionable issues in PR #977 at 04b0c1d. All four passed independent validation.

Testing passed: 703 tests, 1 skipped, 13 expected failures. I also exercised real skill downloads, Cursor config preservation, real Codex registration, Claude scope handling, custom uv environments, JSON output, and terminal output using temporary configurations. GitHub checks are green. Testing used isolated temporary client configurations; no source changes were made.

I recommend fixing these before merging:

  1. P1: Setup can report read-only while Claude remains destructive. setup.py:538 checks only the executable, not its arguments. With the real Claude CLI, an older local entry kept its old server and destructive mode while setup reported success. Verify the complete effective command and arguments, and reject failed readback.

  2. P2: Custom uv environments overwrite global configuration. setup.py:268 recognizes only environments named .venv. A real project using UV_PROJECT_ENVIRONMENT=…/env wrote Cursor’s global configuration instead of its project configuration. Honor the configured environment and retain project scope.

  3. P2: Failed Claude replacement removes the working registration. setup.py:521 removes the existing entry before adding its replacement. If addition fails, the old entry stays deleted. Restore it on failure, as the previous installer did.

  4. P2: Failed skill replacement leaves the skill unavailable. setup.py:410 moves the old skill aside but never restores it if the final rename fails. Fault injection confirmed the active skill disappeared into a hidden .old directory. Roll back the rename on failure.

The full installer flow and Windows behavior were not exercised locally. Add regression tests for these four cases alongside the fixes.

htahir1 and others added 2 commits September 7, 2026 11:42
macOS ships bash 3.2, where expanding an empty array under `set -u`
is an unbound-variable error. `SETUP_SERVER_ARGS` is empty unless the
user passed --server, so the installer died right before running
`kitaru setup` on every default macOS install. Use the same
`${arr[@]:+${arr[@]}}` idiom the script already uses for EXTRA_PKGS.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QAUAys8r9ZkczYYGnnJ5ae
@htahir1
htahir1 merged commit 6350632 into develop Sep 7, 2026
39 checks passed
@htahir1
htahir1 deleted the feat/kitaru-setup branch September 7, 2026 11:19
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.

Add a kitaru setup command that wires skills and the MCP server into detected coding agents

2 participants