From f02e30450c77807aaff49bd373ea5b0a7839ae99 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 21:00:45 +0100 Subject: [PATCH] fix(studio): review round 2 - a conversion that fails after its commit recovers without a restart - The convert job reloads the course on success and on failure, so learners get the intact import (rolled back) or the published studio course, never a course whose folder moved away. - POST /convert on a course with an incomplete conversion (.converting-, staging or retired folders) queues a job that recovers and finishes it instead of answering 409; the studio page shows "Conversion incomplete" with Retry for the owner. - The 409 for pending import jobs names their kinds and ids; the dead withLock in git.ts is gone; the dot-folder discovery rule is documented in docs/operations.md and spec section 5. - Route tests (SQLite and Postgres) inject a failed cross-filesystem copy, a checksum mismatch and a pre-commit failure. Co-Authored-By: Claude Opus 5.5 --- docs/operations.md | 9 +++ .../specs/2026-10-30-stcourses-design.md | 1 + packages/feature-studio/src/server/feature.ts | 36 +++++++---- packages/feature-studio/src/server/git.ts | 12 ---- packages/feature-studio/src/server/store.ts | 24 +++++++- .../feature-studio/src/ui/ConvertPanel.tsx | 32 +++++++++- .../feature-studio/src/ui/CourseStudio.tsx | 4 +- packages/feature-studio/src/ui/api.ts | 4 +- packages/feature-studio/test/app-helpers.ts | 6 +- .../test/convert-routes.test.ts | 59 ++++++++++++++++++- .../feature-studio/test/convert-ui.test.tsx | 15 ++++- 11 files changed, 168 insertions(+), 34 deletions(-) diff --git a/docs/operations.md b/docs/operations.md index 5ea07cf..fc88288 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -143,6 +143,15 @@ Things to know: - After conversion the importer can no longer add decks, code or transcripts to that course (it only edits `/courses`). Add them in the studio. - `/media` is fixed under the data directory (there is no separate setting). +- **Discovery skips dot-folders.** Every course directory (`STC_COURSE_DIRS`, `/courses`, `/worktrees`) + is scanned to depth 2 for `stcourse.yaml`, and folders whose name starts with `.` are never entered. Do not give a + course folder a name that starts with a dot. The conversion's `.converting-` and `.converted-.` folders + rely on this. +- **A failed conversion.** If the media move fails after the commit (disk full, or a copy that fails its checksum), + learners are switched to the studio course right away; its media are hard links into the hidden import folder. + The course's studio page shows **Conversion incomplete** with a **Retry** button for the owner. A retry (or the + next server start) finishes the move. A failure before the commit is rolled back, and learners keep the import + unchanged. ## Memory budget diff --git a/docs/superpowers/specs/2026-10-30-stcourses-design.md b/docs/superpowers/specs/2026-10-30-stcourses-design.md index bf00889..0462bc3 100644 --- a/docs/superpowers/specs/2026-10-30-stcourses-design.md +++ b/docs/superpowers/specs/2026-10-30-stcourses-design.md @@ -181,6 +181,7 @@ Phase 0 (section 10) pins down the exact schema. These principles are fixed: - The two reference courses must load by adding only a manifest and front matter, without moving or rewriting their existing files. - Their existing tooling (test scripts, `tools/*.mjs`) must keep working. - **Derived data lives in a cache directory that can be rebuilt from the files:** search index, slide images and text, transcripts, video posters and AI embeddings. +- **Discovery** (v1.1 note): the platform finds `stcourse.yaml` at depth 0–2 under each course directory and skips folders whose name starts with `.` (and `node_modules`). A course folder must therefore not be a dot-folder. The studio relies on this to hide an import folder while it is converted (`.converting-`, `.converted-.`). ## 6. Accounts, roles and visibility diff --git a/packages/feature-studio/src/server/feature.ts b/packages/feature-studio/src/server/feature.ts index 46f430d..5f819b6 100644 --- a/packages/feature-studio/src/server/feature.ts +++ b/packages/feature-studio/src/server/feature.ts @@ -6,7 +6,7 @@ import { ManifestSchema, parseManifest, resolveInside, type LoadedCourse } from import { aclRepo } from "@stc/db"; import { createDraftWriter, type DraftWriter } from "./draft-writer"; import { storeDirs, studioWorktreesDir } from "./paths"; -import { checkRelPath, createStudioStore, MANIFEST, ownerRequiredFor, StudioError, type FileWrite, type StudioStore } from "./store"; +import { checkRelPath, createStudioStore, MANIFEST, ownerRequiredFor, StudioError, type FileWrite, type StudioStore, type StudioStoreOptions } from "./store"; import { createPublishedWatcher, type PublishedWatcher } from "./watch"; export const OUTSIDE_MESSAGE = "This course is managed outside STCourses"; @@ -58,6 +58,8 @@ export interface StudioFeatureOptions { /** Poll the published worktrees for external commits (default true). */ watch?: boolean; watchIntervalMs?: number; + /** Tests only: conversion failure injection (see StudioStoreOptions.testHooks). */ + storeTestHooks?: StudioStoreOptions["testHooks"]; } const json = (body: unknown, status = 200) => Response.json(body, { status }); @@ -76,7 +78,7 @@ function manifestToYaml(m: unknown): string { } export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = {}): StudioFeature { - const store = createStudioStore(storeDirs(ctx.config)); + const store = createStudioStore({ ...storeDirs(ctx.config), testHooks: opts.storeTestHooks }); const watcher = createPublishedWatcher(store, ctx.courses, { intervalMs: opts.watchIntervalMs }); const acl = aclRepo(ctx.db); const wtDir = resolve(ctx.config.studioWorktreesDir ?? studioWorktreesDir(ctx.config.dataDir)); @@ -117,18 +119,27 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { const p = (j.payload ?? {}) as { courseId?: unknown; options?: { courseId?: unknown } }; return p.courseId === courseId || p.options?.courseId === courseId; }); - const IMPORT_RUNNING = "an import job for this course is still running; make it editable once it has finished"; + /** 409 text naming the blocking importer jobs (id and kind), so the owner can find them on the import page. */ + const importRunning = (jobs: { id: string; kind: string }[]) => + `an import job for this course is still running (${jobs.map((j) => `${j.kind} ${j.id}`).join(", ")}); make it editable once it has finished`; /** The conversion job: payload `{ courseId, actor }`, checked again by the store under the course lock. */ const convertJob: JobHandler = async (payload, jc) => { await ready; const p = (payload ?? {}) as { courseId?: unknown; actor?: { id?: unknown; name?: unknown; email?: unknown } }; const courseId = typeof p.courseId === "string" ? p.courseId : ""; - if ((await pendingImports(courseId)).length) throw new StudioError(IMPORT_RUNNING, 409); + const blocking = await pendingImports(courseId); + if (blocking.length) throw new StudioError(importRunning(blocking), 409); const actor = { name: String(p.actor?.name ?? "STCourses"), email: String(p.actor?.email ?? "studio@stcourses.invalid") }; - const r = await store.convertImported(courseId, actor, { onProgress: (f, m) => jc.reportProgress(f, m) }); - await afterPublishedChange(courseId); - return { courseId, ...r }; + try { + const r = await store.convertImported(courseId, actor, { onProgress: (f, m) => jc.reportProgress(f, m) }); + return { courseId, ...r }; + } finally { + // Success or failure, learners must get whatever is on disk now: the intact import (failure before the commit, + // rolled back) or the published worktree (after it), never a course whose folder has moved away. + await ctx.courses.reload(courseId).catch((e) => console.error("[studio] reload after conversion", e)); + if (store.isManaged(courseId)) await store.publishedHead(courseId).then((h) => watcher.note(courseId, h), () => {}); + } }; const afterPublishedChange = async (courseId: string) => { @@ -184,6 +195,8 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { } return { readOnly: false, + conversionIncomplete: store.conversionIncomplete(courseId), + canRetryConversion: isOwner(user), course: { id: courseId, title: (manifestRaw as { title?: string } | null)?.title ?? courseId, managed: true }, status, manifestText: m?.content ?? "", @@ -257,14 +270,17 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { POST: guard(async (_req, rc, user) => { const courseId = rc.params.courseId ?? ""; if (!isOwner(user)) return fail(403, "only the owner can make an imported course editable in the studio"); - if (store.isManaged(courseId)) return fail(409, `course "${courseId}" is already a studio course`); - if (!store.isConvertibleImport(courseId)) { + // An incomplete conversion (failed after its commit) is finished by a new job: it recovers first. + const incomplete = store.conversionIncomplete(courseId); + if (store.isManaged(courseId) && !incomplete) return fail(409, `course "${courseId}" is already a studio course`); + if (!incomplete && !store.isConvertibleImport(courseId)) { const c = ctx.courses.get(courseId); return c ? fail(409, `${readOnlyMessage(c) === IMPORTED_MESSAGE ? "this imported course" : OUTSIDE_MESSAGE}: only imported courses in ${importedRoot} can be made editable`) : fail(404, "no such imported course"); } const pending = (await ctx.jobs.list(CONVERT_JOB)).find((j) => (j.payload as { courseId?: string } | null)?.courseId === courseId && (j.status === "queued" || j.status === "running")); if (pending) return fail(409, "this course is already being made editable"); - if ((await pendingImports(courseId)).length) return fail(409, IMPORT_RUNNING); + const blocking = await pendingImports(courseId); + if (blocking.length) return fail(409, importRunning(blocking)); const jobId = await ctx.jobs.enqueue(CONVERT_JOB, { courseId, actor: { id: user.id, name: who(user).name, email: who(user).email } }); return json({ jobId }, 202); }), diff --git a/packages/feature-studio/src/server/git.ts b/packages/feature-studio/src/server/git.ts index 68a2895..074e051 100644 --- a/packages/feature-studio/src/server/git.ts +++ b/packages/feature-studio/src/server/git.ts @@ -55,15 +55,3 @@ export async function gitOk(cwd: string, args: string[], who?: Identity): Promis export function splitZ(out: string): string[] { return out.split("\0").filter(Boolean); } - -const locks = new Map>(); - -/** Serialises git work per key (the course's bare repo path) across every store instance in the process. */ -export function withLock(key: string, fn: () => Promise): Promise { - const prev = locks.get(key) ?? Promise.resolve(); - const next = prev.then(fn, fn); - const tail = next.catch(() => {}); - locks.set(key, tail); - tail.then(() => { if (locks.get(key) === tail) locks.delete(key); }); - return next; -} diff --git a/packages/feature-studio/src/server/store.ts b/packages/feature-studio/src/server/store.ts index 8ad3e65..799ca93 100644 --- a/packages/feature-studio/src/server/store.ts +++ b/packages/feature-studio/src/server/store.ts @@ -146,8 +146,12 @@ export interface StudioStoreOptions { publishedBranch?: string; /** Tests only: allow local-path remotes (the HTTP feature never sets this). */ allowLocalRemotes?: boolean; - /** Tests only: simulate a crash at a named conversion step, or a cross-filesystem media move. */ - testHooks?: { crashAt?: string; forceExdev?: boolean }; + /** + * Tests only, read on every use (a test may change them between calls): simulate a crash (`crashAt`) or an ordinary + * failure (`failAt`) at a named conversion step, a cross-filesystem media move (`forceExdev`), or a copy whose + * bytes do not match MEDIA_REFS (`corruptStaging`). + */ + testHooks?: { crashAt?: string; failAt?: string; forceExdev?: boolean; corruptStaging?: boolean }; } export function createStudioStore(opts: StudioStoreOptions) { @@ -188,7 +192,10 @@ export function createStudioStore(opts: StudioStoreOptions) { const lock = (courseId: string, fn: () => Promise) => { paths(courseId); return withCourseLock(courseId, fn); }; const convertingDir = (courseId: string) => join(importedDir, `.converting-${courseId}`); const listDir = (dir: string): string[] => { try { return readdirSync(dir); } catch { return []; } }; - const crash = async (step: string) => { if (opts.testHooks?.crashAt === step) throw new SimulatedCrash(step); }; + const crash = async (step: string) => { + if (opts.testHooks?.crashAt === step) throw new SimulatedCrash(step); + if (opts.testHooks?.failAt === step) throw new Error(`injected failure at ${step}`); + }; const rename = async (from: string, to: string) => fsRename(from, to); const rev = async (cwd: string, ref: string) => (await gitOk(cwd, ["rev-parse", ref])).trim(); @@ -380,6 +387,7 @@ export function createStudioStore(opts: StudioStoreOptions) { const staging = join(mediaDir, `.staging-${courseId}.${Bun.randomUUIDv7()}`); try { await cp(from, staging, { recursive: true, verbatimSymlinks: true, errorOnExist: true, force: false }); + if (opts.testHooks?.corruptStaging && refs[0]) await writeFile(join(staging, refs[0].path), "corrupted copy"); await crash("staging-copied"); await fsyncTree(staging); if (!(await mediaComplete(staging, refs, true))) throw new Error(`the copied media in ${staging} do not match ${MEDIA_REFS}`); @@ -545,6 +553,16 @@ export function createStudioStore(opts: StudioStoreOptions) { } }, + /** + * A conversion that got past its commit but did not finish (the media move failed or was interrupted): the course + * is served from the published worktree, and `convertImported` (which recovers first) completes it. + */ + conversionIncomplete(courseId: string): boolean { + if (!ID_RE.test(courseId)) return false; + if (lexists(convertingDir(courseId))) return true; + return listDir(mediaDir).some((n) => n.startsWith(`.staging-${courseId}.`)) || listDir(importedDir).some((n) => n.startsWith(`.converted-${courseId}.`)); + }, + /** Re-links referenced media into the published worktree (after an outside checkout removed them, say). */ repairMedia: (courseId: string) => lock(courseId, async () => { const p = requireManaged(courseId); diff --git a/packages/feature-studio/src/ui/ConvertPanel.tsx b/packages/feature-studio/src/ui/ConvertPanel.tsx index fd215e0..3288d1d 100644 --- a/packages/feature-studio/src/ui/ConvertPanel.tsx +++ b/packages/feature-studio/src/ui/ConvertPanel.tsx @@ -34,8 +34,8 @@ export function ConvertPanelView({ phase, onConvert }: { phase: ConvertPhase; on ); } -/** Starts the conversion job and polls it; calls `onDone` once the course is a studio course. */ -export function ConvertPanel({ courseId, onDone }: { courseId: string; onDone: () => void }) { +/** Starts the conversion job (`POST …/convert`) and polls it; calls `onDone` once it has finished. */ +function useConvertJob(courseId: string, onDone: () => void) { const [phase, setPhase] = useState({ kind: "idle" }); const live = useRef(true); useEffect(() => () => { live.current = false; }, []); @@ -54,9 +54,37 @@ export function ConvertPanel({ courseId, onDone }: { courseId: string; onDone: ( setPhase({ kind: "running", progress: j.data.progress ?? 0, message: j.data.message }); } }; + return { phase, start }; +} + +export function ConvertPanel({ courseId, onDone }: { courseId: string; onDone: () => void }) { + const { phase, start } = useConvertJob(courseId, onDone); return ; } +/** A conversion that failed after its commit: the course is served, but its media move did not finish. */ +export function IncompleteConversionView({ phase, canRetry, onRetry }: { phase: ConvertPhase; canRetry: boolean; onRetry: () => void }) { + return ( +
+ Conversion incomplete. Learners see this course, but moving its imported media into place did not + finish. {canRetry ? "Retry to complete it." : "Ask the owner to retry it."} + {phase.kind === "running" ? ( +
+ + {phase.message ?? "Waiting to start…"} +
+ ) : null} + {phase.kind === "failed" ?

{phase.error}

: null} + {canRetry ?
Retry
: null} +
+ ); +} + +export function IncompleteConversion({ courseId, canRetry, onDone }: { courseId: string; canRetry: boolean; onDone: () => void }) { + const { phase, start } = useConvertJob(courseId, onDone); + return ; +} + /** What the studio shows for a course it does not manage. */ export function ReadOnlyView({ courseId, info, onConverted }: { courseId: string; info: ReadOnlyInfo; onConverted: () => void }) { if (info.imported) { diff --git a/packages/feature-studio/src/ui/CourseStudio.tsx b/packages/feature-studio/src/ui/CourseStudio.tsx index f48a104..f7084e9 100644 --- a/packages/feature-studio/src/ui/CourseStudio.tsx +++ b/packages/feature-studio/src/ui/CourseStudio.tsx @@ -3,7 +3,7 @@ import { Link, useParams, useSearchParams } from "react-router-dom"; import { CourseCover, Pill } from "@stc/ui"; import { api, coursePath, uniqueId, type Diagnostic, type ManifestLesson } from "./api"; import { AccessMessage } from "./StudioHome"; -import { ReadOnlyView } from "./ConvertPanel"; +import { IncompleteConversion, ReadOnlyView } from "./ConvertPanel"; import { clone, useWorkspace, type Workspace } from "./workspace"; import { CourseSettings } from "./editors/CourseSettings"; import { LessonEditor } from "./editors/LessonEditor"; @@ -107,6 +107,8 @@ function Workbench({ ws }: { ws: Workspace }) { Publish + {ws.state.conversionIncomplete ? : null} + {notice ? (
{notice.text} diff --git a/packages/feature-studio/src/ui/api.ts b/packages/feature-studio/src/ui/api.ts index b0fad74..ec8b49d 100644 --- a/packages/feature-studio/src/ui/api.ts +++ b/packages/feature-studio/src/ui/api.ts @@ -42,7 +42,9 @@ export interface DraftStatus { export interface Diagnostic { level: "error" | "warning"; lessonId?: string; path?: string; message: string } export interface CourseState { - readOnly: false; course: { id: string; title: string }; status: DraftStatus; manifestText: string; manifestHash: string | null; + readOnly: false; course: { id: string; title: string }; status: DraftStatus; + /** A conversion from an import that did not finish its media move (v1.1 S); the owner may retry it. */ + conversionIncomplete?: boolean; canRetryConversion?: boolean; manifestText: string; manifestHash: string | null; manifestRaw: ManifestDoc | null; manifestErrors: string[]; diagnostics: Diagnostic[]; files: string[]; } export interface ReadOnlyState { readOnly: true; message: string; imported?: boolean; convertible?: boolean; course: { id: string; title: string; root: string }; diagnostics: Diagnostic[] } diff --git a/packages/feature-studio/test/app-helpers.ts b/packages/feature-studio/test/app-helpers.ts index b5e456d..32213ba 100644 --- a/packages/feature-studio/test/app-helpers.ts +++ b/packages/feature-studio/test/app-helpers.ts @@ -3,7 +3,7 @@ import type { AppConfig, AuthUser, JobQueue, ServerContext } from "@stc/contract import { createApp, createJobQueue, resolveConfig } from "@stc/core"; import { createCourseRepository } from "@stc/course-format"; import { connect, migrate } from "@stc/db"; -import { studioFeature } from "../src/server/feature"; +import { studioFeature, type StudioFeatureOptions } from "../src/server/feature"; import { studioCourseDirs } from "../src/server/paths"; import { tempDir } from "./helpers"; @@ -15,7 +15,7 @@ export const users: Record = { }; /** A server context like main.ts builds, with header-selected users (`x-user: ada`). */ -export async function setupApp(extraCourseDirs: string[] = [], dbUrl = "sqlite://:memory:", prepare?: (dataDir: string) => void, configOverrides: Partial = {}) { +export async function setupApp(extraCourseDirs: string[] = [], dbUrl = "sqlite://:memory:", prepare?: (dataDir: string) => void, configOverrides: Partial = {}, featureOpts: StudioFeatureOptions = {}) { const dataDir = tempDir(); prepare?.(dataDir); let dropSchema = async () => {}; @@ -37,7 +37,7 @@ export async function setupApp(extraCourseDirs: string[] = [], dbUrl = "sqlite:/ config, db, courses, jobs: null as unknown as JobQueue, auth: { userFromRequest: async (req: Request) => users[req.headers.get("x-user") ?? ""] ?? null }, } satisfies ServerContext; - const feature = studioFeature(ctx, { watch: false }); + const feature = studioFeature(ctx, { watch: false, ...featureOpts }); // the real queue, as composeServer builds it from the features' jobs const queue = createJobQueue(db, feature.jobs ?? {}, { pollMs: 20 }); ctx.jobs = queue; diff --git a/packages/feature-studio/test/convert-routes.test.ts b/packages/feature-studio/test/convert-routes.test.ts index ad958d8..29cdc23 100644 --- a/packages/feature-studio/test/convert-routes.test.ts +++ b/packages/feature-studio/test/convert-routes.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { existsSync, readFileSync } from "node:fs"; +import { existsSync, readdirSync, readFileSync } from "node:fs"; import { join, resolve } from "node:path"; import { studioFeature } from "../src/server/feature"; import { SimulatedCrash } from "../src/server/convert"; @@ -84,9 +84,11 @@ describe.each(dbs)("conversion and importer jobs [%s]", (_name, dbUrl) => { env = await setupApp([], dbUrl, (d) => makeImportedCourse(d)); // no importer feature in this app: its jobs stay queued await env.ctx.jobs.enqueue("import.code-repo", { kind: "code-repo", path: "/x", options: { courseId: "imp" } }); + const blocker = await env.ctx.jobs.list("import.code-repo"); const r = await env.call("owner", "POST", "/api/studio/courses/imp/convert"); expect(r.status).toBe(409); expect(r.json.error).toContain("an import job for this course is still running"); + expect(r.json.error).toContain(`import.code-repo ${blocker[0]!.id}`); // a convert job that was queued anyway (before the import was) fails cleanly and changes nothing const id = await env.ctx.jobs.enqueue("studio.convert-import", { courseId: "imp", actor: { id: "u-own", name: "O", email: "o@x" } }); const j = await env.waitJob(id); @@ -122,6 +124,61 @@ describe.each(dbs)("conversion and importer jobs [%s]", (_name, dbUrl) => { }); }); +describe.each(dbs)("a conversion that fails without a restart [%s]", (_name, dbUrl) => { + const media = (as: string) => env.app.fetch(new Request(`http://x/api/media/imp/${VIDEO.split("/").map(encodeURIComponent).join("/")}`, { headers: { "x-user": as, range: "bytes=10-19" } })); + const learnerView = async (where: "import" | "studio") => { + const all = env.ctx.courses.list().filter((c) => c.id === "imp"); + expect(all).toHaveLength(1); + expect(all[0]!.status).toBe("ok"); + expect(all[0]!.root).toBe(join(env.dataDir, where === "import" ? "courses/imp" : "worktrees/imp/published")); + const r = await media("lea"); + expect(r.status).toBe(206); + expect(new Uint8Array(await r.arrayBuffer())).toEqual(videoBytes(4096, 7).slice(10, 20)); + }; + + const cases: [string, { forceExdev?: boolean; failAt?: string; corruptStaging?: boolean }, "import" | "studio", RegExp][] = [ + ["the cross-filesystem copy fails (disk full)", { forceExdev: true, failAt: "staging-copied" }, "studio", /injected failure at staging-copied/], + ["the copied media fail their checksum", { forceExdev: true, corruptStaging: true }, "studio", /do not match/], + ["the checkout fails before the commit point", { failAt: "worktree" }, "import", /injected failure at worktree/], + ]; + test.each(cases)("%s: learners keep a working course and the owner's retry completes", async (_c, hooks, after, error) => { + env = await setupApp([], dbUrl, (d) => makeImportedCourse(d), {}, { storeTestHooks: hooks }); + await env.db.sql`INSERT INTO course_acl (course_id, principal_type, principal_id) VALUES ('imp', 'user', 'u-lea')`; + await learnerView("import"); + + const r = await env.call("owner", "POST", "/api/studio/courses/imp/convert"); + expect(r.status).toBe(202); + const j = await env.waitJob(r.json.jobId); + expect(j.status).toBe("failed"); + expect(j.error).toMatch(error); + // no restart: learners already get the intact import or the finished studio course + await learnerView(after); + + if (after === "studio") { + const st = (await env.call("owner", "GET", "/api/studio/courses/imp")).json; + expect(st).toMatchObject({ readOnly: false, conversionIncomplete: true, canRetryConversion: true }); + expect((await env.call("ada", "GET", "/api/studio/courses/imp")).json).toMatchObject({ conversionIncomplete: true, canRetryConversion: false }); + } else { + expect((await env.call("owner", "GET", "/api/studio/courses/imp")).json).toMatchObject({ readOnly: true, convertible: true }); + } + + // the cause goes away (space freed, disk fixed); the owner retries through the same route + delete hooks.failAt; + delete hooks.corruptStaging; + expect((await env.call("ada", "POST", "/api/studio/courses/imp/convert")).status).toBe(403); + const again = await env.call("owner", "POST", "/api/studio/courses/imp/convert"); + expect(again.status).toBe(202); + const j2 = await env.waitJob(again.json.jobId); + expect(j2.error).toBeNull(); + expect(j2.status).toBe("done"); + await learnerView("studio"); + expect((await env.call("owner", "GET", "/api/studio/courses/imp")).json.conversionIncomplete).toBe(false); + expect(readdirSync(join(env.dataDir, "courses"))).toEqual([]); + expect(readdirSync(join(env.dataDir, "media"))).toEqual(["imp"]); + expect((await env.call("owner", "POST", "/api/studio/courses/imp/convert")).status).toBe(409); + }); +}); + describe("convert route refusals", () => { test("only the owner; cross-site requests are refused; nothing changes", async () => { env = await setupApp([], undefined, (d) => makeImportedCourse(d)); diff --git a/packages/feature-studio/test/convert-ui.test.tsx b/packages/feature-studio/test/convert-ui.test.tsx index a26d303..c8359a8 100644 --- a/packages/feature-studio/test/convert-ui.test.tsx +++ b/packages/feature-studio/test/convert-ui.test.tsx @@ -1,6 +1,6 @@ import { expect, test } from "bun:test"; import { renderToString } from "react-dom/server"; -import { ConvertPanelView, ReadOnlyView } from "../src/ui/ConvertPanel"; +import { ConvertPanelView, IncompleteConversionView, ReadOnlyView } from "../src/ui/ConvertPanel"; const noop = () => {}; @@ -36,3 +36,16 @@ test("read-only view: imported for the owner offers the action; for others expla expect(outside).toContain("/repos/o"); expect(outside).not.toContain(" { + const owner = renderToString(); + expect(owner).toContain("Conversion incomplete"); + expect(owner).toContain(">Retry"); + const author = renderToString(); + expect(author).toContain("Conversion incomplete"); + expect(author).not.toContain("); + expect(failed).toContain('role="alert"'); + expect(failed).toContain("disk full"); +});