Skip to content

fix(mcp): transient test failures - #1725

Open
marcossevilla wants to merge 2 commits into
mainfrom
fix/mcp-classify-deterministic-test-failures
Open

fix(mcp): transient test failures#1725
marcossevilla wants to merge 2 commits into
mainfrom
fix/mcp-classify-deterministic-test-failures

Conversation

@marcossevilla

@marcossevilla marcossevilla commented Sep 9, 2026

Copy link
Copy Markdown
Member

Status

READY

Description

Closes #1722

Fixes the status codes for transient failures in MCP.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@marcossevilla

Copy link
Copy Markdown
Member Author

@vgvbot fix CI failures

@unicoderbot

This comment was marked as outdated.

@marcossevilla
marcossevilla marked this pull request as ready for review September 10, 2026 14:13
@marcossevilla
marcossevilla requested a review from a team as a code owner September 10, 2026 14:13
@marcossevilla

Copy link
Copy Markdown
Member Author

hey @jmadren, adding you as reviewer since you opened the issue initially, lmk if it fixes your problem 👍

@marcossevilla marcossevilla self-assigned this Sep 10, 2026

final isDartInstalled = await _dartInstalled(logger: _logger);
if (!await _dartInstalled(logger: _logger)) {
return ExitCode.success.code;

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.

Should this be success?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this was the behavior already, I just moved it as a guard to reduce nesting, but we could return another exitcode like unavailable if needed, it's just not the scope of this PR

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.

I think unavailable would be more proper, we can change it as a follow up

/// Logs [error], along with its uncovered lines when it carries any, and
/// returns the exit code an unmet coverage threshold reports.
static int handleMinCoverageNotMet(
MinCoverageNotMet error, {

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.

why we moved from positional arguments here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the only positional is the error, the other ones are named parameters because one is optional and the other is not the actual error. changed it for readability but we can revert


final isFlutterInstalled = await _flutterInstalled(logger: _logger);
if (!await _flutterInstalled(logger: _logger)) {
return ExitCode.success.code;

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(mcp): test failures are classified as transient, telling agents to retry a deterministic failure

3 participants