fix(web): 修复 Shadow DOM 隔离渲染下自定义 HTML 深浅色失效的问题及关于渲染隔离的探讨 - #5889
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a ChangesIsolated HTML Content Dark-Mode Sync
Estimated code review effort: 1 (Trivial) | ~5 minutes Sequence Diagram(s)sequenceDiagram
participant DocumentElement as document.documentElement
participant Observer as MutationObserver
participant Sync as syncDarkClass
participant Wrapper as Shadow DOM Wrapper
DocumentElement->>Observer: class attribute changes
Observer->>Sync: invoke callback(wrapper)
Sync->>DocumentElement: check for "dark" class
Sync->>Wrapper: toggle "dark" class
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Shadow DOM 的样式隔离特性导致外部 html 元素上的 dark class 无法被 Shadow DOM 内部的 Tailwind dark: 选择器匹配到。通过 MutationObserver 监听 document.documentElement 的 class 变化,将 dark class 同步到 Shadow DOM 内的包装容器上,使深色模式样式正常生效。
c22c177 to
bb921cb
Compare
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/classic/src/pages/About/index.jsx (1)
62-73: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick wini18n key mismatch: component still references removed key "New API项目仓库地址:".
The locale files in this PR (en.json, fr.json, ja.json) renamed this translation key to
"OmniAPI项目仓库地址:"and dropped the old key, but this component (line 65) still callst('New API项目仓库地址:'). Since the old key no longer exists in any locale file, this will fall back to displaying the raw Chinese key text to all users (including non-Chinese locales), rather than the intended localized OmniAPI string.This component needs to be updated to use the new key.
🌐 Proposed fix
- {t('New API项目仓库地址:')} + {t('OmniAPI项目仓库地址:')}🤖 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 `@web/classic/src/pages/About/index.jsx` around lines 62 - 73, The About page still uses the removed i18n key in customDescription, so update the t(...) call in About/index.jsx to the renamed OmniAPI项目仓库地址: key used by the locale files. Verify the surrounding customDescription JSX still renders the repo link correctly and that no other references to the old New API项目仓库地址: key remain in this component.relay/mjproxy_handler.go (1)
236-247: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winRedact request headers before persisting logs
common.GetRequestHeadersonly stripsAuthorizationandCookie; the rest ofc.Request.Headeris written intoLog.Otherand returned by the log APIs, so any other credential-bearing header would leak into persisted logs and exports. Consider allowlisting the few headers you actually need here and at the shared call sites.🤖 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 `@relay/mjproxy_handler.go` around lines 236 - 247, The consume-log path in mjproxy_handler’s RecordConsumeLog call is persisting almost all request headers via common.GetRequestHeaders, which can leak credential-bearing headers into Log.Other and API exports. Update the shared header extraction used here and at other call sites to allowlist only the minimal safe headers needed, or otherwise redact sensitive headers before passing them into model.RecordConsumeLog. Keep the fix centered on common.GetRequestHeaders and the RecordConsumeLogParams.RequestHeaders population so all persisted logs are sanitized consistently.
🧹 Nitpick comments (14)
web/default/src/components/html-content.tsx (1)
161-182: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueSync logic looks correct; minor optional optimization.
Wrapper creation, dark-class sync, and observer cleanup are all correctly handled, and
props.htmlis pre-sanitized upstream so no new injection surface is introduced. One nice-to-have: eachIsolatedHtmlContentinstance spins up its ownMutationObserverondocument.documentElement; if many isolated blocks render on the same page, a single shared/module-level observer notifying all mounted wrappers would reduce redundant observers.♻️ Optional: share a single observer across instances
+const darkClassListeners = new Set<() => void>() +let sharedObserver: MutationObserver | null = null + +function subscribeDarkClass(listener: () => void): () => void { + darkClassListeners.add(listener) + if (!sharedObserver) { + sharedObserver = new MutationObserver(() => { + darkClassListeners.forEach((l) => l()) + }) + sharedObserver.observe(document.documentElement, { + attributes: true, + attributeFilter: ['class'], + }) + } + return () => { + darkClassListeners.delete(listener) + if (darkClassListeners.size === 0) { + sharedObserver?.disconnect() + sharedObserver = null + } + } +}🤖 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 `@web/default/src/components/html-content.tsx` around lines 161 - 182, The current IsolatedHtmlContent effect creates a separate MutationObserver for every mounted instance, which is redundant when multiple blocks are on the page. Refactor the observer setup in the html-content.tsx effect around syncDarkClass, wrapper, and the cleanup return so that a single shared/module-level MutationObserver watches document.documentElement and notifies all active wrappers, while still disconnecting cleanly when the last instance unmounts.Dockerfile (2)
40-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
COPYinstead ofADDfor plain files.
go.mod/go.sumare plain text files;ADD's archive-extraction/URL-fetch behavior is unnecessary here and can hide unexpected behavior if the source ever changes.🔧 Proposed fix
-ADD go.mod go.sum ./ +COPY go.mod go.sum ./🤖 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 `@Dockerfile` around lines 40 - 42, The Dockerfile is using ADD for plain text dependency files, which is unnecessary and can introduce unintended behavior. Update the build stage to use COPY for go.mod and go.sum, and keep the vendor directory copied with COPY as well; the relevant instructions are in the Dockerfile around the dependency setup block.Source: Linters/SAST tools
30-30: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winGo base image patch version is behind the latest security release.
golang:1.26.1-alpineis valid, but the Go team has since shipped 1.26.2/1.26.3/1.26.4 with additional CVE fixes (e.g. crypto/x509, html/template, net/url). Consider bumping to the latest 1.26.x patch to pick up those fixes.🤖 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 `@Dockerfile` at line 30, The builder stage is still pinned to an older Go patch release, so update the golang base image used by the builder2 stage to the latest 1.26.x Alpine tag. Keep the existing Dockerfile stage structure intact and replace the current golang:1.26.1-alpine reference with the newer patch version so the build uses the latest security fixes.controller/billing.go (1)
83-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared
types.ErrorTypeNewAPIErrorconstant instead of hardcoding the string.This literal duplicates
types.ErrorTypeNewAPIError(defined intypes/error.go) and is also hardcoded separately incontroller/relay.go/middleware/utils.go. Centralizing avoids future drift if the error-type value changes again.♻️ Proposed fix
openAIError := types.OpenAIError{ Message: err.Error(), - Type: "omniapi_error", + Type: string(types.ErrorTypeNewAPIError), }🤖 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 `@controller/billing.go` around lines 83 - 87, The OpenAI error construction in billing.go is hardcoding the error type string instead of using the shared types.ErrorTypeNewAPIError constant. Update the OpenAIError initialization in the relevant billing handler to reference types.ErrorTypeNewAPIError, matching the existing pattern used around types.OpenAIError and keeping the value centralized alongside types/error.go, controller/relay.go, and middleware/utils.go.service/log_info_generate.go (1)
82-105: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift
appendIsImageGenerationuses a narrower image-detection signal thantext_quota.go's isImage logic.This only checks
common.IsImageGenerationModel(relayInfo.OriginModelName), while the billing path inservice/text_quota.goadditionally checksctx.GetBool("image_generation_call")and stashed log images to catch chat-route image generation (e.g. gpt-4o/gemini flash-image viamessage.images) that isn't in the static model-name list. IfGenerateTextOtherInforuns after image stashing has occurred, mirroring that broader check here would makeother["is_image"]consistently accurate for logs/analytics rather than relying on a separate downstream write to reconcile it.Also note
appendRequestHeadersrecomputescommon.GetRequestHeaders(ctx)independently of the same call already made inservice/quota.goforRecordConsumeLogParams.RequestHeaders— worth consolidating to a single call site.🤖 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 `@service/log_info_generate.go` around lines 82 - 105, `appendIsImageGeneration` is using only the static model-name check, so `other["is_image"]` can miss chat-route image generation cases already handled in `text_quota.go`. Update `appendIsImageGeneration` in `GenerateTextOtherInfo` to use the same broader image-detection logic as the billing path, including the `ctx.GetBool("image_generation_call")` flag and any stashed log images, while still keeping the `relaycommon.RelayInfo`/`common.IsImageGenerationModel` check. Also consider reusing the request headers already collected for quota logging instead of calling `common.GetRequestHeaders` again in `appendRequestHeaders`.service/log_image_test.go (1)
1-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for
PersistLogImages(billing/log invariant).Tests cover
ImageMimeFromFormat,parseDataURIImage, andStashImageURLs, butPersistLogImages— which writesother["image_count"]/other["log_images"]and decides whether to enqueue an upload — has no test. Given this directly shapes persisted log data, a small table test (url item vs. base64 item, storage enabled/disabled, oversized/no-data base64) would protect a real, user-visible logging contract.
As per coding guidelines: "Backend tests must protect real behavior, API contracts, billing/accounting invariants, data compatibility, or regression paths."🤖 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 `@service/log_image_test.go` around lines 1 - 65, PersistLogImages is untested, and it controls persisted log fields and upload decisions. Add a focused table-driven test around PersistLogImages that covers URL images vs. base64/data-URI images, storage enabled vs. disabled, and oversized or empty base64 inputs. Verify the other["image_count"] and other["log_images"] mutations, plus whether the upload/enqueue path is triggered, so the billing/log contract stays protected.Source: Path instructions
service/log_image.go (1)
65-92: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBase64 is fully decoded before the size cap is enforced.
base64.StdEncoding.DecodeStringallocates and decodes the entire payload beforelen(decoded) > maxStashImageBytesis checked (Line 84). A pathological upstream response (or a compromised/misbehaving channel) with a very large base64 string still forces a full allocation+decode, only to be discarded afterward. This partially defeats the purpose of the size guard as a memory-safety measure. Consider pre-checkinglen(img.Base64)(base64 length correlates directly with decoded length) before callingDecodeString, so oversized payloads are rejected without decoding.🛡️ Suggested fix
if img.Base64 == "" { continue } + // Reject clearly oversized payloads before decoding to avoid the + // allocation/CPU cost of decoding data we're going to discard. + if base64.StdEncoding.DecodedLen(len(img.Base64)) > maxStashImageBytes { + logger.LogWarn(ctx, "StashLogImages: image too large, skip storing bytes") + existing = append(existing, stashedImage{mimeType: img.MimeType}) + continue + } decoded, err := base64.StdEncoding.DecodeString(img.Base64)🤖 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 `@service/log_image.go` around lines 65 - 92, StashLogImages currently decodes the entire Base64 payload before checking the size cap, so oversized inputs still cause a large allocation. Add an upfront length check on img.Base64 in StashLogImages, using the encoded size to reject obviously too-large payloads before calling base64.StdEncoding.DecodeString. Keep the existing decoded-byte limit check as a secondary guard, and preserve the current handling for URL, decode failures, and mimeType-only placeholders.relay/channel/openai/relay_image.go (1)
239-262: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winStream handler still double-parses the response body.
The rationale added in
OpenaiImageHandlerexplicitly calls out avoiding re-deserializing large base64 image payloads twice.OpenaiImageJSONAsStreamHandlerstill runs two separatecommon.Unmarshalcalls on the sameresponseBody(once intoimageResp, once intousageResp), which is exactly the pattern the sibling handler was refactored to avoid. Consider consolidating into a single parse (reusing the same combined struct pattern) for consistency and to avoid the double-parse cost on base64-heavy image responses.🤖 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 `@relay/channel/openai/relay_image.go` around lines 239 - 262, OpenaiImageJSONAsStreamHandler is still parsing the same responseBody twice, which reintroduces the double-deserialization cost for large base64 image payloads. Refactor this handler to use a single unmarshal path like OpenaiImageHandler, reusing one combined response struct or equivalent so both image data and usage are extracted from the same parse. Keep the existing error handling and downstream calls to normalizeOpenAIUsage, applyUsagePostProcessing, and stashOpenAIImages intact while removing the redundant common.Unmarshal call.service/object_storage.go (1)
76-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for the pure key/extension helpers.
buildObjectKeyandmimeToExtare pure, easily-testable functions that encode important invariants (content-hash idempotency, extension fallback to png). Neither has test coverage in this PR.🤖 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 `@service/object_storage.go` around lines 76 - 107, Add unit tests for the pure helpers buildObjectKey and mimeToExt in service/object_storage.go. Cover buildObjectKey’s invariants: same input data should produce the same hash-based filename, a custom prefix should be preserved in the returned path, and an empty or dotted extension should fall back/normalize to the expected extension behavior. Cover mimeToExt’s mapping and fallback behavior for common MIME strings (jpeg/jpg, webp, gif, png) plus unknown or empty values returning png.web/default/src/features/usage-logs/components/dialogs/details-dialog.tsx (1)
1219-1223: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
LogImageCard's inline image type duplicates the already-importedLogImageItem.Reuse the shared type instead of redeclaring an equivalent inline shape.
♻️ Proposed fix
+import type { LogImageItem } from '../../types' function LogImageCard(props: { - image: { type: 'url' | 'base64'; url?: string; key?: string } + image: LogImageItem index: number requestId: string }) {🤖 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 `@web/default/src/features/usage-logs/components/dialogs/details-dialog.tsx` around lines 1219 - 1223, `LogImageCard` is redefining an inline image prop type that already exists as the imported `LogImageItem`. Update `LogImageCard` to use the shared `LogImageItem` type for its image prop instead of the duplicated inline shape, and keep the rest of the component signature unchanged so the type stays consistent across `details-dialog.tsx`.web/default/src/features/system-settings/integrations/object-storage-settings-section.tsx (2)
106-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueProp destructuring and file size vs. guidelines.
The component destructures
{ defaultValues }in its signature; the guideline prefers directprops.xxxaccess. Also, at ~375 lines with many repetitiveFormFieldblocks, this file exceeds the guideline's ~200-line rule of thumb and could extract a small generic field renderer to cut duplication.As per coding guidelines: "Do not destructure objects unless necessary, especially component props; prefer direct property access such as
props.xxxfor clarity" and "Keep files reasonably small; when a single file grows beyond about 200 lines, consider extracting subcomponents or custom hooks."🤖 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 `@web/default/src/features/system-settings/integrations/object-storage-settings-section.tsx` around lines 106 - 120, The component should stop destructuring props in the signature and use direct access via props.defaultValues in ObjectStorageSettingsSection for consistency with the prop-access guideline. Also reduce the file size by extracting the repetitive FormField sections into a small reusable field renderer or subcomponent, keeping the main ObjectStorageSettingsSection file closer to the recommended size.Source: Coding guidelines
53-67: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeprecated Zod error-message API.
Passing a plain string as the second argument to
.refine()/.min()is Zod 3-style and is deprecated (though still functional) in Zod 4 in favor of the unifiederrorparameter.♻️ Example migration to unified `error` param
endpoint: z.string().refine((value) => { const trimmed = value.trim() if (!trimmed) return true return /^https?:\/\//.test(trimmed) - }, t('Provide a valid URL starting with http:// or https://')), + }, { error: t('Provide a valid URL starting with http:// or https://') }), ... url_expire_seconds: z.coerce .number() .int() - .min(60, t('Minimum is 60 seconds')), + .min(60, { error: t('Minimum is 60 seconds') }),🤖 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 `@web/default/src/features/system-settings/integrations/object-storage-settings-section.tsx` around lines 53 - 67, The Zod schema in object-storage-settings-section.tsx still uses deprecated Zod 3-style string messages in the `refine` and `min` calls. Update the `endpoint` validation and `url_expire_seconds` validation to use the unified `error` parameter instead of passing a plain string as the second argument, keeping the existing messages and locating the changes in the schema chain where `z.string().refine(...)` and `z.coerce.number().int().min(...)` are defined.web/default/src/features/system-settings/object-storage/index.tsx (1)
27-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate default values across files.
defaultObjectStorageSettingshere duplicates the same literal defaults (region: 'auto',key_prefix: 'log-images',url_expire_seconds: 3600, etc.) also inlined insection-registry.tsx'sbuildfallback (?? 'auto',?? 'log-images',?? 3600). Consider extracting a single sharedDEFAULT_OBJECT_STORAGE_SETTINGSconstant to avoid the two drifting apart.🤖 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 `@web/default/src/features/system-settings/object-storage/index.tsx` around lines 27 - 37, The object storage defaults are duplicated between `defaultObjectStorageSettings` and the `build` fallback in `section-registry.tsx`, which risks them drifting apart. Extract a single shared `DEFAULT_OBJECT_STORAGE_SETTINGS` constant (or equivalent shared source) and use it in both `defaultObjectStorageSettings` and the fallback logic so `ObjectStorageSettingsType` defaults stay consistent.web/default/src/i18n/custom/index.ts (1)
41-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCustom bundles only cover en/zh.
Other supported locales (fr, ja, ru, vi) will fall back to the raw English key text for object-storage/log-images strings since no bundle is registered for them. Likely acceptable for an initial rollout, but worth tracking for full localization parity.
🤖 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 `@web/default/src/i18n/custom/index.ts` around lines 41 - 51, The custom bundle registration in custom/index.ts only adds objectStorageEn/logImagesEn and objectStorageZh/logImagesZh, so supported locales like fr, ja, ru, and vi never get these translation keys. Update CUSTOM_BUNDLES (and the i18n.addResourceBundle loop) to register the custom bundles for all supported locales, or explicitly alias those locales to the appropriate fallback bundle so object-storage/log-images strings resolve consistently.
🤖 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 @.gitignore:
- Line 9: The Docker build currently depends on vendor/ but .gitignore excludes
it, so fresh checkouts won’t have the directory available. Update the build flow
around Dockerfile and the docker build invocation so vendor/ is generated first
by running go mod vendor immediately before the image build, or otherwise ensure
vendor/ is committed and present when COPY vendor ./vendor and -mod=vendor are
used.
In `@common/gin.go`:
- Around line 385-398: GetRequestHeaders in common/gin.go is currently
persisting most incoming headers and only excluding Authorization and Cookie,
which can leak credentials or PII into
RecordConsumeLogParams.Other.request_headers and the usage-logs UI. Update
GetRequestHeaders to use a safe allowlist of low-risk diagnostic headers, or
broaden the denylist to exclude additional sensitive headers such as X-API-Key,
X-Goog-Api-Key, Proxy-Authorization, Referer, and forwarded-IP headers. Keep the
behavior centered around GetRequestHeaders so the downstream usage-log recording
stays unchanged while the returned map only contains safe values.
In `@controller/log.go`:
- Around line 89-94: The log query handler in the index parsing block currently
rejects a missing index instead of honoring the documented default of 0. Update
the logic around c.Query("index") and strconv.Atoi so an empty or absent index
is treated as 0, while still rejecting malformed or negative values; keep the
validation in the same log handler path that calls common.ApiErrorMsg.
In `@docker-compose.dev.yml`:
- Line 50: The Postgres image in the dev compose setup was upgraded from 15 to
17, but the existing dev_pg_data volume will be incompatible for anyone with old
local data. Update the dev environment guidance where docker-compose.dev.yml is
used to explicitly tell developers to remove the old volume with docker compose
down -v or to migrate their data before starting with postgres:17-alpine, so
existing setups do not fail on startup.
In `@Dockerfile`:
- Line 6: The retry loop in the builder stages is swallowing a failed bun
install because the last command in the loop can exit 0, so the Docker RUN step
succeeds even when dependencies were not installed. Update the builder and
builder-classic install steps to propagate the real bun install exit status
after retries, and keep the build reproducible by using the pinned oven/bun
image digest and preserving the lockfile-based install flags in the relevant
build stages.
In `@electron/main.js`:
- Line 75: The rebrand is incomplete because other strings in the same Electron
main flow still reference “New API” while this message already uses “OmniAPI”;
update the remaining user-facing labels and text in the relevant main-process
code paths, especially the window-title/notification text that mentions “另一个 New
API 窗口” and “New API 图标”, plus the tray menu label in the tray setup around the
“Show New API” item, so all occurrences consistently say OmniAPI.
In `@model/log.go`:
- Around line 456-509: BackfillLogImageKeys currently claims the retry loop
waits “about 10 seconds,” but the maxRetry backoff in BackfillLogImageKeys
actually adds up to much longer. Either reduce the retry count/sleep schedule so
the total wait matches the intended window, or update the comment to reflect the
real duration; keep the fix aligned with the retry logic around LOG_DB query
handling and the gorm.ErrRecordNotFound path.
In `@relay/channel/gemini/relay-gemini.go`:
- Around line 1662-1671: The stashed log image is always being labeled as
image/png even though Gemini predictions already carry the real MIME type.
Update the image logging path in relay-gemini.go around the openAIResponse/Data
append and service.StashLogImages call to use prediction.MimeType instead of a
hardcoded value, and keep the existing base64 guard so only non-empty images are
stashed.
In `@setting/system_setting/object_storage.go`:
- Around line 12-32: The ObjectStorageSetting field SecretAccessKey is still
being returned in plaintext because GetOptions only redacts names matching its
current suffix rules. Update the option-building/redaction logic to explicitly
treat object_storage.secret_access_key as sensitive, or broaden the filter so
ObjectStorageSetting.SecretAccessKey is excluded before the payload is sent to
the frontend. Keep the rest of the ObjectStorageSetting fields unchanged.
In `@web/default/package.json`:
- Around line 43-48: The dependency entries in package.json are using wildcard
versions, which breaks catalog-driven version consistency. Update the affected
dependencies in the package manifest to use explicit catalog references or
appropriate version ranges instead of *, and make sure the same fix is applied
to all listed occurrences, including the ones around the axios/clsx/dayjs block
and the react-icons/sse.js/oxfmt/oxlint block.
In `@web/default/src/features/system-settings/hooks/use-update-option.ts`:
- Around line 42-99: The shared module-level debounce/toast state in
useUpdateOption is coupling unrelated callers and can drop valid success
feedback. Update scheduleSuccessToast so it defers and re-checks hasBatchError
right before firing instead of returning early, and consider moving
invalidateOptionsTimer, invalidateStatusTimer, successToastTimer, hasBatchError,
and batchErrorTimer into per-hook/per-call-site state (for example via a
useRef-backed factory) so one settings update flow does not suppress another.
In
`@web/default/src/features/system-settings/integrations/email-settings-section.tsx`:
- Line 373: The placeholder in the email settings field is using an HTML-escaped
translation key, so React will render the entities literally. Update the
placeholder in the email settings component to use the normal text form with
angle brackets, and make sure the same text is used consistently in the
associated i18n entries instead of the escaped key. Use the existing
email-settings-section placeholder and its translation key as the place to fix
this.
In
`@web/default/src/features/system-settings/integrations/object-storage-settings-section.tsx`:
- Around line 122-157: The object-storage settings submit flow in onSubmit
currently sends each changed field through updateOption.mutateAsync one by one,
which can leave a partially applied configuration if one request fails. Change
this to apply the full set of updates atomically by batching all changed keys in
a single request if supported, or add rollback/compensation around the per-field
updates so the settings stay consistent. Use onSubmit, plainKeys, and
updateOption as the main points to refactor.
- Around line 92-104: The `toFormValues` mapping is pre-filling
`object_storage.secret_access_key` from `PrefixedDefaults`, which exposes a
sensitive secret in the form. Update `toFormValues` in
`object-storage-settings-section` to set `secret_access_key` to an empty local
default instead of using the server-provided value, and ensure the related
options payload handling omits `object_storage.secret_access_key` server-side.
In
`@web/default/src/features/usage-logs/components/columns/common-logs-columns.tsx`:
- Around line 828-860: The User-Agent column tooltip is using the wrong Base UI
wrapper props, so update the tooltip block in common-logs-columns.tsx to match
the same Base UI pattern used elsewhere in this file. In the cell renderer for
the user_agent column, replace TooltipProvider delayDuration with delay and
switch TooltipTrigger away from asChild to the wrapper’s render-based usage,
keeping the existing tooltip content and layout intact.
In `@web/default/src/features/usage-logs/components/dialogs/details-dialog.tsx`:
- Around line 1131-1144: The Request Headers section in details-dialog still
renders all captured headers verbatim, so add redaction before display or ensure
`common.GetRequestHeaders` filters additional sensitive names beyond
`Authorization` and `Cookie`. Update the rendering around
`other.request_headers` and `DetailRow` to mask or omit headers such as
`Proxy-Authorization`, `X-API-Key`, and `X-Auth-Token` so only safe values are
shown.
In `@web/default/src/i18n/locales/fr.json`:
- Line 4858: The French locale entry is out of sync with the OmniAPI rebrand and
still says “la nouvelle API” instead of “OmniAPI”. Update the translation value
for the warning string in fr.json to match the branded term used by the source
key, keeping the rest of the message unchanged. Use the exact locale entry for
the “Warning: Base URL should not end with /v1...” key as the place to fix it.
In `@web/default/src/i18n/locales/ja.json`:
- Around line 2699-2701: The Japanese translation for the OmniAPI sender entry
is broken because the placeholder replaced the email address with __ PH_0 __;
update the `OmniAPI <noreply@example.com>` key in `ja.json` to preserve
the full sender text, including the email address, so it stays consistent with
the other locale files.
In `@web/default/src/i18n/locales/ru.json`:
- Line 4858: The Russian locale entry still uses the old “New API” wording
instead of the OmniAPI brand. Update the translation for the warning string in
ru.json so it matches the rebranded OmniAPI terminology, consistent with the
other locale entries and the key text for this warning message.
In `@web/default/src/i18n/locales/zh.json`:
- Line 4603: The translation for the shared API key update string was made
provider-specific by inserting “OmniAPI” into zh.json, but the key is generic
and reused across edit-key flows. Update the zh.json entry for the string
“Update the API key by providing necessary info.” to match the provider-agnostic
wording used by the same key in other locales, keeping it generic and aligned
with the existing translation pattern.
---
Outside diff comments:
In `@relay/mjproxy_handler.go`:
- Around line 236-247: The consume-log path in mjproxy_handler’s
RecordConsumeLog call is persisting almost all request headers via
common.GetRequestHeaders, which can leak credential-bearing headers into
Log.Other and API exports. Update the shared header extraction used here and at
other call sites to allowlist only the minimal safe headers needed, or otherwise
redact sensitive headers before passing them into model.RecordConsumeLog. Keep
the fix centered on common.GetRequestHeaders and the
RecordConsumeLogParams.RequestHeaders population so all persisted logs are
sanitized consistently.
In `@web/classic/src/pages/About/index.jsx`:
- Around line 62-73: The About page still uses the removed i18n key in
customDescription, so update the t(...) call in About/index.jsx to the renamed
OmniAPI项目仓库地址: key used by the locale files. Verify the surrounding
customDescription JSX still renders the repo link correctly and that no other
references to the old New API项目仓库地址: key remain in this component.
---
Nitpick comments:
In `@controller/billing.go`:
- Around line 83-87: The OpenAI error construction in billing.go is hardcoding
the error type string instead of using the shared types.ErrorTypeNewAPIError
constant. Update the OpenAIError initialization in the relevant billing handler
to reference types.ErrorTypeNewAPIError, matching the existing pattern used
around types.OpenAIError and keeping the value centralized alongside
types/error.go, controller/relay.go, and middleware/utils.go.
In `@Dockerfile`:
- Around line 40-42: The Dockerfile is using ADD for plain text dependency
files, which is unnecessary and can introduce unintended behavior. Update the
build stage to use COPY for go.mod and go.sum, and keep the vendor directory
copied with COPY as well; the relevant instructions are in the Dockerfile around
the dependency setup block.
- Line 30: The builder stage is still pinned to an older Go patch release, so
update the golang base image used by the builder2 stage to the latest 1.26.x
Alpine tag. Keep the existing Dockerfile stage structure intact and replace the
current golang:1.26.1-alpine reference with the newer patch version so the build
uses the latest security fixes.
In `@relay/channel/openai/relay_image.go`:
- Around line 239-262: OpenaiImageJSONAsStreamHandler is still parsing the same
responseBody twice, which reintroduces the double-deserialization cost for large
base64 image payloads. Refactor this handler to use a single unmarshal path like
OpenaiImageHandler, reusing one combined response struct or equivalent so both
image data and usage are extracted from the same parse. Keep the existing error
handling and downstream calls to normalizeOpenAIUsage, applyUsagePostProcessing,
and stashOpenAIImages intact while removing the redundant common.Unmarshal call.
In `@service/log_image_test.go`:
- Around line 1-65: PersistLogImages is untested, and it controls persisted log
fields and upload decisions. Add a focused table-driven test around
PersistLogImages that covers URL images vs. base64/data-URI images, storage
enabled vs. disabled, and oversized or empty base64 inputs. Verify the
other["image_count"] and other["log_images"] mutations, plus whether the
upload/enqueue path is triggered, so the billing/log contract stays protected.
In `@service/log_image.go`:
- Around line 65-92: StashLogImages currently decodes the entire Base64 payload
before checking the size cap, so oversized inputs still cause a large
allocation. Add an upfront length check on img.Base64 in StashLogImages, using
the encoded size to reject obviously too-large payloads before calling
base64.StdEncoding.DecodeString. Keep the existing decoded-byte limit check as a
secondary guard, and preserve the current handling for URL, decode failures, and
mimeType-only placeholders.
In `@service/log_info_generate.go`:
- Around line 82-105: `appendIsImageGeneration` is using only the static
model-name check, so `other["is_image"]` can miss chat-route image generation
cases already handled in `text_quota.go`. Update `appendIsImageGeneration` in
`GenerateTextOtherInfo` to use the same broader image-detection logic as the
billing path, including the `ctx.GetBool("image_generation_call")` flag and any
stashed log images, while still keeping the
`relaycommon.RelayInfo`/`common.IsImageGenerationModel` check. Also consider
reusing the request headers already collected for quota logging instead of
calling `common.GetRequestHeaders` again in `appendRequestHeaders`.
In `@service/object_storage.go`:
- Around line 76-107: Add unit tests for the pure helpers buildObjectKey and
mimeToExt in service/object_storage.go. Cover buildObjectKey’s invariants: same
input data should produce the same hash-based filename, a custom prefix should
be preserved in the returned path, and an empty or dotted extension should fall
back/normalize to the expected extension behavior. Cover mimeToExt’s mapping and
fallback behavior for common MIME strings (jpeg/jpg, webp, gif, png) plus
unknown or empty values returning png.
In `@web/default/src/components/html-content.tsx`:
- Around line 161-182: The current IsolatedHtmlContent effect creates a separate
MutationObserver for every mounted instance, which is redundant when multiple
blocks are on the page. Refactor the observer setup in the html-content.tsx
effect around syncDarkClass, wrapper, and the cleanup return so that a single
shared/module-level MutationObserver watches document.documentElement and
notifies all active wrappers, while still disconnecting cleanly when the last
instance unmounts.
In
`@web/default/src/features/system-settings/integrations/object-storage-settings-section.tsx`:
- Around line 106-120: The component should stop destructuring props in the
signature and use direct access via props.defaultValues in
ObjectStorageSettingsSection for consistency with the prop-access guideline.
Also reduce the file size by extracting the repetitive FormField sections into a
small reusable field renderer or subcomponent, keeping the main
ObjectStorageSettingsSection file closer to the recommended size.
- Around line 53-67: The Zod schema in object-storage-settings-section.tsx still
uses deprecated Zod 3-style string messages in the `refine` and `min` calls.
Update the `endpoint` validation and `url_expire_seconds` validation to use the
unified `error` parameter instead of passing a plain string as the second
argument, keeping the existing messages and locating the changes in the schema
chain where `z.string().refine(...)` and `z.coerce.number().int().min(...)` are
defined.
In `@web/default/src/features/system-settings/object-storage/index.tsx`:
- Around line 27-37: The object storage defaults are duplicated between
`defaultObjectStorageSettings` and the `build` fallback in
`section-registry.tsx`, which risks them drifting apart. Extract a single shared
`DEFAULT_OBJECT_STORAGE_SETTINGS` constant (or equivalent shared source) and use
it in both `defaultObjectStorageSettings` and the fallback logic so
`ObjectStorageSettingsType` defaults stay consistent.
In `@web/default/src/features/usage-logs/components/dialogs/details-dialog.tsx`:
- Around line 1219-1223: `LogImageCard` is redefining an inline image prop type
that already exists as the imported `LogImageItem`. Update `LogImageCard` to use
the shared `LogImageItem` type for its image prop instead of the duplicated
inline shape, and keep the rest of the component signature unchanged so the type
stays consistent across `details-dialog.tsx`.
In `@web/default/src/i18n/custom/index.ts`:
- Around line 41-51: The custom bundle registration in custom/index.ts only adds
objectStorageEn/logImagesEn and objectStorageZh/logImagesZh, so supported
locales like fr, ja, ru, and vi never get these translation keys. Update
CUSTOM_BUNDLES (and the i18n.addResourceBundle loop) to register the custom
bundles for all supported locales, or explicitly alias those locales to the
appropriate fallback bundle so object-storage/log-images strings resolve
consistently.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 76619f39-4d77-4152-83b2-e6f0049e3825
⛔ Files ignored due to path filters (4)
go.sumis excluded by!**/*.sumweb/default/public/favicon.icois excluded by!**/*.icoweb/default/public/logo.pngis excluded by!**/*.pngweb/default/public/logo_raw.pngis excluded by!**/*.png
📒 Files selected for processing (102)
.dockerignore.gitignoreDockerfileVERSIONcommon/constants.gocommon/gin.gocommon/init.gocontroller/billing.gocontroller/log.gocontroller/relay.godocker-compose.dev.ymldto/openai_request.godto/openai_response.goelectron/build.shelectron/main.jselectron/package.jsongo.modmain.gomiddleware/recover.gomiddleware/utils.gomodel/log.gorelay/channel/gemini/relay-gemini.gorelay/channel/openai/adaptor.gorelay/channel/openai/chat_via_responses.gorelay/channel/openai/chat_via_responses_test.gorelay/channel/openai/relay-openai.gorelay/channel/openai/relay_image.gorelay/channel/openai/relay_responses.gorelay/mjproxy_handler.gorouter/api-router.goservice/error.goservice/log_image.goservice/log_image_test.goservice/log_info_generate.goservice/object_storage.goservice/quota.goservice/task_billing.goservice/text_quota.goservice/violation_fee.gosetting/system_setting/object_storage.gosetting/system_setting/theme.gotypes/error.goweb/classic/index.htmlweb/classic/src/components/layout/Footer.jsxweb/classic/src/components/table/channels/modals/EditChannelModal.jsxweb/classic/src/helpers/utils.jsxweb/classic/src/i18n/locales/en.jsonweb/classic/src/i18n/locales/fr.jsonweb/classic/src/i18n/locales/ja.jsonweb/classic/src/i18n/locales/ru.jsonweb/classic/src/i18n/locales/vi.jsonweb/classic/src/i18n/locales/zh-CN.jsonweb/classic/src/i18n/locales/zh-TW.jsonweb/classic/src/i18n/locales/zh.jsonweb/classic/src/pages/About/index.jsxweb/default/index.htmlweb/default/package.jsonweb/default/scripts/sync-i18n.mjsweb/default/src/assets/logo.tsxweb/default/src/components/html-content.tsxweb/default/src/components/layout/components/footer.tsxweb/default/src/components/layout/components/system-brand.tsxweb/default/src/components/layout/config/system-settings.config.tsweb/default/src/features/about/index.tsxweb/default/src/features/auth/components/legal-consent.tsxweb/default/src/features/auth/components/terms-footer.tsxweb/default/src/features/channels/components/drawers/channel-mutate-drawer.tsxweb/default/src/features/channels/constants.tsweb/default/src/features/home/components/sections/hero.tsxweb/default/src/features/home/index.tsxweb/default/src/features/system-settings/auth/passkey-section.tsxweb/default/src/features/system-settings/general/system-info-section.tsxweb/default/src/features/system-settings/hooks/use-update-option.tsweb/default/src/features/system-settings/integrations/email-settings-section.tsxweb/default/src/features/system-settings/integrations/object-storage-settings-section.tsxweb/default/src/features/system-settings/object-storage/index.tsxweb/default/src/features/system-settings/object-storage/section-registry.tsxweb/default/src/features/system-settings/site/index.tsxweb/default/src/features/system-settings/types.tsweb/default/src/features/usage-logs/api.tsweb/default/src/features/usage-logs/components/columns/common-logs-columns.tsxweb/default/src/features/usage-logs/components/dialogs/details-dialog.tsxweb/default/src/features/usage-logs/components/log-image-icon-button.tsxweb/default/src/features/usage-logs/lib/format.tsweb/default/src/features/usage-logs/types.tsweb/default/src/i18n/custom/index.tsweb/default/src/i18n/custom/log-images.en.jsonweb/default/src/i18n/custom/log-images.zh.jsonweb/default/src/i18n/custom/object-storage.en.jsonweb/default/src/i18n/custom/object-storage.zh.jsonweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.jsonweb/default/src/lib/constants.tsweb/default/src/main.tsxweb/default/src/routeTree.gen.tsweb/default/src/routes/_authenticated/system-settings/object-storage/$section.tsxweb/default/src/routes/_authenticated/system-settings/object-storage/index.tsxweb/package.json
Important
请提供人工撰写的简洁摘要,避免直接粘贴未经整理的 AI 输出。
📝 变更描述 / Description
核心修复:深浅色模式适配
修复了自定义 HTML 页面在 Shadow DOM 隔离渲染模式下,无法随系统自动切换深浅色主题的问题。
在 Shadow DOM 内部增加了一层包装容器(
wrapper),并通过MutationObserver实时监听外部主文档(document.documentElement)的class变化,将dark类名动态同步至该包装容器上。这使得被克隆进 Shadow DOM 的 Tailwind 深色模式 CSS 规则能够重新匹配生效。🔍 问题背景与根因排查
关于自定义页面的渲染逻辑,最近经历了数次变更,导致了样式连环失效。经过在本地的详细排查与测试,还原了完整的原因:
1bff599): 出于防止恶意代码影响全局的考虑,核心团队将渲染方案重写为了 Shadow DOM 隔离渲染 并严格限制了DOMPurify规则。虽然杜绝了外部污染,但 Shadow DOM 的“绝对隔离”特性使得系统原本的 Tailwind 样式彻底进不去自定义内容中,导致了“样式白板”现象。<style>标签克隆进 Shadow DOM 来恢复排版。但遗漏了 Tailwind 深浅色生效的核心机制(依赖祖先元素上的.dark类名)。由于 Shadow DOM 内部无法感知外部标签的 class,这些深色 CSS 永远无法命中触发,导致深浅色切换失效。本 PR 补齐了这“最后一块拼图”。
🤔 关于技术路线的架构探讨
虽然本 PR 已彻底修复当前 Shadow DOM 下的深浅色问题,但借此想向核心团队提出一个产品层面的抉择探讨:
采用目前的 Shadow DOM + 强过滤 机制,意味着管理员在后台添加的任何包含
<script>的代码都必定无法执行。这将导致以下常见场景永远无法在自定义页面中实现:如果团队认为安全性高于一切,本 PR 提供了完善的深浅色修复,可直接合并(预期内隔离)。
如果团队认为应将选择权交还给管理员(毕竟是最高权限后台配置),建议后续考虑退回到 #5795 最初的逻辑,允许管理员自行对其配置的外部代码安全负责。抛砖引玉,供各位定夺。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
📸 运行证明 / Proof of Work
bun run typecheck与bunx oxlint均通过无异常。Summary by CodeRabbit