Repository navigation
fix: make the bundled MCP servers and SQL safety hooks work out of the box - #28
Merged
Merged
Conversation
The cockroachdb-toolbox-http entry in .mcp.json pointed at http://127.0.0.1:5000/mcp, so it failed to connect at startup for anyone not already running Toolbox in HTTP mode on that port. Its execute-sql tool also bypassed the SQL safety hook, which only matches the stdio server, and Anthropic's plugin directory validation blocks MCP server URLs that aren't https. The stdio cockroachdb-toolbox server provides the same tools. The README already listed the HTTP backend as an optional alternative; it now shows how to add it with claude mcp add, and the backends table says it isn't shipped.
Claude Code passes an unset bare ${VAR} reference through as the literal
text ${VAR}. With COCKROACHDB_* unset, Toolbox received strings like
${COCKROACHDB_PORT} and failed to parse its connection URL, which also
defeated the defaults in tools.yaml. The Cloud server's mcp-cluster-id
header had the same problem and failed with HTTP 400.
The Toolbox env block now uses ${VAR:-default} with the same defaults as
tools.yaml. Toolbox uses an empty value as-is rather than falling back,
so every connection setting except the password gets a real default. The
cluster ID header defaults to empty, which the Cloud server treats as
absent, so the connection reaches every cluster the user's role allows.
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 from /mcp, the optional
cluster ID, and the first-party CockroachDB MCP Server as an
alternative, and it uses the current /plugin install command.
CONTRIBUTING and AGENTS no longer recommend bare ${VAR} references and
document the scoped tool names that hook matchers need.
Fixes #27
The validate-sql hook never ran for anyone who installed the plugin.
Claude Code names tools from a plugin's own MCP servers
mcp__plugin_<plugin>_<server>__<tool>, and the matcher only listed the
bare mcp__cockroachdb-toolbox__cockroachdb-execute-sql name, which
Claude Code compares as an exact string. The matcher now lists both
names.
Both hooks also failed to find their scripts on current Claude Code.
The bootstrap read ${CLAUDE_PLUGIN_ROOT} from inside single quotes, but
Claude Code no longer substitutes the placeholder in shell-form
commands. It exports CLAUDE_PLUGIN_ROOT and leaves the shell to expand
it, which single quotes prevent, so Python looked for a literal
${CLAUDE_PLUGIN_ROOT}/scripts path and the hook failed open. The
bootstrap now uses the substituted path when a host provides one and
the exported variable otherwise, and exits quietly when neither leads
to the script.
The Windows long-path prefix also evaluated to \?\ instead of \\?\. It
is now built with chr(92) so no escaping layer can change it.
test-hooks.sh gains cases that leave the placeholder unsubstituted and
export CLAUDE_PLUGIN_ROOT, in both the sh and PowerShell forms. The old
bootstrap fails them.
viragtripathi
changed the title
The documented bootstrap snippet still showed the version that reads ${CLAUDE_PLUGIN_ROOT} from inside single quotes. It now matches hooks/hooks.json, and AGENTS notes the environment-variable fallback as part of the load-bearing pattern.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
cockroachdb-toolbox-httpserver. It pointed athttp://127.0.0.1:5000/mcp, so it failed at startup unless Toolbox was already running in HTTP mode. Its execute-sql tool also skipped the SQL safety hook, and Anthropic's plugin directory blocks non-https MCP server URLs.${VAR}through as literal text, so Toolbox received strings like${COCKROACHDB_PORT}and couldn't build its connection URL. The env block now uses${VAR:-default}with the same defaults astools.yaml. The optionalmcp-cluster-idheader defaults to empty, which the Cloud server treats as absent; before, an unset cluster ID made the server fail with HTTP 400.mcp__plugin_cockroachdb_<server>__<tool>, and the matcher only listed the bare name, sovalidate-sqlnever fired. Separately, current Claude Code no longer substitutes${CLAUDE_PLUGIN_ROOT}inside shell-form commands, and the bootstrap read it from inside single quotes, so both hooks failed open. The bootstrap now falls back to the exportedCLAUDE_PLUGIN_ROOT, and the Windows long-path prefix, which evaluated to\?\instead of\\?\, is fixed./mcp, the optional cluster ID, the first-party CockroachDB MCP Server as an alternative backend, and the current/plugin installcommand. CONTRIBUTING and AGENTS no longer recommend bare${VAR}references and document the scoped tool names hook matchers need.Testing
Run on macOS with Claude Code 2.1.295, MCP Toolbox 1.12.0, and a local CockroachDB v25.4 cluster.
bash scripts/test-hooks.sh: all pass, including new cases that leave the placeholder unsubstituted and exportCLAUDE_PLUGIN_ROOT, in both the sh and PowerShell forms. The previous bootstrap fails those cases.claude plugin validate .: passes, with the same pre-existing warning aboutCLAUDE.mdat the repo root.claude -p --plugin-dir:TRUNCATEis now blocked by the hook ("TRUNCATE is blocked by CockroachDB plugin safety hook"). With the previous matcher the hook never ran, and only Toolbox's read-only mode stopped the statement..sqlfile that usesSERIALtriggers the PostToolUse lint.localhost,26257,root,defaultdb,require, and an empty password.claude mcp listwith the cluster ID unset: the old header form fails with "HTTP 400: invalid cluster_id", and the new form shows "Needs authentication".CRDB_READONLY_VIOLATIONin the shipped configuration and allows them withenableWriteMode: true.CRDB_MCP_ENABLE_WRITE_QUERIES=true, and refuses aDELETEwithoutWHERE.Not tested: Windows (the long-path prefix fix was checked by evaluating the string), Claude Desktop, and Cowork.
Fixes #27.