feat(j19): replace arcane_help tool with help action/subaction routing#5
feat(j19): replace arcane_help tool with help action/subaction routing#5
Conversation
…in help and CI drift test
…action into arcane tool
📝 WalkthroughWalkthroughIntegrates help into the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ArcaneHandler as Arcane Handler
participant DispatchHelp as dispatchHelp()
participant Response
Client->>ArcaneHandler: Request (action=help OR action=<domain>, subaction=help)
ArcaneHandler->>ArcaneHandler: Validate request, compute optional domain hint
ArcaneHandler->>DispatchHelp: dispatchHelp(domain?)
DispatchHelp->>DispatchHelp: Lookup domain or return full help markdown
DispatchHelp-->>ArcaneHandler: Return help text
ArcaneHandler-->>Response: Wrap into MCP response
Response-->>Client: Deliver help content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR replaces the standalone arcane_help MCP tool with a unified help action/subaction routing inside the existing arcane tool, centralizing and expanding help/docs output across all Arcane domains.
Changes:
- Removed the
arcane_helptool and introducedarcane action=helpplussubaction=helpper domain. - Added a new
dispatchHelp()dispatcher providing top-level and per-domain documentation strings. - Added/updated tests to cover help routing and to detect domain/help drift in CI.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/mcp/tools/dispatch/dispatchers.test.ts | Adds unit tests for dispatchHelp() output and basic drift checking. |
| tests/mcp/tools/arcane.test.ts | Adds integration-style tests asserting help routing behavior through the arcane tool handler. |
| src/mcp/tools/dispatch/index.ts | Re-exports the new dispatchHelp dispatcher. |
| src/mcp/tools/dispatch/help.ts | Introduces the new help text module and dispatchHelp(domain?) routing. |
| src/mcp/tools/arcane.ts | Removes arcane_help registration; wires help into schema and routing logic for arcane. |
| package.json | Bumps package version to 1.2.0. |
| CHANGELOG.md | Documents the help routing change and notes the removal of arcane_help as breaking. |
Comments suppressed due to low confidence (1)
src/mcp/tools/arcane.ts:91
- The published tool schema still requires
subactionand restricts it to the operation enum. That conflicts with the documented/advertised usagearcane action=help(no subaction) andarcane action=help subaction=container(domain hint), which schema-driven MCP clients may reject or not suggest. Consider makingsubactionoptional and/or expanding the schema to allow domain names whenaction === "help"(or adding a dedicateddomainfield for the hint), then keep the existing runtime guard for non-help actions.
subaction: z
.enum([
"list",
"get",
"create",
"update",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/mcp/tools/dispatch/help.ts
Outdated
| export function dispatchHelp(domain?: string): string { | ||
| if (!domain || domain === "all") return HELP_FULL; | ||
| const help = DOMAIN_HELP[domain]; | ||
| if (!help) | ||
| throw new ValidationError( | ||
| `Unknown domain '${domain}'. Valid domains: environment, project, container, image, network, volume, system, image-update, vulnerability, registry, gitops`, |
There was a problem hiding this comment.
dispatchHelp() duplicates the valid-domain list in the error message as a hardcoded string, separate from DOMAIN_HELP keys. This will drift the next time a domain is added/renamed. Consider generating the list from Object.keys(DOMAIN_HELP) (and possibly sorting it) so the thrown message always matches the actual supported domains.
| export function dispatchHelp(domain?: string): string { | |
| if (!domain || domain === "all") return HELP_FULL; | |
| const help = DOMAIN_HELP[domain]; | |
| if (!help) | |
| throw new ValidationError( | |
| `Unknown domain '${domain}'. Valid domains: environment, project, container, image, network, volume, system, image-update, vulnerability, registry, gitops`, | |
| const VALID_DOMAINS = Object.keys(DOMAIN_HELP).sort().join(", "); | |
| export function dispatchHelp(domain?: string): string { | |
| if (!domain || domain === "all") return HELP_FULL; | |
| const help = DOMAIN_HELP[domain]; | |
| if (!help) | |
| throw new ValidationError( | |
| `Unknown domain '${domain}'. Valid domains: ${VALID_DOMAINS}`, |
| "name": "arcane-mcp", | ||
| "mcpName": "tv.tootie/arcane-mcp", | ||
| "version": "1.1.5", | ||
| "version": "1.2.0", |
There was a problem hiding this comment.
This release is explicitly marked as a breaking change (removal of arcane_help), but the package version is bumped from 1.1.5 to 1.2.0. Since the repo states it adheres to Semantic Versioning, this should be a major version bump (e.g., 2.0.0) and the changelog header should match.
| "version": "1.2.0", | |
| "version": "2.0.0", |
| it("dispatchHelp(undefined) returns string containing all 11 domain names", () => { | ||
| const help = dispatchHelp(undefined); | ||
| expect(typeof help).toBe("string"); | ||
| for (const domain of ALL_DOMAINS) { | ||
| expect(help).toContain(domain); | ||
| } | ||
| }); | ||
|
|
||
| it('dispatchHelp("container") returns string containing stop, restart, create, params', () => { | ||
| const help = dispatchHelp("container"); | ||
| expect(help).toContain("stop"); | ||
| expect(help).toContain("restart"); | ||
| expect(help).toContain("create"); | ||
| expect(help).toContain("params"); | ||
| }); | ||
|
|
||
| it('dispatchHelp("registry") returns string containing "NOT environment-scoped"', () => { | ||
| const help = dispatchHelp("registry"); | ||
| expect(help).toContain("NOT environment-scoped"); | ||
| }); | ||
|
|
||
| it('dispatchHelp("unknown-domain") throws ValidationError', () => { | ||
| expect(() => dispatchHelp("unknown-domain")).toThrow(ValidationError); | ||
| }); | ||
|
|
||
| it('dispatchHelp("all") returns same result as dispatchHelp(undefined)', () => { | ||
| expect(dispatchHelp("all")).toBe(dispatchHelp(undefined)); | ||
| }); | ||
|
|
||
| it("CI drift test: each known domain appears in top-level help output", () => { | ||
| const fullHelp = dispatchHelp(undefined); | ||
| for (const domain of ALL_DOMAINS) { | ||
| expect( | ||
| fullHelp, | ||
| `Expected domain '${domain}' to appear in top-level help`, | ||
| ).toContain(domain); | ||
| } |
There was a problem hiding this comment.
The last test ("CI drift test") re-checks that each domain appears in the top-level help output, which is already asserted by the earlier test "dispatchHelp(undefined) returns string containing all 11 domain names". Consider consolidating these to a single test to reduce duplication while keeping the better failure message.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ca244705a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "help", | ||
| ]) | ||
| .describe("Resource type to operate on"), | ||
| subaction: z |
There was a problem hiding this comment.
action=help without requiring a subaction
The new tool description and changelog instruct clients to call action=help, but subaction is still a required enum field in the declared input schema. In MCP clients that validate tool arguments against the schema (and in model tool planners constrained by that schema), {"action":"help"} is rejected before this handler runs, so the advertised top-level help path is unavailable after removing arcane_help.
Useful? React with 👍 / 👎.
src/mcp/tools/arcane.ts
Outdated
| "delete-backup", | ||
| "status", | ||
| "sync", | ||
| "help", | ||
| ]) |
There was a problem hiding this comment.
action=help schema
The handler explicitly supports domain hints for help (action=help with subaction equal to a domain like container), but the subaction enum does not include any domain names. As a result, schema-validated MCP calls cannot provide those values, so this routing path is unreachable in real protocol usage despite being tested via direct handler invocation.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 10-17: The markdown headings in CHANGELOG.md (e.g., "## [1.2.0] -
2026-04-05", "### Added", "### Removed") violate MD022 by not being surrounded
by blank lines; fix this by inserting a blank line before and after each heading
so each heading is separated from surrounding content (ensure there is an empty
line above "## [1.2.0] - 2026-04-05", an empty line between that H2 and the "###
Added" H3, and empty lines before and after "### Added" and "### Removed").
In `@package.json`:
- Line 4: The CI is failing due to a version mismatch: package.json has
"version": "1.2.0" while plugin.json still contains "version": "1.1.5"; open
plugin.json and update its top-level "version" field to "1.2.0" so both files
match (ensure you update the exact "version" property in plugin.json to mirror
package.json).
In `@src/mcp/tools/arcane.ts`:
- Around line 148-172: Extract the duplicated DOMAINS list into a single
exported constant in help.ts (e.g., export const VALID_DOMAINS) and update
arcane.ts to import and use that constant instead of defining DOMAINS locally;
specifically replace the local DOMAINS used in the help branch (the array
checked against subaction and passed to dispatchHelp) with the imported
VALID_DOMAINS so DOMAIN_HELP and the routing logic stay in sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 18c34f61-32e4-4f9d-aba4-bfe65c71f277
📒 Files selected for processing (7)
CHANGELOG.mdpackage.jsonsrc/mcp/tools/arcane.tssrc/mcp/tools/dispatch/help.tssrc/mcp/tools/dispatch/index.tstests/mcp/tools/arcane.test.tstests/mcp/tools/dispatch/dispatchers.test.ts
| "name": "arcane-mcp", | ||
| "mcpName": "tv.tootie/arcane-mcp", | ||
| "version": "1.1.5", | ||
| "version": "1.2.0", |
There was a problem hiding this comment.
Version mismatch with plugin.json causing CI failure.
The pipeline reports package.json=1.2.0 but plugin.json=1.1.5. Update plugin.json to 1.2.0 to resolve the version sync check failure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 4, The CI is failing due to a version mismatch:
package.json has "version": "1.2.0" while plugin.json still contains "version":
"1.1.5"; open plugin.json and update its top-level "version" field to "1.2.0" so
both files match (ensure you update the exact "version" property in plugin.json
to mirror package.json).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="package.json">
<violation number="1" location="package.json:4">
P2: This declares a breaking change but only applies a minor version bump. Use a major SemVer bump to avoid publishing a breaking API change as backward-compatible.</violation>
</file>
<file name="src/mcp/tools/dispatch/help.ts">
<violation number="1" location="src/mcp/tools/dispatch/help.ts:581">
P3: Generate the valid-domain portion of this error message from `DOMAIN_HELP` keys instead of hardcoding it. Keeping two independent domain lists here will drift and can return misleading validation guidance.</violation>
</file>
<file name="CHANGELOG.md">
<violation number="1" location="CHANGELOG.md:10">
P2: This release documents a breaking change but uses a minor version (`1.2.0`); bump to a major version to stay SemVer-compliant.</violation>
</file>
<file name="src/mcp/tools/arcane.ts">
<violation number="1" location="src/mcp/tools/arcane.ts:164">
P1: The new `action=help` routing is incompatible with the input schema: `subaction` is still required and does not allow domain names, so documented help calls are rejected before reaching this logic.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| "name": "arcane-mcp", | ||
| "mcpName": "tv.tootie/arcane-mcp", | ||
| "version": "1.1.5", | ||
| "version": "1.2.0", |
There was a problem hiding this comment.
P2: This declares a breaking change but only applies a minor version bump. Use a major SemVer bump to avoid publishing a breaking API change as backward-compatible.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 4:
<comment>This declares a breaking change but only applies a minor version bump. Use a major SemVer bump to avoid publishing a breaking API change as backward-compatible.</comment>
<file context>
@@ -1,7 +1,7 @@
"name": "arcane-mcp",
"mcpName": "tv.tootie/arcane-mcp",
- "version": "1.1.5",
+ "version": "1.2.0",
"description": "MCP server for the Arcane Docker management API",
"type": "module",
</file context>
| "version": "1.2.0", | |
| "version": "2.0.0", |
|
|
||
| ## [Unreleased] | ||
|
|
||
| ## [1.2.0] - 2026-04-05 |
There was a problem hiding this comment.
P2: This release documents a breaking change but uses a minor version (1.2.0); bump to a major version to stay SemVer-compliant.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At CHANGELOG.md, line 10:
<comment>This release documents a breaking change but uses a minor version (`1.2.0`); bump to a major version to stay SemVer-compliant.</comment>
<file context>
@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
+## [1.2.0] - 2026-04-05
+
+### Added
</file context>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/plugin/HOOKS.md (1)
8-14:⚠️ Potential issue | 🟡 MinorFile location diagram is inconsistent with actual script locations.
The "File location" section still shows scripts under
hooks/scripts/, but scripts have been moved tobin/. This contradicts the updated command paths in the JSON example below (lines 27, 32, 44).📝 Proposed fix to update the file location diagram
## File locationhooks/
-├── hooks.json # Hook definitions
-└── scripts/
- ├── sync-env.sh # Sync userConfig to .env
- ├── fix-env-perms.sh # Fix .env permissions
- └── ensure-ignore-files.sh # Verify ignore patterns
+└── hooks.json # Hook definitions
+bin/
+├── sync-env.sh # Sync userConfig to .env
+├── fix-env-perms.sh # Fix .env permissions
+└── ensure-ignore-files.sh # Verify ignore patterns🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plugin/HOOKS.md` around lines 8 - 14, Update the "File location" diagram in HOOKS.md so it matches the actual layout used by the examples: move the three scripts (sync-env.sh, fix-env-perms.sh, ensure-ignore-files.sh) out of the old scripts/ subtree into a top-level bin/ directory and leave hooks.json at the hooks/ root; ensure the ASCII tree shows hooks/ containing hooks.json and a separate bin/ directory containing the three script filenames to match the command paths referenced in the JSON examples.src/mcp/tools/arcane.ts (1)
87-132:⚠️ Potential issue | 🔴 CriticalHelp routing with domain hints is unreachable due to schema constraints.
The
subactionenum (lines 87-132) lists only operation names (list,get,create, etc.) andhelp, but the help routing logic at lines 150-151 expects domain names (container,environment,project, etc.) to select domain-specific help. Documented calls likeaction=help subaction=containerfail schema validation before reaching the handler.The schema must include domain names in the
subactionenum and mark it optional to support:
action=help subaction=<domain>→ domain-specific help (routed via line 150-151)action=<domain> subaction=help→ domain-specific help (routed via line 158-161)action=help(no subaction) → full help (routed via line 150-151 with undefined domainHint)Proposed fix
Add domain names (
environment,project,container,image,network,volume,system,image-update,vulnerability,registry,gitops) to thesubactionenum and make it optional with.optional()chained before.describe().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcp/tools/arcane.ts` around lines 87 - 132, The subaction z.enum currently only contains operation names and prevents domain-hint help calls from passing validation; update the subaction schema (the z.enum(...) assigned to subaction) to include the domain names environment, project, container, image, network, volume, system, image-update, vulnerability, registry, and gitops, and chain .optional() before the existing .describe() so subaction can be omitted (supporting action=help with no subaction) — keep the existing operation strings as well so the same schema supports both action=<domain> subaction=help and action=help subaction=<domain>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/hooks.json`:
- Around line 9-11: The SessionStart hook in hooks.json now contains an inline
npm install command that syncs package.json and runs npm install (the command
string starting with diff -q "${CLAUDE_PLUGIN_ROOT}/package.json" ... npm
install), but this behavior isn't documented; update docs/plugin/HOOKS.md by
adding a SessionStart example entry that describes this npm install hook,
include the exact purpose and behavior (compare root vs data package.json, copy
when different, run npm install, and cleanup on failure), and show the command
string or an equivalent snippet and its timeout setting so the documentation
matches the hooks.json configuration.
In `@src/mcp/tools/arcane.ts`:
- Around line 69-70: Tests and docs still reference a removed tool named
arcane_help; restore compatibility by either reintroducing an alias named
"arcane_help" that delegates to the existing arcane tool implementation in
src/mcp/tools/arcane.ts (so existing behavior and help output remain identical),
or update tests and documentation to remove/replace references to arcane_help:
modify tests/test_live.sh to stop asserting / calling arcane_help (remove
protocol checks for arcane_help and the
test_arcane_help_http/test_arcane_help_stdio functions) and update
TEST_COVERAGE.md to reflect the current exposed tools list. Use the symbol
"arcane_help" to add the alias or update the test function names
test_arcane_help_http and test_arcane_help_stdio and the protocol assertions
accordingly.
In `@tests/mcp/tools/arcane.test.ts`:
- Around line 1048-1086: The current tests call the handler directly via
setupTool()/callTool(), bypassing MCP SDK Zod validation and letting invalid
subaction values (e.g., "container") slip through; add integration-style tests
that exercise the registered tool contract through the MCP SDK validation layer
instead of the raw handler so schema errors are surfaced. Specifically, create
tests that invoke the public tool entrypoint (the same function used by the SDK
registration) rather than the raw handler from setupTool, and include cases that
pass invalid subaction values (like "container") expecting validation errors;
reference the existing test utilities (setupTool and callTool) and HELP_DOMAINS
to ensure help routing behavior is still asserted but only after validating via
the SDK contract. Ensure the new tests assert that inputs failing the Zod schema
produce validation failures and that valid help queries still return the
expected domain/help text.
---
Outside diff comments:
In `@docs/plugin/HOOKS.md`:
- Around line 8-14: Update the "File location" diagram in HOOKS.md so it matches
the actual layout used by the examples: move the three scripts (sync-env.sh,
fix-env-perms.sh, ensure-ignore-files.sh) out of the old scripts/ subtree into a
top-level bin/ directory and leave hooks.json at the hooks/ root; ensure the
ASCII tree shows hooks/ containing hooks.json and a separate bin/ directory
containing the three script filenames to match the command paths referenced in
the JSON examples.
In `@src/mcp/tools/arcane.ts`:
- Around line 87-132: The subaction z.enum currently only contains operation
names and prevents domain-hint help calls from passing validation; update the
subaction schema (the z.enum(...) assigned to subaction) to include the domain
names environment, project, container, image, network, volume, system,
image-update, vulnerability, registry, and gitops, and chain .optional() before
the existing .describe() so subaction can be omitted (supporting action=help
with no subaction) — keep the existing operation strings as well so the same
schema supports both action=<domain> subaction=help and action=help
subaction=<domain>.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8ee9d46d-3974-49c6-9d4c-b0abd38533c7
📒 Files selected for processing (17)
bin/check-docker-security.shbin/check-no-baked-env.shbin/check-outdated-deps.shbin/ensure-ignore-files.shbin/fix-env-perms.shbin/smoke-test.tsbin/sync-env.shdocs/INVENTORY.mddocs/plugin/HOOKS.mddocs/repo/SCRIPTS.mdhooks/hooks.jsonhooks/scripts/ensure-ignore-files.shsrc/mcp/tools/arcane.tssrc/mcp/tools/dispatch/help.tssrc/mcp/tools/dispatch/index.tstests/mcp/tools/arcane.test.tstests/mcp/tools/dispatch/dispatchers.test.ts
💤 Files with no reviewable changes (1)
- hooks/scripts/ensure-ignore-files.sh
hooks/hooks.json
Outdated
| "command": "diff -q \"${CLAUDE_PLUGIN_ROOT}/package.json\" \"${CLAUDE_PLUGIN_DATA}/package.json\" >/dev/null 2>&1 || (cd \"${CLAUDE_PLUGIN_DATA}\" && cp \"${CLAUDE_PLUGIN_ROOT}/package.json\" . && npm install) || rm -f \"${CLAUDE_PLUGIN_DATA}/package.json\"", | ||
| "timeout": 60 | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if the inline npm install command is documented in HOOKS.md
echo "=== Checking if npm install hook is documented in HOOKS.md ==="
if grep -q "npm install" docs/plugin/HOOKS.md; then
echo "✓ npm install is documented"
else
echo "✗ npm install hook is NOT documented in HOOKS.md"
fi
echo ""
echo "=== Current SessionStart hooks in hooks.json ==="
cat hooks/hooks.json | grep -A 30 '"SessionStart"'Repository: jmagar/arcane-mcp
Length of output: 1206
Update docs/plugin/HOOKS.md to document the new SessionStart npm install hook.
The hooks.json file now includes an inline npm install command in the SessionStart hooks (lines 9-11) that handles package.json synchronization. This command is not documented in docs/plugin/HOOKS.md, creating a documentation gap. Add the npm install hook and its logic to the SessionStart example in the documentation to keep it in sync with the actual configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/hooks.json` around lines 9 - 11, The SessionStart hook in hooks.json
now contains an inline npm install command that syncs package.json and runs npm
install (the command string starting with diff -q
"${CLAUDE_PLUGIN_ROOT}/package.json" ... npm install), but this behavior isn't
documented; update docs/plugin/HOOKS.md by adding a SessionStart example entry
that describes this npm install hook, include the exact purpose and behavior
(compare root vs data package.json, copy when different, run npm install, and
cleanup on failure), and show the command string or an equivalent snippet and
its timeout setting so the documentation matches the hooks.json configuration.
| "Orchestrate Arcane Docker resources. Manages environments, projects, containers, images, networks, volumes, system operations, image updates, vulnerabilities, registries, and GitOps syncs. For image-update:check: pass id (Arcane imageId) OR params.imageRef (image reference like nginx:latest) — not both. Run with action=help for top-level help, or action=<domain> subaction=help for per-domain docs.", | ||
| { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'arcane_help' tests/test_live.sh tests/TEST_COVERAGE.md -C2Repository: jmagar/arcane-mcp
Length of output: 7625
🏁 Script executed:
#!/bin/bash
# Check tool definitions and exports in arcane.ts
cd src/mcp/tools
wc -l arcane.tsRepository: jmagar/arcane-mcp
Length of output: 74
🏁 Script executed:
# Search for tool registration/definition patterns in arcane.ts
rg -n 'arcane_help|registerTool|export.*tool' src/mcp/tools/arcane.ts -A2 -B2Repository: jmagar/arcane-mcp
Length of output: 43
🏁 Script executed:
# Look for the complete tool list/registry
rg -n 'const.*tools|tools.*=|register|Tool\(' src/mcp/tools/arcane.ts | head -20Repository: jmagar/arcane-mcp
Length of output: 205
🏁 Script executed:
# Read the tool registration section of arcane.ts
sed -n '60,120p' src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 2042
🏁 Script executed:
# Check the entire tool definitions - look for tool list structure
rg -n 'tools|Tool' src/mcp/tools/arcane.ts | head -30Repository: jmagar/arcane-mcp
Length of output: 99
🏁 Script executed:
# Look for where tools are registered/listed
sed -n '1,100p' src/mcp/tools/arcane.ts | tail -50Repository: jmagar/arcane-mcp
Length of output: 1872
🏁 Script executed:
# Search for all server.tool calls in arcane.ts
rg -n 'server\.tool' src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 78
🏁 Script executed:
# Check the end of the file to see if there are multiple tool registrations
tail -100 src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 4607
🏁 Script executed:
# Search for arcane_help anywhere in the entire src directory
rg -l 'arcane_help' src/Repository: jmagar/arcane-mcp
Length of output: 43
🏁 Script executed:
# Verify that arcane_help is not defined anywhere in src
rg 'arcane_help' src/ && echo "FOUND" || echo "NOT_FOUND"Repository: jmagar/arcane-mcp
Length of output: 70
🏁 Script executed:
# Double-check: count how many tools are registered in arcane.ts
rg -c 'server\.tool\(' src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 62
Tests still expect arcane_help tool that no longer exists in implementation.
This PR removes the arcane_help tool from src/mcp/tools/arcane.ts, but tests/test_live.sh and tests/TEST_COVERAGE.md actively reference it in protocol assertions and test functions:
- Protocol assertions check for
arcane_helpin the tools list (lines 419, 454 in test_live.sh) - Test functions
test_arcane_help_httpandtest_arcane_help_stdio(lines 462–490 in test_live.sh) call the removed tool - Documentation describes
arcane_helpas one of the two exposed tools (TEST_COVERAGE.md line 20)
These tests will fail when the tool list no longer includes arcane_help.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/mcp/tools/arcane.ts` around lines 69 - 70, Tests and docs still reference
a removed tool named arcane_help; restore compatibility by either reintroducing
an alias named "arcane_help" that delegates to the existing arcane tool
implementation in src/mcp/tools/arcane.ts (so existing behavior and help output
remain identical), or update tests and documentation to remove/replace
references to arcane_help: modify tests/test_live.sh to stop asserting / calling
arcane_help (remove protocol checks for arcane_help and the
test_arcane_help_http/test_arcane_help_stdio functions) and update
TEST_COVERAGE.md to reflect the current exposed tools list. Use the symbol
"arcane_help" to add the alias or update the test function names
test_arcane_help_http and test_arcane_help_stdio and the protocol assertions
accordingly.
| describe("help routing", () => { | ||
| let callTool: ReturnType<typeof setupTool>["callTool"]; | ||
| beforeEach(() => { | ||
| ({ callTool } = setupTool(createMockServices())); | ||
| }); | ||
|
|
||
| it("action=help subaction=list returns top-level help containing domain names", async () => { | ||
| const result = await callTool({ action: "help", subaction: "list" }); | ||
| expect(result.isError).toBeFalsy(); | ||
| const text = result.content[0].text; | ||
| expect(text).toContain("environment"); | ||
| expect(text).toContain("container"); | ||
| expect(text).toContain("registry"); | ||
| }); | ||
|
|
||
| it("action=help subaction=help returns top-level help (not an error)", async () => { | ||
| const result = await callTool({ action: "help", subaction: "help" }); | ||
| expect(result.isError).toBeFalsy(); | ||
| expect(result.content[0].text).toContain("environment"); | ||
| }); | ||
|
|
||
| it("action=help subaction=container returns container-specific help", async () => { | ||
| const result = await callTool({ action: "help", subaction: "container" }); | ||
| expect(result.isError).toBeFalsy(); | ||
| expect(result.content[0].text).toContain("Container Domain"); | ||
| }); | ||
|
|
||
| it("action=container subaction=help returns container help", async () => { | ||
| const result = await callTool({ action: "container", subaction: "help" }); | ||
| expect(result.isError).toBeFalsy(); | ||
| expect(result.content[0].text).toContain("Container Domain"); | ||
| }); | ||
|
|
||
| it("action=registry subaction=help contains NOT environment-scoped note", async () => { | ||
| const result = await callTool({ action: "registry", subaction: "help" }); | ||
| expect(result.isError).toBeFalsy(); | ||
| expect(result.content[0].text).toContain("NOT environment-scoped"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Show direct handler extraction bypassing SDK validation:"
rg -n --type=ts '_registeredTools|getHandler\(|return entry.handler' tests/mcp/tools/arcane.test.ts -C2
echo
echo "Show help tests using direct handler with domain-hint subaction:"
rg -n --type=ts 'describe\("help routing"|action:\s*"help"|subaction:\s*"container"' tests/mcp/tools/arcane.test.ts -C2Repository: jmagar/arcane-mcp
Length of output: 2480
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Finding Zod schema for arcane tool ==="
rg -n "schema|Zod|z\." tests/mcp/tools/arcane.test.ts | head -30
echo
echo "=== Look for handler definition and schema in main source files ==="
find . -name "*.ts" -type f ! -path "./node_modules/*" | xargs rg -l "registerArcaneTool|schema" | head -20Repository: jmagar/arcane-mcp
Length of output: 354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for the arcane tool registration and schema definition ==="
rg -n "registerArcaneTool|const.*schema|export.*schema" --type=ts -B2 -A5 | grep -A10 -B10 "arcane\|schema"Repository: jmagar/arcane-mcp
Length of output: 3266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find the handler implementation ==="
find . -name "*.ts" -type f ! -path "./node_modules/*" | xargs rg -l "action.*help.*container" | head -10Repository: jmagar/arcane-mcp
Length of output: 133
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Read arcane.ts to see tool definition and schema ==="
wc -l src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 145
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== First part: tool registration, schema definition ==="
head -150 src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 4852
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for schema in arcane.ts ==="
rg -n "schema|Zod|z\." src/mcp/tools/arcane.ts | head -40Repository: jmagar/arcane-mcp
Length of output: 451
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Read handler implementation (lines 150-250) ==="
sed -n '150,250p' src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 3225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Read handler implementation showing help dispatch ==="
sed -n '145,200p' src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 2269
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find HELP_DOMAINS definition ==="
rg -n "HELP_DOMAINS" src/mcp/tools/arcane.ts -B2 -A5Repository: jmagar/arcane-mcp
Length of output: 601
🏁 Script executed:
#!/bin/bash
set -euo pipefuralkıpio pipefail
echo "=== Check where HELP_DOMAINS is defined (dispatch module) ==="
rg -n "HELP_DOMAINS\s*=" src/mcp/tools/dispatch/ -B2 -A5Repository: jmagar/arcane-mcp
Length of output: 120
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check where HELP_DOMAINS is defined (dispatch module) ==="
rg -n "HELP_DOMAINS\s*=" src/mcp/tools/dispatch/ -B2 -A10Repository: jmagar/arcane-mcp
Length of output: 942
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find dispatch/index.ts to see HELP_DOMAINS export ==="
cat src/mcp/tools/dispatch/index.tsRepository: jmagar/arcane-mcp
Length of output: 783
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check what's in DOMAIN_HELP to confirm container is present ==="
rg -n "container.*:" src/mcp/tools/dispatch/help.ts | head -20Repository: jmagar/arcane-mcp
Length of output: 1634
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Verify the full subaction enum in schema ==="
sed -n '95,135p' src/mcp/tools/arcane.ts | grep -A40 'subaction'Repository: jmagar/arcane-mcp
Length of output: 109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Get full subaction enum definition ==="
sed -n '101,133p' src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 1043
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== List keys in DOMAIN_HELP ==="
rg -n "^ \w+:" src/mcp/tools/dispatch/help.ts | head -20Repository: jmagar/arcane-mcp
Length of output: 399
Tests bypass Zod schema validation and accept invalid subaction values.
These tests extract the raw handler directly, bypassing MCP SDK schema validation. For example, line 1070 passes subaction: "container", which violates the registered schema (the subaction enum does not include domain names). While the handler accepts this for help routing (checking HELP_DOMAINS), the schema mismatch should be caught by integration tests that validate inputs through the registered tool contract. Add tests that call the tool through the MCP SDK validation layer to ensure the public contract matches the implementation.
As per coding guidelines, **/*.{test,spec}.{js,jsx,ts,tsx,py}: "Write tests for all public functions and critical business logic".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/mcp/tools/arcane.test.ts` around lines 1048 - 1086, The current tests
call the handler directly via setupTool()/callTool(), bypassing MCP SDK Zod
validation and letting invalid subaction values (e.g., "container") slip
through; add integration-style tests that exercise the registered tool contract
through the MCP SDK validation layer instead of the raw handler so schema errors
are surfaced. Specifically, create tests that invoke the public tool entrypoint
(the same function used by the SDK registration) rather than the raw handler
from setupTool, and include cases that pass invalid subaction values (like
"container") expecting validation errors; reference the existing test utilities
(setupTool and callTool) and HELP_DOMAINS to ensure help routing behavior is
still asserted but only after validating via the SDK contract. Ensure the new
tests assert that inputs failing the Zod schema produce validation failures and
that valid help queries still return the expected domain/help text.
There was a problem hiding this comment.
3 issues found across 17 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/hooks.json">
<violation number="1" location="hooks/hooks.json:9">
P2: Syncing only package.json means lockfile-only dependency updates won’t be applied in the plugin data dir. Include package-lock.json in the diff/copy so dependency changes tracked in the lockfile trigger a reinstall.</violation>
<violation number="2" location="hooks/hooks.json:9">
P3: Document this new SessionStart package sync/install command in `docs/plugin/HOOKS.md` so the documented hook sequence matches `hooks/hooks.json`.</violation>
</file>
<file name="docs/plugin/HOOKS.md">
<violation number="1" location="docs/plugin/HOOKS.md:27">
P3: Update the “File location” tree/description to reference `bin/` so the doc matches the new command paths.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| { | ||
| "type": "command", | ||
| "command": "${CLAUDE_PLUGIN_ROOT}/hooks/scripts/sync-env.sh", | ||
| "command": "diff -q \"${CLAUDE_PLUGIN_ROOT}/package.json\" \"${CLAUDE_PLUGIN_DATA}/package.json\" >/dev/null 2>&1 || (cd \"${CLAUDE_PLUGIN_DATA}\" && cp \"${CLAUDE_PLUGIN_ROOT}/package.json\" . && npm install) || rm -f \"${CLAUDE_PLUGIN_DATA}/package.json\"", |
There was a problem hiding this comment.
P2: Syncing only package.json means lockfile-only dependency updates won’t be applied in the plugin data dir. Include package-lock.json in the diff/copy so dependency changes tracked in the lockfile trigger a reinstall.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/hooks.json, line 9:
<comment>Syncing only package.json means lockfile-only dependency updates won’t be applied in the plugin data dir. Include package-lock.json in the diff/copy so dependency changes tracked in the lockfile trigger a reinstall.</comment>
<file context>
@@ -6,12 +6,17 @@
{
"type": "command",
- "command": "${CLAUDE_PLUGIN_ROOT}/hooks/scripts/sync-env.sh",
+ "command": "diff -q \"${CLAUDE_PLUGIN_ROOT}/package.json\" \"${CLAUDE_PLUGIN_DATA}/package.json\" >/dev/null 2>&1 || (cd \"${CLAUDE_PLUGIN_DATA}\" && cp \"${CLAUDE_PLUGIN_ROOT}/package.json\" . && npm install) || rm -f \"${CLAUDE_PLUGIN_DATA}/package.json\"",
+ "timeout": 60
+ },
</file context>
| "command": "diff -q \"${CLAUDE_PLUGIN_ROOT}/package.json\" \"${CLAUDE_PLUGIN_DATA}/package.json\" >/dev/null 2>&1 || (cd \"${CLAUDE_PLUGIN_DATA}\" && cp \"${CLAUDE_PLUGIN_ROOT}/package.json\" . && npm install) || rm -f \"${CLAUDE_PLUGIN_DATA}/package.json\"", | |
| "command": "diff -q \"${CLAUDE_PLUGIN_ROOT}/package.json\" \"${CLAUDE_PLUGIN_DATA}/package.json\" >/dev/null 2>&1 && diff -q \"${CLAUDE_PLUGIN_ROOT}/package-lock.json\" \"${CLAUDE_PLUGIN_DATA}/package-lock.json\" >/dev/null 2>&1 || (cd \"${CLAUDE_PLUGIN_DATA}\" && cp \"${CLAUDE_PLUGIN_ROOT}/package.json\" \"${CLAUDE_PLUGIN_ROOT}/package-lock.json\" . && npm install) || rm -f \"${CLAUDE_PLUGIN_DATA}/package.json\" \"${CLAUDE_PLUGIN_DATA}/package-lock.json\"", |
| { | ||
| "type": "command", | ||
| "command": "${CLAUDE_PLUGIN_ROOT}/hooks/scripts/sync-env.sh", | ||
| "command": "diff -q \"${CLAUDE_PLUGIN_ROOT}/package.json\" \"${CLAUDE_PLUGIN_DATA}/package.json\" >/dev/null 2>&1 || (cd \"${CLAUDE_PLUGIN_DATA}\" && cp \"${CLAUDE_PLUGIN_ROOT}/package.json\" . && npm install) || rm -f \"${CLAUDE_PLUGIN_DATA}/package.json\"", |
There was a problem hiding this comment.
P3: Document this new SessionStart package sync/install command in docs/plugin/HOOKS.md so the documented hook sequence matches hooks/hooks.json.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/hooks.json, line 9:
<comment>Document this new SessionStart package sync/install command in `docs/plugin/HOOKS.md` so the documented hook sequence matches `hooks/hooks.json`.</comment>
<file context>
@@ -6,12 +6,17 @@
{
"type": "command",
- "command": "${CLAUDE_PLUGIN_ROOT}/hooks/scripts/sync-env.sh",
+ "command": "diff -q \"${CLAUDE_PLUGIN_ROOT}/package.json\" \"${CLAUDE_PLUGIN_DATA}/package.json\" >/dev/null 2>&1 || (cd \"${CLAUDE_PLUGIN_DATA}\" && cp \"${CLAUDE_PLUGIN_ROOT}/package.json\" . && npm install) || rm -f \"${CLAUDE_PLUGIN_DATA}/package.json\"",
+ "timeout": 60
+ },
</file context>
| { | ||
| "type": "command", | ||
| "command": "${CLAUDE_PLUGIN_ROOT}/hooks/scripts/sync-env.sh", | ||
| "command": "${CLAUDE_PLUGIN_ROOT}/bin/sync-env.sh", |
There was a problem hiding this comment.
P3: Update the “File location” tree/description to reference bin/ so the doc matches the new command paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/plugin/HOOKS.md, line 27:
<comment>Update the “File location” tree/description to reference `bin/` so the doc matches the new command paths.</comment>
<file context>
@@ -24,12 +24,12 @@ hooks/
{
"type": "command",
- "command": "${CLAUDE_PLUGIN_ROOT}/hooks/scripts/sync-env.sh",
+ "command": "${CLAUDE_PLUGIN_ROOT}/bin/sync-env.sh",
"timeout": 10
},
</file context>
Blocks any *.env* file from being committed except .env.example. Prevents credential leaks like the .env.backup incidents.
- Add gitleaks/gitleaks-action@v2 job (full history scan, fetch-depth: 0) - Fix docker-security steps to reference bin/ (scripts/ dir removed)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
docs/mcp/MCPORTER.md (1)
5-21:⚠️ Potential issue | 🟠 MajorSmoke-test behavior text is stale after
arcane_helpremoval.The section still says the smoke test validates
arcane_help, but this PR removes that tool. Update this list to reflect routed help viaarcane(action=help/ domainsubaction=help).📝 Proposed doc update
-2. Lists available tools -- verifies `arcane` and `arcane_help` are registered -3. Calls `arcane_help` -- verifies it returns markdown text +2. Lists available tools -- verifies `arcane` is registered +3. Calls `arcane` help routes -- verifies markdown help output (`action=help` and domain-scoped `subaction=help`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/mcp/MCPORTER.md` around lines 5 - 21, The docs list for bin/smoke-test.ts is stale: it still claims the smoke test verifies the removed tool `arcane_help`; update the "What it tests" list to remove `arcane_help` and instead state that help is validated via `arcane` with help parameters (e.g., action=help and domain/subaction=help), and adjust step 3 to reflect calling `arcane` with help routing and verifying markdown text; ensure references to `arcane_help` are deleted and any mention of help testing points to `arcane` (bin/smoke-test.ts) with the proper action/subaction parameters.docs/INVENTORY.md (1)
86-86:⚠️ Potential issue | 🟡 MinorInventory still lists removed
arcane_helptool.Line 86 lists
arcane_helpas an available MCP tool, but this tool is being removed in this PR (replaced byaction=helpandsubaction=helprouting within thearcanetool). Remove this row to keep the inventory accurate.,
📝 Suggested fix
| `arcane` | `gitops` | `browse` | Browse GitOps repository | no | -| `arcane_help` | -- | -- | Return action reference as Markdown | no | +| `arcane` | `help` | -- | Return top-level action reference as Markdown | no |Alternatively, add rows for the new help routing patterns:
arcanewithaction=helpfor top-level help- Each domain with
subaction=helpfor domain-specific help🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/INVENTORY.md` at line 86, Remove the outdated `arcane_help` inventory entry: delete the row referencing `arcane_help` and, if desired, replace it with entries documenting the new help routing patterns by adding rows for the `arcane` tool with `action=help` (top-level help) and for domain-specific help using `arcane` with `subaction=help` for each domain; update any description text to reflect the new routing.hooks/hooks.json (2)
2-2:⚠️ Potential issue | 🟡 MinorDescription no longer matches actual hook behavior.
The description states "Sync userConfig credentials to .env, enforce 600 permissions, ensure gitignore" but the current configuration only performs package.json sync and npm install. Update the description to accurately reflect the new behavior.
📝 Suggested fix
{ - "description": "Sync userConfig credentials to .env, enforce 600 permissions, ensure gitignore", + "description": "Sync package.json and install dependencies on session start", "hooks": {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/hooks.json` at line 2, Update the top-level "description" value in hooks.json to reflect the current hook behavior (it no longer syncs userConfig credentials or manages .env permissions); replace the existing text ("Sync userConfig credentials to .env, enforce 600 permissions, ensure gitignore") with a concise description that states the hook now synchronizes package.json and runs npm install (e.g., "Sync package.json and run npm install").
4-14:⚠️ Potential issue | 🔴 CriticalCritical security hooks removed without replacement.
The previous hooks that performed essential security functions have been removed:
sync-env.sh(SessionStart) — Synced credentials from Claude Code userConfig to.env. Without this,ARCANE_API_KEY,ARCANE_API_URL, andARCANE_MCP_TOKENwon't be populated from userConfig, causing authentication failures.
fix-env-perms.sh(PostToolUse) — Enforcedchmod 600on.envafter any Write/Edit/Bash operations. Without this, sensitive credentials in.envmay have overly permissive file permissions.
ensure-ignore-files.sh(SessionStart) — Verified.gitignoreand.dockerignorecontain required patterns.The scripts still exist in
bin/but are no longer invoked. If these hooks were intentionally removed, this represents a security regression. If they should still run, restore them alongside the new npm install hook.🔧 Suggested fix to restore security hooks
{ - "description": "Sync userConfig credentials to .env, enforce 600 permissions, ensure gitignore", + "description": "Sync credentials, enforce permissions, ensure gitignore, sync dependencies", "hooks": { "SessionStart": [ { "hooks": [ + { + "type": "command", + "command": "${CLAUDE_PLUGIN_ROOT}/bin/sync-env.sh", + "timeout": 10 + }, + { + "type": "command", + "command": "${CLAUDE_PLUGIN_ROOT}/bin/ensure-ignore-files.sh", + "timeout": 10 + }, { "type": "command", "command": "diff -q \"${CLAUDE_PLUGIN_ROOT}/package.json\" \"${CLAUDE_PLUGIN_DATA}/package.json\" >/dev/null 2>&1 || (cd \"${CLAUDE_PLUGIN_DATA}\" && cp \"${CLAUDE_PLUGIN_ROOT}/package.json\" . && npm install) || rm -f \"${CLAUDE_PLUGIN_DATA}/package.json\"", "timeout": 60 } ] } - ] + ], + "PostToolUse": [ + { + "matcher": "Write|Edit|MultiEdit|Bash", + "hooks": [ + { + "type": "command", + "command": "${CLAUDE_PLUGIN_ROOT}/bin/fix-env-perms.sh", + "timeout": 5 + } + ] + } + ] } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/hooks.json` around lines 4 - 14, Restore the removed security hooks by adding back the SessionStart entries that invoke bin/sync-env.sh and bin/ensure-ignore-files.sh (so ARCANE_API_KEY/URL/MCP_TOKEN are populated and ignore files validated) and add a PostToolUse hook to run bin/fix-env-perms.sh (to enforce chmod 600 on .env) while keeping the new npm-install command present; ensure you reference the existing "SessionStart" and "PostToolUse" sections in hooks.json, preserve the current npm install command string ("diff -q \"${CLAUDE_PLUGIN_ROOT}/package.json\" ... npm install") and set reasonable timeouts for the restored hooks consistent with the other entries.docs/mcp/TESTS.md (1)
58-63:⚠️ Potential issue | 🟡 MinorDocumentation references removed
arcane_helptool.Line 61 states the smoke test verifies "Tool listing returns
arcaneandarcane_help", but according to the PR objectives, thearcane_helptool is being removed. This documentation is now stale and should be updated to reflect that only thearcanetool is returned (withaction=helpfor help functionality).,
📝 Suggested fix
TypeScript smoke test that connects to the MCP server and verifies: -- Tool listing returns `arcane` and `arcane_help` -- Help tool returns valid markdown +- Tool listing returns `arcane` +- Help action returns valid markdown - Basic tool calls succeed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/mcp/TESTS.md` around lines 58 - 63, Update the smoke test documentation to remove the obsolete reference to the `arcane_help` tool and state that the tool listing returns only `arcane`; explain that help functionality is accessed via `action=help` on the `arcane` tool (referencing the smoke test script `bin/smoke-test.ts` and the "Tool listing" bullet). Ensure the three verification points reflect: tool listing returns `arcane` only, help via `arcane` with `action=help` returns valid markdown, and basic tool calls succeed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.pre-commit-config.yaml:
- Around line 39-40: The current pre-commit hook pattern files: '\.env' is too
broad; update the files pattern (the entry labeled files: '\.env') to a
dotenv-style regex that only matches filenames like .env, .env.local,
.env.development, etc. (for example use a pattern such as '(^|/)\.env(\.|$)' or
'(^|/)\.env(\..+)?$') so hooks only run on true dotenv files, and leave the
existing exclude: '\.env\.example$' unchanged.
In `@bin/block-env-commits.sh`:
- Around line 4-6: The check currently inspects all staged paths (including
deletions) via the staged variable and then blocks any matching .env filenames;
update staged to exclude deletions so removing a tracked .env isn't blocked.
Replace the git command that populates staged to filter out deleted files (e.g.,
use git diff --cached --name-only --diff-filter=ACM or otherwise exclude the D
flag) so blocked only matches added/modified staged paths; keep the existing
blocked grep pattern unchanged.
---
Outside diff comments:
In `@docs/INVENTORY.md`:
- Line 86: Remove the outdated `arcane_help` inventory entry: delete the row
referencing `arcane_help` and, if desired, replace it with entries documenting
the new help routing patterns by adding rows for the `arcane` tool with
`action=help` (top-level help) and for domain-specific help using `arcane` with
`subaction=help` for each domain; update any description text to reflect the new
routing.
In `@docs/mcp/MCPORTER.md`:
- Around line 5-21: The docs list for bin/smoke-test.ts is stale: it still
claims the smoke test verifies the removed tool `arcane_help`; update the "What
it tests" list to remove `arcane_help` and instead state that help is validated
via `arcane` with help parameters (e.g., action=help and domain/subaction=help),
and adjust step 3 to reflect calling `arcane` with help routing and verifying
markdown text; ensure references to `arcane_help` are deleted and any mention of
help testing points to `arcane` (bin/smoke-test.ts) with the proper
action/subaction parameters.
In `@docs/mcp/TESTS.md`:
- Around line 58-63: Update the smoke test documentation to remove the obsolete
reference to the `arcane_help` tool and state that the tool listing returns only
`arcane`; explain that help functionality is accessed via `action=help` on the
`arcane` tool (referencing the smoke test script `bin/smoke-test.ts` and the
"Tool listing" bullet). Ensure the three verification points reflect: tool
listing returns `arcane` only, help via `arcane` with `action=help` returns
valid markdown, and basic tool calls succeed.
In `@hooks/hooks.json`:
- Line 2: Update the top-level "description" value in hooks.json to reflect the
current hook behavior (it no longer syncs userConfig credentials or manages .env
permissions); replace the existing text ("Sync userConfig credentials to .env,
enforce 600 permissions, ensure gitignore") with a concise description that
states the hook now synchronizes package.json and runs npm install (e.g., "Sync
package.json and run npm install").
- Around line 4-14: Restore the removed security hooks by adding back the
SessionStart entries that invoke bin/sync-env.sh and bin/ensure-ignore-files.sh
(so ARCANE_API_KEY/URL/MCP_TOKEN are populated and ignore files validated) and
add a PostToolUse hook to run bin/fix-env-perms.sh (to enforce chmod 600 on
.env) while keeping the new npm-install command present; ensure you reference
the existing "SessionStart" and "PostToolUse" sections in hooks.json, preserve
the current npm install command string ("diff -q
\"${CLAUDE_PLUGIN_ROOT}/package.json\" ... npm install") and set reasonable
timeouts for the restored hooks consistent with the other entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 88ed4025-d005-4343-ab11-3cdeb7f22f55
📒 Files selected for processing (17)
.pre-commit-config.yamlCLAUDE.mdJustfilebin/CLAUDE.mdbin/block-env-commits.shbin/check-docker-security.shbin/check-no-baked-env.shbin/check-outdated-deps.shbin/ensure-ignore-files.shbin/smoke-test.tsdocs/INVENTORY.mddocs/mcp/DEV.mddocs/mcp/MCPORTER.mddocs/mcp/PRE-COMMIT.mddocs/mcp/TESTS.mdhooks/CLAUDE.mdhooks/hooks.json
| files: '\.env' | ||
| exclude: '\.env\.example$' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Tighten hook file matcher to dotenv-style paths.
Current files: '\.env' is very broad and may trigger unnecessarily. Consider matching dotenv-like filenames explicitly to keep hook execution scoped.
🔧 Suggested regex refinement
- files: '\.env'
- exclude: '\.env\.example$'
+ files: '(^|/)[^/]*\.env[^/]*$'
+ exclude: '(^|/)[^/]*\.env\.example$'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| files: '\.env' | |
| exclude: '\.env\.example$' | |
| files: '(^|/)[^/]*\.env[^/]*$' | |
| exclude: '(^|/)[^/]*\.env\.example$' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.pre-commit-config.yaml around lines 39 - 40, The current pre-commit hook
pattern files: '\.env' is too broad; update the files pattern (the entry labeled
files: '\.env') to a dotenv-style regex that only matches filenames like .env,
.env.local, .env.development, etc. (for example use a pattern such as
'(^|/)\.env(\.|$)' or '(^|/)\.env(\..+)?$') so hooks only run on true dotenv
files, and leave the existing exclude: '\.env\.example$' unchanged.
| staged=$(git diff --cached --name-only) | ||
| blocked=$(printf '%s | ||
| ' "$staged" | grep -E '(^|/)[^/]*\.env[^/]*$' | grep -v '\.env\.example$' || true) |
There was a problem hiding this comment.
Do not block staged deletions of .env files.
This currently scans all staged paths, including deletes, which can prevent users from removing previously tracked secret files.
🔧 Proposed fix
-staged=$(git diff --cached --name-only)
+staged=$(git diff --cached --name-only --diff-filter=ACMR)
blocked=$(printf '%s
' "$staged" | grep -E '(^|/)[^/]*\.env[^/]*$' | grep -v '\.env\.example$' || true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/block-env-commits.sh` around lines 4 - 6, The check currently inspects
all staged paths (including deletions) via the staged variable and then blocks
any matching .env filenames; update staged to exclude deletions so removing a
tracked .env isn't blocked. Replace the git command that populates staged to
filter out deleted files (e.g., use git diff --cached --name-only
--diff-filter=ACM or otherwise exclude the D flag) so blocked only matches
added/modified staged paths; keep the existing blocked grep pattern unchanged.
There was a problem hiding this comment.
5 issues found across 17 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/mcp/TESTS.md">
<violation number="1" location="docs/mcp/TESTS.md:58">
P3: Smoke-test documentation still references the removed `arcane_help` tool, so the expected results are outdated after this breaking change.</violation>
</file>
<file name="docs/mcp/PRE-COMMIT.md">
<violation number="1" location="docs/mcp/PRE-COMMIT.md:36">
P3: The documented `docker-security` hook command does not match the actual pre-commit entry path.</violation>
<violation number="2" location="docs/mcp/PRE-COMMIT.md:44">
P3: The documented `no-baked-env` hook command uses `bin/` but pre-commit is still configured to run `scripts/`.</violation>
<violation number="3" location="docs/mcp/PRE-COMMIT.md:48">
P3: The `ensure-ignore-files` command in docs is out of sync with the configured pre-commit hook path.</violation>
</file>
<file name="bin/block-env-commits.sh">
<violation number="1" location="bin/block-env-commits.sh:4">
P2: This includes staged deletions in the check. A user trying to `git rm` a previously tracked `.env` file will be blocked. Add `--diff-filter=ACMR` to exclude deletions.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| staged=$(git diff --cached --name-only) |
There was a problem hiding this comment.
P2: This includes staged deletions in the check. A user trying to git rm a previously tracked .env file will be blocked. Add --diff-filter=ACMR to exclude deletions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bin/block-env-commits.sh, line 4:
<comment>This includes staged deletions in the check. A user trying to `git rm` a previously tracked `.env` file will be blocked. Add `--diff-filter=ACMR` to exclude deletions.</comment>
<file context>
@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+staged=$(git diff --cached --name-only)
+blocked=$(printf '%s
+' "$staged" | grep -E '(^|/)[^/]*\.env[^/]*$' | grep -v '\.env\.example$' || true)
</file context>
| staged=$(git diff --cached --name-only) | |
| staged=$(git diff --cached --name-only --diff-filter=ACMR) |
| ### Smoke test (`bin/smoke-test.ts`) | ||
|
|
||
| TypeScript smoke test that connects to the MCP server and verifies: | ||
| - Tool listing returns `arcane` and `arcane_help` |
There was a problem hiding this comment.
P3: Smoke-test documentation still references the removed arcane_help tool, so the expected results are outdated after this breaking change.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/mcp/TESTS.md, line 58:
<comment>Smoke-test documentation still references the removed `arcane_help` tool, so the expected results are outdated after this breaking change.</comment>
<file context>
@@ -55,7 +55,7 @@ bash tests/test_live.sh --mode stdio
The script exercises all action domains against the real Arcane API.
-### Smoke test (`scripts/smoke-test.ts`)
+### Smoke test (`bin/smoke-test.ts`)
TypeScript smoke test that connects to the MCP server and verifies:
</file context>
| ### ensure-ignore-files | ||
|
|
||
| Runs `bash scripts/ensure-ignore-files.sh --check .`. Verifies `.gitignore` and `.dockerignore` contain required patterns (`.env`, `*.secret`, etc.). | ||
| Runs `bash bin/ensure-ignore-files.sh --check .`. Verifies `.gitignore` and `.dockerignore` contain required patterns (`.env`, `*.secret`, etc.). |
There was a problem hiding this comment.
P3: The ensure-ignore-files command in docs is out of sync with the configured pre-commit hook path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/mcp/PRE-COMMIT.md, line 48:
<comment>The `ensure-ignore-files` command in docs is out of sync with the configured pre-commit hook path.</comment>
<file context>
@@ -33,19 +33,19 @@ Runs `npx skills-ref validate skills/` when any Markdown file under `skills/` ch
### ensure-ignore-files
-Runs `bash scripts/ensure-ignore-files.sh --check .`. Verifies `.gitignore` and `.dockerignore` contain required patterns (`.env`, `*.secret`, etc.).
+Runs `bash bin/ensure-ignore-files.sh --check .`. Verifies `.gitignore` and `.dockerignore` contain required patterns (`.env`, `*.secret`, etc.).
## Claude Code session hooks
</file context>
| Runs `bash bin/ensure-ignore-files.sh --check .`. Verifies `.gitignore` and `.dockerignore` contain required patterns (`.env`, `*.secret`, etc.). | |
| Runs `bash scripts/ensure-ignore-files.sh --check .`. Verifies `.gitignore` and `.dockerignore` contain required patterns (`.env`, `*.secret`, etc.). |
| ### no-baked-env | ||
|
|
||
| Runs `bash scripts/check-no-baked-env.sh .`. Scans Docker-related files for baked credentials. Triggers on changes to Dockerfile, compose, dockerignore, or entrypoint. | ||
| Runs `bash bin/check-no-baked-env.sh .`. Scans Docker-related files for baked credentials. Triggers on changes to Dockerfile, compose, dockerignore, or entrypoint. |
There was a problem hiding this comment.
P3: The documented no-baked-env hook command uses bin/ but pre-commit is still configured to run scripts/.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/mcp/PRE-COMMIT.md, line 44:
<comment>The documented `no-baked-env` hook command uses `bin/` but pre-commit is still configured to run `scripts/`.</comment>
<file context>
@@ -33,19 +33,19 @@ Runs `npx skills-ref validate skills/` when any Markdown file under `skills/` ch
### no-baked-env
-Runs `bash scripts/check-no-baked-env.sh .`. Scans Docker-related files for baked credentials. Triggers on changes to Dockerfile, compose, dockerignore, or entrypoint.
+Runs `bash bin/check-no-baked-env.sh .`. Scans Docker-related files for baked credentials. Triggers on changes to Dockerfile, compose, dockerignore, or entrypoint.
### ensure-ignore-files
</file context>
| Runs `bash bin/check-no-baked-env.sh .`. Scans Docker-related files for baked credentials. Triggers on changes to Dockerfile, compose, dockerignore, or entrypoint. | |
| Runs `bash scripts/check-no-baked-env.sh .`. Scans Docker-related files for baked credentials. Triggers on changes to Dockerfile, compose, dockerignore, or entrypoint. |
| ### docker-security | ||
|
|
||
| Runs `bash scripts/check-docker-security.sh Dockerfile`. Checks for: | ||
| Runs `bash bin/check-docker-security.sh Dockerfile`. Checks for: |
There was a problem hiding this comment.
P3: The documented docker-security hook command does not match the actual pre-commit entry path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/mcp/PRE-COMMIT.md, line 36:
<comment>The documented `docker-security` hook command does not match the actual pre-commit entry path.</comment>
<file context>
@@ -33,19 +33,19 @@ Runs `npx skills-ref validate skills/` when any Markdown file under `skills/` ch
### docker-security
-Runs `bash scripts/check-docker-security.sh Dockerfile`. Checks for:
+Runs `bash bin/check-docker-security.sh Dockerfile`. Checks for:
- Non-root USER directive
- No COPY of `.env` files
</file context>
| Runs `bash bin/check-docker-security.sh Dockerfile`. Checks for: | |
| Runs `bash scripts/check-docker-security.sh Dockerfile`. Checks for: |
- Fix action=help schema: make subaction optional with z.union so domain names (e.g. "container") are accepted without being in the enum, and action=help calls without subaction are no longer rejected (P1) - Sync .claude-plugin/plugin.json version to 1.2.0 to match package.json (P2) - Fix CHANGELOG.md MD022: add blank lines after every ## and ### heading (P2) - VALID_DOMAINS deduplication and dynamic error message already done in commit 886941b (P3 — no additional changes needed)
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".claude-plugin/plugin.json">
<violation number="1" location=".claude-plugin/plugin.json:3">
P2: Version bump is inconsistent across plugin manifests; `.codex-plugin/plugin.json` and `gemini-extension.json` remain at 1.1.5 while this file is 1.2.0.</violation>
</file>
<file name="src/mcp/tools/arcane.ts">
<violation number="1" location="src/mcp/tools/arcane.ts:136">
P2: `z.string()` in the `subaction` union makes `subaction` effectively unconstrained, so invalid operations are accepted by schema validation and fail only at runtime.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| { | ||
| "name": "arcane-mcp", | ||
| "version": "1.1.5", | ||
| "version": "1.2.0", |
There was a problem hiding this comment.
P2: Version bump is inconsistent across plugin manifests; .codex-plugin/plugin.json and gemini-extension.json remain at 1.1.5 while this file is 1.2.0.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude-plugin/plugin.json, line 3:
<comment>Version bump is inconsistent across plugin manifests; `.codex-plugin/plugin.json` and `gemini-extension.json` remain at 1.1.5 while this file is 1.2.0.</comment>
<file context>
@@ -1,6 +1,6 @@
{
"name": "arcane-mcp",
- "version": "1.1.5",
+ "version": "1.2.0",
"description": "Manage Docker containers, images, volumes, networks, and stacks via the Arcane API.",
"author": {
</file context>
| ]), | ||
| // Accept domain names as subaction when action=help | ||
| // e.g. { action: "help", subaction: "container" } for per-domain docs | ||
| z.string(), |
There was a problem hiding this comment.
P2: z.string() in the subaction union makes subaction effectively unconstrained, so invalid operations are accepted by schema validation and fail only at runtime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/tools/arcane.ts, line 136:
<comment>`z.string()` in the `subaction` union makes `subaction` effectively unconstrained, so invalid operations are accepted by schema validation and fail only at runtime.</comment>
<file context>
@@ -85,52 +85,60 @@ export function registerArcaneTool(
+ ]),
+ // Accept domain names as subaction when action=help
+ // e.g. { action: "help", subaction: "container" } for per-domain docs
+ z.string(),
])
- .describe("Operation to perform"),
</file context>
| z.string(), | |
| z.enum([ | |
| "environment", | |
| "project", | |
| "container", | |
| "image", | |
| "network", | |
| "volume", | |
| "system", | |
| "image-update", | |
| "vulnerability", | |
| "registry", | |
| "gitops", | |
| ]), |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 106-115: In the gitleaks job replace mutable refs with immutable
40-char commit SHAs: change the uses entries "actions/checkout@v4" and
"gitleaks/gitleaks-action@v2" to their corresponding full commit SHAs (e.g.,
actions/checkout@<40-char-sha> and gitleaks/gitleaks-action@<40-char-sha>),
keeping the surrounding keys (runs-on, steps, env: GITHUB_TOKEN, and
fetch-depth) intact so the job behavior doesn't change; after updating, verify
no remaining unpinned actions by running the provided verification script.
In `@src/mcp/tools/arcane.ts`:
- Around line 157-160: The code currently silences invalid help domain hints by
mapping unknown subaction values to undefined; change the logic in the help
branch where domainHint is computed (the block that references HELP_DOMAINS,
action === "help", and subaction) to validate subaction explicitly: if subaction
is present and not contained in HELP_DOMAINS, surface an error (throw or return
a structured error response) instead of downgrading to undefined, otherwise set
domainHint to the valid subaction; ensure any callers of this branch handle the
new error shape.
- Around line 88-138: The current subaction schema uses z.union([z.enum(...),
z.string()]) which makes the enum meaningless because z.string() will always
accept any string; change this to conditional schemas keyed off the action field
(e.g., replace the loose z.union with a z.discriminatedUnion('action', [...]) or
separate object schemas) so that for action === "help" subaction may be
z.string(), but for all other actions subaction is strictly z.enum([...]) or
omitted; update the schema where subaction and action are defined to use this
discriminated/conditional approach instead of the global z.string() branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ef052b90-66e2-45a9-83b7-4ebcaf5233a5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.jsonand included by**/*
📒 Files selected for processing (4)
.claude-plugin/plugin.json.github/workflows/ci.ymlCHANGELOG.mdsrc/mcp/tools/arcane.ts
| gitleaks: | ||
| name: Secret Scan | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| - uses: gitleaks/gitleaks-action@v2 | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
Pin action refs to immutable SHAs in the new secret-scan job.
Using mutable tags (@v4, @v2) allows upstream tag retargeting and weakens CI supply-chain integrity. Please pin to full commit SHAs.
Suggested change
gitleaks:
name: Secret Scan
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v4
+ - uses: actions/checkout@<full-40-char-commit-sha>
with:
fetch-depth: 0
- - uses: gitleaks/gitleaks-action@v2
+ - uses: gitleaks/gitleaks-action@<full-40-char-commit-sha>
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}Run this to verify unpinned actions across workflows:
#!/bin/bash
set -euo pipefail
# Lists `uses:` entries that are NOT pinned to a 40-char SHA.
rg -n '^\s*-\s*uses:\s*[^@]+@([^\s]+)' .github/workflows \
| awk '
{
line=$0
ref=$0
sub(/^.*@/, "", ref)
if (ref !~ /^[0-9a-fA-F]{40}$/) {
print line
}
}'Expected result after fix: no output.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 106 - 115, In the gitleaks job replace
mutable refs with immutable 40-char commit SHAs: change the uses entries
"actions/checkout@v4" and "gitleaks/gitleaks-action@v2" to their corresponding
full commit SHAs (e.g., actions/checkout@<40-char-sha> and
gitleaks/gitleaks-action@<40-char-sha>), keeping the surrounding keys (runs-on,
steps, env: GITHUB_TOKEN, and fetch-depth) intact so the job behavior doesn't
change; after updating, verify no remaining unpinned actions by running the
provided verification script.
| .union([ | ||
| z.enum([ | ||
| "list", | ||
| "get", | ||
| "create", | ||
| "update", | ||
| "delete", | ||
| "start", | ||
| "stop", | ||
| "restart", | ||
| "pull", | ||
| "up", | ||
| "down", | ||
| "prune", | ||
| "scan", | ||
| "stats", | ||
| // NOTE: "logs" intentionally omitted — the Arcane REST API has no | ||
| // logs endpoint; dispatchContainer retains a "logs" case as a safety | ||
| // net against future schema drift, but exposing it in the enum would | ||
| // be a lying contract that always errors. | ||
| "destroy", | ||
| "redeploy", | ||
| "test", | ||
| "build", | ||
| "stop-all", | ||
| "start-all", | ||
| "docker-info", | ||
| "convert", | ||
| "check-all", | ||
| "check", | ||
| "check-batch", | ||
| "summary", | ||
| "ignore", | ||
| "unignore", | ||
| "scanner-status", | ||
| "list-ignored", | ||
| "browse", | ||
| "list-backups", | ||
| "create-backup", | ||
| "restore", | ||
| "restore-files", | ||
| "delete-backup", | ||
| "status", | ||
| "sync", | ||
| "help", | ||
| ]), | ||
| // Accept domain names as subaction when action=help | ||
| // e.g. { action: "help", subaction: "container" } for per-domain docs | ||
| z.string(), | ||
| ]) | ||
| .describe("Operation to perform"), | ||
| .optional() |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Zod, does z.union([z.enum(["list","get"]), z.string()]) accept arbitrary strings like "foobar" outside the enum?
💡 Result:
Yes. Because z.union([...]) “tests the input against each of the options in order” and succeeds if any option validates; since one option is z.string(), any string (including "foobar") will be accepted. [1]
Also, the inferred TypeScript type for z.union([z.enum(["list","get"]), z.string()]) collapses to just string (the enum doesn’t narrow it), because string already includes those literals. [1]
Source(s): [1]
🏁 Script executed:
# Check the file structure and line counts
wc -l src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 88
🏁 Script executed:
# Examine lines 88-138 in context
sed -n '85,140p' src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 1886
🏁 Script executed:
# Examine lines 171-176 in context
sed -n '168,180p' src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 495
🏁 Script executed:
# Look for where this schema is used/validated downstream
rg -A 5 -B 5 "subaction" src/mcp/tools/arcane.tsRepository: jmagar/arcane-mcp
Length of output: 9646
🏁 Script executed:
# Get exact context around lines 171-176 with line numbers
sed -n '171,176p' src/mcp/tools/arcane.ts | cat -nRepository: jmagar/arcane-mcp
Length of output: 332
🏁 Script executed:
# Search for other union definitions with enum + z.string()
rg -n "z\.union" src/mcp/tools/arcane.ts -A 2 -B 2Repository: jmagar/arcane-mcp
Length of output: 43
🏁 Script executed:
# Search for all z.union patterns (case-insensitive)
rg -i "union" src/mcp/tools/arcane.ts -nRepository: jmagar/arcane-mcp
Length of output: 80
🏁 Script executed:
# Look at the full schema definition around lines 88-176
sed -n '70,180p' src/mcp/tools/arcane.ts | cat -nRepository: jmagar/arcane-mcp
Length of output: 4440
🏁 Script executed:
# Look at the exact lines 171-176 in the file (not relative to sed output)
sed -n '165,180p' src/mcp/tools/arcane.ts | cat -nRepository: jmagar/arcane-mcp
Length of output: 672
🏁 Script executed:
# Search for any other schema definitions or patterns that might use z.union
rg -n "subaction\|z\.union\|z\.enum" src/mcp/tools/arcane.ts -B 1 -A 1Repository: jmagar/arcane-mcp
Length of output: 43
🏁 Script executed:
# Look at the full file to understand structure better
head -n 200 src/mcp/tools/arcane.ts | tail -n 50 | cat -nRepository: jmagar/arcane-mcp
Length of output: 2416
Remove z.string() from the subaction union or condition validation by action.
The union at lines 18–68 accepts any string because z.union([z.enum([...]), z.string()]) succeeds if any branch validates; z.string() makes the enum validation ineffective. While the inline comment indicates this is intentional for the help action (to accept domain names), the schema applies globally to all actions, allowing unsupported subactions and pushing validation errors downstream. Either use conditional validation based on the action value, or make the enum constraint strict and handle domain-name flexibility separately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/mcp/tools/arcane.ts` around lines 88 - 138, The current subaction schema
uses z.union([z.enum(...), z.string()]) which makes the enum meaningless because
z.string() will always accept any string; change this to conditional schemas
keyed off the action field (e.g., replace the loose z.union with a
z.discriminatedUnion('action', [...]) or separate object schemas) so that for
action === "help" subaction may be z.string(), but for all other actions
subaction is strictly z.enum([...]) or omitted; update the schema where
subaction and action are defined to use this discriminated/conditional approach
instead of the global z.string() branch.
| if (action === "help") { | ||
| const domainHint = | ||
| subaction && HELP_DOMAINS.has(subaction) ? subaction : undefined; | ||
| return { |
There was a problem hiding this comment.
Do not silently ignore invalid domain hints in action=help.
Line 159 currently downgrades unknown subaction values to undefined, so typos (e.g., contianer) return top-level help instead of surfacing an invalid domain error.
🔧 Proposed fix
if (action === "help") {
- const domainHint =
- subaction && HELP_DOMAINS.has(subaction) ? subaction : undefined;
+ const domainHint =
+ subaction && subaction !== "help" ? subaction : undefined;
return {
content: [
{ type: "text" as const, text: dispatchHelp(domainHint) },
],
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (action === "help") { | |
| const domainHint = | |
| subaction && HELP_DOMAINS.has(subaction) ? subaction : undefined; | |
| return { | |
| if (action === "help") { | |
| const domainHint = | |
| subaction && subaction !== "help" ? subaction : undefined; | |
| return { | |
| content: [ | |
| { type: "text" as const, text: dispatchHelp(domainHint) }, | |
| ], | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/mcp/tools/arcane.ts` around lines 157 - 160, The code currently silences
invalid help domain hints by mapping unknown subaction values to undefined;
change the logic in the help branch where domainHint is computed (the block that
references HELP_DOMAINS, action === "help", and subaction) to validate subaction
explicitly: if subaction is present and not contained in HELP_DOMAINS, surface
an error (throw or return a structured error response) instead of downgrading to
undefined, otherwise set domainHint to the valid subaction; ensure any callers
of this branch handle the new error shape.
Summary
arcane_helpMCP toolaction=helpto thearcanetool for top-level helpsubaction=helpto all 11 domains for per-domain docs with full params/examplesBeads
Breaking Change
arcane_helptool is removed. Usearcane action=helpinstead.Testing
npm run typecheckcleanUsage after this change
arcane action=helparcane action=container subaction=helparcane action=help subaction=containerSummary by cubic
Replaced the
arcane_helptool with built‑in help routing onarcane, added CI hardening, and patched high‑severityviteCVEs. Breaking change: usearcane action=helporaction=<domain> subaction=help. Satisfies J19.New Features
action=helpandsubaction=helpacross all 11 domains with full params/examples; introduceddispatchHelpplus drift and routing tests.gitleaks/gitleaks-action@v2secret scan and ablock-env-commitspre-commit hook to block.env*files (except.env.example).Refactors
bin/, updated docs and CI/Justfile references; SessionStart hook now installs deps inCLAUDE_PLUGIN_DATAwhenpackage.jsonchanges.subactionis optional foraction=helpand accepts domain names; exportedHELP_DOMAINSto dedupe domain lists; moved smoke test tobin/smoke-test.ts.viteto8.0.5to resolve CVEs and fixed the lockfile to preserve@emnapi/coreand@emnapi/runtime; synced versions to1.2.0inpackage.jsonand.claude-plugin/plugin.json.Written for commit 0ed553e. Summary will update on new commits.
Summary by CodeRabbit
New Features
Breaking Changes
Tests
Documentation
Chores