Skip to content

feat: add agentty agent support to the install CLI - #2483

Closed
davidwed wants to merge 0 commit into
DeusData:mainfrom
davidwed:main
Closed

davidwed wants to merge 0 commit into
DeusData:mainfrom
davidwed:main

Conversation

@davidwed

@davidwed davidwed commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Add automatic detection, skill installation, MCP registration, uninstall, and --clients selection for the agentty coding agent (https://github.com/1ay1/agentty), following the established detected-agent pattern (OpenHands template):

  • Detection: ~/.agentty/ directory or agentty CLI on PATH; new cbm_detected_agents_t.agentty field probed in cbm_detect_agents()
  • Selection: --clients=agentty token via the CLI_CLIENTS table
  • Install: codebase-memory skill to ~/.agentty/skills/ (Claude-compatible SKILL.md; user scope = trusted on load) and MCP server entry in ~/.agentty/mcp.json (standard mcpServers shape via the existing cbm_install_editor_mcp adapter). Instructions/hooks/tiered profiles deliberately withheld — agentty documents no AGENTS.md equivalent.
  • Uninstall: owned-document matching counterpart; user-modified files and foreign servers survive; skill dir removed only when empty
  • Docs: README agent table row, pkg/npm/README.md (40 detected), docs/llms.txt + docs/index.html + badge (46 surfaces / 40 detected), contract test cli_supported_agent_surfaces_match_installers and the --help enumeration aligned to 46/40 in lockstep

Tests: detection, --clients vocabulary + leak guard, install plan and schema checks, and an install/uninstall roundtrip asserting foreign-server and user-skill preservation in both directions.

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test)
  • Lint passes (make -f Makefile.cbm lint-ci)
  • New behavior is covered by a test (reproduce-first for bug fixes)

@davidwed
davidwed requested a review from DeusData as a code owner October 2, 2026 11:08
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

@DeusData DeusData left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you, @davidwed! This is a clean, well-tested addition:

  • The paths match agentty's own docs (~/.agentty/mcp.json at user scope, ~/.agentty/skills/).
  • Uninstall removes only what we own.
  • Every headline count and the contract test stay in lockstep.

We'd like agentty in, with two changes before merge:

  1. Please register agentty in the table-driven client registry (src/cli/agent_clients.c) instead of the legacy detector.

    • Every client added since Grok Build lives there: Qoder, Kimi Code, GitLab Duo, Rovo Dev, Amp, Devin, Tabnine, CodeBuddy, IBM Bob Shell, Pochi, Pi and Oh My Pi. Each entry carries its stability class, capability bits, detection command and install/remove callbacks.
    • The legacy path is cbm_detected_agents_t, CLI_CLIENTS, the plan-names table and the hand-written install/uninstall blocks. We are no longer extending it.
    • Moving agentty to the registry also gives you detection output, --clients, --plan and uninstall from that one entry.
    • On the current legacy path, print_detected_agents doesn't list agentty. A machine with only agentty would print "Detected agents: (none)" and then install anyway.
  2. Honor $AGENTTY_MCP_CONFIG.

    • agentty's plugin docs say this variable points at an explicit file that overrides both the user and the project mcp.json.
    • When it is set, agentty never reads the entry in ~/.agentty/mcp.json, yet install still reports success.
    • Writing to the override file when the variable is set, as we already do for CLAUDE_CONFIG_DIR, GROK_HOME and VIBE_HOME, closes that gap.

One small thing: the suite comment in tests/test_cli.c changes "6 tests" to "7 tests", but the group already has more than that, so it's fine to drop the count.

Thanks again for picking this up and for modelling it so carefully on the existing clients!

davidwed added a commit to davidwed/codebase-memory-mcp that referenced this pull request Oct 8, 2026
…TTY_MCP_CONFIG

Address the two pre-merge review requests on PR DeusData#2483:

1. Move agentty from the legacy detector to the table-driven client
   registry (src/cli/agent_clients.c), the home of every client added
   since Grok Build. The entry carries its stability class, capability
   bits (MCP | SKILL), detection command, and install/remove callbacks,
   which gives detection output, --clients selection, --plan receipts,
   and uninstall from that one row. The legacy path —
   cbm_detected_agents_t.agentty, the CLI_CLIENTS token, the plan-names
   row, and the hand-written install/uninstall blocks — is removed, so
   print_detected_agents no longer shows a machine with only agentty as
   "(none)" while installing anyway.

2. Honor $AGENTTY_MCP_CONFIG in both directions. When the variable is
   set, install and uninstall target that explicit file (with leading-~
   expansion against HOME, matching CLAUDE_CONFIG_DIR/GROK_HOME/
   VIBE_HOME) and leave ~/.agentty/mcp.json untouched, closing the gap
   where install reported success while agentty read only its override.

Also drop the stale "7 tests" count from the suite comment and cover
the registry entry, the env override, and the override-aware
install/uninstall roundtrip in the CLI and registry suites.

Validated: build clean; focused cli + agent_clients + agent_profiles
runs at 388 passed / 20 failed, the identical pre-change baseline
(install/uninstall staging under the local ASan runner; green on CI).
The seven touched agentty/registry tests pass in isolation.
@davidwed
davidwed requested a review from DeusData October 9, 2026 07:11
@DeusData DeusData added enhancement New feature or request editor/integration Editor compatibility and CLI integration priority/backlog Valuable contribution, lower scheduling urgency; review when maintainer capacity opens. labels Oct 19, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thanks again for the agentty integration work. Following up on the review: the move to the table-driven client registry and support for AGENTTY_MCP_CONFIG are still the requested next steps in the thread. Please let us know if either request needs clarification; no additional scope is being requested here.

@DeusData

Copy link
Copy Markdown
Owner

Thank you, @davidwed, and apologies: you addressed both review points on Oct 8, and our follow-up on the 19th wrongly listed them as still open. The move into the table-driven registry is exactly what we asked for, $AGENTTY_MCP_CONFIG is honored in both directions, and the install/uninstall roundtrip that proves foreign servers and user skills survive is the kind of test we wish every integration had.

The branch had fallen behind main, so we used maintainer edits to push two small commits on top of yours instead of sending you another round:

  1. Merged current main. The only conflict was the README, which main has reworded since. We kept main's wording and re-applied your changes (46 surfaces, 40 detected, the agentty row), plus the client count in a new intro sentence main added.
  2. Moved the $AGENTTY_MCP_CONFIG lookup into the registry setup in cli.c, next to the other client overrides (CBM_CODY_CONFIG_PATH, KIMI_CODE_HOME, and so on). It now goes through the same helper they use and reaches agent_clients.c as a resolve option, so the resolver stays free of environment reads and the extra public helper is no longer needed. Absolute and ~ values behave exactly as in your version. A relative value is now ignored, as for the other overrides, instead of pointing at a file relative to whatever directory install happens to run from.
  3. Adjusted the tests to match: cli_agentty_mcp_config_env_override checks the resolve option directly and the environment path through the install plan (absolute, ~, relative). The roundtrip test now restores HOME and PATH before reporting a failure, because a failure there used to leave the rest of the cli suite running without git on PATH. It also checks the exit code of the second uninstall.

All of your own commits are unchanged. Thanks again for modelling this so carefully on the existing clients.

@DeusData

Copy link
Copy Markdown
Owner

Thank you again for the agentty integration. The October 20 correction stands: you had already addressed the registry and override requests, and the subsequent maintainer adjustments are recorded above. No repeat work is requested. Sorry for the delay in our remaining follow-up—I’ll follow up soon.

@DeusData

Copy link
Copy Markdown
Owner

Thank you, @davidwed, for bringing the branch up to date with main on the 23rd. Keeping it current like that is a real help to us.

Main had reworded several of the docs that carry the client counts, so the web merge kept main's text in those spots, and with it the old numbers. The contract test you extended caught it right away, which is exactly why that test is worth having. We used maintainer edits once more to put it right:

  1. Re-applied your counts on top of main's new wording: 46 client surfaces and 40 detected automatically, with agentty added to the detected lists. This covers the intro and Multi-Agent Support lines in README.md, plus pkg/npm/README.md, docs/llms.txt and docs/index.html. This is what cli_supported_agent_surfaces_match_installers was failing on in every test job.
  2. Merged current main again so CI checks the change against today's tree. There were no conflicts this time.

Your code and tests are unchanged, and nothing more is needed from you. If you push to the branch again, please pull first so these commits stay in. Thanks again for such a careful, well-tested integration.

@DeusData

Copy link
Copy Markdown
Owner

Thank you for your patience, @davidwed. CI went green on this branch, but this evening's merges on main touched files this pull request also changes, so that green no longer covered today's main. We merged current main into the branch once more (one merge commit, 5af18b9; your changes are untouched) and will merge once the fresh checks are green. Please pull before pushing again.

@DeusData

Copy link
Copy Markdown
Owner

Thank you again for the agentty install support, @davidwed. This pull request was closed a moment ago, together with a reset of your fork's main branch to our main. Resetting a fork's main (for example "Sync fork" with "Discard commits") does exactly that when a pull request comes from that branch, and our merges of main into your branch made it look out of date - sorry for the confusion that caused.

If closing was intentional, that is completely fine, and thank you for the work. If it was not: nothing is lost. The change was approved and was only waiting for its last CI legs. Its last state is commit 5af18b9, which GitHub still serves from this pull request, so you can restore it on a separate branch and open a new pull request from there:

git fetch https://github.com/DeusData/codebase-memory-mcp 5af18b91ad40005fbd870b273fe1a57464791caa
git switch -c agentty-install FETCH_HEAD
git push -u origin agentty-install

Opening it from a branch other than main avoids the same thing happening again. We will merge it as soon as CI is green. Thanks a lot!

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

Labels

editor/integration Editor compatibility and CLI integration enhancement New feature or request priority/backlog Valuable contribution, lower scheduling urgency; review when maintainer capacity opens.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants