Feat/add workspace trust feature#4738
Open
MMhunter wants to merge 10 commits into
Open
Conversation
Contributor
Author
|
/next |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough本次PR引入工作区信任（受限模式）完整功能集，包含核心信任状态管理服务、扩展白名单过滤、多个UI贡献点（状态栏、设置面板）、命令处理以及依赖注入配置；同时对 Select 组件进行增强并更新多语言本地化字符串。 Changes工作区信任核心服务与UI集成
组件与基础设施增强
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/extension/src/browser/vscode/api/main.thread.language.ts (1)
263-307:⚠️ Potential issue | 🟠 Major | ⚡ Quick win代码变更与 PR 目标不一致。
PR 标题和描述表明这是要添加"工作区信任功能"和"受限模式"，但此文件中的更改是关于补全项范围（completion range）的处理逻辑重构，与工作区信任功能无关。
请确认：
- 此更改是否应该包含在此 PR 中？
- 工作区信任功能的实际实现代码在哪里？
- 是否应将此补全逻辑修复拆分到单独的 PR 中？
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/extension/src/browser/vscode/api/main.thread.language.ts` around lines 263 - 307, The changes to completion range handling in inflateSuggestDto (function inflateSuggestDto in main.thread.language.ts, and usage of ISuggestDataDtoField.range / MonacoRange.lift) appear unrelated to the PR description about workspace trust / restricted mode; confirm whether these edits belong in this PR—if not, remove the inflateSuggestDto/range refactor from this branch and put it into a separate focused PR, update the current PR description to only cover workspace trust changes, and if the completion-range fix is intended here, add a brief note in the PR explaining why it is needed and point reviewers to inflateSuggestDto and ISuggestDataDtoField.range so they can review the change in context.
🧹 Nitpick comments (4)
packages/extension/src/browser/vscode/api/main.thread.language.ts (1)
277-283: ⚡ Quick win建议添加数组长度验证。
在访问
dtoRange[0]到dtoRange[3]之前，应验证数组包含至少 4 个元素，以避免潜在的运行时错误。🛡️ 建议的防御性代码
if (Array.isArray(dtoRange)) { + if (dtoRange.length !== 4) { + // 记录错误或使用默认值 + targetRange = defaultRange; + } else { targetRange = MonacoRange.lift({ startLineNumber: dtoRange[0], startColumn: dtoRange[1], endLineNumber: dtoRange[2], endColumn: dtoRange[3], }) + } } else {🤖 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 `@packages/extension/src/browser/vscode/api/main.thread.language.ts` around lines 277 - 283, The code uses dtoRange[0]..dtoRange[3] without checking length; update the Array.isArray(dtoRange) branch to also verify dtoRange.length >= 4 before calling MonacoRange.lift and assigning targetRange (or handle the short-array case by logging/returning/null). Specifically, guard the MonacoRange.lift call that references dtoRange elements and ensure dtoRange has at least four elements (or fall back to a safe default) to prevent runtime errors when dtoRange is too short.packages/workspace-trust/src/browser/workspace-trust-setting-panel.module.less (1)
30-36: 💤 Low value按钮样式使用相同背景色
.buttonTrust和.buttonRestrict都使用了相同的var(--button-background)背景色。如果这两个按钮在 UI 中需要视觉上的区分（例如信任操作 vs 限制操作），建议考虑使用不同的 CSS 变量或颜色以提高用户体验。🤖 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 `@packages/workspace-trust/src/browser/workspace-trust-setting-panel.module.less` around lines 30 - 36, .buttonTrust and .buttonRestrict currently share the same background-color var(--button-background), so make them visually distinct by assigning different CSS variables or color values: update .buttonTrust to use a trust-specific variable (e.g., var(--button-trust-background)) and .buttonRestrict to use a restrict-specific variable (e.g., var(--button-restrict-background)), add those variables to the appropriate theme/variables file or :root, and ensure contrast/accessibility is preserved for both classes..claude/workspace-trust-design.md (1)
13-13: 💤 Low value建议为代码块添加语言标识符
Markdown 代码块缺少语言标识符，建议添加以提升文档可读性和语法高亮支持。例如：
- 第 13 行：可使用
```text或```mermaid（如果支持流程图）- 第 45 行：可使用
```text- 第 70 行：可使用
```text或```bash- 第 133 行：可使用
```text静态分析工具提示：markdownlint (MD040)
Also applies to: 45-45, 70-70, 133-133
🤖 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 @.claude/workspace-trust-design.md at line 13, The markdown file .claude/workspace-trust-design.md has several fenced code blocks without language identifiers (notably at lines 13, 45, 70, and 133); update each fence to include an appropriate language hint (e.g., use ```mermaid for diagrams, ```bash for shell snippets, or ```text for plain text) so markdownlint MD040 is satisfied and syntax highlighting improves; locate the code fences and change the opening backticks to include the chosen identifier (e.g., ```text, ```mermaid, or ```bash) for the corresponding blocks.packages/workspace-trust/src/browser/workspace-trust-command.contribution.ts (1)
41-41: ⚡ Quick win使用 MessageType 常量替代魔法数字
代码中使用了魔法数字
1并添加注释说明其表示MessageType.Info，但应该直接导入并使用该常量以提高代码可读性和可维护性。♻️ 建议的改进
在文件顶部添加导入：
import { MessageType } from '@opensumi/ide-overlay';然后修改代码：
- type: 1, // MessageType.Info + type: MessageType.Info,🤖 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 `@packages/workspace-trust/src/browser/workspace-trust-command.contribution.ts` at line 41, Replace the magic number 1 used for a message type with the MessageType enum: add an import for MessageType from '@opensumi/ide-overlay' at the top of workspace-trust-command.contribution.ts and change the object property setting type: 1 to type: MessageType.Info so the code uses the named constant instead of a magic number.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/components/src/select/index.tsx`:
- Around line 305-307: The component currently still runs internal filtering
when externalSearchBehavior is true; update the filtering logic (the function
that calls filterOption around lines 484-507) to short-circuit when
externalSearchBehavior is truthy: at the top of that function (referencing
externalSearchBehavior and filterOption), if externalSearchBehavior is true
simply return the unfiltered options (or the original options list) without
invoking filterOption or performing any test, and ensure any downstream code
expects that no filtered subset was produced; this keeps externalSearchBehavior
semantics consistent and avoids extra work.
In
`@packages/workspace-trust/src/browser/workspace-trust-command.contribution.ts`:
- Line 46: Replace the forced-cast string literal passed to
workspaceTrustService.setTrustState with the enum value: import
WorkspaceTrustState from the module that exports it (the ../common export) and
call this.workspaceTrustService.setTrustState(WorkspaceTrustState.Trusted)
instead of using 'trusted' as any; update the import list to include
WorkspaceTrustState so the call is type-safe.
In `@packages/workspace-trust/src/browser/workspace-trust-setting-panel.tsx`:
- Around line 18-20: Replace the unsafe 'as any' type assertions by importing
and using the WorkspaceTrustState enum: add an import for WorkspaceTrustState
from the common module at the top, then update the calls to
workspaceTrustService.setTrustState(...) in handleClick to pass
WorkspaceTrustState.Trusted and WorkspaceTrustState.Restricted (or the exact
enum member names) instead of the string literals with 'as any', ensuring full
type-safety when calling setTrustState.
- Around line 29-37: The default strings passed to the two localize(...) calls
for the variable description are in Chinese; replace those default texts with
English defaults (e.g., "Currently in restricted mode..."/"This workspace is
trusted...") while keeping the localization keys
workspace.trust.settings.restrictedDesc and workspace.trust.settings.trustedDesc
unchanged; then add the Chinese translations into the Chinese language pack
(zh-CN.lang.ts) under those same keys so Chinese users still see the translated
text.
- Around line 15-23: The handleClick callback currently awaits
workspaceTrustService.setTrustState without error handling, which can leave
loading=true and the UI stuck; wrap the async calls in a try-catch-finally
inside handleClick (references: handleClick, setLoading,
workspaceTrustService.setTrustState, clientApp.fireOnReload) so you catch and
log or surface errors in the catch and always call setLoading(false) in finally
(and still call clientApp.fireOnReload(true) when appropriate or rethrow after
logging).
In `@packages/workspace-trust/src/browser/workspace-trust.contribution.ts`:
- Line 30: Replace the string literal check that uses ('undecided' as any) with
the enum value: call this.workspaceTrustService.getTrustState() ===
WorkspaceTrustState.Undecided and ensure WorkspaceTrustState is imported from
../common at the top of the file; update the import list to include
WorkspaceTrustState so the type-safe enum is used instead of the forced string
cast.
In `@packages/workspace-trust/src/browser/workspace-trust.service.ts`:
- Around line 121-129: The i18n default strings passed to localize in
workspace-trust.service.ts are Chinese; replace the second-argument defaults
with English text for the keys used in the dialog (localize calls for
'workspace.trust.dialog.title', 'workspace.trust.dialog.message',
'workspace.trust.dialog.button.trust', and
'workspace.trust.dialog.button.restricted') so the fallback language is
English—update the dialog title, message (preserving the existing newline
formatting), and both button labels to appropriate English defaults.
- Around line 133-139: The dialog button compare uses localized text which can
differ; change the logic in the workspace trust dialog to compare the returned
button index (or a stable button id) instead of comparing with localize(...)
strings: when handling the result from dialogService.open, check the numeric
index for the "Trust" button and call await
this.setTrustState(WorkspaceTrustState.Trusted) / return
WorkspaceTrustState.Trusted for that index, otherwise set and return
WorkspaceTrustState.Restricted; ensure you reference the WorkspaceTrustState
enum values (WorkspaceTrustState.Trusted / WorkspaceTrustState.Restricted)
rather than string literals and update the button construction to make the
trusted button's index deterministic.
---
Outside diff comments:
In `@packages/extension/src/browser/vscode/api/main.thread.language.ts`:
- Around line 263-307: The changes to completion range handling in
inflateSuggestDto (function inflateSuggestDto in main.thread.language.ts, and
usage of ISuggestDataDtoField.range / MonacoRange.lift) appear unrelated to the
PR description about workspace trust / restricted mode; confirm whether these
edits belong in this PR—if not, remove the inflateSuggestDto/range refactor from
this branch and put it into a separate focused PR, update the current PR
description to only cover workspace trust changes, and if the completion-range
fix is intended here, add a brief note in the PR explaining why it is needed and
point reviewers to inflateSuggestDto and ISuggestDataDtoField.range so they can
review the change in context.
---
Nitpick comments:
In @.claude/workspace-trust-design.md:
- Line 13: The markdown file .claude/workspace-trust-design.md has several
fenced code blocks without language identifiers (notably at lines 13, 45, 70,
and 133); update each fence to include an appropriate language hint (e.g., use
```mermaid for diagrams, ```bash for shell snippets, or ```text for plain text)
so markdownlint MD040 is satisfied and syntax highlighting improves; locate the
code fences and change the opening backticks to include the chosen identifier
(e.g., ```text, ```mermaid, or ```bash) for the corresponding blocks.
In `@packages/extension/src/browser/vscode/api/main.thread.language.ts`:
- Around line 277-283: The code uses dtoRange[0]..dtoRange[3] without checking
length; update the Array.isArray(dtoRange) branch to also verify dtoRange.length
>= 4 before calling MonacoRange.lift and assigning targetRange (or handle the
short-array case by logging/returning/null). Specifically, guard the
MonacoRange.lift call that references dtoRange elements and ensure dtoRange has
at least four elements (or fall back to a safe default) to prevent runtime
errors when dtoRange is too short.
In
`@packages/workspace-trust/src/browser/workspace-trust-command.contribution.ts`:
- Line 41: Replace the magic number 1 used for a message type with the
MessageType enum: add an import for MessageType from '@opensumi/ide-overlay' at
the top of workspace-trust-command.contribution.ts and change the object
property setting type: 1 to type: MessageType.Info so the code uses the named
constant instead of a magic number.
In
`@packages/workspace-trust/src/browser/workspace-trust-setting-panel.module.less`:
- Around line 30-36: .buttonTrust and .buttonRestrict currently share the same
background-color var(--button-background), so make them visually distinct by
assigning different CSS variables or color values: update .buttonTrust to use a
trust-specific variable (e.g., var(--button-trust-background)) and
.buttonRestrict to use a restrict-specific variable (e.g.,
var(--button-restrict-background)), add those variables to the appropriate
theme/variables file or :root, and ensure contrast/accessibility is preserved
for both classes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b4729937-bcc9-42fc-9012-9b3b70dfefb2
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (24)
.claude/workspace-trust-design.mdconfigs/ts/references/tsconfig.workspace-trust.jsonconfigs/ts/tsconfig.build.jsonconfigs/ts/tsconfig.resolve.jsonpackages/components/src/select/index.tsxpackages/extension/package.jsonpackages/extension/src/browser/extension.service.tspackages/extension/src/browser/vscode/api/main.thread.language.tspackages/file-tree-next/src/browser/dialog/file-dialog.view.tsxpackages/i18n/src/common/en-US.lang.tspackages/i18n/src/common/zh-CN.lang.tspackages/preferences/src/browser/preference-settings.service.tspackages/startup/src/browser/common-modules.tspackages/workspace-trust/package.jsonpackages/workspace-trust/src/browser/index.tspackages/workspace-trust/src/browser/workspace-trust-command.contribution.tspackages/workspace-trust/src/browser/workspace-trust-setting-panel.module.lesspackages/workspace-trust/src/browser/workspace-trust-setting-panel.tsxpackages/workspace-trust/src/browser/workspace-trust-settings.contribution.tspackages/workspace-trust/src/browser/workspace-trust-statusbar.contribution.tspackages/workspace-trust/src/browser/workspace-trust.contribution.tspackages/workspace-trust/src/browser/workspace-trust.service.tspackages/workspace-trust/src/common/index.tspackages/workspace-trust/src/index.ts
Comment on lines
+305
to
307
| externalSearchBehavior = externalSearchBehavior ?? !!onSearchChange | ||
|
|
||
| const handleToggleOpen = useCallback( |
Contributor
There was a problem hiding this comment.
externalSearchBehavior 模式下仍执行内部过滤，和新语义不一致
Line 305 已启用“外部托管”语义，但 Line 484-507 仍会在输入时调用 filterOption。建议在 external 模式直接跳过内部过滤，避免契约偏差和额外开销。
建议修复
- const filteredOptions = useMemo(() => {
+ const filteredOptions = useMemo(() => {
+ if (externalSearchBehavior) {
+ return options;
+ }
if (!searchInput) {
return options;
}
if (isDataOptions(options)) {
return options.filter((o) => filterOption(searchInput, o));
}
@@
- }, [options, searchInput, filterOption]);
+ }, [options, searchInput, filterOption, externalSearchBehavior]);Also applies to: 484-507
🤖 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 `@packages/components/src/select/index.tsx` around lines 305 - 307, The
component currently still runs internal filtering when externalSearchBehavior is
true; update the filtering logic (the function that calls filterOption around
lines 484-507) to short-circuit when externalSearchBehavior is truthy: at the
top of that function (referencing externalSearchBehavior and filterOption), if
externalSearchBehavior is true simply return the unfiltered options (or the
original options list) without invoking filterOption or performing any test, and
ensure any downstream code expects that no filtered subset was produced; this
keeps externalSearchBehavior semantics consistent and avoids extra work.
| }); | ||
|
|
||
| if (msg === okText) { | ||
| await this.workspaceTrustService.setTrustState('trusted' as any); |
Contributor
There was a problem hiding this comment.
使用枚举值替代字符串字面量
这里使用字符串字面量 'trusted' 并通过 as any 强制转换，应该使用 WorkspaceTrustState.Trusted 枚举值以确保类型安全。
🔧 建议的修复
- await this.workspaceTrustService.setTrustState('trusted' as any);
+ await this.workspaceTrustService.setTrustState(WorkspaceTrustState.Trusted);需要从 ../common 导入 WorkspaceTrustState：
import { WorkspaceTrustState } from '../common';🤖 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 `@packages/workspace-trust/src/browser/workspace-trust-command.contribution.ts`
at line 46, Replace the forced-cast string literal passed to
workspaceTrustService.setTrustState with the enum value: import
WorkspaceTrustState from the module that exports it (the ../common export) and
call this.workspaceTrustService.setTrustState(WorkspaceTrustState.Trusted)
instead of using 'trusted' as any; update the import list to include
WorkspaceTrustState so the call is type-safe.
Comment on lines
+15
to
+23
| const handleClick = useCallback(async () => { | ||
| setLoading(true); | ||
| if (isRestricted) { | ||
| await workspaceTrustService.setTrustState('trusted' as any); | ||
| } else { | ||
| await workspaceTrustService.setTrustState('restricted' as any); | ||
| } | ||
| clientApp.fireOnReload(true); | ||
| }, [isRestricted, workspaceTrustService, clientApp]); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
考虑处理异步操作中的错误
handleClick 回调中的异步操作（setTrustState）没有错误处理。如果操作失败，加载状态会一直保持为 true，按钮将永久禁用。建议添加 try-catch-finally 块。
🛡️ 建议的改进
const handleClick = useCallback(async () => {
setLoading(true);
- if (isRestricted) {
- await workspaceTrustService.setTrustState('trusted' as any);
- } else {
- await workspaceTrustService.setTrustState('restricted' as any);
+ try {
+ if (isRestricted) {
+ await workspaceTrustService.setTrustState(WorkspaceTrustState.Trusted);
+ } else {
+ await workspaceTrustService.setTrustState(WorkspaceTrustState.Restricted);
+ }
+ clientApp.fireOnReload(true);
+ } catch (error) {
+ // 可以在此处显示错误消息
+ console.error('Failed to update workspace trust state:', error);
+ } finally {
+ setLoading(false);
}
- clientApp.fireOnReload(true);
}, [isRestricted, workspaceTrustService, clientApp]);🤖 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 `@packages/workspace-trust/src/browser/workspace-trust-setting-panel.tsx`
around lines 15 - 23, The handleClick callback currently awaits
workspaceTrustService.setTrustState without error handling, which can leave
loading=true and the UI stuck; wrap the async calls in a try-catch-finally
inside handleClick (references: handleClick, setLoading,
workspaceTrustService.setTrustState, clientApp.fireOnReload) so you catch and
log or surface errors in the catch and always call setLoading(false) in finally
(and still call clientApp.fireOnReload(true) when appropriate or rethrow after
logging).
Comment on lines
+18
to
+20
| await workspaceTrustService.setTrustState('trusted' as any); | ||
| } else { | ||
| await workspaceTrustService.setTrustState('restricted' as any); |
Contributor
There was a problem hiding this comment.
应使用 WorkspaceTrustState 枚举而非类型断言
第 18 行和第 20 行使用 'as any' 绕过了类型检查，应该从 common 模块导入并使用 WorkspaceTrustState 枚举值。这会提升类型安全性并防止潜在的运行时错误。
🔧 建议的修复
在文件顶部添加导入：
+import { WorkspaceTrustState } from '../common';然后更新 handleClick 函数：
const handleClick = useCallback(async () => {
setLoading(true);
if (isRestricted) {
- await workspaceTrustService.setTrustState('trusted' as any);
+ await workspaceTrustService.setTrustState(WorkspaceTrustState.Trusted);
} else {
- await workspaceTrustService.setTrustState('restricted' as any);
+ await workspaceTrustService.setTrustState(WorkspaceTrustState.Restricted);
}
clientApp.fireOnReload(true);
}, [isRestricted, workspaceTrustService, clientApp]);📝 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.
Suggested change
| await workspaceTrustService.setTrustState('trusted' as any); | |
| } else { | |
| await workspaceTrustService.setTrustState('restricted' as any); | |
| import { WorkspaceTrustState } from '../common'; | |
| // ... other imports and code ... | |
| const handleClick = useCallback(async () => { | |
| setLoading(true); | |
| if (isRestricted) { | |
| await workspaceTrustService.setTrustState(WorkspaceTrustState.Trusted); | |
| } else { | |
| await workspaceTrustService.setTrustState(WorkspaceTrustState.Restricted); | |
| } | |
| clientApp.fireOnReload(true); | |
| }, [isRestricted, workspaceTrustService, clientApp]); |
🤖 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 `@packages/workspace-trust/src/browser/workspace-trust-setting-panel.tsx`
around lines 18 - 20, Replace the unsafe 'as any' type assertions by importing
and using the WorkspaceTrustState enum: add an import for WorkspaceTrustState
from the common module at the top, then update the calls to
workspaceTrustService.setTrustState(...) in handleClick to pass
WorkspaceTrustState.Trusted and WorkspaceTrustState.Restricted (or the exact
enum member names) instead of the string literals with 'as any', ensuring full
type-safety when calling setTrustState.
Comment on lines
+29
to
+37
| const description = isRestricted | ||
| ? localize( | ||
| 'workspace.trust.settings.restrictedDesc', | ||
| '当前处于受限模式。当前 IDE 提供可以自动在此文件夹中执行文件的功能。点击以信任此工作区中文件的作者。', | ||
| ) | ||
| : localize( | ||
| 'workspace.trust.settings.trustedDesc', | ||
| '此工作区已被信任。点击以进入受限模式并禁用可能存在风险的功能。', | ||
| ); |
Contributor
There was a problem hiding this comment.
国际化文本应提供英文默认值
localize 调用的默认文本使用了中文，这会导致非中文用户看到中文描述。应该提供英文作为默认值，中文翻译应放在对应的语言包文件中。
🌐 建议的修复
const description = isRestricted
? localize(
'workspace.trust.settings.restrictedDesc',
- '当前处于受限模式。当前 IDE 提供可以自动在此文件夹中执行文件的功能。点击以信任此工作区中文件的作者。',
+ 'Currently in restricted mode. The IDE can automatically execute files in this folder. Click to trust the authors of files in this workspace.',
)
: localize(
'workspace.trust.settings.trustedDesc',
- '此工作区已被信任。点击以进入受限模式并禁用可能存在风险的功能。',
+ 'This workspace is trusted. Click to enter restricted mode and disable potentially risky features.',
);然后在对应的中文语言包文件 packages/i18n/src/common/zh-CN.lang.ts 中添加翻译条目。
🤖 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 `@packages/workspace-trust/src/browser/workspace-trust-setting-panel.tsx`
around lines 29 - 37, The default strings passed to the two localize(...) calls
for the variable description are in Chinese; replace those default texts with
English defaults (e.g., "Currently in restricted mode..."/"This workspace is
trusted...") while keeping the localization keys
workspace.trust.settings.restrictedDesc and workspace.trust.settings.trustedDesc
unchanged; then add the Chinese translations into the Chinese language pack
(zh-CN.lang.ts) under those same keys so Chinese users still see the translated
text.
| await this.workspaceTrustService.initialize(workspacePath); | ||
|
|
||
| // If no saved trust state, show dialog | ||
| if (this.workspaceTrustService.getTrustState() === ('undecided' as any)) { |
Contributor
There was a problem hiding this comment.
使用枚举值替代字符串字面量
这里使用了字符串字面量 'undecided' 并通过 as any 强制类型转换，绕过了类型检查。应该直接使用从 ../common 导入的 WorkspaceTrustState.Undecided 枚举值，以确保类型安全。
🔧 建议的修复
- if (this.workspaceTrustService.getTrustState() === ('undecided' as any)) {
+ if (this.workspaceTrustService.getTrustState() === WorkspaceTrustState.Undecided) {需要确保在文件顶部从 ../common 导入 WorkspaceTrustState：
import { WorkspaceTrustState } from '../common';🤖 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 `@packages/workspace-trust/src/browser/workspace-trust.contribution.ts` at line
30, Replace the string literal check that uses ('undecided' as any) with the
enum value: call this.workspaceTrustService.getTrustState() ===
WorkspaceTrustState.Undecided and ensure WorkspaceTrustState is imported from
../common at the top of the file; update the import list to include
WorkspaceTrustState so the type-safe enum is used instead of the forced string
cast.
Comment on lines
+121
to
+129
| message: `${localize('workspace.trust.dialog.title', '是否信任此文件夹中的文件的作者？')}\n\n${localize( | ||
| 'workspace.trust.dialog.message', | ||
| '当前 IDE 提供可以自动在此文件夹中执行文件的功能。\n\n如果不信任这些文件的作者，则建议继续使用受限模式，因为这些文件可能是恶意文件。', | ||
| )}`, | ||
| type: 1, // MessageType.Info | ||
| buttons: [ | ||
| localize('workspace.trust.dialog.button.trust', '是的，我信任他们'), | ||
| localize('workspace.trust.dialog.button.restricted', '受限模式'), | ||
| ], |
Contributor
There was a problem hiding this comment.
国际化字符串的默认值应使用英文
localize 函数的第二个参数（默认值）应该使用英文，而不是中文。这是国际化的最佳实践，确保在没有翻译时有一个通用的英文回退。
🌐 建议的修复
- message: `${localize('workspace.trust.dialog.title', '是否信任此文件夹中的文件的作者？')}\n\n${localize(
- 'workspace.trust.dialog.message',
- '当前 IDE 提供可以自动在此文件夹中执行文件的功能。\n\n如果不信任这些文件的作者，则建议继续使用受限模式，因为这些文件可能是恶意文件。',
+ message: `${localize('workspace.trust.dialog.title', 'Do you trust the authors of the files in this folder?')}\n\n${localize(
+ 'workspace.trust.dialog.message',
+ 'The IDE provides features that can automatically execute files in this folder.\n\nIf you don\'t trust the authors of these files, it is recommended to continue in Restricted Mode as these files may be malicious.',
)}`,
type: 1, // MessageType.Info
buttons: [
- localize('workspace.trust.dialog.button.trust', '是的，我信任他们'),
- localize('workspace.trust.dialog.button.restricted', '受限模式'),
+ localize('workspace.trust.dialog.button.trust', 'Yes, I trust the authors'),
+ localize('workspace.trust.dialog.button.restricted', 'Restricted Mode'),
],🤖 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 `@packages/workspace-trust/src/browser/workspace-trust.service.ts` around lines
121 - 129, The i18n default strings passed to localize in
workspace-trust.service.ts are Chinese; replace the second-argument defaults
with English text for the keys used in the dialog (localize calls for
'workspace.trust.dialog.title', 'workspace.trust.dialog.message',
'workspace.trust.dialog.button.trust', and
'workspace.trust.dialog.button.restricted') so the fallback language is
English—update the dialog title, message (preserving the existing newline
formatting), and both button labels to appropriate English defaults.
Comment on lines
+133
to
+139
| if (result === localize('workspace.trust.dialog.button.trust', 'Yes, I trust the authors')) { | ||
| await this.setTrustState(WorkspaceTrustState.Trusted); | ||
| return WorkspaceTrustState.Trusted; | ||
| } else { | ||
| await this.setTrustState(WorkspaceTrustState.Restricted); | ||
| return WorkspaceTrustState.Restricted; | ||
| } |
Contributor
There was a problem hiding this comment.
对话框按钮比较逻辑存在严重缺陷
当前代码将对话框返回的结果与本地化字符串进行比较，这会导致逻辑失败：
- Line 127 定义按钮文本为
localize('workspace.trust.dialog.button.trust', '是的，我信任他们') - Line 133 将结果与
localize('workspace.trust.dialog.button.trust', 'Yes, I trust the authors')比较 - 即使使用相同的 key，两次调用的默认值不同，且用户看到的是本地化后的文本
dialogService.open返回的是用户点击的按钮文本，而不是索引
这会导致按钮判断失效。正确的做法是比较按钮索引或者使用一致的按钮文本引用。
🐛 建议的修复方案
方案 1（推荐）：使用按钮索引
+ const trustButton = localize('workspace.trust.dialog.button.trust', 'Yes, I trust the authors');
+ const restrictedButton = localize('workspace.trust.dialog.button.restricted', 'Restricted Mode');
+
const result = await this.dialogService.open({
message: `${localize('workspace.trust.dialog.title', 'Do you trust the authors of the files in this folder?')}\n\n${localize(
'workspace.trust.dialog.message',
'The IDE provides features that can automatically execute files in this folder.\n\nIf you don\'t trust the authors of these files, it is recommended to continue in Restricted Mode as these files may be malicious.',
)}`,
type: 1, // MessageType.Info
- buttons: [
- localize('workspace.trust.dialog.button.trust', '是的，我信任他们'),
- localize('workspace.trust.dialog.button.restricted', '受限模式'),
- ],
+ buttons: [trustButton, restrictedButton],
closable: false,
});
- if (result === localize('workspace.trust.dialog.button.trust', 'Yes, I trust the authors')) {
+ if (result === trustButton) {
await this.setTrustState(WorkspaceTrustState.Trusted);
return WorkspaceTrustState.Trusted;
} else {
await this.setTrustState(WorkspaceTrustState.Restricted);
return WorkspaceTrustState.Restricted;
}另外，需要将字符串字面量 WorkspaceTrustState.Trusted 和 WorkspaceTrustState.Restricted 改为实际的枚举值引用。
🤖 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 `@packages/workspace-trust/src/browser/workspace-trust.service.ts` around lines
133 - 139, The dialog button compare uses localized text which can differ;
change the logic in the workspace trust dialog to compare the returned button
index (or a stable button id) instead of comparing with localize(...) strings:
when handling the result from dialogService.open, check the numeric index for
the "Trust" button and call await
this.setTrustState(WorkspaceTrustState.Trusted) / return
WorkspaceTrustState.Trusted for that index, otherwise set and return
WorkspaceTrustState.Restricted; ensure you reference the WorkspaceTrustState
enum values (WorkspaceTrustState.Trusted / WorkspaceTrustState.Restricted)
rather than string literals and update the button construction to make the
trusted button's index deterministic.
Contributor
Author
|
/next |
Contributor
Author
|
/next |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/core-browser/src/workspace-trust/workspace-trust-setting-panel.tsx (2)
19-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick win缺少错误处理导致 UI 状态可能卡死
handleClick回调中的异步操作没有错误处理。如果setTrustState失败，loading状态将永远保持为true，按钮会被永久禁用。🛡️ 建议的修复
const handleClick = useCallback(async () => { setLoading(true); - if (isRestricted) { - await workspaceTrustService.setTrustState(WorkspaceTrustState.Trusted); - } else { - await workspaceTrustService.setTrustState(WorkspaceTrustState.Restricted); + try { + if (isRestricted) { + await workspaceTrustService.setTrustState(WorkspaceTrustState.Trusted); + } else { + await workspaceTrustService.setTrustState(WorkspaceTrustState.Restricted); + } + clientApp.fireOnReload(true); + } catch (error) { + console.error('Failed to update workspace trust state:', error); + // 可选：显示错误提示给用户 + } finally { + setLoading(false); } - clientApp.fireOnReload(true); }, [isRestricted, workspaceTrustService, clientApp]);🤖 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 `@packages/core-browser/src/workspace-trust/workspace-trust-setting-panel.tsx` around lines 19 - 27, The handleClick async callback lacks error handling so a rejected workspaceTrustService.setTrustState call can leave loading true and the UI stuck; modify handleClick (the useCallback function) to wrap the await calls in try/catch/finally: in try call workspaceTrustService.setTrustState as currently done and then clientApp.fireOnReload(true) on success, in catch handle or surface the error (e.g., set an error state or log it), and in finally ensure setLoading(false) is always called so the button is re-enabled even when setTrustState fails.
33-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick win国际化文本应使用英文作为默认值
localize调用中使用了中文作为默认文本，违反了 i18n 最佳实践。🌐 建议的修复
const description = isRestricted ? localize( 'workspace.trust.settings.restrictedDesc', - '当前处于受限模式。当前 IDE 提供可以自动在此文件夹中执行文件的功能。点击以信任此工作区中文件的作者。', + 'Currently in restricted mode. The IDE can automatically execute files in this folder. Click to trust the authors of files in this workspace.', ) : localize( 'workspace.trust.settings.trustedDesc', - '此工作区已被信任。点击以进入受限模式并禁用可能存在风险的功能。', + 'This workspace is trusted. Click to enter restricted mode and disable potentially risky features.', );然后在
packages/i18n/src/common/zh-CN.lang.ts中添加对应的中文翻译。🤖 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 `@packages/core-browser/src/workspace-trust/workspace-trust-setting-panel.tsx` around lines 33 - 41, The description constant uses localize with Chinese default strings; replace the Chinese default texts in the two localize(...) calls (keys 'workspace.trust.settings.restrictedDesc' and 'workspace.trust.settings.trustedDesc') with English default messages and keep the same keys, then add the corresponding Chinese translations into the zh-CN language map (packages/i18n/src/common/zh-CN.lang.ts) so the UI still shows Chinese for that locale.
🧹 Nitpick comments (2)
packages/core-browser/src/workspace-trust/workspace-trust.contribution.ts (1)
34-34: ⚡ Quick win使用枚举值代替字符串字面量
第 34 行使用字符串字面量
'undecided'进行比较，应该使用WorkspaceTrustState.Undecided枚举值以获得更好的类型安全性和重构支持。♻️ 建议的修复
-if (this.workspaceTrustService.getTrustState() === 'undecided') { +if (this.workspaceTrustService.getTrustState() === WorkspaceTrustState.Undecided) { await this.workspaceTrustService.ensureTrustDecided(); }🤖 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 `@packages/core-browser/src/workspace-trust/workspace-trust.contribution.ts` at line 34, Replace the string literal comparison against 'undecided' with the enum value WorkspaceTrustState.Undecided: update the condition that calls this.workspaceTrustService.getTrustState() to compare with WorkspaceTrustState.Undecided, and add/import the WorkspaceTrustState enum where needed so the code uses the enum for type-safe comparison.packages/core-browser/src/workspace-trust/workspace-trust.service.ts (1)
36-46: 工作区信任服务的存储命名空间应与数据范围一致第 38 行使用
STORAGE_NAMESPACE.GLOBAL_EXTENSIONS存储工作区信任状态。虽然代码通过在存储键中包含workspacePath实现了按工作区隔离（第 52 行），但这与存储命名空间的语义不符：
GLOBAL_EXTENSIONS使用GLOBAL模式，语义上指代全局数据- 但实际存储的是每个工作区的独立信任状态，即工作区特定的数据
- 其他服务遵循清晰的模式：
EXTENSIONS（工作区作用域）和GLOBAL_LAYOUT（全局作用域）分别用于不同范围的数据建议改用
STORAGE_NAMESPACE.EXTENSIONS（工作区作用域）或创建专属的WORKSPACE_TRUST命名空间，以保持代码的一致性和可维护性。🤖 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 `@packages/core-browser/src/workspace-trust/workspace-trust.service.ts` around lines 36 - 46, The initialize method sets up trustStorage using STORAGE_NAMESPACE.GLOBAL_EXTENSIONS which is semantically wrong for per-workspace data; change the storage namespace passed to storageProvider in initialize (where trustStorage is assigned) from STORAGE_NAMESPACE.GLOBAL_EXTENSIONS to a workspace-scoped namespace (e.g., STORAGE_NAMESPACE.EXTENSIONS or a new STORAGE_NAMESPACE.WORKSPACE_TRUST) and update any related storage key usages in getTrustStateFromStorage and places that read/write trustStorage so the namespace consistently reflects workspace-scoped trust state; ensure the trustDecidedDeferred.resolve behavior remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/core-browser/src/workspace-trust/workspace-trust-statusbar.contribution.ts`:
- Around line 22-36: The async callback passed to runWhenIdle can produce an
unhandled Promise rejection from whenTrustDecided(); change the callback to
return void by calling this.workspaceTrustService.whenTrustDecided().then(...)
and handling errors with .catch(...), moving the
isRestricted()/statusBarService.addElement(...) logic into the .then() handler
and logging or reporting errors in the .catch(); keep references to runWhenIdle,
whenTrustDecided, isRestricted, statusBarService.addElement and
WORKSPACE_TRUST_EXIT_RESTRICTED_COMMAND.id so the change is localized and
ensures no unhandled Promise rejections.
In `@packages/core-browser/src/workspace-trust/workspace-trust.service.ts`:
- Around line 111-132: The default texts passed to localize inside
showTrustDialog are Chinese; change each localize call in
WorkspaceTrustService.showTrustDialog to use English default strings (e.g., "Do
you trust the authors of files in this folder?", "The IDE provides features that
can automatically execute files in this folder. If you do not trust these files'
authors, continue in Restricted Mode because files may be malicious.", "Yes, I
trust the authors", "Restricted Mode"), and move the Chinese translations into
the Chinese language pack resource instead of inlining them here; update the
localize call arguments only (keep the same keys) so the i18n system picks up
the English defaults and the external locale file supplies the Chinese strings.
In `@packages/extension/src/browser/allowed-extension.service.ts`:
- Around line 36-40: The aggregation of allowed extension IDs lacks error
isolation and validation: wrap the call to c.getAllowedExtensionIds() (inside
the loop over this.contributions.getContributions()) in a try/catch, validate
that the result is an array before iterating, and skip (and optionally log a
warning) when the call throws or returns a non-array so a single faulty
contribution does not break the allIds collection used by
AllowedExtensionService; ensure you still add valid ids to allIds.
In `@packages/overlay/src/browser/dialog.contribution.ts`:
- Around line 37-50: The inline execute argument types on
registry.registerCommand disagree with OpenDialogArgs: change the inline shape
to use the shared OpenDialogArgs (or at least set the type field to MessageType)
so execute: (args: OpenDialogArgs) => ... and ensure MessageType is
imported/used for the type property referenced in the call to
this.dialogService.open; update any optional properties (buttons, closable) to
match OpenDialogArgs to restore type safety for DIALOG_COMMANDS.OPEN.id and the
dialogService.open invocation.
---
Duplicate comments:
In `@packages/core-browser/src/workspace-trust/workspace-trust-setting-panel.tsx`:
- Around line 19-27: The handleClick async callback lacks error handling so a
rejected workspaceTrustService.setTrustState call can leave loading true and the
UI stuck; modify handleClick (the useCallback function) to wrap the await calls
in try/catch/finally: in try call workspaceTrustService.setTrustState as
currently done and then clientApp.fireOnReload(true) on success, in catch handle
or surface the error (e.g., set an error state or log it), and in finally ensure
setLoading(false) is always called so the button is re-enabled even when
setTrustState fails.
- Around line 33-41: The description constant uses localize with Chinese default
strings; replace the Chinese default texts in the two localize(...) calls (keys
'workspace.trust.settings.restrictedDesc' and
'workspace.trust.settings.trustedDesc') with English default messages and keep
the same keys, then add the corresponding Chinese translations into the zh-CN
language map (packages/i18n/src/common/zh-CN.lang.ts) so the UI still shows
Chinese for that locale.
---
Nitpick comments:
In `@packages/core-browser/src/workspace-trust/workspace-trust.contribution.ts`:
- Line 34: Replace the string literal comparison against 'undecided' with the
enum value WorkspaceTrustState.Undecided: update the condition that calls
this.workspaceTrustService.getTrustState() to compare with
WorkspaceTrustState.Undecided, and add/import the WorkspaceTrustState enum where
needed so the code uses the enum for type-safe comparison.
In `@packages/core-browser/src/workspace-trust/workspace-trust.service.ts`:
- Around line 36-46: The initialize method sets up trustStorage using
STORAGE_NAMESPACE.GLOBAL_EXTENSIONS which is semantically wrong for
per-workspace data; change the storage namespace passed to storageProvider in
initialize (where trustStorage is assigned) from
STORAGE_NAMESPACE.GLOBAL_EXTENSIONS to a workspace-scoped namespace (e.g.,
STORAGE_NAMESPACE.EXTENSIONS or a new STORAGE_NAMESPACE.WORKSPACE_TRUST) and
update any related storage key usages in getTrustStateFromStorage and places
that read/write trustStorage so the namespace consistently reflects
workspace-scoped trust state; ensure the trustDecidedDeferred.resolve behavior
remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c0816dd0-ab7c-4b4f-a6fd-52aea66f6c9e
📒 Files selected for processing (16)
packages/core-browser/src/bootstrap/inner-providers.tspackages/core-browser/src/common/common.command.tspackages/core-browser/src/index.tspackages/core-browser/src/workspace-trust/common.tspackages/core-browser/src/workspace-trust/index.tspackages/core-browser/src/workspace-trust/workspace-trust-command.contribution.tspackages/core-browser/src/workspace-trust/workspace-trust-setting-panel.module.lesspackages/core-browser/src/workspace-trust/workspace-trust-setting-panel.tsxpackages/core-browser/src/workspace-trust/workspace-trust-settings.contribution.tspackages/core-browser/src/workspace-trust/workspace-trust-statusbar.contribution.tspackages/core-browser/src/workspace-trust/workspace-trust.contribution.tspackages/core-browser/src/workspace-trust/workspace-trust.service.tspackages/extension/src/browser/allowed-extension.service.tspackages/extension/src/browser/extension.service.tspackages/extension/src/browser/index.tspackages/overlay/src/browser/dialog.contribution.ts
✅ Files skipped from review due to trivial changes (3)
- packages/core-browser/src/workspace-trust/workspace-trust-setting-panel.module.less
- packages/core-browser/src/workspace-trust/workspace-trust-settings.contribution.ts
- packages/core-browser/src/workspace-trust/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/extension/src/browser/extension.service.ts
Comment on lines
+22
to
+36
| runWhenIdle(async () => { | ||
| await this.workspaceTrustService.whenTrustDecided(); | ||
| if (this.workspaceTrustService.isRestricted()) { | ||
| this.statusBarService.addElement(RESTRICTED_MODE_STATUSBAR_ID, { | ||
| text: `$(shield) ${localize('workspace.trust.statusbar.restricted', 'Restricted Mode')}`, | ||
| alignment: StatusBarAlignment.RIGHT, | ||
| tooltip: localize( | ||
| 'workspace.trust.statusbar.restricted.tooltip', | ||
| 'Restricted Mode - Some features are disabled for security', | ||
| ), | ||
| className: 'workspace-trust-restricted-status', | ||
| command: WORKSPACE_TRUST_EXIT_RESTRICTED_COMMAND.id, | ||
| }); | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== 查找 runWhenIdle 定义 =="
rg -n -C2 "export function runWhenIdle|function runWhenIdle|declare function runWhenIdle" --type ts
echo
echo "== 查找 runWhenIdle 的 async 回调调用点 =="
rg -n -C2 "runWhenIdle\\s*\\(\\s*async\\s*\\(" --type tsRepository: opensumi/core
Length of output: 81
🏁 Script executed:
cat -n packages/core-browser/src/workspace-trust/workspace-trust-statusbar.contribution.tsRepository: opensumi/core
Length of output: 1906
🏁 Script executed:
find . -type f -name "*.ts" -path "*ide-core-common*" | head -20Repository: opensumi/core
Length of output: 39
🏁 Script executed:
rg -n "runWhenIdle" --type ts | head -30Repository: opensumi/core
Length of output: 3096
🏁 Script executed:
sed -n '381,425p' packages/utils/src/async.tsRepository: opensumi/core
Length of output: 1342
🏁 Script executed:
sed -n '245,260p' packages/workspace-edit/src/browser/workspace-edit.service.tsRepository: opensumi/core
Length of output: 729
🏁 Script executed:
sed -n '335,345p' packages/monaco/src/browser/contrib/merge-editor/merge-editor.service.tsRepository: opensumi/core
Length of output: 464
避免 runWhenIdle 异步回调中的未处理拒绝
第 22-36 行直接将 async 回调传给 runWhenIdle，违反了类型签名（期望 void 返回，而非 Promise）。若 whenTrustDecided() 拒绝或 async 函数抛出错误，产生的 Promise 拒绝将无处理。建议改为显式 Promise 链并兜底 catch。
建议修改
onStart() {
- runWhenIdle(async () => {
- await this.workspaceTrustService.whenTrustDecided();
- if (this.workspaceTrustService.isRestricted()) {
- this.statusBarService.addElement(RESTRICTED_MODE_STATUSBAR_ID, {
- text: `$(shield) ${localize('workspace.trust.statusbar.restricted', 'Restricted Mode')}`,
- alignment: StatusBarAlignment.RIGHT,
- tooltip: localize(
- 'workspace.trust.statusbar.restricted.tooltip',
- 'Restricted Mode - Some features are disabled for security',
- ),
- className: 'workspace-trust-restricted-status',
- command: WORKSPACE_TRUST_EXIT_RESTRICTED_COMMAND.id,
- });
- }
- });
+ runWhenIdle(() => {
+ void this.workspaceTrustService
+ .whenTrustDecided()
+ .then(() => {
+ if (this.workspaceTrustService.isRestricted()) {
+ this.statusBarService.addElement(RESTRICTED_MODE_STATUSBAR_ID, {
+ text: `$(shield) ${localize('workspace.trust.statusbar.restricted', 'Restricted Mode')}`,
+ alignment: StatusBarAlignment.RIGHT,
+ tooltip: localize(
+ 'workspace.trust.statusbar.restricted.tooltip',
+ 'Restricted Mode - Some features are disabled for security',
+ ),
+ className: 'workspace-trust-restricted-status',
+ command: WORKSPACE_TRUST_EXIT_RESTRICTED_COMMAND.id,
+ });
+ }
+ })
+ .catch(() => {
+ // 建议接入统一日志上报
+ });
+ });
}🤖 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
`@packages/core-browser/src/workspace-trust/workspace-trust-statusbar.contribution.ts`
around lines 22 - 36, The async callback passed to runWhenIdle can produce an
unhandled Promise rejection from whenTrustDecided(); change the callback to
return void by calling this.workspaceTrustService.whenTrustDecided().then(...)
and handling errors with .catch(...), moving the
isRestricted()/statusBarService.addElement(...) logic into the .then() handler
and logging or reporting errors in the .catch(); keep references to runWhenIdle,
whenTrustDecided, isRestricted, statusBarService.addElement and
WORKSPACE_TRUST_EXIT_RESTRICTED_COMMAND.id so the change is localized and
ensures no unhandled Promise rejections.
Comment on lines
+111
to
+132
| async showTrustDialog(): Promise<WorkspaceTrustState> { | ||
| const trustLabel = localize('workspace.trust.dialog.button.trust', 'Yes, I trust the authors'); | ||
| const restrictedLabel = localize('workspace.trust.dialog.button.restricted', 'Restricted Mode'); | ||
|
|
||
| const result = await this.commandService.executeCommand<string>('dialog.open', { | ||
| message: `${localize('workspace.trust.dialog.title', '是否信任此文件夹中的文件的作者？')}\n\n${localize( | ||
| 'workspace.trust.dialog.message', | ||
| '当前 IDE 提供可以自动在此文件夹中执行文件的功能。\n\n如果不信任这些文件的作者，则建议继续使用受限模式，因为这些文件可能是恶意文件。', | ||
| )}`, | ||
| type: MessageType.Info, | ||
| buttons: [trustLabel, restrictedLabel], | ||
| closable: false, | ||
| } as OpenDialogArgs); | ||
|
|
||
| if (result === trustLabel) { | ||
| await this.setTrustState(WorkspaceTrustState.Trusted); | ||
| return WorkspaceTrustState.Trusted; | ||
| } else { | ||
| await this.setTrustState(WorkspaceTrustState.Restricted); | ||
| return WorkspaceTrustState.Restricted; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
国际化文本应使用英文作为默认值
localize 函数调用中使用了中文作为默认文本（第 116-119 行）。这违反了国际化最佳实践，会导致非中文用户在缺少翻译时看到中文内容。
建议：
- 默认文本应使用英文
- 中文翻译应放在对应的语言包文件中
🌐 建议的修复
async showTrustDialog(): Promise<WorkspaceTrustState> {
const trustLabel = localize('workspace.trust.dialog.button.trust', 'Yes, I trust the authors');
const restrictedLabel = localize('workspace.trust.dialog.button.restricted', 'Restricted Mode');
const result = await this.commandService.executeCommand<string>('dialog.open', {
- message: `${localize('workspace.trust.dialog.title', '是否信任此文件夹中的文件的作者？')}\n\n${localize(
+ message: `${localize('workspace.trust.dialog.title', 'Do you trust the authors of the files in this folder?')}\n\n${localize(
'workspace.trust.dialog.message',
- '当前 IDE 提供可以自动在此文件夹中执行文件的功能。\n\n如果不信任这些文件的作者，则建议继续使用受限模式，因为这些文件可能是恶意文件。',
+ 'The IDE can automatically execute files in this folder.\n\nIf you don\'t trust the authors of these files, we recommend continuing in Restricted Mode as the files may be malicious.',
)}`,
type: MessageType.Info,
buttons: [trustLabel, restrictedLabel],
closable: false,
} as OpenDialogArgs);然后在中文语言包中添加对应的翻译。
📝 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.
Suggested change
| async showTrustDialog(): Promise<WorkspaceTrustState> { | |
| const trustLabel = localize('workspace.trust.dialog.button.trust', 'Yes, I trust the authors'); | |
| const restrictedLabel = localize('workspace.trust.dialog.button.restricted', 'Restricted Mode'); | |
| const result = await this.commandService.executeCommand<string>('dialog.open', { | |
| message: `${localize('workspace.trust.dialog.title', '是否信任此文件夹中的文件的作者？')}\n\n${localize( | |
| 'workspace.trust.dialog.message', | |
| '当前 IDE 提供可以自动在此文件夹中执行文件的功能。\n\n如果不信任这些文件的作者，则建议继续使用受限模式，因为这些文件可能是恶意文件。', | |
| )}`, | |
| type: MessageType.Info, | |
| buttons: [trustLabel, restrictedLabel], | |
| closable: false, | |
| } as OpenDialogArgs); | |
| if (result === trustLabel) { | |
| await this.setTrustState(WorkspaceTrustState.Trusted); | |
| return WorkspaceTrustState.Trusted; | |
| } else { | |
| await this.setTrustState(WorkspaceTrustState.Restricted); | |
| return WorkspaceTrustState.Restricted; | |
| } | |
| } | |
| async showTrustDialog(): Promise<WorkspaceTrustState> { | |
| const trustLabel = localize('workspace.trust.dialog.button.trust', 'Yes, I trust the authors'); | |
| const restrictedLabel = localize('workspace.trust.dialog.button.restricted', 'Restricted Mode'); | |
| const result = await this.commandService.executeCommand<string>('dialog.open', { | |
| message: `${localize('workspace.trust.dialog.title', 'Do you trust the authors of the files in this folder?')}\n\n${localize( | |
| 'workspace.trust.dialog.message', | |
| 'The IDE can automatically execute files in this folder.\n\nIf you don\'t trust the authors of these files, we recommend continuing in Restricted Mode as the files may be malicious.', | |
| )}`, | |
| type: MessageType.Info, | |
| buttons: [trustLabel, restrictedLabel], | |
| closable: false, | |
| } as OpenDialogArgs); | |
| if (result === trustLabel) { | |
| await this.setTrustState(WorkspaceTrustState.Trusted); | |
| return WorkspaceTrustState.Trusted; | |
| } else { | |
| await this.setTrustState(WorkspaceTrustState.Restricted); | |
| return WorkspaceTrustState.Restricted; | |
| } | |
| } |
🤖 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 `@packages/core-browser/src/workspace-trust/workspace-trust.service.ts` around
lines 111 - 132, The default texts passed to localize inside showTrustDialog are
Chinese; change each localize call in WorkspaceTrustService.showTrustDialog to
use English default strings (e.g., "Do you trust the authors of files in this
folder?", "The IDE provides features that can automatically execute files in
this folder. If you do not trust these files' authors, continue in Restricted
Mode because files may be malicious.", "Yes, I trust the authors", "Restricted
Mode"), and move the Chinese translations into the Chinese language pack
resource instead of inlining them here; update the localize call arguments only
(keep the same keys) so the i18n system picks up the English defaults and the
external locale file supplies the Chinese strings.
Comment on lines
+36
to
+40
| const contribs = this.contributions.getContributions(); | ||
| for (const c of contribs) { | ||
| for (const id of c.getAllowedExtensionIds()) { | ||
| allIds.add(id); | ||
| } |
Contributor
There was a problem hiding this comment.
为 contribution 聚合增加容错，避免单点失败影响全部扩展筛选
Line 36-40 当前对 c.getAllowedExtensionIds() 没有异常隔离和返回值校验；任一 contribution 抛错/返回非数组会中断整个 allowlist 计算，进而影响受限模式下扩展加载。
建议修改
getAllowedExtensionIds(): string[] {
const allIds = new Set(DEFAULT_ALLOWED_EXTENSION_IDS);
const contribs = this.contributions.getContributions();
for (const c of contribs) {
- for (const id of c.getAllowedExtensionIds()) {
- allIds.add(id);
- }
+ try {
+ const ids = c.getAllowedExtensionIds();
+ if (!Array.isArray(ids)) {
+ continue;
+ }
+ for (const id of ids) {
+ if (typeof id === 'string' && id.length > 0) {
+ allIds.add(id);
+ }
+ }
+ } catch {
+ // ignore invalid contribution and continue merging others
+ }
}
return Array.from(allIds);
}📝 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.
Suggested change
| const contribs = this.contributions.getContributions(); | |
| for (const c of contribs) { | |
| for (const id of c.getAllowedExtensionIds()) { | |
| allIds.add(id); | |
| } | |
| getAllowedExtensionIds(): string[] { | |
| const allIds = new Set(DEFAULT_ALLOWED_EXTENSION_IDS); | |
| const contribs = this.contributions.getContributions(); | |
| for (const c of contribs) { | |
| try { | |
| const ids = c.getAllowedExtensionIds(); | |
| if (!Array.isArray(ids)) { | |
| continue; | |
| } | |
| for (const id of ids) { | |
| if (typeof id === 'string' && id.length > 0) { | |
| allIds.add(id); | |
| } | |
| } | |
| } catch { | |
| // ignore invalid contribution and continue merging others | |
| } | |
| } | |
| return Array.from(allIds); | |
| } |
🤖 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 `@packages/extension/src/browser/allowed-extension.service.ts` around lines 36
- 40, The aggregation of allowed extension IDs lacks error isolation and
validation: wrap the call to c.getAllowedExtensionIds() (inside the loop over
this.contributions.getContributions()) in a try/catch, validate that the result
is an array before iterating, and skip (and optionally log a warning) when the
call throws or returns a non-array so a single faulty contribution does not
break the allIds collection used by AllowedExtensionService; ensure you still
add valid ids to allIds.
Comment on lines
+37
to
+50
| registry.registerCommand( | ||
| { | ||
| id: DIALOG_COMMANDS.OPEN.id, | ||
| }, | ||
| { | ||
| execute: (args: { message: string; type: number; buttons?: string[]; closable?: boolean }) => | ||
| this.dialogService.open<string>({ | ||
| message: args.message, | ||
| type: args.type, | ||
| buttons: args.buttons, | ||
| closable: args.closable, | ||
| }), | ||
| }, | ||
| ); |
Contributor
There was a problem hiding this comment.
类型定义不一致导致类型安全问题
第 42 行的内联类型注解将 type 定义为 number，但 OpenDialogArgs 接口中 type 的类型是 MessageType。这种不一致会削弱类型安全性，可能允许传入无效的消息类型值。
🔧 建议的修复
+import { OpenDialogArgs } from '@opensumi/ide-core-browser';
+
registerCommands(registry: CommandRegistry) {
// ... existing ENSURE command ...
registry.registerCommand(
{
id: DIALOG_COMMANDS.OPEN.id,
},
{
- execute: (args: { message: string; type: number; buttons?: string[]; closable?: boolean }) =>
+ execute: (args: OpenDialogArgs) =>
this.dialogService.open<string>({
message: args.message,
type: args.type,
buttons: args.buttons,
closable: args.closable,
}),
},
);🤖 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 `@packages/overlay/src/browser/dialog.contribution.ts` around lines 37 - 50,
The inline execute argument types on registry.registerCommand disagree with
OpenDialogArgs: change the inline shape to use the shared OpenDialogArgs (or at
least set the type field to MessageType) so execute: (args: OpenDialogArgs) =>
... and ensure MessageType is imported/used for the type property referenced in
the call to this.dialogService.open; update any optional properties (buttons,
closable) to match OpenDialogArgs to restore type safety for
DIALOG_COMMANDS.OPEN.id and the dialogService.open invocation.
Contributor
Author
|
/new |
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1778484729.0 |
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1778487605.0 |
Contributor
Author
|
/next |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core-browser/src/workspace-trust/workspace-trust.service.ts`:
- Around line 138-140: ensureTrustDecided currently awaits showTrustDialog but
if showTrustDialog throws (due to handler timeout or dialog.open failure)
trustDecidedDeferred is never resolved and whenTrustDecided callers hang; modify
ensureTrustDecided to catch errors from showTrustDialog (and any dialog-related
async failures) and explicitly mark the trust decision as failed-safe by setting
this.currentTrustState = WorkspaceTrustState.Restricted and resolving/rejecting
trustDecidedDeferred appropriately (e.g., resolve as decided with Restricted) so
that whenTrustDecided consumers are not left awaiting forever; reference
ensureTrustDecided, showTrustDialog, trustDecidedDeferred, whenTrustDecided, and
WorkspaceTrustState.Restricted when making the change.
In `@packages/core-common/src/command.ts`:
- Around line 541-563: The waitCommandHandlerRegistered logic is inconsistent:
it uses getActiveHandler() to short-circuit but the waiting emitter is only
fired on direct register paths, so handlers enabled later or activated via
delegation never wake waiters. Fix by making the activation paths and the
initial check consistent—either change the initial fast-path to test for the
same registration condition that triggers the emitter (e.g., check the raw
registration map used when firing the emitter) or, preferably, ensure all code
paths that make a handler "active" (registerHandler, any enable/disable toggles
like setHandlerEnabled, and delegate-related methods) call the emitter for that
commandId (this._handlerWaitingEmitters.get(commandId)?.fire()). Update
waitCommandHandlerRegistered/getActiveHandler or the register/enable/delegate
functions so they use the same activation semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: da508cb5-9370-48ae-840c-0c89fcb9990f
📒 Files selected for processing (2)
packages/core-browser/src/workspace-trust/workspace-trust.service.tspackages/core-common/src/command.ts
Comment on lines
+138
to
+140
| async ensureTrustDecided(): Promise<void> { | ||
| if (this.currentTrustState === WorkspaceTrustState.Undecided) { | ||
| await this.showTrustDialog(); |
Contributor
There was a problem hiding this comment.
信任决策失败时需要显式降级，而不是一直挂起。
showTrustDialog() 一旦因为 handler 超时或 dialog.open 执行失败而抛错，trustDecidedDeferred 就永远不会完成，所有 whenTrustDecided() 调用方都会卡住。这里应至少 fail closed 到 Restricted，或者显式 reject 这条决策链。
建议修复
async ensureTrustDecided(): Promise<void> {
if (this.currentTrustState === WorkspaceTrustState.Undecided) {
- await this.showTrustDialog();
+ try {
+ await this.showTrustDialog();
+ } catch (error) {
+ await this.setTrustState(WorkspaceTrustState.Restricted);
+ }
}
}📝 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.
Suggested change
| async ensureTrustDecided(): Promise<void> { | |
| if (this.currentTrustState === WorkspaceTrustState.Undecided) { | |
| await this.showTrustDialog(); | |
| async ensureTrustDecided(): Promise<void> { | |
| if (this.currentTrustState === WorkspaceTrustState.Undecided) { | |
| try { | |
| await this.showTrustDialog(); | |
| } catch (error) { | |
| await this.setTrustState(WorkspaceTrustState.Restricted); | |
| } | |
| } | |
| } |
🤖 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 `@packages/core-browser/src/workspace-trust/workspace-trust.service.ts` around
lines 138 - 140, ensureTrustDecided currently awaits showTrustDialog but if
showTrustDialog throws (due to handler timeout or dialog.open failure)
trustDecidedDeferred is never resolved and whenTrustDecided callers hang; modify
ensureTrustDecided to catch errors from showTrustDialog (and any dialog-related
async failures) and explicitly mark the trust decision as failed-safe by setting
this.currentTrustState = WorkspaceTrustState.Restricted and resolving/rejecting
trustDecidedDeferred appropriately (e.g., resolve as decided with Restricted) so
that whenTrustDecided consumers are not left awaiting forever; reference
ensureTrustDecided, showTrustDialog, trustDecidedDeferred, whenTrustDecided, and
WorkspaceTrustState.Restricted when making the change.
Comment on lines
+541
to
+563
| waitCommandHandlerRegistered(commandId: string, timeoutMs = 30000): Promise<void> { | ||
| // Resolve immediately if a handler already exists | ||
| if (this.getActiveHandler(commandId)) { | ||
| return Promise.resolve(); | ||
| } | ||
|
|
||
| return new Promise<void>((resolve, reject) => { | ||
| let waitingEmitter = this._handlerWaitingEmitters.get(commandId); | ||
| if (!waitingEmitter) { | ||
| waitingEmitter = new Emitter<void>(); | ||
| this._handlerWaitingEmitters.set(commandId, waitingEmitter); | ||
| } | ||
|
|
||
| const timer = setTimeout(() => { | ||
| listener.dispose(); | ||
| reject(new Error(`Timeout waiting for command handler registration: ${commandId}`)); | ||
| }, timeoutMs); | ||
|
|
||
| const listener = waitingEmitter.event(() => { | ||
| clearTimeout(timer); | ||
| resolve(); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
waitCommandHandlerRegistered 的判定条件前后不一致。
这里用 getActiveHandler() 判断“已注册”，但真正唤醒等待者只发生在 registerHandler(commandId, ...) 的直接注册路径上。带 isEnabled 的 handler，或经由 delegate 才可执行的命令，都会出现误判并导致不必要的超时。
可考虑的最小修复
waitCommandHandlerRegistered(commandId: string, timeoutMs = 30000): Promise<void> {
- // Resolve immediately if a handler already exists
- if (this.getActiveHandler(commandId)) {
+ // Resolve immediately if a handler has been directly registered
+ if ((this._handlers[commandId]?.length ?? 0) > 0) {
return Promise.resolve();
}🤖 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 `@packages/core-common/src/command.ts` around lines 541 - 563, The
waitCommandHandlerRegistered logic is inconsistent: it uses getActiveHandler()
to short-circuit but the waiting emitter is only fired on direct register paths,
so handlers enabled later or activated via delegation never wake waiters. Fix by
making the activation paths and the initial check consistent—either change the
initial fast-path to test for the same registration condition that triggers the
emitter (e.g., check the raw registration map used when firing the emitter) or,
preferably, ensure all code paths that make a handler "active" (registerHandler,
any enable/disable toggles like setHandlerEnabled, and delegate-related methods)
call the emitter for that commandId
(this._handlerWaitingEmitters.get(commandId)?.fire()). Update
waitCommandHandlerRegistered/getActiveHandler or the register/enable/delegate
functions so they use the same activation semantics.
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1778742934.0 |
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1778757454.0 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/extension/src/browser/extension.service.ts (1)
330-357:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
initExtensionInstanceData在子集初始化时会过早派发「全集就绪」事件Lines 350-356 在方法尾部无条件派发
ExtensionsInitializedEvent、DebugConfigurationsReadyEvent，并对 node/worker/view 进程做updateExtensionData/initExtension。当activate()先用 allowed 子集调用一次、再用 pending 过滤集调用一次时，这两个事件以及updateExtensionData都会被触发两次——第一次携带的是「部分初始化」的扩展实例集合。DebugConfigurationsReadyEvent等订阅方通常按「一次性就绪」语义实现，可能在拿到部分集合时即冻结调试配置等下游状态。建议仅在「最终扩展集合确定」之后才派发这些事件，例如增加一个
final参数或者将事件派发从initExtensionInstanceData抽出到activate()中只调用一次。♻️ 建议方案
- private async initExtensionInstanceData(extensions?: IExtensionMetaData[]) { + private async initExtensionInstanceData(extensions?: IExtensionMetaData[], final = true) { const metaDataArr = extensions ?? this.extensionMetaDataArr; for (const extensionMetaData of metaDataArr) { ... } - this.eventBus.fire(new ExtensionsInitializedEvent(this.extensionInstanceManageService.getExtensionInstances())); - this.eventBus.fire(new DebugConfigurationsReadyEvent(undefined)); - const extensionInstanceList = this.extensionInstanceManageService.getExtensionInstances(); this.nodeExtensionService.updateExtensionData(extensionInstanceList); this.workerExtensionService.updateExtensionData(extensionInstanceList); this.viewExtensionService.initExtension(extensionInstanceList); + + if (final) { + this.eventBus.fire(new ExtensionsInitializedEvent(extensionInstanceList)); + this.eventBus.fire(new DebugConfigurationsReadyEvent(undefined)); + } }并在
activate()中第一次（子集）调用传final=false，第二次（完整集）保持默认。🤖 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 `@packages/extension/src/browser/extension.service.ts` around lines 330 - 357, initExtensionInstanceData currently always fires ExtensionsInitializedEvent and DebugConfigurationsReadyEvent and calls node/worker/view update/init, which causes duplicate/partial-ready notifications when activate() is called twice for subsets; change initExtensionInstanceData to accept a boolean parameter (e.g., final = true) and only fire ExtensionsInitializedEvent/DebugConfigurationsReadyEvent and call nodeExtensionService.updateExtensionData, workerExtensionService.updateExtensionData, and viewExtensionService.initExtension when final is true; update callers (activate) to pass final=false for the intermediate subset call and only call initExtensionInstanceData() with final=true (or dispatch the events) once the final/full extension set is determined.
🧹 Nitpick comments (1)
packages/extension/src/browser/extension.service.ts (1)
246-248: ⚡ Quick win非受限工作区下也走两阶段初始化，存在不必要的延迟
无论工作区是否处于受限状态，这里都先将扩展拆分为 allowed/pending 两组，导致非受限场景下 pending 扩展仍要等到
waitTrustDecided()完成后才初始化。即使该 promise 立即 resolve，也至少多了一次微任务跳转及上面提到的两次runEagerExtensionsContributes/事件派发开销。建议在拆分前先短路：若workspaceTrustService.isRestricted()为 false，则按原先单次初始化路径处理。🤖 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 `@packages/extension/src/browser/extension.service.ts` around lines 246 - 248, The code always splits extensions into allowed/pending (using allowedExtensionService.getAllowedExtensionIds and extensionMetaDataArr) causing unnecessary two-phase init and delay even for non-restricted workspaces; modify the initialization path to short-circuit by checking workspaceTrustService.isRestricted() first and, if false, skip splitting and initialize all extensions in the original single-phase flow (so pendingExtensions and waitTrustDecided() are not awaited and runEagerExtensionsContributes/events are only invoked once); keep the restricted path unchanged so that when isRestricted() is true you still compute allowedIds, allowedExtensions, pendingExtensions and await waitTrustDecided() before initializing pending extensions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/extension/src/browser/extension.service.ts`:
- Around line 252-263: The code calls runEagerExtensionsContributes() twice
which re-registers the same pre-execute hook
(commandRegistry.beforeExecuteCommand) and re-dispatches
ExtensionDidContributes; split runEagerExtensionsContributes into two parts: one
function (e.g. prepareEagerContributes) that only computes/initializes
contributes data used by extensions, and a second one (e.g.
finalizeEagerContributes) that registers the one-time pre-execute hook via
commandRegistry.beforeExecuteCommand and emits the ExtensionDidContributes
event; then change activate() flow to call prepareEagerContributes() before
waiting on allowedExtensionService.waitTrustDecided() and
initExtensionInstanceData(pending), and call finalizeEagerContributes() only
once after the trust decision and instance initialization so hooks and events
are registered/dispatched a single time.
- Around line 254-266: The await on
this.allowedExtensionService.waitTrustDecided() can hang indefinitely; update
ExtensionServiceImpl.activate() to proactively trigger trust resolution by
calling WorkspaceTrustContribution.ensureTrustDecided() (or
this.allowedExtensionService.ensureTrustDecided() if exposed) before awaiting
waitTrustDecided(), and/or wrap waitTrustDecided() with a timeout fallback
(e.g., 30s) that treats unresolved state as restricted so
initExtensionInstanceData, runEagerExtensionsContributes and the final
filterExtensions call proceed; ensure you reference and protect the calls around
waitTrustDecided(), initExtensionInstanceData, runEagerExtensionsContributes and
extensionMetaDataArr filtering so the editor lifecycle cannot be blocked by an
unresolved trust promise.
---
Outside diff comments:
In `@packages/extension/src/browser/extension.service.ts`:
- Around line 330-357: initExtensionInstanceData currently always fires
ExtensionsInitializedEvent and DebugConfigurationsReadyEvent and calls
node/worker/view update/init, which causes duplicate/partial-ready notifications
when activate() is called twice for subsets; change initExtensionInstanceData to
accept a boolean parameter (e.g., final = true) and only fire
ExtensionsInitializedEvent/DebugConfigurationsReadyEvent and call
nodeExtensionService.updateExtensionData,
workerExtensionService.updateExtensionData, and
viewExtensionService.initExtension when final is true; update callers (activate)
to pass final=false for the intermediate subset call and only call
initExtensionInstanceData() with final=true (or dispatch the events) once the
final/full extension set is determined.
---
Nitpick comments:
In `@packages/extension/src/browser/extension.service.ts`:
- Around line 246-248: The code always splits extensions into allowed/pending
(using allowedExtensionService.getAllowedExtensionIds and extensionMetaDataArr)
causing unnecessary two-phase init and delay even for non-restricted workspaces;
modify the initialization path to short-circuit by checking
workspaceTrustService.isRestricted() first and, if false, skip splitting and
initialize all extensions in the original single-phase flow (so
pendingExtensions and waitTrustDecided() are not awaited and
runEagerExtensionsContributes/events are only invoked once); keep the restricted
path unchanged so that when isRestricted() is true you still compute allowedIds,
allowedExtensions, pendingExtensions and await waitTrustDecided() before
initializing pending extensions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 77bb6b80-59ae-4d7d-aea2-05c0b3d170e1
📒 Files selected for processing (2)
packages/extension/src/browser/allowed-extension.service.tspackages/extension/src/browser/extension.service.ts
Comment on lines
252
to
+263
| await this.runEagerExtensionsContributes(); | ||
|
|
||
| if (pendingExtensions.length > 0) { | ||
| // Wait for trust decision, then initialize the rest | ||
| await this.allowedExtensionService.waitTrustDecided(); | ||
| const allowedPending = this.allowedExtensionService.filterExtensions(pendingExtensions); | ||
| if (allowedPending.length > 0) { | ||
| await this.initExtensionInstanceData(allowedPending); | ||
| } | ||
| // Run eager extension contributes once all extensions are initialized | ||
| await this.runEagerExtensionsContributes(); | ||
| } |
Contributor
There was a problem hiding this comment.
runEagerExtensionsContributes() 被调用两次会重复注册命令前置钩子并重复派发事件
当 pendingExtensions.length > 0 时，Lines 252 与 262 会两次调用 runEagerExtensionsContributes()。该方法内部执行了 this.commandRegistry.beforeExecuteCommand(...) 注册一个 pre-execute 钩子，且未保存或释放该订阅。第二次调用会再注册一个相同的钩子，导致此后每次命令执行都会 两次 触发 onCommand 激活事件；同时 ExtensionDidContributes 事件也会被重复派发，下游订阅者若不是幂等的将出现重复贡献注册等问题。
♻️ 建议方案
将 runEagerExtensionsContributes 拆分为「contributes 初始化」与「一次性的钩子注册/事件派发」两部分，让钩子注册和 ExtensionDidContributes 仅在受信决策完成、扩展集最终敲定后执行一次：
private async runEagerExtensionsContributes() {
await Promise.all([this.contributesService.initialize(), this.sumiContributesService.initialize()]);
-
- this.commandRegistry.beforeExecuteCommand(async (command, args) => {
- await this.activationEventService.fireEvent('onCommand', command);
- return args;
- });
- this.eventBus.fire(new ExtensionDidContributes());
+ }
+
+ private eagerContributesFinalized = false;
+ private finalizeEagerContributes() {
+ if (this.eagerContributesFinalized) {
+ return;
+ }
+ this.eagerContributesFinalized = true;
+ this.commandRegistry.beforeExecuteCommand(async (command, args) => {
+ await this.activationEventService.fireEvent('onCommand', command);
+ return args;
+ });
+ this.eventBus.fire(new ExtensionDidContributes());
}并在 activate() 中只在最后一次扩展集合敲定后调用 finalizeEagerContributes()。
🤖 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 `@packages/extension/src/browser/extension.service.ts` around lines 252 - 263,
The code calls runEagerExtensionsContributes() twice which re-registers the same
pre-execute hook (commandRegistry.beforeExecuteCommand) and re-dispatches
ExtensionDidContributes; split runEagerExtensionsContributes into two parts: one
function (e.g. prepareEagerContributes) that only computes/initializes
contributes data used by extensions, and a second one (e.g.
finalizeEagerContributes) that registers the one-time pre-execute hook via
commandRegistry.beforeExecuteCommand and emits the ExtensionDidContributes
event; then change activate() flow to call prepareEagerContributes() before
waiting on allowedExtensionService.waitTrustDecided() and
initExtensionInstanceData(pending), and call finalizeEagerContributes() only
once after the trust decision and instance initialization so hooks and events
are registered/dispatched a single time.
Comment on lines
+254
to
+266
| if (pendingExtensions.length > 0) { | ||
| // Wait for trust decision, then initialize the rest | ||
| await this.allowedExtensionService.waitTrustDecided(); | ||
| const allowedPending = this.allowedExtensionService.filterExtensions(pendingExtensions); | ||
| if (allowedPending.length > 0) { | ||
| await this.initExtensionInstanceData(allowedPending); | ||
| } | ||
| // Run eager extension contributes once all extensions are initialized | ||
| await this.runEagerExtensionsContributes(); | ||
| } | ||
|
|
||
| // Final extension list after trust filtering | ||
| this.extensionMetaDataArr = this.allowedExtensionService.filterExtensions(this.extensionMetaDataArr); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 查看 WorkspaceTrustService.whenTrustDecided 的实现与所有 resolve/reject 路径
fd 'workspace-trust.service' --type f --exec rg -nP -C5 'whenTrustDecided|trustDecided|Deferred|resolve\(|reject\(' {} \;Repository: opensumi/core
Length of output: 1557
🏁 Script executed:
fd 'workspace-trust' --type f -e tsRepository: opensumi/core
Length of output: 449
🏁 Script executed:
cat -n $(fd 'workspace-trust.service' --type f)Repository: opensumi/core
Length of output: 5459
🏁 Script executed:
rg -A 20 'initWorkspaceTrust' $(fd 'workspace-trust.service' --type f)Repository: opensumi/core
Length of output: 39
🏁 Script executed:
rg -B 5 -A 10 'whenTrustDecided|showTrustDialog' $(fd 'workspace-trust' --type f -e ts)Repository: opensumi/core
Length of output: 5532
🏁 Script executed:
rg -B 10 -A 5 'waitTrustDecided|allowedExtensionService' packages/extension/src/browser/extension.service.ts | head -80Repository: opensumi/core
Length of output: 2011
🏁 Script executed:
rg 'ensureTrustDecided|showTrustDialog' packages/extension/src/browser/Repository: opensumi/core
Length of output: 39
🏁 Script executed:
fd 'allowed-extension.service' --type f -e tsRepository: opensumi/core
Length of output: 117
🏁 Script executed:
rg -B 5 -A 15 'waitTrustDecided' packages/extensionRepository: opensumi/core
Length of output: 2721
🏁 Script executed:
cat -n packages/extension/src/browser/allowed-extension.service.tsRepository: opensumi/core
Length of output: 2381
🏁 Script executed:
rg -B 5 -A 10 'workspaceTrustService.initialize|ensureTrustDecided' packages/core-browser packages/extension --type tsRepository: opensumi/core
Length of output: 3092
🏁 Script executed:
cat -n packages/core-browser/src/workspace-trust/workspace-trust.contribution.ts | head -60Repository: opensumi/core
Length of output: 1881
🏁 Script executed:
rg -B 5 -A 5 'extensionService.activate|activate\(\)' packages/extension/src/browser/ --type ts | head -100Repository: opensumi/core
Length of output: 8028
🏁 Script executed:
rg 'ClientAppContribution|IExtensionService' packages/extension/src/browser/extension.service.ts | head -20Repository: opensumi/core
Length of output: 39
🏁 Script executed:
rg -B 3 -A 3 'class ExtensionService|implements.*Contribution' packages/extension/src/browser/extension.service.ts | head -20Repository: opensumi/core
Length of output: 336
🏁 Script executed:
rg 'ClientAppContribution' packages/extension/src/browser/extension.service.tsRepository: opensumi/core
Length of output: 39
🏁 Script executed:
rg 'ExtensionService.*implements' packages/extension/src/browser/ -t tsRepository: opensumi/core
Length of output: 194
🏁 Script executed:
rg -B 10 'public async activate' packages/extension/src/browser/extension.service.ts | head -30Repository: opensumi/core
Length of output: 264
🏁 Script executed:
rg '@Domain\(ClientAppContribution\)' packages/extension/src/browser/Repository: opensumi/core
Length of output: 145
🏁 Script executed:
cat -n packages/extension/src/browser/extension.contribution.ts | head -80Repository: opensumi/core
Length of output: 3025
🏁 Script executed:
rg -A 30 'class ExtensionClientAppContribution' packages/extension/src/browser/extension.contribution.ts | grep -A 20 'onStart'Repository: opensumi/core
Length of output: 39
🏁 Script executed:
sed -n '71,200p' packages/extension/src/browser/extension.contribution.ts | grep -A 25 'onStart'Repository: opensumi/core
Length of output: 686
🏁 Script executed:
rg 'extensionService.activate\(\)|ExtensionService.*activate' packages/ --type ts | grep -v node_modules | head -20Repository: opensumi/core
Length of output: 1368
🏁 Script executed:
rg -B 5 -A 5 'this.extensionService.activate' packages/extension/src/browser/Repository: opensumi/core
Length of output: 1075
🏁 Script executed:
rg 'ClientAppContribution' packages/core-browser/src/common/common.define.ts -A 10Repository: opensumi/core
Length of output: 560
🏁 Script executed:
rg 'onStart|initialize.*function\|method' packages/core-browser/src/common/ -t ts | head -20Repository: opensumi/core
Length of output: 226
🏁 Script executed:
rg -B 5 -A 15 'initialize.*ClientAppContribution|initialize()' packages/extension/src/browser/extension.contribution.tsRepository: opensumi/core
Length of output: 664
🏁 Script executed:
sed -n '71,150p' packages/extension/src/browser/extension.contribution.tsRepository: opensumi/core
Length of output: 2307
🏁 Script executed:
rg -B 5 -A 20 'executeCommand.*dialog.open' packages/core-browser/src/workspace-trust/Repository: opensumi/core
Length of output: 6014
🏁 Script executed:
rg 'closable.*false|dialog.*cancel|dialog.*error' packages/ --type ts | head -20Repository: opensumi/core
Length of output: 723
🏁 Script executed:
rg -B 3 -A 10 'try.*catch|error|reject' packages/core-browser/src/workspace-trust/workspace-trust.service.tsRepository: opensumi/core
Length of output: 39
🏁 Script executed:
rg -B 10 -A 10 'class BaseClientApp|ClientAppContribution' packages/core-browser/src/browser-module/application.ts | head -60Repository: opensumi/core
Length of output: 153
🏁 Script executed:
rg 'initialize.*onStart|lifecycle.*hook' packages/core-browser/src/browser-module/ -t ts | head -20Repository: opensumi/core
Length of output: 139
确认信任决策 promise 存在挂起风险，建议添加保护机制
WorkspaceTrustService.whenTrustDecided() 返回的 promise 仅在以下两种情况下才会 resolve：
- 工作空间有已保存的信任状态（在
initialize()时） - 调用
setTrustState()成功（通常通过信任对话框的用户选择触发）
但在以下场景会导致 promise 永不 resolve：
- 工作空间没有已保存的信任状态，且在
extension.service.ts第 256 行的await this.allowedExtensionService.waitTrustDecided()前，WorkspaceTrustContribution.onStart()还未执行完毕（ensureTrustDecided()尚未触发对话框） showTrustDialog()中dialog.open命令执行失败抛出异常，导致setTrustState()永不被调用
建议：
- 在
ExtensionServiceImpl.activate()中主动调用ensureTrustDecided()以保证触发信任对话框，或 - 为
waitTrustDecided()添加超时保护（如 30s），超时时默认按受限模式处理，避免编辑器生命周期卡死
🤖 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 `@packages/extension/src/browser/extension.service.ts` around lines 254 - 266,
The await on this.allowedExtensionService.waitTrustDecided() can hang
indefinitely; update ExtensionServiceImpl.activate() to proactively trigger
trust resolution by calling WorkspaceTrustContribution.ensureTrustDecided() (or
this.allowedExtensionService.ensureTrustDecided() if exposed) before awaiting
waitTrustDecided(), and/or wrap waitTrustDecided() with a timeout fallback
(e.g., 30s) that treats unresolved state as restricted so
initExtensionInstanceData, runEagerExtensionsContributes and the final
filterExtensions call proceed; ensure you reference and protect the calls around
waitTrustDecided(), initExtensionInstanceData, runEagerExtensionsContributes and
extensionMetaDataArr filtering so the editor lifecycle cannot be blocked by an
unresolved trust promise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Types
Background or solution
Changelog
Summary by CodeRabbit
新功能
改进
多语言