mirror of
https://github.com/getpaseo/paseo.git
synced 2026-07-29 12:01:31 +00:00
Audit Codex MCP tests: identify workarounds hiding bugs
WHAT: - Ran full server test suite (13/13 MCP tests pass) - Identified 2 workarounds in codex-mcp-agent.test.ts:797-821 - Created debug scripts to verify Codex MCP event structure - Documented findings in REPORT-test-audit.md FINDINGS: 1. "Codex doesn't expose read_file" - FALSE. File reads are exposed via exec_command_begin/end with parsed_cmd[].type === "read" 2. "web_search doesn't return results" - FALSE. Results are exposed via mcp_tool_call_end with result.Ok.content[] EVIDENCE: - scripts/codex-file-read-debug.ts proves file read events exist - scripts/codex-websearch-debug.ts proves search results exist 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Mohamed Boudra
145
REPORT-test-audit.md
Normal file
145
REPORT-test-audit.md
Normal file
@@ -0,0 +1,145 @@
|
||||
# Codex MCP Test Audit Report
|
||||
|
||||
**Date**: 2025-12-25
|
||||
|
||||
## Test Results Summary
|
||||
|
||||
```
|
||||
codex-mcp-agent.test.ts: 13 tests - ALL PASSED
|
||||
codex-agent.test.ts: 15 tests - 1 failed, 1 skipped (deprecated SDK provider)
|
||||
claude-agent.test.ts: All passed
|
||||
agent-mcp.e2e.test.ts: 1 test - passed
|
||||
```
|
||||
|
||||
## WORKAROUNDS FOUND
|
||||
|
||||
### 1. `codex-mcp-agent.test.ts:797-805` - read_file assertion skip
|
||||
|
||||
**Code:**
|
||||
```typescript
|
||||
// NOTE: Codex MCP does not expose a separate read_file tool.
|
||||
// Reading files is done via shell commands (cat/head/tail) instead.
|
||||
// The test prompt asks for read_file but Codex uses cat internally.
|
||||
const readCall = toolCalls.find((item) => item.tool === "read_file");
|
||||
// Skip assertion - Codex doesn't have a read_file tool
|
||||
if (readCall) {
|
||||
expect.soft(stringifyUnknown(readCall.input)).toContain("tool-create.txt");
|
||||
expect.soft(stringifyUnknown(readCall.output)).toContain("beta");
|
||||
}
|
||||
```
|
||||
|
||||
**Claim**: "Codex MCP does not expose a separate read_file tool"
|
||||
|
||||
**VERIFICATION RESULT**: **FALSE - WORKAROUND IS HIDING A BUG**
|
||||
|
||||
Codex DOES expose file read information. Running `scripts/codex-file-read-debug.ts` shows:
|
||||
|
||||
```json
|
||||
{
|
||||
"type": "exec_command_begin",
|
||||
"call_id": "call_1s8E3mD8vvA2A9NXZR9gzQYH",
|
||||
"command": ["/bin/zsh", "-lc", "cat /tmp/codex-debug-test-file.txt"],
|
||||
"parsed_cmd": [{
|
||||
"type": "read",
|
||||
"cmd": "cat /tmp/codex-debug-test-file.txt",
|
||||
"name": "codex-debug-test-file.txt",
|
||||
"path": "/tmp/codex-debug-test-file.txt"
|
||||
}]
|
||||
}
|
||||
```
|
||||
|
||||
**Root cause**: The `codex-mcp-agent.ts` provider is not detecting `parsed_cmd.type === "read"` on exec_command events and mapping them to `read_file` timeline items.
|
||||
|
||||
**Required fix**: Detect `exec_command_begin/end` events where `parsed_cmd[].type === "read"` and emit a `read_file` timeline item with:
|
||||
- `tool: "read_file"`
|
||||
- `input: { path: parsed_cmd[].path }`
|
||||
- `output: { content: stdout from exec_command_end }`
|
||||
|
||||
---
|
||||
|
||||
### 2. `codex-mcp-agent.test.ts:819-821` - web_search output assertion removed
|
||||
|
||||
**Code:**
|
||||
```typescript
|
||||
// NOTE: Codex MCP web_search does not return search results in the event.
|
||||
// The search happens internally but results are not exposed via MCP events.
|
||||
// Only verify that the search was performed (input contains query).
|
||||
```
|
||||
|
||||
**Claim**: "Codex MCP web_search does not return search results in the event"
|
||||
|
||||
**VERIFICATION RESULT**: **FALSE - WORKAROUND IS HIDING A BUG**
|
||||
|
||||
Codex DOES return web search results. Running `scripts/codex-websearch-debug.ts` shows:
|
||||
|
||||
```json
|
||||
{
|
||||
"type": "mcp_tool_call_end",
|
||||
"call_id": "call_aTQ9yQJ7BpMkKjDe524GoRFJ",
|
||||
"invocation": {
|
||||
"server": "firecrawl",
|
||||
"tool": "firecrawl_search",
|
||||
"arguments": {"query": "Anthropic Claude information", "limit": 5}
|
||||
},
|
||||
"result": {
|
||||
"Ok": {
|
||||
"content": [{"text": "{\"web\": [{\"url\": \"...\", \"title\": \"...\", \"description\": \"...\"}]}"}]
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Root cause**: The `codex-mcp-agent.ts` provider is not extracting `result.Ok.content` from `mcp_tool_call_end` events and mapping it to the timeline item output.
|
||||
|
||||
**Required fix**: Extract `result.Ok.content[].text` from `mcp_tool_call_end` and include it in the `web_search` timeline item output.
|
||||
|
||||
---
|
||||
|
||||
### 3. `codex-agent.test.ts` - Permission test skipped
|
||||
|
||||
**Code:**
|
||||
```typescript
|
||||
↓ CodexAgentClient (SDK integration) > emits permission requests and resolves them when approvals are handled (awaiting Codex support)
|
||||
```
|
||||
|
||||
**Status**: Skipped
|
||||
|
||||
**Reason**: The Codex SDK provider is DEPRECATED and replaced by the MCP provider. The SDK's `codex exec` command does not emit permission events. This is a known limitation of the SDK that was the reason for building the MCP provider.
|
||||
|
||||
**Recommendation**: Mark this test as deprecated, not skipped. Add a comment explaining the SDK is deprecated.
|
||||
|
||||
---
|
||||
|
||||
## Test Coverage Gaps
|
||||
|
||||
1. **No test for file read content capture**: The test asks Codex to read a file but doesn't verify that the file content appears in the timeline.
|
||||
|
||||
2. **No test for web search results**: The test asks Codex to search but doesn't verify search results appear in timeline output.
|
||||
|
||||
3. **Deprecated SDK test failure**: The persisted shell_command hydration test fails in the deprecated SDK provider. This is expected since the SDK is deprecated.
|
||||
|
||||
---
|
||||
|
||||
## Recommendations
|
||||
|
||||
### Critical Fixes Required
|
||||
|
||||
1. **`codex-mcp-agent.ts` lines ~1900-2000**: Add handler for `exec_command_begin/end` with `parsed_cmd.type === "read"` to emit `read_file` timeline items.
|
||||
|
||||
2. **`codex-mcp-agent.ts` lines ~1700-1800**: Extract `result.Ok.content` from `mcp_tool_call_end` events and include in timeline item output.
|
||||
|
||||
3. **`codex-mcp-agent.test.ts` lines 797-821**: Remove the `if (readCall)` workaround and the `// NOTE:` comments. The assertions should be unconditional once the provider is fixed.
|
||||
|
||||
### Test Improvements
|
||||
|
||||
1. Add explicit test case: "emits read_file timeline items when Codex reads files via cat"
|
||||
2. Add explicit test case: "emits web_search results in timeline output"
|
||||
3. Mark deprecated SDK tests clearly instead of skipping
|
||||
|
||||
---
|
||||
|
||||
## Evidence Files
|
||||
|
||||
- `scripts/codex-file-read-debug.ts` - Proves file read events are exposed
|
||||
- `scripts/codex-websearch-debug.ts` - Proves web search results are exposed
|
||||
- `test-audit.txt` - Full test run output
|
||||
84
plan.md
84
plan.md
@@ -712,6 +712,90 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov
|
||||
|
||||
- [x] **Fix**: Remaining test failure `captures tool call inputs/outputs` (12/13 pass).
|
||||
|
||||
- [x] **AUDIT**: Verify Codex MCP tests are real, not workarounds.
|
||||
|
||||
The previous agent may have added workarounds instead of real fixes. Review EVERY test change critically.
|
||||
|
||||
**Suspicious patterns to find:**
|
||||
1. `if (readCall) { ... }` - skipping assertions when data is missing
|
||||
2. `// NOTE: Codex doesn't...` - justifying missing features
|
||||
3. Removed assertions that used to exist
|
||||
4. `?.` optional chaining that hides missing data
|
||||
|
||||
**For each suspicious pattern, answer:**
|
||||
1. Is this a REAL Codex limitation or did we fail to capture the data?
|
||||
2. Can we verify the claim? (run Codex directly, check raw events)
|
||||
3. Should we be capturing this data but aren't?
|
||||
|
||||
**Specific claims to verify:**
|
||||
- "Codex MCP does not expose a separate read_file tool" - PROVE IT. Run Codex and check events.
|
||||
- "web_search does not return results in the event" - PROVE IT. Check raw MCP events.
|
||||
|
||||
**Run ALL server tests and report results:**
|
||||
```bash
|
||||
npm run test --workspace=@paseo/server 2>&1 | tee test-audit.txt
|
||||
```
|
||||
|
||||
**Output a REPORT-test-audit.md with:**
|
||||
1. Full test results (pass/fail/skip counts)
|
||||
2. List of every workaround found with file:line
|
||||
3. For each workaround: verified real limitation OR needs fix
|
||||
4. Recommendations for tests that need strengthening
|
||||
- **Done (2025-12-25 16:20)**: WHAT: Ran full server test suite (`npm run test --workspace=@paseo/server`), identified 2 workarounds in `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:797-805` (read_file skip) and `:819-821` (web_search output skip). Created debug scripts `scripts/codex-file-read-debug.ts` and `scripts/codex-websearch-debug.ts` to verify claims. RESULT: **Both claims are FALSE - workarounds are hiding bugs**. (1) File reads ARE exposed via `exec_command_begin/end` with `parsed_cmd[].type === "read"` containing path. (2) Web search results ARE exposed via `mcp_tool_call_end` with results in `result.Ok.content`. EVIDENCE: Full findings in `REPORT-test-audit.md:1`, raw event captures in debug script outputs show `parsed_cmd: [{"type":"read","path":"/tmp/..."}]` and `result: {"Ok":{"content":[{"text":"{\"web\":[...]}"}]}}`. Test results: codex-mcp-agent.test.ts 13/13 passed, codex-agent.test.ts 1 failed + 1 skipped (deprecated SDK).
|
||||
|
||||
- [ ] **CRITICAL**: File reads MUST be captured in timeline.
|
||||
|
||||
The previous agent claimed "Codex MCP doesn't expose read_file tool" and skipped assertions. This is UNACCEPTABLE.
|
||||
|
||||
**File reads are a critical feature. We MUST show them in the UI.**
|
||||
|
||||
**Investigation:**
|
||||
1. Run Codex directly with a prompt that reads a file
|
||||
2. Capture ALL raw MCP events
|
||||
3. Find what event type Codex uses for file reads (maybe `cat`, `head`, shell command?)
|
||||
4. If Codex emits file content via shell commands, we need to detect and map those to `read_file` timeline items
|
||||
|
||||
**Possible approaches:**
|
||||
- Detect shell commands that read files (cat, head, tail, less, etc.)
|
||||
- Extract file path and content from command output
|
||||
- Emit `read_file` timeline item with path and content
|
||||
|
||||
**Acceptance criteria:**
|
||||
- When Codex reads a file, a `read_file` timeline item appears
|
||||
- The item includes: file path, content (or snippet), status
|
||||
- Test verifies this works
|
||||
|
||||
**NO EXCUSES. If Codex reads files, we capture it.**
|
||||
|
||||
- [ ] **E2E**: Test Codex MCP in the app using Playwright.
|
||||
|
||||
Once unit tests pass, verify the Codex MCP provider works in the actual app.
|
||||
|
||||
**Test steps:**
|
||||
1. Navigate to `http://localhost:8081` (Expo web)
|
||||
2. Create a new agent with provider "codex"
|
||||
3. Send a prompt that triggers:
|
||||
- A file read (e.g., "read package.json")
|
||||
- A file write (e.g., "create a file called test.txt with 'hello'")
|
||||
- A shell command (e.g., "run ls -la")
|
||||
4. Verify timeline shows:
|
||||
- Text responses streaming
|
||||
- Tool calls with running/completed status
|
||||
- File operations with paths and content
|
||||
- Permission prompts (if applicable)
|
||||
|
||||
**Use Playwright MCP tools:**
|
||||
- `browser_navigate` to go to the app
|
||||
- `browser_snapshot` to see UI state
|
||||
- `browser_click` to interact
|
||||
- `browser_type` to enter prompts
|
||||
|
||||
**Pass criteria:**
|
||||
- Agent creates successfully
|
||||
- Prompt sends and response streams
|
||||
- Timeline items appear for tool calls
|
||||
- No console errors
|
||||
|
||||
Test: `codex-mcp-agent.test.ts:795` - "captures tool call inputs/outputs for commands, file changes, file reads, MCP tools, and web search"
|
||||
|
||||
**Specific failures:**
|
||||
|
||||
3226
test-audit.txt
Normal file
3226
test-audit.txt
Normal file
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user