fix: hide api key when provider is excluded from export#423
Conversation
测试结论
改动说明
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough重组自定义配置步骤的若干 UI 布局（错误冷却、排除导出），新增“模型映射”可增删条目，并在提供者编辑流程中基于 Changes
Sequence Diagram(s)（无） Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
诗篇
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
更新说明已按反馈调整：从导出与备份中排除 不再放在 错误冷冻 分组里，而是改成创建页上的同级独立区块显示。 本次改动
测试结论
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/providers/components/provider-edit-flow.tsx (1)
637-689:⚠️ Potential issue | 🟡 Minor隐藏 API Key 后应同步调整网格列数，避免空白列。
Line [637] 仍固定为
md:grid-cols-2。当 Line [662] 条件为 false 时，右侧会留下空列，编辑页出现明显留白。建议按shouldShowApiKeyField动态切换列数。可选修复示例
- <div className="grid grid-cols-1 md:grid-cols-2 gap-6"> + <div + className={`grid grid-cols-1 gap-6 ${ + shouldShowApiKeyField ? 'md:grid-cols-2' : '' + }`} + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/provider-edit-flow.tsx` around lines 637 - 689, The two-column grid is fixed to md:grid-cols-2 causing an empty column when shouldShowApiKeyField is false; update the container around the inputs (the div with className "grid grid-cols-1 md:grid-cols-2 gap-6") to compute the md column count from shouldShowApiKeyField (e.g. switch between md:grid-cols-2 and md:grid-cols-1 or set a cols variable) so the layout collapses to a single column when shouldShowApiKeyField is false; ensure you only change the className logic (not the Input components) and reference shouldShowApiKeyField in that computation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@web/src/pages/providers/components/provider-edit-flow.tsx`:
- Around line 637-689: The two-column grid is fixed to md:grid-cols-2 causing an
empty column when shouldShowApiKeyField is false; update the container around
the inputs (the div with className "grid grid-cols-1 md:grid-cols-2 gap-6") to
compute the md column count from shouldShowApiKeyField (e.g. switch between
md:grid-cols-2 and md:grid-cols-1 or set a cols variable) so the layout
collapses to a single column when shouldShowApiKeyField is false; ensure you
only change the className logic (not the Input components) and reference
shouldShowApiKeyField in that computation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1e80531f-86e2-42a9-b571-7a0439f876cb
📒 Files selected for processing (2)
web/src/pages/providers/components/custom-config-step.tsxweb/src/pages/providers/components/provider-edit-flow.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: playwright
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-11T07:29:12.071Z
Learning: Reviewer ymkiux (repo: awsl-project/maxx) requires strict attention to breaking changes in all PRs. Always thoroughly audit public API, interface, and behavioral contract changes — especially in sidebar/auth/layout components — before raising review comments. Do not flag superficial style issues; focus on correctness, regressions, and breaking changes.
🔇 Additional comments (2)
web/src/pages/providers/components/provider-edit-flow.tsx (1)
310-310:shouldShowApiKeyField守卫实现清晰且符合目标。这里用
!provider.excludeFromExport作为单一判断源，逻辑直观，且与“排除导出时隐藏 API Key”目标一致。web/src/pages/providers/components/custom-config-step.tsx (1)
228-242: 错误冷却与导出排除开关的拆分实现正确。两个开关现在是独立配置区块，语义更清楚，且
onCheckedChange仍分别写回对应字段，未引入提交流程回归。Also applies to: 244-259
|
@coderabbitai re-review ！Stop making breaking changes, do a proper review！ |
继续修复已按评论建议继续调整编辑页布局：当 本次改动
测试结论
@coderabbitai re-review ！Stop making breaking changes, do a proper review！ |
🧩 Analysis chain🏁 Script executed: Length of output: 42188
|
| Location | Severity | Issue |
|---|---|---|
provider-edit-flow.tsx |
🔴 Breaking | API key permanently uneditable for excluded providers; no user feedback |
provider-edit-flow.tsx |
🟡 Design | excludeFromExport flag conflated with edit permission |
custom-config-step.tsx |
✅ | excludeFromExport section separation is correct |
custom-config-step.tsx |
🟡 Minor | createModelMapping omits providerType, priority, isEnabled |
🧠 Learnings used
Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-11T07:29:12.071Z
Learning: Reviewer ymkiux (repo: awsl-project/maxx) requires strict attention to breaking changes in all PRs. Always thoroughly audit public API, interface, and behavioral contract changes — especially in sidebar/auth/layout components — before raising review comments. Do not flag superficial style issues; focus on correctness, regressions, and breaking changes.
✅ Actions performed
Full review triggered.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 42188
However, the 🔴 breaking change I raised previously is still present and there is actually a compounding issue now visible in the code: 🔴 Still Breaking: API key permanently inaccessible for excluded providersIssue 1 — No user explanation. When Issue 2 — excludeFromExport: !!provider.excludeFromExport, // ← always from the original prop
A minimal fix would be to add a readonly notice in place of the hidden field, e.g.: {shouldShowApiKeyField ? (
<div>
{/* existing API key input */}
</div>
) : (
<p className="text-xs text-muted-foreground p-3 border border-border rounded-lg bg-muted/50">
{t('provider.apiKeyHiddenForExcluded')}
</p>
)}Or alternatively, surface the 🟡 Still Present:
|
| Item | Status |
|---|---|
| Dynamic grid cols (layout fix) | ✅ |
excludeFromExport section separated in custom-config-step.tsx |
✅ |
| Existing API key preserved on save/clone | ✅ |
excludeFromExport switch binding in custom-config-step.tsx |
✅ |
🧠 Learnings used
Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-11T07:29:12.071Z
Learning: Reviewer ymkiux (repo: awsl-project/maxx) requires strict attention to breaking changes in all PRs. Always thoroughly audit public API, interface, and behavioral contract changes — especially in sidebar/auth/layout components — before raising review comments. Do not flag superficial style issues; focus on correctness, regressions, and breaking changes.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/src/pages/providers/components/custom-config-step.tsx (1)
244-259: UI 不一致：排除导出部分缺少内部标签与 Error Cooldown 部分相比，该部分的卡片内部缺少标签/标题。Error Cooldown 在卡片内有
text-sm font-medium的标签和text-xs的描述，而此处只有描述段落。♻️ 建议添加内部标签以保持一致性
<div className="flex items-center justify-between p-4 bg-card border border-border rounded-xl"> <div className="pr-4"> + <div className="text-sm font-medium text-foreground"> + {t('provider.excludeFromExport')} + </div> <p className="text-xs text-muted-foreground mt-1"> {t('provider.excludeFromExportDesc')} </p> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/custom-config-step.tsx` around lines 244 - 259, The card for the exclude-from-export toggle is missing the internal label/title that Error Cooldown uses; inside the card wrapping the Switch (the same div containing the description and Switch), add a label element (matching the style used for Error Cooldown, e.g., class "text-sm font-medium") that renders the title text via t('provider.excludeFromExport') or a shorter label, keep the existing description paragraph using t('provider.excludeFromExportDesc'), and ensure the Switch still binds to formData.excludeFromExport and updateFormData({ excludeFromExport: checked }) so the visual structure matches Error Cooldown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/pages/providers/components/provider-edit-flow.tsx`:
- Line 310: The change hides the API Key field when provider.excludeFromExport
is true, which unnecessarily prevents key rotation; update the rendering logic
in ProviderEditFlow (where shouldShowApiKeyField is computed) so that
excludeFromExport no longer fully hides the API Key input—instead either (A)
always render the API Key input and keep it editable while still excluding the
provider from export, or (B) render the API Key field as readOnly/disabled and
display a small explanatory hint when provider.excludeFromExport is true; adjust
the code that uses shouldShowApiKeyField (and any conditional JSX around the API
Key input) to implement one of these behaviors and ensure the UI shows a clear
message tied to provider.excludeFromExport.
---
Nitpick comments:
In `@web/src/pages/providers/components/custom-config-step.tsx`:
- Around line 244-259: The card for the exclude-from-export toggle is missing
the internal label/title that Error Cooldown uses; inside the card wrapping the
Switch (the same div containing the description and Switch), add a label element
(matching the style used for Error Cooldown, e.g., class "text-sm font-medium")
that renders the title text via t('provider.excludeFromExport') or a shorter
label, keep the existing description paragraph using
t('provider.excludeFromExportDesc'), and ensure the Switch still binds to
formData.excludeFromExport and updateFormData({ excludeFromExport: checked }) so
the visual structure matches Error Cooldown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 665b87f3-36e6-4ee5-aef8-8b6faf463061
📒 Files selected for processing (2)
web/src/pages/providers/components/custom-config-step.tsxweb/src/pages/providers/components/provider-edit-flow.tsx
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-11T07:29:12.071Z
Learning: Reviewer ymkiux (repo: awsl-project/maxx) requires strict attention to breaking changes in all PRs. Always thoroughly audit public API, interface, and behavioral contract changes — especially in sidebar/auth/layout components — before raising review comments. Do not flag superficial style issues; focus on correctness, regressions, and breaking changes.
🔇 Additional comments (2)
web/src/pages/providers/components/custom-config-step.tsx (1)
228-241: LGTM!Error Cooldown 部分的布局重构清晰合理，flex 布局使开关控件与文本的对齐更加一致。
web/src/pages/providers/components/provider-edit-flow.tsx (1)
637-639: 布局处理正确当 API Key 字段被隐藏时，网格布局从双列切换到单列，避免了空白区域。这是处理条件渲染时的正确布局策略。
|
@coderabbitai re-review ！Stop making breaking changes, do a proper review！ |
继续修复已按最新评论继续调整：编辑页在 本次改动
测试结论
@coderabbitai re-review ！Stop making breaking changes, do a proper review！ |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 43047
|
| Item | Status |
|---|---|
| API key silently removed | ✅ Fixed — informative notice shown |
en.json / zh.json i18n key |
✅ Both present |
| Two-column layout | ✅ Restored |
| Save/clone preserves API key | ✅ |
| Clone mapping fields complete | ✅ Fixed |
excludeFromExport not editable in edit form |
🟡 Residual UX limitation |
createModelMapping missing fields in custom-config-step.tsx |
🟡 Still present |
The 🔴 breaking change from the previous reviews has been resolved. The two remaining 🟡 items are lower severity — the createModelMapping one in particular is worth fixing to match the clone path behavior.
🧠 Learnings used
Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-11T07:29:12.071Z
Learning: Reviewer ymkiux (repo: awsl-project/maxx) requires strict attention to breaking changes in all PRs. Always thoroughly audit public API, interface, and behavioral contract changes — especially in sidebar/auth/layout components — before raising review comments. Do not flag superficial style issues; focus on correctness, regressions, and breaking changes.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 42711
|
| Item | Status |
|---|---|
| API key placeholder when hidden | ✅ Fixed |
| Layout (no empty column) | ✅ Correct |
| Save/clone preserves API key | ✅ Correct |
| Clone model mappings copy all fields | ✅ Correct |
excludeFromExport section in custom-config-step |
✅ Correct |
excludeFromExport toggle absent from edit form |
🟡 Design gap — intentional? |
createModelMapping omits isEnabled at creation |
🟡 Minor — depends on backend default |
🧠 Learnings used
Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-11T07:29:12.071Z
Learning: Reviewer ymkiux (repo: awsl-project/maxx) requires strict attention to breaking changes in all PRs. Always thoroughly audit public API, interface, and behavioral contract changes — especially in sidebar/auth/layout components — before raising review comments. Do not flag superficial style issues; focus on correctness, regressions, and breaking changes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/src/pages/providers/components/provider-edit-flow.tsx (1)
310-310:⚠️ Potential issue | 🟠 Major仍有行为回退：
excludeFromExport使 API Key 轮换能力不可用。现在只是补了提示文案，但仍然完全阻断了编辑 API Key。
excludeFromExport是导出/备份语义，不应同时变成“禁止密钥轮换”的权限开关；这会让已开启该选项的 Provider 在编辑流里无法完成常见运维动作（密钥轮换）。建议至少保留可编辑输入（仅在导出层排除密钥），或在编辑页提供可逆的excludeFromExport控制。Based on learnings: Reviewer ymkiux (repo: awsl-project/maxx) requires strict attention to breaking changes in all PRs. Always thoroughly audit public API, interface, and behavioral contract changes — especially in sidebar/auth/layout components — before raising review comments. Do not flag superficial style issues; focus on correctness, regressions, and breaking changes.
Also applies to: 662-700
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/provider-edit-flow.tsx` at line 310, The current boolean shouldShowApiKeyField (used in provider-edit-flow.tsx) hides the API Key input whenever provider.excludeFromExport is true, which incorrectly blocks key rotation; change the behavior so the API Key input remains editable regardless of excludeFromExport (i.e., do not gate visibility of the key field on excludeFromExport), and instead keep excludeFromExport only as a metadata flag used at export/backup time or surface it as a separate toggle (e.g., add an explicit reversible control bound to provider.excludeFromExport) so editing/rotating the API key (in the component handling the API key input and functions that persist key changes) is always possible even when excludeFromExport is set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/pages/providers/components/provider-edit-flow.tsx`:
- Around line 678-683: The API key show/hide button currently uses
tabIndex={-1}, which removes it from keyboard focus; remove the tabIndex prop
from the button element so it remains reachable by keyboard users and add an
appropriate aria-label (e.g., referencing showApiKey state) to the button in the
provider-edit-flow component so screen reader users understand its purpose and
state.
---
Duplicate comments:
In `@web/src/pages/providers/components/provider-edit-flow.tsx`:
- Line 310: The current boolean shouldShowApiKeyField (used in
provider-edit-flow.tsx) hides the API Key input whenever
provider.excludeFromExport is true, which incorrectly blocks key rotation;
change the behavior so the API Key input remains editable regardless of
excludeFromExport (i.e., do not gate visibility of the key field on
excludeFromExport), and instead keep excludeFromExport only as a metadata flag
used at export/backup time or surface it as a separate toggle (e.g., add an
explicit reversible control bound to provider.excludeFromExport) so
editing/rotating the API key (in the component handling the API key input and
functions that persist key changes) is always possible even when
excludeFromExport is set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 935331e1-d22c-40a6-a476-5c5465b479ed
📒 Files selected for processing (3)
web/src/locales/en.jsonweb/src/locales/zh.jsonweb/src/pages/providers/components/provider-edit-flow.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: playwright
- GitHub Check: e2e
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-11T07:29:12.071Z
Learning: Reviewer ymkiux (repo: awsl-project/maxx) requires strict attention to breaking changes in all PRs. Always thoroughly audit public API, interface, and behavioral contract changes — especially in sidebar/auth/layout components — before raising review comments. Do not flag superficial style issues; focus on correctness, regressions, and breaking changes.
📚 Learning: 2026-03-11T07:29:12.071Z
Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-11T07:29:12.071Z
Learning: Reviewer ymkiux (repo: awsl-project/maxx) requires strict attention to breaking changes in all PRs. Always thoroughly audit public API, interface, and behavioral contract changes — especially in sidebar/auth/layout components — before raising review comments. Do not flag superficial style issues; focus on correctness, regressions, and breaking changes.
Applied to files:
web/src/pages/providers/components/provider-edit-flow.tsx
🔇 Additional comments (2)
web/src/locales/en.json (1)
1199-1199: 文案新增与界面行为一致，认可此修改。该提示文案能明确解释字段隐藏原因，避免“无提示消失”的困惑。
web/src/locales/zh.json (1)
1199-1199: 中文文案与英文语义对齐，修改合理。提示信息清晰，能帮助用户理解为何当前不可编辑 API Key。
继续修复已按最新评论继续调整为更安全的行为： 本次改动
测试结论
@coderabbitai re-review ！Stop making breaking changes, do a proper review！ |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 146643
|
| Item | Status |
|---|---|
| API key always editable (breaking change resolved) | ✅ |
| Informational note below input for excluded providers | ✅ |
en.json / zh.json i18n keys correct |
✅ |
Accessibility (aria-label, no tabIndex={-1}) |
✅ |
| Layout (always two-column) | ✅ |
| Save/clone preserves API key | ✅ |
createModelMapping missing providerType, priority, isEnabled in custom-config-step.tsx |
🟡 Minor |
The 🔴 breaking change from earlier reviews is fully resolved. The design is now correct. Only the one 🟡 minor issue remains in custom-config-step.tsx.
🧠 Learnings used
Learnt from: ymkiux
Repo: awsl-project/maxx PR: 0
File: :0-0
Timestamp: 2026-03-11T07:29:12.071Z
Learning: Reviewer ymkiux (repo: awsl-project/maxx) requires strict attention to breaking changes in all PRs. Always thoroughly audit public API, interface, and behavioral contract changes — especially in sidebar/auth/layout components — before raising review comments. Do not flag superficial style issues; focus on correctness, regressions, and breaking changes.
2 participants
Summary
When a custom provider has 从导出与备份中排除 / Exclude from export enabled, editing that provider should not still display the editable API key field.
This change hides the API Key (leave empty to keep current) field in the custom provider edit flow when
excludeFromExportis enabled for the provider.Why
If a provider is explicitly marked to be excluded from export/backup, continuing to expose the editable API key field in the edit page weakens the intent of that setting and creates a confusing UX.
Changes
shouldShowApiKeyFieldguard inProviderEditFlowprovider.excludeFromExportistrueTesting
pnpm -C web exec tsc --noEmitSummary by CodeRabbit
发布说明
新功能
用户界面改进
文案/本地化