mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-23 11:34:44 -07:00
Merge branch 'main' into memory_usage3
This commit is contained in:
@@ -119,6 +119,7 @@ The following tools are available in Plan Mode:
|
||||
- **Directives:** If the request is a **Directive** (e.g., "Fix bug Y"), follow the workflow below.
|
||||
5. **Plan Storage:** Save plans as Markdown (.md) using descriptive filenames.
|
||||
6. **Direct Modification:** If asked to modify code, explain you are in Plan Mode and use \`exit_plan_mode\` to request approval.
|
||||
7. **Presenting Plan:** When seeking informal agreement on a plan, or any time the user asks to see the plan, you MUST output the full content of the plan in the chat response. This overrides the "Minimal Output" guideline.
|
||||
|
||||
## Planning Workflow
|
||||
Plan Mode uses an adaptive planning workflow where the research depth, plan structure, and consultation level are proportional to the task's complexity.
|
||||
@@ -139,6 +140,7 @@ Write the implementation plan to \`/tmp/plans/\`. The plan's structure adapts to
|
||||
- **Simple Tasks:** Include a bulleted list of specific **Changes** and **Verification** steps.
|
||||
- **Standard Tasks:** Include an **Objective**, **Key Files & Context**, **Implementation Steps**, and **Verification & Testing**.
|
||||
- **Complex Tasks:** Include **Background & Motivation**, **Scope & Impact**, **Proposed Solution**, **Alternatives Considered**, a phased **Implementation Plan**, **Verification**, and **Migration & Rollback** strategies.
|
||||
- **Alignment Check:** After drafting the plan, you MUST present it to the user in the chat (adhering to Rule 7 for presenting plans) to ensure alignment on the specific details. Ask for feedback or confirmation, and proceed to Step 4 (Review & Approval) once the user agrees with the detailed plan.
|
||||
|
||||
### 4. Review & Approval
|
||||
ONLY use the \`exit_plan_mode\` tool to present the plan for formal approval AFTER you have reached an informal agreement with the user in the chat regarding the proposed strategy. When called, this tool will present the plan and formally request approval.
|
||||
@@ -297,6 +299,7 @@ The following tools are available in Plan Mode:
|
||||
- **Directives:** If the request is a **Directive** (e.g., "Fix bug Y"), follow the workflow below.
|
||||
5. **Plan Storage:** Save plans as Markdown (.md) using descriptive filenames.
|
||||
6. **Direct Modification:** If asked to modify code, explain you are in Plan Mode and use \`exit_plan_mode\` to request approval.
|
||||
7. **Presenting Plan:** When seeking informal agreement on a plan, or any time the user asks to see the plan, you MUST output the full content of the plan in the chat response. This overrides the "Minimal Output" guideline.
|
||||
|
||||
## Planning Workflow
|
||||
Plan Mode uses an adaptive planning workflow where the research depth, plan structure, and consultation level are proportional to the task's complexity.
|
||||
@@ -317,6 +320,7 @@ Write the implementation plan to \`/tmp/plans/\`. The plan's structure adapts to
|
||||
- **Simple Tasks:** Include a bulleted list of specific **Changes** and **Verification** steps.
|
||||
- **Standard Tasks:** Include an **Objective**, **Key Files & Context**, **Implementation Steps**, and **Verification & Testing**.
|
||||
- **Complex Tasks:** Include **Background & Motivation**, **Scope & Impact**, **Proposed Solution**, **Alternatives Considered**, a phased **Implementation Plan**, **Verification**, and **Migration & Rollback** strategies.
|
||||
- **Alignment Check:** After drafting the plan, you MUST present it to the user in the chat (adhering to Rule 7 for presenting plans) to ensure alignment on the specific details. Ask for feedback or confirmation, and proceed to Step 4 (Review & Approval) once the user agrees with the detailed plan.
|
||||
|
||||
### 4. Review & Approval
|
||||
ONLY use the \`exit_plan_mode\` tool to present the plan for formal approval AFTER you have reached an informal agreement with the user in the chat regarding the proposed strategy. When called, this tool will present the plan and formally request approval.
|
||||
@@ -596,6 +600,7 @@ The following tools are available in Plan Mode:
|
||||
- **Directives:** If the request is a **Directive** (e.g., "Fix bug Y"), follow the workflow below.
|
||||
5. **Plan Storage:** Save plans as Markdown (.md) using descriptive filenames.
|
||||
6. **Direct Modification:** If asked to modify code, explain you are in Plan Mode and use \`exit_plan_mode\` to request approval.
|
||||
7. **Presenting Plan:** When seeking informal agreement on a plan, or any time the user asks to see the plan, you MUST output the full content of the plan in the chat response. This overrides the "Minimal Output" guideline.
|
||||
|
||||
## Planning Workflow
|
||||
Plan Mode uses an adaptive planning workflow where the research depth, plan structure, and consultation level are proportional to the task's complexity.
|
||||
@@ -616,6 +621,7 @@ Write the implementation plan to \`/tmp/project-temp/plans/\`. The plan's struct
|
||||
- **Simple Tasks:** Include a bulleted list of specific **Changes** and **Verification** steps.
|
||||
- **Standard Tasks:** Include an **Objective**, **Key Files & Context**, **Implementation Steps**, and **Verification & Testing**.
|
||||
- **Complex Tasks:** Include **Background & Motivation**, **Scope & Impact**, **Proposed Solution**, **Alternatives Considered**, a phased **Implementation Plan**, **Verification**, and **Migration & Rollback** strategies.
|
||||
- **Alignment Check:** After drafting the plan, you MUST present it to the user in the chat (adhering to Rule 7 for presenting plans) to ensure alignment on the specific details. Ask for feedback or confirmation, and proceed to Step 4 (Review & Approval) once the user agrees with the detailed plan.
|
||||
|
||||
### 4. Review & Approval
|
||||
ONLY use the \`exit_plan_mode\` tool to present the plan for formal approval AFTER you have reached an informal agreement with the user in the chat regarding the proposed strategy. When called, this tool will present the plan and formally request approval.
|
||||
|
||||
@@ -586,6 +586,7 @@ ${options.planModeToolsList}
|
||||
- **Directives:** If the request is a **Directive** (e.g., "Fix bug Y"), follow the workflow below.
|
||||
5. **Plan Storage:** Save plans as Markdown (.md) using descriptive filenames.
|
||||
6. **Direct Modification:** If asked to modify code, explain you are in Plan Mode and use ${formatToolName(EXIT_PLAN_MODE_TOOL_NAME)} to request approval.
|
||||
7. **Presenting Plan:** When seeking informal agreement on a plan, or any time the user asks to see the plan, you MUST output the full content of the plan in the chat response. This overrides the "Minimal Output" guideline.
|
||||
|
||||
## Planning Workflow
|
||||
Plan Mode uses an adaptive planning workflow where the research depth, plan structure, and consultation level are proportional to the task's complexity.
|
||||
@@ -605,7 +606,7 @@ The depth of your consultation should be proportional to the task's complexity.
|
||||
Write the implementation plan to \`${options.plansDir}/\`. The plan's structure adapts to the task:
|
||||
- **Simple Tasks:** Include a bulleted list of specific **Changes** and **Verification** steps.
|
||||
- **Standard Tasks:** Include an **Objective**, **Key Files & Context**, **Implementation Steps**, and **Verification & Testing**.
|
||||
- **Complex Tasks:** Include **Background & Motivation**, **Scope & Impact**, **Proposed Solution**, **Alternatives Considered**, a phased **Implementation Plan**, **Verification**, and **Migration & Rollback** strategies.
|
||||
- **Complex Tasks:** Include **Background & Motivation**, **Scope & Impact**, **Proposed Solution**, **Alternatives Considered**, a phased **Implementation Plan**, **Verification**, and **Migration & Rollback** strategies.${options.interactive ? '\n- **Alignment Check:** After drafting the plan, you MUST present it to the user in the chat (adhering to Rule 7 for presenting plans) to ensure alignment on the specific details. Ask for feedback or confirmation, and proceed to Step 4 (Review & Approval) once the user agrees with the detailed plan.' : ''}
|
||||
|
||||
### 4. Review & Approval
|
||||
ONLY use the ${formatToolName(EXIT_PLAN_MODE_TOOL_NAME)} tool to present the plan for formal approval AFTER you have reached an informal agreement with the user in the chat regarding the proposed strategy. When called, this tool will present the plan and ${options.interactive ? 'formally request approval.' : 'begin implementation.'}
|
||||
|
||||
@@ -37,6 +37,7 @@ import {
|
||||
createSandboxDenialCache,
|
||||
type SandboxDenialCache,
|
||||
} from '../utils/sandboxDenialUtils.js';
|
||||
import { isErrnoException } from '../utils/fsUtils.js';
|
||||
import { handleReadWriteCommands } from '../utils/sandboxReadWriteUtils.js';
|
||||
import { buildBwrapArgs } from './bwrapArgsBuilder.js';
|
||||
|
||||
@@ -116,9 +117,12 @@ function touch(filePath: string, isDirectory: boolean) {
|
||||
assertValidPathString(filePath);
|
||||
try {
|
||||
// If it exists (even as a broken symlink), do nothing
|
||||
if (fs.lstatSync(filePath)) return;
|
||||
} catch {
|
||||
// Ignore ENOENT
|
||||
fs.lstatSync(filePath);
|
||||
return;
|
||||
} catch (e: unknown) {
|
||||
if (isErrnoException(e) && e.code !== 'ENOENT') {
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
if (isDirectory) {
|
||||
@@ -136,9 +140,22 @@ function touch(filePath: string, isDirectory: boolean) {
|
||||
export class LinuxSandboxManager implements SandboxManager {
|
||||
private static maskFilePath: string | undefined;
|
||||
private readonly denialCache: SandboxDenialCache = createSandboxDenialCache();
|
||||
private governanceFilesInitialized = false;
|
||||
|
||||
constructor(private readonly options: GlobalSandboxOptions) {}
|
||||
|
||||
private ensureGovernanceFilesExist(workspace: string): void {
|
||||
if (this.governanceFilesInitialized) return;
|
||||
|
||||
// These must exist on the host before running the sandbox to ensure they are protected.
|
||||
for (const file of GOVERNANCE_FILES) {
|
||||
const filePath = join(workspace, file.path);
|
||||
touch(filePath, file.isDirectory);
|
||||
}
|
||||
|
||||
this.governanceFilesInitialized = true;
|
||||
}
|
||||
|
||||
isKnownSafeCommand(args: string[]): boolean {
|
||||
return isKnownSafeCommand(args);
|
||||
}
|
||||
@@ -258,17 +275,14 @@ export class LinuxSandboxManager implements SandboxManager {
|
||||
mergedAdditional,
|
||||
);
|
||||
|
||||
for (const file of GOVERNANCE_FILES) {
|
||||
const filePath = join(this.options.workspace, file.path);
|
||||
touch(filePath, file.isDirectory);
|
||||
}
|
||||
this.ensureGovernanceFilesExist(resolvedPaths.workspace.resolved);
|
||||
|
||||
const bwrapArgs = await buildBwrapArgs({
|
||||
resolvedPaths,
|
||||
workspaceWrite,
|
||||
networkAccess: mergedAdditional.network ?? false,
|
||||
maskFilePath: this.getMaskFilePath(),
|
||||
isWriteCommand: req.command === '__write',
|
||||
isReadOnlyCommand: req.command === '__read',
|
||||
});
|
||||
|
||||
const bpfPath = getSeccompBpfPath();
|
||||
|
||||
@@ -92,7 +92,7 @@ describe.skipIf(os.platform() === 'win32')('buildBwrapArgs', () => {
|
||||
workspaceWrite: false,
|
||||
networkAccess: false,
|
||||
maskFilePath: '/tmp/mask',
|
||||
isWriteCommand: false,
|
||||
isReadOnlyCommand: false,
|
||||
};
|
||||
|
||||
it('should correctly format the base arguments', async () => {
|
||||
@@ -188,7 +188,7 @@ describe.skipIf(os.platform() === 'win32')('buildBwrapArgs', () => {
|
||||
expect(args[args.indexOf('/opt/tools') - 1]).toBe('--bind-try');
|
||||
});
|
||||
|
||||
it('should bind the parent directory of a non-existent path', async () => {
|
||||
it('should bind the parent directory of a non-existent path with --bind-try when isReadOnlyCommand is false', async () => {
|
||||
vi.mocked(fs.existsSync).mockImplementation((p) => {
|
||||
if (p === '/home/user/workspace/new-file.txt') return false;
|
||||
return true;
|
||||
@@ -196,10 +196,10 @@ describe.skipIf(os.platform() === 'win32')('buildBwrapArgs', () => {
|
||||
|
||||
const args = await buildBwrapArgs({
|
||||
...defaultOptions,
|
||||
isReadOnlyCommand: false,
|
||||
resolvedPaths: createResolvedPaths({
|
||||
policyAllowed: ['/home/user/workspace/new-file.txt'],
|
||||
}),
|
||||
isWriteCommand: true,
|
||||
});
|
||||
|
||||
const parentDir = '/home/user/workspace';
|
||||
@@ -208,6 +208,26 @@ describe.skipIf(os.platform() === 'win32')('buildBwrapArgs', () => {
|
||||
expect(args[bindIndex - 2]).toBe('--bind-try');
|
||||
});
|
||||
|
||||
it('should bind the parent directory of a non-existent path with --ro-bind-try when isReadOnlyCommand is true', async () => {
|
||||
vi.mocked(fs.existsSync).mockImplementation((p) => {
|
||||
if (p === '/home/user/workspace/new-file.txt') return false;
|
||||
return true;
|
||||
});
|
||||
|
||||
const args = await buildBwrapArgs({
|
||||
...defaultOptions,
|
||||
isReadOnlyCommand: true,
|
||||
resolvedPaths: createResolvedPaths({
|
||||
policyAllowed: ['/home/user/workspace/new-file.txt'],
|
||||
}),
|
||||
});
|
||||
|
||||
const parentDir = '/home/user/workspace';
|
||||
const bindIndex = args.lastIndexOf(parentDir);
|
||||
expect(bindIndex).not.toBe(-1);
|
||||
expect(args[bindIndex - 2]).toBe('--ro-bind-try');
|
||||
});
|
||||
|
||||
it('should parameterize forbidden paths and explicitly deny them', async () => {
|
||||
vi.mocked(fs.statSync).mockImplementation((p) => {
|
||||
if (p.toString().includes('cache')) {
|
||||
|
||||
@@ -23,7 +23,7 @@ export interface BwrapArgsOptions {
|
||||
workspaceWrite: boolean;
|
||||
networkAccess: boolean;
|
||||
maskFilePath: string;
|
||||
isWriteCommand: boolean;
|
||||
isReadOnlyCommand: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -37,7 +37,7 @@ export async function buildBwrapArgs(
|
||||
workspaceWrite,
|
||||
networkAccess,
|
||||
maskFilePath,
|
||||
isWriteCommand,
|
||||
isReadOnlyCommand,
|
||||
} = options;
|
||||
const { workspace } = resolvedPaths;
|
||||
|
||||
@@ -79,10 +79,13 @@ export async function buildBwrapArgs(
|
||||
bwrapArgs.push('--bind-try', allowedPath, allowedPath);
|
||||
} else {
|
||||
// If the path doesn't exist, we still want to allow access to its parent
|
||||
// to enable creating it. Since allowedPath is already resolved by resolveSandboxPaths,
|
||||
// its parent is also correctly resolved.
|
||||
// to enable creating it.
|
||||
const parent = dirname(allowedPath);
|
||||
bwrapArgs.push(isWriteCommand ? '--bind-try' : bindFlag, parent, parent);
|
||||
bwrapArgs.push(
|
||||
isReadOnlyCommand ? '--ro-bind-try' : '--bind-try',
|
||||
parent,
|
||||
parent,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { MacOsSandboxManager } from './MacOsSandboxManager.js';
|
||||
import type { ExecutionPolicy } from '../../services/sandboxManager.js';
|
||||
import { type ExecutionPolicy } from '../../services/sandboxManager.js';
|
||||
import * as seatbeltArgsBuilder from './seatbeltArgsBuilder.js';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
@@ -191,28 +191,6 @@ describe('MacOsSandboxManager', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('governance files', () => {
|
||||
it('should ensure governance files exist', async () => {
|
||||
await manager.prepareCommand({
|
||||
command: 'echo',
|
||||
args: [],
|
||||
cwd: mockWorkspace,
|
||||
env: {},
|
||||
policy: mockPolicy,
|
||||
});
|
||||
|
||||
// The seatbelt builder internally handles governance files, so we simply verify
|
||||
// it is invoked correctly with the right workspace.
|
||||
expect(seatbeltArgsBuilder.buildSeatbeltProfile).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
resolvedPaths: expect.objectContaining({
|
||||
workspace: { resolved: mockWorkspace, original: mockWorkspace },
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('allowedPaths', () => {
|
||||
it('should parameterize allowed paths and normalize them', async () => {
|
||||
await manager.prepareCommand({
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
*/
|
||||
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import path, { join } from 'node:path';
|
||||
import os from 'node:os';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import {
|
||||
@@ -33,6 +33,7 @@ import {
|
||||
} from './commandSafety.js';
|
||||
import { verifySandboxOverrides } from '../utils/commandUtils.js';
|
||||
import { parseWindowsSandboxDenials } from './windowsSandboxDenialUtils.js';
|
||||
import { isErrnoException } from '../utils/fsUtils.js';
|
||||
import {
|
||||
isSubpath,
|
||||
resolveToRealPath,
|
||||
@@ -53,10 +54,13 @@ const __dirname = path.dirname(__filename);
|
||||
*/
|
||||
export class WindowsSandboxManager implements SandboxManager {
|
||||
static readonly HELPER_EXE = 'GeminiSandbox.exe';
|
||||
|
||||
private readonly helperPath: string;
|
||||
private initialized = false;
|
||||
private readonly denialCache: SandboxDenialCache = createSandboxDenialCache();
|
||||
|
||||
private static helperCompiled = false;
|
||||
private governanceFilesInitialized = false;
|
||||
|
||||
constructor(private readonly options: GlobalSandboxOptions) {
|
||||
this.helperPath = path.resolve(__dirname, WindowsSandboxManager.HELPER_EXE);
|
||||
}
|
||||
@@ -86,33 +90,20 @@ export class WindowsSandboxManager implements SandboxManager {
|
||||
return this.options;
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensures a file or directory exists.
|
||||
*/
|
||||
private touch(filePath: string, isDirectory: boolean): void {
|
||||
assertValidPathString(filePath);
|
||||
try {
|
||||
// If it exists (even as a broken symlink), do nothing
|
||||
if (fs.lstatSync(filePath)) return;
|
||||
} catch {
|
||||
// Ignore ENOENT
|
||||
private ensureGovernanceFilesExist(workspace: string): void {
|
||||
if (this.governanceFilesInitialized) return;
|
||||
|
||||
// These must exist on the host before running the sandbox to ensure they are protected.
|
||||
for (const file of GOVERNANCE_FILES) {
|
||||
const filePath = join(workspace, file.path);
|
||||
touch(filePath, file.isDirectory);
|
||||
}
|
||||
|
||||
if (isDirectory) {
|
||||
fs.mkdirSync(filePath, { recursive: true });
|
||||
} else {
|
||||
const dir = path.dirname(filePath);
|
||||
if (!fs.existsSync(dir)) {
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
}
|
||||
fs.closeSync(fs.openSync(filePath, 'a'));
|
||||
}
|
||||
this.governanceFilesInitialized = true;
|
||||
}
|
||||
|
||||
private async ensureInitialized(): Promise<void> {
|
||||
if (this.initialized) return;
|
||||
if (os.platform() !== 'win32') {
|
||||
this.initialized = true;
|
||||
private async ensureHelperCompiled(): Promise<void> {
|
||||
if (WindowsSandboxManager.helperCompiled || os.platform() !== 'win32') {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -207,14 +198,14 @@ export class WindowsSandboxManager implements SandboxManager {
|
||||
);
|
||||
}
|
||||
|
||||
this.initialized = true;
|
||||
WindowsSandboxManager.helperCompiled = true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Prepares a command for sandboxed execution on Windows.
|
||||
*/
|
||||
async prepareCommand(req: SandboxRequest): Promise<SandboxedCommand> {
|
||||
await this.ensureInitialized();
|
||||
await this.ensureHelperCompiled();
|
||||
|
||||
const sanitizationConfig = getSecureSanitizationConfig(
|
||||
req.policy?.sanitizationConfig,
|
||||
@@ -276,6 +267,8 @@ export class WindowsSandboxManager implements SandboxManager {
|
||||
mergedAdditional,
|
||||
);
|
||||
|
||||
this.ensureGovernanceFilesExist(resolvedPaths.workspace.resolved);
|
||||
|
||||
// 1. Collect all forbidden paths.
|
||||
// We start with explicitly forbidden paths from the options and request.
|
||||
const forbiddenManifest = new Set(
|
||||
@@ -402,14 +395,6 @@ export class WindowsSandboxManager implements SandboxManager {
|
||||
// No-op for read access on Windows.
|
||||
}
|
||||
|
||||
// 4. Protected governance files
|
||||
// These must exist on the host before running the sandbox to prevent
|
||||
// the sandboxed process from creating them with Low integrity.
|
||||
for (const file of GOVERNANCE_FILES) {
|
||||
const filePath = path.join(resolvedPaths.workspace.resolved, file.path);
|
||||
this.touch(filePath, file.isDirectory);
|
||||
}
|
||||
|
||||
// 5. Generate Manifests
|
||||
const tempDir = await fs.promises.mkdtemp(
|
||||
path.join(os.tmpdir(), 'gemini-cli-sandbox-'),
|
||||
@@ -471,3 +456,29 @@ export class WindowsSandboxManager implements SandboxManager {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensures a file or directory exists.
|
||||
*/
|
||||
function touch(filePath: string, isDirectory: boolean): void {
|
||||
assertValidPathString(filePath);
|
||||
try {
|
||||
// If it exists (even as a broken symlink), do nothing
|
||||
fs.lstatSync(filePath);
|
||||
return;
|
||||
} catch (e: unknown) {
|
||||
if (isErrnoException(e) && e.code !== 'ENOENT') {
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
if (isDirectory) {
|
||||
fs.mkdirSync(filePath, { recursive: true });
|
||||
} else {
|
||||
const dir = path.dirname(filePath);
|
||||
if (!fs.existsSync(dir)) {
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
}
|
||||
fs.closeSync(fs.openSync(filePath, 'a'));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -53,7 +53,7 @@ vi.mock('../utils/events.js', () => ({
|
||||
}));
|
||||
|
||||
vi.mock('../utils/debugLogger.js', () => ({
|
||||
debugLogger: { log: vi.fn() },
|
||||
debugLogger: { debug: vi.fn() },
|
||||
}));
|
||||
|
||||
vi.mock('node:os', async (importOriginal) => {
|
||||
@@ -153,14 +153,14 @@ describe('KeychainService', () => {
|
||||
|
||||
// Because it falls back to FileKeychain, it is always available.
|
||||
expect(available).toBe(true);
|
||||
expect(debugLogger.log).toHaveBeenCalledWith(
|
||||
expect(debugLogger.debug).toHaveBeenCalledWith(
|
||||
expect.stringContaining('encountered an error'),
|
||||
'locked',
|
||||
);
|
||||
expect(coreEvents.emitTelemetryKeychainAvailability).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ available: false }),
|
||||
);
|
||||
expect(debugLogger.log).toHaveBeenCalledWith(
|
||||
expect(debugLogger.debug).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Using FileKeychain fallback'),
|
||||
);
|
||||
expect(FileKeychain).toHaveBeenCalled();
|
||||
@@ -173,7 +173,7 @@ describe('KeychainService', () => {
|
||||
const available = await service.isAvailable();
|
||||
|
||||
expect(available).toBe(true);
|
||||
expect(debugLogger.log).toHaveBeenCalledWith(
|
||||
expect(debugLogger.debug).toHaveBeenCalledWith(
|
||||
expect.stringContaining('failed structural validation'),
|
||||
expect.objectContaining({ getPassword: expect.any(Array) }),
|
||||
);
|
||||
@@ -191,7 +191,7 @@ describe('KeychainService', () => {
|
||||
const available = await service.isAvailable();
|
||||
|
||||
expect(available).toBe(true);
|
||||
expect(debugLogger.log).toHaveBeenCalledWith(
|
||||
expect(debugLogger.debug).toHaveBeenCalledWith(
|
||||
expect.stringContaining('functional verification failed'),
|
||||
);
|
||||
expect(FileKeychain).toHaveBeenCalled();
|
||||
@@ -243,7 +243,7 @@ describe('KeychainService', () => {
|
||||
);
|
||||
expect(mockKeytar.setPassword).not.toHaveBeenCalled();
|
||||
expect(FileKeychain).toHaveBeenCalled();
|
||||
expect(debugLogger.log).toHaveBeenCalledWith(
|
||||
expect(debugLogger.debug).toHaveBeenCalledWith(
|
||||
expect.stringContaining('MacOS default keychain not found'),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -114,7 +114,7 @@ export class KeychainService {
|
||||
}
|
||||
|
||||
// If native failed or was skipped, return the secure file fallback.
|
||||
debugLogger.log('Using FileKeychain fallback for secure storage.');
|
||||
debugLogger.debug('Using FileKeychain fallback for secure storage.');
|
||||
return new FileKeychain();
|
||||
}
|
||||
|
||||
@@ -130,7 +130,7 @@ export class KeychainService {
|
||||
|
||||
// Probing macOS prevents process-blocking popups when no keychain exists.
|
||||
if (os.platform() === 'darwin' && !this.isMacOSKeychainAvailable()) {
|
||||
debugLogger.log(
|
||||
debugLogger.debug(
|
||||
'MacOS default keychain not found; skipping functional verification.',
|
||||
);
|
||||
return null;
|
||||
@@ -140,12 +140,15 @@ export class KeychainService {
|
||||
return keychainModule;
|
||||
}
|
||||
|
||||
debugLogger.log('Keychain functional verification failed');
|
||||
debugLogger.debug('Keychain functional verification failed');
|
||||
return null;
|
||||
} catch (error) {
|
||||
// Avoid logging full error objects to prevent PII exposure.
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
debugLogger.log('Keychain initialization encountered an error:', message);
|
||||
debugLogger.debug(
|
||||
'Keychain initialization encountered an error:',
|
||||
message,
|
||||
);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@@ -162,7 +165,7 @@ export class KeychainService {
|
||||
return potential as Keychain;
|
||||
}
|
||||
|
||||
debugLogger.log(
|
||||
debugLogger.debug(
|
||||
'Keychain module failed structural validation:',
|
||||
result.error.flatten().fieldErrors,
|
||||
);
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -20,6 +20,8 @@ import { MCPOAuthTokenStorage } from '../mcp/oauth-token-storage.js';
|
||||
import { OAuthUtils } from '../mcp/oauth-utils.js';
|
||||
import type { PromptRegistry } from '../prompts/prompt-registry.js';
|
||||
import {
|
||||
ErrorCode,
|
||||
McpError,
|
||||
PromptListChangedNotificationSchema,
|
||||
ResourceListChangedNotificationSchema,
|
||||
ToolListChangedNotificationSchema,
|
||||
@@ -35,6 +37,7 @@ import {
|
||||
isEnabled,
|
||||
McpClient,
|
||||
populateMcpServerCommand,
|
||||
discoverPrompts,
|
||||
type McpContext,
|
||||
} from './mcp-client.js';
|
||||
import type { ToolRegistry } from './tool-registry.js';
|
||||
@@ -320,6 +323,25 @@ describe('mcp-client', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('should return empty array for discoverPrompts on MethodNotFound error without diagnostic', async () => {
|
||||
const mockedClient = {
|
||||
getServerCapabilities: vi.fn().mockReturnValue({ prompts: {} }),
|
||||
listPrompts: vi
|
||||
.fn()
|
||||
.mockRejectedValue(
|
||||
new McpError(ErrorCode.MethodNotFound, 'Method not supported'),
|
||||
),
|
||||
};
|
||||
const result = await discoverPrompts(
|
||||
'test-server',
|
||||
mockedClient as unknown as ClientLib.Client,
|
||||
MOCK_CONTEXT,
|
||||
);
|
||||
expect(result).toEqual([]);
|
||||
// MethodNotFound errors should be silently ignored regardless of message text
|
||||
expect(MOCK_CONTEXT.emitMcpDiagnostic).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not discover tools if server does not support them', async () => {
|
||||
const mockedClient = {
|
||||
connect: vi.fn(),
|
||||
|
||||
@@ -27,6 +27,8 @@ import {
|
||||
ReadResourceResultSchema,
|
||||
ResourceListChangedNotificationSchema,
|
||||
ToolListChangedNotificationSchema,
|
||||
ErrorCode,
|
||||
McpError,
|
||||
PromptListChangedNotificationSchema,
|
||||
ProgressNotificationSchema,
|
||||
type GetPromptResult,
|
||||
@@ -1250,6 +1252,10 @@ export async function connectAndDiscover(
|
||||
}
|
||||
}
|
||||
|
||||
function isMcpMethodNotFoundError(error: unknown): boolean {
|
||||
return error instanceof McpError && error.code === ErrorCode.MethodNotFound;
|
||||
}
|
||||
|
||||
/**
|
||||
* Discovers and sanitizes tools from a connected MCP client.
|
||||
* It retrieves function declarations from the client, filters out disabled tools,
|
||||
@@ -1329,10 +1335,7 @@ export async function discoverTools(
|
||||
}
|
||||
return discoveredTools;
|
||||
} catch (error) {
|
||||
if (
|
||||
error instanceof Error &&
|
||||
!error.message?.includes('Method not found')
|
||||
) {
|
||||
if (!isMcpMethodNotFoundError(error)) {
|
||||
cliConfig.emitMcpDiagnostic(
|
||||
'error',
|
||||
`Error discovering tools from ${mcpServerName}: ${getErrorMessage(
|
||||
@@ -1456,8 +1459,7 @@ export async function discoverPrompts(
|
||||
),
|
||||
}));
|
||||
} catch (error) {
|
||||
// It's okay if the method is not found, which is a common case.
|
||||
if (error instanceof Error && error.message?.includes('Method not found')) {
|
||||
if (isMcpMethodNotFoundError(error)) {
|
||||
return [];
|
||||
}
|
||||
cliConfig.emitMcpDiagnostic(
|
||||
@@ -1505,7 +1507,7 @@ async function listResources(
|
||||
cursor = response.nextCursor ?? undefined;
|
||||
} while (cursor);
|
||||
} catch (error) {
|
||||
if (error instanceof Error && error.message?.includes('Method not found')) {
|
||||
if (isMcpMethodNotFoundError(error)) {
|
||||
return [];
|
||||
}
|
||||
cliConfig.emitMcpDiagnostic(
|
||||
|
||||
@@ -195,6 +195,7 @@ describe('compatibility', () => {
|
||||
desc: '256 colors are not supported',
|
||||
},
|
||||
])('should return $expected when $desc', ({ depth, term, expected }) => {
|
||||
vi.stubEnv('COLORTERM', '');
|
||||
process.stdout.getColorDepth = vi.fn().mockReturnValue(depth);
|
||||
if (term !== undefined) {
|
||||
vi.stubEnv('TERM', term);
|
||||
@@ -203,6 +204,13 @@ describe('compatibility', () => {
|
||||
}
|
||||
expect(supports256Colors()).toBe(expected);
|
||||
});
|
||||
|
||||
it('should return true when COLORTERM is kmscon', () => {
|
||||
process.stdout.getColorDepth = vi.fn().mockReturnValue(4);
|
||||
vi.stubEnv('TERM', 'linux');
|
||||
vi.stubEnv('COLORTERM', 'kmscon');
|
||||
expect(supports256Colors()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('supportsTrueColor', () => {
|
||||
@@ -230,6 +238,12 @@ describe('compatibility', () => {
|
||||
expected: true,
|
||||
desc: 'getColorDepth returns >= 24',
|
||||
},
|
||||
{
|
||||
colorterm: 'kmscon',
|
||||
depth: 4,
|
||||
expected: true,
|
||||
desc: 'COLORTERM is kmscon',
|
||||
},
|
||||
{
|
||||
colorterm: '',
|
||||
depth: 8,
|
||||
@@ -409,6 +423,18 @@ describe('compatibility', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('should return no color warnings for kmscon terminal', () => {
|
||||
vi.mocked(os.platform).mockReturnValue('linux');
|
||||
vi.stubEnv('TERMINAL_EMULATOR', '');
|
||||
vi.stubEnv('TERM', 'linux');
|
||||
vi.stubEnv('COLORTERM', 'kmscon');
|
||||
process.stdout.getColorDepth = vi.fn().mockReturnValue(4);
|
||||
|
||||
const warnings = getCompatibilityWarnings();
|
||||
expect(warnings.find((w) => w.id === '256-color')).toBeUndefined();
|
||||
expect(warnings.find((w) => w.id === 'true-color')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should return no warnings in a standard environment with true color', () => {
|
||||
vi.mocked(os.platform).mockReturnValue('darwin');
|
||||
vi.stubEnv('TERMINAL_EMULATOR', '');
|
||||
|
||||
@@ -85,6 +85,11 @@ export function supports256Colors(): boolean {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Terminals supporting true color (like kmscon) also support 256 colors
|
||||
if (supportsTrueColor()) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -95,7 +100,8 @@ export function supportsTrueColor(): boolean {
|
||||
// Check COLORTERM environment variable
|
||||
if (
|
||||
process.env['COLORTERM'] === 'truecolor' ||
|
||||
process.env['COLORTERM'] === '24bit'
|
||||
process.env['COLORTERM'] === '24bit' ||
|
||||
process.env['COLORTERM'] === 'kmscon'
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user