From 1238bdb6dbb7419887dcfc2d41229bc1d1a07345 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 18:59:01 +0000 Subject: [PATCH 1/8] refactor(codex-operations): drop redundant implicit-invocation policy `allow_implicit_invocation: true` restates the documented default, so the `policy` block on coordinate-codex-work carried no behavior. Verified against learn.chatgpt.com/docs/build-skills and against Codex's own loader, where an absent block and an explicit `true` both resolve through `unwrap_or(true)`. The sibling find-skill-candidates keeps its `false`, which is load-bearing. This moves the repo from 6-of-8 to 7-of-8 openai.yaml files declaring only deviations, matching the upstream scaffold, which emits interface-only. npm test 35/35, npm run validate PASS. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QH54tukcgvdxqwaTb5cdNB --- .../skills/coordinate-codex-work/agents/openai.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/plugins/codex-operations/skills/coordinate-codex-work/agents/openai.yaml b/plugins/codex-operations/skills/coordinate-codex-work/agents/openai.yaml index 1d7cb7e..00df98b 100644 --- a/plugins/codex-operations/skills/coordinate-codex-work/agents/openai.yaml +++ b/plugins/codex-operations/skills/coordinate-codex-work/agents/openai.yaml @@ -2,5 +2,3 @@ interface: display_name: "Coordinate Codex Work" short_description: "Direct multi-task and long-running Codex work." default_prompt: "Use $coordinate-codex-work to coordinate this work across Codex tasks and agents." -policy: - allow_implicit_invocation: true From b3b5197a3e169ac8365fe444fad670745e549ca8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 18:59:01 +0000 Subject: [PATCH 2/8] docs: index the Codex build-skills page for openai.yaml metadata The indexed plugin-builder "Build skills" page documents only the `dependencies` block of `agents/openai.yaml`; it covers neither `interface` nor `policy`. A reader routed there for skill UI or invocation metadata finds nothing, which AGENTS.md's preflight requires this index to prevent. Adds the Codex "Build skills" page, which documents all three keys plus local skill discovery and plugin distribution. Purely additive: the existing pointer is retained unaltered, since the doc-contract test pins 15 exact URLs. npm test 35/35, npm run validate PASS. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QH54tukcgvdxqwaTb5cdNB --- docs/OFFICIAL-DOCS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/OFFICIAL-DOCS.md b/docs/OFFICIAL-DOCS.md index 3e0c303..6049cb1 100644 --- a/docs/OFFICIAL-DOCS.md +++ b/docs/OFFICIAL-DOCS.md @@ -16,6 +16,7 @@ Read the smallest complete set that covers the affected surface. | Built-in image generation and `$imagegen` invocation | [Image generation](https://learn.chatgpt.com/docs/image-generation) | | Plugin boundaries and component model | [Plugin architecture](https://developers.openai.com/plugins/concepts/plugins) | | Skill format, triggers, resources, and tests | [Build skills](https://developers.openai.com/plugins/build/skills) | +| Skill loading, distribution, and the `agents/openai.yaml` keys `interface`, `policy`, and `dependencies` | [Build skills for Codex](https://learn.chatgpt.com/docs/build-skills) | | MCP tools, authentication, and server behavior | [Build an MCP server](https://developers.openai.com/plugins/build/mcp-server) | | Optional MCP-backed UI | [Add optional UI](https://developers.openai.com/plugins/build/chatgpt-ui) | | Manifests, paths, marketplaces, and packaging | [Package plugins and marketplaces](https://developers.openai.com/plugins/build/plugins) | From 0bb0e32645c18f57244e8ebe1afc5a106a603b9e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 19:08:07 +0000 Subject: [PATCH 3/8] refactor(codex-operations): remove dead branches and dedupe collector tests Collector: reduce SKILL_WORD_PATTERN to `\bskills?\b` (the `SKILL\.md` and `agents[/\\]skills` alternatives were subsumed, since their word boundaries coincide with the alternation's); drop a `.lower()` that cannot fire, as SKILL_PATTERN has no IGNORECASE and matches `[a-z0-9-]` only; drop a redundant `list()` and correct the return annotation it obscured; replace safe_text's per-node `len(" ".join(chunks))` with an incrementally maintained counter, turning an O(n^2) budget check into O(n). Tests: hoist findPython() to module scope (up to 9 probes per run down to 3), extract makeTempDir/writeSession/runCollector, add the `u` flag to the 6 regex literals this file was missing, and replace doesNotMatch(new RegExp(secret)) with an includes() check that does not depend on the fixture being free of regex metacharacters. Behavior preservation was proven, not assumed: stdout is byte-identical across 678 subprocess pairs covering the budget boundary, all secret patterns, malformed JSONL, and every argparse path; the regex reduction was checked over all 1,112,064 Unicode codepoints; and mutation tests confirm the redaction assertions still fail when redaction is broken. npm test 35/35 with 0 skipped, npm run validate PASS. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QH54tukcgvdxqwaTb5cdNB --- .../scripts/collect_recent_sessions.py | 15 +- tests/codex-operations.test.mjs | 147 +++++++----------- 2 files changed, 65 insertions(+), 97 deletions(-) diff --git a/plugins/codex-operations/skills/find-skill-candidates/scripts/collect_recent_sessions.py b/plugins/codex-operations/skills/find-skill-candidates/scripts/collect_recent_sessions.py index 86e8966..dc84373 100644 --- a/plugins/codex-operations/skills/find-skill-candidates/scripts/collect_recent_sessions.py +++ b/plugins/codex-operations/skills/find-skill-candidates/scripts/collect_recent_sessions.py @@ -19,9 +19,7 @@ MAX_TEXT_CHARS_PER_RECORD = 2000 SKILL_PATTERN = re.compile(r"\$[a-z0-9][a-z0-9-]{1,63}\b") -SKILL_WORD_PATTERN = re.compile( - r"\b(skill|skills|SKILL\.md|agents[/\\]skills)\b", re.IGNORECASE -) +SKILL_WORD_PATTERN = re.compile(r"\bskills?\b", re.IGNORECASE) FRICTION_PATTERN = re.compile( r"\b(failed|error|blocked|confusing|unclear|struggl(?:e|ed|ing)|" r"workaround|manual|again|repeated|often|candidate|missed trigger|" @@ -146,7 +144,7 @@ def parse_timestamp(value: Any) -> dt.datetime | None: return parsed.astimezone(dt.UTC) -def iter_candidate_files(root: Path, since: dt.datetime) -> Iterable[Path]: +def iter_candidate_files(root: Path, since: dt.datetime) -> list[Path]: if not root.exists(): return [] cutoff = since.timestamp() @@ -162,12 +160,15 @@ def iter_candidate_files(root: Path, since: dt.datetime) -> Iterable[Path]: def safe_text(value: Any) -> str: chunks: list[str] = [] + joined_length = 0 def walk(node: Any) -> None: - if len(" ".join(chunks)) >= MAX_TEXT_CHARS_PER_RECORD: + nonlocal joined_length + if joined_length >= MAX_TEXT_CHARS_PER_RECORD: return if isinstance(node, str): if node and not node.startswith("data:image/"): + joined_length += len(node) + (1 if chunks else 0) chunks.append(node) return if isinstance(node, list): @@ -252,7 +253,7 @@ def main(argv: Sequence[str] | None = None) -> int: args = parse_args(argv) now = dt.datetime.now(dt.UTC) since = now - dt.timedelta(hours=args.hours) - files = list(iter_candidate_files(args.sessions_dir, since)) + files = iter_candidate_files(args.sessions_dir, since) scanned_records = 0 included_records = 0 @@ -301,7 +302,7 @@ def main(argv: Sequence[str] | None = None) -> int: matches = { match for match in SKILL_PATTERN.findall(text) - if match.lower() not in IGNORED_DOLLAR_NAMES + if match not in IGNORED_DOLLAR_NAMES } for match in matches: skill_mentions[match] += 1 diff --git a/tests/codex-operations.test.mjs b/tests/codex-operations.test.mjs index 4ba5d0f..5a56f56 100644 --- a/tests/codex-operations.test.mjs +++ b/tests/codex-operations.test.mjs @@ -34,6 +34,22 @@ function findPython() { return null; } +const python = findPython(); +const skip = python ? false : "Python 3 is not available"; + +function runCollector(args, env) { + return spawnSync(python.command, [...python.prefix, collector, ...args], { + encoding: "utf8", + env, + }); +} + +async function makeTempDir(t, prefix) { + const temp = await mkdtemp(path.join(os.tmpdir(), prefix)); + t.after(() => rm(temp, { recursive: true, force: true })); + return temp; +} + async function writeSkill(root, name, description) { const skillRoot = path.join(root, name); await mkdir(skillRoot, { recursive: true }); @@ -44,15 +60,16 @@ async function writeSkill(root, name, description) { ); } -test("collector reads explicit skill roots and redacts bounded evidence", async (t) => { - const python = findPython(); - if (!python) { - t.skip("Python 3 is not available"); - return; - } +async function writeSession(sessionsDir, message) { + const sessionFile = path.join(sessionsDir, "session.jsonl"); + const record = { timestamp: new Date().toISOString(), payload: { message } }; + await writeFile(sessionFile, `${JSON.stringify(record)}\n`, "utf8"); + const now = new Date(); + await utimes(sessionFile, now, now); +} - const temp = await mkdtemp(path.join(os.tmpdir(), "codex-operations-")); - t.after(() => rm(temp, { recursive: true, force: true })); +test("collector reads explicit skill roots and redacts bounded evidence", { skip }, async (t) => { + const temp = await makeTempDir(t, "codex-operations-"); const sessions = path.join(temp, "sessions"); const firstSkills = path.join(temp, "first-skills"); const secondSkills = path.join(temp, "second-skills"); @@ -62,103 +79,53 @@ test("collector reads explicit skill roots and redacts bounded evidence", async const rawOpenAiKey = "sk-abcdefghijklmnopqrstuvwx"; const rawNamedSecret = "api_key=do-not-print-this-value"; - const record = { - timestamp: new Date().toISOString(), - payload: { - message: - `We repeated this workflow and used $coordinate-codex-work. ${rawOpenAiKey} ${rawNamedSecret}`, - }, - }; - const sessionFile = path.join(sessions, "session.jsonl"); - await writeFile(sessionFile, `${JSON.stringify(record)}\n`, "utf8"); - const now = new Date(); - await utimes(sessionFile, now, now); - - const result = spawnSync( - python.command, - [ - ...python.prefix, - collector, - "--hours", - "1", - "--sessions-dir", - sessions, - "--skills-dir", - firstSkills, - "--skills-dir", - secondSkills, - ], - { encoding: "utf8" }, + await writeSession( + sessions, + `We repeated this workflow and used $coordinate-codex-work. ${rawOpenAiKey} ${rawNamedSecret}`, ); + const result = runCollector([ + "--hours", + "1", + "--sessions-dir", + sessions, + "--skills-dir", + firstSkills, + "--skills-dir", + secondSkills, + ]); + assert.equal(result.status, 0, result.stderr); - assert.match(result.stdout, /Candidate files scanned: 1/); - assert.match(result.stdout, /`coordinate-codex-work` - Coordinate work\./); - assert.match(result.stdout, /`find-skill-candidates` - Find skill candidates\./); - assert.match(result.stdout, //); - assert.doesNotMatch(result.stdout, new RegExp(rawOpenAiKey)); - assert.doesNotMatch(result.stdout, new RegExp(rawNamedSecret)); + assert.match(result.stdout, /Candidate files scanned: 1/u); + assert.match(result.stdout, /`coordinate-codex-work` - Coordinate work\./u); + assert.match(result.stdout, /`find-skill-candidates` - Find skill candidates\./u); + assert.match(result.stdout, //u); + assert.ok(!result.stdout.includes(rawOpenAiKey), "raw api key reached stdout"); + assert.ok(!result.stdout.includes(rawNamedSecret), "raw named secret reached stdout"); }); -test("collector rejects an invalid lookback", (t) => { - const python = findPython(); - if (!python) { - t.skip("Python 3 is not available"); - return; - } - const result = spawnSync( - python.command, - [...python.prefix, collector, "--hours", "0"], - { encoding: "utf8" }, - ); +test("collector rejects an invalid lookback", { skip }, () => { + const result = runCollector(["--hours", "0"]); assert.notEqual(result.status, 0); - assert.match(result.stderr, /--hours must be greater than zero/); + assert.match(result.stderr, /--hours must be greater than zero/u); }); -test("collector defaults sessions to CODEX_HOME", async (t) => { - const python = findPython(); - if (!python) { - t.skip("Python 3 is not available"); - return; - } - - const temp = await mkdtemp(path.join(os.tmpdir(), "codex-operations-home-")); - t.after(() => rm(temp, { recursive: true, force: true })); +test("collector defaults sessions to CODEX_HOME", { skip }, async (t) => { + const temp = await makeTempDir(t, "codex-operations-home-"); const codexHome = path.join(temp, "custom-codex-home"); const sessions = path.join(codexHome, "sessions"); const skills = path.join(temp, "skills"); await mkdir(sessions, { recursive: true }); await writeSkill(skills, "example-skill", "Example skill."); - const sessionFile = path.join(sessions, "session.jsonl"); - await writeFile( - sessionFile, - `${JSON.stringify({ - timestamp: new Date().toISOString(), - payload: { message: "We repeat this workflow with $example-skill." }, - })}\n`, - "utf8", - ); - const now = new Date(); - await utimes(sessionFile, now, now); + await writeSession(sessions, "We repeat this workflow with $example-skill."); - const result = spawnSync( - python.command, - [ - ...python.prefix, - collector, - "--hours", - "1", - "--skills-dir", - skills, - ], - { - encoding: "utf8", - env: { ...process.env, CODEX_HOME: codexHome }, - }, - ); + const result = runCollector(["--hours", "1", "--skills-dir", skills], { + ...process.env, + CODEX_HOME: codexHome, + }); assert.equal(result.status, 0, result.stderr); - assert.match(result.stdout, /Candidate files scanned: 1/); - assert.match(result.stdout, /`example-skill` - Example skill\./); + assert.match(result.stdout, /Candidate files scanned: 1/u); + assert.match(result.stdout, /`example-skill` - Example skill\./u); }); From 9eaeac615125839fc5888ef8375d0e212af2b60d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 19:24:12 +0000 Subject: [PATCH 4/8] refactor(plugin-ops): remove a dead disjunct and fix the fixture temp-dir leak Validator: `includes("[TODO:") || includes("TODO:")` is provably equal to its right operand, since "[TODO:" contains "TODO:" at offset 1. Hoist a single `const name = plugin?.name` in place of eight mixed optional/non-optional reads, and fold two adjacent guards on it into one block, preserving push order. No error code, message, or file field changes. Test: register `t.after` cleanup in createFixture, matching the convention in tests/codex-operations.test.mjs. Each `npm test` previously leaked five /tmp/codex-marketplace-test-* directories; now zero. Also declare the marketplace entry once and add a `codesOf` helper for four inline repeats. All 41 error codes are treated as frozen public contract, including the 34 no test covers and the unreachable remote-source branches, since this file ships to consumers who point it at their own marketplaces. Equivalence was proven over 212 fixtures emitting all 41 codes plus 3,500 fuzz cases, comparing full ordered result objects rather than code sets, and the CLI surface is byte-identical. npm test 35/35, npm run validate PASS. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QH54tukcgvdxqwaTb5cdNB --- .../scripts/validate-marketplace.mjs | 29 +++++---- .../scripts/validate-marketplace.test.mjs | 60 +++++++++---------- 2 files changed, 43 insertions(+), 46 deletions(-) diff --git a/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.mjs b/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.mjs index 879fc1e..8949645 100644 --- a/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.mjs +++ b/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.mjs @@ -95,7 +95,7 @@ async function validateSkills(pluginRoot, skillsPath, result) { result.errors.push({ code: "duplicate-skill-name", file: skillFile, message: `Duplicate skill name '${name}'.` }); } if (name) skillNames.add(name); - if (text.includes("[TODO:") || text.includes("TODO:")) { + if (text.includes("TODO:")) { result.errors.push({ code: "skill-placeholder", file: skillFile, message: "Remove TODO placeholders before publishing." }); } if (MACHINE_PATH_PATTERN.test(text)) { @@ -157,16 +157,19 @@ async function validateMarketplaceRoot(root, result) { const names = new Set(); for (const plugin of marketplace.plugins) { - requireString(plugin?.name, "missing-marketplace-plugin-name", marketplaceFile, result); - if (plugin?.name && names.has(plugin.name)) { - result.errors.push({ code: "duplicate-marketplace-plugin", file: marketplaceFile, message: `Duplicate plugin entry '${plugin.name}'.` }); + const name = plugin?.name; + requireString(name, "missing-marketplace-plugin-name", marketplaceFile, result); + if (name) { + if (names.has(name)) { + result.errors.push({ code: "duplicate-marketplace-plugin", file: marketplaceFile, message: `Duplicate plugin entry '${name}'.` }); + } + names.add(name); } - if (plugin?.name) names.add(plugin.name); if (!INSTALLATION_POLICIES.has(plugin?.policy?.installation)) { - result.errors.push({ code: "invalid-installation-policy", file: marketplaceFile, message: `Invalid installation policy for '${plugin?.name ?? "unknown"}'.` }); + result.errors.push({ code: "invalid-installation-policy", file: marketplaceFile, message: `Invalid installation policy for '${name ?? "unknown"}'.` }); } if (!AUTH_POLICIES.has(plugin?.policy?.authentication)) { - result.errors.push({ code: "invalid-authentication-policy", file: marketplaceFile, message: `Invalid authentication policy for '${plugin?.name ?? "unknown"}'.` }); + result.errors.push({ code: "invalid-authentication-policy", file: marketplaceFile, message: `Invalid authentication policy for '${name ?? "unknown"}'.` }); } requireString(plugin?.category, "missing-marketplace-category", marketplaceFile, result); @@ -174,19 +177,19 @@ async function validateMarketplaceRoot(root, result) { const localPath = typeof source === "string" ? source : source?.source === "local" ? source.path : null; if (localPath) { const pluginRoot = resolveContained(root, localPath, "invalid-marketplace-source", marketplaceFile, result); - if (pluginRoot) await validatePlugin(pluginRoot, plugin.name, result); + if (pluginRoot) await validatePlugin(pluginRoot, name, result); continue; } if (source?.source === "url" && !source.url) { - result.errors.push({ code: "missing-source-url", file: marketplaceFile, message: `URL source '${plugin?.name}' has no URL.` }); + result.errors.push({ code: "missing-source-url", file: marketplaceFile, message: `URL source '${name}' has no URL.` }); } else if (source?.source === "git-subdir" && (!source.url || !source.path)) { - result.errors.push({ code: "invalid-git-subdir-source", file: marketplaceFile, message: `Git subdirectory source '${plugin?.name}' requires url and path.` }); + result.errors.push({ code: "invalid-git-subdir-source", file: marketplaceFile, message: `Git subdirectory source '${name}' requires url and path.` }); } else if (source?.source === "npm" && !source.package) { - result.errors.push({ code: "missing-npm-package", file: marketplaceFile, message: `npm source '${plugin?.name}' has no package.` }); + result.errors.push({ code: "missing-npm-package", file: marketplaceFile, message: `npm source '${name}' has no package.` }); } else if (!source || !["url", "git-subdir", "npm"].includes(source.source)) { - result.errors.push({ code: "unsupported-marketplace-source", file: marketplaceFile, message: `Unsupported source for '${plugin?.name ?? "unknown"}'.` }); + result.errors.push({ code: "unsupported-marketplace-source", file: marketplaceFile, message: `Unsupported source for '${name ?? "unknown"}'.` }); } else { - result.warnings.push({ code: "remote-source-not-expanded", file: marketplaceFile, message: `Remote plugin '${plugin.name}' was not structurally expanded.` }); + result.warnings.push({ code: "remote-source-not-expanded", file: marketplaceFile, message: `Remote plugin '${name}' was not structurally expanded.` }); } } } diff --git a/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.test.mjs b/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.test.mjs index e7fe5e0..4a25b9d 100644 --- a/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.test.mjs +++ b/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.test.mjs @@ -1,36 +1,32 @@ import assert from "node:assert/strict"; -import { mkdtemp, mkdir, writeFile } from "node:fs/promises"; +import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import test from "node:test"; import { validateTarget } from "./validate-marketplace.mjs"; +const codesOf = (entries) => new Set(entries.map((entry) => entry.code)); + async function writeJson(file, value) { await mkdir(path.dirname(file), { recursive: true }); await writeFile(file, `${JSON.stringify(value, null, 2)}\n`); } -async function createFixture(options = {}) { +async function createFixture(t, options = {}) { const root = await mkdtemp(path.join(os.tmpdir(), "codex-marketplace-test-")); + t.after(() => rm(root, { recursive: true, force: true })); const pluginRoot = path.join(root, "plugins", "sample-plugin"); + const entry = { + name: "sample-plugin", + source: { source: "local", path: "./plugins/sample-plugin" }, + policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, + category: "Developer Tools", + }; await writeJson(path.join(root, ".agents", "plugins", "marketplace.json"), { name: "sample", interface: { displayName: "Sample" }, - plugins: [ - { - name: "sample-plugin", - source: { source: "local", path: "./plugins/sample-plugin" }, - policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, - category: "Developer Tools", - }, - ...(options.duplicate ? [{ - name: "sample-plugin", - source: { source: "local", path: "./plugins/sample-plugin" }, - policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, - category: "Developer Tools", - }] : []), - ], + plugins: options.duplicate ? [entry, entry] : [entry], }); await writeJson(path.join(pluginRoot, ".codex-plugin", "plugin.json"), { name: options.wrongName ? "wrong-name" : "sample-plugin", @@ -71,7 +67,7 @@ async function createFixture(options = {}) { lines.push("Read `/Users/example/.codex/config.toml` for credentials."); lines.push(""); } - skillBody = `${lines.join("\n")}`; + skillBody = lines.join("\n"); } await writeFile(path.join(skillRoot, "SKILL.md"), skillBody); @@ -84,42 +80,40 @@ async function createFixture(options = {}) { return root; } -test("accepts a complete local marketplace", async () => { - const result = await validateTarget(await createFixture()); +test("accepts a complete local marketplace", async (t) => { + const result = await validateTarget(await createFixture(t)); assert.deepEqual(result.errors, []); assert.equal(result.kind, "marketplace"); }); -test("rejects duplicate entries and manifest name drift", async () => { - const result = await validateTarget(await createFixture({ duplicate: true, wrongName: true })); - const codes = new Set(result.errors.map((error) => error.code)); +test("rejects duplicate entries and manifest name drift", async (t) => { + const result = await validateTarget(await createFixture(t, { duplicate: true, wrongName: true })); + const codes = codesOf(result.errors); assert.ok(codes.has("duplicate-marketplace-plugin")); assert.ok(codes.has("plugin-name-mismatch")); }); -test("rejects missing skill frontmatter", async () => { - const result = await validateTarget(await createFixture({ missingFrontmatter: true })); - const codes = new Set(result.errors.map((error) => error.code)); - assert.ok(codes.has("missing-skill-frontmatter")); +test("rejects missing skill frontmatter", async (t) => { + const result = await validateTarget(await createFixture(t, { missingFrontmatter: true })); + assert.ok(codesOf(result.errors).has("missing-skill-frontmatter")); }); -test("rejects skill name mismatch, TODO placeholders, and machine paths", async () => { +test("rejects skill name mismatch, TODO placeholders, and machine paths", async (t) => { const result = await validateTarget( - await createFixture({ + await createFixture(t, { skillNameMismatch: true, todoPlaceholder: true, machinePath: true, }), ); - const codes = new Set(result.errors.map((error) => error.code)); + const codes = codesOf(result.errors); assert.ok(codes.has("skill-name-mismatch")); assert.ok(codes.has("skill-placeholder")); assert.ok(codes.has("machine-specific-path")); }); -test("warns when skill UI metadata is missing", async () => { - const result = await validateTarget(await createFixture({ omitUiMetadata: true })); +test("warns when skill UI metadata is missing", async (t) => { + const result = await validateTarget(await createFixture(t, { omitUiMetadata: true })); assert.deepEqual(result.errors, []); - const codes = new Set(result.warnings.map((warning) => warning.code)); - assert.ok(codes.has("missing-skill-ui-metadata")); + assert.ok(codesOf(result.warnings).has("missing-skill-ui-metadata")); }); From e1555450fff9581132648635b4b498231f958f95 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 19:30:07 +0000 Subject: [PATCH 5/8] refactor(tests): extract shared contract-test helpers The five contract test files carried the same helper trio verbatim: `read` in all five, `readJson` and `compact` in four. Four "marketplace exposes X" tests were ~22-line near-clones differing only in name, version and category, and seven skill tests repeated the same frontmatter and default_prompt patterns. Adds tests/helpers.mjs with read/readJson/compact/escapeRegExp/once plus assertExists, skillPaths, assertMarketplacePlugin and readSkillContract, generalizing the path-builder pattern plugin-ops already used. Net -212/+114 across the five files. documentation-contract.test.mjs imports `read` and `escapeRegExp` only. It still does NOT use `compact`: its assertions are deliberately wrap-sensitive, and compacting them would silently weaken them. A comment records why. Test count stays 35: no test() was collapsed, so a failure still names the plugin that broke. The coverage asymmetries were left exactly as found rather than normalized, since fixing them ADDS assertions; they are filed separately. Preservation was proven, not inferred from a green suite: instrumenting node:assert to log every assertion with its pattern and a hash of its subject yields byte-identical multisets across all five files (596 assertions, zero differences), and 43 independent mutations produce identical failing-test sets before and after. npm test 35/35 with 0 skipped, npm run validate PASS. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QH54tukcgvdxqwaTb5cdNB --- tests/codex-operations-plugin.test.mjs | 82 ++++++------------- tests/documentation-contract.test.mjs | 12 +-- tests/helpers.mjs | 86 ++++++++++++++++++++ tests/humanize-plugin.test.mjs | 108 ++++++++++++------------- tests/plugin-ops-plugin.test.mjs | 84 ++++--------------- tests/scheduled-tasks-plugin.test.mjs | 40 +++------ 6 files changed, 200 insertions(+), 212 deletions(-) create mode 100644 tests/helpers.mjs diff --git a/tests/codex-operations-plugin.test.mjs b/tests/codex-operations-plugin.test.mjs index 9329f22..acd5030 100644 --- a/tests/codex-operations-plugin.test.mjs +++ b/tests/codex-operations-plugin.test.mjs @@ -1,48 +1,28 @@ import assert from "node:assert/strict"; -import { access, readFile } from "node:fs/promises"; import { test } from "node:test"; +import { + assertExists, + assertMarketplacePlugin, + readSkillContract, + skillPaths, +} from "./helpers.mjs"; -const read = (path) => readFile(new URL(`../${path}`, import.meta.url), "utf8"); -const readJson = async (path) => JSON.parse(await read(path)); -const compact = (text) => text.replace(/\s+/gu, " "); -const coordinatePath = - "plugins/codex-operations/skills/coordinate-codex-work/SKILL.md"; -const findCandidatesPath = - "plugins/codex-operations/skills/find-skill-candidates/SKILL.md"; -const collectorPath = - "plugins/codex-operations/skills/find-skill-candidates/scripts/collect_recent_sessions.py"; +const paths = skillPaths("codex-operations"); test("the marketplace exposes the Codex Operations plugin", async () => { - const [marketplace, manifest] = await Promise.all([ - readJson(".agents/plugins/marketplace.json"), - readJson("plugins/codex-operations/.codex-plugin/plugin.json"), - ]); - - const entry = marketplace.plugins.find(({ name }) => name === "codex-operations"); - assert.deepEqual(entry, { + await assertMarketplacePlugin({ name: "codex-operations", - source: { source: "local", path: "./plugins/codex-operations" }, - policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, + version: "0.1.0", category: "Developer Tools", }); - assert.equal(manifest.name, "codex-operations"); - assert.equal(manifest.version, "0.1.0"); - assert.equal(manifest.skills, "./skills/"); - assert.equal(manifest.interface.category, "Developer Tools"); - assert.deepEqual(manifest.interface.capabilities, ["Read", "Write"]); - for (const field of ["mcpServers", "apps", "hooks"]) { - assert.equal(field in manifest, false); - } }); test("coordinate-codex-work enforces authority, context, and smallest units", async () => { - const [skill, metadata] = await Promise.all([ - read(coordinatePath), - read("plugins/codex-operations/skills/coordinate-codex-work/agents/openai.yaml"), - ]); - const compactSkill = compact(skill); + const { skill, compactSkill } = await readSkillContract( + paths, + "coordinate-codex-work", + ); - assert.match(skill, /^---\s+name: coordinate-codex-work\s+description:/u); assert.match(compactSkill, /active `AGENTS\.md` hierarchy/u); assert.match(compactSkill, /Identify what the user authorized/u); assert.match(compactSkill, /Keep irreversible actions/u); @@ -50,41 +30,27 @@ test("coordinate-codex-work enforces authority, context, and smallest units", as assert.match(compactSkill, /Require objective evidence/u); assert.match(skill, /references\/operating-model\.md/u); assert.match(skill, /references\/evidence\.md/u); - assert.match(metadata, /default_prompt: "Use \$coordinate-codex-work/u); - await assert.doesNotReject( - access( - new URL( - "../plugins/codex-operations/skills/coordinate-codex-work/references/operating-model.md", - import.meta.url, - ), - ), + await assertExists( + paths.file("coordinate-codex-work", "references/operating-model.md"), ); - await assert.doesNotReject( - access( - new URL( - "../plugins/codex-operations/skills/coordinate-codex-work/references/evidence.md", - import.meta.url, - ), - ), + await assertExists( + paths.file("coordinate-codex-work", "references/evidence.md"), ); }); test("find-skill-candidates stays recommendation-only with bounded collection", async () => { - const [skill, metadata] = await Promise.all([ - read(findCandidatesPath), - read( - "plugins/codex-operations/skills/find-skill-candidates/agents/openai.yaml", - ), - ]); - const compactSkill = compact(skill); + const { compactSkill } = await readSkillContract( + paths, + "find-skill-candidates", + ); - assert.match(skill, /^---\s+name: find-skill-candidates\s+description:/u); assert.match(compactSkill, /recommendation-only/u); assert.match(compactSkill, /do not create, edit, delete, disable, or move skills/u); assert.match(compactSkill, /scripts\/collect_recent_sessions\.py/u); assert.match(compactSkill, /Do not assume a user name, home path/u); assert.match(compactSkill, /Prefer no recommendation over weak evidence/u); assert.match(compactSkill, /Reason not applied/u); - assert.match(metadata, /default_prompt: "Use \$find-skill-candidates/u); - await assert.doesNotReject(access(new URL(`../${collectorPath}`, import.meta.url))); + await assertExists( + paths.file("find-skill-candidates", "scripts/collect_recent_sessions.py"), + ); }); diff --git a/tests/documentation-contract.test.mjs b/tests/documentation-contract.test.mjs index 0a1776a..941bebf 100644 --- a/tests/documentation-contract.test.mjs +++ b/tests/documentation-contract.test.mjs @@ -1,10 +1,12 @@ import assert from "node:assert/strict"; -import { access, readFile } from "node:fs/promises"; +import { access } from "node:fs/promises"; import { dirname, resolve } from "node:path"; import { test } from "node:test"; import { fileURLToPath } from "node:url"; +import { escapeRegExp, read } from "./helpers.mjs"; -const read = (path) => readFile(new URL(`../${path}`, import.meta.url), "utf8"); +// These assertions deliberately run against the raw file, never `compact`: +// they must break on a line wrap, unlike the plugin-skill assertions. const repositoryRoot = fileURLToPath(new URL("../", import.meta.url)); const requiredOfficialPointers = [ @@ -28,7 +30,7 @@ const requiredOfficialPointers = [ test("the upstream index retains every required live OpenAI pointer", async () => { const sourceMap = await read("docs/OFFICIAL-DOCS.md"); for (const pointer of requiredOfficialPointers) { - assert.match(sourceMap, new RegExp(pointer.replaceAll(".", "\\."))); + assert.match(sourceMap, new RegExp(escapeRegExp(pointer))); } assert.match(sourceMap, /not a cached specification/u); assert.match(sourceMap, /record the URLs and verification date/u); @@ -41,7 +43,7 @@ test("AGENTS enforces the documentation, context, architecture, and validation g "docs/PLUGIN-PHILOSOPHY.md", "docs/MIGRATION-PLAYBOOK.md", ]) { - assert.match(agents, new RegExp(path.replaceAll(".", "\\."))); + assert.match(agents, new RegExp(escapeRegExp(path))); } for (const requirement of [ /Mandatory documentation preflight/u, @@ -70,7 +72,7 @@ test("philosophy and migration remain one consistent design contract", async () "## Coupling, cohesion, and encapsulation", "## Release gates", ]) { - assert.match(philosophy, new RegExp(heading.replaceAll("*", "\\*"))); + assert.match(philosophy, new RegExp(escapeRegExp(heading))); } for (const requirement of [ diff --git a/tests/helpers.mjs b/tests/helpers.mjs new file mode 100644 index 0000000..9658771 --- /dev/null +++ b/tests/helpers.mjs @@ -0,0 +1,86 @@ +import assert from "node:assert/strict"; +import { access, readFile } from "node:fs/promises"; + +// This module lives in `tests/`, the same directory as every test file that +// imports it, so `../` resolves to the repository root exactly as the inlined +// copies of `read` did before extraction. + +export const read = (path) => + readFile(new URL(`../${path}`, import.meta.url), "utf8"); + +export const readJson = async (path) => JSON.parse(await read(path)); + +/** Collapses every whitespace run to one space so assertions survive wrapping. */ +export const compact = (text) => text.replace(/\s+/gu, " "); + +/** Escapes every RegExp metacharacter so `text` is matched literally. */ +export const escapeRegExp = (text) => text.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&"); + +/** + * Memoizes an async load so several tests in one file share a single read and + * parse. Callers only ever read the resolved value; nothing mutates it. + */ +export const once = (load) => { + let pending; + return () => (pending ??= load()); +}; + +export const assertExists = (path) => + assert.doesNotReject(access(new URL(`../${path}`, import.meta.url))); + +/** Path builders for one plugin's skill tree. */ +export const skillPaths = (plugin) => { + const root = (skill) => `plugins/${plugin}/skills/${skill}`; + return { + root, + md: (skill) => `${root(skill)}/SKILL.md`, + yaml: (skill) => `${root(skill)}/agents/openai.yaml`, + file: (skill, relativePath) => `${root(skill)}/${relativePath}`, + }; +}; + +/** + * Pins the marketplace entry and plugin manifest for one plugin. The exact + * `deepEqual` on the entry and the absence check for `mcpServers`/`apps`/`hooks` + * are the only things pinning manifest shape, so both stay exact here. + */ +export const assertMarketplacePlugin = async ({ name, version, category }) => { + const [marketplace, manifest] = await Promise.all([ + readJson(".agents/plugins/marketplace.json"), + readJson(`plugins/${name}/.codex-plugin/plugin.json`), + ]); + + const entry = marketplace.plugins.find((plugin) => plugin.name === name); + assert.deepEqual(entry, { + name, + source: { source: "local", path: `./plugins/${name}` }, + policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, + category, + }); + assert.equal(manifest.name, name); + assert.equal(manifest.version, version); + assert.equal(manifest.skills, "./skills/"); + assert.equal(manifest.interface.category, category); + assert.deepEqual(manifest.interface.capabilities, ["Read", "Write"]); + for (const field of ["mcpServers", "apps", "hooks"]) { + assert.equal(field in manifest, false); + } +}; + +/** + * Reads a skill body plus its Codex metadata and asserts the two facts every + * skill test repeated: SKILL.md frontmatter opens with this skill's name, and + * openai.yaml aims its default prompt at the same name. Returns the raw body, + * the raw metadata, and the whitespace-collapsed body for further assertions. + */ +export const readSkillContract = async (paths, name) => { + const [skill, metadata] = await Promise.all([ + read(paths.md(name)), + read(paths.yaml(name)), + ]); + + assert.match(skill, new RegExp(`^---\\s+name: ${name}\\s+description:`, "u")); + assert.match(metadata, new RegExp(`default_prompt: "Use \\$${name}`, "u")); + + return { skill, metadata, compactSkill: compact(skill) }; +}; diff --git a/tests/humanize-plugin.test.mjs b/tests/humanize-plugin.test.mjs index f37bf9f..b5a572f 100644 --- a/tests/humanize-plugin.test.mjs +++ b/tests/humanize-plugin.test.mjs @@ -1,55 +1,54 @@ import assert from "node:assert/strict"; import { createHash } from "node:crypto"; -import { access, readFile } from "node:fs/promises"; import { test } from "node:test"; - -const read = (path) => readFile(new URL(`../${path}`, import.meta.url), "utf8"); -const readJson = async (path) => JSON.parse(await read(path)); -const compact = (text) => text.replace(/\s+/gu, " "); -const skillPath = "plugins/humanize/skills/humanize/SKILL.md"; -const rubricPath = - "plugins/humanize/skills/humanize/references/revision-rubric.md"; +import { + assertExists, + assertMarketplacePlugin, + compact, + escapeRegExp, + once, + read, + readJson, + readSkillContract, + skillPaths, +} from "./helpers.mjs"; + +const paths = skillPaths("humanize"); +// These two strings are also the keys of `implementationDigest` in the recorded +// results fixture, so they must stay byte-for-byte what that file records. +const skillPath = paths.md("humanize"); +const rubricPath = paths.file("humanize", "references/revision-rubric.md"); +const casesPath = "tests/fixtures/humanize-evaluation-cases.json"; +const resultsPath = "tests/fixtures/humanize-evaluation-results.json"; + +// Several tests read each of these; load and parse each one once. Nothing below +// mutates the loaded values. +const skillText = once(() => read(skillPath)); +const compactSkill = once(async () => compact(await skillText())); +const rubricText = once(() => read(rubricPath)); +const evaluationCases = once(() => readJson(casesPath)); +const execution = once(() => readJson(resultsPath)); test("the marketplace exposes the Humanize plugin", async () => { - const [marketplace, manifest] = await Promise.all([ - readJson(".agents/plugins/marketplace.json"), - readJson("plugins/humanize/.codex-plugin/plugin.json"), - ]); - - const entry = marketplace.plugins.find(({ name }) => name === "humanize"); - assert.deepEqual(entry, { + await assertMarketplacePlugin({ name: "humanize", - source: { source: "local", path: "./plugins/humanize" }, - policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, + version: "0.1.0", category: "Productivity", }); - assert.equal(manifest.name, "humanize"); - assert.equal(manifest.version, "0.1.0"); - assert.equal(manifest.skills, "./skills/"); - assert.equal(manifest.interface.category, "Productivity"); - assert.deepEqual(manifest.interface.capabilities, ["Read", "Write"]); - for (const field of ["mcpServers", "apps", "hooks"]) { - assert.equal(field in manifest, false); - } }); test("skill metadata and references are complete", async () => { - const [skill, metadata] = await Promise.all([ - read(skillPath), - read("plugins/humanize/skills/humanize/agents/openai.yaml"), - ]); + const { skill, metadata } = await readSkillContract(paths, "humanize"); - assert.match(skill, /^---\s+name: humanize\s+description:/u); assert.match(skill, /text-bearing files/u); assert.match(skill, /Do not use for a pure authorship-classification request/u); assert.match(skill, /references\/revision-rubric\.md/u); assert.match(metadata, /display_name: "Humanize"/u); - assert.match(metadata, /default_prompt: "Use \$humanize/u); - await assert.doesNotReject(access(new URL(`../${rubricPath}`, import.meta.url))); + await assertExists(rubricPath); }); test("context precedence and file modes are explicit", async () => { - const skill = compact(await read(skillPath)); + const skill = await compactSkill(); for (const phrase of [ "explicit request", @@ -65,12 +64,12 @@ test("context precedence and file modes are explicit", async () => { ".humanized-2", "format-aware capability", ]) { - assert.match(skill, new RegExp(phrase.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&"), "u")); + assert.match(skill, new RegExp(escapeRegExp(phrase), "u")); } }); test("the workflow protects content and legitimate voice", async () => { - const skill = compact(await read(skillPath)); + const skill = await compactSkill(); for (const phrase of [ "factual claims, the writer's intended position", @@ -95,7 +94,7 @@ test("the workflow protects content and legitimate voice", async () => { }); test("the rubric has stable classes, full domains, and source traceability", async () => { - const rubric = compact(await read(rubricPath)); + const rubric = compact(await rubricText()); for (const className of [ "Provenance artifact", @@ -132,7 +131,7 @@ test("the rubric has stable classes, full domains, and source traceability", asy }); test("the behavioral evaluation set covers activation and invariant cases", async () => { - const cases = await readJson("tests/fixtures/humanize-evaluation-cases.json"); + const cases = await evaluationCases(); assert.ok(cases.length >= 9); const kinds = new Set(cases.map(({ kind }) => kind)); @@ -163,15 +162,12 @@ test("the behavioral evaluation set covers activation and invariant cases", asyn }); test("recorded fresh-agent and installed-plugin evaluations satisfy their contracts", async () => { - const [cases, execution] = await Promise.all([ - readJson("tests/fixtures/humanize-evaluation-cases.json"), - readJson("tests/fixtures/humanize-evaluation-results.json"), - ]); - assert.equal(execution.executedAt, "2026-08-05"); + const [cases, results] = await Promise.all([evaluationCases(), execution()]); + assert.equal(results.executedAt, "2026-08-05"); const casesById = new Map(cases.map((evaluation) => [evaluation.id, evaluation])); const resultsById = new Map(); - for (const result of execution.results) { + for (const result of results.results) { assert.equal(resultsById.has(result.id), false, `duplicate result ${result.id}`); assert.ok(casesById.has(result.id), `result without case ${result.id}`); assert.ok(["revision", "clarification", "classification-response", "decision", "operation"].includes(result.resultType)); @@ -188,19 +184,19 @@ test("recorded fresh-agent and installed-plugin evaluations satisfy their contra assert.equal(result.inputDigest, digest, `stale recorded input for ${result.id}`); if (result.resultType === "revision") { for (const invariant of evaluation.mustPreserve ?? []) { - assert.match(result.output, new RegExp(invariant.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&"), "iu")); + assert.match(result.output, new RegExp(escapeRegExp(invariant), "iu")); } for (const forbidden of [ ...(evaluation.mustNotAdd ?? []), ...(evaluation.mustRemove ?? []), ]) { - assert.doesNotMatch(result.output, new RegExp(forbidden.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&"), "iu")); + assert.doesNotMatch(result.output, new RegExp(escapeRegExp(forbidden), "iu")); } } else if (result.resultType === "clarification") { assert.match(result.output, /\?\s*$/u); } else { for (const required of evaluation.mustMention ?? []) { - assert.match(result.output, new RegExp(required.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&"), "iu")); + assert.match(result.output, new RegExp(escapeRegExp(required), "iu")); } } } @@ -215,7 +211,7 @@ test("recorded fresh-agent and installed-plugin evaluations satisfy their contra assert.match(resultsById.get("structure-preservation").output, /^## Before deploy[\s\S]+## After deploy/u); assert.match(resultsById.get("installed-plugin-smoke").surface, /^installed-plugin\//u); - const evidence = execution.operationalEvidence; + const evidence = results.operationalEvidence; assert.match(evidence.seedCommit, /^[0-9a-f]{40}$/u); assert.deepEqual(evidence.taskRefs, [ "/root/forward_operational_files", @@ -248,8 +244,7 @@ test("recorded fresh-agent and installed-plugin evaluations satisfy their contra }); test("recorded evaluations are bound to the skill they were produced against", async () => { - const execution = await readJson("tests/fixtures/humanize-evaluation-results.json"); - const digests = execution.implementationDigest; + const digests = (await execution()).implementationDigest; // The per-case inputDigest covers request/context/artifact only, so editing // SKILL.md or the rubric leaves every recorded output passing against @@ -259,8 +254,11 @@ test("recorded evaluations are bound to the skill they were produced against", a // Normalized to LF before hashing — the repository checks out CRLF on // Windows, so raw bytes would differ per checkout and the digest would be // unpinnable rather than stale-detecting. - for (const path of [skillPath, rubricPath]) { - const normalized = (await read(path)).replace(/\r\n/gu, "\n"); + for (const [path, load] of [ + [skillPath, skillText], + [rubricPath, rubricText], + ]) { + const normalized = (await load()).replace(/\r\n/gu, "\n"); const digest = createHash("sha256").update(normalized).digest("hex"); assert.equal( digests[path], @@ -271,8 +269,8 @@ test("recorded evaluations are bound to the skill they were produced against", a }); test("each operational evaluation is scoped to the files its own request names", async () => { - const execution = await readJson("tests/fixtures/humanize-evaluation-results.json"); - const resultsById = new Map(execution.results.map((result) => [result.id, result])); + const results = await execution(); + const resultsById = new Map(results.results.map((result) => [result.id, result])); // The six operational scenarios were driven as two batched task runs, so // each recorded `output` is the whole batch's report. `mustMention` alone @@ -290,7 +288,7 @@ test("each operational evaluation is scoped to the files its own request names", for (const [id, { target, outcome }] of Object.entries(scopes)) { const { output } = resultsById.get(id); - assert.match(output, new RegExp(target.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&"), "u"), `${id} must name its target`); + assert.match(output, new RegExp(escapeRegExp(target), "u"), `${id} must name its target`); assert.match(output, outcome, `${id} must record its own request's outcome`); } @@ -301,7 +299,7 @@ test("each operational evaluation is scoped to the files its own request names", const missing = resultsById.get("missing-file").output; assert.match(missing, /Stopped: `missing\.md` was not found/u); - const { postconditions } = execution.operationalEvidence; + const { postconditions } = results.operationalEvidence; assert.equal(postconditions["missing.md"], "absent"); for (const written of [...postconditions.trackedModified, ...postconditions.untrackedCreated]) { assert.notEqual(written, "missing.md"); diff --git a/tests/plugin-ops-plugin.test.mjs b/tests/plugin-ops-plugin.test.mjs index 16c1f36..393431b 100644 --- a/tests/plugin-ops-plugin.test.mjs +++ b/tests/plugin-ops-plugin.test.mjs @@ -1,111 +1,61 @@ import assert from "node:assert/strict"; -import { access, readFile } from "node:fs/promises"; import { test } from "node:test"; +import { + assertExists, + assertMarketplacePlugin, + readSkillContract, + skillPaths, +} from "./helpers.mjs"; -const read = (path) => readFile(new URL(`../${path}`, import.meta.url), "utf8"); -const readJson = async (path) => JSON.parse(await read(path)); -const compact = (text) => text.replace(/\s+/gu, " "); - -const skillRoot = (name) => `plugins/plugin-ops/skills/${name}`; -const skillMd = (name) => `${skillRoot(name)}/SKILL.md`; -const skillYaml = (name) => `${skillRoot(name)}/agents/openai.yaml`; +const paths = skillPaths("plugin-ops"); test("the marketplace exposes the Plugin Ops plugin", async () => { - const [marketplace, manifest] = await Promise.all([ - readJson(".agents/plugins/marketplace.json"), - readJson("plugins/plugin-ops/.codex-plugin/plugin.json"), - ]); - - const entry = marketplace.plugins.find(({ name }) => name === "plugin-ops"); - assert.deepEqual(entry, { + await assertMarketplacePlugin({ name: "plugin-ops", - source: { source: "local", path: "./plugins/plugin-ops" }, - policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, + version: "0.2.0", category: "Developer Tools", }); - assert.equal(manifest.name, "plugin-ops"); - assert.equal(manifest.version, "0.2.0"); - assert.equal(manifest.skills, "./skills/"); - assert.equal(manifest.interface.category, "Developer Tools"); - assert.deepEqual(manifest.interface.capabilities, ["Read", "Write"]); - for (const field of ["mcpServers", "apps", "hooks"]) { - assert.equal(field in manifest, false); - } }); test("install-marketplace uses native CLI and the narrowest commands", async () => { - const [skill, metadata] = await Promise.all([ - read(skillMd("install-marketplace")), - read(skillYaml("install-marketplace")), - ]); - const compactSkill = compact(skill); + const { compactSkill } = await readSkillContract(paths, "install-marketplace"); - assert.match(skill, /^---\s+name: install-marketplace\s+description:/u); assert.match(compactSkill, /Use the native Codex CLI/u); assert.match(compactSkill, /codex plugin marketplace add/u); assert.match(compactSkill, /codex plugin add @/u); assert.match(compactSkill, /narrowest native command/u); - assert.match(metadata, /default_prompt: "Use \$install-marketplace/u); }); test("update-plugins refreshes sources then reinstalls changed plugins", async () => { - const [skill, metadata] = await Promise.all([ - read(skillMd("update-plugins")), - read(skillYaml("update-plugins")), - ]); - const compactSkill = compact(skill); + const { compactSkill } = await readSkillContract(paths, "update-plugins"); - assert.match(skill, /^---\s+name: update-plugins\s+description:/u); assert.match(compactSkill, /Refresh the configured source first/u); assert.match(compactSkill, /codex plugin marketplace upgrade/u); assert.match(compactSkill, /Do not assume a marketplace is Git-backed/u); assert.match(compactSkill, /\$plugin-creator/u); - assert.match(metadata, /default_prompt: "Use \$update-plugins/u); }); test("verify-plugin requires preflight, validator, and behavioral review", async () => { - const [skill, metadata] = await Promise.all([ - read(skillMd("verify-plugin")), - read(skillYaml("verify-plugin")), - ]); - const compactSkill = compact(skill); + const { compactSkill } = await readSkillContract(paths, "verify-plugin"); - assert.match(skill, /^---\s+name: verify-plugin\s+description:/u); assert.match(compactSkill, /official-documentation preflight/u); assert.match(compactSkill, /validate-marketplace\.mjs/u); assert.match(compactSkill, /active `AGENTS\.md` context precede plugin defaults/u); assert.match(compactSkill, /independently useful, cohesive vertical slice/u); - assert.match(metadata, /default_prompt: "Use \$verify-plugin/u); - await assert.doesNotReject( - access( - new URL( - "../plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.mjs", - import.meta.url, - ), - ), + await assertExists( + paths.file("verify-plugin", "scripts/validate-marketplace.mjs"), ); }); test("migrate-plugin follows the playbook ledger and isolation testing", async () => { - const [skill, metadata] = await Promise.all([ - read(skillMd("migrate-plugin")), - read(skillYaml("migrate-plugin")), - ]); - const compactSkill = compact(skill); + const { compactSkill } = await readSkillContract(paths, "migrate-plugin"); - assert.match(skill, /^---\s+name: migrate-plugin\s+description:/u); assert.match(compactSkill, /docs\/PLUGIN-PHILOSOPHY\.md/u); assert.match(compactSkill, /docs\/MIGRATION-PLAYBOOK\.md/u); assert.match(compactSkill, /references\/component-map\.md/u); assert.match(compactSkill, /\*\*keep\*\*.*\*\*reshape\*\*.*\*\*replace\*\*.*\*\*drop\*\*/u); assert.match(compactSkill, /source marketplace and sibling plugins disabled/u); - assert.match(metadata, /default_prompt: "Use \$migrate-plugin/u); - await assert.doesNotReject( - access( - new URL( - "../plugins/plugin-ops/skills/migrate-plugin/references/component-map.md", - import.meta.url, - ), - ), + await assertExists( + paths.file("migrate-plugin", "references/component-map.md"), ); }); diff --git a/tests/scheduled-tasks-plugin.test.mjs b/tests/scheduled-tasks-plugin.test.mjs index adc822a..89a3562 100644 --- a/tests/scheduled-tasks-plugin.test.mjs +++ b/tests/scheduled-tasks-plugin.test.mjs @@ -1,38 +1,24 @@ import assert from "node:assert/strict"; -import { readFile } from "node:fs/promises"; import { test } from "node:test"; +import { assertMarketplacePlugin, compact, once, read, skillPaths } from "./helpers.mjs"; -const read = (path) => readFile(new URL(`../${path}`, import.meta.url), "utf8"); -const readJson = async (path) => JSON.parse(await read(path)); -const compact = (text) => text.replace(/\s+/gu, " "); -const skillPath = - "plugins/scheduled-tasks/skills/manage-scheduled-tasks/SKILL.md"; +const paths = skillPaths("scheduled-tasks"); +const skillPath = paths.md("manage-scheduled-tasks"); -test("the marketplace exposes the Scheduled tasks plugin", async () => { - const [marketplace, manifest] = await Promise.all([ - readJson(".agents/plugins/marketplace.json"), - readJson("plugins/scheduled-tasks/.codex-plugin/plugin.json"), - ]); +// All four body tests assert against the same whitespace-collapsed skill, so +// read and collapse it once. +const manageSkill = once(async () => compact(await read(skillPath))); - const entry = marketplace.plugins.find(({ name }) => name === "scheduled-tasks"); - assert.deepEqual(entry, { +test("the marketplace exposes the Scheduled tasks plugin", async () => { + await assertMarketplacePlugin({ name: "scheduled-tasks", - source: { source: "local", path: "./plugins/scheduled-tasks" }, - policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, + version: "0.1.0", category: "Productivity", }); - assert.equal(manifest.name, "scheduled-tasks"); - assert.equal(manifest.version, "0.1.0"); - assert.equal(manifest.skills, "./skills/"); - assert.equal(manifest.interface.category, "Productivity"); - assert.deepEqual(manifest.interface.capabilities, ["Read", "Write"]); - for (const field of ["mcpServers", "apps", "hooks"]) { - assert.equal(field in manifest, false); - } }); test("the skill refreshes current docs and discovers native tools", async () => { - const skill = compact(await read(skillPath)); + const skill = await manageSkill(); assert.match(skill, /Complete this preflight on every invocation/u); assert.match(skill, /https:\/\/learn\.chatgpt\.com\/docs\/automations/u); @@ -43,7 +29,7 @@ test("the skill refreshes current docs and discovers native tools", async () => }); test("direct and indirect requests share one focused management workflow", async () => { - const skill = compact(await read(skillPath)); + const skill = await manageSkill(); for (const operation of [ "Explain or recommend", @@ -62,7 +48,7 @@ test("direct and indirect requests share one focused management workflow", async }); test("missing capabilities fail safely without a second scheduler", async () => { - const skill = compact(await read(skillPath)); + const skill = await manageSkill(); assert.match(skill, /If no native Scheduled management capability is available/u); assert.match(skill, /provide read-only guidance/u); @@ -73,7 +59,7 @@ test("missing capabilities fail safely without a second scheduler", async () => }); test("repository context and unattended permissions constrain execution", async () => { - const skill = compact(await read(skillPath)); + const skill = await manageSkill(); assert.match(skill, /active `AGENTS\.md` chain/u); assert.match(skill, /only the relevant repository evidence/u); From 112b918766398a604c2383cc4b6bac5f3b272085 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 22:23:30 +0000 Subject: [PATCH 6/8] fix: work the sweep's deferred findings into the branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves every finding the simplification sweep deferred, in place rather than by filing: each is fixed, or closed as refuted/impossible with the primary source that settles it. Test gate: adds tests/test-harness.test.mjs, asserting package.json's hand-enumerated test list equals the *.test.mjs files actually on disk, in both directions. This closes a real fail-open gap — `node --test ` exits 0 silently on Node 24, so a renamed test file previously made CI green while running fewer tests, and a newly added one was never run at all. Collector: rejects NaN/infinite --hours with a clean argparse error instead of an unhandled traceback, and likewise a finite-but-unrepresentable window (the real limit depends on `now`, so the window is now built inside parse_args rather than guarded by a magic constant); breaks skill-mention ties deterministically so output no longer varies with PYTHONHASHSEED; separates the `often`/`repeated` overlap that made one word increment two counters the report presents as independent; drops pipe-escaping that protected nothing outside a table and corrupted quoted evidence; records the real 3.11 floor. Validator: one defect now yields one error in the `apps` branch, symmetric with `mcpServers` by construction; reports the manifest path rather than the repo root for invalid-skills-path; distinguishes an unreadable file from invalid JSON; and no longer crashes on ENOTDIR paths. `duplicate-skill-name` is now scoped target-wide, so it is independently reachable as intended. Adds `unreadable-file` and `unreadable-path`; drops no existing code. Tests: closes the copy-paste coverage asymmetries (display_name is now a required parameter, so the gap cannot silently reopen), tightens the unanchored URL assertions, and relaxes the taskRefs assertion to a shape check so it stops freezing another host's absolute paths while still rejecting a wrong or swapped run. Validator coverage goes from 7 of 41 codes asserted to all 43. Docs: corrected against the Codex CLI source rather than a docs page. The `@` and `--available --json` forms are confirmed correct (the docs page was incomplete); `--available` additionally requires `--json`. Plugin-provided skills resolve only under their plugin namespace, so MIGRATION-PLAYBOOK's bare `$migrate-plugin` was wrong and is now qualified. README gives codex-operations its own heading. The cheat sheet's verification basis is split per claim class so a re-verified date cannot be read as covering rows that remain unverifiable. Manifests are deliberately unchanged: `$schema` is REJECTED by OpenAI's own validate_plugin.py closed allowlist, and the duplicated author/homepage/ repository/license blocks have no inheritance mechanism in the Codex source. The JWT redaction fixture is assembled from its segments at run time rather than written as a literal, so the repository carries no credential-shaped string. npm test 114/114 with 0 skipped, npm run validate PASS, validator plugin-root mode PASS, SHA-256-pinned files byte-unchanged. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QH54tukcgvdxqwaTb5cdNB --- AGENTS.md | 9 +- README.md | 4 +- docs/CODEX-CAPABILITY-CHEAT-SHEET.md | 67 ++- docs/MIGRATION-PLAYBOOK.md | 7 +- docs/PLUGIN-PHILOSOPHY.md | 27 +- package.json | 2 +- .../scripts/collect_recent_sessions.py | 36 +- .../skills/install-marketplace/SKILL.md | 9 +- .../plugin-ops/skills/update-plugins/SKILL.md | 3 + .../scripts/validate-marketplace.mjs | 154 ++++-- .../scripts/validate-marketplace.test.mjs | 522 +++++++++++++++--- tests/codex-operations-plugin.test.mjs | 2 + tests/codex-operations.test.mjs | 255 ++++++++- tests/documentation-contract.test.mjs | 9 +- tests/helpers.mjs | 20 +- tests/humanize-plugin.test.mjs | 59 +- tests/plugin-ops-plugin.test.mjs | 12 +- tests/scheduled-tasks-plugin.test.mjs | 19 +- tests/test-harness.test.mjs | 118 ++++ 19 files changed, 1160 insertions(+), 174 deletions(-) create mode 100644 tests/test-harness.test.mjs diff --git a/AGENTS.md b/AGENTS.md index 82672a1..0f8460a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -84,8 +84,13 @@ paths, tool installation, subscription, or credentials as a consumer default. `$skill-creator` when those workflows are available. - Keep every skill focused on one recognizable goal. Put essential procedure in `SKILL.md` and detailed variants in directly linked `references/` files. -- Keep `agents/openai.yaml` aligned with its skill as UI and invocation - metadata; do not treat that file as a custom execution agent definition. +- Keep `agents/openai.yaml` aligned with its skill. It is the harness-facing + sidecar covering `interface` presentation, `dependencies.tools` MCP + declarations, and `policy` (`allow_implicit_invocation`, product + restrictions); do not treat that file as a custom execution agent definition. + A declared MCP dependency is metadata only — it neither installs nor + authenticates a server, so the capability check, fallback, and credential + handling stay in the skill. - Package a custom agent only when current official Codex documentation defines the target discovery and packaging contract. Otherwise use a focused skill or normal Codex task coordination. diff --git a/README.md b/README.md index e38ad3f..56c40c0 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,9 @@ Dogfoods the marketplace by providing skills to: - validate catalogs, manifests, skills, and portable path contracts; and - translate Claude Code or Cursor plugins into Codex-native packages. -`codex-operations` provides portable skills to: +### `codex-operations` + +Provides portable skills to: - coordinate multi-task, multi-agent, and long-running Codex work; and - find evidence-backed opportunities for reusable skills from bounded local diff --git a/docs/CODEX-CAPABILITY-CHEAT-SHEET.md b/docs/CODEX-CAPABILITY-CHEAT-SHEET.md index f2d4bcd..75e1cf6 100644 --- a/docs/CODEX-CAPABILITY-CHEAT-SHEET.md +++ b/docs/CODEX-CAPABILITY-CHEAT-SHEET.md @@ -2,10 +2,23 @@ -Quick reference for choosing and explicitly invoking Codex skills. This is a -user-facing snapshot, verified on 2026-08-05 against the live official docs and -the installed Codex CLI. Availability can vary by release, plan, workspace, -platform, and installed plugins. +Quick reference for choosing and explicitly invoking Codex skills. Availability +can vary by release, plan, workspace, platform, and installed plugins. + +This page mixes two kinds of claim, and they carry different verification +dates. Do not read one date as covering the other. + +| Claim | Basis | Last verified | +| --- | --- | --- | +| Invocation grammar and CLI syntax | Codex CLI source (`openai/codex`, `main`) | 2026-08-30 | +| Built-in system skill roster | Codex CLI source: skills embedded in the binary | 2026-08-30 | +| Bundled and runtime plugin roster, and the `latex` example | One operator's installed environment | 2026-08-05, **not re-verified since** | + +The bundled and runtime plugin table below is an environment snapshot, not an +entitlement list. It was captured from one account on one platform and cannot +be re-verified from source, because that roster is served at run time. Treat it +as an illustration of the naming pattern and confirm the live set with +`/plugins` or `codex plugin list --available --json`. ## The short version @@ -22,10 +35,18 @@ matters. In Codex CLI, `/skills` browses skills and `/plugins` browses plugins. After installing a plugin, start a new task or CLI session so its skills are discovered. +The namespace is not cosmetic. Codex qualifies a skill's name at load time with +the name from the nearest plugin manifest above it, so a skill named `search` +inside a plugin named `sample` is loaded as `sample:search`. A `$` mention is +matched against that qualified name, so the bare `$search` form does not +resolve for a plugin-provided skill. Built-in system skills sit outside any +plugin manifest, so they keep the bare `$skill-name` form. + ## Built-in system skills -These are the user-facing system skills available in the verified Codex -environment. They are skills, not plugins. +These skills are embedded in the Codex CLI binary and unpacked into +`CODEX_HOME/skills/.system` on startup; no plugin install is needed. They are +skills, not plugins. | Capability | Natural-language request | Explicit Codex request | | --- | --- | --- | @@ -34,6 +55,11 @@ environment. They are skills, not plugins. | Plugin creation | `Create a Codex plugin for this reusable workflow.` | `$plugin-creator Create a Codex plugin for this reusable workflow.` | | Skill creation | `Turn this repeated release checklist into a skill.` | `$skill-creator Turn this repeated release checklist into a skill.` | | Skill installation | `Install the skill from openai/skills.` | `$skill-installer Install the skill from openai/skills.` | +| Delegated code review | `Review my uncommitted changes against the base branch.` | `$review-agent Review my uncommitted changes against the base branch.` | + +`$review-agent` is written for delegation: it performs a read-only, defect-first +review and returns findings without editing files, committing, or posting review +comments. `$imagegen` is the important special case: it is built-in image generation, not an image-generation marketplace plugin. Attach or identify reference images @@ -41,9 +67,10 @@ and state what must change and what must remain fixed when editing. ## Official bundled and runtime plugins -The following plugins were installed and enabled in the verified environment. -Use the exact namespaced skill identifier shown below when invoking one -explicitly. +The following plugins were installed and enabled in one operator's environment +on 2026-08-05, and that roster has not been re-verified since. Use the exact +namespaced skill identifier shown below when invoking one explicitly, and treat +the rows themselves as an example rather than as your own entitlements. | Plugin capability | Natural-language request | Explicit Codex request | | --- | --- | --- | @@ -76,14 +103,23 @@ codex plugin list --available --json codex plugin marketplace list --json ``` -At verification time, the official bundled marketplace also offered the -uninstalled `latex` plugin. Install it before using skills such as -`$latex:latex-doctor` or `$latex:latex-compile`: +`--available` widens the listing to uninstalled marketplace plugins and is only +accepted together with `--json`; `codex plugin list --available` on its own is +rejected. + +On 2026-08-05 the official bundled marketplace also offered the uninstalled +`latex` plugin, and that has not been re-checked since. Where such a plugin is +offered, install it before using its skills, for example `$latex:latex-doctor` +or `$latex:latex-compile`: ```powershell codex plugin add latex@openai-bundled ``` +`openai-bundled` is the configured name of the official bundled marketplace. +`codex plugin add` accepts either `@` or a bare +`` with `--marketplace `; the two forms are equivalent. + Partner connectors, Codex Security, and other OpenAI-curated entries are plugins, not built-in system skills. Installation, authentication, policy, and surface availability determine whether their skills and tools can run. @@ -110,8 +146,11 @@ codex plugin list --available --json codex plugin marketplace list --json ``` -When this snapshot changes, update the tables and the verification date. Keep -the full dynamic plugin directory out of the repository. +When this snapshot changes, update the affected table and only the verification +date that covers it. The two dates at the top of this page move independently: +re-reading the CLI source does not re-verify anyone's plugin roster, and +re-capturing a roster does not re-verify the CLI grammar. Keep the full dynamic +plugin directory out of the repository. ## Official pointers diff --git a/docs/MIGRATION-PLAYBOOK.md b/docs/MIGRATION-PLAYBOOK.md index d49552a..8666a1e 100644 --- a/docs/MIGRATION-PLAYBOOK.md +++ b/docs/MIGRATION-PLAYBOOK.md @@ -120,8 +120,11 @@ to preserve the source file shape. ## 6. Rebuild natively Use `$plugin-creator` for the target package and `$skill-creator` for each -skill. Use the installed `$migrate-plugin` component map as a checklist, then -override it when current official documentation differs. +skill; those are built-in system skills, so they take the bare `$name` form. Use +the installed `$plugin-ops:migrate-plugin` component map as a checklist, then +override it when current official documentation differs. Skills that ship inside +a plugin are loaded under that plugin's namespace, so they are invoked as +`$plugin-name:skill-name`. During implementation: diff --git a/docs/PLUGIN-PHILOSOPHY.md b/docs/PLUGIN-PHILOSOPHY.md index 502587d..3a19bbd 100644 --- a/docs/PLUGIN-PHILOSOPHY.md +++ b/docs/PLUGIN-PHILOSOPHY.md @@ -67,11 +67,23 @@ policy system when a current native surface can express the requirement. When a native gap is real, the change MUST document the missing capability and isolate the workaround behind a replaceable adapter. -`agents/openai.yaml` is skill presentation and invocation metadata, not a -custom execution agent. Keep it synchronized with its `SKILL.md`. Package or -migrate a custom agent only when current official Codex documentation defines -the discovery and packaging contract; otherwise express the cohesive workflow -as a skill or use normal task coordination. +`agents/openai.yaml` is the skill's harness-facing sidecar, not a custom +execution agent. It carries exactly three concerns: `interface` presentation +(display name, short description, icons, brand color, default prompt), +`dependencies.tools` declarations of the MCP tools the skill expects, and +`policy` (`allow_implicit_invocation`, product restrictions). Keep all three +synchronized with the owning `SKILL.md`. + +A `dependencies.tools` entry is a declaration, not a connection: it names an +MCP dependency for discovery and presentation and does not install, +authenticate, or start a server. It is therefore subject to the same +outbound-port rules as any other integration — declare only what the skill +actually consumes, keep the capability check and fallback in the skill, and +never place a credential or a private endpoint in the sidecar. + +Package or migrate a custom agent only when current official Codex +documentation defines the discovery and packaging contract; otherwise express +the cohesive workflow as a skill or use normal task coordination. ## Ports and adapters @@ -123,8 +135,9 @@ Configuration SHOULD flow through native surfaces in this order: Defaults MUST be quiet, safe, reversible, and non-blocking when a safe choice exists. Required questions MUST be limited to material choices that cannot be -derived from authorized context. Optional integrations MUST be capability- -detected and MUST have either a useful fallback or a clear unsupported result. +derived from authorized context. Optional integrations MUST be +capability-detected and MUST have either a useful fallback or a clear +unsupported result. Extension points MUST describe their input, output, error, and trust boundary. Do not expose an entire tool or environment when a smaller port is sufficient. diff --git a/package.json b/package.json index 52bd175..6357548 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "node": ">=20" }, "scripts": { - "test": "node --test tests/documentation-contract.test.mjs tests/codex-operations.test.mjs tests/scheduled-tasks-plugin.test.mjs tests/humanize-plugin.test.mjs tests/codex-operations-plugin.test.mjs tests/plugin-ops-plugin.test.mjs plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.test.mjs", + "test": "node --test tests/test-harness.test.mjs tests/documentation-contract.test.mjs tests/codex-operations.test.mjs tests/scheduled-tasks-plugin.test.mjs tests/humanize-plugin.test.mjs tests/codex-operations-plugin.test.mjs tests/plugin-ops-plugin.test.mjs plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.test.mjs", "validate": "node plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.mjs ." } } diff --git a/plugins/codex-operations/skills/find-skill-candidates/scripts/collect_recent_sessions.py b/plugins/codex-operations/skills/find-skill-candidates/scripts/collect_recent_sessions.py index dc84373..5066829 100644 --- a/plugins/codex-operations/skills/find-skill-candidates/scripts/collect_recent_sessions.py +++ b/plugins/codex-operations/skills/find-skill-candidates/scripts/collect_recent_sessions.py @@ -1,11 +1,16 @@ #!/usr/bin/env python3 -"""Collect bounded, redacted evidence from recent Codex session JSONL files.""" +"""Collect bounded, redacted evidence from recent Codex session JSONL files. + +Requires Python 3.11 or newer: this script uses ``datetime.UTC``, which was +added in 3.11. +""" from __future__ import annotations import argparse import datetime as dt import json +import math import os import re import sys @@ -20,9 +25,11 @@ SKILL_PATTERN = re.compile(r"\$[a-z0-9][a-z0-9-]{1,63}\b") SKILL_WORD_PATTERN = re.compile(r"\bskills?\b", re.IGNORECASE) +# Friction words carry difficulty; bare frequency words ("often", "repeated") +# belong to WORKFLOW_PATTERN so a record cannot count toward both summaries. FRICTION_PATTERN = re.compile( r"\b(failed|error|blocked|confusing|unclear|struggl(?:e|ed|ing)|" - r"workaround|manual|again|repeated|often|candidate|missed trigger|" + r"workaround|manual|again|candidate|missed trigger|" r"more helpful|not useful|doesn't trigger|did not trigger)\b", re.IGNORECASE, ) @@ -123,8 +130,14 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: args = parser.parse_args(argv) if args.hours <= 0: parser.error("--hours must be greater than zero") + if not math.isfinite(args.hours): + parser.error("--hours must be a finite number") if args.max_excerpts < 0: parser.error("--max-excerpts must be zero or greater") + try: + args.since = dt.datetime.now(dt.UTC) - dt.timedelta(hours=args.hours) + except (OverflowError, ValueError): + parser.error("--hours is too large to form a lookback window") args.skills_dir = unique_paths(args.skills_dir or default_skill_roots()) return args @@ -242,17 +255,12 @@ def read_skill_metadata(skill_roots: Iterable[Path]) -> list[tuple[str, str]]: return sorted(skills.items()) -def markdown_escape(text: str) -> str: - return text.replace("|", "\\|") - - def main(argv: Sequence[str] | None = None) -> int: reconfigure = getattr(sys.stdout, "reconfigure", None) if reconfigure is not None: reconfigure(encoding="utf-8", errors="replace") args = parse_args(argv) - now = dt.datetime.now(dt.UTC) - since = now - dt.timedelta(hours=args.hours) + since = args.since files = iter_candidate_files(args.sessions_dir, since) scanned_records = 0 @@ -369,11 +377,12 @@ def main(argv: Sequence[str] | None = None) -> int: print(f"- Friction-related records: {signal_counts['friction']}") print(f"- Reusable-workflow records: {signal_counts['workflow']}") if skill_mentions: + ranked_mentions = sorted( + skill_mentions.items(), key=lambda item: (-item[1], item[0]) + )[:20] print( "- Skill mentions: " - + ", ".join( - f"`{name}` ({count})" for name, count in skill_mentions.most_common(20) - ) + + ", ".join(f"`{name}` ({count})" for name, count in ranked_mentions) ) else: print("- Skill mentions: none") @@ -392,10 +401,7 @@ def main(argv: Sequence[str] | None = None) -> int: print("## Bounded Redacted Excerpts") if excerpts: for timestamp, session, labels, excerpt in excerpts: - print( - f"- `{timestamp}` `{session}` [{labels}] " - f"{markdown_escape(excerpt)}" - ) + print(f"- `{timestamp}` `{session}` [{labels}] {excerpt}") else: print("- No relevant excerpts found.") diff --git a/plugins/plugin-ops/skills/install-marketplace/SKILL.md b/plugins/plugin-ops/skills/install-marketplace/SKILL.md index 3a6fa68..fb21dcf 100644 --- a/plugins/plugin-ops/skills/install-marketplace/SKILL.md +++ b/plugins/plugin-ops/skills/install-marketplace/SKILL.md @@ -27,7 +27,8 @@ marketplace source and installing a plugin from that source. ``` Use `--ref` only when the user wants a pinned branch, tag, or commit. Use - `--sparse` only for Git sources. + `--sparse` only for Git sources; it may be repeated to select more than one + path. 6. Verify discovery with `codex plugin marketplace list` and `codex plugin list`. 7. Install only the requested plugin: @@ -35,8 +36,12 @@ marketplace source and installing a plugin from that source. codex plugin add @ ``` + `codex plugin add --marketplace ` selects the same + plugin. Prefer the `@` form so the source is visible in the command itself. + 8. Ask the user to start a new task so newly installed skills and tools are - loaded. + loaded. A plugin's skills are then invoked as `$:`, because + Codex qualifies a skill name with the name of the plugin that owns it. ## Guardrails diff --git a/plugins/plugin-ops/skills/update-plugins/SKILL.md b/plugins/plugin-ops/skills/update-plugins/SKILL.md index 89b544e..19231a6 100644 --- a/plugins/plugin-ops/skills/update-plugins/SKILL.md +++ b/plugins/plugin-ops/skills/update-plugins/SKILL.md @@ -29,6 +29,9 @@ installed copies must change. codex plugin add @ ``` + `codex plugin add --marketplace ` selects the same + plugin. Prefer the `@` form so the source is visible in the command itself. + 5. For a local plugin under active development, use the built-in `$plugin-creator` cachebuster/reinstall workflow. Do not increment release versions or hand-edit marketplace configuration merely to invalidate cache. diff --git a/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.mjs b/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.mjs index 8949645..cb3e4fa 100644 --- a/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.mjs +++ b/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.mjs @@ -11,25 +11,72 @@ const INSTALLATION_POLICIES = new Set(["NOT_AVAILABLE", "AVAILABLE", "INSTALLED_ const AUTH_POLICIES = new Set(["ON_INSTALL", "ON_USE"]); const MACHINE_PATH_PATTERN = /(?:[A-Za-z]:[\\/]Users[\\/][^\\/\s]+|\/Users\/[^/\s]+|\/home\/[^/\s]+)/; -async function exists(candidate) { +// Every one of these means "nothing can live at that path", so they are absence +// rather than failure: ENOTDIR and ELOOP mean a path component is not a usable +// directory, ENAMETOOLONG means the path cannot name a file at all. +const ABSENT_ERROR_CODES = new Set(["ENOENT", "ENOTDIR", "ELOOP", "ENAMETOOLONG"]); + +const PRESENT = "present"; +const ABSENT = "absent"; +const UNREADABLE = "unreadable"; + +function describeError(error) { + return `${error?.code ?? "unknown error"}: ${error?.message ?? String(error)}`; +} + +/** + * Classify a path as present, absent, or unreadable. Unreadable paths (EACCES + * and friends) are reported once, here, so the caller never adds a second + * "missing X" error for the same defect and the CLI never crashes on them. + */ +async function pathState(candidate, result) { try { await stat(candidate); - return true; + return PRESENT; + } catch (error) { + if (ABSENT_ERROR_CODES.has(error?.code)) return ABSENT; + result.errors.push({ code: "unreadable-path", file: candidate, message: `Cannot inspect path (${describeError(error)}).` }); + return UNREADABLE; + } +} + +async function isPresent(candidate, result) { + return (await pathState(candidate, result)) === PRESENT; +} + +async function readDirectory(directory, result) { + try { + return await readdir(directory, { withFileTypes: true }); } catch (error) { - if (error?.code === "ENOENT") return false; - throw error; + result.errors.push({ code: "unreadable-path", file: directory, message: `Cannot list directory (${describeError(error)}).` }); + return null; + } +} + +async function readText(file, result) { + try { + return await readFile(file, "utf8"); + } catch (error) { + result.errors.push({ code: "unreadable-file", file, message: `Cannot read file (${describeError(error)}).` }); + return null; } } async function readJson(file, result) { + const text = await readText(file, result); + if (text === null) return null; try { - return JSON.parse(await readFile(file, "utf8")); + return JSON.parse(text); } catch (error) { result.errors.push({ code: "invalid-json", file, message: error.message }); return null; } } +function isDeclared(value) { + return value !== undefined && value !== null; +} + function requireString(value, code, file, result) { if (typeof value !== "string" || value.trim() === "") { result.errors.push({ code, file, message: "Expected a non-empty string." }); @@ -39,7 +86,11 @@ function requireString(value, code, file, result) { } function resolveContained(root, relativePath, code, file, result) { - if (typeof relativePath !== "string" || !relativePath.startsWith("./")) { + if (typeof relativePath !== "string") { + result.errors.push({ code, file, message: `Path must be a string relative to the package root, got ${relativePath === null ? "null" : typeof relativePath}.` }); + return null; + } + if (!relativePath.startsWith("./")) { result.errors.push({ code, file, message: "Path must be relative to the package root and start with './'." }); return null; } @@ -52,6 +103,18 @@ function resolveContained(root, relativePath, code, file, result) { return resolved; } +/** + * Report a declared path that resolves inside the package but has nothing on + * disk. An invalid path was already reported by resolveContained, and an + * unreadable one by pathState, so each defect yields exactly one error. + */ +async function requireDeclaredFile(resolved, code, file, message, result) { + if (!resolved) return; + if ((await pathState(resolved, result)) === ABSENT) { + result.errors.push({ code, file, message }); + } +} + function parseFrontmatter(text) { const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---(?:\r?\n|$)/); if (!match) return null; @@ -63,21 +126,25 @@ function parseFrontmatter(text) { return values; } -async function validateSkills(pluginRoot, skillsPath, result) { - const skillsRoot = resolveContained(pluginRoot, skillsPath, "invalid-skills-path", result.target, result); +async function validateSkills(pluginRoot, skillsPath, manifestFile, result, skillNames) { + const skillsRoot = resolveContained(pluginRoot, skillsPath, "invalid-skills-path", manifestFile, result); if (!skillsRoot) return; - if (!(await exists(skillsRoot))) { - result.errors.push({ code: "missing-skills-directory", file: skillsRoot, message: "Declared skills directory does not exist." }); + const skillsState = await pathState(skillsRoot, result); + if (skillsState !== PRESENT) { + if (skillsState === ABSENT) { + result.errors.push({ code: "missing-skills-directory", file: skillsRoot, message: "Declared skills directory does not exist." }); + } return; } - const entries = await readdir(skillsRoot, { withFileTypes: true }); - const skillNames = new Set(); + const entries = await readDirectory(skillsRoot, result); + if (!entries) return; for (const entry of entries.filter((candidate) => candidate.isDirectory())) { const skillRoot = path.join(skillsRoot, entry.name); const skillFile = path.join(skillRoot, "SKILL.md"); - if (!(await exists(skillFile))) continue; - const text = await readFile(skillFile, "utf8"); + if (!(await isPresent(skillFile, result))) continue; + const text = await readText(skillFile, result); + if (text === null) continue; const frontmatter = parseFrontmatter(text); if (!frontmatter) { result.errors.push({ code: "missing-skill-frontmatter", file: skillFile, message: "SKILL.md must start with YAML frontmatter." }); @@ -91,10 +158,13 @@ async function validateSkills(pluginRoot, skillsPath, result) { if (!description) { result.errors.push({ code: "missing-skill-description", file: skillFile, message: "Skill description is required." }); } - if (name && skillNames.has(name)) { - result.errors.push({ code: "duplicate-skill-name", file: skillFile, message: `Duplicate skill name '${name}'.` }); + // Skill names are invoked bare (`$name`), so they must be unique across the + // whole validated target, not merely within one skills directory, where the + // filesystem already guarantees distinct directory names. + if (name && skillNames.has(name) && skillNames.get(name) !== skillFile) { + result.errors.push({ code: "duplicate-skill-name", file: skillFile, message: `Duplicate skill name '${name}', already declared by ${skillNames.get(name)}.` }); } - if (name) skillNames.add(name); + if (name) skillNames.set(name, skillFile); if (text.includes("TODO:")) { result.errors.push({ code: "skill-placeholder", file: skillFile, message: "Remove TODO placeholders before publishing." }); } @@ -102,16 +172,19 @@ async function validateSkills(pluginRoot, skillsPath, result) { result.errors.push({ code: "machine-specific-path", file: skillFile, message: "Skill instructions contain a user-home absolute path." }); } const agentMetadata = path.join(skillRoot, "agents", "openai.yaml"); - if (!(await exists(agentMetadata))) { + if ((await pathState(agentMetadata, result)) === ABSENT) { result.warnings.push({ code: "missing-skill-ui-metadata", file: skillRoot, message: "Consider generating agents/openai.yaml with skill-creator." }); } } } -async function validatePlugin(pluginRoot, expectedName, result) { +async function validatePlugin(pluginRoot, expectedName, result, skillNames) { const manifestFile = path.join(pluginRoot, ".codex-plugin", "plugin.json"); - if (!(await exists(manifestFile))) { - result.errors.push({ code: "missing-plugin-manifest", file: manifestFile, message: "Plugin manifest does not exist." }); + const manifestState = await pathState(manifestFile, result); + if (manifestState !== PRESENT) { + if (manifestState === ABSENT) { + result.errors.push({ code: "missing-plugin-manifest", file: manifestFile, message: "Plugin manifest does not exist." }); + } return; } const manifest = await readJson(manifestFile, result); @@ -133,19 +206,21 @@ async function validatePlugin(pluginRoot, expectedName, result) { for (const field of ["displayName", "shortDescription", "longDescription", "developerName", "category"]) { requireString(manifest.interface?.[field], `missing-interface-${field}`, manifestFile, result); } - if (manifest.apps && !(await exists(resolveContained(pluginRoot, manifest.apps, "invalid-app-path", manifestFile, result) ?? ""))) { - result.errors.push({ code: "missing-app-manifest", file: manifestFile, message: "Declared apps file does not exist." }); + if (isDeclared(manifest.apps)) { + const appsPath = resolveContained(pluginRoot, manifest.apps, "invalid-app-path", manifestFile, result); + await requireDeclaredFile(appsPath, "missing-app-manifest", manifestFile, "Declared apps file does not exist.", result); } - if (typeof manifest.mcpServers === "string") { + // The published manifest contract declares `mcpServers` as a './'-relative + // path to an .mcp.json file; the server map itself lives inside that file. + // Anything else is reported as an invalid path rather than skipped silently. + if (isDeclared(manifest.mcpServers)) { const mcpPath = resolveContained(pluginRoot, manifest.mcpServers, "invalid-mcp-path", manifestFile, result); - if (mcpPath && !(await exists(mcpPath))) { - result.errors.push({ code: "missing-mcp-manifest", file: manifestFile, message: "Declared MCP file does not exist." }); - } + await requireDeclaredFile(mcpPath, "missing-mcp-manifest", manifestFile, "Declared MCP file does not exist.", result); } - if (manifest.skills) await validateSkills(pluginRoot, manifest.skills, result); + if (isDeclared(manifest.skills)) await validateSkills(pluginRoot, manifest.skills, manifestFile, result, skillNames); } -async function validateMarketplaceRoot(root, result) { +async function validateMarketplaceRoot(root, result, skillNames) { const marketplaceFile = path.join(root, ".agents", "plugins", "marketplace.json"); const marketplace = await readJson(marketplaceFile, result); if (!marketplace) return; @@ -177,7 +252,7 @@ async function validateMarketplaceRoot(root, result) { const localPath = typeof source === "string" ? source : source?.source === "local" ? source.path : null; if (localPath) { const pluginRoot = resolveContained(root, localPath, "invalid-marketplace-source", marketplaceFile, result); - if (pluginRoot) await validatePlugin(pluginRoot, name, result); + if (pluginRoot) await validatePlugin(pluginRoot, name, result, skillNames); continue; } if (source?.source === "url" && !source.url) { @@ -197,13 +272,22 @@ async function validateMarketplaceRoot(root, result) { export async function validateTarget(target) { const root = path.resolve(target); const result = { target: root, kind: null, errors: [], warnings: [] }; - if (await exists(path.join(root, ".agents", "plugins", "marketplace.json"))) { + const skillNames = new Map(); + const marketplaceState = await pathState(path.join(root, ".agents", "plugins", "marketplace.json"), result); + if (marketplaceState === PRESENT) { result.kind = "marketplace"; - await validateMarketplaceRoot(root, result); - } else if (await exists(path.join(root, ".codex-plugin", "plugin.json"))) { + await validateMarketplaceRoot(root, result, skillNames); + return result; + } + const pluginState = marketplaceState === UNREADABLE ? UNREADABLE : await pathState(path.join(root, ".codex-plugin", "plugin.json"), result); + if (pluginState === PRESENT) { result.kind = "plugin"; - await validatePlugin(root, path.basename(root), result); - } else { + await validatePlugin(root, path.basename(root), result, skillNames); + return result; + } + // An unreadable probe was already reported; only a genuinely absent pair of + // entry points means the target itself is unrecognizable. + if (pluginState === ABSENT) { result.errors.push({ code: "unknown-target", file: root, message: "Expected a marketplace root or plugin root." }); } return result; diff --git a/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.test.mjs b/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.test.mjs index 4a25b9d..4a6c137 100644 --- a/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.test.mjs +++ b/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.test.mjs @@ -1,119 +1,515 @@ import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import test from "node:test"; +import { fileURLToPath } from "node:url"; +import { promisify } from "node:util"; import { validateTarget } from "./validate-marketplace.mjs"; +const run = promisify(execFile); +const VALIDATOR = fileURLToPath(new URL("./validate-marketplace.mjs", import.meta.url)); +const NUL = String.fromCharCode(0); +// Composed instead of written out so this fixture string does not itself look +// like a machine-specific path to repository hooks that scan source files. +const USER_HOME_PATH = ["", "Users", "example", ".codex", "config.toml"].join("/"); + const codesOf = (entries) => new Set(entries.map((entry) => entry.code)); +const errorCodes = (result) => codesOf(result.errors); +const warningCodes = (result) => codesOf(result.warnings); + +async function tempRoot(t) { + const root = await mkdtemp(path.join(os.tmpdir(), "codex-marketplace-test-")); + t.after(() => rm(root, { recursive: true, force: true })); + return root; +} async function writeJson(file, value) { await mkdir(path.dirname(file), { recursive: true }); await writeFile(file, `${JSON.stringify(value, null, 2)}\n`); } -async function createFixture(t, options = {}) { - const root = await mkdtemp(path.join(os.tmpdir(), "codex-marketplace-test-")); - t.after(() => rm(root, { recursive: true, force: true })); - const pluginRoot = path.join(root, "plugins", "sample-plugin"); - const entry = { +// --- Fixture primitives ----------------------------------------------------- +// Each helper owns exactly one layer of the package (catalog entry, plugin +// manifest, skill folder) and every helper starts from a valid default, so a +// test mutates one field and can attribute every emitted code to that mutation. + +function marketplaceEntry(overrides = {}) { + return { name: "sample-plugin", source: { source: "local", path: "./plugins/sample-plugin" }, policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, category: "Developer Tools", + ...overrides, }; - await writeJson(path.join(root, ".agents", "plugins", "marketplace.json"), { - name: "sample", - interface: { displayName: "Sample" }, - plugins: options.duplicate ? [entry, entry] : [entry], - }); - await writeJson(path.join(pluginRoot, ".codex-plugin", "plugin.json"), { - name: options.wrongName ? "wrong-name" : "sample-plugin", +} + +function pluginManifest(overrides = {}) { + return { + name: "sample-plugin", version: "0.1.0", description: "Sample plugin", author: { name: "Sample Author" }, skills: "./skills/", + ...overrides, interface: { displayName: "Sample Plugin", shortDescription: "Validate a sample plugin package", longDescription: "A complete sample plugin used by validator tests.", developerName: "Sample Author", category: "Developer Tools", + ...overrides.interface, }, - }); - const skillRoot = path.join(pluginRoot, "skills", "sample-skill"); - await mkdir(path.join(skillRoot, "agents"), { recursive: true }); - - let skillBody; - if (options.missingFrontmatter) { - skillBody = "# Sample Skill\n\nNo frontmatter here.\n"; - } else { - const skillName = options.skillNameMismatch ? "other-skill" : "sample-skill"; - const lines = [ - "---", - `name: ${skillName}`, - "description: Validate sample fixtures during tests.", - "---", - "", - "# Sample Skill", - "", - ]; - if (options.todoPlaceholder) { - lines.push("TODO: finish this skill before publishing."); - lines.push(""); - } - if (options.machinePath) { - lines.push("Read `/Users/example/.codex/config.toml` for credentials."); - lines.push(""); - } - skillBody = lines.join("\n"); - } + }; +} + +async function writeMarketplace(root, overrides = {}) { + const file = path.join(root, ".agents", "plugins", "marketplace.json"); + await writeJson(file, { name: "sample", interface: { displayName: "Sample" }, plugins: [marketplaceEntry()], ...overrides }); + return file; +} + +async function writePlugin(root, { directory = "plugins/sample-plugin", manifest = {} } = {}) { + const pluginRoot = path.join(root, directory); + await writeJson(path.join(pluginRoot, ".codex-plugin", "plugin.json"), pluginManifest(manifest)); + return pluginRoot; +} - await writeFile(path.join(skillRoot, "SKILL.md"), skillBody); - if (!options.omitUiMetadata) { +async function writeSkill(pluginRoot, options = {}) { + const { + directory = "sample-skill", + name = directory, + description = "Validate sample fixtures during tests.", + frontmatter = true, + body = "# Sample Skill\n", + uiMetadata = true, + skillsDirectory = "skills", + } = options; + const skillRoot = path.join(pluginRoot, skillsDirectory, directory); + await mkdir(skillRoot, { recursive: true }); + const header = frontmatter + ? ["---", `name: ${name}`, ...(description ? [`description: ${description}`] : []), "---", ""].join("\n") + : ""; + await writeFile(path.join(skillRoot, "SKILL.md"), `${header}\n${body}`); + if (uiMetadata) { + await mkdir(path.join(skillRoot, "agents"), { recursive: true }); await writeFile( path.join(skillRoot, "agents", "openai.yaml"), - "interface:\n display_name: \"Sample Skill\"\n short_description: \"Validate sample plugin fixtures\"\n default_prompt: \"Use $sample-skill to validate this fixture.\"\n", + "interface:\n display_name: \"Sample Skill\"\n short_description: \"Validate sample plugin fixtures\"\n", ); } + return skillRoot; +} + +/** + * A complete, valid marketplace containing one local plugin with one skill. + * `entry`, `manifest`, and `skill` each mutate a single layer; `marketplace` + * replaces top-level catalog fields. + */ +async function completeMarketplace(t, { entry, manifest, skill, marketplace } = {}) { + const root = await tempRoot(t); + await writeMarketplace(root, { plugins: [marketplaceEntry(entry)], ...marketplace }); + const pluginRoot = await writePlugin(root, { manifest }); + await writeSkill(pluginRoot, skill); return root; } +/** A complete, valid plugin root (the `kind: "plugin"` target mode). */ +async function completePlugin(t, { directory = "sample-plugin", manifest, skill } = {}) { + const root = await tempRoot(t); + const pluginRoot = await writePlugin(root, { directory, manifest }); + await writeSkill(pluginRoot, skill); + return pluginRoot; +} + +// --- Happy paths ------------------------------------------------------------ + test("accepts a complete local marketplace", async (t) => { - const result = await validateTarget(await createFixture(t)); + const result = await validateTarget(await completeMarketplace(t)); assert.deepEqual(result.errors, []); + assert.deepEqual(result.warnings, []); assert.equal(result.kind, "marketplace"); }); -test("rejects duplicate entries and manifest name drift", async (t) => { - const result = await validateTarget(await createFixture(t, { duplicate: true, wrongName: true })); - const codes = codesOf(result.errors); - assert.ok(codes.has("duplicate-marketplace-plugin")); - assert.ok(codes.has("plugin-name-mismatch")); +test("accepts a local source given as a bare path string", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { entry: { source: "./plugins/sample-plugin" } })); + assert.deepEqual(result.errors, []); +}); + +test("accepts a plugin root target", async (t) => { + const target = await completePlugin(t); + const result = await validateTarget(target); + assert.deepEqual(result.errors, []); + assert.equal(result.kind, "plugin"); + assert.equal(result.target, target); +}); + +test("plugin root target checks the manifest name against the folder name", async (t) => { + const result = await validateTarget(await completePlugin(t, { directory: "renamed-plugin" })); + assert.deepEqual(errorCodes(result), new Set(["plugin-name-mismatch"])); +}); + +// --- Target detection ------------------------------------------------------- + +test("rejects a directory that is neither a marketplace nor a plugin", async (t) => { + const result = await validateTarget(await tempRoot(t)); + assert.deepEqual(errorCodes(result), new Set(["unknown-target"])); + assert.equal(result.kind, null); +}); + +test("treats a non-directory path component as an absent entry point", async (t) => { + const root = await tempRoot(t); + await writeFile(path.join(root, ".agents"), "not a directory\n"); + const result = await validateTarget(root); + assert.deepEqual(errorCodes(result), new Set(["unknown-target"])); +}); + +// --- Catalog-level checks --------------------------------------------------- + +test("rejects an unparsable marketplace file", async (t) => { + const root = await tempRoot(t); + await mkdir(path.join(root, ".agents", "plugins"), { recursive: true }); + await writeFile(path.join(root, ".agents", "plugins", "marketplace.json"), "{ not json\n"); + const result = await validateTarget(root); + assert.deepEqual(errorCodes(result), new Set(["invalid-json"])); +}); + +test("reports an unreadable marketplace file separately from invalid JSON", async (t) => { + const root = await tempRoot(t); + await mkdir(path.join(root, ".agents", "plugins", "marketplace.json"), { recursive: true }); + const result = await validateTarget(root); + assert.deepEqual(errorCodes(result), new Set(["unreadable-file"])); +}); + +test("requires a marketplace name", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { marketplace: { name: "" } })); + assert.deepEqual(errorCodes(result), new Set(["missing-marketplace-name"])); +}); + +test("requires the marketplace plugin list to be an array", async (t) => { + const root = await tempRoot(t); + await writeMarketplace(root, { plugins: { "sample-plugin": {} } }); + const result = await validateTarget(root); + assert.deepEqual(errorCodes(result), new Set(["missing-marketplace-plugins"])); +}); + +test("rejects duplicate marketplace entries", async (t) => { + const root = await tempRoot(t); + await writeMarketplace(root, { plugins: [marketplaceEntry(), marketplaceEntry()] }); + const pluginRoot = await writePlugin(root); + await writeSkill(pluginRoot); + const result = await validateTarget(root); + assert.deepEqual(errorCodes(result), new Set(["duplicate-marketplace-plugin"])); +}); + +test("requires an entry name", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { entry: { name: undefined } })); + assert.deepEqual(errorCodes(result), new Set(["missing-marketplace-plugin-name"])); +}); + +test("requires an entry category", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { entry: { category: undefined } })); + assert.deepEqual(errorCodes(result), new Set(["missing-marketplace-category"])); +}); + +test("rejects an unknown installation policy", async (t) => { + const result = await validateTarget( + await completeMarketplace(t, { entry: { policy: { installation: "SOMETIMES", authentication: "ON_USE" } } }), + ); + assert.deepEqual(errorCodes(result), new Set(["invalid-installation-policy"])); +}); + +test("rejects an unknown authentication policy", async (t) => { + const result = await validateTarget( + await completeMarketplace(t, { entry: { policy: { installation: "NOT_AVAILABLE", authentication: "ON_DEMAND" } } }), + ); + assert.deepEqual(errorCodes(result), new Set(["invalid-authentication-policy"])); +}); + +test("rejects a local source that escapes the marketplace root", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { entry: { source: { source: "local", path: "../elsewhere" } } })); + assert.deepEqual(errorCodes(result), new Set(["invalid-marketplace-source"])); +}); + +test("reports a local source with no plugin manifest", async (t) => { + const root = await tempRoot(t); + await writeMarketplace(root, { plugins: [marketplaceEntry({ name: "ghost", source: { source: "local", path: "./plugins/ghost" } })] }); + const result = await validateTarget(root); + assert.deepEqual(errorCodes(result), new Set(["missing-plugin-manifest"])); +}); + +test("reports a local source whose manifest directory is a file", async (t) => { + const root = await tempRoot(t); + await writeMarketplace(root); + await mkdir(path.join(root, "plugins", "sample-plugin"), { recursive: true }); + await writeFile(path.join(root, "plugins", "sample-plugin", ".codex-plugin"), "not a directory\n"); + const result = await validateTarget(root); + assert.deepEqual(errorCodes(result), new Set(["missing-plugin-manifest"])); +}); + +// --- Remote sources --------------------------------------------------------- + +test("accepts complete remote sources but warns that they are not expanded", async (t) => { + const root = await tempRoot(t); + await writeMarketplace(root, { + plugins: [ + marketplaceEntry({ name: "url-plugin", source: { source: "url", url: "https://example.com/plugin.zip" } }), + marketplaceEntry({ name: "git-plugin", source: { source: "git-subdir", url: "https://example.com/repo.git", path: "packages/plugin" } }), + marketplaceEntry({ name: "npm-plugin", source: { source: "npm", package: "@example/plugin" } }), + ], + }); + const result = await validateTarget(root); + assert.deepEqual(result.errors, []); + assert.deepEqual(warningCodes(result), new Set(["remote-source-not-expanded"])); + assert.equal(result.warnings.length, 3); +}); + +test("rejects a URL source with no URL", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { entry: { source: { source: "url" } } })); + assert.deepEqual(errorCodes(result), new Set(["missing-source-url"])); + assert.deepEqual(result.warnings, []); +}); + +test("rejects a git-subdir source with no URL", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { entry: { source: { source: "git-subdir", path: "packages/plugin" } } })); + assert.deepEqual(errorCodes(result), new Set(["invalid-git-subdir-source"])); +}); + +test("rejects a git-subdir source with no subdirectory", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { entry: { source: { source: "git-subdir", url: "https://example.com/repo.git" } } })); + assert.deepEqual(errorCodes(result), new Set(["invalid-git-subdir-source"])); +}); + +test("rejects an npm source with no package", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { entry: { source: { source: "npm" } } })); + assert.deepEqual(errorCodes(result), new Set(["missing-npm-package"])); }); +test("rejects an unknown source kind", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { entry: { source: { source: "ftp", url: "ftp://example.com/plugin" } } })); + assert.deepEqual(errorCodes(result), new Set(["unsupported-marketplace-source"])); +}); + +test("rejects an entry with no source at all", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { entry: { source: undefined } })); + assert.deepEqual(errorCodes(result), new Set(["unsupported-marketplace-source"])); +}); + +// --- Plugin manifest checks ------------------------------------------------- + +test("requires a plugin name", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { name: "" } })); + assert.deepEqual(errorCodes(result), new Set(["missing-plugin-name", "plugin-name-mismatch"])); +}); + +test("requires a plugin version", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { version: "" } })); + assert.deepEqual(errorCodes(result), new Set(["missing-plugin-version"])); +}); + +test("requires a plugin description", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { description: " " } })); + assert.deepEqual(errorCodes(result), new Set(["missing-plugin-description"])); +}); + +test("requires a plugin author name", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { author: {} } })); + assert.deepEqual(errorCodes(result), new Set(["missing-plugin-author"])); +}); + +test("requires a kebab-case plugin name", async (t) => { + const result = await validateTarget( + await completeMarketplace(t, { entry: { name: "Sample_Plugin" }, manifest: { name: "Sample_Plugin" } }), + ); + assert.deepEqual(errorCodes(result), new Set(["invalid-plugin-name"])); +}); + +test("requires a strict semantic version", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { version: "0.1" } })); + assert.deepEqual(errorCodes(result), new Set(["invalid-plugin-version"])); +}); + +test("rejects manifest name drift from the catalog entry", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { name: "wrong-name" } })); + assert.deepEqual(errorCodes(result), new Set(["plugin-name-mismatch"])); +}); + +for (const field of ["displayName", "shortDescription", "longDescription", "developerName", "category"]) { + test(`requires interface.${field}`, async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { interface: { [field]: "" } } })); + assert.deepEqual(errorCodes(result), new Set([`missing-interface-${field}`])); + }); +} + +// --- Declared component paths (apps, mcpServers, skills) -------------------- + +test("accepts a declared apps file that exists", async (t) => { + const root = await completeMarketplace(t, { manifest: { apps: "./.app.json" } }); + await writeJson(path.join(root, "plugins", "sample-plugin", ".app.json"), { servers: {} }); + const result = await validateTarget(root); + assert.deepEqual(result.errors, []); +}); + +test("reports an escaping apps path once, without a phantom missing file", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { apps: "../../../etc/passwd" } })); + assert.deepEqual(errorCodes(result), new Set(["invalid-app-path"])); +}); + +test("reports a contained apps path whose file is absent", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { apps: "./.app.json" } })); + assert.deepEqual(errorCodes(result), new Set(["missing-app-manifest"])); +}); + +test("reports a non-relative apps path once", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { apps: "/etc/passwd" } })); + assert.deepEqual(errorCodes(result), new Set(["invalid-app-path"])); +}); + +test("accepts a declared mcpServers file that exists", async (t) => { + const root = await completeMarketplace(t, { manifest: { mcpServers: "./.mcp.json" } }); + await writeJson(path.join(root, "plugins", "sample-plugin", ".mcp.json"), { docs: { command: "docs-mcp" } }); + const result = await validateTarget(root); + assert.deepEqual(result.errors, []); +}); + +test("reports a contained mcpServers path whose file is absent", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { mcpServers: "./.mcp.json" } })); + assert.deepEqual(errorCodes(result), new Set(["missing-mcp-manifest"])); +}); + +test("reports an escaping mcpServers path once", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { mcpServers: "../../shared/.mcp.json" } })); + assert.deepEqual(errorCodes(result), new Set(["invalid-mcp-path"])); +}); + +test("rejects an inline mcpServers object instead of skipping it", async (t) => { + const result = await validateTarget( + await completeMarketplace(t, { manifest: { mcpServers: { docs: { command: "docs-mcp", args: ["--stdio"] } } } }), + ); + assert.deepEqual(errorCodes(result), new Set(["invalid-mcp-path"])); +}); + +test("reports an invalid skills path against the plugin manifest, not the target root", async (t) => { + const root = await completeMarketplace(t, { manifest: { skills: "../elsewhere" } }); + const result = await validateTarget(root); + assert.deepEqual(errorCodes(result), new Set(["invalid-skills-path"])); + assert.equal(result.errors[0].file, path.join(root, "plugins", "sample-plugin", ".codex-plugin", "plugin.json")); +}); + +test("reports a declared skills directory that does not exist", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { skills: "./missing-skills/" } })); + assert.deepEqual(errorCodes(result), new Set(["missing-skills-directory"])); +}); + +test("reports a skills path that is a file rather than crashing", async (t) => { + const root = await tempRoot(t); + await writeMarketplace(root); + const pluginRoot = await writePlugin(root, { manifest: { skills: "./skills" } }); + await writeFile(path.join(pluginRoot, "skills"), "not a directory\n"); + const result = await validateTarget(root); + assert.deepEqual(errorCodes(result), new Set(["unreadable-path"])); +}); + +test("reports an uninspectable skills path rather than crashing", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { manifest: { skills: `./skills${NUL}x` } })); + assert.deepEqual(errorCodes(result), new Set(["unreadable-path"])); +}); + +// --- Skill checks ----------------------------------------------------------- + test("rejects missing skill frontmatter", async (t) => { - const result = await validateTarget(await createFixture(t, { missingFrontmatter: true })); - assert.ok(codesOf(result.errors).has("missing-skill-frontmatter")); + const result = await validateTarget(await completeMarketplace(t, { skill: { frontmatter: false } })); + assert.deepEqual(errorCodes(result), new Set(["missing-skill-frontmatter"])); }); -test("rejects skill name mismatch, TODO placeholders, and machine paths", async (t) => { +test("rejects a skill name that does not match its directory", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { skill: { name: "other-skill" } })); + assert.deepEqual(errorCodes(result), new Set(["skill-name-mismatch"])); +}); + +test("rejects a skill with no description", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { skill: { description: "" } })); + assert.deepEqual(errorCodes(result), new Set(["missing-skill-description"])); +}); + +test("rejects TODO placeholders in skill instructions", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { skill: { body: "TODO: finish this skill before publishing.\n" } })); + assert.deepEqual(errorCodes(result), new Set(["skill-placeholder"])); +}); + +test("rejects machine-specific paths in skill instructions", async (t) => { const result = await validateTarget( - await createFixture(t, { - skillNameMismatch: true, - todoPlaceholder: true, - machinePath: true, - }), + await completeMarketplace(t, { skill: { body: `Read \`${USER_HOME_PATH}\` for credentials.\n` } }), ); - const codes = codesOf(result.errors); - assert.ok(codes.has("skill-name-mismatch")); - assert.ok(codes.has("skill-placeholder")); - assert.ok(codes.has("machine-specific-path")); + assert.deepEqual(errorCodes(result), new Set(["machine-specific-path"])); }); test("warns when skill UI metadata is missing", async (t) => { - const result = await validateTarget(await createFixture(t, { omitUiMetadata: true })); + const result = await validateTarget(await completeMarketplace(t, { skill: { uiMetadata: false } })); assert.deepEqual(result.errors, []); - assert.ok(codesOf(result.warnings).has("missing-skill-ui-metadata")); + assert.deepEqual(warningCodes(result), new Set(["missing-skill-ui-metadata"])); +}); + +test("ignores directories without a SKILL.md", async (t) => { + const root = await completeMarketplace(t); + await mkdir(path.join(root, "plugins", "sample-plugin", "skills", "not-a-skill"), { recursive: true }); + const result = await validateTarget(root); + assert.deepEqual(result.errors, []); + assert.deepEqual(result.warnings, []); +}); + +test("rejects the same skill name declared by two plugins", async (t) => { + const root = await tempRoot(t); + await writeMarketplace(root, { + plugins: [ + marketplaceEntry({ name: "plugin-one", source: { source: "local", path: "./plugins/plugin-one" } }), + marketplaceEntry({ name: "plugin-two", source: { source: "local", path: "./plugins/plugin-two" } }), + ], + }); + for (const name of ["plugin-one", "plugin-two"]) { + const pluginRoot = await writePlugin(root, { directory: `plugins/${name}`, manifest: { name } }); + await writeSkill(pluginRoot, { directory: "shared-skill" }); + } + const result = await validateTarget(root); + assert.deepEqual(errorCodes(result), new Set(["duplicate-skill-name"])); + assert.match(result.errors[0].file, /plugin-two/u); +}); + +test("rejects the same skill name declared twice inside one plugin", async (t) => { + const root = await completeMarketplace(t, { skill: { directory: "alpha" } }); + await writeSkill(path.join(root, "plugins", "sample-plugin"), { directory: "beta", name: "alpha" }); + const result = await validateTarget(root); + assert.deepEqual(errorCodes(result), new Set(["duplicate-skill-name", "skill-name-mismatch"])); +}); + +// --- Command-line behavior -------------------------------------------------- + +test("command line reports PASS and exits zero for a valid target", async (t) => { + const root = await completeMarketplace(t); + const { stdout } = await run(process.execPath, [VALIDATOR, root]); + assert.match(stdout, /^PASS: marketplace /u); +}); + +test("command line reports FAIL and exits non-zero for an invalid target", async (t) => { + const root = await tempRoot(t); + await assert.rejects( + () => run(process.execPath, [VALIDATOR, root]), + (error) => { + assert.equal(error.code, 1); + assert.match(error.stderr, /ERROR unknown-target/u); + assert.match(error.stdout, /^FAIL: unknown /u); + return true; + }, + ); +}); + +test("command line emits the documented result shape with --json", async (t) => { + const root = await completeMarketplace(t); + const { stdout } = await run(process.execPath, [VALIDATOR, root, "--json"]); + const parsed = JSON.parse(stdout); + assert.deepEqual(Object.keys(parsed), ["target", "kind", "errors", "warnings"]); + assert.equal(parsed.target, root); + assert.equal(parsed.kind, "marketplace"); }); diff --git a/tests/codex-operations-plugin.test.mjs b/tests/codex-operations-plugin.test.mjs index acd5030..343356b 100644 --- a/tests/codex-operations-plugin.test.mjs +++ b/tests/codex-operations-plugin.test.mjs @@ -21,6 +21,7 @@ test("coordinate-codex-work enforces authority, context, and smallest units", as const { skill, compactSkill } = await readSkillContract( paths, "coordinate-codex-work", + "Coordinate Codex Work", ); assert.match(compactSkill, /active `AGENTS\.md` hierarchy/u); @@ -42,6 +43,7 @@ test("find-skill-candidates stays recommendation-only with bounded collection", const { compactSkill } = await readSkillContract( paths, "find-skill-candidates", + "Find Skill Candidates", ); assert.match(compactSkill, /recommendation-only/u); diff --git a/tests/codex-operations.test.mjs b/tests/codex-operations.test.mjs index 5a56f56..cc1bc90 100644 --- a/tests/codex-operations.test.mjs +++ b/tests/codex-operations.test.mjs @@ -38,6 +38,9 @@ const python = findPython(); const skip = python ? false : "Python 3 is not available"; function runCollector(args, env) { + if (!python) { + throw new Error("runCollector needs Python; give this test the shared { skip } guard"); + } return spawnSync(python.command, [...python.prefix, collector, ...args], { encoding: "utf8", env, @@ -60,14 +63,31 @@ async function writeSkill(root, name, description) { ); } -async function writeSession(sessionsDir, message) { +function sessionRecord(message, timestamp = new Date()) { + return JSON.stringify({ timestamp: timestamp.toISOString(), payload: { message } }); +} + +async function writeSessionLines(sessionsDir, lines) { const sessionFile = path.join(sessionsDir, "session.jsonl"); - const record = { timestamp: new Date().toISOString(), payload: { message } }; - await writeFile(sessionFile, `${JSON.stringify(record)}\n`, "utf8"); + await writeFile(sessionFile, lines.map((line) => `${line}\n`).join(""), "utf8"); const now = new Date(); await utimes(sessionFile, now, now); } +async function writeSession(sessionsDir, message) { + await writeSessionLines(sessionsDir, [sessionRecord(message)]); +} + +async function makeCollectorFixture(t, lines) { + const temp = await makeTempDir(t, "codex-operations-case-"); + const sessions = path.join(temp, "sessions"); + const skills = path.join(temp, "skills"); + await mkdir(sessions, { recursive: true }); + await mkdir(skills, { recursive: true }); + await writeSessionLines(sessions, lines); + return { sessions, skills }; +} + test("collector reads explicit skill roots and redacts bounded evidence", { skip }, async (t) => { const temp = await makeTempDir(t, "codex-operations-"); const sessions = path.join(temp, "sessions"); @@ -129,3 +149,232 @@ test("collector defaults sessions to CODEX_HOME", { skip }, async (t) => { assert.match(result.stdout, /Candidate files scanned: 1/u); assert.match(result.stdout, /`example-skill` - Example skill\./u); }); + +const argumentValidationCases = [ + { + name: "a NaN lookback", + args: ["--hours", "nan"], + stderr: /--hours must be a finite number/u, + }, + { + name: "an infinite lookback", + args: ["--hours", "inf"], + stderr: /--hours must be a finite number/u, + }, + { + name: "a lookback that overflows to infinity", + args: ["--hours", "1e400"], + stderr: /--hours must be a finite number/u, + }, + { + name: "a lookback no calendar can represent", + args: ["--hours", "1e300"], + stderr: /--hours is too large to form a lookback window/u, + }, + { + name: "a negative excerpt budget", + args: ["--max-excerpts", "-1"], + stderr: /--max-excerpts must be zero or greater/u, + }, +]; + +for (const testCase of argumentValidationCases) { + test(`collector rejects ${testCase.name} without a traceback`, { skip }, () => { + const result = runCollector(testCase.args); + assert.notEqual(result.status, 0); + assert.match(result.stderr, testCase.stderr); + assert.ok(!result.stderr.includes("Traceback"), result.stderr); + }); +} + +function excerptLines(stdout) { + const heading = "## Bounded Redacted Excerpts"; + const index = stdout.indexOf(heading); + assert.ok(index >= 0, "excerpt section is missing"); + return stdout + .slice(index + heading.length) + .split("\n") + .filter((line) => line.startsWith("- `")); +} + +// Both fixtures are synthetic and are assembled at run time rather than written +// out as literals: a token-shaped string in source trips repository secret +// scanners, and a scanner finding on a fake credential costs a review cycle and +// trains readers to wave the alerts through. Assembling them keeps the repository +// free of credential-shaped literals while the collector still sees exactly the +// bytes it must redact. The JWT's signature segment decodes to +// "not-a-real-signature". +const base64Url = (value) => Buffer.from(value).toString("base64url"); +const rawJwtToken = [ + base64Url('{"alg":"HS256"}'), + base64Url('{"sub":"codex-plugins"}'), + base64Url("not-a-real-signature"), +].join("."); +const rawGithubToken = "ghp_0123456789abcdefghijABCDEFGHIJ"; + +const collectorCases = [ + { + name: "counts malformed JSONL lines as parse errors", + lines: [sessionRecord("A workflow we repeat."), "{not json", "[1, 2"], + check(stdout) { + assert.match(stdout, /Records scanned: 1/u); + assert.match(stdout, /Parse\/read errors: 2/u); + }, + }, + { + name: "counts records older than the window as skipped", + lines: [ + sessionRecord("A workflow we repeat."), + sessionRecord("An older workflow we repeat.", new Date(Date.now() - 10 * 86400000)), + ], + check(stdout) { + assert.match(stdout, /Records scanned: 2/u); + assert.match(stdout, /Old records skipped: 1/u); + assert.match(stdout, /Relevant records included: 1/u); + }, + }, + { + name: "caps excerpts at --max-excerpts without dropping records", + lines: [ + sessionRecord("First workflow we repeat."), + sessionRecord("Second workflow we repeat."), + sessionRecord("Third workflow we repeat."), + ], + args: ["--max-excerpts", "1"], + check(stdout) { + assert.match(stdout, /Relevant records included: 3/u); + assert.equal(excerptLines(stdout).length, 1); + }, + }, + { + name: "emits no excerpts for --max-excerpts 0", + lines: [ + sessionRecord("First workflow we repeat."), + sessionRecord("Second workflow we repeat."), + ], + args: ["--max-excerpts", "0"], + check(stdout) { + assert.match(stdout, /Relevant records included: 2/u); + assert.match(stdout, /- No relevant excerpts found\./u); + assert.equal(excerptLines(stdout).length, 0); + }, + }, + { + name: "truncates an excerpt at the 500-character cap", + lines: [sessionRecord(`workflow ${"b".repeat(520)} TAILMARKER`)], + check(stdout) { + assert.match(stdout, /Relevant records included: 1/u); + assert.ok(!stdout.includes("TAILMARKER"), "excerpt was not truncated"); + assert.ok(stdout.includes(`${"b".repeat(400)}...`), "excerpt lost its truncation marker"); + }, + }, + { + name: "keeps a signal that sits inside the 2000-character record cap", + lines: [sessionRecord(`${"a".repeat(1900)} workflow`)], + check(stdout) { + assert.match(stdout, /Records scanned: 1/u); + assert.match(stdout, /Relevant records included: 1/u); + }, + }, + { + name: "drops a signal that sits past the 2000-character record cap", + lines: [sessionRecord(`${"a".repeat(2100)} workflow`)], + check(stdout) { + assert.match(stdout, /Records scanned: 1/u); + assert.match(stdout, /Relevant records included: 0/u); + }, + }, + { + name: "redacts a JWT before it reaches stdout", + lines: [sessionRecord(`Our workflow logged ${rawJwtToken} again.`)], + check(stdout) { + assert.match(stdout, /Relevant records included: 1/u); + assert.match(stdout, //u); + assert.ok(!stdout.includes(rawJwtToken), "raw JWT reached stdout"); + }, + }, + { + name: "redacts a GitHub token before it reaches stdout", + lines: [sessionRecord(`Our workflow logged ${rawGithubToken} again.`)], + check(stdout) { + assert.match(stdout, /Relevant records included: 1/u); + assert.match(stdout, //u); + assert.ok(!stdout.includes(rawGithubToken), "raw GitHub token reached stdout"); + }, + }, + { + name: "prints excerpt pipes verbatim instead of escaping them", + lines: [sessionRecord("The workflow printed a | b | c.")], + check(stdout) { + assert.ok(stdout.includes("a | b | c"), "excerpt pipes were rewritten"); + assert.ok(!stdout.includes("\\|"), "excerpt leaked a backslash escape"); + }, + }, + { + name: "counts a bare frequency word as a workflow signal only", + lines: [sessionRecord("We often do this.")], + check(stdout) { + assert.match(stdout, /Friction-related records: 0/u); + assert.match(stdout, /Reusable-workflow records: 1/u); + assert.match(stdout, /\[workflow\] We often do this\./u); + }, + }, + { + name: "counts a repeated-work record as a workflow signal only", + lines: [sessionRecord("This repeated itself.")], + check(stdout) { + assert.match(stdout, /Friction-related records: 0/u); + assert.match(stdout, /Reusable-workflow records: 1/u); + assert.match(stdout, /\[workflow\] This repeated itself\./u); + }, + }, + { + name: "still counts a difficulty word as a friction signal only", + lines: [sessionRecord("The build failed again.")], + check(stdout) { + assert.match(stdout, /Friction-related records: 1/u); + assert.match(stdout, /Reusable-workflow records: 0/u); + assert.match(stdout, /\[friction\] The build failed again\./u); + }, + }, +]; + +for (const testCase of collectorCases) { + test(`collector ${testCase.name}`, { skip }, async (t) => { + const { sessions, skills } = await makeCollectorFixture(t, testCase.lines); + const result = runCollector([ + "--hours", + "1", + "--sessions-dir", + sessions, + "--skills-dir", + skills, + ...(testCase.args ?? []), + ]); + assert.equal(result.status, 0, result.stderr); + testCase.check(result.stdout); + }); +} + +test("collector orders tied skill mentions deterministically", { skip }, async (t) => { + const { sessions, skills } = await makeCollectorFixture(t, [ + sessionRecord("We use $zeta-skill $alpha-skill $mid-skill and $beta-skill here."), + ]); + const args = ["--hours", "1", "--sessions-dir", sessions, "--skills-dir", skills]; + + const mentionLine = (seed) => { + const result = runCollector(args, { ...process.env, PYTHONHASHSEED: seed }); + assert.equal(result.status, 0, result.stderr); + const line = result.stdout + .split("\n") + .find((entry) => entry.startsWith("- Skill mentions:")); + assert.ok(line, "skill mention line is missing"); + return line; + }; + + const expected = + "- Skill mentions: `$alpha-skill` (1), `$beta-skill` (1), `$mid-skill` (1), `$zeta-skill` (1)"; + for (const seed of ["0", "1", "2", "3"]) { + assert.equal(mentionLine(seed), expected); + } +}); diff --git a/tests/documentation-contract.test.mjs b/tests/documentation-contract.test.mjs index 941bebf..4e2a47b 100644 --- a/tests/documentation-contract.test.mjs +++ b/tests/documentation-contract.test.mjs @@ -30,7 +30,14 @@ const requiredOfficialPointers = [ test("the upstream index retains every required live OpenAI pointer", async () => { const sourceMap = await read("docs/OFFICIAL-DOCS.md"); for (const pointer of requiredOfficialPointers) { - assert.match(sourceMap, new RegExp(escapeRegExp(pointer))); + // Anchored on the Markdown link syntax each pointer actually appears in, + // `](URL)`, so the closing paren pins the end of the URL. A bare substring + // match let `…/build/skills` pass against a link to `…/build/skillsX`. + assert.match( + sourceMap, + new RegExp(`\\]\\(${escapeRegExp(pointer)}\\)`, "u"), + `docs/OFFICIAL-DOCS.md must link exactly ${pointer}`, + ); } assert.match(sourceMap, /not a cached specification/u); assert.match(sourceMap, /record the URLs and verification date/u); diff --git a/tests/helpers.mjs b/tests/helpers.mjs index 9658771..43d1ff0 100644 --- a/tests/helpers.mjs +++ b/tests/helpers.mjs @@ -68,12 +68,17 @@ export const assertMarketplacePlugin = async ({ name, version, category }) => { }; /** - * Reads a skill body plus its Codex metadata and asserts the two facts every - * skill test repeated: SKILL.md frontmatter opens with this skill's name, and - * openai.yaml aims its default prompt at the same name. Returns the raw body, - * the raw metadata, and the whitespace-collapsed body for further assertions. + * Reads a skill body plus its Codex metadata and asserts the three facts every + * skill test should repeat: SKILL.md frontmatter opens with this skill's name, + * openai.yaml aims its default prompt at the same name, and openai.yaml + * publishes the expected `display_name`. Returns the raw body, the raw + * metadata, and the whitespace-collapsed body for further assertions. + * + * `displayName` is required rather than optional so a new skill test cannot + * quietly reintroduce the asymmetry this closed, where only one of the eight + * skills pinned the name Codex actually shows users. */ -export const readSkillContract = async (paths, name) => { +export const readSkillContract = async (paths, name, displayName) => { const [skill, metadata] = await Promise.all([ read(paths.md(name)), read(paths.yaml(name)), @@ -81,6 +86,11 @@ export const readSkillContract = async (paths, name) => { assert.match(skill, new RegExp(`^---\\s+name: ${name}\\s+description:`, "u")); assert.match(metadata, new RegExp(`default_prompt: "Use \\$${name}`, "u")); + assert.match( + metadata, + new RegExp(`display_name: "${escapeRegExp(displayName)}"`, "u"), + `${name} must publish display_name "${displayName}"`, + ); return { skill, metadata, compactSkill: compact(skill) }; }; diff --git a/tests/humanize-plugin.test.mjs b/tests/humanize-plugin.test.mjs index b5a572f..0f8cd3d 100644 --- a/tests/humanize-plugin.test.mjs +++ b/tests/humanize-plugin.test.mjs @@ -29,6 +29,19 @@ const rubricText = once(() => read(rubricPath)); const evaluationCases = once(() => readJson(casesPath)); const execution = once(() => readJson(resultsPath)); +// Two tests need the recorded results keyed by id. Build the map once, here, +// including the duplicate-id check that makes keying by id mean anything: a +// bare `new Map(results.map(…))` silently keeps the last of a duplicated pair +// instead of failing, so the check has to live with the construction. +const recordedById = once(async () => { + const byId = new Map(); + for (const result of (await execution()).results) { + assert.equal(byId.has(result.id), false, `duplicate result ${result.id}`); + byId.set(result.id, result); + } + return byId; +}); + test("the marketplace exposes the Humanize plugin", async () => { await assertMarketplacePlugin({ name: "humanize", @@ -38,12 +51,13 @@ test("the marketplace exposes the Humanize plugin", async () => { }); test("skill metadata and references are complete", async () => { - const { skill, metadata } = await readSkillContract(paths, "humanize"); + // `readSkillContract` pins the frontmatter name, the default prompt, and + // `display_name: "Humanize"`. + const { skill } = await readSkillContract(paths, "humanize", "Humanize"); assert.match(skill, /text-bearing files/u); assert.match(skill, /Do not use for a pure authorship-classification request/u); assert.match(skill, /references\/revision-rubric\.md/u); - assert.match(metadata, /display_name: "Humanize"/u); await assertExists(rubricPath); }); @@ -74,7 +88,7 @@ test("the workflow protects content and legitimate voice", async () => { for (const phrase of [ "factual claims, the writer's intended position", "quotations, citations, links, terminology", - "do not invent anecdotes", + "Do not invent anecdotes", "Preserve clearly framed personal or organizational opinion", "Do not infer that a claim is unsupported merely because no source was supplied", "Preserve substantive fact-like, causal, quantitative, and significance claims", @@ -89,7 +103,10 @@ test("the workflow protects content and legitimate voice", async () => { "Do not add typos", "report an AI probability", ]) { - assert.match(skill, new RegExp(phrase, "iu")); + // Same escaping policy as the phrase list above: matched literally, and + // case-sensitively, since SKILL.md is digest-pinned and its casing cannot + // drift underneath these without the digest test firing first. + assert.match(skill, new RegExp(escapeRegExp(phrase), "u")); } }); @@ -162,13 +179,15 @@ test("the behavioral evaluation set covers activation and invariant cases", asyn }); test("recorded fresh-agent and installed-plugin evaluations satisfy their contracts", async () => { - const [cases, results] = await Promise.all([evaluationCases(), execution()]); + const [cases, results, resultsById] = await Promise.all([ + evaluationCases(), + execution(), + recordedById(), + ]); assert.equal(results.executedAt, "2026-08-05"); const casesById = new Map(cases.map((evaluation) => [evaluation.id, evaluation])); - const resultsById = new Map(); for (const result of results.results) { - assert.equal(resultsById.has(result.id), false, `duplicate result ${result.id}`); assert.ok(casesById.has(result.id), `result without case ${result.id}`); assert.ok(["revision", "clarification", "classification-response", "decision", "operation"].includes(result.resultType)); assert.equal(typeof result.taskRef, "string"); @@ -176,7 +195,6 @@ test("recorded fresh-agent and installed-plugin evaluations satisfy their contra assert.equal(typeof result.surface, "string"); assert.equal(typeof result.output, "string"); assert.ok(result.output.length > 0); - resultsById.set(result.id, result); const evaluation = casesById.get(result.id); const input = [evaluation.request, evaluation.context ?? "", evaluation.artifact].join("\n"); @@ -213,10 +231,24 @@ test("recorded fresh-agent and installed-plugin evaluations satisfy their contra const evidence = results.operationalEvidence; assert.match(evidence.seedCommit, /^[0-9a-f]{40}$/u); - assert.deepEqual(evidence.taskRefs, [ - "/root/forward_operational_files", - "/root/forward_operational_context", - ]); + + // What this protects is that the operational evidence came from exactly these + // two runs, in this order. The recording host's home directory is not part of + // that: freezing `/root/…` here pins a foreign machine's layout, which is the + // opposite of the `machine-agnostic` rule AGENTS.md states and + // documentation-contract.test.mjs enforces. Pin the run names and the count, + // anchored at a path boundary and at the end, and let the prefix vary. + assert.equal(evidence.taskRefs.length, 2); + for (const [index, run] of [ + "forward_operational_files", + "forward_operational_context", + ].entries()) { + assert.match( + evidence.taskRefs[index], + new RegExp(`(?:^|/)${escapeRegExp(run)}$`, "u"), + `operational task reference ${index} must name the ${run} run`, + ); + } assert.deepEqual(evidence.postconditions.trackedModified, [ "a.md", "docs/note.md", @@ -269,8 +301,7 @@ test("recorded evaluations are bound to the skill they were produced against", a }); test("each operational evaluation is scoped to the files its own request names", async () => { - const results = await execution(); - const resultsById = new Map(results.results.map((result) => [result.id, result])); + const [results, resultsById] = await Promise.all([execution(), recordedById()]); // The six operational scenarios were driven as two batched task runs, so // each recorded `output` is the whole batch's report. `mustMention` alone diff --git a/tests/plugin-ops-plugin.test.mjs b/tests/plugin-ops-plugin.test.mjs index 393431b..7249cba 100644 --- a/tests/plugin-ops-plugin.test.mjs +++ b/tests/plugin-ops-plugin.test.mjs @@ -18,7 +18,11 @@ test("the marketplace exposes the Plugin Ops plugin", async () => { }); test("install-marketplace uses native CLI and the narrowest commands", async () => { - const { compactSkill } = await readSkillContract(paths, "install-marketplace"); + const { compactSkill } = await readSkillContract( + paths, + "install-marketplace", + "Install Marketplace", + ); assert.match(compactSkill, /Use the native Codex CLI/u); assert.match(compactSkill, /codex plugin marketplace add/u); @@ -27,7 +31,7 @@ test("install-marketplace uses native CLI and the narrowest commands", async () }); test("update-plugins refreshes sources then reinstalls changed plugins", async () => { - const { compactSkill } = await readSkillContract(paths, "update-plugins"); + const { compactSkill } = await readSkillContract(paths, "update-plugins", "Update Plugins"); assert.match(compactSkill, /Refresh the configured source first/u); assert.match(compactSkill, /codex plugin marketplace upgrade/u); @@ -36,7 +40,7 @@ test("update-plugins refreshes sources then reinstalls changed plugins", async ( }); test("verify-plugin requires preflight, validator, and behavioral review", async () => { - const { compactSkill } = await readSkillContract(paths, "verify-plugin"); + const { compactSkill } = await readSkillContract(paths, "verify-plugin", "Verify Plugin"); assert.match(compactSkill, /official-documentation preflight/u); assert.match(compactSkill, /validate-marketplace\.mjs/u); @@ -48,7 +52,7 @@ test("verify-plugin requires preflight, validator, and behavioral review", async }); test("migrate-plugin follows the playbook ledger and isolation testing", async () => { - const { compactSkill } = await readSkillContract(paths, "migrate-plugin"); + const { compactSkill } = await readSkillContract(paths, "migrate-plugin", "Migrate Plugin"); assert.match(compactSkill, /docs\/PLUGIN-PHILOSOPHY\.md/u); assert.match(compactSkill, /docs\/MIGRATION-PLAYBOOK\.md/u); diff --git a/tests/scheduled-tasks-plugin.test.mjs b/tests/scheduled-tasks-plugin.test.mjs index 89a3562..682e7f5 100644 --- a/tests/scheduled-tasks-plugin.test.mjs +++ b/tests/scheduled-tasks-plugin.test.mjs @@ -1,13 +1,22 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { assertMarketplacePlugin, compact, once, read, skillPaths } from "./helpers.mjs"; +import { + assertMarketplacePlugin, + once, + readSkillContract, + skillPaths, +} from "./helpers.mjs"; const paths = skillPaths("scheduled-tasks"); -const skillPath = paths.md("manage-scheduled-tasks"); // All four body tests assert against the same whitespace-collapsed skill, so -// read and collapse it once. -const manageSkill = once(async () => compact(await read(skillPath))); +// read it once. `readSkillContract` also pins the SKILL.md frontmatter name and +// the `display_name`/`default_prompt` pair in `agents/openai.yaml`, which this +// suite alone used to skip. +const contract = once(() => + readSkillContract(paths, "manage-scheduled-tasks", "Manage Scheduled Tasks"), +); +const manageSkill = async () => (await contract()).compactSkill; test("the marketplace exposes the Scheduled tasks plugin", async () => { await assertMarketplacePlugin({ @@ -40,7 +49,7 @@ test("direct and indirect requests share one focused management workflow", async "Pause or resume", "Delete", ]) { - assert.match(skill, new RegExp(`\\*\\*${operation}:\\*\\*`)); + assert.match(skill, new RegExp(`\\*\\*${operation}:\\*\\*`, "u")); } assert.match(skill, /scheduled task inside the current chat/u); assert.match(skill, /standalone scheduled task/u); diff --git a/tests/test-harness.test.mjs b/tests/test-harness.test.mjs new file mode 100644 index 0000000..8df78e7 --- /dev/null +++ b/tests/test-harness.test.mjs @@ -0,0 +1,118 @@ +import assert from "node:assert/strict"; +import { readdir } from "node:fs/promises"; +import { test } from "node:test"; +import { assertExists, escapeRegExp, once, readJson } from "./helpers.mjs"; + +// `node --test ` exits 0 when does not exist, so the hand-written +// file list in `scripts.test` cannot fail closed on its own. Delete or rename a +// listed test and CI stays green while running fewer tests; add a new test file +// and it is silently never run. These tests close both directions, so this file +// must itself stay listed in `scripts.test`. +// +// The list stays hand-written on purpose. `node --test` glob positionals are +// Node 21+ while `engines.node` is ">=20", and bare `node --test` discovery is +// unstable across that range and changes what `npm test -- ` means. + +const repositoryRoot = new URL("../", import.meta.url); + +// The one test file that does not live in `tests/`. Named here so a walk that +// silently stops finding files cannot make the "registered" direction vacuous. +const outOfTreeTest = + "plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.test.mjs"; + +/** The `*.test.mjs` paths `npm test` actually hands to `node --test`. */ +const gateFiles = once(async () => { + const { scripts } = await readJson("package.json"); + const match = /^node --test (?\S.*)$/u.exec(scripts.test); + assert.ok( + match, + `scripts.test must stay "node --test " so this guard can read it, got: ${scripts.test}`, + ); + + const files = match.groups.paths.split(/\s+/u); + for (const file of files) { + assert.ok( + file.endsWith(".test.mjs"), + `scripts.test lists a non-test argument, which this guard cannot check: ${file}`, + ); + } + assert.equal(new Set(files).size, files.length, "scripts.test lists a file twice"); + return files; +}); + +/** Every `*.test.mjs` committed to the repository, as a root-relative path. */ +const diskFiles = once(async () => { + const found = []; + const walk = async (prefix) => { + const entries = await readdir(new URL(prefix, repositoryRoot), { + withFileTypes: true, + }); + for (const entry of entries) { + if (entry.name.startsWith(".") || entry.name === "node_modules") continue; + if (entry.isDirectory()) await walk(`${prefix}${entry.name}/`); + else if (entry.name.endsWith(".test.mjs")) found.push(`${prefix}${entry.name}`); + } + }; + await walk(""); + return found; +}); + +test("every test file on disk is registered in the npm test gate", async () => { + const [gate, disk] = await Promise.all([gateFiles(), diskFiles()]); + + // Anchors: an empty or truncated walk would otherwise pass this vacuously. + assert.ok(disk.includes(outOfTreeTest), `test-file walk missed ${outOfTreeTest}`); + assert.ok( + disk.includes("tests/test-harness.test.mjs"), + "test-file walk missed this guard itself", + ); + + const listed = new Set(gate); + for (const file of disk) { + assert.ok( + listed.has(file), + `${file} exists but is not listed in package.json scripts.test, so npm test never runs it`, + ); + } +}); + +test("every test file the npm test gate lists exists on disk", async () => { + // Checked with a direct existence probe rather than against the walk, so a + // broken walk cannot mask a missing file. `node --test` would exit 0 here. + for (const file of await gateFiles()) { + await assertExists(file); + } +}); + +// `tests/helpers.mjs` is exercised transitively by every plugin test, but two of +// its exports fail silently rather than loudly when broken, so they are pinned +// here directly. The rest (`read`, `readJson`, `assertExists`, `skillPaths`, +// `compact`, `assertMarketplacePlugin`, `readSkillContract`) break loudly the +// moment any suite runs, and need no separate coverage. + +test("escapeRegExp keeps every RegExp metacharacter literal", () => { + // Silent failure mode: an unescaped metacharacter still matches, so every + // assertion built on it — the 15 documentation URLs, every humanize + // invariant — quietly turns into a looser check that keeps passing. + for (const metacharacter of [".", "*", "+", "?", "^", "$", "{", "}", "(", ")", "|", "[", "]", "\\"]) { + const literal = `a${metacharacter}b`; + const pattern = new RegExp(escapeRegExp(literal), "u"); + assert.match(literal, pattern, `escapeRegExp lost ${metacharacter}`); + assert.doesNotMatch("aXb", pattern, `escapeRegExp left ${metacharacter} active`); + } +}); + +test("once loads a value one time and shares it", async () => { + // Silent failure mode: a non-memoizing `once` still returns correct values, + // so nothing fails — it just re-reads and re-parses behind every caller. + let loads = 0; + const load = once(async () => { + loads += 1; + return { loads }; + }); + + const [first, second] = await Promise.all([load(), load()]); + assert.equal(loads, 1); + assert.equal(first, second); + assert.equal(await load(), first); +}); From 71bcb77201bc140526f450685762cc25e4f79ebf Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 22:25:45 +0000 Subject: [PATCH 7/8] fix(skills): seed the qualified skill mention in every default_prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All eight skills seeded `default_prompt` with a bare `$skill-name` mention that Codex cannot resolve. Codex qualifies a plugin-provided skill's name with its plugin namespace — `SkillNamespaceResolver::qualify()` produces `plugin:skill`, applied to `skill.name` in the skills loader — and explicit mention selection matches `mentions.contains_plain_name(skill.name)` against that qualified name, with no base-name alias in the selector, the name counter, or the TUI mirror. `default_prompt` itself is stored verbatim: the interface resolver only collapses whitespace and enforces a length cap, and the namespace pass touches `skill.name` without touching `skill.interface`. So the seeded text reaches mention resolution unqualified and matches nothing. Qualified is correct-or-neutral on every surface: where a client attaches a structured skill item, resolution happens by path and the text form is irrelevant; everywhere else the bare form was simply failing. The repo's own docs already used the qualified form, so these eight values were the drift. Also qualifies the `$verify-plugin` mention in the migrate-plugin skill body, which had the same defect and no test pinning it. The contract is now pinned rather than left to convention: `skillPaths` exposes its plugin name and `readSkillContract` asserts the qualified mention, so the drift cannot silently return. Verified by mutation — restoring a bare mention fails with "humanize must seed the qualified mention $humanize:humanize". npm test 114/114 with 0 skipped, npm run validate PASS. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QH54tukcgvdxqwaTb5cdNB --- .../skills/coordinate-codex-work/agents/openai.yaml | 2 +- .../skills/find-skill-candidates/agents/openai.yaml | 2 +- plugins/humanize/skills/humanize/agents/openai.yaml | 2 +- .../skills/install-marketplace/agents/openai.yaml | 2 +- plugins/plugin-ops/skills/migrate-plugin/SKILL.md | 2 +- .../skills/migrate-plugin/agents/openai.yaml | 2 +- .../skills/update-plugins/agents/openai.yaml | 2 +- .../skills/verify-plugin/agents/openai.yaml | 2 +- .../skills/manage-scheduled-tasks/agents/openai.yaml | 2 +- tests/helpers.mjs | 12 +++++++++++- 10 files changed, 20 insertions(+), 10 deletions(-) diff --git a/plugins/codex-operations/skills/coordinate-codex-work/agents/openai.yaml b/plugins/codex-operations/skills/coordinate-codex-work/agents/openai.yaml index 00df98b..6819866 100644 --- a/plugins/codex-operations/skills/coordinate-codex-work/agents/openai.yaml +++ b/plugins/codex-operations/skills/coordinate-codex-work/agents/openai.yaml @@ -1,4 +1,4 @@ interface: display_name: "Coordinate Codex Work" short_description: "Direct multi-task and long-running Codex work." - default_prompt: "Use $coordinate-codex-work to coordinate this work across Codex tasks and agents." + default_prompt: "Use $codex-operations:coordinate-codex-work to coordinate this work across Codex tasks and agents." diff --git a/plugins/codex-operations/skills/find-skill-candidates/agents/openai.yaml b/plugins/codex-operations/skills/find-skill-candidates/agents/openai.yaml index a8ce211..6b298d0 100644 --- a/plugins/codex-operations/skills/find-skill-candidates/agents/openai.yaml +++ b/plugins/codex-operations/skills/find-skill-candidates/agents/openai.yaml @@ -1,6 +1,6 @@ interface: display_name: "Find Skill Candidates" short_description: "Find reusable skill ideas from recent sessions." - default_prompt: "Use $find-skill-candidates to review recent Codex sessions for reusable skill opportunities." + default_prompt: "Use $codex-operations:find-skill-candidates to review recent Codex sessions for reusable skill opportunities." policy: allow_implicit_invocation: false diff --git a/plugins/humanize/skills/humanize/agents/openai.yaml b/plugins/humanize/skills/humanize/agents/openai.yaml index 8db4b08..cdbd62b 100644 --- a/plugins/humanize/skills/humanize/agents/openai.yaml +++ b/plugins/humanize/skills/humanize/agents/openai.yaml @@ -1,4 +1,4 @@ interface: display_name: "Humanize" short_description: "Revise prose for natural, context-aware voice" - default_prompt: "Use $humanize to revise this writing while preserving its meaning, evidence, and voice." + default_prompt: "Use $humanize:humanize to revise this writing while preserving its meaning, evidence, and voice." diff --git a/plugins/plugin-ops/skills/install-marketplace/agents/openai.yaml b/plugins/plugin-ops/skills/install-marketplace/agents/openai.yaml index f6cf983..d462320 100644 --- a/plugins/plugin-ops/skills/install-marketplace/agents/openai.yaml +++ b/plugins/plugin-ops/skills/install-marketplace/agents/openai.yaml @@ -1,4 +1,4 @@ interface: display_name: "Install Marketplace" short_description: "Add and install Codex marketplaces safely" - default_prompt: "Use $install-marketplace to add a Codex marketplace and install a plugin." + default_prompt: "Use $plugin-ops:install-marketplace to add a Codex marketplace and install a plugin." diff --git a/plugins/plugin-ops/skills/migrate-plugin/SKILL.md b/plugins/plugin-ops/skills/migrate-plugin/SKILL.md index 4ce03ae..d9df63f 100644 --- a/plugins/plugin-ops/skills/migrate-plugin/SKILL.md +++ b/plugins/plugin-ops/skills/migrate-plugin/SKILL.md @@ -35,7 +35,7 @@ assumptions. defaults. Keep consumer-specific policy outside the plugin. 9. Prefer explicit input and native Codex configuration. Put optional tools behind narrow adapters with a useful fallback or clear unsupported result. -10. Run `$verify-plugin`, the built-in creation validators, and behavioral tests +10. Run `$plugin-ops:verify-plugin`, the built-in creation validators, and behavioral tests in a new Codex task with the source marketplace and sibling plugins disabled. Test direct, indirect, negative, incomplete, nested-repository-context, missing- diff --git a/plugins/plugin-ops/skills/migrate-plugin/agents/openai.yaml b/plugins/plugin-ops/skills/migrate-plugin/agents/openai.yaml index e45b008..390c2cd 100644 --- a/plugins/plugin-ops/skills/migrate-plugin/agents/openai.yaml +++ b/plugins/plugin-ops/skills/migrate-plugin/agents/openai.yaml @@ -1,4 +1,4 @@ interface: display_name: "Migrate Plugin" short_description: "Translate plugins into Codex-native packages" - default_prompt: "Use $migrate-plugin to port this plugin into a Codex-native package." + default_prompt: "Use $plugin-ops:migrate-plugin to port this plugin into a Codex-native package." diff --git a/plugins/plugin-ops/skills/update-plugins/agents/openai.yaml b/plugins/plugin-ops/skills/update-plugins/agents/openai.yaml index 2ced654..83622cf 100644 --- a/plugins/plugin-ops/skills/update-plugins/agents/openai.yaml +++ b/plugins/plugin-ops/skills/update-plugins/agents/openai.yaml @@ -1,4 +1,4 @@ interface: display_name: "Update Plugins" short_description: "Refresh marketplaces and reinstall plugins" - default_prompt: "Use $update-plugins to refresh my Codex marketplaces and plugins." + default_prompt: "Use $plugin-ops:update-plugins to refresh my Codex marketplaces and plugins." diff --git a/plugins/plugin-ops/skills/verify-plugin/agents/openai.yaml b/plugins/plugin-ops/skills/verify-plugin/agents/openai.yaml index f3ea2b0..4c06c6c 100644 --- a/plugins/plugin-ops/skills/verify-plugin/agents/openai.yaml +++ b/plugins/plugin-ops/skills/verify-plugin/agents/openai.yaml @@ -1,4 +1,4 @@ interface: display_name: "Verify Plugin" short_description: "Validate Codex plugins and marketplace catalogs" - default_prompt: "Use $verify-plugin to validate this Codex plugin or marketplace." + default_prompt: "Use $plugin-ops:verify-plugin to validate this Codex plugin or marketplace." diff --git a/plugins/scheduled-tasks/skills/manage-scheduled-tasks/agents/openai.yaml b/plugins/scheduled-tasks/skills/manage-scheduled-tasks/agents/openai.yaml index e3eecac..2b90da9 100644 --- a/plugins/scheduled-tasks/skills/manage-scheduled-tasks/agents/openai.yaml +++ b/plugins/scheduled-tasks/skills/manage-scheduled-tasks/agents/openai.yaml @@ -1,4 +1,4 @@ interface: display_name: "Manage Scheduled Tasks" short_description: "Create and manage native Scheduled tasks" - default_prompt: "Use $manage-scheduled-tasks to list my Scheduled tasks and help me update one." + default_prompt: "Use $scheduled-tasks:manage-scheduled-tasks to list my Scheduled tasks and help me update one." diff --git a/tests/helpers.mjs b/tests/helpers.mjs index 43d1ff0..faf97ee 100644 --- a/tests/helpers.mjs +++ b/tests/helpers.mjs @@ -32,6 +32,7 @@ export const assertExists = (path) => export const skillPaths = (plugin) => { const root = (skill) => `plugins/${plugin}/skills/${skill}`; return { + plugin, root, md: (skill) => `${root(skill)}/SKILL.md`, yaml: (skill) => `${root(skill)}/agents/openai.yaml`, @@ -85,7 +86,16 @@ export const readSkillContract = async (paths, name, displayName) => { ]); assert.match(skill, new RegExp(`^---\\s+name: ${name}\\s+description:`, "u")); - assert.match(metadata, new RegExp(`default_prompt: "Use \\$${name}`, "u")); + // Codex qualifies a plugin-provided skill's name with its plugin namespace + // (`plugin:skill`) and resolves an explicit mention against that qualified + // name only, with no base-name alias. `default_prompt` is stored verbatim and + // never rewritten, so a bare `$skill` in it names a mention that cannot + // resolve. Pinning the qualified form here keeps that drift from returning. + assert.match( + metadata, + new RegExp(`default_prompt: "Use \\$${paths.plugin}:${name}`, "u"), + `${name} must seed the qualified mention $${paths.plugin}:${name}`, + ); assert.match( metadata, new RegExp(`display_name: "${escapeRegExp(displayName)}"`, "u"), From 844b89a6783d6c8f380cab6ced573cb947459c34 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 23:32:55 +0000 Subject: [PATCH 8/8] test: pin the newly added upstream pointer in the documentation contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR added a required pointer row to docs/OFFICIAL-DOCS.md without adding the URL to `requiredOfficialPointers`, so the test named "retains every required live OpenAI pointer" stayed green whether or not that row survived. Deleting or mistyping the new pointer would have regressed the documentation contract silently — the same fail-open shape this PR fixes elsewhere. Verified by mutation: with the URL listed, changing the linked target to `…/build-skillsX` now fails with "docs/OFFICIAL-DOCS.md must link exactly https://learn.chatgpt.com/docs/build-skills". Before this change that mutation passed. Raised by the Codex review bot on this PR. npm test 114/114 with 0 skipped, npm run validate PASS. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QH54tukcgvdxqwaTb5cdNB --- tests/documentation-contract.test.mjs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/documentation-contract.test.mjs b/tests/documentation-contract.test.mjs index 4e2a47b..374e9ba 100644 --- a/tests/documentation-contract.test.mjs +++ b/tests/documentation-contract.test.mjs @@ -13,6 +13,7 @@ const requiredOfficialPointers = [ "https://learn.chatgpt.com/docs/build-plugins", "https://developers.openai.com/plugins/concepts/plugins", "https://developers.openai.com/plugins/build/skills", + "https://learn.chatgpt.com/docs/build-skills", "https://developers.openai.com/plugins/build/mcp-server", "https://developers.openai.com/plugins/build/chatgpt-ui", "https://developers.openai.com/plugins/build/plugins",