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 87f7ea9b..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. @@ -59,7 +63,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 @@ -83,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 | @@ -103,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