fix(classroom): return 410 for tombstoned courses and fast-fail availability retry (#1396)

[maheshsingh20]

Fixes #1396

Summary

Tombstoned courses were indistinguishable from courses still provisioning: both GET /api/persistence/documents/:id and GET /api/stage-meta/:id returned 404 for a deleted course, the same code returned for an ID that never existed. The classroom's availability retry treated any 404 as "not yet available" and kept polling, so /classroom/[id] never reached a not-found state — it stayed on an empty Stage shell showing "Loading..." and a "0/0" counter indefinitely.

This PR fixes the root cause end-to-end: the server distinguishes "gone" from "not found," and the client fast-fails on "gone" instead of retrying.

Design Trade-offs & Behavioral Notes

• Tombstone Opacity vs Fast-Fail Availability:
  Returning 410 with deleted_at distinguishes a deleted course from an ID that never existed (which 404s). While this changes the strict tombstone-opacity design (where deleted courses answered 404 to avoid acting as an existence oracle), existence of live courses is already observable, so the delta is narrow. This deliberate trade-off fast-fails tombstoned courses and prevents infinite retry loops.
• Server-Authoritative Tombstone Short-Circuit:
  On main, a tombstoned server-persisted ID fell through to the /api/classroom share-store fallback and got restored under the same ID. Throwing DocumentGoneError from the storage layer now intentionally short-circuits that fallback for server-backed persistence, making server tombstones authoritative (the comment in lib/classroom/load-classroom.ts has been updated accordingly).

Changes

Server

• lib/persistence/document-access.ts: decideDocumentAccess returns { outcome: 'gone', deletedAt } for tombstoned records, reserving 'not-found' for IDs that never existed.
• app/api/persistence/[...path]/route.ts: returns 410 Gone with { error: { code: 'DOCUMENT_GONE', message: '...', details: { deleted_at } }, deleted_at } for tombstoned documents.
• app/api/stage-meta/[stageId]/route.ts: calls readStageAccessIncludingDeleted(stageId); returns 410 Gone with deleted_at for tombstoned stages, 404 only when nonexistent.
• lib/server/stage-access.ts: docstrings updated to document readStageAccessIncludingDeleted for callers handling tombstones vs resolveStageAccess returning null for active presentation gates.

Storage Package & Sidecar Client

• packages/@openmaic/storage: minor version bump to 0.30.0 (breaking change: loadDocument/getScene throw DocumentGoneError instead of returning null for tombstoned courses).
• packages/@openmaic/storage/docs/document-http-contract.md: updated with 410 DOCUMENT_GONE row and documented error.details.deleted_at.
• packages/@openmaic/storage/src/document/http.ts: strictly keys on error.code === 'DOCUMENT_GONE' rather than raw HTTP status alone; extracts deleted_at from details.deleted_at.
• lib/classroom/stage-meta-client.ts: StageMetaResult gains { outcome: 'gone', deletedAt }; enforces bounded timeout (DEFAULT_STAGE_META_TIMEOUT_MS = 5_000) and supports AbortSignal.

Classroom Surface & Availability Retry

• components/classroom/ClassroomSurface.tsx:
  • Fixed spinner gate (!notFound added to loading || (variant === 'pane' && !error && !notFound && loadedClassroomId !== classroomId)), ensuring the not-found card renders instead of spinning indefinitely when notFound is set.
  • Gated availability polling delay by variant (variant === 'pane' ? paneAvailabilityRetryDelay : () => null), eliminating the 31-second retry delay for never-existed IDs on the standalone page.
  • Removed dead notFoundMessageRef.
• lib/classroom/progressive-load-policy.ts: unified availability polling logic (startClassroomAvailabilityPolling).
• app/classroom/[id]/page.tsx: mounts <ClassroomSurface classroomId={classroomId} variant="page" />.

Tests

• tests/agent-runtime/stage-meta-routes.test.ts: 404 for nonexistent course, 410 + deleted_at for tombstoned course.
• tests/persistence/stage-access-fidelity.test.ts: 410 + deleted_at on persistence route for tombstoned courses.
• tests/classroom/availability-retry.test.ts: unit tests for 404 timeout, propagation success, and 410 fast-fail.
• tests/classroom/classroom-surface-render.test.ts: component rendering tests verifying <ClassroomSurface /> actually unmounts spinner and displays not-found card in both pane and page variants for both tombstoned (410) and nonexistent (404) courses.

[wyuc]

Thanks — the server half of this is right: 410 + deleted_at on both endpoints, DocumentGoneError, the route tests, and decideDocumentAccess returning the gone object only for reads all check out (I also confirmed the object variant fails closed in authorizeDocuments). The client half doesn't close #1396 yet, and the storage change needs its contract updated. Findings, most severe first.

1. (blocking) The workbench pane still spins forever. components/classroom/ClassroomSurface.tsx:360 is unchanged:

{loading || (variant === 'pane' && !error && loadedClassroomId !== classroomId) ? <spinner/> : notFound ? ... }

On main the pane escaped that gate because its terminal path did setError(notFoundMessageRef.current). The PR replaces every pane terminal path (DocumentGoneError catch, onDeleted, onNotFoundTimeout, the retry button) with setNotFound(true); setLoading(false); setError(null). With error === null and no stage in the store the gate stays true, so notFound never renders. For a tombstoned course in WorkspaceClassroomPane that is exactly the #1396 symptom, and for a never-existed id it is a regression from main, which showed the not-found copy after the backoff. notFoundMessageRef is now written but never read, which is the tell. Fix the gate (&& !notFound, or keep setting error), and please add a test that actually renders ClassroomSurface in both variants for gone / never-existed — tests/workbench/classroom-pane-edit-lock.test.ts mocks the component out, so CI green does not cover this.

2. HttpDocumentStore keys on the raw status. packages/@openmaic/storage/src/document/http.ts:248,309 use error.status === 410 || error.code === 'DOCUMENT_GONE'. docs/document-http-contract.md:62 says explicitly that status alone is not sufficient. A proxy/CDN answering 410 for a live course would now put the surface in the terminal not-found state (which deliberately has no retry) instead of the availability backoff. Key on the code only.

3. Breaking change to @openmaic/storage shipped as a patch, contract doc stale. loadDocument/getScene go from returning null to throwing for a class of responses. That needs a minor bump (0.30.0), a 410 DOCUMENT_GONE row in docs/document-http-contract.md, and the deleted_at field documented (right now the client probes six positions for it; pick one, ideally error.details.deleted_at).

4. The tombstone-opacity design is reversed while the docstrings still assert it. app/api/stage-meta/[stageId]/route.ts:11-17 and lib/server/stage-access.ts:118-126 both say a deleted course must 404 so the endpoint is not an existence oracle, and the PR now answers 410 with a deletion timestamp to any caller. Existence is already observable for live courses, so the new information is narrow, but this is a deliberate design change: update or remove those comments and state the tradeoff in the PR description.

5. fetchStageMeta is now awaited on the critical path with no timeout. On main it was fire-and-forget; now loadClassroom cannot return any outcome until it settles, and it is a bare fetch with no AbortSignal. A stalled /api/stage-meta request reintroduces the "never reaches a terminal state" bug this PR is fixing. Give it a bounded timeout.

6. The standalone page blanks for ~31 s on a never-existed id. startClassroomAvailabilityPolling is variant-agnostic, so the page gets the 1/2/4/8/16 s pane schedule; during that window loading is already false and <Stage> renders its empty stage-editor-loading div. Main's page was worse (it stayed blank forever), so this is an improvement, but the page has no availability gap to probe — gate the schedule on variant.

Minor: notFoundMessageRef is dead after the fix, variant is stale in the effect deps at :242, and the scripts/openmaic-packages.mjs CRLF line plus the 249-line page.tsx → ClassroomSurface refactor are unrelated to #1396 — the refactor is welcome (the component was already a copy of the page) but it belongs in its own commit or PR. Also worth noting in the description: on main a tombstoned server-persisted id fell through to the /api/classroom share-store fallback and got restored under the same id; the throw now short-circuits that, which is correct for a server-authoritative tombstone but contradicts the surviving comment in load-classroom.ts.

Checks run on the PR head: full vitest suite (671 files / 7478 tests), the storage package suite, tsc --noEmit, pnpm run lint, pnpm run check, version-bump and i18n scripts — all green, which is consistent with 1 being untested rather than absent. Happy to re-review once 1–3 and 5 are addressed.

[maheshsingh20]

Hi @wyuc
Thanks for the thorough review. All items have been addressed:

• Workbench Pane Spinner Gate & Component Tests:

  • Added !notFound to the gate in ClassroomSurface.tsx (loading || (variant === 'pane' && !error && !notFound && loadedClassroomId !== classroomId)), ensuring the not-found card renders and the spinner cleanly unmounts when a course is tombstoned or availability times out.
  • Added classroom-surface-render.test.ts which directly mounts and renders <ClassroomSurface /> in both pane and page variants for both tombstoned (410) and never-existed (404) courses.

• HttpDocumentStore Error Keying:

  • In packages/@openmaic/storage/src/document/http.ts, both loadDocument and getScene now key strictly on error.code === 'DOCUMENT_GONE' rather than raw HTTP status 410, matching the contract guidelines.

• Storage Package Minor Bump & Contract Documentation:

  • Bumped @openmaic/storage to 0.30.0 in package.json for the breaking DocumentGoneError throw change.
  • Added the 410 DOCUMENT_GONE row to packages/@openmaic/storage/docs/document-http-contract.md.
  • Standardized deleted_at retrieval exclusively on error.details.deleted_at across the persistence route, client, and test harness.

• Tombstone Opacity vs Fast-Fail Availability Trade-off:

  • Updated docstrings in app/api/stage-meta/[stageId]/route.ts and lib/server/stage-access.ts explaining that resolveStageAccess returns null for active presentation gates, while endpoints that need to fast-fail polling loops use readStageAccessIncludingDeleted.
  • Stated the design trade-off in the PR description: existence is already observable for live courses, and returning 410 avoids endless availability polling.

• Bounded Timeout on fetchStageMeta:

  • Added DEFAULT_STAGE_META_TIMEOUT_MS = 5_000 with an AbortController timer and support for an optional signal?: AbortSignal in lib/classroom/stage-meta-client.ts so stalled network requests cannot block classroom loading.

• Standalone Page Retries Gated on Variant:

  • Gated availability polling delay by variant (getRetryDelay: variant === 'pane' ? paneAvailabilityRetryDelay : () => null), eliminating the 31-second retry window on the standalone /classroom/[id] page for nonexistent IDs.

• Minor Cleanup:

  • Removed dead notFoundMessageRef from ClassroomSurface.tsx.
  • Reverted the CRLF normalization change in scripts/openmaic-packages.mjs so it cleanly matches origin/main.
  • Updated the comment in lib/classroom/load-classroom.ts clarifying that DocumentGoneError short-circuits the share-store fallback when a server-authoritative tombstone exists.

[wyuc]

Round 2 verified on 8add971: the pane gate now includes !notFound and the new classroom-surface-render.test.ts really renders the component in all four page/pane × gone/never-existed cells (mutation of the gate turns two of them red); http.ts keys on DOCUMENT_GONE only; storage is 0.30.0 and the contract doc has the 410 row; docstrings updated; fetchStageMeta has a 5 s abort; the page variant fast-fails (mutation of the variant gate turns the test red); all minor items and the CRLF line are cleaned up. 1367 + 1031 tests, tsc, lint, prettier and the version-bump check pass. Three small things left, then this merges.

1. The server still emits deleted_at in three positions. app/api/persistence/[...path]/route.ts (the gone branch) returns error.details.deleted_at, error.deleted_at and top-level deleted_at. The client now reads only error.details.deleted_at, which is the documented position, so the other two are undocumented surface; tests/persistence/stage-access-fidelity.test.ts pins them, and ErrorResponseBody.deleted_at in packages/@openmaic/storage/src/document/http.ts is now unread. Please emit only error.details.deleted_at, update that test, and drop the dead field.

2. The stage-meta timeout has no test. grep -rn "DEFAULT_STAGE_META_TIMEOUT_MS\|timeoutMs" tests/ finds nothing for lib/classroom/stage-meta-client.ts. One case with fake timers: the fetch never resolves, the 5 s timer fires, fetchStageMeta resolves { outcome: 'unavailable' } and the abort signal was triggered.

3. jsdom is not declared. The new test's // @vitest-environment jsdom pragma resolves only through pnpm's hoisted virtual store (node_modules/jsdom does not exist at the root; it is a devDependency of packages/@openmaic/editor and renderer). Add it to the root devDependencies so the test does not depend on hoisting.

Non-blocking, for the record (none of these should be fixed in this PR):

• A transient 5xx from /api/classroom or stage-meta collapses into the terminal "this course does not exist" card with no retry. That was already how the standalone page behaved before this PR (load complete with no stage went straight to not-found), so it is not a regression here; I will file it separately.
• The warm-store bypass from round 1 stands: when the pane already holds the course and the server has since tombstoned it, loadFromStorage short-circuits and the tombstone signal is stage-meta alone; if that probe times out or 5xxs, the stale classroom renders as loaded. Before this PR the await simply hung with the same visible result, so also not a regression; the follow-up is to treat 'unavailable' on a warm hit as non-terminal.
• The four new render cases all reach the terminal state through the error/deleted callbacks; one success case (stage lands, Stage renders) would complete the matrix.
• deletedAt is populated on StageMetaResult and DocumentGoneError but consumed by nothing outside tests; fine to keep, just noting the 410 alone is what fixes #1396.
• Delegating app/classroom/[id]/page.tsx to ClassroomSurface also brings the standalone page the server-job resume gate, canvas reset, i18n copy and the awaited stage-meta probe. Worth a sentence in the PR body since it goes beyond #1396.