Repository navigation
01: CI runs the repo's checks on push and PR - #11
Merged
Merged
Conversation
This was referenced Oct 13, 2026
The base branch carried three files Prettier would rewrite, so format:check failed on every run of this workflow: .ome/project.json, test/browser.test.ts and test/session-row-ui.test.ts. Whitespace only.
A browser closed just before the test's folder is removed can still be writing its profile, which left the folder non-empty and failed the run with ENOTEMPTY (run 38424667589, visible-models-ui). The removal now retries.
This was referenced Oct 13, 2026
T-Crypt
marked this pull request as ready for review
October 13, 2026 16:20
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.
CI runs the repo's own checks on every push and pull request. Before this they ran by hand.
What changed
.github/workflows/ci.ymlrunsnpm ci,npm run check,npm run lint,npm run format:check,and
npm teston Node 22.19 and Node 24. Actions are pinned to their current majors. The token isread-only.
pushnames the base branches, so a branch with an open PR runs once throughpull_request, and agh-pagespush fromsite:publishdoes not trigger a run.package.jsongainspackageManager,which setup-node needs before it caches npm.
The job now names the browser it uses.
PI_POCKET_BROWSER: /usr/bin/google-chromepicks the deb'sGoogle Chrome. The image's own
chromiumis a snapshot build unpacked under/usr/local/share/chromiumand reached through the/usr/bin/chromiumsymlink, which PATH ordermakes
findBrowser()return first; it has none of its dependencies installed and no AppArmorprofile, so Ubuntu 24.04's restriction on unprivileged user namespaces kills it as it starts, and
the DevTools pipe closes. That is the failure of run 38350258246, both legs: 628 tests, 491 pass,
137 fail, every browser file dying in its
beforehook withThe browser closed.PI_POCKET_BROWSER_ARGS: --no-sandboxis the remedy the README already documents for a machinewhere sandboxes are unavailable, which a CI runner is.
A step before
npm testrunstest -x /usr/bin/google-chrome. WithPI_POCKET_BROWSERnaming apath that is not a file,
findBrowser()returns undefined and every browser test skips itself, so ajob that had lost its browser would go green with 137 tests doing nothing. The guard turns that
silent loss into a red step.
test/visible-models-ui.test.tscounted the picker's router rows as model rows. The picker'sRouters group is rendered as soon as the
api("routers")fetch resolves, so the test failedwhenever that won the race, 3 of 5 single-file runs here, with
standard on no model yetin therows. Its
menu()helper now reads.pop-menu-list .pop-menu-row:not(.router-row). Without this theworkflow cannot be green whatever the browser does, so it rides here rather than waiting for ticket
06.
Proof
Run in the worktree on Node 26.11.1:
npm cinpm run checknpm run lintnpm run format:checknpm testPI_POCKET_BROWSER=/nonexistent-browser-on-ci npm testnode --test test/visible-models-ui.test.ts, five runs after the fixpython3 -c 'import yaml; yaml.safe_load(open(".github/workflows/ci.yml"))'The second-to-last line is why the fallback is safe before it is needed, and the counts are the
measure that the browser tests ran rather than skipped: on a machine with no browser this suite is
137 skipped and 0 fail.
Review
Clean on the workflow itself. Pinned majors checked against the GitHub API, the Node matrix matches
engines,permissions: contents: readis right, andformat:checkdoes cover YAML.Open
beforehook in run 38350258246. If one of them turns out to be runner-specific (fonts, layout,timing), the fallback is one line: replace the
env:block withPI_POCKET_BROWSER: /nonexistent-browser-on-ciand delete the guard step, which is the statemeasured above.
--no-sandboxin CI. Reversible in one line; if the deb's Chrome keeps its sandbox on the runner,dropping the line is the stronger posture.
The browser closed.cannot name its own cause, which is why the run above wasdiagnosed from the image's build script rather than from the log. The DevTools pipe's close event
(
cdp.ts:31-33) closes the connection before the child's exit handler (cdp.ts:218-223) canreport the exit code and the browser's stderr, so the reason a browser died never surfaces. Worth
a ticket of its own.
LAUNCH_ARGSinsrc/server/browser/cdp.tsnever passes--no-sandboxby itself, which iscorrect: the flag comes from the environment, which is what
PI_POCKET_BROWSER_ARGSis for./usr/bin/google-chromecomes from reading the image's build script, not from running the image.A wrong path fails loudly at the guard step rather than skipping silently; the cheap correction is
to drop
PI_POCKET_BROWSERand keep only the flag, which then applies to the snapshot Chromium.Ticket 01. Run folder:
~/handoffs/oh-my-pocket/2026-10-13-pr11-13-rerun/.Two more fixes, found by running the workflow here
format:checkfailed on three files the base had left unformatted:.ome/project.json,test/browser.test.tsandtest/session-row-ui.test.ts. It failed on both legs in 40 seconds, before the tests could run at all. Whitespace only.With a browser that launches, one test failed on its teardown: a browser closed just before the test's temporary folder is removed can still be writing its profile, so the removal raced it and the run ended
ENOTEMPTY: directory not empty, rmdir .../browser/profile/Default(run 38424667589, 631 tests, 630 pass, 1 fail).test/helpers.ts'scleanUpnow retries the removal, which is what Node'smaxRetriesis for.