From fdd46bd971a65c4eb85fff25ebdf3716ef3696e8 Mon Sep 17 00:00:00 2001 From: Slava Zavadsky Date: Tue, 28 Jul 2026 21:18:43 -0400 Subject: [PATCH] fix(coding-agent): pair checkpoint/rewind for restricted sessions too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The !restrictToolNames guard on the pairing blocks was wrong: a restricted session with tools:[checkpoint] passes isToolAllowed (requestedTools is defined) but the pairing is skipped, stranding the agent without rewind. Remove the guard — this is a safety pairing, not a convenience widening. Added restricted-session tests in both createTools and SDK active-set paths. --- packages/coding-agent/src/sdk.ts | 6 ++++-- packages/coding-agent/src/tools/index.ts | 5 +++-- .../test/sdk-autolearn-active-tools.test.ts | 19 +++++++++++++++++++ .../coding-agent/test/tools/index.test.ts | 15 +++++++++++++++ 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 995e3e2d9..4134f5168 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -2796,8 +2796,10 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} // Checkpoint and rewind are a pair: `createTools` auto-includes the sister // tool in the registry, but an explicit `toolNames` list would otherwise // drop it from the ACTIVE set — leaving the agent able to checkpoint but - // unable to rewind (or vice versa). Mirror the pairing here. - if (!restrictToolNames && explicitlyRequestedToolNames) { + // unable to rewind (or vice versa). Mirror the pairing here. Unlike the + // manage_skill/learn mirror above, this is a safety pairing — it applies + // to restricted sessions too. + if (explicitlyRequestedToolNames) { if (builtInToolNames.includes("checkpoint") && !explicitlyRequestedToolNames.includes("rewind")) { explicitlyRequestedToolNames.push("rewind"); } else if (builtInToolNames.includes("rewind") && !explicitlyRequestedToolNames.includes("checkpoint")) { diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts index 6d62140a5..3155ae642 100644 --- a/packages/coding-agent/src/tools/index.ts +++ b/packages/coding-agent/src/tools/index.ts @@ -505,8 +505,9 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P // Checkpoint and rewind are a pair: listing one without the other strands // the agent (it can checkpoint but not rewind, or vice versa). Auto-include // the sister tool so a one-sided frontmatter `tools:` entry still works. - // Like the AST/auto-learn siblings below, restricted callers own the list. - if (requestedTools && !restrictToolNames && session.settings.get("checkpoint.enabled")) { + // Unlike the AST/auto-learn convenience auto-includes below, this is a + // safety pairing — it applies to restricted sessions too. + if (requestedTools && session.settings.get("checkpoint.enabled")) { if (requestedTools.includes("checkpoint") && !requestedTools.includes("rewind")) { requestedTools.push("rewind"); } else if (requestedTools.includes("rewind") && !requestedTools.includes("checkpoint")) { diff --git a/packages/coding-agent/test/sdk-autolearn-active-tools.test.ts b/packages/coding-agent/test/sdk-autolearn-active-tools.test.ts index f00a9c231..6dd520107 100644 --- a/packages/coding-agent/test/sdk-autolearn-active-tools.test.ts +++ b/packages/coding-agent/test/sdk-autolearn-active-tools.test.ts @@ -122,4 +122,23 @@ describe("createAgentSession auto-learn tool activation", () => { expect(names).toContain("checkpoint"); expect(names).toContain("rewind"); }); + + it("activates checkpoint and rewind in a restricted session with one-sided toolNames", async () => { + const { session } = await createAgentSession({ + cwd: registryDir, + agentDir: registryDir, + modelRegistry, + sessionManager: SessionManager.inMemory(), + settings: Settings.isolated({ "checkpoint.enabled": true }), + model: getBundledModel("openai", "gpt-4o-mini"), + disableExtensionDiscovery: true, + toolNames: ["checkpoint"], + requireYieldTool: true, + restrictToolNames: true, + }); + sessions.push(session); + const names = session.getActiveToolNames(); + expect(names).toContain("checkpoint"); + expect(names).toContain("rewind"); + }); }); diff --git a/packages/coding-agent/test/tools/index.test.ts b/packages/coding-agent/test/tools/index.test.ts index 7eeb76cd2..e7700146f 100644 --- a/packages/coding-agent/test/tools/index.test.ts +++ b/packages/coding-agent/test/tools/index.test.ts @@ -397,6 +397,21 @@ describe("createTools", () => { expect(names).not.toContain("rewind"); }); + it("auto-pairs checkpoint/rewind in a restricted subagent with one-sided list", async () => { + const names = ( + await createTools( + createTestSession({ + taskDepth: 1, + restrictToolNames: true, + settings: createSettingsWithOverrides({ "checkpoint.enabled": true }), + }), + ["checkpoint"], + ) + ).map(t => t.name); + expect(names).toContain("checkpoint"); + expect(names).toContain("rewind"); + }); + it("HIDDEN_TOOLS contains yield and goal", () => { expect(Object.keys(HIDDEN_TOOLS).sort()).toEqual(["goal", "yield"]); });