Repository navigation
feat(devtools): frigg validate, and make the app-definition schema match the runtime (ADR-051) - #666
Conversation
seanspeaks
added
release
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
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, comment @claude review on this pull request, or push a new commit, to trigger a review.
✅ Deploy Preview for friggframework-org canceled.
|
|
ADR reconciliation needed after Stack A/B — drafted with Claude Code at Sean's request Severity: must-reconcile. ADR-051 §1: "every key a builder, core or the CLI reads is in the schema". Stack A adds many keys, and it edits the #671 copy of App-definition keys Stack A reads (all need schema entries and
Integration
Module definitions: Environment (ADR-027): Stack B reads Merge order: this PR and #671 should converge on one schema first. Stack A's schema hunks then rebase onto it. |
Add frigg validate --module (also frigg validate module) to load Definition from ./index.js, run validateApiModuleDefinition and checkModuleDefinition, and exit non-zero with JSON field paths. Supports --json and -v. Stacked on #701 (api-module schema fields) and includes #666 (ADR-051 frigg validate app mode) so the CLI canary has both. Co-authored-by: Sean Matthews <sean.matthews@lefthook.co>
Add frigg validate --module (also frigg validate module) to load Definition from ./index.js, run validateApiModuleDefinition and checkModuleDefinition, and exit non-zero with JSON field paths. Supports --json and -v. Stacked on #701 (api-module schema fields) and includes #666 (ADR-051 frigg validate app mode) so the CLI canary has both. Co-authored-by: Sean Matthews <sean.matthews@lefthook.co>
There was a problem hiding this comment.
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.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 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 923c975. Configure here.
Frigg 2.0.1 review (Group 2) — COMMENT onlyChecklist
Verdict (would): changes needed Findings
ADR-051 body + contract test approach look good once the enum gaps above are closed. |
| "enum": ["kms", "aes"], | ||
| "default": "aes" | ||
| "description": "Field-level encryption key. 'kms': Frigg creates or discovers a KMS key and sets KMS_KEY_ARN. 'aes': the app supplies AES_KEY_ID and AES_KEY (list both in `environment`). Omitted: no key is provisioned and deployed stages store sensitive fields unencrypted (core logs a warning). Stages dev, test and local never encrypt.", | ||
| "enum": ["kms", "aes"] |
There was a problem hiding this comment.
blocking: enum is ["kms","aes"] — missing 'none' from #663.
Scenario: encryption: { fieldLevelEncryptionMethod: 'none' } (the documented fail-closed opt-out) fails schema validation, so frigg build/deploy stop before they can set FRIGG_ENCRYPTION_DISABLED. Add 'none'; do not reintroduce default: "aes".
There was a problem hiding this comment.
Fixed in 89f6545. The enum is now kms | aes | none, with no default; a schema test asserts the default stays absent. 'none' is treated as an explicit opt-out: frigg validate emits neither the no-field-encryption warning nor the encryption-bypassed warning for it. The contract matrix also has an external db, encryption none case.
🤖 Generated with Claude Code
| "enum": ["discover", "create-new", "existing"], | ||
| "default": "discover" | ||
| "description": "How Frigg gets the Aurora cluster (ignored when managementMode is 'managed' or 'existing'). 'discover' (also when omitted): find one, else create one in the stack. 'managed': create it in the stack. 'use-existing': connect to `endpoint` without creating anything.", | ||
| "enum": ["discover", "managed", "use-existing"] |
There was a problem hiding this comment.
blocking: enum omits 'external', but #639 is already on next and the scaffold uses database.postgres.management: 'external'. Semantic checks already treat 'external' as “Frigg provisions no DB”.
Scenario: frigg init → frigg validate / frigg build fail on /database/postgres/management with an enum error even though the Aurora builder accepts 'external'. Add 'external' to this enum.
There was a problem hiding this comment.
Fixed in 89f6545. database.postgres.management now accepts discover | managed | use-existing | external (the Aurora builder's values plus external, no default), and the description says what external does. Covered by a validate test that loads the real frigg init template (templates/backend/index.js) and passes --stage prod with no errors, and by a new frigg init scaffold (external db, no vpc) case in the schema contract matrix.
🤖 Generated with Claude Code
…e reads
- integrations (and adminScripts, reports) are validated as a serialised
view in which each class is { Definition }: apps list classes, which
`type: object` rejected, so every app with an integration failed.
- enums are the values the builders accept: postgres.management
discover|managed|use-existing, vpc.management discover|create-new|
use-existing, managementMode adds 'existing'; subnet/NAT management add
the values the VPC builder acts on.
- adds the keys builders and core read but the schema rejected
(ownership/external blocks, usePrismaLambdaLayer, scheduler, aws.discovery,
telemetry, admin, adminScripts, reports, deployment, encryption.disable,
kmsKeyAlias, keyAlias, lambda.keepNestedNodeModules, ...).
- keys nothing reads are marked deprecated and lose their defaults.
- every remaining default equals what the code does when the key is
absent; defaults the code does not share are removed
(fieldLevelEncryptionMethod, managementMode, vpcIsolation, ...).
- applyAppDefinitionDefaults(): the one schema-driven defaults step, loaded
without Ajv, for core's loader and devtools (ADR-051).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
loadAppDefinition passed `user` through unchanged, so the documented defaults never applied: a user block without individualUserRequired made POST /user/login throw, and an app without one got userConfig = null and a TypeError on every authenticated request. The loader now applies the schema defaults (applyAppDefinitionDefaults from @friggframework/schemas) before reading the definition, and maps the deprecated user.password to usePassword (ADR-051). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…and start `frigg validate [--stage] [--json]` loads index.js the way build does, validates it against the app-definition schema and runs the checks the schema cannot express (integration classes and their module definitions, an enabled database, management settings, encryption, a user config that can log in, deprecated keys), with JSON pointers and fix hints; it exits 1 on errors. `frigg build` and `frigg deploy` run the same validation first and stop on errors (--skip-validate bypasses it, loudly); `frigg start` only warns; `frigg init` validates the generated index.js instead of a stand-in object. createFriggInfrastructure applies the schema defaults through the same step as core. A contract test composes the infrastructure for a matrix of definitions, records every key the builders read and checks it against the schema in both directions, and checks that the schema defaults change nothing in the composed output (ADR-051). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Records the decision (the schema describes what the runtime reads, its defaults are applied by one loader step, frigg validate gates build and deploy, a contract test stops the drift), adds `frigg validate` to the CLI README, CLAUDE.md and the frigg skill, and corrects the PostgreSQL docs, which used management values and keys the Aurora builder does not read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One $id pattern for every Frigg JSON Schema, https://friggframework.org/schemas/<name>/v<major>.<minor>.json, served from that URL by the website; immutable versions once a stable release ships them; validation that resolves ids from the installed package and never the network; $schema usage in module manifests; and the move of the ten @friggframework/schemas ids to v1.0 with the old ids kept as aliases for one release. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013b2UUFaQ5Lk6TFPqzRr1j7
- app-definition schema: database.postgres.management accepts 'external' (the frigg init scaffold, #639), and fieldLevelEncryptionMethod accepts 'none' (the explicit opt-out, #663) with no default. - adopterJwt: a jwtConfig without a `secret` key is still an error, but an empty value (`secret: process.env.JWT_SECRET` with the variable unset in the shell; build and deploy validate before .env is loaded) is now a warning that the secret must be set at runtime. The schema no longer marks `secret` required, since it cannot tell the two apart. - unreachable-database: under managementMode 'managed' or 'existing' the Aurora builder ignores postgres.management 'use-existing', so the check now warns in that case too. - resolveBackendDir stops at the nearest backend/package.json like core's findNearestBackendPackageJson, and reports a missing index.js there instead of validating a farther app. - 'none' is not warned about as plaintext or as a bypassed stage. - Contract matrix: the frigg init scaffold and an external/none case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013b2UUFaQ5Lk6TFPqzRr1j7
923c975 to
89f6545
Compare
app-definition.schema.json defined adminScripts, reports and admin twice: once from next (#675) and once from the ADR-051 rewrite. JSON.parse keeps the last, so the ADR-051 definitions were already the ones in force; the earlier block is removed and the parsed schema is unchanged. A new test parses every schema file with a duplicate-key check, so a key written twice fails CI instead of silently replacing the first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013b2UUFaQ5Lk6TFPqzRr1j7
|
Reply to the Frigg 2.0.1 review (Group 2) and the bugbot review. The branch is rebased onto Rebase conflicts
Findings
Bugbot: the JWT secret, Aurora reachability and nearer-backend findings are fixed, with tests (see the inline replies). Tests on Node 24.10: schemas 183/183, devtools frigg-cli plus the schema contract test 409 passed / 7 skipped, and 🤖 Generated with Claude Code |
Add frigg validate --module (also frigg validate module) to load Definition from ./index.js, run validateApiModuleDefinition and checkModuleDefinition, and exit non-zero with JSON field paths. Supports --json and -v. Stacked on #701 (api-module schema fields) and includes #666 (ADR-051 frigg validate app mode) so the CLI canary has both. Co-authored-by: Sean Matthews <sean.matthews@lefthook.co>
|
|
Review follow-ups, rebased onto Fixed in
Deferred, deliberately:
🤖 Generated with Claude Code |
Add frigg validate --module (also frigg validate module) to load Definition from ./index.js, run validateApiModuleDefinition and checkModuleDefinition, and exit non-zero with JSON field paths. Supports --json and -v. Stacked on #701 (api-module schema fields) and includes #666 (ADR-051 frigg validate app mode) so the CLI canary has both. Co-authored-by: Sean Matthews <sean.matthews@lefthook.co>
|
🚀 PR was released in |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013b2UUFaQ5Lk6TFPqzRr1j7
Summary
Adds
frigg validateand makesapp-definition.schema.jsondescribe what Frigg actually reads. The decision is recorded in ADR-051.Integration classes validate.
integrations.itemswastype: object, but apps list integration classes (functions), so every app with an integration failedvalidateAppDefinition. Nobody noticed because onlyfrigg initran the schema, as a warning, against a hard-coded stand-in object.validateAppDefinitionnow checks a serialised view where each class inintegrations,adminScriptsandreportsbecomes{ Definition }.frigg validatechecks each class structurally, using the same rules asModule.validateDefinitionand the integration builder.The schema matches the builders. The enums now hold the values the builders accept. The schema now includes the keys the builders and core read but used to reject (over 50 of them). Keys nothing reads are marked
deprecatedand lose their defaults. The drift table below lists every change.One step applies the defaults.
applyAppDefinitionDefaults()lives in@friggframework/schemasand loads only the JSON, not Ajv. It is called by core'sloadAppDefinition, by devtools'createFriggInfrastructureand byfrigg validate, and nothing else applies schema defaults. Rule: a schema default must equal what the code does when the key is absent; where it didn't, the default was removed. The one intended behaviour change is that theuserdefaults now apply, which fixes login (see "Behaviour changes").frigg validate [--stage <s>] [--json]index.jsthe same way build does.'ssm'env entries withoutssm.enableWiring
frigg buildandfrigg deployrun the same validation first and stop on errors.--skip-validatebypasses it and prints a highlightedVALIDATION SKIPPEDline.frigg startonly warns.frigg initvalidates the generatedindex.jsthrough the same code path (as warnings).Contract test (
packages/devtools/infrastructure/app-definition-schema-contract.test.js): composes the infrastructure for 12 definitions with discovery mocked and records every key the builders read through a proxy. It asserts that:Run against the old schema, it fails on more than 50 keys.
Docs: ADR-051, the CLI README, the schemas README, CLAUDE.md "Frigg CLI Features", the frigg skill's CLI list, and the PostgreSQL docs. The PostgreSQL docs used
management: 'create-new'and keys the Aurora builder never reads (scaling,databaseName,deletionProtection, ...).Behaviour changes (all in
user, throughloadAppDefinition)userblockuserConfig = null, so authenticated routes and/user/loginthrow a TypeErroruser: { usePassword: true }/user/loginthrows "must require either individualUserRequired or organizationUserRequired"user: { password: true }(deprecated spelling)usePassword: trueuser: { organizationUserRequired: true }withindividualUserRequiredunsetindividualUserRequireddefaults to true, which the x-frigg header path and theUsermodel already assumed.frigg validatewarns and suggestsindividualUserRequired: falseThe schema default of
user.usePasswordchanges fromtruetofalse, andauthModes.sharedSecretchanges fromfalsetotrue. Both now match what core already did when the key was absent, so apps that identify users by appUserId, or that use the shared secret, keep working.frigg buildandfrigg deploycan now stop on definitions that used to deploy, when the definition has an unknown key (e.g. a typo such asencrpytion, which used to deploy without encryption) or a value the builders would reject anyway.Drift table: schema vs runtime
integrations[]type: object, rejects classes{ Definition }view, plus structural checks infrigg validatemanagementModemanaged,existing, else discover; default discovermanaged|discover|custom, defaultmanaged+existing;custom(= discover) warns; no defaultvpcIsolation|| 'shared'in translation, undefined = isolated elsewhereisolatedmanagedvpc.managementdiscover|create-new|use-existingdiscover|create-new|existingvpc.subnets.managementcreate,use-existingdiscover|create-new|existing(all no-ops)vpc.natGateway.managementcreateAndManagediscover|create-new|existing(no-ops)createAndManage; no-ops warnvpc.selfHealconfig.selfHeal !== false)falsevpc.shareAcrossStages/ownership/external/configdatabase.postgres.managementdiscover|managed|use-existingdiscover|create-new|existing(2 of 3 fail the build)database.postgres.enable=== true; migrations/Prisma layer!== falsefalsedatabase.postgres.maxCapacitypostgres.engineVersion/ownership/external/clusterId/instanceId,database.dynamodb.enablepostgres.selfHeal,database.uri,mongoDB.uriencryption.fieldLevelEncryptionMethod'kms'acts; omitted = no key (core reads env vars, not this key)aesencryption.createResourceIfNoneFoundfalseencryption.ownership/kmsKeyAlias/keyAlias/disableuser.*defaultsusePassword→false,sharedSecret→true(code behaviour)user.passwordtrueuser.strictUserValidationauth.*,security.*,webhooks.*,custom.*,secretsManager,label,user.fields/model,websockets.routeSelectionExpressionfrigg validatewarnslogging.level/logging.formatinfo/jsonusePrismaLambdaLayer,lambda.keepNestedNodeModules,scheduler.enable,aws.discovery.*,migration.ownership.*telemetry.*,admin.*,adminScripts,reports,deployment.*,stage,regionregiondeprecated: the region comes fromAWS_REGION)rateLimit(see #656)api-module-definitionfrigg validatewarns when setOverlap with #639 and #663, and merge order
frigg initscaffolding #639 (scaffold template,database.postgres.management: 'external'): it changes the samepostgres.managementenum line andPOSTGRES-CONFIGURATION.md. Its template usesmanagement: 'external', so onnextwithout fix: lazy-load scheduler AWS SDK in core and repairfrigg initscaffolding #639's Aurora builder change,frigg validatecorrectly rejects it (the builder would too). Suggested order: merge fix: lazy-load scheduler AWS SDK in core and repairfrigg initscaffolding #639 first, then rebase this PR and add'external'to thepostgres.managementenum (the semantic checks already treat'external'as "Frigg provisions no database", and the contract matrix can add anexternalcase). If this PR merges first, fix: lazy-load scheduler AWS SDK in core and repairfrigg initscaffolding #639's rebase resolves the same one-line enum conflict by keeping this PR's values plus'external'. Its scaffold also setsuserexplicitly, which stays correct.fieldLevelEncryptionMethod: 'none', fail-closed encryption): it changes the samefieldLevelEncryptionMethodlines, adding'none'and keepingdefault: "aes". When resolving, keep'none'in the enum and drop the default: the default is not applied by anything and would contradict feat(core)!: fail closed on field-level encryption in deployed stages #663's own fail-closed rule. After feat(core)!: fail closed on field-level encryption in deployed stages #663, the "no method on a deployed stage" warning should become an error unless the method is'none'. That is a one-line change invalidate-command/checks/config.js(checkEncryption).rateLimit): this PR only warns thatrateLimitis ignored. If feat(core): honour Retry-After and let API modules declare rate limits #656 makes the runtime read it, remove themodule-rate-limit-ignoredcheck.Test plan
packages/schemas: 158 passed. Newapp-definition-defaults.test.js(view, defaults walker,userdefaults, alias, idempotence, no mutation) andapp-definition-runtime-keys.test.js(builder enums, runtime keys, defaults equal the runtime's, deprecated keys).packages/core: new loader tests (user defaults applied, missinguser→ defaults, app values kept). Full core suite has the same 64 failures asorigin/next, all environment-related (MongoMemoryServer, Boom message text, deprecation output), and none new.packages/devtools/frigg-cli: 45 new validate-command tests (integration checks, semantic checks, command and--json, load errors, backend discovery, preflight for build/deploy/start/skip, init) and 4 wiring tests. Same 4 failing suites asorigin/next(@inquirer/prompts/tmpload error,dependencies.test.js).packages/devtools/infrastructure: the contract test (7 tests) and thecreateFriggInfrastructuredefaults test pass. The full devtools run has the same 32 failing suites asorigin/next.template-generation.test.jsfails identically (7/34) on both when run alone.packages/admin-scripts: 278 passed (unchanged).Manual: a scaffold-style backend (the fix: lazy-load scheduler AWS SDK in core and repair
frigg initscaffolding #639 template plus an OAuth2 integration class with an API module definition) passesfrigg validate --stage prod(exit 0,--jsonvalid). With the template'smanagement: 'external'onnext, it reports/database/postgres/management(expected until fix: lazy-load scheduler AWS SDK in core and repairfrigg initscaffolding #639).Manual: a deliberately broken backend makes
frigg validateexit 1 and lists 9 errors and 2 warnings, each with a pointer and a fix:encrpytion→ "Did you mean "encryption"?"postgres.management: 'create-new'vpc.management: 'existing'testAuthRequestaeswithoutAES_KEY_ID/AES_KEYcustomhas no effect (warning)On the same backend,
frigg buildandfrigg deploystop beforeosls, and--skip-validateproceeds with the banner.npm cilock:package-lock.jsongains only core's new@friggframework/schemasdependency.Left for follow-ups
managementMode: 'managed'logs that it ignores the legacy VPC keys but the VPC builder still applies them (vpc-builder.jstranslateLegacyConfig).frigg validatewarns; the builder fix is separate.deploy-commandrequires../../infrastructure/env-validator, which does not exist (the file isdomains/shared/validation/env-validator.js), so it always falls back to basic env checks.STAGEdev/test/local, even on a deployeddevstage.frigg validate --stage devwarns; feat(core)!: fail closed on field-level encryption in deployed stages #663 is the place to change it.docs/reference/aurora-public-access.mddocumentsallowedIpAddressesandscaling, which nothing implements. This PR adds a status note; the feature or the page needs a decision.🤖 Generated with Claude Code
📦 Published PR as canary version:
2.0.0--canary.666.abcf913.0✨ Test out this PR locally via:
npm install @friggframework/admin-scripts@2.0.0--canary.666.abcf913.0 npm install @friggframework/core@2.0.0--canary.666.abcf913.0 npm install @friggframework/devtools@2.0.0--canary.666.abcf913.0 npm install @friggframework/eslint-config@2.0.0--canary.666.abcf913.0 npm install @friggframework/prettier-config@2.0.0--canary.666.abcf913.0 npm install @friggframework/schemas@2.0.0--canary.666.abcf913.0 npm install @friggframework/serverless-plugin@2.0.0--canary.666.abcf913.0 npm install @friggframework/test@2.0.0--canary.666.abcf913.0 npm install @friggframework/ui@2.0.0--canary.666.abcf913.0 # or yarn add @friggframework/admin-scripts@2.0.0--canary.666.abcf913.0 yarn add @friggframework/core@2.0.0--canary.666.abcf913.0 yarn add @friggframework/devtools@2.0.0--canary.666.abcf913.0 yarn add @friggframework/eslint-config@2.0.0--canary.666.abcf913.0 yarn add @friggframework/prettier-config@2.0.0--canary.666.abcf913.0 yarn add @friggframework/schemas@2.0.0--canary.666.abcf913.0 yarn add @friggframework/serverless-plugin@2.0.0--canary.666.abcf913.0 yarn add @friggframework/test@2.0.0--canary.666.abcf913.0 yarn add @friggframework/ui@2.0.0--canary.666.abcf913.0Note
Medium Risk
Changes default user/auth behaviour at runtime and can block builds/deploys on configs that previously slipped through; escape hatch is
--skip-validate.Overview
Introduces
frigg validate(schema + semantic checks onindex.js,--stage,--json) and wires it into the CLI:frigg buildandfrigg deployfail on errors (--skip-validatebypasses with a loud warning);frigg startandfrigg initonly warn.Aligns
app-definition.schema.jsonwith what builders and core actually read (ADR-051): integration classes validate via a serialised{ Definition }view plus structural checks; enums and keys match the infrastructure builders; unused keys are deprecated.applyAppDefinitionDefaults()in@friggframework/schemasis the single place schema defaults are applied—used byloadAppDefinition,createFriggInfrastructure, and validation—fixinguserconfig when the block is missing or incomplete (login/auth behaviour changes documented in the ADR).Adds a schema–builder contract test, core/schemas/devtools tests, and doc updates (CLI README, PostgreSQL/Aurora guides, hosted-schema notes in ADR-051).
Reviewed by Cursor Bugbot for commit abcf913. Bugbot is set up for automated code reviews on this repo. Configure here.
Version
Published prerelease version:
v2.0.0-next.124Changelog
🚀 Enhancement
@friggframework/core,@friggframework/devtools,@friggframework/schemas🐛 Bug Fix
@friggframework/admin-scripts,@friggframework/core,@friggframework/devtools,@friggframework/eslint-config,@friggframework/prettier-config,@friggframework/schemas,@friggframework/serverless-plugin,@friggframework/test,@friggframework/uiAuthors: 1