Skip to content

test+ci: codegen regression tests (#34) + PR verify gate#47

Merged
johnxie merged 2 commits into
mainfrom
test/codegen-and-ci-gate
Jun 5, 2026
Merged

test+ci: codegen regression tests (#34) + PR verify gate#47
johnxie merged 2 commits into
mainfrom
test/codegen-and-ci-gate

Conversation

@johnxie

@johnxie johnxie commented Jun 5, 2026

Copy link
Copy Markdown
Member

What & why

The repo had zero tests and CI only published — nothing verified that a PR builds, lints, or that the generated tools are current. The normalizeAllOf fix for #34 was shipped untested.

Changes

  • vitest + root test script.
  • packages/openapi-codegen/src/normalizeAllOf.test.tsBug: taskCreate fails with -32602 error — allOf schema validation incorrectly rejects content/contentType fields #34 regression guard: a taskCreate-shaped allOf + anyOf + additionalProperties:false schema must distribute content/contentType into the strict branch (the exact bug that returned -32602). Plus plain-allOf merge + non-allOf passthrough.
  • packages/openapi-codegen/src/runtime.test.ts — path/query/body split in prepareToolCallOperation.
  • .github/workflows/ci.yml — on PRs/pushes: install → build → lint → test + git diff --exit-code tools.generated.ts (catches stale generated output).

Zero-regression

  • Purely additive (new files + workflow + dev dep). Does not touch release.yml/force-release.yml.
  • Verified locally: yarn build → no drift, yarn lint clean, 5 tests pass.

Recommended to land first

Optional follow-up

  • Add *.test.ts to an .npmignore for @taskade/mcp-openapi-codegen so tests aren't packed (out of scope here).

- Add vitest + 'test' script. New tests in packages/openapi-codegen/src:
  - normalizeAllOf.test.ts: guards the #34 fix (sibling content/contentType
    distributed into strict anyOf branches) + plain allOf merge + passthrough.
  - runtime.test.ts: path/query/body splitting in prepareToolCallOperation.
- Add .github/workflows/ci.yml: on PRs/pushes, runs install + build + lint +
  test and asserts tools.generated.ts is up to date (no stale generated output).

Previously CI only published; nothing verified a PR built or that generated
output was current. 5 tests pass; build produces no drift locally.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a basic verification pipeline (tests + CI) to prevent regressions in the OpenAPI codegen/runtime behavior and to ensure generated MCP tools remain in sync with source.

Changes:

  • Introduces Vitest at the repo root with a yarn test script.
  • Adds regression/unit tests for normalizeAllOf (#34) and prepareToolCallOperation.
  • Adds a CI workflow to run install → build → lint → test and verify generated outputs don’t drift.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
yarn.lock Adds the Vitest/Vite dependency tree required for running tests.
package.json Adds test script (vitest run) and the vitest devDependency.
packages/openapi-codegen/src/runtime.test.ts Tests request splitting logic in prepareToolCallOperation (path/query/body).
packages/openapi-codegen/src/normalizeAllOf.test.ts Adds regression coverage for #34 and additional normalizeAllOf behaviors.
.github/workflows/ci.yml New PR/push verification workflow: build/lint/test + generated file drift check.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/ci.yml Outdated
Comment on lines +25 to +29
- name: Build (regenerates MCP tools)
run: yarn build

- name: Ensure generated tools are up to date
run: git diff --exit-code packages/server/src/tools.generated.ts
Addresses Copilot review: the gate only checked tools.generated.ts, but
'yarn build' also runs lint:fix — a lint issue could be silently auto-fixed
and slip through. Assert 'git diff --exit-code' over the whole tree so both
stale generated output and any auto-fixed file fail the gate.
@changeset-bot

changeset-bot Bot commented Jun 5, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 45f1616

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@johnxie

johnxie commented Jun 5, 2026

Copy link
Copy Markdown
Member Author

✅ Validated → fixed → ready to merge

Copilot finding Verdict Action
yarn build runs lint:fix; checking only tools.generated.ts lets auto-fixed lint issues slip through VALID Clean-check now asserts the whole tree: git diff --exit-code.

QA (exact CI sequence locally):

build OK → git diff --exit-code → ✓ TREE CLEAN → lint OK → 5 tests pass

Confidence: high · zero regression (additive workflow + tests). Merging.

@johnxie johnxie merged commit 3d0c141 into main Jun 5, 2026
1 check passed
@johnxie johnxie deleted the test/codegen-and-ci-gate branch June 5, 2026 12:02
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.

2 participants