From ac2b959c3e1f8fd97dd888714008d778e3574e86 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:59:18 -0400 Subject: [PATCH 1/3] Skip review on release PRs; widen the reviewer tool allowlist The review failed on the v1.19.0 release PR with 20 permission denials and "Reached maximum number of turns (40)". Two causes, both fixed. Release PRs are the wrong thing to review. A dev -> main PR aggregates commits that were each already reviewed on their own PR, so re-reviewing the whole release adds nothing - and the diff is large enough to reliably exhaust the turn budget, which then puts a red X on the release itself. Now skipped. The allowlist was still too narrow. --allowedTools REPLACES the default tool set rather than adding to it, so anything omitted is denied at runtime - and a denied call still consumes a turn. A short list therefore burns the budget and the review dies having posted nothing, which is how both #399 (10 denials) and this one (20) failed. Added the ordinary read-only exploration tools plus TodoWrite, and raised the cap to 60. Everything on the list is read-only except the two comment-posting tools. Co-Authored-By: Claude Fable 5 --- .github/workflows/claude-code-review.yml | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 1aaec8e..c3916c3 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -32,9 +32,15 @@ jobs: # Drafts aren't ready for review, and a fork PR gets no secrets (so # CLAUDE_CODE_OAUTH_TOKEN would be empty) and a read-only token it could # not post with. Skip rather than fail a contributor's PR with a red X. + # + # Also skip the dev -> main release PR. It is an aggregation of commits that + # were each already reviewed on their own PR, so re-reviewing the whole + # release adds nothing — and the diff is large enough that it reliably + # exhausts the turn budget and fails, putting a red X on the release. if: | github.event.pull_request.draft == false && - github.event.pull_request.head.repo.full_name == github.repository + github.event.pull_request.head.repo.full_name == github.repository && + !(github.event.pull_request.head.ref == 'dev' && github.event.pull_request.base.ref == 'main') runs-on: ubuntu-latest steps: @@ -91,14 +97,16 @@ jobs: `confirmed: true`) to flag specific lines. Only post GitHub comments - do not return review text as a message. - # --allowedTools REPLACES the default tool set rather than adding to it. - # Without Read/Grep/Glob the reviewer cannot open a single file in the - # checked-out branch, which is exactly what the prompt above asks it to - # do. PR #399 burned all 20 turns on denied calls - # (permission_denials_count was 10) and failed without posting anything. + # --allowedTools REPLACES the default tool set rather than adding to it, + # so anything omitted here is denied at runtime. A denied call still + # consumes a turn, so a too-narrow list burns the budget and the review + # fails having posted nothing — that is how #399 (10 denials) and the + # v1.19.0 release PR (20 denials) both died. + # + # Everything here is read-only except the two comment-posting tools. claude_args: | - --max-turns 40 - --allowedTools "Read,Grep,Glob,mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(git diff:*),Bash(git log:*)" + --max-turns 60 + --allowedTools "Read,Grep,Glob,TodoWrite,mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh api:*),Bash(git diff:*),Bash(git log:*),Bash(git show:*),Bash(git status:*),Bash(ls:*),Bash(cat:*),Bash(head:*),Bash(tail:*),Bash(wc:*),Bash(find:*),Bash(rg:*)" # Fork PRs: GitHub withholds repository secrets from `pull_request` runs on # forks and issues a read-only GITHUB_TOKEN, so this workflow cannot review From 7cd92185b3904379c3e3c6c1c62fad3d14f2fb4f Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:16:27 -0400 Subject: [PATCH 2/3] Harden every clipboard path against a busy Windows clipboard The Windows clipboard is a shared resource; when another process holds it, SetTextAsync throws CLIPBRD_E_CANT_OPEN, and an unguarded await in an async void handler crashes the app (issue #415, Robot Advice Copy to Clipboard). - New ClipboardHelper: all reads/writes retry briefly and never throw - Convert all 15 write sites and both paste sites in PlanViewer.App - Replace DataGrid's built-in Ctrl+C (unguarded inside Avalonia) with a guarded copy on all four grids, mirroring its exact key/modifier gate - Intercept TextBox Copy/Cut/Paste app-wide via class handlers; Avalonia's own implementations are unguarded async void / TimeoutException-only - Cut (editor and TextBoxes) only deletes the selection after the text actually reached the clipboard - Clipboard-busy feedback on Advice window, Copy Repro, and MCP copy button - Log AppDomain/TaskScheduler unhandled exceptions to LocalApplicationData\PerformanceStudio\crash.log Fixes #415 Co-Authored-By: Claude Fable 5 --- src/PlanViewer.App/AboutWindow.axaml.cs | 10 +-- src/PlanViewer.App/App.axaml.cs | 4 + .../Controls/PlanViewerControl.Interaction.cs | 9 +-- .../Controls/PlanViewerControl.Schema.cs | 11 +-- .../Controls/PlanViewerControl.Statements.cs | 5 +- .../Controls/PlanViewerControl.axaml.cs | 2 + .../Controls/QuerySessionControl.Editor.cs | 17 ++-- .../Controls/QuerySessionControl.Format.cs | 17 +--- .../Controls/QuerySessionControl.Schema.cs | 10 +-- .../QueryStoreGridControl.Selection.cs | 14 ++-- .../Controls/QueryStoreGridControl.axaml.cs | 2 + .../QueryStoreHistoryControl.axaml.cs | 31 +++++++- .../Dialogs/SettingsWindow.axaml.cs | 3 + .../Helpers/DataGridBehaviors.cs | 50 +++++++++++- src/PlanViewer.App/MainWindow.FileOps.cs | 8 +- src/PlanViewer.App/MainWindow.PlanViewer.cs | 10 +-- src/PlanViewer.App/MainWindow.Tabs.cs | 2 +- src/PlanViewer.App/Program.cs | 12 +++ .../Services/AdviceWindowHelper.cs | 13 ++- .../Services/ClipboardHelper.cs | 79 +++++++++++++++++++ src/PlanViewer.App/Services/CrashLogger.cs | 43 ++++++++++ .../Services/TextBoxClipboardGuard.cs | 64 +++++++++++++++ 22 files changed, 329 insertions(+), 87 deletions(-) create mode 100644 src/PlanViewer.App/Services/ClipboardHelper.cs create mode 100644 src/PlanViewer.App/Services/CrashLogger.cs create mode 100644 src/PlanViewer.App/Services/TextBoxClipboardGuard.cs diff --git a/src/PlanViewer.App/AboutWindow.axaml.cs b/src/PlanViewer.App/AboutWindow.axaml.cs index 1432a2e..5db3a71 100644 --- a/src/PlanViewer.App/AboutWindow.axaml.cs +++ b/src/PlanViewer.App/AboutWindow.axaml.cs @@ -10,7 +10,6 @@ using System.Runtime.InteropServices; using Avalonia.Controls; using Avalonia.Input; -using Avalonia.Input.Platform; using Avalonia.Interactivity; using PlanViewer.App.Mcp; using PlanViewer.App.Services; @@ -111,12 +110,9 @@ private async void CopyMcpCommand_Click(object? sender, RoutedEventArgs e) { var port = int.TryParse(McpPortInput.Text, out var p) && p >= 1024 && p <= 65535 ? p : 5152; var command = $"claude mcp add --transport streamable-http --scope user performance-studio http://localhost:{port}/"; - var clipboard = TopLevel.GetTopLevel(this)?.Clipboard; - if (clipboard != null) - { - await clipboard.SetTextAsync(command); - McpCopyStatus.Text = "Copied to clipboard!"; - } + McpCopyStatus.Text = await ClipboardHelper.TrySetTextAsync(this, command) + ? "Copied to clipboard!" + : "Clipboard busy - try again"; } private string? _updateUrl; diff --git a/src/PlanViewer.App/App.axaml.cs b/src/PlanViewer.App/App.axaml.cs index 29fd241..0f328bf 100644 --- a/src/PlanViewer.App/App.axaml.cs +++ b/src/PlanViewer.App/App.axaml.cs @@ -21,6 +21,10 @@ public override void Initialize() public override void OnFrameworkInitializationCompleted() { + // Before any window exists: route every TextBox clipboard operation through + // the guarded helper so a locked clipboard can't crash the app (issue #415). + TextBoxClipboardGuard.Register(); + if (ApplicationLifetime is IClassicDesktopStyleApplicationLifetime desktop) { desktop.MainWindow = new MainWindow(); diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Interaction.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Interaction.cs index 0848235..a67609e 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Interaction.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Interaction.cs @@ -7,6 +7,7 @@ using Avalonia.Interactivity; using Avalonia.Media; using Avalonia.Platform.Storage; +using PlanViewer.App.Services; using PlanViewer.Core.Models; namespace PlanViewer.App.Controls; @@ -138,12 +139,8 @@ private ContextMenu BuildCanvasContextMenu() return menu; } - private async System.Threading.Tasks.Task SetClipboardTextAsync(string text) - { - var topLevel = TopLevel.GetTopLevel(this); - if (topLevel?.Clipboard != null) - await topLevel.Clipboard.SetTextAsync(text); - } + private System.Threading.Tasks.Task SetClipboardTextAsync(string text) + => ClipboardHelper.TrySetTextAsync(this, text); private void ZoomIn_Click(object? sender, RoutedEventArgs e) => SetZoom(_zoomLevel + ZoomStep); diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Schema.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Schema.cs index 5f55059..e772bb5 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Schema.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Schema.cs @@ -10,6 +10,7 @@ using Avalonia.Media; using AvaloniaEdit.TextMate; using Microsoft.Data.SqlClient; +using PlanViewer.App.Services; using PlanViewer.Core.Interfaces; using PlanViewer.Core.Models; using PlanViewer.Core.Services; @@ -117,19 +118,13 @@ private void ShowSchemaResult(string title, string content) var copyItem = new MenuItem { Header = "Copy" }; copyItem.Click += async (_, _) => { - var clipboard = TopLevel.GetTopLevel(this)?.Clipboard; - if (clipboard == null) return; var sel = editor.TextArea.Selection; if (!sel.IsEmpty) - await clipboard.SetTextAsync(sel.GetText()); + await ClipboardHelper.TrySetTextAsync(this, sel.GetText()); }; var copyAllItem = new MenuItem { Header = "Copy All" }; copyAllItem.Click += async (_, _) => - { - var clipboard = TopLevel.GetTopLevel(this)?.Clipboard; - if (clipboard == null) return; - await clipboard.SetTextAsync(editor.Text); - }; + await ClipboardHelper.TrySetTextAsync(this, editor.Text); var selectAllItem = new MenuItem { Header = "Select All" }; selectAllItem.Click += (_, _) => editor.SelectAll(); editor.TextArea.ContextMenu = new ContextMenu diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Statements.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Statements.cs index a03c712..daaaaf5 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Statements.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Statements.cs @@ -8,6 +8,7 @@ using Avalonia.Interactivity; using Avalonia.Layout; using Avalonia.Media; +using PlanViewer.App.Services; using PlanViewer.Core.Models; using PlanViewer.Core.Services; @@ -168,9 +169,7 @@ private async void CopyStatementText_Click(object? sender, RoutedEventArgs e) var text = row.Statement.StatementText; if (string.IsNullOrEmpty(text)) return; - var topLevel = TopLevel.GetTopLevel(this); - if (topLevel?.Clipboard != null) - await topLevel.Clipboard.SetTextAsync(text); + await ClipboardHelper.TrySetTextAsync(this, text); } private void OpenInEditor_Click(object? sender, RoutedEventArgs e) diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs b/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs index 195fdb5..ed97937 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs @@ -164,6 +164,8 @@ public PlanViewerControl() _zoomTransform = (ScaleTransform)layoutTransform.LayoutTransform!; Helpers.DataGridBehaviors.Attach(StatementsGrid); + Helpers.DataGridBehaviors.AttachCopyGuard(StatementsGrid, + item => item is StatementRow row ? row.Statement.StatementText : null); // Wire minimap resize grip (defined in AXAML, not in canvas) MinimapResizeGrip.PointerPressed += MinimapResizeGrip_PointerPressed; diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Editor.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Editor.cs index 883292d..9a612cc 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Editor.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Editor.cs @@ -10,7 +10,6 @@ using System.Xml.Linq; using Avalonia.Controls; using Avalonia.Input; -using Avalonia.Input.Platform; using Avalonia.Interactivity; using Avalonia.Layout; using Avalonia.Media; @@ -42,31 +41,27 @@ private void SetupEditorContextMenu() var cutItem = new MenuItem { Header = "Cut" }; cutItem.Click += async (_, _) => { - var clipboard = TopLevel.GetTopLevel(this)?.Clipboard; - if (clipboard == null) return; var selection = QueryEditor.TextArea.Selection; if (selection.IsEmpty) return; var text = selection.GetText(); - await clipboard.SetTextAsync(text); - selection.ReplaceSelectionWithText(""); + // Only remove the selection once the text has actually reached the + // clipboard; a failed copy must not destroy the user's text. + if (await ClipboardHelper.TrySetTextAsync(this, text)) + selection.ReplaceSelectionWithText(""); }; var copyItem = new MenuItem { Header = "Copy" }; copyItem.Click += async (_, _) => { - var clipboard = TopLevel.GetTopLevel(this)?.Clipboard; - if (clipboard == null) return; var selection = QueryEditor.TextArea.Selection; if (selection.IsEmpty) return; - await clipboard.SetTextAsync(selection.GetText()); + await ClipboardHelper.TrySetTextAsync(this, selection.GetText()); }; var pasteItem = new MenuItem { Header = "Paste" }; pasteItem.Click += async (_, _) => { - var clipboard = TopLevel.GetTopLevel(this)?.Clipboard; - if (clipboard == null) return; - var text = await clipboard.TryGetTextAsync(); + var text = await ClipboardHelper.TryGetTextAsync(this); if (string.IsNullOrEmpty(text)) return; QueryEditor.TextArea.PerformTextInput(text); }; diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Format.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Format.cs index e913411..577504d 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Format.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Format.cs @@ -60,19 +60,10 @@ otherwise fall back to the currently selected database */ source: "Performance Studio", isAzureSqlDb: IsAzureConnection); - try - { - var topLevel = TopLevel.GetTopLevel(this); - if (topLevel?.Clipboard != null) - { - await topLevel.Clipboard.SetTextAsync(reproScript); - SetStatus("Repro script copied to clipboard"); - } - } - catch (Exception ex) - { - SetStatus($"Clipboard error: {ex.Message}"); - } + if (await ClipboardHelper.TrySetTextAsync(this, reproScript)) + SetStatus("Repro script copied to clipboard"); + else + SetStatus("Clipboard busy - could not copy repro script"); } private async void Format_Click(object? sender, RoutedEventArgs e) diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Schema.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Schema.cs index 492ca1d..9682cda 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Schema.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Schema.cs @@ -104,19 +104,13 @@ private void AddSchemaTab(string label, string content, bool isSql) var schemaCopy = new MenuItem { Header = "Copy" }; schemaCopy.Click += async (_, _) => { - var clipboard = TopLevel.GetTopLevel(this)?.Clipboard; - if (clipboard == null) return; var sel = editor.TextArea.Selection; if (!sel.IsEmpty) - await clipboard.SetTextAsync(sel.GetText()); + await ClipboardHelper.TrySetTextAsync(this, sel.GetText()); }; var schemaCopyAll = new MenuItem { Header = "Copy All" }; schemaCopyAll.Click += async (_, _) => - { - var clipboard = TopLevel.GetTopLevel(this)?.Clipboard; - if (clipboard == null) return; - await clipboard.SetTextAsync(editor.Text); - }; + await ClipboardHelper.TrySetTextAsync(this, editor.Text); var schemaSelectAll = new MenuItem { Header = "Select All" }; schemaSelectAll.Click += (_, _) => editor.SelectAll(); editor.TextArea.ContextMenu = new ContextMenu diff --git a/src/PlanViewer.App/Controls/QueryStoreGridControl.Selection.cs b/src/PlanViewer.App/Controls/QueryStoreGridControl.Selection.cs index d9c63ea..0d5b62c 100644 --- a/src/PlanViewer.App/Controls/QueryStoreGridControl.Selection.cs +++ b/src/PlanViewer.App/Controls/QueryStoreGridControl.Selection.cs @@ -159,7 +159,7 @@ private void ContextMenu_Opening(object? sender, System.ComponentModel.CancelEve CopyPlanHashItem.Tag = row.QueryPlanHash; CopyModuleItem.Tag = row.ModuleName; CopyQueryTextItem.Tag = row.FullQueryText; - CopyRowItem.Tag = $"{row.QueryId}\t{row.PlanId}\t{row.QueryHash}\t{row.QueryPlanHash}\t{row.ModuleName}\t{row.LastExecutedLocal}\t{row.ExecsDisplay}\t{row.TotalCpuDisplay}\t{row.AvgCpuDisplay}\t{row.TotalDurDisplay}\t{row.AvgDurDisplay}\t{row.TotalReadsDisplay}\t{row.AvgReadsDisplay}\t{row.TotalWritesDisplay}\t{row.AvgWritesDisplay}\t{row.TotalPhysReadsDisplay}\t{row.AvgPhysReadsDisplay}\t{row.TotalMemDisplay}\t{row.AvgMemDisplay}\t{row.FullQueryText}"; + CopyRowItem.Tag = FormatRowForClipboard(row); CopyQueryIdItem.Click += CopyMenuItem_Click; CopyPlanIdItem.Click += CopyMenuItem_Click; @@ -176,10 +176,10 @@ private async void CopyMenuItem_Click(object? sender, RoutedEventArgs e) await SetClipboardTextAsync(text); } - private async System.Threading.Tasks.Task SetClipboardTextAsync(string text) - { - var topLevel = Avalonia.Controls.TopLevel.GetTopLevel(this); - if (topLevel?.Clipboard != null) - await topLevel.Clipboard.SetTextAsync(text); - } + /// One results row as a tab-separated line, shared by Copy Row and Ctrl+C. + private static string FormatRowForClipboard(QueryStoreRow row) => + $"{row.QueryId}\t{row.PlanId}\t{row.QueryHash}\t{row.QueryPlanHash}\t{row.ModuleName}\t{row.LastExecutedLocal}\t{row.ExecsDisplay}\t{row.TotalCpuDisplay}\t{row.AvgCpuDisplay}\t{row.TotalDurDisplay}\t{row.AvgDurDisplay}\t{row.TotalReadsDisplay}\t{row.AvgReadsDisplay}\t{row.TotalWritesDisplay}\t{row.AvgWritesDisplay}\t{row.TotalPhysReadsDisplay}\t{row.AvgPhysReadsDisplay}\t{row.TotalMemDisplay}\t{row.AvgMemDisplay}\t{row.FullQueryText}"; + + private System.Threading.Tasks.Task SetClipboardTextAsync(string text) + => ClipboardHelper.TrySetTextAsync(this, text); } diff --git a/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml.cs b/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml.cs index f2690c1..41ba627 100644 --- a/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml.cs +++ b/src/PlanViewer.App/Controls/QueryStoreGridControl.axaml.cs @@ -86,6 +86,8 @@ public QueryStoreGridControl(ServerConnection serverConnection, ICredentialServi ResultsGrid.ItemsSource = _filteredRows; Helpers.DataGridBehaviors.Attach(ResultsGrid); + Helpers.DataGridBehaviors.AttachCopyGuard(ResultsGrid, + item => item is QueryStoreRow row ? FormatRowForClipboard(row) : null); EnsureFilterPopup(); SetupColumnHeaders(); PopulateDatabaseBox(databases, initialDatabase); diff --git a/src/PlanViewer.App/Controls/QueryStoreHistoryControl.axaml.cs b/src/PlanViewer.App/Controls/QueryStoreHistoryControl.axaml.cs index a3910c3..155eff9 100644 --- a/src/PlanViewer.App/Controls/QueryStoreHistoryControl.axaml.cs +++ b/src/PlanViewer.App/Controls/QueryStoreHistoryControl.axaml.cs @@ -9,6 +9,7 @@ using Avalonia.Interactivity; using Avalonia.Media; using Avalonia.VisualTree; +using PlanViewer.App.Services; using PlanViewer.Core.Models; using PlanViewer.Core.Services; using ScottPlot; @@ -135,6 +136,8 @@ public QueryStoreHistoryControl(string connectionString, string queryHash, InitializeComponent(); Helpers.DataGridBehaviors.Attach(HistoryDataGrid); + Helpers.DataGridBehaviors.AttachCopyGuard(HistoryDataGrid, + item => item is QueryStoreHistoryRow row ? FormatRowForClipboard(row) : null); QueryIdentifierText.Text = $"Query Store History: {queryHash} in [{database}]"; QueryTextBox.Text = queryText; @@ -307,11 +310,33 @@ private void MetricSelector_SelectionChanged(object? sender, SelectionChangedEve private async void CopyQuery_Click(object? sender, RoutedEventArgs e) { if (string.IsNullOrEmpty(_queryText)) return; - var clipboard = TopLevel.GetTopLevel(this)?.Clipboard; - if (clipboard != null) - await clipboard.SetTextAsync(_queryText); + await ClipboardHelper.TrySetTextAsync(this, _queryText); } + /// One history row as a tab-separated line matching the grid's column order. + private static string FormatRowForClipboard(QueryStoreHistoryRow row) => + string.Join("\t", + row.QueryPlanHash, + row.IntervalStartLocal, + row.CountExecutions, + row.AvgDurationMsDisplay, + row.AvgCpuMsDisplay, + row.AvgLogicalReadsDisplay, + row.AvgLogicalWritesDisplay, + row.AvgPhysicalReadsDisplay, + row.AvgMemoryMbDisplay, + row.AvgRowcountDisplay, + row.TotalDurationMsDisplay, + row.TotalCpuMsDisplay, + row.TotalLogicalReadsDisplay, + row.TotalLogicalWritesDisplay, + row.TotalPhysicalReadsDisplay, + row.TotalMemoryMbDisplay, + row.MinDop, + row.MaxDop, + row.LastExecutionLocal, + row.ExecutionTypeDesc); + private void Close_Click(object? sender, RoutedEventArgs e) { // When in a detached window, close it (this destroys the history view) diff --git a/src/PlanViewer.App/Dialogs/SettingsWindow.axaml.cs b/src/PlanViewer.App/Dialogs/SettingsWindow.axaml.cs index 36eefa4..9aaa24b 100644 --- a/src/PlanViewer.App/Dialogs/SettingsWindow.axaml.cs +++ b/src/PlanViewer.App/Dialogs/SettingsWindow.axaml.cs @@ -332,6 +332,9 @@ private Control BuildScriptOptionsSection() FontSize = 13, }; + Helpers.DataGridBehaviors.AttachCopyGuard(_formatGrid, + item => item is FormatOptionRow row ? $"{row.Name}\t{row.CurrentValue}" : null); + _formatGrid.Columns.Add(new DataGridTextColumn { Header = "Setting", diff --git a/src/PlanViewer.App/Helpers/DataGridBehaviors.cs b/src/PlanViewer.App/Helpers/DataGridBehaviors.cs index 65c2bfa..cd3d9a8 100644 --- a/src/PlanViewer.App/Helpers/DataGridBehaviors.cs +++ b/src/PlanViewer.App/Helpers/DataGridBehaviors.cs @@ -1,15 +1,17 @@ using System; +using System.Collections.Generic; using Avalonia; using Avalonia.Controls; using Avalonia.Controls.Primitives; using Avalonia.Input; using Avalonia.Interactivity; using Avalonia.VisualTree; +using PlanViewer.App.Services; namespace PlanViewer.App.Helpers; /// -/// Attaches middle-mouse-button pan behavior to a DataGrid. +/// Attached input behaviors for DataGrids: middle-mouse pan and guarded clipboard copy. /// public static class DataGridBehaviors { @@ -19,6 +21,52 @@ public static void Attach(DataGrid grid) AttachMiddleClickPan(grid); } + /// + /// Replaces the DataGrid's built-in Ctrl+C / Ctrl+Insert copy, which awaits the + /// clipboard unguarded inside Avalonia and can crash the app when the clipboard + /// is locked by another process (issue #415). The tunnel handler runs before the + /// grid's own key handling and routes the copy through ClipboardHelper instead. + /// turns one selected row item into its clipboard + /// line; return null to skip the item. + /// + public static void AttachCopyGuard(DataGrid grid, Func formatRow) + { + grid.AddHandler(InputElement.KeyDownEvent, (_, e) => + { + // Mirror the built-in copy's exact gate: platform command modifier + // (Ctrl; Cmd on macOS), no Shift, no Alt. + var commandModifiers = TopLevel.GetTopLevel(grid)?.PlatformSettings?.HotkeyConfiguration.CommandModifiers + ?? KeyModifiers.Control; + if (e.Key is not (Key.C or Key.Insert) + || !e.KeyModifiers.HasFlag(commandModifiers) + || e.KeyModifiers.HasFlag(KeyModifiers.Shift) + || e.KeyModifiers.HasFlag(KeyModifiers.Alt)) + return; + + // In-cell editors own their Ctrl+C (e.g. the settings grid's TextBoxes). + if (e.Source is Visual source && source.FindAncestorOfType(includeSelf: true) != null) + return; + + var selected = grid.SelectedItems; + if (selected == null || selected.Count == 0) + return; + + // Take over whenever rows are selected so the DataGrid's unguarded + // built-in copy never runs, even if no line ends up on the clipboard. + e.Handled = true; + + var lines = new List(); + foreach (var item in selected) + { + if (item != null && formatRow(item) is { } line) + lines.Add(line); + } + + if (lines.Count > 0) + _ = ClipboardHelper.TrySetTextAsync(grid, string.Join(Environment.NewLine, lines)); + }, RoutingStrategies.Tunnel); + } + // ───────────────────────────────────────────────────────────────────────── // Middle-mouse-button drag → pan (scroll) the grid // ───────────────────────────────────────────────────────────────────────── diff --git a/src/PlanViewer.App/MainWindow.FileOps.cs b/src/PlanViewer.App/MainWindow.FileOps.cs index 2abefe3..2a8d397 100644 --- a/src/PlanViewer.App/MainWindow.FileOps.cs +++ b/src/PlanViewer.App/MainWindow.FileOps.cs @@ -9,7 +9,6 @@ using Avalonia; using Avalonia.Controls; using Avalonia.Input; -using Avalonia.Input.Platform; using Avalonia.Interactivity; using Avalonia.Layout; using Avalonia.Media; @@ -229,13 +228,10 @@ private void LoadPlanFile(string filePath) private async Task PasteXmlAsync() { - var clipboard = this.Clipboard; - if (clipboard == null) return; - - var xml = await clipboard.TryGetTextAsync(); + var xml = await ClipboardHelper.TryGetTextAsync(this); if (string.IsNullOrWhiteSpace(xml)) { - ShowError("The clipboard does not contain any text."); + ShowError("Could not read any text from the clipboard."); return; } diff --git a/src/PlanViewer.App/MainWindow.PlanViewer.cs b/src/PlanViewer.App/MainWindow.PlanViewer.cs index dc44089..b1ab639 100644 --- a/src/PlanViewer.App/MainWindow.PlanViewer.cs +++ b/src/PlanViewer.App/MainWindow.PlanViewer.cs @@ -115,11 +115,11 @@ private DockPanel CreatePlanTabContent(PlanViewerControl viewer) queryText, database, planXml, isolationLevel: null, source: "Performance Studio"); - var clipboard = this.Clipboard; - if (clipboard != null) - { - await clipboard.SetTextAsync(reproScript); - } + copyReproBtn.Content = await ClipboardHelper.TrySetTextAsync(this, reproScript) + ? "\U0001f4cb Copied!" + : "\U0001f4cb Clipboard busy"; + await Task.Delay(1500); + copyReproBtn.Content = "\U0001f4cb Copy Repro"; }; copyReproBtn.Click += async (_, _) => await copyRepro(); diff --git a/src/PlanViewer.App/MainWindow.Tabs.cs b/src/PlanViewer.App/MainWindow.Tabs.cs index a6299e7..3c642e0 100644 --- a/src/PlanViewer.App/MainWindow.Tabs.cs +++ b/src/PlanViewer.App/MainWindow.Tabs.cs @@ -142,7 +142,7 @@ private void TabContextMenu_Click(object? sender, RoutedEventArgs e) { var path = GetTabFilePath(pathTab); if (path != null) - _ = this.Clipboard?.SetTextAsync(path); + _ = ClipboardHelper.TrySetTextAsync(this, path); } break; diff --git a/src/PlanViewer.App/Program.cs b/src/PlanViewer.App/Program.cs index 8d2f1c7..3d750fc 100644 --- a/src/PlanViewer.App/Program.cs +++ b/src/PlanViewer.App/Program.cs @@ -2,6 +2,7 @@ using System; using System.IO; using System.IO.Pipes; +using System.Threading.Tasks; using PlanViewer.App.Services; using Velopack; @@ -14,6 +15,17 @@ class Program [STAThread] public static void Main(string[] args) { + // Last-resort diagnostics: issue #415 was a crash-to-desktop that left + // nothing on disk. These can't stop a dispatcher-thread crash, but they + // leave a stack trace in the crash log. + AppDomain.CurrentDomain.UnhandledException += (_, e) => + CrashLogger.Write("AppDomain.UnhandledException", e.ExceptionObject as Exception); + TaskScheduler.UnobservedTaskException += (_, e) => + { + CrashLogger.Write("TaskScheduler.UnobservedTaskException", e.Exception); + e.SetObserved(); + }; + var velopack = VelopackApp.Build(); if (OperatingSystem.IsWindows()) { diff --git a/src/PlanViewer.App/Services/AdviceWindowHelper.cs b/src/PlanViewer.App/Services/AdviceWindowHelper.cs index 372fd15..9ce37d1 100644 --- a/src/PlanViewer.App/Services/AdviceWindowHelper.cs +++ b/src/PlanViewer.App/Services/AdviceWindowHelper.cs @@ -111,14 +111,11 @@ public static void Show( copyBtn.Click += async (_, _) => { - var clipboard = window.Clipboard; - if (clipboard != null) - { - await clipboard.SetTextAsync(content); - copyBtn.Content = "Copied!"; - await Task.Delay(1500); - copyBtn.Content = "Copy to Clipboard"; - } + copyBtn.Content = await ClipboardHelper.TrySetTextAsync(window, content) + ? "Copied!" + : "Clipboard busy - try again"; + await Task.Delay(1500); + copyBtn.Content = "Copy to Clipboard"; }; closeBtn.Click += (_, _) => window.Close(); diff --git a/src/PlanViewer.App/Services/ClipboardHelper.cs b/src/PlanViewer.App/Services/ClipboardHelper.cs new file mode 100644 index 0000000..f3016a8 --- /dev/null +++ b/src/PlanViewer.App/Services/ClipboardHelper.cs @@ -0,0 +1,79 @@ +using System; +using System.Threading.Tasks; +using Avalonia; +using Avalonia.Controls; +using Avalonia.Input.Platform; + +namespace PlanViewer.App.Services; + +/// +/// Clipboard access that never throws. The Windows clipboard is a shared resource: +/// clipboard managers, RDP, and Office clipboard sync briefly lock it, and OpenClipboard +/// then fails with CLIPBRD_E_CANT_OPEN. An unguarded await on SetTextAsync inside an +/// async void handler turns that transient failure into an app crash (issue #415), +/// so all clipboard reads and writes go through here. +/// +internal static class ClipboardHelper +{ + private const int MaxAttempts = 3; + private const int RetryDelayMs = 100; + + /// + /// Writes text to the clipboard, retrying briefly if it is locked by another + /// process. Returns false instead of throwing when all attempts fail. + /// + public static async Task TrySetTextAsync(Visual source, string text) + { + var clipboard = TopLevel.GetTopLevel(source)?.Clipboard; + if (clipboard == null) + return false; + + for (var attempt = 1; attempt <= MaxAttempts; attempt++) + { + try + { + await clipboard.SetTextAsync(text); + return true; + } + catch (Exception) when (attempt < MaxAttempts) + { + await Task.Delay(RetryDelayMs); + } + catch (Exception) + { + return false; + } + } + + return false; + } + + /// + /// Reads text from the clipboard, retrying briefly if it is locked by another + /// process. Returns null when no text is available or all attempts fail. + /// + public static async Task TryGetTextAsync(Visual source) + { + var clipboard = TopLevel.GetTopLevel(source)?.Clipboard; + if (clipboard == null) + return null; + + for (var attempt = 1; attempt <= MaxAttempts; attempt++) + { + try + { + return await clipboard.TryGetTextAsync(); + } + catch (Exception) when (attempt < MaxAttempts) + { + await Task.Delay(RetryDelayMs); + } + catch (Exception) + { + return null; + } + } + + return null; + } +} diff --git a/src/PlanViewer.App/Services/CrashLogger.cs b/src/PlanViewer.App/Services/CrashLogger.cs new file mode 100644 index 0000000..379029b --- /dev/null +++ b/src/PlanViewer.App/Services/CrashLogger.cs @@ -0,0 +1,43 @@ +using System; +using System.IO; + +namespace PlanViewer.App.Services; + +/// +/// Last-resort exception logging to the app's local data directory. +/// Added for crash reports like issue #415, where the app died with nothing +/// on disk and the reporter had to pull Event Viewer data themselves. +/// Must never throw: it runs inside exception handlers. +/// +internal static class CrashLogger +{ + private const long MaxLogBytes = 1_000_000; + private static readonly object Sync = new(); + + private static readonly string LogPath = Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), + "PerformanceStudio", "crash.log"); + + public static void Write(string source, Exception? exception) + { + try + { + lock (Sync) + { + Directory.CreateDirectory(Path.GetDirectoryName(LogPath)!); + + var file = new FileInfo(LogPath); + if (file.Exists && file.Length > MaxLogBytes) + file.Delete(); + + File.AppendAllText( + LogPath, + $"[{DateTime.Now:yyyy-MM-dd HH:mm:ss.fff}] {source}{Environment.NewLine}{exception}{Environment.NewLine}{Environment.NewLine}"); + } + } + catch + { + // Swallow everything: logging a crash must not cause one. + } + } +} diff --git a/src/PlanViewer.App/Services/TextBoxClipboardGuard.cs b/src/PlanViewer.App/Services/TextBoxClipboardGuard.cs new file mode 100644 index 0000000..bc102d0 --- /dev/null +++ b/src/PlanViewer.App/Services/TextBoxClipboardGuard.cs @@ -0,0 +1,64 @@ +using Avalonia.Controls; +using Avalonia.Interactivity; + +namespace PlanViewer.App.Services; + +/// +/// App-wide interception of TextBox clipboard operations. Avalonia's built-in +/// TextBox.Copy()/Cut() are async void with an unguarded clipboard await, and +/// Paste() catches only TimeoutException, so a locked Windows clipboard +/// (CLIPBRD_E_CANT_OPEN) crashes the app (issue #415). TextBox raises these +/// routed events before touching the clipboard and skips its internal path when +/// they are handled, so one class handler per event covers every TextBox in the +/// app, including ones created later. +/// +internal static class TextBoxClipboardGuard +{ + public static void Register() + { + TextBox.CopyingToClipboardEvent.AddClassHandler(OnCopying); + TextBox.CuttingToClipboardEvent.AddClassHandler(OnCutting); + TextBox.PastingFromClipboardEvent.AddClassHandler(OnPasting); + } + + // CanCopy/CanCut/CanPaste mirror the built-in gates (password box, read-only, + // empty selection); when they are false the built-in path is a no-op that never + // touches the clipboard, so leaving the event unhandled there is safe. + + private static void OnCopying(TextBox textBox, RoutedEventArgs e) + { + if (!textBox.CanCopy) + return; + + e.Handled = true; + _ = ClipboardHelper.TrySetTextAsync(textBox, textBox.SelectedText); + } + + private static async void OnCutting(TextBox textBox, RoutedEventArgs e) + { + if (!textBox.CanCut) + return; + + e.Handled = true; + // Handling the event suppresses the built-in DeleteSelection, so clear the + // selection here - and only once the text actually reached the clipboard. + if (await ClipboardHelper.TrySetTextAsync(textBox, textBox.SelectedText)) + textBox.SelectedText = ""; + } + + private static async void OnPasting(TextBox textBox, RoutedEventArgs e) + { + if (!textBox.CanPaste) + return; + + e.Handled = true; + var text = await ClipboardHelper.TryGetTextAsync(textBox); + if (string.IsNullOrEmpty(text)) + return; + + if (!textBox.AcceptsReturn) + text = text.Replace("\r", "").Replace("\n", ""); + + textBox.SelectedText = text; + } +} From 5260445ba257b88fc7bbb6dc60ffcc66a7f389e9 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:31:23 -0400 Subject: [PATCH 3/3] Bump version to 1.19.1 Co-Authored-By: Claude Fable 5 --- CITATION.cff | 4 ++-- src/Directory.Build.props | 2 +- src/PlanViewer.Ssms/Properties/AssemblyInfo.cs | 4 ++-- src/PlanViewer.Ssms/source.extension.vsixmanifest | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CITATION.cff b/CITATION.cff index 80b26f2..137f805 100644 --- a/CITATION.cff +++ b/CITATION.cff @@ -9,8 +9,8 @@ authors: website: "https://erikdarling.com" repository-code: "https://github.com/erikdarlingdata/PerformanceStudio" license: MIT -version: "1.19.0" -date-released: "2026-07-25" +version: "1.19.1" +date-released: "2026-07-29" keywords: - sql-server - execution-plan diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 14c7ad3..4637066 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -15,7 +15,7 @@ Tests and server/ projects are outside src/ and are unaffected. --> - 1.19.0 + 1.19.1 Erik Darling Darling Data LLC Performance Studio diff --git a/src/PlanViewer.Ssms/Properties/AssemblyInfo.cs b/src/PlanViewer.Ssms/Properties/AssemblyInfo.cs index e96b5e8..a0682f2 100644 --- a/src/PlanViewer.Ssms/Properties/AssemblyInfo.cs +++ b/src/PlanViewer.Ssms/Properties/AssemblyInfo.cs @@ -7,5 +7,5 @@ [assembly: AssemblyProduct("Performance Studio for SSMS")] [assembly: AssemblyCopyright("Copyright Darling Data 2026")] [assembly: ComVisible(false)] -[assembly: AssemblyVersion("1.19.0.0")] -[assembly: AssemblyFileVersion("1.19.0.0")] +[assembly: AssemblyVersion("1.19.1.0")] +[assembly: AssemblyFileVersion("1.19.1.0")] diff --git a/src/PlanViewer.Ssms/source.extension.vsixmanifest b/src/PlanViewer.Ssms/source.extension.vsixmanifest index 0cba24e..b1eae0a 100644 --- a/src/PlanViewer.Ssms/source.extension.vsixmanifest +++ b/src/PlanViewer.Ssms/source.extension.vsixmanifest @@ -3,7 +3,7 @@ xmlns:d="http://schemas.microsoft.com/developer/vsx-schema-design/2011"> Performance Studio for SSMS