Make the extension the resource and review a state of it by admdly · Pull Request #190 · FOSSBilling/api

admdly · 2026-08-08T07:40:51Z


`POST /extensions`	Creates the record. Holds its id immediately, out of both catalogues until first approval.
`PUT /extensions/{id}`	Proposes an edit. Published content is unchanged until approved.
`DELETE /extensions/{id}`	Withdraws an unpublished extension, releasing its id.
`GET /extensions/mine`, `/extensions/mine/{id}`	Owner view: `published` + `pending_revision` + `last_review`.
`GET /extensions/{id}/revisions`	History, for the owner or a moderator.
`GET /moderation/extensions`	Review queue.
`POST /extensions/{id}/revisions/{revisionId}/approve\|reject`	Review.

The owner view returns three independent fields, not a derived status enum. published, pending_revision and last_review together are the state — a live extension with an unreviewed edit has all three. A derived enum on top could only disagree with them. The README has the mapping table.

Owner reads are separate routes from the public ones rather than one path whose 200 changes shape with the caller. A polymorphic response would force the generated client into a union it narrows at every call site, and the public catalogue read is the hotter path.

Submissions were modelled as a resource parallel to extensions: an extension only existed once a moderator approved one, so in-flight work lived in extension_submissions under a target id reachable only through the payload JSON. Rendering "my extensions" meant reading two separately-paginated collections and reconciling them client-side, with rules that were nowhere in the contract. The same service already solved this problem the other way for developer profiles, which are edited in place with review state on the row. Extensions now work that way too. POST /extensions creates the record, which holds its id immediately and stays out of both catalogues until its first revision is approved. PUT /extensions/{id} proposes an edit. extension_submissions becomes extension_revisions: one proposed content version, always attached to a real extension row. Consequences worth calling out: - A revision carries extension content only. Approving one no longer rewrites the developer profile and resets its approval as a side effect. - No request body names a developer; a user owns at most one profile, so the server derives it. - An extension cannot be renamed or moved to another developer by an edit. - The approval-time reserved-id re-checks are gone. Ids are validated once at creation, against a row that exists from then on. - target_key and its partial index are gone. UNIQUE(lower(id)) on extensions plus UNIQUE(extension_id) WHERE status='pending' do the same work structurally. - v1 and the v2 catalogue filter on published_at, which is what keeps their output identical now that an unpublished row can exist. Migration 0021 rebuilds extensions, extension_revisions and developers, since SQLite cannot relax NOT NULL, add a CHECK, or add a foreign key in place. Two renames ride along on a rebuild already paid for: extensions.author_id becomes developer_id, and developers' created_at and updated_at lose the placeholder 1970 default that no writer ever produced. It ends by failing the deploy if the rebuild would carry a dangling reference through its foreign_keys=OFF window, which is what lets the catalogue reads inner-join developers instead of defending against a row that cannot exist.

cloudflare-workers-and-pages · 2026-08-08T07:40:56Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	api	`64adc51`	Commit Preview URL Branch Preview URL	Aug 08 2026, 11:12 AM

cubic-dev-ai

All reported issues were addressed across 33 files

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

Six findings from review, each now covered by a test that fails without its fix. Owner list paginated without an index. The catalogue-order indexes were made partial on published_at, but GET /extensions/mine cannot filter on it, so the query fell back to a plain developer_id index and sorted into a temp B-tree past the first page. idx_extensions_developer_order is unfiltered and covers both that query and the public developer_id filter, which replaces the partial one. Owner type filter hid drafts. extensions.type is NULL until first approval, so ?type=mod dropped every unpublished extension from its own owner's list. It now falls back to the pending revision's content, then the last reviewed one, which covers both unpublished states. Id case handling was inconsistent. Reads resolved ids case-insensitively for legacy mixed-case rows, but propose/withdraw/approve/reject and the revision list did not, so such an extension was readable but not editable, and its revision list came back empty. Writes now resolve the same way, and the revision list is given the stored spelling rather than the requested one. Withdrawal had no active-account guard. Every other write re-checks users.deleted_at inside the statement; this one trusted requireActiveAuth(), which can only reject before the write. Gated, and mapped to ACCOUNT_INACTIVE/403. Migration 0021 carried two hazards forward. A legacy submission targeting a reserved id would have materialised an extension that GET /extensions/{id} can never serve, now that approval no longer re-checks the id; the migration fails the deploy instead, matching 0020. And a pending submission whose developer or ownership state can never satisfy approve()'s predicate would have sat pending forever while holding the one-pending-per-extension slot, blocking the owner's next edit; those are rejected during the migration with the same note the transfer and account-deletion paths already use. The case-collision test also seeded the id it claimed to differ from, so it only re-tested the plain duplicate path.

cubic-dev-ai

Review completed against the latest diff

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

cubic-dev-ai

0 issues found across 8 files (changes from recent commits).

_{Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.

Re-trigger cubic}

Three of these four findings were one bug wearing four hats. Every guarded write repeats an active-account check inside its own statement, because requireActiveAuth() can only reject before the write. But when such a statement affected no rows, only withdraw() asked whether the account had gone; create, propose, approve and reject each let that case fall through to whatever branch their diagnosis ended on. A caller deactivated mid-request was told their id was taken, that they had hit the pending-revision limit, or that the revision was no longer pending - the last being flatly untrue, since the revision was still sitting there pending. inactiveActorError() is now the first question each blocked-write diagnosis asks, and statusFromWriteErrorCode maps the result to the 403 those routes already document. It takes includeNotFound the way statusFromErrorCode takes includeConflict, so POST /extensions, which creates rather than addresses a row and declares no 404, cannot emit one. withdraw() asks the same question rather than concluding inactivity by elimination. Separately, migration 0021 created idx_extensions_id_nocase over a catalogue that may already hold ids differing only in case. That aborted the rebuild halfway with a bare "UNIQUE constraint failed: index 'idx_extensions_id_nocase'" naming no rows. It is now checked up front, alongside the reserved-target check, so the migration fails before rewriting anything and says what to reconcile. Which of the two ids survives is not a decision a migration can make: both are public and consumers pin them. Each fix has a test that fails without it, driven through the db-interceptor hook for the mid-request deactivations.

The three pre-flight checks all aborted with an anonymous "CHECK constraint failed", which tells an operator that something is wrong with their data but not what, or which rows. SQLite has no RAISE() outside a trigger, so the constraint name is the only place a diagnosis can go. Each check now selects the offending rows into a scratch table whose named CHECK can never hold: a clean database inserts nothing and passes, a dirty one fails with extension_ids_must_not_differ_only_by_case, submission_target_ids_must_not_be_reserved, or extension_references_must_resolve. The scratch table's columns name the rows involved, so the same query listed in the migration is what an operator runs to find them. Case-colliding ids are still not reconciled automatically. The pair is already ambiguous to every reader - v1 and v2 both resolve ids with LOWER(), so one of the two rows is unreachable today depending on which the query happens to return first - but choosing which id survives, and whether the other is renamed or deleted, is a decision about published data that a migration should not make silently.

cubic-dev-ai

Review completed against the latest diff

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

Revision history is an audit log that outlives the rules its content was written under. ExtensionRevisionSchema promised every stored revision satisfies today's input validation - at least one release, among other things - which migration 0021 cannot honour, since it carries legacy submissions through verbatim, and which any future tightening would break again for older rows. StoredExtensionContentSchema now describes what a revision may actually hold, and approve() re-validates against the strict schema before publishing. That keeps the public catalogue's contract exactly as strict as it was while letting history be history, and it puts the check at the boundary that matters rather than trusting submission-time validation - the same reasoning that had the pre-0021 code re-check reserved ids at approval. A revision that cannot be published now says so, instead of publishing content the public schema disowns. Also from review: The owner view could show an older decision as last_review. reviewed_at comes from CURRENT_TIMESTAMP and is second-granular, so two reviews can share one, and the tie was broken by a random UUID. Broken by rowid now, which is assigned in insert order - and since only one revision per extension may be pending, insert order is review order. withdraw() classified published-ness and ownership before asking whether the account had been deactivated, so a deactivated owner of a published extension got 409. It asks first now, like the other blocked-write diagnoses already did. The README's owner-state table claimed to be the whole state space while omitting two reachable rows: an extension adopted from the pre-v2 catalogue (live, no review history at all) and a rejected revision the owner has already resubmitted. Both are now documented and both have a test proving they are reachable.

cubic-dev-ai

All reported issues were addressed across 35 files

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

A submission naming a developer that does not exist was filtered out of the materialisation and vanished. Such a row is already unapprovable - the pre-0021 approve() only ever UPDATEs a developer, never inserts one, so approving it marked the submission approved and published nothing - but losing the record without saying so is not a migration's call. It now fails with submissions_must_name_an_existing_developer, and the filter is gone so the check is the only mechanism rather than a second one masking it. The developer is deliberately not backfilled from payload.developer. Creating a profile would mint an ownership grant no moderator approved, which is what the claim and approval flows exist to prevent. StoredExtensionContentSchema only relaxed releases, so migrated content missing any other field still contradicted the advertised contract. Every field is optional now: this schema describes what is *there*, and history written under older rules is exactly the case it exists for. Field types and upper bounds stay, since those remain true. Publication is unaffected - approve() still revalidates against the strict schema. explainNoOpTransition returned getById()'s NOT_FOUND before asking whether the moderator had been deactivated, so a reject racing deactivation on an already-gone revision reported 404 instead of the documented 403. Actor first, matching the other diagnoses. The stored-content test now parses the served response back through ExtensionRevisionSchema. Hono does not validate responses at runtime, so without that assertion nothing catches a response schema disagreeing with the data until a generated client does.

cubic-dev-ai

All reported issues were addressed across 35 files

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

OwnedExtensionSchema.published required at least one release, but v1 constrained extensions.releases to NOT NULL and nothing more, so a row adopted by migration 0021 can be published with none. The owner view is where someone looks at what is actually there, and it was advertising a shape the data cannot always take. published now uses a schema whose releases has no minimum. Only the owner detail view changes: the owner list omits releases entirely, and approve() still requires one, so this can only ever describe a pre-v2 row. The public ExtensionSchema is deliberately left strict. The same legacy rows flow through it, but that contract predates this PR and every catalogue consumer relies on it; relaxing it would push the empty-array case onto all of them to fix a condition v1 has always had. Worth revisiting as its own change rather than smuggling it in here. The test parses the served body back through OwnedExtensionSchema, since Hono does not validate responses and nothing else would notice the contract drifting from the data.

cubic-dev-ai

All reported issues were addressed across 35 files

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

The oversized-content case sent a 100_001-character readme, which the readme field's own .max(100_000) rejects at roughly 100 KB. The 422 was real but came from the wrong constraint, and refineContentSize had no coverage at all: deleting the guard outright left the whole suite green. Reaching it needs content that is valid field by field yet large in aggregate, since the biggest single field is the readme. 100 releases - the maximum - carrying maximum-length URLs comes to roughly 440 KB. The test asserts the size guard's own message rather than just a 422, so it cannot start passing for a different reason again, and covers the edit body as well since ExtensionUpdateSchema carries the same refinement. The readme case stays, now asserting too_big at ["readme"] so it is pinned to the bound it actually tests.

cubic-dev-ai

0 issues found across 1 file (changes from recent commits).

_{Requires human review: Migration 0021 rebuilds the schema and defers data-conflict choices to a human, while the v2 API replaces submission endpoints with new revision/moderation routes. A human must review the data migration and public contract changes.

Re-trigger cubic}

* Migrate to the restructured Extensions v2 API Submissions are no longer a separate resource — an extension exists from creation and review is a state of it, not a parallel record to reconcile. Regenerates the API client against FOSSBilling/api#190's branch preview and updates every caller: - api/client.ts: create/update/withdraw an extension, list/read owner extensions (published + pending_revision + last_review), list revision history, and moderate by extension id + revision id, replacing the submissions endpoints. - extensions-data.ts: new getOwnedExtension adapter splits the owner detail response into published/pendingRevision/lastReview per the API's state table instead of collapsing them into a derived status. - extension-form.ts (renamed from submission-form.ts): payloads no longer carry a developer object; a release is required whenever there's nothing already published to carry through (new extension, or edited after rejection with nothing ever published). - Account pages: /account no longer merges a separate submissions list: one list from /extensions/mine with inline pending/rejected state. The edit page blocks with a banner while a revision is awaiting review, matching the api's one-pending-revision-at-a-time rule. - Moderation: routes move from account/moderate/[id]/ to account/moderate/[id]/[revisionId]/, since revisions are now addressed by extension id + revision id, not a submission id. npm run check, npm run test, and npm run format:check are all clean. npm run api:check currently shows a diff since this was generated from #190's branch preview, not production — re-run it once #190 merges and deploys. * Address PR review findings - extension-form.ts: reject a new release when it duplicates an existing tag (would silently duplicate the entry on approval, since the api replaces the whole releases array verbatim) or when the extension is already at the api's 100-release cap, instead of submitting a payload the api would either mangle or 422 on. - ConfirmDeleteDialog: add an optional formAction so a confirmation dialog can post somewhere other than the current page. - New /account/extensions/[id]/withdraw.ts route + a "Withdraw Extension" danger-zone control on the edit page for extensions that have never been published. Previously withdrawExtension existed on the api client but had no UI control, so a developer whose only extension was pending or rejected-with-nothing-published had no way to clear it and delete their developer profile or account. - account/index.astro: a rejected edit no longer shows a destructive "Rejected" badge when the extension is still live under its previous published content — that only reflects the latest edit attempt, not the extension itself. Live extensions with a rejected edit now show a neutral "Edit rejected" badge instead. npm run check, npm run test (83 passing), and npm run format:check are all clean.

admdly self-assigned this Aug 8, 2026