chore: T01.o repasse en doing, le Result porte le round 2

This commit is contained in:
Sylvain Duchesne
2026-09-24 19:05:30 +02:00
parent 84699c42b5
commit b778fcff02
@@ -89,7 +89,7 @@ there are two views, and `surface/use-shape.ts` applies the envelope one.
## Result ## Result
Status: done. Four commits on `caps-p1a-and-virtual-user-boundary`, `3a89411` `e16fd6a` `e86e249` `b1b099b`. Whole change: **20 files, 1117 insertions, 247 deletions** (`git diff 5993a8c --shortstat`). The published surface is `1.0.0-dev.11`, a major, with the change-policy entry written beside the guarantee it changes. Status: REFUTED once, repaired, and back for a fresh adversary. First pass was four commits on `caps-p1a-and-virtual-user-boundary`, `3a89411` `e16fd6a` `e86e249` `b1b099b`. Whole change: **20 files, 1117 insertions, 247 deletions** (`git diff 5993a8c --shortstat`). The published surface is `1.0.0-dev.11`, a major, with the change-policy entry written beside the guarantee it changes.
### The enumeration's conclusion ### The enumeration's conclusion
@@ -161,3 +161,54 @@ Anti-cheat: no `any`, no `@ts-ignore`, no `as unknown as`, no widened type, no t
- T01.p item 1 gets easier: the published `useShape` now returns `ShapeResult`, which is NOT published from `index.ts`. Either publish it or decide the return type stays unnameable, and `DeepSignalSet` is still exported with no published signature using it. - T01.p item 1 gets easier: the published `useShape` now returns `ShapeResult`, which is NOT published from `index.ts`. Either publish it or decide the return type stays unnameable, and `DeepSignalSet` is still exported with no published signature using it.
- `e2e/polyfill-entry.ts` and `test/wallet-fake.ts` still inject `(() => {}) as unknown as UseShapeLike` at the call sites that never read a shape. Pre-existing, harmless, and now inconsistent with the one real fake. - `e2e/polyfill-entry.ts` and `test/wallet-fake.ts` still inject `(() => {}) as unknown as UseShapeLike` at the call sites that never read a shape. Pre-existing, harmless, and now inconsistent with the one real fake.
- The collection view answers `has` as a filtering FUNCTION even on the array form, where the target has no such method. Not a leak (it filters), a shape it grows. - The collection view answers `has` as a filtering FUNCTION even on the array form, where the target has no such method. Not a leak (it filters), a shape it grows.
---
## Round 2 - the review refuted the first delivery, and it was right
**FINDING 1 was a real rights leak and it is closed.** `Set.prototype.add` returns `this`, so the three mutations - forwarded BOUND TO THE TARGET - handed the unfiltered collection back as `add`'s return value. Every way IN was filtered and the way OUT gave the whole set away, through the FIRST member the ORM's own documentation example uses (`expenses.add({...})`).
Verified before changing anything, by running it rather than reading it:
```
Set.add returns this: true DeepSignalSet.add returns === the deepSignal proxy: true
Set.delete returns: boolean DeepSignalSet.delete returns: boolean true
Set.clear returns: undefined DeepSignalSet.clear returns: undefined
```
So one of the three was unsafe in the way the reviewer said, and the other two were safe in their RETURN and unsafe in their REACH - which the reviewer's own repro had shown (`delete(aliceItem)` returned `true` and removed it). The rule is therefore one sentence covering all three: **a mutation reaches the collection underneath, but neither hands back nor acts past what the view shows.**
- `add` hands back the VIEW, so chaining lands on the filtered collection.
- `delete` acts only on an item the view shows - the same question `has` already declines to answer, and the two used to disagree.
- `clear` empties the view, not the collection every identity shares.
Writing still reaches the document: `e2e/orm-live-read.ts` `5 passed, 0 failed` against the deployed broker, with `.delete()` removing the triples.
**The reviewer's own repro, replayed through the published call**: `add returns the raw collection : false`, and what it hands back holds only bob's items.
**THE PATTERN, taken seriously.** Five leaks, one cause: a WRITTEN JUSTIFICATION ABOUT A RETURN VALUE NOBODY EXECUTED. So the first thing done here was to run the return values, and the second was to run a SWEEP of every handled member on both proxies asking "does this hand back the target?". It found one more, on the other proxy: a page command of the ENVELOPE that returned the collection handed it back RAW. `nextPage` is declared `() => void` and yields nothing today, so it is not a live defect - it is the same unexecuted ground on the other side, and it is now closed with a test that pins the RULE. Three copies of that reasoning are now ONE function, `handBack`.
**THE THREE MINORS, all fixed:**
- The array form answered `keys`/`entries`/`forEach` with SET semantics. It now yields indices, `[index, item]`, and the index as `forEach`'s second argument. Owner-path output, no leak.
- **The shared fake was more correct than the target** - `isLoading: !loaded` against the shipped `ormSubscription.isReady`. That re-creates the exact blindness this task removes, and the reviewer is right that it is worse than minor in kind. The fake now reproduces the shipped expression, the file says the inversion is upstream's, and the test asserts the inverted value on purpose. The contract carries it as a non-guarantee telling a consumer to branch on `data === undefined` instead.
- Ten stale `as unknown as UseShapeLike`, not two. All ten take the shared fake, with no cast.
**The contract was FALSE and is now true, verified by running it.** Every clause of `:263` has a test that fails if the clause is false; the two newest were demonstrated failing against the pre-fix code before the fix was written. "mutations pass through" is gone, replaced by what the mutations actually do.
### Round 2 gate, verbatim
```
480 pass / 0 fail / 1393 expect() calls, six consecutive runs (baseline 465 pass / 0 fail)
src exit=0 e2e exit=0 concept lint: 0 ERROR
== Polyfill e2e summary: 47 passed, 0 failed, 47 total ==
== ORM live-read summary: 5 passed, 0 failed, 5 declared - run 0.6 min ==
```
### What round 2 did NOT re-open
The twenty-five access routes the review found closed were left alone, and the sweep re-confirmed that no member of the collection view hands back the target.
### Still open, named rather than fixed
`delete` and `clear` now act only on what the view shows. That is a WRITE-side narrowing justified by the read boundary, and it is the one place this task went beyond the finding: it was done because `has` already refuses the same question, so the view was contradicting itself. A fresh adversary should weigh whether a holder losing the ability to clear a collection they cannot fully see is the right trade.