chore(lambdas): rebuild lambda-cli around route tables + normalize specs [8/8] - #348
Merged
Conversation
No lambda is converted yet -- this only lands the package the conversions
build on, so it is a pure addition.
The six handlers each route with a chain of `if (normalizedPath === ...)`
statements that test two or three path spellings per route, because API
Gateway's {proxy+} forwards the full path (/projects/7) while the shared
dev-server strips the first segment (/7). Params come out of hand-rolled
`split('/')[2]` and regex tests, correctness depends on `if` ordering that
nothing enforces, and `json()` is defined six times over with `requireAuth`
three times.
@branch/lambda-http replaces that with a declarative route table:
- dispatch({ prefix, routes }) canonicalizes the path to the prefixed shape so
one table serves both callers, matches `:param` segments, and centralizes
OPTIONS preflight, /<prefix>/health, 404 and 500.
- json() with CORS headers, parseBody(), requireAuth() and a createAuthGuard()
factory that binds a service's db-scoped authenticateRequest.
- 28 unit tests, including route precedence and both path shapes.
The dispatch/match/response/types modules are recovered from the closed PR
#257; the auth and body helpers are new. No infrastructure change is needed --
{proxy+} and ANY already landed on main via PR #279.
CI: both workflows build lambda-http after lambda-auth (it consumes that
package's dist), a shared-http job runs its tests and is added to the
lambda-tests gate, and lambda-deploy triggers on shared/lambda-http/**.
Note: lambda-deploy still does not trigger on shared/lambda-auth/**, a
pre-existing gap left alone here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pure reorganization, no behaviour change:
- handler.ts is now a one-liner: dispatch(event, { prefix: 'users', routes }).
- routes.ts holds the ordered Route[] table, bracketed by the ROUTES-START/
ROUTES-END markers (moved here from handler.ts) in the same order as the
original if-chain: GET /users, GET /users/:userId, PATCH /users/:userId,
DELETE /users/:userId, POST /users.
- controllers/users.ts holds one RouteHandler per route, calling Kysely
directly (no services/ layer — this lambda is thin). Auth now goes through
createAuthGuard(authenticateRequest) from @branch/lambda-http instead of a
handler-local requireAuth/checkAuthorization pairing; @branch/lambda-auth's
checkAuthorization (which the shared requireAuth calls) is behaviourally
identical to the removed local copy for every level this lambda uses.
- Local json()/requireAuth() helpers deleted in favor of the @branch/lambda-http
exports. dev-server.ts, db.ts, auth.ts, validation-utils.ts, swagger-utils.ts
untouched.
- Added @branch/lambda-http as a dependency, regenerated package-lock.json.
- tsconfig.json now includes controllers/**/*.ts.
- Added a route-precedence unit test (literal /users/me vs /users/:userId).
Existing suites pass unmodified.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replace the if-chain in handler.ts with a Route[] table dispatched via @branch/lambda-http's dispatch(). handler.ts is now a one-liner; route logic moved into controllers/donors.ts (GET/POST /donors, DELETE /donors/:id) and controllers/donations.ts (GET/POST /donors/donations, DELETE /donors/donations/:id), in the same order as the original if chain. No services/ layer added, per this lambda's existing shape. Local json() removed in favor of the shared one; auth stays manual (authenticateRequest + custom 401/403 messages) since this lambda's authorization messages don't match @branch/lambda-http's generic requireAuth reasons. ROUTES-START/END markers moved into routes.ts, now bracketing the route table entries. Added @branch/lambda-http as a dependency and regenerated package-lock.json. Added one test asserting GET /donors/donations reaches the donations controller rather than a donor-id route. No behavior change: same status codes, messages, and validation order. Verified via tsc --noEmit and jest (--runInBand to avoid DB contention with sibling lambda test runs): 51 passed, 1 pre-existing failure (health test requires a live dev-server on :3000, fails identically on main).
Replaces the if-chain in handler.ts with a Route[] table (routes.ts) and
one RouteHandler per route (controllers/reports.ts). handler.ts is now
a thin `dispatch(event, { prefix: 'reports', routes })`.
- Local json() and the local async requireAuth() are gone; dispatch
provides json/OPTIONS/health/404/500 centrally, and
createAuthGuard(authenticateRequest) replaces the local requireAuth,
preserving its exact 401 "Authentication required" message.
- Route order preserved from the original if-chain: POST /generate and
GET /upload-url stay ahead of the /:id pattern they'd otherwise be
swallowed by (both are 2-segment paths, same as /reports/:id).
- REPORT_ID_ROUTE/REPORT_DOWNLOAD_ROUTE's \d+ constraint is now an
explicit numeric check in getReport/deleteReport/downloadReport, so a
non-numeric :id still falls through to the same 404 instead of being
looked up as a report id.
- report-service.ts is untouched; controllers still parse/validate/
respond and delegate to it. S3 presigning stays in the controllers,
matching where it lived in handler.ts.
- Added two route-precedence unit tests (GET /reports/upload-url and
POST /reports/generate each reach their own controller, not
/reports/:id or the generic POST /reports controller).
- ROUTES-START/END markers moved into routes.ts around the route array.
No behaviour change: same status codes, messages, validation order,
and S3/TTL values as before.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ambda-http
Replaces the if-chain in handler.ts with dispatch() + a routes.ts table,
matching the shared @branch/lambda-http package adopted repo-wide.
- handler.ts: now just `dispatch(event, { prefix: 'expenditures', routes })`.
- routes.ts: ordered Route[] table, ROUTES-START/END markers preserved
around the entries. Route order matches the original if-chain, notably
keeping /expenditures/upload-url before /expenditures/:id.
- controllers/expenditures.ts: one RouteHandler per route — validates
input, calls the service layer, shapes the response. Same status codes,
messages, and validation order as before.
- services/expenditures.ts: Kysely queries, S3 presigning, and
receiptKeyFromUrl, unchanged in behavior, just relocated.
- Local json()/requireAuth() dropped in favor of the @branch/lambda-http
exports (requireAuth's ADMIN gate on PATCH /expenditures/:id/status is
now backed by the real @branch/lambda-auth checkAuthorization instead
of the handler-local wrapper; identical logic).
- package.json: added @branch/lambda-http as a dependency;
package-lock.json regenerated via `npm install --legacy-peer-deps`.
- test/expenditures.unit.test.ts: added a route-precedence regression
test for GET /expenditures/upload-url vs /expenditures/:id.
No behavior change: same status codes, response shapes, S3 TTLs, and
content-type restriction as the previous if-chain.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adopts @branch/lambda-http: handler.ts is now a one-line dispatch() call over routes.ts's ordered Route[] table, keeping the CLI ROUTES-START/END markers around the table entries. Moved, not changed: - controllers/auth.ts: login, respond-challenge, refresh, me, logout - controllers/register.ts: register, verify-email, resend-code - controllers/password.ts: forgot-password, reset-password - services/cognito.ts: cognitoClient, USER_POOL_CLIENT_ID/ID, CHALLENGE_SPECS, authResultResponse, challengeResponse, mapCognitoAuthError, validatePassword (byte-identical rules) Local json()/parseBody() deleted in favor of @branch/lambda-http's versions (identical implementations). Route order matches the original if-chain exactly. No status codes, response bodies, message strings, Cognito calls/params, password rules or auth gates changed. Added @branch/lambda-http as a dependency and regenerated package-lock.json; extended tsconfig include for controllers/services.
Adopts @branch/lambda-http's dispatch() in place of the hand-rolled if/endsWith routing. handler.ts is now a one-liner; routes.ts holds the ordered Route[] table (bracketed by the ROUTES-START/END markers, now here instead of handler.ts). Split the 769-line handler into: - controllers/projects.ts: list, get, create, update, delete - controllers/dashboard.ts: dashboard, overview - controllers/members.ts: members, assignable-staff - controllers/donors.ts: project donors - controllers/expenditures.ts: project expenditures - services/projects.ts: loadProjectAggregates, syncMemberships, findUnknownUserIds, isProjectActive, indexByProject, projectIdFrom, toIsoDate (the former bottom-of-file helpers) Route order preserves the original if-chain exactly: /projects/dashboard and /projects/assignable-staff are declared before /projects/:id since dispatch is first-match-wins and both are same-length path shapes that :id would otherwise swallow. auth.ts, db.ts, validation-utils.ts, swagger-utils.ts and dev-server.ts are untouched. Each handler now authenticates via createAuthGuard(authenticateRequest) at the AUTHENTICATED level, replicating the old global "authContext.isAuthenticated && authContext.user" gate exactly (same 401 body). Domain authorization (canAccessProject/canEditProject/ canCreateProject/canDeleteProject/canListAssignableStaff) is unchanged and still runs per-route from the untouched auth.ts. No behaviour change: same status codes, messages, validation order, SQL and transaction boundaries. The known dashboard anti-pattern (bulk expenditure select + JS bucketing) is moved verbatim, not fixed. Added a route-precedence regression test asserting GET /projects/dashboard reaches the dashboard controller rather than GET /projects/:id. Adds @branch/lambda-http as a dependency (file: to shared/lambda-http) and regenerates package-lock.json. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # .github/workflows/lambda-deploy.yml # .github/workflows/lambda-tests.yml
# Conflicts: # apps/backend/lambdas/donors/handler.ts
# Conflicts: # apps/backend/lambdas/reports/handler.ts
# Conflicts: # apps/backend/lambdas/expenditures/handler.ts
# Conflicts: # apps/backend/lambdas/projects/handler.ts
The six lambdas moved from if-chain handlers to a declarative Route[]
table (@branch/lambda-http's dispatch()), but the generator and docs
still assumed the old shape. Bring them in line:
- lambda-cli.js: init-handler scaffolds a thin dispatch() handler.ts,
an empty routes.ts, and db.ts; add-route inserts a routes.ts entry,
scaffolds a controllers/<service>.ts stub, and updates openapi.yaml
(auto-prefixing the path with the service name, {param} -> :param).
list-routes/checkSimilarRoutes now read routes.ts instead of parsing
if-conditions out of handler.ts.
- openapi.yaml (all six): normalized to full-prefixed paths matching
each routes.ts table; added donors' missing GET /donations and
expenditures' missing GET /upload-url + GET /{id}/receipt. Stripped
redundant service-prefix suffixes from `servers:` entries so paths
don't double-prefix through the dev-server's swagger UI.
- AGENTS.md (root, backend, lambdas): document @branch/lambda-http and
the routes.ts/controllers/services layout; drop the stale if-chain
handler pattern and normalizedPath codegen references.
- READMEs regenerated via `generate-readme`.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
nourshoreibah
added
no-review
nourshoreibah
had a problem deploying
to
preview
August 22, 2026 23:25 — with 
GitHub Actions
Failure
GitHub Actions
Failure
Contributor
|
🌿 ⏳ Creating preview environment… (logs) |
Contributor
|
🌿 Preview environment — failed ❌ to create. See the workflow logs. |
The lambda declares @branch/lambda-http as a file: dependency, but the Dockerfile only copied and built shared/lambda-auth, so npm install inside the image resolved a path that was never copied and `make up` failed at build time. Mirrors the existing lambda-auth stage, placed after it: lambda-http resolves lambda-auth as file:../lambda-auth and consumes its dist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Preview deploys were failing at esbuild with "Could not resolve @branch/lambda-http" while lambda-tests and lambda-deploy were green. Three workflows each encoded their own copy of "build the shared packages a lambda depends on before packaging it", and adding @branch/lambda-http updated only two of them. preview-env.yml still built lambda-auth alone, so the lambda's npm ci installed a file: dependency whose dist had never been built and the bundle could not resolve the import. Replaces all of it with .github/actions/build-shared-packages, used by lambda-tests (test + shared-http), lambda-deploy (build) and preview-env (deploy). Build order lives in one place now: lambda-http declares lambda-auth as file:../lambda-auth and compiles against its dist, so it goes second. The next shared package added is the actual test of this: one edit instead of four, with no fourth copy left to forget. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ctor/lambda-users
…nversion
The lambda-readme workflow regenerates every README and pushes the result, and
it ran the old CLI against a converted lambda: extractRoutesFromHandler parses
if-conditions out of handler.ts, which is now four lines, so it found no routes
and auto-committed a README with all five of users' endpoints deleted.
- extractRoutesFromHandler now prefers a sibling routes.ts and falls back to the
if-chain parse, so it reads converted and unconverted lambdas alike. That
matters inside this stack, where only some lambdas have been converted at any
given commit.
- collectRoutes lists health once, under the service prefix for a converted
lambda and bare otherwise, instead of hardcoding /health and duplicating a
spec entry that spells it the other way.
- users/openapi.yaml is normalized to match: paths carry the /users prefix and
servers is the bare host. It previously contradicted itself -- servers ended
in /users AND the /users path was prefixed, so Swagger built /users/users,
while /{userId} had no prefix at all.
The fuller CLI rebuild lands in 8/8; this is the subset needed for the README
workflow to stop rewriting these files as each lambda converts.
expenditures/README.md picks up a POST /expenditures row: pre-existing drift on
main that the workflow would have auto-committed anyway.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lambda declares @branch/lambda-http as a file: dependency, but the Dockerfile only copied and built shared/lambda-auth, so npm install inside the image resolved a path that was never copied and `make up` failed at build time. Mirrors the existing lambda-auth stage, placed after it: lambda-http resolves lambda-auth as file:../lambda-auth and consumes its dist. README regenerated so the lambda-readme workflow has nothing to push. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lambda declares @branch/lambda-http as a file: dependency, but the Dockerfile only copied and built shared/lambda-auth, so npm install inside the image resolved a path that was never copied and `make up` failed at build time. Mirrors the existing lambda-auth stage, placed after it: lambda-http resolves lambda-auth as file:../lambda-auth and consumes its dist. README regenerated so the lambda-readme workflow has nothing to push. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
nourshoreibah
removed
the
test-environment
Contributor
|
🌿 Preview environment torn down 🧹 — the stack for this PR has been destroyed. |
nourshoreibah
added
the
test-environment
Contributor
|
🌿 ⏳ Creating preview environment… (logs) |
Contributor
🌿 Preview environment — ready ✅Open: https://d3nmtjoh6ir9ym.cloudfront.net/pr-348/ Shared RDS + Cognito (prod data); DB migrations are not applied here — if this PR adds a migration, endpoints using the new columns will fail until it merges. New commits update this environment in place — a note is posted here on each update. Remove the |
# Conflicts: # apps/backend/lambdas/auth/README.md # apps/backend/lambdas/auth/handler.ts
# Conflicts: # apps/backend/lambdas/auth/README.md
# Conflicts: # apps/backend/lambdas/auth/README.md
Contributor
nourshoreibah
marked this pull request as ready for review
August 23, 2026 00:38
# Conflicts: # apps/backend/lambdas/tools/lambda-cli.js
Contributor
Contributor
|
🌿 Preview environment torn down 🧹 — the stack for this PR has been destroyed. |
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.
Stack 8/8, last one. Base is #347, not
main. No lambda runtime code changes here — this is the code generator, the OpenAPI specs and the docs catching up to the seven PRs beneath it.Without this,
node tools/lambda-cli.js add-routekeeps emittingif (normalizedPath === ...)blocks into ahandler.tsthat no longer has a routing chain, andapps/backend/lambdas/AGENTS.mdkeeps documenting an architecture that no longer exists.CLI
templateHandlerTs/templateHandlerTsCleannow emit the 4-linedispatchhandler, plus newtemplateRoutesTsandtemplateControllerFile. The generated localjson()and the inline OPTIONS/health/404/500 preamble are gone — the shared package owns all four.addRouteToHandler→addRouteToRoutes: inserts a table entry, auto-prefixes the path with the service name, converts{param}→:param, scaffolds a stubRouteHandlerincontrollers/, and keeps the import line inroutes.tsin sync (dedup'd, collision-safe naming). The oldnormalizedPath.split('/')[N]param codegen is deleted outright — params arrive asctx.params.extractRoutesFromHandler→extractRoutesFromRoutes: reads the table fromroutes.tsinstead of parsingifconditions. Deliberately independent of marker placement, becauseprojects/routes.tsbrackets the wholeconst routes = [...]while the other five bracket just the entries.extractRoutesFromOpenApinormalizes{param}→:paramso specs and tables are comparable;checkSimilarRoutes's near-duplicate warning updated for:-prefixed params.list-routesand README generation now source fromroutes.tsonly. Health/OPTIONS/404/500 are dispatch-level, so they are not table entries and not counted.init-handlernever wrote adb.tsthat its own generatedauth.tsimports, so a freshly scaffolded lambda didn't typecheck. AddedtemplateDbTs, pluskysely/pg/@types/pg/@branch/lambda-httpin the generatedpackage.jsonandcontrollers/**/*.tsin the generatedtsconfig.json.OpenAPI normalization
The six conversions deliberately left
openapi.yamlalone, so several drifted from their route tables. All six are now on one convention: paths always full-prefixed,servers:always bare host. Mixing the two is what caused the mess — several specs had the prefix baked intoserversand partially into the paths, so Swagger UI's "Try it out" would double it.Fixed, per service:
auth/login), prefix hidden inserversdonorsservers, paths bare; missingGET /donors/donations(implemented + tested)expendituresGET /expenditures/upload-urlandGET /expenditures/{id}/receipt(both implemented)projects/healthand/dashboardunprefixedreports/healthunprefixed; twoserversentries both suffixed/reportsusers/usersprefixed but/{userId}bareuserswas reported as already-consistent by an earlier pass; it wasn't. Worth noting as a reminder that "verify rather than assume" earned its keep here.Docs
apps/backend/lambdas/AGENTS.md— "Lambda anatomy" and "Handler pattern" rewritten for the route table,RouteCtx/params, and@branch/lambda-http;add-routedocs updated for the new output.AGENTS.mdandapps/backend/AGENTS.md— both omitted@branch/lambda-httpfrom the shared-package inventory ("two packages" → "three").projects/handler.ts:83reference to the dashboard's "JS-loop" aggregation is corrected: that logic is nowcontrollers/dashboard.ts, and it already aggregates in SQL (monthExpr+groupBy), so AGENTS.md's counter-example was doubly out of date. Verified before changing it.lambda-readmeCI check stays green.Verification
init-handler scratchsvc→add-routewith a path param →add-routewith a body →list-routeslisted both correctly →npm install && npx tsc --noEmitexited 0 against the real built@branch/lambda-http. A generator that emits non-compiling code is the main risk here, so this is the check that matters. Duplicate-route detection blocks an exact re-add and warns-but-proceeds on a param-name-only variant.scratchsvcdeleted, confirmed absent fromgit status.list-routesagainst all six real lambdas: projects 11, auth 10, reports 7, expenditures 7, donors 6, users 5 — exact match to the tables. This is the real test of theroutes.tsparser.js-yaml, and comparing all path+verb pairs against every route table shows zero entries in a spec but not a table, or vice versa, for all six services.Left alone
reports/openapi.yamlkeeps a vestigialhttp://localhost:3005server entry that no dev-server code references. Flagged rather than removed — deleting it is unrelated to this PR.🤖 Generated with Claude Code