From afa2338ad3804f984f59f8db48f698507008d35a Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 20:32:59 +0100 Subject: [PATCH] feat(studio): owner-only "Make editable in the studio" route and conversion job POST /api/studio/courses/:id/convert (owner only, CSRF-guarded by core) queues studio.convert-import; GET /api/studio/jobs/:id reports progress. Read-only responses say whether the caller may convert. Resolves TODO(studio) (ruling R9). Co-Authored-By: Claude Opus 5.5 --- packages/feature-studio/src/index.ts | 2 +- packages/feature-studio/src/server/feature.ts | 61 +++++++-- packages/feature-studio/test/app-helpers.ts | 21 ++- .../test/convert-routes.test.ts | 128 ++++++++++++++++++ packages/feature-studio/test/security.test.ts | 7 +- 5 files changed, 201 insertions(+), 18 deletions(-) create mode 100644 packages/feature-studio/test/convert-routes.test.ts diff --git a/packages/feature-studio/src/index.ts b/packages/feature-studio/src/index.ts index 6ced9bb..4889acf 100644 --- a/packages/feature-studio/src/index.ts +++ b/packages/feature-studio/src/index.ts @@ -1,5 +1,5 @@ // Server entry of @stc/feature-studio. The browser imports "@stc/feature-studio/web" instead (no server code). -export { studioFeature, OUTSIDE_MESSAGE, IMPORTED_MESSAGE, uploadMaxBytes, UPLOAD_EXTENSIONS, type StudioFeature, type StudioFeatureOptions } from "./server/feature"; +export { studioFeature, OUTSIDE_MESSAGE, IMPORTED_MESSAGE, CONVERT_JOB, uploadMaxBytes, UPLOAD_EXTENSIONS, type StudioFeature, type StudioFeatureOptions } from "./server/feature"; export { createDraftWriter, type DraftWriter, type DraftWriterOptions } from "./server/draft-writer"; export { createStudioStore, contentHash, isAllowedRemote, ownerRequiredFor, type StudioStore, type Identity, type DraftStatus, type PublishResult, type SyncResult } from "./server/store"; export { createPublishedWatcher, type PublishedWatcher } from "./server/watch"; diff --git a/packages/feature-studio/src/server/feature.ts b/packages/feature-studio/src/server/feature.ts index 9d1d4ca..49a3185 100644 --- a/packages/feature-studio/src/server/feature.ts +++ b/packages/feature-studio/src/server/feature.ts @@ -1,6 +1,6 @@ import { existsSync, realpathSync } from "node:fs"; import { basename, extname, resolve, sep } from "node:path"; -import type { AuthUser, Feature, RequestContext, RouteHandler, ServerContext } from "@stc/contracts"; +import type { AuthUser, Feature, JobHandler, RequestContext, RouteHandler, ServerContext } from "@stc/contracts"; import { canView, serveMedia } from "@stc/core"; import { ManifestSchema, parseManifest, resolveInside, type LoadedCourse } from "@stc/course-format"; import { aclRepo } from "@stc/db"; @@ -10,7 +10,8 @@ import { checkRelPath, createStudioStore, MANIFEST, ownerRequiredFor, StudioErro import { createPublishedWatcher, type PublishedWatcher } from "./watch"; export const OUTSIDE_MESSAGE = "This course is managed outside STCourses"; -export const IMPORTED_MESSAGE = "Imported course: read-only in the studio for now"; +export const IMPORTED_MESSAGE = "Imported course: not editable in the studio until the owner makes it editable"; +export const CONVERT_JOB = "studio.convert-import"; /** Upload cap in bytes from AppConfig.uploadMaxMb (STC_UPLOAD_MAX_MB, default 100; review I2). */ export function uploadMaxBytes(mb: number | undefined): number { @@ -44,6 +45,7 @@ async function readCapped(req: Request, max: number): Promise; store: StudioStore; watcher: PublishedWatcher; draftWriter: DraftWriter; @@ -99,13 +101,22 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { const readOnlyMessage = (c: LoadedCourse): string => { const kind = c.manifest?.source?.kind; const underImports = c.root === importedRoot || c.root.startsWith(importedRoot + sep); - if (kind && kind !== "authored" && underImports) { - // TODO(studio): convert imported course to a studio-managed repo (git init into /studio/repos/.git - // plus a published worktree), so authors can edit it and the owner-only visibility rule applies on publish. - return IMPORTED_MESSAGE; - } + if (kind && kind !== "authored" && underImports) return IMPORTED_MESSAGE; return OUTSIDE_MESSAGE; }; + const isOwner = (u: AuthUser) => u.roles.includes("owner"); + /** v1.1 S: only the owner may turn an imported course into a studio course ("Make editable in the studio"). */ + const convertible = (u: AuthUser, courseId: string) => isOwner(u) && store.isConvertibleImport(courseId); + + /** The conversion job: payload `{ courseId, actor }`, checked again by the store under the course lock. */ + const convertJob: JobHandler = async (payload, jc) => { + const p = (payload ?? {}) as { courseId?: unknown; actor?: { id?: unknown; name?: unknown; email?: unknown } }; + const courseId = typeof p.courseId === "string" ? p.courseId : ""; + 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 }; + }; const afterPublishedChange = async (courseId: string) => { await ctx.courses.reload(courseId); @@ -188,7 +199,11 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { if (all) { for (const c of ctx.courses.list()) { if (isStudioCourse(c) || managed.includes(c.id)) continue; - courses.push({ id: c.id, managed: false, title: c.manifest?.title ?? c.id, pitch: c.manifest?.pitch ?? null, accent: c.manifest?.accent ?? null, publishedStatus: c.status, message: readOnlyMessage(c) }); + const message = readOnlyMessage(c); + courses.push({ + id: c.id, managed: false, title: c.manifest?.title ?? c.id, pitch: c.manifest?.pitch ?? null, accent: c.manifest?.accent ?? null, publishedStatus: c.status, message, + imported: message === IMPORTED_MESSAGE, convertible: convertible(user, c.id), + }); } } return json({ courses, canCreate: all }); @@ -212,12 +227,38 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { if (!store.isManaged(courseId)) { const c = ctx.courses.get(courseId); if (!c) return fail(404, "no such course"); - return json({ readOnly: true, message: readOnlyMessage(c), course: { id: c.id, title: c.manifest?.title ?? c.id, managed: false, root: c.root }, diagnostics: c.diagnostics }); + const message = readOnlyMessage(c); + return json({ readOnly: true, message, imported: message === IMPORTED_MESSAGE, convertible: convertible(user, c.id), course: { id: c.id, title: c.manifest?.title ?? c.id, managed: false, root: c.root }, diagnostics: c.diagnostics }); } return json(await courseState(courseId, user)); }), }, + "/api/studio/courses/:courseId/convert": { + 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)) { + 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"); + 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); + }), + }, + + "/api/studio/jobs/:jobId": { + GET: guard(async (_req, rc, user) => { + if (!isOwner(user)) return fail(403, "only the owner can follow studio jobs"); + const j = await ctx.jobs.get(rc.params.jobId ?? ""); + if (!j || !j.kind.startsWith("studio.")) return fail(404, "no such job"); + return json({ id: j.id, kind: j.kind, status: j.status, progress: j.progress, message: j.message, error: j.error, result: j.result }); + }), + }, + "/api/studio/courses/:courseId/file": { GET: course(async (_req, rc, user, courseId) => { const path = rc.url.searchParams.get("path") ?? ""; @@ -364,5 +405,5 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { watcher.start(); } - return { name: "studio", routes, multipartRoutes: ["/api/studio/courses/:courseId/upload"], store, watcher, draftWriter, stop: () => watcher.stop() }; + return { name: "studio", routes, jobs: { [CONVERT_JOB]: convertJob }, multipartRoutes: ["/api/studio/courses/:courseId/upload"], store, watcher, draftWriter, stop: () => watcher.stop() }; } diff --git a/packages/feature-studio/test/app-helpers.ts b/packages/feature-studio/test/app-helpers.ts index ea53c32..b5e456d 100644 --- a/packages/feature-studio/test/app-helpers.ts +++ b/packages/feature-studio/test/app-helpers.ts @@ -1,6 +1,6 @@ import { resolve } from "node:path"; -import type { AppConfig, AuthUser, ServerContext } from "@stc/contracts"; -import { createApp, resolveConfig } from "@stc/core"; +import type { AppConfig, AuthUser, JobQueue, ServerContext } from "@stc/contracts"; +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"; @@ -34,10 +34,14 @@ export async function setupApp(extraCourseDirs: string[] = [], dbUrl = "sqlite:/ const config = resolveConfig({ port: 0, databaseUrl: "sqlite://:memory:", dataDir, courseDirs: extraCourseDirs, secret: "t", publicUrl: "http://x", ...configOverrides }); const courses = await createCourseRepository(studioCourseDirs(config)); const ctx = { - config, db, courses, jobs: null as never, + 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 }); + // the real queue, as composeServer builds it from the features' jobs + const queue = createJobQueue(db, feature.jobs ?? {}, { pollMs: 20 }); + ctx.jobs = queue; + queue.start(); const app = createApp(ctx, [feature]); const call = async (as: string | null, method: string, path: string, body?: unknown) => { const headers: Record = { origin: "http://x" }; @@ -51,5 +55,14 @@ export async function setupApp(extraCourseDirs: string[] = [], dbUrl = "sqlite:/ try { json = JSON.parse(text); } catch { json = text; } return { status: r.status, json }; }; - return { ctx, db, dataDir, feature, app, call, stop: async () => { feature.stop(); await db.close(); await dropSchema(); } }; + /** Polls a job until it is done or failed. */ + const waitJob = async (id: string) => { + for (let i = 0; i < 500; i++) { + const j = await queue.get(id); + if (j && (j.status === "done" || j.status === "failed")) return j; + await Bun.sleep(20); + } + throw new Error(`job ${id} did not finish`); + }; + return { ctx, db, dataDir, feature, app, call, waitJob, stop: async () => { feature.stop(); await queue.stop(); await db.close(); await dropSchema(); } }; } diff --git a/packages/feature-studio/test/convert-routes.test.ts b/packages/feature-studio/test/convert-routes.test.ts new file mode 100644 index 0000000..2f8f87a --- /dev/null +++ b/packages/feature-studio/test/convert-routes.test.ts @@ -0,0 +1,128 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { existsSync, readFileSync } from "node:fs"; +import { join, resolve } from "node:path"; +import { REPO, setupApp } from "./app-helpers"; +import { makeImportedCourse, VIDEO, videoBytes } from "./import-fixture"; + +let env: Awaited>; +afterEach(() => env?.stop()); + +const PG = process.env.STC_TEST_PG_URL; +const dbs: [string, string][] = [["sqlite", "sqlite://:memory:"], ...(PG ? [["postgres", PG] as [string, string]] : [])]; + +async function convert(as = "owner", id = "imp") { + const r = await env.call(as, "POST", `/api/studio/courses/${id}/convert`); + expect(r.status).toBe(202); + const job = await env.waitJob(r.json.jobId); + expect(job.error).toBeNull(); + expect(job.status).toBe("done"); + return job; +} + +describe.each(dbs)("convert routes [%s]", (_name, dbUrl) => { + test("the owner converts; the course is then served from the published worktree, still restricted, ACL kept", async () => { + env = await setupApp([], dbUrl, (d) => makeImportedCourse(d)); + const { call, ctx, db } = env; + await db.sql`INSERT INTO course_acl (course_id, principal_type, principal_id) VALUES ('imp', 'user', 'u-lea')`; + expect(ctx.courses.get("imp")?.root).toBe(join(env.dataDir, "courses/imp")); + + const ro = (await call("owner", "GET", "/api/studio/courses/imp")).json; + expect(ro).toMatchObject({ readOnly: true, convertible: true }); + expect((await call("ada", "GET", "/api/studio/courses/imp")).json).toMatchObject({ readOnly: true, convertible: false }); + expect((await call("owner", "GET", "/api/studio/courses")).json.courses).toContainEqual(expect.objectContaining({ id: "imp", managed: false, imported: true, convertible: true })); + + const job = await convert(); + expect(job.result).toMatchObject({ courseId: "imp", media: 3, resumed: false }); + const c = ctx.courses.get("imp")!; + expect(c.root).toBe(join(env.dataDir, "worktrees/imp/published")); + expect(c.status).toBe("ok"); + expect(c.manifest?.visibility).toBe("restricted"); + expect(ctx.courses.list().filter((x) => x.id === "imp")).toHaveLength(1); + expect(await db.sql`SELECT principal_id FROM course_acl WHERE course_id = 'imp'`).toEqual([{ principal_id: "u-lea" }]); + + // media through core's route, Range included + const r = await env.app.fetch(new Request(`http://x/api/media/imp/${VIDEO.split("/").map(encodeURIComponent).join("/")}`, { headers: { "x-user": "lea", range: "bytes=10-19" } })); + expect(r.status).toBe(206); + expect(new Uint8Array(await r.arrayBuffer())).toEqual(videoBytes(4096, 7).slice(10, 20)); + // anonymous users still see nothing + expect((await env.app.fetch(new Request(`http://x/api/media/imp/${encodeURI(VIDEO)}`))).status).toBe(404); + + // the studio now edits it + const st = (await call("owner", "GET", "/api/studio/courses/imp")).json; + expect(st.readOnly).toBe(false); + expect(st.files).toContain(VIDEO); + expect((await call("owner", "GET", "/api/studio/courses")).json.courses).toContainEqual(expect.objectContaining({ id: "imp", managed: true })); + }); + + test("an author edits and publishes a lesson title but cannot make the course public; the owner can", async () => { + env = await setupApp([], dbUrl, (d) => makeImportedCourse(d)); + const { call, ctx } = env; + await convert(); + let st = (await call("ada", "GET", "/api/studio/courses/imp")).json; + const m = structuredClone(st.manifestRaw); + m.modules[0].lessons[0].title = "Welcome aboard"; + expect((await call("ada", "PUT", "/api/studio/courses/imp/files", { message: "Rename", manifest: m, manifestBaseHash: st.manifestHash })).status).toBe(200); + expect((await call("ada", "POST", "/api/studio/courses/imp/publish")).json).toMatchObject({ ok: true, published: true }); + expect(ctx.courses.get("imp")?.lessonIndex.get("l1")?.lesson.title).toBe("Welcome aboard"); + + st = (await call("ada", "GET", "/api/studio/courses/imp")).json; + const pub = structuredClone(st.manifestRaw); + pub.visibility = "public"; + expect((await call("ada", "PUT", "/api/studio/courses/imp/files", { message: "Go public", manifest: pub, manifestBaseHash: st.manifestHash })).status).toBe(403); + st = (await call("owner", "GET", "/api/studio/courses/imp")).json; + expect((await call("owner", "PUT", "/api/studio/courses/imp/files", { message: "Go public", manifest: { ...st.manifestRaw, visibility: "public" }, manifestBaseHash: st.manifestHash })).status).toBe(200); + expect((await call("owner", "POST", "/api/studio/courses/imp/publish")).status).toBe(200); + expect(ctx.courses.get("imp")?.manifest?.visibility).toBe("public"); + }); +}); + +describe("convert route refusals", () => { + test("only the owner; cross-site requests are refused; nothing changes", async () => { + env = await setupApp([], undefined, (d) => makeImportedCourse(d)); + for (const who of ["ada", "lea"]) expect((await env.call(who, "POST", "/api/studio/courses/imp/convert")).status).toBe(403); + expect((await env.call(null, "POST", "/api/studio/courses/imp/convert")).status).toBe(401); + const forged = await env.app.fetch(new Request("http://x/api/studio/courses/imp/convert", { method: "POST", headers: { "x-user": "owner", origin: "http://evil.example" } })); + expect(forged.status).toBe(403); + const noOrigin = await env.app.fetch(new Request("http://x/api/studio/courses/imp/convert", { method: "POST", headers: { "x-user": "owner" } })); + expect(noOrigin.status).toBe(403); + expect(existsSync(join(env.dataDir, "courses/imp/stcourse.yaml"))).toBe(true); + expect(existsSync(join(env.dataDir, "studio/repos/imp.git"))).toBe(false); + expect(await env.ctx.jobs.list("studio.convert-import")).toEqual([]); + }); + + test("overlay, studio and unknown courses; a second request while one is pending", async () => { + env = await setupApp([resolve(REPO, "fixtures/courses")], undefined, (d) => makeImportedCourse(d)); + expect((await env.call("owner", "POST", "/api/studio/courses/sample/convert")).status).toBe(409); + expect((await env.call("owner", "POST", "/api/studio/courses/nope/convert")).status).toBe(404); + await env.call("owner", "POST", "/api/studio/courses", { id: "c1", title: "C" }); + expect((await env.call("owner", "POST", "/api/studio/courses/c1/convert")).status).toBe(409); + + await env.ctx.jobs.enqueue("studio.convert-import", { courseId: "imp", actor: { id: "u-own", name: "O", email: "o@x" } }); + // whether the first job is still queued, running or already done, a second request is refused + const r = await env.call("owner", "POST", "/api/studio/courses/imp/convert"); + expect(r.status).toBe(409); + }); + + test("job status is owner-only and limited to studio jobs", async () => { + env = await setupApp([], undefined, (d) => makeImportedCourse(d)); + const { jobId } = (await env.call("owner", "POST", "/api/studio/courses/imp/convert")).json; + expect((await env.call("ada", "GET", `/api/studio/jobs/${jobId}`)).status).toBe(403); + await env.waitJob(jobId); + const j = (await env.call("owner", "GET", `/api/studio/jobs/${jobId}`)).json; + expect(j).toMatchObject({ id: jobId, status: "done", progress: 1, error: null }); + const other = await env.ctx.jobs.enqueue("import.transcribe", { courseId: "x" }); + expect((await env.call("owner", "GET", `/api/studio/jobs/${other}`)).status).toBe(404); + }); + + test("a failed conversion reports the error and leaves the import as it was", async () => { + env = await setupApp([], undefined, (d) => makeImportedCourse(d)); + const manifest = join(env.dataDir, "courses/imp/stcourse.yaml"); + // the route only checks that this is an imported course; the job validates the manifest fully + await Bun.write(manifest, readFileSync(manifest, "utf8").replace("id: imp", "id: other")); + const { jobId } = (await env.call("owner", "POST", "/api/studio/courses/imp/convert")).json; + const j = await env.waitJob(jobId); + expect(j.status).toBe("failed"); + expect(j.error).toContain('"other"'); + expect(existsSync(join(env.dataDir, "studio/repos/imp.git"))).toBe(false); + }); +}); diff --git a/packages/feature-studio/test/security.test.ts b/packages/feature-studio/test/security.test.ts index e9b3690..0b5ec29 100644 --- a/packages/feature-studio/test/security.test.ts +++ b/packages/feature-studio/test/security.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, test } from "bun:test"; import { existsSync, mkdirSync, readFileSync, symlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; +import { IMPORTED_MESSAGE } from "../src/server/feature"; import { isAllowedRemote, ownerRequiredFor } from "../src/server/store"; import { ada, extGit, newStore, tempDir } from "./helpers"; import { setupApp } from "./app-helpers"; @@ -229,11 +230,11 @@ describe("I4: layout and imported courses", () => { }); expect(env.ctx.courses.get("imp")?.status).toBe("ok"); const list = (await env.call("ada", "GET", "/api/studio/courses")).json; - expect(list.courses).toContainEqual(expect.objectContaining({ id: "imp", managed: false, message: "Imported course: read-only in the studio for now" })); - expect((await env.call("ada", "GET", "/api/studio/courses/imp")).json.message).toBe("Imported course: read-only in the studio for now"); + expect(list.courses).toContainEqual(expect.objectContaining({ id: "imp", managed: false, message: IMPORTED_MESSAGE })); + expect((await env.call("ada", "GET", "/api/studio/courses/imp")).json.message).toBe(IMPORTED_MESSAGE); const w = await env.call("ada", "PUT", "/api/studio/courses/imp/files", { message: "x", files: [{ path: "a.md", content: "x" }] }); expect(w.status).toBe(403); - expect(w.json.error).toBe("Imported course: read-only in the studio for now"); + expect(w.json.error).toBe(IMPORTED_MESSAGE); expect((await env.call("ada", "POST", "/api/studio/courses", { id: "imp", title: "Dup" })).status).toBe(409); // studio repos are not in the importer's folder expect((await env.call("ada", "POST", "/api/studio/courses", { id: "c1", title: "C" })).status).toBe(201);