From a7e49c51c35c6a39ea5c30616d0de3c01a32101e Mon Sep 17 00:00:00 2001 From: viragtripathi Date: Fri, 9 Oct 2026 03:14:17 -0400 Subject: [PATCH 1/3] fix: stop passing unset connection variables to Toolbox as literal text The plugin's .mcp.json mapped each COCKROACHDB_* variable to ${VAR}. Copilot CLI passes an unset ${VAR} through as the literal text ${VAR}, so Toolbox failed to build its connection URL, and the Cloud server rejected the literal mcp-cluster-id header with HTTP 400. VS Code turns an unset ${VAR} into an empty string, which Toolbox also uses as the value instead of falling back to its tools.yaml default. Both clients pass the parent environment to stdio servers, so the Toolbox server no longer has an env block: set variables still reach Toolbox and unset ones fall back to tools.yaml. No placeholder is safe in both clients for the optional cluster ID header, so the plugin config leaves it out; .vscode/mcp.json keeps it, because VS Code resolves ${env:VAR} to an empty string, which the server treats as absent. Toolbox runs with readOnlyMode, so the execute-sql tool description, the agent files, and the README no longer promise DDL and DML. The README now covers installing Toolbox on Windows, the running cluster the Toolbox server needs, turning off a server in Copilot CLI and VS Code, limiting the Cloud server to one cluster, and the first-party CockroachDB MCP Server as an alternative backend. Related to cockroachdb/claude-plugin#27 --- .github/agents/cockroachdb-dba.agent.md | 2 +- .github/agents/cockroachdb-developer.agent.md | 2 +- .github/agents/cockroachdb-operator.agent.md | 2 +- .mcp.json | 15 +-- .vscode/mcp.json | 10 +- README.md | 111 ++++++++++++++++-- tools.yaml | 8 +- 7 files changed, 112 insertions(+), 38 deletions(-) diff --git a/.github/agents/cockroachdb-dba.agent.md b/.github/agents/cockroachdb-dba.agent.md index cac3bcd..af287ee 100644 --- a/.github/agents/cockroachdb-dba.agent.md +++ b/.github/agents/cockroachdb-dba.agent.md @@ -31,7 +31,7 @@ You are a CockroachDB database administration expert. You specialize in: ## Available MCP Tools **Via MCP Toolbox** (self-hosted, any cluster): -- `cockroachdb-execute-sql`: Execute any SQL statement +- `cockroachdb-execute-sql`: Run SQL (read-only by default: SELECT, SHOW, EXPLAIN; writes and DDL need write mode enabled in the Toolbox config) - `cockroachdb-list-schemas`: List database schemas - `cockroachdb-list-tables`: List tables with column details diff --git a/.github/agents/cockroachdb-developer.agent.md b/.github/agents/cockroachdb-developer.agent.md index 3950341..4066b8c 100644 --- a/.github/agents/cockroachdb-developer.agent.md +++ b/.github/agents/cockroachdb-developer.agent.md @@ -177,7 +177,7 @@ When bulk DML exceeds 250K-500K rows (or 1M+ without secondary indexes): ## Available MCP Tools **Via MCP Toolbox** (self-hosted, any cluster): -- `cockroachdb-execute-sql`: Execute any SQL statement +- `cockroachdb-execute-sql`: Run SQL (read-only by default: SELECT, SHOW, EXPLAIN; writes and DDL need write mode enabled in the Toolbox config) - `cockroachdb-list-schemas`: List database schemas - `cockroachdb-list-tables`: List tables with column details diff --git a/.github/agents/cockroachdb-operator.agent.md b/.github/agents/cockroachdb-operator.agent.md index 3d1016e..44ea81f 100644 --- a/.github/agents/cockroachdb-operator.agent.md +++ b/.github/agents/cockroachdb-operator.agent.md @@ -260,7 +260,7 @@ For batch DML exceeding 250K-500K rows: ## Available MCP Tools **Via MCP Toolbox** (self-hosted, any cluster): -- `cockroachdb-execute-sql`: Execute any SQL statement (diagnostics, SHOW commands, DDL) +- `cockroachdb-execute-sql`: Run SQL for diagnostics and SHOW commands (read-only by default; DDL and other writes need write mode enabled in the Toolbox config) - `cockroachdb-list-schemas`: List database schemas - `cockroachdb-list-tables`: List tables with column details diff --git a/.mcp.json b/.mcp.json index 4dd8f14..6e95117 100644 --- a/.mcp.json +++ b/.mcp.json @@ -2,22 +2,11 @@ "mcpServers": { "cockroachdb-toolbox": { "command": "toolbox", - "args": ["--config", "${CLAUDE_PLUGIN_ROOT}/tools.yaml", "--stdio"], - "env": { - "COCKROACHDB_HOST": "${COCKROACHDB_HOST}", - "COCKROACHDB_PORT": "${COCKROACHDB_PORT}", - "COCKROACHDB_USER": "${COCKROACHDB_USER}", - "COCKROACHDB_PASSWORD": "${COCKROACHDB_PASSWORD}", - "COCKROACHDB_DATABASE": "${COCKROACHDB_DATABASE}", - "COCKROACHDB_SSLMODE": "${COCKROACHDB_SSLMODE}" - } + "args": ["--config", "${CLAUDE_PLUGIN_ROOT}/tools.yaml", "--stdio"] }, "cockroachdb-cloud": { "type": "http", - "url": "https://cockroachlabs.cloud/mcp", - "headers": { - "mcp-cluster-id": "${COCKROACHDB_CLUSTER_ID}" - } + "url": "https://cockroachlabs.cloud/mcp" } } } diff --git a/.vscode/mcp.json b/.vscode/mcp.json index 8451855..05d2b43 100644 --- a/.vscode/mcp.json +++ b/.vscode/mcp.json @@ -2,15 +2,7 @@ "servers": { "cockroachdb-toolbox": { "command": "toolbox", - "args": ["--config", "${workspaceFolder}/tools.yaml", "--stdio"], - "env": { - "COCKROACHDB_HOST": "${env:COCKROACHDB_HOST}", - "COCKROACHDB_PORT": "${env:COCKROACHDB_PORT}", - "COCKROACHDB_USER": "${env:COCKROACHDB_USER}", - "COCKROACHDB_PASSWORD": "${env:COCKROACHDB_PASSWORD}", - "COCKROACHDB_DATABASE": "${env:COCKROACHDB_DATABASE}", - "COCKROACHDB_SSLMODE": "${env:COCKROACHDB_SSLMODE}" - } + "args": ["--config", "${workspaceFolder}/tools.yaml", "--stdio"] }, "cockroachdb-cloud": { "type": "http", diff --git a/README.md b/README.md index cce036e..099fecf 100644 --- a/README.md +++ b/README.md @@ -23,15 +23,44 @@ Clone the repository into your project. Copilot reads `.github/skills`, `.github ### Prerequisites -This plugin connects to CockroachDB via MCP (Model Context Protocol) using [MCP Toolbox for Databases](https://github.com/googleapis/mcp-toolbox) (v1.0.0+): +The skills, agents, and hooks work without any setup. Each of the plugin's MCP servers needs its own: + +- **`cockroachdb-toolbox`** (any self-hosted or Cloud cluster) needs [MCP Toolbox for Databases](https://github.com/googleapis/mcp-toolbox) v1.0.0 or later on your `PATH`, and a running CockroachDB cluster it can reach. Toolbox connects when the session starts, so without a reachable cluster this server fails to start. If you don't run a cluster, [turn the server off](#turn-off-a-server-you-dont-use). +- **`cockroachdb-cloud`** (CockroachDB Cloud) needs a CockroachDB Cloud account, and signs in with OAuth through your browser. + +#### Install MCP Toolbox + +macOS or Linux: ```bash brew install mcp-toolbox ``` +Windows: download `toolbox.exe` into a folder on your `PATH`. In PowerShell, set the version to the latest one on the [Toolbox releases page](https://github.com/googleapis/mcp-toolbox/releases), and use `windows/arm64` in the URL on Arm devices: + +```powershell +$VERSION = "1.14.0" +curl.exe -o toolbox.exe "https://storage.googleapis.com/mcp-toolbox-for-databases/v$VERSION/windows/amd64/toolbox.exe" +``` + +Restart VS Code or your terminal after changing your `PATH`, then confirm the install with `toolbox --version`. For other platforms, the container image, and building from source, see [Install Toolbox](https://github.com/googleapis/mcp-toolbox#install-toolbox). + ## Configuration -Set environment variables for your CockroachDB connection: +### Self-hosted clusters (MCP Toolbox) + +The `cockroachdb-toolbox` server reads its connection settings from environment variables, which Copilot passes through to it. Each one is optional, and an unset variable falls back to the default in [`tools.yaml`](tools.yaml): + +| Variable | Default | Notes | +|------------------------|-------------|------------------------------------------------------------------------------| +| `COCKROACHDB_HOST` | `localhost` | | +| `COCKROACHDB_PORT` | `26257` | | +| `COCKROACHDB_USER` | `root` | | +| `COCKROACHDB_PASSWORD` | (empty) | | +| `COCKROACHDB_DATABASE` | `defaultdb` | | +| `COCKROACHDB_SSLMODE` | `require` | Use `disable` for a local `--insecure` cluster, `verify-full` for production | + +Set them in the environment Copilot starts from. On macOS or Linux, add them to your shell profile: ```bash export COCKROACHDB_HOST="your-cluster-host" @@ -42,7 +71,24 @@ export COCKROACHDB_DATABASE="your-database" export COCKROACHDB_SSLMODE="verify-full" ``` -For CockroachDB Cloud, find connection details in the [Cloud Console](https://cockroachlabs.cloud/). +On Windows, set them as user environment variables, for example `setx COCKROACHDB_HOST "your-cluster-host"`, then restart VS Code or your terminal. + +For a CockroachDB Cloud cluster, find the connection details in the [Cloud Console](https://cockroachlabs.cloud/). + +Toolbox runs in read-only mode: `SELECT`, `SHOW`, and `EXPLAIN` work, and writes and schema changes are rejected. To allow writes, run Toolbox with your own copy of [`tools.yaml`](tools.yaml) that sets `enableWriteMode: true`, configured as your own MCP server, and turn off the plugin's `cockroachdb-toolbox` server. + +### CockroachDB Cloud + +The `cockroachdb-cloud` server connects to the [managed MCP server](https://www.cockroachlabs.com/docs/cockroachcloud/connect-to-the-cockroachdb-cloud-mcp-server) that Cockroach Labs hosts. The consent screen asks you to grant read access, write access, or both. The connection can reach every cluster your CockroachDB Cloud role allows. + +To limit a connection to one cluster, add your own server entry with an `mcp-cluster-id` header, as shown under [Alternative MCP Backends](#alternative-mcp-backends). When you open this repository as a workspace, `.vscode/mcp.json` sends the header with `COCKROACHDB_CLUSTER_ID` if you set it. + +### Turn off a server you don't use + +Copilot starts both MCP servers in every session. If you only use one of them, turn off the other: + +- **Copilot CLI:** run `/mcp disable cockroachdb-toolbox` (or `cockroachdb-cloud`) in a session, and the choice persists across sessions. To skip a server for a single session, start Copilot with `--disable-mcp-server cockroachdb-toolbox`. +- **VS Code:** in the Extensions view, right-click the server under **MCP SERVERS - INSTALLED** and select **Disable**. ### MCP configuration files @@ -58,12 +104,14 @@ The default backend is the **MCP Toolbox** over stdio. The managed **CockroachDB
CockroachDB Cloud MCP Server (OAuth/API key) -The official [managed MCP server](https://www.cockroachlabs.com/blog/cockroachdb-ai-agents-managed-mcp-server/) is hosted by Cockroach Labs and requires no infrastructure setup. Authenticate via OAuth 2.1 (PKCE) or a service account API key. Read-only by default; write access requires explicit consent. +The official [managed MCP server](https://www.cockroachlabs.com/blog/cockroachdb-ai-agents-managed-mcp-server/) is hosted by Cockroach Labs and requires no infrastructure setup. The plugin already includes it as `cockroachdb-cloud`; add your own entry to limit it to one cluster or to use a service account API key. With OAuth 2.1 (PKCE), the consent screen asks you to grant read access, write access, or both (scopes `mcp:read` and `mcp:write`). + +Give your entry its own name, such as `cockroachdb-cloud-cluster`, so it doesn't clash with the plugin's `cockroachdb-cloud` server, and turn the plugin's server off. In VS Code (`mcp.json`): ```json { "servers": { - "cockroachdb-cloud": { + "cockroachdb-cloud-cluster": { "type": "http", "url": "https://cockroachlabs.cloud/mcp", "headers": { @@ -74,7 +122,49 @@ The official [managed MCP server](https://www.cockroachlabs.com/blog/cockroachdb } ``` -For headless or autonomous agents, add an `Authorization: Bearer {your-service-account-api-key}` header. See the [quickstart guide](https://www.cockroachlabs.com/docs/cockroachcloud/connect-to-the-cockroachdb-cloud-mcp-server) for detailed setup. +With the Copilot CLI: + +```bash +copilot mcp add --transport http cockroachdb-cloud-cluster https://cockroachlabs.cloud/mcp --header "mcp-cluster-id: {your-cluster-id}" +``` + +Leave out the `mcp-cluster-id` header to reach every cluster your user or service account can access. For headless or autonomous agents, add an `Authorization: Bearer {your-service-account-api-key}` header. See the [quickstart guide](https://www.cockroachlabs.com/docs/cockroachcloud/connect-to-the-cockroachdb-cloud-mcp-server) for detailed setup. +
+ +
+CockroachDB MCP Server (first-party, self-hosted) + +[CockroachDB MCP Server](https://github.com/cockroachdb/cockroachdb-mcp-server) is Cockroach Labs' own MCP server for clusters you run yourself. By default it registers only read-only tools, such as `list_databases`, `list_tables`, `get_table_schema`, `select_query`, `explain_query`, `show_statement`, and `show_running_queries`. Setting `CRDB_MCP_ENABLE_WRITE_QUERIES=true` adds `create_database`, `create_table`, `insert_rows`, `update_rows`, and `delete_rows`, and the server refuses an `UPDATE` or `DELETE` without a `WHERE` clause. + +**Install:** `go install github.com/cockroachdb/cockroachdb-mcp-server@latest` (Go 1.26+). Linux and Windows binaries and a Docker image are listed on the [releases page](https://github.com/cockroachdb/cockroachdb-mcp-server/releases). There are no prebuilt macOS binaries, so on macOS use `go install` or Docker. + +**Configure** with certificate authentication (recommended). In VS Code (`mcp.json`): + +```json +{ + "servers": { + "cockroachdb-mcp-server": { + "command": "cockroachdb-mcp-server", + "env": { + "CRDB_HOST": "your-cluster-host", + "CRDB_USERNAME": "ai_agent", + "CRDB_SSL_MODE": "verify-full", + "CRDB_SSL_CA_PATH": "/certs/ca.crt", + "CRDB_SSL_CERTFILE": "/certs/client.ai_agent.crt", + "CRDB_SSL_KEYFILE": "/certs/client.ai_agent.key" + } + } + } +} +``` + +For a local `--insecure` development cluster with the Copilot CLI: + +```bash +copilot mcp add cockroachdb-mcp-server --env CRDB_DATABASE_URL="postgresql://root@localhost:26257/defaultdb?sslmode=disable" --env CRDB_MCP_ALLOW_INSECURE_DB=true -- cockroachdb-mcp-server +``` + +Password authentication is off unless you set `CRDB_MCP_ALLOW_PASSWORD_AUTH=true`. If the server doesn't start, give the binary's absolute path as the `command`, since apps launched outside a terminal may not search `~/go/bin`. The plugin's SQL safety hook applies to the bundled Toolbox server; this server enforces its own guardrails. See the [server's README](https://github.com/cockroachdb/cockroachdb-mcp-server#configuration) for every setting.
@@ -91,10 +181,11 @@ See the [ccloud reference](https://www.cockroachlabs.com/docs/cockroachcloud/ccl ### MCP Backends -| Backend | Status | Transport | Use Case | -|------------------------|-----------|-----------------|--------------------------------------------------------------------------------------------------------------------------------------| -| `cockroachdb-toolbox` | Active | stdio | Any CockroachDB cluster via [MCP Toolbox](https://github.com/googleapis/mcp-toolbox) | -| `cockroachdb-cloud` | Active | Streamable HTTP | [Managed MCP Server](https://www.cockroachlabs.com/blog/cockroachdb-ai-agents-managed-mcp-server/), CockroachDB Cloud (OAuth/API key) | +| Backend | Status | Transport | Use Case | +|--------------------------|-----------|-----------------|---------------------------------------------------------------------------------------------------------------------------------------| +| `cockroachdb-toolbox` | Active | stdio | Any CockroachDB cluster via [MCP Toolbox](https://github.com/googleapis/mcp-toolbox) | +| `cockroachdb-cloud` | Active | Streamable HTTP | [Managed MCP Server](https://www.cockroachlabs.com/blog/cockroachdb-ai-agents-managed-mcp-server/), CockroachDB Cloud (OAuth/API key) | +| `cockroachdb-mcp-server` | Available | stdio, HTTPS | First-party server for self-hosted clusters (not shipped; add it yourself, see Alternative MCP Backends) | ### Skills diff --git a/tools.yaml b/tools.yaml index 453c048..1802d2f 100644 --- a/tools.yaml +++ b/tools.yaml @@ -21,9 +21,11 @@ tools: type: cockroachdb-execute-sql source: crdb-source description: | - Execute ad-hoc SQL statements against a CockroachDB database. - Use this for any SQL: SELECT queries, DDL statements (CREATE, ALTER, DROP), - and DML statements (INSERT, UPDATE, DELETE). + Execute an ad-hoc SQL statement against a CockroachDB database. + The connection is read-only by default: SELECT, SHOW, and EXPLAIN work, + while writes and schema changes (INSERT, UPDATE, DELETE, CREATE, ALTER, + DROP) are rejected unless write mode is enabled in the Toolbox + configuration (enableWriteMode: true). cockroachdb-list-schemas: type: cockroachdb-list-schemas From a2e9d2fdfd481d5c9d36acb54ad7eb5623341797 Mon Sep 17 00:00:00 2001 From: viragtripathi Date: Fri, 9 Oct 2026 03:14:17 -0400 Subject: [PATCH 2/3] fix: make the SQL safety hook fire for plugin installs Copilot CLI names a plugin's MCP tools -, so the validate-sql matcher, which listed only mcp__cockroachdb-toolbox__cockroachdb-execute-sql, never matched cockroachdb-toolbox-cockroachdb-execute-sql and the hook never ran. The matcher now lists the Copilot CLI name, the Claude Code plugin name, and the bare name. Claude Code also stopped substituting ${CLAUDE_PLUGIN_ROOT} inside shell-form commands; it exports the variable instead, and the bootstrap read the placeholder from inside single quotes. The bootstrap now uses the substituted path when a host provides one, as Copilot CLI does, and otherwise reads CLAUDE_PLUGIN_ROOT, COPILOT_PLUGIN_ROOT, or PLUGIN_ROOT. The Windows long-path prefix, which evaluated to \?\ instead of \\?\, is now built with chr(92). test-hooks.sh gains cases for an unsubstituted placeholder with each exported variable, in both the sh and PowerShell forms; the old bootstrap fails them. CONTRIBUTING and AGENTS document the bootstrap and the per-host tool names. --- .github/hooks/cockroachdb.json | 6 +++--- AGENTS.md | 3 ++- CONTRIBUTING.md | 8 +++++--- scripts/test-hooks.sh | 21 ++++++++++++++++++++- 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/.github/hooks/cockroachdb.json b/.github/hooks/cockroachdb.json index 95be03b..ff320a1 100644 --- a/.github/hooks/cockroachdb.json +++ b/.github/hooks/cockroachdb.json @@ -2,11 +2,11 @@ "hooks": { "PreToolUse": [ { - "matcher": "mcp__cockroachdb-toolbox__cockroachdb-execute-sql", + "matcher": "cockroachdb-toolbox-cockroachdb-execute-sql|mcp__plugin_cockroachdb_cockroachdb-toolbox__cockroachdb-execute-sql|mcp__cockroachdb-toolbox__cockroachdb-execute-sql", "hooks": [ { "type": "command", - "command": "python3 -c 'import sys, os, runpy; p = os.path.normpath(r\"${CLAUDE_PLUGIN_ROOT}/scripts/validate-sql.py\"); p = (\"\\\\?\\\\\" + p) if os.name == \"nt\" else p; runpy.run_path(p, run_name=\"__main__\")'; exit 0", + "command": "python3 -c 'import os, runpy; r = r\"${CLAUDE_PLUGIN_ROOT}\"; r = r if r and not r.startswith(\"$\") else next((os.environ[k] for k in (\"CLAUDE_PLUGIN_ROOT\", \"COPILOT_PLUGIN_ROOT\", \"PLUGIN_ROOT\") if os.environ.get(k)), \"\"); p = os.path.normpath(os.path.join(r, \"scripts\", \"validate-sql.py\")) if r else \"\"; p = chr(92) * 2 + \"?\" + chr(92) + p if p and os.name == \"nt\" else p; p and os.path.isfile(p) and runpy.run_path(p, run_name=\"__main__\")'; exit 0", "timeout": 10 } ] @@ -18,7 +18,7 @@ "hooks": [ { "type": "command", - "command": "python3 -c 'import sys, os, runpy; p = os.path.normpath(r\"${CLAUDE_PLUGIN_ROOT}/scripts/check-sql-files.py\"); p = (\"\\\\?\\\\\" + p) if os.name == \"nt\" else p; runpy.run_path(p, run_name=\"__main__\")'; exit 0", + "command": "python3 -c 'import os, runpy; r = r\"${CLAUDE_PLUGIN_ROOT}\"; r = r if r and not r.startswith(\"$\") else next((os.environ[k] for k in (\"CLAUDE_PLUGIN_ROOT\", \"COPILOT_PLUGIN_ROOT\", \"PLUGIN_ROOT\") if os.environ.get(k)), \"\"); p = os.path.normpath(os.path.join(r, \"scripts\", \"check-sql-files.py\")) if r else \"\"; p = chr(92) * 2 + \"?\" + chr(92) + p if p and os.name == \"nt\" else p; p and os.path.isfile(p) and runpy.run_path(p, run_name=\"__main__\")'; exit 0", "timeout": 15 } ] diff --git a/AGENTS.md b/AGENTS.md index a0d5ae6..c028ac2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,7 +11,8 @@ The CockroachDB plugin for GitHub Copilot: plugin manifest (`plugin.json` at the - **Never edit `.github/skills/` by hand.** Skills come from the [cockroachdb-skills](https://github.com/cockroachlabs/cockroachdb-skills) submodule via `scripts/sync-skills.sh`, which flattens the domain-grouped upstream tree (Copilot requires `.github/skills//` with the directory equal to the frontmatter `name:`), dereferences symlinks, and runs `scripts/transform-skills-for-copilot.py`. Edit the sync or transform scripts, or contribute upstream, never the synced output. - **The transform exists to pass the awesome-copilot marketplace lint** (`@microsoft/vally`), which enforces three things per skill: no file links outside the skill directory, every file under `references/` reachable from SKILL.md, and SKILL.md at most 500 lines. Two subtleties the transform already handles, so keep them intact: link reachability ignores links inside fenced code blocks (an unbalanced fence count hides everything after it), and the over-length split must only cut at `## ` headers outside code fences. - **Two MCP configs with different top-level keys, both on purpose:** `.mcp.json` (plugin installs) uses `mcpServers`; `.vscode/mcp.json` (workspace use) uses `servers`. Keep them in sync when adding a backend. -- **Hook scripts emit both output contracts** (top-level `permissionDecision`/`additionalContext` for the Copilot CLI, `hookSpecificOutput`/`systemMessage` for VS Code and Claude Code) and accept `tool_input`, `toolInput`, or `toolArgs` with `file_path`, `filePath`, or `path`. VS Code ignores hook matchers and runs hooks on every tool call, so scripts must exit fast and silent on irrelevant input. Keep the `runpy` long-path bootstrap and trailing `; exit 0` in hook commands. +- **No `env` block or optional header placeholders in `.mcp.json`.** It is read by both Copilot CLI and VS Code, which expand an unset variable differently (literal `${VAR}` in Copilot CLI, empty in VS Code), and Toolbox uses either as the value. Both clients pass the parent environment to stdio servers, so `tools.yaml` defaults apply when a variable is unset (issue cockroachdb/claude-plugin#27). +- **Hook scripts emit both output contracts** (top-level `permissionDecision`/`additionalContext` for the Copilot CLI, `hookSpecificOutput`/`systemMessage` for VS Code and Claude Code) and accept `tool_input`, `toolInput`, or `toolArgs` with `file_path`, `filePath`, or `path`. VS Code ignores hook matchers and runs hooks on every tool call, so scripts must exit fast and silent on irrelevant input. Keep the `runpy` long-path bootstrap, its fallback to the exported plugin-root variables (Claude Code exports `CLAUDE_PLUGIN_ROOT` instead of substituting it), and the trailing `; exit 0` in hook commands. Matchers must list each host's tool name: Copilot CLI uses `-`, Claude Code uses `mcp__plugin____`. - **Agent frontmatter is `name` and `description` only.** Do not add Claude-only fields like `model:` or `color:`; the user's selected Copilot model should apply. - **Never bump versions by hand.** Release Please owns `version` in `plugin.json`, `.release-please-manifest.json`, and `CHANGELOG.md`. Conventional commits: `fix:`/`feat:` cut a release, `chore:`/`docs:` do not. - **No counts in descriptions.** Counts go stale; name the things instead. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bbee6af..cd3a55d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -111,15 +111,17 @@ Never bump the version in `plugin.json` or `.release-please-manifest.json` manua - Hook scripts must be Python 3 with **no external dependencies** (stdlib only). - Read JSON from stdin, write JSON to stdout. PreToolUse blocks a tool with `hookSpecificOutput.permissionDecision` set to `deny`. - VS Code ignores hook matchers (hooks run on every tool invocation), so scripts must exit early when the input is not relevant. VS Code also passes camelCase tool inputs (`filePath`) where Claude uses snake_case (`file_path`); accept both. -- Load hook scripts through the long-path-safe bootstrap below instead of passing the script path straight to `python3`. The plugin root can resolve to a deeply nested cache path that exceeds the 260-character `MAX_PATH` limit on Windows; passing the path directly makes Python fail to open the script. The bootstrap loads the script with `runpy`, prefixes the path with the `\\?\` long-path escape on Windows, keeps it inside single quotes so paths with spaces still work, and uses `; exit 0` so a failed bootstrap never disrupts editing: +- Load hook scripts through the bootstrap below instead of passing the script path straight to `python3`. Hosts treat `${CLAUDE_PLUGIN_ROOT}` differently: Copilot CLI substitutes it into the command text, while Claude Code exports it as an environment variable and leaves the placeholder to the shell, which never expands it inside single quotes. The bootstrap uses the substituted path when there is one and otherwise reads `CLAUDE_PLUGIN_ROOT`, `COPILOT_PLUGIN_ROOT`, or `PLUGIN_ROOT`. The plugin root can also exceed the 260-character Windows `MAX_PATH` limit, so on Windows the bootstrap prefixes the `\\?\` long-path escape, built with `chr(92)` so no escaping layer can change it. It exits quietly when it can't find the script, and `; exit 0` keeps a failed bootstrap from disrupting editing: ```json - "command": "python3 -c 'import sys, os, runpy; p = os.path.normpath(r\"${CLAUDE_PLUGIN_ROOT}/scripts/your-script.py\"); p = (\"\\\\?\\\\\" + p) if os.name == \"nt\" else p; runpy.run_path(p, run_name=\"__main__\")'; exit 0" + "command": "python3 -c 'import os, runpy; r = r\"${CLAUDE_PLUGIN_ROOT}\"; r = r if r and not r.startswith(\"$\") else next((os.environ[k] for k in (\"CLAUDE_PLUGIN_ROOT\", \"COPILOT_PLUGIN_ROOT\", \"PLUGIN_ROOT\") if os.environ.get(k)), \"\"); p = os.path.normpath(os.path.join(r, \"scripts\", \"your-script.py\")) if r else \"\"; p = chr(92) * 2 + \"?\" + chr(92) + p if p and os.name == \"nt\" else p; p and os.path.isfile(p) and runpy.run_path(p, run_name=\"__main__\")'; exit 0" ``` +- Hook matchers must name the tool the way each host does. Copilot CLI joins the server and tool names with a hyphen (`cockroachdb-toolbox-cockroachdb-execute-sql`), and Claude Code uses `mcp__plugin_cockroachdb_cockroachdb-toolbox__cockroachdb-execute-sql`. VS Code ignores matchers. ### MCP Configuration - `.mcp.json` (top-level `mcpServers`) is used by plugin installs; `.vscode/mcp.json` (top-level `servers`) is used in workspaces. Keep both in sync when adding a backend. -- Use `${ENV_VAR}` syntax for environment variable references. +- Don't give the Toolbox server an `env` block. Copilot CLI passes an unset `${VAR}` through as the literal text `${VAR}`, VS Code turns it into an empty string, and Toolbox uses either one as the value, so its `tools.yaml` defaults never apply (issue cockroachdb/claude-plugin#27). Both clients pass the parent environment to stdio servers, so set variables still reach Toolbox and unset ones fall back to `tools.yaml`. +- No placeholder works in both clients for an optional header: `${VAR}` stays literal in Copilot CLI when unset, and `${VAR:-}` stays literal in VS Code. That's why the plugin's `.mcp.json` leaves out the optional `mcp-cluster-id` header. `.vscode/mcp.json` is read only by VS Code, where `${env:VAR}` resolves to an empty string, which the Cloud server treats as absent. - The `tools.yaml` file uses Toolbox v1.1.0 map-based format with `${VAR:default}` syntax for defaults. ### Skills diff --git a/scripts/test-hooks.sh b/scripts/test-hooks.sh index 55c966f..4065244 100755 --- a/scripts/test-hooks.sh +++ b/scripts/test-hooks.sh @@ -22,11 +22,18 @@ hook_cmd() { # $1=event $2=root } # check: desc, event, root, stdin, want_rc, mode(empty|all), space-separated tokens +# The command runs with no plugin-root variables set unless ENV_ROOT is set, in +# which case ENV_NAME (default CLAUDE_PLUGIN_ROOT) is exported with that value, +# the way hosts export the plugin root to hook processes. check() { local desc="$1" event="$2" root="$3" stdin="$4" want_rc="$5" mode="$6" tokens="${7:-}" local cmd out rc ok=1 cmd="$(hook_cmd "$event" "$root")" - out="$(printf '%s' "$stdin" | sh -c "$cmd" 2>/dev/null)"; rc=$? + if [ -n "${ENV_ROOT:-}" ]; then + out="$(printf '%s' "$stdin" | env -u CLAUDE_PLUGIN_ROOT -u COPILOT_PLUGIN_ROOT -u PLUGIN_ROOT "${ENV_NAME:-CLAUDE_PLUGIN_ROOT}=$ENV_ROOT" sh -c "$cmd" 2>/dev/null)"; rc=$? + else + out="$(printf '%s' "$stdin" | env -u CLAUDE_PLUGIN_ROOT -u COPILOT_PLUGIN_ROOT -u PLUGIN_ROOT sh -c "$cmd" 2>/dev/null)"; rc=$? + fi [ "$rc" = "$want_rc" ] || ok=0 if [ "$mode" = "empty" ]; then [ -z "$out" ] || ok=0 @@ -57,6 +64,18 @@ check "non-SQL edit: no output" PostToolUse "$ROOT" "{\"tool_input\":{ check "PreToolUse fail-open on bad root" PreToolUse '${CLAUDE_PLUGIN_ROOT}' '{"tool_input":{"sql":"DROP DATABASE x"}}' 0 empty check "PostToolUse fail-open on bad root" PostToolUse '${CLAUDE_PLUGIN_ROOT}' "{\"tool_input\":{\"file_path\":\"$TMP/a.sql\"}}" 0 empty +# placeholder left unsubstituted, plugin root exported (Claude Code today; Copilot +# CLI exports CLAUDE_PLUGIN_ROOT, COPILOT_PLUGIN_ROOT, and PLUGIN_ROOT to hooks). +# Single quotes keep the shell from expanding the placeholder, so the bootstrap +# must read the exported variable. PowerShell hosts rewrite the placeholder to +# ${env:NAME}, which single quotes also leave alone. +ENV_ROOT="$ROOT" check "PreToolUse uses exported CLAUDE_PLUGIN_ROOT" PreToolUse '${CLAUDE_PLUGIN_ROOT}' '{"tool_input":{"sql":"DROP DATABASE x"}}' 0 all "permissionDecision hookSpecificOutput" +ENV_ROOT="$ROOT" ENV_NAME=COPILOT_PLUGIN_ROOT check "PreToolUse uses exported COPILOT_PLUGIN_ROOT" PreToolUse '${CLAUDE_PLUGIN_ROOT}' '{"toolArgs":{"sql":"DROP DATABASE x"}}' 0 all "permissionDecision" +ENV_ROOT="$ROOT" ENV_NAME=PLUGIN_ROOT check "PreToolUse uses exported PLUGIN_ROOT" PreToolUse '${CLAUDE_PLUGIN_ROOT}' '{"toolArgs":{"sql":"DROP DATABASE x"}}' 0 all "permissionDecision" +ENV_ROOT="$ROOT" check "PostToolUse uses exported CLAUDE_PLUGIN_ROOT" PostToolUse '${CLAUDE_PLUGIN_ROOT}' "{\"tool_input\":{\"file_path\":\"$TMP/a.sql\"}}" 0 all "systemMessage additionalContext" +ENV_ROOT="$ROOT" check "PreToolUse uses exported root in PowerShell form" PreToolUse '${env:CLAUDE_PLUGIN_ROOT}' '{"tool_input":{"sql":"DROP DATABASE x"}}' 0 all "permissionDecision hookSpecificOutput" +ENV_ROOT="$TMP/missing" check "PreToolUse fail-open when exported root has no script" PreToolUse '${CLAUDE_PLUGIN_ROOT}' '{"tool_input":{"sql":"DROP DATABASE x"}}' 0 empty + echo if [ "$fails" -eq 0 ]; then echo "All hook regression tests passed." From 50d0d638b15eb5f6411a509be04e4b0e01eb2ecb Mon Sep 17 00:00:00 2001 From: viragtripathi Date: Fri, 9 Oct 2026 03:26:26 -0400 Subject: [PATCH 3/3] fix: lint files edited with apply_patch GPT models in Copilot edit files with apply_patch, whose input is the patch text rather than an object with a file path, so check-sql-files.py silently skipped those edits. It now reads the edited paths from the patch headers and resolves them against the payload's cwd. The lint matcher also lists Copilot's runtime edit tool names next to the Claude names. The scripts are shared verbatim with the codex plugin, which also adds a --codex output mode. The copilot hooks don't pass it, so their output is unchanged. Both scripts now accept a tool input that arrives as a JSON string, as Copilot's camelCase hook format sends it. --- .github/hooks/cockroachdb.json | 2 +- scripts/check-sql-files.py | 109 ++++++++++++++++++++++++++------- scripts/test-hooks.sh | 14 +++++ scripts/validate-sql.py | 51 +++++++++++---- 4 files changed, 141 insertions(+), 35 deletions(-) diff --git a/.github/hooks/cockroachdb.json b/.github/hooks/cockroachdb.json index ff320a1..0ee6b32 100644 --- a/.github/hooks/cockroachdb.json +++ b/.github/hooks/cockroachdb.json @@ -14,7 +14,7 @@ ], "PostToolUse": [ { - "matcher": "Write|Edit|MultiEdit", + "matcher": "Write|Edit|MultiEdit|create|edit|str_replace_editor|apply_patch", "hooks": [ { "type": "command", diff --git a/scripts/check-sql-files.py b/scripts/check-sql-files.py index 3702a28..76e2b5d 100755 --- a/scripts/check-sql-files.py +++ b/scripts/check-sql-files.py @@ -5,6 +5,12 @@ The advisory message is emitted under both ``systemMessage`` (VS Code Copilot / Claude Code, user-visible) and ``additionalContext`` (GitHub Copilot CLI, model-visible), so the lint surfaces regardless of which tool runs the hook. + +OpenAI Codex and Copilot edit files with ``apply_patch``, whose input is patch +text (in ``tool_input.command`` for Codex, as the whole ``tool_input`` string +for Copilot) with no file path key, so the paths come from the patch headers. +Codex also rejects unknown top-level output keys, so the Codex hooks.json +passes ``--codex``. """ import json @@ -14,45 +20,109 @@ SQL_EXTENSIONS = {".sql", ".go", ".py", ".js", ".ts", ".java", ".rb"} - - -def main(): - try: - data = json.load(sys.stdin) - except (json.JSONDecodeError, EOFError): - sys.exit(0) - - # tool_input/toolInput (VS Code / Claude) or toolArgs (Copilot CLI). File - # path key varies by tool: file_path (Claude), filePath / path (others). +CODEX = "--codex" in sys.argv[1:] +# apply_patch file headers (codex-rs/apply-patch/src/parser.rs). +PATCH_FILE_MARKERS = ("*** Add File: ", "*** Update File: ", "*** Move to: ") + + +def patch_file_paths(patch_text, base_dir): + paths = [] + for line in patch_text.splitlines(): + for marker in PATCH_FILE_MARKERS: + if line.startswith(marker): + path = line[len(marker):].strip() + if path and base_dir and not os.path.isabs(path): + path = os.path.join(base_dir, path) + if path: + paths.append(path) + return paths + + +def candidate_files(data): + # tool_input/toolInput (Codex / VS Code / Claude) or toolArgs (Copilot CLI). + # File path key varies by tool: file_path (Claude), filePath / path (others). tool_input = ( data.get("tool_input") or data.get("toolInput") or data.get("toolArgs") or {} ) + base_dir = data.get("cwd") or "" + # Copilot passes apply_patch input as the raw patch text, and its camelCase + # format can pass other arguments as a JSON string. + if isinstance(tool_input, str): + if "*** Begin Patch" in tool_input: + return patch_file_paths(tool_input, base_dir) + try: + tool_input = json.loads(tool_input) + except json.JSONDecodeError: + return [] if not isinstance(tool_input, dict): - sys.exit(0) + return [] file_path = ( tool_input.get("file_path") or tool_input.get("filePath") or tool_input.get("path") or "" ) - if not file_path: + if file_path: + return [file_path] + command = tool_input.get("command") + if isinstance(command, str) and "*** Begin Patch" in command: + return patch_file_paths(command, base_dir) + return [] + + +def main(): + try: + data = json.load(sys.stdin) + except (json.JSONDecodeError, EOFError): + sys.exit(0) + if not isinstance(data, dict): + sys.exit(0) + + findings = [] + for file_path in dict.fromkeys(candidate_files(data)): + warnings = lint(file_path) + if warnings: + findings.append((file_path, warnings)) + if not findings: sys.exit(0) + if len(findings) == 1: + message = "CockroachDB lint: " + " ".join(findings[0][1]) + else: + message = "CockroachDB lint: " + " ".join( + f"{os.path.basename(path)}: {' '.join(warnings)}" + for path, warnings in findings + ) + if CODEX: + payload = { + "systemMessage": message, + "hookSpecificOutput": { + "hookEventName": "PostToolUse", + "additionalContext": message, + }, + } + else: + payload = {"systemMessage": message, "additionalContext": message} + json.dump(payload, sys.stdout) + sys.exit(0) + + +def lint(file_path): ext = os.path.splitext(file_path)[1] if ext not in SQL_EXTENSIONS: - sys.exit(0) + return [] if not os.path.isfile(file_path): - sys.exit(0) + return [] try: with open(file_path, "r", encoding="utf-8", errors="ignore") as f: content = f.read() except OSError: - sys.exit(0) + return [] warnings = [] @@ -88,14 +158,7 @@ def main(): "Use cockroachdb-jdbc-wrapper RetryableExecutor." ) - if warnings: - message = "CockroachDB lint: " + " ".join(warnings) - json.dump({ - "systemMessage": message, - "additionalContext": message, - }, sys.stdout) - - sys.exit(0) + return warnings if __name__ == "__main__": diff --git a/scripts/test-hooks.sh b/scripts/test-hooks.sh index 4065244..89e64e8 100755 --- a/scripts/test-hooks.sh +++ b/scripts/test-hooks.sh @@ -60,6 +60,12 @@ check "lint via path key" PostToolUse "$ROOT" "{\"tool_input\":{ check "lint via toolArgs container (CLI)" PostToolUse "$ROOT" "{\"toolArgs\":{\"filePath\":\"$TMP/a.sql\"}}" 0 all "additionalContext" check "non-SQL edit: no output" PostToolUse "$ROOT" "{\"tool_input\":{\"file_path\":\"$TMP/a.md\"}}" 0 empty +# apply_patch edits (GPT models): the input is patch text, so paths come from +# the patch headers, resolved against the payload's cwd. +PATCH='*** Begin Patch\n*** Add File: a.sql\n+CREATE TABLE t (id SERIAL PRIMARY KEY);\n*** End Patch\n' +check "lint apply_patch string input" PostToolUse "$ROOT" "{\"cwd\":\"$TMP\",\"tool_name\":\"apply_patch\",\"tool_input\":\"$PATCH\"}" 0 all "systemMessage additionalContext" +check "lint apply_patch command input" PostToolUse "$ROOT" "{\"cwd\":\"$TMP\",\"tool_name\":\"apply_patch\",\"tool_input\":{\"command\":\"$PATCH\"}}" 0 all "systemMessage" + # fail-open when plugin root is missing / unsubstituted (issues #20, #23) check "PreToolUse fail-open on bad root" PreToolUse '${CLAUDE_PLUGIN_ROOT}' '{"tool_input":{"sql":"DROP DATABASE x"}}' 0 empty check "PostToolUse fail-open on bad root" PostToolUse '${CLAUDE_PLUGIN_ROOT}' "{\"tool_input\":{\"file_path\":\"$TMP/a.sql\"}}" 0 empty @@ -76,6 +82,14 @@ ENV_ROOT="$ROOT" check "PostToolUse uses exported CLAU ENV_ROOT="$ROOT" check "PreToolUse uses exported root in PowerShell form" PreToolUse '${env:CLAUDE_PLUGIN_ROOT}' '{"tool_input":{"sql":"DROP DATABASE x"}}' 0 all "permissionDecision hookSpecificOutput" ENV_ROOT="$TMP/missing" check "PreToolUse fail-open when exported root has no script" PreToolUse '${CLAUDE_PLUGIN_ROOT}' '{"tool_input":{"sql":"DROP DATABASE x"}}' 0 empty +# The SQL hook must match the name Copilot CLI gives the Toolbox tool: +# -, with no mcp__ prefix. +if python3 -c 'import json,sys; m=json.load(open(sys.argv[1]))["hooks"]["PreToolUse"][0]["matcher"]; sys.exit(0 if "cockroachdb-toolbox-cockroachdb-execute-sql" in m.split("|") else 1)' "$HOOKS"; then + echo "ok - PreToolUse matcher includes the Copilot CLI tool name" +else + echo "FAIL - PreToolUse matcher includes the Copilot CLI tool name"; fails=$((fails + 1)) +fi + echo if [ "$fails" -eq 0 ]; then echo "All hook regression tests passed." diff --git a/scripts/validate-sql.py b/scripts/validate-sql.py index 87d03ab..e53f17d 100755 --- a/scripts/validate-sql.py +++ b/scripts/validate-sql.py @@ -9,31 +9,49 @@ - VS Code Copilot and Claude Code read ``hookSpecificOutput`` (and ``systemMessage``). Emitting both keeps this one script working across all three surfaces. + +OpenAI Codex rejects any top-level key it does not know (its hook output +schema uses deny_unknown_fields), so the Codex hooks.json passes ``--codex`` +and this script then emits only the Codex shape. """ import json import re import sys +CODEX = "--codex" in sys.argv[1:] + def deny(reason): - json.dump({ + hook_output = { + "hookEventName": "PreToolUse", "permissionDecision": "deny", "permissionDecisionReason": reason, - "hookSpecificOutput": { - "hookEventName": "PreToolUse", + } + if CODEX: + payload = {"hookSpecificOutput": hook_output} + else: + payload = { "permissionDecision": "deny", "permissionDecisionReason": reason, - }, - }, sys.stdout) + "hookSpecificOutput": hook_output, + } + json.dump(payload, sys.stdout) sys.exit(0) def warn(message): - json.dump({ - "systemMessage": message, - "additionalContext": message, - }, sys.stdout) + if CODEX: + payload = { + "systemMessage": message, + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "additionalContext": message, + }, + } + else: + payload = {"systemMessage": message, "additionalContext": message} + json.dump(payload, sys.stdout) sys.exit(0) @@ -43,8 +61,19 @@ def main(): except (json.JSONDecodeError, EOFError): sys.exit(0) - # tool_input (VS Code / Claude) or toolArgs (Copilot CLI camelCase) - tool_input = data.get("tool_input") or data.get("toolArgs") or {} + if not isinstance(data, dict): + sys.exit(0) + + # tool_input (Codex / VS Code / Claude) or toolArgs (Copilot CLI camelCase, + # which can arrive as a JSON string) + tool_input = ( + data.get("tool_input") or data.get("toolInput") or data.get("toolArgs") or {} + ) + if isinstance(tool_input, str): + try: + tool_input = json.loads(tool_input) + except json.JSONDecodeError: + sys.exit(0) if not isinstance(tool_input, dict): sys.exit(0) sql = tool_input.get("sql", "") or tool_input.get("statement", "")