diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c345e4..07970b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,29 @@ # Changelog +## Unreleased + +This change aligns the installer with the harness skill discovery rules so +that shared skills are discovered exactly once. + +### Changes + +- Install Codex, OpenCode, and pi skills into the shared `~/.agents/skills/` + directory they all discover, instead of duplicating a copy into + `~/.config/opencode/skills/` and `~/.pi/agent/skills/`. Claude Code keeps + `~/.claude/skills/`. Duplicate definitions, same-name override warnings, and + stale copies that shadowed updated shared skills are gone. +- Keep artifacts in their harness-specific destinations, and keep the Claude + frontmatter adaptation for the Claude copy. +- Add `--migrate` to remove legacy 0.4.0 copies from + `~/.config/opencode/skills/` and `~/.pi/agent/skills/` after installing. + Only copies that match the current mstack source byte for byte are removed; + modified copies and unrelated skills are kept and reported, with backups + under the legacy directory. `--dry-run` previews the migration and reports + legacy copies left behind without `--migrate`. +- Group shared targets in installer output, note OpenCode's residual scan of + `~/.claude/skills/`, and document the discovery model in the README and the + harness adapter reference. + ## 0.4.0 - 2026-09-12 This release adds a shared project workflow for teams using pstack and mstack diff --git a/README.md b/README.md index 6ee7450..3e930b7 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,33 @@ backups therefore stay inside that Harness's `skills/` directory, and custom artifact backups stay beside the artifact target. mstack retains backups until you inspect and remove them. +Codex, OpenCode, and pi share one install directory. OpenCode and pi also +discover `~/.agents/skills/`, so installing a separate copy for them would make +every Harness report duplicate definitions and keep serving stale copies after +an update. The installer therefore plans skills per discovery directory: Codex, +OpenCode, and pi share `~/.agents/skills/`, while Claude Code keeps +`~/.claude/skills/`. Installing with `--harness all` reports the shared target +as `codex, opencode, pi (shared)` in `--dry-run` output. + +OpenCode also scans `~/.claude/skills/`, so the shared copy and the Claude copy +are both discoverable there; upstream OpenCode reports same-named skills as +duplicates and applies one of them. mstack keeps both copies current on every +install, so either choice loads the same instructions. Use `HARNESS_SKILLS_OPENCODE_DIR` +to pin OpenCode to a directory only it discovers when that matters. + +Installations made by mstack 0.4.0 placed OpenCode and pi copies in their own +config directories. After installing, run once with `--migrate` to remove the +redundant copies from those legacy directories: + +```bash +npx @3metajun/mstack --harness all --replace --migrate +``` + +`--migrate` only removes legacy copies that match the current mstack source +byte for byte, and only for skills selected by that install. Modified copies +and unrelated skills are kept and reported. Use `--dry-run` to preview which +copies would be removed or kept. + ## Install optional artifacts The same installer can copy the portable artifacts that accompany the skills. @@ -61,9 +88,14 @@ The available installable artifacts are: | Artifact | Default destination | | --- | --- | -| `agents` | Codex: `$CODEX_HOME/agents/` as TOML; other harnesses: `agents/` beside `skills/` as Markdown | -| `meta-mode-tools` | `tools/meta-mode/` beside `skills/` | -| `guide` | `docs/guide/` beside `skills/` | +| `agents` | Codex: `$CODEX_HOME/agents/` as TOML; Claude Code: `~/.claude/agents/`; OpenCode: `~/.config/opencode/agents/`; pi: `~/.pi/agent/agents/`, as Markdown | +| `meta-mode-tools` | `tools/meta-mode/` under the harness root: Codex `~/.agents/`, Claude Code `~/.claude/`, OpenCode `~/.config/opencode/`, pi `~/.pi/agent/` | +| `guide` | `docs/guide/` under the same harness roots | + +Artifacts keep their Harness-specific destinations even though OpenCode and pi +skills are shared with Codex. Relocating a harness's skills with +`HARNESS_SKILLS_*_DIR` or an environment target also relocates that harness's +artifact base beside the relocated skills directory. Codex skills default to `~/.agents/skills/`, while Codex agents default to `~/.codex/agents/`. An unset or empty `CODEX_HOME` uses `~/.codex`. @@ -109,14 +141,22 @@ written or merged explicitly by the user. ## Choose installation directories The default directories are defined in -[`profiles/harnesses.json`](./profiles/harnesses.json): +[`profiles/harnesses.json`](./profiles/harnesses.json). They follow each +harness's discovery rules: OpenCode and pi both discover the shared +`~/.agents/skills/` directory, so they install there instead of keeping their +own copies. | Harness | Default directory | Override | | --- | --- | --- | | Codex | `~/.agents/skills/` | `HARNESS_SKILLS_CODEX_DIR` | | Claude Code | `~/.claude/skills/` | `HARNESS_SKILLS_CLAUDE_DIR` | -| OpenCode | `~/.config/opencode/skills/` | `HARNESS_SKILLS_OPENCODE_DIR` | -| pi | `~/.pi/agent/skills/` | `HARNESS_SKILLS_PI_DIR` | +| OpenCode | `~/.agents/skills/` (shared with Codex) | `HARNESS_SKILLS_OPENCODE_DIR` | +| pi | `~/.agents/skills/` (shared with Codex) | `HARNESS_SKILLS_PI_DIR` | + +An override relocates only where mstack writes the files; the harness still +scans its own discovery directories. OpenCode additionally reads +`~/.claude/skills/`, which is why the Claude copy is installed there as well +and kept identical apart from the Claude frontmatter adaptation. Set an override to install into a mounted Fleet directory or another local path. The path must be absolute or start with `~/`. diff --git a/docs/harness-adapters.md b/docs/harness-adapters.md index 040deba..ec61f07 100644 --- a/docs/harness-adapters.md +++ b/docs/harness-adapters.md @@ -19,6 +19,18 @@ Sources: - [OpenCode skills](https://opencode.ai/docs/skills) - [pi skills](https://pi.dev/docs/latest/skills) +User-level discovery crosses these roots. OpenCode reads its own root plus +`~/.agents/skills/` and `~/.claude/skills/`, and pi reads its own root plus +`~/.agents/skills/`. The installer therefore plans one copy per discovery +directory instead of one copy per harness: Codex, OpenCode, and pi share +`~/.agents/skills/`, and Claude Code keeps `~/.claude/skills/`. OpenCode sees +the shared copy and the Claude copy and reports same-named skills as +duplicates; the installer refreshes both on every install, so either loads the +same instructions apart from the Claude frontmatter adaptation. Legacy mstack +0.4.0 copies under `~/.config/opencode/skills/` and `~/.pi/agent/skills/` are +removed by installing with `--migrate`, which keeps copies that differ from +the current source and preserves unrelated skills. + The canonical tree keeps the Agent Skills fields that all supported Harnesses can read. Claude Code accepts `metadata` but does not act on its contents, so `adapters/claude.json` removes that map and surfaces the logger requirement in diff --git a/profiles/harnesses.json b/profiles/harnesses.json index 8f2c6c0..a340af9 100644 --- a/profiles/harnesses.json +++ b/profiles/harnesses.json @@ -2,6 +2,10 @@ "codex": { "directoryVariable": "HARNESS_SKILLS_CODEX_DIR", "fallback": [".agents", "skills"], + "skills": { + "install": "own", + "discovers": ["own"] + }, "runtime": { "command": "codex", "headlessArgs": ["exec", "--sandbox", "read-only"], @@ -14,6 +18,10 @@ "configVariable": "CLAUDE_CONFIG_DIR", "configFallback": ".claude", "suffix": ["skills"], + "skills": { + "install": "own", + "discovers": ["own"] + }, "runtime": { "command": "claude", "headlessArgs": ["--print", "--no-session-persistence", "--output-format", "text"], @@ -27,6 +35,11 @@ "configFallback": ".config", "prefix": ["opencode"], "suffix": ["skills"], + "skills": { + "install": "codex", + "discovers": ["own", "codex", "claude"], + "legacy": "own" + }, "runtime": { "command": "opencode", "headlessArgs": ["run", "--pure"], @@ -39,6 +52,11 @@ "configVariable": "PI_CODING_AGENT_DIR", "configFallback": ".pi/agent", "suffix": ["skills"], + "skills": { + "install": "codex", + "discovers": ["own", "codex"], + "legacy": "own" + }, "runtime": { "command": "pi", "headlessArgs": ["-p", "--no-session"], diff --git a/scripts/harness-targets.mjs b/scripts/harness-targets.mjs new file mode 100644 index 0000000..58abb0c --- /dev/null +++ b/scripts/harness-targets.mjs @@ -0,0 +1,74 @@ +import { homedir } from "node:os"; +import { isAbsolute, join, resolve } from "node:path"; + +export function expandHome(path, home) { + if (path === "~") return home; + if (/^~[\\/]/.test(path)) return join(home, path.slice(2)); + return path; +} + +export function configuredPath(value, fallback, label, home = homedir()) { + const expanded = expandHome(value ?? fallback, home); + if (value && !isAbsolute(expanded)) { + throw new Error(`${label} must be an absolute path or start with ~/`); + } + return resolve(expanded); +} + +export function pathFromParts(parts, home = homedir()) { + return parts.reduce((current, part) => join(current, part), home); +} + +function harnessConfigRoot(registry, harness, env, home) { + const config = registry[harness]; + const configured = env[config.configVariable] + ? configuredPath(env[config.configVariable], "", config.configVariable, home) + : pathFromParts([config.configFallback], home); + return join(configured, ...(config.prefix ?? [])); +} + +export function ownConfigRoot(registry, harness, env = process.env, home = homedir()) { + if (harness === "codex") { + return configuredPath(env.CODEX_HOME || undefined, join(home, ".codex"), "CODEX_HOME", home); + } + return harnessConfigRoot(registry, harness, env, home); +} + +export function defaultSkillsDir(registry, harness, env = process.env, home = homedir()) { + const config = registry[harness]; + const skills = config.skills ?? {}; + if (skills.install && skills.install !== "own") { + const referenced = registry[skills.install]; + if (!referenced?.fallback) { + throw new Error( + `Harness ${harness} installs skills into the ${skills.install} directory, but ${skills.install} defines no fallback`, + ); + } + return pathFromParts(referenced.fallback, home); + } + if (config.fallback) return pathFromParts(config.fallback, home); + return join(harnessConfigRoot(registry, harness, env, home), ...(config.suffix ?? [])); +} + +export function skillsTarget(registry, harness, options = {}) { + const env = options.env ?? process.env; + const home = options.home ?? homedir(); + const config = registry[harness]; + if (options.environmentTarget) { + return configuredPath( + options.environmentTarget, + "", + `${options.environmentName ?? "environment"}.${harness}`, + home, + ); + } + const directory = env[config.directoryVariable]; + if (directory) return configuredPath(directory, "", config.directoryVariable, home); + return defaultSkillsDir(registry, harness, env, home); +} + +export function legacySkillsDir(registry, harness, env = process.env, home = homedir()) { + const config = registry[harness]; + if (config.skills?.legacy !== "own") return undefined; + return join(harnessConfigRoot(registry, harness, env, home), ...(config.suffix ?? [])); +} diff --git a/scripts/install.mjs b/scripts/install.mjs index 97c7468..0b99b9b 100644 --- a/scripts/install.mjs +++ b/scripts/install.mjs @@ -6,6 +6,7 @@ import { existsSync, mkdirSync, lstatSync, + mkdtempSync, openSync, readFileSync, realpathSync, @@ -15,12 +16,18 @@ import { unlinkSync, writeFileSync, } from "node:fs"; -import { homedir } from "node:os"; +import { homedir, tmpdir } from "node:os"; import { basename, dirname, isAbsolute, join, resolve } from "node:path"; import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; import { readEnvironment } from "./environment-lib.mjs"; import { convertAgentMarkdown } from "./agent-format.mjs"; +import { + configuredPath, + legacySkillsDir, + ownConfigRoot, + skillsTarget, +} from "./harness-targets.mjs"; import { artifactPathParts, localPathKey, @@ -53,11 +60,15 @@ function valueAfter(flag) { } if (args.includes("--help") || !args.includes("--harness")) { - console.log(`Usage: node scripts/install.mjs --harness [--environment ] [--skill ] [--artifact ] [--no-skills] [--dry-run] [--replace] + console.log(`Usage: node scripts/install.mjs --harness [--environment ] [--skill ] [--artifact ] [--no-skills] [--dry-run] [--replace] [--migrate] Installs canonical skills and optional artifacts into user-level harness directories. +Skills shared across harnesses are installed once per discovery directory; OpenCode +and pi install into the shared ~/.agents/skills directory they already discover. Artifacts: ${validArtifacts.join(", ")} (use --artifact all for installable artifacts). -Existing directories are preserved unless --replace is supplied.`); +Existing directories are preserved unless --replace is supplied. +--migrate removes legacy 0.4.0 skill copies from the harness-owned directories that +are no longer install targets, keeping copies that differ from the current source.`); process.exit(args.includes("--help") ? 0 : 1); } @@ -73,6 +84,7 @@ if (new Set(requestedHarnesses).size !== requestedHarnesses.length) { const harnesses = [...requestedHarnesses]; const dryRun = args.includes("--dry-run"); const replace = args.includes("--replace"); +const migrate = args.includes("--migrate"); const userHome = homedir(); const hasEnvironment = args.includes("--environment"); const environmentName = valueAfter("--environment"); @@ -80,28 +92,22 @@ if (hasEnvironment && (!environmentName || environmentName.startsWith("--"))) { throw new Error("--environment requires a name"); } -function expandHome(path) { - if (path === "~") return userHome; - if (/^~[\\/]/.test(path)) return join(userHome, path.slice(2)); - return path; -} - -function configuredPath(value, fallback, label) { - const expanded = expandHome(value ?? fallback); - if (value && !isAbsolute(expanded)) { - throw new Error(`${label} must be an absolute path or start with ~/`); - } - return resolve(expanded); -} - -function pathFromParts(parts) { - return parts.reduce((current, part) => join(current, part), userHome); +function pinnedSkillsRoot(harness) { + if (environmentTargets[harness]) return targets[harness]; + if (process.env[harnessRegistry[harness].directoryVariable]) return targets[harness]; + return undefined; } function artifactBase(name, harness) { const base = artifactRegistry[name]?.harnesses?.[harness]?.base; if (base === "codex-home") { - return configuredPath(process.env.CODEX_HOME || undefined, join(userHome, ".codex"), "CODEX_HOME"); + return configuredPath(process.env.CODEX_HOME || undefined, join(userHome, ".codex"), "CODEX_HOME", userHome); + } + const pinned = pinnedSkillsRoot(harness); + if (pinned) return dirname(pinned); + const skills = harnessRegistry[harness].skills ?? {}; + if (skills.install && skills.install !== "own") { + return ownConfigRoot(harnessRegistry, harness, process.env, userHome); } return dirname(targets[harness]); } @@ -118,23 +124,20 @@ if (environment.transport === "ssh") { } const environmentTargets = environment.targets; const environmentArtifacts = environment.artifacts; -function harnessTarget(harness) { - const config = harnessRegistry[harness]; - if (environmentTargets[harness]) { - return configuredPath(environmentTargets[harness], "", `${environmentName}.${harness}`); - } - const directory = process.env[config.directoryVariable]; - if (directory) return configuredPath(directory, "", config.directoryVariable); - - if (config.fallback) return pathFromParts(config.fallback); - const configRoot = process.env[config.configVariable] - ? configuredPath(process.env[config.configVariable], "", config.configVariable) - : pathFromParts([config.configFallback]); - return join(configRoot, ...(config.prefix ?? []), ...(config.suffix ?? [])); -} - -const targets = Object.fromEntries(validHarnesses.map((harness) => [harness, harnessTarget(harness)])); +const targets = Object.fromEntries( + validHarnesses.map((harness) => + [ + harness, + skillsTarget(harnessRegistry, harness, { + env: process.env, + home: userHome, + environmentTarget: environmentTargets[harness], + environmentName, + }), + ], + ), +); function artifactEnvironmentPath(name, harness) { const byArtifact = environmentArtifacts[name]; @@ -177,8 +180,6 @@ function artifactTarget(name, harness) { return target; } -const targetKeys = harnesses.map((harness) => localPathKey(targets[harness])); - const adapters = Object.fromEntries( validHarnesses.map((harness) => [ harness, @@ -325,6 +326,9 @@ if (args.includes("--no-skills") && hasSkillFilter) { throw new Error("--no-skills cannot be combined with --skill"); } const skills = args.includes("--no-skills") ? [] : requestedSkills ?? availableSkills; +if (migrate && args.includes("--no-skills")) { + throw new Error("--migrate requires skills to be installed"); +} const hasArtifactFilter = args.includes("--artifact"); const artifactFilter = valueAfter("--artifact"); @@ -390,25 +394,45 @@ for (const item of rawArtifactPlan) { artifactTargetsByPath.set(key, item); artifactPlan.push(item); } -const allTargetKeys = [ - ...targetKeys, - ...artifactPlan.map(({ target }) => localPathKey(target)), -]; -if (new Set(allTargetKeys).size !== allTargetKeys.length) { - throw new Error("Selected skills and artifacts resolve to the same target directory"); + +function adapterSignature(harness, skill) { + const adapter = adapters[harness]; + return JSON.stringify([ + adapter.removeFrontmatter?.[skill] ?? [], + adapter.frontmatter?.[skill] ?? null, + ]); +} + +const skillPlan = []; +const skillTargetsByPath = new Map(); +for (const harness of harnesses) { + for (const skill of skills) { + const key = localPathKey(join(targets[harness], skill)); + const existing = skillTargetsByPath.get(key); + if (existing) { + if (adapterSignature(existing.harness, skill) !== adapterSignature(harness, skill)) { + throw new Error( + `Skill ${skill} resolves to the same target directory for ${existing.harness} and ${harness} ` + + "with different harness adapters; configure separate skill directories", + ); + } + continue; + } + const entry = { harness, skill, target: join(targets[harness], skill) }; + skillTargetsByPath.set(key, entry); + skillPlan.push(entry); + } } const plan = [ - ...harnesses.flatMap((harness) => - skills.map((skill) => ({ - kind: "skill", - harness, - name: skill, - skill, - source: join(sourceRoot, skill), - target: join(targets[harness], skill), - })), - ), + ...skillPlan.map(({ harness, skill, target }) => ({ + kind: "skill", + harness, + name: skill, + skill, + source: join(sourceRoot, skill), + target, + })), ...artifactPlan, ]; if (plan.length === 0) throw new Error("Nothing selected: choose skills or --artifact"); @@ -427,9 +451,124 @@ for (let left = 0; left < plan.length; left += 1) { } } for (const source of new Set(plan.map((item) => item.source))) validateSourceTree(source); +const allTargetKeys = plan.map(({ target }) => localPathKey(target)); +if (new Set(allTargetKeys).size !== allTargetKeys.length) { + throw new Error("Selected skills and artifacts resolve to the same target directory"); +} const conflicts = plan.filter(({ target }) => existsSync(target)); -for (const harness of harnesses) console.log(`${harness}: ${targets[harness]}`); +function legacySkillCandidates(harness) { + const legacy = legacySkillsDir(harnessRegistry, harness, process.env, userHome); + if (!legacy || localPathKey(legacy) === localPathKey(targets[harness]) || !existsSync(legacy)) { + return { legacy, entries: [] }; + } + const selected = new Set(skills); + const entries = readdirSync(legacy, { withFileTypes: true }) + .filter((entry) => entry.isDirectory() && !entry.name.startsWith(".")) + .map((entry) => entry.name) + .sort() + .filter((name) => availableSkills.includes(name)) + .map((name) => ({ name, path: join(legacy, name), selected: selected.has(name) })); + return { legacy, entries }; +} + +function treesMatch(left, right) { + const leftEntries = readdirSync(left, { withFileTypes: true }).filter((entry) => entry.name !== "node_modules"); + const rightEntries = readdirSync(right, { withFileTypes: true }).filter((entry) => entry.name !== "node_modules"); + if (leftEntries.length !== rightEntries.length) return false; + for (const entry of leftEntries) { + const other = rightEntries.find((candidate) => candidate.name === entry.name); + if (!other) return false; + const leftPath = join(left, entry.name); + const rightPath = join(right, entry.name); + if (entry.isSymbolicLink() || other.isSymbolicLink()) return false; + if (entry.isDirectory() !== other.isDirectory()) return false; + if (entry.isDirectory()) { + if (!treesMatch(leftPath, rightPath)) return false; + } else if (entry.isFile() && other.isFile()) { + if (!readFileSync(leftPath).equals(readFileSync(rightPath))) return false; + } else { + return false; + } + } + return true; +} + +function legacyMatchesSource(harness, skill, legacyPath) { + const expectedRoot = mkdtempSync(join(tmpdir(), "mstack-migrate-")); + try { + const expected = join(expectedRoot, skill); + copyDirectoryContents(join(sourceRoot, skill), expected); + applyAdapter(expected, harness, skill); + return treesMatch(legacyPath, expected); + } catch { + return false; + } finally { + rmSync(expectedRoot, { recursive: true, force: true }); + } +} + +function migrationPlanLines() { + const lines = []; + for (const harness of harnesses) { + const { legacy, entries } = legacySkillCandidates(harness); + if (!legacy || !entries.length) continue; + for (const entry of entries) { + if (!entry.selected) lines.push(` migrate: keep ${entry.path} (not selected by this install)`); + else if (legacyMatchesSource(harness, entry.name, entry.path)) { + lines.push(` migrate: remove ${entry.path} (matches the current mstack source)`); + } else { + lines.push(` migrate: keep ${entry.path} (differs from the current mstack source)`); + } + } + } + return lines; +} + +function migrationHints() { + const lines = []; + for (const harness of harnesses) { + const { legacy, entries } = legacySkillCandidates(harness); + if (legacy && entries.length) { + lines.push(`hint: ${harness} also discovers ${legacy}; run with --migrate to remove redundant legacy copies there`); + } + } + return lines; +} + +function crossDiscoveryNotes() { + const notes = []; + for (const harness of harnesses) { + for (const discovered of harnessRegistry[harness].skills?.discovers ?? []) { + if (discovered === "own" || !harnessRegistry[discovered]) continue; + const discoveredTarget = targets[discovered]; + if (localPathKey(discoveredTarget) === localPathKey(targets[harness])) continue; + if (!harnesses.includes(discovered) && !existsSync(discoveredTarget)) continue; + notes.push( + `note: ${harness} also discovers ${discoveredTarget} (${discovered}); ` + + "same-named skills in both directories may be reported as duplicate definitions", + ); + } + } + return [...new Set(notes)]; +} + +const targetGroups = []; +const groupByTarget = new Map(); +for (const harness of harnesses) { + const key = localPathKey(targets[harness]); + let group = groupByTarget.get(key); + if (!group) { + group = { harnesses: [], target: targets[harness] }; + groupByTarget.set(key, group); + targetGroups.push(group); + } + group.harnesses.push(harness); +} +for (const group of targetGroups) { + const shared = group.harnesses.length > 1 ? " (shared)" : ""; + console.log(`${group.harnesses.join(", ")}: ${group.target}${shared}`); +} for (const item of plan.filter(({ kind }) => kind === "artifact")) { console.log(` artifact ${item.name}: ${item.target}`); } @@ -438,6 +577,12 @@ if (dryRun) { const action = existsSync(item.target) ? (replace ? "replace with backup" : "conflict") : "install"; console.log(` ${action}: ${item.kind === "skill" ? item.skill : `artifact/${item.name}`}`); } + for (const note of crossDiscoveryNotes()) console.log(note); + if (migrate) { + for (const line of migrationPlanLines()) console.log(line); + } else { + for (const line of migrationHints()) console.log(line); + } if (conflicts.length && !replace) process.exitCode = 2; else console.log("Dry run complete."); process.exit(); @@ -639,3 +784,63 @@ try { const skillCount = plan.filter(({ kind }) => kind === "skill").length; const artifactCount = plan.length - skillCount; console.log(`Installed ${skillCount} skill copies and ${artifactCount} artifact copies; replaced ${conflicts.length}.`); + +function runMigration() { + const failures = []; + const migrationLocks = []; + const legacyDirs = harnesses + .map((harness) => legacySkillCandidates(harness).legacy) + .filter((legacy) => legacy && existsSync(legacy)) + .sort((left, right) => left.localeCompare(right)); + try { + for (const legacy of legacyDirs) { + const lockPath = join(legacy, ".harness-skills-install.lock"); + const descriptor = openInstallLock(lockPath, lockOwner); + migrationLocks.push({ descriptor, lockPath, owner: lockOwner }); + } + for (const harness of harnesses) { + const { legacy, entries } = legacySkillCandidates(harness); + if (!legacy || !entries.length) continue; + for (const entry of entries) { + if (!entry.selected) { + console.log(`migrate: kept ${entry.path} (not selected by this install)`); + continue; + } + if (!legacyMatchesSource(harness, entry.name, entry.path)) { + console.log(`migrate: kept ${entry.path} (differs from the current mstack source)`); + continue; + } + const backup = join(legacy, ".harness-skills-backups", stamp, entry.name); + try { + mkdirSync(dirname(backup), { recursive: true, mode: 0o700 }); + renameSync(entry.path, backup); + console.log(`migrate: removed ${entry.path} (backup: ${backup})`); + } catch (error) { + failures.push(`${entry.path}: ${error.message}`); + } + } + } + } catch (error) { + failures.push(error.message); + } finally { + for (const { descriptor, lockPath, owner } of migrationLocks.reverse()) { + closeSync(descriptor); + try { + if (readFileSync(lockPath, "utf8").trim() === owner) unlinkSync(lockPath); + } catch { + // A missing lock is already released; leave a changed lock to its owner. + } + } + } + return failures; +} + +if (!migrate) { + for (const line of migrationHints()) console.log(line); +} else { + const failures = runMigration(); + if (failures.length) { + console.error(`Migration completed with failures:\n${failures.map((line) => ` ${line}`).join("\n")}`); + process.exitCode = 1; + } +} diff --git a/scripts/install.test.mjs b/scripts/install.test.mjs index cc925a8..476ea1b 100644 --- a/scripts/install.test.mjs +++ b/scripts/install.test.mjs @@ -33,6 +33,24 @@ function fixture() { }; } +function defaultHomeFixture() { + const root = mkdtempSync(join(tmpdir(), "mstack 默认目录测试-")); + const env = { ...process.env, HOME: join(root, "home"), USERPROFILE: join(root, "home") }; + for (const key of [ + "HARNESS_SKILLS_CODEX_DIR", + "HARNESS_SKILLS_CLAUDE_DIR", + "HARNESS_SKILLS_OPENCODE_DIR", + "HARNESS_SKILLS_PI_DIR", + "CLAUDE_CONFIG_DIR", + "XDG_CONFIG_HOME", + "PI_CODING_AGENT_DIR", + "MSTACK_ENVIRONMENTS_FILE", + ]) { + delete env[key]; + } + return { root, home: env.HOME, env }; +} + function run(arguments_, env) { return spawnSync(process.execPath, [installer, ...arguments_], { cwd: resolve("."), @@ -132,15 +150,30 @@ test("recovers a dead installer lock but preserves a live lock", () => { } }); -test("uses pi's agent directory for the default skill target", () => { - const { env } = fixture(); - env.USERPROFILE = join(env.TEMP ?? tmpdir(), "mstack-pi-home"); - env.HOME = env.USERPROFILE; - delete env.HARNESS_SKILLS_PI_DIR; +test("installs shared skills once per discovery directory by default", () => { + const { root, home, env } = defaultHomeFixture(); + try { + const result = run(["--harness", "all", "--skill", "meta-mode,show-me-your-work"], env); + assert.equal(result.status, 0, result.stderr); - const result = run(["--harness", "pi", "--skill", "meta-mode", "--dry-run"], env); - assert.equal(result.status, 0, result.stderr); - assert.ok(result.stdout.includes(join(env.USERPROFILE, ".pi", "agent", "skills"))); + const shared = join(home, ".agents", "skills"); + const claude = join(home, ".claude", "skills"); + assert.deepEqual(readdirSync(shared).sort(), ["meta-mode", "show-me-your-work"]); + assert.deepEqual(readdirSync(claude).sort(), ["meta-mode", "show-me-your-work"]); + assert.equal(existsSync(join(home, ".config", "opencode", "skills")), false); + assert.equal(existsSync(join(home, ".pi", "agent", "skills")), false); + + const sharedFrontmatter = readFileSync(join(shared, "show-me-your-work", "SKILL.md"), "utf8"); + assert.match(sharedFrontmatter, /^metadata:/m); + const claudeFrontmatter = readFileSync(join(claude, "show-me-your-work", "SKILL.md"), "utf8"); + assert.match(claudeFrontmatter, /^compatibility:/m); + + assert.match(result.stdout, /^codex, opencode, pi: .+ \(shared\)$/m); + assert.match(result.stdout, /^claude: .+$/m); + assert.match(result.stdout, /Installed 4 skill copies and 0 artifact copies/); + } finally { + rmSync(root, { recursive: true, force: true }); + } }); test("installs harness-specific frontmatter and preserves conflicts", () => { @@ -702,6 +735,103 @@ test("rejects artifact sources outside the repository", () => { } }); +test("deduplicates aliased skill directories when harness adapters agree", () => { + const { root, env } = fixture(); + env.HARNESS_SKILLS_CLAUDE_DIR = env.HARNESS_SKILLS_CODEX_DIR; + try { + const result = run(["--harness", "codex,claude", "--skill", "meta-mode"], env); + assert.equal(result.status, 0, result.stderr); + assert.equal(existsSync(join(env.HARNESS_SKILLS_CODEX_DIR, "meta-mode", "SKILL.md")), true); + assert.match(result.stdout, /^codex, claude: .+ \(shared\)$/m); + assert.match(result.stdout, /Installed 1 skill copies and 0 artifact copies/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("dry-run reports cross-discovery notes and legacy migration hints", () => { + const { root, home, env } = defaultHomeFixture(); + try { + const legacyPi = join(home, ".pi", "agent", "skills", "meta-mode"); + mkdirSync(legacyPi, { recursive: true }); + writeFileSync(join(legacyPi, "marker.txt"), "old", "utf8"); + const legacyOpencode = join(home, ".config", "opencode", "skills", "unrelated-skill"); + mkdirSync(legacyOpencode, { recursive: true }); + writeFileSync(join(legacyOpencode, "marker.txt"), "user", "utf8"); + + const result = run(["--harness", "all", "--skill", "meta-mode", "--dry-run"], env); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /note: opencode also discovers .+ \(claude\); same-named skills/); + assert.match(result.stdout, /hint: pi also discovers .+\.pi.+agent.+skills/); + assert.doesNotMatch(result.stdout, /hint: opencode also discovers/); + assert.equal(existsSync(legacyPi), true); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("dry-run with --migrate plans removals without touching the file system", () => { + const { root, home, env } = defaultHomeFixture(); + try { + const legacyPi = join(home, ".pi", "agent", "skills"); + cpSync(resolve("skills", "meta-mode"), join(legacyPi, "meta-mode"), { recursive: true }); + writeFileSync(join(legacyPi, "meta-mode", "user-note.txt"), "keep", "utf8"); + const legacyOpencode = join(home, ".config", "opencode", "skills"); + cpSync(resolve("skills", "meta-mode"), join(legacyOpencode, "meta-mode"), { recursive: true }); + + const result = run(["--harness", "pi,opencode", "--skill", "meta-mode", "--migrate", "--dry-run"], env); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /migrate: remove .+meta-mode \(matches the current mstack source\)/); + assert.match(result.stdout, /migrate: keep .+meta-mode \(differs from the current mstack source\)/); + assert.equal(existsSync(join(legacyPi, "meta-mode", "user-note.txt")), true); + assert.equal(existsSync(join(legacyOpencode, "meta-mode", "SKILL.md")), true); + assert.equal(existsSync(join(home, ".agents", "skills")), false); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("migrate removes matching legacy copies and preserves modified or unrelated skills", () => { + const { root, home, env } = defaultHomeFixture(); + try { + const legacyPi = join(home, ".pi", "agent", "skills"); + const legacyOpencode = join(home, ".config", "opencode", "skills"); + cpSync(resolve("skills", "meta-mode"), join(legacyPi, "meta-mode"), { recursive: true }); + cpSync(resolve("skills", "show-me-your-work"), join(legacyPi, "show-me-your-work"), { recursive: true }); + writeFileSync(join(legacyPi, "show-me-your-work", "user-note.txt"), "keep me", "utf8"); + cpSync(resolve("skills", "bro"), join(legacyPi, "bro"), { recursive: true }); + mkdirSync(join(legacyPi, "user-own-tool"), { recursive: true }); + writeFileSync(join(legacyPi, "user-own-tool", "SKILL.md"), "---\nname: user-own-tool\ndescription: mine\n---\n", "utf8"); + cpSync(resolve("skills", "meta-mode"), join(legacyOpencode, "meta-mode"), { recursive: true }); + + const result = run( + ["--harness", "pi,opencode", "--skill", "meta-mode,show-me-your-work", "--migrate"], + env, + ); + assert.equal(result.status, 0, result.stderr); + + const shared = join(home, ".agents", "skills"); + assert.equal(existsSync(join(shared, "meta-mode", "SKILL.md")), true); + assert.equal(existsSync(join(shared, "show-me-your-work", "SKILL.md")), true); + + assert.equal(existsSync(join(legacyPi, "meta-mode")), false); + assert.equal(existsSync(join(legacyOpencode, "meta-mode")), false); + assert.ok(findFile(join(legacyPi, ".harness-skills-backups"), "SKILL.md"), "expected a backup of the removed legacy copy"); + assert.ok(findFile(join(legacyOpencode, ".harness-skills-backups"), "SKILL.md"), "expected a backup beside the opencode legacy copy"); + + assert.equal(readFileSync(join(legacyPi, "show-me-your-work", "user-note.txt"), "utf8"), "keep me"); + assert.equal(existsSync(join(legacyPi, "bro", "SKILL.md")), true); + assert.equal(existsSync(join(legacyPi, "user-own-tool", "SKILL.md")), true); + + assert.match(result.stdout, /migrate: removed .+meta-mode \(backup: .+\)/); + assert.match(result.stdout, /migrate: kept .+show-me-your-work \(differs from the current mstack source\)/); + assert.match(result.stdout, /migrate: kept .+bro \(not selected by this install\)/); + assert.doesNotMatch(result.stdout, /hint: /); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + function findFile(root, name) { const entries = readdirSync(root, { withFileTypes: true }); for (const entry of entries) { diff --git a/scripts/smoke-harnesses.mjs b/scripts/smoke-harnesses.mjs index 40d56e8..30d54b4 100644 --- a/scripts/smoke-harnesses.mjs +++ b/scripts/smoke-harnesses.mjs @@ -7,6 +7,7 @@ import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; import { processInvocation } from "./runtime-lib.mjs"; import { parseCliArgs } from "./cli-args.mjs"; +import { skillsTarget } from "./harness-targets.mjs"; const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); const registry = JSON.parse(readFileSync(join(repoRoot, "profiles", "harnesses.json"), "utf8")); @@ -14,21 +15,8 @@ const options = parseCliArgs(process.argv.slice(2), [ "--harness", "--skill", "--file", "--model", "--parent-model", ], ["--execute", "--require-installed", "--json"]); -function expandHome(path) { - if (path === "~") return homedir(); - return path?.startsWith("~/") || path?.startsWith("~\\") ? join(homedir(), path.slice(2)) : path; -} - function targetFor(harness) { - const config = registry[harness]; - const override = process.env[config.directoryVariable]; - if (override) return resolve(expandHome(override)); - if (config.fallback) return config.fallback.reduce((path, part) => join(path, part), homedir()); - const configuredRoot = process.env[config.configVariable]; - const configRoot = configuredRoot - ? resolve(expandHome(configuredRoot)) - : join(homedir(), config.configFallback); - return join(configRoot, ...(config.prefix ?? []), ...(config.suffix ?? [])); + return skillsTarget(registry, harness, { env: process.env, home: homedir() }); } function run(command, commandArgs) {