Skip to content

[Testing 08] test: route-level integration tests + real-Supabase auth/RLS suites - #233

Merged
willchen96 merged 5 commits into
Open-Legal-Products:mainfrom
amal66:olp-pr/backend-integration-tests
Jul 23, 2026
Merged

[Testing 08] test: route-level integration tests + real-Supabase auth/RLS suites#233
willchen96 merged 5 commits into
Open-Legal-Products:mainfrom
amal66:olp-pr/backend-integration-tests

Conversation

@amal66

@amal66 amal66 commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Route-level integration tests for the API — 94 tests across chat, projects, project-chat, tabular, user, document-upload and health routes, plus two env-gated suites that exercise the real Supabase stack (auth contract, deny-all RLS, cross-tenant isolation). Ported from the amal66 fork (reference index #205) and adapted to this repo's routes/ layout. Stacks on #228 (vitest harness).

Changes

  • One mechanical production change: backend/src/index.ts is split into app.ts (builds and exports the express app — the old file verbatim minus the listen call) and a tiny index.ts that imports it and listens. Same middleware order, same limiters, same log line; npm run build and the start script are unchanged. This is the standard seam supertest needs.
  • backend/src/__tests__/integration/ — mocked-Supabase suites: health (5), chat (6), projects (18), projectChat (3), tabular (31), user (27), documentsUpload (4). Coverage priorities: 401s for unauthenticated requests, ownership/authorization (user A cannot touch user B's resources), input validation, and happy paths.
  • Env-gated real-stack suites (skip cleanly unless SUPABASE_TEST_URL/SUPABASE_TEST_SERVICE_ROLE_KEY are set): access.supabase.test.ts (shared-project document filtering drops foreign document IDs) and stack.supabase.test.ts (GoTrue↔API auth contract, deny-all RLS on public tables, tenant isolation, service-key leak sweep). scripts/test-stack.sh + npm run test:stack read the keys from supabase status automatically.
  • supertest + @types/supertest dev-dependencies.

Why

The e2e suite (#220) proves the flows through a browser; nothing proves the API's authorization boundaries directly. For a product holding privileged legal documents, "user A cannot read user B's project" deserves a test that runs in seconds on every PR — and the RLS/auth contract deserves a suite you re-run on every Supabase bump.

What was deliberately not ported

Fork-only feature suites (orgs, credits, DMS connectors) and cases tied to fork-only behavior (credit reservation, upload magic-byte sniffing, download-zip doc cap). Two of those absences are worth flagging as hardening gaps here: uploads are validated by extension only (no content sniffing), and download-zip has no document-count cap. Happy to raise either as its own PR.

Testing

  • npm test --prefix backend — 8 files passed, 2 skipped (the gated ones without env); 106 passed, 5 skipped, 0 failed.
  • With a local Supabase running: npm run test:stack --prefix backend5/5 passed live.
  • npm run build --prefix backend — passes.

Includes #228's commit as its base until proper stacking is possible — review the top commit only.

🤖 Generated with Claude Code

Ports the backend route-level integration suites from the amal66 fork
(index: Open-Legal-Products#205), adapted from the fork's
apps/api modules/services layout to this repo's monolithic
backend/src/routes/*.ts layout.

app/index split (the only production change; mechanical, zero behavior
change): backend/src/index.ts previously built the express app and
called app.listen at the bottom. Everything except the listen call
moved verbatim into backend/src/app.ts, which now exports `app` (same
middleware order, same routes, same rate-limiter setup, dotenv/config
still imported first). index.ts is now a tiny entry that imports
{ app } and calls listen with the same log line. `npm run build` still
emits dist/index.js and the `start` script is unchanged.

New suites under backend/src/__tests__/integration/ (94 tests):
- health.test.ts (5): /health, requireAuth 401 paths, 404 fallthrough
- chat.routes.test.ts (6): POST /chat validation + SSE happy/error paths
- projects.routes.test.ts (18): overview/create/detail/patch/delete,
  sharing normalisation, ownership guards
- projectChat.routes.test.ts (3): project access guard + SSE paths
- tabular.routes.test.ts (31): review CRUD, access guards,
  document-access filtering, missing_api_key guards
- user.routes.test.ts (27): profile, API-key crypto boundary, MFA
  guards, export/deletion endpoints
- documentsUpload.routes.test.ts (4): upload validation + download-zip
  bounds/access
- access.supabase.test.ts (1) + stack.supabase.test.ts (4): gated on
  SUPABASE_TEST_URL / SUPABASE_TEST_SERVICE_ROLE_KEY (stack suite also
  needs SUPABASE_TEST_ANON_KEY); describe.skip otherwise
- scripts/test-stack.sh + `npm run test:stack`: reads a running
  `supabase status -o json` and runs the gated suites

Adds supertest + @types/supertest as devDependencies.

Dropped relative to the fork (subjects that do not exist in this repo):
- orgs.routes, credits.concurrency.supabase, dmsConnectors suites
  (fork-only features)
- all credit-reservation cases (429 CREDIT_LIMIT_EXCEEDED,
  reserve-then-refund) in chat/projectChat: no credit system here
- health /ready case: no /ready endpoint here
- org-membership project access case: no org model here
- upload magic-byte validation cases: this repo validates extension
  only (the fork adds content sniffing; replaced with a missing-file
  400 case)
- download-zip 50-document cap case: no cap here (replaced with a
  no-accessible-documents 404 case)
- stack.supabase PUBLIC_TABLES updated to this repo's schema; the
  fork's credit-RPC coverage is n/a

Verified locally: backend `npm test` -> 8 files passed, 2 skipped;
106 tests passed, 5 skipped (gated suites skip without env).
`npm run build` passes. The gated suites were additionally run against
a live local Supabase stack: 2 files, 5/5 tests passed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@CLAassistant

CLAassistant commented Jul 20, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ willchen96
❌ QA Runner


QA Runner seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@amal66 amal66 changed the title test: route-level integration tests + real-Supabase auth/RLS suites [Testing 08] test: route-level integration tests + real-Supabase auth/RLS suites Jul 21, 2026
willchen96 and others added 2 commits July 22, 2026 19:56
…ile)

The "Merge branch 'main'" commit (71a7aba) auto-merged backend/package.json
and backend/package-lock.json without raising a conflict, but git's
line-level merge produced invalid JSON in both files: package.json lost the
comma after the new "test:stack" script, and package-lock.json lost the two
closing-brace lines of the supertest/cookie-signature entry. npm treats an
unparseable lockfile as absent, which is why CI failed with the misleading
"npm ci can only install with an existing package-lock.json".

package.json: restore the comma. package-lock.json: rebuilt from main's
known-good copy via `npm install --package-lock-only` against this branch's
package.json. Verified locally: npm ci, npm test (259 passed), npm run build.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@amal66

amal66 commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

The backend CI failure here ("npm ci can only install with an existing package-lock.json") was a red herring — the lockfile existed. The Merge branch 'main' commit (71a7aba) auto-merged backend/package.json and backend/package-lock.json into invalid JSON with no conflict raised: git's line-level merge dropped the comma after the new "test:stack" script and the closing braces of the supertest/cookie-signature lockfile entry (both sides had inserted entries at the same alphabetical position). npm treats an unparseable lockfile as missing, hence the misleading error.

Fixed in 6aed350: comma restored, lockfile rebuilt from main's known-good copy via npm install --package-lock-only (net diff vs. the corrupted files is 3 lines). Verified locally: npm ci, npm test (259 passed, real-Supabase suites skip without env), npm run build.

Prevention so this class of failure can't recur silently: #240 (merge=binary for lockfiles + a CI parse-check that fails with the real reason).

willchen96 pushed a commit that referenced this pull request Jul 23, 2026
PR #233's CI failed with "npm ci can only install with an existing
package-lock.json" even though the file existed: a "Merge branch 'main'"
commit had auto-merged backend/package.json and package-lock.json into
invalid JSON with no conflict raised, and npm reports an unparseable
lockfile as if it were missing.

Two guards: .gitattributes marks package-lock.json/bun.lock merge=binary so
concurrent lockfile changes surface as explicit conflicts (resolve by
regenerating, never hand-merging), and CI parse-checks package.json and the
lockfile before npm ci so any corruption that still lands fails with the
real reason.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@willchen96

Copy link
Copy Markdown
Collaborator

Added two commits:

  • Initializes an empty local Supabase stack from backend/schema.sql.
  • Grants service_role required backend privileges.
  • Reports seed insertion errors clearly.
  • Avoids resetting populated databases.

@willchen96
willchen96 merged commit c0ff440 into Open-Legal-Products:main Jul 23, 2026
3 of 4 checks passed
willchen96 added a commit to amal66/mike that referenced this pull request Jul 25, 2026
The backend-integration-tests PR (Open-Legal-Products#233) added vitest, supertest, and
@vitest/coverage-v8 to backend/package.json but did not update
bun.lock, leaving the lockfile behind the manifest.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
duncanmcqueen pushed a commit to duncanmcqueen/mike that referenced this pull request Aug 3, 2026
PR Open-Legal-Products#233's CI failed with "npm ci can only install with an existing
package-lock.json" even though the file existed: a "Merge branch 'main'"
commit had auto-merged backend/package.json and package-lock.json into
invalid JSON with no conflict raised, and npm reports an unparseable
lockfile as if it were missing.

Two guards: .gitattributes marks package-lock.json/bun.lock merge=binary so
concurrent lockfile changes surface as explicit conflicts (resolve by
regenerating, never hand-merging), and CI parse-checks package.json and the
lockfile before npm ci so any corruption that still lands fails with the
real reason.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
duncanmcqueen pushed a commit to duncanmcqueen/mike that referenced this pull request Aug 3, 2026
…-integration-tests

[Testing 08] test: route-level integration tests + real-Supabase auth/RLS suites
duncanmcqueen pushed a commit to duncanmcqueen/mike that referenced this pull request Aug 3, 2026
The backend-integration-tests PR (Open-Legal-Products#233) added vitest, supertest, and
@vitest/coverage-v8 to backend/package.json but did not update
bun.lock, leaving the lockfile behind the manifest.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

3 participants