Merge branch 'main' into workspace-command-scope-20737

This commit is contained in:
Dmitry Lyalin
2026-03-10 15:20:46 -04:00
committed by GitHub
52 changed files with 2484 additions and 1306 deletions
@@ -120,6 +120,9 @@ jobs:
if (recentRuns.length > 0) {
core.setOutput('dispatched_run_urls', recentRuns.map(r => r.html_url).join(','));
core.setOutput('dispatched_run_ids', recentRuns.map(r => r.id).join(','));
const markdownLinks = recentRuns.map(r => `- [View dispatched workflow run](${r.html_url})`).join('\n');
core.setOutput('dispatched_run_links', markdownLinks);
}
- name: 'Comment on Failure'
@@ -138,16 +141,19 @@ jobs:
token: '${{ secrets.GITHUB_TOKEN }}'
issue-number: '${{ github.event.issue.number }}'
body: |
**Patch workflow(s) dispatched successfully!**
🚀 **[Step 1/4] Patch workflow(s) waiting for approval!**
**📋 Details:**
- **Channels**: `${{ steps.dispatch_patch.outputs.dispatched_channels }}`
- **Commit**: `${{ steps.pr_status.outputs.MERGE_COMMIT_SHA }}`
- **Workflows Created**: ${{ steps.dispatch_patch.outputs.dispatched_run_count }}
**⏳ Status:** The patch creation workflow has been triggered and is waiting for deployment approval. Please visit the specific workflow links below and approve the runs.
**🔗 Track Progress:**
- [View patch workflows](https://github.com/${{ github.repository }}/actions/workflows/release-patch-1-create-pr.yml)
- [This workflow run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }})
${{ steps.dispatch_patch.outputs.dispatched_run_links }}
- [View patch workflow history](https://github.com/${{ github.repository }}/actions/workflows/release-patch-1-create-pr.yml)
- [This trigger workflow run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }})
- name: 'Final Status Comment - Dispatch Success (No URL)'
if: "always() && startsWith(github.event.comment.body, '/patch') && steps.dispatch_patch.outcome == 'success' && !steps.dispatch_patch.outputs.dispatched_run_urls"
@@ -156,16 +162,18 @@ jobs:
token: '${{ secrets.GITHUB_TOKEN }}'
issue-number: '${{ github.event.issue.number }}'
body: |
**Patch workflow(s) dispatched successfully!**
🚀 **[Step 1/4] Patch workflow(s) waiting for approval!**
**📋 Details:**
- **Channels**: `${{ steps.dispatch_patch.outputs.dispatched_channels }}`
- **Commit**: `${{ steps.pr_status.outputs.MERGE_COMMIT_SHA }}`
- **Workflows Created**: ${{ steps.dispatch_patch.outputs.dispatched_run_count }}
**⏳ Status:** The patch creation workflow has been triggered and is waiting for deployment approval. Please visit the workflow history link below and approve the runs.
**🔗 Track Progress:**
- [View patch workflows](https://github.com/${{ github.repository }}/actions/workflows/release-patch-1-create-pr.yml)
- [This workflow run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }})
- [View patch workflow history](https://github.com/${{ github.repository }}/actions/workflows/release-patch-1-create-pr.yml)
- [This trigger workflow run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }})
- name: 'Final Status Comment - Failure'
if: "always() && startsWith(github.event.comment.body, '/patch') && (steps.dispatch_patch.outcome == 'failure' || steps.dispatch_patch.outcome == 'cancelled')"
@@ -174,7 +182,7 @@ jobs:
token: '${{ secrets.GITHUB_TOKEN }}'
issue-number: '${{ github.event.issue.number }}'
body: |
❌ **Patch workflow dispatch failed!**
❌ **[Step 1/4] Patch workflow dispatch failed!**
There was an error dispatching the patch creation workflow.
+4
View File
@@ -77,6 +77,10 @@ You can run the review tool in two ways:
./scripts/review.sh <PR_NUMBER> [model]
```
**Warning:** If you run `scripts/review.sh`, you must have first verified
that the code for the PR being reviewed is safe to run and does not contain
data exfiltration attacks.
**Authors are strongly encouraged to run this script on their own PRs**
immediately after creation. This allows you to catch and fix simple issues
locally before a maintainer performs a full review.
+1
View File
@@ -125,6 +125,7 @@ they appear in the UI.
| ------------------------------------- | ----------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------- |
| Disable YOLO Mode | `security.disableYoloMode` | Disable YOLO mode, even if enabled by a flag. | `false` |
| Allow Permanent Tool Approval | `security.enablePermanentToolApproval` | Enable the "Allow for all future sessions" option in tool confirmation dialogs. | `false` |
| Auto-add to Policy by Default | `security.autoAddToPolicyByDefault` | When enabled, the "Allow for all future sessions" option becomes the default choice for low-risk tools in trusted workspaces. | `false` |
| Blocks extensions from Git | `security.blockGitExtensions` | Blocks installing and loading extensions from Git. | `false` |
| Extension Source Regex Allowlist | `security.allowedExtensions` | List of Regex patterns for allowed extensions. If nonempty, only extensions that match the patterns in this list are allowed. Overrides the blockGitExtensions setting. | `[]` |
| Folder Trust | `security.folderTrust.enabled` | Setting to track whether Folder trust is enabled. | `true` |
+955 -694
View File
File diff suppressed because it is too large Load Diff
+71 -27
View File
@@ -1,23 +1,22 @@
# Local development guide
This guide provides instructions for setting up and using local development
features, such as tracing.
features for Gemini CLI.
## Tracing
Traces are OpenTelemetry (OTel) records that help you debug your code by
instrumenting key events like model calls, tool scheduler operations, and tool
calls.
Gemini CLI uses OpenTelemetry (OTel) to record traces that help you debug agent
behavior. Traces instrument key events like model calls, tool scheduler
operations, and tool calls.
Traces provide deep visibility into agent behavior and are invaluable for
debugging complex issues. They are captured automatically when telemetry is
enabled.
Traces provide deep visibility into agent behavior and help you debug complex
issues. They are captured automatically when you enable telemetry.
### Viewing traces
### View traces
You can view traces using either Jaeger or the Genkit Developer UI.
You can view traces using Genkit Developer UI, Jaeger, or Google Cloud.
#### Using Genkit
#### Use Genkit
Genkit provides a web-based UI for viewing traces and other telemetry data.
@@ -29,11 +28,8 @@ Genkit provides a web-based UI for viewing traces and other telemetry data.
npm run telemetry -- --target=genkit
```
The script will output the URL for the Genkit Developer UI, for example:
```
Genkit Developer UI: http://localhost:4000
```
The script will output the URL for the Genkit Developer UI. For example:
`Genkit Developer UI: http://localhost:4000`
2. **Run Gemini CLI:**
@@ -48,21 +44,22 @@ Genkit provides a web-based UI for viewing traces and other telemetry data.
Open the Genkit Developer UI URL in your browser and navigate to the
**Traces** tab to view the traces.
#### Using Jaeger
#### Use Jaeger
You can view traces in the Jaeger UI. To get started, follow these steps:
You can view traces in the Jaeger UI for local development.
1. **Start the telemetry collector:**
Run the following command in your terminal to download and start Jaeger and
an OTEL collector:
an OTel collector:
```bash
npm run telemetry -- --target=local
```
This command also configures your workspace for local telemetry and provides
a link to the Jaeger UI (usually `http://localhost:16686`).
This command configures your workspace for local telemetry and provides a
link to the Jaeger UI (usually `http://localhost:16686`).
- **Collector logs:** `~/.gemini/tmp/<projectHash>/otel/collector.log`
2. **Run Gemini CLI:**
@@ -77,16 +74,63 @@ You can view traces in the Jaeger UI. To get started, follow these steps:
After running your command, open the Jaeger UI link in your browser to view
the traces.
#### Use Google Cloud
You can use an OpenTelemetry collector to forward telemetry data to Google Cloud
Trace for custom processing or routing.
> **Warning:** Ensure you complete the
> [Google Cloud telemetry prerequisites](./cli/telemetry.md#prerequisites)
> (Project ID, authentication, IAM roles, and APIs) before using this method.
1. **Configure `.gemini/settings.json`:**
```json
{
"telemetry": {
"enabled": true,
"target": "gcp",
"useCollector": true
}
}
```
2. **Start the telemetry collector:**
Run the following command to start a local OTel collector that forwards to
Google Cloud:
```bash
npm run telemetry -- --target=gcp
```
The script outputs links to view traces, metrics, and logs in the Google
Cloud Console.
- **Collector logs:** `~/.gemini/tmp/<projectHash>/otel/collector-gcp.log`
3. **Run Gemini CLI:**
In a separate terminal, run your Gemini CLI command:
```bash
gemini
```
4. **View logs, metrics, and traces:**
After sending prompts, view your data in the Google Cloud Console. See the
[telemetry documentation](./cli/telemetry.md#view-google-cloud-telemetry)
for links to Logs, Metrics, and Trace explorers.
For more detailed information on telemetry, see the
[telemetry documentation](./cli/telemetry.md).
### Instrumenting code with traces
### Instrument code with traces
You can add traces to your own code for more detailed instrumentation. This is
useful for debugging and understanding the flow of execution.
You can add traces to your own code for more detailed instrumentation.
Use the `runInDevTraceSpan` function to wrap any section of code in a trace
span.
Adding traces helps you debug and understand the flow of execution. Use the
`runInDevTraceSpan` function to wrap any section of code in a trace span.
Here is a basic example:
@@ -102,13 +146,13 @@ await runInDevTraceSpan(
},
},
async ({ metadata }) => {
// The `metadata` object allows you to record the input and output of the
// metadata allows you to record the input and output of the
// operation as well as other attributes.
metadata.input = { key: 'value' };
// Set custom attributes.
metadata.attributes['custom.attribute'] = 'custom.value';
// Your code to be traced goes here
// Your code to be traced goes here.
try {
const output = await somethingRisky();
metadata.output = output;
+5
View File
@@ -872,6 +872,11 @@ their corresponding top-level category object in your `settings.json` file.
confirmation dialogs.
- **Default:** `false`
- **`security.autoAddToPolicyByDefault`** (boolean):
- **Description:** When enabled, the "Allow for all future sessions" option
becomes the default choice for low-risk tools in trusted workspaces.
- **Default:** `false`
- **`security.blockGitExtensions`** (boolean):
- **Description:** Blocks installing and loading extensions from Git.
- **Default:** `false`
+1 -1
View File
@@ -111,7 +111,7 @@
"badge": "🔬",
"slug": "docs/cli/notifications"
},
{ "label": "Plan mode", "badge": "🔬", "slug": "docs/cli/plan-mode" },
{ "label": "Plan mode", "slug": "docs/cli/plan-mode" },
{
"label": "Subagents",
"badge": "🔬",
+116
View File
@@ -0,0 +1,116 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, expect } from 'vitest';
import {
TRACKER_CREATE_TASK_TOOL_NAME,
TRACKER_UPDATE_TASK_TOOL_NAME,
} from '@google/gemini-cli-core';
import { evalTest, assertModelHasOutput } from './test-helper.js';
import fs from 'node:fs';
import path from 'node:path';
const FILES = {
'package.json': JSON.stringify({
name: 'test-project',
version: '1.0.0',
scripts: { test: 'echo "All tests passed!"' },
}),
'src/login.js':
'function login(username, password) {\n if (!username) throw new Error("Missing username");\n // BUG: missing password check\n return true;\n}',
} as const;
describe('tracker_mode', () => {
evalTest('USUALLY_PASSES', {
name: 'should manage tasks in the tracker when explicitly requested during a bug fix',
params: {
settings: { experimental: { taskTracker: true } },
},
files: FILES,
prompt:
'We have a bug in src/login.js: the password check is missing. First, create a task in the tracker to fix it. Then fix the bug, and mark the task as closed.',
assert: async (rig, result) => {
const wasCreateCalled = await rig.waitForToolCall(
TRACKER_CREATE_TASK_TOOL_NAME,
);
expect(
wasCreateCalled,
'Expected tracker_create_task tool to be called',
).toBe(true);
const toolLogs = rig.readToolLogs();
const createCall = toolLogs.find(
(log) => log.toolRequest.name === TRACKER_CREATE_TASK_TOOL_NAME,
);
expect(createCall).toBeDefined();
const args = JSON.parse(createCall!.toolRequest.args);
expect(
(args.title?.toLowerCase() ?? '') +
(args.description?.toLowerCase() ?? ''),
).toContain('login');
const wasUpdateCalled = await rig.waitForToolCall(
TRACKER_UPDATE_TASK_TOOL_NAME,
);
expect(
wasUpdateCalled,
'Expected tracker_update_task tool to be called',
).toBe(true);
const updateCall = toolLogs.find(
(log) => log.toolRequest.name === TRACKER_UPDATE_TASK_TOOL_NAME,
);
expect(updateCall).toBeDefined();
const updateArgs = JSON.parse(updateCall!.toolRequest.args);
expect(updateArgs.status).toBe('closed');
const loginContent = fs.readFileSync(
path.join(rig.testDir!, 'src/login.js'),
'utf-8',
);
expect(loginContent).not.toContain('// BUG: missing password check');
assertModelHasOutput(result);
},
});
evalTest('USUALLY_PASSES', {
name: 'should implicitly create tasks when asked to build a feature plan',
params: {
settings: { experimental: { taskTracker: true } },
},
files: FILES,
prompt:
'I need to build a complex new feature for user authentication in our project. Create a detailed implementation plan and organize the work into bite-sized chunks. Do not actually implement the code yet, just plan it.',
assert: async (rig, result) => {
// The model should proactively use tracker_create_task to organize the work
const wasToolCalled = await rig.waitForToolCall(
TRACKER_CREATE_TASK_TOOL_NAME,
);
expect(
wasToolCalled,
'Expected tracker_create_task to be called implicitly to organize plan',
).toBe(true);
const toolLogs = rig.readToolLogs();
const createCalls = toolLogs.filter(
(log) => log.toolRequest.name === TRACKER_CREATE_TASK_TOOL_NAME,
);
// We expect it to create at least one task for authentication, likely more.
expect(createCalls.length).toBeGreaterThan(0);
// Verify it didn't write any code since we asked it to just plan
const loginContent = fs.readFileSync(
path.join(rig.testDir!, 'src/login.js'),
'utf-8',
);
expect(loginContent).toContain('// BUG: missing password check');
assertModelHasOutput(result);
},
});
});
@@ -4,7 +4,7 @@ exports[`runNonInteractive > should emit appropriate error event in streaming JS
"{"type":"init","timestamp":"<TIMESTAMP>","session_id":"test-session-id","model":"test-model"}
{"type":"message","timestamp":"<TIMESTAMP>","role":"user","content":"Loop test"}
{"type":"error","timestamp":"<TIMESTAMP>","severity":"warning","message":"Loop detected, stopping execution"}
{"type":"result","timestamp":"<TIMESTAMP>","status":"success","stats":{"total_tokens":0,"input_tokens":0,"output_tokens":0,"cached":0,"input":0,"duration_ms":<DURATION>,"tool_calls":0}}
{"type":"result","timestamp":"<TIMESTAMP>","status":"success","stats":{"total_tokens":0,"input_tokens":0,"output_tokens":0,"cached":0,"input":0,"duration_ms":<DURATION>,"tool_calls":0,"models":{}}}
"
`;
@@ -12,7 +12,7 @@ exports[`runNonInteractive > should emit appropriate error event in streaming JS
"{"type":"init","timestamp":"<TIMESTAMP>","session_id":"test-session-id","model":"test-model"}
{"type":"message","timestamp":"<TIMESTAMP>","role":"user","content":"Max turns test"}
{"type":"error","timestamp":"<TIMESTAMP>","severity":"error","message":"Maximum session turns exceeded"}
{"type":"result","timestamp":"<TIMESTAMP>","status":"success","stats":{"total_tokens":0,"input_tokens":0,"output_tokens":0,"cached":0,"input":0,"duration_ms":<DURATION>,"tool_calls":0}}
{"type":"result","timestamp":"<TIMESTAMP>","status":"success","stats":{"total_tokens":0,"input_tokens":0,"output_tokens":0,"cached":0,"input":0,"duration_ms":<DURATION>,"tool_calls":0,"models":{}}}
"
`;
@@ -23,7 +23,7 @@ exports[`runNonInteractive > should emit appropriate events for streaming JSON o
{"type":"tool_use","timestamp":"<TIMESTAMP>","tool_name":"testTool","tool_id":"tool-1","parameters":{"arg1":"value1"}}
{"type":"tool_result","timestamp":"<TIMESTAMP>","tool_id":"tool-1","status":"success","output":"Tool executed successfully"}
{"type":"message","timestamp":"<TIMESTAMP>","role":"assistant","content":"Final answer","delta":true}
{"type":"result","timestamp":"<TIMESTAMP>","status":"success","stats":{"total_tokens":0,"input_tokens":0,"output_tokens":0,"cached":0,"input":0,"duration_ms":<DURATION>,"tool_calls":0}}
{"type":"result","timestamp":"<TIMESTAMP>","status":"success","stats":{"total_tokens":0,"input_tokens":0,"output_tokens":0,"cached":0,"input":0,"duration_ms":<DURATION>,"tool_calls":0,"models":{}}}
"
`;
+12
View File
@@ -1496,6 +1496,18 @@ const SETTINGS_SCHEMA = {
'Enable the "Allow for all future sessions" option in tool confirmation dialogs.',
showInDialog: true,
},
autoAddToPolicyByDefault: {
type: 'boolean',
label: 'Auto-add to Policy by Default',
category: 'Security',
requiresRestart: false,
default: false,
description: oneLine`
When enabled, the "Allow for all future sessions" option becomes the
default choice for low-risk tools in trusted workspaces.
`,
showInDialog: true,
},
blockGitExtensions: {
type: 'boolean',
label: 'Blocks extensions from Git',
@@ -411,7 +411,7 @@ describe('ToolConfirmationMessage', () => {
unmount();
});
it('should show "Allow for all future sessions" when setting is true', async () => {
it('should show "Allow for all future sessions" when trusted', async () => {
const mockConfig = {
isTrustedFolder: () => true,
getIdeMode: () => false,
@@ -434,7 +434,10 @@ describe('ToolConfirmationMessage', () => {
);
await waitUntilReady();
expect(lastFrame()).toContain('Allow for all future sessions');
const output = lastFrame();
expect(output).toContain('future sessions');
// Verify it is the default selection (matching the indicator in the snapshot)
expect(output).toMatchSnapshot();
unmount();
});
});
@@ -246,9 +246,9 @@ export const ToolConfirmationMessage: React.FC<
});
if (allowPermanentApproval) {
options.push({
label: 'Allow for all future sessions',
label: 'Allow for this file in all future sessions',
value: ToolConfirmationOutcome.ProceedAlwaysAndSave,
key: 'Allow for all future sessions',
key: 'Allow for this file in all future sessions',
});
}
}
@@ -282,7 +282,7 @@ export const ToolConfirmationMessage: React.FC<
});
if (allowPermanentApproval) {
options.push({
label: `Allow for all future sessions`,
label: `Allow this command for all future sessions`,
value: ToolConfirmationOutcome.ProceedAlwaysAndSave,
key: `Allow for all future sessions`,
});
@@ -388,266 +388,301 @@ export const ToolConfirmationMessage: React.FC<
return Math.max(availableTerminalHeight - surroundingElementsHeight, 1);
}, [availableTerminalHeight, getOptions, handlesOwnUI]);
const { question, bodyContent, options, securityWarnings } = useMemo<{
question: string;
bodyContent: React.ReactNode;
options: Array<RadioSelectItem<ToolConfirmationOutcome>>;
securityWarnings: React.ReactNode;
}>(() => {
let bodyContent: React.ReactNode | null = null;
let securityWarnings: React.ReactNode | null = null;
let question = '';
const options = getOptions();
const { question, bodyContent, options, securityWarnings, initialIndex } =
useMemo<{
question: string;
bodyContent: React.ReactNode;
options: Array<RadioSelectItem<ToolConfirmationOutcome>>;
securityWarnings: React.ReactNode;
initialIndex: number;
}>(() => {
let bodyContent: React.ReactNode | null = null;
let securityWarnings: React.ReactNode | null = null;
let question = '';
const options = getOptions();
if (deceptiveUrlWarningText) {
securityWarnings = <WarningMessage text={deceptiveUrlWarningText} />;
}
if (confirmationDetails.type === 'ask_user') {
bodyContent = (
<AskUserDialog
questions={confirmationDetails.questions}
onSubmit={(answers) => {
handleConfirm(ToolConfirmationOutcome.ProceedOnce, { answers });
}}
onCancel={() => {
handleConfirm(ToolConfirmationOutcome.Cancel);
}}
width={terminalWidth}
availableHeight={availableBodyContentHeight()}
/>
);
return {
question: '',
bodyContent,
options: [],
securityWarnings: null,
};
}
if (confirmationDetails.type === 'exit_plan_mode') {
bodyContent = (
<ExitPlanModeDialog
planPath={confirmationDetails.planPath}
getPreferredEditor={getPreferredEditor}
onApprove={(approvalMode) => {
handleConfirm(ToolConfirmationOutcome.ProceedOnce, {
approved: true,
approvalMode,
});
}}
onFeedback={(feedback) => {
handleConfirm(ToolConfirmationOutcome.ProceedOnce, {
approved: false,
feedback,
});
}}
onCancel={() => {
handleConfirm(ToolConfirmationOutcome.Cancel);
}}
width={terminalWidth}
availableHeight={availableBodyContentHeight()}
/>
);
return { question: '', bodyContent, options: [], securityWarnings: null };
}
if (confirmationDetails.type === 'edit') {
if (!confirmationDetails.isModifying) {
question = `Apply this change?`;
let initialIndex = 0;
if (isTrustedFolder && allowPermanentApproval) {
// It is safe to allow permanent approval for info, edit, and mcp tools
// in trusted folders because the generated policy rules are narrowed
// to specific files, patterns, or tools (rather than allowing all access).
const isSafeToPersist =
confirmationDetails.type === 'info' ||
confirmationDetails.type === 'edit' ||
confirmationDetails.type === 'mcp';
if (
isSafeToPersist &&
settings.merged.security.autoAddToPolicyByDefault
) {
const alwaysAndSaveIndex = options.findIndex(
(o) => o.value === ToolConfirmationOutcome.ProceedAlwaysAndSave,
);
if (alwaysAndSaveIndex !== -1) {
initialIndex = alwaysAndSaveIndex;
}
}
}
} else if (confirmationDetails.type === 'exec') {
const executionProps = confirmationDetails;
if (executionProps.commands && executionProps.commands.length > 1) {
question = `Allow execution of ${executionProps.commands.length} commands?`;
} else {
question = `Allow execution of: '${sanitizeForDisplay(executionProps.rootCommand)}'?`;
if (deceptiveUrlWarningText) {
securityWarnings = <WarningMessage text={deceptiveUrlWarningText} />;
}
} else if (confirmationDetails.type === 'info') {
question = `Do you want to proceed?`;
} else if (confirmationDetails.type === 'mcp') {
// mcp tool confirmation
const mcpProps = confirmationDetails;
question = `Allow execution of MCP tool "${sanitizeForDisplay(mcpProps.toolName)}" from server "${sanitizeForDisplay(mcpProps.serverName)}"?`;
}
if (confirmationDetails.type === 'edit') {
if (!confirmationDetails.isModifying) {
if (confirmationDetails.type === 'ask_user') {
bodyContent = (
<DiffRenderer
diffContent={stripUnsafeCharacters(confirmationDetails.fileDiff)}
filename={sanitizeForDisplay(confirmationDetails.fileName)}
availableTerminalHeight={availableBodyContentHeight()}
terminalWidth={terminalWidth}
<AskUserDialog
questions={confirmationDetails.questions}
onSubmit={(answers) => {
handleConfirm(ToolConfirmationOutcome.ProceedOnce, { answers });
}}
onCancel={() => {
handleConfirm(ToolConfirmationOutcome.Cancel);
}}
width={terminalWidth}
availableHeight={availableBodyContentHeight()}
/>
);
}
} else if (confirmationDetails.type === 'exec') {
const executionProps = confirmationDetails;
const commandsToDisplay =
executionProps.commands && executionProps.commands.length > 1
? executionProps.commands
: [executionProps.command];
const containsRedirection = commandsToDisplay.some((cmd) =>
hasRedirection(cmd),
);
let bodyContentHeight = availableBodyContentHeight();
let warnings: React.ReactNode = null;
if (bodyContentHeight !== undefined) {
bodyContentHeight -= 2; // Account for padding;
return {
question: '',
bodyContent,
options: [],
securityWarnings: null,
initialIndex: 0,
};
}
if (containsRedirection) {
// Calculate lines needed for Note and Tip
const safeWidth = Math.max(terminalWidth, 1);
const tipText = `Toggle auto-edit (${formatCommand(Command.CYCLE_APPROVAL_MODE)}) to allow redirection in the future.`;
if (confirmationDetails.type === 'exit_plan_mode') {
bodyContent = (
<ExitPlanModeDialog
planPath={confirmationDetails.planPath}
getPreferredEditor={getPreferredEditor}
onApprove={(approvalMode) => {
handleConfirm(ToolConfirmationOutcome.ProceedOnce, {
approved: true,
approvalMode,
});
}}
onFeedback={(feedback) => {
handleConfirm(ToolConfirmationOutcome.ProceedOnce, {
approved: false,
feedback,
});
}}
onCancel={() => {
handleConfirm(ToolConfirmationOutcome.Cancel);
}}
width={terminalWidth}
availableHeight={availableBodyContentHeight()}
/>
);
return {
question: '',
bodyContent,
options: [],
securityWarnings: null,
initialIndex: 0,
};
}
const noteLength =
REDIRECTION_WARNING_NOTE_LABEL.length +
REDIRECTION_WARNING_NOTE_TEXT.length;
const tipLength = REDIRECTION_WARNING_TIP_LABEL.length + tipText.length;
if (confirmationDetails.type === 'edit') {
if (!confirmationDetails.isModifying) {
question = `Apply this change?`;
}
} else if (confirmationDetails.type === 'exec') {
const executionProps = confirmationDetails;
const noteLines = Math.ceil(noteLength / safeWidth);
const tipLines = Math.ceil(tipLength / safeWidth);
const spacerLines = 1;
const warningHeight = noteLines + tipLines + spacerLines;
if (executionProps.commands && executionProps.commands.length > 1) {
question = `Allow execution of ${executionProps.commands.length} commands?`;
} else {
question = `Allow execution of: '${sanitizeForDisplay(executionProps.rootCommand)}'?`;
}
} else if (confirmationDetails.type === 'info') {
question = `Do you want to proceed?`;
} else if (confirmationDetails.type === 'mcp') {
// mcp tool confirmation
const mcpProps = confirmationDetails;
question = `Allow execution of MCP tool "${sanitizeForDisplay(mcpProps.toolName)}" from server "${sanitizeForDisplay(mcpProps.serverName)}"?`;
}
if (confirmationDetails.type === 'edit') {
if (!confirmationDetails.isModifying) {
bodyContent = (
<DiffRenderer
diffContent={stripUnsafeCharacters(confirmationDetails.fileDiff)}
filename={sanitizeForDisplay(confirmationDetails.fileName)}
availableTerminalHeight={availableBodyContentHeight()}
terminalWidth={terminalWidth}
/>
);
}
} else if (confirmationDetails.type === 'exec') {
const executionProps = confirmationDetails;
const commandsToDisplay =
executionProps.commands && executionProps.commands.length > 1
? executionProps.commands
: [executionProps.command];
const containsRedirection = commandsToDisplay.some((cmd) =>
hasRedirection(cmd),
);
let bodyContentHeight = availableBodyContentHeight();
let warnings: React.ReactNode = null;
if (bodyContentHeight !== undefined) {
bodyContentHeight = Math.max(
bodyContentHeight - warningHeight,
MINIMUM_MAX_HEIGHT,
bodyContentHeight -= 2; // Account for padding;
}
if (containsRedirection) {
// Calculate lines needed for Note and Tip
const safeWidth = Math.max(terminalWidth, 1);
const noteLength =
REDIRECTION_WARNING_NOTE_LABEL.length +
REDIRECTION_WARNING_NOTE_TEXT.length;
const tipText = `Toggle auto-edit (${formatCommand(Command.CYCLE_APPROVAL_MODE)}) to allow redirection in the future.`;
const tipLength =
REDIRECTION_WARNING_TIP_LABEL.length + tipText.length;
const noteLines = Math.ceil(noteLength / safeWidth);
const tipLines = Math.ceil(tipLength / safeWidth);
const spacerLines = 1;
const warningHeight = noteLines + tipLines + spacerLines;
if (bodyContentHeight !== undefined) {
bodyContentHeight = Math.max(
bodyContentHeight - warningHeight,
MINIMUM_MAX_HEIGHT,
);
}
warnings = (
<>
<Box height={1} />
<Box>
<Text color={theme.text.primary}>
<Text bold>{REDIRECTION_WARNING_NOTE_LABEL}</Text>
{REDIRECTION_WARNING_NOTE_TEXT}
</Text>
</Box>
<Box>
<Text color={theme.border.default}>
<Text bold>{REDIRECTION_WARNING_TIP_LABEL}</Text>
{tipText}
</Text>
</Box>
</>
);
}
warnings = (
<>
<Box height={1} />
<Box>
<Text color={theme.text.primary}>
<Text bold>{REDIRECTION_WARNING_NOTE_LABEL}</Text>
{REDIRECTION_WARNING_NOTE_TEXT}
bodyContent = (
<Box flexDirection="column">
<MaxSizedBox
maxHeight={bodyContentHeight}
maxWidth={Math.max(terminalWidth, 1)}
>
<Box flexDirection="column">
{commandsToDisplay.map((cmd, idx) => (
<Box
key={idx}
flexDirection="column"
paddingBottom={idx < commandsToDisplay.length - 1 ? 1 : 0}
>
{colorizeCode({
code: cmd,
language: 'bash',
maxWidth: Math.max(terminalWidth, 1),
settings,
hideLineNumbers: true,
})}
</Box>
))}
</Box>
</MaxSizedBox>
{warnings}
</Box>
);
} else if (confirmationDetails.type === 'info') {
const infoProps = confirmationDetails;
const displayUrls =
infoProps.urls &&
!(
infoProps.urls.length === 1 &&
infoProps.urls[0] === infoProps.prompt
);
bodyContent = (
<Box flexDirection="column">
<Text color={theme.text.link}>
<RenderInline
text={infoProps.prompt}
defaultColor={theme.text.link}
/>
</Text>
{displayUrls && infoProps.urls && infoProps.urls.length > 0 && (
<Box flexDirection="column" marginTop={1}>
<Text color={theme.text.primary}>URLs to fetch:</Text>
{infoProps.urls.map((urlString) => (
<Text key={urlString}>
{' '}
- <RenderInline text={toUnicodeUrl(urlString)} />
</Text>
))}
</Box>
)}
</Box>
);
} else if (confirmationDetails.type === 'mcp') {
// mcp tool confirmation
const mcpProps = confirmationDetails;
bodyContent = (
<Box flexDirection="column">
<>
<Text color={theme.text.link}>
MCP Server: {sanitizeForDisplay(mcpProps.serverName)}
</Text>
</Box>
<Box>
<Text color={theme.border.default}>
<Text bold>{REDIRECTION_WARNING_TIP_LABEL}</Text>
{tipText}
<Text color={theme.text.link}>
Tool: {sanitizeForDisplay(mcpProps.toolName)}
</Text>
</Box>
</>
</>
{hasMcpToolDetails && (
<Box flexDirection="column" marginTop={1}>
<Text color={theme.text.primary}>MCP Tool Details:</Text>
{isMcpToolDetailsExpanded ? (
<>
<Text color={theme.text.secondary}>
(press {expandDetailsHintKey} to collapse MCP tool
details)
</Text>
<Text color={theme.text.link}>{mcpToolDetailsText}</Text>
</>
) : (
<Text color={theme.text.secondary}>
(press {expandDetailsHintKey} to expand MCP tool details)
</Text>
)}
</Box>
)}
</Box>
);
}
bodyContent = (
<Box flexDirection="column">
<MaxSizedBox
maxHeight={bodyContentHeight}
maxWidth={Math.max(terminalWidth, 1)}
>
<Box flexDirection="column">
{commandsToDisplay.map((cmd, idx) => (
<Box
key={idx}
flexDirection="column"
paddingBottom={idx < commandsToDisplay.length - 1 ? 1 : 0}
>
{colorizeCode({
code: cmd,
language: 'bash',
maxWidth: Math.max(terminalWidth, 1),
settings,
hideLineNumbers: true,
})}
</Box>
))}
</Box>
</MaxSizedBox>
{warnings}
</Box>
);
} else if (confirmationDetails.type === 'info') {
const infoProps = confirmationDetails;
const displayUrls =
infoProps.urls &&
!(
infoProps.urls.length === 1 && infoProps.urls[0] === infoProps.prompt
);
bodyContent = (
<Box flexDirection="column">
<Text color={theme.text.link}>
<RenderInline
text={infoProps.prompt}
defaultColor={theme.text.link}
/>
</Text>
{displayUrls && infoProps.urls && infoProps.urls.length > 0 && (
<Box flexDirection="column" marginTop={1}>
<Text color={theme.text.primary}>URLs to fetch:</Text>
{infoProps.urls.map((urlString) => (
<Text key={urlString}>
{' '}
- <RenderInline text={toUnicodeUrl(urlString)} />
</Text>
))}
</Box>
)}
</Box>
);
} else if (confirmationDetails.type === 'mcp') {
// mcp tool confirmation
const mcpProps = confirmationDetails;
bodyContent = (
<Box flexDirection="column">
<>
<Text color={theme.text.link}>
MCP Server: {sanitizeForDisplay(mcpProps.serverName)}
</Text>
<Text color={theme.text.link}>
Tool: {sanitizeForDisplay(mcpProps.toolName)}
</Text>
</>
{hasMcpToolDetails && (
<Box flexDirection="column" marginTop={1}>
<Text color={theme.text.primary}>MCP Tool Details:</Text>
{isMcpToolDetailsExpanded ? (
<>
<Text color={theme.text.secondary}>
(press {expandDetailsHintKey} to collapse MCP tool details)
</Text>
<Text color={theme.text.link}>{mcpToolDetailsText}</Text>
</>
) : (
<Text color={theme.text.secondary}>
(press {expandDetailsHintKey} to expand MCP tool details)
</Text>
)}
</Box>
)}
</Box>
);
}
return { question, bodyContent, options, securityWarnings };
}, [
confirmationDetails,
getOptions,
availableBodyContentHeight,
terminalWidth,
handleConfirm,
deceptiveUrlWarningText,
isMcpToolDetailsExpanded,
hasMcpToolDetails,
mcpToolDetailsText,
expandDetailsHintKey,
getPreferredEditor,
settings,
]);
return { question, bodyContent, options, securityWarnings, initialIndex };
}, [
confirmationDetails,
getOptions,
availableBodyContentHeight,
terminalWidth,
handleConfirm,
deceptiveUrlWarningText,
isMcpToolDetailsExpanded,
hasMcpToolDetails,
mcpToolDetailsText,
expandDetailsHintKey,
getPreferredEditor,
isTrustedFolder,
allowPermanentApproval,
settings,
]);
const bodyOverflowDirection: 'top' | 'bottom' =
confirmationDetails.type === 'mcp' && isMcpToolDetailsExpanded
@@ -710,6 +745,7 @@ export const ToolConfirmationMessage: React.FC<
items={options}
onSelect={handleSelect}
isFocused={isFocused}
initialIndex={initialIndex}
/>
</Box>
</>
@@ -1,5 +1,21 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
exports[`ToolConfirmationMessage > enablePermanentToolApproval setting > should show "Allow for all future sessions" when trusted 1`] = `
"╭──────────────────────────────────────────────────────────────────────────────╮
│ │
│ No changes detected. │
│ │
╰──────────────────────────────────────────────────────────────────────────────╯
Apply this change?
● 1. Allow once
2. Allow for this session
3. Allow for this file in all future sessions
4. Modify with external editor
5. No, suggest changes (esc)
"
`;
exports[`ToolConfirmationMessage > should display multiple commands for exec type when provided 1`] = `
"echo "hello"
@@ -3510,6 +3510,116 @@ describe('useGeminiStream', () => {
expect(result.current.loopDetectionConfirmationRequest).not.toBeNull();
});
});
describe('Race Condition Prevention', () => {
it('should reject concurrent submitQuery when already responding', async () => {
// Stream that stays open (simulates "still responding")
mockSendMessageStream.mockReturnValue(
(async function* () {
yield {
type: ServerGeminiEventType.Content,
value: 'First response',
};
// Keep the stream open
await new Promise(() => {});
})(),
);
const { result } = renderTestHook();
// Start first query without awaiting (fire-and-forget, like existing tests)
await act(async () => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
result.current.submitQuery('first query');
});
// Wait for the stream to start responding
await waitFor(() => {
expect(result.current.streamingState).toBe(StreamingState.Responding);
});
// Try a second query while first is still responding
await act(async () => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
result.current.submitQuery('second query');
});
// Should have only called sendMessageStream once (second was rejected)
expect(mockSendMessageStream).toHaveBeenCalledTimes(1);
});
it('should allow continuation queries via loop detection retry', async () => {
const mockLoopDetectionService = {
disableForSession: vi.fn(),
};
const mockClient = {
...new MockedGeminiClientClass(mockConfig),
getLoopDetectionService: () => mockLoopDetectionService,
};
mockConfig.getGeminiClient = vi.fn().mockReturnValue(mockClient);
// First call triggers loop detection
mockSendMessageStream.mockReturnValueOnce(
(async function* () {
yield {
type: ServerGeminiEventType.LoopDetected,
};
})(),
);
// Retry call succeeds
mockSendMessageStream.mockReturnValueOnce(
(async function* () {
yield {
type: ServerGeminiEventType.Content,
value: 'Retry success',
};
yield {
type: ServerGeminiEventType.Finished,
value: { reason: 'STOP' },
};
})(),
);
const { result } = renderTestHook();
await act(async () => {
await result.current.submitQuery('test query');
});
await waitFor(() => {
expect(
result.current.loopDetectionConfirmationRequest,
).not.toBeNull();
});
// User selects "disable" which triggers a continuation query
await act(async () => {
result.current.loopDetectionConfirmationRequest?.onComplete({
userSelection: 'disable',
});
});
// Verify disableForSession was called
expect(
mockLoopDetectionService.disableForSession,
).toHaveBeenCalledTimes(1);
// Continuation query should have gone through (2 total calls)
await waitFor(() => {
expect(mockSendMessageStream).toHaveBeenCalledTimes(2);
expect(mockSendMessageStream).toHaveBeenNthCalledWith(
2,
'test query',
expect.any(AbortSignal),
expect.any(String),
undefined,
false,
'test query',
);
});
});
});
});
describe('Agent Execution Events', () => {
+34 -14
View File
@@ -216,7 +216,15 @@ export const useGeminiStream = (
const previousApprovalModeRef = useRef<ApprovalMode>(
config.getApprovalMode(),
);
const [isResponding, setIsResponding] = useState<boolean>(false);
const [isResponding, setIsRespondingState] = useState<boolean>(false);
const isRespondingRef = useRef<boolean>(false);
const setIsResponding = useCallback(
(value: boolean) => {
setIsRespondingState(value);
isRespondingRef.current = value;
},
[setIsRespondingState],
);
const [thought, thoughtRef, setThought] =
useStateAndRef<ThoughtSummary | null>(null);
const [pendingHistoryItem, pendingHistoryItemRef, setPendingHistoryItem] =
@@ -320,11 +328,14 @@ export const useGeminiStream = (
return (executingShellTool as TrackedExecutingToolCall | undefined)?.pid;
}, [toolCalls]);
const onExec = useCallback(async (done: Promise<void>) => {
setIsResponding(true);
await done;
setIsResponding(false);
}, []);
const onExec = useCallback(
async (done: Promise<void>) => {
setIsResponding(true);
await done;
setIsResponding(false);
},
[setIsResponding],
);
const {
handleShellCommand,
@@ -538,7 +549,7 @@ export const useGeminiStream = (
setIsResponding(false);
}
prevActiveShellPtyIdRef.current = activeShellPtyId;
}, [activeShellPtyId, addItem]);
}, [activeShellPtyId, addItem, setIsResponding]);
useEffect(() => {
if (
@@ -700,6 +711,7 @@ export const useGeminiStream = (
cancelAllToolCalls,
toolCalls,
activeShellPtyId,
setIsResponding,
]);
useKeypress(
@@ -952,7 +964,13 @@ export const useGeminiStream = (
setIsResponding(false);
setThought(null); // Reset thought when user cancels
},
[addItem, pendingHistoryItemRef, setPendingHistoryItem, setThought],
[
addItem,
pendingHistoryItemRef,
setPendingHistoryItem,
setThought,
setIsResponding,
],
);
const handleErrorEvent = useCallback(
@@ -1358,14 +1376,15 @@ export const useGeminiStream = (
async ({ metadata: spanMetadata }) => {
spanMetadata.input = query;
const queryId = `${Date.now()}-${Math.random()}`;
activeQueryIdRef.current = queryId;
if (
(streamingState === StreamingState.Responding ||
(isRespondingRef.current ||
streamingState === StreamingState.Responding ||
streamingState === StreamingState.WaitingForConfirmation) &&
!options?.isContinuation
)
return;
const queryId = `${Date.now()}-${Math.random()}`;
activeQueryIdRef.current = queryId;
const userMessageTimestamp = Date.now();
@@ -1452,7 +1471,7 @@ export const useGeminiStream = (
loopDetectedRef.current = false;
// Show the confirmation dialog to choose whether to disable loop detection
setLoopDetectionConfirmationRequest({
onComplete: (result: {
onComplete: async (result: {
userSelection: 'disable' | 'keep';
}) => {
setLoopDetectionConfirmationRequest(null);
@@ -1468,8 +1487,7 @@ export const useGeminiStream = (
});
if (lastQueryRef.current && lastPromptIdRef.current) {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
submitQuery(
await submitQuery(
lastQueryRef.current,
{ isContinuation: true },
lastPromptIdRef.current,
@@ -1537,6 +1555,7 @@ export const useGeminiStream = (
maybeAddSuppressedToolErrorNote,
maybeAddLowVerbosityFailureNote,
settings.merged.billing?.overageStrategy,
setIsResponding,
],
);
@@ -1803,6 +1822,7 @@ export const useGeminiStream = (
isLowErrorVerbosity,
maybeAddSuppressedToolErrorNote,
maybeAddLowVerbosityFailureNote,
setIsResponding,
],
);
+1
View File
@@ -74,6 +74,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
input: 0,
duration_ms: 0,
tool_calls: 0,
models: {},
}),
})),
uiTelemetryService: {
@@ -70,7 +70,7 @@ class McpToolInvocation extends BaseToolInvocation<
};
}
protected override getPolicyUpdateOptions(
override getPolicyUpdateOptions(
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
return {
@@ -177,7 +177,7 @@ class TypeTextInvocation extends BaseToolInvocation<
};
}
protected override getPolicyUpdateOptions(
override getPolicyUpdateOptions(
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
return {
+33 -30
View File
@@ -553,6 +553,7 @@ export interface ConfigParameters {
truncateToolOutputThreshold?: number;
eventEmitter?: EventEmitter;
useWriteTodos?: boolean;
workspacePoliciesDir?: string;
policyEngineConfig?: PolicyEngineConfig;
directWebFetch?: boolean;
policyUpdateConfirmationRequest?: PolicyUpdateConfirmationRequest;
@@ -746,6 +747,7 @@ export class Config implements McpContext, AgentLoopContext {
private readonly fileExclusions: FileExclusions;
private readonly eventEmitter?: EventEmitter;
private readonly useWriteTodos: boolean;
private readonly workspacePoliciesDir: string | undefined;
private readonly _messageBus: MessageBus;
private readonly policyEngine: PolicyEngine;
private policyUpdateConfirmationRequest:
@@ -956,6 +958,7 @@ export class Config implements McpContext, AgentLoopContext {
this.useWriteTodos = isPreviewModel(this.model)
? false
: (params.useWriteTodos ?? true);
this.workspacePoliciesDir = params.workspacePoliciesDir;
this.enableHooksUI = params.enableHooksUI ?? true;
this.enableHooks = params.enableHooks ?? true;
this.disabledHooks = params.disabledHooks ?? [];
@@ -1187,7 +1190,7 @@ export class Config implements McpContext, AgentLoopContext {
if (this.getSkillManager().getSkills().length > 0) {
this.getToolRegistry().unregisterTool(ActivateSkillTool.Name);
this.getToolRegistry().registerTool(
new ActivateSkillTool(this, this._messageBus),
new ActivateSkillTool(this, this.messageBus),
);
}
}
@@ -1999,6 +2002,10 @@ export class Config implements McpContext, AgentLoopContext {
return this.geminiMdFilePaths;
}
getWorkspacePoliciesDir(): string | undefined {
return this.workspacePoliciesDir;
}
setGeminiMdFilePaths(paths: string[]): void {
this.geminiMdFilePaths = paths;
}
@@ -2621,7 +2628,7 @@ export class Config implements McpContext, AgentLoopContext {
if (this.getSkillManager().getSkills().length > 0) {
this.getToolRegistry().unregisterTool(ActivateSkillTool.Name);
this.getToolRegistry().registerTool(
new ActivateSkillTool(this, this._messageBus),
new ActivateSkillTool(this, this.messageBus),
);
} else {
this.getToolRegistry().unregisterTool(ActivateSkillTool.Name);
@@ -2805,7 +2812,7 @@ export class Config implements McpContext, AgentLoopContext {
}
async createToolRegistry(): Promise<ToolRegistry> {
const registry = new ToolRegistry(this, this._messageBus);
const registry = new ToolRegistry(this, this.messageBus);
// helper to create & register core tools that are enabled
const maybeRegister = (
@@ -2835,10 +2842,10 @@ export class Config implements McpContext, AgentLoopContext {
};
maybeRegister(LSTool, () =>
registry.registerTool(new LSTool(this, this._messageBus)),
registry.registerTool(new LSTool(this, this.messageBus)),
);
maybeRegister(ReadFileTool, () =>
registry.registerTool(new ReadFileTool(this, this._messageBus)),
registry.registerTool(new ReadFileTool(this, this.messageBus)),
);
if (this.getUseRipgrep()) {
@@ -2851,85 +2858,81 @@ export class Config implements McpContext, AgentLoopContext {
}
if (useRipgrep) {
maybeRegister(RipGrepTool, () =>
registry.registerTool(new RipGrepTool(this, this._messageBus)),
registry.registerTool(new RipGrepTool(this, this.messageBus)),
);
} else {
logRipgrepFallback(this, new RipgrepFallbackEvent(errorString));
maybeRegister(GrepTool, () =>
registry.registerTool(new GrepTool(this, this._messageBus)),
registry.registerTool(new GrepTool(this, this.messageBus)),
);
}
} else {
maybeRegister(GrepTool, () =>
registry.registerTool(new GrepTool(this, this._messageBus)),
registry.registerTool(new GrepTool(this, this.messageBus)),
);
}
maybeRegister(GlobTool, () =>
registry.registerTool(new GlobTool(this, this._messageBus)),
registry.registerTool(new GlobTool(this, this.messageBus)),
);
maybeRegister(ActivateSkillTool, () =>
registry.registerTool(new ActivateSkillTool(this, this._messageBus)),
registry.registerTool(new ActivateSkillTool(this, this.messageBus)),
);
maybeRegister(EditTool, () =>
registry.registerTool(new EditTool(this, this._messageBus)),
registry.registerTool(new EditTool(this, this.messageBus)),
);
maybeRegister(WriteFileTool, () =>
registry.registerTool(new WriteFileTool(this, this._messageBus)),
registry.registerTool(new WriteFileTool(this, this.messageBus)),
);
maybeRegister(WebFetchTool, () =>
registry.registerTool(new WebFetchTool(this, this._messageBus)),
registry.registerTool(new WebFetchTool(this, this.messageBus)),
);
maybeRegister(ShellTool, () =>
registry.registerTool(new ShellTool(this, this._messageBus)),
registry.registerTool(new ShellTool(this, this.messageBus)),
);
maybeRegister(MemoryTool, () =>
registry.registerTool(new MemoryTool(this._messageBus)),
registry.registerTool(new MemoryTool(this.messageBus)),
);
maybeRegister(WebSearchTool, () =>
registry.registerTool(new WebSearchTool(this, this._messageBus)),
registry.registerTool(new WebSearchTool(this, this.messageBus)),
);
maybeRegister(AskUserTool, () =>
registry.registerTool(new AskUserTool(this._messageBus)),
registry.registerTool(new AskUserTool(this.messageBus)),
);
if (this.getUseWriteTodos()) {
maybeRegister(WriteTodosTool, () =>
registry.registerTool(new WriteTodosTool(this._messageBus)),
registry.registerTool(new WriteTodosTool(this.messageBus)),
);
}
if (this.isPlanEnabled()) {
maybeRegister(ExitPlanModeTool, () =>
registry.registerTool(new ExitPlanModeTool(this, this._messageBus)),
registry.registerTool(new ExitPlanModeTool(this, this.messageBus)),
);
maybeRegister(EnterPlanModeTool, () =>
registry.registerTool(new EnterPlanModeTool(this, this._messageBus)),
registry.registerTool(new EnterPlanModeTool(this, this.messageBus)),
);
}
if (this.isTrackerEnabled()) {
maybeRegister(TrackerCreateTaskTool, () =>
registry.registerTool(
new TrackerCreateTaskTool(this, this._messageBus),
),
registry.registerTool(new TrackerCreateTaskTool(this, this.messageBus)),
);
maybeRegister(TrackerUpdateTaskTool, () =>
registry.registerTool(
new TrackerUpdateTaskTool(this, this._messageBus),
),
registry.registerTool(new TrackerUpdateTaskTool(this, this.messageBus)),
);
maybeRegister(TrackerGetTaskTool, () =>
registry.registerTool(new TrackerGetTaskTool(this, this._messageBus)),
registry.registerTool(new TrackerGetTaskTool(this, this.messageBus)),
);
maybeRegister(TrackerListTasksTool, () =>
registry.registerTool(new TrackerListTasksTool(this, this._messageBus)),
registry.registerTool(new TrackerListTasksTool(this, this.messageBus)),
);
maybeRegister(TrackerAddDependencyTool, () =>
registry.registerTool(
new TrackerAddDependencyTool(this, this._messageBus),
new TrackerAddDependencyTool(this, this.messageBus),
),
);
maybeRegister(TrackerVisualizeTool, () =>
registry.registerTool(new TrackerVisualizeTool(this, this._messageBus)),
registry.registerTool(new TrackerVisualizeTool(this, this.messageBus)),
);
}
+7
View File
@@ -172,6 +172,13 @@ export class Storage {
return path.join(this.getGeminiDir(), 'policies');
}
getWorkspaceAutoSavedPolicyPath(): string {
return path.join(
this.getWorkspacePoliciesDir(),
AUTO_SAVED_POLICY_FILENAME,
);
}
getAutoSavedPolicyPath(): string {
return path.join(Storage.getUserPoliciesDir(), AUTO_SAVED_POLICY_FILENAME);
}
@@ -122,6 +122,7 @@ export interface UpdatePolicy {
type: MessageBusType.UPDATE_POLICY;
toolName: string;
persist?: boolean;
persistScope?: 'workspace' | 'user';
argsPattern?: string;
commandPrefix?: string | string[];
mcpName?: string;
@@ -154,6 +154,7 @@ describe('StreamJsonFormatter', () => {
input: 50,
duration_ms: 1200,
tool_calls: 2,
models: {},
},
};
@@ -180,6 +181,7 @@ describe('StreamJsonFormatter', () => {
input: 50,
duration_ms: 1200,
tool_calls: 0,
models: {},
},
};
@@ -304,6 +306,15 @@ describe('StreamJsonFormatter', () => {
input: 50,
duration_ms: 1200,
tool_calls: 2,
models: {
'gemini-2.0-flash': {
total_tokens: 80,
input_tokens: 50,
output_tokens: 30,
cached: 0,
input: 50,
},
},
});
});
@@ -347,6 +358,22 @@ describe('StreamJsonFormatter', () => {
input: 150,
duration_ms: 3000,
tool_calls: 5,
models: {
'gemini-pro': {
total_tokens: 80,
input_tokens: 50,
output_tokens: 30,
cached: 0,
input: 50,
},
'gemini-ultra': {
total_tokens: 170,
input_tokens: 100,
output_tokens: 70,
cached: 0,
input: 100,
},
},
});
});
@@ -376,6 +403,15 @@ describe('StreamJsonFormatter', () => {
input: 20,
duration_ms: 1200,
tool_calls: 0,
models: {
'gemini-pro': {
total_tokens: 80,
input_tokens: 50,
output_tokens: 30,
cached: 30,
input: 20,
},
},
});
});
@@ -392,6 +428,7 @@ describe('StreamJsonFormatter', () => {
input: 0,
duration_ms: 100,
tool_calls: 0,
models: {},
});
});
@@ -521,6 +558,7 @@ describe('StreamJsonFormatter', () => {
input: 0,
duration_ms: 0,
tool_calls: 0,
models: {},
},
} as ResultEvent,
];
@@ -544,6 +582,7 @@ describe('StreamJsonFormatter', () => {
input: 50,
duration_ms: 1200,
tool_calls: 2,
models: {},
},
};
@@ -4,7 +4,11 @@
* SPDX-License-Identifier: Apache-2.0
*/
import type { JsonStreamEvent, StreamStats } from './types.js';
import type {
JsonStreamEvent,
ModelStreamStats,
StreamStats,
} from './types.js';
import type { SessionMetrics } from '../telemetry/uiTelemetry.js';
/**
@@ -31,7 +35,7 @@ export class StreamJsonFormatter {
/**
* Converts SessionMetrics to simplified StreamStats format.
* Aggregates token counts across all models.
* Includes per-model token breakdowns and aggregated totals.
* @param metrics - The session metrics from telemetry
* @param durationMs - The session duration in milliseconds
* @returns Simplified stats for streaming output
@@ -40,20 +44,35 @@ export class StreamJsonFormatter {
metrics: SessionMetrics,
durationMs: number,
): StreamStats {
let totalTokens = 0;
let inputTokens = 0;
let outputTokens = 0;
let cached = 0;
let input = 0;
const { totalTokens, inputTokens, outputTokens, cached, input, models } =
Object.entries(metrics.models).reduce(
(acc, [modelName, modelMetrics]) => {
const modelStats: ModelStreamStats = {
total_tokens: modelMetrics.tokens.total,
input_tokens: modelMetrics.tokens.prompt,
output_tokens: modelMetrics.tokens.candidates,
cached: modelMetrics.tokens.cached,
input: modelMetrics.tokens.input,
};
// Aggregate token counts across all models
for (const modelMetrics of Object.values(metrics.models)) {
totalTokens += modelMetrics.tokens.total;
inputTokens += modelMetrics.tokens.prompt;
outputTokens += modelMetrics.tokens.candidates;
cached += modelMetrics.tokens.cached;
input += modelMetrics.tokens.input;
}
acc.models[modelName] = modelStats;
acc.totalTokens += modelStats.total_tokens;
acc.inputTokens += modelStats.input_tokens;
acc.outputTokens += modelStats.output_tokens;
acc.cached += modelStats.cached;
acc.input += modelStats.input;
return acc;
},
{
totalTokens: 0,
inputTokens: 0,
outputTokens: 0,
cached: 0,
input: 0,
models: {} as Record<string, ModelStreamStats>,
},
);
return {
total_tokens: totalTokens,
@@ -63,6 +82,7 @@ export class StreamJsonFormatter {
input,
duration_ms: durationMs,
tool_calls: metrics.tools.totalCalls,
models,
};
}
}
+9
View File
@@ -77,6 +77,14 @@ export interface ErrorEvent extends BaseJsonStreamEvent {
message: string;
}
export interface ModelStreamStats {
total_tokens: number;
input_tokens: number;
output_tokens: number;
cached: number;
input: number;
}
export interface StreamStats {
total_tokens: number;
input_tokens: number;
@@ -86,6 +94,7 @@ export interface StreamStats {
input: number;
duration_ms: number;
tool_calls: number;
models: Record<string, ModelStreamStats>;
}
export interface ResultEvent extends BaseJsonStreamEvent {
+63 -17
View File
@@ -29,7 +29,7 @@ import { type MessageBus } from '../confirmation-bus/message-bus.js';
import { coreEvents } from '../utils/events.js';
import { debugLogger } from '../utils/debugLogger.js';
import { SHELL_TOOL_NAMES } from '../utils/shell-utils.js';
import { SHELL_TOOL_NAME } from '../tools/tool-names.js';
import { SHELL_TOOL_NAME, SENSITIVE_TOOLS } from '../tools/tool-names.js';
import { isNodeError } from '../utils/errors.js';
import { MCP_TOOL_PREFIX } from '../tools/mcp-tool.js';
@@ -46,13 +46,20 @@ export const WORKSPACE_POLICY_TIER = 3;
export const USER_POLICY_TIER = 4;
export const ADMIN_POLICY_TIER = 5;
// Specific priority offsets and derived priorities for dynamic/settings rules.
// These are added to the tier base (e.g., USER_POLICY_TIER).
/**
* The fractional priority of "Always allow" rules (e.g., 950/1000).
* Higher fraction within a tier wins.
*/
export const ALWAYS_ALLOW_PRIORITY_FRACTION = 950;
// Workspace tier (3) + high priority (950/1000) = ALWAYS_ALLOW_PRIORITY
// This ensures user "always allow" selections are high priority
// within the workspace tier but still lose to user/admin policies.
export const ALWAYS_ALLOW_PRIORITY = WORKSPACE_POLICY_TIER + 0.95;
/**
* The fractional priority offset for "Always allow" rules (e.g., 0.95).
* This ensures consistency between in-memory rules and persisted rules.
*/
export const ALWAYS_ALLOW_PRIORITY_OFFSET =
ALWAYS_ALLOW_PRIORITY_FRACTION / 1000;
// Specific priority offsets and derived priorities for dynamic/settings rules.
export const MCP_EXCLUDED_PRIORITY = USER_POLICY_TIER + 0.9;
export const EXCLUDE_TOOLS_FLAG_PRIORITY = USER_POLICY_TIER + 0.4;
@@ -60,6 +67,18 @@ export const ALLOWED_TOOLS_FLAG_PRIORITY = USER_POLICY_TIER + 0.3;
export const TRUSTED_MCP_SERVER_PRIORITY = USER_POLICY_TIER + 0.2;
export const ALLOWED_MCP_SERVER_PRIORITY = USER_POLICY_TIER + 0.1;
// These are added to the tier base (e.g., USER_POLICY_TIER).
// Workspace tier (3) + high priority (950/1000) = ALWAYS_ALLOW_PRIORITY
export const ALWAYS_ALLOW_PRIORITY =
WORKSPACE_POLICY_TIER + ALWAYS_ALLOW_PRIORITY_OFFSET;
/**
* Returns the fractional priority of ALWAYS_ALLOW_PRIORITY scaled to 1000.
*/
export function getAlwaysAllowPriorityFraction(): number {
return Math.round((ALWAYS_ALLOW_PRIORITY % 1) * 1000);
}
/**
* Gets the list of directories to search for policy files, in order of increasing priority
* (Default -> Extension -> Workspace -> User -> Admin).
@@ -492,6 +511,19 @@ export function createPolicyUpdater(
if (message.commandPrefix) {
// Convert commandPrefix(es) to argsPatterns for in-memory rules
const patterns = buildArgsPatterns(undefined, message.commandPrefix);
const tier =
message.persistScope === 'user'
? USER_POLICY_TIER
: WORKSPACE_POLICY_TIER;
const priority = tier + getAlwaysAllowPriorityFraction() / 1000;
if (SENSITIVE_TOOLS.has(toolName) && !message.commandPrefix) {
debugLogger.warn(
`Attempted to update policy for sensitive tool '${toolName}' without a commandPrefix. Skipping.`,
);
return;
}
for (const pattern of patterns) {
if (pattern) {
// Note: patterns from buildArgsPatterns are derived from escapeRegex,
@@ -499,7 +531,7 @@ export function createPolicyUpdater(
policyEngine.addRule({
toolName,
decision: PolicyDecision.ALLOW,
priority: ALWAYS_ALLOW_PRIORITY,
priority,
argsPattern: new RegExp(pattern),
source: 'Dynamic (Confirmed)',
});
@@ -518,10 +550,23 @@ export function createPolicyUpdater(
? new RegExp(message.argsPattern)
: undefined;
const tier =
message.persistScope === 'user'
? USER_POLICY_TIER
: WORKSPACE_POLICY_TIER;
const priority = tier + getAlwaysAllowPriorityFraction() / 1000;
if (SENSITIVE_TOOLS.has(toolName) && !message.argsPattern) {
debugLogger.warn(
`Attempted to update policy for sensitive tool '${toolName}' without an argsPattern. Skipping.`,
);
return;
}
policyEngine.addRule({
toolName,
decision: PolicyDecision.ALLOW,
priority: ALWAYS_ALLOW_PRIORITY,
priority,
argsPattern,
source: 'Dynamic (Confirmed)',
});
@@ -530,7 +575,10 @@ export function createPolicyUpdater(
if (message.persist) {
persistenceQueue = persistenceQueue.then(async () => {
try {
const policyFile = storage.getAutoSavedPolicyPath();
const policyFile =
message.persistScope === 'workspace'
? storage.getWorkspaceAutoSavedPolicyPath()
: storage.getAutoSavedPolicyPath();
await fs.mkdir(path.dirname(policyFile), { recursive: true });
// Read existing file
@@ -560,21 +608,19 @@ export function createPolicyUpdater(
}
// Create new rule object
const newRule: TomlRule = {};
const newRule: TomlRule = {
decision: 'allow',
priority: getAlwaysAllowPriorityFraction(),
};
if (message.mcpName) {
newRule.mcpName = message.mcpName;
// Extract simple tool name
const simpleToolName = toolName.startsWith(`${message.mcpName}__`)
newRule.toolName = toolName.startsWith(`${message.mcpName}__`)
? toolName.slice(message.mcpName.length + 2)
: toolName;
newRule.toolName = simpleToolName;
newRule.decision = 'allow';
newRule.priority = 200;
} else {
newRule.toolName = toolName;
newRule.decision = 'allow';
newRule.priority = 100;
}
if (message.commandPrefix) {
+116 -135
View File
@@ -4,25 +4,22 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {
describe,
it,
expect,
vi,
beforeEach,
afterEach,
type Mock,
} from 'vitest';
import * as fs from 'node:fs/promises';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import * as path from 'node:path';
import { createPolicyUpdater, ALWAYS_ALLOW_PRIORITY } from './config.js';
import {
createPolicyUpdater,
getAlwaysAllowPriorityFraction,
} from './config.js';
import { PolicyEngine } from './policy-engine.js';
import { MessageBus } from '../confirmation-bus/message-bus.js';
import { MessageBusType } from '../confirmation-bus/types.js';
import { Storage, AUTO_SAVED_POLICY_FILENAME } from '../config/storage.js';
import { ApprovalMode } from './types.js';
import { vol, fs as memfs } from 'memfs';
// Use memfs for all fs operations in this test
vi.mock('node:fs/promises', () => import('memfs').then((m) => m.fs.promises));
vi.mock('node:fs/promises');
vi.mock('../config/storage.js');
describe('createPolicyUpdater', () => {
@@ -31,6 +28,8 @@ describe('createPolicyUpdater', () => {
let mockStorage: Storage;
beforeEach(() => {
vi.useFakeTimers();
vol.reset();
policyEngine = new PolicyEngine({
rules: [],
checkers: [],
@@ -43,202 +42,184 @@ describe('createPolicyUpdater', () => {
afterEach(() => {
vi.restoreAllMocks();
vi.useRealTimers();
});
it('should persist policy when persist flag is true', async () => {
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const userPoliciesDir = '/mock/user/.gemini/policies';
const policyFile = path.join(userPoliciesDir, AUTO_SAVED_POLICY_FILENAME);
const policyFile = '/mock/user/.gemini/policies/auto-saved.toml';
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
(fs.mkdir as unknown as Mock).mockResolvedValue(undefined);
(fs.readFile as unknown as Mock).mockRejectedValue(
new Error('File not found'),
); // Simulate new file
const mockFileHandle = {
writeFile: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
};
(fs.open as unknown as Mock).mockResolvedValue(mockFileHandle);
(fs.rename as unknown as Mock).mockResolvedValue(undefined);
const toolName = 'test_tool';
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
toolName,
toolName: 'test_tool',
persist: true,
});
// Wait for async operations (microtasks)
await new Promise((resolve) => setTimeout(resolve, 0));
// Policy updater handles persistence asynchronously in a promise queue.
// We use advanceTimersByTimeAsync to yield to the microtask queue.
await vi.advanceTimersByTimeAsync(100);
expect(fs.mkdir).toHaveBeenCalledWith(userPoliciesDir, {
recursive: true,
});
const fileExists = memfs.existsSync(policyFile);
expect(fileExists).toBe(true);
expect(fs.open).toHaveBeenCalledWith(expect.stringMatching(/\.tmp$/), 'wx');
// Check written content
const expectedContent = expect.stringContaining(`toolName = "test_tool"`);
expect(mockFileHandle.writeFile).toHaveBeenCalledWith(
expectedContent,
'utf-8',
);
expect(fs.rename).toHaveBeenCalledWith(
expect.stringMatching(/\.tmp$/),
policyFile,
);
const content = memfs.readFileSync(policyFile, 'utf-8') as string;
expect(content).toContain('toolName = "test_tool"');
expect(content).toContain('decision = "allow"');
const expectedPriority = getAlwaysAllowPriorityFraction();
expect(content).toContain(`priority = ${expectedPriority}`);
});
it('should not persist policy when persist flag is false or undefined', async () => {
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const policyFile = '/mock/user/.gemini/policies/auto-saved.toml';
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
toolName: 'test_tool',
});
await new Promise((resolve) => setTimeout(resolve, 0));
await vi.advanceTimersByTimeAsync(100);
expect(fs.writeFile).not.toHaveBeenCalled();
expect(fs.rename).not.toHaveBeenCalled();
expect(memfs.existsSync(policyFile)).toBe(false);
});
it('should persist policy with commandPrefix when provided', async () => {
it('should append to existing policy file', async () => {
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const userPoliciesDir = '/mock/user/.gemini/policies';
const policyFile = path.join(userPoliciesDir, AUTO_SAVED_POLICY_FILENAME);
const policyFile = '/mock/user/.gemini/policies/auto-saved.toml';
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
(fs.mkdir as unknown as Mock).mockResolvedValue(undefined);
(fs.readFile as unknown as Mock).mockRejectedValue(
new Error('File not found'),
);
const mockFileHandle = {
writeFile: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
};
(fs.open as unknown as Mock).mockResolvedValue(mockFileHandle);
(fs.rename as unknown as Mock).mockResolvedValue(undefined);
const toolName = 'run_shell_command';
const commandPrefix = 'git status';
const existingContent =
'[[rule]]\ntoolName = "existing_tool"\ndecision = "allow"\n';
const dir = path.dirname(policyFile);
memfs.mkdirSync(dir, { recursive: true });
memfs.writeFileSync(policyFile, existingContent);
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
toolName,
toolName: 'new_tool',
persist: true,
commandPrefix,
});
await new Promise((resolve) => setTimeout(resolve, 0));
await vi.advanceTimersByTimeAsync(100);
// In-memory rule check (unchanged)
const rules = policyEngine.getRules();
const addedRule = rules.find((r) => r.toolName === toolName);
expect(addedRule).toBeDefined();
expect(addedRule?.priority).toBe(ALWAYS_ALLOW_PRIORITY);
expect(addedRule?.argsPattern).toEqual(
new RegExp(`"command":"git\\ status(?:[\\s"]|\\\\")`),
);
// Verify file written
expect(fs.open).toHaveBeenCalledWith(expect.stringMatching(/\.tmp$/), 'wx');
expect(mockFileHandle.writeFile).toHaveBeenCalledWith(
expect.stringContaining(`commandPrefix = "git status"`),
'utf-8',
);
const content = memfs.readFileSync(policyFile, 'utf-8') as string;
expect(content).toContain('toolName = "existing_tool"');
expect(content).toContain('toolName = "new_tool"');
});
it('should persist policy with mcpName and toolName when provided', async () => {
it('should handle toml with multiple rules correctly', async () => {
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const userPoliciesDir = '/mock/user/.gemini/policies';
const policyFile = path.join(userPoliciesDir, AUTO_SAVED_POLICY_FILENAME);
const policyFile = '/mock/user/.gemini/policies/auto-saved.toml';
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
(fs.mkdir as unknown as Mock).mockResolvedValue(undefined);
(fs.readFile as unknown as Mock).mockRejectedValue(
new Error('File not found'),
);
const mockFileHandle = {
writeFile: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
};
(fs.open as unknown as Mock).mockResolvedValue(mockFileHandle);
(fs.rename as unknown as Mock).mockResolvedValue(undefined);
const existingContent = `
[[rule]]
toolName = "tool1"
decision = "allow"
const mcpName = 'my-jira-server';
const simpleToolName = 'search';
const toolName = `${mcpName}__${simpleToolName}`;
[[rule]]
toolName = "tool2"
decision = "deny"
`;
const dir = path.dirname(policyFile);
memfs.mkdirSync(dir, { recursive: true });
memfs.writeFileSync(policyFile, existingContent);
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
toolName,
toolName: 'tool3',
persist: true,
mcpName,
});
await new Promise((resolve) => setTimeout(resolve, 0));
await vi.advanceTimersByTimeAsync(100);
// Verify file written
expect(fs.open).toHaveBeenCalledWith(expect.stringMatching(/\.tmp$/), 'wx');
const writeCall = mockFileHandle.writeFile.mock.calls[0];
const writtenContent = writeCall[0] as string;
expect(writtenContent).toContain(`mcpName = "${mcpName}"`);
expect(writtenContent).toContain(`toolName = "${simpleToolName}"`);
expect(writtenContent).toContain('priority = 200');
const content = memfs.readFileSync(policyFile, 'utf-8') as string;
expect(content).toContain('toolName = "tool1"');
expect(content).toContain('toolName = "tool2"');
expect(content).toContain('toolName = "tool3"');
});
it('should escape special characters in toolName and mcpName', async () => {
it('should include argsPattern if provided', async () => {
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const userPoliciesDir = '/mock/user/.gemini/policies';
const policyFile = path.join(userPoliciesDir, AUTO_SAVED_POLICY_FILENAME);
const policyFile = '/mock/user/.gemini/policies/auto-saved.toml';
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
(fs.mkdir as unknown as Mock).mockResolvedValue(undefined);
(fs.readFile as unknown as Mock).mockRejectedValue(
new Error('File not found'),
);
const mockFileHandle = {
writeFile: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
};
(fs.open as unknown as Mock).mockResolvedValue(mockFileHandle);
(fs.rename as unknown as Mock).mockResolvedValue(undefined);
const mcpName = 'my"jira"server';
const toolName = `my"jira"server__search"tool"`;
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
toolName,
toolName: 'test_tool',
persist: true,
mcpName,
argsPattern: '^foo.*$',
});
await new Promise((resolve) => setTimeout(resolve, 0));
await vi.advanceTimersByTimeAsync(100);
expect(fs.open).toHaveBeenCalledWith(expect.stringMatching(/\.tmp$/), 'wx');
const writeCall = mockFileHandle.writeFile.mock.calls[0];
const writtenContent = writeCall[0] as string;
const content = memfs.readFileSync(policyFile, 'utf-8') as string;
expect(content).toContain('argsPattern = "^foo.*$"');
});
// Verify escaping - should be valid TOML
it('should include mcpName if provided', async () => {
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const policyFile = '/mock/user/.gemini/policies/auto-saved.toml';
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
toolName: 'search"tool"',
persist: true,
mcpName: 'my"jira"server',
});
await vi.advanceTimersByTimeAsync(100);
const writtenContent = memfs.readFileSync(policyFile, 'utf-8') as string;
// Verify escaping - should be valid TOML and contain the values
// Note: @iarna/toml optimizes for shortest representation, so it may use single quotes 'foo"bar'
// instead of "foo\"bar\"" if there are no single quotes in the string.
try {
expect(writtenContent).toContain(`mcpName = "my\\"jira\\"server"`);
expect(writtenContent).toContain('mcpName = "my\\"jira\\"server"');
} catch {
expect(writtenContent).toContain(`mcpName = 'my"jira"server'`);
expect(writtenContent).toContain('mcpName = \'my"jira"server\'');
}
try {
expect(writtenContent).toContain(`toolName = "search\\"tool\\""`);
expect(writtenContent).toContain('toolName = "search\\"tool\\""');
} catch {
expect(writtenContent).toContain(`toolName = 'search"tool"'`);
expect(writtenContent).toContain('toolName = \'search"tool"\'');
}
});
it('should persist to workspace when persistScope is workspace', async () => {
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const workspacePoliciesDir = '/mock/project/.gemini/policies';
const policyFile = path.join(
workspacePoliciesDir,
AUTO_SAVED_POLICY_FILENAME,
);
vi.spyOn(mockStorage, 'getWorkspaceAutoSavedPolicyPath').mockReturnValue(
policyFile,
);
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
toolName: 'test_tool',
persist: true,
persistScope: 'workspace',
});
await vi.advanceTimersByTimeAsync(100);
expect(memfs.existsSync(policyFile)).toBe(true);
const content = memfs.readFileSync(policyFile, 'utf-8') as string;
expect(content).toContain('toolName = "test_tool"');
});
});
@@ -19,6 +19,7 @@ import {
type PolicyUpdateOptions,
} from '../tools/tools.js';
import * as shellUtils from '../utils/shell-utils.js';
import { escapeRegex } from './utils.js';
vi.mock('node:fs/promises');
vi.mock('../config/storage.js');
@@ -75,7 +76,9 @@ describe('createPolicyUpdater', () => {
expect.objectContaining({
toolName: 'run_shell_command',
priority: ALWAYS_ALLOW_PRIORITY,
argsPattern: new RegExp('"command":"echo(?:[\\s"]|\\\\")'),
argsPattern: new RegExp(
escapeRegex('"command":"echo') + '(?:[\\s"]|\\\\")',
),
}),
);
expect(policyEngine.addRule).toHaveBeenNthCalledWith(
@@ -83,7 +86,9 @@ describe('createPolicyUpdater', () => {
expect.objectContaining({
toolName: 'run_shell_command',
priority: ALWAYS_ALLOW_PRIORITY,
argsPattern: new RegExp('"command":"ls(?:[\\s"]|\\\\")'),
argsPattern: new RegExp(
escapeRegex('"command":"ls') + '(?:[\\s"]|\\\\")',
),
}),
);
});
@@ -103,7 +108,9 @@ describe('createPolicyUpdater', () => {
expect.objectContaining({
toolName: 'run_shell_command',
priority: ALWAYS_ALLOW_PRIORITY,
argsPattern: new RegExp('"command":"git(?:[\\s"]|\\\\")'),
argsPattern: new RegExp(
escapeRegex('"command":"git') + '(?:[\\s"]|\\\\")',
),
}),
);
});
+17 -16
View File
@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect } from 'vitest';
import { expect, describe, it } from 'vitest';
import { escapeRegex, buildArgsPatterns, isSafeRegExp } from './utils.js';
describe('policy/utils', () => {
@@ -43,20 +43,20 @@ describe('policy/utils', () => {
});
it('should return false for invalid regexes', () => {
expect(isSafeRegExp('[')).toBe(false);
expect(isSafeRegExp('([a-z)')).toBe(false);
expect(isSafeRegExp('*')).toBe(false);
});
it('should return false for extremely long regexes', () => {
expect(isSafeRegExp('a'.repeat(2049))).toBe(false);
it('should return false for long regexes', () => {
expect(isSafeRegExp('a'.repeat(3000))).toBe(false);
});
it('should return false for nested quantifiers (potential ReDoS)', () => {
it('should return false for nested quantifiers (ReDoS heuristic)', () => {
expect(isSafeRegExp('(a+)+')).toBe(false);
expect(isSafeRegExp('(a+)*')).toBe(false);
expect(isSafeRegExp('(a*)+')).toBe(false);
expect(isSafeRegExp('(a*)*')).toBe(false);
expect(isSafeRegExp('(a|b+)+')).toBe(false);
expect(isSafeRegExp('(a|b)*')).toBe(true);
expect(isSafeRegExp('(.*)*')).toBe(false);
expect(isSafeRegExp('([a-z]+)+')).toBe(false);
expect(isSafeRegExp('(.*)+')).toBe(false);
});
});
@@ -69,14 +69,14 @@ describe('policy/utils', () => {
it('should build pattern from a single commandPrefix', () => {
const result = buildArgsPatterns(undefined, 'ls', undefined);
expect(result).toEqual(['"command":"ls(?:[\\s"]|\\\\")']);
expect(result).toEqual(['\\"command\\":\\"ls(?:[\\s"]|\\\\")']);
});
it('should build patterns from an array of commandPrefixes', () => {
const result = buildArgsPatterns(undefined, ['ls', 'cd'], undefined);
const result = buildArgsPatterns(undefined, ['echo', 'ls'], undefined);
expect(result).toEqual([
'"command":"ls(?:[\\s"]|\\\\")',
'"command":"cd(?:[\\s"]|\\\\")',
'\\"command\\":\\"echo(?:[\\s"]|\\\\")',
'\\"command\\":\\"ls(?:[\\s"]|\\\\")',
]);
});
@@ -87,7 +87,7 @@ describe('policy/utils', () => {
it('should prioritize commandPrefix over commandRegex and argsPattern', () => {
const result = buildArgsPatterns('raw', 'prefix', 'regex');
expect(result).toEqual(['"command":"prefix(?:[\\s"]|\\\\")']);
expect(result).toEqual(['\\"command\\":\\"prefix(?:[\\s"]|\\\\")']);
});
it('should prioritize commandRegex over argsPattern if no commandPrefix', () => {
@@ -98,14 +98,15 @@ describe('policy/utils', () => {
it('should escape characters in commandPrefix', () => {
const result = buildArgsPatterns(undefined, 'git checkout -b', undefined);
expect(result).toEqual([
'"command":"git\\ checkout\\ \\-b(?:[\\s"]|\\\\")',
'\\"command\\":\\"git\\ checkout\\ \\-b(?:[\\s"]|\\\\")',
]);
});
it('should correctly escape quotes in commandPrefix', () => {
const result = buildArgsPatterns(undefined, 'git "fix"', undefined);
expect(result).toEqual([
'"command":"git\\ \\\\\\"fix\\\\\\"(?:[\\s"]|\\\\")',
// eslint-disable-next-line no-useless-escape
'\\\"command\\\":\\\"git\\ \\\\\\\"fix\\\\\\\"(?:[\\s\"]|\\\\\")',
]);
});
@@ -142,7 +143,7 @@ describe('policy/utils', () => {
const gitRegex = new RegExp(gitPatterns[0]!);
// git\status -> {"command":"git\\status"}
const gitAttack = '{"command":"git\\\\status"}';
expect(gitRegex.test(gitAttack)).toBe(false);
expect(gitAttack).not.toMatch(gitRegex);
});
});
});
+39 -6
View File
@@ -63,16 +63,22 @@ export function buildArgsPatterns(
? commandPrefix
: [commandPrefix];
// Expand command prefixes to multiple patterns.
// We append [\\s"] to ensure we match whole words only (e.g., "git" but not
// "github"). Since we match against JSON stringified args, the value is
// always followed by a space or a closing quote.
return prefixes.map((prefix) => {
const jsonPrefix = JSON.stringify(prefix).slice(1, -1);
// JSON.stringify safely encodes the prefix in quotes.
// We remove ONLY the trailing quote to match it as an open prefix string.
const encodedPrefix = JSON.stringify(prefix);
const openQuotePrefix = encodedPrefix.substring(
0,
encodedPrefix.length - 1,
);
// Escape the exact JSON literal segment we expect to see
const matchSegment = escapeRegex(`"command":${openQuotePrefix}`);
// We allow [\s], ["], or the specific sequence [\"] (for escaped quotes
// in JSON). We do NOT allow generic [\\], which would match "git\status"
// -> "gitstatus".
return `"command":"${escapeRegex(jsonPrefix)}(?:[\\s"]|\\\\")`;
return `${matchSegment}(?:[\\s"]|\\\\")`;
});
}
@@ -82,3 +88,30 @@ export function buildArgsPatterns(
return [argsPattern];
}
/**
* Builds a regex pattern to match a specific file path in tool arguments.
* This is used to narrow tool approvals for edit tools to specific files.
*
* @param filePath The relative path to the file.
* @returns A regex string that matches "file_path":"<path>" in a JSON string.
*/
export function buildFilePathArgsPattern(filePath: string): string {
// JSON.stringify safely encodes the path (handling quotes, backslashes, etc)
// and wraps it in double quotes. We simply prepend the key name and escape
// the entire sequence for Regex matching without any slicing.
const encodedPath = JSON.stringify(filePath);
return escapeRegex(`"file_path":${encodedPath}`);
}
/**
* Builds a regex pattern to match a specific "pattern" in tool arguments.
* This is used to narrow tool approvals for search tools like glob/grep to specific patterns.
*
* @param pattern The pattern to match.
* @returns A regex string that matches "pattern":"<pattern>" in a JSON string.
*/
export function buildPatternArgsPattern(pattern: string): string {
const encodedPattern = JSON.stringify(pattern);
return escapeRegex(`"pattern":${encodedPattern}`);
}
+99 -1
View File
@@ -16,8 +16,12 @@ import {
import { checkPolicy, updatePolicy, getPolicyDenialError } from './policy.js';
import type { Config } from '../config/config.js';
import type { MessageBus } from '../confirmation-bus/message-bus.js';
import { MessageBusType } from '../confirmation-bus/types.js';
import {
MessageBusType,
type SerializableConfirmationDetails,
} from '../confirmation-bus/types.js';
import { ApprovalMode, PolicyDecision } from '../policy/types.js';
import { escapeRegex } from '../policy/utils.js';
import {
ToolConfirmationOutcome,
type AnyDeclarativeTool,
@@ -219,6 +223,8 @@ describe('policy.ts', () => {
it('should handle standard policy updates with persistence', async () => {
const mockConfig = {
isTrustedFolder: vi.fn().mockReturnValue(false),
getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined),
setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>;
@@ -453,6 +459,8 @@ describe('policy.ts', () => {
it('should handle MCP ProceedAlwaysAndSave (persist: true)', async () => {
const mockConfig = {
isTrustedFolder: vi.fn().mockReturnValue(false),
getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined),
setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>;
@@ -487,6 +495,96 @@ describe('policy.ts', () => {
}),
);
});
it('should determine persistScope: workspace in trusted folders', async () => {
const mockConfig = {
isTrustedFolder: vi.fn().mockReturnValue(true),
getWorkspacePoliciesDir: vi
.fn()
.mockReturnValue('/mock/project/policies'),
setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>;
const mockMessageBus = {
publish: vi.fn(),
} as unknown as Mocked<MessageBus>;
const tool = { name: 'test-tool' } as AnyDeclarativeTool;
await updatePolicy(
tool,
ToolConfirmationOutcome.ProceedAlwaysAndSave,
undefined,
{ config: mockConfig, messageBus: mockMessageBus },
);
expect(mockMessageBus.publish).toHaveBeenCalledWith(
expect.objectContaining({
persistScope: 'workspace',
}),
);
});
it('should determine persistScope: user in untrusted folders', async () => {
const mockConfig = {
isTrustedFolder: vi.fn().mockReturnValue(false),
getWorkspacePoliciesDir: vi
.fn()
.mockReturnValue('/mock/project/policies'),
setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>;
const mockMessageBus = {
publish: vi.fn(),
} as unknown as Mocked<MessageBus>;
const tool = { name: 'test-tool' } as AnyDeclarativeTool;
await updatePolicy(
tool,
ToolConfirmationOutcome.ProceedAlwaysAndSave,
undefined,
{ config: mockConfig, messageBus: mockMessageBus },
);
expect(mockMessageBus.publish).toHaveBeenCalledWith(
expect.objectContaining({
persistScope: 'user',
}),
);
});
it('should narrow edit tools with argsPattern', async () => {
const mockConfig = {
isTrustedFolder: vi.fn().mockReturnValue(false),
getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined),
getTargetDir: vi.fn().mockReturnValue('/mock/dir'),
setApprovalMode: vi.fn(),
} as unknown as Mocked<Config>;
const mockMessageBus = {
publish: vi.fn(),
} as unknown as Mocked<MessageBus>;
const tool = { name: 'write_file' } as AnyDeclarativeTool;
const details: SerializableConfirmationDetails = {
type: 'edit',
title: 'Edit',
filePath: 'src/foo.ts',
fileName: 'foo.ts',
fileDiff: '--- foo.ts\n+++ foo.ts\n@@ -1 +1 @@\n-old\n+new',
originalContent: 'old',
newContent: 'new',
};
await updatePolicy(
tool,
ToolConfirmationOutcome.ProceedAlwaysAndSave,
details,
{ config: mockConfig, messageBus: mockMessageBus },
);
expect(mockMessageBus.publish).toHaveBeenCalledWith(
expect.objectContaining({
toolName: 'write_file',
argsPattern: escapeRegex('"file_path":"src/foo.ts"'),
}),
);
});
});
describe('getPolicyDenialError', () => {
+40 -3
View File
@@ -20,8 +20,11 @@ import {
import {
ToolConfirmationOutcome,
type AnyDeclarativeTool,
type AnyToolInvocation,
type PolicyUpdateOptions,
} from '../tools/tools.js';
import { buildFilePathArgsPattern } from '../policy/utils.js';
import { makeRelative } from '../utils/paths.js';
import { DiscoveredMCPTool } from '../tools/mcp-tool.js';
import { EDIT_TOOL_NAMES } from '../tools/tool-names.js';
import type { ValidatingToolCall } from './types.js';
@@ -94,7 +97,11 @@ export async function updatePolicy(
tool: AnyDeclarativeTool,
outcome: ToolConfirmationOutcome,
confirmationDetails: SerializableConfirmationDetails | undefined,
deps: { config: Config; messageBus: MessageBus },
deps: {
config: Config;
messageBus: MessageBus;
toolInvocation?: AnyToolInvocation;
},
): Promise<void> {
// Mode Transitions (AUTO_EDIT)
if (isAutoEditTransition(tool, outcome)) {
@@ -102,6 +109,20 @@ export async function updatePolicy(
return;
}
// Determine persist scope if we are persisting.
let persistScope: 'workspace' | 'user' | undefined;
if (outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave) {
// If folder is trusted and workspace policies are enabled, we prefer workspace scope.
if (
deps.config.isTrustedFolder() &&
deps.config.getWorkspacePoliciesDir() !== undefined
) {
persistScope = 'workspace';
} else {
persistScope = 'user';
}
}
// Specialized Tools (MCP)
if (confirmationDetails?.type === 'mcp') {
await handleMcpPolicyUpdate(
@@ -109,6 +130,7 @@ export async function updatePolicy(
outcome,
confirmationDetails,
deps.messageBus,
persistScope,
);
return;
}
@@ -119,6 +141,9 @@ export async function updatePolicy(
outcome,
confirmationDetails,
deps.messageBus,
persistScope,
deps.toolInvocation,
deps.config,
);
}
@@ -148,21 +173,31 @@ async function handleStandardPolicyUpdate(
outcome: ToolConfirmationOutcome,
confirmationDetails: SerializableConfirmationDetails | undefined,
messageBus: MessageBus,
persistScope?: 'workspace' | 'user',
toolInvocation?: AnyToolInvocation,
config?: Config,
): Promise<void> {
if (
outcome === ToolConfirmationOutcome.ProceedAlways ||
outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave
) {
const options: PolicyUpdateOptions = {};
const options: PolicyUpdateOptions =
toolInvocation?.getPolicyUpdateOptions?.(outcome) || {};
if (confirmationDetails?.type === 'exec') {
if (!options.commandPrefix && confirmationDetails?.type === 'exec') {
options.commandPrefix = confirmationDetails.rootCommands;
} else if (!options.argsPattern && confirmationDetails?.type === 'edit') {
const filePath = config
? makeRelative(confirmationDetails.filePath, config.getTargetDir())
: confirmationDetails.filePath;
options.argsPattern = buildFilePathArgsPattern(filePath);
}
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
toolName: tool.name,
persist: outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave,
persistScope,
...options,
});
}
@@ -180,6 +215,7 @@ async function handleMcpPolicyUpdate(
{ type: 'mcp' }
>,
messageBus: MessageBus,
persistScope?: 'workspace' | 'user',
): Promise<void> {
const isMcpAlways =
outcome === ToolConfirmationOutcome.ProceedAlways ||
@@ -204,5 +240,6 @@ async function handleMcpPolicyUpdate(
toolName,
mcpName: confirmationDetails.serverName,
persist,
persistScope,
});
}
+1
View File
@@ -608,6 +608,7 @@ export class Scheduler {
await updatePolicy(toolCall.tool, outcome, lastDetails, {
config: this.config,
messageBus: this.messageBus,
toolInvocation: toolCall.invocation,
});
}
@@ -211,6 +211,87 @@ describe('ToolExecutor', () => {
});
});
it('should return cancelled result when executeToolWithHooks rejects with AbortError', async () => {
const mockTool = new MockTool({
name: 'webSearchTool',
description: 'Mock web search',
});
const invocation = mockTool.build({});
const abortErr = new Error('The user aborted a request.');
abortErr.name = 'AbortError';
vi.mocked(coreToolHookTriggers.executeToolWithHooks).mockRejectedValue(
abortErr,
);
const scheduledCall: ScheduledToolCall = {
status: CoreToolCallStatus.Scheduled,
request: {
callId: 'call-abort',
name: 'webSearchTool',
args: {},
isClientInitiated: false,
prompt_id: 'prompt-abort',
},
tool: mockTool,
invocation: invocation as unknown as AnyToolInvocation,
startTime: Date.now(),
};
const result = await executor.execute({
call: scheduledCall,
signal: new AbortController().signal,
onUpdateToolCall: vi.fn(),
});
expect(result.status).toBe(CoreToolCallStatus.Cancelled);
if (result.status === CoreToolCallStatus.Cancelled) {
const response = result.response.responseParts[0]?.functionResponse
?.response as Record<string, unknown>;
expect(response['error']).toContain('Operation cancelled.');
}
});
it('should return cancelled result when executeToolWithHooks rejects with "Operation cancelled by user" message', async () => {
const mockTool = new MockTool({
name: 'someTool',
description: 'Mock',
});
const invocation = mockTool.build({});
const cancelErr = new Error('Operation cancelled by user');
vi.mocked(coreToolHookTriggers.executeToolWithHooks).mockRejectedValue(
cancelErr,
);
const scheduledCall: ScheduledToolCall = {
status: CoreToolCallStatus.Scheduled,
request: {
callId: 'call-cancel-msg',
name: 'someTool',
args: {},
isClientInitiated: false,
prompt_id: 'prompt-cancel-msg',
},
tool: mockTool,
invocation: invocation as unknown as AnyToolInvocation,
startTime: Date.now(),
};
const result = await executor.execute({
call: scheduledCall,
signal: new AbortController().signal,
onUpdateToolCall: vi.fn(),
});
expect(result.status).toBe(CoreToolCallStatus.Cancelled);
if (result.status === CoreToolCallStatus.Cancelled) {
const response = result.response.responseParts[0]?.functionResponse
?.response as Record<string, unknown>;
expect(response['error']).toContain('User cancelled tool execution.');
}
});
it('should return cancelled result when signal is aborted', async () => {
const mockTool = new MockTool({
name: 'slowTool',
+8 -5
View File
@@ -16,6 +16,7 @@ import {
type AgentLoopContext,
type ToolLiveOutput,
} from '../index.js';
import { isAbortError } from '../utils/errors.js';
import { SHELL_TOOL_NAME } from '../tools/tool-names.js';
import { ShellToolInvocation } from '../tools/shell.js';
import { DiscoveredMCPTool } from '../tools/mcp-tool.js';
@@ -159,15 +160,17 @@ export class ToolExecutor {
}
} catch (executionError: unknown) {
spanMetadata.error = executionError;
const isAbortError =
executionError instanceof Error &&
(executionError.name === 'AbortError' ||
const abortedByError =
isAbortError(executionError) ||
(executionError instanceof Error &&
executionError.message.includes('Operation cancelled by user'));
if (signal.aborted || isAbortError) {
if (signal.aborted || abortedByError) {
completedToolCall = await this.createCancelledResult(
call,
'User cancelled tool execution.',
isAbortError(executionError)
? 'Operation cancelled.'
: 'User cancelled tool execution.',
);
} else {
const error =
+58 -33
View File
@@ -20,11 +20,14 @@ import {
type ToolLocation,
type ToolResult,
type ToolResultDisplay,
type PolicyUpdateOptions,
} from './tools.js';
import { buildFilePathArgsPattern } from '../policy/utils.js';
import type { MessageBus } from '../confirmation-bus/message-bus.js';
import { ToolErrorType } from './tool-error.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { isNodeError } from '../utils/errors.js';
import { correctPath } from '../utils/pathCorrector.js';
import type { Config } from '../config/config.js';
import { ApprovalMode } from '../policy/types.js';
import { CoreToolCallStatus } from '../scheduler/types.js';
@@ -44,7 +47,6 @@ import {
logEditCorrectionEvent,
} from '../telemetry/loggers.js';
import { correctPath } from '../utils/pathCorrector.js';
import {
EDIT_TOOL_NAME,
READ_FILE_TOOL_NAME,
@@ -442,6 +444,8 @@ class EditToolInvocation
extends BaseToolInvocation<EditToolParams, ToolResult>
implements ToolInvocation<EditToolParams, ToolResult>
{
private readonly resolvedPath: string;
constructor(
private readonly config: Config,
params: EditToolParams,
@@ -450,10 +454,31 @@ class EditToolInvocation
displayName?: string,
) {
super(params, messageBus, toolName, displayName);
if (!path.isAbsolute(this.params.file_path)) {
const result = correctPath(this.params.file_path, this.config);
if (result.success) {
this.resolvedPath = result.correctedPath;
} else {
this.resolvedPath = path.resolve(
this.config.getTargetDir(),
this.params.file_path,
);
}
} else {
this.resolvedPath = this.params.file_path;
}
}
override toolLocations(): ToolLocation[] {
return [{ path: this.params.file_path }];
return [{ path: this.resolvedPath }];
}
override getPolicyUpdateOptions(
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
return {
argsPattern: buildFilePathArgsPattern(this.params.file_path),
};
}
private async attemptSelfCorrection(
@@ -471,7 +496,7 @@ class EditToolInvocation
const initialContentHash = hashContent(currentContent);
const onDiskContent = await this.config
.getFileSystemService()
.readTextFile(params.file_path);
.readTextFile(this.resolvedPath);
const onDiskContentHash = hashContent(onDiskContent.replace(/\r\n/g, '\n'));
if (initialContentHash !== onDiskContentHash) {
@@ -582,7 +607,7 @@ class EditToolInvocation
try {
currentContent = await this.config
.getFileSystemService()
.readTextFile(params.file_path);
.readTextFile(this.resolvedPath);
originalLineEnding = detectLineEnding(currentContent);
currentContent = currentContent.replace(/\r\n/g, '\n');
fileExists = true;
@@ -615,7 +640,7 @@ class EditToolInvocation
isNewFile: false,
error: {
display: `File not found. Cannot apply edit. Use an empty old_string to create a new file.`,
raw: `File not found: ${params.file_path}`,
raw: `File not found: ${this.resolvedPath}`,
type: ToolErrorType.FILE_NOT_FOUND,
},
originalLineEnding,
@@ -630,7 +655,7 @@ class EditToolInvocation
isNewFile: false,
error: {
display: `Failed to read content of file.`,
raw: `Failed to read content of existing file: ${params.file_path}`,
raw: `Failed to read content of existing file: ${this.resolvedPath}`,
type: ToolErrorType.READ_CONTENT_FAILURE,
},
originalLineEnding,
@@ -645,7 +670,7 @@ class EditToolInvocation
isNewFile: false,
error: {
display: `Failed to edit. Attempted to create a file that already exists.`,
raw: `File already exists, cannot create: ${params.file_path}`,
raw: `File already exists, cannot create: ${this.resolvedPath}`,
type: ToolErrorType.ATTEMPT_TO_CREATE_EXISTING_FILE,
},
originalLineEnding,
@@ -727,7 +752,7 @@ class EditToolInvocation
return false;
}
const fileName = path.basename(this.params.file_path);
const fileName = path.basename(this.resolvedPath);
const fileDiff = Diff.createPatch(
fileName,
editData.currentContent ?? '',
@@ -739,14 +764,14 @@ class EditToolInvocation
const ideClient = await IdeClient.getInstance();
const ideConfirmation =
this.config.getIdeMode() && ideClient.isDiffingEnabled()
? ideClient.openDiff(this.params.file_path, editData.newContent)
? ideClient.openDiff(this.resolvedPath, editData.newContent)
: undefined;
const confirmationDetails: ToolEditConfirmationDetails = {
type: 'edit',
title: `Confirm Edit: ${shortenPath(makeRelative(this.params.file_path, this.config.getTargetDir()))}`,
title: `Confirm Edit: ${shortenPath(makeRelative(this.resolvedPath, this.config.getTargetDir()))}`,
fileName,
filePath: this.params.file_path,
filePath: this.resolvedPath,
fileDiff,
originalContent: editData.currentContent,
newContent: editData.newContent,
@@ -771,7 +796,7 @@ class EditToolInvocation
getDescription(): string {
const relativePath = makeRelative(
this.params.file_path,
this.resolvedPath,
this.config.getTargetDir(),
);
if (this.params.old_string === '') {
@@ -797,11 +822,7 @@ class EditToolInvocation
* @returns Result of the edit operation
*/
async execute(signal: AbortSignal): Promise<ToolResult> {
const resolvedPath = path.resolve(
this.config.getTargetDir(),
this.params.file_path,
);
const validationError = this.config.validatePathAccess(resolvedPath);
const validationError = this.config.validatePathAccess(this.resolvedPath);
if (validationError) {
return {
llmContent: validationError,
@@ -843,7 +864,7 @@ class EditToolInvocation
}
try {
await this.ensureParentDirectoriesExistAsync(this.params.file_path);
await this.ensureParentDirectoriesExistAsync(this.resolvedPath);
let finalContent = editData.newContent;
// Restore original line endings if they were CRLF, or use OS default for new files
@@ -856,15 +877,15 @@ class EditToolInvocation
}
await this.config
.getFileSystemService()
.writeTextFile(this.params.file_path, finalContent);
.writeTextFile(this.resolvedPath, finalContent);
let displayResult: ToolResultDisplay;
if (editData.isNewFile) {
displayResult = `Created ${shortenPath(makeRelative(this.params.file_path, this.config.getTargetDir()))}`;
displayResult = `Created ${shortenPath(makeRelative(this.resolvedPath, this.config.getTargetDir()))}`;
} else {
// Generate diff for display, even though core logic doesn't technically need it
// The CLI wrapper will use this part of the ToolResult
const fileName = path.basename(this.params.file_path);
const fileName = path.basename(this.resolvedPath);
const fileDiff = Diff.createPatch(
fileName,
editData.currentContent ?? '', // Should not be null here if not isNewFile
@@ -883,7 +904,7 @@ class EditToolInvocation
displayResult = {
fileDiff,
fileName,
filePath: this.params.file_path,
filePath: this.resolvedPath,
originalContent: editData.currentContent,
newContent: editData.newContent,
diffStat,
@@ -893,8 +914,8 @@ class EditToolInvocation
const llmSuccessMessageParts = [
editData.isNewFile
? `Created new file: ${this.params.file_path} with provided content.`
: `Successfully modified file: ${this.params.file_path} (${editData.occurrences} replacements).`,
? `Created new file: ${this.resolvedPath} with provided content.`
: `Successfully modified file: ${this.resolvedPath} (${editData.occurrences} replacements).`,
];
// Return a diff of the file before and after the write so that the agent
@@ -985,16 +1006,20 @@ export class EditTool
return "The 'file_path' parameter must be non-empty.";
}
let filePath = params.file_path;
if (!path.isAbsolute(filePath)) {
// Attempt to auto-correct to an absolute path
const result = correctPath(filePath, this.config);
if (!result.success) {
return result.error;
let resolvedPath: string;
if (!path.isAbsolute(params.file_path)) {
const result = correctPath(params.file_path, this.config);
if (result.success) {
resolvedPath = result.correctedPath;
} else {
resolvedPath = path.resolve(
this.config.getTargetDir(),
params.file_path,
);
}
filePath = result.correctedPath;
} else {
resolvedPath = params.file_path;
}
params.file_path = filePath;
const newPlaceholders = detectOmissionPlaceholders(params.new_string);
if (newPlaceholders.length > 0) {
@@ -1009,7 +1034,7 @@ export class EditTool
}
}
return this.config.validatePathAccess(params.file_path);
return this.config.validatePathAccess(resolvedPath);
}
protected createInvocation(
+11
View File
@@ -14,12 +14,15 @@ import {
Kind,
type ToolInvocation,
type ToolResult,
type PolicyUpdateOptions,
type ToolConfirmationOutcome,
} from './tools.js';
import { shortenPath, makeRelative } from '../utils/paths.js';
import { type Config } from '../config/config.js';
import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js';
import { ToolErrorType } from './tool-error.js';
import { GLOB_TOOL_NAME, GLOB_DISPLAY_NAME } from './tool-names.js';
import { buildPatternArgsPattern } from '../policy/utils.js';
import { getErrorMessage } from '../utils/errors.js';
import { debugLogger } from '../utils/debugLogger.js';
import { GLOB_DEFINITION } from './definitions/coreTools.js';
@@ -118,6 +121,14 @@ class GlobToolInvocation extends BaseToolInvocation<
return description;
}
override getPolicyUpdateOptions(
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
return {
argsPattern: buildPatternArgsPattern(this.params.pattern),
};
}
async execute(signal: AbortSignal): Promise<ToolResult> {
try {
const workspaceContext = this.config.getWorkspaceContext();
+11
View File
@@ -21,6 +21,8 @@ import {
Kind,
type ToolInvocation,
type ToolResult,
type PolicyUpdateOptions,
type ToolConfirmationOutcome,
} from './tools.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { getErrorMessage, isNodeError } from '../utils/errors.js';
@@ -29,6 +31,7 @@ import type { Config } from '../config/config.js';
import type { FileExclusions } from '../utils/ignorePatterns.js';
import { ToolErrorType } from './tool-error.js';
import { GREP_TOOL_NAME } from './tool-names.js';
import { buildPatternArgsPattern } from '../policy/utils.js';
import { debugLogger } from '../utils/debugLogger.js';
import { GREP_DEFINITION } from './definitions/coreTools.js';
import { resolveToolDeclaration } from './definitions/resolver.js';
@@ -285,6 +288,14 @@ class GrepToolInvocation extends BaseToolInvocation<
}
}
override getPolicyUpdateOptions(
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
return {
argsPattern: buildPatternArgsPattern(this.params.pattern),
};
}
/**
* Checks if a command is available in the system's PATH.
* @param {string} command The command name (e.g., 'git', 'grep').
+11
View File
@@ -13,12 +13,15 @@ import {
Kind,
type ToolInvocation,
type ToolResult,
type PolicyUpdateOptions,
type ToolConfirmationOutcome,
} from './tools.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import type { Config } from '../config/config.js';
import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js';
import { ToolErrorType } from './tool-error.js';
import { LS_TOOL_NAME } from './tool-names.js';
import { buildFilePathArgsPattern } from '../policy/utils.js';
import { debugLogger } from '../utils/debugLogger.js';
import { LS_DEFINITION } from './definitions/coreTools.js';
import { resolveToolDeclaration } from './definitions/resolver.js';
@@ -123,6 +126,14 @@ class LSToolInvocation extends BaseToolInvocation<LSToolParams, ToolResult> {
return shortenPath(relativePath);
}
override getPolicyUpdateOptions(
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
return {
argsPattern: buildFilePathArgsPattern(this.params.dir_path),
};
}
// Helper for consistent error formatting
private errorResult(
llmContent: string,
+1 -1
View File
@@ -184,7 +184,7 @@ export class DiscoveredMCPToolInvocation extends BaseToolInvocation<
);
}
protected override getPolicyUpdateOptions(
override getPolicyUpdateOptions(
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
return { mcpName: this.serverName };
+11
View File
@@ -14,8 +14,11 @@ import {
type ToolInvocation,
type ToolLocation,
type ToolResult,
type PolicyUpdateOptions,
type ToolConfirmationOutcome,
} from './tools.js';
import { ToolErrorType } from './tool-error.js';
import { buildFilePathArgsPattern } from '../policy/utils.js';
import type { PartUnion } from '@google/genai';
import {
@@ -88,6 +91,14 @@ class ReadFileToolInvocation extends BaseToolInvocation<
];
}
override getPolicyUpdateOptions(
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
return {
argsPattern: buildFilePathArgsPattern(this.params.file_path),
};
}
async execute(): Promise<ToolResult> {
const validationError = this.config.validatePathAccess(
this.resolvedPath,
@@ -11,11 +11,14 @@ import {
Kind,
type ToolInvocation,
type ToolResult,
type PolicyUpdateOptions,
type ToolConfirmationOutcome,
} from './tools.js';
import { getErrorMessage } from '../utils/errors.js';
import * as fsPromises from 'node:fs/promises';
import * as path from 'node:path';
import { glob, escape } from 'glob';
import { buildPatternArgsPattern } from '../policy/utils.js';
import {
detectFileType,
processSingleFileContent,
@@ -155,6 +158,16 @@ ${finalExclusionPatternsForDescription
)}".`;
}
override getPolicyUpdateOptions(
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
// We join the include patterns to match the JSON stringified arguments.
// buildPatternArgsPattern handles JSON stringification.
return {
argsPattern: buildPatternArgsPattern(JSON.stringify(this.params.include)),
};
}
async execute(signal: AbortSignal): Promise<ToolResult> {
const { include, exclude = [], useDefaultExcludes = true } = this.params;
+1 -1
View File
@@ -90,7 +90,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
return description;
}
protected override getPolicyUpdateOptions(
override getPolicyUpdateOptions(
outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
if (
+23 -6
View File
@@ -154,12 +154,22 @@ export const LS_TOOL_NAME_LEGACY = 'list_directory'; // Just to be safe if anyth
export const EDIT_TOOL_NAMES = new Set([EDIT_TOOL_NAME, WRITE_FILE_TOOL_NAME]);
// Tool Display Names
export const WRITE_FILE_DISPLAY_NAME = 'WriteFile';
export const EDIT_DISPLAY_NAME = 'Edit';
export const ASK_USER_DISPLAY_NAME = 'Ask User';
export const READ_FILE_DISPLAY_NAME = 'ReadFile';
export const GLOB_DISPLAY_NAME = 'FindFiles';
/**
* Tools that can access local files or remote resources and should be
* treated with extra caution when updating policies.
*/
export const SENSITIVE_TOOLS = new Set([
GLOB_TOOL_NAME,
GREP_TOOL_NAME,
READ_MANY_FILES_TOOL_NAME,
WEB_FETCH_TOOL_NAME,
READ_FILE_TOOL_NAME,
LS_TOOL_NAME,
WRITE_FILE_TOOL_NAME,
EDIT_TOOL_NAME,
SHELL_TOOL_NAME,
]);
export const TRACKER_CREATE_TASK_TOOL_NAME = 'tracker_create_task';
export const TRACKER_UPDATE_TASK_TOOL_NAME = 'tracker_update_task';
export const TRACKER_GET_TASK_TOOL_NAME = 'tracker_get_task';
@@ -167,6 +177,13 @@ export const TRACKER_LIST_TASKS_TOOL_NAME = 'tracker_list_tasks';
export const TRACKER_ADD_DEPENDENCY_TOOL_NAME = 'tracker_add_dependency';
export const TRACKER_VISUALIZE_TOOL_NAME = 'tracker_visualize';
// Tool Display Names
export const WRITE_FILE_DISPLAY_NAME = 'WriteFile';
export const EDIT_DISPLAY_NAME = 'Edit';
export const ASK_USER_DISPLAY_NAME = 'Ask User';
export const READ_FILE_DISPLAY_NAME = 'ReadFile';
export const GLOB_DISPLAY_NAME = 'FindFiles';
/**
* Mapping of legacy tool names to their current names.
* This ensures backward compatibility for user-defined policies, skills, and hooks.
+10 -1
View File
@@ -68,12 +68,21 @@ export interface ToolInvocation<
updateOutput?: (output: ToolLiveOutput) => void,
shellExecutionConfig?: ShellExecutionConfig,
): Promise<TResult>;
/**
* Returns tool-specific options for policy updates.
* This is used by the scheduler to narrow policy rules when a tool is approved.
*/
getPolicyUpdateOptions?(
outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined;
}
/**
* Options for policy updates that can be customized by tool invocations.
*/
export interface PolicyUpdateOptions {
argsPattern?: string;
commandPrefix?: string | string[];
mcpName?: string;
}
@@ -130,7 +139,7 @@ export abstract class BaseToolInvocation<
* Subclasses can override this to provide additional options like
* commandPrefix (for shell) or mcpName (for MCP tools).
*/
protected getPolicyUpdateOptions(
getPolicyUpdateOptions(
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
return undefined;
+18
View File
@@ -12,7 +12,9 @@ import {
type ToolInvocation,
type ToolResult,
type ToolConfirmationOutcome,
type PolicyUpdateOptions,
} from './tools.js';
import { buildPatternArgsPattern } from '../policy/utils.js';
import type { MessageBus } from '../confirmation-bus/message-bus.js';
import { ToolErrorType } from './tool-error.js';
import { getErrorMessage } from '../utils/errors.js';
@@ -291,6 +293,22 @@ ${textContent}
return `Processing URLs and instructions from prompt: "${displayPrompt}"`;
}
override getPolicyUpdateOptions(
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
if (this.params.url) {
return {
argsPattern: buildPatternArgsPattern(this.params.url),
};
}
if (this.params.prompt) {
return {
argsPattern: buildPatternArgsPattern(this.params.prompt),
};
}
return undefined;
}
protected override async getConfirmationDetails(
_abortSignal: AbortSignal,
): Promise<ToolCallConfirmationDetails | false> {
+7 -1
View File
@@ -16,7 +16,7 @@ import {
} from './tools.js';
import { ToolErrorType } from './tool-error.js';
import { getErrorMessage } from '../utils/errors.js';
import { getErrorMessage, isAbortError } from '../utils/errors.js';
import { type Config } from '../config/config.js';
import { getResponseText } from '../utils/partUtils.js';
import { debugLogger } from '../utils/debugLogger.js';
@@ -175,6 +175,12 @@ class WebSearchToolInvocation extends BaseToolInvocation<
sources,
};
} catch (error: unknown) {
if (isAbortError(error)) {
return {
llmContent: 'Web search was cancelled.',
returnDisplay: 'Search cancelled.',
};
}
const errorMessage = `Error during web search for query "${
this.params.query
}": ${getErrorMessage(error)}`;
+10
View File
@@ -24,7 +24,9 @@ import {
type ToolLocation,
type ToolResult,
type ToolConfirmationOutcome,
type PolicyUpdateOptions,
} from './tools.js';
import { buildFilePathArgsPattern } from '../policy/utils.js';
import { ToolErrorType } from './tool-error.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { getErrorMessage, isNodeError } from '../utils/errors.js';
@@ -164,6 +166,14 @@ class WriteFileToolInvocation extends BaseToolInvocation<
return [{ path: this.resolvedPath }];
}
override getPolicyUpdateOptions(
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
return {
argsPattern: buildFilePathArgsPattern(this.params.file_path),
};
}
override getDescription(): string {
const relativePath = makeRelative(
this.resolvedPath,
+7
View File
@@ -1461,6 +1461,13 @@
"default": false,
"type": "boolean"
},
"autoAddToPolicyByDefault": {
"title": "Auto-add to Policy by Default",
"description": "When enabled, the \"Allow for all future sessions\" option becomes the default choice for low-risk tools in trusted workspaces.",
"markdownDescription": "When enabled, the \"Allow for all future sessions\" option becomes the default choice for low-risk tools in trusted workspaces.\n\n- Category: `Security`\n- Requires restart: `no`\n- Default: `false`",
"default": false,
"type": "boolean"
},
"blockGitExtensions": {
"title": "Blocks extensions from Git",
"description": "Blocks installing and loading extensions from Git.",
+11 -8
View File
@@ -128,7 +128,7 @@ async function main() {
let commentBody;
if (success) {
commentBody = `✅ **Patch Release Complete!**
commentBody = `✅ **[Step 4/4] Patch Release Complete!**
**📦 Release Details:**
- **Version**: [\`${releaseVersion}\`](https://github.com/${repo.owner}/${repo.repo}/releases/tag/${releaseTag})
@@ -144,9 +144,10 @@ async function main() {
**🔗 Links:**
- [GitHub Release](https://github.com/${repo.owner}/${repo.repo}/releases/tag/${releaseTag})
- [Workflow Run](https://github.com/${repo.owner}/${repo.repo}/actions/runs/${runId})`;
- [This release workflow run](https://github.com/${repo.owner}/${repo.repo}/actions/runs/${runId})
- [Workflow History](https://github.com/${repo.owner}/${repo.repo}/actions/workflows/release-patch-3-release.yml)`;
} else if (raceConditionFailure) {
commentBody = `⚠️ **Patch Release Cancelled - Concurrent Release Detected**
commentBody = `⚠️ **[Step 4/4] Patch Release Cancelled - Concurrent Release Detected**
**🚦 What Happened:**
Another patch release completed while this one was in progress, causing a version conflict.
@@ -163,7 +164,7 @@ Another patch release completed while this one was in progress, causing a versio
- **Next patch should be**: \`${currentReleaseVersion}\`
- **New release tag**: \`${currentReleaseTag || 'unknown'}\``
: `
- **Status**: Version information updated since this release started`
- **Status**: Version information updated since this release was triggered`
}
**🔄 Next Steps:**
@@ -175,9 +176,10 @@ Another patch release completed while this one was in progress, causing a versio
Multiple patch releases can't run simultaneously. When they do, the second one is automatically cancelled to maintain version consistency.
**🔗 Details:**
- [View cancelled workflow run](https://github.com/${repo.owner}/${repo.repo}/actions/runs/${runId})`;
- [This release workflow run](https://github.com/${repo.owner}/${repo.repo}/actions/runs/${runId})
- [Workflow History](https://github.com/${repo.owner}/${repo.repo}/actions/workflows/release-patch-3-release.yml)`;
} else {
commentBody = `❌ **Patch Release Failed!**
commentBody = `❌ **[Step 4/4] Patch Release Failed!**
**📋 Details:**
- **Version**: \`${releaseVersion || 'Unknown'}\`
@@ -190,8 +192,9 @@ Multiple patch releases can't run simultaneously. When they do, the second one i
3. You may need to retry the patch once the issue is resolved
**🔗 Troubleshooting:**
- [View workflow run](https://github.com/${repo.owner}/${repo.repo}/actions/runs/${runId})
- [View workflow logs](https://github.com/${repo.owner}/${repo.repo}/actions/runs/${runId})`;
- [This release workflow run](https://github.com/${repo.owner}/${repo.repo}/actions/runs/${runId})
- [View workflow logs](https://github.com/${repo.owner}/${repo.repo}/actions/runs/${runId})
- [Workflow History](https://github.com/${repo.owner}/${repo.repo}/actions/workflows/release-patch-3-release.yml)`;
}
if (testMode) {
+17 -13
View File
@@ -145,7 +145,7 @@ async function main() {
manualCommands = manualCommandsMatch[1].trim();
}
commentBody = `🔒 **GitHub App Permission Issue**
commentBody = `🔒 **[Step 2/4] GitHub App Permission Issue**
The patch creation failed due to insufficient GitHub App permissions for creating workflow files.
@@ -169,7 +169,7 @@ After running these commands, you can re-run the patch workflow.`
const prMatch = logContent.match(/Found existing PR #(\d+): (.*)/);
if (prMatch) {
const [, prNumber, prUrl] = prMatch;
commentBody = `️ **Patch PR already exists!**
commentBody = `️ **[Step 2/4] Patch PR already exists!**
A patch PR for this change already exists: [#${prNumber}](${prUrl}).
@@ -185,7 +185,7 @@ A patch PR for this change already exists: [#${prNumber}](${prUrl}).
const branchMatch = logContent.match(/Hotfix branch (.*) already exists/);
if (branchMatch) {
const [, branch] = branchMatch;
commentBody = `️ **Patch branch exists but no PR found!**
commentBody = `️ **[Step 2/4] Patch branch exists but no PR found!**
A patch branch [\`${branch}\`](https://github.com/${repository}/tree/${branch}) exists but has no open PR.
@@ -213,7 +213,7 @@ A patch branch [\`${branch}\`](https://github.com/${repository}/tree/${branch})
logContent.includes('Cherry-pick has conflicts') ||
logContent.includes('[CONFLICTS]');
commentBody = `🚀 **Patch PR Created!**
commentBody = `🚀 **[Step 2/4] Patch PR Created!**
**📋 Patch Details:**
- **Environment**: \`${environment}\`
@@ -228,7 +228,8 @@ ${hasConflicts ? '3' : '2'}. Once merged, the patch release will automatically t
${hasConflicts ? '4' : '3'}. You'll receive updates here when the release completes
**🔗 Track Progress:**
- [View hotfix PR #${mockPrNumber}](${mockPrUrl})`;
- [View hotfix PR #${mockPrNumber}](${mockPrUrl})
- [This patch creation workflow run](https://github.com/${repository}/actions/runs/${runId})`;
} else if (hasGitHubCli) {
// Find the actual PR for the new branch using gh CLI
try {
@@ -269,7 +270,7 @@ ${hasConflicts ? '4' : '3'}. You'll receive updates here when the release comple
logContent.includes('Cherry-pick has conflicts') ||
pr.title.includes('[CONFLICTS]');
commentBody = `🚀 **Patch PR Created!**
commentBody = `🚀 **[Step 2/4] Patch PR Created!**
**📋 Patch Details:**
- **Environment**: \`${environment}\`
@@ -284,10 +285,11 @@ ${hasConflicts ? '3' : '2'}. Once merged, the patch release will automatically t
${hasConflicts ? '4' : '3'}. You'll receive updates here when the release completes
**🔗 Track Progress:**
- [View hotfix PR #${pr.number}](${pr.url})`;
- [View hotfix PR #${pr.number}](${pr.url})
- [This patch creation workflow run](https://github.com/${repository}/actions/runs/${runId})`;
} else {
// Fallback if PR not found yet
commentBody = `🚀 **Patch PR Created!**
commentBody = `🚀 **[Step 2/4] Patch PR Created!**
The patch release PR for this change has been created on branch [\`${branch}\`](https://github.com/${repository}/tree/${branch}).
@@ -296,23 +298,25 @@ The patch release PR for this change has been created on branch [\`${branch}\`](
2. Once merged, the patch release will automatically trigger
**🔗 Links:**
- [View all patch PRs](https://github.com/${repository}/pulls?q=is%3Apr+is%3Aopen+label%3Apatch)`;
- [View all patch PRs](https://github.com/${repository}/pulls?q=is%3Apr+is%3Aopen+label%3Apatch)
- [This patch creation workflow run](https://github.com/${repository}/actions/runs/${runId})`;
}
} catch (error) {
console.log('Error finding PR for branch:', error.message);
// Fallback
commentBody = `🚀 **Patch PR Created!**
commentBody = `🚀 **[Step 2/4] Patch PR Created!**
The patch release PR for this change has been created.
**🔗 Links:**
- [View all patch PRs](https://github.com/${repository}/pulls?q=is%3Apr+is%3Aopen+label%3Apatch)`;
- [View all patch PRs](https://github.com/${repository}/pulls?q=is%3Apr+is%3Aopen+label%3Apatch)
- [This patch creation workflow run](https://github.com/${repository}/actions/runs/${runId})`;
}
}
}
} else {
// Failure
commentBody = `❌ **Patch creation failed!**
commentBody = `❌ **[Step 2/4] Patch creation failed!**
There was an error creating the patch release.
@@ -326,7 +330,7 @@ There was an error creating the patch release.
}
if (!commentBody) {
commentBody = `❌ **Patch creation failed!**
commentBody = `❌ **[Step 2/4] Patch creation failed!**
No output was generated during patch creation.
+5 -3
View File
@@ -115,6 +115,7 @@ async function main() {
const isDryRun = argv.dryRun || body.includes('[DRY RUN]');
const forceSkipTests =
argv.forceSkipTests || process.env.FORCE_SKIP_TESTS === 'true';
const runId = process.env.GITHUB_RUN_ID || '0';
if (!headRef) {
throw new Error(
@@ -264,7 +265,7 @@ async function main() {
console.log(`Commenting on original PR ${originalPr}...`);
const npmTag = channel === 'stable' ? 'latest' : 'preview';
const commentBody = `🚀 **Patch Release Started!**
const commentBody = `🚀 **[Step 3/4] Patch Release ${environment === 'prod' ? 'Waiting for Approval' : 'Triggered'}!**
**📋 Release Details:**
- **Environment**: \`${environment}\`
@@ -273,10 +274,11 @@ async function main() {
- **Hotfix PR**: Merged
- **Release Branch**: [\`${releaseRef}\`](https://github.com/${context.repo.owner}/${context.repo.repo}/tree/${releaseRef})
** Status:** The patch release is now running. You'll receive another update when it completes.
** Status:** The patch release has been triggered${environment === 'prod' ? ' and is waiting for deployment approval. Please visit the specific workflow run link below and approve the deployment' : ''}. You'll receive another update when it completes.
**🔗 Track Progress:**
- [View release workflow](https://github.com/${context.repo.owner}/${context.repo.repo}/actions)`;
- [View release workflow history](https://github.com/${context.repo.owner}/${context.repo.repo}/actions/workflows/${workflowId})
- [This trigger workflow run](https://github.com/${context.repo.owner}/${context.repo.repo}/actions/runs/${runId})`;
if (!testMode) {
let tempDir;
+4 -2
View File
@@ -70,8 +70,10 @@ echo "review: Changing directory to $WORKTREE_PATH"
cd "$WORKTREE_PATH" || exit 1
# 4. Checkout the PR
echo "review: Checking out PR $pr..."
gh pr checkout "$pr" -f -R "$REPO"
echo "review: Cleaning worktree and checking out PR $pr..."
git reset --hard
git clean -fd
gh pr checkout "$pr" --branch "review-$pr" -f -R "$REPO"
# 5. Clean and Build
echo "review: Clearing possibly stale node_modules..."
+29 -15
View File
@@ -57,7 +57,7 @@ describe('patch-create-comment', () => {
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('🚀 **Patch PR Created!**');
expect(result.stdout).toContain('🚀 **[Step 2/4] Patch PR Created!**');
expect(result.stdout).toContain('Environment**: `prod`');
});
@@ -68,7 +68,7 @@ describe('patch-create-comment', () => {
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('🚀 **Patch PR Created!**');
expect(result.stdout).toContain('🚀 **[Step 2/4] Patch PR Created!**');
expect(result.stdout).toContain('Environment**: `dev`');
});
@@ -90,7 +90,7 @@ describe('patch-create-comment', () => {
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('🚀 **Patch PR Created!**');
expect(result.stdout).toContain('🚀 **[Step 2/4] Patch PR Created!**');
expect(result.stdout).toContain('Environment**: `prod`');
});
});
@@ -106,7 +106,7 @@ describe('patch-create-comment', () => {
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('🚀 **Patch PR Created!**');
expect(result.stdout).toContain('🚀 **[Step 2/4] Patch PR Created!**');
expect(result.stdout).toContain('Channel**: `preview`');
expect(result.stdout).toContain('Commit**: `abc1234`');
});
@@ -118,7 +118,9 @@ describe('patch-create-comment', () => {
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('❌ **Patch creation failed!**');
expect(result.stdout).toContain(
'❌ **[Step 2/4] Patch creation failed!**',
);
expect(result.stdout).toContain(
'There was an error creating the patch release',
);
@@ -136,7 +138,7 @@ describe('patch-create-comment', () => {
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('🚀 **Patch PR Created!**');
expect(result.stdout).toContain('🚀 **[Step 2/4] Patch PR Created!**');
expect(result.stdout).toContain('Channel**: `stable`');
expect(result.stdout).toContain('Commit**: `abc1234`');
expect(result.stdout).not.toContain('⚠️ Status');
@@ -152,7 +154,7 @@ describe('patch-create-comment', () => {
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('🚀 **Patch PR Created!**');
expect(result.stdout).toContain('🚀 **[Step 2/4] Patch PR Created!**');
expect(result.stdout).toContain(
'⚠️ Status**: Cherry-pick conflicts detected',
);
@@ -174,7 +176,9 @@ describe('patch-create-comment', () => {
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('️ **Patch PR already exists!**');
expect(result.stdout).toContain(
'️ **[Step 2/4] Patch PR already exists!**',
);
expect(result.stdout).toContain(
'A patch PR for this change already exists: [#8700](https://github.com/google-gemini/gemini-cli/pull/8700)',
);
@@ -194,7 +198,7 @@ describe('patch-create-comment', () => {
expect(result.success).toBe(true);
expect(result.stdout).toContain(
'️ **Patch branch exists but no PR found!**',
'️ **[Step 2/4] Patch branch exists but no PR found!**',
);
expect(result.stdout).toContain(
'Delete the branch: `git branch -D hotfix/v0.5.0-preview.2/preview/cherry-pick-jkl3456`',
@@ -213,7 +217,9 @@ describe('patch-create-comment', () => {
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('❌ **Patch creation failed!**');
expect(result.stdout).toContain(
'❌ **[Step 2/4] Patch creation failed!**',
);
expect(result.stdout).toContain(
'There was an error creating the patch release',
);
@@ -231,7 +237,9 @@ describe('patch-create-comment', () => {
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('❌ **Patch creation failed!**');
expect(result.stdout).toContain(
'❌ **[Step 2/4] Patch creation failed!**',
);
expect(result.stdout).toContain(
'There was an error creating the patch release',
);
@@ -292,7 +300,9 @@ describe('patch-create-comment', () => {
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('❌ **Patch creation failed!**');
expect(result.stdout).toContain(
'❌ **[Step 2/4] Patch creation failed!**',
);
expect(result.stdout).toContain(
'There was an error creating the patch release',
);
@@ -316,7 +326,9 @@ git push origin hotfix/v0.4.1/stable/cherry-pick-abc1234
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('🔒 **GitHub App Permission Issue**');
expect(result.stdout).toContain(
'🔒 **[Step 2/4] GitHub App Permission Issue**',
);
expect(result.stdout).toContain(
'Please run these commands manually to create the release branch:',
);
@@ -339,7 +351,7 @@ git push origin hotfix/v0.4.1/stable/cherry-pick-abc1234
expect(result.stdout).toContain(
'🧪 TEST MODE - No API calls will be made',
);
expect(result.stdout).toContain('🚀 **Patch PR Created!**');
expect(result.stdout).toContain('🚀 **[Step 2/4] Patch PR Created!**');
});
it('should generate mock content in test mode for failure', () => {
@@ -348,7 +360,9 @@ git push origin hotfix/v0.4.1/stable/cherry-pick-abc1234
);
expect(result.success).toBe(true);
expect(result.stdout).toContain('❌ **Patch creation failed!**');
expect(result.stdout).toContain(
'❌ **[Step 2/4] Patch creation failed!**',
);
});
});
});