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/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) | 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/coordinate-codex-work/agents/openai.yaml b/plugins/codex-operations/skills/coordinate-codex-work/agents/openai.yaml index 1d7cb7e..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,6 +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." -policy: - allow_implicit_invocation: true + 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/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..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 @@ -19,12 +24,12 @@ 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 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, ) @@ -125,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 @@ -146,7 +157,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 +173,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): @@ -241,18 +255,13 @@ 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) - files = list(iter_candidate_files(args.sessions_dir, since)) + since = args.since + files = iter_candidate_files(args.sessions_dir, since) scanned_records = 0 included_records = 0 @@ -301,7 +310,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 @@ -368,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") @@ -391,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/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/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/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/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/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/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.mjs b/plugins/plugin-ops/skills/verify-plugin/scripts/validate-marketplace.mjs index 879fc1e..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,27 +158,33 @@ 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 (text.includes("[TODO:") || text.includes("TODO:")) { + if (name) skillNames.set(name, skillFile); + if (text.includes("TODO:")) { result.errors.push({ code: "skill-placeholder", file: skillFile, message: "Remove TODO placeholders before publishing." }); } if (MACHINE_PATH_PATTERN.test(text)) { 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; @@ -157,16 +232,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 +252,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, skillNames); 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.` }); } } } @@ -194,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 e7fe5e0..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,125 +1,515 @@ import assert from "node:assert/strict"; -import { mkdtemp, mkdir, writeFile } from "node:fs/promises"; +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(options = {}) { - const root = await mkdtemp(path.join(os.tmpdir(), "codex-marketplace-test-")); - const pluginRoot = path.join(root, "plugins", "sample-plugin"); - 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", - }] : []), - ], - }); - await writeJson(path.join(pluginRoot, ".codex-plugin", "plugin.json"), { - name: options.wrongName ? "wrong-name" : "sample-plugin", +// --- 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, + }; +} + +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; } -test("accepts a complete local marketplace", async () => { - const result = await validateTarget(await createFixture()); +/** 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 completeMarketplace(t)); assert.deepEqual(result.errors, []); + assert.deepEqual(result.warnings, []); 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)); - 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 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 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 skill name mismatch, TODO placeholders, and machine paths", async () => { +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 completeMarketplace(t, { skill: { frontmatter: false } })); + assert.deepEqual(errorCodes(result), new Set(["missing-skill-frontmatter"])); +}); + +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({ - skillNameMismatch: true, - todoPlaceholder: true, - machinePath: true, - }), + await completeMarketplace(t, { skill: { body: `Read \`${USER_HOME_PATH}\` for credentials.\n` } }), ); - const codes = new Set(result.errors.map((error) => error.code)); - 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 () => { - const result = await validateTarget(await createFixture({ omitUiMetadata: true })); +test("warns when skill UI metadata is missing", async (t) => { + const result = await validateTarget(await completeMarketplace(t, { skill: { uiMetadata: false } })); assert.deepEqual(result.errors, []); - const codes = new Set(result.warnings.map((warning) => warning.code)); - assert.ok(codes.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/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/codex-operations-plugin.test.mjs b/tests/codex-operations-plugin.test.mjs index 9329f22..343356b 100644 --- a/tests/codex-operations-plugin.test.mjs +++ b/tests/codex-operations-plugin.test.mjs @@ -1,48 +1,29 @@ 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", + "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 +31,28 @@ 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", + "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/codex-operations.test.mjs b/tests/codex-operations.test.mjs index 4ba5d0f..cc1bc90 100644 --- a/tests/codex-operations.test.mjs +++ b/tests/codex-operations.test.mjs @@ -34,6 +34,25 @@ function findPython() { return null; } +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, + }); +} + +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 +63,33 @@ 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; - } +function sessionRecord(message, timestamp = new Date()) { + return JSON.stringify({ timestamp: timestamp.toISOString(), payload: { message } }); +} - const temp = await mkdtemp(path.join(os.tmpdir(), "codex-operations-")); - t.after(() => rm(temp, { recursive: true, force: true })); +async function writeSessionLines(sessionsDir, lines) { + const sessionFile = path.join(sessionsDir, "session.jsonl"); + 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"); const firstSkills = path.join(temp, "first-skills"); const secondSkills = path.join(temp, "second-skills"); @@ -62,103 +99,282 @@ 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 = 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/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); + }); +} - const result = spawnSync( - python.command, - [ - ...python.prefix, - collector, +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, - ], - { - encoding: "utf8", - env: { ...process.env, CODEX_HOME: codexHome }, - }, - ); + ...(testCase.args ?? []), + ]); + assert.equal(result.status, 0, result.stderr); + testCase.check(result.stdout); + }); +} - assert.equal(result.status, 0, result.stderr); - assert.match(result.stdout, /Candidate files scanned: 1/); - assert.match(result.stdout, /`example-skill` - Example skill\./); +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 0a1776a..374e9ba 100644 --- a/tests/documentation-contract.test.mjs +++ b/tests/documentation-contract.test.mjs @@ -1,16 +1,19 @@ 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 = [ "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", @@ -28,7 +31,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(pointer.replaceAll(".", "\\."))); + // 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); @@ -41,7 +51,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 +80,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..faf97ee --- /dev/null +++ b/tests/helpers.mjs @@ -0,0 +1,106 @@ +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 { + plugin, + 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 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, displayName) => { + 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")); + // 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"), + `${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 f37bf9f..0f8cd3d 100644 --- a/tests/humanize-plugin.test.mjs +++ b/tests/humanize-plugin.test.mjs @@ -1,55 +1,68 @@ 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)); + +// 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 () => { - 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"), - ]); + // `readSkillContract` pins the frontmatter name, the default prompt, and + // `display_name: "Humanize"`. + const { skill } = await readSkillContract(paths, "humanize", "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,17 +78,17 @@ 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", "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", @@ -90,12 +103,15 @@ 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")); } }); 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 +148,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,16 +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, execution] = await Promise.all([ - readJson("tests/fixtures/humanize-evaluation-cases.json"), - readJson("tests/fixtures/humanize-evaluation-results.json"), + const [cases, results, resultsById] = await Promise.all([ + evaluationCases(), + execution(), + recordedById(), ]); - assert.equal(execution.executedAt, "2026-08-05"); + 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) { - assert.equal(resultsById.has(result.id), false, `duplicate result ${result.id}`); + for (const result of results.results) { 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"); @@ -180,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"); @@ -188,19 +202,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,12 +229,26 @@ 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", - "/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", @@ -248,8 +276,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 +286,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 +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 execution = await readJson("tests/fixtures/humanize-evaluation-results.json"); - const resultsById = new Map(execution.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 @@ -290,7 +319,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 +330,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..7249cba 100644 --- a/tests/plugin-ops-plugin.test.mjs +++ b/tests/plugin-ops-plugin.test.mjs @@ -1,111 +1,65 @@ 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", + "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", "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", "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", "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..682e7f5 100644 --- a/tests/scheduled-tasks-plugin.test.mjs +++ b/tests/scheduled-tasks-plugin.test.mjs @@ -1,38 +1,33 @@ import assert from "node:assert/strict"; -import { readFile } from "node:fs/promises"; import { test } from "node:test"; +import { + assertMarketplacePlugin, + once, + 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 skillPath = - "plugins/scheduled-tasks/skills/manage-scheduled-tasks/SKILL.md"; +const paths = skillPaths("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 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; - 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 +38,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", @@ -54,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); @@ -62,7 +57,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 +68,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); 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); +});