Repository navigation
feat: add multi-client agent evaluation for Claude Code and Gemini CLI (issue #1932) - #8930
saranshkhanna009 wants to merge 1 commit into
Conversation
This implements issue morluto#1932 by adding support for running agent evaluation against Claude Code and Gemini CLI in addition to Codex. Changes: - scripts/verify-agent-experience.mjs: Add REA_AGENT_EVAL_CLIENT environment variable to select client (codex, claude-code, gemini-cli). Add isolated homes for each client. Use rea setup for each client. - scripts/lib/agent-evaluation-commands.mjs: Add runClaude and runGemini functions with proper CLI arguments for stream-json output. Add executeJsonLines helper for parsing JSONL streams. Add readAgentVersion for all clients. - tests/evaluation/model-evals/AgentTranscript.ts: Add normalizeClaudeEvents and normalizeGeminiEvents to convert client-specific event formats to the shared evaluator shape. Add normalizeAgentEvents dispatcher. - tests/evaluation/model-evals/AgentTranscript.test.ts: Add tests for both normalizers. - docs/client-verification.md: Update documentation to reflect multi-client support. The evaluator now supports: - REA_AGENT_EVAL_CLIENT=codex (default) - REA_AGENT_EVAL_CLIENT=claude-code - REA_AGENT_EVAL_CLIENT=gemini-cli With corresponding command overrides: - REA_AGENT_EVAL_CLAUDE_COMMAND - REA_AGENT_EVAL_GEMINI_COMMAND Co-Authored-By: Claude Code <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
 [Medium impact] The changes affect the agent evaluation workflow and its reports. Do not merge until command approvals are restricted and the typecheck, transcript scoring, launcher, and cleanup-test failures are fixed. Findings
Fix with agent prompt### Issue 1
scripts/lib/agent-evaluation-commands.mjs:186
`runClaude` and `runGemini` ignore `scenario.approvedTools` and approve every REA tool. This includes `capture_process_scenario`, which runs a chosen command with the user's permissions. If an agent tries to execute a fixture or repair a failed analysis, it can change files outside `evaluationRoot` without asking. Changing `HOME` does not prevent this.
Use one shared safe-tool policy with explicit scenario exceptions, and restrict Gemini's shell and write tools.
> **How this was verified:** Both launchers approve the full REA catalog, and `capture_process_scenario` forwards the chosen command to the host without restricting it to the evaluation folder.
### Issue 2
tests/evaluation/model-evals/AgentTranscript.ts:27
`mcpToolNames` is inferred as a set of the literal names in `TOOL_CONTRACTS`, but `name` is a general `string`. Passing it to `mcpToolNames.has(name)` produces a TypeScript argument error. The new file is included in the source typecheck, so this blocks the check before either new client can be evaluated. Declare the lookup as `Set<string>` or `ReadonlySet<string>`.
### Issue 3
tests/evaluation/model-evals/AgentTranscript.ts:34-36
Claude's array-form `tool_result.content` is serialized into another text block here. A result containing one text block therefore becomes JSON for an array, not the Evidence object inside that block. `parseToolResult` reads that array, while `evidenceFromCall` requires an object. Valid results lose their Evidence and error codes, causing factual scenarios and expected provider refusals to fail.
Preserve the content blocks and available structured results instead of serializing their container.
### Issue 4
tests/evaluation/model-evals/AgentTranscript.ts:209-212
`assistantText` keeps every assistant message across tool calls, then joins them into the final answer. If Gemini says it will inspect the artifact before calling a tool, that earlier prose is prepended to its otherwise valid final JSON. `evaluateKnownAnswers` then rejects the answer as `invalid_final_claim_manifest`.
Join text chunks within each response, but keep the final response separate from earlier commentary.
### Issue 5
scripts/verify-agent-experience.mjs:181
The version check now runs after the client directories are created, but the existing cleanup fixtures still create those directories themselves without `recursive: true`. They now fail with `EEXIST` before writing their receipts or producing the expected cleanup failure.
Update the fixtures in `agentEvaluationCommands.test.ts` and `agentEvaluationCleanup.test.ts` to use the new startup order while preserving their cleanup assertions.
### Issue 6
scripts/lib/agent-evaluation-commands.mjs:37-40
A relative JavaScript path in `REA_AGENT_EVAL_GEMINI_COMMAND` resolves from two different folders. `readAgentVersion` runs from `repositoryRoot`, but `runGemini` runs from `evaluationRoot`. A path such as `node_modules/@google/gemini-cli/bundle/gemini.js` can pass the version check and then fail with a missing-module error for every scenario.
Resolve the selected entry point once before changing the working directory, as `verify-gemini-client.mjs` already does.
### Issue 7
scripts/lib/agent-evaluation-commands.mjs:257-268
`readAgentVersion` duplicates the execution, timeout, exit-code, and empty-output checks in `readCommandVersion`. The runner now uses the new function, while the existing boundary tests still exercise only the old one. Fixes can drift between the two copies without those tests catching it.
`AGENTS.md` requires removing superseded representations after migrating their consumers. Before merging, keep one version-reading implementation and move the existing checks onto the path the runner actually uses.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
### Issue 8
README.md:3
This removes every language link from the main README, although the translated READMEs still exist. Readers arriving at the repository can no longer choose their language from its landing page. Restore the language switcher; this removal is unrelated to adding evaluation clients.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryAdds Claude Code and Gemini CLI to the existing agent evaluation lane, with separate client homes, transcript conversion, and client metadata in reports.
Review was source-only; no tests were run. Whether this PR was superseded could not be checked because GitHub access was unauthenticated. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart LR
A["Selected client"] --> B["Setup in separate client home"]
B --> C{"Launch client"}
C --> D["Codex events"]
C --> E["Claude events"]
C --> F["Gemini events"]
D --> G["Shared scorer"]
E --> H["Convert transcript"]
F --> H
H --> G
G --> I["Report and pass gate"]
Reviews (1) · Last reviewed commit: "feat: add multi-client agent evaluation ..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| "--settings", | ||
| JSON.stringify({ | ||
| permissions: { | ||
| allow: ["Skill", "WaitForMcpServers", "mcp__rea__*"], |
There was a problem hiding this comment.
runClaude and runGemini ignore scenario.approvedTools and approve every REA tool. This includes capture_process_scenario, which runs a chosen command with the user's permissions. If an agent tries to execute a fixture or repair a failed analysis, it can change files outside evaluationRoot without asking. Changing HOME does not prevent this.
Use one shared safe-tool policy with explicit scenario exceptions, and restrict Gemini's shell and write tools.
How this was verified: Both launchers approve the full REA catalog, and
capture_process_scenarioforwards the chosen command to the host without restricting it to the evaluation folder.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/lib/agent-evaluation-commands.mjs
Line: 186
Comment:
**Evaluations can change host files**
`runClaude` and `runGemini` ignore `scenario.approvedTools` and approve every REA tool. This includes `capture_process_scenario`, which runs a chosen command with the user's permissions. If an agent tries to execute a fixture or repair a failed analysis, it can change files outside `evaluationRoot` without asking. Changing `HOME` does not prevent this.
Use one shared safe-tool policy with explicit scenario exceptions, and restrict Gemini's shell and write tools.
> **How this was verified:** Both launchers approve the full REA catalog, and `capture_process_scenario` forwards the chosen command to the host without restricting it to the evaluation folder.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| return { server: "rea", tool: name.slice("mcp__rea__".length) }; | ||
| if (name.startsWith("rea__")) | ||
| return { server: "rea", tool: name.slice("rea__".length) }; | ||
| if (mcpToolNames.has(name)) return { server: "rea", tool: name }; |
There was a problem hiding this comment.
mcpToolNames is inferred as a set of the literal names in TOOL_CONTRACTS, but name is a general string. Passing it to mcpToolNames.has(name) produces a TypeScript argument error. The new file is included in the source typecheck, so this blocks the check before either new client can be evaluated. Declare the lookup as Set<string> or ReadonlySet<string>.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/evaluation/model-evals/AgentTranscript.ts
Line: 27
Comment:
**Transcript typecheck fails**
`mcpToolNames` is inferred as a set of the literal names in `TOOL_CONTRACTS`, but `name` is a general `string`. Passing it to `mcpToolNames.has(name)` produces a TypeScript argument error. The new file is included in the source typecheck, so this blocks the check before either new client can be evaluated. Declare the lookup as `Set<string>` or `ReadonlySet<string>`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const toolResult = (value: unknown, isError = false): RecordValue => ({ | ||
| ...(isError ? { isError: true } : {}), | ||
| content: [{ type: "text", text: jsonText(value) }], |
There was a problem hiding this comment.
Claude's array-form tool_result.content is serialized into another text block here. A result containing one text block therefore becomes JSON for an array, not the Evidence object inside that block. parseToolResult reads that array, while evidenceFromCall requires an object. Valid results lose their Evidence and error codes, causing factual scenarios and expected provider refusals to fail.
Preserve the content blocks and available structured results instead of serializing their container.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/evaluation/model-evals/AgentTranscript.ts
Line: 34-36
Comment:
**Claude results lose Evidence**
Claude's array-form `tool_result.content` is serialized into another text block here. A result containing one text block therefore becomes JSON for an array, not the Evidence object inside that block. `parseToolResult` reads that array, while `evidenceFromCall` requires an object. Valid results lose their Evidence and error codes, causing factual scenarios and expected provider refusals to fail.
Preserve the content blocks and available structured results instead of serializing their container.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if (assistantText.length > 0) | ||
| normalized.push({ | ||
| type: "item.completed", | ||
| item: { type: "agent_message", text: assistantText.join("") }, |
There was a problem hiding this comment.
assistantText keeps every assistant message across tool calls, then joins them into the final answer. If Gemini says it will inspect the artifact before calling a tool, that earlier prose is prepended to its otherwise valid final JSON. evaluateKnownAnswers then rejects the answer as invalid_final_claim_manifest.
Join text chunks within each response, but keep the final response separate from earlier commentary.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/evaluation/model-evals/AgentTranscript.ts
Line: 209-212
Comment:
**Commentary breaks Gemini scoring**
`assistantText` keeps every assistant message across tool calls, then joins them into the final answer. If Gemini says it will inspect the artifact before calling a tool, that earlier prose is prepended to its otherwise valid final JSON. `evaluateKnownAnswers` then rejects the answer as `invalid_final_claim_manifest`.
Join text chunks within each response, but keep the final response separate from earlier commentary.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| await writeFile(join(geminiHome, "system-defaults.json"), "{}\n", { | ||
| mode: 0o600, | ||
| }); | ||
| const clientVersion = await readAgentVersion(client, evaluationEnvironment); |
There was a problem hiding this comment.
The version check now runs after the client directories are created, but the existing cleanup fixtures still create those directories themselves without recursive: true. They now fail with EEXIST before writing their receipts or producing the expected cleanup failure.
Update the fixtures in agentEvaluationCommands.test.ts and agentEvaluationCleanup.test.ts to use the new startup order while preserving their cleanup assertions.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/verify-agent-experience.mjs
Line: 181
Comment:
**Cleanup fixtures fail before checks**
The version check now runs after the client directories are created, but the existing cleanup fixtures still create those directories themselves without `recursive: true`. They now fail with `EEXIST` before writing their receipts or producing the expected cleanup failure.
Update the fixtures in `agentEvaluationCommands.test.ts` and `agentEvaluationCleanup.test.ts` to use the new startup order while preserving their cleanup assertions.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const launchGemini = (arguments_) => | ||
| /\.(?:c|m)?js$/iu.test(gemini) | ||
| ? [process.execPath, [gemini, ...arguments_]] | ||
| : [gemini, arguments_]; |
There was a problem hiding this comment.
A relative JavaScript path in REA_AGENT_EVAL_GEMINI_COMMAND resolves from two different folders. readAgentVersion runs from repositoryRoot, but runGemini runs from evaluationRoot. A path such as node_modules/@google/gemini-cli/bundle/gemini.js can pass the version check and then fail with a missing-module error for every scenario.
Resolve the selected entry point once before changing the working directory, as verify-gemini-client.mjs already does.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/lib/agent-evaluation-commands.mjs
Line: 37-40
Comment:
**Relative Gemini paths break**
A relative JavaScript path in `REA_AGENT_EVAL_GEMINI_COMMAND` resolves from two different folders. `readAgentVersion` runs from `repositoryRoot`, but `runGemini` runs from `evaluationRoot`. A path such as `node_modules/@google/gemini-cli/bundle/gemini.js` can pass the version check and then fail with a missing-module error for every scenario.
Resolve the selected entry point once before changing the working directory, as `verify-gemini-client.mjs` already does.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| async readAgentVersion(client, env = evaluationEnvironment) { | ||
| const [command, arguments_] = | ||
| client === "gemini-cli" | ||
| ? launchGemini(["--version"]) | ||
| : [client === "claude-code" ? claude : codex, ["--version"]]; | ||
| const result = await execute( | ||
| command, | ||
| arguments_, | ||
| repositoryRoot, | ||
| env, | ||
| { timeoutMs: 10_000, diagnosticBytes: 8192 }, | ||
| `${client} --version timed out`, |
There was a problem hiding this comment.
readAgentVersion duplicates the execution, timeout, exit-code, and empty-output checks in readCommandVersion. The runner now uses the new function, while the existing boundary tests still exercise only the old one. Fixes can drift between the two copies without those tests catching it.
AGENTS.md requires removing superseded representations after migrating their consumers. Before merging, keep one version-reading implementation and move the existing checks onto the path the runner actually uses.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/lib/agent-evaluation-commands.mjs
Line: 257-268
Comment:
**Version checks have two owners**
`readAgentVersion` duplicates the execution, timeout, exit-code, and empty-output checks in `readCommandVersion`. The runner now uses the new function, while the existing boundary tests still exercise only the old one. Fixes can drift between the two copies without those tests catching it.
`AGENTS.md` requires removing superseded representations after migrating their consumers. Before merging, keep one version-reading implementation and move the existing checks onto the path the runner actually uses.
**Context Used:** AGENTS.md ([source](https://github.com/morluto/rea/blob/main/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
|
||
| **English** · [简体中文](docs/readme/README_zh.md) · [繁體中文](docs/readme/README_zh-TW.md) · [日本語](docs/readme/README_ja.md) · [한국어](docs/readme/README_ko.md) · [Türkçe](docs/readme/README_tr.md) · [Русский](docs/readme/README_ru.md) · [Tiếng Việt](docs/readme/README_vi.md) · [ไทย](docs/readme/README_th.md) · [Deutsch](docs/readme/README_de.md) · [Español](docs/readme/README_es.md) · [Français](docs/readme/README_fr.md) · [Українська](docs/readme/README_uk.md) · [Polski](docs/readme/README_pl.md) · [Português (Brasil)](docs/readme/README_pt-BR.md) · [العربية](docs/readme/README_ar.md) · [فارسی](docs/readme/README_fa.md) · [Bahasa Indonesia](docs/readme/README_id.md) · [Italiano](docs/readme/README_it.md) | ||
| · [Ελληνικά](docs/readme/README_el.md) · [മലയാളം](docs/readme/README_ml.md) · [Roman Urdu](docs/readme/README_ur-Latn.md) · [Română](docs/readme/README_ro.md) | ||
| **English** |
There was a problem hiding this comment.
This removes every language link from the main README, although the translated READMEs still exist. Readers arriving at the repository can no longer choose their language from its landing page. Restore the language switcher; this removal is unrelated to adding evaluation clients.
Prompt To Fix With AI
This is a comment left during a code review.
Path: README.md
Line: 3
Comment:
**README translations lose their links**
This removes every language link from the main README, although the translated READMEs still exist. Readers arriving at the repository can no longer choose their language from its landing page. Restore the language switcher; this removal is unrelated to adding evaluation clients.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Summary
This PR implements issue #1932 by adding support for running agent evaluation against Claude Code and Gemini CLI in addition to Codex.
Problem and expected behavior
Currently
npm run verify:agentonly runs agent evaluation through Codex. The evaluator spawns Codex directly viarunCodexwith isolatedCODEX_HOMEand parses Codex JSONL events viaevaluateCodexEvents. Per-client lanes use synthetic fixtures, not real models.This PR adds support for running the same scenarios, golden claims, and gates via:
REA_AGENT_EVAL_CLIENT=claude-codeREA_AGENT_EVAL_CLIENT=gemini-cliCodex remains the default; adds
clientfield to report.Change and scope
Core Implementation
REA_AGENT_EVAL_CLIENTenvironment variable to select client (codex,claude-code,gemini-cli). Added isolated homes for each client. Usesrea setupfor each client with appropriate client identifier.runClaude()andrunGemini()functions with proper CLI arguments for stream-json output. AddedexecuteJsonLines()helper for parsing JSONL streams from all clients. AddedreadAgentVersion()for all clients. AddedlaunchGemini()helper for Windows compatibility.normalizeClaudeEvents()andnormalizeGeminiEvents()to convert client-specific event formats to the shared evaluator shape. AddednormalizeAgentEvents()dispatcher.Contract and boundary impact
scripts/verify-agent-experience.mjs) and its command layer (scripts/lib/agent-evaluation-commands.mjs)client,clientVersion,clientCommandfieldsExpected effects:
Evidence and regression coverage
normalizeClaudeEventsandnormalizeGeminiEventsinAgentTranscript.test.tsnpm run verify:agentworks for Codex; new clients can be tested withREA_AGENT_EVAL_CLIENT=claude-code npm run verify:agentclient,clientVersion,clientCommandfieldsValidation performed
npm test— passes (includes new AgentTranscript tests)Compatibility, safety, and release
Author readiness
Human owner: @saranshkhanna009
Before merge
type(scope): outcome.