Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
ac069d3
docs(issue-662): scope minor-audit bug folder with acceptance criteria
drmoisan Sep 1, 2026
e25a8df
docs(issue-662): add research and reconcile acceptance criteria
drmoisan Sep 1, 2026
1b1f378
docs(agent-memory): record task-researcher notes from issue 662
drmoisan Sep 1, 2026
d33ed8b
docs(issue-662): author minor-audit atomic plan
drmoisan Sep 1, 2026
e592f0c
docs(agent-memory): record atomic-planner notes from issue 662
drmoisan Sep 1, 2026
9c58963
docs(issue-662): record preflight round 1 delta
drmoisan Sep 1, 2026
ea67540
docs(issue-662): apply preflight round 1 delta to the plan
drmoisan Sep 1, 2026
fb02abe
docs(agent-memory): record atomic-planner revision-round notes for is…
drmoisan Sep 1, 2026
c379ae2
docs(issue-662): record preflight round 2 delta
drmoisan Sep 1, 2026
8f56523
docs(issue-662): apply preflight round 2 delta to the plan
drmoisan Sep 1, 2026
c753337
docs(agent-memory): record atomic-planner round-3 notes for issue 662
drmoisan Sep 1, 2026
db59adf
docs(issue-662): remove the absolute host path from the plan
drmoisan Sep 1, 2026
25a8371
docs(issue-662): record preflight round 3 clearance
drmoisan Sep 1, 2026
0cf4843
docs(agent-memory): record orchestrator notes from issue 662 preparation
drmoisan Sep 1, 2026
0387db7
Merge branch 'main' into bug/efcselectionguard-banner-prefix-arity-an…
drmoisan Sep 1, 2026
ab5c5bf
Merge remote-tracking branch 'origin/main' into bug/efcselectionguard…
drmoisan Sep 1, 2026
63ef2e8
docs(issue-662): correct the stale diff anchor in three plan gates
drmoisan Sep 1, 2026
b08aeb3
docs(issue-662): record Phase 0 baseline evidence
drmoisan Sep 1, 2026
4bc3d71
fix(issue-662): rename the EFC guard's banner prefix and correct the …
drmoisan Sep 1, 2026
8680c0b
chore(issue-662): record Phase 2 toolchain-loop evidence
drmoisan Sep 1, 2026
974f9be
docs(issue-662): record acceptance-criteria verification and final QC…
drmoisan Sep 1, 2026
f7fc348
chore(agent-memory): record issue 662 execution lessons
drmoisan Sep 1, 2026
8a40a58
docs(issue-662): restore XML well-formedness in committed TRX evidence
drmoisan Sep 1, 2026
3167197
docs(issue-662): record feature-review audit artifacts
drmoisan Sep 1, 2026
c038156
chore(agent-memory): record issue 662 review and hygiene lessons
drmoisan Sep 1, 2026
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
24 changes: 24 additions & 0 deletions .claude/agent-memory/_shared_no_absolute_host_paths.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,30 @@ test. On issue #468 that was 946 leaked paths in a single TRX, and 16 committed
**Always substitute case-insensitively, in binary mode**, and verify with a case-insensitive
fixed-string sweep (`grep -I -i -F`) rather than trusting the header.

## Angle-bracket placeholders corrupt XML: escape them, and re-parse after redacting

The four required placeholders are written with angle brackets, and a raw `<` is not legal inside
an XML attribute value. `vstest.console.exe` writes the account and host names into attribute
values in the TRX header (`<TestRun name=`, `runUser=`, `<Deployment runDeploymentRoot=`,
`computerName=`, `storage=`, `codeBase=`). A blind textual substitution therefore leaves every
redacted TRX **not well-formed**, failing to parse at line 2. Issue #662 shipped six corrupt TRX
files this way; the executor applied the plan's hygiene rule exactly as written.

Two rules:
1. When the target is an XML-family file (`.trx`, `.xml`, `.coveragexml`, `.cobertura.xml`),
substitute the **escaped** form — `&lt;user&gt;`, `&lt;host&gt;`, `&lt;repo-root&gt;`,
`&lt;user-profile&gt;`. The parsed attribute value is then still exactly the placeholder, so
redaction is unchanged and the document parses. Blind replacement of the bracketed token is
safe in a TRX because no TRX element is named `user`, `host`, `repo-root` or `user-profile`.
2. **`ResidualMatchCount=0` is not a sufficient hygiene gate.** It measures only that the
identifiers were removed, never that the file it rewrote still parses, so a sweep that corrupts
every XML artifact it touches still reports success. Always add an assertion that every
rewritten XML-family file re-parses.

Related structural gap seen on the same issue: a hygiene sweep that excludes `plan.md` from its
residual scan by path cannot detect a host path reintroduced into the plan, and a sweep that
rewrites only `evidence/` while scanning the whole feature folder detects residuals it cannot fix.

Two further verification rules learned the same way:
- **Scope the verification sweep to the files your branch changed** (`git diff --name-only
<base>..HEAD`). A repo-wide sweep returns thousands of pre-existing hits in other feature folders
Expand Down
218 changes: 81 additions & 137 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
@@ -0,0 +1,31 @@
---
name: never-predict-an-observation-into-an-artifact
description: Never write an observed value into an evidence artifact before actually observing it, even when the plan's ordering makes the value predictable; write the placeholder, commit, observe, then append.
metadata:
type: feedback
---

Never write a command's output into an evidence artifact before running that
command, even when the plan's own ordering makes the value obvious and even when
writing it early would satisfy the task's stated field set in one pass.

**Why:** On issue #662, P2-T23 required the artifact to record
`git status --porcelain -uall -- .claude/agent-memory`, taken *after* the main
commit. Drafting the artifact in one pass, I wrote a plausible three-line status
naming memory files I had not yet created. It would have been committed as an
observation. The same defect class appeared earlier in the same run with
timestamps: labels of `16-03` through `16-12` were written while the wall clock
read `15-55`, because I incremented a remembered value instead of reading the
clock. Both are the same error — a value that reads as measured but was authored.
An evidence artifact whose figures were predicted rather than read is worthless
for the audit it exists to support, and nothing downstream can detect it.

**How to apply:** When a plan orders an observation after a commit, write a short
placeholder section saying the value is recorded after that step, make the
commit, run the command, then append the real output and stage the artifact
update in the follow-up commit the plan provides for exactly this. For
timestamps, call `date` rather than incrementing; if labels have already drifted
ahead of the clock, correct them against hard anchors — file `LastWriteTime`,
`git log -1 --format=%cI`, a script's recorded start time — rather than leaving
future-dated evidence. See
[[project_evidence_timestamp_labels_drift_ahead_of_write_time]].
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
name: full-suite-run-hangs-while-earlier-runs-idle
description: A QuickFiler.Test full-assembly run that passed in 14s at baseline can hang mid-run later in the same session; sample testhost CPU to prove hang vs slow, and check the baseline before calling the failures a regression.
metadata:
type: project
---

A full-assembly `vstest.console.exe` run that completed in ~14 seconds during
Phase 0 can hang partway through when the same command is re-run later in the
same session. Prove hang versus slow by sampling the `testhost` process CPU over
60 seconds: a hung run moves the counter by hundredths of a second while the
transcript line count stays frozen. Kill only the `vstest.console` and
`testhost` processes whose `StartTime` matches your own run, then re-run once.

**Why:** On issue #662 the P0-T11 baseline ran the byte-identical command and
reported 1286/1286 passed in 14.5s. The P2-T7 re-run of that same command hung at
1328 transcript lines with 15 failures, every one a 60000 ms timeout, all in
`WinFormsPumpHost` harness tests and `UiThread` dispatcher-scope tests. Testhost
CPU moved 24.05 -> 24.08 over a 60s window. The single re-run passed all 1287
tests in 13.4s, including all 15. The failures were an environmental scheduling
flake, not a regression: the same tests pass before the change and after it, and
they fail only by wall-clock timeout rather than by assertion. Two unrelated
`vstest.console` processes over 24 hours old were present in the process table
during both the passing baseline and the hung run, so they are not the
differentiator and must not be killed.

**How to apply:** When a full-assembly run shows failures, diff the failing set
against the Phase 0 baseline for the same assembly before treating it as caused
by the change. Timeout-only failures in pump-host or dispatcher tests are the
load-flaky class. Re-run exactly once to characterise, record BOTH runs in the
evidence artifact with the hang diagnosis and the per-test names, and never
retry silently until green. Launch these runs detached: they outlive the
foreground 600s tool timeout. See [[project_long_runs_need_detached_process]] and
[[project_winformspumphost_tests_load_flaky]].
3 changes: 3 additions & 0 deletions .claude/agent-memory/atomic-planner/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
- [#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
- [#662](project_662_banner_prefix_arity_plan_seams.md) — `AC5` is a prefix of `AC5b`; `("===")` not a substring of `("====")`
- [#662 R3](project_662_round3_trx_hygiene_and_verbatim_seams.md) — `*.trx` is NOT gitignored; sweep case-insensitively; exclude the plan file from the sweep's own zero-hit gate
- [#662 R2](project_662_banner_prefix_revision_round_seams.md) — `'*.xml'` scope gate hits the plan's own cobertura evidence; loop-restart needs a baseline-relative failure test
- [#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
Expand Up @@ -24,7 +24,15 @@ before the first `dotnet tool restore` and before the first `msbuild`, in this o
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
3. **Back-fill skewed analyzer packages — RE-MEASURE BEFORE ASSERTING THIS.** As of 2026-08-31 the skew
is **RESOLVED** in the tree: every `<Analyzer Include>` across all 16 first-party `.csproj` names
`Meziantou.Analyzer.3.0.194` / `Roslynator.Analyzers.5.0.0`, and every `packages.config` pins the same
two versions, so step 2 alone now suffices and a back-fill task would be a no-op. Do not carry the
historical figures below into a plan without re-deriving them: grep `packages\\(Meziantou\.Analyzer|Roslynator\.Analyzers)\.[0-9.]+`
over `*.csproj` and compare with the `version=` values in `packages.config`. The historical failure,
preserved because Dependabot can reintroduce it:

**Historical (2026-08, no longer true).** Step 2 alone was NOT
enough. All 16 first-party `.csproj` files carry UNCONDITIONAL `<Analyzer Include>` 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`.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
---
name: project-662-banner-prefix-arity-plan-seams
description: "#662 minimal-audit planning seams — AC5 is a prefix of AC5b; a repo-wide csharpier format pass endangers two zero-diff ACs; the closing quote+paren anchors (\"===\") against (\"====\"); the coverage runner throws at <80% before it rewrites the Cobertura"
metadata:
type: project
---

Seams found while authoring the issue #662 minimal-audit plan (`EfcSelectionGuard` banner-prefix rename
plus stale-comment fix). All re-derived against the tree at base `2b85134b`.

- **`AC5` is a prefix of `AC5b`.** The issue carries AC1–AC9 *plus* AC5b. A check-off edit of
`- [ ] AC5` would also match the AC5b line. Anchor every check-off on the identifier followed by a
space and the em dash (`- [ ] AC5 —` versus `- [ ] AC5b —`). Same class as the AC1/AC10 trap in
[[project_469_comment_accuracy_plan_seams]].
- **A repo-wide `csharpier format .` is a live threat to a zero-diff AC.** AC5b and AC7 assert
`git diff <base> --stat -- <path>` is empty for `BreadcrumbRowBuilder.cs` and
`EfcFormControllerTests.cs`. CLAUDE.md mandates the repo-wide form, so pre-existing drift in either
protected file would be repaired by the mandated command and make the AC unsatisfiable. Remedy that
costs nothing: a Phase 0 task running `dotnet tool run csharpier check <path>` on each protected file,
so the condition is visible before any edit rather than after the format pass. See
[[repo-wide-csharpier-format-breaks-zero-diff-acs]].
- **`("===")` is NOT a substring of `("====")`.** The closing quote plus paren is the anchor: matching
`(`,`"`,`=`,`=`,`=`,`"`,`)` against `(`,`"`,`=`,`=`,`=`,`=`,`"`,`)` fails at the fourth `=`. That makes
a paired two-count gate (`("===")` → 2 and `("====")` → 2) a clean shape pin for a test asserting both
arities. It only works if the plan mandates four explicit assertions; a `foreach` over
`new[] { "===", "====" }` produces zero matches of either pattern.
- **`Invoke-MSTestWithCoverage.ps1` throws at <80% repo-wide line coverage**
(`Invoke-MSTestWithCoverage.Helpers.ps1:487-490`), and the throw sits *between*
`ConvertTo-KoverageCoberturaXml` and the `Set-Content` that writes the post-processed file. So on a
sub-threshold run `coverage/coverage.cobertura.xml` is left as the RAW dotnet-coverage output. Make the
baseline task's acceptance "the artifact records the numeric `line-rate`/`lines-covered`/`lines-valid`",
not "the script exits 0" — the numbers are readable either way and the task stays satisfiable.
- **Exactly ONE `[TestCategory("LiveOutlook")]` attribute exists**, at
`TaskMaster.Test/AppGlobals/LiveOutlookHookupIntegrationTests.cs:72`. A research artifact for this issue
claimed "three times in two files"; the other hits are doc-comment prose. Neither `QuickFiler.Test` nor
`UtilitiesCS.Test` carries one, so the exclusion filter is a no-op for those two assemblies.
- **An agent worktree under `.claude/worktrees/` contains no nested `.claude/worktrees/`,** so recursive
`*.Test.dll` discovery from ITS root cannot pick up a sibling worktree. Verify with a glob before
writing the `\.claude\` exclusion into a task; in this worktree the exclusion was unnecessary.
- **CSharpier print width is 100 and no `.csharpierrc` exists.** Renaming `BannerPrefix` to
`BannerRejectionPrefix` takes the two `StartsWith` call-site lines from 83 to 92 characters, so they
stay on one line and a `StartsWith(BannerRejectionPrefix` search remains a single-line match. Compute
the post-rename column count before writing an identifier-bearing search as an acceptance condition.
- **The regex `const +string +[A-Za-z_]*BannerPrefix` does not match `BannerRejectionPrefix`,** because
the inserted word breaks the required `Banner`+`Prefix` adjacency. That is what makes the declaration
inventory fall from three to one on a rename alone, with no deletion at the guard site.

**Why:** the issue's own Expected Behavior reads as though the arities should be unified upward to four
characters; that direction relaxes a merged filing guard and fails `EfcFormControllerTests.cs:463` while
`:462` — the assertion that *looks* like the consistency guard — still passes. The plan must state the
prohibited direction in prose, not only in an AC.

**How to apply:** on any follow-up in this EFC surface, re-derive the three anchored counts
(`= "===";` → 1, `= "====";` → 2 pre-change, declaration regex → 3 pre-change) with the `-- '*.cs'`
pathspec. Unscoped, the same text appears in closed-feature records under
`docs/features/active/efc-controller-surface-defects-464/` and in the feature's own documents, so the
unscoped figure grows as the feature is authored and must never be asserted.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
---
name: project-662-banner-prefix-revision-round-seams
description: Issue #662 round-2 revision seams — xml scope gate hits the plan's own cobertura evidence, post-format line numbers cannot be pinned, a delta's "first sentence" often means two, and test steps pulled into a restart loop need a baseline-relative failure definition
metadata:
type: project
---

Applying the round-1 preflight delta to the issue #662 atomic plan surfaced five seams that generalize beyond this issue.

**1. A scope-boundary `git diff`/`git status` gate scoped by `'*.xml'` matches the plan's own coverage evidence.** The delta's P2-T23 asked for `git diff <base> --name-only -- '*.cs' '*.csproj' '*.props' '*.targets' '*.xml' 'packages.config'` and required the union with porcelain status to be exactly the four in-scope source files. But the same plan writes `coverage-baseline.cobertura.xml` and `coverage-postchange.cobertura.xml` under `<FEATURE>/evidence/`, and an earlier phase commits them, so the diff lists them and the union can never equal four. Fix: append `':(exclude)<feature-folder-path>'` to both pathspecs. The exclusion does not blind the check to a formatter rewrite, because `.csharpierignore:4` is `**/evidence/**`.

**Why:** the `*.xml` operand was added to catch a CSharpier 1.2.6 rewrite of a non-`.cs` file; nobody re-checked what else in the tree ends in `.xml` after the plan's own tasks run.

**How to apply:** whenever a plan gates its change set with a suffix pathspec, enumerate the files the plan itself creates with that suffix before writing the gate.

**2. Post-format line numbers cannot be pinned as literals when earlier tasks shift them.** The delta named `EfcSelectionGuard.cs:49`, `:75` and `FolderSuggestionTree.cs:197` as "post-format line numbers" for a changed-code coverage figure. Those are the *pre-change* numbers: P1-T4 replaces a one-line XML doc with a multi-line one above `:49`, P1-T6 deletes a declaration above `:197`, and CSharpier then wraps the rewritten reader. Fix: identify each changed statement by its enclosing member and the token it contains, instruct the executor to resolve the line number from the post-format file, and require the resolved numbers to be recorded. Pre-change numbers stay valid in the Phase 0 baseline task, which runs before any edit — say so explicitly so the two tasks are not conflated.

**3. A delta that says "replace the first sentence" often means the first two.** Twice the replacement prose subsumed the sentence after the one it named. Replacing only the literal first sentence left the plan carrying "If any of *the four* fails ... restart" immediately after "If any of *those eight* fails ... restart", and left a duplicated `git add`/`git status` sequence in two commit tasks. Read the replacement for what it subsumes, not only for what the instruction names, and report the reading in the handoff.

**4. Pulling test steps into a QC restart loop needs a per-step failure definition, and it must be baseline-relative.** The delta's Phase 2 preamble extended the restart loop to cover the two full-assembly test runs, but those tasks' acceptance was "records EXIT_CODE and transcribes counters", which always holds — so the loop could never restart. Adding "any failure restarts" would instead loop forever when the baseline already carries failures. The satisfiable form is: restart when `failed` **exceeds** the same assembly's Phase 0 baseline `failed`; when equal and non-zero, do not restart and let the AC task record `REMEDIATION-REQUIRED`.

**5. `TaskMaster.runsettings` at the repository root carries the Code Coverage collector; `scripts/vscode/TaskMaster.cli.runsettings` deliberately does not.** The CLI variant is MSTest `<Parallelize>` only, documented as such at `Invoke-MSTestWithCoverage.ps1:20-26`. Pair `/EnableCodeCoverage` only with the root file, or the run instruments `Deedle` and `FSharp.Core` (referenced by both `QuickFiler.Test/packages.config` and `UtilitiesCS.Test/packages.config`). A `vstest` run passing no `/EnableCodeCoverage` may keep the CLI variant — no collector is activated, so no exclusion list is needed.

Related: [[absolute-counts-in-shared-files-go-stale]], [[observation-scope-must-match-blast-radius]], [[reference_vstest_scoped_run_command]], [[project_501_r3_preflight_seams]], [[project_662_banner_prefix_arity_plan_seams]].
Loading
Loading