Repository navigation
Conversation
The vendored @pdpp/polyfill-connectors tarball pins patchright 1.63.0 exactly, while the root and reference-implementation pin 1.64.1. npm installed both, so connector code (acquireIsolatedBrowser) launched the nested 1.63.0 and needed Chromium revision 1243, while the Dockerfile and `patchright install` fetch the 1.64.1 revision (1248). Add a scoped npm override that points the tarball's patchright at the root version, so one patchright and one patchright-core remain. The tarball is unchanged: the freshness check requires it to match a rebuild of the pinned upstream commit. The lockfile loses the two nested entries and nothing else. scripts/patchright-single-version.test.ts fails on any extra Patchright copy in package-lock.json, or one that differs from the version the image installs browsers for. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
The single-version guard checked the lockfile outcome only. Deleting the root overrides entry would pass until the next lock regeneration nests the tarball pin again, so assert the entry itself. Also check that the Dockerfile installs browsers for reference-implementation's exact patchright pin, which the guard's header comment relies on. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
The test matched four source substrings in the Dockerfile, so commented lines passed it and equivalent rewrites failed it. It said nothing about the built image. The docker-images workflow already builds the core image and launches Patchright in it (core-headed-patchright-runtime-oracle), so the substring check is removed rather than replaced. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Contributor
Author
|
Closing as superseded by #622, which covers all three lockfiles and adds the check that fails on a partial Patchright bump. Assisted-by: AI |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
package-lock.jsoninstalls two Patchright versions. The root andreference-implementationpinpatchright1.64.1. The vendored@pdpp/polyfill-connectorstarball pins 1.63.0 exactly, so npm nests a second copy under it. Connector code in that package (acquireIsolatedBrowser) launches the nested 1.63.0 and needs Chromium revision 1243, whilenpx patchright installand the Dockerfile's browser layer fetch the 1.64.1 revision (1248). On a dev or CI install the browser the tooling installs is not the one connectors launch. That is why an explicitnpx patchright install chromiumdid not fix the ubuntu-26.04 failure in #622.This adds a scoped npm override,
"@pdpp/polyfill-connectors": { "patchright": "$patchright" }, so the tarball's Patchright follows the root pin and onepatchrightand onepatchright-coreremain. The tarball itself is unchanged, becausescripts/check-polyfill-connectors-tarball-freshness.mjsrequires it to equal a rebuild of the pinned upstream commit. The lock diff only removes the two nested entries.1.63.0 to 1.64.1 is additive in
patchright-core's public types: an optionalincludeShadowargument oncontent(), newgetByRefandpage.webmcp, and two methods that now returnPromise<Disposable>instead ofPromise<void>.browsers.jsonmoves Chromium from 1243 to 1248. I did not audit every connector call site against the two changed return types.scripts/patchright-single-version.test.tsfails when the lock holds a Patchright copy that differs fromreference-implementation/package.json's exact pin, which is the version the Dockerfile installs browsers for. It fails on main (lists the 1.63.0 entries) and passes here (3 tests).browser-executable.test.tsandowner-connection-delete.test.tspass (51 of 51). Before the 1248 headless shell was installed locally, the delete test failed asking forchromium_headless_shell-1248, which shows connectors now launch the root version.The test does not cover the Dockerfile's browser install. An earlier version matched Dockerfile substrings, which proved nothing about the image, so it was removed. The built image is exercised by the
docker-imagesworkflow, which runsscripts/core-headed-patchright-runtime-oracle.shagainst the built core image; that oracle launches Patchright and checks the full Chromium binary under/opt/patchright-browsers.Production images built from main are probably not affected: the nested copy is a dev dependency, and the image installs with
--omit=dev. I have not built an image to confirm that.Assisted-by: AI