Skip to content

Commit cb6156a

Browse files
committed
fix(session-management): reject mutually exclusive defaults in session-set-defaults via schema refine; fix exclusivePairs pruning to ignore null/undefined and not override session defaults; update tests
1 parent 9046aa9 commit cb6156a

4 files changed

Lines changed: 57 additions & 18 deletions

File tree

src/mcp/tools/session-management/__tests__/session_set_defaults.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,5 +88,25 @@ describe('session-set-defaults tool', () => {
8888
expect(current.simulatorName).toBe('iPhone 16');
8989
expect(current.simulatorId).toBeUndefined();
9090
});
91+
92+
it('should reject when both projectPath and workspacePath are provided', async () => {
93+
const res = await plugin.handler({
94+
projectPath: '/app/App.xcodeproj',
95+
workspacePath: '/app/App.xcworkspace',
96+
});
97+
expect(res.isError).toBe(true);
98+
expect(res.content[0].text).toContain('Parameter validation failed');
99+
expect(res.content[0].text).toContain('projectPath and workspacePath are mutually exclusive');
100+
});
101+
102+
it('should reject when both simulatorId and simulatorName are provided', async () => {
103+
const res = await plugin.handler({
104+
simulatorId: 'SIM-1',
105+
simulatorName: 'iPhone 16',
106+
});
107+
expect(res.isError).toBe(true);
108+
expect(res.content[0].text).toContain('Parameter validation failed');
109+
expect(res.content[0].text).toContain('simulatorId and simulatorName are mutually exclusive');
110+
});
91111
});
92112
});

src/mcp/tools/session-management/session_set_defaults.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { createTypedTool } from '../../../utils/typed-tool-factory.ts';
44
import { getDefaultCommandExecutor } from '../../../utils/execution/index.ts';
55
import type { ToolResponse } from '../../../types/common.ts';
66

7-
const schemaObj = z.object({
7+
const baseSchema = z.object({
88
projectPath: z.string().optional(),
99
workspacePath: z.string().optional(),
1010
scheme: z.string().optional(),
@@ -16,6 +16,16 @@ const schemaObj = z.object({
1616
arch: z.enum(['arm64', 'x86_64']).optional(),
1717
});
1818

19+
const schemaObj = baseSchema
20+
.refine((v) => !(v.projectPath && v.workspacePath), {
21+
message: 'projectPath and workspacePath are mutually exclusive',
22+
path: ['projectPath'],
23+
})
24+
.refine((v) => !(v.simulatorId && v.simulatorName), {
25+
message: 'simulatorId and simulatorName are mutually exclusive',
26+
path: ['simulatorId'],
27+
});
28+
1929
type Params = z.infer<typeof schemaObj>;
2030

2131
export async function sessionSetDefaultsLogic(params: Params): Promise<ToolResponse> {
@@ -42,6 +52,6 @@ export default {
4252
name: 'session-set-defaults',
4353
description:
4454
'Set the session defaults needed by many tools. Most tools require one or more session defaults to be set before they can be used. Agents should set the relevant defaults at the beginning of a session.',
45-
schema: schemaObj.shape,
55+
schema: baseSchema.shape,
4656
handler: createTypedTool(schemaObj, sessionSetDefaultsLogic, getDefaultCommandExecutor),
4757
};

src/utils/__tests__/session-aware-tool-factory.test.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ describe('createSessionAwareTool', () => {
110110
expect(result.content[0].text).toContain('Tip: set session defaults');
111111
});
112112

113-
it('exclusivePairs should prune conflicting session defaults when user provides null', async () => {
113+
it('exclusivePairs should NOT prune session defaults when user provides null (treat as not provided)', async () => {
114114
const handlerWithExclusive = createSessionAwareTool<Params>({
115115
internalSchema,
116116
logicFunction: logic,
@@ -125,14 +125,15 @@ describe('createSessionAwareTool', () => {
125125
sessionStore.setDefaults({
126126
scheme: 'App',
127127
projectPath: '/path/proj.xcodeproj',
128+
simulatorId: 'SIM-1',
128129
});
129130

130131
const res = await handlerWithExclusive({ workspacePath: null as unknown as string });
131-
expect(res.isError).toBe(true);
132-
expect(res.content[0].text).toContain('Provide a project or workspace');
132+
expect(res.isError).toBe(false);
133+
expect(res.content[0].text).toBe('OK');
133134
});
134135

135-
it('exclusivePairs should prune when user provides undefined (key present)', async () => {
136+
it('exclusivePairs should NOT prune when user provides undefined (key present)', async () => {
136137
const handlerWithExclusive = createSessionAwareTool<Params>({
137138
internalSchema,
138139
logicFunction: logic,
@@ -147,10 +148,11 @@ describe('createSessionAwareTool', () => {
147148
sessionStore.setDefaults({
148149
scheme: 'App',
149150
projectPath: '/path/proj.xcodeproj',
151+
simulatorId: 'SIM-1',
150152
});
151153

152154
const res = await handlerWithExclusive({ workspacePath: undefined as unknown as string });
153-
expect(res.isError).toBe(true);
154-
expect(res.content[0].text).toContain('Provide a project or workspace');
155+
expect(res.isError).toBe(false);
156+
expect(res.content[0].text).toBe('OK');
155157
});
156158
});

src/utils/typed-tool-factory.ts

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -89,19 +89,26 @@ export function createSessionAwareTool<TParams>(opts: {
8989

9090
return async (rawArgs: Record<string, unknown>): Promise<ToolResponse> => {
9191
try {
92+
// Sanitize args: treat null/undefined as "not provided" so they don't override session defaults
93+
const sanitizedArgs: Record<string, unknown> = {};
94+
for (const [k, v] of Object.entries(rawArgs)) {
95+
if (v !== null && v !== undefined) sanitizedArgs[k] = v;
96+
}
97+
9298
// Start with session defaults merged with explicit args (args override session)
93-
const merged: Record<string, unknown> = { ...sessionStore.getAll(), ...rawArgs };
99+
const merged: Record<string, unknown> = { ...sessionStore.getAll(), ...sanitizedArgs };
94100

95-
// Apply exclusive pair pruning: if caller provided/touched any key in a pair (even null/undefined),
96-
// remove other keys from that pair which came only from session defaults (not explicitly provided).
97-
// This ensures requirements and validation reflect the effective, post-prune payload.
101+
// Apply exclusive pair pruning: only when caller provided a concrete (non-null/undefined) value
102+
// for any key in the pair. When activated, drop other keys in the pair coming from session defaults.
98103
for (const pair of exclusivePairs) {
99-
const userTouched = pair.some((k) => Object.prototype.hasOwnProperty.call(rawArgs, k));
100-
if (userTouched) {
101-
for (const k of pair) {
102-
if (rawArgs[k] == null && merged[k] != null) {
103-
delete merged[k];
104-
}
104+
const userProvidedConcrete = pair.some((k) =>
105+
Object.prototype.hasOwnProperty.call(sanitizedArgs, k),
106+
);
107+
if (!userProvidedConcrete) continue;
108+
109+
for (const k of pair) {
110+
if (!Object.prototype.hasOwnProperty.call(sanitizedArgs, k) && k in merged) {
111+
delete merged[k];
105112
}
106113
}
107114
}

0 commit comments

Comments
 (0)