Skip to content

Commit e78679b

Browse files
committed
Refactor debug MCP tools: shared projections, registry helpers, doc updates
Tier 1 cleanup based on PR review feedback (patricksullivansf, yhsieh1) and self-review. DRY: - Extract shared projection helpers to SDK (projections.ts): truncateValue, isPrimitiveType, projectFrame, projectVariable, projectBreakpoint, projectThreadLocation, plus exported types MappedFrame/MappedVariable/MappedBreakpoint/MappedLocation. Used by both MCP debug tools and CLI RPC mode — eliminates 5+ copies of the same projection code and the MAX_VALUE_LENGTH/PRIMITIVE_TYPES constants. - Add getSessionEntry/getRegistry helpers to session-registry. Replace the 4-line "registry not available" + getSessionOrThrow boilerplate in 11 tools. - Move halt-waiter promise pattern into DebugSessionRegistry.waitForHalt. Used by debug_wait_for_stop and debug_capture_at_breakpoint instead of duplicating the promise/timer/cleanup logic. - Replace step-action switch with a lookup map. API: - DebugSessionRegistry.registerSession now takes an options object (yhsieh1 nit) instead of 5 positional parameters. Docs: - Add "Recovery from broken or orphaned sessions" section to docs/mcp/tools/diagnostics.md (patricksullivansf concern). - Link to main authentication and configuration guides. - Add JSDoc to ServerContext explaining stdio-per-client isolation vs shared transport caveats (patricksullivansf concern). Tool descriptions (yhsieh1 audit): - Drop user-facing prose, sharpen for agent consumption. - debug_start_session: explicitly mention SFRA controllers, custom API scripts, hooks, jobs as use cases. - debug_end_session: IMPORTANT call-out to always end sessions. Coverage stays at 99.81% statements/lines, 94.81% branches, 98.93% functions — all above thresholds.
1 parent b1276e4 commit e78679b

19 files changed

Lines changed: 730 additions & 503 deletions

docs/mcp/tools/diagnostics.md

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,25 @@ description: MCP tools for script debugging on B2C Commerce instances.
44

55
# Diagnostics Tools
66

7-
MCP tools for connecting to the B2C Commerce Script Debugger API (SDAPI), setting breakpoints, inspecting variables, and stepping through server-side code. These tools are available in the **CARTRIDGES**, **SCAPI**, and **STOREFRONTNEXT** toolsets.
7+
MCP tools for connecting to the B2C Commerce Script Debugger API (SDAPI), setting breakpoints, inspecting variables, and stepping through server-side code. These tools are available in the **CARTRIDGES** and **SCAPI** toolsets.
88

99
## Authentication
1010

1111
All debug tools require **Basic Auth** credentials (username and password) for a Business Manager user with the `WebDAV_Manage_Customization` permission.
1212

13-
The script debugger must be enabled on the instance: Business Manager > Administration > Development Configuration > Script Debugger > Enable.
13+
The script debugger must also be enabled on the instance: Business Manager > Administration > Development Configuration > Script Debugger > Enable.
14+
15+
See the [Authentication guide](../../guide/authentication) and [Configuration guide](../../guide/configuration) for credential setup details.
16+
17+
## Recovery from broken or orphaned sessions
18+
19+
Debug sessions are stateful and live in the MCP server process. If the agent loses track of an active session (context flush, crash, restart), or breakpoints stop firing as expected:
20+
21+
1. **List active sessions** — call `debug_list_sessions` (no args). It returns all sessions known to the server with their `session_id`, `hostname`, halted threads, and currently armed breakpoints.
22+
2. **End orphaned sessions** — call `debug_end_session` with the `session_id` to free the debugger slot on the instance.
23+
3. **SDAPI single-client guarantee** — the script debugger only supports one client per `client_id` per host. Calling `debug_start_session` with the same `client_id` against the same host implicitly takes over (replaces) any prior client. This is the safety net when a session is lost without a clean shutdown.
24+
4. **Idle cleanup** — sessions inactive for 30 minutes are automatically cleaned up by the server.
25+
5. **Restart the MCP server** — as a last resort, restarting the MCP server destroys all session state. The orphaned debugger slot on the instance will be freed by SDAPI's own 60-second halt-timeout or by the next `debug_start_session` with the same client ID.
1426

1527
---
1628

packages/b2c-cli/src/utils/debug/rpc.ts

Lines changed: 34 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,13 @@ import type {
1212
SourceMapper,
1313
} from '@salesforce/b2c-tooling-sdk/operations/debug';
1414
import type {CartridgeMapping} from '@salesforce/b2c-tooling-sdk/operations/code';
15-
import {resolveBreakpointPath} from '@salesforce/b2c-tooling-sdk/operations/debug';
16-
17-
const MAX_VALUE_LENGTH = 200;
15+
import {
16+
projectBreakpoint,
17+
projectFrame,
18+
projectThreadLocation,
19+
projectVariable,
20+
resolveBreakpointPath,
21+
} from '@salesforce/b2c-tooling-sdk/operations/debug';
1822

1923
interface RpcRequest {
2024
id?: number | string;
@@ -63,18 +67,9 @@ export class DebugRpc {
6367
this.currentThreadId = thread.id;
6468
this.currentFrameIndex = 0;
6569

66-
const topFrame = thread.call_stack?.[0];
67-
const loc = topFrame?.location;
6870
this.emitEvent('thread_stopped', {
6971
thread_id: thread.id,
70-
location: loc
71-
? {
72-
file: this.sourceMapper.toLocalPath(loc.script_path) ?? null,
73-
line: loc.line_number,
74-
function_name: loc.function_name,
75-
script_path: loc.script_path,
76-
}
77-
: null,
72+
location: projectThreadLocation(thread, this.sourceMapper),
7873
});
7974
}
8075

@@ -113,13 +108,13 @@ export class DebugRpc {
113108
private async handleCommand(command: string, args: Record<string, unknown>): Promise<unknown> {
114109
switch (command) {
115110
case 'continue': {
116-
const threadId = (args.thread_id as number) ?? this.getThread();
111+
const threadId = this.resolveThreadId(args);
117112
await this.manager.resume(threadId);
118113
return {thread_id: threadId, status: 'resumed'};
119114
}
120115

121116
case 'evaluate': {
122-
const threadId = (args.thread_id as number) ?? this.getThread();
117+
const threadId = this.resolveThreadId(args);
123118
const frameIndex = (args.frame_index as number) ?? this.currentFrameIndex;
124119
const expression = args.expression as string;
125120
if (!expression) throw new Error('Missing required arg: expression');
@@ -128,86 +123,45 @@ export class DebugRpc {
128123
}
129124

130125
case 'get_stack': {
131-
const threadId = (args.thread_id as number) ?? this.getThread();
126+
const threadId = this.resolveThreadId(args);
132127
const thread = await this.manager.client.getThread(threadId);
133128
return {
134129
thread_id: thread.id,
135-
frames: thread.call_stack.map((frame) => ({
136-
index: frame.index,
137-
function_name: frame.location.function_name,
138-
file: this.sourceMapper.toLocalPath(frame.location.script_path) ?? null,
139-
line: frame.location.line_number,
140-
script_path: frame.location.script_path,
141-
})),
130+
frames: thread.call_stack.map((frame) => projectFrame(frame, this.sourceMapper)),
142131
};
143132
}
144133

145134
case 'get_variables': {
146-
const threadId = (args.thread_id as number) ?? this.getThread();
135+
const threadId = this.resolveThreadId(args);
147136
const frameIndex = (args.frame_index as number) ?? this.currentFrameIndex;
148137
const objectPath = args.object_path as string | undefined;
149138

150139
if (objectPath) {
151140
const result = await this.manager.client.getMembers(threadId, frameIndex, objectPath);
152-
return {
153-
variables: result.object_members.map((m) => ({
154-
name: m.name,
155-
type: m.type,
156-
value: truncateValue(m.value),
157-
has_children: !isPrimitive(m.type),
158-
})),
159-
};
141+
return {variables: result.object_members.map((m) => projectVariable(m, {includeScope: false}))};
160142
}
161143

162144
const result = await this.manager.client.getVariables(threadId, frameIndex);
163-
let members = result.object_members;
164-
if (args.scope) {
165-
members = members.filter((m) => m.scope === args.scope);
166-
}
167-
return {
168-
variables: members.map((m) => ({
169-
name: m.name,
170-
type: m.type,
171-
value: truncateValue(m.value),
172-
scope: m.scope,
173-
has_children: !isPrimitive(m.type),
174-
})),
175-
};
145+
const members = args.scope
146+
? result.object_members.filter((m) => m.scope === args.scope)
147+
: result.object_members;
148+
return {variables: members.map((m) => projectVariable(m))};
176149
}
177150

178151
case 'list_breakpoints': {
179152
const bps = await this.manager.client.getBreakpoints();
180-
return {
181-
breakpoints: bps.map((bp) => ({
182-
id: bp.id,
183-
file: this.sourceMapper.toLocalPath(bp.script_path) ?? null,
184-
line: bp.line_number,
185-
script_path: bp.script_path,
186-
condition: bp.condition,
187-
})),
188-
};
153+
return {breakpoints: bps.map((bp) => projectBreakpoint(bp, this.sourceMapper))};
189154
}
190155

191156
case 'list_threads': {
192157
const threads = this.manager.getKnownThreads();
193158
return {
194-
threads: threads.map((t) => {
195-
const topFrame = t.call_stack?.[0];
196-
const loc = topFrame?.location;
197-
return {
198-
thread_id: t.id,
199-
status: t.status,
200-
current: t.id === this.currentThreadId,
201-
location: loc
202-
? {
203-
file: this.sourceMapper.toLocalPath(loc.script_path) ?? null,
204-
line: loc.line_number,
205-
function_name: loc.function_name,
206-
script_path: loc.script_path,
207-
}
208-
: null,
209-
};
210-
}),
159+
threads: threads.map((t) => ({
160+
thread_id: t.id,
161+
status: t.status,
162+
current: t.id === this.currentThreadId,
163+
location: projectThreadLocation(t, this.sourceMapper),
164+
})),
211165
};
212166
}
213167

@@ -237,31 +191,23 @@ export class DebugRpc {
237191
}));
238192

239193
const result = await this.manager.setBreakpoints(inputs);
240-
return {
241-
breakpoints: result.map((bp) => ({
242-
id: bp.id,
243-
file: this.sourceMapper.toLocalPath(bp.script_path) ?? null,
244-
line: bp.line_number,
245-
script_path: bp.script_path,
246-
condition: bp.condition,
247-
})),
248-
};
194+
return {breakpoints: result.map((bp) => projectBreakpoint(bp, this.sourceMapper))};
249195
}
250196

251197
case 'step_into': {
252-
const threadId = (args.thread_id as number) ?? this.getThread();
198+
const threadId = this.resolveThreadId(args);
253199
await this.manager.stepInto(threadId);
254200
return {thread_id: threadId, action: 'step_into'};
255201
}
256202

257203
case 'step_out': {
258-
const threadId = (args.thread_id as number) ?? this.getThread();
204+
const threadId = this.resolveThreadId(args);
259205
await this.manager.stepOut(threadId);
260206
return {thread_id: threadId, action: 'step_out'};
261207
}
262208

263209
case 'step_over': {
264-
const threadId = (args.thread_id as number) ?? this.getThread();
210+
const threadId = this.resolveThreadId(args);
265211
await this.manager.stepOver(threadId);
266212
return {thread_id: threadId, action: 'step_over'};
267213
}
@@ -295,16 +241,12 @@ export class DebugRpc {
295241
}
296242
}
297243

244+
/** Resolve a thread_id arg, falling back to the currently selected thread. */
245+
private resolveThreadId(args: Record<string, unknown>): number {
246+
return (args.thread_id as number) ?? this.getThread();
247+
}
248+
298249
private sendResponse(response: RpcResponse): void {
299250
this.output.write(JSON.stringify(response) + '\n');
300251
}
301252
}
302-
303-
function isPrimitive(type: string): boolean {
304-
return ['boolean', 'Boolean', 'null', 'number', 'Number', 'string', 'String', 'undefined'].includes(type);
305-
}
306-
307-
function truncateValue(value: string): string {
308-
if (value.length <= MAX_VALUE_LENGTH) return value;
309-
return value.slice(0, MAX_VALUE_LENGTH) + '...';
310-
}

packages/b2c-dx-mcp/src/server-context.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,27 @@
66

77
import {DebugSessionRegistry} from './tools/diagnostics/session-registry.js';
88

9+
/**
10+
* Server-scoped persistent state that lives for the lifetime of the MCP
11+
* server process. Holds registries for stateful resources (debug sessions,
12+
* future log watches) that need to span multiple tool invocations.
13+
*
14+
* ## Multi-agent / shared-state caveats
15+
*
16+
* One `ServerContext` is created per MCP server process. With the default
17+
* stdio transport, each MCP client connection spawns its own server
18+
* subprocess, so this state is naturally isolated per client/agent.
19+
*
20+
* If this server is ever wired up to a shared transport (e.g. HTTP with
21+
* multiple connected clients), `ServerContext` state would be shared
22+
* across all connected clients. Sub-agents that run within the same MCP
23+
* client would also share the same context. Tools that mutate registries
24+
* (like the debug tools) should not assume single-tenant access.
25+
*
26+
* The debug registry already handles concurrent sessions via session IDs
27+
* and host:client_id pairs, so multi-agent use is functional but agents
28+
* may see each other's sessions via `debug_list_sessions`.
29+
*/
930
export class ServerContext {
1031
readonly debugSessions: DebugSessionRegistry;
1132

0 commit comments

Comments
 (0)