diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 5db2fbd..5c2993b 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -6,6 +6,7 @@ updates: interval: "daily" allow: - dependency-name: "@letta-ai/letta-code" + versioning-strategy: "increase" labels: - "dependencies" commit-message: diff --git a/package.json b/package.json index 5b5601d..c98c6fd 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/src/interactiveToolPolicy.test.ts b/src/interactiveToolPolicy.test.ts new file mode 100644 index 0000000..11037c9 --- /dev/null +++ b/src/interactiveToolPolicy.test.ts @@ -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); + }); +}); diff --git a/src/interactiveToolPolicy.ts b/src/interactiveToolPolicy.ts new file mode 100644 index 0000000..5f0c5df --- /dev/null +++ b/src/interactiveToolPolicy.ts @@ -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); +} diff --git a/src/session.test.ts b/src/session.test.ts index 389a602..ed366a0 100644 --- a/src/session.test.ts +++ b/src/session.test.ts @@ -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, + ): Promise { // @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 diff --git a/src/session.ts b/src/session.ts index 3d99f4c..d066d76 100644 --- a/src/session.ts +++ b/src/session.ts @@ -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 { 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}`); } /** diff --git a/src/transport.test.ts b/src/transport.test.ts index 1267ef2..20218ce 100644 --- a/src/transport.test.ts +++ b/src/transport.test.ts @@ -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"); + }); +}); diff --git a/src/transport.ts b/src/transport.ts index d3f3949..5dc2d18 100644 --- a/src/transport.ts +++ b/src/transport.ts @@ -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; } diff --git a/src/types.ts b/src/types.ts index b6471f2..edbce21 100644 --- a/src/types.ts +++ b/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;