roboomp
fe34d0f392
Same shape of bug the reviewer flagged for custom tools: forwarding
`LoadExtensionsResult` from parent to subagent reused Extension instances
whose factories closed over the parent's `ExtensionAPI` — cwd, eventBus,
and runtime all pointed at the parent. Any tool/handler/command that
referenced `api.exec()`, `api.events`, or `api.runtime` still acted on the
parent session/worktree from inside an isolated subagent.
Forward only the path list; each session rebuilds extensions through
`loadExtensions` so factories see the right `ExtensionAPI`.
- `extensibility/extensions/loader.ts`: extract `discoverExtensionPaths`
(FS scan only) from `discoverAndLoadExtensions`. The combined helper now
composes the two. New export added to the package barrel.
- `sdk.ts`:
- Add `discoverSessionExtensionPaths()` (the `disableExtensionDiscovery`-aware
path-only counterpart of `loadSessionExtensions`).
- Add `preloadedExtensionPaths?: string[]` to `CreateAgentSessionOptions`.
Three loader branches: `preloadedExtensions` (CLI same-process reuse,
still shallow-cloned), `preloadedExtensionPaths` (subagent: skip scan,
reload locally), or full discovery.
- Document `preloadedExtensions` as same-process-only; subagent
forwarding MUST use `preloadedExtensionPaths`.
- `tools/index.ts`: `ToolSession.extensionsResult` → `extensionPaths:
string[]` for the same reason.
- `task/executor.ts` and `task/index.ts`: forward `extensionPaths`. Drop
the forward for the isolated `runSubprocess` branch — worktree cwd ≠
parent cwd, so the subagent re-discovers extensions against its own
tree.
- New `test/sdk-extensions-per-session-binding.test.ts` pins the contract:
two `loadExtensions` calls on the same path with different `cwd` and
different `EventBus` instances yield distinct Extension + runtime
objects whose factories close over the per-call bindings.
- Updated `executor-pass-through` and `sdk-preloaded-extensions-isolation`
tests for the new option name and comment context.
Refs PR review on #2193
79 lines
3.1 KiB
TypeScript
79 lines
3.1 KiB
TypeScript
/**
|
|
* Regression guard for issue #2190 / PR #2193 review.
|
|
*
|
|
* The CLI loads extensions early to parse custom flags, then hands the result
|
|
* back through `preloadedExtensions` so its OWN session can reuse the loaded
|
|
* instances without redoing the FS scan. `createAgentSession()` augments the
|
|
* result with inline extensions (autoresearch + custom-tools wrapper), so it
|
|
* MUST clone the caller's `extensions` array before mutating it — otherwise
|
|
* the caller's array accumulates session-local wrappers it never authored.
|
|
*
|
|
* Subagent forwarding is a separate path (`preloadedExtensionPaths`) which
|
|
* reloads extensions per session so each session's `ExtensionAPI` is its own.
|
|
*/
|
|
import { afterAll, beforeAll, describe, expect, it } from "bun:test";
|
|
import * as fs from "node:fs";
|
|
import * as os from "node:os";
|
|
import * as path from "node:path";
|
|
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
|
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
|
import type { LoadExtensionsResult } from "@oh-my-pi/pi-coding-agent/extensibility/extensions/types";
|
|
import { createAgentSession } from "@oh-my-pi/pi-coding-agent/sdk";
|
|
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
|
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
|
|
|
describe("createAgentSession preloadedExtensions isolation (issue #2190)", () => {
|
|
let sharedDir: string;
|
|
let authStorage: AuthStorage;
|
|
let modelRegistry: ModelRegistry;
|
|
|
|
beforeAll(async () => {
|
|
sharedDir = fs.mkdtempSync(path.join(os.tmpdir(), "pi-preloaded-ext-"));
|
|
authStorage = await AuthStorage.create(path.join(sharedDir, "auth.db"));
|
|
modelRegistry = new ModelRegistry(authStorage, path.join(sharedDir, "models.yml"));
|
|
});
|
|
|
|
afterAll(() => {
|
|
authStorage.close();
|
|
fs.rmSync(sharedDir, { recursive: true, force: true });
|
|
});
|
|
|
|
it("does not mutate the caller's extensions array when preloadedExtensions is provided", async () => {
|
|
const preloaded: LoadExtensionsResult = {
|
|
extensions: [],
|
|
errors: [],
|
|
runtime: {
|
|
flagValues: new Map(),
|
|
pendingProviderRegistrations: [],
|
|
// Cast: only the fields we touch matter; the SDK happily accepts a
|
|
// minimal runtime when no extension hooks fire.
|
|
} as unknown as LoadExtensionsResult["runtime"],
|
|
};
|
|
const beforeLength = preloaded.extensions.length;
|
|
const beforeArrayRef = preloaded.extensions;
|
|
|
|
await createAgentSession({
|
|
cwd: sharedDir,
|
|
agentDir: sharedDir,
|
|
sessionManager: SessionManager.inMemory(),
|
|
modelRegistry,
|
|
settings: Settings.isolated(),
|
|
preloadedExtensions: preloaded,
|
|
// Disable everything that would touch the network / FS scans.
|
|
enableLsp: false,
|
|
enableMCP: false,
|
|
skipPythonPreflight: true,
|
|
skills: [],
|
|
rules: [],
|
|
preloadedCustomToolPaths: [],
|
|
contextFiles: [],
|
|
promptTemplates: [],
|
|
});
|
|
|
|
// The session's own `extensionsResult` carries inline wrappers, but the
|
|
// caller's array (and its identity) must be untouched.
|
|
expect(preloaded.extensions).toBe(beforeArrayRef);
|
|
expect(preloaded.extensions.length).toBe(beforeLength);
|
|
});
|
|
});
|