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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 24 additions & 32 deletions .claude/agent-memory/atomic-executor/MEMORY.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,21 @@ leaves outputs cleaned, so follow it with a passing analyzer Rebuild to restore
any coverage run; and `Invoke-VSBuild.ps1` also runs `Sync-PackageReferences.ps1` on every
invocation, so verify `git status` afterwards.

**This is NOT specific to `Invoke-VSBuild.ps1`.** Re-measured 2026-08-08 (#505 preflight) with a
direct `MSBuild.exe` call — the exact `CLAUDE.md` analyzer command
(`TaskMaster.sln /t:Build /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true
/p:EnforceCodeStyleInBuild=true`) run immediately after a plain `/t:Build` returned `EXIT_CODE=0`
with **18 `Skipping target "CoreCompile"` notices and 0 `csc.exe` invocations**. The analyzer gate
therefore analyzed nothing. Measure it with
`/fl /flp:"logfile=<log>;verbosity=normal"` then count `Skipping target "CoreCompile"` and
`csc.exe` occurrences — `csc.exe = 0` is the unambiguous vacuity signal.

**Plan-review consequence:** in any final-QC phase the analyzer step almost always follows an
earlier build of the same tree (and, on a loop restart, follows the type-check `/t:Rebuild`), so a
`/t:Build` analyzer gate is vacuous by construction. Require `/t:Rebuild` for the analyzer gate, or
require the acceptance artifact to record a non-zero `csc.exe`/`CoreCompile` count for
`TaskMaster` and `TaskMaster.Test`. CI does not hit this because it always starts from a clean
checkout; `ci.yml` states the same rationale in a comment on its type-check step.

Related: [[project_repo_sdk_and_nullable_rebuild]], [[project_364_nullable_gate_preexisting_blockers]],
[[project_vs18_build_toolchain_paths]].
[[project_vs18_build_toolchain_paths]], [[project_nullable_build_gate_is_vacuous_incremental]].
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
name: koverage-cobertura-postprocessing-shape
description: Invoke-MSTestWithCoverage.ps1 post-processes the Cobertura dump - backslash filenames, classes already merged per file, test packages stripped, root attributes recomputed - so per-file coverage queries pinned to forward slashes or to summing sibling <class> nodes silently match nothing
metadata:
type: project
---

`scripts/vscode/Invoke-MSTestWithCoverage.ps1` does not emit a raw `dotnet-coverage` Cobertura
dump. Before writing `-CoverageOutput` it calls `ConvertTo-KoverageCoberturaXml`
(`Invoke-MSTestWithCoverage.Helpers.ps1`) with **no `-PathSeparator` argument**, so four
transformations apply:

1. **`filename` uses `\`, not `/`.** `-PathSeparator` defaults to
`[System.IO.Path]::DirectorySeparatorChar`, which is `\` on Windows. Attributes read
`TaskMaster\Ribbon\EngineToggleStateCoordinator.cs`. A query pinned to
`TaskMaster/Ribbon/...` matches **zero** rows and a coverage gate built on it reports 0 or
nothing while appearing to run.
2. **`<class>` nodes are already merged per file.** `Merge-CoberturaClassesByFilename` collapses
the `<Method>d__N` async state-machine classes and `<>c` closure classes into one `<class>` per
filename and rewrites its `line-rate`/`branch-rate`. Read that attribute directly; the advice to
"sum `lines-covered`/`lines-valid` across all `<class>` elements sharing the filename" applies
only to an unprocessed raw dump.
3. **Test packages are stripped.** `Get-KoverageProjectAllowlist` enumerates every non-`*.Test`
`*.csproj`/`*.vbproj`/`*.fsproj` assembly name under the repo root; packages outside that set are
removed from `<packages>`. First-party production projects (including `TaskMaster`) are in.
4. **Root `<coverage>` attributes are recomputed** after stripping (`line-rate`, `branch-rate`,
`lines-covered`, `lines-valid`, `branches-covered`, `branches-valid`), so the headline figure is
already a production-only denominator — do not re-derive it.

Also: `$repoRoot` is `Resolve-Path "$PSScriptRoot\..\.."`, i.e. the **worktree** root, and the
`*.Test.dll` glob applies no `\.claude\` filter, so the script is safe to run from an agent
worktree rooted under `.claude/worktrees/`. It hard-fails if `vswhere.exe`, the vswhere-resolved
`Common7\IDE\Extensions\TestPlatform\vstest.console.exe`, or a global `dotnet-coverage` is missing —
note that vstest path differs from the `CommonExtensions\Microsoft\TestWindow` one used for direct
`vstest.console.exe` calls.

Related: [[project_coverage_delta_reproduce_baseline_counting_method]],
[[project_csharp_canonical_coverage_artifact_conversion]],
[[project_dotnet_coverage_denominator_nondeterminism]].
Original file line number Diff line number Diff line change
@@ -1,10 +1,23 @@
---
name: missing-vsto-runtime-breaks-baseline-gates
description: On this host the VSTO Office Tools runtime assemblies are absent, so the analyzer and nullable solution builds fail with 4x CS0234 in TaskMaster/ThisAddIn.Designer.cs and UtilitiesCS.Test/TaskMaster.Test never build - collapsing repo-wide coverage
description: HISTORICAL (2026-08-04) - absent VSTO Office Tools assemblies once broke the solution build and collapsed repo-wide coverage; NOT reproducing as of 2026-08-08, so always re-verify by building before citing this
metadata:
type: project
---

> **RE-VERIFIED 2026-08-08 (issue #505 worktree) — DOES NOT REPRODUCE.** The two named
> assemblies (`Microsoft.Office.Tools.Common.v4.0.Utilities`,
> `Microsoft.Office.Tools.Outlook.v4.0.Utilities`) are still absent from the GAC, the VS
> install, and `Common Files\Microsoft Shared\VSTO` — yet
> `msbuild TaskMaster.sln /t:Build /p:Configuration=Debug "/p:Platform=Any CPU"` after
> `nuget restore TaskMaster.sln` completed **EXIT 0 with zero errors** and produced all
> 18 project outputs, including `TaskMaster.dll` and all **9** `*.Test.dll`. The
> `Microsoft.Office.Tools.*` (non-`v4.0.Utilities`) assemblies present in
> `C:\Windows\Microsoft.NET\assembly\GAC_MSIL` are apparently sufficient for
> `ThisAddIn.Designer.cs` to bind. **Never assert the CS0234/6-assembly failure below from
> memory — build first.** Treat the text that follows as a record of one historical
> environment state.

`msbuild TaskMaster.sln` (both the analyzer gate and the nullable gate) fails on this machine
with 4 `CS0234` errors in `TaskMaster/ThisAddIn.Designer.cs`, naming
`Microsoft.Office.Tools.Outlook.OutlookAddInBase`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ Building/testing the TaskMaster net48 VSTO solution in a fresh worktree on this
- Format: csharpier is a local tool v1.x → `dotnet csharpier check .` / `dotnet csharpier format <files>` (subcommands; bare `dotnet csharpier .` is v0 syntax). Needs `DOTNET_ROOT=.dotnet-sdk` on PATH.
- Coverage: `dotnet-coverage` global exe. It re-parses the command string and splits on spaces, so a quoted `"C:\Program Files\...vstest.console.exe"` breaks with `'C:\Program' ... parameter is incorrect`. Use the `--` separator form: `dotnet-coverage collect --output OUT --output-format cobertura -- "$VSTEST" "UtilitiesCS.Test\bin\Debug\UtilitiesCS.Test.dll" /InIsolation`. Cobertura `<class>`/`<method>` elements put `line-rate`/`branch-rate` BEFORE `name`.
- Nullable gate (`/p:Nullable=enable /p:TreatWarningsAsErrors=true`): incremental `/t:Build` after an analyzer build is a no-op (0/0). A genuine `/t:Rebuild` surfaces ~34 distinct pre-existing nullable errors confined to vendored `SVGControl.csproj` (see [[project_repo_sdk_and_nullable_rebuild]]); first-party UtilitiesCS/UtilitiesCS.Test are 0. Rebuild-with-dependencies double-reports (34→68) under parallel; use `-m:1` for distinct counts.
- `pwsh -NoProfile -Command` tokenization for MSBuild switches (measured 2026-08-08): `/p:Platform='Any CPU'` and `/flp:'logfile=<abs>.log;verbosity=normal'` each collapse to ONE argument with the quotes stripped, and the embedded `;` does NOT split the pwsh statement — the plan-style quoting is safe as written. `/fl /flp:logfile=...` needs the target directory to already exist. Inside a pwsh `-Command` string use `| Out-Null`, never `> /dev/null` (pwsh resolves it as the literal path `C:\dev\null` and the whole statement dies).
- Under coverage instrumentation ~17 Deedle/DataFrame/ETL tests (FromDefaultFolder_*, FromArray2D_*, DeedleDoodles, etc.) flake; they pass with 0 failures when instrumentation is off. Pre-existing, unrelated to Folder scoring.
1 change: 1 addition & 0 deletions .claude/agent-memory/atomic-planner/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
- [Repo-wide csharpier format breaks zero-diff ACs](csharpier-repowide-format-breaks-zero-diff-acs.md) — scope the mutating pass to the plan's own path list; keep `check .` read-only; re-verify the zero-line diff AFTER formatting
- [Embedded-resource fail-proof needs a rebuild gate](embedded-resource-failproof-rebuild-gate.md) — edit → rebuild → assert embedded bytes → `[expect-fail]` run; skipping the assert makes the fail-proof itself vacuous
- [#503 ribbon readiness plan seams](project_503_ribbon_readiness_plan_seams.md) — RibbonViewer 487/500 forces a 26-member region move; 6+4 Compile entries; compile-time red + dossier; #504-#508 already promoted
- [#505 toggle-state guards plan seams](project_505_toggle_state_guards_plan_seams.md) — runtime red (no dossier); R4 red between catalog and XML tasks; raw cobertura to gitignored coverage/; #522 CI type-check cmd; manual-verification kind accepted
- [CSharpier gate: format not pipe-files](csharpier-format-not-pipe-files-gate.md) — formatting tasks must use `csharpier format` + scoped `csharpier check` exit 0; `pipe-files` is stdout-only/non-enforcing and masked a 500-line overflow in #400
- [#400 partial-class headroom placement](project_400_partial_class_headroom_placement.md) — put new coverage cases in existing `.Part2.cs` `[TestClass] partial` files to keep the 17-class filter/count assertions stable
- [Manager AsyncLazy shared seam](project_manager_asynclazy_shared_seam.md) — Globals.AF.Manager is shared across all classifier subsystems; use a key-specific accessor, never retype the dictionary value for one key
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
name: project-505-toggle-state-guards-plan-seams
description: "#505/#506/#518 bundled full-bug plan facts: runtime red (not compile red), R4 red captured between catalog and XML tasks, coordinator+EngineToggleCatalog seam, raw cobertura to gitignored coverage/, manual-verification evidence kind accepted"
metadata:
type: project
---

Plan seams for the #505/#506/#518 bundled `full-bug` delivery (plan.2026-08-08T19-22.md in feature 2026-08-08-ribbon-engine-toggle-state-guards-505).

**Why:** three causally coupled defects in `RibbonViewer.EngineCommands.cs`; the spec pins a two-guard-shape split (4 toggle sites -> new `EngineToggleStateCoordinator`, 6 command sites -> existing `RunEngineCommandAsync` gate + 6 new `EngineCommandCatalog` entries + XML `getEnabled`).

**How to apply:**
- Unlike #503 (compile-time red + exception dossier), #505's red is a RUNTIME red: reflection shape-pin tests compile against pre-fix code and fail on execution. No fail-before exception dossier needed when a genuine failing run exists. Reflection-invoke keeps R2 compiling across the signature change.
- R4 (existing catalog-derived XML set-equality tests) has no test-code change; its red is captured as a dedicated `[expect-fail]` run task BETWEEN the catalog-extension task and the XML-edit task. "Land atomically" means same commit, not same task — a captured red between the two edits satisfies AC-15 and the atomicity risk.
- `EngineCommandCatalogTests.ControlIds_ContainsExactlyTheEightEngineBackedControlIds` hard-codes the 8-entry set; extending the catalog requires renaming/extending it (research §8 predicted this).
- The `RibbonViewer(RibbonController)` public ctor is field-assignment-only (`Controller.Try` is inside an uninvoked lambda) — safe to construct in a unit test; `RibbonController()` parameterless ctor exists and #507 tests already use it.
- Raw Cobertura goes to the gitignored `coverage\` dir (`.gitignore` `coverage/*`), never under docs/features (81 MB incident) and never `artifacts/csharp/coverage.xml` (SubagentStop hook hard-codes an 85% floor there vs the real 80% policy).
- `evidence/manual-verification/` is an accepted evidence kind (spec AC-22 requires it verbatim; #503 executed with it; not on the forbidden list).
- Type-check gate is CI's `msbuild /t:Rebuild /m ... /p:TreatWarningsAsErrors=true` WITHOUT `/p:Nullable=enable` (issue #522, known-defective); cite #522 in every type-check task so the executor does not "correct" it back to the CLAUDE.md form.
- `Invoke-MSTestWithCoverage.ps1` applies no `\.claude\` filter and derives repoRoot internally, so it is safe in an agent worktree rooted under `.claude\worktrees\`; expected assembly count 9, and 0 discovered = filter bug, never an empty suite.

Related: [[project-503-ribbon-readiness-plan-seams]], [[async-state-machine-coverage-aggregation]], [[csharpier-format-not-pipe-files-gate]].
1 change: 1 addition & 0 deletions .claude/agent-memory/feature-review/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,4 @@
- [mandated nullable solution gate is vacuous](project_nullable_build_gate_is_vacuous.md) — #503: /t:Build with only /p: changes skips CoreCompile so the gate cannot fail; force /t:Rebuild on the changed project and attribute the errors by file
- [null-conditional fix relocates NRE, check callers](project_null-conditional-fix-relocates-nre-check-callers.md) — #507: `Globals.Engines`->`Globals?.Engines` matched sibling `SB` precedent and passed full evidence, but all 11 real `RibbonViewer.cs` callers are unguarded, so the NRE just moves one frame later; grep every call site before crediting a throw->null fix with resolving the reachable crash
- [coverage hook needs label+coverage+PASS/FAIL on one line](project_coverage-hook-label-plus-verdict-same-line-507.md) — #507 R1: `Test-LanguageCoverageRow` requires the language label, a coverage keyword, and PASS/FAIL all on the SAME line, and rejects any label+coverage line carrying a banned narrowing word anywhere; dot-source and simulate before finalizing, don't trust a wrapped narrative paragraph
- [505 coordinator prime/toggle race (CR-1)](project_505-coordinator-prime-toggle-race.md) — EngineToggleStateCoordinator lazy prime can overwrite a fresher toggle write and stick stale (no re-prime); Major non-blocking, TryAdd fix + promotion recommended — check status in later ribbon reviews
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
name: 505-coordinator-prime-toggle-race
description: 'EngineToggleStateCoordinator (#505 review CR-1): lazy prime can overwrite a fresher toggle-written cache value and persist stale for the session; recommended TryAdd fix, promotion recommended — check status in any later ribbon/coordinator review'
metadata:
type: project
---

The #505/#506/#518 review (2026-08-08, PASS with 0 blocking) recorded one Major non-blocking finding, CR-1: in `TaskMaster/Ribbon/EngineToggleStateCoordinator.cs`, `ApplyPrimeAsync` writes `_pressedState[engineName] = active` unconditionally. An in-flight prime that read `EngineActiveAsync` before a toggle flipped the setting can write its stale value AFTER `ExecuteToggleAsync` wrote the fresh one; the successful prime task stays registered in `_primeTasks` (`ContainsKey` guard), so no re-prime ever occurs and the stale display persists until the next click. The spec's "every cache write is followed by an invalidation, so the UI converges" assumption fails in this interleaving.

**Why:** display-only, narrow window (prime resolving concurrently with a click during initial config load), strictly better than merge-base behavior, so dispositioned non-blocking with a recommended fix (`_pressedState.TryAdd` in the prime; residual toggle-vs-toggle double-click case needs write versioning) and a promotion recommendation. Also recorded: CR-2 canceled prime ignored by `CompletePrime` (`Task.Exception` null for Canceled -> marker never removed, key never re-primes, nothing logged); CR-3 the 2 uncovered defensive-guard lines are trivially testable with the existing harness.

**How to apply:** in any later review touching `EngineToggleStateCoordinator`, the ribbon toggle surface, or a follow-up issue citing this race, check whether CR-1/CR-2 were promoted or fixed before re-deriving the interleaving. Full analysis: `docs/features/active/2026-08-08-ribbon-engine-toggle-state-guards-505/code-review.2026-08-08T21-59.md`.
3 changes: 3 additions & 0 deletions .claude/agent-memory/orchestrator/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,6 @@
- [C# coverage has two denominators](csharp-coverage-denominator-two-figures.md) — filtered first-party ~85.9% clears the gate, unfiltered ~70.4% doesn't; measure before trusting the "85% floor trap" note, and project Cobertura to JaCoCo
- [Preflight catches vacuous gates](preflight-catches-vacuous-gates.md) — MCP plan validator ok:true is not enough; executor preflight found 6 gates that passed while verifying nothing (empty diffs, unsatisfiable acceptance, uncompilable assertions)
- [Bash tool mangles MSBuild switches](bash-tool-mangles-msbuild-switches.md) — /m becomes M:/ (MSB1008); run C# tools via pwsh -NoProfile with absolute paths, after nuget restore; carries verified VS18 tool locations
- [Analyzer gate is vacuous without /t:Rebuild](msbuild-analyzer-gate-vacuous-without-rebuild.md) — /t:Build after any earlier build skips CoreCompile on all 18 projects and returns EXIT 0 having compiled NOTHING; demand /t:Rebuild + an /fl log + a csc.exe-count > 0 acceptance
- [Aggregate vstest crash: isolate per assembly](vstest-aggregate-crash-isolate-per-assembly.md) — "Test host process crashed / Total tests: Unknown" is environmental (QuickFiler WinFormsPumpHost, #511); re-run the 9 assemblies with /InIsolation for the real verdict, and check the csproj graph before owning the blame
- [One executor per worktree](one-executor-per-worktree.md) — a stale checkpoint is NOT a dead delegation (executors don't own it); never launch a second executor into a live worktree; recover by re-verifying the committed tree, not by adjudicating artifact provenance
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
name: msbuild-analyzer-gate-vacuous-without-rebuild
description: An msbuild /t:Build analyzer gate that follows any earlier build of the same tree compiles NOTHING and returns EXIT 0 — always use /t:Rebuild plus a csc.exe-count non-vacuity proof
metadata:
type: project
---

MSBuild's legacy non-SDK up-to-date check is **timestamp-based and does not invalidate on a `/p:`
change**. So an analyzer gate written as

```
msbuild TaskMaster.sln /t:Build ... /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true
```

that runs after any earlier build of the same tree skips `CoreCompile` for every project and reports
`EXIT 0` having analyzed **nothing**. Measured on #505: **18 `Skipping target "CoreCompile"`, 0
`csc.exe` invocations, EXIT 0.** The same command as `/t:Rebuild` gives 18 `csc.exe` invocations.

This is vacuous *by construction* in any plan where the analyzer step follows an implementation-phase
build or a previous QC-loop iteration — which is essentially every plan. CI escapes it only because a
runner starts from a clean checkout; `.github/workflows/ci.yml:106-112` states this exact rationale
for its own `/t:Rebuild` type-check step.

**How to apply.** In every plan's analyzer and baseline-analyzer task:

1. Use `/t:Rebuild`, never `/t:Build`.
2. Add a file log and read a count back out of it as a mandatory acceptance condition:
`/fl "/flp:logfile=<REPO>\coverage\analyzer.log;verbosity=normal"`, then count
`PathToTool=.*csc\.exe` matches. **The count must be > 0 and must include the projects you
touched.** `EXIT_CODE: 0` with a zero `csc.exe` count is a FAILED gate, not a passing one.
3. Apply the same treatment to the *baseline* task, or the final comparison silently depends on
execution order.
4. `/nodeReuse:false` is worth adding: `/m` parallel rebuilds leave ~17 resident MSBuild worker
processes that saturate the box and destabilize the subsequent test run.

**Why this matters beyond analyzers:** it is the general shape of a vacuous gate — a command whose
exit code is green because it did no work. Pair it with
[[preflight-catches-vacuous-gates]]: the structural MCP plan validator returns `ok:true` on a plan
full of these; only the `atomic-executor` preflight, *running the command and measuring*, catches
them. On #505 the preflight caught this empirically and it was the single highest-value finding of
the run.

Tool paths and the Bash-mangling caveat: [[bash-tool-mangles-msbuild-switches]].
Loading
Loading