fix(wait): settle --idle on held-open streams; report settled URL in target#5
Merged
Merged
Conversation
wait --idle counted requestWillBeSent up and loadingFinished/Failed down and required inflight == 0. A websocket / long-poll / EventSource request fires requestWillBeSent but never loadingFinished, so inflight never returned to zero and --idle hung until --timeout on SPAs that hold such a stream open (observed on Workday: a 20s timeout on an already-loaded page). Settle when EITHER no requests are in flight for the window (the clean path, unchanged for pages whose requests all complete) OR requests remain open but the connection has been silent for ~2s (the stalled path). Response/data events reset the silence clock, so an in-progress download is never mistaken for a stalled stream — only a genuinely quiet still-open request settles. Adds TestWaitIdleStalledStream (a held-open request settles via the stalled path in ~2s instead of hanging); TestWaitIdle still guards the clean path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The envelope's target was captured before the action ran, so after a nav or a --url / redirect wait it showed the pre-navigation URL — misleading (a wait --url that matched could echo a target URL that didn't even contain the matched substring). Wait now reads the tab's location after the condition holds and returns it as result.url; targetAction syncs target.url from any result carrying a url key (nav already returned it). No extra round-trips. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-up (/simplify + /deslop) — no behavior change: - waitIdle tracked two clocks (lastZero + lastActivity); lastZero is derivable (it only diverged from lastActivity in the pre-Enable in-flight-request edge, where a single clock is actually more correct — it respects active bytes instead of settling early). Collapse to one lastActivity clock and select the threshold by whether requests are in flight: `window` when idle, `idleStall` when a stream is held open. Drops a field, a branch, and one idle predicate. - Dedup the "held-open stream / not-a-stalled-download" rationale that was stated three times (doc comment + two inline comments) down to once. Live idle tests (clean + stalled paths) and the full race suite stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <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
1 participant
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.
Two wait/nav accuracy fixes surfaced while driving Workday over CDP.
1.
wait --idlehung on persistent-connection SPAswaitIdlerequiredinflight == 0(requestWillBeSent up, loadingFinished/Failed down). A websocket / long-poll / EventSource request firesrequestWillBeSentbut neverloadingFinished, soinflightnever returned to zero and--idlewaited out the whole--timeout. Observed live: a 20s timeout on an already-loaded Workday page.Now settles when either:
responseReceived/dataReceivedreset the silence clock, so an in-progress download is never mistaken for a stalled stream — only a genuinely quiet still-open request settles.2. Envelope
target.urlwas the pre-action URLThe target was captured before the action, so after a
navor--url/redirectwaitit echoed the old URL — await --urlthat matched could even report a target URL not containing the matched substring.Waitnow reads the settled location and returns it;targetActionsyncstarget.urlfrom any result carrying aurlkey (navalready returned it). No extra round-trips.Tests
TestWaitIdleStalledStream— a held-open request settles via the stalled path in ~2s instead of hanging (teardown ordered so Chrome closes before the httptest server, avoiding a/hangdeadlock).TestWaitIdle— still guards the clean path (post-load fetch completes before idle returns).TestNavReportsSettledURL/TestWaitReportsSettledURL— target reflects the settled URL.Verification
gofmt -lclean ·go vet ./...clean ·go test -race ./...(full, live Chrome, 300s bound) green.🤖 Generated with Claude Code