Activity

1. drmoisan

   added

   bugSomething isn't workingSomething isn't working

   on Aug 26, 2026
2. drmoisan commented on Aug 26, 2026

   

   drmoisan

   on Aug 26, 2026

   OwnerAuthor

   More actions

   qfc-unsynchronized-undo-handoff-after-batch-move (Issue #633)

   • Date captured: 2026-08-26
   • Author: Dan Moisan
   • Status: Promoted -> docs/features/active/qfc-unsynchronized-undo-handoff-after-batch-move/ (Issue Bug: qfc-unsynchronized-undo-handoff-after-batch-move #633)
   • Captures: follow-up candidate 7 of ## Follow-up Candidates in
     docs/features/active/qfc-collection-controller-defects-468/spec.md
   • Origin: issue Bug: qfc-collection-controller-unreachable-load-paths #468 defect family, task [P14-T5]

   Automation note: Keep the section headings below unchanged; the promotion tooling maps each of them into the GitHub bug issue template.

   • Issue: Bug: qfc-unsynchronized-undo-handoff-after-batch-move #633
   • Issue URL: Bug: qfc-unsynchronized-undo-handoff-after-batch-move #633
   • Last Updated: 2026-08-26

   Summary

   The batch-move path treats the undo stack as populated by the time the move completes, but the push is
   performed asynchronously on a queue worker and may not have happened yet.
   MoveMailAsync only enqueues the filer
   (QuickFiler/Controllers/QfcItemController.MailActions.cs:111) and returns Task.CompletedTask
   (:112). The push onto the global undo stack happens later, on the queue's worker. So when
   BackGroundMoveAsync proceeds to WriteMetrics
   (QuickFiler/Controllers/QfcFormController.EventHandlers.cs:228-231) and then CleanupBackground()
   (:233), the undo entries for that batch may not yet exist.

   This does not break undo in the observed configuration — the entries land eventually and are
   serialized — but the handoff is unsynchronized, and nothing in the code expresses the ordering it
   relies on.

   Environment

   • OS/version: Windows 11 Pro 10.0.26200
   • Framework: .NET Framework 4.8.1, VSTO Outlook add-in
   • Command/flags used: not reproducible from a command line; requires a live Outlook session and a batch move
   • Data source or fixture: any QuickFiler batch move of two or more emails

   Steps to Reproduce

   1. Select two or more emails in a QuickFiler session and assign destination folders.
   2. Confirm the move, so BackGroundMoveAsync runs.
   3. Observe the contents of the global undo stack at the moment CleanupBackground() is reached.

   Expected Behavior

   Either the batch-move completion awaits the undo pushes for that batch, or the ordering dependency is
   made explicit so that a future change to WriteMetrics or CleanupBackground cannot start depending
   on entries that are not yet present.

   Actual Behavior

   CleanupBackground() may run while some or all of the batch's undo entries are still queued. Nothing
   observes the gap today, so the defect is latent rather than active.

   Logs / Screenshots

   • Attached minimal logs or screenshot
   • Snippet: no captured log; identified by triage during issue Bug: qfc-collection-controller-unreachable-load-paths #468, recorded at
     docs/features/active/qfc-collection-controller-defects-468/spec.md:1040-1049
     ("Deferred observation — unsynchronized undo handoff").

   Impact / Severity

   • Blocker
   • High
   • Medium
   • Low

   Latent. The entries land eventually and are serialized, so undo works in the observed configuration.
   The severity comes from the absence of any expressed ordering constraint: a future caller that reads
   the stack immediately after a batch move would see an incomplete stack with no diagnostic.

   Suspected Cause / Notes

   MoveMailAsync returning Task.CompletedTask after an enqueue makes the operation look synchronous
   to its awaiter while the real work is still pending. That is the same shape as an async void
   boundary: the caller has no handle on the work it started.

   Files to inspect: QuickFiler/Controllers/QfcItemController.MailActions.cs,
   QuickFiler/Controllers/QfcFormController.EventHandlers.cs,
   UtilitiesCS/EmailIntelligence/EmailParsingSorting/EmailFiler.cs.

   This observation is explicitly out of scope for all seven issues in the #468 family, because it lives
   entirely outside QuickFiler/Controllers/QfcCollectionController.cs.

   Proposed Fix / Validation Ideas

   • Unit coverage areas: a test that enqueues two moves through a fake filer queue and asserts that
     the completion signal the caller awaits does not complete before both pushes have run
   • Integration scenario to retest: a batch move of several emails followed immediately by an undo
   • Manual verification notes: confirm the metrics written by WriteMetrics are unaffected by any
     added synchronization

   Next Step

   • Promote to GitHub issue (bug-report template)
   • Move to active fix folder / branch

   Source: triage during issue #468, deliberately not absorbed because it touches no file in that
   feature's owned set.
3. drmoisan commented on Sep 1, 2026

   

   drmoisan

   on Sep 1, 2026

   OwnerAuthor

   More actions

   Fix delivered on bug/qfc-unsynchronized-undo-handoff-after-batch-move-633

   The unsynchronized undo handoff is closed. The batch-move path now expresses its ordering dependency as
   a control-flow property rather than an assumption.

   What changed

   QuickFiler/Controllers/FilerQueue.cs

   • Added public Task WhenDrainedAsync(), a counted, per-batch, awaitable quiesce. It returns an
     already-completed task when nothing is outstanding, and otherwise a task that completes when the
     outstanding-work count next reaches zero. It is idempotent and safe to await repeatedly or
     concurrently, and it completes rather than faulting, so a logged item failure is not converted into an
     unhandled exception on the batch-move path.
   • Repaired the producer/consumer handshake. The ThreadSafeSingleShotGuard start gate is replaced
     by a start/stop decision taken under a single monitor. The consumer-running flag is now cleared in the
     same critical section in which TryTake fails, which closes the orphaned-item window: previously a
     producer whose Queue.Add landed between the worker's loop exit and its guard reinstall read the
     already-tripped guard, started no worker, and left its item stranded. This repair is a precondition
     for a sound barrier, not an opportunistic refactor — a barrier over the old handshake would have
     reported "drained" while an item was stranded, or never completed at all.
   • The outstanding-work counter is decremented in a finally, so a throwing item still decrements and
     the drain cannot hang. The existing per-item catch, its item.Helpers.First() diagnostic, and its
     logger.Error call are preserved unchanged.
   • Added an internal Func<FilerQueueItem, Task> ItemProcessor seam whose production default preserves
     the existing call. It exists so the queue can be driven deterministically from a unit test; the real
     EmailFiler.SortAsync is non-virtual and casts to a COM folder.
   • Consumer is retained with its type, accessibility and Task.CompletedTask default. The change is
     additive on the public surface.

   QuickFiler/Controllers/QfcFormController.EventHandlers.cs

   • BackGroundMoveAsync now awaits _parent.FilerQueue.WhenDrainedAsync() between the batch move and
     the WriteMetrics dispatch. There is no longer any control-flow path from a completed batch move to
     WriteMetrics or CleanupBackground that does not pass through the barrier. Metrics-before-cleanup
     order is unchanged.
   • Added a _parent null check to the method's early-return guard, required because the barrier
     dereferences _parent and cleanup sets it to null.
   • Deleted the two now-subsumed await _parent.FilerQueue.Consumer; statements. Both were strictly
     subsumed: each was immediately preceded by an await of the same BackGroundMoveAsync task, and the
     barrier waits on the whole outstanding count rather than on one worker task.

   Verification

   The defect carries a genuine fail-before / pass-after pair. Both
   BackGroundMoveAsync_WithPendingQueueItem_DoesNotWriteMetricsBeforeDrain and
   ...DoesNotDispatchCleanupBeforeDrain failed against the pre-fix tree — with one item parked behind a
   closed gate, the metrics recorder count was deterministically 1 by the time an equal-priority dispatcher
   probe completed — and both pass after the fix. Determinism comes from dispatcher enqueue order and
   TaskCompletionSource gates, never from a sleep, delay, poll, or timeout.

   Twelve tests were added: seven queue-level cases covering the drain contract, the orphaned-item
   regression, and a throwing processor; and five ordering cases covering both barrier paths, the
   metrics-then-cleanup order, and both guard branches.

   Toolchain, in one uninterrupted pass: csharpier check reports 0 unformatted files; both
   msbuild /t:Rebuild gates exit 0 with zero Skipping target "CoreCompile" occurrences in their logs;
   6924 tests run, 6924 passed, 0 failed. Repository-wide line coverage moved from 85.32 percent to 85.39
   percent over the same filtered denominator. FilerQueue.cs reaches a per-file rate of 1.00, and zero of
   the 138 changed production lines are uncovered.

   The production diff touches only the two files named above.

   Follow-up raised separately

   UtilitiesCS.Test/.../DASLFilterParserTests.PrintTree_WritesIndentedTreeToConsole redirects
   process-global Console.Out and has no [DoNotParallelize] attribute, so a sibling class's
   Console.SetOut can clobber its redirect under the class-level parallel scope. It failed once during
   this work and passed in isolation and on re-run. The identical hazard is already mitigated on
   PrettyPrint_Tests, which carries [DoNotParallelize] with an explanatory comment. This is unrelated
   to #633 and outside its authorized scope, so it was not fixed here and should be raised as its own
   issue.
4. drmoisan

   added a commit that references this issue on Sep 1, 2026

   Merge pull request #717 from drmoisan/bug/qfc-unsynchronized-undo-han…

   ...

   8996b28
5. drmoisan

   closed this as completedin #717on Sep 1, 2026