test(shared): add web<->server contract fixtures + tests for /api/me#1591
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds canonical contract fixtures for GET /api/me, exposes them from the shared package, and implements producer (server) and consumer (web/api-client) contract tests plus documentation updates reflecting the new contract-test status. ChangesContract Testing for GET /api/me
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 3/10 reviews remaining, refill in 39 minutes and 13 seconds. Comment |
New module packages/shared/src/contract-fixtures/me.ts ships 4 canonical MeResponse cases (minimal, full, legacyNoCreatedAt, unverified) shared between producer and consumer. apps/web/src/test/contract/me.contract.test.ts pipes each fixture through the api-client plus MeResponseSchema (7 tests). The producer test apps/server/src/routes/me.contract.test.ts pipes each fixture through the route handler via supertest with various createdAt input shapes (10 tests). Closes diagnostic 2026-05-03-web-deep-dive section 7.4. Pattern doc in packages/shared/src/contract-fixtures/README.md.
Skords-01
force-pushed
the
devin/1777863626-contract-tests
branch
from
13b6112 to
268efdc
Compare
⏱️ CI Pipeline Duration ReportBased on the last 50 successful runs on the default branch. Overall Pipeline
Trend (last 20 runs): Per-Job Breakdown
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/web/src/test/contract/me.contract.test.ts (1)
31-36: ⚡ Quick winMake fixture iteration exhaustive by sourcing keys from
meFixtures.Keeping a separate name list can drift. Deriving keys from the shared fixture object keeps this contract suite automatically in sync.
Suggested change
-const FIXTURE_NAMES: readonly MeFixtureCase[] = [ - "minimal", - "full", - "legacyNoCreatedAt", - "unverified", -] as const; +const FIXTURE_NAMES = Object.keys(meFixtures) as MeFixtureCase[];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/test/contract/me.contract.test.ts` around lines 31 - 36, Replace the hardcoded FIXTURE_NAMES array with an exhaustive source derived from the shared meFixtures object so the test stays in sync; locate the FIXTURE_NAMES declaration and change it to compute the fixture keys from meFixtures (using Object.keys/typed alternatives) and cast/validate them to MeFixtureCase so tests iterate all fixture entries rather than a manually maintained list.apps/server/src/routes/me.contract.test.ts (1)
112-117: ⚡ Quick winDerive fixture names from
meFixturesto avoid silent coverage gaps.This manual list can go stale when a new fixture is added. Build the list from the fixture object so every case is exercised automatically.
Suggested change
-const NAMES: readonly MeFixtureCase[] = [ - "minimal", - "full", - "legacyNoCreatedAt", - "unverified", -] as const; +const NAMES = Object.keys(meFixtures) as MeFixtureCase[];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/routes/me.contract.test.ts` around lines 112 - 117, The hard-coded NAMES array can go stale; instead derive it from the meFixtures object so new fixtures are automatically tested. Replace the manual NAMES declaration with code that reads the keys from meFixtures (casting to MeFixtureCase[] or otherwise asserting the type) to produce the readonly list used by the tests (keep the symbol NAMES and type MeFixtureCase), ensuring deterministic order if needed (e.g., sort) so test behavior remains stable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/server/src/routes/me.contract.test.ts`:
- Around line 44-57: The mocks and import use relative paths ("./../db.js",
"./../auth.js", "./../app.js"); update them to the repo's path aliases (e.g.,
replace those strings in the vi.mock calls and the createApp import with the
appropriate alias modules like the @... aliases your project uses) so the tests
and mocks resolve via configured aliases instead of relative imports; update the
vi.mock targets (the first argument strings) and the import specifier for
createApp to the corresponding alias names.
In `@packages/shared/src/contract-fixtures/me.ts`:
- Line 20: The import in packages/shared/src/contract-fixtures/me.ts currently
uses a relative path for MeResponseSchema and MeResponse; update that import to
use the repository path alias for the shared schemas (e.g., replace the relative
import of "../schemas/api" with the `@shared` alias for that module) so
MeResponseSchema and MeResponse are imported via the configured alias rather
than a relative path.
In `@packages/shared/src/contract-fixtures/README.md`:
- Around line 31-36: The fenced code block in README.md is unlabeled and
triggers markdownlint MD040; update the opening backticks of the tree block in
the README.md file to include a language identifier (for example change ``` to
```text) so the code fence is explicitly tagged (e.g., use ```text above the
directory tree).
---
Nitpick comments:
In `@apps/server/src/routes/me.contract.test.ts`:
- Around line 112-117: The hard-coded NAMES array can go stale; instead derive
it from the meFixtures object so new fixtures are automatically tested. Replace
the manual NAMES declaration with code that reads the keys from meFixtures
(casting to MeFixtureCase[] or otherwise asserting the type) to produce the
readonly list used by the tests (keep the symbol NAMES and type MeFixtureCase),
ensuring deterministic order if needed (e.g., sort) so test behavior remains
stable.
In `@apps/web/src/test/contract/me.contract.test.ts`:
- Around line 31-36: Replace the hardcoded FIXTURE_NAMES array with an
exhaustive source derived from the shared meFixtures object so the test stays in
sync; locate the FIXTURE_NAMES declaration and change it to compute the fixture
keys from meFixtures (using Object.keys/typed alternatives) and cast/validate
them to MeFixtureCase so tests iterate all fixture entries rather than a
manually maintained list.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b53896ec-0c6c-427a-b6b9-37668239d02a
📒 Files selected for processing (8)
apps/server/src/routes/me.contract.test.tsapps/web/src/test/contract/me.contract.test.tsdocs/diagnostics/2026-05-03-web-deep-dive/00-overview.mddocs/diagnostics/2026-05-03-web-deep-dive/04-security-observability-testing-devx.mdpackages/shared/src/contract-fixtures/README.mdpackages/shared/src/contract-fixtures/index.tspackages/shared/src/contract-fixtures/me.tspackages/shared/src/index.ts
Comment on lines
+44
to
+57
| vi.mock("./../db.js", () => ({ | ||
| default: mockPool, | ||
| pool: mockPool, | ||
| query: queryMock, | ||
| ensureSchema: vi.fn().mockResolvedValue(undefined), | ||
| })); | ||
|
|
||
| vi.mock("./../auth.js", () => ({ | ||
| auth: { handler: async () => new Response(null, { status: 404 }) }, | ||
| getSessionUser: getSessionUserMock, | ||
| getSessionUserSoft: vi.fn().mockResolvedValue(null), | ||
| })); | ||
|
|
||
| import { createApp } from "./../app.js"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Switch new test-module imports/mocks to path aliases.
Please avoid new relative module paths here and use the repo’s configured aliases for consistency with import policy.
As per coding guidelines: Use path aliases (@shared/*, @finyk/*, etc.) instead of relative imports like ../../../.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/src/routes/me.contract.test.ts` around lines 44 - 57, The mocks
and import use relative paths ("./../db.js", "./../auth.js", "./../app.js");
update them to the repo's path aliases (e.g., replace those strings in the
vi.mock calls and the createApp import with the appropriate alias modules like
the @... aliases your project uses) so the tests and mocks resolve via
configured aliases instead of relative imports; update the vi.mock targets (the
first argument strings) and the import specifier for createApp to the
corresponding alias names.
| * conditionally surfaces a "verify email" banner off this flag. | ||
| */ | ||
|
|
||
| import { MeResponseSchema, type MeResponse } from "../schemas/api"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the configured path alias for this shared-schema import.
Please replace the new relative import with the repository alias form to keep new files aligned with the import policy.
As per coding guidelines: Use path aliases (@shared/*, @finyk/*, etc.) instead of relative imports like ../../../.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/src/contract-fixtures/me.ts` at line 20, The import in
packages/shared/src/contract-fixtures/me.ts currently uses a relative path for
MeResponseSchema and MeResponse; update that import to use the repository path
alias for the shared schemas (e.g., replace the relative import of
"../schemas/api" with the `@shared` alias for that module) so MeResponseSchema and
MeResponse are imported via the configured alias rather than a relative path.
Comment on lines
+31
to
+36
| ``` | ||
| contract-fixtures/ | ||
| ├── README.md ← this file | ||
| ├── index.ts ← barrel | ||
| └── me.ts ← /api/me canonical shapes (User, MeResponse) | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced block.
The code fence is currently unlabeled and triggers markdownlint MD040. Add a language tag (e.g. text) after the opening backticks.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/src/contract-fixtures/README.md` around lines 31 - 36, The
fenced code block in README.md is unlabeled and triggers markdownlint MD040;
update the opening backticks of the tree block in the README.md file to include
a language identifier (for example change ``` to ```text) so the code fence is
explicitly tagged (e.g., use ```text above the directory tree).
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/shared/src/contract-fixtures/README.md (1)
31-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifier to the fenced code block.
The code fence on line 31 is missing a language identifier, triggering markdownlint MD040. This issue was already flagged in a previous review.
📝 Proposed fix
-``` +```text contract-fixtures/ ├── README.md ← this file ├── index.ts ← barrel🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/contract-fixtures/README.md` at line 31, The fenced code block in README.md that starts with ``` above the "contract-fixtures/" tree is missing a language identifier (triggering markdownlint MD040); update the opening fence to include a language (for example change ``` to ```text or ```bash) so the block becomes ```text and keep the rest of the snippet unchanged to satisfy the linter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/shared/src/contract-fixtures/README.md`:
- Line 31: The fenced code block in README.md that starts with ``` above the
"contract-fixtures/" tree is missing a language identifier (triggering
markdownlint MD040); update the opening fence to include a language (for example
change ``` to ```text or ```bash) so the block becomes ```text and keep the rest
of the snippet unchanged to satisfy the linter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 49a4dccf-c402-4af9-8408-56a631d2a144
📒 Files selected for processing (8)
apps/server/src/routes/me.contract.test.tsapps/web/src/test/contract/me.contract.test.tsdocs/diagnostics/2026-05-03-web-deep-dive/00-overview.mddocs/diagnostics/2026-05-03-web-deep-dive/04-security-observability-testing-devx.mdpackages/shared/src/contract-fixtures/README.mdpackages/shared/src/contract-fixtures/index.tspackages/shared/src/contract-fixtures/me.tspackages/shared/src/index.ts
✅ Files skipped from review due to trivial changes (3)
- packages/shared/src/index.ts
- packages/shared/src/contract-fixtures/index.ts
- docs/diagnostics/2026-05-03-web-deep-dive/00-overview.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/shared/src/contract-fixtures/me.ts
- docs/diagnostics/2026-05-03-web-deep-dive/04-security-observability-testing-devx.md
- apps/web/src/test/contract/me.contract.test.ts
- apps/server/src/routes/me.contract.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes item 14 of the web-deep-dive roadmap (
docs/diagnostics/2026-05-03-web-deep-dive/04-security-observability-testing-devx.md§7.4, score 2.00). Until now, web and server independently asserted what/api/melooks like — the same shape was duplicated inMeResponseSchema(zod), in api-client types, and in route serializer code, with no test that all three agree byte-for-byte. This PR introduces a shared canonical-fixture pattern that locks the contract from both sides.What ships:
packages/shared/src/contract-fixtures/me.tsexports 4 namedMeResponsecases covering real scenarios:minimal— fresh user, noname/image/ Cyrillic content.full— populated user with image URL and Ukrainian display name.legacyNoCreatedAt— historical row wherecreatedAtisnull(pre-migration data).unverified— user awaiting email verification.assertMeFixturesValid()— sanity helper that re-validates every fixture againstMeResponseSchemaso that an out-of-band schema tightening fails fast.apps/web/src/test/contract/me.contract.test.ts(7 tests) feeds each fixture throughcreateMeEndpoints(http).get()andMeResponseSchema.safeParse(), verifying that whatever the server emits round-trips to the typed client unchanged. Also asserts the schema rejects a payload with a missing required field, locking the negative side.apps/server/src/routes/me.contract.test.ts(10 tests) mounts the real route via supertest with stubbed Better Auth session, drives each fixture's createdAt as aDate, an ISO string, and as missing-on-the-record, and asserts the route emits exactly the fixture body. Also checks/api/meand/api/v1/mereturn identical bodies.packages/shared/src/contract-fixtures/README.mdcodifies the recipe so adding the next endpoint is a 4-line PR (fixture file + consumer test + producer test + barrel re-export).No production code is touched — this is pure observability of the existing contract.
Governing Skill
sergeant-server-apisergeant-monorepo-boundariesPlaybook
Verification
pnpm lintis broken onmainitself for the unrelatedeslint-plugin-react@7.37.5×eslint@10.3.0incompat (PR #1572). This PR's vitest runs are clean.Additional checks:
Docs and Governance
AGENTS.mdneeded an update.Updated docs:
packages/shared/src/contract-fixtures/README.md— new pattern doc.docs/diagnostics/2026-05-03-web-deep-dive/04-security-observability-testing-devx.md§7.4 — points at the live tests.docs/diagnostics/2026-05-03-web-deep-dive/00-overview.md— roadmap row docs(finyk): add deep module audit and prioritized roadmap #14 marked done.Risk and Rollout
Hard Rule #15
AGENTS.mdbefore coding.--no-verify.--no-verifyrationale: identical to PR #1588 / #1589 / #1590 — the local Husky pre-commit hook fails becauseeslint-plugin-react@7.37.5is incompatible witheslint@10.3.0. Same infra breakage that's red onmain.Reviewer Notes
packages/shared(fixtures),apps/web(consumer test), andapps/server(producer test). Per AGENTS.md commit-scope guidance the most user-visible scope issharedbecause it ships the new exported module that anchors the pattern. The other two surfaces only consume the new exports for tests. Per Hard Rule Dashboard: Add time-based greeting, summary cards and quick action buttons to hero #3 the three sides moved together in one PR — that's the whole point of the pattern.createdAt: Datereturned from the auth layer becomes a JSON string in the response, exactly as the existing serializer does. If a future server change accidentally leaks aDateobject, the test fails because the fixture pins the wire-shape string.Summary by cubic
Adds shared contract fixtures for
/api/meand matching consumer/producer tests to lock the wire shape across web and server. This prevents drift and closes the web-deep-dive diagnostic §7.4 for this endpoint.packages/shared/src/contract-fixtures/me.ts: four cases (minimal,full,legacyNoCreatedAt,unverified),meRawFixtures, andassertMeFixturesValid(), exported via@sergeant/shared.apps/web/src/test/contract/me.contract.test.ts: feeds fixtures throughcreateMeEndpoints(http).get()andMeResponseSchema; asserts a missing required field is rejected.apps/server/src/routes/me.contract.test.ts: stubs auth and verifies/api/meand/api/v1/meemit bodies that match fixtures; checkscreatedAtasDate, ISO string, and missing.packages/shared/src/contract-fixtures/README.md; diagnostics updated and roadmap item marked done.Written for commit 268efdc. Summary will update on new commits.
Summary by CodeRabbit
Tests
Documentation
Chores