From 8410abb0d55c47a0be8631caed18653ccc41b6dd Mon Sep 17 00:00:00 2001 From: Walker Hughes <74113220+walkerhughes@users.noreply.github.com> Date: Sat, 25 Jul 2026 00:28:19 -0700 Subject: [PATCH] refactor: drop the setup skill; move auth recovery into the error path Now that `harbor auth login` resolves automatically from ~/.harbor/credentials.json, the setup skill was mostly restating what two cheaper mechanisms already cover. Fixes a live bug while moving it. AUTH_SUGGESTIONS said: "Set HARBOR_API_KEY in this repo's .env file (see .env.example)." That is the exact advice that caused the original HTTP 401, and it points at a path the server stopped reading. Any auth failure handed the agent actively misleading guidance. tools/writes.py had the same stale ".env" advice for HARBOR_MCP_ENABLE_WRITES. Where the guidance lives now: - AUTH_SUGGESTIONS carries the whole recovery procedure: diagnose with `harbor auth status` (noting it exits 0 either way, so read the output), `harbor auth login` if unauthenticated, `logout` then `login` if a stored key is revoked, restart, confirm with whoami. It fires exactly when auth breaks and costs nothing otherwise. - The MCP server's `instructions` field carries the standing guidance. Claude Code loads it into every session already, no invocation needed. A CLAUDE.md at the plugin root was considered and does not work: the plugins reference states it "is not loaded as project context." Deleting the skill also drops a standing cost. A skill's description is loaded every session so the model can decide whether to trigger it, which is a poor trade for a procedure that runs once at first install. Also adds the marketplace description that `claude plugin validate` warned about, and bumps to 0.4.0 so installed copies receive this. Verified: `claude plugin validate` passes for both manifests (a plugin with no skills is valid). Ruff clean, 72 unit tests pass. The existing errors test caught the stale assertion and now asserts the new procedure is present and that ".env" never reappears in it. Rendered the real auth-failure payload to confirm what an agent would actually receive. --- .claude-plugin/marketplace.json | 1 + harbor-mcp/.claude-plugin/plugin.json | 2 +- harbor-mcp/README.md | 6 +- harbor-mcp/skills/setup/SKILL.md | 89 ----------------------- harbor-mcp/src/harbor_mcp/infra/errors.py | 14 +++- harbor-mcp/src/harbor_mcp/tools/writes.py | 5 +- harbor-mcp/tests/unit/test_errors.py | 9 ++- 7 files changed, 29 insertions(+), 97 deletions(-) delete mode 100644 harbor-mcp/skills/setup/SKILL.md diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index bb767e9..357ab54 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -1,5 +1,6 @@ { "name": "walkerhughes-mcps", + "description": "MCP servers that give agents direct access to external services: the Harbor evaluation hub, with more to come.", "owner": { "name": "Walker Hughes", "url": "https://github.com/walkerhughes" diff --git a/harbor-mcp/.claude-plugin/plugin.json b/harbor-mcp/.claude-plugin/plugin.json index 3da456f..a8904aa 100644 --- a/harbor-mcp/.claude-plugin/plugin.json +++ b/harbor-mcp/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "harbor-mcp", - "version": "0.3.0", + "version": "0.4.0", "description": "MCP server for the Harbor hub: inspect evaluation jobs, trials, and uploads; publish tasks and datasets when writes are enabled.", "author": { "name": "Walker Hughes" diff --git a/harbor-mcp/README.md b/harbor-mcp/README.md index a8246db..b9e11f8 100644 --- a/harbor-mcp/README.md +++ b/harbor-mcp/README.md @@ -20,7 +20,7 @@ Two separate commands. The first opens a prompt that expects only the `owner/rep Requires [`uv`](https://docs.astral.sh/uv/) on your PATH; the plugin builds its own environment from the checked-in `uv.lock` on first launch, which takes a few seconds before the tools appear. -Then run `/harbor-mcp:setup`. It checks your credentials, walks you through login if needed, and summarizes what is currently in your account. +If you have already run `harbor auth login`, that is the whole setup: the server reads `~/.harbor/credentials.json` on its own. Ask the agent to call `whoami` to confirm. ## Credentials @@ -32,7 +32,9 @@ harbor auth login That mints a key scoped to the logged-in user and stores it in `~/.harbor/credentials.json`, which harbor reads on its own. The plugin needs no further configuration, and no key ever goes into a config file or a `.env`. -You do not need a global harbor install: harbor ships inside the plugin's environment, so `uv run --project harbor auth login` works too, which is what `/harbor-mcp:setup` falls back to. +You do not need a global harbor install: harbor ships inside the plugin's environment, so `uv run --project harbor auth login` works too. + +There is no setup skill. If a tool hits an authentication error, it returns the recovery steps (`harbor auth status`, then `login` or `logout`) in its `suggestions`, so the agent gets them exactly when they are needed and they cost nothing otherwise. The plugin's standing guidance lives in the MCP server's `instructions`, which Claude Code loads into every session. A `CLAUDE.md` at the plugin root would *not* work: the [plugins reference](https://code.claude.com/docs/en/plugins-reference) states it is not loaded as project context. `harbor auth status` shows what is stored, but it only reads that local file, so it still reports success for a key that has since been revoked. `whoami` is what confirms the credential is live; it returns the key *id* and source, never the key. Restart the server after changing credentials. diff --git a/harbor-mcp/skills/setup/SKILL.md b/harbor-mcp/skills/setup/SKILL.md deleted file mode 100644 index 2ec66a3..0000000 --- a/harbor-mcp/skills/setup/SKILL.md +++ /dev/null @@ -1,89 +0,0 @@ ---- -name: setup -description: Check Harbor hub authentication for the harbor-mcp server and summarize the account's current state. Use when the user runs /harbor-mcp:setup, when they ask how to authenticate or connect harbor-mcp, when they want to enable Harbor write tools, or when a harbor-hub tool has just failed with an authentication error. ---- - -# Harbor hub setup - -`harbor auth login` is the only supported way to authenticate: it mints a key -scoped to the logged-in user and stores it in `~/.harbor/credentials.json`, which -harbor reads on its own. Never write, echo, or read back a key value, and never -run `harbor auth login` on the user's behalf; it is an interactive OAuth flow -they must complete themselves. - -## Step 1: check stored credentials - -```bash -command -v harbor >/dev/null && harbor auth status || uv run --project "${CLAUDE_PLUGIN_ROOT}" harbor auth status -``` - -The fallback matters: harbor ships inside this plugin's own environment, so the -user does not need a global harbor install. Whichever path runs, use that same -form for any `harbor` command you tell the user to run, so it matches their setup. - -**This command exits 0 whether or not the user is authenticated**, so branch on -its output, not its exit code: - -- Contains `Not authenticated` → go to Step 2. -- Reports `Logged in as (API key sk-harbor-...)` → go to Step 3. The - printed key is already truncated to a public prefix, so it is safe to repeat. - -## Step 2: not authenticated - -Tell the user to run this themselves and say when it is done: - -```bash -harbor auth login -``` - -If `harbor` was not on their PATH in Step 1, give them the plugin form instead: - -```bash -uv run --project "${CLAUDE_PLUGIN_ROOT}" harbor auth login -``` - -Once they confirm, note that the harbor-hub server must be restarted to pick up -new credentials, then return to Step 1. - -## Step 3: confirm the key actually works - -Call the harbor-hub `whoami` tool. - -Step 1 only reads a local file, so it reports success for a key that has since -been revoked or expired. `whoami` is what proves the credential is live. It -returns `user_id`, `key_source`, and `key_id`, never the key. - -If `whoami` fails while Step 1 said the user is logged in, the stored key is -revoked or expired. Tell them to run `harbor auth logout` and then -`harbor auth login` again. - -## Step 4: summarize the account - -Call `list_jobs` once with default arguments, then report a brief snapshot so the -rest of the session has context: - -- How many jobs exist in total -- The two or three most recent, with name and status -- Anything still running, or finished with errors - -Keep it to a few lines. Do not page through the whole account, and do not write -this to a file: it goes stale as jobs run, and re-calling `list_jobs` costs the -same as reading a cache. Call it again whenever fresh data matters. - -`check_task_published` and `resolve_dataset` both require an explicit org and -name, so there is nothing to enumerate for the registry. Skip them here. - -## Step 5: report write-tool status - -Read tools work with any valid key. The seven write tools (`upload_job`, -`publish_task`, `publish_dataset`, `download_job`, `set_job_visibility`, -`share_job`, `delete_job`) refuse unless `HARBOR_MCP_ENABLE_WRITES=true` is set -in the server's environment. Read-only is the intended default. - -State the current status in one line. Only if the user asks to enable writes, -tell them to export `HARBOR_MCP_ENABLE_WRITES=true` and restart the server. Note -that `delete_job` is permanent and additionally requires `confirm=true` per call, -which you will pass only after they explicitly approve deleting a specific job. - -Do not enable writes proactively, and do not raise it when the user only asked to -get connected. diff --git a/harbor-mcp/src/harbor_mcp/infra/errors.py b/harbor-mcp/src/harbor_mcp/infra/errors.py index c85599f..1a62f61 100644 --- a/harbor-mcp/src/harbor_mcp/infra/errors.py +++ b/harbor-mcp/src/harbor_mcp/infra/errors.py @@ -12,10 +12,18 @@ logger = logging.getLogger(__name__) +# The whole auth recovery procedure lives here rather than in a skill: it costs +# nothing until auth actually fails, which is the only moment it is useful. AUTH_SUGGESTIONS = [ - "Set HARBOR_API_KEY in this repo's .env file (see .env.example).", - "Mint a key by running `harbor auth login` in a terminal.", - "Call the whoami tool to verify credentials once set.", + "Run `harbor auth status` in a terminal to see whether a credential is " + "stored. It exits 0 either way, so read its output, not its exit code.", + "If it reports `Not authenticated`, ask the user to run `harbor auth login` " + "themselves; it is an interactive OAuth flow and stores a key scoped to " + "them in ~/.harbor/credentials.json.", + "If it reports a logged-in user, the stored key is revoked or expired: ask " + "the user to run `harbor auth logout` and then `harbor auth login` again.", + "The server must be restarted to pick up new credentials. Then call whoami " + "to confirm.", ] diff --git a/harbor-mcp/src/harbor_mcp/tools/writes.py b/harbor-mcp/src/harbor_mcp/tools/writes.py index 6959af0..cf54c18 100644 --- a/harbor-mcp/src/harbor_mcp/tools/writes.py +++ b/harbor-mcp/src/harbor_mcp/tools/writes.py @@ -61,7 +61,10 @@ def _writes_disabled() -> str | None: return None return error_response( "Write tools are disabled.", - suggestions=["Set HARBOR_MCP_ENABLE_WRITES=true in .env to enable hub writes."], + suggestions=[ + "Ask the user to export HARBOR_MCP_ENABLE_WRITES=true and restart " + "the server. Read-only is the intended default." + ], ) diff --git a/harbor-mcp/tests/unit/test_errors.py b/harbor-mcp/tests/unit/test_errors.py index 40c1616..dca8edc 100644 --- a/harbor-mcp/tests/unit/test_errors.py +++ b/harbor-mcp/tests/unit/test_errors.py @@ -31,7 +31,14 @@ async def tool() -> str: payload = json.loads(await tool()) assert "Not authenticated" in payload["error"] - assert any("HARBOR_API_KEY" in s for s in payload["suggestions"]) + # The suggestions are the whole auth recovery procedure, since there is no + # setup skill: diagnose with `harbor auth status`, then login. + suggestions = " ".join(payload["suggestions"]) + assert "harbor auth status" in suggestions + assert "harbor auth login" in suggestions + # Must not send anyone back to .env; the server stopped reading it, and a + # stale key there is what shadowed a good credentials.json once already. + assert ".env" not in suggestions async def test_guarded_tool_maps_postgrest_error():