Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
70121e2
fix(core): show edit/exec diffs when a PreToolUse hook returns ask
yiliang114 Aug 19, 2026
e0e0617
fix(core): preserve hookAskReason when restricting bubbled edit/exec …
yiliang114 Aug 19, 2026
b996ea4
fix(core,cli): close PreToolUse ask-bounce abort/rendering gaps (#943…
yiliang114 Aug 19, 2026
1b7e768
test(core): pin hookAskReason passthrough in the approval restriction
yiliang114 Aug 19, 2026
4fbc8b3
Merge branch 'main' into fix/9434-hook-ask-diff
yiliang114 Aug 19, 2026
4585a3d
fix(shell): keep a user-confirmed sed edit across a re-entrant preview
yiliang114 Aug 19, 2026
b0558f8
fix(core): re-apply plan-shell policy in the PreToolUse ask bounce
yiliang114 Aug 19, 2026
60db259
fix(core): close round-4 ask-bounce findings (#9434)
yiliang114 Aug 19, 2026
2e64af9
fix(core): close round-5 ask-bounce findings (#9434)
yiliang114 Aug 19, 2026
611d995
fix(core): close round-6 ask-bounce findings (#9434)
yiliang114 Aug 20, 2026
c2bfc3b
fix(core): invalidate round-1 IDE diff on PreToolUse ask bounce (#9434)
yiliang114 Aug 20, 2026
57b6b90
fix(core): close round-8 ask-bounce findings (#9434)
yiliang114 Aug 20, 2026
b74c60a
Merge remote-tracking branch 'origin/main' into fix/9434-hook-ask-diff
yiliang114 Aug 20, 2026
adda82a
Merge remote-tracking branch 'origin/main' into HEAD
yiliang114 Aug 24, 2026
fd61782
test(core): remove duplicate scheduler stub
yiliang114 Aug 24, 2026
2b43f0c
test(cli): colocate permission-suggestions test with its module
yiliang114 Aug 24, 2026
f1c7b66
fix(core): narrow hook ask diff confirmation
yiliang114 Aug 30, 2026
c43d529
Merge branch 'main' into fix/9434-hook-ask-diff
yiliang114 Aug 30, 2026
fe8174d
fix(core): harden PreToolUse ask bounce confirmations
yiliang114 Aug 30, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1094,4 +1094,62 @@ describe('ToolConfirmationMessage', () => {
expect(frame).toMatch(/\.{3} last \d+ lines hidden \.{3}/);
});
});

describe('PreToolUse hook ask reason (#9434)', () => {
it('shows the hook reason alongside the edit diff', () => {
const confirmationDetails: ToolCallConfirmationDetails = {
type: 'edit',
title: 'Confirm Edit: config.yaml',
fileName: 'config.yaml',
filePath: '/repo/config.yaml',
fileDiff: '-old\n+new',
originalContent: 'old',
newContent: 'new',
hideAlwaysAllow: true,
hookAskReason: 'path requires human review',
onConfirm: vi.fn(),
};

const { lastFrame } = renderWithProviders(
<ToolConfirmationMessage
confirmationDetails={confirmationDetails}
config={mockConfig}
availableTerminalHeight={30}
contentWidth={80}
/>,
);

const frame = lastFrame() ?? '';
// The hook reason is surfaced above the tool's diff view...
expect(frame).toContain(
'Hook requested confirmation: path requires human review',
);
// ...and the edit confirmation itself still renders.
expect(frame).toContain('Apply this change?');
});

it('does not show a hook reason line for ordinary confirmations', () => {
const confirmationDetails: ToolCallConfirmationDetails = {
type: 'edit',
title: 'Confirm Edit',
fileName: 'test.txt',
filePath: '/test.txt',
fileDiff: '...diff...',
originalContent: 'a',
newContent: 'b',
onConfirm: vi.fn(),
};

const { lastFrame } = renderWithProviders(
<ToolConfirmationMessage
confirmationDetails={confirmationDetails}
config={mockConfig}
availableTerminalHeight={30}
contentWidth={80}
/>,
);

expect(lastFrame()).not.toContain('Hook requested confirmation:');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ export const ToolConfirmationMessage: React.FC<
}) => {
const { onConfirm } = confirmationDetails;
const autoModeFallback = confirmationDetails.autoModeFallback;
// Reason a PreToolUse hook gave when escalating this call to an
// interactive confirmation while the tool's own view (e.g. edit diff)
// is being shown (#9434). Undefined for ordinary confirmations.
const hookAskReason = confirmationDetails.hookAskReason;

const settings = useSettings();
const preferredEditor = settings.merged.general?.preferredEditor as
Expand Down Expand Up @@ -192,14 +196,24 @@ export const ToolConfirmationMessage: React.FC<
hard: true,
}).split('\n').length + 1
: 0;
const HOOK_ASK_REASON_HEIGHT = hookAskReason
? wrapAnsi(
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
`⚠ ${t('Hook requested confirmation: {{reason}}', {
reason: hookAskReason,
})}`,
warningContentWidth,
{ trim: false, hard: true },
).split('\n').length + 1
: 0;

const surroundingElementsHeight =
PADDING_OUTER_Y +
MARGIN_BODY_BOTTOM +
HEIGHT_QUESTION +
MARGIN_QUESTION_BOTTOM +
HEIGHT_OPTIONS +
AUTO_MODE_FALLBACK_HEIGHT;
AUTO_MODE_FALLBACK_HEIGHT +
HOOK_ASK_REASON_HEIGHT;
return Math.max(availableTerminalHeight - surroundingElementsHeight, 1);
}

Expand Down Expand Up @@ -696,6 +710,25 @@ export const ToolConfirmationMessage: React.FC<
);
}

// A PreToolUse hook escalated this call while the tool's own view (e.g.
// the edit diff) is shown — surface the hook's reason above the body so
// the user knows why the prompt appeared (#9434).
if (hookAskReason) {
bodyContent = (
<Box flexDirection="column">
<Box paddingX={1} marginLeft={1} marginBottom={1}>
<Text color={theme.status.warning}>
⚠{' '}
{t('Hook requested confirmation: {{reason}}', {
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
reason: hookAskReason,
})}
</Text>
</Box>
{bodyContent}
</Box>
);
}

// For exec/mcp confirmations the type-specific question text would
// restate what the body already shows (the full command, or the labeled
// server + tool). Use the generic prompt so the question line acts as a
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/agents/workflow-run-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1046,6 +1046,7 @@ function restrictWorkflowConfirmationDetails(
hideModify: true,
skipIdeDiff: true,
warnings: details.warnings ? [...details.warnings] : undefined,
hookAskReason: details.hookAskReason,
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
};
case 'exec':
return {
Expand All @@ -1055,6 +1056,7 @@ function restrictWorkflowConfirmationDetails(
rootCommand: details.rootCommand,
hideAlwaysAllow: true,
warnings: details.warnings ? [...details.warnings] : undefined,
hookAskReason: details.hookAskReason,
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
};
case 'mcp':
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
return {
Expand Down
94 changes: 94 additions & 0 deletions packages/core/src/core/coreToolScheduler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10786,6 +10786,7 @@ describe('CoreToolScheduler telemetry spans', () => {
options.experimentalZedIntegration ?? false,
getShouldAvoidPermissionPrompts: () =>
options.shouldAvoidPermissionPrompts ?? false,
getIdeMode: () => false,
getTelemetryIncludeSensitiveSpanAttributes: () =>
options.includeSensitiveSpanAttributes ?? false,
getTelemetrySensitiveSpanAttributeMaxLength: () =>
Expand Down Expand Up @@ -12440,6 +12441,99 @@ describe('CoreToolScheduler telemetry spans', () => {
expect(getToolSpans()[0].ended).toBe(false);
});

it('bounces a PreToolUse ask on an edit tool to its diff confirmation with the hook reason attached', async () => {
// #9434: a hook 'ask' on Edit/WriteFile must surface the tool's diff
// view, not a bare reason prompt, with the hook reason attached.
const toolOnConfirm = vi.fn();
const execute = vi.fn().mockResolvedValue({
llmContent: 'ok',
returnDisplay: 'ok',
});
const editTool = new MockTool({
name: 'mockTool',
execute,
getConfirmationDetails: async () => ({
type: 'edit',
title: 'Confirm Edit: test.txt',
fileName: 'test.txt',
filePath: '/tmp/test.txt',
fileDiff: '-old\n+new',
originalContent: 'old',
newContent: 'new',
onConfirm: toolOnConfirm,
}),
});
const messageBus = askMessageBus('path requires human review');
const { onToolCallsUpdate, onAllToolCallsComplete } = await scheduleWithAsk(
{ messageBus, tools: [editTool] },
);

const waiting = (await waitForStatus(
onToolCallsUpdate,
'awaiting_approval',
)) as WaitingToolCall;

const details =
waiting.confirmationDetails as ToolCallConfirmationDetails & {
fileDiff?: string;
hideAlwaysAllow?: boolean;
};
// The tool's own diff view is reused instead of a bare reason prompt...
expect(details.type).toBe('edit');
expect(details.fileDiff).toBe('-old\n+new');
// ...with the hook's reason attached for display.
expect(details.hookAskReason).toBe('path requires human review');
// The hook re-evaluates on every call → no persisted "always allow".
expect(details.hideAlwaysAllow).toBe(true);

// Approving routes through the tool's onConfirm and re-executes once.
await details.onConfirm(ToolConfirmationOutcome.ProceedOnce);
await vi.waitFor(() => {
expect(onAllToolCallsComplete).toHaveBeenCalled();
});
const completed = onAllToolCallsComplete.mock.calls.at(
-1,
)?.[0] as ToolCall[];
expect(completed[0].status).toBe('success');
expect(execute).toHaveBeenCalledTimes(1);
expect(toolOnConfirm).toHaveBeenCalledWith(
ToolConfirmationOutcome.ProceedOnce,
undefined,
);
});

it('keeps the synthetic reason prompt when a PreToolUse ask bounces a non-structured tool', async () => {
// Tools whose confirmation view is a plain 'info' description keep the
// hook-reason prompt (the description adds nothing over it).
const infoTool = new MockTool({
name: 'mockTool',
getConfirmationDetails: async () => ({
type: 'info',
title: 'Confirm mockTool',
prompt: 'A mock tool invocation for mockTool',
onConfirm: vi.fn(),
}),
});
const messageBus = askMessageBus('confirm this');
const { onToolCallsUpdate } = await scheduleWithAsk({
messageBus,
tools: [infoTool],
});

const waiting = (await waitForStatus(
onToolCallsUpdate,
'awaiting_approval',
)) as WaitingToolCall;
expect(waiting.confirmationDetails.type).toBe('info');
expect(
(waiting.confirmationDetails as { prompt: string }).prompt,
).toContain('confirm this');
expect(
(waiting.confirmationDetails as { renderPromptAsPlainText?: boolean })
.renderPromptAsPlainText,
).toBe(true);
});

it('executes the tool exactly once when the user approves an ask (no re-ask loop)', async () => {
const execute = vi.fn().mockResolvedValue({
llmContent: 'ok',
Expand Down
Loading
Loading