feat(media): add flat folder API foundation - #2584
Conversation
🦋 Changeset detectedLatest commit: 8858dd3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Scope checkThis PR changes 1,578 lines across 33 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | 8858dd3 | Aug 26 2026, 05:11 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | 8858dd3 | Aug 26 2026, 05:06 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-do | 8858dd3 | Aug 26 2026, 05:09 PM |
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds a coherent flat media-folders foundation: a migration, repository, handlers, Astro routes, client methods, OpenAPI additions, and REST docs. The implementation closely follows existing EmDash patterns—cursor pagination, ApiResult handlers, requirePerm/requireOwnerPerm, FK/cascade behavior, and a D1-compatible migration. Test coverage is broad (migrations, repository integration, handler/unit, route authorization, OpenAPI, and client serialization).
The main process concern is the one the PR already flags: AGENTS.md requires a maintainer-approved Discussion for a feature, and no folder-specific Discussion exists yet. The code should not merge until that approval is in place; as a draft PR the author is aware, so this raises the approach risk without making it a line finding.
Code-level issues are minor and mostly on the public surface: the changeset is too terse for a minor feature, the typed client introduces a duplicate local-media shape next to an already-inaccurate MediaItem, and the update-body schema accepts the query-only unfiled sentinel. None of these are blockers, but they should be cleaned up before the feature ships.
github-actions
Bot
added
review/awaiting-author
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
cad314c to
822dfb5
Compare
822dfb5 to
151d46b
Compare
151d46b to
edfd4e3
Compare
khoinguyenpham04
added
the
bot:review
There was a problem hiding this comment.
This remains a coherent, scope-disciplined PR1 for flat media folders. The backend architecture follows existing EmDash conventions: cursor pagination for folder lists, ApiResult handlers, requirePerm/requireOwnerPerm authorization, FK onDelete("set null") for safe folder deletion, a D1-compatible migration (072_media_folders) with rollback coverage, and broad tests across migrations, repositories, handlers, routes, OpenAPI, and the typed client. The folder routes are wired correctly before /media/[id] so static /media/folders paths take precedence.
Re-review status
- The expanded changeset now meets the .changeset/README.md standard: it leads with the released behavior, names the affected surface (REST and typed client), explains usage, permissions, defaults, and safe-deletion semantics.
- The prior note about accepting the query-only
unfiledsentinel inPUT /media/:idupdate bodies is now documented as explicit behavior in the REST reference, so I’m not treating it as an unaddressed bug. - The prior typed-client concern is still open: the client-side
MediaIteminterface is structurally wrong for the API responses returned bymediaList,mediaGet, andmediaUpload. This is the only remaining code-level issue.
What I checked
- Diff files, full source of changed handlers/routes/repositories/schemas/client.
rbac.tspermission mapping (media:read,media:edit_own,media:edit_any,media:upload, token scopes).- Migration registration in
runner.tsand the migration’s up/down/Idempotency handling. database/types.tsforMediaFolderTableandfolder_idonMediaTable.- Route injection order in
astro/integration/routes.ts. openapi/document.tsfor folder paths and response schemas.- Updated REST docs (
rest-api.mdx) and media-library guide. - Unit and integration tests for folders, pagination, routes, and client serialization.
Headline conclusion
No security, data-loss, or authorization blockers. The foundation is clean and well-tested. Before the feature ships, the typed-client MediaItem shape needs to be reconciled with the actual REST MediaItem schemas.
One process caveat from the prior review carries into this round: AGENTS.md requires a maintainer-approved Discussion for a feature. The PR description correctly notes that folder-specific approval is still needed and that this must not merge until it exists. I have not made that a line finding because the author already flags it.
Findings
-
[needs fixing]
packages/core/src/client/index.ts:187-191The client-side
MediaIteminterface does not match the REST API’smediaItemSchemaandmediaListReadItemSchema, which this PR’s new folder-aware methods (mediaList,mediaGet,mediaSetFolder,mediaUpload) all return or re-use. Specifically:- The API returns
storageKey, but this type useskey. - The API returns
sizeas nullable, but this type types it asnumber. - The API always returns
status,authorId,contentHash,blurhash, anddominantColor, which are absent here. folderIdis optional (?) even though every media response now includes it asstring | null.updatedAtis present here but is not part of the API media schema.
This makes the typed client misleading for consumers trying to use the new folder fields or any other media metadata. The type should be aligned with the canonical API shape (with
urlandusagekept optional because they’re added by list/get envelopes)./** Media item */ export interface MediaItem { id: string; filename: string; mimeType: string; size: number | null; width: number | null; height: number | null; alt: string | null; caption: string | null; storageKey: string; status: "pending" | "ready" | "failed"; contentHash: string | null; blurhash: string | null; dominantColor: string | null; createdAt: string; authorId: string | null; folderId: string | null; url?: string; usage?: MediaUsageSummary; } - The API returns
github-actions
Bot
added
review/awaiting-author
b913497 to
8858dd3
Compare
github-actions
Bot
added
review/needs-rereview
2 participants
What does this PR do?
Adds the backend and typed-client foundation for flat, shared Media Library folders. Local media can belong to one folder or the Main library, list requests can select All media, Main library media, or one folder, and deleting a folder returns its media to the Main library without changing media IDs, storage keys, URLs, or usage records.
This is PR1 of the folders sequence and intentionally has no admin UI, upload-to-folder behavior, nesting, bulk moves, provider changes, CLI commands, or MCP commands. PR2 will add the focused Media Library UI on top of this branch.
This PR is stacked on #2582 so reviewers see only the folder foundation. It will ultimately merge into
mainafter the pagination dependency lands.Related media roadmap Discussions:
A folder-specific maintainer-approved Discussion was not found. Code review can proceed, but this feature must not merge until the folder scope is approved.
Type of change
Checklist
emdashpackagemessages.pochangesemdashminor changesetAI-generated code disclosure
Screenshots / test output
PR1 has no visual changes.