Skip to content

Ship the grounding snippets checked at start - #41

Merged
jserv merged 3 commits into
sysprog21:mainfrom
ColtenOuO:fix-grounding-start-snapshot
Sep 15, 2026
Merged

jserv merged 3 commits into
sysprog21:mainfrom
ColtenOuO:fix-grounding-start-snapshot

Conversation

@ColtenOuO

@ColtenOuO ColtenOuO commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to the review on #40. On a lobby that requires sign-in, the start handler read the checked snippet indexes before /api/login but looked up their text after it. A JD picked during that wait replaced the lists, so the packet sent snippets the candidate never checked. This predates #39 and #40.

Changes

  • web/app.js: copy the three grounding lists next to selected before the sign-in await, and build the packet from that copy.
  • tests/browser/lobby.test.js: hold /api/login, load a second JD during the hold, and assert the stored packet still has the first JD's snippet. Fails on main (ships Must know SQL), passes with the fix.

Summary by cubic

Builds the grounding packet when Start is pressed, before the sign-in round trip, so a JD chosen during login cannot replace the snippets the candidate checked. Previously, the handler looked up snippet text after login and could store the wrong selections; it now stores the packet only after sign-in succeeds, rechecks consent at that point, and rejects invalid starts before signing in.

  • Adds browser coverage for a JD change during login, consent withdrawal during login, and a refused start.

Written for commit 813b146. Summary will update on new commits.

Review in cubic

@ColtenOuO

Copy link
Copy Markdown
Contributor Author

Waiting for #40 to be merged without conflicts, then I'll mark this ready for review.

@ColtenOuO

Copy link
Copy Markdown
Contributor Author

The check failure here is not from this change. The only failing gate is cargo-audit, which flags rustls 0.23.43 under RUSTSEC-2026-0285 (TLS 1.3 handshake messages accepted across encryption level boundaries), published 2026-09-14. The advisory landed between runs: this branch passed in run 34843141378 and failed in run 34874694806 the same day.

The fix is rustls >= 0.23.45, in #42 . I will rebase this branch once that merges.

@jserv

jserv commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

The fix is rustls >= 0.23.45, in #42 . I will rebase this branch once that merges.

Go ahead.

@ColtenOuO
ColtenOuO force-pushed the fix-grounding-start-snapshot branch from 17fbd8d to 57bd1e5 Compare September 14, 2026 20:13
@ColtenOuO

Copy link
Copy Markdown
Contributor Author

Fixed the implementation according to the review on #40.

@ColtenOuO
ColtenOuO marked this pull request as ready for review September 15, 2026 06:39

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

Comment thread tests/browser/lobby.test.js Outdated
Comment thread web/app.js
The start handler read which snippets were checked before the sign-in
round trip, but looked their text up in the extracted lists after it.
A JD picked while a gated start was waiting on /api/login replaced
those lists, so the stored indexes pointed into the new file and the
packet carried snippets the candidate never checked. The lists are now
copied alongside the selection, the way the rest of the form is read.
Copying the extracted lists before the sign-in round trip kept the
indices and the text together, but still assembled the packet after
it, and a start the packet was always going to refuse spent a login
first. The packet is now built with the rest of the form, and only
stored once the sign-in has answered.
Building the packet before the sign-in round trip also snapshotted
consent, so a candidate who cleared grounding or unchecked consent
while the login was pending still had the pre-click snippets stored.
Consent is now read again when the packet is stored, and a withdrawn
consent stores nothing, which is what clearing did on main.

The existing snapshot test unchecked a box Clear had already
unchecked, so it never exercised that path. It now re-checks consent
before the login resolves, and a separate test pins that withdrawing
consent alone leaves no grounding in session storage.
@ColtenOuO
ColtenOuO force-pushed the fix-grounding-start-snapshot branch from 2fd628c to 813b146 Compare September 15, 2026 14:26
@jserv
jserv merged commit 2c9e151 into sysprog21:main Sep 15, 2026
6 checks passed
@jserv

jserv commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Thank @ColtenOuO for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants