Skip to content

fix(instance): clean up stale install profiles when changing or removing loaders - #2009

Open
UNIkeEN wants to merge 1 commit into
codex/simplify-cleanroom-installerfrom
fix/loader-install-profile-lifecycle
Open

UNIkeEN wants to merge 1 commit into
codex/simplify-cleanroom-installerfrom
fix/loader-install-profile-lifecycle

Conversation

@UNIkeEN

@UNIkeEN UNIkeEN commented Sep 23, 2026 •

Copy link
Copy Markdown
Owner

Checklist

  • 已在本地测试改动并确认功能正常。
  • 所有工作流测试均已通过。
  • 已按需更新文档(本次无需更新)。
  • 代码格式和提交信息符合项目规范。
  • 已按需为复杂逻辑添加注释(本次无需额外注释)。

This PR is a ..

  • 🐞 Bug fix(缺陷修复)

Related Issues

作为 #2002 的后续改进,合入其分支。

Description

切换或移除加载器后,版本目录可能残留旧的 install_profile.json。本次通过公共 helper 在切换、移除加载器时统一清理该文件,覆盖切换到 Fabric、Quilt 等场景;文件不存在时正常继续,其他删除错误正常上报。

安装完成阶段仅允许 Forge 和 NeoForge 执行 processors,替代原有的 Cleanroom 专用清理分支,避免历史残留 profile 影响 Cleanroom 安装。

Additional Context

已通过 cargo check --all-targets、格式检查和 git diff --check。仍有 9 条既有编译警告,未进行实际游戏安装验证。

Summary by Sourcery

Prevent stale loader installation profiles from interfering with mod loader changes and installations.

Bug Fixes:

  • Clean up stale install profiles when changing or removing a mod loader, preventing old loader metadata from affecting subsequent installations.

Enhancements:

  • Restrict installation processor execution to Forge and NeoForge profiles.

@sourcery-ai

sourcery-ai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Fix stale install_profile.json handling by centralizing cleanup during loader changes/removal and limiting installation processors to Forge and NeoForge, preventing leftover profiles from affecting Fabric, Quilt, Cleanroom, and other loader scenarios.

Sequence diagram for loader change and stale profile cleanup

sequenceDiagram
    participant Caller
    participant InstanceCommands
    participant LoaderCommon
    participant VersionDirectory

    Caller->>InstanceCommands: change_mod_loader()
    InstanceCommands->>LoaderCommon: remove_install_profile(instance)
    LoaderCommon->>VersionDirectory: remove_file(install_profile.json)
    VersionDirectory-->>LoaderCommon: success or file absent
    LoaderCommon-->>InstanceCommands: SJMCLResult
    InstanceCommands->>InstanceCommands: schedule_progressive_task_group()
Loading

Sequence diagram for loader removal and profile cleanup

sequenceDiagram
    participant Caller
    participant InstanceCommands
    participant LoaderCommon
    participant VersionDirectory

    Caller->>InstanceCommands: remove_mod_loader()
    InstanceCommands->>InstanceCommands: remove_mod_loader_from_client_info()
    InstanceCommands->>LoaderCommon: remove_install_profile(instance)
    LoaderCommon->>VersionDirectory: remove_file(install_profile.json)
    VersionDirectory-->>LoaderCommon: success or file absent
    LoaderCommon-->>InstanceCommands: SJMCLResult
    InstanceCommands->>InstanceCommands: update mod_loader to Unknown
Loading

Flow diagram for loader-specific install profile processing

flowchart TD
    A["finish_mod_loader_install()"] --> B{Loader type is Forge or NeoForge?}
    B -->|No| C["Skip execute_processors()"]
    B -->|Yes| D{install_profile.json exists?}
    D -->|No| E[Continue installation]
    D -->|Yes| F["load_json_async()"]
    F --> G["execute_processors()"]
    G --> E
Loading

File-Level Changes

Change Details Files
Centralize cleanup of stale install profiles when loader state changes.
  • Add a shared helper that removes version-level install_profile.json when present and propagates other filesystem errors.
  • Invoke cleanup before scheduling a loader change and when removing a loader.
src-tauri/src/instance/helpers/loader/common.rs
src-tauri/src/instance/commands.rs
Restrict post-install processor execution to loaders that use install profiles.
  • Execute processors only for Forge and NeoForge.
  • Stop using the Cleanroom-specific branch that deleted a leftover profile after installation.
src-tauri/src/instance/commands.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@github-actions github-actions Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 23, 2026
@UNIkeEN
UNIkeEN requested a review from xunying123 September 23, 2026 11:26

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src-tauri/src/instance/commands.rs" line_range="1522-1523" />
<code_context>
     .await?;
   }

+  remove_install_profile(&instance)?;
+
   if !modloader_task_params.is_empty() {
     schedule_progressive_task_group(
</code_context>
<issue_to_address>
**issue (bug_risk):** `remove_install_profile(&instance)` deletes the newly generated `install_profile.json` after `install_mod_loader` has prepared a Forge or NeoForge installation. `finish_mod_loader_install` then finds no profile and skips `execute_processors`, so processor-based installation steps are never run when switching to Forge or NeoForge.

**Triggers:** When changing an existing instance to Forge or NeoForge.

**Suggested fix:** Remove the stale profile before calling `install_mod_loader`, or only perform this post-install cleanup when the new loader does not require processors.
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 1 finding to address first, and the change deletes an existing install_profile.json when a loader is changed or removed, and also stops processing profiles for other loader types. If that cleanup is wrong, reverting will not restore a deleted profile, but the profile and associated installation state should be recreatable by reinstalling or rerunning the loader setup.

Blocking findings: src-tauri/src/instance/commands.rs:1523


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment on lines +1522 to +1523
remove_install_profile(&instance)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): remove_install_profile(&instance) deletes the newly generated install_profile.json after install_mod_loader has prepared a Forge or NeoForge installation. finish_mod_loader_install then finds no profile and skips execute_processors, so processor-based installation steps are never run when switching to Forge or NeoForge.

Triggers: When changing an existing instance to Forge or NeoForge.

Suggested fix: Remove the stale profile before calling install_mod_loader, or only perform this post-install cleanup when the new loader does not require processors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant