fix(kirocrew): install Playwright deps via dnf on AL2023 (do not merge yet) - #96
fix(kirocrew): install Playwright deps via dnf on AL2023 (do not merge yet)#96royosherove wants to merge 2 commits into
Conversation
…94) Codex flagged: playwright-cli install-browser --with-deps shells out to apt-get unconditionally. On AL2023 (the deploy target) that fails silently at the deps phase, browser install falls through to the warn-and-continue path, and Chromium is left unusable at runtime. Root cause confirmed by upstream tracking: amazonlinux/amazon-linux-2023#820 'when i install playwright using pip in amazon linux 2023 first of all it recognized it as an ubuntu system and runs apt commands to download its packages, ultimately it fails, i have to manually install the required packages...' AWS has not shipped an AL2023 fix; the community workaround is manual dnf install of the runtime libs. This PR implements that workaround. Split the install into three sub-steps: 2a. Distro-detect via /etc/os-release; if RHEL-family (amzn/rhel/ fedora/centos/rocky/almalinux), sudo dnf install the runtime libraries. Package list per Roy's confirmed AL2023 recipe: libXcomposite libXdamage libXrandr nss alsa-lib atk cups-libs gtk3 libdrm mesa-libgbm pango libicu woff2 Skip step on non-dnf distros with an info line (defensive). 2b. playwright-cli install-browser (no --with-deps). 2c. playwright-cli install --skills (agent skills, non-fatal). All steps warn-not-fail individually. Failure of 2a doesn't abort 2b/2c; the browser install can still succeed and the agent surfaces the runtime error clearly if libs are missing. Verified: bash -n packs/kirocrew/install.sh: OK. Diff: +46 / -8.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fda6f61351
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| warn "dnf install of Chromium libs returned non-zero — browser may fail at launch; check dnf log" | ||
| fi | ||
| else | ||
| info "Non-dnf distro or dnf unavailable — skipping RPM deps; browser install will still run" |
There was a problem hiding this comment.
Replace the undefined
info logger
On the supported Ubuntu 22.04 path—and whenever dnf is unavailable—this invokes info, but packs/common.sh only defines log, ok, fail, and warn. On minimal hosts the command is absent, and because the installer uses set -e, installation terminates here before Chromium or KiroCrew is installed; if GNU Info happens to exist, it treats the message as a nonexistent manual topic and can likewise return nonzero.
Useful? React with 👍 / 👎.
| # 2b. Install Chromium binary itself (~/.cache/ms-playwright/, per-user). | ||
| # No --with-deps: system libs handled by 2a above on AL2023. | ||
| log "Installing Chromium browser binary (may take 30-90s)..." | ||
| if playwright-cli install-browser 2>&1 | while IFS= read -r line; do log " playwright: ${line}"; done; then |
There was a problem hiding this comment.
Keep installing apt dependencies on Ubuntu
For a clean Ubuntu 22.04 host, which manifest.yaml explicitly lists as supported, the new distro branch skips dependency installation and this command downloads only the browser binary. The previous install-browser --with-deps invocation installed the required apt libraries, so after the undefined logger is corrected, Chromium can still fail at launch on Ubuntu; retain --with-deps for apt-based distributions while using the manual dnf path for AL2023.
Useful? React with 👍 / 👎.
…deps for Ubuntu Two P1s from Codex review of the AL2023-fix PR: P1 #1 (line 389): 'info' isn't defined in packs/common.sh (only log/ok/fail/warn). Under set -e the installer would terminate on non-dnf hosts before Chromium ever installs. P1 #2 (line 396): I dropped --with-deps for the entire distro branch, but manifest.yaml lists ubuntu2204 as supported. On Ubuntu, Playwright's --with-deps IS the correct path (apt-get is what Playwright actually invokes there). Only AL2023 needed the manual dnf detour. Fix — three-branch distro dispatch: 1. RHEL-family (amzn/rhel/fedora/centos/rocky/almalinux): sudo dnf install the explicit RPM list, then install-browser WITHOUT --with-deps. 2. Debian-family (ubuntu/debian): install-browser --with-deps (unchanged from original PR #94). 3. Unknown distro: install-browser --with-deps as a best-effort with a 'log' line (not 'info') noting the unknown distro. Warn on failure. Distro detection unchanged: /etc/os-release ID/ID_LIKE match against the two families. Verified: bash -n packs/kirocrew/install.sh OK. Diff on top of fda6f61.
Fixes the Codex P1 on merged PR #94.
The bug
playwright-cli install-browser --with-depsshells out toapt-get. AL2023 usesdnf. On the deployed instance the deps phase silently fails, and Chromium is left unusable at runtime.Confirmed upstream: amazonlinux/amazon-linux-2023#820 — 'when i install playwright... in amazon linux 2023 first of all it recognized it as an ubuntu system and runs apt commands to download its packages, ultimately it fails.' AWS has not shipped a fix; the community workaround is manual
dnf installof the runtime libs.What we ship on
template.yaml:1801):al2023-ami-kernel-6.1-arm64(Amazon Linux 2023, ARM64).al2023andubuntu2204, but every real deploy resolves to AL2023 via the SSM AMI parameter.Fix — three sub-steps
Replaces the single
playwright-cli install-browser --with-depscall inpacks/kirocrew/install.sh:2a. dnf deps — Distro-detect via
/etc/os-release; on RHEL-family (amzn/rhel/fedora/centos/rocky/almalinux),sudo dnf install -ythe runtime libs. Package list per Roy's confirmed AL2023 recipe:libXcomposite libXdamage libXrandr nss alsa-lib atk cups-libs gtk3 libdrm mesa-libgbm pango libicu woff2Skips on non-dnf distros with an
infoline (defensive).2b. Browser binary —
playwright-cli install-browser(no--with-deps).2c. Agent skills —
playwright-cli install --skills(non-fatal).All three sub-steps warn-not-fail individually; failure of 2a doesn't abort 2b/2c.
Verified
bash -n packs/kirocrew/install.sh: OKNot merging per Aug 22 20:34 rule.