feat(api-key): distinguish expired from expiring-soon + cover service unwrap

Fix 1: an already-expired key rendered the forward-looking "Expiring soon"
badge/tooltip ("expires within 30 days"), which is false for a past expiry.
Add a pure isExpired() helper and, in ApiKeysManager, render a red "Expired"
badge ("This key has expired") for past expiries; keep the orange "Expiring
soon" badge only for keys expiring in the future within 30 days (the two
states are made mutually exclusive at the call site). Extend utils.test.ts
with isExpired coverage (past/future/null/exact-now boundary) and add a
component assertion that an expired key shows "Expired" and NOT "Expiring
soon".

Fix 2: the response-contract unwrap (res.data turning the server envelope
{data:{token,apiKey},success,status} into {token,apiKey}) was exercised by no
test — component tests fully mock the service. Add a service-layer unit test
that mocks the api-client (axios instance) and asserts createApiKey unwraps to
the inner {token,apiKey} payload and getApiKeys resolves to the row array.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

apps/client/src/features/api-key/components/api-keys-manager.test.tsx

@@ -33,6 +33,7 @@ import ApiKeysManager from "./api-keys-manager";
 
 const ISO_SOON = new Date(Date.now() + 10 * 864e5).toISOString();
 const ISO_FAR = new Date(Date.now() + 200 * 864e5).toISOString();
+const ISO_EXPIRED = new Date(Date.now() - 3 * 864e5).toISOString();
 
 // Dump the storage stub via the Web Storage API — its data lives in a closure
 // (see vitest.setup.ts), so JSON.stringify(localStorage) would be vacuous.
@@ -102,6 +103,17 @@ describe("ApiKeysManager — list rendering", () => {
     expect(screen.getAllByText("Expiring soon")).toHaveLength(1);
   });
 
+  it('shows "Expired" (not "Expiring soon") for an already-expired key', async () => {
+    vi.mocked(getApiKeys).mockResolvedValue([
+      makeKey({ id: "k-dead", name: "Dead key", expiresAt: ISO_EXPIRED }),
+    ]);
+    renderManager(UserRole.MEMBER);
+    await screen.findByText("Dead key");
+    // A past expiry is labelled "Expired", never the forward-looking badge.
+    expect(screen.getByText("Expired")).toBeDefined();
+    expect(screen.queryByText("Expiring soon")).toBeNull();
+  });
+
   it('shows "Never" for an unlimited key and no highlight', async () => {
     vi.mocked(getApiKeys).mockResolvedValue([
       makeKey({ id: "k-forever", name: "Forever", expiresAt: null }),

apps/client/src/features/api-key/components/api-keys-manager.tsx

@@ -27,7 +27,11 @@ import {
   IApiKey,
   ICreateApiKeyResponse,
 } from "@/features/api-key/types/api-key.types";
-import { isExpiringSoon, lastUsedBucket } from "@/features/api-key/utils";
+import {
+  isExpired,
+  isExpiringSoon,
+  lastUsedBucket,
+} from "@/features/api-key/utils";
 import { CreateApiKeyModal } from "./create-api-key-modal";
 import { ShowTokenModal } from "./show-token-modal";
 
@@ -120,7 +124,11 @@ export default function ApiKeysManager() {
   }
 
   const rows = (keys ?? []).map((key) => {
-    const soon = isExpiringSoon(key.expiresAt);
+    // Mutually exclusive: an already-expired key is labelled "Expired" (a past
+    // expiry) rather than the forward-looking "Expiring soon". isExpiringSoon
+    // also matches past expiries, so gate "soon" on !expired.
+    const expired = isExpired(key.expiresAt);
+    const soon = !expired && isExpiringSoon(key.expiresAt);
     return (
       <Table.Tr key={key.id}>
         <Table.Td>
@@ -131,6 +139,18 @@ export default function ApiKeysManager() {
           {key.expiresAt ? (
             <Group gap={6} wrap="nowrap">
               <Text size="sm">{formatDate(key.expiresAt)}</Text>
+              {expired && (
+                <Tooltip label={t("This key has expired")} withArrow>
+                  <Badge
+                    color="red"
+                    variant="light"
+                    size="sm"
+                    leftSection={<IconAlertTriangle size={12} />}
+                  >
+                    {t("Expired")}
+                  </Badge>
+                </Tooltip>
+              )}
               {soon && (
                 <Tooltip
                   label={t("This key expires within 30 days")}

apps/client/src/features/api-key/services/api-key-service.test.ts

@@ -0,0 +1,78 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+// Mock the api-client (axios instance). vi.mock replaces the whole module, so
+// the real response interceptor — which returns `response.data`, i.e. the
+// server envelope { data, success, status } — is bypassed. Our mocked post/get
+// therefore resolve to that post-interceptor envelope shape directly, and the
+// service under test reads `.data` off it to unwrap the inner payload. This is
+// the one bug-prone line the component tests (which fully mock the service)
+// never exercise.
+const { post, get } = vi.hoisted(() => ({ post: vi.fn(), get: vi.fn() }));
+vi.mock("@/lib/api-client", () => ({
+  default: { post, get },
+}));
+
+import {
+  createApiKey,
+  getApiKeys,
+} from "@/features/api-key/services/api-key-service";
+
+beforeEach(() => {
+  vi.clearAllMocks();
+});
+
+describe("api-key-service response-contract unwrap", () => {
+  it("createApiKey unwraps the envelope to { token, apiKey }", async () => {
+    const payload = {
+      token: "gm_secret-abc",
+      apiKey: {
+        id: "key-1",
+        name: "CI token",
+        expiresAt: "2027-07-11T12:00:00.000Z",
+        createdAt: "2026-07-11T12:00:00.000Z",
+      },
+    };
+    // The server envelope as the interceptor hands it to the service.
+    post.mockResolvedValue({ data: payload, success: true, status: 200 });
+
+    const result = await createApiKey({
+      name: "CI token",
+      expiresAt: "2027-07-11T12:00:00.000Z",
+    });
+
+    // The inner payload only — not the { data, success, status } wrapper.
+    expect(result).toEqual(payload);
+    expect(result.token).toBe("gm_secret-abc");
+    expect(result.apiKey).toEqual(payload.apiKey);
+    expect(post).toHaveBeenCalledWith("/api-keys/create", {
+      name: "CI token",
+      expiresAt: "2027-07-11T12:00:00.000Z",
+    });
+  });
+
+  it("getApiKeys unwraps the envelope to the array of rows", async () => {
+    const rows = [
+      {
+        id: "key-1",
+        name: "CI token",
+        expiresAt: null,
+        lastUsedAt: null,
+        createdAt: "2026-01-01T00:00:00.000Z",
+      },
+      {
+        id: "key-2",
+        name: "Deploy token",
+        expiresAt: "2027-01-01T00:00:00.000Z",
+        lastUsedAt: null,
+        createdAt: "2026-02-01T00:00:00.000Z",
+      },
+    ];
+    post.mockResolvedValue({ data: rows, success: true, status: 200 });
+
+    const result = await getApiKeys();
+
+    expect(result).toEqual(rows);
+    expect(result).toHaveLength(2);
+    expect(post).toHaveBeenCalledWith("/api-keys/list");
+  });
+});

apps/client/src/features/api-key/utils.test.ts

@@ -2,6 +2,7 @@ import { describe, it, expect } from "vitest";
 import {
   DEFAULT_LIFETIME,
   EXPIRY_WARNING_DAYS,
+  isExpired,
   isExpiringSoon,
   lastUsedBucket,
   lifetimeToExpiresAt,
@@ -50,6 +51,36 @@ describe("isExpiringSoon (acceptance #3 highlight)", () => {
   });
 });
 
+describe("isExpired (past vs future expiry)", () => {
+  it("an unlimited key is never expired", () => {
+    expect(isExpired(null, NOW)).toBe(false);
+  });
+
+  it("a past expiry is expired", () => {
+    expect(isExpired(daysFromNow(-3), NOW)).toBe(true);
+    expect(isExpired(daysFromNow(-1), NOW)).toBe(true);
+  });
+
+  it("a future expiry is not expired (even within the warning window)", () => {
+    expect(isExpired(daysFromNow(1), NOW)).toBe(false);
+    expect(isExpired(daysFromNow(EXPIRY_WARNING_DAYS - 1), NOW)).toBe(false);
+    expect(isExpired(daysFromNow(200), NOW)).toBe(false);
+  });
+
+  it("an expiry exactly at 'now' counts as expired (boundary is inclusive)", () => {
+    expect(isExpired(NOW.toISOString(), NOW)).toBe(true);
+  });
+
+  it("is mutually distinguishable from isExpiringSoon: expired vs soon-but-future", () => {
+    // A key 3 days in the past: expired, and (by design) also matches
+    // isExpiringSoon — the UI resolves this by checking isExpired first.
+    expect(isExpired(daysFromNow(-3), NOW)).toBe(true);
+    // A key 10 days in the future: NOT expired, but expiring soon.
+    expect(isExpired(daysFromNow(10), NOW)).toBe(false);
+    expect(isExpiringSoon(daysFromNow(10), NOW)).toBe(true);
+  });
+});
+
 describe("lastUsedBucket (within-the-last-hour semantics)", () => {
   const minutesAgo = (n: number) =>
     new Date(NOW.getTime() - n * 60 * 1000).toISOString();

apps/client/src/features/api-key/utils.ts

@@ -32,8 +32,24 @@ export function lifetimeToExpiresAt(
   }
 }
 
+// True when a bounded key's expiry is already in the past (or exactly now). An
+// unlimited key (null) is never expired. This is distinct from "expiring soon":
+// the two states are mutually exclusive at the call site (see api-keys-manager),
+// so an already-expired key is labelled "Expired", not "Expiring soon".
+export function isExpired(
+  expiresAt: string | null,
+  now: Date = new Date(),
+): boolean {
+  if (!expiresAt) return false;
+  const expiry = new Date(expiresAt).getTime();
+  if (Number.isNaN(expiry)) return false;
+  return expiry <= now.getTime();
+}
+
 // True when a bounded key expires within the warning window (or is already
-// expired). An unlimited key (null) is never "expiring soon".
+// expired). An unlimited key (null) is never "expiring soon". Callers that need
+// to distinguish an already-past expiry should check isExpired() first, as this
+// predicate deliberately also covers the already-expired case.
 export function isExpiringSoon(
   expiresAt: string | null,
   now: Date = new Date(),