fix(872): assert the scan-bound log line, add progress-package source ownership, and remove the dormant tracker by drmoisan · Pull Request #893 · drmoisan/TaskMaster

drmoisan · 2026-09-14T11:34:20Z

Suggested title

fix(872): assert the scan-bound log line, add progress-package source ownership, and remove the dormant tracker

Summary

Adds content assertions for the high-confidence dequeue gate's scan-bound log line, which previously had no test coverage on either its bound token or its stop decision.
Gives ProgressPackage an ownership-aware disposal contract so it releases a CancellationTokenSource it constructed and never releases one the caller supplied.
Releases the CancellationTokenSource that the subject-map RebuildAsync constructs, on the completing and faulting paths alike.
Deletes ProgressTrackerAsync and its test class as dormant production code with no construction site, and removes the matching Compile item from each of the two owning project files.
Consolidates three independent minor-audit defects into one delivery, following the precedent of closing several small residuals together. The three fixes touch disjoint files.
Production footprint is eight files, exactly the Write Set declared in issue.md, with no scope creep.

Why

Three small defects were each individually too narrow to justify a separate delivery, and their files do not overlap.

The scan-bound log line was not content-asserted. An earlier fix added three log lines to the dequeue confidence gate. The launch and checkpoint lines were already content-asserted, but the scan-bound line emitted by the private LogScanBoundReached was asserted nowhere, so a regression in its Bound= value or its Decision=stop token would have passed the suite. The two tests that did reach the bounded exit passed no debugLog delegate at all, so they observed no log output. The emitted line already carried every field an assertion needs, so this defect is closed test-side only and the gate's production source is deliberately unmodified.

Two CancellationTokenSource instances were never disposed. ProgressPackage constructed a source in each of its two InitializeAsync overloads when the caller supplied none, and implemented no disposal contract at all. SubjectMapSco.Orchestration constructed a local source in RebuildAsync and never disposed it. In both cases the timer and registration resources were released only by finalization.

The disposal rule here is deliberately not a blanket one. The source ProgressPackage constructs is shared beyond the package: it is handed to the tracker the same overload constructs, copied by reference into every child produced by SpawnChild, and returned to callers through the tuple factories. A caller-supplied source belongs to that caller and must never be disposed by the package, so the fix has to distinguish an owned source from an injected one.

ProgressTrackerAsync was dormant. It had no construction site outside its own test class. Removal was preferred to wiring a caller, because the type has had no caller since it was added and an earlier null-race fix on it protected code that nothing executes.

What Changed

Core behaviour

UtilitiesCS/Threading/ProgressPackage.cs — declares IDisposable; records per-instance ownership state assigned at the construction site inside each InitializeAsync overload; Dispose releases the source only when the class constructed it. The ownership flag is deliberately never assigned through the public CancelSource property setter, because SpawnChild assigns through that setter and a child must not claim its parent's source.
UtilitiesCS/EmailIntelligence/SubjectMap/SubjectMapSco.Orchestration.cs — holds the source RebuildAsync constructs through a using declaration, so it is released after the last use of the token and of the tracker that holds it, on both the completing and the faulting path.

Removals

UtilitiesCS/Threading/ProgressTrackerAsync.cs and UtilitiesCS.Test/Threading/ProgressTrackerAsync_Tests.cs deleted.
UtilitiesCS/UtilitiesCS.csproj and UtilitiesCS.Test/UtilitiesCS.Test.csproj each lose exactly one Compile item. These projects are not SDK-style and have no wildcard glob, so deleting a source requires removing its Compile item in the same change or the build fails.

Tests

QuickFiler.Test/Controllers/QfcStreamingDequeueConfidenceGateTests.Part4.cs — two tests that drive the gate to the item-cap bound and to the time-ceiling bound with a debugLog delegate injected, asserting the cutoff in force, the accepted and scanned counts, the distinct bound tokens scan-cap and zero-acceptance-ceiling, and Decision=stop. Using distinct tokens means a regression collapsing the two bounds to one value would fail.
UtilitiesCS.Test/Threading/ProgressPackage_Tests.cs — three disposal tests: a constructed source is released, a caller-supplied source is left usable, and a child from SpawnChild does not release its parent's source. All three drive the tracker overload with a non-null injected tracker and an explicit stop watch so no UI thread is touched, and all three probe release by observing that the source's Token getter throws rather than by inspecting a timer.

Architecture / How It Fits Together

The ownership change is confined to ProgressPackage and is additive at the public boundary. The class gains a disposal contract; its existing tuple factories keep returning the source to the caller unchanged, and the SpawnChild copy semantics are untouched. The one invariant that makes this safe is that ownership is recorded only at the two construction sites and never through the property setter, so the borrowed-source relationship between a parent package and its children is preserved.

The two viewer types that actually hold the source already catch ObjectDisposedException and document the borrowed-source contract, so the tracker and tracker-pane types needed no change.

Verification

Completed

The branch was merged with origin/main at 03d2ece20 and every gate was re-run against the merged tree, in the order CLAUDE.md mandates. The loop completed in a single pass; no stage failed and no stage rewrote a file.

Stage	Result
`dotnet tool run csharpier check .`	exit 0, 1633 files checked, none unformatted
Analyzer rebuild, `/t:Rebuild` with `EnableNETAnalyzers` and `EnforceCodeStyleInBuild`	exit 0, 0 warnings, 0 errors
Nullable rebuild, `/t:Rebuild` with `TreatWarningsAsErrors`	exit 0, 0 warnings, 0 errors
`UtilitiesCS.Test` via vstest	exit 0, 4901 of 4901 passed
`QuickFiler.Test` via vstest	exit 0, 1436 of 1436 passed
Repository-wide coverage run	exit 0, 7261 of 7261 passed

Both rebuilds were checked for non-vacuity against the detailed file log rather than accepted on exit code alone: each recorded 18 CoreCompile target runs and zero occurrences of the up-to-date skip message. The nullable gate supplied no Nullable property on the command line, matching the CI workflow.

Repository first-party coverage is 85.84 percent line and 80.01 percent branch, both above the floors CLAUDE.md governs of 80 percent line and 75 percent branch.

All twelve acceptance criteria in issue.md were re-verified against the merged tree. The two that are stated as deltas were re-derived against the new base rather than carried forward:

Compile item counts fall by exactly one in each project file, 492 to 491 and 479 to 478.
Executed-test counts change by exactly minus six in UtilitiesCS.Test and plus two in QuickFiler.Test, derived by counting [TestMethod] in exactly the three test files this delivery touches. No file carries a DataRow attribute, so the executed count equals the method count.
Per-file coverage on ProgressPackage.cs is 61 of 61 lines covered against a baseline of 52 of 52, a ratio of 1.00 against 1.00 with zero uncovered lines.

Full detail is in docs/features/active/2026-09-11-minor-audit-trio-gate-cts-tracker-872/evidence/qa-gates/postmerge-revalidation.2026-09-14T07-30.md.

Backward Compatibility / Migration Notes

ProgressPackage now implements IDisposable. This is additive: existing callers that do not dispose the package behave exactly as before, and the class releases only a source it constructed itself.
ProgressTrackerAsync is removed. It had no construction site outside its own test class, so no caller is affected. The only other reference in the tree is a prose mention inside a <c> tag in an XML doc comment, which is not a compile-time dependency and is left untouched.
No public contract that an existing caller depends on is altered.

Risks and Mitigations

Disposing a shared source too early. Mitigated by recording ownership only at the two construction sites and never through the CancelSource setter, and by a test proving a child from SpawnChild does not release its parent's source.
Dropping an unrelated Compile item while removing one. Mitigated by asserting that each project file's Compile count falls by exactly one rather than merely that the target item is gone. A fall of more than one would indicate a sibling was dropped.
Coverage denominator movement. Deleting the dormant tracker removes a production file from the denominator, so the repository headline moves up. This is expected and is the stated purpose of the dormant-code defect, not a masked regression.
RebuildAsync is not unit-testable. It installs a WindowsFormsSynchronizationContext and starts a long-running task, and carries [ExcludeFromCodeCoverage], which makes it invisible to the coverage report rather than reporting it at zero. That criterion is therefore verified structurally and by the build, not by a coverage number. This is stated in issue.md rather than left implicit.

Review Guide

Suggested order:

UtilitiesCS/Threading/ProgressPackage.cs — the only non-trivial logic change. Check that ownership is assigned at both construction sites and nowhere else.
UtilitiesCS.Test/Threading/ProgressPackage_Tests.cs — the three disposal tests that pin the ownership contract.
UtilitiesCS/EmailIntelligence/SubjectMap/SubjectMapSco.Orchestration.cs — a one-line change to a using declaration.
QuickFiler.Test/Controllers/QfcStreamingDequeueConfidenceGateTests.Part4.cs — the two log-assertion tests.
The two .csproj files and the two deletions, which are mechanical and should be read together.

The remainder of the diff is the feature folder: plan, issue, research and evidence artifacts. It is large by line count and mechanical to review.

Follow-ups

An ownership-aware disposal contract on the progress package closes the defect for a holder that disposes the package, and does not close it for paths on which nothing holds the package. The two static tuple factories construct a package locally, return the source to the caller inside a tuple, and discard the package, so no holder is left to dispose. The same shape recurs in the two LoadIfNullAsync overloads of the Bayesian performance measurement type.

Nine production call sites carry that residual across six files. None is in this Write Set and none is edited here, for three reasons: three sites sit in host-bound methods carrying [ExcludeFromCodeCoverage] that cannot be covered by a unit test; disposal at each site requires first proving the derived token is no longer in flight, because a token whose source has been disposed still answers IsCancellationRequested but throws from Register and from WaitHandle; and the Bayesian performance measurement file exceeds fifteen hundred lines, so changing its ownership shape is not a minor-audit change. The residual is real and is recorded in issue.md and in the research artifact so that it is tracked rather than buried, and it needs its own issue.

A separate, pre-existing defect in the coverage runner's threshold helper is tracked as issue 891: its document-level threshold is hard-coded, so a scope-narrowed invocation cannot exit 0 however well its tests do. It was not encountered here because this run was repository-wide, and the runner script was not modified.

The generated coverage summary at the repository root names the deleted type and becomes stale. It is a generated artifact, is not compiled, and is not edited by this delivery.

GitHub Auto-close

None

No closing keyword is emitted. The GitHub CLI was unavailable when the PR context was collected, so no closing issue could be verified from GitHub metadata. The context bundle's author-asserted candidate list is not a substitute: it is harvested from prose and contained thirteen entries, including two research scope-finding labels that are not issue numbers at all, and most of the remainder are citations to prior work rather than issues this change resolves. Issue 872 is expected to need a manual close after review.

🤖 Generated with Claude Code

https://claude.ai/code/session_012YZxqEe1udErQiYYt6Bb2d

…flighted plan

…tion-only decision

Every vstest span in the approved plan carried /Logger:trx with no LogFileName= operand. vstest then composes the TRX file name from the account name and the host name, and P2-T14, P2-T15 and P2-T16 transcribe that file name into committed evidence artifacts, which would place both tokens in the repository. All six spans now carry a quoted /Logger:trx;LogFileName= value naming the task that produced it, matching the reference form already in use on item 816. Spans corrected: P0-T8, P0-T9, P1-T15, P1-T16, P2-T5 and P2-T6. The count was re-derived against the working tree rather than taken on trust; six spans were found, each on its own line, and no line carried two. D10 recorded this as an undischarged residual and asserted that no span in the plan set a log file name. That assertion is false after this change, so the D10 prose is rewritten to record item 4 of the issue 671 decision as discharged. The correction is acceptance-condition-neutral. No acceptance condition referenced a TRX file name. The most-recent-write selection rule stated by P2-T14 through P2-T16 is retained and is simply inert now that a Phase 2 restart overwrites one fixed file instead of adding a second. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…pans The six direct vstest spans appended the MSTest runsettings file under the vscode scripts directory. Its entire content is an MSTest Parallelize block with ClassLevel scope and a zero worker count, which runs test classes concurrently across every logical processor. Under that parallelism three tests in the QuickFiler zero-batch email-queue test class fail with a type initialization exception for the Deedle reflection type against netstandard 2.1. One race during concurrent class initialization poisons the type, and the CLR caches a failed static initializer for the process lifetime, so the failure reproduces identically on every later run and reads as deterministic. The same command with only that switch removed was measured at 1393 of 1393 passed, exit 0. The CI MSTest coverage workflow passes no settings file, so the defect is invisible to the merge gate. Parity is therefore with CI, which is what the merge gate runs, rather than with the repository runner. Retaining the switch made the exit-zero and zero-failure acceptance conditions of P0-T9, P2-T6 and therefore AC11 unsatisfiable by any work this plan performs, and would have tripped the P0-T14 red-baseline gate before Phase 1 began. This is a gate that cannot pass, not a gate that cannot fail, so it is repaired rather than measured again. Nothing else in the six spans changes. The pinned test-case filter is still byte-identical between Phase 0 and Phase 2, so the D5 comparability guarantee and the AC11 delta arithmetic are untouched; parallelism affects which tests pass, not how many are discovered. Neither the runner script nor the runsettings file is edited. Both are outside the Write Set. P0-T10 and P2-T7 invoke the runner and still cannot avoid the switch, because the runner resolves the runsettings path internally and exposes no override parameter; that residual is reported to the caller rather than worked around. Recorded as D14 in the plan. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…gate-cts-tracker-872

…d main The mandated reconciliation merged main into this branch. Main added exactly one Compile item to UtilitiesCS.csproj and one to UtilitiesCS.Test.csproj, so the P0-T12 expected baseline moves from 491 and 476 to 492 and 477. The figures were re-derived against the post-merge working tree rather than inferred from the merge diff. Every other pinned fact was re-measured and is unchanged: the Compile item for the dormant tracker is still at line 971 of the production project file and line 496 of the test project file, both neighbours on either side are still Compile items, the dormant tracker's test class still has 9 test methods and 0 data rows, the subject-map orchestration partial still has 4 coverage-exclusion attributes, and the four tracked file line counts are still 347, 120, 150 and 274. AC7 is unaffected because it is expressed as a fall of exactly one relative to the base commit rather than against an absolute count. The research artifact still records 491 and 476. That is a correct measurement of the tree at its own timestamp and is deliberately left unrewritten; the plan now says which figures are operative. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Phase 0 executed P0-T1 through P0-T14. Thirteen of the fourteen tasks passed. P0-T10, the repository-wide coverage baseline, failed with exit 1 and the P0-T14 halt gate therefore records PHASE0_GATE: RED. Phase 1 has not begun and no Write Set path was created, modified or deleted. Baselines captured. UtilitiesCS test assembly 4903 of 4903 passed; QuickFiler test assembly 1394 of 1394 passed; both at exit 0. Compile item counts 492 and 477 on both patterns, matching the reconciled expectations. Repository-wide raw Cobertura line coverage 71.15 percent and branch coverage 59.91 percent. Per-file ProgressPackage.cs baseline 52 covered of 52 total, merged by line number under the maximum hits rule; naive summation would give 54 of 53, so the de-duplicating merge is load-bearing. The blocker is pre-existing and is not a regression of this delivery. The coverage runner threw at its own line 236 after three tests in QfcInitEmailQueueZeroBatchTests failed with a TypeInitializationException for Deedle.Reflection against netstandard 2.1. The runner appends the MSTest runsettings file at its line 76 and resolves that path internally, exposing no override parameter, so its ClassLevel parallelism cannot be avoided from the call site. P0-T9 is the corroborating control: the identical QuickFiler assembly, minutes earlier, without that runsettings file, 1394 of 1394 at exit 0. Neither the runner nor the runsettings file was edited; both are outside the Write Set. A second consequence is recorded for whoever resolves this: the runner threw before post-processing, so it emitted a raw rather than post-processed Cobertura document and printed neither of its two summary lines. The P0-T11 baseline was derived from that raw document, so a Phase 2 comparison must come from a document produced the same way or it is not like-for-like. A second, independent pre-existing repository defect surfaced during bootstrap and is recorded in the P0-T6 artifact: 15 of the 16 first-party projects carry an Analyzer Include HintPath naming Meziantou.Analyzer 3.0.203 while packages.config resolves 3.0.235. A cold worktree fails the analyzer rebuild with two CS0006 errors until 3.0.203 is provisioned into the git-ignored packages directory. It is present on main identically, it affects every cold worktree in this cohort, and repairing it properly would edit 14 project files outside the Write Set, so it was worked around rather than fixed and needs its own issue. Checklist state matches evidence on disk: exactly thirteen checkboxes were flipped and the P0-T10 box is left unchecked per the fail-closed evidence rule. No raw TRX, MSBuild log or Cobertura XML is committed; all such output stays under the git-ignored TestResults directory. No artifacts/csharp coverage document was created, so no repository coverage floor was activated. No evidence artifact contains an absolute host path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…gate-cts-tracker-872

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…p, remove dormant tracker Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…p, remove dormant tracker

…validation Resolves the single conflict in .claude/agent-memory/orchestrator/MEMORY.md by keeping this branch's committed index per the standing agent-memory rule. UtilitiesCS.Test.csproj auto-merged, retaining this branch's ProgressTrackerAsync_Tests Compile-item removal alongside main's UiThreadApartmentMeasurement_Tests addition. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Re-runs all four toolchain gates and re-derives every inherited numeric baseline against the merged tree. All twelve acceptance criteria still hold. Records the four figures the merge moved, the AC7 and AC11 deltas re-derived against the new base, and the item 873 coverage-runner discard change that invalidated the inherited AC12 measurement recipe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

drmoisan and others added 18 commits September 12, 2026 12:19

chore(872): prepare minor-audit trio feature folder, research and pre…

0e42447

…flighted plan

fix(872): route raw coverage XML out of committed evidence per projec…

09b4ee2

…tion-only decision

Merge remote-tracking branch 'origin/main' into bug/minor-audit-trio-…

b162eb7

…gate-cts-tracker-872

Merge remote-tracking branch 'origin/main' into bug/minor-audit-trio-…

037e136

…gate-cts-tracker-872

chore(872): re-anchor Phase 0 against the 877 fix merged as PR 880

430e2a1

chore(872): re-measure Phase 0 baselines against the post-merge tree

d5eddb0

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

chore(872): withdraw the false AC12 pane-overload coverage exemption

d1fe984

fix(872): assert scan-bound log, add progress package source ownershi…

12cba3d

…p, remove dormant tracker Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

fix(872): assert scan-bound log, add progress package source ownershi…

7c10e3b

…p, remove dormant tracker

chore(872): record terminal evidence for the minor-audit trio

d31c759

chore(872): record the terminal plan check-off

05e44b3

chore(872): record the session-root PR gate confirmation in agent memory

280596b

drmoisan merged commit e4a3375 into main Sep 14, 2026
5 checks passed

drmoisan mentioned this pull request Sep 14, 2026

Bug: minor-audit-trio-gate-log-assertion-cts-disposal-dormant-tracker #872

Closed

5 tasks

Navigation Menu

fix(872): assert the scan-bound log line, add progress-package source ownership, and remove the dormant tracker - #893

fix(872): assert the scan-bound log line, add progress-package source ownership, and remove the dormant tracker#893
drmoisan merged 18 commits into
mainfrom
bug/minor-audit-trio-gate-cts-tracker-872

drmoisan commented Sep 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

drmoisan commented Sep 14, 2026

Suggested title

Summary

Why

What Changed

Core behaviour

Removals

Tests

Architecture / How It Fits Together

Verification

Completed

Recommended

Backward Compatibility / Migration Notes

Risks and Mitigations

Review Guide

Follow-ups

GitHub Auto-close

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant