From 5d088d6c660dcb2d1ac951a9030fbca5ba5c0baa Mon Sep 17 00:00:00 2001 From: Ross Miller Date: Tue, 21 Jul 2026 09:25:38 -0500 Subject: [PATCH 1/2] test(e2e): wait for the writes journey a asserts, rather than racing them The post-merge run on master failed on generatedUpTo, expected 1 and received 0, in a test that had passed locally many times and on the pull request. It is a race in the test, not a bug in the app, and the ordering makes that unambiguous. start-book.ts streams chapter 1, saves it, generates its quiz, and only then writes generatedUpTo and status in its finalize step. Chapter 1's prose therefore reaches the screen several steps before those fields exist on disk. Reading the book once at that moment asks a question the server has not answered yet, and the only reason it ever passed is that the intervening work takes under a millisecond against fakes on an idle machine. The busier post-merge runner lost that gamble. Reproduced deterministically before fixing it, by giving chapter 1's scripted stream a per chunk delay so the marker sentence renders while the stream is still running. A bare read at that instant returns 0 every time, and a polled read returns 1. Both on-disk assertions in that test now poll. Polling waits for a write on a deadline exactly the way a web-first assertion waits for the screen, and it weakens nothing, because the expected values are unchanged and a value that never arrives still fails. The revised table of contents assertion gets the same treatment for the same reason, since the parse and the save both happen after that stream ends too, and it was one busy runner away from the same failure. Every other on-disk assertion in the suite was checked and left alone. Each one already waits on a signal the server produces after the write it cares about, a reload, a refetched list, a dialog that only closes on success, or a recorded model request, so a bare read there is both correct and the stronger claim. No retries were added. A retry would have turned this into a green run and taught us nothing. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5 --- e2e/journeys/create-book.spec.ts | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/e2e/journeys/create-book.spec.ts b/e2e/journeys/create-book.spec.ts index 608959c..51734e3 100644 --- a/e2e/journeys/create-book.spec.ts +++ b/e2e/journeys/create-book.spec.ts @@ -97,16 +97,28 @@ test('revises the table of contents, persists it, and generates chapter 1 on app // Persistence, checked on disk rather than on screen: the revision wrote // through to toc.yml, not just into the streaming buffer the page renders. - const toc = await bookRepository(app.dataDir).getToc(id) - expect(toc.chapters.map(chapter => chapter.title)).toEqual(TOC_REVISED_CHAPTERS.map(chapter => chapter.title)) + // + // Polled rather than read once, because the two are not simultaneous. + // Titles appear on screen while `revise-toc.ts` is still streaming, and the + // parse and the save both happen after the stream ends, so a bare read here + // races a write that is strictly later. Polling waits for that write on a + // deadline, the same way a web-first assertion waits for the screen, and it + // weakens nothing because the expected value is unchanged. + await expect.poll(async () => (await bookRepository(app.dataDir).getToc(id)).chapters.map(chapter => chapter.title)) + .toEqual(TOC_REVISED_CHAPTERS.map(chapter => chapter.title)) await wizard(page).approveToc() await expect(page.getByText(chapterMarker(1)).first()).toBeVisible() - const book = await readBook(app.dataDir, id) - expect(book.generatedUpTo).toBe(1) - expect(book.status).toBe('reading') + // Same reasoning, and this is the one that actually caught it. `start-book.ts` + // streams the chapter, saves it, generates its quiz, and only then writes + // generatedUpTo and status in its finalize step. Chapter 1's prose is on + // screen well before that, so reading the book once at this point asks a + // question the server has not answered yet. It won on every local run and on + // the pull request, and lost on the busier post-merge runner. + await expect.poll(async () => (await readBook(app.dataDir, id)).generatedUpTo).toBe(1) + expect((await readBook(app.dataDir, id)).status).toBe('reading') // And the model path /start actually drives: a skill classification call // and a chapter-1 stream, not just the revise-toc stream from earlier. From 08aa80a3a92d39b4f2b7d3d6cb100b88474e5379 Mon Sep 17 00:00:00 2001 From: Ross Miller Date: Tue, 21 Jul 2026 09:28:53 -0500 Subject: [PATCH 2/2] ci(e2e): repair the electron binary the shared store cache can drop The electron job failed with "Electron failed to install correctly" on a run whose only change was a test assertion, having passed twice before on identical workflow code. That pattern is the tell for a cache race rather than for anything in the diff. All three jobs share one pnpm store cache keyed on the lockfile, and two of them set ELECTRON_SKIP_BINARY_DOWNLOAD because they never launch Electron. Whichever of those saves the store first can hand this job an electron package whose postinstall was skipped and whose binary is therefore absent. Which job wins that write varies run to run, so the failure reads as flake even though the cause is deterministic once you know it is there. The job now runs the electron package's own postinstall after installing. It checks the installed version first and downloads only what is missing, reusing the binary cache the step above restores, so it is idempotent and costs nothing when the store cache was already clean. Verified locally that a second run against a complete install exits zero without downloading. Repairing here rather than dropping the skip flag from the other two jobs, because those jobs genuinely have no use for a ninety megabyte binary and making them pay for it to protect a third job would be the wrong trade. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5 --- .github/workflows/ci.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe5b8f2..bd28061 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -108,6 +108,21 @@ jobs: - run: pnpm install --frozen-lockfile + # The other two jobs set ELECTRON_SKIP_BINARY_DOWNLOAD, and all three + # share one pnpm store cache keyed on the lockfile, so whichever of them + # saves that cache first can hand this job an electron package whose + # postinstall was skipped and whose binary is therefore missing. The + # symptom is "Electron failed to install correctly" out of + # electron.launch, and because it depends on which parallel job wins the + # cache write, it appears as flake rather than as a broken job. + # + # install.js is the package's own postinstall. It checks the installed + # version first and downloads only what is missing, reusing the binary + # cache restored above, so running it here is idempotent and costs + # nothing when the store cache was clean. + - name: Repair the Electron binary if the shared store cache skipped it + run: node node_modules/electron/install.js + - run: pnpm e2e --project=electron - uses: actions/upload-artifact@v4