diff --git a/.claude/agent-memory/atomic-executor/MEMORY.md b/.claude/agent-memory/atomic-executor/MEMORY.md index 6d898689a..28fff2d41 100644 --- a/.claude/agent-memory/atomic-executor/MEMORY.md +++ b/.claude/agent-memory/atomic-executor/MEMORY.md @@ -71,6 +71,7 @@ - [pwsh -Command quoting + backtick stripping](project_pwsh_command_quoting_from_bash.md) — backticks vanish; `-split "\r\n"` gives 1 element - [pwsh -File binds a list as ONE string](project_pwsh_file_array_param_from_bash.md) — `-Tokens a,b,c` is a 1-element array - [Compile-time red needs body-level refs](project_compile_red_needs_body_level_references.md) — a missing type in a SIGNATURE hides body errors +- [Cross-task shell-variable splat gates](project_cross_task_shell_variable_splat_gate.md) — a bare `@assemblies` in a fresh session runs zero assemblies and still reports zero failures; require same-payload enumeration + a `>= 1` count floor - [Shared evidence artifact + floating ](project_shared_evidence_artifact_floating_ts.md) — N tasks told to append to "the same" artifact whose ## Test execution & isolation - [Long runs need a detached process](project_long_runs_need_detached_process.md) — background runners die at ~1h diff --git a/.claude/agent-memory/atomic-executor/project_cross_task_shell_variable_splat_gate.md b/.claude/agent-memory/atomic-executor/project_cross_task_shell_variable_splat_gate.md new file mode 100644 index 000000000..a14016a14 --- /dev/null +++ b/.claude/agent-memory/atomic-executor/project_cross_task_shell_variable_splat_gate.md @@ -0,0 +1,24 @@ +--- +name: cross-task-shell-variable-splat-gate +description: A plan task that consumes a shell variable produced by an earlier task is vacuous unless the enumeration body is re-executed in the SAME -Command payload and a count-identity gate plus a >= 1 floor is asserted. +metadata: + type: project +--- + +When a plan enumerates test assemblies in task A (`$assemblies = @(...)`) and splats them in task B +(`& $vstest @assemblies ...`), no shell state persists between tool invocations, so a bare +`@assemblies` in task B expands to nothing. vstest then runs zero assemblies and still reports a +zero failure count, so the gate passes while measuring nothing. + +**Why:** #491 preflight round 2 blocked on exactly this. The fix that cleared round 3 was two-part: +(1) task B must re-execute task A's enumeration BODY (not the whole `pwsh -NoProfile -Command '...'` +wrapper — nesting pwsh puts the variable in a child session) inside the same payload; (2) acceptance +must assert the assembly count on the command line equals the count recorded in task A's artifact +AND is at least 1. The `>= 1` conjunct is what makes it falsifiable; the equality alone normally +always holds because nothing between the two tasks adds a test project. + +**How to apply:** Whenever a plan passes a shell variable across task boundaries, demand both the +same-payload clause and the `>= 1` floor. Also check the composed payload's quoting: a vswhere +re-resolution embeds single quotes (`& 'C:\Program Files (x86)\...'`) and needs them doubled inside +a single-quoted `-Command` payload. See [[project_pwsh_command_quoting_from_bash]] and +[[project_preflight_selfderived_gate_thresholds_are_blind]]. diff --git a/.claude/agent-memory/atomic-planner/MEMORY.md b/.claude/agent-memory/atomic-planner/MEMORY.md index 85dedbb8f..d31bf1dc7 100644 --- a/.claude/agent-memory/atomic-planner/MEMORY.md +++ b/.claude/agent-memory/atomic-planner/MEMORY.md @@ -7,6 +7,7 @@ - [#501 R3](project_501_r3_preflight_seams.md) — repo-wide 0-skipped gates unsatisfiable; BASELINE_FAILURE_SET subset; `Task.CompletedTask` singleton - [#511 R1](project_511_r1_preflight_delta_seams.md) — mid-cycle evidence deletion; git-log scans post-commit; Start-Process for 20-min runs - [#484](project_484_qfc_revision_seams.md) — ownership change sweeps plan→issue.md→spec.md (spec is the AC source); old-cardinal grep sweep +- [QfcItemController test-capacity squeeze](project_qfcitemcontroller_test_capacity_squeeze.md) — four owned test files hold only ~471 aggregate spare lines and `.csproj` edits are barred; budget in Phase 0, permit relocation, mandate DataRow/shared-arrange compaction - [#494](project_494_threshold_reconciliation_plan_seams.md) — coverage runner throws before post-processing; reported-floor must not become hook-Blocking - [#498](conditional-ladder-and-unowned-class-gates.md) — gate EVERY rung of a recorded-selector ladder incl. rung 1; scope failing-identifier clauses to owned test classes; 0/0 changed-line figure → NOT APPLICABLE - [#503](project_503_ribbon_readiness_plan_seams.md) — RibbonViewer 487/500 forces a region move; 6+4 Compile entries; compile-time red + dossier @@ -40,6 +41,7 @@ - [Wiring gates must be wiring-sensitive](feedback_wiring_gates_must_be_wiring_sensitive.md) — count floors deflate with the defect they guard - [Research claims as acceptance clauses](research-claims-as-acceptance-clauses.md) — never encode an unmeasured third-party claim as a literal AC clause - [Literal-call clauses block file-size tightening](literal-call-clauses-block-file-size-tightening.md) — pinning a call in 2+ places near a 500-line file is unsatisfiable +- [Enumeration variable must match its consumer](enumeration-variable-must-match-consumer.md) — `$kept` produced vs `@assemblies` splatted = zero-assembly vstest run reporting zero failures; same-payload re-execution + a count-parity floor - [Diff gates need a commit task](diff-gates-need-a-commit-task.md) — `git diff ..HEAD` passes vacuously with no commit task - [Never pin a HEAD SHA as a plan expectation](never-pin-head-sha-as-plan-expectation.md) — gate on tree invariants instead - [Absolute counts in shared files go stale](absolute-counts-in-shared-files-go-stale.md) — lower-bound/baseline-relative for co-owned files; keep exact the count the task changes @@ -48,7 +50,7 @@ ## C# toolchain and test mechanics -- [Phase 0 toolchain bootstrap](project_csharp_phase0_toolchain_bootstrap.md) — global csharpier/dotnet-coverage exe + mandatory NuGet restore +- [Phase 0 toolchain bootstrap](project_csharp_phase0_toolchain_bootstrap.md) — `dotnet tool run csharpier` works once the SDK is bootstrapped (global.json's missing .dotnet-sdk was the real blocker, not the manifest); mandatory NuGet restore - [Agent worktrees need SDK + NuGet + analyzer backfill](agent-worktrees-need-sdk-and-nuget-bootstrap.md) — four Phase 0 steps; CS0006 is an error, not a warning - [vstest scoped-run + csharpier 1.2.6 commands](reference_vstest_scoped_run_command.md) — vswhere + `/InIsolation` + `/TestCaseFilter`; csharpier needs a subcommand - [CSharpier gate: format not pipe-files](csharpier-format-not-pipe-files-gate.md) — `pipe-files` is stdout-only and non-enforcing @@ -64,6 +66,8 @@ - [Pester exits 0 on failing It blocks](pester-invoke-does-not-exit-nonzero.md) — scope every exit-code clause to a named channel - [PowerShell gate observables](powershell-gate-observables.md) — no Invoke-Pester exit code; explicit `scan_folders`; aggregate-only `CoveragePercent` - [Legacy csproj wiring](project_legacy_csproj_explicit_compile_include.md) — `Compile Include` + own `Reference`; ProjectReference gives no compile-time flow +- [Invoke-VSBuild rewrites csproj HintPaths](invoke-vsbuild-rewrites-csproj-hintpaths.md) — the wrapper runs Sync-PackageReferences over EVERY csproj; with a forbidden .csproj the build itself commits the scope violation — use vswhere-resolved MSBuild +- [Declaration-only seam task for fail-before](declaration-only-seam-task-for-fail-before.md) — tests citing not-yet-existing internals redden the whole assembly; order compile-clean tests first, open the fix phase with a no-behaviour seam task + whole-set assertion-time red run - [net48 / nullable context mismatch](project_nullable_context_mismatch_prod_vs_test.md) — check `#nullable enable` in prod AND missing `` in the test csproj - [Worktree root breaks the `\.claude\` exclusion](worktree-root-breaks-dotclaude-exclusion.md) — assert a workspace-root prefix instead @@ -80,7 +84,10 @@ - [#511 R1 preflight delta seams](project_511_r1_preflight_delta_seams.md) — mid-cycle raw-evidence deletion breaks resolves gates; git-log scans post-commit only; absolute MSBuild path; Start-Process mechanic for 20-min runs; per-class coverage noise -0.50pp - [CSharpier "Formatted N files" is processed count](csharpier-formatted-n-is-processed-count.md) — restart-on-rewrite loops keyed on it never terminate; define rewritten-count via before/after SHA-256 - [Terminal-phase planner traps](terminal-phase-planner-traps.md) — sweep the last phase for an unowned "a follow-up issue should carry it", artifacts written after the clean-tree commit task, and a false "clarification against the spec's wording" +- [#493 UiThread dispatcher plan seams](project_493_uithread_dispatcher_plan_seams.md) — signature-change fail-before gets a REAL red build by staging the two `` lines; the coverage script IS the parallelized run; scope `SwapUiThreadDispatcher`/`UiThreadDispatcherGate` greps to `QuickFiler.Test/` - [#553 CI parallel-split plan seams](project_553_ci_parallel_split_plan_seams.md) — workflow-only scope: no C# toolchain; Phase 0 snapshot for byte-identity; ruleset PUT + gh pr create orchestrator-gated; no jq (ConvertTo-Json -Depth 20); pathspec anchoring; BRANCH/SCRATCH conventions +- [#442 QuickFiler metrics plan seams](project_442_quickfiler_metrics_plan_seams.md) — commented-out code defeats zero-hit grep gates; an AC conjunct already green pre-fix; declare the seam BEFORE the red tests or they don't compile; Invoke-MSTestWithCoverage throws below 80% +- [#468 QfcCollectionController plan seams](project_468_qfc_collection_controller_plan_seams.md) — ToggleUnGroupConv is not COM-free drivable; MakeSpaceForItems never touches Size; a sign-defect seam must land carrying the defect; `LoadItemGroup(` needs the paren - [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-wide figure non-blocking, change-scoped gates blocking - [JaCoCo hook, Cobertura also accepted](project_csharp_coverage_gate_jacoco_format.md) — follow the format the delta names - [Async state machines split the denominator](async-state-machine-coverage-aggregation.md) — aggregate by `filename` or a >=90% gate fails for measurement reasons diff --git a/.claude/agent-memory/atomic-planner/agent-worktrees-need-sdk-and-nuget-bootstrap.md b/.claude/agent-memory/atomic-planner/agent-worktrees-need-sdk-and-nuget-bootstrap.md index 918b7742d..15c34d95b 100644 --- a/.claude/agent-memory/atomic-planner/agent-worktrees-need-sdk-and-nuget-bootstrap.md +++ b/.claude/agent-memory/atomic-planner/agent-worktrees-need-sdk-and-nuget-bootstrap.md @@ -15,7 +15,11 @@ before the first `dotnet tool restore` and before the first `msbuild`, in this o `pwsh -NoProfile -File .\scripts\vscode\Install-RepoDotNetSdk.ps1`, or mirror the populated `.dotnet-sdk` tree from the main checkout. Falsifiable acceptance: `dotnet --version` prints `8.0.205` AND `dotnet --list-sdks` includes a path ending `.dotnet-sdk\sdk`. -2. **`nuget restore TaskMaster.sln`.** A fresh worktree has no `packages/`. Every project declares +2. **`nuget restore TaskMaster.sln`.** Prefer `pwsh -NoProfile -File .\scripts\vscode\Invoke-Restore.ps1`, + which resolves MSBuild through `vswhere` and runs `/t:Restore /p:RestorePackagesConfig=true /m` — the + `packages.config`-aware form. `nuget.exe` is not guaranteed to be on `PATH` in an agent worktree, so a + plan task whose only stated command is `nuget restore` can be unrunnable. + A fresh worktree has no `packages/`. Every project declares `` whose `` fires before compilation when the tree is missing (e.g. `QuickFiler.Test.csproj:452-466`), and every `Reference` `HintPath` under `..\packages\` is unresolvable. CI does not hit this because diff --git a/.claude/agent-memory/atomic-planner/declaration-only-seam-task-for-fail-before.md b/.claude/agent-memory/atomic-planner/declaration-only-seam-task-for-fail-before.md new file mode 100644 index 000000000..c070e7908 --- /dev/null +++ b/.claude/agent-memory/atomic-planner/declaration-only-seam-task-for-fail-before.md @@ -0,0 +1,19 @@ +--- +name: declaration-only-seam-task-for-fail-before +description: When regression tests reference internal seams that do not exist yet, the whole test ASSEMBLY goes red and no test in it can run — order compile-clean tests first, then open Phase 2 with a declaration-only seam task so every test fails at assertion time +metadata: + type: feedback +--- + +A C# `[expect-fail]` phase whose tests reference not-yet-existing `internal` members produces a compile error, not a failing test. Because the failure is per-ASSEMBLY, it also destroys the assertion-time evidence of every other test in that phase, including ones that would have compiled fine. + +Two ordering rules fix it: + +1. **Order the fail-first phase so every compile-clean test is authored and RUN before the first compile-breaking one.** Reflection/structural tests and argument-guard tests against existing members compile today; marshalling tests through a new internal constructor and state assertions through a new internal observation member do not. Run and record the former group first. +2. **Open the fix phase with a declaration-only seam task.** It declares exactly the new members (constructor overload, `internal bool` observation properties, nullable collaborator field) with no behaviour change, and its acceptance is `EXIT_CODE: 0` on a `/t:Build`. The next task then runs the WHOLE test set and records "N discovered, N failed, 0 passed, 0 build errors" — that artifact, not the compile errors, is the authoritative fail-before record, and no behavioural fix precedes it. + +Give each compile-time-red task its own falsifiable acceptance anyway: "a `[TestMethod]` named `` exists in `` AND the build is red with the error list recorded verbatim". The method name is a distinct literal per task, so two tasks that share the same broken build are still separately checkable. Quote every such method name in a plan prose block so rule G5 exonerates it. + +**Why:** on the #476 plan, six of eight Phase 1 tasks referenced `internal WebView2BreadcrumbHost(` / `IsAttached` / `HasUiDispatcher`. Without this shape the plan would have claimed a fail-before it could not show, and a preflight reviewer cannot tell a genuine assertion failure from a typo that broke the build. + +**How to apply:** whenever a bug plan's tests need a NEW seam, sort the phase by "compiles against HEAD" first, then add the declaration-only task and the whole-set fail-before run as the first two tasks of the fix phase. Related: [[acceptance-edits-must-be-false-before-true-after]], [[one-ac-per-checkoff-task]]. diff --git a/.claude/agent-memory/atomic-planner/diff-gates-need-a-commit-task.md b/.claude/agent-memory/atomic-planner/diff-gates-need-a-commit-task.md index 58af58a78..deb4abb07 100644 --- a/.claude/agent-memory/atomic-planner/diff-gates-need-a-commit-task.md +++ b/.claude/agent-memory/atomic-planner/diff-gates-need-a-commit-task.md @@ -15,4 +15,6 @@ Any plan gate expressed as `git diff --numstat ..HEAD` or `git diff 2. **Phase 0 porcelain is non-empty by construction.** Never write "empty `git status --porcelain`" as a Phase 0 acceptance. Record the verbatim output and make the binary outcome a *type* assertion instead: no `.cs`, `.csproj`, `.xml`, or `.sln` path appears. Docs/evidence/agent-memory paths are expected. 3. **Scope-lock diff gates must whitelist docs.** Once commits exist, the diff legitimately contains `docs/features/` and `.claude/agent-memory/` paths. Phrase the gate as "every path is a scope-lock member OR under docs/evidence" and keep the hard clause on source extensions only. In a companion file-size audit, state that Markdown docs are exempt from the 500-line cap per `.claude/rules/general-code-change.md`. +4. **Branch history can make a `base..HEAD` gate unsatisfiable no matter how it is whitelisted.** An epic child branch often already carries commits the child does not own — #491's branch head sat on a merge of `main` that restored 58 `docs/features/potential/` records, so a P5 scope-lock gate enumerating five `QuickFiler.Test/**` paths failed before the executor touched anything. When the gate's intent is "this child changed only its own paths", gate the child's OWN commit with `git show --name-only --format= HEAD` run right after the plan's commit task, and demote the branch-wide `base..HEAD` list to a recorded observation rather than a pass/fail condition. + Related: [[never-pin-head-sha-as-plan-expectation]] — the fix is commit tasks plus tree-invariant gates, never a pinned SHA equality check. diff --git a/.claude/agent-memory/atomic-planner/enumeration-variable-must-match-consumer.md b/.claude/agent-memory/atomic-planner/enumeration-variable-must-match-consumer.md new file mode 100644 index 000000000..ae09eb583 --- /dev/null +++ b/.claude/agent-memory/atomic-planner/enumeration-variable-must-match-consumer.md @@ -0,0 +1,17 @@ +--- +name: enumeration-variable-must-match-consumer +description: A plan task that enumerates test assemblies must assign the exact variable name later vstest tasks splat, and enumeration plus invocation must sit in one pwsh -Command payload — otherwise vstest runs on zero assemblies and still reports zero failures +metadata: + type: feedback +--- + +When one task computes a list and a later task splats it (`& $vstest @assemblies ...`), the producing command must assign that exact name, and the producer's body must be re-executed inside the SAME `pwsh -NoProfile -Command` payload that invokes the consumer. + +**Why:** #491 preflight revision 3. P0-T17 assigned `$all`, `$claude`, and `$kept`; P0-T18 and P3-T6 both splatted `@assemblies`. An undefined `@assemblies` expands to nothing, so vstest would have run against ZERO assemblies and still reported a zero failure count — a silently green full-suite gate. The plan's own conventions paragraph already stated the no-shell-state-persists rule and still shipped the mismatch, because the rule was written about session lifetime and nobody re-read the producing command for the name it actually emits. P0-T17 is itself a `pwsh -Command` child process, so no variable it sets can survive into a later tool invocation regardless. + +**How to apply:** Two checks before a plan leaves preflight. + +1. **Name check.** Grep the plan for every `@` splat and every `$` consumption, then confirm the producing command literally assigns that identifier. Renaming the producer's variable is the cheaper fix than renaming every consumer; a display label such as `KEPT={2}` in a format string may stay as-is and should be called out as deliberate so a reviewer does not flag the mismatch. +2. **Payload check.** Every consuming task's text must say explicitly that it re-executes the enumeration body verbatim in the same payload, and that a bare `@` in a fresh session is forbidden. Add an acceptance clause pinning the count actually passed on the command line to the count recorded in the enumerating task's artifact, with a `>= 1` floor — otherwise the empty-splat failure mode has no gate that can catch it. + +Related: [[wiring-gates-must-be-wiring-sensitive]] — a count floor is the observable that makes an empty run fail; [[powershell-gate-observables]] for the quoting rules of the enclosing payload. diff --git a/.claude/agent-memory/atomic-planner/invoke-vsbuild-rewrites-csproj-hintpaths.md b/.claude/agent-memory/atomic-planner/invoke-vsbuild-rewrites-csproj-hintpaths.md new file mode 100644 index 000000000..4124367af --- /dev/null +++ b/.claude/agent-memory/atomic-planner/invoke-vsbuild-rewrites-csproj-hintpaths.md @@ -0,0 +1,17 @@ +--- +name: invoke-vsbuild-rewrites-csproj-hintpaths +description: Invoke-VSBuild.ps1 runs Sync-PackageReferences.ps1, which rewrites in EVERY csproj — so when a feature has a forbidden .csproj, the build wrapper itself can commit the scope violation; use direct MSBuild via vswhere instead +metadata: + type: feedback +--- + +When a plan carries a forbidden-file list that includes a `.csproj`, do NOT plan `scripts/vscode/Invoke-VSBuild.ps1`. Resolve `MSBuild.exe` through `vswhere` and pass the `CLAUDE.md` argument list directly. + +**Why:** `Invoke-VSBuild.ps1:152-155` unconditionally invokes `scripts/vscode/Sync-PackageReferences.ps1 -SolutionRoot $repoRoot` before building. That script scans every `*.csproj` for `..\packages\...\lib\...` values that do not resolve on disk and rewrites them in place (`Sync-PackageReferences.ps1:55-154`). In a fresh agent worktree — where `packages/` is absent or holds versions skewed from `packages.config` — that is a live rewrite path, not a theoretical one. On #476 the forbidden list included `QuickFiler/QuickFiler.csproj`, so a single wrapper-driven build would have produced a scope violation that the feature's own scope-containment gate then reports against the executor. + +Two related facts: + +- The wrapper otherwise supports everything the mandated commands need (`-Target Rebuild`, `-EnableNETAnalyzers`, `-EnforceCodeStyleInBuild`, `-TreatWarningsAsErrors`), so the reason to skip it is exclusively the csproj rewrite. The general "prefer repo-defined tasks" guidance in `policy-compliance-order` still holds everywhere else. +- The wrapper `throw`s on a non-zero MSBuild exit (`:165-167`), so a `pwsh -File` run of it exits 1 rather than surfacing MSBuild's own code. `scripts/vscode/Invoke-MSTestWithCoverage.ps1:236` does the same. Baseline tasks must take `EXIT_CODE:` from the pwsh process and must not gate on `EXIT_CODE: 0`. + +**How to apply:** grep a draft plan for `Invoke-VSBuild`; if the feature forbids any `.csproj`, swap to the vswhere-resolved `& $msbuild TaskMaster.sln /t:Rebuild ...` form and say in the plan why. Also add a Phase 5 gate that no `.csproj` outside the writable set appears in the change inventory. Related: [[poshqc-mcp-and-msbuild-invocation-facts]], [[project-csharp-phase0-toolchain-bootstrap]]. diff --git a/.claude/agent-memory/atomic-planner/project_442_quickfiler_metrics_plan_seams.md b/.claude/agent-memory/atomic-planner/project_442_quickfiler_metrics_plan_seams.md new file mode 100644 index 000000000..344ee94f8 --- /dev/null +++ b/.claude/agent-memory/atomic-planner/project_442_quickfiler_metrics_plan_seams.md @@ -0,0 +1,59 @@ +--- +name: project-442-quickfiler-metrics-plan-seams +description: "#442/#443/#451 QuickFiler home-controller metrics plan seams: commented-out code defeats zero-hit grep gates, an AC conjunct that is already green, a seam-first ordering so red tests compile, and the repo coverage runner that throws below 80%" +metadata: + type: project +--- + +Planning seams found while authoring the atomic plan for epic-child bug #442 +(`docs/features/active/quickfiler-home-controller-metrics-442`). Each cost a real +correction and generalizes past this feature. + +**Why:** the spec's own acceptance criteria contained two constructions that would have +shipped an unsatisfiable gate and a no-op gate respectively, and the red-before ordering +was not derivable from the spec's rollout list alone. + +**How to apply:** check these four seams on any C# bugfix plan in this repo. + +1. **A commented-out occurrence defeats a "returns no match" gate.** + `QfcHomeController.Metrics.cs:120` is `//Duration = _stopWatchMoved.Elapsed.Seconds;` + sitting directly above the live read on `:121`. AC-7 asserts a search for + `Elapsed.Seconds` under `QuickFiler/Controllers/` returns no match. Fixing only the + live line leaves the comment and the gate can never pass. Always grep the target + pattern BEFORE writing a zero-hit gate and add an explicit deletion task for every + commented, XML-doc, and string-literal hit. Complements + [[zero-hit-grep-gates-need-carveouts]]. + +2. **Read every AC conjunct for "already true at branch head".** AC-7's second conjunct — + `BuildQuickFileMetricLines` with `elapsedSeconds = 90` renders `90` not `30` — is green + on the pre-fix source, because the 0-59 truncation lives at + `EfcHomeController.Metrics.cs:23` where the `TimeSpan` component is read, not inside the + pure function. Keep the test as a declared pin, tag it NOT `[expect-fail]`, and say in + the plan which conjunct carries the falsifiability. See + [[acceptance-edits-must-be-false-before-true-after]]. + +3. **Declare the injectable seam in its own task BEFORE the red tests.** The #442 flush + tests assert an injected `MetricsFileWriter`. If the seam does not exist yet the tests do + not COMPILE, and a non-compiling suite is not a clean red state. Order: (a) add the seam + property only, production call path unchanged, solution compiles; (b) author the tests, + which now compile and fail because the writer is never invoked; (c) rewire + `WriteMetricsAsync`. Same shape applies to any writer/clock/delegate seam introduced by a + bugfix. + +4. **`scripts/vscode/Invoke-MSTestWithCoverage.ps1` throws below a repo-wide 80% line + floor.** `Assert-CoberturaLineCoverageThreshold` (in + `Invoke-MSTestWithCoverage.Helpers.ps1`) throws BEFORE the Koverage `Set-Content`, so a + sub-80% run leaves the RAW dotnet-coverage cobertura at + `coverage\coverage.cobertura.xml` (absolute paths, third-party packages still present) + and exits non-zero. Repo baseline is ~70%, so both the baseline and the final coverage + task WILL exit non-zero. Plan for it: record `EXIT_CODE:` as observed, extract `line-rate` + from the file as written, label it raw-aggregate, and gate on the change-scoped per-file + figures instead. Extends [[project_494_threshold_reconciliation_plan_seams]]. + +Ownership context worth keeping: this epic child owns five `QuickFiler/Controllers/*.cs` +production files plus two `QuickFiler.Test/Controllers/*MetricsTests.cs` files. Both +`.csproj` files are unowned legacy non-SDK projects, so NO new `.cs` file is possible; and +`QuickFiler.Test/Controllers/EfcHomeControllerTests.cs` is NOT owned, which makes the +`int` to `double` widening's effect on it a Phase 0 verification task rather than something +the plan can fix. `QfcHomeControllerMetricsTests.cs` starts at 421 lines against the 500 cap +— deleting the orphaned `NonBlockingProducer_DelaySeam_*` test is the plan's line budget. diff --git a/.claude/agent-memory/atomic-planner/project_468_qfc_collection_controller_plan_seams.md b/.claude/agent-memory/atomic-planner/project_468_qfc_collection_controller_plan_seams.md new file mode 100644 index 000000000..cf09cc8f3 --- /dev/null +++ b/.claude/agent-memory/atomic-planner/project_468_qfc_collection_controller_plan_seams.md @@ -0,0 +1,54 @@ +--- +name: project-468-qfc-collection-controller-plan-seams +description: "#468 QfcCollectionController seven-issue bugfix — ToggleUnGroupConv cannot be driven COM-free, MakeSpaceForItems never touches Size, the ShrinkByRows seam must land sign-preserving, and LoadItemGroup is a substring of a live member" +metadata: + type: project +--- + +Planning seams found while writing the `#468` plan (closes #286, #468, #469, #470, #471, #473, #474; +all in `QuickFiler/Controllers/QfcCollectionController.cs`, 2349 lines). + +1. **`ToggleUnGroupConv` cannot be driven COM-free.** Its first two statements are + `SafeSetTlpLayout(false)` and `UnregisterNavigation()`, and `MakeSpaceForItems` reaches + `TableLayoutHelper.InsertSpecificRow` on `_itemTlp`. So the `#470-2` above/equal/below-reservation + cases and the `baseEmailIndex == -1` guard have no permanent red-then-green test at that level. + Move the assertions onto pure static helpers (`ResolveConversationInsertions`, + `ReconcileInsertionCount`) and put the behavioural pre-fix red state in the dossier. Research + §3.5 assumed the post-fix reconciliation would throw before the loop; the spec chose + log-and-proceed instead, which removes that assumption — check which one the spec adopted before + copying a research test recipe. + +2. **`MakeSpaceForItems` adjusts `MinimumSize` only; `EliminateSpaceForItems` adjusts both + `MinimumSize` and `Size`.** AC-11's "make-then-eliminate is height-neutral" is therefore only + true for `MinimumSize`. Scope the neutrality assertion and record the `Size` asymmetry as + pre-existing, or the acceptance clause is false after a correct fix. + +3. **A behaviour-preserving seam over a sign defect must land carrying the defect.** `ShrinkByRows` + extracted from `EliminateSpaceForItems` has to keep the inverted argument at the call site so the + seam commit changes nothing observable (AC-20). The fail-before for `#471` therefore lives at the + CALL SITE (the STA panel test), never on the pure helper — the helper is correct by construction + the moment it exists. + +4. **`LoadItemGroup` is a substring of the live `LoadItemGroupsAndViewers_02`.** A fixed-string + zero-hit gate on the dead member must assert `LoadItemGroup(` with the paren. The same trap + almost applies to `AnyOpenDropDowns` / `AnyOpenDropDownsAsync` and + `LoadConversationsAndFolders_04` / `LoadConversationsAndFoldersAsync`; only the `Async` suffix + saves those two. + +5. **Scope every `#468` identifier sweep to the single production file.** All thirteen identifiers + appear in `docs/features/**` (spec, both research docs, the promoted potential entries, and the + plan itself), so a repository-wide zero-hit gate is unsatisfiable by construction. + +6. **`QuickFiler.Test.csproj` line facts (base commit `988e819b`):** 116 is the + `QfcCollectionControllerTests.cs` entry, 117 the `QfcCollectionControllerDarkModeTests.cs` entry, + 118 the `QfcDatamodelTests.cs` entry. New `QfcCollectionController*` entries go between 117 and + 118. `QfcCollectionControllerTests.cs` is exactly 500 lines and takes no new method; + `QfcCollectionControllerDarkModeTests.cs` is 155. + +7. **The class carries `[ExcludeFromCodeCoverage]` at `:21` and removing it is out of scope.** Never + author an acceptance clause claiming the feature raises coverage on that file — it cannot fail. + Capture baseline and final coverage numerically anyway and state the non-attribution explicitly. + +Related: [[trx-needs-resultsdirectory]], [[agent-worktrees-need-sdk-and-nuget-bootstrap]], +[[worktree-root-breaks-dotclaude-exclusion]], [[feedback_ac_checkoff_one_per_task]], +[[diff-gates-need-a-commit-task]]. diff --git a/.claude/agent-memory/atomic-planner/project_493_uithread_dispatcher_plan_seams.md b/.claude/agent-memory/atomic-planner/project_493_uithread_dispatcher_plan_seams.md new file mode 100644 index 000000000..157549d38 --- /dev/null +++ b/.claude/agent-memory/atomic-planner/project_493_uithread_dispatcher_plan_seams.md @@ -0,0 +1,46 @@ +--- +name: project-493-uithread-dispatcher-plan-seams +description: Seams found planning #493 (QuickFiler.Test UiThread dispatcher restore scope) — signature-change fail-before needs a staged Compile Include, the coverage script IS the parallelized run, and the two doomed grep tokens live all over docs/ +metadata: + type: project +--- + +Three reusable findings from planning issue #493 (`docs/features/active/quickfiler-test-uithread-dispatcher-493`). + +**1. A signature-change bug CAN have a real executed fail-before run — stage the ``.** +The regression tests could not compile at `HEAD` (`EnsureUiThreadDispatcher` returned `void`; the new +fixture type did not exist), so the delegation proposed a prose-only "note that it does not compile". +A real red build is available instead: author the new *tests* file and add ONLY its `` +line, run `msbuild /t:Rebuild`, and capture the diagnostics. Defer the *fixture* file's `` to the next phase — otherwise MSBuild fails with a missing-SOURCE-FILE error (CS2001) rather +than the intended missing-TYPE error, and the artifact proves the wrong thing. Record +`ExpectedExitCode: 1` on the artifact. + +**Why:** it converts a prose dossier into an executed gate with a non-zero exit code and named +diagnostics, which is what the Bugfix Workflow actually asks for. + +**How to apply:** whenever the fix is a return-type or signature change, split the csproj wiring across +two phases and put the red build between them. Related: [[legacy-csproj-explicit-compile-include]]. + +**2. `Invoke-MSTestWithCoverage.ps1` already runs class-level-parallelized, so it doubles as the +parallelized supporting run.** It reads `scripts/vscode/TaskMaster.cli.runsettings` +(`0ClassLevel`). A plan that needs BOTH a CI-parity +sequential gating run and a parallelized supporting run does not need three test tasks: the +`vstest.console.exe` run with no `/Settings:` is the gating one, and the coverage script invocation is +the parallelized one. `-SearchRoot QuickFiler.Test` scopes discovery to one project (the param is +joined to the repo root), which also sidesteps the stale `.claude/worktrees` discovery defect. + +**Why:** #493 planning initially carried a third, redundant ~20-minute run. + +**How to apply:** state the double duty in the coverage task's text so a reviewer does not read the +missing parallel run as a gap. Related: [[reference-invoke-mstest-with-coverage-script]]. + +**3. `SwapUiThreadDispatcher` and `UiThreadDispatcherGate` occur in ~16 files under `docs/`** — +including #493's own `spec.md`, `research/`, and plan, plus the #511 and #230 feature folders. A +repo-wide "grep returns no hits" gate on either token is unsatisfiable by construction. Scope every +such gate with `-- QuickFiler.Test/`. Conversely the `"_dispatcher"` reflection literal has exactly +three source sites (`QfcItemController.TestSupport.cs`, `...InitializationTests.Part2.cs`, +`WpfUiDispatcherTests.cs`), and the third is an accepted residual risk that must SURVIVE — so the +"exactly one implementation" gate asserts two remaining hits, not zero. + +Related: [[zero-hit-grep-gates-need-carveouts]], [[agent-memory-is-tracked-scope-git-gates]]. diff --git a/.claude/agent-memory/atomic-planner/project_csharp_phase0_toolchain_bootstrap.md b/.claude/agent-memory/atomic-planner/project_csharp_phase0_toolchain_bootstrap.md index b90ea6e70..2e8dd9bf8 100644 --- a/.claude/agent-memory/atomic-planner/project_csharp_phase0_toolchain_bootstrap.md +++ b/.claude/agent-memory/atomic-planner/project_csharp_phase0_toolchain_bootstrap.md @@ -1,6 +1,6 @@ --- name: csharp-phase0-toolchain-bootstrap -description: C# Phase 0 must resolve the toolchain explicitly — prefer the global csharpier/dotnet-coverage exes, never `dotnet tool run`, and always include a NuGet restore task in a fresh agent worktree +description: C# Phase 0 must resolve the toolchain explicitly — bootstrap the .NET SDK first, then `dotnet tool run csharpier` works fine (the manifest finder walks up to the repo-root dotnet-tools.json); always include a NuGet restore task in a fresh agent worktree metadata: type: project --- @@ -9,7 +9,7 @@ Every C# atomic plan in this repo must resolve its toolchain explicitly in Phase **Verified 2026-08-08 (issue #508 preflight, agent worktree):** -1. `dotnet tool run csharpier` is **broken and must not be planned**. There is no `.config/dotnet-tools.json` (the manifest sits at repo root as `dotnet-tools.json`, which `dotnet tool run` does not read), and `global.json` pins an SDK under an absent `.dotnet-sdk`, so every `dotnet` SDK command fails with the missing-SDK error. +1. `dotnet tool run csharpier` fails in a fresh worktree, but **the manifest is not the reason** (corrected 2026-08-24, #476 planning). The SDK's manifest finder checks BOTH `/dotnet-tools.json` and `/.config/dotnet-tools.json` while walking up, so the repo-root `dotnet-tools.json` (csharpier 1.2.6) IS found — `.github/workflows/_format-check.yml:37` runs bare `dotnet tool restore` at the repo root and passes. The only blocker is that `global.json` pins the SDK under an absent `.dotnet-sdk`, so every `dotnet` command prints the `global.json` `errorMessage` first. Once `Install-RepoDotNetSdk.ps1` has run, `dotnet tool restore` + `dotnet tool run csharpier format .` are plannable exactly as `CLAUDE.md` writes them. Prefer them over a global `csharpier.exe`, whose version differs from the pinned one and disagrees with CI. 2. Prefer the **global tools**, which were confirmed on PATH: `\.dotnet\tools\csharpier.exe` (1.3.0) and `\.dotnet\tools\dotnet-coverage.exe` (18.5.2). CSharpier 1.x needs the `format` / `check` subcommand; bare `csharpier .` is invalid. 3. `vstest.console.exe` is NOT on PATH; resolve via `C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe`. 4. A **NuGet restore task is mandatory** in a fresh agent worktree: `packages/` does not exist and there is no `bin\Debug` output, so analyzer and nullable baselines are vacuous (or fail CS0006) without it. Use `pwsh -File scripts/vscode/Invoke-Restore.ps1` (`msbuild /t:Restore /p:RestorePackagesConfig=true`, no .NET SDK required); fall back to the WinGet `nuget.exe restore TaskMaster.sln`. Watch for analyzer version skew between `` HintPaths and the `packages.config` pins — that is an environment issue, not a plan defect. Parameters confirmed 2026-08-10: `-SolutionPath` (default `TaskMaster.sln`), `-Configuration`, `-Platform`. diff --git a/.claude/agent-memory/atomic-planner/project_qfcitemcontroller_test_capacity_squeeze.md b/.claude/agent-memory/atomic-planner/project_qfcitemcontroller_test_capacity_squeeze.md new file mode 100644 index 000000000..cffbe6176 --- /dev/null +++ b/.claude/agent-memory/atomic-planner/project_qfcitemcontroller_test_capacity_squeeze.md @@ -0,0 +1,41 @@ +--- +name: project-qfcitemcontroller-test-capacity-squeeze +description: The four QfcItemController owned test files have only ~471 lines of aggregate 500-line headroom, so a multi-defect feature must compute the test-capacity budget in Phase 0 before assigning any test to a file +metadata: + type: project +--- + +For work confined to `QuickFiler.Test/Controllers/QfcItemController.{FocusAndTheme,EventWiring,ViewerSetup,MailActions}Tests.cs`, +compute the aggregate 500-line headroom BEFORE assigning tests to files. Measured 2026-08-24: +497 / 374 / 474 / 184 lines, i.e. 3 / 126 / 26 / 316 spare, **471 aggregate and only 468 usable** +(the 3 lines in `FocusAndThemeTests.cs` cannot hold a test method). + +**Why:** #484 (closes #480/#481/#483/#484/#485) needs roughly 443 lines of new MSTest+Moq+FluentAssertions +tests under CSharpier's 100-column wrapping — a ~25-line aggregate margin. The usual escape hatches are +all closed by the spec: no `.csproj` edit (the alphabetically-ordered `Compile Include` group at +`QuickFiler.Test.csproj:57-175` is shared with sibling epic children), so no new test file and no +`.Part2.cs`; and the changed-file set is itself an acceptance criterion. Moving existing tests between +owned files creates no capacity because the aggregate is the binding constraint. + +**How to apply:** +1. Make the per-file 500-line cap the binding acceptance on every test-adding task, and state the + aggregate arithmetic (headroom, planned addition, margin) in a Phase 0 budget artifact. +2. Treat the per-group file assignment as a starting allocation, and explicitly permit relocation to a + different OWNED test file under a header comment naming the issue. A rigid per-file mandate is not + plannable at a 25-line margin. +3. Mandate the compaction levers up front: one shared private arrange helper per test group, and a + `[DataTestMethod]` with one `[DataRow]` per case where every case asserts the same outcome shape. + Record that each `[DataRow]` surfaces as its own TRX result, which is what satisfies a spec clause + reading "each case has its own regression test". +4. Define the escalation as a recorded blocker artifact — never a `.csproj` edit, a new file, or a + file left above 500 lines. +5. Do NOT refactor the two pre-existing headless real-`ItemViewer` tests in `EventWiringTests.cs` + (lines 229-309 and 319-372) to reclaim lines: an acceptance criterion counts real-`ItemViewer` + constructions across the four files (baseline 4, expected 5), and extracting a shared fixture + changes that arithmetic. + +Sizing reference for the same area: a compact seam-and-inject MSTest case is ~13-18 lines; a +16-assertion Moq `VerifyRemove` block is ~52; a headless real-`ItemViewer` fixture test is ~80. + +Related: [[literal-call-clauses-block-file-size-tightening]], [[project_400_partial_class_headroom_placement]], +[[feedback_postformat_file_size_audit]]. diff --git a/.claude/agent-memory/orchestrator/MEMORY.md b/.claude/agent-memory/orchestrator/MEMORY.md index 6f72f96cf..3539f3bc8 100644 --- a/.claude/agent-memory/orchestrator/MEMORY.md +++ b/.claude/agent-memory/orchestrator/MEMORY.md @@ -63,7 +63,7 @@ - [Model-routing: use the portable PS modules](model-routing-scripts-absent-on-epic-integration-base.md) — when scripts/dev_tools is absent, .claude/lib/model-routing/ModelRouting.psm1 has Get-ComplexityFloor / Resolve-DelegationModel (param is -Band) - [Swordfish epic F5 ScoDictionary blocker (RESOLVED)](project_swordfish_epic_f5_blocked_on_old_scodictionary.md) — grep the OLD class base and using, not just the *New replacement - [Epic-child stale local integration ref](project_epic_child_stale_local_integration_ref.md) — `git fetch` and branch from `origin/`, never the bare local name -- [Portable completion gate allows blocked child](portable-completion-gate-allows-blocked-child.md) — no Python validator here, so a blocked child can still terminate and open a PR +- [Portable completion gate is FULL parity (corrected)](portable-completion-gate-allows-blocked-child.md) — not lenient; needs required_agents/skills/mcp_tools arrays IN the checkpoint, with a bug-route tool swap - [Epic-child rebase shared-memory conflict](epic-child-rebase-shared-memory-conflict.md) — the only rebase conflict is the shared MEMORY.md index; union it, no rebuild needed - [Epic-child PR-gate gotchas](epic-child-pr-gate-gotchas.md) — isolated-worktree collect_pr_context writes to the wrong checkout; the hook reads nested epic_context - [Parallel preparation children share one worktree](parallel-preparation-children-shared-worktree.md) — prep-mode children run concurrently in ONE dir; use a child-scoped checkpoint path diff --git a/.claude/agent-memory/orchestrator/epic-kickoff-facts-need-independent-measurement.md b/.claude/agent-memory/orchestrator/epic-kickoff-facts-need-independent-measurement.md index 80f79edc1..c2bca099e 100644 --- a/.claude/agent-memory/orchestrator/epic-kickoff-facts-need-independent-measurement.md +++ b/.claude/agent-memory/orchestrator/epic-kickoff-facts-need-independent-measurement.md @@ -31,6 +31,21 @@ kickoff disagreed and the plan was right. - Record the correction in the checkpoint (a `plan_drift_notes`-style key) so the epic parent can see the kickoff defect rather than silently inheriting it. +**Second instance, 2026-08-24 (#476 child of `quickfiler-bug-family`).** The kickoff asserted that +the `Compile Include` ItemGroup at `QuickFiler.Test/QuickFiler.Test.csproj:57-175` "is alphabetically +ordered", and instructed that any new test file's entry go "in the alphabetical WebView2* +neighbourhood". The group is NOT alphabetically ordered — around the WebView2 entry it reads +`...SeamFactoryTests`, `MailItemActionsAdapterTests`, `WpfUiDispatcherTests`, +`WebView2CoreInitializerTests`, `QfcQueueTests`. Acting on the stated ordering would have meant +re-sorting an ItemGroup the kickoff itself says is "shared with siblings" — a large gratuitous diff +in exactly the file most likely to conflict at fan-in. The salvaged spec had already caught this and +recorded it under a `### Premise correction` heading, and the plan's own decision (insert two +contiguous lines immediately after the existing WebView2 entry, do not re-sort) was right. + +Note the shape: both instances are kickoff claims about a file's *structure* used to prescribe an +action. `sed -n '57,175p'` settles it. When the kickoff and the salvaged/committed artifacts +disagree, the artifacts were written against the tree — and both times they were correct. + Related: [[feedback_plan_phase0_paths_are_stale_in_epic_children]] (the same kickoff also named a stale preparation worktree as WORKTREE), [[feedback_verify_subagent_capability_claims]], -[[feedback_verify_child_preflight_clearance]]. +[[feedback_verify_child_preflight_clearance]], [[parallel-epic-children-name-collisions]]. diff --git a/.claude/agent-memory/orchestrator/portable-completion-gate-allows-blocked-child.md b/.claude/agent-memory/orchestrator/portable-completion-gate-allows-blocked-child.md index e40e5f329..423ea24dc 100644 --- a/.claude/agent-memory/orchestrator/portable-completion-gate-allows-blocked-child.md +++ b/.claude/agent-memory/orchestrator/portable-completion-gate-allows-blocked-child.md @@ -1,12 +1,58 @@ --- name: portable-completion-gate-allows-blocked-child -description: This repo lacks the Python orchestrator-state validator, so the orchestrator SubagentStop completion hook uses the portable PowerShell path, which does NOT require full completion — a blocked/partial child orchestrator can terminate cleanly. +description: CORRECTED 2026-08-24 — the portable PowerShell completion gate is now FULL --require-complete parity, not lenient; it also demands required_agents/required_skills/required_mcp_tools arrays inside the checkpoint matching the routing matrix, with a bug-route tool swap. metadata: type: project --- -In TaskMaster, `python -c 'import scripts.dev_tools.validate_orchestration_artifacts'` fails (module not present). The orchestrator SubagentStop hook `validate-orchestrator-output.ps1` therefore falls back to the portable `Test-OrchestratorStateCompletionReadiness` (`.claude/lib/orchestrator-state/OrchestratorStateCompletion.psm1`) via the `Test-PythonOrchestratorValidatorAvailable` seam. +**This memory previously said the portable gate was lenient. That is no longer true.** Issue #475 +rewrote `.claude/hooks/validate-orchestrator-output.ps1` so the portable PowerShell path is the ONLY +path: the interpreter-subprocess leg and its capability probe are both gone. `scripts/dev_tools/` +does not exist in TaskMaster, but that no longer buys any leniency. -**Why this matters:** the portable path does NOT enforce full completion (no require-complete). It only checks: (a) base presence — all `REQUIRED_STATE_KEYS` present, every `step5..10_status` in the enum, `blocked_reason` in the enum; and (b) the model-routing existence gate — every delegated agent (`delegation_receipts[].agent_name` + a delegating `next_step`) has a matching `model_routing_receipts[].agent`. It does NOT recompute the model formula (Python-only, documented non-goal). +For `artifact_type: orchestrator-state` the hook runs +`Test-OrchestratorStateCompletionReadiness` (`.claude/lib/orchestrator-state/OrchestratorStateCompletion.psm1`), +described in its own header as a row-by-row port of the Python surface +`--require-complete --require-model-routing`. It composes step statuses, `blocked_reason`, the +`pr_gate` and `ci_gate` contracts, phase completeness, the routing contract, the preparation +terminal contract, and the model-routing gate. Only `epic-orchestrator-state` and +`parallel-orchestrator-state` get the lenient structural check (exists / parses / object root). -**How to apply:** a child orchestrator that legitimately halts (e.g. `blocked_pending_maintainer_ratification`) CAN terminate cleanly. To also open the PR past `enforce-pr-author-skill.ps1`, set `blocked_reason: "none"` and steps 5-8 to a non-`pending`/non-`blocked` status (e.g. `verified`/`not-applicable`), and keep `local_execution_overrides`/`delegation_bypasses` empty. Record the real halt state in `next_step` (a non-delegating label so it does not trip the routing gate), plus custom fields like `terminal_status`/`ratification_pending`. `blocked_reason`'s enum has no "maintainer ratification" value — the halt is expressed outside that field. See [[orchestrator-state-flat-keys-and-enum]] and [[pr-author-hook-blocks-gh-in-this-repo]]. +**The non-obvious requirement: the checkpoint must carry the route's required-* lists itself.** +`Get-OrchestratorStateRoutingContractError` compares three checkpoint arrays against +`config/orchestration-routing.json` and emits, for a mismatch or absence: + +``` +Checkpoint required_agents must match routing matrix for route . +Checkpoint required_skills must match routing matrix for route . +Checkpoint required_mcp_tools must match routing matrix for route . +``` + +So `required_agents`, `required_skills`, and `required_mcp_tools` are top-level checkpoint keys, not +just config. **Bug-route swap:** when `promotion-type` is exactly `"bug"`, every +`new_potential_entry` occurrence in the route's `required_mcp_tools` is substituted with +`new_potential_bug_entry`, preserving matrix order. The substituted list drives BOTH the exact-match +check and the receipt-presence loop, so record `new_potential_bug_entry` in the checkpoint array and +in `mcp_call_receipts` — never the feature-type name. See +[[completion-gate-receipt-shapes]] for the `evidence` key that receipts also need +(`skill_receipts` need `skill` + `required: true` + non-blank `evidence`; `mcp_call_receipts` need +`tool` + `ok: true` + non-blank `evidence`). + +**Preparation route is explicitly modelled.** `Get-OrchestratorStatePreparationTerminalError` is +value-gated on the RAW route value being exactly `preparation` +(`route_id` when the KEY is present, else `path_selected`). It then requires `next_step` to equal the +preparation sentinel and ALL SIX `step5..10_status` keys to read exactly `not-applicable`. A +preparation run therefore passes the completion gate cleanly without asserting completion. + +**How to apply:** stop assuming a partial or halted child slides past this hook. Run the gate +yourself before terminating — it is a two-line PowerShell call and it names each violated invariant: + +``` +Import-Module ./.claude/lib/orchestrator-state/OrchestratorStateCompletion.psm1 -Force +Test-OrchestratorStateCompletionReadiness -CheckpointPath artifacts/orchestration/orchestrator-state.json +``` + +`ExitCode 0` with empty `Output` is the pass. Related: +[[orchestrator-state-flat-keys-and-enum]], [[orchestrator-state-validator-divergence]], +[[blocked-reason-enum-cannot-express-substantive-halt]], +[[checkpoint-bootstrap-blocked-by-its-own-gate]]. diff --git a/.claude/agent-memory/prd-feature/MEMORY.md b/.claude/agent-memory/prd-feature/MEMORY.md index 147aa8bff..e30c776d3 100644 --- a/.claude/agent-memory/prd-feature/MEMORY.md +++ b/.claude/agent-memory/prd-feature/MEMORY.md @@ -5,3 +5,4 @@ - [full-bug means spec.md is the only AC source](feedback_full_bug_spec_only.md) — no user-story.md by default (Expected Outputs header vs AC-tracking skill); two exceptions (epic-prep route, cross-reference instruction) handled by making it checkbox-free narrative with a banner - [#522 nullable type-check — RESOLVED in CLAUDE.md](project_522_nullable_typecheck_deviation.md) — as of 2026-08-26 quote CLAUDE.md's msbuild commands directly; no deviation note; keep the /t:Rebuild non-vacuity reasoning - [MSBuild non-vacuity assertion](project_msbuild_nonvacuity_assertion.md) — prove a build compiled with zero `Skipping target "CoreCompile"`; csc.exe counts and CoreCompile headers both mislead +- [UiThread seam conversion belongs to #584](project_uithread_static_seam_belongs_to_584.md) — don't open a new issue for the IUiDispatcher seam (~62 refs/29 files); #584 and #493 share the same static root diff --git a/.claude/agent-memory/prd-feature/project_uithread_static_seam_belongs_to_584.md b/.claude/agent-memory/prd-feature/project_uithread_static_seam_belongs_to_584.md new file mode 100644 index 000000000..27796c70d --- /dev/null +++ b/.claude/agent-memory/prd-feature/project_uithread_static_seam_belongs_to_584.md @@ -0,0 +1,22 @@ +--- +name: uithread-static-seam-belongs-to-584 +description: The IUiDispatcher seam conversion (~62 refs / 29 production files) should attach to existing issue #584, not a new issue — #584 and #493 share the UiThread.Dispatcher static as root object +metadata: + type: project +--- + +Do not promote a new issue for "replace the `UiThread.Dispatcher` static with the existing +`IUiDispatcher` seam". Record the scope on existing issue **#584** instead, and cross-link #493. + +**Why:** #584 (`UiThread.Dispatcher null race`) was promoted by epic child #449 and its recorded +structural root cause is that `UiThread.Dispatcher` is "backed by a `null!`-initialised static with +no lazy initialisation" (`docs/features/archive/2026-08-07-quickfiler-explorer-controller-latent-defects-449/policy-audit.2026-08-22T10-58.md:75`; +listed as an open follow-up at `docs/features/epics/quickfiler-suite-determinism-foundation/epic-status.md:168`). +#493 is unrestored/unsynchronized *mutation* of the same static. Different symptoms, same root +object, and the seam conversion dissolves both. A third issue would fragment the tracking. + +**How to apply:** When any feature touching `UtilitiesCS/Threading/UiThread.cs` recommends the seam +conversion as follow-up, check #584 first. The measured scope is ~62 references across 29 +first-party production files (heaviest: `QuickFiler/Controllers/QfcCollectionController.cs`, +`QfcQueue.cs`, `QuickFiler/Helper Classes/ItemViewerQueue.cs`, +`TaskMaster/Ribbon/RibbonViewer.EngineCommands.cs`). Related: [[promote-latent-defects-to-issues]]. diff --git a/.claude/agent-memory/task-researcher/MEMORY.md b/.claude/agent-memory/task-researcher/MEMORY.md index d1ace4550..7b5a64586 100644 --- a/.claude/agent-memory/task-researcher/MEMORY.md +++ b/.claude/agent-memory/task-researcher/MEMORY.md @@ -32,7 +32,14 @@ - [ribbon-toggle-state-guards-505](project_ribbon_toggle_state_guards_505.md) — #505/#506/#518: toggle(config) vs command(InboxEngines) guard asymmetry; catalog<->XML set-equality tests; MessageBox in notify sink blocks viewer-level tests (2026-08-08) - [coverage-threshold-reconciliation-494](project_coverage_threshold_reconciliation_494.md) — #494: 85/75 is foreign-import leakage reintroduced after #178 rejected it; the only numeric coverage gate is evadable by withholding its input; repo-wide coverage has a +/-15pt run-to-run spread (2026-08-10) - [cobertura-root-attrs-raw-vs-postprocessed](project_cobertura_root_attrs_raw_vs_postprocessed.md) — #441/#478: raw dotnet-coverage root totals are class-level-only; the Koverage post-processor overwrites them with a doubled both-axes sum — never compare the two (2026-08-10) +- [breadcrumb-navigation-defects-439-440-498-499](project_breadcrumb_navigation_defects_439_440_498_499.md) — fixing the #439 lineage REGRESSES the percentage join + filing target; #440 contradicts landed #400 AC-9; Efc/Qfc use different html documents (2026-08-24) +- [qfc-item-controller-defects-484](project_qfc_item_controller_defects_484.md) — #484 (closes #480/#481/#483/#484/#485): all 5 promoted-potential "Suspected Fix" sections were wrong; verify callers + interface decls + coverage-exempt enclosure first (2026-08-24) - [toolchain-gate-fidelity-512](project_toolchain_gate_fidelity_512.md) — #512/#492/#509/#522: AGENTS.md/.agents/.github-instructions are externally owned (generator absent); Invoke-VSBuild.ps1 is an unenumerated executable carrier; ~1.2s vs ~17s = vacuity tell (2026-08-10) +- [webview2-host-initializer-defects-476](project_webview2_host_initializer_defects_476.md) — #476/#458/#477: EfcViewerQueue is NOT a recycle pool (#458 latent); real WebView2 controls ARE built in QuickFiler tests; CaptureProductionDispatcher captures nothing from the control (2026-08-24) +- [uithread-dispatcher-restore-scope-493](project_uithread_dispatcher_restore_scope_493.md) — #493: never share ONE semaphore between a brief helper and a long fixture swap (2-lock split); editorconfig analyzer catch-all makes IDisposable returns safe; CI runs QuickFiler.Test serially, runsettings do not (2026-08-24) +- [qfc-efc-metrics-442](project_qfc_efc_metrics_442.md) — #442/#443/#451: QFC MoveAndIterate stopwatch race unfixable in owned files; legacy csproj blocks any new .cs; metrics CSV has zero in-repo readers (2026-08-24) +- [qfc-collection-defects-468](project_qfc_collection_defects_468.md) — #468: MovedMails already pushed by EmailFiler (param redundant); no log4net ref in QuickFiler.Test; ConcurrentDictionary order blocks a deterministic pre-fix test (2026-08-24) +- [qfc-collection-controller-defects-468](project_qfc_collection_controller_defects_468.md) — #468 family: #474 doc's "unrelated interfaces" claim is FALSE (IQfcFormController derives from IFilerFormController); #469-4 undo is NOT broken; #474-2 fix needs a non-owned file (2026-08-24) - [efc614-store-root-stem-leak](project_efc614_store_root_stem_leak.md) — #614: verbatim ToArchiveRelativePath + unvalidated DestinationOlStem; #609 fix is FolderPredictor-only by design; FolderConverterTests.cs:329 codifies a bug (2026-08-26) ## Artifact hygiene diff --git a/.claude/agent-memory/task-researcher/project_breadcrumb_navigation_defects_439_440_498_499.md b/.claude/agent-memory/task-researcher/project_breadcrumb_navigation_defects_439_440_498_499.md new file mode 100644 index 000000000..c262b5bf1 --- /dev/null +++ b/.claude/agent-memory/task-researcher/project_breadcrumb_navigation_defects_439_440_498_499.md @@ -0,0 +1,45 @@ +--- +name: breadcrumb-navigation-defects-439-440-498-499 +description: Breadcrumb feature (#439/#440/#498/#499) research 2026-08-24 — fixing the lineage defect REGRESSES the percentage join and the filing target; #440 contradicts landed #400 AC-9 +metadata: + type: project +--- + +Research for `docs/features/active/breadcrumb-router-navigation-defects-498/` (2026-08-24, HEAD 988e819b). +Four counter-intuitive facts that cost real tracing time and are not visible from reading any one file. + +**1. The percentage works BECAUSE the lineage is broken.** `BreadcrumbRowBuilder.BuildRow` joins the +probability on `segments[last].FullPath`. With the chain unresolved, that is the presented text — which +equals the `FolderScorer` key, so the join hits. Resolve the chain and it becomes the full Outlook path +while the score index stays keyed on the archive-relative stem, so **every suggestion row silently loses +its percentage**. The #439 potential feared the opposite. Same shape hits the filing target: +`SelectedFolderPath = row.LeafSegment?.FullPath` flips from stem to full path, and `EfcDataModel` passes +it as `DestinationOlStem` next to `OlAncestor = ArchiveRootPath`. Existing router tests do not catch +either, because the provider mock echoes the presented path back as the key. + +**Why:** the presented row text is an archive-root-relative stem (`FolderPredictor.GetOlSubpath` against +`_globals.Ol.ArchiveRootPath`), while `OutlookFolderHierarchyProvider.ResolveLeafKeyAsync` compares against +the raw full `MAPIFolder.FolderPath`. The snapshot node's *other* field, `RelativePath`, does not help — +it is **store**-root-relative (`Archive\Projects\Alpha`), not archive-root-relative (`Projects\Alpha`). + +**How to apply:** any future breadcrumb path-form work must carry explicit ACs for the percentage join AND +the filing target, and must use a `MockBehavior.Strict` path-form-sensitive provider mock so the RED test +fails for the intended reason. + +**2. #440 contradicts a landed, checked-off acceptance criterion.** `#400 spec.md:247` AC-9 is `[x]`: +"Left and Right preserve the existing breadcrumb expand, collapse, and unhandled-key behavior". It is +enforced by a live test (`FolderBreadcrumbAssetContractTests.LeftAndRightBreadcrumbMessages_RemainSupported`). +#440 proposes changing exactly that. **How to apply:** treat this as a spec-level supersession decision, +never as an implementation detail. + +**3. Efc and Qfc use DIFFERENT breadcrumb documents.** `FolderBreadcrumb.html` is Qfc only. The Efc surface +generates its document from `BreadcrumbDocumentAssets.BridgeJs` + `BreadcrumbHtmlRenderer`, whose separator +is `>`, not `→`. Several #439 citations point at the wrong surface. **How to apply:** when a breadcrumb +report says "the html", establish which surface first. + +**4. `_host.Raise(h => h.MessageReceived += null, _host.Object, json)` is the only seam that exercises the +`async void` host-event boundary** (`BreadcrumbBridgeRouterQueueTests.cs:201`). Every other test calls +`ProcessInboundAsync` directly and therefore cannot observe an escaping exception. + +See also [[qfc-breadcrumb-webview2-351]], [[efcviewer-breadcrumb-webview2-349]], +[[folder-hierarchy-provider-350]], [[qfc-folder-tree-percentage-325]]. diff --git a/.claude/agent-memory/task-researcher/project_qfc_collection_controller_defects_468.md b/.claude/agent-memory/task-researcher/project_qfc_collection_controller_defects_468.md new file mode 100644 index 000000000..18413f0c5 --- /dev/null +++ b/.claude/agent-memory/task-researcher/project_qfc_collection_controller_defects_468.md @@ -0,0 +1,44 @@ +--- +name: qfc-collection-controller-defects-468 +description: Issue #468 feature research (2026-08-24) — two promoted-document claims that are FALSE against source, plus the owned-file-set boundary that blocks the #474-2 fix +metadata: + type: project +--- + +Research for feature `qfc-collection-controller-defects` (issues #286/#468/#469/#470/#471/#473/#474) +found three things a future reader would otherwise re-derive or get wrong by trusting the promoted +potential documents. + +**1. The #474 document's core premise is false against source.** It says +`QuickFiler.Controllers.IQfcFormController` and `QuickFiler.Interfaces.IFilerFormController` are +unrelated siblings and "neither is a superset of the other." In fact `IQfcFormController` DERIVES +from `IFilerFormController`. That makes retyping the `_parent` field the cheap fix (2 owned files) +rather than the expensive one. Also: there are THREE types named `IQfcFormController` in this repo +(`Controllers/`, `Interfaces/`, and a non-compiled `Notes/notes_interfaces.cs`); only the +`Controllers` one has an implementer. + +**2. The #469 defect-4 "undo record silently dropped" hypothesis is wrong; undo is NOT broken.** +`MoveEmailsAsync` really does ignore its `SloStack` parameter, but the caller passes +`_globals.AF.MovedMails` — the same global instance that `EmailFiler.PushToUndoStack` writes to +several layers down the move path. The parameter is redundant, not load-bearing. The document said +this triage could raise severity to High; it does not. + +**3. #474 defect 2 (MessageBox in the `ReadyForMove` getter) cannot be fully fixed inside the +declared owned file set.** Its only consumer is `QfcFormController.EventHandlers.cs`, which is not +owned, and the dialog is the only user feedback on that path — so "let the caller present the UI" +requires a scope extension. The in-scope answer is a behaviour-preserving split (pure predicate plus +thin UI wrapper), not relocation. + +**Why:** All seven promoted documents' line numbers matched the source exactly, which makes them +feel authoritative; the two wrong claims above are analysis conclusions, not line citations, and they +did not survive verification. Trusting them would have produced a much larger and partly +out-of-bounds change. + +**How to apply:** When planning or reviewing any of these seven issues, verify interface inheritance +and cross-layer data flow against source before accepting a promoted document's root-cause +narrative. Related: [[feedback-exemption-audit-check-proven-techniques]]. + +Secondary facts worth keeping: `QfcCollectionController` carries `[ExcludeFromCodeCoverage]`, so +#468's "inflates the coverage denominator" rationale does not currently hold. And `KbdActions`' +`IEnumerable` constructor performs NO duplicate check while both `Add` overloads throw — that +asymmetry is why issue #444's duplicate `Keys.Down` is silent rather than an exception. diff --git a/.claude/agent-memory/task-researcher/project_qfc_collection_defects_468.md b/.claude/agent-memory/task-researcher/project_qfc_collection_defects_468.md new file mode 100644 index 000000000..44fbc24b2 --- /dev/null +++ b/.claude/agent-memory/task-researcher/project_qfc_collection_defects_468.md @@ -0,0 +1,48 @@ +--- +name: qfc-collection-defects-468 +description: Issue #468 test-feasibility findings - MovedMails already populated by EmailFiler (so the MoveEmailsAsync param is redundant), log4net absent from QuickFiler.Test, ConcurrentDictionary ordering blocks a deterministic pre-fix test +metadata: + type: project +--- + +Four non-obvious findings from the #468 (`QfcCollectionController`) test-feasibility research +(2026-08-24). Full artifact: +`docs/features/active/qfc-collection-controller-defects-468/research/test-harness-feasibility.md`. + +1. **#469 defect 4 is probably "remove the parameter", not "populate it".** The undo stack handed to + `MoveEmailsAsync` is `_globals.AF.MovedMails`, and that same stack IS already pushed to on the real + move path (`QfcItemController.MoveMailAsync` enqueues an `EmailFiler`, which pushes at + `UtilitiesCS/EmailIntelligence/EmailParsingSorting/EmailFiler.cs:188`). The potential document's + worry that "undo-after-move is broken" is therefore not supported. + **Why:** the resolution choice changes the owned-file footprint - removing the parameter forces an + edit to `QuickFiler/Controllers/QfcFormController.EventHandlers.cs:225`, which #468's `issue.md` + does NOT list as owned. + **How to apply:** re-verify the EmailFiler push before recommending either resolution; do not + assume the parameter is load-bearing just because it is on the interface. + +2. **`QuickFiler.Test` has no log4net reference at all** - only a binding redirect in `app.config`. + Any "assert exactly N log entries" acceptance criterion in a QuickFiler feature needs either a new + package + csproj reference, or a delegate error-sink seam. + **Why:** the file's own ratified precedent is a delegate seam (`_removeGroupByEntryId`, + `QfcCollectionController.cs:1060-1074`), which is smaller than a `MemoryAppender` and avoids + process-wide log4net state. + **How to apply:** prefer an observable proxy first - e.g. `VerifyGet(x => x.Subject, Times.Never())` + proves a double-dereference was removed without counting logs at all. + +3. **A `ConcurrentDictionary`-ordering defect has no deterministic pre-fix red state.** Reference-type + keys hash by runtime identity, so "index i resolves to a different group after a mutation" passes on + some runs. + **Why:** the determinism rule in `.claude/rules/general-unit-test.md` forbids shipping that test. + **How to apply:** pair a deterministic structural guard (assert the backing field's declared type is + order-preserving, in the style of `QuickFiler.Test/NoLiveFormInTestAssemblyTests.cs`) with a + post-fix behavioural ordering test, and record the missing pre-fix red in the dossier. + +4. **`UiThread.Init()` shows a real form** (`UtilitiesCS/Threading/UiThread.cs:54`), so + `UiThread.Dispatcher` is permanently null in tests and any code path reaching + `await UiThread.Dispatcher.InvokeAsync(...)` NREs. + **Why:** that NRE is useful - it is the cheapest way to force a mid-method throw when testing + try/finally hygiene. + **How to apply:** never call `UiThread.Init()` from a test; treat the null dispatcher as a seam. + +See also [[winforms-testability-epic-298]] for the ratified STA-control last-resort rule +(`*.StaTests.cs` + `[STATestClass]`), which applies to the one `TableLayoutPanel`-bound test here. diff --git a/.claude/agent-memory/task-researcher/project_qfc_efc_metrics_442.md b/.claude/agent-memory/task-researcher/project_qfc_efc_metrics_442.md new file mode 100644 index 000000000..f7c2c47f7 --- /dev/null +++ b/.claude/agent-memory/task-researcher/project_qfc_efc_metrics_442.md @@ -0,0 +1,46 @@ +--- +name: qfc-efc-metrics-442 +description: "#442/#443/#451 metrics research: QFC MoveAndIterate stopwatch race is unfixable in owned files; legacy csproj blocks any new .cs; session-metrics CSV has zero in-repo readers" +metadata: + type: project +--- + +Research findings for epic child `quickfiler-home-controller-metrics` (#442 flush, #443 duration +misread, #451 EFC inert duration), 2026-08-24. Owned files were the five +`Qfc/EfcHomeController*.cs` partials. + +**Four non-obvious constraints that cost real time to establish:** + +1. **`QuickFiler.csproj` and `QuickFiler.Test.csproj` are legacy non-SDK with explicit + ``.** Neither was in the owned-file list, so **no new production or test + `.cs` file could be created at all**. Every design had to land in an existing partial. This + invalidates the usual "extract to a new file to stay under 500 lines" escape hatch — check the + csproj style and its ownership before proposing any new file in QuickFiler. + +2. **The QFC `MoveAndIterate` stopwatch race cannot be fixed from the home controller.** + `SwapStopWatch()` lives in `QfcHomeController.Iteration.cs` and all three call sites are in + `QfcFormController.EventHandlers.cs` — both owned by feature 446. On the queue-non-empty branch + the swap (via `LoadUiFromQueue`) races the metrics write (via a non-awaited `BackGroundMoveAsync` + task). Three owned-file workarounds were evaluated and all fail: a property-setter snapshot fixes + *which value* not *when*; self-swapping from the writer creates a second race; capturing at + `CacheMoveObjects()` needs two forbidden files. The end-of-database branch IS deterministic and + IS fixable with one line. Do not re-derive this. + +3. **The session metrics CSV (`Globals.FS.Filenames.EmailSession`) has zero readers in the + repository** — a code-file-type grep returns only three writers plus the settings plumbing. Any + column-shape change is therefore low-risk, which is what unblocked the EFC missing-separator fix. + +4. **`TimeProvider.GetTimestamp()` / `GetElapsedTime()` DO work on net481 here**, proven + behaviourally by compiling production code in `QfcStreamingDequeueConfidenceGate.cs`, not by + reading a manifest (the `packages/` dir is unrestored in agent worktrees). But `Stopwatch` is not + TimeProvider-driven and abstracting it would require editing `IFilerHomeController` — out of + reach. + +**Why:** the epic split ownership file-by-file across siblings 446/464/468, so several "obvious" +fixes sat one file outside scope. Establishing *which* half of a defect is reachable was the +expensive part of the research, not finding the defect. + +**How to apply:** when researching another child of this epic, first map the call graph across the +ownership boundary and state decisively which half is reachable — that is the finding the +orchestrator actually needs. See [[winforms-testability-epic-298]] for the sibling-coordination +pattern. diff --git a/.claude/agent-memory/task-researcher/project_qfc_item_controller_defects_484.md b/.claude/agent-memory/task-researcher/project_qfc_item_controller_defects_484.md new file mode 100644 index 000000000..c81c16f9b --- /dev/null +++ b/.claude/agent-memory/task-researcher/project_qfc_item_controller_defects_484.md @@ -0,0 +1,38 @@ +--- +name: qfc-item-controller-defects-484 +description: "#484 epic child (closes #480/#481/#483/#484/#485): promoted-potential Suspected Fix sections were wrong in 5 of 5 issues; verify callers + interface declarations before planning" +metadata: + type: project +--- + +Feature `qfc-item-controller-defects` (issue #484) closes five pre-existing `QfcItemController` bugs. +Research on 2026-08-24 found that **every one of the five promoted potential documents had a materially +wrong or incomplete "Suspected Fix" section**, while their *diagnoses* were accurate. + +**Why:** the potentials were captured during preparation research for a different epic child (#453/F10) as +drive-by observations. They were never validated against caller sets, interface declarations, or test +reachability, because that was out of scope at capture time. + +**How to apply:** when planning from a promoted potential in this repository, treat the Affected Code and +Why-This-Is-a-Defect sections as authoritative and the Suspected Fix section as an untested hypothesis. +Before accepting a suggested fix, check in this order: + +1. **Enumerate callers of the defective member.** #480's potential said "some caller may have been written to + compensate"; there are in fact ZERO production callers of the one-arg `ToggleNavigation(bool)` overload — + all four `QfcCollectionController` sites use the two-arg overload. Removal was unconditionally safe and + the caution was unnecessary. +2. **Check whether the member is on a public interface.** #483's potential offered "rethrow OR return a + failure result"; the return-result option is impossible because `Task MoveMailAsync()` is declared on + `IQfcItemController` and implemented by the out-of-scope `EfcItemController`. Half the suggested option + space did not exist. +3. **Check whether the fix lands in a coverage-exempt member.** #485's suggested in-place `Uri.TryCreate` + guard is code-correct but would have added zero covered lines and zero regression tests, because the + enclosing `InitializeWebViewAsync` is `[ExcludeFromCodeCoverage]` and needs a live WebView2 runtime. + Extraction into a pure member was required to make the fix verifiable at all. +4. **Check whether the named remedy is sufficient.** #484's "dispose the timer" is necessary but does not + abort an in-flight callback; the callback itself dereferences four fields `Cleanup()` nulls. +5. **Recount anything the potential quantified.** #481 claimed "25 `+=` in EventWiring.cs"; the real figure + is 22 event subscriptions (the count included two commented-out lines and one arithmetic `+=`). + +Related: [[qfc-item-controller-227-r2-denial]], [[feedback-exemption-audit-check-proven-techniques]], +[[qfc227-headless-itemviewer-and-tlpcellsnapshot]]. diff --git a/.claude/agent-memory/task-researcher/project_uithread_dispatcher_restore_scope_493.md b/.claude/agent-memory/task-researcher/project_uithread_dispatcher_restore_scope_493.md new file mode 100644 index 000000000..1a9e70f53 --- /dev/null +++ b/.claude/agent-memory/task-researcher/project_uithread_dispatcher_restore_scope_493.md @@ -0,0 +1,43 @@ +--- +name: uithread-dispatcher-restore-scope-493 +description: "Issue #493 research: a single shared gate for both the Ensure helper and the pump-fixture swap is unsafe; the two-lock split, plus the editorconfig analyzer catch-all and the CI-vs-runsettings parallelization asymmetry" +metadata: + type: project +--- + +Researched 2026-08-24 for issue #493 (`QfcItemControllerTestSupport.EnsureUiThreadDispatcher` never +restores `UtilitiesCS.UiThread._dispatcher`). Four findings that are not visible from a single read +of the code: + +1. **Do not put the "brief" helper and the long-lived pump-fixture swap on the SAME semaphore.** + The obvious design (one shared `SemaphoreSlim`, briefly acquired by the helper) has three + failure modes: the helper's two unowned call sites in `QfcItemController.FocusAndThemeTests.cs` + carry no `[Timeout]`, so they block up to 60 s and hang FOREVER if a pump test expires before + its restore; and any regression test that holds the gate and then calls the helper self-deadlocks + (`SemaphoreSlim` is not reentrant). Correct split: a Monitor guarding each individual reflection + read-modify-write (atomicity, taken by everyone) plus a separate `SemaphoreSlim(1,1)` held only by + long transactions (mutual exclusion). Lock order gate -> field lock, never the reverse. +2. **The real #230 mechanism is a lost update on a check-then-act, not "two long transactions + overlapping."** Making each mutation atomic is what makes it unrepresentable; the long gate alone + was never the load-bearing part for the helper. +3. **`.editorconfig:27` sets `dotnet_analyzer_diagnostic.severity = suggestion` as a global + catch-all** (only `MSTEST0032` is a warning). So `CA2000`/`CA1806`/`IDISP004` cannot break the + toolchain, and changing a helper's return type from `void` to `IDisposable` is safe even at call + sites that discard the result. Check this before proposing an IDisposable-returning API. +4. **CI and local coverage runs disagree about parallelization.** `QuickFiler.Test` has no + `[assembly: Parallelize]`, and `.github/workflows/_mstest-coverage.yml` passes no `/Settings:`, so + it runs SEQUENTIALLY in CI; but `TaskMaster.runsettings` and `scripts/vscode/TaskMaster.cli.runsettings` + both force `Workers=0 / ClassLevel` on every assembly. A regression test for a class-parallelism + race must therefore create its own threads — it cannot rely on MSTest scheduling two classes + concurrently. + +**Why:** these were derived while checking the orchestrator's design hypothesis for holes; the +hypothesis was sound in intent but would have converted a bounded pump-test failure into an +unbounded hang in a file the feature is forbidden to edit. + +**How to apply:** reuse finding 1 whenever a test helper and a test fixture both mutate the same +process-wide static; reuse finding 3 before rejecting an `IDisposable` return type on analyzer +grounds; reuse finding 4 for any regression test targeting a parallelization race in this repo. + +Related: `.claude/agent-memory/atomic-executor/project_uithread_dispatcher_static_swap_race.md`, +`.claude/agent-memory/atomic-executor/project_mstest_donotparallelize_overlaps_parallel_bucket.md`. diff --git a/.claude/agent-memory/task-researcher/project_webview2_host_initializer_defects_476.md b/.claude/agent-memory/task-researcher/project_webview2_host_initializer_defects_476.md new file mode 100644 index 000000000..557c5648d --- /dev/null +++ b/.claude/agent-memory/task-researcher/project_webview2_host_initializer_defects_476.md @@ -0,0 +1,55 @@ +--- +name: webview2-host-initializer-defects-476 +description: "#476/#458/#477 research findings: EfcViewerQueue is NOT a recycle pool so #458's premise is latent; real WebView2 controls ARE constructible in QuickFiler unit tests; BreadcrumbUiDispatcher captures nothing from the control" +metadata: + type: project +--- + +Research completed 2026-08-24 for feature `webview2-host-initializer-defects` (issue #476, closing +#458, #476, #477). Artifact: +`docs/features/active/webview2-host-initializer-defects-476/research/2026-08-24T00-45-webview2-host-initializer-defects-research.md` + +Four findings that contradict or go beyond the issue text and are expensive to re-derive: + +1. **`EfcViewerQueue` is a pre-warm pool of FRESH instances, not a recycle pool.** `ViewerQueueCore` + has no return-to-pool method; every enqueue calls `_viewerFactory()`. Issue #458's "pooled viewer + reuse" premise therefore does not occur in production, and the class XML doc that cites + `EfcViewerQueue` is wrong. The defect is real at the type level but LATENT. A regression test must + be unit-level (two hosts, one control), not a production repro. + **Why:** an executor can burn a cycle trying to reproduce a recycle that does not exist. + **How to apply:** check `ViewerQueueCore` before accepting any "pooled viewer" claim about + QuickFiler viewers. + +2. **`EfcFormController.Cleanup()` does not touch `_breadcrumbHost` or `_router`, and the controller + implements no `IDisposable`.** No disposal path anywhere reaches the breadcrumb host, so an + `IDisposable`/`Detach()` remedy has zero possible callers without editing `EfcFormController.cs`. + **Why:** the "obvious" fix for a handler-retention leak is dead on arrival here. + **How to apply:** for any QuickFiler lifetime fix, verify a caller exists before designing a + Dispose-based remedy. + +3. **Real `Microsoft.Web.WebView2.WinForms.WebView2` controls ARE already constructed in + QuickFiler unit tests** — transitively via `new ItemViewer()` on `WinFormsPumpHost`, and a test + asserts both children report `IsHandleCreated == true`. Constructing the CONTROL needs no + Evergreen runtime; only `EnsureCoreWebView2Async` / `CoreWebView2Environment.CreateAsync` do. + `QuickFiler.Test` references both WebView2 assemblies directly. + **Why:** the standing assumption that "WebView2 cannot exist in a unit-test host" is false and has + been used to justify coverage exemptions. + **How to apply:** when auditing a WebView2 coverage exemption, the barrier is core INIT, not + control construction. + +4. **`WebView2Messenger.CaptureProductionDispatcher(coreWebView)` captures NOTHING from the control.** + It null-guards the argument then returns `BreadcrumbUiDispatcher.CaptureCurrent()`, which captures + the ambient `SynchronizationContext` and thread id. `CaptureCurrent()` THROWS when + `SynchronizationContext.Current` is null, so copying that precedent into a constructor adds a new + throwing precondition. + **Why:** the factory's control parameter reads as if it were the capture source; it is only an + argument-order device to preserve the `ArgumentNullException` contract. + **How to apply:** prefer capturing from an explicitly supplied `SynchronizationContext` (e.g. the + one `InitializeAsync` already receives) over `CaptureCurrent()` at construction. + +Also recorded: `QuickFiler/Viewers/WebView2CoreInitializer.cs` and `IWebViewCoreInitializer.cs` are +NOT `#nullable enable` (per-file opt-in; no `Directory.Build.props`), so adding the directive +conscripts them into the `TreatWarningsAsErrors` gate. `WebView2BreadcrumbHost.cs` IS nullable-enabled. + +Related: [[qfc-breadcrumb-webview2-351]], [[efcviewer-breadcrumb-webview2-349]], +[[winforms-pump-seam-230]], [[qfc-item-controller-227-r2-denial]]. diff --git a/docs/features/parallel/bug-families-01/selection.md b/docs/features/parallel/bug-families-01/selection.md new file mode 100644 index 000000000..9d011a694 --- /dev/null +++ b/docs/features/parallel/bug-families-01/selection.md @@ -0,0 +1,150 @@ +# Parallel run bug-families-01 — selection record + +Run slug: `bug-families-01`. Plan-home branch: `parallel/bug-families-01-plan`. + +Selected 2026-08-23 from the 78 open `bug`-labelled issues on `main @ 988e819b`. + +## Selection rule + +One issue per blast-radius contention family, preferring the highest-impact bug in each family. +Contention is computed at the project-module level, so any two bugs in the same C# project +conflict on `module_overlap` regardless of which files they touch. The distinct families that +actually carry open bugs are: `QuickFiler`, `QuickFiler.Test`, `UtilitiesCS`, `UtilitiesCS.Test`, +`TaskMaster`, and the module-free `scripts/vscode` coverage tooling. Six families therefore set +the ceiling on a single cohort; items 7 and 8 deliberately double up on the two largest +families so the run uses all 8 concurrency slots across an expected 2 cohorts. + +Final cohort assignment is computed from plan-derived blast radii after preparation, not from +this table. The predicted split is an expectation, not a guarantee. + +## Selected items (8) + +| issue | family | predicted cohort | rationale | +| --- | --- | --- | --- | +| #446 | `QuickFiler` | 0 | IterateQueueAsync closes the queue before its deadline, so queued items are dropped silently. Silent data loss is the most severe class in this family. | +| #504 | `TaskMaster` | 0 | Ribbon callback names referenced by the ribbon XML no longer resolve, so the callbacks are dead. Only selected bug whose radius is TaskMaster/Ribbon/**. | +| #584 | `UtilitiesCS` | 0 | UiThread dispatcher null race in ProgressTrackerAsync. A race in shared threading infrastructure is used by every downstream project. | +| #516 | `UtilitiesCS.Test` | 0 | TimeoutAfter tests race a real wall-clock deadline, violating the determinism-infrastructure rule in general-unit-test.md (banned real wall-clock waits). | +| #531 | `scripts/vscode` | 0 | MSTest coverage discovery does not exclude .claude worktrees, so local coverage runs load stale assemblies. Carries no C# project module, so it is its own contention family. | +| #469 | `QuickFiler.Test` | 0 | QfcCollection move-diagnostics defects. Chosen as the QuickFiler.Test representative; if its plan reaches QuickFiler production sources it will re-cohort with #446. | +| #448 | `QuickFiler` | 1 | UndoConsumer non-terminating loop (hang). Deliberate second QuickFiler item: it conflicts with #446 on module_overlap and is expected to seed cohort 1. | +| #287 | `UtilitiesCS` | 1 | StoreWrapper dialog is imprecise for a genuine failure. Deliberate second UtilitiesCS item: conflicts with #584 on module_overlap and is expected to seed cohort 1. | + +### Titles + +- #446 — Bug: iteratequeueasync-deadline-closes-queue-early +- #504 — Bug: ribbon-dead-callback-names +- #584 — Bug: uithread-dispatcher-null-race-progresstrackerasync +- #516 — Bug: timeoutafter-tests-race-real-wall-clock-deadline +- #531 — Bug: mstest-coverage-discovery-claude-worktree-exclusion +- #469 — Bug: qfc-collection-move-diagnostics-defects +- #448 — Bug: quickfiler-undoconsumer-nonterminating-loop +- #287 — Bug: storewrapper-dialog-imprecise-for-genuine-failure + +## Excluded: not fixable in this checkout (6) + +These target `drm-copilot` MCP tools or `scripts/dev_tools/`. Verified 2026-08-23 that +`extensions/` and `scripts/dev_tools/` are both ABSENT from TaskMaster; TaskMaster is a +C#/PowerShell repository. These belong upstream in the drm-copilot repository. + +- #589 — Bug: collect-pr-context-shared-path-race-across-concurrent-children +- #555 — Bug: orchestrator-hooks-reference-absent-python-validators +- #554 — Bug: potential-to-issue-promoted-copy-not-written +- #546 — Bug: research-doc-cohort-library-false-negative +- #536 — Bug: poshqc-test-coverage-capture-records-zero +- #513 — Bug: collect-pr-context-misclassifies-csharp-as-documentation + +## Excluded: CI / coverage-threshold chain (6) + +This cluster carries real ordering (the threshold policy must be reconciled before any gate is +built against it). The parallel surface cannot express ordering: `depends_on` and `wave` are +prohibited keys and cohorts only guarantee non-concurrency, never sequence. Route to +`/epic-plan` + `/epic-orchestrate`. + +- #565 — Invoke-MSTestWithCoverage.ps1 asserts coverage threshold before Set-Content, leaving the raw un-post-processed Cobertura on disk when the gate fails +- #564 — CLAUDE.md cites ci.yml for three toolchain commands the #553 split moved into reusable workflows +- #563 — Coverage threshold contradiction remains: CLAUDE.md/csharp.md say 80%, general-unit-test.md/quality-tiers.md say 85%/75%, and two live gates disagree +- #562 — No Pester job in CI: production PowerShell under scripts/vscode has zero CI coverage +- #561 — CI collects coverage but enforces no threshold: _mstest-coverage.yml never converts to Cobertura or compares a floor +- #569 — Bug: ci-nuget-cache-fallback-masks-stale-package-refs + +## Deferred to the serial queue (58) + +Predominantly the QuickFiler cluster. Every pair conflicts on `module_overlap`, so scheduling +them in a parallel run yields one item per cohort with a full CI cycle and PR merge between +each. A serial queue delivers the same throughput without the cohort-barrier overhead. +Deferred is not dropped: any of these can be admitted later via `/parallel-add` into an +open-mode run, or planned as their own run once the QuickFiler cluster drains. + +- #597 — Bug: csproj-analyzer-paths-stale-after-dependabot-bump-breaks-fresh-clone +- #586 — Bug: utilitiescs-test-form1-live-form +- #570 — Bug: system-reactive-7-packages-config-unsupported +- #560 — Bug: overload-name-collision-under-exclusion +- #559 — Bug: local-functions-in-exempt-members-remain-counted +- #537 — Bug: cobertura-max-hits-update-branch-untested +- #532 — Bug: agent-memory-cobertura-dedup-generalization-wrong +- #530 — Bug: cobertura-merged-class-methods-incomplete +- #529 — Bug: cobertura-package-rates-not-recomputed +- #525 — Bug: engine-toggle-prime-last-writer-race +- #524 — Bug: ribbon-controller-intelligence-unguarded-globals-deref +- #520 — Bug: console-setout-races-under-class-parallelism +- #502 — Bug: breadcrumb-suggestions-upgrade-silently-stale-on-superseded-lease +- #501 — Bug: breadcrumb-hub-postjson-caches-before-broadcast-starves-attachments +- #500 — Bug: breadcrumb-webview-post-executes-under-upgrade-lifetime-lock +- #499 — Bug: breadcrumb-router-stale-selectedfolderpath-after-rebind +- #498 — Bug: breadcrumb-router-segment-index-unvalidated-host-crash +- #493 — Bug: uithread-dispatcher-static-swap-no-restore +- #490 — Bug: itemviewer-display-and-folder-contract-defects +- #489 — Bug: itemviewer-ui-thread-marshalling-divergence +- #488 — Bug: itemviewer-breadcrumb-pipeline-lifecycle +- #487 — Bug: itemviewer-parentchanged-console-and-cast +- #486 — Bug: itemviewer-move-option-menu-defects +- #485 — Bug: qfc-item-controller-webview-handler-unguarded-inputs +- #484 — Bug: qfc-item-controller-cleanup-timer-and-stale-field-defects +- #483 — Bug: qfc-item-controller-mailactions-error-handling-defects +- #482 — Bug: qfc-item-controller-expansion-registry-divergence +- #481 — Bug: qfc-item-controller-no-event-unwiring-path +- #480 — Bug: qfc-item-controller-togglenavigation-double-toggle +- #477 — Bug: iwebviewcoreinitializer-contract-defects +- #476 — Bug: webview2breadcrumbhost-unmarshalled-sdk-call-and-unsynchronized-state +- #475 — Bug: breadcrumb-capturecurrentortests-silently-degrades-in-production +- #474 — Bug: qfc-collection-controller-coupling-and-modal-getter +- #473 — Bug: qfc-collection-background-task-and-catch-defects +- #472 — Bug: qfc-collection-navigation-digits-desync +- #471 — Bug: qfc-collection-eliminate-space-sign-error +- #470 — Bug: qfc-collection-conversation-index-defects +- #468 — Bug: qfc-collection-controller-unreachable-load-paths +- #467 — Bug: efc-viewer-processcmdkey-swallows-alt-mnemonics +- #466 — Bug: efc-dead-code-and-latent-nre-traps +- #465 — Bug: efc-form-controller-lifecycle-and-selection-defects +- #464 — Bug: efc-controllers-null-guard-and-async-void-boundary-defects +- #463 — Bug: quickfiler-webview2-incognito-arg-en-dash +- #462 — Bug: breadcrumb-dropdown-coordinator-stale-closepending-drops-reopen +- #461 — Bug: efc-item-controller-dead-conversation-expanded-handler +- #460 — Bug: efc-item-controller-cleanup-nre-and-timer-leak +- #459 — Bug: efc-item-controller-keyboard-registration-defects +- #458 — Bug: webview2breadcrumbhost-handler-retention-pooled-viewer +- #451 — Bug: efc-home-controller-metrics-inert-duration +- #444 — Bug: kbdactions-enumerable-ctor-bypasses-duplicate-guard +- #443 — Bug: qfc-home-controller-metrics-duration-misread +- #442 — Bug: qfc-home-controller-metrics-never-flushed +- #440 — Bug: breadcrumb-left-right-arrow-parent-child-navigation +- #439 — Bug: efcviewer-missing-lineage-and-segment-navigation +- #427 — Bug: quickfiler-post-show-duplicate-scoring +- #426 — Bug: emailmovemonitor-rejected-item-hook-retention +- #286 — Bug: qfc-collectioncontroller-removespecificcontrolgroup-counter-leak +- #285 — Bug: timeouttask-runwithtimeout-exception-type-mismatch + +## Contention visible outside this run + +At planning time these branches held in-flight work on QuickFiler and the coverage surfaces, +and are invisible to this run's cohort scheduling: + +- `feature/quickfiler-per-file-coverage-capstone-r2` +- `feature/quickfiler-breadcrumb-bridge-coverage-r2` +- `bug/quickfiler-test-form1-live-form-491-exec` +- `bug/winformspumphost-suite-determinism-511-exec` + +Items #446, #448, #469 (QuickFiler) and #531 (coverage tooling) are the exposed selections. +Re-check these branches before executing the run. + diff --git a/docs/features/potential/promoted/2026-08-28-quickfiler-carry-folder-predictor-to-item-controller.md b/docs/features/potential/promoted/2026-08-28-quickfiler-carry-folder-predictor-to-item-controller.md new file mode 100644 index 000000000..9a8b8515e --- /dev/null +++ b/docs/features/potential/promoted/2026-08-28-quickfiler-carry-folder-predictor-to-item-controller.md @@ -0,0 +1,126 @@ +# quickfiler-carry-folder-predictor-to-item-controller (Issue #678) + +- Date captured: 2026-08-28 (originally identified 2026-08-24; promoted from a stranded worktree during cleanup) +- Author: Dan Moisan +- Status: Promoted -> docs/features/active/quickfiler-carry-folder-predictor-to-item-controller/ (Issue #678) +- Found during: preparation of epic child `quickfiler-queue-datamodel-defects` (primary issue #446) + +> Automation note: Keep the section headings below unchanged; the promotion tooling maps each of them into the GitHub bug issue template. + +- Issue: #678 +- Issue URL: https://github.com/drmoisan/TaskMaster/issues/678 +- Last Updated: 2026-08-28 +## Summary + +Issue #427 reports that every accepted QuickFiler mail item is scored twice in high-confidence mode. +Preparation research for the `quickfiler-queue-datamodel-defects` feature established that the fix +proposed in the original #427 potential document does not actually remove the second scoring pass, +so #427 cannot be fully resolved by carrying the top-folder string alone. This entry records the +remaining consumer-side work. + +## Environment + +- OS/version: Windows 11 Pro 10.0.26200 +- Runtime: C# / .NET Framework 4.8.1 VSTO add-in +- Command/flags used: QuickFiler launched from the TaskMaster ribbon with `QfSettings.HighConfidenceModeEnabled = true` +- Data source or fixture: Live Outlook mailbox + +## Steps to Reproduce + +1. Enable High Confidence mode and launch QuickFiler. +2. Enable debug logging and inspect the `Probability debug` entries for a single accepted item. +3. Observe one entry from the pre-UI scan and a second, independent classification after the form is shown. + +## Expected Behavior + +An item accepted by the confidence gate carries its already-initialised folder predictor forward, so +the item controller populates the folder combo, the suggestion list and the folder array from that +result instead of recomputing them. + +## Actual Behavior + +The initialised predictor is discarded and the full `FolderPredictor.InitAsync(InitOptions.FromField)` +sequence runs a second time per accepted item after `Show()`. + +## Logs / Screenshots + +- [ ] Attached minimal logs or screenshot +- Snippet: two `Probability debug` lines per accepted item, as recorded in the original #427 potential document. + +## Impact / Severity + +- [ ] Blocker +- [ ] High +- [ ] Medium +- [x] Low + +Low: wasted work, not incorrect behavior. It occurs after `Show()`. The user-visible effect is slower +folder-combo population and redundant Outlook COM traffic proportional to the number of items on screen. + +## Suspected Cause / Notes + +Verified at `988e819b` during preparation research for issue #446. Full analysis was recorded at +`docs/features/active/quickfiler-queue-datamodel-defects-446/research/2026-08-24T09-50-quickfiler-queue-datamodel-defects-research.md` +§ 4.5 in the worktree that captured it; that worktree's copy of the feature folder is a superseded +pre-execution draft and was not carried into the merged feature (the merged version does not include +this consumer-side follow-up). + +The original #427 potential document proposed activating the dormant +`QfcFormController.LoadItemsAsync(IList)` overload so the predetermined folder is +carried forward. That premise is incorrect: + +- `_predeterminedFolder` is consumed only for combo-box *selection* inside `AssignFolderComboBox` + (`QuickFiler/Controllers/QfcItemController.FolderHandling.cs:193-199`). +- The surrounding code still requires a fully-initialised predictor: `FolderArray`, `Suggestions` + and `FolderRowArray` all come from `_folderHandler` (`IFolderSearchHandler`, declared + `QuickFiler/Controllers/QfcItemController.cs:41`), which is produced only by + `LoadFolderHandler`/`LoadFolderHandlerAsync`. +- So even on the carrier path the item controller must still run + `FolderPredictor.InitAsync(FromField)`. Carrying only the top-folder string changes which entry is + preselected, a behavior the code already implements, and saves no scoring work. + +Removing the second scoring pass requires carrying the initialised `FolderPredictor` / +`IFolderSearchHandler` from `FolderScoringService.ScoreAsync` +(`QuickFiler/Controllers/QfcHighConfidencePreFilter.cs:184`, where it is discarded) through to +`_folderHandler`. + +Line numbers above were verified against commit `988e819b` (2026-08-24) and should be re-checked +against current `main` before planning, since the referenced files may have moved since. + +## Proposed Fix / Validation Ideas + +Files that must change, none of which were owned by the `quickfiler-queue-datamodel-defects` feature: + +- `QuickFiler/Controllers/QfcHighConfidencePreFilter.cs` — widen `IFolderScoringService.ScoreAsync` to surface the predictor +- `QuickFiler/Controllers/QfcItemGroup.cs:50` — new carried member +- `QuickFiler/Controllers/QfcCollectionController.cs:428-471`, `:616` +- `QuickFiler/Controllers/QfcItemController.cs:41`, `:83-89` +- `QuickFiler/Controllers/QfcItemController.Initialization.cs:63-64`, `:108`, `:398-400` +- `QuickFiler/Controllers/QfcHomeController.cs:310` — the sole overload-selection call site + +Prerequisite already landed by the `quickfiler-queue-datamodel-defects` feature (Scope 427-A): the +producer side no longer discards the scoring result, and the datamodel boundary exposes +`QfcPreScoredItem` carriers on its dequeue batch. Nothing consumes them yet; this entry is that +consumer work. + +Pinned tests that must be deliberately rewritten, not deleted, because they encode the landed +decision of issue #233 that high-confidence enforcement moved from post-display filtering to +dequeue-time gating: + +- `QuickFiler.Test/Controllers/QfcHomeControllerIssue218Tests.cs:137-259` +- `QuickFiler.Test/Controllers/QfcHomeControllerRunAsyncHighConfidenceTests.cs:246`, `:277` + +The `Times.Never` assertion on `HighConfidencePreFilterLoader` should stay: the pre-filter class +remains dormant, and only the carrier overload would become live. + +- [ ] Unit coverage areas: predictor carry-through, `QfcItemController` folder-handler population, overload selection +- [ ] Integration scenario to retest: high-confidence launch, confirming one scoring pass per accepted item and an unchanged folder-combo selection +- [ ] Manual verification notes: compare `Probability debug` log output before and after; confirm the preselected folder matches the previous behavior + +Tests must use MSTest with Moq and FluentAssertions, no live Outlook COM and no temporary files, per +repository unit-test policy. + +## Next Step + +- [ ] Promote to GitHub issue (bug-report template), or attach as a scoped follow-up to issue #427 +- [ ] Coordinate with the epic children that own the six files listed above