Repository navigation
Integrate current-main check repairs - #8941
Conversation
…egrate-web-determinism-repairs
…egrate-web-determinism-repairs
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds an isolated Bun test runner, routes web package and CI execution through it, adds regression coverage, updates an iOS duration assertion, adjusts Swift defaults and state handling, adds a Ghostty test stub, and applies concurrency-related fixes. ChangesShared web test runner
Swift behavior and warning updates
iOS transition duration assertion
Ghostty runtime test stubs
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant WebPackage
participant RunTests
participant Bun
CI->>WebPackage: run bun run test
WebPackage->>RunTests: execute scripts/run-tests.sh
RunTests->>Bun: run test --isolate with discovered files or forwarded discovery options
Bun-->>RunTests: return test results
RunTests-->>CI: return test status
Possibly related PRs
🚥 Pre-merge checks | ✅ 23 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (23 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryIntegrates current-main compatibility repairs across native and web test infrastructure.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains in the reviewed changes. Important Files Changed
Reviews (8): Last reviewed commit: "Merge current main before integration ga..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@web/tests/web-test-runner-isolation.test.ts`:
- Around line 105-110: Remove the fixed timeout and its associated killSignal
option from the spawnSync invocation in the test harness. Keep the existing
command, arguments, working directory, and encoding unchanged so cancellation
remains owned by the harness.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 54083722-f668-48b0-91c9-c45a5ef80835
📒 Files selected for processing (6)
.github/workflows/ci.ymlPackages/iOS/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/TerminalDockKeyboardTransitionPlannerTests.swifttests/test_ci_self_hosted_guard.shweb/package.jsonweb/scripts/run-tests.shweb/tests/web-test-runner-isolation.test.ts
|
CodeRabbit's |
…egrate-web-determinism-repairs
lawrencecchen
changed the title
|
@coderabbitai review |
✅ Action performedReview finished.
|
…egrate-web-determinism-repairs
|
@codex review |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12ec8e1168
ℹ️ 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".
|
@codex review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 `@Sources/NotificationFeedHistoryStore.swift`:
- Around line 245-250: Update the persistence loop around
consumePendingPersistenceSnapshot and
NotificationFeedHistoryPersistence.persist(_:) to remove the unnecessary await
when calling the synchronous persist method; keep the existing snapshot
consumption and finishPersistenceTask flow unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 71ef8ca4-0a88-40b9-9f09-70e1178864e0
📒 Files selected for processing (10)
Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.cPackages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.hSources/ClosedWorkspaceHistory.swiftSources/ContentView.swiftSources/Mobile/MobileTerminalRenderGridAnchorRegistry.swiftSources/NotificationFeedHistoryStore.swiftSources/TerminalController+MobileNotificationSync.swifttests/test_ci_self_hosted_guard.shweb/scripts/run-tests.shweb/tests/web-test-runner-isolation.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24cc58a1d9
ℹ️ 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".
| exit 1 | ||
| fi | ||
|
|
||
| exec bun test --isolate "${test_files[@]}" "$@" |
There was a problem hiding this comment.
When the complete web suite runs, --isolate still executes every file in one Bun process and does not prevent the repository's process-global mock.module state from contaminating later files (this behavior is also documented in web/tests/dashboard-billing-page.test.tsx). With Bun 1.2.14, bash scripts/run-tests.sh deterministically adds 10 founders-welcome-route.test.ts failures, while the previous sorted invocation has none of those failures; the new focused test passes only because it runs two hand-picked files without the earlier module loads. Since the workflow now calls this default path, web CI needs per-file processes or another mechanism that actually resets the module registry.
Useful? React with 👍 / 👎.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Exact integration head is @codex review |
|
✅ Action performedReview finished.
|
# Conflicts: # tests/test_ci_self_hosted_guard.sh # web/scripts/run-tests.sh # web/tests/web-test-runner-isolation.test.ts
|
Exact head is @codex review |
|
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Integrates seven exact reviewed heads, including repairs that could not merge independently against earlier
main:3ed4fa809fa25c5c3e6a41ed1a053f06798e6eb19bb1151f3c0e4b09c766ce57ab120103b03df96f8ad567fa96a965b4920973677dfc1f126468321e7e092af21a628ac7d7e54b8a5b599281cddb8b6bf97af18fa7a2d2a5ac06401f061b0b7d25c6557df151bd30d5dc4633643c88d221f442a26cea0fa57a26db9e779edbacbbf4f95b034bdbd05aeb685fEvery source head and current
mainare ancestors. After #8891 and #8892 landed independently, the reviewed output-order and Bun-version repairs were retained while reconciling current main. The remaining diff is 11 paths.Validation
bash tests/test_ci_self_hosted_guard.shpython3 scripts/check-test-determinism.py --strict(0 findings)swift test --package-path Packages/iOS/CmuxMobileTerminalKit --filter TerminalDockKeyboardTransitionPlannerTests(6 passed)cd web && bun run test(1,012 tests: 890 passed, 122 skipped, 0 failed) after current-main reconciliationcd web && bun run typecheckafter current-main reconciliationswift test --package-path Packages/macOS/CmuxTerminallinked and passed all 118 tests in 19 suites; SwiftPM then emitted the workflow-tolerated unexpected-binary-name diagnosticBun 1.2.14 reproduced 14 mock-isolation failures. The runner and required web job now enforce Bun 1.3.14, with behavioral rejection tests for 1.2.14 and 1.3.13.
Exact-head automated reviews and speculative merge CI are rerun after every head change.