Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .claude/agent-memory/atomic-executor/MEMORY.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Atomic Executor Memory Index

## Plan validation & gates
- [Verify line citations with numbered output](feedback_verify_line_citations_with_numbered_output.md) — never hand-count from a `sed` window; a wrong #438 advisory got applied and corrupted 3 correct citations in plan + spec
- [Self-derived gate thresholds are blind](project_preflight_selfderived_gate_thresholds_are_blind.md) — a "count >= floor" gate whose floor comes from the runs it validates is deflation-blind + scope-incommensurable; use git-enumeration + `/ListTests` existence proofs
- [Merge-base diff gates need a commit cadence](project_preflight_mergebase_diff_gates_need_commit_cadence.md) — `<MERGE_BASE>..HEAD` gates are vacuous while HEAD == merge-base and unsatisfiable once HEAD is ahead; on a later cycle scope-audit via `git show --numstat --format= HEAD`
- [Inserted plan tasks force renumbering](project_plan_task_ids_digit_only_forces_renumbering.md) — suffixed IDs (`P3-T5a`) fail validation; say "insert + renumber downstream", then verify defs-vs-mentions mechanically
Expand Down Expand Up @@ -34,6 +35,8 @@
- [Compile-time red needs body-level refs](project_compile_red_needs_body_level_references.md) — a missing type in a method SIGNATURE suppresses body binding, so an `[expect-fail]` task requiring N named CS0246s reports only 1; construct the types inline in test bodies

## Test execution & isolation
- [Tests must mock GUI; no visible window](feedback_tests_must_mock_gui_no_visible_window.md) — user saw a window pop up during a QuickFiler.Test run; new tests must use headless seams (Mock<IItemViewer>, injected show/focus delegates, host-neutral types), never Form.Show/Application.Run
- [WinFormsPumpHost tests are load-flaky](project_winformspumphost_tests_load_flaky.md) — QfcItemController_InitializationTests fail with "window handle has been created"/60s timeouts when the box is CPU-saturated; re-run when load drops, don't treat as a red baseline
- [vstest /InIsolation + FilePathHelper serialization](project_vstest_isolation_and_filepathhelper_serialization.md) — Moq assemblies need /InIsolation (else STTE Setup FileNotFound); FilePathHelper.FilePath is "" default but null after JSON deserialize
- [Invoke-MSTest.ps1 dies on a single test assembly](project_418_invoke_mstest_single_assembly_bug.md) — StrictMode + `.Count` on a scalar String throws before vstest runs; call vstest.console.exe directly with the script's arg list
- [Timed-out MSTest leaves detached runner](project_timedout_mstest_leaves_detached_runner.md) — leaves a pwsh runner respawning testhosts → user.config hangs; kill the pwsh runner too, verify 0, rerun >=8min
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
name: tests-must-mock-gui-no-visible-window
description: New/changed C# tests must mock GUI elements behind seams; no real window may appear during a test run
metadata:
type: feedback
---

Any test this agent develops or changes must mock GUI elements behind appropriate seams. No visible window, form, or popup may appear while the test suite runs.

**Why:** The user observed a window popping up during a full `QuickFiler.Test` run (2026-08-08). A visible window means a real WinForms host is being created and pumped instead of being substituted at a seam. Besides being disruptive, it makes tests machine-, focus-, and load-dependent — the same `WinFormsPumpHost`-based tests in `QuickFiler.Test/TestSupport/WinFormsPumpHost.cs` are also the ones that fail with "Invoke or BeginInvoke cannot be called on a control until the window handle has been created" and with 60s `[Timeout]` expiries under machine load.

**How to apply:** In this repo the established headless seams are:
- `Mock<IItemViewer>` at the controller seam.
- `Mock<IBreadcrumbDropDownHost>` plus the headless `ItemViewerDropDownHarness` (constructs a `UserControl` but never shows it, and replaces the show/focus delegates).
- The `BreadcrumbDropDownHostTests` private `Harness`, which injects `showPopup` / `focusPending` / `focusAnchor` as counting delegates so no native popup is ever shown.
- Host-neutral `UtilitiesCS.OutlookObjects.Folder` types (`BreadcrumbSelectionSession`, `FolderBreadcrumbBridgeRouter`, `BreadcrumbStateModel`) which have no UI dependency at all.

Never call `Form.Show`, `Application.Run`, `Control.CreateControl`, or drive a real message pump in a new test. Prefer the host-neutral layer; when a `Control` is unavoidable, inject the show/focus operations as delegates and assert call counts. See [[project_configcontroller_sta_pump_deadlock]] and [[project_uithread_dispatcher_static_swap_race]] for the failure modes this prevents.
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
name: verify-line-citations-with-numbered-output
description: Never assert a file:line citation from an unnumbered sed/head window — always use grep -n "" or Read; a hand-counted preflight finding got applied and corrupted three correct citations
metadata:
type: feedback
---

When asserting any `file:line` or `file:start-end` citation in a preflight finding, audit, or
review, derive it from **line-numbered output** (`grep -n "" <file> | sed -n 'A,Bp'`, the Grep
tool with `-n`, or Read). Never hand-count offsets from a `sed -n 'A,Bp'` / `head` window.

**Why:** During #438 preflight cycle 1, I hand-counted line offsets from a `sed -n '300,395p'`
window and reported three citations as off-by-one (advisory "A3"). All three were already
correct. The planner faithfully applied my delta in revision 1, which *corrupted* previously
correct citations in both the plan and `spec.md` — `EventHandlers.cs:177` became `:178` (a
closing brace), the sanctioned test span `313-350` became `313-349` (dropping the method's
closing brace), and `355-388` became `353-387`. Cycle 2 caught it only because I re-verified
with `grep -n ""`. The coordinator's course correction was explicit: "this time verify any
line citation against line-numbered output before asserting it."

**How to apply:**
- Applies to every preflight/audit/review finding, not just blocking ones. A downstream planner
treats even an advisory as authoritative and will apply it verbatim.
- Blank lines and attribute lines are the usual miscount source: a `[TestMethod]` attribute, the
`/// <summary>` opener, and the trailing `}` are each easy to slide by one.
- A method-span citation convention in this repo includes the attribute line through the closing
brace (e.g. `313-350`), not the signature through the last statement.
- If a citation looks wrong, re-read with numbers before writing the delta; the cost of one extra
numbered read is far below the cost of a corrupted plan revision cycle.
- Related: [[project_418_plan_rationale_clauses_are_evidence]] — unmeasured world-state claims in
plan prose are the other recurring source of preflight churn.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
name: winformspumphost-tests-load-flaky
description: QuickFiler.Test WinFormsPumpHost tests fail nondeterministically when the machine is CPU-saturated; not a red baseline
metadata:
type: project
---

`QuickFiler.Test/TestSupport/WinFormsPumpHost.cs`-based tests (notably `QfcItemController_InitializationTests.Initialize*_ThroughThePumpHost_*`) fail nondeterministically with either `InvalidOperationException: Invoke or BeginInvoke cannot be called on a control until the window handle has been created` or a 60s `[Timeout]` expiry. Observed 2026-08-08 at HEAD 904b4c38 with a byte-clean source tree.

**Why:** The pump host creates a real WinForms control and drives a message loop. With MSTest `Workers: 0` (24 on this box, `scripts/vscode/TaskMaster.cli.runsettings`) plus `dotnet-coverage` instrumentation, handle creation can lose the race when the machine is CPU-saturated. Measured across five full-suite runs on a box at ~96% CPU (a `node` process at 207k CPU-seconds, several VS Code Insiders windows, a second `claude` session, Docker): run 1 hung outright, run 2 = 1 failure in 40.7s, run 3 = 7 timeout failures in 5.9min, runs 4-5 = the same 2 failures in ~40s. Three consecutive isolated runs of the class failed 2/9; a fourth, minutes later, passed 9/9.

**How to apply:** Do not classify these as a pre-existing red baseline on the first red run. Check machine load (`Get-CimInstance Win32_Processor | Measure-Object LoadPercentage -Average`) and re-run when it drops. A `FullyQualifiedName~QfcItemController` scoped gate includes this class, so a plan gate demanding EXIT 0 on that filter inherits the flakiness — narrow the filter or re-run. Related: [[project_utilitiescs_test_parallelism_flakiness]], [[project_uithread_dispatcher_static_swap_race]], [[project_configcontroller_sta_pump_deadlock]], and the GUI-seam rule in [[tests-must-mock-gui-no-visible-window]].
1 change: 1 addition & 0 deletions .claude/agent-memory/atomic-planner/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
- [Coverage threshold conflict: CLAUDE.md vs general-unit-test.md](project_coverage_threshold_conflict_claude_md_vs_general_unit_test.md) — 80/90 vs 85/75; repo baseline 70.19/58.30 → repo-wide figure reported non-blocking on fixes (#424 precedent), change-scoped gates blocking
- [Planner may lack the MCP plan validator](project_planner_mcp_validator_not_in_tool_surface.md) — file-only tool surface (no Bash/no mcp__drm-copilot__*); never claim the gate passed, report VALIDATOR NOT RUN + structural self-check
- [#349 breadcrumb plan seams](project_349_efcviewer_breadcrumb_plan_seams.md) — P0-T6 halt-gate on 9101 provider; evidence/repro/ authorized; EfcViewer3 mechanical swap only; Newtonsoft in UtilitiesCS only
- [#438 search-focus plan seams](project_438_search_focus_plan_seams.md) — near-500 files → partial headroom; additive interface overload broke 7 test files (3 fakes + 4 loose mocks) → dispatch default path on old overload; git diff misses untracked files in size audits
- [#424 QuickFiler deadline plan seams](project_424_quickfiler_deadline_plan_seams.md) — 12s const; Part2 partial no-[TestClass]; overload migration breaks loose-mock Setup/Verify (Issue218 "dormant" misclassification); grep old overload shape in ALL test files
- [vstest scoped-run + csharpier 1.2.6 commands](reference_vstest_scoped_run_command.md) — vswhere-resolved vstest.console.exe + /InIsolation + /TestCaseFilter (join `|`); every run task needs an explicit command; csharpier needs format/check subcommands
- [#230 WinForms pump seam plan facts](project_230_winforms_pump_seam_plan_facts.md) — 19 census sites verified; Initialization.cs 466/500; test-file headroom; factory seam params before SaveParameters; CreateAsync awaited-tail faults; 19→11 target
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
---
name: project-438-search-focus-plan-seams
description: "#438 plan seams: six near-500 breadcrumb files get new-partial-file headroom; HostTests harness is private (partial Part2 pattern) while ItemViewerDropDownHarness is internal file-scope (new standalone file OK); fail-before split real-run vs dossier; caller's csharpier command was v0 syntax"
metadata:
type: project
---

Plan `docs/features/active/2026-08-07-quickfiler-search-keystroke-focus-steal-438/plan.2026-08-08T09-57.md` (9 phases, 57 tasks) encodes these verified decisions; keep them stable across preflight revision loops:

- **Partial-file headroom (verified non-blank counts 2026-08-08):** BreadcrumbBridgeCoordinator 487, FolderBreadcrumbBridgeRouter 485, BreadcrumbItemViewerLifecycleCoordinator 481, BreadcrumbDropDownHost 480, BreadcrumbDropDownOpenLifetime 477, BreadcrumbSelectionSession 474 — all six take new/relocated members in NEW partial files (`.Search.cs`/`.Open.cs`/`.Focus.cs`/`.Highlight.cs`/`.SearchPresentation.cs`), base class gains only the `partial` keyword. OpenCoordinator (309), IItemViewer (133), IBreadcrumbDropDownHost (42), ItemViewer.FolderSearch (74), EventHandlers (219) edited in place.
- **Test-harness accessibility asymmetry:** `BreadcrumbDropDownHostTests.cs` (499 lines) has a PRIVATE nested `Harness` → new tests require the partial + `.Part2.cs`-without-`[TestClass]` pattern (one-token `partial` on the base file, declared a structural enabler, not a test modification vs the "single justified test modification" clause). `ItemViewerDropDownHarness`/`TrackingMessenger` in `BreadcrumbDropDownIntegrationTests.cs` (500 lines, zero headroom) are `internal` file-scope → new standalone test file reuses them with NO edit to the 500-line file. `BreadcrumbDropDownOpenCoordinatorTests` already partial with Part2 → new cases go in Part3.
- **Fail-before split (D5):** negatives-only controller regression (`SetFolderDroppedDown`/`FocusFolderDropDown`/`SetFolderSelectedIndex` never) compiles against the pre-fix surface → real observed-failing run BEFORE any production edit; intent-once + all new-seam tests (`PresentFolderSearchResults`, `HighlightRow`, `ReplaceItemsPreservingSession`, 4-param `OpenAsync`) are compile-impossible pre-change → one `fail-before-exception.<ts>.md` dossier + a second expect-fail run just before the controller flip.
- **Caller command correction (D1):** the delegation prompt's "verbatim" format command `dotnet tool run csharpier .` is v0 syntax; repo pin is 1.2.6 → plan substitutes `format`/`check` subcommands while keeping the `./.dotnet-sdk/dotnet.exe` pinning. Do not copy caller toolchain commands into a plan without checking `dotnet-tools.json`.
- **Interface-addition sweep:** `IBreadcrumbDropDownHost` overload addition requires a grep-and-update of manual (non-Moq) implementers in the same task (P3-T1), plus the #424 loose-mock lesson: P5-T4 greps all test files for `Setup`/`Verify` of the OLD 3-param `OpenAsync` shape.

**Preflight revision 1 outcomes (three blocking findings):**
- **An additive interface overload is never test-neutral.** `IBreadcrumbDropDownHost` overload broke 7 existing test files: 3 compile-time (manual fakes `ControlledHost` OpenCoordinatorTests.cs:374, `RecordingHost` LifecycleCoordinatorTests.cs:257, `RecordingDropDownHost` SelectorOpenRetryTests.cs:291) + 4 runtime (loose mocks configured only for the 3-param shape return null `Task`, tripping the `?? throw` at `BreadcrumbDropDownOpenCoordinator.cs:195`: IntegrationTests:344-352, SubfolderActivationTests:307, PendingOpenCloseTests:167, QfcItemControllerBreadcrumbDropDownTests:132). Fixes: (a) the coordinator DISPATCHES — default `takeFocus:true` opens keep calling the existing 3-param overload unchanged; only `false` uses the 4-param — leaving all four loose-mock files byte-unmodified; (b) a dedicated task adds additive delegating members to the three fakes; (c) P3-T1..T3 declared one compile unit with the build gate at the fakes task; (d) spec AC-11 restated from "single test modification" (file) to "single test-METHOD modification" + sanctioned structural non-test edits. Lesson: before claiming "only one test file changes", enumerate BOTH manual implementers AND loose-mock Setup shapes of every touched interface member.
- **`git diff --name-only <sha>` does not list untracked files** — a file-size audit built on it silently misses every NEW file. Add `git ls-files --others --exclude-standard -- '*.cs'` and enumerate the expected new files in the accept clause.
- **Porcelain-clean baselines must allow promotion-lifecycle residue**: ` D docs/features/potential/promoted/<name>.md` is an expected pre-existing deletion after feature promotion; a Phase 0 accept clause that only allows `<FEATURE>/` + agent-memory fails on the first executed task.
- **Cycle-2 retraction of the citation advisory:** cycle-1's "line-citation drift" advisory was itself wrong — the executor hand-counted from an unnumbered window; line-numbered re-verification confirmed the ORIGINAL research/spec citations (`EventHandlers.cs:177`; method span `313-350` incl. closing brace, doc comment `308-312`; Down-arrow `355-388`). Cycle 2 reverted the three "fixes". Lessons: only accept citation corrections backed by line-numbered output, and a cited method span must include its closing brace. Still true from cycle 1: `FolderHandling.cs:202` is `SetFolderSelectedIndex(FolderArray.Length == 1 ? 0 : 1)`, not a literal `(1)`; and `ItemViewerDropDownHarness`'s 3-param `OpenAsync` setup's `.Callback` sets a private `_hostOpen` backing `Host.IsOpen`, so a new-file 4-param setup must also re-register `SetupGet(h => h.IsOpen)` (legal Moq re-registration on the same mock).

**Why:** these came from direct repo verification (grep counts, class-modifier greps, dotnet-tools.json) during planning plus the executor's read-only preflight; a revision loop that grows a 48x-line file in place or adds `[TestClass]` to a partial Part2 breaks the 500-line gate or CS0579.

**How to apply:** on `PREFLIGHT: REVISIONS REQUIRED`, update the same plan file in place and re-verify these seams survive the delta. Related: [[project_400_partial_class_headroom_placement]], [[project-424-quickfiler-deadline-plan-seams]], [[reference-vstest-scoped-run-command]], [[project_legacy_csproj_explicit_compile_include]], [[feedback-ac-checkoff-one-per-task]].
4 changes: 3 additions & 1 deletion .claude/agent-memory/feature-review/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
- [TestResults coverage XML cross-module check](project_testresults-coverage-xml-cross-module-check.md) — an uncommitted `TestResults/*.xml` from a single-project vstest run also instruments other loaded first-party modules; grep it to spot-check a "repository coverage" claim without rerunning coverage
- [plan trailer preflight directive is benign](project_plan-trailer-preflight-directive-benign.md) — a trailing `DIRECTIVE: PREFLIGHT VALIDATION ONLY` line in plan.md is standard planner/executor handoff text, not an injection aimed at feature-review; don't flag it as scope narrowing
- [feature-evidence Cobertura counts as the C# coverage artifact](project_feature-evidence-cobertura-counts-as-coverage-artifact.md) — #424: committed `<FEATURE>/evidence/` Cobertura = artifact present; verify from it, don't repeat #309's procedural FAIL; dot-source the hook for end-to-end simulation
- [pr-context-summary-misclassifies-cs, recurring (#171/#181/#244/#251/#253/#270/#278/#283/#208/#292/#328/#418/#424)](project_pr-context-summary-misclassifies-cs.md) — near-certain per-review defect; check every C#-touching feature unprompted, correct the summary in place (paths with SPACES like `ToDoModel/Data Model/...` are skipped by the hook regex — list space-free `.cs` paths to make it enumerate C#), and grep your own policy-audit for banned narrowing phrases co-occurring with "C#"/"coverage"/"dotnet"/"csharp" before finalizing
- [438 review findings (both cycles)](project_438_cycle1_findings.md) — cycle 2 PASS/0 blocking, R1 resolved 4/4; residuals: R2 disposition, classifier-defect promotion, HV-1
- [csharp coverage constants nondeterministic](project_csharp-coverage-constants-nondeterministic.md) — never gate on cross-session repo-wide constants (~0.015-pt band, #511); use same-session baseline + per-file covered-line diff to attribute variance
- [pr-context-summary-misclassifies-cs, recurring (#171/#181/#244/#251/#253/#270/#278/#283/#208/#292/#328/#418/#424/#230/#438)](project_pr-context-summary-misclassifies-cs.md) — near-certain per-review defect; check every C#-touching feature unprompted, correct the summary in place (paths with SPACES like `ToDoModel/Data Model/...` are skipped by the hook regex — list space-free `.cs` paths to make it enumerate C#), and grep your own policy-audit for banned narrowing phrases co-occurring with "C#"/"coverage"/"dotnet"/"csharp" before finalizing
- [TaskMaster validator memories are cross-repo](project_taskmaster-validator-memories-are-cross-repo.md) — the `validate_orchestration_artifacts`/heading-template memories describe a different repo (mix-calculator/drm-copilot); TaskMaster's only real gate is `validate-feature-review-coverage.ps1` (3-path advertisement + per-language coverage-row PASS/FAIL/no-narrowing check)
- [Stale caller-supplied merge-base](project_stale-caller-merge-base.md) — #244 cycle 2: caller SHA was one merged PR behind actual `main`; always recompute via `git merge-base HEAD origin/main`, don't trust the supplied value
- [modified-workflow green-run gate is manual](project_modified-workflow-green-run-manual-check.md) — #267: Test-ModifiedWorkflowNeedsGreenRun.ps1 doesn't exist in TaskMaster; check via `git diff --name-only` for `.github/workflows/**`, not the summary's truncated top-10 overview bullets
Expand Down
Loading
Loading