From af57998795827830bda4f98845d1ee3401a27ce3 Mon Sep 17 00:00:00 2001 From: 3metaJun <251347867+3metaJun@users.noreply.github.com> Date: Fri, 11 Sep 2026 15:54:54 +0800 Subject: [PATCH 1/2] fix: adapt agent artifacts for Codex --- .codex-plugin/plugin.json | 2 +- README.md | 2 +- docs/harness-adapters.md | 4 +- package.json | 2 +- profiles/artifacts.json | 7 +++ profiles/environments.example.json | 2 +- scripts/check-package.mjs | 13 +++++ scripts/environment.test.mjs | 28 ++++++++++- scripts/install-paths.mjs | 17 +++---- scripts/install.mjs | 50 +++++++++++++++++-- scripts/install.test.mjs | 11 ++-- scripts/remote-install.mjs | 17 +++++-- scripts/validate.mjs | 19 +++++++ scripts/version-integrity.test.mjs | 13 +++++ skills/meta-mode/playbooks/opening-a-pr.md | 2 +- .../reflect/references/divergent-reviewer.md | 2 +- .../reflect/references/judgment-reviewer.md | 2 +- skills/reflect/references/tooling-reviewer.md | 2 +- 18 files changed, 167 insertions(+), 28 deletions(-) create mode 100644 scripts/version-integrity.test.mjs diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index 4052a87..72dde90 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "mstack", - "version": "0.2.0", + "version": "0.2.1", "description": "Portable engineering skills for Codex, Claude Code, OpenCode, and pi.", "author": { "name": "3metaJun" diff --git a/README.md b/README.md index 8069250..23d0297 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ The available installable artifacts are: | Artifact | Default destination (beside the harness `skills/` directory) | | --- | --- | -| `agents` | `agents/` | +| `agents` | `agents/` (Codex: `$CODEX_HOME/agents/` as `.toml`) | | `meta-mode-tools` | `tools/meta-mode/` | | `guide` | `docs/guide/` | diff --git a/docs/harness-adapters.md b/docs/harness-adapters.md index db76b50..e5dfd33 100644 --- a/docs/harness-adapters.md +++ b/docs/harness-adapters.md @@ -28,7 +28,9 @@ their official references support it. ## Delegation and session records Codex stores agent configuration in `.codex/agents/*.toml` and -`~/.codex/agents/*.toml`. Claude Code stores subagent definitions in +`~/.codex/agents/*.toml`. The installer converts the portable Markdown agent +artifacts to Codex TOML with `name`, `description`, and +`developer_instructions`. Claude Code stores subagent definitions in `.claude/agents/`. OpenCode stores agent definitions in `.opencode/agents/` or `~/.config/opencode/agents/`. pi does not require a separate agent file for skill use; it loads skills through discovery, the `--skill` flag, or the diff --git a/package.json b/package.json index 0d7a89f..00a1e7e 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "install-skills": "node scripts/install.mjs", "optimize-context": "node scripts/optimize-context.mjs", "reconcile-context": "node scripts/reconcile-context.mjs", - "test": "node scripts/validate.mjs && node --test scripts/install.test.mjs scripts/context.test.mjs scripts/audit-context.test.mjs scripts/worktree-audit.test.mjs scripts/sync-upstream.test.mjs scripts/runtime.test.mjs scripts/environment.test.mjs scripts/skill-integrity.test.mjs" + "test": "node scripts/validate.mjs && node --test scripts/install.test.mjs scripts/context.test.mjs scripts/audit-context.test.mjs scripts/worktree-audit.test.mjs scripts/sync-upstream.test.mjs scripts/runtime.test.mjs scripts/environment.test.mjs scripts/skill-integrity.test.mjs scripts/version-integrity.test.mjs" }, "bin": { "mstack": "scripts/install.mjs" diff --git a/profiles/artifacts.json b/profiles/artifacts.json index 283fd6e..f8b3fb8 100644 --- a/profiles/artifacts.json +++ b/profiles/artifacts.json @@ -3,6 +3,13 @@ "agents": { "source": "agents", "path": ["agents"], + "harnesses": { + "codex": { + "path": ["agents"], + "base": "codex-home", + "format": "codex-toml" + } + }, "description": "Portable agent role definitions for harnesses that support agent files." }, "meta-mode-tools": { diff --git a/profiles/environments.example.json b/profiles/environments.example.json index f00a5da..2d0e036 100644 --- a/profiles/environments.example.json +++ b/profiles/environments.example.json @@ -35,7 +35,7 @@ }, "artifacts": { "agents": { - "codex": "/home/dev/.agents/agents", + "codex": "/home/dev/.codex/agents", "claude": "/home/dev/.claude/agents", "opencode": "/home/dev/.config/opencode/agents", "pi": "/home/dev/.pi/agent/agents" diff --git a/scripts/check-package.mjs b/scripts/check-package.mjs index 87559b2..8384e52 100644 --- a/scripts/check-package.mjs +++ b/scripts/check-package.mjs @@ -1,5 +1,18 @@ +import { readFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; import { spawnSync } from "node:child_process"; +const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const packageManifest = JSON.parse(readFileSync(join(repoRoot, "package.json"), "utf8")); +const pluginManifest = JSON.parse(readFileSync(join(repoRoot, ".codex-plugin", "plugin.json"), "utf8")); +if (packageManifest.version !== pluginManifest.version) { + throw new Error( + `Package and Codex plugin versions must match (package.json=${packageManifest.version}, ` + + `.codex-plugin/plugin.json=${pluginManifest.version}).`, + ); +} + const npmArgs = ["pack", "--dry-run", "--json"]; const npmExecutable = process.env.npm_execpath ? process.execPath : "npm"; const result = spawnSync( diff --git a/scripts/environment.test.mjs b/scripts/environment.test.mjs index 296069b..b7f5028 100644 --- a/scripts/environment.test.mjs +++ b/scripts/environment.test.mjs @@ -165,6 +165,32 @@ test("installer delegates an SSH environment to a no-connect dry run", () => { } }); +test("remote Codex agents use the official user agent directory", () => { + const root = mkdtempSync(join(tmpdir(), "mstack-remote-codex-agents-test-")); + const configPath = join(root, "environments.json"); + writeFileSync(configPath, JSON.stringify({ + fleet: { + transport: "ssh", + host: "dev@tailnet-host", + shell: "posix", + targets: { codex: "/home/dev/.agents/skills" }, + }, + }), "utf8"); + try { + const result = spawnSync(process.execPath, [remoteInstaller, + "--harness", "codex", + "--environment", "fleet", + "--no-skills", + "--artifact", "agents", + "--dry-run", + ], { cwd: resolve("."), env: { ...process.env, MSTACK_ENVIRONMENTS_FILE: configPath }, encoding: "utf8" }); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /agents -> \/home\/dev\/\.codex\/agents/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + test("SSH environment transport overrides are validated before spawning", () => { const root = mkdtempSync(join(tmpdir(), "mstack-remote-config-test-")); const configPath = join(root, "environments.json"); @@ -250,7 +276,7 @@ test("remote staging ignores inherited local artifact target overrides", () => { writeFileSync(join(localArtifact, "marker.txt"), "keep local", "utf8"); writeFileSync(helperPath, [ 'const [mode, ...args] = process.argv.slice(2);', - 'if (mode === "ssh" && args.at(-1)?.includes("mktemp -d")) console.log("/srv/remote/.mstack-stage.fake");', + 'if (mode === "ssh" && args.at(-1)?.includes("mktemp -d")) console.log("/srv/remote/.codex/.mstack-stage.fake");', ].join("\n"), "utf8"); writeFileSync(configPath, JSON.stringify({ fleet: { diff --git a/scripts/install-paths.mjs b/scripts/install-paths.mjs index af6a670..1164040 100644 --- a/scripts/install-paths.mjs +++ b/scripts/install-paths.mjs @@ -10,20 +10,19 @@ export function pathIsWithin(root, candidate, platform = process.platform) { return difference === "" || (!isAbsolute(difference) && difference !== ".." && !difference.startsWith(`..${sep}`)); } -export function artifactPathParts(name, definition) { - const parts = definition?.path; - if ( - !Array.isArray(parts) || - parts.length === 0 || - parts.some((part) => +export function artifactPathParts(name, definition, harness) { + const baseParts = definition?.path; + const parts = definition?.harnesses?.[harness]?.path ?? baseParts; + for (const candidate of [baseParts, parts]) { + if (!Array.isArray(candidate) || candidate.length === 0 || candidate.some((part) => typeof part !== "string" || part.trim().length === 0 || part === "." || part === ".." || part.includes("/") || - part.includes("\\")) - ) { - throw new Error(`Artifact ${name} must define a safe non-empty path array`); + part.includes("\\"))) { + throw new Error(`Artifact ${name} must define a safe non-empty path array`); + } } return parts; } diff --git a/scripts/install.mjs b/scripts/install.mjs index 110363a..f98c253 100644 --- a/scripts/install.mjs +++ b/scripts/install.mjs @@ -93,6 +93,14 @@ function pathFromParts(parts) { return parts.reduce((current, part) => join(current, part), userHome); } +function artifactBase(name, harness) { + const base = artifactRegistry[name]?.harnesses?.[harness]?.base; + if (base === "codex-home") { + return configuredPath(process.env.CODEX_HOME, join(userHome, ".codex"), "CODEX_HOME"); + } + return dirname(targets[harness]); +} + const environment = readEnvironment(environmentName, harnesses); if (environment.transport === "ssh") { const remote = spawnSync(process.execPath, [join(repoRoot, "scripts", "remote-install.mjs"), ...args], { @@ -156,8 +164,8 @@ function artifactTarget(name, harness) { const variable = artifactVariable(name, harness); const override = environmentPath ?? process.env[variable]; if (override) return configuredPath(override, "", environmentPath ? `${environmentName}.artifacts.${name}.${harness}` : variable); - const root = dirname(targets[harness]); - const target = resolve(root, ...artifactPathParts(name, definition)); + const root = artifactBase(name, harness); + const target = resolve(root, ...artifactPathParts(name, definition, harness)); if (!pathIsWithin(root, target)) { throw new Error(`Artifact ${name} target must stay within the harness configuration directory`); } @@ -241,6 +249,39 @@ function copyDirectoryContents(source, target) { } } +function readAgentFrontmatter(content, path) { + const match = content.match(/^---\r?\n([\s\S]*?)\r?\n---\r?\n/); + if (!match) throw new Error(`${path} has no frontmatter`); + const field = (name) => { + const line = match[1].match(new RegExp(`^${name}:\\s*(.+)$`, "m")); + if (!line?.[1]) throw new Error(`${path} has no ${name}`); + const value = line[1].trim(); + try { return JSON.parse(value); } catch { return value; } + }; + return { name: field("name"), description: field("description"), body: content.slice(match[0].length) }; +} + +function applyArtifactAdapter(staged, item) { + const format = artifactRegistry[item.name]?.harnesses?.[item.harness]?.format; + if (format !== "codex-toml") return; + for (const entry of readdirSync(staged, { withFileTypes: true })) { + if (!entry.isFile() || !entry.name.endsWith(".md")) continue; + const sourcePath = join(staged, entry.name); + const agent = readAgentFrontmatter(readFileSync(sourcePath, "utf8"), sourcePath); + if (typeof agent.name !== "string" || typeof agent.description !== "string" || typeof agent.body !== "string" || !agent.body.trim()) { + throw new Error(`${sourcePath} is not a valid Codex agent definition`); + } + const targetPath = join(staged, `${basename(entry.name, ".md")}.toml`); + writeFileSync(targetPath, [ + `name = ${JSON.stringify(agent.name)}`, + `description = ${JSON.stringify(agent.description)}`, + `developer_instructions = ${JSON.stringify(agent.body)}`, + "", + ].join("\n"), "utf8"); + unlinkSync(sourcePath); + } +} + function validateSourceTree(source) { const pending = [source]; while (pending.length) { @@ -325,7 +366,9 @@ if (unsupportedArtifacts.length) { .join("\n"); throw new Error(`Unsupported artifact selection:\n${details}`); } -for (const name of requestedArtifacts) artifactPathParts(name, artifactRegistry[name]); +for (const name of requestedArtifacts) { + for (const harness of harnesses) artifactPathParts(name, artifactRegistry[name], harness); +} const artifactSources = Object.fromEntries( requestedArtifacts.map((name) => [name, resolveArtifactSource(name)]), ); @@ -559,6 +602,7 @@ try { const staged = stagingPath(item.target, transactionId, index); stagedPaths.push(staged); copyDirectoryContents(item.source, staged); + if (item.kind === "artifact") applyArtifactAdapter(staged, item); if (item.kind === "skill") { applyAdapter(staged, item.harness, item.skill); validateAdaptedSkill(staged, item.skill); diff --git a/scripts/install.test.mjs b/scripts/install.test.mjs index e5cb10d..81e6c2b 100644 --- a/scripts/install.test.mjs +++ b/scripts/install.test.mjs @@ -25,6 +25,7 @@ function fixture() { env: { ...process.env, HARNESS_SKILLS_CODEX_DIR: join(root, "codex skills"), + CODEX_HOME: join(root, "codex home"), HARNESS_SKILLS_CLAUDE_DIR: join(root, "claude skills"), HARNESS_SKILLS_OPENCODE_DIR: join(root, "opencode skills"), HARNESS_SKILLS_PI_DIR: join(root, "pi skills"), @@ -354,9 +355,12 @@ test("installs selected optional artifacts without skills", () => { env, ); assert.equal(result.status, 0, result.stderr); + const codexAgents = join(root, "codex home", "agents"); const configRoot = join(root, "codex skills", ".."); - assert.equal(existsSync(join(configRoot, "agents", "meta-agent.md")), true); - assert.equal(existsSync(join(configRoot, "agents", "comment-reviewer.md")), true); + assert.equal(existsSync(join(codexAgents, "meta-agent.toml")), true); + assert.equal(existsSync(join(codexAgents, "comment-reviewer.toml")), true); + assert.equal(existsSync(join(codexAgents, "meta-agent.md")), false); + assert.match(readFileSync(join(codexAgents, "meta-agent.toml"), "utf8"), /developer_instructions = /); assert.equal(existsSync(join(configRoot, "tools", "meta-mode", "package.json")), true); assert.equal(existsSync(join(configRoot, "tools", "meta-mode", "node_modules")), false); assert.equal(existsSync(env.HARNESS_SKILLS_CODEX_DIR), false); @@ -462,7 +466,8 @@ test("deduplicates a shared artifact target across harnesses", () => { env, ); assert.equal(result.status, 0, result.stderr); - assert.equal(existsSync(join(sharedAgents, "meta-agent.md")), true); + assert.equal(existsSync(join(sharedAgents, "meta-agent.toml")), true); + assert.equal(existsSync(join(sharedAgents, "meta-agent.md")), false); assert.match(result.stdout, /Installed 0 skill copies and 1 artifact copies/); } finally { rmSync(root, { recursive: true, force: true }); diff --git a/scripts/remote-install.mjs b/scripts/remote-install.mjs index bdbae7c..ec58da7 100644 --- a/scripts/remote-install.mjs +++ b/scripts/remote-install.mjs @@ -81,7 +81,9 @@ if (unknownArtifacts.length) throw new Error(`Unknown artifact: ${unknownArtifac const unsupported = artifacts.filter((name) => artifactRegistry[name].installable === false); if (unsupported.length) throw new Error(`Unsupported artifact selection: ${unsupported.join(", ")}`); const artifactPaths = Object.fromEntries( - artifacts.map((name) => [name, artifactPathParts(name, artifactRegistry[name])]), + artifacts.map((name) => [name, Object.fromEntries( + harnesses.map((harness) => [harness, artifactPathParts(name, artifactRegistry[name], harness)]), + )]), ); function artifactEnvironmentPath(name, harness) { @@ -95,7 +97,10 @@ function artifactEnvironmentPath(name, harness) { function remoteArtifactTarget(name, harness) { const custom = artifactEnvironmentPath(name, harness); if (custom) return custom; - return posix.join(posix.dirname(environment.targets[harness]), ...artifactPaths[name]); + const skillParent = posix.dirname(environment.targets[harness]); + const codexHome = artifactRegistry[name].harnesses?.[harness]?.base === "codex-home"; + const base = codexHome && posix.basename(skillParent) === ".agents" ? posix.dirname(skillParent) : skillParent; + return posix.join(base, ...(codexHome ? [".codex"] : []), ...artifactPaths[name][harness]); } const requestedItems = harnesses.flatMap((harness) => [ @@ -178,6 +183,12 @@ for (const name of Object.keys(childEnv)) { if (/^MSTACK_ARTIFACT_.*_DIR$/i.test(name)) delete childEnv[name]; } for (const harness of harnesses) childEnv[harnessRegistry[harness].directoryVariable] = join(localRoot, harness, "skills"); +for (const harness of harnesses) { + for (const name of artifacts) { + const variable = `MSTACK_ARTIFACT_${name.replaceAll(/[^A-Za-z0-9]+/g, "_").toUpperCase()}_${harness.toUpperCase()}_DIR`; + childEnv[variable] = join(localRoot, harness, ...artifactPaths[name][harness]); + } +} const localInstallerArgs = [ join(repoRoot, "scripts", "install.mjs"), "--harness", harnesses.join(","), @@ -213,7 +224,7 @@ try { for (const item of items) { const localSource = item.kind === "skill" ? join(localRoot, item.harness, "skills", item.name) - : join(localRoot, item.harness, ...artifactPaths[item.name]); + : join(localRoot, item.harness, ...artifactPaths[item.name][item.harness]); if (!existsSync(localSource)) throw new Error(`Staged source is missing: ${localSource}`); const parent = posix.dirname(item.target); const stageOutput = remoteScript(`set -eu; mkdir -p ${quotePosix(parent)}; mktemp -d ${quotePosix(posix.join(parent, ".mstack-stage.XXXXXX"))}`); diff --git a/scripts/validate.mjs b/scripts/validate.mjs index 4332a94..075b899 100644 --- a/scripts/validate.mjs +++ b/scripts/validate.mjs @@ -31,6 +31,25 @@ try { if (!Array.isArray(artifact.path) || artifact.path.length === 0 || artifact.path.some((part) => typeof part !== "string" || !part || part === "." || part === ".." || part.includes("/") || part.includes("\\"))) { errors.push(`artifact ${name} must define a safe non-empty path array`); } + if (artifact.harnesses !== undefined && (!artifact.harnesses || typeof artifact.harnesses !== "object" || Array.isArray(artifact.harnesses))) { + errors.push(`artifact ${name} harnesses must be an object`); + } else { + for (const [harness, override] of Object.entries(artifact.harnesses ?? {})) { + if (!override || typeof override !== "object" || Array.isArray(override)) { + errors.push(`artifact ${name} harness ${harness} must be an object`); + continue; + } + if (override.path !== undefined && (!Array.isArray(override.path) || override.path.length === 0 || override.path.some((part) => typeof part !== "string" || !part || part === "." || part === ".." || part.includes("/") || part.includes("\\")))) { + errors.push(`artifact ${name} harness ${harness} must define a safe non-empty path array`); + } + if (override.base !== undefined && !["harness", "codex-home"].includes(override.base)) { + errors.push(`artifact ${name} harness ${harness} has an unsupported base`); + } + if (override.format !== undefined && typeof override.format !== "string") { + errors.push(`artifact ${name} harness ${harness} format must be a string`); + } + } + } } else if (typeof artifact.reason !== "string" || !artifact.reason) { errors.push(`unsupported artifact ${name} must explain its reason`); } diff --git a/scripts/version-integrity.test.mjs b/scripts/version-integrity.test.mjs new file mode 100644 index 0000000..17965b0 --- /dev/null +++ b/scripts/version-integrity.test.mjs @@ -0,0 +1,13 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import test from "node:test"; + +const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); + +test("package and Codex plugin versions stay synchronized", () => { + const packageManifest = JSON.parse(readFileSync(join(repoRoot, "package.json"), "utf8")); + const pluginManifest = JSON.parse(readFileSync(join(repoRoot, ".codex-plugin", "plugin.json"), "utf8")); + assert.equal(pluginManifest.version, packageManifest.version); +}); diff --git a/skills/meta-mode/playbooks/opening-a-pr.md b/skills/meta-mode/playbooks/opening-a-pr.md index 4a1b6ff..db0873a 100644 --- a/skills/meta-mode/playbooks/opening-a-pr.md +++ b/skills/meta-mode/playbooks/opening-a-pr.md @@ -2,7 +2,7 @@ Invoked at the end of every other playbook. -**Worktree.** Work from a git worktree off main. Subagents inherit it. Multiple `Task` calls on the same branch each get their own worktree, or `git fetch && git reset --hard origin/` between them. Dirty branch with unrelated work: patch out, fresh worktree, apply. Snarled worktree: reset from main, redo minimally. +**Worktree.** Work from a git worktree off main. Subagents inherit it. Multiple delegation calls on the same branch each get their own worktree, or `git fetch && git reset --hard origin/` between them. Dirty branch with unrelated work: patch out, fresh worktree, apply. Snarled worktree: reset from main, redo minimally. **Commits.** Commit liberally. Rebase into small, ordered commits before opening PRs. Each commit is a future PR: landable, ordered to tell the story. Amend when the fix belongs in a just-made commit. New commit when separable. diff --git a/skills/reflect/references/divergent-reviewer.md b/skills/reflect/references/divergent-reviewer.md index a71535b..bc57be9 100644 --- a/skills/reflect/references/divergent-reviewer.md +++ b/skills/reflect/references/divergent-reviewer.md @@ -21,7 +21,7 @@ Scan for: Findings must point to skills, tools, or MCPs invoked in this transcript. Speculative routings to skills the parent never opened do not count. To check whether a skill was used, scan the transcript for: - `Read` tool calls against any `SKILL.md` file in the active Harness's project, user, or plugin skill roots. -- `Task` prompts that name a skill path +- Delegation prompts that name a skill path - Tool calls (Shell, Grep, MCP, etc.) that match a skill's documented commands Two valid finding shapes: diff --git a/skills/reflect/references/judgment-reviewer.md b/skills/reflect/references/judgment-reviewer.md index 2d7fc10..ab6ac68 100644 --- a/skills/reflect/references/judgment-reviewer.md +++ b/skills/reflect/references/judgment-reviewer.md @@ -20,7 +20,7 @@ Scan for: Findings must point to skills, tools, or MCPs invoked in this transcript. Speculative routings to skills the parent never opened do not count. To check whether a skill was used, scan the transcript for: - `Read` tool calls against any `SKILL.md` file in the active Harness's project, user, or plugin skill roots. -- `Task` prompts that name a skill path +- Delegation prompts that name a skill path - Tool calls (Shell, Grep, MCP, etc.) that match a skill's documented commands Two valid finding shapes: diff --git a/skills/reflect/references/tooling-reviewer.md b/skills/reflect/references/tooling-reviewer.md index 8ad762f..e10fff1 100644 --- a/skills/reflect/references/tooling-reviewer.md +++ b/skills/reflect/references/tooling-reviewer.md @@ -33,7 +33,7 @@ Scan for: Findings must point to skills, tools, or MCPs invoked in this transcript. Speculative routings to skills the parent never opened do not count. To check whether a skill was used, scan the transcript for: - `Read` tool calls against any `SKILL.md` file in the active Harness's project, user, or plugin skill roots. -- `Task` prompts that name a skill path +- Delegation prompts that name a skill path - Tool calls (Shell, Grep, MCP, etc.) that match a skill's documented commands Two valid finding shapes: From 1d1e4bd10aeb86fd4cb3f5c1555c7079aa1090f9 Mon Sep 17 00:00:00 2001 From: 3metaJun <251347867+3metaJun@users.noreply.github.com> Date: Fri, 11 Sep 2026 17:18:15 +0800 Subject: [PATCH 2/2] fix: validate Codex artifact paths and formats before install --- README.md | 23 +++- docs/harness-adapters.md | 7 +- package.json | 2 +- profiles/environments.example.json | 2 +- scripts/agent-format.mjs | 66 ++++++++++ scripts/agent-format.test.mjs | 105 ++++++++++++++++ scripts/environment.test.mjs | 143 ++++++++++++++++++---- scripts/install-paths.mjs | 31 +++++ scripts/install.mjs | 45 +++---- scripts/install.test.mjs | 138 ++++++++++++++++++++- scripts/remote-install.mjs | 23 +++- scripts/validate.mjs | 28 +---- skills/meta-mode/playbooks/orchestrate.md | 2 +- 13 files changed, 523 insertions(+), 92 deletions(-) create mode 100644 scripts/agent-format.mjs create mode 100644 scripts/agent-format.test.mjs diff --git a/README.md b/README.md index 23d0297..7da4060 100644 --- a/README.md +++ b/README.md @@ -59,11 +59,24 @@ npx @3metajun/mstack --harness all --no-skills --artifact all The available installable artifacts are: -| Artifact | Default destination (beside the harness `skills/` directory) | +| Artifact | Default destination | | --- | --- | -| `agents` | `agents/` (Codex: `$CODEX_HOME/agents/` as `.toml`) | -| `meta-mode-tools` | `tools/meta-mode/` | -| `guide` | `docs/guide/` | +| `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/` | + +Codex skills default to `~/.agents/skills/`, while Codex agents default to +`~/.codex/agents/`. An unset or empty `CODEX_HOME` uses `~/.codex`. +`HARNESS_SKILLS_CODEX_DIR` relocates skills only; agents follow `CODEX_HOME` +unless an artifact override is supplied. Codex TOML and Markdown agents must +use separate target directories. Shared targets are allowed only when the +artifact source and output format match. + +For SSH installs, both `/home/dev/.agents/skills` and +`/home/dev/.codex/skills` map agents to `/home/dev/.codex/agents`. +For a custom remote layout or remote `CODEX_HOME`, set +`artifacts.agents.codex` to that remote agent directory. The installer cannot +infer a remote home from an arbitrary skill path or the local `CODEX_HOME`. Artifact destinations can be overridden per harness with `MSTACK_ARTIFACT___DIR`, for example @@ -78,7 +91,7 @@ overrides in either shape below (artifact-first is the documented form): }, "artifacts": { "agents": { - "codex": "C:\\path\\to\\Fleet\\shared\\agents" + "codex": "C:\\path\\to\\Fleet\\codex\\agents" } } } diff --git a/docs/harness-adapters.md b/docs/harness-adapters.md index e5dfd33..b6c1e3b 100644 --- a/docs/harness-adapters.md +++ b/docs/harness-adapters.md @@ -30,7 +30,12 @@ their official references support it. Codex stores agent configuration in `.codex/agents/*.toml` and `~/.codex/agents/*.toml`. The installer converts the portable Markdown agent artifacts to Codex TOML with `name`, `description`, and -`developer_instructions`. Claude Code stores subagent definitions in +`developer_instructions`. Canonical agent files must be top-level Markdown +with non-empty, single-line `name` and `description` strings. Plain, JSON +double-quoted, and YAML single-quoted strings are supported. Unsupported +metadata and nested agent directories fail conversion before installation; +an existing same-name TOML file is never overwritten by conversion. +Claude Code stores subagent definitions in `.claude/agents/`. OpenCode stores agent definitions in `.opencode/agents/` or `~/.config/opencode/agents/`. pi does not require a separate agent file for skill use; it loads skills through discovery, the `--skill` flag, or the diff --git a/package.json b/package.json index 00a1e7e..7bdfcce 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "install-skills": "node scripts/install.mjs", "optimize-context": "node scripts/optimize-context.mjs", "reconcile-context": "node scripts/reconcile-context.mjs", - "test": "node scripts/validate.mjs && node --test scripts/install.test.mjs scripts/context.test.mjs scripts/audit-context.test.mjs scripts/worktree-audit.test.mjs scripts/sync-upstream.test.mjs scripts/runtime.test.mjs scripts/environment.test.mjs scripts/skill-integrity.test.mjs scripts/version-integrity.test.mjs" + "test": "node scripts/validate.mjs && node --test scripts/install.test.mjs scripts/context.test.mjs scripts/audit-context.test.mjs scripts/worktree-audit.test.mjs scripts/sync-upstream.test.mjs scripts/runtime.test.mjs scripts/environment.test.mjs scripts/skill-integrity.test.mjs scripts/version-integrity.test.mjs scripts/agent-format.test.mjs" }, "bin": { "mstack": "scripts/install.mjs" diff --git a/profiles/environments.example.json b/profiles/environments.example.json index 2d0e036..ff5dac4 100644 --- a/profiles/environments.example.json +++ b/profiles/environments.example.json @@ -9,7 +9,7 @@ }, "artifacts": { "agents": { - "codex": "C:\\path\\to\\Fleet\\shared\\agents", + "codex": "C:\\path\\to\\Fleet\\codex\\agents", "claude": "C:\\path\\to\\Fleet\\shared\\agents", "opencode": "C:\\path\\to\\Fleet\\shared\\agents", "pi": "C:\\path\\to\\Fleet\\shared\\agents" diff --git a/scripts/agent-format.mjs b/scripts/agent-format.mjs new file mode 100644 index 0000000..a17534d --- /dev/null +++ b/scripts/agent-format.mjs @@ -0,0 +1,66 @@ +const supportedFields = new Set(["name", "description"]); + +function readScalar(raw, path, field) { + const value = raw.trim(); + let result; + if (value.startsWith('"')) { + const match = value.match(/^("(?:[^"\\]|\\.)*")(?:[ \t]+#.*)?$/); + try { + if (!match) throw new Error(); + result = JSON.parse(match[1]); + } catch { + throw new Error(`${path}: ${field} must use a valid single-line JSON double-quoted string`); + } + } else if (value.startsWith("'")) { + const match = value.match(/^'((?:[^']|'')*)'(?:[ \t]+#.*)?$/); + if (!match) throw new Error(`${path}: ${field} has an invalid single-quoted string`); + result = match[1].replace(/''/g, "'"); + } else { + result = value.replace(/(?:^|[ \t]+)#.*$/, "").trimEnd(); + if (/^[|>&*!\[\]{},@`%]|^[?:-](?:[ \t]|$)|:(?:[ \t]|$)/.test(result)) { + throw new Error(`${path}: ${field} uses unsupported frontmatter syntax; use a single-line string`); + } + if (/^(?:null|true|false|~|[-+]?(?:0x[0-9a-f]+|0o[0-7]+|(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]+)(?:e[-+]?[0-9]+)?|\.inf|\.nan))$/i.test(result)) { + throw new Error(`${path}: ${field} must be a string; quote scalar values`); + } + } + if (!result.trim()) throw new Error(`${path}: ${field} must be a non-empty string`); + return result; +} + +function tomlString(value, path, field) { + for (const character of value) { + const point = character.codePointAt(0); + if (point >= 0xd800 && point <= 0xdfff) { + throw new Error(`${path}: ${field} contains an unpaired Unicode surrogate`); + } + } + // TOML also forbids a literal DEL, which JSON.stringify leaves untouched. + return JSON.stringify(value).replace(/\u007f/g, "\\u007f"); +} + +export function convertAgentMarkdown(content, path) { + const match = content.match(/^---\r?\n([\s\S]*?)\r?\n---(?:\r?\n|$)/); + if (!match) throw new Error(`${path}: agent Markdown has no complete frontmatter`); + const fields = new Map(); + for (const line of match[1].split(/\r?\n/)) { + if (!line.trim() || line.trimStart().startsWith("#")) continue; + const entry = line.match(/^([a-zA-Z_][\w-]*):(?:[ \t]+(.*)|$)$/); + if (!entry) throw new Error(`${path}: unsupported agent frontmatter line ${JSON.stringify(line)}`); + const [, field, raw = ""] = entry; + if (!supportedFields.has(field)) throw new Error(`${path}: unsupported agent frontmatter field ${field}`); + if (fields.has(field)) throw new Error(`${path}: duplicate agent frontmatter field ${field}`); + fields.set(field, readScalar(raw, path, field)); + } + for (const field of supportedFields) { + if (!fields.has(field)) throw new Error(`${path}: agent frontmatter is missing ${field}`); + } + const body = content.slice(match[0].length); + if (!body.trim()) throw new Error(`${path}: agent body must be non-empty`); + return [ + `name = ${tomlString(fields.get("name"), path, "name")}`, + `description = ${tomlString(fields.get("description"), path, "description")}`, + `developer_instructions = ${tomlString(body, path, "body")}`, + "", + ].join("\n"); +} diff --git a/scripts/agent-format.test.mjs b/scripts/agent-format.test.mjs new file mode 100644 index 0000000..b2d75c1 --- /dev/null +++ b/scripts/agent-format.test.mjs @@ -0,0 +1,105 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import test from "node:test"; +import { convertAgentMarkdown } from "./agent-format.mjs"; + +function markdown(frontmatter, body = "Review the supplied files.\n") { + return `---\n${frontmatter}\n---\n${body}`; +} + +test("converts plain, JSON double-quoted and YAML single-quoted agent metadata", () => { + assert.equal( + convertAgentMarkdown(markdown("name: reviewer # routing name\ndescription: 'Don''t skip # details'"), "agent.md"), + 'name = "reviewer"\ndescription = "Don\'t skip # details"\ndeveloper_instructions = "Review the supplied files.\\n"\n', + ); + assert.equal( + convertAgentMarkdown(markdown('name: "reviewer" # routing name\ndescription: "Read \\"quoted\\" paths: C:\\\\work"'), "agent.md"), + 'name = "reviewer"\ndescription = "Read \\"quoted\\" paths: C:\\\\work"\ndeveloper_instructions = "Review the supplied files.\\n"\n', + ); +}); + +test("preserves CRLF body whitespace, Unicode, and control characters in valid TOML escapes", () => { + const source = '---\r\nname: reviewer\r\ndescription: "Unicode \\u4e2d \\ud83d\\ude00 and newline\\n"\r\n---\r\n\r\n body\t\u007f\u0000\r\n'; + assert.equal( + convertAgentMarkdown(source, "agent.md"), + 'name = "reviewer"\ndescription = "Unicode δΈ­ πŸ˜€ and newline\\n"\ndeveloper_instructions = "\\r\\n body\\t\\u007f\\u0000\\r\\n"\n', + ); +}); + +test("rejects malformed metadata without reading a following field as its value", () => { + const cases = [ + ["name:\ndescription: valid", /name must be a non-empty string/], + ["name: # empty\ndescription: valid", /name must be a non-empty string/], + ['name: ""\ndescription: valid', /name must be a non-empty string/], + ["name: reviewer\nname: other\ndescription: valid", /duplicate.*name/], + ["name: reviewer", /missing description/], + ["description: valid", /missing name/], + ["name: reviewer\ndescription: |\n detail", /unsupported.*syntax/], + ["name: reviewer\ndescription: >\n detail", /unsupported.*syntax/], + ["name: reviewer\ndescription: [one, two]", /unsupported.*syntax/], + ["name: reviewer\ndescription: {text: value}", /unsupported.*syntax/], + ["name: reviewer\ndescription: &description text", /unsupported.*syntax/], + ["name: reviewer\ndescription: *description", /unsupported.*syntax/], + ["name: reviewer\ndescription: !!str value", /unsupported.*syntax/], + ["name: reviewer\ndescription: text: value", /unsupported.*syntax/], + ["name: reviewer\ndescription: valid\n nested: value", /unsupported.*line/], + ["name:reviewer\ndescription: valid", /unsupported.*line/], + ["name: reviewer\ndescription: valid\nmodel: custom", /unsupported.*field model/], + ['name: reviewer\ndescription: "bad\\escape"', /valid.*double-quoted string/], + ['name: reviewer\ndescription: "unfinished', /valid.*double-quoted string/], + ["name: reviewer\ndescription: 'Don't'", /invalid single-quoted string/], + ['name: reviewer\ndescription: "valid" trailing', /valid.*double-quoted string/], + ]; + for (const [frontmatter, expected] of cases) { + assert.throws(() => convertAgentMarkdown(markdown(frontmatter), "broken.md"), (error) => { + assert.match(error.message, /^broken\.md:/); + assert.match(error.message, expected); + return true; + }); + } + for (const scalar of ["123", "true", "false", "null", "~", ".nan", "0x12", "1.5e3"]) { + assert.throws(() => convertAgentMarkdown(markdown(`name: ${scalar}\ndescription: valid`), "broken.md"), /must be a string/); + } +}); + +test("rejects missing frontmatter, empty body, and unpaired Unicode surrogates", () => { + assert.throws(() => convertAgentMarkdown("# No metadata", "broken.md"), /no complete frontmatter/); + assert.throws(() => convertAgentMarkdown(markdown("name: reviewer\ndescription: valid", " \n"), "broken.md"), /body must be non-empty/); + for (const invalid of ["\ud800", "\udfff"]) { + assert.throws(() => convertAgentMarkdown(markdown("name: reviewer\ndescription: valid", invalid), "broken.md"), /body contains an unpaired Unicode surrogate/); + assert.throws(() => convertAgentMarkdown(markdown(`name: reviewer\ndescription: "${invalid}"`), "broken.md"), /description contains an unpaired Unicode surrogate/); + } +}); + +test("round-trips actual agent files and escaped strings through Python's standard TOML parser", (t) => { + const candidates = process.platform === "win32" ? ["python", "python3"] : ["python3", "python"]; + const python = candidates.find((command) => spawnSync(command, ["-c", "import tomllib"], { encoding: "utf8" }).status === 0); + if (!python) return t.skip("Python 3.11+ with tomllib is unavailable; literal conversion assertions still run"); + const fixtures = [ + ["comment-reviewer.md", "comment-reviewer"], + ["meta-agent.md", "meta-agent"], + ].map(([file, name]) => { + const source = readFileSync(new URL(`../agents/${file}`, import.meta.url), "utf8"); + const frontmatter = source.match(/^---\r?\n([\s\S]*?)\r?\n---\r?\n/); + return { + toml: convertAgentMarkdown(source, file), + expected: { + name, + description: frontmatter[1].split(/\r?\n/).find((line) => line.startsWith("description: ")).slice("description: ".length), + developer_instructions: source.slice(frontmatter[0].length), + }, + }; + }); + const description = 'Quoted "paths": C:\\work, δΈ­ζ–‡ πŸ˜€, newline\n'; + const body = ` All controls: ${Array.from({ length: 32 }, (_, index) => String.fromCharCode(index)).join("")}\u007f\r\nUnicode δΈ­ζ–‡ πŸ˜€\n`; + fixtures.push({ + toml: convertAgentMarkdown(markdown(`name: reviewer\ndescription: ${JSON.stringify(description)}`, body), "escaped.md"), + expected: { name: "reviewer", description, developer_instructions: body }, + }); + const result = spawnSync(python, ["-c", "import json, sys, tomllib; fixtures = json.load(sys.stdin); [None if tomllib.loads(f['toml']) == f['expected'] else sys.exit('TOML round-trip mismatch') for f in fixtures]"], { + input: JSON.stringify(fixtures), + encoding: "utf8", + }); + assert.equal(result.status, 0, result.stderr); +}); diff --git a/scripts/environment.test.mjs b/scripts/environment.test.mjs index b7f5028..45dc180 100644 --- a/scripts/environment.test.mjs +++ b/scripts/environment.test.mjs @@ -165,27 +165,82 @@ test("installer delegates an SSH environment to a no-connect dry run", () => { } }); -test("remote Codex agents use the official user agent directory", () => { +test("remote Codex agents use the official directory from either standard skill path", () => { const root = mkdtempSync(join(tmpdir(), "mstack-remote-codex-agents-test-")); const configPath = join(root, "environments.json"); - writeFileSync(configPath, JSON.stringify({ - fleet: { - transport: "ssh", - host: "dev@tailnet-host", - shell: "posix", - targets: { codex: "/home/dev/.agents/skills" }, - }, - }), "utf8"); try { - const result = spawnSync(process.execPath, [remoteInstaller, - "--harness", "codex", - "--environment", "fleet", - "--no-skills", - "--artifact", "agents", - "--dry-run", - ], { cwd: resolve("."), env: { ...process.env, MSTACK_ENVIRONMENTS_FILE: configPath }, encoding: "utf8" }); - assert.equal(result.status, 0, result.stderr); - assert.match(result.stdout, /agents -> \/home\/dev\/\.codex\/agents/); + for (const target of ["/home/dev/.agents/skills", "/home/dev/.codex/skills"]) { + writeFileSync(configPath, JSON.stringify({ + fleet: { + transport: "ssh", + host: "dev@tailnet-host", + shell: "posix", + targets: { codex: target }, + }, + }), "utf8"); + const result = spawnSync(process.execPath, [remoteInstaller, + "--harness", "codex", + "--environment", "fleet", + "--no-skills", + "--artifact", "agents", + "--dry-run", + ], { cwd: resolve("."), env: { ...process.env, MSTACK_ENVIRONMENTS_FILE: configPath }, encoding: "utf8" }); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /agents -> \/home\/dev\/\.codex\/agents\r?\n/, target); + } + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("custom remote Codex skill paths require an explicit agent target before connecting", () => { + const root = mkdtempSync(join(tmpdir(), "mstack-remote-custom-codex-test-")); + const configPath = join(root, "environments.json"); + const helperPath = join(root, "fake-transport.mjs"); + const logPath = join(root, "transport.log"); + writeFileSync(helperPath, [ + 'import { appendFileSync } from "node:fs";', + 'appendFileSync(process.env.MSTACK_FAKE_TRANSPORT_LOG, "connected\\n");', + 'process.exit(91);', + ].join("\n"), "utf8"); + try { + for (const target of ["/srv/mstack/skills", "/home/dev/.agents/custom-skills"]) { + const environment = { + transport: "ssh", + host: "dev@tailnet-host", + targets: { codex: target }, + sshCommand: process.execPath, + sshArgs: [helperPath], + }; + const env = { + ...process.env, + MSTACK_ENVIRONMENTS_FILE: configPath, + MSTACK_FAKE_TRANSPORT_LOG: logPath, + }; + const args = [remoteInstaller, + "--harness", "codex", + "--environment", "fleet", + "--no-skills", + "--artifact", "agents", + ]; + writeFileSync(configPath, JSON.stringify({ fleet: environment }), "utf8"); + const missing = spawnSync(process.execPath, args, { cwd: resolve("."), env, encoding: "utf8" }); + assert.notEqual(missing.status, 0); + assert.match(missing.stderr, /artifacts\.agents\.codex/, target); + assert.equal(existsSync(logPath), false, "invalid paths must fail before connecting"); + + for (const artifacts of [ + { agents: { codex: "/srv/custom-codex/agents" } }, + { codex: { agents: "/srv/custom-codex/agents" } }, + ]) { + writeFileSync(configPath, JSON.stringify({ fleet: { ...environment, artifacts } }), "utf8"); + const explicit = spawnSync(process.execPath, [...args, "--dry-run"], { + cwd: resolve("."), env, encoding: "utf8", + }); + assert.equal(explicit.status, 0, explicit.stderr); + assert.match(explicit.stdout, /agents -> \/srv\/custom-codex\/agents\r?\n/); + } + } } finally { rmSync(root, { recursive: true, force: true }); } @@ -284,6 +339,7 @@ test("remote staging ignores inherited local artifact target overrides", () => { host: "dev@tailnet-host", shell: "posix", targets: { codex: "/srv/remote/skills" }, + artifacts: { agents: { codex: "/srv/remote/.codex/agents" } }, sshCommand: process.execPath, sshArgs: [helperPath, "ssh"], rsyncCommand: process.execPath, @@ -314,7 +370,7 @@ test("remote staging ignores inherited local artifact target overrides", () => { } }); -test("remote installer deduplicates shared artifacts and locks every parent in sorted order", () => { +test("remote installer deduplicates artifacts with matching formats and locks every parent in sorted order", () => { const root = mkdtempSync(join(tmpdir(), "mstack-remote-shared-artifact-test-")); const configPath = join(root, "environments.json"); const helperPath = join(root, "fake-transport.mjs"); @@ -336,16 +392,16 @@ test("remote installer deduplicates shared artifacts and locks every parent in s host: "dev@tailnet-host", shell: "posix", targets: { - codex: "/srv/z/codex/skills", + opencode: "/srv/z/opencode/skills", claude: "/srv/y/claude/skills", }, artifacts: { agents: { - codex: "/srv/z/shared-agents", + opencode: "/srv/z/shared-agents", claude: "/srv/z/shared-agents", }, guide: { - codex: "/srv/m/docs/guide", + opencode: "/srv/m/docs/guide", claude: "/srv/a/docs/guide", }, }, @@ -357,7 +413,7 @@ test("remote installer deduplicates shared artifacts and locks every parent in s }), "utf8"); try { const result = spawnSync(process.execPath, [remoteInstaller, - "--harness", "codex,claude", + "--harness", "opencode,claude", "--environment", "fleet", "--no-skills", "--artifact", "agents,guide", @@ -391,6 +447,47 @@ test("remote installer deduplicates shared artifacts and locks every parent in s } }); +test("remote installer rejects conflicting artifact formats in either order before connecting", () => { + const root = mkdtempSync(join(tmpdir(), "mstack-remote-format-collision-test-")); + const configPath = join(root, "environments.json"); + const helperPath = join(root, "fake-transport.mjs"); + const logPath = join(root, "transport.log"); + writeFileSync(helperPath, [ + 'import { appendFileSync } from "node:fs";', + 'appendFileSync(process.env.MSTACK_FAKE_TRANSPORT_LOG, "connected\\n");', + 'process.exit(91);', + ].join("\n"), "utf8"); + writeFileSync(configPath, JSON.stringify({ + fleet: { + transport: "ssh", + host: "dev@tailnet-host", + targets: { codex: "/home/dev/.agents/skills", claude: "/home/dev/.claude/skills" }, + artifacts: { agents: { codex: "/srv/shared-agents", claude: "/srv/shared-agents" } }, + sshCommand: process.execPath, + sshArgs: [helperPath], + }, + }), "utf8"); + try { + for (const harnesses of ["codex,claude", "claude,codex"]) { + const result = spawnSync(process.execPath, [remoteInstaller, + "--harness", harnesses, + "--environment", "fleet", + "--no-skills", + "--artifact", "agents", + ], { + cwd: resolve("."), + env: { ...process.env, MSTACK_ENVIRONMENTS_FILE: configPath, MSTACK_FAKE_TRANSPORT_LOG: logPath }, + encoding: "utf8", + }); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Remote target collision/, harnesses); + assert.equal(existsSync(logPath), false, "format conflicts must fail before connecting"); + } + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + test("remote installer rejects non-shareable target collisions before connecting", () => { const root = mkdtempSync(join(tmpdir(), "mstack-remote-collision-test-")); const configPath = join(root, "environments.json"); diff --git a/scripts/install-paths.mjs b/scripts/install-paths.mjs index 1164040..d145d92 100644 --- a/scripts/install-paths.mjs +++ b/scripts/install-paths.mjs @@ -27,6 +27,37 @@ export function artifactPathParts(name, definition, harness) { return parts; } +export function validateArtifactOverrides(name, definition, validHarnesses) { + artifactPathParts(name, definition); + const overrides = definition.harnesses; + if (overrides === undefined) return; + if (!overrides || typeof overrides !== "object" || Array.isArray(overrides)) { + throw new Error(`Artifact ${name} harnesses must be an object`); + } + for (const [harness, override] of Object.entries(overrides)) { + if (!validHarnesses.includes(harness)) throw new Error(`Artifact ${name} has unknown harness: ${harness}`); + if (!override || typeof override !== "object" || Array.isArray(override)) { + throw new Error(`Artifact ${name} harness ${harness} must be an object`); + } + for (const field of Object.keys(override)) { + if (!["path", "base", "format"].includes(field)) { + throw new Error(`Artifact ${name} harness ${harness} has unsupported field: ${field}`); + } + } + if (override.path === null) throw new Error(`Artifact ${name} must define a safe non-empty path array`); + artifactPathParts(name, definition, harness); + if (override.base !== undefined && !["harness", "codex-home"].includes(override.base)) { + throw new Error(`Artifact ${name} harness ${harness} has unsupported base: ${override.base}`); + } + if (override.format !== undefined && !["copy", "codex-toml"].includes(override.format)) { + throw new Error(`Artifact ${name} harness ${harness} has unsupported format: ${override.format}`); + } + if (harness !== "codex" && (override.base === "codex-home" || override.format === "codex-toml")) { + throw new Error(`Artifact ${name}: codex-home and codex-toml are only valid for codex`); + } + } +} + export function stagingPath(target, transactionId, sequence) { return join(dirname(target), `.harness-skills-stage-${transactionId}-${sequence}`); } diff --git a/scripts/install.mjs b/scripts/install.mjs index f98c253..97c7468 100644 --- a/scripts/install.mjs +++ b/scripts/install.mjs @@ -20,12 +20,14 @@ 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 { artifactPathParts, localPathKey, pathIsWithin, stagingPath, targetPathsOverlap, + validateArtifactOverrides, } from "./install-paths.mjs"; if (Number.parseInt(process.versions.node, 10) < 18) { @@ -41,6 +43,9 @@ const artifactProfile = JSON.parse(readFileSync(join(repoRoot, "profiles", "arti const artifactRegistry = artifactProfile.artifacts ?? {}; const validHarnesses = Object.keys(harnessRegistry); const validArtifacts = Object.keys(artifactRegistry); +for (const [name, definition] of Object.entries(artifactRegistry)) { + if (definition.installable !== false) validateArtifactOverrides(name, definition, validHarnesses); +} function valueAfter(flag) { const index = args.indexOf(flag); @@ -96,7 +101,7 @@ function pathFromParts(parts) { function artifactBase(name, harness) { const base = artifactRegistry[name]?.harnesses?.[harness]?.base; if (base === "codex-home") { - return configuredPath(process.env.CODEX_HOME, join(userHome, ".codex"), "CODEX_HOME"); + return configuredPath(process.env.CODEX_HOME || undefined, join(userHome, ".codex"), "CODEX_HOME"); } return dirname(targets[harness]); } @@ -167,7 +172,7 @@ function artifactTarget(name, harness) { const root = artifactBase(name, harness); const target = resolve(root, ...artifactPathParts(name, definition, harness)); if (!pathIsWithin(root, target)) { - throw new Error(`Artifact ${name} target must stay within the harness configuration directory`); + throw new Error(`Artifact ${name} target must stay within its configured base directory`); } return target; } @@ -249,35 +254,15 @@ function copyDirectoryContents(source, target) { } } -function readAgentFrontmatter(content, path) { - const match = content.match(/^---\r?\n([\s\S]*?)\r?\n---\r?\n/); - if (!match) throw new Error(`${path} has no frontmatter`); - const field = (name) => { - const line = match[1].match(new RegExp(`^${name}:\\s*(.+)$`, "m")); - if (!line?.[1]) throw new Error(`${path} has no ${name}`); - const value = line[1].trim(); - try { return JSON.parse(value); } catch { return value; } - }; - return { name: field("name"), description: field("description"), body: content.slice(match[0].length) }; -} - function applyArtifactAdapter(staged, item) { - const format = artifactRegistry[item.name]?.harnesses?.[item.harness]?.format; - if (format !== "codex-toml") return; + if (item.format !== "codex-toml") return; for (const entry of readdirSync(staged, { withFileTypes: true })) { + if (entry.isDirectory()) throw new Error(`Codex agent files must be top-level: ${entry.name}`); if (!entry.isFile() || !entry.name.endsWith(".md")) continue; const sourcePath = join(staged, entry.name); - const agent = readAgentFrontmatter(readFileSync(sourcePath, "utf8"), sourcePath); - if (typeof agent.name !== "string" || typeof agent.description !== "string" || typeof agent.body !== "string" || !agent.body.trim()) { - throw new Error(`${sourcePath} is not a valid Codex agent definition`); - } const targetPath = join(staged, `${basename(entry.name, ".md")}.toml`); - writeFileSync(targetPath, [ - `name = ${JSON.stringify(agent.name)}`, - `description = ${JSON.stringify(agent.description)}`, - `developer_instructions = ${JSON.stringify(agent.body)}`, - "", - ].join("\n"), "utf8"); + if (existsSync(targetPath)) throw new Error(`Converted Codex agent target already exists: ${targetPath}`); + writeFileSync(targetPath, convertAgentMarkdown(readFileSync(sourcePath, "utf8"), sourcePath), { encoding: "utf8", flag: "wx" }); unlinkSync(sourcePath); } } @@ -384,6 +369,7 @@ const rawArtifactPlan = harnesses.flatMap((harness) => harness, name, source: artifactSources[name], + format: artifactRegistry[name].harnesses?.[harness]?.format ?? "copy", target: artifactTargets[`${harness}:${name}`], })), ); @@ -393,7 +379,12 @@ for (const item of rawArtifactPlan) { const key = localPathKey(item.target); const existing = artifactTargetsByPath.get(key); if (existing) { - if (existing.name === item.name && existing.source === item.source) continue; + if (existing.name === item.name && existing.source === item.source) { + if (existing.format !== item.format) { + throw new Error(`Target ${item.target} has conflicting artifact formats for ${existing.harness} and ${item.harness}; configure separate artifact directories`); + } + continue; + } throw new Error("Selected skills and artifacts resolve to the same target directory"); } artifactTargetsByPath.set(key, item); diff --git a/scripts/install.test.mjs b/scripts/install.test.mjs index 81e6c2b..cc925a8 100644 --- a/scripts/install.test.mjs +++ b/scripts/install.test.mjs @@ -442,7 +442,7 @@ test("replaces a custom artifact with a backup on the artifact target volume", ( } }); -test("deduplicates a shared artifact target across harnesses", () => { +test("deduplicates a shared artifact target with the same output format", () => { const { root, env } = fixture(); const environmentFile = join(root, "environments.json"); const sharedAgents = join(root, "fleet", "shared", "agents"); @@ -451,10 +451,10 @@ test("deduplicates a shared artifact target across harnesses", () => { JSON.stringify({ fleet: { targets: { - codex: join(root, "fleet", "codex", "skills"), + opencode: join(root, "fleet", "opencode", "skills"), claude: join(root, "fleet", "claude", "skills"), }, - artifacts: { agents: { codex: sharedAgents, claude: sharedAgents } }, + artifacts: { agents: { opencode: sharedAgents, claude: sharedAgents } }, }, }), "utf8", @@ -462,18 +462,144 @@ test("deduplicates a shared artifact target across harnesses", () => { env.MSTACK_ENVIRONMENTS_FILE = environmentFile; try { const result = run( - ["--harness", "codex,claude", "--environment", "fleet", "--no-skills", "--artifact", "agents"], + ["--harness", "opencode,claude", "--environment", "fleet", "--no-skills", "--artifact", "agents"], env, ); assert.equal(result.status, 0, result.stderr); - assert.equal(existsSync(join(sharedAgents, "meta-agent.toml")), true); - assert.equal(existsSync(join(sharedAgents, "meta-agent.md")), false); + assert.equal(existsSync(join(sharedAgents, "meta-agent.md")), true); + assert.equal(existsSync(join(sharedAgents, "meta-agent.toml")), false); assert.match(result.stdout, /Installed 0 skill copies and 1 artifact copies/); } finally { rmSync(root, { recursive: true, force: true }); } }); +test("rejects shared artifact targets with conflicting output formats in either order", () => { + const { root, env } = fixture(); + const target = join(root, "shared-agents"); + mkdirSync(target); + writeFileSync(join(target, "marker.txt"), "preserve"); + env.MSTACK_ARTIFACT_AGENTS_CODEX_DIR = target; + env.MSTACK_ARTIFACT_AGENTS_CLAUDE_DIR = target; + try { + for (const harnesses of ["codex,claude", "claude,codex"]) { + const result = run(["--harness", harnesses, "--no-skills", "--artifact", "agents", "--replace"], env); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /conflicting artifact formats/); + assert.deepEqual(readdirSync(target), ["marker.txt"]); + assert.equal(readFileSync(join(target, "marker.txt"), "utf8"), "preserve"); + assert.deepEqual(readdirSync(root), ["shared-agents"]); + } + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("Codex agents default to the user home when CODEX_HOME is unset or empty", () => { + for (const configuredHome of [undefined, ""]) { + const { root, env } = fixture(); + env.HOME = root; + env.USERPROFILE = root; + if (configuredHome === undefined) delete env.CODEX_HOME; + else env.CODEX_HOME = configuredHome; + try { + const result = run(["--harness", "codex", "--no-skills", "--artifact", "agents"], env); + assert.equal(result.status, 0, result.stderr); + assert.equal(existsSync(join(root, ".codex", "agents", "meta-agent.toml")), true); + assert.equal(existsSync(env.HARNESS_SKILLS_CODEX_DIR), false); + } finally { + rmSync(root, { recursive: true, force: true }); + } + } +}); + +test("artifact override typos fail validation and local and remote installs before writes", () => { + const fixture_ = copiedInstallerFixture(); + cpSync(resolve("skills"), join(fixture_.repository, "skills"), { recursive: true }); + const profilePath = join(fixture_.repository, "profiles", "artifacts.json"); + const baseline = JSON.parse(readFileSync(profilePath, "utf8")); + const environmentFile = join(fixture_.root, "environments.json"); + writeFileSync(environmentFile, JSON.stringify({ fleet: { + transport: "ssh", host: "dev@example.test", targets: { codex: "/home/dev/.agents/skills" }, + } })); + try { + for (const [overrides, diagnostic] of [ + [{ codex: { format: "codex_TOML" } }, /unsupported format/], + [{ cdoex: { format: "codex-toml" } }, /unknown harness/], + [{ codex: { base: "codex_home" } }, /unsupported base/], + [{ claude: { base: "codex-home" } }, /only valid for codex/], + [{ codex: { path: [".."] } }, /safe non-empty path/], + ]) { + const profile = structuredClone(baseline); + profile.artifacts.agents.harnesses = overrides; + writeFileSync(profilePath, JSON.stringify(profile)); + for (const [script, args] of [ + [fixture_.installer, ["--harness", "codex", "--artifact", "agents", "--no-skills", "--dry-run"]], + [fixture_.remoteInstaller, ["--harness", "codex", "--artifact", "agents", "--no-skills", "--environment", "fleet", "--dry-run"]], + [join(fixture_.repository, "scripts", "validate.mjs"), []], + ]) { + const result = runCopied(script, args, { ...fixture_.env, MSTACK_ENVIRONMENTS_FILE: environmentFile }, fixture_.repository); + assert.notEqual(result.status, 0); + assert.match(result.stderr, diagnostic); + } + assert.equal(existsSync(join(fixture_.root, "install")), false); + } + } finally { + rmSync(fixture_.root, { recursive: true, force: true }); + } +}); + +test("Codex conversion failures preserve an existing install", () => { + for (const scenario of ["nested", "collision", "metadata"]) { + const fixture_ = copiedInstallerFixture(); + const source = join(fixture_.repository, "agents"); + const codexHome = join(fixture_.root, "codex-home"); + const target = join(codexHome, "agents"); + mkdirSync(target, { recursive: true }); + writeFileSync(join(target, "previous.toml"), "keep previous install"); + if (scenario === "nested") { + mkdirSync(join(source, "nested")); + cpSync(join(source, "meta-agent.md"), join(source, "nested", "meta-agent.md")); + } else if (scenario === "collision") { + writeFileSync(join(source, "meta-agent.toml"), "do not overwrite"); + } else { + writeFileSync(join(source, "meta-agent.md"), "---\nname:\ndescription: valid\n---\nReview\n"); + } + try { + const result = runCopied(fixture_.installer, + ["--harness", "codex", "--artifact", "agents", "--no-skills", "--replace"], + { ...fixture_.env, CODEX_HOME: codexHome }, fixture_.repository); + assert.notEqual(result.status, 0); + const diagnostic = { nested: /must be top-level/, collision: /already exists/, metadata: /name must be a non-empty string/ }; + assert.match(result.stderr, diagnostic[scenario]); + assert.deepEqual(readdirSync(target), ["previous.toml"]); + assert.equal(readFileSync(join(target, "previous.toml"), "utf8"), "keep previous install"); + assert.deepEqual(readdirSync(codexHome), ["agents"]); + if (scenario === "collision") assert.equal(readFileSync(join(source, "meta-agent.toml"), "utf8"), "do not overwrite"); + } finally { + rmSync(fixture_.root, { recursive: true, force: true }); + } + } +}); + +test("a harness artifact path override controls the staged output destination", () => { + const fixture_ = copiedInstallerFixture(); + const profilePath = join(fixture_.repository, "profiles", "artifacts.json"); + const profile = JSON.parse(readFileSync(profilePath, "utf8")); + profile.artifacts.agents.harnesses.codex.path = ["review", "roles"]; + writeFileSync(profilePath, JSON.stringify(profile)); + const codexHome = join(fixture_.root, "codex-home"); + try { + const result = runCopied(fixture_.installer, ["--harness", "codex", "--no-skills", "--artifact", "agents"], + { ...fixture_.env, CODEX_HOME: codexHome }, fixture_.repository); + assert.equal(result.status, 0, result.stderr); + assert.equal(existsSync(join(codexHome, "review", "roles", "meta-agent.toml")), true); + assert.equal(existsSync(join(codexHome, "agents")), false); + } finally { + rmSync(fixture_.root, { recursive: true, force: true }); + } +}); + test("rejects unsupported artifacts and invalid artifact filters", () => { const { root, env } = fixture(); try { diff --git a/scripts/remote-install.mjs b/scripts/remote-install.mjs index ec58da7..97e69bc 100644 --- a/scripts/remote-install.mjs +++ b/scripts/remote-install.mjs @@ -11,7 +11,7 @@ import { dirname, join, posix, resolve } from "node:path"; import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; import { parseRemoteStage, quotePosix, readEnvironment } from "./environment-lib.mjs"; -import { artifactPathParts } from "./install-paths.mjs"; +import { artifactPathParts, validateArtifactOverrides } from "./install-paths.mjs"; const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); const profilesRoot = join(repoRoot, "profiles"); @@ -80,6 +80,9 @@ const unknownArtifacts = artifacts.filter((name) => !Object.hasOwn(artifactRegis if (unknownArtifacts.length) throw new Error(`Unknown artifact: ${unknownArtifacts.join(", ")}`); const unsupported = artifacts.filter((name) => artifactRegistry[name].installable === false); if (unsupported.length) throw new Error(`Unsupported artifact selection: ${unsupported.join(", ")}`); +for (const name of artifacts) { + validateArtifactOverrides(name, artifactRegistry[name], Object.keys(harnessRegistry)); +} const artifactPaths = Object.fromEntries( artifacts.map((name) => [name, Object.fromEntries( harnesses.map((harness) => [harness, artifactPathParts(name, artifactRegistry[name], harness)]), @@ -97,10 +100,18 @@ function artifactEnvironmentPath(name, harness) { function remoteArtifactTarget(name, harness) { const custom = artifactEnvironmentPath(name, harness); if (custom) return custom; - const skillParent = posix.dirname(environment.targets[harness]); + const skillTarget = posix.normalize(environment.targets[harness]); + const skillParent = posix.dirname(skillTarget); const codexHome = artifactRegistry[name].harnesses?.[harness]?.base === "codex-home"; - const base = codexHome && posix.basename(skillParent) === ".agents" ? posix.dirname(skillParent) : skillParent; - return posix.join(base, ...(codexHome ? [".codex"] : []), ...artifactPaths[name][harness]); + if (codexHome) { + if (posix.basename(skillTarget) !== "skills" || ![".agents", ".codex"].includes(posix.basename(skillParent))) { + throw new Error( + `Cannot infer remote Codex home from ${skillTarget}; configure environment artifacts.${name}.${harness} explicitly`, + ); + } + return posix.join(posix.dirname(skillParent), ".codex", ...artifactPaths[name][harness]); + } + return posix.join(skillParent, ...artifactPaths[name][harness]); } const requestedItems = harnesses.flatMap((harness) => [ @@ -116,6 +127,7 @@ const requestedItems = harnesses.flatMap((harness) => [ name, kind: "artifact", source: artifactRegistry[name].source, + format: artifactRegistry[name].harnesses?.[harness]?.format ?? "copy", target: posix.normalize(remoteArtifactTarget(name, harness)), })), ]); @@ -144,7 +156,8 @@ for (const item of requestedItems) { item.kind === "artifact" && existing.kind === "artifact" && item.name === existing.name && - item.source === existing.source; + item.source === existing.source && + item.format === existing.format; if (!sharedArtifact) { throw new Error( `Remote target collision: ${item.target} is selected by ` + diff --git a/scripts/validate.mjs b/scripts/validate.mjs index 075b899..ec3a843 100644 --- a/scripts/validate.mjs +++ b/scripts/validate.mjs @@ -1,11 +1,13 @@ import { existsSync, readFileSync, readdirSync, statSync } from "node:fs"; import { dirname, join, relative, resolve, sep } from "node:path"; import { fileURLToPath } from "node:url"; +import { validateArtifactOverrides } from "./install-paths.mjs"; const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); const skillsRoot = join(repoRoot, "skills"); const EXPECTED_SKILLS = JSON.parse(readFileSync(join(repoRoot, "profiles", "skills.json"), "utf8")).skills; const artifactsProfilePath = join(repoRoot, "profiles", "artifacts.json"); +const harnesses = Object.keys(JSON.parse(readFileSync(join(repoRoot, "profiles", "harnesses.json"), "utf8"))); const errors = []; @@ -28,27 +30,10 @@ try { } } if (artifact.installable !== false) { - if (!Array.isArray(artifact.path) || artifact.path.length === 0 || artifact.path.some((part) => typeof part !== "string" || !part || part === "." || part === ".." || part.includes("/") || part.includes("\\"))) { - errors.push(`artifact ${name} must define a safe non-empty path array`); - } - if (artifact.harnesses !== undefined && (!artifact.harnesses || typeof artifact.harnesses !== "object" || Array.isArray(artifact.harnesses))) { - errors.push(`artifact ${name} harnesses must be an object`); - } else { - for (const [harness, override] of Object.entries(artifact.harnesses ?? {})) { - if (!override || typeof override !== "object" || Array.isArray(override)) { - errors.push(`artifact ${name} harness ${harness} must be an object`); - continue; - } - if (override.path !== undefined && (!Array.isArray(override.path) || override.path.length === 0 || override.path.some((part) => typeof part !== "string" || !part || part === "." || part === ".." || part.includes("/") || part.includes("\\")))) { - errors.push(`artifact ${name} harness ${harness} must define a safe non-empty path array`); - } - if (override.base !== undefined && !["harness", "codex-home"].includes(override.base)) { - errors.push(`artifact ${name} harness ${harness} has an unsupported base`); - } - if (override.format !== undefined && typeof override.format !== "string") { - errors.push(`artifact ${name} harness ${harness} format must be a string`); - } - } + try { + validateArtifactOverrides(name, artifact, harnesses); + } catch (error) { + errors.push(error.message); } } else if (typeof artifact.reason !== "string" || !artifact.reason) { errors.push(`unsupported artifact ${name} must explain its reason`); @@ -82,7 +67,6 @@ const forbidden = [ /\bAskQuestion\b/i, ]; -const harnesses = Object.keys(JSON.parse(readFileSync(join(repoRoot, "profiles", "harnesses.json"), "utf8"))); const adapterAllowedFields = new Set(["compatibility", "metadata"]); const adapterRemovableFields = new Set(["metadata"]); diff --git a/skills/meta-mode/playbooks/orchestrate.md b/skills/meta-mode/playbooks/orchestrate.md index 40cc038..ba6b27d 100644 --- a/skills/meta-mode/playbooks/orchestrate.md +++ b/skills/meta-mode/playbooks/orchestrate.md @@ -13,7 +13,7 @@ Three rules carry the rest. #### Roles and placement - **Coordinator (this chat).** Local. Frames, authors briefs, drains the inbox, owns the human report, makes judgment calls. It never authors or edits code. Conflicted merges, restacks, and code changes are always tasks. Mechanically landing a verified unit (fast-forward or clean cherry-pick of a worker's commit, then push) is bookkeeping the coordinator may do itself on repos where local git is cheap. Queueing finished work behind an idle stacker is how a deadline harvests nothing. The loop is agentic end to end. Agents are spawned, resumed, and drained only through the delegation tool. State reads and writes go through `/orch/orch.ts` at drain points, one command in and one line out. The CLI never spawns, waits, or wakes anything. -- **Sub-coordinator.** Always local, durable, one per track, and only when the program exceeds what one coordinator's drains can manage. A track the coordinator can drain itself needs no middle layer. Each nested layer re-pays a full orientation preamble, and a blocking sub-coordinator hides its children while the parent idles. Owns its track's units and boards, authors its workers' briefs, spawns its own workers and verifiers (nesting works to depth 3, and a nested spawn has the full Task schema including `environment`). Rolls up aggregates at wave boundaries. Never forwards raw child reports. Cap in-flight children at what one drain can process, roughly ten, as a rolling window. Never as blocking batches, which cost the slowest child of every batch. +- **Sub-coordinator.** Always local, durable, one per track, and only when the program exceeds what one coordinator's drains can manage. A track the coordinator can drain itself needs no middle layer. Each nested layer re-pays a full orientation preamble, and a blocking sub-coordinator hides its children while the parent idles. Owns its track's units and boards, authors its workers' briefs, and spawns its own workers and verifiers when the active Harness supports nested delegation. Respect its depth limit and supported delegation fields. Rolls up aggregates at wave boundaries. Never forwards raw child reports. Cap in-flight children at what one drain can process, roughly ten, as a rolling window. Never as blocking batches, which cost the slowest child of every batch. - **Worker / verifier.** Use a configured remote environment unless the task needs this machine. Use the active harness's runtime driver for live verification. Read session history through the history adapter. Local simulators, IDE state, and local credentials stay on this machine. Remote workers cannot read the local store, so their briefs point at repository paths and saved artifacts. Prefer fewer, broader workers. One writer per worktree or branch (principle-separate-before-serializing-shared-state). Run a unit's verifier on a different model family from its worker. Depth stays at coordinator, track, worker. Author the track decomposition per project (build, landing, and verification are common cuts, not a required shape). Hard-coded swarm trees were tried and parked as too rigid.