Fix issue #105

[rferraton]

What does this PR do?

A clear description of the change and why it's being made.

Which component(s) does this affect?

• Desktop App (PlanViewer.App)
• Core Library (PlanViewer.Core)
• CLI Tool (PlanViewer.Cli)
• SSMS Extension (PlanViewer.Ssms)
• Tests
• Documentation

How was this tested?

• Compile
• load query store
• load a plan from QS

Checklist

• I have read the contributing guide
• My code builds with zero warnings (dotnet build -c Debug)
• [x ] All tests pass (dotnet test)
• [x ] I have not introduced any hardcoded credentials or server names

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Resolved potential issues with status message clearing in the query session control to improve overall stability.

[coderabbitai]

📝 Walkthrough

Walkthrough

Improves cancellation token lifecycle management in QuerySessionControl's SetStatus method by properly canceling and disposing the previous status clear token before creating a new one, with the field reset to null to prevent stale references.

Changes

Cohort / File(s)	Summary
Status Clear Token Lifecycle Management src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs	Adds cancellation, disposal, and null assignment of _statusClearCts field in SetStatus method before new token assignment. Ensures proper cleanup when status text clears after delay. Minor formatting adjustments around Connect_Click.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Fix issue #105' is vague and generic, lacking specific information about what was actually changed or fixed in the codebase.	Replace with a descriptive title that explains the actual fix, e.g., 'Fix cancellation token lifecycle in SetStatus method' to clearly communicate the change to reviewers.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs (1)

333-341: ⚠️ Potential issue | 🟠 Major

Guard delayed status clear from overwriting newer state and ensure proper resource disposal

Line 340 unconditionally nulls _statusClearCts without verifying this callback belongs to the current CTS instance. If SetStatus() is called again before the delayed callback executes, the older callback can clear the new status and CTS reference. Additionally, the CTS created at line 333 is never disposed on the natural expiry path—it is only disposed if SetStatus() is called again, creating a resource leak if the method is not invoked after the delay completes.

🔧 Proposed fix

         if (autoClear && !string.IsNullOrEmpty(text))
         {
-            _statusClearCts = new CancellationTokenSource();
-            var token = _statusClearCts.Token;
+            var cts = new CancellationTokenSource();
+            _statusClearCts = cts;
+            var token = cts.Token;
             _ = Task.Delay(3000, token).ContinueWith(_ =>
             {
                 Avalonia.Threading.Dispatcher.UIThread.Post(() =>
                 {
+                    if (!ReferenceEquals(_statusClearCts, cts))
+                        return;
+
                     StatusText.Text = "";
-                    _statusClearCts = null; // ← also clear after natural expiry
+                    cts.Dispose();
+                    _statusClearCts = null; // ← clear after natural expiry
                 });
             }, TaskContinuationOptions.OnlyOnRanToCompletion);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs` around lines 333 -
341, The delayed callback is unconditionally nulling and never disposing the CTS
it created, which can clear a newer status and leak resources; fix SetStatus()
so you capture the local CTS (e.g., var localCts = _statusClearCts) before
awaiting Task.Delay, then in the continuation verify that _statusClearCts ==
localCts before clearing StatusText.Text and setting _statusClearCts = null, and
finally dispose the local CancellationTokenSource (call localCts.Dispose()) on
the natural expiry path; ensure you continue to respect cancellation by passing
the captured token to Task.Delay and not disposing the CTS while it may still be
in use.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs`:
- Around line 333-341: The delayed callback is unconditionally nulling and never
disposing the CTS it created, which can clear a newer status and leak resources;
fix SetStatus() so you capture the local CTS (e.g., var localCts =
_statusClearCts) before awaiting Task.Delay, then in the continuation verify
that _statusClearCts == localCts before clearing StatusText.Text and setting
_statusClearCts = null, and finally dispose the local CancellationTokenSource
(call localCts.Dispose()) on the natural expiry path; ensure you continue to
respect cancellation by passing the captured token to Task.Delay and not
disposing the CTS while it may still be in use.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 08863d33-cb6c-4e2c-9386-55c84ab0eaf7

📥 Commits

Reviewing files that changed from the base of the PR and between f4f66ec and 0273a66.

📒 Files selected for processing (1)

• src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs