fix(thread-status): 🐛 Prevent runState deadlock when loadSnapshot IPC fails#175
Merged
Conversation
… fails - Reset runMachine to "failed" in loadSnapshot catch block so the optimistic "running" state does not permanently block pending run effect - Downgrade SMAppService errors in settings_set to warn-and-continue so login item failures cannot break the IPC command response
AI Code Review SummaryPR: #175 (fix(thread-status): 🐛 Prevent runState deadlock when loadSnapshot IPC fails) Overall AssessmentNo blocking issue was detected in the reviewed diff; keep focused regression testing before merge. Major Findings by SeverityNo major issues identified from the reviewed diff. Actionable Suggestions
Potential Risks
Test Suggestions
File-Level Coverage Notes
Inline Downgraded Items (processed but not inline)
Coverage Status
Uncovered list:
No-patch covered list:
Runtime/Budget
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // block the pending run effect when the snapshot IPC fails. | ||
| // Use "failed" rather than "idle" because Guard 2 in threadStore rejects | ||
| // idle/null writes when an optimistic running state with a real runId exists. | ||
| if (threadId) runMachine.reset("failed", { runId: null, errorMessage: message, retryCount: 0 }); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| #[cfg(target_os = "macos")] | ||
| { | ||
| startup_manager::set_launch_at_login(enabled)?; | ||
| if let Err(error) = startup_manager::set_launch_at_login(enabled) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| @@ -67,7 +67,9 @@ pub async fn settings_set( | |||
| Ok(enabled) => { | |||
There was a problem hiding this comment.
Automated review completed for this PR diff. No concrete inline issue was selected after aggregation.
| @@ -67,7 +67,9 @@ pub async fn settings_set( | |||
| Ok(enabled) => { | |||
There was a problem hiding this comment.
Automated review completed for this PR diff. No concrete inline issue was selected after aggregation.
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.
Summary
"failed"inloadSnapshotcatch block so the optimistic"running"state does not permanently block the pending run effect when the snapshot IPC call failssettings_setto warn-and-continue so login item failures cannot break the IPC command responseTest Plan
npm run typecheckpassescargo fmt --checkpasses🤖 Generated with TiyCode