singularity/singularity-forge
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix(extensions): use bundledExtensionKeys for conflict detection instead of broken path heuristic (#3305)

Browse source 
Fixes #2075

The isBuiltIn check in detectExtensionConflicts used a path-based
heuristic that excluded paths containing /.gsd/agent/extensions/.
Since initResources() syncs all bundled extensions into that same
directory, the heuristic could never return true for bundled extensions,
preventing the "supersedes" hint from being appended. This caused
downstream cli.ts to display "Extension load error" instead of the
intended "Extension conflict" for benign precedence collisions.

Replace the path heuristic with an explicit bundledExtensionKeys set
(already computed by buildResourceLoader) threaded through to
detectExtensionConflicts. The set is matched against the extension
directory name extracted from the owner path.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher 2026-04-05 01:04:50 -04:00 • committed by GitHub
parent 85ece7ea69
commit 34e73b01d1
3 changed files with 324 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

143
packages/pi-coding-agent/src/core/resource-loader.ts
View file

@ -1,6 +1,6 @@
import { existsSync, readdirSync, readFileSync, statSync } from "node:fs";
import { homedir } from "node:os";
import { basename, dirname, join, resolve, sep } from "node:path";
import { basename, dirname, join, relative, resolve, sep } from "node:path";
import chalk from "chalk";
import { CONFIG_DIR_NAME, getAgentDir } from "../config.js";
import { loadThemeFromPath, type Theme } from "../modes/interactive/theme/theme.js";
@ -121,6 +121,7 @@ export interface DefaultResourceLoaderOptions {
additionalPromptTemplatePaths?: string[];
additionalThemePaths?: string[];
extensionFactories?: ExtensionFactory[];
bundledExtensionKeys?: Set<string>;
noExtensions?: boolean;
noSkills?: boolean;
noPromptTemplates?: boolean;
@ -161,6 +162,7 @@ export class DefaultResourceLoader implements ResourceLoader {
private settingsManager: SettingsManager;
private eventBus: EventBus;
private packageManager: DefaultPackageManager;
private bundledExtensionKeys: Set<string>;
private additionalExtensionPaths: string[];
private additionalSkillPaths: string[];
private additionalPromptTemplatePaths: string[];
@ -218,6 +220,7 @@ export class DefaultResourceLoader implements ResourceLoader {
agentDir: this.agentDir,
settingsManager: this.settingsManager,
});
this.bundledExtensionKeys = options.bundledExtensionKeys ?? new Set();
this.additionalExtensionPaths = options.additionalExtensionPaths ?? [];
this.additionalSkillPaths = options.additionalSkillPaths ?? [];
this.additionalPromptTemplatePaths = options.additionalPromptTemplatePaths ?? [];
@ -827,66 +830,96 @@ export class DefaultResourceLoader implements ResourceLoader {
}
private detectExtensionConflicts(extensions: Extension[]): Array<{ path: string; message: string }> {
const conflicts: Array<{ path: string; message: string }> = [];
return detectExtensionConflicts(extensions, this.bundledExtensionKeys, join(this.agentDir, "extensions"));
}
}
// Track which extension registered each tool, command, and flag
const toolOwners = new Map<string, string>();
const commandOwners = new Map<string, string>();
const flagOwners = new Map<string, string>();
/**
* Extract the extension directory name (key) from a full extension path.
* Given extensionsDir `/home/user/.gsd/agent/extensions` and
* ownerPath `/home/user/.gsd/agent/extensions/mcp-client/index.js`,
* returns `"mcp-client"`. Returns `undefined` when the path is not
* under extensionsDir.
*/
export function extractExtensionKey(ownerPath: string, extensionsDir: string): string | undefined {
const normalizedDir = resolve(extensionsDir);
const normalizedPath = resolve(ownerPath);
const prefix = normalizedDir.endsWith(sep) ? normalizedDir : `${normalizedDir}${sep}`;
if (!normalizedPath.startsWith(prefix)) {
return undefined;
}
const relPath = relative(normalizedDir, normalizedPath);
const firstSegment = relPath.split(/[\\/]/)[0];
return firstSegment?.replace(/\.(?:ts|js)$/, "") || undefined;
}
for (const ext of extensions) {
// Check tools
for (const toolName of ext.tools.keys()) {
const existingOwner = toolOwners.get(toolName);
if (existingOwner && existingOwner !== ext.path) {
// Determine if the existing owner is a bundled extension by checking
// its name against the canonical bundled extensions list
const ownerName = this.getExtensionNameFromPath(existingOwner);
const isBuiltIn = this.bundledExtensionNames.has(ownerName);
const hint = isBuiltIn
? ` (built-in tool supersedes — consider removing ${ext.path})`
: "";
conflicts.push({
path: ext.path,
message: `Tool "${toolName}" conflicts with ${existingOwner}${hint}`,
});
} else {
toolOwners.set(toolName, ext.path);
}
}
/**
* Detect tool/command/flag name collisions across loaded extensions.
*
* When the first-registered owner of a name is a bundled extension
* (its key appears in `bundledExtensionKeys`), the conflict message
* includes a "supersedes" hint so downstream display can downgrade the
* severity from "Extension load error" to "Extension conflict".
*/
export function detectExtensionConflicts(
extensions: Extension[],
bundledExtensionKeys: Set<string>,
extensionsDir: string,
): Array<{ path: string; message: string }> {
const conflicts: Array<{ path: string; message: string }> = [];
// Check commands
for (const commandName of ext.commands.keys()) {
const existingOwner = commandOwners.get(commandName);
if (existingOwner && existingOwner !== ext.path) {
const ownerName = this.getExtensionNameFromPath(existingOwner);
const isBuiltIn = this.bundledExtensionNames.has(ownerName);
const hint = isBuiltIn
? ` (built-in command supersedes — consider removing ${ext.path})`
: "";
conflicts.push({
path: ext.path,
message: `Command "/${commandName}" conflicts with ${existingOwner}${hint}`,
});
} else {
commandOwners.set(commandName, ext.path);
}
}
const toolOwners = new Map<string, string>();
const commandOwners = new Map<string, string>();
const flagOwners = new Map<string, string>();
// Check flags
for (const flagName of ext.flags.keys()) {
const existingOwner = flagOwners.get(flagName);
if (existingOwner && existingOwner !== ext.path) {
conflicts.push({
path: ext.path,
message: `Flag "--${flagName}" conflicts with ${existingOwner}`,
});
} else {
flagOwners.set(flagName, ext.path);
}
const isBundled = (ownerPath: string): boolean => {
const key = extractExtensionKey(ownerPath, extensionsDir);
return key !== undefined && bundledExtensionKeys.has(key);
};
for (const ext of extensions) {
for (const toolName of ext.tools.keys()) {
const existingOwner = toolOwners.get(toolName);
if (existingOwner && existingOwner !== ext.path) {
const hint = isBundled(existingOwner)
? ` (built-in tool supersedes — consider removing ${ext.path})`
: "";
conflicts.push({
path: ext.path,
message: `Tool "${toolName}" conflicts with ${existingOwner}${hint}`,
});
} else {
toolOwners.set(toolName, ext.path);
}
}
return conflicts;
for (const commandName of ext.commands.keys()) {
const existingOwner = commandOwners.get(commandName);
if (existingOwner && existingOwner !== ext.path) {
const hint = isBundled(existingOwner)
? ` (built-in command supersedes — consider removing ${ext.path})`
: "";
conflicts.push({
path: ext.path,
message: `Command "/${commandName}" conflicts with ${existingOwner}${hint}`,
});
} else {
commandOwners.set(commandName, ext.path);
}
}
for (const flagName of ext.flags.keys()) {
const existingOwner = flagOwners.get(flagName);
if (existingOwner && existingOwner !== ext.path) {
conflicts.push({
path: ext.path,
message: `Flag "--${flagName}" conflicts with ${existingOwner}`,
});
} else {
flagOwners.set(flagName, ext.path);
}
}
}
return conflicts;
}

2
src/resource-loader.ts
View file

@ -606,7 +606,7 @@ export function buildResourceLoader(agentDir: string): DefaultResourceLoader {
return new DefaultResourceLoader({
agentDir,
additionalExtensionPaths: piExtensionPaths,
bundledExtensionNames: bundledKeys,
bundledExtensionKeys: bundledKeys,
extensionPathsTransform: (paths: string[]) => {
// 1. Filter community extensions through the GSD registry
const filteredPaths = paths.filter((entryPath) => {

235
src/tests/resource-loader-conflicts.test.ts Normal file
View file

@ -0,0 +1,235 @@
import { describe, it } from "node:test";
import assert from "node:assert/strict";
import { join, resolve, relative, sep } from "node:path";
// ─── Inline the pure functions under test to avoid import-chain issues ───────
// These are copied from packages/pi-coding-agent/src/core/resource-loader.ts
// (detectExtensionConflicts + extractExtensionKey). The test validates the
// algorithm; integration coverage lives in the full build tests.
interface MinimalExtension {
path: string;
tools: Map<string, unknown>;
commands: Map<string, unknown>;
flags: Map<string, unknown>;
}
function extractExtensionKey(ownerPath: string, extensionsDir: string): string | undefined {
const normalizedDir = resolve(extensionsDir);
const normalizedPath = resolve(ownerPath);
const prefix = normalizedDir.endsWith(sep) ? normalizedDir : `${normalizedDir}${sep}`;
if (!normalizedPath.startsWith(prefix)) {
return undefined;
}
const relPath = relative(normalizedDir, normalizedPath);
const firstSegment = relPath.split(/[\\/]/)[0];
return firstSegment?.replace(/\.(?:ts|js)$/, "") || undefined;
}
function detectExtensionConflicts(
extensions: MinimalExtension[],
bundledExtensionKeys: Set<string>,
extensionsDir: string,
): Array<{ path: string; message: string }> {
const conflicts: Array<{ path: string; message: string }> = [];
const toolOwners = new Map<string, string>();
const commandOwners = new Map<string, string>();
const flagOwners = new Map<string, string>();
const isBundled = (ownerPath: string): boolean => {
const key = extractExtensionKey(ownerPath, extensionsDir);
return key !== undefined && bundledExtensionKeys.has(key);
};
for (const ext of extensions) {
for (const toolName of ext.tools.keys()) {
const existingOwner = toolOwners.get(toolName);
if (existingOwner && existingOwner !== ext.path) {
const hint = isBundled(existingOwner)
? ` (built-in tool supersedes — consider removing ${ext.path})`
: "";
conflicts.push({ path: ext.path, message: `Tool "${toolName}" conflicts with ${existingOwner}${hint}` });
} else {
toolOwners.set(toolName, ext.path);
}
}
for (const commandName of ext.commands.keys()) {
const existingOwner = commandOwners.get(commandName);
if (existingOwner && existingOwner !== ext.path) {
const hint = isBundled(existingOwner)
? ` (built-in command supersedes — consider removing ${ext.path})`
: "";
conflicts.push({ path: ext.path, message: `Command "/${commandName}" conflicts with ${existingOwner}${hint}` });
} else {
commandOwners.set(commandName, ext.path);
}
}
for (const flagName of ext.flags.keys()) {
const existingOwner = flagOwners.get(flagName);
if (existingOwner && existingOwner !== ext.path) {
conflicts.push({ path: ext.path, message: `Flag "--${flagName}" conflicts with ${existingOwner}` });
} else {
flagOwners.set(flagName, ext.path);
}
}
}
return conflicts;
}
// ─── helpers ──────────────────────────────────────────────────────────────────
function makeExtension(
path: string,
overrides: { tools?: string[]; commands?: string[]; flags?: string[] } = {},
): MinimalExtension {
const tools = new Map<string, unknown>();
for (const name of overrides.tools ?? []) tools.set(name, {});
const commands = new Map<string, unknown>();
for (const name of overrides.commands ?? []) commands.set(name, {});
const flags = new Map<string, unknown>();
for (const name of overrides.flags ?? []) flags.set(name, {});
return { path, tools, commands, flags };
}
// ─── extractExtensionKey ─────────────────────────────────────────────────────
describe("extractExtensionKey", () => {
const extensionsDir = "/home/user/.gsd/agent/extensions";
it("extracts directory name from a nested extension path", () => {
assert.equal(
extractExtensionKey("/home/user/.gsd/agent/extensions/mcp-client/index.js", extensionsDir),
"mcp-client",
);
});
it("strips .ts/.js suffix from flat extension files", () => {
assert.equal(
extractExtensionKey("/home/user/.gsd/agent/extensions/my-ext.ts", extensionsDir),
"my-ext",
);
});
it("returns undefined when the path is not under extensionsDir", () => {
assert.equal(
extractExtensionKey("/other/path/some-ext/index.js", extensionsDir),
undefined,
);
});
});
// ─── detectExtensionConflicts ─────────────────────────────────────────────────
describe("detectExtensionConflicts", () => {
const extensionsDir = "/home/user/.gsd/agent/extensions";
it("returns no conflicts when extensions have unique tool names", () => {
const extensions = [
makeExtension(join(extensionsDir, "ext-a/index.js"), { tools: ["tool_a"] }),
makeExtension(join(extensionsDir, "ext-b/index.js"), { tools: ["tool_b"] }),
];
const conflicts = detectExtensionConflicts(extensions, new Set(["ext-a"]), extensionsDir);
assert.equal(conflicts.length, 0);
});
it("adds supersedes hint when first-registered tool owner is a bundled extension", () => {
const bundledPath = join(extensionsDir, "mcp-client/index.js");
const userPath = join(extensionsDir, "mcporter/index.ts");
const extensions = [
makeExtension(bundledPath, { tools: ["mcp_servers"] }),
makeExtension(userPath, { tools: ["mcp_servers"] }),
];
const conflicts = detectExtensionConflicts(extensions, new Set(["mcp-client"]), extensionsDir);
assert.equal(conflicts.length, 1);
assert.ok(
conflicts[0].message.includes("supersedes"),
`Expected "supersedes" in message, got: ${conflicts[0].message}`,
);
assert.equal(conflicts[0].path, userPath);
});
it("omits supersedes hint when first-registered tool owner is NOT bundled", () => {
const userPathA = join(extensionsDir, "mcporter/index.ts");
const userPathB = join(extensionsDir, "mcporter-v2/index.ts");
const extensions = [
makeExtension(userPathA, { tools: ["mcp_servers"] }),
makeExtension(userPathB, { tools: ["mcp_servers"] }),
];
const conflicts = detectExtensionConflicts(extensions, new Set(["mcp-client"]), extensionsDir);
assert.equal(conflicts.length, 1);
assert.ok(
!conflicts[0].message.includes("supersedes"),
`Expected no "supersedes" in message, got: ${conflicts[0].message}`,
);
});
it("adds supersedes hint for command conflicts with bundled extensions", () => {
const bundledPath = join(extensionsDir, "mcp-client/index.js");
const userPath = join(extensionsDir, "mcporter/index.ts");
const extensions = [
makeExtension(bundledPath, { commands: ["mcp"] }),
makeExtension(userPath, { commands: ["mcp"] }),
];
const conflicts = detectExtensionConflicts(extensions, new Set(["mcp-client"]), extensionsDir);
assert.equal(conflicts.length, 1);
assert.ok(
conflicts[0].message.includes("supersedes"),
`Expected "supersedes" in command conflict, got: ${conflicts[0].message}`,
);
});
it("works with an empty bundledExtensionKeys set (backwards compat)", () => {
const pathA = join(extensionsDir, "ext-a/index.js");
const pathB = join(extensionsDir, "ext-b/index.js");
const extensions = [
makeExtension(pathA, { tools: ["shared_tool"] }),
makeExtension(pathB, { tools: ["shared_tool"] }),
];
const conflicts = detectExtensionConflicts(extensions, new Set(), extensionsDir);
assert.equal(conflicts.length, 1);
assert.ok(
!conflicts[0].message.includes("supersedes"),
`Expected no "supersedes" when bundledKeys empty, got: ${conflicts[0].message}`,
);
});
it("reproduces issue #2075: bundled extension under /.gsd/agent/extensions/ was never identified as built-in", () => {
// Before the fix, the isBuiltIn check used path heuristics that excluded
// paths containing /.gsd/agent/extensions/, so bundled extensions placed
// there by initResources() could never be recognized as built-in.
const bundledPath = "/home/user/.gsd/agent/extensions/mcp-client/index.js";
const userPath = "/home/user/.gsd/agent/extensions/mcporter/index.ts";
const extensions = [
makeExtension(bundledPath, { tools: ["mcp_servers", "mcp_discover", "mcp_call"] }),
makeExtension(userPath, { tools: ["mcp_servers", "mcp_discover", "mcp_call"] }),
];
const bundledKeys = new Set(["mcp-client"]);
const conflicts = detectExtensionConflicts(extensions, bundledKeys, "/home/user/.gsd/agent/extensions");
// All three conflicting tools should include the supersedes hint
assert.equal(conflicts.length, 3);
for (const conflict of conflicts) {
assert.ok(
conflict.message.includes("supersedes"),
`Conflict for tool should include "supersedes" hint, got: ${conflict.message}`,
);
}
});
});