Skip to content

Bug: efc-form-controller-lifecycle-and-selection-defects #465

Closed

Description

@drmoisan
  • Work Mode: full-bug

Summary

Four independent defects in EfcFormController: a non-idempotent Cleanup() reachable twice from one
user gesture, a cross-thread WinForms control read inside Task.Run, duplicate "Trash to Delete" rows
accumulating on repeated delete actions, and an inconsistent banner-prefix test across three sites.

Environment

  • OS/version: Windows 11 Pro 10.0.26200
  • Runtime: .NET Framework 4.8.1 WinForms VSTO add-in
  • UI path: QuickFiler/Controllers/EfcFormController.cs
  • Data source or fixture: an Email Filer session with folder suggestions populated

Steps to Reproduce

A - double cleanup. Trigger the OK action with the Return key while the OK button Click
subscription is also live; observe the second ActionOkAsync dereference a nulled field.

C - duplicate trash rows. Invoke the delete action twice (via the 'T' keyboard action or the
delete button) and observe two "Trash to Delete" rows in the folder list.

Expected Behavior

  • Cleanup() is idempotent, or is guarded so it cannot run twice for one gesture.
  • WinForms control properties are read only on the UI thread.
  • The trash row appears at most once regardless of how many times delete is invoked.
  • A row's banner-prefix classification is identical everywhere it is tested.

Actual Behavior

A - Cleanup() is not idempotent and has no re-entrancy guard (EfcFormController.cs:189-196).
A second invocation dereferences the already-nulled _globals at :191. Two independent paths can
invoke the OK action for a single user gesture: the always-on Return key binding (:365,
new KaKeyAsync("Collection", Keys.Return, (k) => ActionOkAsync())) and the OK button Click
subscription (:391). If both fire, the second ActionOkAsync throws NullReferenceException at
:705 on the nulled _formViewer. EfcHomeController.TryBeginExecuteMoves
(EfcHomeController.ExecuteMoves.cs:48-57) shows the _isExecuting-style guard this path lacks.

B - cross-thread control read (EfcFormController.cs:800-803). RefreshSuggestionsAsync
evaluates _formViewer.SearchText.Text inside the Task.Run lambda. Reading Control.Text off
the UI thread is an illegal cross-thread control access. SearchText_TextChanged (:558) reads the
same property correctly on the UI thread.

C - duplicate trash rows (EfcFormController.cs:742-750). ActionDeleteAsync reads _folderRows,
inserts "Trash to Delete" at index 0, and rebinds. BindFolderRows (:881) then stores the result

  • which now contains the trash row - back into _folderRows. A second invocation inserts a second
    trash row. No dedupe guard exists.

D - inconsistent banner-prefix detection. IsValidSelection (:1049) tests
Substring(0, 3) == "===" (three characters); ActionOkAsync (:708) tests StartsWith("====")
(four); BreadcrumbRowBuilder.BannerPrefix (BreadcrumbRowBuilder.cs:19) is "====" (four). A row
beginning with exactly three = is rejected by IsValidSelection but accepted by ActionOkAsync and
classified as a suggestion by the row builder.

Logs / Screenshots

  • Attached minimal logs or screenshot
  • Code-read evidence recorded above (verified 2026-08-07 against the working tree).

Impact / Severity

  • Blocker
  • High
  • Medium
  • Low

Defect A is an unhandled exception on the primary filing gesture. Defect B is an illegal cross-thread
access whose symptom is intermittent and environment-dependent, which makes it hard to attribute.

Source

From: docs/features/potential/2026-08-07-efc-form-controller-lifecycle-and-selection-defects.md

Activity

  1. added a commit that references this issue on Aug 28, 2026
  2. drmoisan commented on Aug 30, 2026

    @drmoisan
    OwnerAuthor

    Closing as already delivered.

    This issue's four claimed defects were re-verified directly against QuickFiler/Controllers/EfcFormController.cs on main during preparation for a parallel-run admission (2026-08-30):

    • Defect A (non-idempotent Cleanup()): fixed. Cleanup() performs a null-conditional detach and clears _parentCleanup before invoking it.
    • Defect B (cross-thread control read): fixed. RefreshSuggestionsAsync reads SearchText.Text into a local on the UI thread before either Task.Run closure uses it.
    • Defect C (duplicate "Trash to Delete" rows): fixed. The write-back into _folderRows that caused duplication is gone; retention moved to BindSourceFolderRows, which the delete path does not call.
    • Defect D (inconsistent banner-prefix check): fixed. A single IsBannerRow helper using StartsWith is now the only check; no Substring(0, 3) comparison remains.

    All four fixes landed under the sibling feature efc-controller-surface-defects-464 (PR #661), which is why nothing here auto-closed — the commit subjects reference that feature's slug, not this issue number.

    No branch or parallel-run item is needed. The preparation branch and worktree created for this admission attempt have been deleted (no unique work was lost; only a user-story draft had been written before the admission was rejected on this evidence).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions