fix(core): Resolve context initialization mismatch in built-in tools

- Update ShellTool, WebFetchTool, and WebSearchTool to handle both Config and AgentLoopContext.
- Add safety checks for config in ToolConfirmationMessage UI.
- Add safety checks for context.config in policy update logic.
- Fixes #23174
This commit is contained in:
mkorwel
2026-03-19 16:32:46 -07:00
parent 8615315711
commit 68d00feb62
6 changed files with 170 additions and 51 deletions
@@ -88,13 +88,14 @@ export const ToolConfirmationMessage: React.FC<
const settings = useSettings();
const allowPermanentApproval =
settings.merged.security.enablePermanentToolApproval &&
!config.getDisableAlwaysAllow();
(config?.getDisableAlwaysAllow ? !config.getDisableAlwaysAllow() : true);
const handlesOwnUI =
confirmationDetails.type === 'ask_user' ||
confirmationDetails.type === 'exit_plan_mode';
const isTrustedFolder =
config.isTrustedFolder() && !config.getDisableAlwaysAllow();
config?.isTrustedFolder?.() &&
(config?.getDisableAlwaysAllow ? !config.getDisableAlwaysAllow() : true);
const handleConfirm = useCallback(
(outcome: ToolConfirmationOutcome, payload?: ToolConfirmationPayload) => {
+3 -4
View File
@@ -119,7 +119,7 @@ export async function updatePolicy(
): Promise<void> {
// Mode Transitions (AUTO_EDIT)
if (isAutoEditTransition(tool, outcome)) {
context.config.setApprovalMode(ApprovalMode.AUTO_EDIT);
context.config?.setApprovalMode?.(ApprovalMode.AUTO_EDIT);
return;
}
@@ -128,9 +128,8 @@ export async function updatePolicy(
if (outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave) {
// If folder is trusted and workspace policies are enabled, we prefer workspace scope.
if (
context.config &&
context.config.isTrustedFolder() &&
context.config.getWorkspacePoliciesDir() !== undefined
context.config?.isTrustedFolder?.() &&
context.config?.getWorkspacePoliciesDir?.() !== undefined
) {
persistScope = 'workspace';
} else {
+41 -24
View File
@@ -45,6 +45,7 @@ import type { MessageBus } from '../confirmation-bus/message-bus.js';
import { getShellDefinition } from './definitions/coreTools.js';
import { resolveToolDeclaration } from './definitions/resolver.js';
import type { AgentLoopContext } from '../config/agent-loop-context.js';
import type { Config } from '../config/config.js';
export const OUTPUT_UPDATE_INTERVAL_MS = 1000;
@@ -62,14 +63,23 @@ export class ShellToolInvocation extends BaseToolInvocation<
ShellToolParams,
ToolResult
> {
private readonly config: Config;
private readonly geminiClient?: GeminiClient;
constructor(
private readonly context: AgentLoopContext,
context: Config | AgentLoopContext,
params: ShellToolParams,
messageBus: MessageBus,
_toolName?: string,
_toolDisplayName?: string,
) {
super(params, messageBus, _toolName, _toolDisplayName);
if ('config' in context) {
this.config = context.config;
this.geminiClient = context.geminiClient;
} else {
this.config = context;
}
}
getDescription(): string {
@@ -168,7 +178,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
.toString('hex')}.tmp`;
const tempFilePath = path.join(os.tmpdir(), tempFileName);
const timeoutMs = this.context.config.getShellToolInactivityTimeout();
const timeoutMs = this.config.getShellToolInactivityTimeout();
const timeoutController = new AbortController();
let timeoutTimer: NodeJS.Timeout | undefined;
@@ -189,10 +199,10 @@ export class ShellToolInvocation extends BaseToolInvocation<
})();
const cwd = this.params.dir_path
? path.resolve(this.context.config.getTargetDir(), this.params.dir_path)
: this.context.config.getTargetDir();
? path.resolve(this.config.getTargetDir(), this.params.dir_path)
: this.config.getTargetDir();
const validationError = this.context.config.validatePathAccess(cwd);
const validationError = this.config.validatePathAccess(cwd);
if (validationError) {
return {
llmContent: validationError,
@@ -271,14 +281,14 @@ export class ShellToolInvocation extends BaseToolInvocation<
}
},
combinedController.signal,
this.context.config.getEnableInteractiveShell(),
this.config.getEnableInteractiveShell(),
{
...shellExecutionConfig,
pager: 'cat',
sanitizationConfig:
shellExecutionConfig?.sanitizationConfig ??
this.context.config.sanitizationConfig,
sandboxManager: this.context.config.sandboxManager,
this.config.sanitizationConfig,
sandboxManager: this.config.sandboxManager,
},
);
@@ -383,7 +393,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
}
let returnDisplayMessage = '';
if (this.context.config.getDebugMode()) {
if (this.config.getDebugMode()) {
returnDisplayMessage = llmContent;
} else {
if (this.params.is_background || result.backgrounded) {
@@ -412,8 +422,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
}
}
const summarizeConfig =
this.context.config.getSummarizeToolOutputConfig();
const summarizeConfig = this.config.getSummarizeToolOutputConfig();
const executionError = result.error
? {
error: {
@@ -424,10 +433,10 @@ export class ShellToolInvocation extends BaseToolInvocation<
: {};
if (summarizeConfig && summarizeConfig[SHELL_TOOL_NAME]) {
const summary = await summarizeToolOutput(
this.context.config,
this.config,
{ model: 'summarizer-shell' },
llmContent,
this.context.geminiClient,
this.geminiClient ?? this.config.getGeminiClient(),
signal,
);
return {
@@ -461,30 +470,38 @@ export class ShellTool extends BaseDeclarativeTool<
ToolResult
> {
static readonly Name = SHELL_TOOL_NAME;
private readonly config: Config;
private readonly geminiClient?: GeminiClient;
constructor(
private readonly context: AgentLoopContext,
context: Config | AgentLoopContext,
messageBus: MessageBus,
) {
void initializeShellParsers().catch(() => {
// Errors are surfaced when parsing commands.
});
const config = 'config' in context ? context.config : context;
const definition = getShellDefinition(
context.config.getEnableInteractiveShell(),
context.config.getEnableShellOutputEfficiency(),
config.getEnableInteractiveShell(),
config.getEnableShellOutputEfficiency(),
);
super(
ShellTool.Name,
'Shell',
definition.base.description!,
Kind.Execute,
Kind.Shell,
definition.base.parametersJsonSchema,
messageBus,
false, // output is not markdown
true, // output can be updated
false, // isOutputMarkdown
true, // canUpdateOutput
);
this.config = config;
if ('config' in context) {
this.geminiClient = context.geminiClient;
}
}
protected override validateToolParamValues(
params: ShellToolParams,
): string | null {
@@ -494,10 +511,10 @@ export class ShellTool extends BaseDeclarativeTool<
if (params.dir_path) {
const resolvedPath = path.resolve(
this.context.config.getTargetDir(),
this.config.getTargetDir(),
params.dir_path,
);
return this.context.config.validatePathAccess(resolvedPath);
return this.config.validatePathAccess(resolvedPath);
}
return null;
}
@@ -509,7 +526,7 @@ export class ShellTool extends BaseDeclarativeTool<
_toolDisplayName?: string,
): ToolInvocation<ShellToolParams, ToolResult> {
return new ShellToolInvocation(
this.context.config,
{ config: this.config, geminiClient: this.geminiClient } as any,
params,
messageBus,
_toolName,
@@ -519,8 +536,8 @@ export class ShellTool extends BaseDeclarativeTool<
override getSchema(modelId?: string) {
const definition = getShellDefinition(
this.context.config.getEnableInteractiveShell(),
this.context.config.getEnableShellOutputEfficiency(),
this.config.getEnableInteractiveShell(),
this.config.getEnableShellOutputEfficiency(),
);
return resolveToolDeclaration(definition, modelId);
}
+34 -17
View File
@@ -38,6 +38,8 @@ import { WEB_FETCH_DEFINITION } from './definitions/coreTools.js';
import { resolveToolDeclaration } from './definitions/resolver.js';
import { LRUCache } from 'mnemonist';
import type { AgentLoopContext } from '../config/agent-loop-context.js';
import type { Config } from '../config/config.js';
import type { GeminiClient } from '../core/client.js';
const URL_FETCH_TIMEOUT_MS = 10000;
const MAX_CONTENT_LENGTH = 250000;
@@ -224,18 +226,27 @@ class WebFetchToolInvocation extends BaseToolInvocation<
WebFetchToolParams,
ToolResult
> {
private readonly config: Config;
private readonly geminiClient?: GeminiClient;
constructor(
private readonly context: AgentLoopContext,
context: Config | AgentLoopContext,
params: WebFetchToolParams,
messageBus: MessageBus,
_toolName?: string,
_toolDisplayName?: string,
) {
super(params, messageBus, _toolName, _toolDisplayName);
if ('config' in context) {
this.config = context.config;
this.geminiClient = context.geminiClient;
} else {
this.config = context;
}
}
private handleRetry(attempt: number, error: unknown, delayMs: number): void {
const maxAttempts = this.context.config.getMaxAttempts();
const maxAttempts = this.config.getMaxAttempts();
const modelName = 'Web Fetch';
const errorType = getRetryErrorType(error);
@@ -248,7 +259,7 @@ class WebFetchToolInvocation extends BaseToolInvocation<
});
logNetworkRetryAttempt(
this.context.config,
this.config,
new NetworkRetryAttemptEvent(
attempt,
maxAttempts,
@@ -302,13 +313,12 @@ class WebFetchToolInvocation extends BaseToolInvocation<
return res;
},
{
retryFetchErrors: this.context.config.getRetryFetchErrors(),
retryFetchErrors: this.config.getRetryFetchErrors(),
onRetry: (attempt, error, delayMs) =>
this.handleRetry(attempt, error, delayMs),
signal,
},
);
);
const bodyBuffer = await this.readResponseWithLimit(
response,
MAX_EXPERIMENTAL_FETCH_SIZE,
@@ -350,7 +360,7 @@ class WebFetchToolInvocation extends BaseToolInvocation<
`[WebFetchTool] Skipped private or local host: ${url}`,
);
logWebFetchFallbackAttempt(
this.context.config,
this.config,
new WebFetchFallbackAttemptEvent('private_ip_skipped'),
);
skipped.push(`[Blocked Host] ${url}`);
@@ -435,7 +445,7 @@ class WebFetchToolInvocation extends BaseToolInvocation<
.join('\n');
try {
const geminiClient = this.context.geminiClient;
const geminiClient = this.geminiClient ?? this.config.getGeminiClient();
const fallbackPrompt = `Follow the user's instructions below using the provided webpage content.
<user_instructions>
@@ -518,7 +528,7 @@ ${aggregatedContent}
): Promise<ToolCallConfirmationDetails | false> {
// Check for AUTO_EDIT approval mode. This tool has a specific behavior
// where ProceedAlways switches the entire session to AUTO_EDIT.
if (this.context.config.getApprovalMode() === ApprovalMode.AUTO_EDIT) {
if (this.config.getApprovalMode() === ApprovalMode.AUTO_EDIT) {
return false;
}
@@ -641,7 +651,7 @@ ${aggregatedContent}
return res;
},
{
retryFetchErrors: this.context.config.getRetryFetchErrors(),
retryFetchErrors: this.config.getRetryFetchErrors(),
onRetry: (attempt, error, delayMs) =>
this.handleRetry(attempt, error, delayMs),
signal,
@@ -752,7 +762,7 @@ Response: ${truncateString(rawResponseText, 10000, '\n\n... [Error response trun
}
async execute(signal: AbortSignal): Promise<ToolResult> {
if (this.context.config.getDirectWebFetch()) {
if (this.config.getDirectWebFetch()) {
return this.executeExperimental(signal);
}
const userPrompt = this.params.prompt!;
@@ -775,7 +785,7 @@ Response: ${truncateString(rawResponseText, 10000, '\n\n... [Error response trun
}
try {
const geminiClient = this.context.geminiClient;
const geminiClient = this.geminiClient ?? this.config.getGeminiClient();
const sanitizedPrompt = `Follow the user's instructions to process the authorized URLs.
<user_instructions>
@@ -867,7 +877,7 @@ ${toFetch.join('\n')}
`[WebFetchTool] Primary fetch failed, falling back: ${getErrorMessage(error)}`,
);
logWebFetchFallbackAttempt(
this.context.config,
this.config,
new WebFetchFallbackAttemptEvent('primary_failed'),
);
// Simple All-or-Nothing Fallback
@@ -884,11 +894,14 @@ export class WebFetchTool extends BaseDeclarativeTool<
ToolResult
> {
static readonly Name = WEB_FETCH_TOOL_NAME;
private readonly config: Config;
private readonly geminiClient?: GeminiClient;
constructor(
private readonly context: AgentLoopContext,
context: Config | AgentLoopContext,
messageBus: MessageBus,
) {
const config = 'config' in context ? context.config : context;
super(
WebFetchTool.Name,
'WebFetch',
@@ -899,12 +912,16 @@ export class WebFetchTool extends BaseDeclarativeTool<
true, // isOutputMarkdown
false, // canUpdateOutput
);
this.config = config;
if ('config' in context) {
this.geminiClient = context.geminiClient;
}
}
protected override validateToolParamValues(
params: WebFetchToolParams,
): string | null {
if (this.context.config.getDirectWebFetch()) {
if (this.config.getDirectWebFetch()) {
if (!params.url) {
return "The 'url' parameter is required.";
}
@@ -940,7 +957,7 @@ export class WebFetchTool extends BaseDeclarativeTool<
_toolDisplayName?: string,
): ToolInvocation<WebFetchToolParams, ToolResult> {
return new WebFetchToolInvocation(
this.context,
{ config: this.config, geminiClient: this.geminiClient } as any,
params,
messageBus,
_toolName,
@@ -950,7 +967,7 @@ export class WebFetchTool extends BaseDeclarativeTool<
override getSchema(modelId?: string) {
const schema = resolveToolDeclaration(WEB_FETCH_DEFINITION, modelId);
if (this.context.config.getDirectWebFetch()) {
if (this.config.getDirectWebFetch()) {
return {
...schema,
description:
+22 -4
View File
@@ -23,6 +23,8 @@ import { WEB_SEARCH_DEFINITION } from './definitions/coreTools.js';
import { resolveToolDeclaration } from './definitions/resolver.js';
import { LlmRole } from '../telemetry/llmRole.js';
import type { AgentLoopContext } from '../config/agent-loop-context.js';
import type { Config } from '../config/config.js';
import type { GeminiClient } from '../core/client.js';
interface GroundingChunkWeb {
uri?: string;
@@ -70,14 +72,23 @@ class WebSearchToolInvocation extends BaseToolInvocation<
WebSearchToolParams,
WebSearchToolResult
> {
private readonly config: Config;
private readonly geminiClient?: GeminiClient;
constructor(
private readonly context: AgentLoopContext,
context: Config | AgentLoopContext,
params: WebSearchToolParams,
messageBus: MessageBus,
_toolName?: string,
_toolDisplayName?: string,
) {
super(params, messageBus, _toolName, _toolDisplayName);
if ('config' in context) {
this.config = context.config;
this.geminiClient = context.geminiClient;
} else {
this.config = context;
}
}
override getDescription(): string {
@@ -85,7 +96,7 @@ class WebSearchToolInvocation extends BaseToolInvocation<
}
async execute(signal: AbortSignal): Promise<WebSearchToolResult> {
const geminiClient = this.context.geminiClient;
const geminiClient = this.geminiClient ?? this.config.getGeminiClient();
try {
const response = await geminiClient.generateContent(
@@ -205,11 +216,14 @@ export class WebSearchTool extends BaseDeclarativeTool<
WebSearchToolResult
> {
static readonly Name = WEB_SEARCH_TOOL_NAME;
private readonly config: Config;
private readonly geminiClient?: GeminiClient;
constructor(
private readonly context: AgentLoopContext,
context: Config | AgentLoopContext,
messageBus: MessageBus,
) {
const config = 'config' in context ? context.config : context;
super(
WebSearchTool.Name,
'GoogleSearch',
@@ -220,6 +234,10 @@ export class WebSearchTool extends BaseDeclarativeTool<
true, // isOutputMarkdown
false, // canUpdateOutput
);
this.config = config;
if ('config' in context) {
this.geminiClient = context.geminiClient;
}
}
/**
@@ -243,7 +261,7 @@ export class WebSearchTool extends BaseDeclarativeTool<
_toolDisplayName?: string,
): ToolInvocation<WebSearchToolParams, WebSearchToolResult> {
return new WebSearchToolInvocation(
this.context.config,
{ config: this.config, geminiClient: this.geminiClient } as any,
params,
messageBus ?? this.messageBus,
_toolName,
@@ -0,0 +1,67 @@
# Implementation Plan - Fix Context Initialization Mismatch in Core Tools
The Gemini CLI is experiencing various "Cannot read properties of undefined" errors (notably `isTrustedFolder` and `publish`) during tool execution and confirmation. This is caused by a structural mismatch where the global `Config` object is passed to tool constructors that now expect an `AgentLoopContext`.
## Background & Reproducibility Analysis
### Root Cause Analysis (Regression)
The regression was introduced in commit **`de656f01d7`** (PR **#22115**), titled *"feat(core): Fully migrate packages/core to AgentLoopContext"*.
In this PR:
- Tool constructors (e.g., `ShellTool`, `WebFetchTool`, `WebSearchTool`) were updated to expect an `AgentLoopContext` instead of a `Config` object.
- `AgentLoopContext` is an interface that includes a `.config` property.
- However, in `packages/core/src/config/config.ts`, the global `Config` object still instantiates these tools by passing `this` (the `Config` instance itself).
- While `Config` implements some getters that overlap with `AgentLoopContext` (like `geminiClient`), it does **not** have a `.config` property.
- Consequently, internal tool calls to `this.context.config.someMethod()` fail because `this.context.config` is undefined.
- Additionally, some parts of the system expect a fully initialized `messageBus` on the context, which can lead to "Cannot read properties of undefined (reading 'publish')" if the context isn't correctly structured or if there's a race in initialization.
### Why this might not be reproducible for all users:
- **Sub-agent Usage:** Sub-agents correctly initialize tools with a full `AgentLoopContext` via `LocalAgentExecutor`.
- **Tool Selection:** The bug only surfaces in tools that specifically access `context.config` or certain context-scoped services.
- **Initialization Order:** Depending on how the CLI is invoked (interactive vs. non-interactive), the `Config` object might be in different states of initialization.
## Objective
Standardize tool initialization so that built-in tools can correctly resolve their required configuration and services whether they are running in the main loop (initialized with `Config`) or as a sub-agent (initialized with `AgentLoopContext`).
## Key Files & Context
- `packages/core/src/config/config.ts`: Where built-in tools are incorrectly instantiated with `this`.
- `packages/core/src/tools/shell.ts`: `ShellTool` constructor expects `AgentLoopContext`.
- `packages/core/src/tools/web-fetch.ts`: `WebFetchTool` constructor expects `AgentLoopContext`.
- `packages/core/src/tools/web-search.ts`: `WebSearchTool` constructor expects `AgentLoopContext`.
- `packages/core/src/scheduler/policy.ts`: Accesses `context.config.isTrustedFolder()`.
- `packages/core/src/scheduler/scheduler.ts`: Initializes its own `messageBus` and `context`.
## Implementation Steps
### Phase 0: Preparation
1. **Update Plan:** (This file) Broaden the scope from just "isTrusted" to "Context Mismatch".
2. **GitHub Issue:** Update GitHub issue #23174 with the refined root cause analysis.
### Phase 1: Robust Tool Context Handling
The built-in tools need to handle being initialized by either the global `Config` or an `AgentLoopContext`.
- **Update `packages/core/src/tools/shell.ts`**, **`web-fetch.ts`**, and **`web-search.ts`**:
- Update constructors to accept `Config | AgentLoopContext`.
- Internalize logic to safely resolve `config`, `geminiClient`, and other services.
- Example:
```typescript
this.config = 'config' in context ? context.config : context;
```
### Phase 2: UI and Policy Hardening
- **Update `packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx`**: Add safety checks for `config`.
- **Update `packages/core/src/scheduler/policy.ts`**: Ensure `context.config` exists before use.
### Phase 3: Scheduler Verification
- Ensure `Scheduler` and `SchedulerStateManager` are receiving a correctly structured `messageBus`.
## Verification & Testing
### Automated Tests
- Create unit tests that specifically instantiate tools with a raw `Config` object and call methods that access `this.config`.
- Verify `npm run test:core` passes.
### Manual Verification
1. Trigger shell commands in the interactive CLI.
2. Attempt "Allow all" actions.
3. Verify no "undefined" crashes occur.