feat(core,engine,compound-engineering): make CE sub-agent sessions load their skills
Closes the U2/U5 skill-discovery carry-forward so the plugin's interactive ce-* sessions actually load the stage's bundled skill in a live agent (not just in scripted-fake tests). Root cause: createFnAgent built its DefaultResourceLoader without forwarding any skill-discovery path, and the interactive seam options couldn't carry one. The loader's skillsOverride only *filters* skills already discovered from cwd's standard roots, so the plugin-local .fusion-ce-skills/<id>/SKILL.md was never discoverable. Fix (end-to-end): - AgentOptions.additionalSkillPaths forwarded into DefaultResourceLoader - CreateInteractiveAiSessionOptions gains requestedSkillNames + additionalSkillPaths - the interactive engine adapter forwards them to createFnAgent (skills + additionalSkillPaths) - the orchestrator runs the session with cwd on the real project root and hands it [stage.skillId] + the install root Proven: a real DefaultResourceLoader with additionalSkillPaths discovers ce-plan and filters out ce-work; the orchestrator passes the right id/path/cwd. Plugin 96, engine 136, core 99 tests green.
This commit is contained in:
@@ -151,6 +151,19 @@ export interface CreateInteractiveAiSessionOptions {
|
||||
defaultProvider?: string;
|
||||
/** Default model ID within the provider */
|
||||
defaultModelId?: string;
|
||||
/**
|
||||
* Skill names the session should load (matched against discovered skills).
|
||||
* Lets a plugin point a session at a specific bundled skill rather than
|
||||
* relying on cwd-only discovery. Forwarded to the engine's skill selection.
|
||||
*/
|
||||
requestedSkillNames?: string[];
|
||||
/**
|
||||
* Extra directories to scan for skills (each holding `<id>/SKILL.md`), in
|
||||
* addition to the default cwd/agent-dir roots. A plugin that installs its
|
||||
* skills to a plugin-local directory passes that directory here so its
|
||||
* `requestedSkillNames` are actually discoverable in the live session.
|
||||
*/
|
||||
additionalSkillPaths?: string[];
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -30,7 +30,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { loadSkills } from "@earendil-works/pi-coding-agent";
|
||||
import { DefaultResourceLoader, loadSkills, type Skill } from "@earendil-works/pi-coding-agent";
|
||||
import {
|
||||
createSkillsOverrideFromSelection,
|
||||
resolveSessionSkills,
|
||||
@@ -156,4 +156,39 @@ describe("U2: CE bundled skill session-resolution (empirical)", () => {
|
||||
expect(resolved).toContain(s);
|
||||
}
|
||||
});
|
||||
|
||||
it("PASSING (real loader): DefaultResourceLoader with additionalSkillPaths + skillsOverride discovers ce-plan — the exact path createFnAgent now feeds", async () => {
|
||||
const installRoot = join(tmp, ".fusion-ce-skills");
|
||||
materializeInstalledSkills(installRoot, ["ce-plan", "ce-work"]);
|
||||
mkdirSync(projectRootDir, { recursive: true });
|
||||
mkdirSync(agentDir, { recursive: true });
|
||||
|
||||
// Build the same skillsOverride createFnAgent builds from `skills: ["ce-plan"]`.
|
||||
const selection = resolveSessionSkills({
|
||||
projectRootDir,
|
||||
requestedSkillNames: ["ce-plan"],
|
||||
sessionPurpose: "executor",
|
||||
});
|
||||
const skillsOverride = createSkillsOverrideFromSelection(selection, {
|
||||
requestedSkillNames: ["ce-plan"],
|
||||
sessionPurpose: "executor",
|
||||
});
|
||||
|
||||
// Construct the loader exactly as pi.ts createFnAgent now does: cwd on the
|
||||
// project root, the install dir passed via additionalSkillPaths, and the
|
||||
// requested-name filter as skillsOverride.
|
||||
const loader = new DefaultResourceLoader({
|
||||
cwd: projectRootDir,
|
||||
agentDir,
|
||||
additionalSkillPaths: [installRoot],
|
||||
skillsOverride,
|
||||
});
|
||||
await loader.reload();
|
||||
|
||||
const names = loader.getSkills().skills.map((s: Skill) => s.name);
|
||||
// ce-plan is discoverable (via additionalSkillPaths) AND survives the filter;
|
||||
// ce-work is discovered but filtered out by the requested-name override.
|
||||
expect(names).toContain("ce-plan");
|
||||
expect(names).not.toContain("ce-work");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -156,6 +156,11 @@ const _createInteractiveAiSessionAdapter: CreateInteractiveAiSessionFactory = (
|
||||
tools: opts.tools,
|
||||
defaultProvider: opts.defaultProvider,
|
||||
defaultModelId: opts.defaultModelId,
|
||||
// Forward skill selection so a plugin can load a specific bundled skill.
|
||||
// `skills` (convenience) auto-builds a SkillSelectionContext; the extra
|
||||
// discovery dirs make those skills actually visible to the loader.
|
||||
...(opts.requestedSkillNames?.length ? { skills: opts.requestedSkillNames } : {}),
|
||||
...(opts.additionalSkillPaths?.length ? { additionalSkillPaths: opts.additionalSkillPaths } : {}),
|
||||
}),
|
||||
options,
|
||||
);
|
||||
|
||||
@@ -964,6 +964,11 @@ export interface AgentOptions {
|
||||
* (and `skillSelection` is not), auto-constructs a SkillSelectionContext
|
||||
* from the cwd and these names. Ignored when `skillSelection` is set. */
|
||||
skills?: string[];
|
||||
/** Extra directories to scan for skills (each holding `<id>/SKILL.md`), in
|
||||
* addition to the default cwd/agent-dir roots. Forwarded to the resource
|
||||
* loader so callers (e.g. plugins that install skills to a private dir) can
|
||||
* make `skills`/`skillSelection` names discoverable in the live session. */
|
||||
additionalSkillPaths?: string[];
|
||||
/** Optional task-scoped env injected into this session's subprocess tools only. */
|
||||
taskEnv?: NodeJS.ProcessEnv;
|
||||
/** Last-chance abort hook fired immediately before `createAgentSession`.
|
||||
@@ -1987,6 +1992,9 @@ export async function createFnAgent(options: AgentOptions): Promise<AgentResult>
|
||||
? [options.systemPromptLayers.dynamic]
|
||||
: [],
|
||||
...(effectiveExtensionPaths.length > 0 ? { additionalExtensionPaths: [...effectiveExtensionPaths] } : {}),
|
||||
...(options.additionalSkillPaths && options.additionalSkillPaths.length > 0
|
||||
? { additionalSkillPaths: [...options.additionalSkillPaths] }
|
||||
: {}),
|
||||
...(skillsOverrideFn ? { skillsOverride: skillsOverrideFn } : {}),
|
||||
});
|
||||
await resourceLoader.reload();
|
||||
|
||||
@@ -1,58 +1,49 @@
|
||||
import { existsSync, mkdtempSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { existsSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { installBundledCeSkills } from "../skill-installation.js";
|
||||
import { resolveStageSkillCwd, buildStageSystemPrompt } from "../session/orchestrator.js";
|
||||
import { resolveStageSkillPaths, buildStageSystemPrompt } from "../session/orchestrator.js";
|
||||
import { getStage } from "../session/stage-registry.js";
|
||||
|
||||
/**
|
||||
* CARRY-FORWARD (U2 → U5): prove the launched stage's ce-* skill is REACHABLE
|
||||
* for the session.
|
||||
* Prove the launched stage's ce-* skill is REACHABLE for the session — now via
|
||||
* the real seam wiring (closes the U2 → U5 carry-forward).
|
||||
*
|
||||
* HONEST SCOPE — proved at the installer/resolver layer (exactly as U2 did),
|
||||
* NOT at the live session layer. The U4 `CreateInteractiveAiSessionOptions`
|
||||
* surface carries only `cwd` (no `requestedSkillNames`/`additionalSkillPaths`/
|
||||
* `skillSelection`), so the orchestrator cannot hand the session an explicit
|
||||
* skill-discovery path. The closest honest wiring is:
|
||||
* 1. point the session `cwd` at the install-target root (where pi's
|
||||
* DefaultResourceLoader can discover `<skillId>/SKILL.md`), and
|
||||
* 2. name the skill id in the system prompt.
|
||||
* This test asserts BOTH: the resolved cwd contains the stage's installed
|
||||
* SKILL.md, and the system prompt names the stage's skill id.
|
||||
* The U4 `CreateInteractiveAiSessionOptions` surface now carries
|
||||
* `requestedSkillNames` + `additionalSkillPaths`, which the engine adapter
|
||||
* forwards into `createFnAgent` (`skills` + the loader's `additionalSkillPaths`).
|
||||
* So the orchestrator hands the session BOTH the stage's skill id and the
|
||||
* install directory to discover it from. This test asserts:
|
||||
* 1. the install directory `resolveStageSkillPaths()` returns actually holds
|
||||
* the stage's `<skillId>/SKILL.md` after install, and
|
||||
* 2. the system prompt names the stage's skill id.
|
||||
*
|
||||
* A complete fix needs U4's options to gain a forwarded
|
||||
* `requestedSkillNames`/`additionalSkillPaths` field — flagged as a carry-
|
||||
* forward for U6/follow-up.
|
||||
* The engine package separately proves (compound-engineering-skill-resolution
|
||||
* .test.ts) that `loadSkills` + the resolver resolve a ce-* skill once that
|
||||
* directory is on the discovery path — together the chain is closed.
|
||||
*/
|
||||
|
||||
describe("stage skill reachability (carry-forward, resolver-layer proof)", () => {
|
||||
let targets: string[] = [];
|
||||
|
||||
describe("stage skill reachability (real seam wiring)", () => {
|
||||
afterEach(() => {
|
||||
targets = [];
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("the resolved session cwd contains the stage's installed SKILL.md", () => {
|
||||
const target = mkdtempSync(join(tmpdir(), "ce-skill-target-"));
|
||||
targets.push(target);
|
||||
it("the resolved additionalSkillPaths directory contains the stage's installed SKILL.md", () => {
|
||||
const stage = getStage("brainstorm")!;
|
||||
|
||||
// Install bundled skills into a plugin-local target.
|
||||
const { results } = installBundledCeSkills({ targetRoot: target });
|
||||
// resolveStageSkillPaths() returns the plugin-local install root the session
|
||||
// is told to discover skills from (never a global one).
|
||||
const skillPaths = resolveStageSkillPaths();
|
||||
expect(skillPaths).toHaveLength(1);
|
||||
expect(skillPaths[0]).toMatch(/\.fusion-ce-skills$/);
|
||||
|
||||
// Install bundled skills into that exact root and assert the stage's skill
|
||||
// is present on the path the session will scan.
|
||||
const { results } = installBundledCeSkills({ targetRoot: skillPaths[0] });
|
||||
expect(results.every((r) => r.outcome === "installed" || r.outcome === "skipped")).toBe(true);
|
||||
|
||||
const stage = getStage("brainstorm")!;
|
||||
// The orchestrator resolves the discovery cwd to the default install-target
|
||||
// root. For this isolation test we assert the SAME structure exists at the
|
||||
// explicit target we installed into (resolveStageSkillCwd returns the
|
||||
// default root, which the production onLoad install populates identically).
|
||||
const installedSkillMd = join(target, stage.skillId, "SKILL.md");
|
||||
const installedSkillMd = join(skillPaths[0], stage.skillId, "SKILL.md");
|
||||
expect(existsSync(installedSkillMd)).toBe(true);
|
||||
|
||||
// resolveStageSkillCwd returns a plugin-local directory (never a global one).
|
||||
const cwd = resolveStageSkillCwd();
|
||||
expect(cwd).toMatch(/\.fusion-ce-skills$/);
|
||||
});
|
||||
|
||||
it("the stage system prompt names the stage's ce-* skill id", () => {
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type {
|
||||
CreateInteractiveAiSessionOptions,
|
||||
InteractiveAiSessionEvent,
|
||||
} from "@fusion/core";
|
||||
import { CeOrchestrator } from "../session/orchestrator.js";
|
||||
import { getStage } from "../session/stage-registry.js";
|
||||
import { resolveDefaultInstallTargetRoot } from "../skill-installation.js";
|
||||
import { makeHarness, makeScriptedSession, type TestHarness } from "./_harness.js";
|
||||
|
||||
/**
|
||||
* Proves the orchestrator hands a launched session the wiring a LIVE agent needs
|
||||
* to actually load the stage's bundled ce-* skill (closes the U2/U5 carry-forward):
|
||||
* - cwd is the real project root (where the agent reads context + writes the
|
||||
* artifact), NOT the skills directory;
|
||||
* - requestedSkillNames names the stage's ce-* skill;
|
||||
* - additionalSkillPaths includes the plugin-local install root so the engine
|
||||
* loader can discover that skill.
|
||||
* The engine adapter forwards these to createFnAgent (skills + additionalSkillPaths);
|
||||
* compound-engineering-skill-resolution.test.ts proves the loader then resolves it.
|
||||
*/
|
||||
describe("session skill wiring", () => {
|
||||
let h: TestHarness;
|
||||
beforeEach(() => {
|
||||
h = makeHarness();
|
||||
});
|
||||
afterEach(() => {
|
||||
h.close();
|
||||
});
|
||||
|
||||
it("start() passes the stage skill id, install path, and project-root cwd to the factory", async () => {
|
||||
const captured: CreateInteractiveAiSessionOptions[] = [];
|
||||
const script: InteractiveAiSessionEvent[] = [
|
||||
{ type: "complete", data: { artifact: "# done" } },
|
||||
];
|
||||
const session = makeScriptedSession(script);
|
||||
const factory = vi.fn(async (opts: CreateInteractiveAiSessionOptions) => {
|
||||
captured.push(opts);
|
||||
return { session };
|
||||
});
|
||||
const orch = new CeOrchestrator({
|
||||
ctx: h.ctx,
|
||||
createInteractiveAiSession: factory,
|
||||
projectRoot: h.projectRoot,
|
||||
turnTimeoutMs: 5000,
|
||||
});
|
||||
|
||||
await orch.start("brainstorm", { openingMessage: "let's go" });
|
||||
|
||||
expect(captured).toHaveLength(1);
|
||||
const opts = captured[0];
|
||||
const stage = getStage("brainstorm")!;
|
||||
// cwd is the project root, not the skills dir.
|
||||
expect(opts.cwd).toBe(h.projectRoot);
|
||||
// the stage's ce-* skill is requested...
|
||||
expect(opts.requestedSkillNames).toEqual([stage.skillId]);
|
||||
// ...and the plugin-local install root is on the discovery path.
|
||||
expect(opts.additionalSkillPaths).toEqual([resolveDefaultInstallTargetRoot()]);
|
||||
expect(opts.additionalSkillPaths?.[0]).toMatch(/\.fusion-ce-skills$/);
|
||||
});
|
||||
});
|
||||
@@ -89,33 +89,24 @@ export interface OrchestratorDeps {
|
||||
}
|
||||
|
||||
/**
|
||||
* CARRY-FORWARD (U2 → U5) — skill discovery wiring.
|
||||
* Skill discovery wiring (closes the U2 → U5 carry-forward).
|
||||
*
|
||||
* U2 proved a `PluginSkillContribution` is NOT auto-ingested by the engine
|
||||
* skill-resolver; a physical install onto a cwd-discoverable path is required.
|
||||
* The U4 `CreateInteractiveAiSessionOptions` surface carries ONLY `cwd` (plus
|
||||
* systemPrompt/tools/provider/model) — it has NO `requestedSkillNames` /
|
||||
* `additionalSkillPaths` / `skillSelection` field. So we cannot point the
|
||||
* session at the install target the way `createFnAgent`'s `skills`/
|
||||
* `skillSelection` options would.
|
||||
*
|
||||
* The closest honest thing we CAN do today:
|
||||
* 1. Set the session `cwd` to a root under which the installed ce-* skills are
|
||||
* discoverable by pi's DefaultResourceLoader (the install-target root).
|
||||
* 2. Name the required skill id in the systemPrompt so the agent is told which
|
||||
* ce-* skill to apply (protocol-level instruction).
|
||||
*
|
||||
* This function computes that cwd. We PROVE reachability at the installer/
|
||||
* resolver layer (like U2 did) in the tests — the U4 options surface cannot yet
|
||||
* carry an explicit skill-path, so a complete fix needs U4's options to gain a
|
||||
* `requestedSkillNames`/`additionalSkillPaths` field forwarded into
|
||||
* `createFnAgent`. That gap is documented as a carry-forward for the follow-up,
|
||||
* which will re-expand this to derive a per-stage/per-project path.
|
||||
* skill-resolver; a physical install onto a discoverable path is required, and
|
||||
* the plugin installs its bundled `ce-*` skills to a plugin-local directory
|
||||
* (`resolveDefaultInstallTargetRoot()`). The U4 seam now carries
|
||||
* `requestedSkillNames` + `additionalSkillPaths`, which the engine adapter
|
||||
* forwards into `createFnAgent` (`skills` + the loader's `additionalSkillPaths`).
|
||||
* So the orchestrator hands the live session BOTH the stage's skill id and the
|
||||
* install directory to discover it from — the session runs with `cwd` at the
|
||||
* real project root (where it reads context and writes artifacts), not at the
|
||||
* skills directory.
|
||||
*/
|
||||
export function resolveStageSkillCwd(): string {
|
||||
// The install target root holds `<skillId>/SKILL.md` for each installed
|
||||
// skill; using it as the discovery root makes the stage's skill loadable.
|
||||
return resolveDefaultInstallTargetRoot();
|
||||
export function resolveStageSkillPaths(): string[] {
|
||||
// The install target root holds `<skillId>/SKILL.md` for each installed skill;
|
||||
// passing it as an additional skill-discovery path makes the stage's skill
|
||||
// loadable while the session cwd stays on the project.
|
||||
return [resolveDefaultInstallTargetRoot()];
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -125,7 +116,7 @@ export function resolveStageSkillCwd(): string {
|
||||
export function buildStageSystemPrompt(stage: CeStageDefinition): string {
|
||||
return [
|
||||
`You are running the Compound Engineering "${stage.stageId}" stage.`,
|
||||
`Apply the bundled skill "${stage.skillId}" (its SKILL.md is discoverable in your working directory).`,
|
||||
`Apply the bundled skill "${stage.skillId}" (it has been loaded into this session).`,
|
||||
"",
|
||||
"Drive the stage as an interactive question/answer flow. On every turn respond with ONLY a JSON object:",
|
||||
' - To ask the user something: {"type":"question","data":{"id":"<unique>","type":"single_select|multi_select|text|confirm","question":"...","options":[{"id":"..","label":".."}]}}',
|
||||
@@ -175,6 +166,28 @@ export class CeOrchestrator {
|
||||
this.turnTimeoutMs = deps.turnTimeoutMs ?? DEFAULT_TURN_TIMEOUT_MS;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the interactive-session options for a stage. The session runs with
|
||||
* `cwd` on the real project root (where it reads context and writes the stage
|
||||
* artifact) and is handed BOTH the stage's `ce-*` skill id and the plugin-local
|
||||
* install directory to discover it from — so the live agent actually loads the
|
||||
* bundled skill (closing the U2/U5 skill-discovery carry-forward). Model
|
||||
* provider/model are setting-gated (U9); omitted keys let the host pick defaults.
|
||||
*/
|
||||
private buildSessionOptions(stage: CeStageDefinition): Parameters<CreateInteractiveAiSessionFactory>[0] {
|
||||
const defaultProvider = getDefaultProvider(this.ctx.settings);
|
||||
const defaultModelId = getDefaultModelId(this.ctx.settings);
|
||||
return {
|
||||
cwd: this.projectRoot,
|
||||
systemPrompt: buildStageSystemPrompt(stage),
|
||||
tools: "coding",
|
||||
requestedSkillNames: [stage.skillId],
|
||||
additionalSkillPaths: resolveStageSkillPaths(),
|
||||
...(defaultProvider ? { defaultProvider } : {}),
|
||||
...(defaultModelId ? { defaultModelId } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
/** Start a fresh session for a registered stage and run the opening turn. */
|
||||
async start(stageId: string, opts: StartStageOptions): Promise<CeStepResult> {
|
||||
const stage = getStage(stageId);
|
||||
@@ -196,24 +209,9 @@ export class CeOrchestrator {
|
||||
});
|
||||
this.store.appendHistory(session.id, { role: "user", text: opts.openingMessage, at: new Date().toISOString() });
|
||||
|
||||
const cwd = resolveStageSkillCwd();
|
||||
const systemPrompt = buildStageSystemPrompt(stage);
|
||||
|
||||
// Setting-gated model selection (U9): pass the operator's default
|
||||
// provider/model through to the host factory; omitted keys let the host
|
||||
// pick its own defaults.
|
||||
const defaultProvider = getDefaultProvider(this.ctx.settings);
|
||||
const defaultModelId = getDefaultModelId(this.ctx.settings);
|
||||
|
||||
let interactive;
|
||||
try {
|
||||
interactive = await this.factory({
|
||||
cwd,
|
||||
systemPrompt,
|
||||
tools: "coding",
|
||||
...(defaultProvider ? { defaultProvider } : {}),
|
||||
...(defaultModelId ? { defaultModelId } : {}),
|
||||
});
|
||||
interactive = await this.factory(this.buildSessionOptions(stage));
|
||||
} catch (err) {
|
||||
return { session: this.failSession(session.id, err), event: undefined };
|
||||
}
|
||||
@@ -331,18 +329,7 @@ export class CeOrchestrator {
|
||||
const stage = getStage(session.stage);
|
||||
if (!stage) throw new Error(`Unknown CE stage: ${session.stage}`);
|
||||
|
||||
const cwd = resolveStageSkillCwd();
|
||||
const systemPrompt = buildStageSystemPrompt(stage);
|
||||
const defaultProvider = getDefaultProvider(this.ctx.settings);
|
||||
const defaultModelId = getDefaultModelId(this.ctx.settings);
|
||||
|
||||
const interactive = await this.factory!({
|
||||
cwd,
|
||||
systemPrompt,
|
||||
tools: "coding",
|
||||
...(defaultProvider ? { defaultProvider } : {}),
|
||||
...(defaultModelId ? { defaultModelId } : {}),
|
||||
});
|
||||
const interactive = await this.factory!(this.buildSessionOptions(stage));
|
||||
const live = interactive.session;
|
||||
|
||||
// Walk the recorded user turns in order. The FIRST user turn is the opening
|
||||
|
||||
Reference in New Issue
Block a user