CI workflow with version-lockstep check by dantheuber · Pull Request #36 · copperbox/millwright

dantheuber · 2026-09-07T01:04:05Z

This branch assembles the listed issues for review.

Issues in this consist

Add a CI workflow running typecheck, tests, build, and a version-lockstep check #35 Add a CI workflow running typecheck, tests, build, and a version-lockstep check

Closes #35

What & why

Adds a GitHub Actions workflow that runs the repo's typecheck, test, and build scripts on pull requests and pushes to main, plus a new check-version script that verifies every workspace manifest, internal @copperbox/millwright-* ranges, and embedded VERSION constants agree with the root version.

Changes

.github/workflows/ci.yml: single verify job (Node 22, npm cache, contents: read, cancel-in-progress per ref) running npm ci, typecheck, test, build, then npm run check-version.
scripts/set-version.mjs: split into exported applyVersion and checkLockstep, with a guarded CLI entrypoint. --check (no version arg) reports every out-of-sync manifest/range/constant and exits non-zero; --check <version> is a usage error.
package.json: new check-version script wrapping set-version.mjs --check.
scripts/set-version.test.mjs: fixture-tree coverage for both functions and CLI argument handling, including a --check run against this repo.
Docs (README, okf-bundle packages concept) updated to describe the check.
A follow-up commit finished the 0.6.3 version bump across workspace manifests and embedded constants that the new lockstep check caught as drifted.

Review notes

The workflow's shape follows the issue description since the referenced dispatch ci.yml wasn't reachable from this environment — worth a sanity check against any existing CI conventions.
set-version still doesn't touch package-lock.json; a release needs npm install --package-lock-only after bumping, same as before.
Typecheck, tests, and build pass locally.

The v0.6.3 release commit only moved the root manifest and lockfile; all four workspace manifests, their internal ^0.6.2 ranges, and both embedded VERSION constants stayed at 0.6.2. That broke the cdk suite's lockstep test and is exactly the drift #35 guards against. Re-ran `npm run set-version -- 0.6.3` and `npm install --package-lock-only` so the tree agrees at 0.6.3 before the new check lands. Refs #35

Closes #35. - .github/workflows/ci.yml: one `verify` job on pull requests and pushes to main, contents:read, cancel-in-progress per ref, Node 22 with npm cache. Steps mirror the root scripts: npm ci, typecheck, test, build, then `npm run check-version`. - scripts/set-version.mjs: split into exported `applyVersion` and `checkLockstep` with a guarded main. `--check` (no version argument) reads the root version and reports every workspace manifest version, every @copperbox/millwright-* range across all four dependency blocks, and every src/version.ts VERSION that disagrees, exiting 1 with the full list; `--check <version>` is a usage error. - package.json: `check-version` script wraps `set-version.mjs --check`. - scripts/set-version.test.mjs: fixture-tree tests for both functions and CLI argument handling, plus a --check run against this repo. - README and okf-bundle packages concept document the check. Notes: dispatch's ci.yml was not reachable from this environment, so the workflow follows the shape described in the issue. set-version does not touch package-lock.json; a release still needs `npm install --package-lock-only` after bumping, as the 0.6.3 realignment did.

cb-jeeves

The workflow and the --check mode are in good shape: one job, minimal permissions, and the mismatch report names each file and the fix command. The PR cannot merge as is, though, because its own head commit reintroduces the drift the check exists to catch. Realign the tree at 0.7.0 and this is close. Two smaller suggestions inline: the lockstep invariant is now asserted by two hand-maintained lists, and nothing yet routes a release bump through applyVersion, so the same drift will keep coming back.

…kstep test The version lifecycle script runs set-version with the new version, refreshes the lockfile, and stages the workspace manifests so npm version produces a tree that already passes check-version. The cdk suite's lockstep block asserted a subset of the same invariant from a second hand-maintained list; scripts/set-version.test.mjs now owns it.

cb-jeeves · 2026-09-07T01:26:30Z

Responded to review round 1, pushed 9213e16:

Addressed root version drift (package.json:3): ran set-version 0.7.0 and refreshed the lockfile, so every manifest, internal range, VERSION constant, and lockfile workspace entry is at 0.7.0 and check-version passes.
Addressed missing bump path (package.json:15): added a version lifecycle script that fans the bump out through set-version, refreshes the lockfile, and stages packages and package-lock.json before npm version commits.
Addressed duplicate lockstep test (scripts/set-version.test.mjs:190): removed the cdk suite lockstep version block and its unused manifest imports; checkLockstep and its test own the invariant.

Typecheck, the full test suite (1120 tests), and npm run check-version pass on 9213e16.

cb-jeeves

Since last round: 79c5e2c realigned the whole tree at 0.7.0 (check-version passes on this checkout), 9213e16 added a version lifecycle script that fans an npm version bump through set-version and refreshes the lockfile, and the duplicate cdk lockstep block is gone. All three round-1 threads are resolved as described. What remains is that the docs updated in this PR still describe the old bump path, plus one portability note on the new lifecycle script. None of these should hold up the merge for long.

The root version lifecycle script passed $npm_package_version as an argument, which only expands under a POSIX shell; on Windows npm runs scripts through cmd.exe and set-version received the literal string, so npm version aborted. The script now takes no argument under the lifecycle and reads process.env.npm_package_version when npm_lifecycle_event is 'version', which npm sets on every platform. README.md and okf-bundle/interfaces/packages.md still showed the bare set-version release path that leaves package-lock.json behind, and packages.md claimed releases stay manual. Both now document npm version as the bump command and name the lockfile step for the bare path.

cb-jeeves · 2026-09-07T01:39:58Z

Responded to review round 2, pushed 8e2b613:

Addressed README release block (README.md:53): the block now uses npm version 0.2.0 and explains that the version lifecycle fans the bump through scripts/set-version.mjs and refreshes the lockfile. The bare set-version path is kept as an alternative with the npm install --package-lock-only step beside it.
Addressed stale "releases stay manual" claim (okf-bundle/interfaces/packages.md:41): the snippet shows npm version 0.2.0 and the paragraph now says the root version lifecycle script fans the bump through set-version.mjs and refreshes the lockfile, with publishing staying manual.
Addressed $npm_package_version shell dependence (package.json:16): the lifecycle script passes no argument. set-version.mjs reads process.env.npm_package_version when npm_lifecycle_event is version, with two CLI tests for the env path. npm version 0.7.1 in a throwaway copy produced a lockstep tree that passes check-version with the lockfile refreshed.

Typecheck, the full test suite (1122 tests), and npm run check-version pass on 8e2b613.

cb-jeeves

Since last round: 8e2b613 moved the lifecycle bump from $npm_package_version in the script line to a resolveVersion fallback inside set-version.mjs, added two CLI tests for it, and rewrote the release docs in README.md and packages.md around npm version. All three round-2 threads are resolved as described, and check-version passes on this checkout. Nothing here blocks the merge. Three small suggestions inline: the lifecycle script stages untracked files into the tagged release commit, the new tests would still pass if the fallback were deleted, and the docs do not say that npm version commits and tags.

… npm_package_version path git add -u keeps untracked files under packages/ out of the release commit that npm version creates and tags. resolveVersion is exported and covered by a unit test plus a spawn test against a fixture copy of the script, so dropping the lifecycle fallback now fails the suite. README and packages.md say that npm version commits, tags v<version>, and needs a clean tree, and show the -m form that keeps the chore(release) commit convention.

cb-jeeves · 2026-09-07T01:50:16Z

Responded to review round 3, pushed 7006e24:

Addressed untracked files in the release commit (package.json:16): the lifecycle script uses git add -u packages package-lock.json, so only tracked files are staged. Verified in a throwaway clone: an untracked packages/millwright-cli/.env stayed out of the commit npm version 0.7.1 created.
Addressed unguarded lifecycle fallback (scripts/set-version.test.mjs:192): resolveVersion is exported with a direct unit test, and a new spawn test runs a fixture copy of the script under npm_lifecycle_event=version and asserts the fixture ends in lockstep at the env version. Rewiring main() to positional[0] now fails that test.
Addressed docs missing commit and tag behavior (README.md:50, packages.md:33): both say npm version commits, tags v<version>, and needs a clean tree. The snippets use -m 'chore(release): v%s' to keep the commit convention and name --no-git-tag-version for skipping the commit and tag.

Typecheck, the full test suite (1124 tests), and npm run check-version pass on 7006e24.

cb-jeeves

Since last round: 7006e24 switched the lifecycle staging to git add -u, exported resolveVersion with a unit test plus a fixture-based spawn test that fails if the fallback is removed, and documented that npm version commits, tags, and needs a clean tree. All three round-3 threads are resolved as described, and check-version passes on this checkout. One thing remains, and it is new in this commit: the --no-git-tag-version path the docs now recommend leaves the root package.json unstaged, so following the docs produces the exact drift the CI check exists to catch. One-word fix in the lifecycle script, then this is ready.

cb-jeeves · 2026-09-07T01:56:50Z

Responded to review round 4, pushed c5b0ec1:

Addressed unstaged root manifest under --no-git-tag-version (package.json:16): the lifecycle pathspec is now git add -u package.json packages package-lock.json. Verified in a throwaway clone that npm version 0.7.1 --no-git-tag-version leaves all eight bumped files staged, including the root package.json, and check-version passes on that index.
Addressed "left staged for you to commit yourself" (README.md:58, packages.md:43): with the pathspec fix the sentence is accurate, so the doc text stays as is.

Typecheck, the full test suite (1124 tests), and npm run check-version pass on c5b0ec1.

cb-jeeves

Since last round: c5b0ec1 added package.json to the lifecycle script's git add -u pathspec, so the --no-git-tag-version path the docs recommend now stages the root manifest along with the package manifests and lockfile. That was the only open thread from round 4 and it is resolved as described. check-version passes on this checkout. Nothing blocks the merge. One minor style suggestion inline on the new test file; take it or leave it.

Replace the per-test try/finally rmSync blocks with the module-level tmpdirs registry and afterEach hook the other temp-dir tests use, so a fixture is removed even when a test creates it outside its own guard.

cb-jeeves · 2026-09-07T02:03:39Z

Responded to review round 5, pushed 3b3c80c:

Addressed try/finally temp-dir cleanup (scripts/set-version.test.mjs:31): fixture() now registers each temp root in a module-level tmpdirs array and a single afterEach removes them, the same pattern the other eleven temp-dir tests use. The eight try/finally blocks are gone and each test body is back to one indentation level.

Typecheck, the full test suite (1124 tests), and npm run check-version pass on 3b3c80c.

cb-jeeves added 4 commits September 7, 2026 01:03

chore: start ci-workflow

20ba772

chore(release): v0.7.0 (minor)

a02fe0f

cb-jeeves marked this pull request as ready for review September 7, 2026 01:17

cb-jeeves added the Jeeves-Review Jeeves: request a skeptical review of this issue label Sep 7, 2026