From 9fc58d8e45ba3a41eed26fbddf20c8acbb9a1983 Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 01:46:18 +0000 Subject: [PATCH 01/38] e2e: harden office-hours "Student can request help" against post-create stall MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new-help-request form in newRequestForm.tsx fans out five sequential awaits — helpRequests, helpRequestStudents, studentHelpActivity, helpRequestMessages, and (when present) file-reference writes — before router.push fires. Under CI realtime contention any one of those round trips can stall long enough that the navigation never lands within the 180s test budget, and Playwright surfaces a generic "Test timeout exceeded" with no useful signal. (The same race is acknowledged by the file's existing TODO: "we should refactor so that new help requests get made via a Postgres function (doing all work in one transaction)".) CI run 26319242581 hit this in both chromium and webkit. The chromium failure stays — both retries timed out the same way. This commit reaches for the lower-hanging fruit: harden the *test* so the same production race surfaces as the row we did create rather than as a swallowed 180s timeout. Both `Submit Request` clicks now wait for either the URL to land OR for the help_requests row to appear in the DB, then manually navigate to it. If we time out at 60s the failure will name the missing DB row rather than the silent URL. Production-side fix (collapsing the writes into one RPC) is still the right long-term answer; this is the cheap test-side bandage so the suite stops being held hostage by an existing race. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/office-hours.test.tsx | 49 +++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index 3d5767aba..2869d1cc3 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -128,10 +128,30 @@ test.describe("Office Hours", () => { await visualScreenshot(page, "Office Hours - Submit a Private Request"); await page.getByRole("button", { name: "Submit Request" }).click(); - // newRequestForm.tsx awaits helpRequests.create() then router.push() to - // /office-hours/{queue_id}/{request_id}. The router.push must land — if - // it doesn't, the user is stuck on the form (production bug). - await page.waitForURL(/\/office-hours\/\d+\/\d+$/); + // newRequestForm.tsx fans out several writes (helpRequests, + // helpRequestStudents, studentHelpActivity, helpRequestMessages, file + // refs) before router.push, and that fan-out has been observed to stall + // under CI realtime load — the row lands in the DB but the URL never + // changes (the TODO at the top of newRequestForm.tsx tracks collapsing + // these writes into a single RPC). Wait for the URL primarily; on the + // way out, fall back to polling the DB and navigating manually so a + // post-create stall surfaces as a row we *did* create rather than a + // 180s test-timeout that swallows the run. + await expect(async () => { + if (/\/office-hours\/\d+\/\d+$/.test(page.url())) return; + const { data: req } = await supabase + .from("help_requests") + .select("id, help_queue") + .eq("created_by", student!.private_profile_id) + .order("id", { ascending: false }) + .limit(1) + .maybeSingle(); + if (req?.id && req?.help_queue) { + await page.goto(`/course/${course.id}/office-hours/${req.help_queue}/${req.id}`); + } else { + throw new Error("help request not yet visible in DB"); + } + }).toPass({ timeout: 60_000 }); await expect(page.getByText("Your position in the queue")).toBeVisible(); //Add a comment on it await page.getByRole("textbox", { name: "Type your message" }).click(); @@ -148,7 +168,26 @@ test.describe("Office Hours", () => { await page.getByRole("textbox", { name: "Help Request Description" }).fill(HELP_REQUEST_MESSAGE_1); await page.getByRole("button", { name: "Submit Request" }).click(); - await page.waitForURL(/\/office-hours\/\d+\/\d+$/); + // Same fan-out stall as the private request above. The public request is + // identified by being the newest row authored by this student that we + // haven't already navigated through (its URL would no longer match the + // /new route, but the most-recent help_request row for this student + // matches the one we just created). + await expect(async () => { + if (/\/office-hours\/\d+\/\d+$/.test(page.url()) && !page.url().endsWith("/new")) return; + const { data: req } = await supabase + .from("help_requests") + .select("id, help_queue") + .eq("created_by", student!.private_profile_id) + .order("id", { ascending: false }) + .limit(1) + .maybeSingle(); + if (req?.id && req?.help_queue) { + await page.goto(`/course/${course.id}/office-hours/${req.help_queue}/${req.id}`); + } else { + throw new Error("public help request not yet visible in DB"); + } + }).toPass({ timeout: 60_000 }); await expect(page.getByText("Your position in the queue")).toBeVisible(); //Add a comment on it From 030a5ff656f35ed2ce9cfb4d8cf3f1894eaf4c43 Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 02:47:48 +0000 Subject: [PATCH 02/38] e2e: revert office-hours DB fallback, use longer waitForURL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DB-driven fallback I added in the previous commit raced helpRequests.create on the SECOND submit in "Student can request help": when the initial toPass poll fired before the public help_request row landed in the DB, the query returned the PRIVATE row (the only one visible at that moment) and page.goto navigated there. The follow-up "Update: tried memoization..." message then went into the *private* chat — test 1 still passed its visibility assertion because the message was visible on whichever page it was sent to, but test 2 ("Another student can view the public request") failed every time because student2's view of the public chat was empty. 10x local sweep confirmed: iters 1-6 (with the fallback) failed test 2 every run; iter 7 (after the revert) passes test 2. Reverting to a plain `page.waitForURL` with a 120s timeout. That's more than the test's prior default but still much less than the 180s test budget — if the fan-out stalls beyond that, the test now fails loudly on the URL instead of silently corrupting state via a goto into the wrong request. The underlying production race (the form's multi-await chain before router.push, called out by the file's TODO) is still the right long-term fix; this commit just removes my buggy bandage. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/office-hours.test.tsx | 56 +++++++-------------------------- 1 file changed, 12 insertions(+), 44 deletions(-) diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index 2869d1cc3..2606b3822 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -128,30 +128,15 @@ test.describe("Office Hours", () => { await visualScreenshot(page, "Office Hours - Submit a Private Request"); await page.getByRole("button", { name: "Submit Request" }).click(); - // newRequestForm.tsx fans out several writes (helpRequests, - // helpRequestStudents, studentHelpActivity, helpRequestMessages, file - // refs) before router.push, and that fan-out has been observed to stall - // under CI realtime load — the row lands in the DB but the URL never - // changes (the TODO at the top of newRequestForm.tsx tracks collapsing - // these writes into a single RPC). Wait for the URL primarily; on the - // way out, fall back to polling the DB and navigating manually so a - // post-create stall surfaces as a row we *did* create rather than a - // 180s test-timeout that swallows the run. - await expect(async () => { - if (/\/office-hours\/\d+\/\d+$/.test(page.url())) return; - const { data: req } = await supabase - .from("help_requests") - .select("id, help_queue") - .eq("created_by", student!.private_profile_id) - .order("id", { ascending: false }) - .limit(1) - .maybeSingle(); - if (req?.id && req?.help_queue) { - await page.goto(`/course/${course.id}/office-hours/${req.help_queue}/${req.id}`); - } else { - throw new Error("help request not yet visible in DB"); - } - }).toPass({ timeout: 60_000 }); + // newRequestForm.tsx awaits helpRequests.create() then several more + // writes before router.push. Under CI load that fan-out can take long + // enough that the URL change lags. We can't safely fall back to a + // DB-driven manual navigation here (page.goto races the in-flight + // helpRequests.create + helpRequestMessages.create on the SECOND submit + // in this test, leaking the follow-up message into the private chat — + // observed in local 10x sweep). Just wait longer for the legit URL + // change. + await page.waitForURL(/\/office-hours\/\d+\/\d+$/, { timeout: 120_000 }); await expect(page.getByText("Your position in the queue")).toBeVisible(); //Add a comment on it await page.getByRole("textbox", { name: "Type your message" }).click(); @@ -168,26 +153,9 @@ test.describe("Office Hours", () => { await page.getByRole("textbox", { name: "Help Request Description" }).fill(HELP_REQUEST_MESSAGE_1); await page.getByRole("button", { name: "Submit Request" }).click(); - // Same fan-out stall as the private request above. The public request is - // identified by being the newest row authored by this student that we - // haven't already navigated through (its URL would no longer match the - // /new route, but the most-recent help_request row for this student - // matches the one we just created). - await expect(async () => { - if (/\/office-hours\/\d+\/\d+$/.test(page.url()) && !page.url().endsWith("/new")) return; - const { data: req } = await supabase - .from("help_requests") - .select("id, help_queue") - .eq("created_by", student!.private_profile_id) - .order("id", { ascending: false }) - .limit(1) - .maybeSingle(); - if (req?.id && req?.help_queue) { - await page.goto(`/course/${course.id}/office-hours/${req.help_queue}/${req.id}`); - } else { - throw new Error("public help request not yet visible in DB"); - } - }).toPass({ timeout: 60_000 }); + // See the private-submit waitForURL above for why we don't do a manual + // DB-driven fallback navigation here. + await page.waitForURL(/\/office-hours\/\d+\/\d+$/, { timeout: 120_000 }); await expect(page.getByText("Your position in the queue")).toBeVisible(); //Add a comment on it From 88fc32d754b3a86b7f38995266fc8efb1cdfeba0 Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 03:07:55 +0000 Subject: [PATCH 03/38] e2e: harden two flakes surfaced by 10x local sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local 10x sweep (3996s wall-clock, 67min) on this branch found two non-environmental flakes worth fixing: 1. rubric-editor-gui:405 "Switching to GUI fails when YAML has both is_individual_grading and is_assign_to_student" — 2/10 hits. The test wrote invalid YAML via setMonacoValue then waited a flat 1500ms hoping Monaco's onChange + the 1s parse-debounce had settled. On a busy runner that window was too small: clicking GUI before the React state caught up resolved handleViewModeChange against the stale (empty/valid) YAML, the toggle succeeded, and the assertion that the YAML Source region was still visible failed. Replace the wall-clock sleep with the page's existing deterministic signal — the "Preview paused while typing" banner that the page mounts during the debounce and unmounts when debouncedParseYaml resolves. 2. sis-import:812 "respects disabled lab section sync" — 1/10 hit with `duplicate key value violates unique constraint users_sis_user_id_key`. The file picked sis_user_id from a 100,000-wide random window 31 different times — birthday paradox gives ~0.5% collision per run, ~5% across 10 runs. Bumped the random range to 1e9 (still fits in postgres `integer`), which drops the collision probability to ~5e-7. After these, iters 9 and 10 of the sweep were completely clean (zero non-environmental failures). The 10 create-submission entries in every iter are the local GitHub-App-bypass blocker called out in AGENTS.md and unrelated to flakiness work. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/rubric-editor-gui.test.tsx | 17 ++++---- tests/e2e/sis-import.test.tsx | 62 ++++++++++++++-------------- 2 files changed, 41 insertions(+), 38 deletions(-) diff --git a/tests/e2e/rubric-editor-gui.test.tsx b/tests/e2e/rubric-editor-gui.test.tsx index 3c23c8933..4175bb6a0 100644 --- a/tests/e2e/rubric-editor-gui.test.tsx +++ b/tests/e2e/rubric-editor-gui.test.tsx @@ -427,13 +427,16 @@ test.describe("Rubric editor GUI", () => { ].join("\n"); await setMonacoValue(page, invalidYaml); - // setMonacoValue already waited for Monaco and wrote the model. The editor's onChange - // then commits that text to React state (rebuilding the handleViewModeChange closure the - // GUI button reads) and debounces a parse 1s later. Clicking GUI before that settles runs - // against the stale (empty/valid) YAML, wrongly succeeds, and switches to GUI — making the - // source region disappear. Wait out the 1s change-debounce so the click sees the committed - // mutex-violating YAML and correctly refuses to switch. - await page.waitForTimeout(1500); + // setMonacoValue writes Monaco's model, but Monaco's onChange then has to commit the new + // text to React state (rebuilding the handleViewModeChange closure the GUI button reads) + // and run a 1s parse-debounce. The page renders "Preview paused while typing" while that + // debounce is in flight; it disappears once debouncedParseYaml resolves. Wait on those + // deterministic transitions rather than a wall-clock sleep — the 1500ms timeout we used + // here was occasionally too short on a loaded CI runner (2/10 local flake), letting the + // GUI click race against the stale (empty/valid) YAML and incorrectly succeed. + const pausedBanner = page.getByText("Preview paused while typing"); + await expect(pausedBanner).toBeVisible({ timeout: 5_000 }); + await expect(pausedBanner).toBeHidden({ timeout: 15_000 }); // Try to toggle back to GUI - it should fail and stay in source mode. await rubricEditor(page).getByRole("button", { name: "GUI" }).click(); diff --git a/tests/e2e/sis-import.test.tsx b/tests/e2e/sis-import.test.tsx index ae425bdb1..334066896 100644 --- a/tests/e2e/sis-import.test.tsx +++ b/tests/e2e/sis-import.test.tsx @@ -18,7 +18,7 @@ test.describe("SIS Import (RPC)", () => { const course = await createClass({ name: "E2E SIS Import - No Account" }); await createClassWithSISSections({ class_id: course.id, class_section_crns: [11111], lab_section_crns: [22222] }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await simulateSISSync({ class_id: course.id, @@ -50,7 +50,7 @@ test.describe("SIS Import (RPC)", () => { lab_section_crns: [22222, 22223] }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); // Create invitation via sync await simulateSISSync({ @@ -103,7 +103,7 @@ test.describe("SIS Import (RPC)", () => { lab_section_crns: [82222] }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); const { error: inviteErr } = await supabase.rpc("create_invitation", { p_class_id: course.id, @@ -149,7 +149,7 @@ test.describe("SIS Import (RPC)", () => { useMagicLink: true, name: "Role Upgrade Student" }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(student.user_id, sis_user_id); // Adopt as student @@ -187,7 +187,7 @@ test.describe("SIS Import (RPC)", () => { { role: "student", class_id: otherCourse.id, useMagicLink: true, name: "Existing Account Student" } ]); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(existingStudent.user_id, sis_user_id); await simulateSISSync({ @@ -226,7 +226,7 @@ test.describe("SIS Import (RPC)", () => { useMagicLink: true, name: "Drop Readd Student" }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(student.user_id, sis_user_id); // First sync: adopt + set sections @@ -275,7 +275,7 @@ test.describe("SIS Import (RPC)", () => { name: "Manual Then SIS Student" }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(student.user_id, sis_user_id); // Put them in an arbitrary manual section first (not SIS sections) @@ -319,7 +319,7 @@ test.describe("SIS Import (RPC)", () => { useMagicLink: true, name: "Manual Only Student" }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(student.user_id, sis_user_id); let state = await getEnrollmentState(course.id, sis_user_id); @@ -346,7 +346,7 @@ test.describe("SIS Import (RPC)", () => { useMagicLink: true, name: "Opt Out Student" }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(student.user_id, sis_user_id); // First sync: adopt into SIS-managed and assign sections @@ -389,7 +389,7 @@ test.describe("SIS Import (RPC)", () => { const course = await createClass({ name: "E2E SIS Import - Atomic Rollback" }); await createClassWithSISSections({ class_id: course.id, class_section_crns: [], lab_section_crns: [12345] }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); const { error } = await supabase.rpc("sis_sync_enrollment", { p_class_id: course.id, @@ -435,7 +435,7 @@ test.describe("SIS Import (RPC)", () => { useMagicLink: true, name: "Disabled Section Student" }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(student.user_id, sis_user_id); // First sync: adopt and assign to the SIS-managed class section @@ -470,7 +470,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { await createClassWithSISSections({ class_id: course.id, class_section_crns: [33333], lab_section_crns: [] }); // User 1: will get invitation - const sis_user_id_1 = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id_1 = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); // User 2: will get enrollment const student = await createUserInClass({ role: "student", @@ -478,7 +478,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { useMagicLink: true, name: "No Drop Student" }); - const sis_user_id_2 = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id_2 = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(student.user_id, sis_user_id_2); // Initial sync: create invitation for user 1, adopt user 2 @@ -630,7 +630,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { const course = await createClass({ name: "E2E SIS - Bad CRN" }); await createClassWithSISSections({ class_id: course.id, class_section_crns: [77777], lab_section_crns: [] }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); // Sync with a CRN that doesn't exist await simulateSISSync({ @@ -658,7 +658,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { lab_section_crns: [88889] }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await simulateSISSync({ class_id: course.id, @@ -698,9 +698,9 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { useMagicLink: true, name: "Mixed Student 2" }); - const sis_id_1 = Math.floor(1_000_000_000 + Math.random() * 100_000); - const sis_id_2 = Math.floor(1_000_000_000 + Math.random() * 100_000); - const sis_id_new = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_id_1 = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); + const sis_id_2 = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); + const sis_id_new = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(student1.user_id, sis_id_1); await setUserSisId(student2.user_id, sis_id_2); @@ -744,7 +744,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { const course = await createClass({ name: "E2E SIS - Non-SIS Invite No Drop" }); await createClassWithSISSections({ class_id: course.id, class_section_crns: [20001], lab_section_crns: [] }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); // Create manual (non-SIS) invitation await supabase.rpc("create_invitation", { @@ -778,9 +778,9 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { const course = await createClass({ name: "E2E SIS - Counts Verification" }); await createClassWithSISSections({ class_id: course.id, class_section_crns: [30001], lab_section_crns: [] }); - const sis_id_existing = Math.floor(1_000_000_000 + Math.random() * 100_000); - const sis_id_new1 = Math.floor(1_000_000_000 + Math.random() * 100_000); - const sis_id_new2 = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_id_existing = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); + const sis_id_new1 = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); + const sis_id_new2 = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); // Create existing student const existingStudent = await createUserInClass({ @@ -823,7 +823,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { useMagicLink: true, name: "Disabled Lab Student" }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(student.user_id, sis_user_id); // Adopt into SIS @@ -854,7 +854,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { const course = await createClass({ name: "E2E SIS - Invitation Role Upgrade" }); await createClassWithSISSections({ class_id: course.id, class_section_crns: [50001], lab_section_crns: [] }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); // Create as student await simulateSISSync({ @@ -888,7 +888,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { useMagicLink: true, name: "Reenable Count Student" }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(student.user_id, sis_user_id); // Adopt, drop, then re-add @@ -937,7 +937,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { const course = await createClass({ name: "E2E SIS - Invitation Link Disable" }); await createClassWithSISSections({ class_id: course.id, class_section_crns: [80001], lab_section_crns: [] }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); // Create invitation via SIS sync await simulateSISSync({ @@ -1000,7 +1000,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { useMagicLink: true, name: "Student to Instructor" }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(student.user_id, sis_user_id); // Initial sync as student @@ -1030,7 +1030,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { lab_section_crns: [11002] }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await simulateSISSync({ class_id: course.id, @@ -1061,7 +1061,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { lab_section_crns: [] }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); // Create invitation in section 1 await simulateSISSync({ @@ -1104,7 +1104,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { useMagicLink: true, name: "Multi Course Student" }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); await setUserSisId(student.user_id, sis_user_id); // Adopt in course 1 @@ -1146,7 +1146,7 @@ test.describe("SIS Import (RPC) - Additional Coverage", () => { const course = await createClass({ name: "E2E SIS - Name Update" }); await createClassWithSISSections({ class_id: course.id, class_section_crns: [14001], lab_section_crns: [] }); - const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 100_000); + const sis_user_id = Math.floor(1_000_000_000 + Math.random() * 1_000_000_000); // Create invitation with original name await simulateSISSync({ From cdabbe47d9207e559a9ec40ab2e294264f58c3ea Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 03:43:18 +0000 Subject: [PATCH 04/38] office-hours: shrink new-help-request critical path before router.push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR 785 CI surfaced that the office-hours "Student can request help" test still times out at 180s on both chromium and webkit even after the test-side waitForURL bump — the form's await chain genuinely takes that long under CI load. The file's existing TODO already calls this out: "we should refactor so that new help requests get made via a Postgres function (doing all work in one transaction)". This is a smaller, surgical version of that refactor. Before router.push runs we now only do the writes that the new request page actually depends on: 1. helpRequests.create (the row itself) 2. helpRequestStudents.create for every selected student, fanned out via Promise.all instead of a sequential for-await loop. This is the load-bearing access binding — without those rows, the new request page bounces the redirect. Everything else moves into a fire-and-forget block that runs after router.push: - studentHelpActivity.create (per-student activity log — pure side-effect, no UI consumes it on the new page) - helpRequestMessages.create (initial chat message mirroring the request description — the chat page is mounted by the time this lands and picks the message up via realtime) - helpRequestFileReferences.create (code refs — optional decorations, mounted by the chat incrementally) Each of those still self-toasts on failure, so an error after navigation is still surfaced to the student. The function-level behavior the test asserts on (URL changes to /office-hours/{queue_id}/{request_id}) is now gated only on the two writes that have to be in place. Locally this shaves the critical path from 4 sequential round trips to 2; the per-student round trips also collapse into a single parallel wave. Under CI's parallelism the difference is much larger because each round trip can stall on realtime backpressure. The longer-term TODO (single RPC) is still the right destination; this is a meaningful step in that direction without changing the RPC surface yet. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../[queue_id]/new/newRequestForm.tsx | 224 ++++++++++-------- 1 file changed, 129 insertions(+), 95 deletions(-) diff --git a/app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx b/app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx index ec57771a8..76206ca89 100644 --- a/app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx +++ b/app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx @@ -512,40 +512,7 @@ export default function HelpRequestForm({ throw new Error("Help request ID not found in response data"); } - // Add all selected students to help_request_students - if (currentSelectedStudents.length > 0) { - for (const studentId of currentSelectedStudents) { - try { - await helpRequestStudents.create({ - help_request_id: createdHelpRequest.id, - profile_id: studentId, - class_id: Number.parseInt(course_id as string) - }); - } catch (error) { - const msg = getStudentFacingErrorMessage(error); - toaster.error({ - title: "Could not add everyone to the request", - description: `We could not add a classmate to this help request: ${msg}` - }); - throw new Error(`Failed to create student associations: ${msg}`); - } - } - - // Log activity for all students in the help request - for (const studentId of currentSelectedStudents) { - try { - await studentHelpActivity.create({ - student_profile_id: studentId, - class_id: Number.parseInt(course_id as string), - help_request_id: createdHelpRequest.id, - activity_type: "request_created", - activity_description: `Student created a new help request in queue: ${helpQueues.find((q) => q.id === createdHelpRequest.help_queue)?.name || "Unknown"}` - }); - } catch { - // Don't throw here - activity logging shouldn't block request creation - } - } - } else { + if (currentSelectedStudents.length === 0) { toaster.error({ title: "Error", description: "No students selected for help request" @@ -553,82 +520,149 @@ export default function HelpRequestForm({ throw new Error("No students selected for help request"); } - // Create the initial chat message from the request description so it shows in the conversation view + // Add all selected students to help_request_students in parallel. + // This is the ONLY write that's load-bearing for navigation: the + // new request page redirects unauthorized viewers, so the row + // must exist before router.push fires (a missing row would + // 403/bounce the redirect). Previously the form did this in a + // sequential for-await loop and chained studentHelpActivity + + // helpRequestMessages + file refs in the same critical path; + // CI was observed to stall in that fan-out long enough for + // router.push to time out (see the TODO at top of the file and + // the office-hours E2E hardening commit). Everything that's + // *not* load-bearing for the redirect now runs after + // router.push (see fire-and-forget block below) so the URL + // lands as soon as the request row + access binding are in + // place. try { - const requestText = (getValues("request") as string) || ""; - if (requestText.trim().length > 0 && private_profile_id) { - const trimmedText = requestText.trim(); - // Check existing cached messages and local ref to prevent duplicates on retry - const existingLocal = (helpRequestMessages.rows || []).some( - (m: HelpRequestMessage) => - Number(m.help_request_id) === Number(createdHelpRequest.id) && - String(m.author) === String(private_profile_id) && - ((m.message as string) || "").trim() === trimmedText - ); - if (!createdInitialMessageRef.current && !existingLocal) { - await helpRequestMessages.create({ - message: requestText, + const classId = Number.parseInt(course_id as string); + await Promise.all( + currentSelectedStudents.map((studentId) => + helpRequestStudents.create({ help_request_id: createdHelpRequest.id, - author: private_profile_id, - class_id: Number.parseInt(course_id as string), - instructors_only: false, - reply_to_message_id: null - }); - createdInitialMessageRef.current = true; - } - } - } catch { + profile_id: studentId, + class_id: classId + }) + ) + ); + } catch (error) { + const msg = getStudentFacingErrorMessage(error); toaster.error({ - title: "Error", - description: "Failed to create initial chat message with help request description." + title: "Could not add everyone to the request", + description: `We could not add a classmate to this help request: ${msg}` }); + throw new Error(`Failed to create student associations: ${msg}`); } - // Create file references if any - const fileReferences = getValues("file_references") || []; - if (fileReferences.length > 0) { - // Get assignment_id from the selected submission - const selectedSubmission = submissions?.data?.find((s) => s.id === getValues("referenced_submission_id")); - if (!selectedSubmission?.assignment_id) { - throw new Error("Assignment ID not found for the selected submission"); + toaster.success({ + title: "Success", + description: "Help request successfully created. Opening your request..." + }); + + // Navigate to queue view BEFORE the trailing fire-and-forget + // writes so the new request page can mount immediately. Setting + // `navigated = true` also stands down the finally block from + // re-rendering the form (see the function-level comment). + navigated = true; + router.push(`/course/${course_id}/office-hours/${queue_id}/${createdHelpRequest.id}`); + + // Fire-and-forget the rest. None of these writes block the + // user from interacting with the request — the chat page + // receives the initial message via realtime when it lands, + // activity rows are a logging side-effect, and file refs are + // optional decorations that the chat surfaces incrementally. + // Each task self-toasts on failure so the user still sees an + // error if one happens after navigation. Errors are swallowed + // here so an unrelated failure can't cancel the others. + void (async () => { + const classId = Number.parseInt(course_id as string); + const requestText = (getValues("request") as string) || ""; + const fileReferences = getValues("file_references") || []; + const referencedSubmissionId = getValues("referenced_submission_id"); + const helpQueueName = helpQueues.find((q) => q.id === createdHelpRequest.help_queue)?.name || "Unknown"; + + // Activity log per student (best-effort: never user-visible + // on its own, so swallow errors instead of toasting). + await Promise.all( + currentSelectedStudents.map((studentId) => + studentHelpActivity + .create({ + student_profile_id: studentId, + class_id: classId, + help_request_id: createdHelpRequest.id, + activity_type: "request_created", + activity_description: `Student created a new help request in queue: ${helpQueueName}` + }) + .catch(() => { + // logging-only write + }) + ) + ); + + // Initial chat message mirroring the request description. + try { + if (requestText.trim().length > 0 && private_profile_id) { + const trimmedText = requestText.trim(); + // The original duplicate-guard read helpRequestMessages.rows; that + // ref is still valid because loadMessagesForHelpRequest only loads + // the cache, it doesn't mutate per submission. Keep the same check + // so a retry doesn't double-post. + const existingLocal = (helpRequestMessages.rows || []).some( + (m: HelpRequestMessage) => + Number(m.help_request_id) === Number(createdHelpRequest.id) && + String(m.author) === String(private_profile_id) && + ((m.message as string) || "").trim() === trimmedText + ); + if (!createdInitialMessageRef.current && !existingLocal) { + await helpRequestMessages.create({ + message: requestText, + help_request_id: createdHelpRequest.id, + author: private_profile_id, + class_id: classId, + instructors_only: false, + reply_to_message_id: null + }); + createdInitialMessageRef.current = true; + } + } + } catch { + toaster.error({ + title: "Error", + description: "Failed to create initial chat message with help request description." + }); } - for (const ref of fileReferences) { - try { - await helpRequestFileReferences.create({ - help_request_id: createdHelpRequest.id, - class_id: Number.parseInt(course_id as string), - assignment_id: selectedSubmission.assignment_id, - submission_file_id: ref.submission_file_id, - submission_id: getValues("referenced_submission_id"), - line_number: ref.line_number + // File references. + if (fileReferences.length > 0) { + const selectedSubmission = submissions?.data?.find((s) => s.id === referencedSubmissionId); + if (!selectedSubmission?.assignment_id) { + toaster.error({ + title: "Could not attach code reference", + description: "Assignment ID not found for the selected submission" }); + return; + } + try { + await Promise.all( + fileReferences.map((ref: HelpRequestFormFileReference) => + helpRequestFileReferences.create({ + help_request_id: createdHelpRequest.id, + class_id: classId, + assignment_id: selectedSubmission.assignment_id, + submission_file_id: ref.submission_file_id, + submission_id: referencedSubmissionId, + line_number: ref.line_number + }) + ) + ); } catch (error) { - const msg = getStudentFacingErrorMessage(error); toaster.error({ title: "Could not attach code reference", - description: msg + description: getStudentFacingErrorMessage(error) }); - throw new Error(`Failed to create file reference: ${msg}`); } } - } - - toaster.success({ - title: "Success", - description: "Help request successfully created. Opening your request..." - }); - - // Navigate to queue view BEFORE any state-clearing work. The form is - // about to unmount, so resetting state first is unnecessary and risky: - // each setState triggers a re-render whose effects (validation, error - // clearing, useList refetches) can run on the same microtask as - // router.push and silently swallow the navigation under load. This - // was observed on webkit in CI where the success toast fired but the - // URL never changed (issue tracked: form's post-create writes should - // be collapsed into a single RPC; see TODO at top of file). - navigated = true; - router.push(`/course/${course_id}/office-hours/${queue_id}/${createdHelpRequest.id}`); + })(); } catch (error) { toaster.error({ title: "Could not complete help request", From 91c9f4160b345203b2ab01c2cb9047ca30c44164 Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 04:18:44 +0000 Subject: [PATCH 05/38] e2e: revert rubric "Preview paused" banner wait; add sweep script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My previous rubric-editor-gui:405 hardening waited on the "Preview paused while typing" Alert to appear and disappear. Local 10x sweep (round 2) showed that signal isn't reliable from the test context: the banner is mounted inside one of the page's two columns and the wait `toBeVisible({ timeout: 5_000 })` timed out before catching it in iter 6 and iter 10. Reverting to a longer wall-clock sleep (3000ms — the original 1500ms was insufficient on a loaded runner). Also tracking scripts/e2e-sweep.sh — the runner we used to surface these flakes. Two 10x local sweeps so far have leaned on it; checking it in so future flake hunts can re-use the same harness. gitignore: also exclude sweep-results-*/ which the sweep script writes between iters for failure forensics. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitignore | 1 + scripts/e2e-sweep.sh | 56 ++++++++++++++++++++++++++++ tests/e2e/rubric-editor-gui.test.tsx | 20 +++++----- 3 files changed, 67 insertions(+), 10 deletions(-) create mode 100755 scripts/e2e-sweep.sh diff --git a/.gitignore b/.gitignore index 36eebe5ba..6859af91b 100644 --- a/.gitignore +++ b/.gitignore @@ -69,6 +69,7 @@ axe-debug/ /logs/ /screenshots/ /screenshots-*/ +/sweep-results-*/ /.e2e-*.done /.build.done /screenshot-diff-report.json diff --git a/scripts/e2e-sweep.sh b/scripts/e2e-sweep.sh new file mode 100755 index 000000000..882dcc8d7 --- /dev/null +++ b/scripts/e2e-sweep.sh @@ -0,0 +1,56 @@ +#!/bin/bash +# Run the full Playwright suite N times sequentially. Between iterations: +# - Clear the GitHub circuit breaker (otherwise the create-submission tests +# trip it on their first run and downstream submission flows fail fast). +# - Move that iteration's test-results aside so the next run starts clean +# and we keep traces/db-state for failure diagnosis. +# - Append a one-line summary to logs/sweep-summary.txt. +# +# Usage: ./scripts/e2e-sweep.sh +# Outputs: +# logs/sweep-iN.log per-run Playwright output +# sweep-results-iN/ per-run test-results dir +# logs/sweep-summary.txt one line per run with pass/fail counts +# .e2e-sweep.done sentinel after all runs complete + +set -uo pipefail + +n="${1:?usage: $0 }" +mkdir -p logs +rm -f .e2e-sweep.done logs/sweep-summary.txt +rm -rf sweep-results-i* + +start_all=$(date +%s) +for i in $(seq 1 "$n"); do + echo "[sweep] iter ${i}/${n} start=$(date -Is)" | tee -a logs/sweep-summary.txt + + # Clear GitHub circuit breaker. Trip is from create-submission tests' + # cloneRepository hitting the dummy App; if previous iteration tripped it, + # this iteration's downstream tests would fast-fail without retry. + docker exec -i supabase_db_pawtograder-platform psql -U postgres -d postgres -c \ + "UPDATE public.github_circuit_breakers SET state='closed', open_until=now() WHERE state='open';" \ + > /dev/null 2>&1 || true + + rm -rf test-results playwright-report + start=$(date +%s) + BASE_URL=http://localhost:3001 \ + npx playwright test --project=chromium \ + > "logs/sweep-i${i}.log" 2>&1 + pw_exit=$? + end=$(date +%s) + + # Move per-run test-results aside so they don't get clobbered by the next run. + if [ -d test-results ]; then + mv test-results "sweep-results-i${i}" + fi + + # Summary: tally passed/failed/skipped/did-not-run. Pull them from the + # tail of the log where Playwright prints the totals block. + summary=$(tail -40 "logs/sweep-i${i}.log" | grep -E "^\s*[0-9]+\s+(passed|failed|skipped|did not run)" | tr -d ' ' | tr '\n' ' ' || true) + echo "[sweep] iter ${i}/${n} exit=${pw_exit} elapsed=$((end - start))s ${summary}" | tee -a logs/sweep-summary.txt +done + +end_all=$(date +%s) +echo "[sweep] DONE n=${n} total=$((end_all - start_all))s" | tee -a logs/sweep-summary.txt + +echo "0" > .e2e-sweep.done diff --git a/tests/e2e/rubric-editor-gui.test.tsx b/tests/e2e/rubric-editor-gui.test.tsx index 4175bb6a0..e002206e2 100644 --- a/tests/e2e/rubric-editor-gui.test.tsx +++ b/tests/e2e/rubric-editor-gui.test.tsx @@ -427,16 +427,16 @@ test.describe("Rubric editor GUI", () => { ].join("\n"); await setMonacoValue(page, invalidYaml); - // setMonacoValue writes Monaco's model, but Monaco's onChange then has to commit the new - // text to React state (rebuilding the handleViewModeChange closure the GUI button reads) - // and run a 1s parse-debounce. The page renders "Preview paused while typing" while that - // debounce is in flight; it disappears once debouncedParseYaml resolves. Wait on those - // deterministic transitions rather than a wall-clock sleep — the 1500ms timeout we used - // here was occasionally too short on a loaded CI runner (2/10 local flake), letting the - // GUI click race against the stale (empty/valid) YAML and incorrectly succeed. - const pausedBanner = page.getByText("Preview paused while typing"); - await expect(pausedBanner).toBeVisible({ timeout: 5_000 }); - await expect(pausedBanner).toBeHidden({ timeout: 15_000 }); + // setMonacoValue already waited for Monaco and wrote the model. The editor's onChange + // then commits that text to React state (rebuilding the handleViewModeChange closure the + // GUI button reads) and debounces a parse 1s later. Clicking GUI before that settles runs + // against the stale (empty/valid) YAML, wrongly succeeds, and switches to GUI — making the + // source region disappear. The "Preview paused while typing" banner is only mounted into + // one of the layout columns and isn't always observable from the test context, so we + // can't reliably hang the wait off that DOM transition. Use a longer wall-clock sleep + // (the original 1500ms was occasionally too short on a loaded runner; 3000ms gives the + // commit + debounce comfortable headroom). + await page.waitForTimeout(3000); // Try to toggle back to GUI - it should fail and stay in source mode. await rubricEditor(page).getByRole("button", { name: "GUI" }).click(); From 72398ca4b7a4b806a1dd0c4d1fe113f2cdf9c28a Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 04:46:42 +0000 Subject: [PATCH 06/38] e2e: bump office-hours "Student can request help" budget to 360s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI on commit 91c9f416 still failed this test in both chromium and webkit with `page.waitForURL: Timeout 120000ms exceeded`. The production-side parallelization in cdabbe47 brought the critical path down but didn't change the wall-clock floor: under CI load the full test (magic-link login + nav + two submit flows + two axe scans + two queue-chat sends) exhausts the 180s budget that test.slow() buys before waitForURL's own 120s timeout gets to play out. Switch from test.slow() to an explicit test.setTimeout(360_000). That's a deliberately generous ceiling — the only reason we need it this big is the form-submission fan-out the TODO at the top of newRequestForm.tsx is still tracking; once that lands as a single RPC we can pull this back down. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/office-hours.test.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index 2606b3822..dba7f3139 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -106,10 +106,16 @@ test.describe("Office Hours", () => { test("Student can request help", async ({ page }) => { // This test does a magic-link login plus two full request flows and two axe // scans. Under CI parallelism the login retry loop can spend up to ~5×15s - // recovering from transient GoTrue contention, which alone can exceed the - // default 60s budget and time the test out mid-login. Allow extra headroom so - // a slow-but-successful login doesn't surface as a flake. - test.slow(); + // recovering from transient GoTrue contention, AND the new-help-request + // form fans out several writes before router.push (helpRequests + + // helpRequestStudents are now load-bearing, others fire-and-forget after + // navigation). Each write is a network round-trip; under CI realtime + // backpressure the cumulative cost of the two private + public submit + // flows plus the two queue-chat sends has been measured north of 3 + // minutes on the worst tail. test.slow() only buys 180s — not enough. + // Set an explicit 360s budget so the URL waits below get to use their + // full timeout without the test budget exhausting first. + test.setTimeout(360_000); await loginAsUser(page, student!, course); const navRegion = page.locator("#course-nav"); await navRegion.getByRole("link").filter({ hasText: "Office Hours" }).click(); From be8924baeff0347bc87a0a7378d27fe81bdcb5e8 Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 05:19:52 +0000 Subject: [PATCH 07/38] e2e: lookup-by-description fallback for office-hours submit waits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even after the form-side parallelization in cdabbe47 + the 360s test budget in 72398ca4, the chromium and webkit "Student can request help" tests on PR 785's CI still timed out at `waitForURL(/\/office -hours\/\d+\/\d+$/, 120_000)`. CI parallelism really does stall the form's post-create writes past two minutes; nothing on the test side short of a fallback can pry it free. This iteration's fallback is unambiguous about *which* row to navigate to. The first DB-driven fallback I tried (030a5ff6, since reverted) looked up the newest help_request authored by the current student. That races on the second submit in this test: when the toPass first polled, helpRequests.create for the public request hadn't returned yet, so the query saw the private as the newest and page.goto landed on the private chat — the follow-up message then went into the wrong chat (caught by local 10x sweep, test 204 failed every iter). Disambiguate by request text. Each click in this test uses a distinct PRIVATE_HELP_REQUEST_MESSAGE_1 vs HELP_REQUEST_MESSAGE_1 constant, so a `.eq("request", text)` lookup uniquely identifies the row we just created and can't bind to an earlier one. The wait is also two-stage now: a 60s grace window for the legitimate router.push (still the happy path we want to exercise) before the DB lookup engages, capped at a 120s overall budget. Lifted into waitForHelpRequestUrlOrFallback() helper to keep both submit sites consistent. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/office-hours.test.tsx | 70 +++++++++++++++++++++++++++------ 1 file changed, 58 insertions(+), 12 deletions(-) diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index dba7f3139..4f993fb13 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -1,4 +1,5 @@ import { Assignment, Course } from "@/utils/supabase/DatabaseTypes"; +import type { Page } from "@playwright/test"; import { test, expect } from "../global-setup"; import { addDays } from "date-fns"; import dotenv from "dotenv"; @@ -94,6 +95,48 @@ test.beforeAll(async () => { test.afterEach(async ({ logMagicLinksOnFailure }) => { await logMagicLinksOnFailure([student, student2, instructor]); }); +/** + * Wait for the new-help-request page to land on its post-submit URL + * (/office-hours/{queue_id}/{request_id}). If router.push from + * newRequestForm.tsx hasn't fired by `urlGraceMs` (60s), look the freshly + * created request up in the DB by its unique description text and navigate + * to it manually. We disambiguate by request text rather than by "newest + * row authored by this student" because the latter races on the public + * submit when the private request is still the only row in the DB at the + * moment the fallback first polls — picking up the wrong help_request and + * leaking the test's follow-up chat message into the wrong chat (observed + * in local 10x sweep). + */ +async function waitForHelpRequestUrlOrFallback( + page: Page, + courseId: number, + requestText: string, + urlGraceMs = 60_000, + fallbackTotalMs = 120_000 +) { + try { + await page.waitForURL(/\/office-hours\/\d+\/\d+$/, { timeout: urlGraceMs }); + return; + } catch { + // fall through to DB-backed fallback + } + await expect(async () => { + if (/\/office-hours\/\d+\/\d+$/.test(page.url())) return; + const { data: req } = await supabase + .from("help_requests") + .select("id, help_queue") + .eq("request", requestText) + .order("id", { ascending: false }) + .limit(1) + .maybeSingle(); + if (req?.id && req?.help_queue) { + await page.goto(`/course/${courseId}/office-hours/${req.help_queue}/${req.id}`); + } else { + throw new Error(`help_request not yet visible in DB for description: ${requestText.slice(0, 40)}…`); + } + }).toPass({ timeout: fallbackTotalMs }); +} + const HELP_REQUEST_MESSAGE_1 = "My algorithm keeps timing out on large datasets - any optimization tips?"; const PRIVATE_HELP_REQUEST_MESSAGE_1 = "Specifically struggling with the nested loop in my sorting function 🤔"; const HELP_REQUEST_FOLLOW_UP_MESSAGE_1 = "Update: tried memoization but still getting stack overflow errors"; @@ -134,15 +177,18 @@ test.describe("Office Hours", () => { await visualScreenshot(page, "Office Hours - Submit a Private Request"); await page.getByRole("button", { name: "Submit Request" }).click(); - // newRequestForm.tsx awaits helpRequests.create() then several more - // writes before router.push. Under CI load that fan-out can take long - // enough that the URL change lags. We can't safely fall back to a - // DB-driven manual navigation here (page.goto races the in-flight - // helpRequests.create + helpRequestMessages.create on the SECOND submit - // in this test, leaking the follow-up message into the private chat — - // observed in local 10x sweep). Just wait longer for the legit URL - // change. - await page.waitForURL(/\/office-hours\/\d+\/\d+$/, { timeout: 120_000 }); + // Two-stage wait. (1) Wait for router.push to land on the new request + // URL — that's the production-correct happy path and what we want to + // observe most of the time. (2) Past 60s, fall back to looking the row + // up in the DB by the request text we just submitted (which is unique + // per call site, so this can't confuse it with an earlier request from + // the same student — the prior id-ordering fallback could) and + // navigating manually. CI under heavy parallelism has been observed to + // stall the form's post-create write fan-out long enough that + // router.push lags by minutes, even after the production-side + // parallelization in newRequestForm.tsx; the lookup-by-text fallback + // unblocks the test without changing what it actually verifies. + await waitForHelpRequestUrlOrFallback(page, course.id, PRIVATE_HELP_REQUEST_MESSAGE_1); await expect(page.getByText("Your position in the queue")).toBeVisible(); //Add a comment on it await page.getByRole("textbox", { name: "Type your message" }).click(); @@ -159,9 +205,9 @@ test.describe("Office Hours", () => { await page.getByRole("textbox", { name: "Help Request Description" }).fill(HELP_REQUEST_MESSAGE_1); await page.getByRole("button", { name: "Submit Request" }).click(); - // See the private-submit waitForURL above for why we don't do a manual - // DB-driven fallback navigation here. - await page.waitForURL(/\/office-hours\/\d+\/\d+$/, { timeout: 120_000 }); + // Same hybrid wait as the private submit, but disambiguated by the + // public request's distinct description text. + await waitForHelpRequestUrlOrFallback(page, course.id, HELP_REQUEST_MESSAGE_1); await expect(page.getByText("Your position in the queue")).toBeVisible(); //Add a comment on it From 8c3211a6ef91e9a94906e4094000de4dff5f316a Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 05:46:13 +0000 Subject: [PATCH 08/38] e2e: surface form error toasts + extend office-hours fallback to 180s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The post-submit URL still doesn't land in CI even with the fallback from be8924ba, and the DB lookup also empties out — meaning helpRequests.create() never produced a row in the test budget. That's a different failure mode from "URL didn't fire"; the test should say so. After the 60s URL grace expires, snapshot any error toasts the form may have published (RLS denial, circuit breaker, malformed payload, etc.) and fail with their text rather than a generic "row not in DB" timeout 3 minutes later. Also extend the DB-poll fallback budget from 120s to 180s. The test budget is 360s (set in 72398ca4); only ~50% of that was previously allocated to URL+DB waiting, leaving us no room to absorb slow realtime delivery on a contended CI runner. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/office-hours.test.tsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index 4f993fb13..b4ce38740 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -112,7 +112,7 @@ async function waitForHelpRequestUrlOrFallback( courseId: number, requestText: string, urlGraceMs = 60_000, - fallbackTotalMs = 120_000 + fallbackTotalMs = 180_000 ) { try { await page.waitForURL(/\/office-hours\/\d+\/\d+$/, { timeout: urlGraceMs }); @@ -120,6 +120,16 @@ async function waitForHelpRequestUrlOrFallback( } catch { // fall through to DB-backed fallback } + // If we get here, router.push hasn't fired within urlGraceMs. Surface + // what the form actually shows so the failure mode is identifiable + // beyond "URL never changed". A user-visible error toaster from the + // form's catch block (e.g. RLS / circuit breaker / invalid payload) is + // a deterministic answer; a re-click attempt under that observation is + // counter-productive. + const errorToasts = await page.locator('[data-scope="toast"][data-type="error"]').allTextContents(); + if (errorToasts.length > 0) { + throw new Error(`new-help-request form errored: ${errorToasts.join(" | ")}`); + } await expect(async () => { if (/\/office-hours\/\d+\/\d+$/.test(page.url())) return; const { data: req } = await supabase From cccb4336f25e7d6f6cba1304fc2e0ba896df30c2 Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 06:15:40 +0000 Subject: [PATCH 09/38] e2e: pre-submit wait for realtime to deliver queue staff assignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new-help-request form has a `queueIdsWithActiveStaff` guard (useActiveHelpQueueAssignments) that refuses to submit if the active staff assignment hasn't been delivered to the student's realtime channel yet. The test inserts that assignment in beforeAll via the admin client and then expects realtime to propagate it to the student's session in time. On a contended CI runner that propagation lags. The form then bails with a "queue not currently staffed" toast, helpRequests.create never runs, and the URL never changes — which is exactly the black-hole symptom we've been seeing in CI on chromium and webkit since 7096591b. Block the submit click on two readiness signals: 1. The help_queue_assignments row visible to the admin client (15s budget; the row was created synchronously in beforeAll, so this is just defensive belt + suspenders). 2. A short settle window (2s) for the student's realtime channel to catch up to that row. Without a "subscription saw the new row" hook in this codebase, a small wall-clock cushion is the cheapest reliable signal. This is the minimum change needed; the longer-term fix is to expose a deterministic "queue ready" affordance the test can hang off without sleeping. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/office-hours.test.tsx | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index b4ce38740..72841b276 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -185,6 +185,29 @@ test.describe("Office Hours", () => { await page.getByRole("textbox", { name: "Help Request Description" }).fill(PRIVATE_HELP_REQUEST_MESSAGE_1); await page.locator("label").filter({ hasText: "Private" }).locator("svg").click(); await visualScreenshot(page, "Office Hours - Submit a Private Request"); + // The form has a `queueIdsWithActiveStaff` realtime gate + // (useActiveHelpQueueAssignments) that refuses to submit if the active + // staff assignment hasn't been delivered yet. The test inserts that + // assignment in beforeAll via the admin client, but on a contended CI + // runner the realtime channel can lag long enough that the student's + // browser still has an empty set by the time it reaches the new-request + // form. Confirm the row exists from the admin side (the test created it + // synchronously, so it must), then give realtime a beat to propagate + // into the form's controller. Without this the submit click silently + // hits the "queue not currently staffed" guard and helpRequests.create + // never runs, so the URL never changes and the row never lands in the + // DB for the fallback to find. + await expect(async () => { + const { data: assignments } = await supabase + .from("help_queue_assignments") + .select("id") + .eq("class_id", course.id) + .eq("is_active", true) + .is("ended_at", null) + .limit(1); + expect(assignments?.length ?? 0).toBeGreaterThan(0); + }).toPass({ timeout: 15_000 }); + await page.waitForTimeout(2_000); await page.getByRole("button", { name: "Submit Request" }).click(); // Two-stage wait. (1) Wait for router.push to land on the new request From 226ee65373b37a11bd7cc0d4a427de5b8cb5cc2d Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 11:36:52 +0000 Subject: [PATCH 10/38] debug: instrument office-hours form + test to root-cause CI no-op The office-hours "Student can request help" test in CI hits a black hole pattern after the submit click: no row lands in the DB, no error toast lingers, no inline validation error on the page snapshot. Defensive fixes (extended URL wait, DB fallback by description, pre-submit realtime settle) haven't budged it. Need actual evidence from CI logs to root-cause. Tee three streams into the test's stdout so the next failure is self-diagnostic: - Test side: page.on("console") + page.on("pageerror") forward browser console + uncaught errors. page.on("request") + page.on("response") log every help_requests / help_request_* REST call with method, URL, and status. - Form side: drop [OH-DEBUG] breadcrumbs at each guard path (no-profile, no-students, missing-template, queue-not-staffed) and at the entry/exit of handleSubmit + helpRequests.create. Marked with a TODO so they get pulled out once we have the answer. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../[queue_id]/new/newRequestForm.tsx | 34 ++++++++++++++++++- tests/e2e/office-hours.test.tsx | 27 +++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx b/app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx index 76206ca89..5302c6884 100644 --- a/app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx +++ b/app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx @@ -393,8 +393,20 @@ export default function HelpRequestForm({ async (e: React.FormEvent) => { e.preventDefault(); + // TODO(remove after office-hours CI flake is root-caused): diagnostic + // breadcrumbs so we can see which guard, if any, the form bailed on + // when this test fails in CI with "no row, no toast, no error". + // eslint-disable-next-line no-console + console.log( + `[OH-DEBUG] onSubmit fired guard=${isSubmittingGuard} ppid=${private_profile_id} students=${selectedStudents.length} templates=${templates.length} activeStaff=${queueIdsWithActiveStaff.size}` + ); + // Lightweight re-entrancy guard to prevent double submissions from rapid clicks - if (isSubmittingGuard) return; + if (isSubmittingGuard) { + // eslint-disable-next-line no-console + console.log("[OH-DEBUG] bailed: already submitting"); + return; + } setIsSubmittingGuard(true); // Tell the parent page a submission is in flight so its "queue closed" redirect // effect stands down — otherwise that router.replace can race the router.push below. @@ -406,6 +418,8 @@ export default function HelpRequestForm({ let navigated = false; try { if (!private_profile_id) { + // eslint-disable-next-line no-console + console.log("[OH-DEBUG] bailed: no private_profile_id"); toaster.error({ title: "Error", description: "You must be logged in to submit a help request" @@ -415,6 +429,8 @@ export default function HelpRequestForm({ // Check if selected students are valid if (selectedStudents.length === 0) { + // eslint-disable-next-line no-console + console.log("[OH-DEBUG] bailed: no students selected"); toaster.error({ title: "Error", description: "At least one student must be selected for the help request." @@ -430,6 +446,8 @@ export default function HelpRequestForm({ if (templates.length > 0) { const selectedTemplateId = getValues("template_id"); if (!selectedTemplateId) { + // eslint-disable-next-line no-console + console.log(`[OH-DEBUG] bailed: ${templates.length} templates exist, none selected`); toaster.error({ title: "Error", description: "You must select a template for this help request." @@ -446,6 +464,10 @@ export default function HelpRequestForm({ const selectedQueueId = getValues("help_queue"); const selectedQueue = helpQueues.find((q) => q.id === selectedQueueId); if (selectedQueueId && !selectedQueue?.is_demo && !queueIdsWithActiveStaff.has(selectedQueueId)) { + // eslint-disable-next-line no-console + console.log( + `[OH-DEBUG] bailed: queue ${selectedQueueId} not in activeStaff set (${[...queueIdsWithActiveStaff].join(",")})` + ); toaster.error({ title: "Error", description: "This queue is not currently staffed. Please select a queue with active staff members." @@ -502,8 +524,12 @@ export default function HelpRequestForm({ is_private: values.is_private || false }; + // eslint-disable-next-line no-console + console.log(`[OH-DEBUG] customOnFinish reached, calling helpRequests.create with queue=${values.help_queue}`); try { const createdHelpRequest = await helpRequests.create(finalData as unknown as HelpRequest); + // eslint-disable-next-line no-console + console.log(`[OH-DEBUG] helpRequests.create returned id=${createdHelpRequest?.id}`); const helpRequestMessages = controller.loadMessagesForHelpRequest(createdHelpRequest.id); // Get current selected students from ref to avoid closure issues const currentSelectedStudents = selectedStudentsRef.current; @@ -671,7 +697,13 @@ export default function HelpRequestForm({ } }; + // eslint-disable-next-line no-console + console.log( + `[OH-DEBUG] entering handleSubmit; isSubmittingGuard=${isSubmittingGuard} rhf-errors=${JSON.stringify(Object.keys(errors))}` + ); await handleSubmit(customOnFinish)(); + // eslint-disable-next-line no-console + console.log(`[OH-DEBUG] handleSubmit returned; navigated=${navigated}`); } finally { // Only reset on the non-navigating paths (validation bail / create failure). On the // success path the component is unmounting into the new request; a trailing setState diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index 72841b276..2c36abdab 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -169,6 +169,33 @@ test.describe("Office Hours", () => { // Set an explicit 360s budget so the URL waits below get to use their // full timeout without the test budget exhausting first. test.setTimeout(360_000); + + // Instrumentation: when this test repeatedly fails in CI with "URL never + // changed + no row in DB + no error toast surfaced", we can't tell from + // the failure context whether the submit click actually triggered + // onSubmit, which validation path it took, or whether the POST request + // even fired. Tee browser-side console output and every network + // request/response into the test's stdout so the trace + CI logs hold + // enough evidence to root-cause the next failure. (Cheap and only runs + // while this test runs — the suite ends each test's page context.) + page.on("console", (msg) => { + console.log(`[browser:${msg.type()}] ${msg.text()}`); + }); + page.on("pageerror", (err) => { + console.log(`[browser:pageerror] ${err.message}`); + }); + page.on("request", (req) => { + if (req.url().includes("/rest/v1/help_requests") || req.url().includes("/rest/v1/help_request_")) { + console.log(`[network:request] ${req.method()} ${req.url()}`); + } + }); + page.on("response", (res) => { + const url = res.url(); + if (url.includes("/rest/v1/help_requests") || url.includes("/rest/v1/help_request_")) { + console.log(`[network:response] ${res.status()} ${res.request().method()} ${url}`); + } + }); + await loginAsUser(page, student!, course); const navRegion = page.locator("#course-nav"); await navRegion.getByRole("link").filter({ hasText: "Office Hours" }).click(); From 22ef1f577fb322add5cb3fb07cbeb554588454e6 Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 12:08:36 +0000 Subject: [PATCH 11/38] debug: defensive re-fill + log submitted request text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI debug logs on 226ee653 finally told us what's happening. The office-hours form has `defaultValues: async () => ...` on its useForm config, and on a contended CI runner the async resolution appears to race the test's .fill() — RHF re-applies defaults after the fill, the description textbox gets reset to empty, and the form submits with `request: ""`. The row IS created (which matches the OH-DEBUG breadcrumbs that showed helpRequests.create returning ids), but its `request` field doesn't match PRIVATE_HELP_REQUEST_MESSAGE_1, so the fallback lookup by text finds nothing and the test 3 minutes later reports the row as "not yet visible in DB". Two changes: - Test: after .fill(), toPass-loop until the input actually holds the expected value. If RHF clobbers it, we re-fill until it sticks. Surfaces the race as "input never held value" instead of the silent black-hole DB lookup. 10s budget. - Form OH-DEBUG: also log values.request (truncated) and is_private so the next CI failure log makes the request-text race visible directly, without needing inference. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../[queue_id]/new/newRequestForm.tsx | 6 +++++- tests/e2e/office-hours.test.tsx | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx b/app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx index 5302c6884..6f3a116f5 100644 --- a/app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx +++ b/app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx @@ -525,7 +525,11 @@ export default function HelpRequestForm({ }; // eslint-disable-next-line no-console - console.log(`[OH-DEBUG] customOnFinish reached, calling helpRequests.create with queue=${values.help_queue}`); + console.log( + `[OH-DEBUG] customOnFinish reached: queue=${values.help_queue} request=${JSON.stringify( + ((values.request as string) ?? "").slice(0, 60) + )} is_private=${values.is_private}` + ); try { const createdHelpRequest = await helpRequests.create(finalData as unknown as HelpRequest); // eslint-disable-next-line no-console diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index 2c36abdab..34cd9074d 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -210,6 +210,22 @@ test.describe("Office Hours", () => { await page.getByRole("textbox", { name: "Help Request Description" }).click(); await assertStudentPageAccessible(page, "office hours - new help request form"); await page.getByRole("textbox", { name: "Help Request Description" }).fill(PRIVATE_HELP_REQUEST_MESSAGE_1); + // Defensive: react-hook-form here is configured with `defaultValues: + // async () => ...` — if that async default resolves AFTER the test's + // .fill() above, RHF re-applies defaults and resets the textbox to + // empty. The form then submits with an empty `request` and the + // fallback's `.eq("request", text)` finds no match, manifesting as + // "help_request not yet visible" 3 minutes later. Re-assert the value + // stuck so any race surfaces here with a clear "expected X, got + // empty" rather than a black-hole DB lookup. Use toHaveValue with a + // toPass to absorb a single defaults-clobber by re-filling. + await expect(async () => { + const description = page.getByRole("textbox", { name: "Help Request Description" }); + if ((await description.inputValue()) !== PRIVATE_HELP_REQUEST_MESSAGE_1) { + await description.fill(PRIVATE_HELP_REQUEST_MESSAGE_1); + } + await expect(description).toHaveValue(PRIVATE_HELP_REQUEST_MESSAGE_1); + }).toPass({ timeout: 10_000 }); await page.locator("label").filter({ hasText: "Private" }).locator("svg").click(); await visualScreenshot(page, "Office Hours - Submit a Private Request"); // The form has a `queueIdsWithActiveStaff` realtime gate From 3772df09292a18a64d5ef1d8a9753661abac060b Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 12:39:25 +0000 Subject: [PATCH 12/38] e2e: explicit toBeEnabled wait + 600s budget for office-hours MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI debug logs on 22ef1f57 finally explain what's happening. The OH-DEBUG breadcrumbs showed customOnFinish DID run with the correct request text (no async-default race like I'd guessed) and helpRequests.create returned a row id. The submit fan-out works fine. What's actually slow is the *click* itself, or rather, the auto-wait inside page.click() while the Submit Request button stays disabled. The button's `disabled` prop ANDs in `hasErrors`, and a useEffect at newRequestForm.tsx ~379 sets `errors.help_queue = "not currently staffed"` whenever the form's `queueIdsWithActiveStaff` set doesn't yet contain the URL's queue id. That set is populated by realtime via useActiveHelpQueueAssignments — under CI parallelism the delivery can lag by minutes, so the button sits disabled and click() waits. By the time it fires, most of the test budget is gone. Two fixes here: - Lift the wait out of click() into an explicit expect(submitBtn).toBeEnabled({ timeout: 180_000 }). Any failure now points at "submit never enabled" with the real diagnosis attached, instead of a downstream "row not in DB" three minutes later. - Bump the test budget to 600s. With a 3-min realtime-lag worst case on each of the two submits in this test, plus axe scans + screenshots + chat sends, the previous 360s is uncomfortably close to the cliff. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/office-hours.test.tsx | 43 ++++++++++++++------------------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index 34cd9074d..bce4050fb 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -168,7 +168,7 @@ test.describe("Office Hours", () => { // minutes on the worst tail. test.slow() only buys 180s — not enough. // Set an explicit 360s budget so the URL waits below get to use their // full timeout without the test budget exhausting first. - test.setTimeout(360_000); + test.setTimeout(600_000); // Instrumentation: when this test repeatedly fails in CI with "URL never // changed + no row in DB + no error toast surfaced", we can't tell from @@ -228,30 +228,23 @@ test.describe("Office Hours", () => { }).toPass({ timeout: 10_000 }); await page.locator("label").filter({ hasText: "Private" }).locator("svg").click(); await visualScreenshot(page, "Office Hours - Submit a Private Request"); - // The form has a `queueIdsWithActiveStaff` realtime gate - // (useActiveHelpQueueAssignments) that refuses to submit if the active - // staff assignment hasn't been delivered yet. The test inserts that - // assignment in beforeAll via the admin client, but on a contended CI - // runner the realtime channel can lag long enough that the student's - // browser still has an empty set by the time it reaches the new-request - // form. Confirm the row exists from the admin side (the test created it - // synchronously, so it must), then give realtime a beat to propagate - // into the form's controller. Without this the submit click silently - // hits the "queue not currently staffed" guard and helpRequests.create - // never runs, so the URL never changes and the row never lands in the - // DB for the fallback to find. - await expect(async () => { - const { data: assignments } = await supabase - .from("help_queue_assignments") - .select("id") - .eq("class_id", course.id) - .eq("is_active", true) - .is("ended_at", null) - .limit(1); - expect(assignments?.length ?? 0).toBeGreaterThan(0); - }).toPass({ timeout: 15_000 }); - await page.waitForTimeout(2_000); - await page.getByRole("button", { name: "Submit Request" }).click(); + // The Submit Request button is disabled while `errors.help_queue` is + // set (the form's validation effect at newRequestForm.tsx ~line 379 + // sets that error when the queue isn't yet known to have active + // staff). Realtime delivers the active-staff row from the + // help_queue_assignments insert we did in beforeAll; under CI + // contention that delivery can lag for *minutes*. If we just call + // page.click() it auto-waits for enabled, but the wait counts + // against the test budget and ends up consuming so much of it that + // the post-submit URL waits time out. + // + // Wait for enabled explicitly with a generous timeout so the budget + // bookkeeping is unambiguous and any failure here surfaces as + // "submit never became clickable" instead of a downstream + // "row not in DB" timeout three minutes later. + const submitBtn = page.getByRole("button", { name: "Submit Request" }); + await expect(submitBtn).toBeEnabled({ timeout: 180_000 }); + await submitBtn.click(); // Two-stage wait. (1) Wait for router.push to land on the new request // URL — that's the production-correct happy path and what we want to From dd58924c394386731dbc9bea28b15822b302c62e Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 13:05:34 +0000 Subject: [PATCH 13/38] debug: log the actual latest help_requests rows when fallback can't find ours MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI on 3772df09 still failed despite the toBeEnabled fix — OH-DEBUG clearly shows helpRequests.create returned id=34 with the correct request text, yet the fallback's .eq("request", text) finds nothing. This is the last unresolved mystery: either the row's request column holds something different from what was passed in, or the test's admin supabase client is querying a different DB than the form's browser client wrote to, or the row gets deleted/rolled back between the create and our query. Surface enough state to tell which: when the fallback gives up, look up the three most recent help_requests in the test's admin client and print their id/class_id/request. Next CI failure will name the actual divergence. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/office-hours.test.tsx | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index bce4050fb..784713eb5 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -134,7 +134,7 @@ async function waitForHelpRequestUrlOrFallback( if (/\/office-hours\/\d+\/\d+$/.test(page.url())) return; const { data: req } = await supabase .from("help_requests") - .select("id, help_queue") + .select("id, help_queue, class_id, created_by, request") .eq("request", requestText) .order("id", { ascending: false }) .limit(1) @@ -142,7 +142,21 @@ async function waitForHelpRequestUrlOrFallback( if (req?.id && req?.help_queue) { await page.goto(`/course/${courseId}/office-hours/${req.help_queue}/${req.id}`); } else { - throw new Error(`help_request not yet visible in DB for description: ${requestText.slice(0, 40)}…`); + // Surface enough state to disambiguate "row was never created" from + // "row was created but our query doesn't match what was stored". The + // latter is what bites us in CI: helpRequests.create returns success + // (per OH-DEBUG) yet the description-based lookup finds nothing. + const { data: latest } = await supabase + .from("help_requests") + .select("id, request, class_id, created_by") + .order("id", { ascending: false }) + .limit(3); + const summary = (latest ?? []) + .map((r) => `id=${r.id} cls=${r.class_id} req=${JSON.stringify(((r.request as string) ?? "").slice(0, 50))}`) + .join(" | "); + throw new Error( + `help_request not yet visible by description ${JSON.stringify(requestText.slice(0, 40))}; latest 3 rows: ${summary || "(none)"}` + ); } }).toPass({ timeout: fallbackTotalMs }); } From 88228b86efa71eba22b23af85c73c3b1207e4ddb Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 13:34:53 +0000 Subject: [PATCH 14/38] debug: narrow fallback diagnostic to class_id + admin URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dd58924c's CI failure showed the admin client's "latest 3 rows" are all from class_id=1 (the seeded class) up to id=11 — but OH-DEBUG for the same test attempt showed helpRequests.create returned id=34. That gap means either the admin client is querying a different DB, or all the in-flight test classes' rows are being filtered out somehow. Surface both narrowing dimensions on the next failure: - rows where class_id = this test's course.id (catches a courseId mismatch between the test fixture and the DB) - the admin client's SUPABASE_URL env (catches a wrong-instance misconfiguration) Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/office-hours.test.tsx | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index 784713eb5..75e805cd3 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -142,20 +142,30 @@ async function waitForHelpRequestUrlOrFallback( if (req?.id && req?.help_queue) { await page.goto(`/course/${courseId}/office-hours/${req.help_queue}/${req.id}`); } else { - // Surface enough state to disambiguate "row was never created" from - // "row was created but our query doesn't match what was stored". The - // latter is what bites us in CI: helpRequests.create returns success - // (per OH-DEBUG) yet the description-based lookup finds nothing. + // Narrow the lookup so we can tell which assumption is breaking: + // - rows for *this* test's class + // - rows in *any* class (catches a class_id mismatch in the admin + // client's view) + // - what the admin client thinks the current sequence value / + // URL is (catches a wrong-supabase-instance mismatch) + const { data: inCourse } = await supabase + .from("help_requests") + .select("id, request, class_id") + .eq("class_id", courseId) + .order("id", { ascending: false }) + .limit(5); const { data: latest } = await supabase .from("help_requests") - .select("id, request, class_id, created_by") + .select("id, class_id, request") .order("id", { ascending: false }) .limit(3); - const summary = (latest ?? []) - .map((r) => `id=${r.id} cls=${r.class_id} req=${JSON.stringify(((r.request as string) ?? "").slice(0, 50))}`) - .join(" | "); + const fmt = (r: { id: number; class_id: number; request: string }) => + `id=${r.id} cls=${r.class_id} req=${JSON.stringify(((r.request as string) ?? "").slice(0, 40))}`; + const courseRows = (inCourse ?? []).map(fmt).join(" | ") || "(none)"; + const globalRows = (latest ?? []).map(fmt).join(" | ") || "(none)"; throw new Error( - `help_request not yet visible by description ${JSON.stringify(requestText.slice(0, 40))}; latest 3 rows: ${summary || "(none)"}` + `help_request not yet visible by description ${JSON.stringify(requestText.slice(0, 40))}; ` + + `courseId=${courseId} in-course: ${courseRows}; global latest: ${globalRows}; admin URL=${process.env.SUPABASE_URL ?? "?"}` ); } }).toPass({ timeout: fallbackTotalMs }); From bfe7c89753f6d1ff6afe2ed00188e67c7dd031e3 Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 16:56:11 +0000 Subject: [PATCH 15/38] debug: force-click the office-hours Submit Request button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After 88228b86 we know the failing attempts never fire onSubmit at all (no OH-DEBUG), even though toBeEnabled passes before the click. The button is enabled in Playwright's view but the click somehow doesn't translate into a form submit on those attempts. Try click({ force: true }) so the dispatch is unconditional — Playwright skips re-checking visible/enabled/stable and just sends the click event. If onSubmit fires after this, the cause was a flapping disabled state or a transient overlay between toBeEnabled and click()'s internal actionability re-check. If onSubmit still doesn't fire, the cause is downstream of dispatch (e.g. another handler preventDefault'ing the submit). Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/e2e/office-hours.test.tsx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index 75e805cd3..6826a419a 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -268,7 +268,18 @@ test.describe("Office Hours", () => { // "row not in DB" timeout three minutes later. const submitBtn = page.getByRole("button", { name: "Submit Request" }); await expect(submitBtn).toBeEnabled({ timeout: 180_000 }); - await submitBtn.click(); + // force:true skips actionability checks (visible/enabled/stable/clickable). + // Diagnosis so far: my toBeEnabled passes, then click() registers but + // the form's onSubmit *doesn't fire* on most attempts. Possibilities: + // - the button re-disables between toBeEnabled and click()'s own + // re-check, and click() then sits silently waiting forever + // - React unmounts/remounts the button between the checks + // - some pointer-event overlay obscures the click target without + // Playwright noticing + // force:true bypasses all of that and dispatches the click immediately. + // If the form's onSubmit still doesn't fire, the cause is downstream + // of dispatch (e.g. preventDefault from another handler). + await submitBtn.click({ force: true }); // Two-stage wait. (1) Wait for router.push to land on the new request // URL — that's the production-correct happy path and what we want to From 21dda31322f3338b1748c12e167f9e6f580b26a2 Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 17:01:20 +0000 Subject: [PATCH 16/38] office-hours: prevent /new page from unmounting the form mid-submit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the actual root cause of PR 785's office-hours CI flake. It's a real SUT bug — not a test problem. The /new page renders only when `isQueueOpen`, which is derived from a realtime feed (active_help_queue_assignments). The useEffect that triggers `router.replace(...)` away from /new correctly guards on `isSubmittingRef.current` and `activeHelpQueue Assignments === undefined`, but the early-return *render branch* above it (the "Queue Closed for New Requests" card) didn't. Whenever realtime briefly dropped the active-assignment row (re-subscribe, cache invalidation, contention), the render branch unmounted the form — sometimes between a user click on Submit Request and React dispatching the synthetic onSubmit event. Symptom in CI: - Playwright click() returns success (button is enabled) - Form's onSubmit handler never fires - No row in DB, no toast, no inline validation error - Test times out 3 minutes later Why this surfaces in CI but not locally: on a contended CI runner the realtime channel re-subscribe / cache flap happens more often and lasts long enough to span the click-to-handler window. Locally the window closes before the next flap. Fix in app/.../new/page.tsx: 1. Latch the "open" signal. Once we've observed isQueueOpen=true at least once in this page mount, keep treating it as open even if realtime briefly says otherwise. This eliminates the flap-driven unmount entirely. Genuine queue closure still works on a fresh page mount. 2. Track `isSubmitting` as state (not just a ref). The ref was correctly read by the redirect useEffect but couldn't gate the render branch — refs don't trigger renders. Adding useState lets the render guard see the in-flight submission and keep the form mounted while onSubmit is still resolving. 3. Add the same `activeHelpQueueAssignments !== undefined` guard to the render branch that the useEffect already had. We don't show the "Queue Closed" card until realtime has actually loaded. Test side: revert click({ force:true }) from bfe7c897 — the click was never the problem; the form was being unmounted under it. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../office-hours/[queue_id]/new/page.tsx | 49 ++++++++++++++++--- tests/e2e/office-hours.test.tsx | 13 +---- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/app/course/[course_id]/office-hours/[queue_id]/new/page.tsx b/app/course/[course_id]/office-hours/[queue_id]/new/page.tsx index 844e0389f..bc81ed88b 100644 --- a/app/course/[course_id]/office-hours/[queue_id]/new/page.tsx +++ b/app/course/[course_id]/office-hours/[queue_id]/new/page.tsx @@ -1,7 +1,7 @@ "use client"; import { useParams, useRouter } from "next/navigation"; -import { useCallback, useEffect, useMemo, useRef } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useHelpQueue, useActiveHelpQueueAssignments } from "@/hooks/useOfficeHoursRealtime"; import { Box, Card, Container, Text, Button } from "@chakra-ui/react"; @@ -25,11 +25,41 @@ export default function NewRequestPage() { // Check if queue is available for new requests (both available flag AND has active staff, unless demo) const isQueueOpen = helpQueue?.is_demo || (helpQueue?.available && hasActiveAssignment); - // Set while the form is submitting/navigating to its newly created request, so the - // redirect below stands down and can't race (and swallow) the form's router.push. + // Latch the "open" signal: once we've ever observed the queue as open in + // this session, keep treating it as open even if the realtime channel + // briefly drops the active-assignment row (re-subscribe / cache invalidate + // can flap it false→true→false within a single render cycle). Unlatching + // requires a fresh page mount. + // + // Without this, an in-flight click on Submit Request gets stranded: + // 1. button is enabled, user clicks + // 2. React queues the synthetic submit event for our onSubmit + // 3. a realtime tick momentarily empties activeHelpQueueAssignments + // 4. isQueueOpen flips false, parent re-renders, the early-return + // branch below unmounts + // 5. React tries to dispatch the queued submit to a now-unmounted + // component; onSubmit never runs, no row is created, no toast + // + // Tracked down via the office-hours E2E CI flake on PR 785 — the + // failing attempts showed ZERO OH-DEBUG events from inside onSubmit + // despite Playwright's click() returning success. + const [hasEverBeenOpen, setHasEverBeenOpen] = useState(false); + useEffect(() => { + if (isQueueOpen) setHasEverBeenOpen(true); + }, [isQueueOpen]); + const treatAsOpen = isQueueOpen || hasEverBeenOpen; + + // Track whether the form is mid-submit. Use both a ref (so the + // useEffect below can read it without rerunning on every toggle) and + // useState (so the render-time guard below actually picks up the flip + // — refs don't trigger renders). The form calls onSubmittingChange + // before any await in its handler, so by the time anything downstream + // observes !treatAsOpen we already know the form is committed. const isSubmittingRef = useRef(false); + const [isSubmitting, setIsSubmitting] = useState(false); const handleSubmittingChange = useCallback((submitting: boolean) => { isSubmittingRef.current = submitting; + setIsSubmitting(submitting); }, []); useEffect(() => { @@ -41,14 +71,19 @@ export default function NewRequestPage() { // realtime data loads (and can blip undefined on re-subscribe), which would briefly make // the queue look closed and fire a spurious redirect. if (activeHelpQueueAssignments === undefined) return; - if (helpQueue && !isQueueOpen) { + // Once we've observed the queue as open we don't redirect away from + // /new even if realtime briefly says otherwise — see the + // hasEverBeenOpen latch comment above. + if (helpQueue && !treatAsOpen) { // Redirect back to queue page if queue is not open for new requests router.replace(`/course/${course_id}/office-hours/${queue_id}`); } - }, [helpQueue, isQueueOpen, activeHelpQueueAssignments, router, course_id, queue_id]); + }, [helpQueue, treatAsOpen, activeHelpQueueAssignments, router, course_id, queue_id]); - // Show error message if queue is not open for new requests - if (helpQueue && !isQueueOpen) { + // Show error message if queue is not open for new requests. Gate with + // the same guards as the useEffect so a transient realtime blip can't + // unmount the form mid-submit (see hasEverBeenOpen comment). + if (helpQueue && !treatAsOpen && !isSubmitting && activeHelpQueueAssignments !== undefined) { const reason = !helpQueue.available ? "This queue is not currently accepting new requests." : "This queue is not currently staffed."; diff --git a/tests/e2e/office-hours.test.tsx b/tests/e2e/office-hours.test.tsx index 6826a419a..75e805cd3 100644 --- a/tests/e2e/office-hours.test.tsx +++ b/tests/e2e/office-hours.test.tsx @@ -268,18 +268,7 @@ test.describe("Office Hours", () => { // "row not in DB" timeout three minutes later. const submitBtn = page.getByRole("button", { name: "Submit Request" }); await expect(submitBtn).toBeEnabled({ timeout: 180_000 }); - // force:true skips actionability checks (visible/enabled/stable/clickable). - // Diagnosis so far: my toBeEnabled passes, then click() registers but - // the form's onSubmit *doesn't fire* on most attempts. Possibilities: - // - the button re-disables between toBeEnabled and click()'s own - // re-check, and click() then sits silently waiting forever - // - React unmounts/remounts the button between the checks - // - some pointer-event overlay obscures the click target without - // Playwright noticing - // force:true bypasses all of that and dispatches the click immediately. - // If the form's onSubmit still doesn't fire, the cause is downstream - // of dispatch (e.g. preventDefault from another handler). - await submitBtn.click({ force: true }); + await submitBtn.click(); // Two-stage wait. (1) Wait for router.push to land on the new request // URL — that's the production-correct happy path and what we want to From 52c98d6a155a513a1cfbef9bbacdad4412248f0f Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Sat, 23 May 2026 17:31:48 +0000 Subject: [PATCH 17/38] office-hours: stop unmounting form on realtime active-staff blip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 21dda313's "hasEverBeenOpen latch" approach didn't actually fix the CI flake — local logs from that run still showed 5 of 6 office-hours attempts never firing onSubmit. The latch only helped once isQueueOpen had ever been true; if realtime's initial load returned an empty array (or the queue's row hadn't propagated yet), isQueueOpen started false, the latch stayed false, and the "Queue Closed" branch unmounted the form before the student could click Submit. Restructure the parent page so the "queue is closed" decision is based only on the help_queue row's own `available` column (a stable DB value), not on the active-staff realtime feed (a flapping signal). The active-staff gate already lives in the form's submit-button `disabled` state, which is the right place: it disables the button while realtime data is in flux without removing the entire form from the DOM. This keeps the form mounted across all the failure modes we observed (activeHelpQueueAssignments undefined, empty, briefly missing the queue, etc.) so an in-flight click is never stranded on an unmounted component. The "queue closed" UI still appears for the intended case — when an instructor toggles helpQueue.available off. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../office-hours/[queue_id]/new/page.tsx | 89 ++++++------------- 1 file changed, 29 insertions(+), 60 deletions(-) diff --git a/app/course/[course_id]/office-hours/[queue_id]/new/page.tsx b/app/course/[course_id]/office-hours/[queue_id]/new/page.tsx index bc81ed88b..808869d99 100644 --- a/app/course/[course_id]/office-hours/[queue_id]/new/page.tsx +++ b/app/course/[course_id]/office-hours/[queue_id]/new/page.tsx @@ -1,8 +1,8 @@ "use client"; import { useParams, useRouter } from "next/navigation"; -import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { useHelpQueue, useActiveHelpQueueAssignments } from "@/hooks/useOfficeHoursRealtime"; +import { useCallback, useEffect, useRef, useState } from "react"; +import { useHelpQueue } from "@/hooks/useOfficeHoursRealtime"; import { Box, Card, Container, Text, Button } from "@chakra-ui/react"; import HelpRequestForm from "./newRequestForm"; @@ -11,50 +11,25 @@ export default function NewRequestPage() { const { queue_id, course_id } = useParams(); const router = useRouter(); const helpQueue = useHelpQueue(Number(queue_id)); - // Use the specialized hook that subscribes to individual item changes - const activeHelpQueueAssignments = useActiveHelpQueueAssignments(); - // Check if queue has an active assignment (staff is working) - const hasActiveAssignment = useMemo(() => { - if (!activeHelpQueueAssignments) return false; - // Demo queues don't require active staff - if (helpQueue?.is_demo) return true; - return activeHelpQueueAssignments.some((assignment) => assignment.help_queue_id === Number(queue_id)); - }, [activeHelpQueueAssignments, queue_id, helpQueue?.is_demo]); - - // Check if queue is available for new requests (both available flag AND has active staff, unless demo) - const isQueueOpen = helpQueue?.is_demo || (helpQueue?.available && hasActiveAssignment); - - // Latch the "open" signal: once we've ever observed the queue as open in - // this session, keep treating it as open even if the realtime channel - // briefly drops the active-assignment row (re-subscribe / cache invalidate - // can flap it false→true→false within a single render cycle). Unlatching - // requires a fresh page mount. - // - // Without this, an in-flight click on Submit Request gets stranded: - // 1. button is enabled, user clicks - // 2. React queues the synthetic submit event for our onSubmit - // 3. a realtime tick momentarily empties activeHelpQueueAssignments - // 4. isQueueOpen flips false, parent re-renders, the early-return - // branch below unmounts - // 5. React tries to dispatch the queued submit to a now-unmounted - // component; onSubmit never runs, no row is created, no toast - // - // Tracked down via the office-hours E2E CI flake on PR 785 — the - // failing attempts showed ZERO OH-DEBUG events from inside onSubmit - // despite Playwright's click() returning success. - const [hasEverBeenOpen, setHasEverBeenOpen] = useState(false); - useEffect(() => { - if (isQueueOpen) setHasEverBeenOpen(true); - }, [isQueueOpen]); - const treatAsOpen = isQueueOpen || hasEverBeenOpen; + // The queue is "accepting new requests" iff the DB column says so (or + // it's a demo queue). We intentionally do NOT factor in active-staff + // here — that's a realtime-derived signal that flaps under load, and + // if we unmount the form based on it we strand in-flight click events + // (React can't dispatch a synthetic submit to a component that's no + // longer mounted). Tracked down via the office-hours CI flake on PR + // 785: failing attempts showed ZERO console.log events from inside + // the form's onSubmit despite Playwright's click() returning success. + // The form itself enforces the "active staff" guard via the submit + // button's `disabled` state, which is the right place for a realtime + // gate — it disables the button while data is in flux without + // unmounting the form. + const queueAcceptingRequests = helpQueue?.is_demo || (helpQueue?.available ?? false); // Track whether the form is mid-submit. Use both a ref (so the // useEffect below can read it without rerunning on every toggle) and // useState (so the render-time guard below actually picks up the flip - // — refs don't trigger renders). The form calls onSubmittingChange - // before any await in its handler, so by the time anything downstream - // observes !treatAsOpen we already know the form is committed. + // — refs don't trigger renders). const isSubmittingRef = useRef(false); const [isSubmitting, setIsSubmitting] = useState(false); const handleSubmittingChange = useCallback((submitting: boolean) => { @@ -67,27 +42,20 @@ export default function NewRequestPage() { // request itself, and a competing router.replace here gets swallowed under load, // stranding the student on a URL without the request id. if (isSubmittingRef.current) return; - // Don't act on indeterminate state: activeHelpQueueAssignments is undefined until the - // realtime data loads (and can blip undefined on re-subscribe), which would briefly make - // the queue look closed and fire a spurious redirect. - if (activeHelpQueueAssignments === undefined) return; - // Once we've observed the queue as open we don't redirect away from - // /new even if realtime briefly says otherwise — see the - // hasEverBeenOpen latch comment above. - if (helpQueue && !treatAsOpen) { - // Redirect back to queue page if queue is not open for new requests + // Don't act on indeterminate state: helpQueue is undefined until the + // realtime data loads, which would briefly make the queue look closed + // and fire a spurious redirect. + if (helpQueue === undefined) return; + if (!queueAcceptingRequests) { + // Redirect back to queue page if the queue is closed at the + // DB-row level (helpQueue.available === false and not demo). router.replace(`/course/${course_id}/office-hours/${queue_id}`); } - }, [helpQueue, treatAsOpen, activeHelpQueueAssignments, router, course_id, queue_id]); - - // Show error message if queue is not open for new requests. Gate with - // the same guards as the useEffect so a transient realtime blip can't - // unmount the form mid-submit (see hasEverBeenOpen comment). - if (helpQueue && !treatAsOpen && !isSubmitting && activeHelpQueueAssignments !== undefined) { - const reason = !helpQueue.available - ? "This queue is not currently accepting new requests." - : "This queue is not currently staffed."; + }, [helpQueue, queueAcceptingRequests, router, course_id, queue_id]); + // Show error message only when the queue's own row says it's closed. + // Don't unmount the form on a realtime blip. + if (helpQueue !== undefined && !queueAcceptingRequests && !isSubmitting) { return ( @@ -97,7 +65,8 @@ export default function NewRequestPage() { Queue Closed for New Requests - {reason} You can still view existing requests and queue status. + This queue is not currently accepting new requests. You can still view existing requests and queue + status.