fix(tests): address Task 4 code review feedback

- Deprecate mockPayloadModule with throw guard (hoisting incompatible)
- Replace `as never` with stubPayloadConfig in payload-articles test (consistency)
- Tighten pages contract to use toHaveLength (exact assertions)
- Header contract: define CONTRACT_HEADER_SEED, assert item count + order

Reviewer: superpowers:code-reviewer (Task 4 of Plan 7).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

packages/blog/src/infrastructure/repositories/payload-articles.repository.test.ts

@@ -109,8 +109,6 @@ describe("PayloadArticlesRepository", () => {
   // -------------------------------------------------------------------------
 
   describe("Payload doc → domain mapping", () => {
-    const mockConfig = {} as never;
-
     beforeEach(() => {
       vi.clearAllMocks();
     });
@@ -135,7 +133,7 @@ describe("PayloadArticlesRepository", () => {
         find: findMock,
       });
 
-      const repo = new PayloadArticlesRepository(mockConfig);
+      const repo = new PayloadArticlesRepository(stubPayloadConfig);
       const result = await repo.getArticleBySlug("hello");
 
       expect(findMock).toHaveBeenCalledWith({
@@ -157,7 +155,7 @@ describe("PayloadArticlesRepository", () => {
         find: vi.fn().mockResolvedValue({ docs: [] }),
       });
 
-      const repo = new PayloadArticlesRepository(mockConfig);
+      const repo = new PayloadArticlesRepository(stubPayloadConfig);
       const result = await repo.getArticleBySlug("missing");
       expect(result).toBeUndefined();
     });

packages/core-testing/src/payload/mock-payload-module.ts

@@ -1,10 +1,27 @@
-import { vi } from "vitest";
 import type { Payload } from "payload";
 
-// Helper to mock the `payload` package with a custom getPayload impl.
-// Call inside a test file BEFORE importing the SUT.
+/**
+ * @deprecated DO NOT USE — hoisting incompatible.
+ *
+ * Vitest statically hoists `vi.mock()` calls to the top of the file.
+ * This helper wraps `vi.mock()` inside a regular function body, so the
+ * hoist never fires — the mock is installed too late and the real
+ * `payload` module loads before being intercepted.
+ *
+ * Instead, write at the TOP LEVEL of your test file:
+ *
+ *     vi.mock("payload", () => ({ getPayload: vi.fn() }));
+ *
+ *     // then in your test:
+ *     const { getPayload } = await import("payload");
+ *     (getPayload as ReturnType<typeof vi.fn>).mockResolvedValue(yourStub);
+ *
+ * This export is preserved only so existing imports don't break and
+ * agents searching for it find this warning.
+ */
 export function mockPayloadModule(impl: Partial<Payload>): void {
-  vi.mock("payload", () => ({
-    getPayload: vi.fn().mockResolvedValue(impl),
-  }));
+  void impl;
+  throw new Error(
+    "mockPayloadModule is deprecated and broken; see JSDoc. Call vi.mock('payload', ...) at the top of your test file instead.",
+  );
 }

packages/marketing-pages/src/__contracts__/pages-repository.contract.ts

@@ -72,7 +72,7 @@ export const pagesRepositoryContract =
 
       it("getPages with no filter returns all seeded pages", async () => {
         const list = await repo.getPages();
-        expect(list.length).toBeGreaterThanOrEqual(2);
+        expect(list).toHaveLength(2);
         for (const page of list) {
           expect(page.id).toBeDefined();
           expect(page.slug).toBeDefined();
@@ -82,7 +82,7 @@ export const pagesRepositoryContract =
 
       it("getPages({ status: 'published' }) returns only published pages", async () => {
         const published = await repo.getPages({ status: "published" });
-        expect(published.length).toBeGreaterThanOrEqual(1);
+        expect(published).toHaveLength(1);
         for (const page of published) {
           expect(page.status).toBe("published");
         }
@@ -90,7 +90,7 @@ export const pagesRepositoryContract =
 
       it("getPages({ status: 'draft' }) returns only draft pages", async () => {
         const drafts = await repo.getPages({ status: "draft" });
-        expect(drafts.length).toBeGreaterThanOrEqual(1);
+        expect(drafts).toHaveLength(1);
         for (const page of drafts) {
           expect(page.status).toBe("draft");
         }

packages/navigation/src/__contracts__/header-repository.contract.ts

@@ -1,12 +1,26 @@
 import { it, expect, beforeEach } from "vitest";
 import { defineContractSuite } from "@repo/core-testing/contract";
 import type { IHeaderRepository } from "../application/repositories/header-repository.interface.js";
+import type { Header } from "../entities/header.js";
+
+/**
+ * Known fixtures that every implementation's `buildSubject` must pre-seed.
+ * Exported so that test files can pass them to `MockHeaderRepository` or the
+ * Payload stub without duplicating definitions.
+ */
+export const CONTRACT_HEADER_SEED: Header = {
+  items: [
+    { label: "Home", href: "/", external: false },
+    { label: "Blog", href: "/blog", external: false },
+    { label: "Docs", href: "/docs", external: true },
+  ],
+};
 
 /**
  * Contract for IHeaderRepository.
  *
  * Header is a singleton (Payload Global). The interface exposes only
- * getHeader(). The contract verifies the shape of the return value.
+ * getHeader(). The contract verifies the shape, count, and order of items.
  */
 export const headerRepositoryContract =
   defineContractSuite<IHeaderRepository>(
@@ -20,10 +34,24 @@ export const headerRepositoryContract =
 
       // --- getHeader ---
 
-      it("getHeader returns an object with an items array", async () => {
+      it("getHeader returns an object with an items array of the seeded length", async () => {
         const header = await repo.getHeader();
         expect(header).toBeDefined();
         expect(header.items).toBeInstanceOf(Array);
+        expect(header.items).toHaveLength(3);
+      });
+
+      it("getHeader items appear in the seeded order with correct shape", async () => {
+        const header = await repo.getHeader();
+        expect(header.items[0]?.label).toBe("Home");
+        expect(header.items[0]?.href).toBe("/");
+        expect(header.items[0]?.external).toBe(false);
+        expect(header.items[1]?.label).toBe("Blog");
+        expect(header.items[1]?.href).toBe("/blog");
+        expect(header.items[1]?.external).toBe(false);
+        expect(header.items[2]?.label).toBe("Docs");
+        expect(header.items[2]?.href).toBe("/docs");
+        expect(header.items[2]?.external).toBe(true);
       });
 
       it("getHeader items have label, href, and external fields", async () => {

packages/navigation/src/infrastructure/repositories/mock-header.repository.test.ts

@@ -1,7 +1,7 @@
 import { describe } from "vitest";
 import { MockHeaderRepository } from "@/infrastructure/repositories/mock-header.repository";
-import { headerRepositoryContract } from "@/__contracts__/header-repository.contract";
+import { headerRepositoryContract, CONTRACT_HEADER_SEED } from "@/__contracts__/header-repository.contract";
 
 describe("MockHeaderRepository", () => {
-  headerRepositoryContract.run(() => new MockHeaderRepository());
+  headerRepositoryContract.run(() => new MockHeaderRepository(CONTRACT_HEADER_SEED));
 });

packages/navigation/src/infrastructure/repositories/mock-header.repository.ts

@@ -2,17 +2,23 @@ import "reflect-metadata";
 import { injectable } from "inversify";
 
 import type { IHeaderRepository } from "../../application/repositories/header-repository.interface";
-import type { Header } from "../../entities/header";
+import type { Header, HeaderItem } from "../../entities/header";
+
+const DEFAULT_ITEMS: HeaderItem[] = [
+  { label: "Home", href: "/", external: false },
+  { label: "Blog", href: "/blog", external: false },
+  { label: "Docs", href: "/docs", external: true },
+];
 
 @injectable()
 export class MockHeaderRepository implements IHeaderRepository {
+  private readonly data: Header;
+
+  constructor(initialData?: Header) {
+    this.data = initialData ?? { items: DEFAULT_ITEMS };
+  }
+
   async getHeader(): Promise<Header> {
-    return {
-      items: [
-        { label: "Home", href: "/", external: false },
-        { label: "Blog", href: "/blog", external: false },
-        { label: "About", href: "/about", external: false },
-      ],
-    };
+    return this.data;
   }
 }

packages/navigation/src/infrastructure/repositories/payload-header.repository.test.ts

@@ -1,6 +1,6 @@
 import { describe, vi } from "vitest";
 import { PayloadHeaderRepository } from "@/infrastructure/repositories/payload-header.repository";
-import { headerRepositoryContract } from "@/__contracts__/header-repository.contract";
+import { headerRepositoryContract, CONTRACT_HEADER_SEED } from "@/__contracts__/header-repository.contract";
 import { stubPayloadConfig } from "@repo/core-testing/payload/stub-config";
 
 // ---------------------------------------------------------------------------
@@ -8,18 +8,9 @@ import { stubPayloadConfig } from "@repo/core-testing/payload/stub-config";
 // ---------------------------------------------------------------------------
 
 function buildHeaderStub() {
-  const globalData = {
-    logo: null,
-    items: [
-      { label: "Home", href: "/", external: false },
-      { label: "Blog", href: "/blog", external: false },
-      { label: "About", href: "/about", external: false },
-    ],
-  };
-
   return {
     findGlobal: vi.fn(async () => {
-      return { ...globalData };
+      return { ...CONTRACT_HEADER_SEED };
     }),
   };
 }