fix: generate inline taxonomy slugs server-side - #2506
Conversation
🦋 Changeset detectedLatest commit: 0de54d1 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 |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| 🔵 In progress View logs |
emdash-playground | 31eb04c | Aug 16 2026, 10:23 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| 🔵 In progress View logs |
emdash-demo-do | 31eb04c | Aug 16 2026, 10:23 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| 🔵 In progress View logs |
emdash-demo-cache | 31eb04c | Aug 16 2026, 10:23 PM |
There was a problem hiding this comment.
This PR correctly moves taxonomy-term slug generation to the server so Unicode labels and inline creation work with the shared Unicode slug utility. The approach matches the architecture: the schema makes slug optional, the handler derives it, and the admin components omit the slug only when it is auto-generated. I checked the changed handler, repository, schemas, MCP tool, admin components, tests, and changeset.
Main concern: handleTermCreate is not atomic between choosing a generated slug and inserting it. Two concurrent requests can pick the same slug; the top-level catch then returns a generic TERM_CREATE_ERROR instead of a structured conflict or a recovered suffix. That's a real robustness gap in the new default path.
Secondary: the new schema-only unit test is tautological; the dialect-aware handler tests already cover the real behavior.
Everything else looks good: Kysely is used safely, locale filtering is present, the changeset follows conventions, there are no logged-out hot-path changes, and no new user-facing untranslated strings.
Findings
-
[needs fixing]
packages/core/src/api/handlers/taxonomies.ts:910-914generateUniqueSlugprobes the database and then the handler inserts, so two concurrent requests can both choose the same generated slug. This catch block then turns the resulting unique-constraint failure into an opaqueTERM_CREATE_ERROR, meaning the new default create path can fail with a generic message instead of a structuredCONFLICTor a recovered suffix.At minimum, detect the unique-constraint violation and return a structured conflict; better, retry generation a bounded number of times so the auto-generator can recover from its own race.
} catch (error) { if (error instanceof Error && error.message.includes("UNIQUE constraint failed")) { return { success: false, error: { code: "CONFLICT", message: `Term with slug '${slug}' already exists in taxonomy '${taxonomyName}'`, }, }; } return { success: false, error: { code: "TERM_CREATE_ERROR", message: "Failed to create term" }, }; } -
[suggestion]
packages/core/tests/unit/taxonomies/term-slug-generation.test.ts:13-15This test asserts that the Zod schema accepts a label-only object, which mostly restates the schema definition rather than validating behavior. The dialect-aware handler tests below already cover the real behavior (server derives slug from label). Drop this standalone schema-only test or fold it into a handler-level assertion.
|
Addressed both automated-review findings in 4c5dd49.
Verified with the 122-test taxonomy/MCP suite, |
There was a problem hiding this comment.
This re-review looks good. The server-side slug generation for inline taxonomy terms correctly moves Unicode slug derivation into handleTermCreate while preserving explicit-slug conflict behavior.
I re-checked the handler, repository, schemas, MCP tool, admin components, and tests. The previous race condition is now handled by a bounded retry loop around generateUniqueSlug, plus structured CONFLICT responses for unique-constraint failures. The tautological schema-only unit test was replaced with dialect-aware handler tests covering Unicode slugs, numeric suffix collisions, concurrent recovery, locale scoping, and explicit-slug conflicts.
Kysely usage remains safe, locale filtering is present where the term will be inserted, the changeset follows conventions, no logged-out hot paths are affected, and no untranslated admin UI strings were added. No remaining blockers.
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Moves inline taxonomy term slug generation from the admin client to the core server/API (and MCP tool), deriving Unicode slugs when omitted and resolving collisions with locale-scoped numeric suffixes.
Changes:
- Make term
slugoptional across API/MCP schemas and admin client types; server derives slug when omitted. - Add server-side unique-slug generation + retry-on-collision logic for concurrent creates.
- Add unit/integration/admin tests covering Unicode slugs, collisions, and “omit slug” client behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/tests/unit/taxonomies/term-slug-generation.test.ts | Adds unit coverage for Unicode slug derivation, collisions, locale scoping, and concurrency. |
| packages/core/tests/integration/mcp/taxonomy.test.ts | Verifies the MCP tool derives a Unicode slug when slug is omitted. |
| packages/core/src/mcp/server.ts | Makes MCP taxonomy_create_term slug optional and documents omission behavior. |
| packages/core/src/database/repositories/taxonomy.ts | Introduces generateUniqueSlug backed by DB collision checks and numeric suffixing. |
| packages/core/src/api/schemas/taxonomies.ts | Makes REST createTerm slug optional with description metadata. |
| packages/core/src/api/handlers/taxonomies.ts | Implements server-side slug derivation, retry loop, and unique-violation handling. |
| packages/admin/tests/components/TaxonomySidebar.test.tsx | Ensures sidebar inline create omits slug and lets server derive it. |
| packages/admin/tests/components/TaxonomyManager.test.tsx | Ensures manager omits slug when auto-generated, and sends slug when manually edited. |
| packages/admin/src/lib/api/taxonomies.ts | Updates CreateTermInput to allow optional slug. |
| packages/admin/src/components/TaxonomySidebar.tsx | Stops client-side slugification for inline creation. |
| packages/admin/src/components/TaxonomyManager.tsx | Omits slug on create when still auto-generated. |
| .changeset/fix-inline-unicode-taxonomies.md | Adds a changeset describing the fix and collision behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
@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-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: |
4c5dd49 to
262e7ca
Compare
262e7ca to
6f73c3c
Compare
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. |
77cda11 to
83b2b12
Compare
79701f3 to
49dd293
Compare
49dd293 to
9102dbc
Compare
There was a problem hiding this comment.
This PR makes the right change: automatic slug generation for taxonomy terms now happens server-side in handleTermCreate, using the shared Unicode slugify utility, locale-scoped collision detection, and a bounded retry loop for concurrent races. Explicit slugs still get the existing per-locale conflict behavior. This is the minimal, architecturally correct fix for #2355.
I re-reviewed the changed files and the surrounding code paths:
- API contract:
createTermBodynow makesslugoptional, matching the handler change; the REST term-create route and the MCPtaxonomy_create_termtool both pass the new shape through unchanged. - Slug generation:
TaxonomyRepository.generateUniqueSluguses a parameterized Kysely query scoped to(name, locale), with a safeLIKEprefix and numeric suffix logic. No SQL interpolation issues. - Race handling:
handleTermCreateretries generated slug inserts on unique-constraint violations and surfaces any final conflict as a structuredCONFLICTresponse. The error-message heuristic checks for both SQLite and Postgres unique-violation phrases and requires the column nameslugto be present. - Admin callers:
TaxonomySidebarno longer sends a slug;TaxonomyManageronly sends a slug whenautoSlugis false. Both call sites still pass the entry/admin locale so the server scopes the term correctly. - Tests: The new dialect-aware handler tests cover Unicode scripts, emoji fallback, numeric suffix collisions, concurrent recovery, locale scoping, and explicit-slug conflicts. Admin component tests verify the request body shape. No tautological/assert-the-implementation tests were added.
- AGENTS.md conventions: All admin strings remain Lingui-wrapped (no new user-visible strings). No changes to logged-out routes. A correct changeset is present. Comments are minimal and describe invariants, not reviewers.
No remaining blockers. This is a clean, focused bug fix that does what it says.
9102dbc to
57ba47b
Compare
f4191c9 to
a1b55e0
Compare
a1b55e0 to
31eb04c
Compare
31eb04c to
0de54d1
Compare
What does this PR do?
Moves automatic taxonomy-term slug generation to the server for inline creation. When callers omit a slug, the API uses the shared Unicode slug utility from #2505 and resolves collisions against the database with locale-scoped numeric suffixes. Manually supplied slugs remain unchanged and still return a conflict when already taken.
The entry-editor taxonomy sidebar now sends only the label (plus locale), while the taxonomy manager omits the slug only while its slug field remains auto-generated.
Depends on #2505.
Closes #2355
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain. N/A: no new user-visible admin strings.AI-generated code disclosure
Screenshots / test output
pnpm --filter @emdash-cms/admin --filter emdash buildpnpm typecheckpnpm lintpnpm lint:json | jq '.diagnostics | length'→0pnpm --filter emdash exec vitest run tests/unit/taxonomies/term-slug-generation.test.ts tests/unit/taxonomies/taxonomy-crud.test.ts tests/unit/taxonomies/term-reorder.test.ts tests/integration/taxonomies/taxonomy-locale-terms.test.ts tests/integration/mcp/taxonomy.test.ts→ 122 passed.pnpm --filter @emdash-cms/admin exec vitest run tests/components/TaxonomySidebar.test.tsx tests/components/TaxonomyManager.test.tsx→ 31 passed.Try this PR
Open a fresh playground →
A full working EmDash site, deployed from this branch. Each visit gets its own session-scoped sandbox: no login needed and no shared state. Try the admin, edit content, hit the public site.
Tracks
codex/taxonomy-unicode-slugs-2355. Updated automatically when the playground redeploys.