Skip to content

fix(ci): deselect non-MCP tests from mcp-compat workflow - #275

Merged
Oaklight merged 3 commits into
masterfrom
worktree-fix+mcp-compat-deselect
Sep 24, 2026
Merged

Oaklight merged 3 commits into
masterfrom
worktree-fix+mcp-compat-deselect

Conversation

@Oaklight

@Oaklight Oaklight commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

Summary

Test plan

  • CI passes on this PR
  • Re-run mcp-compat.yml after merge — both legs should pass

TestPTCInBatch requires codecell (ptc extra) and
TestOpenAPIExecutionStack requires openapi deps — neither are
MCP-related so deselect them from the MCP compat run.
- Use fixed issue titles (`[CI] MCP SDK v1.x/v2.x compatibility failure`)
  and exact title match for dedup — eliminates collision risk from title
  edits (review non-blocking #1)
- Tolerate pip install failure gracefully when a matrix leg's SDK line
  doesn't exist on PyPI yet (review non-blocking #2)
- Gate test/upload/issue steps on install-mcp skip output

@elena-oaklight elena-oaklight 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.

LGTM 👍

Addresses both non-blocking flags from #272 review:

  1. Dedup fix: Fixed titles ([CI] MCP SDK v${line}.x compatibility failure) + exact match (=== title) eliminates the manual-edit collision risk

  2. PyPI graceful handling: pip install failure now sets skip=true with a warning instead of failing the job — handles missing SDK versions cleanly

  3. Deselects: --deselect for TestPTCInBatch and TestOpenAPIExecutionStack is the right fix — those tests need codecell/openapi deps unrelated to MCP compat

  4. Condition gating: Downstream steps properly check both skip outputs

Clean follow-up. 🚀

@milo-oaklight milo-oaklight 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.

LGTM — addresses both non-blocking flags from #272 review.

Changes look good:

  • --deselect cleanly excludes tests needing deps not installed in this workflow
  • Fixed title + exact match for reliable dedup
  • Graceful skip on pip failure

Minor nit (non-blocking): Line 75's 2>&1 is unnecessary — the warning message already explains the situation, and dropping the redirect lets pip's actual error remain visible in the log:

if ! pip install "${{ steps.resolve.outputs.mcp_spec }}"; then

@clementine-oaklight clementine-oaklight 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.

Addresses both flags from #272 cleanly:

  1. Dedup fix — fixed title [CI] MCP SDK v${line}.x compatibility failure + exact match (i.title === title) eliminates the substring collision risk.

  2. PyPI graceful handling — pip install failure now emits a warning and skips the leg instead of failing the job. Explicit skip=false on success ensures the conditional chain works.

  3. Test deselection — --deselect for TestPTCInBatch and TestOpenAPIExecutionStack is the right fix since those require codecell/OpenAPI deps not in this workflow's scope.

All step conditions properly gate on both resolve.outputs.skip and install-mcp.outputs.skip.

LGTM — approve once CI is green.

@Oaklight
Oaklight merged commit 3f2ff32 into master Sep 24, 2026
3 checks passed
@Oaklight
Oaklight deleted the worktree-fix+mcp-compat-deselect branch September 24, 2026 04:27
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.

1 participant