fix(windows): export KAIDEN_BINARY when installing Kaiden via -pdUrl#582
fix(windows): export KAIDEN_BINARY when installing Kaiden via -pdUrl#582serbangeorge-m wants to merge 1 commit into
Conversation
Signed-off-by: serbangeorge-m <[email protected]>
📝 WalkthroughWalkthroughThe Windows E2E runner gains Kaiden app detection via an ChangesKaiden Binary Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pde2e-image/README.md`:
- Around line 294-295: The Kaiden setup docs currently imply `-appName Kaiden`
and `-repo kaiden` are interchangeable, but the setup path logic in the runner
depends on `$appName` for resolving `$appName.exe`. Update the README section
that describes setup-flow requirements to explicitly state that setup installs
must pass `-appName Kaiden` for `KAIDEN_BINARY` to point to the expected
executable, and only mention `-repo kaiden` as valid for non-setup/direct-path
scenarios if that is supported by the runner.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a1a7305-c6e4-4e96-bf61-82a0db2c6ffe
📒 Files selected for processing (3)
pde2e-image/README.mdpde2e-image/common/windows/common.ps1pde2e-image/lib/windows/runner.ps1
| When `-appName Kaiden` (or `-repo kaiden`) is used, the runner keeps `Kaiden.exe` at | ||
| `%LOCALAPPDATA%\Programs\kaiden\` and exports `KAIDEN_BINARY` for Kaiden's Playwright CDP harness. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify setup-flow requirements in the Kaiden docs.
Line 294 says -appName Kaiden or -repo kaiden, but setup-path resolution still depends on $appName ($appName.exe). Please clarify that setup installs should pass -appName Kaiden (or document that repo-only works only in non-setup/direct-path scenarios).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pde2e-image/README.md` around lines 294 - 295, The Kaiden setup docs
currently imply `-appName Kaiden` and `-repo kaiden` are interchangeable, but
the setup path logic in the runner depends on `$appName` for resolving
`$appName.exe`. Update the README section that describes setup-flow requirements
to explicitly state that setup installs must pass `-appName Kaiden` for
`KAIDEN_BINARY` to point to the expected executable, and only mention `-repo
kaiden` as valid for non-setup/direct-path scenarios if that is supported by the
runner.
|
I still maintain these kind of changes need to be done in a Kaiden related fork of this repo, putting two projects in the same image/container is not a scalable solution long term. The problem will only increase as we go forward in time and divergence between PD and Kaiden increases. |
Kaiden MAPT e2e runs install via -pdUrl but Playwright needs KAIDEN_BINARY, not PODMAN_DESKTOP_BINARY. This sets that env var after install and keeps Kaiden.exe instead of renaming it.