From 7b49231882ac150451f87bf01007217d49842f7f Mon Sep 17 00:00:00 2001 From: 3metaJun <251347867+3metaJun@users.noreply.github.com> Date: Tue, 6 Oct 2026 18:36:42 +0800 Subject: [PATCH 1/4] feat(install): add project-scope install Co-Authored-By: Claude Sonnet 5.5 --- README.md | 44 ++++++++++ docs/harness-adapters.md | 28 ++++++- docs/t3code.md | 17 ++++ profiles/harnesses.json | 6 ++ scripts/harness-targets.mjs | 41 ++++++++++ scripts/harness-targets.test.mjs | 46 ++++++++++- scripts/install.mjs | 70 ++++++++++++++-- scripts/install.test.mjs | 136 +++++++++++++++++++++++++++++++ scripts/skill-discovery.test.mjs | 20 +++++ 9 files changed, 397 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index cb20b11..1a8eda9 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,50 @@ skills and failed replacements must stay outside discovery roots because OpenCode also scans hidden subdirectories. Artifact backups stay beside the artifact target. The installer prints each backup path and retains its contents. +### Install into a project + +Some harnesses read project skills only from the working directory, which matters +when an app starts agents in per-thread git worktrees such as +[T3 Code](docs/t3code.md#project-install-for-t3-worktrees). `--project` copies +skills into an existing local repository instead of the user directories: + +```bash +npx @3metajun/mstack --harness codex,claude,pi --project /path/to/repo --dry-run +npx @3metajun/mstack --harness codex,claude,pi --project /path/to/repo +``` + +| Harness | Project root under `` | Reads it from | +| --- | --- | --- | +| Codex | `.agents/skills/` | the working directory up to the repository root | +| OpenCode | `.agents/skills/` | the working directory up to the git worktree root (also reads `.opencode/skills/`) | +| pi | `.agents/skills/` | the working directory up to the repository root | +| Grok | `.agents/skills/` | every directory between the working directory and the repository root | +| Antigravity (`agy`) | `.agents/skills/` | the working directory (also reads `.gemini/skills/` and `.agent/skills/`) | +| Claude Code | `.claude/skills/` | the working directory only | + +Codex, OpenCode, pi, Grok and Antigravity share one copy per skill in +`.agents/skills/`. Cursor reads that directory and `.claude/skills/` too, so it +needs no target of its own. Claude gets the same adapter output as at user level +(no frontmatter `name`). Project installs copy and never link, keep the +installer's staging, locking, rollback and `--replace` backup behavior, and +ignore `HARNESS_SKILLS_*_DIR` overrides. + +Rules: + +- The directory must exist. The installer refuses your home directory (use a + user-level install), anything inside the mstack package, and any path inside + `.harness-skills-*` transaction storage. +- Skills only. `--artifact` is refused with `--project`: agent roles, tools, the + guide and session context are user-owned configuration and should not land in a + repository implicitly. `--migrate` and `--environment` (including SSH) are + refused too. +- `--replace` backups go to `/.agents/.harness-skills-backups/` and + `/.claude/.harness-skills-backups/`, beside the skill roots and never under + them. Delete them when you no longer need them, and add `.harness-skills-*` to + `.gitignore`. +- Commit the installed skills deliberately, or gitignore them. A new git worktree + holds only committed files, so uncommitted project skills do not appear in it. + ## Use with Claude Code and Claude Desktop Claude Code and the Code tab of the Claude desktop app share one skill system, diff --git a/docs/harness-adapters.md b/docs/harness-adapters.md index 95a49b2..7385fad 100644 --- a/docs/harness-adapters.md +++ b/docs/harness-adapters.md @@ -9,8 +9,8 @@ to the official documentation used for each entry. | --- | --- | --- | --- | | Codex | `~/.agents/skills/` | `.agents/skills/` | Agent Skills fields; Codex-specific `agents/openai.yaml` lives beside each skill | | Claude Code | `~/.claude/skills/` | `.claude/skills/` | `name`, `description`, `license`, `compatibility`, `metadata`, and Claude invocation fields | -| OpenCode | `~/.agents/skills/`, `~/.config/opencode/skills/`, `~/.claude/skills/` | `.opencode/skills/` | `license`, `compatibility`, and `metadata` | -| pi | `~/.agents/skills/`, `~/.pi/agent/skills/` | `.pi/skills/` | `license`, `compatibility`, `metadata`, `allowed-tools`, and `disable-model-invocation` | +| OpenCode | `~/.agents/skills/`, `~/.config/opencode/skills/`, `~/.claude/skills/` | `.opencode/skills/`, `.claude/skills/`, `.agents/skills/` | `license`, `compatibility`, and `metadata` | +| pi | `~/.agents/skills/`, `~/.pi/agent/skills/` | `.pi/skills/`, `.agents/skills/` | `license`, `compatibility`, `metadata`, `allowed-tools`, and `disable-model-invocation` | | Antigravity (`agy`) | `~/.gemini/config/skills/`, `~/.gemini/antigravity-cli/skills/` | `.gemini/skills/`, `.agents/skills/`, `.agent/skills/` | `name` (required) and `description`; other fields are not documented | | Grok | `~/.grok/skills/`, `~/.agents/skills/`, `~/.claude/skills/`, `~/.cursor/skills/` | `.grok/skills/`, `.agents/skills/`, `.claude/skills/`, `.cursor/skills/` | `name`, `description`, `license`, `compatibility`, `metadata`, `allowed-tools`, `user-invocable`, and `disable-model-invocation` | | Cursor | `~/.cursor/skills/`, `~/.agents/skills/` | `.cursor/skills/`, `.agents/skills/`, `.claude/skills/` | Agent Skills fields | @@ -31,6 +31,30 @@ Sources: which listed skills from `~/.agents/skills/`. - [Cursor skills](https://cursor.com/docs/context/skills) +### Project install targets + +`--project ` writes to one project root per harness, defined as `project` in +`profiles/harnesses.json`. Where a harness reads several project directories, +mstack picks the one that lets harnesses share a copy: + +| Harness | mstack project target | Evidence | +| --- | --- | --- | +| Codex | `.agents/skills/` | Codex skills documentation | +| OpenCode | `.agents/skills/` | The OpenCode skills page lists `.opencode/skills/`, `.claude/skills/` and `.agents/skills/` as project paths and walks up to the git worktree root | +| pi | `.agents/skills/` | The pi skills page: project `.agents/skills/` directories are discovered from the working directory through its ancestors, stopping at the repository root. `.pi/skills/` is also read | +| Grok | `.agents/skills/` | Grok user guide: scans `.agents/skills/` at each tier and walks every directory between the working directory and the repository root | +| Antigravity | `.agents/skills/` | T3 Code `AntigravitySkills.ts` scans `/.gemini/skills`, `/.agents/skills` and `/.agent/skills`. Its canonical frontmatter is unchanged, so it shares the copy | +| Claude Code | `.claude/skills/` | Claude reads only `/.claude/skills` (T3 Code `ClaudeSkills.ts`); it needs its own copy without the frontmatter `name` | +| Cursor | none needed | T3 Code `CursorSkills.ts` scans `/.cursor`, `.agents`, `.codex` and `.claude` skills, so the two targets above cover it | + +Unlike the user level, Antigravity joins the shared copy because it does read +`.agents/skills/` in a project. The Cursor, Claude and Antigravity rows were +checked against T3 Code commit `8f75697`. The OpenCode, pi and Grok rows come from +their documentation and were not run in a thread. Project installs are skills +only: agent files have project locations for only some harnesses +(`.claude/agents`, `.opencode/agents`, `.codex/agents`), in different formats, so +`--artifact` is refused with `--project`. + 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/docs/t3code.md b/docs/t3code.md index dee8e23..402faa7 100644 --- a/docs/t3code.md +++ b/docs/t3code.md @@ -82,6 +82,23 @@ By default T3 creates a worktree for a thread at: - `LAST_CHAT` scans Claude Code, Codex, and pi transcripts only. Threads run through Cursor, Grok, OpenCode, or Antigravity, and T3's own thread history, are not read, so those worktrees can look idle. Set `MSTACK_TRANSCRIPTS_DIR` to scan one directory instead. - Treat its `safe` bucket as a report. T3 has its own cleanup under **Settings → Storage** that removes only T3-managed worktrees without running sessions or uncommitted work. I did not check how T3 reacts when a worktree it manages is removed with `git worktree remove`, so use T3's cleanup or thread deletion for those. +## Project install for T3 worktrees + +A user-level install is the simplest route. When a provider should find mstack only for one repository, or Claude and Antigravity need a copy in the thread's own directory, install into the project: + +```bash +npx @3metajun/mstack --harness codex,claude,pi --project /path/to/repo --dry-run +npx @3metajun/mstack --harness codex,claude,pi --project /path/to/repo +``` + +This copies the skills into `/.agents/skills` (Codex, Pi, OpenCode, Grok, Antigravity) and `/.claude/skills` (Claude Code). Cursor reads both. The README has the [per-harness table](../README.md#install-into-a-project) and the rules: skills only, existing directory required, `--replace` backups under `.harness-skills-backups/` beside the skill roots. + +- **Commit it or ignore it, on purpose.** T3 creates a worktree with `git worktree add` from a ref (`GitVcsDriverCore.ts` in T3 commit `8f75697`). A new worktree therefore holds only what is committed at that ref. Project skills you installed but did not commit exist in the main checkout and are missing from a fresh T3 thread. If the team should share them, commit them to the base branch before starting threads. If they are personal, add `.agents/skills/`, `.claude/skills/` and `.harness-skills-*` to `.gitignore` (or `.git/info/exclude`) and install again in each worktree, or keep using the user-level install. +- **An existing thread's worktree** has its own copy of the tree. Run the installer with `--project ` to add skills there, then restart the agent session. +- **Precedence.** For Claude, a user copy beats a project copy of the same name. Cursor deduplicates by name with project roots first. Re-run the install after updating mstack, with `--replace`, to refresh a project copy. +- **Pi** still needs the project approved in Pi before it lists project skills. +- Not checked in a live T3 thread: that OpenCode, Pi, and Grok list `.agents/skills` from a worktree cwd. The paths come from their documentation. + ## A `t3.json` setup script T3 reads `t3.json` at the repository root. An invalid file is ignored as a whole, including its icon and scripts. For a project that uses mstack, a worktree starts from the committed tree, so skills and `.harness/` files are already there. The one gap is usually dependencies and gitignored files. This recipe installs dependencies before the agent starts and makes new threads default to worktrees: diff --git a/profiles/harnesses.json b/profiles/harnesses.json index 2a6ec61..2ba8ceb 100644 --- a/profiles/harnesses.json +++ b/profiles/harnesses.json @@ -1,6 +1,7 @@ { "codex": { "directoryVariable": "HARNESS_SKILLS_CODEX_DIR", + "project": [".agents", "skills"], "fallback": [".agents", "skills"], "runtime": { "command": "codex", @@ -11,6 +12,7 @@ }, "claude": { "directoryVariable": "HARNESS_SKILLS_CLAUDE_DIR", + "project": [".claude", "skills"], "configVariable": "CLAUDE_CONFIG_DIR", "configFallback": ".claude", "suffix": ["skills"], @@ -23,6 +25,7 @@ }, "opencode": { "directoryVariable": "HARNESS_SKILLS_OPENCODE_DIR", + "project": [".agents", "skills"], "sharedSkills": true, "configVariable": "XDG_CONFIG_HOME", "configFallback": ".config", @@ -37,6 +40,7 @@ }, "pi": { "directoryVariable": "HARNESS_SKILLS_PI_DIR", + "project": [".agents", "skills"], "sharedSkills": true, "configVariable": "PI_CODING_AGENT_DIR", "configFallback": ".pi/agent", @@ -50,6 +54,7 @@ }, "antigravity": { "directoryVariable": "HARNESS_SKILLS_ANTIGRAVITY_DIR", + "project": [".agents", "skills"], "fallback": [".gemini", "antigravity-cli", "skills"], "runtime": { "command": "agy", @@ -61,6 +66,7 @@ }, "grok": { "directoryVariable": "HARNESS_SKILLS_GROK_DIR", + "project": [".agents", "skills"], "sharedSkills": true, "fallback": [".grok", "skills"], "runtime": { diff --git a/scripts/harness-targets.mjs b/scripts/harness-targets.mjs index 71776ba..f59f8eb 100644 --- a/scripts/harness-targets.mjs +++ b/scripts/harness-targets.mjs @@ -1,5 +1,7 @@ +import { statSync } from "node:fs"; import { homedir } from "node:os"; import { isAbsolute, join, resolve } from "node:path"; +import { pathIsWithin, physicalPathKey } from "./install-paths.mjs"; export function configuredPath(value, fallback, label, home = homedir()) { const path = value ?? fallback; @@ -32,3 +34,42 @@ export function resolveHarnessRoots(registry, { env = process.env, home = homedi } return { skills, native, legacy, sharedRoot, externalClaudeRoot }; } + +const TRANSACTION_SEGMENT = /^\.harness-skills-/; + +// Project roots come only from the registry's `project` path; user-scope overrides never apply. +export function resolveProjectRoots(registry, projectDir) { + const roots = {}; + for (const [harness, config] of Object.entries(registry)) { + const parts = config.project; + if (!Array.isArray(parts) || parts.length === 0 || parts.some((part) => + typeof part !== "string" || !part.trim() || part === "." || part === ".." || /[\\/]/.test(part))) { + throw new Error(`Harness ${harness} has no safe project skill root`); + } + roots[harness] = join(projectDir, ...parts); + } + return roots; +} + +export function validateProjectDir(value, { packageRoot, home = homedir(), cwd = process.cwd() } = {}) { + const expanded = value === "~" ? home : /^~[\\/]/.test(value) ? join(home, value.slice(2)) : value; + const project = resolve(cwd, expanded); + let status; + try { + status = statSync(project); + } catch { + throw new Error(`--project must name an existing directory: ${project}`); + } + if (!status.isDirectory()) throw new Error(`--project must name a directory: ${project}`); + if (project.split(/[\\/]/).some((segment) => TRANSACTION_SEGMENT.test(segment))) { + throw new Error(`--project cannot be inside installer transaction storage: ${project}`); + } + const physical = physicalPathKey(project); + if (physical === physicalPathKey(home)) { + throw new Error("--project cannot be the home directory; omit --project for a user-level install"); + } + if (packageRoot && pathIsWithin(physicalPathKey(packageRoot), physical)) { + throw new Error(`--project cannot be inside the mstack package: ${project}`); + } + return project; +} diff --git a/scripts/harness-targets.test.mjs b/scripts/harness-targets.test.mjs index 28b4699..a1af042 100644 --- a/scripts/harness-targets.test.mjs +++ b/scripts/harness-targets.test.mjs @@ -4,7 +4,7 @@ import { tmpdir } from "node:os"; import { delimiter, join, resolve } from "node:path"; import { spawnSync } from "node:child_process"; import test from "node:test"; -import { configuredPath, resolveHarnessRoots } from "./harness-targets.mjs"; +import { configuredPath, resolveHarnessRoots, resolveProjectRoots, validateProjectDir } from "./harness-targets.mjs"; const repository = resolve("."); const registry = JSON.parse(readFileSync(join(repository, "profiles", "harnesses.json"), "utf8")); @@ -82,6 +82,50 @@ test("configured paths expand home and reject relative overrides at the common b assert.throws(() => resolveHarnessRoots(registry, { home, env: {}, environmentTargets: { pi: "relative" }, environmentName: "example" }), /example.pi must be an absolute path/); }); +test("project roots come from the registry and ignore user-scope variables", () => { + const project = resolve("fixture-project"); + const roots = resolveProjectRoots(registry, project); + assert.deepEqual(roots, { + codex: join(project, ".agents", "skills"), + claude: join(project, ".claude", "skills"), + opencode: join(project, ".agents", "skills"), + pi: join(project, ".agents", "skills"), + antigravity: join(project, ".agents", "skills"), + grok: join(project, ".agents", "skills"), + }); + for (const [harness, config] of Object.entries(registry)) { + assert.deepEqual(config.project, [harness === "claude" ? ".claude" : ".agents", "skills"], harness); + } + assert.throws(() => resolveProjectRoots({ codex: { project: ["..", "skills"] } }, project), /no safe project skill root/); + assert.throws(() => resolveProjectRoots({ codex: { project: ["a/b"] } }, project), /no safe project skill root/); + assert.throws(() => resolveProjectRoots({ codex: {} }, project), /no safe project skill root/); +}); + +test("project directories must exist, be directories, and stay out of home, the package, and transaction storage", (t) => { + const root = mkdtempSync(join(tmpdir(), "mstack-project-dir-")); + t.after(() => rmSync(root, { recursive: true, force: true })); + const fakeHome = join(root, "home"); + const packageRoot = join(root, "package"); + const project = join(root, "project"); + const file = join(root, "file"); + for (const directory of [fakeHome, packageRoot, join(packageRoot, "skills"), project, join(root, ".harness-skills-backups", "p")]) { + mkdirSync(directory, { recursive: true }); + } + writeFileSync(file, "x", "utf8"); + const options = { packageRoot, home: fakeHome, cwd: root }; + assert.equal(validateProjectDir(project, options), project); + assert.equal(validateProjectDir("project", options), project); + assert.throws(() => validateProjectDir(join(root, "missing"), options), /existing directory/); + assert.throws(() => validateProjectDir(file, options), /must name a directory/); + assert.throws(() => validateProjectDir(fakeHome, options), /home directory/); + assert.throws(() => validateProjectDir("~", options), /home directory/); + assert.throws(() => validateProjectDir(packageRoot, options), /inside the mstack package/); + assert.throws(() => validateProjectDir(join(packageRoot, "skills"), options), /inside the mstack package/); + assert.throws(() => validateProjectDir(join(root, ".harness-skills-backups", "p"), options), /transaction storage/); + // A repository that merely contains the package, such as one with it in node_modules, is a valid project. + assert.equal(validateProjectDir(root, options), root); +}); + for (const explicit of [false, true]) { test(`smoke finds skills from a real isolated ${explicit ? "explicit" : "default"} installation`, (t) => { const root = mkdtempSync(join(tmpdir(), "mstack-harness-targets-")); diff --git a/scripts/install.mjs b/scripts/install.mjs index 7d655b4..8aa9100 100644 --- a/scripts/install.mjs +++ b/scripts/install.mjs @@ -11,16 +11,17 @@ import { realpathSync, readdirSync, renameSync, + rmdirSync, rmSync, unlinkSync, writeFileSync, } from "node:fs"; import { homedir } from "node:os"; -import { basename, dirname, isAbsolute, join, resolve } from "node:path"; +import { basename, dirname, isAbsolute, join, resolve, sep } from "node:path"; import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; import { readEnvironment } from "./environment-lib.mjs"; -import { configuredPath, resolveHarnessRoots } from "./harness-targets.mjs"; +import { configuredPath, resolveHarnessRoots, resolveProjectRoots, validateProjectDir } from "./harness-targets.mjs"; import { planSkillMigration } from "./install-migration.mjs"; import { convertAgentMarkdown } from "./agent-format.mjs"; import { @@ -56,14 +57,20 @@ 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 [--project ] [--environment ] [--skill ] [--artifact ] [--no-skills] [--dry-run] [--replace] Installs canonical skills and optional artifacts into user-level harness directories. Artifacts: ${validArtifacts.join(", ")} (use --artifact all for installable artifacts). Codex, OpenCode, pi and Grok share ~/.agents/skills by default. Antigravity reads only its own ~/.gemini/antigravity-cli/skills. Claude uses directory-based names. Use --migrate --replace to back up recognized legacy copies outside skill discovery roots. -Existing directories are preserved unless --replace is supplied.`); +Existing directories are preserved unless --replace is supplied. + +--project installs skills only, as copies, into an existing local repository instead of +the user directories: /.agents/skills for Codex, OpenCode, pi, Grok and Antigravity, and +/.claude/skills for Claude. Commit the result deliberately or gitignore it; a fresh git +worktree contains only committed files. It cannot be combined with --artifact, --migrate or +--environment, and ignores HARNESS_SKILLS_*_DIR overrides.`); process.exit(args.includes("--help") ? 0 : 1); } @@ -87,6 +94,21 @@ const environmentName = valueAfter("--environment"); if (hasEnvironment && (!environmentName || environmentName.startsWith("--"))) { throw new Error("--environment requires a name"); } +const hasProject = args.includes("--project"); +const projectValue = valueAfter("--project"); +if (hasProject && (!projectValue?.trim() || projectValue.startsWith("--"))) { + throw new Error("--project requires a directory"); +} +if (hasProject && hasEnvironment) { + throw new Error("--project cannot be combined with --environment: project installs write only to a local repository"); +} +if (hasProject && migrate) throw new Error("--migrate applies to user-level installs and cannot be combined with --project"); +if (hasProject && args.includes("--artifact")) { + // Agent roles, tools, the guide and session context are user-owned configuration; none has a verified + // project location for every harness, so a repository never receives them implicitly. + throw new Error("--artifact is not supported with --project: project installs copy skills only. Install artifacts at user level or copy them into the repository deliberately."); +} +const projectDir = hasProject ? validateProjectDir(projectValue, { packageRoot: repoRoot, home: userHome }) : undefined; function artifactBase(name, harness) { const base = artifactRegistry[name]?.harnesses?.[harness]?.base; @@ -108,11 +130,21 @@ if (environment.transport === "ssh") { } const environmentTargets = environment.targets; const environmentArtifacts = environment.artifacts; -const { skills: targets, native: nativeTargets, legacy: legacyTargets, sharedRoot, externalClaudeRoot } = +const { skills: userTargets, native: nativeTargets, legacy: legacyTargets, sharedRoot, externalClaudeRoot } = resolveHarnessRoots(harnessRegistry, { home: userHome, environmentTargets, environmentName }); +const targets = projectDir ? resolveProjectRoots(harnessRegistry, projectDir) : userTargets; const discoveryRoots = [...new Set([ - ...Object.values(targets), ...Object.values(nativeTargets), ...Object.values(legacyTargets), sharedRoot, externalClaudeRoot, + ...Object.values(targets), ...Object.values(userTargets), ...Object.values(nativeTargets), ...Object.values(legacyTargets), sharedRoot, externalClaudeRoot, ].map(physicalPathKey))]; +if (projectDir) { + // A symlinked project directory could alias the home skill roots, which is a user-level install. + const userKeys = new Set([...Object.values(userTargets), ...Object.values(nativeTargets), ...Object.values(legacyTargets), sharedRoot, externalClaudeRoot].map(physicalPathKey)); + for (const harness of harnesses) { + if (userKeys.has(physicalPathKey(targets[harness]))) { + throw new Error(`--project resolves ${harness} to a user-level skill root: ${targets[harness]}`); + } + } +} function artifactEnvironmentPath(name, harness) { const byArtifact = environmentArtifacts[name]; @@ -412,7 +444,7 @@ for (const item of harnesses.flatMap((harness) => skillPlan.push(item); } } -const migrationEnabled = skills.length > 0 && !hasEnvironment && skillPlan.some((item) => +const migrationEnabled = skills.length > 0 && !hasEnvironment && !projectDir && skillPlan.some((item) => item.harness !== "claude" && localPathKey(dirname(item.target)) === localPathKey(sharedRoot)); if (migrate && !migrationEnabled) { throw new Error("--migrate requires a local default shared skill target; run the installer on the target machine without --environment"); @@ -446,6 +478,7 @@ for (const item of plan) { for (const category of ["backups", "failed", "stage"]) transactionPath(item, category); } +if (projectDir) console.log(`project: ${projectDir}`); for (const harness of harnesses) console.log(`${harness}: ${targets[harness]}`); for (const item of skillPlan.filter((item) => item.consumers.length > 1)) { console.log(` shared ${item.name}: ${item.target} (${item.consumers.join(", ")})`); @@ -491,6 +524,21 @@ function transactionPath(item, category) { return path; } +// Skill staging lives under /.harness-skills-stage///; drop the empty +// shell once staging is cleaned so a repository does not keep stray directories. Backups and failed +// replacements are never pruned because they hold recoverable data. +function pruneEmptyStageDirectories(staged) { + if (!staged.includes(`${sep}.harness-skills-stage${sep}`)) return; + for (let directory = dirname(staged); ; directory = dirname(directory)) { + try { + rmdirSync(directory); + } catch { + return; + } + if (basename(directory) === ".harness-skills-stage") return; + } +} + function processIsAlive(pid) { try { process.kill(pid, 0); @@ -664,7 +712,10 @@ try { const results = rollbackCommitted(); throw new Error(`${error.message}\nRollback results:\n${results.join("\n")}`, { cause: error }); } finally { - for (const staged of stagedPaths) rmSync(staged, { recursive: true, force: true }); + for (const staged of stagedPaths) { + rmSync(staged, { recursive: true, force: true }); + pruneEmptyStageDirectories(staged); + } for (const { descriptor, lockPath, owner } of locks.reverse()) { closeSync(descriptor); try { @@ -678,3 +729,6 @@ try { const skillCount = plan.filter(({ kind }) => kind === "skill").length; const artifactCount = artifactPlan.length; console.log(`Installed ${skillCount} skill copies and ${artifactCount} artifact copies; replaced ${conflicts.length}; retired ${migration.retirements.length} legacy entries.`); +if (projectDir) { + console.log(`Project install in ${projectDir}: commit the skill directories deliberately or gitignore them. A new git worktree contains only committed files.`); +} diff --git a/scripts/install.test.mjs b/scripts/install.test.mjs index da15305..cc5130a 100644 --- a/scripts/install.test.mjs +++ b/scripts/install.test.mjs @@ -7,6 +7,7 @@ import { readFileSync, readdirSync, rmSync, + lstatSync, symlinkSync, writeFileSync, } from "node:fs"; @@ -762,6 +763,141 @@ test("rejects artifact sources outside the repository", () => { } }); +function projectFixture() { + const { root, env } = fixture(); + const project = join(root, "repo with spaces"); + mkdirSync(project); + const home = join(root, "home"); + mkdirSync(home); + env.HOME = home; + env.USERPROFILE = home; + return { root, env, project, home }; +} + +test("project install writes per-harness project roots, shares one copy, and ignores user overrides", () => { + const { root, env, project, home } = projectFixture(); + try { + const result = run(["--harness", "all", "--project", project, "--skill", "show-me-your-work"], env); + assert.equal(result.status, 0, result.stderr); + const shared = join(project, ".agents", "skills", "show-me-your-work"); + const claude = join(project, ".claude", "skills", "show-me-your-work"); + assert.deepEqual(readdirSync(project).sort(), [".agents", ".claude"]); + assert.deepEqual(readdirSync(join(project, ".agents")).sort(), ["skills"]); + assert.equal(result.stdout.match(/shared show-me-your-work: .* \(codex, opencode, pi, antigravity, grok\)/)?.length, 1); + assert.match(readFileSync(join(shared, "SKILL.md"), "utf8"), /^name: /m); + const claudeFrontmatter = readFileSync(join(claude, "SKILL.md"), "utf8").split(/\r?\n---\r?\n/)[0]; + assert.doesNotMatch(claudeFrontmatter, /^name:/m); + assert.match(claudeFrontmatter, /^compatibility:/m); + assert.ok(existsSync(join(shared, ".mstack-install.json"))); + assert.equal(lstatSync(shared).isSymbolicLink(), false); + for (const variable of ["CODEX", "CLAUDE", "OPENCODE", "PI", "ANTIGRAVITY", "GROK"]) { + assert.equal(existsSync(env[`HARNESS_SKILLS_${variable}_DIR`]), false, variable); + } + assert.deepEqual(readdirSync(home), []); + assert.match(result.stdout, /commit the skill directories deliberately or gitignore them/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("project install maps a single harness to its own root", () => { + const { root, env, project } = projectFixture(); + try { + for (const [harness, parts] of Object.entries({ + codex: [".agents", "skills"], claude: [".claude", "skills"], opencode: [".agents", "skills"], + pi: [".agents", "skills"], antigravity: [".agents", "skills"], grok: [".agents", "skills"], + })) { + const result = run(["--harness", harness, "--project", project, "--skill", "bro", "--dry-run"], env); + assert.equal(result.status, 0, `${harness}: ${result.stderr}`); + assert.ok(result.stdout.includes(`${harness}: ${join(project, ...parts)}`), harness); + } + assert.deepEqual(readdirSync(project), []); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("project dry-run prints the plan and replace backups outside the skill roots", () => { + const { root, env, project } = projectFixture(); + try { + assert.equal(run(["--harness", "codex,claude", "--project", project, "--skill", "bro"], env).status, 0); + const conflict = run(["--harness", "codex,claude", "--project", project, "--skill", "bro", "--dry-run"], env); + assert.equal(conflict.status, 2); + assert.match(conflict.stdout, /conflict: bro/); + + const dry = run(["--harness", "codex,claude", "--project", project, "--skill", "bro", "--replace", "--dry-run"], env); + assert.equal(dry.status, 0, dry.stderr); + assert.ok(dry.stdout.includes(`project: ${project}`)); + assert.match(dry.stdout, /replace with backup: bro/); + assert.ok(dry.stdout.includes(join(project, ".agents", ".harness-skills-backups", "skills"))); + assert.ok(dry.stdout.includes(join(project, ".claude", ".harness-skills-backups", "skills"))); + + writeFileSync(join(project, ".agents", "skills", "bro", "marker.txt"), "old", "utf8"); + const real = run(["--harness", "codex,claude", "--project", project, "--skill", "bro", "--replace"], env); + assert.equal(real.status, 0, real.stderr); + assert.equal(existsSync(join(project, ".agents", "skills", "bro", "marker.txt")), false); + assert.equal(readFileSync(findFile(join(project, ".agents", ".harness-skills-backups"), "marker.txt"), "utf8"), "old"); + assert.equal(findFile(join(project, ".agents", "skills"), "marker.txt"), undefined); + assert.equal(findFile(join(project, ".claude", "skills"), ".harness-skills-backups"), undefined); + assert.equal(readdirSync(join(project, ".agents", "skills")).includes(".harness-skills-backups"), false); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("project install rolls back earlier targets when a later one fails", () => { + const { root, env, project } = projectFixture(); + try { + assert.equal(run(["--harness", "codex,claude", "--project", project, "--skill", "bro"], env).status, 0); + writeFileSync(join(project, ".agents", "skills", "bro", "marker.txt"), "old codex", "utf8"); + writeFileSync(join(project, ".claude", "skills", "bro", "marker.txt"), "old claude", "utf8"); + // A file where the Claude backup directory must go makes the second commit fail after Codex committed. + mkdirSync(join(project, ".claude", ".harness-skills-backups")); + writeFileSync(join(project, ".claude", ".harness-skills-backups", "skills"), "block", "utf8"); + const result = run(["--harness", "codex,claude", "--project", project, "--skill", "bro", "--replace"], env); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Rollback results:/); + assert.match(result.stderr, /restored original/); + assert.equal(readFileSync(join(project, ".agents", "skills", "bro", "marker.txt"), "utf8"), "old codex"); + assert.equal(readFileSync(join(project, ".claude", "skills", "bro", "marker.txt"), "utf8"), "old claude"); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("project install refuses unsupported combinations and unsafe directories before writing", () => { + const { root, env, project, home } = projectFixture(); + const file = join(root, "plain-file"); + const transaction = join(root, ".harness-skills-backups", "repo"); + writeFileSync(file, "x", "utf8"); + mkdirSync(transaction, { recursive: true }); + try { + const base = ["--harness", "codex", "--skill", "bro", "--dry-run"]; + for (const [extra, message] of [ + [["--project", project, "--artifact", "agents"], /--artifact is not supported with --project/], + [["--project", project, "--artifact", "all"], /--artifact is not supported with --project/], + [["--project", project, "--migrate", "--replace"], /cannot be combined with --project/], + [["--project", project, "--environment", "fleet-ssh"], /cannot be combined with --environment/], + [["--project"], /--project requires a directory/], + [["--project", "--replace"], /--project requires a directory/], + [["--project", join(root, "missing")], /existing directory/], + [["--project", file], /must name a directory/], + [["--project", home], /cannot be the home directory/], + [["--project", join(resolve("."), "scripts")], /inside the mstack package/], + [["--project", resolve(".")], /inside the mstack package/], + [["--project", transaction], /transaction storage/], + ]) { + const result = run([...base, ...extra], env); + assert.notEqual(result.status, 0, extra.join(" ")); + assert.match(result.stderr, message, extra.join(" ")); + } + assert.deepEqual(readdirSync(project), []); + assert.deepEqual(readdirSync(home), []); + } 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/skill-discovery.test.mjs b/scripts/skill-discovery.test.mjs index 80dc169..870412c 100644 --- a/scripts/skill-discovery.test.mjs +++ b/scripts/skill-discovery.test.mjs @@ -80,6 +80,26 @@ test("Grok shares the default copy while Antigravity keeps its own native root", assert.deepEqual(skillFiles(join(root, ".agents/skills")), ["meta-mode/SKILL.md"]); }); +test("project install leaves only skill roots in the repository and none under the home directory", (t) => { + const { root, env } = fixture(t); + const project = join(root, "workspace", "repo"); + mkdirSync(project, { recursive: true }); + succeeded(run(["--harness", "all", "--project", project, "--skill", "meta-mode,show-me-your-work"], env)); + assert.deepEqual(skillFiles(project), [ + ".agents/skills/meta-mode/SKILL.md", + ".agents/skills/show-me-your-work/SKILL.md", + ".claude/skills/meta-mode/SKILL.md", + ".claude/skills/show-me-your-work/SKILL.md", + ]); + assert.deepEqual(readdirSync(project).sort(), [".agents", ".claude"]); + assert.deepEqual(readdirSync(join(project, ".agents")), ["skills"]); + assert.deepEqual(readdirSync(join(project, ".claude")), ["skills"]); + assert.deepEqual(readdirSync(root), ["workspace"]); + const claude = installedText(project, ".claude/skills", "meta-mode"); + assert.doesNotMatch(claude.split(/^---\s*$/m)[1], /^name:/m); + assert.match(installedText(project, ".agents/skills", "meta-mode"), /^name: meta-mode\r?$/m); +}); + test("explicit Grok and Antigravity roots override their defaults", (t) => { const { root, env } = fixture(t); env.HARNESS_SKILLS_GROK_DIR = join(root, "custom-grok/skills"); From 3628b45940f33ae0bd8cdd5a70f761e96818e4a8 Mon Sep 17 00:00:00 2001 From: 3metaJun <251347867+3metaJun@users.noreply.github.com> Date: Tue, 6 Oct 2026 18:51:39 +0800 Subject: [PATCH 2/4] fix(install): refuse project roots overlapping user skill roots Co-Authored-By: Claude Sonnet 5.5 --- scripts/install.mjs | 15 ++++++++--- scripts/install.test.mjs | 54 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/scripts/install.mjs b/scripts/install.mjs index 8aa9100..4bcaa80 100644 --- a/scripts/install.mjs +++ b/scripts/install.mjs @@ -137,13 +137,20 @@ const discoveryRoots = [...new Set([ ...Object.values(targets), ...Object.values(userTargets), ...Object.values(nativeTargets), ...Object.values(legacyTargets), sharedRoot, externalClaudeRoot, ].map(physicalPathKey))]; if (projectDir) { - // A symlinked project directory could alias the home skill roots, which is a user-level install. - const userKeys = new Set([...Object.values(userTargets), ...Object.values(nativeTargets), ...Object.values(legacyTargets), sharedRoot, externalClaudeRoot].map(physicalPathKey)); + // Project files must never mix with user-level skills. Compare physical, case-normalized paths in both + // directions so aliases, nesting (/repo/.agents/skills) and a project that contains a user root + // are all refused, while unrelated siblings pass. + const userKeys = [...new Set([...Object.values(userTargets), ...Object.values(nativeTargets), ...Object.values(legacyTargets), sharedRoot, externalClaudeRoot].map(physicalPathKey))]; for (const harness of harnesses) { - if (userKeys.has(physicalPathKey(targets[harness]))) { - throw new Error(`--project resolves ${harness} to a user-level skill root: ${targets[harness]}`); + const targetKey = physicalPathKey(targets[harness]); + const overlapping = userKeys.find((userKey) => targetPathsOverlap(targetKey, userKey)); + if (overlapping) { + throw new Error(`--project would overlap a user-level skill root for ${harness}: ${targets[harness]} and ${overlapping}`); } } + const projectKey = physicalPathKey(projectDir); + const contained = userKeys.find((userKey) => pathIsWithin(projectKey, userKey)); + if (contained) throw new Error(`--project cannot contain a user-level skill root: ${projectDir} contains ${contained}`); } function artifactEnvironmentPath(name, harness) { diff --git a/scripts/install.test.mjs b/scripts/install.test.mjs index cc5130a..f3bda29 100644 --- a/scripts/install.test.mjs +++ b/scripts/install.test.mjs @@ -898,6 +898,60 @@ test("project install refuses unsupported combinations and unsafe directories be } }); +test("project install refuses overlap with user-level skill roots in either direction and allows siblings", () => { + const { root, env, project, home } = projectFixture(); + const nested = join(home, ".agents", "skills", "repo"); + const claudeNested = join(home, ".claude", "skills", "repo"); + const sibling = join(root, "home-sibling"); + const customRoot = env.HARNESS_SKILLS_PI_DIR; + const insideCustom = join(customRoot, "repo"); + for (const directory of [nested, claudeNested, sibling, insideCustom]) mkdirSync(directory, { recursive: true }); + try { + const base = ["--harness", "all", "--skill", "bro", "--dry-run"]; + for (const [directory, message] of [ + [nested, /would overlap a user-level skill root/], + [claudeNested, /would overlap a user-level skill root/], + [insideCustom, /would overlap a user-level skill root/], + [root, /cannot contain a user-level skill root/], + [home, /cannot be the home directory/], + ]) { + const result = run([...base, "--project", directory], env); + assert.notEqual(result.status, 0, directory); + assert.match(result.stderr, message, directory); + } + // A user root nested in the project target counts too: the custom Pi root sits under the project here. + const aliasEnv = { ...env, HARNESS_SKILLS_PI_DIR: join(project, ".agents", "skills", "pi") }; + const containing = run([...base, "--project", project], aliasEnv); + assert.notEqual(containing.status, 0); + assert.match(containing.stderr, /would overlap a user-level skill root|cannot contain a user-level skill root/); + for (const allowed of [project, sibling]) { + const result = run([...base, "--project", allowed], env); + assert.equal(result.status, 0, `${allowed}: ${result.stderr}`); + } + assert.deepEqual(readdirSync(project), []); + assert.deepEqual(readdirSync(sibling), []); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("project overlap check ignores path case on case-insensitive local filesystems", { skip: process.platform !== "win32" }, () => { + const { root, env, home } = projectFixture(); + const nested = join(home, ".agents", "skills", "repo"); + mkdirSync(nested, { recursive: true }); + try { + const shouted = join(home.toUpperCase(), ".AGENTS", "SKILLS", "REPO"); + const result = run(["--harness", "codex", "--skill", "bro", "--dry-run", "--project", shouted], env); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /would overlap a user-level skill root/); + const parent = run(["--harness", "codex", "--skill", "bro", "--dry-run", "--project", root.toUpperCase()], env); + assert.notEqual(parent.status, 0); + assert.match(parent.stderr, /cannot contain a user-level skill root/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + function findFile(root, name) { const entries = readdirSync(root, { withFileTypes: true }); for (const entry of entries) { From 813a19898e683aa2e11a4d552b475db613836589 Mon Sep 17 00:00:00 2001 From: 3metaJun <251347867+3metaJun@users.noreply.github.com> Date: Tue, 6 Oct 2026 18:56:25 +0800 Subject: [PATCH 3/4] fix(install): validate project targets by physical path Co-Authored-By: Claude Sonnet 5.5 --- README.md | 8 ++- scripts/harness-targets.mjs | 67 ++++++++++++++++++-- scripts/harness-targets.test.mjs | 23 ++++++- scripts/install.mjs | 38 ++++++----- scripts/install.test.mjs | 104 ++++++++++++++++++++++++++++++- 5 files changed, 214 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 1a8eda9..fd563de 100644 --- a/README.md +++ b/README.md @@ -78,8 +78,12 @@ ignore `HARNESS_SKILLS_*_DIR` overrides. Rules: - The directory must exist. The installer refuses your home directory (use a - user-level install), anything inside the mstack package, and any path inside - `.harness-skills-*` transaction storage. + user-level install), anything inside the mstack package, any path inside + `.harness-skills-*` transaction storage, and any directory inside a skill + discovery root such as `.agents/skills` or `.claude/skills`, because backups + there would be scanned as skills. It checks physical paths, so symlinks, + junctions and letter case cannot redirect a target, backup or stage directory + outside the project or into the package or your user-level skill roots. - Skills only. `--artifact` is refused with `--project`: agent roles, tools, the guide and session context are user-owned configuration and should not land in a repository implicitly. `--migrate` and `--environment` (including SSH) are diff --git a/scripts/harness-targets.mjs b/scripts/harness-targets.mjs index f59f8eb..09a948d 100644 --- a/scripts/harness-targets.mjs +++ b/scripts/harness-targets.mjs @@ -1,7 +1,7 @@ import { statSync } from "node:fs"; import { homedir } from "node:os"; -import { isAbsolute, join, resolve } from "node:path"; -import { pathIsWithin, physicalPathKey } from "./install-paths.mjs"; +import { dirname, isAbsolute, join, resolve } from "node:path"; +import { pathIsWithin, physicalPathKey, targetPathsOverlap } from "./install-paths.mjs"; export function configuredPath(value, fallback, label, home = homedir()) { const path = value ?? fallback; @@ -36,6 +36,59 @@ export function resolveHarnessRoots(registry, { env = process.env, home = homedi } const TRANSACTION_SEGMENT = /^\.harness-skills-/; +// Directories under which some harness scans for skills recursively. Segments arrive lower-cased on +// case-insensitive platforms, so the names here are lower-case. +const DISCOVERY_PARENTS = [".agents", ".claude", ".cursor", ".codex", ".gemini", ".agent", ".pi", ".opencode", ".grok"]; + +function enclosingDiscoveryRoot(segments) { + for (let index = 0; index < segments.length - 1; index += 1) { + if (DISCOVERY_PARENTS.includes(segments[index]) && segments[index + 1] === "skills") { + return `${segments[index]}/skills`; + } + } + return undefined; +} + +// An override that resolveHarnessRoots would reject must not block a project install, which ignores overrides. +export function withoutInvalidOverrides(registry, env, home = homedir()) { + const cleaned = { ...env }; + for (const config of Object.values(registry)) { + for (const variable of [config.directoryVariable, config.configVariable]) { + if (!variable || !cleaned[variable]) continue; + try { + configuredPath(cleaned[variable], "", variable, home); + } catch { + delete cleaned[variable]; + } + } + } + return cleaned; +} + +// Every target and its transaction storage must physically stay inside the project, away from the mstack +// package, and out of user-level skill roots. Compare physical paths so symlinks and junctions cannot +// redirect a write, and check both directions so nesting is caught either way. +export function validateProjectTargets({ projectDir, targets, userRoots, packageRoot }) { + const projectKey = physicalPathKey(projectDir); + const packageKey = packageRoot ? physicalPathKey(packageRoot) : undefined; + const userKeys = userRoots.map(physicalPathKey); + const check = (label, path) => { + const key = physicalPathKey(path); + if (!pathIsWithin(projectKey, key)) throw new Error(`--project ${label} resolves outside the project directory: ${path} -> ${key}`); + if (packageKey && targetPathsOverlap(key, packageKey)) throw new Error(`--project ${label} overlaps the mstack package: ${path}`); + const user = userKeys.find((userKey) => targetPathsOverlap(key, userKey)); + if (user) throw new Error(`--project would overlap a user-level skill root: ${label} ${path} and ${user}`); + }; + for (const [harness, target] of Object.entries(targets)) { + check(`${harness} skill root`, target); + for (const category of ["backups", "failed", "stage"]) { + // Mirrors the installer: storage sits beside the skills root, one level above each skill directory. + check(`${harness} ${category} storage`, join(dirname(target), `.harness-skills-${category}`)); + } + } + const contained = userKeys.find((userKey) => pathIsWithin(projectKey, userKey)); + if (contained) throw new Error(`--project cannot contain a user-level skill root: ${projectDir} contains ${contained}`); +} // Project roots come only from the registry's `project` path; user-scope overrides never apply. export function resolveProjectRoots(registry, projectDir) { @@ -61,10 +114,16 @@ export function validateProjectDir(value, { packageRoot, home = homedir(), cwd = throw new Error(`--project must name an existing directory: ${project}`); } if (!status.isDirectory()) throw new Error(`--project must name a directory: ${project}`); - if (project.split(/[\\/]/).some((segment) => TRANSACTION_SEGMENT.test(segment))) { + // Judge the physical, case-normalized path: a symlink or an uppercase spelling must not slip past. + const physical = physicalPathKey(project); + const segments = physical.split(/[\\/]/).filter(Boolean); + if (segments.some((segment) => TRANSACTION_SEGMENT.test(segment))) { throw new Error(`--project cannot be inside installer transaction storage: ${project}`); } - const physical = physicalPathKey(project); + const discoveryRoot = enclosingDiscoveryRoot(segments); + if (discoveryRoot) { + throw new Error(`--project cannot be inside a skill discovery root (${discoveryRoot}): ${project}`); + } if (physical === physicalPathKey(home)) { throw new Error("--project cannot be the home directory; omit --project for a user-level install"); } diff --git a/scripts/harness-targets.test.mjs b/scripts/harness-targets.test.mjs index a1af042..1f2d3a9 100644 --- a/scripts/harness-targets.test.mjs +++ b/scripts/harness-targets.test.mjs @@ -4,7 +4,7 @@ import { tmpdir } from "node:os"; import { delimiter, join, resolve } from "node:path"; import { spawnSync } from "node:child_process"; import test from "node:test"; -import { configuredPath, resolveHarnessRoots, resolveProjectRoots, validateProjectDir } from "./harness-targets.mjs"; +import { configuredPath, resolveHarnessRoots, resolveProjectRoots, validateProjectDir, withoutInvalidOverrides } from "./harness-targets.mjs"; const repository = resolve("."); const registry = JSON.parse(readFileSync(join(repository, "profiles", "harnesses.json"), "utf8")); @@ -126,6 +126,27 @@ test("project directories must exist, be directories, and stay out of home, the assert.equal(validateProjectDir(root, options), root); }); +test("invalid overrides are dropped for project installs and valid ones are kept", () => { + const cleaned = withoutInvalidOverrides(registry, { + HARNESS_SKILLS_PI_DIR: "relative", + HARNESS_SKILLS_GROK_DIR: "~/grok-explicit", + XDG_CONFIG_HOME: "relative-xdg", + UNRELATED: "relative", + }, home); + assert.deepEqual(cleaned, { HARNESS_SKILLS_GROK_DIR: "~/grok-explicit", UNRELATED: "relative" }); + assert.doesNotThrow(() => resolveHarnessRoots(registry, { home, env: cleaned })); +}); + +test("project directories inside discovery roots are refused on the physical path", (t) => { + const root = mkdtempSync(join(tmpdir(), "mstack-project-roots-")); + t.after(() => rmSync(root, { recursive: true, force: true })); + const nested = join(root, ".Agents", "Skills", "repo"); + mkdirSync(nested, { recursive: true }); + const options = { packageRoot: join(root, "package"), home: join(root, "home"), cwd: root }; + assert.throws(() => validateProjectDir(nested, options), /inside a skill discovery root/); + assert.equal(validateProjectDir(root, options), root); +}); + for (const explicit of [false, true]) { test(`smoke finds skills from a real isolated ${explicit ? "explicit" : "default"} installation`, (t) => { const root = mkdtempSync(join(tmpdir(), "mstack-harness-targets-")); diff --git a/scripts/install.mjs b/scripts/install.mjs index 4bcaa80..5f49718 100644 --- a/scripts/install.mjs +++ b/scripts/install.mjs @@ -21,7 +21,14 @@ import { basename, dirname, isAbsolute, join, resolve, sep } from "node:path"; import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; import { readEnvironment } from "./environment-lib.mjs"; -import { configuredPath, resolveHarnessRoots, resolveProjectRoots, validateProjectDir } from "./harness-targets.mjs"; +import { + configuredPath, + resolveHarnessRoots, + resolveProjectRoots, + validateProjectDir, + validateProjectTargets, + withoutInvalidOverrides, +} from "./harness-targets.mjs"; import { planSkillMigration } from "./install-migration.mjs"; import { convertAgentMarkdown } from "./agent-format.mjs"; import { @@ -130,27 +137,26 @@ if (environment.transport === "ssh") { } const environmentTargets = environment.targets; const environmentArtifacts = environment.artifacts; +// Project installs ignore user-level overrides as targets, so an invalid one must not block them; the +// valid ones still count as user roots for the safety checks below. const { skills: userTargets, native: nativeTargets, legacy: legacyTargets, sharedRoot, externalClaudeRoot } = - resolveHarnessRoots(harnessRegistry, { home: userHome, environmentTargets, environmentName }); + resolveHarnessRoots(harnessRegistry, { + home: userHome, + env: projectDir ? withoutInvalidOverrides(harnessRegistry, process.env, userHome) : process.env, + environmentTargets, + environmentName, + }); const targets = projectDir ? resolveProjectRoots(harnessRegistry, projectDir) : userTargets; const discoveryRoots = [...new Set([ ...Object.values(targets), ...Object.values(userTargets), ...Object.values(nativeTargets), ...Object.values(legacyTargets), sharedRoot, externalClaudeRoot, ].map(physicalPathKey))]; if (projectDir) { - // Project files must never mix with user-level skills. Compare physical, case-normalized paths in both - // directions so aliases, nesting (/repo/.agents/skills) and a project that contains a user root - // are all refused, while unrelated siblings pass. - const userKeys = [...new Set([...Object.values(userTargets), ...Object.values(nativeTargets), ...Object.values(legacyTargets), sharedRoot, externalClaudeRoot].map(physicalPathKey))]; - for (const harness of harnesses) { - const targetKey = physicalPathKey(targets[harness]); - const overlapping = userKeys.find((userKey) => targetPathsOverlap(targetKey, userKey)); - if (overlapping) { - throw new Error(`--project would overlap a user-level skill root for ${harness}: ${targets[harness]} and ${overlapping}`); - } - } - const projectKey = physicalPathKey(projectDir); - const contained = userKeys.find((userKey) => pathIsWithin(projectKey, userKey)); - if (contained) throw new Error(`--project cannot contain a user-level skill root: ${projectDir} contains ${contained}`); + validateProjectTargets({ + projectDir, + targets: Object.fromEntries(harnesses.map((harness) => [harness, targets[harness]])), + userRoots: [...Object.values(userTargets), ...Object.values(nativeTargets), ...Object.values(legacyTargets), sharedRoot, externalClaudeRoot], + packageRoot: repoRoot, + }); } function artifactEnvironmentPath(name, harness) { diff --git a/scripts/install.test.mjs b/scripts/install.test.mjs index f3bda29..cbd3d74 100644 --- a/scripts/install.test.mjs +++ b/scripts/install.test.mjs @@ -909,8 +909,8 @@ test("project install refuses overlap with user-level skill roots in either dire try { const base = ["--harness", "all", "--skill", "bro", "--dry-run"]; for (const [directory, message] of [ - [nested, /would overlap a user-level skill root/], - [claudeNested, /would overlap a user-level skill root/], + [nested, /inside a skill discovery root/], + [claudeNested, /inside a skill discovery root/], [insideCustom, /would overlap a user-level skill root/], [root, /cannot contain a user-level skill root/], [home, /cannot be the home directory/], @@ -943,7 +943,7 @@ test("project overlap check ignores path case on case-insensitive local filesyst const shouted = join(home.toUpperCase(), ".AGENTS", "SKILLS", "REPO"); const result = run(["--harness", "codex", "--skill", "bro", "--dry-run", "--project", shouted], env); assert.notEqual(result.status, 0); - assert.match(result.stderr, /would overlap a user-level skill root/); + assert.match(result.stderr, /inside a skill discovery root/); const parent = run(["--harness", "codex", "--skill", "bro", "--dry-run", "--project", root.toUpperCase()], env); assert.notEqual(parent.status, 0); assert.match(parent.stderr, /cannot contain a user-level skill root/); @@ -952,6 +952,104 @@ test("project overlap check ignores path case on case-insensitive local filesyst } }); +function linkDirectory(target, link) { + try { + symlinkSync(target, link, process.platform === "win32" ? "junction" : "dir"); + return true; + } catch { + return false; + } +} + +test("project install judges targets and transaction storage by their physical paths", (t) => { + const { root, env, project } = projectFixture(); + const outside = join(root, "outside"); + mkdirSync(outside); + try { + const base = ["--harness", "codex,claude", "--skill", "bro", "--replace", "--dry-run", "--project", project]; + // .agents redirected to the mstack package would make the codex target /skills. Dry run only. + if (!linkDirectory(resolve("."), join(project, ".agents"))) { + t.skip("the OS refused to create a directory link"); + return; + } + let result = run(base, env); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /resolves outside the project directory/); + rmSync(join(project, ".agents"), { recursive: true, force: true }); + assert.ok(existsSync(resolve("skills")), "removing the link must not remove package skills"); + + // A real .agents whose transaction storage is a link to elsewhere. + mkdirSync(join(project, ".agents")); + assert.ok(linkDirectory(outside, join(project, ".agents", ".harness-skills-backups"))); + result = run(base, env); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /backups storage resolves outside the project directory/); + assert.deepEqual(readdirSync(outside), []); + + // A project directory that is a link into installer transaction storage. + const storage = join(root, ".harness-skills-backups", "repo"); + mkdirSync(storage, { recursive: true }); + const alias = join(root, "innocent-name"); + assert.ok(linkDirectory(storage, alias)); + result = run(["--harness", "codex", "--skill", "bro", "--dry-run", "--project", alias], env); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /transaction storage/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("project install rejects an uppercase spelling of transaction storage on case-insensitive Windows", { skip: process.platform !== "win32" }, () => { + const { root, env } = projectFixture(); + mkdirSync(join(root, ".harness-skills-backups", "repo"), { recursive: true }); + try { + const result = run(["--harness", "codex", "--skill", "bro", "--dry-run", "--project", join(root, ".HARNESS-SKILLS-BACKUPS", "REPO")], env); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /transaction storage/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("project install rejects directories inside any skill discovery root", () => { + const { root, env, project } = projectFixture(); + try { + for (const parts of [ + [".agents", "skills", "nested"], [".claude", "skills", "nested"], [".cursor", "skills", "nested"], + [".codex", "skills", "nested"], [".gemini", "skills", "nested"], [".agent", "skills", "nested"], + [".pi", "skills", "nested"], [".opencode", "skills", "nested"], [".agents", "skills", "deep", "nested"], + ]) { + const nested = join(project, ...parts); + mkdirSync(nested, { recursive: true }); + const result = run(["--harness", "codex", "--skill", "bro", "--dry-run", "--project", nested], env); + assert.notEqual(result.status, 0, parts.join("/")); + assert.match(result.stderr, /inside a skill discovery root/, parts.join("/")); + } + // A repository merely named like a root parent, or holding a sibling named skills, is fine. + const fine = join(project, ".agents", "tools", "skills-like"); + mkdirSync(fine, { recursive: true }); + assert.equal(run(["--harness", "codex", "--skill", "bro", "--dry-run", "--project", fine], env).status, 0); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("project install ignores invalid user-level overrides it would not use", () => { + const { root, env, project } = projectFixture(); + try { + const bad = { ...env, HARNESS_SKILLS_PI_DIR: "relative", HARNESS_SKILLS_GROK_DIR: "also/relative", CLAUDE_CONFIG_DIR: "relative-config", XDG_CONFIG_HOME: "relative-xdg" }; + const result = run(["--harness", "codex", "--skill", "bro", "--project", project], bad); + assert.equal(result.status, 0, result.stderr); + assert.ok(existsSync(join(project, ".agents", "skills", "bro", "SKILL.md"))); + // The same override still fails a user-level install. + const userLevel = run(["--harness", "pi", "--skill", "bro", "--dry-run"], bad); + assert.notEqual(userLevel.status, 0); + assert.match(userLevel.stderr, /must be an absolute path/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + function findFile(root, name) { const entries = readdirSync(root, { withFileTypes: true }); for (const entry of entries) { From 61e9979a512c2e977721f15bfbc9f8fffa16a453 Mon Sep 17 00:00:00 2001 From: 3metaJun <251347867+3metaJun@users.noreply.github.com> Date: Tue, 6 Oct 2026 19:00:30 +0800 Subject: [PATCH 4/4] fix(install): validate concrete project transaction paths Co-Authored-By: Claude Sonnet 5.5 --- scripts/harness-targets.mjs | 3 +++ scripts/harness-targets.test.mjs | 25 +++++++++++++++-- scripts/install.mjs | 18 +++++++------ scripts/install.test.mjs | 46 ++++++++++++++++++++++++++++++-- 4 files changed, 80 insertions(+), 12 deletions(-) diff --git a/scripts/harness-targets.mjs b/scripts/harness-targets.mjs index 09a948d..9f7c5d3 100644 --- a/scripts/harness-targets.mjs +++ b/scripts/harness-targets.mjs @@ -68,6 +68,8 @@ export function withoutInvalidOverrides(registry, env, home = homedir()) { // Every target and its transaction storage must physically stay inside the project, away from the mstack // package, and out of user-level skill roots. Compare physical paths so symlinks and junctions cannot // redirect a write, and check both directions so nesting is caught either way. +// Returns `assertProjectPath(label, path)`, the single boundary check. The installer calls it again on +// every concrete transaction path (stage, backup, failed) because a link can sit below the storage roots. export function validateProjectTargets({ projectDir, targets, userRoots, packageRoot }) { const projectKey = physicalPathKey(projectDir); const packageKey = packageRoot ? physicalPathKey(packageRoot) : undefined; @@ -88,6 +90,7 @@ export function validateProjectTargets({ projectDir, targets, userRoots, package } const contained = userKeys.find((userKey) => pathIsWithin(projectKey, userKey)); if (contained) throw new Error(`--project cannot contain a user-level skill root: ${projectDir} contains ${contained}`); + return check; } // Project roots come only from the registry's `project` path; user-scope overrides never apply. diff --git a/scripts/harness-targets.test.mjs b/scripts/harness-targets.test.mjs index 1f2d3a9..44f104f 100644 --- a/scripts/harness-targets.test.mjs +++ b/scripts/harness-targets.test.mjs @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { delimiter, join, resolve } from "node:path"; import { spawnSync } from "node:child_process"; @@ -140,13 +140,34 @@ test("invalid overrides are dropped for project installs and valid ones are kept test("project directories inside discovery roots are refused on the physical path", (t) => { const root = mkdtempSync(join(tmpdir(), "mstack-project-roots-")); t.after(() => rmSync(root, { recursive: true, force: true })); - const nested = join(root, ".Agents", "Skills", "repo"); + const nested = join(root, ".agents", "skills", "repo"); mkdirSync(nested, { recursive: true }); const options = { packageRoot: join(root, "package"), home: join(root, "home"), cwd: root }; assert.throws(() => validateProjectDir(nested, options), /inside a skill discovery root/); assert.equal(validateProjectDir(root, options), root); }); +// Case folding only applies where the platform treats paths as case-insensitive, so probe the real +// temporary filesystem instead of assuming from the platform name. +function caseInsensitiveTemporaryFilesystem(directory) { + const probe = join(directory, "CaseProbe"); + writeFileSync(probe, "x", "utf8"); + return existsSync(join(directory, "caseprobe")); +} + +test("mixed-case discovery roots are refused where the filesystem folds case", (t) => { + const root = mkdtempSync(join(tmpdir(), "mstack-project-case-")); + t.after(() => rmSync(root, { recursive: true, force: true })); + if (!caseInsensitiveTemporaryFilesystem(root) || !["win32", "darwin"].includes(process.platform)) { + t.skip("the temporary filesystem or platform treats path case as significant"); + return; + } + const nested = join(root, ".Agents", "Skills", "repo"); + mkdirSync(nested, { recursive: true }); + const options = { packageRoot: join(root, "package"), home: join(root, "home"), cwd: root }; + assert.throws(() => validateProjectDir(nested, options), /inside a skill discovery root/); +}); + for (const explicit of [false, true]) { test(`smoke finds skills from a real isolated ${explicit ? "explicit" : "default"} installation`, (t) => { const root = mkdtempSync(join(tmpdir(), "mstack-harness-targets-")); diff --git a/scripts/install.mjs b/scripts/install.mjs index 5f49718..92912b8 100644 --- a/scripts/install.mjs +++ b/scripts/install.mjs @@ -150,14 +150,12 @@ const targets = projectDir ? resolveProjectRoots(harnessRegistry, projectDir) : const discoveryRoots = [...new Set([ ...Object.values(targets), ...Object.values(userTargets), ...Object.values(nativeTargets), ...Object.values(legacyTargets), sharedRoot, externalClaudeRoot, ].map(physicalPathKey))]; -if (projectDir) { - validateProjectTargets({ - projectDir, - targets: Object.fromEntries(harnesses.map((harness) => [harness, targets[harness]])), - userRoots: [...Object.values(userTargets), ...Object.values(nativeTargets), ...Object.values(legacyTargets), sharedRoot, externalClaudeRoot], - packageRoot: repoRoot, - }); -} +const assertProjectPath = projectDir ? validateProjectTargets({ + projectDir, + targets: Object.fromEntries(harnesses.map((harness) => [harness, targets[harness]])), + userRoots: [...Object.values(userTargets), ...Object.values(nativeTargets), ...Object.values(legacyTargets), sharedRoot, externalClaudeRoot], + packageRoot: repoRoot, +}) : undefined; function artifactEnvironmentPath(name, harness) { const byArtifact = environmentArtifacts[name]; @@ -534,6 +532,8 @@ function transactionPath(item, category) { throw new Error(`Transaction storage would be discoverable as skills: ${path}; choose non-nested skill targets`); } } + // A link below the storage root (for example .harness-skills-stage/skills) can still point elsewhere. + assertProjectPath?.(`${category} path`, path); return path; } @@ -684,6 +684,7 @@ try { for (const [index, item] of plan.entries()) { if (item.kind === "retired") continue; + assertProjectPath?.("skill root", dirname(item.target)); mkdirSync(dirname(item.target), { recursive: true }); const staged = item.kind === "artifact" ? stagingPath(item.target, transactionId, index) : transactionPath(item, "stage"); mkdirSync(dirname(staged), { recursive: true }); @@ -702,6 +703,7 @@ try { for (const item of plan) { const { harness, target } = item; + assertProjectPath?.("skill root", dirname(target)); if (existsSync(target) && !replace) { throw new Error(`Refusing to replace ${target} without --replace`); } diff --git a/scripts/install.test.mjs b/scripts/install.test.mjs index cbd3d74..c5c6ca4 100644 --- a/scripts/install.test.mjs +++ b/scripts/install.test.mjs @@ -935,8 +935,21 @@ test("project install refuses overlap with user-level skill roots in either dire } }); -test("project overlap check ignores path case on case-insensitive local filesystems", { skip: process.platform !== "win32" }, () => { +// The installer folds case by platform (win32, darwin); probe the real temporary filesystem as well. +function foldsPathCase(directory) { + if (!["win32", "darwin"].includes(process.platform)) return false; + const probe = join(directory, "CaseProbe"); + writeFileSync(probe, "x", "utf8"); + return existsSync(join(directory, "caseprobe")); +} + +test("project overlap check ignores path case on case-insensitive local filesystems", (t) => { const { root, env, home } = projectFixture(); + if (!foldsPathCase(root)) { + rmSync(root, { recursive: true, force: true }); + t.skip("path case is significant on this platform or temporary filesystem"); + return; + } const nested = join(home, ".agents", "skills", "repo"); mkdirSync(nested, { recursive: true }); try { @@ -999,8 +1012,37 @@ test("project install judges targets and transaction storage by their physical p } }); -test("project install rejects an uppercase spelling of transaction storage on case-insensitive Windows", { skip: process.platform !== "win32" }, () => { +test("project install validates the concrete transaction paths below a link inside storage", (t) => { + const { root, env, project } = projectFixture(); + const outside = join(root, "outside"); + mkdirSync(outside); + try { + for (const category of ["stage", "backups", "failed"]) { + rmSync(join(project, ".agents"), { recursive: true, force: true }); + mkdirSync(join(project, ".agents", `.harness-skills-${category}`), { recursive: true }); + // Storage root is real; the per-skills-directory level below it links to an external directory. + if (!linkDirectory(outside, join(project, ".agents", `.harness-skills-${category}`, "skills"))) { + t.skip("the OS refused to create a directory link"); + return; + } + const result = run(["--harness", "codex", "--skill", "bro", "--replace", "--project", project], env); + assert.notEqual(result.status, 0, category); + assert.match(result.stderr, new RegExp(`${category} path resolves outside the project directory`), category); + assert.deepEqual(readdirSync(outside), [], category); + assert.equal(existsSync(join(project, ".agents", "skills")), false, category); + } + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("project install rejects an uppercase spelling of transaction storage on case-insensitive filesystems", (t) => { const { root, env } = projectFixture(); + if (!foldsPathCase(root)) { + rmSync(root, { recursive: true, force: true }); + t.skip("path case is significant on this platform or temporary filesystem"); + return; + } mkdirSync(join(root, ".harness-skills-backups", "repo"), { recursive: true }); try { const result = run(["--harness", "codex", "--skill", "bro", "--dry-run", "--project", join(root, ".HARNESS-SKILLS-BACKUPS", "REPO")], env);