Merge pull request #3621 from jeremymcs/fix/3616-db-tools-missing-subagent

fix(pi-coding-agent): restore extension tools after session switch (#3616)
This commit is contained in:
Jeremy McSpadden 2026-04-06 10:12:32 -05:00 committed by GitHub
commit 4aa7fe3940
5 changed files with 197 additions and 1 deletions

View file

@ -0,0 +1,64 @@
// GSD-2 — Regression tests for #3616: tool list persistence across newSession() calls
// Copyright (c) 2026 Jeremy McSpadden <jeremy@fluxlabs.net>
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",
);
});
});

View file

@ -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)

View file

@ -0,0 +1,42 @@
// GSD-2 — Regression test for #3616: reload() must reset jiti extension loader cache
// Copyright (c) 2026 Jeremy McSpadden <jeremy@fluxlabs.net>
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<void>");
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",
);
});
});

View file

@ -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<void> {
// 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,

View file

@ -0,0 +1,76 @@
// GSD-2 — Regression test for #3616: discuss tool scoping must not leak into subsequent sessions
// Copyright (c) 2026 Jeremy McSpadden <jeremy@fluxlabs.net>
/**
* 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}`,
);
});
});