Skip to content

fix(635): settle the residual reflective-caller risk with a measured, non-vacuous audit - #688

Merged
drmoisan merged 6 commits into
mainfrom
bug/issue-468-residual-reflective-caller-risk-635
Aug 29, 2026
Merged

fix(635): settle the residual reflective-caller risk with a measured, non-vacuous audit#688
drmoisan merged 6 commits into
mainfrom
bug/issue-468-residual-reflective-caller-risk-635

Conversation

@drmoisan

Copy link
Copy Markdown
Owner

Suggested title

fix(635): settle the residual reflective-caller risk with a measured, non-vacuous audit

Summary

  • Discharges the open verification obligation inherited from issue Bug: qfc-collection-controller-unreachable-load-paths #468 acceptance criterion AC-16: whether anything still reaches, by runtime name resolution, any of the thirteen members that commit 63eebd47 removed from the QuickFiler collection controller. The recorded decision is DECISION: RESIDUAL RISK CLOSED.
  • No executable code changes. The entire diff is 32 Markdown files. No .cs, .csproj, .props, .targets, .resx, .config, .settings, .xaml, or .ps1 file is added, modified, or deleted anywhere on the branch.
  • Widens the AC-16 search on all three axes on which it was narrow: it searches the thirteenth identifier _templateTlp that AC-16 omitted, covers the GetField( family that AC-16 never inspected, and raises the file-type scope from 153 files to 683.
  • Makes every zero result auditable rather than asserted. Each of the 37 zero-result searches records its command, scope, patterns, result, and a measured scope size, and a non-vacuity control proves the same pathspec returns real content for a token that is genuinely present.
  • Corrects two statements in the AC-16 record without editing that historical artifact, and records the one class of caller that was not proved absent rather than claiming complete closure.

Why

Issue #468 removed twelve methods and one private field from QuickFiler/Controllers/QfcCollectionController.cs. A successful compilation proves no compile-time caller survived. It cannot prove the absence of a caller that resolves a member by name at run time, through reflection, a configuration token, or a late-bound host invocation. AC-16 of issue #468 required a residual-risk search to close that gap, and that search returned no caller.

The AC-16 search is now known to have been incomplete in three specific respects, and one factual statement it recorded is no longer true:

  1. It searched twelve identifiers and omitted the private field _templateTlp. Field reflection is the only name-based mechanism that demonstrably exists anywhere near the affected type, so the omitted identifier is the one for which the search mattered most.
  2. Its reflection inventory covered only GetMethod( and InvokeMember(. It did not cover the GetField( family, which is the family actually used against the affected type.
  3. Its file-type scope covered six build-input extensions and reached no PowerShell, YAML, XML, XSD, text, backup, solution, or extensionless tracked file.
  4. It recorded zero occurrences of any removed identifier anywhere in the QuickFiler test tree. A documentation comment naming WireUpKeyboardHandler has since been added to that tree.

This item is therefore a procedural gap, not a behavioral defect. Nothing in the product behaves differently before and after it.

What Changed

Docs and evidence only. There is no other category.

Requirements and planning documents (4 files): issue.md, spec.md, plan.2026-08-29T00-23.md, and research/reflective-caller-closure.md.

Evidence artifacts (21 files) under the feature folder's evidence/ subtree:

  • baseline/ — the policy-read record, the requirements inputs, the worktree baseline, the commit-level derivation of the thirteen-identifier search set from 63eebd47, and the scope census.
  • other/ — the three sweep partitions, the non-vacuity control, the untracked-file pass, the seventeen-pattern reflection inventory, the production-tree reflection classification, the variable-argument closure argument, the binding and serialization surface check, the AC-16 corrections, the decision record, and the zero-result audit.
  • regression-testing/ — the fail-before exception dossier.
  • qa-gates/ — the no-modification proof, the toolchain gate record, the host-identity scan, and the acceptance-criteria reconciliation.

Review artifacts (3 files): policy-audit, code-review, and feature-audit, all timestamped 2026-08-29T06-50.

Agent memory (4 files) under .claude/agent-memory/, written by the planner, researcher, orchestrator, and reviewer as their own bookkeeping. These are carved out explicitly by plan tasks P4-T2 and P4-T8 and are not part of the item's change set.

Architecture / How It Fits Together

The audit is a total classification rather than a hit count, and that choice is the load-bearing design decision.

A repository-wide "zero hits" condition is unsatisfiable by construction here, for two measured reasons: LoadSequentialAsync names three live and unrelated members in the TaskMaster startup assembly that are not going to be renamed, and the docs and .claude trees quote every one of the thirteen identifiers thousands of times in authored prose and generated evidence. An acceptance condition written as a count would have been red before the work started.

The sweep is therefore run in three partitions, and each hit is assigned to exactly one category by a mechanical test derived from the path or the line, applied in a stated order:

  • Partition A — tracked non-.cs files outside the docs and .claude trees. This is the only partition carrying a zero-hit assertion, and only because its pathspec excludes both prose trees. Scope: 683 files. Result: no output, exit 1.
  • Partition B — the same sweep including both prose trees. Categories D (docs) and E (.claude) sum to the total; category G, "genuine name-based caller", is empty.
  • Partition C — tracked .cs files. 31 hits, each enumerated individually with its file, line, matched identifier, and category. Category G is empty here too, by an independent route.

The reflection inventory then covers seventeen patterns across both QuickFiler trees. All sixteen name-resolving patterns return zero in the production tree. System.Reflection returns 39 there, and every one of those 39 is classified: 26 are the log4net logger-declaration idiom calling MethodBase.GetCurrentMethod(), 3 are using directives, 3 are comments, and 7 are project-file or package-manifest entries. None takes a member-name argument, so class L5 — a call site that could resolve a member by name — is empty.

Verification

Completed

Gate Result
No-modification proof (git diff --name-only origin/main...HEAD plus git status --porcelain) EXIT_CODE 0, pass
Acceptance criteria reconciliation AC_CHECKED=15, AC_UNCHECKED=0
Host-identity scan across all artifacts HOST_IDENTITY_HITS=0 over a non-empty scanned set
Zero-result audit 37 zero-result searches, each with scope, patterns, result, and measured scope size
Feature review GO, 0 blocking findings, 15 of 15 acceptance criteria PASS

Not applicable, with the reason stated rather than skipped

The C# formatting, analyzer, nullable, and test gates and the PowerShell gates have no input in this branch. TOOLCHAIN_BRANCH: 2 was taken because the branch diff contains no file in any of those languages. No coverage command was run and no coverage artifact was emitted, because no executable line changes and emitting one would fabricate a measurement of something this branch did not touch.

Recommended for a reviewer who wants to re-derive the result

git grep -n -I -F -e WireUpKeyboardHandler -e AnyOpenDropDownsAsync -e LoadGroups_02cAsync -e LoadGroups_02bAsync -e LoadGroup_03bAsync -e LoadConversationsAndFoldersAsync -e LoadItemGroup -e LoadSequentialAsync -e LoadGroupSequential -e CacheTlpForMove -e SwapTlp -e CaptureTlpTemplate -e _templateTlp -- ":(exclude)*.cs" ":(exclude)docs/*" ":(exclude).claude/*"

Expect no output and exit code 1. Then replace the identifier list with QfcCollectionController under the identical pathspec and expect 13 hits across 4 files; that control is what makes the first result a measurement rather than an artifact of an empty search set.

Backward Compatibility / Migration Notes

None. No public surface, CLI flag, configuration schema, binary, or version changes. No behavior differs before and after this branch.

Risks and Mitigations

Risk Mitigation
The closure argument is overstated. Its limit is recorded in three places rather than argued away: the argument does not cover a member name assembled at run time by concatenation or interpolation. No such construction was observed at any enumerated site, but its absence in general was not proved.
A zero result comes from an empty search set rather than genuine absence. Every zero carries a measured scope size, and the P1-T2 control returns 13 hits under the identical pathspec, two of them in files no extension-based search could reach.
The measured figures drift as prose accretes. Acceptance conditions are written as classification identities, never as counts. The Partition B total moved 2229 to 2337 to 2474 across three commits while both identities held at every one.
Scope creep into a source-file change. AC-12 makes a Markdown-only diff a blocking acceptance criterion, independently verified over the full 32-path diff.

Review Guide

Suggested order:

  1. evidence/other/p3-t3-decision-record.2026-08-29T04-55.md — the conclusion and what it does and does not cover.
  2. evidence/other/p1-t1-partition-a-sweep.2026-08-29T04-55.md and p1-t2-partition-a-control.2026-08-29T04-55.md — read these as a pair; the control is what gives the zero its meaning.
  3. evidence/other/p2-t3-variable-argument-closure.2026-08-29T04-55.md — the only genuinely argumentative artifact, and the place to push back if you are going to push back anywhere.
  4. feature-audit.2026-08-29T06-50.md — the independent adjudication, including eight non-blocking findings the reviewer measured rather than merely noted.

The remaining evidence artifacts are mechanical records of commands and their output and can be skimmed.

Follow-ups

  • Owed at merge: spec.md retains the superseded figure of six variable-argument reflection call sites in AC-9. The mechanical derivation yields eight (seven GetField( sites and one GetMethod( site), and no six-element subset is identifiable with the specification's six. All eight are enumerated individually, which is a superset of what AC-9 requires. The approved specification was deliberately not edited, because altering approved criterion text is prohibited; a maintainer amendment of that baseline figure is the correct disposition.
  • dynamic late binding was not enumerated as a mechanism in the inventory. It is the one class in the same family as the stated limit, because it needs no string literal. The reviewer measured it independently and found 1 production hit, a comment in a Moq context, and none in the test tree. Adding it to future sweeps is recommended.
  • The AC-16 record's own 398-file scope figure was never reconciled with the 153-file comparable scope re-derived here.
  • A pre-existing, unrelated analyzer version skew makes every msbuild invocation fail CS0006 in a fresh worktree: packages.config pins Meziantou.Analyzer 3.0.174 and Roslynator.Analyzers 4.16.1 while the hand-written Analyzer items name 3.0.156 and 4.16.0. It is not introduced by this branch and belongs to its own issue.

GitHub Auto-close

Issue #468 is referenced throughout as the origin of the verification obligation. It is already closed and is deliberately not named with a closing keyword here.

drmoisan and others added 6 commits August 29, 2026 01:35
Preparation-mode planning output for issue 635, which settles the
residual reflective-caller risk left open by issue 468. Issue 468
removed thirteen dead members from the QuickFiler collection
controller; compilation proved no compile-time caller survived but
cannot prove the absence of a caller that resolves a member by name
at runtime.

Adds the active feature folder with spec.md (15 acceptance criteria),
the research artifact, and the preflight-cleared atomic plan. The plan
is an evidence-producing audit: it modifies no production or test
source file.

Three corrections to the prior AC-16 record are carried into the plan:
the removal set is thirteen identifiers, not twelve, because the dead
_templateTlp field was omitted; AC-16 searched only GetMethod and
InvokeMember and so never saw the GetField family, which has 172 test
tree hits including eight variable-argument sites against the removed
members' own type; and AC-16's "zero hits in the test tree" claim no
longer holds because a doc comment added since then names one of the
identifiers.

The plan measures its own search scope so every zero result is
demonstrably non-vacuous, and it uses a total classification with one
empty class rather than a repository-wide zero-hit assertion, which is
unsatisfiable by construction here.

Plan validator: ok, no warnings. Preflight: ALL CLEAR after two rounds.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BmFjm2aSpooVerwWWWuN8S
Adds an orchestrator memory entry for a failure found during issue 635
preparation: a pwsh -NoProfile -Command block written with a bash
double-quoted outer string fails twice over in an isolated agent
worktree. The isolation guard refuses the call because of the $, and
where it does not fire bash pre-expands the variables to empty strings
so the command silently computes wrong values. An atomic plan reached
preflight with all eight of its pwsh blocks in that form, because the
commands had been validated in a \$-escaped variant rather than in the
bytes the executor would actually run.

Also compacts the memory index, which a PostToolUse hook flagged as
approaching its 24.4KB read limit. All 124 entries and the section
heading are preserved; only the one-line hooks are tightened, since
the detail already lives in the linked topic files. 24.9KB to 17.1KB.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BmFjm2aSpooVerwWWWuN8S
Records the policy-audit, code-review, and feature-audit for the
reflective-caller residual audit. Verdict GO, zero blocking findings,
15 of 15 acceptance criteria PASS.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BmFjm2aSpooVerwWWWuN8S
…oint gates

The claim that feature-review always routes to fable holds only under
fable_policy preferred; under available its C3 cell is opus. Records the
measured table and the preferred-overlay mechanism behind the difference.

Also records that the pre-implementation gate requires lifecycle_ready
and that delegation_receipts.agents must be a list, both verified on
this run.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BmFjm2aSpooVerwWWWuN8S
@drmoisan
drmoisan merged commit 5781a57 into main Aug 29, 2026
5 checks passed
@drmoisan
drmoisan deleted the bug/issue-468-residual-reflective-caller-risk-635 branch September 2, 2026 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Settle the issue #468 residual reflective-caller risk repository-wide

1 participant