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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ jobs:
run: |-
npm run build

- name: 'Bundle CLI for E2E tests'
run: |-
npm run bundle

- name: 'Set up Docker'
if: |-
${{ matrix.sandbox == 'sandbox:docker' }}
Expand Down Expand Up @@ -103,6 +107,10 @@ jobs:
run: |-
npm run build

- name: 'Bundle CLI for E2E tests'
run: |-
npm run bundle

- name: 'Run E2E tests'
env:
OPENAI_API_KEY: '${{ secrets.OPENAI_API_KEY }}'
Expand Down
35 changes: 12 additions & 23 deletions integration-tests/sdk-typescript/abort-and-lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,22 +314,13 @@ describe('AbortController and Process Lifecycle (E2E)', () => {
});

it('should handle control responses when stdin closes before replies', async () => {
const testFilePath = await helper.getPath('test.txt');
await helper.createFile('test.txt', 'original content');

let canUseToolCalled = false;
let canUseToolCalledResolve: () => void = () => {};
const canUseToolCalledPromise = new Promise<void>((resolve, reject) => {
const canUseToolCalledPromise = new Promise<void>((resolve) => {
canUseToolCalledResolve = resolve;
setTimeout(() => {
reject(new Error('canUseTool callback not called'));
}, 15000);
});

let inputStreamDoneResolve: () => void = () => {};
const inputStreamDonePromise = new Promise<void>((resolve, reject) => {
inputStreamDoneResolve = resolve;
setTimeout(() => {
reject(new Error('inputStreamDonePromise timeout'));
}, 15000);
});

let firstResultResolve: () => void = () => {};
Expand Down Expand Up @@ -362,12 +353,10 @@ describe('AbortController and Process Lifecycle (E2E)', () => {
session_id: sessionId,
message: {
role: 'user',
content:
'Write "updated" to test.txt. Stop if any exception occurs.',
content: `Use the write_file tool to write "updated" to the file at ${testFilePath}. Then reply with "done".`,
},
parent_tool_use_id: null,
};
await inputStreamDonePromise;
}

const q = query({
Expand All @@ -378,10 +367,8 @@ describe('AbortController and Process Lifecycle (E2E)', () => {
permissionMode: 'default',
coreTools: ['read_file', 'write_file'],
canUseTool: async (toolName, input) => {
inputStreamDoneResolve();
await new Promise((resolve) => setTimeout(resolve, 1000));
canUseToolCalled = true;
canUseToolCalledResolve();

return {
behavior: 'allow',
updatedInput: input,
Expand All @@ -394,10 +381,8 @@ describe('AbortController and Process Lifecycle (E2E)', () => {
try {
const loop = async () => {
let resultCount = 0;
for await (const _message of q) {
console.log(JSON.stringify(_message, null, 2));
// Consume messages until completion.
if (isSDKResultMessage(_message)) {
for await (const message of q) {
if (isSDKResultMessage(message)) {
resultCount += 1;
if (resultCount === 1) {
firstResultResolve();
Expand All @@ -416,8 +401,12 @@ describe('AbortController and Process Lifecycle (E2E)', () => {
await canUseToolCalledPromise;
await secondResultPromise;

// Signal stdin is done so CLI stops waiting
q.endInput();

const content = await helper.readFile('test.txt');
expect(content).toBe('original content');
expect(canUseToolCalled).toBe(true);
expect(content).toBe('updated');
} finally {
await q.close();
}
Expand Down
46 changes: 16 additions & 30 deletions packages/cli/src/acp-integration/session/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ import {
readManyFiles,
Storage,
ToolNames,
buildPermissionCheckContext,
evaluatePermissionRules,
fireNotificationHook,
firePermissionRequestHook,
firePreToolUseHook,
Expand All @@ -53,6 +51,9 @@ import {
getPlanModeSystemReminder,
getSubagentSystemReminder,
getArenaSystemReminder,
evaluatePermissionFlow,
needsConfirmation,
isPlanModeBlocked,
} from '@qwen-code/qwen-code-core';

import { RequestError } from '@agentclientprotocol/sdk';
Expand Down Expand Up @@ -1368,58 +1369,43 @@ export class Session implements SessionContext {
// The VS Code extension is just a UI layer for requestPermission.
const isAskUserQuestionTool = fc.name === ToolNames.ASK_USER_QUESTION;

// ---- L3: Tool's default permission ----
// In YOLO mode, force 'allow' for everything except ask_user_question.
const defaultPermission =
this.config.getApprovalMode() !== ApprovalMode.YOLO ||
isAskUserQuestionTool
? await invocation.getDefaultPermission()
: 'allow';

// ---- L4: PermissionManager override (if relevant rules exist) ----
// ---- L3→L4: Shared permission flow ----
const toolParams = invocation.params as Record<string, unknown>;
const pmCtx = buildPermissionCheckContext(
const flowResult = await evaluatePermissionFlow(
this.config,
invocation,
fc.name,
toolParams,
this.config.getTargetDir?.() ?? '',
);
const { finalPermission, pmForcedAsk } = await evaluatePermissionRules(
pm,
defaultPermission,
pmCtx,
);

const needsConfirmation = finalPermission === 'ask';
const { finalPermission, pmForcedAsk, pmCtx, denyMessage } = flowResult;

// ---- L5: ApprovalMode overrides ----
const isPlanMode = approvalMode === ApprovalMode.PLAN;

if (finalPermission === 'deny') {
return earlyErrorResponse(
new Error(
defaultPermission === 'deny'
? `Tool "${fc.name}" is denied: command substitution is not allowed for security reasons.`
: `Tool "${fc.name}" is denied by permission rules.`,
),
new Error(denyMessage ?? `Tool "${fc.name}" is denied.`),
fc.name,
);
}

let didRequestPermission = false;
let confirmationDetails: ToolCallConfirmationDetails | undefined;

if (needsConfirmation) {
if (needsConfirmation(finalPermission, approvalMode, fc.name)) {
confirmationDetails =
await invocation.getConfirmationDetails(abortSignal);

// Centralised rule injection (for display and persistence)
injectPermissionRulesIfMissing(confirmationDetails, pmCtx);

if (
isPlanMode &&
!isExitPlanModeTool &&
!isAskUserQuestionTool &&
confirmationDetails.type !== 'info'
isPlanModeBlocked(
isPlanMode,
isExitPlanModeTool,
isAskUserQuestionTool,
confirmationDetails,
)
) {
return earlyErrorResponse(
new Error(
Expand Down
67 changes: 25 additions & 42 deletions packages/core/src/core/coreToolScheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,15 @@ import { CONCURRENCY_SAFE_KINDS } from '../tools/tools.js';
import { isShellCommandReadOnly } from '../utils/shellReadOnlyChecker.js';
import { stripShellWrapper } from '../utils/shell-utils.js';
import {
buildPermissionCheckContext,
evaluatePermissionRules,
injectPermissionRulesIfMissing,
persistPermissionOutcome,
} from './permission-helpers.js';
import {
evaluatePermissionFlow,
needsConfirmation,
isPlanModeBlocked,
isAutoEditApproved,
} from './permissionFlow.js';
import { getResponseTextFromParts } from '../utils/generateContentResponseUtilities.js';
import type { ModifyContext } from '../tools/modifiable-tool.js';
import {
Expand Down Expand Up @@ -987,20 +991,16 @@ export class CoreToolScheduler {
// L3→L4→L5 Permission Flow
// =================================================================

// ---- L3: Tool's default permission ----
const defaultPermission: string =
await invocation.getDefaultPermission();

// ---- L4: PermissionManager override (if relevant rules exist) ----
const pm = this.config.getPermissionManager?.();
// ---- L3→L4: Shared permission flow ----
const toolParams = invocation.params as Record<string, unknown>;
const pmCtx = buildPermissionCheckContext(
const flowResult = await evaluatePermissionFlow(
this.config,
invocation,
reqInfo.name,
toolParams,
this.config.getTargetDir?.() ?? '',
);
const { finalPermission, pmForcedAsk } =
await evaluatePermissionRules(pm, defaultPermission, pmCtx);
const { finalPermission, pmForcedAsk, pmCtx, denyMessage } =
flowResult;

// ---- L5: Final decision based on permission + ApprovalMode ----
const approvalMode = this.config.getApprovalMode();
Expand All @@ -1019,22 +1019,12 @@ export class CoreToolScheduler {

if (finalPermission === 'deny') {
// Hard deny: security violation or PM explicit deny
let denyMessage: string;
if (defaultPermission === 'deny') {
denyMessage = `Tool "${reqInfo.name}" is denied: command substitution is not allowed for security reasons.`;
} else {
const matchingRule = pm?.findMatchingDenyRule(pmCtx);
const ruleInfo = matchingRule
? ` Matching deny rule: "${matchingRule}".`
: '';
denyMessage = `Tool "${reqInfo.name}" is denied by permission rules.${ruleInfo}`;
}
this.setStatusInternal(
reqInfo.callId,
'error',
createErrorResponse(
reqInfo,
new Error(denyMessage),
new Error(denyMessage ?? `Tool "${reqInfo.name}" is denied.`),
ToolErrorType.EXECUTION_DENIED,
),
);
Expand All @@ -1049,7 +1039,7 @@ export class CoreToolScheduler {
reqInfo.name === ToolNames.ASK_USER_QUESTION;
let confirmationDetails: ToolCallConfirmationDetails | undefined;

if (approvalMode === ApprovalMode.YOLO && !isAskUserQuestionTool) {
if (!needsConfirmation(finalPermission, approvalMode, reqInfo.name)) {
this.setToolCallOutcome(
reqInfo.callId,
ToolConfirmationOutcome.ProceedAlways,
Expand All @@ -1063,10 +1053,12 @@ export class CoreToolScheduler {
injectPermissionRulesIfMissing(confirmationDetails, pmCtx);

if (
isPlanMode &&
!isExitPlanModeTool &&
!isAskUserQuestionTool &&
confirmationDetails.type !== 'info'
isPlanModeBlocked(
isPlanMode,
isExitPlanModeTool,
isAskUserQuestionTool,
confirmationDetails,
)
) {
this.setStatusInternal(reqInfo.callId, 'error', {
callId: reqInfo.callId,
Expand All @@ -1083,11 +1075,7 @@ export class CoreToolScheduler {
}

// AUTO_EDIT mode: auto-approve edit-like and info tools
if (
approvalMode === ApprovalMode.AUTO_EDIT &&
(confirmationDetails.type === 'edit' ||
confirmationDetails.type === 'info')
) {
if (isAutoEditApproved(approvalMode, confirmationDetails)) {
this.setToolCallOutcome(
reqInfo.callId,
ToolConfirmationOutcome.ProceedAlways,
Expand Down Expand Up @@ -1917,22 +1905,17 @@ export class CoreToolScheduler {
for (const pendingTool of pendingTools) {
try {
// Re-run L3→L4 to see if the tool can now be auto-approved
const defaultPermission =
await pendingTool.invocation.getDefaultPermission();
const toolParams = pendingTool.invocation.params as Record<
string,
unknown
>;
const pmCtx = buildPermissionCheckContext(
const flowResult = await evaluatePermissionFlow(
this.config,
pendingTool.invocation,
pendingTool.request.name,
toolParams,
this.config.getTargetDir?.() ?? '',
);
const { finalPermission } = await evaluatePermissionRules(
this.config.getPermissionManager?.(),
defaultPermission,
pmCtx,
);
const { finalPermission } = flowResult;

if (finalPermission === 'allow') {
this.setToolCallOutcome(
Expand Down
Loading
Loading