feat: Add site custom header override priority - #584
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new per-site boolean field ChangescustomHeadersOverrideRequestHeaders end-to-end feature
Sequence Diagram(s)sequenceDiagram
actor User
participant Sites.tsx
participant POST_PUT_sites as POST/PUT /api/sites
participant siteProxy.ts
participant mergeHeadersWithSiteCustomHeaders
participant backupService.ts
User->>Sites.tsx: Toggle customHeadersOverrideRequestHeaders checkbox
Sites.tsx->>POST_PUT_sites: Save site with customHeadersOverrideRequestHeaders
POST_PUT_sites->>POST_PUT_sites: normalizeCustomHeadersOverrideRequestHeadersFlag (400 if invalid)
POST_PUT_sites-->>Sites.tsx: Persisted site row
Note over siteProxy.ts: On outbound proxy request
siteProxy.ts->>siteProxy.ts: resolveSiteRequestConfigByRequestUrl → customHeadersOverrideRequestHeaders
siteProxy.ts->>mergeHeadersWithSiteCustomHeaders: siteCustomHeaders, requestHeaders,<br/>priority('site'|'request')
mergeHeadersWithSiteCustomHeaders-->>siteProxy.ts: merged Headers
Note over backupService.ts: On backup export/import
backupService.ts->>backupService.ts: include customHeadersOverrideRequestHeaders<br/>(default false) in site rows
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/services/databaseMigrationService.ts (1)
327-345:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve all persisted
sitesconfig columns in migration statements.Line [327] adds the new header override field, but the
sitesstatement still omitspost_refresh_probe_enabled,post_refresh_probe_model,post_refresh_probe_scope, andpost_refresh_probe_latency_threshold_ms. Those values are dropped during migration and reset to defaults in the target DB.Proposed fix
- columns: ['id', 'name', 'url', 'external_checkin_url', 'platform', 'proxy_url', 'use_system_proxy', 'custom_headers', 'custom_headers_override_request_headers', 'status', 'is_pinned', 'sort_order', 'global_weight', 'api_key', 'created_at', 'updated_at'], + columns: ['id', 'name', 'url', 'external_checkin_url', 'platform', 'proxy_url', 'use_system_proxy', 'custom_headers', 'custom_headers_override_request_headers', 'status', 'is_pinned', 'sort_order', 'global_weight', 'api_key', 'post_refresh_probe_enabled', 'post_refresh_probe_model', 'post_refresh_probe_scope', 'post_refresh_probe_latency_threshold_ms', 'created_at', 'updated_at'], values: [ asNumber(row.id, 0), asNullableString(row.name), asNullableString(row.url), asNullableString(row.externalCheckinUrl), asNullableString(row.platform), asNullableString(row.proxyUrl), asBoolean(row.useSystemProxy, false), serializeColumnValue('sites', 'custom_headers', row.customHeaders, contract), asBoolean(row.customHeadersOverrideRequestHeaders, false), asNullableString(row.status) ?? 'active', asBoolean(row.isPinned, false), asNumber(row.sortOrder, 0), asNumber(row.globalWeight, 1), asNullableString(row.apiKey), + asBoolean((row as { postRefreshProbeEnabled?: unknown }).postRefreshProbeEnabled, false), + asNullableString((row as { postRefreshProbeModel?: unknown }).postRefreshProbeModel) ?? '', + asNullableString((row as { postRefreshProbeScope?: unknown }).postRefreshProbeScope) ?? 'single', + asNumber((row as { postRefreshProbeLatencyThresholdMs?: unknown }).postRefreshProbeLatencyThresholdMs, 0), asNullableString(row.createdAt), asNullableString(row.updatedAt), ],🤖 Prompt for 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. In `@src/server/services/databaseMigrationService.ts` around lines 327 - 345, The migration statement for the sites table is omitting four persisted columns that should be preserved: post_refresh_probe_enabled, post_refresh_probe_model, post_refresh_probe_scope, and post_refresh_probe_latency_threshold_ms. Add these four column names to the columns array and add the corresponding converted values from the row object to the values array in the INSERT statement. Use appropriate type conversion functions (asBoolean for the enabled flag, asNullableString for the model and scope, asNumber for the latency threshold) consistent with the pattern used for existing fields in this migration block.
🤖 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.
Outside diff comments:
In `@src/server/services/databaseMigrationService.ts`:
- Around line 327-345: The migration statement for the sites table is omitting
four persisted columns that should be preserved: post_refresh_probe_enabled,
post_refresh_probe_model, post_refresh_probe_scope, and
post_refresh_probe_latency_threshold_ms. Add these four column names to the
columns array and add the corresponding converted values from the row object to
the values array in the INSERT statement. Use appropriate type conversion
functions (asBoolean for the enabled flag, asNullableString for the model and
scope, asNumber for the latency threshold) consistent with the pattern used for
existing fields in this migration block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 70c39bfc-fbb2-49c6-b32d-d890b7ead934
⛔ Files ignored due to path filters (3)
src/server/db/generated/mysql.bootstrap.sqlis excluded by!**/generated/**src/server/db/generated/postgres.bootstrap.sqlis excluded by!**/generated/**src/server/db/generated/schemaContract.jsonis excluded by!**/generated/**
📒 Files selected for processing (21)
drizzle/0027_site_custom_headers_override_request_headers.sqldrizzle/meta/_journal.jsonsrc/server/contracts/siteRoutePayloads.tssrc/server/db/index.tssrc/server/db/schema.tssrc/server/db/schemaContract.test.tssrc/server/db/siteSchemaCompatibility.test.tssrc/server/db/siteSchemaCompatibility.tssrc/server/routes/api/sites.proxyUrl.test.tssrc/server/routes/api/sites.tssrc/server/services/backupService.test.tssrc/server/services/backupService.tssrc/server/services/databaseMigrationService.test.tssrc/server/services/databaseMigrationService.tssrc/server/services/siteCustomHeaders.test.tssrc/server/services/siteCustomHeaders.tssrc/server/services/siteProxy.test.tssrc/server/services/siteProxy.tssrc/web/pages/Sites.tsxsrc/web/pages/helpers/sitesEditor.test.tssrc/web/pages/helpers/sitesEditor.ts
Related issuesWhyThis directly fixes the site custom header priority problem reported in #530: downstream clients such as Hermes / OpenAI Python SDK can currently send their own |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
- Adds per-site 'customHeadersOverrideRequestHeaders' flag (default false). - When true, site-level custom headers (User-Agent, etc.) override same-name downstream passthrough headers instead of being overridden by them. - Schema: new sqlite migration 0027, mysql/postgres bootstrap updates. - Frontend: new checkbox in site editor; non-boolean API payload returns 400. - Backwards compatible: existing sites default to request-priority.
91f68d1 to
fa35544
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa35544563
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Summary
Adds an opt-in site-level setting that lets site custom headers override same-name outgoing request headers.
By default, metapi keeps the existing behavior: explicit request/runtime headers remain authoritative over site custom headers. When the new setting is enabled for a site, that site's configured custom headers are applied last, so headers like
User-Agent,Originator,Version, or SDK-specific client headers can be forced per upstream site.What changed
customHeadersOverrideRequestHeaders/custom_headers_override_request_headersto the site schema.0027_site_custom_headers_override_request_headers.mergeHeadersWithSiteCustomHeaders()with explicit merge priority:request: current default behavior, request/runtime headers override site headers.site: site custom headers override same-name request/runtime headers.Why
metapi previously always merged headers as:
That means a site-configured
User-Agentcan be overwritten by downstream clients such as Hermes, Cherry Studio, OpenAI SDK, or Python SDK before the upstream request is sent.This change keeps the current behavior as the default for compatibility, while giving each site an explicit opt-in override mode. It avoids broad downstream header passthrough and keeps the behavior scoped to existing site custom headers.
This is useful for upstream sites that require stable client-identifying headers while still preserving metapi's existing routing, fallback, account, check-in, and balance workflows.
Compatibility
customHeadersOverrideRequestHeaders = false.Summary by CodeRabbit
customHeadersOverrideRequestHeadersto site settings, letting users choose whether custom site headers override same-named outbound request headers. Available in the site editor and supported across site create/update flows.