Conversation
… new tab On the web client a pane pointed at a loopback address stayed on the new-tab page forever. The framable probe refuses every loopback by design (its SSRF guard), so there is no iframe; the streaming fallback that should have taken over is the seed in RemoteBrowserPanel, and it refused loopback wholesale. The pane held the right url in its store and navigated nowhere. The seed now reads the url the pane IS on (knownPaneUrl: a chat-opened pane gets its url through the pane store, so initialUrl alone missed exactly this case) and, for loopback, asks /api/browsers/port-listening first. The port answers and the pane loads it through the server, which runs on the same machine and can reach it; nothing answers and the pane says which port is dead and when it looked. The SSRF guard is untouched. Covers BROWSER-CHAT-04.
Two things kept the pane dead after the first fix. The guard: 'not blank' is not the same as 'already loaded'. browser.url also holds the url the pane was OPENED on, and a chat-opened pane reads its own target back within milliseconds, so the post-probe re-check skipped the navigation in exactly this card's case and left the pane on a context the server had never been told to load. Only a DIFFERENT page now counts as someone having got there first. The test: it asserted on the pane's innerText, which is empty by construction because rrweb rebuilds the page in a same-origin mirror iframe; and it paid for Chromium's cold start inside its own budget, which made every run fail the first attempt and pass the retry. The marker is now read through the mirror iframe, and the browser is launched in a beforeAll hook. 2 passed in 37s, no retry.
check:bloat went red on this branch for a file it does not own: server/services/dispatch-capacity.test.ts at 810 lines, ten past the ceiling, carried in by the merge of card f5ba4d47 together with server.ts, task-dispatcher.ts and routes/tasks.ts. Recorded instead of split: cutting up another card's test to unblock this one would move its coupling, not remove it. The note in the baseline says which entries came from where, so the next reader is not left guessing.
Both files had already learned this lesson and both had one wait left behind. projectLayoutSync.durability scales BUDGET_MS by the factor the runner measures but settle() was still a flat 2500 ms, so under a fleet the 500 ms debounce could still be pending when it elapsed. tabLink's settleUntil() ceiling was a flat 2000 ms, and it gives up SILENTLY, so the retry chain was still in flight when the assertion read the state. Five tests went red that way in the sharded round on card 30f55ca9 (13m23, plan already reduced), on a diff that loads none of them, and all 74 pass run alone. Forcing TOPICS_TEST_TIME_SLACK=4 widens the run 13.6s -> 20.5s, which is the scaling actually taking effect.
…k too Third file of the same family, same shape: waitFor() polls for the push but its ceiling was a flat 10 s, and it gives up SILENTLY, so when the machine is slow the assertion reads a push that never landed. That is how 'one edit is one PUT' went red in the sharded round on card 30f55ca9, on a diff that does not load this file, green on its own. The per-test ceiling moves with it: a widened wait under an unchanged 20 s ceiling would only trade a clean red for a timeout. Note the '?? 1': parseForcedSlack answers null when the env is unset, and null multiplied is zero, which makes the wait return instantly and turns the whole file red. Caught here, and the two files of the previous commit already had the fallback.
# Conflicts: # scripts/bloat-baseline.json
The seed's guard was wrong in the way review caught: needsSeed() treated current === seedUrl as 'not loaded yet', and after onUrlChange persists the pane's url those two strings are ALWAYS equal. So a web-client reload, or a second device opening the same pane, re-navigated a context already sitting on that page. navigate() has no same-url shortcut and the server does page.goto: a real reload, discarding scroll, a half-filled form and session state, for every viewer of a shared session. The discriminant now comes from the server: /api/browsers/:id answers 404 until the context exists and reports its real url once it does. 404 or a blank url means seed it; any real url means somebody is there, INCLUDING the case where that url is the one we were about to load. When the question cannot be answered the seed stands down, because a skipped seed is recoverable from the address bar and a reload under someone's hands is not. The rule was never written down, which is why it could regress in silence: BROWSER-CHAT-04 now carries it as a scenario, and contextHasPage.test.ts pins the three cases plus the two failure modes. The seed moved into useSeedPaneUrl.ts with its helpers, so RemoteBrowserPanel.tsx goes 1529 -> 1411, below the 1451 it had before this branch. The bloat baseline changes by exactly that one line instead of being regenerated whole, and the e2e no longer accepts 'error' as an outcome on a port this test keeps alive.
…ning its baseline canSeedUrl for isSeedableUrl (seedable is not a word) and stopped for cancelled. Moving the seed into its own file made both of them NEW declarations, which is exactly what the gate watches for, and the right answer to that is a better name and not an extra line in the baseline. The baseline still changes by one line, in the direction that costs nothing: isSeedableUrl leaves RemoteBrowserPanel's list because the name genuinely no longer exists there.
# Conflicts: # scripts/bloat-baseline.json
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.
Draft opened by the Topics board for commit fb134d4 of card 30f55ca9, to read the pull request CI (e2e jobs, unit tests). Not a request to merge: a person marks it ready when the card lands.