[seanspeaks]

Summary

Gets Frigg CI green on next (runbook G0). On next, CI had failed on every recent run. Core's suite never ran, and devtools had about 30 failing suites. The tests had gone stale while CI stayed red, and a few real bugs were hiding underneath.

Mongo root cause and fix

• Root cause. mongodb-memory-server 8.x defaults to MongoDB 5.0.19. Its only Linux builds target Ubuntu 20.04 and link OpenSSL 1.1. ubuntu-latest is now 24.04, which has no libcrypto.so.1.1. Core's jest globalSetup started that server before every run, so a failed start (StdoutInstanceError) meant none of core's ~200 suites ran.
• Fix, in two parts:
  1. Core's globalSetup no longer starts MongoDB at all. Nothing in the default suite used it; repository tests mock Prisma. It keeps only the STAGE=dev default. Unit suites must not need a database.
  2. @friggframework/test now uses mongodb-memory-server 11. TestMongo pins MongoDB 8.0.17, which has native ubuntu2404 builds; MONGOMS_VERSION still overrides it. TestMongo stays as an opt-in helper.
• The lockfile diff is limited to the mongodb-memory-server subtree, its mongodb driver and bson. Unrelated peer-flag churn was left out.

Real-database tests split out (*.db.test.js, npm run test:db)

• The suites that talk to real databases are now named *.db.test.js. test:all excludes them, and a new parallel db job runs them:
  • the three encryption fix-verification suites;
  • the two repository parity suites, which run every adapter (MongoDB, DocumentDB-on-Mongo, PostgreSQL) through the same cases, about 1,400 tests;
  • a TestMongo smoke test.
• The db job provides MongoDB 8.0 as a replica set (Prisma needs one for MongoDB) and a postgres:16 service, with migrations applied and the mongodb-memory-server binary cached.
• Follow-up: per-port repository contract tests. That means one shared suite per repository interface, run against each adapter (Mongo, Postgres, and Mongo standing in for DocumentDB), plus in-memory fakes for use-case tests. The parity suites are the seed of that.
• There is no tenant-isolation suite on next yet. The Postgres service in the db job is what it will use when it lands (runbook DG3); it only needs FRIGG_TEST_POSTGRES_URL, which the job already sets.

Bugs found by the revived tests

• fix(core): API-module encryption.credentialFields were never registered. A wrong relative require path threw, and the error was swallowed at debug level, so those credential fields were stored unencrypted. Fixing the path exposed two more problems, both fixed:

  • a module listing a core field (e.g. api_key) failed validation and dropped every module's fields;
  • module fields replaced the app's own custom schema instead of merging with it.

  Values already stored as plaintext still read back.

• fix(core): QueuerUtil.batchSend reused and cleared one array across every SQS batch command.

• fix(devtools): VpcBuilder attached only the first of vpc.securityGroupIds.

• fix(devtools): VpcBuilder applied legacy VPC options under managementMode: 'managed' even though it warns that it ignores them. Managed + isolated with no stack VPC then failed.

Stale tests updated

Each commit names the change that made the test stale. In short:

• core: encryption registry and DocumentDB envelope format, { externalId, type } API, Boom status codes, 32-byte keys, AWS SDK v3 SQS mock, deterministic timing, and Node 22 warnings in the require-graph probe.
• devtools:
  • composer and postgres tests rewritten for the domain-builder composer (they required modules deleted in 7b36f0f);
  • composer tests no longer run a real npm install into layers/prisma, and template-generation no longer runs real AWS discovery;
  • health, VPC and discovery tests updated to the current shapes;
  • frigg-cli tests: Jest 30, timezone, a hung deploy, a 42 s UI suite now 0.15 s, git identity on Ubuntu;
  • a jest config that stops collecting helper files and management-ui's Vitest suites.

Skips (both describe.skip with a comment and an issue)

Suite	Tests	Reason	Issue
devtools/infrastructure/scripts/build-time-discovery.test.js	23	Its subject requires the deleted aws-discovery module and nothing calls it; decide whether to delete or port it	#708
devtools/.../orphan-detection-relationship-analysis.test.js	4	Red-phase TDD spec for a feature that was never implemented	#709

Also not in this PR:

• Two test files were deleted rather than skipped, because their subjects no longer exist: devtools/frigg-cli/index.test.js and devtools/infrastructure/integration.test.js.
• management-ui's Vitest suites are excluded from devtools' jest. management-ui isn't an npm workspace, so its dependencies are never installed; running them in CI is a follow-up.
• Pre-existing skips are untouched: 2 it.skip in core's requester tests, plus one skipped suite and others in devtools.

Workflow changes (one commit each, so any can be dropped)

• (f) on.push includes next.
• (a) Node matrix is 22.x and 24.x, with fail-fast: false.
• speed: shallow checkout (the full fetch pulled thousands of tags), setup-node npm cache, no global npm reinstall, and newer pushes cancel in-flight runs.
• (b) lint:fix plus git-auto-commit is replaced by a read-only lint job (prettier --check + eslint). It is non-blocking (continue-on-error) because the backlog is large: core alone has about 390 files that need prettier and 96 eslint errors. No bot pushes to PR branches any more.
• DB job: see above.
• (c) test-cli.yml is retired. It failed on every run: wrong path (packages/frigg-cli moved) and Node 18/20/21. Its unit tests already run in test:all. The frigg <cmd> --help checks are now a step in Frigg CI. The Windows/macOS legs and the Codecov gate are dropped.
• (d) codesee-arch-diagram.yml is left as is. CodeSee's action repo turned the action into a no-op and then reverted that; runs still succeed and codesee.io still responds. I couldn't confirm the service is defunct, so that's the maintainer's call.
• release.yml and codeql.yml are untouched.

Review follow-ups

• These follow-ups came out of the adversarial review; the review reply has per-item detail.
  • Encryption registry: errors are no longer swallowed at debug level, and an invalid encryption config fails closed in deployed stages.
  • Prisma encryption path: it no longer mistakes plaintext containing colons for ciphertext, and values keep their type (a numeric string stays a string).
  • CI concurrency: runs are cancelled only for superseded PR pushes, never on next.
  • Route-association tests: the disputed tests are skipped pending devtools: VpcBuilder renames private subnet route-table associations on existing stacks (upgrade risk) #714.
  • MongoDB binary: @friggframework/test now uses mongodb-memory-server-core, so nothing downloads a binary at install time.
  • Smaller CI and docs fixes: CI and TESTING.md nits.
• QueuerUtil.batchSend (cd616a5): this fix only affects what tests and other observers see. The SQS requests were already correct, so it isn't a production fix.
• Not changed here, needs an issue: vpc.selfHeal: false no longer throws when an existing NAT gateway is misplaced. Since 7b36f0f, VpcBuilder silently creates a new, billable NAT gateway instead (vpc-builder.js ~240, ~1706). The removed composer tests covered that guard; the vpc-builder tests cover NAT placement but not this.

Test plan

• npm run test:all passes locally on Node 22 and 24.
• npm run test:db passes against mongo:8.0 (replica set) and postgres:16.
• devtools jest run twice; layers/prisma stays clean.
• Frigg CI green on this PR (build 22.x / 24.x, db, lint).

🤖 Generated with Claude Code

https://claude.ai/code/session_013b2UUFaQ5Lk6TFPqzRr1j7

---

Note

High Risk
Changes touch credential encryption registration, envelope handling, and production startup on invalid encryption config—areas where mistakes could leave secrets plaintext or block deploys; CI workflow changes also gate every next publish.

Overview
Restructures Frigg CI so next can go green: pushes on next, Node 22/24 matrix, PR concurrency cancellation, read-only tokens, a dedicated db job (Mongo replica set + Postgres, npm run test:db), read-only lint (non-blocking), and a frigg --help command-list smoke test; test-cli.yml is removed. Docs describe *.db.test.js and local DB setup.

Core test infrastructure no longer starts in-memory Mongo in global setup (fixes Ubuntu 24 / mongodb-memory-server breakage); real DB suites move to *.db.test.js / jest.db.config.js. Lockfile bumps mongodb-memory-server (and driver stack).

Field-level encryption gets stricter Cryptor envelope detection (legacy plaintext with colons no longer treated as ciphertext), JSON type-preserving encrypt/decrypt, shared isEncryptedEnvelope, and startup fails closed on invalid app encryption config in deployed stages. Module encryption.credentialFields registration is fixed (correct require path, per-field validation, merge with app custom schema, skip core duplicates)—fields that were silently unencrypted should encrypt on new writes; plaintext legacy values still read.

findIntegrationContextByExternalEntityId now takes { externalId, type } (docs/tests updated). QueuerUtil.batchSend copies each SQS batch instead of clearing a shared Entries array. Many stale unit/integration tests are updated (AWS SDK v3 SQS, Boom assertions, timing mocks, etc.).

^{Reviewed by Cursor Bugbot for commit 7cfcf12. Bugbot is set up for automated code reviews on this repo. Configure here.}

📦 Published PR as canary version: 2.0.0--canary.710.7cfcf12.0

✨ Test out this PR locally via:

npm install @friggframework/admin-scripts@2.0.0--canary.710.7cfcf12.0
npm install @friggframework/core@2.0.0--canary.710.7cfcf12.0
npm install @friggframework/devtools@2.0.0--canary.710.7cfcf12.0
npm install @friggframework/eslint-config@2.0.0--canary.710.7cfcf12.0
npm install @friggframework/prettier-config@2.0.0--canary.710.7cfcf12.0
npm install @friggframework/schemas@2.0.0--canary.710.7cfcf12.0
npm install @friggframework/serverless-plugin@2.0.0--canary.710.7cfcf12.0
npm install @friggframework/test@2.0.0--canary.710.7cfcf12.0
npm install @friggframework/ui@2.0.0--canary.710.7cfcf12.0
# or 
yarn add @friggframework/admin-scripts@2.0.0--canary.710.7cfcf12.0
yarn add @friggframework/core@2.0.0--canary.710.7cfcf12.0
yarn add @friggframework/devtools@2.0.0--canary.710.7cfcf12.0
yarn add @friggframework/eslint-config@2.0.0--canary.710.7cfcf12.0
yarn add @friggframework/prettier-config@2.0.0--canary.710.7cfcf12.0
yarn add @friggframework/schemas@2.0.0--canary.710.7cfcf12.0
yarn add @friggframework/serverless-plugin@2.0.0--canary.710.7cfcf12.0
yarn add @friggframework/test@2.0.0--canary.710.7cfcf12.0
yarn add @friggframework/ui@2.0.0--canary.710.7cfcf12.0

[claude]

⚠️ Code review skipped — your organization has no extra usage available to pay for this review.

If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.

Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.

[netlify]

✅ Deploy Preview for friggframework-org canceled.

Name	Link
🔨 Latest commit	7cfcf12
🔍 Latest deploy log	https://app.netlify.com/projects/friggframework-org/deploys/6adc33fdf68b9e75ae55a5b8

[gitguardian]

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.

---

^{_{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}}

[seanspeaks]

Verdict: no blockers. The CI changes and most of the test updates hold up. Fix the two encryption should-fixes before this ships in a release, because the registry fix turns encryption on for fields that have only ever held plaintext.

Reviewed at 9045bbc. Everything below was checked locally, either by running it or by reading the source. Anything I couldn't confirm is labelled.

Blocking

None.

Should-fix

1. Module encryption errors are still swallowed at debug level, so one bad entry leaves every module field unencrypted. packages/core/database/encryption/encryption-schema-registry.js:274, also :149
The root cause in 2a2adff was a require error hidden by logger.debug. That handler is unchanged, so the next failure will be just as quiet. Repro: a module declares encryption: { credentialFields: ['api_key', 42] }. Then field.startsWith throws TypeError: field.startsWith is not a function at :149. loadCustomEncryptionSchema catches it at debug level, and no module field from any module gets registered. The fields are written in plaintext and nothing reaches the logs at the default level. An invalid app encryption.schema has the same effect, because it throws before modules load. Suggested fix:

• log at warn or error when an app definition was found but loading failed;
• validate or skip each module's entries one at a time, so one bad module can't drop the rest.

2. Some plaintext values don't survive the newly registered module fields on the Prisma path. packages/core/database/encryption/field-encryption-service.js:149-155, :239-249
The commit says plaintext values still read back. That holds on the DocumentDB service, which needs exactly 4 base64 parts and more than 50 chars, but not on the Prisma FieldEncryptionService. I checked with an AES Cryptor and a module listing webhook_secret and pin:

• Values with 3 or more colons are treated as ciphertext. Example: 'a:b:c:d', or any URL with userinfo and a port. _isEncrypted only checks split(':').length >= 4, which causes two problems:

  • on write, _shouldEncrypt skips the value, so it stays plaintext even though the field is registered;
  • on read, cryptor.decrypt throws Encryption key not found, and prisma-encryption-extension.js doesn't catch it, so loading the whole Credential fails.

  Before this PR these module fields weren't registered, so such values read fine. After upgrading, they break existing credentials.

• Numeric strings come back as numbers. '123456' is stored encrypted and reads back as 123456, because _deserializeAfterDecryption runs JSON.parse on it. A module that calls string methods on the field will throw.

Both problems already exist for core fields; this PR exposes every module field to them. Suggested fix: share the DocumentDB _isEncryptedValue check, and make the read path keep the value's type, at least for primitives that were strings before encryption.

3. The concurrency group also cancels CI on pushes to next. .github/workflows/frigg-ci.js.yml:15-17
group: ${{ github.workflow }}-${{ github.ref }} with cancel-in-progress: true means two quick merges to next cancel the first commit's run. release.yml publishes a prerelease on every push to next with no CI gate, so that published version never gets a CI result. Suggested fix: cancel-in-progress: ${{ github.event_name == 'pull_request' }}.

4. Two VpcBuilder tests were flipped to match the source on a disputed backwards-compatibility point. packages/devtools/infrastructure/domains/networking/vpc-builder.test.js:839-843, :1606-1611
On next, both tests asserted that a stack already containing FriggSubnet{1,2}RouteAssociation keeps those logical IDs. The second test's comment says "We MUST use these same logical IDs to avoid AlreadyExists errors". The source at vpc-builder.js:1825-1829 always emits FriggPrivateSubnet{1,2}RouteTableAssociation. The PR rewrote both tests to bless the source behaviour. The tests and that source logic came from the same commit (37a354e), so the intent isn't clear from history.

The risk is an EC2 constraint I didn't deploy to confirm. A subnet can have only one explicit route-table association, and CloudFormation creates the new logical resource before deleting the old one. So on a stack whose old associations are intact, the update can fail with Resource.AlreadyAssociated and roll back. Rather than settle this in a CI-green PR, it.skip these two with an issue, or get the 37a354e author to confirm that new IDs are intended.

Nits

• packages/test/mongodb.js:9: the pinned version means two MongoDB downloads. The pin is 8.0.17, but mongodb-memory-server 11's postinstall downloads its own default, 8.2.6. After npm ci, 8.2.6 was in node_modules/.cache/mongodb-memory-server, and TestMongo then downloaded 8.0.17. The pinned version is never pre-fetched, and the default is fetched but never used. That happens:

  • on every CI build and lint run, which aren't cached;
  • in every app, since @friggframework/devtools has @friggframework/test as a runtime dependency.

  Pin to the library default, or set MONGOMS_VERSION / MONGOMS_DISABLE_POSTINSTALL where it matters.

• frigg-ci.js.yml:104-108: the replica-set wait can time out without failing the step. If the node never becomes primary, the loop ends with status 0 (the last command is sleep 1), and the failure only appears later as a confusing prisma db push error. Add a check after the loop. The first loop is fine: if mongosh never answers, rs.initiate fails the step.

• frigg-ci.js.yml:56-58: the CLI smoke loop can't catch a removed command. npx frigg nonexistent-cmd --help exits 0, because commander prints the top-level help. Also, frigg-cli/index.js:78-89 requires every command module eagerly, so frigg --help alone already proves they load. To catch a dropped or renamed command, grep the --help output for each name.

• The build job has no permissions: block. Now that git-auto-commit is gone, add a top-level permissions: contents: read.

• docs/TESTING.md:241-256 is stale. It still says integration tests automatically get an in-memory MongoDB with MONGO_URI. That's no longer true for core; point to *.db.test.js, npm run test:db and opt-in TestMongo.

• cd616a5 calls the batchSend issue a delivery-path bug, but it isn't one. The old in-place splice ran after await sqs.send(...) returned, so SDK middleware and retries saw the right entries; only later observers such as the mock recorder saw 5/5/5. The fix is correct; just don't list it as a production fix in release notes.

• 65014bc says the NAT tests it removed are "covered" in vpc-builder tests, which is only partly true. Never reusing a NAT gateway placed in a private subnet is covered (vpc-builder.test.js:850-896). Throwing when selfHeal: false is not: that guard no longer exists. The current code (vpc-builder.js:240, :1706) silently creates a new, billable NAT gateway. That changed in 7b36f0f, not here, but it deserves an issue.

Tests that no longer run, with verdicts

What	Tests	Verdict
devtools/infrastructure/scripts/build-time-discovery.test.js, describe.skip	23	OK. Its subject requires the deleted aws-discovery. Note that build-time-discovery.js and run-discovery.js still ship and would throw if run (#708).
orphan-detection-relationship-analysis.test.js, describe.skip	4	OK. findOrphanedResourcesWithRelationships doesn't exist (#709).
devtools/frigg-cli/index.test.js, deleted	3	OK. It asserted the pre-#639 install flow, which install.test.js now covers.
devtools/infrastructure/integration.test.js, deleted	9	OK. Its subject, serverless-template and BuildTimeDiscovery plumbing, is gone.
management-ui Vitest suites, excluded by devtools/jest.config.js	4 files	OK as stated, but nothing runs them now.
infrastructure-composer.test.js, 91 → 76 tests	15	Mostly OK. Discovery gating is in resource-discovery.test.js, NAT placement is in the vpc-builder tests, and the CIDR fallback is gone from the source. The exception is the selfHeal throw (nit above).
postgres-config.test.js, 26 → 17 tests	9	OK. Deletion protection, CloudWatch log exports and the VPC/2-subnet errors are no longer in AuroraBuilder.
find-integration-context…test.js: "entity user missing" removed	1	OK. The use case no longer looks up the user (a7bacab).
webhook-flow.integration.test.js: default ON_WEBHOOK log assertion	1	OK. The base handler has been a no-op since b74f21e.
Core *.db.test.js (3 encryption suites, 2 parity suites) and the TestMongo smoke test	—	Moved to the db job, which does run them. I pointed the Postgres URL at a dead port and npm run test:db exited 1, so failures fail the job. There's no continue-on-error.

I found no .only, no raised timeouts and no new passWithNoTests. The other updated assertions match the intended behaviour in source: Boom status codes, the DocumentDB envelope, { externalId, type }, 32-byte keys, Lambda supportsResourceUpdate, the drift PropertyPath shape, the default-SG reuse from 37a354e, and the KMS env-var fallback (kms-builder.js:222-233).

Checked and fine

• Encryption merge. Module fields merge with the app's custom schema, and calling the loader repeatedly gives the same result. A module that lists a core field like api_key is filtered out rather than failing validation. Modules can only add fields: the opt-out comes only from the app's encryption.disable, and nothing reads opt-outs from module definitions.
• VpcBuilder fixes. The securityGroupIds fix is correct and covered (infrastructure-composer.test.js:425-433). The managed-mode legacy fix is correct and covered: reverting 82ddf5c makes vpc-builder.test.js fail.
• Global Mongo removal. No code or unit test in the monorepo relies on MONGO_URI or global.testMongo from core's global setup; prisma.test.js sets its own. @friggframework/test's exports and globalSetup/globalTeardown are unchanged.
• Lockfile. There are no downgrades, and the changes stay within the mongodb-memory-server subtree. The hoisted mongodb moves from 4.17.2 to 7.5.0, but nothing in the workspace requires mongodb.
• Workflows.
  • pull_request: branches: ["**"] covers stacked PRs.
  • continue-on-error on lint applies only to the step.
  • test-cli.yml was path-filtered on packages/frigg-cli/**, which no longer exists, so it only ever ran when the workflow file itself changed. Nothing is lost by deleting it.
  • No secrets are used and no pull_request_target was added.

What I ran

• npm ci on Node 24.10.0 and 22.11.0: both pass. On 22.11 there are only engine warnings that already existed.
• npm run test:all on both Node versions: everything passes. Core has 198 suites (2620 passed, 2 skipped), devtools 90 + 3 skipped (1944 passed, 37 skipped), admin-scripts 278, schemas 129 and serverless-plugin 46.
• npm run test:db on Node 24 against mongo:8.0 (replica set) and postgres:16 in Docker, with migrations and db push applied as the workflow does: core passed 5 suites (1438 tests) and test passed 1 suite.
• The CLI smoke loop.
• Small probes of the encryption registry and service, using a real AES Cryptor.

🤖 Generated with Claude Code

[seanspeaks]

Verdict: no blockers. The CI changes and most of the test updates hold up. Fix the two encryption should-fixes before this ships in a release, because the registry fix turns encryption on for fields that have only ever held plaintext.

Reviewed at 9045bbc. Everything below was checked locally, either by running it or by reading the source. Anything I couldn't confirm is labelled.

Blocking

None.

Should-fix

1. Module encryption errors are still swallowed at debug level, so one bad entry leaves every module field unencrypted. packages/core/database/encryption/encryption-schema-registry.js:274, also :149
The root cause in 2a2adff was a require error hidden by logger.debug. That handler is unchanged, so the next failure will be just as quiet. Repro: a module declares encryption: { credentialFields: ['api_key', 42] }. Then field.startsWith throws TypeError: field.startsWith is not a function at :149. loadCustomEncryptionSchema catches it at debug level, and no module field from any module gets registered. The fields are written in plaintext and nothing reaches the logs at the default level. An invalid app encryption.schema has the same effect, because it throws before modules load. Suggested fix:

• log at warn or error when an app definition was found but loading failed;
• validate or skip each module's entries one at a time, so one bad module can't drop the rest.

2. Some plaintext values don't survive the newly registered module fields on the Prisma path. packages/core/database/encryption/field-encryption-service.js:149-155, :239-249
The commit says plaintext values still read back. That holds on the DocumentDB service, which needs exactly 4 base64 parts and more than 50 chars, but not on the Prisma FieldEncryptionService. I checked with an AES Cryptor and a module listing webhook_secret and pin:

• Values with 3 or more colons are treated as ciphertext. Example: 'a:b:c:d', or any URL with userinfo and a port. _isEncrypted only checks split(':').length >= 4, which causes two problems:

  • on write, _shouldEncrypt skips the value, so it stays plaintext even though the field is registered;
  • on read, cryptor.decrypt throws Encryption key not found, and prisma-encryption-extension.js doesn't catch it, so loading the whole Credential fails.

  Before this PR these module fields weren't registered, so such values read fine. After upgrading, they break existing credentials.

• Numeric strings come back as numbers. '123456' is stored encrypted and reads back as 123456, because _deserializeAfterDecryption runs JSON.parse on it. A module that calls string methods on the field will throw.

Both problems already exist for core fields; this PR exposes every module field to them. Suggested fix: share the DocumentDB _isEncryptedValue check, and make the read path keep the value's type, at least for primitives that were strings before encryption.

3. The concurrency group also cancels CI on pushes to next. .github/workflows/frigg-ci.js.yml:15-17
group: ${{ github.workflow }}-${{ github.ref }} with cancel-in-progress: true means two quick merges to next cancel the first commit's run. release.yml publishes a prerelease on every push to next with no CI gate, so that published version never gets a CI result. Suggested fix: cancel-in-progress: ${{ github.event_name == 'pull_request' }}.

4. Two VpcBuilder tests were flipped to match the source on a disputed backwards-compatibility point. packages/devtools/infrastructure/domains/networking/vpc-builder.test.js:839-843, :1606-1611
On next, both tests asserted that a stack already containing FriggSubnet{1,2}RouteAssociation keeps those logical IDs. The second test's comment says "We MUST use these same logical IDs to avoid AlreadyExists errors". The source at vpc-builder.js:1825-1829 always emits FriggPrivateSubnet{1,2}RouteTableAssociation. The PR rewrote both tests to bless the source behaviour. The tests and that source logic came from the same commit (37a354e), so the intent isn't clear from history.

The risk is an EC2 constraint I didn't deploy to confirm. A subnet can have only one explicit route-table association, and CloudFormation creates the new logical resource before deleting the old one. So on a stack whose old associations are intact, the update can fail with Resource.AlreadyAssociated and roll back. Rather than settle this in a CI-green PR, it.skip these two with an issue, or get the 37a354e author to confirm that new IDs are intended.

Nits

• packages/test/mongodb.js:9: the pinned version means two MongoDB downloads. The pin is 8.0.17, but mongodb-memory-server 11's postinstall downloads its own default, 8.2.6. After npm ci, 8.2.6 was in node_modules/.cache/mongodb-memory-server, and TestMongo then downloaded 8.0.17. The pinned version is never pre-fetched, and the default is fetched but never used. That happens:

  • on every CI build and lint run, which aren't cached;
  • in every app, since @friggframework/devtools has @friggframework/test as a runtime dependency.

  Pin to the library default, or set MONGOMS_VERSION / MONGOMS_DISABLE_POSTINSTALL where it matters.

• frigg-ci.js.yml:104-108: the replica-set wait can time out without failing the step. If the node never becomes primary, the loop ends with status 0 (the last command is sleep 1), and the failure only appears later as a confusing prisma db push error. Add a check after the loop. The first loop is fine: if mongosh never answers, rs.initiate fails the step.

• frigg-ci.js.yml:56-58: the CLI smoke loop can't catch a removed command. npx frigg nonexistent-cmd --help exits 0, because commander prints the top-level help. Also, frigg-cli/index.js:78-89 requires every command module eagerly, so frigg --help alone already proves they load. To catch a dropped or renamed command, grep the --help output for each name.

• The build job has no permissions: block. Now that git-auto-commit is gone, add a top-level permissions: contents: read.

• docs/TESTING.md:241-256 is stale. It still says integration tests automatically get an in-memory MongoDB with MONGO_URI. That's no longer true for core; point to *.db.test.js, npm run test:db and opt-in TestMongo.

• cd616a5 calls the batchSend issue a delivery-path bug, but it isn't one. The old in-place splice ran after await sqs.send(...) returned, so SDK middleware and retries saw the right entries; only later observers such as the mock recorder saw 5/5/5. The fix is correct; just don't list it as a production fix in release notes.

• 65014bc says the NAT tests it removed are "covered" in vpc-builder tests, which is only partly true. Never reusing a NAT gateway placed in a private subnet is covered (vpc-builder.test.js:850-896). Throwing when selfHeal: false is not: that guard no longer exists. The current code (vpc-builder.js:240, :1706) silently creates a new, billable NAT gateway. That changed in 7b36f0f, not here, but it deserves an issue.

Tests that no longer run, with verdicts

What	Tests	Verdict
devtools/infrastructure/scripts/build-time-discovery.test.js, describe.skip	23	OK. Its subject requires the deleted aws-discovery. Note that build-time-discovery.js and run-discovery.js still ship and would throw if run (#708).
orphan-detection-relationship-analysis.test.js, describe.skip	4	OK. findOrphanedResourcesWithRelationships doesn't exist (#709).
devtools/frigg-cli/index.test.js, deleted	3	OK. It asserted the pre-#639 install flow, which install.test.js now covers.
devtools/infrastructure/integration.test.js, deleted	9	OK. Its subject, serverless-template and BuildTimeDiscovery plumbing, is gone.
management-ui Vitest suites, excluded by devtools/jest.config.js	4 files	OK as stated, but nothing runs them now.
infrastructure-composer.test.js, 91 → 76 tests	15	Mostly OK. Discovery gating is in resource-discovery.test.js, NAT placement is in the vpc-builder tests, and the CIDR fallback is gone from the source. The exception is the selfHeal throw (nit above).
postgres-config.test.js, 26 → 17 tests	9	OK. Deletion protection, CloudWatch log exports and the VPC/2-subnet errors are no longer in AuroraBuilder.
find-integration-context…test.js: "entity user missing" removed	1	OK. The use case no longer looks up the user (a7bacab).
webhook-flow.integration.test.js: default ON_WEBHOOK log assertion	1	OK. The base handler has been a no-op since b74f21e.
Core *.db.test.js (3 encryption suites, 2 parity suites) and the TestMongo smoke test	—	Moved to the db job, which does run them. I pointed the Postgres URL at a dead port and npm run test:db exited 1, so failures fail the job. There's no continue-on-error.

I found no .only, no raised timeouts and no new passWithNoTests. The other updated assertions match the intended behaviour in source: Boom status codes, the DocumentDB envelope, { externalId, type }, 32-byte keys, Lambda supportsResourceUpdate, the drift PropertyPath shape, the default-SG reuse from 37a354e, and the KMS env-var fallback (kms-builder.js:222-233).

Checked and fine

• Encryption merge. Module fields merge with the app's custom schema, and calling the loader repeatedly gives the same result. A module that lists a core field like api_key is filtered out rather than failing validation. Modules can only add fields: the opt-out comes only from the app's encryption.disable, and nothing reads opt-outs from module definitions.
• VpcBuilder fixes. The securityGroupIds fix is correct and covered (infrastructure-composer.test.js:425-433). The managed-mode legacy fix is correct and covered: reverting 82ddf5c makes vpc-builder.test.js fail.
• Global Mongo removal. No code or unit test in the monorepo relies on MONGO_URI or global.testMongo from core's global setup; prisma.test.js sets its own. @friggframework/test's exports and globalSetup/globalTeardown are unchanged.
• Lockfile. There are no downgrades, and the changes stay within the mongodb-memory-server subtree. The hoisted mongodb moves from 4.17.2 to 7.5.0, but nothing in the workspace requires mongodb.
• Workflows.
  • pull_request: branches: ["**"] covers stacked PRs.
  • continue-on-error on lint applies only to the step.
  • test-cli.yml was path-filtered on packages/frigg-cli/**, which no longer exists, so it only ever ran when the workflow file itself changed. Nothing is lost by deleting it.
  • No secrets are used and no pull_request_target was added.

What I ran

• npm ci on Node 24.10.0 and 22.11.0: both pass. On 22.11 there are only engine warnings that already existed.
• npm run test:all on both Node versions: everything passes. Core has 198 suites (2620 passed, 2 skipped), devtools 90 + 3 skipped (1944 passed, 37 skipped), admin-scripts 278, schemas 129 and serverless-plugin 46.
• npm run test:db on Node 24 against mongo:8.0 (replica set) and postgres:16 in Docker, with migrations and db push applied as the workflow does: core passed 5 suites (1438 tests) and test passed 1 suite.
• The CLI smoke loop.
• Small probes of the encryption registry and service, using a real AES Cryptor.

🤖 Generated with Claude Code

[seanspeaks]

Thanks for the review. Every should-fix and nit is addressed in the commits listed below. Each code fix comes with a test that fails on the previous source. CI is green at 7cfcf12: https://github.com/friggframework/frigg/actions/runs/40062491876

1. Encryption-registry errors swallowed at debug level (325d9af)

• Module credentialFields are validated one entry at a time. An invalid entry, such as 42, is skipped with a warning that names the module. Valid fields from that module and from other modules are still registered.
• Debug level is now used only when there is no backend or app definition. Once an app definition is found, an invalid encryption.schema, encryption.disable or module schema is a configuration error:
  • in deployed stages it is logged at error level and thrown as EncryptionSchemaError;
  • in dev, test and local it is a warning, and loading continues.
• EncryptionConfigurationError isn't on next yet; feat(core)!: fail closed on field-level encryption in deployed stages #663 adds it.
• prisma.js now rethrows EncryptionSchemaError instead of logging "Continuing without encryption", so startup fails rather than writing plaintext. Other extension-initialisation failures are left to feat(core)!: fail closed on field-level encryption in deployed stages #663.
• Tests: encryption-schema-registry-loading.test.js and prisma-encryption-startup.test.js. 4 of their 5 tests fail on the old source.

2. Plaintext and typed values on the Prisma path (3993629)

• Colons: both paths now share ciphertext-format.isEncryptedEnvelope, the strict check from the DocumentDB service: exactly 4 base64-alphabet parts and more than 50 characters.
  • 'a:b:c:d' and URLs with userinfo and a port are encrypted on write and read back as plaintext.
  • A real envelope that fails to decrypt still throws.
• Types: every value is JSON-encoded before encryption, so '123456' reads back as '123456' and 1234 as 1234.
  • Raw strings written by earlier versions still read back. The exception is a legacy raw string that is itself valid JSON (such as '123456'); it still parses to a number, as it always did.
  • Older readers handle the new format, because JSON.parse('"x"') is 'x'.
• New test: prisma-extension-legacy-values.test.js runs through the extension with a real AES Cryptor; 3 of its 5 tests fail on the old source. The unit-test mock cryptors now emit the real envelope shape.
• The DocumentDB path still stores raw strings, so its numeric-string ambiguity remains. Out of scope here.

3. Concurrency on next (be66998)

• cancel-in-progress: ${{ github.event_name == 'pull_request' }}, so pushes to next (which release.yml publishes) are never cancelled.

4. Route-association tests (3c342b4)

• History doesn't settle the intent. The source comment in 37a354e says the new IDs are deliberate, to recreate drifted associations; the tests in the same commit expected the old IDs.
• Given the AlreadyAssociated risk you describe, I left the source alone:
  • "should add UpdateReplacePolicy…" is back to its original old-ID assertions and it.skipped;
  • the backwards-compatibility test keeps its NAT-route and VPC-endpoint assertions, but no longer asserts the association IDs either way.
• Tracked in devtools: VpcBuilder renames private subnet route-table associations on existing stacks (upgrade risk) #714.

Nits

• Two MongoDB downloads (3126961): @friggframework/test now depends on mongodb-memory-server-core, which has no postinstall, so only the pinned 8.0.17 is ever fetched.
  • This covers apps too. MONGOMS_DISABLE_POSTINSTALL, or config in this repo, can't reach an app's install, because the hook reads INIT_CWD.
  • install-footprint.test.js fails on the old dependency.
• Replica-set wait (bf11c4c): the step now fails with an error and the container log if no primary is elected within 30 s.
• CLI smoke test (40c6a2a): it now checks that frigg --help lists each expected command. I checked that a missing command fails the step.
• permissions (ced54b0): contents: read is now set at workflow level.
• docs/TESTING.md (7cfcf12): now covers *.db.test.js, npm run test:db, the FRIGG_TEST_* URLs, local and CI setup, and TestMongo as an opt-in helper.
• batchSend: agreed, it isn't a delivery-path bug. I'll describe it as test-observability only, not a production fix.
• selfHeal NAT guard: noted in the PR body as pre-existing (7b36f0f), not changed here.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 7cfcf12. Configure here.}

[cursor]

Local stage check misses development

Medium Severity

isLocalStage treats development as deployed even though that is both its own fallback and the usual NODE_ENV for local Node. An invalid encryption.schema now throws and blocks startup, contrary to the lenient local-stage path. It is also case-insensitive, while Prisma’s encryption bypass is not, so STAGE=DEV can encrypt with a warned-away schema.

Additional Locations (1)

• packages/core/database/prisma.js#L37-L40

 

^{Reviewed by Cursor Bugbot for commit 7cfcf12. Configure here.}

[cursor]

DocumentDB validates schema when encryption is off

Medium Severity

DocumentDBEncryptionService now runs loadCustomEncryptionSchema before it decides encryption is off. That load can throw EncryptionSchemaError, so constructing the repository fails even when the stage would bypass encryption or no KMS/AES keys are set. Prisma only loads the schema after encryption is enabled, so the same config still starts on Mongo/Postgres.

Additional Locations (1)

• packages/core/database/encryption/encryption-schema-registry.js#L259-L267

 

^{Reviewed by Cursor Bugbot for commit 7cfcf12. Configure here.}

[seanspeaks]

🚀 PR was released in v2.0.0-next.122 🚀

[sonarqubecloud]

Quality Gate failed

Failed conditions
17.6% Duplication on New Code (required ≤ 3%)
E Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE