feat!: drop review and assess tools (ADR-001)#28
Merged
Conversation
Retire `review` (bundled reviewer prompts + depth selector) and `assess`
(diff complexity classifier calibrated to review depths). Caller-supplied
prompts via the existing `codex` tool, or native `codex review --base`,
replace both. Rationale captured in ADR-001.
Removed:
- src/tools/review.ts and src/tools/assess.ts plus their tests
- prompts/review-{agentic,agentic-with-serena,focused,quick}.md
- scripts/serena-test.mjs (review A/B benchmark)
- Review-only env vars: scanTimeoutMs, focused/deep base/per-file/cap/fallback
timeouts in src/utils/env.ts (only consumer was review.ts)
Changed:
- Tool surface: codex, query, search, structured, ping, listSessions
- Version 0.5.1 -> 0.6.0 (package.json, server.json, package-lock.json)
- README + DESIGN + AGENTS + SECURITY updated to reflect reduced surface
- server.json description shortened to fit 100-char registry limit
Kept (still needed):
- getMcpServerOverride() in env.ts (codex/query/structured/search consumers)
- validateBaseRef, parseNumstat, getDiffStat in git.ts (codex tool path)
Verification: 341/341 vitest, npm run typecheck/lint/build clean,
external codex review (agentic depth) found 1 issue (lockfile version
mismatch) which is fixed.
Refs: docs/decisions/001-remove-review-and-assess-tools.md
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 4 minutes and 12 seconds. ⌛ 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR removes the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
tests/tools/descriptions.test.ts (1)
54-54: Consider addingqueryto the description-validation list.The post-removal public surface is six tools:
codex,query,search,structured,ping,listSessions. This array covers five —queryis not validated for non-empty / size-bounded description. Ifqueryis registered withregisterTool("query", …)insrc/index.ts, including it here keeps coverage symmetric with the documented surface.Proposed change
- const tools = ["codex", "search", "structured", "listSessions", "ping"]; + const tools = ["codex", "query", "search", "structured", "listSessions", "ping"];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/tools/descriptions.test.ts` at line 54, The test's tools list is missing the "query" tool, so update the const tools array used in descriptions.test.ts to include "query" alongside "codex", "search", "structured", "listSessions", and "ping" so the description validation (non-empty and size-bounded checks) covers the full public surface; locate the const tools declaration and add "query" to the list used by the validation assertions.docs/decisions/001-remove-review-and-assess-tools.md (1)
33-35: Optional: make the cross-reference an actual link.Line 23 uses a relative markdown link to the README anchor, but line 35's "Cross-references" entry is plain text. For consistency and click-through utility:
Proposed change
-- README § Code review with this CLI +- [README § Code review with this CLI](../../README.md#code-review-with-this-cli)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/decisions/001-remove-review-and-assess-tools.md` around lines 33 - 35, Replace the plain-text entry under the "Cross-references" heading with an actual Markdown link that targets the README anchor titled "Code review with this CLI": change the "README § Code review with this CLI" line to a Markdown link pointing to the README's "Code review with this CLI" section so readers can click through; update the line in docs/decisions/001-remove-review-and-assess-tools.md under the "Cross-references" heading accordingly.README.md (1)
29-30: Duplicate worktree note within a few sections.The "
-Cis broken forcodex review, run inside the worktree" advice appears twice in close proximity (lines 29–30 in the bash example, then again at lines 47–49 right after). Consider consolidating to a single mention with the issue link to keep the README tight.Also applies to: 47-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 29 - 30, The README contains a duplicated advisory about "`-C` is broken for `codex review`, run inside the worktree`" (the bash example block and the later note); remove the redundant occurrence and keep a single clear mention (preferably in the bash example) that includes the issue link, consolidating both instances into one cohesive sentence so the README is tighter and avoids repetition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 169-182: The fenced code block in README.md is missing a language
specifier which triggers markdownlint MD040; update the opening fence for the
example block (the triple backticks that currently start the snippet) to include
a language such as "text" or "markdown" (e.g., change ``` to ```text) so
renderers don't apply unintended highlighting; ensure the closing fence remains
``` and that surrounding text (the "Review the following diff:" line) stays
inside the fenced block.
In `@SECURITY.md`:
- Around line 58-60: The README incorrectly states `read-only` is the default
for `sandbox`; update the text to clarify that the Zod schema currently marks
`sandbox` as .optional() with no .default(), and the implementation passes
undefined to the Codex CLI so the upstream CLI's default actually controls
behavior; either (A) change the doc to say "`read-only` (recommended; specify
explicitly for guaranteed read-only mode)" and note that the CLI default is used
when unset, or (B) alter the code to set a concrete default (use Zod
.default('read-only') or explicitly pass 'read-only' to the CLI) so the behavior
is enforced—mention the Zod `sandbox` schema, the use of .optional()/.default(),
and the Codex CLI invocation when making the change.
In `@server.json`:
- Line 4: Update the package description string to accurately list all exposed
tools by name: replace the current value of "description" (the sentence that
lists tools) so it includes: codex, query, search, structured, ping,
listSessions — using the exact tool identifier "listSessions" (not "sessions")
and adding "ping". Ensure the new description text mirrors this exact tool list
for consistency with the bridge's actual exposed tools.
---
Nitpick comments:
In `@docs/decisions/001-remove-review-and-assess-tools.md`:
- Around line 33-35: Replace the plain-text entry under the "Cross-references"
heading with an actual Markdown link that targets the README anchor titled "Code
review with this CLI": change the "README § Code review with this CLI" line to a
Markdown link pointing to the README's "Code review with this CLI" section so
readers can click through; update the line in
docs/decisions/001-remove-review-and-assess-tools.md under the
"Cross-references" heading accordingly.
In `@README.md`:
- Around line 29-30: The README contains a duplicated advisory about "`-C` is
broken for `codex review`, run inside the worktree`" (the bash example block and
the later note); remove the redundant occurrence and keep a single clear mention
(preferably in the bash example) that includes the issue link, consolidating
both instances into one cohesive sentence so the README is tighter and avoids
repetition.
In `@tests/tools/descriptions.test.ts`:
- Line 54: The test's tools list is missing the "query" tool, so update the
const tools array used in descriptions.test.ts to include "query" alongside
"codex", "search", "structured", "listSessions", and "ping" so the description
validation (non-empty and size-bounded checks) covers the full public surface;
locate the const tools declaration and add "query" to the list used by the
validation assertions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b2028fee-d0dc-48d4-9538-aec8a020db45
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
AGENTS.mdCHANGELOG.mdDESIGN.mdREADME.mdSECURITY.mddocs/decisions/001-remove-review-and-assess-tools.mdpackage.jsonprompts/review-agentic-with-serena.mdprompts/review-agentic.mdprompts/review-focused.mdprompts/review-quick.mdscripts/serena-test.mjsscripts/smoke-test.mjsserver.jsonsrc/annotations.tssrc/index.tssrc/tools/assess.tssrc/tools/review.tssrc/utils/codex-config.tssrc/utils/env.tssrc/utils/git.tssrc/utils/progress.tstests/tools/annotations.test.tstests/tools/assess.test.tstests/tools/descriptions.test.tstests/tools/review-prompts.test.tstests/tools/review-timeout.test.tstests/tools/review.test.tstests/utils/env.test.tstests/utils/git.test.ts
💤 Files with no reviewable changes (13)
- tests/tools/review-prompts.test.ts
- prompts/review-agentic.md
- scripts/serena-test.mjs
- tests/tools/annotations.test.ts
- src/index.ts
- tests/tools/assess.test.ts
- src/tools/assess.ts
- prompts/review-agentic-with-serena.md
- prompts/review-focused.md
- tests/tools/review-timeout.test.ts
- tests/tools/review.test.ts
- prompts/review-quick.md
- src/tools/review.ts
- README.md: add `text` language to fenced code block (markdownlint MD040) - src/index.ts: enforce `read-only` sandbox default in Zod schema so the documented default is no longer reliant on upstream Codex CLI behavior - server.json: correct registry description to list all 6 exposed tools (was missing `ping`, used `sessions` instead of `listSessions`)
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
reviewandassessMCP tools. Caller-supplied prompts via the existingcodextool (sandbox: read-only) or nativecodex review --basereplace both paths.Breaking change
Tool surface change. Callers using
mcp__codex__review/mcp__codex__assessmust migrate before upgrading to 0.6.0. Migration paths:mcp__codex__codexwithsandbox: "read-only"and a prompt that includes diff context (reference files or inlined diff).codex review --base <branch>from the host shell.The
assesstool's role was selecting areviewdepth; withoutreview, those recommendations have no consumer.Removed
src/tools/review.ts,src/tools/assess.ts, plus their tests (review.test.ts,review-prompts.test.ts,review-timeout.test.ts,assess.test.ts)prompts/review-{agentic,agentic-with-serena,focused,quick}.mdscripts/serena-test.mjs(review-tool A/B benchmark, no consumer)src/utils/env.ts:scanTimeoutMs,focused{Base,PerFile,Cap,Fallback}Ms,deep{Base,PerFile,Fallback}Ms. The only consumer wasreview.ts. Per-calltimeouton thecodextool replaces these for callers that need bounded review runs.Kept (still load-bearing)
getMcpServerOverride()inenv.ts: still consumed by codex / query / structured / search.validateBaseRef,parseNumstat,getDiffStatingit.ts: still consumed by thecodextool's caller-supplied diff path.Changed
package.json,server.json(both fields), andpackage-lock.json.server.jsondescription shortened to 73 chars to fit the registry's 100-char hard limit.Review
package-lock.jsonversion drift (0.5.1 vs 0.6.0). Applied vianpm install --package-lock-only, included in this commit.Changes
31 files changed, +169 / -2830
Test plan
npm test, 341/341 pass (re-run after lockfile bump)npm run typecheckcleannpm run lintcleannpm run buildclean;dist/tools/contains only kept toolsRefs: ADR-001, docs/decisions/001-remove-review-and-assess-tools.md
Summary by CodeRabbit
Release Notes
Removals
reviewandassessMCP tools. Code review now performed viacodextool withsandbox: "read-only"and user-supplied prompts, or nativecodex reviewcommand.Documentation
Chores