fix: three spawn regressions from PR #88 session-manager refactor
1. No-issue spawn: use session/{sessionId} branch instead of defaultBranch
to avoid git worktree conflicts with the main repo's checked-out branch.
2. Plugin resolution: accept importFn parameter in loadBuiltins/loadFromConfig
so CLI can pass its own import() context for pnpm strict resolution. Added
all plugin packages as CLI workspace dependencies.
3. Ad-hoc issue IDs: gracefully handle IssueNotFoundError by continuing
without tracker context instead of throwing. Non-issue errors (auth,
network) still fail fast.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
59c490a3af
commit
04dff444ae
|
|
@ -34,10 +34,22 @@
|
|||
},
|
||||
"dependencies": {
|
||||
"@composio/ao-core": "workspace:*",
|
||||
"@composio/ao-plugin-agent-aider": "workspace:*",
|
||||
"@composio/ao-plugin-agent-claude-code": "workspace:*",
|
||||
"@composio/ao-plugin-agent-codex": "workspace:*",
|
||||
"@composio/ao-plugin-agent-aider": "workspace:*",
|
||||
"@composio/ao-plugin-notifier-composio": "workspace:*",
|
||||
"@composio/ao-plugin-notifier-desktop": "workspace:*",
|
||||
"@composio/ao-plugin-notifier-slack": "workspace:*",
|
||||
"@composio/ao-plugin-notifier-webhook": "workspace:*",
|
||||
"@composio/ao-plugin-runtime-process": "workspace:*",
|
||||
"@composio/ao-plugin-runtime-tmux": "workspace:*",
|
||||
"@composio/ao-plugin-scm-github": "workspace:*",
|
||||
"@composio/ao-plugin-terminal-iterm2": "workspace:*",
|
||||
"@composio/ao-plugin-terminal-web": "workspace:*",
|
||||
"@composio/ao-plugin-tracker-github": "workspace:*",
|
||||
"@composio/ao-plugin-tracker-linear": "workspace:*",
|
||||
"@composio/ao-plugin-workspace-clone": "workspace:*",
|
||||
"@composio/ao-plugin-workspace-worktree": "workspace:*",
|
||||
"chalk": "^5.4.0",
|
||||
"commander": "^13.0.0",
|
||||
"ora": "^8.1.0",
|
||||
|
|
|
|||
|
|
@ -26,7 +26,10 @@ async function getRegistry(config: OrchestratorConfig): Promise<PluginRegistry>
|
|||
if (!registryPromise) {
|
||||
registryPromise = (async () => {
|
||||
const registry = createPluginRegistry();
|
||||
await registry.loadFromConfig(config);
|
||||
// Pass CLI's import context so pnpm strict resolution can find plugin packages.
|
||||
// Core can't resolve @composio/ao-plugin-* from its own module context because
|
||||
// they aren't in core's dependencies. The CLI has them as workspace deps.
|
||||
await registry.loadFromConfig(config, (pkg: string) => import(pkg));
|
||||
return registry;
|
||||
})();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -264,10 +264,10 @@ describe("spawn", () => {
|
|||
expect(session.issueId).toBe("INT-100");
|
||||
});
|
||||
|
||||
it("fails when issue not found in tracker", async () => {
|
||||
it("succeeds with ad-hoc issue string when tracker returns IssueNotFoundError", async () => {
|
||||
const mockTracker: Tracker = {
|
||||
name: "mock-tracker",
|
||||
getIssue: vi.fn().mockRejectedValue(new Error("Issue not found")),
|
||||
getIssue: vi.fn().mockRejectedValue(new Error("Issue INT-9999 not found")),
|
||||
isCompleted: vi.fn().mockResolvedValue(false),
|
||||
issueUrl: vi.fn().mockReturnValue(""),
|
||||
branchName: vi.fn().mockReturnValue("feat/INT-9999"),
|
||||
|
|
@ -290,13 +290,14 @@ describe("spawn", () => {
|
|||
registry: registryWithTracker,
|
||||
});
|
||||
|
||||
await expect(sm.spawn({ projectId: "my-app", issueId: "INT-9999" })).rejects.toThrow(
|
||||
"does not exist in tracker",
|
||||
);
|
||||
// Ad-hoc issue string should succeed — IssueNotFoundError is gracefully ignored
|
||||
const session = await sm.spawn({ projectId: "my-app", issueId: "INT-9999" });
|
||||
|
||||
// Should not create workspace or runtime when validation fails
|
||||
expect(mockWorkspace.create).not.toHaveBeenCalled();
|
||||
expect(mockRuntime.create).not.toHaveBeenCalled();
|
||||
expect(session.issueId).toBe("INT-9999");
|
||||
expect(session.branch).toBe("feat/INT-9999");
|
||||
// Workspace and runtime should still be created
|
||||
expect(mockWorkspace.create).toHaveBeenCalled();
|
||||
expect(mockRuntime.create).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("fails on tracker auth errors", async () => {
|
||||
|
|
@ -340,7 +341,9 @@ describe("spawn", () => {
|
|||
const session = await sm.spawn({ projectId: "my-app" });
|
||||
|
||||
expect(session.issueId).toBeNull();
|
||||
expect(session.branch).toBe("main"); // Uses defaultBranch
|
||||
// Uses session/{sessionId} to avoid conflicts with default branch
|
||||
expect(session.branch).toMatch(/^session\/app-\d+$/);
|
||||
expect(session.branch).not.toBe("main");
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -745,3 +748,51 @@ describe("send", () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("PluginRegistry.loadBuiltins importFn", () => {
|
||||
it("should use provided importFn instead of built-in import", async () => {
|
||||
const { createPluginRegistry: createReg } = await import("../plugin-registry.js");
|
||||
const registry = createReg();
|
||||
const importedPackages: string[] = [];
|
||||
|
||||
const fakeImportFn = async (pkg: string): Promise<unknown> => {
|
||||
importedPackages.push(pkg);
|
||||
// Return a valid plugin module for runtime-tmux
|
||||
if (pkg === "@composio/ao-plugin-runtime-tmux") {
|
||||
return {
|
||||
manifest: { name: "tmux", slot: "runtime", description: "test", version: "0.0.0" },
|
||||
create: () => ({ name: "tmux" }),
|
||||
};
|
||||
}
|
||||
// Throw for everything else to simulate not-installed
|
||||
throw new Error(`Module not found: ${pkg}`);
|
||||
};
|
||||
|
||||
await registry.loadBuiltins(undefined, fakeImportFn);
|
||||
|
||||
// importFn should have been called for all builtin plugins
|
||||
expect(importedPackages.length).toBeGreaterThan(0);
|
||||
expect(importedPackages).toContain("@composio/ao-plugin-runtime-tmux");
|
||||
|
||||
// The tmux plugin should be registered
|
||||
const tmux = registry.get("runtime", "tmux");
|
||||
expect(tmux).not.toBeNull();
|
||||
});
|
||||
|
||||
it("should pass importFn through loadFromConfig to loadBuiltins", async () => {
|
||||
const { createPluginRegistry: createReg } = await import("../plugin-registry.js");
|
||||
const registry = createReg();
|
||||
const importedPackages: string[] = [];
|
||||
|
||||
const fakeImportFn = async (pkg: string): Promise<unknown> => {
|
||||
importedPackages.push(pkg);
|
||||
throw new Error(`Not found: ${pkg}`);
|
||||
};
|
||||
|
||||
await registry.loadFromConfig(config, fakeImportFn);
|
||||
|
||||
// Should have attempted to import builtin plugins via the provided importFn
|
||||
expect(importedPackages.length).toBeGreaterThan(0);
|
||||
expect(importedPackages).toContain("@composio/ao-plugin-runtime-tmux");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -85,10 +85,14 @@ export function createPluginRegistry(): PluginRegistry {
|
|||
return result;
|
||||
},
|
||||
|
||||
async loadBuiltins(orchestratorConfig?: OrchestratorConfig): Promise<void> {
|
||||
async loadBuiltins(
|
||||
orchestratorConfig?: OrchestratorConfig,
|
||||
importFn?: (pkg: string) => Promise<unknown>,
|
||||
): Promise<void> {
|
||||
const doImport = importFn ?? ((pkg: string) => import(pkg));
|
||||
for (const builtin of BUILTIN_PLUGINS) {
|
||||
try {
|
||||
const mod = (await import(builtin.pkg)) as PluginModule;
|
||||
const mod = (await doImport(builtin.pkg)) as PluginModule;
|
||||
if (mod.manifest && typeof mod.create === "function") {
|
||||
const pluginConfig = orchestratorConfig
|
||||
? extractPluginConfig(builtin.slot, builtin.name, orchestratorConfig)
|
||||
|
|
@ -101,9 +105,12 @@ export function createPluginRegistry(): PluginRegistry {
|
|||
}
|
||||
},
|
||||
|
||||
async loadFromConfig(config: OrchestratorConfig): Promise<void> {
|
||||
async loadFromConfig(
|
||||
config: OrchestratorConfig,
|
||||
importFn?: (pkg: string) => Promise<unknown>,
|
||||
): Promise<void> {
|
||||
// Load built-ins with orchestrator config so plugins receive their settings
|
||||
await this.loadBuiltins(config);
|
||||
await this.loadBuiltins(config, importFn);
|
||||
|
||||
// Then, load any additional plugins specified in project configs
|
||||
// (future: support npm package names and local file paths)
|
||||
|
|
|
|||
|
|
@ -302,12 +302,8 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager {
|
|||
} catch (err) {
|
||||
// Issue fetch failed - determine why
|
||||
if (isIssueNotFoundError(err)) {
|
||||
// Issue doesn't exist - fail fast with clear message
|
||||
throw new Error(
|
||||
`Issue ${spawnConfig.issueId} does not exist in tracker. ` +
|
||||
`Create the issue first, then spawn with the created issue ID.`,
|
||||
{ cause: err },
|
||||
);
|
||||
// Ad-hoc issue string — proceed without tracker context.
|
||||
// Branch will be generated as feat/{issueId} (line 329-331)
|
||||
} else {
|
||||
// Other error (auth, network, etc) - fail fast
|
||||
throw new Error(`Failed to fetch issue ${spawnConfig.issueId}: ${err}`, { cause: err });
|
||||
|
|
@ -357,7 +353,7 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager {
|
|||
} else if (spawnConfig.issueId) {
|
||||
branch = `feat/${spawnConfig.issueId}`;
|
||||
} else {
|
||||
branch = project.defaultBranch;
|
||||
branch = `session/${sessionId}`;
|
||||
}
|
||||
|
||||
// Create workspace (if workspace plugin is available)
|
||||
|
|
|
|||
|
|
@ -937,10 +937,16 @@ export interface PluginRegistry {
|
|||
list(slot: PluginSlot): PluginManifest[];
|
||||
|
||||
/** Load built-in plugins, optionally with orchestrator config for plugin settings */
|
||||
loadBuiltins(config?: OrchestratorConfig): Promise<void>;
|
||||
loadBuiltins(
|
||||
config?: OrchestratorConfig,
|
||||
importFn?: (pkg: string) => Promise<unknown>,
|
||||
): Promise<void>;
|
||||
|
||||
/** Load plugins from config (npm packages, local paths) */
|
||||
loadFromConfig(config: OrchestratorConfig): Promise<void>;
|
||||
loadFromConfig(
|
||||
config: OrchestratorConfig,
|
||||
importFn?: (pkg: string) => Promise<unknown>,
|
||||
): Promise<void>;
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
|
|
|
|||
|
|
@ -53,9 +53,45 @@ importers:
|
|||
'@composio/ao-plugin-agent-codex':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/agent-codex
|
||||
'@composio/ao-plugin-notifier-composio':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/notifier-composio
|
||||
'@composio/ao-plugin-notifier-desktop':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/notifier-desktop
|
||||
'@composio/ao-plugin-notifier-slack':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/notifier-slack
|
||||
'@composio/ao-plugin-notifier-webhook':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/notifier-webhook
|
||||
'@composio/ao-plugin-runtime-process':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/runtime-process
|
||||
'@composio/ao-plugin-runtime-tmux':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/runtime-tmux
|
||||
'@composio/ao-plugin-scm-github':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/scm-github
|
||||
'@composio/ao-plugin-terminal-iterm2':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/terminal-iterm2
|
||||
'@composio/ao-plugin-terminal-web':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/terminal-web
|
||||
'@composio/ao-plugin-tracker-github':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/tracker-github
|
||||
'@composio/ao-plugin-tracker-linear':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/tracker-linear
|
||||
'@composio/ao-plugin-workspace-clone':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/workspace-clone
|
||||
'@composio/ao-plugin-workspace-worktree':
|
||||
specifier: workspace:*
|
||||
version: link:../plugins/workspace-worktree
|
||||
chalk:
|
||||
specifier: ^5.4.0
|
||||
version: 5.6.2
|
||||
|
|
|
|||
Loading…
Reference in New Issue