From 49a58d33c7d13080362b6b33be970ae1aea366ac Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Tue, 8 Sep 2026 15:08:58 +0200 Subject: [PATCH 01/18] feat: add support for Codex Wires the MCP servers, enforcement hooks, and reviewer agent into Codex, so the harness gets the whole plugin rather than only the skills it already reads from `.agents/skills/`. The hook scripts stay single-sourced. Codex passes `tool_name: "Bash"` with a plain string command and accepts the same `permissionDecision` JSON, and plain stdout from a SessionStart hook is injected the same way, so four of the six scripts port with no edits. Only the edit hooks differ: Codex's file-editing tool is `apply_patch` and it hands the hook a raw patch with no `file_path`, so `hook-payload-common.sh` reads both payload shapes and `analyze.sh` / `format.sh` stay identical across harnesses. `codex/install.sh` installs skills, MCP servers, hooks, and agents, merging into an existing `hooks.json` rather than overwriting it. `codex/loader_test.sh` asserts Codex actually loads all of it via `codex debug prompt-input`, which needs no credentials, and runs as a new `codex-loader` CI job. The reviewer agent's read-only contract is held by `sandbox_mode = "read-only"`, since Codex has no per-agent tool allowlist or agent-scoped PreToolUse hook. Verified against Codex CLI 0.153.4. Hooks there are a stable, default-on feature (`[features] hooks`) and do run on Windows, so the caveats in the issue no longer apply; the Windows limit is that these scripts need bash and jq. The Claude Code path is unchanged: `hooks/hooks.json` and `agents/flutter-reviewer.md` stay authoritative and `codex/` mirrors them. Closes #46 Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yaml | 24 +- AGENTS.md | 26 +- CLAUDE.md | 17 +- CONTRIBUTING.md | 43 ++- README.md | 49 ++++ codex/agents/flutter-reviewer.toml | 131 +++++++++ codex/config.toml | 26 ++ codex/hooks.json | 60 +++++ codex/install.sh | 308 ++++++++++++++++++++++ codex/install_test.sh | 228 ++++++++++++++++ codex/loader_test.sh | 214 +++++++++++++++ config/cspell.json | 1 + hooks/scripts/analyze.sh | 24 +- hooks/scripts/format.sh | 22 +- hooks/scripts/hook-payload-common.sh | 92 +++++++ hooks/scripts/hook-payload-common_test.sh | 170 ++++++++++++ 16 files changed, 1413 insertions(+), 22 deletions(-) create mode 100644 codex/agents/flutter-reviewer.toml create mode 100644 codex/config.toml create mode 100644 codex/hooks.json create mode 100755 codex/install.sh create mode 100755 codex/install_test.sh create mode 100755 codex/loader_test.sh create mode 100755 hooks/scripts/hook-payload-common.sh create mode 100755 hooks/scripts/hook-payload-common_test.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 73c0e97..a063647 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -87,7 +87,23 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - - name: Read-only git hook tests - run: bash hooks/scripts/allow-readonly-git_test.sh - - name: Block CLI workarounds hook tests - run: bash hooks/scripts/block-cli-workarounds_test.sh + - name: Hook script tests + run: | + status=0 + for test in hooks/scripts/*_test.sh; do + echo "::group::$test" + bash "$test" || status=1 + echo "::endgroup::" + done + exit $status + - name: Codex installer tests + run: bash codex/install_test.sh + codex-loader: + name: 🤖 Codex Loader + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - name: Install Codex CLI + run: npm install -g @openai/codex + - name: Assert the plugin installs and loads in Codex + run: bash codex/loader_test.sh diff --git a/AGENTS.md b/AGENTS.md index f70cd55..1d1d106 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,6 +12,14 @@ VGV AI Flutter Plugin provides best-practices skills for Flutter and Dart develo plugin.json # Plugin manifest (name, version, keywords) agents/ flutter-reviewer.md # Read-only Flutter code reviewer subagent +codex/ # Codex wiring — the harness reads skills directly, the rest is installed + config.toml # ~/.codex/config.toml reference: dart + very-good-cli MCP, hooks feature + hooks.json # Codex hook definitions (__VGV_PLUGIN_ROOT__ substituted at install time) + install.sh # Installs skills, MCP, hooks, and agents into $CODEX_HOME + install_test.sh # Tests install.sh, including the non-destructive hooks.json merge + loader_test.sh # Asserts Codex actually loads all of it (run by the codex-loader CI job) + agents/ + flutter-reviewer.toml # Codex port of agents/flutter-reviewer.md docs/ plan/ # Planning and design documents evals/ @@ -46,6 +54,7 @@ hooks/ block-cli-workarounds.sh # Prevents direct CLI bypass via Bash check-vgv-cli.sh # Validates VGV CLI installed and >= 1.3.0 format.sh # Runs dart format on modified .dart files + hook-payload-common.sh # Reads Claude Code file_path and Codex apply_patch payloads vgv-cli-common.sh # Shared utilities for VGV CLI hook scripts warn-missing-mcp.sh # Warns at session start if VGV CLI is missing/outdated skills/ # every / ships SKILL.md + agents/openai.yaml (Codex sidecar) @@ -167,10 +176,23 @@ documentation in the same change: automatically, so verify each one by hand. - **Adding or changing a hook** in `hooks/hooks.json` — update the **Hooks** section in `README.md` (and the `## Hooks` section in `CLAUDE.md` if behavior - changes). + changes), and mirror the change in `codex/hooks.json`. The two files wire up the + same scripts and nothing keeps them in sync; `codex/loader_test.sh` only checks + that whatever `codex/hooks.json` names actually exists on disk. - **Adding or changing an MCP tool** — update the **MCP Integration** tools table in `README.md`, and check whether any skill's `allowed-tools` names a tool that - was renamed or removed. Nothing validates those names. + was renamed or removed. Nothing validates those names. A new **server** also has + to be registered for Codex, in both `codex/config.toml` and `codex/install.sh`. +- **Changing what a hook script reads from its payload** — the two harnesses + describe an edit differently (Claude Code `tool_input.file_path`, Codex + `tool_input.command` holding an apply_patch envelope). `hook-payload-common.sh` + is the only place that difference is handled; extend it there rather than + branching per harness in `analyze.sh` or `format.sh`, and add a case to + `hook-payload-common_test.sh`. +- **Changing `agents/flutter-reviewer.md`** — port the same change to + `codex/agents/flutter-reviewer.toml`. Its output contract (the four-column + findings table) is consumed verbatim by callers on both harnesses, so the two + must not drift. ## Evals diff --git a/CLAUDE.md b/CLAUDE.md index dbaa815..9d726e4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,7 +31,20 @@ from `vgv-cli-common.sh`. The following hook is **agent-scoped** — it is decla These run **after** a tool call completes: -- `Edit|Write` matcher → `analyze.sh` — runs `dart analyze` on the modified `.dart` file; exits 2 on failure (blocking — Claude must fix the issue) -- `Edit|Write` matcher → `format.sh` — runs `dart format` on the modified `.dart` file; always exits 0 (non-blocking) +- `Edit|Write` matcher → `analyze.sh` — runs `dart analyze` on the modified `.dart` file(s); exits 2 on failure (blocking — Claude must fix the issue) +- `Edit|Write` matcher → `format.sh` — runs `dart format` on the modified `.dart` file(s); always exits 0 (non-blocking) + +Both read the changed files through `hook-payload-common.sh`, which handles Claude Code's +`tool_input.file_path` and Codex's `tool_input.command` (an `apply_patch` envelope, which can +name several files at once). That is the only harness-specific branch in the hook scripts. All hook scripts require **jq** to parse the hook payload (they skip gracefully if `jq` is not installed). + +### Codex + +`codex/` holds the Codex-side wiring: `codex/hooks.json` mirrors `hooks/hooks.json` with +`${CLAUDE_PLUGIN_ROOT}` replaced at install time and `Edit|Write` widened to +`apply_patch|Edit|Write`, and `codex/agents/flutter-reviewer.toml` ports the reviewer agent. +`codex/install.sh` installs skills, MCP servers, hooks, and agents; `codex/loader_test.sh` proves +Codex loads them. Change a hook or the reviewer agent and you have to change both harnesses — see +`AGENTS.md` → Maintaining Existing Skills, Hooks, and MCP Tools. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 108322f..5ac041d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -193,6 +193,44 @@ frontmatter equivalent, and `interface.short_description`, which takes precedenc spec-legal `metadata: short-description` key. The `SKILL.md` body stays the one source of truth; the sidecar is thin, with no build step. Add one for every new skill. +**Codex runtime (`codex/`)** — skills reach Codex through the standard, but hooks, MCP, and +subagents do not, so `codex/` carries that wiring and `codex/install.sh` applies it. Verified +against Codex CLI 0.153.4: + +- **Hooks are a stable, default-on feature**, not experimental. The flag is `[features] hooks` + (`codex features list` shows it enabled); there is no `codex_hooks` flag. Codex also runs hooks + on Windows and offers a `commandWindows` override — but these scripts are `bash` and need `jq`, + so Windows means WSL or Git Bash. Codex **silently ignores a malformed `hooks.json`**, which + disables the whole enforcement layer with no error, so `codex/loader_test.sh` validates the + installed file directly. +- **Four of the six scripts port unchanged.** Codex passes `tool_name: "Bash"` with + `tool_input.command` as a plain string, and accepts the same `permissionDecision` allow/deny + JSON, so `check-vgv-cli.sh`, `block-cli-workarounds.sh`, and `allow-readonly-git.sh` need no + edits; plain stdout from a `SessionStart` hook is injected as a developer message exactly as on + Claude Code, so `warn-missing-mcp.sh` ports as-is too. Only the edit hooks differ: Codex's + file-editing tool is `apply_patch` and it hands the hook the raw patch, with no `file_path` and + no changed-file list, so `hook-payload-common.sh` parses the envelope. Keep that difference in + that one file. +- **`${CLAUDE_PLUGIN_ROOT}` is not available** to hooks in `~/.codex/hooks.json` (Codex resolves it + only for hooks that come from an installed Codex *plugin*, which this repo is not). The installer + substitutes the checkout path for `__VGV_PLUGIN_ROOT__` instead. +- **MCP goes through `codex mcp add`**, not a hand-written TOML block, so it merges rather than + replacing a user's config. `codex/config.toml` documents the same thing for anyone doing it by + hand. +- **The subagent trades a tool allowlist for a sandbox.** Codex custom agents are standalone TOML + in `~/.codex/agents/` (or project-scoped `.codex/agents/`) needing `name`, `description`, and + `developer_instructions`, plus any `config.toml` key. There is no per-agent tool allowlist and no + agent-scoped `PreToolUse` hook, so `flutter-reviewer` sets `sandbox_mode = "read-only"` to hold + the read-only contract that `allow-readonly-git.sh` holds on Claude Code. Codex ships no + validator for agent files, so `codex/loader_test.sh` parses them and asserts that + `sandbox_mode` is still `read-only`. +- **Do not weaken the Claude Code path** to make Codex simpler. `hooks/hooks.json` and + `agents/flutter-reviewer.md` stay authoritative; `codex/` mirrors them. + +Run `bash codex/loader_test.sh` before pushing a change to any of it. It needs the `codex` CLI but +no credentials — it asserts through `codex debug prompt-input`, which renders the model-visible +prompt without calling a model. + **Invocation** — every skill in this plugin is **model-invoked**: the model may reach for it autonomously when the context fits (that is the point of a best-practice skill), so neither `disable-model-invocation` (Claude Code) nor a `policy` block (Codex) is set. All trigger @@ -215,6 +253,8 @@ session and exercise it before you commit. - **Dart SDK** and **jq** on your `PATH` — the hooks need both. - **Very Good CLI** ≥ 1.3.0 (`dart pub global activate very_good_cli`) for the Very Good CLI MCP server tools. +- **Codex CLI** (`npm install -g @openai/codex`) only if you touch `codex/` — + `codex/loader_test.sh` needs it. Everything else runs without it. See the README [Hooks](README.md#hooks) and [MCP Integration](README.md#mcp-integration) sections for the full prerequisite details. @@ -308,7 +348,8 @@ Every pull request runs the following checks automatically: | Spelling | Runs cspell on all `*.md` files | `config/cspell.json` | | Skill validation | Validates **every** `SKILL.md`'s frontmatter and structure against the Agent Skills spec, so a malformed skill fails the build instead of silently vanishing on another host | `Flash-Brew-Digital/validate-skill@v1` | | Plugin validation | Validates and test-installs the plugin | `claude plugin validate .` | -| Script tests | Runs the hook scripts' own test suites | `hooks/scripts/*_test.sh` | +| Script tests | Runs every hook script test suite, plus the Codex installer's | `hooks/scripts/*_test.sh`, `codex/install_test.sh` | +| Codex loader | Installs the plugin into a throwaway Codex home and asserts all 15 skills, both MCP servers, the hooks, and the reviewer agent load | `codex/loader_test.sh` | Evals do **not** run on a pull request. They call real models, so they run after a merge to `main` instead, scoped to the skills that changed: diff --git a/README.md b/README.md index a0075c9..6f0160f 100644 --- a/README.md +++ b/README.md @@ -76,11 +76,55 @@ This plugin includes SessionStart, PreToolUse, and PostToolUse hooks that valida | **Analyze** (`analyze.sh`) | PostToolUse (`Edit`/`Write`) | Runs `dart analyze` on the modified `.dart` file; exits 2 on failure (blocking — Claude must fix issues before continuing) | | **Format** (`format.sh`) | PostToolUse (`Edit`/`Write`) | Runs `dart format` on the modified `.dart` file; always exits 0 (non-blocking — formatting is applied silently) | +The triggers above are the Claude Code ones. The same scripts run on Codex — see [Codex](#codex) +for the wiring and the two behavioral differences. + ### Prerequisites - **Dart SDK** — must be available on your `PATH` - **jq** — used to parse the hook payload; hooks are skipped gracefully if `jq` is not installed +## Codex + +The skills follow the [Agent Skills open standard][agent_skills_link], so Codex loads them from +`~/.agents/skills/` with no adapter. The MCP servers, hooks, and reviewer agent need wiring up +once: + +```bash +git clone https://github.com/VeryGoodOpenSource/vgv-ai-flutter-plugin.git && bash vgv-ai-flutter-plugin/codex/install.sh +``` + +| Component | Where it lands | Notes | +| --------- | -------------- | ----- | +| Skills | `~/.agents/skills/` | Symlinked to the checkout, so `git pull` updates them. Use `--copy` for real copies | +| MCP servers | `~/.codex/config.toml` | `dart` and `very-good-cli`, registered with `codex mcp add` | +| Hooks | `~/.codex/hooks.json` | Merged into whatever is already there, never overwritten | +| Reviewer agent | `~/.codex/agents/flutter-reviewer.toml` | Ask Codex to spawn `flutter-reviewer` | + +Restart Codex afterwards, then approve the new hooks with `/hooks` — Codex requires a review before +a hook runs for the first time. Re-running the installer replaces what it installed before instead +of adding a second copy. `--dry-run` prints the changes without making them, and `--uninstall` +reverses all four steps. + +### How Codex differs from Claude Code + +- **The hooks are the same scripts.** Only the wiring differs. Codex calls its file-editing tool + `apply_patch` and hands the hook a raw patch rather than a file path, so `analyze.sh` and + `format.sh` read both shapes; `${CLAUDE_PLUGIN_ROOT}` means nothing to Codex, so the installer + bakes the checkout path into `hooks.json`. +- **The reviewer agent is sandboxed instead of tool-restricted.** On Claude Code `flutter-reviewer` + has no write tools and an agent-scoped hook limits its Bash to `git diff`/`git status`. Codex has + no per-agent tool allowlist, so the agent declares `sandbox_mode = "read-only"` — the OS refuses + every write, which covers the same "never edits files" guarantee. +- **Hooks are on by default.** They are a stable Codex feature; `codex features list` shows + `hooks` enabled. The installer pins `[features] hooks = true` only in case something in your + config had turned it off. +- **Windows needs a POSIX shell.** Codex itself runs hooks on Windows, but every script here is + `bash` and needs `jq`, so run Codex under WSL or Git Bash. + +Codex truncates a skill `description` at 1024 characters and concatenates all of them into every +request, which is why descriptions in this repo are kept to triggers and scope. + ## Evals Skill evals ask whether Claude routes to a skill and follows it. [promptfoo](https://www.promptfoo.dev) sends each case's prompt through the Claude Agent SDK twice — once with this plugin loaded, once sealed with nothing loaded — so a grader that passes in both columns is measuring the model rather than the skill. They authenticate through your local Claude Code session, so they need no API key. @@ -187,6 +231,11 @@ The Very Good CLI MCP server exposes Very Good CLI commands to Claude. The `.mcp.json` file at the project root registers the `dart` and `very-good-cli` MCP servers using stdio transport. When Claude Code detects this configuration, it connects to both servers and gains access to the tools above. The skills continue to provide knowledge and best practices while the MCP tools handle execution. +On Codex the same two servers are registered in `~/.codex/config.toml` instead — see +[Codex](#codex). Skills that drive an MCP tool always name the equivalent `very_good`, `dart`, or +`flutter` command as a fallback, so they keep working on a host where neither server is connected. + +[agent_skills_link]: https://agentskills.io/specification [marketplace_link]: https://github.com/VeryGoodOpenSource/very-good-claude-code-marketplace [claude_code_link]: https://claude.ai/code [vgv_link]: https://verygood.ventures diff --git a/codex/agents/flutter-reviewer.toml b/codex/agents/flutter-reviewer.toml new file mode 100644 index 0000000..1f959f3 --- /dev/null +++ b/codex/agents/flutter-reviewer.toml @@ -0,0 +1,131 @@ +name = "flutter-reviewer" +description = "Read-only Flutter code reviewer. Spawn after writing or changing Dart code to review changed code against VGV bloc, testing, security, and accessibility standards. Never edits files." + +# The Claude Code agent enforces its read-only contract two ways: it declares no +# write tools, and an agent-scoped PreToolUse hook restricts Bash to `git diff` +# and `git status`. Codex has no agent-scoped tool allowlist, so the contract is +# enforced by the sandbox instead. `read-only` is a stronger guarantee than the +# hook it replaces — the OS refuses every write, not just the shell commands a +# matcher anticipated — and the shell restriction is restated as an instruction +# below so the agent does not waste turns on commands the sandbox will reject. +sandbox_mode = "read-only" + +model_reasoning_effort = "high" + +developer_instructions = ''' +You are a read-only Flutter code reviewer for Very Good Ventures. You review changed Dart code +against four VGV standards and report findings as a markdown table. When a parent agent spawns +you, it consumes your table verbatim. + +## Read-only contract + +You **never** edit files. You run in a `read-only` sandbox, so every write is refused: editing a +file, `git checkout`, `git apply`, `sed -i`, and output redirection all fail. Restrict your shell +use to read-only git inspection — only `git diff` and `git status`. Do not attempt to work around +the sandbox; it is intentional. + +If you ever conclude that a fix requires editing a file, describe the fix in the `fix` column of +your findings table. Do not apply it. + +## Standards + +Load these four VGV skills and treat them as your only standards source: + +- **`bloc`** — Bloc/Cubit state management conventions. +- **`testing`** — unit, widget, and golden test conventions. +- **`static-security`** — Flutter static security review. +- **`accessibility`** — WCAG-aligned Flutter accessibility. + +Load all four before you report, and load nothing else as a standard. Every finding you report +must trace back to one of them. If a problem does not map to one of the four, do not report it +(see "What not to report"). + +If a skill will not load, say so in one line before your table and review against the ones that +did — never substitute your own conventions for a standard you could not read. + +## Diff scoping + +Scope your review to changed Dart code only. Never review the whole repository. + +Determine the change set adaptively, from the repository root: + +1. **Uncommitted changes first.** Run `git status` and `git diff` (staged and unstaged). If there + are uncommitted `.dart` changes, review those. +2. **Otherwise, branch-vs-base.** If the working tree is clean, fall back to the branch's changes + against its merge base: `git diff ...HEAD` (typically `main...HEAD`). Use `git status` and + `git diff` to enumerate the changed files. +3. **Include untracked `.dart` files.** `git status` surfaces untracked files; review untracked + `.dart` files as new code. +4. **Monorepo / subdirectory.** Always scope from the repository root and apply the four standards + per affected package. + +Read the changed files in full to review their context, not just the diff hunks. When the Dart MCP +server is connected, you may use its `analyze_files` tool to corroborate a skill-based judgment, +but analyzer output is not itself a findings source (see "What not to report"). If it is +unavailable, rely on the four standards alone — do not run `dart analyze` yourself just to fill the +gap, because analyzer findings are out of scope either way. + +### When scoping fails + +If you cannot determine a change scope — not a git repository, detached HEAD, no merge base, or the +git commands fail — report that you could not determine a change scope and stop. Do not guess and do +not review the whole repository. + +## Output + +Output **exactly one** markdown table, one row per finding. Do **not** split findings into multiple +tables, do **not** group them by file, and do **not** introduce section headings or extra columns +around the table. The table has exactly these four columns, in this order — `location`, `problem`, +`fix`, `standard`: + +```markdown +| location | problem | fix | standard | +| --------------------------------- | ---------------------------------------- | ------------------------------------ | -------------- | +| lib/counter/counter_cubit.dart:12 | Mutable state field breaks immutability | Mark state class fields `final` | bloc | +| test/counter/counter_test.dart:30 | Tautological assertion `expect(x, x)` | Assert against the expected value | testing | +``` + +Rules: + +- `location` — `path:line` of the finding, in a single column. Always include the file path on every + row; never move the path into a heading and never reduce this column to a bare line number. +- `problem` — what is wrong, concisely. +- `fix` — the change you recommend. Describe it; never apply it. +- `standard` — exactly one of `bloc`, `testing`, `static-security`, `accessibility`, in its own + column on every row. Every row must name one of these four. Never convey the standard through a + section heading instead of this column. +- Align the pipe characters vertically (VGV markdown convention). + +A one-line note after the table (per "Out-of-domain changes" below) is allowed. Any other prose, +grouping, or additional tables is not. + +### No changed Dart files + +If the change scope contains no `.dart` files (clean tree, or only non-Dart changes), report +`No changed Dart files to review.` and stop. Never emit an empty table and never invent findings. + +### Out-of-domain changes + +Your four standards do not cover every domain. If changed Dart code touches areas outside them — +for example navigation, theming, internationalization, or layered architecture — you have no loaded +standard to cite, so you stay silent on findings there. Add a one-line note after the table listing +the changed areas that fall outside your four standards, so a clean review is not mistaken for full +coverage. For example: + +> Note: changes in `lib/routing/` and `lib/theme/` are outside the loaded standards (bloc, testing, +> static-security, accessibility) and were not reviewed. + +### What not to report + +- **Analyzer-only findings.** Raw `dart analyze` errors (unused imports, dead null-aware operators, + etc.) do not trace to any of your four standards, so they are out of scope for your table. + Do not report them and do not introduce a `dart-analyzer` pseudo-standard. Such errors are caught + separately by the plugin's PostToolUse `analyze.sh` hook when code is written, not here. Use the + analyzer only to corroborate a skill-based judgment. +- **Untraceable findings.** If a finding cannot name one of the four standards, omit it. + +## Dispatch contract + +When a parent agent spawns you, you self-scope via the adaptive diff procedure above — the caller +does not pass you a file list — and the caller consumes your findings table verbatim. +''' diff --git a/codex/config.toml b/codex/config.toml new file mode 100644 index 0000000..fae005a --- /dev/null +++ b/codex/config.toml @@ -0,0 +1,26 @@ +# Codex configuration for the VGV AI Flutter Plugin. +# +# This is the `~/.codex/config.toml` equivalent of the repository-root `.mcp.json` +# that Claude Code reads. `codex/install.sh` applies all of it for you through the +# supported `codex mcp add` and `codex features enable` commands, which merge into +# an existing config instead of replacing it. Merge it by hand only if you would +# rather not run the installer. + +# The Dart and Flutter MCP server ships with the Dart SDK. The `cli` feature +# category is off by default, so `dart_format` has to be enabled explicitly — +# the `green-gate` skill's format gate calls that tool. Test execution stays +# disabled on purpose; the Very Good CLI `test` tool is used instead. +[mcp_servers.dart] +command = "dart" +args = ["mcp-server", "--enable", "dart_format"] + +# Very Good CLI >= 1.3.0 must be on your PATH: +# dart pub global activate very_good_cli +[mcp_servers.very-good-cli] +command = "very_good" +args = ["mcp"] + +# Hooks are stable and enabled by default in Codex. Pinning the flag here only +# matters if something in your config or profile previously set it to false. +[features] +hooks = true diff --git a/codex/hooks.json b/codex/hooks.json new file mode 100644 index 0000000..6d246dd --- /dev/null +++ b/codex/hooks.json @@ -0,0 +1,60 @@ +{ + "description": "VGV AI Flutter Plugin hooks for Dart and Flutter development (Codex)", + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "bash \"__VGV_PLUGIN_ROOT__/hooks/scripts/warn-missing-mcp.sh\"", + "statusMessage": "Checking Very Good CLI", + "timeout": 10 + } + ] + } + ], + "PreToolUse": [ + { + "matcher": "mcp__.*very-good-cli__.*", + "hooks": [ + { + "type": "command", + "command": "bash \"__VGV_PLUGIN_ROOT__/hooks/scripts/check-vgv-cli.sh\"", + "statusMessage": "Verifying Very Good CLI version", + "timeout": 10 + } + ] + }, + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "bash \"__VGV_PLUGIN_ROOT__/hooks/scripts/block-cli-workarounds.sh\"", + "statusMessage": "Checking for CLI bypass", + "timeout": 10 + } + ] + } + ], + "PostToolUse": [ + { + "matcher": "apply_patch|Edit|Write", + "hooks": [ + { + "type": "command", + "command": "bash \"__VGV_PLUGIN_ROOT__/hooks/scripts/analyze.sh\"", + "statusMessage": "dart analyze", + "timeout": 30 + }, + { + "type": "command", + "command": "bash \"__VGV_PLUGIN_ROOT__/hooks/scripts/format.sh\"", + "statusMessage": "dart format", + "timeout": 30 + } + ] + } + ] + } +} diff --git a/codex/install.sh b/codex/install.sh new file mode 100755 index 0000000..f36e40b --- /dev/null +++ b/codex/install.sh @@ -0,0 +1,308 @@ +#!/bin/bash +# Install the VGV AI Flutter Plugin into Codex. +# +# Codex has no marketplace entry for this plugin, so the four pieces are wired up +# individually: +# +# skills -> ~/.agents/skills/ (symlinked, or copied with --copy) +# MCP -> ~/.codex/config.toml (via `codex mcp add`) +# hooks -> ~/.codex/hooks.json (merged, never overwritten) +# agent -> ~/.codex/agents/.toml +# +# Re-running is safe: every step replaces what a previous run installed rather +# than stacking a second copy. +# +# Usage: +# bash codex/install.sh [options] +# +# Options: +# --skills-dir DIR Where to install skills (default: $HOME/.agents/skills) +# --copy Copy skills instead of symlinking the checkout +# --dry-run Print what would change and exit +# --uninstall Remove everything this script installs +# -h, --help Show this help +# +# Environment: +# CODEX_HOME Codex config directory (default: $HOME/.codex) + +set -uo pipefail + +PLUGIN_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +CODEX_HOME="${CODEX_HOME:-$HOME/.codex}" +SKILLS_DIR="${HOME}/.agents/skills" +INSTALL_MODE="link" +DRY_RUN=0 +UNINSTALL=0 + +# Handler commands this script owns. Anything else in hooks.json is left alone. +VGV_HOOK_PATTERN='hooks/scripts/(warn-missing-mcp|check-vgv-cli|block-cli-workarounds|analyze|format)\.sh' + +# Print the header comment block as help text. +usage() { + awk 'NR > 1 && /^#/ { sub(/^# ?/, ""); print; next } NR > 1 { exit }' "${BASH_SOURCE[0]}" +} + +while [ $# -gt 0 ]; do + case "$1" in + --skills-dir) SKILLS_DIR="$2"; shift 2 ;; + --copy) INSTALL_MODE="copy"; shift ;; + --dry-run) DRY_RUN=1; shift ;; + --uninstall) UNINSTALL=1; shift ;; + -h|--help) usage; exit 0 ;; + *) echo "unknown option: $1" >&2; usage >&2; exit 2 ;; + esac +done + +info() { printf ' %s\n' "$1"; } +step() { printf '\n\033[1m%s\033[0m\n' "$1"; } +warn() { printf ' \033[33mwarning\033[0m %s\n' "$1" >&2; } +die() { printf '\033[31merror\033[0m %s\n' "$1" >&2; exit 1; } +run() { if [ "$DRY_RUN" -eq 1 ]; then info "would run: $*"; else "$@"; fi; } + +if ! command -v jq &>/dev/null; then + die "jq is required (the hooks parse their payload with it). Install jq and re-run." +fi + +HAS_CODEX=1 +if ! command -v codex &>/dev/null; then + HAS_CODEX=0 +fi + +# ---------------------------------------------------------------- skills + +install_skills() { + step "Skills -> $SKILLS_DIR" + if [ "$DRY_RUN" -eq 0 ]; then + mkdir -p "$SKILLS_DIR" || die "cannot create $SKILLS_DIR" + fi + local src name dest count=0 + for src in "$PLUGIN_ROOT"/skills/*/; do + [ -f "$src/SKILL.md" ] || continue + name="$(basename "$src")" + dest="$SKILLS_DIR/$name" + if [ "$DRY_RUN" -eq 1 ]; then + info "would install $name ($INSTALL_MODE)" + else + rm -rf "$dest" + if [ "$INSTALL_MODE" = "copy" ]; then + cp -R "${src%/}" "$dest" || die "failed to copy $name" + else + ln -s "${src%/}" "$dest" || die "failed to link $name" + fi + fi + count=$((count + 1)) + done + info "$count skills ($INSTALL_MODE)" +} + +uninstall_skills() { + step "Removing skills from $SKILLS_DIR" + local src name dest count=0 + for src in "$PLUGIN_ROOT"/skills/*/; do + name="$(basename "$src")" + dest="$SKILLS_DIR/$name" + [ -e "$dest" ] || [ -L "$dest" ] || continue + run rm -rf "$dest" + count=$((count + 1)) + done + info "$count skills removed" +} + +# ------------------------------------------------------------------- MCP + +install_mcp() { + step "MCP servers -> $CODEX_HOME/config.toml" + if [ "$HAS_CODEX" -eq 0 ]; then + warn "codex is not on your PATH; skipping MCP setup." + warn "Merge codex/config.toml into $CODEX_HOME/config.toml by hand." + return + fi + run env CODEX_HOME="$CODEX_HOME" codex mcp add dart -- dart mcp-server --enable dart_format \ + || warn "could not register the dart MCP server" + run env CODEX_HOME="$CODEX_HOME" codex mcp add very-good-cli -- very_good mcp \ + || warn "could not register the very-good-cli MCP server" + run env CODEX_HOME="$CODEX_HOME" codex features enable hooks \ + || warn "could not pin the hooks feature (it is on by default)" +} + +uninstall_mcp() { + step "Removing MCP servers from $CODEX_HOME/config.toml" + if [ "$HAS_CODEX" -eq 0 ]; then + warn "codex is not on your PATH; remove [mcp_servers.dart] and" + warn "[mcp_servers.very-good-cli] from $CODEX_HOME/config.toml by hand." + return + fi + run env CODEX_HOME="$CODEX_HOME" codex mcp remove dart >/dev/null 2>&1 + run env CODEX_HOME="$CODEX_HOME" codex mcp remove very-good-cli >/dev/null 2>&1 + info "dart and very-good-cli removed" +} + +# ----------------------------------------------------------------- hooks + +# Merge our hook handlers into an existing hooks.json without disturbing anyone +# else's. Handlers this script installed are stripped first, so re-running +# replaces them instead of appending a duplicate. +merge_hooks() { + local existing="$1" incoming="$2" + jq -n \ + --slurpfile cur "$existing" \ + --slurpfile new "$incoming" \ + --arg pattern "$VGV_HOOK_PATTERN" ' + def is_vgv: (.command // "") | test($pattern); + def strip_vgv: + map(.hooks = ((.hooks // []) | map(select(is_vgv | not)))) + | map(select((.hooks | length) > 0)); + + ($cur[0] // {}) as $base + | ($new[0].hooks // {}) as $add + | ( + ($base.hooks // {}) + | with_entries(.value |= strip_vgv) + | with_entries(select((.value | length) > 0)) + ) as $stripped + | $base + + { hooks: ( + reduce ($add | to_entries[]) as $e ($stripped; + .[$e.key] = ((.[$e.key] // []) + $e.value)) + ) } + ' +} + +strip_hooks() { + local existing="$1" + jq --arg pattern "$VGV_HOOK_PATTERN" ' + def is_vgv: (.command // "") | test($pattern); + def strip_vgv: + map(.hooks = ((.hooks // []) | map(select(is_vgv | not)))) + | map(select((.hooks | length) > 0)); + .hooks = ((.hooks // {}) + | with_entries(.value |= strip_vgv) + | with_entries(select((.value | length) > 0))) + ' "$existing" +} + +# Write $2 over $1, keeping a timestamped backup of whatever was there. +write_hooks_file() { + local target="$1" content="$2" + if [ "$DRY_RUN" -eq 1 ]; then + info "would write $target:" + printf '%s\n' "$content" | sed 's/^/ /' + return + fi + mkdir -p "$(dirname "$target")" + if [ -f "$target" ]; then + local backup="$target.bak-$(date +%Y%m%d%H%M%S)" + cp "$target" "$backup" && info "backed up to $backup" + fi + printf '%s\n' "$content" > "$target.tmp" && mv "$target.tmp" "$target" +} + +install_hooks() { + step "Hooks -> $CODEX_HOME/hooks.json" + local template="$PLUGIN_ROOT/codex/hooks.json" + [ -f "$template" ] || die "missing $template" + + # ${CLAUDE_PLUGIN_ROOT} is resolved by Claude Code and means nothing to Codex, + # so the absolute path to this checkout is baked in at install time. + local resolved + resolved=$(jq --arg root "$PLUGIN_ROOT" \ + 'walk(if type == "string" then gsub("__VGV_PLUGIN_ROOT__"; $root) else . end)' \ + "$template") || die "could not read $template" + + local target="$CODEX_HOME/hooks.json" + local scratch current incoming merged + scratch=$(mktemp -d) || die "could not create a temp directory" + incoming="$scratch/incoming.json" + printf '%s\n' "$resolved" > "$incoming" + if [ -f "$target" ]; then + current="$target" + else + current="$scratch/current.json" + echo '{}' > "$current" + fi + + merged=$(merge_hooks "$current" "$incoming") + local status=$? + rm -rf "$scratch" + [ $status -eq 0 ] || die "could not merge $target" + write_hooks_file "$target" "$merged" + info "SessionStart, PreToolUse (2), PostToolUse (2)" +} + +uninstall_hooks() { + step "Removing hooks from $CODEX_HOME/hooks.json" + local target="$CODEX_HOME/hooks.json" + if [ ! -f "$target" ]; then + info "nothing to remove" + return + fi + local stripped + stripped=$(strip_hooks "$target") || die "could not rewrite $target" + write_hooks_file "$target" "$stripped" +} + +# ----------------------------------------------------------------- agents + +install_agents() { + step "Agents -> $CODEX_HOME/agents" + local src name count=0 + for src in "$PLUGIN_ROOT"/codex/agents/*.toml; do + [ -f "$src" ] || continue + name="$(basename "$src")" + if [ "$DRY_RUN" -eq 1 ]; then + info "would install $name" + else + mkdir -p "$CODEX_HOME/agents" + cp "$src" "$CODEX_HOME/agents/$name" || die "failed to install $name" + fi + count=$((count + 1)) + done + info "$count agents" +} + +uninstall_agents() { + step "Removing agents from $CODEX_HOME/agents" + local src name count=0 + for src in "$PLUGIN_ROOT"/codex/agents/*.toml; do + [ -f "$src" ] || continue + name="$(basename "$src")" + [ -f "$CODEX_HOME/agents/$name" ] || continue + run rm -f "$CODEX_HOME/agents/$name" + count=$((count + 1)) + done + info "$count agents removed" +} + +# ------------------------------------------------------------------- main + +if [ "$UNINSTALL" -eq 1 ]; then + printf '\033[1mUninstalling VGV AI Flutter Plugin from Codex\033[0m\n' + info "plugin root: $PLUGIN_ROOT" + info "codex home: $CODEX_HOME" + uninstall_skills + uninstall_mcp + uninstall_hooks + uninstall_agents + printf '\nDone. Restart Codex to pick up the change.\n' + exit 0 +fi + +printf '\033[1mInstalling VGV AI Flutter Plugin into Codex\033[0m\n' +info "plugin root: $PLUGIN_ROOT" +info "codex home: $CODEX_HOME" +[ "$DRY_RUN" -eq 1 ] && info "dry run — nothing will be written" + +install_skills +install_mcp +install_hooks +install_agents + +if ! command -v dart &>/dev/null; then + warn "dart is not on your PATH — the analyze and format hooks will do nothing." +fi +if ! command -v very_good &>/dev/null; then + warn "very_good is not on your PATH — install with: dart pub global activate very_good_cli" +fi + +printf '\nDone. Restart Codex to pick up the change.\n' +printf 'Codex asks you to review new hooks before they run; approve them with /hooks.\n' diff --git a/codex/install_test.sh b/codex/install_test.sh new file mode 100755 index 0000000..fba52cd --- /dev/null +++ b/codex/install_test.sh @@ -0,0 +1,228 @@ +#!/bin/bash +# Tests for codex/install.sh +# +# Usage: bash codex/install_test.sh +# +# Every case runs the installer against a throwaway CODEX_HOME and skills +# directory, so nothing touches the real ~/.codex. The `codex` CLI is not +# required — the installer warns and skips the MCP step when it is missing, and +# these tests only assert on the parts that are pure file manipulation (skills, +# hooks, agents). +# +# The hooks merge is the part worth guarding: it edits a shared file that other +# tools also write to, so it has to leave foreign entries alone and it has to be +# idempotent across re-runs. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PLUGIN_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +INSTALLER="$SCRIPT_DIR/install.sh" + +PASSED=0 +FAILED=0 + +pass() { printf " \033[32mPASS\033[0m %s\n" "$1"; PASSED=$((PASSED + 1)); } +fail() { + printf " \033[31mFAIL\033[0m %s\n" "$1" + if [ $# -gt 1 ]; then printf " %s\n" "$2"; fi + FAILED=$((FAILED + 1)) +} + +assert_eq() { + local label="$1" expected="$2" actual="$3" + if [ "$expected" = "$actual" ]; then + pass "$label" + else + fail "$label" "expected [$expected], got [$actual]" + fi +} + +# Run the installer in a fresh sandbox. Sets HOME_DIR / CODEX_DIR / SKILLS_DIR +# for the assertions that follow. +new_sandbox() { + SANDBOX=$(mktemp -d) + CODEX_DIR="$SANDBOX/codex" + SKILLS_DIR="$SANDBOX/skills" + mkdir -p "$CODEX_DIR" +} + +install() { + CODEX_HOME="$CODEX_DIR" bash "$INSTALLER" --skills-dir "$SKILLS_DIR" "$@" >"$SANDBOX/out.log" 2>&1 +} + +hooks_json() { cat "$CODEX_DIR/hooks.json"; } + +# Count handler entries across every event. +count_handlers() { + hooks_json | jq '[.hooks[][].hooks[]] | length' +} + +# Count handlers belonging to this plugin. +count_vgv_handlers() { + hooks_json | jq '[.hooks[][].hooks[] | select(.command | test("hooks/scripts/"))] | length' +} + +cleanup() { [ -n "${SANDBOX:-}" ] && rm -rf "$SANDBOX"; } +trap cleanup EXIT + +skill_count=$(find "$PLUGIN_ROOT/skills" -maxdepth 2 -name SKILL.md | wc -l | tr -d ' ') + +echo "=== Fresh install ===" +new_sandbox +install +assert_eq "installs every skill" "$skill_count" "$(ls "$SKILLS_DIR" | wc -l | tr -d ' ')" +if [ -L "$SKILLS_DIR/bloc" ]; then + pass "skills are symlinked by default" +else + fail "skills are symlinked by default" +fi +if [ -f "$SKILLS_DIR/bloc/SKILL.md" ]; then + pass "a linked skill resolves to its SKILL.md" +else + fail "a linked skill resolves to its SKILL.md" +fi +if [ -f "$CODEX_DIR/agents/flutter-reviewer.toml" ]; then + pass "installs the flutter-reviewer agent" +else + fail "installs the flutter-reviewer agent" +fi +assert_eq "installs 5 hook handlers" "5" "$(count_vgv_handlers)" +assert_eq "hooks.json has no leftover placeholder" "0" \ + "$(hooks_json | grep -c '__VGV_PLUGIN_ROOT__')" +assert_eq "hook commands point at this checkout" "5" \ + "$(hooks_json | jq --arg r "$PLUGIN_ROOT" '[.hooks[][].hooks[] | select(.command | contains($r))] | length')" +assert_eq "PostToolUse matches Codex apply_patch" "apply_patch|Edit|Write" \ + "$(hooks_json | jq -r '.hooks.PostToolUse[0].matcher')" +assert_eq "PreToolUse guards the very-good-cli MCP tools" "1" \ + "$(hooks_json | jq '[.hooks.PreToolUse[] | select(.matcher == "mcp__.*very-good-cli__.*")] | length')" +assert_eq "PreToolUse guards Bash" "1" \ + "$(hooks_json | jq '[.hooks.PreToolUse[] | select(.matcher == "Bash")] | length')" +cleanup + +echo "" +echo "=== --copy ===" +new_sandbox +install --copy +if [ -d "$SKILLS_DIR/bloc" ] && [ ! -L "$SKILLS_DIR/bloc" ]; then + pass "--copy installs real directories" +else + fail "--copy installs real directories" +fi +cleanup + +echo "" +echo "=== Re-running is idempotent ===" +new_sandbox +install +first=$(count_vgv_handlers) +install +assert_eq "handler count is unchanged after a second run" "$first" "$(count_vgv_handlers)" +install +assert_eq "handler count is unchanged after a third run" "$first" "$(count_vgv_handlers)" +cleanup + +echo "" +echo "=== Merging into someone else's hooks.json ===" +new_sandbox +cat > "$CODEX_DIR/hooks.json" <<'EOF' +{ + "description": "someone else's hooks", + "hooks": { + "SessionStart": [ + { "hooks": [ { "type": "command", "command": "bash /opt/other/notify.sh" } ] } + ], + "UserPromptSubmit": [ + { "hooks": [ { "type": "command", "command": "bash /opt/other/prompt.sh" } ] } + ] + } +} +EOF +install +assert_eq "our SessionStart group is added next to theirs" "2" \ + "$(hooks_json | jq '.hooks.SessionStart | length')" +assert_eq "foreign notify.sh is still registered" "1" \ + "$(hooks_json | jq '[.hooks[][].hooks[] | select(.command | contains("other/notify.sh"))] | length')" +assert_eq "foreign UserPromptSubmit event survives" "1" \ + "$(hooks_json | jq '[.hooks[][].hooks[] | select(.command | contains("other/prompt.sh"))] | length')" +assert_eq "unrelated top-level keys survive" "someone else's hooks" \ + "$(hooks_json | jq -r '.description')" +assert_eq "our handlers were added alongside" "5" "$(count_vgv_handlers)" +assert_eq "total handlers is theirs plus ours" "7" "$(count_handlers)" +if ls "$CODEX_DIR"/hooks.json.bak-* >/dev/null 2>&1; then + pass "backs up the previous hooks.json" +else + fail "backs up the previous hooks.json" +fi +# A second run must not duplicate ours or drop theirs. +install +assert_eq "re-run keeps the foreign handlers" "2" \ + "$(hooks_json | jq '[.hooks[][].hooks[] | select(.command | test("/opt/other/"))] | length')" +assert_eq "re-run does not duplicate ours" "5" "$(count_vgv_handlers)" +cleanup + +echo "" +echo "=== --uninstall ===" +new_sandbox +cat > "$CODEX_DIR/hooks.json" <<'EOF' +{ + "hooks": { + "SessionStart": [ + { "hooks": [ { "type": "command", "command": "bash /opt/other/notify.sh" } ] } + ] + } +} +EOF +install +install --uninstall +assert_eq "removes our hook handlers" "0" "$(count_vgv_handlers)" +assert_eq "leaves the foreign handler in place" "1" "$(count_handlers)" +assert_eq "removes the installed skills" "0" "$(ls "$SKILLS_DIR" 2>/dev/null | wc -l | tr -d ' ')" +if [ ! -f "$CODEX_DIR/agents/flutter-reviewer.toml" ]; then + pass "removes the flutter-reviewer agent" +else + fail "removes the flutter-reviewer agent" +fi +cleanup + +echo "" +echo "=== --uninstall on a hooks.json we never touched ===" +new_sandbox +echo '{"hooks":{"Stop":[{"hooks":[{"type":"command","command":"bash /opt/other/stop.sh"}]}]}}' \ + > "$CODEX_DIR/hooks.json" +install --uninstall +assert_eq "leaves it alone" "1" "$(count_handlers)" +cleanup + +echo "" +echo "=== --dry-run ===" +new_sandbox +install --dry-run +if [ ! -e "$CODEX_DIR/hooks.json" ] && [ ! -e "$SKILLS_DIR" ]; then + pass "--dry-run writes nothing" +else + fail "--dry-run writes nothing" +fi +if grep -q 'would install' "$SANDBOX/out.log"; then + pass "--dry-run reports what it would do" +else + fail "--dry-run reports what it would do" +fi +cleanup + +echo "" +echo "=== Bad input ===" +new_sandbox +if install --nonsense; then + fail "rejects an unknown option" +else + pass "rejects an unknown option" +fi +cleanup + +echo "" +echo "=== Results: $PASSED passed, $FAILED failed ===" + +if [ "$FAILED" -gt 0 ]; then + exit 1 +fi diff --git a/codex/loader_test.sh b/codex/loader_test.sh new file mode 100755 index 0000000..d68f957 --- /dev/null +++ b/codex/loader_test.sh @@ -0,0 +1,214 @@ +#!/bin/bash +# Asserts that Codex actually loads what codex/install.sh installs. +# +# Usage: bash codex/loader_test.sh +# +# Requires the `codex` CLI. Everything runs against a throwaway CODEX_HOME and a +# throwaway HOME, so your real Codex configuration is never touched. +# +# The checks that need Codex use `codex debug prompt-input`, which renders the +# model-visible prompt as JSON without contacting a model, so this needs no +# credentials and costs nothing. Codex silently ignores a malformed hooks.json +# and has no validator for agent files, so those two are checked here directly +# rather than through the CLI. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PLUGIN_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" + +PASSED=0 +FAILED=0 +pass() { printf " \033[32mPASS\033[0m %s\n" "$1"; PASSED=$((PASSED + 1)); } +fail() { + printf " \033[31mFAIL\033[0m %s\n" "$1" + if [ $# -gt 1 ]; then printf " %s\n" "$2"; fi + FAILED=$((FAILED + 1)) +} + +for tool in codex jq python3; do + if ! command -v "$tool" &>/dev/null; then + printf "\033[31merror\033[0m %s is required to run the Codex loader test\n" "$tool" >&2 + exit 1 + fi +done + +SANDBOX=$(mktemp -d) +trap 'rm -rf "$SANDBOX"' EXIT +FAKE_HOME="$SANDBOX/home" +FAKE_CODEX_HOME="$SANDBOX/codex" +WORKDIR="$SANDBOX/project" +mkdir -p "$FAKE_HOME" "$FAKE_CODEX_HOME" "$WORKDIR" + +# Codex scans for repo-scoped skills up to the repository root, so the scratch +# project needs to be a git repository for discovery to behave as it would for a +# real user. +git -C "$WORKDIR" init -q + +printf '\033[1mCodex %s\033[0m\n' "$(codex --version 2>/dev/null | head -1)" + +echo "" +echo "=== Install ===" +if CODEX_HOME="$FAKE_CODEX_HOME" bash "$SCRIPT_DIR/install.sh" \ + --skills-dir "$FAKE_HOME/.agents/skills" >"$SANDBOX/install.log" 2>&1; then + pass "codex/install.sh completes" +else + fail "codex/install.sh completes" "$(tail -5 "$SANDBOX/install.log")" + cat "$SANDBOX/install.log" >&2 + exit 1 +fi + +# Everything below runs as if $FAKE_HOME were the user's home directory. +codex_in_sandbox() { + env HOME="$FAKE_HOME" CODEX_HOME="$FAKE_CODEX_HOME" codex "$@" +} + +echo "" +echo "=== Skills load ===" +PROMPT_JSON="$SANDBOX/prompt-input.json" +if (cd "$WORKDIR" && codex_in_sandbox debug prompt-input "hello" >"$PROMPT_JSON" 2>"$SANDBOX/pi.err"); then + pass "codex debug prompt-input succeeds" +else + fail "codex debug prompt-input succeeds" "$(tail -5 "$SANDBOX/pi.err")" + exit 1 +fi + +expected=0 +missing="" +for dir in "$PLUGIN_ROOT"/skills/*/; do + [ -f "$dir/SKILL.md" ] || continue + name="$(basename "$dir")" + expected=$((expected + 1)) + # Codex namespaces a skill when it can resolve a plugin manifest above it, so + # accept both the bare and the namespaced listing. + if ! grep -qE -- "- (vgv-ai-flutter-plugin:)?$name: " "$PROMPT_JSON"; then + missing="$missing $name" + fi +done + +if [ "$expected" -eq 0 ]; then + fail "found skills to check" "no SKILL.md files under $PLUGIN_ROOT/skills" +elif [ -n "$missing" ]; then + fail "all $expected skills appear in the Codex prompt" "missing:$missing" +else + pass "all $expected skills appear in the Codex prompt" +fi + +echo "" +echo "=== Config loads ===" +DOCTOR_JSON="$SANDBOX/doctor.json" +codex_in_sandbox doctor --json >"$DOCTOR_JSON" 2>/dev/null +# `codex doctor` exits non-zero when it cannot find credentials, which is the +# normal state here, so assert on the individual checks instead of the exit code. +for check in config.load mcp.config; do + status=$(jq -r --arg id "$check" '.checks[] | select(.id == $id) | .status' "$DOCTOR_JSON" 2>/dev/null) + if [ "$status" = "ok" ]; then + pass "codex doctor: $check is ok" + else + fail "codex doctor: $check is ok" "got [${status:-no such check}]" + fi +done + +for server in dart very-good-cli; do + if codex_in_sandbox mcp get "$server" >/dev/null 2>&1; then + pass "MCP server '$server' is registered" + else + fail "MCP server '$server' is registered" + fi +done + +echo "" +echo "=== Hooks are well-formed ===" +# Codex ignores a malformed hooks.json without reporting anything, so a broken +# file would disable the whole enforcement layer silently. Check it here. +INSTALLED_HOOKS="$FAKE_CODEX_HOME/hooks.json" +if jq -e . "$INSTALLED_HOOKS" >/dev/null 2>&1; then + pass "installed hooks.json is valid JSON" +else + fail "installed hooks.json is valid JSON" +fi + +for event in SessionStart PreToolUse PostToolUse; do + if jq -e --arg e "$event" '.hooks[$e] | arrays and (length > 0)' "$INSTALLED_HOOKS" >/dev/null 2>&1; then + pass "$event is wired" + else + fail "$event is wired" + fi +done + +bad_shape=$(jq '[.hooks[][] | .hooks[]? | select((.type != "command") or ((.command | type) != "string"))] | length' "$INSTALLED_HOOKS") +if [ "$bad_shape" = "0" ]; then + pass "every handler is a command handler with a string command" +else + fail "every handler is a command handler with a string command" "$bad_shape malformed" +fi + +unresolved=$(grep -c '__VGV_PLUGIN_ROOT__' "$INSTALLED_HOOKS") +if [ "$unresolved" = "0" ]; then + pass "no unresolved plugin-root placeholder" +else + fail "no unresolved plugin-root placeholder" "$unresolved left" +fi + +# Every script a hook points at must exist and be readable, or the hook is a +# silent no-op at runtime. +missing_scripts="" +while IFS= read -r script; do + [ -n "$script" ] || continue + if [ ! -f "$script" ]; then + missing_scripts="$missing_scripts $script" + fi +done < <(jq -r '[.hooks[][] | .hooks[]?.command] | .[]' "$INSTALLED_HOOKS" \ + | sed -n 's/^bash "\(.*\)"$/\1/p') +if [ -z "$missing_scripts" ]; then + pass "every hook script exists on disk" +else + fail "every hook script exists on disk" "missing:$missing_scripts" +fi + +echo "" +echo "=== Agents are well-formed ===" +# Codex ships no validator for custom agent files either. +for agent in "$PLUGIN_ROOT"/codex/agents/*.toml; do + [ -f "$agent" ] || continue + name="$(basename "$agent")" + if python3 - "$agent" <<'PY' +import sys, tomllib +with open(sys.argv[1], "rb") as fh: + data = tomllib.load(fh) +missing = [k for k in ("name", "description", "developer_instructions") if not data.get(k)] +if missing: + print("missing required fields: " + ", ".join(missing), file=sys.stderr) + sys.exit(1) +PY + then + pass "$name parses and has the required fields" + else + fail "$name parses and has the required fields" + fi + + if [ -f "$FAKE_CODEX_HOME/agents/$name" ]; then + pass "$name is installed into CODEX_HOME/agents" + else + fail "$name is installed into CODEX_HOME/agents" + fi +done + +# The read-only reviewer must stay read-only: Codex has no per-agent tool +# allowlist, so the sandbox is the only thing enforcing it. +reviewer="$PLUGIN_ROOT/codex/agents/flutter-reviewer.toml" +if [ -f "$reviewer" ]; then + mode=$(python3 -c 'import sys,tomllib;print(tomllib.load(open(sys.argv[1],"rb")).get("sandbox_mode",""))' "$reviewer") + if [ "$mode" = "read-only" ]; then + pass "flutter-reviewer is sandboxed read-only" + else + fail "flutter-reviewer is sandboxed read-only" "sandbox_mode is [${mode:-unset}]" + fi +fi + +echo "" +echo "=== Results: $PASSED passed, $FAILED failed ===" + +if [ "$FAILED" -gt 0 ]; then + exit 1 +fi diff --git a/config/cspell.json b/config/cspell.json index 876321b..deb0a9a 100644 --- a/config/cspell.json +++ b/config/cspell.json @@ -52,6 +52,7 @@ "pubspecs", "redirections", "rubric", + "sandboxed", "serialization", "snackbars", "stdio", diff --git a/hooks/scripts/analyze.sh b/hooks/scripts/analyze.sh index bba41ca..6fa45c7 100755 --- a/hooks/scripts/analyze.sh +++ b/hooks/scripts/analyze.sh @@ -10,16 +10,26 @@ if ! command -v jq &>/dev/null; then exit 0 fi -# Extract file path from the tool input -file_path=$(jq -r '.tool_input.file_path // empty' <<< "$input") +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=hooks/scripts/hook-payload-common.sh +source "$SCRIPT_DIR/hook-payload-common.sh" -# Skip if no file path or not a Dart file -if [[ -z "$file_path" || "$file_path" != *.dart ]]; then +# Collect the Dart files this edit touched. Claude Code reports one `file_path`; +# Codex reports an apply_patch envelope that may cover several files. +files=() +while IFS= read -r file; do + if [ -n "$file" ]; then + files+=("$file") + fi +done < <(changed_dart_files "$input") + +# Nothing Dart in this edit +if [ ${#files[@]} -eq 0 ]; then exit 0 fi -# Run dart analyze on the single file -output=$(dart analyze "$file_path" 2>&1) || { +# Run dart analyze on the changed files +output=$(dart analyze "${files[@]}" 2>&1) || { echo "$output" >&2 exit 2 -} \ No newline at end of file +} diff --git a/hooks/scripts/format.sh b/hooks/scripts/format.sh index e58b601..a4497e4 100755 --- a/hooks/scripts/format.sh +++ b/hooks/scripts/format.sh @@ -10,13 +10,23 @@ if ! command -v jq &>/dev/null; then exit 0 fi -# Extract file path from the tool input -file_path=$(jq -r '.tool_input.file_path // empty' <<< "$input") +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=hooks/scripts/hook-payload-common.sh +source "$SCRIPT_DIR/hook-payload-common.sh" -# Skip if no file path or not a Dart file -if [[ -z "$file_path" || "$file_path" != *.dart ]]; then +# Collect the Dart files this edit touched. Claude Code reports one `file_path`; +# Codex reports an apply_patch envelope that may cover several files. +files=() +while IFS= read -r file; do + if [ -n "$file" ]; then + files+=("$file") + fi +done < <(changed_dart_files "$input") + +# Nothing Dart in this edit +if [ ${#files[@]} -eq 0 ]; then exit 0 fi -# Run dart format on the single file (auto-fix, always exit 0) -dart format "$file_path" &>/dev/null || true \ No newline at end of file +# Run dart format on the changed files (auto-fix, always exit 0) +dart format "${files[@]}" &>/dev/null || true diff --git a/hooks/scripts/hook-payload-common.sh b/hooks/scripts/hook-payload-common.sh new file mode 100755 index 0000000..a34b18a --- /dev/null +++ b/hooks/scripts/hook-payload-common.sh @@ -0,0 +1,92 @@ +#!/bin/bash +# Shared helpers for reading a hook payload that may come from either harness. +# +# Claude Code and Codex describe the same edit differently: +# +# Claude Code Edit / Write -> .tool_input.file_path (one path) +# Codex apply_patch -> .tool_input.command (an apply_patch envelope) +# +# changed_dart_files() normalizes both into a newline-separated list of existing +# `.dart` paths, so analyze.sh and format.sh stay single-sourced across harnesses. +# +# Every branch is written with `if` rather than `&&` so that sourcing this file +# from a script running under `set -e` cannot abort on a false test. + +# Print the paths an apply_patch envelope creates or updates, one per line. +# +# $1 = the raw envelope. +# +# Grammar (from the Codex apply_patch parser): +# begin_patch: "*** Begin Patch" LF +# add_hunk: "*** Add File: " filename LF add_line+ +# delete_hunk: "*** Delete File: " filename LF +# update_hunk: "*** Update File: " filename LF change_move? change? +# change_move: "*** Move to: " filename LF +# +# Deleted files are skipped — there is nothing left to analyze or format. For a +# renamed file the `Move to:` destination wins, because that is the path on disk +# once the patch lands. +apply_patch_paths() { + printf '%s\n' "$1" | awk ' + function flush() { if (path != "") { print path; path = "" } } + /^\*\*\* (Add|Update) File: / { flush(); path = substr($0, index($0, ": ") + 2); next } + /^\*\*\* Move to: / { path = substr($0, index($0, ": ") + 2); next } + /^\*\*\* Delete File: / { flush(); next } + /^\*\*\* End Patch/ { flush(); next } + END { flush() } + ' +} + +# Print the `.dart` files a hook payload touched, one per line. +# +# $1 = the raw hook payload JSON. Only files that exist on disk are printed, so a +# deleted or moved-away path never reaches `dart analyze`. +changed_dart_files() { + local input="$1" + local file_path command cwd path + + # Claude Code: a single explicit path. + file_path=$(printf '%s' "$input" | jq -r '.tool_input.file_path // empty') + if [ -n "$file_path" ]; then + case "$file_path" in + *.dart) + if [ -f "$file_path" ]; then + printf '%s\n' "$file_path" + fi + ;; + esac + return 0 + fi + + # Codex: an apply_patch envelope in `command`. + command=$(printf '%s' "$input" | jq -r '.tool_input.command // empty') + case "$command" in + '*** Begin Patch'*) ;; + *) return 0 ;; + esac + + cwd=$(printf '%s' "$input" | jq -r '.cwd // empty') + while IFS= read -r path; do + if [ -z "$path" ]; then + continue + fi + case "$path" in + *.dart) ;; + *) continue ;; + esac + # apply_patch paths may be relative to the session working directory. + case "$path" in + /*) ;; + *) + if [ -n "$cwd" ]; then + path="$cwd/$path" + fi + ;; + esac + if [ -f "$path" ]; then + printf '%s\n' "$path" + fi + done < <(apply_patch_paths "$command") + + return 0 +} diff --git a/hooks/scripts/hook-payload-common_test.sh b/hooks/scripts/hook-payload-common_test.sh new file mode 100755 index 0000000..89cd833 --- /dev/null +++ b/hooks/scripts/hook-payload-common_test.sh @@ -0,0 +1,170 @@ +#!/bin/bash +# Tests for hook-payload-common.sh +# +# Usage: bash hooks/scripts/hook-payload-common_test.sh +# +# changed_dart_files() reads a hook payload and prints the `.dart` files it +# touched. It has to read both shapes the two harnesses produce: +# +# Claude Code Edit / Write -> .tool_input.file_path +# Codex apply_patch -> .tool_input.command (an apply_patch envelope) +# +# Each case builds real files in a temp tree, because the helper only reports +# paths that exist on disk. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=hooks/scripts/hook-payload-common.sh +source "$SCRIPT_DIR/hook-payload-common.sh" + +PASSED=0 +FAILED=0 + +WORK=$(mktemp -d) +trap 'rm -rf "$WORK"' EXIT + +mkdir -p "$WORK/lib" +: > "$WORK/lib/a.dart" +: > "$WORK/lib/b.dart" +: > "$WORK/lib/renamed.dart" +: > "$WORK/README.md" + +# Compare the helper's output (sorted) against the expected newline-separated list. +assert_files() { + local label="$1" payload="$2" expected="$3" + local actual + actual=$(changed_dart_files "$payload" | LC_ALL=C sort | tr '\n' ' ') + expected=$(printf '%s' "$expected" | tr '\n' ' ') + if [ "$actual" = "$expected" ]; then + printf " \033[32mPASS\033[0m %s\n" "$label" + PASSED=$((PASSED + 1)) + else + printf " \033[31mFAIL\033[0m %s\n expected: [%s]\n actual: [%s]\n" \ + "$label" "$expected" "$actual" + FAILED=$((FAILED + 1)) + fi +} + +patch_payload() { + jq -n --arg c "$1" --arg d "$WORK" '{"cwd":$d,"tool_input":{"command":$c}}' +} + +echo "=== Claude Code payloads (tool_input.file_path) ===" + +assert_files "Edit on a .dart file" \ + "$(jq -n --arg p "$WORK/lib/a.dart" '{"tool_input":{"file_path":$p}}')" \ + "$WORK/lib/a.dart " + +assert_files "Write to a non-Dart file is ignored" \ + "$(jq -n --arg p "$WORK/README.md" '{"tool_input":{"file_path":$p}}')" \ + "" + +assert_files "path that does not exist is ignored" \ + "$(jq -n --arg p "$WORK/lib/gone.dart" '{"tool_input":{"file_path":$p}}')" \ + "" + +assert_files "empty payload" '{}' "" + +echo "" +echo "=== Codex payloads (tool_input.command, apply_patch envelope) ===" + +assert_files "Update File with an absolute path" \ + "$(patch_payload "*** Begin Patch +*** Update File: $WORK/lib/a.dart +@@ +- print(\"hi\"); ++ print(\"bye\"); +*** End Patch")" \ + "$WORK/lib/a.dart " + +assert_files "Update File with a path relative to cwd" \ + "$(patch_payload '*** Begin Patch +*** Update File: lib/a.dart +@@ +-old ++new +*** End Patch')" \ + "$WORK/lib/a.dart " + +assert_files "Add File" \ + "$(patch_payload '*** Begin Patch +*** Add File: lib/b.dart ++void main() {} +*** End Patch')" \ + "$WORK/lib/b.dart " + +assert_files "several files in one patch" \ + "$(patch_payload '*** Begin Patch +*** Add File: lib/b.dart ++void main() {} +*** Update File: lib/a.dart +@@ +-old ++new +*** End Patch')" \ + "$WORK/lib/a.dart $WORK/lib/b.dart " + +assert_files "Delete File is skipped" \ + "$(patch_payload '*** Begin Patch +*** Delete File: lib/a.dart +*** End Patch')" \ + "" + +assert_files "Delete File does not swallow the preceding file" \ + "$(patch_payload '*** Begin Patch +*** Update File: lib/a.dart +@@ +-old ++new +*** Delete File: lib/gone.dart +*** End Patch')" \ + "$WORK/lib/a.dart " + +assert_files "Move to wins over the original path" \ + "$(patch_payload '*** Begin Patch +*** Update File: lib/gone.dart +*** Move to: lib/renamed.dart +@@ +-old ++new +*** End Patch')" \ + "$WORK/lib/renamed.dart " + +assert_files "non-Dart files in a patch are ignored" \ + "$(patch_payload '*** Begin Patch +*** Update File: README.md +@@ +-old ++new +*** End Patch')" \ + "" + +assert_files "a patch that touches nothing that exists" \ + "$(patch_payload '*** Begin Patch +*** Update File: lib/nope.dart +@@ +-old ++new +*** End Patch')" \ + "" + +echo "" +echo "=== Payloads that must not be read as a patch ===" + +# A Bash command that merely mentions a .dart file must never be treated as an +# edit — otherwise the PostToolUse hooks would fire on every shell call. +assert_files "a Bash command is not an apply_patch envelope" \ + "$(patch_payload 'cat lib/a.dart')" \ + "" + +assert_files "a shell command quoting patch markers is not an envelope" \ + "$(patch_payload 'echo "*** Begin Patch"')" \ + "" + +echo "" +echo "=== Results: $PASSED passed, $FAILED failed ===" + +if [ "$FAILED" -gt 0 ]; then + exit 1 +fi From 634f913a4c941484bdc002a1e2561598a4f0dfc2 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Tue, 8 Sep 2026 15:14:20 +0200 Subject: [PATCH 02/18] fix: make the Codex loader test portable across environments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real problems the first CI run surfaced. `codex doctor` reports `mcp.config` as a warning, not ok, when the server executables are absent — the normal state on a runner with no Dart SDK. The check now treats a warning as acceptable and asserts the registration itself instead, so a warning can no longer hide a server pointing at the wrong command. `tomllib` needs Python 3.11, so the agent-file parsing died with a traceback on older interpreters (macOS system Python is 3.9). The parser is now resolved once in the preflight, falling back to tomli, with an actionable message when neither is available. Co-Authored-By: Claude Opus 5 --- CONTRIBUTING.md | 6 ++- codex/loader_test.sh | 102 +++++++++++++++++++++++++++++++++---------- config/cspell.json | 2 + 3 files changed, 86 insertions(+), 24 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5ac041d..e5519a9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -253,8 +253,10 @@ session and exercise it before you commit. - **Dart SDK** and **jq** on your `PATH` — the hooks need both. - **Very Good CLI** ≥ 1.3.0 (`dart pub global activate very_good_cli`) for the Very Good CLI MCP server tools. -- **Codex CLI** (`npm install -g @openai/codex`) only if you touch `codex/` — - `codex/loader_test.sh` needs it. Everything else runs without it. +- **Codex CLI** (`npm install -g @openai/codex`) and **Python 3.11+** only if you + touch `codex/` — `codex/loader_test.sh` needs both (Python parses the agent + TOML; on 3.10 or older, `python3 -m pip install tomli`). Everything else runs + without them. See the README [Hooks](README.md#hooks) and [MCP Integration](README.md#mcp-integration) sections for the full prerequisite details. diff --git a/codex/loader_test.sh b/codex/loader_test.sh index d68f957..1c293a5 100755 --- a/codex/loader_test.sh +++ b/codex/loader_test.sh @@ -33,6 +33,21 @@ for tool in codex jq python3; do fi done +# The agent files are TOML and Codex ships no validator for them, so they are +# parsed here. tomllib is standard from Python 3.11; older versions need tomli. +TOML_MODULE="" +for candidate in tomllib tomli; do + if python3 -c "import $candidate" 2>/dev/null; then + TOML_MODULE="$candidate" + break + fi +done +if [ -z "$TOML_MODULE" ]; then + printf "\033[31merror\033[0m no TOML parser available for %s\n" "$(python3 -V 2>&1)" >&2 + printf " use Python 3.11+ (stdlib tomllib), or: python3 -m pip install tomli\n" >&2 + exit 1 +fi + SANDBOX=$(mktemp -d) trap 'rm -rf "$SANDBOX"' EXIT FAKE_HOME="$SANDBOX/home" @@ -100,22 +115,56 @@ DOCTOR_JSON="$SANDBOX/doctor.json" codex_in_sandbox doctor --json >"$DOCTOR_JSON" 2>/dev/null # `codex doctor` exits non-zero when it cannot find credentials, which is the # normal state here, so assert on the individual checks instead of the exit code. -for check in config.load mcp.config; do - status=$(jq -r --arg id "$check" '.checks[] | select(.id == $id) | .status' "$DOCTOR_JSON" 2>/dev/null) - if [ "$status" = "ok" ]; then - pass "codex doctor: $check is ok" - else - fail "codex doctor: $check is ok" "got [${status:-no such check}]" - fi -done +doctor_status() { + jq -r --arg id "$1" '.checks[] | select(.id == $id) | .status' "$DOCTOR_JSON" 2>/dev/null +} -for server in dart very-good-cli; do - if codex_in_sandbox mcp get "$server" >/dev/null 2>&1; then - pass "MCP server '$server' is registered" - else +# config.load proves the TOML the installer wrote actually parses. +status=$(doctor_status config.load) +if [ "$status" = "ok" ]; then + pass "codex doctor: config.load is ok" +else + fail "codex doctor: config.load is ok" "got [${status:-no such check}]" +fi + +# mcp.config downgrades to a warning when the server executables are missing, +# which is the normal state anywhere without the Dart SDK and Very Good CLI +# installed — CI runners included. Only a hard failure means the config is +# wrong; what the servers were registered *as* is asserted below instead. +status=$(doctor_status mcp.config) +case "$status" in + ok) + pass "codex doctor: mcp.config is ok" + ;; + warning) + pass "codex doctor: mcp.config has no errors (warning, likely no dart/very_good on PATH)" + ;; + *) + fail "codex doctor: mcp.config has no errors" "got [${status:-no such check}]" + ;; +esac + +# Assert what each server was registered as, so a warning above can never hide a +# server pointing at the wrong command. +assert_mcp_server() { + local server="$1" want_command="$2" want_args="$3" out + out=$(codex_in_sandbox mcp get "$server" 2>/dev/null) + if [ -z "$out" ]; then fail "MCP server '$server' is registered" + return fi -done + pass "MCP server '$server' is registered" + for field in "enabled: true" "transport: stdio" "command: $want_command" "args: $want_args"; do + if printf '%s\n' "$out" | grep -qF "$field"; then + pass " $server $field" + else + fail " $server $field" "$(printf '%s' "$out" | tr '\n' ' ')" + fi + done +} + +assert_mcp_server dart dart "mcp-server --enable dart_format" +assert_mcp_server very-good-cli very_good "mcp" echo "" echo "=== Hooks are well-formed ===" @@ -168,19 +217,28 @@ fi echo "" echo "=== Agents are well-formed ===" -# Codex ships no validator for custom agent files either. + +# Print one top-level key from an agent file, or nothing if it is absent. +agent_field() { + python3 -c " +import sys, $TOML_MODULE as toml +with open(sys.argv[1], 'rb') as fh: + print(toml.load(fh).get(sys.argv[2], '')) +" "$1" "$2" 2>/dev/null +} + for agent in "$PLUGIN_ROOT"/codex/agents/*.toml; do [ -f "$agent" ] || continue name="$(basename "$agent")" - if python3 - "$agent" <<'PY' -import sys, tomllib -with open(sys.argv[1], "rb") as fh: - data = tomllib.load(fh) -missing = [k for k in ("name", "description", "developer_instructions") if not data.get(k)] + if python3 -c " +import sys, $TOML_MODULE as toml +with open(sys.argv[1], 'rb') as fh: + data = toml.load(fh) +missing = [k for k in ('name', 'description', 'developer_instructions') if not data.get(k)] if missing: - print("missing required fields: " + ", ".join(missing), file=sys.stderr) + print('missing required fields: ' + ', '.join(missing), file=sys.stderr) sys.exit(1) -PY +" "$agent" then pass "$name parses and has the required fields" else @@ -198,7 +256,7 @@ done # allowlist, so the sandbox is the only thing enforcing it. reviewer="$PLUGIN_ROOT/codex/agents/flutter-reviewer.toml" if [ -f "$reviewer" ]; then - mode=$(python3 -c 'import sys,tomllib;print(tomllib.load(open(sys.argv[1],"rb")).get("sandbox_mode",""))' "$reviewer") + mode=$(agent_field "$reviewer" sandbox_mode) if [ "$mode" = "read-only" ]; then pass "flutter-reviewer is sandboxed read-only" else diff --git a/config/cspell.json b/config/cspell.json index deb0a9a..a05558b 100644 --- a/config/cspell.json +++ b/config/cspell.json @@ -60,6 +60,8 @@ "subagents", "subclassing", "tappable", + "tomli", + "tomllib", "tooltipped", "unmirrored", "unrouted", From 921fdb504c836fb72aff150371609659bdd6b049 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Tue, 8 Sep 2026 15:34:51 +0200 Subject: [PATCH 03/18] refactor: install into Codex natively instead of via a script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex has its own plugin system, so the installer was unnecessary. Adding `.codex-plugin/plugin.json` and a marketplace entry in `.agents/plugins/marketplace.json` makes this repo a Codex plugin, and the install becomes the same two commands as Claude Code: codex plugin marketplace add VeryGoodOpenSource/vgv-ai-flutter-plugin codex plugin add vgv-ai-flutter-plugin@very-good-ventures Codex then reads `skills/`, `.mcp.json`, and `hooks/hooks.json` from the very files Claude Code uses, resolving `${CLAUDE_PLUGIN_ROOT}` as a compatibility alias for the installed plugin directory. So `codex/hooks.json` and `codex/config.toml` are gone — there is no second copy of the hooks and no MCP block to keep in sync. The only change to the shared wiring is widening the PostToolUse matcher to `apply_patch|Edit|Write`, which is inert on Claude Code. A plugin cannot ship a Codex subagent: agents load only from `~/.codex/agents/` or a project's `.codex/agents/`, and `agents` is neither a manifest field nor a discovery path. `codex/agents/flutter-reviewer.toml` stays as a file users copy, and it is all that is left in `codex/`. release-please now bumps both manifests, and the loader test fails if their versions drift. The loader test itself was rewritten to install the working tree the way a user does and assert what Codex picked up: 35 assertions, up from 17. Deletes codex/install.sh, codex/install_test.sh, codex/hooks.json, codex/config.toml. Co-Authored-By: Claude Opus 5 --- .agents/plugins/marketplace.json | 20 ++ .codex-plugin/plugin.json | 28 +++ .github/workflows/ci.yaml | 2 - .release-please-config.json | 32 +++- AGENTS.md | 40 ++-- CLAUDE.md | 18 +- CONTRIBUTING.md | 85 +++++---- README.md | 59 +++--- codex/config.toml | 26 --- codex/hooks.json | 60 ------ codex/install.sh | 308 ------------------------------- codex/install_test.sh | 228 ----------------------- codex/loader_test.sh | 236 ++++++++++++++--------- hooks/hooks.json | 2 +- 14 files changed, 331 insertions(+), 813 deletions(-) create mode 100644 .agents/plugins/marketplace.json create mode 100644 .codex-plugin/plugin.json delete mode 100644 codex/config.toml delete mode 100644 codex/hooks.json delete mode 100755 codex/install.sh delete mode 100755 codex/install_test.sh diff --git a/.agents/plugins/marketplace.json b/.agents/plugins/marketplace.json new file mode 100644 index 0000000..3fd5806 --- /dev/null +++ b/.agents/plugins/marketplace.json @@ -0,0 +1,20 @@ +{ + "name": "very-good-ventures", + "interface": { + "displayName": "Very Good Ventures" + }, + "plugins": [ + { + "name": "vgv-ai-flutter-plugin", + "source": { + "source": "local", + "path": "." + }, + "policy": { + "installation": "AVAILABLE", + "authentication": "ON_INSTALL" + }, + "category": "Productivity" + } + ] +} diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json new file mode 100644 index 0000000..c705bfd --- /dev/null +++ b/.codex-plugin/plugin.json @@ -0,0 +1,28 @@ +{ + "name": "vgv-ai-flutter-plugin", + "version": "0.0.5", + "description": "Best-practice skills for Flutter and Dart development from Very Good Ventures.", + "author": { + "name": "Very Good Ventures", + "email": "hello@verygood.ventures", + "url": "https://verygood.ventures" + }, + "homepage": "https://github.com/VeryGoodOpenSource/vgv-ai-flutter-plugin", + "repository": "https://github.com/VeryGoodOpenSource/vgv-ai-flutter-plugin", + "license": "MIT", + "mcpServers": "./.mcp.json", + "interface": { + "displayName": "VGV AI Flutter Plugin", + "shortDescription": "Flutter and Dart best practices from Very Good Ventures", + "longDescription": "Best-practice skills for Flutter and Dart covering accessibility, animations, BLoC, testing, theming, navigation, security, internationalization, layered architecture, license compliance, UI packages, project creation, SDK/lint upgrades, and an autonomous quality-gate loop that drives analyze, format, test, and coverage to green — plus automated dart analyze and format hooks.", + "developerName": "Very Good Ventures", + "category": "Productivity", + "capabilities": ["Write"], + "defaultPrompt": [ + "Create a new Flutter app with Very Good CLI", + "Add a bloc for user authentication", + "Drive this package to green" + ], + "websiteURL": "https://verygood.ventures" + } +} diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a063647..eb907f4 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -96,8 +96,6 @@ jobs: echo "::endgroup::" done exit $status - - name: Codex installer tests - run: bash codex/install_test.sh codex-loader: name: 🤖 Codex Loader runs-on: ubuntu-latest diff --git a/.release-please-config.json b/.release-please-config.json index 9dbfd32..15c43cf 100644 --- a/.release-please-config.json +++ b/.release-please-config.json @@ -1,13 +1,28 @@ { "$schema": "https://raw.githubusercontent.com/googleapis/release-please/main/schemas/config.json", "changelog-sections": [ - { "type": "feat", "section": "Features" }, - { "type": "fix", "section": "Bug Fixes" }, - { "type": "refactor", "section": "Refactors" }, - { "type": "chore", "section": "Miscellaneous Chores" }, - { "type": "docs", "section": "Docs" } + { + "type": "feat", + "section": "Features" + }, + { + "type": "fix", + "section": "Bug Fixes" + }, + { + "type": "refactor", + "section": "Refactors" + }, + { + "type": "chore", + "section": "Miscellaneous Chores" + }, + { + "type": "docs", + "section": "Docs" + } ], - "pull-request-header": ":rotating_light: There are changes ready for release :rocket:\n\nℹ Merge this PR once the team confirms the release is ready.\n", + "pull-request-header": ":rotating_light: There are changes ready for release :rocket:\n\n\u2139 Merge this PR once the team confirms the release is ready.\n", "pull-request-title-pattern": "chore: ${version}", "extra-label": "no-auto-update", "include-component-in-tag": false, @@ -22,6 +37,11 @@ "type": "json", "path": ".claude-plugin/plugin.json", "jsonpath": "$.version" + }, + { + "type": "json", + "path": ".codex-plugin/plugin.json", + "jsonpath": "$.version" } ] } diff --git a/AGENTS.md b/AGENTS.md index 1d1d106..d6f9ae3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,19 +7,20 @@ VGV AI Flutter Plugin provides best-practices skills for Flutter and Dart develo ## Repository Structure ```text -.mcp.json # MCP server configuration (Dart and Very Good CLI) +.mcp.json # MCP server configuration (Dart and Very Good CLI); read by both harnesses +.agents/ + plugins/ + marketplace.json # Codex marketplace entry, so `codex plugin add` can install this repo .claude-plugin/ - plugin.json # Plugin manifest (name, version, keywords) + plugin.json # Claude Code plugin manifest (name, version, keywords) +.codex-plugin/ + plugin.json # Codex plugin manifest (interface metadata + mcpServers -> ./.mcp.json) agents/ flutter-reviewer.md # Read-only Flutter code reviewer subagent -codex/ # Codex wiring — the harness reads skills directly, the rest is installed - config.toml # ~/.codex/config.toml reference: dart + very-good-cli MCP, hooks feature - hooks.json # Codex hook definitions (__VGV_PLUGIN_ROOT__ substituted at install time) - install.sh # Installs skills, MCP, hooks, and agents into $CODEX_HOME - install_test.sh # Tests install.sh, including the non-destructive hooks.json merge - loader_test.sh # Asserts Codex actually loads all of it (run by the codex-loader CI job) +codex/ # The only Codex-specific assets; skills, MCP and hooks are shared + loader_test.sh # Installs the repo as a Codex plugin and asserts it loads (codex-loader CI job) agents/ - flutter-reviewer.toml # Codex port of agents/flutter-reviewer.md + flutter-reviewer.toml # Codex port of agents/flutter-reviewer.md — users copy it to ~/.codex/agents/ docs/ plan/ # Planning and design documents evals/ @@ -176,13 +177,17 @@ documentation in the same change: automatically, so verify each one by hand. - **Adding or changing a hook** in `hooks/hooks.json` — update the **Hooks** section in `README.md` (and the `## Hooks` section in `CLAUDE.md` if behavior - changes), and mirror the change in `codex/hooks.json`. The two files wire up the - same scripts and nothing keeps them in sync; `codex/loader_test.sh` only checks - that whatever `codex/hooks.json` names actually exists on disk. + changes). The file is shared with Codex, so keep any `PostToolUse` matcher + covering `apply_patch` as well as `Edit|Write`, or the hook stops firing there. - **Adding or changing an MCP tool** — update the **MCP Integration** tools table in `README.md`, and check whether any skill's `allowed-tools` names a tool that - was renamed or removed. Nothing validates those names. A new **server** also has - to be registered for Codex, in both `codex/config.toml` and `codex/install.sh`. + was renamed or removed. Nothing validates those names. A new **server** goes in + `.mcp.json` only; both harnesses read that file. +- **Editing either plugin manifest** — `.claude-plugin/plugin.json` and + `.codex-plugin/plugin.json` describe the same plugin. Keep + `interface.longDescription` in the Codex manifest in step with `description` in + the Claude Code one, and leave `version` to release-please, which bumps both. + `codex/loader_test.sh` fails if the versions drift. - **Changing what a hook script reads from its payload** — the two harnesses describe an edit differently (Claude Code `tool_input.file_path`, Codex `tool_input.command` holding an apply_patch envelope). `hook-payload-common.sh` @@ -190,9 +195,10 @@ documentation in the same change: branching per harness in `analyze.sh` or `format.sh`, and add a case to `hook-payload-common_test.sh`. - **Changing `agents/flutter-reviewer.md`** — port the same change to - `codex/agents/flutter-reviewer.toml`. Its output contract (the four-column - findings table) is consumed verbatim by callers on both harnesses, so the two - must not drift. + `codex/agents/flutter-reviewer.toml`. A Codex plugin cannot ship a subagent, so + that file is a separate copy users install by hand. Its output contract (the + four-column findings table) is consumed verbatim by callers on both harnesses, + so the two must not drift. ## Evals diff --git a/CLAUDE.md b/CLAUDE.md index 9d726e4..7d55f47 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,8 +31,8 @@ from `vgv-cli-common.sh`. The following hook is **agent-scoped** — it is decla These run **after** a tool call completes: -- `Edit|Write` matcher → `analyze.sh` — runs `dart analyze` on the modified `.dart` file(s); exits 2 on failure (blocking — Claude must fix the issue) -- `Edit|Write` matcher → `format.sh` — runs `dart format` on the modified `.dart` file(s); always exits 0 (non-blocking) +- `apply_patch|Edit|Write` matcher → `analyze.sh` — runs `dart analyze` on the modified `.dart` file(s); exits 2 on failure (blocking — Claude must fix the issue) +- `apply_patch|Edit|Write` matcher → `format.sh` — runs `dart format` on the modified `.dart` file(s); always exits 0 (non-blocking) Both read the changed files through `hook-payload-common.sh`, which handles Claude Code's `tool_input.file_path` and Codex's `tool_input.command` (an `apply_patch` envelope, which can @@ -42,9 +42,13 @@ All hook scripts require **jq** to parse the hook payload (they skip gracefully ### Codex -`codex/` holds the Codex-side wiring: `codex/hooks.json` mirrors `hooks/hooks.json` with -`${CLAUDE_PLUGIN_ROOT}` replaced at install time and `Edit|Write` widened to -`apply_patch|Edit|Write`, and `codex/agents/flutter-reviewer.toml` ports the reviewer agent. -`codex/install.sh` installs skills, MCP servers, hooks, and agents; `codex/loader_test.sh` proves -Codex loads them. Change a hook or the reviewer agent and you have to change both harnesses — see +This repo is a Codex plugin too. `.codex-plugin/plugin.json` plus the marketplace entry in +`.agents/plugins/marketplace.json` let `codex plugin add` install it, and Codex then reads +`skills/`, `.mcp.json`, and this same `hooks/hooks.json` — resolving `${CLAUDE_PLUGIN_ROOT}` as a +compatibility alias. That is why the `PostToolUse` matcher says `apply_patch|Edit|Write`: Codex +names its file-editing tool `apply_patch`, and the extra alternative is inert on Claude Code. + +The only Codex-specific asset is `codex/agents/flutter-reviewer.toml`, because a plugin cannot ship +a Codex subagent. `codex/loader_test.sh` installs the repo the way a user would and asserts Codex +picks it all up. Change a hook or the reviewer agent and both harnesses are affected — see `AGENTS.md` → Maintaining Existing Skills, Hooks, and MCP Tools. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e5519a9..2723b9f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -193,43 +193,52 @@ frontmatter equivalent, and `interface.short_description`, which takes precedenc spec-legal `metadata: short-description` key. The `SKILL.md` body stays the one source of truth; the sidecar is thin, with no build step. Add one for every new skill. -**Codex runtime (`codex/`)** — skills reach Codex through the standard, but hooks, MCP, and -subagents do not, so `codex/` carries that wiring and `codex/install.sh` applies it. Verified -against Codex CLI 0.153.4: - +**Codex runtime** — Codex has its own plugin system, and this repo is a Codex plugin as well as a +Claude Code one. `.codex-plugin/plugin.json` plus the marketplace entry in +`.agents/plugins/marketplace.json` are all it takes; Codex then reads `skills/`, `.mcp.json`, and +`hooks/hooks.json` from the very files Claude Code uses. There is no install script and no second +copy of the hooks. Verified against Codex CLI 0.153.4: + +- **Two manifests, one source of truth.** `.codex-plugin/plugin.json` carries only what Codex needs + that Claude Code's manifest cannot express — the `interface` block and `mcpServers: "./.mcp.json"`, + which is what pulls the MCP servers in. Its `version` is bumped by release-please alongside + `.claude-plugin/plugin.json` (both are listed under `extra-files`), and `codex/loader_test.sh` + fails if the two drift. Keep `interface.longDescription` in step with the `description` in + `.claude-plugin/plugin.json`. `keywords` is deliberately **not** duplicated: it only affects + plugin search, and a second copy of a 50-plus entry list would rot. +- **The plugin manifest rejects a `hooks` field.** Hooks arrive purely through default discovery at + `/hooks/hooks.json`, and Codex resolves `${CLAUDE_PLUGIN_ROOT}` inside it as a + compatibility alias for the installed plugin directory. That is why the Claude Code hooks file + works unchanged — do not add a `hooks` key to `.codex-plugin/plugin.json`, validation refuses it. - **Hooks are a stable, default-on feature**, not experimental. The flag is `[features] hooks` - (`codex features list` shows it enabled); there is no `codex_hooks` flag. Codex also runs hooks - on Windows and offers a `commandWindows` override — but these scripts are `bash` and need `jq`, - so Windows means WSL or Git Bash. Codex **silently ignores a malformed `hooks.json`**, which - disables the whole enforcement layer with no error, so `codex/loader_test.sh` validates the - installed file directly. -- **Four of the six scripts port unchanged.** Codex passes `tool_name: "Bash"` with - `tool_input.command` as a plain string, and accepts the same `permissionDecision` allow/deny - JSON, so `check-vgv-cli.sh`, `block-cli-workarounds.sh`, and `allow-readonly-git.sh` need no - edits; plain stdout from a `SessionStart` hook is injected as a developer message exactly as on - Claude Code, so `warn-missing-mcp.sh` ports as-is too. Only the edit hooks differ: Codex's - file-editing tool is `apply_patch` and it hands the hook the raw patch, with no `file_path` and - no changed-file list, so `hook-payload-common.sh` parses the envelope. Keep that difference in - that one file. -- **`${CLAUDE_PLUGIN_ROOT}` is not available** to hooks in `~/.codex/hooks.json` (Codex resolves it - only for hooks that come from an installed Codex *plugin*, which this repo is not). The installer - substitutes the checkout path for `__VGV_PLUGIN_ROOT__` instead. -- **MCP goes through `codex mcp add`**, not a hand-written TOML block, so it merges rather than - replacing a user's config. `codex/config.toml` documents the same thing for anyone doing it by - hand. -- **The subagent trades a tool allowlist for a sandbox.** Codex custom agents are standalone TOML - in `~/.codex/agents/` (or project-scoped `.codex/agents/`) needing `name`, `description`, and + (`codex features list` shows it enabled); there is no `codex_hooks` flag. Codex also runs hooks on + Windows and offers a `commandWindows` override — but these scripts are `bash` and need `jq`, so + Windows means WSL or Git Bash. Codex **silently ignores a malformed `hooks.json`**, which disables + the whole enforcement layer with no error, so `codex/loader_test.sh` validates the installed file. +- **Five of the six scripts need nothing.** Codex passes `tool_name: "Bash"` with + `tool_input.command` as a plain string and accepts the same `permissionDecision` allow/deny JSON, + so `check-vgv-cli.sh`, `block-cli-workarounds.sh`, and `allow-readonly-git.sh` are untouched; + plain stdout from a `SessionStart` hook is injected as a developer message exactly as on Claude + Code, so `warn-missing-mcp.sh` is too. Only the edit hooks differ: Codex's file-editing tool is + `apply_patch`, so the `PostToolUse` matcher reads `apply_patch|Edit|Write` (the extra alternative + is inert on Claude Code), and the payload hands over the raw patch with no `file_path` and no + changed-file list, so `hook-payload-common.sh` parses the envelope. Keep that difference in that + one file. +- **A plugin cannot ship a Codex subagent.** Codex loads custom agents only from `~/.codex/agents/` + or a project's `.codex/agents/`, and `agents` is not a plugin manifest field or a discovery path. + `codex/agents/flutter-reviewer.toml` is therefore a file users copy, and it is the only thing left + in `codex/`. Codex custom agents are standalone TOML needing `name`, `description`, and `developer_instructions`, plus any `config.toml` key. There is no per-agent tool allowlist and no - agent-scoped `PreToolUse` hook, so `flutter-reviewer` sets `sandbox_mode = "read-only"` to hold - the read-only contract that `allow-readonly-git.sh` holds on Claude Code. Codex ships no - validator for agent files, so `codex/loader_test.sh` parses them and asserts that - `sandbox_mode` is still `read-only`. + agent-scoped `PreToolUse` hook, so it sets `sandbox_mode = "read-only"` to hold the read-only + contract that `allow-readonly-git.sh` holds on Claude Code. Codex ships no validator for agent + files, so the loader test parses them and asserts `sandbox_mode` is still `read-only`. - **Do not weaken the Claude Code path** to make Codex simpler. `hooks/hooks.json` and - `agents/flutter-reviewer.md` stay authoritative; `codex/` mirrors them. + `agents/flutter-reviewer.md` stay authoritative. -Run `bash codex/loader_test.sh` before pushing a change to any of it. It needs the `codex` CLI but -no credentials — it asserts through `codex debug prompt-input`, which renders the model-visible -prompt without calling a model. +Run `bash codex/loader_test.sh` before pushing a change to any of it. It installs the working tree +the way a user would — `codex plugin marketplace add` then `codex plugin add`, into a throwaway +`CODEX_HOME` — and asserts what Codex picked up. It needs the `codex` CLI but no credentials, since +it reads `codex debug prompt-input` and `codex doctor --json` rather than calling a model. **Invocation** — every skill in this plugin is **model-invoked**: the model may reach for it autonomously when the context fits (that is the point of a best-practice skill), so neither @@ -254,9 +263,9 @@ session and exercise it before you commit. - **Very Good CLI** ≥ 1.3.0 (`dart pub global activate very_good_cli`) for the Very Good CLI MCP server tools. - **Codex CLI** (`npm install -g @openai/codex`) and **Python 3.11+** only if you - touch `codex/` — `codex/loader_test.sh` needs both (Python parses the agent - TOML; on 3.10 or older, `python3 -m pip install tomli`). Everything else runs - without them. + touch the Codex manifests, the hooks, or `codex/` — `codex/loader_test.sh` needs + both (Python parses the agent TOML; on 3.10 or older, + `python3 -m pip install tomli`). Everything else runs without them. See the README [Hooks](README.md#hooks) and [MCP Integration](README.md#mcp-integration) sections for the full prerequisite details. @@ -350,8 +359,8 @@ Every pull request runs the following checks automatically: | Spelling | Runs cspell on all `*.md` files | `config/cspell.json` | | Skill validation | Validates **every** `SKILL.md`'s frontmatter and structure against the Agent Skills spec, so a malformed skill fails the build instead of silently vanishing on another host | `Flash-Brew-Digital/validate-skill@v1` | | Plugin validation | Validates and test-installs the plugin | `claude plugin validate .` | -| Script tests | Runs every hook script test suite, plus the Codex installer's | `hooks/scripts/*_test.sh`, `codex/install_test.sh` | -| Codex loader | Installs the plugin into a throwaway Codex home and asserts all 15 skills, both MCP servers, the hooks, and the reviewer agent load | `codex/loader_test.sh` | +| Script tests | Runs every hook script test suite | `hooks/scripts/*_test.sh` | +| Codex loader | Installs the plugin as a Codex plugin into a throwaway Codex home and asserts all 15 skills, both MCP servers, and the hooks load | `codex/loader_test.sh` | Evals do **not** run on a pull request. They call real models, so they run after a merge to `main` instead, scoped to the skills that changed: diff --git a/README.md b/README.md index 6f0160f..64e26df 100644 --- a/README.md +++ b/README.md @@ -73,11 +73,11 @@ This plugin includes SessionStart, PreToolUse, and PostToolUse hooks that valida | **Check VGV CLI** (`check-vgv-cli.sh`) | PreToolUse (`mcp__.*very-good-cli__.*`) | Auto-approves Very Good CLI MCP tool calls in every run mode via a PreToolUse `allow` decision, so they never dead-end when the tool isn't on `permissions.allow` (including under `skipAutoPermissionPrompt`); denies with an install/upgrade message if the CLI is missing or < 1.3.0 | | **Block CLI Workarounds** (`block-cli-workarounds.sh`) | PreToolUse (`Bash`) | Blocks direct CLI bypass of Very Good CLI commands through the Bash tool; exits 2 on failure (blocking) | | **Allow Read-only Git** (`allow-readonly-git.sh`) | PreToolUse (`Bash`, `flutter-reviewer` agent only) | Restricts the `flutter-reviewer` agent's Bash to `git diff`/`git status`; exits 2 on anything else (blocking). Scoped via the agent's frontmatter, not `hooks.json` | -| **Analyze** (`analyze.sh`) | PostToolUse (`Edit`/`Write`) | Runs `dart analyze` on the modified `.dart` file; exits 2 on failure (blocking — Claude must fix issues before continuing) | -| **Format** (`format.sh`) | PostToolUse (`Edit`/`Write`) | Runs `dart format` on the modified `.dart` file; always exits 0 (non-blocking — formatting is applied silently) | +| **Analyze** (`analyze.sh`) | PostToolUse (`apply_patch`/`Edit`/`Write`) | Runs `dart analyze` on the modified `.dart` file; exits 2 on failure (blocking — Claude must fix issues before continuing) | +| **Format** (`format.sh`) | PostToolUse (`apply_patch`/`Edit`/`Write`) | Runs `dart format` on the modified `.dart` file; always exits 0 (non-blocking — formatting is applied silently) | -The triggers above are the Claude Code ones. The same scripts run on Codex — see [Codex](#codex) -for the wiring and the two behavioral differences. +Codex runs this same `hooks/hooks.json` and these same scripts — `apply_patch` is its file-editing +tool, which is why that matcher covers it. See [Codex](#codex) for the differences. ### Prerequisites @@ -86,39 +86,41 @@ for the wiring and the two behavioral differences. ## Codex -The skills follow the [Agent Skills open standard][agent_skills_link], so Codex loads them from -`~/.agents/skills/` with no adapter. The MCP servers, hooks, and reviewer agent need wiring up -once: +Codex has its own plugin system, so installation mirrors the Claude Code flow — two commands, no +scripts: ```bash -git clone https://github.com/VeryGoodOpenSource/vgv-ai-flutter-plugin.git && bash vgv-ai-flutter-plugin/codex/install.sh +codex plugin marketplace add VeryGoodOpenSource/vgv-ai-flutter-plugin && codex plugin add vgv-ai-flutter-plugin@very-good-ventures ``` -| Component | Where it lands | Notes | -| --------- | -------------- | ----- | -| Skills | `~/.agents/skills/` | Symlinked to the checkout, so `git pull` updates them. Use `--copy` for real copies | -| MCP servers | `~/.codex/config.toml` | `dart` and `very-good-cli`, registered with `codex mcp add` | -| Hooks | `~/.codex/hooks.json` | Merged into whatever is already there, never overwritten | -| Reviewer agent | `~/.codex/agents/flutter-reviewer.toml` | Ask Codex to spawn `flutter-reviewer` | +That one install gives you the skills, both MCP servers, and the hooks. Codex reads them from the +same files Claude Code does — `skills/`, `.mcp.json`, and `hooks/hooks.json` — via +`.codex-plugin/plugin.json`. Restart Codex afterwards, then approve the hooks with `/hooks`, since +Codex requires a review before a hook runs for the first time. -Restart Codex afterwards, then approve the new hooks with `/hooks` — Codex requires a review before -a hook runs for the first time. Re-running the installer replaces what it installed before instead -of adding a second copy. `--dry-run` prints the changes without making them, and `--uninstall` -reverses all four steps. +The reviewer agent is the one piece a plugin cannot carry, because Codex only loads custom agents +from `~/.codex/agents/` or a project's `.codex/agents/`. Copy it in once: + +```bash +mkdir -p ~/.codex/agents && cp codex/agents/flutter-reviewer.toml ~/.codex/agents/ +``` + +Then ask Codex to spawn `flutter-reviewer`. ### How Codex differs from Claude Code -- **The hooks are the same scripts.** Only the wiring differs. Codex calls its file-editing tool - `apply_patch` and hands the hook a raw patch rather than a file path, so `analyze.sh` and - `format.sh` read both shapes; `${CLAUDE_PLUGIN_ROOT}` means nothing to Codex, so the installer - bakes the checkout path into `hooks.json`. -- **The reviewer agent is sandboxed instead of tool-restricted.** On Claude Code `flutter-reviewer` - has no write tools and an agent-scoped hook limits its Bash to `git diff`/`git status`. Codex has - no per-agent tool allowlist, so the agent declares `sandbox_mode = "read-only"` — the OS refuses - every write, which covers the same "never edits files" guarantee. +- **The hooks are the same files.** `hooks/hooks.json` and every script under `hooks/scripts/` are + shared. Codex resolves `${CLAUDE_PLUGIN_ROOT}` as a compatibility alias for the installed plugin + directory, and it calls its file-editing tool `apply_patch` and hands the hook a raw patch rather + than a file path — so the `PostToolUse` matcher covers `apply_patch` and `analyze.sh` / + `format.sh` read both payload shapes. +- **The reviewer agent is sandboxed instead of tool-restricted.** On Claude Code + `flutter-reviewer` has no write tools and an agent-scoped hook limits its Bash to + `git diff`/`git status`. Codex has no per-agent tool allowlist, so the agent declares + `sandbox_mode = "read-only"` — the OS refuses every write, which covers the same "never edits + files" guarantee. - **Hooks are on by default.** They are a stable Codex feature; `codex features list` shows - `hooks` enabled. The installer pins `[features] hooks = true` only in case something in your - config had turned it off. + `hooks` enabled. - **Windows needs a POSIX shell.** Codex itself runs hooks on Windows, but every script here is `bash` and needs `jq`, so run Codex under WSL or Git Bash. @@ -235,7 +237,6 @@ On Codex the same two servers are registered in `~/.codex/config.toml` instead [Codex](#codex). Skills that drive an MCP tool always name the equivalent `very_good`, `dart`, or `flutter` command as a fallback, so they keep working on a host where neither server is connected. -[agent_skills_link]: https://agentskills.io/specification [marketplace_link]: https://github.com/VeryGoodOpenSource/very-good-claude-code-marketplace [claude_code_link]: https://claude.ai/code [vgv_link]: https://verygood.ventures diff --git a/codex/config.toml b/codex/config.toml deleted file mode 100644 index fae005a..0000000 --- a/codex/config.toml +++ /dev/null @@ -1,26 +0,0 @@ -# Codex configuration for the VGV AI Flutter Plugin. -# -# This is the `~/.codex/config.toml` equivalent of the repository-root `.mcp.json` -# that Claude Code reads. `codex/install.sh` applies all of it for you through the -# supported `codex mcp add` and `codex features enable` commands, which merge into -# an existing config instead of replacing it. Merge it by hand only if you would -# rather not run the installer. - -# The Dart and Flutter MCP server ships with the Dart SDK. The `cli` feature -# category is off by default, so `dart_format` has to be enabled explicitly — -# the `green-gate` skill's format gate calls that tool. Test execution stays -# disabled on purpose; the Very Good CLI `test` tool is used instead. -[mcp_servers.dart] -command = "dart" -args = ["mcp-server", "--enable", "dart_format"] - -# Very Good CLI >= 1.3.0 must be on your PATH: -# dart pub global activate very_good_cli -[mcp_servers.very-good-cli] -command = "very_good" -args = ["mcp"] - -# Hooks are stable and enabled by default in Codex. Pinning the flag here only -# matters if something in your config or profile previously set it to false. -[features] -hooks = true diff --git a/codex/hooks.json b/codex/hooks.json deleted file mode 100644 index 6d246dd..0000000 --- a/codex/hooks.json +++ /dev/null @@ -1,60 +0,0 @@ -{ - "description": "VGV AI Flutter Plugin hooks for Dart and Flutter development (Codex)", - "hooks": { - "SessionStart": [ - { - "hooks": [ - { - "type": "command", - "command": "bash \"__VGV_PLUGIN_ROOT__/hooks/scripts/warn-missing-mcp.sh\"", - "statusMessage": "Checking Very Good CLI", - "timeout": 10 - } - ] - } - ], - "PreToolUse": [ - { - "matcher": "mcp__.*very-good-cli__.*", - "hooks": [ - { - "type": "command", - "command": "bash \"__VGV_PLUGIN_ROOT__/hooks/scripts/check-vgv-cli.sh\"", - "statusMessage": "Verifying Very Good CLI version", - "timeout": 10 - } - ] - }, - { - "matcher": "Bash", - "hooks": [ - { - "type": "command", - "command": "bash \"__VGV_PLUGIN_ROOT__/hooks/scripts/block-cli-workarounds.sh\"", - "statusMessage": "Checking for CLI bypass", - "timeout": 10 - } - ] - } - ], - "PostToolUse": [ - { - "matcher": "apply_patch|Edit|Write", - "hooks": [ - { - "type": "command", - "command": "bash \"__VGV_PLUGIN_ROOT__/hooks/scripts/analyze.sh\"", - "statusMessage": "dart analyze", - "timeout": 30 - }, - { - "type": "command", - "command": "bash \"__VGV_PLUGIN_ROOT__/hooks/scripts/format.sh\"", - "statusMessage": "dart format", - "timeout": 30 - } - ] - } - ] - } -} diff --git a/codex/install.sh b/codex/install.sh deleted file mode 100755 index f36e40b..0000000 --- a/codex/install.sh +++ /dev/null @@ -1,308 +0,0 @@ -#!/bin/bash -# Install the VGV AI Flutter Plugin into Codex. -# -# Codex has no marketplace entry for this plugin, so the four pieces are wired up -# individually: -# -# skills -> ~/.agents/skills/ (symlinked, or copied with --copy) -# MCP -> ~/.codex/config.toml (via `codex mcp add`) -# hooks -> ~/.codex/hooks.json (merged, never overwritten) -# agent -> ~/.codex/agents/.toml -# -# Re-running is safe: every step replaces what a previous run installed rather -# than stacking a second copy. -# -# Usage: -# bash codex/install.sh [options] -# -# Options: -# --skills-dir DIR Where to install skills (default: $HOME/.agents/skills) -# --copy Copy skills instead of symlinking the checkout -# --dry-run Print what would change and exit -# --uninstall Remove everything this script installs -# -h, --help Show this help -# -# Environment: -# CODEX_HOME Codex config directory (default: $HOME/.codex) - -set -uo pipefail - -PLUGIN_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -CODEX_HOME="${CODEX_HOME:-$HOME/.codex}" -SKILLS_DIR="${HOME}/.agents/skills" -INSTALL_MODE="link" -DRY_RUN=0 -UNINSTALL=0 - -# Handler commands this script owns. Anything else in hooks.json is left alone. -VGV_HOOK_PATTERN='hooks/scripts/(warn-missing-mcp|check-vgv-cli|block-cli-workarounds|analyze|format)\.sh' - -# Print the header comment block as help text. -usage() { - awk 'NR > 1 && /^#/ { sub(/^# ?/, ""); print; next } NR > 1 { exit }' "${BASH_SOURCE[0]}" -} - -while [ $# -gt 0 ]; do - case "$1" in - --skills-dir) SKILLS_DIR="$2"; shift 2 ;; - --copy) INSTALL_MODE="copy"; shift ;; - --dry-run) DRY_RUN=1; shift ;; - --uninstall) UNINSTALL=1; shift ;; - -h|--help) usage; exit 0 ;; - *) echo "unknown option: $1" >&2; usage >&2; exit 2 ;; - esac -done - -info() { printf ' %s\n' "$1"; } -step() { printf '\n\033[1m%s\033[0m\n' "$1"; } -warn() { printf ' \033[33mwarning\033[0m %s\n' "$1" >&2; } -die() { printf '\033[31merror\033[0m %s\n' "$1" >&2; exit 1; } -run() { if [ "$DRY_RUN" -eq 1 ]; then info "would run: $*"; else "$@"; fi; } - -if ! command -v jq &>/dev/null; then - die "jq is required (the hooks parse their payload with it). Install jq and re-run." -fi - -HAS_CODEX=1 -if ! command -v codex &>/dev/null; then - HAS_CODEX=0 -fi - -# ---------------------------------------------------------------- skills - -install_skills() { - step "Skills -> $SKILLS_DIR" - if [ "$DRY_RUN" -eq 0 ]; then - mkdir -p "$SKILLS_DIR" || die "cannot create $SKILLS_DIR" - fi - local src name dest count=0 - for src in "$PLUGIN_ROOT"/skills/*/; do - [ -f "$src/SKILL.md" ] || continue - name="$(basename "$src")" - dest="$SKILLS_DIR/$name" - if [ "$DRY_RUN" -eq 1 ]; then - info "would install $name ($INSTALL_MODE)" - else - rm -rf "$dest" - if [ "$INSTALL_MODE" = "copy" ]; then - cp -R "${src%/}" "$dest" || die "failed to copy $name" - else - ln -s "${src%/}" "$dest" || die "failed to link $name" - fi - fi - count=$((count + 1)) - done - info "$count skills ($INSTALL_MODE)" -} - -uninstall_skills() { - step "Removing skills from $SKILLS_DIR" - local src name dest count=0 - for src in "$PLUGIN_ROOT"/skills/*/; do - name="$(basename "$src")" - dest="$SKILLS_DIR/$name" - [ -e "$dest" ] || [ -L "$dest" ] || continue - run rm -rf "$dest" - count=$((count + 1)) - done - info "$count skills removed" -} - -# ------------------------------------------------------------------- MCP - -install_mcp() { - step "MCP servers -> $CODEX_HOME/config.toml" - if [ "$HAS_CODEX" -eq 0 ]; then - warn "codex is not on your PATH; skipping MCP setup." - warn "Merge codex/config.toml into $CODEX_HOME/config.toml by hand." - return - fi - run env CODEX_HOME="$CODEX_HOME" codex mcp add dart -- dart mcp-server --enable dart_format \ - || warn "could not register the dart MCP server" - run env CODEX_HOME="$CODEX_HOME" codex mcp add very-good-cli -- very_good mcp \ - || warn "could not register the very-good-cli MCP server" - run env CODEX_HOME="$CODEX_HOME" codex features enable hooks \ - || warn "could not pin the hooks feature (it is on by default)" -} - -uninstall_mcp() { - step "Removing MCP servers from $CODEX_HOME/config.toml" - if [ "$HAS_CODEX" -eq 0 ]; then - warn "codex is not on your PATH; remove [mcp_servers.dart] and" - warn "[mcp_servers.very-good-cli] from $CODEX_HOME/config.toml by hand." - return - fi - run env CODEX_HOME="$CODEX_HOME" codex mcp remove dart >/dev/null 2>&1 - run env CODEX_HOME="$CODEX_HOME" codex mcp remove very-good-cli >/dev/null 2>&1 - info "dart and very-good-cli removed" -} - -# ----------------------------------------------------------------- hooks - -# Merge our hook handlers into an existing hooks.json without disturbing anyone -# else's. Handlers this script installed are stripped first, so re-running -# replaces them instead of appending a duplicate. -merge_hooks() { - local existing="$1" incoming="$2" - jq -n \ - --slurpfile cur "$existing" \ - --slurpfile new "$incoming" \ - --arg pattern "$VGV_HOOK_PATTERN" ' - def is_vgv: (.command // "") | test($pattern); - def strip_vgv: - map(.hooks = ((.hooks // []) | map(select(is_vgv | not)))) - | map(select((.hooks | length) > 0)); - - ($cur[0] // {}) as $base - | ($new[0].hooks // {}) as $add - | ( - ($base.hooks // {}) - | with_entries(.value |= strip_vgv) - | with_entries(select((.value | length) > 0)) - ) as $stripped - | $base - + { hooks: ( - reduce ($add | to_entries[]) as $e ($stripped; - .[$e.key] = ((.[$e.key] // []) + $e.value)) - ) } - ' -} - -strip_hooks() { - local existing="$1" - jq --arg pattern "$VGV_HOOK_PATTERN" ' - def is_vgv: (.command // "") | test($pattern); - def strip_vgv: - map(.hooks = ((.hooks // []) | map(select(is_vgv | not)))) - | map(select((.hooks | length) > 0)); - .hooks = ((.hooks // {}) - | with_entries(.value |= strip_vgv) - | with_entries(select((.value | length) > 0))) - ' "$existing" -} - -# Write $2 over $1, keeping a timestamped backup of whatever was there. -write_hooks_file() { - local target="$1" content="$2" - if [ "$DRY_RUN" -eq 1 ]; then - info "would write $target:" - printf '%s\n' "$content" | sed 's/^/ /' - return - fi - mkdir -p "$(dirname "$target")" - if [ -f "$target" ]; then - local backup="$target.bak-$(date +%Y%m%d%H%M%S)" - cp "$target" "$backup" && info "backed up to $backup" - fi - printf '%s\n' "$content" > "$target.tmp" && mv "$target.tmp" "$target" -} - -install_hooks() { - step "Hooks -> $CODEX_HOME/hooks.json" - local template="$PLUGIN_ROOT/codex/hooks.json" - [ -f "$template" ] || die "missing $template" - - # ${CLAUDE_PLUGIN_ROOT} is resolved by Claude Code and means nothing to Codex, - # so the absolute path to this checkout is baked in at install time. - local resolved - resolved=$(jq --arg root "$PLUGIN_ROOT" \ - 'walk(if type == "string" then gsub("__VGV_PLUGIN_ROOT__"; $root) else . end)' \ - "$template") || die "could not read $template" - - local target="$CODEX_HOME/hooks.json" - local scratch current incoming merged - scratch=$(mktemp -d) || die "could not create a temp directory" - incoming="$scratch/incoming.json" - printf '%s\n' "$resolved" > "$incoming" - if [ -f "$target" ]; then - current="$target" - else - current="$scratch/current.json" - echo '{}' > "$current" - fi - - merged=$(merge_hooks "$current" "$incoming") - local status=$? - rm -rf "$scratch" - [ $status -eq 0 ] || die "could not merge $target" - write_hooks_file "$target" "$merged" - info "SessionStart, PreToolUse (2), PostToolUse (2)" -} - -uninstall_hooks() { - step "Removing hooks from $CODEX_HOME/hooks.json" - local target="$CODEX_HOME/hooks.json" - if [ ! -f "$target" ]; then - info "nothing to remove" - return - fi - local stripped - stripped=$(strip_hooks "$target") || die "could not rewrite $target" - write_hooks_file "$target" "$stripped" -} - -# ----------------------------------------------------------------- agents - -install_agents() { - step "Agents -> $CODEX_HOME/agents" - local src name count=0 - for src in "$PLUGIN_ROOT"/codex/agents/*.toml; do - [ -f "$src" ] || continue - name="$(basename "$src")" - if [ "$DRY_RUN" -eq 1 ]; then - info "would install $name" - else - mkdir -p "$CODEX_HOME/agents" - cp "$src" "$CODEX_HOME/agents/$name" || die "failed to install $name" - fi - count=$((count + 1)) - done - info "$count agents" -} - -uninstall_agents() { - step "Removing agents from $CODEX_HOME/agents" - local src name count=0 - for src in "$PLUGIN_ROOT"/codex/agents/*.toml; do - [ -f "$src" ] || continue - name="$(basename "$src")" - [ -f "$CODEX_HOME/agents/$name" ] || continue - run rm -f "$CODEX_HOME/agents/$name" - count=$((count + 1)) - done - info "$count agents removed" -} - -# ------------------------------------------------------------------- main - -if [ "$UNINSTALL" -eq 1 ]; then - printf '\033[1mUninstalling VGV AI Flutter Plugin from Codex\033[0m\n' - info "plugin root: $PLUGIN_ROOT" - info "codex home: $CODEX_HOME" - uninstall_skills - uninstall_mcp - uninstall_hooks - uninstall_agents - printf '\nDone. Restart Codex to pick up the change.\n' - exit 0 -fi - -printf '\033[1mInstalling VGV AI Flutter Plugin into Codex\033[0m\n' -info "plugin root: $PLUGIN_ROOT" -info "codex home: $CODEX_HOME" -[ "$DRY_RUN" -eq 1 ] && info "dry run — nothing will be written" - -install_skills -install_mcp -install_hooks -install_agents - -if ! command -v dart &>/dev/null; then - warn "dart is not on your PATH — the analyze and format hooks will do nothing." -fi -if ! command -v very_good &>/dev/null; then - warn "very_good is not on your PATH — install with: dart pub global activate very_good_cli" -fi - -printf '\nDone. Restart Codex to pick up the change.\n' -printf 'Codex asks you to review new hooks before they run; approve them with /hooks.\n' diff --git a/codex/install_test.sh b/codex/install_test.sh deleted file mode 100755 index fba52cd..0000000 --- a/codex/install_test.sh +++ /dev/null @@ -1,228 +0,0 @@ -#!/bin/bash -# Tests for codex/install.sh -# -# Usage: bash codex/install_test.sh -# -# Every case runs the installer against a throwaway CODEX_HOME and skills -# directory, so nothing touches the real ~/.codex. The `codex` CLI is not -# required — the installer warns and skips the MCP step when it is missing, and -# these tests only assert on the parts that are pure file manipulation (skills, -# hooks, agents). -# -# The hooks merge is the part worth guarding: it edits a shared file that other -# tools also write to, so it has to leave foreign entries alone and it has to be -# idempotent across re-runs. - -set -uo pipefail - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -PLUGIN_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" -INSTALLER="$SCRIPT_DIR/install.sh" - -PASSED=0 -FAILED=0 - -pass() { printf " \033[32mPASS\033[0m %s\n" "$1"; PASSED=$((PASSED + 1)); } -fail() { - printf " \033[31mFAIL\033[0m %s\n" "$1" - if [ $# -gt 1 ]; then printf " %s\n" "$2"; fi - FAILED=$((FAILED + 1)) -} - -assert_eq() { - local label="$1" expected="$2" actual="$3" - if [ "$expected" = "$actual" ]; then - pass "$label" - else - fail "$label" "expected [$expected], got [$actual]" - fi -} - -# Run the installer in a fresh sandbox. Sets HOME_DIR / CODEX_DIR / SKILLS_DIR -# for the assertions that follow. -new_sandbox() { - SANDBOX=$(mktemp -d) - CODEX_DIR="$SANDBOX/codex" - SKILLS_DIR="$SANDBOX/skills" - mkdir -p "$CODEX_DIR" -} - -install() { - CODEX_HOME="$CODEX_DIR" bash "$INSTALLER" --skills-dir "$SKILLS_DIR" "$@" >"$SANDBOX/out.log" 2>&1 -} - -hooks_json() { cat "$CODEX_DIR/hooks.json"; } - -# Count handler entries across every event. -count_handlers() { - hooks_json | jq '[.hooks[][].hooks[]] | length' -} - -# Count handlers belonging to this plugin. -count_vgv_handlers() { - hooks_json | jq '[.hooks[][].hooks[] | select(.command | test("hooks/scripts/"))] | length' -} - -cleanup() { [ -n "${SANDBOX:-}" ] && rm -rf "$SANDBOX"; } -trap cleanup EXIT - -skill_count=$(find "$PLUGIN_ROOT/skills" -maxdepth 2 -name SKILL.md | wc -l | tr -d ' ') - -echo "=== Fresh install ===" -new_sandbox -install -assert_eq "installs every skill" "$skill_count" "$(ls "$SKILLS_DIR" | wc -l | tr -d ' ')" -if [ -L "$SKILLS_DIR/bloc" ]; then - pass "skills are symlinked by default" -else - fail "skills are symlinked by default" -fi -if [ -f "$SKILLS_DIR/bloc/SKILL.md" ]; then - pass "a linked skill resolves to its SKILL.md" -else - fail "a linked skill resolves to its SKILL.md" -fi -if [ -f "$CODEX_DIR/agents/flutter-reviewer.toml" ]; then - pass "installs the flutter-reviewer agent" -else - fail "installs the flutter-reviewer agent" -fi -assert_eq "installs 5 hook handlers" "5" "$(count_vgv_handlers)" -assert_eq "hooks.json has no leftover placeholder" "0" \ - "$(hooks_json | grep -c '__VGV_PLUGIN_ROOT__')" -assert_eq "hook commands point at this checkout" "5" \ - "$(hooks_json | jq --arg r "$PLUGIN_ROOT" '[.hooks[][].hooks[] | select(.command | contains($r))] | length')" -assert_eq "PostToolUse matches Codex apply_patch" "apply_patch|Edit|Write" \ - "$(hooks_json | jq -r '.hooks.PostToolUse[0].matcher')" -assert_eq "PreToolUse guards the very-good-cli MCP tools" "1" \ - "$(hooks_json | jq '[.hooks.PreToolUse[] | select(.matcher == "mcp__.*very-good-cli__.*")] | length')" -assert_eq "PreToolUse guards Bash" "1" \ - "$(hooks_json | jq '[.hooks.PreToolUse[] | select(.matcher == "Bash")] | length')" -cleanup - -echo "" -echo "=== --copy ===" -new_sandbox -install --copy -if [ -d "$SKILLS_DIR/bloc" ] && [ ! -L "$SKILLS_DIR/bloc" ]; then - pass "--copy installs real directories" -else - fail "--copy installs real directories" -fi -cleanup - -echo "" -echo "=== Re-running is idempotent ===" -new_sandbox -install -first=$(count_vgv_handlers) -install -assert_eq "handler count is unchanged after a second run" "$first" "$(count_vgv_handlers)" -install -assert_eq "handler count is unchanged after a third run" "$first" "$(count_vgv_handlers)" -cleanup - -echo "" -echo "=== Merging into someone else's hooks.json ===" -new_sandbox -cat > "$CODEX_DIR/hooks.json" <<'EOF' -{ - "description": "someone else's hooks", - "hooks": { - "SessionStart": [ - { "hooks": [ { "type": "command", "command": "bash /opt/other/notify.sh" } ] } - ], - "UserPromptSubmit": [ - { "hooks": [ { "type": "command", "command": "bash /opt/other/prompt.sh" } ] } - ] - } -} -EOF -install -assert_eq "our SessionStart group is added next to theirs" "2" \ - "$(hooks_json | jq '.hooks.SessionStart | length')" -assert_eq "foreign notify.sh is still registered" "1" \ - "$(hooks_json | jq '[.hooks[][].hooks[] | select(.command | contains("other/notify.sh"))] | length')" -assert_eq "foreign UserPromptSubmit event survives" "1" \ - "$(hooks_json | jq '[.hooks[][].hooks[] | select(.command | contains("other/prompt.sh"))] | length')" -assert_eq "unrelated top-level keys survive" "someone else's hooks" \ - "$(hooks_json | jq -r '.description')" -assert_eq "our handlers were added alongside" "5" "$(count_vgv_handlers)" -assert_eq "total handlers is theirs plus ours" "7" "$(count_handlers)" -if ls "$CODEX_DIR"/hooks.json.bak-* >/dev/null 2>&1; then - pass "backs up the previous hooks.json" -else - fail "backs up the previous hooks.json" -fi -# A second run must not duplicate ours or drop theirs. -install -assert_eq "re-run keeps the foreign handlers" "2" \ - "$(hooks_json | jq '[.hooks[][].hooks[] | select(.command | test("/opt/other/"))] | length')" -assert_eq "re-run does not duplicate ours" "5" "$(count_vgv_handlers)" -cleanup - -echo "" -echo "=== --uninstall ===" -new_sandbox -cat > "$CODEX_DIR/hooks.json" <<'EOF' -{ - "hooks": { - "SessionStart": [ - { "hooks": [ { "type": "command", "command": "bash /opt/other/notify.sh" } ] } - ] - } -} -EOF -install -install --uninstall -assert_eq "removes our hook handlers" "0" "$(count_vgv_handlers)" -assert_eq "leaves the foreign handler in place" "1" "$(count_handlers)" -assert_eq "removes the installed skills" "0" "$(ls "$SKILLS_DIR" 2>/dev/null | wc -l | tr -d ' ')" -if [ ! -f "$CODEX_DIR/agents/flutter-reviewer.toml" ]; then - pass "removes the flutter-reviewer agent" -else - fail "removes the flutter-reviewer agent" -fi -cleanup - -echo "" -echo "=== --uninstall on a hooks.json we never touched ===" -new_sandbox -echo '{"hooks":{"Stop":[{"hooks":[{"type":"command","command":"bash /opt/other/stop.sh"}]}]}}' \ - > "$CODEX_DIR/hooks.json" -install --uninstall -assert_eq "leaves it alone" "1" "$(count_handlers)" -cleanup - -echo "" -echo "=== --dry-run ===" -new_sandbox -install --dry-run -if [ ! -e "$CODEX_DIR/hooks.json" ] && [ ! -e "$SKILLS_DIR" ]; then - pass "--dry-run writes nothing" -else - fail "--dry-run writes nothing" -fi -if grep -q 'would install' "$SANDBOX/out.log"; then - pass "--dry-run reports what it would do" -else - fail "--dry-run reports what it would do" -fi -cleanup - -echo "" -echo "=== Bad input ===" -new_sandbox -if install --nonsense; then - fail "rejects an unknown option" -else - pass "rejects an unknown option" -fi -cleanup - -echo "" -echo "=== Results: $PASSED passed, $FAILED failed ===" - -if [ "$FAILED" -gt 0 ]; then - exit 1 -fi diff --git a/codex/loader_test.sh b/codex/loader_test.sh index 1c293a5..324a4dc 100755 --- a/codex/loader_test.sh +++ b/codex/loader_test.sh @@ -1,21 +1,26 @@ #!/bin/bash -# Asserts that Codex actually loads what codex/install.sh installs. +# Asserts that Codex loads this plugin through its own native install path. # # Usage: bash codex/loader_test.sh # -# Requires the `codex` CLI. Everything runs against a throwaway CODEX_HOME and a -# throwaway HOME, so your real Codex configuration is never touched. +# Installs the working tree as a Codex plugin into a throwaway CODEX_HOME — +# `codex plugin marketplace add` then `codex plugin add`, the same two commands +# a user runs — and then checks what Codex actually picked up. Your real Codex +# configuration is never touched. # -# The checks that need Codex use `codex debug prompt-input`, which renders the -# model-visible prompt as JSON without contacting a model, so this needs no -# credentials and costs nothing. Codex silently ignores a malformed hooks.json -# and has no validator for agent files, so those two are checked here directly -# rather than through the CLI. +# Needs the `codex` CLI but no credentials: the assertions go through +# `codex debug prompt-input` and `codex doctor --json`, which render local state +# without contacting a model. +# +# Codex silently ignores a malformed hooks.json and ships no validator for agent +# files, so those two are checked here directly rather than through the CLI. set -uo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" PLUGIN_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +MARKETPLACE="$PLUGIN_ROOT/.agents/plugins/marketplace.json" +MANIFEST="$PLUGIN_ROOT/.codex-plugin/plugin.json" PASSED=0 FAILED=0 @@ -25,6 +30,9 @@ fail() { if [ $# -gt 1 ]; then printf " %s\n" "$2"; fi FAILED=$((FAILED + 1)) } +assert_eq() { + if [ "$2" = "$3" ]; then pass "$1"; else fail "$1" "expected [$2], got [$3]"; fi +} for tool in codex jq python3; do if ! command -v "$tool" &>/dev/null; then @@ -56,27 +64,65 @@ WORKDIR="$SANDBOX/project" mkdir -p "$FAKE_HOME" "$FAKE_CODEX_HOME" "$WORKDIR" # Codex scans for repo-scoped skills up to the repository root, so the scratch -# project needs to be a git repository for discovery to behave as it would for a -# real user. +# project is a git repository, matching what a real user would have. git -C "$WORKDIR" init -q +codex_in_sandbox() { + env HOME="$FAKE_HOME" CODEX_HOME="$FAKE_CODEX_HOME" codex "$@" +} + printf '\033[1mCodex %s\033[0m\n' "$(codex --version 2>/dev/null | head -1)" echo "" -echo "=== Install ===" -if CODEX_HOME="$FAKE_CODEX_HOME" bash "$SCRIPT_DIR/install.sh" \ - --skills-dir "$FAKE_HOME/.agents/skills" >"$SANDBOX/install.log" 2>&1; then - pass "codex/install.sh completes" +echo "=== Manifests ===" +MARKETPLACE_NAME=$(jq -r '.name // empty' "$MARKETPLACE" 2>/dev/null) +PLUGIN_NAME=$(jq -r '.name // empty' "$MANIFEST" 2>/dev/null) +if [ -n "$MARKETPLACE_NAME" ]; then + pass "marketplace.json is valid JSON (name: $MARKETPLACE_NAME)" else - fail "codex/install.sh completes" "$(tail -5 "$SANDBOX/install.log")" - cat "$SANDBOX/install.log" >&2 + fail "marketplace.json is valid JSON" exit 1 fi +if [ -n "$PLUGIN_NAME" ]; then + pass "plugin.json is valid JSON (name: $PLUGIN_NAME)" +else + fail "plugin.json is valid JSON" + exit 1 +fi +assert_eq "the marketplace entry names this plugin" "$PLUGIN_NAME" \ + "$(jq -r --arg n "$PLUGIN_NAME" '.plugins[] | select(.name == $n) | .name' "$MARKETPLACE")" +# release-please bumps both manifests; drift means one of them is stale. +assert_eq "plugin.json version matches .claude-plugin/plugin.json" \ + "$(jq -r .version "$PLUGIN_ROOT/.claude-plugin/plugin.json")" \ + "$(jq -r .version "$MANIFEST")" +# `mcpServers` is what carries .mcp.json into Codex; without it there is no MCP. +assert_eq "plugin.json points mcpServers at .mcp.json" "./.mcp.json" \ + "$(jq -r '.mcpServers // empty' "$MANIFEST")" -# Everything below runs as if $FAKE_HOME were the user's home directory. -codex_in_sandbox() { - env HOME="$FAKE_HOME" CODEX_HOME="$FAKE_CODEX_HOME" codex "$@" -} +echo "" +echo "=== Native install ===" +if codex_in_sandbox plugin marketplace add "$PLUGIN_ROOT" >"$SANDBOX/mp.log" 2>&1; then + pass "codex plugin marketplace add accepts this repo" +else + fail "codex plugin marketplace add accepts this repo" "$(tail -3 "$SANDBOX/mp.log")" + cat "$SANDBOX/mp.log" >&2 + exit 1 +fi +if codex_in_sandbox plugin add "$PLUGIN_NAME@$MARKETPLACE_NAME" >"$SANDBOX/add.log" 2>&1; then + pass "codex plugin add installs the plugin" +else + fail "codex plugin add installs the plugin" "$(tail -3 "$SANDBOX/add.log")" + cat "$SANDBOX/add.log" >&2 + exit 1 +fi + +INSTALLED_ROOT=$(sed -n 's/^Installed plugin root: //p' "$SANDBOX/add.log" | tail -1) +if [ -n "$INSTALLED_ROOT" ] && [ -d "$INSTALLED_ROOT" ]; then + pass "the installed plugin root exists" +else + fail "the installed plugin root exists" "reported [${INSTALLED_ROOT:-none}]" + exit 1 +fi echo "" echo "=== Skills load ===" @@ -94,13 +140,11 @@ for dir in "$PLUGIN_ROOT"/skills/*/; do [ -f "$dir/SKILL.md" ] || continue name="$(basename "$dir")" expected=$((expected + 1)) - # Codex namespaces a skill when it can resolve a plugin manifest above it, so - # accept both the bare and the namespaced listing. - if ! grep -qE -- "- (vgv-ai-flutter-plugin:)?$name: " "$PROMPT_JSON"; then + # Codex namespaces a plugin's skills as :; accept either form. + if ! grep -qE -- "- ($PLUGIN_NAME:)?$name: " "$PROMPT_JSON"; then missing="$missing $name" fi done - if [ "$expected" -eq 0 ]; then fail "found skills to check" "no SKILL.md files under $PLUGIN_ROOT/skills" elif [ -n "$missing" ]; then @@ -108,50 +152,44 @@ elif [ -n "$missing" ]; then else pass "all $expected skills appear in the Codex prompt" fi +# They must come from the installed plugin, not from some other skills root. +if grep -qF "$INSTALLED_ROOT/skills" "$PROMPT_JSON"; then + pass "the skills root is the installed plugin" +else + fail "the skills root is the installed plugin" "$INSTALLED_ROOT/skills not listed" +fi echo "" -echo "=== Config loads ===" +echo "=== MCP servers load ===" DOCTOR_JSON="$SANDBOX/doctor.json" codex_in_sandbox doctor --json >"$DOCTOR_JSON" 2>/dev/null -# `codex doctor` exits non-zero when it cannot find credentials, which is the -# normal state here, so assert on the individual checks instead of the exit code. doctor_status() { jq -r --arg id "$1" '.checks[] | select(.id == $id) | .status' "$DOCTOR_JSON" 2>/dev/null } -# config.load proves the TOML the installer wrote actually parses. status=$(doctor_status config.load) -if [ "$status" = "ok" ]; then - pass "codex doctor: config.load is ok" -else - fail "codex doctor: config.load is ok" "got [${status:-no such check}]" -fi +assert_eq "codex doctor: config.load is ok" "ok" "${status:-no such check}" -# mcp.config downgrades to a warning when the server executables are missing, -# which is the normal state anywhere without the Dart SDK and Very Good CLI -# installed — CI runners included. Only a hard failure means the config is -# wrong; what the servers were registered *as* is asserted below instead. +# mcp.config degrades to a warning when the server executables are absent, which +# is the normal state anywhere without the Dart SDK and Very Good CLI installed, +# CI runners included. Only a hard failure means the config is wrong; what the +# servers were registered as is asserted below. status=$(doctor_status mcp.config) case "$status" in - ok) - pass "codex doctor: mcp.config is ok" - ;; - warning) - pass "codex doctor: mcp.config has no errors (warning, likely no dart/very_good on PATH)" - ;; - *) - fail "codex doctor: mcp.config has no errors" "got [${status:-no such check}]" - ;; + ok) pass "codex doctor: mcp.config is ok" ;; + warning) pass "codex doctor: mcp.config has no errors (warning, likely no dart/very_good on PATH)" ;; + *) fail "codex doctor: mcp.config has no errors" "got [${status:-no such check}]" ;; esac -# Assert what each server was registered as, so a warning above can never hide a -# server pointing at the wrong command. -assert_mcp_server() { - local server="$1" want_command="$2" want_args="$3" out +# Assert what each server was registered as, straight from .mcp.json, so this +# cannot drift from the file Claude Code reads. +while IFS= read -r server; do + want_command=$(jq -r --arg s "$server" '.mcpServers[$s].command' "$PLUGIN_ROOT/.mcp.json") + want_args=$(jq -r --arg s "$server" '(.mcpServers[$s].args // []) | join(" ")' "$PLUGIN_ROOT/.mcp.json") out=$(codex_in_sandbox mcp get "$server" 2>/dev/null) if [ -z "$out" ]; then fail "MCP server '$server' is registered" - return + continue fi pass "MCP server '$server' is registered" for field in "enabled: true" "transport: stdio" "command: $want_command" "args: $want_args"; do @@ -161,16 +199,19 @@ assert_mcp_server() { fail " $server $field" "$(printf '%s' "$out" | tr '\n' ' ')" fi done -} - -assert_mcp_server dart dart "mcp-server --enable dart_format" -assert_mcp_server very-good-cli very_good "mcp" +done < <(jq -r '.mcpServers | keys[]' "$PLUGIN_ROOT/.mcp.json") echo "" -echo "=== Hooks are well-formed ===" -# Codex ignores a malformed hooks.json without reporting anything, so a broken -# file would disable the whole enforcement layer silently. Check it here. -INSTALLED_HOOKS="$FAKE_CODEX_HOME/hooks.json" +echo "=== Hooks ===" +# Codex discovers a plugin's hooks at /hooks/hooks.json — the same +# file Claude Code uses — and resolves ${CLAUDE_PLUGIN_ROOT} in it as a +# compatibility alias for the installed plugin directory. +INSTALLED_HOOKS="$INSTALLED_ROOT/hooks/hooks.json" +if [ -f "$INSTALLED_HOOKS" ]; then + pass "hooks.json is installed at the plugin hook-discovery path" +else + fail "hooks.json is installed at the plugin hook-discovery path" "$INSTALLED_HOOKS" +fi if jq -e . "$INSTALLED_HOOKS" >/dev/null 2>&1; then pass "installed hooks.json is valid JSON" else @@ -186,39 +227,46 @@ for event in SessionStart PreToolUse PostToolUse; do done bad_shape=$(jq '[.hooks[][] | .hooks[]? | select((.type != "command") or ((.command | type) != "string"))] | length' "$INSTALLED_HOOKS") -if [ "$bad_shape" = "0" ]; then - pass "every handler is a command handler with a string command" -else - fail "every handler is a command handler with a string command" "$bad_shape malformed" -fi +assert_eq "every handler is a command handler with a string command" "0" "$bad_shape" -unresolved=$(grep -c '__VGV_PLUGIN_ROOT__' "$INSTALLED_HOOKS") -if [ "$unresolved" = "0" ]; then - pass "no unresolved plugin-root placeholder" +# Codex names its file-editing tool apply_patch, so a matcher that only says +# Edit|Write would never fire there. Claude Code ignores the extra alternative. +if jq -e '[.hooks.PostToolUse[].matcher] | all(test("apply_patch"))' "$INSTALLED_HOOKS" >/dev/null 2>&1; then + pass "PostToolUse matchers cover Codex's apply_patch" else - fail "no unresolved plugin-root placeholder" "$unresolved left" + fail "PostToolUse matchers cover Codex's apply_patch" \ + "$(jq -c '[.hooks.PostToolUse[].matcher]' "$INSTALLED_HOOKS")" fi -# Every script a hook points at must exist and be readable, or the hook is a -# silent no-op at runtime. +# Every referenced script has to exist inside the installed plugin, or the hook +# is a silent no-op. This is also what proves ${CLAUDE_PLUGIN_ROOT} points at a +# real tree once expanded. missing_scripts="" -while IFS= read -r script; do +checked=0 +while IFS= read -r command; do + [ -n "$command" ] || continue + script=$(printf '%s' "$command" \ + | sed -n 's|.*\${CLAUDE_PLUGIN_ROOT}/\([^" ]*\).*|\1|p') [ -n "$script" ] || continue - if [ ! -f "$script" ]; then + checked=$((checked + 1)) + if [ ! -f "$INSTALLED_ROOT/$script" ]; then missing_scripts="$missing_scripts $script" fi -done < <(jq -r '[.hooks[][] | .hooks[]?.command] | .[]' "$INSTALLED_HOOKS" \ - | sed -n 's/^bash "\(.*\)"$/\1/p') -if [ -z "$missing_scripts" ]; then - pass "every hook script exists on disk" +done < <(jq -r '[.hooks[][] | .hooks[]?.command] | .[]' "$INSTALLED_HOOKS") + +if [ "$checked" -eq 0 ]; then + fail "hook commands reference plugin-root scripts" "no \${CLAUDE_PLUGIN_ROOT} references found" +elif [ -n "$missing_scripts" ]; then + fail "all $checked hook scripts exist in the installed plugin" "missing:$missing_scripts" else - fail "every hook script exists on disk" "missing:$missing_scripts" + pass "all $checked hook scripts exist in the installed plugin" fi echo "" -echo "=== Agents are well-formed ===" - -# Print one top-level key from an agent file, or nothing if it is absent. +echo "=== Agents ===" +# Codex reads custom agents from ~/.codex/agents or a project's .codex/agents, +# neither of which a plugin can populate, so the reviewer agent is a file users +# copy. Validate it here since Codex will not. agent_field() { python3 -c " import sys, $TOML_MODULE as toml @@ -244,24 +292,30 @@ if missing: else fail "$name parses and has the required fields" fi - - if [ -f "$FAKE_CODEX_HOME/agents/$name" ]; then - pass "$name is installed into CODEX_HOME/agents" - else - fail "$name is installed into CODEX_HOME/agents" - fi done +# Codex copies a custom agent verbatim, so the installed tree must carry it for +# the documented `cp` to work. +if [ -f "$INSTALLED_ROOT/codex/agents/flutter-reviewer.toml" ]; then + pass "the reviewer agent ships inside the installed plugin" +else + fail "the reviewer agent ships inside the installed plugin" +fi + # The read-only reviewer must stay read-only: Codex has no per-agent tool # allowlist, so the sandbox is the only thing enforcing it. reviewer="$PLUGIN_ROOT/codex/agents/flutter-reviewer.toml" if [ -f "$reviewer" ]; then mode=$(agent_field "$reviewer" sandbox_mode) - if [ "$mode" = "read-only" ]; then - pass "flutter-reviewer is sandboxed read-only" - else - fail "flutter-reviewer is sandboxed read-only" "sandbox_mode is [${mode:-unset}]" - fi + assert_eq "flutter-reviewer is sandboxed read-only" "read-only" "${mode:-unset}" +fi + +echo "" +echo "=== Uninstall ===" +if codex_in_sandbox plugin remove "$PLUGIN_NAME@$MARKETPLACE_NAME" >"$SANDBOX/rm.log" 2>&1; then + pass "codex plugin remove uninstalls it" +else + fail "codex plugin remove uninstalls it" "$(tail -3 "$SANDBOX/rm.log")" fi echo "" diff --git a/hooks/hooks.json b/hooks/hooks.json index c97b65b..e651aa5 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -36,7 +36,7 @@ ], "PostToolUse": [ { - "matcher": "Edit|Write", + "matcher": "apply_patch|Edit|Write", "hooks": [ { "type": "command", From fdf76a25b09c070d6ba9dd7843ad3f4510a9405f Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Tue, 8 Sep 2026 15:47:07 +0200 Subject: [PATCH 04/18] docs: record why the Codex marketplace entry has to live in this repo `codex plugin add` only takes PLUGIN@MARKETPLACE, and Codex silently drops any marketplace entry that is not a `local` source resolving inside the marketplace root. It does parse Claude Code's `.claude-plugin/marketplace.json`, but every entry in very-good-claude-code-marketplace uses `source: github`, so adding that marketplace to Codex yields "No marketplace plugins found" with no error. Writing this down so nobody tries to move the entry there and hits a silent dead end. Co-Authored-By: Claude Opus 5 --- CONTRIBUTING.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2723b9f..51e0050 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -224,6 +224,17 @@ copy of the hooks. Verified against Codex CLI 0.153.4: is inert on Claude Code), and the payload hands over the raw patch with no `file_path` and no changed-file list, so `hook-payload-common.sh` parses the envelope. Keep that difference in that one file. +- **The marketplace entry has to live here.** `codex plugin add` only accepts + `PLUGIN@MARKETPLACE` — there is no direct-path or `owner/repo` install — and + `codex plugin marketplace add` refuses a root with no marketplace manifest. Codex will + read a Claude Code `.claude-plugin/marketplace.json`, but it silently drops any entry + whose source is not `local` with a path resolving **inside** the marketplace root: a + `github` or `git` source, or a `../sibling` path, yields "No marketplace plugins found" + with no error. That is why `.agents/plugins/marketplace.json` sits in this repo with + `"path": "."`, and why the existing `very-good-claude-code-marketplace` (whose entries + all use `source: github`) cannot serve Codex as-is. Repo-level marketplaces are not + discovered implicitly — only `~/.agents/plugins/marketplace.json` is — so the file is + inert until someone runs `codex plugin marketplace add`. - **A plugin cannot ship a Codex subagent.** Codex loads custom agents only from `~/.codex/agents/` or a project's `.codex/agents/`, and `agents` is not a plugin manifest field or a discovery path. `codex/agents/flutter-reviewer.toml` is therefore a file users copy, and it is the only thing left From a8bf82f2d32183f469cafce7e5624f6bb11adb95 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Tue, 8 Sep 2026 16:04:34 +0200 Subject: [PATCH 05/18] refactor: install Codex from the shared VGV marketplace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex marketplace entries do support a remote source — `{"source": "url", "url": "..."}` — which I had missed. So there is no need for a second marketplace: the entry belongs in very-good-claude-code-marketplace next to the Claude Code one, pointing back at this repo, and `.agents/plugins/marketplace.json` is deleted from here. Codex resolves `url` and `local` sources only. It silently drops `github`, `git`, `git-subdir`, and any path outside the marketplace root — the marketplace adds fine and `codex plugin list` reports "No marketplace plugins found" with no error. That is why the existing `.claude-plugin/marketplace.json` cannot serve Codex as-is, and why the two manifests coexist in the marketplace repo. Written up in CONTRIBUTING.md with the exact entry to add. Because a `url` source always resolves the default branch, the loader test now synthesizes its own throwaway marketplace pointing at the working tree, so CI still tests the checkout rather than main. It also validates every field Codex ingestion requires in .codex-plugin/plugin.json, which nothing else checked once the scaffold validator was out of the picture: 44 assertions, up from 35. Requires the companion entry in very-good-claude-code-marketplace before the documented install command works. Co-Authored-By: Claude Opus 5 --- .agents/plugins/marketplace.json | 20 ----------- AGENTS.md | 6 ++-- CLAUDE.md | 5 +-- CONTRIBUTING.md | 38 ++++++++++++++------ README.md | 5 ++- codex/loader_test.sh | 60 ++++++++++++++++++++++++-------- 6 files changed, 80 insertions(+), 54 deletions(-) delete mode 100644 .agents/plugins/marketplace.json diff --git a/.agents/plugins/marketplace.json b/.agents/plugins/marketplace.json deleted file mode 100644 index 3fd5806..0000000 --- a/.agents/plugins/marketplace.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "name": "very-good-ventures", - "interface": { - "displayName": "Very Good Ventures" - }, - "plugins": [ - { - "name": "vgv-ai-flutter-plugin", - "source": { - "source": "local", - "path": "." - }, - "policy": { - "installation": "AVAILABLE", - "authentication": "ON_INSTALL" - }, - "category": "Productivity" - } - ] -} diff --git a/AGENTS.md b/AGENTS.md index d6f9ae3..b7a7607 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,13 +8,11 @@ VGV AI Flutter Plugin provides best-practices skills for Flutter and Dart develo ```text .mcp.json # MCP server configuration (Dart and Very Good CLI); read by both harnesses -.agents/ - plugins/ - marketplace.json # Codex marketplace entry, so `codex plugin add` can install this repo .claude-plugin/ plugin.json # Claude Code plugin manifest (name, version, keywords) .codex-plugin/ - plugin.json # Codex plugin manifest (interface metadata + mcpServers -> ./.mcp.json) + plugin.json # Codex plugin manifest (interface metadata + mcpServers -> ./.mcp.json); + # the marketplace entry pointing here lives in very-good-claude-code-marketplace agents/ flutter-reviewer.md # Read-only Flutter code reviewer subagent codex/ # The only Codex-specific assets; skills, MCP and hooks are shared diff --git a/CLAUDE.md b/CLAUDE.md index 7d55f47..77d3ff4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -42,8 +42,9 @@ All hook scripts require **jq** to parse the hook payload (they skip gracefully ### Codex -This repo is a Codex plugin too. `.codex-plugin/plugin.json` plus the marketplace entry in -`.agents/plugins/marketplace.json` let `codex plugin add` install it, and Codex then reads +This repo is a Codex plugin too. `.codex-plugin/plugin.json` makes it installable, with the +marketplace entry living in `very-good-claude-code-marketplace` alongside the Claude Code one. +Codex then reads `skills/`, `.mcp.json`, and this same `hooks/hooks.json` — resolving `${CLAUDE_PLUGIN_ROOT}` as a compatibility alias. That is why the `PostToolUse` matcher says `apply_patch|Edit|Write`: Codex names its file-editing tool `apply_patch`, and the extra alternative is inert on Claude Code. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 51e0050..09c2b53 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -224,17 +224,33 @@ copy of the hooks. Verified against Codex CLI 0.153.4: is inert on Claude Code), and the payload hands over the raw patch with no `file_path` and no changed-file list, so `hook-payload-common.sh` parses the envelope. Keep that difference in that one file. -- **The marketplace entry has to live here.** `codex plugin add` only accepts - `PLUGIN@MARKETPLACE` — there is no direct-path or `owner/repo` install — and - `codex plugin marketplace add` refuses a root with no marketplace manifest. Codex will - read a Claude Code `.claude-plugin/marketplace.json`, but it silently drops any entry - whose source is not `local` with a path resolving **inside** the marketplace root: a - `github` or `git` source, or a `../sibling` path, yields "No marketplace plugins found" - with no error. That is why `.agents/plugins/marketplace.json` sits in this repo with - `"path": "."`, and why the existing `very-good-claude-code-marketplace` (whose entries - all use `source: github`) cannot serve Codex as-is. Repo-level marketplaces are not - discovered implicitly — only `~/.agents/plugins/marketplace.json` is — so the file is - inert until someone runs `codex plugin marketplace add`. +- **One marketplace serves both harnesses.** `codex plugin add` only accepts + `PLUGIN@MARKETPLACE`, so a marketplace is mandatory — but it is + `very-good-claude-code-marketplace`, the same repo Claude Code uses, not this one. That + repo carries a Codex manifest at `.agents/plugins/marketplace.json` beside its existing + `.claude-plugin/marketplace.json`, and the Codex entry points back here with a remote + `url` source: + + ```json + { + "name": "vgv-ai-flutter-plugin", + "source": { + "source": "url", + "url": "https://github.com/VeryGoodOpenSource/vgv-ai-flutter-plugin.git" + }, + "policy": { "installation": "AVAILABLE", "authentication": "ON_INSTALL" }, + "category": "Productivity" + } + ``` + + The source type matters and fails quietly when wrong. Codex resolves `url` and `local` + (with a path inside the marketplace root); it **silently drops** an entry using `github`, + `git`, `git-subdir`, or a `../sibling` path — the marketplace adds fine and + `codex plugin list` just reports "No marketplace plugins found", with no error anywhere. + That is why Codex cannot read the existing `.claude-plugin/marketplace.json`, whose + entries all use `source: github`, and why the two manifests coexist in that repo. + Because the `url` source always resolves the default branch, `codex/loader_test.sh` + synthesizes its own throwaway marketplace pointing at the working tree instead. - **A plugin cannot ship a Codex subagent.** Codex loads custom agents only from `~/.codex/agents/` or a project's `.codex/agents/`, and `agents` is not a plugin manifest field or a discovery path. `codex/agents/flutter-reviewer.toml` is therefore a file users copy, and it is the only thing left diff --git a/README.md b/README.md index 64e26df..a2d0813 100644 --- a/README.md +++ b/README.md @@ -86,11 +86,10 @@ tool, which is why that matcher covers it. See [Codex](#codex) for the differenc ## Codex -Codex has its own plugin system, so installation mirrors the Claude Code flow — two commands, no -scripts: +Codex installs from the same marketplace as Claude Code: ```bash -codex plugin marketplace add VeryGoodOpenSource/vgv-ai-flutter-plugin && codex plugin add vgv-ai-flutter-plugin@very-good-ventures +codex plugin marketplace add VeryGoodOpenSource/very-good-claude-code-marketplace && codex plugin add vgv-ai-flutter-plugin@very-good-claude-code-marketplace ``` That one install gives you the skills, both MCP servers, and the hooks. Codex reads them from the diff --git a/codex/loader_test.sh b/codex/loader_test.sh index 324a4dc..2652d76 100755 --- a/codex/loader_test.sh +++ b/codex/loader_test.sh @@ -19,7 +19,6 @@ set -uo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" PLUGIN_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" -MARKETPLACE="$PLUGIN_ROOT/.agents/plugins/marketplace.json" MANIFEST="$PLUGIN_ROOT/.codex-plugin/plugin.json" PASSED=0 @@ -74,23 +73,28 @@ codex_in_sandbox() { printf '\033[1mCodex %s\033[0m\n' "$(codex --version 2>/dev/null | head -1)" echo "" -echo "=== Manifests ===" -MARKETPLACE_NAME=$(jq -r '.name // empty' "$MARKETPLACE" 2>/dev/null) +echo "=== Plugin manifest ===" PLUGIN_NAME=$(jq -r '.name // empty' "$MANIFEST" 2>/dev/null) -if [ -n "$MARKETPLACE_NAME" ]; then - pass "marketplace.json is valid JSON (name: $MARKETPLACE_NAME)" -else - fail "marketplace.json is valid JSON" - exit 1 -fi if [ -n "$PLUGIN_NAME" ]; then pass "plugin.json is valid JSON (name: $PLUGIN_NAME)" else fail "plugin.json is valid JSON" exit 1 fi -assert_eq "the marketplace entry names this plugin" "$PLUGIN_NAME" \ - "$(jq -r --arg n "$PLUGIN_NAME" '.plugins[] | select(.name == $n) | .name' "$MARKETPLACE")" + +# Codex ingestion requires all of these; a missing one makes the plugin +# uninstallable, and nothing else in this repo checks them. +for field in .version .description .author.name \ + .interface.displayName .interface.shortDescription \ + .interface.longDescription .interface.developerName \ + .interface.category .interface.capabilities .interface.defaultPrompt; do + if [ -n "$(jq -r "$field // empty" "$MANIFEST")" ]; then + pass "plugin.json has $field" + else + fail "plugin.json has $field" + fi +done + # release-please bumps both manifests; drift means one of them is stale. assert_eq "plugin.json version matches .claude-plugin/plugin.json" \ "$(jq -r .version "$PLUGIN_ROOT/.claude-plugin/plugin.json")" \ @@ -101,13 +105,41 @@ assert_eq "plugin.json points mcpServers at .mcp.json" "./.mcp.json" \ echo "" echo "=== Native install ===" -if codex_in_sandbox plugin marketplace add "$PLUGIN_ROOT" >"$SANDBOX/mp.log" 2>&1; then - pass "codex plugin marketplace add accepts this repo" +# The published marketplace entry lives in very-good-claude-code-marketplace and +# points here with a remote `url` source, so it always resolves the default +# branch. To test *this* working tree instead, synthesize a throwaway marketplace +# whose single entry is a local path — a symlink back to the checkout. +MARKETPLACE_NAME="loader-test" +MARKETPLACE_ROOT="$SANDBOX/marketplace" +mkdir -p "$MARKETPLACE_ROOT/.agents/plugins" "$MARKETPLACE_ROOT/plugins" +ln -s "$PLUGIN_ROOT" "$MARKETPLACE_ROOT/plugins/$PLUGIN_NAME" +jq -n --arg mp "$MARKETPLACE_NAME" --arg n "$PLUGIN_NAME" '{ + name: $mp, + interface: { displayName: "Loader Test" }, + plugins: [ { + name: $n, + source: { source: "local", path: ("./plugins/" + $n) }, + policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, + category: "Productivity" + } ] +}' > "$MARKETPLACE_ROOT/.agents/plugins/marketplace.json" + +if codex_in_sandbox plugin marketplace add "$MARKETPLACE_ROOT" >"$SANDBOX/mp.log" 2>&1; then + pass "codex plugin marketplace add accepts the marketplace" else - fail "codex plugin marketplace add accepts this repo" "$(tail -3 "$SANDBOX/mp.log")" + fail "codex plugin marketplace add accepts the marketplace" "$(tail -3 "$SANDBOX/mp.log")" cat "$SANDBOX/mp.log" >&2 exit 1 fi +# A marketplace entry Codex cannot resolve is dropped silently, so confirm the +# plugin is actually listed before trying to install it. +if codex_in_sandbox plugin list 2>/dev/null | grep -q "$PLUGIN_NAME@$MARKETPLACE_NAME"; then + pass "the plugin resolves from the marketplace entry" +else + fail "the plugin resolves from the marketplace entry" \ + "$(codex_in_sandbox plugin list 2>&1 | tail -2)" + exit 1 +fi if codex_in_sandbox plugin add "$PLUGIN_NAME@$MARKETPLACE_NAME" >"$SANDBOX/add.log" 2>&1; then pass "codex plugin add installs the plugin" else From cdc13540fe67b9f7155b3b50b64cdd05ddd7d976 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 9 Sep 2026 10:03:12 +0200 Subject: [PATCH 06/18] refactor: drop the optional Codex plugin manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `.codex-plugin/plugin.json` is not required. Codex falls back to `.claude-plugin/plugin.json` for the plugin's name and version and discovers `.mcp.json` on its own, so an install without it is indistinguishable from one with it — same 15 skills, same `vgv-ai-flutter-plugin:` namespacing, same two MCP servers, same `codex plugin list` output. Verified both ways. What it would have added is Codex app presentation metadata (displayName, category, capabilities, defaultPrompt, icons), which is not worth a second manifest to keep version-synced. Removed from release-please's extra-files too. The consequence is recorded in CONTRIBUTING.md and AGENTS.md: `.claude-plugin/plugin.json` is now load-bearing for both harnesses rather than Claude Code alone. This repo now carries no Codex-specific plugin configuration at all. The only Codex-only files left are the reviewer agent TOML and the loader test. Co-Authored-By: Claude Opus 5 --- .codex-plugin/plugin.json | 28 --------------------------- .release-please-config.json | 5 ----- AGENTS.md | 13 ++++--------- CLAUDE.md | 10 +++++----- CONTRIBUTING.md | 19 ++++++++++++------- README.md | 17 +++++++++++------ codex/loader_test.sh | 38 +++++++++++++------------------------ 7 files changed, 45 insertions(+), 85 deletions(-) delete mode 100644 .codex-plugin/plugin.json diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json deleted file mode 100644 index c705bfd..0000000 --- a/.codex-plugin/plugin.json +++ /dev/null @@ -1,28 +0,0 @@ -{ - "name": "vgv-ai-flutter-plugin", - "version": "0.0.5", - "description": "Best-practice skills for Flutter and Dart development from Very Good Ventures.", - "author": { - "name": "Very Good Ventures", - "email": "hello@verygood.ventures", - "url": "https://verygood.ventures" - }, - "homepage": "https://github.com/VeryGoodOpenSource/vgv-ai-flutter-plugin", - "repository": "https://github.com/VeryGoodOpenSource/vgv-ai-flutter-plugin", - "license": "MIT", - "mcpServers": "./.mcp.json", - "interface": { - "displayName": "VGV AI Flutter Plugin", - "shortDescription": "Flutter and Dart best practices from Very Good Ventures", - "longDescription": "Best-practice skills for Flutter and Dart covering accessibility, animations, BLoC, testing, theming, navigation, security, internationalization, layered architecture, license compliance, UI packages, project creation, SDK/lint upgrades, and an autonomous quality-gate loop that drives analyze, format, test, and coverage to green — plus automated dart analyze and format hooks.", - "developerName": "Very Good Ventures", - "category": "Productivity", - "capabilities": ["Write"], - "defaultPrompt": [ - "Create a new Flutter app with Very Good CLI", - "Add a bloc for user authentication", - "Drive this package to green" - ], - "websiteURL": "https://verygood.ventures" - } -} diff --git a/.release-please-config.json b/.release-please-config.json index 15c43cf..5bcadf8 100644 --- a/.release-please-config.json +++ b/.release-please-config.json @@ -37,11 +37,6 @@ "type": "json", "path": ".claude-plugin/plugin.json", "jsonpath": "$.version" - }, - { - "type": "json", - "path": ".codex-plugin/plugin.json", - "jsonpath": "$.version" } ] } diff --git a/AGENTS.md b/AGENTS.md index b7a7607..9510081 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,10 +9,7 @@ VGV AI Flutter Plugin provides best-practices skills for Flutter and Dart develo ```text .mcp.json # MCP server configuration (Dart and Very Good CLI); read by both harnesses .claude-plugin/ - plugin.json # Claude Code plugin manifest (name, version, keywords) -.codex-plugin/ - plugin.json # Codex plugin manifest (interface metadata + mcpServers -> ./.mcp.json); - # the marketplace entry pointing here lives in very-good-claude-code-marketplace + plugin.json # Plugin manifest (name, version, keywords); Codex falls back to this too agents/ flutter-reviewer.md # Read-only Flutter code reviewer subagent codex/ # The only Codex-specific assets; skills, MCP and hooks are shared @@ -181,11 +178,9 @@ documentation in the same change: in `README.md`, and check whether any skill's `allowed-tools` names a tool that was renamed or removed. Nothing validates those names. A new **server** goes in `.mcp.json` only; both harnesses read that file. -- **Editing either plugin manifest** — `.claude-plugin/plugin.json` and - `.codex-plugin/plugin.json` describe the same plugin. Keep - `interface.longDescription` in the Codex manifest in step with `description` in - the Claude Code one, and leave `version` to release-please, which bumps both. - `codex/loader_test.sh` fails if the versions drift. +- **Editing `.claude-plugin/plugin.json`** — Codex falls back to this manifest for + the plugin's name and version, so it is no longer Claude-Code-only. Renaming the + plugin changes the skill namespace on both harnesses. - **Changing what a hook script reads from its payload** — the two harnesses describe an edit differently (Claude Code `tool_input.file_path`, Codex `tool_input.command` holding an apply_patch envelope). `hook-payload-common.sh` diff --git a/CLAUDE.md b/CLAUDE.md index 77d3ff4..43fc485 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -42,14 +42,14 @@ All hook scripts require **jq** to parse the hook payload (they skip gracefully ### Codex -This repo is a Codex plugin too. `.codex-plugin/plugin.json` makes it installable, with the -marketplace entry living in `very-good-claude-code-marketplace` alongside the Claude Code one. -Codex then reads +This repo installs as a Codex plugin with no Codex-specific config: the marketplace entry lives in +`very-good-claude-code-marketplace` alongside the Claude Code one, and Codex falls back to +`.claude-plugin/plugin.json` for the plugin's identity. It reads `skills/`, `.mcp.json`, and this same `hooks/hooks.json` — resolving `${CLAUDE_PLUGIN_ROOT}` as a compatibility alias. That is why the `PostToolUse` matcher says `apply_patch|Edit|Write`: Codex names its file-editing tool `apply_patch`, and the extra alternative is inert on Claude Code. -The only Codex-specific asset is `codex/agents/flutter-reviewer.toml`, because a plugin cannot ship -a Codex subagent. `codex/loader_test.sh` installs the repo the way a user would and asserts Codex +The only Codex-specific asset is `codex/agents/flutter-reviewer.toml`, because Codex has no way to +bundle a subagent in a plugin — users copy it to `~/.codex/agents/` themselves. `codex/loader_test.sh` installs the repo the way a user would and asserts Codex picks it all up. Change a hook or the reviewer agent and both harnesses are affected — see `AGENTS.md` → Maintaining Existing Skills, Hooks, and MCP Tools. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 09c2b53..80812c5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -199,13 +199,18 @@ Claude Code one. `.codex-plugin/plugin.json` plus the marketplace entry in `hooks/hooks.json` from the very files Claude Code uses. There is no install script and no second copy of the hooks. Verified against Codex CLI 0.153.4: -- **Two manifests, one source of truth.** `.codex-plugin/plugin.json` carries only what Codex needs - that Claude Code's manifest cannot express — the `interface` block and `mcpServers: "./.mcp.json"`, - which is what pulls the MCP servers in. Its `version` is bumped by release-please alongside - `.claude-plugin/plugin.json` (both are listed under `extra-files`), and `codex/loader_test.sh` - fails if the two drift. Keep `interface.longDescription` in step with the `description` in - `.claude-plugin/plugin.json`. `keywords` is deliberately **not** duplicated: it only affects - plugin search, and a second copy of a 50-plus entry list would rot. +- **No Codex manifest, deliberately.** A `.codex-plugin/plugin.json` is optional and this repo + ships none. Codex falls back to `.claude-plugin/plugin.json` for the plugin's name and version + and discovers `.mcp.json` by itself, so an install without it is indistinguishable from one with + it: same skills, same namespacing, same two MCP servers, same `codex plugin list` output. The one + thing it would add is Codex app presentation metadata (`interface.displayName`, `category`, + `capabilities`, `defaultPrompt`, brand color, icons), which has no other home. That was judged + not worth a second manifest to keep in version-sync with the Claude Code one. Note the + consequence: `.claude-plugin/plugin.json` is now load-bearing for **both** harnesses, so its + `name` and `version` are not Claude-only fields any more. If you ever add + `.codex-plugin/plugin.json`, `codex/loader_test.sh` will fail until you restore field checks for + it — the manifest is rejected outright if it carries a `hooks` key or any field outside Codex's + allowed set. - **The plugin manifest rejects a `hooks` field.** Hooks arrive purely through default discovery at `/hooks/hooks.json`, and Codex resolves `${CLAUDE_PLUGIN_ROOT}` inside it as a compatibility alias for the installed plugin directory. That is why the Claude Code hooks file diff --git a/README.md b/README.md index a2d0813..d819401 100644 --- a/README.md +++ b/README.md @@ -92,13 +92,14 @@ Codex installs from the same marketplace as Claude Code: codex plugin marketplace add VeryGoodOpenSource/very-good-claude-code-marketplace && codex plugin add vgv-ai-flutter-plugin@very-good-claude-code-marketplace ``` -That one install gives you the skills, both MCP servers, and the hooks. Codex reads them from the -same files Claude Code does — `skills/`, `.mcp.json`, and `hooks/hooks.json` — via -`.codex-plugin/plugin.json`. Restart Codex afterwards, then approve the hooks with `/hooks`, since -Codex requires a review before a hook runs for the first time. +That one install gives you the skills, both MCP servers, and the hooks. Codex discovers them from +the same files Claude Code uses — `skills/`, `.mcp.json`, and `hooks/hooks.json` — so there is no +Codex-specific configuration in this repo at all. Restart Codex afterwards, then approve the hooks +with `/hooks`, since Codex requires a review before a hook runs for the first time. -The reviewer agent is the one piece a plugin cannot carry, because Codex only loads custom agents -from `~/.codex/agents/` or a project's `.codex/agents/`. Copy it in once: +The reviewer agent is the one piece `codex plugin add` does **not** install: Codex has no way to +bundle a subagent in a plugin, and loads custom agents only from `~/.codex/agents/` or a project's +`.codex/agents/`. Copy it in once: ```bash mkdir -p ~/.codex/agents && cp codex/agents/flutter-reviewer.toml ~/.codex/agents/ @@ -113,6 +114,10 @@ Then ask Codex to spawn `flutter-reviewer`. directory, and it calls its file-editing tool `apply_patch` and hands the hook a raw patch rather than a file path — so the `PostToolUse` matcher covers `apply_patch` and `analyze.sh` / `format.sh` read both payload shapes. +- **Codex reuses the Claude Code plugin manifest.** It falls back to `.claude-plugin/plugin.json` + for the plugin's name and version, and finds `.mcp.json` on its own, so no second manifest is + needed. The trade is that Codex has no plugin-specific presentation metadata for this plugin — + icons, brand color, and starter prompts in the Codex app come from the fallback. - **The reviewer agent is sandboxed instead of tool-restricted.** On Claude Code `flutter-reviewer` has no write tools and an agent-scoped hook limits its Bash to `git diff`/`git status`. Codex has no per-agent tool allowlist, so the agent declares diff --git a/codex/loader_test.sh b/codex/loader_test.sh index 2652d76..bcb59e3 100755 --- a/codex/loader_test.sh +++ b/codex/loader_test.sh @@ -19,7 +19,10 @@ set -uo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" PLUGIN_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" -MANIFEST="$PLUGIN_ROOT/.codex-plugin/plugin.json" +# This repo ships no Codex plugin manifest. Codex falls back to the Claude Code +# one for the plugin's identity, which makes that file load-bearing for both +# harnesses — hence reading the name from it here. +MANIFEST="$PLUGIN_ROOT/.claude-plugin/plugin.json" PASSED=0 FAILED=0 @@ -73,35 +76,20 @@ codex_in_sandbox() { printf '\033[1mCodex %s\033[0m\n' "$(codex --version 2>/dev/null | head -1)" echo "" -echo "=== Plugin manifest ===" +echo "=== Plugin identity ===" PLUGIN_NAME=$(jq -r '.name // empty' "$MANIFEST" 2>/dev/null) if [ -n "$PLUGIN_NAME" ]; then - pass "plugin.json is valid JSON (name: $PLUGIN_NAME)" + pass "plugin identity resolves from .claude-plugin/plugin.json (name: $PLUGIN_NAME)" else - fail "plugin.json is valid JSON" + fail "plugin identity resolves from .claude-plugin/plugin.json" exit 1 fi - -# Codex ingestion requires all of these; a missing one makes the plugin -# uninstallable, and nothing else in this repo checks them. -for field in .version .description .author.name \ - .interface.displayName .interface.shortDescription \ - .interface.longDescription .interface.developerName \ - .interface.category .interface.capabilities .interface.defaultPrompt; do - if [ -n "$(jq -r "$field // empty" "$MANIFEST")" ]; then - pass "plugin.json has $field" - else - fail "plugin.json has $field" - fi -done - -# release-please bumps both manifests; drift means one of them is stale. -assert_eq "plugin.json version matches .claude-plugin/plugin.json" \ - "$(jq -r .version "$PLUGIN_ROOT/.claude-plugin/plugin.json")" \ - "$(jq -r .version "$MANIFEST")" -# `mcpServers` is what carries .mcp.json into Codex; without it there is no MCP. -assert_eq "plugin.json points mcpServers at .mcp.json" "./.mcp.json" \ - "$(jq -r '.mcpServers // empty' "$MANIFEST")" +if [ ! -e "$PLUGIN_ROOT/.codex-plugin" ]; then + pass "no Codex-specific plugin manifest to keep in sync" +else + fail "no Codex-specific plugin manifest to keep in sync" \ + "found .codex-plugin — either delete it or restore its checks here" +fi echo "" echo "=== Native install ===" From 86073618f7aa3e8f67c731e0773c27b33d416a94 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 9 Sep 2026 10:08:09 +0200 Subject: [PATCH 07/18] docs: document both scopes for installing the Codex reviewer agent Codex cannot bundle a subagent in a plugin, so the agent file is copied in. The README only showed the personal scope (~/.codex/agents/); the project scope (.codex/agents/ committed to the Flutter repo) is how most repos in the wild actually do it and gives a whole team the reviewer with no per-developer setup. Both are now documented, with links to the open upstream issues that would make the step unnecessary. Co-Authored-By: Claude Opus 5 --- CONTRIBUTING.md | 11 +++++++++-- README.md | 16 +++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 80812c5..5653727 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -258,8 +258,12 @@ copy of the hooks. Verified against Codex CLI 0.153.4: synthesizes its own throwaway marketplace pointing at the working tree instead. - **A plugin cannot ship a Codex subagent.** Codex loads custom agents only from `~/.codex/agents/` or a project's `.codex/agents/`, and `agents` is not a plugin manifest field or a discovery path. - `codex/agents/flutter-reviewer.toml` is therefore a file users copy, and it is the only thing left - in `codex/`. Codex custom agents are standalone TOML needing `name`, `description`, and + `codex/agents/flutter-reviewer.toml` is therefore a file users copy, either into + `~/.codex/agents/` for themselves or committed to a project's `.codex/agents/` for a whole team — + the latter is how most repos in the wild do it. Distributing agents any other way currently means + an install script, which this plugin deliberately does not ship. Upstream requests to bundle + agents in a plugin are open ([openai/codex#18988][codex_agents_issue], + [openai/codex#28491][codex_agents_issue_2]); if either lands, the copy step goes away. Codex custom agents are standalone TOML needing `name`, `description`, and `developer_instructions`, plus any `config.toml` key. There is no per-agent tool allowlist and no agent-scoped `PreToolUse` hook, so it sets `sandbox_mode = "read-only"` to hold the read-only contract that `allow-readonly-git.sh` holds on Claude Code. Codex ships no validator for agent @@ -432,3 +436,6 @@ type(scope): description - Fill out the [PR template](.github/PULL_REQUEST_TEMPLATE.md) completely. - Ensure all CI checks pass before requesting review. - Link any related issues in the PR description. + +[codex_agents_issue]: https://github.com/openai/codex/issues/18988 +[codex_agents_issue_2]: https://github.com/openai/codex/issues/28491 diff --git a/README.md b/README.md index d819401..1e89883 100644 --- a/README.md +++ b/README.md @@ -98,14 +98,23 @@ Codex-specific configuration in this repo at all. Restart Codex afterwards, then with `/hooks`, since Codex requires a review before a hook runs for the first time. The reviewer agent is the one piece `codex plugin add` does **not** install: Codex has no way to -bundle a subagent in a plugin, and loads custom agents only from `~/.codex/agents/` or a project's -`.codex/agents/`. Copy it in once: +bundle a subagent in a plugin ([openai/codex#18988][codex_agents_issue]), and loads custom agents +only from `~/.codex/agents/` or a project's `.codex/agents/`. Pick whichever scope fits. + +For yourself, across every project: ```bash mkdir -p ~/.codex/agents && cp codex/agents/flutter-reviewer.toml ~/.codex/agents/ ``` -Then ask Codex to spawn `flutter-reviewer`. +For a whole team, commit it into the Flutter project instead — then everyone gets the reviewer with +no per-developer setup: + +```bash +mkdir -p .codex/agents && cp codex/agents/flutter-reviewer.toml .codex/agents/ +``` + +Either way, ask Codex to spawn `flutter-reviewer`. ### How Codex differs from Claude Code @@ -241,6 +250,7 @@ On Codex the same two servers are registered in `~/.codex/config.toml` instead [Codex](#codex). Skills that drive an MCP tool always name the equivalent `very_good`, `dart`, or `flutter` command as a fallback, so they keep working on a host where neither server is connected. +[codex_agents_issue]: https://github.com/openai/codex/issues/18988 [marketplace_link]: https://github.com/VeryGoodOpenSource/very-good-claude-code-marketplace [claude_code_link]: https://claude.ai/code [vgv_link]: https://verygood.ventures From 3135d6e5347a25abf79947537c8afc94467da076 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 9 Sep 2026 10:22:33 +0200 Subject: [PATCH 08/18] chore: revert incidental reformatting of .release-please-config.json A JSON round-trip while adding and then removing the Codex manifest entry reflowed the file and escaped a non-ASCII character. Net change is zero, so restore it byte-for-byte. Co-Authored-By: Claude Opus 5 --- .release-please-config.json | 27 ++++++--------------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/.release-please-config.json b/.release-please-config.json index 5bcadf8..9dbfd32 100644 --- a/.release-please-config.json +++ b/.release-please-config.json @@ -1,28 +1,13 @@ { "$schema": "https://raw.githubusercontent.com/googleapis/release-please/main/schemas/config.json", "changelog-sections": [ - { - "type": "feat", - "section": "Features" - }, - { - "type": "fix", - "section": "Bug Fixes" - }, - { - "type": "refactor", - "section": "Refactors" - }, - { - "type": "chore", - "section": "Miscellaneous Chores" - }, - { - "type": "docs", - "section": "Docs" - } + { "type": "feat", "section": "Features" }, + { "type": "fix", "section": "Bug Fixes" }, + { "type": "refactor", "section": "Refactors" }, + { "type": "chore", "section": "Miscellaneous Chores" }, + { "type": "docs", "section": "Docs" } ], - "pull-request-header": ":rotating_light: There are changes ready for release :rocket:\n\n\u2139 Merge this PR once the team confirms the release is ready.\n", + "pull-request-header": ":rotating_light: There are changes ready for release :rocket:\n\nℹ Merge this PR once the team confirms the release is ready.\n", "pull-request-title-pattern": "chore: ${version}", "extra-label": "no-auto-update", "include-component-in-tag": false, From a880e079f7d9d1aa2c077aa98252737326cfdf7a Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 9 Sep 2026 10:22:58 +0200 Subject: [PATCH 09/18] chore: drop unused cspell entry tomllib only appears in codex/loader_test.sh, which cspell does not check. Co-Authored-By: Claude Opus 5 --- config/cspell.json | 1 - 1 file changed, 1 deletion(-) diff --git a/config/cspell.json b/config/cspell.json index a05558b..d28c1c6 100644 --- a/config/cspell.json +++ b/config/cspell.json @@ -61,7 +61,6 @@ "subclassing", "tappable", "tomli", - "tomllib", "tooltipped", "unmirrored", "unrouted", From e92da73861aec01ad45dcf96c2a8ccef05c34799 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 9 Sep 2026 10:37:46 +0200 Subject: [PATCH 10/18] docs: correct hook claims against the official references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified this PR's assumptions against both hooks references. Three fixes. `analyze.sh` was described as blocking. It is a PostToolUse hook, and Claude Code's docs are explicit that exit 2 there does not block because the tool has already run — stderr is shown to the model instead. The effect (the model sees the analyzer output and fixes it) is what we wanted; the mechanism was described wrong. `block-cli-workarounds.sh` is PreToolUse, where exit 2 really does block, so those lines stand. The claim that a Codex plugin manifest rejects a `hooks` field was overstated: the scaffold validator refuses it, but the runtime docs say a manifest may override the hooks path. Moot here since we ship no manifest, so the bullet now just states that discovery defaults to /hooks/hooks.json. Recorded the matcher trap. Claude Code treats a matcher of only letters, digits, _, -, space, comma and | as exact tool names, and anything else as an unanchored regex. `apply_patch|Edit|Write` stays on the exact path, which is why it matches those three names and not MultiEdit. Adding a regex metacharacter would silently pull in MultiEdit and NotebookEdit, whose payloads hook-payload-common.sh does not read. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 2 +- CONTRIBUTING.md | 17 +++++++++++++---- README.md | 2 +- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 43fc485..b4d06c6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,7 +31,7 @@ from `vgv-cli-common.sh`. The following hook is **agent-scoped** — it is decla These run **after** a tool call completes: -- `apply_patch|Edit|Write` matcher → `analyze.sh` — runs `dart analyze` on the modified `.dart` file(s); exits 2 on failure (blocking — Claude must fix the issue) +- `apply_patch|Edit|Write` matcher → `analyze.sh` — runs `dart analyze` on the modified `.dart` file(s); on failure exits 2, which feeds the analyzer output back to the model as a message. `PostToolUse` runs after the tool, so this does not block or revert the edit - `apply_patch|Edit|Write` matcher → `format.sh` — runs `dart format` on the modified `.dart` file(s); always exits 0 (non-blocking) Both read the changed files through `hook-payload-common.sh`, which handles Claude Code's diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5653727..a14c41a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -211,10 +211,19 @@ copy of the hooks. Verified against Codex CLI 0.153.4: `.codex-plugin/plugin.json`, `codex/loader_test.sh` will fail until you restore field checks for it — the manifest is rejected outright if it carries a `hooks` key or any field outside Codex's allowed set. -- **The plugin manifest rejects a `hooks` field.** Hooks arrive purely through default discovery at - `/hooks/hooks.json`, and Codex resolves `${CLAUDE_PLUGIN_ROOT}` inside it as a - compatibility alias for the installed plugin directory. That is why the Claude Code hooks file - works unchanged — do not add a `hooks` key to `.codex-plugin/plugin.json`, validation refuses it. +- **Hooks come from default discovery.** Codex looks for a plugin's hooks at + `/hooks/hooks.json` — the same path and file Claude Code uses — and resolves + `${CLAUDE_PLUGIN_ROOT}` inside it, documented as a compatibility alias alongside its own + `PLUGIN_ROOT`. That is the whole reason one hooks file serves both harnesses. A plugin manifest + can override the path with a `hooks` entry, but this repo ships no Codex manifest, so the default + is what applies. +- **Keep the `PostToolUse` matcher on Claude Code's exact-match path.** Claude Code treats a + matcher containing only letters, digits, `_`, `-`, spaces, `,` and `|` as a list of exact tool + names; anything else is an unanchored regex tested with `RegExp.test`. `apply_patch|Edit|Write` + qualifies as exact, so it matches those three tool names and nothing else. Add a `.` or `*` and + it silently becomes a regex that also matches `MultiEdit` and `NotebookEdit`, whose payloads this + plugin does not read (`MultiEdit` nests `file_path` inside `edits[]` rather than at the top + level). Widen the matcher only together with `hook-payload-common.sh`. - **Hooks are a stable, default-on feature**, not experimental. The flag is `[features] hooks` (`codex features list` shows it enabled); there is no `codex_hooks` flag. Codex also runs hooks on Windows and offers a `commandWindows` override — but these scripts are `bash` and need `jq`, so diff --git a/README.md b/README.md index 1e89883..f06dad7 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ This plugin includes SessionStart, PreToolUse, and PostToolUse hooks that valida | **Check VGV CLI** (`check-vgv-cli.sh`) | PreToolUse (`mcp__.*very-good-cli__.*`) | Auto-approves Very Good CLI MCP tool calls in every run mode via a PreToolUse `allow` decision, so they never dead-end when the tool isn't on `permissions.allow` (including under `skipAutoPermissionPrompt`); denies with an install/upgrade message if the CLI is missing or < 1.3.0 | | **Block CLI Workarounds** (`block-cli-workarounds.sh`) | PreToolUse (`Bash`) | Blocks direct CLI bypass of Very Good CLI commands through the Bash tool; exits 2 on failure (blocking) | | **Allow Read-only Git** (`allow-readonly-git.sh`) | PreToolUse (`Bash`, `flutter-reviewer` agent only) | Restricts the `flutter-reviewer` agent's Bash to `git diff`/`git status`; exits 2 on anything else (blocking). Scoped via the agent's frontmatter, not `hooks.json` | -| **Analyze** (`analyze.sh`) | PostToolUse (`apply_patch`/`Edit`/`Write`) | Runs `dart analyze` on the modified `.dart` file; exits 2 on failure (blocking — Claude must fix issues before continuing) | +| **Analyze** (`analyze.sh`) | PostToolUse (`apply_patch`/`Edit`/`Write`) | Runs `dart analyze` on the modified `.dart` file(s); on failure exits 2, which surfaces the analyzer output to the model as feedback so it fixes the issue. The edit itself already happened and is not reverted | | **Format** (`format.sh`) | PostToolUse (`apply_patch`/`Edit`/`Write`) | Runs `dart format` on the modified `.dart` file; always exits 0 (non-blocking — formatting is applied silently) | Codex runs this same `hooks/hooks.json` and these same scripts — `apply_patch` is its file-editing From 24206e4e4ddfb4f4e9bd50957ed2b97adc8b270a Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 9 Sep 2026 11:02:36 +0200 Subject: [PATCH 11/18] refactor: inline the payload reading instead of a shared helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces hooks/scripts/hook-payload-common.sh with the same six-line jq expression in analyze.sh and format.sh. Codex hook payloads carry no file path and no changed-file list, so the paths still have to come out of the patch headers, but that no longer needs its own script. Simpler than the helper it replaces: selecting only Add/Update/Move headers means a delete contributes nothing, and a rename lists both paths so the existing-file check drops the stale one. No Move-to bookkeeping, no awk. Tests moved to dart-hooks_test.sh, which drives both hooks through a stub `dart` on PATH and asserts exactly which files reach the SDK, plus the exit codes. That covers more than the old helper-function tests did and still needs no Dart SDK in CI. Also verified against the real SDK by hand. Mutation testing while writing it found a genuine gap: removing the "*** Begin Patch" guard did not fail the suite, because the ^ anchor already rejects single-line shell commands. Added the cases that do catch it — a heredoc whose body starts with a patch marker, and a marker on a later line. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 9 +- CLAUDE.md | 7 +- CONTRIBUTING.md | 9 +- hooks/scripts/analyze.sh | 39 +++- hooks/scripts/dart-hooks_test.sh | 217 ++++++++++++++++++++++ hooks/scripts/format.sh | 39 +++- hooks/scripts/hook-payload-common.sh | 92 --------- hooks/scripts/hook-payload-common_test.sh | 170 ----------------- 8 files changed, 295 insertions(+), 287 deletions(-) create mode 100755 hooks/scripts/dart-hooks_test.sh delete mode 100755 hooks/scripts/hook-payload-common.sh delete mode 100755 hooks/scripts/hook-payload-common_test.sh diff --git a/AGENTS.md b/AGENTS.md index 9510081..7a4b633 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,7 +50,6 @@ hooks/ block-cli-workarounds.sh # Prevents direct CLI bypass via Bash check-vgv-cli.sh # Validates VGV CLI installed and >= 1.3.0 format.sh # Runs dart format on modified .dart files - hook-payload-common.sh # Reads Claude Code file_path and Codex apply_patch payloads vgv-cli-common.sh # Shared utilities for VGV CLI hook scripts warn-missing-mcp.sh # Warns at session start if VGV CLI is missing/outdated skills/ # every / ships SKILL.md + agents/openai.yaml (Codex sidecar) @@ -183,10 +182,10 @@ documentation in the same change: plugin changes the skill namespace on both harnesses. - **Changing what a hook script reads from its payload** — the two harnesses describe an edit differently (Claude Code `tool_input.file_path`, Codex - `tool_input.command` holding an apply_patch envelope). `hook-payload-common.sh` - is the only place that difference is handled; extend it there rather than - branching per harness in `analyze.sh` or `format.sh`, and add a case to - `hook-payload-common_test.sh`. + `tool_input.command` holding an apply_patch envelope). `analyze.sh` and + `format.sh` each read both shapes with the same inline `jq` expression — keep + the two copies identical, and add a case to `dart-hooks_test.sh`, which drives + both scripts through a stub `dart` so it needs no SDK. - **Changing `agents/flutter-reviewer.md`** — port the same change to `codex/agents/flutter-reviewer.toml`. A Codex plugin cannot ship a subagent, so that file is a separate copy users install by hand. Its output contract (the diff --git a/CLAUDE.md b/CLAUDE.md index b4d06c6..ef13fbf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -34,9 +34,10 @@ These run **after** a tool call completes: - `apply_patch|Edit|Write` matcher → `analyze.sh` — runs `dart analyze` on the modified `.dart` file(s); on failure exits 2, which feeds the analyzer output back to the model as a message. `PostToolUse` runs after the tool, so this does not block or revert the edit - `apply_patch|Edit|Write` matcher → `format.sh` — runs `dart format` on the modified `.dart` file(s); always exits 0 (non-blocking) -Both read the changed files through `hook-payload-common.sh`, which handles Claude Code's -`tool_input.file_path` and Codex's `tool_input.command` (an `apply_patch` envelope, which can -name several files at once). That is the only harness-specific branch in the hook scripts. +Both resolve the changed files with the same inline `jq` expression, handling Claude Code's +`tool_input.file_path` and Codex's `tool_input.command` (an `apply_patch` envelope, which can name +several files at once). That is the only harness-specific branch in the hook scripts, and the two +copies must stay identical — `dart-hooks_test.sh` covers both. All hook scripts require **jq** to parse the hook payload (they skip gracefully if `jq` is not installed). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a14c41a..19c7472 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -223,7 +223,8 @@ copy of the hooks. Verified against Codex CLI 0.153.4: qualifies as exact, so it matches those three tool names and nothing else. Add a `.` or `*` and it silently becomes a regex that also matches `MultiEdit` and `NotebookEdit`, whose payloads this plugin does not read (`MultiEdit` nests `file_path` inside `edits[]` rather than at the top - level). Widen the matcher only together with `hook-payload-common.sh`. + level). Widen the matcher only together with the payload reading in `analyze.sh` and + `format.sh`. - **Hooks are a stable, default-on feature**, not experimental. The flag is `[features] hooks` (`codex features list` shows it enabled); there is no `codex_hooks` flag. Codex also runs hooks on Windows and offers a `commandWindows` override — but these scripts are `bash` and need `jq`, so @@ -236,8 +237,10 @@ copy of the hooks. Verified against Codex CLI 0.153.4: Code, so `warn-missing-mcp.sh` is too. Only the edit hooks differ: Codex's file-editing tool is `apply_patch`, so the `PostToolUse` matcher reads `apply_patch|Edit|Write` (the extra alternative is inert on Claude Code), and the payload hands over the raw patch with no `file_path` and no - changed-file list, so `hook-payload-common.sh` parses the envelope. Keep that difference in that - one file. + changed-file list, so both hooks read the paths out of the patch headers with the same inline + `jq` expression. A rename lists the old and new path and a delete lists none, so an existence + check is all the bookkeeping needed. The expression is duplicated in the two scripts rather than + shared through a third file; keep the copies identical and covered by `dart-hooks_test.sh`. - **One marketplace serves both harnesses.** `codex plugin add` only accepts `PLUGIN@MARKETPLACE`, so a marketplace is mandatory — but it is `very-good-claude-code-marketplace`, the same repo Claude Code uses, not this one. That diff --git a/hooks/scripts/analyze.sh b/hooks/scripts/analyze.sh index 6fa45c7..d7d1a0d 100755 --- a/hooks/scripts/analyze.sh +++ b/hooks/scripts/analyze.sh @@ -10,18 +10,43 @@ if ! command -v jq &>/dev/null; then exit 0 fi -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -# shellcheck source=hooks/scripts/hook-payload-common.sh -source "$SCRIPT_DIR/hook-payload-common.sh" +# Which files did this edit touch? The two harnesses answer differently: +# +# Claude Code Edit / Write -> .tool_input.file_path (one path) +# Codex apply_patch -> .tool_input.command (a patch envelope, no path) +# +# Codex hook payloads carry no file path and no changed-file list, so the paths +# are read out of the patch headers. A rename emits both the old and the new +# path; the old one no longer exists, so the -f test below drops it. Deleted +# files never match, since only Add/Update/Move headers are selected. +paths=$(jq -r ' + if .tool_input.file_path then .tool_input.file_path + else + (.tool_input.command // "") + | select(startswith("*** Begin Patch")) + | split("\n")[] + | select(test("^\\*\\*\\* (Add File|Update File|Move to): ")) + | sub("^\\*\\*\\* (Add File|Update File|Move to): "; "") + end' <<< "$input") + +cwd=$(jq -r '.cwd // empty' <<< "$input") -# Collect the Dart files this edit touched. Claude Code reports one `file_path`; -# Codex reports an apply_patch envelope that may cover several files. files=() while IFS= read -r file; do - if [ -n "$file" ]; then + [ -n "$file" ] || continue + case "$file" in + *.dart) ;; + *) continue ;; + esac + # apply_patch paths may be relative to the session working directory. + case "$file" in + /*) ;; + *) if [ -n "$cwd" ]; then file="$cwd/$file"; fi ;; + esac + if [ -f "$file" ]; then files+=("$file") fi -done < <(changed_dart_files "$input") +done <<< "$paths" # Nothing Dart in this edit if [ ${#files[@]} -eq 0 ]; then diff --git a/hooks/scripts/dart-hooks_test.sh b/hooks/scripts/dart-hooks_test.sh new file mode 100755 index 0000000..9832289 --- /dev/null +++ b/hooks/scripts/dart-hooks_test.sh @@ -0,0 +1,217 @@ +#!/bin/bash +# Tests for analyze.sh and format.sh +# +# Usage: bash hooks/scripts/dart-hooks_test.sh +# +# Both hooks have to read two different payload shapes: +# +# Claude Code Edit / Write -> .tool_input.file_path +# Codex apply_patch -> .tool_input.command (a patch envelope) +# +# Getting that wrong fails silently — the hook exits 0 having done nothing — so +# these cases assert exactly which files reach the Dart SDK. A stub `dart` on +# PATH records its arguments, which keeps the suite fast and means CI needs no +# Dart SDK installed. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ANALYZE="$SCRIPT_DIR/analyze.sh" +FORMAT="$SCRIPT_DIR/format.sh" + +PASSED=0 +FAILED=0 + +WORK=$(mktemp -d) +trap 'rm -rf "$WORK"' EXIT + +# A `dart` that records the files it was asked to act on, and fails when told to. +mkdir -p "$WORK/bin" +cat > "$WORK/bin/dart" <<'STUB' +#!/bin/bash +subcommand="$1"; shift +: > "$DART_STUB_LOG" +for arg in "$@"; do + printf '%s\n' "$(basename "$arg")" >> "$DART_STUB_LOG" +done +if [ -n "${DART_STUB_FAIL:-}" ] && [ "$subcommand" = "analyze" ]; then + echo "error - stubbed analyzer failure" >&2 + exit 1 +fi +exit 0 +STUB +chmod +x "$WORK/bin/dart" +export DART_STUB_LOG="$WORK/dart.log" + +mkdir -p "$WORK/repo/lib" +: > "$WORK/repo/lib/a.dart" +: > "$WORK/repo/lib/b.dart" +: > "$WORK/repo/lib/renamed.dart" +: > "$WORK/repo/README.md" + +# Run a hook with the stub first on PATH. Echoes the basenames dart received. +run_hook() { + local hook="$1" payload="$2" + : > "$DART_STUB_LOG" + PATH="$WORK/bin:$PATH" bash "$hook" <<< "$payload" >/dev/null 2>&1 + LC_ALL=C sort "$DART_STUB_LOG" | tr '\n' ' ' +} + +assert_files() { + local label="$1" hook="$2" payload="$3" expected="$4" actual + actual=$(run_hook "$hook" "$payload") + expected=$(printf '%s' "$expected" | tr '\n' ' ') + if [ "$actual" = "$expected" ]; then + printf " \033[32mPASS\033[0m %s\n" "$label" + PASSED=$((PASSED + 1)) + else + printf " \033[31mFAIL\033[0m %s\n expected: [%s]\n actual: [%s]\n" \ + "$label" "$expected" "$actual" + FAILED=$((FAILED + 1)) + fi +} + +assert_exit() { + local label="$1" hook="$2" payload="$3" expected="$4" actual + PATH="$WORK/bin:$PATH" bash "$hook" <<< "$payload" >/dev/null 2>&1 + actual=$? + if [ "$actual" = "$expected" ]; then + printf " \033[32mPASS\033[0m %s\n" "$label" + PASSED=$((PASSED + 1)) + else + printf " \033[31mFAIL\033[0m %s (expected exit %s, got %s)\n" "$label" "$expected" "$actual" + FAILED=$((FAILED + 1)) + fi +} + +claude_payload() { jq -n --arg p "$1" '{"tool_input":{"file_path":$p}}'; } +codex_payload() { jq -n --arg c "$1" --arg d "$WORK/repo" '{"cwd":$d,"tool_input":{"command":$c}}'; } + +echo "=== Claude Code payloads (tool_input.file_path) ===" +assert_files "Edit on a .dart file" "$ANALYZE" \ + "$(claude_payload "$WORK/repo/lib/a.dart")" "a.dart " +assert_files "non-Dart file is ignored" "$ANALYZE" \ + "$(claude_payload "$WORK/repo/README.md")" "" +assert_files "path that does not exist is ignored" "$ANALYZE" \ + "$(claude_payload "$WORK/repo/lib/gone.dart")" "" +assert_files "empty payload" "$ANALYZE" '{}' "" + +echo "" +echo "=== Codex payloads (tool_input.command, apply_patch envelope) ===" +assert_files "Update File, absolute path" "$ANALYZE" \ + "$(codex_payload "*** Begin Patch +*** Update File: $WORK/repo/lib/a.dart +@@ +-old ++new +*** End Patch")" "a.dart " + +assert_files "Update File, path relative to cwd" "$ANALYZE" \ + "$(codex_payload '*** Begin Patch +*** Update File: lib/a.dart +@@ +-old ++new +*** End Patch')" "a.dart " + +assert_files "Add File" "$ANALYZE" \ + "$(codex_payload '*** Begin Patch +*** Add File: lib/b.dart ++void main() {} +*** End Patch')" "b.dart " + +assert_files "several files in one patch" "$ANALYZE" \ + "$(codex_payload '*** Begin Patch +*** Add File: lib/b.dart ++void main() {} +*** Update File: lib/a.dart +@@ +-old ++new +*** End Patch')" "a.dart b.dart " + +assert_files "Delete File is skipped" "$ANALYZE" \ + "$(codex_payload '*** Begin Patch +*** Delete File: lib/a.dart +*** End Patch')" "" + +assert_files "Delete File does not swallow the preceding file" "$ANALYZE" \ + "$(codex_payload '*** Begin Patch +*** Update File: lib/a.dart +@@ +-old ++new +*** Delete File: lib/gone.dart +*** End Patch')" "a.dart " + +# A rename lists both paths; only the destination exists once the patch lands. +assert_files "Move to wins over the original path" "$ANALYZE" \ + "$(codex_payload '*** Begin Patch +*** Update File: lib/gone.dart +*** Move to: lib/renamed.dart +@@ +-old ++new +*** End Patch')" "renamed.dart " + +assert_files "non-Dart files in a patch are ignored" "$ANALYZE" \ + "$(codex_payload '*** Begin Patch +*** Update File: README.md +@@ +-old ++new +*** End Patch')" "" + +assert_files "a patch touching nothing that exists" "$ANALYZE" \ + "$(codex_payload '*** Begin Patch +*** Update File: lib/nope.dart +@@ +-old ++new +*** End Patch')" "" + +echo "" +echo "=== Payloads that must not be read as an edit ===" +# Otherwise the hooks would fire on ordinary shell calls. +assert_files "a Bash command naming a .dart file" "$ANALYZE" \ + "$(codex_payload 'cat lib/a.dart')" "" +assert_files "a shell command quoting patch markers" "$ANALYZE" \ + "$(codex_payload 'echo "*** Add File: lib/a.dart"')" "" +# A heredoc puts a patch marker at the start of its own line, so only the +# "*** Begin Patch" guard distinguishes this from a real edit. +assert_files "a heredoc whose body starts with a patch marker" "$ANALYZE" \ + "$(codex_payload 'cat < notes.txt +*** Add File: lib/a.dart +EOF')" "" +assert_files "a patch envelope that is not the first thing in the command" "$ANALYZE" \ + "$(codex_payload 'echo hi +*** Update File: lib/a.dart')" "" + +echo "" +echo "=== Exit codes ===" +assert_exit "analyze exits 0 when the analyzer passes" "$ANALYZE" \ + "$(claude_payload "$WORK/repo/lib/a.dart")" 0 +DART_STUB_FAIL=1 assert_exit "analyze exits 2 so the model sees the failure" "$ANALYZE" \ + "$(claude_payload "$WORK/repo/lib/a.dart")" 2 +assert_exit "analyze exits 0 with nothing to do" "$ANALYZE" '{}' 0 + +echo "" +echo "=== format.sh ===" +assert_files "format reads a Claude payload" "$FORMAT" \ + "$(claude_payload "$WORK/repo/lib/a.dart")" "a.dart " +assert_files "format reads a Codex patch" "$FORMAT" \ + "$(codex_payload '*** Begin Patch +*** Update File: lib/a.dart +@@ +-old ++new +*** End Patch')" "a.dart " +assert_exit "format never blocks, even on failure" "$FORMAT" \ + "$(claude_payload "$WORK/repo/lib/a.dart")" 0 + +echo "" +echo "=== Results: $PASSED passed, $FAILED failed ===" + +if [ "$FAILED" -gt 0 ]; then + exit 1 +fi diff --git a/hooks/scripts/format.sh b/hooks/scripts/format.sh index a4497e4..ac161b8 100755 --- a/hooks/scripts/format.sh +++ b/hooks/scripts/format.sh @@ -10,18 +10,43 @@ if ! command -v jq &>/dev/null; then exit 0 fi -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -# shellcheck source=hooks/scripts/hook-payload-common.sh -source "$SCRIPT_DIR/hook-payload-common.sh" +# Which files did this edit touch? The two harnesses answer differently: +# +# Claude Code Edit / Write -> .tool_input.file_path (one path) +# Codex apply_patch -> .tool_input.command (a patch envelope, no path) +# +# Codex hook payloads carry no file path and no changed-file list, so the paths +# are read out of the patch headers. A rename emits both the old and the new +# path; the old one no longer exists, so the -f test below drops it. Deleted +# files never match, since only Add/Update/Move headers are selected. +paths=$(jq -r ' + if .tool_input.file_path then .tool_input.file_path + else + (.tool_input.command // "") + | select(startswith("*** Begin Patch")) + | split("\n")[] + | select(test("^\\*\\*\\* (Add File|Update File|Move to): ")) + | sub("^\\*\\*\\* (Add File|Update File|Move to): "; "") + end' <<< "$input") + +cwd=$(jq -r '.cwd // empty' <<< "$input") -# Collect the Dart files this edit touched. Claude Code reports one `file_path`; -# Codex reports an apply_patch envelope that may cover several files. files=() while IFS= read -r file; do - if [ -n "$file" ]; then + [ -n "$file" ] || continue + case "$file" in + *.dart) ;; + *) continue ;; + esac + # apply_patch paths may be relative to the session working directory. + case "$file" in + /*) ;; + *) if [ -n "$cwd" ]; then file="$cwd/$file"; fi ;; + esac + if [ -f "$file" ]; then files+=("$file") fi -done < <(changed_dart_files "$input") +done <<< "$paths" # Nothing Dart in this edit if [ ${#files[@]} -eq 0 ]; then diff --git a/hooks/scripts/hook-payload-common.sh b/hooks/scripts/hook-payload-common.sh deleted file mode 100755 index a34b18a..0000000 --- a/hooks/scripts/hook-payload-common.sh +++ /dev/null @@ -1,92 +0,0 @@ -#!/bin/bash -# Shared helpers for reading a hook payload that may come from either harness. -# -# Claude Code and Codex describe the same edit differently: -# -# Claude Code Edit / Write -> .tool_input.file_path (one path) -# Codex apply_patch -> .tool_input.command (an apply_patch envelope) -# -# changed_dart_files() normalizes both into a newline-separated list of existing -# `.dart` paths, so analyze.sh and format.sh stay single-sourced across harnesses. -# -# Every branch is written with `if` rather than `&&` so that sourcing this file -# from a script running under `set -e` cannot abort on a false test. - -# Print the paths an apply_patch envelope creates or updates, one per line. -# -# $1 = the raw envelope. -# -# Grammar (from the Codex apply_patch parser): -# begin_patch: "*** Begin Patch" LF -# add_hunk: "*** Add File: " filename LF add_line+ -# delete_hunk: "*** Delete File: " filename LF -# update_hunk: "*** Update File: " filename LF change_move? change? -# change_move: "*** Move to: " filename LF -# -# Deleted files are skipped — there is nothing left to analyze or format. For a -# renamed file the `Move to:` destination wins, because that is the path on disk -# once the patch lands. -apply_patch_paths() { - printf '%s\n' "$1" | awk ' - function flush() { if (path != "") { print path; path = "" } } - /^\*\*\* (Add|Update) File: / { flush(); path = substr($0, index($0, ": ") + 2); next } - /^\*\*\* Move to: / { path = substr($0, index($0, ": ") + 2); next } - /^\*\*\* Delete File: / { flush(); next } - /^\*\*\* End Patch/ { flush(); next } - END { flush() } - ' -} - -# Print the `.dart` files a hook payload touched, one per line. -# -# $1 = the raw hook payload JSON. Only files that exist on disk are printed, so a -# deleted or moved-away path never reaches `dart analyze`. -changed_dart_files() { - local input="$1" - local file_path command cwd path - - # Claude Code: a single explicit path. - file_path=$(printf '%s' "$input" | jq -r '.tool_input.file_path // empty') - if [ -n "$file_path" ]; then - case "$file_path" in - *.dart) - if [ -f "$file_path" ]; then - printf '%s\n' "$file_path" - fi - ;; - esac - return 0 - fi - - # Codex: an apply_patch envelope in `command`. - command=$(printf '%s' "$input" | jq -r '.tool_input.command // empty') - case "$command" in - '*** Begin Patch'*) ;; - *) return 0 ;; - esac - - cwd=$(printf '%s' "$input" | jq -r '.cwd // empty') - while IFS= read -r path; do - if [ -z "$path" ]; then - continue - fi - case "$path" in - *.dart) ;; - *) continue ;; - esac - # apply_patch paths may be relative to the session working directory. - case "$path" in - /*) ;; - *) - if [ -n "$cwd" ]; then - path="$cwd/$path" - fi - ;; - esac - if [ -f "$path" ]; then - printf '%s\n' "$path" - fi - done < <(apply_patch_paths "$command") - - return 0 -} diff --git a/hooks/scripts/hook-payload-common_test.sh b/hooks/scripts/hook-payload-common_test.sh deleted file mode 100755 index 89cd833..0000000 --- a/hooks/scripts/hook-payload-common_test.sh +++ /dev/null @@ -1,170 +0,0 @@ -#!/bin/bash -# Tests for hook-payload-common.sh -# -# Usage: bash hooks/scripts/hook-payload-common_test.sh -# -# changed_dart_files() reads a hook payload and prints the `.dart` files it -# touched. It has to read both shapes the two harnesses produce: -# -# Claude Code Edit / Write -> .tool_input.file_path -# Codex apply_patch -> .tool_input.command (an apply_patch envelope) -# -# Each case builds real files in a temp tree, because the helper only reports -# paths that exist on disk. - -set -uo pipefail - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -# shellcheck source=hooks/scripts/hook-payload-common.sh -source "$SCRIPT_DIR/hook-payload-common.sh" - -PASSED=0 -FAILED=0 - -WORK=$(mktemp -d) -trap 'rm -rf "$WORK"' EXIT - -mkdir -p "$WORK/lib" -: > "$WORK/lib/a.dart" -: > "$WORK/lib/b.dart" -: > "$WORK/lib/renamed.dart" -: > "$WORK/README.md" - -# Compare the helper's output (sorted) against the expected newline-separated list. -assert_files() { - local label="$1" payload="$2" expected="$3" - local actual - actual=$(changed_dart_files "$payload" | LC_ALL=C sort | tr '\n' ' ') - expected=$(printf '%s' "$expected" | tr '\n' ' ') - if [ "$actual" = "$expected" ]; then - printf " \033[32mPASS\033[0m %s\n" "$label" - PASSED=$((PASSED + 1)) - else - printf " \033[31mFAIL\033[0m %s\n expected: [%s]\n actual: [%s]\n" \ - "$label" "$expected" "$actual" - FAILED=$((FAILED + 1)) - fi -} - -patch_payload() { - jq -n --arg c "$1" --arg d "$WORK" '{"cwd":$d,"tool_input":{"command":$c}}' -} - -echo "=== Claude Code payloads (tool_input.file_path) ===" - -assert_files "Edit on a .dart file" \ - "$(jq -n --arg p "$WORK/lib/a.dart" '{"tool_input":{"file_path":$p}}')" \ - "$WORK/lib/a.dart " - -assert_files "Write to a non-Dart file is ignored" \ - "$(jq -n --arg p "$WORK/README.md" '{"tool_input":{"file_path":$p}}')" \ - "" - -assert_files "path that does not exist is ignored" \ - "$(jq -n --arg p "$WORK/lib/gone.dart" '{"tool_input":{"file_path":$p}}')" \ - "" - -assert_files "empty payload" '{}' "" - -echo "" -echo "=== Codex payloads (tool_input.command, apply_patch envelope) ===" - -assert_files "Update File with an absolute path" \ - "$(patch_payload "*** Begin Patch -*** Update File: $WORK/lib/a.dart -@@ -- print(\"hi\"); -+ print(\"bye\"); -*** End Patch")" \ - "$WORK/lib/a.dart " - -assert_files "Update File with a path relative to cwd" \ - "$(patch_payload '*** Begin Patch -*** Update File: lib/a.dart -@@ --old -+new -*** End Patch')" \ - "$WORK/lib/a.dart " - -assert_files "Add File" \ - "$(patch_payload '*** Begin Patch -*** Add File: lib/b.dart -+void main() {} -*** End Patch')" \ - "$WORK/lib/b.dart " - -assert_files "several files in one patch" \ - "$(patch_payload '*** Begin Patch -*** Add File: lib/b.dart -+void main() {} -*** Update File: lib/a.dart -@@ --old -+new -*** End Patch')" \ - "$WORK/lib/a.dart $WORK/lib/b.dart " - -assert_files "Delete File is skipped" \ - "$(patch_payload '*** Begin Patch -*** Delete File: lib/a.dart -*** End Patch')" \ - "" - -assert_files "Delete File does not swallow the preceding file" \ - "$(patch_payload '*** Begin Patch -*** Update File: lib/a.dart -@@ --old -+new -*** Delete File: lib/gone.dart -*** End Patch')" \ - "$WORK/lib/a.dart " - -assert_files "Move to wins over the original path" \ - "$(patch_payload '*** Begin Patch -*** Update File: lib/gone.dart -*** Move to: lib/renamed.dart -@@ --old -+new -*** End Patch')" \ - "$WORK/lib/renamed.dart " - -assert_files "non-Dart files in a patch are ignored" \ - "$(patch_payload '*** Begin Patch -*** Update File: README.md -@@ --old -+new -*** End Patch')" \ - "" - -assert_files "a patch that touches nothing that exists" \ - "$(patch_payload '*** Begin Patch -*** Update File: lib/nope.dart -@@ --old -+new -*** End Patch')" \ - "" - -echo "" -echo "=== Payloads that must not be read as a patch ===" - -# A Bash command that merely mentions a .dart file must never be treated as an -# edit — otherwise the PostToolUse hooks would fire on every shell call. -assert_files "a Bash command is not an apply_patch envelope" \ - "$(patch_payload 'cat lib/a.dart')" \ - "" - -assert_files "a shell command quoting patch markers is not an envelope" \ - "$(patch_payload 'echo "*** Begin Patch"')" \ - "" - -echo "" -echo "=== Results: $PASSED passed, $FAILED failed ===" - -if [ "$FAILED" -gt 0 ]; then - exit 1 -fi From 5121b476a2ea61588e815aca6ef3fce7ec0b37a4 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 9 Sep 2026 11:13:43 +0200 Subject: [PATCH 12/18] docs: record why analyze and format stay separate hooks Two scripts is a deliberate maintainability choice over merging them. Notes the two consequences so neither reads as a bug later: Claude Code runs hooks in a matcher group in parallel, so the pair race on the same file, and the payload jq is duplicated rather than shared. Co-Authored-By: Claude Opus 5 --- CONTRIBUTING.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 19c7472..45a3709 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -268,6 +268,14 @@ copy of the hooks. Verified against Codex CLI 0.153.4: entries all use `source: github`, and why the two manifests coexist in that repo. Because the `url` source always resolves the default branch, `codex/loader_test.sh` synthesizes its own throwaway marketplace pointing at the working tree instead. +- **`analyze.sh` and `format.sh` stay two separate hooks.** They are easier to maintain and + reason about apart, which is a deliberate choice over merging them. Two consequences to know. + Claude Code runs every hook in a matcher group **in parallel**, so the two race on the same + file: `dart format` rewrites it while `dart analyze` reads it. Formatting does not change + semantics, so the analyzer reports the same findings either way, though line numbers can refer + to the pre-format file. And because they do not share a helper, the payload-reading `jq` + expression is duplicated in both — keep the copies identical, and add cases to + `dart-hooks_test.sh`, which exercises both scripts. - **A plugin cannot ship a Codex subagent.** Codex loads custom agents only from `~/.codex/agents/` or a project's `.codex/agents/`, and `agents` is not a plugin manifest field or a discovery path. `codex/agents/flutter-reviewer.toml` is therefore a file users copy, either into From 1663a40c9abf59872fdd4d8fe2786760cdb8801f Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 9 Sep 2026 11:19:05 +0200 Subject: [PATCH 13/18] refactor: drop the Codex loader test and the dart hooks test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes codex/loader_test.sh, hooks/scripts/dart-hooks_test.sh, and the codex-loader CI job, per review: no new test suites, only the two existing ones. Consequences, recorded in CONTRIBUTING.md rather than left implicit. CI no longer exercises Codex at all, so a change to the hooks or codex/ has to be verified by hand — the instructions for doing that replace the "run the loader test" step. And the payload reading in analyze.sh and format.sh ships without automated coverage, so the two jq copies have to be kept identical by review. Also drops the cspell entries for words that only existed in the removed documentation. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yaml | 9 - AGENTS.md | 4 +- CLAUDE.md | 8 +- CONTRIBUTING.md | 31 ++- codex/loader_test.sh | 346 ------------------------------- config/cspell.json | 1 - hooks/scripts/dart-hooks_test.sh | 217 ------------------- 7 files changed, 19 insertions(+), 597 deletions(-) delete mode 100755 codex/loader_test.sh delete mode 100755 hooks/scripts/dart-hooks_test.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index eb907f4..6562e56 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -96,12 +96,3 @@ jobs: echo "::endgroup::" done exit $status - codex-loader: - name: 🤖 Codex Loader - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v7 - - name: Install Codex CLI - run: npm install -g @openai/codex - - name: Assert the plugin installs and loads in Codex - run: bash codex/loader_test.sh diff --git a/AGENTS.md b/AGENTS.md index 7a4b633..b7ec72f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,7 +13,6 @@ VGV AI Flutter Plugin provides best-practices skills for Flutter and Dart develo agents/ flutter-reviewer.md # Read-only Flutter code reviewer subagent codex/ # The only Codex-specific assets; skills, MCP and hooks are shared - loader_test.sh # Installs the repo as a Codex plugin and asserts it loads (codex-loader CI job) agents/ flutter-reviewer.toml # Codex port of agents/flutter-reviewer.md — users copy it to ~/.codex/agents/ docs/ @@ -184,8 +183,7 @@ documentation in the same change: describe an edit differently (Claude Code `tool_input.file_path`, Codex `tool_input.command` holding an apply_patch envelope). `analyze.sh` and `format.sh` each read both shapes with the same inline `jq` expression — keep - the two copies identical, and add a case to `dart-hooks_test.sh`, which drives - both scripts through a stub `dart` so it needs no SDK. + the two copies identical. - **Changing `agents/flutter-reviewer.md`** — port the same change to `codex/agents/flutter-reviewer.toml`. A Codex plugin cannot ship a subagent, so that file is a separate copy users install by hand. Its output contract (the diff --git a/CLAUDE.md b/CLAUDE.md index ef13fbf..5caeffa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,7 +37,7 @@ These run **after** a tool call completes: Both resolve the changed files with the same inline `jq` expression, handling Claude Code's `tool_input.file_path` and Codex's `tool_input.command` (an `apply_patch` envelope, which can name several files at once). That is the only harness-specific branch in the hook scripts, and the two -copies must stay identical — `dart-hooks_test.sh` covers both. +copies must stay identical. All hook scripts require **jq** to parse the hook payload (they skip gracefully if `jq` is not installed). @@ -51,6 +51,6 @@ compatibility alias. That is why the `PostToolUse` matcher says `apply_patch|Edi names its file-editing tool `apply_patch`, and the extra alternative is inert on Claude Code. The only Codex-specific asset is `codex/agents/flutter-reviewer.toml`, because Codex has no way to -bundle a subagent in a plugin — users copy it to `~/.codex/agents/` themselves. `codex/loader_test.sh` installs the repo the way a user would and asserts Codex -picks it all up. Change a hook or the reviewer agent and both harnesses are affected — see -`AGENTS.md` → Maintaining Existing Skills, Hooks, and MCP Tools. +bundle a subagent in a plugin — users copy it to `~/.codex/agents/` themselves. Change a hook or +the reviewer agent and both harnesses are affected — see `AGENTS.md` → Maintaining Existing +Skills, Hooks, and MCP Tools. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 45a3709..aa948b5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -208,9 +208,8 @@ copy of the hooks. Verified against Codex CLI 0.153.4: not worth a second manifest to keep in version-sync with the Claude Code one. Note the consequence: `.claude-plugin/plugin.json` is now load-bearing for **both** harnesses, so its `name` and `version` are not Claude-only fields any more. If you ever add - `.codex-plugin/plugin.json`, `codex/loader_test.sh` will fail until you restore field checks for - it — the manifest is rejected outright if it carries a `hooks` key or any field outside Codex's - allowed set. + `.codex-plugin/plugin.json`, note that Codex rejects a manifest carrying a `hooks` key or any + field outside its allowed set. - **Hooks come from default discovery.** Codex looks for a plugin's hooks at `/hooks/hooks.json` — the same path and file Claude Code uses — and resolves `${CLAUDE_PLUGIN_ROOT}` inside it, documented as a compatibility alias alongside its own @@ -229,7 +228,7 @@ copy of the hooks. Verified against Codex CLI 0.153.4: (`codex features list` shows it enabled); there is no `codex_hooks` flag. Codex also runs hooks on Windows and offers a `commandWindows` override — but these scripts are `bash` and need `jq`, so Windows means WSL or Git Bash. Codex **silently ignores a malformed `hooks.json`**, which disables - the whole enforcement layer with no error, so `codex/loader_test.sh` validates the installed file. + the whole enforcement layer with no error, so validate it by hand after editing it. - **Five of the six scripts need nothing.** Codex passes `tool_name: "Bash"` with `tool_input.command` as a plain string and accepts the same `permissionDecision` allow/deny JSON, so `check-vgv-cli.sh`, `block-cli-workarounds.sh`, and `allow-readonly-git.sh` are untouched; @@ -240,7 +239,7 @@ copy of the hooks. Verified against Codex CLI 0.153.4: changed-file list, so both hooks read the paths out of the patch headers with the same inline `jq` expression. A rename lists the old and new path and a delete lists none, so an existence check is all the bookkeeping needed. The expression is duplicated in the two scripts rather than - shared through a third file; keep the copies identical and covered by `dart-hooks_test.sh`. + shared through a third file; keep the copies identical. - **One marketplace serves both harnesses.** `codex plugin add` only accepts `PLUGIN@MARKETPLACE`, so a marketplace is mandatory — but it is `very-good-claude-code-marketplace`, the same repo Claude Code uses, not this one. That @@ -266,8 +265,8 @@ copy of the hooks. Verified against Codex CLI 0.153.4: `codex plugin list` just reports "No marketplace plugins found", with no error anywhere. That is why Codex cannot read the existing `.claude-plugin/marketplace.json`, whose entries all use `source: github`, and why the two manifests coexist in that repo. - Because the `url` source always resolves the default branch, `codex/loader_test.sh` - synthesizes its own throwaway marketplace pointing at the working tree instead. + Note that the `url` source always resolves the default branch, so testing an unmerged change + means pointing a throwaway marketplace at your checkout with a `local` source instead. - **`analyze.sh` and `format.sh` stay two separate hooks.** They are easier to maintain and reason about apart, which is a deliberate choice over merging them. Two consequences to know. Claude Code runs every hook in a matcher group **in parallel**, so the two race on the same @@ -275,7 +274,7 @@ copy of the hooks. Verified against Codex CLI 0.153.4: semantics, so the analyzer reports the same findings either way, though line numbers can refer to the pre-format file. And because they do not share a helper, the payload-reading `jq` expression is duplicated in both — keep the copies identical, and add cases to - `dart-hooks_test.sh`, which exercises both scripts. + the two scripts. - **A plugin cannot ship a Codex subagent.** Codex loads custom agents only from `~/.codex/agents/` or a project's `.codex/agents/`, and `agents` is not a plugin manifest field or a discovery path. `codex/agents/flutter-reviewer.toml` is therefore a file users copy, either into @@ -291,10 +290,11 @@ copy of the hooks. Verified against Codex CLI 0.153.4: - **Do not weaken the Claude Code path** to make Codex simpler. `hooks/hooks.json` and `agents/flutter-reviewer.md` stay authoritative. -Run `bash codex/loader_test.sh` before pushing a change to any of it. It installs the working tree -the way a user would — `codex plugin marketplace add` then `codex plugin add`, into a throwaway -`CODEX_HOME` — and asserts what Codex picked up. It needs the `codex` CLI but no credentials, since -it reads `codex debug prompt-input` and `codex doctor --json` rather than calling a model. +Nothing in CI exercises Codex, so verify a change to any of it by hand. Install the working tree +into a throwaway `CODEX_HOME` the way a user would (`codex plugin marketplace add` then +`codex plugin add`, with a scratch marketplace whose entry is a `local` path to your checkout), +then check what Codex picked up with `codex debug prompt-input` and `codex doctor --json`. Both +read local state without calling a model, so this needs the `codex` CLI but no credentials. **Invocation** — every skill in this plugin is **model-invoked**: the model may reach for it autonomously when the context fits (that is the point of a best-practice skill), so neither @@ -318,10 +318,8 @@ session and exercise it before you commit. - **Dart SDK** and **jq** on your `PATH` — the hooks need both. - **Very Good CLI** ≥ 1.3.0 (`dart pub global activate very_good_cli`) for the Very Good CLI MCP server tools. -- **Codex CLI** (`npm install -g @openai/codex`) and **Python 3.11+** only if you - touch the Codex manifests, the hooks, or `codex/` — `codex/loader_test.sh` needs - both (Python parses the agent TOML; on 3.10 or older, - `python3 -m pip install tomli`). Everything else runs without them. +- **Codex CLI** (`npm install -g @openai/codex`) only if you touch the hooks or + `codex/`, to verify the change by hand. Everything else runs without it. See the README [Hooks](README.md#hooks) and [MCP Integration](README.md#mcp-integration) sections for the full prerequisite details. @@ -416,7 +414,6 @@ Every pull request runs the following checks automatically: | Skill validation | Validates **every** `SKILL.md`'s frontmatter and structure against the Agent Skills spec, so a malformed skill fails the build instead of silently vanishing on another host | `Flash-Brew-Digital/validate-skill@v1` | | Plugin validation | Validates and test-installs the plugin | `claude plugin validate .` | | Script tests | Runs every hook script test suite | `hooks/scripts/*_test.sh` | -| Codex loader | Installs the plugin as a Codex plugin into a throwaway Codex home and asserts all 15 skills, both MCP servers, and the hooks load | `codex/loader_test.sh` | Evals do **not** run on a pull request. They call real models, so they run after a merge to `main` instead, scoped to the skills that changed: diff --git a/codex/loader_test.sh b/codex/loader_test.sh deleted file mode 100755 index bcb59e3..0000000 --- a/codex/loader_test.sh +++ /dev/null @@ -1,346 +0,0 @@ -#!/bin/bash -# Asserts that Codex loads this plugin through its own native install path. -# -# Usage: bash codex/loader_test.sh -# -# Installs the working tree as a Codex plugin into a throwaway CODEX_HOME — -# `codex plugin marketplace add` then `codex plugin add`, the same two commands -# a user runs — and then checks what Codex actually picked up. Your real Codex -# configuration is never touched. -# -# Needs the `codex` CLI but no credentials: the assertions go through -# `codex debug prompt-input` and `codex doctor --json`, which render local state -# without contacting a model. -# -# Codex silently ignores a malformed hooks.json and ships no validator for agent -# files, so those two are checked here directly rather than through the CLI. - -set -uo pipefail - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -PLUGIN_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" -# This repo ships no Codex plugin manifest. Codex falls back to the Claude Code -# one for the plugin's identity, which makes that file load-bearing for both -# harnesses — hence reading the name from it here. -MANIFEST="$PLUGIN_ROOT/.claude-plugin/plugin.json" - -PASSED=0 -FAILED=0 -pass() { printf " \033[32mPASS\033[0m %s\n" "$1"; PASSED=$((PASSED + 1)); } -fail() { - printf " \033[31mFAIL\033[0m %s\n" "$1" - if [ $# -gt 1 ]; then printf " %s\n" "$2"; fi - FAILED=$((FAILED + 1)) -} -assert_eq() { - if [ "$2" = "$3" ]; then pass "$1"; else fail "$1" "expected [$2], got [$3]"; fi -} - -for tool in codex jq python3; do - if ! command -v "$tool" &>/dev/null; then - printf "\033[31merror\033[0m %s is required to run the Codex loader test\n" "$tool" >&2 - exit 1 - fi -done - -# The agent files are TOML and Codex ships no validator for them, so they are -# parsed here. tomllib is standard from Python 3.11; older versions need tomli. -TOML_MODULE="" -for candidate in tomllib tomli; do - if python3 -c "import $candidate" 2>/dev/null; then - TOML_MODULE="$candidate" - break - fi -done -if [ -z "$TOML_MODULE" ]; then - printf "\033[31merror\033[0m no TOML parser available for %s\n" "$(python3 -V 2>&1)" >&2 - printf " use Python 3.11+ (stdlib tomllib), or: python3 -m pip install tomli\n" >&2 - exit 1 -fi - -SANDBOX=$(mktemp -d) -trap 'rm -rf "$SANDBOX"' EXIT -FAKE_HOME="$SANDBOX/home" -FAKE_CODEX_HOME="$SANDBOX/codex" -WORKDIR="$SANDBOX/project" -mkdir -p "$FAKE_HOME" "$FAKE_CODEX_HOME" "$WORKDIR" - -# Codex scans for repo-scoped skills up to the repository root, so the scratch -# project is a git repository, matching what a real user would have. -git -C "$WORKDIR" init -q - -codex_in_sandbox() { - env HOME="$FAKE_HOME" CODEX_HOME="$FAKE_CODEX_HOME" codex "$@" -} - -printf '\033[1mCodex %s\033[0m\n' "$(codex --version 2>/dev/null | head -1)" - -echo "" -echo "=== Plugin identity ===" -PLUGIN_NAME=$(jq -r '.name // empty' "$MANIFEST" 2>/dev/null) -if [ -n "$PLUGIN_NAME" ]; then - pass "plugin identity resolves from .claude-plugin/plugin.json (name: $PLUGIN_NAME)" -else - fail "plugin identity resolves from .claude-plugin/plugin.json" - exit 1 -fi -if [ ! -e "$PLUGIN_ROOT/.codex-plugin" ]; then - pass "no Codex-specific plugin manifest to keep in sync" -else - fail "no Codex-specific plugin manifest to keep in sync" \ - "found .codex-plugin — either delete it or restore its checks here" -fi - -echo "" -echo "=== Native install ===" -# The published marketplace entry lives in very-good-claude-code-marketplace and -# points here with a remote `url` source, so it always resolves the default -# branch. To test *this* working tree instead, synthesize a throwaway marketplace -# whose single entry is a local path — a symlink back to the checkout. -MARKETPLACE_NAME="loader-test" -MARKETPLACE_ROOT="$SANDBOX/marketplace" -mkdir -p "$MARKETPLACE_ROOT/.agents/plugins" "$MARKETPLACE_ROOT/plugins" -ln -s "$PLUGIN_ROOT" "$MARKETPLACE_ROOT/plugins/$PLUGIN_NAME" -jq -n --arg mp "$MARKETPLACE_NAME" --arg n "$PLUGIN_NAME" '{ - name: $mp, - interface: { displayName: "Loader Test" }, - plugins: [ { - name: $n, - source: { source: "local", path: ("./plugins/" + $n) }, - policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, - category: "Productivity" - } ] -}' > "$MARKETPLACE_ROOT/.agents/plugins/marketplace.json" - -if codex_in_sandbox plugin marketplace add "$MARKETPLACE_ROOT" >"$SANDBOX/mp.log" 2>&1; then - pass "codex plugin marketplace add accepts the marketplace" -else - fail "codex plugin marketplace add accepts the marketplace" "$(tail -3 "$SANDBOX/mp.log")" - cat "$SANDBOX/mp.log" >&2 - exit 1 -fi -# A marketplace entry Codex cannot resolve is dropped silently, so confirm the -# plugin is actually listed before trying to install it. -if codex_in_sandbox plugin list 2>/dev/null | grep -q "$PLUGIN_NAME@$MARKETPLACE_NAME"; then - pass "the plugin resolves from the marketplace entry" -else - fail "the plugin resolves from the marketplace entry" \ - "$(codex_in_sandbox plugin list 2>&1 | tail -2)" - exit 1 -fi -if codex_in_sandbox plugin add "$PLUGIN_NAME@$MARKETPLACE_NAME" >"$SANDBOX/add.log" 2>&1; then - pass "codex plugin add installs the plugin" -else - fail "codex plugin add installs the plugin" "$(tail -3 "$SANDBOX/add.log")" - cat "$SANDBOX/add.log" >&2 - exit 1 -fi - -INSTALLED_ROOT=$(sed -n 's/^Installed plugin root: //p' "$SANDBOX/add.log" | tail -1) -if [ -n "$INSTALLED_ROOT" ] && [ -d "$INSTALLED_ROOT" ]; then - pass "the installed plugin root exists" -else - fail "the installed plugin root exists" "reported [${INSTALLED_ROOT:-none}]" - exit 1 -fi - -echo "" -echo "=== Skills load ===" -PROMPT_JSON="$SANDBOX/prompt-input.json" -if (cd "$WORKDIR" && codex_in_sandbox debug prompt-input "hello" >"$PROMPT_JSON" 2>"$SANDBOX/pi.err"); then - pass "codex debug prompt-input succeeds" -else - fail "codex debug prompt-input succeeds" "$(tail -5 "$SANDBOX/pi.err")" - exit 1 -fi - -expected=0 -missing="" -for dir in "$PLUGIN_ROOT"/skills/*/; do - [ -f "$dir/SKILL.md" ] || continue - name="$(basename "$dir")" - expected=$((expected + 1)) - # Codex namespaces a plugin's skills as :; accept either form. - if ! grep -qE -- "- ($PLUGIN_NAME:)?$name: " "$PROMPT_JSON"; then - missing="$missing $name" - fi -done -if [ "$expected" -eq 0 ]; then - fail "found skills to check" "no SKILL.md files under $PLUGIN_ROOT/skills" -elif [ -n "$missing" ]; then - fail "all $expected skills appear in the Codex prompt" "missing:$missing" -else - pass "all $expected skills appear in the Codex prompt" -fi -# They must come from the installed plugin, not from some other skills root. -if grep -qF "$INSTALLED_ROOT/skills" "$PROMPT_JSON"; then - pass "the skills root is the installed plugin" -else - fail "the skills root is the installed plugin" "$INSTALLED_ROOT/skills not listed" -fi - -echo "" -echo "=== MCP servers load ===" -DOCTOR_JSON="$SANDBOX/doctor.json" -codex_in_sandbox doctor --json >"$DOCTOR_JSON" 2>/dev/null -doctor_status() { - jq -r --arg id "$1" '.checks[] | select(.id == $id) | .status' "$DOCTOR_JSON" 2>/dev/null -} - -status=$(doctor_status config.load) -assert_eq "codex doctor: config.load is ok" "ok" "${status:-no such check}" - -# mcp.config degrades to a warning when the server executables are absent, which -# is the normal state anywhere without the Dart SDK and Very Good CLI installed, -# CI runners included. Only a hard failure means the config is wrong; what the -# servers were registered as is asserted below. -status=$(doctor_status mcp.config) -case "$status" in - ok) pass "codex doctor: mcp.config is ok" ;; - warning) pass "codex doctor: mcp.config has no errors (warning, likely no dart/very_good on PATH)" ;; - *) fail "codex doctor: mcp.config has no errors" "got [${status:-no such check}]" ;; -esac - -# Assert what each server was registered as, straight from .mcp.json, so this -# cannot drift from the file Claude Code reads. -while IFS= read -r server; do - want_command=$(jq -r --arg s "$server" '.mcpServers[$s].command' "$PLUGIN_ROOT/.mcp.json") - want_args=$(jq -r --arg s "$server" '(.mcpServers[$s].args // []) | join(" ")' "$PLUGIN_ROOT/.mcp.json") - out=$(codex_in_sandbox mcp get "$server" 2>/dev/null) - if [ -z "$out" ]; then - fail "MCP server '$server' is registered" - continue - fi - pass "MCP server '$server' is registered" - for field in "enabled: true" "transport: stdio" "command: $want_command" "args: $want_args"; do - if printf '%s\n' "$out" | grep -qF "$field"; then - pass " $server $field" - else - fail " $server $field" "$(printf '%s' "$out" | tr '\n' ' ')" - fi - done -done < <(jq -r '.mcpServers | keys[]' "$PLUGIN_ROOT/.mcp.json") - -echo "" -echo "=== Hooks ===" -# Codex discovers a plugin's hooks at /hooks/hooks.json — the same -# file Claude Code uses — and resolves ${CLAUDE_PLUGIN_ROOT} in it as a -# compatibility alias for the installed plugin directory. -INSTALLED_HOOKS="$INSTALLED_ROOT/hooks/hooks.json" -if [ -f "$INSTALLED_HOOKS" ]; then - pass "hooks.json is installed at the plugin hook-discovery path" -else - fail "hooks.json is installed at the plugin hook-discovery path" "$INSTALLED_HOOKS" -fi -if jq -e . "$INSTALLED_HOOKS" >/dev/null 2>&1; then - pass "installed hooks.json is valid JSON" -else - fail "installed hooks.json is valid JSON" -fi - -for event in SessionStart PreToolUse PostToolUse; do - if jq -e --arg e "$event" '.hooks[$e] | arrays and (length > 0)' "$INSTALLED_HOOKS" >/dev/null 2>&1; then - pass "$event is wired" - else - fail "$event is wired" - fi -done - -bad_shape=$(jq '[.hooks[][] | .hooks[]? | select((.type != "command") or ((.command | type) != "string"))] | length' "$INSTALLED_HOOKS") -assert_eq "every handler is a command handler with a string command" "0" "$bad_shape" - -# Codex names its file-editing tool apply_patch, so a matcher that only says -# Edit|Write would never fire there. Claude Code ignores the extra alternative. -if jq -e '[.hooks.PostToolUse[].matcher] | all(test("apply_patch"))' "$INSTALLED_HOOKS" >/dev/null 2>&1; then - pass "PostToolUse matchers cover Codex's apply_patch" -else - fail "PostToolUse matchers cover Codex's apply_patch" \ - "$(jq -c '[.hooks.PostToolUse[].matcher]' "$INSTALLED_HOOKS")" -fi - -# Every referenced script has to exist inside the installed plugin, or the hook -# is a silent no-op. This is also what proves ${CLAUDE_PLUGIN_ROOT} points at a -# real tree once expanded. -missing_scripts="" -checked=0 -while IFS= read -r command; do - [ -n "$command" ] || continue - script=$(printf '%s' "$command" \ - | sed -n 's|.*\${CLAUDE_PLUGIN_ROOT}/\([^" ]*\).*|\1|p') - [ -n "$script" ] || continue - checked=$((checked + 1)) - if [ ! -f "$INSTALLED_ROOT/$script" ]; then - missing_scripts="$missing_scripts $script" - fi -done < <(jq -r '[.hooks[][] | .hooks[]?.command] | .[]' "$INSTALLED_HOOKS") - -if [ "$checked" -eq 0 ]; then - fail "hook commands reference plugin-root scripts" "no \${CLAUDE_PLUGIN_ROOT} references found" -elif [ -n "$missing_scripts" ]; then - fail "all $checked hook scripts exist in the installed plugin" "missing:$missing_scripts" -else - pass "all $checked hook scripts exist in the installed plugin" -fi - -echo "" -echo "=== Agents ===" -# Codex reads custom agents from ~/.codex/agents or a project's .codex/agents, -# neither of which a plugin can populate, so the reviewer agent is a file users -# copy. Validate it here since Codex will not. -agent_field() { - python3 -c " -import sys, $TOML_MODULE as toml -with open(sys.argv[1], 'rb') as fh: - print(toml.load(fh).get(sys.argv[2], '')) -" "$1" "$2" 2>/dev/null -} - -for agent in "$PLUGIN_ROOT"/codex/agents/*.toml; do - [ -f "$agent" ] || continue - name="$(basename "$agent")" - if python3 -c " -import sys, $TOML_MODULE as toml -with open(sys.argv[1], 'rb') as fh: - data = toml.load(fh) -missing = [k for k in ('name', 'description', 'developer_instructions') if not data.get(k)] -if missing: - print('missing required fields: ' + ', '.join(missing), file=sys.stderr) - sys.exit(1) -" "$agent" - then - pass "$name parses and has the required fields" - else - fail "$name parses and has the required fields" - fi -done - -# Codex copies a custom agent verbatim, so the installed tree must carry it for -# the documented `cp` to work. -if [ -f "$INSTALLED_ROOT/codex/agents/flutter-reviewer.toml" ]; then - pass "the reviewer agent ships inside the installed plugin" -else - fail "the reviewer agent ships inside the installed plugin" -fi - -# The read-only reviewer must stay read-only: Codex has no per-agent tool -# allowlist, so the sandbox is the only thing enforcing it. -reviewer="$PLUGIN_ROOT/codex/agents/flutter-reviewer.toml" -if [ -f "$reviewer" ]; then - mode=$(agent_field "$reviewer" sandbox_mode) - assert_eq "flutter-reviewer is sandboxed read-only" "read-only" "${mode:-unset}" -fi - -echo "" -echo "=== Uninstall ===" -if codex_in_sandbox plugin remove "$PLUGIN_NAME@$MARKETPLACE_NAME" >"$SANDBOX/rm.log" 2>&1; then - pass "codex plugin remove uninstalls it" -else - fail "codex plugin remove uninstalls it" "$(tail -3 "$SANDBOX/rm.log")" -fi - -echo "" -echo "=== Results: $PASSED passed, $FAILED failed ===" - -if [ "$FAILED" -gt 0 ]; then - exit 1 -fi diff --git a/config/cspell.json b/config/cspell.json index d28c1c6..deb0a9a 100644 --- a/config/cspell.json +++ b/config/cspell.json @@ -60,7 +60,6 @@ "subagents", "subclassing", "tappable", - "tomli", "tooltipped", "unmirrored", "unrouted", diff --git a/hooks/scripts/dart-hooks_test.sh b/hooks/scripts/dart-hooks_test.sh deleted file mode 100755 index 9832289..0000000 --- a/hooks/scripts/dart-hooks_test.sh +++ /dev/null @@ -1,217 +0,0 @@ -#!/bin/bash -# Tests for analyze.sh and format.sh -# -# Usage: bash hooks/scripts/dart-hooks_test.sh -# -# Both hooks have to read two different payload shapes: -# -# Claude Code Edit / Write -> .tool_input.file_path -# Codex apply_patch -> .tool_input.command (a patch envelope) -# -# Getting that wrong fails silently — the hook exits 0 having done nothing — so -# these cases assert exactly which files reach the Dart SDK. A stub `dart` on -# PATH records its arguments, which keeps the suite fast and means CI needs no -# Dart SDK installed. - -set -uo pipefail - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -ANALYZE="$SCRIPT_DIR/analyze.sh" -FORMAT="$SCRIPT_DIR/format.sh" - -PASSED=0 -FAILED=0 - -WORK=$(mktemp -d) -trap 'rm -rf "$WORK"' EXIT - -# A `dart` that records the files it was asked to act on, and fails when told to. -mkdir -p "$WORK/bin" -cat > "$WORK/bin/dart" <<'STUB' -#!/bin/bash -subcommand="$1"; shift -: > "$DART_STUB_LOG" -for arg in "$@"; do - printf '%s\n' "$(basename "$arg")" >> "$DART_STUB_LOG" -done -if [ -n "${DART_STUB_FAIL:-}" ] && [ "$subcommand" = "analyze" ]; then - echo "error - stubbed analyzer failure" >&2 - exit 1 -fi -exit 0 -STUB -chmod +x "$WORK/bin/dart" -export DART_STUB_LOG="$WORK/dart.log" - -mkdir -p "$WORK/repo/lib" -: > "$WORK/repo/lib/a.dart" -: > "$WORK/repo/lib/b.dart" -: > "$WORK/repo/lib/renamed.dart" -: > "$WORK/repo/README.md" - -# Run a hook with the stub first on PATH. Echoes the basenames dart received. -run_hook() { - local hook="$1" payload="$2" - : > "$DART_STUB_LOG" - PATH="$WORK/bin:$PATH" bash "$hook" <<< "$payload" >/dev/null 2>&1 - LC_ALL=C sort "$DART_STUB_LOG" | tr '\n' ' ' -} - -assert_files() { - local label="$1" hook="$2" payload="$3" expected="$4" actual - actual=$(run_hook "$hook" "$payload") - expected=$(printf '%s' "$expected" | tr '\n' ' ') - if [ "$actual" = "$expected" ]; then - printf " \033[32mPASS\033[0m %s\n" "$label" - PASSED=$((PASSED + 1)) - else - printf " \033[31mFAIL\033[0m %s\n expected: [%s]\n actual: [%s]\n" \ - "$label" "$expected" "$actual" - FAILED=$((FAILED + 1)) - fi -} - -assert_exit() { - local label="$1" hook="$2" payload="$3" expected="$4" actual - PATH="$WORK/bin:$PATH" bash "$hook" <<< "$payload" >/dev/null 2>&1 - actual=$? - if [ "$actual" = "$expected" ]; then - printf " \033[32mPASS\033[0m %s\n" "$label" - PASSED=$((PASSED + 1)) - else - printf " \033[31mFAIL\033[0m %s (expected exit %s, got %s)\n" "$label" "$expected" "$actual" - FAILED=$((FAILED + 1)) - fi -} - -claude_payload() { jq -n --arg p "$1" '{"tool_input":{"file_path":$p}}'; } -codex_payload() { jq -n --arg c "$1" --arg d "$WORK/repo" '{"cwd":$d,"tool_input":{"command":$c}}'; } - -echo "=== Claude Code payloads (tool_input.file_path) ===" -assert_files "Edit on a .dart file" "$ANALYZE" \ - "$(claude_payload "$WORK/repo/lib/a.dart")" "a.dart " -assert_files "non-Dart file is ignored" "$ANALYZE" \ - "$(claude_payload "$WORK/repo/README.md")" "" -assert_files "path that does not exist is ignored" "$ANALYZE" \ - "$(claude_payload "$WORK/repo/lib/gone.dart")" "" -assert_files "empty payload" "$ANALYZE" '{}' "" - -echo "" -echo "=== Codex payloads (tool_input.command, apply_patch envelope) ===" -assert_files "Update File, absolute path" "$ANALYZE" \ - "$(codex_payload "*** Begin Patch -*** Update File: $WORK/repo/lib/a.dart -@@ --old -+new -*** End Patch")" "a.dart " - -assert_files "Update File, path relative to cwd" "$ANALYZE" \ - "$(codex_payload '*** Begin Patch -*** Update File: lib/a.dart -@@ --old -+new -*** End Patch')" "a.dart " - -assert_files "Add File" "$ANALYZE" \ - "$(codex_payload '*** Begin Patch -*** Add File: lib/b.dart -+void main() {} -*** End Patch')" "b.dart " - -assert_files "several files in one patch" "$ANALYZE" \ - "$(codex_payload '*** Begin Patch -*** Add File: lib/b.dart -+void main() {} -*** Update File: lib/a.dart -@@ --old -+new -*** End Patch')" "a.dart b.dart " - -assert_files "Delete File is skipped" "$ANALYZE" \ - "$(codex_payload '*** Begin Patch -*** Delete File: lib/a.dart -*** End Patch')" "" - -assert_files "Delete File does not swallow the preceding file" "$ANALYZE" \ - "$(codex_payload '*** Begin Patch -*** Update File: lib/a.dart -@@ --old -+new -*** Delete File: lib/gone.dart -*** End Patch')" "a.dart " - -# A rename lists both paths; only the destination exists once the patch lands. -assert_files "Move to wins over the original path" "$ANALYZE" \ - "$(codex_payload '*** Begin Patch -*** Update File: lib/gone.dart -*** Move to: lib/renamed.dart -@@ --old -+new -*** End Patch')" "renamed.dart " - -assert_files "non-Dart files in a patch are ignored" "$ANALYZE" \ - "$(codex_payload '*** Begin Patch -*** Update File: README.md -@@ --old -+new -*** End Patch')" "" - -assert_files "a patch touching nothing that exists" "$ANALYZE" \ - "$(codex_payload '*** Begin Patch -*** Update File: lib/nope.dart -@@ --old -+new -*** End Patch')" "" - -echo "" -echo "=== Payloads that must not be read as an edit ===" -# Otherwise the hooks would fire on ordinary shell calls. -assert_files "a Bash command naming a .dart file" "$ANALYZE" \ - "$(codex_payload 'cat lib/a.dart')" "" -assert_files "a shell command quoting patch markers" "$ANALYZE" \ - "$(codex_payload 'echo "*** Add File: lib/a.dart"')" "" -# A heredoc puts a patch marker at the start of its own line, so only the -# "*** Begin Patch" guard distinguishes this from a real edit. -assert_files "a heredoc whose body starts with a patch marker" "$ANALYZE" \ - "$(codex_payload 'cat < notes.txt -*** Add File: lib/a.dart -EOF')" "" -assert_files "a patch envelope that is not the first thing in the command" "$ANALYZE" \ - "$(codex_payload 'echo hi -*** Update File: lib/a.dart')" "" - -echo "" -echo "=== Exit codes ===" -assert_exit "analyze exits 0 when the analyzer passes" "$ANALYZE" \ - "$(claude_payload "$WORK/repo/lib/a.dart")" 0 -DART_STUB_FAIL=1 assert_exit "analyze exits 2 so the model sees the failure" "$ANALYZE" \ - "$(claude_payload "$WORK/repo/lib/a.dart")" 2 -assert_exit "analyze exits 0 with nothing to do" "$ANALYZE" '{}' 0 - -echo "" -echo "=== format.sh ===" -assert_files "format reads a Claude payload" "$FORMAT" \ - "$(claude_payload "$WORK/repo/lib/a.dart")" "a.dart " -assert_files "format reads a Codex patch" "$FORMAT" \ - "$(codex_payload '*** Begin Patch -*** Update File: lib/a.dart -@@ --old -+new -*** End Patch')" "a.dart " -assert_exit "format never blocks, even on failure" "$FORMAT" \ - "$(claude_payload "$WORK/repo/lib/a.dart")" 0 - -echo "" -echo "=== Results: $PASSED passed, $FAILED failed ===" - -if [ "$FAILED" -gt 0 ]; then - exit 1 -fi From 733c1f4cd255cd7bd2d46bb9c149ef28362663f2 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 9 Sep 2026 11:21:08 +0200 Subject: [PATCH 14/18] chore: revert unrelated script-tests glob change Naming the two suites explicitly is fine now that this PR adds none, and the change was unrelated to Codex. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yaml | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 6562e56..73c0e97 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -87,12 +87,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - - name: Hook script tests - run: | - status=0 - for test in hooks/scripts/*_test.sh; do - echo "::group::$test" - bash "$test" || status=1 - echo "::endgroup::" - done - exit $status + - name: Read-only git hook tests + run: bash hooks/scripts/allow-readonly-git_test.sh + - name: Block CLI workarounds hook tests + run: bash hooks/scripts/block-cli-workarounds_test.sh From 300deeab870895a1948d9c25105ea8c06af33666 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 9 Sep 2026 11:34:36 +0200 Subject: [PATCH 15/18] docs: drop stale MultiEdit reference MultiEdit was removed from Claude Code in v2.0.8 and is absent from the current tools documentation, so the matcher warning only needs to mention NotebookEdit. Co-Authored-By: Claude Opus 5 --- CONTRIBUTING.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index aa948b5..d5c533b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -220,10 +220,8 @@ copy of the hooks. Verified against Codex CLI 0.153.4: matcher containing only letters, digits, `_`, `-`, spaces, `,` and `|` as a list of exact tool names; anything else is an unanchored regex tested with `RegExp.test`. `apply_patch|Edit|Write` qualifies as exact, so it matches those three tool names and nothing else. Add a `.` or `*` and - it silently becomes a regex that also matches `MultiEdit` and `NotebookEdit`, whose payloads this - plugin does not read (`MultiEdit` nests `file_path` inside `edits[]` rather than at the top - level). Widen the matcher only together with the payload reading in `analyze.sh` and - `format.sh`. + it silently becomes a regex that also matches `NotebookEdit`, which this plugin has no reason to + act on. Widen the matcher only together with the payload reading in `analyze.sh` and `format.sh`. - **Hooks are a stable, default-on feature**, not experimental. The flag is `[features] hooks` (`codex features list` shows it enabled); there is no `codex_hooks` flag. Codex also runs hooks on Windows and offers a `commandWindows` override — but these scripts are `bash` and need `jq`, so From 399143510041962b726aae03e7f7cc5136bafa02 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 9 Sep 2026 13:14:30 +0200 Subject: [PATCH 16/18] docs: trim the Codex README section to installation Drops the Claude Code comparison list. The mechanics it described belong in CONTRIBUTING.md, which already covers them for contributors; the README section now just says how to install and how to add the reviewer agent. Co-Authored-By: Claude Opus 5 --- README.md | 26 +------------------------- config/cspell.json | 1 - 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/README.md b/README.md index f06dad7..4d490c4 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ This plugin includes SessionStart, PreToolUse, and PostToolUse hooks that valida | **Format** (`format.sh`) | PostToolUse (`apply_patch`/`Edit`/`Write`) | Runs `dart format` on the modified `.dart` file; always exits 0 (non-blocking — formatting is applied silently) | Codex runs this same `hooks/hooks.json` and these same scripts — `apply_patch` is its file-editing -tool, which is why that matcher covers it. See [Codex](#codex) for the differences. +tool, which is why that matcher covers it. ### Prerequisites @@ -116,30 +116,6 @@ mkdir -p .codex/agents && cp codex/agents/flutter-reviewer.toml .codex/agents/ Either way, ask Codex to spawn `flutter-reviewer`. -### How Codex differs from Claude Code - -- **The hooks are the same files.** `hooks/hooks.json` and every script under `hooks/scripts/` are - shared. Codex resolves `${CLAUDE_PLUGIN_ROOT}` as a compatibility alias for the installed plugin - directory, and it calls its file-editing tool `apply_patch` and hands the hook a raw patch rather - than a file path — so the `PostToolUse` matcher covers `apply_patch` and `analyze.sh` / - `format.sh` read both payload shapes. -- **Codex reuses the Claude Code plugin manifest.** It falls back to `.claude-plugin/plugin.json` - for the plugin's name and version, and finds `.mcp.json` on its own, so no second manifest is - needed. The trade is that Codex has no plugin-specific presentation metadata for this plugin — - icons, brand color, and starter prompts in the Codex app come from the fallback. -- **The reviewer agent is sandboxed instead of tool-restricted.** On Claude Code - `flutter-reviewer` has no write tools and an agent-scoped hook limits its Bash to - `git diff`/`git status`. Codex has no per-agent tool allowlist, so the agent declares - `sandbox_mode = "read-only"` — the OS refuses every write, which covers the same "never edits - files" guarantee. -- **Hooks are on by default.** They are a stable Codex feature; `codex features list` shows - `hooks` enabled. -- **Windows needs a POSIX shell.** Codex itself runs hooks on Windows, but every script here is - `bash` and needs `jq`, so run Codex under WSL or Git Bash. - -Codex truncates a skill `description` at 1024 characters and concatenates all of them into every -request, which is why descriptions in this repo are kept to triggers and scope. - ## Evals Skill evals ask whether Claude routes to a skill and follows it. [promptfoo](https://www.promptfoo.dev) sends each case's prompt through the Claude Agent SDK twice — once with this plugin loaded, once sealed with nothing loaded — so a grader that passes in both columns is measuring the model rather than the skill. They authenticate through your local Claude Code session, so they need no API key. diff --git a/config/cspell.json b/config/cspell.json index deb0a9a..876321b 100644 --- a/config/cspell.json +++ b/config/cspell.json @@ -52,7 +52,6 @@ "pubspecs", "redirections", "rubric", - "sandboxed", "serialization", "snackbars", "stdio", From 624a39faf5d847b27b20802c1c9fcbba3fe465d0 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 9 Sep 2026 13:17:16 +0200 Subject: [PATCH 17/18] feat: add the Codex plugin manifest Adds .codex-plugin/plugin.json so this repo declares itself as a Codex plugin rather than relying on the fallback to .claude-plugin/plugin.json. It is also the only place Codex app presentation metadata can live (displayName, category, capabilities, defaultPrompt) and it points mcpServers at ./.mcp.json. release-please now bumps the version in both manifests via extra-files, so they cannot drift. The marketplace entry pointing here is handled in very-good-claude-code-marketplace. Verified: installs into a throwaway CODEX_HOME with 15/15 skills, both MCP servers registered, and codex doctor reporting config.load and mcp.config ok. Co-Authored-By: Claude Opus 5 --- .codex-plugin/plugin.json | 28 ++++++++++++++++++++++++++++ .release-please-config.json | 5 +++++ AGENTS.md | 12 ++++++++---- CLAUDE.md | 5 ++--- CONTRIBUTING.md | 19 ++++++++----------- 5 files changed, 51 insertions(+), 18 deletions(-) create mode 100644 .codex-plugin/plugin.json diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json new file mode 100644 index 0000000..c705bfd --- /dev/null +++ b/.codex-plugin/plugin.json @@ -0,0 +1,28 @@ +{ + "name": "vgv-ai-flutter-plugin", + "version": "0.0.5", + "description": "Best-practice skills for Flutter and Dart development from Very Good Ventures.", + "author": { + "name": "Very Good Ventures", + "email": "hello@verygood.ventures", + "url": "https://verygood.ventures" + }, + "homepage": "https://github.com/VeryGoodOpenSource/vgv-ai-flutter-plugin", + "repository": "https://github.com/VeryGoodOpenSource/vgv-ai-flutter-plugin", + "license": "MIT", + "mcpServers": "./.mcp.json", + "interface": { + "displayName": "VGV AI Flutter Plugin", + "shortDescription": "Flutter and Dart best practices from Very Good Ventures", + "longDescription": "Best-practice skills for Flutter and Dart covering accessibility, animations, BLoC, testing, theming, navigation, security, internationalization, layered architecture, license compliance, UI packages, project creation, SDK/lint upgrades, and an autonomous quality-gate loop that drives analyze, format, test, and coverage to green — plus automated dart analyze and format hooks.", + "developerName": "Very Good Ventures", + "category": "Productivity", + "capabilities": ["Write"], + "defaultPrompt": [ + "Create a new Flutter app with Very Good CLI", + "Add a bloc for user authentication", + "Drive this package to green" + ], + "websiteURL": "https://verygood.ventures" + } +} diff --git a/.release-please-config.json b/.release-please-config.json index 9dbfd32..3dc37cd 100644 --- a/.release-please-config.json +++ b/.release-please-config.json @@ -22,6 +22,11 @@ "type": "json", "path": ".claude-plugin/plugin.json", "jsonpath": "$.version" + }, + { + "type": "json", + "path": ".codex-plugin/plugin.json", + "jsonpath": "$.version" } ] } diff --git a/AGENTS.md b/AGENTS.md index b7ec72f..d171801 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,7 +9,9 @@ VGV AI Flutter Plugin provides best-practices skills for Flutter and Dart develo ```text .mcp.json # MCP server configuration (Dart and Very Good CLI); read by both harnesses .claude-plugin/ - plugin.json # Plugin manifest (name, version, keywords); Codex falls back to this too + plugin.json # Claude Code plugin manifest (name, version, keywords) +.codex-plugin/ + plugin.json # Codex plugin manifest (interface metadata + mcpServers -> ./.mcp.json) agents/ flutter-reviewer.md # Read-only Flutter code reviewer subagent codex/ # The only Codex-specific assets; skills, MCP and hooks are shared @@ -176,9 +178,11 @@ documentation in the same change: in `README.md`, and check whether any skill's `allowed-tools` names a tool that was renamed or removed. Nothing validates those names. A new **server** goes in `.mcp.json` only; both harnesses read that file. -- **Editing `.claude-plugin/plugin.json`** — Codex falls back to this manifest for - the plugin's name and version, so it is no longer Claude-Code-only. Renaming the - plugin changes the skill namespace on both harnesses. +- **Editing either plugin manifest** — `.claude-plugin/plugin.json` and + `.codex-plugin/plugin.json` describe the same plugin. Keep + `interface.longDescription` in the Codex manifest in step with `description` in + the Claude Code one; release-please bumps `version` in both. Renaming the plugin + changes the skill namespace on both harnesses. - **Changing what a hook script reads from its payload** — the two harnesses describe an edit differently (Claude Code `tool_input.file_path`, Codex `tool_input.command` holding an apply_patch envelope). `analyze.sh` and diff --git a/CLAUDE.md b/CLAUDE.md index 5caeffa..50da965 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -43,9 +43,8 @@ All hook scripts require **jq** to parse the hook payload (they skip gracefully ### Codex -This repo installs as a Codex plugin with no Codex-specific config: the marketplace entry lives in -`very-good-claude-code-marketplace` alongside the Claude Code one, and Codex falls back to -`.claude-plugin/plugin.json` for the plugin's identity. It reads +This repo installs as a Codex plugin via `.codex-plugin/plugin.json`, with the marketplace entry +living in `very-good-claude-code-marketplace` alongside the Claude Code one. Codex reads `skills/`, `.mcp.json`, and this same `hooks/hooks.json` — resolving `${CLAUDE_PLUGIN_ROOT}` as a compatibility alias. That is why the `PostToolUse` matcher says `apply_patch|Edit|Write`: Codex names its file-editing tool `apply_patch`, and the extra alternative is inert on Claude Code. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d5c533b..f3c1018 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -199,17 +199,14 @@ Claude Code one. `.codex-plugin/plugin.json` plus the marketplace entry in `hooks/hooks.json` from the very files Claude Code uses. There is no install script and no second copy of the hooks. Verified against Codex CLI 0.153.4: -- **No Codex manifest, deliberately.** A `.codex-plugin/plugin.json` is optional and this repo - ships none. Codex falls back to `.claude-plugin/plugin.json` for the plugin's name and version - and discovers `.mcp.json` by itself, so an install without it is indistinguishable from one with - it: same skills, same namespacing, same two MCP servers, same `codex plugin list` output. The one - thing it would add is Codex app presentation metadata (`interface.displayName`, `category`, - `capabilities`, `defaultPrompt`, brand color, icons), which has no other home. That was judged - not worth a second manifest to keep in version-sync with the Claude Code one. Note the - consequence: `.claude-plugin/plugin.json` is now load-bearing for **both** harnesses, so its - `name` and `version` are not Claude-only fields any more. If you ever add - `.codex-plugin/plugin.json`, note that Codex rejects a manifest carrying a `hooks` key or any - field outside its allowed set. +- **Two manifests, one plugin.** `.codex-plugin/plugin.json` is the Codex twin of + `.claude-plugin/plugin.json`. It is what carries the Codex app presentation metadata + (`interface.displayName`, `category`, `capabilities`, `defaultPrompt`), which has no other home, + and it points `mcpServers` at `./.mcp.json`. Keep `interface.longDescription` in step with + `description` in the Claude Code manifest, and leave `version` to release-please, which bumps + both through `extra-files`. `keywords` is deliberately not duplicated: it only affects plugin + search, and a second copy of a 50-plus entry list would rot. Codex rejects a manifest carrying a + `hooks` key or any field outside its allowed set, so do not add one. - **Hooks come from default discovery.** Codex looks for a plugin's hooks at `/hooks/hooks.json` — the same path and file Claude Code uses — and resolves `${CLAUDE_PLUGIN_ROOT}` inside it, documented as a compatibility alias alongside its own From 32abedc627e1726715f3a909e94e833d302599df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20=C5=A0imon=C3=ADk?= <32575328+ryzizub@users.noreply.github.com> Date: Wed, 9 Sep 2026 14:05:32 +0200 Subject: [PATCH 18/18] Update .codex-plugin/plugin.json Co-authored-by: Marcos Sevilla <31174242+marcossevilla@users.noreply.github.com> --- .codex-plugin/plugin.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index c705bfd..6bb4fee 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -4,7 +4,7 @@ "description": "Best-practice skills for Flutter and Dart development from Very Good Ventures.", "author": { "name": "Very Good Ventures", - "email": "hello@verygood.ventures", + "email": "tools@verygood.ventures", "url": "https://verygood.ventures" }, "homepage": "https://github.com/VeryGoodOpenSource/vgv-ai-flutter-plugin",