✨ Added boot-time validation functionality support for adapters#29371
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdapter configurations for S3 storage, S3 redirects, and route settings now use Zod schemas with normalization and static validation methods. AdapterManager resolves configured adapters and feature variants, validates adapter contracts and configurations, deduplicates equivalent validations, and aggregates failures. Core boot invokes adapter initialization early, with tests covering schema validation, feature discovery, deduplication, and error aggregation. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:ci:integration |
✅ Succeeded | 3m | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 2m 53s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 2m 54s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 32s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 18s | View ↗ |
nx run-many -t test:unit -p ghost |
✅ Succeeded | 30s | View ↗ |
nx run-many -t lint -p ghost,ghost-monorepo |
✅ Succeeded | 18s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | <1s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 4s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-16 17:14:09 UTC
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #29371 +/- ##
==========================================
- Coverage 74.19% 74.17% -0.02%
==========================================
Files 1592 1592
Lines 138811 139000 +189
Branches 16858 16865 +7
==========================================
+ Hits 102990 103107 +117
- Misses 34804 34843 +39
- Partials 1017 1050 +33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/core/core/server/adapters/route-settings/FileStore.ts`:
- Line 31: Update the z.custom schema for getBackupFilePath in FileStore.ts to
validate that the runtime value is callable, and update the z.custom schema for
s3Client in S3Storage.ts to validate an object with a callable send method.
Apply the corresponding predicates at both affected sites so boot validation
rejects invalid values before adapter use.
In `@ghost/core/core/server/adapters/storage/S3Storage.ts`:
- Around line 70-81: Update the multipart byte-count schemas, including
multipartChunkSizeBytes and multipartUploadThresholdBytes, to require integer
values with Zod’s integer validation so fractional configuration fails before
reaching fs.createReadStream highWaterMark.
- Around line 86-88: Update the S3 configuration schema around accessKeyId,
secretAccessKey, and sessionToken to reject partial credential configurations,
matching the credential-pair refinement used by S3RedirectsStore. Ensure
credentials are accepted only when the required key fields are provided
together, rather than allowing incomplete values to fall back to ambient AWS
credentials.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6107e492-f1b8-4249-866d-e7e39fb4eabf
📒 Files selected for processing (10)
ghost/core/core/boot.jsghost/core/core/server/adapters/redirects/S3RedirectsStore.tsghost/core/core/server/adapters/route-settings/FileStore.tsghost/core/core/server/adapters/storage/S3Storage.tsghost/core/core/server/services/adapter-manager/adapter-manager.tsghost/core/core/server/services/adapter-manager/types.tsghost/core/core/server/services/adapter-manager/utils.tsghost/core/test/unit/server/adapters/storage/s3-storage.test.tsghost/core/test/unit/server/services/adapter-manager/adapter-manager.test.tsghost/core/test/unit/server/services/adapter-manager/utils.test.ts
491e1bd to
d4e570a
Compare
allouis
left a comment
allouis
left a comment
There was a problem hiding this comment.
Looks good!
One small thing - is it worth making the init and validate methods async - so that we allow for the potential of async validation in future?
Hah so my first local iteration of this actually did that, ultimately I opted to go the simpler route upfront. It wouldn't be difficult to add in later should we need to |
144b7a8 to
f680be7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/core/core/server/services/adapter-manager/utils.ts`:
- Around line 99-100: Update the adapter resolution logic around the
string-feature branch to retain the selected adapter class while setting
adapterConfig to undefined when settings[featureAdapter] is absent, instead of
unconditionally returning the string. Ensure resolveAdapterOptions validates
that selected class and rejects missing configuration rather than falling back
to the active adapter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 22145d10-ef18-42e2-9a75-59eb37432ed6
📒 Files selected for processing (10)
ghost/core/core/boot.jsghost/core/core/server/adapters/redirects/S3RedirectsStore.tsghost/core/core/server/adapters/route-settings/FileStore.tsghost/core/core/server/adapters/storage/S3Storage.tsghost/core/core/server/services/adapter-manager/adapter-manager.tsghost/core/core/server/services/adapter-manager/types.tsghost/core/core/server/services/adapter-manager/utils.tsghost/core/test/unit/server/adapters/storage/s3-storage.test.tsghost/core/test/unit/server/services/adapter-manager/adapter-manager.test.tsghost/core/test/unit/server/services/adapter-manager/utils.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- ghost/core/core/server/services/adapter-manager/types.ts
- ghost/core/test/unit/server/services/adapter-manager/adapter-manager.test.ts
- ghost/core/core/boot.js
- ghost/core/test/unit/server/adapters/storage/s3-storage.test.ts
- ghost/core/test/unit/server/services/adapter-manager/utils.test.ts
- ghost/core/core/server/adapters/redirects/S3RedirectsStore.ts
- ghost/core/core/server/services/adapter-manager/adapter-manager.ts
- ghost/core/core/server/adapters/route-settings/FileStore.ts
ref https://linear.app/ghost/issue/PLA-247/implement-boot-time-adapter-config-validation - add new optional static `validate` method to adapter manager logic - validate is passed normalized adapter config options and can optionally throw IncorrectUsageError if configuration options are absent - refactor S3/FileStore adapter extensions to leverage Zod schemas for config validation - wire up validation to adapterManager init method, called during boot
f680be7 to
2dbf648
Compare
2 participants
ref https://linear.app/ghost/issue/PLA-247/implement-boot-time-adapter-config-validation
validatemethod to adapter manager logic