refactor(expenditures): declarative route table [5/8]

[nourshoreibah]

Stack 5/8. Base is #344, not main. Pure reorganization of the expenditures lambda; no behaviour change.

handler.ts drops from 505 lines to 4. The seven routes, in the order the if chain tested them:

Method	Pattern	Controller
GET	/expenditures	getExpenditures
POST	/expenditures	createExpenditure
GET	/expenditures/upload-url	getUploadUrl
GET	/expenditures/:id/receipt	getReceipt
GET	/expenditures/:id	getExpenditureById
DELETE	/expenditures/:id	deleteExpenditure
PATCH	/expenditures/:id/status	patchExpenditureStatus

This lambda is the reason the refactor exists. upload-url had to precede the :id GET purely by if-ordering — nothing enforced it, and reordering two blocks would have quietly routed GET /expenditures/upload-url into "load expenditure with id upload-url". That ordering is now explicit in one readable table, with a test pinning it. The endsWith('receipt') / endsWith('status') suffix sniffing and the split('/').filter(Boolean) index juggling are gone in favour of :param patterns.

Layout

At 505 lines this one earns a service layer:

• controllers/expenditures.ts — validation and response shaping.
• services/expenditures.ts — Kysely queries, S3 presigning, receiptKeyFromUrl.
• validation-utils.ts stays as-is; it was already the validation layer.
• Local json() and requireAuth() deleted in favour of the shared ones.

Admin gate confirmed

PATCH /expenditures/:id/status still runs requireAuth(authContext, 'ADMIN') before it touches the id or the body. One nuance: it now reaches the real checkAuthorization in @branch/lambda-auth via the shared requireAuth, rather than the handler-local wrapper that forwarded to the same function. The ADMIN branch was compared against the unit test's mock — identical logic — so this is one fewer layer of indirection, not a behaviour change.

Tests

• npx jest test/expenditures.unit.test.ts — 69/69 (68 original with no assertions changed, + 1 new precedence test for upload-url vs :id).
• tsc --noEmit clean.
• test/expenditures.e2e.test.ts was not run, and is not claimed to pass. Postgres was reachable, but db/testkit.ts's resetData() does TRUNCATE ... RESTART IDENTITY CASCADE over the whole branch schema in every beforeEach, and five sibling conversions were running against that same database at the time. Running it would have truncated rows out from under their in-flight suites. Worth a run in CI, which gets its own Postgres per lambda.

tsconfig.json's include gains controllers/**/*.ts and services/**/*.ts.

Dead code dropped

Two if (!id) return json(400, { message: 'id is required' }) checks in the GET and DELETE /:id handlers were already unreachable — the routing regex guaranteed a non-empty segment. matchPattern's :id capture makes the same guarantee, so they are gone. No observable behaviour change.

Noticed, not fixed

expenditures/openapi.yaml documents /expenditures, /expenditures/{id} and /expenditures/{id}/status, but has no entry for GET /expenditures/upload-url or GET /expenditures/{id}/receipt, both of which the handler implements. Pre-existing spec gap.

Request bodies deliberately keep raw JSON.parse rather than the shared parseBody: parseBody returns null on malformed JSON instead of throwing, which would turn today's 500 into something else.

Stack-wide note: unmatched paths now 404 instead of 401

Previously the top-level auth check ran before route matching, so an unauthenticated request to a nonexistent path got 401. dispatch returns 404 centrally without running auth, so an unauthenticated caller can now distinguish "route exists" (401) from "route doesn't" (404). Accepted deliberately — the route inventory is already in the repo's openapi.yaml files, and auth-before-routing is exactly what forces each lambda to hand-roll public-route exemptions. Flagged in every stack PR so it is a decision rather than an accident.

🤖 Generated with Claude Code