Import sibling SharedOutputFiles folder during log import

[JosephPilov-MSFT (PiJoCoder)]

#556 - Import files from SQL LogScout's sibling "SharedOutputFiles" folder

WHAT & WHY

Adds support for SQL LogScout "All Instances" captures where non-instance-specific (host/OS) logs live in a sibling \SharedOutputFiles folder next to the pointed instance folder. When that sibling exists, SQL Nexus now searches both the primary folder and the sibling; when it does not exist, behavior is unchanged.

SQL LogScout's "All Instances" capture splits output: instance-specific files go in an instance subfolder (e.g. output\SERVER_SQL2019), while non-instance host/OS diagnostics (running drivers, disk info, event logs, some .blg/.perf) go in a sibling folder named "SharedOutputFiles". Previously SQL Nexus only imported the folder the user selected, so the host/OS data was silently missed.

This PR makes SQL Nexus also scan the sibling SharedOutputFiles folder (when it exists as a direct sibling of the selected folder), with guards so files present in both folders are never imported twice.

BEHAVIOR

• If a sibling "SharedOutputFiles" folder exists, both it and the selected folder
  are searched; if it does not exist, behavior is unchanged (single folder).
• Per-file importers (Rowset .OUT/.TXT, ErrorLog, RawFile): a sibling file whose
  name matches one already selected from the primary folder is skipped (primary
  wins). Same name + same size -> quiet skip; same name + DIFFERENT size -> one
  aggregated warning (a copy was not imported).
• Mask-based/aggregating importers (Perfmon .blg, ReadTrace): the sibling is a
  FALLBACK - scanned only when the primary matched nothing, so table setup is not
  re-run over already-imported data. If the primary matched, an informational
  (non-modal) note explains the sibling copies were not also imported.
• Custom XEL (SQLDiag / AlwaysOn_health / system_health) still reads the primary
  folder only; sibling-ONLY Custom XEL files are surfaced as a "move these" note
  (they are never auto-imported because the load uses SELECT * INTO + DROP TABLE).
• LinuxPerfImporter's working directory is repointed per row and restored after.

UI / ACCESSIBILITY

• Rows sourced from the sibling are labeled " (from SharedOutputFiles)".
• A muted info banner ("Also scanning: ") docks under the path box
  when a sibling exists; full path in tooltip, AccessibleName, and log. It updates
  in lockstep with the path box and no longer pops a modal dialog on form open -
  the loud one-time announcement fires only on a user-driven path change.
• Import rows expose AccessibleName on the progress bar and status label.
• Informational "nothing to import" notices are log + status bar, not modal.

SIGNING (enabling test coverage)

To let the unit test project reach internal members without widening them to
public plugin API, sqlnexus.exe and SqlNexus.McpServer.exe are now strong-named
with the existing shared SqlNexus.snk, the test project is strong-named with the
same key, and the InternalsVisibleTo grants (sqlnexus, ErrorLogImporter,
SqlNexus.McpServer) use the key-qualified form. Plugin loading is by path
(Assembly.LoadFile), so it is unaffected; Authenticode release signing is
orthogonal and unaffected. Re-enabling the sqlnexus tests in the CLI runner also
surfaced a latent IsDbNameValid(null/"") crash, now fixed (fail closed).

RESILIENCE / ERRORS

• Each per-directory Directory.GetFiles is wrapped (UnauthorizedAccessException |
  IOException | PathTooLongException): a locked/too-long sibling is logged and the
  scan continues on the primary folder.
• A missing PRIMARY source folder is logged (was silently "no files"); a missing
  sibling stays silent (normal).

TESTS

• SharedOutputFolder is WinForms-free and unit-tested: ~40 tests covering path
  resolution (happy path, no-sibling, null/empty/whitespace, trailing separator,
  quoted input, drive root, UNC, case-insensitive folder name, .. normalization),
  duplicate classification (same/different/unknown size), sibling-only detection,
  the mask-skip rule, and the row target-path / display-text composition helpers.
• Full suite: 298 passing (net48).

DOCS

• Root README.md and in-app Help note the SharedOutputFiles behavior and the
  "(from SharedOutputFiles)" suffix.
• TestingInfrastructure/README.md updated for the 5 project references and the
  key-qualified InternalsVisibleTo requirement.

OUT OF SCOPE / HYGIENE

• Deleted sqlnexus/fmnexus.cs.2 (an untracked duplicate of an old file).

[JamesFerebee-MSFT (JamesFerebee)]

PR #558 — Change Checklist

PR: Import sibling SharedOutputFiles folder during log import · Issue: #556 · Commit reviewed: ed01cf7

Nice work — the design is right and the sibling-absent case is a true no-op. Below is everything I'd like changed, shortest-path first.

---

Must fix before merge

• 1. Delete the dead sibling guard — sqlnexus/SharedOutputFolder.cs
isDirectSibling can never be false (parent is already normalized and SharedFolderName has no separators), and the comment claiming it "rejects traversal or symlink-style tricks" is wrong — Path.GetFullPath collapses .. first, and Directory.Exists follows junctions. Remove the block and the comment, or add a real reparse-point check.

• 2. Fix LinuxPerfImporter reading the wrong folder — sqlnexus/fmImport.cs
ConfigValues.WorkingDirectory is still set to srcPath only, and LinuxPerfImporter uses that instead of the path passed to Initialize. A *.perf file found in SharedOutputFiles gets a row that reports success while importing nothing.

• 3. Log the shared folder — sqlnexus/fmImport.cs
  Nothing in sqlnexus.log records that a second folder was searched; provenance exists only as a GUI suffix. Add a line naming the resolved sibling plus per-folder file counts.

• 4. Fix the duplicated "files blocked" log line — sqlnexus/fmImport.cs
"Number of files blocked for import..." now fires once per directory per mask instead of once per mask, so the log shows two competing numbers.

• 5. Reconsider the signing change — SqlNexus.UnitTests.csproj, ErrorLogImporter
  The csproj comment says a strong-named test assembly can't load an unsigned reference — that's CS8002, a warning, already in <NoWarn>. The real blocker was that sqlnexus's InternalsVisibleTo has no PublicKey. Restore SignAssembly=true and add [assembly: InternalsVisibleTo("SqlNexus.UnitTests, PublicKey=0024...")] to sqlnexus/Properties/AssemblyInfo.cs — then ErrorLogImporter's 6 members go back to internal instead of becoming permanent public plugin API.

---

Please confirm intent (may be by design)

• 6. Sibling is scanned with every importer mask — *.trc, *.xel, *.blg, *.out, *.txt, *.perf. #556 only asks for host/OS files. For mask-based importers this triggers a second Initialize+DoImport; for ReadTrace that can re-run table setup over the first run's results.

• 7. Duplicate file names aren't deduped — if the same name exists in both folders (e.g. SQLDIAG*.OUT), both rows import into the same tables. Silent duplicate rows. Warn or skip.

• 8. Two call sites weren't extended — CustomXELImporter.ImportCustomXELFiles(...) and the XEL/TRC conflict pre-check at the top of EnumFiles() still see only the primary folder.

---

Code quality

• 9. Log the swallowed exceptions — three catch (Exception) blocks in SharedOutputFolder.cs return silently. Use catch (Exception ex) + Util.Logger.LogMessage(..., MessageOptions.Silent) per the repo's exception-handling rules.

• 10. ResolveSharedSibling can throw — it's public, its parameter is named normalizedPrimary, but the two NormalizePath calls inside the isDirectSibling expression sit outside any try. Make it private or guard them.

• 11. Fix three misleading comments — the traversal/symlink claim (#1), the "no injection surface" note sitting beside dead validation code, and the CS8002 justification (#5).

• 12. Small cleanups — AddFileRow is now a one-line pass-through to AddFileRowReturningLabel; if (includedFiles.Length > 0) is unreachable-false. Also, deleting sqlnexus/fmnexus.cs.2 is good hygiene but is unrelated scope creep in a feature PR.

---

Accessibility / UX

• 13. Set AccessibleName on the new row controls — AddFileRowReturningLabel sets none on the progress bar or status label. Two rows can now differ only by a suffix, so progress bars are ambiguous to a screen reader. pb.AccessibleName = labelText; and lab2.AccessibleName = labelText + " status";

• 14. Rename the label suffix — " (SharedOutput)" → " (from SharedOutputFiles)", derived from SharedOutputFolder.SharedFolderName rather than a separate literal. Reads better aloud and matches the real folder name.

• 15. Add a form-level affordance — the path combo box still shows only the instance folder, so a second scanned folder is only inferable from per-row suffixes. A status label ("Also scanning ..\SharedOutputFiles") makes it discoverable.

---

Tests

• 16. Cover the risky half of the change. SharedOutputFolderTests is solid, but the part that changed behavior for every import row — m_RowTargetPaths and the display-suffix composition — has zero coverage, as do AddFilesFromDirectory and RawFileImporter. Extracting two pure helpers (basePath + name/mask → target path; name + isShared → display text) makes them testable without WinForms.

• 17. Add missing edge cases — UNC paths (\\server\share\output\Instance1), quoted input (GetImportSearchPaths strips quotes internally but that's untested), and case-insensitive folder name.

---

Docs

• 18. Update TestingInfrastructure/README.md — it's now factually wrong: it lists 2 project references (there are 4) and tells contributors to use InternalsVisibleTo("SqlNexus.UnitTests"), which can no longer work for any strong-named product assembly while the test project is unsigned.

• 19. Document the user-visible behavior change — Nexus now reads files from outside the folder the user selected. Worth a note in the Getting Started wiki / Help/*.htm so users aren't surprised by rows for files they didn't point at.

---

Verified clean — no action needed

• No compile errors on any changed file.
• No security regressions: folder name is a compile-time constant, nothing is concatenated into SQL, RawFileImporter still uses SqlParameter + IsSafeSqlIdentifier.
• GUI rows are consistent with existing rows (same label/progress-bar/status triple, same Tag, same progress updates).
• The IsDbNameValid null/empty fix is already covered by existing null/""/" " rows in DatabaseCommandTextTests.cs.

[JosephPilov-MSFT (PiJoCoder)]

Added new logging:

2026-09-09T10:20:47.6434280-05:00	Information: 0 : SQL Nexus connection to server: '.', database: 'sqlnexus_allInstances_SQL2019'
2026-09-09T10:20:47.9619109-05:00	Information: 0 : Shared output folder detected. Import will search two folders: primary 'D:\SQLLogScout\output\DESKTOP-COMP_SQL2019' (25 file(s)) and shared 'D:\SQLLogScout\output\SharedOutputFiles' (22 file(s)).

[JamesFerebee-MSFT (JamesFerebee)]

Review — PR #558 · Import sibling SharedOutputFiles folder during log import

What this change does (plain English)

SQL LogScout's "All Instances" capture splits its output into two folders: one per SQL instance
(e.g. output\SERVER_SQL2019) and one shared folder next to it (output\SharedOutputFiles) that
holds machine-wide files such as driver lists and disk info. Today, if you point SQL Nexus at the
instance folder, everything in the shared folder is ignored.

This PR makes SQL Nexus look in both folders. If the shared folder isn't there, nothing changes.
Rows that came from the shared folder are labelled (from SharedOutputFiles), a yellow
"Also scanning: …" note appears under the path box, and a file that exists in both folders is
imported only once (the instance folder wins).

It also fixes the assembly-signing setup so the unit test project can actually run against the main
app again — which immediately exposed a real crash bug in Program.IsDbNameValid (now fixed).

Overall: solid, well-commented work with a genuinely good new test file. The blocking issues are
a high-DPI layout break, an unguarded folder scan that can crash the import, and documentation that
lands in a file no user will ever see.

---

Checklist

Legend: 🔴 Must fix · 🟡 Should fix · ⚪ Nit

1. Are there bugs?

• 🔴 High-DPI layout break — the new banner covers the Import/Close buttons.
  ShowSharedFolderLabel grows the header by a hard-coded 22 pixels, but the form uses
  AutoScaleMode.Font. At 125%/150% display scaling the label grows and the 22 doesn't, so the
  banner sits on top of the Import and Close buttons.
  Fix: grow by the label's runtime Height/PreferredHeight instead of the constant — better,
  dock the label to the bottom of paTop (or use an auto-sizing layout row) so no manual resize is
  needed at all. → sqlnexus/fmImport.cs, sqlnexus/fmImport.Designer.cs

• 🔴 An unreadable shared folder crashes the whole import scan.
  AddFilesFromDirectory calls Directory.GetFiles(basePath, Mask) with no try/catch, and its
  caller (AddFiles, invoked in the importer loop) isn't wrapped either. If SharedOutputFiles has
  restrictive permissions or a too-long path, an UnauthorizedAccessException / PathTooLongException
  takes down the entire enumeration — for a folder the user never selected. Same gap in
  RawFileImporter.DoImport.
  Fix: wrap each per-directory enumeration in try/catch (UnauthorizedAccessException | IOException | PathTooLongException), log it with MessageOptions.All, and carry on with the primary folder.

• 🟡 LinuxPerfImporter working directory is changed and never put back.
  ConfigValues.WorkingDirectory is a static global. When a Linux-perf row comes from the shared
  folder it's repointed there and left that way for everything that runs afterwards.
  Fix: save the previous value and restore it in a finally after the row completes.

• 🟡 Data can be dropped where the user can't see it.
  For mask-based importers (Perfmon *.blg, ReadTrace) the shared folder is skipped whenever the
  instance folder already matched — logged only with MessageOptions.Silent. If a customer really
  has .blg files in both folders, half the data silently disappears.
  Fix: log that skip with MessageOptions.All and name the folder that was skipped.

• ⚪ A typo'd source path now fails quietly. if (!Directory.Exists(basePath)) return false;
  turns a bad path into "no files found" instead of an error.
  Fix: log a warning when the primary folder doesn't exist.

• ⚪ Header keeps 22px of dead space. When the form expands into the import view, the flag is
  reset and the label hidden, but paTop.Height/ClientSize are never shrunk back.
  Fix: reverse the resize, or (better) remove the manual resize entirely per the first item.

• ⚪ UI work on every keystroke. tbPath_TextChanged now also runs Path.GetFullPath +
  a second Directory.Exists per character typed. On an unreachable UNC path this freezes the dialog.
  Fix: only recompute when the text is a rooted path, or debounce.

2. Is logging adequate?

• Good: a clear provenance line at import start ("searching two folders… N files each"), per-file
  skip reasons, and every new catch logs. Nothing is swallowed silently.
• 🟡 The most important warning is the least visible. The "same name, DIFFERENT size" case is
  logged with MessageOptions.Both = status bar + log file. Status-bar text is overwritten
  immediately, so the user will never read it.
  Fix: aggregate these into one MessageOptions.All (dialog) message with a count and the file
  names, instead of one status-bar line per file.
• ⚪ New log noise. "Number of files blocked for import…: 0" is now written once per mask for
  every file importer, even when nothing matched (previously only when files were found).
  Fix: only log when blockedCounter > 0.
• ⚪ Privacy tidy-up. The loud messages embed full folder paths (which contain host/server
  names). Consistent with existing behaviour, but per copilot-instructions.md prefer file names
  • counts in All-level messages and keep full paths at Silent.

3. Accessibility concerns

• Good: pb.AccessibleName and lab2.AccessibleName were added to the progress bar and status
  label of each import row. Previously a screen reader announced every progress bar identically —
  this is a real improvement, and necessary now that two rows can differ only by a suffix.
• Good: colour isn't the only signal — the words "Also scanning:" carry the meaning.
• 🟡 A screen reader user is never told the second folder was added. laSharedFolder is a
  plain Label: not focusable, no live-region semantics. Typing a path silently changes it.
  Fix: mirror the text into cbPath.AccessibleDescription (that control is focusable), or raise a
  UIA notification when the banner appears.
• 🟡 The full path is mouse-only. It lives in a tooltip. Screen readers get it via
  AccessibleName, but a sighted keyboard-only user sees just the abbreviated text (WCAG 2.1.1).
  Fix: also surface the full path somewhere keyboard-reachable (log line at All, or widen/wrap
  the label).
• ⚪ Stale accessible name. When the sibling disappears, Text and the tooltip are cleared but
  AccessibleName keeps the old path.
  Fix: clear AccessibleName (and set AccessibleDescription) in the else branch too.

4. Are errors raised appropriately?

• Good: IsDbNameValid now fails closed on null/empty/whitespace instead of throwing —
  correct direction. Path normalisation also fails closed and logs why.
• 🔴 See the unguarded Directory.GetFiles item under Bugs — that's the one real gap.
• ⚪ Bare catch { } in AbbreviatePath. It does return a sensible fallback, but the repo
  convention is catch (Exception ex) plus a Silent log.
  Fix: name the exception and log it.
• ⚪ MainForm isn't null-checked inside UpdateSharedFolderAffordance's catch, so an error
  on the parameterless-constructor path would throw out of the error handler.
  Fix: MainForm?.LogMessage(...).

5. If tests are implemented, are they sufficient?

• Good: SharedOutputFolderTests.cs — ~30 tests, temp-folder fixtures, proper cleanup, and it
  covers happy path, no-sibling (unchanged behaviour), null/empty/whitespace, trailing separator,
  quoted input, drive root, UNC, case-insensitive folder names, a file named SharedOutputFiles,
  .. segments, same/different/unknown-size duplicates, and the label/path composition helpers.
  This is the right level of rigour.
• 🟡 The code that actually changed behaviour has no tests. All the new tests target the pure
  helpers; none cover the orchestration:
  RawFileImporter.DoImport's two-folder loop, the "skip the sibling when the primary already
  matched" rule for mask importers, and the m_RowTargetPaths fallback path.
  Fix: extract those decisions into small pure helpers (exactly the pattern this PR already used
  successfully for SharedOutputFolder) and unit-test them.
• ⚪ One test documents behaviour the PR description denies.
  GetImportSearchPaths_PrimaryContainsDotDotSegment_NormalizesAndResolvesSibling asserts that ..
  is normalised and accepted, and its comment notes the direct-sibling guard was removed. The
  description still claims traversal is rejected. Pick one and make them agree.

6. Syntax errors / comments needed?

• No compile errors in any changed file.
• ⚪ Stale comment: the doc block above AddFilesFromDirectory says the suffix is
  "(SharedOutput)"; the real suffix is " (from SharedOutputFiles)".
• ⚪ Stale marker: the closing brace of AddFilesFromDirectory still says //end of AddFiles.
• ⚪ Dead wrapper: AddFileRow is now a one-liner around AddFileRowReturningLabel.
  Fix: call the new method directly and delete the wrapper.
• ⚪ Encoding: new lines in TestingInfrastructure/README.md use a raw Windows-1252 em dash
  (byte 0x97), which GitHub renders as �. Fix: use -- or a proper UTF-8 em dash.

7. README / docs changes needed?

• 🔴 The PR description no longer matches the code. Three statements are now the opposite of
  what shipped:

  Description says	Code actually does
  "validates the sibling is a real direct sibling … rejects traversal"	the direct-sibling guard was removed; .. is normalised and accepted
  "Mask-based importers … get a second row pointing at the sibling"	the sibling row is skipped when the primary already matched
  "SqlNexus.UnitTests: SignAssembly=false" / "ErrorLogImporter: remove key-qualified InternalsVisibleTo and widen 6 members to public"	test project stays signed; the key-qualified InternalsVisibleTo was added; the members stay internal
  Fix: rewrite the description to describe the final commits.

• 🟡 Root README.md not updated. It's the front door and already points at SQL LogScout.
  Fix: add two lines explaining the SharedOutputFiles behaviour and the (from SharedOutputFiles)
  row suffix, so users can decode what they see in the UI.

• 🟡 Confirm the strong-naming change is intended and safe.
  sqlnexus.csproj flips SignAssembly false → true for the shipping EXE, and
  SqlNexus.McpServer.csproj gains strong naming. That's a packaging change well beyond "import a
  sibling folder". The csproj comment claims "a strong-named test assembly cannot load an unsigned
  reference at runtime (FileLoadException)" — that isn't generally true on .NET Framework (CS8002 is
  only a warning, and it's already in NoWarn).
  Fix: confirm the EXE genuinely needed signing rather than just the key-qualified
  InternalsVisibleTo, and confirm the installer / release-signing pipeline is unaffected. If it
  wasn't required, revert it to keep this PR focused.

9. Do we have proper test suite coverage?

• Big win, and worth calling out: fixing the signing/InternalsVisibleTo setup re-enabled the
  existing sqlnexus tests in the command-line runner. That's what surfaced the IsDbNameValid
  crash — the null / "" / " " cases in DatabaseCommandTextTests.cs had existed for a while but
  had never actually executed. Good catch.
• 🟡 Coverage gap on the orchestration code — see item 5.
• ⚪ "170/170 passing" couldn't be independently verified here — dotnet test can't restore in
  this environment (Microsoft.SqlServer.XEvent.XELite returns HTTP 402 from the private feed, a
  known long-standing issue unrelated to this PR).
  Fix: attach the test-run output or a CI link to the PR.

10. Is the GUI updating consistently with other elements in the same section?

• Good: the banner is wired to tbPath_TextChanged — the same handler that enables the Import
  button — so it updates in lockstep with the rest of the path section.
• 🟡 It doesn't follow the app's theming, unlike its neighbours. Every other control in
  paTop (laPath, laInstructions, llOptions, cbPath) is painted by ThemeManager.ApplyTheme.
  The new label hard-codes SystemColors.Info / InfoText and deliberately re-applies them over
  the theme on every path change. In the Aquatic dark theme (#202020) this will read as a
  foreign pale-yellow strip; same mismatch in Desert.
  Fix: take the colours from ThemeManager for the three themes, and bypass to SystemColors only
  when SystemInformation.HighContrast is on — matching ThemeManager.ApplyHighContrastTheme().
• 🔴 It also doesn't follow the panel's layout model. Everything else in paTop is anchored;
  this label is absolutely positioned at (0, 63) where it overlaps the Options link and the
  Import/Close buttons, and only ends up in the right place thanks to the runtime +22px fudge —
  which is the high-DPI bug in item 1.
  Fix: dock it to the bottom of paTop (or let paTop auto-size) so it flows like everything else.

---

Suggested merge order

1. 🔴 High-DPI layout / dock the banner properly (items 1 & 10)
2. 🔴 Guard the folder enumeration against permission/path errors (item 1)
3. 🔴 Put the user documentation somewhere users can actually read it (item 7)
4. 🔴 Refresh the PR description to match the code (item 7)
5. 🟡 Everything else above
6. ⚪ Nits — fine to batch into a follow-up

Also worth a separate mention in the description: this branch deletes sqlnexus/fmnexus.cs.2
(~3,160 lines, an old backup copy). It's a good cleanup, just unrelated to the feature.

---

---

Appendix A — Detailed issue context

Line-level detail, evidence and repro steps for every item in the checklist above.

🔴 B1. Banner covers the Import/Close buttons at non-100% display scaling

Where: sqlnexus/fmImport.cs, sqlnexus/fmImport.Designer.cs

What the code does now

The new laSharedFolder label is placed by the designer at a fixed spot, Location = (0, 63),
Size = 455 x 18, anchored Bottom | Left | Right. At that position it physically sits on top of
three existing controls in the same panel:

Control	Location	Size	Occupies Y
llOptions ("Options" link)	(9, 66)	43 x 13	66–79
tsbGo ("Import" button)	(295, 56)	75 x 25	56–81
btnClose ("Close" button)	(376, 56)	75 x 25	56–81
laSharedFolder	(0, 63)	455 x 18	63–81 ← overlaps all three

It only appears correct because ShowSharedFolderLabel manually grows the panel and the form by a
hard-coded constant, which pushes the bottom-anchored label down out of the way:

private const int SharedFolderLabelRowHeight = 22;   // line 2361

int delta = show ? SharedFolderLabelRowHeight : -SharedFolderLabelRowHeight;
paTop.Height += delta;
this.ClientSize = new Size(this.ClientSize.Width, this.ClientSize.Height + delta);

Why it breaks

fmImport sets AutoScaleMode = AutoScaleMode.Font with AutoScaleDimensions = 6F, 13F
(fmImport.Designer.cs). At runtime WinForms scales every
designer coordinate and size by the user's DPI / font setting — but a constant written in C# code
is not scaled. So:

Display scaling	Label height after scaling	Panel grows by	Result
100%	18 px	22 px	OK (4 px clearance)
125%	~23 px	22 px	Label overlaps the buttons by ~1 px
150%	~27 px	22 px	Label overlaps the buttons by ~5 px
200%	~36 px	22 px	Label covers the button row

Repro: Windows Settings → Display → Scale = 150% → sign out/in → open SQL Nexus → Import →
point at a LogScout instance folder that has a SharedOutputFiles sibling. The yellow banner is drawn
over the Import and Close buttons.

Expected: the banner always sits below the button row at any scaling, and no manual pixel maths is
needed.

Secondary defect in the same method: the growth is never reversed when the dialog expands into the
import view — fmImport.cs sets m_sharedFolderLabelShown = false
and hides the label but leaves paTop.Height 22 px taller, so the header keeps a dead gap.

---

🔴 B2. An unreadable SharedOutputFiles folder aborts the whole import scan

Where: sqlnexus/fmImport.cs, sqlnexus/fmImport.cs, sqlnexus/FileImporter.cs

What the code does now

// fmImport.cs:307 — inside AddFilesFromDirectory, no try/catch
string[] allMatches = Directory.GetFiles(basePath, Mask);

// fmImport.cs:1301 — the call site, also no try/catch
foreach (string s in prod.SupportedMasks)
{
    if (AddFiles(s, prod))
        anyFilesFound = true;
}

// FileImporter.cs:75 — RawFileImporter.DoImport, same pattern
string[] files = Directory.GetFiles(searchPath, rawfile.Mask);

Why it's worse than before this PR

Before the change there was exactly one folder to enumerate, and it was the one the user explicitly
browsed to and which tsbGo.Enabled = Directory.Exists(cbPath.Text) had already validated. Now a
second, implicitly discovered folder is enumerated with the same unguarded call. Directory.Exists
returning true does not guarantee GetFiles will succeed — it can still throw:

• UnauthorizedAccessException — the folder or a file in it has restrictive ACLs (common when a
  capture is copied off a locked-down production box, or unzipped by a different user).
• PathTooLongException — the sibling path plus a long LogScout filename exceeds MAX_PATH on
  .NET Framework 4.8.
• IOException — the folder is on a network share that dropped mid-scan.

Any of these propagates out of AddFiles → out of the importer enumeration loop → to the global
handler. The user gets a crash dialog and zero files imported, including everything in the
folder they actually chose.

Repro: create output\SharedOutputFiles, then icacls output\SharedOutputFiles /deny "%USERNAME%":(R).
Point Nexus at output\SERVER_SQL2019 and press Import.

Expected: the sibling folder is skipped with a visible warning; the primary folder still imports
normally.

---

🔴 D1. The user documentation lands in a file nobody reads

Where: sqlnexus/Help/QuickStartPostmortemAnalysis.htm

The PR adds a well-written 15-line explanation of the new behaviour to that page. Verified facts:

1. QuickStartPostmortemAnalysis.htm does not appear in the [FILES] section of
   sqlnexus/Help/sqlnexus2.hhp. That list is only: UsingSQLNexus.htm (default topic),
   AddingNewReports.htm, CollectingDiagnostics.htm, CustomizingReports.htm,
   EmailingReports.htm, ExportingReports.htm, Overview.htm, RunningReports.htm.
2. No other help page links to it (a repo-wide search for the filename returns only the file's own
   self-referencing <link rel=File-List> tag).
3. It is not a Content item in sqlnexus.csproj, so it isn't even copied to the output folder.
4. sqlnexus.chm is a checked-in binary that only changes when someone recompiles from the
   .hhp. This PR does not touch the .chm.

Net effect: the text exists only in the git repo. Pressing F1 in the app will never show it.

Expected: the note reaches users — either in a compiled help topic, or in the repo README.md /
the project wiki, which is where the rest of the user-facing guidance actually lives.

---

🔴 D2. The PR description contradicts the shipped code in three places

Reviewers, release notes and future git archaeology all read the description first. Three of its
claims are now the opposite of what the branch contains:

Description claims	What commit 536ab97 actually does	Evidence
"validates the sibling is a real direct sibling … rejects traversal, fails closed"	GetImportSearchPaths runs Path.GetFullPath first, which collapses ..; the direct-sibling guard was deleted	SharedOutputFolderTests.GetImportSearchPaths_PrimaryContainsDotDotSegment_NormalizesAndResolvesSibling asserts the traversal path succeeds, and its own comment says "after removing the (dead) direct-sibling guard"
"Mask-based importers (e.g. Perfmon BLG) get a second row pointing at the sibling"	The sibling row is skipped whenever the primary folder already produced a row	fmImport.cs — if (isShared && anyAdded && !(Importer is INexusFileImporter)) continue; (added later, in commit 3f16ee9)
"SqlNexus.UnitTests: SignAssembly=false" and "ErrorLogImporter: remove key-qualified InternalsVisibleTo and widen 6 members from internal to public"	The test project stays SignAssembly=true (now using sqlnexus\SqlNexus.snk); the key-qualified InternalsVisibleTo was added; the ErrorLogImporter members remain internal	SqlNexus.UnitTests.csproj, ErrorLogImporter/Properties/AssemblyInfo.cs, ErrorLogImporter/ErrorLogImporter.cs

The description was clearly accurate for the first commit and was never updated as review feedback
reshaped the branch across the following 10 commits. (For the record, the final signing approach is
the better one — it keeps the product internals internal.)

---

🟡 B3. LinuxPerfImporter working directory is a global that is never restored

Where: sqlnexus/fmImport.cs, sqlnexus/fmImport.cs

LinuxPerfImporter.Model.ConfigValues.WorkingDirectory is a static property. It's set once before
the import loop:

LinuxPerfImporter.Model.ConfigValues.WorkingDirectory = srcPath;   // line 1539

then re-pointed inside the loop for a Linux-perf row that came from the sibling folder:

if (ri.Name != null && ri.Name.IndexOf("Linux Performance", StringComparison.OrdinalIgnoreCase) >= 0)
{
    string targetDir = Path.GetDirectoryName(targetPath);
    if (!string.IsNullOrEmpty(targetDir))
        LinuxPerfImporter.Model.ConfigValues.WorkingDirectory = targetDir;   // line 1658
}

There is no corresponding restore. Once a shared-folder Linux-perf row runs, the global points at
...\SharedOutputFiles for the remainder of the process — including any later import, post-processing
step, or a second Import run in the same session that reuses the cached value before line 1539 runs
again. The bug is latent today but is exactly the kind that surfaces months later as "the second
import in a session reads the wrong folder".

Expected: scope the change to the row, e.g. capture the previous value and restore it in a
finally.

---

🟡 L1. Files are dropped where the user cannot see it

Where: sqlnexus/fmImport.cs

if (isShared && anyAdded && !(Importer is INexusFileImporter))
{
    MainForm.LogMessage(
        "Shared folder: skipping duplicate files matching '" + Mask + "' ...",
        MessageOptions.Silent);          // ← file-only
    continue;
}

For mask-based importers (Perfmon *.blg, ReadTrace *.trc/*.xel) this discards every matching
file in SharedOutputFiles as soon as the instance folder produced one match. The reasoning in the
comment is sound (a second row would re-run table setup over the first run's results), but
MessageOptions.Silent writes only to sqlnexus.log. A user whose Perfmon counters are split across
both folders sees a successful import with half the data and no indication why.

Expected: this is a data-completeness decision, not a debug detail. Surface it at
MessageOptions.All (status bar + log + dialog) with the folder name and the count of skipped files.

---

🟡 L2. The most important warning is effectively invisible

Where: sqlnexus/fmImport.cs

The "same name, different size" case is genuinely ambiguous — the two files may be different
captures, and the code deliberately refuses to guess. Its own comment says it should be "surfaced
clearly so the user can decide". But it is logged as:

MainForm.LogMessage("... WARNING: it exists in BOTH ... with a DIFFERENT size ...",
    MessageOptions.Both);

and in NexusInterfaces/interfaces.cs:

StatusBar = 2,
Silent    = 4,
Dialog    = 8,
Both      = StatusBar | Silent,     // ← no Dialog
All       = StatusBar | Silent | Dialog,

Both = status bar + log file only. It is emitted once per colliding file inside a tight loop, so
each message overwrites the previous one within milliseconds. In practice the user sees a flicker and
then whatever the next log line is.

Expected: collect the ambiguous collisions, then emit one MessageOptions.All message at the
end of the scan listing the count and the file names. Keep the per-file detail at Silent.

---

🟡 A1 / A2. Accessibility gaps on the new banner

Where: sqlnexus/fmImport.cs

Two distinct problems:

A1 — screen reader users are never told the second folder was added. laSharedFolder is a plain
Label: it is not focusable, not in the tab order, and has no UIA live-region semantics. It is
populated from tbPath_TextChanged, i.e. while the user is typing in cbPath. Focus never leaves
cbPath, so NVDA/Narrator/JAWS have no reason to read the label and the user is simply not informed
that a second folder will be scanned. The AccessibleName is set correctly, but nothing ever asks
for it.
Fix direction: mirror the message into cbPath.AccessibleDescription (that control is focused,
so it will be announced), or raise a UIA notification.

A2 — the full path is mouse-only. The label shows an abbreviated path
(AbbreviatePath(sibling, 48) → C:\...\output\SharedOutputFiles) plus AutoEllipsis; the complete
path exists only in toolTip1. Tooltips in WinForms appear on mouse hover and are not reachable
by keyboard, so a sighted keyboard-only user cannot read the full path (WCAG 2.1.1 Keyboard).
Fix direction: also write the full path to the log at a user-visible level, or let the label wrap /
show the full path when the dialog is wide enough.

A3 (nit) — stale accessible name. In the else branch the visible Text and the tooltip are
cleared, but AccessibleName still holds the previous folder's full path, so a screen reader can read
out a folder that is no longer being scanned.

---

🟡 T1. The behaviour that actually changed has no tests

SharedOutputFolderTests.cs (~30 tests) is good work, but every test targets a pure helper.
The three pieces of logic that determine what actually gets imported are untested because they are
embedded in WinForms / SQL code:

Untested logic	Where	Why it matters
"skip the sibling when the primary already matched" for mask importers	fmImport.cs	Decides whether Perfmon data is dropped (L1)
Two-folder loop + name dedupe in RawFileImporter.DoImport	FileImporter.cs	Decides whether raw files are double-imported
m_RowTargetPaths lookup + legacy srcPath + ll.Text fallback	fmImport.cs	Decides which file each importer actually opens

The PR already demonstrates the right technique — SharedOutputFolder was extracted specifically so
it could be tested without WinForms. Apply the same technique once more to these three.

---

🟡 S1. Strong-naming the shipping EXE is out of scope and rests on a questionable claim

Where: sqlnexus/sqlnexus.csproj, SqlNexus.McpServer/SqlNexus.McpServer.csproj

sqlnexus.csproj flips SignAssembly from false to true — the main product EXE is now
strong-named — and SqlNexus.McpServer.csproj gains strong naming too. The justification in the
csproj comment is:

A strong-named test assembly cannot load an unsigned reference at runtime (FileLoadException), and a
signed assembly can only grant InternalsVisibleTo to a strong-named friend.

The second half is correct and is the real reason the key-qualified InternalsVisibleTo was
needed. The first half is not generally true on .NET Framework: referencing an unsigned assembly
from a strong-named one produces compiler warning CS8002, which this project already suppresses
via <NoWarn>$(NoWarn);8002</NoWarn>, and it loads fine at runtime. If that's right, signing the EXE
was not required to unblock the tests.

Why it matters: strong-naming a shipping executable is a packaging change with downstream effects
(installer/WiX, the release signing pipeline, anything that binds by full assembly name). It deserves
its own PR or at least an explicit "yes, we want this" from the maintainer — not a side effect of a
folder-scanning feature.

---

⚪ Nits, with locations

#	Item	Location
N1	Missing primary folder now returns "no files" instead of an error — add a log line	fmImport.cs
N2	"Number of files blocked for import…: 0" now logged per-mask even when nothing matched	fmImport.cs
N3	Path.GetFullPath + a second Directory.Exists run on every keystroke in the path box	fmImport.cs
N4	MainForm is not null-checked inside the catch, so an error could throw out of the error handler	fmImport.cs
N5	Bare catch { } with no exception variable and no log (repo convention violation)	fmImport.cs
N6	Comment says the suffix is "(SharedOutput)"; the real suffix is " (from SharedOutputFiles)"	fmImport.cs
N7	Closing brace still marked //end of AddFiles — the method is now AddFilesFromDirectory	fmImport.cs
N8	AddFileRow is now a one-line wrapper over AddFileRowReturningLabel	fmImport.cs
N9	New README lines contain a raw Windows-1252 em dash (byte 0x97) → renders as � on GitHub	TestingInfrastructure/README.md
N10	Loud log messages embed full folder paths (host/server names); prefer names + counts at All, full paths at Silent	fmImport.cs

---

---

Appendix B — Copy-paste prompt for the AI

Paste everything in the block below into Copilot / your agent of choice, with the
ImportSharedOutputDir_556_pijocoder_090326 branch checked out.

You are working in the SqlNexus repo (C# 7.3, .NET Framework 4.8, WinForms) on branch
ImportSharedOutputDir_556_pijocoder_090326. Follow .github/copilot-instructions.md at all times:
minimal focused edits, no reformatting of untouched code, no new build warnings, every behavior
change covered by an MSTest unit test under
TestingInfrastructure/UnitTests/SqlNexus.UnitTests/, and no empty catch blocks.

Apply the following fixes from code review of PR #558. Work through them in order. After each
group, build sqlnexus.csproj and report any errors.

=== GROUP 1 — BLOCKERS ===

1. Fix the high-DPI layout break in the "Also scanning:" banner.
   Files: sqlnexus/fmImport.Designer.cs, sqlnexus/fmImport.cs (ShowSharedFolderLabel ~line 2360).
   Problem: laSharedFolder is absolutely positioned at (0,63) where it overlaps llOptions (9,66),
   tsbGo (295,56) and btnClose (376,56). It only looks right because ShowSharedFolderLabel adds a
   hard-coded 22 px to paTop.Height and this.ClientSize. The form uses AutoScaleMode.Font, so the
   label scales with DPI but the 22 does not — at 125%+ the banner covers the Import/Close buttons.
   Required fix: remove the SharedFolderLabelRowHeight constant and the manual pixel arithmetic.
   Make the layout scale-independent instead — dock laSharedFolder to the bottom of paTop (or place
   it in an auto-sizing container row) and move it below the button row in the designer so it never
   overlaps. If a runtime resize is still unavoidable, compute the delta from
   laSharedFolder.PreferredHeight (or .Height) at runtime, never from a literal.
   Also fix the related defect at fmImport.cs ~line 1428: when the dialog expands into the import
   view it sets m_sharedFolderLabelShown = false and hides the label but never reverses the panel
   growth, leaving a dead gap in the header.
   Verify: the banner sits fully below the Import/Close buttons at 100%, 125%, 150% and 200%
   display scaling, and the header has no leftover gap after the dialog expands.

2. Stop an unreadable SharedOutputFiles folder from aborting the entire import.
   Files: sqlnexus/fmImport.cs (AddFilesFromDirectory ~line 307), sqlnexus/FileImporter.cs
   (RawFileImporter.DoImport ~line 75).
   Problem: Directory.GetFiles is called with no try/catch, and the call site fmImport.cs ~line 1301
   (foreach over prod.SupportedMasks) is unguarded too. Directory.Exists returning true does not
   guarantee GetFiles succeeds — UnauthorizedAccessException, PathTooLongException and IOException
   are all reachable for the implicitly-discovered sibling folder. Today any of them crashes the
   whole enumeration and nothing at all gets imported.
   Required fix: wrap each per-directory enumeration in try/catch for UnauthorizedAccessException,
   PathTooLongException and IOException (catch the specific types, not bare Exception). Log with
   MainForm.LogMessage(..., MessageOptions.All) naming the folder and the reason, then continue so
   the primary folder still imports. Do the same in RawFileImporter.DoImport using
   Util.Logger.LogMessage.
   Verify: with `icacls output\SharedOutputFiles /deny "%USERNAME%":(R)` applied, importing
   output\SERVER_SQL2019 still succeeds and shows a clear warning about the skipped folder.

3. Put the user documentation where users can actually read it.
   Problem: sqlnexus/Help/QuickStartPostmortemAnalysis.htm is an orphan — it is NOT in the [FILES]
   list of sqlnexus/Help/sqlnexus2.hhp, no other help page links to it, and it is not a Content item
   in sqlnexus.csproj. sqlnexus.chm is a checked-in binary that only changes when the .hhp is
   recompiled, and this PR does not touch it. The new text therefore reaches no user.
   Required fix: move the SharedOutputFiles explanation into a help topic that IS compiled — best
   candidates are sqlnexus/Help/UsingSQLNexus.htm (the default topic) or
   sqlnexus/Help/CollectingDiagnostics.htm. Keep the wording and the existing HTML style of the
   target page. IMPORTANT: those files are plain ASCII while WordSources/*.htm are Windows-1252 —
   do not let the editor re-encode anything; after saving, confirm no byte > 127 was introduced that
   was not there before. Note in the PR description that sqlnexus.chm must be recompiled from
   Help/sqlnexus2.hhp for the change to ship.
   Also add a short (2–4 line) note to the repo root README.md explaining the SharedOutputFiles
   behavior and the "(from SharedOutputFiles)" row suffix, since that is the front door for users.

=== GROUP 2 — SHOULD FIX ===

4. Scope the LinuxPerfImporter working-directory change to the row that needs it.
   File: sqlnexus/fmImport.cs ~lines 1646-1660.
   Problem: LinuxPerfImporter.Model.ConfigValues.WorkingDirectory is a static global. It is
   re-pointed at the sibling folder for a Linux-perf row and never restored, so it stays wrong for
   everything that runs afterwards in the process.
   Required fix: capture the previous value before assigning and restore it in a finally block
   around that row's Initialize/DoImport.

5. Make dropped files visible to the user.
   File: sqlnexus/fmImport.cs ~lines 269-281.
   Problem: for mask-based importers (Perfmon *.blg, ReadTrace) every matching file in
   SharedOutputFiles is discarded once the primary folder matched, and this is logged only at
   MessageOptions.Silent. A user whose Perfmon data spans both folders silently loses half of it.
   Required fix: raise this to MessageOptions.All and include the skipped folder path and the count
   of skipped files.

6. Make the ambiguous-duplicate warning actually reach the user.
   File: sqlnexus/fmImport.cs ~lines 336-348.
   Problem: the "same name but DIFFERENT size" warning uses MessageOptions.Both, which is
   StatusBar|Silent only (see NexusInterfaces/interfaces.cs) — no dialog — and it is emitted once
   per file in a tight loop, so each message instantly overwrites the previous one.
   Required fix: accumulate the ambiguous collisions during the scan, then emit ONE
   MessageOptions.All message at the end containing the count and the file names. Keep the per-file
   detail at MessageOptions.Silent.

7. Fix the accessibility gaps on the banner.
   File: sqlnexus/fmImport.cs, UpdateSharedFolderAffordance ~lines 2287-2326.
   (a) laSharedFolder is a non-focusable Label updated while focus is in cbPath, so a screen reader
       never announces that a second folder will be scanned. Mirror the message into
       cbPath.AccessibleDescription (that control IS focused) so it is announced, or raise a UIA
       notification.
   (b) The full sibling path exists only in toolTip1, which is mouse-only (WCAG 2.1.1 Keyboard).
       Make the full path reachable without a mouse — e.g. also log it at a user-visible level
       and/or let the label show the full path when width allows.
   (c) In the else branch, clear laSharedFolder.AccessibleName (and AccessibleDescription) as well
       as Text and the tooltip, so a stale folder path is not read out.

8. Theme the banner like the rest of the panel.
   File: sqlnexus/fmImport.cs ~lines 2305-2310, sqlnexus/fmImport.Designer.cs.
   Problem: every other control in paTop (laPath, laInstructions, llOptions, cbPath) is painted by
   ThemeManager.ApplyTheme. The banner hard-codes SystemColors.Info/InfoText and deliberately
   re-applies them OVER the theme on every path change, so in the Aquatic dark theme (#202020) it is
   a foreign pale-yellow strip, and it clashes in Desert too.
   Required fix: derive the banner colors from ThemeManager (ThemeManager.CurrentThemeName,
   CurrentForeColor, CurrentBackColor, CurrentOtherColor) for the Default/Aquatic/Desert themes, and
   fall back to SystemColors ONLY when ThemeManager.IsHighContrastEnabled is true — matching the
   pattern in ThemeManager.ApplyHighContrastTheme. Keep the "Also scanning:" wording so meaning is
   never carried by color alone, and keep contrast within the theme's own palette.

9. Add unit tests for the behavior that actually changed.
   Problem: all ~30 new tests target pure helpers; the three pieces of logic that decide what gets
   imported are untested because they are embedded in WinForms/SQL code.
   Required fix: apply the same extraction technique the PR already used for SharedOutputFolder, and
   add tests in TestingInfrastructure/UnitTests/SqlNexus.UnitTests/sqlnexus/ for:
     - the "skip the sibling when the primary already matched" rule for mask-based importers
       (extract a pure predicate such as ShouldSearchSiblingFolder(bool importerIsPerFile,
       bool primaryAlreadyAdded) from fmImport.AddFiles ~line 269);
     - the two-folder loop + name-dedupe in RawFileImporter.DoImport (extract the folder/mask
       selection so it can be exercised without a SQL connection);
     - the m_RowTargetPaths lookup including the legacy `srcPath + ll.Text` fallback path.
   Cover happy path, boundary cases and negative cases for each, per copilot-instructions.md.

10. Justify or revert strong-naming the shipping EXE.
    Files: sqlnexus/sqlnexus.csproj, SqlNexus.McpServer/SqlNexus.McpServer.csproj.
    Problem: SignAssembly was flipped false -> true for the main product EXE (and added to the MCP
    server) as a side effect of unblocking the tests. The csproj comment claims "a strong-named test
    assembly cannot load an unsigned reference at runtime (FileLoadException)" — that is not
    generally true on .NET Framework; it produces warning CS8002, which is already suppressed via
    <NoWarn>$(NoWarn);8002</NoWarn>.
    Required action: empirically verify whether the tests pass with sqlnexus.csproj SignAssembly
    back at false while keeping the key-qualified InternalsVisibleTo. If they do, revert the
    SignAssembly changes to keep this PR focused. If they do not, correct the csproj comment to
    state the real reason and flag the packaging/installer/signing-pipeline impact in the PR
    description.

=== GROUP 3 — NITS ===

11. fmImport.cs ~304: log a warning (MessageOptions.All) when the PRIMARY import folder does not
    exist, instead of silently returning "no files found".
12. fmImport.cs ~288: only emit "Number of files blocked for import…" when blockedCounter > 0.
13. fmImport.cs ~2278: tbPath_TextChanged now runs Path.GetFullPath plus a second Directory.Exists
    on every keystroke, which blocks the UI thread on an unreachable UNC path. Only recompute when
    the text is a rooted, plausible path, or debounce.
14. fmImport.cs ~2319: use MainForm?.LogMessage inside the catch so an error cannot throw out of the
    error handler.
15. fmImport.cs ~2350: replace the bare `catch { }` in AbbreviatePath with
    `catch (Exception ex)` plus a MessageOptions.Silent log, per copilot-instructions.md.
16. fmImport.cs ~297: the comment says the suffix is "(SharedOutput)"; the real suffix is
    " (from SharedOutputFiles)". Fix the comment.
17. fmImport.cs ~440: the closing brace still says "//end of AddFiles" but the method is now
    AddFilesFromDirectory. Fix the marker.
18. fmImport.cs ~442: AddFileRow is now a one-line wrapper over AddFileRowReturningLabel — call the
    new method directly and delete the wrapper.
19. TestingInfrastructure/README.md ~50: the newly added lines contain a raw Windows-1252 em dash
    (byte 0x97) which renders as U+FFFD on GitHub. Replace with "--" or a proper UTF-8 em dash. Do
    not disturb the pre-existing 0x97 bytes elsewhere in the file unless you convert the whole file
    deliberately.
20. Trim the block comments that only restate what the next few lines do (keep the ones that explain
    WHY — e.g. why the primary copy wins, why the sibling is not scanned for Custom XEL, why the
    Linux importer is matched by name). Per copilot-instructions.md, comments should state what the
    code cannot show on its own.

=== FINALLY ===

21. Rewrite the PR description to match the code as it now stands. Three statements are currently the
    opposite of what shipped:
      - it claims traversal is rejected, but Path.GetFullPath collapses ".." and the direct-sibling
        guard was removed (see the test
        GetImportSearchPaths_PrimaryContainsDotDotSegment_NormalizesAndResolvesSibling);
      - it claims mask-based importers get a second row for the sibling, but the code skips the
        sibling when the primary already matched;
      - it claims SqlNexus.UnitTests uses SignAssembly=false and that ErrorLogImporter's
        key-qualified InternalsVisibleTo was removed with 6 members widened to public — the final
        code does the opposite (test project stays signed, key-qualified IVT added, members stay
        internal).
    Also mention that this branch deletes sqlnexus/fmnexus.cs.2 (~3,160 lines, an old backup copy) —
    a good cleanup, but unrelated to the feature.

22. Run the full unit test suite and paste the result (or a CI link) into the PR. Note: in some
    environments `dotnet test` cannot restore because Microsoft.SqlServer.XEvent.XELite returns
    HTTP 402 from the private feed — that is a known feed/billing issue unrelated to this PR.

[JosephPilov-MSFT (PiJoCoder)]

Review — PR #558 · Import sibling SharedOutputFiles folder during log import

What this change does (plain English)

SQL LogScout's "All Instances" capture splits its output into two folders: one per SQL instance (e.g. output\SERVER_SQL2019) and one shared folder next to it (output\SharedOutputFiles) that holds machine-wide files such as driver lists and disk info. Today, if you point SQL Nexus at the instance folder, everything in the shared folder is ignored.

This PR makes SQL Nexus look in both folders. If the shared folder isn't there, nothing changes. Rows that came from the shared folder are labelled (from SharedOutputFiles), a yellow "Also scanning: …" note appears under the path box, and a file that exists in both folders is imported only once (the instance folder wins).

It also fixes the assembly-signing setup so the unit test project can actually run against the main app again — which immediately exposed a real crash bug in Program.IsDbNameValid (now fixed).

Overall: solid, well-commented work with a genuinely good new test file. The blocking issues are a high-DPI layout break, an unguarded folder scan that can crash the import, and documentation that lands in a file no user will ever see.

Checklist

Legend: 🔴 Must fix · 🟡 Should fix · ⚪ Nit

1. Are there bugs?

• 🔴 High-DPI layout break — the new banner covers the Import/Close buttons.
  ShowSharedFolderLabel grows the header by a hard-coded 22 pixels, but the form uses
  AutoScaleMode.Font. At 125%/150% display scaling the label grows and the 22 doesn't, so the
  banner sits on top of the Import and Close buttons.
  Fix: grow by the label's runtime Height/PreferredHeight instead of the constant — better,
  dock the label to the bottom of paTop (or use an auto-sizing layout row) so no manual resize is
  needed at all. → sqlnexus/fmImport.cs, sqlnexus/fmImport.Designer.cs
• 🔴 An unreadable shared folder crashes the whole import scan.
  AddFilesFromDirectory calls Directory.GetFiles(basePath, Mask) with no try/catch, and its
  caller (AddFiles, invoked in the importer loop) isn't wrapped either. If SharedOutputFiles has
  restrictive permissions or a too-long path, an UnauthorizedAccessException / PathTooLongException
  takes down the entire enumeration — for a folder the user never selected. Same gap in
  RawFileImporter.DoImport.
  Fix: wrap each per-directory enumeration in try/catch (UnauthorizedAccessException | IOException | PathTooLongException), log it with MessageOptions.All, and carry on with the primary folder.
• 🟡 LinuxPerfImporter working directory is changed and never put back.
  ConfigValues.WorkingDirectory is a static global. When a Linux-perf row comes from the shared
  folder it's repointed there and left that way for everything that runs afterwards.
  Fix: save the previous value and restore it in a finally after the row completes.
• 🟡 Data can be dropped where the user can't see it.
  For mask-based importers (Perfmon *.blg, ReadTrace) the shared folder is skipped whenever the
  instance folder already matched — logged only with MessageOptions.Silent. If a customer really
  has .blg files in both folders, half the data silently disappears.
  Fix: log that skip with MessageOptions.All and name the folder that was skipped.
• ⚪ A typo'd source path now fails quietly. if (!Directory.Exists(basePath)) return false;
  turns a bad path into "no files found" instead of an error.
  Fix: log a warning when the primary folder doesn't exist.
• ⚪ Header keeps 22px of dead space. When the form expands into the import view, the flag is
  reset and the label hidden, but paTop.Height/ClientSize are never shrunk back.
  Fix: reverse the resize, or (better) remove the manual resize entirely per the first item.
• ⚪ UI work on every keystroke. tbPath_TextChanged now also runs Path.GetFullPath +
  a second Directory.Exists per character typed. On an unreachable UNC path this freezes the dialog.

.....

=== FINALLY ===

21. Rewrite the PR description to match the code as it now stands. Three statements are currently the
    opposite of what shipped:

    • it claims traversal is rejected, but Path.GetFullPath collapses ".." and the direct-sibling
      guard was removed (see the test
      GetImportSearchPaths_PrimaryContainsDotDotSegment_NormalizesAndResolvesSibling);
    • it claims mask-based importers get a second row for the sibling, but the code skips the
      sibling when the primary already matched;
    • it claims SqlNexus.UnitTests uses SignAssembly=false and that ErrorLogImporter's
      key-qualified InternalsVisibleTo was removed with 6 members widened to public — the final
      code does the opposite (test project stays signed, key-qualified IVT added, members stay
      internal).
      Also mention that this branch deletes sqlnexus/fmnexus.cs.2 (~3,160 lines, an old backup copy) —
      a good cleanup, but unrelated to the feature.

22. Run the full unit test suite and paste the result (or a CI link) into the PR. Note: in some
    environments dotnet test cannot restore because Microsoft.SqlServer.XEvent.XELite returns
    HTTP 402 from the private feed — that is a known feed/billing issue unrelated to this PR.

---

Reviewer follow-up - what changed today, and where we diverged

Thanks for the thorough pass. Addressed across 7 commits; build clean, 298 tests
pass. Summary of what we did, plus the two places we intentionally diverged.

DONE

Bugs / resilience

• Unguarded Directory.GetFiles: wrapped in AddFilesFromDirectory AND
  RawFileImporter.DoImport (UnauthorizedAccessException | IOException |
  PathTooLongException) - log at All and continue with the primary folder.
  [9d28d02]
• RawFileImporter two-folder loop had NO dedup (could double-insert a file present
  in both folders); now filtered via the tested GetSiblingOnlyFiles. [earlier]
• LinuxPerf ConfigValues.WorkingDirectory (static global) saved before each row and
  restored in finally, so it no longer leaks to later importers. [9369135]
• Missing PRIMARY folder now logged instead of silently "no files"; missing sibling
  stays silent. [9369135]

High-DPI / layout (your item 1 + 10-RED)

• Banner no longer overlaps the buttons: replaced the fixed 22px growth with the
  label's runtime height [9d28d02], then docked the label to paTop bottom
  (Dock=Bottom, AutoSize) so it flows like every other control and the manual
  resize is measured, not hard-coded. Also reversed the leftover header growth on
  expand. [5d840ef]

Logging visibility / noise

• "same name, DIFFERENT size" is now ONE aggregated dialog (count + file names),
  not per-file status-bar lines that get overwritten. [bb44402]
• "blocked ... : 0" only logs when > 0. [bb44402]

Accessibility

• Sibling announcement mirrored into cbPath.AccessibleDescription (focusable) and
  logged (keyboard-reachable full path, WCAG 2.1.1); stale AccessibleName cleared
  when the sibling disappears. [bb44402]

Error handling

• MainForm?.LogMessage in the affordance catch; AbbreviatePath bare catch -> named
  • Silent log. [bb44402]

Comments / docs / tests

• Extracted ShouldSkipSiblingForMask as a pure helper and unit-tested the mask
  fallback rule (your item 5); also added target-path/display-text helper tests
  earlier. [5d840ef]
• Fixed stale "(SharedOutput)" comment and "//end of AddFiles" marker; fixed the
  Windows-1252 em-dash (0x97) in TestingInfrastructure/README.md; added a root
  README section on SharedOutputFiles. [5d840ef]

Modal-noise reduction (extends your theme of dialogs-only-for-actionable-things)

• Mask-skip notice reworded + de-escalated from modal to Both (it read like an
  error though data WAS imported from the primary). [8ee7c26]
• Shared-folder announcement no longer pops on form open (remembered path);
  fires only on user-driven path change. [5b08b39]
• "Importer enabled but no matching files" (and the Custom XEL equivalent) dropped
  from modal to Both. [ad6667a]

WHERE WE DIVERGED (please weigh in)

1. "Remove the dead wrapper AddFileRow" - NOT done. It still has 6 live callers
   (Custom XEL / Raw file / Post-Process / PerfStats / Runtime-count / Enum-reports
   rows); it's a convenience overload that discards the returned label, not dead
   code. Deleting it would mean editing 6 call sites for no benefit.

2. Message severity - we went the OPPOSITE direction on one earlier request.
   An earlier round asked to RAISE the different-folder/skip notices to a dialog for
   visibility. A real import test showed those dialogs are misleading ("NOT importing
   ..." while data was in fact imported from the primary) and annoying on form open.
   So: genuine "a copy was not imported" (different size) stays a dialog, but the
   pure informational cases (mask-skip when primary already imported, importer with
   no files, on-open announcement) are now log + status bar, not modal.

PARTIAL / MY CALL

• Theming of the banner: kept the stand-out SystemColors.Info band for now (it also
  adapts under High Contrast). Full ThemeManager integration for the Aquatic/Desert
  dark themes is deferred pending a visual decision - happy to do it if strongly preferred.

CONFIRMS (outside the code)

• Strong-naming the EXEs WAS required: with only the test project signed we hit
  FileLoadException (0x80131044) and 79 test failures at runtime, not just CS8002.
  The comment can be reworded, but the change stands. Please confirm the release /
  Authenticode pipeline tolerates strong-named EXEs (we believe it's orthogonal).
• PR description has been rewritten to match the final commits (the earlier text
  described the pre-iteration design and is now corrected).
  ================================================================================

[JamesFerebee-MSFT (JamesFerebee)]

Review — PR #558: Import sibling SharedOutputFiles folder during log import

How to read this list: items tagged [PR] were introduced by this PR and are the priority.
Items tagged [Existing] are pre-existing problems this PR merely touches or reveals — they are
explicitly not this PR's fault and can be deferred to a follow-up. Anything the PR description or
an earlier commit already handled has been dropped or narrowed below — nothing here re-litigates a
decision you've already documented.

---

Must fix

• [PR] The in-app Help update doesn't actually reach any user.
  The note was added to sqlnexus/Help/QuickStartPostmortemAnalysis.htm, but that page is an orphan:
  it is not listed in Help/sqlnexus2.hhp under [FILES], it is not a Content item in
  sqlnexus.csproj, and no other help page links to it. So the text ships to nobody, and the PR
  description's "in-app Help" claim is not accurate.
  Risk if not fixed: Users see import rows for files that are not in the folder they selected.
  With no Help entry explaining that, the natural reading is "Nexus is importing the wrong data" or
  "this list is buggy" — a support-case generator. Worse, the PR merges with a description asserting
  the documentation exists, so anyone later auditing "is this documented?" ticks the box and the gap
  never gets found.
  Fix: Move the SharedOutputFiles note into a help page that is actually in the .hhp [FILES]
  list (CollectingDiagnostics.htm or UsingSQLNexus.htm are the natural homes), then rebuild
  sqlnexus.chm from Help/sqlnexus2.hhp — the .chm is a checked-in binary, so editing the
  .htm alone changes nothing. Alternatively, drop the "in-app Help" line from the PR description.

Should fix

• [PR] The new "Also scanning" banner ignores the app's theme engine.
  laSharedFolder hard-codes SystemColors.Info / InfoText in the designer and re-applies them
  at runtime specifically to overwrite ThemeManager.ApplyTheme. Every other control in that header
  panel is themed. In the Aquatic (dark, #202020) and Desert (#FFFAEF) themes, a pale
  yellow band will look out of place. This is the one place the PR visibly diverges from how the rest
  of the section renders.
  Risk if not fixed: On the dark Aquatic theme a bright pale-yellow block on #202020 reads as a
  rendering defect, and it sits on the first dialog users touch — expect it reported as a bug. It
  also sets a precedent that new controls may bypass ThemeManager, so the next control copies the
  pattern and theme coverage erodes. There's a latent trap too: fore and back colors are currently
  pinned together, so if someone later themes only one of them, the text becomes unreadable.
  Fix: Add an info-banner background/foreground color pair to ThemeManager for all three themes
  and have the banner read from ThemeManager.CurrentThemeName instead of hard-coding colors. Keep
  the existing SystemInformation.HighContrast behavior (defer to SystemColors) unchanged, and
  keep the literal "Also scanning:" text so meaning is never carried by color alone.

• [PR] Reconsider the one remaining modal, on a user-driven path change.
  This is a deliberate choice, not an oversight — you already removed the form-open dialog and the PR
  description states the announcement "fires only on a user-driven path change." The concern with
  what's left: tbPath_TextChanged fires on every keystroke, so MessageOptions.All shows a
  MessageBox and steals focus the instant a typed path resolves to a folder with a sibling. The
  banner, tooltip, and cbPath.AccessibleDescription already convey the same information.
  (This does not affect automated runs — any command-line argument sets
  Globals.ConsoleMode = true, which suppresses dialogs.)
  Risk if not fixed: Because it fires from TextChanged, the dialog can appear mid-entry. Any
  keystrokes typed after it pops go to the dialog instead of the text box, so the path can end up
  truncated and the user has to retype it. For keyboard and screen-reader users, focus is yanked out
  of the field and has to be navigated back. It repeats for each new capture folder that has a
  sibling. Annoyance and possible mis-typed input rather than data loss — but it hits every user.
  Fix (if you agree): Downgrade to MessageOptions.Both and let the banner be the affordance —
  which would also make m_suppressAffordanceDialog and its two try/finally blocks unnecessary. If
  you'd rather keep the dialog, raise it from Validated / SelectedIndexChanged instead of
  TextChanged so it can't interrupt mid-typing.

• [PR] Custom XEL file masks are copy-pasted and can silently drift apart.
  fmImport.CustomXelMasks re-declares *_SQLDIAG*.xel, *AlwaysOn_health*.xel,
  *system_health*.xel, which already exist as local variables inside
  CustomXELImporter.Load*Files(). They match today (verified), and a comment says "kept in sync",
  but nothing enforces it — if someone changes a mask in CustomXELImporter, the sibling-gap warning
  quietly stops working with no build error.
  Risk if not fixed: This is a silent-failure time bomb, and it fails in exactly the way this PR
  exists to prevent. If LogScout ever renames its output (or someone edits a mask), the copy in
  fmImport keeps matching the old pattern, the "these files exist only in SharedOutputFiles" warning
  stops firing, and users silently lose Custom XEL data with no indication anything is missing. No
  compiler error, no failing test — it just goes quiet. That's issue Enhance SQL Nexus to check an additional folder \SharedOutputFiles under the output for files to import #556 reintroduced in a new place.
  Fix: Promote the three masks to internal static readonly string[] (or three consts) on
  CustomXELImporter, use them in Load*Files(), and have fmImport reference that single source.

• [PR] RawFileImporter doesn't behave the way the PR description says it does.
  The description puts the size-aware rule under "Per-file importers (Rowset .OUT/.TXT, ErrorLog,
  RawFile): … same name + same size -> quiet skip; same name + DIFFERENT size -> one aggregated
  warning." fmImport does exactly that via FilterDuplicateSiblingFiles — but
  RawFileImporter.DoImport uses the name-only GetSiblingOnlyFiles, with no size comparison at all.
  So a raw file present in both folders with a different size is dropped with only a log line, and
  the user is never told a genuinely different file was skipped. That makes this a behavior/doc
  mismatch rather than just an internal inconsistency.
  Risk if not fixed: Two separate risks. (1) Data: a raw file that exists in both folders with
  genuinely different content is dropped, and the only trace is a log line — the status-bar half of
  that message is overwritten within moments, so in practice nobody sees it. Real diagnostic data goes
  missing silently, which is the Enhance SQL Nexus to check an additional folder \SharedOutputFiles under the output for files to import #556 failure mode again. (2) Trust: the PR description documents a
  safety net that doesn't exist for this importer, so a reviewer, maintainer, or support engineer
  debugging "why is this file missing?" reasons from a false model and looks in the wrong place.
  Fix: Switch RawFileImporter to FilterDuplicateSiblingFiles so both paths share one rule and
  one warning style. If name-only really is intended for raw files, correct the PR description instead
  and add a one-line comment saying why size doesn't matter there.

• [PR] An incorrect technical justification is baked into a project-file comment.
  SqlNexus.McpServer.csproj states "A strong-named test assembly cannot load an unsigned reference
  at runtime (FileLoadException)." That is not true on .NET Framework — a strong-named assembly can
  reference a weak-named one; CS8002 is only a warning and is already in <NoWarn>. The other half
  of the sentence (a signed assembly can only grant InternalsVisibleTo to a strong-named friend) is
  correct and is the real reason. Worth fixing so the next person doesn't inherit a false constraint.
  Risk if not fixed: That false claim is the stated justification for strong-naming two shipped
  executables. A maintainer who later wants to undo the signing change will read the comment, assume
  it's load-bearing, and leave it — making a possibly-unnecessary change to shipped binaries permanent.
  Someone hitting CS8002 elsewhere may also over-correct and strong-name more assemblies for no
  reason. Small blast radius, but it's misinformation baked into the build.
  Fix: Reword the comment to cite only the InternalsVisibleTo requirement. While you're there,
  double-check whether strong-naming sqlnexus.exe and SqlNexus.McpServer.exe was genuinely needed
  to get the tests compiling; if not, those two SignAssembly flips could be reverted, keeping just
  the test-project signing plus the key-qualified InternalsVisibleTo grants.

Test coverage

• [PR] Good core coverage, but three new code paths have none.
  SharedOutputFolder is nicely tested (~40 cases: happy path, no sibling, null/empty/whitespace,
  trailing separator, quoted input, drive root, UNC, case-insensitive folder name, .. normalization,
  duplicate classification, sibling-only detection, mask-skip rule, display/path composition). Not
  covered: RawFileImporter's new two-folder loop, WarnIfSharedFolderHasUnimportedCustomXel, and
  AbbreviatePath.
  Risk if not fixed: The three uncovered paths are the ones where a regression would be silent.
  RawFileImporter's loop is the code that actually touches user data — a mistake there means extra or
  missing rows in the imported tables, typically discovered much later during analysis, if ever.
  WarnIfSharedFolderHasUnimportedCustomXel is the only thing telling users about a known data gap,
  so if it breaks, users lose data with no warning. AbbreviatePath does index arithmetic
  (parts.Length - 2) on split paths, so an unusual or very short path could throw or render garbage
  in the UI. Note also that the solution currently can't be built or tested locally or in CI (the
  private NuGet feed returns 402 Payment Required for XELite / Azure.Identity \u2014 pre-existing
  and unrelated to this PR), so unit tests are effectively the only regression gate these paths have.
  Fix: Move AbbreviatePath out of fmImport into SharedOutputFolder (it's pure string logic
  with no WinForms dependency) and unit-test it — especially UNC paths, the maxLength boundary, and
  paths with fewer than two segments. Add temp-folder-based tests for RawFileImporter's
  primary-vs-sibling dedupe. The Custom XEL warning becomes testable once the mask constants are
  shared per the item above.

Nits

None of these can break an import. The shared risk is that four of them are comments or docs that
are now factually wrong — stale comments are worse than no comment, because the next person editing
this code will trust them and act on bad information.

• [PR] A comment says the label relies on AutoEllipsis to truncate, but AutoEllipsis is
  never set — and AutoSize = true would prevent it anyway.
  Risk: nobody adds real truncation because the comment says it's handled; a long UNC path then
  widens the banner and can push the header layout.
  Fix: Either set AutoEllipsis = true with AutoSize = false, or delete the claim from the comment.
• [PR] In DoImport, a comment says the header growth is reversed "BEFORE overriding the size
  below", but the this.Height = 650; this.Width = 1100; overrides are above that block.
  Risk: someone reorders that resize code trusting the comment and breaks the size math.
  Fix: Reword the comment to match the actual order.
• [PR] The SharedOutputFolderTests class summary still advertises that it "verifies the
  direct-sibling security guard (no directory traversal / non-sibling matches)", while a test in
  the same file documents that the guard was removed as dead code.
  Risk: a maintainer believes a traversal guard is in place and tested when neither is true.
  Fix: Update the class summary so it no longer describes a guard that no longer exists.
• [PR] laSharedFolder.TabIndex = 7 is set on a Label, which is never focusable.
  Risk: none functionally — just dead config that implies the label is in the tab order.
  Fix: Remove the TabIndex assignment.
• [PR] AddFileRow is now just a pass-through to AddFileRowReturningLabel.
  Risk: none — one extra hop to read through.
  Fix: Collapse into one method (let the existing callers ignore the return value).
• [PR] CountFilesInFolder runs a full directory enumeration on both folders purely to
  produce one log line, on top of the per-mask enumerations.
  Risk: on large captures (tens of thousands of files, or a slow/UNC share) that's two extra
  full directory scans, adding a visible stall before the import starts — for one log line.
  Fix: Only compute the counts when a sibling was actually found, or drop the counts from the message.
• [Existing] this.Height = 650; this.Width = 1100; in DoImport are hard-coded pixels, and the
  form uses AutoScaleMode.Font, so the expanded window is mis-sized at 125%/150% DPI. Pre-existing;
  the PR's own banner sizing correctly uses the measured control height instead.
  Risk: on a high-DPI laptop the expanded import window is wrong-sized — content clipped or the
  window larger than intended. Pre-existing, so not a blocker for this PR.
  Fix (follow-up): Replace the fixed pixel sizes with layout/Scale-aware sizing.

[JosephPilov-MSFT (PiJoCoder)]

Review — PR #558: Import sibling SharedOutputFiles folder during log import

How to read this list: items tagged [PR] were introduced by this PR and are the priority.
Items tagged [Existing] are pre-existing problems this PR merely touches or reveals — they are
explicitly not this PR's fault and can be deferred to a follow-up. Anything the PR description or
an earlier commit already handled has been dropped or narrowed below — nothing here re-litigates a
decision you've already documented.

Must fix

• [PR] The in-app Help update doesn't actually reach any user.
  The note was added to sqlnexus/Help/QuickStartPostmortemAnalysis.htm, but that page is an orphan:
  it is not listed in Help/sqlnexus2.hhp under [FILES], it is not a Content item in
  sqlnexus.csproj, and no other help page links to it. So the text ships to nobody, and the PR
  description's "in-app Help" claim is not accurate.
  Risk if not fixed: Users see import rows for files that are not in the folder they selected.
  With no Help entry explaining that, the natural reading is "Nexus is importing the wrong data" or
  "this list is buggy" — a support-case generator. Worse, the PR merges with a description asserting
  the documentation exists, so anyone later auditing "is this documented?" ticks the box and the gap
  never gets found.
  Fix: Move the SharedOutputFiles note into a help page that is actually in the .hhp [FILES]
  list (CollectingDiagnostics.htm or UsingSQLNexus.htm are the natural homes), then rebuild
  sqlnexus.chm from Help/sqlnexus2.hhp — the .chm is a checked-in binary, so editing the
  .htm alone changes nothing. Alternatively, drop the "in-app Help" line from the PR description.

We cannot rebuild .CHM file. Can you file a separate issue on this and we can address in the future to perhaps remove the .CHM file?

Should fix

• [PR] The new "Also scanning" banner ignores the app's theme engine.
  laSharedFolder hard-codes SystemColors.Info / InfoText in the designer and re-applies them
  at runtime specifically to overwrite ThemeManager.ApplyTheme. Every other control in that header
  panel is themed. In the Aquatic (dark, #202020) and Desert (#FFFAEF) themes, a pale
  yellow band will look out of place. This is the one place the PR visibly diverges from how the rest
  of the section renders.
  Risk if not fixed: On the dark Aquatic theme a bright pale-yellow block on #202020 reads as a
  rendering defect, and it sits on the first dialog users touch — expect it reported as a bug. It
  also sets a precedent that new controls may bypass ThemeManager, so the next control copies the
  pattern and theme coverage erodes. There's a latent trap too: fore and back colors are currently
  pinned together, so if someone later themes only one of them, the text becomes unreadable.
  Fix: Add an info-banner background/foreground color pair to ThemeManager for all three themes
  and have the banner read from ThemeManager.CurrentThemeName instead of hard-coding colors. Keep
  the existing SystemInformation.HighContrast behavior (defer to SystemColors) unchanged, and
  keep the literal "Also scanning:" text so meaning is never carried by color alone.

Already worked on this in previous iteration. I made the choice not to change this - documented in previous iteration.

• [PR] Reconsider the one remaining modal, on a user-driven path change.
  This is a deliberate choice, not an oversight — you already removed the form-open dialog and the PR
  description states the announcement "fires only on a user-driven path change." The concern with
  what's left: tbPath_TextChanged fires on every keystroke, so MessageOptions.All shows a
  MessageBox and steals focus the instant a typed path resolves to a folder with a sibling. The
  banner, tooltip, and cbPath.AccessibleDescription already convey the same information.
  (This does not affect automated runs — any command-line argument sets
  Globals.ConsoleMode = true, which suppresses dialogs.)
  Risk if not fixed: Because it fires from TextChanged, the dialog can appear mid-entry. Any
  keystrokes typed after it pops go to the dialog instead of the text box, so the path can end up
  truncated and the user has to retype it. For keyboard and screen-reader users, focus is yanked out
  of the field and has to be navigated back. It repeats for each new capture folder that has a
  sibling. Annoyance and possible mis-typed input rather than data loss — but it hits every user.
  Fix (if you agree): Downgrade to MessageOptions.Both and let the banner be the affordance —
  which would also make m_suppressAffordanceDialog and its two try/finally blocks unnecessary. If
  you'd rather keep the dialog, raise it from Validated / SelectedIndexChanged instead of
  TextChanged so it can't interrupt mid-typing.

Implemented - removed the Modal diaglog. Simpler and not needed reaally

• [PR] Custom XEL file masks are copy-pasted and can silently drift apart.
  fmImport.CustomXelMasks re-declares *_SQLDIAG*.xel, *AlwaysOn_health*.xel,
  *system_health*.xel, which already exist as local variables inside
  CustomXELImporter.Load*Files(). They match today (verified), and a comment says "kept in sync",
  but nothing enforces it — if someone changes a mask in CustomXELImporter, the sibling-gap warning
  quietly stops working with no build error.
  Risk if not fixed: This is a silent-failure time bomb, and it fails in exactly the way this PR
  exists to prevent. If LogScout ever renames its output (or someone edits a mask), the copy in
  fmImport keeps matching the old pattern, the "these files exist only in SharedOutputFiles" warning
  stops firing, and users silently lose Custom XEL data with no indication anything is missing. No
  compiler error, no failing test — it just goes quiet. That's issue Enhance SQL Nexus to check an additional folder \SharedOutputFiles under the output for files to import #556 reintroduced in a new place.
  Fix: Promote the three masks to internal static readonly string[] (or three consts) on
  CustomXELImporter, use them in Load*Files(), and have fmImport reference that single source.

Implemented - simplifies things

• [PR] RawFileImporter doesn't behave the way the PR description says it does.
  The description puts the size-aware rule under "Per-file importers (Rowset .OUT/.TXT, ErrorLog,
  RawFile): … same name + same size -> quiet skip; same name + DIFFERENT size -> one aggregated
  warning." fmImport does exactly that via FilterDuplicateSiblingFiles — but
  RawFileImporter.DoImport uses the name-only GetSiblingOnlyFiles, with no size comparison at all.
  So a raw file present in both folders with a different size is dropped with only a log line, and
  the user is never told a genuinely different file was skipped. That makes this a behavior/doc
  mismatch rather than just an internal inconsistency.
  Risk if not fixed: Two separate risks. (1) Data: a raw file that exists in both folders with
  genuinely different content is dropped, and the only trace is a log line — the status-bar half of
  that message is overwritten within moments, so in practice nobody sees it. Real diagnostic data goes
  missing silently, which is the Enhance SQL Nexus to check an additional folder \SharedOutputFiles under the output for files to import #556 failure mode again. (2) Trust: the PR description documents a
  safety net that doesn't exist for this importer, so a reviewer, maintainer, or support engineer
  debugging "why is this file missing?" reasons from a false model and looks in the wrong place.
  Fix: Switch RawFileImporter to FilterDuplicateSiblingFiles so both paths share one rule and
  one warning style. If name-only really is intended for raw files, correct the PR description instead
  and add a one-line comment saying why size doesn't matter there.

Do nothing to code; fix the doc is the option, but not going to bother at this point.

• [PR] An incorrect technical justification is baked into a project-file comment.
  SqlNexus.McpServer.csproj states "A strong-named test assembly cannot load an unsigned reference
  at runtime (FileLoadException)." That is not true on .NET Framework — a strong-named assembly can
  reference a weak-named one; CS8002 is only a warning and is already in <NoWarn>. The other half
  of the sentence (a signed assembly can only grant InternalsVisibleTo to a strong-named friend) is
  correct and is the real reason. Worth fixing so the next person doesn't inherit a false constraint.
  Risk if not fixed: That false claim is the stated justification for strong-naming two shipped
  executables. A maintainer who later wants to undo the signing change will read the comment, assume
  it's load-bearing, and leave it — making a possibly-unnecessary change to shipped binaries permanent.
  Someone hitting CS8002 elsewhere may also over-correct and strong-name more assemblies for no
  reason. Small blast radius, but it's misinformation baked into the build.
  Fix: Reword the comment to cite only the InternalsVisibleTo requirement. While you're there,
  double-check whether strong-naming sqlnexus.exe and SqlNexus.McpServer.exe was genuinely needed
  to get the tests compiling; if not, those two SignAssembly flips could be reverted, keeping just
  the test-project signing plus the key-qualified InternalsVisibleTo grants.

Fixed the comment

Test coverage

• [PR] Good core coverage, but three new code paths have none.
  SharedOutputFolder is nicely tested (~40 cases: happy path, no sibling, null/empty/whitespace,
  trailing separator, quoted input, drive root, UNC, case-insensitive folder name, .. normalization,
  duplicate classification, sibling-only detection, mask-skip rule, display/path composition). Not
  covered: RawFileImporter's new two-folder loop, WarnIfSharedFolderHasUnimportedCustomXel, and
  AbbreviatePath.
  Risk if not fixed: The three uncovered paths are the ones where a regression would be silent.
  RawFileImporter's loop is the code that actually touches user data — a mistake there means extra or
  missing rows in the imported tables, typically discovered much later during analysis, if ever.
  WarnIfSharedFolderHasUnimportedCustomXel is the only thing telling users about a known data gap,
  so if it breaks, users lose data with no warning. AbbreviatePath does index arithmetic
  (parts.Length - 2) on split paths, so an unusual or very short path could throw or render garbage
  in the UI. Note also that the solution currently can't be built or tested locally or in CI (the
  private NuGet feed returns 402 Payment Required for XELite / Azure.Identity \u2014 pre-existing
  and unrelated to this PR), so unit tests are effectively the only regression gate these paths have.
  Fix: Move AbbreviatePath out of fmImport into SharedOutputFolder (it's pure string logic
  with no WinForms dependency) and unit-test it — especially UNC paths, the maxLength boundary, and
  paths with fewer than two segments. Add temp-folder-based tests for RawFileImporter's
  primary-vs-sibling dedupe. The Custom XEL warning becomes testable once the mask constants are
  shared per the item above.

Nits

None of these can break an import. The shared risk is that four of them are comments or docs that are now factually wrong — stale comments are worse than no comment, because the next person editing this code will trust them and act on bad information.

• [PR] A comment says the label relies on AutoEllipsis to truncate, but AutoEllipsis is
  never set — and AutoSize = true would prevent it anyway.
  Risk: nobody adds real truncation because the comment says it's handled; a long UNC path then
  widens the banner and can push the header layout.
  Fix: Either set AutoEllipsis = true with AutoSize = false, or delete the claim from the comment.
• [PR] In DoImport, a comment says the header growth is reversed "BEFORE overriding the size
  below", but the this.Height = 650; this.Width = 1100; overrides are above that block.
  Risk: someone reorders that resize code trusting the comment and breaks the size math.
  Fix: Reword the comment to match the actual order.
• [PR] The SharedOutputFolderTests class summary still advertises that it "verifies the
  direct-sibling security guard (no directory traversal / non-sibling matches)", while a test in
  the same file documents that the guard was removed as dead code.
  Risk: a maintainer believes a traversal guard is in place and tested when neither is true.
  Fix: Update the class summary so it no longer describes a guard that no longer exists.
• [PR] laSharedFolder.TabIndex = 7 is set on a Label, which is never focusable.
  Risk: none functionally — just dead config that implies the label is in the tab order.
  Fix: Remove the TabIndex assignment.
• [PR] AddFileRow is now just a pass-through to AddFileRowReturningLabel.
  Risk: none — one extra hop to read through.
  Fix: Collapse into one method (let the existing callers ignore the return value).
• [PR] CountFilesInFolder runs a full directory enumeration on both folders purely to
  produce one log line, on top of the per-mask enumerations.
  Risk: on large captures (tens of thousands of files, or a slow/UNC share) that's two extra
  full directory scans, adding a visible stall before the import starts — for one log line.
  Fix: Only compute the counts when a sibling was actually found, or drop the counts from the message.
• [Existing] this.Height = 650; this.Width = 1100; in DoImport are hard-coded pixels, and the
  form uses AutoScaleMode.Font, so the expanded window is mis-sized at 125%/150% DPI. Pre-existing;
  the PR's own banner sizing correctly uses the measured control height instead.
  Risk: on a high-DPI laptop the expanded import window is wrong-sized — content clipped or the
  window larger than intended. Pre-existing, so not a blocker for this PR.
  Fix (follow-up): Replace the fixed pixel sizes with layout/Scale-aware sizing.

At this point, small things these Nits. We'll leave them for some other time.