fix: route lambda sub-paths via API Gateway proxy + shared dispatcher#257
Draft
nourshoreibah wants to merge 7 commits into
Draft
fix: route lambda sub-paths via API Gateway proxy + shared dispatcher#257nourshoreibah wants to merge 7 commits into
nourshoreibah wants to merge 7 commits into
Conversation
The REST API exposed only single-segment resources (/auth, /projects, ...)
with per-method integrations and no {proxy+}, so any sub-path (/auth/login,
/projects/123/members) matched no resource and API Gateway returned 403 — the
lambda never ran. Even bare paths 404'd inside the lambda because handlers were
written for the dev-server's prefix-stripped shape while API Gateway forwards
the full path. Separately, shared file: deps were never bundled into the deploy
zip (latent MODULE_NOT_FOUND).
Changes:
- infrastructure/aws/api_gateway.tf: greedy {proxy+} + ANY per lambda (forwards
full path, routes OPTIONS preflight, fixes missing PUT); deployment redeploy
trigger. Removes the per-method map.
- shared/lambda-http (@branch/lambda-http): dispatch({prefix,routes}) route-table
router with :param matching + path canonicalization (one table works behind
API Gateway's full path and the dev-server's stripped path); centralizes
json()/CORS, OPTIONS, /health, 404, 500.
- All 6 handlers converted to route tables with full prefixed patterns; business
logic preserved.
- esbuild bundling: each lambda's package script bundles handler + shared deps
into one dist/handler.js (@aws-sdk external); CI builds lambda-http. Fixes the
shared-dep packaging gap.
- lambda-cli.js emits route-table entries; openapi specs normalized to full
prefixed paths (+ fixed pre-existing donors dup key / reports YAML colon).
- next.config.ts: dev rewrites no longer strip the service prefix.
- AGENTS.md (root, backend, lambdas, infra, frontend) updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Each service Dockerfile copied + built shared/lambda-auth into /shared so its file: dep resolved; @branch/lambda-http (new runtime dep) needs the same or the container can't resolve it. Add the lambda-http copy+build to the 5 repo-root services, and switch auth's build context to the repo root (it was ./lambdas/auth, which can't see ../../../../shared) with a matching Dockerfile. Verified: docker compose build + up for auth and projects — /auth/health, /auth/login (routed, 400 not 404), /projects health/list/:id/members all 200, unknown path -> 404. Shared dev-server (npm run dev) verified separately. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Auto-formatted .tf files with terraform fmt - Updated README.md with terraform-docs Co-authored-by: nourshoreibah <nourshoreibah@users.noreply.github.com>
Reconcile the shared-dispatcher routing refactor with main's CORS fixes (#277/#279), S3 report uploads, and static-export frontend. Resolution: - Handlers keep the @branch/lambda-http dispatch() architecture. Re-applied main's functional deltas on top: projects PUT uses ProjectValidationUtils .buildUpdateValues (+ JSON guard, reject empty); reports gains POST /reports/generate, GET /reports/upload-url (S3 presign), and POST /reports (record uploaded file), and saveReportRecord now passes title. - api_gateway.tf keeps the branch ANY-on-root + {proxy+} model; ported main's aws_api_gateway_gateway_response.cors so APIGW 4XX/5XX errors carry CORS headers. Dropped main's duplicated proxy resources left by the auto-merge. - next.config.ts takes main's static-export config (dev rewrites obsolete; apiFetch targets service ports / NEXT_PUBLIC_API_BASE_URL directly). - Lambda build scripts keep cjs + --external:@aws-sdk/*; reports retains main's pdfmake font-copy step. - Regenerated lambda package-lock.json files. Verified: all 6 lambdas + frontend typecheck clean; reports unit 44/44, projects unit + dashboard unit pass; terraform fmt + validate succeed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nourshoreibah
added
the
test-environment
Contributor
|
🌿 ⏳ Creating preview environment… (logs) |
Contributor
|
🌿 Preview environment — failed ❌ to create. See the workflow logs. |
…ilds Two issues surfaced packaging lambdas in the preview env: 1. The main merge left a duplicate `esbuild` devDependency in all six lambda package.json files (branch's ^0.25.12 + main's ^0.25.0). JSON tolerates duplicate keys (last wins) so it validated, but esbuild warns and the older pin would win. Kept ^0.25.12, dropped ^0.25.0. 2. `@branch/lambda-http` failed to resolve during `npm run package` because preview-env.yml built shared/lambda-auth but not shared/lambda-http. The shared-package list was hardcoded in three workflows (lambda-deploy, lambda-tests, preview-env), so adding lambda-http meant it silently broke wherever a line was missed. Replace the hardcoded lines with scripts/build-shared.sh, which builds every shared package that declares a `build` script (skips shared/types). Adding a shared package now needs no workflow edits. All three workflows call it. Verified: all six lambdas `npm run package` clean (no duplicate-key warning, lambda-http resolves); the three workflows parse. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Terraform Plan 📖
|
Contributor
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.
Why
Routing logic was duplicated across all six lambda handlers — each hand-rolled its own prefix stripping, OPTIONS/CORS handling, health check, and
if-chain routing. This PR extracts that into one shared dispatcher so handlers only declare routes, and reconciles the refactor with the CORS, S3-upload, and static-export work that landed onmainin the meantime.What
shared/lambda-http(@branch/lambda-http) —dispatch(event, { prefix, routes }): a route-table router with:parammatching and path canonicalization, so one table works behind API Gateway (full path) and the dev-server (prefix-stripped). Centralizesjson()/CORS, OPTIONS preflight,/health, 404, and 500.infrastructure/aws/api_gateway.tf—ANYon the bare resource +{proxy+}per lambda (no per-method enumeration); each lambda owns its routing. Keepsmain'sgateway_response.corsso API-Gateway-generated 4XX/5XX errors carry CORS headers instead of surfacing as opaque browser CORS errors.dist/handler.js(@aws-sdk/*external, provided by the node20 runtime); reports also ships pdfmake fonts.tools/lambda-cli.jsnow scaffolds dispatch-based handlers / route-table entries.Verification
Make sure things look ok in the test environment
Notes
NONE; lambdas keep their own Cognito authz.test-environmentuses the prod DB + Cognito — smoke-test with care.🤖 Generated with Claude Code