Skip to content

BUGBOT.md

Latest commit

 

History

History
84 lines (58 loc) · 3.42 KB

BUGBOT.md

File metadata and controls

84 lines (58 loc) · 3.42 KB

Bugbot Review Guide for XcodeBuildMCP

Project Snapshot

XcodeBuildMCP is an MCP server exposing Xcode / Swift workflows as tools and resources. Stack: TypeScript · Node.js · plugin-based auto-discovery (src/mcp/tools, src/mcp/resources).

For full details see README.md and docs/ARCHITECTURE.md.

1. Security Checklist — Critical

  • No hard-coded secrets, tokens or DSNs.
  • MCP tool logic functions that orchestrate long-running processes with sub-processes (e.g., xcodebuild) must flow through CommandExecutor with validated arguments. Standalone utility modules that invoke simple, short-lived commands may use direct child_process/fs imports and standard vitest mocking.
  • Paths must be sanitised via helpers in src/utils/validation.ts.
  • Sentry breadcrumbs / logs must NOT include user PII.

2. Architecture Checklist — Critical

Rule Quick diff heuristic
Dependency injection for tool logic New child_process | fs import in MCP tool logic ⇒ warning (check if process is complex/long-running)
Handler / Logic split handler > 20 LOC or contains branching ⇒ critical
Plugin auto-registration Manual registerTool(...) / registerResource(...) ⇒ critical

Positive pattern skeleton:

// src/mcp/tools/foo-bar.ts
export async function fooBarLogic(
  params: FooBarParams,
  exec: CommandExecutor = getDefaultCommandExecutor(),
  fs: FileSystemExecutor = getDefaultFileSystemExecutor(),
) {
  // ...
}

export const handler = (p: FooBarParams) => fooBarLogic(p);

3. Testing Checklist

  • External-boundary rule for tool logic: Use createMockExecutor / createMockFileSystemExecutor for complex process orchestration (xcodebuild, multi-step pipelines) in MCP tool logic functions.
  • Simple utilities: Standalone utility modules with simple command calls can use direct imports and standard vitest mocking.
  • Internal mocking is allowed: vi.mock, vi.fn, vi.spyOn, and .mock* are acceptable for internal modules/collaborators.
  • Each tool must have tests covering happy-path and at least one failure path.
  • Avoid the any type unless justified with an inline comment.

4. Documentation Checklist

  • docs/TOOLS.md must exactly mirror the structure of src/mcp/tools/** (exclude __tests__ and *-shared). Diff heuristic: if a PR adds/removes a tool but does not change docs/TOOLS.md ⇒ warning.
  • Update public docs when CLI parameters or tool names change.

5. Common Anti-Patterns (and fixes)

Anti-pattern Preferred approach
Complex logic in handler Move to *Logic function
Re-implementing logging Use src/utils/logger.ts
Direct fs / child_process in tool logic orchestrating complex processes Inject FileSystemExecutor / CommandExecutor
Chained re-exports Export directly from source

How Bugbot Can Verify Rules

  1. External-boundary violations: confirm tests use injected executors/filesystem for external side effects.
  2. DI compliance: check direct child_process / fs imports in MCP tool logic; standalone utilities with simple commands are acceptable.
  3. Docs accuracy: compare docs/TOOLS.md against src/mcp/tools/**.
  4. Style: ensure ESLint and Prettier pass (npm run lint, npm run format:check).

Happy reviewing 🚀