[danieljbruce]

Description

Adds a TypeScript public interface restrictiveness check in core/test-utils.

Impact

Detects breaking changes when public TypeScript inputs narrow or outputs expand.

Testing

• Verified pnpm --dir core/test-utils test across Node.js and Bun (4 passed, 4 total suites, 35 passed tests, >28,000 compiled consumer probes across public packages with <1% base invalid ratio).
• Verified pnpm --dir core/test-utils run check and node ./bin/linter.mjs --strict.

Note: this was opened in favour of #9825

[gemini-code-assist]

Code Review

This pull request introduces a new CI package, ci/breaking-change-check, to verify public interface restrictiveness and compilation. It updates the pnpm workspace and lockfile to include this package. Additionally, ci/run_conditional_tests.sh is updated to support a new --list-dirs option, integrate the ci directory into the conditional test execution flow (while exempting it from Windows, samples, and system tests), and refine change detection logic to ignore changes within ci/breaking-change-check and ci/run_conditional_tests.sh when determining if the entire test suite should run. There are no review comments, so I have no feedback to provide.

[danieljbruce]

#9826 was previous written, but it is a bit too verbose. Let's minimize the changes to run_conditional_tests.sh.

[danieljbruce]

[Generated] What these changes do and their impact on PR #9826:

1. Background: How ci/run_conditional_tests.sh handles changes in ci/

In normal PR presubmit runs (GIT_DIFF_ARG="HEAD^1"), ci/run_conditional_tests.sh checks each package directory d with git diff --quiet ${GIT_DIFF_ARG} ${d} so unit tests only run for packages modified by the PR. However, lines 105–117 check whether files under ci/ changed: if that git diff detects any change, the script logs "change detected in ci, we should test everything" and resets GIT_DIFF_ARG="", which causes the loop at lines 260–290 to schedule every package across the entire monorepo (~300+ packages across 5 shards × Node 22/24/26, Bun, and Windows).

2. Line-by-line explanation of the changes

1. commit_range="${GIT_DIFF_ARG%% -- *}" (line 105):
   • Extracts only the revision range before any -- pathspec separator in GIT_DIFF_ARG.
   • When ci/run_conditional_tests.sh --strict is run with pathspec exclusions already in GIT_DIFF_ARG (such as GIT_DIFF_ARG="upstream/main...HEAD -- :!packages"), passing ${GIT_DIFF_ARG} directly into git diff --quiet ${GIT_DIFF_ARG} -- ci ... would pass two -- separators to git diff (causing git diff to treat the second -- as a file path) or combine conflicting pathspecs. Stripping -- * preserves just the commit range (HEAD^1 or upstream/main...HEAD).
2. -- ci ':!ci/breaking-change-check' ':!ci/run_conditional_tests.sh' (lines 107, 114–115):
   • Adds Git negative pathspecs (:!) to exclude ci/breaking-change-check and ci/run_conditional_tests.sh from the ci/ "test-everything" check.
   • Changes to shared test runner scripts in ci/ (such as ci/run_single_test.sh or ci/export_test_telemetry.sh) still match ci and still trigger the full monorepo test suite (GIT_DIFF_ARG="").

3. Impact on PR #9826

• Prevents a full-monorepo unit test explosion on this PR (and future PRs touching ci/breaking-change-check): PR ci: add public TypeScript interface restrictiveness check #9826 adds the standalone ci/breaking-change-check/ package (which is executed by its own .github/workflows/breaking-change-check.yaml workflow and does not affect how other packages' unit tests run) and modifies ci/run_conditional_tests.sh. Without these exclusions, PR ci: add public TypeScript interface restrictiveness check #9826 itself would trigger GIT_DIFF_ARG="" in presubmit, presubmit-bun, and presubmit-windows, forcing CI to compile and run unit tests for every package in the monorepo across 5 shards instead of only testing the packages modified in the PR.
• Relationship to stacked PR ci: run ci/ package unit tests in run_conditional_tests.sh #9827: In the initial commit of PR ci: add public TypeScript interface restrictiveness check #9826 (f2c8a75143), ci/run_conditional_tests.sh also included broader changes (--list-dirs and adding ci to subdirs to run unit tests inside ci/* packages). Commit 2d67a7a63e ([Address CI errors] ci: minimize run_conditional_tests.sh changes) moved those test-runner enhancements into stacked PR ci: run ci/ package unit tests in run_conditional_tests.sh #9827 (ci: run ci/ package unit tests in run_conditional_tests.sh), leaving only this minimal 7-line guard in PR ci: add public TypeScript interface restrictiveness check #9826.

[danieljbruce]

I don't like the fact that in this PR we have to change this file just because the new project lives in the CI folder. I think a better solution would just be to create an independent breaking-change folder.

[danieljbruce]

Moved breaking-change-check out of ci/ into core/test-utils/test/breaking-change-check.test.ts (commit b3194a5c03) and reverted all changes to ci/run_conditional_tests.sh, pnpm-workspace.yaml, and pnpm-lock.yaml so ci/run_conditional_tests.sh remains completely untouched.

[danieljbruce]

Here are the options I explored in addition to the option of storing the breaking-change-check inside of ci which was rejected outright due to the changes required in run_conditional_tests.sh.

[quirogas]

Where is the --no-coverage flag coming from? (I thought it was a jest flag, but I didn't' see it in the jest documentation.

Also, if -t is the name of the test, we should ensure that it accurately captures what the test does. We need to make sure that anyone seeing failures for these tests understands the specific breaking changes we are looking for (e.g., TypeScript types, broken code, etc.).

[danieljbruce]

Good catches on both:

1. --no-coverage: Jest uses yargs under the hood, which implicitly negates any boolean flag (--coverage) via --no-<flag>, even though the Jest CLI docs only document --coverage=false. Since core/test-utils/jest.config.js doesn't enable coverage by default anyway (only "test": "jest --coverage" does), I've removed --no-coverage in 87dd8f8.
2. Test naming and failure message: Agreed — I've updated -t and the describe/it names in 87dd8f8 to "public TypeScript interface breaking-change check" / "ensures exported TypeScript types, parameter types, return types, and public symbols do not break existing consumer code against the base branch", and included the full findings table and explanation (narrowed input/parameter types, widened return/output types, added required properties, or removed exports causing consumer TypeScript code to fail compilation) directly in the assertion failure message.