Make the extension the resource and review a state of it

src/services/extensions/v1/database.ts

@@ -1,4 +1,4 @@
-import { eq, sql } from "drizzle-orm";
+import { and, eq, isNotNull, sql } from "drizzle-orm";
 import { ExtensionsDb } from "../../../lib/db";
 import { extensions, developers } from "../v2/db/schema";
 import { DatabaseResult } from "../../../lib/interfaces";
@@ -14,7 +14,7 @@ import {
 const EXTENSION_COLUMNS = {
   id: extensions.id,
   type: extensions.type,
-  authorId: extensions.authorId,
+  developerId: extensions.developerId,
   authorType: developers.type,
   authorName: developers.name,
   authorUrl: developers.url,
@@ -30,16 +30,22 @@ const EXTENSION_COLUMNS = {
   downloadUrl: extensions.downloadUrl
 };
 
-// A LEFT JOIN can produce no matching developers row, so every joined
-// (author*) column is nullable regardless of developers' own NOT NULL
-// constraints - only extensions' own columns (besides iconUrl) are
-// guaranteed non-null.
+// An inner join: extensions.developer_id is NOT NULL with an enforced foreign
+// key, so the author* columns are as non-null as developers' own constraints
+// make them.
+//
+// extensions' own content columns became nullable in migration 0021, where an
+// extension row starts existing before it is published. They are still
+// non-null here because both queries below filter on published_at IS NOT NULL
+// and extensions_published_content_check makes that filter sufficient - a
+// published row cannot be missing any of them. That filter is also what keeps
+// unreviewed extensions out of the v1 catalogue.
 interface ExtensionRow {
   id: string;
   type: string;
-  authorId: string;
-  authorType: string | null;
-  authorName: string | null;
+  developerId: string;
+  authorType: string;
+  authorName: string;
   authorUrl: string | null;
   name: string;
   description: string;
@@ -59,11 +65,14 @@ export class ExtensionsDatabase {
   async getAllExtensions(type?: string): Promise<DatabaseResult<Extension[]>> {
     let rows: ExtensionRow[];
     try {
-      const query = this.db
+      const published = isNotNull(extensions.publishedAt);
+      rows = (await this.db
         .select(EXTENSION_COLUMNS)
         .from(extensions)
-        .leftJoin(developers, eq(extensions.authorId, developers.id));
-      rows = type ? await query.where(eq(extensions.type, type)) : await query;
+        .innerJoin(developers, eq(extensions.developerId, developers.id))
+        .where(
+          type ? and(published, eq(extensions.type, type)) : published
+        )) as ExtensionRow[];
     } catch (error) {
       return {
         data: null,
@@ -80,11 +89,16 @@ export class ExtensionsDatabase {
   async getExtensionById(id: string): Promise<DatabaseResult<Extension>> {
     let rows: ExtensionRow[];
     try {
-      rows = await this.db
+      rows = (await this.db
         .select(EXTENSION_COLUMNS)
         .from(extensions)
-        .leftJoin(developers, eq(extensions.authorId, developers.id))
-        .where(sql`LOWER(${extensions.id}) = LOWER(${id})`);
+        .innerJoin(developers, eq(extensions.developerId, developers.id))
+        .where(
+          and(
+            sql`LOWER(${extensions.id}) = LOWER(${id})`,
+            isNotNull(extensions.publishedAt)
+          )
+        )) as ExtensionRow[];
     } catch (error) {
       return {
         data: null,
@@ -118,9 +132,9 @@ function parseExtensionRow(row: ExtensionRow): Extension {
     name: row.name,
     description: row.description,
     author: {
-      type: (row.authorType as "organization" | "user") ?? "user",
-      name: row.authorName ?? "",
-      id: (row.authorId as Lowercase<string>) ?? ("" as Lowercase<string>),
+      type: row.authorType as "organization" | "user",
+      name: row.authorName,
+      id: row.developerId as Lowercase<string>,
       URL: row.authorUrl ?? undefined
     } as Author,
     releases: sortReleasesDescending(releases),

src/services/extensions/v2/README.md

@@ -2,7 +2,7 @@
 
 **Base Path:** `/extensions/v2`
 
-Self-service extension submission, developer-profile ownership, moderation, and public catalogue browsing.
+Self-service extension publishing, developer-profile ownership, moderation, and public catalogue browsing.
 
 This service owns the complete Extensions domain and its `DB_EXTENSIONS` schema: users, developers, submissions, claims, transfers, history, and catalogue data. The separate Extensions site keeps OIDC/session state but reaches this domain through the generated HTTPS API client; it must not bind or migrate `DB_EXTENSIONS`.
 
@@ -13,6 +13,57 @@ Endpoints are not listed here. The service publishes its own contract:
 - **OpenAPI document:** `GET /extensions/v2/openapi.json`
 - **Reference UI:** `GET /extensions/v2/docs`
 
+## The Extension Lifecycle
+
+An extension is a resource from the moment a developer creates it, not from the
+moment a moderator approves it. There is no separate "submission" to reconcile
+against it.
+
+- `POST /extensions` creates the record. It holds its id immediately and stays
+  out of both catalogues until its first revision is approved.
+- `PUT /extensions/{id}` proposes an edit. The published content does not
+  change until a moderator approves the revision, and only one revision per
+  extension may be unreviewed at a time.
+- `DELETE /extensions/{id}` withdraws an extension that has never been
+  published, releasing its id. A published extension cannot be withdrawn by its
+  owner — consumers pin the id.
+- `POST /extensions/{id}/revisions/{revisionId}/approve` publishes the
+  revision's content. `reject` leaves the published content untouched and the
+  extension available to edit and resubmit.
+
+The id and the developer are properties of the extension, not of a revision: an
+edit cannot rename an extension or move it to another developer, and approving
+one no longer rewrites the developer profile as a side effect. A user owns at
+most one developer profile, so no request body names one.
+
+### Reading Owner State
+
+`GET /extensions/mine` and `GET /extensions/mine/{id}` return three independent
+fields rather than a single derived status, because together they are the
+state and a derived enum could only disagree with them:
+
+| `published` | `pending_revision` | `last_review` | Meaning                                 |
+| ----------- | ------------------ | ------------- | --------------------------------------- |
+| `null`      | set                | `null`        | Awaiting first review                   |
+| `null`      | set                | rejected      | Rejected, and already resubmitted       |
+| `null`      | `null`             | rejected      | Rejected; edit and resubmit             |
+| set         | `null`             | approved      | Live, no unreviewed edit                |
+| set         | `null`             | `null`        | Live, adopted from the pre-v2 catalogue |
+| set         | set                | either        | Live, with an edit awaiting review      |
+
+The adopted row is the one worth reading twice: migration 0021 published every
+extension that already existed, and those have no revisions at all, so a live
+extension with no review history is normal rather than a gap. `published`
+being set is the only thing that means "in the catalogue".
+
+These are separate routes from the public `GET /extensions` and
+`GET /extensions/{id}`, which only ever return published content. A single path
+whose 200 changes shape with the caller would force every generated client to
+narrow a union at each call site, and the public read is the hotter path.
+
+`GET /extensions/{id}/revisions` lists the full history for the extension's
+owner or any moderator.
+
 ## Authentication
 
 Requests carry a short-lived bearer assertion minted by the Extensions site and verified here with a shared HMAC secret (`ASSERTION_SIGNING_SECRET`; see the root README for where to configure it).
@@ -38,18 +89,49 @@ For organization developer IDs, GitHub membership is used for automatic verifica
 
 ## List Pagination
 
-`GET /extensions/v2/extensions` returns bounded pages of lightweight catalogue items. List items intentionally omit `readme` and `releases`; retrieve the full object from `GET /extensions/v2/extensions/{id}` for detail views. Follow `pagination.next_cursor` by passing it unchanged as `cursor`, and treat cursors as opaque. The default page size is 50 and `limit` may be set from 1 through 100.
+`GET /extensions/v2/extensions` returns bounded pages of lightweight catalogue items, filtered to published extensions. List items intentionally omit `readme` and `releases`; retrieve the full object from `GET /extensions/v2/extensions/{id}` for detail views. Follow `pagination.next_cursor` by passing it unchanged as `cursor`, and treat cursors as opaque. The default page size is 50 and `limit` may be set from 1 through 100.
 
 Cursors carry a version field and are validated on decode, so a cursor from an older format is rejected with `INVALID_CURSOR` (HTTP 422) rather than being misread. Clients should treat that as "restart pagination from the first page", not as an error to surface.
 
 ## Database
 
 Uses the D1 binding `DB_EXTENSIONS`, shared with v1 (read-only there). This service owns the schema and the migrations.
 
+`extensions` holds the record; its content columns are the _published_
+projection and are NULL until a first approval, with `published_at` as the
+marker both catalogues filter on and `extensions_published_content_check`
+guaranteeing a published row is never half-written. `extension_revisions`
+(renamed from `extension_submissions` in migration 0021) holds proposed
+content, always attached to a real extension row and cascading with it.
+
+Migration 0021 rebuilds `extensions`, `extension_revisions` and `developers`
+in one step, because SQLite cannot relax `NOT NULL`, add a `CHECK`, or add a
+foreign key in place. It also renames `extensions.author_id` to `developer_id`
+(nothing public depended on the old name — v1's response field is `author`
+either way) and replaces `developers.created_at`/`updated_at`'s placeholder
+1970 default with `CURRENT_TIMESTAMP`, which is what every writer already
+uses. Existing 1970 values are left alone: they are the only record those rows
+have, and a timestamp invented at migration time would look real without being
+so.
+
 Apply migrations **only from this repository**, from `db/migrations`, with `npm run db:migrate:extensions-v2:local` / `:remote`. The Extensions site has no D1 migration source.
 
 Migration `0020` is a check, not a schema change: it fails if an adopted row holds an id that a static route shadows (`extensions.id = 'mine'`, or `developers.id` of `me`/`claims`/`unapproved`), which would make that row's detail page unreachable. If it fails, rename the row deliberately — the id is public and consumers pin it.
 
+Migration `0021` refuses to run against data it cannot migrate, rather than aborting halfway through the rebuild. Each check selects the offending rows into a scratch table whose named `CHECK` can never hold, so the constraint name is the error message — SQLite has no `RAISE()` outside a trigger:
+
+| Failure                                      | Meaning                                                                 |
+| -------------------------------------------- | ----------------------------------------------------------------------- |
+| `extension_ids_must_not_differ_only_by_case` | Two catalogue ids collide under `idx_extensions_id_nocase`              |
+| `submission_target_ids_must_not_be_reserved` | A submission targets an id a static route shadows                       |
+| `extension_references_must_resolve`          | The `foreign_keys=OFF` rebuild would carry a dangling reference through |
+
+None of these are repaired automatically: each is a decision about published data that belongs to a human. Reconcile and re-run — the migration has touched nothing at that point.
+
+It does resolve one case itself: pending submissions whose ownership state can never satisfy approval are rejected, since one pending revision per extension would otherwise block the owner's next edit forever.
+
+Migration `0021` also drops any submission filed under a developer that no longer exists: there is no `developer_id` such a row could carry that satisfies the new foreign key, and the profile it was filed under is already gone.
+
 ## Code Layout
 
 See `AGENTS.md` for what belongs in `routes/`, `db/`, `schemas/`, `github/`, and `middleware.ts`. This service is the reference layout for larger services.

src/services/extensions/v2/db/developer-profiles.ts

@@ -7,7 +7,6 @@ import {
   developerHistory,
   developerTransfers,
   extensions,
-  extensionSubmissions,
   users
 } from "./schema";
 import { databaseError } from "./errors";
@@ -68,6 +67,33 @@ function parseDeveloperRowWithOwner(row: {
 
 export class DeveloperProfilesDatabase {
   constructor(private db: ExtensionsDb) {}
+
+  // The minimum an extension write needs about the caller's profile: which
+  // developer to publish under, and the ownership epoch to pin the write to.
+  // getOwn() is the full profile projection and deliberately does not expose
+  // ownership_epoch, which is an internal concurrency token rather than part
+  // of the public contract.
+  async getOwnRef(
+    userId: string
+  ): Promise<DatabaseResult<{ id: string; ownershipEpoch: number } | null>> {
+    try {
+      const [row] = await this.db
+        .select({
+          id: developers.id,
+          ownershipEpoch: developers.ownershipEpoch
+        })
+        .from(developers)
+        .where(eq(developers.ownerUserId, userId));
+      return {
+        data: row
+          ? { id: row.id, ownershipEpoch: Number(row.ownershipEpoch ?? 1) }
+          : null,
+        error: null
+      };
+    } catch (error) {
+      return databaseError("getOwnRef", error);
+    }
+  }
   async getOwn(
     userId: string
   ): Promise<
@@ -450,36 +476,19 @@ export class DeveloperProfilesDatabase {
     const [extensionCount] = await this.db
       .select({ count: sql<number>`COUNT(*)` })
       .from(extensions)
-      .where(eq(extensions.authorId, developerId));
+      .where(eq(extensions.developerId, developerId));
     const extensionsCount = extensionCount?.count ?? 0;
     if (extensionsCount > 0) {
       return {
         code: "CONFLICT",
-        message: `You have ${extensionsCount} published extension(s) under this profile. Transfer ownership or remove them before deleting it.`
-      };
-    }
-
-    const [pendingCount] = await this.db
-      .select({ count: sql<number>`COUNT(*)` })
-      .from(extensionSubmissions)
-      .where(
-        and(
-          eq(extensionSubmissions.developerId, developerId),
-          eq(extensionSubmissions.status, "pending")
-        )
-      );
-    if ((pendingCount?.count ?? 0) > 0) {
-      return {
-        code: "CONFLICT",
-        message:
-          "You have a pending submission under review. Wait for it to be resolved before deleting your profile."
+        message: `You have ${extensionsCount} extension(s) under this profile. Transfer ownership, or withdraw the unpublished ones, before deleting it.`
       };
     }
 
     // The guard failed but a fresh look finds nothing wrong — whatever
-    // blocked it (someone else's transfer/claim landing, a submission
-    // that has since been resolved) has already cleared. Ask the caller
-    // to retry rather than guessing at a reason that's no longer true.
+    // blocked it (someone else's transfer/claim landing, an extension that
+    // has since been withdrawn) has already cleared. Ask the caller to retry
+    // rather than guessing at a reason that's no longer true.
     return {
       code: "CONFLICT",
       message:
@@ -490,9 +499,12 @@ export class DeveloperProfilesDatabase {
   // Permanently removes the caller's own developer profile, for a
   // privacy-focused account-deletion flow. Refuses while anything would be
   // left dangling in a way that isn't just historical record-keeping:
-  // published extensions (someone still needs to own them) and pending
-  // submissions (nothing left to approve/reject against once the named
-  // developer is gone). developer_history is deliberately left alone —
+  // any extensions at all (someone still needs to own them, and extensions
+  // .developer_id is a hard FK). Unpublished ones count: withdrawing them is
+  // owner's own one-call operation. Pending revisions need no separate check
+  // since migration 0021 - every revision belongs to an extension and
+  // cascades with it, so "no extensions" already implies "nothing left to
+  // review". developer_history is deliberately left alone —
   // it's an append-only audit log, moderator-only, never rendered publicly,
   // and 0009_drop_developer_history_fk.sql dropped its FK to developers(id)
   // specifically so a deleted developer's history rows can outlive it.
@@ -513,10 +525,9 @@ export class DeveloperProfilesDatabase {
       }
 
       // Every statement re-checks eligibility (still owned by this caller,
-      // no published extensions, no pending submission) at the moment it
-      // runs, rather than trusting the SELECT above: ownership can move
-      // (an accepted transfer/claim) and a new extension or pending
-      // submission can appear between that check and this write, and this
+      // no extensions attached) at the moment it runs, rather than trusting
+      // the SELECT above: ownership can move (an accepted transfer/claim) and
+      // a new extension can appear between that check and this write, and this
       // delete is the caller's only authorization check. The same guard is
       // repeated on all three statements — not just the last — so they're
       // all-or-nothing: if it fails, nothing here is touched, instead of
@@ -533,12 +544,7 @@ export class DeveloperProfilesDatabase {
       // the inner table shadow the outer and pass whenever *any* profile were
       // deletable. Parameters, in order: owner user id, then active user id.
       const deletable = (dev: string) => `${dev}.owner_user_id = ?
-                    AND NOT EXISTS (SELECT 1 FROM extensions WHERE extensions.author_id = ${dev}.id)
-                    AND NOT EXISTS (
-                      SELECT 1 FROM extension_submissions
-                      WHERE extension_submissions.developer_id = ${dev}.id
-                        AND extension_submissions.status = 'pending'
-                    )
+                    AND NOT EXISTS (SELECT 1 FROM extensions WHERE extensions.developer_id = ${dev}.id)
                     AND EXISTS (
                       SELECT 1 FROM users active_user
                       WHERE active_user.id = ? AND active_user.deleted_at IS NULL

src/services/extensions/v2/db/developer-transfers.ts

@@ -286,7 +286,7 @@ export class DeveloperTransfersDatabase {
       // pending work attached to a profile whose owner just changed would put
       // it in front of the wrong moderator.
       const rejectPendingIn = (
-        table: "extension_submissions" | "developer_claims"
+        table: "extension_revisions" | "developer_claims"
       ) =>
         toD1Statement(this.db.$client, {
           sql: `UPDATE ${table}
@@ -297,9 +297,7 @@ export class DeveloperTransfersDatabase {
                 AND status = 'pending'`,
           params: [tokenHash, userId]
         });
-      const rejectPendingSubmissionsStmt = rejectPendingIn(
-        "extension_submissions"
-      );
+      const rejectPendingRevisionsStmt = rejectPendingIn("extension_revisions");
       const rejectPendingClaimsStmt = rejectPendingIn("developer_claims");
 
       let results;
@@ -308,7 +306,7 @@ export class DeveloperTransfersDatabase {
           claimStmt,
           updateDeveloperStmt,
           assertTransferStmt,
-          rejectPendingSubmissionsStmt,
+          rejectPendingRevisionsStmt,
           rejectPendingClaimsStmt
         ]);
       } catch (error) {