BUGBOT.md•2.79 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](README.md) and [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md).
---
## 1. Security Checklist — Critical
* No hard-coded secrets, tokens or DSNs.
* All shell commands must flow through `CommandExecutor` with validated arguments (no direct `child_process` calls).
* 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 only | New `child_process` \| `fs` import ⇒ **critical** |
| Handler / Logic split | `handler` > 20 LOC or contains branching ⇒ **critical** |
| Plugin auto-registration | Manual `registerTool(...)` / `registerResource(...)` ⇒ **critical** |
Positive pattern skeleton:
```ts
// 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
* **Ban on Vitest mocking** (`vi.mock`, `vi.fn`, `vi.spyOn`, `.mock*`) ⇒ critical. Use `createMockExecutor` / `createMockFileSystemExecutor`.
* 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` usage | Inject `FileSystemExecutor` / `CommandExecutor` |
| Chained re-exports | Export directly from source |
---
### How Bugbot Can Verify Rules
1. **Mocking violations**: search `*.test.ts` for `vi.` → critical.
2. **DI compliance**: search for direct `child_process` / `fs` imports outside executors.
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 🚀