feat: /codex-plan-review and spec-gap-review skill (v0.12.0)#39
Open
stevegoldenberg wants to merge 7 commits into
Open
feat: /codex-plan-review and spec-gap-review skill (v0.12.0)#39stevegoldenberg wants to merge 7 commits into
stevegoldenberg wants to merge 7 commits into
Conversation
Roll up the in-flight 0.12.0 work into a versioned release: bump version, extend the CHANGELOG, propagate command/skill counts through manifests and docs, document /codex-plan-review across user-facing entry points, and harden the Codex installer. Major changes on this branch: - /codex-plan-review slash command: the iterative Codex-driven plan review loop landed in ee71ed4 is now wired through CLAUDE.md, INSTALL.md, USAGE.md, and the setup templates so users discover it from every documented entry point. INSTALL.md gains a note explaining the two prerequisites (Codex CLI authenticated, Codex skills symlinked via npx metaswarm init --codex). - spec-gap-review skill: Codex-runnable audit of implementation guides, design docs, UX specs, infrastructure plans, and roadmaps against the actual repository (skill body landed in ee71ed4). USAGE.md now documents its 6-dimension rubric, stable gap IDs, file:line citation validation, and how /codex-plan-review drives the iterative loop. - Installer hardening (.codex/install.sh + cli/metaswarm.js): * Legacy sweep — removes dangling metaswarm-* symlinks left in ~/.agents/skills/ by 0.10/0.11 so upgraders don't accumulate stale links. Safe no-op on machines that never had the legacy directory. * Real-directory conflict messaging — when a skill target exists as a real directory instead of a managed symlink, both installers now print a clear remediation hint (rm -rf and re-run) instead of a bare warning. - lib/sync-resources.js: explanatory comment on why codex-plan-review is intentionally NOT mapped into Gemini's TOML command set — the command shells out to the Codex CLI, which Gemini users wouldn't have locally. - Version bump 0.11.0 → 0.12.0 in package.json, plugin.json, and gemini-extension.json. CHANGELOG.md gets a 0.12.0 entry covering the added command, added skill, and changed installer behavior. - Skill/command counts updated across all manifests and docs: 13 → 14 skills, 15 → 16 commands (README.md, GEMINI.md, plugin.json, gemini-extension.json, root CLAUDE.md, AGENTS.md, CLAUDE-append.md, and the equivalents under skills/setup/templates/). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughBumps release to 0.12.0, migrates Codex skills install path to ${CODEX_HOME:-$HOME/.codex}/skills, adds the ChangesInstallation / Codex home migration
Spec-gap-review skill (new)
codex-plan-review command (new)
Manifests, counts, and release notes
Sequence Diagram(s)sequenceDiagram
participant User as CLI/User
participant Metaswarm as metaswarm CLI (commands/codex-plan-review)
participant Codex as Codex CLI
participant Skill as spec-gap-review (Codex skill)
participant FS as Repository (files)
User->>Metaswarm: run /codex-plan-review (flags, profile)
Metaswarm->>Codex: invoke codex exec (baseline/round prompt) < /dev/null
Codex->>Skill: execute $spec-gap-review prompt
Skill->>FS: read plan & repo files (validate `#L` refs)
FS-->>Skill: file contents / validation results
Skill-->>Codex: review output (scores, gap tracker, patches)
Codex-->>Metaswarm: skill output
Metaswarm->>User: ACCEPT/STALLED/MAX_ROUNDS + patch suggestions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 53 minutes and 5 seconds.Comment |
Greptile SummaryThis PR adds the
Confidence Score: 4/5Safe to merge with the symlink ordering fix applied; the P1 affects an edge case (legacy real-directory conflict during a re-install) but silently removes a working skill symlink with no replacement. One P1 bug present (symlink deleted before legacy-conflict guard in both install scripts), which sets a 4/5 ceiling per severity guidelines. All previously flagged issues are resolved; the rest of the PR is well-structured. .codex/install.sh and cli/metaswarm.js — the symlink ordering bug appears in both files and should be fixed together.
|
| Filename | Overview |
|---|---|
| .codex/install.sh | Migrates SKILLS_DIR from ~/.agents/skills to $CODEX_ROOT/skills and adds legacy sweep + real-dir conflict hints; contains a P1 ordering bug where the new target symlink can be deleted before a legacy-path conflict triggers continue, leaving the skill un-linked. |
| cli/metaswarm.js | Mirrors the install.sh path change (codexHome variable, skills dir) and adds legacy-symlink sweep; has the same P1 ordering bug as install.sh where linkPath is unlinked before skipForLegacyDir is evaluated. |
| commands/codex-plan-review.md | New 451-line command spec for the iterative Codex plan-review loop; CODEX_LOG is now defined (mktemp) per prior review feedback, hang_killed flag is correctly initialized and set before break — previously reported issues addressed. |
| skills/spec-gap-review/SKILL.md | New Codex skill definition with stable 6-dimension rubric, gap IDs, score caps, multi-round progression rules, and Critical-Only Mode; well-structured and internally consistent. |
| skills/spec-gap-review/scripts/validate_line_links.py | New validator script; encoding, file-handle, and relative-path issues from prior review are all fixed — explicit UTF-8, with-block for line count, URL-scheme guard, and relative-path resolution via md_path.parent. |
| tests/codex/test-codex-skills.sh | New test assertions for the updated install path (CODEX_ROOT/skills), spec-gap-review presence in README, and codex-plan-review preflight path; all checks align with actual file content. |
| tests/cli/test-installer.sh | Adds test 5b verifying the JS installer uses codexHome/skills; grep strings match the current metaswarm.js implementation exactly. |
| lib/sync-resources.js | Adds explanatory comment explaining why codex-plan-review is intentionally excluded from Gemini TOML mapping; no functional change. |
Comments Outside Diff (1)
-
.codex/install.sh, line 71-90 (link)Symlink deleted before legacy-conflict guard can prevent it
When
$targetis an existing symlink (i.e., this is a re-run/update on a 0.12 installation), it is removed on line 73 unconditionally. The legacy-conflict guard that follows on lines 80–87 can then triggercontinuewithout creating the replacement, leaving the skill slot empty. A user runninginstall.shtwice where$SKILLS_DIR/metaswarm-<skill>happens to be a real directory will silently lose the working$targetsymlink with no replacement and no clear hint that the removal happened.The same structural issue is reproduced in
cli/metaswarm.js(fs.unlinkSync(linkPath)in the first try-block fires before theskipForLegacyDirguard is evaluated). Fix: check both$targetand$legacy_targetfor conflicts before removing anything, or swap the block order so the legacy-conflict check runs first andcontinues before the target removal.Prompt To Fix With AI
This is a comment left during a code review. Path: .codex/install.sh Line: 71-90 Comment: **Symlink deleted before legacy-conflict guard can prevent it** When `$target` is an existing symlink (i.e., this is a re-run/update on a 0.12 installation), it is removed on line 73 unconditionally. The legacy-conflict guard that follows on lines 80–87 can then trigger `continue` without creating the replacement, leaving the skill slot empty. A user running `install.sh` twice where `$SKILLS_DIR/metaswarm-<skill>` happens to be a real directory will silently lose the working `$target` symlink with no replacement and no clear hint that the removal happened. The same structural issue is reproduced in `cli/metaswarm.js` (`fs.unlinkSync(linkPath)` in the first try-block fires before the `skipForLegacyDir` guard is evaluated). Fix: check both `$target` and `$legacy_target` for conflicts **before** removing anything, or swap the block order so the legacy-conflict check runs first and `continue`s before the target removal. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
.codex/install.sh:71-90
**Symlink deleted before legacy-conflict guard can prevent it**
When `$target` is an existing symlink (i.e., this is a re-run/update on a 0.12 installation), it is removed on line 73 unconditionally. The legacy-conflict guard that follows on lines 80–87 can then trigger `continue` without creating the replacement, leaving the skill slot empty. A user running `install.sh` twice where `$SKILLS_DIR/metaswarm-<skill>` happens to be a real directory will silently lose the working `$target` symlink with no replacement and no clear hint that the removal happened.
The same structural issue is reproduced in `cli/metaswarm.js` (`fs.unlinkSync(linkPath)` in the first try-block fires before the `skipForLegacyDir` guard is evaluated). Fix: check both `$target` and `$legacy_target` for conflicts **before** removing anything, or swap the block order so the legacy-conflict check runs first and `continue`s before the target removal.
Reviews (5): Last reviewed commit: "fix: track hang-watcher kill via flag so..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (1)
91-93:⚠️ Potential issue | 🟡 MinorStale Claude command count in repository structure block.
The PR adds
/codex-plan-reviewas a Claude-only command (perlib/sync-resources.jscomment and PR objectives noting 15→16 commands), but this block still reports "Claude Code commands (15 files)". Consider bumping to16 filesfor consistency with the skill count update on line 3 and the rest of the PR’s docs.📝 Proposed fix
-│ ├── *.md # Claude Code commands (15 files) +│ ├── *.md # Claude Code commands (16 files) │ └── metaswarm/*.toml # Gemini CLI commands (12 files)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 91 - 93, The README's repository tree still shows "Claude Code commands (15 files)" while the PR adds the /codex-plan-review Claude-only command (see comment in lib/sync-resources.js and PR notes increasing Claude commands to 16); update the string "Claude Code commands (15 files)" to "Claude Code commands (16 files)" in the commands block so the count matches the added command..codex/README.md (1)
82-89:⚠️ Potential issue | 🟡 MinorUninstall instructions don't honor
CODEX_HOME.The manual install (Lines 16–23) explains how to substitute
$CODEX_HOMEfor~/.codex, but the uninstall block hardcodes~/.codex/metaswarmand~/.codex/skills. Users who installed withCODEX_HOMEset will be surprised when this no-ops or removes the wrong tree.📝 Suggested wording
## Uninstall ```bash +# If CODEX_HOME is set, replace ~/.codex below with "$CODEX_HOME". # Remove skill symlinks for d in ~/.codex/metaswarm/skills/*/; do rm -f ~/.codex/skills/$(basename "$d"); done # Remove installation rm -rf ~/.codex/metaswarm</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In @.codex/README.md around lines 82 - 89, Update the "Uninstall" section to
respect CODEX_HOME by replacing hardcoded ~/.codex paths with a variable that
defaults to ~/.codex when CODEX_HOME is unset; specifically, modify the
uninstall commands in the Uninstall block so they use "$CODEX_HOME" (or
"${CODEX_HOME:-$HOME/.codex}") for both the metaswarm and skills paths and add a
short note explaining the substitution so users who set CODEX_HOME won't be
surprised.</details> </blockquote></details> </blockquote></details>🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed. Inline comments: In @.codex/install.sh: - Around line 75-79: The current install logic handles a non-symlink legacy_target by only warning and then proceeding to create the new target, which can cause duplicate skill entries; change the branch that detects an existing non-symlink legacy_target (variable legacy_target) to mirror the linkPath real-directory branch: after echoing the warning, skip creation of target (i.e., continue/return) rather than proceeding, or else expand the warning message to include a clear manual-removal hint and instructions for safe cleanup; apply the same fix to the analogous handling near linkPath in the CLI script (the block that checks for an existing real directory and currently only warns) so both paths uniformly avoid creating a duplicate symlink when a real metaswarm-<skill> directory exists. - Around line 12-14: SKILLS_DIR duplicates the expansion used to set CODEX_ROOT; update the SKILLS_DIR assignment to reuse CODEX_ROOT (e.g., set SKILLS_DIR to "$CODEX_ROOT/skills") so the script is DRY and no longer re-expands ${CODEX_HOME:-$HOME/.codex}; modify the SKILLS_DIR variable only and keep CODEX_ROOT and INSTALL_DIR as-is. - Around line 38-52: Enable bash nullglob before iterating to prevent the unmatched glob from expanding to a literal string: add shopt -s nullglob (and optionally restore the previous state with shopt -u nullglob after the loop) before the for loop that uses LEGACY_AGENTS_DIR/metaswarm-*; keep the existing guards using [ -L "$legacy_link" ] and the legacy_removed counter logic in the loop so behavior remains idempotent while avoiding the spurious iteration over the literal pattern. In `@cli/metaswarm.js`: - Around line 57-59: Replace the duplicated expression with the new codexHome helper by setting skillsDir to use the codexHome constant (i.e., const skillsDir = path.join(codexHome, 'skills')) instead of re-deriving process.env.CODEX_HOME; then update tests/cli/test-installer.sh to assert behavior (set CODEX_HOME=$TMP_DIR and check that symlinks or files are created under $TMP_DIR/skills or grep for the joined path) rather than relying on a brittle literal substring match. In `@commands/codex-plan-review.md`: - Line 3: Update skill invocations to use the required `$` prefix: change occurrences of spec-gap-review and plan-review-gate (and any other skill names in this file) to `$spec-gap-review` and `$plan-review-gate`, and ensure this `$name` syntax is used consistently throughout the document (including the installer-transition note that references metaswarm-prefixed paths and the CODEX_HOME skill path) to comply with SKILL.md and the `**/*.{js,ts,py,md}` guideline. - Around line 49-50: The doc currently uses two different options for handling a stale sibling `<plan-stem>-codexreview.md`: one occurrence says “overwrite or append” and another says “overwrite or continue”; make the behavior wording consistent by choosing one phrasing (recommend “overwrite or append”), and update both occurrences of the sibling-path handling text in commands/codex-plan-review.md so the prompt and the preflight warning use the exact same phrase (“overwrite or append”) whenever describing the choice for handling an existing sibling file. - Line 250: Remove the unsupported assumption that "## Detailed findings" always includes GID prefixes: instead of falling back to "## Detailed findings" when "## Prioritized issues" bullets lack GIDs, update the logic to (a) require GID prefixes in the contract/prompt for "## Detailed findings" explicitly, or (b) detect missing GIDs in both "## Prioritized issues" and "## Detailed findings" and treat the response as undeterministic by routing it to the file-parse flow immediately; reference the section headings "## Prioritized issues" and "## Detailed findings" when implementing the detection and fallback change. - Around line 74-80: The markdown has several triple-backtick fenced code blocks missing a language tag (MD040); update each affected fence in commands/codex-plan-review.md (examples around the profile/max_rounds block and the ranges noted: 84-86, 98-103, 109-115, 119-125, 302-312, 323-333, 342-344, 365-373, 379-383, 413-416) to include an explicit language identifier—use "text" for plain text blocks and "bash" for shell examples—so every ``` becomes either ```text or ```bash as appropriate to satisfy markdownlint. In `@skills/spec-gap-review/scripts/validate_line_links.py`: - Around line 16-26: The validate_file loop currently reads md_path with the locale default encoding and re-opens target for every LINK_RE match without deterministic closing, risking resource leaks, quadratic IO, and failures on binary files; fix validate_file by reading md_path with an explicit encoding (e.g., 'utf-8') and introduce a small in-memory cache (dict keyed by target.resolve() or str(target)) that stores the result for each target (e.g., total line count or an error marker) so you open each target only once using a with target.open('r', encoding='utf-8') as f: block; on UnicodeDecodeError or other IO errors record a per-link error in errors and continue rather than letting the exception abort the run, and reference LINK_RE, md_path, target, and errors when applying these changes. - Around line 14-29: The validate_file function currently treats link targets as absolute OS paths; change its signature to accept a repo_root argument (default Path.cwd()) and resolve targets as (repo_root / match.group(1).lstrip("/")).resolve() using LINK_RE to locate matches; call md_path.read_text(encoding="utf-8") and when counting lines use a with target.open("r", encoding="utf-8") as f pattern, caching the computed line count per resolved target path to avoid reopening the same file, and wrap the file-existence/checking and read operations in try/except (catching OSError, UnicodeDecodeError) to treat non-text or unreadable files gracefully and append an appropriate error instead of raising. In `@skills/spec-gap-review/SKILL.md`: - Line 71: The ordered list item currently uses "11." which triggers MD029; update that list item prefix from "11." to "1." and ensure the entire ordered list uses the repository's single-numbering style (e.g., all items prefixed with "1.") so the list in SKILL.md (the item "Validate every file:line reference if you save a review file.") conforms to markdownlint rules. In `@tests/cli/test-installer.sh`: - Around line 55-60: The test's fixed-string grep couples it to a specific expression; update the assertion in tests/cli/test-installer.sh so it uses a pattern-based check against metaswarm.js that confirms the installer references either CODEX_HOME or codexHome and the 'skills' path together (e.g., with grep -E and a regex like "(CODEX_HOME|codexHome).*(skills)|(skills).*(CODEX_HOME|codexHome)"), or alternatively replace this unit check with a behavioral test that runs installCodex (stubbing git and setting CODEX_HOME) and asserts the created symlink points to $CODEX_HOME/skills; reference the metaswarm.js symbol (installCodex / the CODEX_HOME usage) when making the change. --- Outside diff comments: In @.codex/README.md: - Around line 82-89: Update the "Uninstall" section to respect CODEX_HOME by replacing hardcoded ~/.codex paths with a variable that defaults to ~/.codex when CODEX_HOME is unset; specifically, modify the uninstall commands in the Uninstall block so they use "$CODEX_HOME" (or "${CODEX_HOME:-$HOME/.codex}") for both the metaswarm and skills paths and add a short note explaining the substitution so users who set CODEX_HOME won't be surprised. In `@README.md`: - Around line 91-93: The README's repository tree still shows "Claude Code commands (15 files)" while the PR adds the /codex-plan-review Claude-only command (see comment in lib/sync-resources.js and PR notes increasing Claude commands to 16); update the string "Claude Code commands (15 files)" to "Claude Code commands (16 files)" in the commands block so the count matches the added command.🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID:
14be07ef-b90f-49c0-83fd-ddcbc6ac0e7d📒 Files selected for processing (27)
.claude-plugin/plugin.json.codex/README.md.codex/install.shCHANGELOG.mdCLAUDE.mdGEMINI.mdINSTALL.mdREADME.mdUSAGE.mdcli/metaswarm.jscommands/codex-plan-review.mdgemini-extension.jsonlib/sync-resources.jspackage.jsonskills/setup/templates/AGENTS.mdskills/setup/templates/CLAUDE-append.mdskills/setup/templates/CLAUDE.mdskills/spec-gap-review/SKILL.mdskills/spec-gap-review/agents/openai.yamlskills/spec-gap-review/references/rubric-patterns.mdskills/spec-gap-review/scripts/validate_line_links.pyskills/start/references/platform-adaptation.mdtemplates/AGENTS.mdtemplates/CLAUDE-append.mdtemplates/CLAUDE.mdtests/cli/test-installer.shtests/codex/test-codex-skills.sh
| - what the current repo does | ||
| - what the document claims | ||
| - what is merely future intent | ||
| 11. Validate every file:line reference if you save a review file. |
There was a problem hiding this comment.
Fix ordered-list prefix style to satisfy markdownlint.
Line 71 uses 11. and triggers MD029 with the repository’s configured ordered-list style. Use 1. for that item (and keep the list style consistent) to avoid lint noise.
Proposed fix
-11. Validate every file:line reference if you save a review file.
+1. Validate every file:line reference if you save a review file.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 11. Validate every file:line reference if you save a review file. | |
| 1. Validate every file:line reference if you save a review file. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 71-71: Ordered list item prefix
Expected: 1; Actual: 11; Style: 1/1/1
(MD029, ol-prefix)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/spec-gap-review/SKILL.md` at line 71, The ordered list item currently
uses "11." which triggers MD029; update that list item prefix from "11." to "1."
and ensure the entire ordered list uses the repository's single-numbering style
(e.g., all items prefixed with "1.") so the list in SKILL.md (the item "Validate
every file:line reference if you save a review file.") conforms to markdownlint
rules.
Collaborator
Author
There was a problem hiding this comment.
Not changing this one. The list (lines 25-71) uses sequential numbering 1, 2, ..., 11 — that's valid MD029 with style ordered, and matches the sequential numbering used by the other ordered list in the same file at line 162-163. Converting only this list to all-1. would create inconsistency with the rest of the repo. The repo doesn't enforce a markdownlint config, so this is a CodeRabbit defaults-vs-repo-convention difference rather than a real lint failure.
There was a problem hiding this comment.
@stevegoldenberg, that's a fair point — sequential numbering (1, 2, …, 11) is fully valid MD029 with the ordered style, and converting only the last item to 1. would indeed create inconsistency within the file. Since there's no enforced markdownlint config, this was a defaults-vs-convention mismatch on my end. I'll note this for future reviews.
(=ↀωↀ=)
✏️ Learnings added
Learnt from: stevegoldenberg
Repo: dsifry/metaswarm PR: 39
File: skills/spec-gap-review/SKILL.md:71-71
Timestamp: 2026-04-26T18:49:10.222Z
Learning: In the dsifry/metaswarm repository, ordered lists use sequential numbering (1, 2, 3, … N) — MD029 `ordered` style — not the `1/1/1` all-`1.` style. There is no enforced markdownlint config in the repo, so MD029 warnings from CodeRabbit's default configuration (expecting `1/1/1`) are false positives and should not be flagged.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: dsifry/metaswarm PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T04:53:04.601Z
Learning: After brainstorming completes with a design document, run the Design Review Gate via `/review-design` before proceeding to planning/implementation
Learnt from: CR
Repo: dsifry/metaswarm PR: 0
File: templates/GEMINI.md:0-0
Timestamp: 2026-03-02T04:54:01.783Z
Learning: Invoke `/metaswarm:review-design` after brainstorming completes and commits a design document before proceeding to planning or implementation
Learnt from: CR
Repo: dsifry/metaswarm PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T04:53:04.601Z
Learning: After any implementation plan is created, run the Plan Review Gate via `plan-review-gate` skill and wait for all 3 adversarial reviewers (Feasibility, Completeness, Scope & Alignment) to PASS before proceeding
Learnt from: CR
Repo: dsifry/metaswarm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T04:54:14.494Z
Learning: After brainstorming, MUST run `$design-review-gate` before planning; after any plan, MUST run `$plan-review-gate` before presenting to user
Learnt from: CR
Repo: dsifry/metaswarm PR: 0
File: GEMINI.md:0-0
Timestamp: 2026-03-02T04:53:43.691Z
Learning: Run design review gate (`/metaswarm:review-design`) after brainstorming and before planning
Learnt from: CR
Repo: dsifry/metaswarm PR: 0
File: skills/setup/templates/GEMINI.md:0-0
Timestamp: 2026-03-02T04:53:52.184Z
Learning: Run the Design Review Gate (`/metaswarm:review-design`) after brainstorming completes before proceeding to planning/implementation
Learnt from: dsifry
Repo: dsifry/metaswarm PR: 21
File: commands/start-task.md:42-45
Timestamp: 2026-02-20T07:52:07.460Z
Learning: In Markdown files, fenced code blocks nested inside list items with indentation that relies on list continuation can trigger MD031 (blanks-around-fences) as false positives. If the rendered output is correct across common Markdown parsers, these warnings can be safely ignored. During reviews, ensure the fence indentation correctly aligns with the surrounding list structure and that spacing follows the project's Markdown flavor rules; consider adding a brief note in the file if you expect this to be a parser-specific quirk.
Collaborator
Author
There was a problem hiding this comment.
@CodeRabbit changes approved: The SKILL.md workflow section was rewritten in commit ba93941 (depth gates / claim-contract matrix additions) — the original line 71 with its bare 11. no longer exists. Both ordered lists in the new SKILL.md (Workflow steps 1–14 and Output Shape steps 1–11) are sequentially numbered with a consistent style throughout, so MD029 should not fire.
There was a problem hiding this comment.
@stevegoldenberg, thanks for the update! Good to know the workflow section was fully rewritten in commit ba93941 — with both ordered lists now using consistent sequential numbering throughout, the MD029 concern is fully resolved. ✅
🧠 Learnings used
Learnt from: dsifry
Repo: dsifry/metaswarm PR: 21
File: commands/start-task.md:42-45
Timestamp: 2026-02-20T07:52:07.460Z
Learning: In Markdown files, fenced code blocks nested inside list items with indentation that relies on list continuation can trigger MD031 (blanks-around-fences) as false positives. If the rendered output is correct across common Markdown parsers, these warnings can be safely ignored. During reviews, ensure the fence indentation correctly aligns with the surrounding list structure and that spacing follows the project's Markdown flavor rules; consider adding a brief note in the file if you expect this to be a parser-specific quirk.
Learnt from: stevegoldenberg
Repo: dsifry/metaswarm PR: 39
File: skills/spec-gap-review/SKILL.md:71-71
Timestamp: 2026-04-26T18:49:10.222Z
Learning: In dsifry/metaswarm Markdown files, ordered lists should use sequential numbering (e.g., `1.`, `2.`, `3.`, … up to `N`) rather than the `1/1/1` all-`1.` style. If CodeRabbit/markdownlint reports MD029 `ordered` style warnings expecting `1/1/1`, treat them as false positives for this repo’s convention and do not flag them when lists are sequentially numbered.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
Four targeted fixes from Greptile's review: - skills/spec-gap-review/scripts/validate_line_links.py:16 Pass encoding="utf-8" to read_text() so non-ASCII markdown decodes correctly on Windows (CP1252) and other non-UTF-8 default platforms. - skills/spec-gap-review/scripts/validate_line_links.py:26 Wrap the line-count file open in a `with` block. The previous generator-expression form leaked the file descriptor on PyPy and other runtimes that defer finalization, risking fd-exhaustion on review files with many citations. Also pass encoding="utf-8" here for symmetry with the surrounding read. - cli/metaswarm.js:59 Use the existing `codexHome` variable when building `skillsDir` instead of re-evaluating `process.env.CODEX_HOME || path.join(...)`. Eliminates drift risk if the fallback path ever changes. tests/cli/test-installer.sh updated to grep the new equivalent form. - commands/codex-plan-review.md (Step 2 invocation) Define `CODEX_LOG=$(mktemp)` and redirect codex stdout/stderr to it so the hang-watcher's `wc -c < "$CODEX_LOG"` signal is real instead of always 0. Without this, the watcher loses one of its four activity signals and can false-positive `codex-hang` on reasoning-heavy rounds that produce no tool calls and no sibling-file writes yet. All tests pass: 13/13 installer, 44/44 codex skills. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Eight fixes from CodeRabbit's CHANGES_REQUESTED review on PR #39. Trivial / mechanical: - README.md:92 — bump 'Claude Code commands (15 files)' to 16 to match the v0.12.0 doc rollout (15 → 16 commands). - .codex/install.sh:14 — reuse the existing $CODEX_ROOT variable for SKILLS_DIR instead of re-expanding ${CODEX_HOME:-$HOME/.codex}. Mirrors the cli/metaswarm.js codexHome cleanup from f16eb99. - .codex/install.sh legacy sweep — wrap the `for legacy_link in ... metaswarm-*` loop in `shopt -s nullglob` / `shopt -u nullglob` so an unmatched glob expands to nothing instead of iterating once over the literal pattern. The existing `[ -L "$legacy_link" ] || continue` guard already made this safe; the change is for cleanliness. - .codex/README.md uninstall block — add a leading comment instructing users to substitute "$CODEX_HOME" (or "${CODEX_HOME:-$HOME/.codex}") for the hardcoded ~/.codex paths if they installed with CODEX_HOME set. - commands/codex-plan-review.md — add language identifiers to nine fenced code blocks that were missing them (MD040). Prompt blocks and terminal report templates use `text`; the recommended-option example uses `markdown`. Real behavioral fixes: - .codex/install.sh:75-79 + cli/metaswarm.js:128-136 — when the legacy metaswarm-<skill> path exists as a real directory (not a symlink the installer can replace), the installer used to warn but then proceed to create the new un-prefixed symlink alongside it. That left two directories that both expose a SKILL.md with the same `name:` field, which Codex would resolve unpredictably. Both installers now warn and `continue` (skip that skill), telling the user to remove the legacy directory manually before re-running. - commands/codex-plan-review.md:49 — sibling-file overwrite phrasing was inconsistent ('overwrite or append' vs. line 442's 'overwrite or continue (skill picks up from the last saved round). Default: continue.'). Standardize on 'overwrite or continue' so the preflight description matches the canonical behavior section below. Test updates: tests/codex/test-codex-skills.sh:39-43 was string-grepping for the old SKILLS_DIR expression; updated to grep for both lines of the new equivalent form (CODEX_ROOT definition + SKILLS_DIR=$CODEX_ROOT/skills). Not addressed (intentional): - skills/spec-gap-review/SKILL.md:71 ordered-list MD029 — current list uses sequential numbering (1, 2, ..., 11), which is valid MD029 'ordered' style and matches the sequential-numbering style used by other ordered lists in the same file (e.g., line 162). Will respond on the thread. - The four design-level CodeRabbit findings (skill-invocation $-prefix consistency, '## Detailed findings' GID assumption, validate_line_links caching/repo_root rewrite, behavioral installer test) are deferred for follow-up. All tests pass: 13/13 installer, 44/44 codex skills. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…hrough mode Pull in updated SKILL.md and rubric-patterns.md from local ~/.codex/skills/ copy where the spec-gap-review skill was iterated against real reviews. - SKILL.md: introduce Depth Gates section that defines when to escalate to breakthrough mode, add claim/contract matrix step, score caps step, and adversarial final pass step in the workflow; add Minimum Repo Exploration guidance per doc type - rubric-patterns.md: add Breakthrough Mode and Claim/Contract Matrix patterns for stuck/late-round reviews scripts/validate_line_links.py is intentionally not included — the repo already has newer changes there from prior PR review feedback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
commands/codex-plan-review.md: - Use $skill-name invocation syntax consistently for $spec-gap-review and $plan-review-gate (per AGENTS.md convention) - Tighten the Step 3 GID-fallback contract: don't assume Detailed findings always carries GIDs; if no deterministic mapping exists in either section, classify as file-parse instead of guessing skills/spec-gap-review/scripts/validate_line_links.py: - Broaden LINK_RE to match repo-relative paths (Codex review files use these by default), resolved against each markdown file's parent - Skip URL schemes (http, https, mailto, etc.) so they aren't treated as file paths to validate - Cache per-target line counts to avoid quadratic re-opens when one file is cited multiple times in the same review - Catch OSError and UnicodeDecodeError per-link so binary or unreadable cited files produce a per-link error rather than aborting the entire run Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Greptile P1 (PR #39): the hang-watcher's SIGTERM exit code (typically 143) was indistinguishable from any other non-zero exit, so the documented "classify as codex-hang" branch was unreachable — implementations would fall through to the generic retry path and never surface the codex-hang diagnostic with the < /dev/null remediation hint. Set hang_killed=true before break, then check it first in the exit-code ladder before consulting \$codex_exit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.codex/install.sh:
- Around line 74-77: The install script currently only skips real directories
but will overwrite existing regular files when it later runs ln -sf; update the
guard around $target so any existing non-symlink path is skipped. Replace the
current directory-only check (the branch that references $target and the echo
lines) with a check like: if [ -e "$target" ] && [ ! -L "$target" ]; then echo
the same warning messages (including the rm -rf suggestion) and continue;
otherwise proceed to create the symlink with ln -sf as before. This ensures
$target, ln -sf, and the echo warnings handle both regular files and directories
(non-symlinks) consistently with cli/metaswarm.js.
In @.codex/README.md:
- Around line 23-24: Update the "Updating" instruction to respect CODEX_HOME by
replacing the hardcoded "~/.codex" path with the environment-variable-aware
expansion "${CODEX_HOME:-$HOME/.codex}" so users who set CODEX_HOME will have
the correct path; locate the Updating command in the README.md and change the cd
command that currently uses "~/.codex/metaswarm" to use the substitution for
CODEX_HOME.
In `@skills/spec-gap-review/SKILL.md`:
- Around line 292-294: Update the validator invocation in SKILL.md so it works
when run from a project repo by using the absolute path to the script instead of
a relative path; replace the relative call to "python3
scripts/validate_line_links.py" with a call that expands CODEX_HOME (e.g.
python3
"${CODEX_HOME:-$HOME/.codex}/skills/spec-gap-review/scripts/validate_line_links.py")
so the validate_line_links.py script is located correctly regardless of cwd, and
ensure the README command example around validate_line_links.py is updated
accordingly.
In `@tests/codex/test-codex-skills.sh`:
- Around line 101-106: The README path check in tests/codex/test-codex-skills.sh
currently only greps for the literal "~/.codex/skills" and will miss
documentation that uses the variable form "${CODEX_HOME:-$HOME/.codex}/skills";
update the conditional that uses grep (the block that sets pass/fail) to broaden
the pattern to accept either the literal "~/.codex/skills" OR the
variable-expanded form "${CODEX_HOME:-$HOME/.codex}/skills" (or a concise regex
that matches both forms) so the test passes when the README documents the
CODEX_HOME-based path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 503c5018-91a3-4f6c-861f-6e535b742ce1
📒 Files selected for processing (10)
.codex/README.md.codex/install.shREADME.mdcli/metaswarm.jscommands/codex-plan-review.mdskills/spec-gap-review/SKILL.mdskills/spec-gap-review/references/rubric-patterns.mdskills/spec-gap-review/scripts/validate_line_links.pytests/cli/test-installer.shtests/codex/test-codex-skills.sh
Comment on lines
74
to
+77
| elif [ -d "$target" ]; then | ||
| echo " Warning: $target exists as a directory, skipping" | ||
| echo " Warning: $target exists as a real directory (not a symlink); skipping." | ||
| echo " Remove it manually (rm -rf \"$target\") and re-run if you want the managed copy." | ||
| continue |
There was a problem hiding this comment.
Prevent clobbering non-symlink files at the managed skill path.
At Line 74, the guard only skips directories. If $target is a regular file, the code falls through to ln -sf and overwrites it. This also diverges from cli/metaswarm.js, which skips any existing non-symlink path.
Suggested patch
- elif [ -d "$target" ]; then
- echo " Warning: $target exists as a real directory (not a symlink); skipping."
- echo " Remove it manually (rm -rf \"$target\") and re-run if you want the managed copy."
+ elif [ -e "$target" ]; then
+ echo " Warning: $target exists as a real path (not a symlink); skipping."
+ echo " Remove it manually (rm -rf \"$target\") and re-run if you want the managed copy."
continue
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| elif [ -d "$target" ]; then | |
| echo " Warning: $target exists as a directory, skipping" | |
| echo " Warning: $target exists as a real directory (not a symlink); skipping." | |
| echo " Remove it manually (rm -rf \"$target\") and re-run if you want the managed copy." | |
| continue | |
| elif [ -e "$target" ]; then | |
| echo " Warning: $target exists as a real path (not a symlink); skipping." | |
| echo " Remove it manually (rm -rf \"$target\") and re-run if you want the managed copy." | |
| continue |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.codex/install.sh around lines 74 - 77, The install script currently only
skips real directories but will overwrite existing regular files when it later
runs ln -sf; update the guard around $target so any existing non-symlink path is
skipped. Replace the current directory-only check (the branch that references
$target and the echo lines) with a check like: if [ -e "$target" ] && [ ! -L
"$target" ]; then echo the same warning messages (including the rm -rf
suggestion) and continue; otherwise proceed to create the symlink with ln -sf as
before. This ensures $target, ln -sf, and the echo warnings handle both regular
files and directories (non-symlinks) consistently with cli/metaswarm.js.
Comment on lines
+23
to
+24
| If `CODEX_HOME` is set, use `$CODEX_HOME` in place of `~/.codex`. | ||
|
|
There was a problem hiding this comment.
Keep Updating instructions consistent with CODEX_HOME support.
Line 23 introduces CODEX_HOME substitution, but the Updating command still hardcodes ~/.codex. Users with custom CODEX_HOME can run the wrong path.
Suggested doc update
-```bash
-cd ~/.codex/metaswarm && git pull
-```
+```bash
+cd "${CODEX_HOME:-$HOME/.codex}/metaswarm" && git pull
+```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.codex/README.md around lines 23 - 24, Update the "Updating" instruction to
respect CODEX_HOME by replacing the hardcoded "~/.codex" path with the
environment-variable-aware expansion "${CODEX_HOME:-$HOME/.codex}" so users who
set CODEX_HOME will have the correct path; locate the Updating command in the
README.md and change the cd command that currently uses "~/.codex/metaswarm" to
use the substitution for CODEX_HOME.
Comment on lines
+101
to
+106
| # 6b. README documents Codex skill installation and the new review skill | ||
| if grep -Fq '~/.codex/skills' "$ROOT/.codex/README.md"; then | ||
| pass ".codex/README.md documents ~/.codex/skills" | ||
| else | ||
| fail ".codex/README.md does not document ~/.codex/skills" | ||
| fi |
There was a problem hiding this comment.
Broaden the README path check.
Line 102 still looks for ~/.codex/skills, which will miss the documented ${CODEX_HOME:-$HOME/.codex}/skills wording and can fail even when the README is correct.
🔧 Suggested fix
-if grep -Fq '~/.codex/skills' "$ROOT/.codex/README.md"; then
+if grep -Fq '${CODEX_HOME:-$HOME/.codex}/skills' "$ROOT/.codex/README.md"; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # 6b. README documents Codex skill installation and the new review skill | |
| if grep -Fq '~/.codex/skills' "$ROOT/.codex/README.md"; then | |
| pass ".codex/README.md documents ~/.codex/skills" | |
| else | |
| fail ".codex/README.md does not document ~/.codex/skills" | |
| fi | |
| # 6b. README documents Codex skill installation and the new review skill | |
| if grep -Fq '${CODEX_HOME:-$HOME/.codex}/skills' "$ROOT/.codex/README.md"; then | |
| pass ".codex/README.md documents ~/.codex/skills" | |
| else | |
| fail ".codex/README.md does not document ~/.codex/skills" | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 102-102: Tilde does not expand in quotes. Use $HOME.
(SC2088)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/codex/test-codex-skills.sh` around lines 101 - 106, The README path
check in tests/codex/test-codex-skills.sh currently only greps for the literal
"~/.codex/skills" and will miss documentation that uses the variable form
"${CODEX_HOME:-$HOME/.codex}/skills"; update the conditional that uses grep (the
block that sets pass/fail) to broaden the pattern to accept either the literal
"~/.codex/skills" OR the variable-expanded form
"${CODEX_HOME:-$HOME/.codex}/skills" (or a concise regex that matches both
forms) so the test passes when the README documents the CODEX_HOME-based path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an iterative Codex-driven plan review loop and the underlying review skill, plus the v0.12.0 release polish (CHANGELOG, version bumps, doc rollout, installer hardening).
/codex-plan-review(commands/codex-plan-review.md, 442 lines) — Codex reviews an implementation plan, Claude reads P0/P1 findings and patches the plan, Codex re-reviews, until all blocking issues close or max rounds hit. Three tunable profiles (speed/balanced/quality), profile-aware terminal conditions (ACCEPT requires score ≥ floor inquality), score-trajectory tracking, hang-watcher, and aDeferred Gaps (codex)schema for partial wins.spec-gap-reviewskill (skills/spec-gap-review/) — Codex-runnable critical review of implementation guides, design docs, UX specs, infrastructure plans, memory designs, and roadmaps against actual repo state. Stable 6-dimension rubric (scope clarity, behavior completeness, contract specificity, repo alignment, technical feasibility, sequencing/operability), gap IDs (G01, G02…) carried across rounds, file:line citations validated byscripts/validate_line_links.py, doc-type focus checks, and a Critical-Only Mode for delta rounds..codex/install.sh+cli/metaswarm.js) — Skills now install to${CODEX_HOME:-$HOME/.codex}/skills/(was~/.agents/skills/metaswarm-*), so symlink names match the SKILL.mdnamefield directly and\$skill-nameinvocation works as documented. One-time legacy sweep removes danglingmetaswarm-*symlinks left behind in~/.agents/skills/by 0.10/0.11. Real-directory conflicts now print a clear remediation hint instead of a bare warning./codex-plan-reviewdocumented inCLAUDE.md,INSTALL.md,USAGE.md, and the setup templates.spec-gap-reviewdocumented inUSAGE.mdandAGENTS.md. Skill/command counts updated everywhere: 13 → 14 skills, 15 → 16 commands.package.json,.claude-plugin/plugin.json, andgemini-extension.json. CHANGELOG entry covers Added/Changed.lib/sync-resources.js: explanatory comment on whycodex-plan-reviewis intentionally NOT mapped into Gemini's TOML command set — it shells out to the Codex CLI, which Gemini users wouldn't have locally.Test plan
npm testpasses (coverstests/cli/test-installer.shandtests/codex/test-codex-skills.sh)npx metaswarm init --codexcreates symlinks under${CODEX_HOME:-\$HOME/.codex}/skills/with un-prefixed names (e.g.spec-gap-review, notmetaswarm-spec-gap-review)~/.agents/skills/metaswarm-*symlinks reports the legacy-sweep count and removes them/codex-plan-review <plan-file>runs end-to-end against a sample plan: produces gap report, applies P0/P1 patches, re-reviews, terminates on ACCEPT or max roundsscripts/validate_line_links.pyverifies file:line citations on a sample gap report/metaswarm:codex-plan-reviewis intentionally absent on Gemini🤖 Generated with Claude Code