[codex] route reviewer findings to source tasks - #321
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughRewrites Changessafety-nets Hook Fix and Version Bump
nihil Version Bump to 0.3.1
Contributor Guide Rewrite and AI Reviewer Coordination
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3ea5d4474
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if dirpath != soln_dir and any(fn.endswith((".sln", ".slnx")) for fn in filenames): | ||
| dirnames[:] = [] | ||
| continue |
There was a problem hiding this comment.
When a repo has a top-level .slnx plus a subdirectory .sln/.slnx used as a convenience solution, this early continue skips the subdirectory's own .csproj files entirely, so newly added projects there are never reported missing from the top-level solution. The hook is advertised as blocking .csproj files on disk that are not registered in the repo's .slnx, so this creates a false negative for those multi-solution layouts; prune known vendored/test-fixture roots more narrowly instead of treating any nested solution file as independent.
Useful? React with 👍 / 👎.
| @@ -1,91 +0,0 @@ | |||
| # Agent Operating Guide - ancplua-claude-plugins | |||
There was a problem hiding this comment.
Deleting the root AGENTS.md removes the repo-level guide that Codex/local agent sessions discover automatically, while the repo still ships .agents/plugins/marketplace.json and Codex-specific artifacts. In those Codex contexts the replacement CLAUDE.md/Cursor/Windsurf copies are not guaranteed to load, so contributors lose rules such as Codex plugin layout, generated-artifact handling, and validation commands unless they manually know to open Claude docs; keep a thin AGENTS.md pointer or copy for the Codex surface.
Useful? React with 👍 / 👎.
| 2. Register it in **both** manifests — `.claude-plugin/marketplace.json` (Claude) | ||
| and `.agents/plugins/marketplace.json` (Codex). Each entry needs `name`, | ||
| `source` (`./plugins/<name>`), `description`. |
There was a problem hiding this comment.
This replacement guide says every plugin entry in both manifests needs source as ./plugins/<name> plus description, but the Codex marketplace entries in .agents/plugins/marketplace.json use a source object with path, policy, and category, and do not have description. Following these lines for the Codex manifest would create entries that do not match the existing Codex marketplace schema, so the guide should spell out the two manifest shapes separately.
Useful? React with 👍 / 👎.
| ignored_dirs = { | ||
| os.path.normpath(os.path.join(soln_dir, p)) for p in out.split("\n") if p.strip() | ||
| } |
There was a problem hiding this comment.
When .gitignore ignores a project file directly (for example *.csproj or scratch/Foo.csproj), git ls-files -oi --directory returns that file path, this set stores it, but the later filter only compares directory names. The hook therefore still reports an ignored .csproj as missing from the .slnx, causing a Stop-hook false positive for generated or scratch projects that are intentionally untracked; either filter ignored files from filenames too or keep only directory entries here.
Useful? React with 👍 / 👎.
1 participant
Summary
slnx-syncfalse-positive hardening together so the published plugin cache sees the hook changeValidation
git diff --cached --checkruby -e 'require "yaml"; YAML.load_file(".coderabbit.yaml")'jqpython3 /Users/ancplua/.codex/skills/migrate-to-codex/scripts/migrate-to-codex.py --validate-target ..codex-plugin/plugin.jsonnihilandsafety-netsshellcheck plugins/safety-nets/hooks/slnx-sync-check.shNotes
The broad Codex plugin validator reports Claude-only plugins that intentionally have no
.codex-plugin/plugin.json; the scoped Codex-plugin loop validates all actual Codex plugins cleanly.Summary by CodeRabbit
Chores
nihilplugin to version 0.3.1 andsafety-netsplugin to version 0.1.1.Bug Fixes