mirror of
https://github.com/mvschwarz/openrig.git
synced 2026-11-07 05:56:21 -10:00
fix(daemon): load the vendored openrig-core plugin's skills once on Claude seats (#2433)
A Claude seat no longer loads openrig-core's skills twice. Each seat's own `openrig` skill plugin received a copy of every selected plugin's skills, and the seat also loads the vendored `openrig-core` plugin directly for its hooks, which includes its skills. The copy is now skipped exactly when that direct load applies (a Claude seat, the vendored manifest present, and a selection that is `<plugins>/openrig-core`). Both routes reach Claude through `CLAUDE_CODE_PLUGIN_DIRS`, so no skill is lost. Existing seats change at their next fresh launch. Codex is unchanged. Thanks to @billbenetti for the report.
This commit is contained in:
1 parent
85881245b4
commit
6a35cd39f3
2 files changed
+51
-3
No files matched your search
@@ -13,6 +13,7 @@ import { resolveStartup } from "./startup-resolver.js";
|
||||
import { discoverSkillsForRuntime, findNamedSkillsForRuntime, parseSkillFrontmatter, scanCacheFor, type SkillRuntime } from "./skill-discovery.js";
|
||||
import { inspectSkillDirectory, resolvePluginSkills, resolveSkillLoadout, type SkillLoadout } from "./skill-catalog.js";
|
||||
import { resolvePluginPath } from "./projection-planner.js";
|
||||
import { selectedCorePluginDir, vendoredCorePluginDir } from "./claude-core-plugin-env.js";
|
||||
|
||||
// -- Types --
|
||||
|
||||
@@ -352,9 +353,13 @@ export function resolveNodeConfig(ctx: ResolutionContext): ResolutionResult {
|
||||
const selectedIds = new Set(selectedResult!.skills.map((skill) => skill.effectiveId));
|
||||
for (const plugin of selectedResult!.plugins) {
|
||||
const resource = plugin.resource as PluginResource;
|
||||
const pluginRoot = resolvePluginPath(resource.source.path, plugin.sourcePath);
|
||||
// Except the vendored openrig-core on a Claude seat: the seat loads that plugin itself, skills included
|
||||
// (CLAUDE_CODE_PLUGIN_DIRS, by seatCorePluginDir's rule), so a loadout copy would load each skill twice.
|
||||
if (runtime === "claude-code" && vendoredCorePluginDir() && selectedCorePluginDir([pluginRoot])) continue;
|
||||
const pluginSkills = resolvePluginSkills({
|
||||
pluginId: resource.id,
|
||||
pluginRoot: resolvePluginPath(resource.source.path, plugin.sourcePath),
|
||||
pluginRoot,
|
||||
runtime,
|
||||
pluginType: resource.pluginType,
|
||||
});
|
||||
|
||||
@@ -920,7 +920,7 @@ describe("selected plugin skills", () => {
|
||||
return plugin;
|
||||
}
|
||||
|
||||
function resolveWithPlugin(root: string, plugin: string, skills: Array<{ id: string; path: string }>) {
|
||||
function resolveWithPlugin(root: string, plugin: string, skills: Array<{ id: string; path: string }>, runtime = "claude-code") {
|
||||
const spec = makeSpec({
|
||||
resources: { skills, guidance: [], subagents: [], plugins: [{ id: "core", source: { kind: "local", path: plugin } }], runtimeResources: [] },
|
||||
profiles: {
|
||||
@@ -929,12 +929,55 @@ describe("selected plugin skills", () => {
|
||||
});
|
||||
return resolveNodeConfig(makeCtx({
|
||||
baseSpec: makeResolved(spec, root),
|
||||
member: makeMember({ cwd: join(root, "work") }),
|
||||
member: makeMember({ cwd: join(root, "work"), runtime }),
|
||||
skillsRoot: join(root, "no-catalog"),
|
||||
homedir: join(root, "home"),
|
||||
}));
|
||||
}
|
||||
|
||||
/** An openrig-core plugin copy at `dir`, with both manifests and one skill. */
|
||||
function writeCore(dir: string): string {
|
||||
for (const manifest of [".claude-plugin", ".codex-plugin"]) {
|
||||
mkdirSync(join(dir, manifest), { recursive: true });
|
||||
writeFileSync(join(dir, manifest, "plugin.json"), JSON.stringify({ name: "openrig-core", version: "0.1.0" }));
|
||||
}
|
||||
mkdirSync(join(dir, "skills", "refocusing"), { recursive: true });
|
||||
writeFileSync(join(dir, "skills", "refocusing", "SKILL.md"), "---\nname: refocusing\ndescription: Use when testing refocusing.\n---\n\n# core\n");
|
||||
return dir;
|
||||
}
|
||||
|
||||
/** Run with OPENRIG_HOME at `root`, so <root>/plugins/openrig-core is the vendored copy a Claude seat loads itself. */
|
||||
function withHome<T>(root: string, run: () => T): T {
|
||||
const previous = process.env.OPENRIG_HOME;
|
||||
process.env.OPENRIG_HOME = root;
|
||||
try { return run(); } finally {
|
||||
if (previous === undefined) delete process.env.OPENRIG_HOME; else process.env.OPENRIG_HOME = previous;
|
||||
}
|
||||
}
|
||||
|
||||
const loadoutIds = (result: ReturnType<typeof resolveWithPlugin>) =>
|
||||
result.ok ? (result.config.skillLoadout?.entries ?? []).map((entry) => entry.id) : [JSON.stringify(result)];
|
||||
|
||||
it("leaves the vendored openrig-core plugin's skills out of a Claude seat's loadout: the seat loads that plugin itself", () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "openrig-profile-core-plugin-"));
|
||||
try {
|
||||
const vendored = writeCore(join(root, "plugins", "openrig-core"));
|
||||
expect(withHome(root, () => loadoutIds(resolveWithPlugin(root, vendored, [])))).toEqual([]);
|
||||
} finally { rmSync(root, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
it("keeps openrig-core's skills in the loadout for a Codex seat, and for a Claude seat that selected another copy", () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "openrig-profile-core-plugin-kept-"));
|
||||
try {
|
||||
const vendored = writeCore(join(root, "plugins", "openrig-core"));
|
||||
const other = writeCore(join(root, "elsewhere", "openrig-core"));
|
||||
// Codex loads its own installed copy only when the person's Codex config enables it, so the loadout keeps them.
|
||||
expect(withHome(root, () => loadoutIds(resolveWithPlugin(root, vendored, [], "codex")))).toEqual(["refocusing"]);
|
||||
// A copy other than the vendored one isn't loaded by the seat, so its skills still come through the loadout.
|
||||
expect(withHome(root, () => loadoutIds(resolveWithPlugin(root, other, [])))).toEqual(["refocusing"]);
|
||||
} finally { rmSync(root, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
it("adds a selected plugin's skills to the managed loadout, not to the plan's skill entries", () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "openrig-profile-plugin-skills-"));
|
||||
try {
|
||||
|
||||
Reference in new issue
Block a user