Skip to content

fix(kirocrew): install Playwright deps via dnf on AL2023 (do not merge yet) - #96

Closed
royosherove wants to merge 2 commits into
mainfrom
fix/kirocrew-playwright-dnf-deps
Closed

fix(kirocrew): install Playwright deps via dnf on AL2023 (do not merge yet)#96
royosherove wants to merge 2 commits into
mainfrom
fix/kirocrew-playwright-dnf-deps

Conversation

@royosherove

Copy link
Copy Markdown
Member

Fixes the Codex P1 on merged PR #94.

The bug

playwright-cli install-browser --with-deps shells out to apt-get. AL2023 uses dnf. 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 install of the runtime libs.

What we ship on

  • CFN AMI (template.yaml:1801): al2023-ami-kernel-6.1-arm64 (Amazon Linux 2023, ARM64).
  • Pack manifest lists al2023 and ubuntu2204, but every real deploy resolves to AL2023 via the SSM AMI parameter.

Fix — three sub-steps

Replaces the single playwright-cli install-browser --with-deps call in packs/kirocrew/install.sh:

  • 2a. dnf deps — Distro-detect via /etc/os-release; on RHEL-family (amzn/rhel/fedora/centos/rocky/almalinux), sudo dnf install -y the 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 woff2

    Skips on non-dnf distros with an info line (defensive).

  • 2b. Browser binaryplaywright-cli install-browser (no --with-deps).

  • 2c. Agent skillsplaywright-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: OK
  • Diff: +46 / -8

Not merging per Aug 22 20:34 rule.

…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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread packs/kirocrew/install.sh Outdated
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"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment thread packs/kirocrew/install.sh Outdated
# 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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.
@royosherove

Copy link
Copy Markdown
Member Author

Superseded by #97 (fix/kirocrew-playwright-runtime), which folds this dnf fix into a broader runtime fix (gateway service PATH + node/playwright-cli symlinks + explicit dnf dep install). #97 merged as c2301c1.

@royosherove
royosherove deleted the fix/kirocrew-playwright-dnf-deps branch August 23, 2026 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant