feat(coding-agent): progress callbacks, conflict detection, URL parsing, tests (#645)

- Add progress callbacks to PackageManager for TUI status during install/remove/update
- Add extension conflict detection (tools, commands, flags with same names)
- Accept raw GitHub/GitLab URLs without git: prefix
- Add tests for package-manager.ts and resource-loader.ts
- Add empty fixture directories for skills tests
This commit is contained in:
Mario Zechner 2026-01-20 23:44:49 +01:00
parent b846a4bfcf
commit 4058680d22
8 changed files with 548 additions and 25 deletions

View file

@ -15,6 +15,15 @@ export interface ResolvedPaths {
export type MissingSourceAction = "install" | "skip" | "error";
export interface ProgressEvent {
type: "start" | "progress" | "complete" | "error";
action: "install" | "remove" | "update" | "clone" | "pull";
source: string;
message?: string;
}
export type ProgressCallback = (event: ProgressEvent) => void;
export interface PackageManager {
resolve(onMissing?: (source: string) => Promise<MissingSourceAction>): Promise<ResolvedPaths>;
install(source: string, options?: { local?: boolean }): Promise<void>;
@ -24,6 +33,7 @@ export interface PackageManager {
sources: string[],
options?: { local?: boolean; temporary?: boolean },
): Promise<ResolvedPaths>;
setProgressCallback(callback: ProgressCallback | undefined): void;
}
interface PackageManagerOptions {
@ -76,6 +86,7 @@ export class DefaultPackageManager implements PackageManager {
private agentDir: string;
private settingsManager: SettingsManager;
private globalNpmRoot: string | undefined;
private progressCallback: ProgressCallback | undefined;
constructor(options: PackageManagerOptions) {
this.cwd = options.cwd;
@ -83,6 +94,14 @@ export class DefaultPackageManager implements PackageManager {
this.settingsManager = options.settingsManager;
}
setProgressCallback(callback: ProgressCallback | undefined): void {
this.progressCallback = callback;
}
private emitProgress(event: ProgressEvent): void {
this.progressCallback?.(event);
}
async resolve(onMissing?: (source: string) => Promise<MissingSourceAction>): Promise<ResolvedPaths> {
const accumulator = this.createAccumulator();
const globalSettings = this.settingsManager.getGlobalSettings();
@ -134,29 +153,47 @@ export class DefaultPackageManager implements PackageManager {
async install(source: string, options?: { local?: boolean }): Promise<void> {
const parsed = this.parseSource(source);
const scope: SourceScope = options?.local ? "project" : "global";
if (parsed.type === "npm") {
await this.installNpm(parsed, scope, false);
return;
this.emitProgress({ type: "start", action: "install", source, message: `Installing ${source}...` });
try {
if (parsed.type === "npm") {
await this.installNpm(parsed, scope, false);
this.emitProgress({ type: "complete", action: "install", source });
return;
}
if (parsed.type === "git") {
await this.installGit(parsed, scope, false);
this.emitProgress({ type: "complete", action: "install", source });
return;
}
throw new Error(`Unsupported install source: ${source}`);
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
this.emitProgress({ type: "error", action: "install", source, message });
throw error;
}
if (parsed.type === "git") {
await this.installGit(parsed, scope, false);
return;
}
throw new Error(`Unsupported install source: ${source}`);
}
async remove(source: string, options?: { local?: boolean }): Promise<void> {
const parsed = this.parseSource(source);
const scope: SourceScope = options?.local ? "project" : "global";
if (parsed.type === "npm") {
await this.uninstallNpm(parsed, scope);
return;
this.emitProgress({ type: "start", action: "remove", source, message: `Removing ${source}...` });
try {
if (parsed.type === "npm") {
await this.uninstallNpm(parsed, scope);
this.emitProgress({ type: "complete", action: "remove", source });
return;
}
if (parsed.type === "git") {
await this.removeGit(parsed, scope, false);
this.emitProgress({ type: "complete", action: "remove", source });
return;
}
throw new Error(`Unsupported remove source: ${source}`);
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
this.emitProgress({ type: "error", action: "remove", source, message });
throw error;
}
if (parsed.type === "git") {
await this.removeGit(parsed, scope, false);
return;
}
throw new Error(`Unsupported remove source: ${source}`);
}
async update(source?: string): Promise<void> {
@ -180,12 +217,28 @@ export class DefaultPackageManager implements PackageManager {
const parsed = this.parseSource(source);
if (parsed.type === "npm") {
if (parsed.pinned) return;
await this.installNpm(parsed, scope, false);
this.emitProgress({ type: "start", action: "update", source, message: `Updating ${source}...` });
try {
await this.installNpm(parsed, scope, false);
this.emitProgress({ type: "complete", action: "update", source });
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
this.emitProgress({ type: "error", action: "update", source, message });
throw error;
}
return;
}
if (parsed.type === "git") {
if (parsed.pinned) return;
await this.updateGit(parsed, scope, false);
this.emitProgress({ type: "start", action: "update", source, message: `Updating ${source}...` });
try {
await this.updateGit(parsed, scope, false);
this.emitProgress({ type: "complete", action: "update", source });
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
this.emitProgress({ type: "error", action: "update", source, message });
throw error;
}
return;
}
}
@ -281,10 +334,11 @@ export class DefaultPackageManager implements PackageManager {
};
}
if (source.startsWith("git:")) {
const repoSpec = source.slice("git:".length).trim();
// Accept git: prefix or raw URLs (https://github.com/..., github.com/...)
if (source.startsWith("git:") || this.looksLikeGitUrl(source)) {
const repoSpec = source.startsWith("git:") ? source.slice("git:".length).trim() : source;
const [repo, ref] = repoSpec.split("@");
const normalized = repo.replace(/^https?:\/\//, "");
const normalized = repo.replace(/^https?:\/\//, "").replace(/\.git$/, "");
const parts = normalized.split("/");
const host = parts.shift() ?? "";
const repoPath = parts.join("/");
@ -301,6 +355,13 @@ export class DefaultPackageManager implements PackageManager {
return { type: "local", path: source };
}
private looksLikeGitUrl(source: string): boolean {
// Match URLs like https://github.com/..., github.com/..., gitlab.com/...
const gitHosts = ["github.com", "gitlab.com", "bitbucket.org", "codeberg.org"];
const normalized = source.replace(/^https?:\/\//, "");
return gitHosts.some((host) => normalized.startsWith(`${host}/`));
}
private parseNpmSpec(spec: string): { name: string; version?: string } {
const match = spec.match(/^(@?[^@]+(?:\/[^@]+)?)(?:@(.+))?$/);
if (!match) {

View file

@ -280,6 +280,13 @@ export class DefaultResourceLoader implements ResourceLoader {
const inlineExtensions = await this.loadExtensionFactories(extensionsResult.runtime);
extensionsResult.extensions.push(...inlineExtensions.extensions);
extensionsResult.errors.push(...inlineExtensions.errors);
// Detect extension conflicts (tools, commands, flags with same names from different extensions)
const conflicts = this.detectExtensionConflicts(extensionsResult.extensions);
for (const conflict of conflicts) {
extensionsResult.errors.push({ path: conflict.path, error: conflict.message });
}
this.extensionsResult = this.extensionsOverride ? this.extensionsOverride(extensionsResult) : extensionsResult;
const skillPaths = this.noSkills
@ -513,4 +520,56 @@ export class DefaultResourceLoader implements ResourceLoader {
return undefined;
}
private detectExtensionConflicts(extensions: Extension[]): Array<{ path: string; message: string }> {
const conflicts: Array<{ path: string; message: string }> = [];
// 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>();
for (const ext of extensions) {
// Check tools
for (const toolName of ext.tools.keys()) {
const existingOwner = toolOwners.get(toolName);
if (existingOwner && existingOwner !== ext.path) {
conflicts.push({
path: ext.path,
message: `Tool "${toolName}" conflicts with ${existingOwner}`,
});
} else {
toolOwners.set(toolName, ext.path);
}
}
// Check commands
for (const commandName of ext.commands.keys()) {
const existingOwner = commandOwners.get(commandName);
if (existingOwner && existingOwner !== ext.path) {
conflicts.push({
path: ext.path,
message: `Command "/${commandName}" conflicts with ${existingOwner}`,
});
} else {
commandOwners.set(commandName, ext.path);
}
}
// 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);
}
}
}
return conflicts;
}
}

View file

@ -116,6 +116,8 @@ export {
export type { ReadonlyFooterDataProvider } from "./core/footer-data-provider.js";
export { convertToLlm } from "./core/messages.js";
export { ModelRegistry } from "./core/model-registry.js";
export type { PackageManager, ProgressCallback, ProgressEvent } from "./core/package-manager.js";
export { DefaultPackageManager } from "./core/package-manager.js";
export type { ResourceDiagnostic, ResourceLoader } from "./core/resource-loader.js";
export { DefaultResourceLoader } from "./core/resource-loader.js";
// SDK for programmatic usage

View file

@ -134,6 +134,15 @@ async function handlePackageCommand(args: string[]): Promise<boolean> {
const settingsManager = SettingsManager.create(cwd, agentDir);
const packageManager = new DefaultPackageManager({ cwd, agentDir, settingsManager });
// Set up progress callback for CLI feedback
packageManager.setProgressCallback((event) => {
if (event.type === "start") {
process.stdout.write(chalk.dim(`${event.message}\n`));
} else if (event.type === "error") {
console.error(chalk.red(`Error: ${event.message}`));
}
});
if (options.command === "install") {
if (!options.source) {
console.error(chalk.red("Missing install source."));
@ -141,7 +150,7 @@ async function handlePackageCommand(args: string[]): Promise<boolean> {
}
await packageManager.install(options.source, { local: options.local });
updateExtensionSources(settingsManager, options.source, options.local, "add");
console.log(`Installed ${options.source}`);
console.log(chalk.green(`Installed ${options.source}`));
return true;
}
@ -152,15 +161,15 @@ async function handlePackageCommand(args: string[]): Promise<boolean> {
}
await packageManager.remove(options.source, { local: options.local });
updateExtensionSources(settingsManager, options.source, options.local, "remove");
console.log(`Removed ${options.source}`);
console.log(chalk.green(`Removed ${options.source}`));
return true;
}
await packageManager.update(options.source);
if (options.source) {
console.log(`Updated ${options.source}`);
console.log(chalk.green(`Updated ${options.source}`));
} else {
console.log("Updated extensions");
console.log(chalk.green("Updated extensions"));
}
return true;
}

View file

View file

@ -0,0 +1,161 @@
import { mkdirSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { DefaultPackageManager, type ProgressEvent } from "../src/core/package-manager.js";
import { SettingsManager } from "../src/core/settings-manager.js";
describe("DefaultPackageManager", () => {
let tempDir: string;
let settingsManager: SettingsManager;
let packageManager: DefaultPackageManager;
beforeEach(() => {
tempDir = join(tmpdir(), `pm-test-${Date.now()}-${Math.random().toString(36).slice(2)}`);
mkdirSync(tempDir, { recursive: true });
const agentDir = join(tempDir, "agent");
mkdirSync(agentDir, { recursive: true });
settingsManager = SettingsManager.inMemory();
packageManager = new DefaultPackageManager({
cwd: tempDir,
agentDir,
settingsManager,
});
});
afterEach(() => {
rmSync(tempDir, { recursive: true, force: true });
});
describe("resolve", () => {
it("should return empty paths when no sources configured", async () => {
const result = await packageManager.resolve();
expect(result.extensions).toEqual([]);
expect(result.skills).toEqual([]);
expect(result.prompts).toEqual([]);
expect(result.themes).toEqual([]);
});
it("should resolve local extension paths from settings", async () => {
const extPath = join(tempDir, "my-extension.ts");
writeFileSync(extPath, "export default function() {}");
settingsManager.setExtensionPaths([extPath]);
const result = await packageManager.resolve();
expect(result.extensions).toContain(extPath);
});
it("should resolve skill paths from settings", async () => {
const skillDir = join(tempDir, "skills");
mkdirSync(skillDir, { recursive: true });
writeFileSync(
join(skillDir, "SKILL.md"),
`---
name: test-skill
description: A test skill
---
Content`,
);
settingsManager.setSkillPaths([skillDir]);
const result = await packageManager.resolve();
expect(result.skills).toContain(skillDir);
});
});
describe("resolveExtensionSources", () => {
it("should resolve local paths", async () => {
const extPath = join(tempDir, "ext.ts");
writeFileSync(extPath, "export default function() {}");
const result = await packageManager.resolveExtensionSources([extPath]);
expect(result.extensions).toContain(extPath);
});
it("should handle directories with pi manifest", async () => {
const pkgDir = join(tempDir, "my-package");
mkdirSync(pkgDir, { recursive: true });
writeFileSync(
join(pkgDir, "package.json"),
JSON.stringify({
name: "my-package",
pi: {
extensions: ["./src/index.ts"],
skills: ["./skills"],
},
}),
);
mkdirSync(join(pkgDir, "src"), { recursive: true });
writeFileSync(join(pkgDir, "src", "index.ts"), "export default function() {}");
mkdirSync(join(pkgDir, "skills"), { recursive: true });
const result = await packageManager.resolveExtensionSources([pkgDir]);
expect(result.extensions).toContain(join(pkgDir, "src", "index.ts"));
expect(result.skills).toContain(join(pkgDir, "skills"));
});
it("should handle directories with auto-discovery layout", async () => {
const pkgDir = join(tempDir, "auto-pkg");
mkdirSync(join(pkgDir, "extensions"), { recursive: true });
mkdirSync(join(pkgDir, "themes"), { recursive: true });
writeFileSync(join(pkgDir, "extensions", "main.ts"), "export default function() {}");
writeFileSync(join(pkgDir, "themes", "dark.json"), "{}");
const result = await packageManager.resolveExtensionSources([pkgDir]);
expect(result.extensions).toContain(join(pkgDir, "extensions"));
expect(result.themes).toContain(join(pkgDir, "themes"));
});
});
describe("progress callback", () => {
it("should emit progress events", async () => {
const events: ProgressEvent[] = [];
packageManager.setProgressCallback((event) => events.push(event));
const extPath = join(tempDir, "ext.ts");
writeFileSync(extPath, "export default function() {}");
// Local paths don't trigger install progress, but we can verify the callback is set
await packageManager.resolveExtensionSources([extPath]);
// For now just verify no errors - npm/git would trigger actual events
expect(events.length).toBe(0);
});
});
describe("source parsing", () => {
it("should emit progress events on install attempt", async () => {
const events: ProgressEvent[] = [];
packageManager.setProgressCallback((event) => events.push(event));
// Use public install method which emits progress events
try {
await packageManager.install("npm:nonexistent-package@1.0.0");
} catch {
// Expected to fail - package doesn't exist
}
// Should have emitted start event before failure
expect(events.some((e) => e.type === "start" && e.action === "install")).toBe(true);
// Should have emitted error event
expect(events.some((e) => e.type === "error")).toBe(true);
});
it("should recognize github URLs without git: prefix", async () => {
const events: ProgressEvent[] = [];
packageManager.setProgressCallback((event) => events.push(event));
// This should be parsed as a git source, not throw "unsupported"
try {
await packageManager.install("https://github.com/nonexistent/repo");
} catch {
// Expected to fail - repo doesn't exist
}
// Should have attempted clone, not thrown unsupported error
expect(events.some((e) => e.type === "start" && e.action === "install")).toBe(true);
});
});
});

View file

@ -0,0 +1,231 @@
import { mkdirSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { DefaultResourceLoader } from "../src/core/resource-loader.js";
import type { Skill } from "../src/core/skills.js";
describe("DefaultResourceLoader", () => {
let tempDir: string;
let agentDir: string;
let cwd: string;
beforeEach(() => {
tempDir = join(tmpdir(), `rl-test-${Date.now()}-${Math.random().toString(36).slice(2)}`);
agentDir = join(tempDir, "agent");
cwd = join(tempDir, "project");
mkdirSync(agentDir, { recursive: true });
mkdirSync(cwd, { recursive: true });
});
afterEach(() => {
rmSync(tempDir, { recursive: true, force: true });
});
describe("reload", () => {
it("should initialize with empty results before reload", () => {
const loader = new DefaultResourceLoader({ cwd, agentDir });
expect(loader.getExtensions().extensions).toEqual([]);
expect(loader.getSkills().skills).toEqual([]);
expect(loader.getPrompts().prompts).toEqual([]);
expect(loader.getThemes().themes).toEqual([]);
});
it("should discover skills from agentDir", async () => {
const skillsDir = join(agentDir, "skills");
mkdirSync(skillsDir, { recursive: true });
writeFileSync(
join(skillsDir, "test-skill.md"),
`---
name: test-skill
description: A test skill
---
Skill content here.`,
);
const loader = new DefaultResourceLoader({ cwd, agentDir });
await loader.reload();
const { skills } = loader.getSkills();
expect(skills.some((s) => s.name === "test-skill")).toBe(true);
});
it("should discover prompts from agentDir", async () => {
const promptsDir = join(agentDir, "prompts");
mkdirSync(promptsDir, { recursive: true });
writeFileSync(
join(promptsDir, "test-prompt.md"),
`---
description: A test prompt
---
Prompt content.`,
);
const loader = new DefaultResourceLoader({ cwd, agentDir });
await loader.reload();
const { prompts } = loader.getPrompts();
expect(prompts.some((p) => p.name === "test-prompt")).toBe(true);
});
it("should discover AGENTS.md context files", async () => {
writeFileSync(join(cwd, "AGENTS.md"), "# Project Guidelines\n\nBe helpful.");
const loader = new DefaultResourceLoader({ cwd, agentDir });
await loader.reload();
const { agentsFiles } = loader.getAgentsFiles();
expect(agentsFiles.some((f) => f.path.includes("AGENTS.md"))).toBe(true);
});
it("should discover SYSTEM.md from cwd/.pi", async () => {
const piDir = join(cwd, ".pi");
mkdirSync(piDir, { recursive: true });
writeFileSync(join(piDir, "SYSTEM.md"), "You are a helpful assistant.");
const loader = new DefaultResourceLoader({ cwd, agentDir });
await loader.reload();
expect(loader.getSystemPrompt()).toBe("You are a helpful assistant.");
});
it("should discover APPEND_SYSTEM.md", async () => {
const piDir = join(cwd, ".pi");
mkdirSync(piDir, { recursive: true });
writeFileSync(join(piDir, "APPEND_SYSTEM.md"), "Additional instructions.");
const loader = new DefaultResourceLoader({ cwd, agentDir });
await loader.reload();
expect(loader.getAppendSystemPrompt()).toContain("Additional instructions.");
});
});
describe("noSkills option", () => {
it("should skip skill discovery when noSkills is true", async () => {
const skillsDir = join(agentDir, "skills");
mkdirSync(skillsDir, { recursive: true });
writeFileSync(
join(skillsDir, "test-skill.md"),
`---
name: test-skill
description: A test skill
---
Content`,
);
const loader = new DefaultResourceLoader({ cwd, agentDir, noSkills: true });
await loader.reload();
const { skills } = loader.getSkills();
expect(skills).toEqual([]);
});
it("should still load additional skill paths when noSkills is true", async () => {
const customSkillDir = join(tempDir, "custom-skills");
mkdirSync(customSkillDir, { recursive: true });
writeFileSync(
join(customSkillDir, "custom.md"),
`---
name: custom
description: Custom skill
---
Content`,
);
const loader = new DefaultResourceLoader({
cwd,
agentDir,
noSkills: true,
additionalSkillPaths: [customSkillDir],
});
await loader.reload();
const { skills } = loader.getSkills();
expect(skills.some((s) => s.name === "custom")).toBe(true);
});
});
describe("override functions", () => {
it("should apply skillsOverride", async () => {
const injectedSkill: Skill = {
name: "injected",
description: "Injected skill",
filePath: "/fake/path",
baseDir: "/fake",
source: "custom",
};
const loader = new DefaultResourceLoader({
cwd,
agentDir,
skillsOverride: () => ({
skills: [injectedSkill],
diagnostics: [],
}),
});
await loader.reload();
const { skills } = loader.getSkills();
expect(skills).toHaveLength(1);
expect(skills[0].name).toBe("injected");
});
it("should apply systemPromptOverride", async () => {
const loader = new DefaultResourceLoader({
cwd,
agentDir,
systemPromptOverride: () => "Custom system prompt",
});
await loader.reload();
expect(loader.getSystemPrompt()).toBe("Custom system prompt");
});
});
describe("extension conflict detection", () => {
it("should detect tool conflicts between extensions", async () => {
// Create two extensions that register the same tool
const ext1Dir = join(agentDir, "extensions", "ext1");
const ext2Dir = join(agentDir, "extensions", "ext2");
mkdirSync(ext1Dir, { recursive: true });
mkdirSync(ext2Dir, { recursive: true });
writeFileSync(
join(ext1Dir, "index.ts"),
`
import type { ExtensionAPI } from "@mariozechner/pi-coding-agent";
import { Type } from "@sinclair/typebox";
export default function(pi: ExtensionAPI) {
pi.registerTool({
name: "duplicate-tool",
description: "First",
parameters: Type.Object({}),
execute: async () => ({ result: "1" }),
});
}`,
);
writeFileSync(
join(ext2Dir, "index.ts"),
`
import type { ExtensionAPI } from "@mariozechner/pi-coding-agent";
import { Type } from "@sinclair/typebox";
export default function(pi: ExtensionAPI) {
pi.registerTool({
name: "duplicate-tool",
description: "Second",
parameters: Type.Object({}),
execute: async () => ({ result: "2" }),
});
}`,
);
const loader = new DefaultResourceLoader({ cwd, agentDir });
await loader.reload();
const { errors } = loader.getExtensions();
expect(errors.some((e) => e.error.includes("duplicate-tool") && e.error.includes("conflicts"))).toBe(true);
});
});
});