fix(pi-coding-agent): restore extension tools after session switch (#3616)
newSession() only rebuilt the tool registry when cwd changed. When cwd stayed the same (e.g., discuss → plan-slice in the same worktree), any tool narrowing from setActiveTools() persisted — stripping gsd_plan_slice and other DB tools from auto-mode subagent sessions. Add an else-branch that calls _refreshToolRegistry with includeAllExtensionTools:true on every session switch, regardless of cwd. Also call resetExtensionLoaderCache() in DefaultResourceLoader.reload() so hot-updated extension code on disk is re-compiled instead of served from the stale jiti module cache. Closes #3616
This commit is contained in:
parent
6dfc422990
commit
90feebeccf
5 changed files with 197 additions and 1 deletions
|
|
@ -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",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue