bug(798): fail loudly on QuickFiler column-add timeout and guard the ribbon boundary

[drmoisan]

Summary

Fixes the QuickFiler launch crash on a folder named "T&E". AddQfcColumnsAsync swallowed three consecutive 3-second timeouts and returned normally, the ETL then proceeded with Outlook's five default columns, and the row builder indexed the missing SentOn key. The resulting AggregateException wrapping KeyNotFoundException was rethrown with throw e, losing the original stack, and escaped an async void ribbon handler with no boundary catch, surfacing to Outlook as an unhandled exception.

Closes #798.

Root cause

The defect was a reporting gap, not a single bad line. Between the accept point and the observed symptom there was no guard at all:

1. Accept point. GetEmailDataInViewAsync awaited AddQfcColumnsAsync, which returns a bare Task. A completed await was the only success signal available, and it validated nothing about the resulting column set.
2. Absorption point. On the third timeout the counter < 2 guard was false, the catch (TimeoutException) body was empty, and the method completed successfully. No exception, no return value, no log line — the method had no failure channel at all.
3. Throw point. Roughly 250 ms later and two layers away, Email2dToRecords indexed columnInfo["SentOn"]. The exception named neither the folder nor the timed-out step, so the symptom was unrelatable to the cause.

A second, independent defect compounded it: TimeoutAfter does not cancel the underlying task, and the retry started a new Task.Run per attempt, so up to three AddQfcColumns invocations could run concurrently against one COM Table from pool threads.

What changed

Six production files, five test files, five project compile-entry files — 16 paths, fixed by AC13 and verified by an anchored diff against the merge base.

• UtilitiesCS — the four column methods were relocated verbatim into a new partial, DfDeedle.QfcColumns.cs. The recursive retry was replaced by a single held task re-deadlined up to three times, so the work starts exactly once and the deadline is re-applied to that same instance; exhaustion now throws a descriptive exception naming the folder and the step. Per-step timing instrumentation was added. A new ValidateRequiredEmailColumns guard is called from both the asynchronous and the synchronous entry points, closing a duplicate unchecked indexing path that the async fix alone would have missed.
• QuickFiler — throw e; became throw; at all three sites in the QfcDatamodel partial family, preserving the original stack.
• TaskMaster — a new RibbonCommandBoundary type wraps the three named ribbon handlers, logging with full detail and presenting a dialog that renders inner-exception detail, so an AggregateException no longer presents as "One or more errors occurred." alone.

The existing four TimeoutAfter overloads are unchanged and no new overload was added. No Task.Delay or Thread.Sleep was introduced anywhere.

Verification

Full toolchain, clean on the first pass with no restart:

Step	Command	Result
Format	dotnet tool run csharpier check .	exit 0, Checked 1593 files in 6008ms.
Analyze	msbuild /t:Rebuild with analyzers enforced	exit 0, 0 Error(s), 0 Warning(s)
Type-check	msbuild /t:Rebuild with warnings as errors	exit 0, 0 Error(s), 0 Warning(s)
Test	full suite with coverage	exit 0, 7048 total, 7048 passed, 0 failed, 0 skipped

The test count reconciles against the 7023-test baseline plus the 25 tests this change adds. /t:Rebuild was used deliberately: a warm /t:Build returns exit 0 with CoreCompile skipped on every project and runs no analyzers. 19 projects and 71 of 74 CoreCompile executions confirm the analyzers actually ran.

Every acceptance criterion carries fail-before / pass-after evidence under the feature folder's evidence/ tree.

Coverage

Change-scoped obligations, all met:

• DfDeedle.QfcColumns.cs — 164/168 lines, rate 0.9762 (obligation: >= 0.90)
• RibbonCommandBoundary.cs — 55/61 lines, rate 0.9016 (obligation: >= 0.90)
• Relocation-adjusted: the source file plus its new partial cover 321 lines against a baseline of 226 for the file they were split from. This adjustment is necessary because a per-file comparison on the source alone would compare different denominators.

Repository-wide line coverage moved up, 0.85827 to 0.85869.

Review guide

Four things a reviewer should know before reading the generated context artifact, because the collector reports them incorrectly:

1. "Core logic changes: 0 files" is wrong. Eleven C# files changed. The classifier ranks by churn, and the two committed Cobertura documents (671k and 668k added lines) displace every source file from the top-N list.
2. "GitHub CLI unavailable" is wrong. The collector runs without gh on its PATH; gh is installed and authenticated.
3. The author-asserted autoclose list scrapes three issue numbers. Only Bug: quickfiler-crash-column-add-timeout-swallowed-keynotfound #798 is closed by this pull request. The other two numbers appear in prose only: one names the pre-existing sporadic test tracked separately, the other is referenced by the research record. Neither is addressed here.
4. Three evidence rows render as Normalized result: fail, and none of them is a failure. Each of those artifacts records several gates but carries a single file-level ExpectedExitCode: 1 that describes its last gate — a scoped test run that legitimately exits 1 because later-phase tests were still red at that point. The collector pairs the file-level expectation with the first command it parses, which exits 0, so the row inverts. This is the documented per-file limitation of the expectation field; the correct authoring is one artifact per gate needing a non-zero expectation. It is recorded as a non-blocking nit rather than repaired, because rewriting committed evidence after the fact would be worse than annotating it.

Feature review

Verdict PASS — 0 blocking findings, 11 non-blocking.

The review surfaced one obligation that no execution artifact reported: repository-wide branch coverage reads 0.6612 against the 0.75 uniform floor, up from a 0.6605 baseline. The coverage evidence reported line coverage only, on both sides. The condition is pre-existing and moves in the correct direction, so it is dispositioned non-blocking, but it was invisible until the reviewer parsed the root element of both Cobertura documents directly.

The review also identified exactly what would close the RibbonCommandBoundary margin: the six uncovered lines are four untested scenarios the unit-test policy requires — a throwing log sink (the current test throws from the presentation sink only), an AggregateException wrapping nothing, and the two ArgumentNullException guards. Four small tests take the file to 61/61.

Outstanding

• AC6 is not checked off. It requires launching QuickFiler on the reproduction folder in a live Outlook process, which no automated test can supply. Manual steps are handed off in evidence/other/ac6-manual-verification-handoff.md. This is the only one of the 14 criteria not met: 13 met, 1 pending manual verification.
• Three follow-up findings are recorded in evidence/other/followup-promotions.md for promotion after merge: an unreachable catch (TimeoutException) in the two repeatAttempts timeout overloads, an unguarded shared-static seam mutation in an existing COM test class, and that same class's pre-existing 500-line-cap violation. They were deliberately not promoted on this branch: creating a promotion file here would add a seventeenth path and falsify the write-set gate this change asserts.

Risk

The behavioural change is deliberate and worth stating plainly: a column-add that exhausts its budget now throws where it previously returned normally. Callers that relied on the silent-success path will now see an exception — which is the point of the fix, since that silent success is what produced the crash three layers away. The narrow catch (OperationCanceledException) in QfcHomeController.LaunchAsync was left exactly as narrow as it was, so user cancellation stays silent and does not reach the new dialog.