fix: align SDK permission-mode args with CLI behavior (#33)
This commit is contained in:
1
.github/dependabot.yml
vendored
1
.github/dependabot.yml
vendored
@@ -6,6 +6,7 @@ updates:
|
||||
interval: "daily"
|
||||
allow:
|
||||
- dependency-name: "@letta-ai/letta-code"
|
||||
versioning-strategy: "increase"
|
||||
labels:
|
||||
- "dependencies"
|
||||
commit-message:
|
||||
|
||||
@@ -26,7 +26,7 @@
|
||||
"url": "https://github.com/letta-ai/letta-code-sdk"
|
||||
},
|
||||
"dependencies": {
|
||||
"@letta-ai/letta-code": "latest"
|
||||
"@letta-ai/letta-code": "0.14.16"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@types/bun": "latest",
|
||||
|
||||
27
src/interactiveToolPolicy.test.ts
Normal file
27
src/interactiveToolPolicy.test.ts
Normal file
@@ -0,0 +1,27 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import {
|
||||
isHeadlessAutoAllowTool,
|
||||
isInteractiveApprovalTool,
|
||||
requiresRuntimeUserInput,
|
||||
} from "./interactiveToolPolicy.js";
|
||||
|
||||
describe("interactive tool policy", () => {
|
||||
test("marks interactive approval tools", () => {
|
||||
expect(isInteractiveApprovalTool("AskUserQuestion")).toBe(true);
|
||||
expect(isInteractiveApprovalTool("EnterPlanMode")).toBe(true);
|
||||
expect(isInteractiveApprovalTool("ExitPlanMode")).toBe(true);
|
||||
expect(isInteractiveApprovalTool("Bash")).toBe(false);
|
||||
});
|
||||
|
||||
test("marks runtime user input tools", () => {
|
||||
expect(requiresRuntimeUserInput("AskUserQuestion")).toBe(true);
|
||||
expect(requiresRuntimeUserInput("ExitPlanMode")).toBe(true);
|
||||
expect(requiresRuntimeUserInput("EnterPlanMode")).toBe(false);
|
||||
});
|
||||
|
||||
test("marks headless auto-allow tools", () => {
|
||||
expect(isHeadlessAutoAllowTool("EnterPlanMode")).toBe(true);
|
||||
expect(isHeadlessAutoAllowTool("AskUserQuestion")).toBe(false);
|
||||
expect(isHeadlessAutoAllowTool("ExitPlanMode")).toBe(false);
|
||||
});
|
||||
});
|
||||
24
src/interactiveToolPolicy.ts
Normal file
24
src/interactiveToolPolicy.ts
Normal file
@@ -0,0 +1,24 @@
|
||||
// Interactive tool policy for SDK permission callbacks.
|
||||
// Centralizes behavior so transport/session logic doesn't hardcode names inline.
|
||||
|
||||
const INTERACTIVE_APPROVAL_TOOLS = new Set([
|
||||
"AskUserQuestion",
|
||||
"EnterPlanMode",
|
||||
"ExitPlanMode",
|
||||
]);
|
||||
|
||||
const RUNTIME_USER_INPUT_TOOLS = new Set(["AskUserQuestion", "ExitPlanMode"]);
|
||||
|
||||
const HEADLESS_AUTO_ALLOW_TOOLS = new Set(["EnterPlanMode"]);
|
||||
|
||||
export function isInteractiveApprovalTool(toolName: string): boolean {
|
||||
return INTERACTIVE_APPROVAL_TOOLS.has(toolName);
|
||||
}
|
||||
|
||||
export function requiresRuntimeUserInput(toolName: string): boolean {
|
||||
return RUNTIME_USER_INPUT_TOOLS.has(toolName);
|
||||
}
|
||||
|
||||
export function isHeadlessAutoAllowTool(toolName: string): boolean {
|
||||
return HEADLESS_AUTO_ALLOW_TOOLS.has(toolName);
|
||||
}
|
||||
@@ -1,35 +1,44 @@
|
||||
import { describe, expect, test, mock } from "bun:test";
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { Session } from "./session.js";
|
||||
|
||||
describe("Session", () => {
|
||||
describe("handleCanUseTool with bypassPermissions", () => {
|
||||
test("auto-approves tools when permissionMode is bypassPermissions", async () => {
|
||||
// Create a session with bypassPermissions
|
||||
const session = new Session({
|
||||
permissionMode: "bypassPermissions",
|
||||
});
|
||||
|
||||
// Access the private method for testing
|
||||
async function invokeCanUseTool(
|
||||
session: Session,
|
||||
tool_name: string,
|
||||
input: Record<string, unknown>,
|
||||
): Promise<unknown> {
|
||||
// @ts-expect-error - accessing private method for testing
|
||||
const handleCanUseTool = session.handleCanUseTool.bind(session);
|
||||
|
||||
// Mock the transport.write to capture the response
|
||||
let capturedResponse: unknown;
|
||||
// @ts-expect-error - accessing private property for testing
|
||||
session.transport.write = async (msg: unknown) => {
|
||||
capturedResponse = msg;
|
||||
};
|
||||
|
||||
// Simulate a control_request for tool approval
|
||||
await handleCanUseTool("test-request-id", {
|
||||
subtype: "can_use_tool",
|
||||
tool_name: "Bash",
|
||||
tool_name,
|
||||
tool_call_id: "test-tool-call-id",
|
||||
input: { command: "ls" },
|
||||
input,
|
||||
permission_suggestions: [],
|
||||
blocked_path: null,
|
||||
});
|
||||
|
||||
return capturedResponse;
|
||||
}
|
||||
|
||||
test("auto-approves tools when permissionMode is bypassPermissions", async () => {
|
||||
// Create a session with bypassPermissions
|
||||
const session = new Session({
|
||||
permissionMode: "bypassPermissions",
|
||||
});
|
||||
|
||||
const capturedResponse = await invokeCanUseTool(session, "Bash", {
|
||||
command: "ls",
|
||||
});
|
||||
|
||||
// Verify the response auto-approves
|
||||
expect(capturedResponse).toEqual({
|
||||
type: "control_response",
|
||||
@@ -51,22 +60,8 @@ describe("Session", () => {
|
||||
permissionMode: "default",
|
||||
});
|
||||
|
||||
// @ts-expect-error - accessing private method for testing
|
||||
const handleCanUseTool = session.handleCanUseTool.bind(session);
|
||||
|
||||
let capturedResponse: unknown;
|
||||
// @ts-expect-error - accessing private property for testing
|
||||
session.transport.write = async (msg: unknown) => {
|
||||
capturedResponse = msg;
|
||||
};
|
||||
|
||||
await handleCanUseTool("test-request-id", {
|
||||
subtype: "can_use_tool",
|
||||
tool_name: "Bash",
|
||||
tool_call_id: "test-tool-call-id",
|
||||
input: { command: "ls" },
|
||||
permission_suggestions: [],
|
||||
blocked_path: null,
|
||||
const capturedResponse = await invokeCanUseTool(session, "Bash", {
|
||||
command: "ls",
|
||||
});
|
||||
|
||||
// Verify the response denies (no callback registered)
|
||||
@@ -84,6 +79,58 @@ describe("Session", () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("auto-allows EnterPlanMode without callback", async () => {
|
||||
const session = new Session({
|
||||
permissionMode: "default",
|
||||
});
|
||||
|
||||
const capturedResponse = await invokeCanUseTool(
|
||||
session,
|
||||
"EnterPlanMode",
|
||||
{},
|
||||
);
|
||||
|
||||
expect(capturedResponse).toEqual({
|
||||
type: "control_response",
|
||||
response: {
|
||||
subtype: "success",
|
||||
request_id: "test-request-id",
|
||||
response: {
|
||||
behavior: "allow",
|
||||
updatedInput: null,
|
||||
updatedPermissions: [],
|
||||
},
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
test("denies AskUserQuestion without callback even in bypassPermissions", async () => {
|
||||
const session = new Session({
|
||||
permissionMode: "bypassPermissions",
|
||||
});
|
||||
|
||||
const capturedResponse = await invokeCanUseTool(
|
||||
session,
|
||||
"AskUserQuestion",
|
||||
{
|
||||
questions: [],
|
||||
},
|
||||
);
|
||||
|
||||
expect(capturedResponse).toEqual({
|
||||
type: "control_response",
|
||||
response: {
|
||||
subtype: "success",
|
||||
request_id: "test-request-id",
|
||||
response: {
|
||||
behavior: "deny",
|
||||
message: "No canUseTool callback registered",
|
||||
interrupt: false,
|
||||
},
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
test("uses canUseTool callback when provided and not bypassPermissions", async () => {
|
||||
const session = new Session({
|
||||
permissionMode: "default",
|
||||
@@ -95,22 +142,8 @@ describe("Session", () => {
|
||||
},
|
||||
});
|
||||
|
||||
// @ts-expect-error - accessing private method for testing
|
||||
const handleCanUseTool = session.handleCanUseTool.bind(session);
|
||||
|
||||
let capturedResponse: unknown;
|
||||
// @ts-expect-error - accessing private property for testing
|
||||
session.transport.write = async (msg: unknown) => {
|
||||
capturedResponse = msg;
|
||||
};
|
||||
|
||||
await handleCanUseTool("test-request-id", {
|
||||
subtype: "can_use_tool",
|
||||
tool_name: "Bash",
|
||||
tool_call_id: "test-tool-call-id",
|
||||
input: { command: "ls" },
|
||||
permission_suggestions: [],
|
||||
blocked_path: null,
|
||||
const capturedResponse = await invokeCanUseTool(session, "Bash", {
|
||||
command: "ls",
|
||||
});
|
||||
|
||||
// Verify callback was used and allowed
|
||||
|
||||
@@ -22,6 +22,10 @@ import type {
|
||||
AnyAgentTool,
|
||||
ExecuteExternalToolRequest,
|
||||
} from "./types.js";
|
||||
import {
|
||||
isHeadlessAutoAllowTool,
|
||||
requiresRuntimeUserInput,
|
||||
} from "./interactiveToolPolicy.js";
|
||||
|
||||
|
||||
// All logging gated behind DEBUG_SDK env var
|
||||
@@ -327,20 +331,35 @@ export class Session implements AsyncDisposable {
|
||||
req: CanUseToolControlRequest
|
||||
): Promise<void> {
|
||||
let response: CanUseToolResponse;
|
||||
const toolName = req.tool_name;
|
||||
const hasCallback = typeof this.options.canUseTool === "function";
|
||||
const toolNeedsRuntimeUserInput = requiresRuntimeUserInput(toolName);
|
||||
const autoAllowWithoutCallback =
|
||||
isHeadlessAutoAllowTool(toolName);
|
||||
|
||||
sessionLog("canUseTool", `tool=${req.tool_name} mode=${this.options.permissionMode || "default"} requestId=${requestId}`);
|
||||
sessionLog("canUseTool", `tool=${toolName} mode=${this.options.permissionMode || "default"} requestId=${requestId}`);
|
||||
|
||||
// If bypassPermissions mode, auto-allow all tools
|
||||
if (this.options.permissionMode === "bypassPermissions") {
|
||||
sessionLog("canUseTool", `AUTO-ALLOW ${req.tool_name} (bypassPermissions)`);
|
||||
// Tools that require runtime user input cannot be auto-allowed without a callback.
|
||||
if (toolNeedsRuntimeUserInput && !hasCallback) {
|
||||
response = {
|
||||
behavior: "deny",
|
||||
message: "No canUseTool callback registered",
|
||||
interrupt: false,
|
||||
};
|
||||
} else if (
|
||||
this.options.permissionMode === "bypassPermissions" &&
|
||||
!toolNeedsRuntimeUserInput
|
||||
) {
|
||||
// bypassPermissions auto-allows non-interactive tools.
|
||||
sessionLog("canUseTool", `AUTO-ALLOW ${toolName} (bypassPermissions)`);
|
||||
response = {
|
||||
behavior: "allow",
|
||||
updatedInput: null,
|
||||
updatedPermissions: [],
|
||||
} satisfies CanUseToolResponseAllow;
|
||||
} else if (this.options.canUseTool) {
|
||||
} else if (hasCallback) {
|
||||
try {
|
||||
const result = await this.options.canUseTool(req.tool_name, req.input);
|
||||
const result = await this.options.canUseTool!(toolName, req.input);
|
||||
if (result.behavior === "allow") {
|
||||
response = {
|
||||
behavior: "allow",
|
||||
@@ -361,6 +380,15 @@ export class Session implements AsyncDisposable {
|
||||
interrupt: false,
|
||||
};
|
||||
}
|
||||
} else if (autoAllowWithoutCallback) {
|
||||
// Default headless behavior matches Claude: EnterPlanMode can proceed
|
||||
// without requiring a callback in bidirectional mode.
|
||||
sessionLog("canUseTool", `AUTO-ALLOW ${toolName} (default behavior)`);
|
||||
response = {
|
||||
behavior: "allow",
|
||||
updatedInput: null,
|
||||
updatedPermissions: [],
|
||||
} satisfies CanUseToolResponseAllow;
|
||||
} else {
|
||||
// No callback registered - deny by default
|
||||
response = {
|
||||
@@ -381,7 +409,7 @@ export class Session implements AsyncDisposable {
|
||||
response,
|
||||
},
|
||||
});
|
||||
sessionLog("canUseTool", `response sent for ${req.tool_name}`);
|
||||
sessionLog("canUseTool", `response sent for ${toolName}`);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { createRequire } from "node:module";
|
||||
import { existsSync } from "node:fs";
|
||||
import { SubprocessTransport } from "./transport.js";
|
||||
|
||||
describe("CLI resolution", () => {
|
||||
test("resolves @letta-ai/letta-code via package main export", () => {
|
||||
@@ -23,3 +24,46 @@ describe("CLI resolution", () => {
|
||||
}).toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("transport args", () => {
|
||||
function buildArgsFor(options: {
|
||||
permissionMode?: "default" | "acceptEdits" | "plan" | "bypassPermissions";
|
||||
allowedTools?: string[];
|
||||
disallowedTools?: string[];
|
||||
} = {}): string[] {
|
||||
const transport = new SubprocessTransport(options);
|
||||
// Access private helper for deterministic argument testing.
|
||||
// biome-ignore lint/suspicious/noExplicitAny: test access to private method
|
||||
return (transport as any).buildArgs();
|
||||
}
|
||||
|
||||
test("acceptEdits uses --permission-mode acceptEdits", () => {
|
||||
const args = buildArgsFor({ permissionMode: "acceptEdits" });
|
||||
expect(args).toContain("--permission-mode");
|
||||
expect(args).toContain("acceptEdits");
|
||||
expect(args).not.toContain("--accept-edits");
|
||||
});
|
||||
|
||||
test("plan mode uses --permission-mode plan", () => {
|
||||
const args = buildArgsFor({ permissionMode: "plan" });
|
||||
expect(args).toContain("--permission-mode");
|
||||
expect(args).toContain("plan");
|
||||
});
|
||||
|
||||
test("bypassPermissions still uses --yolo alias", () => {
|
||||
const args = buildArgsFor({ permissionMode: "bypassPermissions" });
|
||||
expect(args).toContain("--yolo");
|
||||
expect(args).not.toContain("--permission-mode");
|
||||
});
|
||||
|
||||
test("allowedTools and disallowedTools are forwarded to CLI flags", () => {
|
||||
const args = buildArgsFor({
|
||||
allowedTools: ["Read", "Bash"],
|
||||
disallowedTools: ["EnterPlanMode", "ExitPlanMode"],
|
||||
});
|
||||
expect(args).toContain("--allowedTools");
|
||||
expect(args).toContain("Read,Bash");
|
||||
expect(args).toContain("--disallowedTools");
|
||||
expect(args).toContain("EnterPlanMode,ExitPlanMode");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -345,15 +345,22 @@ export class SubprocessTransport {
|
||||
|
||||
// Permission mode
|
||||
if (this.options.permissionMode === "bypassPermissions") {
|
||||
// Keep using alias for backwards compatibility
|
||||
args.push("--yolo");
|
||||
} else if (this.options.permissionMode === "acceptEdits") {
|
||||
args.push("--accept-edits");
|
||||
} else if (
|
||||
this.options.permissionMode &&
|
||||
this.options.permissionMode !== "default"
|
||||
) {
|
||||
args.push("--permission-mode", this.options.permissionMode);
|
||||
}
|
||||
|
||||
// Allowed tools
|
||||
if (this.options.allowedTools) {
|
||||
args.push("--allowedTools", this.options.allowedTools.join(","));
|
||||
}
|
||||
if (this.options.disallowedTools) {
|
||||
args.push("--disallowedTools", this.options.disallowedTools.join(","));
|
||||
}
|
||||
|
||||
return args;
|
||||
}
|
||||
|
||||
13
src/types.ts
13
src/types.ts
@@ -210,6 +210,7 @@ export interface InternalSessionOptions {
|
||||
|
||||
// Permissions
|
||||
allowedTools?: string[];
|
||||
disallowedTools?: string[];
|
||||
permissionMode?: PermissionMode;
|
||||
canUseTool?: CanUseToolCallback;
|
||||
|
||||
@@ -220,7 +221,11 @@ export interface InternalSessionOptions {
|
||||
cwd?: string;
|
||||
}
|
||||
|
||||
export type PermissionMode = "default" | "acceptEdits" | "bypassPermissions";
|
||||
export type PermissionMode =
|
||||
| "default"
|
||||
| "acceptEdits"
|
||||
| "plan"
|
||||
| "bypassPermissions";
|
||||
|
||||
/**
|
||||
* Options for createSession() and resumeSession() - restricted to options that can be applied to existing agents (LRU/Memo).
|
||||
@@ -236,6 +241,9 @@ export interface CreateSessionOptions {
|
||||
/** List of allowed tool names */
|
||||
allowedTools?: string[];
|
||||
|
||||
/** List of disallowed tool names */
|
||||
disallowedTools?: string[];
|
||||
|
||||
/** Permission mode */
|
||||
permissionMode?: PermissionMode;
|
||||
|
||||
@@ -287,6 +295,9 @@ export interface CreateAgentOptions {
|
||||
/** List of allowed tool names */
|
||||
allowedTools?: string[];
|
||||
|
||||
/** List of disallowed tool names */
|
||||
disallowedTools?: string[];
|
||||
|
||||
/** Permission mode */
|
||||
permissionMode?: PermissionMode;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user