Skip to content

Commit 054b899

Browse files
cameroncookecodex
andcommitted
fix(tests): Normalize LLDB offsets and stale tools
Normalize LLDB breakpoint byte offsets in snapshot output and update the affected CLI/MCP JSON/text fixtures. Also avoid retaining stale MCP tool registrations when a previously registered tool fails to import during workflow re-selection. Co-Authored-By: OpenAI Codex <noreply@openai.com>
1 parent d954699 commit 054b899

8 files changed

Lines changed: 123 additions & 8 deletions

File tree

src/snapshot-tests/__fixtures__/cli/json/debugging/lldb-command--success.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
" Names:",
1212
" dap",
1313
"",
14-
" 1.1: where = CalculatorApp.debug.dylib`closure #1 in closure #1 in closure #1 in ContentView.body.getter + 1428 at ContentView.swift:42:31, address = <ADDR>, resolved, hit count = 0"
14+
" 1.1: where = CalculatorApp.debug.dylib`closure #1 in closure #1 in closure #1 in ContentView.body.getter + <OFFSET> at ContentView.swift:42:31, address = <ADDR>, resolved, hit count = 0"
1515
]
1616
}
1717
}

src/snapshot-tests/__fixtures__/cli/text/debugging/lldb-command--success.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,4 @@ Output:
1111
Names:
1212
dap
1313

14-
1.1: where = CalculatorApp.debug.dylib`closure #1 in closure #1 in closure #1 in ContentView.body.getter + 1428 at ContentView.swift:42:31, address = <ADDR>, resolved, hit count = 0
14+
1.1: where = CalculatorApp.debug.dylib`closure #1 in closure #1 in closure #1 in ContentView.body.getter + <OFFSET> at ContentView.swift:42:31, address = <ADDR>, resolved, hit count = 0

src/snapshot-tests/__fixtures__/mcp/json/debugging/lldb-command--success.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
" Names:",
1212
" dap",
1313
"",
14-
" 1.1: where = CalculatorApp.debug.dylib`closure #1 in closure #1 in closure #1 in ContentView.body.getter + 1428 at ContentView.swift:42:31, address = <ADDR>, resolved, hit count = 0"
14+
" 1.1: where = CalculatorApp.debug.dylib`closure #1 in closure #1 in closure #1 in ContentView.body.getter + <OFFSET> at ContentView.swift:42:31, address = <ADDR>, resolved, hit count = 0"
1515
]
1616
}
1717
}

src/snapshot-tests/__fixtures__/mcp/text/debugging/lldb-command--success.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@ Output:
99
Names:
1010
dap
1111

12-
1.1: where = CalculatorApp.debug.dylib`closure #1 in closure #1 in closure #1 in ContentView.body.getter + 1428 at ContentView.swift:42:31, address = <ADDR>, resolved, hit count = 0
12+
1.1: where = CalculatorApp.debug.dylib`closure #1 in closure #1 in closure #1 in ContentView.body.getter + <OFFSET> at ContentView.swift:42:31, address = <ADDR>, resolved, hit count = 0

src/snapshot-tests/__tests__/normalize.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,16 @@ describe('normalizeSnapshotOutput', () => {
4444
);
4545
});
4646

47+
it('normalizes LLDB breakpoint byte offsets', () => {
48+
expect(
49+
normalizeSnapshotOutput(
50+
' 1.1: where = App.debug.dylib`ContentView.body.getter + 1428 at ContentView.swift:42:31, address = 0x123456789, unresolved, hit count = 0\n',
51+
),
52+
).toBe(
53+
' 1.1: where = App.debug.dylib`ContentView.body.getter + <OFFSET> at ContentView.swift:42:31, address = <ADDR>, unresolved, hit count = 0\n',
54+
);
55+
});
56+
4757
it('normalizes process identifiers in string output', () => {
4858
expect(normalizeSnapshotOutput('appName: PID 123456\nkill: 123456: No such process\n')).toBe(
4959
'appName: PID <PID>\nkill: <PID>: No such process\n',

src/snapshot-tests/normalize.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const THREAD_ID_REGEX = /Thread \d{5,}/g;
2424
const HEX_ADDRESS_REGEX = /0x[0-9a-fA-F]{8,}/g;
2525

2626
const LLDB_FRAME_OFFSET_REGEX = /(`[^`\n]+):(\d+)$/gm;
27+
const LLDB_BREAKPOINT_BYTE_OFFSET_REGEX = /\+ \d+ at /g;
2728
const LLDB_SYS_FRAME_FUNC_REGEX =
2829
/(frame #\d+: )\S+( at (?:\/usr\/lib\/|\/Library\/Developer\/CoreSimulator\/)[^`\n]*`)[^:\n]+(:<OFFSET>)/gm;
2930
const LLDB_FRAME_NUMBER_REGEX = / frame #\d+:/g;
@@ -204,6 +205,7 @@ export function normalizeSnapshotOutput(text: string): string {
204205
normalized = normalized.replace(THREAD_ID_REGEX, 'Thread <THREAD_ID>');
205206
normalized = normalized.replace(HEX_ADDRESS_REGEX, '<ADDR>');
206207
normalized = normalized.replace(LLDB_FRAME_OFFSET_REGEX, '$1:<OFFSET>');
208+
normalized = normalized.replace(LLDB_BREAKPOINT_BYTE_OFFSET_REGEX, '+ <OFFSET> at ');
207209
normalized = normalized.replace(LLDB_SYS_FRAME_FUNC_REGEX, '$1<FUNC>$2<FUNC>$3');
208210
normalized = normalized.replace(LLDB_FRAME_NUMBER_REGEX, ' frame #<N>:');
209211
normalized = normalized.replace(LLDB_BREAKPOINT_LOCATIONS_REGEX, 'locations = <LOCATIONS>');

src/utils/__tests__/tool-registry.test.ts

Lines changed: 106 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,46 @@
1-
import { describe, expect, it } from 'vitest';
2-
import { createCustomWorkflowsFromConfig } from '../tool-registry.ts';
1+
import { describe, expect, it, vi, beforeEach } from 'vitest';
2+
import type { ToolSchemaShape } from '../../core/plugin-types.ts';
33
import type { ResolvedManifest } from '../../core/manifest/schema.ts';
4+
import type { PredicateContext } from '../../visibility/predicate-types.ts';
5+
6+
const testState = vi.hoisted(() => {
7+
const registeredTools = new Map<string, { remove: ReturnType<typeof vi.fn> }>();
8+
return {
9+
manifest: undefined as ResolvedManifest | undefined,
10+
importToolModule: vi.fn(),
11+
registeredTools,
12+
server: {
13+
registerTool: vi.fn((name: string) => {
14+
const registeredTool = { remove: vi.fn() };
15+
registeredTools.set(name, registeredTool);
16+
return registeredTool;
17+
}),
18+
},
19+
};
20+
});
21+
22+
vi.mock('../../server/server-state.ts', () => ({
23+
server: testState.server,
24+
}));
25+
26+
vi.mock('../../core/manifest/load-manifest.ts', () => ({
27+
loadManifest: () => {
28+
if (!testState.manifest) {
29+
throw new Error('No manifest configured for test');
30+
}
31+
return testState.manifest;
32+
},
33+
}));
34+
35+
vi.mock('../../core/manifest/import-tool-module.ts', () => ({
36+
importToolModule: testState.importToolModule,
37+
}));
38+
39+
import {
40+
__resetToolRegistryForTests,
41+
applyWorkflowSelectionFromManifest,
42+
createCustomWorkflowsFromConfig,
43+
} from '../tool-registry.ts';
444

545
function createManifestFixture(): ResolvedManifest {
646
return {
@@ -37,6 +77,7 @@ function createManifestFixture(): ResolvedManifest {
3777
description: 'Built-in simulator workflow',
3878
targetPlatforms: ['iOS'],
3979
availability: { mcp: true, cli: true },
80+
selection: { mcp: { defaultEnabled: true, autoInclude: false } },
4081
predicates: [],
4182
tools: ['build_run_sim'],
4283
},
@@ -46,6 +87,36 @@ function createManifestFixture(): ResolvedManifest {
4687
};
4788
}
4889

90+
function createToolModule() {
91+
return {
92+
schema: {} as ToolSchemaShape,
93+
handler: vi.fn().mockResolvedValue({ content: [] }),
94+
};
95+
}
96+
97+
function createPredicateContext(): PredicateContext {
98+
return {
99+
runtime: 'mcp',
100+
config: {
101+
enabledWorkflows: [],
102+
customWorkflows: {},
103+
debug: false,
104+
sentryDisabled: false,
105+
experimentalWorkflowDiscovery: false,
106+
disableSessionDefaults: false,
107+
disableXcodeAutoSync: false,
108+
showTestTiming: false,
109+
uiDebuggerGuardMode: 'error',
110+
incrementalBuildsEnabled: false,
111+
dapRequestTimeoutMs: 30_000,
112+
dapLogEvents: false,
113+
launchJsonWaitMs: 8000,
114+
debuggerBackend: 'dap',
115+
},
116+
runningUnderXcode: false,
117+
};
118+
}
119+
49120
describe('createCustomWorkflowsFromConfig', () => {
50121
it('creates custom workflows and resolves tool IDs', () => {
51122
const manifest = createManifestFixture();
@@ -76,3 +147,36 @@ describe('createCustomWorkflowsFromConfig', () => {
76147
expect(result.warnings).toHaveLength(3);
77148
});
78149
});
150+
151+
describe('applyWorkflowSelectionFromManifest', () => {
152+
beforeEach(() => {
153+
__resetToolRegistryForTests();
154+
testState.manifest = createManifestFixture();
155+
testState.importToolModule.mockReset();
156+
testState.server.registerTool.mockClear();
157+
testState.registeredTools.clear();
158+
});
159+
160+
it('removes a stale registered tool when its module no longer imports', async () => {
161+
testState.importToolModule.mockResolvedValueOnce(createToolModule());
162+
163+
const initialRegistration = await applyWorkflowSelectionFromManifest(
164+
undefined,
165+
createPredicateContext(),
166+
);
167+
168+
const registeredTool = testState.registeredTools.get('build_run_sim');
169+
expect(initialRegistration.registeredToolCount).toBe(1);
170+
expect(registeredTool).toBeDefined();
171+
172+
testState.importToolModule.mockRejectedValueOnce(new Error('import failed'));
173+
174+
const nextRegistration = await applyWorkflowSelectionFromManifest(
175+
undefined,
176+
createPredicateContext(),
177+
);
178+
179+
expect(nextRegistration.registeredToolCount).toBe(0);
180+
expect(registeredTool?.remove).toHaveBeenCalledTimes(1);
181+
});
182+
});

src/utils/tool-registry.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,13 +431,12 @@ async function enumerateAndRegisterTools(
431431
continue;
432432
}
433433

434-
desiredToolNames.add(toolManifest.names.mcp);
435-
436434
const toolModule = await tryImportToolModule(toolManifest, moduleCache);
437435
if (!toolModule) {
438436
continue;
439437
}
440438

439+
desiredToolNames.add(toolManifest.names.mcp);
441440
catalogTools.push(toCatalogTool(toolManifest, workflow, toolModule));
442441
registerToolFromManifest(toolManifest, toolModule);
443442
}

0 commit comments

Comments
 (0)