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
1 change: 1 addition & 0 deletions .claude/agent-memory/atomic-executor/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@
- [Async state machine emits no `<method>` element](project_async_state_machine_emits_no_method_element.md) — per-method union is empty; `.//line` double-counts
- [First-party coverage denominator (#197)](project_coverage_firstparty_denominator_method.md) — count per-`<line>`
- [dotnet-coverage denominator nondeterminism](project_dotnet_coverage_denominator_nondeterminism.md) — 47% vs 81% from
- [Failed coverage run leaves RAW Cobertura](project_failed_coverage_run_leaves_raw_unprocessed_cobertura.md) — throw precedes post-processing; 0.70 raw vs 0.85 processed
- [Koverage Cobertura post-processing shape](project_koverage_cobertura_postprocessing_shape.md) — passing run = proce
- [C# canonical coverage artifact conversion](project_csharp_canonical_coverage_artifact_conversion.md) — hook reads JaCoC
- [Cobertura runsettings `<Attributes>` override](project_cobertura_runsettings_attributes_override.md) — silently disable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ would have failed the feature for files it never touched — including
**Why:** two-dot `<base>..HEAD` is "in HEAD, not in base", which after a merge is precisely the
branch's own additions. A diff against a pre-merge SHA has no such property.

**Three-dot does NOT protect you.** Confirmed again on #656, 2026-09-01, where the plan wrote every
footprint gate as `<PINNED_SHA>...HEAD`. When the pinned SHA is an *ancestor* of HEAD,
`merge-base(PINNED, HEAD)` is the pinned SHA itself, so the three-dot form silently degenerates to
the two-dot diff against it and inherits the whole defect. Measured: `<PINNED>...HEAD` = **299**
paths including 9 under `QuickFiler/`+`QuickFiler.Test/` and a `.csproj`, versus
`origin/main...HEAD` = **10**. Four gates asserting "exactly the single line" and "both outputs
empty" were unsatisfiable as written. The tell is that the plan's base predates the branch's
reconciliation merge — check `git merge-base <PINNED> HEAD` against `<PINNED>` before trusting any
`...` gate.

**How to apply:** run the command the task names AND the `<base>..HEAD` form, record both, and make
the `<base>..HEAD` classification the authoritative one with an explicit attribution column for the
merge-induced rows. Do not silently substitute — the task text is still the plan of record. Same
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
name: failed-coverage-run-leaves-raw-unprocessed-cobertura
description: A non-zero Invoke-MSTestWithCoverage run leaves coverage.cobertura.xml RAW (absolute class/@filename, no root lines-covered/lines-valid, all-modules line-rate ~0.70 not ~0.85); recover in memory with ConvertTo-KoverageCoberturaXml
metadata:
type: project
---

`scripts/vscode/Invoke-MSTestWithCoverage.ps1` throws on a non-zero inner vstest exit code
(`:235-237`) BEFORE the Koverage post-processing block (`:339-343`). `dotnet-coverage` has
already written `coverage\coverage.cobertura.xml` by then, so the file exists but is the RAW
collector output:

- `class/@filename` is still an ABSOLUTE path, so a lookup keyed on the repo-relative
`QuickFiler\Viewers\Foo.cs` form finds nothing.
- The root `coverage` node carries no `lines-covered` / `lines-valid`; those are set only at
`Invoke-MSTestWithCoverage.Helpers.ps1:442-445`.
- The root `line-rate` is the ALL-MODULES rate, not the first-party allowlist rate. Measured
gap: issue #608's failed run recorded `line-rate="0.7017"` / `lines-valid="81570"`, against
~0.853 / ~64k on processed runs of comparable trees. Reading the raw number as a coverage
regression is a false conclusion.

**Why:** the throw is not caught (only a `finally` that removes derived settings), so it
propagates past `Set-Content` and nothing rewrites the document.

**How to apply:** when a coverage run exits non-zero but you still need the figures, do not read
the emitted file directly. Dot-source `scripts/vscode/Invoke-MSTestWithCoverage.Helpers.ps1` and
convert in memory:
`ConvertTo-KoverageCoberturaXml -XmlContent (Get-Content coverage\coverage.cobertura.xml -Raw -Encoding UTF8) -RepoRoot (Get-Location).Path`
(both parameters are mandatory strings), then read the six values from the converted document.
Note the same ordering means `Assert-CoberturaLineCoverageThreshold` never runs on a failed run,
so a sub-80% raw rate is not itself the cause of the non-zero exit.

Related: [[project_dotnet_coverage_denominator_nondeterminism]],
[[project_koverage_cobertura_postprocessing_shape]].
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 @@ -26,6 +26,7 @@
- [#644 cycle 2](project_644_cycle2_sweep_gate_evasion_seams.md) — rewording out of a detector's match set is gate evasion; SHA-256 pair
- [#644 PA-7](project_644_pa7_redaction_plan_seams.md) — untracked audit artifact still enters main; name-status diff blind to it
- [#648](project_648_ungated_static_swap_plan_seams.md) — lines-valid equality UNSATISFIABLE, use 5% tolerance; `git tag` not idempotent
- [#656](project_656_closecompleted_guard_plan_seams.md) — no TestCaseFilter override in either wrapper; wrapper writes no TRX; class nodes lack lines-covered/lines-valid; TestResults\ must be created first
- [#680](project_680_menu_mode_plan_seams.md) — HostTests.cs 499 not 500; TRX 5-shape identifiers, `grep -a`; exact line arithmetic

## Plan-structure traps
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
name: project-656-closecompleted-guard-plan-seams
description: Planning seams found authoring the #656 CloseCore completed-close guard plan — wrapper scripts accept no TestCaseFilter, the coverage runner prints no percentage, and MSBuild non-vacuity is better proved by assembly mtimes than by log text
metadata:
type: project
---

Seams found while authoring the atomic plan for issue #656 (`BreadcrumbDropDownOpenCoordinator.CloseCore` completed-close guard).

**Why:** each of these would have produced an unsatisfiable or vacuous acceptance condition, and none is visible from the plan text alone.

**How to apply:** re-check these before authoring any TaskMaster C# plan that runs a scoped test, asserts a coverage number, or asserts MSBuild non-vacuity.

- **Neither test wrapper accepts a `TestCaseFilter` override.** `scripts/vscode/Invoke-MSTest.ps1:54` and `scripts/vscode/Invoke-MSTestWithCoverage.ps1:76` both hardcode `'/InIsolation', '/TestCaseFilter:TestCategory!=LiveOutlook'` with no parameter to extend it. A `[expect-fail]` red run scoped to one test therefore requires a **direct** `vstest.console.exe` invocation that reproduces both protections explicitly — `/InIsolation` plus a `TestCategory!=LiveOutlook&FullyQualifiedName~<name>` conjunction. That is not the prohibited "bare" call; editing the wrapper would breach a footprint boundary.
- **`Invoke-MSTestWithCoverage.ps1` prints no coverage percentage on a successful run.** `Assert-CoberturaLineCoverageThreshold` (`Invoke-MSTestWithCoverage.Helpers.ps1:459-491`) only *throws* below 80%. Never assert a printed percentage. Read `line-rate` from `coverage/coverage.cobertura.xml` instead. That path is git-ignored (`.gitignore:144`, `coverage/*`).
- **Its `EXIT_CODE: 0` strictly implies zero failed tests**, because `Invoke-MSTestWithCoverage.ps1:235-237` throws when the inner vstest exit code is non-zero. That fact is what lets a plan assert zero failures without asserting a console literal.
- **Cobertura `class/@filename` is repo-relative with backslashes**, and classes are merged one node per file (`ConvertTo-KoverageRelativePath` at `Helpers.ps1:50-97`, `Merge-CoberturaClassesByFilename`). Per-file coverage lookups must use `QuickFiler\Viewers\X.cs`, not a forward-slash or absolute form.
- **Prove MSBuild non-vacuity with output-assembly `LastWriteTime`, not log text.** Asserting the absence of `Skipping target "CoreCompile"` needs a positive control, and whether that string appears at `Verbosity=normal` is an unobserved assumption. Record a `Gate Start:` wall-clock stamp and assert `bin\Debug\<asm>.dll` `LastWriteTime` is later — that proves recompilation regardless of verbosity.
- **`Select-String` has no `-Recurse` parameter.** Pipe `Get-ChildItem -Recurse -Filter *.cs` into it. And use `@(Select-String ...).Count`, never `(Select-String ...).Count`, so a zero-match result is a number.
- **`Invoke-VSBuild.ps1` is unusable under a footprint boundary** — it runs `Sync-PackageReferences.ps1` over every `.csproj` and rewrites `HintPath` values. Resolve MSBuild through vswhere directly, as `Invoke-Restore.ps1:22-30` does.
- **Bootstrap facts for a fresh agent worktree:** `.dotnet-sdk/` and `packages/` are both absent and both git-ignored (`.gitignore:350` `.dotnet*/`; `.gitignore:191` `**/[Pp]ackages/*`), and `dotnet-tools.json` sits at the worktree ROOT, not under `.config/`. `dotnet-coverage` must also resolve or `Invoke-MSTestWithCoverage.ps1:292-294` throws.
- **Guard-literal replacement was safely zero-hit checkable:** `if (_closeCompleted)` is NOT a substring of `if (_closeCompleted && !hostOpen)` because the searched literal carries the closing parenthesis. Pair it with a rule that the new doc comments must not contain the old literal.
- **Count `_host.` on non-comment lines only** (`^\s*[^/\s].*_host\.`, baseline 5 lines). The AC-19 doc comments would otherwise inflate the count that AC-2's lock-discipline gate reads.

**Round-2 preflight delta (2026-08-31) added four more:**

- **A Cobertura `class` node carries no `lines-covered`/`lines-valid`.** `Invoke-MSTestWithCoverage.Helpers.ps1:374-380` sets exactly `line-rate`, `branch-rate`, `complexity` on a class node; `:442-445` sets `lines-covered`/`lines-valid` on the ROOT `coverage` node only. `GetAttribute` returns an empty string for the missing pair, so a per-file covered/valid gate reading them is unsatisfiable. Derive per-file counts from the class-relative `./lines/line` set (count, and count with `hits > 0`) — the descendant axis double-counts.
- **The coverage wrapper produces NO TRX.** `Invoke-MSTestWithCoverage.ps1:70-76` passes no `/Logger:trx` and `scripts/vscode/TaskMaster.cli.runsettings` declares no logger. Test counts and failing names for a wrapper run must come from the tee'd console log, never from a TRX. Only a direct `vstest.console.exe` call that passes `/Logger:trx` writes one.
- **`TestResults\` does not exist in a fresh worktree**, and neither the msbuild file logger nor `Tee-Object -FilePath` creates a missing parent directory. Add a Phase 0 task that `New-Item -Force`s every subdirectory the plan writes into, before the first task that writes one.
- **A wrapper failure anywhere kills the coverage baseline, not just the test gate.** The throw at `:235-237` precedes the post-processing at `:339-343`, so on a non-zero exit the emitted `coverage\coverage.cobertura.xml` is the RAW document: absolute `class/@filename`, no root `lines-covered`/`lines-valid`. A "record the failure and continue" tolerance branch must say how the baseline coverage numbers are still obtained (dot-source the helpers and run `ConvertTo-KoverageCoberturaXml` in memory) or declare BLOCKED.
- **Anchor a per-file coverage no-regression gate on `lines-valid`, not the rate.** Per-file `lines-covered` drifts by up to four lines between two runs of the same tree (`.claude/agent-memory/orchestrator/coverage-lines-covered-is-nondeterministic.md`), so a near-1.0 baseline rate fails on correct work. Require a reproduction run before failing.

See [[declaration-only-seam-task-for-fail-before]] (not needed here — the test compiles against unmodified production code, so the red is runtime), [[trx-needs-resultsdirectory]], [[absolute-counts-in-shared-files-go-stale]].
1 change: 1 addition & 0 deletions .claude/agent-memory/feature-review/MEMORY.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
- [breadcrumb Close returns before OpenState=false](project_breadcrumb-close-returns-before-openstate-false.md) — #656: reopen-path enumeration doesn't prove `_closeCompleted && IsOpen` unreachable; CompleteClose is queued, not synchronous
- [verify-zero-own-effect-coverage-noise-491](project_verify-zero-own-effect-coverage-noise-491.md) — verify "coverage shortfall is noise": grep both Cobertura XMLs for the assembly; distrust deleted-raw-XML narratives (#491)
- [poshqc-bundled-coverage-artifact-reads-zero](project_poshqc-bundled-coverage-artifact-reads-zero.md) — run_poshqc_test's canonical Pester XML reads 0 covered (invalid capture) -> honest non-blocking FAIL row; 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 call; NF-1 uncovered Helpers.ps1:220; #529-#532 OPEN deliberately
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
---
name: breadcrumb-close-returns-before-openstate-false
description: "#656: BreadcrumbDropDownHost.Close returns true before OpenState=false, so `_closeCompleted && IsOpen` IS occupiable in production; reopen-path enumerations answer a narrower question than they are credited with"
metadata:
type: project
---

`BreadcrumbDropDownHost.Close` (`QuickFiler/Viewers/BreadcrumbDropDownHost.cs:251-254`) returns
`true` after only *scheduling* `CompleteClose`. `CompleteClose` (`:397-411`) is what sets
`OpenState = false`, and it is dispatched via `BreadcrumbDropDownOpenLifetime.ScheduleInvalidating`
-> `ScheduleObserved` -> `RunOnOwnerAsync` -> `_uiOperations.PostAsync`, i.e. queued on the **same**
`BreadcrumbPopupUiOperations` dispatcher the coordinator posts its own work to.

Consequence: `_closeCompleted == true && _host.IsOpen == true` is a state the **production** host can
occupy, with no substituted seam and no reopen.

**Why:** #656's spec and research artifact proved by exhaustive enumeration that no production path
reopens the host without `RequestOpen`/`Invalidate` (only `OpenState = true` is
`BreadcrumbDropDownOpenLifetime.cs:268`, reachable only via `RequestOpen`, which clears the flag).
That enumeration is correct, but it answers "can the host be *reopened* without the entry points?"
The guard `if (_closeCompleted && !hostOpen)` depends on a different proposition — "can both flags be
true at once?" — which the asynchronous close window answers yes. The spec reported the narrower
result as though it settled the broader one, and concluded a rollback would be "observationally
identical on every shipped path". Not established.

Second-order effect: a second `_host.Close` inside that window re-enters the `OpenState == true`
branch, whose `InvalidateAndSchedule` bumps the lifetime generation and makes the *first* scheduled
`CompleteClose` fail its lease check. The second reason wins, and `FinishClose` calls
`_cancelSelection()` only for `Uncommitted` (`:449-451`) — so an Uncommitted-then-ExplicitCommit pair
can drop the selection cancel.

Also: every test fake clears `IsOpen` synchronously inside `Close`
(`BreadcrumbDropDownOpenCoordinatorTests.cs:436-437`), so no test represents this timing. 100%
changed-line coverage and a 91% class branch rate did not touch it.

**How to apply:** when any breadcrumb review credits a reopen-path enumeration with proving a guard
unreachable, check whether the guard's actual predicate is about reopening or about two states
coinciding. Trace `Close` to whatever sets `OpenState = false` and confirm it is synchronous before
accepting the claim. Related: [[verify-the-asserted-evidence-mechanism]],
[[505-coordinator-prime-toggle-race]].
1 change: 1 addition & 0 deletions .claude/agent-memory/task-researcher/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
- [reflective-caller-closure-635](project_reflective_caller_closure_635.md) — #635/#468: removal was THIRTEEN members (`_templateTlp` omitted from AC-16); `GetField(` never searched (172 hits) yet is the only reaching mechanism; QuickFiler is ComVisible(false) (2026-08-29)
- [selectrow-two-families-637](project_selectrow_two_families_637.md) — #637: TWO unrelated SelectRow families (bare grep over-counts ~10x); blanket TryMakeArchiveRelative rejects relative/Trash rows; ButtonOK_Click does NOT rethrow (2026-08-29)
- [analyzer-severity-ceiling-and-runsettings-split](project_analyzer_severity_ceiling_and_runsettings_split.md) — MSTEST0032 is the ONLY rule above suggestion; no .globalconfig; Invoke-MSTest.ps1 docstring names the wrong runsettings (2026-08-31)
- [issue-656-bypass-path-does-not-exist](project_issue_656_bypass_path_does_not_exist.md) — #656 premise FALSE: no bypassing reopen path exists; SR-4's "no IsOpen under _sync" already violated at :112; both nominated owner files at the 500-line cap (2026-08-31)

## Artifact hygiene
- [Never embed absolute host paths](../_shared_no_absolute_host_paths.md) — no `C:\Users\<account>\...`, bare account, or machine name in ANY artifact; use `<repo-root>` / `<user-profile>` / `<user>` / `<host>`. vstest names TRX `<account>_<HOST>_<ts>.trx` by default, so control `/ResultsDirectory:` + `LogFileName=` or rename before citing.
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
name: issue-656-bypass-path-does-not-exist
description: "#656 _closeCompleted residual: the bypassing reopen path the issue assumes does NOT exist; only open transition is BreadcrumbDropDownOpenLifetime.cs:268, reachable only via RequestOpen; the two nominated owner files are at the 500-line cap"
metadata:
type: project
---

Issue #656 (`_closeCompleted` stays stale after a host reopen that bypasses `RequestOpen`/`Invalidate`)
has a **false premise on shipped code**. Verified 2026-08-31 by exhaustive enumeration:

- The repository's ONLY statement that makes the breadcrumb drop-down host open is
`QuickFiler/Viewers/BreadcrumbDropDownOpenLifetime.cs:268` (`_host.OpenState = true;`).
`BreadcrumbDropDownHost.IsOpen => OpenState` is get-only; all four other `OpenState` writes are `false`.
- Closed chain: `RequestOpen (…OpenCoordinator.cs:115, clears the flag at :114) → OpenCoreAsync (:218)
→ BeginOpenCore (:258/:259) → BreadcrumbDropDownHost.Open.cs:22/:37 → :88 →
BreadcrumbDropDownOpenLifetime.cs:67-69 → :243 → :268`. No production caller of
`IBreadcrumbDropDownHost.OpenAsync` exists outside `BeginOpenCore`.
- Native-show family also checked: `ShowPopup` / `_showPopup` / `ShowOwnedPopup` are invoked once each,
downstream of `OpenState = true`; the only `ToolStripDropDown` event subscribed is `Closed`.

**Why:** the issue's "Suspected Cause" asserts the bypassing paths "live in the ItemViewer breadcrumb
lifecycle host surface" (feature 488's files). They do not. That claim came from #501's SR-4 known
limitation being recorded as an ownership hand-off, not from an enumeration.

**How to apply:** treat #656 as latent-correctness hardening, not a user-facing defect. Do NOT site a
fix in `BreadcrumbItemViewerLifecycleCoordinator.cs` (497 lines) or `BreadcrumbDropDownHost.cs`
(498 lines) — both are 2-3 lines from the repo's 500-line cap, so any edit forces a partial split first.
The only sane footprint is `BreadcrumbDropDownOpenCoordinator.cs` (378 lines) plus
`QuickFiler.Test/Viewers/BreadcrumbDropDownOpenCoordinatorTests.Part3.cs` (173 lines; Part2 is 455 and
the primary partial is 463, both too full for a new test).

Two facts that constrain any remedy:

1. SR-4 (`docs/features/active/breadcrumb-coordinator-hub-defects-501/spec.md:426-437`) rejected
`if (_closeCompleted && !_host.IsOpen) return true;` because it reads `_host.IsOpen` under `_sync`.
**Honest qualification:** `RequestOpen` at `…OpenCoordinator.cs:112` ALREADY reads `_host.IsOpen`
under `_sync`, so "never read IsOpen under the lock" is not an invariant the file holds. SR-4's real
objection is adding a *second* instance of a pattern a sibling feature was removing. Don't overstate it.
2. The refinement would be a **no-op in production anyway**: `BreadcrumbUiDispatcher.Dispatch`/`DispatchValue`
run INLINE on the captured boundary (`BreadcrumbUiDispatcher.cs:78-95`, `:166-178`), so
`CompleteClose` → `OpenState = false` executes synchronously inside `_host.Close` before it returns
`true`. `!_host.IsOpen` is therefore already true when the suppression is evaluated.

Bypass test seam already exists and needs no new plumbing: `ControlledHost.SetOpen(bool)` at
`QuickFiler.Test/Viewers/BreadcrumbDropDownOpenCoordinatorTests.cs:407`, already used for exactly this
purpose at `…Part2.cs:349`. No test anywhere references `_closeCompleted` by name or reflection.

Related: [[breadcrumb-navigation-defects-439-440-498-499]], [[issue-469-already-fixed-residual-is-629]].
Loading
Loading