Skip to content

Conversation

@louis-jan
Copy link
Contributor

@louis-jan louis-jan commented Aug 12, 2025

Describe Your Changes

This PR resolves the issue preventing users from performing a factory reset while an MCP server is running.

Changes:

  1. Stop all MCP servers before performing factory reset
    2.There should not be any asynchronous event emission for this type of process cleanup.

Fixes Issues

  • Closes #
  • Closes #

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

Important

Fixes factory reset blockage by stopping MCP servers before reset and removing async event emissions.

  • Behavior:
    • factory_reset in cmd.rs now stops all MCP servers using clean_up_mcp_servers before proceeding.
    • Removed asynchronous event emission for MCP server cleanup.
  • Functions:
    • Added clean_up_mcp_servers in mcp.rs to stop and clear MCP servers and restart counts.
    • Removed kill-mcp-servers event listener in setup.rs.
  • Misc:
    • Updated on_window_event in lib.rs to use clean_up_mcp_servers directly for cleanup.

This description was created by Ellipsis for 9ed9861. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 9ed9861 in 1 minute and 8 seconds. Click for details.
  • Reviewed 129 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src-tauri/src/core/cmd.rs:127
  • Draft comment:
    Good cleanup ordering in factory_reset: invoking clean_up_mcp_servers before cleanup_processes ensures MCP servers stop before data deletion. Consider logging or handling errors from these cleanup calls.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. src-tauri/src/core/mcp.rs:757
  • Draft comment:
    In clean_up_mcp_servers, the error from stop_mcp_servers is discarded (using let _ = ...). Consider logging or handling the error to aid debugging if MCP servers fail to stop.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
3. src-tauri/src/core/setup.rs:199
  • Draft comment:
    Removal of the kill-mcp-servers event listener is appropriate since cleanup is now directly performed during shutdown and factory reset. Ensure no other part of the application relies on this event.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
4. src-tauri/src/lib.rs:150
  • Draft comment:
    Directly invoking clean_up_mcp_servers and cleanup_processes on window close and during RunEvent::Exit avoids async event emission. Using block_in_place to synchronously wait for these async cleanups is acceptable if cleanup is quick; verify that this blocking doesn’t adversely affect shutdown.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_k0OWtZtGj75YtG9N

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@github-actions
Copy link
Contributor

Barecheck - Code coverage report

Total: 28.69%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

@louis-jan louis-jan merged commit 34b97e4 into dev Aug 12, 2025
27 checks passed
@louis-jan louis-jan deleted the fix/factory-reset-got-blocked branch August 12, 2025 05:57
@github-project-automation github-project-automation bot moved this to QA in Jan Aug 12, 2025
@github-actions github-actions bot added this to the v0.6.8 milestone Aug 12, 2025
ramonpzg pushed a commit that referenced this pull request Aug 15, 2025
…cked

fix: factory reset process got blocked
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants