fix: moderation queue dedupe, native Lambda alias, prod hardening, and NestJS 12 / Node 26 upgrades - #49
Merged
Conversation
…nary The SAM transform (AWS::Serverless-2016-10-31 plus L1 property overrides) needed a one-time manual `aws lambda delete-alias` and had already required five follow-up fixes (#41, #42, #44, #45, #46) for alias-migration cycles and a silently no-op'd invoke permission. publishCurrentAlias now uses lambda.Alias and, only when a canary is requested, codedeploy.LambdaDeploymentGroup with two rollback alarms (alias Errors, and new-version Errors so an old-version error cannot fail a good deploy). Prod keeps the 10%/5min canary. Dev and the SES forwarder cut over immediately with no CodeDeploy deployment, so dev deploys no longer wait on a canary that provides no value there. The alias is named `current`, not `live`: the SAM-managed `live` alias still exists on the deployed functions, and CloudFormation creates before it deletes, so reusing the name would 409 and bring the manual step back. Also drops the BTFP_INVOKE_ALIAS env var (only existed to dodge "version exists") and the fromFunctionAttributes(sameEnvironment) workaround; a real Alias grants invoke permission natively. Adds the repo's first CDK tests (vitest + aws-cdk-lib/assertions) covering the alias, the canary group and alarms, and the absence of the SAM Transform. Deploy note: `cdk diff BtfpDev/Api` shows BffFunction as a replacement (SAM Serverless::Function -> Lambda::Function). The function has an auto-generated name, so CloudFormation creates the new one first. The log group name will change. Prod's first deploy of this is not a canary (new alias on a new function), so review the prod-diff job before approving.
buildMycotaAuthConfig fell back to 'change-me-in-local-env' for JWT_SECRET, and enableCors fell back to origin: true (reflect any origin) with credentials enabled. In a deployed stage where the env var was ever unset (CI defaults the JWT secrets to REPLACE_BEFORE_DEPLOYING_* placeholders), that meant silently running with a publicly known signing secret, or an open credentialed CORS policy. Outside production behavior is unchanged. In production a missing or placeholder JWT_SECRET / WEB_ORIGIN now throws at startup, so a bad deploy fails loudly (and trips the prod canary alarm) instead of running insecurely. Verified before changing: both deployed stages currently have a real JWT_SECRET and WEB_ORIGIN set, so this does not affect a running stage.
… thing Turn on forbidNonWhitelisted so an undeclared field is a 400 instead of being silently stripped. Silent stripping is the bug class behind the repeated POST /contributions failures (#22-#26), and it was hiding a second, real bug: SubmitPage put `thingId` inside the contribution payload rather than passing it as api.submitContribution's second argument. CreateThingDto has no thingId, so whitelist stripped it and every "edit an existing entry" was proposed as a brand-new thing with no link to the entry being edited (relying only on a name match in findDuplicate). The page now passes editingId as the top-level thingId the DTO actually declares. Audited every body-accepting endpoint against what the web app really sends (email sign-in, professional verification, quiz, contributions); they all match their DTOs exactly, so strict mode is safe. The MCP controller takes a union type and is not validated by the pipe. The pipe options now live in one exported constant used by both app.ts and the DTO specs, replacing a hand-copied config in the spec that could drift. New specs cover a missing payload, a thingId nested in payload, and unknown top-level fields, plus the valid edit shape. Not exercised in a browser: the edit form needs a signed-in session and a running backend. Covered by the DTO contract tests and typecheck instead.
Diagnosing the last prod-shaped incident from CloudWatch was slow: stack traces pointed at minified lambda.js:687:76249, Nest log contexts were mangled to "[e]", and every stack was split across separate log events. In production the BFF now logs one JSON object per line (level, time, requestId, context, message, stack) via JsonLogger, so a stack stays inside one event. The Lambda handler runs each invocation inside an AsyncLocalStorage context carrying awsRequestId, so every line, including cold-start logs, is tagged with its request. Local dev keeps Nest's readable console logger; LOG_LEVEL=debug enables debug lines. The bundle now keeps class names (Nest prints them as the log context) and the image ships lambda.js.map so NODE_OPTIONS=--enable-source-maps can map frames back to src/*.ts (the env var is set in the follow-up infra change). Verified against the built bundle in production mode: JSON lines with the right request ids and readable contexts, one event per error, source-mapped frames, and a placeholder JWT_SECRET or missing WEB_ORIGIN aborts startup.
CLAUDE.md held the detailed procedure while .cursor/rules/sync-before-code-changes
deferred to it ("Follow CLAUDE.md"), so the same guidance lived in two places.
Move all of it into Cursor rules split by topic and reduce CLAUDE.md and the new
AGENTS.md to pointers.
- sync-before-code-changes (always): the fetch/compare/PR-merged decision
checklist and the merge/rebase guard, now self-contained.
- recover-from-merged-branch: the stash-and-restart sequence. Corrected while
moving it: the old order (`git checkout main` then `git fetch . origin/main:main`)
fails because git refuses to fetch into the checked-out branch, so main is now
fast-forwarded before switching to it.
CLAUDE.md imports both with @ so Claude Code still loads them every session;
AGENTS.md indexes all rules (including bff-typescript-cdk-best-practices) for
other tools and points at .claude/skills and the sibling mycota repo.
…that waits on mycota Everything that does not depend on the @bubltec/mycota-* packages moves to the registry's latest. NestJS, Fastify, @fastify/*, class-validator and mycota itself are deliberately held back: mycota 0.6.0 pins NestJS 10 as a hard dependency, so the app cannot move first without ending up with two Nest copies. That set is prepared separately and lands once mycota's Nest 12 release is published. Major bumps: - TypeScript 5.9 -> 7.0, vitest 2 -> 5, vite 6 -> 8, @vitejs/plugin-react 4 -> 6, oxfmt 0.59 -> 0.68, lefthook 1 -> 2, esbuild 0.24 -> 0.28, @types/node 26.6.2 - aws-cdk-lib 2.261 -> 2.270, aws-cdk, constructs, aws-sdk clients, playwright, react 19.3, react-router, tailwind, oxlint, turbo, tsx - @base-ui-components/react (deprecated, renamed) -> @base-ui/react 1.8.0 Changes the new toolchain required: - packages/config/tsconfig-node.json: "types": ["node"]. TypeScript 7 no longer auto-includes every @types/* package, so process/console/fetch/URL were unresolved in every Node workspace. - Five web components: import path @base-ui-components/react -> @base-ui/react. No API or type changes were needed. - data/seed/src/scrape-vetmeds.ts: one line wrapped by the newer oxfmt. Verified: typecheck (10 tasks), build, lint, format, and unit tests (137 across bff, scraper, shared-types, seed, infra) all pass. The e2e workspace is not part of this check (CI excludes it; it needs a browser and a deployed target). aws-cdk-lib: synthesized every stack before and after and diffed the templates. Data, DNS, CI and Scraper are unchanged apart from the CDKMetadata version stamp. Api Version/Alias hashes differ only because the bundle was rebuilt by the new compiler. Each Web stack's BucketDeployment handler picks up the library's newer bundled code. No unexpected infrastructure changes. base-ui: verified in a browser against a mock API. Select (opens, options, selection updates the app), Combobox (typing filters to one option, popup-open state), and Dialog (opens centered with backdrop, focus enters the input, Escape closes and focus returns to the trigger) all behave correctly with no console warnings.
- Add .npmrc with save-exact=true and `saveExact: true` in pnpm-workspace.yaml. Both are needed: tested against pnpm 11.20, `.npmrc` alone is ignored (a bare `pnpm add x` still wrote a caret range) while the workspace setting works. The .npmrc keeps npm and older pnpm consistent. This affects new additions; the existing ranges are pinned in the follow-up upgrade change. - CI and deploy workflows use `node-version-file: .nvmrc` instead of a hard-coded `node-version: 26` (7 places), so the Node version is changed in one place.
…ode in shared-types Follows the save-exact setting: convert the existing caret ranges in all package.json files (89 specs) to exact versions. Each one is pinned to the version that is actually installed (read from `pnpm ls`), not to the floor of its old range. That matters: stripping the caret would have silently downgraded four packages (@nestjs/common, @nestjs/core and @nestjs/platform-fastify to 10.4.0, and class-validator to 0.14.1, instead of the installed 10.4.22 and 0.14.4). No resolved version changes from the pinning itself: the lockfile diff for it contains only `specifier:` lines. Also fixes a latent bug from the TypeScript 7 upgrade: packages/shared-types extends the Node tsconfig preset (which now sets "types": ["node"]) but never declared @types/node. It only compiled because a hoisted copy happened to exist at the repo root, so the previous commit passed by luck and failed after a clean reinstall (TS2688 "Cannot find type definition file for 'node'"). It is now an explicit devDependency. Verified from scratch: wiped every node_modules, `pnpm install --frozen-lockfile`, then typecheck, build and test across all workspaces except e2e (16 tasks).
The rule I wrote earlier praised things that turned out to be weak and did not cover what changed since. Corrections: - lambda-canary.ts is no longer "the model": the SAM transform it used was replaced by a native alias and deployment group, and the rule now says why (five follow-up fixes and a manual migration step). The alias is `current`, not `live`. - SearchService.invalidate() only clears the cache in one container; stated. - Validation: the pipe options are one exported constant, and unknown fields are a 400 (forbidNonWhitelisted) rather than silently dropped. - Fail-closed configuration is now enforced (requireInProduction, corsOrigin), not just advised. - CDK tests exist now; "verification" says so, and requires a before/after synth diff for aws-cdk-lib bumps. Added: logging and alerts, the TypeScript 7 `types`/@types/node requirement, exact-pin policy (pin to the installed version, never strip the caret), the mycota peer-dependency and "upgrade mycota first" workflow, the single Fastify override, Node from .nvmrc, the pnpm 11 quirks (.npmrc save-exact is ignored; the env prefix is pnpm_config_), and four new pitfalls, including the ESM-first `import.meta.url` bundle crash that no unit test could catch. Also a Known Deviations section listing what the repo still does not do (plaintext JWT/Brave env vars, actions pinned by tag not SHA, default oxlint rules, full-table Scan, single AWS account, DTO/shared-types duplication) and the two pieces of work that are written but not yet committed or released.
The root `engines` still said >=24 although .nvmrc, CI, the Lambda image and @types/node are all Node 26. Raise it to >=26 and set `engineStrict` so a wrong Node version fails the install instead of only warning. Only `engineStrict` in pnpm-workspace.yaml is honored by pnpm 11; `engine-strict` in .npmrc is ignored. Verified: install under strict engines, then typecheck, build and test across all workspaces except e2e.
…ta 1.0.0) Bumped, all as exact pins: - @nestjs/common, core, platform-fastify 10.4 -> 12.0.3; @nestjs/jwt 12.0.2, @nestjs/passport 12.0.0, @nestjs/config 3.3 -> 12.0.0, @nestjs/swagger 8 -> 12.0.1 - fastify 4 -> 5.12.5, @fastify/cookie 9 -> 11.1.2, @fastify/aws-lambda 4 -> 6.4.1 - class-validator 0.14 -> 0.15.1 - @bubltec/mycota-auth, -dynamo, -professional-verification, -cdk -> 1.0.0 mycota 1.0.0 declares NestJS and class-validator as peer dependencies, so this app owns the single copy. Confirmed in the regenerated lockfile and with `pnpm why`: one @nestjs/common@12.0.3, one fastify, one class-validator, no Nest 10 entries, and every mycota package resolves to the published 1.0.0. Two things this upgrade required: 1. Bundle crashed at startup. @nestjs/swagger 12 is ESM-first and runs `createRequire(import.meta.url)` at load time; esbuild turns import.meta into an empty object in our CommonJS bundle, so it threw and the Lambda could not start (every cold start would have failed). Typecheck and unit tests cannot catch this. build-lambda.mjs now defines import.meta.url via a banner. The UI asset lookup after it only runs from SwaggerModule.setup, which we never call. 2. Two Fastify copies. @nestjs/platform-fastify pins fastify to an exact version as a regular dependency, so the app's own fastify installed a second copy and the @fastify/cookie plugin types stopped lining up (TS2345). A workspace override keeps a single copy; keep it equal to apps/bff's fastify version. No application code changes were needed. Verified from a wiped node_modules with `pnpm install --frozen-lockfile` against the published packages: - typecheck, build, lint, format and unit tests pass (16 turbo tasks, twice) - the real production Lambda bundle, driven with real requests: swagger document generates (200); mycota's JwtAuthGuard and VerifiedGuard resolve through DI and reject anonymous callers (401); mycota's own DTO is validated by class-validator (400 "email must be an email"), which only works with a single copy; strict forbidNonWhitelisted applies to that DTO too (400); unknown routes 404; no error-level log events - cdk synth: every stack identical to before except the Api stacks' image-derived Version/Alias hashes
`pnpm build` in apps/bff compiles into .tsc-out (see scripts/build-lambda.mjs) while turbo runs the tests in parallel. Vitest 3+ no longer excludes build output, so it collected half-written .tsc-out/*.spec.js copies and failed with "Cannot find module". It had only passed before by timing. Reproduced by running the tests while a build was in progress (failures), and confirmed fixed the same way (8 files, 37 tests pass). Also two forced full turbo runs, both 16/16. apps/bff/vitest.config.ts excludes .tsc-out and dist*.
Its comment described the old SAM/imported alias, where CDK skipped addPermission because the alias ARN was a token, so the HTTP API had no invoke permission. With the native lambda.Alias, HttpLambdaIntegration already creates that permission itself, so this was a second, duplicate resource policy statement on the same alias. Verified by synthesizing: Dev and Prod each still have exactly one AWS::Lambda::Permission for the API, and it targets the current alias.
jjosef
added a commit
that referenced
this pull request
Sep 20, 2026
…e for agent instructions (#50) * docs: fix statements made stale by the alias, Nest 12 and tooling changes Reviewed docs/* and README.md against what PR #49 changed. Actively wrong, now fixed: - ci-cd.md "Known accepted risks" still described the SAM setup (`DeploymentPreference`, the `live` alias, dev on `AllAtOnce`). It now describes the native canary, both rollback alarms, and the new prod 5xx alarm (which only notifies, it does not roll back). - contributing.md described the mycota release rule as "no marker = next prerelease, marker = real release". mycota's CI actually publishes a real release by default and reads the whole squash message; corrected, with a pointer to mycota's release-on-merge rule. - contributing.md told people to test unpublished mycota changes with `pnpm link`. mycota now declares Nest and class-validator as peer dependencies, so a linked package resolves its own copy and the app gets two (broken DI, dead validation). Replaced with the pack-and-override procedure that was validated for the upgrade. - contributing.md said `pnpm turbo run typecheck build test` matches CI; CI passes `--filter=!@btfp/e2e`, and without it e2e fails locally. Fixed. - README.md called packages/config an ESLint config (it holds tsconfig presets) and omitted apps/scraper and apps/e2e. Missing, now documented: - Node 26 from .nvmrc with engineStrict (prerequisites), and CI reading .nvmrc. - The exact-pin policy, including the "pin to the installed version, never strip the caret" rule and the pnpm 11 `.npmrc` quirk. - Strict request validation (unknown fields are a 400). - infra.md: how each secret reaches the Lambda (GitHub client secret from SSM, JWT and Brave from deploy-time env vars) and that production now refuses to start with a missing or placeholder JWT_SECRET / WEB_ORIGIN; how to find the generated log group and follow one request by requestId; and that the function's original /aws/lambda group is left behind. That last point was verified against the deployed dev stack, which also showed the function was NOT replaced by the alias migration (same physical name); logging simply moved to the new explicit group. - The upgrade order: mycota first, then the app. - .cursor/rules as the source of truth for agent instructions. Checked: every internal anchor and referenced path exists, mycota-pull.sh does pin exact versions, and no stale SAM/AutoPublishAlias/DeploymentPreference terms remain. * docs: tell the API-endpoint skill about strict request validation Unknown request fields are now a 400 (forbidNonWhitelisted), and a required nested object needs @isdefined() as well as @ValidateNested(). The skill for adding an endpoint did not mention either, so a new endpoint written from it could ship a DTO that rejects what the client sends, or one that 500s on a missing nested body. Points at VALIDATION_PIPE_OPTIONS and the existing DTO spec as the pattern to copy. * docs: move the task playbooks into .cursor/rules; .claude and CLAUDE.md only point The three playbooks (adding an API endpoint, an infra resource, a thing type) were the last instructions living outside .cursor/rules. Move them there as agent-requested rules, so .cursor/rules is the single source of truth. - .cursor/rules/adding-{an-api-endpoint,an-infra-resource,a-thing-type}.mdc hold the content unchanged, with Cursor frontmatter (description, alwaysApply: false). Their ../../docs links still resolve (same directory depth). - .claude/skills/*.md are now one-file pointers, so Claude Code still lists the playbooks as skills but there is nothing to keep in sync. - AGENTS.md indexes all six rules; CLAUDE.md points at the three playbooks. - README, docs/contributing.md and the architecture rule no longer describe .claude/skills as where instructions live. .claude/launch.json stays: it is configuration for the browser-preview tool, not instructions, and is read from that exact path. .claude/settings.local.json is a local, gitignored permissions file. Checked: every pointer target exists, every rule is in the AGENTS.md index, and the @-imports in CLAUDE.md resolve. Git records the moves as add + modify rather than renames because the stubs reuse the old paths.
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.
What this contains
This branch started as the moderation-queue dedupe work and grew. It is organised as one commit per concern, so it can be reviewed (or split) commit by commit.
Moderation queue / contributions (
ea4e146, pre-existing)shared-types, scraper and ModerationPage updates.Infra
ea4ea8bReplace the SAMAutoPublishAliastransform with a nativelambda.Alias+LambdaDeploymentGroup. Alias is namedcurrent(notlive). Prod keeps the 10% / 5 min canary with two rollback alarms; dev flips instantly with no CodeDeploy wait. Adds the repo's first CDK tests.HttpApiInvokeAliasCfnPermissionworkaround is removed: the native alias gets its invoke permission from the HTTP API integration (synth shows exactly one per stack, on thecurrentalias).7dd9b4bExplicit log groups with retention (14d dev / 30d prod),NODE_OPTIONS=--enable-source-maps, and a prod 5xx alarm that emails via SNS.BFF hardening
073b47bProduction fails closed: a missing or placeholderJWT_SECRET/WEB_ORIGINnow aborts startup instead of falling back tochange-me/ reflect-any-origin CORS.39cb35bforbidNonWhitelistedis on, so unknown fields are a 400. This exposed a real bug: the edit flow putthingIdinside the payload, so edits were silently proposed as new things. Fixed inSubmitPage.636e882One-JSON-object-per-line logging with the Lambda request id, readable class names, and source-mapped stack traces.Dependencies and tooling
99f6562Upgrade to registry-latest (TypeScript 7, vitest 5, vite 8, aws-cdk-lib 2.270, ...).@base-ui-components/react(deprecated) replaced by@base-ui/react1.8.0.d7cb73b,80334b5,1a81129save-exact(.npmrcandsaveExact, since pnpm 11 ignores.npmrc), every dependency pinned exactly, Node 26 enforced viaengines+engineStrict, workflows usenode-version-file: .nvmrc. Also declares the missing@types/nodeinshared-types.e24ab4cNestJS 12, Fastify 5, class-validator 0.15, on the published@bubltec/mycota-*1.0.0.a4af2f7Stop vitest collecting the Lambda build's temporary.tsc-out(a latent flaky failure).Agent docs
640357b,f15ff99.cursor/rules/is now the source of truth for agent instructions;AGENTS.md/CLAUDE.mdpoint at it.Please read before deploying
Serverless::Functionbecomes a plainLambda::Function; the name is auto-generated so CloudFormation creates the new one first). The log group name changes, and prod's first deploy of the new alias is not a canary. Read theprod-diffjob before approving.node_modules, and by driving the real production Lambda bundle with real requests. That caught a startup crash (@nestjs/swagger12 usesimport.meta.url, which esbuild empties in CJS output) that no unit test could. Fixed inbuild-lambda.mjs.BtfpEmailis not deployed by CI. It already has undeployed changes (zip to container image), and this PR adds an alias to it. Nothing here deploys it.aws-cdk-lib2.261 to 2.270 was checked by diffing all synthesized templates before and after: only the Api stacks' image-derived Version/Alias hashes changed.Known follow-ups (not in this PR)
JWT_SECRETand the Brave key are still plaintext Lambda env vars (only the GitHub client secret uses SSM)..cursor/rules/bff-typescript-cdk-best-practices.mdc.Verification
node_modules+pnpm install --frozen-lockfile: typecheck, build, lint, format and unit tests pass (16 turbo tasks, run twice). The e2e workspace is excluded from that (CI excludes it too).base-uiSelect, Combobox and Dialog were exercised in a browser against a mock API.