Repository navigation
Give the document-entry visit test an account of its own - #787
Merged
Merged
Conversation
The test that checks a parent renamed while its child closes still publishes the parent as the visited document timed out waiting for GET /api/navigation to report the parent as lastDocumentId (#783). On CI it failed all three Playwright attempts of a job and passed on a re-run; locally it failed in 2 of 2 runs of the file and in 4 of 30 repeats of it, every time with some other document as lastDocumentId. The last visited document is one row per account, and PATCH /api/navigation replaces it, last write wins, from any page of that account. The test signed in as ana, as do the other three tests of its file and eleven other spec files, and the tests of one run share one server and one database, so a page another test opened as ana could record a visit between the parent's visit and the test's poll. Logging every committed visit on the server showed the test's own page recording its child and then the parent, in that order, and the first test of the file recording its child about a second later as the last visit. The test's own trace had exactly two PATCH requests; navigation reads between them returned the child and then the parent, and every read of the poll returned a document this page never visited. Nothing in the product misorders a tab's visits (the client chains them), and a person's last visit being whichever tab visited last is the design, so nothing there changes. The test now signs in as ana-<its room id>, an account no other test uses, as document-creation and hosted already do for the tests that assert on one account's navigation. Its assertions are untouched: with the server made to ignore visits to parent documents, it still fails, 3 of 3, at the same poll. Fixes #783 Assistant-model: Claude Sonnet 5.5 Assistant-workflow: goal (run 4545dc40-39f5-4e69-8642-76e0fee3e70d) Assistant-duration: 28m converged, estimated 1h Assistant-verification: red before fix passed: E2E_SKIP_BUILD=1 bun run e2e e2e/document-entry.e2e.ts on main failed the visit test at its lastDocumentId poll in 2 of 2 runs, and --repeat-each=30 failed it 4 of 30 while the file's other three tests passed 90 of 90, each time with another test's document as lastDocumentId Assistant-verification: server visit log passed: with temporary logging of each committed PATCH /api/navigation (reverted), the test's child then parent visits committed in order and a later visit by the file's first test became the last record for ana Assistant-verification: E2E_SKIP_BUILD=1 bun run e2e e2e/document-entry.e2e.ts --repeat-each=30 passed after the change: 120 of 120 with 4 workers, against 4 failures in 120 before it Assistant-verification: taskset -c 0,1 bun run e2e e2e/document-entry.e2e.ts --repeat-each=30 passed: 120 of 120 pinned to two CPU cores Assistant-verification: CI=true bun run e2e --shard=11/12 --workers=2 --reporter=github,blob,list,./e2e/shard-balance.ts first attempt failed: 67 passed, and 2 machines.e2e.ts tests failed at git commit in their fixture repository because the local global git config signs commits with a key defined only under a work directory; the document-entry tests passed Assistant-verification: GIT_CONFIG_GLOBAL=/dev/null CI=true bun run e2e --shard=11/12 --workers=2 --reporter=github,blob,list,./e2e/shard-balance.ts passed: the 69 tests of the shard that holds document-entry, none retried Assistant-verification: mutation check passed: with the server ignoring visits to parent documents, the visit test failed 3 of 3 at the same poll; restored, it passes Assistant-verification: bun run fix passed: no change beyond this edit, 72 existing lint warnings, none in the file Assistant-verification: bun run types passed: whole workspace and e2e Assistant-verification: bun run ci passed: dprint, oxlint (72 existing warnings, 0 errors), token and design checks Assistant-verification: bun test unavailable: not run, no product code changed Assistant-verification: bun run db:up unavailable: port 5432 was taken by another container, and bun run e2e starts its own four databases Co-authored-by: Alex Lavaee <lavaman131@github.com>
lavaman131
force-pushed
the
fix-783-document-entry-flake
branch
from
November 5, 2026 00:58
6fdf10d to
044922f
Compare
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.
Summary
e2e/document-entry.e2e.ts› "a parent rename while its child closes publishes the parent as the visited document" no longer shares its account with the rest of the suite. It signs in asana-<its room id>instead ofana, so no other test's page can overwrite the last visited document it polls for. The product is unchanged.Cause
The test ends by polling
GET /api/navigationuntillastDocumentIdis the parent. That value is one row per account (user_navigation.last_document_id), andPATCH /api/navigationreplaces it, last write wins, from any page of the account. The test signed in asana, as do the file's other three tests and eleven other spec files, and the tests of one run share one server and one database. A page another test opens asanarecords a visit of its own, and if it lands between the parent's visit and the poll's first read, the poll never sees the parent: it reads the other test's document until the 10 s timeout.Evidence, from an unfixed checkout of
main:bun run e2e e2e/document-entry.e2e.ts(4 workers) failed this test at thelastDocumentIdpoll in 2 of 2 runs, withReceiveda document other thanroom, exactly as in the CI log (job 126809564779, shard 11/12, all three attempts).--repeat-each=30failed it 4 of 30; the file's other three tests passed 90 of 90.The failing run's Playwright trace has exactly two
PATCH /api/navigationrequests from the page: its child, thenroom. Navigation reads between them returned the child, then the parent; every read of the poll afterwards returned a document the page never visited.Temporary server logging of each committed visit (not in the diff) for one failing run, all for
anaon the same server:1400ef89(child ofe814ebe3)b6819b74e814ebe3(the parent)4780ae72b6819b74(renamed)d3bb39d8("Source review a2d1b0f5")The poll expected
e814ebe3and receivedd3bb39d8.The test's own visits are recorded in the right order (the client chains them through
visitTail), and a person's last visit being whichever tab visited last is the intended behaviour, so there is no product race to fix. It is an isolation defect in the test: it asserts on account-wide state that its neighbours write.Fix
One line in the test:
authenticate(page, `ana-${room}`, baseURL!), with a comment saying why it is not the sharedana.document-creationandhostedalready use a handle per test for the same reason. The assertions are untouched, so the test still catches a wrong last visit: with the server temporarily made to ignore visits to parent documents, it fails 3 of 3 at the same poll.Verification
All on this branch's head unless noted.
E2E_SKIP_BUILD=1 bun run e2e e2e/document-entry.e2e.ts --repeat-each=30: 120 of 120 pass (4 workers, production client built with the child-motion toggle).taskset -c 0,1: 120 of 120 pass, 6.5 min.document-entry; shard assignment frome2e/shard-balance.tsandshard-timings.json):CI=true bun run e2e --shard=11/12 --workers=2 --reporter=github,blob,list,./e2e/shard-balance.ts. The first attempt had 67 passes and 2machines.e2e.tsfailures atgit commitin their fixture repository, because this machine's global git config signs commits with a key defined only under a work directory (CI has none). WithGIT_CONFIG_GLOBAL=/dev/null: 69 of 69 pass, none retried.bun run fixchanges nothing beyond the edit (72 existing lint warnings, none in the file);bun run typesandbun run cipass.bun testwas not run: no product code changed.bun run db:upcould not start because port 5432 was taken by another container;bun run e2estarts its own four databases.Not reproduced: CI's three failed attempts in a row. The mechanism explains them (each attempt has the same neighbour worker opening documents as
ana), but I only reproduced the failure locally at the rates above.Fixes #783
Assistant-model: Claude Sonnet 5.5
Assistant-workflow: goal (run 4545dc40-39f5-4e69-8642-76e0fee3e70d)
Assistant-duration: 28m converged, estimated 1h
Assistant-verification: red before fix passed: E2E_SKIP_BUILD=1 bun run e2e e2e/document-entry.e2e.ts on main failed the visit test at its lastDocumentId poll in 2 of 2 runs, and --repeat-each=30 failed it 4 of 30 while the file's other three tests passed 90 of 90, each time with another test's document as lastDocumentId
Assistant-verification: server visit log passed: with temporary logging of each committed PATCH /api/navigation (reverted), the test's child then parent visits committed in order and a later visit by the file's first test became the last record for ana
Assistant-verification: E2E_SKIP_BUILD=1 bun run e2e e2e/document-entry.e2e.ts --repeat-each=30 passed after the change: 120 of 120 with 4 workers, against 4 failures in 120 before it
Assistant-verification: taskset -c 0,1 bun run e2e e2e/document-entry.e2e.ts --repeat-each=30 passed: 120 of 120 pinned to two CPU cores
Assistant-verification: CI=true bun run e2e --shard=11/12 --workers=2 --reporter=github,blob,list,./e2e/shard-balance.ts first attempt failed: 67 passed, and 2 machines.e2e.ts tests failed at git commit in their fixture repository because the local global git config signs commits with a key defined only under a work directory; the document-entry tests passed
Assistant-verification: GIT_CONFIG_GLOBAL=/dev/null CI=true bun run e2e --shard=11/12 --workers=2 --reporter=github,blob,list,./e2e/shard-balance.ts passed: the 69 tests of the shard that holds document-entry, none retried
Assistant-verification: mutation check passed: with the server ignoring visits to parent documents, the visit test failed 3 of 3 at the same poll; restored, it passes
Assistant-verification: bun run fix passed: no change beyond this edit, 72 existing lint warnings, none in the file
Assistant-verification: bun run types passed: whole workspace and e2e
Assistant-verification: bun run ci passed: dprint, oxlint (72 existing warnings, 0 errors), token and design checks
Assistant-verification: bun test unavailable: not run, no product code changed
Assistant-verification: bun run db:up unavailable: port 5432 was taken by another container, and bun run e2e starts its own four databases
Co-authored-by: Alex Lavaee lavaman131@github.com