合并 - #6418
Conversation
Enable multi-arch image publishing to ghcr.io on version tags and manual dispatch. Co-authored-by: Cursor <cursoragent@cursor.com>
Checkout the triggering commit directly instead of treating the image tag as a git ref. Co-authored-by: Cursor <cursoragent@cursor.com>
Docker requires lowercase registry paths; normalize github.repository before tagging. Co-authored-by: Cursor <cursoragent@cursor.com>
The create_manifests job needs write access to publish multi-arch tags. Co-authored-by: Cursor <cursoragent@cursor.com>
Create a token with the selected group and open CC Switch or Cherry Studio via deep link, returning the plaintext key from AddToken for one-click setup. Co-authored-by: Cursor <cursoragent@cursor.com>
Expose enable_groups_by_endpoint in pricing and narrow the wizard to Anthropic/OpenAI models for the selected protocol and group. Co-authored-by: Cursor <cursoragent@cursor.com>
# Conflicts: # web/default/src/i18n/locales/_reports/_sync-report.json # web/default/src/i18n/locales/en.json # web/default/src/i18n/locales/fr.json # web/default/src/i18n/locales/ja.json # web/default/src/i18n/locales/ru.json # web/default/src/i18n/locales/vi.json # web/default/src/i18n/locales/zh-TW.json # web/default/src/i18n/locales/zh.json
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
WalkthroughThis change adds extension administration, custom pages, availability monitoring, lottery gameplay, strict Turnstile verification, API-key connection tooling, updated home-page embedding, localization, routing, and a GHCR multi-architecture Docker publishing workflow. ChangesExtensions and lottery
Security and integrations
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant ExtensionsSettingsPage
participant UpdateOption
participant GetStatus
participant Sidebar
Admin->>ExtensionsSettingsPage: edit extension settings
ExtensionsSettingsPage->>UpdateOption: persist changed options
UpdateOption->>GetStatus: refresh status-related configuration
GetStatus-->>Sidebar: return enabled extensions and visibility
Sidebar-->>Admin: render available extension entries
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)web/default/src/i18n/locales/zh.jsonTraceback (most recent call last): web/package.jsonTraceback (most recent call last): .github/workflows/docker-ghcr-custom.ymlTraceback (most recent call last):
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 Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
middleware/turnstile_strict_test.go (1)
12-89: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winNew test file doesn't use
testify.None of
TestTurnstileCheckStrictDisabled,TestTurnstileCheckStrictRequiresToken, orTestTurnstileCheckStrictEveryRequestusetestify/requireortestify/assert; they rely on rawt.Fatalf.As per coding guidelines: "New or substantially rewritten Go backend tests must use
testify/requirefor setup and fatal assertions andtestify/assertfor non-fatal checks."🤖 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 `@middleware/turnstile_strict_test.go` around lines 12 - 89, Update TestTurnstileCheckStrictDisabled, TestTurnstileCheckStrictRequiresToken, and TestTurnstileCheckStrictEveryRequest to use testify/require for setup and fatal assertions, and testify/assert for non-fatal checks. Replace the custom contains helper and raw t.Fatalf assertions with the appropriate testify assertions while preserving the existing test behavior.Source: Coding guidelines
🟡 Minor comments (39)
web/default/src/i18n/locales/en.json-3631-3631 (1)
3631-3631: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the duplicate
No changes to savekey.The same key already exists at Line 2941. Duplicate JSON keys make parser and tooling behavior ambiguous; retain one canonical entry.
Proposed fix
- "No changes to save": "No changes to save",🤖 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/locales/en.json` at line 3631, Remove the duplicate “No changes to save” entry from the locale JSON, retaining the existing canonical entry near the earlier occurrence. Ensure the resulting object contains this key only once.web/default/src/i18n/locales/en.json-3601-3601 (1)
3601-3601: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the corrupted suffix from the English translation.
V me 50!makes the English source string incoherent and will be displayed that way to users. Replace it with approved English copy or confirm the marketing text is intentional.🤖 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/locales/en.json` at line 3601, Update the English translation value for the key “Prize pool and free prize amounts are doubled today. V me 50!” to remove the corrupted “V me 50!” suffix, using the approved English marketing copy while preserving the intended prize-pool message.web/default/src/i18n/locales/fr.json-1456-1456 (1)
1456-1456: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse consistent terminology for “redemption code”.
These entries use “code de réduction”, which means discount code, while the surrounding locale consistently uses “code d’échange”. The current wording changes the feature’s meaning.
Suggested translations
- "Do you want to download the created redemption codes as a text file?": "Voulez-vous télécharger les codes de réduction créés sous forme de fichier texte ?", + "Do you want to download the created redemption codes as a text file?": "Voulez-vous télécharger les codes d’échange créés sous forme de fichier texte ?", - "Filter by name, ID, or redemption code...": "Filtrer par nom, ID ou code de réduction...", + "Filter by name, ID, or redemption code...": "Filtrer par nom, ID ou code d’échange...", - "The download will use the redemption name as the filename.": "Le fichier sera enregistré en utilisant le nom du code de réduction comme nom de fichier.", + "The download will use the redemption name as the filename.": "Le fichier sera enregistré en utilisant le nom du code d’échange comme nom de fichier.",Also applies to: 2004-2004, 4513-4513
🤖 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/locales/fr.json` at line 1456, Update the French translations for the redemption-code entries at the shown locations to use the established “code d’échange” terminology instead of “code de réduction,” including the download prompt and the two additional matching entries.web/default/src/i18n/locales/fr.json-827-828 (1)
827-828: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the full “Availability Monitor” name.
“Availability Monitor” is translated as generic “surveillance”, although the established label is “Surveillance de disponibilité” at Line [532]. This is ambiguous alongside other monitoring features.
Suggested translations
- "Choose who can see the Availability Monitor entry in the Extensions sidebar.": "Choisissez qui voit la surveillance dans Extensions.", + "Choose who can see the Availability Monitor entry in the Extensions sidebar.": "Choisissez qui peut voir l’entrée « Surveillance de disponibilité » dans la barre latérale Extensions.", - "Enable availability monitor": "Activer la surveillance", + "Enable availability monitor": "Activer la surveillance de disponibilité",Also applies to: 1596-1596
🤖 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/locales/fr.json` around lines 827 - 828, Update the French translation for “Choose who can see the Availability Monitor entry in the Extensions sidebar.” to use the established full “Surveillance de disponibilité” label, matching the terminology used elsewhere in fr.json. Apply the same terminology correction to the additional occurrence referenced by the review comment, while leaving the generic page translation unchanged.web/default/src/i18n/locales/fr.json-2925-2925 (1)
2925-2925: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep “provider” in provider-type translations.
The current French values say only “type”, omitting the provider qualifier used by these selectors and empty states.
Suggested translations
- "No groups available for this provider type": "Aucun groupe disponible pour ce type", + "No groups available for this provider type": "Aucun groupe disponible pour ce type de fournisseur", - "No provider types are available for your current groups.": "Aucun type n’est disponible pour vos groupes actuels.", + "No provider types are available for your current groups.": "Aucun type de fournisseur n’est disponible pour vos groupes actuels.", - "No provider types are available. Types unlock when pricing models match Anthropic / OpenAI / Gemini / xAI for your groups — having channels alone is not enough.": "Aucun type disponible. Les types s’activent quand les modèles tarifés correspondent à Anthropic / OpenAI / Gemini / xAI pour vos groupes — avoir des canaux ne suffit pas.", + "No provider types are available. Types unlock when pricing models match Anthropic / OpenAI / Gemini / xAI for your groups — having channels alone is not enough.": "Aucun type de fournisseur disponible. Ces types s’activent quand les modèles tarifés correspondent à Anthropic / OpenAI / Gemini / xAI pour vos groupes — avoir des canaux ne suffit pas.", - "No provider types are available. Types unlock when pricing models match Anthropic / OpenAI for your groups — having channels alone is not enough.": "Aucun type disponible. Les types s’activent quand les modèles tarifés correspondent à Anthropic / OpenAI pour vos groupes — avoir des canaux ne suffit pas.", + "No provider types are available. Types unlock when pricing models match Anthropic / OpenAI for your groups — having channels alone is not enough.": "Aucun type de fournisseur disponible. Ces types s’activent quand les modèles tarifés correspondent à Anthropic / OpenAI pour vos groupes — avoir des canaux ne suffit pas.", - "Pick a provider type, group, model, and client. We create an API key and open the tool for you.": "Choisissez le type, le groupe, le modèle et le client. Nous créons une clé API et ouvrons l’outil.", + "Pick a provider type, group, model, and client. We create an API key and open the tool for you.": "Choisissez le type de fournisseur, le groupe, le modèle et le client. Nous créons une clé API et ouvrons l’outil.",Also applies to: 2988-2990, 3381-3381
🤖 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/locales/fr.json` at line 2925, Update the French translations for the provider-type messages, including “No groups available for this provider type” and the additionally referenced entries, so each value explicitly retains the “provider” qualifier while preserving the existing meaning and French wording.web/default/src/i18n/locales/fr.json-41-41 (1)
41-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve “custom” in custom-page labels and messages.
These translations reduce “custom page(s)” to generic “page(s)”, making the feature less precise and inconsistent with the surrounding translations.
Suggested translations
- "{{count}} custom pages deleted. Click \"Save Settings\" to apply.": "{{count}} pages supprimées. Cliquez sur « Enregistrer » pour appliquer.", + "{{count}} custom pages deleted. Click \"Save Settings\" to apply.": "{{count}} pages personnalisées supprimées. Cliquez sur « Enregistrer » pour appliquer.", - "Add Custom Page": "Ajouter une page", + "Add Custom Page": "Ajouter une page personnalisée", - "Custom page added. Click \"Save Settings\" to apply.": "Page ajoutée. Cliquez sur « Enregistrer » pour appliquer.", + "Custom page added. Click \"Save Settings\" to apply.": "Page personnalisée ajoutée. Cliquez sur « Enregistrer » pour appliquer.", - "Custom page deleted. Click \"Save Settings\" to apply.": "Page supprimée. Cliquez sur « Enregistrer » pour appliquer.", + "Custom page deleted. Click \"Save Settings\" to apply.": "Page personnalisée supprimée. Cliquez sur « Enregistrer » pour appliquer.", - "Edit Custom Page": "Modifier la page", + "Edit Custom Page": "Modifier la page personnalisée",Also applies to: 181-181, 1236-1237, 1551-1551
🤖 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/locales/fr.json` at line 41, Update the French translations for the affected custom-page labels and messages, including the entry containing "{{count}} custom pages deleted..." and the other referenced entries, to explicitly preserve the meaning of “custom” rather than translating only “page(s)”. Keep the existing French wording and formatting otherwise unchanged.web/default/src/i18n/locales/fr.json-3808-3808 (1)
3808-3808: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the request-health thresholds.
The source explains the exact normal/warning/abnormal thresholds, but the French translation omits them, preventing users from interpreting the badge colors.
Suggested translation
- "Request health by billing group for the latest 100 consume/error logs. Green bars show latency; red bars are failures. Badge uses overall success rate (≥95% normal, ≥80% warning, below 80% abnormal).": "Santé des requêtes par groupe (100 derniers logs). Barres vertes = latence, rouges = échecs. Badge selon le taux de succès.", + "Request health by billing group for the latest 100 consume/error logs. Green bars show latency; red bars are failures. Badge uses overall success rate (≥95% normal, ≥80% warning, below 80% abnormal).": "Santé des requêtes par groupe pour les 100 derniers journaux de consommation et d’erreur. Les barres vertes indiquent la latence ; les barres rouges, les échecs. Le badge utilise le taux de réussite global (≥95 % : normal, ≥80 % : avertissement, <80 % : anormal).",🤖 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/locales/fr.json` at line 3808, Update the French translation for the request-health description in the locale entry beginning “Request health by billing group” to preserve the explicit badge thresholds: at least 95% normal, at least 80% warning, and below 80% abnormal. Keep the existing latency and failure-bar explanation intact while adding these threshold details.web/default/src/i18n/locales/ja.json-2527-2527 (1)
2527-2527: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate “providers” as providers, not types.
The source says “Loading available providers…”, but the Japanese text says “loading available types,” which mislabels the resource being loaded. Use
利用可能なプロバイダーを読み込み中….Proposed fix
- "Loading available providers...": "利用可能なタイプを読み込み中…", + "Loading available providers...": "利用可能なプロバイダーを読み込み中…",🤖 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/locales/ja.json` at line 2527, Update the translation value for the “Loading available providers...” key in the Japanese locale to use “利用可能なプロバイダーを読み込み中…” instead of referring to types.web/default/src/i18n/locales/ru.json-1457-1457 (1)
1457-1457: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate “Docs” as documentation.
Документыmeans documents; useДокументацияto match the adjacentDocumentationtranslation and the intended navigation label.🤖 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/locales/ru.json` at line 1457, Update the “Docs” locale entry in ru.json from “Документы” to “Документация”, matching the existing “Documentation” translation and intended navigation label.web/default/src/i18n/locales/ru.json-2527-2527 (1)
2527-2527: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not translate providers as types.
The source says “Loading available providers…”, but the translation says “Loading available types…”. This misleads users in the connect-tool flow.
🤖 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/locales/ru.json` at line 2527, Update the Russian translation for the “Loading available providers...” locale entry to refer to providers, not types, while preserving the existing loading message and punctuation.web/default/src/i18n/locales/ru.json-1596-1596 (1)
1596-1596: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve “availability” in the setting label.
Включить мониторингis ambiguous. Translate this asВключить мониторинг доступности, consistent withAvailability Monitor.🤖 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/locales/ru.json` at line 1596, Update the Russian translation for the “Enable availability monitor” key to explicitly include “доступности,” using “Включить мониторинг доступности” while leaving the key and other locale entries unchanged.web/default/src/i18n/locales/ru.json-124-124 (1)
124-124: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate “Abnormal” as a status, not a failure.
Сбойmeans failure/outage, whereas this status is paired withNormal(Норма). Use a status equivalent such asАномальноorНенорма.🤖 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/locales/ru.json` at line 124, Update the “Abnormal” translation in the locale data to use a Russian status equivalent such as “Аномально” or “Ненорма,” matching the existing “Normal” status pairing; do not use “Сбой,” which denotes a failure or outage.web/default/src/i18n/locales/ru.json-827-828 (1)
827-828: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the “Extensions” section name consistently.
These strings leave
Extensionsin English even though the locale translates the same section asРасширенияat line 1800 and elsewhere.Also applies to: 4220-4220
🤖 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/locales/ru.json` around lines 827 - 828, Update the Russian translations for the Availability Monitor and page visibility strings to use the established localized section name “Расширения” instead of “Extensions,” including the additional occurrence identified by the review.web/default/src/i18n/locales/ru.json-1456-1456 (1)
1456-1456: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse consistent terminology for redemption codes.
The locale consistently translates “redemption code” as
код активации, but these entries useкод пополнения, which refers to a different concept.Also applies to: 4513-4513
🤖 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/locales/ru.json` at line 1456, Update the Russian translations for the redemption-code entries, including the entry near the referenced “Do you want to download…” key and the corresponding entry near the second occurrence, replacing “код пополнения” terminology with the established “код активации” wording while preserving the surrounding sentence meaning.web/default/src/i18n/locales/ru.json-3670-3670 (1)
3670-3670: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid invalid Russian count inflection.
Последние {{count}} записейis incorrect for counts such as 1 or 21. Use invariant wording such asЗаписей: {{count}}or locale-aware pluralization.🤖 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/locales/ru.json` at line 3670, Update the Russian translation for “Recent {{count}} records” to avoid fixed, grammatically incorrect count inflection. Use invariant wording such as “Записей: {{count}}” or the locale’s pluralization mechanism while preserving the {{count}} placeholder.web/default/src/i18n/locales/ru.json-41-41 (1)
41-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the exact “Save Settings” label.
These translations say
Сохранить, while the source explicitly references theSave Settingsbutton, translated elsewhere asСохранить настройки. Keep the instruction consistent with the actual UI label.Also applies to: 1236-1237, 1239-1239
🤖 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/locales/ru.json` at line 41, Update the Russian translation for the custom-pages deletion message and the additional occurrences to use the exact UI label translation “Сохранить настройки” instead of “Сохранить,” while preserving the rest of each message.web/default/src/i18n/locales/ru.json-3808-3808 (1)
3808-3808: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the badge interpretation details in the translation.
The translation omits the source’s explanation of green/red bars and the success-rate thresholds (
≥95%,≥80%, below80%), leaving Russian users without the operational meaning of the badge.🤖 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/locales/ru.json` at line 3808, Update the Russian translation for the request-health description in the locale entry to preserve the source’s full operational details: billing-group scope, latest 100 consume/error logs, green bars for latency, red bars for failures, and badge thresholds of ≥95% normal, ≥80% warning, and below 80% abnormal.web/default/src/i18n/locales/vi.json-2884-2884 (1)
2884-2884: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the established billing-group terminology.
This entry uses “nhóm thanh toán”, while related billing strings use “nhóm tính phí”. Align the wording to avoid confusing users in the billing UI.
🤖 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/locales/vi.json` at line 2884, Update the Vietnamese translation for “No billing groups configured.” to use the established “nhóm tính phí” terminology instead of “nhóm thanh toán”, matching the wording used by related billing strings.web/default/src/i18n/locales/vi.json-2925-2925 (1)
2925-2925: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRetain “provider type” in the empty state.
“Không có nhóm nào cho loại này” drops “provider” and makes “type” ambiguous. Use “Không có nhóm nào cho loại nhà cung cấp này”.
🤖 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/locales/vi.json` at line 2925, Update the Vietnamese translation for “No groups available for this provider type” to retain the provider-specific meaning by using “Không có nhóm nào cho loại nhà cung cấp này”.web/default/src/i18n/locales/vi.json-2527-2527 (1)
2527-2527: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate “providers”, not “types”.
“Loading available providers...” is rendered as “Đang tải loại khả dụng…” (“loading available types”), which changes the UI meaning. Use “Đang tải các nhà cung cấp khả dụng…”.
🤖 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/locales/vi.json` at line 2527, Update the Vietnamese translation for the “Loading available providers...” locale entry to use “Đang tải các nhà cung cấp khả dụng…”, replacing the current wording that refers to types.web/default/src/i18n/locales/vi.json-2988-2990 (1)
2988-2990: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not reduce “provider types” to generic “types”.
These translations repeatedly omit “nhà cung cấp”, making the availability condition unclear. Use “loại nhà cung cấp” throughout these messages.
🤖 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/locales/vi.json` around lines 2988 - 2990, Update the Vietnamese translations for the three provider-type availability keys to use “loại nhà cung cấp” consistently instead of the generic “loại”, while preserving the existing meaning and availability conditions.web/default/src/i18n/locales/vi.json-3307-3307 (1)
3307-3307: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate “Past” as a time range.
“Trước” means “before”, not “past”. Use “Đã qua” or the project’s established Vietnamese term for historical time ranges.
🤖 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/locales/vi.json` at line 3307, Update the Vietnamese “Past” translation in the locale entry to use “Đã qua” or the project’s established term for a historical time range, replacing “Trước” while leaving other translations unchanged.web/default/src/i18n/locales/vi.json-3381-3381 (1)
3381-3381: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep “provider type” in the setup prompt.
“Chọn loại…” omits that the first selection is a provider type. Use “Chọn loại nhà cung cấp, nhóm, mô hình và client…”.
🤖 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/locales/vi.json` at line 3381, Update the Vietnamese translation for the setup prompt to explicitly include “nhà cung cấp” after “loại,” preserving the existing meaning and the remainder of the translated message.web/default/src/i18n/locales/vi.json-3808-3808 (1)
3808-3808: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not omit the chart’s interpretation details.
The translation drops the “latest 100 consume/error logs” scope and the badge thresholds (≥95% normal, ≥80% warning, below 80% abnormal), which are essential for interpreting this monitoring UI.
🤖 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/locales/vi.json` at line 3808, Update the Vietnamese translation value for the request-health description to preserve the latest 100 consume/error logs scope and explicitly include the badge thresholds: at least 95% normal, at least 80% warning, and below 80% abnormal, while retaining the latency and failure bar explanations.web/default/src/i18n/locales/vi.json-1236-1239 (1)
1236-1239: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve “custom page” in the lifecycle messages.
“Custom page added/deleted/updated” is translated as generic “trang”, making these notifications ambiguous. Use “trang tùy chỉnh” consistently.
🤖 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/locales/vi.json` around lines 1236 - 1239, Update the Vietnamese translations for the “Custom page added,” “Custom page deleted,” and “Custom page updated” entries to consistently use “trang tùy chỉnh” instead of generic “trang,” while preserving the existing notification meaning and formatting.web/default/src/i18n/locales/vi.json-827-828 (1)
827-828: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the full sidebar context in these translations.
The translations omit “sidebar”; line 827 also reduces “Availability Monitor” to generic “giám sát” and leaves “Extensions” untranslated. Use consistent wording such as “mục Giám sát khả dụng trong thanh bên Mở rộng”.
🤖 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/locales/vi.json` around lines 827 - 828, Update the two Vietnamese translations in the locale entries for “Choose who can see the Availability Monitor entry...” and “Choose who can see this page...” to preserve the full context: explicitly mention the Availability Monitor as “Giám sát khả dụng,” identify the sidebar as “thanh bên,” and translate “Extensions” consistently as “Mở rộng.”web/default/src/i18n/locales/zh-TW.json-41-42 (1)
41-42: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTranslate the newly added
zh-TWvalues.Many entries leave the Traditional Chinese value identical to the English source, causing mixed-language UI. Translate these values while preserving placeholders and technical terms.
As per coding guidelines, all user-facing copy must support i18n.
Also applies to: 124-125, 182-182, 249-249, 401-401, 539-539, 541-541, 556-556, 846-847, 983-983, 1009-1010, 1018-1018, 1053-1053, 1154-1154, 1167-1167, 1256-1261, 1484-1484, 1579-1579, 1616-1616, 1624-1624, 1668-1668, 1784-1784, 1829-1829, 1908-1908, 1941-1941, 2332-2332, 2563-2563, 2605-2605, 2864-2864, 2936-2936, 2958-2958, 2977-2977, 3040-3042, 3046-3046, 3098-3098, 3119-3119, 3180-3180, 3200-3200, 3362-3362, 3436-3436, 3643-3643, 3737-3737, 3787-3787, 3833-3833, 3878-3878, 4130-4130, 4145-4145, 4179-4179, 4205-4205, 4290-4291, 4603-4603, 4642-4642, 4662-4662, 4718-4718, 4877-4877, 5010-5010, 5164-5164
🤖 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/locales/zh-TW.json` around lines 41 - 42, Translate every newly added Traditional Chinese locale value currently identical to its English source, including the listed entries in the zh-TW locale, while preserving placeholders such as {{count}} and established technical terms. Keep the translation keys unchanged and ensure all affected user-facing messages are natural Traditional Chinese.Source: Coding guidelines
web/default/src/i18n/locales/zh-TW.json-631-631 (1)
631-631: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the quota translation.
“Current quota” is translated as “current balance,” which changes the lottery restriction’s meaning.
- "Bet cannot exceed your current quota": "投入額度不能超過當前餘額", + "Bet cannot exceed your current quota": "投入額度不能超過目前額度",As per coding guidelines, user-facing copy must accurately support i18n.
🤖 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/locales/zh-TW.json` at line 631, Update the zh-TW translation for the “Bet cannot exceed your current quota” key so “current quota” is accurately translated as the user’s current quota rather than current balance. Preserve the existing key and JSON structure.Source: Coding guidelines
web/default/src/i18n/locales/zh.json-2563-2563 (1)
2563-2563: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTranslate “providers” as providers, not types.
The current text says “正在加载可用类型…”, which changes the meaning of the loading state.
Proposed fix
- "Loading available providers...": "正在加载可用类型…", + "Loading available providers...": "正在加载可用提供商...",🤖 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/locales/zh.json` at line 2563, Update the zh.json translation for “Loading available providers...” so “providers” is translated as providers rather than types, while preserving the existing loading-state meaning and punctuation.web/default/src/i18n/locales/zh.json-401-401 (1)
401-401: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep “API key” terminology consistent.
Line 401 translates “API key” as “令牌”, while the surrounding entries use “API 密钥”. This can confuse users about which credential was created.
Proposed fix
- "API key created. Opening the selected tool...": "令牌已创建，正在打开所选工具...", + "API key created. Opening the selected tool...": "API 密钥已创建，正在打开所选工具...",🤖 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/locales/zh.json` at line 401, Update the zh.json translation for “API key created. Opening the selected tool...” to use the established “API 密钥” terminology instead of “令牌”, while preserving the rest of the message meaning.middleware/turnstile_strict.go-52-61 (1)
52-61: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winRaw internal error returned to the client.
err.Error()from the Cloudflare call is sent verbatim in the JSON response body (Line 57), in addition to being logged (Line 54). This can leak internal network/DNS/infra details to the caller. Return a generic message and rely on the existingcommon.SysLogfor diagnostics.🤖 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 `@middleware/turnstile_strict.go` around lines 52 - 61, The Turnstile verification error path exposes the raw Cloudflare error through the JSON response. In the handler around turnstileVerifyFunc, keep logging err.Error() with common.SysLog, but replace the client-facing message in c.JSON with a generic verification failure message while preserving the existing failure status and abort behavior.middleware/turnstile_strict_test.go-27-44 (1)
27-44: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMissing cleanup leaks global Turnstile state to later tests.
Unlike
TestTurnstileCheckStrictEveryRequest(which restores state viat.Cleanup, Lines 55-58), this test setscommon.TurnstileCheckEnabled = trueandcommon.TurnstileSecretKey = "secret"but never resets them, leaving mutated package-level state for subsequent tests in the binary.🧹 Proposed fix
func TestTurnstileCheckStrictRequiresToken(t *testing.T) { gin.SetMode(gin.TestMode) common.TurnstileCheckEnabled = true common.TurnstileSecretKey = "secret" + t.Cleanup(func() { + common.TurnstileCheckEnabled = false + common.TurnstileSecretKey = "" + }) r := gin.New()🤖 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 `@middleware/turnstile_strict_test.go` around lines 27 - 44, Update TestTurnstileCheckStrictRequiresToken to register t.Cleanup that restores common.TurnstileCheckEnabled and common.TurnstileSecretKey to their original values after the test, matching the cleanup pattern in TestTurnstileCheckStrictEveryRequest.controller/option.go-155-165 (1)
155-165: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEmpty value can never actually clear
TurnstileSecretKey.Treating both
""and"********"as "keep existing" means there's no way to intentionally unset the secret once configured — the request returnssuccess: truebut silently makes no change, which is misleading if the admin's intent was to clear it.Consider only treating the mask sentinel (
"********") as "keep existing", and letting a genuinely empty submission clear the value as usual.🔧 Proposed fix to allow explicit clearing
if option.Key == "TurnstileSecretKey" { secret := option.Value.(string) - if secret == "" || secret == "********" { + if secret == "********" { c.JSON(http.StatusOK, gin.H{ "success": true, "message": "", }) return } }🤖 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/option.go` around lines 155 - 165, Update the TurnstileSecretKey handling in the option update flow to treat only the "********" mask sentinel as “keep existing.” Allow an empty secret value to continue through the normal update path so it explicitly clears the configured value, while preserving the current success response and early return for masked submissions.web/default/src/features/extensions/availability/components/heartbeat-bars.tsx-49-76 (1)
49-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNon-unique React
keyrisk for duplicate timestamp/duration records.The composite key
${created_at}-${use_time}-${ok ? 'ok' : 'err'}isn't guaranteed unique — e.g., multiple failed requests logged in the same second withuse_time: 0would collide, risking incorrect DOM reuse/reconciliation across re-renders.🔑 Proposed fix using index for uniqueness
- props.records.map((record) => { + props.records.map((record, index) => { let heightPct = FAIL_HEIGHT_PCT ... return ( <div - key={`${record.created_at}-${record.use_time}-${record.ok ? 'ok' : 'err'}`} + key={`${index}-${record.created_at}-${record.ok ? 'ok' : 'err'}`}🤖 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/extensions/availability/components/heartbeat-bars.tsx` around lines 49 - 76, The records rendered in the heartbeat-bars map can produce duplicate React keys when timestamp, duration, and status match. Update the map callback around the heartbeat bar JSX to include the record index in the key, while preserving the existing key components and rendering behavior.web/default/src/features/extensions/lottery/hooks/use-marquee-spin.ts-42-55 (1)
42-55: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winTimers leak on unmount; the free-run
tickloop keeps rescheduling forever.There is no unmount cleanup. If the component unmounts mid-spin,
runningRef.currentstaystrue, sotickkeeps callingsetActiveIndexand re-scheduling itself indefinitely — a runaway timer loop plus state updates on an unmounted component. Add an effect that clears timers and stops running on unmount.🧹 Proposed cleanup on unmount
-import { useCallback, useRef, useState } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react'const clearTimers = useCallback(() => { for (const id of timersRef.current) window.clearTimeout(id) timersRef.current = [] }, []) + + useEffect(() => { + return () => { + runningRef.current = false + stoppingRef.current = false + for (const id of timersRef.current) window.clearTimeout(id) + timersRef.current = [] + } + }, [])🤖 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/extensions/lottery/hooks/use-marquee-spin.ts` around lines 42 - 55, Add an unmount cleanup effect in the hook containing tick and clearTimers that clears all pending timers and sets runningRef.current to false. Ensure the cleanup runs only on unmount so the free-run loop cannot reschedule or call setActiveIndex after the component is removed.setting/operation_setting/lottery_setting.go-75-91 (1)
75-91: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
normalizeLotterySettingmutates the shared global on every read.
GetLotterySettingreturns a pointer to the package-globallotterySettingand normalizes it in place on each call. When legacyQuotais configured, the write tos.FreePrizes[i].Usdruns concurrently across per-request draw handlers, which the race detector will flag. Consider normalizing once at config load (or into a copy) rather than on every accessor call.🤖 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 `@setting/operation_setting/lottery_setting.go` around lines 75 - 91, Remove the in-place normalization from the read path in GetLotterySetting. Normalize lotterySetting once during configuration loading, or copy it before calling normalizeLotterySetting, so concurrent callers never mutate the shared global while preserving legacy Quota conversion.model/lottery_pick_test.go-14-14 (1)
14-14: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRestore
lotteryRandIntnafter each test override.These assignments mutate a package-level hook and leave it changed for the rest of the test process. Later tests that rely on random selection can become order-dependent; save the original function and restore it with
t.Cleanup.Also applies to: 18-18, 55-55
🤖 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 `@model/lottery_pick_test.go` at line 14, Update each test overriding the package-level lotteryRandIntn hook to save its original function before assignment and register t.Cleanup to restore it afterward. Apply this to all override sites, including the assignments near the referenced test lines, so later tests retain the default random-selection behavior.web/default/src/features/system-settings/extensions/custom-pages-section.tsx-303-305 (1)
303-305: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFragmented button label breaks i18n.
Splitting the label into
t('Delete (')+{selectedIds.length})produces a non-semantic translation key and prevents translators from reordering the count for other locales. Use a single interpolated key like the{{count}}pattern already used elsewhere in this file (Line 231).As per coding guidelines: i18n keys should be semantic and consistent, and all user-facing text must support i18n.
💚 Proposed fix
- <Trash2 className='mr-2 h-4 w-4' /> - {t('Delete (')} - {selectedIds.length}) + <Trash2 className='mr-2 h-4 w-4' /> + {t('Delete ({{count}})', { count: selectedIds.length })}🤖 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/extensions/custom-pages-section.tsx` around lines 303 - 305, Replace the fragmented Delete button text in the selected-items action with one semantic translation key using count interpolation, matching the existing {{count}} pattern in this component. Pass selectedIds.length as the count value so translators can reorder or format the number per locale.Source: Coding guidelines
setting/console_setting/availability_test.go-31-33 (1)
31-33: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse
require.Lenbefore indexingforAll[0].
assert.Lendoes not stop execution on failure, so ifGetCustomPagesForRole(false)returns an empty slice, line 33'sforAll[0]["id"]panics with index-out-of-range, masking the real assertion failure.custom_pages_test.goalready usesrequire.Lenfor this exact reason.As per coding guidelines: "Backend tests must use
testify/requirefor setup and fatal assertions andtestify/assertfor non-fatal checks."💚 Proposed fix
- forAll := GetCustomPagesForRole(false) - assert.Len(t, forAll, 1) - assert.Equal(t, "cp_all", forAll[0]["id"]) + forAll := GetCustomPagesForRole(false) + require.Len(t, forAll, 1) + assert.Equal(t, "cp_all", forAll[0]["id"])Add the import:
import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" )🤖 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 `@setting/console_setting/availability_test.go` around lines 31 - 33, Replace assert.Len with require.Len in the test before indexing forAll[0], and add the testify/require import if needed. Keep the existing assert.Equal check for the non-fatal value comparison.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d2a637f4-f6f1-4cfb-a36b-3cc8316f61a2
⛔ Files ignored due to path filters (1)
web/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (84)
.github/workflows/docker-ghcr-custom.yml.gitignorecontroller/extensions_availability.gocontroller/lottery.gocontroller/misc.gocontroller/option.gocontroller/token.gomiddleware/turnstile_strict.gomiddleware/turnstile_strict_test.gomodel/log.gomodel/log_availability_test.gomodel/lottery.gomodel/lottery_pick_test.gomodel/main.gomodel/pricing.gomodel/redemption.gomodel/redemption_test.gorouter/api-router.gosetting/console_setting/availability.gosetting/console_setting/availability_test.gosetting/console_setting/config.gosetting/console_setting/custom_pages_test.gosetting/console_setting/validation.gosetting/operation_setting/lottery_setting.gosetting/operation_setting/lottery_setting_test.goweb/default/package.jsonweb/default/src/components/layout/config/system-settings.config.tsweb/default/src/components/sign-out-dialog.tsxweb/default/src/components/turnstile.tsxweb/default/src/features/auth/hooks/use-auth-redirect.tsweb/default/src/features/auth/sign-up/components/sign-up-form.tsxweb/default/src/features/auth/types.tsweb/default/src/features/extensions/availability/api.tsweb/default/src/features/extensions/availability/components/availability-group-card.tsxweb/default/src/features/extensions/availability/components/heartbeat-bars.tsxweb/default/src/features/extensions/availability/hooks/use-availability.tsweb/default/src/features/extensions/availability/index.tsxweb/default/src/features/extensions/availability/lib/status.test.tsweb/default/src/features/extensions/availability/lib/status.tsweb/default/src/features/extensions/lottery/api.tsweb/default/src/features/extensions/lottery/components/lucky-prize-board.tsxweb/default/src/features/extensions/lottery/components/prize-result-dialog.tsxweb/default/src/features/extensions/lottery/hooks/use-marquee-spin.tsweb/default/src/features/extensions/lottery/index.tsxweb/default/src/features/extensions/lottery/lib/grid-layout.tsweb/default/src/features/extensions/lottery/lib/symbols.tsweb/default/src/features/extensions/lottery/types.tsweb/default/src/features/home/components/home-html-frame.tsxweb/default/src/features/home/components/sections/hero.tsxweb/default/src/features/home/index.tsxweb/default/src/features/keys/api.tsweb/default/src/features/keys/components/api-keys-dialogs.tsxweb/default/src/features/keys/components/api-keys-primary-buttons.tsxweb/default/src/features/keys/components/dialogs/connect-tool-dialog.tsxweb/default/src/features/keys/lib/connect-tool.tsweb/default/src/features/keys/types.tsweb/default/src/features/pricing/types.tsweb/default/src/features/redemption-codes/components/redemptions-mutate-drawer.tsxweb/default/src/features/redemption-codes/components/redemptions-table.tsxweb/default/src/features/system-settings/auth/bot-protection-section.tsxweb/default/src/features/system-settings/extensions/availability-monitor-section.tsxweb/default/src/features/system-settings/extensions/constants.tsweb/default/src/features/system-settings/extensions/custom-pages-section.tsxweb/default/src/features/system-settings/extensions/index.tsxweb/default/src/features/system-settings/extensions/lottery-settings-section.tsxweb/default/src/features/system-settings/extensions/section-registry.tsweb/default/src/features/system-settings/hooks/use-update-option.tsweb/default/src/features/system-settings/types.tsweb/default/src/hooks/use-sidebar-data.tsweb/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-TW.jsonweb/default/src/i18n/locales/zh.jsonweb/default/src/i18n/static-keys.tsweb/default/src/routeTree.gen.tsweb/default/src/routes/_authenticated/custom-pages/$pageId.tsxweb/default/src/routes/_authenticated/extensions/availability.tsxweb/default/src/routes/_authenticated/extensions/lottery.tsxweb/default/src/routes/_authenticated/system-settings/extensions/$section.tsxweb/default/src/routes/_authenticated/system-settings/extensions/index.tsxweb/package.json
| - id: meta | ||
| run: | | ||
| echo "image_name=$(echo '${{ github.repository }}' | tr '[:upper:]' '[:lower:]')" >> "$GITHUB_OUTPUT" | ||
| if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then | ||
| TAG="${{ github.event.inputs.image_tag }}" | ||
| else | ||
| TAG=${GITHUB_REF#refs/tags/} | ||
| fi | ||
| echo "tag=${TAG}" >> "$GITHUB_OUTPUT" | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Untrusted input interpolated directly into a shell script (script injection).
TAG="${{ github.event.inputs.image_tag }}" splices a workflow_dispatch string input directly into a run: block instead of passing it via env:. A crafted image_tag value containing shell metacharacters (e.g. `...` or $(...)) can execute arbitrary commands in the runner, which holds packages: write credentials. This tainted value then propagates through prepare's output and is reused via further ${{ }} interpolation in later run: steps (e.g. Lines 68, 72, 108-109, 124-125, 137-139, 144-146, 152-154), so hardening at the source doesn't automatically fix the downstream reuses — each run: step should consume the value via an env:-mapped shell variable.
🔒 Proposed fix at the source
steps:
- id: meta
+ env:
+ IMAGE_TAG_INPUT: ${{ github.event.inputs.image_tag }}
run: |
echo "image_name=$(echo '${{ github.repository }}' | tr '[:upper:]' '[:lower:]')" >> "$GITHUB_OUTPUT"
if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
- TAG="${{ github.event.inputs.image_tag }}"
+ TAG="$IMAGE_TAG_INPUT"
else
TAG=${GITHUB_REF#refs/tags/}
fi
echo "tag=${TAG}" >> "$GITHUB_OUTPUT"Apply the same env:-mapping pattern (e.g. env: TAG: ${{ needs.prepare.outputs.tag }} then reference $TAG) to the other run: steps that currently interpolate ${{ needs.prepare.outputs.tag }} / ${{ needs.prepare.outputs.image_name }} directly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - id: meta | |
| run: | | |
| echo "image_name=$(echo '${{ github.repository }}' | tr '[:upper:]' '[:lower:]')" >> "$GITHUB_OUTPUT" | |
| if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then | |
| TAG="${{ github.event.inputs.image_tag }}" | |
| else | |
| TAG=${GITHUB_REF#refs/tags/} | |
| fi | |
| echo "tag=${TAG}" >> "$GITHUB_OUTPUT" | |
| - id: meta | |
| env: | |
| IMAGE_TAG_INPUT: ${{ github.event.inputs.image_tag }} | |
| run: | | |
| echo "image_name=$(echo '${{ github.repository }}' | tr '[:upper:]' '[:lower:]')" >> "$GITHUB_OUTPUT" | |
| if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then | |
| TAG="$IMAGE_TAG_INPUT" | |
| else | |
| TAG=${GITHUB_REF#refs/tags/} | |
| fi | |
| echo "tag=${TAG}" >> "$GITHUB_OUTPUT" |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 33-33: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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 @.github/workflows/docker-ghcr-custom.yml around lines 29 - 38, Harden the
Docker workflow against shell injection by passing workflow_dispatch.image_tag
through an env-mapped variable before using it in the meta step, and validate or
safely handle the resulting TAG. Update every later run step that directly
interpolates needs.prepare.outputs.tag or needs.prepare.outputs.image_name to
map those outputs through env and reference shell variables instead, including
the affected publish, tagging, and signing steps; preserve the existing image
and tag behavior.
Source: Linters/SAST tools
| - name: Check out | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## workflow excerpt"
sed -n '1,120p' .github/workflows/docker-ghcr-custom.yml 2>/dev/null || true
echo
echo "## dockerignore files"
fd -a '.dockerignore$|Dockerfile$|compose.*' . | sed 's#^\./##' | sort | head -200
echo
echo "## .dockerignore contents (if any)"
while IFS= read -r f; do
echo "--- $f"
cat -n "$f" | sed -n '1,200p'
done < <(fd '.dockerignore$' .)
echo
echo "## repo .git directory status"
python3 - <<'PY'
from pathlib import Path
p = Path('.git')
print('exists=', p.exists() or p.is_symlink(), 'is_dir=', p.is_dir(), 'is_symlink=', p.is_symlink())
if p.exists():
print('contents=', sorted(str(x) for x in p.iterdir() if x.name not in {'config.example'})[:50])
PY
echo
echo "## checks for persist-credentials and build context usage"
rg -n "persist-credentials|docker/build-push-action|context:|\.dockerignore" .github/workflows/docker-ghcr-custom.yml .github/workflows || trueRepository: QuantumNous/new-api
Length of output: 3850
🌐 Web query:
actions/checkout v4 default persist-credentials true GITHUB_TOKEN .git/config
💡 Result:
In actions/checkout, the persist-credentials input controls whether the action configures the Git credential (such as the GITHUB_TOKEN or an SSH key) in the local Git configuration to enable authenticated Git operations [1][2]. As of version 6.0.0 and later, the behavior has been improved for security [3][4]: 1. Default Behavior: The input defaults to true, meaning the action automatically configures credentials [2][3]. 2. Storage Mechanism (v6.0.0+): Instead of writing credentials directly into the repository's .git/config file (which was the behavior in versions v5 and earlier), actions/checkout now stores them in a separate file located under $RUNNER_TEMP [3][4][5]. It uses Git's includeIf.gitdir mechanism to conditionally apply these credentials to the repository [4][5]. 3. Cleanup: The action includes a post-job step that removes the credential file to ensure it does not persist after the job finishes [1][3]. 4. Opt-Out: Users can set persist-credentials: false in their workflow step if they do not want the action to configure these Git credentials [1][6]. If you are using an older version (v5 or earlier), the credentials are written directly into the local .git/config [6][3]. In all versions, if persist-credentials is enabled, the token is available for the duration of the job to any process that can read the Git configuration [7].
Citations:
- 1: https://github.com/actions/checkout/blob/v4/README.md
- 2: https://github.com/actions/checkout/blob/v4/action.yml
- 3: https://github.com/actions/checkout/tree/v6.0.2?tab=readme-ov-file
- 4: actions/checkout@v5.0.1...v6.0.0
- 5: https://github.com/actions/checkout/pull/2286/files
- 6: https://github.com/actions/checkout
- 7: Remove
persist-credentialsor change the default tofalseactions/checkout#485
🌐 Web query:
Docker dockerignore .git excluded from build context documentation
💡 Result:
The.git directory is not automatically excluded from the build context by the.dockerignore file, though it is handled differently depending on how the build context is provided [1][2][3]. When building from a local directory, the.dockerignore file does not exclude the.git directory by default [2][4]. If you wish to exclude.git from your build context, you must explicitly add it to your.dockerignore file [3]. Using the pattern /.git ensures that all.git directories within your project, including those in subdirectories, are excluded from the build context [3]. When building directly from a remote Git repository URL, Docker BuildKit automatically excludes the.git directory by default [1]. You can override this behavior if necessary by using the --keep-git-dir=true flag [1]. In summary: - Local builds: You must explicitly add /.git to your.dockerignore file if you want to exclude it [3]. - Remote Git repository builds: The.git directory is excluded by default [1]. It is a common best practice to include.git in your.dockerignore file to reduce the size of the build context and prevent sensitive version control metadata from being included in your image [2][4].
Citations:
- 1: https://docs.docker.com/reference/dockerfile
- 2: https://docs.docker.com/build/concepts/context/
- 3: https://stackoverflow.com/questions/54793349/ignore-all-git-folders-in-dockerignore
- 4: https://github.com/docker/docker-ce/blob/master/components/cli/docs/reference/builder.md
Prevent actions/checkout credentials from entering the Docker build
actions/checkout@v4 persists the job credential by default, and this workflow builds with context: . without a .dockerignore that excludes .git. Set persist-credentials: false if the job does not need authenticated Git operations, then add /.git to .dockerignore to avoid leaking the checkout context/token into the image.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 61-64: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/docker-ghcr-custom.yml around lines 61 - 64, Update the
“Check out” actions/checkout@v4 step to set persist-credentials to false, and
add /.git to the repository’s .dockerignore so the Docker build context excludes
Git metadata and checkout credentials.
Source: Linters/SAST tools
| groups := make([]extensionsAvailabilityGroup, 0, len(groupNames)) | ||
| for _, groupName := range groupNames { | ||
| records, err := model.GetRecentGroupAvailabilityLogs(groupName, 100) | ||
| if err != nil { | ||
| common.ApiError(c, err) | ||
| return | ||
| } | ||
| okCount := 0 | ||
| successUseTimeSum := 0 | ||
| for _, record := range records { | ||
| if record.Ok { | ||
| okCount++ | ||
| successUseTimeSum += record.UseTime | ||
| } | ||
| } | ||
| successRate, avgUseTime, status := console_setting.SummarizeAvailabilityRecords( | ||
| okCount, | ||
| len(records), | ||
| successUseTimeSum, | ||
| ) | ||
| groups = append(groups, extensionsAvailabilityGroup{ | ||
| Group: groupName, | ||
| Records: records, | ||
| SuccessRate: successRate, | ||
| AvgUseTime: avgUseTime, | ||
| Status: status, | ||
| Total: len(records), | ||
| SuccessCount: okCount, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Per-request N+1 DB query pattern with no caching or rate limiting.
Each call fans out into one GetRecentGroupAvailabilityLogs query per billing group. The frontend polls this endpoint as often as every 5s (MIN_REFRESH_SECONDS), and the route has no rate-limit middleware (apiRouter.GET("/extensions/availability", middleware.UserAuth(), ...) — no CriticalRateLimit()/similar). With many groups and concurrent viewers, this multiplies DB load significantly and has no backpressure.
Since per-group log data is identical across users (only the exposed group set differs by permission), a short-TTL cache (e.g., keyed by group name, refreshed every few seconds) shared across requests would eliminate most of the duplicate work without touching per-user filtering logic.
🤖 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/extensions_availability.go` around lines 53 - 82, The extensions
availability handler currently issues one database query per group on every
request; add a short-TTL shared cache around GetRecentGroupAvailabilityLogs
keyed by group name, reusing cached records within the TTL and refreshing
expired or missing entries. Preserve the existing per-request group iteration,
permission-based group set, error handling, and availability summary
calculations in the handler.
| package middleware | ||
|
|
||
| import ( | ||
| "encoding/json" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Direct encoding/json use instead of the project's JSON wrapper.
Decoding the Cloudflare siteverify response is application-level deserialization, not a Gin response helper, so per the coding guideline it should go through common/json.go's wrappers rather than encoding/json directly.
As per coding guidelines: "All JSON marshal and unmarshal operations in business code must use the wrappers in common/json.go; do not directly call encoding/json operations."
Also applies to: 25-28
🤖 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 `@middleware/turnstile_strict.go` at line 4, Replace the direct encoding/json
dependency and its decode usage in the Cloudflare siteverify response flow with
the JSON marshal/unmarshal wrapper from common/json.go. Update the relevant code
near the response decoding block while preserving the existing deserialization
target and error handling.
Source: Coding guidelines
| func defaultTurnstileVerify(secret, response, remoteIP string) (bool, error) { | ||
| rawRes, err := http.PostForm("https://challenges.cloudflare.com/turnstile/v0/siteverify", url.Values{ | ||
| "secret": {secret}, | ||
| "response": {response}, | ||
| "remoteip": {remoteIP}, | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
No timeout on the Cloudflare siteverify call.
http.PostForm uses http.DefaultClient, which has no timeout. Since this middleware now runs on every login, registration, and lottery-draw request, a slow/unresponsive Cloudflare endpoint can hang request-serving goroutines indefinitely, risking resource exhaustion under load.
🔒 Proposed fix to add a bounded HTTP client
+var turnstileHTTPClient = &http.Client{Timeout: 5 * time.Second}
+
func defaultTurnstileVerify(secret, response, remoteIP string) (bool, error) {
- rawRes, err := http.PostForm("https://challenges.cloudflare.com/turnstile/v0/siteverify", url.Values{
+ rawRes, err := turnstileHTTPClient.PostForm("https://challenges.cloudflare.com/turnstile/v0/siteverify", url.Values{
"secret": {secret},
"response": {response},
"remoteip": {remoteIP},
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func defaultTurnstileVerify(secret, response, remoteIP string) (bool, error) { | |
| rawRes, err := http.PostForm("https://challenges.cloudflare.com/turnstile/v0/siteverify", url.Values{ | |
| "secret": {secret}, | |
| "response": {response}, | |
| "remoteip": {remoteIP}, | |
| }) | |
| var turnstileHTTPClient = &http.Client{Timeout: 5 * time.Second} | |
| func defaultTurnstileVerify(secret, response, remoteIP string) (bool, error) { | |
| rawRes, err := turnstileHTTPClient.PostForm("https://challenges.cloudflare.com/turnstile/v0/siteverify", url.Values{ | |
| "secret": {secret}, | |
| "response": {response}, | |
| "remoteip": {remoteIP}, | |
| }) |
🤖 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 `@middleware/turnstile_strict.go` around lines 15 - 20, Update
defaultTurnstileVerify to perform the Cloudflare siteverify request through an
HTTP client with an explicit bounded timeout instead of http.PostForm’s
http.DefaultClient. Preserve the existing form fields and response handling
while ensuring slow or unresponsive requests terminate within the configured
timeout.
| function loadTurnstileScript(): Promise<void> { | ||
| if (typeof window === 'undefined') { | ||
| return Promise.reject(new Error('no window')) | ||
| } | ||
| if (window.turnstile) { | ||
| return Promise.resolve() | ||
| } | ||
|
|
||
| const existing = document.getElementById(SCRIPT_ID) as HTMLScriptElement | null | ||
| if (existing) { | ||
| return new Promise((resolve, reject) => { | ||
| if (window.turnstile) { | ||
| resolve() | ||
| return | ||
| } | ||
| existing.addEventListener('load', () => resolve(), { once: true }) | ||
| existing.addEventListener( | ||
| 'error', | ||
| () => reject(new Error('turnstile script failed')), | ||
| { once: true } | ||
| ) | ||
| // Script may already be loaded but turnstile not yet attached | ||
| let tries = 0 | ||
| const timer = window.setInterval(() => { | ||
| tries += 1 | ||
| if (window.turnstile) { | ||
| window.clearInterval(timer) | ||
| resolve() | ||
| } else if (tries > 40) { | ||
| window.clearInterval(timer) | ||
| reject(new Error('turnstile script timeout')) | ||
| } | ||
| }, 50) | ||
| }) | ||
| } | ||
|
|
||
| return new Promise((resolve, reject) => { | ||
| const s = document.createElement('script') | ||
| s.id = SCRIPT_ID | ||
| s.src = SCRIPT_SRC | ||
| s.async = true | ||
| s.defer = true | ||
| s.onload = () => resolve() | ||
| s.onerror = () => reject(new Error('turnstile script failed')) | ||
| document.head.appendChild(s) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Failed Turnstile script load permanently breaks the widget for the session.
The reused-script path attaches one-time load/error listeners to the existing <script id="cf-turnstile"> tag. If that tag already fired error once (e.g. transient network failure), those listeners can never fire again, so every future mount attempt just polls for 2s and times out — Turnstile never actually retries loading the script until a full page reload.
🔁 Proposed fix: remove the failed tag so a retry can create a fresh one
const existing = document.getElementById(SCRIPT_ID) as HTMLScriptElement | null
if (existing) {
return new Promise((resolve, reject) => {
if (window.turnstile) {
resolve()
return
}
- existing.addEventListener('load', () => resolve(), { once: true })
- existing.addEventListener(
- 'error',
- () => reject(new Error('turnstile script failed')),
- { once: true }
- )
+ existing.addEventListener('load', () => resolve(), { once: true })
+ existing.addEventListener(
+ 'error',
+ () => {
+ existing.remove()
+ reject(new Error('turnstile script failed'))
+ },
+ { once: true }
+ )
// Script may already be loaded but turnstile not yet attached
let tries = 0
const timer = window.setInterval(() => {
tries += 1
if (window.turnstile) {
window.clearInterval(timer)
resolve()
} else if (tries > 40) {
window.clearInterval(timer)
+ existing.remove()
reject(new Error('turnstile script timeout'))
}
}, 50)
})
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function loadTurnstileScript(): Promise<void> { | |
| if (typeof window === 'undefined') { | |
| return Promise.reject(new Error('no window')) | |
| } | |
| if (window.turnstile) { | |
| return Promise.resolve() | |
| } | |
| const existing = document.getElementById(SCRIPT_ID) as HTMLScriptElement | null | |
| if (existing) { | |
| return new Promise((resolve, reject) => { | |
| if (window.turnstile) { | |
| resolve() | |
| return | |
| } | |
| existing.addEventListener('load', () => resolve(), { once: true }) | |
| existing.addEventListener( | |
| 'error', | |
| () => reject(new Error('turnstile script failed')), | |
| { once: true } | |
| ) | |
| // Script may already be loaded but turnstile not yet attached | |
| let tries = 0 | |
| const timer = window.setInterval(() => { | |
| tries += 1 | |
| if (window.turnstile) { | |
| window.clearInterval(timer) | |
| resolve() | |
| } else if (tries > 40) { | |
| window.clearInterval(timer) | |
| reject(new Error('turnstile script timeout')) | |
| } | |
| }, 50) | |
| }) | |
| } | |
| return new Promise((resolve, reject) => { | |
| const s = document.createElement('script') | |
| s.id = SCRIPT_ID | |
| s.src = SCRIPT_SRC | |
| s.async = true | |
| s.defer = true | |
| s.onload = () => resolve() | |
| s.onerror = () => reject(new Error('turnstile script failed')) | |
| document.head.appendChild(s) | |
| }) | |
| } | |
| function loadTurnstileScript(): Promise<void> { | |
| if (typeof window === 'undefined') { | |
| return Promise.reject(new Error('no window')) | |
| } | |
| if (window.turnstile) { | |
| return Promise.resolve() | |
| } | |
| const existing = document.getElementById(SCRIPT_ID) as HTMLScriptElement | null | |
| if (existing) { | |
| return new Promise((resolve, reject) => { | |
| if (window.turnstile) { | |
| resolve() | |
| return | |
| } | |
| existing.addEventListener('load', () => resolve(), { once: true }) | |
| existing.addEventListener( | |
| 'error', | |
| () => { | |
| existing.remove() | |
| reject(new Error('turnstile script failed')) | |
| }, | |
| { once: true } | |
| ) | |
| // Script may already be loaded but turnstile not yet attached | |
| let tries = 0 | |
| const timer = window.setInterval(() => { | |
| tries += 1 | |
| if (window.turnstile) { | |
| window.clearInterval(timer) | |
| resolve() | |
| } else if (tries > 40) { | |
| window.clearInterval(timer) | |
| existing.remove() | |
| reject(new Error('turnstile script timeout')) | |
| } | |
| }, 50) | |
| }) | |
| } | |
| return new Promise((resolve, reject) => { | |
| const s = document.createElement('script') | |
| s.id = SCRIPT_ID | |
| s.src = SCRIPT_SRC | |
| s.async = true | |
| s.defer = true | |
| s.onload = () => resolve() | |
| s.onerror = () => reject(new Error('turnstile script failed')) | |
| document.head.appendChild(s) | |
| }) | |
| } |
🤖 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/turnstile.tsx` around lines 49 - 95, Update
loadTurnstileScript’s existing-script error handling so a failed SCRIPT_ID
element is removed from the document before rejecting, allowing subsequent calls
to create and append a fresh script element. Preserve the current success and
polling behavior for scripts that are still loading or become available.
| import assert from 'node:assert/strict' | ||
| import { describe, test } from 'node:test' | ||
|
|
||
| import { availabilityStatusFromSuccessRate } from './status' | ||
|
|
||
| describe('availabilityStatusFromSuccessRate', () => { | ||
| test('maps thresholds', () => { | ||
| assert.equal(availabilityStatusFromSuccessRate(1, 0), 'ok') | ||
| assert.equal(availabilityStatusFromSuccessRate(0.95, 100), 'ok') | ||
| assert.equal(availabilityStatusFromSuccessRate(0.949, 100), 'warn') | ||
| assert.equal(availabilityStatusFromSuccessRate(0.8, 100), 'warn') | ||
| assert.equal(availabilityStatusFromSuccessRate(0.799, 100), 'error') | ||
| }) | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use Vitest instead of Node's built-in test runner.
This test imports describe/test from node:test and assert from node:assert/strict rather than using Vitest. As per coding guidelines, web/**/*.test.ts: "工具函数和纯逻辑优先使用 Vitest 单元测试，测试文件命名为 *.test.ts" — pure-logic utilities like availabilityStatusFromSuccessRate should be tested with Vitest. Mixing test runners risks this file not being correctly collected/reported by the project's Vitest pipeline.
♻️ Proposed fix to use Vitest
-import assert from 'node:assert/strict'
-import { describe, test } from 'node:test'
+import { describe, expect, test } from 'vitest'
import { availabilityStatusFromSuccessRate } from './status'
describe('availabilityStatusFromSuccessRate', () => {
test('maps thresholds', () => {
- assert.equal(availabilityStatusFromSuccessRate(1, 0), 'ok')
- assert.equal(availabilityStatusFromSuccessRate(0.95, 100), 'ok')
- assert.equal(availabilityStatusFromSuccessRate(0.949, 100), 'warn')
- assert.equal(availabilityStatusFromSuccessRate(0.8, 100), 'warn')
- assert.equal(availabilityStatusFromSuccessRate(0.799, 100), 'error')
+ expect(availabilityStatusFromSuccessRate(1, 0)).toBe('ok')
+ expect(availabilityStatusFromSuccessRate(0.95, 100)).toBe('ok')
+ expect(availabilityStatusFromSuccessRate(0.949, 100)).toBe('warn')
+ expect(availabilityStatusFromSuccessRate(0.8, 100)).toBe('warn')
+ expect(availabilityStatusFromSuccessRate(0.799, 100)).toBe('error')
})
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import assert from 'node:assert/strict' | |
| import { describe, test } from 'node:test' | |
| import { availabilityStatusFromSuccessRate } from './status' | |
| describe('availabilityStatusFromSuccessRate', () => { | |
| test('maps thresholds', () => { | |
| assert.equal(availabilityStatusFromSuccessRate(1, 0), 'ok') | |
| assert.equal(availabilityStatusFromSuccessRate(0.95, 100), 'ok') | |
| assert.equal(availabilityStatusFromSuccessRate(0.949, 100), 'warn') | |
| assert.equal(availabilityStatusFromSuccessRate(0.8, 100), 'warn') | |
| assert.equal(availabilityStatusFromSuccessRate(0.799, 100), 'error') | |
| }) | |
| }) | |
| import { describe, expect, test } from 'vitest' | |
| import { availabilityStatusFromSuccessRate } from './status' | |
| describe('availabilityStatusFromSuccessRate', () => { | |
| test('maps thresholds', () => { | |
| expect(availabilityStatusFromSuccessRate(1, 0)).toBe('ok') | |
| expect(availabilityStatusFromSuccessRate(0.95, 100)).toBe('ok') | |
| expect(availabilityStatusFromSuccessRate(0.949, 100)).toBe('warn') | |
| expect(availabilityStatusFromSuccessRate(0.8, 100)).toBe('warn') | |
| expect(availabilityStatusFromSuccessRate(0.799, 100)).toBe('error') | |
| }) | |
| }) |
🤖 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/extensions/availability/lib/status.test.ts` around
lines 19 - 32, Update the availabilityStatusFromSuccessRate test to use Vitest
imports and assertions instead of node:test and node:assert/strict. Preserve the
existing describe block and threshold cases while replacing the test runner and
assertion APIs with the project’s Vitest equivalents.
Source: Coding guidelines
| const handleSubmit = async () => { | ||
| if (!endpointConfig || !group || !model) return | ||
| setIsSubmitting(true) | ||
| setManualHint(false) | ||
| try { | ||
| const tokenName = buildConnectTokenName(endpointConfig.label, group) | ||
| const result = await createApiKey({ | ||
| name: tokenName, | ||
| remain_quota: 0, | ||
| expired_time: -1, | ||
| unlimited_quota: true, | ||
| model_limits_enabled: false, | ||
| model_limits: '', | ||
| allow_ips: '', | ||
| group, | ||
| cross_group_retry: false, | ||
| }) | ||
|
|
||
| if (!result.success || !result.data?.key) { | ||
| toast.error(result.message || t(ERROR_MESSAGES.CREATE_FAILED)) | ||
| return | ||
| } | ||
|
|
||
| const apiKey = result.data.key.startsWith('sk-') | ||
| ? result.data.key | ||
| : `sk-${result.data.key}` | ||
| setCreatedKey(apiKey) | ||
| triggerRefresh() | ||
|
|
||
| const launchUrl = | ||
| toolId === 'cc-switch' | ||
| ? buildCCSwitchImportURL({ | ||
| app: endpointConfig.ccSwitchApp, | ||
| name: tokenName, | ||
| model, | ||
| apiKey, | ||
| }) | ||
| : buildCherryStudioImportURL(apiKey) | ||
|
|
||
| window.open(launchUrl, '_blank') | ||
| toast.success(t('API key created. Opening the selected tool...')) | ||
| setManualHint(true) | ||
| } catch { | ||
| toast.error(t(ERROR_MESSAGES.UNEXPECTED)) | ||
| } finally { | ||
| setIsSubmitting(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files matching connect-tool-dialog.tsx:\n'
fd -a 'connect-tool-dialog\.tsx$' . || true
file="$(fd 'connect-tool-dialog\.tsx$' . | head -n 1 || true)"
if [ -n "${file:-}" ]; then
printf '\nFile: %s\n' "$file"
wc -l "$file"
printf '\nRelevant handleSubmit/build functions:\n'
sed -n '180,270p' "$file" | cat -n
sed -n '360,420p' "$file" | cat -n
fi
printf '\nFind createApiKey/build url definitions:\n'
rg -n "const createApiKey|function createApiKey|export .*createApiKey|buildCCSwitchImportURL|buildCherryStudioImportURL" web/default/src || true
printf '\nSearch for any existing sync window.open workarounds:\n'
rg -n "window\.open\('', '_blank'|window\.open\(" web/default/src || trueRepository: QuantumNous/new-api
Length of output: 9943
🌐 Web query:
MDN window.open user gesture call stack popup blocker after async await window.open
💡 Result:
Modern browsers enforce strict popup blocking policies that require window.open to be called in direct response to a user gesture, such as a click or keypress [1][2][3]. When you use async/await or other asynchronous patterns (like setTimeout or promises) between the user gesture and the window.open call, the browser may no longer consider the action to be directly initiated by the user, leading the popup blocker to intervene [4][5][6]. The most reliable, industry-standard workaround to bypass this restriction is to open a blank window synchronously within the initial event handler and update its location asynchronously once the required data is available [4][7][5]. Recommended Implementation: 1. Immediately invoke window.open('', '_blank') synchronously within the user-initiated event handler (e.g., inside the click listener) [7]. 2. Store the returned window reference [3][7]. 3. Perform your asynchronous operation (e.g., fetch, await) [7]. 4. Update the stored window's location once the operation completes [4][7]. Example: const newWindow = window.open('', '_blank'); // Open immediately during gesture [7] try { const data = await fetchData; // Perform async work [7] if (newWindow) { newWindow.location.href = data.url; // Redirect the already-opened window [4][7] } } catch (error) { newWindow?.close; // Clean up if the process fails [7] } This approach works because the initial call to window.open satisfies the browser's requirement for a synchronous gesture, while subsequent navigation of that window is generally permitted [7][5]. Note that if the browser blocks the initial synchronous open call, the returned reference will be null, and your code should handle this (e.g., by falling back to navigating the current tab or displaying a notification to the user) [1][3][7].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/Window/open
- 2: https://github.com/mdn/content/blob/main/files/en-us/web/api/window/open/index.md
- 3: https://developer.mozilla.org/en-US/docs/Web/API/Window_Management_API/Using
- 4: https://www.ryanthomson.net/articles/you-shouldnt-call-window-open-asynchronously/
- 5: https://dontpaniclabs.com/blog/post/2025/07/29/understanding-window-open-behavior-on-ios-safari/
- 6: https://powernerdinsights.com/2025/05/01/web-resource-scripts-chromium-pop-up-blocking/
- 7: fix(frontend): open customer portal in popup synchronously to bypass Safari blocker windmill-labs/windmill#9242
Open the deep-link window synchronously before awaiting createApiKey.
createApiKey() is awaited before window.open(launchUrl, '_blank'), so some browsers can treat the launch window as a popup and block it. Keep the same one-click flow by opening a placeholder window in the click handler, then assign window.location.href = launchUrl after the API succeeds; handle null when the placeholder open is blocked.
🤖 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/keys/components/dialogs/connect-tool-dialog.tsx`
around lines 208 - 255, Update handleSubmit to open a placeholder window
synchronously before awaiting createApiKey, retaining the returned window
reference even when window.open returns null. After successful key creation and
launchUrl construction, assign launchUrl to the placeholder’s location.href;
handle a blocked/null placeholder without throwing, while preserving existing
success, error, and cleanup behavior.
| export function filterModelsForGroup( | ||
| models: PricingModel[], | ||
| endpointId: EndpointTypeId, | ||
| group: string | ||
| ): PricingModel[] { | ||
| const endpoint = getEndpointTypeConfig(endpointId) | ||
| if (!endpoint) return [] | ||
|
|
||
| if (models.some((model) => model.enable_groups_by_endpoint)) { | ||
| return models.filter((model) => { | ||
| if (!modelMatchesProviderHeuristic(model, endpoint)) return false | ||
| const byEndpointGroups = groupsForModelEndpoint(model, endpoint) | ||
| if (byEndpointGroups === null) return false | ||
| return ( | ||
| byEndpointGroups.includes(group) || byEndpointGroups.includes('all') | ||
| ) | ||
| }) | ||
| } | ||
|
|
||
| return filterModelsForEndpoint(models, endpointId).filter((model) => { | ||
| const groups = model.enable_groups || [] | ||
| return groups.includes(group) || groups.includes('all') | ||
| }) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
filterModelsForGroup's endpoint-data check is unscoped, breaking legacy channels during partial rollout.
models.some((model) => model.enable_groups_by_endpoint) scans the entire models array, not just models for endpointId. If any model on any provider has been migrated to the new field, every model lacking it (including legacy channels of the current endpoint) gets groupsForModelEndpoint(...) === null → excluded from the list entirely. getGroupsForEndpoint avoids this exact trap by gating its "used by endpoint" flag on modelMatchesProviderHeuristic first (Line 171); this function should follow the same pattern.
🐛 Proposed fix to scope the endpoint-data check per matching model
export function filterModelsForGroup(
models: PricingModel[],
endpointId: EndpointTypeId,
group: string
): PricingModel[] {
const endpoint = getEndpointTypeConfig(endpointId)
if (!endpoint) return []
- if (models.some((model) => model.enable_groups_by_endpoint)) {
- return models.filter((model) => {
- if (!modelMatchesProviderHeuristic(model, endpoint)) return false
- const byEndpointGroups = groupsForModelEndpoint(model, endpoint)
- if (byEndpointGroups === null) return false
- return (
- byEndpointGroups.includes(group) || byEndpointGroups.includes('all')
- )
- })
- }
+ const matchingModels = models.filter((model) =>
+ modelMatchesProviderHeuristic(model, endpoint)
+ )
+ if (matchingModels.some((model) => groupsForModelEndpoint(model, endpoint) !== null)) {
+ return matchingModels.filter((model) => {
+ const byEndpointGroups = groupsForModelEndpoint(model, endpoint)
+ if (byEndpointGroups === null) return false
+ return (
+ byEndpointGroups.includes(group) || byEndpointGroups.includes('all')
+ )
+ })
+ }
return filterModelsForEndpoint(models, endpointId).filter((model) => {
const groups = model.enable_groups || []
return groups.includes(group) || groups.includes('all')
})
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function filterModelsForGroup( | |
| models: PricingModel[], | |
| endpointId: EndpointTypeId, | |
| group: string | |
| ): PricingModel[] { | |
| const endpoint = getEndpointTypeConfig(endpointId) | |
| if (!endpoint) return [] | |
| if (models.some((model) => model.enable_groups_by_endpoint)) { | |
| return models.filter((model) => { | |
| if (!modelMatchesProviderHeuristic(model, endpoint)) return false | |
| const byEndpointGroups = groupsForModelEndpoint(model, endpoint) | |
| if (byEndpointGroups === null) return false | |
| return ( | |
| byEndpointGroups.includes(group) || byEndpointGroups.includes('all') | |
| ) | |
| }) | |
| } | |
| return filterModelsForEndpoint(models, endpointId).filter((model) => { | |
| const groups = model.enable_groups || [] | |
| return groups.includes(group) || groups.includes('all') | |
| }) | |
| } | |
| export function filterModelsForGroup( | |
| models: PricingModel[], | |
| endpointId: EndpointTypeId, | |
| group: string | |
| ): PricingModel[] { | |
| const endpoint = getEndpointTypeConfig(endpointId) | |
| if (!endpoint) return [] | |
| const matchingModels = models.filter((model) => | |
| modelMatchesProviderHeuristic(model, endpoint) | |
| ) | |
| if ( | |
| matchingModels.some( | |
| (model) => groupsForModelEndpoint(model, endpoint) !== null | |
| ) | |
| ) { | |
| return matchingModels.filter((model) => { | |
| const byEndpointGroups = groupsForModelEndpoint(model, endpoint) | |
| if (byEndpointGroups === null) return false | |
| return ( | |
| byEndpointGroups.includes(group) || byEndpointGroups.includes('all') | |
| ) | |
| }) | |
| } | |
| return filterModelsForEndpoint(models, endpointId).filter((model) => { | |
| const groups = model.enable_groups || [] | |
| return groups.includes(group) || groups.includes('all') | |
| }) | |
| } |
🤖 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/keys/lib/connect-tool.ts` around lines 188 - 211,
Update filterModelsForGroup so the enable_groups_by_endpoint detection is scoped
to models matching the requested endpoint via
modelMatchesProviderHeuristic(model, endpoint), rather than scanning all models.
Preserve the existing filtering behavior, including excluding models with null
endpoint groups once the matching endpoint uses the new field, and retain the
legacy filter path otherwise.
1 participant
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit