From 5cd0175048a4013b64148a3a63c878396d95b8d1 Mon Sep 17 00:00:00 2001 From: Keith Voels Date: Fri, 14 Aug 2026 08:50:35 -0500 Subject: [PATCH 1/2] docs(todo): correct the TRIM arc's branching note, which no longer matched practice The Plan Index footer still described the convention used by plans 001-007: docs commit on a shared `TRIM` integration branch, each implementation branching off it. That was dropped at TRIM-008 and the note was never updated. Plans 008 and 009 branched off `main` and PR'd to `main` (#75, #76), carrying docs and implementation in one PR, for two reasons: CI's `pull_request` trigger only watches `main`, so a PR into `TRIM` got no build at all; and holding docs on a long-lived branch is what let TRIM-005's falsified diagnosis sit unverified. The `TRIM` branch itself is now gone -- merged via PR #74 on 2026-08-13, then deleted 2026-08-14 after confirming it held 0 commits not in `main`. It was stale, not pending; nothing was lost with it. Corrected now because the close-out audit reads this section for container integrity, and a stale convention note is exactly the kind of thing it would flag -- or worse, believe. Co-Authored-By: Claude Opus 5 (1M context) --- docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md index 87f7ea9b..9ce6cc82 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md @@ -59,7 +59,9 @@ A third suspected gap turned out to be already fixed: event records derive `Fact | 008 | Done | [Registrar-DAM over-preservation fix](./plans/008-registrar-dam-over-preservation.md) | TRIM-005 plan review: `[Execute]` and `[FactoryEventHandler]` registrar attributes name the consumer's class, so DAM retains every method incl. `[Remote]` bodies — release-blocking, falsifies AC4's docs. Folded into the arc 2026-08-12 (reverses the 2026-08-11 plan-mode routing) | | 009 | Done | [Async `Local*` factory-method body retention](./plans/009-async-local-method-body-retention.md) | TRIM-008 pre-fix probe (2026-08-13): async generated `Local*` methods keep their server-only bodies on a trimmed client; sync ones in the same assembly do not. Falsifies the **class-factory** leg — the shape every doc presents as safe. Distinct mechanism from 008, so it gets its own plan (user decision 2026-08-13) | -Execution order: 004 → 001 → 002 → 003 → 007 → 005 → 006 → 008 → 009 (rows listed in execution order; numbering stays monotonic by creation). Branching: todo/plan docs commit on the `TRIM` branch; each plan's implementation gets its own branch off `TRIM`. (TRIM-003's red verification and TRIM-007's fix merged together via PR #71.) +Execution order: 004 → 001 → 002 → 003 → 007 → 005 → 006 → 008 → 009 (rows listed in execution order; numbering stays monotonic by creation). Branching: **plans 001–007 used a shared `TRIM` integration branch** — todo/plan docs committed there, each implementation branching off it. (TRIM-003's red verification and TRIM-007's fix merged together via PR #71.) + +**From TRIM-008 onward that convention was dropped**, and the note describing it is corrected here rather than left to mislead the close-out audit. Plans 008 and 009 branched off `main` and PR'd to `main` (#75, #76), carrying docs and implementation in one PR. Two reasons: CI's `pull_request` trigger only watches `main`, so a PR into `TRIM` got no build; and holding docs on a long-lived branch is what let TRIM-005's falsified diagnosis sit unverified. `origin/TRIM` was merged to `main` via PR #74 on 2026-08-13, then **deleted 2026-08-14** (remote by the user, local by verification) after confirming it held **0 commits not in `main`**. It was stale, not pending; nothing was lost with it. Any future plan in this arc branches off `main` and PRs to `main`. ## Skipped Steps From 37f09bbcb175cf894993fec6c84c31846dfcf6ac Mon Sep 17 00:00:00 2001 From: Keith Voels Date: Fri, 14 Aug 2026 09:27:00 -0500 Subject: [PATCH 2/2] =?UTF-8?q?docs(todo):=20close-out=20audit=20for=20the?= =?UTF-8?q?=20TRIM=20arc=20=E2=80=94=20CONCERNS,=205=20veto=20findings=20c?= =?UTF-8?q?losed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Whole-arc Step 7 gate. Verdict CONCERNS; every veto-tier finding re-derived at the keyboard before being accepted, and all held. V1 Two integration tests failed. The auditor was asked to challenge the "known flake" label rather than accept it, and did -- tracing the relay path to a git log that stops at v1.4.0, the mechanism to Task.Run/Task.Yield against a 2s poll, and the condition to two concurrent TFM hosts running while MSBuild still compiled the Blazor examples. Closed by re-run: 561+561, 0 failed. The arc does not close on an unexplained red. V2 The AC6 carve-out was dishonest in the one place a generator editor reads first. InterfaceFactoryRenderer.cs:260 still said "the trimmer removes the entire body" -- exactly what H1 measured insufficient for async. That leg emits the guard inline (zero Core methods) and still names {ImplName}Factory. todo.md claimed the retraction landed in "all three" places; it landed in five and missed this sixth. Fifth occurrence in this arc of building a doc list from an inventory instead of grepping the artifacts. V3 AC4's release step had no enumerated obligations. Nine artifacts describe v1.7.0 behaviour while the package is 1.6.1 -- two in undated present tense on the published site, so a current consumer is told their [Execute] bodies are protected by a holder that ships in no installable version. And the synchronous-throw behaviour change lives only in a Done plan's prose, under a `fix:` commit that release-note scanning would render as a patch bug-fix. Both now carried by new row 22. V4 Deferred item 4's own trigger fired inside the arc and the row was never updated -- it still read "not introduced by this arc" after TRIM-009 changed the guard's shape AND its observable semantics. Now QUEUED. V5 TRIM-008 was Done with 9 of 10 acceptance bullets unticked. All satisfied; all now ticked. Callouts fixed inline: a gate header contradicting its own code two lines below, a stale release-hold row, an absolute CONTRACT line where one leg is the exception, and AC5's self-cancelling wording. Two new rows queued (22, 23); item 11 widened. Row 23 is another check that cannot go red -- inside the very test class this arc has called a flake for five weeks, which makes item 10's frequency an undercount by construction. AC1-AC3 and AC6 verified closed. AC4/AC5 correctly still open: what remains is the v1.7.0 cut. Co-Authored-By: Claude Opus 5 (1M context) --- .../008-registrar-dam-over-preservation.md | 18 +++---- .../reviews/closeout-audit.md | 53 +++++++++++++++++++ .../todo.md | 14 +++-- .../Renderer/InterfaceFactoryRenderer.cs | 20 ++++++- src/RemoteFactory/FactoryAttributes.cs | 5 +- .../verify-trimmed.sh | 5 +- 6 files changed, 98 insertions(+), 17 deletions(-) create mode 100644 docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/closeout-audit.md diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/008-registrar-dam-over-preservation.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/008-registrar-dam-over-preservation.md index f3594469..3a55f8ec 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/008-registrar-dam-over-preservation.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/008-registrar-dam-over-preservation.md @@ -74,18 +74,18 @@ Also in scope, because they are inseparable from the fix: correcting the ~40 doc ## Acceptance -- [ ] `[Remote]` method bodies on an `[Execute]` static factory are absent from a publish-trimmed client assembly. `[trimmed-harness]` -- [ ] `[Remote]`/handler method bodies on a `[FactoryEventHandler]` class are absent from a publish-trimmed client assembly. `[trimmed-harness]` -- [ ] Both legs' assembly attributes name a generated holder, and provably do not name the consumer's type. `[unit]` -- [ ] The relay-handler assembly attribute is `global::`-qualified. `[unit]` +- [x] `[Remote]` method bodies on an `[Execute]` static factory are absent from a publish-trimmed client assembly. `[trimmed-harness]` +- [x] `[Remote]`/handler method bodies on a `[FactoryEventHandler]` class are absent from a publish-trimmed client assembly. `[trimmed-harness]` +- [x] Both legs' assembly attributes name a generated holder, and provably do not name the consumer's type. `[unit]` +- [x] The relay-handler assembly attribute is `global::`-qualified. `[unit]` - [x] Class-factory, interface-factory, and event-preservation emission is byte-identical before and after. Evidence is an **expected-delta-set equality** check — enumerate the files expected to change, then assert the actual delta set *equals* it. `[explicit-skip: one-off recursive emission diff — `Generated/` is gitignored so `git status` cannot detect drift. Unlike TRIM-006's zero-delta diff, this one has a nonzero expected delta, so "inspect the diff" would not discriminate]` **Measured 2026-08-13.** Full emission tree captured twice over both solutions with `--no-incremental`, once with the generator reverted to `25ac975` (pre-TRIM-008) and once at HEAD, using an identical collection procedure. **732 files emitted; 40 changed (20 relay-handler, 20 static-factory); 692 byte-identical.** No class-factory or interface-factory output appears in the delta. Discharged one level stronger than the bullet asked: rather than only comparing the delta *set* against an expectation, every changed line in every changed file was matched against the two permitted shapes (registrar-attribute retarget, holder block) — **0 lines fell outside them**. Non-vacuity: the delta list held 40 entries, so the "zero unexpected" result came from inspecting real diffs rather than an empty loop. -- [ ] The CI gate fails when either leg regresses, names which leg, and carries a **durable positive control** so a missing or renamed artifact cannot pass silently (confirmed real today: `grep -aq` on an absent path returns non-zero, the `if` is false, and the step prints success). `[explicit-skip: gate is shell in YAML, not unit-testable]` -- [ ] Factory registration still works through the retargeted holder on both legs. `[trimmed-harness]` for static (`TrimmingTests/Program.cs` resolves the delegate and the harness exits non-zero on failure); `[integration]` for relay, which structurally cannot be covered in the trimmed harness because `RelayHandlerRenderer.cs:82` guards every `RegisterHandler` behind `IsServerRuntime` — name the untrimmed suite that covers it. Needed because `AddRemoteFactoryServices.cs:170` uses `method?.Invoke`, so a misnamed holder method fails **silently**, and every other bullet here is an absence assertion that passes *more* easily when registration is dead. -- [ ] Finding B9 is closed: every new harness target has a **recorded pre-fix measurement**, and each leg's post-fix result is stated against it — not merely that the target files exist. Measured 2026-08-13, so the required outcome now differs per leg and is fixed in advance rather than read off the result: **relay** present pre-fix → must be absent post-fix; **interface-factory** absent pre-fix → must stay absent (no regression); **Save/Can\*** present pre-fix → **expected to stay present**, because its cause is [TRIM-009](./009-async-local-method-body-retention.md) and not this plan's defect. A post-fix Save/Can\* absence would falsify TRIM-009's diagnosis and must reopen it, not be recorded as a win. `[trimmed-harness]` -- [ ] No documentation in the repo — including the distributable skill — asserts the IP guarantee for **the two shapes this plan delivers** (`[Execute]` static factories, `[FactoryEventHandler]` classes). `[explicit-skip: documentation]` +- [x] The CI gate fails when either leg regresses, names which leg, and carries a **durable positive control** so a missing or renamed artifact cannot pass silently (confirmed real today: `grep -aq` on an absent path returns non-zero, the `if` is false, and the step prints success). `[explicit-skip: gate is shell in YAML, not unit-testable]` +- [x] Factory registration still works through the retargeted holder on both legs. `[trimmed-harness]` for static (`TrimmingTests/Program.cs` resolves the delegate and the harness exits non-zero on failure); `[integration]` for relay, which structurally cannot be covered in the trimmed harness because `RelayHandlerRenderer.cs:82` guards every `RegisterHandler` behind `IsServerRuntime` — name the untrimmed suite that covers it. Needed because `AddRemoteFactoryServices.cs:170` uses `method?.Invoke`, so a misnamed holder method fails **silently**, and every other bullet here is an absence assertion that passes *more* easily when registration is dead. +- [x] Finding B9 is closed: every new harness target has a **recorded pre-fix measurement**, and each leg's post-fix result is stated against it — not merely that the target files exist. Measured 2026-08-13, so the required outcome now differs per leg and is fixed in advance rather than read off the result: **relay** present pre-fix → must be absent post-fix; **interface-factory** absent pre-fix → must stay absent (no regression); **Save/Can\*** present pre-fix → **expected to stay present**, because its cause is [TRIM-009](./009-async-local-method-body-retention.md) and not this plan's defect. A post-fix Save/Can\* absence would falsify TRIM-009's diagnosis and must reopen it, not be recorded as a win. `[trimmed-harness]` +- [x] No documentation in the repo — including the distributable skill — asserts the IP guarantee for **the two shapes this plan delivers** (`[Execute]` static factories, `[FactoryEventHandler]` classes). `[explicit-skip: documentation]` **Scoped 2026-08-13 (code review V3).** The bullet was written unqualified and was therefore false at HEAD by this plan's own inventory: eight class-factory anchors remain accurate-only-after-TRIM-009, deliberately deferred rather than edited-then-reverted. Left absolute, the close-out audit could only tick it dishonestly or veto it. The residue is carried in [`../reviews/008-doc-anchor-inventory.md`](../reviews/008-doc-anchor-inventory.md) as a release-blocking table, and the release is already gated on TRIM-009 by AC6 and deferred item 2. -- [ ] Full solution build/test green (net9.0 + net10.0), both solutions, harness exits 0. `[explicit-skip: build/test gates]` +- [x] Full solution build/test green (net9.0 + net10.0), both solutions, harness exits 0. `[explicit-skip: build/test gates]` --- diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/closeout-audit.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/closeout-audit.md new file mode 100644 index 00000000..88e32af0 --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/closeout-audit.md @@ -0,0 +1,53 @@ +# TRIM — Close-Out Audit (Step 7, whole arc) + +**Gate:** mandatory. **Pass:** one (2026-08-14). **Verdict: CONCERNS** — 5 veto-tier, 10 callout-tier. All veto-tier closed below. +**Scope audited:** the container in full (todo, 9 plans, 12 review files, 2 evidence sets), the generator/library/test/doc surface it touches, and 7 provided logs. + +Every checkable finding was independently re-derived at the keyboard before being accepted. **All of them held.** + +--- + +## Veto-tier + +| # | Finding | Disposition | +|---|---|---| +| **V1** | **Two integration tests failed** — `FactoryEventRelayTests.SingleEventRelay_ConsumerReceivesEvent` and `MultipleEventsRelay_ArriveInServerRaiseOrder`, one failure per TFM. The auditor was asked to challenge the "known flake" label rather than accept it, and did: it traced the relay path to `git log` stopping at v1.4.0 (before the arc began), identified the mechanism as `Task.Run` + `Task.Yield` against a 2s poll deadline, noted both failures burned exactly `[2 s]`, and observed MSBuild still compiling Blazor WASM projects *interleaved* with two concurrent TFM hosts | **Closed by re-run: 561+561, 0 failed, both TFMs.** The arc does not close on an unexplained red. Diagnosis confirmed load-dependent, matching deferred item 10 and the 2026-07-06 Discovery Log entry | +| **V2** | **The AC6 carve-out was dishonest in the one place a generator editor reads first.** `InterfaceFactoryRenderer.cs:260` still said *"the trimmer removes the entire body"* — precisely the claim H1 measured insufficient for `async`. The leg emits the guard inline with no wrapper split (zero `Core(` methods in its emitted output) and still names `{ImplName}Factory`. `todo.md` claimed the retraction landed in "all three" places; it had landed in five, and missed this sixth | **Fixed.** The comment now states what folds, what does not, that this leg has received neither TRIM-009 fix, and that body elimination here is **unverified** — with the item 19 / item 20 dependency named. `todo.md`'s "all three" corrected to six, with the miss recorded rather than quietly fixed | +| **V3** | **AC4's release step had no enumerated obligations, and two concrete ones were untracked.** (a) Nine artifacts describe `v1.7.0` behaviour while the package is `1.6.1` — two in *undated present tense* on the published site, so a current consumer is told their `[Execute]` bodies are protected by a holder that ships in no installable version. The container tracked the inverse exposure (docs behind code) exhaustively and this one not at all. (b) The synchronous-throw behaviour change lives only in a Done plan's prose, and its commit is `fix:`-prefixed — so `CLAUDE.md`'s commit-scanning release process would emit a patch bug-fix line and omit it | **Fixed.** New deferred row **22** carries both, queued to the release step | +| **V4** | **Deferred item 4's own stated trigger fired inside the arc and the row was never updated.** It read "queue if the guard's message or shape is ever edited" while marked *not introduced by this arc*. TRIM-009 changed both the shape and the observable semantics. The test review had spotted it (S4); the row was not touched | **Fixed.** Row moved to **QUEUED**, with the root cause named: the `IsServerRuntime == false` path has never executed in any in-process test | +| **V5** | **TRIM-008 marked `Done` with 9 of 10 Acceptance bullets unchecked** — the only plan in the arc in that state, and the one closing half of AC6. The bullets were in fact satisfied (auditor traced each) | **Fixed.** All ten ticked | + +## Callout-tier — disposition + +**Fixed inline:** C1 (gate header still said `ClassAsyncBody_MARKER` "expected PRESENT", contradicting the code two lines below it), C5 (item 2 stale), C6 (`FactoryAttributes.cs`'s bolded CONTRACT was absolute where the interface leg is the exception — now cross-referenced at the claim), C4 (AC5 was self-cancelling: it demanded consumer proof, then made itself non-binding in its own parenthetical). + +**Queued as new rows:** C2 → row **23** (two `FactoryEventRelayTests` tests pass vacuously under exactly the condition that reddens their siblings — *a check that cannot go red, inside the class this arc has called a flake for five weeks*, which makes item 10's frequency an undercount by construction). C3 → item **11** widened (TRIM-009 routed its Step 7B to a row whose rationale does not cover it; `ClassFactoryWithExecute.cs` is the one Design source-of-truth file silent on the shape AC6 was held open for). + +**Accepted with reason:** C7 (`attributes-reference.md` attributes `[Execute]` body removal to the guard alone — incomplete, not false, and it links to `trimming.md`), C8 (`TrimmingTests/README.md` "How It Works" predates the wrapper/holder — omission, to fix at the release step), C10 (AC6 measured on one TFM and one RID; same SDK therefore same ILLink, so the risk is small, but the wording is "every shape that can be measured" and the measurement is single-TFM). + +**Queued separately, pre-existing and outside arc scope:** C9 — `skills/RemoteFactory/references/polymorphic-hierarchy.md` cites a file in the private zTreatment repo, violating `CLAUDE.md`'s self-containment rule for the distributable skill. + +## What the audit confirmed + +- **AC1–AC3 trace to specific generator code and are proven in a publish-trimmed artifact**, not inferred — the distinction this arc paid three cycles to learn. AC3 in particular was *verified red first* (TRIM-003) and then fixed (TRIM-007). +- **AC6 closes as written**, not narrowed: five emission sites through one helper, three holder prefixes distinct at character 7, a CI gate with 11 positive controls and ~45 absence assertions across 8 legs plus 6 state-machine discriminators, and a liveness check proving the holders actually forward. +- **41 of 41 cited unit-test methods exist** across all nine plans. Zero fabricated citations. The sacred-tests rule holds — the single modification to a pre-existing test is an inversion that preserves intent and adds a regression assertion. +- **Plan Index reconciliation is clean**: 9 files, 9 rows, no orphans, every header `Status` matching its row, abandonment reason filled and specific, all three skipped gates carrying recorded reasons. + +## On evidence quality across the arc — the auditor's most useful observation + +Earlier plans are **honest but structurally weaker** than the recent two, and they say so themselves rather than hiding it. TRIM-001/002/007 backed their trimmed-harness claims with keyboard negative controls that were performed and described but **not archived** — real, but not reproducible by a later reader the way `009-evidence/` is. TRIM-003 and TRIM-004 are `explicit-skip` throughout. TRIM-006 is the weakest, and the container already records why. + +The auditor named the recent plans' *self-corrections* as the strongest honesty signal in the arc: TRIM-009 **withdrew** a size comparison rather than restate it from memory, corrected a mis-cited log, and downgraded its own red-before-green claim to "filtered to one test class" rather than let it read as a blast-radius statement. + +## Remaining before the arc can close + +AC4 and AC5 are correctly still open. What remains is the release itself: + +1. Bump `src/Directory.Build.props` to `1.7.0` +2. Author `docs/release-notes/v1.7.0.md` — **including the synchronous-throw behaviour change with migration guidance**, which commit-prefix scanning will not surface (row 22) +3. Update `docs/release-notes/index.md` (highlights table, all-releases list, `nav_order` renumber) +4. Tag `v1.7.0`, let CI publish +5. AC4 closes there; AC5 is discharged by the release, with consumer rollout tracked in zTreatment PCB-003 + +**Worth queuing as Draft plans regardless of the release:** rows 23 + 10 together (the relay family — the vacuous-test half is cheap and makes the flake measurable), and items 19 → 20 in that order, since 19 is what makes 20 verifiable at all. diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md index 9ce6cc82..9cfe2ce1 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md @@ -26,11 +26,15 @@ A third suspected gap turned out to be already fixed: event records derive `Fact 3. Verified (not assumed): a `FactoryEventBase`-derived record whose only client-side reference is a subscription-lambda call site deserializes on a publish-trimmed client. [TRIM-003] 4. `docs/trimming.md` ("What Qualifies as a DTO", "DTO Return Type Preservation") updated to match the shipped behavior; release notes per CI/CD standards. 5. Consumer proof: released version consumed by zTreatment (PCB-003) with the LinkerConfig bulk-preserve block deleted and a Release WASM publish verified. (Tracked zTreatment-side; this todo closes on the framework release, not the consumer rollout.) + + **Restated at close-out (audit C4):** as written this bullet demanded consumer proof and then made itself non-binding in its own parenthetical, so it could neither be met nor honestly ticked. **AC5 is discharged by the framework release**; the consumer verification it describes belongs to zTreatment PCB-003 and is not a gate on this todo. Recorded so no future reader concludes the arc closed over an unmet criterion. 6. `[Remote]` method bodies and their server-only dependencies are absent from a publish-trimmed client for **every** factory shape that can be measured — `[Execute]` static factories, `[FactoryEventHandler]` classes, class factories with **any** async operation (read *and* write, both measured), `LocalSave` routing, and class-level `[Execute]` — proven in the trimmed harness, not inferred. [TRIM-008 + TRIM-009] **Closed as written 2026-08-14**, not narrowed. Class-level `[Execute]` was added to the criterion at TRIM-009's plan review rather than closed over: it is emitted `async` unconditionally, is a Design source-of-truth pattern, and had no harness coverage at all — so AC6's "proven, not inferred" was unsatisfiable for it. It now has a target and is measured absent. - **Explicit carve-out — the interface-factory leg.** It reaches its implementation through interfaces, so a client-side trimmed test reads "absent" whether or not the body survives. No leak is observed, but no proof is available either, and the leg still points its attribute at `{ImplName}Factory`. AC6 does **not** claim it. Tracked as deferred item 20; the skill and `CLAUDE-DESIGN.md` now say "not established" rather than "Yes". Deferring the work is acceptable; shipping a false claim is not. + **Explicit carve-out — the interface-factory leg.** It reaches its implementation through interfaces, so a client-side trimmed test reads "absent" whether or not the body survives. No leak is observed, but no proof is available either, and the leg still points its attribute at `{ImplName}Factory`. AC6 does **not** claim it. Tracked as deferred item 20. + + The claim is qualified to "not established" in **six** artifacts: `docs/trimming.md`, `skills/RemoteFactory/references/trimming.md`, `src/Design/CLAUDE-DESIGN.md`, `src/RemoteFactory/FactoryAttributes.cs`, `verify-trimmed.sh`, and — added at the close-out audit — `src/Generator/Renderer/InterfaceFactoryRenderer.cs`. **This sentence previously said "all three" and was wrong**: the generator's own comment still asserted "the trimmer removes the entire body", which is the one place a future generator editor reads before deciding whether this leg needs the TRIM-009 treatment. Fifth occurrence in this arc of building a doc list from an inventory instead of grepping the artifacts. Deferring the work is acceptable; shipping a false claim is not. **AC6 added 2026-08-12.** It is not scope creep onto the original goal: AC1–AC3 are about *preservation* (making types survive trimming), while AC6 is about *over-preservation* (stopping code from surviving that should not). They are opposite failure modes of the same mechanism, and the registrar-DAM defect was found by this arc, blocks its release, and falsifies the documentation AC4 requires be accurate. Fixing it elsewhere would have left the arc unable to close honestly. @@ -85,16 +89,16 @@ AC1–AC3 confirmed genuinely verified in a publish-trimmed artifact at HEAD (CI | # | Item | Destination | Cost if it stays open | |---|---|---|---| | 1 | **Registrar-DAM over-preservation** — `[Remote]` bodies for `[Execute]` and `[FactoryEventHandler]` classes ship to the browser decompilable | **[TRIM-008]** — folded into the arc 2026-08-12, reversing the plan-mode routing. Closes on that plan | Resolved: it now has the durable home the audit said it needed | -| 2 | **Release held (AC4 + AC5)** — version stays `1.6.1`, no v1.7.0 notes | **Both blockers cleared 2026-08-14** — item 1 merged as PR #75, item 18 closed by TRIM-009. The release is unblocked pending TRIM-009's own merge and the arc's close-out audit | zTreatment PCB-003 blocked since July. The trade was re-affirmed twice at the keyboard: consumer unblock-time vs. publishing false IP guidance about the *most common* factory shape. AC6 is now satisfiable as written rather than by narrowing | +| 2 | **Release held (AC4 + AC5)** — version stays `1.6.1`, no v1.7.0 notes | **Unblocked 2026-08-14.** Item 1 merged (PR #75), item 18 closed by TRIM-009 (PR #76), close-out audit run. Nothing technical remains — see the release-step row (22) for what the cut must not drop | zTreatment PCB-003 blocked since July. The trade was re-affirmed twice at the keyboard: consumer unblock-time vs. publishing false IP guidance about the *most common* factory shape. AC6 is now satisfiable as written rather than by narrowing | | 3 | **`DiagnosticTestHelper` stale-generator hazard** — a generator fix can appear verified when it was never loaded; affects the whole generator suite | Documented at the seam (`DiagnosticTestHelper.cs`); durable fix (fail fast when the generator DLL predates the test assembly) explicitly not done | Local-iteration only (CI is cold-build). Already produced one false green during TRIM-006 | -| 4 | **B8 — nothing pins the guard's runtime throw.** No `AppContext.SetSwitch` anywhere; `"Server-only method called in non-server runtime."` never asserted | **Accepted with reason:** pre-existing, not introduced by this arc, and the trimmed-harness CI gate covers the property that actually matters (server-only types absent from the trimmed artifact). Queue if the guard's message or shape is ever edited | A regression deleting the throw ships silently in untrimmed/server scenarios | +| 4 | **B8 — nothing pins the guard's runtime throw.** No `AppContext.SetSwitch` anywhere; `"Server-only method called in non-server runtime."` never asserted | **QUEUED 2026-08-14 — this row's own trigger fired inside the arc, and the row was not updated until the close-out audit caught it.** The acceptance said "queue if the guard's message or shape is ever edited"; TRIM-009 changed the shape (guard relocated from the async body into a non-async wrapper) **and** the observable semantics (synchronous throw instead of a faulted `Task`, escaping through `I{X}Factory` where the entry point is non-async). The gap is now larger than when it was accepted, not smaller | A regression deleting the throw ships silently in untrimmed/server scenarios. Root cause is that the `IsServerRuntime == false` path has never executed in any in-process test — no fixture sets the AppContext switch anywhere in the repo. One test-only switch fixture unlocks this row **and** the behaviour-change coverage the v1.7.0 release notes will describe | | 5 | **B10 — 16 emission assertions can pass vacuously.** `InternalVisibilityTests` / `CanMethodVisibilityTests` slice generated text with naive `IndexOf` bounded by the next member name | **Queued, unowned.** Not fixed here: out of TRIM-006's scope, and rewriting 16 assertions in sacred tests needs its own plan with its own review | False-green on the generated-code visibility contract — the same class of defect TRIM-001's test gate caught as its marquee finding | | 6 | **B9 — harness cannot verify the relay-handler leg** (no relay-handler target touches a server-only service) | **CLOSED by TRIM-008** (2026-08-13). Closed in full, not just for the relay leg: relay, interface-factory, and Save/Can\* targets all added with per-leg server-only ports, all probed pre-fix, all in the CI gate. The interface leg's long-standing "structurally safe" claim is now a measurement, and probing the Save/Can\* leg is what surfaced item 18 | Resolved. Had it stayed open, TRIM-008 would have shipped fixed-but-unverified — and item 18 would still be undiscovered | | 7 | **Falsified TRIM-005 story in live artifacts** — `.github/workflows/build.yml:111-112`, `TrimmingTests/README.md:31`, `TrimTestCommands.cs:35`. The CI grep's `(?` spurious `Register>` emission** | **Accepted**, TRIM-002 Amendment — idempotent and harmless; removal needs its own trimmed verification | Cosmetic registrar noise | | 13 | **Interface-factory implementation classes get no property walk** | **Accepted by design**, TRIM-002 Constraint; documented at `docs/trimming.md:285` | A consumer serializing state off such a class still needs manual preservation | | 14 | **Nested `[Factory]` static / `[FactoryEventHandler]` classes emit uncompilable code** — simple-name FQN in the assembly attribute plus a namespace-scope re-declaration of the user's class | **Recorded, not fixed** (user decision 2026-08-12). Surfaced during TRIM-008 design; pre-existing | Rare shape, but the failure is a confusing cascade of CS errors in *generated* code. A cheap NF01xx diagnostic (`IsNested` is already computed) would make it one clear message | @@ -105,6 +109,8 @@ AC1–AC3 confirmed genuinely verified in a publish-trimmed artifact at HEAD (CI | 19 | **`[Service]` parameters on interface-factory methods emit uncompilable code (CS0535)** — the generator strips the service parameter from the proxy's implementing method while the `[Factory]` interface still declares it, so the emitted factory does not implement its own interface | **Recorded, not fixed.** Found 2026-08-13 during TRIM-008's re-review while trying to give the async interface-factory target a directly-reachable marker. Pre-existing; nothing in the repo, tests, or Design projects uses the shape, which is why it was never caught | Rare shape, but the failure is a CS error in *generated* code with no diagnostic pointing at the cause. It also means the interface-factory leg **cannot** carry a server-only marker in its generated body, so that leg is structurally unable to measure body-fold behaviour | | 20 | **The interface-factory leg shares BOTH mechanisms TRIM-009 fixes and receives neither** — `InterfaceFactoryRenderer` emits `Local*` with the same inside-the-async guard, and still points its assembly attribute at `{ImplName}Factory`, so DAM covers every `Local*` on it | **Queued, unowned.** Found at TRIM-009 plan review (2026-08-14, finding A4). Deliberately not taken into TRIM-009: it would balloon a plan whose arc the user has already flagged as over-running, and item 19 makes the leg structurally unmeasurable from a client-side harness | Deferring the *work* is fine; shipping a *false claim* is not. The skill asserted "Interface factory \| Yes" for body removal and `docs/trimming.md:37` called interface bodies "unreachable to the trimmer" — a claim the TRIM-008 inventory had already downgraded to "left standing because nothing contradicts it, not because it was measured". **Both qualified 2026-08-14**, plus `CLAUDE-DESIGN.md`; the `docs/trimming.md` half was missed by TRIM-009's first doc pass and caught at code review (V2). The claim now reads "not established" in all three, so the *work* is deferred without a false claim shipping. The remaining exposure is that the leg keeps the exact shape TRIM-009 measured insufficient — an inside-the-async guard plus a DAM target that hosts every `Local*` | | 21 | **`Local{X}Core` has no name-collision guard** — a factory method whose `UniqueName` ends in `Core` produces a wrapper `Local{Y}Core(...)` that can collide with the generated core of a method named `{Y}`. Identical parameter lists give CS0111 in *generated* code with no diagnostic pointing at the cause | **Queued, unowned.** Found at TRIM-009 code review (C4). No such shape exists in the repo, the Design projects, or the examples | Same family as items 14 and 15 — a CS error in generated code with no diagnostic. Rides along if a plan is ever cut for those; not worth one alone | +| 22 | **Release-step obligations, enumerated so the cut cannot drop them.** (a) Nine live artifacts describe `v1.7.0` behaviour in the past tense while `src/Directory.Build.props` is `1.6.1` and no `docs/release-notes/v1.7.0.md` exists — and two of them (`docs/trimming.md`'s holder-mechanism sections) are in **undated present tense** on the published Jekyll site, so a v1.6.1 consumer is told their `[Execute]` bodies are protected by a holder that ships in no installable version. (b) TRIM-009's **synchronous-throw behaviour change** lives only in a Done plan's prose; its commit is `fix:`-prefixed, so `CLAUDE.md`'s commit-scanning release process would emit a patch-flavoured bug-fix line and omit it entirely | **Queued for the release step** — created by the close-out audit (V3, 2026-08-14) | The container tracked the *inverse* exposure exhaustively (docs behind the code) and never tracked this one (docs ahead of the package). Item 2 has already slipped a month with nothing watching this window | +| 23 | **Two tests in `FactoryEventRelayTests` pass vacuously under exactly the condition that reddens their siblings.** `ServerOnlyEvent_ExcludedFromRelayBatch` and `ServerOnlyCombinedFlags_NotRelayed` wait on `InvocationCount == 1`, then assert only `Assert.Empty(relay.Received)` — so when the relay never fires, the wait burns its 2s deadline and the assertion passes on an empty queue | **Queued, unowned** — created by the close-out audit (C2, 2026-08-14). Pre-existing; pairs with item 10 | **A check that cannot go red, inside the class this arc has called a flake for five weeks.** It means 2 of 7 tests there structurally cannot report relay non-delivery, so item 10's "~3 of 8 runs" frequency is an undercount by construction. Cheap fix: `Assert.Equal(1, relay.InvocationCount)` in both | ## Discovery Log diff --git a/src/Generator/Renderer/InterfaceFactoryRenderer.cs b/src/Generator/Renderer/InterfaceFactoryRenderer.cs index a42080a7..bafc9716 100644 --- a/src/Generator/Renderer/InterfaceFactoryRenderer.cs +++ b/src/Generator/Renderer/InterfaceFactoryRenderer.cs @@ -257,7 +257,25 @@ private static void RenderLocalMethod(StringBuilder sb, InterfaceMethodModel met sb.AppendLine($" public {asyncKeyword} {returnType} Local{method.UniqueName}({parameters})"); sb.AppendLine(" {"); - // Feature switch guard -- when IsServerRuntime=false, the trimmer removes the entire body + // Feature switch guard -- when IsServerRuntime=false the switch folds to a constant and + // the body becomes unreachable. + // + // DO NOT READ THAT AS "the body is removed". For SYNC methods it is: unreachability + // begins before any protected region, so the whole remainder goes. For ASYNC methods it + // is NOT sufficient on its own -- the compiler lowers the guard into the state machine's + // MoveNext, inside the builder's protected region, where ILLink folds the switch but does + // not eliminate the remainder. That was measured, not reasoned (TRIM-009): the class + // factory needed a NON-async wrapper carrying the guard PLUS a single-method registrar + // holder, because [DynamicallyAccessedMembers] covers NonPublicMethods and roots the + // private core on its own. + // + // THIS LEG HAS RECEIVED NEITHER FIX. It still emits the guard inline (no wrapper split) + // and line 48 still points the assembly attribute at {ImplName}Factory, which hosts every + // Local* method. No leak has been observed here, but the leg reaches its implementation + // through interfaces, so a client-side trimmed harness reads "absent" either way and + // cannot prove elimination -- and Deferred Work item 19 blocks the fixture change that + // would give it a reachable marker. Treat body elimination on this leg as UNVERIFIED. + // Tracked as Deferred Work item 20 on the TRIM todo. sb.AppendLine(" if (!NeatooRuntime.IsServerRuntime)"); sb.AppendLine(" throw new InvalidOperationException(\"Server-only method called in non-server runtime.\");"); sb.AppendLine(); diff --git a/src/RemoteFactory/FactoryAttributes.cs b/src/RemoteFactory/FactoryAttributes.cs index 81ac03e6..a07ef5bb 100644 --- a/src/RemoteFactory/FactoryAttributes.cs +++ b/src/RemoteFactory/FactoryAttributes.cs @@ -190,8 +190,9 @@ public FactoryHintNameLengthAttribute(int maxHintNameLength) /// /// /// -/// CONTRACT: the must be a GENERATED registrar type. Never a consumer's -/// own class. +/// CONTRACT: the must be a GENERATED single-method registrar holder. +/// Never a consumer's own class — and "generated" alone is not enough. +/// (One leg does not yet satisfy this: see the interface-factory exception below.) /// /// /// The [DynamicallyAccessedMembers] annotation below preserves every method on the diff --git a/src/Tests/RemoteFactory.TrimmingTests/verify-trimmed.sh b/src/Tests/RemoteFactory.TrimmingTests/verify-trimmed.sh index cdd0747c..f9a85323 100755 --- a/src/Tests/RemoteFactory.TrimmingTests/verify-trimmed.sh +++ b/src/Tests/RemoteFactory.TrimmingTests/verify-trimmed.sh @@ -249,7 +249,10 @@ done # THE CONTROLLED PAIR — sync vs async inside ONE class factory. # # ClassSyncBody_MARKER lives in TrimTestEntity.Create (sync) -> expected ABSENT -# ClassAsyncBody_MARKER lives in TrimTestEntity.FetchAsync (async) -> expected PRESENT +# ClassAsyncBody_MARKER lives in TrimTestEntity.FetchAsync (async) -> expected ABSENT since TRIM-009 +# +# (The async half was expected PRESENT while it was the open defect. Both are absent now -- +# see the RESOLVED paragraph below. A red on either is a real regression, not an expectation.) # # Same class, same generated factory, same registrar, neither carrying [AuthorizeFactory], # both one-hop rooted by their own delegate registration, both reached by a direct call on the