feat(agents): add typescript-reviewer agent#647
Conversation
|
Analysis Failed
Troubleshooting
Retry: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDocs updated to register four new agents ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Greptile SummaryThis PR adds a dedicated The agent's review-priority rubric is thorough and well-scoped: it correctly uses Key points:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([typescript-reviewer invoked]) --> B[Step 1: Establish review scope\ngit diff / gh pr view / git show --patch]
B --> C[Step 2: Inspect merge readiness\ngh pr view --json mergeStateStatus]
C -->|Conflicts / failing CI| D([Stop & report])
C -->|Merge-ready or unverifiable| E[Step 3: Run canonical typecheck\nnpm/pnpm/yarn/bun run typecheck\nor tsc --noEmit -p config]
E --> F[Step 4: Run eslint\neslint . --ext .ts,.tsx,.js,.jsx]
F -->|Fails| G([Stop & report errors])
F -->|Passes| H{Step 5: Any TS/JS\nchanges in diff?}
H -->|No| I([Stop & report — scope\ncannot be established])
H -->|Yes| J[Step 6: Read modified files\n+ surrounding context]
J --> K[Step 7: Begin review\nagainst priority rubric]
K --> L{Issues found?}
L -->|CRITICAL or HIGH| M([Block])
L -->|MEDIUM only| N([Warning])
L -->|None| O([Approve])
style E fill:#ffdddd,stroke:#cc0000
style F fill:#ffdddd,stroke:#cc0000
style H fill:#ffdddd,stroke:#cc0000
subgraph note[" ⚠️ Current ordering flaw"]
E
F
H
end
Last reviewed commit: "fix(agents): keep re..." |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
AGENTS.md (1)
3-40:⚠️ Potential issue | 🟡 MinorAgent totals conflict with the “Available Agents” table.
Line 3 and Line 138 claim 26 agents, but the table in this section currently lists fewer entries. Please sync the stated totals and the enumerated list to avoid contradictory docs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 3 - 40, The document header’s agent count ("26" agents in the intro lines) is inconsistent with the actual enumerated table under "Available Agents"; update either the numeric totals (the "26" mentions at the top) or the table entries so they match. Locate the "Available Agents" table and reconcile the count by either adding the missing agent rows to the table or reducing the numeric total references to the actual number of listed agents (ensure you update every occurrence of the totals in the intro paragraphs). Also search for the specific phrases "26 specialized agents" and "Available Agents" to ensure all references remain consistent after the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 39: The docs are missing a mapping entry for the new agent
"typescript-reviewer"; update the command-to-agent mapping in
COMMAND-AGENT-MAP.md by adding a row that maps the TypeScript/JavaScript review
command to the "typescript-reviewer" agent (match the table format used for
other agents and include the description "TypeScript/JavaScript code review" and
the scope "TypeScript/JavaScript projects") so the new agent is discoverable in
the command mapping docs.
In `@README.md`:
- Line 1045: The README has inconsistent Claude Code agent counts (one place
shows "26 agents" in the Agents table row containing "| Agents | ✅ 26 agents | ✅
12 agents | **Claude Code leads** |" while the cross-tool parity table later
shows 21); pick the correct authoritative count, then update every occurrence of
the Claude Code agent count (the Agents table row text and the Claude Code entry
in the cross-tool parity table, plus any other mentions) to that single agreed
value so all references match.
---
Outside diff comments:
In `@AGENTS.md`:
- Around line 3-40: The document header’s agent count ("26" agents in the intro
lines) is inconsistent with the actual enumerated table under "Available
Agents"; update either the numeric totals (the "26" mentions at the top) or the
table entries so they match. Locate the "Available Agents" table and reconcile
the count by either adding the missing agent rows to the table or reducing the
numeric total references to the actual number of listed agents (ensure you
update every occurrence of the totals in the intro paragraphs). Also search for
the specific phrases "26 specialized agents" and "Available Agents" to ensure
all references remain consistent after the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6beb9070-fcae-459c-b542-32915c68ba14
📒 Files selected for processing (3)
AGENTS.mdREADME.mdagents/typescript-reviewer.md
There was a problem hiding this comment.
5 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="agents/typescript-reviewer.md">
<violation number="1" location="agents/typescript-reviewer.md:12">
P2: Inconsistent lint command guidance (`eslint .` vs explicit TS/JS extensions) can cause incomplete TypeScript lint checks.</violation>
<violation number="2" location="agents/typescript-reviewer.md:14">
P2: Reviewer workflow starts manual review immediately without checking required CI-pass and merge-conflict gates mandated by team feedback.</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:1045">
P2: README has conflicting catalog totals (26/108/57 vs stale 21/52/102), indicating partial synchronization and inconsistent user-facing metrics.</violation>
</file>
<file name="AGENTS.md">
<violation number="1" location="AGENTS.md:3">
P3: AGENTS.md has inconsistent hardcoded skill counts (108 in intro vs 102 in Project Structure), leaving repository metadata contradictory.</violation>
<violation number="2" location="AGENTS.md:3">
P2: Agent totals are inconsistent: the document claims 26 agents but lists only 23 in the Available Agents table.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="agents/typescript-reviewer.md">
<violation number="1" location="agents/typescript-reviewer.md:11">
P2: Hard-coded `origin/main` makes diff-base resolution brittle across repos/remotes and can break review scope detection.</violation>
<violation number="2" location="agents/typescript-reviewer.md:11">
P2: Documented fallback to `HEAD~1` on `main` is not actually implemented; the unconditional diff can evaluate to `git diff HEAD HEAD` and miss all changes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="agents/typescript-reviewer.md">
<violation number="1" location="agents/typescript-reviewer.md:11">
P1: Base ref discovery is ambiguous and may select the wrong remote HEAD, producing inaccurate diff scope for reviews.</violation>
<violation number="2" location="agents/typescript-reviewer.md:11">
P1: Fallback to `git diff -- <pathspec>` can return an empty diff in clean/shallow environments, causing the reviewer to miss changed TS/JS files.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
3 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="agents/typescript-reviewer.md">
<violation number="1" location="agents/typescript-reviewer.md:12">
P2: Fallbacking to `git diff HEAD~1` can miss earlier commits in a multi-commit PR branch, causing incomplete review coverage.</violation>
<violation number="2" location="agents/typescript-reviewer.md:13">
P2: Shallow-checkout fallback uses `git show --name-only --stat`, which hides patch hunks and prevents code-level diff review.</violation>
<violation number="3" location="agents/typescript-reviewer.md:17">
P2: Review instructions do not enforce required CI/conflict preconditions and instead tell the agent to proceed immediately, conflicting with team gating policy.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="agents/typescript-reviewer.md">
<violation number="1" location="agents/typescript-reviewer.md:12">
P2: The updated review workflow removed the fallback for missing `HEAD~1`, so diff collection can fail in shallow or single-commit checkouts.</violation>
<violation number="2" location="agents/typescript-reviewer.md:12">
P2: PR diff instructions hard-code `main` as the comparison base, which can produce incorrect or failing diffs for non-`main` PR targets.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="agents/typescript-reviewer.md">
<violation number="1" location="agents/typescript-reviewer.md:15">
P2: TypeScript check instruction is inconsistent: it detects `tsconfig.*.json` but runs bare `tsc --noEmit`, which does not automatically use non-default config filenames.</violation>
<violation number="2" location="agents/typescript-reviewer.md:19">
P2: CI/merge readiness is documented as advisory instead of a blocking precondition, allowing review to proceed when checks/conflicts should defer review.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Analysis Failed
Troubleshooting
Retry: |
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="agents/typescript-reviewer.md">
<violation number="1" location="agents/typescript-reviewer.md:19">
P2: Type-check guidance over-prioritizes `tsconfig.json`, which can miss relevant package/project-reference checks and produce false-green reviews.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="agents/typescript-reviewer.md">
<violation number="1" location="agents/typescript-reviewer.md:93">
P2: Fallback `tsc -b` type-check guidance can write build artifacts during review because `--noEmit` is not required.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="agents/typescript-reviewer.md">
<violation number="1" location="agents/typescript-reviewer.md:19">
P2: Project-reference typecheck guidance became ambiguous by removing the explicit `tsc -b <solution-config>` fallback, which can lead reviewers to run only package-scoped checks and miss cross-project type errors.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| - If required checks are failing or pending, stop and report that review should wait for green CI. | ||
| - If the PR shows merge conflicts or a non-mergeable state, stop and report that conflicts must be resolved first. | ||
| - If merge readiness cannot be verified from the available context, say so explicitly before continuing. | ||
| 3. Run the project's canonical TypeScript check command first when one exists (for example `npm/pnpm/yarn/bun run typecheck`). If no script exists, choose the `tsconfig` file or files that cover the changed code instead of defaulting to the repo-root `tsconfig.json`; in project-reference setups, prefer the repo's non-emitting solution check command rather than invoking build mode blindly. Otherwise use `tsc --noEmit -p <relevant-config>`. Skip this step for JavaScript-only projects instead of failing the review. |
There was a problem hiding this comment.
P2: Project-reference typecheck guidance became ambiguous by removing the explicit tsc -b <solution-config> fallback, which can lead reviewers to run only package-scoped checks and miss cross-project type errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At agents/typescript-reviewer.md, line 19:
<comment>Project-reference typecheck guidance became ambiguous by removing the explicit `tsc -b <solution-config>` fallback, which can lead reviewers to run only package-scoped checks and miss cross-project type errors.</comment>
<file context>
@@ -16,7 +16,7 @@ When invoked:
- If the PR shows merge conflicts or a non-mergeable state, stop and report that conflicts must be resolved first.
- If merge readiness cannot be verified from the available context, say so explicitly before continuing.
-3. Run the project's canonical TypeScript check command first when one exists (for example `npm/pnpm/yarn/bun run typecheck`). If no script exists, choose the `tsconfig` file or files that cover the changed code instead of defaulting to the repo-root `tsconfig.json`; for project references, prefer `tsc -b <solution-config>`, otherwise use `tsc --noEmit -p <relevant-config>`. Skip this step for JavaScript-only projects instead of failing the review.
+3. Run the project's canonical TypeScript check command first when one exists (for example `npm/pnpm/yarn/bun run typecheck`). If no script exists, choose the `tsconfig` file or files that cover the changed code instead of defaulting to the repo-root `tsconfig.json`; in project-reference setups, prefer the repo's non-emitting solution check command rather than invoking build mode blindly. Otherwise use `tsc --noEmit -p <relevant-config>`. Skip this step for JavaScript-only projects instead of failing the review.
4. Run `eslint . --ext .ts,.tsx,.js,.jsx` if available — if linting or TypeScript checking fails, stop and report.
5. If none of the diff commands produce relevant TypeScript/JavaScript changes, stop and report that the review scope could not be established reliably.
</file context>
| 3. Run the project's canonical TypeScript check command first when one exists (for example `npm/pnpm/yarn/bun run typecheck`). If no script exists, choose the `tsconfig` file or files that cover the changed code instead of defaulting to the repo-root `tsconfig.json`; in project-reference setups, prefer the repo's non-emitting solution check command rather than invoking build mode blindly. Otherwise use `tsc --noEmit -p <relevant-config>`. Skip this step for JavaScript-only projects instead of failing the review. | ||
| 4. Run `eslint . --ext .ts,.tsx,.js,.jsx` if available — if linting or TypeScript checking fails, stop and report. | ||
| 5. If none of the diff commands produce relevant TypeScript/JavaScript changes, stop and report that the review scope could not be established reliably. | ||
| 6. Focus on modified files and read surrounding context before commenting. | ||
| 7. Begin review |
There was a problem hiding this comment.
Steps 3 and 4 invoke tsc and eslint unconditionally before step 5 checks whether the diff even contains TypeScript/JavaScript files. For a PR that touches only Python, Go, or config files, the agent will:
- Run
npm run typecheck(ortsc --noEmit) — potentially reporting pre-existing type errors completely unrelated to the PR - Run
eslint . --ext .ts,.tsx,.js,.jsx— same problem - Then reach step 5, find no TS/JS diff, and stop
This creates two concrete problems: (a) the agent wastes time running expensive tooling on unchanged code, and (b) it may produce a misleading Block outcome due to pre-existing type errors that have nothing to do with the PR under review.
Compare to rust-reviewer and go-reviewer, which diff the relevant files first and only then run diagnostics.
The guard in step 5 should move to immediately after step 2 (merge-readiness check), so the agent exits early before touching any tooling:
| 3. Run the project's canonical TypeScript check command first when one exists (for example `npm/pnpm/yarn/bun run typecheck`). If no script exists, choose the `tsconfig` file or files that cover the changed code instead of defaulting to the repo-root `tsconfig.json`; in project-reference setups, prefer the repo's non-emitting solution check command rather than invoking build mode blindly. Otherwise use `tsc --noEmit -p <relevant-config>`. Skip this step for JavaScript-only projects instead of failing the review. | |
| 4. Run `eslint . --ext .ts,.tsx,.js,.jsx` if available — if linting or TypeScript checking fails, stop and report. | |
| 5. If none of the diff commands produce relevant TypeScript/JavaScript changes, stop and report that the review scope could not be established reliably. | |
| 6. Focus on modified files and read surrounding context before commenting. | |
| 7. Begin review | |
| 3. If none of the diff commands from step 1 produce relevant TypeScript/JavaScript changes, stop and report that the review scope could not be established reliably. | |
| 4. Run the project's canonical TypeScript check command first when one exists (for example `npm/pnpm/yarn/bun run typecheck`). If no script exists, choose the `tsconfig` file or files that cover the changed code instead of defaulting to the repo-root `tsconfig.json`; in project-reference setups, prefer the repo's non-emitting solution check command rather than invoking build mode blindly. Otherwise use `tsc --noEmit -p <relevant-config>`. Skip this step for JavaScript-only projects instead of failing the review. | |
| 5. Run `eslint . --ext .ts,.tsx,.js,.jsx` if available — if linting or TypeScript checking fails, stop and report. | |
| 6. Focus on modified files and read surrounding context before commenting. | |
| 7. Begin review |
| ## Diagnostic Commands | ||
|
|
||
| ```bash | ||
| npm run typecheck --if-present # Canonical TypeScript check when the project defines one |
There was a problem hiding this comment.
--if-present flag is npm-only
npm run typecheck --if-present uses an npm-specific flag. Its behavior differs across package managers:
pnpm run --if-present typecheck— works, but the flag position differsyarn run typecheck— exits with a non-zero code if the script is absent (no--if-presentequivalent in Yarn classic)bun run typecheck— also exits with an error if the script is absent
Step 3 correctly describes using npm/pnpm/yarn/bun run typecheck, but the Diagnostic Commands block shows only the npm form. Users copy-pasting the block in a pnpm/yarn/bun project will get unexpected errors. Adding equivalents (or at minimum a clarifying note) would keep this consistent with the multi-PM awareness shown elsewhere in the file:
| npm run typecheck --if-present # Canonical TypeScript check when the project defines one | |
| npm run typecheck --if-present # npm: canonical TypeScript check (skip if script absent) | |
| pnpm run --if-present typecheck # pnpm equivalent | |
| yarn run typecheck # yarn (exits with error if script absent — check package.json first) | |
| bun run typecheck # bun equivalent |
Summary
agents/typescript-reviewer.mdas a dedicated TypeScript/JavaScript reviewer agentAGENTS.mdand syncREADME.mdagent counts so repository catalog checks stay greenWhy
TypeScript already has first-class rules in ECC, but it was missing a dedicated language reviewer agent unlike Go, Kotlin, Python, Java, and Rust. This keeps the contribution small and merge-friendly while filling a real gap in the agent catalog.
Related to #543.
Type
Testing
node scripts/ci/validate-agents.jsnpm testChecklist
Summary by cubic
Adds a dedicated
typescript-revieweragent for TS/JS code review with scoped diffs, non-emitting type/lint checks, and merge-readiness gates. Updates docs to 26 agents and adds direct-invoke guidance indocs/COMMAND-AGENT-MAP.md.New Features
typescript-reviewerfocused on type safety, async correctness, security, and idiomatic Node/React/Next.git showfallback; scope to TS/JS; run non-emitting typecheck and lint; gate on CI/merge state; output Approve/Warning/Block with diagnostic commands (types/lint/tests and package audits fornpm/yarn/pnpm/bun).Bug Fixes
typecheckscripts over rawtsc; when falling back, run non-emitting checks (tsc --noEmit -p <config>or a non-emitting solution check) and avoidtsc -b/build mode.Written for commit 975d2e7. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation