-
-
Notifications
You must be signed in to change notification settings - Fork 193
ref(tools): Generate manifest-driven CLI docs #198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
commit: |
WalkthroughThis pull request transitions the xcodebuildmcp package from version 🚥 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. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
151-151:⚠️ Potential issue | 🟡 MinorInconsistent version reference remains.
Line 151 still references
xcodebuildmcp@latestwhile all other installation examples have been updated toxcodebuildmcp@beta. This appears to be an oversight.Proposed fix
-When configuring a client manually, ensure the command includes the `mcp` subcommand (for example, `npx -y xcodebuildmcp@latest mcp`). +When configuring a client manually, ensure the command includes the `mcp` subcommand (for example, `npx -y xcodebuildmcp@beta mcp`).
🤖 Fix all issues with AI agents
In `@docs/CLI.md`:
- Around line 3-5: Update the documentation text for the xcodebuildmcp
description: change the redundant phrase "CLI interface" to just "CLI" and
insert the missing article "a" before "first-class" so the sentence reads that
xcodebuildmcp "provides both an MCP server and direct tool access via a
first-class CLI" and later "Use xcodebuildmcp CLI to invoke tools..." (refer to
the string containing `xcodebuildmcp` and the phrases "CLI interface" and
"first-class" to locate the lines to edit).
In `@skills/xcodebuilmcp-cli/SKILL.md`:
- Around line 1-4: The directory name contains a typo: rename the folder from
"xcodebuilmcp-cli" to "xcodebuildmcp-cli" (add the missing 'd') and update any
references to the old directory name (CI configs, docs, import paths, package
manifests, or tooling scripts) to use "xcodebuildmcp-cli"; verify the SKILL.md
frontmatter "name" still matches after rename and run any discovery/tests that
rely on the directory name to ensure tooling picks up the corrected name.
- Around line 101-107: Update the swift-package tool list to use the real CLI
command names instead of the short names: replace `build`, `clean`, `list`,
`run`, `stop`, `test` with `swift-package-build`, `swift-package-clean`,
`swift-package-list`, `swift-package-run`, `swift-package-stop`,
`swift-package-test` in the `swift-package` section of SKILL.md so documentation
matches docs/TOOLS-CLI.md and the actual CLI.
In `@src/cli/commands/tools.ts`:
- Around line 25-37: The manifestPath is being resolved relative to __dirname
which at runtime is build/cli/commands and thus points to
build/cli/commands/tools-manifest.json (missing); update how manifestPath is
computed so it targets the build root (where build/tools-manifest.json is
emitted) before calling loadManifest(): change the resolution used by
manifestPath to go up from __dirname to the build root (e.g., navigate up the
directory tree by two levels) so loadManifest() reads build/tools-manifest.json,
and keep the existing fs.existsSync/fs.readFileSync logic intact.
🧹 Nitpick comments (3)
scripts/generate-tools-manifest.ts (1)
51-60: Remove duplicatetoKebabCasefunction and import from existing module.This function duplicates the implementation in
src/runtime/naming.ts. Import it instead to maintain a single source of truth.Proposed fix
import * as fs from 'fs'; import * as path from 'path'; import { fileURLToPath } from 'url'; import { getStaticToolAnalysis } from './analysis/tools-analysis.js'; +import { toKebabCase } from '../src/runtime/naming.js'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); const projectRoot = path.resolve(__dirname, '..'); type ToolManifestEntry = { name: string; mcpName: string; cliName: string; workflow: string; description: string; originWorkflow?: string; isCanonical: boolean; stateful: boolean; }; type WorkflowManifestEntry = { name: string; displayName: string; description: string; toolCount: number; canonicalCount: number; reExportCount: number; }; type ToolsManifest = { generatedAt: string; stats: { totalTools: number; canonicalTools: number; reExportTools: number; workflowCount: number; }; workflows: WorkflowManifestEntry[]; tools: ToolManifestEntry[]; }; -function toKebabCase(name: string): string { - return name - .trim() - .replace(/_/g, '-') - .replace(/([a-z])([A-Z])/g, '$1-$2') - .replace(/\s+/g, '-') - .toLowerCase() - .replace(/-+/g, '-') - .replace(/^-|-$/g, ''); -} - async function main(): Promise<void> {scripts/update-tools-docs.ts (2)
144-153: Consider adding runtime validation for the manifest structure.The
as ToolsManifesttype assertion provides no runtime guarantees. If the manifest file is malformed or has an unexpected structure (e.g., after a build tool version mismatch), this will silently produce incorrect data rather than failing fast.Proposed validation approach
function loadManifest(): ToolsManifest { if (!fs.existsSync(manifestPath)) { throw new Error( `Missing tools manifest at ${path.relative(projectRoot, manifestPath)}. Run \"npm run build\" first.`, ); } const raw = fs.readFileSync(manifestPath, 'utf-8'); - return JSON.parse(raw) as ToolsManifest; + const manifest = JSON.parse(raw); + if (!manifest.generatedAt || !manifest.stats || !Array.isArray(manifest.workflows) || !Array.isArray(manifest.tools)) { + throw new Error('Invalid tools manifest structure. Regenerate with "npm run build".'); + } + return manifest as ToolsManifest; }
239-260: Potential type narrowing issue with optionalworkflowfield.
tool.workflowis typed as optional (string | undefined) viaDocumentationTool, but it's passed directly tocliExcludedWorkflows.has()and used as a Map key without explicit handling. Whilst JavaScript'sSet.has(undefined)returnsfalseandMap.get(undefined)returnsundefined, relying on this implicit behaviour reduces code clarity.Explicit handling for clarity
for (const tool of manifest.tools) { - if (cliExcludedWorkflows.has(tool.workflow)) { + const workflow = tool.workflow ?? ''; + if (cliExcludedWorkflows.has(workflow)) { continue; } if (tool.isCanonical) { canonicalToolCount++; } - const tools = toolsByWorkflow.get(tool.workflow) ?? []; + const tools = toolsByWorkflow.get(workflow) ?? []; const originWorkflow = tool.originWorkflow ? workflowMeta.get(tool.originWorkflow)?.displayName ?? tool.originWorkflow : undefined; // ... - toolsByWorkflow.set(tool.workflow, tools); + toolsByWorkflow.set(workflow, tools); }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
…docs Co-authored-by: web <web@cameroncooke.com>
…r script Co-authored-by: web <web@cameroncooke.com>
…tions Co-authored-by: web <web@cameroncooke.com>
Switch CLI tools listing and docs generation to the tools manifest, keeping CLI names and re-export metadata consistent with the build output. This also adds a CLI tools reference doc and wires simulator workflow to expose stop-sim-log-cap via re-export, plus updates install examples to the beta package.
The manifest already exists as the single source of truth for tool metadata, so using it avoids drift between runtime, docs, and CLI output.
Alternative considered: continue per-script AST analysis for docs and CLI list. Rejected to avoid duplicate parsing logic and inconsistent handling of re-exports/CLI names.
Notes: the CLI tools list now requires build/tools-manifest.json to exist (generated during build).
Note
Medium Risk
Moderate risk because the
xcodebuildmcp toolsCLI and docs generation now depend on a generatedbuild/tools-manifest.json, so packaging/build regressions or missing manifest files could break tool discovery/output.Overview
Moves tool metadata to a single manifest source of truth. The build now generates
build/tools-manifest.jsonfrom static tool analysis, and both CLI output and docs are updated to consume this manifest (including re-export origin workflow and per-toolstateful/CLI name metadata).Improves CLI and docs consistency. The
toolscommand no longer relies on the runtime catalog and instead formats grouped/flat JSON with counts andcanonicalWorkflowfor re-exports; docs generation now outputs both MCP (docs/TOOLS.md) and a new CLI tool reference (docs/TOOLS-CLI.md).Updates distribution/installation guidance. Readme/getting-started examples are switched to
@beta, the skill installer is made interactive (client + MCP vs CLI skill) with optional--refand conflict handling, and the simulator workflow re-exportsstop-sim-log-capso it appears under simulator tools.Written by Cursor Bugbot for commit c1836d1. This will update automatically on new commits. Configure here.