Fix #1972: resync terminal portal after restore-time bind#1973
Fix #1972: resync terminal portal after restore-time bind#1973austinywang merged 2 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
@austinywang I have started the AI code review. It will take a few minutes to complete. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request adds a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
|
To use Codex here, create a Codex account and connect to github. |
1 similar comment
|
To use Codex here, create a Codex account and connect to github. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes #1972 by adding a single Key changes:
The fix is minimal and the double-hop mechanics are already well-tested by the existing sync test suite. No regressions are expected because Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant App as App / Restore
participant Portal as WindowTerminalPortal
participant MQ1 as Main Queue (hop 1)
participant MQ2 as Main Queue (hop 2)
App->>Portal: bind(hostedView:to:anchor)
Portal->>Portal: synchronizeHostedView() — seeds frame
Portal->>Portal: scheduleDeferredFullSynchronizeAll()
Portal->>MQ1: scheduleExternalGeometrySynchronize() [NEW]<br/>enqueue outer block
App->>MQ1: sidebar/split layout shift (queued after bind returns)
note over MQ1: Main queue processes in order
MQ1->>MQ2: outer block fires → hasExternalGeometrySyncScheduled=false<br/>enqueue performSync (hop 2)
MQ1->>Portal: ancestor layout shift applied (frame.origin.x += Δ)
MQ2->>Portal: performSync fires<br/>synchronizeAllEntriesFromExternalGeometryChange()
Portal->>Portal: portal frame updated to settled geometry ✅
Reviews (1): Last reviewed commit: "fix: resync terminal portal after restor..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| window.displayIfNeeded() | ||
| } | ||
|
|
||
| RunLoop.current.run(until: Date().addingTimeInterval(0.05)) |
There was a problem hiding this comment.
RunLoop.current.run(until: Date().addingTimeInterval(0.05)) has to drain three async hops in 50 ms (outer block from scheduleExternalGeometrySynchronize → inner performSync block → the queued layout-shift block). On a heavily loaded CI runner the budget can be tight.
The pre-existing testScheduledExternalGeometrySyncWaitsForQueuedLayoutShift uses the same 50 ms pattern and is presumably passing, so this is not a new risk — but if either test ever becomes intermittent, the fix is two sequential drainMainQueue() calls instead of the wall-clock sleep:
// instead of RunLoop.current.run(until: Date().addingTimeInterval(0.05))
drainMainQueue() // fires hop-1 → queues hop-2; layout-shift block also dispatched by now
drainMainQueue() // fires hop-2 (performSync) — portal is now at settled geometryNo action required unless CI flakiness is observed.
…anaflow-ai#1973) * test: cover queued restore-time terminal portal shift * fix: resync terminal portal after restore-time bind
1 participant
Summary
Testing
testBindQueuesExternalGeometrySyncForQueuedLayoutShiftincmuxTests/TerminalAndGhosttyTests.swift../scripts/reload.sh --tag issue-1972successfully.Demo Video
For UI or behavior changes, include a short demo video (GitHub upload, Loom, or other direct link).
Review Trigger (Copy/Paste as PR comment)
Checklist
Summary by CodeRabbit
Release Notes