Skip to content

feat: support recreate watcher process after reconnected#4517

Merged
erha19 merged 5 commits intomainfrom
feat/support-watcher-process-restart
Apr 25, 2025
Merged

feat: support recreate watcher process after reconnected#4517
erha19 merged 5 commits intomainfrom
feat/support-watcher-process-restart

Conversation

@erha19
Copy link
Copy Markdown
Member

@erha19 erha19 commented Apr 22, 2025

Types

  • 🎉 New Features
  • 🐛 Bug Fixes

Background or solution

Changelog

支持 Watcher Process 的重连逻辑

Summary by CodeRabbit

  • 新功能
    • 文件服务客户端新增了重连和资源释放功能,提升了断线后的自动恢复能力。
  • 优化
    • 文件搜索和文件监视的排除规则现在将动态适配默认配置,提升了搜索和监控的准确性与灵活性。
    • 文件监视器在重建时会自动释放旧连接,避免重复实例和资源浪费。
  • 修复
    • 修正了文件监视器可能因未正确释放导致的重复监听问题。
  • 移除
    • 文件树相关的重连和重新监听功能被移除,简化了文件树服务逻辑。

@erha19
Copy link
Copy Markdown
Member Author

erha19 commented Apr 22, 2025

/next

@opensumi opensumi Bot added the 🎨 feature feature required label Apr 22, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 22, 2025

"""

Walkthrough

本次更改主要围绕文件服务和文件搜索相关逻辑进行调整。文件搜索功能中的排除模式由硬编码数组改为动态获取默认排除规则。文件服务客户端引入了日志记录器,增强了文件变更监听的管理,新增了重连(reconnect)方法以支持断线重连和资源释放。文件服务贡献类也新增了重连处理逻辑。文件树相关的重连和重新监听逻辑被移除。文件监视进程管理器则在创建新进程时会关闭已有的重复连接,防止多实例冲突。接口声明也相应扩展以支持新方法。

Changes

文件/路径分组 变更摘要
packages/ai-native/src/browser/components/ChatMentionInput.tsx
packages/ai-native/src/browser/mcp/tools/fileSearch.ts
文件搜索服务的排除模式由硬编码数组切换为动态获取默认排除规则。
packages/file-service/src/browser/file-service-client.ts 注入日志记录器,watcher 逻辑优化(重复 watcher 先销毁再新建),新增 reconnect 方法支持重连与日志记录。
packages/file-service/src/browser/file-service-contribution.ts 注入 fileServiceClient 和 logger,新增 onReconnect 生命周期方法,支持重连并记录错误。
packages/file-service/src/common/file-service-client.ts IFileServiceClient 接口新增 reconnect()dispose() 方法声明。
packages/file-service/src/node/watcher-process-manager.ts 创建进程时若已存在同 clientId 的 watcher server,则先关闭再新建,避免重复实例。
packages/file-tree-next/src/browser/file-tree-contribution.ts
packages/file-tree-next/src/browser/file-tree.service.ts
移除文件树相关的重连和重新监听方法(onReconnect 和 reWatch)。
packages/file-tree-next/tests/browser/file-tree-contribution.test.ts
packages/file-tree-next/tests/browser/file-tree.service.test.ts
移除文件树重连相关的测试用例。

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant FileServiceContribution
    participant FileServiceClient
    participant FileProvider
    participant Logger

    User->>FileServiceContribution: 触发 onReconnect()
    FileServiceContribution->>FileServiceClient: 调用 reconnect()
    FileServiceClient->>FileProvider: 初始化(如支持)
    loop 遍历所有 watcher
        FileServiceClient->>FileServiceClient: 重新调用 watchFileChanges
    end
    FileServiceClient-->>FileServiceContribution: 完成(或抛出错误)
    FileServiceContribution->>Logger: 记录错误(如有)
Loading

Suggested labels

🐞 bug

Suggested reviewers

  • Ricbet
  • Aaaaash
    """

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

yarn install v1.22.22
[1/4] Resolving packages...
warning eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options.
warning eslint > @humanwhocodes/config-array@0.13.0: Use @eslint/config-array instead
warning eslint > @humanwhocodes/config-array > @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
warning eslint > file-entry-cache > flat-cache > rimraf@3.0.2: Rimraf versions prior to v4 are no longer supported
warning eslint > file-entry-cache > flat-cache > rimraf > glob@7.2.3: Glob versions prior to v9 are no longer supported
warning eslint > file-entry-cache > flat-cache > rimraf > glob > inflight@1.0.6: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
warning eslint-import-resolver-typescript > glob@7.2.3: Glob versions prior to v9 are no longer supported
error Couldn't find any versions for "@opensumi/ide-dev-tool" that matches "workspace:*"
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b4c46c2 and 6be07dd.

📒 Files selected for processing (1)
  • packages/file-service/src/browser/file-service-client.ts (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/file-service/src/browser/file-service-client.ts
⏰ Context from checks skipped due to timeout of 90000ms (9)
  • GitHub Check: 🚀🚀🚀 Next Version for pull request
  • GitHub Check: build (ubuntu-latest, 20.x)
  • GitHub Check: build (macos-latest, 20.x)
  • GitHub Check: build-windows
  • GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
  • GitHub Check: unittest (macos-latest, 18.x, jsdom)
  • GitHub Check: unittest (macos-latest, 18.x, node)
  • GitHub Check: ubuntu-latest, Node.js 20.x
  • GitHub Check: unittest (ubuntu-latest, 18.x, node)

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
packages/file-service/src/common/file-service-client.ts (1)

91-94: 接口新增方法缺少说明与使用示例

reconnectdispose 的加入非常必要,但接口注释尚未同步更新,也缺少对调用时机的说明。建议为两个新方法补充 JSDoc,阐明:

  1. 何时应显式调用 reconnect(如网络重连、IPC 重建场景)。
  2. dispose 需要释放哪些资源、能否多次调用。
    这样可避免实现方语义不一致的问题。
packages/file-service/src/browser/file-service-contribution.ts (1)

31-36: 重复注入同一 Token 导致维护成本增加

fileSystemFileServiceClient)本身已实现 IFileServiceClient,再次注入 fileServiceClient 属性意义不大,且易造成以后两处对象引用不一致。建议保留一处即可:

-@Autowired(IFileServiceClient)
-protected readonly fileServiceClient: IFileServiceClient;
+// 直接复用 fileSystem 即可

同时,ILogger 在 browser 侧使用的是 @opensumi/ide-core-browser 的 Token,而其它文件使用 @opensumi/ide-core-common,请保持一致以避免依赖冲突。

packages/file-service/src/browser/file-service-client.ts (1)

119-121: ILogger Token 不统一

本文件从 ide-core-common 注入 ILogger,而 file-service-contribution.tside-core-browser 注入。若两处 Token 不同,将导致注入两个实例。请确认实际导出位置并保持一致。

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bebda40 and 932a5c4.

📒 Files selected for processing (8)
  • packages/ai-native/src/browser/components/ChatMentionInput.tsx (2 hunks)
  • packages/ai-native/src/browser/mcp/tools/fileSearch.ts (2 hunks)
  • packages/file-service/src/browser/file-service-client.ts (4 hunks)
  • packages/file-service/src/browser/file-service-contribution.ts (3 hunks)
  • packages/file-service/src/common/file-service-client.ts (1 hunks)
  • packages/file-service/src/node/watcher-process-manager.ts (1 hunks)
  • packages/file-tree-next/src/browser/file-tree-contribution.ts (0 hunks)
  • packages/file-tree-next/src/browser/file-tree.service.ts (0 hunks)
💤 Files with no reviewable changes (2)
  • packages/file-tree-next/src/browser/file-tree.service.ts
  • packages/file-tree-next/src/browser/file-tree-contribution.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/ai-native/src/browser/components/ChatMentionInput.tsx (1)
packages/core-common/src/preferences/file-watch.ts (1)
  • defaultFilesWatcherExcludes (1-6)
packages/ai-native/src/browser/mcp/tools/fileSearch.ts (1)
packages/core-common/src/preferences/file-watch.ts (1)
  • defaultFilesWatcherExcludes (1-6)
packages/file-service/src/browser/file-service-contribution.ts (3)
packages/file-service/src/common/file-service-client.ts (2)
  • IFileServiceClient (27-27)
  • IFileServiceClient (29-116)
packages/core-common/src/types/file.ts (1)
  • IFileServiceClient (12-12)
packages/core-common/src/log.ts (2)
  • ILogger (305-305)
  • ILogger (306-306)
packages/file-service/src/browser/file-service-client.ts (4)
packages/core-common/src/log.ts (2)
  • ILogger (305-305)
  • ILogger (306-306)
packages/connection/src/common/types.ts (1)
  • ILogger (1-5)
packages/file-service/src/common/watcher.ts (1)
  • IFileServiceWatcher (17-21)
packages/utils/src/uri.ts (2)
  • URI (6-6)
  • URI (17-311)
⏰ Context from checks skipped due to timeout of 90000ms (8)
  • GitHub Check: 🚀🚀🚀 Next Version for pull request
  • GitHub Check: unittest (ubuntu-latest, 18.x, node)
  • GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
  • GitHub Check: unittest (macos-latest, 18.x, jsdom)
  • GitHub Check: build (macos-latest, 20.x)
  • GitHub Check: build (ubuntu-latest, 20.x)
  • GitHub Check: build-windows
  • GitHub Check: ubuntu-latest, Node.js 20.x
🔇 Additional comments (5)
packages/ai-native/src/browser/mcp/tools/fileSearch.ts (2)

6-6: 优秀的导入添加

引入 defaultFilesWatcherExcludes 常量可以实现文件排除模式的统一管理,避免硬编码。这是一个很好的改进。


67-67: 统一使用配置的排除模式,提高一致性

将硬编码的排除模式数组 ['**/node_modules/**'] 替换为 Object.keys(defaultFilesWatcherExcludes),这样做有几个好处:

  1. 与系统其他部分使用相同的排除规则,保持一致性
  2. 扩展了排除范围,现在不仅排除 node_modules,还包括 .git 相关目录等
  3. 当需要调整排除规则时,只需修改一处配置

这个修改很好地提高了代码的可维护性和灵活性。

packages/ai-native/src/browser/components/ChatMentionInput.tsx (2)

9-9: 合理的导入添加

添加 defaultFilesWatcherExcludes 的导入,为后续使用统一的文件排除模式做准备。


220-220: 统一的排除策略应用

使用 Object.keys(defaultFilesWatcherExcludes) 代替之前可能硬编码的排除模式数组,这个改动:

  1. 确保了文件夹搜索与文件搜索使用相同的排除规则
  2. 简化了维护工作,当排除规则需要调整时只需要修改 defaultFilesWatcherExcludes 定义
  3. 增强了代码的一致性和可读性

这是一个很好的实践,消除了代码中的重复定义。

packages/file-service/src/browser/file-service-contribution.ts (1)

63-65: onReconnect 方法缺少生命周期挂载点

ClientAppContribution 并未强制调用 onReconnect,需要确认框架层会在重连事件发生时触发此方法;否则此实现不会生效。若需监听 WebSocket/IPC 重连事件,应通过 IConnectionService 或相关 EventBus 订阅,再显式调用 fileSystem.reconnect()

Comment thread packages/file-service/src/node/watcher-process-manager.ts
Comment thread packages/file-service/src/browser/file-service-client.ts
Comment thread packages/file-service/src/browser/file-service-client.ts
@opensumi
Copy link
Copy Markdown
Contributor

opensumi Bot commented Apr 22, 2025

🎉 PR Next publish successful!

3.8.3-next-1745295840.0

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2025

Codecov Report

Attention: Patch coverage is 31.42857% with 24 lines in your changes missing coverage. Please review.

Project coverage is 53.11%. Comparing base (34699ec) to head (4994a73).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...es/file-service/src/browser/file-service-client.ts 37.93% 13 Missing and 5 partials ⚠️
...s/file-service/src/node/watcher-process-manager.ts 0.00% 4 Missing and 1 partial ⚠️
...ages/ai-native/src/browser/mcp/tools/fileSearch.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4517      +/-   ##
==========================================
- Coverage   53.12%   53.11%   -0.02%     
==========================================
  Files        1665     1665              
  Lines      102711   102731      +20     
  Branches    22240    22240              
==========================================
  Hits        54567    54567              
- Misses      40050    40065      +15     
- Partials     8094     8099       +5     
Flag Coverage Δ
jsdom 48.61% <31.42%> (-0.01%) ⬇️
node 12.12% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

erha19 and others added 2 commits April 22, 2025 15:03
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@erha19
Copy link
Copy Markdown
Member Author

erha19 commented Apr 22, 2025

/next

@opensumi
Copy link
Copy Markdown
Contributor

opensumi Bot commented Apr 22, 2025

🎉 PR Next publish successful!

3.8.3-next-1745307078.0

@erha19
Copy link
Copy Markdown
Member Author

erha19 commented Apr 22, 2025

/next

@opensumi
Copy link
Copy Markdown
Contributor

opensumi Bot commented Apr 22, 2025

🎉 PR Next publish successful!

3.8.3-next-1745309832.0

@erha19 erha19 merged commit 24334e4 into main Apr 25, 2025
12 of 16 checks passed
@erha19 erha19 deleted the feat/support-watcher-process-restart branch April 25, 2025 02:00
@erha19 erha19 mentioned this pull request May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🎨 feature feature required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants