fix(generator): stop shipping [Remote] bodies to trimmed clients for [Execute] and [FactoryEventHandler<T>] (TRIM-008) - #75
Merged
Conversation
…e arc Reverses the 2026-08-11 routing that sent the registrar-DAM defect to plan mode outside any todo. Three things made that untenable: the defect is release-blocking, so the arc cannot close without it; it falsifies the documentation AC4 requires be accurate, so AC4 cannot close honestly while it stands; and the close-out audit found its only record was a Discovery Log entry that archives when the todo closes — it would have lost its tracker at exactly the moment it still mattered. Adds AC6 to the todo, making the IP guarantee a named requirement. Notes why that is not scope creep: AC1-AC3 are about preservation (making types survive trimming), AC6 is about over-preservation (stopping code surviving that should not) — opposite failure modes of the same mechanism. The plan uses a top-level generated holder that FORWARDS to the existing registrar, not the nested type floated at TRIM-005's review. Two reasons. The nested variant rests on "DAM does not extend to nested types", which this codebase has never verified and which the TRIM-007 precedent does not prove since that holder is top-level. And design work found the advisory's unstated second reason for nesting: [Execute] methods are private static by the repo's own convention and the registrar body calls them, so a sibling holder that hosts the registrar is CS0122. Forwarding satisfies both without depending on unverified ILLink behavior, and removes no member, so it is fix: not feat!:. Records two latent generator bugs found during design and deliberately not fixed: nested [Factory] static / handler classes emit uncompilable code, and a class carrying both attributes emits CS0111. Both pre-existing. Deferred items 1, 6, 7, 8 now route to TRIM-008; 14 and 15 added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ing holder
The assembly attribute's [DynamicallyAccessedMembers] preserves every method
on whatever type it names, bodies included. StaticFactoryRenderer named the
consumer's own [Factory] class, so ILLink retained their [Remote] server-only
method bodies and shipped them to trimmed browser clients, decompilable —
contradicting the IP-protection guarantee the docs advertise.
Emits a top-level `internal static class NeatooFactoryRegistrar_{TypeName}`
whose single method forwards to the FactoryServiceRegistrar that stays on the
user's partial class, and points the attribute there. The DAM blast radius
becomes that one forwarding method.
Forwarding rather than hosting is required: [Execute] methods are private
static by the repo's own convention and the registrar body calls them, so a
sibling type that hosted the registrar would be CS0122. Forwarding also
removes no member, so this is fix: rather than feat!:.
Measured, not inferred. Publish-trimmed win-x64, TrimMode=full,
IsServerRuntime=false, diffed against an archived pre-fix artifact:
_DoWork PRESENT -> absent
_ProcessRecord PRESENT -> absent
IServerOnlyRepository PRESENT -> absent
DoServerWork PRESENT -> absent
TrimTestCommands PRESENT -> PRESENT (delegates still rooted)
TrimRecordResult PRESENT -> PRESENT (AC1 intact)
NeatooEventPreservation PRESENT -> PRESENT (AC3 intact)
Harness exits 0 with ValidateOnBuild=true and resolves the static delegate,
so registration still works through the indirection. This also settles the
open question of whether ILLink applies feature-switch substitution to a
method reached only by a call from a DAM-preserved method: it does.
The holder is named with a PREFIX deliberately. A suffix form leaves
global::Ns.{TypeName} a substring of the holder's own FQN, which would make
the "attribute must not name the consumer's type" regression assertion a
false red and an unclosed Contains a false green.
Plan review returned CONCERNS with 5 veto findings; all are folded into the
plan before further implementation. The most consequential reordered the
steps: harness targets and their pre-fix probe must precede the leg that
fixes them, because a marker first measured after the fix cannot distinguish
"the fix removed it" from "it was never rooted".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pre-fix Plan-review B1 required every new marker to have a pre-fix baseline before the leg that fixes it lands. Three legs added, each with its own server-only port so a marker names one culprit, then probed publish-trimmed with the relay leg still unfixed. Results: - relay leg: all four markers present. The baseline Step 4 needs. - interface-factory leg: all five markers absent. The arc's structural claim about this leg is now a measurement (closes half of B9). - Save/Can* leg: PRESENT, unexpectedly. Async generated Local* methods retain their server-only bodies; sync ones in the same assembly do not. Rooted by the class-factory registrar's own unguarded delegate closures, so this is a different defect from TRIM-008's and the holder fix does not reach it. Carried as deferred item 18; blocks AC6 as worded. The probe's self-check against the untrimmed assembly caught a defect in the probe itself: it decoded UTF-16 only from byte offset 0, so every string literal at an odd offset was invisible and reported "absent". Uncorrected it would have reported the Save/Can* break as a clean pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…TRIM-009 User decision. The alternative was narrowing AC6 to the two shapes TRIM-008 owns and shipping v1.7.0 sooner with a disclosure. Rejected: the leaking shape is the class factory, which the docs, the skill, and CLAUDE-DESIGN.md all hold up as the shape that works. A release closing AC4 while that claim stays false is what AC6 exists to prevent. - AC6 reworded to name all three broken shapes; routes to TRIM-008 + TRIM-009 - TRIM-009 stub added, carrying the measured evidence and the controlled sync-vs-async comparison, with no remedy prescribed - Deferred item 18 routed to TRIM-009; item 2 (release held) widened - TRIM-008 scope corrected: it no longer unblocks the release by itself and must not be closed out as delivering AC6 TRIM-008 is unchanged in scope and continues at Step 4. TRIM-009 inherits the 2026-08-13 pre-fix baseline rather than needing its own probe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng holder Second and last leg of the registrar-DAM fix. The [FactoryEventHandler<T>] assembly attribute named the consumer's own handler class, so DAM retained every method on it including the handler body and the server-only service it reaches. Adds a forwarding holder and the long-missing global:: qualifier. Holder prefix is NeatooEventHandlerRegistrar_, deliberately distinct from the static leg's NeatooFactoryRegistrar_: a class carrying both [Factory] and [FactoryEventHandler<T>] already emits duplicate registrars (CS0111, broken at HEAD, deferred item 15), and a shared prefix would stack CS0101 on top. Measured publish-trimmed, present-before/absent-after on all four relay markers including the body's string literal. Interface-factory markers absent in both probes (no regression); Save/Can* markers present in both, as TRIM-009 predicts. Tests (the relay leg had none before): - StaticFactory_EmitsAssemblyAttribute retargeted, intent preserved - new: attribute does not name the consumer type, on both legs - new: holder forwards to the user's registrar, on both legs - new: relay output compiles without errors Proven non-vacuous: with the renderer reverted, the three relay emission tests go red and the other eight stay green. DiagnosticTestHelper gains DI/Logging/ComponentModel references. Without them no generated tree could compile in this harness, so "generated code is valid" was not an assertion anyone could have written — which matters most for the relay renderer, whose output bypasses NormalizeWhitespace and whose parse failures are swallowed into a comment rather than thrown. FactoryEventRelayTests flakes under full-suite parallel load (deferred item 10). Verified pre-existing: identical failures with this change stashed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ing gate Step 5 — NeatooFactoryRegistrarAttribute now states the rule that was only ever implicit: the Type must be a generated registrar type, never a consumer's class, because [DynamicallyAccessedMembers] retains every method on it, bodies included. Documented with the reason rather than as a bare rule, plus the two points a future editor would otherwise re-litigate — why the DAM is deliberately not narrowed, and that the method name is a literal looked up with a null-conditional invoke, so a rename fails silently. No API surface changes: XML documentation only. No member added, removed, renamed, or re-annotated. Step 8 — the (?<!I)ServerOnlyRepository exemption is deleted. It was justified by the TRIM-005 diagnosis this arc disproved; the real cause was the static-factory registrar naming the consumer's class. With that fixed, IServerOnlyRepository is measured absent and now asserted absent. The gate moved from shell-in-YAML into verify-trimmed.sh, which is the point: the acceptance bullet carried [explicit-skip: not unit-testable], and that skip is how the old gate shipped with an untestable exemption and a grep -aq that printed success on a missing path. The script was run against archived artifacts and observed FAILING four ways — pre-fix relay DLL names the relay leg, pre-static-fix DLL names the static leg, missing path fails loudly, fixed DLL passes. It searches two encodings. Metadata names are UTF-8; body string literals are UTF-16LE, which grep -a on raw bytes can never match — the obvious gate would report every literal marker absent and pass unconditionally. tr -d '\000' gives a second view, and three positive controls cover both extraction paths so a failure in either is caught rather than silently disabling half the gate. Save/Can* markers are asserted PRESENT on purpose: omitted, the gate would keep passing after TRIM-009 lands. Asserted, CI fails the moment the leak is fixed and says to promote them. A pending marker, not an endorsement. .gitattributes pins *.sh to LF so a CRLF checkout cannot break the gate with a "bad interpreter" error that reads like a missing shell. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 9 and Step 10 of TRIM-008, plus the emission delta evidence.
The ~40-anchor inventory was rebuilt by reading the files rather than copying
the plan's list (plan review A4), and recorded at
reviews/008-doc-anchor-inventory.md. It falsified the plan's own "verified
do-NOT-touch" list: the class-factory anchors there are false today because of
TRIM-009. Deferred to TRIM-009 with the eight anchors enumerated and
release-blocking, rather than edited now and reverted after.
Corrections:
- Static/relay per-shape claims, plus the holder mechanism, in docs/trimming.md
and CLAUDE-DESIGN.md — whose attribute-target table had documented the defect
as intended design ("the static class itself").
- The four falsified-TRIM-005 artifacts. ServerOnlyTypes.cs needed no change: it
said the interface should be absent, contradicting the other three, and was
right the whole time.
- Five silences: ExecuteAttribute and FactoryEventHandlerAttribute<T> XML docs,
attributes-reference.md, the skill's static-factory and trimming pages, and
AllPatterns.cs.
- Bucket 4 — "preserve all methods on the referenced type" now says which type
that is and why it is never the consumer's.
Two things users were never told, now documented: [Remote] is decorative on
[Execute] (the IsServerRuntime guard makes the body trimmable, not the
attribute), and the [FactoryEventHandler<T>] leg cannot be verified from a
client-side test because every registration is server-guarded.
Both docs pages told readers to grep the published DLL for server-only names.
That works for metadata names (UTF-8) and silently fails for string literals
(UTF-16) — the same trap that nearly made our own probe report a broken leg as
clean. Both now carry the tr -d '\000' step and say to prove the check against
an untrimmed build first.
Emission delta, measured over both solutions with the generator reverted to
25ac975 and again at HEAD: 732 files emitted, 40 changed (20 relay, 20 static),
692 byte-identical, no class- or interface-factory output in the delta. Checked
one level stronger than the acceptance bullet asked — every changed line matched
against the two permitted shapes, 0 outside them.
Container: deferred items 6 and 7 closed, item 8 partially closed with its
premise corrected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ten acceptance bullets mapped to the tests or measurements that discharge them. Two tiers upgraded rather than skipped: the emission byte-identity bullet was measured instead of explicit-skipped, and the CI-gate bullet's "shell in YAML, not unit-testable" skip is retired now that the gate is a script exercised against archived known-bad artifacts. Records one deliberate gap: nothing rejects a CONSUMER hand-writing [NeatooFactoryRegistrar] against their own type. The Step 5 contract is documentation, not a diagnostic. Generator regressions are caught by the two DoesNotNameConsumerType tests; hand-written consumer usage is not caught at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both gates ran; every checkable finding was re-verified at the keyboard before
being accepted, and all held.
The measurement that changed a diagnosis: adding async targets to three legs
(P2/S7) contradicted TRIM-009's stated cause. Async [Execute], async relay
handlers, and an async interface-factory method ALL trim clean — only the
class-factory write path leaks. It narrows rather than falsifies: none of the
clean shapes is the leaking shape (static/relay use a wrapping guard in a
non-async registrar; the interface leg reaches its impl through an interface,
so no body is statically reachable). The leak needs an early-throw guard AND
async AND a direct call on a concrete type. TRIM-009's stub now carries the
five-shape table and is told not to treat "async is the cause" as settled.
Veto-tier:
- V1: the rewritten gate had silently dropped a marker the old one carried —
grep -F "IServerOnlyRepository" cannot match the bare implementation name that
(?<!I)ServerOnlyRepository existed to catch. A coverage regression inside the
artifact this plan calls its Step-8 deliverable, narrated as strengthening.
ServerOnlyRepository_MARKER restored, measured PRESENT untrimmed first.
- V2: RelayLegBackend was reported as a present->absent pair; the probe recorded
it absent both times. Four discriminators, not five. Gate now labels every
marker [D] or [R]; the blanket "measured PRESENT before, ABSENT after" header
was wrong for 8 of 16.
- V3: two claims added by this plan implied that naming a *generated* type is
what makes a leg safe. It isn't — {X}Factory is generated and hosts every
Local* method. Reworded; the unqualified "no documentation asserts..." bullet
scoped to the two shapes this plan delivers.
Must-cover:
- P1/C2: the holder's method name was not pinned despite the Constraint and the
test's own remarks claiming it was — the user's partial emits a byte-identical
signature. Both tests now anchor it to the holder's class declaration; proven
by renaming only the holder's method and observing RED.
- P3: ServerOnlyHelper had zero references, so its absence was unconditional
while serving as evidence. Wired into DoServerWork so the transitive-removal
property it claimed is actually tested.
- S8: TrimTestEntity gets its own IClassLegPort so class-factory leaks stop
reporting as static-factory failures — matters now that TRIM-009 edits it.
- S5/S6/S9: relay compile test no longer passes on zero trees; private-handler
fixture added; both-attributes CS0111-without-CS0101 claim now pinned.
- C4/S10: integration counts corrected to 561+561, nothing filtered.
- C5: all four gate runs captured.
Accepted with reason: C3 (gate exercised win-x64 only, CI is linux-x64),
C6 (BuildReferences widening changes the semantic model suite-wide),
C7 (nested/global/generic shapes traced, not assumed).
611+611 unit, 561+561 integration, 86+86 Design, harness exit 0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rms async Both gates re-ran. Every checkable finding re-verified at the keyboard first; all held. This round changed no generator source — only harness, gate, tests, and docs — so the byte-identity and incremental-cache conclusions stand. THE CONTROLLED EXPERIMENT (V5c). The code review showed my three-condition narrowing was unsupported: two conjuncts rested on rows that cannot discriminate, and the class-factory pair I called "single-variable" actually differed in four more ways — an auth block, target-from-DI vs from-parameter, one-hop vs two-hop rooting (there is no InsertDelegate), and an extra catch arm plus lifecycle probes. That last is the dimension the arc's disproven TRIM-004 story blamed. TrimTestEntity now carries an async [Remote][Fetch] beside its sync [Remote][Create], each writing its own literal into its own body. Same type, same factory, same registrar, no auth either side, one-hop rooting both, direct concrete call both. Trimmed: sync marker ABSENT, async marker PRESENT; both PRESENT untrimmed. async is confirmed as the operative variable, now on a controlled comparison. TRIM-009's scope is right — for a better reason than the one it was created with. Still unestablished, and now said so: whether the guard shape and the direct- concrete-call shape are NECESSARY. Neither has independent evidence. Other findings: - V4: [D]/[R] labels were themselves wrong — IServerOnlyRepository and DoServerWork were present pre-fix, so they are discriminators, and the marker whose flip is Step 8's headline evidence was under-claimed. Added [N] for new-baseline markers that never had a pre-fix measurement. - V6: the gate's success line claimed async class-factory coverage that did not exist. Now true, and states exactly which bodies are absent vs expected-present. - V7: the "necessary, not sufficient" paragraph reached CLAUDE-DESIGN.md but not the published docs/trimming.md. Ported. - C9: the container never received the async result. Deferred item 18 rewritten; Discovery Log entry records the reversal, not just the endpoint. - C10: positive controls named in full — the prefix control was satisfied by the sync handler class alone; added DoAsyncWork and async-iface resolutions. - C11: "untrimmed-PRESENT so it can fail" is unsound — ServerOnlyHelper was untrimmed-PRESENT for months and still could not fail. Header narrowed. - N1/item 19: [Service] on interface-factory methods emits uncompilable code (CS0535). Found trying to give that leg a reachable marker; it means the leg is STRUCTURALLY unable to measure body elimination. Recorded, stated at the target and in the gate rather than left to be inferred from a clean result. - N2/N3/C12/C13/C15: fixture-drift guard, async-iface resolution check, stale numbers and artifact names, probe header. Gate re-validated four ways: passes on the fixed artifact, fails naming the relay leg, fails naming the static leg, fails on a missing path. It also caught its own new consequence on first run — IClassLegPort/ClassLegInvoke flipped to present once FetchAsync existed, which is per-leg attribution working. 611+611 unit, 86+86 Design, harness exit 0. One FactoryEventRelayTests failure on net9.0 only, passing in isolation — the deferred item 10 flake, already proven pre-existing by stashing this branch's changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d AC Third pass on both reviewers. Both reached the same verdict: the deliverable was done, the bookkeeping was not. Every checkable finding re-verified at the keyboard first; all held. Two real defects, not bookkeeping: - The gate ran its assert-PRESENT blocks AFTER positive controls failed, so an archived artifact that predates the current targets emitted eight "the async diagnosis is wrong and must be reopened" instructions. Printing "do not trust the results below" and then printing them anyway is not enough when the text below is the most action-directing in the gate. It now exits at the controls. Consequence: the old red transcripts no longer demonstrate per-leg naming, so a better demonstration replaces them — current code published with the feature switch left ON, where all 6 controls pass and the gate names every leg across 30 errors (gate-red-nofold.txt). - AC6 and the release-blocking doc inventory both said "async WRITE operations". FetchAsync is a [Fetch] — a read — and it leaks. Widened to any async operation; the criterion that gates the release was understating the defect. The async conclusion is restated as "async-SHAPED emission". Five constructs are async-only in the generated body: the extra catch arm and four interface type-tests (IFactoryOnStartAsync/OnCompleteAsync/OnCancelled/OnCancelledAsync). Type-tests are a different ILLink retention mechanism from a state machine, so two hypotheses survive and the experiment cannot separate them — one of them the disproven TRIM-004 story returning in async-only form. They imply different fixes, so TRIM-009's first step is now to separate them from inside the generator. Also recorded: DAM roots LocalFetchAsync via typeof(factory), so de-rooting is not an available remedy. Also: the controlled pair's service asymmetry is load-bearing and now documented as do-not-tidy (symmetry would turn the static-factory [D] markers red for a misleading reason); the Test Evidence [D]/[R] row contradicted the script it described and is corrected; artifacts are cited by role against a manifest after three consecutive passes of stale filenames. Gate records written (008-test-review.md, 008-code-review.md) with evidence archived in 008-evidence/ — every other gated plan in this arc had both and this one asserted them without records. Plan and index moved to Done. TRIM-008 delivers two of AC6's three shapes and is NOT closed out as delivering AC6. The release stays blocked on TRIM-009. 611+611 unit, 561+561 integration, 86+86 Design, 0 failed, nothing filtered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
[assembly: NeatooFactoryRegistrar(typeof(T))]carries[DynamicallyAccessedMembers(PublicMethods | NonPublicMethods)], which preserves every method on the named type, bodies included.Class and interface factories name the generated
{X}Factory. Static[Factory]classes and[FactoryEventHandler<T>]classes have no separate generated type — the generator re-opens the user's own partial to hostFactoryServiceRegistrar— so the attribute named the consumer's class, and their[Remote]server-only bodies shipped to Blazor WASM clients decompilable. Since v0.21.2 (2026-03-08).This is the plan that makes RemoteFactory's headline IP-protection claim true for those two shapes.
The fix
A generated top-level forwarding holder per leg, with the attribute pointing at it, so preservation reaches one method instead of everything on the user's class:
Forwarding rather than hosting because
[Execute]methods areprivate staticby the repo's own convention and the registrar body calls them — a hosting sibling would be CS0122. Distinct prefix per leg (NeatooFactoryRegistrar_/NeatooEventHandlerRegistrar_) so a class carrying both attributes doesn't gain a CS0101 on top of its existing CS0111. The relay leg's long-missingglobal::qualifier is fixed at the same time.Two generator files changed; no model, builder, or transform touched.
git diff --name-only 25ac975..HEAD -- src/Generator/returns exactly those two.Evidence
Measured on publish-trimmed artifacts, present-before / absent-after:
[Execute]static factory_DoWork,_ProcessRecord,IServerOnlyRepository,DoServerWorkpresent[FactoryEventHandler<T>]Emission delta over both solutions: 732 files, 40 changed (20 relay, 20 static), 692 byte-identical, no class- or interface-factory output in the delta — and every changed line matched against the two permitted shapes, zero outside them.
What this does NOT deliver
AC6 is not closed. The pre-fix probe found a third broken shape by a different mechanism: async generated
Local*class-factory methods retain their server-only bodies. A controlled experiment (same class, same factory, no auth either side, one-hop rooting both, direct concrete call both — differing only in async-shaped emission) confirmed it. Carried as TRIM-009; the v1.7.0 release stays blocked on it.The trimming gate asserts those markers PRESENT on purpose, so CI fails loudly the moment TRIM-009 lands rather than silently passing.
Also in this PR
verify-trimmed.shso it can be run against known-bad artifacts and observed failing. Searches two encodings — body string literals are UTF-16 andgrep -acan never match them, which would have made every literal check pass unconditionally. Carries positive controls covering both extraction paths, exits at the controls rather than emitting remediation advice on an untrustworthy artifact, and names the leg on failure. The(?<!I)ServerOnlyRepositoryexemption is deleted — it was justified by a diagnosis this arc disproved.Typemust be a generated registrar type, never a consumer type.src/Design/. Bothdocs/trimming.mdand the skill also gained the UTF-16 caveat for readers verifying their own builds.[Service]params on interface-factory methods emit uncompilable code (CS0535), and the interface leg is structurally unable to measure body elimination.Gates
plan-reviewer(CONCERNS, 5 veto) plustest-reviewerandcode-reviewerat three passes each. Records and archived evidence indocs/todos/TRIM-dto-trimming-preservation-gaps/reviews/.611+611 unit, 561+561 integration, 86+86 Design, 0 failed, nothing filtered. Trimmed harness exits 0.
Note for the merge run: the gate has only been exercised on
win-x64; this is its firstlinux-x64run. A failure there is informative, not a falsification of TRIM-009.🤖 Generated with Claude Code