diff --git a/platform/api/migrations/015_scenario_attempts_abandonment.sql b/platform/api/migrations/015_scenario_attempts_abandonment.sql new file mode 100644 index 0000000..32db14a --- /dev/null +++ b/platform/api/migrations/015_scenario_attempts_abandonment.sql @@ -0,0 +1,20 @@ +-- Abandoned/interrupted attempt handling — Phase 4.1, docs/PHASE4_1_PREPRODUCTION_HARDENING.md +-- §13. 'abandoned' was already a valid scenario_attempts.status value (migration 012's CHECK +-- constraint) but nothing ever set it — an attempt left open when a Lab was reaped, an +-- environment destroyed, a session ended, or a student moved on to a different scenario just +-- stayed 'in_progress' forever, an honest but incomplete signal. This is NOT an academic +-- failure marker — an abandoned attempt has no `result`, doesn't count against the student, +-- and the row is never deleted (see src/attempts.js abandonAttempt for the enforcement of +-- that). It exists so instructor views and needs-attention logic can tell "still working" from +-- "stopped, for a reason we know." + +ALTER TABLE scenario_attempts ADD COLUMN IF NOT EXISTS abandoned_at TIMESTAMPTZ; +ALTER TABLE scenario_attempts ADD COLUMN IF NOT EXISTS abandoned_reason TEXT; + +DO $$ BEGIN + ALTER TABLE scenario_attempts ADD CONSTRAINT scenario_attempts_abandoned_reason_check + CHECK (abandoned_reason IS NULL OR abandoned_reason IN + ('environment_destroyed', 'session_ended', 'scenario_switched')); +EXCEPTION + WHEN duplicate_object THEN NULL; +END $$; diff --git a/platform/api/src/attempts.js b/platform/api/src/attempts.js index 036ba4a..b782aaa 100644 --- a/platform/api/src/attempts.js +++ b/platform/api/src/attempts.js @@ -2,11 +2,55 @@ // session on a scenario (not per validation click, see migrations/012). Every mutation here // updates that one row in place; nothing is ever inserted twice for the same open attempt. // -// Deliberately does NOT try to detect "abandoned" attempts automatically (e.g. on Lab destroy) -// — that would need a reaper-style background job wired to Provisioner environment_events, -// which is more machinery than the instructional-visibility goal here needs. An attempt just -// stays 'in_progress' if the student never returns to finish it; instructors can already see -// "in_progress, N failed validations" as a real signal. Documented as a known limitation. +// Phase 4.1 (docs/PHASE4_1_PREPRODUCTION_HARDENING.md §13) added real abandonment handling — +// see abandonAttempt/abandonOpenAttemptsFor* below and their call sites (environment +// destroy/reset in server.js and admin-routes.js, logout in server.js, the session sweeper in +// sessions.js, and the "different scenario" transition inline in getOrCreateOpenAttempt just +// below). Still deliberately NOT a new standalone background job — every trigger either rides +// an existing request handler or the session sweeper that already runs periodically. + +// An attempt is never silently deleted or marked as a failure by abandonment — result stays +// NULL, the row stays queryable, instructors see "abandoned (reason)" as a real, honest status +// rather than a punitive one. No-op (and does not overwrite an existing abandoned_reason) if +// the attempt is already terminal. +async function abandonAttempt(pool, attemptId, reason) { + const { rows } = await pool.query( + `UPDATE scenario_attempts SET status = 'abandoned', abandoned_at = now(), abandoned_reason = $2 + WHERE attempt_id = $1 AND status = 'in_progress' RETURNING *`, + [attemptId, reason] + ); + return rows[0] || null; +} + +// Trigger: environment destroyed (idle reaper, student reset, admin reset/destroy — see +// server.js's student reset route and admin-routes.js's reset/destroy routes, all of which +// call this right after telling the Provisioner to tear the environment down). A reset creates +// a fresh environment_id in some flows and reuses the same one in others (see +// provisioner lifecycle.js's reset — same environment_id, fresh container) — either way, any +// attempt still open against the OLD container state is honestly "not going to be resumed as +// the same working session," so it's abandoned rather than left open indefinitely. +async function abandonOpenAttemptsForEnvironment(pool, environmentId) { + const { rows } = await pool.query( + `UPDATE scenario_attempts SET status = 'abandoned', abandoned_at = now(), abandoned_reason = 'environment_destroyed' + WHERE environment_id = $1 AND status = 'in_progress' RETURNING attempt_id`, + [environmentId] + ); + return rows.length; +} + +// Trigger: logout (server.js POST /api/logout) and the session sweeper's periodic pass for +// students whose sessions have all expired (sessions.js). Deliberately student-scoped, not +// environment-scoped — a student can log out while their Lab keeps running (it isn't +// destroyed until reaped or explicitly ended), but the WORKING SESSION on whatever scenario +// they had open is over the moment they're no longer authenticated to record more progress on it. +async function abandonOpenAttemptsForStudent(pool, studentId) { + const { rows } = await pool.query( + `UPDATE scenario_attempts SET status = 'abandoned', abandoned_at = now(), abandoned_reason = 'session_ended' + WHERE student_id = $1 AND status = 'in_progress' RETURNING attempt_id`, + [studentId] + ); + return rows.length; +} async function getOrCreateOpenAttempt(pool, { studentId, scenarioId, environmentId, cohortId, curriculumStage, assessmentSessionId }) { const { rows: existing } = await pool.query( @@ -17,6 +61,17 @@ async function getOrCreateOpenAttempt(pool, { studentId, scenarioId, environment ); if (existing[0]) return existing[0]; + // Trigger: student starts a different scenario while a previous one is still open (started + // via /start or implicitly via /validate, never finished). Only one scenario is "the one + // currently being worked on" per student+environment at a time from the app's own UX, so an + // open attempt on ANY OTHER scenario in this same environment is abandoned before the new + // one is created — never silently left as a phantom "still in progress" row. + await pool.query( + `UPDATE scenario_attempts SET status = 'abandoned', abandoned_at = now(), abandoned_reason = 'scenario_switched' + WHERE student_id = $1 AND environment_id = $2 AND scenario_id != $3 AND status = 'in_progress'`, + [studentId, environmentId, scenarioId] + ); + const { rows } = await pool.query( `INSERT INTO scenario_attempts (student_id, scenario_id, environment_id, cohort_id, curriculum_stage, started_at, status, assessment_session_id) @@ -80,4 +135,7 @@ async function getAttemptSummary(pool, { studentId, scenarioId, environmentId }) return rows[0] || null; } -module.exports = { getOrCreateOpenAttempt, recordValidation, recordHintReveal, getAttemptSummary }; +module.exports = { + getOrCreateOpenAttempt, recordValidation, recordHintReveal, getAttemptSummary, + abandonAttempt, abandonOpenAttemptsForEnvironment, abandonOpenAttemptsForStudent, +}; diff --git a/platform/api/src/sessions.js b/platform/api/src/sessions.js index 0eeac60..cc0bd52 100644 --- a/platform/api/src/sessions.js +++ b/platform/api/src/sessions.js @@ -104,11 +104,28 @@ async function listInstructorSessions(pool, instructorId) { // Periodic cleanup of long-dead rows so the table doesn't grow forever — cosmetic/storage only, // not a security control (expired/revoked rows already fail resolveSession's WHERE clause). +// +// Phase 4.1 §13 also piggybacks the "session expires" abandonment trigger onto this same +// existing interval, rather than adding a new background job: any scenario_attempts row still +// 'in_progress' for a student with NO live session left (every session expired, idle-timed-out, +// or revoked, not just deleted by the sweep above) gets abandoned. Deliberately does the +// attempts update in the SAME pool.query call — one round trip, no separate job to keep in sync. function startSessionSweeper(pool) { setInterval(() => { pool.query(`DELETE FROM sessions WHERE expires_at < now() - interval '7 days'`).catch((e) => { console.error('[session sweep] failed:', e.message); }); + pool.query(` + UPDATE scenario_attempts SET status = 'abandoned', abandoned_at = now(), abandoned_reason = 'session_ended' + WHERE status = 'in_progress' + AND NOT EXISTS ( + SELECT 1 FROM sessions + WHERE sessions.student_id = scenario_attempts.student_id + AND sessions.revoked_at IS NULL AND sessions.expires_at > now() AND sessions.idle_expires_at > now() + ) + `).catch((e) => { + console.error('[session sweep] attempt abandonment failed:', e.message); + }); }, 60 * 60 * 1000).unref(); }