fix: use direct process termination instead of console events on Windows

[qnixsynapse]

Describe Your Changes

Simplified Windows process cleanup by removing CTRL+C event logic
and using direct child.kill() method. More reliable for headless processes.

Fixes Issues

• Closes bug: Application takes 3-4 seconds to close on Windows #5910 (probably)

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

Simplifies Windows process termination by using child.kill() directly, removing CTRL+C logic, and updating dependencies and logging.

• Behavior:
  • Simplifies Windows process termination by removing CTRL+C event logic and using child.kill() directly in cleanup_processes() in cleanup.rs and unload_llama_model() in server.rs.
  • Logs warnings for unsupported graceful termination on Windows and handles potential errors during child.kill().
• Dependencies:
  • Removes windows-sys dependency from Cargo.toml as it's no longer needed for process termination.
• Misc:
  • Minor logging adjustments in cleanup.rs and server.rs to reflect changes in process termination logic.

^{This description was created by for 48b68f5. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to e767e6c in 1 minute and 9 seconds. Click for details.

• Reviewed 117 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 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/utils/extensions/inference_llamacpp_extension/cleanup.rs:36

• Draft comment:
  Good update for Windows: Direct termination using child.kill() (with a 2‑second timeout) replaces the CTRL-C event, which should help resolve the shutdown delay. Consider logging errors returned by child.kill() for easier debugging if termination fails.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. src-tauri/src/core/utils/extensions/inference_llamacpp_extension/server.rs:331

• Draft comment:
  In unload_llama_model, the removal of CTRL-C event code and the new direct use of child.kill() on Windows should improve shutdown performance. Again, consider handling or logging any errors from child.kill() for better traceability.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_l6xX2DkDM5rEKJSr

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

[github-actions]

Barecheck - Code coverage report

Total: 38.61%

Your code coverage diff: 0.01% ▴

✅ All code changes are covered

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 44e34f2 in 1 minute and 40 seconds. Click for details.

• Reviewed 126 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 3 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/Cargo.toml:64

• Draft comment:
  The windows-sys dependency is now commented out. Confirm it’s not used elsewhere now that we use direct termination. This cleanup looks correct if console events are no longer needed.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 30% <= threshold 50% The comment is asking the author to confirm that the windows-sys dependency is not used elsewhere, which is against the rules. However, it also provides a suggestion that the cleanup looks correct if console events are no longer needed, which is somewhat useful. Overall, the comment leans more towards asking for confirmation, which is not allowed.

2. src-tauri/src/core/utils/extensions/inference_llamacpp_extension/cleanup.rs:36

• Draft comment:
  In the Windows branch, the PR switches from waiting for a graceful shutdown to using direct kill. This direct call to child.kill() with proper error logging should help avoid the 3–4 second delay. Ensure that any race conditions (e.g. process already terminated) are handled as expected.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 40% <= threshold 50% The comment is informative and suggests ensuring that race conditions are handled. It doesn't directly ask the author to confirm their intention or to test the change, but it does imply a need for careful handling of potential issues. However, it doesn't provide a specific suggestion or point out a clear issue with the code. It seems to be more of a cautionary note rather than a direct code review comment.

3. src-tauri/src/core/utils/extensions/inference_llamacpp_extension/server.rs:331

• Draft comment:
  The unload command’s Windows branch now force-kills the process (with a warning log) instead of attempting a graceful shutdown. The logic is consistent with the cleanup changes and should address the slow shutdown issue. Consider refactoring to a shared helper to avoid duplication.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment accurately describes the change and makes a reasonable suggestion about potential code duplication. However, looking at the full file, I don't see obvious duplication that would benefit from a shared helper - the Unix and Windows code paths are quite different in their approach and error handling. The suggestion seems speculative without clear evidence of problematic duplication. The suggestion about refactoring to a shared helper seems premature without clear evidence of problematic duplication. The Unix and Windows paths handle process termination quite differently. While the comment accurately describes the change, the refactoring suggestion isn't clearly justified by the code structure, as the platform-specific termination logic is necessarily different. The comment should be deleted since it's primarily descriptive of changes that are already clear from the code and makes a speculative suggestion without strong justification.

Workflow ID: wflow_vQ3gQs4CrWUECMTk

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

[louis-menlo]

LGTM and works great!

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 48b68f5 in 26 minutes and 3 seconds. Click for details.

• Reviewed 16 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 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/Cargo.toml:64

• Draft comment:
  Removed unused windows-sys dependency comments. This cleanup is appropriate since the new direct kill approach on Windows renders these references unnecessary.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_Xq3MKkkGmlQVOn9n

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