[PR #1430] [MERGED] fix(staffml): stale closure in gauntlet keyboard handler and importProgress partial-write #9099

New Issue

GiteaMirror · 2026-05-03T01:20:31-05:00

GiteaMirror commented

2026-05-03 01:20:31 -05:00

📋 Pull Request Information

Original PR: https://github.com/harvard-edge/cs249r_book/pull/1430
Author: @Shashank-Tripathi-07
Created: 4/22/2026
Status: ✅ Merged
Merged: 4/22/2026
Merged by: @profvjreddi

Base: dev ← Head: fix/staffml-stale-closure-and-import-rollback

📝 Commits (1)

08cb905 fix(staffml): stale closure in gauntlet keyboard handler and importProgress partial-write

📊 Changes

2 files changed (+55 additions, -40 deletions)

View changed files

📝 interviews/staffml/src/app/gauntlet/page.tsx (+22 -21)
📝 interviews/staffml/src/lib/progress.ts (+33 -19)

📄 Description

What this fixes

Two independent bugs in StaffML that silently corrupt user state under specific but realistic conditions.

Bug 1: Stale closure in gauntlet keyboard shortcuts

File: interviews/staffml/src/app/gauntlet/page.tsx

scoreAndNext was a plain function (not wrapped in useCallback). The keyboard shortcut useEffect had a dependency array of [phase, showAnswer], which meant the event handler captured stale values of currentIdx and scores from the render in which the effect last ran.

Repro: open a gauntlet, answer Q1 with the mouse (state updates), then press key 2 to score Q2. The handler fires with currentIdx = 0 still captured, so Q1 gets scored a second time and Q2 is skipped entirely.

Fix: wrap scoreAndNext in useCallback with its actual deps (questions, currentIdx, scores, timeRemaining, selectedDuration, selectedTrack, selectedLevel), add it to the keyboard effect's dependency array, and move the effect below the scoreAndNext declaration to satisfy TypeScript's temporal dead zone check.

Bug 2: importProgress partial-write with no rollback

File: interviews/staffml/src/lib/progress.ts

importProgress validated and wrote keys to localStorage one at a time. If attempts passed validation and was written, then gauntlets failed its Array.isArray check, the function returned false but STORAGE_KEY already held the new attempts data while GAUNTLET_KEY still held the old gauntlet history. The user ends up with a silently inconsistent state: a mix of new and old data that skews cross-session analytics.

The same class of failure applies to QuotaExceededError (private browsing mode with limited quota) mid-write.

Fix: validate all shapes first before touching storage, snapshot all relevant localStorage keys upfront, and restore the snapshot if any write throws.

Verification

Both changes pass tsc --noEmit with zero errors. No new dependencies introduced.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/harvard-edge/cs249r_book/pull/1430 **Author:** [@Shashank-Tripathi-07](https://github.com/Shashank-Tripathi-07) **Created:** 4/22/2026 **Status:** ✅ Merged **Merged:** 4/22/2026 **Merged by:** [@profvjreddi](https://github.com/profvjreddi) **Base:** `dev` ← **Head:** `fix/staffml-stale-closure-and-import-rollback` --- ### 📝 Commits (1) - [`08cb905`](https://github.com/harvard-edge/cs249r_book/commit/08cb905b012c838a505dc6e4829a15d95e8800d2) fix(staffml): stale closure in gauntlet keyboard handler and importProgress partial-write ### 📊 Changes **2 files changed** (+55 additions, -40 deletions) <details> <summary>View changed files</summary> 📝 `interviews/staffml/src/app/gauntlet/page.tsx` (+22 -21) 📝 `interviews/staffml/src/lib/progress.ts` (+33 -19) </details> ### 📄 Description ## What this fixes Two independent bugs in StaffML that silently corrupt user state under specific but realistic conditions. ### Bug 1: Stale closure in gauntlet keyboard shortcuts **File:** `interviews/staffml/src/app/gauntlet/page.tsx` `scoreAndNext` was a plain function (not wrapped in `useCallback`). The keyboard shortcut `useEffect` had a dependency array of `[phase, showAnswer]`, which meant the event handler captured stale values of `currentIdx` and `scores` from the render in which the effect last ran. Repro: open a gauntlet, answer Q1 with the mouse (state updates), then press key `2` to score Q2. The handler fires with `currentIdx = 0` still captured, so Q1 gets scored a second time and Q2 is skipped entirely. Fix: wrap `scoreAndNext` in `useCallback` with its actual deps (`questions`, `currentIdx`, `scores`, `timeRemaining`, `selectedDuration`, `selectedTrack`, `selectedLevel`), add it to the keyboard effect's dependency array, and move the effect below the `scoreAndNext` declaration to satisfy TypeScript's temporal dead zone check. ### Bug 2: importProgress partial-write with no rollback **File:** `interviews/staffml/src/lib/progress.ts` `importProgress` validated and wrote keys to `localStorage` one at a time. If `attempts` passed validation and was written, then `gauntlets` failed its `Array.isArray` check, the function returned `false` but `STORAGE_KEY` already held the new attempts data while `GAUNTLET_KEY` still held the old gauntlet history. The user ends up with a silently inconsistent state: a mix of new and old data that skews cross-session analytics. The same class of failure applies to `QuotaExceededError` (private browsing mode with limited quota) mid-write. Fix: validate all shapes first before touching storage, snapshot all relevant `localStorage` keys upfront, and restore the snapshot if any write throws. ## Verification Both changes pass `tsc --noEmit` with zero errors. No new dependencies introduced. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

GiteaMirror added the pull-request label 2026-05-03 01:20:31 -05:00

GiteaMirror closed this issue

2026-05-03 01:20:31 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/cs249r_book#9099