Choose which orchestration skills get installed (#2680)
* feat(settings): choose which orchestration skills to install The skills row installed the whole bundle or nothing. A settings control next to it now opens a sheet where you keep every bundled skill or pick a subset, and saving converges ~/.agents, ~/.claude, and ~/.codex to it. "All skills" reads the bundle directory instead of a hardcoded catalog, so a skill added in a later release installs itself without another code change. Retired bundle names stay in a separate cleanup list so old copies on disk are still removed but never become selectable. The selection persists under Electron user data, separate from install state, so uninstalling does not erase the choice — a later install brings back exactly the chosen set. Installs with no saved selection read as "all", which is what they already had. * Restore skill selection after failed save * fix(settings): don't save a skill selection that never reached disk A failed save wrote the preference, converged, then wrote the old preference back. That leaves a window where a selection the user was told did not save is the one on disk, and a crash inside it makes the window permanent. Converge first and persist only on success — nothing to roll back, and a failed save now leaves disk and preference exactly as Cancel would. Resolving the bundle path when the command handlers are wired also broke daemon-manager's tests: the packaged branch reads process.resourcesPath, which is undefined outside Electron. Resolve targets per command instead — registering handlers should not touch the environment. * fix(settings): make skills convergence single-writer and confirm removals Startup auto-update and the settings commands were separate writers over the same skill directories. Either could apply a plan computed from a selection the other had already replaced — narrow the selection while startup is repairing drift and the repair puts the removed skill back. There is now one controller that owns scan, apply, and persist, and every entry point queues through it, so concurrent callers see a consistent result instead of an interleaved one. Save also had no recovery once it had converged: if the selection could not be committed afterwards, disk was changed while the preference still said the old thing, and the next startup silently undid the change. Save now puts the managed directories back to what the committed selection describes before it reports the failure, so a failed Save lands where Cancel would. Deselecting a skill deletes its directory in all three agent homes, including files the user put inside it. That is now confirmed like every other destructive action, naming the skills and the directories. Adding skills or saving an unchanged selection still saves without a prompt, and a skill the host has not installed yet deletes nothing so it does not prompt either. * fix(settings): confirm removal for skills installed in only some agent homes The removal confirmation read an `add` op as "not on disk yet". An `add` op means "missing from at least one target", so a skill sitting in ~/.claude but not the other two looked absent, and deselecting it deleted that directory — with anything the user had put inside it — without asking. The status snapshot now carries the installed skill names directly, taken from union presence across the three agent homes, and the confirmation reads that. `ops` keeps its one meaning. A host that predates the field cannot say what is on disk, so the app bridge assumes the saved selection is installed. Asking about a directory that turns out to be absent is harmless; the reverse is not. * fix(settings): warn about every skill directory a save deletes The confirmation asked "was this selected before and is it selected now", which is a different question from the one the host answers. The host converges to the desired set and deletes every managed directory outside it, so a skill on disk that the saved preference never mentioned — reinstalled by hand, restored by a sync, left behind by drift — was deleted with no warning, user files included. The predicate now mirrors the host rule: everything installed that the draft will not keep, regardless of what was saved before. It moved out of the sheet into its own module so the rule has one owner and can be tested directly. Saving with nothing to delete still does not prompt. * fix(settings): a failed save keeps files the user added inside a skill Save converges before it commits, and convergence deletes whole skill directories. When the commit then failed, the recovery re-installed the committed selection — which rebuilds the bundled files and nothing else, so notes, hooks, and anything else the user had put inside a deselected skill were gone for good. Cancel does not do that, and a failed save is supposed to land where Cancel would. Save now runs as a transaction. Before converging it stages a copy of every managed skill directory across the three agent homes, next to the tree so the copy stays on one filesystem. Committing drops the staging; failing anywhere puts each directory back exactly as it was, deletes the ones the candidate had created, and then drops the staging. Staging left by a process that died mid-save is cleaned up by the next one. The controller only sees begin/commit/rollback; the filesystem work lives in skills-transaction.ts. * fix(settings): make skill removal confirmation authoritative * fix(settings): verify skill convergence before commit * fix(settings): preserve concurrent skill edits on rollback * fix(settings): preserve staged skill metadata * fix(settings): stage skill deletions atomically * fix(settings): stage removals per filesystem * fix(settings): restore staged skill symlinks * fix(settings): canonicalize skill transaction roots * fix(settings): keep automatic skill updates non-destructive * fix(settings): report committed skill saves accurately * fix(settings): keep skill updates non-destructive * fix(settings): route skill removals through selection * fix(settings): close skill selection edge cases * fix(settings): preserve skill data during maintenance * fix(settings): hide unavailable skill install * fix(desktop): keep install policy at command boundary * fix(desktop): restore staged skill directories atomically * test(app): retry transient skills sandbox cleanup * fix(desktop): recover externally deleted skill stages * fix(desktop): preserve conflicting skill rollback data * fix(desktop): generalize skill rollback conflicts * fix(desktop): preserve skill rollback collisions * fix(desktop): recover quarantined skill backups * fix(app): inset adaptive sheet content on compact The compact Choose skills sheet rendered edge to edge: the All skills card touched both screen edges, the section label was glued to it, and the skill list ran to the edges too. Desktop was correct. AdaptiveModalSheet expressed its content inset through BottomSheetScrollView.contentContainerStyle. That is a library prop, not the style prop Unistyles registers, and gorhom's scroller was never rewritten by the Babel plugin, so on web the emitted class was never applied and the padding and gap resolved to nothing. The desktop card uses a real ScrollView, which Unistyles does track, which is why only compact was affected. The inset now lives on a View the sheet owns, rendered by every presentation, so one definition covers compact and desktop and a scroller only ever gets theme-free layout. The caller-facing prop is contentStyle, since it no longer lands on a scroll content container. Choose skills drops its ad-hoc label for SettingsSection, so the label carries the app's section rhythm instead of sitting equidistant between two cards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(desktop): prevent skill rollback data loss Preserve externally replaced directory trees during recovery and reject managed child symlinks before synchronization can write outside a skill directory. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Mohamed Boudra
co-authored by
Claude Opus 5
Claude Opus 5
parent
f3fee73976
commit
91da0bac4d
@@ -23,12 +23,7 @@ import {
|
||||
getCliInstallStatus,
|
||||
installCli,
|
||||
} from "../integrations/cli-install/index.js";
|
||||
import {
|
||||
getSkillsStatus,
|
||||
installSkills,
|
||||
uninstallSkills,
|
||||
updateSkills,
|
||||
} from "../integrations/skills/index.js";
|
||||
import { createSkillsCommandHandlers, getSkillsController } from "../integrations/skills/index.js";
|
||||
import {
|
||||
openLocalTransportSession,
|
||||
sendLocalTransportMessage,
|
||||
@@ -623,10 +618,7 @@ export function createDaemonCommandHandlers(): Record<string, DesktopCommandHand
|
||||
get_local_daemon_version: () => getLocalDaemonVersion(),
|
||||
install_cli: () => installCli(),
|
||||
get_cli_install_status: () => getCliInstallStatus(),
|
||||
get_skills_status: () => getSkillsStatus(),
|
||||
install_skills: () => installSkills(),
|
||||
update_skills: () => updateSkills(),
|
||||
uninstall_skills: () => uninstallSkills(),
|
||||
...createSkillsCommandHandlers({ controller: getSkillsController() }),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -1,11 +1,49 @@
|
||||
export {
|
||||
autoUpdateInstalledSkills,
|
||||
getSkillsStatus,
|
||||
installSkills,
|
||||
uninstallSkills,
|
||||
updateSkills,
|
||||
type SkillOp,
|
||||
type SkillsState,
|
||||
type SkillsStatus,
|
||||
type SkillTargets,
|
||||
import { app } from "electron";
|
||||
|
||||
import type { SkillsStatus, SkillTargets } from "./operations.js";
|
||||
import {
|
||||
getAgentsSkillsDir,
|
||||
getBundledSkillsDir,
|
||||
getClaudeSkillsDir,
|
||||
getCodexSkillsDir,
|
||||
} from "./paths.js";
|
||||
import { createSkillSelectionStore } from "./selection-store.js";
|
||||
import { createSkillsController, type SkillsController } from "./skills-controller.js";
|
||||
|
||||
export { createSkillsCommandHandlers } from "./skills-commands.js";
|
||||
export { type SkillsController, type SkillsSnapshot } from "./skills-controller.js";
|
||||
export type {
|
||||
SkillOp,
|
||||
SkillSelection,
|
||||
SkillsState,
|
||||
SkillsStatus,
|
||||
SkillTargets,
|
||||
} from "./operations.js";
|
||||
|
||||
let controller: SkillsController | null = null;
|
||||
|
||||
function getSkillTargets(): SkillTargets {
|
||||
return {
|
||||
sourceDir: getBundledSkillsDir(),
|
||||
agentsDir: getAgentsSkillsDir(),
|
||||
claudeDir: getClaudeSkillsDir(),
|
||||
codexDir: getCodexSkillsDir(),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* One controller per process. Startup convergence and the settings commands must
|
||||
* share it, or they are two writers over the same directories again.
|
||||
*/
|
||||
export function getSkillsController(): SkillsController {
|
||||
controller ??= createSkillsController({
|
||||
resolveTargets: getSkillTargets,
|
||||
selectionStore: createSkillSelectionStore({ userDataPath: app.getPath("userData") }),
|
||||
});
|
||||
return controller;
|
||||
}
|
||||
|
||||
/** Startup convergence: keep whatever the user selected in sync with the bundle. */
|
||||
export async function autoUpdateInstalledSkills(): Promise<SkillsStatus> {
|
||||
return getSkillsController().autoUpdate();
|
||||
}
|
||||
|
||||
@@ -1,25 +1,25 @@
|
||||
import { promises as fs } from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("electron", () => ({
|
||||
app: {
|
||||
getPath: vi.fn(() => "/tmp/paseo-user-data"),
|
||||
isPackaged: false,
|
||||
},
|
||||
}));
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
autoUpdateInstalledSkills,
|
||||
getSkillsStatus,
|
||||
installSkills,
|
||||
PASEO_SKILL_NAMES,
|
||||
LEGACY_SKILL_NAMES,
|
||||
type SkillSelection,
|
||||
type SkillTargets,
|
||||
uninstallSkills,
|
||||
updateSkills,
|
||||
} from "./operations";
|
||||
|
||||
const ALL_SKILLS: SkillSelection = { mode: "all" };
|
||||
|
||||
function only(...skills: string[]): SkillSelection {
|
||||
return { mode: "custom", skills };
|
||||
}
|
||||
|
||||
interface Sandbox {
|
||||
root: string;
|
||||
targets: SkillTargets;
|
||||
@@ -85,6 +85,14 @@ async function pathExists(p: string): Promise<boolean> {
|
||||
.catch(() => false);
|
||||
}
|
||||
|
||||
async function installedIn(targets: SkillTargets, name: string): Promise<boolean[]> {
|
||||
return Promise.all(
|
||||
[targets.agentsDir, targets.claudeDir, targets.codexDir].map((dir) =>
|
||||
pathExists(path.join(dir, name)),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
describe("getSkillsStatus", () => {
|
||||
let sandbox: Sandbox;
|
||||
|
||||
@@ -99,7 +107,7 @@ describe("getSkillsStatus", () => {
|
||||
it("returns not-installed with add ops for every bundled skill when nothing is on disk", async () => {
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets);
|
||||
const status = await getSkillsStatus(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status.state).toBe("not-installed");
|
||||
expect(status.ops).toEqual([
|
||||
@@ -108,13 +116,46 @@ describe("getSkillsStatus", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("reports every bundled skill as available", async () => {
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
await writeBundleSkill(sandbox.targets.sourceDir, "paseo-advisor", {
|
||||
"SKILL.md": "advisor-v1",
|
||||
});
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets, only("paseo"));
|
||||
|
||||
expect(status.available).toEqual(["paseo", "paseo-advisor", "paseo-loop"]);
|
||||
});
|
||||
|
||||
it("reports a skill present in only one target as installed", async () => {
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
await writeOnDiskSkill(sandbox.targets.claudeDir, "paseo", { "SKILL.md": "paseo-v1" });
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
// `add` means "missing from at least one target", so it cannot answer
|
||||
// "is there a directory here to delete". `installed` answers that.
|
||||
expect(status.installed).toEqual(["paseo"]);
|
||||
expect(status.ops).toEqual([
|
||||
{ kind: "add", name: "paseo" },
|
||||
{ kind: "add", name: "paseo-loop" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("reports legacy skill directories left on disk as installed", async () => {
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
await writeOnDiskSkill(sandbox.targets.agentsDir, "paseo-chat", { "SKILL.md": "chat-old" });
|
||||
|
||||
expect((await getSkillsStatus(sandbox.targets, ALL_SKILLS)).installed).toEqual(["paseo-chat"]);
|
||||
});
|
||||
|
||||
it("returns not-installed when only user-personal skill dirs exist (the live bug)", async () => {
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
for (const name of ["unslop", "tdd", "devbox"]) {
|
||||
await writeOnDiskSkill(sandbox.targets.agentsDir, name, { "SKILL.md": `user-${name}` });
|
||||
}
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets);
|
||||
const status = await getSkillsStatus(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status.state).toBe("not-installed");
|
||||
expect(status.ops).toEqual([
|
||||
@@ -128,9 +169,14 @@ describe("getSkillsStatus", () => {
|
||||
await writeOnDiskSkillToAllTargets(sandbox.targets, "paseo", { "SKILL.md": "paseo-v1" });
|
||||
await writeOnDiskSkillToAllTargets(sandbox.targets, "paseo-loop", { "SKILL.md": "loop-v1" });
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets);
|
||||
const status = await getSkillsStatus(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status).toEqual({ state: "up-to-date", ops: [] });
|
||||
expect(status).toEqual({
|
||||
state: "up-to-date",
|
||||
ops: [],
|
||||
available: ["paseo", "paseo-loop"],
|
||||
installed: ["paseo", "paseo-loop"],
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores user-added files inside current managed skill dirs in every target", async () => {
|
||||
@@ -150,9 +196,14 @@ describe("getSkillsStatus", () => {
|
||||
"hooks/guard.sh": "user guard",
|
||||
});
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets);
|
||||
const status = await getSkillsStatus(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status).toEqual({ state: "up-to-date", ops: [] });
|
||||
expect(status).toEqual({
|
||||
state: "up-to-date",
|
||||
ops: [],
|
||||
available: ["paseo", "paseo-loop"],
|
||||
installed: ["paseo", "paseo-loop"],
|
||||
});
|
||||
});
|
||||
|
||||
it("returns drift with a single update op when one bundled file diverges", async () => {
|
||||
@@ -162,7 +213,7 @@ describe("getSkillsStatus", () => {
|
||||
await writeOnDiskSkill(sandbox.targets.codexDir, "paseo", { "SKILL.md": "paseo-v1" });
|
||||
await writeOnDiskSkillToAllTargets(sandbox.targets, "paseo-loop", { "SKILL.md": "loop-v1" });
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets);
|
||||
const status = await getSkillsStatus(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status.state).toBe("drift");
|
||||
expect(status.ops).toEqual([{ kind: "update", name: "paseo" }]);
|
||||
@@ -177,7 +228,7 @@ describe("getSkillsStatus", () => {
|
||||
await writeOnDiskSkill(sandbox.targets.codexDir, "paseo", { "SKILL.md": "paseo-v1" });
|
||||
await writeOnDiskSkill(sandbox.targets.codexDir, "paseo-loop", { "SKILL.md": "loop-v1" });
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets);
|
||||
const status = await getSkillsStatus(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status.state).toBe("drift");
|
||||
expect(status.ops).toEqual([{ kind: "update", name: "paseo" }]);
|
||||
@@ -187,7 +238,7 @@ describe("getSkillsStatus", () => {
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
await writeOnDiskSkillToAllTargets(sandbox.targets, "paseo", { "SKILL.md": "paseo-v1" });
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets);
|
||||
const status = await getSkillsStatus(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status.state).toBe("drift");
|
||||
expect(status.ops).toEqual([{ kind: "add", name: "paseo-loop" }]);
|
||||
@@ -199,7 +250,7 @@ describe("getSkillsStatus", () => {
|
||||
await writeOnDiskSkillToAllTargets(sandbox.targets, "paseo-loop", { "SKILL.md": "loop-v1" });
|
||||
await writeOnDiskSkill(sandbox.targets.agentsDir, "paseo-chat", { "SKILL.md": "chat-old" });
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets);
|
||||
const status = await getSkillsStatus(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status.state).toBe("drift");
|
||||
expect(status.ops).toEqual([{ kind: "delete", name: "paseo-chat" }]);
|
||||
@@ -212,7 +263,7 @@ describe("getSkillsStatus", () => {
|
||||
await writeOnDiskSkill(sandbox.targets.codexDir, "paseo", { "SKILL.md": "paseo-v1" });
|
||||
await writeOnDiskSkill(sandbox.targets.agentsDir, "paseo-chat", { "SKILL.md": "chat-old" });
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets);
|
||||
const status = await getSkillsStatus(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status.state).toBe("drift");
|
||||
expect(status.ops).toEqual([
|
||||
@@ -223,6 +274,116 @@ describe("getSkillsStatus", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("custom skill selection", () => {
|
||||
let sandbox: Sandbox;
|
||||
|
||||
beforeEach(async () => {
|
||||
sandbox = await makeSandbox();
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
await writeBundleSkill(sandbox.targets.sourceDir, "paseo-advisor", {
|
||||
"SKILL.md": "advisor-v1",
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await fs.rm(sandbox.root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("installs only the selected skills", async () => {
|
||||
const status = await installSkills(sandbox.targets, only("paseo", "paseo-loop"));
|
||||
|
||||
expect(status).toEqual({
|
||||
state: "up-to-date",
|
||||
ops: [],
|
||||
available: ["paseo", "paseo-advisor", "paseo-loop"],
|
||||
installed: ["paseo", "paseo-loop"],
|
||||
});
|
||||
expect(await installedIn(sandbox.targets, "paseo")).toEqual([true, true, true]);
|
||||
expect(await installedIn(sandbox.targets, "paseo-loop")).toEqual([true, true, true]);
|
||||
expect(await installedIn(sandbox.targets, "paseo-advisor")).toEqual([false, false, false]);
|
||||
});
|
||||
|
||||
it("reports up-to-date while an unselected bundled skill is absent", async () => {
|
||||
await installSkills(sandbox.targets, only("paseo"));
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets, only("paseo"));
|
||||
|
||||
expect(status).toEqual({
|
||||
state: "up-to-date",
|
||||
ops: [],
|
||||
available: ["paseo", "paseo-advisor", "paseo-loop"],
|
||||
installed: ["paseo"],
|
||||
});
|
||||
});
|
||||
|
||||
it("removes a previously installed skill once it leaves the selection", async () => {
|
||||
await installSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
const status = await installSkills(sandbox.targets, only("paseo"));
|
||||
|
||||
expect(status.state).toBe("up-to-date");
|
||||
expect(await installedIn(sandbox.targets, "paseo")).toEqual([true, true, true]);
|
||||
expect(await installedIn(sandbox.targets, "paseo-loop")).toEqual([false, false, false]);
|
||||
expect(await installedIn(sandbox.targets, "paseo-advisor")).toEqual([false, false, false]);
|
||||
});
|
||||
|
||||
it("reports a delete op for a deselected skill before it is applied", async () => {
|
||||
await installSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
const status = await getSkillsStatus(sandbox.targets, only("paseo", "paseo-advisor"));
|
||||
|
||||
expect(status.state).toBe("drift");
|
||||
expect(status.ops).toEqual([{ kind: "delete", name: "paseo-loop" }]);
|
||||
});
|
||||
|
||||
it("leaves unrelated user skills untouched", async () => {
|
||||
await writeOnDiskSkill(sandbox.targets.agentsDir, "unslop", { "SKILL.md": "user-unslop" });
|
||||
|
||||
await installSkills(sandbox.targets, only("paseo"));
|
||||
|
||||
expect(
|
||||
await fs.readFile(path.join(sandbox.targets.agentsDir, "unslop", "SKILL.md"), "utf-8"),
|
||||
).toBe("user-unslop");
|
||||
});
|
||||
|
||||
it("treats an empty custom selection as no managed skills installed", async () => {
|
||||
await installSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
const status = await installSkills(sandbox.targets, only());
|
||||
|
||||
expect(status).toEqual({
|
||||
state: "not-installed",
|
||||
ops: [],
|
||||
available: ["paseo", "paseo-advisor", "paseo-loop"],
|
||||
installed: [],
|
||||
});
|
||||
expect(await installedIn(sandbox.targets, "paseo")).toEqual([false, false, false]);
|
||||
expect(await installedIn(sandbox.targets, "paseo-loop")).toEqual([false, false, false]);
|
||||
});
|
||||
|
||||
it("ignores selected names that the bundle does not ship", async () => {
|
||||
const status = await installSkills(sandbox.targets, only("paseo", "not-a-skill"));
|
||||
|
||||
expect(status).toEqual({
|
||||
state: "up-to-date",
|
||||
ops: [],
|
||||
available: ["paseo", "paseo-advisor", "paseo-loop"],
|
||||
installed: ["paseo"],
|
||||
});
|
||||
expect(await installedIn(sandbox.targets, "not-a-skill")).toEqual([false, false, false]);
|
||||
});
|
||||
|
||||
it("still deletes legacy skill names that are not selectable", async () => {
|
||||
await writeOnDiskSkillToAllTargets(sandbox.targets, "paseo-orchestrator", {
|
||||
"SKILL.md": "orchestrator-old",
|
||||
});
|
||||
|
||||
await installSkills(sandbox.targets, only("paseo"));
|
||||
|
||||
expect(await installedIn(sandbox.targets, "paseo-orchestrator")).toEqual([false, false, false]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("installSkills / updateSkills", () => {
|
||||
let sandbox: Sandbox;
|
||||
|
||||
@@ -238,9 +399,14 @@ describe("installSkills / updateSkills", () => {
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
await writeOnDiskSkill(sandbox.targets.agentsDir, "unslop", { "SKILL.md": "user-unslop" });
|
||||
|
||||
const status = await installSkills(sandbox.targets);
|
||||
const status = await installSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status).toEqual({ state: "up-to-date", ops: [] });
|
||||
expect(status).toEqual({
|
||||
state: "up-to-date",
|
||||
ops: [],
|
||||
available: ["paseo", "paseo-loop"],
|
||||
installed: ["paseo", "paseo-loop"],
|
||||
});
|
||||
for (const name of ["paseo", "paseo-loop"]) {
|
||||
expect(
|
||||
await fs.readFile(path.join(sandbox.targets.agentsDir, name, "SKILL.md"), "utf-8"),
|
||||
@@ -255,16 +421,21 @@ describe("installSkills / updateSkills", () => {
|
||||
).toBe("user-unslop");
|
||||
});
|
||||
|
||||
it("converges to up-to-date when state has missing + edited + legacy skills", async () => {
|
||||
it("repairs missing and edited skills without deleting a legacy directory", async () => {
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
await writeOnDiskSkill(sandbox.targets.agentsDir, "paseo", { "SKILL.md": "stale" });
|
||||
await writeOnDiskSkill(sandbox.targets.agentsDir, "paseo-chat", { "SKILL.md": "chat-old" });
|
||||
await writeOnDiskSkill(sandbox.targets.claudeDir, "paseo-chat", { "SKILL.md": "chat-old" });
|
||||
await writeOnDiskSkill(sandbox.targets.codexDir, "paseo-chat", { "SKILL.md": "chat-old" });
|
||||
|
||||
const status = await updateSkills(sandbox.targets);
|
||||
const status = await updateSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status).toEqual({ state: "up-to-date", ops: [] });
|
||||
expect(status).toEqual({
|
||||
state: "drift",
|
||||
ops: [{ kind: "delete", name: "paseo-chat" }],
|
||||
available: ["paseo", "paseo-loop"],
|
||||
installed: ["paseo", "paseo-chat", "paseo-loop"],
|
||||
});
|
||||
expect(
|
||||
await fs.readFile(path.join(sandbox.targets.agentsDir, "paseo", "SKILL.md"), "utf-8"),
|
||||
).toBe("paseo-v1");
|
||||
@@ -276,7 +447,7 @@ describe("installSkills / updateSkills", () => {
|
||||
sandbox.targets.claudeDir,
|
||||
sandbox.targets.codexDir,
|
||||
]) {
|
||||
expect(await pathExists(path.join(dir, "paseo-chat"))).toBe(false);
|
||||
expect(await pathExists(path.join(dir, "paseo-chat"))).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -297,10 +468,15 @@ describe("installSkills / updateSkills", () => {
|
||||
"prompts/local.md": "codex prompt",
|
||||
});
|
||||
|
||||
const status = await updateSkills(sandbox.targets);
|
||||
const status = await updateSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status).toEqual({ state: "up-to-date", ops: [] });
|
||||
expect(await getSkillsStatus(sandbox.targets)).toEqual({ state: "up-to-date", ops: [] });
|
||||
expect(status.state).toBe("up-to-date");
|
||||
expect(await getSkillsStatus(sandbox.targets, ALL_SKILLS)).toEqual({
|
||||
state: "up-to-date",
|
||||
ops: [],
|
||||
available: ["paseo", "paseo-loop"],
|
||||
installed: ["paseo", "paseo-loop"],
|
||||
});
|
||||
expect(
|
||||
await fs.readFile(
|
||||
path.join(sandbox.targets.agentsDir, "paseo", "hooks", "guard.sh"),
|
||||
@@ -329,9 +505,9 @@ describe("installSkills / updateSkills", () => {
|
||||
await writeOnDiskSkill(sandbox.targets.codexDir, "paseo", { "SKILL.md": "paseo-v1" });
|
||||
await writeOnDiskSkill(sandbox.targets.codexDir, "paseo-loop", { "SKILL.md": "loop-v1" });
|
||||
|
||||
const status = await updateSkills(sandbox.targets);
|
||||
const status = await updateSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status).toEqual({ state: "up-to-date", ops: [] });
|
||||
expect(status.state).toBe("up-to-date");
|
||||
expect(
|
||||
await fs.readFile(path.join(sandbox.targets.claudeDir, "paseo-loop", "SKILL.md"), "utf-8"),
|
||||
).toBe("loop-v1");
|
||||
@@ -344,10 +520,10 @@ describe("installSkills / updateSkills", () => {
|
||||
"hooks/guard.sh": "user guard",
|
||||
});
|
||||
|
||||
const status = await autoUpdateInstalledSkills(sandbox.targets);
|
||||
const status = await autoUpdateInstalledSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status).toEqual({ state: "up-to-date", ops: [] });
|
||||
expect(await getSkillsStatus(sandbox.targets)).toEqual({ state: "up-to-date", ops: [] });
|
||||
expect(status.state).toBe("up-to-date");
|
||||
expect((await getSkillsStatus(sandbox.targets, ALL_SKILLS)).state).toBe("up-to-date");
|
||||
expect(
|
||||
await fs.readFile(path.join(sandbox.targets.agentsDir, "paseo", "SKILL.md"), "utf-8"),
|
||||
).toBe("paseo-v1");
|
||||
@@ -362,7 +538,7 @@ describe("installSkills / updateSkills", () => {
|
||||
it("does not auto-install skills on a clean machine", async () => {
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
|
||||
const status = await autoUpdateInstalledSkills(sandbox.targets);
|
||||
const status = await autoUpdateInstalledSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status).toEqual({
|
||||
state: "not-installed",
|
||||
@@ -370,20 +546,20 @@ describe("installSkills / updateSkills", () => {
|
||||
{ kind: "add", name: "paseo" },
|
||||
{ kind: "add", name: "paseo-loop" },
|
||||
],
|
||||
available: ["paseo", "paseo-loop"],
|
||||
installed: [],
|
||||
});
|
||||
expect(await pathExists(path.join(sandbox.targets.agentsDir, "paseo"))).toBe(false);
|
||||
expect(await pathExists(path.join(sandbox.targets.claudeDir, "paseo"))).toBe(false);
|
||||
expect(await pathExists(path.join(sandbox.targets.codexDir, "paseo"))).toBe(false);
|
||||
expect(await installedIn(sandbox.targets, "paseo")).toEqual([false, false, false]);
|
||||
});
|
||||
|
||||
it("is idempotent — running install twice keeps state at up-to-date", async () => {
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
|
||||
const first = await installSkills(sandbox.targets);
|
||||
const second = await installSkills(sandbox.targets);
|
||||
const first = await installSkills(sandbox.targets, ALL_SKILLS);
|
||||
const second = await installSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(first).toEqual({ state: "up-to-date", ops: [] });
|
||||
expect(second).toEqual({ state: "up-to-date", ops: [] });
|
||||
expect(first.state).toBe("up-to-date");
|
||||
expect(second.state).toBe("up-to-date");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -400,18 +576,16 @@ describe("uninstallSkills", () => {
|
||||
|
||||
it("removes every Paseo skill from all three targets and preserves user dirs", async () => {
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
await installSkills(sandbox.targets);
|
||||
await installSkills(sandbox.targets, ALL_SKILLS);
|
||||
for (const name of ["unslop", "tdd", "devbox"]) {
|
||||
await writeOnDiskSkill(sandbox.targets.agentsDir, name, { "SKILL.md": `user-${name}` });
|
||||
}
|
||||
|
||||
const status = await uninstallSkills(sandbox.targets);
|
||||
const status = await uninstallSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status.state).toBe("not-installed");
|
||||
for (const name of PASEO_SKILL_NAMES) {
|
||||
expect(await pathExists(path.join(sandbox.targets.agentsDir, name))).toBe(false);
|
||||
expect(await pathExists(path.join(sandbox.targets.claudeDir, name))).toBe(false);
|
||||
expect(await pathExists(path.join(sandbox.targets.codexDir, name))).toBe(false);
|
||||
for (const name of ["paseo", "paseo-loop", ...LEGACY_SKILL_NAMES]) {
|
||||
expect(await installedIn(sandbox.targets, name)).toEqual([false, false, false]);
|
||||
}
|
||||
for (const name of ["unslop", "tdd", "devbox"]) {
|
||||
expect(
|
||||
@@ -423,7 +597,7 @@ describe("uninstallSkills", () => {
|
||||
it("is a no-op when nothing is installed", async () => {
|
||||
await writeCurrentBundle(sandbox.targets.sourceDir);
|
||||
|
||||
const status = await uninstallSkills(sandbox.targets);
|
||||
const status = await uninstallSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status.state).toBe("not-installed");
|
||||
});
|
||||
@@ -438,15 +612,9 @@ describe("uninstallSkills", () => {
|
||||
await writeOnDiskSkill(dir, "paseo-chat", { "SKILL.md": "chat-old" });
|
||||
}
|
||||
|
||||
const status = await uninstallSkills(sandbox.targets);
|
||||
const status = await uninstallSkills(sandbox.targets, ALL_SKILLS);
|
||||
|
||||
expect(status.state).toBe("not-installed");
|
||||
for (const dir of [
|
||||
sandbox.targets.agentsDir,
|
||||
sandbox.targets.claudeDir,
|
||||
sandbox.targets.codexDir,
|
||||
]) {
|
||||
expect(await pathExists(path.join(dir, "paseo-chat"))).toBe(false);
|
||||
}
|
||||
expect(await installedIn(sandbox.targets, "paseo-chat")).toEqual([false, false, false]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,12 +1,6 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import { promises as fs } from "node:fs";
|
||||
import path from "node:path";
|
||||
import {
|
||||
getAgentsSkillsDir,
|
||||
getBundledSkillsDir,
|
||||
getClaudeSkillsDir,
|
||||
getCodexSkillsDir,
|
||||
} from "./paths.js";
|
||||
import { listFilesRecursive, removeSkill, syncSkills } from "./sync.js";
|
||||
|
||||
export type SkillsState = "not-installed" | "up-to-date" | "drift";
|
||||
@@ -16,9 +10,20 @@ export type SkillOp =
|
||||
| { kind: "update"; name: string }
|
||||
| { kind: "delete"; name: string };
|
||||
|
||||
/** What the user asked to have installed. `all` follows the bundle as it grows. */
|
||||
export type SkillSelection = { mode: "all" } | { mode: "custom"; skills: readonly string[] };
|
||||
|
||||
export interface SkillsStatus {
|
||||
state: SkillsState;
|
||||
ops: SkillOp[];
|
||||
/** Every skill the bundle currently ships, sorted. The selectable catalog. */
|
||||
available: string[];
|
||||
/**
|
||||
* Managed skills with a directory in at least one agent home, sorted. An `add`
|
||||
* op means "missing from at least one target", so it cannot answer whether
|
||||
* there is anything on disk to delete — this can.
|
||||
*/
|
||||
installed: string[];
|
||||
}
|
||||
|
||||
export interface SkillTargets {
|
||||
@@ -28,14 +33,10 @@ export interface SkillTargets {
|
||||
codexDir: string;
|
||||
}
|
||||
|
||||
export const PASEO_SKILL_NAMES = [
|
||||
"paseo",
|
||||
"paseo-advisor",
|
||||
// Names the bundle used to ship. They are never selectable, but every scan still
|
||||
// covers them so an older install's copies get cleaned up.
|
||||
export const LEGACY_SKILL_NAMES = [
|
||||
"paseo-chat",
|
||||
"paseo-committee",
|
||||
"paseo-handoff",
|
||||
"paseo-loop",
|
||||
// Keep removed bundle names here so auto-update deletes stale installed copies.
|
||||
"paseo-epic",
|
||||
"paseo-orchestrate",
|
||||
"paseo-orchestrator",
|
||||
@@ -44,13 +45,35 @@ export const PASEO_SKILL_NAMES = [
|
||||
type SkillFiles = Map<string, string>;
|
||||
type TargetSkills = Map<string, SkillFiles>;
|
||||
|
||||
function resolveSkillTargets(): SkillTargets {
|
||||
return {
|
||||
sourceDir: getBundledSkillsDir(),
|
||||
agentsDir: getAgentsSkillsDir(),
|
||||
claudeDir: getClaudeSkillsDir(),
|
||||
codexDir: getCodexSkillsDir(),
|
||||
};
|
||||
/**
|
||||
* The bundle directory is the catalog. Reading it instead of a hardcoded list is
|
||||
* what makes `all` pick up skills added in a later release with no code change.
|
||||
*/
|
||||
async function listBundledSkills(sourceDir: string): Promise<string[]> {
|
||||
const entries = await fs.readdir(sourceDir, { withFileTypes: true }).catch(() => []);
|
||||
return entries
|
||||
.filter((entry) => entry.isDirectory())
|
||||
.map((entry) => entry.name)
|
||||
.sort(compareStrings);
|
||||
}
|
||||
|
||||
/** Every name Paseo owns on disk: what it ships now plus what it used to ship. */
|
||||
function managedSkillNames(available: readonly string[]): string[] {
|
||||
return [...new Set([...available, ...LEGACY_SKILL_NAMES])].sort(compareStrings);
|
||||
}
|
||||
|
||||
/** The names a convergence may create, replace, or delete. */
|
||||
export async function listManagedSkillNames(sourceDir: string): Promise<string[]> {
|
||||
return managedSkillNames(await listBundledSkills(sourceDir));
|
||||
}
|
||||
|
||||
function resolveDesiredSkills(
|
||||
selection: SkillSelection,
|
||||
available: readonly string[],
|
||||
): Set<string> {
|
||||
if (selection.mode === "all") return new Set(available);
|
||||
const chosen = new Set(selection.skills);
|
||||
return new Set(available.filter((name) => chosen.has(name)));
|
||||
}
|
||||
|
||||
async function hashSkillDir(skillDir: string): Promise<SkillFiles | null> {
|
||||
@@ -67,19 +90,24 @@ async function hashSkillDir(skillDir: string): Promise<SkillFiles | null> {
|
||||
return files;
|
||||
}
|
||||
|
||||
async function hashSkills(rootDir: string): Promise<Map<string, SkillFiles>> {
|
||||
const out = new Map<string, SkillFiles>();
|
||||
for (const name of PASEO_SKILL_NAMES) {
|
||||
async function hashSkills(rootDir: string, names: readonly string[]): Promise<TargetSkills> {
|
||||
const out: TargetSkills = new Map();
|
||||
for (const name of names) {
|
||||
const files = await hashSkillDir(path.join(rootDir, name));
|
||||
if (files !== null) out.set(name, files);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function diff(bundle: TargetSkills, disks: readonly TargetSkills[]): SkillOp[] {
|
||||
function diff(
|
||||
bundle: TargetSkills,
|
||||
disks: readonly TargetSkills[],
|
||||
names: readonly string[],
|
||||
desired: ReadonlySet<string>,
|
||||
): SkillOp[] {
|
||||
const ops: SkillOp[] = [];
|
||||
for (const name of PASEO_SKILL_NAMES) {
|
||||
const b = bundle.get(name);
|
||||
for (const name of names) {
|
||||
const b = desired.has(name) ? bundle.get(name) : undefined;
|
||||
const targetFiles = disks.map((disk) => disk.get(name));
|
||||
const installedTargets = targetFiles.filter(
|
||||
(files): files is SkillFiles => files !== undefined,
|
||||
@@ -101,6 +129,10 @@ function hasInstalledPaseoSkill(disks: readonly TargetSkills[]): boolean {
|
||||
return disks.some((disk) => disk.size > 0);
|
||||
}
|
||||
|
||||
function installedSkillNames(disks: readonly TargetSkills[], names: readonly string[]): string[] {
|
||||
return names.filter((name) => disks.some((disk) => disk.has(name)));
|
||||
}
|
||||
|
||||
function bundleFilesMatch(bundle: SkillFiles, disk: SkillFiles): boolean {
|
||||
for (const [rel, sha] of bundle) {
|
||||
if (disk.get(rel) !== sha) return false;
|
||||
@@ -118,27 +150,33 @@ function compareStrings(a: string, b: string): number {
|
||||
return 0;
|
||||
}
|
||||
|
||||
export async function getSkillsStatus(targets?: SkillTargets): Promise<SkillsStatus> {
|
||||
const t = targets ?? resolveSkillTargets();
|
||||
export async function getSkillsStatus(
|
||||
targets: SkillTargets,
|
||||
selection: SkillSelection,
|
||||
): Promise<SkillsStatus> {
|
||||
const available = await listBundledSkills(targets.sourceDir);
|
||||
const names = managedSkillNames(available);
|
||||
const [bundle, agentsDisk, claudeDisk, codexDisk] = await Promise.all([
|
||||
hashSkills(t.sourceDir),
|
||||
hashSkills(t.agentsDir),
|
||||
hashSkills(t.claudeDir),
|
||||
hashSkills(t.codexDir),
|
||||
hashSkills(targets.sourceDir, available),
|
||||
hashSkills(targets.agentsDir, names),
|
||||
hashSkills(targets.claudeDir, names),
|
||||
hashSkills(targets.codexDir, names),
|
||||
]);
|
||||
const disks = [agentsDisk, claudeDisk, codexDisk];
|
||||
const ops = diff(bundle, disks);
|
||||
const ops = diff(bundle, disks, names, resolveDesiredSkills(selection, available));
|
||||
const installed = installedSkillNames(disks, names);
|
||||
|
||||
if (!hasInstalledPaseoSkill(disks)) return { state: "not-installed", ops };
|
||||
if (ops.length === 0) return { state: "up-to-date", ops };
|
||||
return { state: "drift", ops };
|
||||
if (!hasInstalledPaseoSkill(disks)) return { state: "not-installed", ops, available, installed };
|
||||
if (ops.length === 0) return { state: "up-to-date", ops, available, installed };
|
||||
return { state: "drift", ops, available, installed };
|
||||
}
|
||||
|
||||
async function applySkills(
|
||||
targets: SkillTargets,
|
||||
selection: SkillSelection,
|
||||
initialStatus?: SkillsStatus,
|
||||
): Promise<SkillsStatus> {
|
||||
const status = initialStatus ?? (await getSkillsStatus(targets));
|
||||
const status = initialStatus ?? (await getSkillsStatus(targets, selection));
|
||||
|
||||
const writes = status.ops
|
||||
.filter((op) => op.kind === "add" || op.kind === "update")
|
||||
@@ -162,32 +200,52 @@ async function applySkills(
|
||||
});
|
||||
}
|
||||
|
||||
return getSkillsStatus(targets);
|
||||
return getSkillsStatus(targets, selection);
|
||||
}
|
||||
|
||||
export async function installSkills(targets?: SkillTargets): Promise<SkillsStatus> {
|
||||
return applySkills(targets ?? resolveSkillTargets());
|
||||
export async function installSkills(
|
||||
targets: SkillTargets,
|
||||
selection: SkillSelection,
|
||||
/** Apply exactly this plan instead of rescanning, so a confirmed plan is the applied plan. */
|
||||
plan?: SkillsStatus,
|
||||
): Promise<SkillsStatus> {
|
||||
return applySkills(targets, selection, plan);
|
||||
}
|
||||
|
||||
export async function updateSkills(targets?: SkillTargets): Promise<SkillsStatus> {
|
||||
return applySkills(targets ?? resolveSkillTargets());
|
||||
export async function updateSkills(
|
||||
targets: SkillTargets,
|
||||
selection: SkillSelection,
|
||||
): Promise<SkillsStatus> {
|
||||
const status = await getSkillsStatus(targets, selection);
|
||||
return applySkills(targets, selection, nonDestructivePlan(status));
|
||||
}
|
||||
|
||||
export async function autoUpdateInstalledSkills(targets?: SkillTargets): Promise<SkillsStatus> {
|
||||
const t = targets ?? resolveSkillTargets();
|
||||
const status = await getSkillsStatus(t);
|
||||
function nonDestructivePlan(status: SkillsStatus): SkillsStatus {
|
||||
return { ...status, ops: status.ops.filter((op) => op.kind !== "delete") };
|
||||
}
|
||||
|
||||
export async function autoUpdateInstalledSkills(
|
||||
targets: SkillTargets,
|
||||
selection: SkillSelection,
|
||||
): Promise<SkillsStatus> {
|
||||
const status = await getSkillsStatus(targets, selection);
|
||||
if (status.state !== "drift") return status;
|
||||
return applySkills(t, status);
|
||||
// Automatic maintenance may repair selected skills, but removal is an
|
||||
// interactive operation because managed directories can contain user files.
|
||||
return applySkills(targets, selection, nonDestructivePlan(status));
|
||||
}
|
||||
|
||||
export async function uninstallSkills(targets?: SkillTargets): Promise<SkillsStatus> {
|
||||
const t = targets ?? resolveSkillTargets();
|
||||
for (const name of PASEO_SKILL_NAMES) {
|
||||
export async function uninstallSkills(
|
||||
targets: SkillTargets,
|
||||
selection: SkillSelection,
|
||||
): Promise<SkillsStatus> {
|
||||
const available = await listBundledSkills(targets.sourceDir);
|
||||
for (const name of managedSkillNames(available)) {
|
||||
await removeSkill(name, {
|
||||
agentsDir: t.agentsDir,
|
||||
claudeDir: t.claudeDir,
|
||||
codexDir: t.codexDir,
|
||||
agentsDir: targets.agentsDir,
|
||||
claudeDir: targets.claudeDir,
|
||||
codexDir: targets.codexDir,
|
||||
});
|
||||
}
|
||||
return getSkillsStatus(t);
|
||||
return getSkillsStatus(targets, selection);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,150 @@
|
||||
import { mkdtemp, readdir, readFile, rm, writeFile } from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
|
||||
import { createSkillSelectionStore } from "./selection-store";
|
||||
|
||||
const directories = new Set<string>();
|
||||
|
||||
async function createTempUserDataDir(): Promise<string> {
|
||||
const dir = await mkdtemp(path.join(os.tmpdir(), "paseo-skill-selection-"));
|
||||
directories.add(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
function selectionFilePath(userDataPath: string): string {
|
||||
return path.join(userDataPath, "skill-selection.json");
|
||||
}
|
||||
|
||||
async function seedSelectionFile(userDataPath: string, raw: string): Promise<void> {
|
||||
await writeFile(selectionFilePath(userDataPath), raw, "utf8");
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
for (const dir of directories) {
|
||||
await rm(dir, { recursive: true, force: true });
|
||||
}
|
||||
directories.clear();
|
||||
});
|
||||
|
||||
describe("createSkillSelectionStore", () => {
|
||||
it("reports all skills when nothing has ever been saved", async () => {
|
||||
const userDataPath = await createTempUserDataDir();
|
||||
const store = createSkillSelectionStore({ userDataPath });
|
||||
|
||||
expect(await store.get()).toEqual({ mode: "all" });
|
||||
expect(await readdir(userDataPath)).toEqual([]);
|
||||
});
|
||||
|
||||
it("round-trips a custom selection through a fresh store", async () => {
|
||||
const userDataPath = await createTempUserDataDir();
|
||||
|
||||
const saved = await createSkillSelectionStore({ userDataPath }).set({
|
||||
mode: "custom",
|
||||
skills: ["paseo", "paseo-loop"],
|
||||
});
|
||||
const reloaded = await createSkillSelectionStore({ userDataPath }).get();
|
||||
|
||||
expect(saved).toEqual({ mode: "custom", skills: ["paseo", "paseo-loop"] });
|
||||
expect(reloaded).toEqual({ mode: "custom", skills: ["paseo", "paseo-loop"] });
|
||||
expect(await readdir(userDataPath)).toEqual(["skill-selection.json"]);
|
||||
});
|
||||
|
||||
it("sorts, de-duplicates, and drops non-string names before persisting", async () => {
|
||||
const userDataPath = await createTempUserDataDir();
|
||||
const store = createSkillSelectionStore({ userDataPath });
|
||||
|
||||
const saved = await store.set({
|
||||
mode: "custom",
|
||||
skills: ["paseo-loop", "paseo", "paseo-loop", " ", 7, null],
|
||||
});
|
||||
|
||||
expect(saved).toEqual({ mode: "custom", skills: ["paseo", "paseo-loop"] });
|
||||
expect(JSON.parse(await readFile(selectionFilePath(userDataPath), "utf8"))).toEqual({
|
||||
version: 1,
|
||||
selection: { mode: "custom", skills: ["paseo", "paseo-loop"] },
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps an empty custom selection as a deliberate choice", async () => {
|
||||
const userDataPath = await createTempUserDataDir();
|
||||
|
||||
await createSkillSelectionStore({ userDataPath }).set({ mode: "custom", skills: [] });
|
||||
|
||||
expect(await createSkillSelectionStore({ userDataPath }).get()).toEqual({
|
||||
mode: "custom",
|
||||
skills: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("switches back to all and forgets the previous custom names", async () => {
|
||||
const userDataPath = await createTempUserDataDir();
|
||||
const store = createSkillSelectionStore({ userDataPath });
|
||||
await store.set({ mode: "custom", skills: ["paseo"] });
|
||||
|
||||
const saved = await store.set({ mode: "all" });
|
||||
|
||||
expect(saved).toEqual({ mode: "all" });
|
||||
expect(await createSkillSelectionStore({ userDataPath }).get()).toEqual({ mode: "all" });
|
||||
});
|
||||
|
||||
it("falls back to all when the saved file is not valid JSON", async () => {
|
||||
const userDataPath = await createTempUserDataDir();
|
||||
await seedSelectionFile(userDataPath, "{ not valid json");
|
||||
|
||||
expect(await createSkillSelectionStore({ userDataPath }).get()).toEqual({ mode: "all" });
|
||||
});
|
||||
|
||||
it("falls back to all when the saved mode is unknown", async () => {
|
||||
const userDataPath = await createTempUserDataDir();
|
||||
await seedSelectionFile(
|
||||
userDataPath,
|
||||
JSON.stringify({ version: 1, selection: { mode: "some-future-mode", skills: ["paseo"] } }),
|
||||
);
|
||||
|
||||
expect(await createSkillSelectionStore({ userDataPath }).get()).toEqual({ mode: "all" });
|
||||
});
|
||||
|
||||
it("normalizes a custom selection that was hand-edited on disk", async () => {
|
||||
const userDataPath = await createTempUserDataDir();
|
||||
await seedSelectionFile(
|
||||
userDataPath,
|
||||
JSON.stringify({
|
||||
version: 1,
|
||||
selection: { mode: "custom", skills: ["paseo-loop", 3, "paseo", "paseo"] },
|
||||
}),
|
||||
);
|
||||
|
||||
expect(await createSkillSelectionStore({ userDataPath }).get()).toEqual({
|
||||
mode: "custom",
|
||||
skills: ["paseo", "paseo-loop"],
|
||||
});
|
||||
});
|
||||
|
||||
it("treats a custom selection with no usable skills array as an empty selection", async () => {
|
||||
const userDataPath = await createTempUserDataDir();
|
||||
await seedSelectionFile(
|
||||
userDataPath,
|
||||
JSON.stringify({ version: 1, selection: { mode: "custom" } }),
|
||||
);
|
||||
|
||||
expect(await createSkillSelectionStore({ userDataPath }).get()).toEqual({
|
||||
mode: "custom",
|
||||
skills: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("leaves no temp files behind when saves overlap", async () => {
|
||||
const userDataPath = await createTempUserDataDir();
|
||||
const store = createSkillSelectionStore({ userDataPath });
|
||||
|
||||
await Promise.all([
|
||||
store.set({ mode: "custom", skills: ["paseo"] }),
|
||||
store.set({ mode: "all" }),
|
||||
store.set({ mode: "custom", skills: ["paseo-loop"] }),
|
||||
]);
|
||||
|
||||
expect(await readdir(userDataPath)).toEqual(["skill-selection.json"]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,101 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { mkdir, readFile, rename, writeFile } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
|
||||
import type { SkillSelection } from "./operations.js";
|
||||
|
||||
export interface SkillSelectionStore {
|
||||
get(): Promise<SkillSelection>;
|
||||
set(selection: unknown): Promise<SkillSelection>;
|
||||
}
|
||||
|
||||
interface PersistedSkillSelectionDocument {
|
||||
version: 1;
|
||||
selection: SkillSelection;
|
||||
}
|
||||
|
||||
const SKILL_SELECTION_FILENAME = "skill-selection.json";
|
||||
|
||||
// Installs that predate the picker have no file. They keep every bundled skill,
|
||||
// which is what they had before, and what `all` keeps meaning as the bundle grows.
|
||||
const DEFAULT_SKILL_SELECTION: SkillSelection = { mode: "all" };
|
||||
|
||||
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === "object" && value !== null && !Array.isArray(value);
|
||||
}
|
||||
|
||||
function isNodeError(error: unknown): error is NodeJS.ErrnoException {
|
||||
return error instanceof Error;
|
||||
}
|
||||
|
||||
export function coerceSkillNames(value: unknown): string[] {
|
||||
if (!Array.isArray(value)) return [];
|
||||
const names = value
|
||||
.filter((entry): entry is string => typeof entry === "string")
|
||||
.map((entry) => entry.trim())
|
||||
.filter((entry) => entry.length > 0);
|
||||
return [...new Set(names)].sort();
|
||||
}
|
||||
|
||||
/**
|
||||
* The one place an unvalidated selection becomes a `SkillSelection`. Whether a
|
||||
* name is actually in the bundle is not decided here — the catalog belongs to
|
||||
* the bundle directory, so `operations` resolves that against the live catalog.
|
||||
*/
|
||||
export function coerceSkillSelection(value: unknown): SkillSelection {
|
||||
if (!isRecord(value)) return DEFAULT_SKILL_SELECTION;
|
||||
if (value.mode === "all") return { mode: "all" };
|
||||
if (value.mode === "custom") return { mode: "custom", skills: coerceSkillNames(value.skills) };
|
||||
return DEFAULT_SKILL_SELECTION;
|
||||
}
|
||||
|
||||
function coerceDocument(value: unknown): PersistedSkillSelectionDocument {
|
||||
const selection = isRecord(value)
|
||||
? coerceSkillSelection(value.selection)
|
||||
: DEFAULT_SKILL_SELECTION;
|
||||
return { version: 1, selection };
|
||||
}
|
||||
|
||||
export function createSkillSelectionStore({
|
||||
userDataPath,
|
||||
}: {
|
||||
userDataPath: string;
|
||||
}): SkillSelectionStore {
|
||||
const filePath = path.join(userDataPath, SKILL_SELECTION_FILENAME);
|
||||
let persistQueue: Promise<void> = Promise.resolve();
|
||||
|
||||
async function persist(document: PersistedSkillSelectionDocument): Promise<void> {
|
||||
const write = async () => {
|
||||
await mkdir(userDataPath, { recursive: true });
|
||||
const tempFilePath = `${filePath}.tmp.${process.pid}.${randomUUID()}`;
|
||||
await writeFile(tempFilePath, `${JSON.stringify(document, null, 2)}\n`, "utf8");
|
||||
await rename(tempFilePath, filePath);
|
||||
};
|
||||
const queued = persistQueue.then(write, write);
|
||||
persistQueue = queued.catch(() => undefined);
|
||||
await queued;
|
||||
}
|
||||
|
||||
return {
|
||||
async get(): Promise<SkillSelection> {
|
||||
let raw: string;
|
||||
try {
|
||||
raw = await readFile(filePath, "utf8");
|
||||
} catch (error) {
|
||||
if (!isNodeError(error) || error.code !== "ENOENT") throw error;
|
||||
return DEFAULT_SKILL_SELECTION;
|
||||
}
|
||||
try {
|
||||
return coerceDocument(JSON.parse(raw)).selection;
|
||||
} catch {
|
||||
return DEFAULT_SKILL_SELECTION;
|
||||
}
|
||||
},
|
||||
|
||||
async set(selection: unknown): Promise<SkillSelection> {
|
||||
const document = coerceDocument({ selection });
|
||||
await persist(document);
|
||||
return document.selection;
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,16 @@
|
||||
import type { DesktopCommandHandler } from "../../settings/desktop-settings-commands.js";
|
||||
import type { SkillsController } from "./skills-controller.js";
|
||||
|
||||
export function createSkillsCommandHandlers({
|
||||
controller,
|
||||
}: {
|
||||
controller: SkillsController;
|
||||
}): Record<string, DesktopCommandHandler> {
|
||||
return {
|
||||
get_skills_status: () => controller.status(),
|
||||
install_skills: () => controller.install(),
|
||||
update_skills: () => controller.update(),
|
||||
uninstall_skills: () => controller.uninstall(),
|
||||
save_skills_selection: (args) => controller.save(args),
|
||||
};
|
||||
}
|
||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,160 @@
|
||||
import {
|
||||
autoUpdateInstalledSkills,
|
||||
getSkillsStatus,
|
||||
installSkills,
|
||||
type SkillSelection,
|
||||
type SkillsStatus,
|
||||
type SkillTargets,
|
||||
uninstallSkills,
|
||||
updateSkills,
|
||||
} from "./operations.js";
|
||||
import {
|
||||
coerceSkillNames,
|
||||
coerceSkillSelection,
|
||||
type SkillSelectionStore,
|
||||
} from "./selection-store.js";
|
||||
import {
|
||||
beginSkillsTransaction,
|
||||
recoverInterruptedSkillTransactions,
|
||||
} from "./skills-transaction.js";
|
||||
|
||||
/** Everything the settings UI needs to render skills in one round trip. */
|
||||
export interface SkillsSnapshot extends SkillsStatus {
|
||||
selection: SkillSelection;
|
||||
}
|
||||
|
||||
export interface SkillsSaveResult extends SkillsSnapshot {
|
||||
/**
|
||||
* Non-null when the save did nothing because these directories would be
|
||||
* deleted and the request had not confirmed them. Retry with the names.
|
||||
*/
|
||||
confirmationRequired: { removals: string[] } | null;
|
||||
}
|
||||
|
||||
export interface SkillsController {
|
||||
status(): Promise<SkillsSnapshot>;
|
||||
install(): Promise<SkillsSnapshot>;
|
||||
update(): Promise<SkillsSnapshot>;
|
||||
uninstall(): Promise<SkillsSnapshot>;
|
||||
autoUpdate(): Promise<SkillsSnapshot>;
|
||||
save(request: unknown): Promise<SkillsSaveResult>;
|
||||
}
|
||||
|
||||
type Converge = (targets: SkillTargets, selection: SkillSelection) => Promise<SkillsStatus>;
|
||||
const MAX_SAVE_ATTEMPTS = 3;
|
||||
|
||||
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === "object" && value !== null && !Array.isArray(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* The single writer for installed skills. Startup convergence and every settings
|
||||
* command run through one queue, so no operation applies a plan computed from a
|
||||
* selection another one has already replaced, and no read sees a half-applied
|
||||
* directory. Ordering between concurrent callers is not promised — consistency
|
||||
* after both finish is.
|
||||
*/
|
||||
export function createSkillsController({
|
||||
resolveTargets,
|
||||
selectionStore,
|
||||
}: {
|
||||
// Resolved per operation, not at wiring time: the bundle path depends on how
|
||||
// the app was packaged, which is not knowable when the controller is built.
|
||||
resolveTargets: () => SkillTargets;
|
||||
selectionStore: SkillSelectionStore;
|
||||
}): SkillsController {
|
||||
let queue: Promise<unknown> = Promise.resolve();
|
||||
|
||||
function serialize<T>(run: () => Promise<T>): Promise<T> {
|
||||
const next = queue.then(run, run);
|
||||
queue = next.then(
|
||||
() => undefined,
|
||||
() => undefined,
|
||||
);
|
||||
return next;
|
||||
}
|
||||
|
||||
async function converge(apply: Converge): Promise<SkillsSnapshot> {
|
||||
const selection = await selectionStore.get();
|
||||
await recoverInterruptedSkillTransactions(resolveTargets(), selection);
|
||||
return { ...(await apply(resolveTargets(), selection)), selection };
|
||||
}
|
||||
|
||||
async function saveSelection(request: unknown): Promise<SkillsSaveResult> {
|
||||
const targets = resolveTargets();
|
||||
const next = coerceSkillSelection(request);
|
||||
const previous = await selectionStore.get();
|
||||
await recoverInterruptedSkillTransactions(targets, previous);
|
||||
const confirmed = new Set(
|
||||
coerceSkillNames(isRecord(request) ? request.confirmedRemovals : null),
|
||||
);
|
||||
|
||||
// The plan comes from a scan taken here, inside the queue, and is the exact
|
||||
// plan applied below. Deciding what will be deleted from an older snapshot
|
||||
// leaves a window for a directory to appear and be deleted unannounced.
|
||||
let plan = await getSkillsStatus(targets, next);
|
||||
for (let attempt = 0; attempt < MAX_SAVE_ATTEMPTS; attempt += 1) {
|
||||
const removals = plan.ops.filter((op) => op.kind === "delete").map((op) => op.name);
|
||||
if (removals.some((name) => !confirmed.has(name))) {
|
||||
return {
|
||||
...(await getSkillsStatus(targets, previous)),
|
||||
selection: previous,
|
||||
confirmationRequired: { removals },
|
||||
};
|
||||
}
|
||||
|
||||
// Capture only paths this frozen plan can mutate. Capturing every managed
|
||||
// path would make rollback delete a new external directory that appeared
|
||||
// after the scan merely because it was absent from the snapshot.
|
||||
const transaction = await beginSkillsTransaction(targets, previous, next, plan.ops);
|
||||
let status: SkillsStatus;
|
||||
try {
|
||||
status = await installSkills(targets, next, {
|
||||
...plan,
|
||||
// beginSkillsTransaction atomically staged these directories. Running
|
||||
// removeSkill afterward would delete a path another writer recreated.
|
||||
ops: plan.ops.filter((op) => op.kind !== "delete"),
|
||||
});
|
||||
} catch (error) {
|
||||
await transaction.rollback();
|
||||
throw error;
|
||||
}
|
||||
|
||||
// An empty custom selection converges to the intentional not-installed
|
||||
// state; zero remaining operations is the invariant, not the label.
|
||||
if (status.ops.length === 0) {
|
||||
try {
|
||||
await selectionStore.set(next);
|
||||
} catch (error) {
|
||||
await transaction.rollback();
|
||||
throw error;
|
||||
}
|
||||
try {
|
||||
await transaction.commit();
|
||||
} catch {
|
||||
// Selection and disk convergence are already durable. The owned
|
||||
// manifest stays behind so the next serialized operation retries
|
||||
// cleanup; reporting this save as failed would misstate its outcome.
|
||||
}
|
||||
return { ...status, selection: next, confirmationRequired: null };
|
||||
}
|
||||
|
||||
// The frozen plan became incomplete while it was being applied. Put only
|
||||
// its mutations back, rescan, and either request confirmation for a new
|
||||
// deletion or retry harmless additions/updates from the fresh plan.
|
||||
await transaction.rollback();
|
||||
plan = await getSkillsStatus(targets, next);
|
||||
}
|
||||
|
||||
throw new Error("Skills changed repeatedly while the selection was being saved");
|
||||
}
|
||||
|
||||
return {
|
||||
status: () => serialize(() => converge(getSkillsStatus)),
|
||||
install: () => serialize(() => converge(updateSkills)),
|
||||
update: () => serialize(() => converge(updateSkills)),
|
||||
uninstall: () => serialize(() => converge(uninstallSkills)),
|
||||
autoUpdate: () => serialize(() => converge(autoUpdateInstalledSkills)),
|
||||
save: (selection) => serialize(() => saveSelection(selection)),
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,607 @@
|
||||
import { createHash, randomUUID } from "node:crypto";
|
||||
import {
|
||||
cp,
|
||||
lstat,
|
||||
mkdir,
|
||||
readdir,
|
||||
readFile,
|
||||
readlink,
|
||||
realpath,
|
||||
rename,
|
||||
rm,
|
||||
stat,
|
||||
writeFile,
|
||||
} from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
|
||||
import {
|
||||
listManagedSkillNames,
|
||||
type SkillOp,
|
||||
type SkillSelection,
|
||||
type SkillTargets,
|
||||
} from "./operations.js";
|
||||
import { listFilesRecursive } from "./sync.js";
|
||||
|
||||
export interface SkillsTransaction {
|
||||
/** Convergence stuck. Drop the staged copies. */
|
||||
commit(): Promise<void>;
|
||||
/** Put every captured directory back exactly as it was, then drop the staging. */
|
||||
rollback(): Promise<void>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Written before anything is touched and updated once capture finishes. A
|
||||
* directory without a readable one of these is not ours and is never deleted —
|
||||
* the prefix alone is a guess, and guessing here deletes someone's files.
|
||||
*/
|
||||
interface TransactionManifest {
|
||||
owner: typeof MANIFEST_OWNER;
|
||||
version: 1;
|
||||
/** `capturing` means convergence never started, so there is nothing to undo. */
|
||||
phase: "capturing" | "converging";
|
||||
previousSelection: SkillSelection;
|
||||
nextSelection: SkillSelection;
|
||||
entries: CapturedDirectory[];
|
||||
}
|
||||
|
||||
interface CapturedDirectory {
|
||||
livePath: string;
|
||||
kind: SkillOp["kind"];
|
||||
/** Relative to the manifest directory, or an absolute same-filesystem delete stage. */
|
||||
backupPath: string | null;
|
||||
}
|
||||
|
||||
const MANIFEST_OWNER = "paseo-skills-transaction";
|
||||
const MANIFEST_FILENAME = "transaction.json";
|
||||
const TRANSACTION_PREFIX = ".paseo-skills-transaction-";
|
||||
const RECOVERED_PREFIX = ".paseo-skills-recovered-";
|
||||
const BACKUP_DIRNAME = "backup";
|
||||
const MANAGED_FILES_MANIFEST = ".paseo-managed-files.json";
|
||||
|
||||
async function isDirectory(target: string): Promise<boolean> {
|
||||
const info = await stat(target).catch(() => null);
|
||||
return info?.isDirectory() === true;
|
||||
}
|
||||
|
||||
function isSkillSelection(value: unknown): value is SkillSelection {
|
||||
if (typeof value !== "object" || value === null) return false;
|
||||
const selection = value as Partial<SkillSelection>;
|
||||
if (selection.mode === "all") return true;
|
||||
return (
|
||||
selection.mode === "custom" &&
|
||||
Array.isArray(selection.skills) &&
|
||||
selection.skills.every((name) => typeof name === "string")
|
||||
);
|
||||
}
|
||||
|
||||
async function readManifest(transactionDir: string): Promise<TransactionManifest | null> {
|
||||
const raw = await readFile(path.join(transactionDir, MANIFEST_FILENAME), "utf8").catch(
|
||||
() => null,
|
||||
);
|
||||
if (raw === null) return null;
|
||||
let parsed: unknown;
|
||||
try {
|
||||
parsed = JSON.parse(raw);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
if (typeof parsed !== "object" || parsed === null) return null;
|
||||
const manifest = parsed as Partial<TransactionManifest>;
|
||||
if (manifest.owner !== MANIFEST_OWNER || manifest.version !== 1) return null;
|
||||
if (manifest.phase !== "capturing" && manifest.phase !== "converging") return null;
|
||||
if (!isSkillSelection(manifest.previousSelection) || !isSkillSelection(manifest.nextSelection)) {
|
||||
return null;
|
||||
}
|
||||
if (!Array.isArray(manifest.entries)) return null;
|
||||
const validEntries = manifest.entries.every(
|
||||
(entry) =>
|
||||
typeof entry?.livePath === "string" &&
|
||||
(entry.kind === "add" || entry.kind === "update" || entry.kind === "delete") &&
|
||||
(entry.backupPath === null || typeof entry.backupPath === "string"),
|
||||
);
|
||||
if (!validEntries) return null;
|
||||
const entries = manifest.entries as CapturedDirectory[];
|
||||
return {
|
||||
owner: MANIFEST_OWNER,
|
||||
version: 1,
|
||||
phase: manifest.phase,
|
||||
previousSelection: manifest.previousSelection,
|
||||
nextSelection: manifest.nextSelection,
|
||||
entries,
|
||||
};
|
||||
}
|
||||
|
||||
async function writeManifest(transactionDir: string, manifest: TransactionManifest): Promise<void> {
|
||||
await writeFile(
|
||||
path.join(transactionDir, MANIFEST_FILENAME),
|
||||
`${JSON.stringify(manifest, null, 2)}\n`,
|
||||
"utf8",
|
||||
);
|
||||
}
|
||||
|
||||
function selectionsEqual(left: SkillSelection, right: SkillSelection): boolean {
|
||||
if (left.mode !== right.mode) return false;
|
||||
if (left.mode === "all" || right.mode === "all") return true;
|
||||
return (
|
||||
left.skills.length === right.skills.length &&
|
||||
left.skills.every((name, index) => name === right.skills[index])
|
||||
);
|
||||
}
|
||||
|
||||
async function validateEntries(
|
||||
targets: SkillTargets,
|
||||
transactionDir: string,
|
||||
entries: CapturedDirectory[],
|
||||
): Promise<boolean> {
|
||||
const names = new Set(await listManagedSkillNames(targets.sourceDir));
|
||||
const roots = [targets.agentsDir, targets.claudeDir, targets.codexDir];
|
||||
return entries.every((entry) => {
|
||||
const rootIndex = roots.findIndex((root) => path.dirname(entry.livePath) === root);
|
||||
if (rootIndex === -1) return false;
|
||||
const name = path.basename(entry.livePath);
|
||||
if (!names.has(name)) return false;
|
||||
if (entry.backupPath === null) return true;
|
||||
const expected = path.join(BACKUP_DIRNAME, String(rootIndex), name);
|
||||
const sameFilesystemStage = path.join(roots[rootIndex]!, path.basename(transactionDir), name);
|
||||
if (entry.kind === "delete" && entry.backupPath === sameFilesystemStage) return true;
|
||||
return (
|
||||
entry.backupPath === expected &&
|
||||
path
|
||||
.resolve(transactionDir, entry.backupPath)
|
||||
.startsWith(`${path.resolve(transactionDir)}${path.sep}`)
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
function resolveBackupPath(transactionDir: string, backupPath: string | null): string | null {
|
||||
if (backupPath === null) return null;
|
||||
return path.isAbsolute(backupPath) ? backupPath : path.join(transactionDir, backupPath);
|
||||
}
|
||||
|
||||
async function discardTransaction(
|
||||
transactionDir: string,
|
||||
entries: readonly CapturedDirectory[],
|
||||
): Promise<void> {
|
||||
const stagingDirs = new Set(
|
||||
entries.flatMap((entry) =>
|
||||
entry.backupPath !== null && path.isAbsolute(entry.backupPath)
|
||||
? [path.dirname(entry.backupPath)]
|
||||
: [],
|
||||
),
|
||||
);
|
||||
for (const stagingDir of stagingDirs) {
|
||||
await rm(stagingDir, { recursive: true, force: true });
|
||||
}
|
||||
await rm(transactionDir, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
async function listAllFiles(rootDir: string): Promise<string[]> {
|
||||
const files: string[] = [];
|
||||
async function walk(dir: string): Promise<void> {
|
||||
const entries = await readdir(dir, { withFileTypes: true }).catch(() => []);
|
||||
for (const entry of entries) {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) await walk(full);
|
||||
else if (entry.isFile()) files.push(path.relative(rootDir, full));
|
||||
}
|
||||
}
|
||||
await walk(rootDir);
|
||||
return files;
|
||||
}
|
||||
|
||||
async function expectedSyncedFiles(sourceDir: string, name: string): Promise<Map<string, Buffer>> {
|
||||
const skillDir = path.join(sourceDir, name);
|
||||
const files = new Map<string, Buffer>();
|
||||
const hashes: Record<string, string> = {};
|
||||
for (const rel of await listFilesRecursive(skillDir)) {
|
||||
const contents = await readFile(path.join(skillDir, rel));
|
||||
files.set(rel, contents);
|
||||
hashes[rel] = createHash("sha256").update(contents).digest("hex");
|
||||
}
|
||||
files.set(
|
||||
MANAGED_FILES_MANIFEST,
|
||||
Buffer.from(`${JSON.stringify({ version: 1, files: hashes }, null, 2)}\n`),
|
||||
);
|
||||
return files;
|
||||
}
|
||||
|
||||
async function copyStagedEntry(source: string, target: string): Promise<void> {
|
||||
const info = await lstat(source);
|
||||
await mkdir(path.dirname(target), { recursive: true });
|
||||
await cp(source, target, {
|
||||
recursive: info.isDirectory(),
|
||||
preserveTimestamps: true,
|
||||
verbatimSymlinks: true,
|
||||
});
|
||||
}
|
||||
|
||||
async function mergeBackupDirectory(livePath: string, backupPath: string | null): Promise<boolean> {
|
||||
if (backupPath === null) return false;
|
||||
const liveInfo = await lstat(livePath).catch(() => null);
|
||||
const destination = liveInfo?.isSymbolicLink() ? await realpath(livePath) : livePath;
|
||||
await mkdir(destination, { recursive: true });
|
||||
let hasConflict = false;
|
||||
|
||||
async function mergeMissing(sourceDir: string, destinationDir: string): Promise<void> {
|
||||
for (const entry of await readdir(sourceDir, { withFileTypes: true })) {
|
||||
const source = path.join(sourceDir, entry.name);
|
||||
const target = path.join(destinationDir, entry.name);
|
||||
const sourceInfo = await lstat(source);
|
||||
const targetInfo = await lstat(target).catch(() => null);
|
||||
if (targetInfo === null) {
|
||||
await copyStagedEntry(source, target);
|
||||
continue;
|
||||
}
|
||||
if (
|
||||
sourceInfo.isDirectory() &&
|
||||
!sourceInfo.isSymbolicLink() &&
|
||||
targetInfo.isDirectory() &&
|
||||
!targetInfo.isSymbolicLink()
|
||||
) {
|
||||
if ((sourceInfo.mode & 0o777) !== (targetInfo.mode & 0o777)) hasConflict = true;
|
||||
await mergeMissing(source, target);
|
||||
continue;
|
||||
}
|
||||
if (sourceInfo.isFile() && targetInfo.isFile()) {
|
||||
const [sourceContents, targetContents] = await Promise.all([
|
||||
readFile(source),
|
||||
readFile(target),
|
||||
]);
|
||||
if (
|
||||
!sourceContents.equals(targetContents) ||
|
||||
(sourceInfo.mode & 0o777) !== (targetInfo.mode & 0o777)
|
||||
) {
|
||||
hasConflict = true;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (sourceInfo.isSymbolicLink() && targetInfo.isSymbolicLink()) {
|
||||
const [sourceTarget, targetTarget] = await Promise.all([
|
||||
readlink(source),
|
||||
readlink(target),
|
||||
]);
|
||||
if (sourceTarget !== targetTarget) hasConflict = true;
|
||||
continue;
|
||||
}
|
||||
hasConflict = true;
|
||||
}
|
||||
}
|
||||
|
||||
await mergeMissing(backupPath, destination);
|
||||
return hasConflict;
|
||||
}
|
||||
|
||||
function recoveryBase(transactionDir: string, livePath: string, backupPath: string): string {
|
||||
const transactionId = path.basename(transactionDir).replace(TRANSACTION_PREFIX, "");
|
||||
// Update backups live inside the central transaction directory; delete stages
|
||||
// live beside their target root. Keep quarantine on the backup's filesystem
|
||||
// so preserving it remains an atomic rename across mounted agent homes.
|
||||
const recoveryParent = backupPath.startsWith(`${transactionDir}${path.sep}`)
|
||||
? path.dirname(transactionDir)
|
||||
: path.dirname(livePath);
|
||||
return path.join(
|
||||
recoveryParent,
|
||||
`${RECOVERED_PREFIX}${path.basename(livePath)}-${transactionId}`,
|
||||
);
|
||||
}
|
||||
|
||||
async function findQuarantinedEntry(
|
||||
transactionDir: string,
|
||||
livePath: string,
|
||||
backupPath: string,
|
||||
claimed: ReadonlySet<string>,
|
||||
): Promise<string | null> {
|
||||
const base = recoveryBase(transactionDir, livePath, backupPath);
|
||||
const name = path.basename(base);
|
||||
const matches = (await readdir(path.dirname(base)).catch(() => []))
|
||||
.filter((entry) => entry === name || entry.startsWith(`${name}-`))
|
||||
.map((entry) => path.join(path.dirname(base), entry))
|
||||
.filter((entry) => !claimed.has(entry))
|
||||
.sort();
|
||||
return matches[0] ?? null;
|
||||
}
|
||||
|
||||
async function quarantineStagedEntry(
|
||||
transactionDir: string,
|
||||
livePath: string,
|
||||
backupPath: string,
|
||||
): Promise<void> {
|
||||
const base = recoveryBase(transactionDir, livePath, backupPath);
|
||||
let destination = base;
|
||||
for (let suffix = 1; await lstat(destination).catch(() => null); suffix += 1) {
|
||||
destination = `${base}-${suffix}`;
|
||||
}
|
||||
await rename(backupPath, destination);
|
||||
}
|
||||
|
||||
async function restoreDeletedDirectory(
|
||||
transactionDir: string,
|
||||
livePath: string,
|
||||
backupPath: string | null,
|
||||
): Promise<void> {
|
||||
if (backupPath === null) return;
|
||||
const liveInfo = await lstat(livePath).catch(() => null);
|
||||
if (liveInfo === null) {
|
||||
await mkdir(path.dirname(livePath), { recursive: true });
|
||||
await rename(backupPath, livePath);
|
||||
return;
|
||||
}
|
||||
const backupInfo = await lstat(backupPath);
|
||||
if (backupInfo.isDirectory() && !backupInfo.isSymbolicLink() && (await isDirectory(livePath))) {
|
||||
const hasConflict = await mergeBackupDirectory(livePath, backupPath);
|
||||
if (hasConflict) await quarantineStagedEntry(transactionDir, livePath, backupPath);
|
||||
return;
|
||||
}
|
||||
// A concurrent writer recreated the path with an incompatible type. Keep its
|
||||
// entry live and move the staged original outside transaction cleanup so
|
||||
// neither side is overwritten and later controller operations can proceed.
|
||||
await quarantineStagedEntry(transactionDir, livePath, backupPath);
|
||||
}
|
||||
|
||||
async function restoreStagedEntry(
|
||||
liveRoot: string,
|
||||
backupRoot: string | null,
|
||||
rel: string,
|
||||
): Promise<void> {
|
||||
const live = path.join(liveRoot, rel);
|
||||
const backup = backupRoot && path.join(backupRoot, rel);
|
||||
await rm(live, { recursive: true, force: true });
|
||||
if (backup === null || !(await lstat(backup).catch(() => null))) return;
|
||||
await copyStagedEntry(backup, live);
|
||||
}
|
||||
|
||||
async function undoSyncedDirectory(
|
||||
transactionDir: string,
|
||||
livePath: string,
|
||||
backupPath: string | null,
|
||||
expectedFiles: ReadonlyMap<string, Buffer>,
|
||||
): Promise<void> {
|
||||
for (const rel of await listAllFiles(livePath)) {
|
||||
const target = path.join(livePath, rel);
|
||||
const current = await readFile(target).catch(() => null);
|
||||
const expected = expectedFiles.get(rel);
|
||||
if (current === null || expected === undefined || !current.equals(expected)) continue;
|
||||
await restoreStagedEntry(livePath, backupPath, rel);
|
||||
}
|
||||
|
||||
// Sync can prune files that an older managed-files manifest owned. Restore
|
||||
// missing staged entries with their type and metadata, but never overwrite a
|
||||
// path another writer recreated.
|
||||
const hasConflict = await mergeBackupDirectory(livePath, backupPath);
|
||||
if (hasConflict && backupPath !== null) {
|
||||
await quarantineStagedEntry(transactionDir, livePath, backupPath);
|
||||
}
|
||||
if ((await readdir(livePath).catch(() => [])).length === 0) {
|
||||
await rm(livePath, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
async function restore(
|
||||
targets: SkillTargets,
|
||||
transactionDir: string,
|
||||
manifest: TransactionManifest,
|
||||
): Promise<void> {
|
||||
// Nothing was converged yet, so the live tree is already the pre-save state.
|
||||
if (manifest.phase === "capturing") return;
|
||||
const expectedByName = new Map<string, Map<string, Buffer>>();
|
||||
const claimedQuarantines = new Set<string>();
|
||||
for (const entry of manifest.entries) {
|
||||
const backup = resolveBackupPath(transactionDir, entry.backupPath);
|
||||
if (backup !== null && !(await lstat(backup).catch(() => null))) {
|
||||
// Atomic rename leaves either the live path or its stage. If neither is
|
||||
// present, an external actor deleted it; there is nothing this
|
||||
// transaction can restore. Other captured entries still need rollback.
|
||||
if (entry.kind === "delete") continue;
|
||||
const quarantined = await findQuarantinedEntry(
|
||||
transactionDir,
|
||||
entry.livePath,
|
||||
backup,
|
||||
claimedQuarantines,
|
||||
);
|
||||
if (quarantined !== null) {
|
||||
claimedQuarantines.add(quarantined);
|
||||
continue;
|
||||
}
|
||||
throw new Error(`Skills transaction backup is missing: ${backup}`);
|
||||
}
|
||||
if (entry.kind === "delete") {
|
||||
await restoreDeletedDirectory(transactionDir, entry.livePath, backup);
|
||||
continue;
|
||||
}
|
||||
const liveInfo = await lstat(entry.livePath).catch(() => null);
|
||||
if (liveInfo !== null && !(await isDirectory(entry.livePath))) {
|
||||
// Add/update rollback has the same conflict state as delete rollback. A
|
||||
// recreated file or file symlink belongs to the external writer; preserve
|
||||
// it, and preserve any captured original outside transaction cleanup.
|
||||
if (backup !== null) {
|
||||
await quarantineStagedEntry(transactionDir, entry.livePath, backup);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
const backupInfo = backup && (await lstat(backup).catch(() => null));
|
||||
if (backupInfo && !backupInfo.isDirectory()) {
|
||||
if (liveInfo !== null && (await isDirectory(entry.livePath))) {
|
||||
// The captured file was replaced by a directory after the transaction
|
||||
// started. Preserve the external tree and quarantine the captured entry
|
||||
// instead of recursively deleting the replacement during recovery.
|
||||
await quarantineStagedEntry(transactionDir, entry.livePath, backup);
|
||||
continue;
|
||||
}
|
||||
await restoreStagedEntry(entry.livePath, backup, ".");
|
||||
continue;
|
||||
}
|
||||
const name = path.basename(entry.livePath);
|
||||
let expected = expectedByName.get(name);
|
||||
if (!expected) {
|
||||
expected = await expectedSyncedFiles(targets.sourceDir, name);
|
||||
expectedByName.set(name, expected);
|
||||
}
|
||||
await undoSyncedDirectory(transactionDir, entry.livePath, backup, expected);
|
||||
}
|
||||
}
|
||||
|
||||
function transactionParents(targets: SkillTargets): string[] {
|
||||
return [
|
||||
...new Set(
|
||||
[targets.agentsDir, targets.claudeDir, targets.codexDir].map((root) => path.dirname(root)),
|
||||
),
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Finishes what an interrupted save started. A transaction directory only exists
|
||||
* while a save is mid-flight, so finding one means the process died between
|
||||
* mutating the directories and committing the selection — the staged copies are
|
||||
* the only place the user's files still exist. Restore them, then drop the
|
||||
* transaction. Anything without our manifest is left where it is.
|
||||
*/
|
||||
export async function recoverInterruptedSkillTransactions(
|
||||
targets: SkillTargets,
|
||||
committedSelection: SkillSelection,
|
||||
): Promise<void> {
|
||||
for (const parent of transactionParents(targets)) {
|
||||
const entries = await readdir(parent).catch(() => []);
|
||||
for (const entry of entries) {
|
||||
if (!entry.startsWith(TRANSACTION_PREFIX)) continue;
|
||||
const transactionDir = path.join(parent, entry);
|
||||
const manifest = await readManifest(transactionDir);
|
||||
if (!manifest) continue;
|
||||
if (!(await validateEntries(targets, transactionDir, manifest.entries))) continue;
|
||||
if (selectionsEqual(committedSelection, manifest.nextSelection)) {
|
||||
await discardTransaction(transactionDir, manifest.entries);
|
||||
continue;
|
||||
}
|
||||
if (!selectionsEqual(committedSelection, manifest.previousSelection)) {
|
||||
throw new Error(
|
||||
`Cannot safely recover interrupted skills transaction at ${transactionDir}`,
|
||||
);
|
||||
}
|
||||
await restore(targets, transactionDir, manifest);
|
||||
await discardTransaction(transactionDir, manifest.entries);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Captures the managed skill directories so a failed save can put the machine
|
||||
* back exactly as it was — including files the user added inside them, which
|
||||
* re-running an install cannot reconstruct.
|
||||
*/
|
||||
export async function beginSkillsTransaction(
|
||||
targets: SkillTargets,
|
||||
previousSelection: SkillSelection,
|
||||
nextSelection: SkillSelection,
|
||||
ops: readonly SkillOp[],
|
||||
): Promise<SkillsTransaction> {
|
||||
const roots = [targets.agentsDir, targets.claudeDir, targets.codexDir];
|
||||
const seenRoots = new Set<string>();
|
||||
const uniqueRoots: Array<{ root: string; rootIndex: number }> = [];
|
||||
for (const [rootIndex, root] of roots.entries()) {
|
||||
const identity = await realpath(root).catch(() => path.resolve(root));
|
||||
if (seenRoots.has(identity)) continue;
|
||||
seenRoots.add(identity);
|
||||
uniqueRoots.push({ root, rootIndex });
|
||||
}
|
||||
// Staged next to the skills tree rather than inside it: same filesystem, so a
|
||||
// restore is a local copy, and never a directory a skill scan walks into.
|
||||
const transactionDir = path.join(path.dirname(roots[0]!), `${TRANSACTION_PREFIX}${randomUUID()}`);
|
||||
const entries: CapturedDirectory[] = [];
|
||||
|
||||
async function discard(): Promise<void> {
|
||||
await discardTransaction(transactionDir, entries);
|
||||
}
|
||||
|
||||
await mkdir(transactionDir, { recursive: true });
|
||||
await writeManifest(transactionDir, {
|
||||
owner: MANIFEST_OWNER,
|
||||
version: 1,
|
||||
phase: "capturing",
|
||||
previousSelection,
|
||||
nextSelection,
|
||||
entries: [],
|
||||
});
|
||||
|
||||
try {
|
||||
for (const { root, rootIndex } of uniqueRoots) {
|
||||
for (const op of ops) {
|
||||
const name = op.name;
|
||||
const livePath = path.join(root, name);
|
||||
const liveInfo = await lstat(livePath).catch(() => null);
|
||||
if (liveInfo === null) {
|
||||
entries.push({ livePath, kind: op.kind, backupPath: null });
|
||||
continue;
|
||||
}
|
||||
const liveIsDirectory = await isDirectory(livePath);
|
||||
const backupPath =
|
||||
op.kind === "delete"
|
||||
? path.join(root, path.basename(transactionDir), name)
|
||||
: path.join(BACKUP_DIRNAME, String(rootIndex), name);
|
||||
if (op.kind !== "delete") {
|
||||
const captureSource =
|
||||
liveInfo.isSymbolicLink() && liveIsDirectory ? await realpath(livePath) : livePath;
|
||||
const captureInfo = await lstat(captureSource);
|
||||
await cp(captureSource, resolveBackupPath(transactionDir, backupPath)!, {
|
||||
recursive: captureInfo.isDirectory(),
|
||||
preserveTimestamps: true,
|
||||
verbatimSymlinks: true,
|
||||
});
|
||||
}
|
||||
entries.push({ livePath, kind: op.kind, backupPath });
|
||||
}
|
||||
}
|
||||
await writeManifest(transactionDir, {
|
||||
owner: MANIFEST_OWNER,
|
||||
version: 1,
|
||||
phase: "converging",
|
||||
previousSelection,
|
||||
nextSelection,
|
||||
entries,
|
||||
});
|
||||
} catch (error) {
|
||||
// Nothing has been converged yet, so failing to stage means failing the save
|
||||
// before it touches anything.
|
||||
await discard();
|
||||
throw error;
|
||||
}
|
||||
|
||||
try {
|
||||
// Each delete is staged inside its own skills root, so rename stays atomic
|
||||
// even when the agent homes are symlinks or mounts on different filesystems.
|
||||
// Any writer
|
||||
// holding the old directory keeps writing into the staged inode; any writer
|
||||
// recreating the old path remains live and is merged on rollback. The later
|
||||
// convergence plan deliberately omits deletes.
|
||||
for (const entry of entries) {
|
||||
if (entry.kind !== "delete" || entry.backupPath === null) continue;
|
||||
const backup = resolveBackupPath(transactionDir, entry.backupPath)!;
|
||||
await mkdir(path.dirname(backup), { recursive: true });
|
||||
await rename(entry.livePath, backup);
|
||||
}
|
||||
} catch (error) {
|
||||
await restore(targets, transactionDir, {
|
||||
owner: MANIFEST_OWNER,
|
||||
version: 1,
|
||||
phase: "converging",
|
||||
previousSelection,
|
||||
nextSelection,
|
||||
entries,
|
||||
});
|
||||
await discard();
|
||||
throw error;
|
||||
}
|
||||
|
||||
return {
|
||||
commit: discard,
|
||||
async rollback(): Promise<void> {
|
||||
await restore(targets, transactionDir, {
|
||||
owner: MANIFEST_OWNER,
|
||||
version: 1,
|
||||
phase: "converging",
|
||||
previousSelection,
|
||||
nextSelection,
|
||||
entries,
|
||||
});
|
||||
await discard();
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -195,6 +195,39 @@ describe("syncSkills", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"rejects managed paths that would write through a symlink",
|
||||
async () => {
|
||||
await writeBundleSkill(sandbox.sourceDir, "paseo", {
|
||||
"SKILL.md": "new skill",
|
||||
"references/guide.md": "new guide",
|
||||
});
|
||||
const onDiskSkill = path.join(sandbox.agentsDir, "paseo");
|
||||
const externalReferences = path.join(sandbox.root, "external-references");
|
||||
await fs.mkdir(onDiskSkill, { recursive: true });
|
||||
await fs.mkdir(externalReferences, { recursive: true });
|
||||
await fs.writeFile(path.join(onDiskSkill, "SKILL.md"), "old skill");
|
||||
await fs.writeFile(path.join(externalReferences, "guide.md"), "external guide");
|
||||
await fs.symlink(externalReferences, path.join(onDiskSkill, "references"), "dir");
|
||||
|
||||
await expect(
|
||||
syncSkills({
|
||||
sourceDir: sandbox.sourceDir,
|
||||
agentsDir: sandbox.agentsDir,
|
||||
claudeDir: sandbox.claudeDir,
|
||||
codexDir: sandbox.codexDir,
|
||||
skillNames: ["paseo"],
|
||||
}),
|
||||
).rejects.toThrow("Cannot sync through symbolic link");
|
||||
|
||||
expect(await fs.readFile(path.join(onDiskSkill, "SKILL.md"), "utf-8")).toBe("old skill");
|
||||
expect(await fs.readFile(path.join(externalReferences, "guide.md"), "utf-8")).toBe(
|
||||
"external guide",
|
||||
);
|
||||
expect((await fs.lstat(path.join(onDiskSkill, "references"))).isSymbolicLink()).toBe(true);
|
||||
},
|
||||
);
|
||||
|
||||
it("reports zero changed files on a no-op resync", async () => {
|
||||
await writeBundleSkill(sandbox.sourceDir, "paseo", {
|
||||
"SKILL.md": "content",
|
||||
|
||||
@@ -107,6 +107,31 @@ async function pruneEmptyParentDirs(rootDir: string, rels: readonly string[]): P
|
||||
}
|
||||
}
|
||||
|
||||
async function assertManagedPathsStayInsideSkill(rootDir: string, rels: readonly string[]) {
|
||||
for (const rel of rels) {
|
||||
const normalized = path.normalize(rel);
|
||||
if (
|
||||
path.isAbsolute(normalized) ||
|
||||
normalized === ".." ||
|
||||
normalized.startsWith(`..${path.sep}`)
|
||||
) {
|
||||
throw new Error(`Cannot sync managed path outside skill directory: ${rel}`);
|
||||
}
|
||||
let current = rootDir;
|
||||
for (const segment of normalized.split(path.sep)) {
|
||||
current = path.join(current, segment);
|
||||
const info = await fs.lstat(current).catch((error: NodeJS.ErrnoException) => {
|
||||
if (error.code === "ENOENT") return null;
|
||||
throw error;
|
||||
});
|
||||
if (info === null) break;
|
||||
if (info.isSymbolicLink()) {
|
||||
throw new Error(`Cannot sync through symbolic link: ${current}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async function syncDirectoryFiles(srcDir: string, dstDir: string): Promise<number> {
|
||||
const files = await listFilesRecursive(srcDir);
|
||||
const srcFileSet = new Set(files);
|
||||
@@ -114,7 +139,13 @@ async function syncDirectoryFiles(srcDir: string, dstDir: string): Promise<numbe
|
||||
for (const rel of files) {
|
||||
srcHashes[rel] = await hashFile(path.join(srcDir, rel));
|
||||
}
|
||||
await assertManagedPathsStayInsideSkill(dstDir, [MANAGED_FILES_MANIFEST]);
|
||||
const previousManifest = await readManagedFilesManifest(dstDir);
|
||||
await assertManagedPathsStayInsideSkill(dstDir, [
|
||||
MANAGED_FILES_MANIFEST,
|
||||
...files,
|
||||
...Object.keys(previousManifest?.files ?? {}),
|
||||
]);
|
||||
let changed = 0;
|
||||
for (const rel of files) {
|
||||
if (await writeFileIfChanged(path.join(srcDir, rel), path.join(dstDir, rel))) {
|
||||
|
||||
Reference in New Issue
Block a user