fix(extension): resolve wait_until=commit when the readyState probe fails#34
Open
hobostay wants to merge 1 commit into
Open
fix(extension): resolve wait_until=commit when the readyState probe fails#34hobostay wants to merge 1 commit into
hobostay wants to merge 1 commit into
Conversation
handleNavigate's `Page.frameNavigated` handler called
`acceptLifecycleEvent("commit", frameId)` without the frame's loaderId.
Once `Page.navigate` resolves, the loader guard always carries the new
loaderId, so `eventLoaderIsRelevant(undefined, guard)` rejected the
commit event outright.
The navigation still completed in the common case only because the
readyState probe short-circuited first. On cross-origin navigations the
probe returns null (the old execution context has been destroyed), so
the wait fell back to `frameNavigated` — which was then rejected — and
`wait_until="commit"` timed out for the full `timeout_ms` instead of
resolving on commit.
Pass `p.frame?.loaderId` through, matching the sibling `lifecycleEvent`
branches. Adds a regression test that makes the readyState probe fail
and asserts commit is reached from the `frameNavigated` event; it fails
(reached "timeout") on the previous code.
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
handleNavigatewithwait_until: "commit"can time out for the fulltimeout_msinstead of resolving on commit, on cross-origin navigations.The
Page.frameNavigatedhandler calledacceptLifecycleEventwithout the frame'sloaderId:Once
Page.navigateresolves, the loader guard always carries the newloaderId, soeventLoaderIsRelevant(undefined, guard)hits theexpected.length > 0 && !eventLoaderIdbranch and rejects the commit event outright (every time).This was masked in the common case because the
document.readyStateprobe short-circuits first (lifecycleAlreadyReached("loading" | "interactive", "commit")istrue). But on a cross-origin navigation the probe returnsnull— the old execution context is destroyed, so theRuntime.evaluatethrows — and the wait falls back toPage.frameNavigated, which is then rejected. Result: the commit wait runs totimeout_ms(default 30s).Fix
Pass the frame's
loaderIdthrough, exactly as the siblingPage.lifecycleEventbranches already do (navigation.tslines that callacceptLifecycleEvent(p.name, p.frameId, p.loaderId)):Test
Adds a regression test that makes the readyState probe throw (returns
null, simulating the destroyed old execution context of a cross-origin nav) and assertswait_until="commit"is reached from thePage.frameNavigatedevent. This test fails on the previous code (reached: "timeout") and passes with the fix. The existing commit test is unaffected (it still resolves via the probe).Validation
Frontend CI gates run locally against this branch:
pnpm ext:test— 388 passed ✅pnpm --filter @browser-skill/extension compile(tsc --noEmit) ✅pnpm lint(biome + stylelint) ✅pnpm ext:build✅🤖 Generated with Claude Code