fix: pin the verification harness install (v0.6.1) - #20
Conversation
Install @playwright/test, typescript and @types/node at the exact versions the skill is tested with, matching the Workbench's Playwright, and run the local binary instead of npx so verification never downloads an unreviewed package. A test fails when the documented install is unpinned or drifts. Closes #18
TueJon
left a comment
TueJon
left a comment
There was a problem hiding this comment.
CHANGELOG.md:13 — The new absolute claim that verification "never downloads an unreviewed package" is stronger than the install recipe. npm i creates a fresh dependency graph on every harness setup; for example, the pinned @types/node@22.20.1 still declares undici-types: ~6.21.0, so a future 6.21.x can be selected without being the version exercised by this repository's lockfile. This is a pre-existing transitive-reproducibility gap rather than a regression in the direct-pin fix, so I consider it non-blocking, but the release claim and the test name currently promise more than the test at tests/harness-pin.test.mjs:7-22 establishes.
Closure checklist:
- Either ship an audited harness lockfile (or an equivalent full-graph integrity pin) and install from it, or narrow the changelog/test wording to exact top-level package pins and prevention of
npxfallback. - If full-graph reproducibility is intended, extend the regression test to cover the lock/integrity-backed install path rather than only the three direct package tokens.
|
Fixed at c932b50: the changelog now limits the claim to exact direct harness dependencies and no package download during harness execution; the regression test name matches that scope. Verified with npm ci --ignore-scripts and npm run check (45/45). |
TueJon
left a comment
TueJon
left a comment
There was a problem hiding this comment.
The previous transitive-reproducibility caveat is closed: the changelog now limits the guarantee to exact direct harness dependencies and no package download during harness execution, and the test name matches that scope.
No blocking findings or fix-induced regressions remain at this head.
TueJon
left a comment
TueJon
left a comment
There was a problem hiding this comment.
The previously closed direct-vs-transitive pinning caveat remains closed at this head.
The merge commit introduces no PR-side content change or fix-induced regression; no blocking findings remain.
1 participant
v0.6.0 removed unpinned package execution from the guidance step, but the verification harness in
references/verify.mdstill rannpm i -D @playwright/test typescript @types/node. That installs whatever the registry serves as latest. A PR risk scan on the v0.6.0 mirror refresh (github/awesome-copilot#3420) flagged the step.@playwright/test@1.61.1,typescript@5.9.3and@types/node@22.20.1with--save-exact. These are the versions this repository tests with, and@playwright/testmatches the Workbench's pinned Playwright../node_modules/.bin/playwright testinstead ofnpx, so a missing install fails rather than downloading a package.tests/harness-pin.test.mjsfails when the documented install is unpinned, usesnpx, or drifts fromscripts/workbench.mjs. Verified red without the fix and green with it.release/v0.6.1.md.Validation:
npm run checkpasses (45 tests plus type and syntax checks).Closes #18