mirror of
https://github.com/wu736139669/hapi.git
synced 2026-08-05 06:24:37 +00:00
fix(claude): apply mid-turn permission mode changes to canCallTool (#764)
`PermissionHandler` stored its own `permissionMode` field and only updated it inside `handleModeChange`, which is called when a new batch is pulled from the queue. The `SetSessionConfig` RPC (web dropdown changes) updates `runClaude.ts`'s `currentPermissionMode` and the session keepalive metadata, but never reaches the handler — so switching to Yolo mid-turn left `canCallTool` checking the stale mode and still prompting for approval. Closes #735. Drop the stored field and read live from `session.getPermissionMode()`, mirroring how the OpenCode permission handler already works. Override `Session.getPermissionMode()` in `claude/session.ts` to return the Claude-narrow `PermissionMode`, sound because the matching `setPermissionMode` setter only accepts that subset. via [HAPI](https://hapi.run) Co-authored-by: HAPI <noreply@hapi.run>
This commit is contained in:
@@ -78,6 +78,13 @@ export class Session extends AgentSessionBase<EnhancedMode> {
|
||||
this.permissionMode = mode;
|
||||
};
|
||||
|
||||
// Override base getPermissionMode to return the Claude-narrow type. Safe
|
||||
// because the only writer (setPermissionMode above) accepts only Claude
|
||||
// PermissionMode, so the field cannot hold a foreign flavor value.
|
||||
getPermissionMode(): PermissionMode | undefined {
|
||||
return this.permissionMode as PermissionMode | undefined;
|
||||
}
|
||||
|
||||
setModel = (model: SessionModel): void => {
|
||||
this.model = model;
|
||||
};
|
||||
|
||||
@@ -5,6 +5,7 @@ import type { Session } from '../session';
|
||||
|
||||
function createFakeSession() {
|
||||
const queueItems: { message: string; mode: unknown }[] = [];
|
||||
let permissionMode: string | undefined;
|
||||
|
||||
const session = {
|
||||
client: {
|
||||
@@ -18,7 +19,10 @@ function createFakeSession() {
|
||||
queueItems.push({ message, mode });
|
||||
}),
|
||||
},
|
||||
setPermissionMode: vi.fn(),
|
||||
setPermissionMode: vi.fn((mode: string) => {
|
||||
permissionMode = mode;
|
||||
}),
|
||||
getPermissionMode: vi.fn(() => permissionMode),
|
||||
} as unknown as Session;
|
||||
|
||||
return { session, queueItems };
|
||||
@@ -105,4 +109,35 @@ describe('PermissionHandler — YOLO plan mode', () => {
|
||||
expect(result.behavior).toBe('allow');
|
||||
expect(queueItems).toHaveLength(0);
|
||||
});
|
||||
|
||||
// Regression: turn-in-progress switch from default to bypassPermissions via
|
||||
// SetSessionConfig RPC updates session.setPermissionMode but doesn't go
|
||||
// through handler.handleModeChange. The next canCallTool must reflect the
|
||||
// new mode. See issue #735.
|
||||
it('reflects session permission mode changes between tool calls', async () => {
|
||||
const { session } = createFakeSession();
|
||||
const handler = new PermissionHandler(session);
|
||||
handler.handleModeChange('default');
|
||||
|
||||
// Simulate RPC handler in runClaude updating the session directly,
|
||||
// bypassing handler.handleModeChange (as happens on web dropdown change).
|
||||
session.setPermissionMode('bypassPermissions');
|
||||
|
||||
handler.onMessage({
|
||||
type: 'assistant',
|
||||
message: {
|
||||
role: 'assistant',
|
||||
content: [{ type: 'tool_use', id: 'tc-4', name: 'Bash', input: { command: 'ls' } }],
|
||||
},
|
||||
} as any);
|
||||
|
||||
const result = await handler.handleToolCall(
|
||||
'Bash',
|
||||
{ command: 'ls' },
|
||||
{ permissionMode: 'bypassPermissions' } as any,
|
||||
{ signal: new AbortController().signal }
|
||||
);
|
||||
|
||||
expect(result.behavior).toBe('allow');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -164,14 +164,22 @@ export class PermissionHandler extends BasePermissionHandler<PermissionResponse,
|
||||
private allowedTools = new Set<string>();
|
||||
private allowedBashLiterals = new Set<string>();
|
||||
private allowedBashPrefixes = new Set<string>();
|
||||
private permissionMode: PermissionMode = 'default';
|
||||
private onPermissionRequestCallback?: (toolCallId: string) => void;
|
||||
|
||||
constructor(session: Session) {
|
||||
super(session.client);
|
||||
this.session = session;
|
||||
}
|
||||
|
||||
|
||||
// Read from the session so mid-turn updates via SetSessionConfig RPC
|
||||
// (which writes through session.setPermissionMode) are picked up by the
|
||||
// next canCallTool check. Previously this was a stored field updated
|
||||
// only on batch boundaries, so web dropdown changes during a turn were
|
||||
// silently ignored — see issue #735.
|
||||
private get permissionMode(): PermissionMode {
|
||||
return this.session.getPermissionMode() ?? 'default';
|
||||
}
|
||||
|
||||
/**
|
||||
* Set callback to trigger when permission request is made
|
||||
*/
|
||||
@@ -180,7 +188,6 @@ export class PermissionHandler extends BasePermissionHandler<PermissionResponse,
|
||||
}
|
||||
|
||||
handleModeChange(mode: PermissionMode) {
|
||||
this.permissionMode = mode;
|
||||
this.session.setPermissionMode(mode);
|
||||
}
|
||||
|
||||
@@ -215,7 +222,6 @@ export class PermissionHandler extends BasePermissionHandler<PermissionResponse,
|
||||
|
||||
// Update permission mode
|
||||
if (response.mode) {
|
||||
this.permissionMode = response.mode;
|
||||
this.session.setPermissionMode(response.mode);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user