test(ci): harden cross-platform reliability gates - #137
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds browser quarantine tooling, restructures CI and packaged desktop E2E jobs, introduces a guarded fake-Codex Electron harness, and updates WebSocket recovery, project recovery, file-read retries, adopted webview handling, and test support. ChangesCI and quarantine
Desktop E2E
Web runtime and browser lifecycle
Platform and test support
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
scripts/quarantine-registry.ts (1)
146-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the runtime-platform mapping into the shared registry module.
runtimeQuarantinePlatformduplicatesquarantinePlatforminapps/web/vitest.browser.quarantine.ts(both mapprocess.platform→QuarantinePlatform | null). Since both files already import from./lib/quarantine-registry, hoist a single helper there so the runtime→platform mapping cannot drift between CI and the browser harness.As per coding guidelines: "avoid duplicating logic across files and prefer reusable modules over local shortcuts."
🤖 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 `@scripts/quarantine-registry.ts` around lines 146 - 150, Move the process.platform-to-QuarantinePlatform mapping from runtimeQuarantinePlatform into the shared quarantine-registry module, and export the reusable helper there. Update both scripts/quarantine-registry.ts and apps/web/vitest.browser.quarantine.ts to import and use that shared helper, removing the duplicate local implementations while preserving the null result for unsupported platforms.Source: Coding guidelines
🤖 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 `@apps/desktop/e2e/desktop.fixture.ts`:
- Around line 957-968: Update the nonNodeInvocationPids filter in the
invocationEvents flatMap so runtime.executable is accessed safely when runtime
is null. Use optional chaining or an explicit runtime guard before
Path.basename, while preserving the existing Node executable exclusion behavior.
In `@apps/desktop/e2e/fixtures/electron-bootstrap.cjs`:
- Around line 85-88: Update the NODE_OPTIONS assignment in the Electron
bootstrap initialization to preserve the existing process.env.NODE_OPTIONS value
while adding the escaped networkGuardPath --require option. Ensure inherited CI
and tooling flags remain available when main.ts builds backend child arguments,
and avoid replacing the existing value.
In `@apps/desktop/e2e/fixtures/fake-codex.ts`:
- Around line 123-127: Replace the synchronous FS.readFileSync(0, "utf8") call
in the text-generation fixture flow with asynchronous consumption of
process.stdin, awaiting the stream until it ends while discarding its contents.
Preserve the existing protocol log ordering and only write the output after
stdin consumption completes.
- Line 8: Make the side-effect import in fake-codex type-safe by adding the
project’s appropriate declaration/shim for the network-guard.cjs module, or
rename the fixture to a .cts file if that matches the existing module setup.
Preserve the import’s runtime behavior and ensure TypeScript no longer reports
an unresolved .cjs module.
---
Nitpick comments:
In `@scripts/quarantine-registry.ts`:
- Around line 146-150: Move the process.platform-to-QuarantinePlatform mapping
from runtimeQuarantinePlatform into the shared quarantine-registry module, and
export the reusable helper there. Update both scripts/quarantine-registry.ts and
apps/web/vitest.browser.quarantine.ts to import and use that shared helper,
removing the duplicate local implementations while preserving the null result
for unsupported platforms.
🪄 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 Plus
Run ID: b4939788-de99-4a53-ae88-62902c4c152f
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (54)
.gitattributes.github/quarantine.yml.github/workflows/ci.yml.gitignoreapps/desktop/e2e/desktop.e2e.tsapps/desktop/e2e/desktop.fixture.test.tsapps/desktop/e2e/desktop.fixture.tsapps/desktop/e2e/fixtures/diagnostic-redaction.cjsapps/desktop/e2e/fixtures/electron-bootstrap.cjsapps/desktop/e2e/fixtures/fake-codex.tsapps/desktop/e2e/fixtures/loopback.cjsapps/desktop/e2e/fixtures/network-guard.cjsapps/desktop/e2e/global-setup.tsapps/desktop/e2e/playwright.config.tsapps/desktop/e2e/support/failureDiagnostics.tsapps/desktop/e2e/support/processTree.test.tsapps/desktop/e2e/support/processTree.tsapps/desktop/e2e/tsconfig.jsonapps/desktop/package.jsonapps/desktop/scripts/smoke-test-windows-job.windows.integration.test.mjsapps/desktop/src/browserManager.test.tsapps/desktop/src/browserManager.tsapps/server/src/provider/windowsJobLauncherSourceContract.test.tsapps/web/BROWSER_TEST_QUARANTINE.mdapps/web/package.jsonapps/web/src/components/ChatMarkdown.test.tsxapps/web/src/components/ChatView.browser.tsxapps/web/src/components/Sidebar.import.test.tsapps/web/src/components/chat/MessagesTimeline.test.tsxapps/web/src/lib/projectCreateRecovery.test.tsapps/web/src/lib/projectCreateRecovery.tsapps/web/src/lib/projectCreation.test.tsapps/web/src/lib/projectCreation.tsapps/web/src/lib/projectReactQuery.test.tsapps/web/src/lib/projectReactQuery.tsapps/web/src/wsNativeApi.test.tsapps/web/src/wsNativeApi.tsapps/web/src/wsTransport.test.tsapps/web/src/wsTransport.tsapps/web/vitest.browser.geometry.config.tsapps/web/vitest.browser.quarantine.tsapps/web/vitest.browser.stable.config.tspackage.jsonpackages/contracts/package.jsonpackages/shared/package.jsonscripts/benchmark-windows-command-discovery.test.tsscripts/lib/quarantine-registry.test.tsscripts/lib/quarantine-registry.tsscripts/lib/release-worktree-cleanliness.test.tsscripts/lib/windows-authenticode.test.tsscripts/lib/workflow-contracts.test.tsscripts/lib/workflow-contracts.tsscripts/package.jsonscripts/quarantine-registry.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
coderabbitai
Bot
added
the
ready-to-merge
There was a problem hiding this comment.
slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
coderabbitai
Bot
removed
the
ready-to-merge
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/lib/quarantine-registry.test.ts">
<violation number="1" location="scripts/lib/quarantine-registry.test.ts:85">
P3: The test name references "generated route-tree staging" but the function under test is `createQuarantineInventoryTemporaryDirectory` which creates a quarantine-inventory temp directory under `.tanstack/`. The route-tree concept belongs to the previous test's `createQuarantineInventoryEnvironment` and `SYNARA_GENERATED_ROUTE_TREE`. Consider renaming to something like "keeps quarantine inventory staging inside .tanstack" to avoid confusing readers.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
slashdevcorpse has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
1 participant
Summary
Verification
Full formatting, lint, and typecheck gates are delegated to hosted CI as required by the repository instructions.
Summary by cubic
Hardens cross-platform CI with a source-registered browser quarantine and deterministic packaged Electron E2E on Windows and Linux. Improves Windows stability with capped Vitest workers, extended targeted timeouts, and safer browser/desktop lifecycles with one-shot WS recovery.
New Features
[quarantine:<id>]in.github/quarantine.yml; Vitest derives stable/quarantine sets per platform; CI validates the registry, inventories the full suite, and posts platform-scoped summaries.@playwright/testwith a fake provider, loopback-only network guard, process-tree teardown, redacted failure diagnostics, and prebuilt-artifact checks; wired into Windows/Linux CI; repotest:e2escript added.--maxWorkers=50%across packages, and extended Windows staging/startup budgets.Bug Fixes
wsNativeApi(server refresh, orchestration dispatch, shell reads); tests forshouldRecoverUnaryRequest.isProjectReadFileCapacityErrorand updated React Query options.Written for commit ddbb014. Summary will update on new commits.
Summary by CodeRabbit
project.createhandling.