[sidney-Hung]

Problem and behavior

Where site data is blocked, reading the sessionStorage global throws. The lobby's Start handler named the global in the arguments of its storage helpers, so the exception came before any helper could handle it. Its message landed in #grounding-error, inside a collapsed <details>, and Start looked like it did nothing.

After this change, Practice without grounding opens the interview with storage unreachable. With grounding selected, Practice shows the existing "Clear grounding to start normally" message, since the packet can only reach the interview page through storage. When Practice starts without grounding, a ticked "share this focus" choice is not carried: storeSharedFocus() already treats a failed write as an unshared focus, and a refusal would land in the collapsed section like the original bug. Strict still stays on the lobby because it cannot run without storage, and #grounding-error now holds storageError instead of the raw exception text.

A real browser block also refuses the session cookie. On a server that admits guests, Practice reaches the interview. On a server that requires sign-in, with nobody signed in and session storage unreachable, the lobby says in #account-status as soon as the session answers: "Signing in needs site data, which this browser is blocking for this site. Allow site data for this site, then reload this page." Start stays on the lobby with its choices kept, and pressing it moves focus to the sign-in field, which has aria-describedby="account-status", so the reason is read with the field and is on screen at narrow widths. Without that guard, the storage fix alone would send the candidate to the interview page, which finds nobody signed in and bounces back to a reset lobby. Enter in the sign-in field and the Use GitHub button both still sign in, so a block on storage alone, with cookies working (Firefox's dom.storage.enabled=false may produce this; I have not checked), keeps a way in by keyboard or mouse; there the site-data message is not accurate. Under a full block the login request succeeds but no session follows, and #account-status shows "GitHub username recorded, but no signed-in session was found. Try signing in again." in place of the reason until a reload.

That guard needs the session state, which is unknown until /api/session answers. With session storage unreachable, Start is held while the answer is pending, without the site-data message since the server may still admit guests, and is released when it arrives. If the check fails with no login attempted, loginResult becomes "unchecked": Start stays held, the account row offers "Retry account check" with a message that does not mention signing in, the retry stays a GET, and Log out stays hidden. The cost: with storage blocked and the check failing, a guest also waits for a successful retry before Start.

A storage global named directly, or read as .localStorage or .sessionStorage on any object, under web/ now fails ESLint, apart from five existing references marked one by one, described below.

Limits, not changed here:

From reading the code, once Start reaches the interview with storage unreachable, some existing choices do not survive, as before for any page without storage: the "Show the LeetCode problem" tick, lobby settings after returning, candidate cases on reload, and in-interview preferences. The grounding refusal is also hidden if the candidate closed the section again before pressing Start.

Strict visibility: the storageError text stays hidden while the <details> is collapsed. Where to show a storage failure is an open question in #600, so this change leaves the <details> as it is.

web/peer-lobby.js has the same failure, at module scope when opened from an invite link, and its fix needs a decision about where the invite token lives once the page has removed it from the URL. Its five references each carry their own eslint-disable-next-line no-restricted-globals, and reportUnusedDisableDirectives is an error, so a sixth reference fails and each fix removes its own directive. I will open a follow-up issue for it, as agreed in review, and another to move storageArea, readStored and writeStored to web/storage.js so the lint message no longer points at the audio module.

Approach

The first commit resolves storageArea("sessionStorage"), added in d5ad668, once at module scope into tabStorage, with tabStorageBlocked derived from it, since the block lifts only on reload. The Start handler and clearGrounding pass tabStorage to their storage helpers, which already handle the null it can be. It also adds the site-data reason and the Start guard above, the hold while the session is pending, and the "unchecked" value of loginResult for a failed check, all only when session storage is unreachable. tests/browser/account.test.js asserted the call text, so it now matches each call through tabStorage and that tabStorage is bound to storageArea("sessionStorage"). tests/browser/lobby-render.test.js pinned the old failed-check state and relied on the runtime having no storage globals; it now makes both localStorage and sessionStorage throw before importing /app.js, and expects the new state. New lobby tests cover Start with storage unreachable in Practice, in Practice with grounding selected, in Practice with a shared focus ticked, in Strict, on a server that requires sign-in (the reason before Start, Enter reaching /api/login like the button, and focus at a 375-pixel width), while the session check is pending, and after it fails, including that Log out stays hidden and that a retry on a guest server does not ask for a sign-in that never happened. They make sessionStorage throw while cookies and localStorage still work, a partial block rather than a real browser setting. Most of the commit's size is these tests.

The second commit adds no-restricted-globals and no-restricted-properties for both storage names, with a message naming web/audio-output.js, and converts the seven references in web/history.js. They go through a small deviceStorage() helper that throws when no store can be reached, so each caller's catch still reports the failure and their results do not change. A new test checks that deleteReport still answers "failed" and "account-deleted-local-failed" with storage unreachable; it also passes on main, where the bare global throws inside the same try, and guards against a conversion that passes the null through. The storageArea() doc comment in web/audio-output.js now says that a caller that must tell a failure from an empty store has to check for null itself, which is what deviceStorage() does. The rules apply to web/**/*.js only; the review workbench is a local developer tool that does not import audio-output.js.

Validation

Each new Start test fails without its part of the change and passes with it. The deleteReport test fails when the null from storageArea() is passed through instead of deviceStorage(), and passes with it. npx eslint . passes across the repository, and reintroducing the module-scope read d5ad668 removed fails it at web/interview.js:480. A script also ran Start in Chromium with its real block-site-data setting: on a server that requires sign-in it stays on the lobby with the message above, and after allowing site data the open page still threw until it was reloaded, which is why the message asks for a reload.

Locally, interview-scroll.test.js fails the same way at the base commit on this machine. review-memory-tests needs a newer gh than the one installed, and shell-syntax flags scripts/git-post-commit.sh; this change touches neither. local-recorder.test.js failed once in a full browser lane and passed six of six runs alone, on this branch and on main; this change does not touch the files it covers.

Not checked: Firefox, the hold and retry states under a real browser block, and the real interview page under a real block on a server that admits guests. Pressing Start again rewrites the same reason, so a screen reader may not announce the repeat, though the field's description now carries it.

Close #600

^{Written for commit 8a49e07. Summary will update on new commits.}

[jserv]

The interactive-review check changed in #610: the explained label no longer passes it, and this pull request is over the large-change thresholds, so its next run needs a signed record of the interactive review.

1. Once: set up a signing key, as "Before your first large change" in CONTRIBUTING.md lists.
2. Rebase this branch onto main; the pre-push hook that signs and sends the record is there.
3. Run /codetrial-review on the branch and push. If the branch did not change, send the record with sh scripts/git-pre-push.sh --publish-review <fork-remote> [<branch>], then close and reopen this pull request.

Each refusal and its fix: https://github.com/sysprog21/codetrial/blob/main/docs/large-change-explanation.md#troubleshooting

[jserv]

Thank @sidney-Hung for contributing!