fix: la suite n'était pas hermétique, et deux tests ne pouvaient pas échouer

Second tour adverse sur le lot D. Trois trouvailles, et une erreur de diagnostic de ma
part qui vaut d'être consignée.

**La suite verte dépendait de l'ordre des fichiers.** `bun test isolation-active
public-store` donnait 5 échecs quand chaque fichier seul était vert — donc un checkout de
CI avec un autre ordre d'inodes livrait rouge. Deux causes distinctes :

- le travail de connexion, lancé sans être attendu par `setCurrentUser`, débordait d'un
  fichier sur le suivant et armait l'émulation. `connectedUser` abandonne désormais dès
  que l'identité pour laquelle il a démarré n'est plus connectée — ce qui est de toute
  façon la bonne sémantique : en amont une session appartient à un utilisateur, et
  changer d'utilisateur est une autre session ;
- et surtout **mon propre test de store public exposait le cap d'un document que
  personne ne détient** — un état que la bibliothèque ne produit jamais. Il ne passait
  que tant que l'émulation était désarmée. Alice crée sa note avant de l'exposer,
  maintenant. Balayage des 21 paires de fichiers : plus aucune ne pollue.

**Le contrôle symétrique ajouté hier ne pouvait pas échouer.** « La liste d'Alice ne
contient pas la note de Bob » lisait un rendu ANTÉRIEUR à l'écriture de Bob : l'attente
de `showScope` était satisfaite au premier sondage par le marqueur déjà à l'écran, sans
synchroniser quoi que ce soit. Alice écrit désormais une note APRÈS celle de Bob —
`writeNote` attend son apparition, donc ce qui suit est un rendu qui post-date. Et le
`.catch` qui avalait le délai d'attente est retiré : une liste qui ne se stabilise jamais
est un échec à voir, pas une dégradation à absorber.

**Le test anti-fork prouvait « pas le premier », pas « le canonique ».** Son minimum
lexicographique était aussi le DERNIER élément, si bien qu'un choix positionnel — la
faute exacte que ce test existe pour attraper — restait vert. Le minimum est déplacé au
milieu ; vérifié par mutation, « prendre le dernier » le fait rougir.

**Mon erreur de diagnostic.** J'ai cru trouver, sous la trouvaille d'ordre, une fuite
entre utilisateurs — les caps d'Alice classés chez Bob — et je l'ai « reproduite ». Le
repro était faux : son faux `ng` ignorait le sujet dans la requête d'inbox, donc l'inbox
de Bob résolvait vers celle d'Alice. Une fois le faux corrigé, la fuite ne se reproduit
plus, ni avec ni sans correctif. Le danger reste réel en lecture du code — trois chemins
classent des caps plusieurs `await` après la garde qui les autorisait — donc
`caps.holderKey`/`learnFor` le ferment par construction, mais les commentaires disent
maintenant ce que c'est : un risque fermé, pas un défaut observé.

189 tests unitaires, e2e 40/40 et applicatif 12/12.
This commit is contained in:
Sylvain Duchesne
2026-08-10 10:02:45 +02:00
parent 30f6263db5
commit b7dc8ca2c3
8 changed files with 164 additions and 20 deletions
+35 -3
View File
@@ -133,9 +133,33 @@ export class CapRegistry {
// --- what the holder holds ----------------------------------------------
/**
* The key of the holder currently connected — capture it when you DECIDE that a cap is
* someone's, and hand it back to {@link learnFor} when you file.
*
* **A hazard closed, not a leak observed** — the distinction matters and I got it wrong
* once while writing this. Filing resolves the holder at the moment it runs, and three
* paths file several `await`s after the check that authorised them (connecting, reading
* an inbox, listing one's own documents). So an application switching identity in the
* gap COULD have the first identity's caps filed into the second one's ring. That is
* structural and visible by reading. What was NOT established is that it happens: the
* reproduction that seemed to show it turned out to be a broken test fake, and once the
* fake was corrected the leak did not reproduce.
*
* The pairing stays because it costs one argument and removes the hazard by
* construction, where a re-check at each of three sites is a discipline. It is not
* evidence of a bug that was found.
*/
holderKey(): string {
return this.holder() ?? ANONYMOUS;
}
/** What the current holder holds, created on first use. */
private heldCaps(): Map<Nuri, ReadCap> {
const key = this.holder() ?? ANONYMOUS;
return this.ringFor(this.holderKey());
}
private ringFor(key: string): Map<Nuri, ReadCap> {
let ring = this.heldByHolder.get(key);
if (!ring) this.heldByHolder.set(key, (ring = new Map()));
return ring;
@@ -155,7 +179,7 @@ export class CapRegistry {
*
* Returns whether the cap was new.
*/
private file(cap: ReadCap): boolean {
private file(cap: ReadCap, key: string = this.holderKey()): boolean {
if (!hasReadCap(cap)) {
throw new Error(
"[ng-eventually] caps: expected a ReadCap (a NURI carrying `:r:`), got a bare " +
@@ -163,7 +187,7 @@ export class CapRegistry {
);
}
const target = targetOf(cap);
const ring = this.heldCaps();
const ring = this.ringFor(key);
if (ring.get(target) === cap) return false;
ring.set(target, cap);
this.issued = true;
@@ -206,6 +230,14 @@ export class CapRegistry {
this.file(cap);
}
/**
* File a cap for a NAMED holder — the one the caller decided for, not whoever happens
* to be connected when the `await` resumes. See {@link holderKey}.
*/
learnFor(key: string, cap: ReadCap): void {
this.file(cap, key);
}
/**
* File a cap a PUBLIC STORE served me — `emulated-verifier/public-store.ts`, the
* emulated *"downloaded from the outerOverlay"*. Held like any other cap, so reading
+27 -2
View File
@@ -56,6 +56,24 @@ export async function connectedUser(): Promise<void> {
const pending = inFlight.get(holder);
if (pending) return pending;
/**
* Is `holder` still the connected identity?
*
* This work is fired un-awaited by `setCurrentUser`, and everything below resolves the
* CURRENT holder when it reads a register — `readLinks` and `myInboxes` both ask
* `getCurrentUser()` at the moment they run. After a switch they would therefore read
* the WRONG user's registers.
*
* The observed symptom was narrower and entirely in the tests: in-flight work from one
* test file armed the cap emulation in the next, making the suite's green depend on
* file order. Abandoning is right for both reasons, and it is what upstream implies —
* a session belongs to one user, and switching user is another session. Nothing is
* lost: the next connection picks it up.
*/
const stillConnected = (): boolean => getCurrentUser() === holder;
// Captured with the identity, handed back at filing time — see `caps.holderKey`.
const holderKey = getCaps().holderKey();
const run = (async (): Promise<void> => {
try {
// Connecting must not PROVISION. `ensureAccount` would create the user on
@@ -64,15 +82,22 @@ export async function connectedUser(): Promise<void> {
// background side effect, at a moment nothing controls. An account that does
// not exist has nothing to restore and no inbox to drain.
if ((await resolveAccount(holder)) === null) return;
if (!stillConnected()) return;
// 1. Durable first: what this user has already applied.
for (const cap of await readLinks()) getCaps().learn(cap);
const links = await readLinks();
if (!stillConnected()) return;
for (const cap of links) getCaps().learnFor(holderKey, cap);
// 2. Then the queues: ALL of them — the user's own inbox, plus one per
// document it opened an inbox on. Both levels, as the PO specified, and
// both are answered by the same User-branch record (`AddInboxCap`).
// Sequential rather than parallel: each `processInbox` writes what it
// applies to the SAME private store, and interleaving those writes buys
// nothing on a queue that is nearly always empty.
for (const inbox of await myInboxes()) await processInbox(inbox);
const inboxes = await myInboxes();
for (const inbox of inboxes) {
if (!stillConnected()) return;
await processInbox(inbox);
}
} catch {
// Not configured yet, or offline. Nothing to restore, and connecting must
// not fail because a queue could not be reached — the next connection, or
@@ -1113,7 +1113,11 @@ export async function listMyEntityDocs(id: string, scope: Scope): Promise<Nuri[]
const holder = getCurrentUser();
if (holder !== null && accountKey(holder) === accountKey(id)) {
const caps = getCaps();
for (const cap of await readStoreCaps(store)) caps.learn(cap);
// The holder was decided just above; `readStoreCaps` awaits, and filing resolves the
// holder when it runs — so hand the captured key back rather than trust that the
// identity has not moved. See `caps.holderKey`.
const holderKey = caps.holderKey();
for (const cap of await readStoreCaps(store)) caps.learnFor(holderKey, cap);
// Which store a document sits in is a registry fact, not one recorded beside the
// caps, so it is re-applied here. Marking only — the caps just came from the Store
// branch above, and minting a second one beside them is the trap `holdOwnCap` warns
+14 -1
View File
@@ -403,6 +403,10 @@ async function assertOwnInbox(targetInbox: Nuri, op: string): Promise<void> {
export async function read(targetInboxLike: NuriLike): Promise<Deposit[]> {
const targetInbox = toNuri(targetInboxLike, "inbox.read");
await assertOwnInbox(targetInbox, "read");
// WHO this read belongs to, captured with the guard that authorised it — see the note
// beside the filing below, and `caps.holderKey`.
const owner = getCurrentUser();
const ownerKey = getCaps().holderKey();
const sid = await sessionId();
// NOTE: cold-start repo opening is done by the COLD DIRECT readers that need it
// (a cold reader that opens the repo before reading), NOT here — `inbox.watch`
@@ -444,10 +448,19 @@ export async function read(targetInboxLike: NuriLike): Promise<Deposit[]> {
// view that was empty for want of that cap re-read instead of staying stale.
const delivered: Deposit[] = [];
const links: ReadCap[] = [];
// The ownership guard ran at entry; the filing happens several awaits later, and filing
// resolves WHO is holding at that moment. So an application switching identity in the
// gap could have this inbox's caps land in the NEW holder's ring. A hazard read off the
// code, not a leak anyone reproduced — see `caps.holderKey`.
//
// Abandoning is the faithful answer: upstream an inbox is processed by ITS owner's
// verifier, and switching user is another session. Nothing is lost — an inbox is not
// consumed by reading, so the next connection under the right identity files them.
const stillOwner = getCurrentUser() === owner;
for (const d of deposits) {
const cap = capOfPayload(d.payload);
if (cap) {
getCaps().learn(cap);
if (stillOwner) getCaps().learnFor(ownerKey, cap);
links.push(cap);
continue;
}