Skip to content

fix(server): classify successful MCP results correctly - #8030

Open
SamarthaB10 wants to merge 2 commits into
pingdotgg:mainfrom
SamarthaB10:codex/fix-successful-mcp-results
Open

fix(server): classify successful MCP results correctly#8030
SamarthaB10 wants to merge 2 commits into
pingdotgg:mainfrom
SamarthaB10:codex/fix-successful-mcp-results

Conversation

@SamarthaB10

@SamarthaB10 SamarthaB10 commented Aug 23, 2026

Copy link
Copy Markdown

What Changed

  • Treat a Codex MCP tool call as completed when it includes a non-null result and no error, even if the upstream status is failed.
  • Keep genuine failed and declined item statuses unchanged.
  • Refactor lifecycle status selection into a named helper for readability.
  • Add regression coverage for inspectTaskChanges returning a result with an incorrect failed status.

Why

Codex can report an MCP tool call with status: "failed" while also returning a valid result and no error. T3 Code displayed these calls as failed even though the agent received and used the result. This change uses the result and error fields to classify the call correctly.

UI Changes

Not applicable. This is a server/provider lifecycle classification fix. The visible effect is limited to the activity status shown for affected tool calls.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Verification

  • git diff --check passed.
  • The focused test could not run locally because vp is not installed and workspace dependencies are not present.

Note

Low Risk
Narrow status-classification fix for MCP tool-call lifecycle events; genuine failed/declined items are unchanged.

Overview
Treats Codex MCP tool calls as completed when they have a non-null result and no error, even if Codex reports status: "failed". Genuine failed and declined items are unchanged.

CodexAdapter now uses itemLifecycleStatus / hasSuccessfulToolResult for that override. projectActivityPayload no longer copies the nested failed status onto a successful MCP result, so the UI does not flip those calls back to failed.

Reviewed by Cursor Bugbot for commit da84927. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix mcpToolCall items with successful results being classified as failed or declined

  • When an mcpToolCall item has a non-null result and null error, it is now treated as successful regardless of the item's status field
  • Adds hasSuccessfulMcpResult helper in ActivityPayloadProjection.ts to prevent downgrading top-level completed status
  • Adds hasSuccessfulToolResult and itemLifecycleStatus helpers in CodexAdapter.ts so mapItemLifecycle emits completed for successful MCP tool calls even when source status is failed or declined
  • Risk: any consumer relying on the nested status of failed/declined for MCP calls that actually have results will now see completed instead

Macroscope summarized da84927.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 14ad8885-6418-4073-aab3-4abfeb89c72c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 23, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 14fa517. Configure here.

Comment thread apps/server/src/provider/Layers/CodexAdapter.ts
@macroscopeapp

macroscopeapp Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at da84927

Macroscope's review found this PR approvable — This is a narrow server-side status-classification fix with targeted tests. It only changes affected MCP activity rows from misleading failure states to completed states while preserving genuine failures and declined outcomes.

You can add or adjust custom eligibility rules. Learn more.

@SamarthaB10

Copy link
Copy Markdown
Author

Addressed the Cursor Bugbot finding in commit da849275c.

The adapter correctly classified the MCP call, but projectActivityPayload could later restore data.item.status: "failed" onto the top-level payload during HTTP/WebSocket projection. The projection now preserves completed when the MCP item has a non-null result and no error.

Added a regression test covering the final projected payload status. git diff --check passes. The focused tests remain blocked locally because vp is not installed and workspace dependencies are not present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant