CHANGELOG.md

@@ -42,6 +42,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   remotely; otherwise a local directory; empty defaults to the in-repo
   `agent-roles-catalog/` folder — see `.env.example`). (#222)
 
+### Fixed
+
+- **A shared page now keeps EXACTLY ONE custom address (`/l/:alias`).** Editing a
+  page's vanity slug previously inserted a second `share_aliases` row instead of
+  renaming the existing one, leaving the old `/l/<old>` link live forever and
+  making the share modal's lookup nondeterministic. Slug edits and confirmed
+  reassigns now rename/retarget the single row, and a new partial unique index on
+  `(workspace_id, page_id)` enforces the invariant in the database. **Upgrade
+  note:** the accompanying migration `20260627T120000` IRREVERSIBLY deletes the
+  orphaned duplicate alias rows the old bug created (keeping the newest per
+  page), so any previously-live duplicate `/l/<old>` link begins returning the
+  generic 404 after upgrade — intended, but not undoable by `down()`. (#226,
+  #227)
+- **Typing a custom address already used by another page no longer looks like a
+  dead end.** The share modal previously flagged such a name with a red "This
+  address is already in use" error, hiding the fact that saving offers to MOVE
+  the address to the current page. The field now shows an informational hint —
+  "This address is in use. Saving will move it to this page." — and keeps Save
+  enabled, so the existing reassign-confirm flow (`409 ALIAS_REASSIGN_REQUIRED` →
+  "Move custom address?") is discoverable instead of reading as terminal. (#227)
+
 ## [0.94.0] - 2026-06-26
 
 This release makes AI chat durable and fast: assistant turns are persisted to

apps/client/public/locales/en-US/translation.json

@@ -1333,6 +1333,7 @@
   "A short, memorable link you can point at any shared page.": "A short, memorable link you can point at any shared page.",
   "Use 2-60 lowercase letters, digits and hyphens": "Use 2-60 lowercase letters, digits and hyphens",
   "This address is already in use": "This address is already in use",
+  "This address is in use. Saving will move it to this page.": "This address is in use. Saving will move it to this page.",
   "Move custom address?": "Move custom address?",
   "Move here": "Move here",
   "The address \"{{alias}}\" currently points to \"{{title}}\". Move it to this page?": "The address \"{{alias}}\" currently points to \"{{title}}\". Move it to this page?",

apps/client/public/locales/ru-RU/translation.json

@@ -1190,6 +1190,7 @@
   "A short, memorable link you can point at any shared page.": "Короткая запоминающаяся ссылка, которую можно направить на любую опубликованную страницу.",
   "Use 2-60 lowercase letters, digits and hyphens": "Используйте 2–60 строчных букв, цифр и дефисов",
   "This address is already in use": "Этот адрес уже занят",
+  "This address is in use. Saving will move it to this page.": "Этот адрес уже используется. При сохранении он будет перемещён на эту страницу.",
   "Move custom address?": "Переместить пользовательский адрес?",
   "Move here": "Переместить сюда",
   "The address \"{{alias}}\" currently points to \"{{title}}\". Move it to this page?": "Адрес «{{alias}}» сейчас указывает на «{{title}}». Переместить его на эту страницу?",

apps/client/src/features/share/components/share-alias-section.test.tsx

@@ -0,0 +1,149 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { render, screen, fireEvent, waitFor } from "@testing-library/react";
+import { MantineProvider } from "@mantine/core";
+import type { IShareAlias } from "@/features/share/types/share.types";
+
+// matchMedia / storage are stubbed globally in vitest.setup.ts.
+
+// The mutation + query hooks reach react-query/network; the availability probe
+// hits the API. Stub them so the section renders in isolation and we can drive
+// the exact branches (taken name -> hint, 409 -> reassign modal).
+const setMutateAsync = vi.fn();
+let currentAlias: IShareAlias | null = null;
+let availabilityResult: {
+  valid: boolean;
+  available: boolean;
+  currentPageId: string | null;
+} = { valid: true, available: true, currentPageId: null };
+
+vi.mock("@/features/share/queries/share-query.ts", () => ({
+  useShareAliasForPageQuery: () => ({ data: currentAlias }),
+  useSetShareAliasMutation: () => ({
+    mutateAsync: setMutateAsync,
+    isPending: false,
+  }),
+  useRemoveShareAliasMutation: () => ({
+    mutateAsync: vi.fn(),
+    isPending: false,
+  }),
+}));
+
+vi.mock("@/features/share/services/share-service.ts", () => ({
+  checkShareAliasAvailability: vi.fn(async () => availabilityResult),
+}));
+
+import ShareAliasSection from "./share-alias-section";
+
+const aliasRow = (alias: string, pageId: string): IShareAlias => ({
+  id: `alias-${alias}`,
+  workspaceId: "ws-1",
+  alias,
+  pageId,
+  creatorId: "user-1",
+  createdAt: new Date().toISOString(),
+  updatedAt: new Date().toISOString(),
+});
+
+function renderSection(pageId = "page-Y") {
+  return render(
+    <MantineProvider>
+      <ShareAliasSection pageId={pageId} readOnly={false} />
+    </MantineProvider>,
+  );
+}
+
+describe("ShareAliasSection — taken-name handling is never a dead end", () => {
+  beforeEach(() => {
+    setMutateAsync.mockReset();
+    currentAlias = null;
+    availabilityResult = { valid: true, available: true, currentPageId: null };
+  });
+
+  it("shows a 'will move it here' HINT (not a terminal error) when the name belongs to another page, and keeps Save enabled", async () => {
+    // Page Y already owns "bee"; the user retypes a name owned by page X.
+    currentAlias = aliasRow("bee", "page-Y");
+    availabilityResult = {
+      valid: true,
+      available: false,
+      currentPageId: "page-X",
+    };
+
+    renderSection("page-Y");
+    const input = screen.getByPlaceholderText("my-page") as HTMLInputElement;
+    fireEvent.change(input, { target: { value: "test2" } });
+
+    // The reassign hint replaces the old dead-end red error.
+    await waitFor(
+      () =>
+        expect(
+          screen.getByText(
+            "This address is in use. Saving will move it to this page.",
+          ),
+        ).toBeDefined(),
+      { timeout: 2000 },
+    );
+    // The old terminal "already in use" error must NOT be shown.
+    expect(screen.queryByText("This address is already in use")).toBeNull();
+
+    // Save stays enabled so the confirm-reassign flow can run.
+    const saveBtn = screen.getByRole("button", {
+      name: "Save",
+    }) as HTMLButtonElement;
+    expect(saveBtn.disabled).toBe(false);
+  });
+
+  it("opens the reassign-confirm modal on a 409 ALIAS_REASSIGN_REQUIRED (path forward, not a dead end)", async () => {
+    currentAlias = aliasRow("bee", "page-Y");
+    availabilityResult = {
+      valid: true,
+      available: false,
+      currentPageId: "page-X",
+    };
+    // The server rejects the un-confirmed save asking the client to confirm.
+    setMutateAsync.mockRejectedValueOnce({
+      status: 409,
+      response: {
+        status: 409,
+        data: {
+          code: "ALIAS_REASSIGN_REQUIRED",
+          currentPageId: "page-X",
+          currentPageTitle: "Alias Test Page X",
+        },
+      },
+    });
+
+    renderSection("page-Y");
+    const input = screen.getByPlaceholderText("my-page") as HTMLInputElement;
+    fireEvent.change(input, { target: { value: "test2" } });
+
+    const saveBtn = screen.getByRole("button", {
+      name: "Save",
+    }) as HTMLButtonElement;
+    await waitFor(() => expect(saveBtn.disabled).toBe(false), {
+      timeout: 2000,
+    });
+    fireEvent.click(saveBtn);
+
+    // First save sent WITHOUT confirmReassign.
+    await waitFor(() =>
+      expect(setMutateAsync).toHaveBeenCalledWith(
+        expect.objectContaining({ alias: "test2", confirmReassign: false }),
+      ),
+    );
+
+    // The "Move custom address?" confirm modal must appear (the path forward).
+    await waitFor(() =>
+      expect(screen.getByText("Move custom address?")).toBeDefined(),
+    );
+    expect(screen.getByRole("button", { name: "Move here" })).toBeDefined();
+
+    // Confirming retries WITH confirmReassign: true.
+    setMutateAsync.mockResolvedValueOnce(aliasRow("test2", "page-Y"));
+    fireEvent.click(screen.getByRole("button", { name: "Move here" }));
+    await waitFor(() =>
+      expect(setMutateAsync).toHaveBeenCalledWith(
+        expect.objectContaining({ alias: "test2", confirmReassign: true }),
+      ),
+    );
+  });
+});

apps/client/src/features/share/components/share-alias-section.tsx

@@ -120,8 +120,13 @@ export default function ShareAliasSection({
   };
 
   const showInvalid = normalized.length > 0 && !isValid;
-  const showTaken =
+  // The typed name is already in use by ANOTHER page. This is NOT a dead end:
-    isValid && !unchanged && availability && !availability.available;
+  // hitting Save triggers the server's 409 `ALIAS_REASSIGN_REQUIRED` and opens
+  // the "Move custom address?" confirm modal that retargets the address here.
+  // So surface it as an informational hint (not a terminal red error) and keep
+  // Save enabled, instead of looking like the address is unusable.
+  const reassignable =
+    isValid && !unchanged && !!availability && !availability.available;
 
   // The slug prefix (e.g. "docs.example.com/l/") is static for the session.
   const prefixLabel = aliasPrefixLabel();
@@ -198,9 +203,12 @@ export default function ShareAliasSection({
         error={
           showInvalid
             ? t("Use 2-60 lowercase letters, digits and hyphens")
-            : showTaken
+            : undefined
-              ? t("This address is already in use")
+        }
-              : undefined
+        description={
+          reassignable
+            ? t("This address is in use. Saving will move it to this page.")
+            : undefined
         }
       />
 

apps/server/src/core/share/share-alias.service.spec.ts

@@ -1,4 +1,5 @@
 import { BadRequestException, ConflictException } from '@nestjs/common';
+import { NoResultError } from 'kysely';
 import { ShareAliasService } from './share-alias.service';
 
 /**
@@ -7,13 +8,18 @@ import { ShareAliasService } from './share-alias.service';
  * request-time readable-target resolution (which re-runs the share boundary).
  */
 describe('ShareAliasService', () => {
+  // Sentinel handed to repo calls so tests can assert they ran inside the tx.
+  const trx = { __trx: true };
+
   function makeService() {
     const shareAliasRepo = {
       findByAliasAndWorkspace: jest.fn(),
       findByPageId: jest.fn(),
       findById: jest.fn(),
       insert: jest.fn(),
+      updateAlias: jest.fn(),
       updatePageId: jest.fn(),
+      deleteOthersForPage: jest.fn(),
       delete: jest.fn(),
     };
     const pageRepo = { findById: jest.fn() };
@@ -21,12 +27,19 @@ describe('ShareAliasService', () => {
       resolveReadableSharePage: jest.fn(),
       isSharingAllowed: jest.fn(),
     };
+    // Fake kysely db: only .transaction().execute(cb) is used by setAlias.
+    const db = {
+      transaction: jest.fn(() => ({
+        execute: jest.fn(async (cb: any) => cb(trx)),
+      })),
+    };
     const service = new ShareAliasService(
       shareAliasRepo as any,
       pageRepo as any,
       shareService as any,
+      db as any,
     );
-    return { service, shareAliasRepo, pageRepo, shareService };
+    return { service, shareAliasRepo, pageRepo, shareService, db };
   }
 
   describe('setAlias', () => {
@@ -43,9 +56,10 @@ describe('ShareAliasService', () => {
       expect(shareAliasRepo.findByAliasAndWorkspace).not.toHaveBeenCalled();
     });
 
-    it('normalizes then inserts a brand-new alias', async () => {
+    it('normalizes then inserts a brand-new alias (page has none yet)', async () => {
       const { service, shareAliasRepo } = makeService();
       shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
+      shareAliasRepo.findByPageId.mockResolvedValue(undefined);
       shareAliasRepo.insert.mockResolvedValue({ id: 'a-1', alias: 'my-page' });
 
       const res = await service.setAlias({
@@ -58,17 +72,70 @@ describe('ShareAliasService', () => {
       expect(shareAliasRepo.findByAliasAndWorkspace).toHaveBeenCalledWith(
         'my-page',
         'ws-1',
+        trx,
+      );
+      expect(shareAliasRepo.insert).toHaveBeenCalledWith(
+        {
+          workspaceId: 'ws-1',
+          alias: 'my-page',
+          pageId: 'p-1',
+          creatorId: 'u-1',
+        },
+        trx,
+      );
+      expect(shareAliasRepo.updateAlias).not.toHaveBeenCalled();
+      // self-heal still runs, keeping just the inserted row
+      expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
+        'p-1',
+        'a-1',
+        'ws-1',
+        trx,
       );
-      expect(shareAliasRepo.insert).toHaveBeenCalledWith({
-        workspaceId: 'ws-1',
-        alias: 'my-page',
-        pageId: 'p-1',
-        creatorId: 'u-1',
-      });
       expect(res).toMatchObject({ id: 'a-1' });
     });
 
-    it('is a no-op when the alias already points at the same page', async () => {
+    it('renames the existing row in place when editing to a free name (te -> ted)', async () => {
+      const { service, shareAliasRepo } = makeService();
+      // The new slug is free...
+      shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
+      // ...but the page already owns an alias named `te`.
+      shareAliasRepo.findByPageId.mockResolvedValue({
+        id: 'a-1',
+        alias: 'te',
+        pageId: 'p-1',
+      });
+      shareAliasRepo.updateAlias.mockResolvedValue({
+        id: 'a-1',
+        alias: 'ted',
+        pageId: 'p-1',
+      });
+
+      const res = await service.setAlias({
+        workspaceId: 'ws-1',
+        pageId: 'p-1',
+        creatorId: 'u-1',
+        alias: 'ted',
+      });
+
+      // RENAME, not INSERT a second row.
+      expect(shareAliasRepo.insert).not.toHaveBeenCalled();
+      expect(shareAliasRepo.updateAlias).toHaveBeenCalledWith(
+        'a-1',
+        'ted',
+        'ws-1',
+        trx,
+      );
+      // ...and any other row for the page is reaped, so `te` cannot survive.
+      expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
+        'p-1',
+        'a-1',
+        'ws-1',
+        trx,
+      );
+      expect(res).toMatchObject({ id: 'a-1', alias: 'ted' });
+    });
+
+    it('is a no-op when the alias already points at the same page (and self-heals)', async () => {
       const { service, shareAliasRepo } = makeService();
       const existing = { id: 'a-1', alias: 'foo', pageId: 'p-1' };
       shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(existing);
@@ -82,7 +149,45 @@ describe('ShareAliasService', () => {
 
       expect(res).toBe(existing);
       expect(shareAliasRepo.insert).not.toHaveBeenCalled();
+      expect(shareAliasRepo.updateAlias).not.toHaveBeenCalled();
       expect(shareAliasRepo.updatePageId).not.toHaveBeenCalled();
+      // self-heal reaps any legacy duplicate rows for the page
+      expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
+        'p-1',
+        'a-1',
+        'ws-1',
+        trx,
+      );
+    });
+
+    it('self-heals a page with pre-existing duplicate rows down to one', async () => {
+      const { service, shareAliasRepo } = makeService();
+      // Name free; the page already has a (legacy) alias row we rename.
+      shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
+      shareAliasRepo.findByPageId.mockResolvedValue({
+        id: 'a-keep',
+        alias: 'old',
+        pageId: 'p-1',
+      });
+      shareAliasRepo.updateAlias.mockResolvedValue({
+        id: 'a-keep',
+        alias: 'new',
+        pageId: 'p-1',
+      });
+
+      await service.setAlias({
+        workspaceId: 'ws-1',
+        pageId: 'p-1',
+        creatorId: 'u-1',
+        alias: 'new',
+      });
+
+      expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
+        'p-1',
+        'a-keep',
+        'ws-1',
+        trx,
+      );
     });
 
     it('throws 409 with current target when name is taken and not confirmed', async () => {
@@ -134,15 +239,190 @@ describe('ShareAliasService', () => {
         'a-1',
         'p-1',
         'ws-1',
+        trx,
       );
+      // ORDER MATTERS: the target page's existing alias row(s) are reaped BEFORE
+      // the retarget, so the non-deferrable (workspace_id, page_id) index never
+      // sees two rows for the page mid-statement. There is no trailing self-heal.
+      expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
+        'p-1',
+        'a-1',
+        'ws-1',
+        trx,
+      );
+      expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledTimes(1);
+      const deleteOrder =
+        shareAliasRepo.deleteOthersForPage.mock.invocationCallOrder[0];
+      const updateOrder =
+        shareAliasRepo.updatePageId.mock.invocationCallOrder[0];
+      expect(deleteOrder).toBeLessThan(updateOrder);
       expect(res).toMatchObject({ pageId: 'p-1' });
     });
 
-    it('maps a unique-violation race to 409', async () => {
+    it('maps a unique-violation race (no constraint info) to 409 "Alias already taken"', async () => {
       const { service, shareAliasRepo } = makeService();
       shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
       shareAliasRepo.insert.mockRejectedValue({ code: '23505' });
 
+      try {
+        await service.setAlias({
+          workspaceId: 'ws-1',
+          pageId: 'p-1',
+          creatorId: 'u-1',
+          alias: 'foo',
+        });
+        fail('expected ConflictException');
+      } catch (err) {
+        expect(err).toBeInstanceOf(ConflictException);
+        expect((err as ConflictException).getResponse()).toMatchObject({
+          message: 'Alias already taken',
+        });
+      }
+    });
+
+    it('maps the (workspace_id, alias) index violation to "Alias already taken"', async () => {
+      const { service, shareAliasRepo } = makeService();
+      shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
+      // postgres@3.x driver exposes the index name as `constraint_name`.
+      shareAliasRepo.insert.mockRejectedValue({
+        code: '23505',
+        constraint_name: 'share_aliases_workspace_id_alias_unique',
+      });
+
+      try {
+        await service.setAlias({
+          workspaceId: 'ws-1',
+          pageId: 'p-1',
+          creatorId: 'u-1',
+          alias: 'foo',
+        });
+        fail('expected ConflictException');
+      } catch (err) {
+        expect((err as ConflictException).getResponse()).toMatchObject({
+          message: 'Alias already taken',
+        });
+      }
+    });
+
+    it('maps the (workspace_id, page_id) index violation to a DISTINCT page-race outcome', async () => {
+      const { service, shareAliasRepo } = makeService();
+      shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
+      shareAliasRepo.insert.mockRejectedValue({
+        code: '23505',
+        constraint_name: 'share_aliases_workspace_id_page_id_unique',
+      });
+
+      try {
+        await service.setAlias({
+          workspaceId: 'ws-1',
+          pageId: 'p-1',
+          creatorId: 'u-1',
+          alias: 'foo',
+        });
+        fail('expected ConflictException');
+      } catch (err) {
+        expect(err).toBeInstanceOf(ConflictException);
+        // NOT the misleading "Alias already taken" — a separate, page-scoped code.
+        expect((err as ConflictException).getResponse()).toMatchObject({
+          code: 'ALIAS_PAGE_RACE',
+        });
+        expect((err as ConflictException).getResponse()).not.toMatchObject({
+          message: 'Alias already taken',
+        });
+      }
+    });
+
+    it('reads the index name from `.constraint` when `.constraint_name` is absent', async () => {
+      const { service, shareAliasRepo } = makeService();
+      shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
+      // Fallback path for non-postgres@3.x drivers.
+      shareAliasRepo.insert.mockRejectedValue({
+        code: '23505',
+        constraint: 'share_aliases_workspace_id_page_id_unique',
+      });
+
+      try {
+        await service.setAlias({
+          workspaceId: 'ws-1',
+          pageId: 'p-1',
+          creatorId: 'u-1',
+          alias: 'foo',
+        });
+        fail('expected ConflictException');
+      } catch (err) {
+        expect((err as ConflictException).getResponse()).toMatchObject({
+          code: 'ALIAS_PAGE_RACE',
+        });
+      }
+    });
+
+    it('maps a concurrent-delete race in the SWAP branch to a retryable 409 (not a 200-without-alias)', async () => {
+      const { service, shareAliasRepo } = makeService();
+      // Name points at another page; reassign confirmed -> swap branch.
+      shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({
+        id: 'a-1',
+        alias: 'foo',
+        pageId: 'p-other',
+      });
+      // A concurrent removeAlias deleted the row between read and UPDATE, so the
+      // repo's executeTakeFirstOrThrow finds 0 rows and throws NoResultError.
+      shareAliasRepo.updatePageId.mockRejectedValue(
+        new NoResultError({} as any),
+      );
+
+      try {
+        await service.setAlias({
+          workspaceId: 'ws-1',
+          pageId: 'p-1',
+          creatorId: 'u-1',
+          alias: 'foo',
+          confirmReassign: true,
+        });
+        fail('expected ConflictException');
+      } catch (err) {
+        // Crucially NOT a resolved 200 carrying `undefined` as the alias.
+        expect(err).toBeInstanceOf(ConflictException);
+        expect((err as ConflictException).getResponse()).toMatchObject({
+          code: 'ALIAS_PAGE_RACE',
+        });
+      }
+    });
+
+    it('maps a concurrent-delete race in the RENAME branch to a retryable 409 (not a generic 400)', async () => {
+      const { service, shareAliasRepo } = makeService();
+      // New slug is free, but the page already owns an alias we rename in place.
+      shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
+      shareAliasRepo.findByPageId.mockResolvedValue({
+        id: 'a-1',
+        alias: 'te',
+        pageId: 'p-1',
+      });
+      // The row vanished before the UPDATE; repo throws NoResultError rather
+      // than returning undefined (which would dereference undefined.id -> 400).
+      shareAliasRepo.updateAlias.mockRejectedValue(new NoResultError({} as any));
+
+      try {
+        await service.setAlias({
+          workspaceId: 'ws-1',
+          pageId: 'p-1',
+          creatorId: 'u-1',
+          alias: 'ted',
+        });
+        fail('expected ConflictException');
+      } catch (err) {
+        expect(err).toBeInstanceOf(ConflictException);
+        expect(err).not.toBeInstanceOf(BadRequestException);
+        expect((err as ConflictException).getResponse()).toMatchObject({
+          code: 'ALIAS_PAGE_RACE',
+        });
+      }
+    });
+
+    it('maps a non-unique-violation db error to BadRequest (Failed to set alias)', async () => {
+      const { service, shareAliasRepo } = makeService();
+      shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
+      shareAliasRepo.insert.mockRejectedValue({ code: '08006' }); // connection error
+
       await expect(
         service.setAlias({
           workspaceId: 'ws-1',
@@ -150,7 +430,7 @@ describe('ShareAliasService', () => {
           creatorId: 'u-1',
           alias: 'foo',
         }),
-      ).rejects.toBeInstanceOf(ConflictException);
+      ).rejects.toBeInstanceOf(BadRequestException);
     });
   });
 

apps/server/src/core/share/share-alias.service.ts

@@ -9,9 +9,24 @@ import { PageRepo } from '@docmost/db/repos/page/page.repo';
 import { ShareService } from './share.service';
 import { Page, ShareAlias } from '@docmost/db/types/entity.types';
 import { isValidShareAlias, normalizeShareAlias } from './share-alias.util';
+import { InjectKysely } from 'nestjs-kysely';
+import { KyselyDB } from '@docmost/db/types/kysely.types';
+import {
+  executeTx,
+  isUniqueViolation,
+  violatedConstraint,
+} from '@docmost/db/utils';
+import { NoResultError } from 'kysely';
 
-/** Postgres unique_violation; the (workspace_id, alias) constraint races here. */
+/**
-const PG_UNIQUE_VIOLATION = '23505';
+ * Unique index name from the share_aliases migrations whose violation we map to
+ * a DISTINCT, non-misleading outcome:
+ *   - PAGE_ID: partial `(workspace_id, page_id) WHERE page_id IS NOT NULL`
+ *             -> a concurrent writer already gave THIS page an alias.
+ * The `(workspace_id, alias)` index (the vanity NAME being taken) needs no
+ * constant: it is the default "Alias already taken" mapping.
+ */
+const UNIQUE_PAGE_ID_INDEX = 'share_aliases_workspace_id_page_id_unique';
 
 export interface ResolvedAliasTarget {
   share: NonNullable<
@@ -28,16 +43,30 @@ export class ShareAliasService {
     private readonly shareAliasRepo: ShareAliasRepo,
     private readonly pageRepo: PageRepo,
     private readonly shareService: ShareService,
+    @InjectKysely() private readonly db: KyselyDB,
   ) {}
 
   /**
-   * Create or retarget a vanity alias. The alias is workspace-scoped:
+   * Create, RENAME or retarget a page's vanity alias. INVARIANT: a page has
-   *   - no row for this name        -> INSERT a new pointer
+   * EXACTLY ONE custom address. The alias name is workspace-scoped:
-   *   - row already points at pageId -> no-op (idempotent)
+   *   - name free, page has no alias yet -> INSERT a new pointer
-   *   - row points elsewhere         -> the "swap". Without confirmReassign we
+   *   - name free, page already has one  -> RENAME that row in place (the slug
-   *     throw 409 carrying the current target so the client can confirm; with
+   *     edit, e.g. `te` -> `ted`); we never spawn a second row, so no orphan
-   *     it we UPDATE the single row's page_id (every /l/<alias> link follows the
+   *     `/l/<old>` link survives
-   *     302 to the new page instantly — no stale 301 cache).
+   *   - name already points at pageId    -> no-op (idempotent)
+   *   - name points at ANOTHER page      -> the "swap". Without confirmReassign
+   *     we throw 409 carrying the current target so the client can confirm;
+   *     with it we UPDATE the single row's page_id (every /l/<alias> link
+   *     follows the 302 to the new page instantly — no stale cache).
+   *
+   * To keep the invariant self-healing we DELETE every other alias row still
+   * pointing at this page (a legacy duplicate, or the target page's own former
+   * alias during a swap). The whole thing runs in one transaction. Because the
+   * `(workspace_id, page_id)` unique index is NON-deferrable (checked at the end
+   * of each statement), the swap branch DELETEs the target page's existing row
+   * BEFORE retargeting, so the page is never transiently carried by two rows;
+   * the other branches self-heal AFTER their write. Either way the page never
+   * ends a statement with duplicate rows.
    *
    * Caller is responsible for authorizing the page (edit rights + public
    * readability); this method owns only the alias-name semantics.
@@ -57,48 +86,128 @@ export class ShareAliasService {
       );
     }
 
-    const existing = await this.shareAliasRepo.findByAliasAndWorkspace(
+    try {
-      alias,
+      return await executeTx(this.db, async (trx) => {
-      workspaceId,
+        const byName = await this.shareAliasRepo.findByAliasAndWorkspace(
-    );
-
-    if (!existing) {
-      try {
-        return await this.shareAliasRepo.insert({
-          workspaceId,
           alias,
-          pageId,
+          workspaceId,
-          creatorId,
+          trx,
-        });
+        );
-      } catch (err: any) {
+
-        // Lost a uniqueness race: another request claimed the name first.
+        // The name is occupied by a DIFFERENT (or dangling) target page.
-        if (err?.code === PG_UNIQUE_VIOLATION) {
+        if (byName && byName.pageId !== pageId) {
-          throw new ConflictException({ message: 'Alias already taken' });
+          if (!confirmReassign) {
+            const currentPage = byName.pageId
+              ? await this.pageRepo.findById(byName.pageId)
+              : null;
+            throw new ConflictException({
+              message: 'Alias already in use',
+              code: 'ALIAS_REASSIGN_REQUIRED',
+              currentPageId: byName.pageId,
+              currentPageTitle: currentPage?.title ?? null,
+            });
+          }
+          // Confirmed swap. ORDER MATTERS: the partial unique index on
+          // `(workspace_id, page_id)` is NON-deferrable, so it is checked at the
+          // end of EVERY statement. If we retargeted `byName` onto `pageId`
+          // first while `pageId` still had its OWN alias row, there would
+          // momentarily be two rows with this page_id -> immediate 23505 and a
+          // rolled-back tx (a misleading "Alias already taken"). So we FIRST drop
+          // the target page's existing alias row(s), THEN retarget. `byName.id`
+          // still points at its old page here, so excluding it via `keepId` is
+          // harmless; after the retarget it is the page's only row, so no
+          // trailing self-heal is needed.
+          await this.shareAliasRepo.deleteOthersForPage(
+            pageId,
+            byName.id,
+            workspaceId,
+            trx,
+          );
+          return await this.shareAliasRepo.updatePageId(
+            byName.id,
+            pageId,
+            workspaceId,
+            trx,
+          );
         }
-        this.logger.error(err);
-        throw new BadRequestException('Failed to set alias');
-      }
-    }
 
-    // Already points at this page -> nothing to do.
+        // The name is FREE, or already points at THIS page. Ensure the page has
-    if (existing.pageId === pageId) {
+        // a single row carrying this name: rename its current one, or insert.
-      return existing;
+        const current =
-    }
+          byName ??
+          (await this.shareAliasRepo.findByPageId(pageId, workspaceId, trx));
 
-    // Name occupied by a different (or dangling) target: require confirmation.
+        let row: ShareAlias;
-    if (!confirmReassign) {
+        if (current) {
-      const currentPage = existing.pageId
+          row =
-        ? await this.pageRepo.findById(existing.pageId)
+            current.alias === alias
-        : null;
+              ? current // same-name no-op
-      throw new ConflictException({
+              : await this.shareAliasRepo.updateAlias(
-        message: 'Alias already in use',
+                  current.id,
-        code: 'ALIAS_REASSIGN_REQUIRED',
+                  alias,
-        currentPageId: existing.pageId,
+                  workspaceId,
-        currentPageTitle: currentPage?.title ?? null,
+                  trx,
+                );
+        } else {
+          row = await this.shareAliasRepo.insert(
+            { workspaceId, alias, pageId, creatorId },
+            trx,
+          );
+        }
+
+        // Self-heal: a page keeps EXACTLY ONE custom address.
+        await this.shareAliasRepo.deleteOthersForPage(
+          pageId,
+          row.id,
+          workspaceId,
+          trx,
+        );
+        return row;
       });
+    } catch (err: any) {
+      if (
+        err instanceof ConflictException ||
+        err instanceof BadRequestException
+      ) {
+        throw err;
+      }
+      // The row we read was deleted (concurrent `removeAlias`) before our UPDATE
+      // matched it, so `executeTakeFirstOrThrow` found no row. Surface a
+      // retryable conflict instead of a 200-without-alias (swap branch) or a
+      // generic 400 from dereferencing `undefined.id` (rename branch).
+      if (err instanceof NoResultError) {
+        this.logger.warn(
+          'share alias update matched no row (concurrent-delete race)',
+        );
+        throw new ConflictException({
+          message: 'The address changed concurrently, please retry',
+          code: 'ALIAS_PAGE_RACE',
+        });
+      }
+      // A unique index fired. Which one decides the message — always log the
+      // constraint so the race is diagnosable.
+      if (isUniqueViolation(err)) {
+        const constraint = violatedConstraint(err);
+        this.logger.warn(
+          `share alias unique violation on ${constraint ?? '<unknown>'}`,
+        );
+        // `(workspace_id, page_id)`: a concurrent request already gave this page
+        // an alias. The page still has exactly one custom address (the racing
+        // writer's), so this is not a user-facing name clash — surface a
+        // distinct, non-misleading message instead of "Alias already taken".
+        if (constraint === UNIQUE_PAGE_ID_INDEX) {
+          throw new ConflictException({
+            message: 'This page is being given an address by another request',
+            code: 'ALIAS_PAGE_RACE',
+          });
+        }
+        // `(workspace_id, alias)` or any other/unknown unique index: treat as
+        // the vanity name being claimed first.
+        throw new ConflictException({ message: 'Alias already taken' });
+      }
+      this.logger.error(err);
+      throw new BadRequestException('Failed to set alias');
     }
-
-    return this.shareAliasRepo.updatePageId(existing.id, pageId, workspaceId);
   }
 
   /** Free a vanity name (no history kept). */

apps/server/src/database/migrations/20260627T120000-share-aliases-one-per-page.ts

@@ -0,0 +1,48 @@
+import { type Kysely, sql } from 'kysely';
+
+/**
+ * Enforce "a page has EXACTLY ONE custom address" at the DB level. The original
+ * `share_aliases` table only had a unique index on `(workspace_id, alias)`, so a
+ * page could accumulate several alias rows (every slug edit used to INSERT a new
+ * one), leaving orphan `/l/<old>` links live forever and making the share
+ * modal's `findByPageId` lookup nondeterministic.
+ *
+ * We first dedup any pre-existing rows (keeping the NEWEST per page — the same
+ * "current" choice the read path now makes), then add a PARTIAL unique index on
+ * `(workspace_id, page_id)`. It is partial (`WHERE page_id IS NOT NULL`) so that
+ * multiple DANGLING aliases (target page deleted -> `page_id` SET NULL) can
+ * still coexist without colliding.
+ *
+ * ⚠️ IRREVERSIBLE DATA LOSS (intended): the dedup DELETE below permanently drops
+ * every alias row but the newest per page. Those duplicates were live `/l/<old>`
+ * pointers (resolved by name via `findByAliasAndWorkspace`, not by page), so
+ * after this upgrade any such OLD vanity link starts returning the SPA 404. This
+ * is the point — it kills the orphan rows the pre-invariant bug accumulated —
+ * but `down()` only drops the unique index; it CANNOT restore the deleted rows.
+ */
+export async function up(db: Kysely<any>): Promise<void> {
+  // Reap legacy duplicates: for each (workspace_id, page_id) keep only the row
+  // with the greatest (created_at, id) — matches ShareAliasRepo.findByPageId.
+  await sql`
+    DELETE FROM share_aliases sa
+    USING share_aliases keep
+    WHERE sa.page_id IS NOT NULL
+      AND sa.workspace_id = keep.workspace_id
+      AND sa.page_id = keep.page_id
+      AND (keep.created_at, keep.id) > (sa.created_at, sa.id)
+  `.execute(db);
+
+  await db.schema
+    .createIndex('share_aliases_workspace_id_page_id_unique')
+    .on('share_aliases')
+    .columns(['workspace_id', 'page_id'])
+    .unique()
+    .where('page_id', 'is not', null)
+    .execute();
+}
+
+export async function down(db: Kysely<any>): Promise<void> {
+  await db.schema
+    .dropIndex('share_aliases_workspace_id_page_id_unique')
+    .execute();
+}

apps/server/src/database/repos/favorite/favorite.repo.ts

@@ -7,7 +7,7 @@ import { executeWithCursorPagination } from '@docmost/db/pagination/cursor-pagin
 import { jsonObjectFrom } from 'kysely/helpers/postgres';
 import { ExpressionBuilder, SelectQueryBuilder, sql } from 'kysely';
 import { DB } from '@docmost/db/types/db';
-import { dbOrTx } from '@docmost/db/utils';
+import { dbOrTx, isUniqueViolation } from '@docmost/db/utils';
 
 export const FavoriteType = {
   PAGE: 'page',
@@ -29,7 +29,8 @@ export class FavoriteRepo {
         .returningAll()
         .executeTakeFirst();
     } catch (err: any) {
-      if (err?.code === '23505') return undefined;
+      // Idempotent favorite: a duplicate (already-favorited) is not an error.
+      if (isUniqueViolation(err)) return undefined;
       throw err;
     }
   }

apps/server/src/database/repos/share-alias/share-alias.repo.spec.ts

@@ -10,16 +10,21 @@ import type { KyselyDB } from '../../types/kysely.types';
 describe('ShareAliasRepo', () => {
   function makeSelectRepo(result: unknown) {
     const where = jest.fn();
+    const orderBy = jest.fn();
     const builder: any = {
       select: jest.fn(() => builder),
       where: jest.fn((...args: unknown[]) => {
         where(...args);
         return builder;
       }),
+      orderBy: jest.fn((...args: unknown[]) => {
+        orderBy(...args);
+        return builder;
+      }),
       executeTakeFirst: jest.fn().mockResolvedValue(result),
     };
     const db = { selectFrom: jest.fn(() => builder) } as unknown as KyselyDB;
-    return { repo: new ShareAliasRepo(db), db, where, builder };
+    return { repo: new ShareAliasRepo(db), db, where, orderBy, builder };
   }
 
   it('findByAliasAndWorkspace scopes by alias AND workspace', async () => {
@@ -34,11 +39,15 @@ describe('ShareAliasRepo', () => {
     expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
   });
 
-  it('findByPageId scopes by page AND workspace', async () => {
+  it('findByPageId scopes by page AND workspace, deterministically ordered', async () => {
-    const { repo, where } = makeSelectRepo(undefined);
+    const { repo, where, orderBy } = makeSelectRepo(undefined);
     await repo.findByPageId('p-1', 'ws-1');
     expect(where).toHaveBeenCalledWith('pageId', '=', 'p-1');
     expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
+    // Explicit ORDER BY removes the nondeterministic heap order for any legacy
+    // duplicate rows (newest createdAt wins, id as a stable tiebreak).
+    expect(orderBy).toHaveBeenCalledWith('createdAt', 'desc');
+    expect(orderBy).toHaveBeenCalledWith('id', 'desc');
   });
 
   it('insert writes the provided columns and returns the row', async () => {
@@ -85,7 +94,9 @@ describe('ShareAliasRepo', () => {
         return builder;
       }),
       returning: jest.fn(() => builder),
-      executeTakeFirst: jest.fn().mockResolvedValue({ id: 'a-1' }),
+      // Retarget uses executeTakeFirstOrThrow so a row reaped by a concurrent
+      // delete (0 rows matched) raises NoResultError instead of returning undefined.
+      executeTakeFirstOrThrow: jest.fn().mockResolvedValue({ id: 'a-1' }),
     };
     const db = { updateTable: jest.fn(() => builder) } as unknown as KyselyDB;
     const repo = new ShareAliasRepo(db);
@@ -99,6 +110,60 @@ describe('ShareAliasRepo', () => {
     expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
   });
 
+  it('updateAlias renames a single row scoped by id + workspace', async () => {
+    const set = jest.fn();
+    const where = jest.fn();
+    const builder: any = {
+      set: jest.fn((s: unknown) => {
+        set(s);
+        return builder;
+      }),
+      where: jest.fn((...args: unknown[]) => {
+        where(...args);
+        return builder;
+      }),
+      returning: jest.fn(() => builder),
+      // Rename uses executeTakeFirstOrThrow so a row reaped by a concurrent
+      // delete (0 rows matched) raises NoResultError instead of returning undefined.
+      executeTakeFirstOrThrow: jest
+        .fn()
+        .mockResolvedValue({ id: 'a-1', alias: 'ted' }),
+    };
+    const db = { updateTable: jest.fn(() => builder) } as unknown as KyselyDB;
+    const repo = new ShareAliasRepo(db);
+
+    const res = await repo.updateAlias('a-1', 'ted', 'ws-1');
+
+    expect(db.updateTable).toHaveBeenCalledWith('shareAliases');
+    expect(set.mock.calls[0][0].alias).toBe('ted');
+    expect(set.mock.calls[0][0].updatedAt).toBeInstanceOf(Date);
+    // a rename must NOT touch page_id (the page's pointer is preserved)
+    expect(set.mock.calls[0][0]).not.toHaveProperty('pageId');
+    expect(where).toHaveBeenCalledWith('id', '=', 'a-1');
+    expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
+    expect(res).toMatchObject({ alias: 'ted' });
+  });
+
+  it('deleteOthersForPage reaps every row for the page except keepId', async () => {
+    const where = jest.fn();
+    const builder: any = {
+      where: jest.fn((...args: unknown[]) => {
+        where(...args);
+        return builder;
+      }),
+      execute: jest.fn().mockResolvedValue(undefined),
+    };
+    const db = { deleteFrom: jest.fn(() => builder) } as unknown as KyselyDB;
+    const repo = new ShareAliasRepo(db);
+
+    await repo.deleteOthersForPage('p-1', 'a-keep', 'ws-1');
+
+    expect(db.deleteFrom).toHaveBeenCalledWith('shareAliases');
+    expect(where).toHaveBeenCalledWith('pageId', '=', 'p-1');
+    expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
+    expect(where).toHaveBeenCalledWith('id', '!=', 'a-keep');
+  });
+
   it('delete scopes by id + workspace', async () => {
     const where = jest.fn();
     const builder: any = {

apps/server/src/database/repos/share-alias/share-alias.repo.ts

@@ -41,7 +41,14 @@ export class ShareAliasRepo {
       .executeTakeFirst();
   }
 
-  /** The alias currently pointing at a page (for the share modal). */
+  /**
+   * The alias currently pointing at a page (for the share modal). The service
+   * enforces a single alias row per page, but legacy rows (pre-invariant) may
+   * still exist until self-healed; the explicit ORDER BY makes the "current"
+   * choice DETERMINISTIC (newest wins — i.e. the most recently created address,
+   * which is the one the user last asked for) instead of an arbitrary Postgres
+   * heap order.
+   */
   async findByPageId(
     pageId: string,
     workspaceId: string,
@@ -52,6 +59,8 @@ export class ShareAliasRepo {
       .select(this.baseFields)
       .where('pageId', '=', pageId)
       .where('workspaceId', '=', workspaceId)
+      .orderBy('createdAt', 'desc')
+      .orderBy('id', 'desc')
       .executeTakeFirst();
   }
 
@@ -79,7 +88,60 @@ export class ShareAliasRepo {
       .executeTakeFirst();
   }
 
-  /** Retarget an existing alias to a new page (the "swap" operation). */
+  /**
+   * Rename an existing alias row in place (the vanity-slug edit, e.g.
+   * `te` -> `ted`). Keeps the row's id/page_id/creator so the page's single
+   * alias pointer is preserved — only the human-readable name changes.
+   *
+   * Uses `executeTakeFirstOrThrow`: if a concurrent `delete` reaps this row
+   * between the service's read and this UPDATE (READ COMMITTED), the UPDATE
+   * matches 0 rows and kysely throws `NoResultError` rather than returning
+   * `undefined` for a `Promise<ShareAlias>`. The service maps that to a
+   * retryable conflict instead of dereferencing `undefined.id`.
+   */
+  async updateAlias(
+    id: string,
+    alias: string,
+    workspaceId: string,
+    trx?: KyselyTransaction,
+  ): Promise<ShareAlias> {
+    return dbOrTx(this.db, trx)
+      .updateTable('shareAliases')
+      .set({ alias, updatedAt: new Date() })
+      .where('id', '=', id)
+      .where('workspaceId', '=', workspaceId)
+      .returning(this.baseFields)
+      .executeTakeFirstOrThrow();
+  }
+
+  /**
+   * Self-heal helper: drop every OTHER alias row still pointing at a page,
+   * keeping only `keepId`. Enforces the "exactly one custom address per page"
+   * invariant after a rename/retarget and reaps any legacy duplicates.
+   */
+  async deleteOthersForPage(
+    pageId: string,
+    keepId: string,
+    workspaceId: string,
+    trx?: KyselyTransaction,
+  ): Promise<void> {
+    await dbOrTx(this.db, trx)
+      .deleteFrom('shareAliases')
+      .where('pageId', '=', pageId)
+      .where('workspaceId', '=', workspaceId)
+      .where('id', '!=', keepId)
+      .execute();
+  }
+
+  /**
+   * Retarget an existing alias to a new page (the "swap" operation).
+   *
+   * Uses `executeTakeFirstOrThrow`: if a concurrent `delete` reaps this row
+   * between the service's read and this UPDATE, the UPDATE matches 0 rows and
+   * kysely throws `NoResultError` instead of returning `undefined` into the 200
+   * response (a "success" with no alias). The service maps that to a retryable
+   * conflict.
+   */
   async updatePageId(
     id: string,
     pageId: string,
@@ -92,7 +154,7 @@ export class ShareAliasRepo {
       .where('id', '=', id)
       .where('workspaceId', '=', workspaceId)
       .returning(this.baseFields)
-      .executeTakeFirst();
+      .executeTakeFirstOrThrow();
   }
 
   async delete(

apps/server/src/database/unique-violation.spec.ts

@@ -0,0 +1,51 @@
+import { isUniqueViolation, violatedConstraint } from './utils';
+
+/**
+ * Unit tests for the driver-bound Postgres unique-violation helpers extracted
+ * from the share-alias service (and now shared with favorite.repo). They encode
+ * two `kysely-postgres-js` / `postgres@3.x` quirks: the SQLSTATE is the string
+ * `'23505'`, and the violated index name arrives as `constraint_name` (with
+ * `constraint` only a fallback for other drivers).
+ */
+describe('isUniqueViolation', () => {
+  it('is true for a 23505 error', () => {
+    expect(isUniqueViolation({ code: '23505' })).toBe(true);
+  });
+
+  it('is false for any other code', () => {
+    expect(isUniqueViolation({ code: '08006' })).toBe(false);
+  });
+
+  it('is false when there is no code / not an object', () => {
+    expect(isUniqueViolation({})).toBe(false);
+    expect(isUniqueViolation(null)).toBe(false);
+    expect(isUniqueViolation(undefined)).toBe(false);
+    expect(isUniqueViolation(new Error('boom'))).toBe(false);
+  });
+});
+
+describe('violatedConstraint', () => {
+  it('reads the postgres@3.x `constraint_name` field', () => {
+    expect(
+      violatedConstraint({ code: '23505', constraint_name: 'idx_a' }),
+    ).toBe('idx_a');
+  });
+
+  it('falls back to `constraint` when `constraint_name` is absent', () => {
+    expect(violatedConstraint({ code: '23505', constraint: 'idx_b' })).toBe(
+      'idx_b',
+    );
+  });
+
+  it('prefers `constraint_name` over `constraint` when both are present', () => {
+    expect(
+      violatedConstraint({ constraint_name: 'idx_a', constraint: 'idx_b' }),
+    ).toBe('idx_a');
+  });
+
+  it('is undefined when neither field is present', () => {
+    expect(violatedConstraint({ code: '23505' })).toBeUndefined();
+    expect(violatedConstraint(null)).toBeUndefined();
+    expect(violatedConstraint(undefined)).toBeUndefined();
+  });
+});

apps/server/src/database/utils.ts

@@ -33,6 +33,35 @@ export function dbOrTx(
   }
 }
 
+/** Postgres `unique_violation` SQLSTATE — raised when a write hits a UNIQUE index. */
+const PG_UNIQUE_VIOLATION = '23505';
+
+/**
+ * Whether `err` is a Postgres unique-violation (SQLSTATE `23505`). THE single
+ * check so repos/services stop re-hardcoding the magic code.
+ *
+ * NOTE (#222): `core/ai-chat/roles/ai-agent-roles.service.ts` still carries its
+ * own inline `23505` check on a separate, unmerged branch; it should adopt this
+ * helper (and {@link violatedConstraint}) after #227 lands.
+ */
+export function isUniqueViolation(err: unknown): boolean {
+  return (err as { code?: unknown } | null | undefined)?.code === PG_UNIQUE_VIOLATION;
+}
+
+/**
+ * The name of the UNIQUE index/constraint a `23505` error violated, or
+ * undefined. The `kysely-postgres-js` / `postgres@3.x` driver surfaces it as
+ * `err.constraint_name` (NOT `.constraint`); `.constraint` is kept only as a
+ * defensive fallback for other drivers.
+ */
+export function violatedConstraint(err: unknown): string | undefined {
+  const e = err as
+    | { constraint_name?: string; constraint?: string }
+    | null
+    | undefined;
+  return e?.constraint_name ?? e?.constraint;
+}
+
 /**
  * Bind a JS array/object as a `jsonb` column value, working around a postgres
  * driver double-encoding quirk. THE single implementation — repos that persist

apps/server/test/integration/share-alias-one-per-page.int-spec.ts

@@ -0,0 +1,344 @@
+import { Kysely, sql } from 'kysely';
+import { randomUUID } from 'node:crypto';
+import { BadRequestException, ConflictException } from '@nestjs/common';
+import { ShareAliasRepo } from '@docmost/db/repos/share-alias/share-alias.repo';
+import { ShareAliasService } from 'src/core/share/share-alias.service';
+import * as onePerPageMigration from 'src/database/migrations/20260627T120000-share-aliases-one-per-page';
+import {
+  getTestDb,
+  destroyTestDb,
+  createWorkspace,
+  createSpace,
+  createPage,
+} from './db';
+
+/**
+ * Issue #226 (regression of #205): "a page has EXACTLY ONE custom address".
+ * Exercises against real Postgres:
+ *   - the partial unique index `(workspace_id, page_id) WHERE page_id IS NOT NULL`
+ *     (migration 20260627T120000) — one alias per page, but dangling aliases
+ *     (page_id NULL) may coexist;
+ *   - the migration's dedup DELETE keeps the NEWEST row per page;
+ *   - ShareAliasService.setAlias renames in place (te -> ted) instead of
+ *     spawning a second row, and self-heals the page down to one alias.
+ */
+describe('share_aliases one-per-page invariant [integration]', () => {
+  let db: Kysely<any>;
+  let repo: ShareAliasRepo;
+  let service: ShareAliasService;
+  let wsId: string;
+  let spaceId: string;
+
+  // setAlias only consults pageRepo on the unconfirmed-reassign (409) path.
+  const pageRepo = {
+    findById: async (id: string) => ({ id, title: `title-${id}` }),
+  };
+
+  beforeAll(async () => {
+    db = getTestDb();
+    repo = new ShareAliasRepo(db as any);
+    service = new ShareAliasService(
+      repo as any,
+      pageRepo as any,
+      {} as any, // shareService — unused by setAlias
+      db as any,
+    );
+    wsId = (await createWorkspace(db)).id;
+    spaceId = (await createSpace(db, wsId)).id;
+  });
+
+  afterAll(async () => {
+    await destroyTestDb();
+  });
+
+  const newPage = async (): Promise<string> =>
+    (await createPage(db, { workspaceId: wsId, spaceId })).id;
+
+  const aliasRowsForWs = (pageId: string, workspaceId: string) =>
+    db
+      .selectFrom('shareAliases')
+      .select(['id', 'alias'])
+      .where('pageId', '=', pageId)
+      .where('workspaceId', '=', workspaceId)
+      .orderBy('alias')
+      .execute();
+
+  const aliasRowsFor = (pageId: string) => aliasRowsForWs(pageId, wsId);
+
+  it('partial unique index rejects a second alias for the same page (23505)', async () => {
+    const pageId = await newPage();
+    await repo.insert({ workspaceId: wsId, alias: 'first', pageId });
+
+    let code: string | undefined;
+    try {
+      await repo.insert({ workspaceId: wsId, alias: 'second', pageId });
+    } catch (err: any) {
+      code = err?.code ?? err?.cause?.code;
+    }
+    expect(code).toBe('23505');
+  });
+
+  it('allows multiple DANGLING aliases (page_id NULL) — partial index excludes them', async () => {
+    const a = await repo.insert({
+      workspaceId: wsId,
+      alias: `dangling-${randomUUID().slice(0, 8)}`,
+      pageId: null as any,
+    });
+    const b = await repo.insert({
+      workspaceId: wsId,
+      alias: `dangling-${randomUUID().slice(0, 8)}`,
+      pageId: null as any,
+    });
+    expect(a.id).toBeDefined();
+    expect(b.id).toBeDefined();
+    expect(a.id).not.toBe(b.id);
+  });
+
+  it("migration dedup DELETE keeps the page's NEWEST alias row", async () => {
+    const pageId = await newPage();
+    // Temporarily drop the guard so we can seed the legacy duplicate shape.
+    await sql`DROP INDEX share_aliases_workspace_id_page_id_unique`.execute(db);
+    try {
+      const mk = async (alias: string, createdAt: string): Promise<string> => {
+        const id = randomUUID();
+        await db
+          .insertInto('shareAliases')
+          .values({ id, workspaceId: wsId, alias, pageId, createdAt })
+          .execute();
+        return id;
+      };
+      await mk('oldest', '2026-01-01T00:00:00Z');
+      await mk('middle', '2026-02-01T00:00:00Z');
+      const newest = await mk('newest', '2026-03-01T00:00:00Z');
+
+      // Exact dedup statement from the migration.
+      await sql`
+        DELETE FROM share_aliases sa
+        USING share_aliases keep
+        WHERE sa.page_id IS NOT NULL
+          AND sa.workspace_id = keep.workspace_id
+          AND sa.page_id = keep.page_id
+          AND (keep.created_at, keep.id) > (sa.created_at, sa.id)
+      `.execute(db);
+
+      const rows = await aliasRowsFor(pageId);
+      expect(rows).toHaveLength(1);
+      expect(rows[0]).toMatchObject({ id: newest, alias: 'newest' });
+    } finally {
+      await sql`
+        CREATE UNIQUE INDEX share_aliases_workspace_id_page_id_unique
+        ON share_aliases (workspace_id, page_id)
+        WHERE page_id IS NOT NULL
+      `.execute(db);
+    }
+  });
+
+  it('migration up() dedups per page and leaves OTHER pages and workspaces untouched', async () => {
+    // Seed legacy duplicates for two pages in this workspace AND a page in a
+    // SECOND workspace, then run the real migration up() (not an inlined copy of
+    // its SQL) and assert it scopes the DELETE to (workspace_id, page_id).
+    const ws2 = (await createWorkspace(db)).id;
+    const space2 = (await createSpace(db, ws2)).id;
+    const pageA = await newPage();
+    const pageB = await newPage();
+    const pageC = (await createPage(db, { workspaceId: ws2, spaceId: space2 }))
+      .id;
+
+    // Drop the guard so we can seed the pre-invariant duplicate shape.
+    await sql`DROP INDEX share_aliases_workspace_id_page_id_unique`.execute(db);
+    const seed = async (
+      workspaceId: string,
+      pageId: string,
+      alias: string,
+      createdAt: string,
+    ): Promise<string> => {
+      const id = randomUUID();
+      await db
+        .insertInto('shareAliases')
+        .values({ id, workspaceId, alias, pageId, createdAt })
+        .execute();
+      return id;
+    };
+    await seed(wsId, pageA, 'a-old', '2026-01-01T00:00:00Z');
+    const aNew = await seed(wsId, pageA, 'a-new', '2026-03-01T00:00:00Z');
+    await seed(wsId, pageB, 'b-old', '2026-01-01T00:00:00Z');
+    const bNew = await seed(wsId, pageB, 'b-new', '2026-03-01T00:00:00Z');
+    await seed(ws2, pageC, 'c-old', '2026-01-01T00:00:00Z');
+    const cNew = await seed(ws2, pageC, 'c-new', '2026-03-01T00:00:00Z');
+
+    // Run the migration. It dedups AND recreates the unique index.
+    await onePerPageMigration.up(db as any);
+
+    const aliasesOf = async (pageId: string) =>
+      (await aliasRowsForWs(pageId, wsId)).map((r) => r.alias);
+    const aRows = await aliasRowsForWs(pageA, wsId);
+    expect(aRows).toEqual([{ id: aNew, alias: 'a-new' }]);
+    const bRows = await aliasRowsForWs(pageB, wsId);
+    expect(bRows).toEqual([{ id: bNew, alias: 'b-new' }]);
+    // The other workspace's page keeps only ITS newest row, untouched by wsId.
+    const cRows = await aliasRowsForWs(pageC, ws2);
+    expect(cRows).toEqual([{ id: cNew, alias: 'c-new' }]);
+    expect(await aliasesOf(pageA)).toEqual(['a-new']);
+  });
+
+  it('setAlias renames te -> ted in place: page ends with ONE row named ted', async () => {
+    const pageId = await newPage();
+    const creatorId = null as any;
+    const first = await service.setAlias({
+      workspaceId: wsId,
+      pageId,
+      creatorId,
+      alias: 'te',
+    });
+    expect(first.alias).toBe('te');
+
+    const renamed = await service.setAlias({
+      workspaceId: wsId,
+      pageId,
+      creatorId,
+      alias: 'ted',
+    });
+    // Same row id — a RENAME, not a new insert.
+    expect(renamed.id).toBe(first.id);
+    expect(renamed.alias).toBe('ted');
+
+    const rows = await aliasRowsFor(pageId);
+    expect(rows).toHaveLength(1);
+    expect(rows[0].alias).toBe('ted'); // the stale `te` row is gone
+
+    // The modal read resolves the current (only) row deterministically.
+    const shown = await service.getAliasForPage(pageId, wsId);
+    expect(shown?.alias).toBe('ted');
+  });
+
+  it('setAlias inserts the first alias, then is a no-op for the same name', async () => {
+    const pageId = await newPage();
+    const inserted = await service.setAlias({
+      workspaceId: wsId,
+      pageId,
+      creatorId: null as any,
+      alias: 'hello',
+    });
+    const again = await service.setAlias({
+      workspaceId: wsId,
+      pageId,
+      creatorId: null as any,
+      alias: 'hello',
+    });
+    expect(again.id).toBe(inserted.id);
+    expect(await aliasRowsFor(pageId)).toHaveLength(1);
+  });
+
+  it('a mid-transaction error becomes BadRequestException and rolls back cleanly', async () => {
+    // A non-23505 failure inside the tx must surface as BadRequest AND leave NO
+    // partial alias state behind (the whole executeTx unit rolls back).
+    const pageId = await newPage();
+    const boom = new Error('disk on fire'); // not a unique-violation
+    // Wrap the real repo so the INSERT succeeds but the trailing self-heal
+    // throws — the row inserted earlier in the tx must not survive.
+    const flakyRepo = Object.create(repo);
+    flakyRepo.deleteOthersForPage = async () => {
+      throw boom;
+    };
+    const flakyService = new ShareAliasService(
+      flakyRepo as any,
+      pageRepo as any,
+      {} as any,
+      db as any,
+    );
+
+    await expect(
+      flakyService.setAlias({
+        workspaceId: wsId,
+        pageId,
+        creatorId: null as any,
+        alias: 'rollback-me',
+      }),
+    ).rejects.toBeInstanceOf(BadRequestException);
+
+    // Rolled back: neither the page nor the name has any row.
+    expect(await aliasRowsFor(pageId)).toHaveLength(0);
+    expect(
+      await db
+        .selectFrom('shareAliases')
+        .select('id')
+        .where('alias', '=', 'rollback-me')
+        .where('workspaceId', '=', wsId)
+        .execute(),
+    ).toHaveLength(0);
+  });
+
+  it('cross-page collision throws 409, and confirmReassign moves the single row', async () => {
+    const pageA = await newPage();
+    const pageB = await newPage();
+    await service.setAlias({
+      workspaceId: wsId,
+      pageId: pageA,
+      creatorId: null as any,
+      alias: 'shared',
+    });
+
+    await expect(
+      service.setAlias({
+        workspaceId: wsId,
+        pageId: pageB,
+        creatorId: null as any,
+        alias: 'shared',
+      }),
+    ).rejects.toBeInstanceOf(ConflictException);
+
+    const moved = await service.setAlias({
+      workspaceId: wsId,
+      pageId: pageB,
+      creatorId: null as any,
+      alias: 'shared',
+      confirmReassign: true,
+    });
+    expect(moved.alias).toBe('shared');
+
+    // The name now belongs to pageB only; pageA has no alias.
+    expect(await aliasRowsFor(pageA)).toHaveLength(0);
+    const bRows = await aliasRowsFor(pageB);
+    expect(bRows).toHaveLength(1);
+    expect(bRows[0].alias).toBe('shared');
+  });
+
+  it('confirmReassign onto a page that ALREADY has its own alias: target ends with ONE row', async () => {
+    // Regression guard for the operation-order bug: A has `shared`, B has its
+    // OWN alias `bee`. Moving `shared` onto B must FIRST drop B's `bee` row,
+    // THEN retarget, or the NON-deferrable (workspace_id, page_id) index fires a
+    // 23505 mid-transaction (two rows momentarily carry page_id = B) and the tx
+    // rolls back into a misleading "Alias already taken".
+    // Distinct names: the workspace is shared across tests, so reuse of an
+    // earlier test's `shared` would trip the 409 guard before we get here.
+    const pageA = await newPage();
+    const pageB = await newPage();
+    await service.setAlias({
+      workspaceId: wsId,
+      pageId: pageA,
+      creatorId: null as any,
+      alias: 'shared-target',
+    });
+    await service.setAlias({
+      workspaceId: wsId,
+      pageId: pageB,
+      creatorId: null as any,
+      alias: 'bee',
+    });
+
+    const moved = await service.setAlias({
+      workspaceId: wsId,
+      pageId: pageB,
+      creatorId: null as any,
+      alias: 'shared-target',
+      confirmReassign: true,
+    });
+    expect(moved.alias).toBe('shared-target');
+
+    // B now carries exactly `shared-target` (its old `bee` is gone); A has none.
+    const bRows = await aliasRowsFor(pageB);
+    expect(bRows).toHaveLength(1);
+    expect(bRows[0].alias).toBe('shared-target');
+    expect(await aliasRowsFor(pageA)).toHaveLength(0);
+  });
+});