Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,48 @@ describe('session-set-defaults tool', () => {
expect(parsed.sessionDefaults?.simulatorName).toBeUndefined();
});

it('should store env as a Record<string, string> default', async () => {
const envVars = { STAGING_ENABLED: '1', DEBUG: 'true' };
const result = await sessionSetDefaultsLogic({ env: envVars }, createContext());

expect(result.isError).toBe(false);
expect(sessionStore.getAll().env).toEqual(envVars);
});

it('should persist env to config when persist is true', async () => {
const yaml = ['schemaVersion: 1', 'sessionDefaults: {}', ''].join('\n');

const writes: { path: string; content: string }[] = [];
const fs = createMockFileSystemExecutor({
existsSync: (targetPath: string) => targetPath === configPath,
readFile: async (targetPath: string) => {
if (targetPath !== configPath) {
throw new Error(`Unexpected readFile path: ${targetPath}`);
}
return yaml;
},
writeFile: async (targetPath: string, content: string) => {
writes.push({ path: targetPath, content });
},
});

await initConfigStore({ cwd, fs });

const envVars = { API_URL: 'https://staging.example.com' };
const result = await sessionSetDefaultsLogic(
{ env: envVars, persist: true },
createContext(),
);

expect(result.content[0].text).toContain('Persisted defaults to');
expect(writes.length).toBe(1);

const parsed = parseYaml(writes[0].content) as {
sessionDefaults?: Record<string, unknown>;
};
expect(parsed.sessionDefaults?.env).toEqual(envVars);
});

it('should not persist when persist is true but no defaults were provided', async () => {
const result = await sessionSetDefaultsLogic({ persist: true }, createContext());

Expand Down
61 changes: 61 additions & 0 deletions src/utils/__tests__/session-aware-tool-factory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,4 +258,65 @@ describe('createSessionAwareTool', () => {
expect(parsed.simulatorId).toBe('SIM-123');
expect(parsed.simulatorName).toBeUndefined();
});

it('deep-merges env so user-provided env vars are additive with session defaults', async () => {
const envSchema = z.object({
scheme: z.string(),
projectPath: z.string().optional(),
env: z.record(z.string(), z.string()).optional(),
});

const envHandler = createSessionAwareTool<z.infer<typeof envSchema>>({
internalSchema: envSchema,
logicFunction: async (params) => ({
content: [{ type: 'text', text: JSON.stringify(params.env) }],
isError: false,
}),
getExecutor: () => createMockExecutor({ success: true }),
requirements: [{ allOf: ['scheme'] }],
});

sessionStore.setDefaults({
scheme: 'App',
projectPath: '/a.xcodeproj',
env: { API_KEY: 'abc123', VERBOSE: '1' },
});

// User provides additional env var; session default env vars should be preserved
const result = await envHandler({ env: { DEBUG: 'true', VERBOSE: '0' } });
expect(result.isError).toBe(false);

const envContent = result.content[0] as { type: 'text'; text: string };
const parsed = JSON.parse(envContent.text);
expect(parsed).toEqual({ API_KEY: 'abc123', DEBUG: 'true', VERBOSE: '0' });
});

it('rejects array passed as env instead of deep-merging it', async () => {
const envSchema = z.object({
scheme: z.string(),
projectPath: z.string().optional(),
env: z.record(z.string(), z.string()).optional(),
});

const envHandler = createSessionAwareTool<z.infer<typeof envSchema>>({
internalSchema: envSchema,
logicFunction: async (params) => ({
content: [{ type: 'text', text: JSON.stringify(params.env) }],
isError: false,
}),
getExecutor: () => createMockExecutor({ success: true }),
requirements: [{ allOf: ['scheme'] }],
});

sessionStore.setDefaults({
scheme: 'App',
projectPath: '/a.xcodeproj',
env: { API_KEY: 'abc123' },
});

// Array should not be deep-merged; Zod validation should reject it
const result = await envHandler({ env: ['not', 'a', 'record'] as unknown });
expect(result.isError).toBe(true);
expect(result.content[0].text).toContain('Parameter validation failed');
});
});
11 changes: 11 additions & 0 deletions src/utils/__tests__/session-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,15 @@ describe('SessionStore', () => {
expect(all.scheme).toBe('App');
expect(all.simulatorId).toBe('SIM-1');
});

it('getAll returns a detached copy of env so mutations do not affect stored defaults', () => {
sessionStore.setDefaults({ env: { API_KEY: 'secret' } });

const copy = sessionStore.getAll();
copy.env!.API_KEY = 'tampered';
copy.env!.EXTRA = 'injected';

const stored = sessionStore.getAll();
expect(stored.env).toEqual({ API_KEY: 'secret' });
});
});
5 changes: 5 additions & 0 deletions src/utils/session-defaults-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const sessionDefaultKeys = [
'preferXcodebuild',
'platform',
'bundleId',
'env',
] as const;

export type SessionDefaultKey = (typeof sessionDefaultKeys)[number];
Expand Down Expand Up @@ -54,4 +55,8 @@ export const sessionDefaultsSchema = z.object({
.string()
.optional()
.describe('Default bundle ID for launch/stop/log tools when working on a single app.'),
env: z
.record(z.string(), z.string())
.optional()
.describe('Default environment variables to pass to launched apps.'),
});
7 changes: 6 additions & 1 deletion src/utils/session-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export type SessionDefaults = {
preferXcodebuild?: boolean;
platform?: string;
bundleId?: string;
env?: Record<string, string>;
};

class SessionStore {
Expand Down Expand Up @@ -54,7 +55,11 @@ class SessionStore {
}

getAll(): SessionDefaults {
return { ...this.defaults };
const copy = { ...this.defaults };
if (copy.env) {
copy.env = { ...copy.env };
}
return copy;
}

getRevision(): number {
Expand Down
14 changes: 13 additions & 1 deletion src/utils/typed-tool-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,19 @@ function createSessionAwareHandler<TParams, TContext>(opts: {
}

// Start with session defaults merged with explicit args (args override session)
const merged: Record<string, unknown> = { ...sessionStore.getAll(), ...sanitizedArgs };
const sessionDefaults = sessionStore.getAll();
const merged: Record<string, unknown> = { ...sessionDefaults, ...sanitizedArgs };

// Deep-merge env: combine session-default env vars with user-provided ones
// (user-provided keys take precedence on conflict)
if (
sessionDefaults.env &&
typeof sanitizedArgs.env === 'object' &&
sanitizedArgs.env &&
!Array.isArray(sanitizedArgs.env)
) {
merged.env = { ...sessionDefaults.env, ...(sanitizedArgs.env as Record<string, string>) };
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty env cannot override defaults

Medium Severity

When sessionDefaults.env exists, passing env: {} no longer overrides defaults. The deep-merge branch rebuilds merged.env from default values, so explicit empty env input is ignored and tools still receive session-level variables.

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a real issue since clearing defaults should go through session-clear-defaults


// Apply exclusive pair pruning: only when caller provided a concrete (non-null/undefined) value
// for any key in the pair. When activated, drop other keys in the pair coming from session defaults.
Expand Down
Loading