Repository navigation
feat(devtools): frigg validate --module and api-module schema fields - #702
seanspeaks merged 9 commits into
Conversation
047d3c4 to
853ee3e
Compare
…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>
Co-authored-by: Sean Matthews <sean.matthews@lefthook.co>
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>
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>
Co-authored-by: Sean Matthews <sean.matthews@lefthook.co>
devtools depends on @friggframework/test at the canary version. A packages/test change ensures lerna/auto includes it when cutting the #702 canary (same pattern as the core package.json bump). Co-authored-by: Sean Matthews <sean.matthews@lefthook.co>
0a3e1ef to
0c74b9c
Compare
Frigg 2.0.1 review (Group 2) — COMMENT onlyChecklist
Verdict (would): changes needed (same blockers as Findings
Module command ( |
| "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 (same as #666): add 'none' for #663’s explicit opt-out. Keep no default: "aes".
| "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 (same as #666): add 'external' — #639 scaffold already uses database.postgres.management: 'external', and semantic checks special-case it. Without the enum value, frigg validate / build preflight reject a valid init app.
Follow-up finding (COMMENT)blocking — JSON.parse / Ajv keep the last occurrence; the first block is dead weight and makes merges with Scenario: Reviewers/tools that don’t last-key-win see contradictory schemas; a rebase can keep the wrong block. Delete the first copies; keep only the ADR-051 definitions. |
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
|
Re the duplicate-keys finding in #702 (comment) Verdict: confirmed. Fixed in ef0dc8c. A Python Choice: I kept the ADR-051 block and deleted the earlier one. Before/after
The change is deletion only (33 lines), with no Test: 🤖 Generated with Claude Code |
|
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.
d5e6de1
into
cursor/api-module-schema-fields-1721
There was a problem hiding this comment.
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 ef0dc8c. Configure here.
| 'Frigg provisions or discovers a private Aurora cluster, but the Lambda functions run outside a VPC and cannot reach it.', | ||
| "Set vpc.enable: true, or use database.postgres.management: 'use-existing' with a reachable endpoint." | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Aurora reachability check ignores managementMode
Medium Severity
checkManagement treats database.postgres.management as unused when managementMode is managed or existing, but provisionsAurora still honors that same key. A managed app with postgres.management: 'use-existing' and vpc.enable off therefore gets the no-effect warning and skips unreachable-database, even though the ignored key is the only reason the reachability check stays quiet. The init-style scaffold is this combination.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ef0dc8c. Configure here.
| const structural = | ||
| definition && typeof definition === 'object' | ||
| ? checkModuleDefinition(definition, '') | ||
| : []; |
There was a problem hiding this comment.
Module mode skips rateLimit warning
Medium Severity
frigg validate --module never runs the module-rate-limit-ignored check that app-mode validation adds. config.rateLimit therefore passes silently even though nothing applies it. A top-level rateLimit is rejected as an unknown key instead of the warning ADR-051 describes.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ef0dc8c. Configure here.
devtools depends on @friggframework/test at the canary version. A packages/test change ensures lerna/auto includes it when cutting the #702 canary (same pattern as the core package.json bump). Co-authored-by: Sean Matthews <sean.matthews@lefthook.co>
devtools depends on @friggframework/test at the canary version. A packages/test change ensures lerna/auto includes it when cutting the #702 canary (same pattern as the core package.json bump). Co-authored-by: Sean Matthews <sean.matthews@lefthook.co>





Summary
Adds
frigg validate --modulefor API module definitions, stacked on #701’s schema field allowances.Canary fixes
frigg initscaffolding #639’s EventBridge scheduler lazy-load into fix(schemas): allow modelName, setAuthParams, getAuthorizationRequirements in api-module definition #701 (this PR rebased onto that). Did not restack onto fix: lazy-load scheduler AWS SDK in core and repairfrigg initscaffolding #639 — that branch is 18 commits behindnextwith a large unrelatedfrigg initsurface; fix: lazy-load scheduler AWS SDK in core and repairfrigg initscaffolding #639’s branch was not pushed to.packages/testso the canary publishes@friggframework/test(devtools depends on it at the canary version). A concurrent fix(schemas): allow modelName, setAuthParams, getAuthorizationRequirements in api-module definition #701/feat(devtools): frigg validate --module and api-module schema fields #702 release race leftcore/testunpublished for0c74b9c; retriggered a solo canary.Verified canary
2.0.0--canary.702.3f342da.0—npx -p @friggframework/devtools@2.0.0--canary.702.3f342da.0 frigg validate --module .exits 0 againstapi-module-librarypackages/v1-ready/fathom.Test plan
📦 Published PR as canary version:
2.0.0--canary.702.d5e6de1.0✨ Test out this PR locally via:
npm install @friggframework/admin-scripts@2.0.0--canary.702.d5e6de1.0 npm install @friggframework/core@2.0.0--canary.702.d5e6de1.0 npm install @friggframework/devtools@2.0.0--canary.702.d5e6de1.0 npm install @friggframework/eslint-config@2.0.0--canary.702.d5e6de1.0 npm install @friggframework/prettier-config@2.0.0--canary.702.d5e6de1.0 npm install @friggframework/schemas@2.0.0--canary.702.d5e6de1.0 npm install @friggframework/serverless-plugin@2.0.0--canary.702.d5e6de1.0 npm install @friggframework/test@2.0.0--canary.702.d5e6de1.0 npm install @friggframework/ui@2.0.0--canary.702.d5e6de1.0 # or yarn add @friggframework/admin-scripts@2.0.0--canary.702.d5e6de1.0 yarn add @friggframework/core@2.0.0--canary.702.d5e6de1.0 yarn add @friggframework/devtools@2.0.0--canary.702.d5e6de1.0 yarn add @friggframework/eslint-config@2.0.0--canary.702.d5e6de1.0 yarn add @friggframework/prettier-config@2.0.0--canary.702.d5e6de1.0 yarn add @friggframework/schemas@2.0.0--canary.702.d5e6de1.0 yarn add @friggframework/serverless-plugin@2.0.0--canary.702.d5e6de1.0 yarn add @friggframework/test@2.0.0--canary.702.d5e6de1.0 yarn add @friggframework/ui@2.0.0--canary.702.d5e6de1.0Note
High Risk
Changes default user/auth behavior at runtime, blocks build/deploy on previously tolerated definition mistakes, and adds a core→schemas dependency—organization-only login may need explicit
individualUserRequired: false.Reviewed by Cursor Bugbot for commit ef0dc8c. Bugbot is set up for automated code reviews on this repo. Configure here.