commit fa3a3517cf4c01ca1f86ce15c0f236ef7dc569d6
parent 33d71745bac4ad981b724fe0c7bb123c8895221a
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Tue, 8 Sep 2026 08:17:50 -0400
plans: the e2e de-flake plan
Four e2e flakes seen across Phase 1's sixteen runs: the editor video-page
wrong-id delete confirmation, the two export "no FOUC" theme tests, the
ask-chat Stop-aborts-mid-sweep test, and one unattributed attribution
timeout. A read-only investigation root-caused the first three (a fixture
whose canonical video id differs from its directory name plus a reset that
mutates the tree before it quiesces the server; a reload that resolves
before the head script runs; a wall-clock route delay the test has to race).
The fourth gets a bounded repeat-each experiment rather than a speculative
fix.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
| A | plans/deflake-e2e.md | | | 117 | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ |
1 file changed, 117 insertions(+), 0 deletions(-)
diff --git a/plans/deflake-e2e.md b/plans/deflake-e2e.md
@@ -0,0 +1,117 @@
+# De-flake the e2e suites
+
+Pinned to `d1acca7` (`one-core/phase-1`). Written 2026-09-08.
+
+## Context
+
+Phase 1 of one-core shipped on `one-core/phase-1` (`7f294df` → `d1acca7`, closing e2e
+515/515). Across that job's sixteen e2e runs one editor spec reddened once without a code
+cause (`video-page.spec.ts:216`), one attribution spec timed out once in a run that was later
+killed, and memory records three export-site specs that flake on the baseline (the two
+"no FOUC" theme tests and ask-chat "Stop aborts mid-sweep"). The operator asked to fix the
+flake and any others we can. A read-only investigation (2026-09-08) root-caused three of the
+four; the fourth is unattributed and gets a bounded experiment.
+
+Branch: commit on top of `one-core/phase-1`. Nothing here touches the lane model. The same
+commits cherry-pick cleanly onto `main`, since every file involved exists there.
+
+## Findings, one per flake
+
+### 1. `editor/e2e/video-page.spec.ts:216` "Delete directory wrong-id confirmation" — root-caused, fixture + helper
+
+Failure: `pathExists(".../channels/test-youtube/data/20240101_test1234567")` is `false` at
+`:237` right after test `:196` ("Delete file") in the same file. The directory is not
+deleted, it is **renamed**: every snapshot generation calls `reconcileVideoDirs`
+(`common/controller/channelSnapshot.ts:606-611` → `common/controller/reconcileVideoDirs.ts:133-145`),
+which renames `data/<name>` to `data/<extractVideoId(webpage_url)>` when they differ. The
+fixture's `metadata.info.json` for that video has `webpage_url` ending `watch?v=test1234567`,
+the only directory in the whole fixture tree whose canonical id ≠ directory name. Test `:196`'s
+delete-file action ends with `requestChannelSnapshot` (`videoActions.ts:413`), arming the
+debounced regen; `resetData` (`editor/e2e/helpers.ts:34-63`) does `rm` + `cp` of the fixture
+FIRST and only then calls `/api/test/invalidate-cache`, so a regen that fires in the gap
+renames the freshly copied directory. The page still renders because
+`app/channels/[slug]/videos/[id]/page.tsx:44-54` swallows the readdir failure. Solo runs pass
+because nothing arms the scheduler beforehand. Product behaviour is correct; the fixture is
+inconsistent with it and the reset ordering is backwards.
+
+Fix:
+- `editor/e2e/fixtures/test-transcripts/one-youtube-channel-with-data/channels/test-youtube/data/20240101_test1234567/metadata.info.json`:
+ set `id` and `webpage_url` so the canonical id equals the directory name
+ (`watch?v=20240101_test1234567`). Do NOT rename the directory to `test1234567` —
+ `video-page.spec.ts:148` creates a different video by that id in the same channel. Nothing
+ asserts this fixture's `webpage_url` (the one `watch?v=test1234567` literal, at `:156`, is
+ the unrelated created video). Grep the editor e2e tree for any other reader of this
+ fixture's `id` before committing.
+- `editor/e2e/helpers.ts:45`: call `GET /api/test/invalidate-cache` BEFORE the `rm`, keeping
+ the existing call at `:63`. Quiesce first, then mutate the tree. Both calls are idempotent;
+ this also subsumes the ENOTEMPTY retry note at `:35-44` (update the comment).
+- `editor/e2e/helpers.ts:25-32` `fileExists`: re-throw when `err.code !== "ENOENT"` so a
+ transient errno never reads as "gone".
+- `plans/STATE.md` (the "One known flake, reported and not fixed" paragraph): the note says
+ nothing deletes the directory; correct it to "nothing deletes it, `reconcileVideoDirs`
+ renamed it" and mark it fixed.
+
+### 2. `export/e2e/theme.spec.ts:40,56` and `theme-family.spec.ts:34` "no FOUC" — test-side timing
+
+`page.reload({waitUntil: "commit"})` resolves before the inline `<head>` script in
+`common/components/ThemeScript.tsx:26-47` is guaranteed to have run, so the following
+`page.evaluate` can read `data-theme: null`. Product is fine.
+
+Fix: the script sets `d.setAttribute('data-theme-ready','1')` in the same synchronous `try`
+block (just before the `catch`); each of the three tests gates its read on
+`page.waitForFunction(() => document.documentElement.dataset.themeReady === "1")` after the
+reload. The marker and the theme attributes are set by the same statement sequence and no
+React has run before a head script, so the test still proves "set before hydration".
+
+### 3. `export/e2e/ask-chat.spec.ts:961` "corpus sweep: Stop aborts mid-sweep" — test-side timing
+
+The route mock delays batch 2 with a wall-clock `setTimeout(r, 500)` at `:985`; the test must
+click Stop (`:1027`) inside that window, and on a loaded box batch 2 lands first.
+
+Fix: replace the timer with a latch the test controls — a deferred promise declared above the
+route; the batch-2 branch awaits it; the test resolves it immediately after the Stop click at
+`:1027`. Assertions at `:1032-1035` unchanged.
+
+### 4. `editor/e2e/attribution.spec.ts:349` "Run from the video page runs that video and no other" — unattributed
+
+One timeout (≈96 s = setup + the 90 s `toContainText("1 done")` wait at `:361-364`) in a run
+later killed with exit 137; the ✘ is 54 tests before the kill, so not a kill victim, but the
+kill suppressed the reporter detail. Passed in every later full run. Ruled out: lost
+pre-hydration click (`StreamActionLog.tsx:184` disables until hydrated) and a stale ollama stub
+(port preflight aborts on a bound 11435).
+
+Bounded experiment, not a fix: run `attribution.spec.ts` alone with `--repeat-each 10` from the
+e2e worktree, editor server stdout captured to a file. If it reproduces, the stream log's last
+line says whether the unit was never dispatched (slot starvation from the previous test) or
+dispatched and never answered (stub); fix accordingly and add the finding here. If 10/10 pass,
+record it in `plans/STATE.md` as unreproduced with the evidence and stop.
+
+## Not changed, deliberately
+
+- `reconcileVideoDirs` keeps mutating during snapshot generation; that is the product's
+ design. The e2e hazard is closed by quiescing before the fixture copy, and the STATE.md note
+ names the class ("a debounced write survives a test boundary until the next reset").
+- `workers: 1`, `fullyParallel: false`, `retries: 0` stay as documented in
+ `editor/playwright.config.ts:42-72`. No `retries`, `test.slow`, or serial-mode markers are
+ added anywhere — the point is determinism, not tolerance.
+
+## Verification
+
+- `tsc --noEmit` in common and export (ThemeScript change); `pnpm --filter yt-dlp-transcript-common test`.
+- Stress the fixed specs from a worktree (the primary checkout cannot run editor e2e because
+ of the untracked `editor/content` symlink; worktree recipe in `plans/FACTS.md`'s one-core
+ sections — copy a composed fixture site into the worktree's `export/public`, e.g. from
+ `.claude/worktrees/duplicates-page/export/public`):
+ - editor: `pnpm e2e -- video-page.spec.ts --repeat-each 10` (the full file, so `:196`
+ precedes `:216` every time) — 10/10. If `--repeat-each` is swallowed after `--`, use
+ Playwright's config/CLI another way (e.g. `pnpm exec playwright test ... --repeat-each 10`
+ with the queue lock taken the way `pnpm e2e` takes it) and say which you used.
+ - editor: `pnpm e2e -- attribution.spec.ts --repeat-each 10` with server stdout captured
+ (the item-4 experiment).
+ - export: `theme.spec.ts theme-family.spec.ts ask-chat.spec.ts --repeat-each 10` — all green
+ (use the export package's own e2e entry; `e2e:2origin` is known-broken on main and is not
+ used).
+- Full editor e2e once at the end from the worktree, detached behind the queue lock, watched
+ with Monitor (not polled): expect 515/515.
+- One commit per flake (fixture+helper; theme; ask-chat; STATE.md + the item-4 record), each
+ with the stress result in its message.
