From c330de806b7c71a00e18f2454537397fcc16dfae Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 22:32:10 +0000 Subject: [PATCH] doctor: check whether setup's docs MCP + skills are actually installed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coding-agent check only looked for the claude/npx binaries and always suggested 'assembly setup install', even right after setup reported every artifact as already installed. Move the presence probes (docs MCP registered via 'claude mcp get', skills on disk under the agent's skills root) out of commands/setup.py into a new core module aai_cli/coding_agent.py — command modules are import-linter independent, so doctor couldn't reach them in setup. Doctor now reports "docs MCP + skills installed." when setup is complete, and otherwise names exactly which artifacts 'assembly setup install' would add. https://claude.ai/code/session_01V26vhHZeP6FsmWt3Dhi6Dp --- .importlinter | 1 + AGENTS.md | 2 +- aai_cli/coding_agent.py | 83 ++++++++++++++++++++++++ aai_cli/commands/doctor.py | 12 +++- aai_cli/commands/setup.py | 67 ++++--------------- tests/test_coding_agent.py | 127 +++++++++++++++++++++++++++++++++++++ tests/test_doctor.py | 26 ++++++++ 7 files changed, 260 insertions(+), 58 deletions(-) create mode 100644 aai_cli/coding_agent.py create mode 100644 tests/test_coding_agent.py diff --git a/.importlinter b/.importlinter index d63a189d..d61c0880 100644 --- a/.importlinter +++ b/.importlinter @@ -10,6 +10,7 @@ source_modules = aai_cli.auth aai_cli.client aai_cli.code_gen + aai_cli.coding_agent aai_cli.config aai_cli.config_builder aai_cli.context diff --git a/AGENTS.md b/AGENTS.md index bcaaffc9..0ad77ad4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -171,7 +171,7 @@ Each file in `aai_cli/commands/` is a Typer sub-app (`transcribe`, `stream`, `ag - **`auth/`** — browser-assisted `assembly login` via AMS + **Stytch B2B OAuth discovery** (`discovery.py`, `flow.py`, `loopback.py`, `ams.py`). Not Stytch Connected Apps. - **`init/`** — scaffolds a self-contained FastAPI + HTML starter (`audio-transcription`/`live-captions`/`voice-agent` templates), optionally installs deps and opens the browser; writes the key to a git-ignored `.env`. - **`telemetry.py`** — anonymous, opt-out usage telemetry (Supabase-CLI model): `context.run_command` wraps each command body in `telemetry.track(ctx.command_path)`, which dispatches one allow-listed event (command path, outcome/exit code, duration, version/OS — never args, paths, or account data) to the Datadog logs intake via a **detached flusher subprocess** (the hidden `assembly telemetry flush`), so commands never wait on telemetry. `SHIPPED_CLIENT_TOKEN` is a committed write-only Datadog *client* token (`pub…`, embeddable by design — never an API key; `AAI_TELEMETRY_CLIENT_TOKEN` overrides). The test suite blanks it via an autouse conftest fixture so no test ever spawns a real flusher. Opt-out: `AAI_TELEMETRY_DISABLED=1` / `DO_NOT_TRACK=1` / `assembly telemetry disable` (persisted as `telemetry_enabled` in config.toml, alongside the random `device_id`). Send-side failures are swallowed (`OSError`/`CLIError`) — telemetry must never break a command. -- **`commands/setup.py`** — `assembly setup install/status/remove` wires a coding agent up to AssemblyAI by installing three artifacts: the `assemblyai-docs` docs MCP (via `claude mcp add`), the AssemblyAI skill (via `npx skills add`), and the bundled `aai-cli` skill (copied out of the wheel, no network). Missing `claude`/`npx` is reported and skipped, not an error. +- **`commands/setup.py`** — `assembly setup install/status/remove` wires a coding agent up to AssemblyAI by installing three artifacts: the `assemblyai-docs` docs MCP (via `claude mcp add`), the AssemblyAI skill (via `npx skills add`), and the bundled `aai-cli` skill (copied out of the wheel, no network). Missing `claude`/`npx` is reported and skipped, not an error. The presence probes (docs MCP registered, skills on disk) live in `aai_cli/coding_agent.py` so `assembly doctor`'s coding-agent check can share them — command modules are import-linter-independent, so neither command may import the other. ## Conventions diff --git a/aai_cli/coding_agent.py b/aai_cli/coding_agent.py new file mode 100644 index 00000000..5f6dd886 --- /dev/null +++ b/aai_cli/coding_agent.py @@ -0,0 +1,83 @@ +"""Coding-agent integration probes (docs MCP + skills). + +`assembly setup` installs these artifacts and `assembly doctor` reports whether +they are present. Command modules are independent of each other (import-linter +contract), so the constants and presence checks they share live here. +""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +MCP_NAME = "assemblyai-docs" +SKILL_NAME = "assemblyai" +CLI_SKILL_NAME = "aai-cli" + + +def run(cmd: list[str], *, timeout: float = 120) -> subprocess.CompletedProcess[str]: + # stdin=DEVNULL so a child that would otherwise prompt (npx's "Ok to proceed?", + # a `claude` confirmation) gets EOF and fails fast instead of hanging forever on + # input the user can't see (its stdout is captured). timeout is a final backstop. + try: + return subprocess.run( + cmd, + capture_output=True, + check=False, + text=True, + stdin=subprocess.DEVNULL, + timeout=timeout, + ) + except subprocess.TimeoutExpired: + return subprocess.CompletedProcess( + args=cmd, + returncode=124, + stdout="", + stderr=f"timed out after {timeout:.0f}s: {' '.join(cmd)}", + ) + + +def skills_root() -> Path: + # Honor CLAUDE_CONFIG_DIR so install/status/remove agree with the agent's actual + # config root rather than assuming ~/.claude. + config_dir = os.environ.get("CLAUDE_CONFIG_DIR") + root = Path(config_dir) if config_dir else Path.home() / ".claude" + return root / "skills" + + +def mcp_present() -> bool: + """Whether the docs MCP is registered. Callers must check `claude` is on PATH.""" + return run(["claude", "mcp", "get", MCP_NAME]).returncode == 0 + + +def skill_dir() -> Path: + return skills_root() / SKILL_NAME + + +def skill_installed() -> bool: + return (skill_dir() / "SKILL.md").exists() + + +def cli_skill_dir() -> Path: + return skills_root() / CLI_SKILL_NAME + + +def cli_skill_installed() -> bool: + return (cli_skill_dir() / "SKILL.md").exists() + + +def missing_components() -> list[str]: + """Names of the `assembly setup install` artifacts that are not yet installed. + + Probes the docs MCP via the `claude` CLI, so callers must check `claude` is on + PATH first. + """ + missing: list[str] = [] + if not mcp_present(): + missing.append("docs MCP") + if not skill_installed(): + missing.append("assemblyai skill") + if not cli_skill_installed(): + missing.append("aai-cli skill") + return missing diff --git a/aai_cli/commands/doctor.py b/aai_cli/commands/doctor.py index bca3ea17..3ae82a1e 100644 --- a/aai_cli/commands/doctor.py +++ b/aai_cli/commands/doctor.py @@ -8,7 +8,7 @@ import typer from rich.markup import escape -from aai_cli import client, config, environments, help_panels, options, output, theme +from aai_cli import client, coding_agent, config, environments, help_panels, options, output, theme from aai_cli.context import AppState, resolve_profile, run_command from aai_cli.errors import CLIError, NotAuthenticated from aai_cli.help_text import examples_epilog @@ -193,10 +193,18 @@ def check_audio() -> Check: def _check_coding_agent() -> Check: missing = [tool for tool in ("claude", "npx") if shutil.which(tool) is None] if not missing: + # Tools are present, so report what `assembly setup install` actually + # installed rather than always suggesting it. + not_installed = coding_agent.missing_components() + if not not_installed: + return _check( + "coding-agent", "ok", "claude and npx found; docs MCP + skills installed." + ) return _check( "coding-agent", "ok", - "claude and npx found; run 'assembly setup install' to wire up the docs MCP + skills.", + "claude and npx found; run 'assembly setup install' to add: " + f"{', '.join(not_installed)}.", ) return _check( "coding-agent", diff --git a/aai_cli/commands/setup.py b/aai_cli/commands/setup.py index b255a41f..986f3f75 100644 --- a/aai_cli/commands/setup.py +++ b/aai_cli/commands/setup.py @@ -1,6 +1,5 @@ from __future__ import annotations -import os import shutil import subprocess from pathlib import Path @@ -8,7 +7,7 @@ import typer -from aai_cli import choices, options, output +from aai_cli import choices, coding_agent, options, output from aai_cli.context import AppState, run_command from aai_cli.help_text import examples_epilog from aai_cli.steps import Step, render_steps @@ -23,32 +22,20 @@ no_args_is_help=True, ) -MCP_NAME = "assemblyai-docs" MCP_URL = "https://mcp.assemblyai.com/docs" SKILL_REPO = "AssemblyAI/assemblyai-skill" _STEPS_HEADING = "AssemblyAI coding-agent setup:" - -def _run(cmd: list[str], *, timeout: float = 120) -> subprocess.CompletedProcess[str]: - # stdin=DEVNULL so a child that would otherwise prompt (npx's "Ok to proceed?", - # a `claude` confirmation) gets EOF and fails fast instead of hanging forever on - # input the user can't see (its stdout is captured). timeout is a final backstop. - try: - return subprocess.run( - cmd, - capture_output=True, - check=False, - text=True, - stdin=subprocess.DEVNULL, - timeout=timeout, - ) - except subprocess.TimeoutExpired: - return subprocess.CompletedProcess( - args=cmd, - returncode=124, - stdout="", - stderr=f"timed out after {timeout:.0f}s: {' '.join(cmd)}", - ) +# The subprocess wrapper, artifact names, and presence probes are shared with +# `assembly doctor` (command modules are independent), so they live in +# aai_cli.coding_agent; the names below keep this module's call sites stable. +MCP_NAME = coding_agent.MCP_NAME +_run = coding_agent.run +_mcp_present = coding_agent.mcp_present +_skill_dir = coding_agent.skill_dir +_skill_installed = coding_agent.skill_installed +_cli_skill_dir = coding_agent.cli_skill_dir +_cli_skill_installed = coding_agent.cli_skill_installed def _proc_detail(proc: subprocess.CompletedProcess[str]) -> str: @@ -56,21 +43,9 @@ def _proc_detail(proc: subprocess.CompletedProcess[str]) -> str: return (proc.stderr or proc.stdout).strip() -def _skills_root() -> Path: - # Honor CLAUDE_CONFIG_DIR so install/status/remove agree with the agent's actual - # config root rather than assuming ~/.claude. - config_dir = os.environ.get("CLAUDE_CONFIG_DIR") - root = Path(config_dir) if config_dir else Path.home() / ".claude" - return root / "skills" - - # --- docs MCP (registered via the `claude` CLI) ------------------------------ -def _mcp_present() -> bool: - return _run(["claude", "mcp", "get", MCP_NAME]).returncode == 0 - - def install_mcp(scope: str, force: bool) -> Step: if shutil.which("claude") is None: return { @@ -132,14 +107,6 @@ def _remove_mcp(scope: str | None) -> Step: _SKILL_ADD_HINT = f"npx skills add {SKILL_REPO} --global" -def _skill_dir() -> Path: - return _skills_root() / "assemblyai" - - -def _skill_installed() -> bool: - return (_skill_dir() / "SKILL.md").exists() - - def install_skill(force: bool) -> Step: if shutil.which("npx") is None: return { @@ -204,23 +171,13 @@ def _remove_skill() -> Step: # --- aai-cli skill (bundled in this package, copied into the agent) ----------- -_CLI_SKILL_NAME = "aai-cli" - - -def _cli_skill_dir() -> Path: - return _skills_root() / _CLI_SKILL_NAME - - -def _cli_skill_installed() -> bool: - return (_cli_skill_dir() / "SKILL.md").exists() - def _bundled_cli_skill() -> Traversable: # Ships inside the wheel (force-included via [tool.hatch.build.targets.wheel] # artifacts). skills/ has no __init__.py, so navigate from the aai_cli package. from importlib import resources - return resources.files("aai_cli") / "skills" / _CLI_SKILL_NAME + return resources.files("aai_cli") / "skills" / coding_agent.CLI_SKILL_NAME def _copy_tree(node: Traversable, dest: Path) -> None: diff --git a/tests/test_coding_agent.py b/tests/test_coding_agent.py new file mode 100644 index 00000000..dc2f0674 --- /dev/null +++ b/tests/test_coding_agent.py @@ -0,0 +1,127 @@ +"""Unit tests for aai_cli.coding_agent — the setup/doctor shared presence probes.""" + +import subprocess + +import pytest + +from aai_cli import coding_agent + + +@pytest.fixture(autouse=True) +def _isolate_home(tmp_path, monkeypatch): + """Keep skill reads inside a temp HOME so tests never touch ~/.claude.""" + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.delenv("CLAUDE_CONFIG_DIR", raising=False) + + +# --- run() --------------------------------------------------------------------- + + +def test_run_passes_safe_subprocess_defaults(monkeypatch): + seen = {} + + def record(cmd, **kwargs): + seen.update(kwargs) + return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="") + + monkeypatch.setattr(coding_agent.subprocess, "run", record) + proc = coding_agent.run(["claude", "--version"]) + assert proc.returncode == 0 + # stdin detached so a prompting child fails fast instead of hanging; output + # captured and decoded; returncode inspected rather than raised; 120s backstop. + assert seen["stdin"] is subprocess.DEVNULL + assert seen["capture_output"] is True + assert seen["text"] is True + assert seen["check"] is False + assert seen["timeout"] == 120 + + +def test_run_timeout_becomes_clean_failure(monkeypatch): + def hang(cmd, **kwargs): + raise subprocess.TimeoutExpired(cmd, kwargs.get("timeout", 0)) + + monkeypatch.setattr(coding_agent.subprocess, "run", hang) + proc = coding_agent.run(["claude", "mcp", "get"], timeout=5) + assert proc.returncode == 124 + assert proc.stdout == "" + assert proc.stderr == "timed out after 5s: claude mcp get" + + +# --- skill locations ------------------------------------------------------------- + + +def test_skills_root_honors_claude_config_dir(monkeypatch, tmp_path): + monkeypatch.setenv("CLAUDE_CONFIG_DIR", str(tmp_path / "agent-config")) + assert coding_agent.skills_root() == tmp_path / "agent-config" / "skills" + + +def test_skills_root_defaults_to_home_dot_claude(tmp_path): + assert coding_agent.skills_root() == tmp_path / ".claude" / "skills" + + +def test_skill_presence_requires_skill_md(tmp_path): + root = tmp_path / ".claude" / "skills" + assert coding_agent.skill_dir() == root / "assemblyai" + assert coding_agent.cli_skill_dir() == root / "aai-cli" + assert not coding_agent.skill_installed() + assert not coding_agent.cli_skill_installed() + for name in ("assemblyai", "aai-cli"): + d = root / name + d.mkdir(parents=True) + (d / "SKILL.md").write_text("# x") + assert coding_agent.skill_installed() + assert coding_agent.cli_skill_installed() + + +# --- mcp_present ----------------------------------------------------------------- + + +def test_mcp_present_when_claude_mcp_get_succeeds(monkeypatch): + calls = [] + + def fake_run(cmd, **kwargs): + calls.append(list(cmd)) + return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="") + + monkeypatch.setattr(coding_agent, "run", fake_run) + assert coding_agent.mcp_present() is True + assert calls == [["claude", "mcp", "get", "assemblyai-docs"]] + + +def test_mcp_absent_when_claude_mcp_get_fails(monkeypatch): + monkeypatch.setattr( + coding_agent, + "run", + lambda cmd, **kw: subprocess.CompletedProcess(args=cmd, returncode=1, stdout="", stderr=""), + ) + assert coding_agent.mcp_present() is False + + +# --- missing_components ----------------------------------------------------------- + + +def _presence(monkeypatch, *, mcp, skill, cli_skill): + monkeypatch.setattr(coding_agent, "mcp_present", lambda: mcp) + monkeypatch.setattr(coding_agent, "skill_installed", lambda: skill) + monkeypatch.setattr(coding_agent, "cli_skill_installed", lambda: cli_skill) + + +def test_missing_components_lists_every_absent_artifact(monkeypatch): + _presence(monkeypatch, mcp=False, skill=False, cli_skill=False) + assert coding_agent.missing_components() == [ + "docs MCP", + "assemblyai skill", + "aai-cli skill", + ] + + +def test_missing_components_empty_when_fully_installed(monkeypatch): + _presence(monkeypatch, mcp=True, skill=True, cli_skill=True) + assert coding_agent.missing_components() == [] + + +def test_missing_components_reports_only_the_absent_ones(monkeypatch): + _presence(monkeypatch, mcp=True, skill=False, cli_skill=True) + assert coding_agent.missing_components() == ["assemblyai skill"] + _presence(monkeypatch, mcp=False, skill=True, cli_skill=False) + assert coding_agent.missing_components() == ["docs MCP", "aai-cli skill"] diff --git a/tests/test_doctor.py b/tests/test_doctor.py index b1a4e967..4d8e2a63 100644 --- a/tests/test_doctor.py +++ b/tests/test_doctor.py @@ -21,6 +21,9 @@ def healthy(monkeypatch): monkeypatch.setattr("aai_cli.commands.doctor.client.validate_key", lambda _key: True) monkeypatch.setattr("aai_cli.commands.doctor.shutil.which", lambda tool: f"/usr/bin/{tool}") monkeypatch.setattr("aai_cli.commands.doctor._probe_input_devices", lambda: 2) + # The MCP probe shells out to `claude mcp get`; keep the suite hermetic and + # report the full setup (docs MCP + both skills) as installed. + monkeypatch.setattr("aai_cli.commands.doctor.coding_agent.missing_components", list) def _checks(result): @@ -113,6 +116,29 @@ def test_doctor_no_microphone_warns(healthy, monkeypatch): assert _checks(result)["audio"]["status"] == "warn" +def test_doctor_coding_agent_fully_set_up_does_not_suggest_install(healthy): + # Regression: with the docs MCP + skills already installed, doctor must say so + # instead of telling the user to run a setup that's already done. + result = runner.invoke(app, ["doctor", "--json"]) + agent_check = _checks(result)["coding-agent"] + assert agent_check["status"] == "ok" + assert agent_check["detail"] == "claude and npx found; docs MCP + skills installed." + + +def test_doctor_coding_agent_not_set_up_names_whats_missing(healthy, monkeypatch): + monkeypatch.setattr( + "aai_cli.commands.doctor.coding_agent.missing_components", + lambda: ["docs MCP", "aai-cli skill"], + ) + result = runner.invoke(app, ["doctor", "--json"]) + assert result.exit_code == 0 # an un-run setup never blocks + agent_check = _checks(result)["coding-agent"] + assert agent_check["status"] == "ok" + assert agent_check["detail"] == ( + "claude and npx found; run 'assembly setup install' to add: docs MCP, aai-cli skill." + ) + + def test_doctor_coding_agent_missing_warns(healthy, monkeypatch): monkeypatch.setattr( "aai_cli.commands.doctor.shutil.which",