Skip to content

fix: don't auto-redirect a slug another entry still holds - #2315

Open
edrpls wants to merge 1 commit into
emdash-cms:mainfrom
edrpls:fix/slug-redirect-locale-aware
Open

fix: don't auto-redirect a slug another entry still holds#2315
edrpls wants to merge 1 commit into
emdash-cms:mainfrom
edrpls:fix/slug-redirect-locale-aware

Conversation

@edrpls

@edrpls edrpls commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a slug rename taking a different entry's live page offline.

createSlugChangeRedirect builds the auto-redirect purely from the collection's url_pattern and the old/new slug — RedirectRepository.createAutoRedirect does a literal urlPattern.replace("{slug}", oldSlug). A URL pattern has no locale token, so every locale variant of an entry generates the same URL, and slugs are unique per (slug, locale) — so a translation legitimately shares its canonical's slug (the standard emdash i18n shape).

Renaming either row therefore emits a 301 whose source is the other row's live URL:

  • Canonical es venue at cineteca-nacional, en twin at the same slug.
  • Rename the twin to cinematheque → writes /sede/cineteca-nacional → /sede/cinematheque.
  • The Spanish page is still served at /sede/cineteca-nacional, but the redirect middleware is registered order: "pre", so it intercepts before routing and the page can never recover.

The mirror case is just as bad: renaming the canonical while the translation still holds the old slug redirects away from a URL the translation answers on.

The fix skips the auto-redirect when another non-deleted entry in the collection still holds the old slug. This is the same invariant createAutoRedirect already enforces in the other direction (it deletes redirects from the new URL because "the new URL serves live content again — any redirect from it would shadow the page"). Any surviving row counts, published or not: a draft that publishes later would otherwise be shadowed by a redirect created while it was invisible. Renames that genuinely free the URL still redirect exactly as before.

I chose "is the old slug still taken?" over the narrower "skip when locale !== defaultLocale" because the latter fixes only one direction (it leaves the canonical-rename case broken) and would also suppress legitimate redirects for sites that serve translations through Astro's locale routing while keeping one unprefixed url_pattern. The occupancy check is correct under either routing model.

Impact worth noting: this is the reason the reporting deployment keeps url_pattern NULL on every collection — accepting junk /<collection>/<slug> redirects that match no route, rather than enable a feature that can take a live page down.

Found during a measured database audit of a production deployment (emdash 0.31.1).

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (or targeted tests for my change)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (if applicable). Do not include messages.po changes except in translation PRs — a workflow extracts catalogs on merge to main. — n/a: no admin UI strings changed
  • I have added a changeset (if this PR changes a published package)
  • New features link to an approved Discussion — n/a: bug fix

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: Claude Fable 5 (Claude Code)

Screenshots / test output

Both destructive directions fail on main (the redirect source is the live canonical URL):

× does not redirect the canonical URL when a translation sharing its slug is renamed
  AssertionError: expected [ '/sede/cineteca-nacional' ] to not include '/sede/cineteca-nacional'
× does not redirect a URL still held by a translation when the canonical is renamed

A third test locks the positive path — a rename that genuinely frees the URL still produces the redirect — so the fix can't regress into "never redirect".

After the fix:

Tests  65 passed   (tests/integration/redirects/)
Tests  193 passed  (redirects + content handlers + i18n + content suites)
Full packages/core suite: 5075 passed — the single virtual-modules.test.ts failure is
pre-existing on a clean main checkout in this environment (macOS temp-dir realpath).

🤖 Generated with Claude Code

@changeset-bot

changeset-bot Bot commented Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ca7fecc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages
Name Type
emdash Patch
@emdash-cms/cloudflare Patch
@emdash-cms/sandbox-workerd Patch
@emdash-cms/plugin-mcp-smoke Patch
@emdash-cms/fixture-perf-site Patch
@emdash-cms/perf-demo-site Patch
@emdash-cms/cache-demo-site Patch
@emdash-cms/do-demo-site Patch
@emdash-cms/do-solo-demo-site Patch
@emdash-cms/admin Patch
@emdash-cms/auth Patch
@emdash-cms/blocks Patch
@emdash-cms/gutenberg-to-portable-text Patch
@emdash-cms/x402 Patch
create-emdash Patch
@emdash-cms/auth-atproto Patch
@emdash-cms/plugin-embeds Patch

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

@pkg-pr-new

pkg-pr-new Bot commented Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@2315

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@2315

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/@emdash-cms/auth-atproto@2315

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@2315

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@2315

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/contentful-to-portable-text@2315

emdash

npm i https://pkg.pr.new/emdash@2315

create-emdash

npm i https://pkg.pr.new/create-emdash@2315

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@2315

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/@emdash-cms/plugin-cli@2315

@emdash-cms/plugin-types

npm i https://pkg.pr.new/@emdash-cms/plugin-types@2315

@emdash-cms/registry-client

npm i https://pkg.pr.new/@emdash-cms/registry-client@2315

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/@emdash-cms/registry-lexicons@2315

@emdash-cms/registry-verification

npm i https://pkg.pr.new/@emdash-cms/registry-verification@2315

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/@emdash-cms/sandbox-workerd@2315

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@2315

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@2315

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@2315

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@2315

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@2315

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@2315

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/@emdash-cms/plugin-field-kit@2315

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@2315

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@2315

commit: ca7fecc

@edrpls @claude
fix: don't auto-redirect a slug another entry still holds
ca7fecc 
A collection's url_pattern has no locale token, so every locale variant
of an entry generates the same URL — and slugs are unique per
(slug, locale), so a translation may legitimately share its canonical's
slug. Renaming either one emitted a 301 whose source was the other's
live URL. The redirect middleware runs order: "pre", so the surviving
page became unreachable with no way for routing to recover.

Skip the auto-redirect when another non-deleted entry in the collection
still holds the old slug. Any surviving row counts, published or not: a
draft that publishes later would otherwise be shadowed by the redirect.
Renames that genuinely free the URL still redirect as before.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@edrpls
edrpls marked this pull request as ready for review August 3, 2026 11:41
@edrpls
edrpls force-pushed the fix/slug-redirect-locale-aware branch from 6938dbe to ca7fecc Compare August 3, 2026 11:41
@github-actions github-actions Bot added the review/needs-review No maintainer or bot review yet label Aug 3, 2026
@edrpls

edrpls commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (ecebade8) and moved out of draft.

Re-verified the bug is still live on today's main before promoting this: with only the test file applied and the guard removed, both destructive directions still reproduce —

× does not redirect the canonical URL when a translation sharing its slug is renamed
× does not redirect a URL still held by a translation when the canonical is renamed

Nothing on main has touched createSlugChangeRedirect, createAutoRedirect, or the redirect middleware since the branch was cut, so the fix still applies cleanly and the rebase was a fast-forward with no conflicts.

Post-rebase: full packages/core suite green at 5095 passing, lint/typecheck/format clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

area/core review/needs-review No maintainer or bot review yet size/M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@edrpls