fix: honor repository guard return values in deleteLink and disableLink

src/__tests__/repository/link-repository.test.ts

@@ -398,4 +398,23 @@ describe("LinkRepository click_count options", () => {
 
     expect(removed).toBe(false);
   });
+
+  it("delete returns every slug it removed on success", async () => {
+    const link = await LinkRepository.create(env.DB, { url: "https://example.com", slug: "primary" });
+    await SlugRepository.addCustom(env.DB, link.id, "secondary");
+
+    const removed = await LinkRepository.delete(env.DB, link.id);
+
+    expect(removed).not.toBe(false);
+    if (removed !== false) {
+      expect([...removed].sort()).toEqual(["primary", "secondary"]);
+    }
+  });
+
+  it("exists reports whether a link row is present", async () => {
+    const link = await LinkRepository.create(env.DB, { url: "https://example.com", slug: "present" });
+
+    expect(await LinkRepository.exists(env.DB, link.id)).toBe(true);
+    expect(await LinkRepository.exists(env.DB, 999999)).toBe(false);
+  });
 });

src/__tests__/service/ownership.test.ts

@@ -1,7 +1,8 @@
-import { beforeAll, beforeEach, describe, expect, it } from "vitest";
+import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
 import { env } from "cloudflare:test";
 import { applyMigrations, resetData } from "../setup";
 import { LinkRepository } from "../../db";
+import { SlugCache } from "../../kv";
 import {
   createLink,
   disableLink,
@@ -272,3 +273,85 @@ describe("List links by owner", () => {
     }
   });
 });
+
+describe("deleteLink: DB-level guard return value is honored", () => {
+  it("returns 400 when the repository delete guard fires after the service-level check", async () => {
+    // Simulate the race where a click is recorded between the service's
+    // total_clicks check and the underlying DB delete: the repository
+    // re-checks inside delete() and returns false. The service must surface
+    // that as a 400, not silently report { deleted: true }.
+    const link = await createOwnedLink();
+
+    const spy = vi.spyOn(LinkRepository, "delete").mockResolvedValueOnce(false);
+    const result = await deleteLink(env as any, link.id, OWNER).finally(() => spy.mockRestore());
+
+    expect(result.ok).toBe(false);
+    if (!result.ok) {
+      expect(result.status).toBe(400);
+    }
+  });
+
+  it("returns 404 when the link is concurrently deleted before the repository delete", async () => {
+    // The link exists at the service's getById and ownership checks, then
+    // vanishes before the repository delete runs. delete() returns false for a
+    // missing row exactly as it does for the click guard, so the service must
+    // re-check existence and surface 404 rather than a misleading 400.
+    const link = await createOwnedLink();
+
+    const spy = vi.spyOn(LinkRepository, "delete").mockImplementationOnce(async (db, id) => {
+      await db.prepare("DELETE FROM links WHERE id = ?").bind(id).run();
+      return false;
+    });
+    const result = await deleteLink(env as any, link.id, OWNER).finally(() => spy.mockRestore());
+
+    expect(result.ok).toBe(false);
+    if (!result.ok) {
+      expect(result.status).toBe(404);
+    }
+  });
+
+  it("link is not in DB after a successful deleteLink", async () => {
+    const link = await createOwnedLink();
+    const result = await deleteLink(env as any, link.id, OWNER);
+    expect(result.ok).toBe(true);
+    expect(await LinkRepository.getById(env.DB, link.id)).toBeNull();
+  });
+
+  it("evicts a custom slug added between the read and the repository delete", async () => {
+    // The link is read, then a custom slug lands before the repository delete
+    // cascades the slug rows. The repository reports the slug set at delete
+    // time, so the service must evict the raced slug from KV. Otherwise the
+    // KV entry (no TTL) keeps the deleted link resolving on redirects.
+    const link = await createOwnedLink();
+    const systemSlug = link.slugs[0].slug;
+    const racedSlug = "raced-slug";
+
+    const realDelete = LinkRepository.delete;
+    const spy = vi.spyOn(LinkRepository, "delete").mockImplementationOnce(async (db, id) => {
+      await addCustomSlugToLink(env as any, id, { slug: racedSlug });
+      return realDelete(db, id);
+    });
+    const result = await deleteLink(env as any, link.id, OWNER).finally(() => spy.mockRestore());
+
+    expect(result.ok).toBe(true);
+    expect(await SlugCache.get(env.SLUG_KV, systemSlug)).toBeNull();
+    expect(await SlugCache.get(env.SLUG_KV, racedSlug)).toBeNull();
+  });
+});
+
+describe("disableLink: null return from repository does not throw", () => {
+  it("returns 404 rather than throwing when the link is concurrently deleted", async () => {
+    // Simulate the narrow race: the link exists at getById time but is
+    // deleted before the UPDATE inside LinkRepository.disable(). Without the
+    // null check the function would crash with a TypeError on disabled!.slugs.
+    const link = await createOwnedLink();
+
+    const spy = vi.spyOn(LinkRepository, "disable").mockResolvedValueOnce(null);
+    const result = await disableLink(env as any, link.id, OWNER).finally(() => spy.mockRestore());
+
+    expect(result.ok).toBe(false);
+    if (!result.ok) {
+      expect(result.status).toBe(404);
+    }
+  });
+});

src/db/link-repository.ts

@@ -137,17 +137,27 @@ export class LinkRepository {
     return LinkRepository.getById(db, id);
   }
 
-  static async delete(db: D1Database, id: number): Promise<boolean> {
+  static async exists(db: D1Database, id: number): Promise<boolean> {
+    const row = await db.prepare("SELECT 1 FROM links WHERE id = ?").bind(id).first();
+    return row !== null;
+  }
+
+  static async delete(db: D1Database, id: number): Promise<string[] | false> {
     // Lifetime guard: a link with any historical clicks (bots, self-referrers,
     // or real users) is preserved so analytics history is not silently dropped.
     const link = await LinkRepository.getById(db, id);
     if (!link) return false;
     if (link.total_clicks > 0) return false;
 
+    // Capture the slug set at delete time. The caller evicts these from KV;
+    // sourcing them here (not from the caller's earlier read) covers a custom
+    // slug added in the window before this delete cascades the slug rows.
+    const slugs = link.slugs.map((s) => s.slug);
+
     await db.prepare("DELETE FROM clicks WHERE slug IN (SELECT slug FROM slugs WHERE link_id = ?)").bind(id).run();
     await db.prepare("DELETE FROM slugs WHERE link_id = ?").bind(id).run();
     await db.prepare("DELETE FROM links WHERE id = ?").bind(id).run();
-    return true;
+    return slugs;
   }
 
   static async search(

src/services/link-management.ts

@@ -153,18 +153,19 @@ export async function disableLink(env: Env, id: number, identity: string): Promi
   if (!link) return fail(404, "Link not found");
   if (link.created_by !== identity) return fail(403, "Only the link owner can disable this link");
   const disabled = await LinkRepository.disable(env.DB, id);
+  if (!disabled) return fail(404, "Link not found");
 
   await Promise.all(
-    disabled!.slugs.map((s) =>
+    disabled.slugs.map((s) =>
       SlugCache.put(env.SLUG_KV, s.slug, {
-        url: disabled!.url,
+        url: disabled.url,
         disabled_at: s.disabled_at,
-        expires_at: disabled!.expires_at,
+        expires_at: disabled.expires_at,
       }),
     ),
   );
 
-  return ok(disabled!);
+  return ok(disabled);
 }
 
 export async function enableLink(env: Env, id: number, identity: string): Promise<ServiceResult<LinkWithSlugs>> {
@@ -194,9 +195,17 @@ export async function deleteLink(env: Env, id: number, identity: string): Promis
   if (link.created_by !== identity) return fail(403, "Only the link owner can delete this link");
   if (link.total_clicks > 0) return fail(400, "Cannot delete a link with clicks, disable it instead");
 
-  const slugsToDelete = link.slugs.map((s) => s.slug);
-  await LinkRepository.delete(env.DB, id);
-  await Promise.all(slugsToDelete.map((s) => SlugCache.delete(env.SLUG_KV, s)));
+  const deletedSlugs = await LinkRepository.delete(env.DB, id);
+  if (!deletedSlugs) {
+    // delete() returns false for two reasons: a concurrent request removed the
+    // link, or a concurrent click pushed total_clicks past the lifetime guard.
+    // A cheap existence check disambiguates: 404 for a vanished link, 400 otherwise.
+    if (!(await LinkRepository.exists(env.DB, id))) return fail(404, "Link not found");
+    return fail(400, "Cannot delete a link with clicks, disable it instead");
+  }
+  // Evict the slug set the repository removed at delete time, not the slugs from
+  // the read above, so a custom slug added in between is not orphaned in KV.
+  await Promise.all(deletedSlugs.map((s) => SlugCache.delete(env.SLUG_KV, s)));
 
   return ok({ deleted: true });
 }