diff --git a/packages/pi-coding-agent/src/core/agent-session-tool-refresh.test.ts b/packages/pi-coding-agent/src/core/agent-session-tool-refresh.test.ts new file mode 100644 index 000000000..f1a14a15b --- /dev/null +++ b/packages/pi-coding-agent/src/core/agent-session-tool-refresh.test.ts @@ -0,0 +1,64 @@ +// GSD-2 — Regression tests for #3616: tool list persistence across newSession() calls +// Copyright (c) 2026 Jeremy McSpadden + +import test, { describe } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +const source = readFileSync( + join(process.cwd(), "packages/pi-coding-agent/src/core/agent-session.ts"), + "utf-8", +); + +describe("#3616 — newSession() must restore full tool set", () => { + test("newSession() calls _refreshToolRegistry with includeAllExtensionTools when cwd is unchanged", () => { + // Find the newSession method + const newSessionStart = source.indexOf("async newSession(options?:"); + assert.ok(newSessionStart >= 0, "should find newSession method"); + + // Get the method body (up to the next top-level method) + const methodBody = source.slice(newSessionStart, newSessionStart + 3000); + + // Verify the cwd-changed branch rebuilds tools + assert.ok( + methodBody.includes("if (this._cwd !== previousCwd)"), + "should have cwd-change guard", + ); + + // Verify the else branch exists and refreshes tools with includeAllExtensionTools + const elseIdx = methodBody.indexOf("} else {"); + assert.ok(elseIdx >= 0, "should have else branch for cwd-unchanged case"); + + const elseBranch = methodBody.slice(elseIdx, elseIdx + 800); + assert.ok( + elseBranch.includes("_refreshToolRegistry"), + "else branch should call _refreshToolRegistry", + ); + assert.ok( + elseBranch.includes("includeAllExtensionTools: true"), + "else branch should pass includeAllExtensionTools: true to restore narrowed tools", + ); + }); + + test("newSession() references #3616 in the else-branch comment", () => { + const idx = source.indexOf("#3616"); + assert.ok(idx >= 0, "source should reference issue #3616 for the tool restore fix"); + }); + + test("agent.reset() does not clear _state.tools (tools persist across reset)", () => { + // This is a structural invariant — if reset() starts clearing tools, + // the newSession() refresh becomes the only defense against tool loss. + const agentSource = readFileSync( + join(process.cwd(), "packages/pi-agent-core/src/agent.ts"), + "utf-8", + ); + const resetStart = agentSource.indexOf("reset()"); + assert.ok(resetStart >= 0, "should find reset() method"); + const resetBody = agentSource.slice(resetStart, resetStart + 400); + assert.ok( + !resetBody.includes("tools"), + "reset() should NOT touch _state.tools — tools are managed by agent-session", + ); + }); +}); diff --git a/packages/pi-coding-agent/src/core/agent-session.ts b/packages/pi-coding-agent/src/core/agent-session.ts index 0598c4575..5c5b6765a 100644 --- a/packages/pi-coding-agent/src/core/agent-session.ts +++ b/packages/pi-coding-agent/src/core/agent-session.ts @@ -1577,6 +1577,16 @@ export class AgentSession { activeToolNames: this.getActiveToolNames(), includeAllExtensionTools: true, }); + } else { + // Even when cwd hasn't changed, restore the full tool set (#3616). + // Extensions (e.g., discuss flows) may narrow the active tool list + // via setActiveTools() during a session. Without this refresh, the + // narrowed set persists into the next session — causing tools like + // gsd_plan_slice to be missing from auto-mode subagent sessions. + this._refreshToolRegistry({ + activeToolNames: this.getActiveToolNames(), + includeAllExtensionTools: true, + }); } // Run setup callback if provided (e.g., to append initial messages) diff --git a/packages/pi-coding-agent/src/core/resource-loader-cache-reset.test.ts b/packages/pi-coding-agent/src/core/resource-loader-cache-reset.test.ts new file mode 100644 index 000000000..f59c557a7 --- /dev/null +++ b/packages/pi-coding-agent/src/core/resource-loader-cache-reset.test.ts @@ -0,0 +1,42 @@ +// GSD-2 — Regression test for #3616: reload() must reset jiti extension loader cache +// Copyright (c) 2026 Jeremy McSpadden + +import test, { describe } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +const source = readFileSync( + join(process.cwd(), "packages/pi-coding-agent/src/core/resource-loader.ts"), + "utf-8", +); + +describe("#3616 — reload() must invalidate jiti module cache", () => { + test("resource-loader imports resetExtensionLoaderCache from loader.js", () => { + assert.ok( + source.includes("resetExtensionLoaderCache"), + "resource-loader.ts should import resetExtensionLoaderCache", + ); + assert.ok( + source.includes('from "./extensions/loader.js"'), + "resetExtensionLoaderCache should be imported from extensions/loader.js", + ); + }); + + test("reload() calls resetExtensionLoaderCache before loadExtensions", () => { + const reloadStart = source.indexOf("async reload(): Promise"); + assert.ok(reloadStart >= 0, "should find reload() method"); + const reloadBody = source.slice(reloadStart, reloadStart + 4000); + + const resetIdx = reloadBody.indexOf("resetExtensionLoaderCache()"); + assert.ok(resetIdx >= 0, "reload() should call resetExtensionLoaderCache()"); + + const loadIdx = reloadBody.indexOf("loadExtensions("); + assert.ok(loadIdx >= 0, "reload() should call loadExtensions"); + + assert.ok( + resetIdx < loadIdx, + "resetExtensionLoaderCache() must be called BEFORE loadExtensions to ensure fresh modules", + ); + }); +}); diff --git a/packages/pi-coding-agent/src/core/resource-loader.ts b/packages/pi-coding-agent/src/core/resource-loader.ts index 4daebde8a..34ab7565e 100644 --- a/packages/pi-coding-agent/src/core/resource-loader.ts +++ b/packages/pi-coding-agent/src/core/resource-loader.ts @@ -9,7 +9,7 @@ import type { ResourceCollision, ResourceDiagnostic } from "./diagnostics.js"; export type { ResourceCollision, ResourceDiagnostic } from "./diagnostics.js"; import { createEventBus, type EventBus } from "./event-bus.js"; -import { createExtensionRuntime, loadExtensionFromFactory, loadExtensions } from "./extensions/loader.js"; +import { createExtensionRuntime, loadExtensionFromFactory, loadExtensions, resetExtensionLoaderCache } from "./extensions/loader.js"; import type { Extension, ExtensionFactory, ExtensionRuntime, LoadExtensionsResult } from "./extensions/types.js"; import { DefaultPackageManager, type PathMetadata } from "./package-manager.js"; import type { PromptTemplate } from "./prompt-templates.js"; @@ -320,6 +320,10 @@ export class DefaultResourceLoader implements ResourceLoader { } async reload(): Promise { + // Invalidate the shared jiti module cache so updated extension code + // on disk is re-compiled instead of served from the stale cache (#3616). + resetExtensionLoaderCache(); + const resolvedPaths = await this.packageManager.resolve(); const cliExtensionPaths = await this.packageManager.resolveExtensionSources(this.additionalExtensionPaths, { temporary: true, diff --git a/src/resources/extensions/gsd/tests/discuss-tool-scope-leak.test.ts b/src/resources/extensions/gsd/tests/discuss-tool-scope-leak.test.ts new file mode 100644 index 000000000..caddc8d84 --- /dev/null +++ b/src/resources/extensions/gsd/tests/discuss-tool-scope-leak.test.ts @@ -0,0 +1,76 @@ +// GSD-2 — Regression test for #3616: discuss tool scoping must not leak into subsequent sessions +// Copyright (c) 2026 Jeremy McSpadden + +/** + * Bug #3616: After a discuss session narrows the active tool set via + * setActiveTools(), the narrowed list persisted into the next auto-mode + * session because newSession() did not restore extension tools when cwd + * was unchanged. This caused gsd_plan_slice and other DB tools to be + * missing from plan-slice subagent sessions. + * + * This test verifies the structural properties that prevent the leak: + * 1. guided-flow.ts narrows tools ONLY for discuss-* unit types + * 2. The narrowed set explicitly excludes gsd_plan_slice (a HEAVY_TOOL) + * 3. agent-session.ts:newSession() has an else-branch that restores + * all extension tools even when cwd hasn't changed + */ + +import { describe, test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; + +import { DISCUSS_TOOLS_ALLOWLIST } from "../constants.ts"; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const guidedFlowSource = readFileSync(join(__dirname, "..", "guided-flow.ts"), "utf-8"); + +describe("#3616 — discuss tool scoping must not leak across sessions", () => { + test("gsd_plan_slice is NOT in DISCUSS_TOOLS_ALLOWLIST", () => { + assert.ok( + !DISCUSS_TOOLS_ALLOWLIST.includes("gsd_plan_slice"), + "gsd_plan_slice should be excluded from discuss scope (it's a heavy planning tool)", + ); + }); + + test("tool scoping only activates for discuss-* unit types", () => { + // The guard must be: if (unitType?.startsWith("discuss-")) + assert.ok( + guidedFlowSource.includes('unitType?.startsWith("discuss-")'), + "tool scoping should only trigger for discuss-* unit types", + ); + }); + + test("discuss tool scoping uses setActiveTools (not setTools) for reversibility", () => { + // setActiveTools changes the active subset but doesn't remove tools from + // the registry. newSession()'s _refreshToolRegistry can restore them. + assert.ok( + guidedFlowSource.includes("pi.setActiveTools(scopedTools)"), + "should use pi.setActiveTools to narrow tools (preserving registry)", + ); + }); + + test("newSession() in agent-session.ts has defense against tool narrowing persistence", () => { + const agentSessionSource = readFileSync( + join(process.cwd(), "packages/pi-coding-agent/src/core/agent-session.ts"), + "utf-8", + ); + const newSessionStart = agentSessionSource.indexOf("async newSession(options?:"); + assert.ok(newSessionStart >= 0, "should find newSession"); + const body = agentSessionSource.slice(newSessionStart, newSessionStart + 3000); + + // Both branches (cwd-changed and cwd-unchanged) must include extension tools + assert.ok( + body.includes("includeAllExtensionTools: true"), + "newSession() must include all extension tools in both branches", + ); + + // Count occurrences — should be at least 2 (one per branch) + const matches = body.match(/includeAllExtensionTools:\s*true/g); + assert.ok( + matches && matches.length >= 2, + `expected >=2 includeAllExtensionTools:true in newSession(), got ${matches?.length ?? 0}`, + ); + }); +});