Skip to content

mcp: extend test version regex to match -rc.N suffix#36142

Merged
bosconi merged 1 commit intomainfrom
bosconi/fix-mcp-rc-version-regex
Apr 19, 2026
Merged

mcp: extend test version regex to match -rc.N suffix#36142
bosconi merged 1 commit intomainfrom
bosconi/fix-mcp-rc-version-regex

Conversation

@bosconi
Copy link
Copy Markdown
Member

@bosconi bosconi commented Apr 18, 2026

Motivation

The MCP datadriven tests in src/environmentd/tests/server.rs strip version
strings to <VERSION> for stable expected output. The regex handled -dev.N
but not -rc.N:

Caught by test/121029 (v26.21.0-rc.1) on test_mcp_agent and test_mcp_developer.

Tips for reviewer

  • One-line change to the regex; behavior on main and -dev builds is unchanged.
  • After this lands on main, it should be cherry-picked onto the v26.21.0-rc.1 release branch so RC qualification can complete green. Earlier RC branches (v26.20, v26.19, v26.18) likely also need the cherry-pick if any further qualification work is planned on them.

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label. (N/A)
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR. (N/A)
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post. (N/A — test-only fix)

The MCP datadriven tests strip version strings to <VERSION> for stable
output. The regex matched -dev.N but not -rc.N, so on release-qualification
builds (e.g. v26.21.0-rc.1) the suffix leaked through, producing
<VERSION>-rc.1 and failing the snapshot.

Caught by test/121029 (v26.21.0-rc.1) on test_mcp_agent and
test_mcp_developer.
@bosconi bosconi requested a review from a team as a code owner April 18, 2026 12:20
@bosconi bosconi requested review from antiguru, def- and ohbadiah April 18, 2026 12:20
/// Helper to set up an MCP test server and run datadriven tests.
fn run_mcp_datadriven(testdata_path: &str, harness: test_util::TestHarness) {
let version_re = Regex::new(r#"\d+\.\d+\.\d+(\.\d+)?(-dev(\.\d+)?)?"#).unwrap();
let version_re = Regex::new(r#"\d+\.\d+\.\d+(\.\d+)?(-(dev|rc)(\.\d+)?)?"#).unwrap();
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 don't we use something like a proper semver library intead? Regex is brittle

@bosconi bosconi merged commit 18454dc into main Apr 19, 2026
124 checks passed
@bosconi bosconi deleted the bosconi/fix-mcp-rc-version-regex branch April 19, 2026 20:38
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.

3 participants