Skip to content

fix: make the bundled MCP servers and SQL safety hook work out of the box - #7

Merged
viragtripathi merged 3 commits into
mainfrom
fix/mcp-env-and-hooks
Oct 9, 2026
Merged

viragtripathi merged 3 commits into
mainfrom
fix/mcp-env-and-hooks

Conversation

@viragtripathi

@viragtripathi viragtripathi commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What

  • Stops 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 couldn't build its connection URL. VS Code turns it into an empty string, which Toolbox also uses as the value instead of 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. .vscode/mcp.json gets the same change.
  • Drops the optional mcp-cluster-id header from the plugin config. With COCKROACHDB_CLUSTER_ID unset, Copilot CLI sent the literal placeholder and the Cloud server failed with HTTP 400. No placeholder is safe in both clients (${VAR:-} stays literal in VS Code), so the plugin config leaves the header out, and the connection reaches every cluster the user's role allows. .vscode/mcp.json keeps it, because VS Code resolves ${env:VAR} to an empty string, which the server treats as absent.
  • Makes the SQL safety hook fire. Copilot CLI names a plugin's MCP tools <server>-<tool>, so the validate-sql matcher (mcp__cockroachdb-toolbox__cockroachdb-execute-sql) never matched cockroachdb-toolbox-cockroachdb-execute-sql. The matcher now lists the Copilot CLI name, the Claude Code plugin name, and the bare name. The hook bootstrap now also falls back to the exported CLAUDE_PLUGIN_ROOT, COPILOT_PLUGIN_ROOT, or PLUGIN_ROOT when a host doesn't substitute the placeholder, as current Claude Code doesn't, and the Windows long-path prefix, which evaluated to \?\ instead of \\?\, is fixed.
  • Lints files edited with apply_patch. GPT models in Copilot edit files with apply_patch, whose input is patch text with no file path key, so check-sql-files.py skipped those edits. It now reads the paths from the patch headers, and the lint matcher lists Copilot's runtime edit tool names. The scripts are shared verbatim with the codex plugin, which adds a --codex output mode that these hooks don't use.
  • Docs. Toolbox runs in read-only mode, 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. CONTRIBUTING and AGENTS explain why the plugin config has no env block, and document the bootstrap and the per-host tool names.

Testing

Run on macOS with GitHub Copilot CLI 1.0.67, MCP Toolbox 1.12.0, and a local CockroachDB v25.4 cluster, using the installed plugin with its files temporarily swapped:

  • A recording hook showed what Copilot CLI sends: tool_name is cockroachdb-toolbox-cockroachdb-execute-sql for the Toolbox SQL tool and Write for file writes. The hook process gets CLAUDE_PLUGIN_ROOT, COPILOT_PLUGIN_ROOT, and PLUGIN_ROOT, runs from the plugin root, and receives ${CLAUDE_PLUGIN_ROOT} already substituted.
  • With the new hooks file, asking Copilot to run TRUNCATE through Toolbox is blocked by the hook ("TRUNCATE is blocked by CockroachDB plugin safety hook"). With the current hooks file, the hook never ran, and only Toolbox's read-only mode stopped the statement.
  • A stub toolbox on PATH showed that the current .mcp.json sends unset variables to Toolbox as literal ${COCKROACHDB_HOST} text, and that variables Copilot doesn't list are still inherited by the server.
  • The current config's Cloud server fails with "HTTP 400: invalid cluster_id". The same server without the header goes to the OAuth flow ("requires authentication").
  • bash scripts/test-hooks.sh: all pass, including new cases for an unsubstituted placeholder with each exported variable in the sh and PowerShell forms (the current bootstrap fails those cases), apply_patch edits passed as a patch string or a command object (the current check-sql-files.py prints nothing for them), and a check that the SQL matcher lists the Copilot CLI tool name.

Not tested: VS Code and Windows (the long-path prefix fix was checked by evaluating the string). The VS Code expansion behavior comes from its source code.

Related to cockroachdb/claude-plugin#27.

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
Copilot CLI names a plugin's MCP tools <server>-<tool>, 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.
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.
@viragtripathi
viragtripathi merged commit 66fcab9 into main Oct 9, 2026
1 check passed
@viragtripathi
viragtripathi deleted the fix/mcp-env-and-hooks branch October 9, 2026 07:35
@github-actions github-actions Bot mentioned this pull request Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant