feat(project-engine-client): stateful mock core — store, resource ops, seeds (LLMO-5460)

[aliciadriani]

What

The stateful Project Engine Counterfact mock — LLMO-5460 (epic LLMO-5459). A generic in-memory store + the confirmed stateful resource ops, the live Counterfact runner that wires them into per-path handlers, named seeds, typed mock factories, AI-unit quota metering, bearer-auth enforcement, a test-only control surface, and an in-package E2E that drives the real client against a booted mock.

Now based directly on main (the earlier stack #1660 → #1661 has merged).

Spike outcome (confirmed against both consumers)

Grepped the real consumers to set the AC floor (docs/mock-statefulness.md):

• spacecat-api-service is the only Project Engine consumer; llmo-data-retrieval-service makes zero calls.
• The write-then-read spine is projects, ai_models (per project), prompts (aio, per project). The prompt slice was reconciled against the consumer's actual call sites (rest-transport.js) and the spec: create is POST .../aio/prompts/tagged, list is POST .../aio/prompts/by_tags, delete is DELETE .../aio/prompts.

Contents

Mock library (mock/)

• store.js — generic InMemoryStore: collection-keyed CRUD + seed/reset, deep-clone on every read/write (a loaded seed is never mutated; reset() restores a pristine copy). Throws on explicit id collision.
• stateful.js — the confirmed stateful set as pure ops over the store (per-workspace/per-project scoping). No Counterfact/HTTP coupling, so unit-tested directly.
• factories.js — the typed mock-factory layer: createProjectMock, createProjectAiModelMock, createAiModelMock, createPromptMock, createBenchmarkMock, createBrandUrlMock, createLanguageMock, createTagNodeMock, createBrandTopicMock — each (Partial<T>) => T typed against the generated schemas. The single tsc-checked source of truth for every entity shape the mock emits.
• seeds.js — named seed sets (empty-workspace, workspace-with-data), DEFAULT_SEED, SEED_IDS, all built through the factories.
• quota.js — AI-unit quota meter (the disguised-405 the live API returns on over-allocation), enforced on project create / prompt write / publish; configurable via buildSeed({ quota }) or the POST /__quota control route.
• auth.js — bearer-auth model: every real route requires Authorization: Bearer <token> (presence, not validity) or returns 401 { detail: 'Not authenticated' }, exactly like the live gateway; the __* control routes are exempt.
• context.js — the Counterfact per-path Context: wires the pure store + ops + factories + quota + auth to a seed selected at startup and exposes reset().
• run.js — the live runner. Materializes the committed handlers into a gitignored .counterfact/ tree (as .ts), injecting the bearer-auth guard onto every handler at the single materialization seam (injectAuthGuard, so no handler can forget it), and launches Counterfact with --serve against the corrected OAS3 artifact (build/openapi3.json) under --prefix /enterprise/projects/api. npm run generate is required before first boot. npm run mock.

Counterfact route handlers (mock/counterfact/routes/)

• Stateful spine: v1/workspaces/{id}/projects (GET/POST), .../{project_id} (GET/PATCH/DELETE), .../ai_models (GET/POST), v2/.../aio/prompts/tagged (POST create), .../by_tags (POST list), .../aio/prompts (DELETE).
• Project surface: v1/v2 ai_models + ai_models/benchmarks, aio/benchmarks/{benchmark_id}/brand_urls, aio/tags, brand-topics, ci/competitors, publish, aio/init_status.
• Global catalogs: v1/ai_models and v1/languages return the full live taxonomy (11 global models, 38 languages — real ids/keys/icons/codes, captured verbatim) mapped through the factories.
• Control routes (__*, auth-exempt): __reset (seed restore), __seed (load a named seed), __quota (set quota), __dump (inspect store state).

Tests, docs, CI

• test/mock/*.test.js — unit tests for store, stateful ops, factories, seeds, context, quota, and auth (100% coverage; counterfact/routes/** + run.js excluded).
• test/e2e/project-engine-mock.e2e.js — boots the live mock and drives it through the real client; gated behind MOCK_E2E=1, outside the default npm test glob. Exercises the prompt write-then-read spine (tagged → by_tags), the quota 405, and the bearer-auth 401, with response validation on.
• test/types/*.type-test.ts — compile-only guards: the public type surface stays assignable to Client<paths>, and the factories stay schema-faithful (workspace_id drift + wrong-field-type are @ts-expect-error canaries).
• docs/mock-usage.md — usage manual for humans + agents (quick start, bearer auth, full endpoint inventory, seeds, control routes, quota, driving from tests, internals, troubleshooting).
• spec/overlays/corrections.yaml — spec corrections (the vendored swagger is never edited); build/openapi3.json is regenerated from it.
• CI — a path-gated E2E (project-engine mock) job (type-surface check + live E2E), scoped via a merge-base diff so it only runs when this package changes.

Validation

• npm test — 169 passing, 100% statements / branches / functions / lines
• npm run lint — clean
• npm run test:types — clean
• npm run test:e2e — 33 passing against the live mock (response validation on)

Follow-up (separate PR)

Adopting the mock into spacecat-api-service's own E2E/local-dev (running its Project Engine calls against this mock) lands with the adoption PR — out of scope here, which delivers the mock and its self-contained E2E. The pre-existing consumer bug getInitStatus hits the v1 aio/init_status path (404s live; v2 is correct) and is tracked for that adoption PR.

🤖 Generated with Claude Code

[github-actions]

This PR will trigger a minor release when merged.

[aliciadriani]

Type-surface finding (surfaced by the new CI guard)

The Type surface check (npm run test:types, added in this PR) caught a real contract/implementation mismatch within minutes of landing — exactly the argument for guarding the type surface rather than deferring it.

The mismatch:

• The generated paths mark Auth-Data-Jwt as a required per-operation header — faithful to the Semrush API contract.
• But the client owns that header via middleware and sets it with headers.set(...), so any value a consumer passes is silently overwritten.

Why it matters (footgun, not cosmetics): the client's public type (Client<paths>) forces every call site in llmo + api-service to pass an Auth-Data-Jwt token that does nothing. It looks load-bearing, so someone will eventually pass a "real" token there and lose time discovering it's ignored. The E2E suite never caught this (plain JS, type-blind); the type-test did.

Options:

1. (leaning) Narrow the client's exported type — a mapped type over paths that drops the injected Auth-Data-Jwt from each operation's params.header, so the public surface reflects that the client supplies it.
2. Accept it — rejected: keeps a recurring footgun in every consumer.

Hard constraint (option 1): do the Omit at the client's exported type only. Do not touch the vendored spec, src/generated/types.ts, or the Pydantic models — those are the honest API contract and the spec-is-the-contract gate must hold.

Parked for the #4 adoption (where real call sites make the cost and the exact shape-to-omit concrete; the transform is a non-trivial recursive mapped type). The client type itself lives in #1661. The type-test fixture moves in lockstep — whatever's decided, test/types/index.type-test.ts updates with it (drop the header: { 'Auth-Data-Jwt': ... } line from the typed-call assertion when the surface is narrowed), or the guard will just re-flag the old surface.

Tracked as an LLMO-5461 sub-task linked to the #4 adoption work.

[aliciadriani]

Inline comments below carry the proposed fix for each finding from the review.

Process — Should Fix (not inline): the PR description is stale relative to the branch. The body says step 5 (Counterfact runner) and step 8 (E2E + CI wiring) are "Deferred to a follow-up PR," but this branch already contains run.js, the route handlers, __reset.js, the full E2E suite, and the new E2E (project-engine mock) CI job. The "Contents" list also omits context.js, run.js, the handlers, and the type-surface test. Please refresh the description to match the 24-file / +1579 diff so reviewers can size scope.

Fix: update the PR body — move steps 5 & 8 from "Deferred" to "Contents," and add context.js, run.js, the counterfact handlers, __reset.js, the E2E suite, and test/types/index.type-test.ts to the contents list.

[aliciadriani]

Should Fix — this create handler is mounted on a path the spec and the consumer don't use.

The spec defines only delete on /aio/prompts (aio-delete-prompt-by-ids-v2) — there is no POST here. The real create is POST /aio/prompts/tagged (aio-create-prompts-with-tags) and the real read is POST /aio/prompts/by_tags (aio-list-prompts-by-tag-ids) — exactly what docs/mock-statefulness.md lists as the consumer's stateful prompt ops.

Consequences as written:

• The E2E creates aio prompts (v2) case drives a non-spec POST, so there's no 200 response schema for Counterfact to validate the { id, name } envelope against — the "response validation stays on" guarantee doesn't cover this handler.
• When spacecat-api-service runs against the mock, prompt create (tagged) and read (by_tags) hit Counterfact's stateless stubs, so the documented prompt write-then-read spine isn't actually stateful.

Fix: keep DELETE in this file, move create into a new aio/prompts/tagged.js, and add a aio/prompts/by_tags.js read handler (the store + ops already support both via createMany / list). Repoint the E2E case at /v2/.../aio/prompts/tagged.

// aio/prompts/tagged.js — POST create (operationId aio-create-prompts-with-tags)
export function POST($) {
  const { path, body, context } = $;
  const created = context.ops.prompts.createMany(
    { workspaceId: path.id, projectId: path.project_id },
    (body?.items ?? []).map((text) => ({ name: text })),
  );
  const first = created[0] ?? {};
  return $.response[200].json({ id: first.id, name: first.name });
}

// aio/prompts/by_tags.js — POST list/read (operationId aio-list-prompts-by-tag-ids)
// shape the envelope to match the spec's 200 response for this op
export function POST($) {
  const { path, context } = $;
  const items = context.ops.prompts.list({ workspaceId: path.id, projectId: path.project_id });
  return $.response[200].json({ items, page: 1, total: items.length });
}

If this is intentionally deferred to the adoption PR, please scope it out in the description and drop the POST /aio/prompts row from the README's stateful-endpoints table, which currently advertises it as "create prompts."

[aliciadriani]

Resolved in cbe35f3 — the bogus POST /aio/prompts handler was removed and the correct paths were wired:

• aio/prompts/tagged.js → POST create (201, IDsWithStatsResponse)
• aio/prompts/by_tags.js → POST list (200, AIOPromptsWithStatusListResponse)
• aio/prompts.js retains only DELETE → 204

The E2E now drives tagged → by_tags with a real write-then-read assertion and response validation on, closing the false-coverage gap.

[aliciadriani]

Nit — seed/created prompt field mismatch. Seed prompts use text, but prompts.createMany stores { name } and the create handler returns { id, name }. If a by_tags read handler is added (see the prompts.js comment), seeded vs. created prompts will have inconsistent shapes. Align the seed to name:

Suggested change

	{ id: 'prompt-1', text: 'What is the best running shoe?' },
	{ id: 'prompt-1', name: 'What is the best running shoe?' },

[aliciadriani]

Resolved in cbe35f3 — seed prompts now use name (+ tags: []) matching the AIOPromptWithStatus shape, consistent with what createMany stores and by_tags returns.

[aliciadriani]

Nit (optional, no change required). create with an explicit existing id silently overwrites via Map.set (a real API would 409). Fine for a mock; flagging only because seeds use fixed ids, so a seed-id collision would be swallowed.

If you want collisions to surface during dev, the minimal guard is:

if (entity.id && this.#collection(name).has(entity.id)) {
  throw new Error(`duplicate id ${entity.id} in collection ${name}`);
}

Otherwise leave as-is — overwrite-on-duplicate is acceptable mock behaviour.

[aliciadriani]

Added in 1f5c8cf — the guard throws on an explicit id collision:

if (entity.id && this.#collection(name).has(entity.id)) {
  throw new Error(`duplicate id ${entity.id} in collection ${name}`);
}

Seed-id collisions now surface immediately instead of being silently swallowed.

[aliciadriani]

Nit (no change required). This POST returns 200 while the projects create returns 201. CI response-validation is on and green, so 200 is what the spec declares for add ai_model — the inconsistency is spec-driven, not a bug. Noting only for awareness.

[aliciadriani]

Acknowledged — spec-driven inconsistency, no action needed.

[aliciadriani]

Nit (no change required). prompts.removeMany (and ai_models.removeMany) compute a removed count that this handler discards — it always returns 204. The spec DELETE is 204, so this is correct; the count is exercised at the ops layer in stateful.test.js. Flagging only so the unused return value reads as deliberate.

[aliciadriani]

Acknowledged — the count is exercised at the ops layer; the handler deliberately discards it and always returns 204 per spec.

[aliciadriani]

Resolved — prompts stateful slice reconciled against the consumer and the spec (commit 4c510c1).

Follow-up to Should Fix #1 (the POST /aio/prompts handler). The prompts statefulness was reconciled against the actual consumer call sites (spacecat-api-service — src/support/serenity/rest-transport.js) and the spec, rather than the spike's guessed default, which had landed on a path the spec defines as delete-only.

What was wrong. The stateful create handler was mounted on POST /v2/.../aio/prompts, incorrect three ways:

• Path — the spec defines only delete on /aio/prompts (aio-delete-prompt-by-ids-v2); there is no POST operation there.
• Body — it read { items: [text] }; the real request is AIOTaggedPromptsCreateRequest → { prompts: { [tagName]: [text, ...] } }.
• Status — it returned 200; the spec's create returns 201.

Because the spec declares no 200 response for that path, the { id, name } envelope was never response-validated — false coverage that looked like the floor was met.

The actual consumer flow (rest-transport.js):

• create — POST /aio/prompts/tagged (aio-create-prompts-with-tags), body { prompts: { [tag]: [text] } }
• list / read — POST /aio/prompts/by_tags (aio-list-prompts-by-tag-ids), body { tag_ids, page, limit, ... }
• delete — DELETE /aio/prompts, body { ids }

handleListPrompts writes via tagged then reads via by_tags, depending on items[].{ id, name, tags } — a genuine write-then-read spine, so prompts does belong in the stateful set.

Fix (option a — wire the correct handlers, repoint the E2E):

• Removed the bogus POST from aio/prompts.js; kept DELETE → 204 (the only op the spec defines on that path).
• Added aio/prompts/tagged.js → 201 IDsWithStatsResponse { ids, existing_count }, wired to the in-memory store; created prompts carry a synthesized AIOTag so by_tags filtering is meaningful.
• Added aio/prompts/by_tags.js → 200 AIOPromptsWithStatusListResponse { items, page, total, unassigned }; empty tag_ids lists all, otherwise OR-filters by tag id.
• Realigned the workspace-with-data prompt seed to the real AIOPromptWithStatus shape (name + tags, replacing the stale text field) — also resolves the seed/created field mismatch (review nit chore: initial setup #2).
• Repointed the E2E to drive tagged → by_tags with a real write-then-read assertion, plus a delete-reflects-in-list case.
• Updated the README stateful-endpoints table.

Validation. The E2E now exercises the real spec operations with response validation on, so the 201/200 envelopes are confirmed spec-valid — the false-coverage gap is closed. All gates green: npm test (51 passing, 100% statements / branches / functions / lines), npm run lint, npm run test:types, npm run test:e2e (8 passing against the live mock).

Spec quirk worth flagging. On tagged, the workspace id is declared as a query parameter, though it is a path segment everywhere else and in the consumer's URL builder (aioPromptsPath). The handler reads $.path.id to match how the consumer actually calls it, and the E2E confirms it end-to-end. Noted because the generated types may model that parameter oddly.

[rainer-friederich]

Heads-up for the rebase onto #1661 (now that it carries the generation-time overlay spec/overlays/corrections.yaml → build/openapi3.json): src/mock/run.js needs two coupled changes, otherwise the mock will not reflect the corrected spec.

1. GET /v1/ai_models will 404 in the mock. run.js feeds Counterfact the pristine vendored swagger (SPEC = spec/projectengine_swagger_public.yaml), which by design does not contain /v1/ai_models — that path exists only in the overlaid build/openapi3.json. So "GET /v1/ai_models stays on Counterfacts auto-generated response" only holds if Counterfact is fed the overlaid artifact. → Point SPEC at build/openapi3.json, and ensure npm run spec:convert && npm run spec:overlay (i.e. npm run generate) has run before the mock boots (build/ is gitignored).

2. The route prefix breaks when you switch specs. Counterfact honors a Swagger-2.0 basePath as a serving prefix, but does not honor an OAS3 servers[0].url (verified empirically). The current raw-swagger setup gets /enterprise/projects/api for free from basePath; the converted/overlaid build/openapi3.json carries it only as servers[0].url, which Counterfact ignores → the real client (which appends /enterprise/projects/api) would 404. → Add --prefix /enterprise/projects/api to the Counterfact args. It applies to both the committed stateful handlers and the spec-derived routes, so the whole surface stays under the /enterprise/projects/api base path.

--no-validate-request already makes the Auth-Data-Jwt removal a no-op for the mock, so nothing is needed there.

Net diff to run.js:

const SPEC = join(packageRoot, 'build', 'openapi3.json'); // was spec/projectengine_swagger_public.yaml
// ...ensure build/openapi3.json exists first (npm run generate)...
[findCounterfactBin(), SPEC, BASE_PATH, '--port', String(PORT), '--serve',
 '--prefix', '/enterprise/projects/api', '--no-validate-request', '--no-update-check']

Alternative (bigger change): keep feeding Swagger 2.0 so basePath keeps prefixing (no --prefix needed), but apply the overlay to a Swagger-2.0 artifact so /v1/ai_models is present there. Since the overlay currently targets OAS3 post-conversion, feeding the overlaid build/openapi3.json + --prefix is the smaller change.

[aliciadriani]

Rebase + review round resolved.

Rebase onto current `main` — the branch is now stacked cleanly on top of the squash-merged #1661 (`0465ab0`). Conflicts were mechanical (all from the foundation files now in `main`); resolved by keeping the overlay-inclusive `generate` script from `main` and the mock-runner additions from this branch.

1. `SPEC` now points at `build/openapi3.json` (the overlay-corrected OAS3 artifact). Counterfact now sees CR1 (`GET /v1/ai_models`) and auto-generates a stub for it.
2. `--prefix /enterprise/projects/api` added — Counterfact ignores OAS3 `servers[0].url` so the prefix must be passed explicitly; all routes (stateful + auto-generated) now serve under the correct base path.
3. Fast-fail guard in `materialize()`: exits with a clear message if `build/openapi3.json` is absent (`run `npm run generate` first`).
4. README Pipeline diagram and Mock section updated to reflect the mock now reads the corrected OAS3 artifact.

Type-surface follow-up (same commit `0ccd959`): `test/types/index.type-test.ts` dropped the `Auth-Data-Jwt` header from the typed call — the overlay (CR2) already removes it from every operation in the generated surface, so passing it was a type error against the current `main` types.

Inline nits (all in `cbe35f3` or `1f5c8cf`):

• Prompts Should Fix: resolved (`cbe35f3`).
• Seeds field mismatch: resolved (`cbe35f3`).
• Store duplicate-id guard: added (`1f5c8cf`).
• `ai_models` 200 vs 201 / DELETE unused count: acknowledged, no change needed.

Process fix: PR description updated below to reflect the full 24-file / +1579 diff.

Gates: `npm test` 98 passing 100% coverage · `npm run lint` clean · `npm run test:types` clean.

[rainer-friederich]

Hey @aliciadriani,

Verdict: Request changes - solid, well-layered mock with a clean published-surface boundary and a faithful auth model; the blocking items are all small (a cross-cutting auth-guard hardening, two real test gaps, and a self-consistency fix), none are hard bugs.
Changes: adds a stateful Counterfact mock of the Semrush Project Engine API to project-engine-client (in-memory store, resource ops, typed factories, seeds, AI-unit quota, bearer auth, a live runner + route handlers, an in-package E2E), plus published-client @ts-check/type regen and a generated python model package. (58 files).

Must fix before merge

1. [Important] Auth-guard injection fails open on handler-style drift - mock/run.js:111 (injectAuthGuard). The bearer guard is injected by regex-matching export [async] function GET|POST|PUT|PATCH|DELETE($) {. Every committed handler matches (all 30 verified, so no live bug), but a future handler in another style (export const GET = ($) =>, destructured params, a new OPTIONS/HEAD) gets no guard and serves unauthenticated silently - and run.js is coverage-excluded, exercised only by the gated E2E. Fix: assert at materialization that every real-route method received the guard (throw on miss), or add a unit test enumerating counterfact/routes/**.

2. [Important] Child-resource writes skip parent-project existence - mock/counterfact/routes/v2/.../ai_models.js:25-34 (and the v1 sibling / prompts / benchmarks). POST .../ai_models calls ops.ai_models.add with no check the project exists, returning 201 where the live API 404s. A consumer flow that adds a model to a deleted project passes the mock but fails prod - the exact bug class the mock exists to catch. Fix: a shared requireProject(scope) guard the child writers run first, or document the gap in mock-statefulness.md.

3. [Important] by_tags tag-filter branch untested even by the E2E - mock/counterfact/routes/v2/.../aio/prompts/by_tags.js:29-31. The E2E only calls by_tags with tag_ids: [] (the list-all branch); the filter branch - the consumer's actual "list by tags" read path, with OR semantics - is never executed, and handlers are coverage-excluded so nothing flags it. Fix: one test creating tagged prompts under two tags and asserting a non-empty tag_ids returns only the matching subset.

4. [Important] Published src/internal.js retry change shipped without a test - src/internal.js:153. result && typeof result.catch === 'function' became result instanceof Promise, a behavior tightening (a non-Promise thenable's rejection now escapes the swallow). The existing test stays green because a native async fn returns a real Promise, so the change is unpinned. It is defensible (out-of-contract input) but it ships in published code. Fix: add an assertion pinning the chosen behavior, or revert to the duck-typed thenable check.

5. [Important] Factory-pattern doc/code inconsistency - init_status.js:22 ({ initialized: true }), publish.js:32 ({ message: 'publish accepted' }), brand_urls.js DELETE. These emit inline response literals, but the package CLAUDE.md states the lone exception to the factory rule is ci/competitors. Fix: add createBasicResponseMock/createInitStatusMock and route through them, or amend the CLAUDE.md exception list to name the scalar wrappers.

Non-blocking (13): minor issues and suggestions

• suggestion: CI workflow has no timeout-minutes (project-engine-mock-e2e.yaml, job runs-on at line 37) - a wedged boot inherits GitHub's 6h default; add timeout-minutes: 15.
• suggestion: E2E spawns the server with stdio: 'ignore', so a boot/transpile failure surfaces only as the opaque "mock did not become ready in time"; capture child stderr and include the tail in the readiness-timeout error.
• nit: .nycrc.json excludes run.js + counterfact/routes/** (justified) - add a one-line comment pointing at the E2E as the compensating control.
• suggestion: E2E teardown does Promise.race([exited, sleep(5000)]) but never escalates to SIGKILL if SIGTERM is ignored.
• suggestion: LIB_FILES in run.js is a hand-maintained list; it could self-validate by parsing context.js's sibling imports (fails loudly at boot today, but far from the edit).
• nit: ci/competitors is the one handler still using an inline literal (CICompetitor has no factory) - add createCiCompetitorMock to close the single-source-of-truth loop.
• nit: package CLAUDE.md says globalThis.crypto is typed via lib dom, but tsconfig.json uses "lib": ["esnext"]; the type comes from @types/node.
• suggestion: store deep-clone isolation has no test mutating a create()/get()/list() return and re-reading (the "mutate a returned entity must not corrupt the store" case); the clone path is covered, so this is a hardening gap.
• nit: __quota with a missing workspaceId calls quota.set(undefined, ...) and returns { id: undefined }; a one-line 400 guard would make the control route self-explaining.
• nit: E2E baseUrl uses http://localhost:${port} while pickPort binds on 127.0.0.1; pin both to 127.0.0.1 to drop a DNS variable.
• suggestion: auth.js - add a whitespace-only-token canary test ('Bearer ') and a comment that .trim() is what makes a trailing-whitespace token fail, locking the .trim()+\S contract.
• suggestion: the E2E readiness probe could hit the auth-exempt /__dump instead of a seeded route, removing the Bearer readiness-probe workaround and the seed dependency.
• nit: python/serenity_project_engine/http_server.py is an inert datamodel-codegen pydantic model (no server) - the filename misleads.

Out of scope, worth tracking

• The api-service consumer getInitStatus calls the dead /v1 init_status path (degrades to initialized: null); the mock faithfully reproduces the live 404 (overlay CR8). The real fix is a spacecat-api-service change - worth a tracking ticket so this PR's CR8 note isn't the only record.
• The E2E proves mock-vs-spec, not consumer-wiring end-to-end; that lands with the adoption PR.

---

No Critical issues, and the architecture/layering, published-surface boundary (files: ["src"], counterfact is a devDependency), supply chain, and bearer-auth model all reviewed clean (the auth model was independently confirmed faithful). The Request-changes state is driven entirely by the 5 Important items above, all fixable on-branch.

[rainer-friederich]

Addressed the local review (the MysticatBot-equivalent pass — this PR's 4900-LOC diff is over the bot's limit) in 4bd7c51b. Gates after: types, lint, unit 154 @ 100%, e2e 30.

Must fix before merge (all 5 addressed)

• [Important] Auth-guard fails open on handler-style drift - done. injectAuthGuard (mock/run.js) now counts every exported HTTP method and throws at materialization if any didn't receive the guard, so an unmatched handler shape can't silently ship unauthenticated. Verified: e2e still boots (all current handlers match).
• [Important] Child-resource writes skip parent-project existence - documented as a deliberate fidelity simplification in docs/mock-statefulness.md. Quota meters at workspace granularity and the quota e2e intentionally writes under never-created project ids; a requireProject guard would break that and isn't needed by any consumer flow. The doc records how to add it if that changes.
• [Important] by_tags filter branch untested - done. New e2e case creates prompts under two tags and asserts a non-empty tag_ids returns only the matching subset (the consumer's real read path).
• [Important] src/internal.js retry tightening shipped without a test - done. Added a test pinning the instanceof Promise guard: a non-Promise thenable return is left untouched (its .catch is never invoked).
• [Important] Factory-pattern doc/code inconsistency - done. Added createBasicResponseMock / createInitStatusMock / createCiCompetitorMock and routed all five BasicResponse/AIOProjectInitializedResponse handlers plus ci/competitors through them. There are now zero inline-literal exceptions; CLAUDE.md updated to match.

Non-blocking

• CI job timeout-minutes: 15 - done.
• E2E discards server stderr - done. stderr is now captured and its tail appended to the readiness-timeout error.
• E2E teardown no SIGKILL escalation - done. Escalates to SIGKILL on the process group if SIGTERM is ignored.
• __quota missing workspaceId - done. Returns 400 instead of storing under a generated id.
• store clone-isolation untested - done. Added a test mutating a create/get return and re-reading.
• auth whitespace-only-token - done. Added a canary test plus a comment pinning why .trim() is load-bearing.
• E2E localhost vs 127.0.0.1 probe - done. Pinned to 127.0.0.1.
• CLAUDE.md says globalThis.crypto is typed via lib dom - done. Corrected to @types/node (lib: ["esnext"]).
• ci/competitors lone inline literal - done. Now routed through createCiCompetitorMock (folded into the factory fix above).
• LIB_FILES self-validation - declined. It already fails loud at boot (a missing lib file throws on materialization); parsing context.js imports to enforce the list is YAGNI given that loud failure mode.
• Readiness probe via auth-exempt /__dump instead of a seeded route - declined. The current Bearer readiness-probe probe works and also exercises the auth path; changing the readiness logic carries more risk than the seed-dependency it would remove.
• python/serenity_project_engine/http_server.py misleading name - declined. It is datamodel-codegen output; renaming requires changing the codegen target, disproportionate to the cosmetic gain.
• Comment in .nycrc.json pointing at the e2e as the compensating control - declined. .nycrc.json is parsed as strict JSON, so a comment would break it; the rationale already lives in run.js and the workflow.

Out of scope, worth tracking

• The api-service consumer getInitStatus calls the dead /v1 init_status path (the mock faithfully reproduces the live 404, overlay CR8). The real fix is a spacecat-api-service change - left for the adoption PR / a tracking ticket.

All 5 blocking findings addressed in 4bd7c51 (see consolidated comment); gates green (types, lint, unit 154 @ 100%, e2e 30).

[rainer-friederich]

Ran a second local multi-agent review (6 agents: staff-engineer, architect, project-conventions, security, sre, qa) over the post-fix tree (this PR's ~4.9k-LOC diff is over the MysticatBot limit, so the local pass is the stand-in). All five prior-review fixes were independently verified as correctly resolved. The new findings are addressed in dbcd395d.

Addressed:

• [Important] Design-of-record understated the shipped stateful set - docs/mock-statefulness.md listed 3 stateful resources but the implementation ships 5 (STATEFUL_RESOURCES = projects, ai_models, prompts, benchmarks, brand_urls). Done: added benchmarks + brand_urls to the consumer-inventory table and the confirmed-set line, with the consumer write-then-read justification; the implementation was the correct side.
• nit: injectAuthGuard's comment claimed the throw catches arrow handlers and OPTIONS/HEAD, but the regexes caught neither. Done: extracted it to a pure mock/inject-auth-guard.js and broadened the declared-method count to every shape the comment names (export const VERB =, plus OPTIONS/HEAD), so an unguardable shape now trips the count-vs-guarded throw - the guarantee matches the code.
• suggestion: the auth-bypass safety net had no unit test (run.js launches the server at import and is coverage-excluded). Done: the extraction makes it a pure transform; added test/mock/inject-auth-guard.test.js (conforming handler guarded, __* untouched, arrow/OPTIONS/renamed-param throw) at 100%.
• suggestion: the benchmark PUT (updateBenchmark) was the only consumer-critical stateful route the E2E never drove. Done: added a create -> PUT brand_aliases -> list-reflects case.
• nit: mock/quota.js described the over-allocation body as nginx-style, but handlers emit JSON { message }. Done: corrected the comment (consumer keys on the 405 status, not the body).
• nit: package enforces branches: 100 vs the root CLAUDE.md's 97%. Done: documented the deviation + the coverage exclusions in the package CLAUDE.md.
• suggestion: existing_count is hard-wired to 0 (no dedup modeled). Done: documented next to the other fidelity simplifications.
• suggestion: clarify ./mock/* is intentionally unpublished. Done: strengthened the package CLAUDE.md note (workspace-symlink-only, never the registry tarball).
• (tracking) unauthenticated __* control routes if ever shared-hosted. Done as a doc caveat: added a bind-to-localhost note in docs/mock-usage.md.

Declined / no change:

• launch() handles exit but not error (sre) - declined: a spawn failure's stderr still reaches the captured stream and is dumped on readiness timeout, so it stays diagnosable; no silent path.
• v1/v2 ai_models POST handlers are near-identical (staff) - declined: extracting a shared helper across two untyped materialized handlers adds more than it saves; left as-is.
• E2E rebuilds build/openapi3.json every CI run rather than caching (sre) - out of scope: CI-cost only, not correctness; not changed here.

Gates after dbcd395d: types, lint, unit 162 @ 100%, e2e 31.

[rainer-friederich]

Live fidelity round 2 — write endpoints + a real swagger gap

Cross-checked the mock against the real Semrush Project Engine API (test workspace adobe-hackathon / c522f571-…, verified 2026-06-25) by replaying every write op the api-service rest-transport makes against both, on a throwaway project that was created and deleted (clean-up confirmed). Reads were already faithful in round 1; this round drove the writes. Fixes are in 4ee03f80.

Mock-handler fixes (the swagger was already correct for these — the bugs were in the materialized handlers):

• createProject — live returns a draft ProjectResponse with the request nested under settings.ai (live_id/draft_id mirrored, is_draft: true, publish_status: 'draft'). The handler echoed the flat request body instead, leaking request-only fields (country_code/language_id/…) and dropping settings. Now mapped through a new typed factory createProjectResponseFromRequest.
• publishProject, deleteBenchmarks, updateBenchmark, deleteBrandUrls — live returns a 202 with an empty body (content-length: 0; the swagger declares no 202 schema). Handlers returned a BasicResponse { message } envelope; now return an empty 202.
• addAiModel — live resolves the catalog model's icon onto the response; createAiModelMock now carries icon.

Overlay fix (the one genuine swagger gap):

• CR9 — model.AISettings gains primary_url. The live settings.ai returns it on every project, but the vendored swagger omits it, so a typed handler/fixture populating it failed tsc. Regenerated src/generated/types.ts (one additive optional field).

Also corrected the CR5 comment (the add path does return name+icon; only key comes back empty) and a stale "three resource groups" note.

Deliberately NOT changed (verified environment/design, not mock bugs):

• createTaggedPrompts and publishProject returned an nginx 405 Not Allowed on the hackathon tenant — that is the tenant gateway gating those routes, not the prod contract (the code documents prod-verified tagged → 201 / publish → 202), so the mock keeps the prod behaviour.
• brand-topics without domain returns live 400; the mock serves 200 because request validation is intentionally off (documented design).

Gates: test:types ✓, lint ✓, unit 165 passing @ 100% (branches: 100) ✓, e2e 31 passing ✓ (e2e now pins the nested create shape, model.icon on add, and the empty-body 202s).

[rainer-friederich]

Follow-up to the round-2 comment — brand-topics missing-param 400 now replicated (54b7f918)

Revisiting the two items I'd flagged as "deliberately not changed":

• brand-topics missing domain/country → now 400 (changed). Both are required query params in the swagger, and live returns 400 { message: '<param> query param is required' } (verified no-domain → 400, 2026-06-25). The mock served 200 regardless because request validation is globally off — so I added a targeted in-handler guard (not global validation). The consumer always passes both, so nothing regresses; this just makes the missing-param path 1:1. e2e pins both the no-domain and no-country 400s.

• createTaggedPrompts / publish nginx-405 → still not replicated (by design). That 405 is the hackathon tenant's gateway gating those two routes (raw nginx 405 Not Allowed, not an app response), not the prod contract. Replicating it would make the mock 405 the consumer's core success path — creating prompts and publishing — which prod onboarding does every day, so the prod contract is tagged → 201 / publish → 202 (what the code documents as prod-verified and what the mock + e2e encode). The mock emulates prod, so it keeps 201/202. Caveat: I confirmed the 405 only on the hackathon mirror this session; if you want hard proof of the prod 201/202 I can pull it from prod Splunk or a prod-enabled Semrush workspace.

Gates after this change: lint ✓, test:types ✓, unit 165 @ 100% (branches: 100) ✓, e2e 32 passing ✓.

[rainer-friederich]

Request validation now enabled — spec-driven, replacing the per-handler guard (48bc83cc)

You asked whether the brand-topics 400 should be spec-driven rather than a hand-coded guard. It should — and the blocker that kept request validation off turned out to be stale, so I enabled it properly.

The old reason (mock/run.js): the vendored spec declared a required Auth-Data-Jwt header param that Counterfact matched case-sensitively against Node's lowercased inbound headers, 400ing every request. But CR2 already removes Auth-Data-Jwt from every operation, and it was the only header param — so there's nothing left to mis-match. Verified empirically: with --no-validate-request dropped, valid calls pass and the overlay-corrected spec drives 400s.

What this gives us (matching live, across every endpoint, not just brand-topics):

• missing required query param → 400 (e.g. brand-topics domain/country)
• missing required body field → 400 (e.g. project-create type)

So I removed the per-handler brand-topics guard — the validator 400s first now. (Trade-off: the 400 body is Counterfact's generic validator message, not the live { message } text; the status is the contract the consumer keys on.)

Enabling it surfaced a few under-specified requests in the e2e — exactly what validation is for — all now fixed to match what the real consumer already sends:

• project create/patch now send the required type (ProjectRequest requires [name, type])
• getProject now sends the required draft query param

I probed all 15 write endpoints; only those (plus brand-topics' query params) were under-specified — the other 13 bodies validated as-is, and the consumer's ?type=ai on list-projects is a declared param so it's accepted.

Gates: lint ✓, test:types ✓, unit 165 @ 100% (branches: 100) ✓, e2e 33 passing ✓ (added a query-param-400 and a body-field-400 lock). Mock-only/test/docs change — no shipped src/ delta, so chore( (non-releasing).

[rainer-friederich]

Hey @aliciadriani,

Verdict: Request changes - the shipping mock code is correct and live-verified; two small, in-scope gaps are worth closing first (a test that doesn't lock a contract it claims to, and a seed helper that can't express two of its own resource types).
Changes: adds a stateful Counterfact mock (store, stateful ops, typed factories, seeds, quota meter, bearer-auth guard, route handlers, runner), a spec overlay, generated types, unit + gated-E2E tests, docs, and a path-gated CI job to spacecat-shared-project-engine-client (64 files).
Note: this PR's diff (~5.3k LOC) is over the MysticatBot limit, so this is the local multi-agent stand-in (7 reviewers). CI is green.

Must fix before merge

1. [Important] E2E asserts state only for delete-benchmarks / update-benchmark / delete-brand-urls - the empty-body 202 contract is locked only for publish - packages/spacecat-shared-project-engine-client/test/e2e/project-engine-mock.e2e.js:516 (details inline)
2. [Important] buildSeed can't seed benchmarks/brand_urls despite its docstring's "mirror the rows it inserted into Postgres" promise - packages/spacecat-shared-project-engine-client/mock/seeds.js:143 (details inline)

Non-blocking (9): suggestions and nits

• suggestion: derive LIB_FILES from context.js's actual imports (or assert the _lib/ set equals them after materialize) instead of the hand-maintained list - a missing entry fails the E2E boot, but the manual sync is a footgun the next contributor will hit - mock/run.js:87.
• suggestion: QUOTA_COLLECTION test asserts the constant equals 'quota' (tautological); instead quota.set(...) then store.list(QUOTA_COLLECTION) to prove they're wired at runtime - test/mock/quota.test.js.
• suggestion: factories.type-test.ts type-asserts only 5 of 12 factories; add createBenchmarkMock and createBrandUrlMock (the ones the overlay drift-guard exists for) - test/types/factories.type-test.ts.
• suggestion: add a push: { branches: [main] } trigger (or keep the documented PR-only-signal intent on purpose) so the live-E2E + type-surface check also run post-merge, not just on PRs - .github/workflows/project-engine-mock-e2e.yaml:16.
• nit: beforeEach's POST /__reset throws an opaque "fetch failed" if the mock died mid-suite; guard on the existing serverExited flag for an actionable message - test/e2e/project-engine-mock.e2e.js.
• nit: copy the lib files with readFileSync(join(here, file), 'utf8') - the current call returns a Buffer; it works only because writeFileSync writes the bytes - mock/run.js:133.
• nit: one-line comment on GUARDABLE_METHOD explaining why OPTIONS/HEAD are in DECLARED_METHOD but intentionally absent here (the fail-closed asymmetry) - mock/inject-auth-guard.js.
• nit: the PR body "## Validation" counts (147 unit / 29 e2e) are stale vs HEAD (the latest fix round reports 165 unit / 33 e2e); refresh them.
• nit: quota.set has a read-then-write that's a TOCTOU only if the store ever becomes multi-writer; a one-line note on the single-writer invariant would age well - mock/quota.js.

Out of scope, worth tracking

• The overlay (corrections.yaml, 9 corrections) is verified manually and timestamped inline; there is no automated freshness gate, so a future upstream Semrush change can silently diverge from a correction. A scheduled contract test against the sandbox workspace would close the loop - belongs with the adoption PR / the Jira epic, not here.
• CI uses floating action tags (actions/checkout@v6, etc.); this matches the repo-wide main.yaml pattern, so SHA-pinning is a repo-wide pass, not this PR.
• The e2e job passes MYSTICAT_DATA_SERVICE_REPO_READ_TOKEN through the shared setup-node-npm action (same as main.yaml). It's the monorepo install convention; if the root npm ci provably doesn't need it for this workflow it could be dropped, but that's an action-level change, not introduced here.

The architecture is sound and the review surfaced no bugs in shipping code: the generic-store -> stateful-ops -> typed-factories -> Counterfact-handlers layering is clean, the auth guard is fail-closed at the single materialization seam (the prior fail-open drift bug is fixed and the drift vectors are unit-proven), the overlay-as-source-of-truth with a gitignored build artifact is the right call, and the 100%-unit / E2E-for-the-server-surface split is the correct trade-off for a mock that can't run under npm test. The live-fidelity verification (timestamped against a real workspace) is exemplary.

[rainer-friederich]

issue (blocking): The empty-body 202 contract is locked only for publish (the raw-fetch expect(await rawPub.text()).to.equal('') at ~line 600). The comment at line 594 says delete-benchmarks, update-benchmark, AND delete-brand-urls all return a 202 with an EMPTY body, but those three tests assert state only - client.DELETE/client.PUT swallow the response body, so a regression that drops body: '' and returns a { message } envelope (the exact Counterfact {status:202} -> "Accepted" gotcha this PR fixes) would still pass. These handlers are coverage-excluded, so the E2E is their only gate. Add a raw-fetch status === 202 && (await res.text()) === '' assertion to the benchmark-delete, benchmark-PUT, and brand-url-delete cases, mirroring the publish test.

[rainer-friederich]

Fixed in 81ff578f — the delete-benchmarks, update-benchmark (PUT v1), and delete-brand-urls e2e cases now raw-fetch and assert status === 202 && (await res.text()) === "", matching the publish lock. A regression to a {message} body now fails the suite.

[rainer-friederich]

issue (blocking): buildSeed's docstring says it lets a caller "mirror the rows it inserted into Postgres," but SeedProject only accepts aiModels and prompts - benchmarks and brand_urls (both now part of the stateful set, and both present in WORKSPACE_WITH_DATA above) can't be expressed. The cross-repo harness this helper exists for would have to hand-write raw collectionKey(...) snapshot JSON, bypassing the factory drift-guard. Either extend SeedProject with optional benchmarks/brandUrls routed through collectionKey (the same way aiModels/prompts are handled on lines 152-153), or narrow the docstring to state the helper covers projects/ai_models/prompts only.

[rainer-friederich]

Fixed in 81ff578f — SeedProject now accepts benchmarks and brandUrls (grouped by benchmark id), routed through collectionKey like aiModels/prompts. Added a seeds.test case that loads both and reads them back via the stateful ops.

[rainer-friederich]

Addressed the local review in 81ff578f (+ the two earlier follow-ups 86135e30 dead-handler removal and 26e2d369 PATCH-fidelity fix). Gates after: lint clean, types clean, unit 169 @ 100% (branches: 100), e2e 33 passing.

Must fix (both done)

• [Important] empty-body 202 not locked for delete-benchmarks / update-benchmark / delete-brand-urls — done. Each now raw-fetches and asserts status === 202 && text() === '' (only publish did before; the typed client swallowed the body).
• [Important] buildSeed couldn't seed benchmarks/brand_urls — done. SeedProject gained benchmarks + brandUrls (grouped by benchmark id), routed through collectionKey; new seeds.test reads them back via the stateful ops.

Non-blocking (all addressed)

• LIB_FILES is now derived from the actual mock/*.js files (every one except run.js + inject-auth-guard.js) instead of a hand-maintained list — a new context.js import is materialized automatically.
• QUOTA_COLLECTION test is no longer tautological — it now set()s and asserts store.list(QUOTA_COLLECTION) reflects it.
• factories.type-test.ts now type-guards createBenchmarkMock, createBrandUrlMock, and applyProjectUpdate (was 5 of 12).
• CI: added a push: [main] trigger so the type-surface check + live E2E also run post-merge, not only on PRs.
• e2e beforeEach now fails with a clear message if the mock died mid-suite (guards on the existing serverExited flag).
• run.js reads lib sources as utf8.
• inject-auth-guard.js documents why OPTIONS/HEAD are fail-closed (counted in DECLARED_METHOD, not in GUARDABLE_METHOD).
• PR body Validation counts refreshed (169 unit / 33 e2e).
• quota.set carries a one-line note on the single-writer (no-TOCTOU) invariant.

Out of scope, left tracked (not in this PR)

• Overlay freshness gate (a scheduled contract test vs the live API) — belongs with the adoption PR / Jira epic, not here.
• Floating CI action tags (actions/checkout@v6, …) — matches the repo-wide main.yaml pattern; SHA-pinning is a repo-wide pass.
• MYSTICAT_DATA_SERVICE_REPO_READ_TOKEN on the e2e job — dropped as a finding: it's the shared setup-node-npm install credential, passed exactly as main.yaml does for the monorepo-root npm ci; not introduced here.

Bonus (from the follow-up audit you asked for, beyond the review)

• Removed the dead v1 ai_models POST handler (consumer adds via v2; v1 is list/delete-only — confirmed against api-service origin/main).
• Fixed updateProject PATCH fidelity: live returns 200 + full body and reflects flat brand fields nested under settings.ai (verified live 2026-06-25); the handler had shallow-merged them to the top level. New applyProjectUpdate factory + an e2e assertion.