-
-
Notifications
You must be signed in to change notification settings - Fork 163
Add lldb + improve session defaults #161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
commit: |
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces a comprehensive debugging subsystem for iOS simulators. It adds a DAP-based debugger backend alongside the existing LLDB CLI backend, with real transport and adapter discovery via lldb-dap. A new DebuggerManager coordinates session lifecycle, breakpoint management, and command execution. Eight new MCP tools enable debugging workflows: attach, detach, breakpoint add/remove, continue, stack, variables, and LLDB command execution. Integration with UI automation guards prevents tool execution when a debugger is paused. Supporting infrastructure includes dependency injection for testability, a session-status resource for runtime state, and documentation updates. Build configuration migrates dynamic imports from JavaScript to TypeScript paths. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (81)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (23)
src/mcp/tools/simulator/list_sims.ts (1)
192-194: LGTM! Helpful hint added to guide users.The hint appropriately suggests saving simulator defaults and helpfully mentions both
simulatorIdandsimulatorNameoptions. The implementation is correct and improves user experience.💡 Optional: Consider adding visual separation
For slightly better readability, you could add an extra newline before the hint to visually separate it from the numbered steps:
responseText += - "4. Get app path: get_sim_app_path({ scheme: 'YOUR_SCHEME', platform: 'iOS Simulator', simulatorId: 'UUID_FROM_ABOVE' })\n"; + "4. Get app path: get_sim_app_path({ scheme: 'YOUR_SCHEME', platform: 'iOS Simulator', simulatorId: 'UUID_FROM_ABOVE' })\n\n"; responseText += "Hint: Save a default simulator with session-set-defaults { simulatorId: 'UUID_FROM_ABOVE' } (or simulatorName).";However, the current formatting is perfectly acceptable.
src/mcp/tools/project-discovery/list_schemes.ts (1)
39-39: Good refactor for consistency.The
createTextBlockhelper standardises text block creation and reduces duplication. The const assertion ensures type safety.src/mcp/tools/session-management/__tests__/session_set_defaults.test.ts (1)
104-128: Consider adding edge case test coverage.Whilst the current tests cover the expected behaviour well, consider adding a test case for explicitly passed
undefinedvalues (e.g.,{projectPath: undefined, workspacePath: '/path'}). This would clarify whether such values should trigger clearing logic or be ignored.🔎 Example test case
it('should not clear mutually exclusive fields when undefined is explicitly passed', async () => { sessionStore.setDefaults({ workspacePath: '/app/App.xcworkspace' }); const result = await sessionSetDefaultsLogic({ projectPath: undefined, scheme: 'MyScheme' }); const current = sessionStore.getAll(); expect(current.workspacePath).toBe('/app/App.xcworkspace'); expect(current.scheme).toBe('MyScheme'); expect(result.content[0].text).not.toContain('Cleared'); });src/mcp/tools/doctor/lib/doctor.deps.ts (1)
100-100: Consider simplifying the pass-through.The intermediate variable assignment is redundant. You could directly return
executorascommandExecutorin the return statement on line 281.🔎 Proposed simplification
Remove line 100 and update line 281:
export function createDoctorDependencies(executor: CommandExecutor): DoctorDependencies { - const commandExecutor = executor; const binaryChecker: BinaryChecker = {Then on line 281:
- return { commandExecutor, binaryChecker, xcode, env, plugins, runtime, features }; + return { commandExecutor: executor, binaryChecker, xcode, env, plugins, runtime, features };src/index.ts (1)
67-77: Consider wrapping disposal in try-catch for robustness.The debugger disposal calls are correctly placed before
server.close(). However, ifdisposeAll()throws an error, it could prevent the process from exiting cleanly. Consider wrapping in a try-catch to ensure graceful shutdown:🔎 Proposed defensive error handling
process.on('SIGTERM', async () => { + try { await getDefaultDebuggerManager().disposeAll(); + } catch (err) { + console.error('Error disposing debugger sessions:', err); + } await server.close(); process.exit(0); }); process.on('SIGINT', async () => { + try { await getDefaultDebuggerManager().disposeAll(); + } catch (err) { + console.error('Error disposing debugger sessions:', err); + } await server.close(); process.exit(0); });src/mcp/tools/ui-testing/__tests__/describe_ui.test.ts (1)
11-13: Unused variable could be removed.The
mockCallsvariable is declared and reset but never used in any test assertions. Consider removing it to keep the test file clean.🔎 Proposed cleanup
describe('Describe UI Plugin', () => { - let mockCalls: any[] = []; - - mockCalls = []; describe('Export Field Validation (Literal)', () => {src/utils/debugger/__tests__/debugger-manager-dap.test.ts (1)
28-51: Consider usingafterEachfor environment variable cleanup.The manual cleanup of
process.env.XCODEBUILDMCP_DEBUGGER_BACKEND(lines 46-50) won't run if the test fails before reaching those lines. Consider moving the cleanup to anafterEachhook or wrapping the test body in a try/finally block to ensure the environment is always restored.🔎 Proposed refactor using afterEach
describe('DebuggerManager DAP selection', () => { + const prevEnvKey = 'XCODEBUILDMCP_DEBUGGER_BACKEND'; + let prevEnv: string | undefined; + + afterEach(() => { + if (prevEnv === undefined) { + delete process.env[prevEnvKey]; + } else { + process.env[prevEnvKey] = prevEnv; + } + }); + it('selects dap backend when env is set', async () => { - const prevEnv = process.env.XCODEBUILDMCP_DEBUGGER_BACKEND; + prevEnv = process.env[prevEnvKey]; process.env.XCODEBUILDMCP_DEBUGGER_BACKEND = 'dap'; let selected: string | null = null; const backend = createBackend({ kind: 'dap' }); const manager = new DebuggerManager({ backendFactory: async (kind) => { selected = kind; return backend; }, }); await manager.createSession({ simulatorId: 'sim-1', pid: 1000 }); expect(selected).toBe('dap'); - - if (prevEnv === undefined) { - delete process.env.XCODEBUILDMCP_DEBUGGER_BACKEND; - } else { - process.env.XCODEBUILDMCP_DEBUGGER_BACKEND = prevEnv; - } });docs/DEBUGGING_ARCHITECTURE.md (1)
1-268: Comprehensive debugging architecture documentationThis documentation provides excellent coverage of the debugging subsystem, including lifecycle flows, backend implementation details, and integration points. The sequence diagrams enhance understanding of the command execution flow.
The static analysis tool flagged a few minor grammatical improvements:
- Line 193: Add comma before "so" → "trim the buffer, so the next command"
- Line 199: Add comma after "prompt" → "next prompt, if present"
- Line 256: Change "is build" → "is to build" or "is building"
These are purely optional style refinements that can be addressed if you're updating the documentation later.
src/mcp/tools/logging/__tests__/start_device_log_cap.test.ts (1)
28-31: Consider removing redundant initialisations.These re-assignments immediately after the declarations are redundant—the arrays are already initialised as empty on lines 24-26. The actual test isolation is handled correctly in
beforeEach.🔎 Proposed fix
let writeFileCalls: Array<{ path: string; content: string }> = []; - // Reset state - commandCalls = []; - mkdirCalls = []; - writeFileCalls = []; - const originalJsonWaitEnv = process.env.XBMCP_LAUNCH_JSON_WAIT_MS;src/utils/debugger/dap/adapter-discovery.ts (1)
10-29: DependencyError may be double-wrapped in the catch block.If
DependencyErroris thrown at lines 13 or 18, the catch block at line 23 will catch it and wrap it again in a newDependencyError. Consider re-throwingDependencyErrorinstances directly.🔎 Proposed fix
} catch (error) { + if (error instanceof DependencyError) { + throw error; + } const message = error instanceof Error ? error.message : String(error); throw new DependencyError( 'DAP backend selected but lldb-dap not found. Ensure Xcode is installed and xcrun can locate lldb-dap, or set XCODEBUILDMCP_DEBUGGER_BACKEND=lldb-cli.', message, ); }src/mcp/tools/ui-testing/describe_ui.ts (1)
38-38: Consider adding type annotation to the Map.The
describeUITimestampsMap lacks explicit type parameters. While TypeScript may infer types from usage, explicit typing improves clarity and catches errors earlier.🔎 Proposed fix
-const describeUITimestamps = new Map(); +const describeUITimestamps = new Map<string, { timestamp: number; simulatorId: string }>();src/mcp/resources/session-status.ts (1)
40-40: Consider usingapplication/jsonfor mimeType.The content returned is JSON-formatted data. Using
application/jsonas the mimeType would be more semantically accurate and could help clients handle the response appropriately.🔎 Proposed fix
- mimeType: 'text/plain', + mimeType: 'application/json',src/mcp/tools/ui-testing/tap.ts (1)
205-214: Handler does not inject debuggerManager for testing.The handler created via
createSessionAwareToolonly passesparamsandexecutortotapLogic, relying on the defaultdebuggerManagerparameter. This works for production but makes it harder to inject a mock debugger manager in tests for this specific code path.Consider whether the handler should support injecting the debugger manager for testability, or if testing via the exported
tapLogicfunction directly with a mock is sufficient.src/mcp/tools/debugging/debug_detach.ts (1)
23-27: Minor inconsistency in session ID resolution.Line 24 resolves
targetIdfor the response message usingparams.debugSessionId ?? ctx.debugger.getCurrentSessionId(), but line 25 passesparams.debugSessionId(potentiallyundefined) todetachSession(). While this works correctly becausedetachSessioninternally resolvesundefinedto the current session, it's slightly confusing to read.Consider passing
targetIdtodetachSessionfor consistency:🔎 Suggested refactor
try { const targetId = params.debugSessionId ?? ctx.debugger.getCurrentSessionId(); - await ctx.debugger.detachSession(params.debugSessionId); + await ctx.debugger.detachSession(targetId ?? undefined); return createTextResponse(`✅ Detached debugger session${targetId ? ` ${targetId}` : ''}.`);src/utils/debugger/debugger-manager.ts (1)
113-125: Sequential cleanup indisposeAllmay be slow with many sessions.The method iterates through sessions sequentially, awaiting each detach/dispose. If performance becomes a concern with multiple sessions, consider parallel cleanup with
Promise.allSettled.🔎 Parallel cleanup alternative
async disposeAll(): Promise<void> { - for (const session of this.sessions.values()) { - try { - await session.backend.detach(); - } catch { - // Best-effort cleanup; detach can fail if the process exited. - } finally { - await session.backend.dispose(); - } - } + await Promise.allSettled( + Array.from(this.sessions.values()).map(async (session) => { + try { + await session.backend.detach(); + } catch { + // Best-effort cleanup; detach can fail if the process exited. + } finally { + await session.backend.dispose(); + } + }), + ); this.sessions.clear(); this.currentSessionId = null; }src/utils/debugger/ui-automation-guard.ts (1)
28-36: Consider logging at 'warn' level for better visibility.The error handling silently returns an empty result after logging at 'debug' level. Whilst this fail-open approach prevents blocking UI automation, it might hide genuine issues such as a corrupted session or backend unavailability. Consider logging at 'warn' level to improve observability in production.
🔎 Proposed fix
} catch (error) { log( - 'debug', + 'warn', `${LOG_PREFIX} ${opts.toolName}: unable to read execution state for ${session.id}: ${String(error)}`, ); return {};src/mcp/tools/debugging/debug_breakpoint_add.ts (1)
44-50: Redundant validation: schema refinements already enforce these constraints.The runtime check at lines 48-50 duplicates validation already performed by the schema refinements (lines 29-34). The schema ensures that when a file-line breakpoint is specified, both
fileandlineare present. This defensive check adds unnecessary code.🔎 Proposed simplification
const spec: BreakpointSpec = params.function ? { kind: 'function', name: params.function } : { kind: 'file-line', file: params.file ?? '', line: params.line ?? 0 }; - if (spec.kind === 'file-line' && (!spec.file || !spec.line)) { - return createErrorResponse('Invalid breakpoint', 'file and line are required.'); - } - const result = await ctx.debugger.addBreakpoint(params.debugSessionId, spec, {Alternatively, if you wish to keep defensive programming, improve the type safety:
const spec: BreakpointSpec = params.function ? { kind: 'function', name: params.function } - : { kind: 'file-line', file: params.file ?? '', line: params.line ?? 0 }; + : { kind: 'file-line', file: params.file!, line: params.line! }; - if (spec.kind === 'file-line' && (!spec.file || !spec.line)) { - return createErrorResponse('Invalid breakpoint', 'file and line are required.'); - }src/mcp/tools/ui-testing/gesture.ts (1)
184-191: Consider passingdebuggerManagertogestureLogicfor testability.The handler invokes
gestureLogicwithout passing adebuggerManager, relying on the default parameter. This works at runtime but may limit the ability to inject a mock debugger manager when testing the handler directly.If handler-level testing with mock debugger managers is desired, consider passing
getDefaultDebuggerManager()explicitly or refactoring to accept it as a parameter.src/utils/debugger/backends/lldb-cli-backend.ts (2)
16-16: Consider adding explicit type annotation forprocessfield.The
processfield lacks a type annotation. While TypeScript can infer the type from the assignment, an explicit annotation would improve code clarity.🔎 Proposed fix
- private readonly process; + private readonly process: ReturnType<InteractiveSpawner>;Or import the
InteractiveProcesstype if available:+import type { InteractiveProcess } from '../../execution/index.ts'; ... - private readonly process; + private readonly process: InteractiveProcess;
210-215: UseCOMMAND_SENTINELconstant instead of hardcoded string in regex.The regex on line 212 uses a hardcoded string
__XCODEBUILDMCP_DONE__instead of theCOMMAND_SENTINELconstant defined on line 9. This creates a maintenance risk if the sentinel value is changed.🔎 Proposed fix
private checkPending(): void { if (!this.pending) return; - const sentinelMatch = this.buffer.match(/(^|\r?\n)__XCODEBUILDMCP_DONE__(\r?\n)/); + const sentinelMatch = this.buffer.match(new RegExp(`(^|\\r?\\n)${COMMAND_SENTINEL}(\\r?\\n)`));Alternatively, if performance is a concern, you could pre-compile the regex as a class property or module-level constant.
src/mcp/tools/debugging/debug_attach_sim.ts (1)
117-125: Success message hardcodes "LLDB" but backend could be "dap".The success message on line 118 says "Attached LLDB to simulator process" but the
createSessionmethod can use either thelldb-cliordapbackend depending on configuration. Consider usingsession.backendto make the message accurate.🔎 Proposed fix
+ const backendLabel = session.backend === 'dap' ? 'DAP debugger' : 'LLDB'; return createTextResponse( - `${warningText}✅ Attached LLDB to simulator process ${pid} (${simulatorId}).\n\n` + + `${warningText}✅ Attached ${backendLabel} to simulator process ${pid} (${simulatorId}).\n\n` + `Debug session ID: ${session.id}\n` +src/utils/execution/interactive-process.ts (1)
56-68: Consider guarding against an empty command array.If
commandis an empty array,executablewill beundefined, causingspawn()to throw a cryptic error. A guard would provide a clearer error message.🔎 Suggested guard
function createInteractiveProcess( command: string[], opts?: SpawnInteractiveOptions, ): InteractiveProcess { + if (command.length === 0) { + throw new Error('Command array must not be empty'); + } const [executable, ...args] = command; const childProcess = spawn(executable, args, {docs/DAP_BACKEND_IMPLEMENTATION_PLAN.md (1)
1-1: Markdown frontmatter may cause rendering issues.Line 1 contains
<chatName="DAP backend plan"/>which appears to be a custom tag (possibly from an AI assistant context). This should be removed as it's not valid Markdown and may render unexpectedly in documentation viewers.🔎 Suggested fix
-<chatName="DAP backend plan"/> - ## Goal & constraints (grounded in current code)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (73)
.smithery/index.cjsdocs/ARCHITECTURE.mddocs/DAP_BACKEND_IMPLEMENTATION_PLAN.mddocs/DEBUGGING_ARCHITECTURE.mddocs/TOOLS.mddocs/investigations/issue-154-screenshot-downscaling.mddocs/investigations/issue-describe-ui-empty-after-debugger-resume.mdexample_projects/iOS_Calculator/CalculatorAppPackage/Sources/CalculatorAppFeature/CalculatorService.swiftpackage.jsonsrc/core/generated-plugins.tssrc/core/generated-resources.tssrc/index.tssrc/mcp/resources/__tests__/session-status.test.tssrc/mcp/resources/session-status.tssrc/mcp/tools/debugging/debug_attach_sim.tssrc/mcp/tools/debugging/debug_breakpoint_add.tssrc/mcp/tools/debugging/debug_breakpoint_remove.tssrc/mcp/tools/debugging/debug_detach.tssrc/mcp/tools/debugging/debug_lldb_command.tssrc/mcp/tools/debugging/debug_stack.tssrc/mcp/tools/debugging/debug_variables.tssrc/mcp/tools/debugging/index.tssrc/mcp/tools/device/__tests__/list_devices.test.tssrc/mcp/tools/device/list_devices.tssrc/mcp/tools/doctor/doctor.tssrc/mcp/tools/doctor/lib/doctor.deps.tssrc/mcp/tools/logging/__tests__/start_device_log_cap.test.tssrc/mcp/tools/logging/__tests__/stop_device_log_cap.test.tssrc/mcp/tools/logging/start_device_log_cap.tssrc/mcp/tools/logging/stop_device_log_cap.tssrc/mcp/tools/project-discovery/__tests__/discover_projs.test.tssrc/mcp/tools/project-discovery/__tests__/list_schemes.test.tssrc/mcp/tools/project-discovery/discover_projs.tssrc/mcp/tools/project-discovery/list_schemes.tssrc/mcp/tools/session-management/__tests__/session_set_defaults.test.tssrc/mcp/tools/session-management/session_set_defaults.tssrc/mcp/tools/simulator/__tests__/list_sims.test.tssrc/mcp/tools/simulator/list_sims.tssrc/mcp/tools/ui-testing/__tests__/describe_ui.test.tssrc/mcp/tools/ui-testing/button.tssrc/mcp/tools/ui-testing/describe_ui.tssrc/mcp/tools/ui-testing/gesture.tssrc/mcp/tools/ui-testing/key_press.tssrc/mcp/tools/ui-testing/key_sequence.tssrc/mcp/tools/ui-testing/long_press.tssrc/mcp/tools/ui-testing/swipe.tssrc/mcp/tools/ui-testing/tap.tssrc/mcp/tools/ui-testing/touch.tssrc/mcp/tools/ui-testing/type_text.tssrc/test-utils/mock-executors.tssrc/utils/__tests__/debugger-simctl.test.tssrc/utils/debugger/__tests__/debugger-manager-dap.test.tssrc/utils/debugger/backends/DebuggerBackend.tssrc/utils/debugger/backends/__tests__/dap-backend.test.tssrc/utils/debugger/backends/dap-backend.tssrc/utils/debugger/backends/lldb-cli-backend.tssrc/utils/debugger/dap/__tests__/transport-framing.test.tssrc/utils/debugger/dap/adapter-discovery.tssrc/utils/debugger/dap/transport.tssrc/utils/debugger/dap/types.tssrc/utils/debugger/debugger-manager.tssrc/utils/debugger/index.tssrc/utils/debugger/simctl.tssrc/utils/debugger/tool-context.tssrc/utils/debugger/types.tssrc/utils/debugger/ui-automation-guard.tssrc/utils/environment.tssrc/utils/execution/index.tssrc/utils/execution/interactive-process.tssrc/utils/log-capture/device-log-sessions.tssrc/utils/log-capture/index.tssrc/utils/session-status.tssrc/utils/typed-tool-factory.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Use.tsfile extensions for all relative imports of internal TypeScript files (e.g.,import { tool } from './tool.ts')
Use.tsextensions for re-exports from internal modules (e.g.,export { default } from '../shared/tool.ts')
Use.jsextensions only for external package imports (e.g.,import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js')
Never use.jsextensions for imports of internal TypeScript files
Files:
src/mcp/resources/__tests__/session-status.test.tssrc/mcp/tools/device/__tests__/list_devices.test.tssrc/mcp/tools/project-discovery/__tests__/discover_projs.test.tssrc/index.tssrc/mcp/tools/doctor/doctor.tssrc/utils/log-capture/device-log-sessions.tssrc/utils/execution/index.tssrc/mcp/tools/logging/__tests__/stop_device_log_cap.test.tssrc/mcp/tools/debugging/debug_lldb_command.tssrc/mcp/tools/ui-testing/type_text.tssrc/mcp/tools/debugging/debug_detach.tssrc/utils/debugger/tool-context.tssrc/utils/debugger/simctl.tssrc/mcp/tools/debugging/debug_breakpoint_add.tssrc/utils/debugger/backends/__tests__/dap-backend.test.tssrc/mcp/tools/ui-testing/__tests__/describe_ui.test.tssrc/test-utils/mock-executors.tssrc/utils/debugger/dap/__tests__/transport-framing.test.tssrc/utils/debugger/dap/types.tssrc/mcp/tools/simulator/list_sims.tssrc/mcp/tools/ui-testing/tap.tssrc/mcp/tools/project-discovery/list_schemes.tssrc/utils/session-status.tssrc/utils/log-capture/index.tssrc/mcp/tools/doctor/lib/doctor.deps.tssrc/utils/debugger/dap/transport.tssrc/utils/__tests__/debugger-simctl.test.tssrc/utils/debugger/backends/dap-backend.tssrc/mcp/tools/ui-testing/swipe.tssrc/mcp/tools/project-discovery/discover_projs.tssrc/mcp/tools/debugging/debug_variables.tssrc/utils/execution/interactive-process.tssrc/mcp/tools/debugging/index.tssrc/core/generated-resources.tssrc/mcp/tools/session-management/__tests__/session_set_defaults.test.tssrc/utils/debugger/debugger-manager.tssrc/mcp/tools/logging/__tests__/start_device_log_cap.test.tssrc/core/generated-plugins.tssrc/utils/debugger/backends/lldb-cli-backend.tssrc/utils/debugger/index.tssrc/mcp/tools/ui-testing/key_press.tssrc/mcp/tools/logging/start_device_log_cap.tssrc/utils/debugger/ui-automation-guard.tssrc/mcp/tools/ui-testing/gesture.tssrc/mcp/tools/ui-testing/long_press.tssrc/utils/debugger/__tests__/debugger-manager-dap.test.tssrc/utils/debugger/dap/adapter-discovery.tssrc/mcp/tools/debugging/debug_attach_sim.tssrc/mcp/tools/debugging/debug_stack.tssrc/mcp/tools/ui-testing/key_sequence.tssrc/mcp/tools/logging/stop_device_log_cap.tssrc/mcp/tools/device/list_devices.tssrc/mcp/tools/debugging/debug_breakpoint_remove.tssrc/mcp/resources/session-status.tssrc/utils/environment.tssrc/utils/debugger/backends/DebuggerBackend.tssrc/mcp/tools/ui-testing/describe_ui.tssrc/utils/debugger/types.tssrc/mcp/tools/session-management/session_set_defaults.tssrc/mcp/tools/simulator/__tests__/list_sims.test.tssrc/mcp/tools/ui-testing/touch.tssrc/mcp/tools/ui-testing/button.tssrc/mcp/tools/project-discovery/__tests__/list_schemes.test.tssrc/utils/typed-tool-factory.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Do NOT use Vitest mocking utilities (vi.mock(), vi.fn(), vi.spyOn(), etc.) - use Dependency Injection with injectable executors instead
Import core Logic functions from tool files and pass mock executors (createMockExecutor or createMockFileSystemExecutor) to simulate different outcomes
Files:
src/mcp/resources/__tests__/session-status.test.tssrc/mcp/tools/device/__tests__/list_devices.test.tssrc/mcp/tools/project-discovery/__tests__/discover_projs.test.tssrc/mcp/tools/logging/__tests__/stop_device_log_cap.test.tssrc/utils/debugger/backends/__tests__/dap-backend.test.tssrc/mcp/tools/ui-testing/__tests__/describe_ui.test.tssrc/utils/debugger/dap/__tests__/transport-framing.test.tssrc/utils/__tests__/debugger-simctl.test.tssrc/mcp/tools/session-management/__tests__/session_set_defaults.test.tssrc/mcp/tools/logging/__tests__/start_device_log_cap.test.tssrc/utils/debugger/__tests__/debugger-manager-dap.test.tssrc/mcp/tools/simulator/__tests__/list_sims.test.tssrc/mcp/tools/project-discovery/__tests__/list_schemes.test.ts
🧠 Learnings (5)
📚 Learning: 2025-12-31T21:42:22.713Z
Learnt from: CR
Repo: cameroncooke/XcodeBuildMCP PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-31T21:42:22.713Z
Learning: Applies to **/*.test.ts : Import core Logic functions from tool files and pass mock executors (createMockExecutor or createMockFileSystemExecutor) to simulate different outcomes
Applied to files:
src/utils/execution/index.tssrc/mcp/tools/logging/__tests__/stop_device_log_cap.test.tssrc/mcp/tools/ui-testing/type_text.tssrc/utils/debugger/tool-context.tssrc/utils/debugger/backends/__tests__/dap-backend.test.tssrc/test-utils/mock-executors.tssrc/utils/debugger/dap/__tests__/transport-framing.test.tssrc/mcp/tools/ui-testing/tap.tssrc/utils/log-capture/index.tssrc/utils/__tests__/debugger-simctl.test.tssrc/mcp/tools/ui-testing/swipe.tssrc/utils/execution/interactive-process.tssrc/mcp/tools/logging/__tests__/start_device_log_cap.test.tssrc/utils/debugger/index.tssrc/mcp/tools/ui-testing/key_press.tsdocs/ARCHITECTURE.mdsrc/mcp/tools/ui-testing/gesture.tssrc/mcp/tools/ui-testing/long_press.tssrc/utils/debugger/__tests__/debugger-manager-dap.test.tssrc/mcp/tools/ui-testing/key_sequence.tssrc/mcp/tools/logging/stop_device_log_cap.tssrc/mcp/tools/ui-testing/describe_ui.tssrc/mcp/tools/simulator/__tests__/list_sims.test.tssrc/mcp/tools/ui-testing/touch.tssrc/mcp/tools/ui-testing/button.ts
📚 Learning: 2025-12-31T21:42:22.713Z
Learnt from: CR
Repo: cameroncooke/XcodeBuildMCP PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-31T21:42:22.713Z
Learning: Use CommandExecutor for handling external shell command interactions in tools and tests
Applied to files:
src/utils/execution/index.tssrc/test-utils/mock-executors.ts
📚 Learning: 2025-12-31T21:42:22.713Z
Learnt from: CR
Repo: cameroncooke/XcodeBuildMCP PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-31T21:42:22.713Z
Learning: Applies to **/*.ts : Use `.ts` extensions for re-exports from internal modules (e.g., `export { default } from '../shared/tool.ts'`)
Applied to files:
src/utils/execution/index.tspackage.json
📚 Learning: 2025-12-31T21:42:22.713Z
Learnt from: CR
Repo: cameroncooke/XcodeBuildMCP PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-31T21:42:22.713Z
Learning: Applies to **/*.test.ts : Do NOT use Vitest mocking utilities (vi.mock(), vi.fn(), vi.spyOn(), etc.) - use Dependency Injection with injectable executors instead
Applied to files:
src/test-utils/mock-executors.tssrc/utils/__tests__/debugger-simctl.test.tssrc/mcp/tools/logging/__tests__/start_device_log_cap.test.ts
📚 Learning: 2025-12-31T21:42:22.713Z
Learnt from: CR
Repo: cameroncooke/XcodeBuildMCP PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-31T21:42:22.713Z
Learning: Set XCODEBUILDMCP_ENABLED_WORKFLOWS environment variable to a comma-separated list of workflow directory names to limit the toolset (e.g., 'simulator,project-discovery'). The 'session-management' workflow is always auto-included
Applied to files:
src/mcp/tools/debugging/index.tssrc/core/generated-plugins.tsdocs/ARCHITECTURE.mddocs/TOOLS.md
🧬 Code graph analysis (34)
src/mcp/resources/__tests__/session-status.test.ts (4)
src/utils/log_capture.ts (1)
activeLogSessions(24-24)src/utils/log-capture/device-log-sessions.ts (1)
activeDeviceLogSessions(13-13)src/utils/debugger/index.ts (1)
getDefaultDebuggerManager(5-8)src/mcp/resources/session-status.ts (1)
sessionStatusResourceLogic(10-34)
src/index.ts (1)
src/utils/debugger/index.ts (1)
getDefaultDebuggerManager(5-8)
src/mcp/tools/debugging/debug_lldb_command.ts (4)
src/utils/schema-helpers.ts (1)
nullifyEmptyStrings(14-24)src/utils/debugger/index.ts (2)
DebuggerToolContext(22-22)getDefaultDebuggerToolContext(11-11)src/utils/debugger/tool-context.ts (2)
DebuggerToolContext(6-9)getDefaultDebuggerToolContext(11-16)src/utils/typed-tool-factory.ts (1)
createTypedToolWithContext(64-70)
src/mcp/tools/ui-testing/type_text.ts (4)
src/utils/debugger/debugger-manager.ts (1)
DebuggerManager(15-196)src/utils/debugger/index.ts (3)
DebuggerManager(10-10)getDefaultDebuggerManager(5-8)guardUiAutomationAgainstStoppedDebugger(13-13)src/utils/responses/index.ts (2)
ToolResponse(15-15)createTextResponse(5-5)src/utils/debugger/ui-automation-guard.ts (1)
guardUiAutomationAgainstStoppedDebugger(15-59)
src/mcp/tools/debugging/debug_detach.ts (3)
src/utils/debugger/index.ts (2)
DebuggerToolContext(22-22)getDefaultDebuggerToolContext(11-11)src/utils/debugger/tool-context.ts (2)
DebuggerToolContext(6-9)getDefaultDebuggerToolContext(11-16)src/utils/typed-tool-factory.ts (1)
createTypedToolWithContext(64-70)
src/utils/debugger/simctl.ts (2)
src/utils/debugger/index.ts (1)
resolveSimulatorAppPid(12-12)src/utils/execution/index.ts (1)
CommandExecutor(9-9)
src/mcp/tools/debugging/debug_breakpoint_add.ts (4)
src/utils/schema-helpers.ts (1)
nullifyEmptyStrings(14-24)src/utils/debugger/tool-context.ts (2)
DebuggerToolContext(6-9)getDefaultDebuggerToolContext(11-16)src/utils/debugger/types.ts (1)
BreakpointSpec(12-14)src/utils/typed-tool-factory.ts (1)
createTypedToolWithContext(64-70)
src/utils/debugger/backends/__tests__/dap-backend.test.ts (3)
src/utils/debugger/dap/types.ts (3)
DapEvent(18-23)DapRequest(1-6)DapResponse(8-16)src/utils/debugger/backends/dap-backend.ts (1)
request(377-390)src/test-utils/mock-executors.ts (3)
createMockInteractiveSpawner(180-257)MockInteractiveSession(165-171)createMockExecutor(30-78)
src/utils/debugger/dap/__tests__/transport-framing.test.ts (3)
src/utils/debugger/dap/types.ts (2)
DapResponse(8-16)DapEvent(18-23)src/test-utils/mock-executors.ts (2)
MockInteractiveSession(165-171)createMockInteractiveSpawner(180-257)src/utils/debugger/dap/transport.ts (1)
DapTransport(23-212)
src/utils/session-status.ts (3)
src/utils/debugger/index.ts (1)
getDefaultDebuggerManager(5-8)src/utils/log-capture/index.ts (1)
listActiveSimulatorLogSessionIds(3-5)src/utils/log-capture/device-log-sessions.ts (1)
activeDeviceLogSessions(13-13)
src/utils/log-capture/index.ts (1)
src/utils/log_capture.ts (1)
activeLogSessions(24-24)
src/mcp/tools/doctor/lib/doctor.deps.ts (1)
src/utils/execution/index.ts (1)
CommandExecutor(9-9)
src/utils/debugger/dap/transport.ts (2)
src/utils/debugger/backends/dap-backend.ts (1)
request(377-390)src/utils/debugger/dap/types.ts (3)
DapRequest(1-6)DapEvent(18-23)DapResponse(8-16)
src/utils/__tests__/debugger-simctl.test.ts (2)
src/test-utils/mock-executors.ts (1)
createMockExecutor(30-78)src/utils/debugger/simctl.ts (1)
resolveSimulatorAppPid(3-38)
src/mcp/tools/ui-testing/swipe.ts (4)
src/utils/debugger/debugger-manager.ts (1)
DebuggerManager(15-196)src/utils/debugger/index.ts (3)
DebuggerManager(10-10)getDefaultDebuggerManager(5-8)guardUiAutomationAgainstStoppedDebugger(13-13)src/utils/responses/index.ts (2)
ToolResponse(15-15)createTextResponse(5-5)src/utils/debugger/ui-automation-guard.ts (1)
guardUiAutomationAgainstStoppedDebugger(15-59)
src/mcp/tools/project-discovery/discover_projs.ts (2)
scripts/check-code-patterns.js (1)
results(569-569)src/types/common.ts (1)
createTextContent(57-59)
src/mcp/tools/debugging/debug_variables.ts (3)
src/utils/debugger/index.ts (2)
DebuggerToolContext(22-22)getDefaultDebuggerToolContext(11-11)src/utils/debugger/tool-context.ts (2)
DebuggerToolContext(6-9)getDefaultDebuggerToolContext(11-16)src/utils/typed-tool-factory.ts (1)
createTypedToolWithContext(64-70)
src/utils/execution/interactive-process.ts (1)
src/utils/execution/index.ts (4)
InteractiveProcess(12-12)SpawnInteractiveOptions(14-14)InteractiveSpawner(13-13)getDefaultInteractiveSpawner(6-6)
src/mcp/tools/session-management/__tests__/session_set_defaults.test.ts (1)
src/utils/session-store.ts (1)
sessionStore(48-48)
src/utils/debugger/backends/lldb-cli-backend.ts (4)
src/utils/debugger/backends/DebuggerBackend.ts (1)
DebuggerBackend(3-19)src/utils/execution/index.ts (2)
InteractiveSpawner(13-13)getDefaultInteractiveSpawner(6-6)src/utils/execution/interactive-process.ts (2)
InteractiveSpawner(15-18)getDefaultInteractiveSpawner(70-80)src/utils/debugger/types.ts (3)
BreakpointSpec(12-14)BreakpointInfo(16-20)DebugExecutionState(24-29)
src/utils/debugger/index.ts (1)
src/utils/debugger/debugger-manager.ts (1)
DebuggerManager(15-196)
src/mcp/tools/ui-testing/gesture.ts (4)
src/utils/debugger/debugger-manager.ts (1)
DebuggerManager(15-196)src/utils/debugger/index.ts (3)
DebuggerManager(10-10)getDefaultDebuggerManager(5-8)guardUiAutomationAgainstStoppedDebugger(13-13)src/utils/responses/index.ts (2)
ToolResponse(15-15)createTextResponse(5-5)src/utils/debugger/ui-automation-guard.ts (1)
guardUiAutomationAgainstStoppedDebugger(15-59)
src/mcp/tools/ui-testing/long_press.ts (3)
src/utils/debugger/debugger-manager.ts (1)
DebuggerManager(15-196)src/utils/debugger/index.ts (3)
DebuggerManager(10-10)getDefaultDebuggerManager(5-8)guardUiAutomationAgainstStoppedDebugger(13-13)src/utils/debugger/ui-automation-guard.ts (1)
guardUiAutomationAgainstStoppedDebugger(15-59)
src/utils/debugger/__tests__/debugger-manager-dap.test.ts (1)
src/utils/debugger/backends/DebuggerBackend.ts (1)
DebuggerBackend(3-19)
src/utils/debugger/dap/adapter-discovery.ts (1)
src/mcp/tools/doctor/lib/doctor.deps.ts (1)
CommandExecutor(284-284)
src/mcp/tools/debugging/debug_attach_sim.ts (6)
src/utils/schema-helpers.ts (1)
nullifyEmptyStrings(14-24)src/utils/debugger/index.ts (3)
DebuggerToolContext(22-22)resolveSimulatorAppPid(12-12)getDefaultDebuggerToolContext(11-11)src/utils/debugger/tool-context.ts (2)
DebuggerToolContext(6-9)getDefaultDebuggerToolContext(11-16)src/utils/simulator-utils.ts (1)
determineSimulatorUuid(27-142)src/utils/debugger/simctl.ts (1)
resolveSimulatorAppPid(3-38)src/utils/typed-tool-factory.ts (2)
getSessionAwareToolSchemaShape(99-104)createSessionAwareToolWithContext(122-130)
src/mcp/tools/debugging/debug_stack.ts (3)
src/utils/debugger/index.ts (2)
DebuggerToolContext(22-22)getDefaultDebuggerToolContext(11-11)src/utils/debugger/tool-context.ts (2)
DebuggerToolContext(6-9)getDefaultDebuggerToolContext(11-16)src/utils/typed-tool-factory.ts (1)
createTypedToolWithContext(64-70)
src/mcp/tools/debugging/debug_breakpoint_remove.ts (3)
src/utils/debugger/index.ts (2)
DebuggerToolContext(22-22)getDefaultDebuggerToolContext(11-11)src/utils/debugger/tool-context.ts (2)
DebuggerToolContext(6-9)getDefaultDebuggerToolContext(11-16)src/utils/typed-tool-factory.ts (1)
createTypedToolWithContext(64-70)
src/mcp/resources/session-status.ts (1)
src/utils/session-status.ts (1)
getSessionRuntimeStatusSnapshot(16-37)
src/utils/debugger/backends/DebuggerBackend.ts (1)
src/utils/debugger/types.ts (3)
BreakpointSpec(12-14)BreakpointInfo(16-20)DebugExecutionState(24-29)
src/mcp/tools/ui-testing/describe_ui.ts (4)
src/utils/debugger/debugger-manager.ts (1)
DebuggerManager(15-196)src/utils/debugger/index.ts (3)
DebuggerManager(10-10)getDefaultDebuggerManager(5-8)guardUiAutomationAgainstStoppedDebugger(13-13)src/utils/responses/index.ts (1)
ToolResponse(15-15)src/utils/debugger/ui-automation-guard.ts (1)
guardUiAutomationAgainstStoppedDebugger(15-59)
src/mcp/tools/session-management/session_set_defaults.ts (2)
src/types/common.ts (1)
ToolResponse(34-39)src/utils/session-store.ts (2)
sessionStore(48-48)SessionDefaults(3-14)
src/mcp/tools/ui-testing/touch.ts (3)
src/utils/debugger/debugger-manager.ts (1)
DebuggerManager(15-196)src/utils/debugger/index.ts (3)
DebuggerManager(10-10)getDefaultDebuggerManager(5-8)guardUiAutomationAgainstStoppedDebugger(13-13)src/utils/debugger/ui-automation-guard.ts (1)
guardUiAutomationAgainstStoppedDebugger(15-59)
src/mcp/tools/ui-testing/button.ts (3)
src/utils/debugger/debugger-manager.ts (1)
DebuggerManager(15-196)src/utils/debugger/index.ts (3)
DebuggerManager(10-10)getDefaultDebuggerManager(5-8)guardUiAutomationAgainstStoppedDebugger(13-13)src/utils/debugger/ui-automation-guard.ts (1)
guardUiAutomationAgainstStoppedDebugger(15-59)
🪛 LanguageTool
docs/DEBUGGING_ARCHITECTURE.md
[uncategorized] ~23-~23: Loose punctuation mark.
Context: ... other command execution. - debugger: a shared DebuggerManager instance. #...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~193-~193: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: .... - Prompt is used to trim the buffer so the next command starts cleanly. ### O...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~199-~199: Possible missing comma found.
Context: ...l. The remainder is trimmed to the next prompt if present. - sanitizeOutput() remove...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~256-~256: Make sure that the noun ‘build’ is correct. Did you mean the past participle “built”?
Context: ...ng simulator app. - The typical flow is build and launch via simulator tools (for exa...
(BE_VB_OR_NN)
docs/investigations/issue-describe-ui-empty-after-debugger-resume.md
[grammar] ~35-~35: Did you mean “because”?
Context: ...restores describe_ui output. ## Root Cause The process is stopped due to breakpoin...
(CAUSE_BECAUSE)
[duplication] ~40-~40: Possible typo: you repeated a word.
Context: ...ows: - stop reason = breakpoint 1.2 at CalculatorButton.swift:18 - stop reason = breakpoint 2.1 at CalculatorInputHandler.swift:12 - Aft...
(ENGLISH_WORD_REPEAT_RULE)
docs/DAP_BACKEND_IMPLEMENTATION_PLAN.md
[grammar] ~188-~188: It seems that the correct verb form here is “add”.
Context: ...ations if stateful. Side effects - Adds a long-lived child process per session....
(AGREEMENT_SENT_START_2)
[uncategorized] ~276-~276: Loose punctuation mark.
Context: ... no-op) 5) mark attached - detach(): - send disconnect with `terminateDe...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~280-~280: Loose punctuation mark.
Context: ... transport / kill process - dispose(): - best-effort cleanup; **must not thr...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~281-~281: Possible missing preposition found.
Context: ...e DebuggerManager.createSession calls dispose on attach failure) **Method mappings (...
(AI_HYDRA_LEO_MISSING_TO)
[formatting] ~366-~366: Did you mean “?”
Context: ... - after parsing breakpoint id: - if opts?.condition, run `breakpoint modify -c "<...
(MULTIPLE_PUNCTATION_MARKS_1)
[uncategorized] ~382-~382: Possible missing comma found.
Context: ...("breakpoint modify ...") **After** - calladdBreakpoint(sessionId, spec, { condi...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~442-~442: Possible missing preposition found.
Context: ...Promise { ... } ``` Reasoning - Prevent races like: - addBreakpoint + removeB...
(AI_HYDRA_LEO_MISSING_TO)
[uncategorized] ~443-~443: Possible missing comma found.
Context: ... - addBreakpoint + removeBreakpoint in parallel reissuing setBreakpoints inconsistent...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~456-~456: Do not use a colon (:) before a series that is introduced by a preposition (‘pending’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...', ...)on process exit while requests pending -DapBackend: - minimal info` logs on attach/detac...
(RP_COLON)
[duplication] ~500-~500: Possible typo: you repeated a word.
Context: ...e fixtures - Validate: - getStack() formatting - getVariables() formatting - breakpoint add/remove registry beha...
(ENGLISH_WORD_REPEAT_RULE)
[typographical] ~517-~517: Do not use a colon (:) before a series that is introduced by a preposition (‘with’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...md Replace/extend the existing outline with: - finalized module list (dap/types.ts`...
(RP_COLON)
[formatting] ~571-~571: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ... must be best-effort and never throw, because DebuggerManager.createSession() will ...
(COMMA_BEFORE_BECAUSE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Seer Code Review
...cts/iOS_Calculator/CalculatorAppPackage/Sources/CalculatorAppFeature/CalculatorService.swift
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Note
Introduces a full debugger workflow and related infrastructure, plus session/defaults improvements and documentation updates.
debuggingworkflow (8 tools: attach, continue, detach, LLDB command, breakpoints add/remove, stack, variables) and integrates a DAP-based backend alongside LLDB CLIdoctornow reportslldb-dapavailabilitysession-statusMCP resource; generated loaders/resources updated to import.tsand include new workflows/resourcesdescribe_uinow warn/guide to usedebug_continueWritten by Cursor Bugbot for commit 9bc63ac. This will update automatically on new commits. Configure here.