From 5a2f572a455b571ba2c006587cf99aa7776b487d Mon Sep 17 00:00:00 2001 From: Cameron Cooke Date: Fri, 13 Feb 2026 11:03:46 +0000 Subject: [PATCH 1/2] fix(sentry): Harden telemetry and keep internal observability Limit Sentry capture to internal infrastructure signals while keeping\nuseful operational visibility for CLI and MCP runtimes.\n\nAdd runtime and dependency context tags, add daemon/bootstrap/tool metrics,\nand tighten redaction and logging behavior to avoid user-domain payloads. Fixes #204 Co-Authored-By: Claude --- CHANGELOG.md | 7 +- README.md | 2 +- docs/CONFIGURATION.md | 7 +- docs/PRIVACY.md | 7 +- docs/SENTRY_AUDIT_REPORT_2026-02-11.md | 184 +++++++ docs/dev/ARCHITECTURE.md | 4 +- package-lock.json | 42 +- package.json | 2 +- src/cli.ts | 17 +- src/core/resources.ts | 3 + src/daemon.ts | 100 +++- src/daemon/daemon-server.ts | 11 +- src/runtime/tool-invoker.ts | 89 ++++ src/server/bootstrap.ts | 57 ++ src/server/server.ts | 40 +- src/server/start-mcp-server.ts | 13 +- src/utils/__tests__/logger.test.ts | 23 + src/utils/__tests__/sentry-redaction.test.ts | 81 +++ src/utils/config-store.ts | 11 + src/utils/logger.ts | 44 +- src/utils/sentry.ts | 533 +++++++++++++++---- src/utils/tool-registry.ts | 29 +- src/utils/xcodemake.ts | 32 +- src/utils/xcodemake/index.ts | 7 +- 24 files changed, 1181 insertions(+), 164 deletions(-) create mode 100644 docs/SENTRY_AUDIT_REPORT_2026-02-11.md create mode 100644 src/utils/__tests__/logger.test.ts create mode 100644 src/utils/__tests__/sentry-redaction.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d3cb1aa..29506975 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased] + +### Fixed + +- Fixed Sentry telemetry scope to capture only internal XcodeBuildMCP runtime failures, removing broad MCP wrapping, PII-heavy tags, and default per-error log capture ([#204](https://github.com/getsentry/XcodeBuildMCP/issues/204)) + ## [2.0.7] ### Changed @@ -273,4 +279,3 @@ Please note that the UI automation features are an early preview and currently i - Initial release of XcodeBuildMCP - Basic support for building iOS and macOS applications - diff --git a/README.md b/README.md index 28a1779c..fd3a115f 100644 --- a/README.md +++ b/README.md @@ -285,7 +285,7 @@ For further information on how to install the skill, see: [docs/SKILLS.md](docs/ ## Privacy -XcodeBuildMCP uses Sentry for error telemetry. For more information or to opt out of error telemetry see [docs/PRIVACY.md](docs/PRIVACY.md). +XcodeBuildMCP uses Sentry for internal runtime error telemetry only. For details and opt-out instructions, see [docs/PRIVACY.md](docs/PRIVACY.md). ## CLI diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index c8837192..5854f2a7 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -259,7 +259,7 @@ Default templates: ## Telemetry -By default, error logs are sent to Sentry. To disable: +By default, only internal XcodeBuildMCP runtime failures are sent to Sentry. User-domain errors (such as project build/test/config failures) are not sent. To disable telemetry entirely: ```yaml # Environment variable only (no config.yaml option) @@ -268,6 +268,11 @@ By default, error logs are sent to Sentry. To disable: See [PRIVACY.md](PRIVACY.md) for more information. +Notes: +- Sentry SDK logging is enabled for internal observability. +- Only explicitly opted-in internal logs are forwarded to Sentry; standard console logs are not auto-forwarded. +- Tool arguments and tool outputs are not captured by MCP wrapper telemetry (`recordInputs: false`, `recordOutputs: false`). + --- ## Quick reference diff --git a/docs/PRIVACY.md b/docs/PRIVACY.md index 652f658c..56573dce 100644 --- a/docs/PRIVACY.md +++ b/docs/PRIVACY.md @@ -3,8 +3,11 @@ XcodeBuildMCP uses Sentry for error monitoring and diagnostics. This helps track crashes and unexpected errors to improve reliability. ## What is sent to Sentry -- Error-level logs and diagnostic information only. -- Error logs may include error messages, stack traces, and in some cases file paths or project names. +- Internal XcodeBuildMCP failures only (for example: daemon/MCP runtime faults and unexpected exceptions in server code). +- User-domain errors are not sent (for example: project build/test failures, invalid user config, missing scheme/destination, simulator/app-state errors). +- Tool inputs/outputs are not captured by default, and environment/system variables are not attached as telemetry tags. +- Internal operational logs are sent only when explicitly marked for Sentry (`{ sentry: true }` in server code). Console logging is not automatically forwarded. +- Event payloads are scrubbed before send to remove request/user context and redact user home paths (for example `/Users//...`). ## Opting out To disable error telemetry, set: diff --git a/docs/SENTRY_AUDIT_REPORT_2026-02-11.md b/docs/SENTRY_AUDIT_REPORT_2026-02-11.md new file mode 100644 index 00000000..4af47a89 --- /dev/null +++ b/docs/SENTRY_AUDIT_REPORT_2026-02-11.md @@ -0,0 +1,184 @@ +# Investigation: Sentry Telemetry Scope and Privacy Audit (#204) + +## Summary +The current implementation captured too much data by default (including broad MCP instrumentation and PII-heavy tagging). The fix narrows telemetry to internal runtime failures only, keeps tracing enabled for observability, disables MCP input/output capture, and adds payload scrubbing/redaction. + +## Symptoms +- Sentry was initialized with `sendDefaultPii: true` and `tracesSampleRate: 1.0`. +- MCP server was wrapped with `wrapMcpServerWithSentry`, enabling broad per-call instrumentation. +- Logger default sent all `error` logs to Sentry, including user-domain failures. +- Sentry tags included sensitive environment/system values (HOME, USER, PATH, Xcode paths). +- Privacy docs understated actual collection scope. + +## Investigation Log + +### Phase 1 - Issue and baseline audit +**Hypothesis:** Telemetry defaults and wrapper behavior were broader than documented. +**Findings:** Issue #204 correctly identified mismatch between docs and implementation. +**Evidence:** +- GitHub issue: https://github.com/getsentry/XcodeBuildMCP/issues/204 +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/src/utils/sentry.ts` +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/src/server/server.ts` +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/src/utils/logger.ts` +**Conclusion:** Confirmed. + +### Phase 2 - Runtime path tracing +**Hypothesis:** User-domain tool failures were reaching Sentry through logger defaults. +**Findings:** `log('error', ...)` implicitly captured to Sentry unless overridden; many tool/runtime paths emit user-domain errors at error level. +**Evidence:** +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/src/utils/logger.ts` +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/src/utils/build-utils.ts` +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/src/runtime/tool-invoker.ts` +**Conclusion:** Confirmed. + +### Phase 3 - Version and docs alignment +**Hypothesis:** Sentry SDK was not at latest patch and docs were not aligned with behavior. +**Findings:** `@sentry/node` moved from `^10.37.0` to latest `10.38.0` and docs were updated to match internal-only policy. +**Evidence:** +- Command: `npm view @sentry/node version` returned `10.38.0` +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/package.json` +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/docs/PRIVACY.md` +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/docs/CONFIGURATION.md` +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/README.md` +**Conclusion:** Confirmed. + +## Root Cause +Three defaults combined to over-collect telemetry: +1. `sendDefaultPii: true` and tracing at 100% in Sentry init. +2. `wrapMcpServerWithSentry` around the MCP server. +3. Logger behavior that captured every `error` log to Sentry by default. + +This effectively blurred boundaries between internal platform failures and user-domain build/config/runtime failures, and increased risk of sensitive metadata leakage. + +## Changes Implemented +1. Sentry initialization hardened (`sendDefaultPii: false`, `tracesSampleRate: 1.0`, breadcrumbs disabled, `beforeSend` scrubbing/redaction). +2. Sensitive tags/context removed (no env dumps, no HOME/USER/PATH/Xcode path tagging). +3. Restored MCP wrapper with explicit safe options (`recordInputs: false`, `recordOutputs: false`) to keep tool-level observability without payload capture. +4. Logger changed to explicit opt-in capture only (`context.sentry === true`). +5. Internal boundary capture retained only where appropriate (startup/shutdown/fatal daemon internal errors). +6. Added tests for explicit capture policy and path redaction. +7. Updated privacy/config/README/architecture docs and changelog. + +## Eliminated Hypotheses +- "Only MCP-level faults are captured today": Eliminated (not true before this patch due to logger defaults and wrapper). +- "Docs accurately reflected telemetry scope": Eliminated. + +## Recommendations +1. Keep Sentry capture explicit and centralized to internal runtime boundaries. +2. Avoid adding environment or filesystem metadata to telemetry tags. +3. Preserve redaction tests to prevent regressions. +4. Continue documenting telemetry scope in user-facing docs whenever behavior changes. + +## Preventive Measures +- CI should keep redaction and logger policy tests running by default. +- Any future telemetry additions should require explicit privacy review with docs update in same PR. + +## Validation +All relevant quality checks were executed after changes: + +- `npm run format:check` ✅ +- `npm run lint` ✅ +- `npm run typecheck` ✅ +- `npm run build` ✅ +- `npm test` ✅ (117 test files passed; 1274 tests passed, 15 skipped) + +Notes: +- Test output includes expected stderr from negative-path parser tests in `src/utils/__tests__/nskeyedarchiver-parser.test.ts`; test run still passed. + +## Addendum: MCP Wrapper Capture Semantics (verified against SDK 10.38.0) +- `wrapMcpServerWithSentry` resolves options as: + - `recordInputs: options?.recordInputs ?? sendDefaultPii` + - `recordOutputs: options?.recordOutputs ?? sendDefaultPii` +- For MCP request spans, `tools/call` and `prompts/get` arguments are added as `mcp.request.argument.*` attributes when `recordInputs=true`. +- Tool/prompt results are added as `mcp.tool.result*` / `mcp.prompt.result*` attributes when `recordOutputs=true`. +- Built-in MCP PII filtering in the SDK only strips network-level fields (`client.address`, `client.port`, `mcp.resource.uri`) when `sendDefaultPii=false`; it does not by itself scrub arbitrary tool argument/output payloads. + +Evidence (source-inspected package build): +- `@sentry/core@10.38.0` `build/cjs/integrations/mcp-server/index.js` +- `@sentry/core@10.38.0` `build/cjs/integrations/mcp-server/methodConfig.js` +- `@sentry/core@10.38.0` `build/cjs/integrations/mcp-server/resultExtraction.js` +- `@sentry/core@10.38.0` `build/cjs/integrations/mcp-server/piiFiltering.js` + +## Addendum: Live Validation (2026-02-12) + +### Findings +- Runtime config/dependency tags are being attached to issues when events are captured after bootstrap context is set. +- Example: issue `XCODEBUILDMCP-6` includes `runtime.mode`, `xcode.version`, `xcode.xcodebuild_path`, `axe.source`, and config tags. +- Startup-phase config parse warnings can occur before full runtime context is attached, so those earlier events may not show the full tag set. +- MCP wrapper instrumentation is active in-process: + - Local debug output shows sampled MCP spans for `initialize`, `notifications/initialized`, and `tools/call session_show_defaults`. + - Local exporter reports spans exported. +- Despite local span export, Sentry project query for spans currently returns `count() = 0` in the tested time window. + +### Evidence +- Local MCP call validation: + - `session_show_defaults` invoked over stdio client; server started successfully. +- Local in-memory instrumentation validation: + - Debug logs show: + - `Starting sampled root span op: mcp.server` + - `Finishing ... tools/call session_show_defaults` + - `SpanExporter exported 3 spans` +- Sentry MCP queries: + - Spans in last hour: `count() = 0` + - Transactions in last hour: `count() = 0` + - Trace for issue `XCODEBUILDMCP-6`: `Total Spans: 0, Errors: 1` + +## Addendum: MCP View + Logs Explorer Deep Dive (2026-02-12) + +### Scope +Investigated two active symptoms: +- MCP tools are not visible in Sentry MCP view. +- Logs are not visible in Sentry Logs Explorer. + +### Key Findings +- Error events are ingesting correctly in the target project. + - Sentry query: errors in last 24h = `11`. +- Logs and spans datasets remain empty in the same project/time windows. + - Sentry query: logs in last 24h/7d = `0`. + - Sentry query: spans in last 24h/14d = `0`. + - Sentry query: transactions in last 14d/15m = `0`. +- SDK-side emission is working for both logs and transactions. + - Direct probe emitted: + - `Sentry.logger.info('envelope logger probe ...')` + - `Sentry.startSpan({ forceTransaction: true, ... })` + - Runtime instrumentation confirmed envelope item types sent: + - `ENVELOPE_ITEM_TYPES log` + - `ENVELOPE_ITEM_TYPES transaction` + - plus expected `event`/`session` items. +- Despite emitted envelopes, Sentry queries still return zero logs/spans/transactions. + - Strongly indicates an ingestion/storage/configuration issue outside current app code path. + +### Code-Path Validation +- MCP wrapper is enabled with safe options: + - `src/server/server.ts:72` uses `wrapMcpServerWithSentry(..., { recordInputs: false, recordOutputs: false })`. +- Sentry logs pipeline is enabled: + - `src/utils/sentry.ts:282` sets `enableLogs: true`. + - `src/utils/sentry.ts:286` sets `beforeSendLog: redactLog`. +- Logger forwards only explicit opt-in internal logs: + - `src/utils/logger.ts:56` (`context?.sentry === true`). + - `src/utils/logger.ts:236` fallback uses `captureMessage` only if logger method is unavailable. +- Runtime split is real: + - Daemon handles `tool.invoke` requests (`src/daemon/daemon-server.ts:117`), including `runtime: 'daemon'` (`src/daemon/daemon-server.ts:128`). + - CLI paths route many invocations through daemon (`src/runtime/tool-invoker.ts:192`). + - MCP wrapper only covers stdio MCP server runtime (`src/server/server.ts:72`). + +### Root Cause Assessment (Current Confidence) +- Most likely primary blocker: Sentry-side configuration/entitlement/pipeline for traces and logs in this project/org (not client emission). +- Secondary (not primary) code risk: + - `process.exit(...)` without explicit `Sentry.flush/close` in shutdown paths can still drop buffered telemetry in some paths: + - `src/server/start-mcp-server.ts:68` + - `src/server/start-mcp-server.ts:83` + - `src/daemon.ts:303` + - `src/daemon.ts:310` + - `src/daemon.ts:402` + +### Eliminated Hypotheses +- "MCP wrapper is removed or disabled." Eliminated. +- "Logs are not being captured by SDK at all." Eliminated (capture hooks + envelope inspection confirm capture/send). +- "Transactions are not being created by SDK at all." Eliminated (manual forced transaction emitted and sent). + +### Recommended Next Steps +1. Verify project-level traces/logs ingestion settings in Sentry (`sentry/xcodebuildmcp`) and any org-level sampling/filtering rules dropping transactions/logs. +2. Verify account/product entitlement for Logs and Performance on this project. +3. Add explicit shutdown drain in app code (`Sentry.flush`/`Sentry.close`) before `process.exit(...)` to reduce telemetry loss on fast shutdown. +4. Keep MCP-view expectation scoped to MCP stdio runtime; add daemon-specific spans if daemon tool-call observability is required in traces. diff --git a/docs/dev/ARCHITECTURE.md b/docs/dev/ARCHITECTURE.md index 0219a5f3..51240f53 100644 --- a/docs/dev/ARCHITECTURE.md +++ b/docs/dev/ARCHITECTURE.md @@ -564,4 +564,6 @@ The guide covers: - Sensitive information scrubbed from errors - Stack traces limited to application code -- Sentry integration respects privacy settings +- Sentry capture is explicit (`{ sentry: true }`) and limited to internal runtime failures +- MCP wrapper auto-instrumentation is enabled for MCP observability, with tool input/output capture disabled (`recordInputs: false`, `recordOutputs: false`) +- Request/user context and user home paths are scrubbed before telemetry is sent diff --git a/package-lock.json b/package-lock.json index 1c8ceb93..49adc72e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.25.1", "@sentry/cli": "^3.1.0", - "@sentry/node": "^10.37.0", + "@sentry/node": "^10.38.0", "bplist-parser": "^0.3.2", "chokidar": "^5.0.0", "uuid": "^11.1.0", @@ -2036,18 +2036,18 @@ } }, "node_modules/@sentry/core": { - "version": "10.37.0", - "resolved": "https://registry.npmjs.org/@sentry/core/-/core-10.37.0.tgz", - "integrity": "sha512-hkRz7S4gkKLgPf+p3XgVjVm7tAfvcEPZxeACCC6jmoeKhGkzN44nXwLiqqshJ25RMcSrhfFvJa/FlBg6zupz7g==", + "version": "10.38.0", + "resolved": "https://registry.npmjs.org/@sentry/core/-/core-10.38.0.tgz", + "integrity": "sha512-1pubWDZE5y5HZEPMAZERP4fVl2NH3Ihp1A+vMoVkb3Qc66Diqj1WierAnStlZP7tCx0TBa0dK85GTW/ZFYyB9g==", "license": "MIT", "engines": { "node": ">=18" } }, "node_modules/@sentry/node": { - "version": "10.37.0", - "resolved": "https://registry.npmjs.org/@sentry/node/-/node-10.37.0.tgz", - "integrity": "sha512-R0n0McfnPy9D1mGKQXNPG3pfw53QxHBHNbCXGAQJoB1LjUHENM+A645tYBheac4SQjMhbu8i9De9ZyHIQKce/Q==", + "version": "10.38.0", + "resolved": "https://registry.npmjs.org/@sentry/node/-/node-10.38.0.tgz", + "integrity": "sha512-wriyDtWDAoatn8EhOj0U4PJR1WufiijTsCGALqakOHbFiadtBJANLe6aSkXoXT4tegw59cz1wY4NlzHjYksaPw==", "license": "MIT", "dependencies": { "@opentelemetry/api": "^1.9.0", @@ -2080,10 +2080,10 @@ "@opentelemetry/sdk-trace-base": "^2.5.0", "@opentelemetry/semantic-conventions": "^1.39.0", "@prisma/instrumentation": "7.2.0", - "@sentry/core": "10.37.0", - "@sentry/node-core": "10.37.0", - "@sentry/opentelemetry": "10.37.0", - "import-in-the-middle": "^2.0.1", + "@sentry/core": "10.38.0", + "@sentry/node-core": "10.38.0", + "@sentry/opentelemetry": "10.38.0", + "import-in-the-middle": "^2.0.6", "minimatch": "^9.0.0" }, "engines": { @@ -2091,15 +2091,15 @@ } }, "node_modules/@sentry/node-core": { - "version": "10.37.0", - "resolved": "https://registry.npmjs.org/@sentry/node-core/-/node-core-10.37.0.tgz", - "integrity": "sha512-gQXf9764ZlEZQqqIaysxOrpHu2H867WN/G8EsHgaUJxxZMJ4ZMixinuFef+iQUE3la3/2MPoGx0A8AR7vRH9hA==", + "version": "10.38.0", + "resolved": "https://registry.npmjs.org/@sentry/node-core/-/node-core-10.38.0.tgz", + "integrity": "sha512-ErXtpedrY1HghgwM6AliilZPcUCoNNP1NThdO4YpeMq04wMX9/GMmFCu46TnCcg6b7IFIOSr2S4yD086PxLlHQ==", "license": "MIT", "dependencies": { "@apm-js-collab/tracing-hooks": "^0.3.1", - "@sentry/core": "10.37.0", - "@sentry/opentelemetry": "10.37.0", - "import-in-the-middle": "^2.0.1" + "@sentry/core": "10.38.0", + "@sentry/opentelemetry": "10.38.0", + "import-in-the-middle": "^2.0.6" }, "engines": { "node": ">=18" @@ -2139,12 +2139,12 @@ } }, "node_modules/@sentry/opentelemetry": { - "version": "10.37.0", - "resolved": "https://registry.npmjs.org/@sentry/opentelemetry/-/opentelemetry-10.37.0.tgz", - "integrity": "sha512-elRAsmgAcaprVFV1JoxxqU9eteBz3g9fpmecfDk5KTe/Txr2OOxdIiLF/4I0g4MMib3DKtr6iGgme2J+l8H8cA==", + "version": "10.38.0", + "resolved": "https://registry.npmjs.org/@sentry/opentelemetry/-/opentelemetry-10.38.0.tgz", + "integrity": "sha512-YPVhWfYmC7nD3EJqEHGtjp4fp5LwtAbE5rt9egQ4hqJlYFvr8YEz9sdoqSZxO0cZzgs2v97HFl/nmWAXe52G2Q==", "license": "MIT", "dependencies": { - "@sentry/core": "10.37.0" + "@sentry/core": "10.38.0" }, "engines": { "node": ">=18" diff --git a/package.json b/package.json index ab71273f..dc22c2d3 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.25.1", "@sentry/cli": "^3.1.0", - "@sentry/node": "^10.37.0", + "@sentry/node": "^10.38.0", "bplist-parser": "^0.3.2", "chokidar": "^5.0.0", "uuid": "^11.1.0", diff --git a/src/cli.ts b/src/cli.ts index 3a37ef11..f44bb606 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -5,6 +5,7 @@ import { buildYargsApp } from './cli/yargs-app.ts'; import { getSocketPath, getWorkspaceKey, resolveWorkspaceRoot } from './daemon/socket-path.ts'; import { startMcpServer } from './server/start-mcp-server.ts'; import { listCliWorkflowIdsFromManifest } from './runtime/tool-catalog.ts'; +import { flushAndCloseSentry, initSentry, recordBootstrapDurationMetric } from './utils/sentry.ts'; function findTopLevelCommand(argv: string[]): string | undefined { const flagsWithValue = new Set(['--socket', '--log-level', '--style']); @@ -30,10 +31,12 @@ function findTopLevelCommand(argv: string[]): string | undefined { } async function main(): Promise { + const cliBootstrapStartedAt = Date.now(); if (process.argv.includes('mcp')) { await startMcpServer(); return; } + initSentry({ mode: 'cli' }); // CLI mode uses disableSessionDefaults to show all tool parameters as flags const result = await bootstrapRuntime({ @@ -87,10 +90,16 @@ async function main(): Promise { cliExposedWorkflowIds, }); + recordBootstrapDurationMetric('cli', Date.now() - cliBootstrapStartedAt); await yargsApp.parseAsync(); } -main().catch((err) => { - console.error(err instanceof Error ? err.message : String(err)); - process.exit(1); -}); +main() + .then(async () => { + await flushAndCloseSentry(2000); + }) + .catch(async (err) => { + console.error(err instanceof Error ? err.message : String(err)); + await flushAndCloseSentry(2000); + process.exit(1); + }); diff --git a/src/core/resources.ts b/src/core/resources.ts index 5a7e984e..ba6d8e8e 100644 --- a/src/core/resources.ts +++ b/src/core/resources.ts @@ -54,6 +54,9 @@ export async function loadResources(): Promise> { for (const resource of RESOURCES) { if (!resource.uri || !resource.handler || typeof resource.handler !== 'function') { log('error', `Invalid resource structure for ${resource.name ?? 'unknown'}`); + log('error', '[infra/resources] invalid resource structure detected during registration', { + sentry: true, + }); continue; } diff --git a/src/daemon.ts b/src/daemon.ts index 02151fa8..e6a2b733 100644 --- a/src/daemon.ts +++ b/src/daemon.ts @@ -28,6 +28,19 @@ import { getDaemonRuntimeActivitySnapshot, hasActiveRuntimeSessions, } from './daemon/idle-shutdown.ts'; +import { getDefaultCommandExecutor } from './utils/command.ts'; +import { resolveAxeBinary } from './utils/axe/index.ts'; +import { + flushAndCloseSentry, + getAxeVersionMetadata, + getXcodeVersionMetadata, + initSentry, + recordBootstrapDurationMetric, + recordDaemonGaugeMetric, + recordDaemonLifecycleMetric, + setSentryRuntimeContext, +} from './utils/sentry.ts'; +import { isXcodemakeBinaryAvailable, isXcodemakeEnabled } from './utils/xcodemake/index.ts'; async function checkExistingDaemon(socketPath: string): Promise { return new Promise((resolve) => { @@ -114,6 +127,7 @@ function resolveLogLevel(): LogLevel | null { } async function main(): Promise { + const daemonBootstrapStart = Date.now(); // Bootstrap runtime first to get config and workspace info const result = await bootstrapRuntime({ runtime: 'daemon', @@ -147,6 +161,9 @@ async function main(): Promise { } } + initSentry({ mode: 'cli-daemon' }); + recordDaemonLifecycleMetric('start'); + log('info', `[Daemon] xcodebuildmcp daemon ${version} starting...`); // Get socket path (env override or workspace-derived) @@ -168,6 +185,7 @@ async function main(): Promise { if (isRunning) { log('error', '[Daemon] Another daemon is already running for this workspace'); console.error('Error: Daemon is already running for this workspace'); + await flushAndCloseSentry(1000); process.exit(1); } @@ -188,6 +206,45 @@ async function main(): Promise { return true; }); const xcodeIdeWorkflowEnabled = daemonWorkflows.includes('xcode-ide'); + const commandExecutor = getDefaultCommandExecutor(); + const xcodeVersion = await getXcodeVersionMetadata(async (command) => { + const result = await commandExecutor(command, 'Get Xcode Version'); + return { success: result.success, output: result.output ?? '' }; + }); + const xcodeAvailable = [ + xcodeVersion.version, + xcodeVersion.buildVersion, + xcodeVersion.developerDir, + xcodeVersion.xcodebuildPath, + ].some((value) => Boolean(value)); + const axeBinary = resolveAxeBinary(); + const axeAvailable = axeBinary !== null; + const axeSource = axeBinary?.source ?? 'unavailable'; + const xcodemakeAvailable = isXcodemakeBinaryAvailable(); + const axeVersion = await getAxeVersionMetadata(async (command) => { + const result = await commandExecutor(command, 'Get AXe Version'); + return { success: result.success, output: result.output ?? '' }; + }, axeBinary?.path); + setSentryRuntimeContext({ + mode: 'cli-daemon', + xcodeAvailable, + enabledWorkflows: daemonWorkflows, + disableSessionDefaults: result.runtime.config.disableSessionDefaults, + disableXcodeAutoSync: result.runtime.config.disableXcodeAutoSync, + incrementalBuildsEnabled: result.runtime.config.incrementalBuildsEnabled, + debugEnabled: result.runtime.config.debug, + uiDebuggerGuardMode: result.runtime.config.uiDebuggerGuardMode, + xcodeIdeWorkflowEnabled, + axeAvailable, + axeSource, + axeVersion, + xcodeDeveloperDir: xcodeVersion.developerDir, + xcodebuildPath: xcodeVersion.xcodebuildPath, + xcodemakeAvailable, + xcodemakeEnabled: isXcodemakeEnabled(), + xcodeVersion: xcodeVersion.version, + xcodeBuildVersion: xcodeVersion.buildVersion, + }); // Build tool catalog using manifest system const catalog = await buildDaemonToolCatalogFromManifest({ @@ -217,6 +274,7 @@ async function main(): Promise { `[Daemon] Idle shutdown enabled: timeout=${idleTimeoutMs}ms interval=${DEFAULT_DAEMON_IDLE_CHECK_INTERVAL_MS}ms`, ); } + recordDaemonGaugeMetric('idle_timeout_ms', idleTimeoutMs); let isShuttingDown = false; let inFlightRequests = 0; @@ -239,6 +297,7 @@ async function main(): Promise { idleCheckTimer = null; } + recordDaemonLifecycleMetric('shutdown'); log('info', '[Daemon] Shutting down...'); // Close the server @@ -250,14 +309,18 @@ async function main(): Promise { removeStaleSocket(socketPath); log('info', '[Daemon] Cleanup complete'); - process.exit(0); + void flushAndCloseSentry(2000).finally(() => { + process.exit(0); + }); }); // Force exit if server doesn't close in time setTimeout(() => { log('warn', '[Daemon] Forced shutdown after timeout'); cleanupWorkspaceDaemonFiles(workspaceKey); - process.exit(1); + void flushAndCloseSentry(1000).finally(() => { + process.exit(1); + }); }, 5000); }; @@ -275,12 +338,23 @@ async function main(): Promise { onRequestStarted: () => { inFlightRequests += 1; markActivity(); + recordDaemonGaugeMetric('inflight_requests', inFlightRequests); + const sessionSnapshot = getDaemonRuntimeActivitySnapshot(); + recordDaemonGaugeMetric('active_sessions', sessionSnapshot.activeOperationCount); }, onRequestFinished: () => { inFlightRequests = Math.max(0, inFlightRequests - 1); markActivity(); + recordDaemonGaugeMetric('inflight_requests', inFlightRequests); + const sessionSnapshot = getDaemonRuntimeActivitySnapshot(); + recordDaemonGaugeMetric('active_sessions', sessionSnapshot.activeOperationCount); }, }); + recordDaemonGaugeMetric('inflight_requests', inFlightRequests); + recordDaemonGaugeMetric( + 'active_sessions', + getDaemonRuntimeActivitySnapshot().activeOperationCount, + ); if (idleTimeoutMs > 0) { idleCheckTimer = setInterval(() => { @@ -288,6 +362,10 @@ async function main(): Promise { return; } + const sessionSnapshot = getDaemonRuntimeActivitySnapshot(); + recordDaemonGaugeMetric('inflight_requests', inFlightRequests); + recordDaemonGaugeMetric('active_sessions', sessionSnapshot.activeOperationCount); + const idleForMs = Date.now() - lastActivityAt; if (idleForMs < idleTimeoutMs) { return; @@ -297,7 +375,6 @@ async function main(): Promise { return; } - const sessionSnapshot = getDaemonRuntimeActivitySnapshot(); if (hasActiveRuntimeSessions(sessionSnapshot)) { return; } @@ -330,6 +407,7 @@ async function main(): Promise { writeLine(`Workspace: ${workspaceRoot}`); writeLine(`Socket: ${socketPath}`); writeLine(`Tools: ${catalog.tools.length}`); + recordBootstrapDurationMetric('cli-daemon', Date.now() - daemonBootstrapStart); }); // Handle graceful shutdown @@ -337,9 +415,19 @@ async function main(): Promise { process.on('SIGINT', shutdown); } -main().catch((err) => { - const message = err instanceof Error ? err.message : String(err); - log('error', `Daemon error: ${message}`); +main().catch(async (err) => { + recordDaemonLifecycleMetric('crash'); + let message = 'Unknown daemon error'; + if (err instanceof Error) { + message = err.message; + } else if (typeof err === 'string') { + message = err; + } else if (err !== null && err !== undefined) { + message = String(err); + } + + log('error', `Daemon error: ${message}`, { sentry: true }); console.error('Daemon error:', message); + await flushAndCloseSentry(2000); process.exit(1); }); diff --git a/src/daemon/daemon-server.ts b/src/daemon/daemon-server.ts index 685acdf6..32a1da9f 100644 --- a/src/daemon/daemon-server.ts +++ b/src/daemon/daemon-server.ts @@ -205,12 +205,13 @@ export function startDaemonServer(ctx: DaemonServerContext): net.Server { }); } } catch (error) { - log('error', `[Daemon] Error handling request: ${error}`); + const message = error instanceof Error ? error.message : String(error); + log('error', `[Daemon] Internal error handling request: ${message}`, { sentry: true }); return writeFrame(socket, { ...base, error: { code: 'INTERNAL', - message: error instanceof Error ? error.message : String(error), + message, }, }); } finally { @@ -218,7 +219,7 @@ export function startDaemonServer(ctx: DaemonServerContext): net.Server { } }, (err) => { - log('error', `[Daemon] Frame parse error: ${err.message}`); + log('warning', `[Daemon] Frame parse error: ${err.message}`); }, ); @@ -227,12 +228,12 @@ export function startDaemonServer(ctx: DaemonServerContext): net.Server { log('info', '[Daemon] Client disconnected'); }); socket.on('error', (err) => { - log('error', `[Daemon] Socket error: ${err.message}`); + log('warning', `[Daemon] Socket error: ${err.message}`); }); }); server.on('error', (err) => { - log('error', `[Daemon] Server error: ${err.message}`); + log('warning', `[Daemon] Server error: ${err.message}`); }); server.on('close', () => { void xcodeIdeService.disconnect(); diff --git a/src/runtime/tool-invoker.ts b/src/runtime/tool-invoker.ts index 739cbe6a..e02f226a 100644 --- a/src/runtime/tool-invoker.ts +++ b/src/runtime/tool-invoker.ts @@ -3,6 +3,14 @@ import type { ToolResponse } from '../types/common.ts'; import { createErrorResponse } from '../utils/responses/index.ts'; import { DaemonClient } from '../cli/daemon-client.ts'; import { ensureDaemonRunning, DEFAULT_DAEMON_STARTUP_TIMEOUT_MS } from '../cli/daemon-control.ts'; +import { log } from '../utils/logger.ts'; +import { + recordInternalErrorMetric, + recordToolInvocationMetric, + type SentryToolInvocationOutcome, + type SentryToolRuntime, + type SentryToolTransport, +} from '../utils/sentry.ts'; /** * Enrich nextSteps for CLI rendering. @@ -40,6 +48,23 @@ function buildDaemonEnvOverrides(opts: InvokeOptions): Record | return Object.keys(envOverrides).length > 0 ? envOverrides : undefined; } +function getErrorKind(error: unknown): string { + if (error instanceof Error) { + return error.name || 'Error'; + } + return typeof error; +} + +function mapRuntimeToSentryToolRuntime(runtime: InvokeOptions['runtime']): SentryToolRuntime { + if (runtime === 'daemon') { + return 'daemon'; + } + if (runtime === 'mcp') { + return 'mcp'; + } + return 'cli'; +} + export class DefaultToolInvoker implements ToolInvoker { constructor(private catalog: ToolCatalog) {} @@ -85,13 +110,38 @@ export class DefaultToolInvoker implements ToolInvoker { args: Record, opts: InvokeOptions, ): Promise { + const startedAt = Date.now(); + const runtime = mapRuntimeToSentryToolRuntime(opts.runtime); + let transport: SentryToolTransport = 'direct'; + + const captureInvocationMetric = (outcome: SentryToolInvocationOutcome): void => { + recordToolInvocationMetric({ + toolName: tool.mcpName, + runtime, + transport, + outcome, + durationMs: Date.now() - startedAt, + }); + }; + + const captureInfraErrorMetric = (error: unknown): void => { + recordInternalErrorMetric({ + component: 'tool-invoker', + runtime, + errorKind: getErrorKind(error), + }); + }; + const xcodeIdeRemoteToolName = tool.xcodeIdeRemoteToolName; const isDynamicXcodeIdeTool = tool.workflow === 'xcode-ide' && typeof xcodeIdeRemoteToolName === 'string'; if (opts.runtime === 'cli' && isDynamicXcodeIdeTool) { + transport = 'xcode-ide-daemon'; const socketPath = opts.socketPath; if (!socketPath) { + captureInfraErrorMetric(new Error('SocketPathMissing')); + captureInvocationMetric('infra_error'); return createErrorResponse( 'Socket path required', `No socket path configured for daemon communication.`, @@ -111,6 +161,13 @@ export class DefaultToolInvoker implements ToolInvoker { env: envOverrideValue, }); } catch (error) { + log( + 'error', + `[infra/tool-invoker] xcode-ide daemon auto-start failed (${getErrorKind(error)})`, + { sentry: true }, + ); + captureInfraErrorMetric(error); + captureInvocationMetric('infra_error'); return createErrorResponse( 'Daemon auto-start failed', (error instanceof Error ? error.message : String(error)) + @@ -122,8 +179,16 @@ export class DefaultToolInvoker implements ToolInvoker { try { const response = await client.invokeXcodeIdeTool(xcodeIdeRemoteToolName, args); + captureInvocationMetric('completed'); return enrichNextStepsForCli(response, this.catalog); } catch (error) { + log( + 'error', + `[infra/tool-invoker] xcode-ide tool invocation transport failed (${getErrorKind(error)})`, + { sentry: true }, + ); + captureInfraErrorMetric(error); + captureInvocationMetric('infra_error'); return createErrorResponse( 'Xcode IDE invocation failed', error instanceof Error ? error.message : String(error), @@ -135,9 +200,12 @@ export class DefaultToolInvoker implements ToolInvoker { if (opts.runtime === 'cli') { if (mustUseDaemon) { + transport = 'daemon'; // Route through daemon with auto-start const socketPath = opts.socketPath; if (!socketPath) { + captureInfraErrorMetric(new Error('SocketPathMissing')); + captureInvocationMetric('infra_error'); return createErrorResponse( 'Socket path required', `No socket path configured for daemon communication.`, @@ -158,6 +226,11 @@ export class DefaultToolInvoker implements ToolInvoker { env: envOverrideValue, }); } catch (error) { + log('error', `[infra/tool-invoker] daemon auto-start failed (${getErrorKind(error)})`, { + sentry: true, + }); + captureInfraErrorMetric(error); + captureInvocationMetric('infra_error'); return createErrorResponse( 'Daemon auto-start failed', (error instanceof Error ? error.message : String(error)) + @@ -169,8 +242,16 @@ export class DefaultToolInvoker implements ToolInvoker { try { const response = await client.invokeTool(tool.mcpName, args); + captureInvocationMetric('completed'); return opts.runtime === 'cli' ? enrichNextStepsForCli(response, this.catalog) : response; } catch (error) { + log( + 'error', + `[infra/tool-invoker] daemon transport invocation failed for ${tool.mcpName} (${getErrorKind(error)})`, + { sentry: true }, + ); + captureInfraErrorMetric(error); + captureInvocationMetric('infra_error'); return createErrorResponse( 'Daemon invocation failed', error instanceof Error ? error.message : String(error), @@ -182,8 +263,16 @@ export class DefaultToolInvoker implements ToolInvoker { // Direct invocation (CLI stateless or daemon internal) try { const response = await tool.handler(args); + captureInvocationMetric('completed'); return opts.runtime === 'cli' ? enrichNextStepsForCli(response, this.catalog) : response; } catch (error) { + log( + 'error', + `[infra/tool-invoker] direct tool handler failed for ${tool.mcpName} (${getErrorKind(error)})`, + { sentry: true }, + ); + captureInfraErrorMetric(error); + captureInvocationMetric('infra_error'); const message = error instanceof Error ? error.message : String(error); return createErrorResponse('Tool execution failed', message); } diff --git a/src/server/bootstrap.ts b/src/server/bootstrap.ts index e5c6d791..3294360e 100644 --- a/src/server/bootstrap.ts +++ b/src/server/bootstrap.ts @@ -15,6 +15,15 @@ import { sessionStore } from '../utils/session-store.ts'; import { startXcodeStateWatcher, lookupBundleId } from '../utils/xcode-state-watcher.ts'; import { getDefaultCommandExecutor } from '../utils/command.ts'; import type { PredicateContext } from '../visibility/predicate-types.ts'; +import { resolveAxeBinary } from '../utils/axe/index.ts'; +import { isXcodemakeBinaryAvailable, isXcodemakeEnabled } from '../utils/xcodemake/index.ts'; +import { + getAxeVersionMetadata, + getXcodeVersionMetadata, + recordBootstrapDurationMetric, + recordXcodeBridgeSyncDurationMetric, + setSentryRuntimeContext, +} from '../utils/sentry.ts'; export interface BootstrapOptions { enabledWorkflows?: string[]; @@ -27,6 +36,7 @@ export async function bootstrapServer( server: McpServer, options: BootstrapOptions = {}, ): Promise { + const bootstrapStartedAt = Date.now(); server.server.setRequestHandler(SetLevelRequestSchema, async (request) => { const { level } = request.params; setLogLevel(level as LogLevel); @@ -160,8 +170,13 @@ export async function bootstrapServer( const xcodeToolsBridge = xcodeToolsAvailable ? getXcodeToolsBridgeManager(server) : null; xcodeToolsBridge?.setWorkflowEnabled(xcodeIdeEnabled); if (xcodeIdeEnabled && xcodeToolsBridge) { + const syncStartedAt = Date.now(); try { const syncResult = await xcodeToolsBridge.syncTools({ reason: 'startup' }); + recordXcodeBridgeSyncDurationMetric( + Date.now() - syncStartedAt, + syncResult.total > 0 ? 'success' : 'empty', + ); // After sync, if Xcode tools are active, re-register with updated context if (syncResult.total > 0 && xcodeDetection.runningUnderXcode) { log('info', `[xcode-ide] Xcode tools active - applying conflict filtering`); @@ -169,12 +184,54 @@ export async function bootstrapServer( await registerWorkflowsFromManifest(enabledWorkflows, ctx); } } catch (error) { + recordXcodeBridgeSyncDurationMetric(Date.now() - syncStartedAt, 'error'); log( 'warning', `[xcode-ide] Startup sync failed: ${error instanceof Error ? error.message : String(error)}`, ); + log('warning', '[infra/bootstrap] xcode-ide startup sync failed', { sentry: true }); } } + const xcodeVersion = await getXcodeVersionMetadata(async (command) => { + const commandResult = await executor(command, 'Get Xcode Version'); + return { success: commandResult.success, output: commandResult.output ?? '' }; + }); + const xcodeAvailable = [ + xcodeVersion.version, + xcodeVersion.buildVersion, + xcodeVersion.developerDir, + xcodeVersion.xcodebuildPath, + ].some((value) => Boolean(value)); + const axeBinary = resolveAxeBinary(); + const axeAvailable = axeBinary !== null; + const axeSource = axeBinary?.source ?? 'unavailable'; + const xcodemakeAvailable = isXcodemakeBinaryAvailable(); + const axeVersion = await getAxeVersionMetadata(async (command) => { + const commandResult = await executor(command, 'Get AXe Version'); + return { success: commandResult.success, output: commandResult.output ?? '' }; + }, axeBinary?.path); + setSentryRuntimeContext({ + mode: 'mcp', + xcodeAvailable, + enabledWorkflows: resolvedWorkflows, + disableSessionDefaults: result.runtime.config.disableSessionDefaults, + disableXcodeAutoSync: result.runtime.config.disableXcodeAutoSync, + incrementalBuildsEnabled: result.runtime.config.incrementalBuildsEnabled, + debugEnabled: result.runtime.config.debug, + uiDebuggerGuardMode: result.runtime.config.uiDebuggerGuardMode, + xcodeIdeWorkflowEnabled: xcodeIdeEnabled, + axeAvailable, + axeSource, + axeVersion, + xcodeDeveloperDir: xcodeVersion.developerDir, + xcodebuildPath: xcodeVersion.xcodebuildPath, + xcodemakeAvailable, + xcodemakeEnabled: isXcodemakeEnabled(), + xcodeVersion: xcodeVersion.version, + xcodeBuildVersion: xcodeVersion.buildVersion, + }); + await registerResources(server); + recordBootstrapDurationMetric('mcp', Date.now() - bootstrapStartedAt); } diff --git a/src/server/server.ts b/src/server/server.ts index 565a048b..4bc514cf 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -14,21 +14,13 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; +import * as Sentry from '@sentry/node'; import { log } from '../utils/logger.ts'; import { version } from '../version.ts'; -import * as Sentry from '@sentry/node'; import { getServer, setServer } from './server-state.ts'; -/** - * Create and configure the MCP server - * @returns Configured MCP server instance - */ -export function createServer(): McpServer { - if (getServer()) { - throw new Error('MCP server already initialized.'); - } - // Create server instance - const baseServer = new McpServer( +function createBaseServerInstance(): McpServer { + return new McpServer( { name: 'xcodebuildmcp', version, @@ -64,16 +56,30 @@ Always start by calling session_show_defaults to see current configuration, then logging: {}, }, }, - ); + ) as unknown as McpServer; +} + +/** + * Create and configure the MCP server + * @returns Configured MCP server instance + */ +export function createServer(): McpServer { + if (getServer()) { + throw new Error('MCP server already initialized.'); + } + // Create server instance + const baseServer = createBaseServerInstance(); + const server = Sentry.wrapMcpServerWithSentry(baseServer, { + recordInputs: false, + recordOutputs: false, + }); - // Wrap server with Sentry for MCP instrumentation - const wrappedServer = Sentry.wrapMcpServerWithSentry(baseServer); - setServer(wrappedServer); + setServer(server); // Log server initialization - log('info', `Server initialized with Sentry MCP instrumentation (version ${version})`); + log('info', `Server initialized (version ${version})`); - return wrappedServer; + return server; } /** diff --git a/src/server/start-mcp-server.ts b/src/server/start-mcp-server.ts index b43c3ef4..c7b0b05b 100644 --- a/src/server/start-mcp-server.ts +++ b/src/server/start-mcp-server.ts @@ -9,7 +9,7 @@ import { createServer, startServer } from './server.ts'; import { log, setLogLevel } from '../utils/logger.ts'; -import { initSentry } from '../utils/sentry.ts'; +import { flushAndCloseSentry, initSentry } from '../utils/sentry.ts'; import { getDefaultDebuggerManager } from '../utils/debugger/index.ts'; import { version } from '../version.ts'; import process from 'node:process'; @@ -27,7 +27,7 @@ export async function startMcpServer(): Promise { // Clients can override via logging/setLevel MCP request setLogLevel('info'); - initSentry(); + initSentry({ mode: 'mcp' }); const server = createServer(); @@ -48,23 +48,24 @@ export async function startMcpServer(): Promise { await shutdownXcodeToolsBridge(); } catch (error) { exitCode = 1; - log('error', `Failed to shutdown Xcode tools bridge: ${String(error)}`); + log('error', `Failed to shutdown Xcode tools bridge: ${String(error)}`, { sentry: true }); } try { await getDefaultDebuggerManager().disposeAll(); } catch (error) { exitCode = 1; - log('error', `Failed to dispose debugger sessions: ${String(error)}`); + log('error', `Failed to dispose debugger sessions: ${String(error)}`, { sentry: true }); } try { await server.close(); } catch (error) { exitCode = 1; - log('error', `Failed to close MCP server: ${String(error)}`); + log('error', `Failed to close MCP server: ${String(error)}`, { sentry: true }); } + await flushAndCloseSentry(2000); process.exit(exitCode); }; @@ -78,7 +79,9 @@ export async function startMcpServer(): Promise { log('info', `XcodeBuildMCP server (version ${version}) started successfully`); } catch (error) { + log('error', `Fatal error in startMcpServer(): ${String(error)}`, { sentry: true }); console.error('Fatal error in startMcpServer():', error); + await flushAndCloseSentry(2000); process.exit(1); } } diff --git a/src/utils/__tests__/logger.test.ts b/src/utils/__tests__/logger.test.ts new file mode 100644 index 00000000..d728b13a --- /dev/null +++ b/src/utils/__tests__/logger.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it } from 'vitest'; +import { __mapLogLevelToSentryForTests, __shouldCaptureToSentryForTests } from '../logger.ts'; + +describe('logger sentry capture policy', () => { + it('does not capture by default', () => { + expect(__shouldCaptureToSentryForTests()).toBe(false); + }); + + it('does not capture when sentry is false', () => { + expect(__shouldCaptureToSentryForTests({ sentry: false })).toBe(false); + }); + + it('captures only when explicitly enabled', () => { + expect(__shouldCaptureToSentryForTests({ sentry: true })).toBe(true); + }); + + it('maps internal levels to Sentry log levels', () => { + expect(__mapLogLevelToSentryForTests('emergency')).toBe('fatal'); + expect(__mapLogLevelToSentryForTests('warning')).toBe('warn'); + expect(__mapLogLevelToSentryForTests('notice')).toBe('info'); + expect(__mapLogLevelToSentryForTests('error')).toBe('error'); + }); +}); diff --git a/src/utils/__tests__/sentry-redaction.test.ts b/src/utils/__tests__/sentry-redaction.test.ts new file mode 100644 index 00000000..2afc6890 --- /dev/null +++ b/src/utils/__tests__/sentry-redaction.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from 'vitest'; +import type * as Sentry from '@sentry/node'; +import { + __parseXcodeVersionForTests, + __redactEventForTests, + __redactLogForTests, +} from '../sentry.ts'; + +describe('sentry redaction', () => { + it('removes identity/request context and redacts user paths', () => { + const event: Sentry.Event = { + message: 'failed to open /Users/cam/project/App/AppDelegate.swift', + user: { id: '123' }, + request: { url: 'https://example.com' }, + breadcrumbs: [{ category: 'test', message: '/Users/cam/tmp' }], + exception: { + values: [ + { + type: 'Error', + value: 'build failed in /Users/cam/project', + stacktrace: { + frames: [ + { + abs_path: '/Users/cam/project/src/tool.ts', + filename: '/Users/cam/project/src/tool.ts', + }, + ], + }, + }, + ], + }, + extra: { + output: 'log at /Users/cam/project/build.log', + attempts: 1, + }, + }; + + const redacted = __redactEventForTests(event); + + expect(redacted.user).toBeUndefined(); + expect(redacted.request).toBeUndefined(); + expect(redacted.breadcrumbs).toBeUndefined(); + expect(redacted.message).toContain('/Users//project/App/AppDelegate.swift'); + expect(redacted.exception?.values?.[0]?.value).toContain('/Users//project'); + expect(redacted.exception?.values?.[0]?.stacktrace?.frames?.[0]?.abs_path).toContain( + '/Users//project/src/tool.ts', + ); + expect(redacted.exception?.values?.[0]?.stacktrace?.frames?.[0]?.filename).toContain( + '/Users//project/src/tool.ts', + ); + expect(redacted.extra?.output).toBe('log at /Users//project/build.log'); + expect(redacted.extra?.attempts).toBe(1); + }); + + it('parses xcode version metadata safely', () => { + const parsed = __parseXcodeVersionForTests('Xcode 16.3\nBuild version 16E123\n'); + expect(parsed).toEqual({ version: '16.3', buildVersion: '16E123' }); + }); + + it('redacts user paths in log payloads', () => { + const log: Sentry.Log = { + level: 'info', + message: 'tool failed at /Users/cam/project/build.log', + attributes: { + file: '/Users/cam/project/App/AppDelegate.swift', + nested: { cwd: '/Users/cam/project' }, + }, + }; + + const redacted = __redactLogForTests(log); + + expect(redacted).toEqual({ + level: 'info', + message: 'tool failed at /Users//project/build.log', + attributes: { + file: '/Users//project/App/AppDelegate.swift', + nested: { cwd: '/Users//project' }, + }, + }); + }); +}); diff --git a/src/utils/config-store.ts b/src/utils/config-store.ts index d1daa65f..ef987660 100644 --- a/src/utils/config-store.ts +++ b/src/utils/config-store.ts @@ -134,6 +134,13 @@ function parseDebuggerBackend(value: string | undefined): DebuggerBackendKind | return undefined; } +function getErrorKind(error: unknown): string { + if (error instanceof Error) { + return error.name || 'Error'; + } + return typeof error; +} + function setIfDefined( config: RuntimeConfigOverrides, key: K, @@ -405,9 +412,13 @@ export async function initConfigStore(opts: { const errorMessage = result.error instanceof Error ? result.error.message : String(result.error); log('warning', `Failed to read or parse project config at ${result.path}. ${errorMessage}`); + log('warning', '[infra/config-store] project config read/parse failed', { sentry: true }); } } catch (error) { log('warning', `Failed to load project config from ${opts.cwd}. ${error}`); + log('warning', `[infra/config-store] project config load threw (${getErrorKind(error)})`, { + sentry: true, + }); } storeState.fileConfig = fileConfig; diff --git a/src/utils/logger.ts b/src/utils/logger.ts index 759ccf26..880ed351 100644 --- a/src/utils/logger.ts +++ b/src/utils/logger.ts @@ -52,6 +52,10 @@ export interface LogContext { sentry?: boolean; } +export function __shouldCaptureToSentryForTests(context?: LogContext): boolean { + return context?.sentry === true; +} + // Client-requested log level ("none" means no output unless explicitly enabled) let clientLogLevel: LogLevel = 'none'; @@ -67,6 +71,7 @@ function isTestEnv(): boolean { } type SentryModule = typeof import('@sentry/node'); +type SentryLogLevel = 'trace' | 'debug' | 'info' | 'warn' | 'error' | 'fatal'; const require = createRequire( typeof __filename === 'string' ? __filename : resolve(process.cwd(), 'package.json'), @@ -95,6 +100,30 @@ function withSentry(cb: (s: SentryModule) => void): void { } } +function mapLogLevelToSentry(level: string): SentryLogLevel { + switch (level.toLowerCase()) { + case 'emergency': + case 'alert': + return 'fatal'; + case 'critical': + case 'error': + return 'error'; + case 'warning': + return 'warn'; + case 'debug': + return 'debug'; + case 'notice': + case 'info': + return 'info'; + default: + return 'info'; + } +} + +export function __mapLogLevelToSentryForTests(level: string): SentryLogLevel { + return mapLogLevelToSentry(level); +} + /** * Set the minimum log level for client-requested filtering * @param level The minimum log level to output @@ -193,12 +222,19 @@ export function log(level: string, message: string, context?: LogContext): void const timestamp = new Date().toISOString(); const logMessage = `[${timestamp}] [${level.toUpperCase()}] ${message}`; - // Default: error level goes to Sentry - // But respect explicit override from context - const captureToSentry = sentryEnabled && (context?.sentry ?? level === 'error'); + // Sentry capture is explicit opt-in only. + const captureToSentry = sentryEnabled && __shouldCaptureToSentryForTests(context); if (captureToSentry) { - withSentry((s) => s.captureMessage(logMessage)); + withSentry((s) => { + const sentryLevel = mapLogLevelToSentry(level); + const loggerMethod = s.logger?.[sentryLevel]; + if (typeof loggerMethod === 'function') { + loggerMethod(message); + return; + } + s.captureMessage(logMessage); + }); } if (logFileStream && clientLogLevel !== 'none') { diff --git a/src/utils/sentry.ts b/src/utils/sentry.ts index 8c7af75b..71918a9f 100644 --- a/src/utils/sentry.ts +++ b/src/utils/sentry.ts @@ -6,79 +6,143 @@ */ import * as Sentry from '@sentry/node'; -import { execSync } from 'child_process'; import { version } from '../version.ts'; -// Inlined system info functions to avoid circular dependencies -function getXcodeInfo(): { version: string; path: string; selectedXcode: string; error?: string } { - try { - const xcodebuildOutput = execSync('xcodebuild -version', { encoding: 'utf8' }).trim(); - const version = xcodebuildOutput.split('\n').slice(0, 2).join(' - '); - const path = execSync('xcode-select -p', { encoding: 'utf8' }).trim(); - const selectedXcode = execSync('xcrun --find xcodebuild', { encoding: 'utf8' }).trim(); - - return { version, path, selectedXcode }; - } catch (error) { - return { - version: 'Not available', - path: 'Not available', - selectedXcode: 'Not available', - error: error instanceof Error ? error.message : String(error), - }; - } -} - -function getEnvironmentVariables(): Record { - const relevantVars = [ - 'INCREMENTAL_BUILDS_ENABLED', - 'PATH', - 'DEVELOPER_DIR', - 'HOME', - 'USER', - 'TMPDIR', - 'NODE_ENV', - 'SENTRY_DISABLED', - ]; - - const envVars: Record = {}; - relevantVars.forEach((varName) => { - envVars[varName] = process.env[varName] ?? ''; - }); +const USER_HOME_PATH_PATTERN = /\/Users\/[^/\s]+/g; +const XCODE_VERSION_PATTERN = /^Xcode\s+(.+)$/m; +const XCODE_BUILD_PATTERN = /^Build version\s+(.+)$/m; +const SENTRY_SELF_TEST_ENV_VAR = 'XCODEBUILDMCP_SENTRY_SELFTEST'; + +export type SentryRuntimeMode = 'mcp' | 'cli-daemon' | 'cli'; +export type SentryToolRuntime = 'cli' | 'daemon' | 'mcp'; +export type SentryToolTransport = 'direct' | 'daemon' | 'xcode-ide-daemon'; +export type SentryToolInvocationOutcome = 'completed' | 'infra_error'; +export type SentryDaemonLifecycleEvent = 'start' | 'shutdown' | 'crash'; +export type SentryXcodeBridgeSyncOutcome = 'success' | 'error' | 'empty'; + +export interface SentryRuntimeContext { + mode: SentryRuntimeMode; + xcodeAvailable?: boolean; + enabledWorkflows?: string[]; + disableSessionDefaults?: boolean; + disableXcodeAutoSync?: boolean; + incrementalBuildsEnabled?: boolean; + debugEnabled?: boolean; + uiDebuggerGuardMode?: string; + xcodeIdeWorkflowEnabled?: boolean; + axeAvailable?: boolean; + axeSource?: 'env' | 'bundled' | 'path' | 'unavailable'; + axeVersion?: string; + xcodeDeveloperDir?: string; + xcodebuildPath?: string; + xcodemakeAvailable?: boolean; + xcodemakeEnabled?: boolean; + xcodeVersion?: string; + xcodeBuildVersion?: string; +} + +function redactPathLikeData(value: string): string { + return value.replace(USER_HOME_PATH_PATTERN, '/Users/'); +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function redactUnknown(value: unknown): unknown { + if (typeof value === 'string') { + return redactPathLikeData(value); + } - Object.keys(process.env).forEach((key) => { - if (key.startsWith('XCODEBUILDMCP_')) { - envVars[key] = process.env[key] ?? ''; + if (Array.isArray(value)) { + return value.map((item) => redactUnknown(item)); + } + + if (isRecord(value)) { + const output: Record = {}; + for (const [key, nested] of Object.entries(value)) { + output[key] = redactUnknown(nested); } - }); + return output; + } - return envVars; + return value; } -function checkBinaryAvailability(binary: string): { available: boolean; version?: string } { - try { - execSync(`which ${binary}`, { stdio: 'ignore' }); - } catch { - return { available: false }; +function redactEvent(event: Sentry.ErrorEvent): Sentry.ErrorEvent { + // Remove default identity/request surfaces entirely. + delete event.user; + delete event.request; + delete event.breadcrumbs; + + if (typeof event.message === 'string') { + event.message = redactPathLikeData(event.message); } - let version: string | undefined; - const versionCommands: Record = { - axe: 'axe --version', - mise: 'mise --version', - }; + const exceptionValues = event.exception?.values ?? []; + for (const exceptionValue of exceptionValues) { + if (typeof exceptionValue.value === 'string') { + exceptionValue.value = redactPathLikeData(exceptionValue.value); + } - if (binary in versionCommands) { - try { - version = execSync(versionCommands[binary], { - encoding: 'utf8', - stdio: ['ignore', 'pipe', 'ignore'], - }).trim(); - } catch { - // Version command failed, but binary exists + const frames = exceptionValue.stacktrace?.frames ?? []; + for (const frame of frames) { + if (typeof frame.abs_path === 'string') { + frame.abs_path = redactPathLikeData(frame.abs_path); + } + if (typeof frame.filename === 'string') { + frame.filename = redactPathLikeData(frame.filename); + } } } - return { available: true, version }; + if (event.extra) { + for (const [key, value] of Object.entries(event.extra)) { + if (typeof value === 'string') { + event.extra[key] = redactPathLikeData(value); + } + } + } + + return event; +} + +function redactLog(log: Sentry.Log): Sentry.Log | null { + if (!log) { + return null; + } + + if (typeof log.message === 'string') { + log.message = redactPathLikeData(log.message); + } + + if (log.attributes !== undefined) { + log.attributes = redactUnknown(log.attributes) as Record; + } + + return log; +} + +export function __redactEventForTests(event: Sentry.Event): Sentry.Event { + const clone = structuredClone(event); + return redactEvent(clone as Sentry.ErrorEvent) as Sentry.Event; +} + +export function __redactLogForTests(log: Sentry.Log): Sentry.Log | null { + const clone = structuredClone(log); + return redactLog(clone); +} + +export function __parseXcodeVersionForTests(output: string): { + version?: string; + buildVersion?: string; +} { + const versionMatch = output.match(XCODE_VERSION_PATTERN); + const buildMatch = output.match(XCODE_BUILD_PATTERN); + return { + version: versionMatch?.[1]?.trim(), + buildVersion: buildMatch?.[1]?.trim(), + }; } let initialized = false; @@ -93,7 +157,150 @@ function isTestEnv(): boolean { return process.env.VITEST === 'true' || process.env.NODE_ENV === 'test'; } -export function initSentry(): void { +function isSentrySelfTestEnabled(): boolean { + const raw = process.env[SENTRY_SELF_TEST_ENV_VAR]?.trim().toLowerCase(); + return raw === '1' || raw === 'true' || raw === 'yes'; +} + +function emitSentrySelfTest(mode: SentryRuntimeMode | undefined): void { + if (!isSentrySelfTestEnabled()) { + return; + } + + const marker = new Date().toISOString(); + const attributes: Record = { + source: 'xcodebuildmcp.sentry_selftest', + marker, + runtime_mode: mode ?? 'unknown', + pid: process.pid, + }; + + Sentry.logger.info('XcodeBuildMCP Sentry self-test log', attributes); + Sentry.startSpan( + { + name: 'XcodeBuildMCP Sentry self-test transaction', + op: 'xcodebuildmcp.sentry_selftest', + forceTransaction: true, + attributes, + }, + () => undefined, + ); +} + +function boolToTag(value: boolean | undefined): string | undefined { + if (value === undefined) { + return undefined; + } + return value ? 'true' : 'false'; +} + +function setTagIfDefined(key: string, value: string | undefined): void { + if (!value) { + return; + } + Sentry.setTag(key, value); +} + +export function setSentryRuntimeContext(context: SentryRuntimeContext): void { + if (!initialized || isSentryDisabled() || isTestEnv()) { + return; + } + + setTagIfDefined('runtime.mode', context.mode); + setTagIfDefined('xcode.available', boolToTag(context.xcodeAvailable)); + setTagIfDefined('config.disable_session_defaults', boolToTag(context.disableSessionDefaults)); + setTagIfDefined('config.disable_xcode_auto_sync', boolToTag(context.disableXcodeAutoSync)); + setTagIfDefined('config.incremental_builds_enabled', boolToTag(context.incrementalBuildsEnabled)); + setTagIfDefined('config.debug_enabled', boolToTag(context.debugEnabled)); + setTagIfDefined('config.ui_debugger_guard_mode', context.uiDebuggerGuardMode); + setTagIfDefined('config.xcode_ide_workflow_enabled', boolToTag(context.xcodeIdeWorkflowEnabled)); + setTagIfDefined('axe.available', boolToTag(context.axeAvailable)); + setTagIfDefined('axe.source', context.axeSource); + setTagIfDefined('axe.path_mode', context.axeSource); + setTagIfDefined('axe.version', context.axeVersion); + setTagIfDefined('xcode.developer_dir', context.xcodeDeveloperDir); + setTagIfDefined('xcode.xcodebuild_path', context.xcodebuildPath); + setTagIfDefined('xcodemake.available', boolToTag(context.xcodemakeAvailable)); + setTagIfDefined('xcodemake.enabled', boolToTag(context.xcodemakeEnabled)); + setTagIfDefined('xcode.version', context.xcodeVersion); + setTagIfDefined('xcode.build_version', context.xcodeBuildVersion); + + if (context.enabledWorkflows) { + Sentry.setTag('config.workflow_count', String(context.enabledWorkflows.length)); + Sentry.setContext('xcodebuildmcp.runtime', { + enabledWorkflows: context.enabledWorkflows.join(','), + }); + } +} + +export async function getXcodeVersionMetadata( + runCommand: (command: string[]) => Promise<{ success: boolean; output: string }>, +): Promise<{ + version?: string; + buildVersion?: string; + developerDir?: string; + xcodebuildPath?: string; +}> { + const metadata: { + version?: string; + buildVersion?: string; + developerDir?: string; + xcodebuildPath?: string; + } = {}; + + try { + const result = await runCommand(['xcodebuild', '-version']); + if (result.success) { + const parsed = __parseXcodeVersionForTests(result.output); + metadata.version = parsed.version; + metadata.buildVersion = parsed.buildVersion; + } + } catch { + // ignore + } + + try { + const result = await runCommand(['xcode-select', '-p']); + if (result.success) { + metadata.developerDir = result.output.trim(); + } + } catch { + // ignore + } + + try { + const result = await runCommand(['xcrun', '--find', 'xcodebuild']); + if (result.success) { + metadata.xcodebuildPath = result.output.trim(); + } + } catch { + // ignore + } + + return metadata; +} + +export async function getAxeVersionMetadata( + runCommand: (command: string[]) => Promise<{ success: boolean; output: string }>, + axePath: string | undefined, +): Promise { + if (!axePath) { + return undefined; + } + + try { + const result = await runCommand([axePath, '--version']); + if (!result.success) { + return undefined; + } + const versionLine = result.output.trim().split('\n')[0]?.trim(); + return versionLine || undefined; + } catch { + return undefined; + } +} + +export function initSentry(context?: Pick): void { if (initialized || isSentryDisabled() || isTestEnv()) { return; } @@ -103,41 +310,185 @@ export function initSentry(): void { Sentry.init({ dsn: process.env.SENTRY_DSN ?? - 'https://798607831167c7b9fe2f2912f5d3c665@o4509258288332800.ingest.de.sentry.io/4509258293837904', - - // Setting this option to true will send default PII data to Sentry - // For example, automatic IP address collection on events - sendDefaultPii: true, + 'https://326e2c19ee84f3b2c892207b5726cde0@o1.ingest.us.sentry.io/4510869777416192', - // Set release version to match application version + // Keep Sentry defaults as lean as possible for privacy: internal failures only. + sendDefaultPii: false, + tracesSampleRate: 1.0, + enableLogs: true, + _experiments: { + enableMetrics: true, + }, + maxBreadcrumbs: 0, + beforeBreadcrumb: () => null, + beforeSend: redactEvent, + beforeSendLog: redactLog, + serverName: '', release: `xcodebuildmcp@${version}`, - - // Always report under production environment environment: 'production', - - // Set tracesSampleRate to 1.0 to capture 100% of transactions for performance monitoring - // We recommend adjusting this value in production - tracesSampleRate: 1.0, }); - const axeAvailable = checkBinaryAvailability('axe'); - const miseAvailable = checkBinaryAvailability('mise'); - const envVars = getEnvironmentVariables(); - const xcodeInfo = getXcodeInfo(); - - // Add additional context that might be helpful for debugging - const tags: Record = { - nodeVersion: process.version, - platform: process.platform, - arch: process.arch, - axeAvailable: axeAvailable.available ? 'true' : 'false', - axeVersion: axeAvailable.version ?? 'Unknown', - miseAvailable: miseAvailable.available ? 'true' : 'false', - miseVersion: miseAvailable.version ?? 'Unknown', - ...Object.fromEntries(Object.entries(envVars).map(([k, v]) => [`env_${k}`, v ?? ''])), - xcodeVersion: xcodeInfo.version ?? 'Unknown', - xcodePath: xcodeInfo.path ?? 'Unknown', + if (context?.mode) { + Sentry.setTag('runtime.mode', context.mode); + } + + emitSentrySelfTest(context?.mode); +} + +export async function flushAndCloseSentry(timeoutMs = 2000): Promise { + if (!initialized || isSentryDisabled() || isTestEnv()) { + return; + } + + try { + await Sentry.close(timeoutMs); + } catch { + // Best effort during shutdown. + } +} + +interface ToolInvocationMetric { + toolName: string; + runtime: SentryToolRuntime; + transport: SentryToolTransport; + outcome: SentryToolInvocationOutcome; + durationMs: number; +} + +interface InternalErrorMetric { + component: string; + runtime: SentryToolRuntime; + errorKind: string; +} + +type DaemonGaugeMetricName = 'inflight_requests' | 'active_sessions' | 'idle_timeout_ms'; + +function sanitizeTagValue(value: string): string { + const trimmed = value.trim().toLowerCase(); + if (!trimmed) { + return 'unknown'; + } + return trimmed.replace(/[^a-z0-9._-]/g, '_').slice(0, 64); +} + +function shouldEmitMetrics(): boolean { + return initialized && !isSentryDisabled() && !isTestEnv(); +} + +export function recordToolInvocationMetric(metric: ToolInvocationMetric): void { + if (!shouldEmitMetrics()) { + return; + } + + const tags = { + tool_name: sanitizeTagValue(metric.toolName), + runtime: metric.runtime, + transport: metric.transport, + outcome: metric.outcome, }; - Sentry.setTags(tags); + try { + Sentry.metrics.count('xcodebuildmcp.tool.invocation.count', 1, { attributes: tags }); + Sentry.metrics.distribution( + 'xcodebuildmcp.tool.invocation.duration_ms', + Math.max(0, metric.durationMs), + { attributes: tags }, + ); + } catch { + // Metrics are best effort and must never affect tool execution. + } +} + +export function recordInternalErrorMetric(metric: InternalErrorMetric): void { + if (!shouldEmitMetrics()) { + return; + } + + try { + Sentry.metrics.count('xcodebuildmcp.internal_error.count', 1, { + attributes: { + component: sanitizeTagValue(metric.component), + runtime: metric.runtime, + error_kind: sanitizeTagValue(metric.errorKind), + }, + }); + } catch { + // Metrics are best effort and must never affect runtime behavior. + } +} + +export function recordDaemonLifecycleMetric(event: SentryDaemonLifecycleEvent): void { + if (!shouldEmitMetrics()) { + return; + } + + try { + Sentry.metrics.count(`xcodebuildmcp.daemon.${event}.count`, 1, { + attributes: { + runtime: 'daemon', + }, + }); + } catch { + // Metrics are best effort and must never affect runtime behavior. + } +} + +export function recordBootstrapDurationMetric( + runtime: SentryRuntimeMode, + durationMs: number, +): void { + if (!shouldEmitMetrics()) { + return; + } + + try { + Sentry.metrics.distribution('xcodebuildmcp.bootstrap.duration_ms', Math.max(0, durationMs), { + attributes: { + runtime, + }, + }); + } catch { + // Metrics are best effort and must never affect runtime behavior. + } +} + +export function recordXcodeBridgeSyncDurationMetric( + durationMs: number, + outcome: SentryXcodeBridgeSyncOutcome, +): void { + if (!shouldEmitMetrics()) { + return; + } + + try { + Sentry.metrics.distribution( + 'xcodebuildmcp.xcode_bridge.sync.duration_ms', + Math.max(0, durationMs), + { + attributes: { + runtime: 'mcp', + outcome, + }, + }, + ); + } catch { + // Metrics are best effort and must never affect runtime behavior. + } +} + +export function recordDaemonGaugeMetric(metricName: DaemonGaugeMetricName, value: number): void { + if (!shouldEmitMetrics()) { + return; + } + + const normalizedValue = Number.isFinite(value) ? Math.max(0, value) : 0; + try { + Sentry.metrics.gauge(`xcodebuildmcp.daemon.${metricName}`, normalizedValue, { + attributes: { + runtime: 'daemon', + }, + }); + } catch { + // Metrics are best effort and must never affect runtime behavior. + } } diff --git a/src/utils/tool-registry.ts b/src/utils/tool-registry.ts index 527d68ab..e62fab9d 100644 --- a/src/utils/tool-registry.ts +++ b/src/utils/tool-registry.ts @@ -8,6 +8,7 @@ import { importToolModule } from '../core/manifest/import-tool-module.ts'; import type { PredicateContext } from '../visibility/predicate-types.ts'; import { selectWorkflowsForMcp, isToolExposedForRuntime } from '../visibility/exposure.ts'; import { getConfig } from './config-store.ts'; +import { recordInternalErrorMetric, recordToolInvocationMetric } from './sentry.ts'; export interface RuntimeToolInfo { enabledWorkflows: string[]; @@ -114,8 +115,32 @@ export async function applyWorkflowSelectionFromManifest( annotations: toolManifest.annotations, }, async (args: unknown): Promise => { - const response = await toolModule.handler(args as Record); - return processToolResponse(response as ToolResponse, 'mcp', 'normal'); + const startedAt = Date.now(); + try { + const response = await toolModule.handler(args as Record); + recordToolInvocationMetric({ + toolName, + runtime: 'mcp', + transport: 'direct', + outcome: 'completed', + durationMs: Date.now() - startedAt, + }); + return processToolResponse(response as ToolResponse, 'mcp', 'normal'); + } catch (error) { + recordInternalErrorMetric({ + component: 'mcp-tool-registry', + runtime: 'mcp', + errorKind: error instanceof Error ? error.name || 'Error' : typeof error, + }); + recordToolInvocationMetric({ + toolName, + runtime: 'mcp', + transport: 'direct', + outcome: 'infra_error', + durationMs: Date.now() - startedAt, + }); + throw error; + } }, ); registryState.tools.set(toolName, registeredTool); diff --git a/src/utils/xcodemake.ts b/src/utils/xcodemake.ts index d3c03be7..d813581b 100644 --- a/src/utils/xcodemake.ts +++ b/src/utils/xcodemake.ts @@ -16,7 +16,7 @@ import { log } from './logger.ts'; import type { CommandResponse } from './command.ts'; import { getDefaultCommandExecutor } from './command.ts'; -import { existsSync, readdirSync } from 'fs'; +import { existsSync, readdirSync, statSync } from 'fs'; import * as path from 'path'; import * as os from 'os'; import * as fs from 'fs/promises'; @@ -44,6 +44,36 @@ function getXcodemakeCommand(): string { return overriddenXcodemakePath ?? 'xcodemake'; } +function isExecutable(path: string): boolean { + try { + const stat = statSync(path); + return stat.isFile() && (stat.mode & 0o111) !== 0; + } catch { + return false; + } +} + +/** + * Check whether xcodemake binary is currently resolvable without side effects. + * This does not attempt download or installation. + */ +export function isXcodemakeBinaryAvailable(): boolean { + if (overriddenXcodemakePath && isExecutable(overriddenXcodemakePath)) { + return true; + } + + const pathValue = process.env.PATH ?? ''; + const entries = pathValue.split(path.delimiter).filter(Boolean); + for (const entry of entries) { + const candidate = path.join(entry, 'xcodemake'); + if (isExecutable(candidate)) { + return true; + } + } + + return false; +} + /** * Override the xcodemake command path * @param path Path to the xcodemake executable diff --git a/src/utils/xcodemake/index.ts b/src/utils/xcodemake/index.ts index cb24f210..c115b16f 100644 --- a/src/utils/xcodemake/index.ts +++ b/src/utils/xcodemake/index.ts @@ -1 +1,6 @@ -export { isXcodemakeEnabled, isXcodemakeAvailable, doesMakefileExist } from '../xcodemake.ts'; +export { + isXcodemakeEnabled, + isXcodemakeAvailable, + isXcodemakeBinaryAvailable, + doesMakefileExist, +} from '../xcodemake.ts'; From 8359a698481777c98da04a1419347e461faac24e Mon Sep 17 00:00:00 2001 From: Cameron Cooke Date: Fri, 13 Feb 2026 11:05:03 +0000 Subject: [PATCH 2/2] chore: Commit all pending workspace changes Capture all remaining updates in the current worktree, including\nworkflow/config/script adjustments and code simplifications across\nCLI, daemon, runtime, server, and utility modules. --- .github/ISSUE_TEMPLATE/bug_report.yml | 4 +- .github/ISSUE_TEMPLATE/config.yml | 2 +- .github/ISSUE_TEMPLATE/feature_request.yml | 10 +- .github/workflows/ci.yml | 50 +- .github/workflows/sentry.yml | 4 +- .github/workflows/stale.yml | 12 +- XcodeBuildMCP.code-workspace | 24 +- config.example.yaml | 24 +- eslint.config.js | 111 +++-- .../iOS/.xcodebuildmcp/config.yaml | 2 +- scripts/check-code-patterns.js | 471 ++++++++++++------ scripts/copy-build-assets.js | 6 +- scripts/generate-github-release-notes.mjs | 13 +- scripts/probe-xcode-mcpbridge.ts | 5 +- scripts/update-tools-docs.ts | 12 +- src/cli.ts | 4 +- src/core/resources.ts | 11 +- src/daemon.ts | 80 ++- .../fixtures/fake-xcode-tools-server.mjs | 1 - src/runtime/tool-invoker.ts | 227 ++++----- src/server/bootstrap.ts | 46 +- src/server/server.ts | 2 - src/utils/logger.ts | 27 +- src/utils/sentry.ts | 30 +- src/utils/tool-registry.ts | 31 +- src/utils/xcodemake.ts | 11 +- tsup.config.ts | 2 +- vitest.smoke.config.ts | 4 +- 28 files changed, 680 insertions(+), 546 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 2a55b6c7..33f3670f 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -1,7 +1,7 @@ name: Bug Report description: Report a bug or issue with XcodeBuildMCP -title: "[Bug]: " -labels: ["bug"] +title: '[Bug]: ' +labels: ['bug'] body: - type: markdown attributes: diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index ec4bb386..3ba13e0c 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1 +1 @@ -blank_issues_enabled: false \ No newline at end of file +blank_issues_enabled: false diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml index 109bf712..5f4d148d 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.yml +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -1,13 +1,13 @@ name: Feature Request description: Suggest a new feature for XcodeBuildMCP -title: "[Feature]: " -labels: ["enhancement"] +title: '[Feature]: ' +labels: ['enhancement'] body: - type: markdown attributes: value: | Thanks for suggesting a new feature for XcodeBuildMCP! - + - type: textarea id: feature-description attributes: @@ -16,7 +16,7 @@ body: placeholder: I would like the AI assistant to be able to... validations: required: true - + - type: textarea id: use-cases attributes: @@ -28,7 +28,7 @@ body: - Automating complex Xcode workflows validations: required: false - + - type: textarea id: example-interactions attributes: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 26dc83b9..9a5bfe31 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,9 +2,9 @@ name: CI on: push: - branches: [ main ] + branches: [main] pull_request: - branches: [ main ] + branches: [main] jobs: build-and-test: @@ -15,36 +15,36 @@ jobs: node-version: [24.x] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v3 - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v3 - with: - node-version: ${{ matrix.node-version }} - cache: 'npm' + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v3 + with: + node-version: ${{ matrix.node-version }} + cache: 'npm' - - name: Install dependencies - run: npm ci + - name: Install dependencies + run: npm ci - - name: Bundle AXe artifacts - run: npm run bundle:axe + - name: Bundle AXe artifacts + run: npm run bundle:axe - - name: Build - run: npm run build + - name: Build + run: npm run build - - name: Validate docs CLI command references - run: npm run docs:check + - name: Validate docs CLI command references + run: npm run docs:check - - name: Lint - run: npm run lint + - name: Lint + run: npm run lint - - name: Check formatting - run: npm run format:check + - name: Check formatting + run: npm run format:check - - name: Type check - run: npm run typecheck + - name: Type check + run: npm run typecheck - - name: Run tests - run: npm test + - name: Run tests + run: npm test - - run: npx pkg-pr-new publish + - run: npx pkg-pr-new publish diff --git a/.github/workflows/sentry.yml b/.github/workflows/sentry.yml index d88f57ed..cd14ac86 100644 --- a/.github/workflows/sentry.yml +++ b/.github/workflows/sentry.yml @@ -32,5 +32,5 @@ jobs: SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }} with: environment: production - sourcemaps: "./build" - version: ${{ steps.get_version.outputs.MCP_VERSION }} \ No newline at end of file + sourcemaps: './build' + version: ${{ steps.get_version.outputs.MCP_VERSION }} diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index c2646032..f0864d37 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -1,8 +1,8 @@ -name: "Stale Issues and PRs" +name: 'Stale Issues and PRs' on: schedule: - - cron: "30 3 * * *" + - cron: '30 3 * * *' workflow_dispatch: {} permissions: @@ -21,12 +21,12 @@ jobs: days-before-issue-close: 7 days-before-pr-stale: 21 days-before-pr-close: 7 - stale-issue-label: "stale" - stale-pr-label: "stale-pr" + stale-issue-label: 'stale' + stale-pr-label: 'stale-pr' exempt-issue-assignees: true exempt-pr-assignees: true - exempt-issue-labels: "no-stale,security,pinned" - exempt-pr-labels: "no-stale,security,pinned" + exempt-issue-labels: 'no-stale,security,pinned' + exempt-pr-labels: 'no-stale,security,pinned' stale-issue-message: > This issue has been inactive for 30 days. It will be closed in 7 days if no further activity occurs. Add a comment to keep it open, or apply diff --git a/XcodeBuildMCP.code-workspace b/XcodeBuildMCP.code-workspace index d6f91c86..54025ef2 100644 --- a/XcodeBuildMCP.code-workspace +++ b/XcodeBuildMCP.code-workspace @@ -1,13 +1,13 @@ { - "folders": [ - { - "path": "." - }, - { - "path": "../XcodeBuildMCP-iOS-Template" - }, - { - "path": "../XcodeBuildMCP-macOS-Template" - } - ] -} \ No newline at end of file + "folders": [ + { + "path": ".", + }, + { + "path": "../XcodeBuildMCP-iOS-Template", + }, + { + "path": "../XcodeBuildMCP-macOS-Template", + }, + ], +} diff --git a/config.example.yaml b/config.example.yaml index 7abb4b68..60df0c0a 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -1,20 +1,20 @@ schemaVersion: 1 -enabledWorkflows: ["simulator", "ui-automation", "debugging"] +enabledWorkflows: ['simulator', 'ui-automation', 'debugging'] experimentalWorkflowDiscovery: false disableSessionDefaults: false incrementalBuildsEnabled: false sessionDefaults: - projectPath: "./MyApp.xcodeproj" # xor workspacePath - workspacePath: "./MyApp.xcworkspace" # xor projectPath - scheme: "MyApp" - configuration: "Debug" - simulatorName: "iPhone 16" # xor simulatorId - simulatorId: "" # xor simulatorName - deviceId: "" + projectPath: './MyApp.xcodeproj' # xor workspacePath + workspacePath: './MyApp.xcworkspace' # xor projectPath + scheme: 'MyApp' + configuration: 'Debug' + simulatorName: 'iPhone 16' # xor simulatorId + simulatorId: '' # xor simulatorName + deviceId: '' useLatestOS: true - arch: "arm64" + arch: 'arm64' suppressWarnings: false - derivedDataPath: "./.derivedData" + derivedDataPath: './.derivedData' preferXcodebuild: false - platform: "iOS" - bundleId: "io.sentry.myapp" + platform: 'iOS' + bundleId: 'io.sentry.myapp' diff --git a/eslint.config.js b/eslint.config.js index 26677ec3..0785a57a 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -6,7 +6,14 @@ export default [ eslint.configs.recommended, ...tseslint.configs.recommended, { - ignores: ['node_modules/**', 'build/**', 'dist/**', 'coverage/**', 'src/core/generated-plugins.ts', 'src/core/generated-resources.ts'], + ignores: [ + 'node_modules/**', + 'build/**', + 'dist/**', + 'coverage/**', + 'src/core/generated-plugins.ts', + 'src/core/generated-resources.ts', + ], }, { // TypeScript files in src/ directory (covered by tsconfig.json) @@ -21,55 +28,78 @@ export default [ }, plugins: { '@typescript-eslint': tseslint.plugin, - 'prettier': prettierPlugin, + prettier: prettierPlugin, }, rules: { 'prettier/prettier': 'error', '@typescript-eslint/explicit-function-return-type': 'warn', '@typescript-eslint/no-explicit-any': 'error', - '@typescript-eslint/no-unused-vars': ['error', { - argsIgnorePattern: 'never', - varsIgnorePattern: 'never' - }], + '@typescript-eslint/no-unused-vars': [ + 'error', + { + argsIgnorePattern: 'never', + varsIgnorePattern: 'never', + }, + ], 'no-console': ['warn', { allow: ['error'] }], - + // Prevent dangerous type casting anti-patterns (errors) - '@typescript-eslint/consistent-type-assertions': ['error', { - assertionStyle: 'as', - objectLiteralTypeAssertions: 'never' - }], + '@typescript-eslint/consistent-type-assertions': [ + 'error', + { + assertionStyle: 'as', + objectLiteralTypeAssertions: 'never', + }, + ], '@typescript-eslint/no-unsafe-argument': 'error', '@typescript-eslint/no-unsafe-assignment': 'error', '@typescript-eslint/no-unsafe-call': 'error', '@typescript-eslint/no-unsafe-member-access': 'error', '@typescript-eslint/no-unsafe-return': 'error', - + // Prevent specific anti-patterns we found - '@typescript-eslint/ban-ts-comment': ['error', { - 'ts-expect-error': 'allow-with-description', - 'ts-ignore': true, - 'ts-nocheck': true, - 'ts-check': false, - }], - + '@typescript-eslint/ban-ts-comment': [ + 'error', + { + 'ts-expect-error': 'allow-with-description', + 'ts-ignore': true, + 'ts-nocheck': true, + 'ts-check': false, + }, + ], + // Encourage best practices (warnings - can be gradually fixed) '@typescript-eslint/prefer-as-const': 'warn', '@typescript-eslint/prefer-nullish-coalescing': 'warn', '@typescript-eslint/prefer-optional-chain': 'warn', - + // Prevent barrel imports to maintain architectural improvements - 'no-restricted-imports': ['error', { - patterns: [ - { - group: ['**/utils/index.js', '../utils/index.js', '../../utils/index.js', '../../../utils/index.js', '**/utils/index.ts', '../utils/index.ts', '../../utils/index.ts', '../../../utils/index.ts'], - message: 'Barrel imports from utils/index are prohibited. Use focused facade imports instead (e.g., utils/logging/index.ts, utils/execution/index.ts).' - }, - { - group: ['./**/*.js', '../**/*.js'], - message: 'Import TypeScript files with .ts extension, not .js. This ensures compatibility with native TypeScript runtimes like Bun and Deno. Change .js to .ts in your import path.' - } - ] - }], + 'no-restricted-imports': [ + 'error', + { + patterns: [ + { + group: [ + '**/utils/index.js', + '../utils/index.js', + '../../utils/index.js', + '../../../utils/index.js', + '**/utils/index.ts', + '../utils/index.ts', + '../../utils/index.ts', + '../../../utils/index.ts', + ], + message: + 'Barrel imports from utils/index are prohibited. Use focused facade imports instead (e.g., utils/logging/index.ts, utils/execution/index.ts).', + }, + { + group: ['./**/*.js', '../**/*.js'], + message: + 'Import TypeScript files with .ts extension, not .js. This ensures compatibility with native TypeScript runtimes like Bun and Deno. Change .js to .ts in your import path.', + }, + ], + }, + ], }, }, { @@ -84,19 +114,22 @@ export default [ }, plugins: { '@typescript-eslint': tseslint.plugin, - 'prettier': prettierPlugin, + prettier: prettierPlugin, }, rules: { 'prettier/prettier': 'error', // Relaxed TypeScript rules for scripts since they're not in the main project '@typescript-eslint/explicit-function-return-type': 'off', '@typescript-eslint/no-explicit-any': 'warn', - '@typescript-eslint/no-unused-vars': ['warn', { - argsIgnorePattern: 'never', - varsIgnorePattern: 'never' - }], + '@typescript-eslint/no-unused-vars': [ + 'warn', + { + argsIgnorePattern: 'never', + varsIgnorePattern: 'never', + }, + ], 'no-console': 'off', // Scripts are allowed to use console - + // Disable project-dependent rules for scripts '@typescript-eslint/no-unsafe-argument': 'off', '@typescript-eslint/no-unsafe-assignment': 'off', @@ -120,7 +153,7 @@ export default [ '@typescript-eslint/no-unused-vars': 'off', '@typescript-eslint/explicit-function-return-type': 'off', 'prefer-const': 'off', - + // Relax unsafe rules for tests - tests often need more flexibility '@typescript-eslint/no-unsafe-argument': 'off', '@typescript-eslint/no-unsafe-assignment': 'off', diff --git a/example_projects/iOS/.xcodebuildmcp/config.yaml b/example_projects/iOS/.xcodebuildmcp/config.yaml index 8edf3397..568d5e4d 100644 --- a/example_projects/iOS/.xcodebuildmcp/config.yaml +++ b/example_projects/iOS/.xcodebuildmcp/config.yaml @@ -1,5 +1,5 @@ schemaVersion: 1 -enabledWorkflows: ["simulator", "ui-automation", "debugging", "logging"] +enabledWorkflows: ['simulator', 'ui-automation', 'debugging', 'logging'] sessionDefaults: projectPath: ./MCPTest.xcodeproj scheme: MCPTest diff --git a/scripts/check-code-patterns.js b/scripts/check-code-patterns.js index 2f896766..06be5c05 100755 --- a/scripts/check-code-patterns.js +++ b/scripts/check-code-patterns.js @@ -2,23 +2,23 @@ /** * XcodeBuildMCP Code Pattern Violations Checker - * + * * Validates that all code files follow XcodeBuildMCP-specific architectural patterns. * This script focuses on domain-specific rules that ESLint cannot express. - * + * * USAGE: * node scripts/check-code-patterns.js [--pattern=vitest|execsync|handler|handler-testing|all] * node scripts/check-code-patterns.js --help - * + * * ARCHITECTURAL RULES ENFORCED: * 1. NO vitest mocking patterns (vi.mock, vi.fn, .mockResolvedValue, etc.) * 2. NO execSync usage in production code (use CommandExecutor instead) * 3. ONLY dependency injection with createMockExecutor() and createMockFileSystemExecutor() * 4. NO handler signature violations (handlers must have exact MCP SDK signatures) * 5. NO handler testing violations (test logic functions, not handlers directly) - * + * * For comprehensive code quality documentation, see docs/dev/CODE_QUALITY.md - * + * * Note: General code quality rules (TypeScript, ESLint) are handled by other tools. * This script only enforces XcodeBuildMCP-specific architectural patterns. */ @@ -34,7 +34,7 @@ const projectRoot = join(__dirname, '..'); // Parse command line arguments const args = process.argv.slice(2); -const patternFilter = args.find(arg => arg.startsWith('--pattern='))?.split('=')[1] || 'all'; +const patternFilter = args.find((arg) => arg.startsWith('--pattern='))?.split('=')[1] || 'all'; const showHelp = args.includes('--help') || args.includes('-h'); if (showHelp) { @@ -71,33 +71,33 @@ EXAMPLES: // Patterns for execSync usage in production code (FORBIDDEN) // Note: execSync is allowed in test files for mocking, but not in production code const EXECSYNC_PATTERNS = [ - /\bexecSync\s*\(/, // Direct execSync usage - /\bexecSyncFn\s*[=:]/, // execSyncFn parameter or assignment - /^import\s+(?!type\s)[^}]*from\s+['"]child_process['"]/m, // Importing from child_process (except type-only imports) + /\bexecSync\s*\(/, // Direct execSync usage + /\bexecSyncFn\s*[=:]/, // execSyncFn parameter or assignment + /^import\s+(?!type\s)[^}]*from\s+['"]child_process['"]/m, // Importing from child_process (except type-only imports) /^import\s+{[^}]*(?:exec|spawn|execSync)[^}]*}\s+from\s+['"](?:node:)?child_process['"]/m, // Named imports of functions ]; // CRITICAL: ALL VITEST MOCKING PATTERNS ARE COMPLETELY FORBIDDEN // ONLY dependency injection with approved mock utilities is allowed const VITEST_GENERIC_PATTERNS = [ - /vi\.mock\s*\(/, // vi.mock() - BANNED - /vi\.fn\s*\(/, // vi.fn() - BANNED - /vi\.mocked\s*\(/, // vi.mocked() - BANNED - /vi\.spyOn\s*\(/, // vi.spyOn() - BANNED - /vi\.clearAllMocks\s*\(/, // vi.clearAllMocks() - BANNED - /\.mockResolvedValue/, // .mockResolvedValue - BANNED - /\.mockRejectedValue/, // .mockRejectedValue - BANNED - /\.mockReturnValue/, // .mockReturnValue - BANNED - /\.mockImplementation/, // .mockImplementation - BANNED - /\.mockClear/, // .mockClear - BANNED - /\.mockReset/, // .mockReset - BANNED - /\.mockRestore/, // .mockRestore - BANNED - /\.toHaveBeenCalled/, // .toHaveBeenCalled - BANNED - /\.toHaveBeenCalledWith/, // .toHaveBeenCalledWith - BANNED - /MockedFunction/, // MockedFunction type - BANNED - /as MockedFunction/, // Type casting to MockedFunction - BANNED - /\bexecSync\b/, // execSync usage - BANNED (use executeCommand instead) - /\bexecSyncFn\b/, // execSyncFn usage - BANNED (use executeCommand instead) + /vi\.mock\s*\(/, // vi.mock() - BANNED + /vi\.fn\s*\(/, // vi.fn() - BANNED + /vi\.mocked\s*\(/, // vi.mocked() - BANNED + /vi\.spyOn\s*\(/, // vi.spyOn() - BANNED + /vi\.clearAllMocks\s*\(/, // vi.clearAllMocks() - BANNED + /\.mockResolvedValue/, // .mockResolvedValue - BANNED + /\.mockRejectedValue/, // .mockRejectedValue - BANNED + /\.mockReturnValue/, // .mockReturnValue - BANNED + /\.mockImplementation/, // .mockImplementation - BANNED + /\.mockClear/, // .mockClear - BANNED + /\.mockReset/, // .mockReset - BANNED + /\.mockRestore/, // .mockRestore - BANNED + /\.toHaveBeenCalled/, // .toHaveBeenCalled - BANNED + /\.toHaveBeenCalledWith/, // .toHaveBeenCalledWith - BANNED + /MockedFunction/, // MockedFunction type - BANNED + /as MockedFunction/, // Type casting to MockedFunction - BANNED + /\bexecSync\b/, // execSync usage - BANNED (use executeCommand instead) + /\bexecSyncFn\b/, // execSyncFn usage - BANNED (use executeCommand instead) ]; // APPROVED mock utilities - ONLY these are allowed @@ -114,7 +114,7 @@ const APPROVED_MOCK_PATTERNS = [ // Manual executors and mock objects are APPROVED when used for dependency injection const UNAPPROVED_MOCK_PATTERNS = [ // ONLY ACTUAL VITEST PATTERNS (vi.* usage) - Everything else is approved - /\bmock[A-Z][a-zA-Z0-9]*\s*=\s*vi\./, // mockSomething = vi.fn() - vitest assignments only + /\bmock[A-Z][a-zA-Z0-9]*\s*=\s*vi\./, // mockSomething = vi.fn() - vitest assignments only // No other patterns - manual executors and mock objects are approved for dependency injection ]; @@ -122,13 +122,13 @@ const UNAPPROVED_MOCK_PATTERNS = [ // Function to check if a line contains unapproved mock patterns function hasUnapprovedMockPattern(line) { // Skip lines that contain approved patterns - const hasApprovedPattern = APPROVED_MOCK_PATTERNS.some(pattern => pattern.test(line)); + const hasApprovedPattern = APPROVED_MOCK_PATTERNS.some((pattern) => pattern.test(line)); if (hasApprovedPattern) { return false; } // Check for unapproved patterns - return UNAPPROVED_MOCK_PATTERNS.some(pattern => pattern.test(line)); + return UNAPPROVED_MOCK_PATTERNS.some((pattern) => pattern.test(line)); } // Combined pattern checker for backward compatibility @@ -136,47 +136,47 @@ const VITEST_MOCKING_PATTERNS = VITEST_GENERIC_PATTERNS; // CRITICAL: ARCHITECTURAL VIOLATIONS - Utilities bypassing CommandExecutor (BANNED) const UTILITY_BYPASS_PATTERNS = [ - /spawn\s*\(/, // Direct Node.js spawn usage in utilities - BANNED - /exec\s*\(/, // Direct Node.js exec usage in utilities - BANNED - /execSync\s*\(/, // Direct Node.js execSync usage in utilities - BANNED - /child_process\./, // Direct child_process module usage in utilities - BANNED + /spawn\s*\(/, // Direct Node.js spawn usage in utilities - BANNED + /exec\s*\(/, // Direct Node.js exec usage in utilities - BANNED + /execSync\s*\(/, // Direct Node.js execSync usage in utilities - BANNED + /child_process\./, // Direct child_process module usage in utilities - BANNED ]; // TypeScript patterns are now handled by ESLint - removed from domain-specific checks // ESLint has comprehensive TypeScript rules with proper test file exceptions // CRITICAL: HANDLER SIGNATURE VIOLATIONS ARE FORBIDDEN -// MCP SDK requires handlers to have exact signatures: +// MCP SDK requires handlers to have exact signatures: // Tools: (args: Record) => Promise // Resources: (uri: URL) => Promise<{ contents: Array<{ text: string }> }> const HANDLER_SIGNATURE_VIOLATIONS = [ - /async\s+handler\s*\([^)]*:\s*[^,)]+,\s*[^)]+\s*:/ms, // Handler with multiple parameters separated by comma - BANNED - /async\s+handler\s*\(\s*args\?\s*:/ms, // Handler with optional args parameter - BANNED (should be required) - /async\s+handler\s*\([^)]*,\s*[^)]*CommandExecutor/ms, // Handler with CommandExecutor parameter - BANNED + /async\s+handler\s*\([^)]*:\s*[^,)]+,\s*[^)]+\s*:/ms, // Handler with multiple parameters separated by comma - BANNED + /async\s+handler\s*\(\s*args\?\s*:/ms, // Handler with optional args parameter - BANNED (should be required) + /async\s+handler\s*\([^)]*,\s*[^)]*CommandExecutor/ms, // Handler with CommandExecutor parameter - BANNED /async\s+handler\s*\([^)]*,\s*[^)]*FileSystemExecutor/ms, // Handler with FileSystemExecutor parameter - BANNED - /async\s+handler\s*\([^)]*,\s*[^)]*Dependencies/ms, // Handler with Dependencies parameter - BANNED - /async\s+handler\s*\([^)]*,\s*[^)]*executor\s*:/ms, // Handler with executor parameter - BANNED - /async\s+handler\s*\([^)]*,\s*[^)]*dependencies\s*:/ms, // Handler with dependencies parameter - BANNED + /async\s+handler\s*\([^)]*,\s*[^)]*Dependencies/ms, // Handler with Dependencies parameter - BANNED + /async\s+handler\s*\([^)]*,\s*[^)]*executor\s*:/ms, // Handler with executor parameter - BANNED + /async\s+handler\s*\([^)]*,\s*[^)]*dependencies\s*:/ms, // Handler with dependencies parameter - BANNED ]; // CRITICAL: HANDLER TESTING IN TESTS IS FORBIDDEN // Tests must ONLY call logic functions with dependency injection, NEVER handlers directly // Handlers are thin wrappers for MCP SDK - testing them violates dependency injection architecture const HANDLER_TESTING_VIOLATIONS = [ - /\.handler\s*\(/, // Direct handler calls in tests - BANNED - /await\s+\w+\.handler\s*\(/, // Awaited handler calls - BANNED + /\.handler\s*\(/, // Direct handler calls in tests - BANNED + /await\s+\w+\.handler\s*\(/, // Awaited handler calls - BANNED /const\s+result\s*=\s*await\s+\w+\.handler/, // Handler result assignment - BANNED /expect\s*\(\s*await\s+\w+\.handler/, // Handler expectation calls - BANNED ]; -// CRITICAL: IMPROPER SERVER TYPING PATTERNS ARE FORBIDDEN +// CRITICAL: IMPROPER SERVER TYPING PATTERNS ARE FORBIDDEN // Server instances must use proper MCP SDK types, not generic Record casts const IMPROPER_SERVER_TYPING_VIOLATIONS = [ - /as Record.*server/, // Casting server to Record - BANNED - /server.*as Record/, // Casting server to Record - BANNED - /mcpServer\?\s*:\s*Record/, // Typing server as Record - BANNED - /server\.server\?\?\s*server.*as Record/, // Complex server casting - BANNED - /interface\s+MCPServerInterface\s*{/, // Custom MCP interfaces when SDK types exist - BANNED + /as Record.*server/, // Casting server to Record - BANNED + /server.*as Record/, // Casting server to Record - BANNED + /mcpServer\?\s*:\s*Record/, // Typing server as Record - BANNED + /server\.server\?\?\s*server.*as Record/, // Complex server casting - BANNED + /interface\s+MCPServerInterface\s*{/, // Custom MCP interfaces when SDK types exist - BANNED ]; // ALLOWED PATTERNS for cleanup (not mocking) @@ -186,8 +186,8 @@ const ALLOWED_CLEANUP_PATTERNS = [ // Patterns that indicate TRUE dependency injection approach const DEPENDENCY_INJECTION_PATTERNS = [ - /createMockExecutor/, // createMockExecutor usage - /createMockFileSystemExecutor/, // createMockFileSystemExecutor usage + /createMockExecutor/, // createMockExecutor usage + /createMockFileSystemExecutor/, // createMockFileSystemExecutor usage /executor\?\s*:\s*CommandExecutor/, // executor?: CommandExecutor parameter ]; @@ -203,7 +203,12 @@ function findTestFiles(dir) { if (stat.isDirectory()) { // Skip node_modules and other non-relevant directories - if (!item.startsWith('.') && item !== 'node_modules' && item !== 'dist' && item !== 'build') { + if ( + !item.startsWith('.') && + item !== 'node_modules' && + item !== 'dist' && + item !== 'build' + ) { traverse(fullPath); } } else if (item.endsWith('.test.ts') || item.endsWith('.test.js')) { @@ -228,10 +233,21 @@ function findToolAndResourceFiles(dir) { if (stat.isDirectory()) { // Skip test directories and other non-relevant directories - if (!item.startsWith('.') && item !== '__tests__' && item !== 'node_modules' && item !== 'dist' && item !== 'build') { + if ( + !item.startsWith('.') && + item !== '__tests__' && + item !== 'node_modules' && + item !== 'dist' && + item !== 'build' + ) { traverse(fullPath); } - } else if ((item.endsWith('.ts') || item.endsWith('.js')) && !item.includes('.test.') && item !== 'index.ts' && item !== 'index.js') { + } else if ( + (item.endsWith('.ts') || item.endsWith('.js')) && + !item.includes('.test.') && + item !== 'index.ts' && + item !== 'index.js' + ) { toolFiles.push(fullPath); } } @@ -253,17 +269,30 @@ function findUtilityFiles(dir) { if (stat.isDirectory()) { // Skip test directories and other non-relevant directories - if (!item.startsWith('.') && item !== '__tests__' && item !== 'node_modules' && item !== 'dist' && item !== 'build') { + if ( + !item.startsWith('.') && + item !== '__tests__' && + item !== 'node_modules' && + item !== 'dist' && + item !== 'build' + ) { traverse(fullPath); } - } else if ((item.endsWith('.ts') || item.endsWith('.js')) && !item.includes('.test.') && item !== 'index.ts' && item !== 'index.js') { + } else if ( + (item.endsWith('.ts') || item.endsWith('.js')) && + !item.includes('.test.') && + item !== 'index.ts' && + item !== 'index.js' + ) { // Only include key utility files that should use CommandExecutor // Exclude command.ts itself as it's the core implementation that is allowed to use spawn() - if (fullPath.includes('/utils/') && ( - fullPath.includes('log_capture.ts') || - fullPath.includes('build.ts') || - fullPath.includes('simctl.ts') - ) && !fullPath.includes('command.ts')) { + if ( + fullPath.includes('/utils/') && + (fullPath.includes('log_capture.ts') || + fullPath.includes('build.ts') || + fullPath.includes('simctl.ts')) && + !fullPath.includes('command.ts') + ) { utilityFiles.push(fullPath); } } @@ -276,7 +305,9 @@ function findUtilityFiles(dir) { // Helper function to determine if a file is a test file function isTestFile(filePath) { - return filePath.includes('__tests__') || filePath.endsWith('.test.ts') || filePath.endsWith('.test.js'); + return ( + filePath.includes('__tests__') || filePath.endsWith('.test.ts') || filePath.endsWith('.test.js') + ); } // Helper function to determine if a file is a production file @@ -287,8 +318,10 @@ function isProductionFile(filePath) { // Helper function to determine if a file is allowed to use child_process function isAllowedChildProcessFile(filePath) { // These files need direct child_process access for their core functionality - return filePath.includes('command.ts') || // Core CommandExecutor implementation - filePath.includes('swift_package_run.ts'); // Needs spawn for background process management + return ( + filePath.includes('command.ts') || // Core CommandExecutor implementation + filePath.includes('swift_package_run.ts') + ); // Needs spawn for background process management } function analyzeTestFile(filePath) { @@ -302,13 +335,13 @@ function analyzeTestFile(filePath) { // 1. Check generic vi.* patterns (always violations) lines.forEach((line, index) => { - VITEST_GENERIC_PATTERNS.forEach(pattern => { + VITEST_GENERIC_PATTERNS.forEach((pattern) => { if (pattern.test(line)) { vitestMockingDetails.push({ line: index + 1, content: line.trim(), pattern: pattern.source, - type: 'vitest-generic' + type: 'vitest-generic', }); } }); @@ -316,12 +349,12 @@ function analyzeTestFile(filePath) { // 2. Check for unapproved mock patterns if (hasUnapprovedMockPattern(line)) { // Find which specific pattern matched for better reporting - const matchedPattern = UNAPPROVED_MOCK_PATTERNS.find(pattern => pattern.test(line)); + const matchedPattern = UNAPPROVED_MOCK_PATTERNS.find((pattern) => pattern.test(line)); vitestMockingDetails.push({ line: index + 1, content: line.trim(), pattern: matchedPattern ? matchedPattern.source : 'unapproved mock pattern', - type: 'unapproved-mock' + type: 'unapproved-mock', }); } }); @@ -332,13 +365,17 @@ function analyzeTestFile(filePath) { const hasTypescriptAntipatterns = false; // Check for handler testing violations (FORBIDDEN - ARCHITECTURAL VIOLATION) - const hasHandlerTestingViolations = HANDLER_TESTING_VIOLATIONS.some(pattern => pattern.test(content)); + const hasHandlerTestingViolations = HANDLER_TESTING_VIOLATIONS.some((pattern) => + pattern.test(content), + ); // Check for improper server typing violations (FORBIDDEN - ARCHITECTURAL VIOLATION) - const hasImproperServerTypingViolations = IMPROPER_SERVER_TYPING_VIOLATIONS.some(pattern => pattern.test(content)); + const hasImproperServerTypingViolations = IMPROPER_SERVER_TYPING_VIOLATIONS.some((pattern) => + pattern.test(content), + ); // Check for dependency injection patterns (TRUE DI) - const hasDIPatterns = DEPENDENCY_INJECTION_PATTERNS.some(pattern => pattern.test(content)); + const hasDIPatterns = DEPENDENCY_INJECTION_PATTERNS.some((pattern) => pattern.test(content)); // Extract specific pattern occurrences for details const execSyncDetails = []; // Not applicable to test files @@ -347,25 +384,24 @@ function analyzeTestFile(filePath) { const improperServerTypingDetails = []; lines.forEach((line, index) => { - // TypeScript anti-patterns now handled by ESLint - removed from domain checks - HANDLER_TESTING_VIOLATIONS.forEach(pattern => { + HANDLER_TESTING_VIOLATIONS.forEach((pattern) => { if (pattern.test(line)) { handlerTestingDetails.push({ line: index + 1, content: line.trim(), - pattern: pattern.source + pattern: pattern.source, }); } }); - IMPROPER_SERVER_TYPING_VIOLATIONS.forEach(pattern => { + IMPROPER_SERVER_TYPING_VIOLATIONS.forEach((pattern) => { if (pattern.test(line)) { improperServerTypingDetails.push({ line: index + 1, content: line.trim(), - pattern: pattern.source + pattern: pattern.source, }); } }); @@ -384,9 +420,20 @@ function analyzeTestFile(filePath) { typescriptAntipatternDetails, handlerTestingDetails, improperServerTypingDetails, - needsConversion: hasVitestMockingPatterns || hasHandlerTestingViolations || hasImproperServerTypingViolations, - isConverted: hasDIPatterns && !hasVitestMockingPatterns && !hasHandlerTestingViolations && !hasImproperServerTypingViolations, - isMixed: (hasVitestMockingPatterns || hasHandlerTestingViolations || hasImproperServerTypingViolations) && hasDIPatterns + needsConversion: + hasVitestMockingPatterns || + hasHandlerTestingViolations || + hasImproperServerTypingViolations, + isConverted: + hasDIPatterns && + !hasVitestMockingPatterns && + !hasHandlerTestingViolations && + !hasImproperServerTypingViolations, + isMixed: + (hasVitestMockingPatterns || + hasHandlerTestingViolations || + hasImproperServerTypingViolations) && + hasDIPatterns, }; } catch (error) { console.error(`Error reading file ${filePath}: ${error.message}`); @@ -400,9 +447,10 @@ function analyzeToolOrResourceFile(filePath) { const relativePath = relative(projectRoot, filePath); // Check for execSync patterns in production code (excluding allowed files) - const hasExecSyncPatterns = isProductionFile(filePath) && + const hasExecSyncPatterns = + isProductionFile(filePath) && !isAllowedChildProcessFile(filePath) && - EXECSYNC_PATTERNS.some(pattern => pattern.test(content)); + EXECSYNC_PATTERNS.some((pattern) => pattern.test(content)); // Check for vitest mocking patterns using new robust approach const vitestMockingDetails = []; @@ -410,13 +458,13 @@ function analyzeToolOrResourceFile(filePath) { // 1. Check generic vi.* patterns (always violations) lines.forEach((line, index) => { - VITEST_GENERIC_PATTERNS.forEach(pattern => { + VITEST_GENERIC_PATTERNS.forEach((pattern) => { if (pattern.test(line)) { vitestMockingDetails.push({ line: index + 1, content: line.trim(), pattern: pattern.source, - type: 'vitest-generic' + type: 'vitest-generic', }); } }); @@ -424,12 +472,12 @@ function analyzeToolOrResourceFile(filePath) { // 2. Check for unapproved mock patterns if (hasUnapprovedMockPattern(line)) { // Find which specific pattern matched for better reporting - const matchedPattern = UNAPPROVED_MOCK_PATTERNS.find(pattern => pattern.test(line)); + const matchedPattern = UNAPPROVED_MOCK_PATTERNS.find((pattern) => pattern.test(line)); vitestMockingDetails.push({ line: index + 1, content: line.trim(), pattern: matchedPattern ? matchedPattern.source : 'unapproved mock pattern', - type: 'unapproved-mock' + type: 'unapproved-mock', }); } }); @@ -440,16 +488,22 @@ function analyzeToolOrResourceFile(filePath) { const hasTypescriptAntipatterns = false; // Check for dependency injection patterns (TRUE DI) - const hasDIPatterns = DEPENDENCY_INJECTION_PATTERNS.some(pattern => pattern.test(content)); + const hasDIPatterns = DEPENDENCY_INJECTION_PATTERNS.some((pattern) => pattern.test(content)); // Check for handler signature violations (FORBIDDEN) - const hasHandlerSignatureViolations = HANDLER_SIGNATURE_VIOLATIONS.some(pattern => pattern.test(content)); + const hasHandlerSignatureViolations = HANDLER_SIGNATURE_VIOLATIONS.some((pattern) => + pattern.test(content), + ); // Check for improper server typing violations (FORBIDDEN - ARCHITECTURAL VIOLATION) - const hasImproperServerTypingViolations = IMPROPER_SERVER_TYPING_VIOLATIONS.some(pattern => pattern.test(content)); + const hasImproperServerTypingViolations = IMPROPER_SERVER_TYPING_VIOLATIONS.some((pattern) => + pattern.test(content), + ); // Check for utility bypass patterns (ARCHITECTURAL VIOLATION) - const hasUtilityBypassPatterns = UTILITY_BYPASS_PATTERNS.some(pattern => pattern.test(content)); + const hasUtilityBypassPatterns = UTILITY_BYPASS_PATTERNS.some((pattern) => + pattern.test(content), + ); // Extract specific pattern occurrences for details const execSyncDetails = []; @@ -460,12 +514,12 @@ function analyzeToolOrResourceFile(filePath) { lines.forEach((line, index) => { if (isProductionFile(filePath) && !isAllowedChildProcessFile(filePath)) { - EXECSYNC_PATTERNS.forEach(pattern => { + EXECSYNC_PATTERNS.forEach((pattern) => { if (pattern.test(line)) { execSyncDetails.push({ line: index + 1, content: line.trim(), - pattern: pattern.source + pattern: pattern.source, }); } }); @@ -473,22 +527,22 @@ function analyzeToolOrResourceFile(filePath) { // TypeScript anti-patterns now handled by ESLint - removed from domain checks - IMPROPER_SERVER_TYPING_VIOLATIONS.forEach(pattern => { + IMPROPER_SERVER_TYPING_VIOLATIONS.forEach((pattern) => { if (pattern.test(line)) { improperServerTypingDetails.push({ line: index + 1, content: line.trim(), - pattern: pattern.source + pattern: pattern.source, }); } }); - UTILITY_BYPASS_PATTERNS.forEach(pattern => { + UTILITY_BYPASS_PATTERNS.forEach((pattern) => { if (pattern.test(line)) { utilityBypassDetails.push({ line: index + 1, content: line.trim(), - pattern: pattern.source + pattern: pattern.source, }); } }); @@ -498,7 +552,7 @@ function analyzeToolOrResourceFile(filePath) { const lines = content.split('\n'); const fullContent = content; - HANDLER_SIGNATURE_VIOLATIONS.forEach(pattern => { + HANDLER_SIGNATURE_VIOLATIONS.forEach((pattern) => { let match; const globalPattern = new RegExp(pattern.source, pattern.flags + 'g'); while ((match = globalPattern.exec(fullContent)) !== null) { @@ -509,7 +563,7 @@ function analyzeToolOrResourceFile(filePath) { handlerSignatureDetails.push({ line: lineNumber, content: match[0].replace(/\s+/g, ' ').trim(), - pattern: pattern.source + pattern: pattern.source, }); } }); @@ -530,9 +584,26 @@ function analyzeToolOrResourceFile(filePath) { handlerSignatureDetails, improperServerTypingDetails, utilityBypassDetails, - needsConversion: hasExecSyncPatterns || hasVitestMockingPatterns || hasHandlerSignatureViolations || hasImproperServerTypingViolations || hasUtilityBypassPatterns, - isConverted: hasDIPatterns && !hasExecSyncPatterns && !hasVitestMockingPatterns && !hasHandlerSignatureViolations && !hasImproperServerTypingViolations && !hasUtilityBypassPatterns, - isMixed: (hasExecSyncPatterns || hasVitestMockingPatterns || hasHandlerSignatureViolations || hasImproperServerTypingViolations || hasUtilityBypassPatterns) && hasDIPatterns + needsConversion: + hasExecSyncPatterns || + hasVitestMockingPatterns || + hasHandlerSignatureViolations || + hasImproperServerTypingViolations || + hasUtilityBypassPatterns, + isConverted: + hasDIPatterns && + !hasExecSyncPatterns && + !hasVitestMockingPatterns && + !hasHandlerSignatureViolations && + !hasImproperServerTypingViolations && + !hasUtilityBypassPatterns, + isMixed: + (hasExecSyncPatterns || + hasVitestMockingPatterns || + hasHandlerSignatureViolations || + hasImproperServerTypingViolations || + hasUtilityBypassPatterns) && + hasDIPatterns, }; } catch (error) { console.error(`Error reading file ${filePath}: ${error.message}`); @@ -548,9 +619,15 @@ function main() { console.log('❌ BANNED: vitest mocking patterns (vi.mock, vi.fn, .mockResolvedValue, etc.)'); console.log('❌ BANNED: execSync usage in production code (use CommandExecutor instead)'); console.log('ℹ️ TypeScript patterns: Handled by ESLint with proper test exceptions'); - console.log('❌ BANNED: handler signature violations (handlers must have exact MCP SDK signatures)'); - console.log('❌ BANNED: handler testing violations (test logic functions, not handlers directly)'); - console.log('❌ BANNED: improper server typing (use McpServer type, not Record)\n'); + console.log( + '❌ BANNED: handler signature violations (handlers must have exact MCP SDK signatures)', + ); + console.log( + '❌ BANNED: handler testing violations (test logic functions, not handlers directly)', + ); + console.log( + '❌ BANNED: improper server typing (use McpServer type, not Record)\n', + ); const testFiles = findTestFiles(join(projectRoot, 'src')); const testResults = testFiles.map(analyzeTestFile).filter(Boolean); @@ -568,7 +645,7 @@ function main() { // Combine test, tool, and utility file results for analysis const results = [...testResults, ...toolResults, ...utilityResults]; const handlerResults = toolResults; - const utilityBypassResults = utilityResults.filter(r => r.hasUtilityBypassPatterns); + const utilityBypassResults = utilityResults.filter((r) => r.hasUtilityBypassPatterns); // Filter results based on pattern type let filteredResults; @@ -576,44 +653,101 @@ function main() { switch (patternFilter) { case 'vitest': - filteredResults = results.filter(r => r.hasVitestMockingPatterns); - console.log(`Filtering to show only vitest mocking violations (${filteredResults.length} files)`); + filteredResults = results.filter((r) => r.hasVitestMockingPatterns); + console.log( + `Filtering to show only vitest mocking violations (${filteredResults.length} files)`, + ); break; case 'execsync': - filteredResults = results.filter(r => r.hasExecSyncPatterns); + filteredResults = results.filter((r) => r.hasExecSyncPatterns); console.log(`Filtering to show only execSync violations (${filteredResults.length} files)`); break; // TypeScript case removed - now handled by ESLint case 'handler': filteredResults = []; - filteredHandlerResults = handlerResults.filter(r => r.hasHandlerSignatureViolations); - console.log(`Filtering to show only handler signature violations (${filteredHandlerResults.length} files)`); + filteredHandlerResults = handlerResults.filter((r) => r.hasHandlerSignatureViolations); + console.log( + `Filtering to show only handler signature violations (${filteredHandlerResults.length} files)`, + ); break; case 'handler-testing': - filteredResults = results.filter(r => r.hasHandlerTestingViolations); - console.log(`Filtering to show only handler testing violations (${filteredResults.length} files)`); + filteredResults = results.filter((r) => r.hasHandlerTestingViolations); + console.log( + `Filtering to show only handler testing violations (${filteredResults.length} files)`, + ); break; case 'server-typing': - filteredResults = results.filter(r => r.hasImproperServerTypingViolations); - console.log(`Filtering to show only improper server typing violations (${filteredResults.length} files)`); + filteredResults = results.filter((r) => r.hasImproperServerTypingViolations); + console.log( + `Filtering to show only improper server typing violations (${filteredResults.length} files)`, + ); break; case 'all': default: - filteredResults = results.filter(r => r.needsConversion); - filteredHandlerResults = handlerResults.filter(r => r.hasHandlerSignatureViolations); - console.log(`Showing all pattern violations (${filteredResults.length} test files + ${filteredHandlerResults.length} handler files)`); + filteredResults = results.filter((r) => r.needsConversion); + filteredHandlerResults = handlerResults.filter((r) => r.hasHandlerSignatureViolations); + console.log( + `Showing all pattern violations (${filteredResults.length} test files + ${filteredHandlerResults.length} handler files)`, + ); break; } const needsConversion = filteredResults; - const converted = results.filter(r => r.isConverted); - const mixed = results.filter(r => r.isMixed); - const execSyncOnly = results.filter(r => r.hasExecSyncPatterns && !r.hasVitestMockingPatterns && true && !r.hasHandlerTestingViolations && !r.hasImproperServerTypingViolations && !r.hasDIPatterns); - const vitestMockingOnly = results.filter(r => r.hasVitestMockingPatterns && !r.hasExecSyncPatterns && true && !r.hasHandlerTestingViolations && !r.hasImproperServerTypingViolations && !r.hasDIPatterns); - const typescriptOnly = results.filter(r => r.false && !r.hasExecSyncPatterns && !r.hasVitestMockingPatterns && !r.hasHandlerTestingViolations && !r.hasImproperServerTypingViolations && !r.hasDIPatterns); - const handlerTestingOnly = results.filter(r => r.hasHandlerTestingViolations && !r.hasExecSyncPatterns && !r.hasVitestMockingPatterns && true && !r.hasImproperServerTypingViolations && !r.hasDIPatterns); - const improperServerTypingOnly = results.filter(r => r.hasImproperServerTypingViolations && !r.hasExecSyncPatterns && !r.hasVitestMockingPatterns && !r.hasHandlerTestingViolations && !r.hasDIPatterns); - const noPatterns = results.filter(r => !r.hasExecSyncPatterns && !r.hasVitestMockingPatterns && true && !r.hasHandlerTestingViolations && !r.hasImproperServerTypingViolations && !r.hasDIPatterns); + const converted = results.filter((r) => r.isConverted); + const mixed = results.filter((r) => r.isMixed); + const execSyncOnly = results.filter( + (r) => + r.hasExecSyncPatterns && + !r.hasVitestMockingPatterns && + true && + !r.hasHandlerTestingViolations && + !r.hasImproperServerTypingViolations && + !r.hasDIPatterns, + ); + const vitestMockingOnly = results.filter( + (r) => + r.hasVitestMockingPatterns && + !r.hasExecSyncPatterns && + true && + !r.hasHandlerTestingViolations && + !r.hasImproperServerTypingViolations && + !r.hasDIPatterns, + ); + const typescriptOnly = results.filter( + (r) => + r.false && + !r.hasExecSyncPatterns && + !r.hasVitestMockingPatterns && + !r.hasHandlerTestingViolations && + !r.hasImproperServerTypingViolations && + !r.hasDIPatterns, + ); + const handlerTestingOnly = results.filter( + (r) => + r.hasHandlerTestingViolations && + !r.hasExecSyncPatterns && + !r.hasVitestMockingPatterns && + true && + !r.hasImproperServerTypingViolations && + !r.hasDIPatterns, + ); + const improperServerTypingOnly = results.filter( + (r) => + r.hasImproperServerTypingViolations && + !r.hasExecSyncPatterns && + !r.hasVitestMockingPatterns && + !r.hasHandlerTestingViolations && + !r.hasDIPatterns, + ); + const noPatterns = results.filter( + (r) => + !r.hasExecSyncPatterns && + !r.hasVitestMockingPatterns && + true && + !r.hasHandlerTestingViolations && + !r.hasImproperServerTypingViolations && + !r.hasDIPatterns, + ); console.log(`📊 CODE PATTERN VIOLATION ANALYSIS`); console.log(`=================================`); @@ -638,7 +772,7 @@ function main() { if (result.execSyncDetails && result.execSyncDetails.length > 0) { console.log(` 🚨 EXECSYNC PATTERNS (${result.execSyncDetails.length}):`); - result.execSyncDetails.slice(0, 2).forEach(detail => { + result.execSyncDetails.slice(0, 2).forEach((detail) => { console.log(` Line ${detail.line}: ${detail.content}`); }); if (result.execSyncDetails.length > 2) { @@ -649,7 +783,7 @@ function main() { if (result.vitestMockingDetails.length > 0) { console.log(` 🧪 VITEST MOCKING PATTERNS (${result.vitestMockingDetails.length}):`); - result.vitestMockingDetails.slice(0, 2).forEach(detail => { + result.vitestMockingDetails.slice(0, 2).forEach((detail) => { console.log(` Line ${detail.line}: ${detail.content}`); }); if (result.vitestMockingDetails.length > 2) { @@ -661,31 +795,41 @@ function main() { if (result.handlerTestingDetails && result.handlerTestingDetails.length > 0) { console.log(` 🚨 HANDLER TESTING VIOLATIONS (${result.handlerTestingDetails.length}):`); - result.handlerTestingDetails.slice(0, 2).forEach(detail => { + result.handlerTestingDetails.slice(0, 2).forEach((detail) => { console.log(` Line ${detail.line}: ${detail.content}`); }); if (result.handlerTestingDetails.length > 2) { - console.log(` ... and ${result.handlerTestingDetails.length - 2} more handler testing violations`); + console.log( + ` ... and ${result.handlerTestingDetails.length - 2} more handler testing violations`, + ); } - console.log(` 🔧 FIX: Replace handler calls with logic function calls using dependency injection`); + console.log( + ` 🔧 FIX: Replace handler calls with logic function calls using dependency injection`, + ); } if (result.improperServerTypingDetails && result.improperServerTypingDetails.length > 0) { - console.log(` 🔧 IMPROPER SERVER TYPING VIOLATIONS (${result.improperServerTypingDetails.length}):`); - result.improperServerTypingDetails.slice(0, 2).forEach(detail => { + console.log( + ` 🔧 IMPROPER SERVER TYPING VIOLATIONS (${result.improperServerTypingDetails.length}):`, + ); + result.improperServerTypingDetails.slice(0, 2).forEach((detail) => { console.log(` Line ${detail.line}: ${detail.content}`); }); if (result.improperServerTypingDetails.length > 2) { - console.log(` ... and ${result.improperServerTypingDetails.length - 2} more server typing violations`); + console.log( + ` ... and ${result.improperServerTypingDetails.length - 2} more server typing violations`, + ); } - console.log(` 🔧 FIX: Import McpServer from SDK and use proper typing instead of Record`); + console.log( + ` 🔧 FIX: Import McpServer from SDK and use proper typing instead of Record`, + ); } console.log(''); }); } - // Utility bypass violations reporting + // Utility bypass violations reporting if (utilityBypassResults.length > 0) { console.log(`🚨 CRITICAL: UTILITY ARCHITECTURAL VIOLATIONS (${utilityBypassResults.length}):`); console.log(`=======================================================`); @@ -696,12 +840,14 @@ function main() { if (result.utilityBypassDetails.length > 0) { console.log(` 🚨 BYPASS PATTERNS (${result.utilityBypassDetails.length}):`); - result.utilityBypassDetails.forEach(detail => { + result.utilityBypassDetails.forEach((detail) => { console.log(` Line ${detail.line}: ${detail.content}`); }); } - console.log(' 🔧 FIX: Refactor to accept CommandExecutor and use it instead of direct spawn/exec calls'); + console.log( + ' 🔧 FIX: Refactor to accept CommandExecutor and use it instead of direct spawn/exec calls', + ); console.log(''); }); } @@ -715,7 +861,7 @@ function main() { if (result.handlerSignatureDetails.length > 0) { console.log(` 🛠️ HANDLER VIOLATIONS (${result.handlerSignatureDetails.length}):`); - result.handlerSignatureDetails.forEach(detail => { + result.handlerSignatureDetails.forEach((detail) => { console.log(` Line ${detail.line}: ${detail.content}`); }); } @@ -744,14 +890,19 @@ function main() { } // Summary for next steps - const hasViolations = needsConversion.length > 0 || filteredHandlerResults.length > 0 || utilityBypassResults.length > 0; + const hasViolations = + needsConversion.length > 0 || + filteredHandlerResults.length > 0 || + utilityBypassResults.length > 0; if (needsConversion.length > 0) { console.log(`🚨 CRITICAL ACTION REQUIRED (TEST FILES):`); console.log(`=======================================`); console.log(`1. IMMEDIATELY remove ALL vitest mocking from ${needsConversion.length} files`); console.log(`2. BANNED: vi.mock(), vi.fn(), .mockResolvedValue(), .toHaveBeenCalled(), etc.`); - console.log(`3. BANNED: Testing handlers directly (.handler()) - test logic functions with dependency injection`); + console.log( + `3. BANNED: Testing handlers directly (.handler()) - test logic functions with dependency injection`, + ); console.log(`4. ONLY ALLOWED: createMockExecutor() and createMockFileSystemExecutor()`); console.log(`4. Update plugin implementations to accept executor?: CommandExecutor parameter`); console.log(`5. Run this script again after each fix to track progress`); @@ -760,16 +911,30 @@ function main() { // Show top files by total violation count const sortedByPatterns = needsConversion .sort((a, b) => { - const totalA = (a.execSyncDetails?.length || 0) + a.vitestMockingDetails.length + (a.handlerTestingDetails?.length || 0) + (a.improperServerTypingDetails?.length || 0); - const totalB = (b.execSyncDetails?.length || 0) + b.vitestMockingDetails.length + (b.handlerTestingDetails?.length || 0) + (b.improperServerTypingDetails?.length || 0); + const totalA = + (a.execSyncDetails?.length || 0) + + a.vitestMockingDetails.length + + (a.handlerTestingDetails?.length || 0) + + (a.improperServerTypingDetails?.length || 0); + const totalB = + (b.execSyncDetails?.length || 0) + + b.vitestMockingDetails.length + + (b.handlerTestingDetails?.length || 0) + + (b.improperServerTypingDetails?.length || 0); return totalB - totalA; }) .slice(0, 5); console.log(`🚨 TOP 5 FILES WITH MOST VIOLATIONS:`); sortedByPatterns.forEach((result, index) => { - const totalPatterns = (result.execSyncDetails?.length || 0) + result.vitestMockingDetails.length + (result.handlerTestingDetails?.length || 0) + (result.improperServerTypingDetails?.length || 0); - console.log(`${index + 1}. ${result.filePath} (${totalPatterns} violations: ${result.execSyncDetails?.length || 0} execSync + ${result.vitestMockingDetails.length} vitest + ${result.handlerTestingDetails?.length || 0} handler + ${result.improperServerTypingDetails?.length || 0} server)`); + const totalPatterns = + (result.execSyncDetails?.length || 0) + + result.vitestMockingDetails.length + + (result.handlerTestingDetails?.length || 0) + + (result.improperServerTypingDetails?.length || 0); + console.log( + `${index + 1}. ${result.filePath} (${totalPatterns} violations: ${result.execSyncDetails?.length || 0} execSync + ${result.vitestMockingDetails.length} vitest + ${result.handlerTestingDetails?.length || 0} handler + ${result.improperServerTypingDetails?.length || 0} server)`, + ); }); console.log(''); } @@ -777,7 +942,9 @@ function main() { if (utilityBypassResults.length > 0) { console.log(`🚨 CRITICAL ACTION REQUIRED (UTILITY FILES):`); console.log(`==========================================`); - console.log(`1. IMMEDIATELY fix ALL architectural violations in ${utilityBypassResults.length} files`); + console.log( + `1. IMMEDIATELY fix ALL architectural violations in ${utilityBypassResults.length} files`, + ); console.log(`2. Refactor utilities to accept CommandExecutor parameter`); console.log(`3. Replace direct spawn/exec calls with executor calls`); console.log(`4. These violations break our entire testing strategy`); @@ -788,10 +955,18 @@ function main() { if (filteredHandlerResults.length > 0) { console.log(`🚨 CRITICAL ACTION REQUIRED (HANDLER FILES):`); console.log(`==========================================`); - console.log(`1. IMMEDIATELY fix ALL handler signature violations in ${filteredHandlerResults.length} files`); - console.log(`2. Tools: Handler must be: async handler(args: Record): Promise`); - console.log(`3. Resources: Handler must be: async handler(uri: URL): Promise<{ contents: Array<{ text: string }> }>`); - console.log(`4. Inject dependencies INSIDE handler body: const executor = getDefaultCommandExecutor()`); + console.log( + `1. IMMEDIATELY fix ALL handler signature violations in ${filteredHandlerResults.length} files`, + ); + console.log( + `2. Tools: Handler must be: async handler(args: Record): Promise`, + ); + console.log( + `3. Resources: Handler must be: async handler(uri: URL): Promise<{ contents: Array<{ text: string }> }>`, + ); + console.log( + `4. Inject dependencies INSIDE handler body: const executor = getDefaultCommandExecutor()`, + ); console.log(`5. Run this script again after each fix to track progress`); console.log(''); } diff --git a/scripts/copy-build-assets.js b/scripts/copy-build-assets.js index 0df07c6e..b1c14038 100644 --- a/scripts/copy-build-assets.js +++ b/scripts/copy-build-assets.js @@ -13,11 +13,7 @@ const __dirname = dirname(__filename); const projectRoot = join(__dirname, '..'); // Set executable permissions for entry points -const executables = [ - 'build/cli.js', - 'build/doctor-cli.js', - 'build/daemon.js' -]; +const executables = ['build/cli.js', 'build/doctor-cli.js', 'build/daemon.js']; for (const file of executables) { const fullPath = join(projectRoot, file); diff --git a/scripts/generate-github-release-notes.mjs b/scripts/generate-github-release-notes.mjs index cf7fd2b6..88d8e6e8 100644 --- a/scripts/generate-github-release-notes.mjs +++ b/scripts/generate-github-release-notes.mjs @@ -107,7 +107,7 @@ function extractChangelogSection(changelog, version) { if (sectionStartLine === -1) { throw new Error( `Missing CHANGELOG section for version: ${normalizedTarget}\n` + - `Add a heading like: ## [${normalizedTarget}] (or ## [v${normalizedTarget}] - YYYY-MM-DD)` + `Add a heading like: ## [${normalizedTarget}] (or ## [v${normalizedTarget}] - YYYY-MM-DD)`, ); } @@ -168,14 +168,9 @@ function buildInstallAndSetupSection(version, packageName) { function buildReleaseBody(version, changelogSection, packageName) { const normalizedVersion = normalizeVersion(version); const installAndSetup = buildInstallAndSetupSection(normalizedVersion, packageName); - return [ - `## Release v${normalizedVersion}`, - '', - changelogSection, - '', - installAndSetup, - '', - ].join('\n'); + return [`## Release v${normalizedVersion}`, '', changelogSection, '', installAndSetup, ''].join( + '\n', + ); } async function main() { diff --git a/scripts/probe-xcode-mcpbridge.ts b/scripts/probe-xcode-mcpbridge.ts index c7d04212..0d5714c5 100644 --- a/scripts/probe-xcode-mcpbridge.ts +++ b/scripts/probe-xcode-mcpbridge.ts @@ -64,7 +64,10 @@ async function main(): Promise { const toolsResult = await client.listTools(undefined, { timeout: 15_000 }); console.log(`tools: ${toolsResult.tools.length}`); - console.log('first tools:', toolsResult.tools.slice(0, limit).map((t) => t.name)); + console.log( + 'first tools:', + toolsResult.tools.slice(0, limit).map((t) => t.name), + ); if (callWindows) { const windows = await client.request( diff --git a/scripts/update-tools-docs.ts b/scripts/update-tools-docs.ts index 32f9c500..a570e341 100644 --- a/scripts/update-tools-docs.ts +++ b/scripts/update-tools-docs.ts @@ -209,7 +209,7 @@ function generateToolsDocumentation(manifest: ToolsManifest): string { const workflowTools = toolsByWorkflow.get(workflow.name) ?? []; const docTools = workflowTools.map((tool) => { const originWorkflow = tool.originWorkflow - ? workflowMeta.get(tool.originWorkflow)?.displayName ?? tool.originWorkflow + ? (workflowMeta.get(tool.originWorkflow)?.displayName ?? tool.originWorkflow) : undefined; return { @@ -283,7 +283,7 @@ function generateCliToolsDocumentation(manifest: ToolsManifest): CliDocumentatio const tools = toolsByWorkflow.get(tool.workflow) ?? []; const originWorkflow = tool.originWorkflow - ? workflowMeta.get(tool.originWorkflow)?.displayName ?? tool.originWorkflow + ? (workflowMeta.get(tool.originWorkflow)?.displayName ?? tool.originWorkflow) : undefined; tools.push({ @@ -427,9 +427,7 @@ async function main(): Promise { ]; const changes = targets.map((target) => { - const existing = fs.existsSync(target.path) - ? fs.readFileSync(target.path, 'utf-8') - : ''; + const existing = fs.existsSync(target.path) ? fs.readFileSync(target.path, 'utf-8') : ''; const changed = existing !== target.content; return { ...target, existing, changed }; @@ -463,7 +461,9 @@ async function main(): Promise { console.log(` - ${path.relative(projectRoot, target.path)} (${target.label})`); } console.log(` - MCP tools: ${manifest.stats.canonicalTools} canonical tools`); - console.log(` - CLI tools: ${cliStats.toolCount} tools across ${cliStats.workflowCount} workflows`); + console.log( + ` - CLI tools: ${cliStats.toolCount} tools across ${cliStats.workflowCount} workflows`, + ); console.log(` - MCP lines: ${mcpContent.split('\n').length}`); console.log(` - CLI lines: ${cliContent.split('\n').length}`); diff --git a/src/cli.ts b/src/cli.ts index f44bb606..4018474c 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -78,15 +78,13 @@ async function main(): Promise { discoveryMode, }); - const workflowNames = cliExposedWorkflowIds; - const yargsApp = buildYargsApp({ catalog, runtimeConfig: result.runtime.config, defaultSocketPath, workspaceRoot, workspaceKey, - workflowNames, + workflowNames: cliExposedWorkflowIds, cliExposedWorkflowIds, }); diff --git a/src/core/resources.ts b/src/core/resources.ts index ba6d8e8e..c62722e8 100644 --- a/src/core/resources.ts +++ b/src/core/resources.ts @@ -53,10 +53,11 @@ export async function loadResources(): Promise> { for (const resource of RESOURCES) { if (!resource.uri || !resource.handler || typeof resource.handler !== 'function') { - log('error', `Invalid resource structure for ${resource.name ?? 'unknown'}`); - log('error', '[infra/resources] invalid resource structure detected during registration', { - sentry: true, - }); + log( + 'error', + `[infra/resources] invalid resource structure for ${resource.name ?? 'unknown'}`, + { sentry: true }, + ); continue; } @@ -75,7 +76,7 @@ export async function loadResources(): Promise> { export async function registerResources(server: McpServer): Promise { const resources = await loadResources(); - for (const [uri, resource] of Array.from(resources)) { + for (const [uri, resource] of resources) { const readCallback = async (resourceUri: URL): Promise => { const result = await resource.handler(resourceUri); return { diff --git a/src/daemon.ts b/src/daemon.ts index e6a2b733..240cac56 100644 --- a/src/daemon.ts +++ b/src/daemon.ts @@ -128,7 +128,6 @@ function resolveLogLevel(): LogLevel | null { async function main(): Promise { const daemonBootstrapStart = Date.now(); - // Bootstrap runtime first to get config and workspace info const result = await bootstrapRuntime({ runtime: 'daemon', configOverrides: { @@ -136,7 +135,6 @@ async function main(): Promise { }, }); - // Compute workspace context const workspaceRoot = resolveWorkspaceRoot({ cwd: result.runtime.cwd, projectConfigPath: result.configPath, @@ -153,12 +151,7 @@ async function main(): Promise { rotateLogIfNeeded(logPath); setLogFile(logPath); - const requestedLogLevel = resolveLogLevel(); - if (requestedLogLevel) { - setLogLevel(requestedLogLevel); - } else { - setLogLevel('info'); - } + setLogLevel(resolveLogLevel() ?? 'info'); } initSentry({ mode: 'cli-daemon' }); @@ -166,7 +159,6 @@ async function main(): Promise { log('info', `[Daemon] xcodebuildmcp daemon ${version} starting...`); - // Get socket path (env override or workspace-derived) const socketPath = getSocketPath({ cwd: result.runtime.cwd, projectConfigPath: result.configPath, @@ -180,7 +172,6 @@ async function main(): Promise { ensureSocketDir(socketPath); - // Check if daemon is already running const isRunning = await checkExistingDaemon(socketPath); if (isRunning) { log('error', '[Daemon] Another daemon is already running for this workspace'); @@ -189,7 +180,6 @@ async function main(): Promise { process.exit(1); } - // Remove stale socket file removeStaleSocket(socketPath); const excludedWorkflows = ['session-management', 'workflow-discovery']; @@ -199,31 +189,28 @@ async function main(): Promise { // Get all workflows from manifest (for reporting purposes and filtering). const manifest = loadManifest(); const allWorkflowIds = Array.from(manifest.workflows.keys()); - const daemonWorkflows = allWorkflowIds.filter((workflowId) => { - if (excludedWorkflows.includes(workflowId)) { - return false; - } - return true; - }); + const daemonWorkflows = allWorkflowIds.filter( + (workflowId) => !excludedWorkflows.includes(workflowId), + ); const xcodeIdeWorkflowEnabled = daemonWorkflows.includes('xcode-ide'); const commandExecutor = getDefaultCommandExecutor(); const xcodeVersion = await getXcodeVersionMetadata(async (command) => { const result = await commandExecutor(command, 'Get Xcode Version'); - return { success: result.success, output: result.output ?? '' }; + return { success: result.success, output: result.output }; }); - const xcodeAvailable = [ - xcodeVersion.version, - xcodeVersion.buildVersion, - xcodeVersion.developerDir, - xcodeVersion.xcodebuildPath, - ].some((value) => Boolean(value)); + const xcodeAvailable = Boolean( + xcodeVersion.version || + xcodeVersion.buildVersion || + xcodeVersion.developerDir || + xcodeVersion.xcodebuildPath, + ); const axeBinary = resolveAxeBinary(); const axeAvailable = axeBinary !== null; const axeSource = axeBinary?.source ?? 'unavailable'; const xcodemakeAvailable = isXcodemakeBinaryAvailable(); const axeVersion = await getAxeVersionMetadata(async (command) => { const result = await commandExecutor(command, 'Get AXe Version'); - return { success: result.success, output: result.output ?? '' }; + return { success: result.success, output: result.output }; }, axeBinary?.path); setSentryRuntimeContext({ mode: 'cli-daemon', @@ -246,7 +233,6 @@ async function main(): Promise { xcodeBuildVersion: xcodeVersion.buildVersion, }); - // Build tool catalog using manifest system const catalog = await buildDaemonToolCatalogFromManifest({ excludeWorkflows: excludedWorkflows, }); @@ -316,7 +302,7 @@ async function main(): Promise { // Force exit if server doesn't close in time setTimeout(() => { - log('warn', '[Daemon] Forced shutdown after timeout'); + log('warning', '[Daemon] Forced shutdown after timeout'); cleanupWorkspaceDaemonFiles(workspaceKey); void flushAndCloseSentry(1000).finally(() => { process.exit(1); @@ -324,7 +310,14 @@ async function main(): Promise { }, 5000); }; - // Start server + const emitRequestGauges = (): void => { + recordDaemonGaugeMetric('inflight_requests', inFlightRequests); + recordDaemonGaugeMetric( + 'active_sessions', + getDaemonRuntimeActivitySnapshot().activeOperationCount, + ); + }; + const server = startDaemonServer({ socketPath, logPath: logPath ?? undefined, @@ -338,23 +331,15 @@ async function main(): Promise { onRequestStarted: () => { inFlightRequests += 1; markActivity(); - recordDaemonGaugeMetric('inflight_requests', inFlightRequests); - const sessionSnapshot = getDaemonRuntimeActivitySnapshot(); - recordDaemonGaugeMetric('active_sessions', sessionSnapshot.activeOperationCount); + emitRequestGauges(); }, onRequestFinished: () => { inFlightRequests = Math.max(0, inFlightRequests - 1); markActivity(); - recordDaemonGaugeMetric('inflight_requests', inFlightRequests); - const sessionSnapshot = getDaemonRuntimeActivitySnapshot(); - recordDaemonGaugeMetric('active_sessions', sessionSnapshot.activeOperationCount); + emitRequestGauges(); }, }); - recordDaemonGaugeMetric('inflight_requests', inFlightRequests); - recordDaemonGaugeMetric( - 'active_sessions', - getDaemonRuntimeActivitySnapshot().activeOperationCount, - ); + emitRequestGauges(); if (idleTimeoutMs > 0) { idleCheckTimer = setInterval(() => { @@ -362,9 +347,7 @@ async function main(): Promise { return; } - const sessionSnapshot = getDaemonRuntimeActivitySnapshot(); - recordDaemonGaugeMetric('inflight_requests', inFlightRequests); - recordDaemonGaugeMetric('active_sessions', sessionSnapshot.activeOperationCount); + emitRequestGauges(); const idleForMs = Date.now() - lastActivityAt; if (idleForMs < idleTimeoutMs) { @@ -375,7 +358,7 @@ async function main(): Promise { return; } - if (hasActiveRuntimeSessions(sessionSnapshot)) { + if (hasActiveRuntimeSessions(getDaemonRuntimeActivitySnapshot())) { return; } @@ -410,21 +393,14 @@ async function main(): Promise { recordBootstrapDurationMetric('cli-daemon', Date.now() - daemonBootstrapStart); }); - // Handle graceful shutdown process.on('SIGTERM', shutdown); process.on('SIGINT', shutdown); } main().catch(async (err) => { recordDaemonLifecycleMetric('crash'); - let message = 'Unknown daemon error'; - if (err instanceof Error) { - message = err.message; - } else if (typeof err === 'string') { - message = err; - } else if (err !== null && err !== undefined) { - message = String(err); - } + const message = + err == null ? 'Unknown daemon error' : err instanceof Error ? err.message : String(err); log('error', `Daemon error: ${message}`, { sentry: true }); console.error('Daemon error:', message); diff --git a/src/integrations/xcode-tools-bridge/__tests__/fixtures/fake-xcode-tools-server.mjs b/src/integrations/xcode-tools-bridge/__tests__/fixtures/fake-xcode-tools-server.mjs index 44267aa6..6718f409 100644 --- a/src/integrations/xcode-tools-bridge/__tests__/fixtures/fake-xcode-tools-server.mjs +++ b/src/integrations/xcode-tools-bridge/__tests__/fixtures/fake-xcode-tools-server.mjs @@ -103,4 +103,3 @@ function applyCatalogChange() { registerInitialTools(); await server.connect(new StdioServerTransport()); - diff --git a/src/runtime/tool-invoker.ts b/src/runtime/tool-invoker.ts index e02f226a..36d5abd1 100644 --- a/src/runtime/tool-invoker.ts +++ b/src/runtime/tool-invoker.ts @@ -56,13 +56,13 @@ function getErrorKind(error: unknown): string { } function mapRuntimeToSentryToolRuntime(runtime: InvokeOptions['runtime']): SentryToolRuntime { - if (runtime === 'daemon') { - return 'daemon'; + switch (runtime) { + case 'daemon': + case 'mcp': + return runtime; + default: + return 'cli'; } - if (runtime === 'mcp') { - return 'mcp'; - } - return 'cli'; } export class DefaultToolInvoker implements ToolInvoker { @@ -105,6 +105,78 @@ export class DefaultToolInvoker implements ToolInvoker { return this.executeTool(tool, args, opts); } + /** + * Route a tool invocation through the daemon, auto-starting it if needed. + */ + private async invokeViaDaemon( + opts: InvokeOptions, + args: Record, + invoke: (client: DaemonClient) => Promise, + context: { + label: string; + errorTitle: string; + captureInfraErrorMetric: (error: unknown) => void; + captureInvocationMetric: (outcome: SentryToolInvocationOutcome) => void; + }, + ): Promise { + const socketPath = opts.socketPath; + if (!socketPath) { + context.captureInfraErrorMetric(new Error('SocketPathMissing')); + context.captureInvocationMetric('infra_error'); + return createErrorResponse( + 'Socket path required', + 'No socket path configured for daemon communication.', + ); + } + + const client = new DaemonClient({ socketPath }); + const envOverrides = buildDaemonEnvOverrides(opts); + + const isRunning = await client.isRunning(); + if (!isRunning) { + try { + await ensureDaemonRunning({ + socketPath, + workspaceRoot: opts.workspaceRoot, + startupTimeoutMs: opts.daemonStartupTimeoutMs ?? DEFAULT_DAEMON_STARTUP_TIMEOUT_MS, + env: envOverrides, + }); + } catch (error) { + log( + 'error', + `[infra/tool-invoker] ${context.label} daemon auto-start failed (${getErrorKind(error)})`, + { sentry: true }, + ); + context.captureInfraErrorMetric(error); + context.captureInvocationMetric('infra_error'); + return createErrorResponse( + 'Daemon auto-start failed', + (error instanceof Error ? error.message : String(error)) + + '\n\nYou can try starting the daemon manually:\n' + + ' xcodebuildmcp daemon start', + ); + } + } + + try { + const response = await invoke(client); + context.captureInvocationMetric('completed'); + return enrichNextStepsForCli(response, this.catalog); + } catch (error) { + log( + 'error', + `[infra/tool-invoker] ${context.label} transport failed (${getErrorKind(error)})`, + { sentry: true }, + ); + context.captureInfraErrorMetric(error); + context.captureInvocationMetric('infra_error'); + return createErrorResponse( + context.errorTitle, + error instanceof Error ? error.message : String(error), + ); + } + } + private async executeTool( tool: ToolDefinition, args: Record, @@ -132,131 +204,34 @@ export class DefaultToolInvoker implements ToolInvoker { }); }; - const xcodeIdeRemoteToolName = tool.xcodeIdeRemoteToolName; - const isDynamicXcodeIdeTool = - tool.workflow === 'xcode-ide' && typeof xcodeIdeRemoteToolName === 'string'; - - if (opts.runtime === 'cli' && isDynamicXcodeIdeTool) { - transport = 'xcode-ide-daemon'; - const socketPath = opts.socketPath; - if (!socketPath) { - captureInfraErrorMetric(new Error('SocketPathMissing')); - captureInvocationMetric('infra_error'); - return createErrorResponse( - 'Socket path required', - `No socket path configured for daemon communication.`, - ); - } - - const envOverrideValue = buildDaemonEnvOverrides(opts); - const client = new DaemonClient({ socketPath }); + const metricContext = { captureInfraErrorMetric, captureInvocationMetric }; - const isRunning = await client.isRunning(); - if (!isRunning) { - try { - await ensureDaemonRunning({ - socketPath, - workspaceRoot: opts.workspaceRoot, - startupTimeoutMs: opts.daemonStartupTimeoutMs ?? DEFAULT_DAEMON_STARTUP_TIMEOUT_MS, - env: envOverrideValue, - }); - } catch (error) { - log( - 'error', - `[infra/tool-invoker] xcode-ide daemon auto-start failed (${getErrorKind(error)})`, - { sentry: true }, - ); - captureInfraErrorMetric(error); - captureInvocationMetric('infra_error'); - return createErrorResponse( - 'Daemon auto-start failed', - (error instanceof Error ? error.message : String(error)) + - `\n\nYou can try starting the daemon manually:\n` + - ` xcodebuildmcp daemon start`, - ); - } - } - - try { - const response = await client.invokeXcodeIdeTool(xcodeIdeRemoteToolName, args); - captureInvocationMetric('completed'); - return enrichNextStepsForCli(response, this.catalog); - } catch (error) { - log( - 'error', - `[infra/tool-invoker] xcode-ide tool invocation transport failed (${getErrorKind(error)})`, - { sentry: true }, - ); - captureInfraErrorMetric(error); - captureInvocationMetric('infra_error'); - return createErrorResponse( - 'Xcode IDE invocation failed', - error instanceof Error ? error.message : String(error), + if (opts.runtime === 'cli') { + const xcodeIdeRemoteToolName = tool.xcodeIdeRemoteToolName; + const isDynamicXcodeIdeTool = + tool.workflow === 'xcode-ide' && typeof xcodeIdeRemoteToolName === 'string'; + + if (isDynamicXcodeIdeTool) { + transport = 'xcode-ide-daemon'; + return this.invokeViaDaemon( + opts, + args, + (client) => client.invokeXcodeIdeTool(xcodeIdeRemoteToolName, args), + { + ...metricContext, + label: 'xcode-ide', + errorTitle: 'Xcode IDE invocation failed', + }, ); } - } - - const mustUseDaemon = tool.stateful; - if (opts.runtime === 'cli') { - if (mustUseDaemon) { + if (tool.stateful) { transport = 'daemon'; - // Route through daemon with auto-start - const socketPath = opts.socketPath; - if (!socketPath) { - captureInfraErrorMetric(new Error('SocketPathMissing')); - captureInvocationMetric('infra_error'); - return createErrorResponse( - 'Socket path required', - `No socket path configured for daemon communication.`, - ); - } - - const client = new DaemonClient({ socketPath }); - const envOverrideValue = buildDaemonEnvOverrides(opts); - - // Check if daemon is running; auto-start if not - const isRunning = await client.isRunning(); - if (!isRunning) { - try { - await ensureDaemonRunning({ - socketPath, - workspaceRoot: opts.workspaceRoot, - startupTimeoutMs: opts.daemonStartupTimeoutMs ?? DEFAULT_DAEMON_STARTUP_TIMEOUT_MS, - env: envOverrideValue, - }); - } catch (error) { - log('error', `[infra/tool-invoker] daemon auto-start failed (${getErrorKind(error)})`, { - sentry: true, - }); - captureInfraErrorMetric(error); - captureInvocationMetric('infra_error'); - return createErrorResponse( - 'Daemon auto-start failed', - (error instanceof Error ? error.message : String(error)) + - `\n\nYou can try starting the daemon manually:\n` + - ` xcodebuildmcp daemon start`, - ); - } - } - - try { - const response = await client.invokeTool(tool.mcpName, args); - captureInvocationMetric('completed'); - return opts.runtime === 'cli' ? enrichNextStepsForCli(response, this.catalog) : response; - } catch (error) { - log( - 'error', - `[infra/tool-invoker] daemon transport invocation failed for ${tool.mcpName} (${getErrorKind(error)})`, - { sentry: true }, - ); - captureInfraErrorMetric(error); - captureInvocationMetric('infra_error'); - return createErrorResponse( - 'Daemon invocation failed', - error instanceof Error ? error.message : String(error), - ); - } + return this.invokeViaDaemon(opts, args, (client) => client.invokeTool(tool.mcpName, args), { + ...metricContext, + label: `daemon/${tool.mcpName}`, + errorTitle: 'Daemon invocation failed', + }); } } diff --git a/src/server/bootstrap.ts b/src/server/bootstrap.ts index 3294360e..df031929 100644 --- a/src/server/bootstrap.ts +++ b/src/server/bootstrap.ts @@ -79,18 +79,15 @@ export async function bootstrapServer( const xcodeToolsAvailable = mcpBridge.available; log('info', `🚀 Initializing server...`); - // Detect if running under Xcode const executor = getDefaultCommandExecutor(); const xcodeDetection = await detectXcodeRuntime(executor); if (xcodeDetection.runningUnderXcode) { log('info', `[xcode] Running under Xcode agent environment`); - // Get project/workspace path from config session defaults (for monorepo disambiguation) const configSessionDefaults = result.runtime.config.sessionDefaults; const projectPath = configSessionDefaults?.projectPath; const workspacePath = configSessionDefaults?.workspacePath; - // Sync session defaults from Xcode's IDE state const xcodeState = await readXcodeIdeState({ executor, cwd: result.runtime.cwd, @@ -102,18 +99,17 @@ export async function bootstrapServer( if (xcodeState.error) { log('debug', `[xcode] Could not read Xcode IDE state: ${xcodeState.error}`); } else { - const syncedDefaults: Record = {}; - if (xcodeState.scheme) { - syncedDefaults.scheme = xcodeState.scheme; - } - if (xcodeState.simulatorId) { - syncedDefaults.simulatorId = xcodeState.simulatorId; - } - if (xcodeState.simulatorName) { - syncedDefaults.simulatorName = xcodeState.simulatorName; - } + const entries = Object.entries({ + scheme: xcodeState.scheme, + simulatorId: xcodeState.simulatorId, + simulatorName: xcodeState.simulatorName, + }).filter( + (entry): entry is [string, string] => + typeof entry[1] === 'string' && entry[1].trim().length > 0, + ); - if (Object.keys(syncedDefaults).length > 0) { + if (entries.length > 0) { + const syncedDefaults = Object.fromEntries(entries); sessionStore.setDefaults(syncedDefaults); log( 'info', @@ -121,7 +117,6 @@ export async function bootstrapServer( ); } - // Look up bundle ID asynchronously (non-blocking) if (xcodeState.scheme) { lookupBundleId(executor, xcodeState.scheme, projectPath, workspacePath) .then((bundleId) => { @@ -136,7 +131,6 @@ export async function bootstrapServer( } } - // Start file watcher to auto-sync when user changes scheme/simulator in Xcode if (!result.runtime.config.disableXcodeAutoSync) { const watcherStarted = await startXcodeStateWatcher({ executor, @@ -153,16 +147,14 @@ export async function bootstrapServer( } } - // Build predicate context for manifest-based registration const ctx: PredicateContext = { runtime: 'mcp', config: result.runtime.config, runningUnderXcode: xcodeDetection.runningUnderXcode, - xcodeToolsActive: false, // Will be updated after Xcode tools bridge sync + xcodeToolsActive: false, xcodeToolsAvailable, }; - // Register workflows using manifest system await registerWorkflowsFromManifest(enabledWorkflows, ctx); const resolvedWorkflows = getRegisteredWorkflows(); @@ -195,21 +187,21 @@ export async function bootstrapServer( const xcodeVersion = await getXcodeVersionMetadata(async (command) => { const commandResult = await executor(command, 'Get Xcode Version'); - return { success: commandResult.success, output: commandResult.output ?? '' }; + return { success: commandResult.success, output: commandResult.output }; }); - const xcodeAvailable = [ - xcodeVersion.version, - xcodeVersion.buildVersion, - xcodeVersion.developerDir, - xcodeVersion.xcodebuildPath, - ].some((value) => Boolean(value)); + const xcodeAvailable = Boolean( + xcodeVersion.version || + xcodeVersion.buildVersion || + xcodeVersion.developerDir || + xcodeVersion.xcodebuildPath, + ); const axeBinary = resolveAxeBinary(); const axeAvailable = axeBinary !== null; const axeSource = axeBinary?.source ?? 'unavailable'; const xcodemakeAvailable = isXcodemakeBinaryAvailable(); const axeVersion = await getAxeVersionMetadata(async (command) => { const commandResult = await executor(command, 'Get AXe Version'); - return { success: commandResult.success, output: commandResult.output ?? '' }; + return { success: commandResult.success, output: commandResult.output }; }, axeBinary?.path); setSentryRuntimeContext({ mode: 'mcp', diff --git a/src/server/server.ts b/src/server/server.ts index 4bc514cf..acb3653b 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -67,7 +67,6 @@ export function createServer(): McpServer { if (getServer()) { throw new Error('MCP server already initialized.'); } - // Create server instance const baseServer = createBaseServerInstance(); const server = Sentry.wrapMcpServerWithSentry(baseServer, { recordInputs: false, @@ -76,7 +75,6 @@ export function createServer(): McpServer { setServer(server); - // Log server initialization log('info', `Server initialized (version ${version})`); return server; diff --git a/src/utils/logger.ts b/src/utils/logger.ts index 880ed351..87afaa75 100644 --- a/src/utils/logger.ts +++ b/src/utils/logger.ts @@ -20,7 +20,6 @@ import { createWriteStream, type WriteStream } from 'node:fs'; import { createRequire } from 'node:module'; import { resolve } from 'node:path'; -// Note: Removed "import * as Sentry from '@sentry/node'" to prevent native module loading at import time function isSentryDisabledFromEnv(): boolean { return ( @@ -79,24 +78,29 @@ const require = createRequire( let cachedSentry: SentryModule | null = null; function loadSentrySync(): SentryModule | null { - if (!sentryEnabled || isTestEnv()) return null; - if (cachedSentry) return cachedSentry; + if (!sentryEnabled || isTestEnv()) { + return null; + } + if (cachedSentry) { + return cachedSentry; + } try { cachedSentry = require('@sentry/node') as SentryModule; return cachedSentry; } catch { - // If @sentry/node is not installed in some environments, fail silently. return null; } } function withSentry(cb: (s: SentryModule) => void): void { const s = loadSentrySync(); - if (!s) return; + if (!s) { + return; + } try { cb(s); } catch { - // no-op: avoid throwing inside logger + // Avoid throwing inside logger } } @@ -193,7 +197,6 @@ export function getLogLevel(): LogLevel { * @returns true if the message should be logged */ function shouldLog(level: string): boolean { - // Suppress logging during tests to keep test output clean if (isTestEnv() && !logFileStream) { return false; } @@ -202,13 +205,11 @@ function shouldLog(level: string): boolean { return false; } - // Check if the level is valid const levelKey = level.toLowerCase() as LogLevel; if (!(levelKey in LOG_LEVELS)) { - return true; // Log unknown levels + return true; } - // Only log if the message level is at or above the client's requested level return LOG_LEVELS[levelKey] <= LOG_LEVELS[clientLogLevel]; } @@ -222,7 +223,6 @@ export function log(level: string, message: string, context?: LogContext): void const timestamp = new Date().toISOString(); const logMessage = `[${timestamp}] [${level.toUpperCase()}] ${message}`; - // Sentry capture is explicit opt-in only. const captureToSentry = sentryEnabled && __shouldCaptureToSentryForTests(context); if (captureToSentry) { @@ -245,12 +245,11 @@ export function log(level: string, message: string, context?: LogContext): void } } - // Check if we should log this level to stderr if (!shouldLog(level)) { return; } - // It's important to use console.error here to ensure logs don't interfere with MCP protocol communication - // see https://modelcontextprotocol.io/docs/tools/debugging#server-side-logging + // Uses stderr to avoid interfering with MCP protocol on stdout + // https://modelcontextprotocol.io/docs/tools/debugging#server-side-logging console.error(logMessage); } diff --git a/src/utils/sentry.ts b/src/utils/sentry.ts index 71918a9f..d4860bef 100644 --- a/src/utils/sentry.ts +++ b/src/utils/sentry.ts @@ -133,7 +133,7 @@ export function __redactLogForTests(log: Sentry.Log): Sentry.Log | null { return redactLog(clone); } -export function __parseXcodeVersionForTests(output: string): { +function parseXcodeVersionOutput(output: string): { version?: string; buildVersion?: string; } { @@ -145,6 +145,13 @@ export function __parseXcodeVersionForTests(output: string): { }; } +export function __parseXcodeVersionForTests(output: string): { + version?: string; + buildVersion?: string; +} { + return parseXcodeVersionOutput(output); +} + let initialized = false; function isSentryDisabled(): boolean { @@ -191,7 +198,7 @@ function boolToTag(value: boolean | undefined): string | undefined { if (value === undefined) { return undefined; } - return value ? 'true' : 'false'; + return String(value); } function setTagIfDefined(key: string, value: string | undefined): void { @@ -233,25 +240,22 @@ export function setSentryRuntimeContext(context: SentryRuntimeContext): void { } } -export async function getXcodeVersionMetadata( - runCommand: (command: string[]) => Promise<{ success: boolean; output: string }>, -): Promise<{ +interface XcodeVersionMetadata { version?: string; buildVersion?: string; developerDir?: string; xcodebuildPath?: string; -}> { - const metadata: { - version?: string; - buildVersion?: string; - developerDir?: string; - xcodebuildPath?: string; - } = {}; +} + +export async function getXcodeVersionMetadata( + runCommand: (command: string[]) => Promise<{ success: boolean; output: string }>, +): Promise { + const metadata: XcodeVersionMetadata = {}; try { const result = await runCommand(['xcodebuild', '-version']); if (result.success) { - const parsed = __parseXcodeVersionForTests(result.output); + const parsed = parseXcodeVersionOutput(result.output); metadata.version = parsed.version; metadata.buildVersion = parsed.buildVersion; } diff --git a/src/utils/tool-registry.ts b/src/utils/tool-registry.ts index e62fab9d..a5d566a5 100644 --- a/src/utils/tool-registry.ts +++ b/src/utils/tool-registry.ts @@ -40,16 +40,7 @@ export function getRegisteredWorkflows(): string[] { return [...registryState.enabledWorkflows]; } -/** - * Get the current MCP predicate context. - * Returns the context used for the most recent workflow registration, - * or a default context if not yet initialized. - */ -export function getMcpPredicateContext(): PredicateContext { - if (registryState.currentContext) { - return registryState.currentContext; - } - // Default context when not yet initialized +function defaultPredicateContext(): PredicateContext { return { runtime: 'mcp', config: getConfig(), @@ -59,6 +50,15 @@ export function getMcpPredicateContext(): PredicateContext { }; } +/** + * Get the current MCP predicate context. + * Returns the context used for the most recent workflow registration, + * or a default context if not yet initialized. + */ +export function getMcpPredicateContext(): PredicateContext { + return registryState.currentContext ?? defaultPredicateContext(); +} + /** * Apply workflow selection using the manifest system. */ @@ -103,7 +103,7 @@ export async function applyWorkflowSelectionFromManifest( try { toolModule = await importToolModule(toolManifest.module); } catch (err) { - log('warn', `Failed to import tool module ${toolManifest.module}: ${err}`); + log('warning', `Failed to import tool module ${toolManifest.module}: ${err}`); continue; } @@ -174,14 +174,7 @@ export async function registerWorkflowsFromManifest( workflowNames?: string[], ctx?: PredicateContext, ): Promise { - const effectiveCtx: PredicateContext = ctx ?? { - runtime: 'mcp', - config: getConfig(), - runningUnderXcode: false, - xcodeToolsActive: false, - xcodeToolsAvailable: false, - }; - await applyWorkflowSelectionFromManifest(workflowNames, effectiveCtx); + await applyWorkflowSelectionFromManifest(workflowNames, ctx ?? defaultPredicateContext()); } /** diff --git a/src/utils/xcodemake.ts b/src/utils/xcodemake.ts index d813581b..3d9f8954 100644 --- a/src/utils/xcodemake.ts +++ b/src/utils/xcodemake.ts @@ -153,16 +153,15 @@ export async function isXcodemakeAvailable(): Promise { return true; } - // If not found, download and install it log('info', 'xcodemake not found in PATH, attempting to download...'); const installed = await installXcodemake(); - if (installed) { - log('info', 'xcodemake installed successfully'); - return true; - } else { - log('warn', 'xcodemake installation failed'); + if (!installed) { + log('warning', 'xcodemake installation failed'); return false; } + + log('info', 'xcodemake installed successfully'); + return true; } catch (error) { log( 'error', diff --git a/tsup.config.ts b/tsup.config.ts index f4c2a1d1..600fec47 100644 --- a/tsup.config.ts +++ b/tsup.config.ts @@ -19,7 +19,7 @@ function rewriteTsImportsInDir(dir: string): void { // Also handles: export ... from "./path.ts" const rewritten = content.replace( /((?:import|export)[^'"]*['"])([^'"]+)(\.ts)(['"])/g, - '$1$2.js$4' + '$1$2.js$4', ); if (rewritten !== content) { writeFileSync(fullPath, rewritten, 'utf-8'); diff --git a/vitest.smoke.config.ts b/vitest.smoke.config.ts index 6a4e494b..2a881182 100644 --- a/vitest.smoke.config.ts +++ b/vitest.smoke.config.ts @@ -4,9 +4,7 @@ export default defineConfig({ test: { environment: 'node', globals: true, - include: [ - 'src/smoke-tests/__tests__/**/*.test.ts', - ], + include: ['src/smoke-tests/__tests__/**/*.test.ts'], pool: 'forks', poolOptions: { forks: {