340 lines
32 KiB
Markdown
340 lines
32 KiB
Markdown
# Study Desk audit evaluation and remediation plan
|
||
|
||
Date: 2026-08-06
|
||
Scope: independent evaluation of `CONFIRMED_FINDINGS.md`, `FINDINGS.json`, the supporting audit reports, and the current repository source.
|
||
Change policy for this evaluation: this plan is the only new file; no application source, tests, migrations, configuration, generated files, database, or existing audit artifact was changed.
|
||
|
||
## Executive verdict
|
||
|
||
The audit is credible and the central findings hold up against the current source. The most urgent problems are not cosmetic:
|
||
|
||
1. The committed migration chain does not create `MaterialGroup` or the three `groupId` columns, while the generated Prisma client selects those fields. A database created only with `prisma migrate deploy` is incompatible with the app.
|
||
2. Production authentication can use a public fallback session secret, making a deployment with an empty `SESSION_SECRET` forgeable.
|
||
3. The in-viewer quiz retake flow loses the retake scope and records the retake as a full, incorrectly scored attempt.
|
||
4. Flashcard resume trusts stale IDs and unguarded JSON, so deletion or malformed progress can leave the study viewer blank or crash it.
|
||
5. The production container path is presently unreliable: the port mapping is wrong without the development override, the conventional override silently changes normal Compose behavior, the build context can include host `node_modules`, and the runner installs a ranged Prisma CLI outside the lockfile.
|
||
|
||
I agree with most of the remaining findings as defects or resilience gaps, but not always with their severity or proposed remedy. In particular:
|
||
|
||
- FIN-04 needs more than wiring in the existing `filterAndClampOrder`: that helper does not preserve the logical current card when a stale ID occurs before the saved index, and it turns a completed index back into the last card.
|
||
- FIN-05 should normally use `3000:3726`, preserving the documented host port while mapping it to the actual container port.
|
||
- FIN-22 should not be fixed with an automatic migration retry loop. Schema conflicts should fail fast with actionable diagnostics and a documented recovery path.
|
||
- FIN-27 is more important than Low because changing quiz content can rewrite the meaning of persisted history.
|
||
- FIN-34 is only partially present: the dedicated deck page already awaits both DELETE requests, but the library start/restart path fires them without waiting.
|
||
- FIN-38 is not currently a defect for this Arizona-based installation. It becomes a portability requirement only if the app is meant to support a configurable local day boundary.
|
||
- FIN-42 does not justify a destructive history rewrite without evidence that the old database contained sensitive data and without coordinating every clone/remote.
|
||
- FIN-50 is an intentional convention. Newest named groups must remain first and Uncategorized must remain last; validation should make that convention harder to corrupt.
|
||
- FIN-52 is an architectural consequence of local grading in an anonymous read-only viewer. It merits a sharing warning, not removal of the answer key unless the product adopts server-side anonymous grading.
|
||
|
||
## Finding-by-finding disposition
|
||
|
||
Priority definitions: P0 = release/deployment blocker; P1 = next remediation batch; P2 = important hardening; P3 = worthwhile backlog; Conditional = verify a stated condition before changing behavior; No separate fix = intentional behavior or covered by another item.
|
||
|
||
| ID | Verdict | Worth fixing? | Priority and disposition |
|
||
|---|---|---:|---|
|
||
| FIN-01 | Agree | Yes | **P0.** Add the missing migration plus a safe adoption runbook for already-`db push`-synchronized databases. |
|
||
| FIN-02 | Agree, conditional on an unset/blank production secret | Yes | **P0.** Production must fail before serving requests; development may use an explicitly development-only value. |
|
||
| FIN-03 | Agree | Yes | **P1.** Make attempted-question scope explicit and derive partial-retake status on the server. |
|
||
| FIN-04 | Agree; proposed helper is insufficient as written | Yes | **P1.** Normalize order, index, and results while preserving the logical current card and completed state. |
|
||
| FIN-05 | Agree | Yes | **P0.** Map host 3000 to container 3726 and smoke-test it. |
|
||
| FIN-06 | Agree with the mechanics; exact runtime failure is unconfirmed | Yes | **P0.** Add `.dockerignore` and prove native modules load in the Linux image. |
|
||
| FIN-07 | Agree | Yes | **P0.** Rename the development override and document explicit dev/prod commands. |
|
||
| FIN-08 | Partly agree: first-login setup is intentional, accidental exposure and dead config are real | Yes | **P1.** Honor a pre-provisioned admin hash or require an explicit one-time setup mode. |
|
||
| FIN-09 | Agree; latent with current IDs | Yes | **P1.** Replace the broad dot bypass with exact public/static path rules. |
|
||
| FIN-10 | Agree | Yes | **P1.** Prevent token overwrite, remove spoofable per-IP trust, and make reset initiation a local/admin operation. |
|
||
| FIN-11 | Agree | Yes | **P1.** Validate the whole attempt envelope, question scope, option ownership, array uniqueness, and types. |
|
||
| FIN-12 | Agree | Yes | **P1.** Add an in-flight guard, disable Finish, and surface failure/retry state. |
|
||
| FIN-13 | Agree as a credible timing race | Yes | **P1.** Persist the attempt and clear full-attempt progress in one server transaction. |
|
||
| FIN-14 | Agree as a credible timing race | Yes | **P2.** Add session identity and monotonic revisions, not only client debouncing. |
|
||
| FIN-15 | Agree | Yes | **P1.** Persist `currentIndex === order.length` and restore the summary state. |
|
||
| FIN-16 | Agree; impact is lower in a protected single-user app | Yes | **P2.** Accept only the queue-eligible card, including the intended learn-ahead rule. |
|
||
| FIN-17 | Agree that the queries are unbounded; user impact is unmeasured | Conditional | **P3.** Benchmark realistic data first, then optimize queries that exceed the budget. |
|
||
| FIN-18 | Agree | Yes | **P2.** Stop mutating shared objects, capture the origin group before mutation, await writes, and rollback/refetch on error. |
|
||
| FIN-19 | Agree | Yes | **P1.** Enforce class/type/group membership and server-owned ordering; retain a share-page defense. |
|
||
| FIN-20 | Agree | Yes | **P1.** Give the viewer a stable session input so unrelated parent renders cannot reset it. |
|
||
| FIN-21 | Agree | Yes | **P1.** Validate progress writes and defensively normalize legacy/corrupt progress on reads. |
|
||
| FIN-22 | Partly agree with the operational gap, not the retry remedy | No separate fix | Fail fast; cover CLI pinning in FIN-23 and recovery/health in FIN-24. Do not retry a conflicting migration automatically. |
|
||
| FIN-23 | Agree | Yes | **P0.** Use `npm ci` and ship a lockfile-pinned Prisma CLI/runtime path. |
|
||
| FIN-24 | Agree | Yes | **P1.** Add a schema-aware health check and tested backup/restore instructions. |
|
||
| FIN-25 | Agree | Yes | **P0/P1.** Gate test, build, Prisma validation/drift, and container smoke. Stage lint until its known baseline is resolved. |
|
||
| FIN-26 | Agree | Yes | **P1.** Return zero for an invalid zero-correct SATA question and test finite totals. |
|
||
| FIN-27 | Agree; severity should be Medium | Yes | **P2.** Persist an immutable result/review snapshot for new attempts, with legacy fallback. |
|
||
| FIN-28 | Agree | Yes | **P2.** Reuse a trimmed card-content schema and reject rather than silently drop incomplete Create-tab rows. |
|
||
| FIN-29 | Agree | Yes | **P2.** Add reasonable string, array, and request-size limits with clear 400/413 errors. |
|
||
| FIN-30 | Agree that the contract is inconsistent | Yes | **P2.** Make SATA imports require at least two correct options, matching the shipped generation instructions; preserve legacy stored data. |
|
||
| FIN-31 | Agree | Yes | **P2.** Validate complete request envelopes and map only actual not-found errors to 404. |
|
||
| FIN-32 | Agree | Yes | **P2.** Delete/reassign/renumber in one transaction and add deterministic item tie-breaks. |
|
||
| FIN-33 | Agree | Yes | **P2.** Add an explicit keyboard-accessible “Move to group” action; do not depend on drag gestures. |
|
||
| FIN-34 | Partly agree | Yes | **P2.** Fix the library path that does not await deletion; make deletion session-aware with FIN-14. |
|
||
| FIN-35 | Agree | Yes | **P3.** Translate concurrent first-review uniqueness conflicts to 409 and refetch. |
|
||
| FIN-36 | Agree as a credible timing race | Yes | **P2.** Disable navigation while grading and cancel/ignore stale animation callbacks. |
|
||
| FIN-37 | Agree | Yes | **P3.** Refresh set/deck membership on focus and after local deck-change events. |
|
||
| FIN-38 | Factually correct, but Arizona is the current intended boundary | Not now | Keep the existing behavior. Revisit only with an explicit portability requirement and a timezone setting/migration plan. |
|
||
| FIN-39 | Agree about the lost clear-cookie response; layout impact is overstated because proxy checks generation | Yes | **P2.** Destroy the cookie on the actual redirect response and test its header. |
|
||
| FIN-40 | Deployment-policy gap, not a bug on intentional HTTP | Yes, as documentation/config | **P1.** Document a TLS proxy profile and `SECURE_COOKIES=true`; do not enable Secure cookies on plain HTTP. |
|
||
| FIN-41 | Agree | Yes | **P1.** Validate a strict target enum and target existence; reject impossible/null-target rows. |
|
||
| FIN-42 | Agree that blobs exist; sensitivity is unproven | Conditional | Do not rewrite history now. Inventory the old DB offline before any public remote; rewrite only with explicit coordination if sensitive data is found. |
|
||
| FIN-43 | Partly agree | Yes | **P3.** Ignore `data/` and production DB sidecars. Confirm ownership, then remove unreferenced CSS scratch files. Do not automatically ignore the audit plan. |
|
||
| FIN-44 | Agree | Yes | **P2.** Add `res.ok`, error, and retry handling; never present a failed load as an empty dashboard. |
|
||
| FIN-45 | Agree | Yes | **P3.** Add abort/error handling to settings/share fetches. |
|
||
| FIN-46 | Agree as a credible timing race | Yes | **P2.** Use `AbortController` or a request generation tied to `classSlug`; remove or bound the module cache. |
|
||
| FIN-47 | Agree | Yes | **P3.** Show logout failure and retry; do not falsely claim logout if the server call failed. |
|
||
| FIN-48 | Partly agree | Yes | **P2.** Guarantee a non-empty unique slug. Keep slugs stable on rename unless redirect/history semantics are deliberately designed. |
|
||
| FIN-49 | Agree; impact is same-browser only | Yes | **P3.** Include the share token in local session keys and migrate/ignore old keys safely. |
|
||
| FIN-50 | Intentional newest-first convention | No separate fix | Preserve descending named-group order and final Uncategorized placement; restrict arbitrary sort-order writes under FIN-19/31. |
|
||
| FIN-51 | Agree, cosmetic only | Not now | Defer unless it can be removed without hydration mismatch or new effect-rule violations. |
|
||
| FIN-52 | Expected local-grading design | Warning only | **P3.** Explain in ShareMenu that recipients can inspect correct answers; do not imply answer secrecy. |
|
||
| FIN-53 | Agree | Yes | **P1.** Replace the false-confidence assertion and add scoring/scheduling edge cases. |
|
||
|
||
## Remediation sequence
|
||
|
||
### Phase 0 — safeguards and test foundation
|
||
|
||
Do this before behavior or schema changes.
|
||
|
||
1. Create a database-backed Vitest harness that always points Prisma at a unique disposable SQLite path. It must refuse to run if the resolved path is `dev.db`, `/app/data/study.db`, or any existing non-test database.
|
||
2. Add helpers to apply committed migrations to the disposable database and dispose of it after the test process.
|
||
3. Record the current lint baseline separately. Do not make a failing baseline a nominally “green” CI gate, and do not mix unrelated lint cleanup into P0 fixes.
|
||
4. Add focused pure tests first for scoring, progress normalization, import schemas, and rate limiting. These give fast feedback before route/service integration tests.
|
||
5. Before testing any real deployment database, make and verify a restorable backup. No remediation command should run against the only copy.
|
||
|
||
Exit criteria:
|
||
|
||
- Tests demonstrably create and use only a disposable DB.
|
||
- A deliberate attempt to point the harness at the real DB aborts.
|
||
- Existing `npm.cmd test` remains green.
|
||
|
||
### Phase 1 — restore a safe, reproducible deployment path
|
||
|
||
Addresses FIN-01, FIN-02, FIN-05–10, FIN-23–25, FIN-39, and FIN-40.
|
||
|
||
#### 1A. Repair migration drift without breaking already-pushed databases
|
||
|
||
1. Generate a new migration; never edit the four applied migrations. It must add:
|
||
- `MaterialGroup` with its Class cascade foreign key.
|
||
- nullable `groupId` on Deck and QuizSet with `ON DELETE SET NULL`.
|
||
- nullable unique `groupId` on ShareLink with `ON DELETE CASCADE`.
|
||
- the indexes Prisma expects.
|
||
2. Review the generated SQLite table-rebuild SQL by hand for preserved rows, foreign keys, defaults, and unique indexes.
|
||
3. Publish a one-time preflight/adoption script that classifies a database as:
|
||
- migration-tracked and missing the group schema: apply the migration normally;
|
||
- already schema-pushed and exactly matching the intended DDL: after backup and exact introspection, mark the new migration applied with `prisma migrate resolve --applied`;
|
||
- partially matching or otherwise inconsistent: stop with diagnostics and require manual recovery; never guess or auto-resolve.
|
||
4. Keep the adoption operation explicit. The normal entrypoint should not silently mutate migration history based on table existence.
|
||
|
||
Verification:
|
||
|
||
- Fresh empty DB: `prisma migrate deploy` succeeds; `prisma migrate diff --from-migrations ... --to-schema ...` is empty; Deck, QuizSet, ShareLink, and MaterialGroup queries succeed.
|
||
- Populated pre-group DB: seed classes/decks/quizzes/shares, apply the new migration, and prove every row/count/relationship is preserved.
|
||
- Disposable `db push`-style DB: introspect exact equivalence, resolve the migration, run deploy again, and prove an empty schema diff.
|
||
- Partial/conflicting DB: preflight exits nonzero without changing schema or `_prisma_migrations`.
|
||
|
||
#### 1B. Make session configuration unforgeable and setup explicit
|
||
|
||
1. Centralize session option construction so `src/lib/auth.ts` and `src/proxy.ts` cannot drift.
|
||
2. Require a nonblank, at-least-32-character production `SESSION_SECRET` at runtime. Do not require or bake the real secret during `docker build`.
|
||
3. Make the container entrypoint fail before migrations/server startup when the secret is absent or equals the known fallback. Make Compose use required-variable expansion.
|
||
4. Allow a development-only fallback only when `NODE_ENV !== "production"`, clearly label it non-production, and cover both branches with tests.
|
||
5. Replace `pathname.includes(".")` with exact public/static rules. Ensure every `/api/*` path other than the intentional auth endpoints remains protected even when the URL contains a dot or encoded dot.
|
||
6. Construct the unauthenticated redirect first and bind `getIronSession` to that response before `destroy()`, so the returned redirect carries the clearing cookie.
|
||
7. Honor a valid `ADMIN_PASSWORD_HASH` when the database has no configured password, or require an explicit one-time setup flag/token. The default production state must not let the first remote request select the admin password.
|
||
8. Prefer a local console/CLI initiation for password reset. If the HTTP request endpoint remains, use a global single-user throttle, never replace an unexpired token, rate-limit verify/complete, and do not trust arbitrary `x-forwarded-for` unless a trusted proxy is explicitly configured.
|
||
|
||
Verification:
|
||
|
||
- Production startup with missing, blank, short, or known fallback secret exits nonzero before serving.
|
||
- A cookie sealed with the old fallback does not authenticate when a real secret is configured.
|
||
- Valid login, logout, reset completion, and session-generation invalidation still work.
|
||
- Dot-containing protected page/API requests redirect or reject; known static assets still load.
|
||
- A stale-generation response contains a `Set-Cookie` deletion header.
|
||
- Pre-provisioned hash rejects a different first password and reports setup complete.
|
||
|
||
#### 1C. Make the image and Compose definitions deterministic
|
||
|
||
1. Add `.dockerignore` for `node_modules`, `.next`, VCS metadata, environment files, databases/data, coverage, temporary caches, and audit scratch artifacts while retaining source, Prisma schema/migrations, lockfile, and public assets.
|
||
2. Change the dependency stage to `npm ci`.
|
||
3. Remove `npm install prisma@^7.8.0` from the runner. Ship a Prisma CLI/runtime installed from `package-lock.json` at the exact repository version. If Prisma is needed at runtime, classify it as a runtime dependency and prune/copy dependencies deterministically rather than re-resolving them.
|
||
4. Map `3000:3726` in production Compose. Keep `PORT=3726` and `EXPOSE 3726` internally unless there is a deliberate decision to standardize everything on 3000.
|
||
5. Rename `docker-compose.override.yml` to a non-auto-merged development filename such as `docker-compose.dev.yml`. Document exact production and development invocations.
|
||
6. Add a minimal internal health route that performs schema-aware DB checks (including a query that touches `Deck.groupId` and MaterialGroup), returning no sensitive details. The container health check must call the actual internal port.
|
||
7. Add a lock-safe SQLite backup command using the SQLite backup API, plus a documented restore drill. A raw copy of only the main DB while WAL writes are active is not an acceptable backup procedure.
|
||
|
||
Verification:
|
||
|
||
- `docker compose -f docker-compose.yml config` shows the production command, production environment, and `3000:3726` only.
|
||
- The explicit development Compose command shows dev mode and source mounts.
|
||
- Build on a Windows-host context and run on Linux; requiring `better-sqlite3` and `argon2` succeeds in the final image.
|
||
- Fresh-volume container becomes healthy and `/login` returns 200 through host port 3000.
|
||
- A migration-drifted disposable volume stays unhealthy with an actionable log.
|
||
- Backup a populated disposable volume, destroy the disposable container/volume, restore it, and compare row counts plus representative content.
|
||
|
||
#### 1D. Add CI gates in a sequence that can actually be green
|
||
|
||
1. Gate `npm ci`, `npm test`, Prisma validate, migration/schema drift, and `npm run build` immediately.
|
||
2. Build the production image, start it on a fresh disposable volume with a generated CI secret, wait for health, and smoke `/login` plus setup status.
|
||
3. Tag immutable images by commit SHA; optionally move the mutable `latest` tag only after all gates pass.
|
||
4. Add strict lint only after the existing 9-error baseline is resolved in an explicit cleanup or after a changed-files lint gate is implemented. Never report baseline lint as passing.
|
||
|
||
### Phase 2 — make quiz attempts authoritative and immutable
|
||
|
||
Addresses FIN-03, FIN-11–13, FIN-20, FIN-26, FIN-27, and FIN-53.
|
||
|
||
1. Replace the client-controlled `isPartialRetake` contract with an explicit ordered `questionIds` scope. The server validates that IDs are unique and belong to the quiz, then derives whether the attempt is partial.
|
||
2. Accept a structured answers object at the route boundary and persist only canonical server-serialized JSON. Validate that:
|
||
- each key is in the attempted scope;
|
||
- every selected option belongs to that question;
|
||
- selected IDs are unique;
|
||
- multiple-choice has at most one selection;
|
||
- missing answers score zero rather than silently disappearing.
|
||
3. Make the in-viewer “Retake Missed” and “Retake Full Quiz” set explicit local attempt scope. Do not infer retake state from the original prop after the viewer has transitioned internally.
|
||
4. Add an `isFinishing` state/ref, disable the Finish button during submission, and show a retryable error without discarding answers.
|
||
5. Move attempt creation and full-attempt SEQUENTIAL-progress deletion into one service transaction. Partial retakes must not overwrite or delete a full in-progress session.
|
||
6. Guard zero-correct SATA scoring with zero points and deduplicate selections defensively in the pure scorer even though the route also validates them.
|
||
7. Add an optional immutable review snapshot to QuizAttempt for new attempts: attempted question/order, prompt/category/rationale, options and correctness, selections, and per-question points. Render new history from that snapshot; keep a clearly tested legacy fallback for old rows.
|
||
8. Stabilize QuizViewer initialization. A parent render that changes topics or another surrounding control must not regenerate option order, clear answers, or reset the index. A deliberate restart/retake must use a new session key.
|
||
|
||
Focused verification:
|
||
|
||
- Full 5-question attempt scores out of 5 and clears only its matching full progress row.
|
||
- Two-question retake scores out of 2, is stored as partial, and leaves full progress untouched.
|
||
- An unanswered question in the explicit scope scores zero and remains in the review snapshot.
|
||
- Duplicate/unknown question or option IDs, wrong shapes, and client-forged partial flags return 400 without creating an attempt.
|
||
- Delayed double-click Finish produces exactly one attempt.
|
||
- Changing or deleting current quiz content after an attempt does not change the stored score, category breakdown, or review copy for snapshot-backed attempts.
|
||
- Parent rerender preserves question order, option order, current index, and answers; explicit restart changes only the intended session.
|
||
|
||
### Phase 3 — make progress and flashcard resume race-safe
|
||
|
||
Addresses FIN-04, FIN-14, FIN-15, FIN-21, FIN-34, and FIN-36.
|
||
|
||
1. Replace or correct `filterAndClampOrder` with a normalization function that accepts saved order, live IDs, saved index, results/answers, and completion state. It must:
|
||
- remove stale and duplicate IDs;
|
||
- preserve the saved current ID when it still exists;
|
||
- when it was deleted, select the next surviving saved card, otherwise the previous survivor;
|
||
- preserve `index === oldOrder.length` as completion by returning `index === newOrder.length`;
|
||
- filter result/answer keys to live IDs;
|
||
- safely fall back when any JSON field is malformed.
|
||
2. Apply normalization before any viewer state initializer calls `JSON.parse`. Treat corrupt legacy progress as recoverable, show a small restore warning, and allow a fresh start.
|
||
3. Add Zod schemas for GET/PATCH/DELETE progress inputs, enum values, nonnegative index, content-specific JSON shape, and referenced content existence.
|
||
4. Add `sessionId` and monotonic `revision` to progress. The server accepts only a newer revision for the same session; a stale session cannot overwrite or delete a newer session. Serialize client saves but retain server-side revision enforcement because request completion order is not guaranteed.
|
||
5. On the last flashcard, persist `currentIndex = order.length`. Restoring that row must show the existing completion summary, not the last card.
|
||
6. Await restart deletion before remount/navigation. Make DELETE conditional on the session being cleared so a late old request cannot delete fresh progress.
|
||
7. Disable Previous, shuffle, and repeated grading during the 350 ms transition, and clear/cancel the timer on restart/unmount.
|
||
|
||
Focused verification:
|
||
|
||
- Stale ID before, at, and after the current index all resume on the correct logical card.
|
||
- All saved cards deleted yields a clear empty/completed state rather than a blank viewer.
|
||
- Completed sessions reopen on the summary; restart opens card 1.
|
||
- Invalid JSON and invalid PATCH payloads produce a safe UI fallback or 400, never a render crash/500.
|
||
- Deliver revisions 3, 1, and 2 in that order; the DB retains revision 3.
|
||
- Delay an old-session DELETE until after a new-session PATCH; the new session remains.
|
||
- Grade then immediately try Previous/restart; no index jump or stale result write occurs.
|
||
|
||
### Phase 4 — enforce group, ordering, and sharing invariants on the server
|
||
|
||
Addresses FIN-18, FIN-19, FIN-32, FIN-33, FIN-41, FIN-49, FIN-50, and FIN-52.
|
||
|
||
1. Move reorder logic into focused services. The route must identify the owning class and content type from the database, not trust the client.
|
||
2. Validate every target group exists, belongs to the same class, and matches DECK/QUIZ type. Reject duplicate IDs, foreign-class IDs, type mismatches, and unknown IDs.
|
||
3. Prefer an ordered list/group assignment contract and compute contiguous `sortOrder` values server-side. If a complete-set contract is required, verify completeness explicitly.
|
||
4. In the library, capture origin group before any update, update state immutably, await `res.ok`, disable conflicting mutations in flight, and rollback/refetch on failure.
|
||
5. Delete a group in one transaction: capture affected items, delete/reassign through the FK behavior, then renumber Uncategorized items deterministically. Add `createdAt`/`id` tie-breakers to reads so legacy duplicates are stable.
|
||
6. Add a keyboard-accessible Move-to-group menu on every item. Preserve drag-and-drop for pointer users and keep drag listeners off action controls.
|
||
7. Validate ShareLink target type and target existence. Enforce exactly one populated target in service logic; consider CHECK constraints in a later SQLite migration after compatibility testing.
|
||
8. Retain the shared-page class/type/token checks and additionally assert each selected group item has the group’s class and expected content type.
|
||
9. Pass the share token/session namespace to shared viewers and include it in localStorage keys. Avoid copying an old item-only session into a different token namespace without explicit user confirmation.
|
||
10. Preserve named-group ordering as `sortOrder desc, createdAt desc`, prepend newly created groups locally, and render Uncategorized last. Remove arbitrary `sortOrder` from the ordinary group rename PATCH contract.
|
||
11. Add a concise ShareMenu warning that a shared quiz necessarily sends answer/rationale data to the recipient’s browser for local grading.
|
||
|
||
Focused verification:
|
||
|
||
- Cross-class and cross-type group assignment returns 400/409 and changes no rows.
|
||
- A failing reorder restores/refetches the visible order.
|
||
- Deleting a group yields unique contiguous Uncategorized item order and does not delete decks/quizzes.
|
||
- Newer named groups stay above older groups; Uncategorized remains last in quizzes, flashcards, and import selectors.
|
||
- A keyboard-only user can move an item between two groups and hear/see confirmation.
|
||
- Invalid share target types and missing IDs create zero rows.
|
||
- A tampered group share cannot render a foreign-class item.
|
||
- Two tokens for the same content maintain independent local sessions.
|
||
|
||
### Phase 5 — validation, SRS integrity, and user-visible error handling
|
||
|
||
Addresses FIN-16, FIN-28–31, FIN-35, FIN-37, FIN-43–48, and the measurable part of FIN-17.
|
||
|
||
1. Reuse shared Zod schemas for card create/edit, deck/quiz create/edit, classes, imports, and material groups. Trim before minimum checks and add documented maximums for names, descriptions, Markdown content, options, questions, and cards.
|
||
2. Reject a Create-tab submission containing any partially filled card; identify the row(s) instead of silently dropping them.
|
||
3. Enforce at least two correct options for imported SATA questions while leaving existing stored questions readable. Add a targeted message explaining how to repair invalid generated JSON.
|
||
4. Narrow Prisma error mapping: P2025 becomes 404, P2002 becomes 409 where appropriate, validation is 400/422, and unexpected failures remain 500 with non-sensitive server diagnostics.
|
||
5. In SRS review, verify the submitted card is the queue-eligible card for the current set/time/new-card allowance (including deliberate learn-ahead). Retain the existing state-version comparison. Map concurrent first-review P2002 to 409 and return/refetch current study state.
|
||
6. Refresh SRS membership on focus and after same-tab deck changes.
|
||
7. Add abortable, checked fetch helpers or a small consistent pattern for Dashboard, GenerateTab, ShareMenu, logout, and class library requests. Distinguish loading, empty, error, and retry states.
|
||
8. Tie library responses to the active `classSlug`; abort or ignore stale responses. Bound or remove module-level caches that can outlive their class data.
|
||
9. Guarantee class slug generation produces a non-empty unique slug, for example a stable `class-<short-id>` fallback. Keep existing slugs stable on rename.
|
||
10. Add `data/`, `study.db`, and sidecars to `.gitignore`. Confirm `out.css`, `temp.css`, and `test.css` are unreferenced scratch artifacts before removing them in a separate cleanup change.
|
||
11. For performance, first seed a disposable benchmark DB approximating expected upper use (for example 100k activity rows, 10k cards, and several SRS sets). Add date predicates/aggregation and narrower SRS queries only where the measured request/poll budget is exceeded. Preserve long-streak correctness when limiting the activity window.
|
||
|
||
Focused verification:
|
||
|
||
- Whitespace-only and oversized content is rejected with an actionable client error and no partial write.
|
||
- SATA 1-correct import fails; 2+-correct passes; multiple-choice still requires exactly one.
|
||
- SRS cannot review a not-due/non-selected/new-limit-exhausted card; valid due and learn-ahead reviews still work.
|
||
- Concurrent first review produces one success and one handled conflict, not a generic 500.
|
||
- Focus refresh removes a deleted deck from the SRS set UI.
|
||
- Failed dashboard/settings/share/logout requests have visible, retryable states and no unhandled promise rejection.
|
||
- Rapid A→B class navigation cannot render A data under B.
|
||
- Creating a class named only punctuation yields a navigable, unique slug; rename does not break the old URL.
|
||
- Performance tests record query counts and latency before/after and prove the 53-week display plus longer current streak remain correct.
|
||
|
||
## Cross-cutting release verification
|
||
|
||
Run after every phase, with focused tests first and the full suite last.
|
||
|
||
Automated gates:
|
||
|
||
1. `npm.cmd test`
|
||
2. `npx.cmd prisma validate`
|
||
3. migration deploy + empty migration/schema diff against a fresh disposable DB whenever schema changes
|
||
4. `npm.cmd run build`
|
||
5. `git diff --check`
|
||
6. production image build, fresh-volume health wait, and HTTP smoke when Docker/deployment files change
|
||
7. lint against changed files; full `npm.cmd run lint` only becomes blocking after the known baseline is cleared
|
||
|
||
Manual browser matrix for affected phases:
|
||
|
||
1. Authenticated routes and corresponding public/shared routes.
|
||
2. Fresh, resumed, completed, restarted, partial-retake, and stale/deleted-content sessions.
|
||
3. Loading, empty, server-error, malformed-persisted-data, and retry states.
|
||
4. Light and dark themes.
|
||
5. Narrow mobile viewport with no horizontal overflow.
|
||
6. Keyboard-only navigation, including group moves and all modified icon controls.
|
||
7. Throttled-network checks for Finish, autosave, restart, class navigation, reorder, and logout.
|
||
8. Two-tab checks for progress revisions, SRS conflicts, and focus refresh.
|
||
|
||
Release/rollback rules:
|
||
|
||
- Back up and restore-test the SQLite database before the first migration-bearing release.
|
||
- Deploy schema and code as one versioned release; do not run a newer generated Prisma client against an older database.
|
||
- Keep the prior image and verified pre-migration backup until post-deploy smoke and representative data checks pass.
|
||
- If migration preflight sees a partial or unknown schema state, stop. Do not run `db push`, edit an applied migration, reset the database, or auto-mark the migration applied.
|
||
|
||
## Explicitly deferred or rejected work
|
||
|
||
- **FIN-38:** no timezone-setting work without a product requirement to support a non-Arizona study day.
|
||
- **FIN-42:** no history rewrite without an offline sensitivity review and explicit remote/clone coordination.
|
||
- **FIN-51:** no cosmetic state-initialization change unless it avoids both hydration mismatch and the existing effect-rule class of lint failures.
|
||
- **FIN-22 retry proposal:** no automatic migration retry/backoff for schema conflicts; fail fast with health diagnostics and the adoption/recovery runbook.
|
||
- No changes to the audit’s already rejected hypotheses (global API auth absence, split-brain SQLite paths, broken cascades/transactions, Markdown XSS, or client/server import violations) unless new evidence appears.
|
||
|
||
## Recommended delivery slices
|
||
|
||
Keep the implementation reviewable rather than landing all remediation at once:
|
||
|
||
1. **Release blocker:** FIN-01/02/05/06/07/23/25 plus migration adoption, container smoke, and backup prerequisites.
|
||
2. **Quiz correctness:** FIN-03/11/12/13/20/26/27/53.
|
||
3. **Progress integrity:** FIN-04/14/15/21/34/36.
|
||
4. **Group/share integrity:** FIN-18/19/32/33/41/49/50/52.
|
||
5. **Auth recovery and deployment posture:** FIN-08/09/10/24/39/40.
|
||
6. **Validation, SRS, UI resilience, and measured performance:** the remaining accepted items.
|
||
|
||
Each slice should be independently releasable, have its own focused regression tests, and finish with the cross-cutting gates above.
|