Generalize agent setup with registry pattern

[jeremy]

Summary

• Agent registry — AgentInfo struct + RegisterAgent() / DetectedAgents() / AllAgents() / FindAgent(). Claude self-registers via init(). Adding a new agent (Cursor, Windsurf, etc.) is a single-file PR.
• Extract skill helpers — installSkillFiles() and linkSkillToClaude() pulled out of monolithic skill install RunE. Claude symlink gated on DetectClaude() so skill install doesn't create ~/.claude/ as a side effect.
• Generalized wizard flow — wizardAgents() replaces wizardClaude(). Shows detected agents, numbered step list, single confirmation, then runs each handler. Auto-generates setup <agent> subcommands from the registry.
• Doctor/auth/quickstart — All iterate DetectedAgents() instead of hardcoding Claude. Auth nudge and quickstart breadcrumbs adapt dynamically.
• Skill link health check — CheckClaudeSkillLink() verifies ~/.claude/skills/basecamp/SKILL.md is reachable. Registered alongside CheckClaudePlugin() so doctor and wizard allGood both catch broken links.
• Non-interactive error surfacing — RunNonInteractive returns errors. Result envelope includes "errors" key and downgrades plugin_installed when setup had failures.
• Interactive repair — runClaudeSetup checks plugin status before requiring the binary, so "plugin installed, skill link broken" is repairable without claude on PATH.

Test plan

• make check passes (fmt, vet, lint, test, e2e)
• basecamp setup — wizard shows agent detection with clear labels
• basecamp setup claude — focused setup still works
• basecamp doctor — shows both plugin and skill link checks for detected agents
• basecamp skill install — installs skill without creating ~/.claude/ when Claude not present

---

Summary by cubic

Generalized coding agent setup with a registry and a unified wizard. Doctor, quickstart, and auth nudges now adapt to detected agents, and setup reports failures accurately.

• New Features

  • Added AgentInfo registry (RegisterAgent/DetectedAgents/AllAgents/FindAgent); Claude self-registers via init().
  • Replaced Claude-only flow with wizardAgents; installs the baseline skill, shows detected agents, runs per‑agent handlers, and auto‑generates setup <agent> subcommands.
  • Doctor, auth login nudge, and quickstart breadcrumbs now iterate DetectedAgents and adapt per agent.
  • Added a Claude skill link health check so doctor/wizard catch missing or broken links.

• Bug Fixes

  • DetectClaude now checks both ~/.claude and a claude binary on PATH.
  • Extracted installSkillFiles/linkSkillToClaude; skill install no longer creates ~/.claude unless Claude is detected (symlink only when present).
  • Interactive and non‑interactive setup can repair a broken Claude skill link; non‑interactive mode returns errors in JSON and downgrades plugin_installed on failures.
  • Fixed plugin detection for Claude’s v2 installed_plugins.json and alternate marketplace keys (e.g., basecamp@37signals) by unwrapping the "plugins" envelope and prefix‑matching.
  • Tightened fallback detection to match "basecamp" or "basecamp@" tokens only, avoiding false positives.
  • Guarded agent registry by panicking on empty or duplicate IDs to keep state consistent.

^{Written for commit dc36a95. Summary will update on new commits.}

[github-actions]

⚠️ Potential breaking changes detected:

• The 'setup claude' subcommand was removed, which constitutes the removal of a CLI subcommand.
• The 'long' descriptions in some setup commands were updated, which can impact scripts relying on detailed descriptions for operational information.

Review carefully before merging. Consider a major version bump.

[Copilot]

Pull request overview

This PR refactors coding-agent integration to be registry-driven, replacing Claude-specific wiring with a generalized agent registry plus a shared setup/wizard flow that can scale to additional agents.

Changes:

• Added a global agent registry (AgentInfo, RegisterAgent, DetectedAgents, AllAgents, FindAgent) and registered Claude via init().
• Reworked setup and the interactive wizard to run agent setup based on detected/registered agents and to auto-generate setup <agent> subcommands.
• Extracted skill install/link helpers and added a Claude skill-link health check used by doctor/setup flows, with expanded tests.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
internal/harness/agent.go	Introduces agent registry primitives for registration, detection, and lookup.
internal/harness/agent_test.go	Adds tests for registry behavior and Claude registration expectations.
internal/harness/claude.go	Claude self-registers as an agent; detection extended (dir or PATH); adds CheckClaudeSkillLink.
internal/harness/claude_test.go	Adds coverage for skill-link check and updated Claude detection semantics.
internal/commands/wizard_agents.go	New generalized agent-setup wizard + per-agent setup handlers + auto-generated setup <agent> subcommands.
internal/commands/wizard.go	Replaces Claude-specific setup with registry-driven subcommands and wizard step.
internal/commands/wizard_test.go	Updates setup tests for new JSON keys and adds tests for baseline skill install + repair scenarios.
internal/commands/skill.go	Extracts installSkillFiles/linkSkillToClaude; gates Claude symlink creation behind DetectClaude().
internal/commands/skill_test.go	Adjusts tests for new Claude detection gating and adds “no Claude side effects” test.
internal/commands/doctor.go	Doctor now runs agent checks for each detected agent (instead of Claude-only).
internal/commands/doctor_test.go	Updates Claude integration test to use registry + adds skill-link check coverage.
internal/commands/quickstart.go	Breadcrumbs now suggest setup <agent> for detected agents with failing checks.
internal/commands/auth.go	Post-login nudge now adapts to detected agents with failing checks.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 81379b1468

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[cubic-dev-ai]

9 issues found across 13 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/harness/claude_test.go">

<violation number="1" location="internal/harness/claude_test.go:64">
P2: Unchecked error from `os.MkdirAll` — inconsistent with all other test setup calls in this file that use `assert.NoError`. If the dir creation fails, the test passes for the wrong reason (no `~/.claude` means `CheckClaudeSkillLink` returns `"warn"` instead of `"fail"`, which would actually fail the assertion, but it's still a setup bug that should surface clearly).</violation>
</file>

<file name="internal/commands/wizard_agents.go">

<violation number="1" location="internal/commands/wizard_agents.go:50">
P1: Non-interactive setup skips skill link repair when binary is missing, even if plugin is already installed. The early `return nil` on missing binary prevents the function from reaching the plugin-already-installed branch that handles skill link repair. Move the plugin status check before the binary check to match the interactive version's behavior.</violation>

<violation number="2" location="internal/commands/wizard_agents.go:92">
P1: `linkSkillToClaude()` is called unconditionally here, but it creates `~/.claude/skills/` (and implicitly `~/.claude/`) via `os.MkdirAll`. When the plugin isn't installed and no binary is found (i.e., Claude is truly absent), this still creates the `~/.claude/` directory. Since `DetectClaude()` treats any `~/.claude` directory as a positive detection, this permanently flips detection to `true`, causing repeated false-positive Claude setup prompts in `doctor`, `login`, and `quickstart` flows. Gate this call on `harness.DetectClaude()` to match the guard used in `skill install`.</violation>
</file>

<file name="internal/harness/agent_test.go">

<violation number="1" location="internal/harness/agent_test.go:71">
P2: The `if found := FindAgent("claude"); found != nil` branch is dead code — prior tests always clear the registry via `resetRegistry()`, so this never executes. The test name claims to verify `init()` registration but always takes the manual fallback path, providing false confidence.

Consider either: (a) capturing a snapshot of the registry before any test runs (e.g., via a `TestMain`), or (b) re-running `init()` registration explicitly before this assertion.</violation>
</file>

<file name="internal/commands/wizard_test.go">

<violation number="1" location="internal/commands/wizard_test.go:260">
P2: Discarded error from `skills.FS.ReadFile` — if the embedded file path is wrong, `embedded` is nil and the assertion gives a misleading result. Use `require.NoError` to catch this.</violation>

<violation number="2" location="internal/commands/wizard_test.go:289">
P2: Unchecked errors from `os.MkdirAll` / `os.WriteFile` in test setup. If these fail, the test produces a confusing assertion failure instead of pointing at the real cause. The same file already uses `require.NoError` for identical calls in `TestBaselineSkillInstalled` — keep it consistent.</violation>

<violation number="3" location="internal/commands/wizard_test.go:294">
P2: Same unchecked-error pattern in test setup. Wrap with `require.NoError` for clear failure diagnostics, consistent with the rest of this file.</violation>
</file>

<file name="internal/commands/doctor_test.go">

<violation number="1" location="internal/commands/doctor_test.go:536">
P2: Unchecked error returns from `os.MkdirAll` and `os.WriteFile` in test setup. The rest of this file consistently wraps these in `require.NoError(t, ...)`. If the setup fails silently, the subsequent assertions could pass or fail for the wrong reason (e.g., skill check fails because dir creation failed, not because the link is absent).</violation>
</file>

<file name="internal/harness/agent.go">

<violation number="1" location="internal/harness/agent.go:27">
P1: Calling user-provided `Detect()` while holding `registryMu.RLock()` risks deadlock. `sync.RWMutex` is not reentrant, so if any `Detect` implementation transitively touches the registry (e.g., `FindAgent`, `RegisterAgent`), the goroutine will deadlock. Copy the slice under the lock, then iterate outside it.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/harness/claude.go">

<violation number="1" location="internal/harness/claude.go:224">
P2: The raw-string fallback `contains(s, '"basecamp')` drops the closing double-quote that previously bounded the match. This will match any JSON string starting with "basecamp" (e.g. `"basecamper"`, `"basecamp-tools"`), falsely reporting the plugin as installed. Use a pattern that's consistent with `matchesPluginKey` — match `"basecamp"` (exact) or `"basecamp@` (prefix with delimiter).</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.