fix: run wb through package manager in playwright web server#742
fix: run wb through package manager in playwright web server#742
Conversation
Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
There was a problem hiding this comment.
Code Review
This pull request updates the Playwright configuration and agent instructions to invoke the wb command using yarn. The review feedback suggests using a dynamic packageManager variable instead of hardcoding yarn to support different package managers like Bun, and provides a suggestion to update the test suite to reflect this change.
Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the Playwright configuration and agent instructions to ensure the wb command is executed via the project's package manager (bun or yarn). Feedback focuses on reducing code duplication by extracting the command string generation into a helper function and moving the package manager detection logic to a shared utility file to adhere to the DRY principle.
Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request centralizes the logic for determining the package manager command (bun or yarn) by introducing a getPackageManagerCommand utility. It refactors the Playwright configuration fixer and agent instruction generator to invoke wb commands through the appropriate package manager, ensuring consistency across different project environments. I have no feedback to provide as there were no review comments to evaluate.
Summary
webServer.commandthrough the target package manager, e.g.yarn wb start --mode testorbun wb start --mode test.src/utils/packageCapabilities.ts.Why
wbas a package dependency, so invokingwbdirectly can fail when the package binary directory is not on PATH.Testing
yarn verifyyarn vitest test/agentInstructions.test.tsNotes
yarn verifystill reports 4 pre-existing Oxlint warnings unrelated to this change.