fix: Linux support (cross-platform Chrome default + GNU-safe ps probe)#110
fix: Linux support (cross-platform Chrome default + GNU-safe ps probe)#110bigsaam wants to merge 2 commits into
Conversation
pro-vi
left a comment
There was a problem hiding this comment.
Note: this review was written by my AI agent against branch head
9915e78. I've read it before sending.
Thanks for this, and sorry for the slow reply. The diagnosis is correct.
Reproduced on debian:stable-slim (procps-ng 4.0.4) — ps -Axww -o command exits 1 with error: must set personality to get -x option. Given the if (r.status !== 0) return 'unknown' on the next line, the --user-data-dir check has been silently skipped on Linux all along, as you say in the PR body.
ps -eww -o command is the right substitution. Ran the full pipeline through dash on the same image: exit 0, one match, and grep -v grep still filters the probe's own sh -c.
One thing to fix before this goes in, plus two optional one-liners.
1. pgrep -f "chrome" matches the launcher itself (blocking)
On Linux this warns on every run, even with no Chrome installed at all. The match is the script's own interpreter — bash ./scripts/designer-chrome.sh — because the filename contains "chrome" and pgrep -f matches the whole command line.
Minimal repro, on a Debian container with zero Chrome processes. The two scripts have identical contents and differ only in filename:
printf 'pgrep -f chrome >/dev/null && echo "would warn"\n' > /tmp/designer-chrome.sh
printf 'pgrep -f chrome >/dev/null && echo "would warn"\n' > /tmp/harmless.sh
$ bash /tmp/designer-chrome.sh
would warn
$ bash /tmp/harmless.sh
$ # silentRun it from a shell whose own command line doesn't contain "chrome", otherwise you'll match the harness rather than the script.
On your branch, a debug line before the check shows the culprit, with no Chrome running:
pgrep -f "chrome" matches:
→ 3130 bash /work/designer-chrome.sh
The same harness against main's script emits no warning, so this is new in the diff rather than ambient.
It's invisible on macOS because of a BSD/GNU divergence:
BSD
pgrep(1):-a— Include process ancestors in the match list. By default, the current pgrep or pkill process and all of its ancestors are excluded.GNU
pgrep(1):-a, --list-full— list PID and full command line.
BSD excludes ancestors by default; GNU excludes only pgrep itself. So the parent shell is dropped on macOS and returned on Linux. Note -a means opposite things on the two platforms.
Matching on the process name instead of the command line fixes it, and also stops chromedriver matching (I tested that one; presumably the same for chrome-sandbox and chrome_crashpad_handler, untested):
if [ "$(uname -s)" = "Darwin" ]; then
chrome_running() { pgrep -f 'Google Chrome' >/dev/null; }
QUIT_HINT="Quit existing Chrome (Cmd+Q) first"
else
chrome_running() { pgrep -x 'chrome|chromium' >/dev/null; }
QUIT_HINT="Quit existing Chrome (or 'pkill chrome') first"
fi
if chrome_running; thenChecked both directions: no self-match, and it still detects a real process named chrome.
2. Two one-line hardenings (optional)
Neither is likely to bite anyone in practice. Both are one-liners, so worth taking while you're here.
CHROME_BIN is no longer nounset-safe. ${CHROME_BIN:-…} tolerated an unset var; [ -n "$CHROME_BIN" ] doesn't. Only reachable if a parent exports SHELLOPTS=nounset (a plain set -u in the parent does not propagate), so: rare. Still:
-if [ -n "$CHROME_BIN" ]; then
+if [ -n "${CHROME_BIN:-}" ]; thenAn inherited CHROME survives the candidate loop. main assigned CHROME unconditionally; the new loop only assigns on a hit, so CHROME="${CHROME:-/usr/bin/google-chrome}" will preserve whatever was in the environment:
else
+ CHROME=""
for c in /usr/bin/google-chrome /usr/bin/google-chrome-stable /usr/bin/chromium /usr/bin/chromium-browser; doThe set -e + [ -x "$c" ] && … && break construct is fine, in case you wondered — the failing test is a non-final command in an && list, so errexit is suppressed. Checked under bash, dash, and busybox ash. No change needed.
3. The README banner needs its prerequisites to agree
Right now the diff changes line 7 but leaves line 21, so the file contradicts itself:
README.md:7 > **Status:** v0.3.14, early. macOS and Linux.
README.md:21 - macOS, Node 22+ …, Google Chrome at `/Applications/Google Chrome.app`.
Separately, unqualified "macOS and Linux" promises more than the code delivers. There's no DISPLAY/WAYLAND_DISPLAY check anywhere in the repo, so I'd expect a headless box to fail at the 30s CDP poll with an opaque message — I haven't run that, so treat it as a guess. I'd also want to know how this behaves with snap-packaged Chromium before claiming stock-Ubuntu support. You tested on Ubuntu — did you use the Google Chrome .deb, or the distro chromium?
Something scoped, like "macOS; Linux with Chrome/Chromium and an active display session", would be safer. I can take this one if you'd rather not.
Not yours — I'll handle separately
cross-platform.tsisChromeRunning()uses the same broadpgrep -f 'chrome', and readingsetup.ts, a match sends it into a five-minute poll that then hard-fails. A straychromedrivershould therefore blockdesigner setup. Pre-existing onmain, not introduced here. I'll file it and confirm it there.defaultChromeBin()'s Linux candidates omitgoogle-chrome-stable, which your.shnow includes. Mostly theoretical — the Chrome.debpostinst runsupdate-alternatives --install /usr/bin/google-chrome, so that path normally exists — but the two resolvers should agree. Our file.
CI
action_required with zero jobs is the fork-PR approval gate, not anything you did. I'll approve the run.
All of the above ran in a debian:stable-slim container, not on real hardware. Let me know if these reproduce on your machine — you have an actual Ubuntu box and I don't. If the pgrep self-match doesn't show up for you, I'd want to know before you change anything.
Maintainer edits are enabled, so I can push these to linux-support and merge with you as author if you prefer. Otherwise push whenever suits and I'll re-review.
Summary
Makes
designerrun on Linux out of the box. It's already ~cross-platform(
package.jsondeclaresos: [darwin, linux, win32]andcross-platform.tsbranches per-OS), but two spots still assumed macOS. Both fixed here:
scripts/designer-chrome.shdefaultedCHROMEto the macOS app path, so thefallback launcher failed on Linux even with Chrome installed. Now resolves
CHROME_BIN→ per-OS default (/usr/bin/google-chrome/chromiumon Linux),and the "already running"
pgreppattern + quit hint are OS-aware.(
setup.tsalready used the Linux-awaredefaultChromeBin(), so this onlyaffected the standalone
.shfallback.)setup.tsused BSDps -Axwwto verify the debug Chrome's--user-data-dir.GNU/Linux
psrejects-x(error: must set personality to get -x option), sothe probe silently returned
unknownon Linux. Now OS-aware:ps -eww -o commandoff macOS. (
unknownis adopt-ok via the DOM backstop, so this was a softdegradation, not a hard failure — but the profile check now actually runs.)
Also updates the README status banner (
macOS only→macOS and Linux).Validation (Ubuntu, Node 24, Chrome 149, headless via Xvfb)
tsc --noEmitclean.designer doctor→ agent-browser ✓, CDP ✓, signed in to claude.ai/design ✓.designer health→ 14 ok / 0 fail (8 state-dependent skips), incl. theShare → handoff export endpoint returning
200 application/zip.agent-browser(Node 24) connects to the debug Chrome over CDP and drives theauthenticated session headlessly.
ps -Axww→error: must set personality to get -x optionon GNU/Linux;
ps -eww -o command→ exit 0, full argv.No behavior change on macOS (the mac branches are string-identical) or Windows
(the
.shpath is only used off-Windows;setup.ts's probe early-returns onIS_WIN).🤖 Generated with Claude Code