Repository navigation
Update Zod, Vitest and esbuild to patched versions - #1575
Conversation
Preserve nested defaults, records and CLI validation messages under Zod 4. Update Vitest, coverage and esbuild together, and retain test isolation with explicit mock cleanup.
|
No confident findings in the specified PR diff. The PR description documents sufficient testing, including a build and five passing representative real-CLI E2E tests for Review was read-only. I did not run, build, or install anything from the PR. |
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
@HengYangDS Thanks, this is a careful migration. I compared the base and head builds on what users already have on disk: 797 schema/input pairs, including config.yaml, teamai.yaml, state.json and step states written by teamai-cli@0.28.1. None is accepted or rejected differently, and no parsed value or default changes; only Zod's own error wording does. A run state written by 0.28.1 then driven through pull, status, doctor, workflow retry/start and a resume prints the same output on base and head. The 13 workflow flag refusals and the bad-scope message keep their text. No z.record uses enum keys. npm ci, the build and --help work, there is no new runtime dependency, and npm audit goes from 6 findings (one critical, tinypool) to 0.
Small things, all fixable here:
- [P2 non-blocking] The #1431 launcher e2e stops testing its runtime names on macOS and Linux.
installed-launcher-readiness.test.ts:31-33now symlinksprocess.execPathinstead of copying it. Through a symlink Node reports the resolved binary, so bothnode-22andnodejsrecord…/bin/node, and the test passes whether or notdoctoraccepts those names. Keeping the copy and skipping only when it cannot start covers the Homebrew case without losing the check:fs.copyFileSync(process.execPath, runtime); // A Homebrew-style runtime loads libnode beside its real path, so a copy cannot start there. if (spawnSync(runtime, ['--version']).status !== 0) return;
- [P3 nit] A scorer whose
labelsis not a list prints its diagnostic twice. Zod 4 keeps running.min(2)after the type error, and both issues carry the same text (src/scorers/definition.ts:31). Withlabels: x,pullanddoctoreach print the line twice. Deduplicating inparseScorerfixes it:[...new Set(parsed.error.issues.flatMap(issueMessages))]. - [P3 nit] A judge request with
tools: []now saystools.0: Invalid input: expected string, received undefinedinstead of "at least 1 element". Only driver authors see it, but the old wording was clearer.
Heads-up for other open PRs: #1561 will need a small rebase in src/session-flow/captures.ts, and PRs written against Zod 3 (single-argument z.record(...), errorMap, required_error) will stop type-checking once this lands.
Lint & Test on macOS and Node 20 ubuntu and two E2E shards were still running when I checked.
@jeff-r2026 Your call; nothing blocking from my side once CI is green.
|
Fixed in The launcher fixtures copy Node again. Both The 112 scorer/workflow/cache tests and five migration compatibility tests pass. Typecheck, lint, build and the real CLI error cases also pass. The complete hosted matrix on this head now has 17 successful checks and one skipped GitHub-provider fixture. CI run. |
|
No confident findings in the specified PR diff. The PR description documents sufficient testing, including a build and five passing representative real-CLI E2E tests for The earlier review raised no findings, so none require resolution. Review was read-only. I did not run, build, or install anything from the PR. |
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Approving at fa3a2da5. The three points from my last review are fixed: the launcher e2e copies Node again and skips only when the copy cannot start, a scorer's duplicate diagnostic is printed once, and an empty judge tools list keeps its old message. CI is green, and files written by 0.28.1 parse the same as before.
@HengYangDS Thanks for the careful migration.
Summary
Upgrade Zod to 4.6.7, Vitest and its coverage provider to 4.1.12, and esbuild to 0.28.2. The resulting lockfile has no findings in
npm audit.Adapt record keys, error options and nested defaults to Zod 4 while preserving stored configuration and workflow behavior. Update Vitest mock types and constructors, replace the removed vite-node fixture runner with an esbuild bundle, and keep CLI fixtures independent of installed agent versions and host Node library paths.
Preserve copied-Node launcher fixtures, report each scorer diagnostic once, and retain the nonempty-tool diagnostic for judge requests.
Type of Change
Test Plan
Current follow-up
fa3a2da:npm run typecheck -- --pretty false,npm run lintandnpm run build.npm test -- src/__tests__/scorers-definition.test.ts src/__tests__/workflow-scoring.test.ts src/__tests__/scorers-cache.test.ts --reporter=dot: 112 passed.npm test -- src/__tests__/zod-schema-compatibility.test.ts --reporter=dot: five passed.npm run test:e2e -- src/__tests__/e2e/installed-launcher-readiness.test.ts src/__tests__/e2e/score-session.test.ts: five passed, two skipped with Homebrew Node 26. The real CLI reports malformed scorer labels once and explains an empty judge tool list. Existing session scoring and local bare-repository publication also pass.mise exec node@22.23.3 -- npm run test:e2e -- src/__tests__/e2e/installed-launcher-readiness.test.ts: bothnode-22andnodejspassed. A copied runtime that cannot start on its host is reported as skipped.The preceding integration
e992024passed the full unit suite: 19,017 tests passed and 24 skipped across 629 passing files and one skipped file. It also passed the five real CLI cases inagy-supportandshare-learnings-1388, with zero findings innpm audit. Manifest and lockfile bytes are unchanged by this follow-up.Related Issues
Follows #1450, now merged. The Git and TOML changes landed there.
Notes for Reviewers
The complete hosted matrix for
fa3a2dafinished with 17 successful checks and one skipped GitHub-provider fixture. Node 20/22/24 on Linux and macOS, Windows, the fork-safe E2E collection and the build passed. CI run.The pre-rebase candidate
80b17d5completed the full E2E collection; that earlier result belongs to that commit. Provider-credential fixtures are outside this fork-safe CI result.