Skip to content

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

Open

Description

@sidney-Hung

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 Mechanize UI color token enforcement check #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.

Activity

  1. sidney-Hung commented on Nov 6, 2026

    @sidney-Hung
    ContributorAuthor

    Correction: the reference count and the index.html lines above came from an older checkout. At 4de45b1 there are 18 references, not 13; the extra five are in web/peer-lobby.js (lines 25, 182, 247, 255 and 258), and the <details> spans web/index.html lines 1907 to 1979. The peer-lobby.js references run outside any try, and converting them would still throw on the null that storageArea() returns, so the PR exempts that file and I will open a separate issue for it. In Strict, storageError now reaches #grounding-error but stays hidden in the collapsed section; the PR leaves that for a follow-up.

  2. added 12 commits that reference this issue on Nov 6, 2026
    e23d9fd
    8a49e07
    7bdf6a2
    970da60
    fad6f07
    3d27e68
    58ec809
    3c991fc
    e6dafc1
    0f7281b
    c72a284
    d6acc77
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions