test(ci): exercise full validation suite#5083
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (47)
📝 WalkthroughWalkthroughThis PR updates Playwright CI artifact packaging and shard allocation, improves embedded-core RPC and browser-input recovery, migrates E2E coverage to current routes, extends test timeouts, and updates Rust integration fixtures and coverage assertions. ChangesCI execution and recovery
Current E2E surfaces
Rust coverage fixtures
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6528df8d38
ℹ️ 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".
| body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'core.ping', params: {} }), | ||
| // A recently stopped listener can take several seconds to reject on | ||
| // Windows. Keep discovery inside resetApp's eight-second RPC budget. | ||
| signal: AbortSignal.timeout(750), |
There was a problem hiding this comment.
Keep RPC discovery within reset budget
When the cached listener is stale and the restarted embedded core binds near the end of the newly-expanded fallback range (for example, 7798), discovery first pings the cached URL and then probes ports sequentially; with this 750 ms timeout, those probes can exceed resetApp's 8-second Promise.race before the live port is reached. In that case resetApp treats a healthy restarted core as “sidecar likely not started” and skips the wipe/re-auth baseline, leaving later desktop E2E shards with stale state. Consider increasing that reset budget or making discovery parallel/shorter per port.
Useful? React with 👍 / 👎.
Summary
Problem
Solution
Submission Checklist
Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm typecheckequivalent passed in PR CI Gate / Frontend Checkspnpm debug unit test/core-rpc-node.test.tspassed (2 tests)Validation Blocked
command: repository-wide pre-push hookerror: intentionally not run locally because the requested workflow delegates CPU-heavy validation to GitHub Actionsimpact: pushes used --no-verify; equivalent and broader exact-head checks passed in PR CI and CI FullBehavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
Bug Fixes
Improvements