Start does nothing visible in the lobby when site data is blocked

Aperta
  • Qualcosa non va?
#600 1 commento 0 reazioni 1 assegnatario Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@sidney-Hung ci sta già lavorando.

Dal 6/11/2026.

  • #603 di @sidney-Hung — aperta

Valutazione

Questa issue non è ancora stata valutata.

Descrizione

Problem

Where site data is blocked, touching the sessionStorage global throws instead of returning a storage that refuses. The lobby's Start handler passes the global itself to resetAssignments, storeGroundingPacket and storeSharedFocus (web/app.js lines 829 to 831 at 4de45b1), so the exception is raised while the arguments are evaluated. The handler's catch writes the message into #grounding-error, which sits inside the collapsed "Optional interview context" <details> (web/index.html lines 1907 to 1979). The candidate sees a Start button that does nothing.

Current behavior

Reproduced on 4de45b1 in Practice with a problem selected, using the same override tests/browser/candidate-cases.test.js uses for unreachable storage:

  1. Make sessionStorage throw: Object.defineProperty(window, "sessionStorage", { get() { throw new DOMException("blocked", "SecurityError"); } }).
  2. Press Start. The page stays in the lobby, the button reads "Start interview" again, and #grounding-error holds "blocked", hidden in the collapsed section. Every later press does the same.
  3. After a reload, make the same getter return null, which is what storageArea("sessionStorage") in web/audio-output.js answers in that situation, and press Start. The lobby navigates to the interview page.

Cause

The callees already handle a missing storage: storeSharedFocus() treats a failure as an unshared focus, and storeGroundingPacket() with no packet ignores one. In Strict, previousAssignment() checks !storage and throws its own storageError. The bare global throws before any of that runs. d5ad668 fixed the same class of failure on the interview page and added storageArea() for it, but the lobby was not converted.

Nothing stops the next such reference either. The rule in .claude/skills/codetrial-review/rules/browser.md ("Browser storage: guard it") is enforced only by review. At 4de45b1, 18 references name a storage global directly:

  • web/app.js lines 829, 830, 831 and 910
  • web/history.js lines 37, 46, 76, 197, 304, 351 and 377 (storage ||= localStorage)
  • web/peer-lobby.js lines 25, 182, 247, 255 and 258
  • .claude/skills/codetrial-review/workbench/app.js lines 713 and 732

The history.js and workbench ones are inside a try and harmless today. The peer-lobby.js ones are not: three run outside any try, one at module scope. Converting them to storageArea() would still throw on the null it returns, so fixing that page needs a decision about where the invite token lives when storage is unreachable.

Desired outcome

  • With storage unreachable, Start in Practice opens the interview, and Strict reaches storageError instead of the raw exception.
  • A storage global named directly under web/ fails the ESLint gate with a message pointing at storageArea(). Built-in no-restricted-globals and no-restricted-properties cover this. Against 4de45b1 that configuration reports exactly the 18 references above and nothing else. Against the parent of d5ad668, it reports the module-scope reads in web/interview.js (lines 194 and 196) and the default parameters in web/audio-output.js (lines 105 and 116) that d5ad668 fixed.

Proposed scope

  • Convert the web/app.js and web/history.js references to storageArea(...). history.js imports it from web/audio-output.js; moving the storage helpers to a module of their own is a separate refactor.
  • Add the two rules to the browser tier of eslint.config.mjs, for web/**/*.js only, with web/peer-lobby.js exempt and the reason stated in a comment.
  • Add browser tests for Start with storage unreachable, in Practice and in Strict.
  • Deliver this as one pull request with two commits: the Start fix with its tests, then the rule with the history.js conversion.

Out of scope

  • web/peer-lobby.js, which needs its own fix (see Cause).
  • The review workbench, a local developer tool that does not import audio-output.js.
  • Where a storage failure is shown (see Open question).
  • Other storage behavior (quota handling, key scoping), CSS, and the color-token rule in #578.

Open question

In Strict, storageError reaches #grounding-error, but that element is inside the collapsed "Optional interview context" <details>, so the message is still not visible. Should a storage failure be shown elsewhere, for example next to Start, or should the catch open the <details>? The section is about grounding, so opening it for an unrelated storage error may confuse a candidate.

Acceptance criteria

  • The new Start tests fail on main and pass with the change.
  • eslint . passes on the converted tree, and reintroducing the module-scope read that d5ad668 removed fails it.
  • The existing unreachable-storage tests in tests/browser/candidate-cases.test.js still pass.
  • ./scripts/test.sh passes, apart from checks that fail on main as well, with skipped checks reviewed.

Related

d5ad668 (the interview-page fix this follows), #578 (mechanizes a different review rule), #593 (adds a lobby storage write through storageArea(), which the proposed rule already accepts; it does not touch the Start lines above).

Edited: the first version listed 13 references and index.html lines 1904 to 1976, taken from an older checkout; both are now verified against 4de45b1, which adds web/peer-lobby.js. The scope now records the choices made where the open questions went unanswered: history.js imports from audio-output.js, the rule covers web/ only with peer-lobby.js exempt, and the work lands as one pull request with two commits.

Lingua principale
Rust
Stelle
223
Fork
148
Merge medio
1g 10h
PR unite (30g)
227

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di sysprog21/codetrial

Tutte le issue di sysprog21/codetrial

Issue simili

Altre issue su Rust

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.