diff --git a/.claude/agent-memory/atomic-executor/MEMORY.md b/.claude/agent-memory/atomic-executor/MEMORY.md index b1b924958..65179214d 100644 --- a/.claude/agent-memory/atomic-executor/MEMORY.md +++ b/.claude/agent-memory/atomic-executor/MEMORY.md @@ -2,86 +2,89 @@ ## Plan validation & gates - [Verify line citations with numbered output](feedback_verify_line_citations_with_numbered_output.md) — never hand-count from a `sed` window; a wrong #438 advisory got applied and corrupted 3 correct citations in plan + spec -- [Self-derived gate thresholds are blind](project_preflight_selfderived_gate_thresholds_are_blind.md) — a "count >= floor" gate whose floor comes from the runs it validates is deflation-blind + scope-incommensurable; use git-enumeration + `/ListTests` existence proofs -- [Multi-pattern gates detach shared qualifiers](project_multipattern_gate_shared_qualifier_detachment.md) — rewriting one clause re-scopes the trailing allowlist to the last pattern only; restate the carve-out per pattern + require per-pattern count identity +- [Self-derived gate thresholds are blind](project_preflight_selfderived_gate_thresholds_are_blind.md) — a "count >= floor" gate whose floor comes from the runs it validates is deflation-blind + scope-incommensurable; use +- [Multi-pattern gates detach shared qualifiers](project_multipattern_gate_shared_qualifier_detachment.md) — rewriting one clause re-scopes the trailing allowlist to the last pattern only; restate the carve-out per pattern + - [Merge-base diff gates need a commit cadence](project_preflight_mergebase_diff_gates_need_commit_cadence.md) — `..HEAD` gates are vacuous while HEAD == merge-base; plan an explicit commit task - [Inserted plan tasks force renumbering](project_plan_task_ids_digit_only_forces_renumbering.md) — suffixed IDs (`P3-T5a`) fail validation; say "insert + renumber downstream", then verify defs-vs-mentions mechanically - [Plan rationale clauses are evidence](project_418_plan_rationale_clauses_are_evidence.md) — #418 needed 3 preflight passes; all blockers were unmeasured world-state claims in prose, never in the fix -- [AC check-off "inline pointer" + artifacts/ tool-output paths](project_preflight_ac_checkoff_and_tooloutput_paths.md) — "record the evidence pointer inline" in an AC check-off task violates acceptance-criteria-tracking; artifacts/pester|csharp coverage XML is a producer path, not an evidence path -- [Exact-count gate vs remediation loop](project_exact_count_gate_vs_remediation_loop.md) — a pinned `TotalCount = 19` collides with an "add tests and restart" remediation path; use `B + N` and re-scan count gates whenever a restart clause is added -- [Tracked agent-memory breaks unscoped git gates](project_agent_memory_tracked_breaks_unscoped_git_gates.md) — `.claude/agent-memory/**` is tracked + dirty at branch head; every git diff/status/grep gate needs an explicit pathspec or it is unsatisfiable / false-positive -- [#418 500-line gate vs mandated plan content](project_418_500line_gate_vs_plan_content.md) — P1-T19 unsatisfiable (193 new lines into 146 headroom); per-block logging clauses block centralizing; delta = extract helpers to a new file +- [AC check-off "inline pointer" + artifacts/ tool-output paths](project_preflight_ac_checkoff_and_tooloutput_paths.md) — inline evidence pointer in an AC task violates the tracking skill; artifacts/*/coverage XML is a producer path +- [Exact-count gate vs remediation loop](project_exact_count_gate_vs_remediation_loop.md) — a pinned `TotalCount = 19` collides with an "add tests and restart" remediation path; use `B + N` and re-scan count +- [Tracked agent-memory breaks unscoped git gates](project_agent_memory_tracked_breaks_unscoped_git_gates.md) — `.claude/agent-memory/**` is tracked + dirty; every git diff/status gate needs an explicit pathspec +- [Absolute-zero gate on a sibling-owned assembly + pinned WS](project_preflight_absolute_zero_gate_on_sibling_owned_assembly.md) — `Failed 0` on an assembly a sibling owns is unsatisfiable; never pin an absolute WS in preparation mode +- [#418 500-line gate vs mandated plan content](project_418_500line_gate_vs_plan_content.md) — P1-T19 unsatisfiable (193 new lines into 146 headroom); per-block logging clauses block centralizing; delta = extract - [#207 Hook() redesign breaks AppEventsTests](project_207_hook_redesign_breaks_appeventstests.md) — needs a plan revision, not a test weakening - [ApplicationGlobalsTests.cs at 500-line ceiling](project_appglobalstests_at_500_line_ceiling.md) — exactly 500 lines; adding a Testable* override requires extraction first - [#376 capstone scope-expansion layers](project_376_capstone_scope_expansion_layers.md) — 5 escalated layers resolved via the 3 authorized patterns - [Swordfish F5 test misclassification](project_swordfish_f5_test_misclassification.md) — verify using/namespace before treating a removal as Swordfish-only ## Build / toolchain environment -- [pwsh/git/gh CLI gotchas](project_pwsh_git_gh_cli_gotchas.md) — jq NOT installed (only `gh --jq`); pwsh won't concatenate `$(git merge-base ...)..HEAD`; bare `packages.config` pathspec matches 0 files +- [pwsh/git/gh CLI gotchas](project_pwsh_git_gh_cli_gotchas.md) — jq NOT installed (only `gh --jq`); pwsh won't concatenate `$(git merge-base ...)..HEAD`; bare `packages.config` - [Project Build/Test Env](project_build_test_env.md) — git-bash quirks (MSBuild switches, MSYS_NO_PATHCONV), csharpier v1 syntax, legacy csproj Compile includes, IVT for Moq -- [VS18 build/test toolchain paths](project_vs18_build_toolchain_paths.md) — use VS **18** full-framework msbuild.exe (not .dotnet-sdk, dies on binary resx MSB3822); nuget.exe restore; dotnet-coverage needs `--` separator -- [Repo-local SDK install + nullable Rebuild](project_repo_sdk_and_nullable_rebuild.md) — .dotnet-sdk install needs pwsh7; csharpier check/format subcommands; nullable debt scope NOT stable across sessions — re-verify which csproj errors come from +- [VS18 build/test toolchain paths](project_vs18_build_toolchain_paths.md) — use VS **18** full-framework msbuild.exe (not .dotnet-sdk, dies on binary resx MSB3822); nuget.exe restore +- [Repo-local SDK install + nullable Rebuild](project_repo_sdk_and_nullable_rebuild.md) — .dotnet-sdk install needs pwsh7; csharpier check/format subcommands; nullable debt scope NOT stable across sessions — - [vstest TestCaseFilter OR-vs-pipe + fresh-worktree bootstrap](project_vstest_testcasefilter_or_operator_and_env_setup.md) — vstest rejects `OR`, needs `|`; fresh worktree needs restore + global `dotnet-coverage` -- [Analyzer version skew on fresh worktree](project_analyzer_version_skew_fresh_worktree.md) — analyzer build can fail CS0006 when csproj `` HintPaths diverge from packages.config; compare the two before blaming restore -- [SecurityCodeScan incompatible with Roslyn 5.6](project_securitycodescan_roslyn56_incompat.md) — SecurityCodeScan.VS2019 5.6.7 throws CS8032/YamlDotNet under VS18, breaking TWAE; Meziantou/Roslynator need roslyn-version subfolders +- [Analyzer version skew on fresh worktree](project_analyzer_version_skew_fresh_worktree.md) — analyzer build can fail CS0006 when csproj `` HintPaths diverge from packages.config; compare the two +- [SecurityCodeScan incompatible with Roslyn 5.6](project_securitycodescan_roslyn56_incompat.md) — SecurityCodeScan.VS2019 5.6.7 throws CS8032/YamlDotNet under VS18, breaking TWAE; Meziantou/Roslynator need - [Missing VSTO runtime breaks baseline gates](project_missing_vsto_runtime_breaks_baseline_gates.md) — HISTORICAL, not reproducing 2026-08-08; build before citing -- [New sln member surfaces MSB3277](project_new_sln_member_surfaces_msb3277_pin_divergence.md) — wiring an unbuilt legacy test project into the sln emits a new MSB3277 when packages.config pins diverge; fix in packages.config/.csproj, never a .cs file -- [Legacy csproj: no transitive compile refs](project_legacy_csproj_no_transitive_compile_refs.md) — non-SDK ProjectReference doesn't flow package types to csc (CS0012 despite copy-local DLL); tests need their own `` + packages.config entry +- [New sln member surfaces MSB3277](project_new_sln_member_surfaces_msb3277_pin_divergence.md) — wiring an unbuilt legacy test project into the sln emits a new MSB3277 when packages.config pins diverge; fix in +- [Legacy csproj: no transitive compile refs](project_legacy_csproj_no_transitive_compile_refs.md) — non-SDK ProjectReference doesn't flow package types to csc (CS0012 despite copy-local DLL); tests need their own - [sln/csproj edits: preserve CRLF](project_sln_csproj_edit_crlf_preserve.md) — git-bash `sed -i` strips CRLF from TaskMaster.sln (churn + BOM loss); use Edit or `perl -0777` w/ explicit `\r\n` -- [Incremental build makes a vacuous baseline](project_incremental_build_vacuous_baseline.md) — Invoke-VSBuild's /t:Build up-to-date check ignores /p: changes → EXIT 0 with 0 CoreCompile; add /t:Rebuild to enumerate diagnostics -- [Nullable /t:Build gate is vacuous](project_nullable_build_gate_is_vacuous_incremental.md) — the standard nullable gate passes without type-checking; isolated `/t:Rebuild ... /p:BuildProjectReferences=false` exposed 223 errors (never add /p:OutputPath — it breaks ProjectReference resolution) -- [CSharpier 1.3.0 formats XML at 100 cols](project_csharpier_formats_xml_print_width.md) — a "reformatting churn" finding on an XML resource can be formatter-mandated; measure line length + run repo-wide `check` before accepting it -- [Evidence collision clobbers committed artifacts](project_evidence_timestamp_collision_clobbers_artifacts.md) — same-day remediation can silently overwrite implementation-cycle evidence; a ` M` under `evidence/` means clobber +- [Incremental build makes a vacuous baseline](project_incremental_build_vacuous_baseline.md) — Invoke-VSBuild's /t:Build up-to-date check ignores /p: changes → EXIT 0 with 0 CoreCompile; add /t:Rebuild to enumerate +- [Nullable /t:Build gate is vacuous](project_nullable_build_gate_is_vacuous_incremental.md) — standard nullable gate passes without type-checking; isolated `/t:Rebuild` exposed 223 errors (no /p:OutputPath) +- [CSharpier 1.3.0 formats XML at 100 cols](project_csharpier_formats_xml_print_width.md) — a "reformatting churn" finding on an XML resource can be formatter-mandated; measure line length + run repo-wide +- [Evidence collision clobbers committed artifacts](project_evidence_timestamp_collision_clobbers_artifacts.md) — same-day remediation silently overwrites implementation evidence; a ` M` under evidence/ means clobber - [csharpier pipe-files is a non-enforcing gate](project_csharpier_pipefiles_nonenforcing_gate.md) — use `csharpier check`/`format`; tests balloon past 500 lines under genuine format (size new files AFTER format) +- [Count-idiom pitfalls: csharpier + Measure-Object](project_count_idiom_pitfalls_csharpier_and_measureobject.md) — "Formatted N files" is a PROCESSED count (hash before/after for rewrites); `Measure-Object -Line` omits blank lines - [PowerShell new files need UTF-8 BOM](powershell-bom-required.md) — PSScriptAnalyzer enforces PSUseBOMForUnicodeEncodedFile; prepend BOM after Write or restart the format loop -- [poshqc test MCP carries no verdict and no numbers](project_poshqc_pester_mcp_exit_minus1.md) — run_poshqc_test returns only {ok,tool,workspace_root,summary}: no exit code, counts or coverage, so any EXIT_CODE/N-failures/expect-fail gate on it is vacuous; scan_folders accepts FILE paths; Pester 5 has no branch metric; pair with direct Invoke-Pester -- [poshqc analyze exits 1 on a Warning](project_poshqc_analyze_exit1_on_warning.md) — "EXIT_CODE 0 with zero error-severity" is self-contradictory; Helpers.ps1 carries a pre-existing PSUseSingularNouns; `Remove-*` needs SupportsShouldProcess +- [poshqc test MCP carries no verdict and no numbers](project_poshqc_pester_mcp_exit_minus1.md) — returns only {ok,tool,workspace_root,summary} - no exit code/counts, so any EXIT_CODE or count gate on it is vacuous +- [poshqc analyze exits 1 on a Warning](project_poshqc_analyze_exit1_on_warning.md) — "EXIT_CODE 0 with zero error-severity" is self-contradictory; Helpers.ps1 carries a pre-existing PSUseSingularNouns - [BOM breaks grep ^ anchor](project_bom_grep_anchor_false_negative.md) — bash grep `^#nullable` misses BOM-prefixed files; use the Grep tool for opt-in classification, never bash grep -- [StrictMode Latest + missing XML attribute throws](project_pester_strictmode_xml_attribute_property_access.md) — a fixture omitting `branch` (or `complexity` on a merge-path ``) throws PropertyNotFoundStrict instead of the assertion diff; enumerate ALL bare `$node.attr` reads on the traversed path, not one attribute at a time -- [Pester 5 result shape: no container .Tests, no -CI + -CodeCoverage](project_pester5_result_shape_container_tests_and_ci_codecoverage.md) — `$_.Tests.Count` silently renders 0 per file (use `TotalCount`); `-CI` and `-CodeCoverage` are different parameter sets and cannot be combined -- [pwsh -Command needs single-quoted outer](project_pwsh_command_quoting_from_bash.md) — a double-quoted outer wrapper lets bash eat `$` → empty counts + inverted exit gates or a hard ParserError; `''` inside single quotes is not an escape +- [StrictMode Latest + missing XML attribute throws](project_pester_strictmode_xml_attribute_property_access.md) — a fixture missing `branch`/`complexity` throws PropertyNotFoundStrict; enumerate ALL bare `$node.attr` reads at once +- [Pester 5 result shape: no container .Tests, no -CI + -CodeCoverage](project_pester5_result_shape_container_tests_and_ci_codecoverage.md) — `$_.Tests.Count` renders 0 per file (use TotalCount); `-CI` and `-CodeCoverage` are different parameter sets +- [pwsh -Command needs single-quoted outer](project_pwsh_command_quoting_from_bash.md) — a double-quoted outer wrapper lets bash eat `$` → empty counts + inverted exit gates or a hard ParserError; `''` inside -- [Compile-time red needs body-level refs](project_compile_red_needs_body_level_references.md) — a missing type in a method SIGNATURE suppresses body binding, so an `[expect-fail]` task requiring N named CS0246s reports only 1; construct the types inline in test bodies +- [Compile-time red needs body-level refs](project_compile_red_needs_body_level_references.md) — a missing type in a method SIGNATURE suppresses body binding, so an `[expect-fail]` task requiring N named CS0246s ## Test execution & isolation - [Tests must mock GUI; no visible window](feedback_tests_must_mock_gui_no_visible_window.md) — use headless seams (mocked viewers, injected show/focus delegates), never Form.Show/Application.Run -- [#511 is a test-host crash, not N failing tests](project_511_is_a_testhost_crash_not_n_failing_tests.md) — load-driven abort with `Total tests: Unknown` (no readable verdict); `/InIsolation` loop gave 0 failed; never gate on a pinned failing-name count -- [WinFormsPumpHost tests are load-flaky](project_winformspumphost_tests_load_flaky.md) — QfcItemController_InitializationTests fail with "window handle has been created"/60s timeouts when the box is CPU-saturated; re-run when load drops, don't treat as a red baseline -- [vstest /InIsolation + FilePathHelper serialization](project_vstest_isolation_and_filepathhelper_serialization.md) — Moq assemblies need /InIsolation (else STTE Setup FileNotFound); FilePathHelper.FilePath is "" default but null after JSON deserialize -- [Invoke-MSTest.ps1 dies on a single test assembly](project_418_invoke_mstest_single_assembly_bug.md) — StrictMode + `.Count` on a scalar String throws before vstest runs; call vstest.console.exe directly with the script's arg list +- [#511 is a test-host crash, not N failing tests](project_511_is_a_testhost_crash_not_n_failing_tests.md) — load-driven abort with `Total tests: Unknown` (no readable verdict); `/InIsolation` loop gave 0 failed; never gate on a +- [WinFormsPumpHost tests are load-flaky](project_winformspumphost_tests_load_flaky.md) — QfcItemController_InitializationTests fail with "window handle has been created"/60s timeouts when the box is +- [vstest /InIsolation + FilePathHelper serialization](project_vstest_isolation_and_filepathhelper_serialization.md) — Moq assemblies need /InIsolation (else STTE Setup FileNotFound); FilePathHelper.FilePath is "" default but null after +- [Invoke-MSTest.ps1 dies on a single test assembly](project_418_invoke_mstest_single_assembly_bug.md) — StrictMode + `.Count` on a scalar String throws before vstest runs; call vstest.console.exe directly with the script's - [Timed-out MSTest leaves detached runner](project_timedout_mstest_leaves_detached_runner.md) — leaves a pwsh runner respawning testhosts → user.config hangs; kill the pwsh runner too, verify 0, rerun >=8min - [Sibling-worktree shared-tooling hazard](project_sibling_worktree_shared_tooling_hazard.md) — a concurrent agent elsewhere crashes your testhost + clobbers /tmp logs; use the session scratchpad -- [Concurrent executor in same worktree](project_concurrent_executor_same_worktree.md) — two executors on one worktree corrupt shared files; detect via mtime progression during your own turn; STOP, don't stash/race -- [UtilitiesCS.Test parallelism flakiness](project_utilitiescs_test_parallelism_flakiness.md) — timing tests time out (~22s) under default parallelism + coverage; lower MSTest Workers to 4 via /Settings for a deterministic gate +- [Concurrent dotnet-coverage deadlock + doc-comment retention gate](project_concurrent_dotnet_coverage_deadlock_and_doccomment_retention_gate.md) — sibling worktree's coverage run hangs yours (CPU-delta test, kill only your chain); an XML doc quoting a pinned literal +- [Concurrent executor in same worktree](project_concurrent_executor_same_worktree.md) — two executors on one worktree corrupt shared files; detect via mtime progression during your own turn; STOP, don't +- [UtilitiesCS.Test parallelism flakiness](project_utilitiescs_test_parallelism_flakiness.md) — timing tests time out (~22s) under default parallelism + coverage; lower MSTest Workers to 4 via /Settings for a - [MSTest [DoNotParallelize] overlaps the parallel bucket](project_mstest_donotparallelize_overlaps_parallel_bucket.md) — a [DoNotParallelize] null-baseline reader still sees parallel-bucket writers; mark every writer too -- [UiThread.Dispatcher static-swap race](project_uithread_dispatcher_static_swap_race.md) — two classes swapping the shared static deadlock on the parked dispatcher; serialize swap-to-restore with a SemaphoreSlim; symptom is a [Timeout] expiry only in the full-suite run -- [runsettings DataCollector default-enabled](project_runsettings_datacollector_default_enabled.md) — a declared Code Coverage `` activates under CLI vstest without /collect; enabled="false" then breaks /collect +- [UiThread.Dispatcher static-swap race](project_uithread_dispatcher_static_swap_race.md) — two classes swapping the shared static deadlock on the parked dispatcher; serialize swap-to-restore with a +- [runsettings DataCollector default-enabled](project_runsettings_datacollector_default_enabled.md) — a declared Code Coverage `` activates under CLI vstest without /collect; enabled="false" then breaks - [dotnet-coverage Deedle/FSharp instrumentation breaks tests](project_dotnet_coverage_deedle_fsharp_instrumentation.md) — pass a module-exclude settings XML to dotnet-coverage (runsettings excludes don't propagate); pair with Workers=4 -- [DispatcherDelay hangs unit tests](project_dispatcherdelay_hangs_unit_tests.md) — WaitAsync never completes in the pump-less MSTest host and hangs the whole assembly; drive coverage via dotnet-coverage collect wrapping vstest -- [ConfigController STA pump deadlock](project_configcontroller_sta_pump_deadlock.md) — SaveAsync posts its continuation to the WinForms STA queue; an STA test must pump (DoEvents + Thread.Yield), not block on GetAwaiter().GetResult() +- [DispatcherDelay hangs unit tests](project_dispatcherdelay_hangs_unit_tests.md) — WaitAsync never completes in the pump-less MSTest host and hangs the whole assembly; drive coverage via dotnet-coverage +- [ConfigController STA pump deadlock](project_configcontroller_sta_pump_deadlock.md) — SaveAsync posts its continuation to the WinForms STA queue; an STA test must pump (DoEvents + Thread.Yield), not block ## Coverage measurement -- [Coverage delta: reproduce the baseline's counting method](project_coverage_delta_reproduce_baseline_counting_method.md) — Cobertura repeats lines under `` AND class ``; deduped vs all-descendant give ~2x denominators → false escalation -- [First-party coverage denominator method (#197)](project_coverage_firstparty_denominator_method.md) — production-only rate = per-`` count across ALL deduped packages INCLUDING vendored Swordfish/SVGControl; reproduces 71.73% +- [Coverage delta: reproduce the baseline's counting method](project_coverage_delta_reproduce_baseline_counting_method.md) — Cobertura repeats lines under `` AND class ``; deduped vs all-descendant give ~2x denominators +- [First-party coverage denominator method (#197)](project_coverage_firstparty_denominator_method.md) — production-only rate = per-`` count across ALL deduped packages INCLUDING vendored Swordfish/SVGControl - [dotnet-coverage denominator nondeterminism](project_dotnet_coverage_denominator_nondeterminism.md) — repo line-rate swings (47% vs 81%) from double-counted denominator; re-baseline via git-stash, trust per-class rates -- [Koverage Cobertura post-processing shape](project_koverage_cobertura_postprocessing_shape.md) — Invoke-MSTestWithCoverage rewrites filenames with `\`, pre-merges per-file `` nodes, strips test packages, recomputes root attrs; forward-slash queries match nothing +- [Koverage Cobertura post-processing shape](project_koverage_cobertura_postprocessing_shape.md) — rewrites filenames with `\`, pre-merges per-file ``, strips test packages; forward-slash queries match nothing - [C# canonical coverage artifact conversion](project_csharp_canonical_coverage_artifact_conversion.md) — hook reads artifacts/csharp/coverage.xml as JaCoCo (85% floor); defer repo-wide to PR CI -- [Cobertura runsettings `` override](project_cobertura_runsettings_attributes_override.md) — a custom `` block replaces the default `` excludes, silently disabling [ExcludeFromCodeCoverage]; re-add it -- [Changed-line coverage: Cobertura hits vs MS-coverage partial](project_changed_line_coverage_cobertura_vs_mscoverage_partial.md) — null-guard throws read "partially covered" in MS.CodeCoverage XML but hits=1 in Cobertura; use Cobertura for >=90% proofs -- [QFC #227 coverage tooling](project_qfc227_coverage_tooling.md) — vstest + Cobertura runsettings (Format under Configuration + attribute-exclude) is the reliable per-class path; .coverage not offline-convertible here -- [#398 test-split gate gotchas](project_398_test_split_gate_gotchas.md) — pre-existing CS2002 duplicate Compile (latent, out of scope); /EnableCodeCoverage has no branch% + .coverage merges to empty cobertura → use Cobertura-runsettings variant +- [Cobertura runsettings `` override](project_cobertura_runsettings_attributes_override.md) — a custom `` block replaces the default `` excludes, silently disabling +- [Changed-line coverage: Cobertura hits vs MS-coverage partial](project_changed_line_coverage_cobertura_vs_mscoverage_partial.md) — null-guard throws read "partially covered" in MS.CodeCoverage XML but hits=1 in Cobertura; use Cobertura for >=90% +- [QFC #227 coverage tooling](project_qfc227_coverage_tooling.md) — vstest + Cobertura runsettings (Format under Configuration + attribute-exclude) is the reliable per-class path +- [#398 test-split gate gotchas](project_398_test_split_gate_gotchas.md) — pre-existing CS2002 duplicate Compile (latent, out of scope); /EnableCodeCoverage has no branch% + .coverage merges to - [ExcludeFromCodeCoverage on partial class = CS0579](project_excludefromcodecoverage_partial_class_cs0579.md) — annotate a partial type ONCE, not both parts, or the build breaks with duplicate-attribute CS0579 - Closed one-offs (low reuse): [#400 dead recovery catch](project_400_completeopenasync_unreachable_recovery_catch.md), [Swordfish vendored-coverage side effect](project_swordfish_removal_epic_incidental_coverage_sideeffect.md), [#298 ScoCollection/live-bridge exemptions](project_taskvis_scocollection_and_livebridge_exemptions.md), [#328 OlObjectsProxy](project_328_rebuild_threading_olobjectsproxy_conflict.md) ## Nullable / C# language -- [Nullable per-file pragma gate mechanics](project_nullable_pragma_gate_mechanics.md) — CI's command (`/t:Rebuild`, TWAE, NO `/p:Nullable=enable`) genuinely passes EXIT 0; the 195+219 error population appears only under the forced flag. Use `/t:Rebuild` so the pass is not vacuous -- [CLAUDE.md nullable command != the CI gate](project_507_nullconditional_return_triggers_cs8603_under_genuine_nullable_check.md) — ci.yml omits `/p:Nullable=enable` and relies on per-file `#nullable` pragmas; forced-flag CS86xx in an unannotated file is NOT a blocker. Repro the CI command before failing an AC -- [Nullable remediation annotation patterns](project_nullable_remediation_annotation_patterns.md) — net481 has no post-condition attrs; struct `= default!`; `.ToString()!` for string cells; IsNullOrEmpty overload gotcha; `x!.M()` for defensive flow-state +- [Nullable per-file pragma gate mechanics](project_nullable_pragma_gate_mechanics.md) — CI's command (`/t:Rebuild`, TWAE, NO `/p:Nullable=enable`) genuinely passes EXIT 0; the 195+219 error population +- [CLAUDE.md nullable command != the CI gate](project_507_nullconditional_return_triggers_cs8603_under_genuine_nullable_check.md) — ci.yml omits `/p:Nullable=enable`; forced-flag CS86xx in an unannotated file is NOT a blocker - repro the CI command +- [Nullable remediation annotation patterns](project_nullable_remediation_annotation_patterns.md) — net481 has no post-condition attrs; struct `= default!`; `.ToString()!` for string cells; IsNullOrEmpty overload - [Nullable annotation CS8632 scoping](project_nullable_annotation_cs8632_scoping.md) — `Type?` in nullable-disabled projects emits CS8632; wrap in `#nullable enable annotations`, not whole-file - [init/record struct fails CS0518 on net48](project_record_struct_isexternalinit_netfx.md) — no IsExternalInit polyfill; use a ctor-initialized readonly struct with get-only props - [CS8714 does not fire on net481](project_nullable_cs8714_not_on_net481.md) — net481 BCL lacks notnull, so `where TKey : notnull` is forward-looking only - Nullable-epic per-issue notes (closed epic, low reuse): [#366 notnull cascade](project_366_notnull_cascades_beyond_wrapperscodictionary.md), [#366 4th file](project_366_scdictionary_constraint_cascades_to_fourth_file.md), [#366 CS8766](project_366_batch7_tnullable_return_cs8766.md), [#372](project_372_email_classifier_nullable_patterns.md), [#371](project_371_outlookobjects_nullable_lessons.md), [#375](project_375_residuals_nullable_gotchas.md) -- [Outlook `Action`/`Exception` ambiguity](project_outlook_action_ambiguity.md) — bare `Action` AND bare `Exception` are CS0104-ambiguous in Outlook-interop files; use `System.Action`/`System.Exception` (surfaces only at analyzer/type-check build) +- [Outlook `Action`/`Exception` ambiguity](project_outlook_action_ambiguity.md) — bare `Action` AND bare `Exception` are CS0104-ambiguous in Outlook-interop files; use ## Component-specific gotchas - [#349 breadcrumb WebView2 gotchas](project_349_breadcrumb_webview2_gotchas.md) — retyped Designer field breaks reflection-injected tests; aggregate async d__ classes for >=90% @@ -90,8 +93,8 @@ - [QfcDatamodel BackgroundWorker async-void IsBusy race](project_qfc_backgroundworker_async_void_race.md) — IsBusy flips false instantly; assert WorkerSupportsCancellation - [QfcItemController pump harness needs SaveParameters](project_qfcitemcontroller_pump_harness_needs_saveparameters.md) — SetField-only injection leaves `??=` factory defaults null (NRE in LoadFolderHandlerAsync) - [TaskController (#297) unit-test gotchas](project_taskvisualization_taskcontroller_test_gotchas.md) — ApplyChanges hangs over Moq; get-only MailItem.TaskSubject throws; STA harness needs TableLayoutPanel parenting -- [ProjectEntry setter raw MessageBox](project_projectentry_setter_raw_messagebox.md) — the ProjectID setter uses RAW un-seamed MessageBox.Show and hangs STA tests; CompareTo tie-break needs a Moq IProjectEntry with shifting ProjectID +- [ProjectEntry setter raw MessageBox](project_projectentry_setter_raw_messagebox.md) — the ProjectID setter uses RAW un-seamed MessageBox.Show and hangs STA tests; CompareTo tie-break needs a Moq - [IApplicationGlobals member forces implementers](project_iapplicationglobals_member_forces_implementers.md) — adding a member breaks 7 hand-written test-double stubs beyond scope lock; Moq mocks auto-implement -- [TimeProvider seam gotchas](project_timeprovider_seam_gotchas.md) — Moq can't mock non-virtual GetLocalNow (use FakeTimeProvider); an optional TimeProvider param forces a Bcl.TimeProvider `` on every consumer (CS0012) +- [TimeProvider seam gotchas](project_timeprovider_seam_gotchas.md) — Moq can't mock non-virtual GetLocalNow (use FakeTimeProvider); an optional TimeProvider param forces a Bcl.TimeProvider - [ScoDictionaryNew needs TryAdd not Add](project_scodictionarynew_tryadd_not_add.md) — retargeting Sco* tests: `.Add(k,v)` won't compile (CS1061); the base exposes `.TryAdd`; swap in the same edit -- [FluentAssertions Equal(params) has no because](project_fluentassertions_equal_params_no_because.md) — a trailing reason on `.Equal(...)` becomes an extra expected element; use `.Equal(new[]{...})` or move the reason to `.HaveCount(n, reason)` +- [FluentAssertions Equal(params) has no because](project_fluentassertions_equal_params_no_because.md) — a trailing reason on `.Equal(...)` becomes an extra expected element; use `.Equal(new[]{...})` or move the reason to diff --git a/.claude/agent-memory/atomic-executor/project_concurrent_dotnet_coverage_deadlock_and_doccomment_retention_gate.md b/.claude/agent-memory/atomic-executor/project_concurrent_dotnet_coverage_deadlock_and_doccomment_retention_gate.md new file mode 100644 index 000000000..5391b5a51 --- /dev/null +++ b/.claude/agent-memory/atomic-executor/project_concurrent_dotnet_coverage_deadlock_and_doccomment_retention_gate.md @@ -0,0 +1,35 @@ +--- +name: concurrent-dotnet-coverage-deadlock-and-doccomment-retention-gate +description: Two #445 execution hazards - a sibling worktree's dotnet-coverage collect deadlocks yours (diagnose by CPU delta, kill only your own chain), and an XML doc comment quoting a code literal breaks a retention count gate +metadata: + type: project +--- + +**1. A concurrent `dotnet-coverage collect` from a sibling agent worktree deadlocks yours.** + +Symptom: `dotnet-coverage collect ... -- vstest.console.exe @assemblies` produces no output file and never returns. The `testhost.exe` accrued 0.02 s of CPU across a 60-second sample (32.046875 to 32.0625) — a hang, not slow progress. + +Diagnose, do not guess: +```powershell +# progress test: sample CPU twice, 60 s apart. Delta ~0 means deadlocked. +# ownership test: read the command line, which carries the absolute worktree path. +Get-CimInstance Win32_Process | Where-Object { $_.Name -eq 'testhost.exe' -or $_.Name -eq 'vstest.console.exe' } | + ForEach-Object { "PID=$($_.ProcessId) PPID=$($_.ParentProcessId)"; $_.CommandLine } +``` +`Get-CimInstance Win32_Process -Filter "Name='testhost.exe'"` throws `Invalid query` from bash; use `Where-Object` instead. + +On #445 the contending workload was a full nine-assembly instrumented run from `agent-a28821f6e56934fc7` (issue #491). Two simultaneous full-solution instrumentation sessions on one machine is the cause. + +Remedy: kill **only your own** chain — `dotnet-coverage` -> `vstest.console` -> `testhost` plus the parent `pwsh` runner (walk `ParentProcessId`; killing the runner matters, see [[project_timedout_mstest_leaves_detached_runner]]). Leave the sibling's processes running. Wait for the machine to clear, then re-run the **unaltered** command. It then completed in normal time with 6441/6441 passing. + +**Never** respond by adding a sleep/retry/timeout to a test or by changing the command. The hang is machine contention, not a regression, and the plan forbids stabilising a test with timing hacks. + +**2. An XML doc comment that quotes a code literal breaks a retention count gate.** + +A plan can pin an out-of-scope expression with a retention gate, e.g. `Key.Substring(other.Length - 1, 1)` must stay at count 1 to prove a deferred defect was not "helpfully" fixed. Writing an XML doc comment that quotes that expression as `Key.Substring(other.Length - 1, 1)` raises the count to **2** and fails the gate — even though no code changed. + +Fix by describing the expression instead of reproducing it ("branch 1's substring offset expression"). Re-run the count to confirm it returns to baseline. + +**Why:** `git grep -F` is text-based and cannot distinguish a code occurrence from a comment occurrence. The gate is doing its job; the doc comment is the defect. + +**How to apply:** After writing any comment or doc block on a file that carries retention gates, re-run every count gate for that file before checking the task off. Prefer prose descriptions over verbatim literals in comments on gated files. Related: [[project_multipattern_gate_shared_qualifier_detachment]], [[project_sibling_worktree_shared_tooling_hazard]]. diff --git a/.claude/agent-memory/atomic-executor/project_count_idiom_pitfalls_csharpier_and_measureobject.md b/.claude/agent-memory/atomic-executor/project_count_idiom_pitfalls_csharpier_and_measureobject.md new file mode 100644 index 000000000..73d1e70d9 --- /dev/null +++ b/.claude/agent-memory/atomic-executor/project_count_idiom_pitfalls_csharpier_and_measureobject.md @@ -0,0 +1,28 @@ +--- +name: count-idiom-pitfalls-csharpier-and-measureobject +description: Two measurement idioms that silently report the wrong number in plan gates - CSharpier's "Formatted N files" is a processed count not a rewrite count, and Measure-Object -Line omits blank lines +metadata: + type: project +--- + +Two counting idioms routinely used in atomic-plan acceptance gates report a different quantity than the gate assumes. Both were hit on #445 and both would have produced a wrong verdict if taken at face value. + +**1. `csharpier format` prints a PROCESSED count, not a REWRITE count.** +`dotnet tool run csharpier format <5 files>` prints `Formatted 5 files in 2054ms.` even when it rewrote **zero** of them. A plan task that says "record how many of the five files the formatter rewrote" and that triggers a phase restart on a non-zero count will restart forever if it reads that line as the answer. +Measure the rewrite count directly: +```powershell +$before = @{}; foreach ($f in $files) { $before[$f] = (Get-FileHash -LiteralPath $f -Algorithm SHA256).Hash } +& $dotnet tool run csharpier format @files +foreach ($f in $files) { if ((Get-FileHash -LiteralPath $f -Algorithm SHA256).Hash -ne $before[$f]) { $rewritten++ } } +``` +`csharpier check .` remains the right read-only verdict (`Checked 1517 files in ...` plus exit 0 means zero need formatting; non-conforming files are printed one per line before the summary). + +**2. `Measure-Object -Line` does NOT count blank lines.** +`(Get-Content -LiteralPath $f | Measure-Object -Line).Lines` returned 86 / 84 / 84 / 17 / 143 for files whose true physical line counts are 95 / 99 / 99 / 18 / 168. Every figure is understated, by exactly the blank-line count. A file-size audit against a 500-line cap using this idiom under-reports and can pass a file that actually violates the cap. +Use `(Get-Content -LiteralPath $f).Count`, and cross-check with `wc -l`. The two agreed exactly on all five files. + +Critically: baseline and final MUST use the same idiom, or the before/after comparison is incommensurable. The Uniform Count Idiom for `git grep` (`(git grep -n -F 'TOKEN' -- 'PATHSPEC' | Measure-Object -Line).Lines`) is fine because `git grep` never emits a blank line. + +**Why:** Both idioms look authoritative and produce a plausible number, so neither failure announces itself. The CSharpier one caused a false restart signal; the Measure-Object one silently understated every file-size baseline. + +**How to apply:** When a plan gate asks "how many files did the formatter change", hash before and after. When a plan gate asks for a file line count, use `(Get-Content).Count` and record the counting method in the baseline artifact so the final-QC task reproduces it. Related: [[project_csharpier_pipefiles_nonenforcing_gate]], [[feedback_verify_line_citations_with_numbered_output]]. diff --git a/.claude/agent-memory/atomic-executor/project_preflight_absolute_zero_gate_on_sibling_owned_assembly.md b/.claude/agent-memory/atomic-executor/project_preflight_absolute_zero_gate_on_sibling_owned_assembly.md new file mode 100644 index 000000000..ecdf27117 --- /dev/null +++ b/.claude/agent-memory/atomic-executor/project_preflight_absolute_zero_gate_on_sibling_owned_assembly.md @@ -0,0 +1,59 @@ +--- +name: preflight-absolute-zero-gate-on-sibling-owned-assembly +description: In an epic, a child plan that demands "Failed 0" on a whole test assembly can be unsatisfiable when a concurrent sibling child owns known intermittent failures in that same assembly; also, preparation-mode plans that pin an absolute WS path. +metadata: + type: project +--- + +Two preflight defect classes found on the #445 keyboard-action-contract plan +(`docs/features/active/2026-08-07-quickfiler-keyboard-action-contract-defects-445/plan..md`), +both invisible unless you cross-read the epic manifest. + +**1. Absolute-zero test gate on an assembly a sibling child owns.** +The plan required `QuickFiler.Test` `Failed 0` unconditionally in two tasks, while applying a +baseline-subset tolerance repo-wide. The epic manifest +(`docs/features/epics/quickfiler-suite-determinism-foundation/epic.md`) documents two +*intermittently failing* tests in that same assembly +(`QuickFiler.Test/Controllers/QfcItemController.InitializationTests.Part3.cs`) owned by a different +wave-0 child (#511/#571) that is NOT guaranteed to have merged first. + +**Why:** the gate can fail for a reason the executing child cannot fix, and blocking is forbidden +after `[P0-T1]`, so the executor is deadlocked or pressured to weaken a test. + +**How to apply:** when preflighting an epic child, list every assembly the child asserts an absolute +pass/fail count on, then grep the epic manifest for known-failing tests in those assemblies. Where a +sibling owns a failure, the delta is to extend the plan's own baseline-subset rule to that assembly +while keeping absolute zero on the test classes the child actually owns. See +[[project-511-is-a-testhost-crash-not-n-failing-tests]] and +[[winformspumphost-tests-load-flaky]] for why that assembly is unreliable. + +**2. Preparation-mode plan pinning an absolute workspace root.** +The plan's `## Resolved Environment` pinned `WS` to the *planning* worktree and labelled the block +"verified; use these, do not re-derive". That worktree was already deleted, and preparation-mode +plans execute later in a third, not-yet-created worktree, so no absolute literal is ever correct. + +**Why:** the "do not re-derive" instruction forbids the one micro-action that would repair it, which +is what turns a stale path from self-healing into blocking. + +**How to apply:** on any preparation-mode preflight, diff the plan's pinned workspace root against +`git worktree list` first. The delta is to replace the literal with +`git rev-parse --show-toplevel` resolved at execution time. Repo-local tool paths that live in the +PRIMARY clone (for example `/.dotnet-sdk/dotnet.exe`) are NOT the same defect: they resolve +from any worktree and stay valid. + +**3. Iteration-2 resolution shape (verified 2026-08-21).** Both defects cleared on re-preflight. Two +things were worth proving rather than assuming: + +- A "failing set is a SUBSET of the baseline failing set" condition is by itself sufficient to catch + a genuine new regression anywhere in the assembly, including outside the classes the child owns: + a new failure is absent from the baseline set, so the subset test fails. Adding "and no failure in + " only converts the owned classes to absolute zero; it does not rescue the rest. +- Unpinning `EXIT_CODE:` on a vstest task that tolerates a pre-existing failure is coherent and NOT + a weakening, provided the downstream AC check-off still conjoins "all stage exit codes are 0". + vstest exits non-zero on any failure, so that conjunct silently re-imposes Failed 0 for the + check-off while the escape clause records the gap. + +**Namespace gotcha:** `QuickFiler.Test`'s test classes live in namespace `QuickFiler.Controllers.Tests`, +so a fully-qualified test name from that assembly does NOT contain the string `QuickFiler.Test`. Any +gate phrased as "the `QuickFiler.Test` portion of the baseline failing set" cannot be evaluated by +FQN string match; partition by run scope instead. diff --git a/.claude/agent-memory/atomic-planner/MEMORY.md b/.claude/agent-memory/atomic-planner/MEMORY.md index 1776c8bc3..08efeb8f8 100644 --- a/.claude/agent-memory/atomic-planner/MEMORY.md +++ b/.claude/agent-memory/atomic-planner/MEMORY.md @@ -1,7 +1,12 @@ # Atomic Planner Memory Index +- [Agent worktrees need SDK + NuGet + analyzer-backfill bootstrap](agent-worktrees-need-sdk-and-nuget-bootstrap.md) — no `.dotnet-sdk`, no `packages/`, and a clean restore still misses the skewed analyzer versions (CS0006, not a warning); three Phase 0 tasks +- [/Logger:trx needs /ResultsDirectory](trx-needs-resultsdirectory.md) — TRX lands in `TestResults\` relative to cwd; TRX-existence-under-evidence acceptance is unsatisfiable without it, and the clean-tree gate won't catch it +- [Per-task TRX subdirectory](trx-needs-resultsdirectory.md) — a shared `/ResultsDirectory:` makes "ten distinct TRX files" ambiguous once `[expect-fail]` runs deposit earlier TRX there; give each task a `p#-t#` segment +- [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" - [#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 +- [#445 keyboard-action plan seams](project_445_keyboard_action_plan_seams.md) — prepared-in-one-worktree plans must resolve WS at execution time; scope epic-child pass/fail gates to owned test classes (sibling-owned flaky tests share the assembly); retention gates need their own justification - [Dead-code removal vs coverage exclusion](project_deadcode_removal_vs_coverage_exclusion.md) — coverage gate blocked by unreachable dead prod code → plan removal (shrink denominator), never exclusion/carve-out/forced-rethrow - [Coverage gate on CLR-invoked private members](coverage-gate-clr-invoked-private-members.md) — never gate AssemblyResolve-style private members at >=90%; split newly-added vs changed per the AC's own wording - [Nullable context mismatch: prod vs test](project_nullable_context_mismatch_prod_vs_test.md) — check `#nullable enable` in the prod file AND missing `` (C# 7.3) in the test csproj; adding `latest` is never "one property" 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 new file mode 100644 index 000000000..59a5211c5 --- /dev/null +++ b/.claude/agent-memory/atomic-planner/agent-worktrees-need-sdk-and-nuget-bootstrap.md @@ -0,0 +1,49 @@ +--- +name: agent-worktrees-need-sdk-and-nuget-bootstrap +description: A fresh agent worktree has neither .dotnet-sdk nor packages/, and a clean nuget restore still leaves the analyzer version skew unresolved, so a C# plan needs THREE explicit Phase 0 bootstrap tasks +metadata: + type: project +--- + +A C# plan executing inside a `.claude/worktrees/` worktree needs THREE Phase 0 bootstrap tasks +before the first `dotnet tool restore` and before the first `msbuild`, in this order: + +1. **Provision `.dotnet-sdk`.** `global.json` pins `sdk.version 8.0.205` with `rollForward: latestFeature` + and `paths: [".dotnet-sdk", "$host$"]`. A fresh worktree has no `.dotnet-sdk`, and a host that only + carries a 10.x SDK cannot satisfy `8.0.205` under `latestFeature`, so `dotnet --version` from the + worktree root prints the `global.json` `errorMessage` instead of a version. Remedy: + `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 + `` 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 + `.github/workflows/_build-analyzers.yml:45` runs `nuget restore` explicitly. +3. **Back-fill `Meziantou.Analyzer 3.0.156` and `Roslynator.Analyzers 4.16.0`.** Step 2 alone is NOT + enough. All 16 first-party `.csproj` files carry UNCONDITIONAL `` items naming + `..\packages\Meziantou.Analyzer.3.0.156\...` and four `..\packages\Roslynator.Analyzers.4.16.0\...` + DLLs (`QuickFiler.Test.csproj:474-478`), while all 16 `packages.config` pin `3.0.174` and `4.16.1`. + Dependabot commit `f8e22af7` bumped only the NuGet-generated `Condition`-guarded `Import`/`Error` + lines and `packages.config`; the hand-authored Issue #181 `Analyzer` items were never realigned. + A missing `Analyzer` path is `error CS0006`, NOT a warning — the compile FAILS. Remedy: + `nuget install -Version -OutputDirectory packages`, or copy the folders from the main + checkout. Both versions exist there and are verifiable with a glob before you write the claim. + +**Do not conclude from green CI that the compile tolerates the skew.** +`_build-analyzers.yml:38` caches `path: packages` with a PREFIX `restore-keys` fallback (lines 40-41). +The bump guarantees an exact-key miss (the key hashes `**/packages.config`), so the fallback restores a +pre-bump tree still holding the old versions and line 45 only adds the new ones beside them. The main +checkout shows the same accumulation (Meziantou `.101/.123/.156/.174`). Green CI is lingering folders. + +None of the three dirties the tree: `.gitignore:350` is `.dotnet*/` and `.gitignore:191` is `**/[Pp]ackages/*`. +Note `.gitignore:191` is the packages pattern — NOT line 349, which is blank. Verify a `.gitignore` +line citation before writing it into an acceptance clause. + +**Why:** Preflight on the #511 plan returned REVISIONS REQUIRED twice: iteration 1 for the missing SDK +and `nuget restore` steps, iteration 2 for the missing analyzer back-fill. In both cases every +`EXIT_CODE: 0` acceptance on a dotnet/msbuild task was unreachable by environment. + +**How to apply:** Add all three as their own Phase 0 tasks with their own binary acceptance — never fold +them into the head of an existing task, which puts two independent outcomes under one task ID. See +[[project_csharp_phase0_toolchain_bootstrap]] and [[one-ac-per-checkoff-task]]. diff --git a/.claude/agent-memory/atomic-planner/project_445_keyboard_action_plan_seams.md b/.claude/agent-memory/atomic-planner/project_445_keyboard_action_plan_seams.md new file mode 100644 index 000000000..c56f630a2 --- /dev/null +++ b/.claude/agent-memory/atomic-planner/project_445_keyboard_action_plan_seams.md @@ -0,0 +1,49 @@ +--- +name: project-445-keyboard-action-plan-seams +description: "#445 epic-child plan revision: prepared-in-one-worktree/executed-in-another forbids absolute WS literals; assembly-wide `Failed 0` gates collide with sibling-owned intermittent tests; retention gates are not vacuous" +metadata: + type: project +--- + +Three seams surfaced when `atomic-executor` preflight rejected the #445 plan (epic +`quickfiler-suite-determinism-foundation`, wave-0 child). + +**1. A prepared plan must not pin an absolute workspace root.** Preparation-mode epic children are +planned in one agent worktree and executed later by `epic-orchestrator` in a worktree that does not +exist yet. Any `C:\...\.claude\worktrees\agent-` literal in a "Resolved Environment (verified; +use these, do not re-derive)" block is wrong on arrival — the planning worktree is gone by then. +Write `WS` as "resolve at execution time via `git rev-parse --show-toplevel`" and keep only the +genuinely worktree-independent literals (repo-local `.dotnet-sdk\dotnet.exe` in the PRIMARY clone, +VS-installed msbuild/vstest paths) under the do-not-re-derive header. + +**Why:** the "do not re-derive" instruction forbids the one action that repairs a dead path. +**How to apply:** in any epic-child or preparation-mode plan, audit every absolute path for +worktree-dependence before writing the environment block. See [[worktree-root-breaks-dotclaude-exclusion]] +for the related trap that `WS` itself sits under `.claude\worktrees\`. + +**2. An epic child must not gate on an assembly-wide `Failed 0`.** `QuickFiler.Test` contains two +intermittently failing pump tests owned by sibling child #511/#571 and a `Form1.cs` defect owned by +#491, all in the same wave with an empty dependency graph, so the sibling fixes are not guaranteed +merged when this child runs. The coverage runsettings (`scripts/vscode/TaskMaster.cli.runsettings`) +sets `0` with `ClassLevel` — full CPU parallelism, the exact load +condition those tests fail under. Scope the gate to the test CLASSES this child owns +(subset-of-baseline PLUS "no failing test in "), and drop the `EXIT_CODE: 0` clause +on the run, since vstest exits non-zero on any failure. + +**Why:** the executor's only in-task recourses for an unfixable sibling failure are to weaken a +sibling's test or record a false pass, both prohibited. +**How to apply:** when planning any child of a multi-child epic, list which test classes in the +shared assembly the child owns and phrase every pass/fail gate against that set only. Thread the +same class-scoped condition through the AC check-off task's escape clause, or the discharge is +unreachable — see [[thread-granted-discharges-through-consumers]]. + +**3. Retention gates need their own justification sentence.** A literal register whose preamble +claims "every count moves" is false as soon as it holds any before==after entry. Say plainly that +the register mixes CHANGE gates (count must move) and RETENTION gates (count must hold on a file +the plan edits), and state why each retention gate can still fail. Related: +[[acceptance-edits-must-be-false-before-true-after]]. + +**4. Assertion form can collide with a retention gate.** A per-element FluentAssertions +`list[0].Should().Be("b")` would push the pinned `Be("b"` occurrence count from 1 to 2 and break the +plan's own AC19 gate. When a plan pins an occurrence count in a test file it also instructs the +executor to edit, name the assertion form explicitly (`.Should().Equal(new[] { ... })`). diff --git a/.claude/agent-memory/atomic-planner/terminal-phase-planner-traps.md b/.claude/agent-memory/atomic-planner/terminal-phase-planner-traps.md new file mode 100644 index 000000000..b35901ea0 --- /dev/null +++ b/.claude/agent-memory/atomic-planner/terminal-phase-planner-traps.md @@ -0,0 +1,43 @@ +--- +name: terminal-phase-planner-traps +description: Three preflight findings that recur in the final phase of a plan — an unowned follow-up obligation, artifacts written after the clean-tree commit task, and a "plan-level clarification" that the spec already says +metadata: + type: feedback +--- + +Three checks to run over the final phase of every plan before handing it to preflight. + +**1. Every "a follow-up issue should carry it" sentence needs a task that files it.** +A latent-defect note in the preamble (analyzer skew, out-of-scope defect, re-attributed +half of an issue) states an obligation. If no task files the issue, the obligation is +never discharged and the defect is re-discovered instead of fixed. Add a `gh issue create` +task plus an `evidence/issue-updates/` mirror, and edit the preamble note so it names the +task ID that discharges it. `gh` is authenticated in this environment. When the epic +forbids children from writing under `docs/features/potential/**`, `gh issue create` is the +right instrument and `potential_to_issue` is wrong (it files a duplicate). + +**Why:** #511 round 3 — the analyzer-note item 6 said a follow-up issue "should carry" +the 16-project `Analyzer`/`packages.config` realignment, and nothing filed it. + +**2. Any task that writes an artifact AFTER the commit task needs a second commit task.** +The commit task's acceptance is normally "`git status --porcelain` produces zero output +lines". A review-handoff index or issue mirror written after it leaves the plan's terminal +state as a worktree with untracked evidence. The handoff task usually cannot move ahead of +the commit, because its acceptance cites the head sha that commit produced. Append a final +commit task with explicit pathspecs and re-assert both the clean tree and the scope lock. +Markdown under the feature folder does not enter a `.cs`/`.csproj`-filtered scope-lock set. + +**Why:** repo standing practice is that all audit-trail evidence is committed and the work +is not done until `git status` is clean. Related: [[diff-gates-need-a-commit-task]]. + +**3. Never write "plan-level clarification recorded against the spec's wording" without +reading the spec sentence.** A false deviation claim invites a feature reviewer to score a +spec deviation that was never made. Read the AC line, and if the carve-out or condition is +already there verbatim, say "quoted from spec AC N" instead. + +**Why:** #511 P6-T11 claimed it was adding the `.claude/agent-memory/` carve-out; spec.md +already carried it. Related: [[agent-memory-is-tracked-scope-git-gates]]. + +**How to apply:** run all three as a sweep over the last phase after the plan is otherwise +final; all three are appends or a one-sentence in-place replacement, so they never force a +renumber. diff --git a/.claude/agent-memory/atomic-planner/trx-needs-resultsdirectory.md b/.claude/agent-memory/atomic-planner/trx-needs-resultsdirectory.md new file mode 100644 index 000000000..7cfce7ffd --- /dev/null +++ b/.claude/agent-memory/atomic-planner/trx-needs-resultsdirectory.md @@ -0,0 +1,33 @@ +--- +name: trx-needs-resultsdirectory +description: /Logger:trx writes to TestResults\ relative to cwd, so a TRX-existence acceptance clause needs an explicit /ResultsDirectory: — and that directory must be private to the task, not shared across tasks +metadata: + type: feedback +--- + +Every `vstest.console.exe` task that passes `/Logger:trx` must also pass +`/ResultsDirectory:`. + +**Why:** `vstest.console.exe` writes the TRX into a `TestResults` folder relative to the working +directory unless `/ResultsDirectory:` is supplied. A task that says "write the ten TRX files to +`/evidence/regression-testing/`" and makes "ten distinct TRX files exist" its acceptance +condition is then asserting against a file the stated command never produces at that location. The +#511 plan had ten such tasks and zero `ResultsDirectory` occurrences; preflight caught it. + +**A shared results directory is a second, independent defect.** Supplying `/ResultsDirectory:` but +pointing every task at the same evidence `` folder makes a count-based acceptance ambiguous. In +the #511 plan eight tasks wrote into `evidence/regression-testing/`, so by the time `P4-T2` asserted +"ten distinct TRX files exist and each records a failed count of exactly 0", that folder already held +roughly 25 earlier TRX files — several of them legitimate FAILURES from `[expect-fail]` Phase 1 runs. +A later auditor cannot tell which ten the gate meant. Append a lower-case task-ID segment to each +value (`.../evidence/regression-testing/p4-t2`) and word the acceptance as "that subdirectory holds +exactly ten TRX files and no others". This also disambiguates two sibling ten-run tasks from each +other. + +**How to apply:** When auditing a plan, grep `Logger:trx` and `ResultsDirectory` and confirm the two +line-number sets are identical AND that every `ResultsDirectory` VALUE is distinct. Match the parent +directory to each task's own evidence `` (`baseline`, `regression-testing`, `qa-gates`); the +task-ID segment goes underneath it, so the canonical `/evidence//` invariant still +holds. Renumbering a task means renaming its segment — re-check after any Delta that shifts IDs. A +stray `TestResults\` from an omission does not break a clean-tree acceptance — `.gitignore:39` is +`[Tt]est[Rr]esult*/` — so the clean-tree gate will NOT catch the missing-flag defect for you. diff --git a/.claude/agent-memory/feature-review/MEMORY.md b/.claude/agent-memory/feature-review/MEMORY.md index 44d9d9b84..794b86811 100644 --- a/.claude/agent-memory/feature-review/MEMORY.md +++ b/.claude/agent-memory/feature-review/MEMORY.md @@ -1,3 +1,4 @@ +- [verify-zero-own-effect-coverage-noise-491](project_verify-zero-own-effect-coverage-noise-491.md) — verify "coverage shortfall is noise" claims: grep both Cobertura XMLs for the changed assembly (zero-effect proof), diff-check attributed unrelated files, distrust deleted-raw-XML "reproducibility" narratives (#491) - [poshqc-bundled-coverage-artifact-reads-zero](project_poshqc-bundled-coverage-artifact-reads-zero.md) — run_poshqc_test writes the canonical artifacts/pester/powershell-coverage.xml with 0 covered lines (invalid capture) → honest FAIL row, non-blocking; adjudicate from committed direct-Pester JaCoCo (#441) - [441-review-residuals-and-494-handoff](project_441-review-residuals-and-494-handoff.md) — #441 PASS/0 blocking; 85.0317% vs 85% margin is #494's decision; NF-1 uncovered Helpers.ps1:220; NF-2 tooling filing owed; #529-#532 OPEN, deliberately unfixed - [nullable-remediation-epic-review-pattern](project_nullable-remediation-epic-review-pattern.md) — utilitiescs-nullable epic children (#363+): epic-integration diff base, per-file #nullable enable, net481; solution gate fails on pre-existing warnings — use per-project AC1 proof @@ -59,3 +60,4 @@ - [epic fan-in artifact path + hook regex traps](project_epic_fanin_artifact_path_and_hook_regex.md) — hook regex demands docs/features/active/; UNVERIFIED is a narrowing word and "Pester" alone satisfies both label+coverage tests - [build-ci-coverage-gate-fidelity epic outcome](project_build-ci-coverage-gate-fidelity-epic-outcome.md) — 80-vs-85 contradiction NOT removed; runner gate 80 vs hook 85; C# 85.55%, PS branch nonexistent - [PowerShell coverage is nondeterministic](project_powershell-coverage-nondeterministic-vsbuild-tests.md) — Invoke-VSBuild.Tests.ps1 runs vswhere + Sync-PackageReferences for real; measure PS coverage in-session, never quote a stored figure +- [449-review-residuals](project_449-review-residuals.md) — PASS/0 blocking; untracked #584 promotion doc owed a non-child route; unused usings in base test file; AC-supersession-via-plan-provision pattern validated diff --git a/.claude/agent-memory/feature-review/project_449-review-residuals.md b/.claude/agent-memory/feature-review/project_449-review-residuals.md new file mode 100644 index 000000000..cf75740fe --- /dev/null +++ b/.claude/agent-memory/feature-review/project_449-review-residuals.md @@ -0,0 +1,16 @@ +--- +name: 449-review-residuals +description: '#449 QfcExplorerController review: PASS/0 blocking; residuals: untracked #584 promotion doc needs non-child commit route; unused usings in base test file; AC-12/AC-16 supersession pattern validated' +metadata: + type: project +--- + +#449 (epic quickfiler-suite-determinism-foundation, wave 0) reviewed 2026-08-22: PASS, 0 blocking, all 16 spec ACs verified. + +**Why:** three residuals survive the merge and matter at epic close or later reviews. + +**How to apply:** +- `docs/features/potential/promoted/2026-08-22-uithread-dispatcher-null-race-progresstrackerasync.md` (Issue #584, the ProgressTrackerAsync STA flake) sits UNTRACKED in the 449 worktree — the epic forbids children committing under `docs/features/potential/**`, so verify at epic close that it reached the repo via a non-child route or that issue #584's body is accepted as the durable record. +- `QuickFiler.Test/Controllers/QfcExplorerControllerTests.cs:1-2` carries two unused usings (`System.Collections`, `System.Collections.Generic`) stranded by the 500-line-cap partial-class split; fix opportunistically on next touch. +- Validated pattern (reusable): when a plan's own split provision ([P6-Tn]) fires and supersedes an AC's literal figure ("exactly one appended line" -> two; 485 -> 486), the AC stays PASS if the supersession is recorded in a dedicated evidence artifact and carried into the check-off notes — do not raise the numeric divergence alone as a defect (cf. [[441-review-residuals-and-494-handoff]]). +- Coverage adjudication precedent: with `artifacts/csharp/coverage.xml` deliberately absent and no pr_context summary, verifying from the executor's raw `coverage/*.cobertura.xml` on disk (recompute root/package/per-file figures with a scratch parser) satisfies the evidence-verification model; a class suppressed by a class-level `[ExcludeFromCodeCoverage]` at baseline is "absent from the report", not 0% — a per-file grep for matching `` elements (0 at baseline, 4 after) proves it. diff --git a/.claude/agent-memory/feature-review/project_verify-zero-own-effect-coverage-noise-491.md b/.claude/agent-memory/feature-review/project_verify-zero-own-effect-coverage-noise-491.md new file mode 100644 index 000000000..825edd5b9 --- /dev/null +++ b/.claude/agent-memory/feature-review/project_verify-zero-own-effect-coverage-noise-491.md @@ -0,0 +1,32 @@ +--- +name: verify-zero-own-effect-coverage-noise-491 +description: technique for independently verifying a "coverage shortfall has zero own-effect" claim from dual committed Cobertura XMLs, used on #491 +metadata: + type: project +--- + +When an executor claims a post-change coverage shortfall is environmental noise unrelated to the +change (rather than a real regression), verify it directly from the two committed Cobertura XML +files rather than accepting the narrative: + +1. Confirm root `` attributes match the claimed + baseline/post-change numbers exactly, and that `lines-valid` (denominator) is identical between + the two files (proves same instrumented surface). +2. For "zero own-effect" claims (e.g. the changed files are in an excluded/uninstrumented + assembly), grep both XML files for the changed assembly's `name="..."` and `filename="..."` + patterns — zero matches in both files is direct proof of zero own-effect, not just an assertion + about the harness's documented exclusion policy. +3. For "attributed to unrelated file X" claims, extract that file's `` node + from both XML files and confirm a real numeric drop; then `git diff --name-only ` + to confirm X is not in the branch's diff. +4. If the executor ran multiple diagnostic capture attempts but deleted the raw XML for all but the + canonical one, treat only the canonical (officially-cited) capture as verified; disclose the + others as narrative-only corroboration in the audit rather than silently accepting "reproducible + across N runs" as proven. + +This four-step check let a #491 AC10 shortfall (85.5627% vs 85.5788% baseline, -10 lines) be +dispositioned non-blocking on independently verified evidence rather than on trust: zero +`QuickFiler.Test` occurrences in either Cobertura file, and the entire shortfall traced to +`SegmentStopWatch.cs` + `OlTableExtensions.Etl.cs`, both confirmed absent from the branch diff. + +Related: [csharp-coverage-constants-nondeterministic], [jacoco-summary-substitution-is-valid-coverage-evidence]. diff --git a/.claude/agent-memory/orchestrator/MEMORY.md b/.claude/agent-memory/orchestrator/MEMORY.md index fc149e926..229719e3c 100644 --- a/.claude/agent-memory/orchestrator/MEMORY.md +++ b/.claude/agent-memory/orchestrator/MEMORY.md @@ -1,7 +1,7 @@ - [CLAUDE.md nullable command != CI gate — RESOLVED by #540](project_claudemd_nullable_command_diverges_from_ci.md) — docs now match ci.yml; a reappearance of `/p:Nullable=enable` or `/t:Build` is a regression. Carries the 195-error UtilitiesCS lower bound for #492 - [PoshQC test drops coverage.xml at repo root](poshqc-test-drops-coverage-xml-at-repo-root.md) — untracked, not gitignored/csharpierignored; inflates the CSharpier file count and leaks into `git add -A` - [Agent-worktree discovery + evidence hygiene](project_agent_worktree_discovery_and_evidence_hygiene.md) — `\.claude\` test-glob filter must use the RELATIVE path; never commit raw Cobertura -- [Completion-gate receipt shapes](completion-gate-receipt-shapes.md) — exact fields require_complete wants: delegation_receipts as a LIST, skill_receipts required:true +- [Completion-gate receipt shapes](completion-gate-receipt-shapes.md) — SOLVED: the missing key is `evidence`; plus the bug-route `new_potential_bug_entry` swap that MCP and the hook disagree on - [JaCoCo not Cobertura for coverage evidence](jacoco-not-cobertura-for-evidence.md) — maintainer deletes committed Cobertura; convert to package-level JaCoCo before pushing - [Store-lockup watchdog null-model hazard](project_store_lockup_watchdog_null_model_hazard.md) — new startup COM scopes need a phase-branch returning before the disable-service write - [VS Code extension location](project_extension_location.md) — the extension lives at `extensions/drm-copilot/`, not the repo root @@ -51,7 +51,8 @@ - [STA controls ratified as last resort](feedback_sta_controls_last_resort_ratified.md) — unshown WinForms controls on STA are OK only after seams, in dedicated *.StaTests.cs - [Epic children need full lifecycle + PRs](feedback_epic_children_require_full_lifecycle_and_prs.md) — maintainer rejected executor-driver shortcuts and direct child merges; PRs are mandatory - [Verify subagent capability claims](feedback_verify_subagent_capability_claims.md) — never relay "agent type not registered" without checking .claude/agents yourself -- [Epic-child plan Phase 0 paths are stale](feedback_plan_phase0_paths_are_stale_in_epic_children.md) — plans cite the planning worktree's absolute paths; redirect the executor to the current one +- [Epic-child plan Phase 0 paths are stale](feedback_plan_phase0_paths_are_stale_in_epic_children.md) — redirect the executor at execution time; in PREPARATION mode fix the plan to resolve WS from git rev-parse +- [Absolute-zero gate on a sibling-owned assembly](absolute-zero-gate-on-sibling-owned-assembly.md) — a child can't pass "Failed 0" over an assembly holding a wave-sibling's failures; scope zero to owned classes - [Unplanned epic-child worktree mechanics](unplanned-epic-child-worktree-mechanics.md) — cross-worktree delegation works via absolute paths; C# tools need pwsh + explicit paths - [Epic generic-constraint cascades across children](epic-generic-constraint-cascades-multiple-children.md) — a `where TKey : notnull` emits CS8714 in EVERY consumer; enumerate all of them first - [Parallel epic children name collisions](parallel-epic-children-name-collisions.md) — siblings coin identical type names; CS0101/CS0104 surface only at rebase; rename YOUR types @@ -78,3 +79,7 @@ - [Removing a halt requires branch propagation](removing-a-halt-requires-branch-propagation.md) — converting a HALT into a recorded blocker strands downstream tasks; propagate to ALL consumers - [Bash tool rejects complex commands in isolated worktrees](bash-tool-rejects-complex-commands-in-isolated-worktree.md) — heredoc+redirect+git compounds are refused as unverifiable; gather with plain chained commands, then author via Write - [#457 coverage moved UP, and the kickoff figure was wrong](project_457_coverage_moved_up_not_down.md) — the denominator fix raised the rate 85.3514%→85.5355%; kickoff's 85.0317% matched no measurement, so #494 must re-measure +- [atomic-planner has no MCP validator tool](atomic-planner-lacks-mcp-validator-tool.md) — it cannot run the mandatory plan gate; run it yourself and tell the planner not to fabricate a result +- [C# agent worktree needs three bootstrap steps](csharp-agent-worktree-needs-three-bootstrap-steps.md) — no `.dotnet-sdk`, no `packages/`, and a clean restore still hits `error CS0006` on skewed analyzer versions; green CI is cache-explained, not tolerance +- [potential_to_issue keeps ONLY the Summary section](potential-to-issue-keeps-only-summary-section.md) — every other section becomes "(not provided in potential file)"; verify the issue body, post the rest as a comment +- [Epic kickoff facts need independent measurement](epic-kickoff-facts-need-independent-measurement.md) — a kickoff predicted a 500-line cap finding from a conflated file; the named file was 323 lines, not 1,065 diff --git a/.claude/agent-memory/orchestrator/absolute-zero-gate-on-sibling-owned-assembly.md b/.claude/agent-memory/orchestrator/absolute-zero-gate-on-sibling-owned-assembly.md new file mode 100644 index 000000000..9d40a20d3 --- /dev/null +++ b/.claude/agent-memory/orchestrator/absolute-zero-gate-on-sibling-owned-assembly.md @@ -0,0 +1,19 @@ +--- +name: absolute-zero-gate-on-sibling-owned-assembly +description: An epic child's plan must not assert an absolute "Failed 0" over a whole test assembly that also carries a wave-sibling's known failures; scope absolute zero to the classes the child owns +metadata: + type: feedback +--- + +An epic-child plan must not state an acceptance gate the child cannot pass by its own effort. The recurring shape is an absolute `Failed 0` asserted over an entire test assembly, when that assembly also contains failing tests owned by a **concurrent wave sibling**. Scope the absolute-zero condition to the test classes the child owns, and use a baseline-subset condition for the rest of the assembly. + +**Why:** in a wave with an empty dependency graph, siblings run concurrently and their fixes are not guaranteed to have merged. The executor then hits a red gate at a point where blocking is forbidden, and its only in-task recourses are to weaken a sibling's test or to record a false pass — both prohibited. Confirmed on #445 (quickfiler-suite-determinism-foundation): `P4-T5`/`P5-T6` pinned `QuickFiler.Test` to `Failed 0`, but that assembly carries #511/#571's two intermittently failing pump tests (`QfcItemController.InitializationTests.Part3.cs`) and #491's live `Form1`, and the runsettings set `0` — full CPU parallelism, exactly the load condition that makes them fail. + +**How to apply:** when preflight proposes relaxing a gate, do not accept the relaxation on the agent's word. Verify the two facts yourself: that the tolerated failures really live in that assembly, and that the sibling really is a concurrent wave peer (`depends_on: []` on both, same wave in the epic manifest). Then require the relaxed gate to remain **falsifiable against the child's own regression**: a baseline-subset condition over test-NAME sets is sufficient, because a failure this child newly introduces is by construction absent from the baseline set, so the subset test still fails. Tolerating two named tests does not tolerate anything else in their class or file. + +Two corollaries worth carrying forward: + +- Unpinning `EXIT_CODE: 0` is correct once a pre-existing failure is tolerated, since `vstest` exits non-zero on any failure. Keep the `EXIT_CODE:` field mandatory in the evidence artifact; only the pinned value goes. +- A scoped run restricted to a class the child owns (via `FullyQualifiedName~`) should still pin `EXIT_CODE: 0` and an exact Passed count. The relaxation belongs only to the whole-assembly run. + +Related: [[preflight-catches-vacuous-gates]], [[plan-phase0-paths-are-stale-in-epic-children]], [[epic-child-pr-gate-gotchas]]. diff --git a/.claude/agent-memory/orchestrator/atomic-planner-lacks-mcp-validator-tool.md b/.claude/agent-memory/orchestrator/atomic-planner-lacks-mcp-validator-tool.md new file mode 100644 index 000000000..ebee39ce7 --- /dev/null +++ b/.claude/agent-memory/orchestrator/atomic-planner-lacks-mcp-validator-tool.md @@ -0,0 +1,17 @@ +--- +name: atomic-planner-lacks-mcp-validator-tool +description: Agent(atomic-planner) has no mcp__drm-copilot__* tool in its surface, so the orchestrator must run the mandatory plan validator gate itself after every planning or revision delegation +metadata: + type: project +--- + +`Agent(atomic-planner)` is provisioned with Read, Grep, Glob, Edit, and Write only. It has no Bash and no `mcp__drm-copilot__*` tool, so it CANNOT run `validate_orchestration_artifacts` — the gate that `atomic-plan-contract` makes mandatory before a plan may be treated as approved. + +**Why:** `atomic-plan-contract` says "run the MCP validator; do not treat human-readable summaries as a substitute for validator success." A planner asked to self-validate can only do one of two things: report honestly that the tool is absent, or fabricate a pass. Observed twice on 2026-08-21 (#449): the planner correctly reported "VALIDATOR NOT RUN — tool unavailable in this agent's surface. I will not claim a gate passed that I did not execute," and substituted a structural self-check (phase-heading form, task-ID sequence, line-ending uniformity). That is the right behavior and should be reinforced, not corrected. + +**How to apply:** +- The ORCHESTRATOR runs the plan validator after every planner delegation. Do not put "run the validator and report its result" in the planner prompt as a hard requirement; instead say explicitly "I will run the validator myself; do not fabricate a validator result." That removes the pressure to invent a pass. +- Independently verify the planner's edit list against the file rather than trusting the report. On #449 the planner's prose said "all 91 tasks unchanged" while its own per-phase enumeration summed to 98 (the real count) — a harmless miscount, but it shows the narrative can drift from the artifact. +- The same absence applies to the planner writing memory: on #449 it declined to write `.claude/agent-memory/` because that tree is tracked and the plan's own final tasks gate on an empty `git status --porcelain`. Sound reasoning; the orchestrator should carry such flagged-for-upstream items in its own report. + +Related: [[preflight-catches-vacuous-gates]], [[verify-subagent-capability-claims]] (verify a capability CLAIM against `.claude/agents` — but a report of a tool's absence that matches the registered tool list is accurate, not an excuse). diff --git a/.claude/agent-memory/orchestrator/collect-pr-context-lands-in-main-checkout.md b/.claude/agent-memory/orchestrator/collect-pr-context-lands-in-main-checkout.md index 6aa6570aa..9f2cbb06e 100644 --- a/.claude/agent-memory/orchestrator/collect-pr-context-lands-in-main-checkout.md +++ b/.claude/agent-memory/orchestrator/collect-pr-context-lands-in-main-checkout.md @@ -1,21 +1,47 @@ --- name: collect-pr-context-lands-in-main-checkout -description: In a worktree, collect_pr_context writes pr_context.* to the MAIN checkout, but the enforce-pr-author hook reads it from the worktree CWD — copy it in before gh pr create +description: From an isolated agent worktree, collect_pr_context returns ok:true but writes to the PRIMARY checkout and claims gh is unavailable - author the PR body from the real diff instead metadata: type: project --- -When the orchestrator runs in a git worktree, `mcp__drm-copilot__collect_pr_context` reports writing `artifacts/pr_context.summary.txt` to the worktree but it actually lands in the MAIN checkout (e.g. `C:/Users/.../repos/TaskMaster/artifacts/pr_context.summary.txt`), not the worktree's `artifacts/`. +Calling `mcp__drm-copilot__collect_pr_context` from a `.claude/worktrees/` worktree returns +`ok:true` and lists artifact paths **inside that worktree**, but the files it actually writes land in +the PRIMARY checkout (`C:\Users\DanMoisan\repos\TaskMaster\artifacts\`). The `workspace_root` +argument does not redirect it. Confirmed again 2026-08-22 (epic child #445): the returned paths had +an mtime ~10 minutes older than the call, while the primary checkout's copy was freshly written. -**Why:** `.claude/hooks/enforce-pr-author-skill.ps1` reads `artifacts/pr_context.summary.txt` relative to the CWD of the `gh pr create` command (the worktree). If it is missing there, the hook blocks with the "context artifact absent" (Case C) reason, and the receipt staleness check has nothing to compare against. +Two further defects make the artifact unusable rather than merely misplaced: -**How to apply (in-thread pr-author flow, since Agent(pr-author) is unavailable — see [[pr-author-hook-blocks-gh-in-this-repo]]):** -1. Run `collect_pr_context --base `; then `cp` the main-checkout `artifacts/pr_context.summary.txt` (and `.appendix.txt`) into the worktree `artifacts/`. -2. Write `artifacts/pr_body_.md`; compute lowercase-hex SHA-256 of its bytes; write `artifacts/pr_body_.receipt.json` with `created_at` STRICTLY NEWER than the (copied) summary's LastWriteTimeUtc — copy the summary FIRST, then sleep, then stamp the receipt. -3. `gh pr create --base --body-file artifacts/pr_body_.md`. +1. **It claims `gh` is unavailable** (`GitHub CLI unavailable: ... not installed`) when `gh auth + status` in the same worktree authenticates fine. Never accept that claim; verify `gh` yourself. +2. **The primary checkout is on a different branch**, so the diff it computes is not your branch's + diff at all. Copying it into the worktree would import a wrong changed-file list. -Also: `artifacts/orchestration/orchestrator-state.json` and `artifacts/pr_body_*`/`artifacts/pr_context.*` are all GITIGNORED in this repo — the checkpoint is local on-disk state (which is exactly what the hooks read); do not expect it in commits/PRs, and a "clean" `git status` does not mean the checkpoint is committed. +**Why:** a PR body built from that artifact misstates the change. In #445 the stale worktree copy +recorded a head SHA one commit behind and omitted all three review artifacts. **Refinement (#327, 2026-07-16, dedicated agent worktree):** when the child orchestrator's session cwd IS the feature worktree (the harness gave me an isolated `.claude/worktrees/agent-` worktree and I `git switch -c` the feature branch there), collect_pr_context wrote `pr_context.*` DIRECTLY into that worktree's `artifacts/` (returned paths were the worktree) and the hook read them there — no main-checkout copy step was needed. The main-checkout-landing behavior above applies when session cwd differs from the feature worktree. TWO quirks still bit: (a) collect_pr_context reported a `Head:` SHA one commit BEHIND my true branch tip and a second call did NOT rewrite the file (identical mtime) — do not trust the summary's `Head:` line; the pushed branch tip and GitHub's own base...head diff are authoritative, so it is harmless. (b) The receipt `created_at > summary mtime` check still held because I wrote the receipt after collect ran; no sleep was needed since minutes had elapsed. Child->integration PR #334 merged fine (merge commit 9559c73c) on blocking_count==0 with zero CI ([[project_epic_child_prs_no_ci]]). **Quirk (a) is stronger than "does not rewrite on a second call" (#441, 2026-08-10).** `collect_pr_context` returned `ok:true` with worktree paths, but the on-disk `artifacts/pr_context.summary.txt` was untouched — mtime and content still belonged to a file the feature-review subagent had HAND-AUTHORED an hour earlier, whose own first line read "collect_pr_context MCP tool unavailable in this session" and whose `Base:` was the old pre-change baseline rather than the `--base` I passed. So the tool will report success over a pre-existing file it did not write. Consequences: (1) never read the summary to learn the diff — author the PR body from `git diff ..HEAD --stat` and `git log ..HEAD` ([[pr-context-summary-unreliable-gh-and-classification]]); (2) the receipt's `created_at > summary mtime` check gets *easier*, not harder, because the stale mtime is older — but verify it rather than assuming; (3) `feature-review` also lacks `collect_pr_context`, so a reviewer may leave a hand-authored decoy in `artifacts/` that survives your own collect call. + +**CROSS-CHILD CONTAMINATION (#449, 2026-08-22) — the most dangerous variant, promoted to issue #589.** In an isolated agent worktree the tool returned `ok:true` with worktree paths, wrote NOTHING there, and wrote to the MAIN checkout instead — a location SHARED by every concurrently running epic/parallel child. Sibling child #491 ran its own collect in the gap between my collect and my copy, so the file I copied was ENTIRELY #491's: `Head ref (resolved): bug/quickfiler-test-form1-live-form-491-exec @ bec83397`, and its `Additional context files` listed 14 artifacts from #491's feature folder. Authoring from it would have described the wrong change on my PR, with `ok:true` and a well-formed file as the only signals. + +**How to apply — verify OWNERSHIP, not just presence.** Presence checks and mtime checks both pass on a sibling's file. After copying, assert the summary's `Head ref (resolved)` SHA equals your own `git rev-parse HEAD`, and sanity-check that the feature-folder references in the file are YOURS (I counted 96 for my folder and 0 for #491's). Re-run and re-copy if it does not match. Then author the body from the real `git diff ..HEAD` regardless — the file is only safe as a file LIST, never as a narrative. + +Two more defects confirmed in the same bundle: the summary reported "GitHub CLI (gh) is not installed" while `gh auth status` and `gh issue view` both worked in the same session; and the `author asserted` autoclose list contained `#AC-1`..`#AC-16` (acceptance-criterion IDs scraped as issue numbers) plus three issues that were not mine to close. Never emit `Closes` from that list. Note also that a child PR into an epic integration branch cannot auto-close anything — GitHub only honors closing keywords merging into the DEFAULT branch — so `Refs #NNN` is the correct form and the epic's final integration-to-main PR carries the close. + +**Independent confirmation and the simplest safe remedy (#445, 2026-08-22).** Same run, same wave: +`ok:true`, worktree paths returned, nothing written there, primary checkout freshly written. The +worktree copy I would have used was a decoy the feature-review subagent had hand-authored (quirk (a) +above), recording a head SHA one commit behind and omitting all three review artifacts. + +Rather than copy-then-verify-ownership, the cheaper remedy is **do not copy at all**. Treat +`collect_pr_context` purely as a receipt formality, then REGENERATE `artifacts/pr_context.summary.txt` +yourself in your own worktree from `git rev-parse HEAD`, `git merge-base`, `git log --oneline +..HEAD`, and `git diff --numstat ...HEAD`. This is strictly safer than the ownership +check, because a file you wrote from your own git state cannot be a sibling's, and it simultaneously +satisfies the pr-author receipt's `created_at > summary mtime` check as long as you write the receipt +afterwards. `pr_context.*` is gitignored, so regenerating dirties nothing. See +[[pr-context-summary-unreliable-gh-and-classification]] and +[[pr-author-hook-blocks-gh-in-this-repo]]. diff --git a/.claude/agent-memory/orchestrator/completion-gate-receipt-shapes.md b/.claude/agent-memory/orchestrator/completion-gate-receipt-shapes.md index 6540dfa7a..8ac6618a1 100644 --- a/.claude/agent-memory/orchestrator/completion-gate-receipt-shapes.md +++ b/.claude/agent-memory/orchestrator/completion-gate-receipt-shapes.md @@ -1,39 +1,46 @@ --- name: completion-gate-receipt-shapes -description: Exact field shapes the MCP orchestrator-state completion gate requires for delegation, skill, and MCP receipts — guessing the key names wastes several validate cycles +description: The exact receipt shape the routing-contract completion gate wants - the missing key was always `evidence`; also the bug-promotion tool-name swap and the unresolvable MCP-vs-hook divergence it creates metadata: type: project --- -The MCP `validate_orchestration_artifacts` completion gate (`require_complete: true`) rejects -plausible-looking receipt shapes. The authoritative source is -`extensions/drm-copilot/src/lib/validate/orchestrator-state-routing.ts` in the drm-copilot repo -(functions `receiptSkills`, `mcpTools`, `receiptAgents`). Read it instead of guessing. - -**Why:** On #508 I burned three validate cycles guessing `skill_name`/`tool_name`/`mcp_receipts`. -The gate silently ignores a receipt that is missing any required field — it does not say *which* -field is wrong, only that the whole receipt is "missing". - -**How to apply:** Use these exact shapes. - -- `delegation_receipts` must be a **list** (not an object namespaced by phase). Each entry needs: - `agent_name`, `step`, `agent_id`, `skill_source`, `started_at`, `completed_at`, `result_signal`, - `artifact_paths`. If you also need promotion receipts, put them under a *separate* top-level key - (for example `promotion_receipts`) — the list form is what supplies the delegated-agent set for - the model-routing gate, so it cannot also be an object. -- `skill_receipts[]` needs exactly `{ skill: , required: true, evidence: }`. - `required` must be the boolean `true`; a missing `required` silently drops the skill. -- MCP receipts live under **`mcp_call_receipts`**, not `mcp_receipts`, and need - `{ tool: , ok: true, evidence: }`. `tool` must match the canonical - name in `required_mcp_tools` — if you invoked a variant (for example `new_potential_bug_entry` - for the `new_potential_entry` requirement), put the canonical name in `tool` and disclose the - actual variant inside `evidence`. -- `ci_gate` needs `verified_at` in addition to `conclusion`. -- `local_execution_overrides` and `delegation_bypasses` must both be present and be **empty lists**. - -Also required earlier, at PR-creation time: `relativeFile`, `long-name`, and `work-mode` (hyphenated) -as flat top-level keys, and steps 5-8 all non-pending. See [[orchestrator-state-flat-keys-and-enum]]. - -TaskMaster has no Python validator, so the PR-author hook uses the portable PowerShell path -(`Test-OrchestratorStatePrCreationReadiness` in `.claude/lib/orchestrator-state/OrchestratorState.psm1`). -Run it directly to preflight before `gh pr create` — it tells you exactly which step is pending. +**SOLVED 2026-08-22 (epic child #445).** The shape that earlier runs could not discover by guessing is +readable in `.claude/lib/orchestrator-state/OrchestratorStateRoutingContract.psm1` +(`Get-CheckpointAcknowledgedName`, ~line 225). Read it; do not guess. + +A receipt counts **only when all three hold together**: + +- `skill_receipts[]` — `skill` non-blank string, `required` **boolean** `true`, **`evidence` non-blank string**. +- `mcp_call_receipts[]` — `tool` non-blank string, `ok` **boolean** `true`, **`evidence` non-blank string**. + +**`evidence` is the key that was always missing.** Earlier attempts tried `outcome`, `detail`, +`skills_used`, `mcp_receipts`, `mcp_tool_calls` and failed, because the harvest reads a +hard-coded `'evidence'` member regardless of the array. Truthy-but-not-`$true` flags +(`1`, `"true"`) deliberately do not count. + +`delegation_receipts[]` is separate and needs `agent_name`, `agent_id`, `step`, `phase`, +`skill_source`, `result_signal`, `artifact_paths` (list). It is a LIST, not an object. + +### Bug-promotion tool-name swap, and the divergence it creates + +`Get-ResolvedRequiredMcpTool` swaps `new_potential_entry` for **`new_potential_bug_entry`** when the +checkpoint's promotion type is `bug`. The MCP TypeScript surface does **not** apply that swap. +Because `required_mcp_tools` is checked by EXACT LIST EQUALITY, the two demands are mutually +exclusive on a bug route and **no single list satisfies both**. + +**Conform to the HOOK** (`new_potential_bug_entry`). It is the mechanism that actually blocks, and it +is semantically right for a bug. Expect the MCP `require_complete` call to keep reporting exactly two +residual errors; they are divergence artifacts, not missing work. + +**Why:** this supersedes the pessimistic conclusion in +[[orchestrator-state-validator-divergence]] that `--require-complete` is simply unsatisfiable for a +resumed-at-execution epic child. It is satisfiable *at the real gate* +(`.claude/hooks/validate-orchestrator-output.ps1` returned `EXIT_CODE=0`); only the MCP surface stays +unsatisfiable, and only on the two swap-related errors. + +**How to apply:** when the completion gate reports missing skill or MCP receipts, add `evidence` to +every receipt before changing anything else. Prove the result with the hook, invoking it as +`$env:CLAUDE_HOOK_INPUT = '{"output":""}'` (top-level `.output`) and asserting `EXIT_CODE=0` +— not with the MCP tool. See [[orchestrator-state-flat-keys-and-enum]] and +[[epic-child-self-merge-step9-passed-vs-verified]]. diff --git a/.claude/agent-memory/orchestrator/csharp-agent-worktree-needs-three-bootstrap-steps.md b/.claude/agent-memory/orchestrator/csharp-agent-worktree-needs-three-bootstrap-steps.md new file mode 100644 index 000000000..5fcfb1a66 --- /dev/null +++ b/.claude/agent-memory/orchestrator/csharp-agent-worktree-needs-three-bootstrap-steps.md @@ -0,0 +1,43 @@ +--- +name: csharp-agent-worktree-needs-three-bootstrap-steps +description: A fresh agent worktree cannot run any C# gate until .dotnet-sdk, packages/, AND two back-filled analyzer versions exist; green CI is NOT evidence the third is unnecessary +metadata: + type: project +--- + +Any C# plan executing inside a `.claude/worktrees/` worktree needs THREE bootstrap steps +before the first `dotnet` command and the first `msbuild`. Preflight will flag their absence as +blocking, because every `EXIT_CODE: 0` acceptance downstream is unreachable without them. + +1. **`.dotnet-sdk` is absent.** `global.json` pins `sdk.version 8.0.205` with + `rollForward: latestFeature` and `paths: [".dotnet-sdk", "$host$"]`. A fresh worktree has none, and + the host SDK (10.0.302) cannot satisfy it. `dotnet --version` from the worktree root prints the + `global.json` `errorMessage` instead of a version. Fix: `scripts/vscode/Install-RepoDotNetSdk.ps1`, + or mirror `C:\Users\DanMoisan\repos\TaskMaster\.dotnet-sdk`. Ignored by `.gitignore:350` (`.dotnet*/`). +2. **`packages/` is absent.** Every project declares `EnsureNuGetPackageBuildImports` whose `` + fires at `BeforeTargets="PrepareForBuild"`, so msbuild hard-fails. Fix: `nuget restore TaskMaster.sln` + (what CI does at `.github/workflows/_build-analyzers.yml:45`). Restored content is ignored by + `.gitignore:191` (`**/[Pp]ackages/*`) — NOT by line 349, which is blank. +3. **A clean restore still breaks the build.** All 16 first-party `.csproj` files carry UNCONDITIONAL + `` items naming `Meziantou.Analyzer.3.0.156` and `Roslynator.Analyzers.4.16.0`, + while all 16 `packages.config` pin `3.0.174` and `4.16.1`. Dependabot `f8e22af7` updated only the + `Condition`-guarded ``/`` lines and `packages.config`, missing the hand-authored + Issue-#181 analyzer items. A missing `/analyzer:` path is **`error CS0006`, not a warning** — + verified by direct `csc.exe` probe, exit 1. Fix without touching tracked files: + `nuget install Meziantou.Analyzer -Version 3.0.156 -OutputDirectory packages` and the same for + `Roslynator.Analyzers -Version 4.16.0`. Only these two packages are skewed; AsyncFixer, + BannedApiAnalyzers, MSTest.Analyzers and SonarAnalyzer all match their pins. + +**Why:** Item 3 is a live repository-wide latent defect, and the obvious disconfirming evidence is +misleading. `_build-analyzers.yml:38-41` caches `path: packages` with a PREFIX `restore-keys` +fallback, so a cache-key miss (which any `packages.config` change guarantees) restores a pre-bump +tree still holding `3.0.156`/`4.16.0`, and `nuget restore` merely adds the new versions beside them. +The main checkout shows the same accumulation (`3.0.101/.123/.156/.174`). So **green CI does not +prove the compile tolerates a missing analyzer path** — it proves the old folders lingered. A cold +cache or a fresh clone would fail. + +**How to apply:** When routing C# work into an agent worktree, expect preflight to require these +three tasks and do not argue them away. Never "fix" item 3 by editing a `.csproj` inside a scoped +child — that breaches the no-project-file-edit constraint; back-fill the untracked `packages/` tree +instead and file a follow-up issue for the real realignment. See +[[whole-repo-ci-gate-not-out-of-scope]] and [[bash-tool-mangles-msbuild-switches]]. 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 new file mode 100644 index 000000000..80f79edc1 --- /dev/null +++ b/.claude/agent-memory/orchestrator/epic-kickoff-facts-need-independent-measurement.md @@ -0,0 +1,36 @@ +--- +name: epic-kickoff-facts-need-independent-measurement +description: An epic kickoff can assert a measured fact that is false by file conflation and instruct the child to act on it; measure the named file yourself before accepting a predicted Blocking finding +metadata: + type: feedback +--- + +Measure every quantitative fact an epic kickoff states about a file before acting on it, especially +when the kickoff uses that fact to PREDICT a finding and prescribe a response. + +**Why:** On the #449 child of `quickfiler-suite-determinism-foundation`, the kickoff stated that +`QuickFiler/Controllers/QfcExplorerController.cs` "is 1,065 lines today", that `feature-review` would +therefore raise the 500-line-cap violation, and that the child should record it as out-of-scope and +"promote the split as its own issue". Measured on the branch, the file was **323** lines — under the +cap. The 1,065-line file is `QuickFiler/Legacy/QuickFileController.cs`, an uncompiled file the change +never touches. The kickoff had conflated two similarly named controllers in the same feature area. + +Acting on the stated figure would have produced a fabricated cap-violation finding, a spurious +promoted issue, and possibly an out-of-scope partial-class split — all defended by citing the +kickoff. The committed plan's own tasks already carried the correct attribution, so the plan and the +kickoff disagreed and the plan was right. + +**How to apply:** +- Treat a kickoff's `file:line` and line-count claims exactly like a promoted potential's citations: + as claims to re-derive, not inputs. `wc -l` is one command. +- A kickoff that predicts a specific finding is the highest-risk case, because the prediction primes + both the orchestrator and the reviewer to confirm it. Brief the reviewer with the MEASURED value + and tell it explicitly not to raise a finding on a file that is not in the diff. +- When the kickoff and the committed plan disagree on a fact, prefer the plan — it was written + against the tree — then verify both against the file. +- 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. + +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]]. diff --git a/.claude/agent-memory/orchestrator/feedback_plan_phase0_paths_are_stale_in_epic_children.md b/.claude/agent-memory/orchestrator/feedback_plan_phase0_paths_are_stale_in_epic_children.md index 9324140d8..e048e9e57 100644 --- a/.claude/agent-memory/orchestrator/feedback_plan_phase0_paths_are_stale_in_epic_children.md +++ b/.claude/agent-memory/orchestrator/feedback_plan_phase0_paths_are_stale_in_epic_children.md @@ -1,6 +1,6 @@ --- name: plan-phase0-paths-are-stale-in-epic-children -description: Epic-child atomic plans cite the planning worktree's absolute paths in Phase 0 policy-read tasks; redirect the executor to the CURRENT worktree's files +description: Epic-child atomic plans cite the planning worktree's absolute paths; redirect the executor at execution time, but fix it in the PLAN at preparation time metadata: type: feedback --- @@ -10,3 +10,5 @@ Epic-child atomic plans are authored during epic planning in a different (planni **Why:** the plan is frozen at planning time; the execution worktree is created later with a different `agent-` path. An executor that reads the cited paths verbatim fails P0 and can wrongly report BLOCKED. **How to apply:** when delegating an epic-child atomic plan to `atomic-executor`, explicitly override the stale Phase 0 paths in the delegation prompt — point CLAUDE.md and the `.claude/rules/*.md` reads at the CURRENT working-directory worktree, and tell the executor to record the actual paths read in the P0-T5 evidence artifact. This is separate from the evidence-output path rule (evidence still goes to `/evidence//`). Confirmed on #265 (F5, store-lockup-resilience epic): plan cited `TaskMaster-wt-2026-07-07-13-21`; execution ran in `agent-aa788d7e018d8924e`. See also [[pr-context-summary-unreliable-gh-and-classification]] (collect_pr_context writing to the main checkout is the same class of worktree-path drift). + +**In PREPARATION mode, fix it in the plan instead.** The override above works only when you already hold the execution worktree. Under `route_id: preparation` there is none — `epic-orchestrator` creates a THIRD worktree later — so no absolute literal is correct and "redirect the executor" has no target. Have `atomic-planner` replace the pinned path with an instruction to resolve the workspace root at execution time from `git rev-parse --show-toplevel`. Confirmed on #445 (quickfiler-suite-determinism-foundation): the plan pinned `agent-aa16be3c847acea9b` inside a block headed "verified; use these, do not re-derive", and preflight raised it as BLOCKING precisely because that header forbids the one micro-action that would repair the value. The distinction is the severity rule: a stale path an executor may silently re-derive is advisory; a stale path under a do-not-re-derive instruction is blocking. See also [[preflight-catches-vacuous-gates]]. diff --git a/.claude/agent-memory/orchestrator/mcp-plan-validator-requires-lf.md b/.claude/agent-memory/orchestrator/mcp-plan-validator-requires-lf.md index 417956b42..2f6ebc52f 100644 --- a/.claude/agent-memory/orchestrator/mcp-plan-validator-requires-lf.md +++ b/.claude/agent-memory/orchestrator/mcp-plan-validator-requires-lf.md @@ -14,3 +14,7 @@ The `mcp__drm-copilot__validate_orchestration_artifacts` plan validator (`artifa **Best detection one-liner (verified 2026-08-10, #394):** `tr -cd '\r' < | wc -c` returns the exact CR byte count (0 = LF). It works identically on a working-tree file and on a raw blob via `git cat-file blob | tr -cd '\r' | wc -c`. Prefer it over `od -c | grep '\\r'`, which produces false positives on files containing Windows paths (the literal backslashes in `UtilitiesCS.Test\bin\Debug\...` confuse the pattern and reported 286 CRs on a file with genuinely zero). Also confirmed `core.autocrlf=true` in this repo, so an LF blob is materialized as CRLF in any FRESH checkout — a plan validated as LF during preparation can be CRLF when a later execution run checks the branch out into a new worktree. Re-check and re-normalize at the start of the execution run rather than trusting the preparation-run result. **Detection pitfall (cost cycles 2026-07-16, #327):** in git-bash, `grep -c $'\r' ` reports `0` even on a genuine CRLF file (grep strips the trailing CR before counting), so it is USELESS for detecting CRLF. Use `cmp -l committed_blob working_file` (git-bash cmp DID surface the 0x0D at the line terminator) or `perl -ne 'print "CR\n" if /\r/'`, or `git show HEAD: | cmp - `. The committed blob was LF but the checked-out working file was CRLF (+1 byte per line: 21495 -> 21656). A red-herring hunt (a `` `[P3-T4]` `` bracketed task-ID in prose on a `- ` bullet line) looked like the cause because `sed -i` on git-bash silently normalized CRLF->LF as a side effect, making the "bracket fix" copy pass for the wrong reason. + +**Counter-observation — CRLF is NOT always rejected (2026-08-21, #449, TaskMaster):** a 198-line CRLF plan (`file` reported "with CRLF line terminators") PASSED the MCP plan validator cleanly four separate times across two revision cycles, at 8 phases and 98 tasks, with `ok:true` and no warnings. No normalization was performed and none was needed. The epic manifest for that run carried a standing instruction to "re-normalize prepared plans to LF before revalidating", which would have been wasted work here. + +**How to apply:** treat CRLF as a *candidate* explanation for a heading/task-format failure, not a precondition to fix pre-emptively. RUN the validator first and read the actual result — the same discipline [[mcp-plan-validator-defective-em-dash]] records for the em-dash. Normalizing a plan that already passes is churn on an approved artifact, and an Edit-tool rewrite of a passing plan risks the pervasive-diff failure in [[mcp-plan-validator-editwrite-pervasive-diff]]. The LF requirement above is real where it was observed; it is version- or content-dependent, not universal. diff --git a/.claude/agent-memory/orchestrator/potential-to-issue-keeps-only-summary-section.md b/.claude/agent-memory/orchestrator/potential-to-issue-keeps-only-summary-section.md new file mode 100644 index 000000000..77054bf3b --- /dev/null +++ b/.claude/agent-memory/orchestrator/potential-to-issue-keeps-only-summary-section.md @@ -0,0 +1,50 @@ +--- +name: potential-to-issue-keeps-only-summary-section +description: potential_to_issue copies ONLY the "## Summary" section into the GitHub issue body and stubs every other bug-template section with "(not provided in potential file)" — all other analysis is silently dropped +metadata: + type: project +--- + +`mcp__drm-copilot__potential_to_issue` does NOT copy the promoted document verbatim into the GitHub +issue. It extracts the `## Summary` section only, then emits the bug template with every remaining +section filled in as the literal string `(not provided in potential file)`: + +``` +## Environment +(not provided in potential file) + +## Steps to Reproduce +(not provided in potential file) +... +## Source +From: docs/features/potential/.md +``` + +Verified 2026-08-22 on issue #584: the promoted document was ~90 lines carrying `## Root Cause`, +`## Impact`, `## Proposed Direction`, and `## Verification Notes`; the resulting issue body was 33 +lines and contained exactly one of five content markers. Root-cause analysis, the counter-example +citation, the proposed remedy, and every `file:line` verification pointer were all dropped. + +**Why this matters more than it looks.** The promotion lifecycle exists so that an out-of-scope +defect survives the archival of the feature folder that discovered it (see +[[feedback_promote_latent_defects_to_issues]]). If the local document is then deleted or never +committed — which is exactly the case inside an epic child, where the plan's hard constraints forbid +writing under `docs/features/potential/**` — the analysis is lost entirely and the issue retains only +a summary paragraph. The promotion appears to have succeeded while silently discarding the reasoning +that made it worth filing. + +**How to apply.** After every `potential_to_issue` call, diff what you wrote against what landed: + +1. `gh issue view --json body -q '.body' | wc -l` and compare against the source document. +2. Grep the issue body for two or three distinctive markers from your analysis sections. +3. If content was dropped, post the missing sections as an issue COMMENT + (`gh issue comment --body-file `). A comment is durable, needs no repository write, and + does not disturb an audited diff. + +Put the load-bearing content in `## Summary` when the document is short enough, or plan on the +follow-up comment when it is not. Do not assume the promoted markdown file is a durable second copy: +inside an agent worktree it is untracked and dies with the worktree. + +Related: [[potential-to-issue-creates-github-issue]] (the tool opens the issue itself — never also +`gh issue create`), [[potential-to-issue-needs-absolute-path]], +[[promotion-potential-md-may-not-persist]]. diff --git a/QuickFiler.Test/Controllers/KaStringAsyncTests.cs b/QuickFiler.Test/Controllers/KaStringAsyncTests.cs index ef44880ef..525542b2d 100644 --- a/QuickFiler.Test/Controllers/KaStringAsyncTests.cs +++ b/QuickFiler.Test/Controllers/KaStringAsyncTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Threading.Tasks; using FluentAssertions; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -131,7 +132,7 @@ public void KeyEquals_SingleCharNonMatchWhileActivated_InvokesToggleControlAndRe } [TestMethod] - public void KeyEquals_MultiCharNonMatch_InvokesUpdateWithFirstCharAndReturnsFalse() + public void KeyEquals_MultiCharNonMatchWhileActivated_InvokesUpdateWithFirstCharAndReturnsFalse() { // Arrange: "zz" is multi-char and not contained in "abc"; the else-if length>1 branch // invokes Update(Key.Substring(0,1)). @@ -164,5 +165,115 @@ public void KeyEquals_NullDelegatesAreToleratedInNonMatchBranches() // Assert act.Should().NotThrow("null Update/ToggleControl are guarded against"); } + + [TestMethod] + public void KeyEquals_MultiCharNonMatchWhileNotActivated_DoesNotInvokeUpdateAndReturnsFalse() + { + // Intent: defect-1 regression. Branch 3 (other.Length > 1, non-matching) must gate its + // Update side effect on Activated exactly as branches 1 and 2 do. Before the fix the + // branch-3 guard read "if (Update is not null)" with no Activated conjunct, so Update + // fired with "a" even though Activated was false. + + // Arrange: Activated is left at its false default. Update is non-null so that an + // ungated invocation is observable rather than swallowed by the null check. + var updates = new List(); + var ka = NewKa("abc", update: s => updates.Add(s)); + + // Act + var result = ka.KeyEquals("zz"); + + // Assert + updates.Should().BeEmpty("no KeyEquals side effect may fire while Activated is false"); + result.Should().BeFalse("a multi-character non-match returns false"); + } + + [TestMethod] + public void KeyEquals_LatchSurvivesMatchThenNonMatchTransition_StillResetsToFirstChar() + { + // Intent: pins the branch-1 early return described in the Hard Anti-Regression + // Constraint. Branch 1 deliberately does NOT clear Activated and returns early, so a row + // that matches at depth 1 and then fails at depth 2 still receives its Key[0] reset. If + // branch 1's early return were removed for symmetry, the first probe would consume the + // latch and the second probe would produce neither the Update nor the ToggleControl. + + // Arrange + var updates = new List(); + bool toggled = false; + var ka = NewKa("abc", update: s => updates.Add(s), toggle: () => toggled = true); + ka.Activated = true; + + // Act: a matching probe, then a non-matching probe on the same instance. + ka.KeyEquals("ab"); + ka.KeyEquals("zz"); + + // Assert: one collection assertion, deliberately not per-element Be(...) calls, so the + // AC19 retention gate on the pre-existing single-character assertion stays at one hit. + updates.Should().Equal(new[] { "b", "a" }); + toggled.Should().BeTrue("the surviving latch lets branch 3 toggle on the second probe"); + ka.Activated.Should().BeFalse("the non-matching branch clears the latch"); + } + + [TestMethod] + public void KeyEquals_EmptyProbe_ThrowsArgumentExceptionNamingOther() + { + // Intent: defect-2 regression. AC6 requires the rejection to hold for EVERY combination + // of instance state, so both variants live in one test method. ThrowExactly is + // load-bearing here: ArgumentOutOfRangeException derives from ArgumentException, so a + // plain Throw would already pass today for variant 2 and would gate + // nothing. + + // Arrange, variant 1: default instance, Activated false and Update null. Before the fix + // this returned true without throwing, because Key.Contains(string.Empty) is true for + // every key, so an empty probe silently matched every registered action. + var defaultKa = NewKa("abc"); + + // Act + Action actDefault = () => defaultKa.KeyEquals(""); + + // Assert + actDefault + .Should() + .ThrowExactly( + "an empty probe would otherwise match every registered action" + ) + .WithParameterName("other"); + + // Arrange, variant 2: Activated true with a non-null Update. Before the fix this threw + // ArgumentOutOfRangeException, because the offset arithmetic evaluated Substring(-1, 1). + var activatedKa = NewKa("abc", update: _ => { }); + activatedKa.Activated = true; + + // Act + Action actActivated = () => activatedKa.KeyEquals(""); + + // Assert + actActivated + .Should() + .ThrowExactly( + "the guard clause runs before any offset arithmetic" + ) + .WithParameterName("other"); + } + + [TestMethod] + public void KeyEquals_NullProbe_ThrowsArgumentNullExceptionNamingOther() + { + // Intent: defect-2 regression for the null case. The parameter-name clause is what + // distinguishes the explicit guard from today's behaviour: before the fix the throw + // originates inside string.Contains, whose parameter is named "value", not "other". The + // guard changes the exception's origin rather than its type, so a type-only assertion + // would pass unchanged and would gate nothing. + + // Arrange + var ka = NewKa("abc"); + + // Act + Action act = () => ka.KeyEquals(null); + + // Assert + act.Should() + .ThrowExactly("a null probe is a caller error") + .WithParameterName("other"); + } } } diff --git a/QuickFiler.Test/Controllers/QfcExplorerController.ConversationViewTests.cs b/QuickFiler.Test/Controllers/QfcExplorerController.ConversationViewTests.cs new file mode 100644 index 000000000..15245b426 --- /dev/null +++ b/QuickFiler.Test/Controllers/QfcExplorerController.ConversationViewTests.cs @@ -0,0 +1,205 @@ +using System.Collections; +using System.Collections.Generic; +using FluentAssertions; +using Microsoft.Office.Interop.Outlook; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using Outlook = Microsoft.Office.Interop.Outlook; + +namespace QuickFiler.Controllers.Tests +{ + /// + /// Conversation-view coverage for : the + /// ExplConvView_ToggleOn and ExplConvView_ToggleOff state transitions and the + /// GetSiblingView lookup. Second part of the partial class declared in + /// QfcExplorerControllerTests.cs, split to keep both files under the 500-line limit. + /// [TestClass] stays on the base file only: it is AllowMultiple = false, so + /// repeating it here would be CS0579. Shares the base file's Setup fixture, + /// CreateController, and ArrangeViewsIndexer helpers. Deterministic — mocked COM + /// only, no live form, no message pump, no sleeps, no temporary files. + /// + public partial class QfcExplorerControllerTests + { + /// + /// ExplConvView_ToggleOn applies the remembered view and clears the flag when + /// BlShowInConversations is set. + /// + [TestMethod] + public void ExplConvView_ToggleOn_WhenFlagSet_AppliesRememberedView() + { + // Arrange + var rememberedView = _repo.Create(); + var views = ArrangeViewsIndexer(rememberedView); + var currentFolder = _repo.Create(); + currentFolder.SetupGet(f => f.Views).Returns(views.Object); + _explorer.SetupGet(e => e.CurrentFolder).Returns(currentFolder.Object); + + var controller = CreateController(); + controller.BlShowInConversations = true; + + // Act + controller.ExplConvView_ToggleOn(); + + // Assert + rememberedView.Verify(v => v.Apply(), Times.Once()); + controller + .BlShowInConversations.Should() + .BeFalse("applying the remembered view completes the toggle-on transition"); + } + + /// + /// ExplConvView_ToggleOn is a no-op when BlShowInConversations is clear: the + /// negative branch must not touch the Views collection or apply any view. + /// + [TestMethod] + public void ExplConvView_ToggleOn_WhenFlagClear_DoesNothing() + { + // Arrange + var rememberedView = _repo.Create(); + var views = ArrangeViewsIndexer(rememberedView); + var currentFolder = _repo.Create(); + currentFolder.SetupGet(f => f.Views).Returns(views.Object); + _explorer.SetupGet(e => e.CurrentFolder).Returns(currentFolder.Object); + + var controller = CreateController(); + controller.BlShowInConversations = false; + + // Act + controller.ExplConvView_ToggleOn(); + + // Assert + views.Verify(v => v[It.IsAny()], Times.Never()); + rememberedView.Verify(v => v.Apply(), Times.Never()); + controller.BlShowInConversations.Should().BeFalse(); + } + + /// + /// ExplConvView_ToggleOff is a no-op when conversations are not grouped: the guard reads + /// the command-bar state and returns without touching the current view. + /// + [TestMethod] + public void ExplConvView_ToggleOff_WhenConversationsNotGrouped_DoesNothing() + { + // Arrange — the fixture already arranges GetPressedMso to return false. + var controller = CreateController(); + controller.BlShowInConversations = false; + + // Act + controller.ExplConvView_ToggleOff(); + + // Assert + controller + .BlShowInConversations.Should() + .BeFalse("the negative guard must leave the flag untouched"); + _explorer.VerifyGet(e => e.CurrentView, Times.Never()); + } + + /// + /// ExplConvView_ToggleOff copies the current view to a temporary conversation-free view, + /// strips the upgrade-to-conversation directive from its XML, saves it, and applies it, when no + /// sibling view of that name already exists. + /// + [TestMethod] + public void ExplConvView_ToggleOff_WhenSiblingViewMissing_CopiesAndSavesTemporaryView() + { + // Arrange — conversations ARE grouped, so the branch is entered. + _commandBars.Setup(c => c.GetPressedMso(ShowInConversationsMso)).Returns(true); + + // A sibling collection with no view named tmpNoConversation, so GetSiblingView returns null. + var siblings = _repo.Create(); + siblings + .Setup(v => v.GetEnumerator()) + .Returns(() => ((IEnumerable)new List()).GetEnumerator()); + + var currentView = _repo.Create(); + currentView.SetupGet(v => v.Name).Returns("Compact"); + currentView.SetupGet(v => v.Parent).Returns(siblings.Object); + currentView + .SetupGet(v => v.XML) + .Returns("1"); + _explorer.SetupGet(e => e.CurrentView).Returns(currentView.Object); + + var temporaryView = _repo.Create(); + currentView + .Setup(v => + v.Copy(TemporaryViewName, OlViewSaveOption.olViewSaveOptionThisFolderOnlyMe) + ) + .Returns(temporaryView.Object); + + var controller = CreateController(); + + // Act + controller.ExplConvView_ToggleOff(); + + // Assert — copy, XML assignment with the directive stripped, save, then apply. + currentView.Verify( + v => v.Copy(TemporaryViewName, OlViewSaveOption.olViewSaveOptionThisFolderOnlyMe), + Times.Once() + ); + temporaryView.VerifySet(v => v.XML = "", Times.Once()); + temporaryView.Verify(v => v.Save(), Times.Once()); + temporaryView.Verify(v => v.Apply(), Times.Once()); + controller + .BlShowInConversations.Should() + .BeTrue("toggling off records that conversations were grouped"); + } + + /// + /// GetSiblingView returns the sibling whose name matches the requested view name. + /// + [TestMethod] + public void GetSiblingView_WhenNamedViewPresent_ReturnsIt() + { + // Arrange + var other = _repo.Create(); + other.SetupGet(v => v.Name).Returns("Compact"); + var wanted = _repo.Create(); + wanted.SetupGet(v => v.Name).Returns(TemporaryViewName); + + var siblings = _repo.Create(); + var list = new List { other.Object, wanted.Object }; + siblings + .Setup(v => v.GetEnumerator()) + .Returns(() => ((IEnumerable)list).GetEnumerator()); + + var currentView = _repo.Create(); + currentView.SetupGet(v => v.Parent).Returns(siblings.Object); + + var controller = CreateController(); + + // Act + Outlook.View result = controller.GetSiblingView(currentView.Object, TemporaryViewName); + + // Assert + result.Should().BeSameAs(wanted.Object); + } + + /// + /// GetSiblingView returns null when the loop is exhausted without a name match. + /// + [TestMethod] + public void GetSiblingView_WhenNamedViewAbsent_ReturnsNull() + { + // Arrange + var other = _repo.Create(); + other.SetupGet(v => v.Name).Returns("Compact"); + + var siblings = _repo.Create(); + var list = new List { other.Object }; + siblings + .Setup(v => v.GetEnumerator()) + .Returns(() => ((IEnumerable)list).GetEnumerator()); + + var currentView = _repo.Create(); + currentView.SetupGet(v => v.Parent).Returns(siblings.Object); + + var controller = CreateController(); + + // Act + Outlook.View result = controller.GetSiblingView(currentView.Object, TemporaryViewName); + + // Assert + result.Should().BeNull(); + } + } +} diff --git a/QuickFiler.Test/Controllers/QfcExplorerControllerTests.cs b/QuickFiler.Test/Controllers/QfcExplorerControllerTests.cs new file mode 100644 index 000000000..ec5ee0920 --- /dev/null +++ b/QuickFiler.Test/Controllers/QfcExplorerControllerTests.cs @@ -0,0 +1,387 @@ +using System.Collections; +using System.Collections.Generic; +using System.Threading.Tasks; +using System.Windows.Forms; +using FluentAssertions; +using Microsoft.Office.Interop.Outlook; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using QuickFiler.Interfaces; +using UtilitiesCS; +using Outlook = Microsoft.Office.Interop.Outlook; + +namespace QuickFiler.Controllers.Tests +{ + /// + /// Unit tests for . Covers the defect-2 regression (the + /// controller must act on the explorer it captured at construction, not one re-resolved later), + /// the injectable modal-dialog seam, and the conversation-view state machine. + /// + /// + /// Branch-control detail that governs every arrangement in this class: the fixture constructs the + /// controller with QfEnums.InitTypeEnum.Find (value 2, per + /// QuickFiler/Helper Classes/QfEnums.cs:8), which makes + /// _initType.HasFlag(QfEnums.InitTypeEnum.Sort) false in OpenQFItem. That does NOT + /// make the CommandBars setup optional. Both conjunctions in OpenQFItem use the + /// NON-SHORT-CIRCUITING & operator rather than &&, so + /// AutoFile.AreConversationsGrouped(_activeExplorer) is still evaluated even when the + /// left operand is false, and that helper reads + /// ActiveExplorer.CommandBars.GetPressedMso("ShowInConversations"). The + /// CommandBars mock setup is therefore MANDATORY on every explorer the controller may + /// consult, not optional. + /// + [TestClass] + public partial class QfcExplorerControllerTests + { + private const string ShowInConversationsMso = "ShowInConversations"; + private const string TemporaryViewName = "tmpNoConversation"; + private const string WideViewName = "Wide"; + + private MockRepository _repo; + private Mock _commandBars; + private Mock _explorer; + private Mock _olApp; + private Mock _globals; + private Mock _formController; + private Mock _parent; + + /// + /// Builds the shared mock graph the constructor requires. The constructor reaches COM at + /// exactly one point, _globals.Ol.App.ActiveExplorer(), so the chain + /// IApplicationGlobals -> IOlObjects.App -> Application.ActiveExplorer() is the + /// whole of the construction dependency. is used so that an + /// unexpected member access surfaces as an assertion failure rather than as a Moq + /// strict-mode exception, which keeps pre-fix failures readable. + /// + [TestInitialize] + public void Setup() + { + _repo = new MockRepository(MockBehavior.Loose); + + _commandBars = _repo.Create(); + _commandBars.Setup(c => c.GetPressedMso(ShowInConversationsMso)).Returns(false); + + _explorer = _repo.Create(); + _explorer.Setup(e => e.CommandBars).Returns(_commandBars.Object); + + _olApp = _repo.Create(); + _olApp.Setup(a => a.ActiveExplorer()).Returns(_explorer.Object); + + _globals = _repo.Create(); + _globals.SetupGet(g => g.Ol.App).Returns(_olApp.Object); + _globals.SetupGet(g => g.Ol.ViewWide).Returns(WideViewName); + + _formController = _repo.Create(); + _parent = _repo.Create(); + _parent.SetupGet(p => p.FormController).Returns(_formController.Object); + } + + /// + /// Creates the system under test against the shared fixture graph. + /// + private QfcExplorerController CreateController() + { + return new QfcExplorerController( + QfEnums.InitTypeEnum.Find, + _globals.Object, + _parent.Object + ); + } + + /// + /// Creates a mock folder whose FolderPath is the supplied value. + /// + private Mock CreateFolder(string folderPath) + { + var folder = _repo.Create(); + folder.SetupGet(f => f.FolderPath).Returns(folderPath); + return folder; + } + + /// + /// Creates a mock mail item whose Parent is the supplied folder. + /// + private Mock CreateMailItem(Mock parentFolder) + { + var mailItem = _repo.Create(); + mailItem.SetupGet(m => m.Parent).Returns(parentFolder.Object); + return mailItem; + } + + /// + /// Arranges the Views collection reached by ExplConvView_ToggleOn through + /// _activeExplorer.CurrentFolder.Views[_objViewMem], returning the supplied view for any + /// index. + /// + /// + /// The PIA declares the Views indexer parameter as object, so the setup uses + /// It.IsAny<object>(). The indexer-mocking form follows the in-repo precedent at + /// QuickFiler.Test/Helper Classes/MailItemInfoTests.cs:64-65. + /// + private Mock ArrangeViewsIndexer(Mock view) + { + var views = _repo.Create(); + views.Setup(v => v[It.IsAny()]).Returns(view.Object); + return views; + } + + /// + /// Defect-2 regression. OpenQFItem reaches the private helper + /// NavigateToOutlookFolder, which must assign the destination folder to the explorer + /// the controller CAPTURED at construction, not to one it re-resolves from + /// _globals.Ol.App.ActiveExplorer() at call time. When the active explorer has changed + /// between construction and the call, re-resolving navigates the wrong window. + /// + /// + /// The two explorers are made distinguishable by sequencing ActiveExplorer(): the first + /// call is consumed by the constructor and yields the captured explorer, the second yields a + /// drifted explorer that only a re-resolution could reach. Before the fix the assignment + /// lands on the drifted explorer and BOTH assertions fail; after the fix it lands on the + /// captured explorer and both pass. + /// + [TestMethod] + public async Task OpenQFItem_WhenActiveExplorerChangesAfterConstruction_UsesTheConstructorCapturedExplorer() + { + // Arrange — a drifted explorer that is reachable ONLY by re-resolving ActiveExplorer(). + // MockBehavior.Loose is deliberate: it lets a pre-fix assignment land harmlessly so the + // failure surfaces as a FluentAssertions/Moq verification message rather than as a + // strict-mode exception thrown from inside production code. + var driftedCommandBars = _repo.Create(); + driftedCommandBars.Setup(c => c.GetPressedMso(ShowInConversationsMso)).Returns(false); + var driftedExplorer = _repo.Create(); + driftedExplorer.Setup(e => e.CommandBars).Returns(driftedCommandBars.Object); + + // First call is consumed by the constructor at QfcExplorerController.cs:35. + // Second call is what the pre-fix line 140 resolves. + _olApp + .SetupSequence(a => a.ActiveExplorer()) + .Returns(_explorer.Object) + .Returns(driftedExplorer.Object); + + // Arrange the guard so the navigation branch is ENTERED: the captured explorer's current + // folder path must differ from the mail item's parent folder path. + var currentFolder = CreateFolder(@"\\Mailbox\A"); + var destination = CreateFolder(@"\\Mailbox\B"); + _explorer.SetupGet(e => e.CurrentFolder).Returns(currentFolder.Object); + var mailItem = CreateMailItem(destination); + + // Keep the not-in-view dialog branch unreachable; this test is about navigation only. + _explorer.Setup(e => e.IsItemSelectableInView(It.IsAny())).Returns(true); + + var controller = CreateController(); + + // Act + await controller.OpenQFItem(mailItem.Object); + + // Assert — the destination is assigned to the CAPTURED explorer exactly once ... + _explorer.VerifySet(e => e.CurrentFolder = destination.Object, Times.Once()); + + // ... and the drifted explorer is never navigated at all. + driftedExplorer.VerifySet(e => e.CurrentFolder = It.IsAny(), Times.Never()); + } + + /// + /// Arranges the not-in-view branch of OpenQFItem: the mail item is not selectable in + /// the current view, and the mail item's parent folder matches the current folder so the + /// navigation branch is skipped and the test isolates the dialog path. + /// + private Mock ArrangeNotInViewBranch() + { + var sameFolder = CreateFolder(@"\\Mailbox\A"); + _explorer.SetupGet(e => e.CurrentFolder).Returns(sameFolder.Object); + var mailItem = CreateMailItem(sameFolder); + _explorer.Setup(e => e.IsItemSelectableInView(It.IsAny())).Returns(false); + return mailItem; + } + + /// + /// The not-in-view branch must route its modal prompt through the injectable + /// NotInViewDialogInvoker seam exactly once, passing the unchanged user-visible text, + /// caption, buttons, and icon. The seam's production default is never exercised, so no dialog + /// is displayed and no message pump is required. + /// + [TestMethod] + public async Task OpenQFItem_WhenItemNotSelectableInView_InvokesDialogSeamOnce() + { + // Arrange + var mailItem = ArrangeNotInViewBranch(); + var controller = CreateController(); + + var invocationCount = 0; + string observedText = null; + string observedCaption = null; + MessageBoxButtons observedButtons = default; + MessageBoxIcon observedIcon = default; + + controller.NotInViewDialogInvoker = (text, caption, buttons, icon) => + { + invocationCount++; + observedText = text; + observedCaption = caption; + observedButtons = buttons; + observedIcon = icon; + return DialogResult.No; + }; + + // Act + await controller.OpenQFItem(mailItem.Object); + + // Assert + invocationCount + .Should() + .Be(1, "the not-in-view branch must consult the dialog seam exactly once"); + observedText.Should().Be("Selected message is not in view. Would you like to open it?"); + observedCaption.Should().Be("Error"); + observedButtons.Should().Be(MessageBoxButtons.YesNo); + observedIcon.Should().Be(MessageBoxIcon.Error); + } + + /// + /// When the user answers Yes to the not-in-view prompt, the mail item is displayed. + /// + [TestMethod] + public async Task OpenQFItem_WhenDialogSeamReturnsYes_DisplaysMailItem() + { + // Arrange + var mailItem = ArrangeNotInViewBranch(); + var controller = CreateController(); + controller.NotInViewDialogInvoker = (text, caption, buttons, icon) => DialogResult.Yes; + + // Act + await controller.OpenQFItem(mailItem.Object); + + // Assert + mailItem.Verify(m => m.Display(It.IsAny()), Times.Once()); + } + + /// + /// When the user answers No to the not-in-view prompt, the mail item is not displayed. + /// + [TestMethod] + public async Task OpenQFItem_WhenDialogSeamReturnsNo_DoesNotDisplayMailItem() + { + // Arrange + var mailItem = ArrangeNotInViewBranch(); + var controller = CreateController(); + controller.NotInViewDialogInvoker = (text, caption, buttons, icon) => DialogResult.No; + + // Act + await controller.OpenQFItem(mailItem.Object); + + // Assert + mailItem.Verify(m => m.Display(It.IsAny()), Times.Never()); + } + + /// + /// Characterises the defect-2 guard at NavigateToOutlookFolder: when the mail item's + /// parent folder path already equals the explorer's current folder path there is nothing to + /// navigate to, so no explorer's CurrentFolder may be assigned. + /// + [TestMethod] + public async Task OpenQFItem_WhenMailIsAlreadyInTheCurrentFolder_DoesNotChangeCurrentFolder() + { + // Arrange — a drifted explorer, so the assertion also proves no re-resolution occurred. + var driftedCommandBars = _repo.Create(); + driftedCommandBars.Setup(c => c.GetPressedMso(ShowInConversationsMso)).Returns(false); + var driftedExplorer = _repo.Create(); + driftedExplorer.Setup(e => e.CommandBars).Returns(driftedCommandBars.Object); + _olApp + .SetupSequence(a => a.ActiveExplorer()) + .Returns(_explorer.Object) + .Returns(driftedExplorer.Object); + + // The SAME folder object on both sides, so the guard's inequality is false. + var sameFolder = CreateFolder(@"\\Mailbox\A"); + _explorer.SetupGet(e => e.CurrentFolder).Returns(sameFolder.Object); + var mailItem = CreateMailItem(sameFolder); + _explorer.Setup(e => e.IsItemSelectableInView(It.IsAny())).Returns(true); + + var controller = CreateController(); + + // Act + await controller.OpenQFItem(mailItem.Object); + + // Assert — neither explorer is navigated. + _explorer.VerifySet(e => e.CurrentFolder = It.IsAny(), Times.Never()); + driftedExplorer.VerifySet(e => e.CurrentFolder = It.IsAny(), Times.Never()); + } + + /// + /// Characterises the in-view positive path of OpenQFItem: when the item is selectable in + /// the current view the selection is cleared and the item added, and no dialog is consulted. + /// + [TestMethod] + public async Task OpenQFItem_WhenItemIsSelectableInView_ClearsAndAddsSelection() + { + // Arrange + var sameFolder = CreateFolder(@"\\Mailbox\A"); + _explorer.SetupGet(e => e.CurrentFolder).Returns(sameFolder.Object); + var mailItem = CreateMailItem(sameFolder); + _explorer.Setup(e => e.IsItemSelectableInView(It.IsAny())).Returns(true); + + var controller = CreateController(); + var dialogInvocations = 0; + controller.NotInViewDialogInvoker = (text, caption, buttons, icon) => + { + dialogInvocations++; + return DialogResult.No; + }; + + // Act + await controller.OpenQFItem(mailItem.Object); + + // Assert + _explorer.Verify(e => e.ClearSelection(), Times.Once()); + _explorer.Verify(e => e.AddToSelection(mailItem.Object), Times.Once()); + dialogInvocations + .Should() + .Be(0, "the in-view path must not consult the not-in-view dialog seam"); + } + + /// + /// The internal CurrentConversationState property is a direct projection of the + /// command-bar pressed state, reached from the test assembly through + /// [assembly: InternalsVisibleTo("QuickFiler.Test")]. + /// + [DataTestMethod] + [DataRow(true)] + [DataRow(false)] + public void CurrentConversationState_ReflectsCommandBarPressedState(bool pressed) + { + // Arrange + _commandBars.Setup(c => c.GetPressedMso(ShowInConversationsMso)).Returns(pressed); + var controller = CreateController(); + + // Act + bool actual = controller.CurrentConversationState; + + // Assert + actual.Should().Be(pressed); + } + + /// + /// ExplConvView_ReturnState delegates to ExplConvView_ToggleOn when the flag is + /// set, which is observable as the remembered view being applied and the flag being cleared. + /// + [TestMethod] + public void ExplConvView_ReturnState_WhenFlagSet_TogglesOn() + { + // Arrange + var rememberedView = _repo.Create(); + var views = ArrangeViewsIndexer(rememberedView); + var currentFolder = _repo.Create(); + currentFolder.SetupGet(f => f.Views).Returns(views.Object); + _explorer.SetupGet(e => e.CurrentFolder).Returns(currentFolder.Object); + + var controller = CreateController(); + controller.BlShowInConversations = true; + + // Act + controller.ExplConvView_ReturnState(); + + // Assert + rememberedView.Verify(v => v.Apply(), Times.Once()); + controller.BlShowInConversations.Should().BeFalse(); + } + } +} diff --git a/QuickFiler.Test/Controllers/QfcHomeControllerTests.cs b/QuickFiler.Test/Controllers/QfcHomeControllerTests.cs index 3de71edbb..763ccd806 100644 --- a/QuickFiler.Test/Controllers/QfcHomeControllerTests.cs +++ b/QuickFiler.Test/Controllers/QfcHomeControllerTests.cs @@ -240,17 +240,6 @@ await _controller.InitAsync( ); } - public class QfcFormViewerDerived : QfcFormViewer - { - public QfcFormViewerDerived() - : base() { } - - public new virtual void Show() => base.Show(); - - //public new virtual DialogResult ShowDialog() => base.ShowDialog(); - public new virtual FormWindowState WindowState { get; set; } - } - //[TestMethod] //public void QuickFileMetrics_WRITE_ExecutesCorrectly() //{ diff --git a/QuickFiler.Test/Form1.Designer.cs b/QuickFiler.Test/Form1.Designer.cs deleted file mode 100644 index f8dbbf41d..000000000 --- a/QuickFiler.Test/Form1.Designer.cs +++ /dev/null @@ -1,227 +0,0 @@ -namespace QuickFiler.Test -{ - public partial class Form1 : System.Windows.Forms.Form - { - /// - /// Required designer variable. - /// - private System.ComponentModel.IContainer components = null; - - /// - /// Clean up any resources being used. - /// - /// true if managed resources should be disposed; otherwise, false. - protected override void Dispose(bool disposing) - { - if (disposing && (components != null)) - { - components.Dispose(); - } - base.Dispose(disposing); - } - - #region Windows Form Designer generated code - - /// - /// Required method for Designer support - do not modify - /// the contents of this method with the code editor. - /// - private void InitializeComponent() - { - this.TableLayoutPanel1 = new System.Windows.Forms.TableLayoutPanel(); - this.ControlGroup3 = new QuickFiler.ItemViewer(); - this.ControlGroup1 = new QuickFiler.ItemViewer(); - this.ControlGroup2 = new QuickFiler.ItemViewer(); - this.Button2 = new System.Windows.Forms.Button(); - this.Button1 = new System.Windows.Forms.Button(); - this.MainPanel = new System.Windows.Forms.Panel(); - this.MainLayoutPanel = new System.Windows.Forms.TableLayoutPanel(); - this.TableLayoutPanel2 = new System.Windows.Forms.TableLayoutPanel(); - this.ButtonAdd = new System.Windows.Forms.Button(); - this.TableLayoutPanel1.SuspendLayout(); - this.MainPanel.SuspendLayout(); - this.MainLayoutPanel.SuspendLayout(); - this.TableLayoutPanel2.SuspendLayout(); - this.SuspendLayout(); - // - // TableLayoutPanel1 - // - this.TableLayoutPanel1.AutoSize = true; - this.TableLayoutPanel1.AutoSizeMode = System.Windows.Forms.AutoSizeMode.GrowAndShrink; - this.TableLayoutPanel1.ColumnCount = 1; - this.TableLayoutPanel1.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Percent, 100F)); - this.TableLayoutPanel1.Controls.Add(this.ControlGroup3, 0, 1); - this.TableLayoutPanel1.Controls.Add(this.ControlGroup1, 0, 0); - this.TableLayoutPanel1.Controls.Add(this.ControlGroup2, 0, 2); - this.TableLayoutPanel1.Dock = System.Windows.Forms.DockStyle.Top; - this.TableLayoutPanel1.Location = new System.Drawing.Point(0, 0); - this.TableLayoutPanel1.Margin = new System.Windows.Forms.Padding(0); - this.TableLayoutPanel1.Name = "TableLayoutPanel1"; - this.TableLayoutPanel1.Padding = new System.Windows.Forms.Padding(20, 19, 20, 19); - this.TableLayoutPanel1.RowCount = 4; - this.TableLayoutPanel1.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 212F)); - this.TableLayoutPanel1.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 212F)); - this.TableLayoutPanel1.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 212F)); - this.TableLayoutPanel1.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Percent, 100F)); - this.TableLayoutPanel1.Size = new System.Drawing.Size(2328, 674); - this.TableLayoutPanel1.TabIndex = 0; - // - // ControlGroup3 - // - this.ControlGroup3.AutoSize = true; - this.ControlGroup3.AutoSizeMode = System.Windows.Forms.AutoSizeMode.GrowAndShrink; - this.ControlGroup3.BorderStyle = System.Windows.Forms.BorderStyle.FixedSingle; - this.ControlGroup3.Dock = System.Windows.Forms.DockStyle.Fill; - this.ControlGroup3.Location = new System.Drawing.Point(32, 243); - this.ControlGroup3.Margin = new System.Windows.Forms.Padding(12); - this.ControlGroup3.MinimumSize = new System.Drawing.Size(3722, 350); - this.ControlGroup3.Name = "ControlGroup3"; - this.ControlGroup3.Padding = new System.Windows.Forms.Padding(6); - this.ControlGroup3.Size = new System.Drawing.Size(3722, 350); - this.ControlGroup3.TabIndex = 2; - // - // ControlGroup1 - // - this.ControlGroup1.AutoSize = true; - this.ControlGroup1.AutoSizeMode = System.Windows.Forms.AutoSizeMode.GrowAndShrink; - this.ControlGroup1.BorderStyle = System.Windows.Forms.BorderStyle.FixedSingle; - this.ControlGroup1.Dock = System.Windows.Forms.DockStyle.Fill; - this.ControlGroup1.Location = new System.Drawing.Point(32, 31); - this.ControlGroup1.Margin = new System.Windows.Forms.Padding(12); - this.ControlGroup1.MinimumSize = new System.Drawing.Size(3722, 350); - this.ControlGroup1.Name = "ControlGroup1"; - this.ControlGroup1.Padding = new System.Windows.Forms.Padding(6); - this.ControlGroup1.Size = new System.Drawing.Size(3722, 350); - this.ControlGroup1.TabIndex = 0; - // - // ControlGroup2 - // - this.ControlGroup2.AutoSize = true; - this.ControlGroup2.AutoSizeMode = System.Windows.Forms.AutoSizeMode.GrowAndShrink; - this.ControlGroup2.BorderStyle = System.Windows.Forms.BorderStyle.FixedSingle; - this.ControlGroup2.Dock = System.Windows.Forms.DockStyle.Fill; - this.ControlGroup2.Location = new System.Drawing.Point(32, 455); - this.ControlGroup2.Margin = new System.Windows.Forms.Padding(12); - this.ControlGroup2.MinimumSize = new System.Drawing.Size(3722, 350); - this.ControlGroup2.Name = "ControlGroup2"; - this.ControlGroup2.Padding = new System.Windows.Forms.Padding(6); - this.ControlGroup2.Size = new System.Drawing.Size(3722, 350); - this.ControlGroup2.TabIndex = 1; - // - // Button2 - // - this.Button2.Location = new System.Drawing.Point(1194, 6); - this.Button2.Margin = new System.Windows.Forms.Padding(6); - this.Button2.Name = "Button2"; - this.Button2.Padding = new System.Windows.Forms.Padding(12); - this.Button2.Size = new System.Drawing.Size(182, 67); - this.Button2.TabIndex = 3; - this.Button2.Text = "Toggle"; - this.Button2.UseVisualStyleBackColor = true; - this.Button2.Click += new System.EventHandler(this.Button2_Click); - // - // Button1 - // - this.Button1.Location = new System.Drawing.Point(994, 6); - this.Button1.Margin = new System.Windows.Forms.Padding(6); - this.Button1.Name = "Button1"; - this.Button1.Size = new System.Drawing.Size(182, 67); - this.Button1.TabIndex = 2; - this.Button1.Text = "OK"; - this.Button1.UseVisualStyleBackColor = true; - this.Button1.Click += new System.EventHandler(this.Button1_Click); - // - // MainPanel - // - this.MainPanel.AutoScroll = true; - this.MainPanel.BorderStyle = System.Windows.Forms.BorderStyle.FixedSingle; - this.MainPanel.Controls.Add(this.TableLayoutPanel1); - this.MainPanel.Dock = System.Windows.Forms.DockStyle.Fill; - this.MainPanel.Location = new System.Drawing.Point(6, 6); - this.MainPanel.Margin = new System.Windows.Forms.Padding(6); - this.MainPanel.Name = "MainPanel"; - this.MainPanel.Size = new System.Drawing.Size(2364, 584); - this.MainPanel.TabIndex = 1; - // - // MainLayoutPanel - // - this.MainLayoutPanel.ColumnCount = 1; - this.MainLayoutPanel.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Percent, 100F)); - this.MainLayoutPanel.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 40F)); - this.MainLayoutPanel.Controls.Add(this.MainPanel, 0, 0); - this.MainLayoutPanel.Controls.Add(this.TableLayoutPanel2, 0, 1); - this.MainLayoutPanel.Dock = System.Windows.Forms.DockStyle.Fill; - this.MainLayoutPanel.Location = new System.Drawing.Point(0, 0); - this.MainLayoutPanel.Margin = new System.Windows.Forms.Padding(6); - this.MainLayoutPanel.Name = "MainLayoutPanel"; - this.MainLayoutPanel.RowCount = 2; - this.MainLayoutPanel.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Percent, 100F)); - this.MainLayoutPanel.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 123F)); - this.MainLayoutPanel.Size = new System.Drawing.Size(2376, 719); - this.MainLayoutPanel.TabIndex = 2; - // - // TableLayoutPanel2 - // - this.TableLayoutPanel2.ColumnCount = 4; - this.TableLayoutPanel2.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Percent, 50F)); - this.TableLayoutPanel2.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 200F)); - this.TableLayoutPanel2.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 200F)); - this.TableLayoutPanel2.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Percent, 50F)); - this.TableLayoutPanel2.Controls.Add(this.ButtonAdd, 0, 0); - this.TableLayoutPanel2.Controls.Add(this.Button2, 2, 0); - this.TableLayoutPanel2.Controls.Add(this.Button1, 1, 0); - this.TableLayoutPanel2.Dock = System.Windows.Forms.DockStyle.Fill; - this.TableLayoutPanel2.Location = new System.Drawing.Point(0, 596); - this.TableLayoutPanel2.Margin = new System.Windows.Forms.Padding(0); - this.TableLayoutPanel2.Name = "TableLayoutPanel2"; - this.TableLayoutPanel2.RowCount = 1; - this.TableLayoutPanel2.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Percent, 100F)); - this.TableLayoutPanel2.Size = new System.Drawing.Size(2376, 123); - this.TableLayoutPanel2.TabIndex = 2; - // - // ButtonAdd - // - this.ButtonAdd.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Top | System.Windows.Forms.AnchorStyles.Right))); - this.ButtonAdd.Location = new System.Drawing.Point(760, 6); - this.ButtonAdd.Margin = new System.Windows.Forms.Padding(6); - this.ButtonAdd.Name = "ButtonAdd"; - this.ButtonAdd.Padding = new System.Windows.Forms.Padding(12); - this.ButtonAdd.Size = new System.Drawing.Size(222, 94); - this.ButtonAdd.TabIndex = 4; - this.ButtonAdd.Text = "Add Control Group"; - this.ButtonAdd.UseVisualStyleBackColor = true; - this.ButtonAdd.Click += new System.EventHandler(this.ButtonAdd_Click); - // - // Form1 - // - this.AutoScaleDimensions = new System.Drawing.SizeF(12F, 25F); - this.AutoScaleMode = System.Windows.Forms.AutoScaleMode.Font; - this.ClientSize = new System.Drawing.Size(2376, 719); - this.Controls.Add(this.MainLayoutPanel); - this.Margin = new System.Windows.Forms.Padding(6); - this.Name = "Form1"; - this.Text = "Form1"; - this.TableLayoutPanel1.ResumeLayout(false); - this.TableLayoutPanel1.PerformLayout(); - this.MainPanel.ResumeLayout(false); - this.MainPanel.PerformLayout(); - this.MainLayoutPanel.ResumeLayout(false); - this.TableLayoutPanel2.ResumeLayout(false); - this.ResumeLayout(false); - - } - - #endregion - - internal System.Windows.Forms.TableLayoutPanel TableLayoutPanel1; - internal ItemViewer ControlGroup1; - internal System.Windows.Forms.Button Button2; - internal System.Windows.Forms.Button Button1; - internal ItemViewer ControlGroup3; - internal ItemViewer ControlGroup2; - internal System.Windows.Forms.Panel MainPanel; - internal System.Windows.Forms.TableLayoutPanel MainLayoutPanel; - internal System.Windows.Forms.TableLayoutPanel TableLayoutPanel2; - internal System.Windows.Forms.Button ButtonAdd; - } -} \ No newline at end of file diff --git a/QuickFiler.Test/Form1.cs b/QuickFiler.Test/Form1.cs deleted file mode 100644 index 2ce9db81b..000000000 --- a/QuickFiler.Test/Form1.cs +++ /dev/null @@ -1,49 +0,0 @@ -using System; - -namespace QuickFiler.Test -{ - public partial class Form1 - { - public Form1() - { - InitializeComponent(); - } - - private void Button1_Click(object sender, EventArgs e) - { - Dispose(); - } - - private void Button2_Click(object sender, EventArgs e) - { - //ControlGroup1.ToggleAccelerator(); - } - - private void LoadControlGroup() - { - var _controlGroup = new ItemViewer(); - TableLayoutPanel1.SuspendLayout(); - TableLayoutPanel1.RowCount += 1; - TableLayoutPanel1.RowStyles.Insert( - TableLayoutPanel1.RowCount - 2, - new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 110.0f) - ); - TableLayoutPanel1.Controls.Add(_controlGroup, 0, TableLayoutPanel1.RowCount - 2); - SetControlGroupOptions(_controlGroup); - TableLayoutPanel1.ResumeLayout(true); - } - - private void SetControlGroupOptions(ItemViewer group) - { - group.AutoSize = true; - group.AutoSizeMode = System.Windows.Forms.AutoSizeMode.GrowAndShrink; - group.Dock = System.Windows.Forms.DockStyle.Fill; - group.Padding = new System.Windows.Forms.Padding(3); - } - - private void ButtonAdd_Click(object sender, EventArgs e) - { - LoadControlGroup(); - } - } -} diff --git a/QuickFiler.Test/Form1.resx b/QuickFiler.Test/Form1.resx deleted file mode 100644 index 1af7de150..000000000 --- a/QuickFiler.Test/Form1.resx +++ /dev/null @@ -1,120 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - text/microsoft-resx - - - 2.0 - - - System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - - System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - \ No newline at end of file diff --git a/QuickFiler.Test/NoLiveFormInTestAssemblyTests.cs b/QuickFiler.Test/NoLiveFormInTestAssemblyTests.cs new file mode 100644 index 000000000..27130a8f2 --- /dev/null +++ b/QuickFiler.Test/NoLiveFormInTestAssemblyTests.cs @@ -0,0 +1,54 @@ +using System; +using System.Linq; +using System.Reflection; +using FluentAssertions; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace QuickFiler.Test +{ + /// + /// Structural guard: no live WinForms window type may be compiled into this unit-test + /// assembly. Reflection is over type metadata only; nothing is instantiated. + /// + [TestClass] + public class NoLiveFormInTestAssemblyTests + { + [TestMethod] + public void ExecutingAssembly_ContainsNoFormDerivedType() + { + // Arrange - metadata only; scoped to the executing assembly, never a referenced one. + Type formType = typeof(System.Windows.Forms.Form); + Assembly executing = Assembly.GetExecutingAssembly(); + + // Act + string[] formDerivedTypeNames = GetLoadableTypes(executing) + .Where(candidate => formType.IsAssignableFrom(candidate)) + .Select(candidate => candidate.FullName) + .OrderBy(name => name, StringComparer.Ordinal) + .ToArray(); + + // Assert + formDerivedTypeNames + .Should() + .BeEmpty( + "a unit-test assembly must not compile a live System.Windows.Forms.Form type" + ); + } + + // Reflection over a large test assembly can hit a single type whose dependencies fail to + // resolve, and GetTypes then throws for the whole assembly. That would leave this guard + // permanently red for a reason unrelated to what it measures, so the loaded subset carried + // on the exception is used instead; its null entries are the types that did not load. + private static Type[] GetLoadableTypes(Assembly assembly) + { + try + { + return assembly.GetTypes(); + } + catch (ReflectionTypeLoadException ex) + { + return ex.Types.Where(candidate => candidate != null).ToArray(); + } + } + } +} diff --git a/QuickFiler.Test/QuickFiler.Test.csproj b/QuickFiler.Test/QuickFiler.Test.csproj index 13e522e10..5f4dddb28 100644 --- a/QuickFiler.Test/QuickFiler.Test.csproj +++ b/QuickFiler.Test/QuickFiler.Test.csproj @@ -117,6 +117,8 @@ + + @@ -158,12 +160,7 @@ - - Form - - - Form1.cs - + @@ -176,11 +173,6 @@ - - - Form1.cs - - diff --git a/QuickFiler/Controllers/KaChar.cs b/QuickFiler/Controllers/KaChar.cs index 2fc040fb4..dfe197733 100644 --- a/QuickFiler/Controllers/KaChar.cs +++ b/QuickFiler/Controllers/KaChar.cs @@ -3,7 +3,6 @@ using System.Linq; using System.Text; using System.Threading.Tasks; -using System.Windows.Forms; using QuickFiler.Interfaces; namespace QuickFiler.Controllers @@ -40,19 +39,7 @@ public Action Delegate set => _action = value; } - public Type DelegateType - { - get => typeof(Action); - } - public bool KeyEquals(char other) => Key == other; - - private Action _update; - public Action Update - { - get => _update; - set => _update = value; - } } public class KaCharAsync : IKbdAction> @@ -88,12 +75,5 @@ public Func Delegate } public bool KeyEquals(char other) => Key == other; - - private Action _update; - public Action Update - { - get => _update; - set => _update = value; - } } } diff --git a/QuickFiler/Controllers/KaKey.cs b/QuickFiler/Controllers/KaKey.cs index 3b7498901..76440ffbb 100644 --- a/QuickFiler/Controllers/KaKey.cs +++ b/QuickFiler/Controllers/KaKey.cs @@ -40,19 +40,7 @@ public Action Delegate set => _action = value; } - public Type DelegateType - { - get => typeof(Action); - } - public bool KeyEquals(Keys other) => Key == other; - - private Action _update; - public Action Update - { - get => _update; - set => _update = value; - } } public class KaKeyAsync : IKbdAction> @@ -88,12 +76,5 @@ public Func Delegate } public bool KeyEquals(Keys other) => Key == other; - - private Action _update; - public Action Update - { - get => _update; - set => _update = value; - } } } diff --git a/QuickFiler/Controllers/KaStringAsync.cs b/QuickFiler/Controllers/KaStringAsync.cs index e048d0e7c..9951e6002 100644 --- a/QuickFiler/Controllers/KaStringAsync.cs +++ b/QuickFiler/Controllers/KaStringAsync.cs @@ -54,8 +54,74 @@ public bool Activated set => _activated = value; } + /// + /// Tests whether this action's matches a keyboard filter probe, and fires + /// this action's gated side effects. + /// + /// + /// + /// Latch contract. Activated is a per-keystroke latch that gates every + /// observable side effect of KeyEquals — both Update and ToggleControl. + /// A matching probe (branch 1) deliberately does not clear the latch and returns + /// early, so a matching element's Update continues to fire on each pass + /// KeyboardHandler makes within one keystroke; that repetition is intentional and is + /// what advances the item-number label. A non-matching probe (branches 2 and 3) + /// clears the latch, so a non-matching element's side effects fire at most once per + /// keystroke regardless of how many times a LINQ predicate is re-enumerated. + /// + /// + /// Branch 1's early return is therefore load-bearing and must not be "completed" into a + /// fall-through to the trailing latch reset for symmetry. KeyboardHandler re-arms the + /// latch only at filter length 1 and then makes three passes within one keystroke; if a + /// matching probe cleared the latch, the first pass would consume the activation and the + /// item-number label would stop advancing. + /// + /// + /// Argument contract. must be non-null and non-empty. The + /// guard clause at the top of this method rejects both fail-fast, so branch 1's substring + /// offset expression is never evaluated with a negative start index. + /// + /// + /// Consequence for callers. KbdActions methods whose key type is + /// string — ContainsKey, FilterKeys, Find, FindIndex, and + /// the indexer — inherit this new precondition: an empty key argument now surfaces an + /// ArgumentException from this predicate rather than matching every element. + /// + /// + /// + /// The keyboard filter probe to compare against . Must be non-null and + /// non-empty. + /// + /// + /// when contains as a + /// substring; otherwise . + /// + /// + /// is . + /// + /// + /// is empty. An empty probe would otherwise match every registered + /// action, because string.Contains(string.Empty) is true for every receiver. + /// public bool KeyEquals(string other) { + // The null test must come first: other.Length on a null reference throws + // NullReferenceException before any later guard could run. + if (other is null) + { + throw new ArgumentNullException(nameof(other)); + } + + if (other.Length == 0) + { + throw new ArgumentException( + "An empty probe is not a valid key. string.Contains(string.Empty) is true for " + + "every receiver, so an empty probe would otherwise match every registered " + + "action rather than none.", + nameof(other) + ); + } + if (Key.Contains(other)) { if (Activated && Update is not null) @@ -69,7 +135,7 @@ public bool KeyEquals(string other) } else if (other.Length > 1) { - if (Update is not null) + if (Activated && Update is not null) Update(Key.Substring(0, 1)); if (Activated && ToggleControl is not null) ToggleControl(); diff --git a/QuickFiler/Controllers/QfcExplorerController.cs b/QuickFiler/Controllers/QfcExplorerController.cs index 371162037..554bd30e6 100644 --- a/QuickFiler/Controllers/QfcExplorerController.cs +++ b/QuickFiler/Controllers/QfcExplorerController.cs @@ -1,23 +1,12 @@ -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.IO; -using System.Linq; -using System.Text; -using System.Text.RegularExpressions; -using System.Threading.Tasks; +using System.Threading.Tasks; using System.Windows.Forms; using Microsoft.Office.Interop.Outlook; using QuickFiler.Interfaces; -using ToDoModel; using UtilitiesCS; -using UtilitiesCS.OutlookExtensions; using Outlook = Microsoft.Office.Interop.Outlook; namespace QuickFiler.Controllers { - [ExcludeFromCodeCoverage] internal class QfcExplorerController : IQfcExplorerController { private static readonly log4net.ILog log = log4net.LogManager.GetLogger( @@ -57,11 +46,21 @@ internal bool CurrentConversationState get => _activeExplorer.CommandBars.GetPressedMso("ShowInConversations"); } - //PRIORITY: Implement ExplConvView_Cleanup - public void ExplConvView_Cleanup() - { - throw new NotImplementedException(); - } + // Injectable seam for the not-in-view prompt. The branch it guards calls a modal WinForms + // dialog, which cannot be exercised in a headless unit test: the dialog blocks on user input + // and requires a message pump. Tests replace this delegate with a stub that records the + // arguments and returns the DialogResult under test. The delegate type is written fully + // qualified as System.Func<...> so the seam does not resurrect the `using System;` directive + // that was removed as orphaned, matching the file's existing fully-qualified style for + // log4net.ILog and System.Reflection.MethodBase above. + internal System.Func< + string, + string, + MessageBoxButtons, + MessageBoxIcon, + DialogResult + > NotInViewDialogInvoker { get; set; } = + (text, caption, buttons, icon) => MessageBox.Show(text, caption, buttons, icon); public void ExplConvView_ReturnState() { @@ -137,7 +136,7 @@ private void NavigateToOutlookFolder(MailItem mailItem) ) { ExplConvView_ReturnState(); - _globals.Ol.App.ActiveExplorer().CurrentFolder = (MAPIFolder)mailItem.Parent; + _activeExplorer.CurrentFolder = (MAPIFolder)mailItem.Parent; BlShowInConversations = AutoFile.AreConversationsGrouped(_activeExplorer); } } @@ -165,7 +164,7 @@ public async Task OpenQFItem(MailItem mailItem) } else { - DialogResult result = MessageBox.Show( + DialogResult result = NotInViewDialogInvoker( "Selected message is not in view. Would you like to open it?", "Error", MessageBoxButtons.YesNo, @@ -179,145 +178,5 @@ public async Task OpenQFItem(MailItem mailItem) if (_initType.HasFlag(QfEnums.InitTypeEnum.Sort) & BlShowInConversations) await Task.Run(() => ExplConvView_ToggleOn()); } - - #region Email Sorting To Rewrite - - private static string SanitizeArrayLineTSV(ref string[] strOutput) - { - if (strOutput.IsInitialized()) - { - return string.Join( - "\t", - strOutput - .Where(s => !string.IsNullOrEmpty(s)) - .Select(s => StripTabsCrLf(s)) - .ToArray() - ); - } - else - { - return ""; - } - } - - internal static string StripTabsCrLf(string str) - { - var _regex = new Regex(@"[\t\n\r]*"); - string result = _regex.Replace(str, " "); - - // ensure max of one space per word - _regex = new Regex(@" +"); - result = _regex.Replace(result, " "); - result = result.Trim(); - return result; - } - - //TODO: Rewrite WriteCSV_StartNewFileIfDoesNotExist To Split it into one task per function - private static void WriteCSV_StartNewFileIfDoesNotExist( - string strFileName, - string strFileLocation - ) - { - string[] strOutput = null; - string[,] strAryOutput; - if (File.Exists(Path.Combine(strFileName, strFileLocation))) - { - strAryOutput = new string[14, 2]; - - strAryOutput[1, 1] = "Triage"; - strAryOutput[2, 1] = "FolderName"; - strAryOutput[3, 1] = "Sent_On"; - strAryOutput[4, 1] = "From"; - strAryOutput[5, 1] = "To"; - strAryOutput[6, 1] = "CC"; - strAryOutput[7, 1] = "Subject"; - strAryOutput[8, 1] = "Body"; - strAryOutput[9, 1] = "fromDomain"; - strAryOutput[10, 1] = "Conversation_ID"; - strAryOutput[11, 1] = "EntryID"; - strAryOutput[12, 1] = "Attachments"; - strAryOutput[13, 1] = "FlaggedAsTask"; - - SanitizeArray(strAryOutput, ref strOutput); - FileIO2.WriteTextFile(strFileName, strOutput, folderpath: strFileLocation); - } - strOutput = null; - strAryOutput = null; - } - - //QUESTION: Does this exist in a utility class? Check FileIO2 - private static void SanitizeArray(string[,] strAryOutput, ref string[] strOutput) - { - if (strAryOutput == null) - { - Debug.WriteLine($"The array {nameof(strAryOutput)} is empty."); - } - else - { - for (int j = 0; j < strAryOutput.GetLength(0); j++) - { - strOutput[j] = string.Join( - "\t", - strAryOutput - .SliceRow(j) - .Where(s => !string.IsNullOrEmpty(s)) - .Select(s => StripTabsCrLf(s)) - .ToArray() - ); - } - } - } - - //TODO: Implement SaveMessageAsMSG - private static void SaveMessageAsMSG(string fileSystem_LOC, IList selItems) - { - throw new NotImplementedException(); - } - - //TODO: Convert GetCurrentExplorerFolder to use the folder handler class - private static Folder GetCurrentExplorerFolder( - Explorer ActiveExplorer, - object objItem = null - ) - { - if (objItem is null) - { - objItem = ActiveExplorer.Selection[0]; - } - - if (objItem is MailItem) - { - MailItem OlMail = (MailItem)objItem; - return (Folder)OlMail.Parent; - } - else if (objItem is AppointmentItem) - { - AppointmentItem OlAppointment = (AppointmentItem)objItem; - return (Folder)OlAppointment.Parent; - } - else if (objItem is MeetingItem) - { - MeetingItem OlMeeting = (MeetingItem)objItem; - return (Folder)OlMeeting.Parent; - } - else if (objItem is TaskItem) - { - TaskItem OlTask = (TaskItem)objItem; - return (Folder)OlTask.Parent; - } - else - { - return null; - } - } - - //public static void Cleanup_Files() - //{ - // // Call WRITE_Text_File - Writes to the recents list - // // Call Email_AutoCategorize.CTF_Incidence_Text_File_WRITE - Writes to the CTF_Incidence file - // // Call Email_AutoCategorize.Subject_MAP_Text_File_WRITE - Writes to the Subject_MAP file - //} - - #endregion } } diff --git a/QuickFiler/Interfaces/IKbdAction.cs b/QuickFiler/Interfaces/IKbdAction.cs index 2c06b9a77..58a58ef0f 100644 --- a/QuickFiler/Interfaces/IKbdAction.cs +++ b/QuickFiler/Interfaces/IKbdAction.cs @@ -12,7 +12,5 @@ public interface IKbdAction T Key { get; set; } U Delegate { get; set; } bool KeyEquals(T other); - //Action Update { get; set; } - //Type DelegateType { get; } } } diff --git a/QuickFiler/Interfaces/IQfcExplorerController.cs b/QuickFiler/Interfaces/IQfcExplorerController.cs index 803b4e719..547dcb043 100644 --- a/QuickFiler/Interfaces/IQfcExplorerController.cs +++ b/QuickFiler/Interfaces/IQfcExplorerController.cs @@ -9,7 +9,6 @@ public interface IQfcExplorerController Task OpenQFItem(MailItem mailItem); void ExplConvView_ToggleOff(); void ExplConvView_ToggleOn(); - void ExplConvView_Cleanup(); void ExplConvView_ReturnState(); } } diff --git a/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/code-review.2026-08-22T10-58.md b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/code-review.2026-08-22T10-58.md new file mode 100644 index 000000000..895f7e2a6 --- /dev/null +++ b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/code-review.2026-08-22T10-58.md @@ -0,0 +1,67 @@ +# Code Review — Issue #449, QuickFiler Explorer Controller Latent Defects + +- **Timestamp:** 2026-08-22T10-58 +- **Reviewer:** feature-review agent +- **Branch:** `bug/quickfiler-explorer-controller-latent-defects-449-exec` at `af6531ed` +- **Diff:** `c551eaba..HEAD` — production: `QuickFiler/Controllers/QfcExplorerController.cs` (323 → 182), `QuickFiler/Interfaces/IQfcExplorerController.cs` (15 → 14); tests: two new files (387 + 205 lines) plus two `` lines in `QuickFiler.Test/QuickFiler.Test.csproj`. + +## Findings + +| ID | Severity | Blocking? | File / line | Finding | +| --- | --- | --- | --- | --- | +| CR-1 | Minor | Non-blocking | `QuickFiler.Test/Controllers/QfcExplorerControllerTests.cs:1-2` | Two unused `using` directives (`System.Collections`, `System.Collections.Generic`), stranded when the conversation-view tests — the only consumers of `IEnumerable`/`List<>` — moved to the continuation file in the [P6-T14] split. Verification: `grep -n "Collections\|IEnumerable\|List<" QuickFiler.Test/Controllers/QfcExplorerControllerTests.cs` matches only lines 1-2. No gate fires (CS8019 hidden; IDE0005 not wired into non-SDK projects). Remove opportunistically on the next touch. | +| CR-2 | Info | Non-blocking | `QuickFiler/Controllers/QfcExplorerController.cs:36,144` | Stale `//PRIORITY: Implement BlShowInConversations` and `//PRIORITY: Implement OpenQFItem` comments survive on members that are implemented and now tested. Pre-existing lines not modified by this diff; recorded only so a later reader does not mistake them for open work. Not chargeable to this change under the minimal-fix rule. | + +**Blocking findings: 0.** + +## Production Change Assessment + +### D1 — interface member removal (`IQfcExplorerController.cs`, `QfcExplorerController.cs`) + +Clean paired removal. The remaining interface carries exactly the five members the spec enumerates. Converting a latent runtime `NotImplementedException` into a compile-time absence is the correct failure-mode direction, and the legacy semantics are durably preserved in `spec.md` (`## Removed contract — legacy semantics for future restoration`) rather than reinvented later. The compatibility clause is satisfied: zero compiled callers (verified — surviving `ExplConvView_Cleanup` hits are only in uncompiled `Legacy/` and `Notes/` files), break called out. + +### D2 — the one-line fix (`QfcExplorerController.cs:139`) + +Correct and minimal. The guard read (line 135, `_activeExplorer.CurrentFolder.FolderPath`) and the assignment write (line 139) now address the same `Explorer` object. Line 24 is the only remaining `ActiveExplorer()` call in the file (verified: single grep hit, the constructor capture). No in-code justification for a fresh call was added, correctly, because the spec establishes no behavioral dependency on re-resolution exists. + +### D3 — dead-region deletion + +139 lines of unreachable duplicated code removed, including two latent defects inside it (transposed `Path.Combine` arguments; write into a null `ref string[]`), which are deleted rather than fixed — the right call for unreachable code under the minimal-fix rule. Verified: zero matches for any of the six identifiers under `QuickFiler` and `QuickFiler.Test` (down from 12 at merge-base per the dossier's non-vacuity check); external callers bind to the maintained `UtilitiesCS`/`ToDoModel` copies, which carry their own tests. + +### D4 — using hygiene + +Nine directives removed, six retained; self-verifying via the clean analyzer and nullable rebuilds (a required directive would have failed CS0246/CS1061). Correctly labelled as hygiene. Mildly ironic residual: the new test file introduces the same class of orphan (CR-1), though at test-file scope and below every gate's threshold. + +### D5 — coverage attribute removal and dialog seam + +The seam is well executed: + +- `internal System.Func` settable auto-property with a production default of `MessageBox.Show` — matches the repository's established settable-delegate idiom (`QfcHomeController.QfcExplorerControllerLoader`) rather than inventing a new abstraction. +- The comment block at lines 49-55 explains *why* (headless testability; why the delegate type is fully qualified so it does not resurrect the removed `using System;`) — this is comment-the-why done correctly. +- User-visible dialog text, caption, buttons, and icon are byte-identical; only the invocation route changed, and the seam-routing test asserts all four arguments verbatim. +- The class enters the coverage denominator at 87.8261% — above the `QuickFiler` package average — so removing the exclusion improved rather than degraded the package figure. No exclusion attribute of any scope was reintroduced (verified: zero grep matches). + +A mutable internal settable property is shared mutable state in principle, but it is the accepted repo pattern, is `internal` (reachable only via `InternalsVisibleTo`), and each test constructs its own controller instance, so no cross-test interference is possible. + +## Test Quality Assessment + +Fifteen test cases from fourteen methods across a `partial class` split. Strengths: + +- **Regression test design (defect 2)** is the strongest element: `SetupSequence` on `ActiveExplorer()` makes the captured and drifted explorers distinguishable, and the paired `VerifySet` assertions (captured `Times.Once()`, drifted `Times.Never()`) fail together before the fix and pass together after. The recorded fail-before run shows the genuine Moq verification failure with a 399 ms duration — a real assertion failure, not a load artifact. +- **The deliberate `MockBehavior.Loose` choice** — so a pre-fix assignment lands harmlessly and surfaces as a readable verification message rather than a strict-mode exception thrown from production code — is documented in-place and is the right trade-off for a fail-before test. +- **The non-short-circuiting `&` trap** (the `CommandBars` setup remains mandatory even when the left conjunct is false) is documented in the class-level ``, which will save the next author a confusing failure. +- **AAA structure, XML doc summaries on every test, per-test fixture reconstruction in `[TestInitialize]`** — all compliant with the unit test policy's structure, documentation, and independence requirements. +- **The partial-class split** follows the in-repo precedent (`QfcStreamingDequeueConfidenceGateTests` + `.Part2.cs`), keeps `[TestClass]` on the base file only (avoiding CS0579), and shares the fixture rather than duplicating 40 lines of mock graph — respecting the no-copy-paste rule. +- **Scenario completeness** for the changed paths: navigation positive/negative, dialog seam invoked/Yes/No, selection positive path, toggle-on/off positive and negative branches, sibling-view present/absent, pressed-state both values. Edge and negative flows are covered for every behavior this change touches. +- **Determinism**: no banned APIs (verified by scan), no temp files, no live forms, mocked COM only; `Task.Run` in production is awaited by the tests, so no timing device exists in test code. Two consecutive full-suite runs produced identical pass sets. + +Weaknesses: CR-1 (unused usings) only. The declined reflection contract test (`Contract_ExplConvView_Cleanup_IsNotDeclaredOnTheInterface`) was the correct omission — see the policy audit's dossier adjudication. + +## Disclosed-Item Assessment (code-review perspective) + +1. **Flaky `ProgressTrackerAsync` test:** disclosure without suppression was the correct engineering response. The evidence (fail once at 793 ms under load; pass at 191 ms in isolation; pass in two subsequent identical full-suite runs on an unchanged tree) is the standard signature of a timing race in an unrelated component, and the root cause identified in the promotion document (#584 — `UiThread.Dispatcher` static `null!` field with no lazy initialisation) is structural and pre-existing. Nothing in this diff can have caused it. Not a blocker. +2. **Fail-before dossiers:** both justified; the compiler gate plus the full-suite set comparison (+15 added / 0 removed) is a stronger verification for a no-caller removal and a dead-code deletion than any constructible test would have been. Detailed adjudication is in the policy audit, Section 5.2. + +## Verdict + +Approve. **0 Blocking findings**; 1 Minor (CR-1) and 1 Informational (CR-2) finding, neither warranting a remediation cycle. diff --git a/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/environment-preconditions.2026-08-22T09-16.md b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/environment-preconditions.2026-08-22T09-16.md new file mode 100644 index 000000000..dfc37fbe6 --- /dev/null +++ b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/environment-preconditions.2026-08-22T09-16.md @@ -0,0 +1,131 @@ +# Baseline — Environment Preconditions (Issue #449, [P0-T6]) + +Timestamp: 2026-08-22T09-16 +WORKTREE: `C:\Users\DanMoisan\repos\TaskMaster\.claude\worktrees\agent-a5600546d71e73061` + +Command: see the per-finding `Command:` lines below; each finding names the exact command that +established it. Aggregate exit code for the verification set: +EXIT_CODE: 0 + +--- + +## (a) No Python toolchain — `poetry run python -m scripts.dev_tools.*` is unrunnable by absence + +Command: `ls -d scripts/dev_tools` +EXIT_CODE: 2 +Output: `ls: cannot access 'scripts/dev_tools': No such file or directory` + +Command: `ls -1 pyproject.toml poetry.lock` +EXIT_CODE: 2 +Output: `ls: cannot access 'pyproject.toml': No such file or directory` / +`ls: cannot access 'poetry.lock': No such file or directory` + +Command: `ls -1 scripts/` +EXIT_CODE: 0 +Output: `dev-tools/`, `temp-extract-coverage.ps1`, `vscode/` + +Command: `ls -1 scripts/dev-tools/` +EXIT_CODE: 0 +Output: `run-actionlint.ps1` + +Command: `git ls-files "*.py"` +EXIT_CODE: 0 +Output (2 files, both inside an ARCHIVED feature folder, neither a dev-tools module): +``` +docs/features/archive/2026-07-18-stale-app-config-binding-redirects-354/scripts/fix_binding_redirects.py +docs/features/archive/2026-07-18-stale-app-config-binding-redirects-354/tests/scripts/test_fix_binding_redirects.py +``` + +**Finding.** There is no `scripts/dev_tools/` directory (the only similar path is +`scripts/dev-tools/`, hyphenated, containing a single PowerShell script), no `pyproject.toml`, and no +`poetry.lock`. The importable package `scripts.dev_tools` therefore does not exist, and there is no +Poetry environment to run it in. Any skill or process step naming +`poetry run python -m scripts.dev_tools.*` is **unrunnable by absence** in this repository. It is +recorded here as such; no result is fabricated for it and it is not silently omitted. See [P7-T15] +for the corresponding final-QC record. + +## (b) No `quality-tiers.yml` at the WORKTREE root — no QuickFiler tier classification can be cited + +Command: `ls -1 quality-tiers.yml` +EXIT_CODE: 2 +Output: `ls: cannot access 'quality-tiers.yml': No such file or directory` + +**Finding.** `.claude/rules/quality-tiers.md` names `quality-tiers.yml` at the repository root as the +source of truth for the T1-T4 tier map, but that file does not exist in this repository. No tier +classification for `QuickFiler.csproj` can be cited, and no tier-dependent gate (property-test +density, mutation score, untyped-escape-hatch budget) is enforceable here. This child cites no tier. + +## (c) The only machine-enforced numeric coverage gate is the repo-wide 80% line rate + +Command: `grep -rn -E "below the required|branch-rate|branchRate|-lt 8[0-9]|-lt 7[0-9]|-lt 9[0-9]" scripts/` +EXIT_CODE: 0 +Output: +``` +scripts/vscode/Invoke-MSTestWithCoverage.ClosureFilter.ps1:386: $classNode.SetAttribute('branch-rate', $retainedBranchRate) +scripts/vscode/Invoke-MSTestWithCoverage.Helpers.ps1:375: $mergedClassNode.SetAttribute('branch-rate', $mergedBranchRate) +scripts/vscode/Invoke-MSTestWithCoverage.Helpers.ps1:443: $xml.coverage.SetAttribute('branch-rate', $coverageSummary.BranchRate) +scripts/vscode/Invoke-MSTestWithCoverage.Helpers.ps1:487: if ($percentage -lt 80) { +scripts/vscode/Invoke-MSTestWithCoverage.Helpers.ps1:489: throw "Cobertura line coverage $formattedPercentage% is below the required 80% threshold." +``` + +Command: `sed -n '480,495p' scripts/vscode/Invoke-MSTestWithCoverage.Helpers.ps1` +EXIT_CODE: 0 +Output: +```powershell + if ($lineRate -lt 0 -or $lineRate -gt 1) { + throw 'Cobertura line-rate must be between 0 and 1.' + } + + $percentage = $lineRate * 100 + if ($percentage -lt 80) { + $formattedPercentage = $percentage.ToString('0.####', [System.Globalization.CultureInfo]::InvariantCulture) + throw "Cobertura line coverage $formattedPercentage% is below the required 80% threshold." + } +``` + +**Finding.** The single machine-enforced numeric coverage threshold anywhere under `scripts/` is the +repo-wide Cobertura root `line-rate` compared against 80% at +`scripts/vscode/Invoke-MSTestWithCoverage.Helpers.ps1:487-489`. The three other `branch-rate` +occurrences are attribute WRITES during Cobertura post-processing, not comparisons, so no +branch-coverage gate exists. There is no per-file gate, no per-assembly gate, and no per-class gate. +Consequently the `QuickFiler` package figure and the `QfcExplorerController` figure recorded by this +plan are reported values, not gated values, and the plan's [P7-T9]/[P7-T10] treatment of them as +delta comparisons rather than absolute threshold checks is correct. + +## (d) PreToolUse hooks are inert; no gate in this plan relies on one + +Command: `grep -rln 'toolInput.command' .claude/hooks/` +EXIT_CODE: 0 +Output: +``` +.claude/hooks/enforce-epic-merge-gate.ps1 +.claude/hooks/enforce-epic-worktree-removal-gate.ps1 +.claude/hooks/enforce-parallel-abandon-gate.ps1 +.claude/hooks/enforce-parallel-worktree-removal-gate.ps1 +.claude/hooks/enforce-pr-author-skill.ps1 +.claude/hooks/enforce-promotion-mcp-only.ps1 +``` + +Command: `grep -rln 'tool_input.command' .claude/hooks/` +EXIT_CODE: 1 +Output: (no match) + +**Finding.** Every command-inspecting hook reads `$toolInput.command`, and no hook reads the nested +`$toolInput.tool_input.command` where the payload actually carries the value. Each therefore sees an +empty command string and returns `permissionDecision: allow`. No gate in this plan relies on a hook: +every gate in Phases 1 through 7 is verified from durable `git` state, from a command's own exit +code, or from a parsed build/coverage artifact. `.claude/**` is read-only for this child, so no hook +is repaired here. + +--- + +## Output Summary + +All four preconditions verified. (a) No Python toolchain exists: no `scripts/dev_tools/`, no +`pyproject.toml`, no `poetry.lock`; the two tracked `.py` files live in an archived feature folder. +Python steps are recorded as unrunnable by absence rather than fabricated or skipped. (b) +`quality-tiers.yml` is absent from the WORKTREE root, so no QuickFiler tier is cited. (c) The only +machine-enforced numeric coverage gate under `scripts/` is the repo-wide 80% line rate at +`Invoke-MSTestWithCoverage.Helpers.ps1:487-489`; there is no per-file, per-assembly, or +branch-coverage gate. (d) All six command-inspecting PreToolUse hooks read the wrong payload key and +are inert; no gate in this plan depends on one. diff --git a/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/file-line-counts.2026-08-22T09-16.md b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/file-line-counts.2026-08-22T09-16.md new file mode 100644 index 000000000..5b7dd51d3 --- /dev/null +++ b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/file-line-counts.2026-08-22T09-16.md @@ -0,0 +1,119 @@ +# Baseline — File Line Counts (Issue #449, [P0-T14], [P0-T15]) + +Timestamp: 2026-08-22T09-16 +WORKTREE: `C:\Users\DanMoisan\repos\TaskMaster\.claude\worktrees\agent-a5600546d71e73061` +Merge-base SHA: `c551eabab0aa0a6b1a284252811a2e1de819634e` (HEAD equals it at baseline) + +Command: +``` +grep -c '' QuickFiler/Controllers/QfcExplorerController.cs \ + QuickFiler/Interfaces/IQfcExplorerController.cs \ + QuickFiler.Test/QuickFiler.Test.csproj \ + UtilitiesCS/EmailIntelligence/EmailParsingSorting/SortEmail.cs \ + QuickFiler/Legacy/QuickFileController.cs +wc -l QuickFiler.Test/QuickFiler.Test.csproj +``` +EXIT_CODE: 0 + +`grep -c ''` is used rather than `wc -l` because `wc -l` counts newline CHARACTERS and therefore +under-reports by one for any file lacking a terminating newline. `QuickFiler.Test/QuickFiler.Test.csproj` +is exactly such a file. + +--- + +## [P0-T14] — Pre-change line counts of the three files AC-16 measures + +| File | Plan expectation | **Measured** | Match | +| --- | --- | --- | --- | +| `QuickFiler/Controllers/QfcExplorerController.cs` | 323 | **323** | yes | +| `QuickFiler/Interfaces/IQfcExplorerController.cs` | 15 | **15** | yes | +| `QuickFiler.Test/QuickFiler.Test.csproj` | 484 | **484** | yes | + +Raw output: +``` +QuickFiler/Controllers/QfcExplorerController.cs:323 +QuickFiler/Interfaces/IQfcExplorerController.cs:15 +QuickFiler.Test/QuickFiler.Test.csproj:484 +``` + +### The `QuickFiler.Test.csproj` terminating-newline caveat, confirmed empirically + +``` +$ wc -l QuickFiler.Test/QuickFiler.Test.csproj +483 QuickFiler.Test/QuickFiler.Test.csproj + +$ grep -c '' QuickFiler.Test/QuickFiler.Test.csproj +484 +``` + +The two tools disagree by exactly one, which confirms the plan's note that the file has no +terminating newline: `wc -l` reports **483** against a true count of **484**. Every figure this plan +states for that file — 484 pre-change, 485 after the single [P1-T2] append, or 486 after a second +append should [P6-T14] force a split — is a TRUE count in the `grep -c ''` sense. Any later +verification of that file's size must use `grep -c ''`, not `wc -l`, or it will appear off by one. + +### Post-change expectations recorded for later comparison + +| File | Pre-change | Expected post-change | Source | +| --- | --- | --- | --- | +| `QfcExplorerController.cs` | 323 | 323 after [P2-T1] (one-line replacement, no count change); ~317 after [P3-T2]; ~178 after [P4-T1]; then further reduced by [P4-T2]/[P5-T2] and increased by the [P5-T3] seam | measured at each phase | +| `IQfcExplorerController.cs` | 15 | 14 after [P3-T1] | [P3-T1] acceptance | +| `QuickFiler.Test.csproj` | 484 | 485 after [P1-T2]; 486 only if [P6-T14] forces a split | [P1-T2] / [P6-T14] acceptance | + +--- + +## [P0-T15] — Pre-existing 500-line-cap violations that this change does NOT touch + +| File | **Measured lines** | Edited by this change? | Appears in the diff? | +| --- | --- | --- | --- | +| `UtilitiesCS/EmailIntelligence/EmailParsingSorting/SortEmail.cs` | **1429** | **No** | **No** | +| `QuickFiler/Legacy/QuickFileController.cs` | **1065** | **No** | **No** | + +Raw output: +``` +UtilitiesCS/EmailIntelligence/EmailParsingSorting/SortEmail.cs:1429 +QuickFiler/Legacy/QuickFileController.cs:1065 +``` + +**Neither file is edited by this change and neither will appear in the diff.** Both are pre-existing +violations of the 500-line file cap in `.claude/rules/general-code-change.md`. This statement is +recorded pre-emptively so that a later reviewer measuring the repository does not attribute either +violation to issue #449. [P7-T13] carries the same attribution statement against the actual diff. + +Why each is out of scope: + +- `SortEmail.cs` (1,429 lines) is the surviving maintained copy of the helpers duplicated inside the + dead region that [P4-T1] deletes. It carries its own tests in `UtilitiesCS.Test`. Consolidating the + three copies is a separate, larger change and is explicitly not planned here. No split refactor is + performed on it. +- `QuickFileController.cs` (1,065 lines) sits in `QuickFiler/Legacy/` and is **not compiled** — + `QuickFiler/QuickFiler.csproj` contains no `Compile Include` entry for the `Legacy/` directory. It + is therefore invisible to every build gate. No split refactor is performed on it. + +### Correction of a premise carried in the epic kickoff + +The epic kickoff described `QuickFiler/Controllers/QfcExplorerController.cs` as 1,065 lines and +predicted that this change would produce a 500-line-cap violation requiring a partial-class split. +That is a misattribution. The measurements above show: + +- `QuickFiler/Controllers/QfcExplorerController.cs` measures **323** lines, comfortably under the + 500-line cap both before and after this change (it only ever SHRINKS, to roughly 178 after the + [P4-T1] dead-region deletion, before the small [P5-T3] seam addition). +- The **1,065** figure actually belongs to `QuickFiler/Legacy/QuickFileController.cs`, the uncompiled + legacy file above, which this change does not edit. + +**No partial-class split is needed and none is in scope.** The kickoff's 1,065 figure is not acted +on. This agrees with the plan's own [P0-T15] and [P7-T13] attribution. + +--- + +## Output Summary + +All five measured counts match the plan's stated expectations exactly. AC-16 files pre-change: +`QfcExplorerController.cs` **323**, `IQfcExplorerController.cs` **15**, +`QuickFiler.Test.csproj` **484** (true count; `wc -l` reports 483 because the file has no terminating +newline, confirmed empirically). Pre-existing over-cap files not touched by this change: +`SortEmail.cs` **1429** and `QuickFiler/Legacy/QuickFileController.cs` **1065** — neither is edited +and neither will appear in the diff. The epic kickoff's claim that `QfcExplorerController.cs` is 1,065 +lines is a misattribution of the legacy file's count; the controller is 323 lines and no +partial-class split is required or in scope. diff --git a/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/git-state.2026-08-22T09-16.md b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/git-state.2026-08-22T09-16.md new file mode 100644 index 000000000..18165e88a --- /dev/null +++ b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/git-state.2026-08-22T09-16.md @@ -0,0 +1,69 @@ +# Baseline — Git State (Issue #449, [P0-T7]) + +Timestamp: 2026-08-22T09-16 +WORKTREE: `C:\Users\DanMoisan\repos\TaskMaster\.claude\worktrees\agent-a5600546d71e73061` +Branch: `bug/quickfiler-explorer-controller-latent-defects-449-exec` + +Command: `git rev-parse --abbrev-ref HEAD` ; `git branch -a --list "*integration*"` ; +`git merge-base HEAD epic/quickfiler-suite-determinism-foundation-integration` ; +`git log -1 --format='%H%n%an%n%ad%n%s'` ; `git status --porcelain` +EXIT_CODE: 0 + +## Integration branch resolution + +The epic integration branch **is present** locally and on the remote, so the merge base is taken +against it rather than against `main`: + +``` ++ epic/quickfiler-suite-determinism-foundation-integration + remotes/origin/epic/quickfiler-suite-determinism-foundation-integration +``` + +The leading `+` marks the branch as checked out in another worktree (the epic orchestrator's +dedicated integration worktree). This worktree is on +`bug/quickfiler-explorer-controller-latent-defects-449-exec` and does not check out the integration +branch. + +## MERGE-BASE SHA (authoritative for every later diff gate in this plan) + +``` +c551eabab0aa0a6b1a284252811a2e1de819634e +``` + +Command: `git merge-base HEAD epic/quickfiler-suite-determinism-foundation-integration` +EXIT_CODE: 0 + +Later diff-based gates — [P3-T3], [P7-T13], and [P7-T14] — read the merge-base SHA from this +artifact. It is recorded once here so no downstream task recomputes it. + +## HEAD at baseline + +``` +c551eabab0aa0a6b1a284252811a2e1de819634e +Dan Moisan +Sat Aug 22 09:05:20 2026 -0400 +docs(epic): seed epic-status.md projection at wave-0 kickoff +``` + +**HEAD currently EQUALS the merge base.** This is recorded as a factual observation, not as a plan +expectation: per [P0-T7] no `HEAD` SHA is pinned and the gates in this plan are tree invariants, not +SHA equalities. The consequence for execution is that any `..HEAD` diff is EMPTY until +[P7-T12] commits. [P7-T12] is therefore a hard prerequisite for [P7-T13] and [P7-T14], exactly as the +plan states, and neither gate may be evaluated before that commit exists. + +## Working tree status + +Command: `git status --porcelain` +EXIT_CODE: 0 +Output: (empty — no output) + +The tree is clean at baseline. `.dotnet-sdk/` and `packages/` are present in the filesystem but do +not appear here because `.gitignore` carries `.dotnet*/` and `**/[Pp]ackages/*`. + +## Output Summary + +Branch `bug/quickfiler-explorer-controller-latent-defects-449-exec`. Merge base against +`epic/quickfiler-suite-determinism-foundation-integration` is +`c551eabab0aa0a6b1a284252811a2e1de819634e`. HEAD equals the merge base at baseline, so all +merge-base diff gates are vacuous until the [P7-T12] commit lands. `git status --porcelain` is empty: +the working tree is clean. diff --git a/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/phase0-instructions-read.md b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/phase0-instructions-read.md new file mode 100644 index 000000000..dc66fb1b2 --- /dev/null +++ b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/phase0-instructions-read.md @@ -0,0 +1,51 @@ +# Phase 0 — Policy Instructions Read (Issue #449) + +Timestamp: 2026-08-22T09-16 + +Policy Order: The order mandated by `.claude/skills/policy-compliance-order/SKILL.md` is +1. `CLAUDE.md` (standing instructions, all sections including the embedded General Code Change Policy, General Unit Test Policy, C# Code Change Policy, and C# Unit Test Policy) +2. `.claude/rules/general-code-change.md` (cross-language code change policy) +3. `.claude/rules/general-unit-test.md` (cross-language unit test policy) +4. Language-specific rules for the files in scope — for C#, `.claude/rules/csharp.md` + +Command: `grep -c '' CLAUDE.md .claude/rules/general-code-change.md .claude/rules/general-unit-test.md .claude/skills/policy-compliance-order/SKILL.md` and `grep -c '' .claude/rules/csharp.md` +EXIT_CODE: 0 + +## Files read end to end, in policy order + +| Order | Path | Line count | Task | +| --- | --- | --- | --- | +| 1 | `CLAUDE.md` | 447 | [P0-T1] | +| 2 | `.claude/rules/general-code-change.md` | 80 | [P0-T2] | +| 3 | `.claude/rules/general-unit-test.md` | 105 | [P0-T3] | +| 4 | `.claude/rules/csharp.md` | 96 | [P0-T4] | + +Supporting reference read for the order itself: `.claude/skills/policy-compliance-order/SKILL.md` (40 lines). + +## [P0-T4] resolution + +`.claude/rules/csharp.md` is PRESENT (96 lines) and was read in full. The planner note that the file +was present at plan-authoring time is confirmed. No absence statement is required. + +Directory listing that established presence: + +``` +architecture-boundaries.md general-code-change.md powershell.md +benchmark-baselines.md general-unit-test.md python.md +ci-workflows.md mermaid.md python-suppressions.md +csharp.md orchestrator-state.md quality-tiers.md + parallel-orchestration.md self-explanatory-code-commenting.md + plan-acceptance-gates.md shell.md + tonality.md + typescript.md + typescript-suppressions.md +``` + +## Output Summary + +All four mandatory policy documents were read end to end in the mandated order. No policy document +was edited; `.claude/**` is read-only for this child. The controlling constraints carried forward +into execution are: CSharpier via `dotnet tool run` only; `/t:Rebuild` never `/t:Build`; no +`/p:Nullable=enable`; MSTest + Moq + FluentAssertions only; no `Thread.Sleep`, `Task.Delay`, +`DateTime.Now`, `Random.Shared`, temporary file, or real wall-clock wait in tests; 500-line file cap +for non-Markdown files. diff --git a/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step1-dotnet-tool-restore.2026-08-22T09-16.md b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step1-dotnet-tool-restore.2026-08-22T09-16.md new file mode 100644 index 000000000..6096bb91f --- /dev/null +++ b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step1-dotnet-tool-restore.2026-08-22T09-16.md @@ -0,0 +1,123 @@ +# Baseline Toolchain Step 1 — Worktree Bootstrap and `dotnet tool restore` (Issue #449, [P0-T8]) + +Timestamp: 2026-08-22T09-16 +WORKTREE: `C:\Users\DanMoisan\repos\TaskMaster\.claude\worktrees\agent-a5600546d71e73061` + +[P0-T8] specifies three commands, (a) SDK install, (b) `nuget restore`, (c) `dotnet tool restore`, +each conditioned on the absence of a directory. (a) and (b) were performed as part of the +agent-worktree bootstrap BEFORE this executor was launched; their preconditions are therefore already +satisfied and their guard conditions are false. Each is recorded below with the command that VERIFIED +its completed state. (c) is per-worktree and was run by this executor. + +--- + +## (a) Repo-local SDK — guard condition FALSE, `.dotnet-sdk` already present + +Conditional command per [P0-T8]: `./scripts/vscode/Install-RepoDotNetSdk.ps1` +Guard: run only "if `.dotnet-sdk` is absent". `.dotnet-sdk` is PRESENT, so the installer was NOT +re-run. Re-running it was explicitly prohibited by the execution conditions for this session. + +Verification Command: `ls -d .dotnet-sdk` +EXIT_CODE: 0 +Output: `.dotnet-sdk/` + +Verification Command: +`pwsh -NoProfile -Command 'Set-Location ""; dotnet --version; "DOTNET_VERSION_EXIT=$LASTEXITCODE"'` +EXIT_CODE: 0 +Output: +``` +8.0.205 +DOTNET_VERSION_EXIT=0 +``` + +`global.json` pins SDK `8.0.205` with `paths: [".dotnet-sdk", "$host$"]`. The resolved version is +exactly `8.0.205`, which proves the repo-local SDK is the one being used and that no `global.json` +`errorMessage` is being raised. Had `.dotnet-sdk` been missing, this command would have failed with +that message instead of printing a version. + +## (b) NuGet packages — guard condition FALSE, `packages/` already present + +Conditional command per [P0-T8]: `nuget restore TaskMaster.sln` +Guard: run only "if `packages/` is absent". `packages/` is PRESENT and mirrored from the main +checkout, so `nuget restore` was NOT re-run. + +Verification Command: `ls -d packages` +EXIT_CODE: 0 +Output: `packages/` + +Verification Command: `ls -1 packages/ | grep -c ''` +EXIT_CODE: 0 +Output: `265` package directories present. + +### Analyzer version skew — verified benign, NOT repaired here + +`QuickFiler/packages.config` pins `Meziantou.Analyzer` at `3.0.174` and `Roslynator.Analyzers` at +`4.16.1`, while `QuickFiler/QuickFiler.csproj` `` items point at `3.0.156` and +`4.16.0`. That divergence between `packages.config` and the `` HintPaths is a +pre-existing repo-wide condition and is sibling child #511's scope. **No `.csproj` and no +`packages.config` is edited by this child to "fix" it.** + +Command: `ls -d packages/Meziantou.Analyzer.* packages/Roslynator.Analyzers.*` +EXIT_CODE: 0 +Output: +``` +packages/Meziantou.Analyzer.3.0.101/ +packages/Meziantou.Analyzer.3.0.123/ +packages/Meziantou.Analyzer.3.0.156/ +packages/Meziantou.Analyzer.3.0.174/ +packages/Roslynator.Analyzers.4.16.0/ +packages/Roslynator.Analyzers.4.16.1/ +``` + +Command: `grep -n -E "Meziantou|Roslynator" QuickFiler/QuickFiler.csproj` +EXIT_CODE: 0 +Output: +``` +3: +576: +582: +583: +584: +585: +586: +``` + +**Both** version families are present on disk: `3.0.174` satisfies the `` at line 3 and the +hard `` at line 576, and `3.0.156`/`4.16.0` satisfy the +`` HintPaths at lines 582-586. No `CS0006` (metadata file not found) is expected +from the analyzer wiring, and the baseline analyzer build in [P0-T10] is the empirical confirmation. + +## (c) `dotnet tool restore` — RUN BY THIS EXECUTOR + +Command: +`pwsh -NoProfile -Command 'Set-Location ""; dotnet tool restore; "TOOL_RESTORE_EXIT=$LASTEXITCODE"'` +EXIT_CODE: 0 +Output: +``` +Tool 'csharpier' (version '1.2.6') was restored. Available commands: csharpier + +Restore was successful. +TOOL_RESTORE_EXIT=0 +``` + +The manifest-pinned CSharpier version `1.2.6` was restored, matching the version +`.github/workflows/ci.yml` uses after its own `dotnet tool restore`. Every later CSharpier invocation +in this plan goes through `dotnet tool run csharpier`, never a global install. + +## Gitignore confirmation — bootstrap trees do not dirty the tree + +`.gitignore` carries `.dotnet*/` and `**/[Pp]ackages/*`, so neither `.dotnet-sdk/` nor `packages/` +appears in `git status`. `git status --porcelain` was empty at handoff and remains empty after +`dotnet tool restore`; see `git-state.2026-08-22T09-16.md`. + +--- + +## Output Summary + +All three [P0-T8] commands accounted for. (a) `.dotnet-sdk` present; `dotnet --version` prints +`8.0.205`, EXIT_CODE 0, matching the `global.json` pin — installer NOT re-run because its guard +condition is false. (b) `packages/` present with 265 package directories — `nuget restore` NOT re-run +because its guard condition is false; the pre-existing `packages.config`-versus-`` +version skew was verified benign because both version families are on disk, and no build file was +edited. (c) `dotnet tool restore` was run by this executor and returned **EXIT_CODE: 0**, restoring +CSharpier 1.2.6. The required `dotnet tool restore` exit code of `0` is satisfied. diff --git a/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step2-csharpier-check.2026-08-22T09-16.md b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step2-csharpier-check.2026-08-22T09-16.md new file mode 100644 index 000000000..2640c0b50 --- /dev/null +++ b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step2-csharpier-check.2026-08-22T09-16.md @@ -0,0 +1,42 @@ +# Baseline Toolchain Step 2 — CSharpier Check, Read-Only (Issue #449, [P0-T9]) + +Timestamp: 2026-08-22T09-16 +WORKTREE: `C:\Users\DanMoisan\repos\TaskMaster\.claude\worktrees\agent-a5600546d71e73061` + +Command: +`pwsh -NoProfile -Command 'Set-Location ""; dotnet tool run csharpier check .; "CSHARPIER_CHECK_EXIT=$LASTEXITCODE"'` +EXIT_CODE: 0 + +Output: +``` +Checked 1517 files in 6644ms. +CSHARPIER_CHECK_EXIT=0 +``` + +## Read-only discipline + +`csharpier format .` was deliberately NOT run at baseline, per [P0-T9]. The baseline must describe the +unmodified merge-base tree, and a mutating format pass would have made the recorded baseline describe +a tree that no commit contains. The mutating pass happens later: scoped to this plan's own new file in +[P1-T4] and [P5-T12]/[P6-T15], and repository-wide only in the final QC loop at [P7-T2]. + +CSharpier was invoked through `dotnet tool run` so the manifest-pinned version `1.2.6` was used, not +any globally installed CSharpier. A different global version produces diffs that disagree with +`.github/workflows/ci.yml`. + +## Baseline formatting state + +**Number of files reported as needing formatting: 0 (zero).** + +CSharpier's `check` subcommand reports each unformatted file by path and exits non-zero when any file +needs formatting. The output contains no per-file report line and the exit code is `0`, so the count +of files needing formatting is zero. 1,517 files were checked, which is the scope CSharpier applies +after `.csharpierignore` exclusions — `*.csproj`, `*.props`, and `*.targets` are excluded there, which +is why the later `QuickFiler.Test.csproj` edit in [P1-T2] cannot be reformatted by CSharpier. + +## Output Summary + +Baseline formatting state is CLEAN: 1,517 files checked, **zero** files reported as needing +formatting, EXIT_CODE 0, in 6,644 ms. Any file reported as needing formatting in the final QC pass +[P7-T3] would therefore be attributable to this change rather than pre-existing. The read-only +`check` subcommand was used and no file was modified by this step. diff --git a/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step3-analyzer-build.2026-08-22T09-16.md b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step3-analyzer-build.2026-08-22T09-16.md new file mode 100644 index 000000000..76f3b19c9 --- /dev/null +++ b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step3-analyzer-build.2026-08-22T09-16.md @@ -0,0 +1,97 @@ +# Baseline Toolchain Step 3 — Analyzer Build (Issue #449, [P0-T10]) + +Timestamp: 2026-08-22T09-16 +WORKTREE: `C:\Users\DanMoisan\repos\TaskMaster\.claude\worktrees\agent-a5600546d71e73061` + +Command: +``` +pwsh -NoProfile -Command 'Set-Location ""; + & "C:\Program Files\Microsoft Visual Studio\18\Community\MSBuild\Current\Bin\MSBuild.exe" ` + TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" ` + /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true /v:n /nologo *> $log; + "MSBUILD_EXIT=$LASTEXITCODE"' +``` +EXIT_CODE: 0 + +Log captured to the session scratchpad (outside the repository, so no helper artifact is retained +under `evidence/`): `.../scratchpad/449/p0t10-analyzer.log`, 4,800 lines. + +MSBuild resolved via `vswhere -latest -prerelease -requires Microsoft.Component.MSBuild` to +`C:\Program Files\Microsoft Visual Studio\18\Community` — the VS **18** full-framework MSBuild, which +is required because the .NET SDK MSBuild fails on this repository's binary `.resx` resources +(MSB3822). + +## Warning and error counts + +``` +5 Warning(s) +0 Error(s) +``` + +All 5 warnings are the same pre-existing, non-actionable diagnostic — `System.Reactive` v7.0 +complaining that the consuming projects use `packages.config` rather than `PackageReference`: + +> `packages\System.Reactive.7.0.0\build\System.Reactive.PackagesConfigCheck.targets(31,5): warning : +> The project contains a packages.config file, which is not supported by System.Reactive v7.0 or +> later. Please migrate to PackageReference.` + +It is emitted once per consuming project (including `TaskMaster.csproj` and +`UtilitiesCS.Test.csproj`). It is unrelated to this change, is not raised by any of the five wired +analyzers, and is the accepted baseline warning level. **Baseline warning count to compare the final +QC build against: 5.** Zero analyzer diagnostics and zero errors. + +## `/t:Rebuild` verification — the `Skipping target "CoreCompile"` gate is ZERO and NON-VACUOUS + +Command: `grep -c 'Skipping target "CoreCompile"' p0t10-analyzer.log` +EXIT_CODE: 1 (grep reports 1 for a zero count) +Output: `0` + +**Count of occurrences of the string `Skipping target "CoreCompile"` in the captured log: 0 (zero).** +The [P0-T10] acceptance condition is satisfied. + +That zero is proven to be a real observation rather than an artifact of the chosen `/v:n` verbosity. +A count of zero would be worthless if the log format never emitted such a line at all, so the log was +additionally searched for the message PREFIX: + +Command: `grep -c 'Skipping target' p0t10-analyzer.log` +EXIT_CODE: 0 +Output: `9` + +Command: `grep -n 'Skipping target' p0t10-analyzer.log` +EXIT_CODE: 0 +Output (all nine, verbatim): +``` +132: Skipping target "CopyMSTestV2Resources" because it has no outputs. +711: Skipping target "CopyMSTestV2Resources" because it has no outputs. +1258: Skipping target "CopyMSTestV2Resources" because it has no outputs. +1566: Skipping target "CopyMSTestV2Resources" because it has no outputs. +1983: Skipping target "CopyMSTestV2Resources" because it has no outputs. +2316: Skipping target "CopyMSTestV2Resources" because it has no outputs. +3412: Skipping target "CopyMSTestV2Resources" because it has no outputs. +3595: Skipping target "CopyMSTestV2Resources" because it has no outputs. +3599: Skipping target "CopyMSTestV2Resources" because it has no outputs. +``` + +`Skipping target "..."` lines ARE emitted at `/v:n`, nine of them, and every one names +`CopyMSTestV2Resources` (skipped because it has no outputs, which is unrelated to incrementality). +Not one names `CoreCompile`. The gate could have fired and did not, so the zero is discriminating. + +Corroborating evidence that compilation genuinely occurred: `grep -c 'Csc\|csc.exe'` returns **36** +and `grep -c 'CoreCompile'` returns **44**. Per repository convention a `csc.exe` count is not used +as the gate — it reads zero even on real compiles under some log shapes — but a non-zero count here is +consistent with a real full rebuild. Elapsed time 00:00:24.83. + +`/t:Build` was NOT used. MSBuild's up-to-date check does not invalidate on a command-line `/p:` +change, so a warm `/t:Build` returns exit 0 with `CoreCompile` skipped on every project and runs no +analyzers — the gate cannot fail. `/t:Rebuild` is what makes this a real gate. + +## Output Summary + +Baseline analyzer build PASSED. **EXIT_CODE 0, 5 warnings, 0 errors.** All 5 warnings are the +pre-existing `System.Reactive` v7.0 `packages.config` advisory, one per consuming project; zero +analyzer diagnostics. The count of `Skipping target "CoreCompile"` in the captured log is **zero**, +and that zero is non-vacuous because the same log carries 9 other `Skipping target "..."` lines (all +`CopyMSTestV2Resources`), proving the message form is visible at this verbosity. 36 `csc` invocations +confirm real compilation. Analyzers ran. The pre-existing `Meziantou.Analyzer` / +`Roslynator.Analyzers` version skew between `packages.config` and the `` HintPaths +produced no `CS0006`, confirming the [P0-T8] finding that both version families are present on disk. diff --git a/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step4-nullable-build.2026-08-22T09-16.md b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step4-nullable-build.2026-08-22T09-16.md new file mode 100644 index 000000000..028e0da3f --- /dev/null +++ b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step4-nullable-build.2026-08-22T09-16.md @@ -0,0 +1,74 @@ +# Baseline Toolchain Step 4 — Nullable / Type Check Build (Issue #449, [P0-T11]) + +Timestamp: 2026-08-22T09-16 +WORKTREE: `C:\Users\DanMoisan\repos\TaskMaster\.claude\worktrees\agent-a5600546d71e73061` + +Command: +``` +pwsh -NoProfile -Command 'Set-Location ""; + & "C:\Program Files\Microsoft Visual Studio\18\Community\MSBuild\Current\Bin\MSBuild.exe" ` + TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" ` + /p:TreatWarningsAsErrors=true /v:n /nologo *> $log; + "MSBUILD_EXIT=$LASTEXITCODE"' +``` +EXIT_CODE: 0 + +Log captured to the session scratchpad: `.../scratchpad/449/p0t11-nullable.log`, 11,213 lines. + +## Error count + +``` +5 Warning(s) +0 Error(s) +``` + +**Baseline error count: 0 (zero).** + +The 5 warnings are the same pre-existing `System.Reactive` v7.0 `packages.config` advisory recorded +in `step3-analyzer-build.2026-08-22T09-16.md`. They are emitted by an imported `.targets` file rather +than by the compiler, which is why `/p:TreatWarningsAsErrors=true` does not promote them to errors +and the build still exits 0. + +## `/p:Nullable=enable` was NOT supplied + +The command line above contains no `/p:Nullable=enable`. This is deliberate and load-bearing: + +- No project in this repository carries a `` element and there is no + `Directory.Build.props`, so the property is a **solution-wide opt-in** that conscripts every file + which has never adopted the `#nullable enable` pragma. +- Forcing it produced 195 errors in `UtilitiesCS.csproj` on 2026-08-10 against zero errors without it. +- `.github/workflows/ci.yml` omits it deliberately; the command above is character-for-character the + CI step "Build with nullable warnings treated as errors". +- Nullable enforcement in this repository is per-file opt-in via `#nullable enable`, and + `/p:TreatWarningsAsErrors=true` then promotes that file's `CS86xx` diagnostics to errors. + +Command: `grep -c 'nullable enable' QuickFiler/Controllers/QfcExplorerController.cs` +EXIT_CODE: 1 +Output: `0` + +`QuickFiler/Controllers/QfcExplorerController.cs` carries **no** `#nullable enable` pragma, so it does +not participate in nullable flow analysis. This gate therefore imposes no new nullable obligation on +the file this change edits; a failure of this gate in Phase 3, 4, or 5 would indicate a genuine +compiler error introduced by a deletion, not a nullable-annotation debt. + +## `/t:Rebuild` was used + +Command: `grep -c 'Skipping target "CoreCompile"' p0t11-nullable.log` +EXIT_CODE: 1 +Output: `0` + +**Count of `Skipping target "CoreCompile"`: 0 (zero).** The same non-vacuity check as [P0-T10] +applies: `grep -c 'Skipping target'` returns **27**, so the message form is emitted at this verbosity +and a `CoreCompile` skip would have been visible. It was not. `/t:Build` was not used, because +MSBuild's up-to-date check does not invalidate on a command-line `/p:` change and a warm `/t:Build` +would return exit 0 having skipped `CoreCompile` on every project, making the gate unable to fail. + +## Output Summary + +Baseline nullable / type-check build PASSED with **EXIT_CODE 0 and 0 errors** (5 pre-existing +`System.Reactive` `packages.config` warnings, not compiler-emitted, therefore not promoted). +`/p:Nullable=enable` was **not** supplied — it is a solution-wide opt-in that CI omits deliberately +and that produced 195 errors in `UtilitiesCS.csproj` when forced. `/t:Rebuild` **was** used, and the +count of `Skipping target "CoreCompile"` in the captured log is **zero** against 27 other +`Skipping target` lines, so the gate is non-vacuous. `QfcExplorerController.cs` carries no +`#nullable enable` pragma, so this gate adds no nullable obligation to the edited file. diff --git a/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step5-vstest-coverage.2026-08-22T09-16.md b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step5-vstest-coverage.2026-08-22T09-16.md new file mode 100644 index 000000000..037daddb4 --- /dev/null +++ b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/baseline/step5-vstest-coverage.2026-08-22T09-16.md @@ -0,0 +1,202 @@ +# Baseline Toolchain Step 5 — Tests with Coverage (Issue #449, [P0-T12], [P0-T13], [P7-T15]) + +Timestamp: 2026-08-22T09-16 +WORKTREE: `C:\Users\DanMoisan\repos\TaskMaster\.claude\worktrees\agent-a5600546d71e73061` + +Command: +``` +dotnet-coverage collect ` + --output \coverage\baseline-p0t12.cobertura.xml ` + --output-format cobertura ` + --settings coverage.config ` + -- "C:\Program Files\Microsoft Visual Studio\18\Community\Common7\IDE\CommonExtensions\Microsoft\TestWindow\vstest.console.exe" ` + <9 discovered assemblies> ` + /Settings:scripts\vscode\TaskMaster.cli.runsettings ` + /InIsolation ` + /TestCaseFilter:TestCategory!=LiveOutlook +``` +EXIT_CODE: 0 + +The Cobertura report is written to the gitignored `coverage/` directory, NOT into the evidence tree, +per [P0-T12]. No helper script is retained under `evidence/`; the discovery-and-invocation script +lives in the session scratchpad outside the repository. + +## Test-assembly discovery + +Discovery recursed from WORKTREE for `*.Test.dll`, then filtered on the path **suffix after +WORKTREE** — never on the absolute path, because WORKTREE itself lies under `.claude\worktrees\` and +an absolute-path `\.claude\` exclusion would discard every assembly in this tree. Filters: keep +`*\bin\Debug\*`; exclude `*\obj\*`, `*\ref\*`, and `*\.claude\*`. This mirrors +`.github/workflows/_mstest-coverage.yml`, which filters on `\bin\Debug\` and excludes `\obj\` and +`\ref\`, with the `\.claude\` suffix exclusion added on top. + +Raw matches: **18**. Retained after filtering: **9**. + +``` +QuickFiler.Test\bin\Debug\QuickFiler.Test.dll +SVGControl.Test\bin\Debug\SVGControl.Test.dll +Tags.Test\bin\Debug\Tags.Test.dll +TaskMaster.Test\bin\Debug\TaskMaster.Test.dll +TaskTree.Test\bin\Debug\TaskTree.Test.dll +TaskVisualization.Test\bin\Debug\TaskVisualization.Test.dll +ToDoModel.Test\bin\Debug\ToDoModel.Test.dll +UtilitiesCS.Test\bin\Debug\UtilitiesCS.Test.dll +VBFunctions.Test\bin\Debug\VBFunctions.Test.dll +``` + +`/InIsolation` was supplied. Without it each assembly's `app.config` binding redirects are ignored +and roughly 1,695 phantom failures appear with empty messages and sub-millisecond durations, +surfacing as a Moq `TypeInitializationException` via `System.Threading.Tasks.Extensions`. No such +mass failure occurred, so the flag took effect and no test was modified. + +## Test counts + +``` +Test Run Successful. +Total tests: 6437 + Passed: 6437 + Total time: 49.6296 Seconds +``` + +| Metric | Baseline value | +| --- | --- | +| Total | **6437** | +| Passed | **6437** | +| Failed | **0** | +| Skipped | **0** | + +`vstest.console.exe` prints `Failed:` and `Skipped:` summary lines only when those counts are +non-zero; neither line is present, and the run is reported `Test Run Successful`, so both counts are +zero. A `grep -c -i failed` over the log returns 7, and all 7 are test METHOD NAMES containing the +word (for example `Passed FailedFactoryTask_ClosesWithoutLeavingAHostOrCallbackSubscription`), each +on a `Passed` line. There is no failing test at baseline. + +## Coverage values (numeric) + +Read from `coverage\baseline-p0t12.cobertura.xml`. + +| Value | Baseline | +| --- | --- | +| Repo-wide root `line-rate` | `0.8532899236682991` = **85.3290 %** | +| Root `lines-covered` / `lines-valid` | 155,943 / 182,755 | +| `QuickFiler` package `line-rate` | `0.8091631603553062` = **80.9163 %** | +| `QfcExplorerController` figure | **ABSENT FROM THE REPORT** (see [P0-T13] below) | + +Full per-package line rates at baseline, recorded so the `QuickFiler` figure is auditable in context: + +| Package | line-rate | % | +| --- | --- | --- | +| QuickFiler.Test | 0.9636752136752137 | 96.3675 | +| SVGControl.Test | 0.8831710709318498 | 88.3171 | +| Tags.Test | 0.9706122448979592 | 97.0612 | +| TaskMaster.Test | 0.9501936912008855 | 95.0194 | +| TaskTree.Test | 1 | 100.0000 | +| TaskVisualization.Test | 0.9686780285582681 | 96.8678 | +| ToDoModel.Test | 0.8302401746724891 | 83.0240 | +| UtilitiesCS.Test | 0.9782362657530894 | 97.8236 | +| VBFunctions.Test | 1 | 100.0000 | +| **QuickFiler** | **0.8091631603553062** | **80.9163** | +| UtilitiesCS | 0.8957463976945245 | 89.5746 | +| TaskVisualization | 0.8984326018808777 | 89.8433 | +| log4net | 0.3019265926030094 | 30.1927 | +| Mono.Reflection | 0.39303482587064675 | 39.3035 | +| SVGControl | 0.47303128371089537 | 47.3031 | +| Microsoft.IO.RecyclableMemoryStream | 0 | 0.0000 | +| ToDoModel | 0.5731056563500534 | 57.3106 | +| Tags | 0.9268929503916449 | 92.6893 | +| TaskMaster | 0.7335945151811949 | 73.3595 | +| TaskTree | 0.9548387096774194 | 95.4839 | +| VBFunctions | 1 | 100.0000 | +| System.Linq.Async | 0.04755332496863237 | 4.7553 | +| System.Interactive | 0.02727272727272727 | 2.7273 | + +Note the denominator caveat that [P7-T9] must carry forward: this direct +`dotnet-coverage collect --settings coverage.config` invocation does not apply the effective-config +test-assembly `ModulePath` exclusion that `scripts/vscode/Invoke-MSTestWithCoverage.ps1` derives, so +the nine `*.Test` packages are IN this denominator. The absolute repo-wide figure is therefore not +directly comparable to the 80% helper gate at +`scripts/vscode/Invoke-MSTestWithCoverage.Helpers.ps1:487-489`. The baseline and post-change runs use +the identical method, so the DELTA is sound even though the absolute value is not the gated figure. + +### Coverage-seam method for the `QfcExplorerController` figure + +The figure is computed by aggregating **every** Cobertura `` element whose `filename` attribute +ends with the path segment `QuickFiler\Controllers\QfcExplorerController.cs`, summing hit and total +line counts across them (filename separators normalised to `\` before the suffix test). This is +required because `OpenQFItem` is `async`, so the compiler emits its state machine as a separate +`` element with a mangled name, and the lambdas passed to `Task.Run` emit further separate +elements. Reading a single `` element would report a figure for a fragment of the file. The +direct `dotnet-coverage collect` invocation performs no closure post-processing, so those elements +are present in the raw report. + +--- + +## [P0-T13] — The baseline `QfcExplorerController` value is ABSENT FROM THE REPORT, not zero + +Search performed over the Cobertura report: + +Search target (suffix match, separators normalised to `\`): +`QuickFiler\Controllers\QfcExplorerController.cs` +Search scope: every `` element under every `` in +`coverage\baseline-p0t12.cobertura.xml`. + +Search result: +``` +QFCEXPL_MATCHED_CLASS_COUNT=0 +QFCEXPL_LINES_HIT=0 +QFCEXPL_LINES_TOTAL=0 +QFCEXPL_LINE_RATE_PCT=ABSENT-FROM-REPORT (zero matching elements) +``` + +**Recorded baseline value for `QfcExplorerController`: absent.** + +The search matched **zero** `` elements. This is not a coverage of 0%: it is the total absence +of the class from the report. The distinction is material and 0% would be a fabricated figure. A rate +requires a denominator, and the denominator here does not exist — the class contributes no +`` element and no lines at all, so no ratio is defined. + +The cause is the class-level `[ExcludeFromCodeCoverage]` attribute at +`QuickFiler/Controllers/QfcExplorerController.cs:20`, verified present at that exact line in the +merge-base tree: + +``` + 20 [ExcludeFromCodeCoverage] + 21 internal class QfcExplorerController : IQfcExplorerController +``` + +The attribute suppresses every member of the class, including the compiler-generated `async` state +machine and lambda display classes, so the instrumentation emits nothing for the file. [P5-T1] +removes that attribute, which is what brings the class into the report for the first time — and, per +[P7-T10], into the coverage DENOMINATOR for the first time, which is the declared reason a +`QuickFiler` package shortfall would be an expected rather than an anomalous outcome. + +--- + +## [P7-T15] — No Python toolchain step was run, because none exists + +Command: `ls -d scripts/dev_tools` -> EXIT_CODE 2, +`ls: cannot access 'scripts/dev_tools': No such file or directory` +Command: `ls -1 pyproject.toml poetry.lock` -> EXIT_CODE 2, both absent +Command: `git ls-files "*.py"` -> EXIT_CODE 0, 2 files, both inside +`docs/features/archive/2026-07-18-stale-app-config-binding-redirects-354/` + +There is no `scripts/dev_tools/` directory (the only similar path is `scripts/dev-tools/`, hyphenated, +holding one PowerShell script) and no Poetry manifest. The importable package `scripts.dev_tools` +does not exist and there is no Poetry environment to run it in, so any skill step naming +`poetry run python -m scripts.dev_tools.*` is **unrunnable by absence**. It is recorded here as such. +No result is fabricated for it and it is not silently omitted. C# coverage in this plan is collected +by `dotnet-coverage` and read from the Cobertura report; no Python coverage runner exists here to +consume a coverage-target argument, which is why no task in this plan states one. See +`environment-preconditions.2026-08-22T09-16.md` finding (a) for the full verification set. + +--- + +## Output Summary + +Baseline suite: **6437 total, 6437 passed, 0 failed, 0 skipped**, EXIT_CODE 0, 49.63 s, across 9 +discovered test assemblies with `/InIsolation` and `/TestCaseFilter:TestCategory!=LiveOutlook`. +Coverage: repo-wide root line rate **85.3290 %** (155,943 / 182,755); `QuickFiler` package line rate +**80.9163 %**; `QfcExplorerController` **absent from the report** — zero matching `` elements, +because the class-level `[ExcludeFromCodeCoverage]` at line 20 suppresses every member, so "absent" is +the correct baseline value and "0%" would be fabricated. No Python toolchain step was run because +none exists in this repository; that absence is recorded rather than fabricated or skipped. diff --git a/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/other/d4-using-hygiene-rationale.2026-08-22T09-16.md b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/other/d4-using-hygiene-rationale.2026-08-22T09-16.md new file mode 100644 index 000000000..1db1968d2 --- /dev/null +++ b/docs/features/active/2026-08-07-quickfiler-explorer-controller-latent-defects-449/evidence/other/d4-using-hygiene-rationale.2026-08-22T09-16.md @@ -0,0 +1,134 @@ +# D4 — `using`-Directive Removal Is HYGIENE, Not a Gate Fix (Issue #449, [P4-T3]) + +Timestamp: 2026-08-22T09-16 +WORKTREE: `C:\Users\DanMoisan\repos\TaskMaster\.claude\worktrees\agent-a5600546d71e73061` + +Command: see the per-claim `Command:` lines below. +EXIT_CODE: 0 + +## Classification + +The removal of the ten orphaned `using` directives from +`QuickFiler/Controllers/QfcExplorerController.cs` is **HYGIENE**, not a gate fix. An orphaned `using` +directive fails **neither** gate in this repository. Nothing in the analyzer build or the nullable +build would have reported these directives, and the file was green on `main` while carrying all +sixteen of them. + +This classification matters because it fixes the burden of proof. Since no gate flags an orphaned +directive, no gate confirms that a removal was safe either. The safety argument therefore rests +entirely on the self-verifying property in [P4-T4], described below. + +## Reason 1 — `IDE0005`'s analyzer is not wired into these projects + +Command: `grep -n '