From eb22976da83cc6418181ebd1b4a5460844c60868 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Mon, 25 May 2026 17:51:32 -0500 Subject: [PATCH] =?UTF-8?q?review:=20address=20code-audit=20findings=20?= =?UTF-8?q?=E2=80=94=20.omo/=20artifact,=20docs=20drift,=20YAML=20quoting,?= =?UTF-8?q?=20validator=20helper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitignore | 3 + ARCHITECTURE.md | 2 +- Makefile | 6 +- docs/authoring.md | 15 ++- tools/adapters/copilot.py | 2 + tools/tests/test_adapters.py | 7 +- tools/tests/test_validate_generated.py | 2 +- tools/validate_generated.py | 137 ++++++++++--------------- 8 files changed, 77 insertions(+), 97 deletions(-) diff --git a/.gitignore b/.gitignore index 618ff0e..6bb643a 100644 --- a/.gitignore +++ b/.gitignore @@ -42,6 +42,9 @@ node_modules .opencode/ opencode.json +# OpenCode session continuation artifacts +.omo/ + # Copilot .copilot/ diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index e4314c5..140dcca 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -4,7 +4,7 @@ Top-level architectural map for the claude-agents marketplace. Detail lives in [ ## Invariants -1. **Single source of truth.** All agent / skill / command authoring happens under `plugins//`. Generated harness-specific artifacts (`.codex/`, `.cursor-plugin/`, `.opencode/`, `.github/agents/`, `.github/skills/`, `commands/`, `agents/`, `skills/` at extension root for Gemini) are produced by adapters and gitignored. Never hand-edit generated files. +1. **Single source of truth.** All agent / skill / command authoring happens under `plugins//`. Generated harness-specific artifacts (`.codex/`, `.cursor-plugin/`, `.opencode/`, `.copilot/`, `commands/`, `agents/`, `skills/` at extension root for Gemini) are produced by adapters and gitignored. Never hand-edit generated files. 2. **One canonical context file.** `AGENTS.md` at repo root is the only context file authored directly. `CLAUDE.md` imports it via `@AGENTS.md`. Gemini CLI reads it via `.gemini/settings.json` `context.fileName`. Codex / Cursor / OpenCode read `AGENTS.md` natively. diff --git a/Makefile b/Makefile index 367a756..680e3b0 100644 --- a/Makefile +++ b/Makefile @@ -161,7 +161,7 @@ clean: # make generate-all # make clean-generated HARNESS=opencode -HARNESSES := codex copilot cursor opencode gemini +HARNESSES := codex copilot cursor gemini opencode generate: ifndef HARNESS @@ -195,7 +195,7 @@ garden: # Full pytest suite — plugin-eval framework + tools/ adapters/validators/gardener. test: - uv run $(EVAL_PROJECT) --extra dev pytest -q plugins/plugin-eval/ tools/tests/ + uv run $(EVAL_PROJECT) pytest -q plugins/plugin-eval/ tools/tests/ # Real-CLI smoke test. Generates artifacts (if not present), then invokes whichever # of opencode / gemini / codex / claude are on PATH. Per-CLI tests skip gracefully @@ -206,7 +206,7 @@ smoke-test: echo "Generating harness artifacts first..."; \ $(MAKE) generate-all; \ fi - uv run $(EVAL_PROJECT) --extra dev pytest -v tools/tests/test_cli_smoke.py + uv run $(EVAL_PROJECT) pytest -v tools/tests/test_cli_smoke.py clean-generated: ifdef HARNESS diff --git a/docs/authoring.md b/docs/authoring.md index 274efda..794af77 100644 --- a/docs/authoring.md +++ b/docs/authoring.md @@ -1,7 +1,6 @@ # Authoring portable plugin content -Plugin content in this repo ships to **five** harnesses: Claude Code (source of truth), -OpenAI Codex CLI, Cursor, OpenCode, and Gemini CLI. The adapter framework handles per-harness +Plugin content in this repo ships to **five** harnesses: OpenAI Codex CLI, Cursor, OpenCode, Gemini CLI, and GitHub Copilot. Claude Code is the source-of-truth. The adapter framework handles per-harness mechanics (frontmatter rewrites, format transforms, output paths) so you author one set of markdown files. But content choices still affect portability — this guide tells you what to do, and what to avoid, so the work you do for Claude Code translates cleanly everywhere. @@ -106,12 +105,12 @@ clean naming — pick distinct names for skill/command pairs within a plugin. ### Model aliases -| Source field | Codex | Cursor | OpenCode | Gemini | -|---|---|---|---|---| -| `model: opus` | `gpt-5` | `inherit` | `anthropic/claude-opus-4-7` | `gemini-2.5-pro` | -| `model: sonnet` | `gpt-5-mini` | `inherit` | `anthropic/claude-sonnet-4-6` | `gemini-2.5-pro` | -| `model: haiku` | `gpt-5-nano` | `inherit` | `anthropic/claude-haiku-4-5-20251001` | `gemini-2.5-flash` | -| `model: inherit` | `gpt-5` | `inherit` | `anthropic/claude-sonnet-4-6` | `gemini-2.5-pro` | +| Source field | Codex | Cursor | OpenCode | Gemini | Copilot | +|---|---|---|---|---|---|---| +| `model: opus` | `gpt-5` | `inherit` | `anthropic/claude-opus-4-7` | `gemini-2.5-pro` | `gpt-5` | +| `model: sonnet` | `gpt-5-mini` | `inherit` | `anthropic/claude-sonnet-4-6` | `gemini-2.5-pro` | `gpt-5-mini` | +| `model: haiku` | `gpt-5-nano` | `inherit` | `anthropic/claude-haiku-4-5-20251001` | `gemini-2.5-flash` | `gpt-5-nano` | +| `model: inherit` | `gpt-5` | `inherit` | `anthropic/claude-sonnet-4-6` | `gemini-2.5-pro` | `gpt-5` | The adapter handles mapping. The `BARE_MODEL_ALIAS` lint is informational — it just notes that the mapping is implicit. If you want explicit, use `inherit`. diff --git a/tools/adapters/copilot.py b/tools/adapters/copilot.py index 5b44a4a..6afa847 100644 --- a/tools/adapters/copilot.py +++ b/tools/adapters/copilot.py @@ -24,6 +24,8 @@ def _needs_yaml_quoting(value: str) -> bool: "false", "yes", "no", + "on", + "off", "null", "~", ) diff --git a/tools/tests/test_adapters.py b/tools/tests/test_adapters.py index 70f71ef..abb2492 100644 --- a/tools/tests/test_adapters.py +++ b/tools/tests/test_adapters.py @@ -1050,7 +1050,12 @@ class TestCopilotAdapter: assert _needs_yaml_quoting("3.14") assert _needs_yaml_quoting("true") assert _needs_yaml_quoting("false") + assert _needs_yaml_quoting("yes") + assert _needs_yaml_quoting("no") + assert _needs_yaml_quoting("on") + assert _needs_yaml_quoting("off") assert _needs_yaml_quoting("null") + assert _needs_yaml_quoting("~") assert not _needs_yaml_quoting("hello world") assert not _needs_yaml_quoting("Use when testing.") @@ -1161,7 +1166,7 @@ class TestCapabilities: def test_every_adapter_id_has_capabilities_entry(self): from tools.adapters.capabilities import CAPABILITIES - for adapter_cls in (CodexAdapter, CopilotAdapter, CursorAdapter, OpenCodeAdapter, GeminiAdapter): + for adapter_cls in (CodexAdapter, CopilotAdapter, CursorAdapter, GeminiAdapter, OpenCodeAdapter): assert adapter_cls.harness_id in CAPABILITIES def test_model_aliases_complete(self): diff --git a/tools/tests/test_validate_generated.py b/tools/tests/test_validate_generated.py index ddf289b..9312f51 100644 --- a/tools/tests/test_validate_generated.py +++ b/tools/tests/test_validate_generated.py @@ -178,7 +178,7 @@ class TestCopilotValidator: report = Report() validate_copilot(report) - assert any("must not be empty" in f.message for f in report.errors()) + assert any("is empty" in f.message for f in report.errors()) def test_missing_description_errors(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch): _patch_worktree(monkeypatch, tmp_path) diff --git a/tools/validate_generated.py b/tools/validate_generated.py index adcb58e..1428c9e 100644 --- a/tools/validate_generated.py +++ b/tools/validate_generated.py @@ -61,6 +61,53 @@ class Report: return [f for f in self.findings if f.severity == "info"] +# ── Helpers ────────────────────────────────────────────────────────────────── + + +def _check_nonempty_str_field( + report: Report, + fm: dict, + field: str, + harness: str, + path: Path, + label: str = "", +) -> bool: + """Validate that `field` exists in `fm`, is a str, and is non-empty. + + Adds one Finding per violation. Returns True if the field passes all checks. + ``label`` is only used in remediation hints (defaults to ``field``). + """ + label = label or field + if field not in fm: + report.add( + severity="error", + harness=harness, + path=path, + message=f"missing required `{field}` field in frontmatter", + remediation=f"Each {harness} {label} needs a `{field}`.", + ) + return False + if not isinstance(fm[field], str): + report.add( + severity="error", + harness=harness, + path=path, + message=f"`{field}` field must be a string", + remediation=f"Set `{field}` to a plain string in the {label} frontmatter.", + ) + return False + if not fm[field].strip(): + report.add( + severity="error", + harness=harness, + path=path, + message=f"`{field}` field is empty", + remediation=f"Provide a non-empty `{field}` in the {label} frontmatter.", + ) + return False + return True + + # ── Codex validators ───────────────────────────────────────────────────────── @@ -583,54 +630,8 @@ def validate_copilot(report: Report) -> None: remediation="Regenerate via `make generate HARNESS=copilot`.", ) continue - if "name" not in fm: - report.add( - severity="error", - harness="copilot", - path=agent_md, - message="missing required `name` field in frontmatter", - remediation="Each Copilot agent needs a name.", - ) - elif not isinstance(fm["name"], str): - report.add( - severity="error", - harness="copilot", - path=agent_md, - message="`name` field must be a string", - remediation="Set `name` to a plain string in the agent frontmatter.", - ) - elif not fm["name"].strip(): - report.add( - severity="error", - harness="copilot", - path=agent_md, - message="`name` field must not be empty", - remediation="Provide a non-empty `name` in the agent frontmatter.", - ) - if "description" not in fm: - report.add( - severity="error", - harness="copilot", - path=agent_md, - message="missing required `description` field in frontmatter", - remediation="Copilot requires `description` in agent frontmatter. Check the source agent file.", - ) - elif not isinstance(fm.get("description"), str): - report.add( - severity="error", - harness="copilot", - path=agent_md, - message="`description` field must be a string", - remediation="Set `description` to a plain string; Copilot agent frontmatter does not accept other types.", - ) - elif not fm["description"].strip(): - report.add( - severity="error", - harness="copilot", - path=agent_md, - message="`description` field is empty", - remediation="Copilot requires a non-empty `description` in agent frontmatter.", - ) + _check_nonempty_str_field(report, fm, "name", "copilot", agent_md, label="agent") + _check_nonempty_str_field(report, fm, "description", "copilot", agent_md, label="agent") # 3. Skills: validate .copilot/skills/*/SKILL.md exists and has valid frontmatter. skills_dir = (WORKTREE / ".copilot" / "skills") if skills_dir.is_dir(): @@ -647,38 +648,8 @@ def validate_copilot(report: Report) -> None: remediation="Regenerate via `make generate HARNESS=copilot`.", ) continue - if "name" not in fm: - report.add( - severity="error", - harness="copilot", - path=skill_md, - message="missing required `name` field in frontmatter", - remediation="Each Copilot skill needs a name.", - ) - elif not isinstance(fm["name"], str) or not fm["name"].strip(): - report.add( - severity="error", - harness="copilot", - path=skill_md, - message="`name` must be a non-empty string", - remediation="Set `name` to a non-empty string in the skill frontmatter.", - ) - if "description" not in fm: - report.add( - severity="error", - harness="copilot", - path=skill_md, - message="missing required `description` field in frontmatter", - remediation="Each Copilot skill needs a description.", - ) - elif not isinstance(fm.get("description"), str) or not fm["description"].strip(): - report.add( - severity="error", - harness="copilot", - path=skill_md, - message="`description` must be a non-empty string", - remediation="Set `description` to a non-empty string in the skill frontmatter.", - ) + _check_nonempty_str_field(report, fm, "name", "copilot", skill_md, label="skill") + _check_nonempty_str_field(report, fm, "description", "copilot", skill_md, label="skill") commands_dir = WORKTREE / ".copilot" / "commands" if commands_dir.is_dir(): @@ -719,10 +690,10 @@ def validate_copilot(report: Report) -> None: _VALIDATORS = { "codex": validate_codex, - "cursor": validate_cursor, - "opencode": validate_opencode, - "gemini": validate_gemini, "copilot": validate_copilot, + "cursor": validate_cursor, + "gemini": validate_gemini, + "opencode": validate_opencode, }