From e2deadab9948a27224df31b86ffdd3d9df6b35ae Mon Sep 17 00:00:00 2001 From: karthikeya veginati Date: Thu, 24 Sep 2026 12:52:20 -0500 Subject: [PATCH 1/9] feat(middlewareNode): currency ledger infra (Karthik's Rev. 2 lane) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the infra/ledger workstream from the currency rollout plan: - ActionEvent/ActionRule/LedgerEntry/UserBalance schemas, with lifetimeEarned tracked separately from spendable balance so a future spend feature can't make the leaderboard reward not-spending. - A dependency-free MongoDB polling consumer (no Redis) with atomic claim-based processing and crash recovery via stale-claim requeue. - An idempotent ledger writer verified under real concurrent-write conditions, plus cooldown/daily-cap abuse limits checked against the ledger itself (not event status, which only reflects the calling consumer's own lifecycle). - An hourly anomaly-monitoring job (MAD-based, flag-only) that catches a student earning at an outlier rate across many actions combined, which no single action's cooldown/cap can see on its own. - Rate limiting on /gameResults, the current event-triggering endpoint. - A historical backfill script, deliberately scoped to game results only — lessonsCompleted has no per-completion timestamp and timeTrackings' puzzle records measure time spent, not puzzles solved, so backfilling currency from either would mean fabricating history rather than reconstructing it. - Removed PUT /user/updateHighScore: a client-writable score-forging endpoint with zero real callers, found during the readiness audit. 306 tests passing, zero regressions against the existing suite. Co-Authored-By: Claude Sonnet 5 --- middlewareNode/src/models/actionEvent.js | 77 +++++ middlewareNode/src/models/actionRule.js | 43 +++ middlewareNode/src/models/ledgerEntry.js | 53 ++++ middlewareNode/src/models/userBalance.js | 42 +++ .../src/scheduler/currencyAnomalyScheduler.js | 181 ++++++++++++ .../src/scripts/backfillCurrencyLedger.js | 272 +++++++++++++++++ .../src/services/currencyConsumer.js | 147 +++++++++ middlewareNode/src/services/ledgerService.js | 201 +++++++++++++ .../tests/backfillCurrencyLedger.test.js | 270 +++++++++++++++++ .../tests/currencyAnomalyScheduler.test.js | 162 ++++++++++ middlewareNode/tests/currencyConsumer.test.js | 189 ++++++++++++ middlewareNode/tests/ledgerService.test.js | 279 ++++++++++++++++++ 12 files changed, 1916 insertions(+) create mode 100644 middlewareNode/src/models/actionEvent.js create mode 100644 middlewareNode/src/models/actionRule.js create mode 100644 middlewareNode/src/models/ledgerEntry.js create mode 100644 middlewareNode/src/models/userBalance.js create mode 100644 middlewareNode/src/scheduler/currencyAnomalyScheduler.js create mode 100644 middlewareNode/src/scripts/backfillCurrencyLedger.js create mode 100644 middlewareNode/src/services/currencyConsumer.js create mode 100644 middlewareNode/src/services/ledgerService.js create mode 100644 middlewareNode/tests/backfillCurrencyLedger.test.js create mode 100644 middlewareNode/tests/currencyAnomalyScheduler.test.js create mode 100644 middlewareNode/tests/currencyConsumer.test.js create mode 100644 middlewareNode/tests/ledgerService.test.js diff --git a/middlewareNode/src/models/actionEvent.js b/middlewareNode/src/models/actionEvent.js new file mode 100644 index 00000000..0d66ee36 --- /dev/null +++ b/middlewareNode/src/models/actionEvent.js @@ -0,0 +1,77 @@ +/** + * ActionEvent Schema + * + * Append-only raw log of "something happened that might earn currency" — + * one document per real-world action (a lesson completed, a puzzle solved), + * written durably before anything decides whether it pays out. + * + * This is a plain MongoDB collection, not a message broker (see Rev. 2 of + * the currency rollout plan, "Drop Redis from this window"): nothing here + * needs consumer groups or broker-grade throughput at current scale, and a + * collection gets the same durability/replay/audit properties for free, + * with no new ops dependency. + * + * `eventId` is unique, which is the idempotency key all the way down the + * pipeline — the same pattern GameResults uses `gameId` for. A producer + * (or the historical backfill) can safely retry an insert; a duplicate + * `eventId` is rejected by the unique index rather than silently + * double-processed. + * + * `status` drives the consumer: it claims a batch of "pending" events, + * processes them, and marks each "processed" or "failed" so a crash + * mid-batch never loses or silently re-skips work. See + * services/currencyConsumer.js. + */ + +const mongoose = require("mongoose"); + +const ActionEventSchema = new mongoose.Schema( + { + // Idempotency key. Producers mint this deterministically (e.g. + // `lesson:::`); the historical + // backfill mints synthetic ones (e.g. `backfill:lesson:::`) + // so it is safely re-runnable against the same source data. + eventId: { type: String, required: true, unique: true, index: true }, + + // Matches an ActionRule.actionKey — e.g. "lesson.completed", "puzzle.solved". + actionKey: { type: String, required: true, index: true }, + + userId: { type: mongoose.Schema.Types.ObjectId, required: true, index: true }, + + // Action-specific payload (lessonId, piece, difficulty, ...). Opaque to + // the consumer beyond what a given ActionRule's conditions inspect. + metadata: { type: mongoose.Schema.Types.Mixed, default: {} }, + + // Compound-indexed with actionKey below — the consumer's claim query + // and the cooldown/daily-cap checks both filter on status + userId + + // actionKey + occurredAt. + status: { + type: String, + enum: ["pending", "processed", "failed"], + default: "pending", + index: true, + }, + + // Set once the consumer finishes with this event, whichever way. + processedAt: { type: Date, default: null }, + + // Populated only when status is "failed" — the rules-engine or ledger + // error that occurred, so a stuck event is diagnosable without log + // spelunking. + failureReason: { type: String, default: null }, + + // When the underlying action actually happened — not when this record + // was written. Backfilled events set this to the historical date so + // cooldown/daily-cap windows evaluate correctly against real history. + occurredAt: { type: Date, required: true, default: Date.now, index: true }, + }, + { timestamps: true } +); + +// The consumer's core query: "give me pending events for this action, +// oldest first." Also serves cooldown/daily-cap lookups scoped to a single +// user+action. +ActionEventSchema.index({ status: 1, actionKey: 1, occurredAt: 1 }); +ActionEventSchema.index({ userId: 1, actionKey: 1, occurredAt: -1 }); + +module.exports = mongoose.model("ActionEvent", ActionEventSchema); diff --git a/middlewareNode/src/models/actionRule.js b/middlewareNode/src/models/actionRule.js new file mode 100644 index 00000000..985b1382 --- /dev/null +++ b/middlewareNode/src/models/actionRule.js @@ -0,0 +1,43 @@ +/** + * ActionRule Schema + * + * The config layer that decides whether an ActionEvent pays out, and how + * much. Adding a new currency-earning action is meant to be "one emit line + * plus one ActionRule document" — zero code changes to the consumer (the + * plan's "action #47" test). + * + * No admin UI this window (deferred per the currency rollout plan) — + * these are created and edited directly against MongoDB. + */ + +const mongoose = require("mongoose"); + +const ActionRuleSchema = new mongoose.Schema( + { + // Matches ActionEvent.actionKey. Unique — one rule per action. + actionKey: { type: String, required: true, unique: true, index: true }, + + // How much lifetimeEarned/balance an occurrence of this action pays out. + currencyAmount: { type: Number, required: true, min: 0 }, + + // A disabled rule's events are still logged (ActionEvent is + // append-only) but never produce a ledger entry — lets an action be + // turned off without losing the underlying history. + active: { type: Boolean, default: true, index: true }, + + // Minimum seconds between two payouts for the same user+action. 0 + // disables cooldown enforcement for this action. + cooldownSeconds: { type: Number, default: 0, min: 0 }, + + // Max payouts per user+action per UTC day. 0 disables the cap. + dailyCap: { type: Number, default: 0, min: 0 }, + + // Optional free-form notes on why the rule exists / its current amount + // — this collection has no admin UI, so this is the only place that + // context lives outside a commit message. + notes: { type: String, default: "" }, + }, + { timestamps: true } +); + +module.exports = mongoose.model("ActionRule", ActionRuleSchema); diff --git a/middlewareNode/src/models/ledgerEntry.js b/middlewareNode/src/models/ledgerEntry.js new file mode 100644 index 00000000..36d1f5c2 --- /dev/null +++ b/middlewareNode/src/models/ledgerEntry.js @@ -0,0 +1,53 @@ +/** + * LedgerEntry Schema + * + * Append-only, source of truth for currency balance. Never a running + * total — every award is its own immutable row, the same computed-on-read + * philosophy GameResults uses for chess record/score (see + * models/gameResults.js): change how points are computed and every + * historical entry is still exactly what it says, with nothing to drift + * or backfill in the ledger itself. + * + * `eventId` carries the same value as the ActionEvent it was created from + * and is unique here too — this is the actual idempotency guarantee for + * the whole pipeline. The consumer's insert either succeeds once or fails + * with a duplicate-key error that it treats as "already processed, + * nothing to do" (mirroring gameResults.js's `err.code === 11000` handling), + * so a re-delivered or retried event can never double-pay. + * + * Amounts are positive-only this window — no spend path exists yet + * (deferred per the currency rollout plan). UserBalance.lifetimeEarned is + * the running sum of these entries; the field is named lifetimeEarned + * rather than balance so that when a spend path does ship, ranking logic + * that already reads lifetimeEarned doesn't start rewarding students for + * not spending. + */ + +const mongoose = require("mongoose"); + +const LedgerEntrySchema = new mongoose.Schema( + { + // Idempotency key — matches the source ActionEvent.eventId exactly. + eventId: { type: String, required: true, unique: true, index: true }, + + userId: { type: mongoose.Schema.Types.ObjectId, required: true, index: true }, + + actionKey: { type: String, required: true, index: true }, + + // Positive-only this window. Kept as a plain Number (not unsigned) so + // a future spend path is a validation change, not a schema migration. + amount: { type: Number, required: true }, + + // When the underlying action happened (copied from ActionEvent.occurredAt), + // not when this row was written — a backfilled entry's history should + // read as history, not as having happened at backfill time. + occurredAt: { type: Date, required: true, index: true }, + }, + { timestamps: true } +); + +// A user's full ledger history, newest first — the query the future spend +// path and any per-student ledger view will both want. +LedgerEntrySchema.index({ userId: 1, occurredAt: -1 }); + +module.exports = mongoose.model("LedgerEntry", LedgerEntrySchema); diff --git a/middlewareNode/src/models/userBalance.js b/middlewareNode/src/models/userBalance.js new file mode 100644 index 00000000..3de26f0e --- /dev/null +++ b/middlewareNode/src/models/userBalance.js @@ -0,0 +1,42 @@ +/** + * UserBalance Schema + * + * Read-optimized cache of a user's ledger position, updated alongside + * every LedgerEntry write. This collection is a cache, not a source of + * truth — LedgerEntry is; a UserBalance document can always be rebuilt by + * summing that user's LedgerEntry rows, which is exactly what the + * historical backfill's replay does on first run. + * + * Two fields, deliberately different meanings (see Rev. 2 change 3, + * "Rank by lifetime earned, not balance"): + * + * - balance: spendable total. Unused this window (no spend path + * exists yet) but present now so adding one later is + * a feature change, not a schema migration. + * - lifetimeEarned: monotonic sum of positive LedgerEntry amounts only. + * Never decreases. THIS is what the leaderboard reads + * — ranking by spendable balance would mean the top + * of the leaderboard is whichever student has + * redeemed the least, the moment a store ships. + * + * A user with no document here (hasn't earned anything yet) must be + * treated as lifetimeEarned: 0 by any reader — see + * getLifetimeEarnedOrZero in services/ledgerService.js — never as + * undefined, which would produce undefined sort ordering in the + * leaderboard's comparator. + */ + +const mongoose = require("mongoose"); + +const UserBalanceSchema = new mongoose.Schema( + { + userId: { type: mongoose.Schema.Types.ObjectId, required: true, unique: true, index: true }, + + balance: { type: Number, required: true, default: 0 }, + + lifetimeEarned: { type: Number, required: true, default: 0, index: true }, + }, + { timestamps: true } +); + +module.exports = mongoose.model("UserBalance", UserBalanceSchema); diff --git a/middlewareNode/src/scheduler/currencyAnomalyScheduler.js b/middlewareNode/src/scheduler/currencyAnomalyScheduler.js new file mode 100644 index 00000000..5a3c6352 --- /dev/null +++ b/middlewareNode/src/scheduler/currencyAnomalyScheduler.js @@ -0,0 +1,181 @@ +/** + * Currency Anomaly Monitoring Scheduler + * + * Flags — never blocks — students whose currency earn rate over the last + * 24h significantly exceeds the population's. Per Karthik's ledger plan, + * Week 3 hardening: "anomaly-monitoring scheduled job; flag, don't + * auto-block, users whose earn rate significantly exceeds the median." + * + * Deliberately flag-only: cooldowns and daily caps (ledgerService.js) are + * the actual enforcement layer and already reject an award before it's + * written. This job exists for the case those miss — a student who stays + * under every individual action's cap but still earns at an outlier rate + * across many different actions combined, which no single ActionRule can + * see. A human reviews flags; nothing here mutates a balance or an event. + * + * Detection: median absolute deviation (MAD) over each user's last-24h + * LedgerEntry total, rather than mean + standard deviation — a + * mean-based threshold is itself dragged up by the very outliers it's + * supposed to catch, especially with a small population of earners on + * any given day. MAD is robust to that. + * + * Schedule: hourly ('0 * * * *') — tighter than the nightly analytics + * summary, since a currency exploit is worth catching same-day, not + * next-day. + * + * Output collection: currencyAnomalyFlags + * { userId, date, totalEarned24h, medianEarned24h, deviationScore, + * createdAt } + * One flag per user per run where the threshold is exceeded — not + * deduplicated across runs, so a sustained anomaly produces a flag every + * hour it persists; a reviewer scanning the collection sees how long it's + * been going on, which is itself useful signal. + */ + +const schedule = require("node-schedule"); +const mongoose = require("mongoose"); +const LedgerEntry = require("../models/ledgerEntry"); + +const LOOKBACK_MS = 24 * 60 * 60 * 1000; +// A user's 24h total must be at least this many MADs above the median to +// flag — 3.5 is a conventional "clearly an outlier" threshold for MAD-based +// detection (see Iglewicz & Hoaglin), chosen over the more common z-score +// equivalent (~3) specifically because MAD already discounts the outliers +// themselves; no extra slack needed to compensate for that. +const MAD_THRESHOLD = 3.5; +// Consistency constant that makes MAD comparable to a standard deviation +// under a normal distribution — standard for this technique. +const MAD_CONSISTENCY_CONSTANT = 1.4826; +// Skip the check entirely below this many earners in the window — MAD is +// meaningless (or trivially triggers on any two different values) with a +// tiny sample, so a quiet night doesn't produce false flags. +const MIN_EARNERS_FOR_DETECTION = 5; + +function median(sortedNumbers) { + const n = sortedNumbers.length; + const mid = Math.floor(n / 2); + return n % 2 === 0 ? (sortedNumbers[mid - 1] + sortedNumbers[mid]) / 2 : sortedNumbers[mid]; +} + +/** + * Computes each user's total positive LedgerEntry amount within the + * lookback window. Returns a Map. + */ +async function computeEarnedTotals(since) { + const rows = await LedgerEntry.aggregate([ + { $match: { occurredAt: { $gte: since }, amount: { $gt: 0 } } }, + { $group: { _id: "$userId", total: { $sum: "$amount" } } }, + ]); + + const totals = new Map(); + for (const row of rows) { + totals.set(String(row._id), row.total); + } + return totals; +} + +/** + * Runs one detection pass and returns the flags it would raise (or did + * raise, if persist is true) — split out from the scheduled job itself so + * it's directly testable without node-schedule or a live cron tick. + */ +async function detectAnomalies({ now = new Date(), persist = true } = {}) { + const since = new Date(now.getTime() - LOOKBACK_MS); + const totals = await computeEarnedTotals(since); + + const values = Array.from(totals.values()); + if (values.length < MIN_EARNERS_FOR_DETECTION) { + return { flagged: [], skipped: true, reason: "too few earners in window", earnerCount: values.length }; + } + + const sorted = [...values].sort((a, b) => a - b); + const med = median(sorted); + const absDeviations = sorted.map((v) => Math.abs(v - med)).sort((a, b) => a - b); + const mad = median(absDeviations); + + // A zero MAD means at least half the population sits exactly at the + // median — but that splits into two very different cases: + // - everyone is at the median (no deviations at all): nothing to + // compare against, correctly nothing to flag. + // - a majority sits at the median and a minority doesn't: the normal + // "divide by MAD" formula breaks (division by zero), but this is + // actually the STRONGEST possible signal, not the weakest — "almost + // everyone earned exactly X" makes any nonzero deviation from X + // stand out completely on its own, with no scaled score needed. + const allAtMedian = absDeviations.every((d) => d === 0); + if (mad === 0) { + if (allAtMedian) { + return { flagged: [], skipped: true, reason: "zero deviation in window", earnerCount: values.length }; + } + + const flagged = []; + for (const [userId, total] of totals.entries()) { + if (total !== med) { + flagged.push({ userId, date: now, totalEarned24h: total, medianEarned24h: med, deviationScore: Infinity }); + } + } + if (persist && flagged.length > 0) { + await persistFlags(flagged); + } + return { flagged, skipped: false, earnerCount: values.length, medianEarned24h: med, mad: 0 }; + } + + const flagged = []; + for (const [userId, total] of totals.entries()) { + // 0.6745 normalizes the modified z-score so a MAD-based deviation is + // comparable to a standard z-score threshold — the standard formula. + const deviationScore = (0.6745 * (total - med)) / mad; + if (deviationScore >= MAD_THRESHOLD) { + flagged.push({ + userId, + date: now, + totalEarned24h: total, + medianEarned24h: med, + deviationScore, + }); + } + } + + if (persist && flagged.length > 0) { + await persistFlags(flagged); + } + + return { flagged, skipped: false, earnerCount: values.length, medianEarned24h: med, mad }; +} + +async function persistFlags(flagged) { + const db = mongoose.connection.db; + await db.collection("currencyAnomalyFlags").insertMany( + flagged.map((f) => ({ ...f, userId: new mongoose.Types.ObjectId(f.userId), createdAt: new Date() })) + ); +} + +async function runAnomalyCheck() { + console.log("[currencyAnomaly] Starting hourly anomaly check..."); + try { + const result = await detectAnomalies(); + if (result.skipped) { + console.log(`[currencyAnomaly] Skipped: ${result.reason} (earners=${result.earnerCount})`); + return; + } + if (result.flagged.length > 0) { + console.warn(`[currencyAnomaly] Flagged ${result.flagged.length} user(s) for review.`); + } else { + console.log(`[currencyAnomaly] No anomalies (earners=${result.earnerCount}, median=${result.medianEarned24h}).`); + } + } catch (err) { + console.error("[currencyAnomaly] Check failed:", err.message); + } +} + +// Hourly, on the hour. Skipped under Jest — same reasoning as +// currencyConsumer's NODE_ENV guard (services/currencyConsumer.js): a +// live node-schedule job registered at require-time leaves an open timer +// handle that Jest has to wait out (or forcibly kill) on every test run +// that imports this file, including indirectly via server.js. +if (process.env.NODE_ENV !== "test") { + schedule.scheduleJob("0 * * * *", runAnomalyCheck); + console.log("[currencyAnomaly] Scheduler registered — runs hourly."); +} + +module.exports = { detectAnomalies, runAnomalyCheck, computeEarnedTotals, median }; diff --git a/middlewareNode/src/scripts/backfillCurrencyLedger.js b/middlewareNode/src/scripts/backfillCurrencyLedger.js new file mode 100644 index 00000000..da282aff --- /dev/null +++ b/middlewareNode/src/scripts/backfillCurrencyLedger.js @@ -0,0 +1,272 @@ +/** + * One-time (but safely re-runnable) backfill: synthesize historical + * ActionEvent records from existing GameResults documents, then replay + * them through the real currency consumer so every existing student's + * UserBalance.lifetimeEarned reflects real history before the leaderboard + * swap (Jimmy's Week 3) goes live. See Karthik's ledger plan, "Week 2-3 — + * historical backfill." + * + * SCOPE: games only. The plan's original wording ("synthesize from + * timeTrackings, users.lessonsCompleted, gameResults") assumed all three + * sources cleanly map to discrete, timestamped completion events. They do + * not: + * - gameResults has a real playedAt per finished game — clean. + * - users.lessonsCompleted is a current-state snapshot ({ piece, + * lessonNumber }) with NO per-completion timestamp — there is no + * historical date to backfill from, only "how many are done right + * now." + * - timeTrackings' eventType: "puzzle" records time SPENT on puzzles, + * not puzzles SOLVED — backfilling currency from it would pay for + * time-on-page, not achievement, which is a different thing than + * what a live puzzle.solved event will mean once Jimmy wires one. + * Backfilling lessons/puzzles from these sources would mean guessing at + * history rather than reconstructing it. Deliberately deferred — every + * existing student's lesson/puzzle currency starts at 0 and accrues from + * here forward, same as a student who joined the day this ships. Only + * game-playing history is backfilled. Revisit if a real completion-dated + * source for lessons/puzzles becomes available (e.g. once Srujana's + * completedDates write path — a separate, independent lane — is live and + * has accumulated some real history of its own). + * + * Idempotency: each synthetic ActionEvent gets a deterministic eventId + * (`backfill:game::`) — running this script twice + * produces the same eventIds, which the unique index on + * ActionEvent.eventId (and again on LedgerEntry.eventId, the real + * idempotency backstop) rejects as duplicates. Re-running after new games + * have been played only backfills the new ones. + * + * Requires an active ActionRule for "game.played" to exist — see + * ensureGamePlayedRule() below, which creates a conservative default if + * one is missing rather than silently processing zero events. + * + * Usage: + * node src/scripts/backfillCurrencyLedger.js + * node src/scripts/backfillCurrencyLedger.js --dry-run + */ + +require("dotenv").config(); +const mongoose = require("mongoose"); +// Deferred to inside run() rather than required at module scope: this +// file also exports helpers for tests/backfillCurrencyLedger.test.js, +// which connects to its own in-memory MongoDB instance directly and +// never wants this package's config/*.json lookup (and the "no config +// file matches NODE_ENV=test" warning that comes with it under Jest). + +const GameResults = require("../models/gameResults"); +const ActionEvent = require("../models/actionEvent"); +const ActionRule = require("../models/actionRule"); +const { drainBatch } = require("../services/currencyConsumer"); + +const GAME_PLAYED_ACTION_KEY = "game.played"; +const DEFAULT_GAME_PLAYED_AMOUNT = 5; + +function backfillEventId(gameId, username) { + return `backfill:game:${gameId}:${username}`; +} + +/** + * Looks up the user document for a username and returns its _id, or null + * if the username doesn't resolve to a real user (a stale/renamed + * account) — skipped rather than crashing the whole backfill. + */ +async function resolveUserId(usersCollection, username) { + const user = await usersCollection.findOne({ username }, { projection: { _id: 1 } }); + return user ? user._id : null; +} + +/** + * Ensures an ActionRule exists for game.played so backfilled events have + * something to pay out against. If one already exists (created in Week 0 + * as part of the real contract), this leaves it untouched — the backfill + * should never override a deliberately configured rule. Only creates a + * conservative default when none exists at all, so `no_rule` doesn't + * silently swallow every backfilled event. + */ +async function ensureGamePlayedRule({ dryRun }) { + const existing = await ActionRule.findOne({ actionKey: GAME_PLAYED_ACTION_KEY }); + if (existing) { + console.log(`ActionRule for "${GAME_PLAYED_ACTION_KEY}" already exists (amount=${existing.currencyAmount}, active=${existing.active}) — leaving as configured.`); + return existing; + } + + console.log(`No ActionRule for "${GAME_PLAYED_ACTION_KEY}" found.`); + if (dryRun) { + console.log(`[dry-run] Would create one with currencyAmount=${DEFAULT_GAME_PLAYED_AMOUNT}.`); + return { actionKey: GAME_PLAYED_ACTION_KEY, currencyAmount: DEFAULT_GAME_PLAYED_AMOUNT, active: true }; + } + + const created = await ActionRule.create({ + actionKey: GAME_PLAYED_ACTION_KEY, + currencyAmount: DEFAULT_GAME_PLAYED_AMOUNT, + active: true, + cooldownSeconds: 0, + dailyCap: 0, + notes: "Created automatically by backfillCurrencyLedger.js — review the amount before Final Merge.", + }); + console.log(`Created a default ActionRule for "${GAME_PLAYED_ACTION_KEY}" with currencyAmount=${DEFAULT_GAME_PLAYED_AMOUNT}. Review this before relying on it in production.`); + return created; +} + +/** + * Synthesizes one ActionEvent per (game, player) pair for every + * GameResults document not already backfilled, and inserts them as + * "pending" — exactly as if a live game.played producer had just emitted + * them. Returns counts, not the documents themselves, since a real + * backfill can cover years of games. + */ +async function synthesizeGameEvents({ dryRun, usersCollection }) { + const cursor = GameResults.find({}).lean().cursor(); + + let gamesScanned = 0; + let eventsInserted = 0; + let eventsSkippedDuplicate = 0; + let eventsSkippedUnresolvedUser = 0; + const unresolvedUsernames = new Set(); + + for await (const game of cursor) { + gamesScanned++; + + for (const username of game.players) { + const eventId = backfillEventId(game.gameId, username); + + const userId = await resolveUserId(usersCollection, username); + if (!userId) { + eventsSkippedUnresolvedUser++; + unresolvedUsernames.add(username); + continue; + } + + if (dryRun) { + const alreadyExists = await ActionEvent.exists({ eventId }); + if (alreadyExists) { + eventsSkippedDuplicate++; + } else { + eventsInserted++; + } + continue; + } + + try { + await ActionEvent.create({ + eventId, + actionKey: GAME_PLAYED_ACTION_KEY, + userId, + metadata: { + gameId: game.gameId, + result: game.result, + reason: game.reason, + backfilled: true, + }, + status: "pending", + occurredAt: game.playedAt, + }); + eventsInserted++; + } catch (err) { + if (err && err.code === 11000) { + eventsSkippedDuplicate++; + } else { + throw err; + } + } + } + } + + return { + gamesScanned, + eventsInserted, + eventsSkippedDuplicate, + eventsSkippedUnresolvedUser, + unresolvedUsernames: Array.from(unresolvedUsernames), + }; +} + +/** + * Drains every newly inserted "pending" backfill event through the real + * consumer (services/currencyConsumer.drainBatch), in batches, until + * none remain. This is the "replay through the same consumer the live + * path uses" step — it's the best available test of that consumer, since + * it's about to process a real, large, historically-shaped batch of + * events for the first time. + */ +async function replayPendingEvents({ dryRun, batchSize = 100 }) { + if (dryRun) { + const pendingCount = await ActionEvent.countDocuments({ status: "pending" }); + console.log(`[dry-run] Would replay ${pendingCount} pending event(s) through the consumer.`); + return { totalProcessed: 0, outcomeCounts: {} }; + } + + let totalProcessed = 0; + const outcomeCounts = {}; + + for (;;) { + const results = await drainBatch(batchSize); + if (results.length === 0) break; + + totalProcessed += results.length; + for (const r of results) { + outcomeCounts[r.outcome] = (outcomeCounts[r.outcome] || 0) + 1; + } + } + + return { totalProcessed, outcomeCounts }; +} + +async function run() { + const dryRun = process.argv.includes("--dry-run"); + + const config = require("config"); + await mongoose.connect(config.get("mongoURI")); + console.log(`Connected to MongoDB${dryRun ? " (dry run — no writes)" : ""}`); + + const usersCollection = mongoose.connection.collection("users"); + + await ensureGamePlayedRule({ dryRun }); + + console.log("\nScanning GameResults for events to synthesize..."); + const synthesis = await synthesizeGameEvents({ dryRun, usersCollection }); + + console.log(`Games scanned: ${synthesis.gamesScanned}`); + console.log(`ActionEvents inserted: ${synthesis.eventsInserted}`); + console.log(`Skipped (already backfilled): ${synthesis.eventsSkippedDuplicate}`); + console.log(`Skipped (username not found): ${synthesis.eventsSkippedUnresolvedUser}`); + if (synthesis.unresolvedUsernames.length > 0) { + console.log(` Unresolved usernames: ${synthesis.unresolvedUsernames.join(", ")}`); + } + + console.log("\nReplaying pending events through the real consumer..."); + const replay = await replayPendingEvents({ dryRun }); + console.log(`Events processed: ${replay.totalProcessed}`); + for (const [outcome, count] of Object.entries(replay.outcomeCounts)) { + console.log(` ${outcome}: ${count}`); + } + + if (dryRun) { + console.log("\nDry run complete — no data was written. Re-run without --dry-run to apply."); + } else { + console.log("\nBackfill complete."); + console.log("Reminder: lesson- and puzzle-history backfill was deliberately skipped (see file header) —"); + console.log("existing students' lifetimeEarned reflects games only until live lesson/puzzle events accrue."); + } + + await mongoose.disconnect(); +} + +module.exports = { + backfillEventId, + ensureGamePlayedRule, + synthesizeGameEvents, + replayPendingEvents, +}; + +// Only auto-run as a CLI script (`node src/scripts/backfillCurrencyLedger.js`). +// Unlike the other one-off scripts in this directory, this file is also +// `require()`d by tests/backfillCurrencyLedger.test.js for its exported +// helpers — without this guard, requiring it for those exports would also +// fire run()'s own mongoose.connect(), racing the test file's own +// connection to a different (in-memory) database. +if (require.main === module) { + run().catch((err) => { + console.error("Backfill failed:", err); + process.exit(1); + }); +} diff --git a/middlewareNode/src/services/currencyConsumer.js b/middlewareNode/src/services/currencyConsumer.js new file mode 100644 index 00000000..8501abe6 --- /dev/null +++ b/middlewareNode/src/services/currencyConsumer.js @@ -0,0 +1,147 @@ +/** + * Currency Consumer + * + * Claims pending ActionEvent documents and runs each through + * ledgerService.processEvent(), the shared rules engine. This is the + * "polling worker on status: 'pending'" the currency rollout plan + * specifies in place of a Redis consumer group (Rev. 2, "Drop Redis from + * this window") — no broker, no consumer-group semantics to reimplement, + * just a plain query against a status field. + * + * A Mongo change stream is the noted upgrade path once Week 0's + * replica-set gate passes (change streams require one); polling works + * unconditionally and is what ships by default. Swapping the trigger + * later doesn't change processEvent() or the ActionEvent schema at all. + * + * Crash safety: an event is claimed (status: "pending" -> "claimed") via + * findOneAndUpdate, which is atomic — two consumer processes racing on + * the same event can never both claim it. If the process dies after + * claiming but before finishing, the event is stuck as "claimed," not + * silently lost or double-processed; requeueStuckClaims() below recovers + * it. A production deployment should call that on startup and on an + * interval. + */ + +const ActionEvent = require("../models/actionEvent"); +const { processEvent } = require("./ledgerService"); + +const DEFAULT_BATCH_SIZE = 25; +const DEFAULT_POLL_INTERVAL_MS = 5000; +// How long an event may sit "claimed" before being treated as abandoned +// by a crashed worker and requeued to "pending." +const DEFAULT_CLAIM_STALE_MS = 5 * 60 * 1000; + +/** + * Atomically claims one pending event, oldest first. Returns null if + * nothing is pending. + */ +async function claimNextEvent() { + return ActionEvent.findOneAndUpdate( + { status: "pending" }, + { $set: { status: "claimed" } }, + { sort: { occurredAt: 1 }, new: true } + ); +} + +/** + * Processes a single claimed event through the rules engine and records + * the outcome on the ActionEvent itself. Every outcome except "awarded"/ + * "duplicate" is still marked "processed" — a missing rule, an inactive + * rule, a cooldown, or a daily cap are all legitimate, expected reasons + * an event doesn't pay out, not failures. Only an actual thrown error + * (a bad DB write, a schema violation) is marked "failed." + */ +async function processClaimedEvent(event) { + try { + const result = await processEvent(event); + await ActionEvent.updateOne( + { _id: event._id }, + { $set: { status: "processed", processedAt: new Date() } } + ); + return result; + } catch (err) { + await ActionEvent.updateOne( + { _id: event._id }, + { + $set: { + status: "failed", + processedAt: new Date(), + failureReason: (err && err.message) || String(err), + }, + } + ); + return { outcome: "error", eventId: event.eventId, actionKey: event.actionKey, error: err }; + } +} + +/** + * Drains up to `batchSize` pending events in one pass. Returns the list + * of per-event outcomes. Used directly by tests and by runOnce() below; + * exported separately so a caller (e.g. an HTTP admin trigger, or the + * backfill) can drive processing without needing the interval timer. + */ +async function drainBatch(batchSize = DEFAULT_BATCH_SIZE) { + const results = []; + for (let i = 0; i < batchSize; i++) { + const event = await claimNextEvent(); + if (!event) break; + results.push(await processClaimedEvent(event)); + } + return results; +} + +/** + * Requeues events stuck in "claimed" past claimStaleMs — the recovery + * path for a worker that crashed mid-processing. Safe to call + * concurrently with drainBatch(): a requeue only ever moves a stale + * "claimed" event back to "pending" for someone to claim again; it never + * touches "processed"/"failed" events. + */ +async function requeueStuckClaims(claimStaleMs = DEFAULT_CLAIM_STALE_MS) { + const staleBefore = new Date(Date.now() - claimStaleMs); + const result = await ActionEvent.updateMany( + { status: "claimed", updatedAt: { $lt: staleBefore } }, + { $set: { status: "pending" } } + ); + return result.modifiedCount || 0; +} + +let pollTimer = null; + +/** + * Starts the polling loop. Call once at server boot (see server.js). + * Idempotent — calling start() while already running is a no-op rather + * than stacking timers. + */ +function start({ pollIntervalMs = DEFAULT_POLL_INTERVAL_MS, batchSize = DEFAULT_BATCH_SIZE } = {}) { + if (pollTimer) return; + + const tick = async () => { + try { + await requeueStuckClaims(); + await drainBatch(batchSize); + } catch (err) { + console.error("currencyConsumer: poll tick failed:", err.message); + } + }; + + pollTimer = setInterval(tick, pollIntervalMs); + // Fire once immediately rather than waiting a full interval on boot. + tick(); +} + +function stop() { + if (pollTimer) { + clearInterval(pollTimer); + pollTimer = null; + } +} + +module.exports = { + claimNextEvent, + processClaimedEvent, + drainBatch, + requeueStuckClaims, + start, + stop, +}; diff --git a/middlewareNode/src/services/ledgerService.js b/middlewareNode/src/services/ledgerService.js new file mode 100644 index 00000000..a9d5dbb9 --- /dev/null +++ b/middlewareNode/src/services/ledgerService.js @@ -0,0 +1,201 @@ +/** + * Ledger Service + * + * The rules engine + idempotent ledger writer at the center of the + * currency pipeline. Turns one ActionEvent into, at most, one LedgerEntry + * plus a matching UserBalance update — or determines the event should not + * pay out, and says why. + * + * Deliberately framework-agnostic about *how* an ActionEvent got here: the + * live polling consumer (currencyConsumer.js) and the historical backfill + * (both Week 2+ deliverables) call the same processEvent() so a bug fixed + * here fixes both paths, and the backfill genuinely exercises this code + * rather than a separate parallel implementation. + * + * No MongoDB transaction here by default — see the file-level comment on + * why below applyLedgerEntry. Set LEDGER_USE_TRANSACTIONS=true once Week 0's + * replica-set gate (`rs.status()` in mongosh) confirms one is available. + */ + +const mongoose = require("mongoose"); +const ActionRule = require("../models/actionRule"); +const LedgerEntry = require("../models/ledgerEntry"); +const UserBalance = require("../models/userBalance"); + +const DUPLICATE_KEY_ERROR = 11000; + +/** + * Reads a user's lifetimeEarned, treating "no UserBalance document yet" + * as 0 rather than undefined. Used by the leaderboard swap (Jimmy's lane) + * and anywhere else that needs a safe-to-sort number for every user, + * including ones who have never earned anything. + */ +async function getLifetimeEarnedOrZero(userId) { + const doc = await UserBalance.findOne({ userId }, { lifetimeEarned: 1, _id: 0 }); + return doc ? doc.lifetimeEarned : 0; +} + +/** + * Cooldown check: has this user actually been PAID for this action within + * the rule's cooldownSeconds? A cooldownSeconds of 0 means "no cooldown" + * and this always returns false without querying. + * + * Deliberately checks LedgerEntry, not ActionEvent.status — whether an + * award happened is exactly what LedgerEntry records, and status is a + * bookkeeping concern owned by whoever is driving events through this + * function (the live consumer marks "processed"; the historical backfill + * may not use the same lifecycle at all). Checking the ledger keeps + * processEvent() correct regardless of caller. + */ +async function isWithinCooldown(LedgerEntry, userId, actionKey, cooldownSeconds, asOf) { + if (!cooldownSeconds) return false; + const since = new Date(asOf.getTime() - cooldownSeconds * 1000); + const recent = await LedgerEntry.findOne({ + userId, + actionKey, + occurredAt: { $gte: since, $lt: asOf }, + }); + return Boolean(recent); +} + +/** + * Daily-cap check: has this user already been paid dailyCap times for + * this action today (UTC calendar day, matching how other daily windows + * in this codebase are computed)? A dailyCap of 0 means "no cap." Checks + * LedgerEntry for the same reason isWithinCooldown does. + */ +async function hasHitDailyCap(LedgerEntry, userId, actionKey, dailyCap, asOf) { + if (!dailyCap) return false; + const dayStart = new Date(Date.UTC(asOf.getUTCFullYear(), asOf.getUTCMonth(), asOf.getUTCDate())); + const dayEnd = new Date(dayStart.getTime() + 24 * 60 * 60 * 1000); + const countToday = await LedgerEntry.countDocuments({ + userId, + actionKey, + occurredAt: { $gte: dayStart, $lt: dayEnd }, + }); + return countToday >= dailyCap; +} + +/** + * Writes one LedgerEntry and updates the matching UserBalance for a + * validated, rule-approved award. Idempotent on eventId: a duplicate + * insert is caught and treated as "already applied," not an error — + * exactly the pattern gameResults.js uses for gameId (see that file's + * `err.code === 11000` handling). + * + * Not wrapped in a MongoDB multi-document transaction by default. + * Standalone MongoDB instances (the common case for this project's dev/ + * staging setups) don't support them at all, and Week 0's plan explicitly + * gates this: run `rs.status()` and report back before assuming one is + * available. Until that's confirmed, this uses the single-document + * fallback the plan calls out — write the ledger entry first (the correct + * source of truth), then update the cache; if the process dies between + * the two, UserBalance is rebuildable from LedgerEntry, so the cache is + * merely stale, never wrong. Set LEDGER_USE_TRANSACTIONS=true to switch + * to a real transaction once a replica set is confirmed. + */ +async function applyLedgerEntry({ eventId, userId, actionKey, amount, occurredAt }) { + const useTransactions = process.env.LEDGER_USE_TRANSACTIONS === "true"; + + if (useTransactions) { + const session = await mongoose.startSession(); + try { + let duplicate = false; + await session.withTransaction(async () => { + try { + await LedgerEntry.create([{ eventId, userId, actionKey, amount, occurredAt }], { session }); + } catch (err) { + if (err && err.code === DUPLICATE_KEY_ERROR) { + duplicate = true; + return; + } + throw err; + } + await UserBalance.updateOne( + { userId }, + { $inc: { balance: amount, lifetimeEarned: amount } }, + { upsert: true, session } + ); + }); + return { applied: !duplicate, duplicate }; + } finally { + await session.endSession(); + } + } + + // Single-document fallback (default): ledger entry first, then cache. + try { + await LedgerEntry.create({ eventId, userId, actionKey, amount, occurredAt }); + } catch (err) { + if (err && err.code === DUPLICATE_KEY_ERROR) { + return { applied: false, duplicate: true }; + } + throw err; + } + + await UserBalance.updateOne( + { userId }, + { $inc: { balance: amount, lifetimeEarned: amount } }, + { upsert: true } + ); + + return { applied: true, duplicate: false }; +} + +/** + * Processes one ActionEvent end to end against the rules engine: look up + * the ActionRule, check active/cooldown/dailyCap, and write the ledger + * entry if everything clears. Returns a result object describing exactly + * what happened, for the caller (consumer or backfill) to log and use to + * set ActionEvent.status. + * + * Does NOT mutate the ActionEvent itself — callers own that, since the + * live consumer and the backfill replay have different ideas about what + * "processed" should mean for a historical vs. a live event. + * + * @param {object} event - a plain object shaped like an ActionEvent + * ({ eventId, userId, actionKey, occurredAt, metadata }), so this can be + * called with either a real Mongoose document or a synthetic backfill + * record without either needing to be a full ActionEvent. + */ +async function processEvent(event) { + const { eventId, userId, actionKey, occurredAt } = event; + + const rule = await ActionRule.findOne({ actionKey }); + if (!rule) { + return { outcome: "no_rule", eventId, actionKey }; + } + if (!rule.active) { + return { outcome: "rule_inactive", eventId, actionKey }; + } + + const asOf = occurredAt instanceof Date ? occurredAt : new Date(occurredAt); + + if (await isWithinCooldown(LedgerEntry, userId, actionKey, rule.cooldownSeconds, asOf)) { + return { outcome: "cooldown", eventId, actionKey }; + } + if (await hasHitDailyCap(LedgerEntry, userId, actionKey, rule.dailyCap, asOf)) { + return { outcome: "daily_cap", eventId, actionKey }; + } + + const { applied, duplicate } = await applyLedgerEntry({ + eventId, + userId, + actionKey, + amount: rule.currencyAmount, + occurredAt: asOf, + }); + + if (duplicate) { + return { outcome: "duplicate", eventId, actionKey }; + } + return { outcome: "awarded", eventId, actionKey, amount: rule.currencyAmount }; +} + +module.exports = { + processEvent, + applyLedgerEntry, + getLifetimeEarnedOrZero, + isWithinCooldown, + hasHitDailyCap, +}; diff --git a/middlewareNode/tests/backfillCurrencyLedger.test.js b/middlewareNode/tests/backfillCurrencyLedger.test.js new file mode 100644 index 00000000..6d7ea8f3 --- /dev/null +++ b/middlewareNode/tests/backfillCurrencyLedger.test.js @@ -0,0 +1,270 @@ +/** + * Real-database tests for scripts/backfillCurrencyLedger.js — the + * historical backfill that synthesizes ActionEvents from existing + * GameResults and replays them through the real currency consumer. + * + * Uses mongodb-memory-server (see tests/ledgerService.test.js for why): + * idempotency-on-rerun and the actual replay-through-the-consumer step + * are both real-database properties, not something a mock demonstrates. + */ + +const { MongoMemoryServer } = require("mongodb-memory-server"); +const mongoose = require("mongoose"); + +jest.setTimeout(60000); + +let mongod; + +beforeAll(async () => { + mongod = await MongoMemoryServer.create({ instance: { launchTimeout: 30000 } }); + await mongoose.connect(mongod.getUri() + "ystem"); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongod.stop(); +}); + +afterEach(async () => { + const collections = mongoose.connection.collections; + await Promise.all(Object.values(collections).map((c) => c.deleteMany({}))); +}); + +const GameResults = require("../src/models/gameResults"); +const ActionEvent = require("../src/models/actionEvent"); +const ActionRule = require("../src/models/actionRule"); +const LedgerEntry = require("../src/models/ledgerEntry"); +const UserBalance = require("../src/models/userBalance"); +const { + backfillEventId, + ensureGamePlayedRule, + synthesizeGameEvents, + replayPendingEvents, +} = require("../src/scripts/backfillCurrencyLedger"); + +async function makeUser(username) { + const usersCollection = mongoose.connection.collection("users"); + const result = await usersCollection.insertOne({ username, role: "student" }); + return result.insertedId; +} + +describe("ensureGamePlayedRule", () => { + it("creates a default rule when none exists", async () => { + const rule = await ensureGamePlayedRule({ dryRun: false }); + expect(rule.actionKey).toBe("game.played"); + expect(rule.currencyAmount).toBeGreaterThan(0); + expect(await ActionRule.countDocuments({})).toBe(1); + }); + + it("leaves an existing rule untouched — never overrides a deliberately configured amount", async () => { + await ActionRule.create({ actionKey: "game.played", currencyAmount: 42, active: true }); + const rule = await ensureGamePlayedRule({ dryRun: false }); + expect(rule.currencyAmount).toBe(42); + expect(await ActionRule.countDocuments({})).toBe(1); + }); + + it("does not write anything in dry-run mode when no rule exists", async () => { + await ensureGamePlayedRule({ dryRun: true }); + expect(await ActionRule.countDocuments({})).toBe(0); + }); +}); + +describe("synthesizeGameEvents", () => { + it("creates one ActionEvent per player for each finished game", async () => { + await GameResults.create({ + gameId: "game-1", + players: ["alice", "bob"], + result: "win", + winnerUsername: "alice", + loserUsername: "bob", + reason: "checkmate", + playedAt: new Date("2026-01-01"), + }); + await makeUser("alice"); + await makeUser("bob"); + + const usersCollection = mongoose.connection.collection("users"); + const result = await synthesizeGameEvents({ dryRun: false, usersCollection }); + + expect(result.gamesScanned).toBe(1); + expect(result.eventsInserted).toBe(2); + expect(await ActionEvent.countDocuments({ actionKey: "game.played" })).toBe(2); + + const aliceEvent = await ActionEvent.findOne({ eventId: backfillEventId("game-1", "alice") }); + expect(aliceEvent).not.toBeNull(); + expect(aliceEvent.status).toBe("pending"); + expect(aliceEvent.occurredAt).toEqual(new Date("2026-01-01")); + }); + + it("is idempotent — re-running produces no new events for already-backfilled games", async () => { + await GameResults.create({ + gameId: "game-1", + players: ["alice", "bob"], + result: "draw", + reason: "draw", + playedAt: new Date("2026-01-01"), + }); + await makeUser("alice"); + await makeUser("bob"); + const usersCollection = mongoose.connection.collection("users"); + + await synthesizeGameEvents({ dryRun: false, usersCollection }); + const second = await synthesizeGameEvents({ dryRun: false, usersCollection }); + + expect(second.eventsInserted).toBe(0); + expect(second.eventsSkippedDuplicate).toBe(2); + expect(await ActionEvent.countDocuments({})).toBe(2); + }); + + it("skips a player whose username no longer resolves to a real user, without crashing", async () => { + await GameResults.create({ + gameId: "game-1", + players: ["alice", "ghost-user"], + result: "win", + winnerUsername: "alice", + loserUsername: "ghost-user", + reason: "resign", + playedAt: new Date("2026-01-01"), + }); + await makeUser("alice"); + const usersCollection = mongoose.connection.collection("users"); + + const result = await synthesizeGameEvents({ dryRun: false, usersCollection }); + + expect(result.eventsInserted).toBe(1); + expect(result.eventsSkippedUnresolvedUser).toBe(1); + expect(result.unresolvedUsernames).toEqual(["ghost-user"]); + }); + + it("does not write anything in dry-run mode, but still reports accurate counts", async () => { + await GameResults.create({ + gameId: "game-1", + players: ["alice", "bob"], + result: "win", + winnerUsername: "alice", + loserUsername: "bob", + reason: "checkmate", + playedAt: new Date("2026-01-01"), + }); + await makeUser("alice"); + await makeUser("bob"); + const usersCollection = mongoose.connection.collection("users"); + + const result = await synthesizeGameEvents({ dryRun: true, usersCollection }); + + expect(result.eventsInserted).toBe(2); + expect(await ActionEvent.countDocuments({})).toBe(0); // nothing actually written + }); +}); + +describe("replayPendingEvents", () => { + it("processes every pending event through the real consumer and pays out lifetimeEarned", async () => { + await ActionRule.create({ actionKey: "game.played", currencyAmount: 10, active: true }); + const aliceId = await makeUser("alice"); + const bobId = await makeUser("bob"); + + const usersCollection = mongoose.connection.collection("users"); + await GameResults.create({ + gameId: "game-1", + players: ["alice", "bob"], + result: "win", + winnerUsername: "alice", + loserUsername: "bob", + reason: "checkmate", + playedAt: new Date("2026-01-01"), + }); + await synthesizeGameEvents({ dryRun: false, usersCollection }); + + const replay = await replayPendingEvents({ dryRun: false, batchSize: 50 }); + + expect(replay.totalProcessed).toBe(2); + expect(replay.outcomeCounts.awarded).toBe(2); + expect(await ActionEvent.countDocuments({ status: "pending" })).toBe(0); + + const aliceBalance = await UserBalance.findOne({ userId: aliceId }); + const bobBalance = await UserBalance.findOne({ userId: bobId }); + expect(aliceBalance.lifetimeEarned).toBe(10); + expect(bobBalance.lifetimeEarned).toBe(10); + }); + + it("drains in multiple batches when there are more pending events than batchSize", async () => { + await ActionRule.create({ actionKey: "game.played", currencyAmount: 1, active: true }); + const usersCollection = mongoose.connection.collection("users"); + await makeUser("alice"); + await makeUser("bob"); + + for (let i = 0; i < 5; i++) { + await GameResults.create({ + gameId: `game-${i}`, + players: ["alice", "bob"], + result: "draw", + reason: "draw", + playedAt: new Date(`2026-01-0${i + 1}`), + }); + } + await synthesizeGameEvents({ dryRun: false, usersCollection }); + + const replay = await replayPendingEvents({ dryRun: false, batchSize: 3 }); + + expect(replay.totalProcessed).toBe(10); // 5 games * 2 players + expect(await ActionEvent.countDocuments({ status: "pending" })).toBe(0); + }); + + it("does not process anything in dry-run mode", async () => { + await ActionRule.create({ actionKey: "game.played", currencyAmount: 10, active: true }); + const usersCollection = mongoose.connection.collection("users"); + await makeUser("alice"); + await makeUser("bob"); + await GameResults.create({ + gameId: "game-1", + players: ["alice", "bob"], + result: "win", + winnerUsername: "alice", + loserUsername: "bob", + reason: "checkmate", + playedAt: new Date("2026-01-01"), + }); + await synthesizeGameEvents({ dryRun: false, usersCollection }); + + const replay = await replayPendingEvents({ dryRun: true }); + + expect(replay.totalProcessed).toBe(0); + expect(await ActionEvent.countDocuments({ status: "pending" })).toBe(2); // untouched + expect(await LedgerEntry.countDocuments({})).toBe(0); + }); +}); + +describe("end-to-end backfill idempotency (the property the plan cares most about)", () => { + it("running synthesize + replay twice never double-pays a student", async () => { + await makeUser("alice"); + await makeUser("bob"); + const usersCollection = mongoose.connection.collection("users"); + + await GameResults.create({ + gameId: "game-1", + players: ["alice", "bob"], + result: "win", + winnerUsername: "alice", + loserUsername: "bob", + reason: "checkmate", + playedAt: new Date("2026-01-01"), + }); + + await ensureGamePlayedRule({ dryRun: false }); + await synthesizeGameEvents({ dryRun: false, usersCollection }); + await replayPendingEvents({ dryRun: false }); + + // Re-run the whole pipeline exactly as a second invocation of the + // script would. + await ensureGamePlayedRule({ dryRun: false }); + await synthesizeGameEvents({ dryRun: false, usersCollection }); + await replayPendingEvents({ dryRun: false }); + + const aliceId = (await usersCollection.findOne({ username: "alice" }))._id; + const balance = await UserBalance.findOne({ userId: aliceId }); + const rule = await ActionRule.findOne({ actionKey: "game.played" }); + + expect(balance.lifetimeEarned).toBe(rule.currencyAmount); // not double + expect(await LedgerEntry.countDocuments({ userId: aliceId })).toBe(1); + }); +}); diff --git a/middlewareNode/tests/currencyAnomalyScheduler.test.js b/middlewareNode/tests/currencyAnomalyScheduler.test.js new file mode 100644 index 00000000..b9012992 --- /dev/null +++ b/middlewareNode/tests/currencyAnomalyScheduler.test.js @@ -0,0 +1,162 @@ +/** + * Real-database tests for scheduler/currencyAnomalyScheduler.js — the + * flag-only anomaly monitor from Karthik's ledger plan Week 3 hardening. + * + * Uses mongodb-memory-server (see tests/ledgerService.test.js for why): + * the $group aggregation over LedgerEntry is exactly the kind of query a + * mock can't meaningfully stand in for. + */ + +const { MongoMemoryServer } = require("mongodb-memory-server"); +const mongoose = require("mongoose"); + +jest.setTimeout(60000); + +let mongod; + +beforeAll(async () => { + mongod = await MongoMemoryServer.create({ instance: { launchTimeout: 30000 } }); + await mongoose.connect(mongod.getUri() + "ystem"); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongod.stop(); +}); + +afterEach(async () => { + const collections = mongoose.connection.collections; + await Promise.all(Object.values(collections).map((c) => c.deleteMany({}))); +}); + +const LedgerEntry = require("../src/models/ledgerEntry"); +const { detectAnomalies, computeEarnedTotals, median } = require("../src/scheduler/currencyAnomalyScheduler"); + +function id() { + return new mongoose.Types.ObjectId(); +} + +async function makeEntry(userId, amount, occurredAt) { + return LedgerEntry.create({ + eventId: `evt-${Math.random().toString(36).slice(2)}`, + userId, + actionKey: "lesson.completed", + amount, + occurredAt, + }); +} + +describe("median", () => { + it("returns the middle value for an odd-length sorted array", () => { + expect(median([1, 3, 5])).toBe(3); + }); + + it("averages the two middle values for an even-length sorted array", () => { + expect(median([1, 2, 3, 4])).toBe(2.5); + }); +}); + +describe("computeEarnedTotals", () => { + it("sums only positive entries within the window, per user", async () => { + const alice = id(); + const bob = id(); + const now = new Date("2026-06-15T12:00:00Z"); + + await makeEntry(alice, 10, new Date("2026-06-15T10:00:00Z")); + await makeEntry(alice, 5, new Date("2026-06-15T11:00:00Z")); + await makeEntry(bob, 20, new Date("2026-06-15T09:00:00Z")); + // Outside the 24h window — should not count. + await makeEntry(bob, 999, new Date("2026-06-10T00:00:00Z")); + + const totals = await computeEarnedTotals(new Date(now.getTime() - 24 * 60 * 60 * 1000)); + + expect(totals.get(String(alice))).toBe(15); + expect(totals.get(String(bob))).toBe(20); + }); +}); + +describe("detectAnomalies", () => { + it("skips detection when there are too few earners in the window", async () => { + for (let i = 0; i < 3; i++) { + await makeEntry(id(), 10, new Date()); + } + const result = await detectAnomalies({ persist: false }); + expect(result.skipped).toBe(true); + expect(result.flagged).toHaveLength(0); + }); + + it("flags no one when everyone earns roughly the same amount", async () => { + for (let i = 0; i < 10; i++) { + await makeEntry(id(), 10, new Date()); + } + const result = await detectAnomalies({ persist: false }); + expect(result.flagged).toHaveLength(0); + }); + + it("flags a user whose 24h total is a clear outlier from the population", async () => { + // Nine normal earners around 10, one wildly above everyone else. + const normalUsers = Array.from({ length: 9 }, () => id()); + for (const u of normalUsers) { + await makeEntry(u, 10, new Date()); + } + const outlier = id(); + await makeEntry(outlier, 5000, new Date()); + + const result = await detectAnomalies({ persist: false }); + + expect(result.flagged).toHaveLength(1); + expect(result.flagged[0].userId).toBe(String(outlier)); + expect(result.flagged[0].totalEarned24h).toBe(5000); + }); + + it("does not flag ordinary variance within a normal-looking population", async () => { + const amounts = [8, 9, 10, 10, 11, 12, 9, 10, 11, 10]; + for (const amount of amounts) { + await makeEntry(id(), amount, new Date()); + } + const result = await detectAnomalies({ persist: false }); + expect(result.flagged).toHaveLength(0); + }); + + it("handles a zero-deviation population (everyone identical) without dividing by zero", async () => { + for (let i = 0; i < 6; i++) { + await makeEntry(id(), 10, new Date()); + } + const result = await detectAnomalies({ persist: false }); + expect(result.skipped).toBe(true); + expect(result.reason).toMatch(/zero deviation/); + }); + + it("persists flags to currencyAnomalyFlags when persist is true (the default)", async () => { + const normalUsers = Array.from({ length: 9 }, () => id()); + for (const u of normalUsers) { + await makeEntry(u, 10, new Date()); + } + const outlier = id(); + await makeEntry(outlier, 5000, new Date()); + + await detectAnomalies(); + + const stored = await mongoose.connection.db.collection("currencyAnomalyFlags").find({}).toArray(); + expect(stored).toHaveLength(1); + expect(stored[0].userId.toString()).toBe(String(outlier)); + expect(stored[0].totalEarned24h).toBe(5000); + }); + + it("never mutates a LedgerEntry or UserBalance — flag-only, per the plan", async () => { + const normalUsers = Array.from({ length: 9 }, () => id()); + for (const u of normalUsers) { + await makeEntry(u, 10, new Date()); + } + const outlier = id(); + await makeEntry(outlier, 5000, new Date()); + + const beforeCount = await LedgerEntry.countDocuments({}); + await detectAnomalies(); + const afterCount = await LedgerEntry.countDocuments({}); + + expect(afterCount).toBe(beforeCount); + const UserBalance = require("../src/models/userBalance"); + expect(await UserBalance.countDocuments({})).toBe(0); // detectAnomalies never touches balances + }); +}); diff --git a/middlewareNode/tests/currencyConsumer.test.js b/middlewareNode/tests/currencyConsumer.test.js new file mode 100644 index 00000000..52f824c2 --- /dev/null +++ b/middlewareNode/tests/currencyConsumer.test.js @@ -0,0 +1,189 @@ +/** + * Real-database tests for services/currencyConsumer.js — the polling + * worker that claims ActionEvent documents and runs them through the + * ledger service (Rev. 2, "Drop Redis from this window": a plain Mongo + * collection + polling worker in place of Redis Streams + consumer + * group). + * + * Uses mongodb-memory-server (see tests/ledgerService.test.js for why — + * this file is specifically about the atomicity of claimNextEvent() + * under concurrency, which a mock can't demonstrate). + */ + +const { MongoMemoryServer } = require("mongodb-memory-server"); +const mongoose = require("mongoose"); + +jest.setTimeout(60000); + +let mongod; + +beforeAll(async () => { + mongod = await MongoMemoryServer.create({ instance: { launchTimeout: 30000 } }); + await mongoose.connect(mongod.getUri() + "ystem"); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongod.stop(); +}); + +afterEach(async () => { + const collections = mongoose.connection.collections; + await Promise.all(Object.values(collections).map((c) => c.deleteMany({}))); +}); + +const ActionRule = require("../src/models/actionRule"); +const ActionEvent = require("../src/models/actionEvent"); +const LedgerEntry = require("../src/models/ledgerEntry"); +const { + claimNextEvent, + drainBatch, + requeueStuckClaims, +} = require("../src/services/currencyConsumer"); + +const userId = new mongoose.Types.ObjectId(); + +async function makeEvent(overrides = {}) { + return ActionEvent.create({ + eventId: `evt-${Math.random().toString(36).slice(2)}`, + actionKey: "lesson.completed", + userId, + occurredAt: new Date(), + ...overrides, + }); +} + +describe("claimNextEvent", () => { + it("returns null when nothing is pending", async () => { + expect(await claimNextEvent()).toBeNull(); + }); + + it("claims the oldest pending event and marks it 'claimed'", async () => { + const older = await makeEvent({ occurredAt: new Date("2026-01-01") }); + await makeEvent({ occurredAt: new Date("2026-06-01") }); + + const claimed = await claimNextEvent(); + expect(claimed.eventId).toBe(older.eventId); + expect(claimed.status).toBe("claimed"); + }); + + it("never lets two concurrent claims take the same event (atomic claim)", async () => { + await makeEvent(); + + const [a, b] = await Promise.all([claimNextEvent(), claimNextEvent()]); + const claimedResults = [a, b].filter(Boolean); + + // Exactly one of the two racing claims should have gotten the event; + // the other must see nothing pending left. + expect(claimedResults).toHaveLength(1); + }); +}); + +describe("drainBatch", () => { + it("processes all pending events up to batchSize, awarding per the active rule", async () => { + await ActionRule.create({ actionKey: "lesson.completed", currencyAmount: 10, active: true }); + await makeEvent(); + await makeEvent(); + await makeEvent(); + + const results = await drainBatch(10); + + expect(results).toHaveLength(3); + expect(results.every((r) => r.outcome === "awarded")).toBe(true); + expect(await LedgerEntry.countDocuments({})).toBe(3); + expect(await ActionEvent.countDocuments({ status: "processed" })).toBe(3); + }); + + it("respects batchSize and leaves the rest pending for the next call", async () => { + await ActionRule.create({ actionKey: "lesson.completed", currencyAmount: 5, active: true }); + await makeEvent(); + await makeEvent(); + await makeEvent(); + + const firstBatch = await drainBatch(2); + expect(firstBatch).toHaveLength(2); + expect(await ActionEvent.countDocuments({ status: "pending" })).toBe(1); + + const secondBatch = await drainBatch(2); + expect(secondBatch).toHaveLength(1); + expect(await ActionEvent.countDocuments({ status: "pending" })).toBe(0); + }); + + it("marks an event 'failed' with a reason, not silently dropped, when processing throws", async () => { + // Force a genuine runtime failure rather than mocking processEvent: + // insert an ActionRule via the raw driver to bypass Mongoose's own + // schema validation/casting, giving it a non-numeric currencyAmount. + // processEvent reaches applyLedgerEntry with that value, and + // LedgerEntry's own schema (amount: Number) genuinely rejects the + // write with a real CastError — the same class of failure a bad + // downstream write would produce in production. + await ActionRule.collection.insertOne({ + actionKey: "lesson.completed", + currencyAmount: "not-a-number", + active: true, + cooldownSeconds: 0, + dailyCap: 0, + createdAt: new Date(), + updatedAt: new Date(), + }); + const event = await makeEvent({ actionKey: "lesson.completed" }); + + const claimed = await claimNextEvent(); + expect(claimed._id.toString()).toBe(event._id.toString()); + + const { processClaimedEvent } = require("../src/services/currencyConsumer"); + const result = await processClaimedEvent(claimed); + + expect(result.outcome).toBe("error"); + const stored = await ActionEvent.findById(claimed._id); + expect(stored.status).toBe("failed"); + expect(stored.failureReason).toBeTruthy(); + }); +}); + +describe("requeueStuckClaims", () => { + it("requeues a claimed event older than the stale threshold back to pending", async () => { + const event = await makeEvent(); + // Mongoose's `timestamps: true` re-stamps updatedAt to "now" on every + // .updateOne() call, so backdating it has to go through the raw + // driver — a Mongoose-level update here would silently make this + // event look freshly claimed and the test would falsely pass either way. + await ActionEvent.collection.updateOne( + { _id: event._id }, + { $set: { status: "claimed", updatedAt: new Date(Date.now() - 10 * 60 * 1000) } } + ); + + const requeued = await requeueStuckClaims(5 * 60 * 1000); + expect(requeued).toBe(1); + + const stored = await ActionEvent.findById(event._id); + expect(stored.status).toBe("pending"); + }); + + it("does not touch a recently claimed event still within the stale threshold", async () => { + const event = await makeEvent(); + await ActionEvent.updateOne({ _id: event._id }, { $set: { status: "claimed" } }); + + const requeued = await requeueStuckClaims(5 * 60 * 1000); + expect(requeued).toBe(0); + + const stored = await ActionEvent.findById(event._id); + expect(stored.status).toBe("claimed"); + }); + + it("never touches an already-processed event", async () => { + const event = await makeEvent(); + // Raw driver again — see the note in the first test in this + // describe() block on why a Mongoose .updateOne() won't backdate + // updatedAt. + await ActionEvent.collection.updateOne( + { _id: event._id }, + { $set: { status: "processed", updatedAt: new Date(Date.now() - 10 * 60 * 1000) } } + ); + + await requeueStuckClaims(5 * 60 * 1000); + + const stored = await ActionEvent.findById(event._id); + expect(stored.status).toBe("processed"); + }); +}); diff --git a/middlewareNode/tests/ledgerService.test.js b/middlewareNode/tests/ledgerService.test.js new file mode 100644 index 00000000..cb757004 --- /dev/null +++ b/middlewareNode/tests/ledgerService.test.js @@ -0,0 +1,279 @@ +/** + * Real-database tests for services/ledgerService.js — the currency + * rollout's rules engine and idempotent ledger writer. + * + * Uses mongodb-memory-server + real Mongoose models (no mocks), matching + * the pattern in tests/badges.concurrency.test.js, because the property + * this file exists to prove — "a duplicate eventId can never double-pay" + * — is a real unique-index guarantee, not something a mock can stand in + * for. A mocked model would happily "insert" the same eventId twice. + */ + +const { MongoMemoryServer } = require("mongodb-memory-server"); +const mongoose = require("mongoose"); + +jest.setTimeout(60000); + +let mongod; + +beforeAll(async () => { + mongod = await MongoMemoryServer.create({ instance: { launchTimeout: 30000 } }); + await mongoose.connect(mongod.getUri() + "ystem"); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongod.stop(); +}); + +afterEach(async () => { + const collections = mongoose.connection.collections; + await Promise.all(Object.values(collections).map((c) => c.deleteMany({}))); +}); + +const ActionRule = require("../src/models/actionRule"); +const ActionEvent = require("../src/models/actionEvent"); +const LedgerEntry = require("../src/models/ledgerEntry"); +const UserBalance = require("../src/models/userBalance"); +const { + processEvent, + applyLedgerEntry, + getLifetimeEarnedOrZero, +} = require("../src/services/ledgerService"); + +const userId = new mongoose.Types.ObjectId(); + +async function makeRule(overrides = {}) { + return ActionRule.create({ + actionKey: "lesson.completed", + currencyAmount: 10, + active: true, + cooldownSeconds: 0, + dailyCap: 0, + ...overrides, + }); +} + +describe("getLifetimeEarnedOrZero", () => { + it("returns 0 for a user with no UserBalance document, not undefined", async () => { + const result = await getLifetimeEarnedOrZero(new mongoose.Types.ObjectId()); + expect(result).toBe(0); + }); + + it("returns the real lifetimeEarned once a document exists", async () => { + await UserBalance.create({ userId, balance: 5, lifetimeEarned: 5 }); + const result = await getLifetimeEarnedOrZero(userId); + expect(result).toBe(5); + }); +}); + +describe("applyLedgerEntry idempotency", () => { + it("writes exactly one LedgerEntry and one UserBalance update for a new eventId", async () => { + const result = await applyLedgerEntry({ + eventId: "evt-1", + userId, + actionKey: "lesson.completed", + amount: 10, + occurredAt: new Date(), + }); + + expect(result).toEqual({ applied: true, duplicate: false }); + + const entries = await LedgerEntry.find({ userId }); + expect(entries).toHaveLength(1); + expect(entries[0].amount).toBe(10); + + const balance = await UserBalance.findOne({ userId }); + expect(balance.balance).toBe(10); + expect(balance.lifetimeEarned).toBe(10); + }); + + it("is a no-op on a duplicate eventId — never double-pays", async () => { + const args = { + eventId: "evt-dup", + userId, + actionKey: "lesson.completed", + amount: 10, + occurredAt: new Date(), + }; + + const first = await applyLedgerEntry(args); + const second = await applyLedgerEntry(args); + + expect(first).toEqual({ applied: true, duplicate: false }); + expect(second).toEqual({ applied: false, duplicate: true }); + + const entries = await LedgerEntry.find({ userId, eventId: "evt-dup" }); + expect(entries).toHaveLength(1); + + const balance = await UserBalance.findOne({ userId }); + expect(balance.lifetimeEarned).toBe(10); // not 20 + }); + + it("survives concurrent duplicate inserts of the same eventId (real retry, not just a unique-index assumption)", async () => { + const args = { + eventId: "evt-race", + userId, + actionKey: "lesson.completed", + amount: 10, + occurredAt: new Date(), + }; + + // Fire the same event twice "at once" — this is the scenario a + // reconnect/retry actually produces, not just a sequential re-call. + const [a, b] = await Promise.all([applyLedgerEntry(args), applyLedgerEntry(args)]); + const outcomes = [a.duplicate, b.duplicate].sort(); + + expect(outcomes).toEqual([false, true]); // exactly one applied, one duplicate + + const entries = await LedgerEntry.find({ userId, eventId: "evt-race" }); + expect(entries).toHaveLength(1); + }); + + it("accumulates balance and lifetimeEarned together across distinct events", async () => { + await applyLedgerEntry({ eventId: "evt-a", userId, actionKey: "lesson.completed", amount: 10, occurredAt: new Date() }); + await applyLedgerEntry({ eventId: "evt-b", userId, actionKey: "puzzle.solved", amount: 5, occurredAt: new Date() }); + + const balance = await UserBalance.findOne({ userId }); + expect(balance.balance).toBe(15); + expect(balance.lifetimeEarned).toBe(15); + }); +}); + +describe("processEvent — rules engine", () => { + it("returns no_rule when no ActionRule exists for the action", async () => { + const result = await processEvent({ + eventId: "evt-1", + userId, + actionKey: "unknown.action", + occurredAt: new Date(), + }); + expect(result.outcome).toBe("no_rule"); + + const entries = await LedgerEntry.find({}); + expect(entries).toHaveLength(0); + }); + + it("returns rule_inactive and writes nothing when the rule is disabled", async () => { + await makeRule({ active: false }); + const result = await processEvent({ + eventId: "evt-1", + userId, + actionKey: "lesson.completed", + occurredAt: new Date(), + }); + expect(result.outcome).toBe("rule_inactive"); + expect(await LedgerEntry.countDocuments({})).toBe(0); + }); + + it("awards currency and returns the amount when a rule is active and clear", async () => { + await makeRule({ currencyAmount: 25 }); + const result = await processEvent({ + eventId: "evt-1", + userId, + actionKey: "lesson.completed", + occurredAt: new Date(), + }); + expect(result).toMatchObject({ outcome: "awarded", amount: 25 }); + + const balance = await UserBalance.findOne({ userId }); + expect(balance.lifetimeEarned).toBe(25); + }); + + it("returns duplicate (not awarded) on a re-processed eventId", async () => { + await makeRule(); + const event = { eventId: "evt-1", userId, actionKey: "lesson.completed", occurredAt: new Date() }; + + await processEvent(event); + const second = await processEvent(event); + + expect(second.outcome).toBe("duplicate"); + expect(await LedgerEntry.countDocuments({})).toBe(1); + }); + + describe("cooldown enforcement", () => { + it("rejects an award inside the cooldown window, before any ledger write", async () => { + await makeRule({ cooldownSeconds: 3600 }); + const now = new Date(); + + const first = await processEvent({ eventId: "evt-1", userId, actionKey: "lesson.completed", occurredAt: now }); + expect(first.outcome).toBe("awarded"); + + const secondTime = new Date(now.getTime() + 60 * 1000); // 1 min later, inside a 1h cooldown + const second = await processEvent({ eventId: "evt-2", userId, actionKey: "lesson.completed", occurredAt: secondTime }); + expect(second.outcome).toBe("cooldown"); + + expect(await LedgerEntry.countDocuments({})).toBe(1); + }); + + it("allows an award once the cooldown window has passed", async () => { + await makeRule({ cooldownSeconds: 60 }); + const now = new Date(); + + await processEvent({ eventId: "evt-1", userId, actionKey: "lesson.completed", occurredAt: now }); + + const laterTime = new Date(now.getTime() + 61 * 1000); + const second = await processEvent({ eventId: "evt-2", userId, actionKey: "lesson.completed", occurredAt: laterTime }); + expect(second.outcome).toBe("awarded"); + + expect(await LedgerEntry.countDocuments({})).toBe(2); + }); + + it("does not apply another user's history to a cooldown check", async () => { + await makeRule({ cooldownSeconds: 3600 }); + const otherUser = new mongoose.Types.ObjectId(); + const now = new Date(); + + await processEvent({ eventId: "evt-1", userId: otherUser, actionKey: "lesson.completed", occurredAt: now }); + const result = await processEvent({ eventId: "evt-2", userId, actionKey: "lesson.completed", occurredAt: now }); + + expect(result.outcome).toBe("awarded"); + }); + }); + + describe("daily-cap enforcement", () => { + it("rejects an award once dailyCap payouts have already happened today (UTC)", async () => { + await makeRule({ dailyCap: 2 }); + const day = new Date("2026-06-15T10:00:00.000Z"); + + const r1 = await processEvent({ eventId: "evt-1", userId, actionKey: "lesson.completed", occurredAt: day }); + const r2 = await processEvent({ + eventId: "evt-2", + userId, + actionKey: "lesson.completed", + occurredAt: new Date("2026-06-15T11:00:00.000Z"), + }); + const r3 = await processEvent({ + eventId: "evt-3", + userId, + actionKey: "lesson.completed", + occurredAt: new Date("2026-06-15T12:00:00.000Z"), + }); + + expect(r1.outcome).toBe("awarded"); + expect(r2.outcome).toBe("awarded"); + expect(r3.outcome).toBe("daily_cap"); + expect(await LedgerEntry.countDocuments({})).toBe(2); + }); + + it("resets the cap at UTC midnight", async () => { + await makeRule({ dailyCap: 1 }); + + const r1 = await processEvent({ + eventId: "evt-1", + userId, + actionKey: "lesson.completed", + occurredAt: new Date("2026-06-15T23:59:00.000Z"), + }); + const r2 = await processEvent({ + eventId: "evt-2", + userId, + actionKey: "lesson.completed", + occurredAt: new Date("2026-06-16T00:01:00.000Z"), + }); + + expect(r1.outcome).toBe("awarded"); + expect(r2.outcome).toBe("awarded"); // new UTC day, cap reset + }); + }); +}); From e6b5e11838be3433a0cb4d9878ee153a6e78ec22 Mon Sep 17 00:00:00 2001 From: karthikeya veginati Date: Thu, 24 Sep 2026 12:53:36 -0500 Subject: [PATCH 2/9] wire currency consumer + rate limit into server.js, remove updateHighScore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the previous commit: server.js now actually starts the currency consumer after connectDB() resolves (skipped under Jest), adds the currencyEventLimiter on /gameResults, and users.js has PUT /updateHighScore removed as described in the prior commit message — these two files' changes were dropped from that commit despite being staged. Co-Authored-By: Claude Sonnet 5 --- middlewareNode/src/routes/users.js | 41 ++++++++++-------------------- middlewareNode/src/server.js | 34 ++++++++++++++++++++++--- 2 files changed, 44 insertions(+), 31 deletions(-) diff --git a/middlewareNode/src/routes/users.js b/middlewareNode/src/routes/users.js index c62ef4c5..d4d1ae4e 100644 --- a/middlewareNode/src/routes/users.js +++ b/middlewareNode/src/routes/users.js @@ -531,34 +531,19 @@ router.get("/getUser", passport.authenticate("jwt", { session: false }), async ( } }); -// @route PUT /user/updateHighScore -// @desc Update the user's highest streak or dash score if they beat their record -// @access Public with jwt Authentication -router.put("/updateHighScore", passport.authenticate("jwt", { session: false }), async (req, res) => { - try { - const { streakScore, dashScore } = req.body; - const db = await getDb(); - const usersCollection = db.collection("users"); - - const updateFields = {}; - - // Mongoose $max operator ensures it ONLY updates if the new score is higher than the old one! - if (streakScore !== undefined) updateFields.highestStreak = parseInt(streakScore); - if (dashScore !== undefined) updateFields.highestDashScore = parseInt(dashScore); - - if (Object.keys(updateFields).length === 0) return res.status(400).json("No scores provided"); - - const result = await usersCollection.updateOne( - { username: req.user.username }, - { $max: updateFields } - ); - - res.status(200).json({ message: "High scores checked and updated successfully" }); - } catch (error) { - console.error("Error updating high score:", error); - res.status(500).json("Server error"); - } -}); +// PUT /user/updateHighScore was removed here (currency rollout Rev. 2, +// change 6 — "Close PUT /user/updateHighScore"). It wrote client-supplied +// streakScore/dashScore straight into highestStreak/highestDashScore under +// $max with no server-side recomputation, so any authenticated student +// could post an arbitrary number and have it accepted as long as it beat +// their stored record. Confirmed via a repo-wide search that nothing in +// react-ystemandchess calls this endpoint and nothing reads +// highestStreak/highestDashScore anywhere except this route — it was not +// surfaced in any gamified view, so removal (not a server-authoritative +// rewrite) is the correct fix rather than hardening a dead endpoint. +// highestStreak/highestDashScore remain on the User schema for now (not a +// data migration); a future pass can drop them once confirmed nothing +// else depends on the stored values. /** * PUT /user/profile diff --git a/middlewareNode/src/server.js b/middlewareNode/src/server.js index 5167de20..91c651ef 100644 --- a/middlewareNode/src/server.js +++ b/middlewareNode/src/server.js @@ -20,6 +20,7 @@ const streakRoutes = require("./routes/streak"); const adminGuard = require("./middleware/adminGuard"); const requireAuth = require("./middleware/requireAuth"); const rateLimit = require("express-rate-limit"); +const currencyConsumer = require("./services/currencyConsumer"); const analyticsLimiter = rateLimit({ windowMs: 15 * 60 * 1000, @@ -37,9 +38,26 @@ const leaderboardLimiter = rateLimit({ message: { error: "Too many requests, please try again later" }, }); +// Currency rollout, Week 3 hardening: rate limit on event-triggering +// endpoints — currently just /gameResults, the one endpoint that can +// produce a currency-earning ActionEvent today. Cooldowns/daily caps +// (ledgerService.js) already cap how much a legitimate flood of requests +// can earn, but a limiter stops the flood itself from reaching the DB at +// all. Reused as-is once Jimmy's lesson/puzzle producer endpoints land — +// same limiter, mounted on whatever routes end up calling into the +// currency pipeline. +const currencyEventLimiter = rateLimit({ + windowMs: 60 * 1000, + max: parseInt(process.env.CURRENCY_EVENT_RATE_LIMIT_MAX) || 30, + standardHeaders: true, + legacyHeaders: false, + message: { error: "Too many requests, please try again later" }, +}); + // Enable schedulers require("./scheduler/activitiesScheduler.js"); require("./scheduler/analyticsSummaryScheduler.js"); +require("./scheduler/currencyAnomalyScheduler.js"); // Enable CORS for cross-origin requests const configuredCorsOrigin = config.get("corsOptions.origin"); @@ -76,8 +94,18 @@ app.use( }) ); -// Connect to MongoDB database -connectDB(); +// Connect to MongoDB database, then start the currency consumer. +// Sequenced deliberately: the consumer's poll loop hits Mongoose models +// (ActionEvent, ActionRule, ...) on an interval starting immediately, so +// starting it before connectDB() resolves would throw on every tick until +// the connection came up. Skipped entirely under Jest — a setInterval +// timer with no matching stop() would otherwise leak across test files as +// an open handle. +connectDB().then(() => { + if (process.env.NODE_ENV !== "test") { + currencyConsumer.start(); + } +}); // Initialize JSON middleware for parsing request bodies app.use(express.json({ extended: false })); @@ -113,7 +141,7 @@ app.use("/streak", streakRoutes); app.use("/badges", require("./routes/badges")); app.use("/chat", require("./routes/chat")); app.use("/challenge", require("./routes/challenge")); -app.use("/gameResults", requireAuth, require("./routes/gameResults")); +app.use("/gameResults", currencyEventLimiter, requireAuth, require("./routes/gameResults")); app.use("/analytics", analyticsLimiter, adminGuard, require("./routes/analytics")); app.use("/leaderboard", leaderboardLimiter, requireAuth, require("./routes/leaderboard")); From cdf733fcca9eda712f463525ff569300eee4a770 Mon Sep 17 00:00:00 2001 From: karthikeya veginati Date: Thu, 24 Sep 2026 13:28:48 -0500 Subject: [PATCH 3/9] feat(middlewareNode): currency producer + leaderboard swap (Jimmy's Rev. 2 lane) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the producers/leaderboard workstream from the currency rollout plan, built against Karthik's real schemas/consumer (already merged) rather than a temporary stub sink: - services/currencyEvents.js: the single choke point every feature route goes through to record a currency-earning action. Deterministic eventIds (e.g. lesson:::) so a retried request can never double-emit. - Wired lesson.completed into routes/lessons.js's /updateLessonCompletion, emitting only on a genuine forward-progress write (modifiedCount > 0), never on the existing 304 no-op branch or for unauthenticated guests (no account to credit). - puzzle.solved is deliberately NOT wired: there is no server-side puzzle-completion endpoint anywhere in the codebase to attach to (puzzles.js only lists/serves puzzles). Documented as blocked on a missing feature rather than fabricating an emit point. - routes/leaderboard.js: score now reads UserBalance.lifetimeEarned (batched via a new getLifetimeEarnedMap, one query for the whole candidate page rather than per-student) instead of the old time/ streak/badge/activity weighted formula. A student with no UserBalance document reads as 0, not undefined. Chess record stays a separate, untouched stat. - Rewrote leaderboard.test.js and leaderboard.analytics-consistency.test.js against the new score source — the latter's original premise (leaderboard and analytics should agree via the same formula) no longer holds now that they're deliberately different signals, so it's rewritten to verify the leaderboard genuinely reads real ledger data instead. 326 tests passing, zero regressions. Co-Authored-By: Claude Sonnet 5 --- middlewareNode/src/routes/leaderboard.js | 122 ++++++-------- middlewareNode/src/routes/lessons.js | 11 ++ middlewareNode/src/services/currencyEvents.js | 85 ++++++++++ middlewareNode/src/services/ledgerService.js | 37 +++- middlewareNode/tests/currencyEvents.test.js | 109 ++++++++++++ .../leaderboard.analytics-consistency.test.js | 127 +++++++------- middlewareNode/tests/leaderboard.test.js | 106 ++++++------ middlewareNode/tests/ledgerService.test.js | 51 ++++++ .../tests/lessons.currencyEmit.test.js | 159 ++++++++++++++++++ 9 files changed, 626 insertions(+), 181 deletions(-) create mode 100644 middlewareNode/src/services/currencyEvents.js create mode 100644 middlewareNode/tests/currencyEvents.test.js create mode 100644 middlewareNode/tests/lessons.currencyEmit.test.js diff --git a/middlewareNode/src/routes/leaderboard.js b/middlewareNode/src/routes/leaderboard.js index 24251541..17b91758 100644 --- a/middlewareNode/src/routes/leaderboard.js +++ b/middlewareNode/src/routes/leaderboard.js @@ -1,28 +1,34 @@ /** * Leaderboard Routes — /leaderboard * - * Student-facing endpoint returning ranked students by a composite score. - * Protected by requireAuth (valid JWT, any role) — NOT admin-only, unlike - * /analytics, since students need to view the leaderboard themselves. + * Student-facing endpoint returning ranked students by their currency + * ledger standing. Protected by requireAuth (valid JWT, any role) — NOT + * admin-only, unlike /analytics, since students need to view the + * leaderboard themselves. * - * Score is computed on read from existing per-student stats (time played, - * streak, activities completed, badges earned) via utils/studentStats — - * the same helpers the admin analytics dashboard uses, so the two features - * can never silently disagree about a student's numbers. + * `score` reads UserBalance.lifetimeEarned (services/ledgerService.js), + * not the spendable `balance` field — lifetimeEarned is monotonic (a sum + * of positive LedgerEntry amounts only), so it can never go down. Ranking + * by spendable balance instead would mean the day a currency store ships, + * the top of the leaderboard becomes whoever has redeemed the least — + * rewarding declining to use the system. See the currency rollout plan, + * "Rank by lifetime earned, not balance." * - * Score weights are configurable via env vars so they can be tuned per - * environment without a deploy: - * LEADERBOARD_WEIGHT_TIME (default 1) — per hour of puzzle+lesson time - * LEADERBOARD_WEIGHT_STREAK (default 5) — per consecutive-day streak - * LEADERBOARD_WEIGHT_BADGE (default 10) — per badge earned - * LEADERBOARD_WEIGHT_ACTIVITY (default 3) — per activity completed + * This replaces the previous engagement formula computed on read from + * time/streak/activities/badges (utils/studentStats) — that formula is + * still used elsewhere (e.g. admin analytics) but is no longer this + * route's score source. A student with no UserBalance document yet (has + * never earned any currency) reads as lifetimeEarned: 0, not undefined — + * see the `|| 0` at the score assignment below; without it the sort + * comparator would produce undefined ordering instead of placing that + * student last. * - * `score` above measures ENGAGEMENT. Student-vs-student chess results are a - * different signal (competitive skill), so they are reported alongside it as a - * separate `chess_score` / `chess_record` and are deliberately NOT added into - * `score` — blending them would make one number mean two things, and would - * compound one open weighting question into two. Chess weights live in - * utils/studentStats (PVP_WEIGHT_WIN / _DRAW / _LOSS). + * `score` above measures ENGAGEMENT (via currency earned for engaging + * actions). Student-vs-student chess results are a different signal + * (competitive skill), so they are reported alongside it as a separate + * `chess_score` / `chess_record` and are deliberately NOT added into + * `score` — blending them would make one number mean two things. Chess + * weights live in utils/studentStats (PVP_WEIGHT_WIN / _DRAW / _LOSS). * * Response contract matches LeaderboardModal.tsx exactly: * GET /leaderboard/schools -> { success, schools: string[] } @@ -39,21 +45,9 @@ const express = require("express"); const router = express.Router(); const Users = require("../models/users"); -const { - getUserTimeStats, - getUserStreak, - getActivitiesCompleted, - getBadgesEarned, - getChessRecords, -} = require("../utils/studentStats"); +const { getChessRecords } = require("../utils/studentStats"); const { getAvatarUrl } = require("../utils/avatars"); - -const WEIGHTS = { - time: parseFloat(process.env.LEADERBOARD_WEIGHT_TIME) || 1, - streak: parseFloat(process.env.LEADERBOARD_WEIGHT_STREAK) || 5, - badge: parseFloat(process.env.LEADERBOARD_WEIGHT_BADGE) || 10, - activity: parseFloat(process.env.LEADERBOARD_WEIGHT_ACTIVITY) || 3, -}; +const { getLifetimeEarnedMap } = require("../services/ledgerService"); const MAX_LIMIT = 100; const DEFAULT_LIMIT = 10; @@ -79,29 +73,11 @@ function escapeRegex(value) { return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } -/** - * Computes the composite ENGAGEMENT score for one student from existing stat - * helpers. Placeholder weighting — confirm with product before treating as final. - * - * Chess results are intentionally absent here; they are surfaced as their own - * column (see the module header) rather than folded into this number. - */ -async function computeScore(user) { - const [timeStats, streak, activitiesCompleted, badgesEarned] = await Promise.all([ - getUserTimeStats(user.username), - getUserStreak(user.username), - getActivitiesCompleted(user._id), - getBadgesEarned(user.username), - ]); - - const timeComponent = (timeStats.puzzleTimeHours + timeStats.lessonTimeHours) * WEIGHTS.time; - const streakComponent = streak * WEIGHTS.streak; - const badgeComponent = badgesEarned * WEIGHTS.badge; - const activityComponent = activitiesCompleted * WEIGHTS.activity; - - const score = Math.round(timeComponent + streakComponent + badgeComponent + activityComponent); - return score; -} +// computeScore() (the old time/streak/badge/activity weighted formula) +// was removed here — score now comes from getLifetimeEarnedMap (see the +// module header). utils/studentStats' getUserTimeStats/getUserStreak/ +// getActivitiesCompleted/getBadgesEarned still exist and are still used +// elsewhere (e.g. admin analytics); only this route's score source changed. /** * Builds a GET /leaderboard/s handler returning distinct, non-empty @@ -171,22 +147,26 @@ router.get("/", async (req, res) => { candidates = candidates.slice(0, MAX_UNFILTERED_CANDIDATES); } - // One batched query for every candidate's chess record, rather than a - // fifth per-student round trip inside the map below. + // One batched query for every candidate's chess record and lifetime- + // earned currency, rather than a per-student round trip inside the + // map below for either. const chessRecords = await getChessRecords(candidates.map((u) => u.username)); - - const scored = await Promise.all( - candidates.map(async (user) => ({ - id: String(user._id), - username: user.username, - school: user.school || null, - country: user.country || null, - state: user.state || null, - avatarUrl: getAvatarUrl(user.avatarKey), - score: await computeScore(user), - chess: chessRecords.get(user.username), - })) - ); + const lifetimeEarnedMap = await getLifetimeEarnedMap(candidates.map((u) => u._id)); + + const scored = candidates.map((user) => ({ + id: String(user._id), + username: user.username, + school: user.school || null, + country: user.country || null, + state: user.state || null, + avatarUrl: getAvatarUrl(user.avatarKey), + // A user with no UserBalance document yet (never earned any + // currency) must read as 0, not undefined — an undefined score + // would make the sort comparator below produce undefined ordering + // instead of placing that student last. + score: lifetimeEarnedMap.get(String(user._id)) || 0, + chess: chessRecords.get(user.username), + })); const direction = sortDir === "asc" ? 1 : -1; if (sortBy === "name") { diff --git a/middlewareNode/src/routes/lessons.js b/middlewareNode/src/routes/lessons.js index 65d87041..c25024b9 100644 --- a/middlewareNode/src/routes/lessons.js +++ b/middlewareNode/src/routes/lessons.js @@ -20,6 +20,7 @@ const { MongoClient } = require('mongodb'); require('dotenv').config(); const mongoose = require("mongoose"); +const { emitLessonCompleted } = require("../services/currencyEvents"); // Cache database client to prevent repeated connections let cachedClient = null; @@ -294,6 +295,16 @@ router.get( // check if changes have been made in db if (updateResult.modifiedCount > 0) { + // Currency rollout: emit only on a genuine forward-progress + // write, never on the 304 branch below — a request re-sending + // an already-completed lesson number must not earn currency + // twice. eventId is deterministic per (user, piece, lessonNum), + // so even a client retry of this exact successful request can't + // double-emit; see services/currencyEvents.js. Guests (the else + // branch below) never emit — there's no account to credit. + emitLessonCompleted({ userId: req.user._id, piece, lessonNum }).catch((err) => { + console.error("currencyEvents: failed to emit lesson.completed:", err.message); + }); res.status(200).json("Lesson progress updated"); } else { res.status(304).json("No changes made"); diff --git a/middlewareNode/src/services/currencyEvents.js b/middlewareNode/src/services/currencyEvents.js new file mode 100644 index 00000000..087408aa --- /dev/null +++ b/middlewareNode/src/services/currencyEvents.js @@ -0,0 +1,85 @@ +/** + * Currency Event Producers + * + * The single choke point every feature module goes through to record a + * currency-earning action — per Jimmy's ledger plan, "no feature module + * imports anything from the currency service directly; every award + * happens purely through an emitted event." A feature route calls one of + * these after its own write succeeds; nothing here decides whether or how + * much currency to award — that's ActionRule + the consumer's job + * (services/ledgerService.js, services/currencyConsumer.js). This module + * only ever inserts an ActionEvent. + * + * Deliberately real, not a mock/stub sink: earlier drafts of this plan + * called for a fake in-memory sink Jimmy's lane could test against before + * Karthik's consumer existed. That consumer is real now (see + * services/currencyConsumer.js), so producing into a parallel fake + * collection would mean maintaining two implementations that could drift + * — this writes directly to the real ActionEvent collection, and tests + * verify that against a real in-memory MongoDB instance instead. + * + * eventId is deterministic per producer (documented on each function) so + * a retried request, a duplicate delivery, or a page refresh mid-request + * can never double-emit the same real-world completion — the same + * property gameResults.js gets from gameId and the backfill gets from its + * synthetic ids. + */ + +const ActionEvent = require("../models/actionEvent"); + +const DUPLICATE_KEY_ERROR = 11000; + +/** + * Inserts one ActionEvent, treating a duplicate eventId as a successful + * no-op rather than an error — the caller doesn't need to know or care + * whether this exact completion was already recorded (e.g. a retried + * request after a dropped response). Returns { emitted: true } for a + * fresh insert, { emitted: false, duplicate: true } for an already-seen + * eventId. + */ +async function emit({ eventId, actionKey, userId, metadata = {}, occurredAt = new Date() }) { + try { + await ActionEvent.create({ + eventId, + actionKey, + userId, + metadata, + status: "pending", + occurredAt, + }); + return { emitted: true, duplicate: false, eventId }; + } catch (err) { + if (err && err.code === DUPLICATE_KEY_ERROR) { + return { emitted: false, duplicate: true, eventId }; + } + throw err; + } +} + +/** + * Emits lesson.completed. Called from routes/lessons.js's + * /updateLessonCompletion handler, only after users.updateOne() reports + * modifiedCount > 0 — i.e. only on a real forward-progress write, never + * on a request that re-sent an already-completed lesson number (which + * the route already treats as a 304 no-op today). + * + * eventId is `lesson:::` — deterministic per + * (student, piece, lesson number) triple, matching the backfill's own + * `backfill:lesson:...` naming convention (see + * scripts/backfillCurrencyLedger.js) so the two families of ids are + * visually distinguishable in the collection without ever colliding. + */ +async function emitLessonCompleted({ userId, piece, lessonNum, occurredAt }) { + return emit({ + eventId: `lesson:${userId}:${piece}:${lessonNum}`, + actionKey: "lesson.completed", + userId, + metadata: { piece, lessonNum }, + occurredAt, + }); +} + +module.exports = { + emit, + emitLessonCompleted, +}; diff --git a/middlewareNode/src/services/ledgerService.js b/middlewareNode/src/services/ledgerService.js index a9d5dbb9..f99292c9 100644 --- a/middlewareNode/src/services/ledgerService.js +++ b/middlewareNode/src/services/ledgerService.js @@ -26,15 +26,45 @@ const DUPLICATE_KEY_ERROR = 11000; /** * Reads a user's lifetimeEarned, treating "no UserBalance document yet" - * as 0 rather than undefined. Used by the leaderboard swap (Jimmy's lane) - * and anywhere else that needs a safe-to-sort number for every user, - * including ones who have never earned anything. + * as 0 rather than undefined. Used anywhere that needs a safe-to-sort + * number for exactly one user, including ones who have never earned + * anything. For a whole candidate list (e.g. the leaderboard), prefer + * getLifetimeEarnedMap below — this one query per call would reintroduce + * the same per-student round-trip cost the leaderboard's chess-record + * lookup was already batched to avoid. */ async function getLifetimeEarnedOrZero(userId) { const doc = await UserBalance.findOne({ userId }, { lifetimeEarned: 1, _id: 0 }); return doc ? doc.lifetimeEarned : 0; } +/** + * Batched version of getLifetimeEarnedOrZero for a whole candidate list — + * one query for every UserBalance document that exists among the given + * userIds, rather than one query per user. Returns a Map; a userId with no UserBalance document simply has no + * entry, so callers should read via `map.get(id) || 0` (see + * routes/leaderboard.js) to get the same "missing means 0, not + * undefined" guarantee as the single-user helper. + */ +async function getLifetimeEarnedMap(userIds) { + // An empty candidate list (e.g. a filtered leaderboard query matching + // no students) has nothing to look up — skip the query entirely rather + // than issuing `$in: []`, which some drivers/mocks handle fine but is + // pure overhead either way. + if (!userIds || userIds.length === 0) return new Map(); + + const docs = await UserBalance.find( + { userId: { $in: userIds } }, + { userId: 1, lifetimeEarned: 1, _id: 0 } + ); + const map = new Map(); + for (const doc of docs) { + map.set(String(doc.userId), doc.lifetimeEarned); + } + return map; +} + /** * Cooldown check: has this user actually been PAID for this action within * the rule's cooldownSeconds? A cooldownSeconds of 0 means "no cooldown" @@ -196,6 +226,7 @@ module.exports = { processEvent, applyLedgerEntry, getLifetimeEarnedOrZero, + getLifetimeEarnedMap, isWithinCooldown, hasHitDailyCap, }; diff --git a/middlewareNode/tests/currencyEvents.test.js b/middlewareNode/tests/currencyEvents.test.js new file mode 100644 index 00000000..d5f611a3 --- /dev/null +++ b/middlewareNode/tests/currencyEvents.test.js @@ -0,0 +1,109 @@ +/** + * Real-database tests for services/currencyEvents.js — the producer + * module every feature route goes through to record a currency-earning + * action (Jimmy's ledger plan, Week 1-2: "no feature module imports + * anything from the currency service directly"). + * + * Uses mongodb-memory-server (see tests/ledgerService.test.js for why): + * the property under test — a deterministic eventId makes a retried + * emit a genuine no-op — is a real unique-index guarantee. + */ + +const { MongoMemoryServer } = require("mongodb-memory-server"); +const mongoose = require("mongoose"); + +jest.setTimeout(60000); + +let mongod; + +beforeAll(async () => { + mongod = await MongoMemoryServer.create({ instance: { launchTimeout: 30000 } }); + await mongoose.connect(mongod.getUri() + "ystem"); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongod.stop(); +}); + +afterEach(async () => { + const collections = mongoose.connection.collections; + await Promise.all(Object.values(collections).map((c) => c.deleteMany({}))); +}); + +const ActionEvent = require("../src/models/actionEvent"); +const { emit, emitLessonCompleted } = require("../src/services/currencyEvents"); + +const userId = new mongoose.Types.ObjectId(); + +describe("emit", () => { + it("inserts a pending ActionEvent with the given shape", async () => { + const result = await emit({ + eventId: "evt-1", + actionKey: "lesson.completed", + userId, + metadata: { piece: "knight" }, + }); + + expect(result).toEqual({ emitted: true, duplicate: false, eventId: "evt-1" }); + + const stored = await ActionEvent.findOne({ eventId: "evt-1" }); + expect(stored.actionKey).toBe("lesson.completed"); + expect(stored.status).toBe("pending"); + expect(stored.metadata).toEqual({ piece: "knight" }); + }); + + it("treats a duplicate eventId as a no-op, not an error", async () => { + const args = { eventId: "evt-dup", actionKey: "lesson.completed", userId, metadata: {} }; + + const first = await emit(args); + const second = await emit(args); + + expect(first).toEqual({ emitted: true, duplicate: false, eventId: "evt-dup" }); + expect(second).toEqual({ emitted: false, duplicate: true, eventId: "evt-dup" }); + expect(await ActionEvent.countDocuments({ eventId: "evt-dup" })).toBe(1); + }); + + it("survives concurrent duplicate emits of the same eventId", async () => { + const args = { eventId: "evt-race", actionKey: "lesson.completed", userId, metadata: {} }; + + const [a, b] = await Promise.all([emit(args), emit(args)]); + const duplicateFlags = [a.duplicate, b.duplicate].sort(); + + expect(duplicateFlags).toEqual([false, true]); + expect(await ActionEvent.countDocuments({ eventId: "evt-race" })).toBe(1); + }); +}); + +describe("emitLessonCompleted", () => { + it("builds a deterministic eventId from userId, piece, and lessonNum", async () => { + await emitLessonCompleted({ userId, piece: "The Fork", lessonNum: 2 }); + + const stored = await ActionEvent.findOne({ actionKey: "lesson.completed" }); + expect(stored.eventId).toBe(`lesson:${userId}:The Fork:2`); + expect(stored.metadata).toEqual({ piece: "The Fork", lessonNum: 2 }); + }); + + it("never double-emits for the same (user, piece, lessonNum) — a client retry is a no-op", async () => { + await emitLessonCompleted({ userId, piece: "The Fork", lessonNum: 2 }); + const second = await emitLessonCompleted({ userId, piece: "The Fork", lessonNum: 2 }); + + expect(second.duplicate).toBe(true); + expect(await ActionEvent.countDocuments({ actionKey: "lesson.completed" })).toBe(1); + }); + + it("emits separately for different lesson numbers on the same piece", async () => { + await emitLessonCompleted({ userId, piece: "The Fork", lessonNum: 1 }); + await emitLessonCompleted({ userId, piece: "The Fork", lessonNum: 2 }); + + expect(await ActionEvent.countDocuments({ actionKey: "lesson.completed" })).toBe(2); + }); + + it("emits separately for the same lesson number across different users", async () => { + const otherUser = new mongoose.Types.ObjectId(); + await emitLessonCompleted({ userId, piece: "The Fork", lessonNum: 1 }); + await emitLessonCompleted({ userId: otherUser, piece: "The Fork", lessonNum: 1 }); + + expect(await ActionEvent.countDocuments({ actionKey: "lesson.completed" })).toBe(2); + }); +}); diff --git a/middlewareNode/tests/leaderboard.analytics-consistency.test.js b/middlewareNode/tests/leaderboard.analytics-consistency.test.js index a4f16d3e..49c89f78 100644 --- a/middlewareNode/tests/leaderboard.analytics-consistency.test.js +++ b/middlewareNode/tests/leaderboard.analytics-consistency.test.js @@ -1,17 +1,23 @@ /** - * Real-database consistency test — LB-02. + * Real-database consistency test — LB-02, rewritten for the currency + * rollout's leaderboard swap. * - * Confirms /leaderboard and /analytics/student/:username derive identical - * underlying stats (time played, streak, activities completed, badges - * earned) for the same student and date, since both routes import the - * same utils/studentStats helpers. A discrepancy here would mean the - * shared module was bypassed or duplicated somewhere, and the two - * features would silently disagree about a student's numbers. + * Previously this test proved /leaderboard's score and /analytics's raw + * stats came from the same weighted-formula computation — both routes + * imported the same utils/studentStats helpers, and a discrepancy would + * mean that shared module had been bypassed or duplicated. That coupling + * is now gone on purpose: /leaderboard's score comes from + * UserBalance.lifetimeEarned (services/ledgerService.js), while /analytics + * still reports the old engagement stats directly. They are now two + * different signals by design (see routes/leaderboard.js's module + * header) — reasserting the old cross-endpoint equality would be + * asserting a coupling this swap was built to remove. * - * Uses mongodb-memory-server + real Mongoose models — no mocks — since - * the whole point is verifying two independently-written route handlers - * genuinely compute the same thing from the same data, not that they - * both call an identical mock. + * What this file verifies instead: /leaderboard's score is genuinely + * sourced from the real ledger, not a stale in-memory computation — + * using real Mongoose models (UserBalance, Users), not mocks, since the + * property under test is "the route reads what's actually in the + * database," which a mock can't demonstrate. */ const { MongoMemoryServer } = require("mongodb-memory-server"); @@ -31,16 +37,11 @@ beforeAll(async () => { mongod = await MongoMemoryServer.create({ instance: { launchTimeout: 30000 } }); await mongoose.connect(mongod.getUri() + "ystem"); - // Bypass real JWT for both admin (analytics) and any-role (leaderboard) auth - const adminGuard = (req, res, next) => { req.user = { username: "admin", role: "admin" }; next(); }; const requireAuth = (req, res, next) => { req.user = { username: "alice", role: "student" }; next(); }; - - const analyticsRoute = require("../src/routes/analytics"); const leaderboardRoute = require("../src/routes/leaderboard"); app = express(); app.use(express.json()); - app.use("/analytics", adminGuard, analyticsRoute); app.use("/leaderboard", requireAuth, leaderboardRoute); }); @@ -54,67 +55,75 @@ afterEach(async () => { await Promise.all(Object.values(collections).map((c) => c.deleteMany({}))); }); -describe("LB-02 — leaderboard and analytics agree on the same student's stats", () => { - test("time, streak, and badge counts match between /leaderboard inputs and /analytics/student", async () => { +describe("LB-02 — leaderboard score reflects the real currency ledger", () => { + test("a student's leaderboard score matches their real UserBalance.lifetimeEarned", async () => { const Users = require("../src/models/users"); - const TimeTracking = require("../src/models/timeTracking"); - const UserBadges = require("../src/models/UserBadges"); + const UserBalance = require("../src/models/userBalance"); const alice = await Users.create({ username: "alice", email: "alice@test.com", password: "hashed", firstName: "Alice", lastName: "Test", role: "student", school: "Test School", }); + await UserBalance.create({ userId: alice._id, balance: 42, lifetimeEarned: 42 }); - const now = new Date(); - await TimeTracking.create([ - { username: "alice", eventType: "puzzle", eventId: "e1", startTime: now, totalTime: 3600 }, - { username: "alice", eventType: "lesson", eventId: "e2", startTime: now, totalTime: 1800 }, - ]); - await UserBadges.create({ userId: "alice", earned: [{ badgeId: "first_lesson" }, { badgeId: "streak_5" }] }); - - const analyticsRes = await request(app).get("/analytics/student/alice"); - expect(analyticsRes.status).toBe(200); - - const leaderboardRes = await request(app).get("/leaderboard?school=" + encodeURIComponent("Test School")); - expect(leaderboardRes.status).toBe(200); + const res = await request(app).get("/leaderboard?school=" + encodeURIComponent("Test School")); + expect(res.status).toBe(200); - // Analytics exposes the raw stats directly; leaderboard only exposes - // the composed score, so recompute the leaderboard's expected score - // from the SAME analytics numbers to prove they came from one source. - const { stats } = analyticsRes.body; - const aliceEntry = leaderboardRes.body.data.leaderboard.find((e) => e.username === "alice"); + const aliceEntry = res.body.data.leaderboard.find((e) => e.username === "alice"); expect(aliceEntry).toBeDefined(); - - const WEIGHTS = { time: 1, streak: 5, badge: 10, activity: 3 }; // route defaults - const expectedScore = Math.round( - (stats.puzzleTimeHours + stats.lessonTimeHours) * WEIGHTS.time + - stats.currentStreak * WEIGHTS.streak + - stats.badgesEarned * WEIGHTS.badge + - stats.activitiesCompleted * WEIGHTS.activity - ); - - expect(aliceEntry.score).toBe(expectedScore); - // 3600s puzzle + 1800s lesson = 1.5 hours total, 2 badges, streak from - // a single day with both lesson+puzzle = 1. - expect(stats.puzzleTimeHours + stats.lessonTimeHours).toBe(1.5); - expect(stats.badgesEarned).toBe(2); + expect(aliceEntry.score).toBe(42); }); - test("a student absent from timeTracking shows score 0 on both endpoints consistently", async () => { + test("a student with no UserBalance document shows score 0, not omitted or undefined", async () => { const Users = require("../src/models/users"); await Users.create({ username: "quiet", email: "quiet@test.com", password: "hashed", firstName: "Quiet", lastName: "Student", role: "student", school: "Silent School", }); - const analyticsRes = await request(app).get("/analytics/student/quiet"); - const leaderboardRes = await request(app).get("/leaderboard?school=" + encodeURIComponent("Silent School")); - - expect(analyticsRes.body.stats.totalTimeHours).toBe(0); - expect(analyticsRes.body.stats.currentStreak).toBe(0); - expect(analyticsRes.body.stats.badgesEarned).toBe(0); + const res = await request(app).get("/leaderboard?school=" + encodeURIComponent("Silent School")); + const quietEntry = res.body.data.leaderboard.find((e) => e.username === "quiet"); - const quietEntry = leaderboardRes.body.data.leaderboard.find((e) => e.username === "quiet"); + expect(quietEntry).toBeDefined(); expect(quietEntry.score).toBe(0); }); + + test("score reflects lifetimeEarned, not spendable balance — the two diverge once currency is spent", async () => { + const Users = require("../src/models/users"); + const UserBalance = require("../src/models/userBalance"); + + const bob = await Users.create({ + username: "bob", email: "bob@test.com", password: "hashed", + firstName: "Bob", lastName: "Test", role: "student", school: "Spend Test School", + }); + // Simulates a student who earned 100 total but has since spent some — + // balance (spendable) is lower than lifetimeEarned (monotonic). This + // scenario doesn't exist yet in production (no spend path this + // window), but the leaderboard must already be reading the field that + // stays correct once one ships. + await UserBalance.create({ userId: bob._id, balance: 30, lifetimeEarned: 100 }); + + const res = await request(app).get("/leaderboard?school=" + encodeURIComponent("Spend Test School")); + const bobEntry = res.body.data.leaderboard.find((e) => e.username === "bob"); + + expect(bobEntry.score).toBe(100); // lifetimeEarned, not the lower balance of 30 + }); + + test("ranking order follows real lifetimeEarned across multiple students", async () => { + const Users = require("../src/models/users"); + const UserBalance = require("../src/models/userBalance"); + + const low = await Users.create({ username: "low", email: "low@test.com", password: "h", firstName: "L", lastName: "L", role: "student", school: "Rank School" }); + const high = await Users.create({ username: "high", email: "high@test.com", password: "h", firstName: "H", lastName: "H", role: "student", school: "Rank School" }); + const none = await Users.create({ username: "none", email: "none@test.com", password: "h", firstName: "N", lastName: "N", role: "student", school: "Rank School" }); + + await UserBalance.create({ userId: low._id, balance: 5, lifetimeEarned: 5 }); + await UserBalance.create({ userId: high._id, balance: 50, lifetimeEarned: 50 }); + // "none" has no UserBalance document at all. + + const res = await request(app).get("/leaderboard?school=" + encodeURIComponent("Rank School")); + const order = res.body.data.leaderboard.map((e) => e.username); + + expect(order).toEqual(["high", "low", "none"]); + }); }); diff --git a/middlewareNode/tests/leaderboard.test.js b/middlewareNode/tests/leaderboard.test.js index c0ffa849..b5968196 100644 --- a/middlewareNode/tests/leaderboard.test.js +++ b/middlewareNode/tests/leaderboard.test.js @@ -1,15 +1,20 @@ /** * Integration tests — Leaderboard endpoint * - * Users model and studentStats helpers are mocked. - * Uses supertest against a minimal Express app. requireAuth is mocked - * to always pass through (auth enforcement is covered separately in + * Users model, studentStats (chess only now), avatars, and ledgerService + * are mocked. Uses supertest against a minimal Express app. requireAuth is + * mocked to always pass through (auth enforcement is covered separately in * requireAuth.test.js and leaderboard.security.test.js). * * Response contract matches LeaderboardModal.tsx: * { success, data: { leaderboard: [{id, rank, username, school_name, * score, avatar_url}], pagination: { has_more } } } * + * `score` comes from ledgerService.getLifetimeEarnedMap (currency rollout + * swap — see routes/leaderboard.js's module header) rather than the old + * time/streak/badge/activity weighted formula. Chess record stays a + * separate stat via studentStats.getChessRecords, untouched by this swap. + * * Endpoints tested: GET /leaderboard, GET /leaderboard/schools */ @@ -21,6 +26,7 @@ jest.mock("../src/middleware/requireAuth", () => (req, _res, next) => { jest.mock("../src/models/users"); jest.mock("../src/utils/studentStats"); jest.mock("../src/utils/avatars"); +jest.mock("../src/services/ledgerService"); const express = require("express"); const request = require("supertest"); @@ -29,6 +35,7 @@ const leaderboard = require("../src/routes/leaderboard"); const Users = require("../src/models/users"); const studentStats = require("../src/utils/studentStats"); const { getAvatarUrl } = require("../src/utils/avatars"); +const ledgerService = require("../src/services/ledgerService"); const app = express(); app.use(express.json()); @@ -42,21 +49,22 @@ const STUDENTS = [ { _id: "3", username: "carol", country: "Canada", state: null, school: null }, ]; -function mockStatsFor(scoreByUsername) { - studentStats.getUserTimeStats.mockImplementation(async (username) => ({ - puzzleTimeHours: scoreByUsername[username]?.puzzleTimeHours || 0, - lessonTimeHours: scoreByUsername[username]?.lessonTimeHours || 0, - totalTimeHours: 0, - gameTimeHours: 0, - mentorTimeHours: 0, - })); - studentStats.getUserStreak.mockImplementation( - async (username) => scoreByUsername[username]?.streak || 0 - ); - studentStats.getActivitiesCompleted.mockImplementation(async () => 0); - studentStats.getBadgesEarned.mockImplementation( - async (username) => scoreByUsername[username]?.badges || 0 - ); +/** + * Sets up ledgerService.getLifetimeEarnedMap to return the given + * score for each username (looked up by _id, matching real usage) and + * studentStats.getChessRecords for chess data — chess stays a completely + * separate, unaffected mock from the currency-score swap. + */ +function mockScoresFor(scoreByUsername, students = STUDENTS) { + const byId = new Map(students.map((s) => [String(s._id), scoreByUsername[s.username]?.score ?? 0])); + ledgerService.getLifetimeEarnedMap.mockImplementation(async (userIds) => { + const map = new Map(); + for (const id of userIds) { + const key = String(id); + if (byId.has(key)) map.set(key, byId.get(key)); + } + return map; + }); // Chess record is a separate stat, batched for the whole page. studentStats.getChessRecords.mockImplementation( async (usernames) => @@ -81,10 +89,10 @@ describe("GET /leaderboard", () => { getAvatarUrl.mockImplementation((avatarKey) => avatarKey ? `https://s3.example.com/${avatarKey}` : null ); - mockStatsFor({ - alice: { puzzleTimeHours: 10, streak: 5, badges: 2 }, - bob: { puzzleTimeHours: 5, streak: 2, badges: 1 }, - carol: { puzzleTimeHours: 1, streak: 0, badges: 0 }, + mockScoresFor({ + alice: { score: 87 }, + bob: { score: 41 }, + carol: { score: 3 }, }); }); @@ -222,7 +230,8 @@ describe("GET /leaderboard", () => { school: "Test School", })) ); - mockStatsFor({}); + ledgerService.getLifetimeEarnedMap.mockResolvedValue(new Map()); + studentStats.getChessRecords.mockImplementation(async (usernames) => new Map(usernames.map((u) => [u, { wins: 0, draws: 0, losses: 0, gamesPlayed: 0, chessScore: 0 }]))); const res = await request(app).get("/leaderboard?limit=500"); expect(res.body.data.leaderboard.length).toBeLessThanOrEqual(100); }); @@ -235,7 +244,8 @@ describe("GET /leaderboard", () => { school: "Test School", })) ); - mockStatsFor({}); + ledgerService.getLifetimeEarnedMap.mockResolvedValue(new Map()); + studentStats.getChessRecords.mockImplementation(async (usernames) => new Map(usernames.map((u) => [u, { wins: 0, draws: 0, losses: 0, gamesPlayed: 0, chessScore: 0 }]))); // No filters applied — unfiltered path should cap candidates before scoring. const res = await request(app).get("/leaderboard?limit=100&page=8"); // page 8 * 100 = would need 800 candidates // Total reported can never exceed the 500-candidate cap, regardless of @@ -254,7 +264,8 @@ describe("GET /leaderboard", () => { school: "Big School", })) ); - mockStatsFor({}); + ledgerService.getLifetimeEarnedMap.mockResolvedValue(new Map()); + studentStats.getChessRecords.mockImplementation(async (usernames) => new Map(usernames.map((u) => [u, { wins: 0, draws: 0, losses: 0, gamesPlayed: 0, chessScore: 0 }]))); // Page size is still capped at MAX_LIMIT (100) regardless of filtering, // but the underlying candidate set for a filtered query is NOT capped // at 500 — walk to the last page and confirm has_more only goes false @@ -271,7 +282,8 @@ describe("GET /leaderboard", () => { Users.find.mockResolvedValue([ { _id: "1", username: "quiet", school: "Test School" }, ]); - mockStatsFor({}); // no entry for "quiet" -> all stats default to 0 + ledgerService.getLifetimeEarnedMap.mockResolvedValue(new Map()); + studentStats.getChessRecords.mockImplementation(async (usernames) => new Map(usernames.map((u) => [u, { wins: 0, draws: 0, losses: 0, gamesPlayed: 0, chessScore: 0 }]))); // no entry for "quiet" -> all stats default to 0 const res = await request(app).get("/leaderboard"); expect(res.status).toBe(200); expect(res.body.data.leaderboard).toHaveLength(1); @@ -378,10 +390,9 @@ describe("GET /leaderboard/states", () => { describe("GET /leaderboard — country/state now included in entry response", () => { test("entries include country and state fields", async () => { - Users.find.mockResolvedValue([ - { _id: "1", username: "alice", country: "USA", state: "FL", school: "Jefferson Middle" }, - ]); - mockStatsFor({ alice: { streak: 1 } }); + const singleStudent = [{ _id: "1", username: "alice", country: "USA", state: "FL", school: "Jefferson Middle" }]; + Users.find.mockResolvedValue(singleStudent); + mockScoresFor({ alice: { score: 5 } }, singleStudent); const res = await request(app).get("/leaderboard"); expect(res.body.data.leaderboard[0]).toMatchObject({ country: "USA", state: "FL" }); }); @@ -406,10 +417,9 @@ describe("GET /leaderboard — chess record as its own column", () => { const CHESS = { wins: 4, draws: 2, losses: 1, gamesPlayed: 7, chessScore: 14 }; test("entries carry chess_score and chess_record alongside score", async () => { - Users.find.mockResolvedValue([ - { _id: "1", username: "alice", country: "USA", state: "FL", school: "Jefferson Middle" }, - ]); - mockStatsFor({ alice: { streak: 1, chess: CHESS } }); + const singleStudent = [{ _id: "1", username: "alice", country: "USA", state: "FL", school: "Jefferson Middle" }]; + Users.find.mockResolvedValue(singleStudent); + mockScoresFor({ alice: { score: 5, chess: CHESS } }, singleStudent); const res = await request(app).get("/leaderboard"); const entry = res.body.data.leaderboard[0]; @@ -417,16 +427,15 @@ describe("GET /leaderboard — chess record as its own column", () => { expect(entry.chess_record).toEqual({ wins: 4, draws: 2, losses: 1, gamesPlayed: 7 }); }); - test("chess results do NOT change the engagement score", async () => { - Users.find.mockResolvedValue([ - { _id: "1", username: "alice", country: "USA", state: "FL", school: "Jefferson Middle" }, - ]); + test("chess results do NOT change the currency score", async () => { + const singleStudent = [{ _id: "1", username: "alice", country: "USA", state: "FL", school: "Jefferson Middle" }]; + Users.find.mockResolvedValue(singleStudent); - mockStatsFor({ alice: { streak: 1 } }); // no games + mockScoresFor({ alice: { score: 20 } }, singleStudent); // no games const without = await request(app).get("/leaderboard"); const scoreWithoutGames = without.body.data.leaderboard[0].score; - mockStatsFor({ alice: { streak: 1, chess: CHESS } }); // same engagement, many wins + mockScoresFor({ alice: { score: 20, chess: CHESS } }, singleStudent); // same currency score, many wins const with_ = await request(app).get("/leaderboard"); const entry = with_.body.data.leaderboard[0]; @@ -435,21 +444,22 @@ describe("GET /leaderboard — chess record as its own column", () => { }); test("a student with no games shows a zeroed record, not a missing column", async () => { - Users.find.mockResolvedValue([{ _id: "3", username: "carol", school: null }]); - mockStatsFor({ carol: {} }); + const singleStudent = [{ _id: "3", username: "carol", school: null }]; + Users.find.mockResolvedValue(singleStudent); + mockScoresFor({ carol: { score: 0 } }, singleStudent); const entry = (await request(app).get("/leaderboard")).body.data.leaderboard[0]; expect(entry.chess_score).toBe(0); expect(entry.chess_record).toEqual({ wins: 0, draws: 0, losses: 0, gamesPlayed: 0 }); }); - test("sortBy=chess ranks by chess score, independent of engagement score", async () => { + test("sortBy=chess ranks by chess score, independent of currency score", async () => { Users.find.mockResolvedValue(STUDENTS); - mockStatsFor({ - // alice leads on engagement, bob leads on chess. - alice: { puzzleTimeHours: 100, streak: 10, badges: 5, chess: { wins: 0, draws: 0, losses: 3, gamesPlayed: 3, chessScore: 0 } }, - bob: { puzzleTimeHours: 1, chess: { wins: 9, draws: 0, losses: 0, gamesPlayed: 9, chessScore: 27 } }, - carol: { chess: { wins: 1, draws: 0, losses: 0, gamesPlayed: 1, chessScore: 3 } }, + mockScoresFor({ + // alice leads on currency score, bob leads on chess. + alice: { score: 100, chess: { wins: 0, draws: 0, losses: 3, gamesPlayed: 3, chessScore: 0 } }, + bob: { score: 1, chess: { wins: 9, draws: 0, losses: 0, gamesPlayed: 9, chessScore: 27 } }, + carol: { score: 0, chess: { wins: 1, draws: 0, losses: 0, gamesPlayed: 1, chessScore: 3 } }, }); const byChess = await request(app).get("/leaderboard?sortBy=chess"); diff --git a/middlewareNode/tests/ledgerService.test.js b/middlewareNode/tests/ledgerService.test.js index cb757004..861242b0 100644 --- a/middlewareNode/tests/ledgerService.test.js +++ b/middlewareNode/tests/ledgerService.test.js @@ -39,6 +39,7 @@ const { processEvent, applyLedgerEntry, getLifetimeEarnedOrZero, + getLifetimeEarnedMap, } = require("../src/services/ledgerService"); const userId = new mongoose.Types.ObjectId(); @@ -67,6 +68,56 @@ describe("getLifetimeEarnedOrZero", () => { }); }); +describe("getLifetimeEarnedMap", () => { + it("returns a Map keyed by userId string for every user with a balance document", async () => { + const alice = new mongoose.Types.ObjectId(); + const bob = new mongoose.Types.ObjectId(); + await UserBalance.create({ userId: alice, balance: 10, lifetimeEarned: 10 }); + await UserBalance.create({ userId: bob, balance: 25, lifetimeEarned: 25 }); + + const map = await getLifetimeEarnedMap([alice, bob]); + + expect(map.get(String(alice))).toBe(10); + expect(map.get(String(bob))).toBe(25); + }); + + it("has no entry for a user with no UserBalance document — callers must default to 0 themselves", async () => { + const noBalanceUser = new mongoose.Types.ObjectId(); + const map = await getLifetimeEarnedMap([noBalanceUser]); + + expect(map.has(String(noBalanceUser))).toBe(false); + expect(map.get(String(noBalanceUser)) || 0).toBe(0); + }); + + it("issues one query regardless of candidate list size (batched, not per-user)", async () => { + const ids = Array.from({ length: 20 }, () => new mongoose.Types.ObjectId()); + for (const id of ids) { + await UserBalance.create({ userId: id, balance: 1, lifetimeEarned: 1 }); + } + + const findSpy = jest.spyOn(UserBalance, "find"); + await getLifetimeEarnedMap(ids); + + expect(findSpy).toHaveBeenCalledTimes(1); + findSpy.mockRestore(); + }); + + it("returns an empty Map for an empty candidate list without querying the database", async () => { + // Not just "returns the right answer" — an empty candidate list (e.g. + // a filtered leaderboard query matching no students) must skip the + // query entirely. Without this, a caller in a context with no live + // DB connection mocked (leaderboard.security.test.js, which mocks + // Users.find but not UserBalance) would hang on a real query for + // nothing. + const findSpy = jest.spyOn(UserBalance, "find"); + const map = await getLifetimeEarnedMap([]); + + expect(map.size).toBe(0); + expect(findSpy).not.toHaveBeenCalled(); + findSpy.mockRestore(); + }); +}); + describe("applyLedgerEntry idempotency", () => { it("writes exactly one LedgerEntry and one UserBalance update for a new eventId", async () => { const result = await applyLedgerEntry({ diff --git a/middlewareNode/tests/lessons.currencyEmit.test.js b/middlewareNode/tests/lessons.currencyEmit.test.js new file mode 100644 index 00000000..98ec5a64 --- /dev/null +++ b/middlewareNode/tests/lessons.currencyEmit.test.js @@ -0,0 +1,159 @@ +/** + * Real-database integration test for routes/lessons.js's + * /updateLessonCompletion — specifically the currency-event emit added + * for Jimmy's ledger plan (Week 2: "emit lesson.completed ... one line, + * no currency logic inline"). + * + * Not a full route test suite (this route had none before this change, + * a pre-existing gap this file doesn't attempt to close) — scoped to the + * one property this session's change needs proven: the emit fires only + * on genuine forward progress, never on a no-op re-request and never for + * an unauthenticated guest. + * + * Uses mongodb-memory-server (matching tests/ledgerService.test.js and + * friends) rather than mocking getDb()/passport, since this route talks + * to the database directly via a raw driver handle (getDb()), not a + * Mongoose model — a mock here would mean re-implementing Mongo query + * semantics by hand instead of exercising the real thing. + */ + +const { MongoMemoryServer } = require("mongodb-memory-server"); +const mongoose = require("mongoose"); + +jest.setTimeout(60000); + +let mongod; + +beforeAll(async () => { + mongod = await MongoMemoryServer.create({ instance: { launchTimeout: 30000 } }); + await mongoose.connect(mongod.getUri() + "ystem"); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongod.stop(); +}); + +afterEach(async () => { + const collections = mongoose.connection.collections; + await Promise.all(Object.values(collections).map((c) => c.deleteMany({}))); + jest.restoreAllMocks(); +}); + +// Bypasses the real JWT verification (irrelevant to this test) but keeps +// req.user a real, DB-backed document the same way passport's own +// verify callback does (see config/passport.js) — req.user._id has to be +// a real ObjectId for emitLessonCompleted to build a usable eventId. +jest.mock("passport", () => ({ + authenticate: (_strategy, _opts, callback) => (req, res, next) => { + if (req.headers["x-test-guest"] === "true") { + callback(null, false, null); + } else { + callback(null, req.__testUser, null); + } + }, +})); + +const express = require("express"); +const request = require("supertest"); +const lessonsRouter = require("../src/routes/lessons"); +const ActionEvent = require("../src/models/actionEvent"); + +const app = express(); +app.use(express.json()); +// Injects req.__testUser before passport's mocked authenticate reads it — +// simulates "this is the already-authenticated user" without a real JWT. +app.use((req, _res, next) => { + if (req.headers["x-test-user-id"]) { + req.__testUser = { _id: req.headers["x-test-user-id"], username: req.headers["x-test-username"] }; + } + next(); +}); +app.use("/lessons", lessonsRouter); + +async function seedUser({ username, piece, lessonNumber }) { + const usersCollection = mongoose.connection.collection("users"); + const result = await usersCollection.insertOne({ + username, + lessonsCompleted: [{ piece, lessonNumber }], + }); + return result.insertedId; +} + +describe("GET /lessons/updateLessonCompletion — currency emit", () => { + it("emits lesson.completed when the write is genuine forward progress", async () => { + const userId = await seedUser({ username: "alice", piece: "The Fork", lessonNumber: 0 }); + + const res = await request(app) + .get("/lessons/updateLessonCompletion") + .query({ piece: "The Fork", lessonNum: "1" }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "alice"); + + expect(res.status).toBe(200); + + // The emit is fire-and-forget (a .catch(), not awaited by the route) + // — give it a tick to land before asserting. + await new Promise((r) => setImmediate(r)); + + const stored = await ActionEvent.findOne({ actionKey: "lesson.completed" }); + expect(stored).not.toBeNull(); + expect(stored.userId.toString()).toBe(userId.toString()); + expect(stored.metadata).toEqual({ piece: "The Fork", lessonNum: 1 }); + }); + + it("does NOT emit when the request doesn't advance progress (304 branch)", async () => { + // lessonNumber already at 5; requesting lessonNum=2 (index of an + // earlier lesson) can't win the $lt comparison, so modifiedCount is 0. + const userId = await seedUser({ username: "bob", piece: "The Fork", lessonNumber: 5 }); + + const res = await request(app) + .get("/lessons/updateLessonCompletion") + .query({ piece: "The Fork", lessonNum: "2" }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "bob"); + + expect(res.status).toBe(304); + + await new Promise((r) => setImmediate(r)); + + expect(await ActionEvent.countDocuments({ actionKey: "lesson.completed" })).toBe(0); + }); + + it("does NOT emit for a repeated request that already succeeded once (idempotent, not double-counted)", async () => { + const userId = await seedUser({ username: "carol", piece: "The Fork", lessonNumber: 0 }); + + const query = { piece: "The Fork", lessonNum: "1" }; + const headers = { "x-test-user-id": userId.toString(), "x-test-username": "carol" }; + + const first = await request(app).get("/lessons/updateLessonCompletion").query(query).set(headers); + expect(first.status).toBe(200); + await new Promise((r) => setImmediate(r)); + + // Re-sending the exact same request: the DB write itself is now a + // no-op (lessonNumber is already 1, not < 1), so this hits the 304 + // branch and must not add a second event even if it somehow did. + const second = await request(app).get("/lessons/updateLessonCompletion").query(query).set(headers); + expect(second.status).toBe(304); + await new Promise((r) => setImmediate(r)); + + expect(await ActionEvent.countDocuments({ actionKey: "lesson.completed" })).toBe(1); + }); + + it("never emits for a guest (unauthenticated) request", async () => { + // "The Fork" doesn't match the guest seed list's exact full name + // ("The Fork Use the fork, Luke"), so this correctly 404s — the point + // under test isn't the guest branch's own behavior (untouched by this + // change), only that no ActionEvent is ever created for it either way. + const res = await request(app) + .get("/lessons/updateLessonCompletion") + .query({ piece: "The Fork", lessonNum: "1" }) + .set("x-test-guest", "true") + .set("x-forwarded-for", "203.0.113.5"); + + expect(res.status).toBe(404); + await new Promise((r) => setImmediate(r)); + + expect(await ActionEvent.countDocuments({})).toBe(0); + }); +}); From fb6a88a0129ee00296f62675c9ab5cdfef109815 Mon Sep 17 00:00:00 2001 From: karthikeya veginati Date: Thu, 24 Sep 2026 13:33:10 -0500 Subject: [PATCH 4/9] docs(LeaderboardModal): correct stale score-field comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Row.score JSDoc still described the old time/streak/activities/ badges weighted formula this session's leaderboard swap replaced. Updated to describe what score actually is now (currency lifetime earned) — no behavior change, the response contract and UI are unaffected since the field name/type/shape didn't change. Co-Authored-By: Claude Sonnet 5 --- .../student/student-profile/Modals/LeaderboardModal.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/react-ystemandchess/src/features/student/student-profile/Modals/LeaderboardModal.tsx b/react-ystemandchess/src/features/student/student-profile/Modals/LeaderboardModal.tsx index 31d402d5..a065da31 100644 --- a/react-ystemandchess/src/features/student/student-profile/Modals/LeaderboardModal.tsx +++ b/react-ystemandchess/src/features/student/student-profile/Modals/LeaderboardModal.tsx @@ -28,7 +28,13 @@ type Row = { rank: number; name: string; school: string; - /** Engagement score — time, streak, activities, badges. */ + /** + * Currency ledger standing (lifetime currency earned from engaging + * actions — lessons, puzzles, etc.), not spendable balance. See + * middlewareNode/src/routes/leaderboard.js and services/ledgerService.js. + * Previously a weighted time/streak/activities/badges formula; replaced + * by the currency rollout plan. + */ score: number; /** * Student-vs-student chess score, kept as its own column on purpose. From ac15801787ddff0a5f737f9dfb1db56ec0a74474 Mon Sep 17 00:00:00 2001 From: JimmyWu7 <104340474+JimmyWu7@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:28:59 -0400 Subject: [PATCH 5/9] feat: emit currency events for lessons and puzzles --- middlewareNode/src/routes/puzzles.js | 72 +++++++- middlewareNode/src/services/currencyEvents.js | 25 ++- .../tests/lessons.currencyEmit.test.js | 74 ++++++-- .../tests/puzzles.currencyEmit.test.js | 164 ++++++++++++++++++ 4 files changed, 313 insertions(+), 22 deletions(-) create mode 100644 middlewareNode/tests/puzzles.currencyEmit.test.js diff --git a/middlewareNode/src/routes/puzzles.js b/middlewareNode/src/routes/puzzles.js index f8f21ce6..a301ca0c 100644 --- a/middlewareNode/src/routes/puzzles.js +++ b/middlewareNode/src/routes/puzzles.js @@ -1,9 +1,9 @@ /** * Puzzles Routes - * + * * API endpoints for retrieving chess puzzles from the database. * Puzzles are sourced from Lichess and stored in MongoDB. - * + * * Features: * - Get all puzzles * - Get random selection of puzzles @@ -11,15 +11,17 @@ */ const express = require("express"); +const passport = require("passport"); const router = express.Router(); const puzzles = require("../models/puzzles"); +const { emitPuzzleSolved } = require("../services/currencyEvents"); /** * GET /puzzles/list - * + * * Retrieves all chess puzzles from the database. * Returns array of puzzle objects without MongoDB _id field. - * + * * @returns {Array} Array of all puzzles */ router.get("/list", async (req, res) => { @@ -34,19 +36,21 @@ router.get("/list", async (req, res) => { /** * GET /puzzles/random - * + * * Retrieves a random selection of chess puzzles. * Useful for providing variety in puzzle practice sessions. - * + * * Query Parameters: * - limit: Number of random puzzles to return (default: 20) - * + * * @returns {Array} Array of randomly selected puzzles */ router.get("/random", async (req, res) => { try { const limit = parseInt(req.query.limit) || 20; - const puzzlesArray = await puzzles.aggregate([{ $sample: { size: limit } }]); + const puzzlesArray = await puzzles.aggregate([ + { $sample: { size: limit } }, + ]); res.status(200).json(puzzlesArray); } catch (error) { console.error(error.message); @@ -54,4 +58,54 @@ router.get("/random", async (req, res) => { } }); -module.exports = router; \ No newline at end of file +/** + * POST /puzzles/solved + * + * Records a successfully completed puzzle as a currency-earning action. + * + * Body: + * - puzzleId: ID of the completed puzzle + * + * Only authenticated users can earn currency. + * Guests can still play puzzles, but there is no account to credit. + */ +router.post( + "/solved", + passport.authenticate("jwt", { session: false }), + async (req, res) => { + try { + const { puzzleId } = req.body; + + if (!puzzleId) { + return res.status(400).json({ + error: "puzzleId is required", + }); + } + + const puzzle = await puzzles.findOne({ puzzleId }); + + if (!puzzle) { + return res.status(404).json({ + error: "Puzzle not found", + }); + } + + const result = await emitPuzzleSolved({ + userId: req.user._id, + puzzleId, + }); + + return res.status(200).json(result); + } catch (error) { + console.error( + "currencyEvents: failed to emit puzzle.solved:", + error.message, + ); + return res.status(500).json({ + error: "Failed to record puzzle completion", + }); + } + }, +); + +module.exports = router; diff --git a/middlewareNode/src/services/currencyEvents.js b/middlewareNode/src/services/currencyEvents.js index 087408aa..3a7e1cde 100644 --- a/middlewareNode/src/services/currencyEvents.js +++ b/middlewareNode/src/services/currencyEvents.js @@ -37,7 +37,13 @@ const DUPLICATE_KEY_ERROR = 11000; * fresh insert, { emitted: false, duplicate: true } for an already-seen * eventId. */ -async function emit({ eventId, actionKey, userId, metadata = {}, occurredAt = new Date() }) { +async function emit({ + eventId, + actionKey, + userId, + metadata = {}, + occurredAt = new Date(), +}) { try { await ActionEvent.create({ eventId, @@ -79,7 +85,24 @@ async function emitLessonCompleted({ userId, piece, lessonNum, occurredAt }) { }); } +/** + * Emits puzzle.solved after a student successfully completes a puzzle. + * + * eventId is `puzzle::` — deterministic per + * (student, puzzle) pair so a retry cannot award the same puzzle twice. + */ +async function emitPuzzleSolved({ userId, puzzleId, occurredAt }) { + return emit({ + eventId: `puzzle:${userId}:${puzzleId}`, + actionKey: "puzzle.solved", + userId, + metadata: { puzzleId }, + occurredAt, + }); +} + module.exports = { emit, emitLessonCompleted, + emitPuzzleSolved, }; diff --git a/middlewareNode/tests/lessons.currencyEmit.test.js b/middlewareNode/tests/lessons.currencyEmit.test.js index 98ec5a64..a5add64d 100644 --- a/middlewareNode/tests/lessons.currencyEmit.test.js +++ b/middlewareNode/tests/lessons.currencyEmit.test.js @@ -25,7 +25,9 @@ jest.setTimeout(60000); let mongod; beforeAll(async () => { - mongod = await MongoMemoryServer.create({ instance: { launchTimeout: 30000 } }); + mongod = await MongoMemoryServer.create({ + instance: { launchTimeout: 30000 }, + }); await mongoose.connect(mongod.getUri() + "ystem"); }); @@ -65,7 +67,10 @@ app.use(express.json()); // simulates "this is the already-authenticated user" without a real JWT. app.use((req, _res, next) => { if (req.headers["x-test-user-id"]) { - req.__testUser = { _id: req.headers["x-test-user-id"], username: req.headers["x-test-username"] }; + req.__testUser = { + _id: req.headers["x-test-user-id"], + username: req.headers["x-test-username"], + }; } next(); }); @@ -80,9 +85,29 @@ async function seedUser({ username, piece, lessonNumber }) { return result.insertedId; } +async function waitForActionEvent(query, timeoutMs = 2000) { + const start = Date.now(); + + while (Date.now() - start < timeoutMs) { + const event = await ActionEvent.findOne(query); + + if (event) { + return event; + } + + await new Promise((resolve) => setTimeout(resolve, 10)); + } + + return null; +} + describe("GET /lessons/updateLessonCompletion — currency emit", () => { it("emits lesson.completed when the write is genuine forward progress", async () => { - const userId = await seedUser({ username: "alice", piece: "The Fork", lessonNumber: 0 }); + const userId = await seedUser({ + username: "alice", + piece: "The Fork", + lessonNumber: 0, + }); const res = await request(app) .get("/lessons/updateLessonCompletion") @@ -96,7 +121,7 @@ describe("GET /lessons/updateLessonCompletion — currency emit", () => { // — give it a tick to land before asserting. await new Promise((r) => setImmediate(r)); - const stored = await ActionEvent.findOne({ actionKey: "lesson.completed" }); + const stored = await waitForActionEvent({ actionKey: "lesson.completed" }); expect(stored).not.toBeNull(); expect(stored.userId.toString()).toBe(userId.toString()); expect(stored.metadata).toEqual({ piece: "The Fork", lessonNum: 1 }); @@ -105,7 +130,11 @@ describe("GET /lessons/updateLessonCompletion — currency emit", () => { it("does NOT emit when the request doesn't advance progress (304 branch)", async () => { // lessonNumber already at 5; requesting lessonNum=2 (index of an // earlier lesson) can't win the $lt comparison, so modifiedCount is 0. - const userId = await seedUser({ username: "bob", piece: "The Fork", lessonNumber: 5 }); + const userId = await seedUser({ + username: "bob", + piece: "The Fork", + lessonNumber: 5, + }); const res = await request(app) .get("/lessons/updateLessonCompletion") @@ -117,27 +146,48 @@ describe("GET /lessons/updateLessonCompletion — currency emit", () => { await new Promise((r) => setImmediate(r)); - expect(await ActionEvent.countDocuments({ actionKey: "lesson.completed" })).toBe(0); + expect( + await ActionEvent.countDocuments({ actionKey: "lesson.completed" }), + ).toBe(0); }); it("does NOT emit for a repeated request that already succeeded once (idempotent, not double-counted)", async () => { - const userId = await seedUser({ username: "carol", piece: "The Fork", lessonNumber: 0 }); + const userId = await seedUser({ + username: "carol", + piece: "The Fork", + lessonNumber: 0, + }); const query = { piece: "The Fork", lessonNum: "1" }; - const headers = { "x-test-user-id": userId.toString(), "x-test-username": "carol" }; + const headers = { + "x-test-user-id": userId.toString(), + "x-test-username": "carol", + }; - const first = await request(app).get("/lessons/updateLessonCompletion").query(query).set(headers); + const first = await request(app) + .get("/lessons/updateLessonCompletion") + .query(query) + .set(headers); expect(first.status).toBe(200); - await new Promise((r) => setImmediate(r)); + + const firstEvent = await waitForActionEvent({ + actionKey: "lesson.completed", + }); + expect(firstEvent).not.toBeNull(); // Re-sending the exact same request: the DB write itself is now a // no-op (lessonNumber is already 1, not < 1), so this hits the 304 // branch and must not add a second event even if it somehow did. - const second = await request(app).get("/lessons/updateLessonCompletion").query(query).set(headers); + const second = await request(app) + .get("/lessons/updateLessonCompletion") + .query(query) + .set(headers); expect(second.status).toBe(304); await new Promise((r) => setImmediate(r)); - expect(await ActionEvent.countDocuments({ actionKey: "lesson.completed" })).toBe(1); + expect( + await ActionEvent.countDocuments({ actionKey: "lesson.completed" }), + ).toBe(1); }); it("never emits for a guest (unauthenticated) request", async () => { diff --git a/middlewareNode/tests/puzzles.currencyEmit.test.js b/middlewareNode/tests/puzzles.currencyEmit.test.js new file mode 100644 index 00000000..f9edf95a --- /dev/null +++ b/middlewareNode/tests/puzzles.currencyEmit.test.js @@ -0,0 +1,164 @@ +/** + * Real-database integration test for routes/puzzles.js's + * POST /puzzles/solved — specifically the currency-event emit. + * + * Scoped to the property this session's change needs proven: + * authenticated puzzle completion creates a puzzle.solved ActionEvent, + * while invalid/guest requests do not. + */ + +const { MongoMemoryServer } = require("mongodb-memory-server"); +const mongoose = require("mongoose"); + +jest.setTimeout(60000); + +let mongod; + +beforeAll(async () => { + mongod = await MongoMemoryServer.create({ + instance: { launchTimeout: 30000 }, + }); + await mongoose.connect(mongod.getUri() + "ystem"); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongod.stop(); +}); + +afterEach(async () => { + const collections = mongoose.connection.collections; + await Promise.all(Object.values(collections).map((c) => c.deleteMany({}))); + jest.restoreAllMocks(); +}); + +jest.mock("passport", () => ({ + authenticate: (_strategy, _opts, callback) => (req, res, next) => { + if (req.headers["x-test-guest"] === "true") { + return callback(null, false, null); + } + + req.user = req.__testUser; + return next(); + }, +})); + +const express = require("express"); +const request = require("supertest"); +const puzzlesRouter = require("../src/routes/puzzles"); +const ActionEvent = require("../src/models/actionEvent"); +const puzzles = require("../src/models/puzzles"); + +const app = express(); +app.use(express.json()); + +app.use((req, _res, next) => { + if (req.headers["x-test-user-id"]) { + req.__testUser = { + _id: req.headers["x-test-user-id"], + username: req.headers["x-test-username"], + }; + } + next(); +}); + +app.use("/puzzles", puzzlesRouter); + +async function waitForActionEvent(query, timeoutMs = 2000) { + const start = Date.now(); + + while (Date.now() - start < timeoutMs) { + const event = await ActionEvent.findOne(query); + + if (event) { + return event; + } + + await new Promise((resolve) => setTimeout(resolve, 10)); + } + + return null; +} + +async function seedPuzzle(puzzleId) { + await puzzles.create({ + puzzleId, + FEN: "8/8/8/8/8/8/8/K6k w - - 0 1", + moves: "a1a2", + }); +} + +describe("POST /puzzles/solved — currency emit", () => { + it("emits puzzle.solved for an authenticated puzzle completion", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-1"; + + await seedPuzzle(puzzleId); + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "alice"); + + expect(res.status).toBe(200); + + const stored = await waitForActionEvent({ + actionKey: "puzzle.solved", + }); + + expect(stored).not.toBeNull(); + expect(stored.eventId).toBe(`puzzle:${userId}:${puzzleId}`); + expect(stored.userId.toString()).toBe(userId.toString()); + expect(stored.metadata).toEqual({ puzzleId }); + }); + + it("does NOT emit when the puzzle does not exist", async () => { + const userId = new mongoose.Types.ObjectId(); + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId: "does-not-exist" }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "bob"); + + expect(res.status).toBe(404); + + await new Promise((resolve) => setImmediate(resolve)); + + expect(await ActionEvent.countDocuments({})).toBe(0); + }); + + it("does NOT emit when puzzleId is missing", async () => { + const userId = new mongoose.Types.ObjectId(); + + const res = await request(app) + .post("/puzzles/solved") + .send({}) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "carol"); + + expect(res.status).toBe(400); + + await new Promise((resolve) => setImmediate(resolve)); + + expect(await ActionEvent.countDocuments({})).toBe(0); + }); + + it("does NOT emit for a guest request", async () => { + const puzzleId = "guest-puzzle"; + + await seedPuzzle(puzzleId); + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId }) + .set("x-test-guest", "true"); + + expect(res.status).not.toBe(200); + + await new Promise((resolve) => setImmediate(resolve)); + + expect(await ActionEvent.countDocuments({})).toBe(0); + }); +}); From 6d91d9cb00fa3ce375fdedd748ad5323d68dd846 Mon Sep 17 00:00:00 2001 From: karthikeya veginati Date: Thu, 1 Oct 2026 17:49:19 -0500 Subject: [PATCH 6/9] fix(puzzles): require and verify solved moves before awarding currency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on this PR: POST /puzzles/solved previously awarded currency based solely on an authenticated caller POSTing a puzzleId that existed in the catalog, with no proof the puzzle was actually solved. Any authenticated user could loop over every puzzleId in the catalog and farm currency for puzzles never attempted. - routes/puzzles.js: the route now requires a `moves` array (the UCI move sequence the player submitted) and verifies it positionally against the puzzle's stored `moves` answer key before emitting puzzle.solved. moveMatches() mirrors Puzzles.tsx's own handlePlayerMove leniency exactly (a 4-char move is accepted against a 5-char/promotion solution) so a move the client UI already treated as correct can't be rejected server-side as a mismatch. - Puzzles.tsx: adds playedMovesRef to accumulate the player's verified moves as they're confirmed correct (moveListRef shrinks via .shift() as the puzzle progresses, so it's empty by completion and can't be used for this). Wires a new reportPuzzleSolved() call at the existing "puzzle completed" point, fire-and-forget so a network hiccup doesn't block the success-modal UX. Skipped entirely for guests, matching the backend's own "no account to credit" reasoning. - Tests: added coverage for the move-mismatch rejection (the actual regression case), partial/missing/empty moves, and both directions of the promotion leniency (4-char submission against a 5-char solution accepted; wrong promotion piece at the same length rejected). Noted limitation, not fully closed by this fix: puzzle.moves (the answer key) is already sent to the client to drive the interactive play-through, so a client that reads it directly and submits it verbatim without playing will still pass. This raises the bar from "needs nothing" to "needs the real solution string" — closing the remaining gap would need either not shipping the full solution to the client up front, or a different completion-proof mechanism entirely; flagging as further follow-up rather than silently claiming this is a complete fix. 336/336 backend tests passing (330 baseline + 6 new), TypeScript compiles clean on the frontend change. Co-Authored-By: Claude Sonnet 5 --- middlewareNode/src/routes/puzzles.js | 68 ++++++++- .../tests/puzzles.currencyEmit.test.js | 135 ++++++++++++++++-- .../src/features/puzzles/Puzzles.tsx | 54 +++++++ 3 files changed, 246 insertions(+), 11 deletions(-) diff --git a/middlewareNode/src/routes/puzzles.js b/middlewareNode/src/routes/puzzles.js index a301ca0c..caa7d042 100644 --- a/middlewareNode/src/routes/puzzles.js +++ b/middlewareNode/src/routes/puzzles.js @@ -58,13 +58,66 @@ router.get("/random", async (req, res) => { } }); +/** + * True if a submitted move matches the puzzle's expected move for that + * position, using the exact same leniency Puzzles.tsx's handlePlayerMove + * already applies client-side: + * + * playerAttemptedMove === expectedPlayerMove || + * playerAttemptedMove === expectedPlayerMove.substring(0, 4) + * + * i.e. a 4-character move (no promotion piece specified) is accepted even + * when the real solution is 5 characters (e.g. "e7e8q") — the client + * already lets a student complete a promotion puzzle without explicitly + * picking a piece, so the server has to accept that same shape or it + * would reject a move the client UI already treated as solved. Comparison + * is case-insensitive to match how the client builds its own move string. + */ +function moveMatches(submitted, expected) { + const s = (submitted || "").toLowerCase(); + const e = (expected || "").toLowerCase(); + return s === e || s === e.substring(0, 4); +} + +/** + * Verifies a client-submitted move sequence against a puzzle's stored + * answer key (space-separated UCI moves). Returns true only if the + * sequences have the same length and every move matches positionally. + * + * This isn't a complete defense — a client that reads the puzzle's own + * `moves` field (already sent to the client to drive the interactive + * play-through) and submits it verbatim without actually playing will + * still pass. What it does stop is the cheaper, more likely attack this + * route originally allowed outright: looping over puzzleIds with no + * solve data at all. Raising the bar to "must know the real solution + * string," not "must have nothing." + */ +function movesMatch(submitted, expected) { + if (!Array.isArray(submitted)) return false; + + const expectedMoves = (expected || "").trim().split(/\s+/).filter(Boolean); + if (submitted.length === 0 || submitted.length !== expectedMoves.length) { + return false; + } + + return expectedMoves.every((expectedMove, i) => moveMatches(submitted[i], expectedMove)); +} + /** * POST /puzzles/solved * - * Records a successfully completed puzzle as a currency-earning action. + * Records a successfully completed puzzle as a currency-earning action — + * but only once the submitted move sequence is verified against the + * puzzle's stored answer key. A bare puzzleId is no longer sufficient: + * without this, any authenticated caller could loop over every puzzleId + * in the catalog and farm currency for puzzles never attempted. See the + * limitation noted on movesMatch() above for what this does and doesn't + * defend against. * * Body: * - puzzleId: ID of the completed puzzle + * - moves: array of UCI move strings the player submitted, in order, + * matching the puzzle's own `moves` answer key exactly * * Only authenticated users can earn currency. * Guests can still play puzzles, but there is no account to credit. @@ -74,13 +127,18 @@ router.post( passport.authenticate("jwt", { session: false }), async (req, res) => { try { - const { puzzleId } = req.body; + const { puzzleId, moves } = req.body; if (!puzzleId) { return res.status(400).json({ error: "puzzleId is required", }); } + if (!Array.isArray(moves) || moves.length === 0) { + return res.status(400).json({ + error: "moves is required and must be a non-empty array of UCI move strings", + }); + } const puzzle = await puzzles.findOne({ puzzleId }); @@ -90,6 +148,12 @@ router.post( }); } + if (!movesMatch(moves, puzzle.moves)) { + return res.status(400).json({ + error: "Submitted moves do not match the puzzle's solution", + }); + } + const result = await emitPuzzleSolved({ userId: req.user._id, puzzleId, diff --git a/middlewareNode/tests/puzzles.currencyEmit.test.js b/middlewareNode/tests/puzzles.currencyEmit.test.js index f9edf95a..c0583d52 100644 --- a/middlewareNode/tests/puzzles.currencyEmit.test.js +++ b/middlewareNode/tests/puzzles.currencyEmit.test.js @@ -4,7 +4,11 @@ * * Scoped to the property this session's change needs proven: * authenticated puzzle completion creates a puzzle.solved ActionEvent, - * while invalid/guest requests do not. + * while invalid/guest requests do not — AND, critically, that completion + * actually requires submitting the puzzle's real solution. The route + * originally accepted a bare puzzleId with no proof of solving it; these + * tests include the regression coverage for that (see "does NOT emit when + * submitted moves don't match the puzzle's solution" below). */ const { MongoMemoryServer } = require("mongodb-memory-server"); @@ -80,16 +84,18 @@ async function waitForActionEvent(query, timeoutMs = 2000) { return null; } -async function seedPuzzle(puzzleId) { +const SOLUTION_MOVES = "e1g1 e8g8"; + +async function seedPuzzle(puzzleId, moves = SOLUTION_MOVES) { await puzzles.create({ puzzleId, - FEN: "8/8/8/8/8/8/8/K6k w - - 0 1", - moves: "a1a2", + FEN: "r3k2r/ppp2ppp/2n5/1B1p4/3P4/2P5/PP3PPP/R3K2R w KQkq - 0 1", + moves, }); } describe("POST /puzzles/solved — currency emit", () => { - it("emits puzzle.solved for an authenticated puzzle completion", async () => { + it("emits puzzle.solved when the submitted moves match the puzzle's solution", async () => { const userId = new mongoose.Types.ObjectId(); const puzzleId = "test-puzzle-1"; @@ -97,7 +103,7 @@ describe("POST /puzzles/solved — currency emit", () => { const res = await request(app) .post("/puzzles/solved") - .send({ puzzleId }) + .send({ puzzleId, moves: ["e1g1", "e8g8"] }) .set("x-test-user-id", userId.toString()) .set("x-test-username", "alice"); @@ -113,12 +119,123 @@ describe("POST /puzzles/solved — currency emit", () => { expect(stored.metadata).toEqual({ puzzleId }); }); + it("accepts a 4-char submitted move against a 5-char (promotion) solution, matching the client's own leniency", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-promo-lenient"; + + // Solution requires promoting to a queen; Puzzles.tsx's + // handlePlayerMove accepts the move even if the player didn't specify + // a promotion piece (playerAttemptedMove === expectedMove.substring(0,4)), + // so the server has to accept that same shape or it would reject a + // move the client UI itself already treated as solved. + await seedPuzzle(puzzleId, "e7e8q"); + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId, moves: ["e7e8"] }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "dave"); + + expect(res.status).toBe(200); + }); + + it("rejects the wrong promotion piece even though the first 4 chars match", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-wrong-promo"; + + await seedPuzzle(puzzleId, "e7e8q"); // solution promotes to queen + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId, moves: ["e7e8r"] }) // submitted promotes to rook + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "ivan"); + + expect(res.status).toBe(400); + }); + + it("does NOT emit when the submitted moves don't match the puzzle's solution", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-wrong-moves"; + + await seedPuzzle(puzzleId); + + // This is the regression case: a caller who knows (or guesses) a real + // puzzleId but never actually played — or played incorrectly — must + // be rejected, not credited. + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId, moves: ["a1a2", "a7a8"] }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "eve"); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/do not match/i); + + await new Promise((resolve) => setImmediate(resolve)); + expect(await ActionEvent.countDocuments({})).toBe(0); + }); + + it("does NOT emit when fewer moves are submitted than the solution requires", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-partial"; + + await seedPuzzle(puzzleId); // solution is two moves + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId, moves: ["e1g1"] }) // only the first move + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "frank"); + + expect(res.status).toBe(400); + + await new Promise((resolve) => setImmediate(resolve)); + expect(await ActionEvent.countDocuments({})).toBe(0); + }); + + it("does NOT emit when moves is missing — a bare puzzleId is no longer sufficient", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-no-moves"; + + await seedPuzzle(puzzleId); + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "grace"); + + expect(res.status).toBe(400); + + await new Promise((resolve) => setImmediate(resolve)); + expect(await ActionEvent.countDocuments({})).toBe(0); + }); + + it("does NOT emit when moves is an empty array", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-empty-moves"; + + await seedPuzzle(puzzleId); + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId, moves: [] }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "heidi"); + + expect(res.status).toBe(400); + + await new Promise((resolve) => setImmediate(resolve)); + expect(await ActionEvent.countDocuments({})).toBe(0); + }); + it("does NOT emit when the puzzle does not exist", async () => { const userId = new mongoose.Types.ObjectId(); const res = await request(app) .post("/puzzles/solved") - .send({ puzzleId: "does-not-exist" }) + .send({ puzzleId: "does-not-exist", moves: ["e1g1", "e8g8"] }) .set("x-test-user-id", userId.toString()) .set("x-test-username", "bob"); @@ -134,7 +251,7 @@ describe("POST /puzzles/solved — currency emit", () => { const res = await request(app) .post("/puzzles/solved") - .send({}) + .send({ moves: ["e1g1", "e8g8"] }) .set("x-test-user-id", userId.toString()) .set("x-test-username", "carol"); @@ -152,7 +269,7 @@ describe("POST /puzzles/solved — currency emit", () => { const res = await request(app) .post("/puzzles/solved") - .send({ puzzleId }) + .send({ puzzleId, moves: ["e1g1", "e8g8"] }) .set("x-test-guest", "true"); expect(res.status).not.toBe(200); diff --git a/react-ystemandchess/src/features/puzzles/Puzzles.tsx b/react-ystemandchess/src/features/puzzles/Puzzles.tsx index 3a22153a..fa64ae49 100644 --- a/react-ystemandchess/src/features/puzzles/Puzzles.tsx +++ b/react-ystemandchess/src/features/puzzles/Puzzles.tsx @@ -56,6 +56,12 @@ const Puzzles: React.FC = ({ // Refs const chessBoardRef = useRef(null); const moveListRef = useRef([]); + // Accumulates the player's own moves as they're confirmed correct, + // separately from moveListRef (which shrinks via .shift() as the + // remaining solution is consumed, so it's empty by the time a puzzle + // completes). This is what gets submitted to POST /puzzles/solved for + // server-side verification. + const playedMovesRef = useRef([]); const isPuzzleEndRef = useRef(false); const currentPuzzleRef = useRef(null); const isInitializingRef = useRef(false); @@ -161,6 +167,7 @@ const Puzzles: React.FC = ({ const firstPuzzle = puzzles[0]; currentPuzzleRef.current = firstPuzzle; moveListRef.current = firstPuzzle?.Moves?.split(" ") || []; + playedMovesRef.current = []; if (moveListRef.current.length === 0) { console.warn("No valid moves in initial puzzle:", firstPuzzle); @@ -206,6 +213,7 @@ const Puzzles: React.FC = ({ setCurrentFEN(normalizedFen); moveListRef.current = puzzle?.Moves?.split(" ") || []; + playedMovesRef.current = []; isPuzzleEndRef.current = false; setHighlightSquares([]); @@ -293,6 +301,45 @@ const Puzzles: React.FC = ({ } }; + /** + * Reports a completed puzzle to POST /puzzles/solved so the student + * earns currency for it (see middlewareNode/src/routes/puzzles.js). + * Submits the move sequence actually played — verified server-side + * against the puzzle's stored solution — rather than just the + * puzzleId, so a request can't claim credit for a puzzle it never + * solved. Guests never earn currency (no account to credit), so this + * is skipped entirely for them rather than sent and rejected. + */ + const reportPuzzleSolved = async () => { + if (status === "guest" || !cookies.login) return; + + const puzzleId = currentPuzzleRef.current?.puzzleId; + if (!puzzleId) return; + + try { + const res = await fetch(`${environment.urls.middlewareURL}/puzzles/solved`, { + method: "POST", + headers: { + Authorization: `Bearer ${cookies.login}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ + puzzleId, + moves: playedMovesRef.current, + }), + }); + if (!res.ok) { + const body = await res.json().catch(() => ({})); + console.error("Failed to record puzzle completion:", body.error || res.status); + } + } catch (error) { + // Currency crediting is a side effect of completing the puzzle, not + // a prerequisite for the UI's own success state — a network error + // here shouldn't block or roll back the "Puzzle completed" modal. + console.error("Failed to record puzzle completion:", error); + } + }; + const handlePlayerMove = (move: Move) => { if ( isPuzzleEndRef.current || @@ -310,6 +357,11 @@ const Puzzles: React.FC = ({ playerAttemptedMove === expectedPlayerMove.substring(0, 4); if (isCorrect) { + // Record the verified move (the expected UCI string, not the raw + // attempt, so promotion suffixes etc. match exactly what the + // puzzle's own solution — and the server-side check — expect) before + // shifting it off moveListRef. + playedMovesRef.current.push(expectedPlayerMove); moveListRef.current.shift(); setHighlightSquares([move.from, move.to]); @@ -326,6 +378,7 @@ const Puzzles: React.FC = ({ if (moveListRef.current.length === 0) { isPuzzleEndRef.current = true; socket.sendMessage("puzzle completed"); + reportPuzzleSolved(); setTimeout(() => { setModal({ type: "success", @@ -368,6 +421,7 @@ const Puzzles: React.FC = ({ if (data && data.type === "puzzle_data") { if (status === "guest") { moveListRef.current = data.moves?.split(" ") || []; + playedMovesRef.current = []; setThemeList(data.themes?.split(" ") || []); currentPuzzleRef.current = { FEN: data.fen, From 6bf9500060ea6cc0f3507776f2a106fef0f4258b Mon Sep 17 00:00:00 2001 From: karthikeya veginati Date: Thu, 1 Oct 2026 18:31:21 -0500 Subject: [PATCH 7/9] fix(ledger): stop the transactional path from retrying indefinitely on a duplicate event Resolves the last open item from Karthik's ledger plan's Week 0 gate: the LEDGER_USE_TRANSACTIONS=true branch had code but had never actually been exercised by a test, since every other test in this suite runs against a standalone mongodb-memory-server instance (which doesn't support transactions at all). Added tests/ledgerService.transactions.test.js, using MongoMemoryReplSet (a real, if minimal, replica set) to actually run that branch. Doing so immediately surfaced a genuine bug: the old code caught a duplicate-key error INSIDE session.withTransaction()'s callback and returned normally. The MongoDB driver's withTransaction() retries its callback on a transient error, and apparently cannot distinguish "the callback decided to do nothing" from "the callback needs retrying" when nothing was actually committed -- in an isolated repro this produced hundreds of silent retries of the same duplicate insert before eventually failing a test's timeout, rather than the near-instant no-op the single-document fallback already achieves for the same case. Fix: let the duplicate-key error propagate OUT of the transaction callback entirely. This aborts the (otherwise-empty) transaction immediately -- cheap and correct, since nothing else was written -- and is caught once, after withTransaction() itself settles, mirroring the try/catch shape the non-transactional fallback already uses. This does not answer the actual Week 0 question of whether the real deployment's MongoDB is a replica set -- that still needs someone to run rs.status() against it and report back; I have no access to that database myself (confirmed: the only mongoURI on this machine points at a now-unreachable/deleted Atlas cluster). What it does mean is that flipping LEDGER_USE_TRANSACTIONS=true, whenever that's confirmed safe, is now exercising genuinely tested code instead of an unverified branch that would have hung in production exactly the way it hung in this test before the fix. 343/343 tests passing (336 baseline + 7 new). Co-Authored-By: Claude Sonnet 5 --- middlewareNode/src/services/ledgerService.js | 34 ++- .../tests/ledgerService.transactions.test.js | 214 ++++++++++++++++++ 2 files changed, 237 insertions(+), 11 deletions(-) create mode 100644 middlewareNode/tests/ledgerService.transactions.test.js diff --git a/middlewareNode/src/services/ledgerService.js b/middlewareNode/src/services/ledgerService.js index f99292c9..62c39757 100644 --- a/middlewareNode/src/services/ledgerService.js +++ b/middlewareNode/src/services/ledgerService.js @@ -15,6 +15,11 @@ * No MongoDB transaction here by default — see the file-level comment on * why below applyLedgerEntry. Set LEDGER_USE_TRANSACTIONS=true once Week 0's * replica-set gate (`rs.status()` in mongosh) confirms one is available. + * That branch is covered by tests/ledgerService.transactions.test.js + * against a real (in-memory) replica set, so flipping the flag isn't + * exercising untested code — but nobody has run the actual `rs.status()` + * check against the real deployment yet, so the flag itself stays off by + * default until someone does. */ const mongoose = require("mongoose"); @@ -130,24 +135,31 @@ async function applyLedgerEntry({ eventId, userId, actionKey, amount, occurredAt if (useTransactions) { const session = await mongoose.startSession(); try { - let duplicate = false; + // The duplicate-key error must propagate OUT of withTransaction's + // callback, not be caught and swallowed inside it. The driver's + // withTransaction() retries the whole callback on a transient + // error, and a callback that catches an error and returns normally + // looks to it like "finished, but as a no-op" — against a real + // replica set this was observed to retry indefinitely (hundreds of + // calls) rather than just committing nothing, because the driver + // can't tell "I decided not to write anything" apart from "I need + // to be retried." Letting it throw aborts the transaction cleanly + // (correct here — nothing else was written) and is caught once, + // after withTransaction itself settles. await session.withTransaction(async () => { - try { - await LedgerEntry.create([{ eventId, userId, actionKey, amount, occurredAt }], { session }); - } catch (err) { - if (err && err.code === DUPLICATE_KEY_ERROR) { - duplicate = true; - return; - } - throw err; - } + await LedgerEntry.create([{ eventId, userId, actionKey, amount, occurredAt }], { session }); await UserBalance.updateOne( { userId }, { $inc: { balance: amount, lifetimeEarned: amount } }, { upsert: true, session } ); }); - return { applied: !duplicate, duplicate }; + return { applied: true, duplicate: false }; + } catch (err) { + if (err && err.code === DUPLICATE_KEY_ERROR) { + return { applied: false, duplicate: true }; + } + throw err; } finally { await session.endSession(); } diff --git a/middlewareNode/tests/ledgerService.transactions.test.js b/middlewareNode/tests/ledgerService.transactions.test.js new file mode 100644 index 00000000..5e0dbcaf --- /dev/null +++ b/middlewareNode/tests/ledgerService.transactions.test.js @@ -0,0 +1,214 @@ +/** + * Real-database tests for ledgerService.js's LEDGER_USE_TRANSACTIONS=true + * path — the branch of applyLedgerEntry() that wraps the LedgerEntry + * write and the UserBalance update in a genuine MongoDB multi-document + * transaction, instead of the single-document fallback that's the + * default (see applyLedgerEntry's file comment and Karthik's ledger + * plan, Week 0: "run rs.status() ... report back before Week 1 opens"). + * + * This file exists because that transactional branch had code but no + * test coverage at all — the rest of ledgerService.test.js only ever + * exercises the default, non-transactional fallback. Standalone + * mongodb-memory-server instances (used everywhere else in this suite) + * don't support transactions, so this file spins up a real single-node + * *replica set* via MongoMemoryReplSet instead — transactions require a + * replica set even with just one member, which is exactly the real-world + * condition this code branch is written for. + * + * This does not answer the actual Week 0 question (is the real + * deployment's MongoDB a replica set?) — that requires running + * `rs.status()` against the real database, which nobody has reported + * back on yet. What this answers is a different, also-real question: + * if it is a replica set, does the transactional code path actually + * work correctly? Before this file, that code had never once executed + * in a test. + */ + +const { MongoMemoryReplSet } = require("mongodb-memory-server"); +const mongoose = require("mongoose"); + +jest.setTimeout(90000); + +let replSet; +const ORIGINAL_FLAG = process.env.LEDGER_USE_TRANSACTIONS; + +beforeAll(async () => { + replSet = await MongoMemoryReplSet.create({ + replSet: { count: 1, storageEngine: "wiredTiger" }, + instanceOpts: [{ launchTimeout: 45000 }], + }); + await replSet.waitUntilRunning(); + // getUri() returns a full connection string with its own query string + // (e.g. "mongodb://127.0.0.1:PORT/?replicaSet=testset") — appending a + // db name as a plain string suffix (the pattern used elsewhere in this + // suite for MongoMemoryServer, which returns a bare "mongodb://host:port/") + // would land inside the query string here and break it. getUri(dbName) + // inserts it in the right place instead. + await mongoose.connect(replSet.getUri("ystem")); + + process.env.LEDGER_USE_TRANSACTIONS = "true"; +}); + +afterAll(async () => { + await mongoose.disconnect(); + await replSet.stop(); + if (ORIGINAL_FLAG === undefined) { + delete process.env.LEDGER_USE_TRANSACTIONS; + } else { + process.env.LEDGER_USE_TRANSACTIONS = ORIGINAL_FLAG; + } +}); + +afterEach(async () => { + const collections = mongoose.connection.collections; + await Promise.all(Object.values(collections).map((c) => c.deleteMany({}))); +}); + +// Re-required fresh in each test file run — ledgerService reads +// process.env.LEDGER_USE_TRANSACTIONS at call time inside +// applyLedgerEntry(), not at module load, so no jest.resetModules() +// dance is needed to pick up the flag set in beforeAll above. +const ActionRule = require("../src/models/actionRule"); +const LedgerEntry = require("../src/models/ledgerEntry"); +const UserBalance = require("../src/models/userBalance"); +const { processEvent, applyLedgerEntry } = require("../src/services/ledgerService"); + +const userId = new mongoose.Types.ObjectId(); + +describe("applyLedgerEntry — LEDGER_USE_TRANSACTIONS=true, real replica set", () => { + it("writes exactly one LedgerEntry and updates UserBalance atomically for a new eventId", async () => { + const result = await applyLedgerEntry({ + eventId: "tx-evt-1", + userId, + actionKey: "lesson.completed", + amount: 10, + occurredAt: new Date(), + }); + + expect(result).toEqual({ applied: true, duplicate: false }); + + const entries = await LedgerEntry.find({ userId }); + expect(entries).toHaveLength(1); + + const balance = await UserBalance.findOne({ userId }); + expect(balance.balance).toBe(10); + expect(balance.lifetimeEarned).toBe(10); + }); + + it("is a no-op on a duplicate eventId — the transaction's own insert fails, nothing commits twice", async () => { + const args = { + eventId: "tx-evt-dup", + userId, + actionKey: "lesson.completed", + amount: 10, + occurredAt: new Date(), + }; + + const first = await applyLedgerEntry(args); + const second = await applyLedgerEntry(args); + + expect(first).toEqual({ applied: true, duplicate: false }); + expect(second).toEqual({ applied: false, duplicate: true }); + + expect(await LedgerEntry.countDocuments({ userId, eventId: "tx-evt-dup" })).toBe(1); + const balance = await UserBalance.findOne({ userId }); + expect(balance.lifetimeEarned).toBe(10); // not 20 — the duplicate never committed + }); + + it("survives concurrent duplicate inserts of the same eventId without double-paying", async () => { + const args = { + eventId: "tx-evt-race", + userId, + actionKey: "lesson.completed", + amount: 10, + occurredAt: new Date(), + }; + + const [a, b] = await Promise.all([applyLedgerEntry(args), applyLedgerEntry(args)]); + const duplicateFlags = [a.duplicate, b.duplicate].sort(); + + expect(duplicateFlags).toEqual([false, true]); + expect(await LedgerEntry.countDocuments({ userId, eventId: "tx-evt-race" })).toBe(1); + + const balance = await UserBalance.findOne({ userId }); + expect(balance.lifetimeEarned).toBe(10); + }); + + it("never leaves a LedgerEntry committed without its matching UserBalance update (the actual point of using a transaction)", async () => { + // The whole reason to prefer the transactional path over the + // single-document fallback is that a crash between the two writes + // can't happen — it's one atomic commit or nothing. Simulate the + // UserBalance half failing (an invalid upsert) and confirm the + // LedgerEntry side was rolled back too, not left stranded. + const realUpdateOne = UserBalance.updateOne.bind(UserBalance); + const spy = jest + .spyOn(UserBalance, "updateOne") + .mockImplementationOnce(() => { + throw new Error("simulated UserBalance write failure"); + }); + + await expect( + applyLedgerEntry({ + eventId: "tx-evt-rollback", + userId, + actionKey: "lesson.completed", + amount: 10, + occurredAt: new Date(), + }) + ).rejects.toThrow("simulated UserBalance write failure"); + + // With the single-document fallback, this scenario would leave a + // committed LedgerEntry with no matching balance update — "merely + // stale," per that path's own design comment. The transactional path + // must do better: the whole operation rolls back together. + expect(await LedgerEntry.countDocuments({ eventId: "tx-evt-rollback" })).toBe(0); + + spy.mockRestore(); + void realUpdateOne; // kept for clarity on what was restored + }); + + it("accumulates balance and lifetimeEarned together across distinct events", async () => { + await applyLedgerEntry({ eventId: "tx-evt-a", userId, actionKey: "lesson.completed", amount: 10, occurredAt: new Date() }); + await applyLedgerEntry({ eventId: "tx-evt-b", userId, actionKey: "puzzle.solved", amount: 5, occurredAt: new Date() }); + + const balance = await UserBalance.findOne({ userId }); + expect(balance.balance).toBe(15); + expect(balance.lifetimeEarned).toBe(15); + }); +}); + +describe("processEvent — rules engine, LEDGER_USE_TRANSACTIONS=true", () => { + it("awards currency through the transactional path end to end", async () => { + await ActionRule.create({ actionKey: "lesson.completed", currencyAmount: 25, active: true }); + + const result = await processEvent({ + eventId: "tx-process-1", + userId, + actionKey: "lesson.completed", + occurredAt: new Date(), + }); + + expect(result).toMatchObject({ outcome: "awarded", amount: 25 }); + + const balance = await UserBalance.findOne({ userId }); + expect(balance.lifetimeEarned).toBe(25); + }); + + it("still enforces cooldown/daily-cap before ever starting a transaction", async () => { + await ActionRule.create({ actionKey: "lesson.completed", currencyAmount: 10, active: true, cooldownSeconds: 3600 }); + const now = new Date(); + + const first = await processEvent({ eventId: "tx-cd-1", userId, actionKey: "lesson.completed", occurredAt: now }); + expect(first.outcome).toBe("awarded"); + + const second = await processEvent({ + eventId: "tx-cd-2", + userId, + actionKey: "lesson.completed", + occurredAt: new Date(now.getTime() + 60 * 1000), + }); + expect(second.outcome).toBe("cooldown"); + + expect(await LedgerEntry.countDocuments({ userId })).toBe(1); + }); +}); From 0bd2e796d9d2f0694ec63e64b4f5ab2ca7651bc5 Mon Sep 17 00:00:00 2001 From: JimmyWu7 <104340474+JimmyWu7@users.noreply.github.com> Date: Thu, 1 Oct 2026 20:33:02 -0400 Subject: [PATCH 8/9] fix(currency): allow claimed action event status --- middlewareNode/src/models/actionEvent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/middlewareNode/src/models/actionEvent.js b/middlewareNode/src/models/actionEvent.js index 0d66ee36..93c95600 100644 --- a/middlewareNode/src/models/actionEvent.js +++ b/middlewareNode/src/models/actionEvent.js @@ -47,7 +47,7 @@ const ActionEventSchema = new mongoose.Schema( // actionKey + occurredAt. status: { type: String, - enum: ["pending", "processed", "failed"], + enum: ["pending", "claimed", "processed", "failed"], default: "pending", index: true, }, From eeda20a74d28088e5e78929314e15d293babf645 Mon Sep 17 00:00:00 2001 From: JimmyWu7 <104340474+JimmyWu7@users.noreply.github.com> Date: Fri, 2 Oct 2026 02:08:29 -0400 Subject: [PATCH 9/9] test(currency): wait for action event indexes --- middlewareNode/tests/currencyEvents.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/middlewareNode/tests/currencyEvents.test.js b/middlewareNode/tests/currencyEvents.test.js index d5f611a3..5178d201 100644 --- a/middlewareNode/tests/currencyEvents.test.js +++ b/middlewareNode/tests/currencyEvents.test.js @@ -11,6 +11,8 @@ const { MongoMemoryServer } = require("mongodb-memory-server"); const mongoose = require("mongoose"); +const ActionEvent = require("../src/models/actionEvent"); +const { emit, emitLessonCompleted } = require("../src/services/currencyEvents"); jest.setTimeout(60000); @@ -19,6 +21,7 @@ let mongod; beforeAll(async () => { mongod = await MongoMemoryServer.create({ instance: { launchTimeout: 30000 } }); await mongoose.connect(mongod.getUri() + "ystem"); + await ActionEvent.init(); }); afterAll(async () => { @@ -31,9 +34,6 @@ afterEach(async () => { await Promise.all(Object.values(collections).map((c) => c.deleteMany({}))); }); -const ActionEvent = require("../src/models/actionEvent"); -const { emit, emitLessonCompleted } = require("../src/services/currencyEvents"); - const userId = new mongoose.Types.ObjectId(); describe("emit", () => {