From db3562fdc714565be06ae6145d5451ca23002ec9 Mon Sep 17 00:00:00 2001 From: HHChien Date: Sat, 7 Nov 2026 00:00:11 +0800 Subject: [PATCH] Let reports set the next practice focus Carry selected report improvements safely through interviews and replay while preserving account ownership and lobby settings. Change-Id: I974546bfeb9ffc69db5be0bab32220f46b647727 --- scripts/rust-style-ceilings.json | 4 +- src/accounts.rs | 5 + src/accounts/sessions.rs | 6 +- src/web/auth.rs | 65 +- src/web/interviews.rs | 11 +- src/web/token.rs | 8 +- tests/browser/account.test.js | 11 +- tests/browser/download.test.js | 19 +- tests/browser/history.test.js | 14 + tests/browser/interview-lifecycle.test.js | 397 ++++++++++ tests/browser/lobby.test.js | 831 +++++++++++++++++++- tests/browser/problem-picker.test.js | 233 ++++++ tests/browser/render.test.js | 6 + tests/browser/replay-practice-focus.test.js | 352 +++++++++ tests/browser/report-recovery.test.js | 1 + tests/browser/strict-selection.test.js | 96 ++- tests/web.rs | 15 + tests/web/account_focus.rs | 252 ++++++ tests/web/accounts.rs | 52 +- tests/web/token.rs | 50 -- web/app.js | 165 +++- web/interview.js | 72 +- web/problem-picker.js | 133 ++++ web/render.js | 4 +- web/replay.js | 31 + web/styles.css | 25 +- 26 files changed, 2659 insertions(+), 199 deletions(-) create mode 100644 tests/browser/replay-practice-focus.test.js create mode 100644 tests/web/account_focus.rs diff --git a/scripts/rust-style-ceilings.json b/scripts/rust-style-ceilings.json index ec7689e5f..79cd3156d 100644 --- a/scripts/rust-style-ceilings.json +++ b/scripts/rust-style-ceilings.json @@ -23,7 +23,7 @@ "tests/unit/livekit/session.rs": 6023, "tests/unit/livekit/turn.rs": 3405, "tests/unit/tasks/session.rs": 1861, - "tests/web/accounts.rs": 1808, + "tests/web/accounts.rs": 1760, "tests/web/recording.rs": 2496, - "tests/web/token.rs": 2151 + "tests/web/token.rs": 2101 } diff --git a/src/accounts.rs b/src/accounts.rs index 50ad71dd9..c7477e776 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -56,6 +56,11 @@ pub struct SignedInUser { /// `None` for every self-declared login, which is the whole point: a typed /// handle is a label, and a recording is delivered to a person. pub verified_email: Option, + /// Stable account identity stored in `users.github_id`. + /// Positive values are GitHub account IDs. Negative values are fresh + /// random identities for self-declared accounts. Unlike `users.id`, this + /// value is not recycled when a throwaway account is deleted. + pub account_identity: i64, /// GitHub's numeric account ID, for a login GitHub vouched for; `None` for /// a self-declared one. pub github_id: Option, diff --git a/src/accounts/sessions.rs b/src/accounts/sessions.rs index a6d607974..c74ff8185 100644 --- a/src/accounts/sessions.rs +++ b/src/accounts/sessions.rs @@ -107,14 +107,16 @@ pub fn session_user( // how an unverified value becomes a delivery target one // refactor later. let verified: i64 = row.get(4)?; + let account_identity: i64 = row.get(5)?; Ok(SignedInUser { id: row.get(0)?, // Trusted only under create_session's caller invariant. - identity_verified: row.get::<_, i64>(5)? > 0, + identity_verified: account_identity > 0, login: row.get(1)?, avatar_url: row.get(2)?, verified_email: row.get::<_, Option>(3)?.filter(|_| verified != 0), - github_id: u64::try_from(row.get::<_, i64>(5)?) + account_identity, + github_id: u64::try_from(account_identity) .ok() .filter(|id| *id > 0), }) diff --git a/src/web/auth.rs b/src/web/auth.rs index 41b186f4f..6a24c7b4d 100644 --- a/src/web/auth.rs +++ b/src/web/auth.rs @@ -52,6 +52,15 @@ pub(crate) fn github_oauth_scope(recording: bool) -> &'static str { } } +/// A stable browser-facing account identifier without exposing the sequential +/// `users.id`. The page only compares owner ids for equality. +pub(crate) fn account_owner_id(accounts: &Accounts, account_identity: i64) -> String { + crate::token::sign_hs256( + &accounts.config().session_secret, + &format!("account-owner:{account_identity}"), + ) +} + pub fn login_config(config: &WebServerConfig) -> Option { let (Some(session_secret), Some(db_path)) = ( config @@ -561,32 +570,40 @@ pub(crate) async fn session_handler( .as_ref() .and_then(|dispatcher| dispatcher.candidate_video()); match current_user(state.accounts.as_ref(), request.headers()).await { - Ok(Some(user)) => json_response( - StatusCode::OK, - super::tasks::with_built_in_task( - &state, - with_peer_flags( + Ok(Some(user)) => { + let Some(accounts) = state.accounts.as_ref() else { + return state.accounts_error(); + }; + let account_id = account_owner_id(accounts, user.account_identity); + json_response( + StatusCode::OK, + super::tasks::with_built_in_task( &state, - json!({ - "signedIn": true, - "identityVerified": user.identity_verified, - - // Task mode admits only a login GitHub vouched for; the - // task page asks before the learner types a PIN. - "githubSignedIn": user.github_id.is_some(), - "loginRequired": login_required, - "guestModeAvailable": state.guest_mode_available(), - "deviceLogin": device_client_id(&state.config).is_some(), - "maxDurationMin": max_duration_min, - "candidateVideo": candidate_video, - "user": { - "login": user.login, - "avatarUrl": user.avatar_url - } - }), + with_peer_flags( + &state, + json!({ + "signedIn": true, + "identityVerified": user.identity_verified, + + // Task mode admits only a login GitHub vouched for; + // the task page asks before the learner types a + // PIN. + "githubSignedIn": user.github_id.is_some(), + "loginRequired": login_required, + "guestModeAvailable": state.guest_mode_available(), + "deviceLogin": device_client_id(&state.config).is_some(), + "maxDurationMin": max_duration_min, + "candidateVideo": candidate_video, + "user": { + "id": account_id, + "login": user.login, + "avatarUrl": user.avatar_url + } + }), + ), ), - ), - ), + ) + } Ok(None) => json_response( StatusCode::OK, super::tasks::with_built_in_task( diff --git a/src/web/interviews.rs b/src/web/interviews.rs index c21d6b9ff..3b3595f93 100644 --- a/src/web/interviews.rs +++ b/src/web/interviews.rs @@ -16,7 +16,7 @@ use crate::accounts::{ }; use crate::current_epoch_seconds_i64; -use super::auth::Owner; +use super::auth::{Owner, account_owner_id}; use super::recordings::finish_recording; use super::{ AppState, MAX_BODY_BYTES, MAX_REPORT_BYTES, json_response, rate_limited_response, @@ -33,8 +33,15 @@ use super::{ pub const REPLAY_RATE_LIMIT: u32 = 120; pub(crate) async fn list_reports_handler(Owner { accounts, user }: Owner) -> Response { + let account_id = account_owner_id(&accounts, user.account_identity); match blocking(move || list_reports(&accounts, user.id)).await { - Ok(reports) => json_response(StatusCode::OK, json!({ "reports": reports })), + Ok(reports) => json_response( + StatusCode::OK, + json!({ + "accountId": account_id, + "reports": reports, + }), + ), Err(_) => json_response( StatusCode::INTERNAL_SERVER_ERROR, json!({ "error": "Could not read reports." }), diff --git a/src/web/token.rs b/src/web/token.rs index 4d5991729..239e11101 100644 --- a/src/web/token.rs +++ b/src/web/token.rs @@ -18,7 +18,7 @@ use crate::config::{DEFAULT_DURATION_MIN, MAX_DURATION_MIN, MIN_DURATION_MIN}; use crate::current_epoch_seconds; use crate::token::{LivekitTokenInput, TOKEN_TTL_SECONDS, livekit_observer_token, livekit_token}; -use super::auth::{Owner, current_user}; +use super::auth::{Owner, account_owner_id, current_user}; use super::consent::{checked_consent, claim_consent}; use super::pool::{ProviderChoice, room_and_available_provider}; use super::{ @@ -882,6 +882,12 @@ pub(crate) async fn token_handler( // The same granted length the agent reads from its token metadata. "durationMin": response.duration_min }); + if let Some(owner) = owner.as_ref() { + body["accountId"] = json!(account_owner_id( + &owner.accounts, + owner.user.account_identity + )); + } if let Some(assignment) = assignment { body["problemPage"] = json!(assignment.page); body["previousAssignment"] = json!(assignment.receipt); diff --git a/tests/browser/account.test.js b/tests/browser/account.test.js index 39a257e3f..9a05364f3 100644 --- a/tests/browser/account.test.js +++ b/tests/browser/account.test.js @@ -97,7 +97,7 @@ test("interview history routes through the shared persistence helper", () => { assert.match(functionBody(script, "renderReport"), /saveResult: null/); assert.match( functionBody(script, "renderReportSaveStatus"), - /querySelector\("#done"\)\.disabled = state\.downloadingReport/, + /querySelector\("#done"\)\.disabled =\s*state\.reportReceiving \|\| state\.downloadingReport/, ); assert.match( functionBody(script, "renderReport"), @@ -306,10 +306,11 @@ test("optional interview profile is accessible, bounded, and omitted when blank" 'if (profile.targetCompany) destination.searchParams.set("company", profile.targetCompany)', ); assert.match(interview, /const interviewProfile = \{/); - // The focus travels in session storage, never the address bar. - assert.match( + // The focus travels in session storage, never the address bar, and sharing + // consent is checked again immediately before the handoff is stored. + assertIncludesCompact( app, - /storeSharedFocus\(sessionStorage, focus\?\.weakness \?\? null\)/, + "storeSharedFocus(sessionStorage, nodes.practiceFocusShareInput.checked ? (focus?.weakness ?? null) : null)", ); assert.doesNotMatch(app, /searchParams\.set\("focus"/); assert.match(interview, /practiceFocus: consumeSharedFocus\(tabStorage\)/); @@ -1509,6 +1510,7 @@ test("updateCategoryComparison renders category progress and degrades cleanly wh "loadReportHistory", "updateCategoryComparison", "downloadReport", + "setPracticeFocusButtonsDisabled", "buildMarkdown", ] .map((name) => `${functionBody(interview, name)}\n}`) @@ -1593,6 +1595,7 @@ test("updateCategoryComparison renders category progress and degrades cleanly wh nodes: { report: { innerHTML: "existing-report-dom", + querySelectorAll: () => [], querySelector(selector) { if (selector === "#download-report") return downloadButton; if (selector === ".category-comparison") { diff --git a/tests/browser/download.test.js b/tests/browser/download.test.js index 20ebb9ff2..676522b5e 100644 --- a/tests/browser/download.test.js +++ b/tests/browser/download.test.js @@ -12,6 +12,7 @@ for (const [path, name] of [ const downloads = []; const button = { disabled: false }; const done = { disabled: false }; + const practice = [{ disabled: false }, { disabled: false }]; const scope = { state: { downloadingReport: false, @@ -20,6 +21,7 @@ for (const [path, name] of [ }, nodes: { report: { + querySelectorAll: () => practice, querySelector: (selector) => (selector === "#done" ? done : button), }, }, @@ -40,6 +42,7 @@ for (const [path, name] of [ "scope", `with (scope) { ${name === "exportSavedReport" ? `${functionBody(read(path), "downloadSavedReport")}\n}` : ""} + ${name === "downloadReport" ? `${functionBody(read(path), "setPracticeFocusButtonsDisabled")}\n}` : ""} ${functionBody(read(path), name)} } return ${name}; @@ -56,6 +59,7 @@ for (const [path, name] of [ run, button, done, + practice, lookups, downloads, scope, @@ -68,7 +72,10 @@ for (const [path, name] of [ const h = harness(); const first = h.run(); assert.equal(h.button.disabled, true); - if (name === "downloadReport") assert.equal(h.done.disabled, true); + if (name === "downloadReport") { + assert.equal(h.done.disabled, true); + assert.ok(h.practice.every((button) => button.disabled)); + } await h.run(); await h.run(); assert.equal(h.lookups.length, 1); @@ -77,7 +84,10 @@ for (const [path, name] of [ await first; assert.equal(h.downloads.length, 1); assert.equal(h.button.disabled, false); - if (name === "downloadReport") assert.equal(h.done.disabled, false); + if (name === "downloadReport") { + assert.equal(h.done.disabled, false); + assert.ok(h.practice.every((button) => !button.disabled)); + } const next = h.run(); assert.equal(h.button.disabled, true); h.lookups[1].resolve(null); @@ -101,7 +111,10 @@ for (const [path, name] of [ else h.lookups[0].resolve(null); await rejected; assert.equal(h.button.disabled, false); - if (name === "downloadReport") assert.equal(h.done.disabled, false); + if (name === "downloadReport") { + assert.equal(h.done.disabled, false); + assert.ok(h.practice.every((button) => !button.disabled)); + } h.scope.downloadMarkdown = originalDownload; const retry = h.run(); h.lookups[1].resolve(null); diff --git a/tests/browser/history.test.js b/tests/browser/history.test.js index b2bf273a8..941ad5d48 100644 --- a/tests/browser/history.test.js +++ b/tests/browser/history.test.js @@ -466,6 +466,8 @@ test("the response window panel says what the number is worth, in words a test c // Wiring: selectors, module paths, tags, classes, dataset and event names. "", "/lib.js", + "./problem-picker.js", + "./audio-output.js", "/render.js", "/report-original-problem.js", "/api/reports", @@ -477,6 +479,9 @@ test("the response window panel says what the number is worth, in words a test c "aria-current", "true", "[data-moment]", + "[data-practice-focus]", + "[data-practice-focus-status]", + "sessionStorage", "replay-item", "replay-line", "2d", @@ -522,6 +527,7 @@ test("the response window panel says what the number is worth, in words a test c "Could not load the replay for this interview.", "Could not load the report.", "Could not read your recordings.", + "Could not save this practice focus.", "No report was saved for this interview.", "Sign in to see your recordings.", "No test run before this point.", @@ -540,6 +546,7 @@ test("the response window panel says what the number is worth, in words a test c // page calls and the separators it joins values with. The events request // is here in the shape it is sent, which is the other half of the done // condition for this feature. + "/", "/api/recordings", "/api/recordings/", "/api/recordings//events?avatar=history", @@ -569,6 +576,13 @@ test("the response window panel says what the number is worth, in words a test c // person reading the test can see it. }); +test("replay practice actions carry the selected improvement to the lobby", () => { + const loadReport = functionBody(script, "loadReport"); + assert.match(loadReport, /data-practice-focus/); + assert.match(loadReport, /storeSelectedPracticeFocus/); + assert.match(loadReport, /window\.location\.href = "\/"/); +}); + test("the replay page asks for the avatar history and renders it beside the moments", () => { // The done condition is the request, not what the page then draws with it. // What it draws is asserted in `tests/browser/replay-render.test.js`, which diff --git a/tests/browser/interview-lifecycle.test.js b/tests/browser/interview-lifecycle.test.js index f2a0d81e2..63ebf5e0a 100644 --- a/tests/browser/interview-lifecycle.test.js +++ b/tests/browser/interview-lifecycle.test.js @@ -243,6 +243,403 @@ test("the Leave button releases the warning before tearing down the session", as assert.equal(target.removed, 1); }); +for (const admittedOwner of ["account:A", "device", null]) { + test(`leaving stores a selected focus only for admitted owner ${admittedOwner}`, async () => { + const source = read("web/interview.js"); + const calls = []; + const target = new CountingTarget(); + const guard = createBeforeUnloadGuard(target); + guard.setEnabled(true); + const location = {}; + Object.defineProperty(location, "href", { + set(value) { + calls.push(`navigate:${value}`); + }, + }); + target.location = location; + const beforeUnloadGuard = { + dispose() { + calls.push("guard-off"); + guard.dispose(); + }, + }; + const focus = { + phase: "Test", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }; + const tabStorage = {}; + const storeSelectedPracticeFocus = ( + storage, + owner, + selected, + interviewMode, + sourceReportId, + ) => { + assert.equal(owner, admittedOwner); + assert.equal(storage, tabStorage); + assert.deepEqual(selected, focus); + assert.equal(interviewMode, "coding"); + assert.equal(sourceReportId, "report-1"); + calls.push("store-focus"); + return owner !== null; + }; + const body = functionBody(source, "leaveRoom"); + const leaveRoom = new Function( + "state", + "reportRecovery", + "beforeUnloadGuard", + "window", + "stopEndingClock", + "stopAvatar", + "stopLocalMedia", + "localRecording", + "storeSelectedPracticeFocus", + "tabStorage", + "admittedPracticeFocusOwner", + "sessionPromise", + "mode", + `${body}\n}\nreturn leaveRoom;`, + )( + { + phase: "ending", + reportReceiving: false, + leaving: false, + reportId: "report-1", + room: { + disconnect() { + calls.push("disconnect"); + return Promise.resolve(); + }, + }, + }, + { finish: () => calls.push("finish") }, + beforeUnloadGuard, + target, + () => calls.push("stop-clock"), + () => calls.push("stop-avatar"), + () => calls.push("stop-media"), + { stop: async () => {} }, + storeSelectedPracticeFocus, + tabStorage, + admittedOwner, + Promise.resolve({ signedIn: true, user: { id: "initial-account" } }), + "coding", + ); + await leaveRoom(focus); + assert.deepEqual( + calls, + admittedOwner + ? [ + "store-focus", + "guard-off", + "stop-clock", + "disconnect", + "stop-avatar", + "stop-media", + "navigate:/", + ] + : ["store-focus"], + ); + }); +} + +test("a failed practice focus save keeps the report open", async () => { + const source = read("web/interview.js"); + const calls = []; + const focus = { + phase: "Test", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }; + const status = { + hidden: true, + textContent: "", + }; + const practiceButton = { + parentElement: { + querySelector(selector) { + assert.equal(selector, "[data-practice-focus-status]"); + return status; + }, + }, + }; + const target = { + location: { + set href(value) { + calls.push(`navigate:${value}`); + }, + }, + }; + const state = { + phase: "report", + reportReceiving: false, + reportPersisted: true, + downloadingReport: false, + leaving: false, + reportId: "report-1", + room: null, + }; + const body = functionBody(source, "leaveRoom"); + const leaveRoom = new Function( + "state", + "reportRecovery", + "beforeUnloadGuard", + "window", + "stopEndingClock", + "stopAvatar", + "stopLocalMedia", + "localRecording", + "storeSelectedPracticeFocus", + "tabStorage", + "admittedPracticeFocusOwner", + "sessionPromise", + "mode", + `${body}\n}\nreturn leaveRoom;`, + )( + state, + {}, + { + dispose() { + calls.push("guard-off"); + }, + }, + target, + () => calls.push("stop-clock"), + () => calls.push("stop-avatar"), + () => calls.push("stop-media"), + { + stop: async () => { + calls.push("stop-recording"); + }, + }, + () => { + calls.push("store-focus"); + return false; + }, + {}, + "account:A", + Promise.resolve(), + "coding", + ); + await leaveRoom(focus, practiceButton); + assert.deepEqual(calls, ["store-focus"]); + assert.equal(state.leaving, false); + assert.equal(status.hidden, false); + assert.equal(status.textContent, "Could not save this practice focus."); +}); + +test("a second leave is ignored while the first is still stopping the recording", async () => { + const source = read("web/interview.js"); + const calls = []; + let releaseRecording; + const recordingStopped = new Promise((resolve) => { + releaseRecording = resolve; + }); + const focus = { + phase: "Test", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }; + const target = { + location: { + set href(value) { + calls.push(`navigate:${value}`); + }, + }, + }; + const body = functionBody(source, "leaveRoom"); + const leaveRoom = new Function( + "state", + "reportRecovery", + "beforeUnloadGuard", + "window", + "stopEndingClock", + "stopAvatar", + "stopLocalMedia", + "localRecording", + "storeSelectedPracticeFocus", + "tabStorage", + "admittedPracticeFocusOwner", + "sessionPromise", + "mode", + `${body}\n}\nreturn leaveRoom;`, + )( + { + phase: "report", + reportReceiving: false, + reportPersisted: true, + downloadingReport: false, + leaving: false, + reportId: "report-1", + room: null, + }, + {}, + { + dispose() { + calls.push("guard-off"); + }, + }, + target, + () => calls.push("stop-clock"), + () => calls.push("stop-avatar"), + () => calls.push("stop-media"), + { + stop: async () => { + calls.push("stop-recording"); + await recordingStopped; + }, + }, + () => { + calls.push("store-focus"); + return true; + }, + {}, + "account:A", + Promise.resolve(), + "coding", + ); + const first = leaveRoom(focus); + // Let the first call reach localRecording.stop(). + await Promise.resolve(); + const second = leaveRoom(focus); + await second; + assert.deepEqual(calls, [ + "store-focus", + "guard-off", + "stop-clock", + "stop-avatar", + "stop-media", + "stop-recording", + ]); + releaseRecording(); + await first; + assert.deepEqual(calls, [ + "store-focus", + "guard-off", + "stop-clock", + "stop-avatar", + "stop-media", + "stop-recording", + "navigate:/", + ]); +}); + +test("cancelling an unsafe leave does not store the selected practice focus", async () => { + const source = read("web/interview.js"); + const calls = []; + const focus = { + phase: "Test", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }; + const target = { + confirm(message) { + calls.push(`confirm:${message}`); + return false; + }, + location: {}, + }; + const beforeUnloadGuard = { + dispose() { + calls.push("guard-off"); + }, + }; + const storeSelectedPracticeFocus = () => { + calls.push("store-focus"); + }; + const body = functionBody(source, "leaveRoom"); + const leaveRoom = new Function( + "state", + "reportRecovery", + "beforeUnloadGuard", + "window", + "stopEndingClock", + "stopAvatar", + "stopLocalMedia", + "localRecording", + "storeSelectedPracticeFocus", + "tabStorage", + "admittedPracticeFocusOwner", + "sessionPromise", + `${body}\n}\nreturn leaveRoom;`, + )( + { + phase: "report", + reportReceiving: false, + reportPersisted: false, + downloadingReport: false, + room: null, + }, + { + stop() { + calls.push("recovery-stop"); + }, + finish() { + calls.push("recovery-finish"); + }, + }, + beforeUnloadGuard, + target, + () => calls.push("stop-clock"), + () => calls.push("stop-avatar"), + () => calls.push("stop-media"), + { + stop: async () => { + calls.push("stop-recording"); + }, + }, + storeSelectedPracticeFocus, + {}, + ); + await leaveRoom(focus); + assert.deepEqual(calls, [ + "confirm:This report may not be saved. Leave this page anyway?", + ]); +}); + +test("practice focus actions wait for saving, receipt and download independently", () => { + const buttons = [{ disabled: false }, { disabled: false }]; + const state = {}; + const nodes = { + report: { querySelectorAll: () => buttons }, + }; + const body = functionBody( + read("web/interview.js"), + "setPracticeFocusButtonsDisabled", + ); + const setDisabled = new Function( + "nodes", + "state", + `${body}\n}\nreturn setPracticeFocusButtonsDisabled;`, + )(nodes, state); + for (const receiving of [false, true]) { + for (const downloading of [false, true]) { + for (const saved of [false, true]) { + Object.assign(state, { + reportReceiving: receiving, + downloadingReport: downloading, + reportSaveComplete: saved, + }); + setDisabled(false); + assert.deepEqual( + buttons.map((button) => button.disabled), + [ + receiving || downloading || !saved, + receiving || downloading || !saved, + ], + ); + } + } + } + setDisabled(true); + assert.ok(buttons.every((button) => button.disabled)); +}); + /// `saveHistory` from web/interview.js, compiled against the module state it /// reads. `whiteboard` and `mode` are passed separately, as the page derives /// them, so a test can set one without the other. diff --git a/tests/browser/lobby.test.js b/tests/browser/lobby.test.js index 7bf2df76f..568988c0a 100644 --- a/tests/browser/lobby.test.js +++ b/tests/browser/lobby.test.js @@ -32,7 +32,7 @@ let base = ""; /// What the stub answers with. Set per test before the page loads, because the /// whole point of most of these is what the lobby does once these arrive. -let session = { signedIn: true, user: { login: "candidate" } }; +let session = { signedIn: true, user: { id: "A", login: "candidate" } }; let reports = []; /// Endpoint paths the stub answers with a 500 instead of a body, so the tests /// about a server that cannot answer are about an actual failure rather than a @@ -100,11 +100,17 @@ before(async () => { reports = []; return json({ deleted }); } + // Capture the owner when this request is admitted. A held response must not + // change owners just because another tab changes the session while it waits. + const accountId = + session.signedIn && typeof session.user?.id === "string" + ? session.user.id + : null; if (holdReports) await holdReports; - return json({ reports }); + return json({ accountId, reports }); } if (url.pathname === "/api/login") { - session = { signedIn: true, user: { login: "candidate" } }; + session = { signedIn: true, user: { id: "A", login: "candidate" } }; if (holdLogin) await holdLogin; return json({ ok: true }); } @@ -125,7 +131,7 @@ before(async () => { /// leaked a stub mode into everything after it and turned a single red test /// into a run nobody could read. beforeEach(() => { - session = { signedIn: true, user: { login: "candidate" } }; + session = { signedIn: true, user: { id: "A", login: "candidate" } }; reports = []; failing = new Set(); holdReports = null; @@ -3374,6 +3380,43 @@ lobbyTest( }, ); +lobbyTest( + "unchecking Share while Start verifies the account does not share the practice focus", + async (page) => { + reports = [focusedAttempt(EASY[0])]; + await lobby(page); + const share = page.locator("#practice-focus-share-input"); + await share.check(); + let releaseSession; + holdSession = new Promise((resolve) => { + releaseSession = resolve; + }); + const requested = page.waitForRequest( + (request) => new URL(request.url()).pathname === "/api/session", + ); + try { + const starting = page.click("#start"); + // Start has already captured the focus and is now waiting for the + // ownership recheck. + await requested; + await share.uncheck(); + releaseSession(); + holdSession = null; + await starting; + await page.waitForURL(/\/interview/); + } finally { + releaseSession?.(); + holdSession = null; + } + assert.equal( + await page.evaluate(() => + sessionStorage.getItem("codetrial.sharedPracticeFocus"), + ), + null, + ); + }, +); + lobbyTest( "loading a resume keeps the JD requirements already checked", async (page) => { @@ -4115,7 +4158,14 @@ for (const first of [0, 1]) { url === "/api/reports" ? new Promise((resolve) => window.pendingHistory.push((reports) => - resolve(new Response(JSON.stringify({ reports }))), + resolve( + new Response( + JSON.stringify({ + accountId: "A", + reports, + }), + ), + ), ), ) : fetch(url, ...args); @@ -4311,6 +4361,485 @@ lobbyTest( }, ); +lobbyTest( + "Practice this selects the exact report improvement for the lobby", + async (page) => { + reports = [ + focusedAttempt(EASY[0], { + weaknesses: ["Test boundaries", "Explain the algorithm clearly"], + }), + ]; + Object.assign(reports[0].payload.report.improvementPlan[1], { + phase: "Algorithm", + drill: "Say the algorithm before coding", + successCriterion: "Explain the full approach before implementation", + }); + const before = await lobby(page); + assert.match(before.focus, /Test boundaries/); + const beforeMode = await page + .locator('[data-mode][aria-pressed="true"]') + .getAttribute("data-mode"); + const editorOptions = page + .getByRole("group", { name: "Editor options" }) + .getByRole("checkbox"); + const beforeEditorOptions = await editorOptions.evaluateAll((inputs) => + inputs.map((input) => ({ + checked: input.checked, + disabled: input.disabled, + })), + ); + const history = page.locator("#attempt-history"); + await history.getByRole("button", { name: "Open report" }).click(); + const practiceFocus = page.locator("#practice-focus"); + await practiceFocus.evaluate((node) => { + node.practiceSelectionScroll = null; + node.scrollIntoView = (options) => { + node.practiceSelectionScroll = options; + }; + }); + + const practiceButtons = history.getByRole("button", { + name: "Practice this", + }); + await practiceButtons.nth(1).click(); + assert.deepEqual( + await practiceFocus.evaluate((node) => node.practiceSelectionScroll), + { block: "nearest" }, + ); + const after = await snapshot(page); + assert.equal( + after.card, + before.card, + "selecting a practice focus must not change the problem", + ); + assert.equal( + after.duration, + before.duration, + "selecting a practice focus must not change session duration", + ); + assert.deepEqual( + after.levels, + before.levels, + "selecting a practice focus must not change difficulty filters", + ); + assert.equal( + await page + .locator('[data-mode][aria-pressed="true"]') + .getAttribute("data-mode"), + beforeMode, + "selecting a practice focus must not change interview format", + ); + assert.deepEqual( + await editorOptions.evaluateAll((inputs) => + inputs.map((input) => ({ + checked: input.checked, + disabled: input.disabled, + })), + ), + beforeEditorOptions, + "selecting a practice focus must not change editor settings", + ); + assert.match( + after.focus, + /Selected practice focus · Algorithm: Explain the algorithm clearly\. Drill: Say the algorithm before coding\. Success: Explain the full approach before implementation\./, + ); + assert.equal( + await page.locator("#practice-focus-share-input").isChecked(), + false, + ); + await page.click("#practice-focus-share-input"); + await page.click("#start"); + await page.waitForURL(/\/interview/); + assert.equal( + await page.evaluate(() => + sessionStorage.getItem("codetrial.sharedPracticeFocus"), + ), + "Explain the algorithm clearly", + ); + }, +); + +lobbyTest( + "a selected practice focus carried from the interview overrides the automatic focus", + async (page) => { + reports = [ + focusedAttempt(EASY[0], { + weaknesses: ["Test boundaries"], + }), + ]; + await page.addInitScript(() => { + sessionStorage.setItem( + "codetrial.selectedPracticeFocus", + JSON.stringify({ + owner: "account:A", + focus: { + phase: "Algorithm", + weakness: "Explain the algorithm clearly", + drill: "Walk through the algorithm step by step", + successCriterion: "Explain each step before coding", + }, + }), + ); + }); + const state = await lobby(page); + assert.match(state.focus, /Selected practice focus/); + assert.match(state.focus, /Algorithm/); + assert.match(state.focus, /Explain the algorithm clearly/); + assert.match(state.focus, /Walk through the algorithm step by step/); + assert.match(state.focus, /Explain each step before coding/); + assert.doesNotMatch(state.focus, /Test boundaries/); + assert.equal( + await page.locator("#practice-focus-share-input").isChecked(), + false, + ); + }, +); + +lobbyTest( + "a selected practice focus survives a lobby reload until Start", + async (page) => { + reports = [ + focusedAttempt(EASY[0], { + weaknesses: ["Automatic focus"], + }), + ]; + await page.addInitScript(() => { + const seeded = "codetrial.test.selectedPracticeFocusSeeded"; + if (sessionStorage.getItem(seeded)) return; + sessionStorage.setItem( + "codetrial.selectedPracticeFocus", + JSON.stringify({ + owner: "account:A", + focus: { + phase: "Algorithm", + weakness: "Explain the algorithm clearly", + drill: "Walk through the algorithm step by step", + successCriterion: "Explain each step before coding", + }, + }), + ); + sessionStorage.setItem(seeded, "1"); + }); + await lobby(page); + assert.match((await snapshot(page)).focus, /Explain the algorithm clearly/); + + assert.notEqual( + await page.evaluate(() => + sessionStorage.getItem("codetrial.selectedPracticeFocus"), + ), + null, + ); + await page.reload({ waitUntil: "domcontentloaded" }); + await page.waitForFunction( + () => document.querySelector("#recommendation").textContent !== "", + ); + + assert.match((await snapshot(page)).focus, /Explain the algorithm clearly/); + assert.notEqual( + await page.evaluate(() => + sessionStorage.getItem("codetrial.selectedPracticeFocus"), + ), + null, + ); + }, +); + +lobbyTest("Start consumes the selected practice focus", async (page) => { + await page.addInitScript(() => { + const seeded = "codetrial.test.selectedPracticeFocusSeeded"; + if (sessionStorage.getItem(seeded)) return; + sessionStorage.setItem( + "codetrial.selectedPracticeFocus", + JSON.stringify({ + owner: "account:A", + focus: { + phase: "Algorithm", + weakness: "Explain the algorithm clearly", + drill: "Walk through the algorithm step by step", + successCriterion: "Explain each step before coding", + }, + }), + ); + sessionStorage.setItem(seeded, "1"); + }); + await lobby(page); + assert.notEqual( + await page.evaluate(() => + sessionStorage.getItem("codetrial.selectedPracticeFocus"), + ), + null, + ); + await page.click("#start"); + await page.waitForURL(/\/interview/); + assert.equal( + await page.evaluate(() => + sessionStorage.getItem("codetrial.selectedPracticeFocus"), + ), + null, + ); +}); + +lobbyTest( + "a persisted pageshow reconciles the selected practice focus from storage", + async (page) => { + reports = [ + focusedAttempt(EASY[0], { + weaknesses: ["Automatic focus"], + }), + ]; + await page.addInitScript(() => { + sessionStorage.setItem( + "codetrial.selectedPracticeFocus", + JSON.stringify({ + owner: "account:A", + focus: { + phase: "Algorithm", + weakness: "Old selected focus", + drill: "Old drill", + successCriterion: "Old success", + }, + }), + ); + }); + await lobby(page); + assert.match((await snapshot(page)).focus, /Old selected focus/); + await page.check("#practice-focus-share-input"); + await page.evaluate(() => { + sessionStorage.removeItem("codetrial.selectedPracticeFocus"); + }); + await restore(page); + await page.waitForFunction(() => + document + .querySelector("#practice-focus") + .textContent.includes("Automatic focus"), + ); + const state = await snapshot(page); + assert.doesNotMatch(state.focus, /Old selected focus/); + assert.match(state.focus, /Automatic focus/); + assert.equal( + await page.locator("#practice-focus-share-input").isChecked(), + false, + ); + }, +); + +lobbyTest( + "deleting saved history clears the selected practice focus", + async (page) => { + reports = [focusedAttempt(EASY[0])]; + await page.addInitScript(() => { + sessionStorage.setItem( + "codetrial.selectedPracticeFocus", + JSON.stringify({ + owner: "account:A", + focus: { + phase: "Test", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }, + }), + ); + }); + await lobby(page); + assert.match((await snapshot(page)).focus, /Selected practice focus/); + page.once("dialog", (dialog) => dialog.accept()); + await page.click("#delete-reports"); + await reaches( + page, + () => document.querySelector("#report-delete-status").textContent !== "", + ); + assert.equal( + await page.evaluate(() => + sessionStorage.getItem("codetrial.selectedPracticeFocus"), + ), + null, + ); + assert.doesNotMatch( + (await snapshot(page)).focus, + /Selected practice focus/, + ); + }, +); + +lobbyTest( + "a whiteboard practice focus keeps the phase label shown in the report", + async (page) => { + const attempt = focusedAttempt(EASY[0], { + phase: "Coding", + weaknesses: ["Trace the algorithm clearly"], + }); + attempt.payload.interviewMode = "whiteboard"; + attempt.payload.report.interviewMode = "whiteboard"; + reports = [attempt]; + await lobby(page); + const history = page.locator("#attempt-history"); + await history.getByRole("button", { name: "Open report" }).click(); + assert.match(await history.textContent(), /Trace/); + await history.getByRole("button", { name: /Practice this:/ }).click(); + + const state = await snapshot(page); + assert.match(state.focus, /Selected practice focus · Trace:/); + assert.doesNotMatch(state.focus, /Selected practice focus · Coding:/); + }, +); + +lobbyTest( + "deleting the source report clears the selected practice focus", + async (page) => { + const attempt = focusedAttempt(EASY[0]); + reports = [ + { + ...attempt, + id: "focus-source", + payload: { + ...attempt.payload, + id: "focus-source", + }, + }, + ]; + await page.addInitScript(() => { + sessionStorage.setItem( + "codetrial.selectedPracticeFocus", + JSON.stringify({ + owner: "account:A", + focus: { + phase: "Test", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }, + sourceReportId: "focus-source", + }), + ); + }); + await lobby(page); + assert.match((await snapshot(page)).focus, /Selected practice focus/); + page.once("dialog", (dialog) => dialog.accept()); + await page.locator("#attempt-history [data-delete-report]").click(); + await reaches( + page, + () => document.querySelector("#report-delete-status").textContent !== "", + ); + assert.equal( + await page.evaluate(() => + sessionStorage.getItem("codetrial.selectedPracticeFocus"), + ), + null, + ); + assert.doesNotMatch( + (await snapshot(page)).focus, + /Selected practice focus/, + ); + }, +); + +lobbyTest( + "deleting another report keeps the selected practice focus", + async (page) => { + const source = focusedAttempt(EASY[0]); + const unrelated = focusedAttempt(EASY[1]); + reports = [ + { + ...source, + id: "focus-source", + payload: { + ...source.payload, + id: "focus-source", + date: "2026-01-02T00:00:00Z", + }, + }, + { + ...unrelated, + id: "unrelated-report", + payload: { + ...unrelated.payload, + id: "unrelated-report", + date: "2026-01-01T00:00:00Z", + }, + }, + ]; + await page.addInitScript(() => { + sessionStorage.setItem( + "codetrial.selectedPracticeFocus", + JSON.stringify({ + owner: "account:A", + focus: { + phase: "Test", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }, + sourceReportId: "focus-source", + }), + ); + }); + await lobby(page); + assert.match((await snapshot(page)).focus, /Selected practice focus/); + const deleteButtons = page.locator("#attempt-history [data-delete-report]"); + page.once("dialog", (dialog) => dialog.accept()); + // Reports are rendered newest first, so delete the older unrelated report. + await deleteButtons.nth(1).click(); + await reaches( + page, + () => document.querySelector("#report-delete-status").textContent !== "", + ); + assert.notEqual( + await page.evaluate(() => + sessionStorage.getItem("codetrial.selectedPracticeFocus"), + ), + null, + ); + assert.match((await snapshot(page)).focus, /Selected practice focus/); + assert.match((await snapshot(page)).focus, /Test boundaries/); + }, +); + +lobbyTest( + "a missing report deletion keeps the selected practice focus", + async (page) => { + const attempt = focusedAttempt(EASY[0]); + reports = [ + { + ...attempt, + id: "focus-source", + payload: { + ...attempt.payload, + id: "focus-source", + }, + }, + ]; + await lobby(page); + const history = page.locator("#attempt-history"); + await history.getByRole("button", { name: "Open report" }).click(); + await history.getByRole("button", { name: /Practice this:/ }).click(); + assert.match((await snapshot(page)).focus, /Selected practice focus/); + const before = await page.evaluate(() => + JSON.parse(sessionStorage.getItem("codetrial.selectedPracticeFocus")), + ); + assert.equal(before.sourceReportId, "focus-source"); + // Simulate the report having already been removed elsewhere. The row is + // still visible in this stale lobby, but DELETE now reports that nothing + // was removed. + reports = []; + page.once("dialog", (dialog) => dialog.accept()); + await page.locator("#attempt-history [data-delete-report]").click(); + await reaches( + page, + () => document.querySelector("#report-delete-status").textContent !== "", + ); + assert.match( + await page.locator("#report-delete-status").textContent(), + /not found/i, + ); + const after = await page.evaluate(() => + JSON.parse(sessionStorage.getItem("codetrial.selectedPracticeFocus")), + ); + assert.equal(after.sourceReportId, "focus-source"); + assert.match((await snapshot(page)).focus, /Selected practice focus/); + }, +); + lobbyTest( "a problem already passed is not what gets recommended", async (page) => { @@ -5441,7 +5970,7 @@ lobbyTest( // The login lands while this page is not the one in front: on the way out to // the interview, or in another tab. The cached page knows none of it, so what // it holds about the session is now wrong in every part. - session = { signedIn: true, user: { login: "candidate" } }; + session = { signedIn: true, user: { id: "A", login: "candidate" } }; await restore(page); await reaches( page, @@ -5970,7 +6499,7 @@ lobbyTest( // `stale_after` reaps that recording ten minutes before the interview ends. session = { signedIn: true, - user: { login: "candidate" }, + user: { id: "A", login: "candidate" }, maxDurationMin: 45, }; @@ -6002,7 +6531,7 @@ lobbyTest( // what this server is able to record. session = { signedIn: true, - user: { login: "candidate" }, + user: { id: "A", login: "candidate" }, maxDurationMin: 30, }; @@ -6022,7 +6551,7 @@ lobbyTest( async (page) => { session = { signedIn: true, - user: { login: "candidate" }, + user: { id: "A", login: "candidate" }, maxDurationMin: 90, }; @@ -6086,7 +6615,7 @@ for (const account of [true, false]) { lobbyTest( `the problem filter keeps ${account ? "account" : "device"} history and charts in sync`, async (page) => { - session = { signedIn: account, user: { login: "candidate" } }; + session = { signedIn: account, user: { id: "A", login: "candidate" } }; const saved = [55, 85, 70, 90, 80].map((score, index) => { const selected = index % 2 === 0; const problemId = selected ? EASY[0] : "retired-problem"; @@ -6784,7 +7313,7 @@ lobbyTest( // Keep the page usable even when the server limit fits no duration button. session = { signedIn: true, - user: { login: "candidate" }, + user: { id: "A", login: "candidate" }, maxDurationMin: 15, }; @@ -7485,7 +8014,7 @@ lobbyTest( route.fulfill({ json: { signedIn: true, - user: { login: "candidate" }, + user: { id: "A", login: "candidate" }, maxDurationMin: 30, }, }), @@ -7513,7 +8042,14 @@ lobbyTest( url === "/api/reports" ? new Promise((resolve) => window.pendingHistory.push((reports) => - resolve(new Response(JSON.stringify({ reports }))), + resolve( + new Response( + JSON.stringify({ + accountId: "A", + reports, + }), + ), + ), ), ) : fetch(url, ...args); @@ -7813,7 +8349,7 @@ for (const trigger of ["header", "Start"]) { "Retry account check", ); assert.equal(await page.locator("#start").isDisabled(), true); - session = { signedIn: true, user: { login: "candidate" } }; + session = { signedIn: true, user: { id: "A", login: "candidate" } }; } await page.click("#login-link"); await awaitReady(page); @@ -9449,7 +9985,10 @@ lobbyTest( const first = ++requests === 1; if (first) await gate; await route.fulfill({ - json: { reports: [savedAttempt(first ? EASY[0] : EASY[1])] }, + json: { + accountId: "A", + reports: [savedAttempt(first ? EASY[0] : EASY[1])], + }, }); }); const pending = page.waitForRequest(`${base}/api/reports`); @@ -9626,7 +10165,7 @@ lobbyTest( async (page) => { session = { signedIn: true, - user: { login: "candidate" }, + user: { id: "A", login: "candidate" }, candidateVideo: true, }; await lobbyAfterSession(page, "Signed in as candidate"); @@ -9663,7 +10202,7 @@ for (const candidateVideo of [true, false]) { async (page) => { session = { signedIn: true, - user: { login: "candidate" }, + user: { id: "A", login: "candidate" }, candidateVideo, }; await lobbyAfterSession(page, "Signed in as candidate"); @@ -9697,7 +10236,7 @@ for (const [name, setup, status] of [ ], [ "is missing", - () => (session = { signedIn: true, user: { login: "candidate" } }), + () => (session = { signedIn: true, user: { id: "A", login: "candidate" } }), "Signed in as candidate", ], ["request fails", () => failing.add("/api/session"), "Signed out"], @@ -9719,7 +10258,7 @@ lobbyTest( async (page) => { session = { signedIn: true, - user: { login: "candidate" }, + user: { id: "A", login: "candidate" }, candidateVideo: true, }; await lobbyAfterSession(page, "Signed in as candidate"); @@ -9744,7 +10283,7 @@ lobbyTest( async (page) => { session = { signedIn: true, - user: { login: "candidate" }, + user: { id: "A", login: "candidate" }, candidateVideo: true, }; // The stale failure changes nothing on the page when it is handled @@ -9796,7 +10335,7 @@ for (const candidateVideo of [true, false]) { async (page) => { session = { signedIn: true, - user: { login: "candidate" }, + user: { id: "A", login: "candidate" }, candidateVideo, }; await lobbyAfterSession(page, "Signed in as candidate"); @@ -9832,7 +10371,7 @@ lobbyTest( async (page) => { session = { signedIn: true, - user: { login: "candidate" }, + user: { id: "A", login: "candidate" }, candidateVideo: false, }; await page.addInitScript(() => @@ -9859,7 +10398,7 @@ lobbyTest( async (page) => { session = { signedIn: true, - user: { login: "candidate" }, + user: { id: "A", login: "candidate" }, candidateVideo: true, }; await page.addInitScript(() => @@ -10007,7 +10546,14 @@ lobbyTest( url === "/api/reports" ? new Promise((resolve) => window.pendingHistory.push((reports) => - resolve(new Response(JSON.stringify({ reports }))), + resolve( + new Response( + JSON.stringify({ + accountId: "A", + reports, + }), + ), + ), ), ) : fetch(url, ...args); @@ -10117,7 +10663,7 @@ lobbyTest( lobbyTest( "the peer practice link shows only for a signed-in, verified member on a server that enables it", async (page) => { - const user = { login: "candidate" }; + const user = { id: "A", login: "candidate" }; const cases = [ [{ peerEnabled: true, identityVerified: true }, true], [{ peerEnabled: true, user: { ...user, identityVerified: true } }, true], @@ -10141,3 +10687,238 @@ lobbyTest( } }, ); + +for (const nextSession of [ + { signedIn: true, user: { id: "B", login: "candidate" } }, + { signedIn: false }, +]) { + lobbyTest( + `selected practice focus is cleared after switching to ${nextSession.signedIn ? "another account with the same handle" : "device owner"}`, + async (page) => { + reports = [focusedAttempt(EASY[0])]; + await lobby(page); + const history = page.locator("#attempt-history"); + await history.getByRole("button", { name: "Open report" }).click(); + await history.getByRole("button", { name: /Practice this:/ }).click(); + assert.match((await snapshot(page)).focus, /Selected practice focus/); + session = nextSession; + reports = []; + let release; + holdSession = new Promise((resolve) => { + release = resolve; + }); + const requested = page.waitForRequest( + (request) => new URL(request.url()).pathname === "/api/session", + ); + await restore(page); + await requested; + assert.equal(await page.locator("#practice-focus").isHidden(), true); + release(); + holdSession = null; + await page.waitForFunction( + () => + sessionStorage.getItem("codetrial.selectedPracticeFocus") === null, + ); + assert.doesNotMatch( + (await snapshot(page)).focus, + /Selected practice focus/, + ); + }, + ); +} + +lobbyTest( + "report owner mismatch discards rows and restarts the account lookup", + async (page) => { + let sessionRequests = 0; + let reportRequests = 0; + await page.route(`${base}/api/session`, (route) => { + sessionRequests++; + return route.fulfill({ + json: { + signedIn: true, + user: { id: "A", login: "candidate" }, + }, + }); + }); + await page.route(`${base}/api/reports`, (route) => { + reportRequests++; + + if (reportRequests === 1) { + const report = focusedAttempt(EASY[0]); + report.payload.problemTitle = "Account B report"; + + return route.fulfill({ + json: { + accountId: "B", + reports: [report], + }, + }); + } + return route.fulfill({ + json: { + accountId: "A", + reports: [], + }, + }); + }); + await page.goto(`${base}/`); + await page.waitForFunction( + () => document.querySelector("#start")?.disabled === false, + ); + assert.ok( + sessionRequests >= 2, + "an owner mismatch must restart the account lookup", + ); + assert.ok( + reportRequests >= 2, + "the restarted account lookup must reload account reports", + ); + assert.doesNotMatch( + await page.locator("#attempt-history").textContent(), + /Account B report/, + ); + }, +); + +lobbyTest( + "a persistent report owner mismatch retries once and then reports an error", + async (page) => { + let sessionRequests = 0; + let reportRequests = 0; + await page.route(`${base}/api/session`, (route) => { + sessionRequests++; + return route.fulfill({ + json: { + signedIn: true, + user: { id: "A", login: "candidate" }, + }, + }); + }); + await page.route(`${base}/api/reports`, (route) => { + reportRequests++; + return route.fulfill({ + json: { + accountId: "B", + reports: [focusedAttempt(EASY[0])], + }, + }); + }); + await page.goto(`${base}/`); + await page.waitForFunction(() => + document + .querySelector("#progress-summary") + ?.textContent.includes("Could not load saved account progress."), + ); + assert.equal( + sessionRequests, + 2, + "an owner mismatch should retry the account lookup only once", + ); + assert.equal( + reportRequests, + 2, + "an owner mismatch should retry reports only once", + ); + assert.doesNotMatch( + await page.locator("#attempt-history").textContent(), + /Practice this/, + ); + }, +); + +lobbyTest( + "Start does not share a practice focus after the account changes", + async (page) => { + session = { + signedIn: true, + user: { id: "A", login: "candidate-a" }, + }; + reports = [ + focusedAttempt(EASY[0], { + weaknesses: ["Account A focus"], + }), + ]; + await lobby(page); + const history = page.locator("#attempt-history"); + await history.getByRole("button", { name: "Open report" }).click(); + await history + .getByRole("button", { name: /Practice this:/ }) + .first() + .click(); + await page.check("#practice-focus-share-input"); + // Another tab changes the signed-in account before Start. + session = { + signedIn: true, + user: { id: "B", login: "candidate-b" }, + }; + await page.click("#start"); + await page.waitForURL(/\/interview/); + assert.equal( + await page.evaluate(() => + sessionStorage.getItem("codetrial.sharedPracticeFocus"), + ), + null, + ); + }, +); + +lobbyTest( + "stale Practice this cannot carry another account's report during refresh", + async (page) => { + session = { + signedIn: true, + user: { id: "A", login: "candidate" }, + }; + reports = [focusedAttempt(EASY[0])]; + await lobby(page); + const history = page.locator("#attempt-history"); + await history.getByRole("button", { name: "Open report" }).click(); + // Keep a reference to account A's old report action. + const stalePractice = history.getByRole("button", { + name: /Practice this:/, + }); + // Switch identity to B, but hold B's reports so A's old row is still + // present while the new account is already known. + session = { + signedIn: true, + user: { id: "B", login: "candidate" }, + }; + reports = []; + let releaseReports; + holdReports = new Promise((resolve) => { + releaseReports = resolve; + }); + const requested = page.waitForRequest( + (request) => new URL(request.url()).pathname === "/api/reports", + ); + await restore(page); + await requested; + try { + // First protection: stale report actions are disabled during refresh. + assert.equal(await stalePractice.isDisabled(), true); + + // Bypass the disabled-button UI behavior and exercise the retained + // listener itself. Its generation guard must still reject the action. + await stalePractice.evaluate((button) => { + button.dispatchEvent( + new MouseEvent("click", { + bubbles: true, + cancelable: true, + }), + ); + }); + assert.equal( + await page.evaluate(() => + sessionStorage.getItem("codetrial.selectedPracticeFocus"), + ), + null, + ); + assert.equal(await page.locator("#practice-focus").isHidden(), true); + } finally { + releaseReports(); + holdReports = null; + } + await awaitReady(page); + }, +); diff --git a/tests/browser/problem-picker.test.js b/tests/browser/problem-picker.test.js index 5436d935c..fc0b2f3e4 100644 --- a/tests/browser/problem-picker.test.js +++ b/tests/browser/problem-picker.test.js @@ -9,6 +9,13 @@ import { sequelSuggestion, storeSharedFocus, suggestDifficulty, + clearSelectedPracticeFocus, + practiceFocusFrom, + practiceFocusOwner, + practiceFocusAccountOwner, + readSelectedPracticeFocus, + readSelectedPracticeFocusSelection, + storeSelectedPracticeFocus, } from "../../web/problem-picker.js"; import { ACTIVE_CONTRACT } from "../../web/lib.js"; @@ -690,6 +697,175 @@ test("a shared focus is handed over once, in session storage, and unsharing clea ); }); +test("selected practice focus stays stored until explicitly cleared", () => { + const store = new Map(); + const storage = { + getItem: (key) => (store.has(key) ? store.get(key) : null), + setItem: (key, value) => store.set(key, String(value)), + removeItem: (key) => store.delete(key), + }; + const focus = { + phase: "Test", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }; + storeSelectedPracticeFocus(storage, "account:A", focus); + assert.deepEqual(readSelectedPracticeFocus(storage, "account:A"), focus); + assert.deepEqual(readSelectedPracticeFocus(storage, "account:A"), focus); + clearSelectedPracticeFocus(storage); + assert.equal(readSelectedPracticeFocus(storage, "account:A"), null); +}); + +test("selected practice focus keeps its source report id", () => { + const store = new Map(); + const storage = { + getItem: (key) => (store.has(key) ? store.get(key) : null), + setItem: (key, value) => store.set(key, String(value)), + removeItem: (key) => store.delete(key), + }; + const focus = { + phase: "Test", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }; + storeSelectedPracticeFocus( + storage, + "account:A", + focus, + "coding", + "focus-source", + ); + assert.deepEqual(readSelectedPracticeFocusSelection(storage, "account:A"), { + focus, + sourceReportId: "focus-source", + }); + assert.deepEqual(readSelectedPracticeFocus(storage, "account:A"), focus); +}); + +test("practice focus account owners use one canonical key format", () => { + assert.equal(practiceFocusAccountOwner("A"), "account:A"); + assert.equal( + practiceFocusAccountOwner("opaque-owner"), + "account:opaque-owner", + ); + assert.equal(practiceFocusAccountOwner(""), null); + assert.equal(practiceFocusAccountOwner(null), null); + assert.equal(practiceFocusAccountOwner(null, true), "device"); +}); + +test("selected practice focus reports whether storage succeeded", () => { + const item = { + phase: "Test", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }; + const storage = { + setItem() {}, + removeItem() {}, + }; + assert.equal( + storeSelectedPracticeFocus( + storage, + "account:A", + item, + "coding", + "report-1", + ), + true, + ); + const blockedStorage = { + setItem() { + throw new Error("blocked"); + }, + removeItem() {}, + }; + assert.equal( + storeSelectedPracticeFocus( + blockedStorage, + "account:A", + item, + "coding", + "report-1", + ), + false, + ); + assert.equal( + storeSelectedPracticeFocus(storage, null, item, "coding", "report-1"), + false, + ); +}); + +test("clearing selected practice focus removes it from storage", () => { + const store = new Map(); + const storage = { + getItem: (key) => (store.has(key) ? store.get(key) : null), + setItem: (key, value) => store.set(key, String(value)), + removeItem: (key) => store.delete(key), + }; + storeSelectedPracticeFocus(storage, "account:A", { + phase: "Test", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }); + clearSelectedPracticeFocus(storage); + assert.equal(readSelectedPracticeFocus(storage, "account:A"), null); +}); + +test("invalid selected practice focus is discarded", () => { + const store = new Map(); + const storage = { + getItem: (key) => (store.has(key) ? store.get(key) : null), + setItem: (key, value) => store.set(key, String(value)), + removeItem: (key) => store.delete(key), + }; + const owner = "account:candidate"; + storage.setItem( + "codetrial.selectedPracticeFocus", + JSON.stringify({ + owner, + focus: { + phase: "Test", + weakness: "", + drill: "Build a test table", + successCriterion: "Predict each output", + }, + }), + ); + assert.equal(readSelectedPracticeFocus(storage, owner), null); + assert.equal(storage.getItem("codetrial.selectedPracticeFocus"), null); +}); + +test("selected practice focus rejects unknown phases", () => { + assert.equal( + practiceFocusFrom({ + phase: "Something else", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }), + null, + ); +}); + +test("selected practice focus rebuilds only bounded fields", () => { + const focus = practiceFocusFrom({ + phase: "Test", + weakness: "x".repeat(450), + drill: "Build a test table", + successCriterion: "Predict each output", + durationMin: 15, + extra: "not carried", + }); + + assert.equal(Array.from(focus.weakness).length, 400); + assert.equal("durationMin" in focus, false); + assert.equal("extra" in focus, false); +}); + test("the interview reads the focus from session storage, never from its address", async () => { const { readFile } = await import("node:fs/promises"); const source = await readFile( @@ -791,3 +967,60 @@ test("a sequel the lobby has no card for is not offered", () => { const cards = [{ id: "pair", difficulty: "Easy", sequel: "retired" }]; assert.equal(sequelSuggestion(cards, [attempt("pair", 2, "HIRE")]), null); }); + +for (const session of [ + { signedIn: true, user: { id: "B", login: "same" } }, + { signedIn: false }, +]) { + test(`selected focus is cleared when owner changes to ${practiceFocusOwner(session)}`, () => { + const values = new Map(); + const storage = { + getItem: (key) => values.get(key) ?? null, + setItem: (key, value) => values.set(key, value), + removeItem: (key) => values.delete(key), + }; + const owner = practiceFocusOwner({ + signedIn: true, + user: { id: "A", login: "same" }, + }); + storeSelectedPracticeFocus(storage, owner, { + phase: "Test", + weakness: "Boundaries", + drill: "Test edges", + successCriterion: "All predicted", + }); + assert.equal(readSelectedPracticeFocus(storage, null), null); + assert.equal(values.size, 1); + assert.equal( + readSelectedPracticeFocus(storage, practiceFocusOwner(session)), + null, + ); + assert.equal(values.size, 0); + }); +} + +test("selected focus tolerates blocked storage", () => { + const storage = { + getItem() { + throw new Error("blocked"); + }, + setItem() { + throw new Error("blocked"); + }, + removeItem() { + throw new Error("blocked"); + }, + }; + storeSelectedPracticeFocus(storage, "account:A", { + phase: "Test", + weakness: "Boundaries", + drill: "Test edges", + successCriterion: "All predicted", + }); + assert.equal(readSelectedPracticeFocus(storage, "account:A"), null); + clearSelectedPracticeFocus(storage); + assert.equal( + practiceFocusOwner({ signedIn: true, user: { login: "same" } }), + null, + ); +}); diff --git a/tests/browser/render.test.js b/tests/browser/render.test.js index 05efc9555..cbe4893da 100644 --- a/tests/browser/render.test.js +++ b/tests/browser/render.test.js @@ -1024,8 +1024,14 @@ test("practice-next drills render accessibly in HTML and Markdown", () => { const markdown = reportMarkdown(session); assert.match(html, /

Practice next<\/h3>/); assert.match(html, /Test · 10 min · high impact/); + assert.match(html, />Practice this<\/button>/); + assert.match( + html, + /aria-label="Practice this: [^"]+">Practice this<\/button>/, + ); assert.match(markdown, /## Practice next/); assert.match(markdown, /Success: Cover four case classes/); + assert.doesNotMatch(markdown, /Practice this/); }); test("the report shows the debrief collapsed", () => { diff --git a/tests/browser/replay-practice-focus.test.js b/tests/browser/replay-practice-focus.test.js new file mode 100644 index 000000000..4abb9f21f --- /dev/null +++ b/tests/browser/replay-practice-focus.test.js @@ -0,0 +1,352 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + practiceFocusAccountOwner, + practiceFocusFrom, +} from "../../web/problem-picker.js"; +import { functionBody, read } from "./source.js"; + +test("replay Practice this carries the exact selected improvement", async () => { + const source = read("web/replay.js"); + const body = functionBody(source, "loadReport"); + + const first = { + phase: "Coding", + weakness: "Implement the traversal", + drill: "Trace one example", + successCriterion: "Identify every connected region", + durationMin: 10, + }; + + const second = { + phase: "Test", + weakness: "Test boundary cases", + drill: "Build a boundary test table", + successCriterion: "Predict every boundary-case result", + durationMin: 15, + }; + + const report = { + improvementPlan: [first, second], + }; + + const buttons = [0, 1].map((index) => { + let click; + return { + dataset: { practiceFocus: String(index) }, + addEventListener(type, handler) { + assert.equal(type, "click"); + click = handler; + }, + click() { + return click(); + }, + }; + }); + + const nodes = { + code: { hidden: true }, + board: { + toDataURL() { + throw new Error("board should not be read"); + }, + }, + report: { + textContent: "", + innerHTML: "", + querySelectorAll(selector) { + assert.equal(selector, "[data-practice-focus]"); + return buttons; + }, + }, + }; + + const stored = []; + + const fakeWindow = { + location: { href: "" }, + }; + + const loadReport = new Function( + "selected", + "drawn", + "nodes", + "latest", + "fetch", + "sanitizeReport", + "reportMarkup", + "unclearLinesOf", + "storageArea", + "storeSelectedPracticeFocus", + "practiceFocusAccountOwner", + "window", + "mountFinalCodeCopy", + "loadOriginalProblem", + `${body}\n}\nreturn loadReport;`, + )( + "recording-1", + false, + nodes, + { language: "javascript", code: "return 1;" }, + async () => ({ + ok: true, + async json() { + return { + accountId: "A", + reports: [ + { + payload: { + id: "report-1", + interviewId: "interview-1", + problemTitle: "Example", + problemId: "example", + report, + }, + }, + ], + }; + }, + }), + (value) => value, + () => "
", + () => [], + (name) => { + assert.equal(name, "sessionStorage"); + return null; + }, + (storage, owner, focus, interviewMode, sourceReportId) => { + assert.equal(owner, "account:A"); + assert.equal(storage, null); + assert.equal(sourceReportId, "report-1"); + stored.push(practiceFocusFrom(focus, interviewMode)); + return true; + }, + practiceFocusAccountOwner, + fakeWindow, + () => {}, + () => Promise.resolve(null), + ); + + await loadReport("interview-1", "recording-1"); + + await buttons[1].click(); + + assert.deepEqual(stored, [ + { + phase: second.phase, + weakness: second.weakness, + drill: second.drill, + successCriterion: second.successCriterion, + }, + ]); + + assert.equal("durationMin" in stored[0], false); + assert.equal(fakeWindow.location.href, "/"); +}); + +test("replay binds Practice this to the report account", async () => { + const source = read("web/replay.js"); + const body = functionBody(source, "loadReport"); + + const item = { + phase: "Test", + weakness: "Test boundary cases", + drill: "Build a boundary test table", + successCriterion: "Predict every boundary-case result", + }; + + let click; + const button = { + dataset: { practiceFocus: "0" }, + addEventListener(type, handler) { + assert.equal(type, "click"); + click = handler; + }, + }; + + const nodes = { + code: { hidden: true }, + board: { + toDataURL() { + throw new Error("board should not be read"); + }, + }, + report: { + textContent: "", + innerHTML: "", + querySelectorAll() { + return [button]; + }, + }, + }; + + const fakeWindow = { + location: { href: "" }, + }; + + let storedOwner = null; + + const loadReport = new Function( + "selected", + "drawn", + "nodes", + "latest", + "fetch", + "sanitizeReport", + "reportMarkup", + "unclearLinesOf", + "storageArea", + "storeSelectedPracticeFocus", + "practiceFocusAccountOwner", + "window", + "mountFinalCodeCopy", + "loadOriginalProblem", + `${body}\n}\nreturn loadReport;`, + )( + "recording-1", + false, + nodes, + { language: "javascript", code: "return 1;" }, + async () => ({ + ok: true, + async json() { + return { + accountId: "B", + reports: [ + { + payload: { + id: "report-2", + interviewId: "interview-1", + problemTitle: "Example", + problemId: "example", + report: { improvementPlan: [item] }, + }, + }, + ], + }; + }, + }), + (value) => value, + () => "
", + () => [], + () => null, + (storage, owner, _focus, _interviewMode, sourceReportId) => { + assert.equal(storage, null); + assert.equal(sourceReportId, "report-2"); + storedOwner = owner; + return true; + }, + practiceFocusAccountOwner, + fakeWindow, + () => {}, + () => Promise.resolve(), + ); + + await loadReport("interview-1", "recording-1"); + await click(); + + assert.equal(storedOwner, "account:B"); + assert.equal(fakeWindow.location.href, "/"); +}); + +test("replay stays on the report when Practice this cannot be stored", async () => { + const source = read("web/replay.js"); + const body = functionBody(source, "loadReport"); + const item = { + phase: "Test", + weakness: "Test boundary cases", + drill: "Build a boundary test table", + successCriterion: "Predict every boundary-case result", + }; + const status = { + hidden: true, + textContent: "", + }; + let click; + const button = { + dataset: { practiceFocus: "0" }, + parentElement: { + querySelector(selector) { + assert.equal(selector, "[data-practice-focus-status]"); + return status; + }, + }, + addEventListener(type, handler) { + assert.equal(type, "click"); + click = handler; + }, + }; + const nodes = { + code: { hidden: true }, + board: { + toDataURL() { + throw new Error("board should not be read"); + }, + }, + report: { + textContent: "", + innerHTML: "", + querySelectorAll() { + return [button]; + }, + }, + }; + const fakeWindow = { + location: { href: "" }, + }; + const loadReport = new Function( + "selected", + "drawn", + "nodes", + "latest", + "fetch", + "sanitizeReport", + "reportMarkup", + "unclearLinesOf", + "storageArea", + "storeSelectedPracticeFocus", + "practiceFocusAccountOwner", + "window", + "mountFinalCodeCopy", + "loadOriginalProblem", + `${body}\n}\nreturn loadReport;`, + )( + "recording-1", + false, + nodes, + { language: "javascript", code: "return 1;" }, + async () => ({ + ok: true, + async json() { + return { + accountId: "A", + reports: [ + { + payload: { + id: "report-1", + interviewId: "interview-1", + problemTitle: "Example", + problemId: "example", + report: { improvementPlan: [item] }, + }, + }, + ], + }; + }, + }), + (value) => value, + () => "
", + () => [], + () => null, + () => false, + practiceFocusAccountOwner, + fakeWindow, + () => {}, + () => Promise.resolve(), + ); + await loadReport("interview-1", "recording-1"); + await click(); + assert.equal(fakeWindow.location.href, ""); + assert.equal(status.hidden, false); + assert.equal(status.textContent, "Could not save this practice focus."); +}); diff --git a/tests/browser/report-recovery.test.js b/tests/browser/report-recovery.test.js index b1d72fff6..349be8d94 100644 --- a/tests/browser/report-recovery.test.js +++ b/tests/browser/report-recovery.test.js @@ -309,6 +309,7 @@ function pageHarness(phase = "live") { saves.push(report); }, renderReport: () => events.push("render"), + setPracticeFocusButtonsDisabled: () => {}, renderReportSaveStatus: () => {}, interviewLoop: "coding_only", roomParticipants: () => [], diff --git a/tests/browser/strict-selection.test.js b/tests/browser/strict-selection.test.js index fa21716df..1cce5c069 100644 --- a/tests/browser/strict-selection.test.js +++ b/tests/browser/strict-selection.test.js @@ -74,7 +74,11 @@ async function interview( query, handler, context = browser, - session = { signedIn: true, loginRequired: true }, + session = { + signedIn: true, + loginRequired: true, + user: { id: "A", login: "candidate" }, + }, ) { return openInterview({ browser, base }, t, query, handler, context, session); } @@ -623,19 +627,54 @@ test("a join finishing after a report's receipt went out does not take the room await context.route(`${base}/`, (route) => route.fulfill({ contentType: "text/html", body: "

Lobby

" }), ); + const mapped = Promise.withResolvers(); + t.after(() => mapped.resolve()); + await context.route("**/problem-pages.json", async (route) => { + await mapped.promise; + await route.continue(); + }); const page = await interview( t, "problem=chargeback-pair-match", (route) => route.fulfill({ - json: grant("chargeback-pair-match", "receipt-before-join"), + json: { + ...grant("chargeback-pair-match", "receipt-before-join"), + accountId: "B", + guest: false, + }, }), context, ); await holdTrackPublishes(page); await preflight(page, { pause: true }); await page.waitForFunction(() => window.roomJoins.length === 1); - await deliverReport(page, { summary: "Delivered before the join finished." }); + await deliverReport(page, { + codingScore: 60, + communicationScore: 60, + decision: "NO_HIRE", + summary: "Delivered before the join finished.", + codingFeedback: { + strengths: [], + improvements: ["Test boundaries"], + }, + communicationFeedback: { + strengths: [], + improvements: [], + }, + improvementPlan: [ + { + phase: "Test", + weakness: "Test boundaries", + impact: "high", + frequency: 1, + drill: "Build a test table", + durationMin: 10, + successCriterion: "Predict each output", + selfReview: ["Name a boundary"], + }, + ], + }); await page.locator("#report-modal").waitFor({ state: "visible" }); // The receipt is hashed before it is published. Wait for the publish, so // the clock below moves the receipt on rather than running out its bound. @@ -659,8 +698,33 @@ test("a join finishing after a report's receipt went out does not take the room await tracksReleased(page); save(); await page.locator("#done:not([disabled])").waitFor(); - await page.locator("#done").click(); + const practice = page.getByRole("button", { name: "Practice this" }); + const download = page.waitForEvent("download"); + await page.locator("#download-report").click(); + assert.equal(await practice.isDisabled(), true); + mapped.resolve(); + await download; + await page.waitForFunction( + () => !document.querySelector("[data-practice-focus]").disabled, + ); + assert.equal(await practice.isEnabled(), true); + await practice.click(); await page.waitForURL(`${base}/`); + const selectedFocus = await page.evaluate(() => + JSON.parse(sessionStorage.getItem("codetrial.selectedPracticeFocus")), + ); + const { sourceReportId, ...selection } = selectedFocus; + assert.deepEqual(selection, { + owner: "account:B", + focus: { + phase: "Test", + weakness: "Test boundaries", + drill: "Build a test table", + successCriterion: "Predict each output", + }, + }); + assert.equal(typeof sourceReportId, "string"); + assert.ok(sourceReportId.length > 0); assert.equal( await page.evaluate(() => sessionStorage.getItem("leftAfterReport")), null, @@ -1737,11 +1801,19 @@ test("the lobby defaults to Practice and starts Strict without a selected proble t.after(() => page.close()); await page.route("**/api/session", (route) => route.fulfill({ - json: { signedIn: true, user: { login: "candidate" } }, + json: { + signedIn: true, + user: { id: "A", login: "candidate" }, + }, }), ); await page.route("**/api/reports", (route) => - route.fulfill({ json: { reports: [] } }), + route.fulfill({ + json: { + accountId: "A", + reports: [], + }, + }), ); await page.route("**/interview?**", (route) => route.fulfill({ @@ -2204,11 +2276,19 @@ test("Practice still filters by difficulty and opens a specifically chosen probl t.after(() => page.close()); await page.route("**/api/session", (route) => route.fulfill({ - json: { signedIn: true, user: { login: "candidate" } }, + json: { + signedIn: true, + user: { id: "A", login: "candidate" }, + }, }), ); await page.route("**/api/reports", (route) => - route.fulfill({ json: { reports: [] } }), + route.fulfill({ + json: { + accountId: "A", + reports: [], + }, + }), ); await page.route("**/interview?**", (route) => route.fulfill({ diff --git a/tests/web.rs b/tests/web.rs index 581031680..95edf1875 100644 --- a/tests/web.rs +++ b/tests/web.rs @@ -267,6 +267,18 @@ async fn record_login(client: &reqwest::Client, base: &str, handle: &str) -> Str .to_string() } +fn expected_account_owner(path: &Path, login: &str) -> String { + let connection = rusqlite::Connection::open(path).unwrap(); + let identity: i64 = connection + .query_row( + "SELECT github_id FROM users WHERE login = ?1", + [login], + |row| row.get(0), + ) + .unwrap(); + codetrial::token::sign_hs256("session-secret", &format!("account-owner:{identity}")) +} + /// A LiveKit token names its project in `iss` and proves it in the signature. /// Checking only `iss` would miss a token minted with one provider's key and /// another's secret, which the SFU rejects and the response body does not show. @@ -1415,6 +1427,9 @@ mod peer_auth; #[path = "web/accounts.rs"] mod accounts; +#[path = "web/account_focus.rs"] +mod account_focus; + #[path = "web/routes.rs"] mod routes; diff --git a/tests/web/account_focus.rs b/tests/web/account_focus.rs new file mode 100644 index 000000000..bd8239526 --- /dev/null +++ b/tests/web/account_focus.rs @@ -0,0 +1,252 @@ +use super::*; + +/// A typed handle is not proof of anything, so it must not be the key an +/// account is found by. If it were, typing a name someone else used would hand +/// over their interview history, and nothing stops anyone typing any name. +#[tokio::test] +async fn a_claimed_handle_does_not_reach_an_earlier_candidates_reports() { + let (base, server, path, client) = account_server("claimed-handle").await; + + let first = record_login(&client, &base, "octocat").await; + let saved = client + .post(format!("{base}/api/reports")) + .header("cookie", &first) + .json(&json!({"id":"report-one","problemId":"two-sum","score":8})) + .send() + .await + .unwrap(); + assert_eq!(saved.status(), 200); + + let second = record_login(&client, &base, "octocat").await; + assert_ne!(first, second, "each recorded login is its own account"); + let mut owners = Vec::new(); + for cookie in [&first, &first, &second] { + let session = client + .get(format!("{base}/api/session")) + .header("cookie", cookie) + .send() + .await + .unwrap() + .json::() + .await + .unwrap(); + assert_eq!(session["user"]["login"], "octocat"); + owners.push(session["user"]["id"].as_str().unwrap().to_owned()); + } + assert_eq!(owners[0], owners[1], "an account has a stable owner id"); + assert_ne!(owners[0], owners[2], "typed handles do not identify owners"); + + let reports = client + .get(format!("{base}/api/reports")) + .header("cookie", &second) + .send() + .await + .unwrap() + .json::() + .await + .unwrap(); + assert_eq!( + reports["reports"].as_array().unwrap().len(), + 0, + "claiming a handle must not inherit the reports filed under it" + ); + + let mine = client + .get(format!("{base}/api/reports")) + .header("cookie", &first) + .send() + .await + .unwrap() + .json::() + .await + .unwrap(); + assert_eq!(mine["reports"][0]["id"], "report-one"); + + server.shutdown().await; + remove_database(path).await; +} + +#[tokio::test] +async fn token_api_matches_frontend_contract_over_http() { + let (config, cookie, db_path) = signed_in_web_config("contract"); + let (base, server) = spawn_web_server(config).await; + let response = http_client() + .post(format!("{base}/api/token")) + .header("cookie", &cookie) + .json(&json!({"problemId":"merge-intervals","durationMin":120})) + .send() + .await + .unwrap(); + + assert_eq!(response.status(), 200); + let body: Value = response.json().await.unwrap(); + let mut keys = body.as_object().unwrap().keys().collect::>(); + keys.sort(); + assert_eq!( + keys, + [ + "accountId", + "durationMin", + "guest", + "roomName", + "serverUrl", + "token" + ] + ); + let session: Value = http_client() + .get(format!("{base}/api/session")) + .header("cookie", &cookie) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + assert!(body["accountId"].is_string()); + assert_eq!(body["accountId"], session["user"]["id"]); + let reports: Value = http_client() + .get(format!("{base}/api/reports")) + .header("cookie", &cookie) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + assert_eq!( + reports["accountId"], session["user"]["id"], + "session and reports must expose the same account owner" + ); + assert_eq!( + body["accountId"], reports["accountId"], + "token and reports must expose the same account owner" + ); + assert_eq!(body["guest"], false); + assert_eq!(body["serverUrl"], "wss://example.livekit.cloud"); + assert!(body["roomName"].as_str().unwrap().starts_with("interview-")); + + let claims = claims(body["token"].as_str().unwrap()); + assert_eq!(claims["name"], "Candidate"); + assert!(claims["sub"].as_str().unwrap().starts_with("candidate-")); + assert_eq!( + claims["exp"].as_u64().unwrap() - claims["nbf"].as_u64().unwrap(), + TOKEN_TTL_SECONDS + ); + assert_eq!(claims["video"]["room"], body["roomName"]); + assert_eq!(claims["video"]["roomJoin"], true); + assert_eq!(claims["video"]["canPublish"], true); + assert_eq!(claims["video"]["canSubscribe"], true); + assert_eq!(claims["video"]["canPublishData"], true); + let metadata: Value = serde_json::from_str(claims["metadata"].as_str().unwrap()).unwrap(); + assert_eq!(metadata["problemId"], "merge-intervals"); + assert_eq!(metadata["durationMin"], 90); + assert_eq!(metadata["candidateIdentity"], claims["sub"]); + + // The page cannot read the metadata: it is inside a signed token it never + // opens. The response body carries the length the agent will enforce, which + // for a whole-minute request is the metadata value itself. + assert_eq!(body["durationMin"], metadata["durationMin"]); + + server.shutdown().await; + remove_database(db_path).await; +} + +#[tokio::test] +async fn guest_token_admission_has_no_account_owner() { + let (mut config, _, db_path) = signed_in_web_config("guest-focus-owner"); + config.guest_mode_enabled = true; + let (base, server) = spawn_web_server_with_guest_dispatcher(config).await; + let response = http_client() + .post(format!("{base}/api/token")) + .json(&json!({"problemId": "two-sum", "durationMin": 45})) + .send() + .await + .unwrap(); + + assert_eq!(response.status(), 200); + let body: Value = response.json().await.unwrap(); + assert_eq!(body["guest"], true); + assert!(body.get("accountId").is_none()); + + server.shutdown().await; + remove_database(db_path).await; +} + +#[tokio::test] +async fn recycling_a_throwaway_user_row_does_not_recycle_the_browser_owner() { + let (base, server, path, client) = account_server("recycled-focus-owner").await; + let first_cookie = record_login(&client, &base, "first-owner").await; + let first_session = client + .get(format!("{base}/api/session")) + .header("cookie", &first_cookie) + .send() + .await + .unwrap() + .json::() + .await + .unwrap(); + let first_owner = first_session["user"]["id"].as_str().unwrap().to_owned(); + let connection = rusqlite::Connection::open(&path).unwrap(); + let (first_user_id, first_account_identity): (i64, i64) = connection + .query_row( + "SELECT id, github_id FROM users WHERE login = 'first-owner'", + [], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .unwrap(); + connection + .execute( + "UPDATE sessions SET expires_at = 0 WHERE user_id = ?1", + [first_user_id], + ) + .unwrap(); + drop(connection); + let accounts = codetrial::accounts::Accounts::open(codetrial::accounts::GitHubLoginConfig { + oauth: None, + session_secret: "session-secret".to_string(), + db_path: path.clone(), + oauth_base_url: "https://github.com".to_string(), + api_base_url: "https://api.github.com".to_string(), + }) + .unwrap(); + assert_eq!( + codetrial::accounts::sweep_expired_sessions(&accounts, 1).unwrap(), + (1, 1), + ); + drop(accounts); + let second_cookie = record_login(&client, &base, "second-owner").await; + let second_session = client + .get(format!("{base}/api/session")) + .header("cookie", &second_cookie) + .send() + .await + .unwrap() + .json::() + .await + .unwrap(); + let second_owner = second_session["user"]["id"].as_str().unwrap().to_owned(); + let connection = rusqlite::Connection::open(&path).unwrap(); + let (second_user_id, second_account_identity): (i64, i64) = connection + .query_row( + "SELECT id, github_id FROM users WHERE login = 'second-owner'", + [], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .unwrap(); + + assert_eq!( + first_user_id, second_user_id, + "SQLite should reproduce the recycled users.id case", + ); + assert_ne!( + first_account_identity, second_account_identity, + "throwaway accounts need distinct stable identities", + ); + assert_ne!( + first_owner, second_owner, + "recycling users.id must not recycle the browser owner", + ); + drop(connection); + server.shutdown().await; + remove_database(path).await; +} diff --git a/tests/web/accounts.rs b/tests/web/accounts.rs index e8f6d2347..85e3ace6b 100644 --- a/tests/web/accounts.rs +++ b/tests/web/accounts.rs @@ -144,6 +144,8 @@ async fn account_routes_record_interviewee_github_login() { .unwrap(); assert_eq!(signed_in["signedIn"], true); assert_eq!(signed_in["user"]["login"], "octo-cat"); + let owner = signed_in["user"]["id"].as_str().unwrap(); + assert_eq!(owner, expected_account_owner(&path, "octo-cat")); assert_eq!(signed_in["githubSignedIn"], false); let reports = client @@ -246,56 +248,6 @@ async fn session_reports_candidate_video_signed_out_and_signed_in() { } } -/// A typed handle is not proof of anything, so it must not be the key an -/// account is found by. If it were, typing a name someone else used would hand -/// over their interview history, and nothing stops anyone typing any name. -#[tokio::test] -async fn a_claimed_handle_does_not_reach_an_earlier_candidates_reports() { - let (base, server, path, client) = account_server("claimed-handle").await; - - let first = record_login(&client, &base, "octocat").await; - let saved = client - .post(format!("{base}/api/reports")) - .header("cookie", &first) - .json(&json!({"id":"report-one","problemId":"two-sum","score":8})) - .send() - .await - .unwrap(); - assert_eq!(saved.status(), 200); - - let second = record_login(&client, &base, "octocat").await; - assert_ne!(first, second, "each recorded login is its own account"); - - let reports = client - .get(format!("{base}/api/reports")) - .header("cookie", &second) - .send() - .await - .unwrap() - .json::() - .await - .unwrap(); - assert_eq!( - reports["reports"].as_array().unwrap().len(), - 0, - "claiming a handle must not inherit the reports filed under it" - ); - - let mine = client - .get(format!("{base}/api/reports")) - .header("cookie", &first) - .send() - .await - .unwrap() - .json::() - .await - .unwrap(); - assert_eq!(mine["reports"][0]["id"], "report-one"); - - server.shutdown().await; - remove_database(path).await; -} - /// The other half of the fail-closed rule, and the one an operator actually /// reaches: the file opens fine, but a rolled-back binary finds a schema /// version from the future and refuses to migrate it. That arm has to answer diff --git a/tests/web/token.rs b/tests/web/token.rs index 895f040e7..962aa13e2 100644 --- a/tests/web/token.rs +++ b/tests/web/token.rs @@ -719,56 +719,6 @@ async fn token_endpoint_rate_limits_a_noisy_client() { remove_database(db_path).await; } -#[tokio::test] -async fn token_api_matches_frontend_contract_over_http() { - let (config, cookie, db_path) = signed_in_web_config("contract"); - let (base, server) = spawn_web_server(config).await; - let response = http_client() - .post(format!("{base}/api/token")) - .header("cookie", &cookie) - .json(&json!({"problemId":"merge-intervals","durationMin":120})) - .send() - .await - .unwrap(); - - assert_eq!(response.status(), 200); - let body: Value = response.json().await.unwrap(); - let mut keys = body.as_object().unwrap().keys().collect::>(); - keys.sort(); - assert_eq!( - keys, - ["durationMin", "guest", "roomName", "serverUrl", "token"] - ); - assert_eq!(body["guest"], false); - assert_eq!(body["serverUrl"], "wss://example.livekit.cloud"); - assert!(body["roomName"].as_str().unwrap().starts_with("interview-")); - - let claims = claims(body["token"].as_str().unwrap()); - assert_eq!(claims["name"], "Candidate"); - assert!(claims["sub"].as_str().unwrap().starts_with("candidate-")); - assert_eq!( - claims["exp"].as_u64().unwrap() - claims["nbf"].as_u64().unwrap(), - TOKEN_TTL_SECONDS - ); - assert_eq!(claims["video"]["room"], body["roomName"]); - assert_eq!(claims["video"]["roomJoin"], true); - assert_eq!(claims["video"]["canPublish"], true); - assert_eq!(claims["video"]["canSubscribe"], true); - assert_eq!(claims["video"]["canPublishData"], true); - let metadata: Value = serde_json::from_str(claims["metadata"].as_str().unwrap()).unwrap(); - assert_eq!(metadata["problemId"], "merge-intervals"); - assert_eq!(metadata["durationMin"], 90); - assert_eq!(metadata["candidateIdentity"], claims["sub"]); - - // The page cannot read the metadata: it is inside a signed token it never - // opens. The response body carries the length the agent will enforce, which - // for a whole-minute request is the metadata value itself. - assert_eq!(body["durationMin"], metadata["durationMin"]); - - server.shutdown().await; - remove_database(db_path).await; -} - #[tokio::test] async fn observer_is_not_the_candidate() { let (config, cookie, db_path) = signed_in_web_config("observer"); diff --git a/web/app.js b/web/app.js index f7ce345a2..aac394d7f 100644 --- a/web/app.js +++ b/web/app.js @@ -20,6 +20,11 @@ import { sequelSuggestion, storeSharedFocus, suggestDifficulty, + clearSelectedPracticeFocus, + practiceFocusFrom, + readSelectedPracticeFocusSelection, + storeSelectedPracticeFocus, + practiceFocusOwner, } from "./problem-picker.js"; import { normalizeProgressEntries, @@ -91,6 +96,10 @@ let mode = "coding"; let retryLanguage = null; let reports = []; let historyCategories = null; +const selectedPracticeFocusStorage = storageArea("sessionStorage"); +let selectedPracticeFocus = null; +let selectedPracticeFocusOwner = null; +let selectedPracticeFocusSourceReportId = null; /// The practice focus the share box currently refers to. let sharedFocus = null; let manualProblem = false; @@ -621,6 +630,7 @@ const selectedProgressPhases = new Map(); /// answers, and never assumed false, because a browser that cannot ask is not a /// browser that knows there is nothing on the server. let accountHistory = null; +let accountHistoryOwnerId = null; for (const filter of [ nodes.progressProblem, @@ -804,8 +814,8 @@ start.addEventListener("click", async () => { destination.searchParams.set("seniority", profile.seniority); if (profile.targetCompany) destination.searchParams.set("company", profile.targetCompany); - const focus = nodes.practiceFocusShareInput.checked - ? practiceFocus(reports) + let focus = nodes.practiceFocusShareInput.checked + ? (selectedPracticeFocus ?? practiceFocus(reports)) : null; let packet; try { @@ -837,12 +847,30 @@ start.addEventListener("click", async () => { // below, and spent a second /api/login doing it. signInFirst = false; } + if (focus) { + try { + const currentSession = await fetchJson("/api/session"); + const currentOwner = practiceFocusOwner(currentSession); + if (!currentOwner || currentOwner !== selectedPracticeFocusOwner) { + focus = null; + nodes.practiceFocusShareInput.checked = false; + } + } catch { + // If ownership cannot be verified, starting without sharing is safer + // than handing another account's practice focus to the interviewer. + focus = null; + nodes.practiceFocusShareInput.checked = false; + } + } start.textContent = "Starting..."; if (!nodes.groundingConsent.checked) packet = null; try { if (strict) resetAssignments(sessionStorage); storeGroundingPacket(sessionStorage, packet); - storeSharedFocus(sessionStorage, focus?.weakness ?? null); + storeSharedFocus( + sessionStorage, + nodes.practiceFocusShareInput.checked ? (focus?.weakness ?? null) : null, + ); } catch (error) { nodes.groundingError.textContent = error.message; starting = false; @@ -850,6 +878,7 @@ start.addEventListener("click", async () => { setStartGate(signInFirst); return; } + clearSelectedPracticeFocus(selectedPracticeFocusStorage); window.location.href = destination.toString(); }); @@ -937,6 +966,8 @@ window.addEventListener("pageshow", (event) => { // late enough to re-enable a button the candidate already pressed and hand // them a second navigation. if (!event.persisted) return; + nodes.practiceFocusShareInput.checked = false; + sharedFocus = null; nodes.problemSearch.value = ""; nodes.problemSearchModes[0].checked = true; nodes.problemSearch.placeholder = "Search scenario titles"; @@ -1039,6 +1070,10 @@ function setStartGate(blocked) { nodes.logout.addEventListener("click", async () => { nodes.logout.disabled = true; + clearSelectedPracticeFocus(selectedPracticeFocusStorage); + selectedPracticeFocus = null; + selectedPracticeFocusOwner = null; + selectedPracticeFocusSourceReportId = null; try { await fetch("/api/logout", { method: "POST" }); } finally { @@ -1055,17 +1090,29 @@ applyDifficulties(); // for good. refreshHistory(); -function refreshHistory() { +function refreshHistory(retryOwnerMismatch = true) { renderProblemAttempts([]); const generation = ++historyLoad; historyRefreshing = generation > 1; historyReady = false; + selectedPracticeFocus = null; + selectedPracticeFocusOwner = null; + selectedPracticeFocusSourceReportId = null; + sharedFocus = null; + nodes.practiceFocusShareInput.checked = false; + nodes.practiceFocus.hidden = true; + nodes.practiceFocusShare.hidden = true; + for (const button of nodes.attemptHistory.querySelectorAll( + "[data-practice-focus]", + )) { + button.disabled = true; + } nodes.reportDeleteStatus.textContent = ""; syncLobbyControls(); // A cached page may retain its selection while its account is unknown. start.disabled = true; // A restored page can start a second load before the first one finishes. - return loadAccount(generation).finally(() => { + return loadAccount(generation, retryOwnerMismatch).finally(() => { if (generation === historyLoad) { historyRefreshing = false; settle(); @@ -1106,12 +1153,20 @@ function settle() { if (focusedCard?.button.hidden) focusPickerSummary(); } -async function loadAccount(generation) { +async function loadAccount(generation, retryOwnerMismatch = true) { accountHistory = null; + accountHistoryOwnerId = null; let signedOutMessage = "Signed out"; try { const session = await fetchJson("/api/session"); if (generation !== historyLoad) return; + selectedPracticeFocusOwner = practiceFocusOwner(session); + const selected = readSelectedPracticeFocusSelection( + selectedPracticeFocusStorage, + selectedPracticeFocusOwner, + ); + selectedPracticeFocus = selected?.focus ?? null; + selectedPracticeFocusSourceReportId = selected?.sourceReportId ?? null; if ( typeof session?.signedIn !== "boolean" || (session.signedIn && typeof session.user?.login !== "string") @@ -1133,6 +1188,7 @@ async function loadAccount(generation) { : null; applyCostNote(); if (session.signedIn) { + accountHistoryOwnerId = session.user.id; loginResult = null; accountHistory = true; nodes.loginLink.textContent = "Use GitHub"; @@ -1141,7 +1197,11 @@ async function loadAccount(generation) { nodes.loginLink.hidden = true; nodes.logout.hidden = false; setStartGate(false); - await renderServerHistory(generation); + await renderServerHistory( + generation, + accountHistoryOwnerId, + retryOwnerMismatch, + ); return; } const unconfirmedLogin = loginResult !== null; @@ -1238,10 +1298,26 @@ async function recordGitHubLogin() { } } -async function renderServerHistory(generation) { +async function renderServerHistory( + generation, + expectedAccountId, + retryOwnerMismatch = true, +) { try { const data = await fetchJson("/api/reports"); if (generation !== historyLoad) return; + if ( + typeof data?.accountId !== "string" || + data.accountId !== expectedAccountId + ) { + if (generation !== historyLoad) return; + if (retryOwnerMismatch) { + void refreshHistory(false); + return; + } + showProgressError("Could not load saved account progress."); + return; + } // The picker needs the account row's timestamp for review scheduling and // the verdict as it was saved; the progress panel normalizes its own. // The server reads its rows back under page names, so no map is needed. @@ -1356,6 +1432,11 @@ async function deleteSavedReports() { "Saved reports and progress were deleted.", "good small", ); + clearSelectedPracticeFocus(selectedPracticeFocusStorage); + selectedPracticeFocus = null; + selectedPracticeFocusSourceReportId = null; + nodes.practiceFocusShareInput.checked = false; + sharedFocus = null; reports = []; showProgress([], "saved"); settle(); @@ -1403,6 +1484,17 @@ async function deleteSavedReport(attempt, when, button) { account: accountHistory === true, }); if (result !== "failed") { + if ( + result !== "missing" && + selectedPracticeFocusSourceReportId && + selectedPracticeFocusSourceReportId === attempt.id + ) { + clearSelectedPracticeFocus(selectedPracticeFocusStorage); + selectedPracticeFocus = null; + selectedPracticeFocusSourceReportId = null; + nodes.practiceFocusShareInput.checked = false; + sharedFocus = null; + } if (result === "account-deleted-local-failed") { message = "The report was deleted from your account, but its copy on this device could not be deleted."; @@ -1441,8 +1533,15 @@ async function reloadAfterDelete(local) { return; } const generation = ++historyLoad; - if (accountHistory && !local) await renderServerHistory(generation); - else await renderLocalHistory(generation); + if (accountHistory && !local) { + if (!accountHistoryOwnerId) { + await refreshHistory(); + return; + } + await renderServerHistory(generation, accountHistoryOwnerId); + } else { + await renderLocalHistory(generation); + } if (generation === historyLoad) settle(); } @@ -1627,13 +1726,15 @@ function recommend(note = "", mode = "recommend") { : `${note}Selected problem: ${title(choice.picked)}.`; } -/// Read off `reports` alone, so it is rendered wherever those change: the two -/// history paths below, and nowhere the selection moves. +/// Render the selected focus when one was carried explicitly; otherwise derive +/// the automatic focus from reports. function renderPracticeFocus() { - const focus = practiceFocus(reports); - nodes.practiceFocus.textContent = focus - ? `Carry forward${focus.occurrences === 1 ? "" : ` (${focus.occurrences} reports)`}: ${focus.weakness}. Drill: ${focus.drill}. Success: ${focus.successCriterion}.` - : ""; + const focus = selectedPracticeFocus ?? practiceFocus(reports); + nodes.practiceFocus.textContent = selectedPracticeFocus + ? `Selected practice focus · ${focus.phase}: ${focus.weakness}. Drill: ${focus.drill}. Success: ${focus.successCriterion}.` + : focus + ? `Carry forward${focus.occurrences === 1 ? "" : ` (${focus.occurrences} reports)`}: ${focus.weakness}. Drill: ${focus.drill}. Success: ${focus.successCriterion}.` + : ""; nodes.practiceFocus.hidden = focus === null; nodes.practiceFocusShare.hidden = focus === null; // Consent is to share this text. Reloaded history can change it, and a box @@ -2114,6 +2215,38 @@ function renderAttemptHistory(attempts) { unclearLines: attempt.unclearLines, }); mountFinalCodeCopy(report, attempt.code); + const reportGeneration = historyLoad; + for (const button of report.querySelectorAll("[data-practice-focus]")) { + button.addEventListener("click", () => { + if ( + !historyReady || + reportGeneration !== historyLoad || + !selectedPracticeFocusOwner + ) { + return; + } + const item = + attempt.report.improvementPlan?.[ + Number(button.dataset.practiceFocus) + ]; + if (!item) return; + const focus = practiceFocusFrom(item, attempt.interviewMode); + if (!focus) return; + selectedPracticeFocus = focus; + selectedPracticeFocusSourceReportId = attempt.id || null; + storeSelectedPracticeFocus( + selectedPracticeFocusStorage, + selectedPracticeFocusOwner, + item, + attempt.interviewMode, + selectedPracticeFocusSourceReportId, + ); + nodes.practiceFocusShareInput.checked = false; + sharedFocus = null; + renderPracticeFocus(); + nodes.practiceFocus.scrollIntoView({ block: "nearest" }); + }); + } report.querySelector(".report-actions")?.remove(); report.hidden = true; open.addEventListener("click", () => { diff --git a/web/interview.js b/web/interview.js index 50f41daf7..83c46c5fa 100644 --- a/web/interview.js +++ b/web/interview.js @@ -36,7 +36,12 @@ import { loadProblem, } from "./problem-data.js"; import { downloadMarkdown, reportFilename } from "./download.js"; -import { consumeSharedFocus } from "./problem-picker.js"; +import { + consumeSharedFocus, + practiceFocusFrom, + practiceFocusAccountOwner, + storeSelectedPracticeFocus, +} from "./problem-picker.js"; import { normalizeProgressEntries } from "./progress.js"; import { compareCategoryAttempt } from "./practice-insights.js"; import { @@ -218,6 +223,7 @@ import { consumeGroundingPacket } from "./document-grounding.js"; /// here because `codingClosed` reads it and `init` runs before the rest of the /// module body does, so a declaration beside its other readers would be in the /// temporal dead zone for the first paint. +let admittedPracticeFocusOwner = null; let frameworkRound = "coding"; let exampleBoard = null; let drawingTurn = null; @@ -524,6 +530,7 @@ const state = { categoryComparison: null, categoryComparisonReady: Promise.resolve(), downloadingReport: false, + leaving: false, reportSaveComplete: false, reportReceiving: false, reportSkipped: false, @@ -1201,7 +1208,7 @@ function bindEvents() { nodes.retryReport.addEventListener("click", () => reportRecovery.retry()); nodes.skipReport.addEventListener("click", () => void skipReport()); nodes.forceReport.addEventListener("click", showReport); - nodes.leaveRoom.addEventListener("click", leaveRoom); + nodes.leaveRoom.addEventListener("click", () => void leaveRoom()); nodes.candidateCaseAdd.addEventListener("click", () => { if (state.candidateCaseEdit) { void saveCandidateCaseEdit(); @@ -1414,6 +1421,16 @@ function bindEvents() { }, }); nodes.report.addEventListener("click", (event) => { + const practiceButton = event.target.closest("[data-practice-focus]"); + if (practiceButton) { + const index = Number(practiceButton.dataset.practiceFocus); + const item = state.report?.improvementPlan?.[index]; + if (item) { + const focus = practiceFocusFrom(item, mode); + if (focus) void leaveRoom(focus, practiceButton); + } + return; + } if (event.target.closest("#done")) leaveRoom(); if (event.target.closest("#download-report")) void downloadReport(); }); @@ -2021,6 +2038,10 @@ async function connect(preflight, presenting = false) { ); } const connection = await response.json(); + admittedPracticeFocusOwner = practiceFocusAccountOwner( + connection.accountId, + connection.guest === true, + ); // First, before anything below can throw: from here the server holds an // interviewer for this start, and only this token can let it go early. grantedStart = { roomName: connection.roomName, token: connection.token }; @@ -2774,6 +2795,7 @@ async function receiveReport(room, payload, flushed = Promise.resolve()) { // Done navigates away, which would drop a receipt still leaving. const [saved] = await Promise.all([saving, flushed]); state.reportReceiving = false; + setPracticeFocusButtonsDisabled(false); renderReportSaveStatus(saved); } catch (error) { reportRecovery.stop(); @@ -2787,6 +2809,7 @@ async function receiveReport(room, payload, flushed = Promise.resolve()) { // has had its chance to leave, even if saving the report failed. await flushed.catch(() => {}); state.reportReceiving = false; + setPracticeFocusButtonsDisabled(false); return; } // The interview is over whether or not the report drew, and only the @@ -4543,8 +4566,8 @@ async function skipReport() { await leaveRoom(); } -async function leaveRoom() { - if (state.downloadingReport) return; +async function leaveRoom(selectedFocus = null, practiceButton = null) { + if (state.downloadingReport || state.leaving) return; if (state.phase === "report_recovery") { if (state.reportSkipped) { reportRecovery.stop(); @@ -4561,6 +4584,27 @@ async function leaveRoom() { ) { return; } + state.leaving = true; + if (selectedFocus) { + const stored = storeSelectedPracticeFocus( + tabStorage, + admittedPracticeFocusOwner, + selectedFocus, + mode, + state.reportId, + ); + if (!stored) { + state.leaving = false; + const status = practiceButton?.parentElement?.querySelector( + "[data-practice-focus-status]", + ); + if (status) { + status.hidden = false; + status.textContent = "Could not save this practice focus."; + } + return; + } + } beforeUnloadGuard.dispose(); stopEndingClock(); void state.room?.disconnect?.(); @@ -4675,6 +4719,16 @@ function reportRenderFailed(error, { offlineSummary }) { nodes.skipReport.hidden = true; } +function setPracticeFocusButtonsDisabled(disabled) { + for (const button of nodes.report.querySelectorAll("[data-practice-focus]")) { + button.disabled = + disabled || + state.reportReceiving || + state.downloadingReport || + !state.reportSaveComplete; + } +} + function renderReport() { if (strict) updateRunAvailability(); // The interview is over. Both report paths, agent-sent and offline, land @@ -4707,6 +4761,7 @@ function renderReport() { unclearLines: unclearLines(), categoryComparison: state.categoryComparison, }); + setPracticeFocusButtonsDisabled(state.reportReceiving); mountFinalCodeCopy(nodes.report, currentCode()); void loadOriginalProblem(nodes.report, problem.page); localRecording.finish(nodes.report); @@ -4801,7 +4856,9 @@ function renderReportSaveStatus(result) { node.textContent = status.message; node.className = status.className; state.reportSaveComplete = true; - nodes.report.querySelector("#done").disabled = state.downloadingReport; + nodes.report.querySelector("#done").disabled = + state.reportReceiving || state.downloadingReport; + setPracticeFocusButtonsDisabled(false); } // Overlapping saves may finish out of order. Only the newest one may decide @@ -4881,6 +4938,7 @@ async function downloadReport() { state.downloadingReport = true; if (button) button.disabled = true; if (done) done.disabled = true; + setPracticeFocusButtonsDisabled(true); let timer; try { const [problemPages] = await Promise.all([ @@ -4903,7 +4961,9 @@ async function downloadReport() { if (button) button.disabled = false; // Saving and exporting can finish in either order. Both must settle // before Done can leave the report without interrupting either one. - if (done) done.disabled = !state.reportSaveComplete; + if (done) + done.disabled = state.reportReceiving || !state.reportSaveComplete; + setPracticeFocusButtonsDisabled(false); } } diff --git a/web/problem-picker.js b/web/problem-picker.js index 7228c3159..a6ad5ce79 100644 --- a/web/problem-picker.js +++ b/web/problem-picker.js @@ -1,3 +1,8 @@ +import { + frameworkChecklist, + frameworkPhases, + modeIsWhiteboard, +} from "./lib.js"; export const LEVELS = ["Easy", "Medium", "Hard"]; /// How many results in a row at one level move a candidate off it. One is a @@ -11,6 +16,25 @@ const STREAK = 2; /// retention model would promise accuracy the interview never measured. const REVIEW_INTERVAL_DAYS = [1, 3, 7, 14, 30]; const DAY_MS = 24 * 60 * 60 * 1000; +const PRACTICE_FOCUS_TEXT_MAX = 400; +const CODING_PHASES = frameworkChecklist("coding", [], "coding").steps; +const WHITEBOARD_PHASES = frameworkChecklist("coding", [], "whiteboard").steps; +const PRACTICE_FOCUS_PHASES = new Set([ + ...frameworkPhases, + ...WHITEBOARD_PHASES.map((step) => step.label), +]); +function practiceFocusText(value) { + return typeof value === "string" + ? Array.from(value).slice(0, PRACTICE_FOCUS_TEXT_MAX).join("").trim() + : ""; +} +function practiceFocusPhase(value, interviewMode) { + const phase = typeof value === "string" ? value : ""; + if (!PRACTICE_FOCUS_PHASES.has(phase)) return ""; + if (!modeIsWhiteboard(interviewMode)) return phase; + const index = CODING_PHASES.findIndex((step) => step.label === phase); + return index < 0 ? phase : WHITEBOARD_PHASES[index].label; +} /// Which level to practise next, read off what the candidate has already done. /// @@ -62,6 +86,7 @@ export function suggestDifficulty(problems, reports) { } const sharedFocusKey = "codetrial.sharedPracticeFocus"; +const selectedPracticeFocusKey = "codetrial.selectedPracticeFocus"; /// The focus a candidate chose to share, handed from the lobby to the interview /// in this tab's session storage rather than the address bar. A URL carrying @@ -87,6 +112,114 @@ export function consumeSharedFocus(storage) { } } +export function practiceFocusFrom(item, interviewMode = "coding") { + const phase = practiceFocusPhase(item?.phase, interviewMode); + const weakness = practiceFocusText(item?.weakness); + const drill = practiceFocusText(item?.drill); + const successCriterion = practiceFocusText(item?.successCriterion); + if (!phase || !weakness || !drill || !successCriterion) { + return null; + } + return { + phase, + weakness, + drill, + successCriterion, + }; +} + +export function storeSelectedPracticeFocus( + storage, + owner, + item, + interviewMode = "coding", + sourceReportId = null, +) { + const focus = practiceFocusFrom(item, interviewMode); + const source = + typeof sourceReportId === "string" && sourceReportId + ? sourceReportId + : null; + + try { + if (focus && owner) { + storage.setItem( + selectedPracticeFocusKey, + JSON.stringify({ + owner, + focus, + sourceReportId: source, + }), + ); + return true; + } else { + storage.removeItem(selectedPracticeFocusKey); + return false; + } + } catch { + // The caller needs to know the selection was not persisted. + return false; + } +} + +export function readSelectedPracticeFocusSelection(storage, owner) { + if (!owner) return null; + try { + const raw = storage.getItem(selectedPracticeFocusKey); + if (!raw) return null; + const selected = JSON.parse(raw); + if (selected?.owner !== owner) { + storage.removeItem(selectedPracticeFocusKey); + return null; + } + const focus = practiceFocusFrom(selected.focus); + if (!focus) { + storage.removeItem(selectedPracticeFocusKey); + return null; + } + const sourceReportId = + typeof selected.sourceReportId === "string" && selected.sourceReportId + ? selected.sourceReportId + : null; + return { + focus, + sourceReportId, + }; + } catch { + try { + storage.removeItem(selectedPracticeFocusKey); + } catch { + /* clearing is best effort */ + } + return null; + } +} + +export function readSelectedPracticeFocus(storage, owner) { + return readSelectedPracticeFocusSelection(storage, owner)?.focus ?? null; +} + +export function clearSelectedPracticeFocus(storage) { + try { + storage.removeItem(selectedPracticeFocusKey); + } catch { + /* clearing is best effort */ + } +} + +export function practiceFocusAccountOwner(id, guest = false) { + if (guest) return "device"; + return typeof id === "string" && id ? `account:${id}` : null; +} + +export function practiceFocusOwner(session) { + if (session?.signedIn !== true && session?.signedIn !== false) return null; + return practiceFocusAccountOwner( + session?.user?.id, + session.signedIn === false, + ); +} + /// Carry one concrete action from the reports into the lobby. A focus repeated /// across reports wins; a tie goes to the newest report and its already-ranked /// first item. This does not alter the next problem or become new assessment diff --git a/web/render.js b/web/render.js index 065054943..a5d409fb3 100644 --- a/web/render.js +++ b/web/render.js @@ -658,11 +658,13 @@ export function reportMarkup({ ? "" : `

Practice next

    ${report.improvementPlan .map( - (item) => ` + (item, index) => `
  1. ${escapeHtml(phaseName(item.phase, atBoard))} · ${escapeHtml(item.durationMin)} min · ${escapeHtml(item.impact)} impact

    ${escapeHtml(item.drill)}

    Success: ${escapeHtml(item.successCriterion)}

      ${item.selfReview.map((check) => `
    • ${escapeHtml(check)}
      • `).join("")}
    + +
    2. `, ) .join("")}
`; diff --git a/web/replay.js b/web/replay.js index 545f23a5d..9765cfe22 100644 --- a/web/replay.js +++ b/web/replay.js @@ -28,6 +28,11 @@ import { createBoard, drawBoard, } from "/whiteboard.js"; +import { + practiceFocusAccountOwner, + storeSelectedPracticeFocus, +} from "./problem-picker.js"; +import { storageArea } from "./audio-output.js"; const nodes = { list: document.querySelector("#replay-list"), @@ -495,6 +500,7 @@ async function loadReport(interviewId, recordingId) { nodes.report.textContent = "Could not load the report."; return; } + const reportOwner = practiceFocusAccountOwner(body?.accountId); // Matched on the interview id the report carries, which is why // `web/interview.js` puts it there. Reports are keyed by their own id and // recordings by theirs, and the clock is not a key. @@ -516,6 +522,31 @@ async function loadReport(interviewId, recordingId) { unclearLines: unclearLinesOf(saved.unclearLines), ...final, }); + for (const button of nodes.report.querySelectorAll("[data-practice-focus]")) { + button.addEventListener("click", async () => { + const item = + report.improvementPlan?.[Number(button.dataset.practiceFocus)]; + if (!item) return; + const stored = storeSelectedPracticeFocus( + storageArea("sessionStorage"), + reportOwner, + item, + saved.interviewMode, + saved.id, + ); + if (!stored) { + const status = button.parentElement?.querySelector( + "[data-practice-focus-status]", + ); + if (status) { + status.hidden = false; + status.textContent = "Could not save this practice focus."; + } + return; + } + window.location.href = "/"; + }); + } mountFinalCodeCopy(nodes.report, final.code); void loadOriginalProblem( nodes.report, diff --git a/web/styles.css b/web/styles.css index 27e843f30..d37f722a5 100644 --- a/web/styles.css +++ b/web/styles.css @@ -2227,7 +2227,8 @@ table.phase-scores { } .report-actions button, -.final-code button { +.final-code button, +.practice-focus-button { border: 1px solid var(--hairline); border-radius: var(--radius-sm); background: var(--accent-fill); @@ -2237,23 +2238,37 @@ table.phase-scores { font-weight: 700; } -/* Done is the way on; the download is a copy to keep, so it is outlined - rather than a second filled button of equal weight beside it. */ +/* Done is the way on; the download is a copy to keep, and Practice this + selects the next focus, so both stay outlined rather than competing with + Done. Practice this fills with the accent on hover. */ + .report-actions .secondary, -.final-code .secondary { +.final-code .secondary, +.practice-focus-button { border-color: var(--control-border); background: transparent; color: var(--ink); } +.practice-focus-button { + margin: 0.75rem 0 0.5rem; +} + .report-actions .secondary:hover, .final-code .secondary:hover { border-color: var(--accent); } +.practice-focus-button:not(:disabled):hover { + border-color: var(--accent); + background: var(--accent-deep); + color: var(--on-accent); +} + /* Saving and copying both keep their controls visibly waiting. */ .report-actions button:disabled, -.final-code button[aria-disabled="true"] { +.final-code button[aria-disabled="true"], +.practice-focus-button:disabled { opacity: 0.55; cursor: progress; }