Skip to content

fix: repair failing tests and type errors across api and shared packages - #118

Open
stooit wants to merge 1 commit into
mainfrom
quantcode/e2e-tier2-2277-1785687930
Open

fix: repair failing tests and type errors across api and shared packages#118
stooit wants to merge 1 commit into
mainfrom
quantcode/e2e-tier2-2277-1785687930

Conversation

@stooit

@stooit stooit commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes all failing tests (was 9 fail / 13 pass) and type errors across the api and shared packages. Final state: 22 pass / 0 fail / tsc --noEmit exit 0. No test files were modified and no dependencies were added.

Fixes

  • Pagination (shared/src/utils/pagination.ts) — implemented the paginate stub (was throwing not implemented): page slicing, total/totalPages, page/pageSize echo, out-of-range page → empty data, empty-array handling. Also guarded against non-finite/negative page/size so the returned page: number/pageSize: number contract can't serialise to null (page→min 1, size→min 0, floored to integers).
  • Auth middleware (api/src/middleware/auth.ts) — fixed a case-sensitivity bug in the public-methods allow-list (["GET", "post"]) that caused POST /users to return 401 instead of being treated as public. Now uses PUBLIC_METHODS = ["GET", "POST"] compared against a normalised c.req.method.toUpperCase().
  • Shared types (shared/src/types.ts) — renamed the planted User.userName field to username to match API usage and the test contract (tests use username throughout and are the immutable contract).
  • Users route (api/src/routes/users.ts) — added the missing badRequest import from ../lib/errors (was a runtime ReferenceError on invalid POST bodies, failing the "returns 400 for missing fields" test).
  • tsconfig.json — added types: ["bun-types"] to resolve bun:test and process in test files and middleware. Kept lib at ["ES2022"] only (verified DOM was unnecessary and would let browser-only globals compile clean inside the Bun/Hono server package).

Assumptions & decisions

  • The task brief suggested making users.ts use userName; that was inverted. The tests use username and cannot be modified, so types.ts (whose own comment flagged it as the bug) was the correct fix site. Changing the route instead would have introduced four new type errors at test call sites.
  • POST being public applies globally (all routes, not just /users), matching the documented middleware policy and an explicit test assertion ("POST is intentionally public"). Preserved as-is.

Reviewer notes (non-blocking, out of scope — no test requires them)

  • Global-public POST means unauthenticated writes are accepted on /posts too; a per-route allow-list would be safer (ISM-1552) if that wasn't the intent.
  • Token comparison in auth.ts uses !== (non-constant-time); crypto.timingSafeEqual would be the ISM-aligned hardening.

Verification

bun test && npx tsc --noEmit → 22 pass, 0 fail, exit 0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant