fix: mon correctif sur createEntityDoc était faux dans les deux sens
Second tour adverse sur le lot C. Six trouvailles, dont trois sur ce que je venais de livrer. **Le correctif de `createEntityDoc` reproduisait le défaut qu'il annonçait avoir fermé.** Il levait sur la PREMIÈRE écriture de registre en échec. Or : - lever sur le listing sautait l'écriture de la clé — détruisant le chemin de récupération que le commentaire d'à côté décrit explicitement (« la clé doit rester récupérable même si le listing a échoué »), et laissant le document orphelin ; - lever sur la clé laissait le document LISTÉ sans clé — précisément l'état « se lit vide pour toujours » que je prétendais empêcher, en pire, puisque l'appelant n'a même plus sa référence. Les deux écritures sont désormais tentées, ce qui atterrit reste, et l'échec est rapporté après en nommant la moitié manquante. **Le refus de `share` reposait sur une valeur qui confond absence et ignorance.** `resolveAccount` avale toute erreur de lecture et rend `null`, si bien qu'un incident réseau faisait répondre « personne ne s'est connecté sous ce nom » à propos de quelqu'un qui existe. `lookupAccount` propage désormais l'erreur ; `resolveAccount` reste la forme tolérante que tous les autres appelants veulent. **J'avais livré ce comportement sans un seul test.** `test/app-surface.test.ts` en ajoute huit, tous sur ce qu'un APPELANT voit : `ensureIdentity` rend l'identité, un appel de placement avant connexion nomme l'erreur, le placement agit comme l'utilisateur connecté, `share` refuse un nom inventé mais laisse remonter une panne, et une création à moitié écrite échoue en disant quelle moitié — dont le cas « le listing a échoué, la clé est quand même là ». En écrivant ces tests j'ai refait dans leur faux la faute que cette revue a corrigée ailleurs : ignorer le sujet dans la requête de compte, ce qui rendait le dossier d'un autre utilisateur. Deux des huit échouaient pour cette raison, sans rapport avec le code. **Et la documentation contredisait le code livré dans le même commit** : le README enseignait encore `createEntityDoc(me, "protected")` — en JS la portée devient `"alice"` — et le contrat déclarait `Nuri` là où le code et la feuille disent `NuriLike`. 197 tests unitaires, e2e 40/40 et applicatif 12/12.
This commit is contained in:
Sylvain Duchesne
@@ -33,10 +33,10 @@ import {
|
||||
} from "@ng-eventually/sdk";
|
||||
|
||||
configure({ ng: realNg, useShape: realUseShape, getSession, sharedWallet });
|
||||
await ensureIdentity(); // resolves who I am, and waits for the connection work
|
||||
const doc = await storeRegistry.createEntityDoc(me, "protected");
|
||||
await ensureIdentity(); // who I am (returned), connection work awaited
|
||||
const doc = await storeRegistry.createEntityDoc("protected");
|
||||
await docs.sparqlUpdate(sid, `INSERT DATA { … }`, doc);
|
||||
const subjects = await readUnion(await storeRegistry.listMyEntityDocs(me, "protected"));
|
||||
const subjects = await readUnion(await storeRegistry.listMyEntityDocs("protected"));
|
||||
```
|
||||
|
||||
## Principle — the polyfill compensates, it never extends
|
||||
@@ -74,8 +74,9 @@ which the e2e suite drives.
|
||||
```ts
|
||||
import { storeRegistry, inbox, readUnion } from "@ng-eventually/sdk";
|
||||
|
||||
// 1. CREATE — you hold its cap, with nothing to declare.
|
||||
const doc = await storeRegistry.createEntityDoc(me, "protected");
|
||||
// 1. CREATE — you hold its cap, with nothing to declare. No identity parameter: a
|
||||
// session belongs to one user, exactly as the target's own `doc_create` assumes.
|
||||
const doc = await storeRegistry.createEntityDoc("protected");
|
||||
|
||||
// 2. GIVE TO READ — name the document and the person. The key is looked up and
|
||||
// sealed into a deposit; the recipient applies it by connecting, with nothing
|
||||
@@ -87,7 +88,7 @@ await inbox.share(doc, "bob");
|
||||
// PUBLIC store, the store serves its read cap to whoever asks, so the bare
|
||||
// reference is enough to read it — and if it does not, the reference still names
|
||||
// it and opens nothing.
|
||||
const publicDoc = await storeRegistry.createEntityDoc(me, "public");
|
||||
const publicDoc = await storeRegistry.createEntityDoc("public");
|
||||
// …put `publicDoc` in a QR code, a message, another document. Nothing else to do.
|
||||
await readUnion([publicDoc]); // a stranger holding only this reads it
|
||||
```
|
||||
|
||||
@@ -575,7 +575,16 @@ async function resolveShimDoc(): Promise<Nuri> {
|
||||
* Cached per account (in `accountCache`); a hit skips the query entirely, so
|
||||
* repeated resolves of the same account are free. `resetRegistryCache` clears it.
|
||||
*/
|
||||
export async function resolveAccount(id: string): Promise<VirtualUserRecord | null> {
|
||||
/**
|
||||
* Does this account exist? **Propagates** a read failure instead of turning it into
|
||||
* "no such account".
|
||||
*
|
||||
* `resolveAccount` below is tolerant by design — most callers want "carry on without a
|
||||
* record". But a caller that REFUSES on absence needs the two apart: telling a user
|
||||
* "nobody has signed in as bob" because one query timed out is a lie, and it made
|
||||
* `inbox.share` reject a recipient who exists (found adversarially, 2026-08-10).
|
||||
*/
|
||||
export async function lookupAccount(id: string): Promise<VirtualUserRecord | null> {
|
||||
const key = accountKey(id);
|
||||
const cached = accountCache.get(key);
|
||||
if (cached) return cached;
|
||||
@@ -611,6 +620,20 @@ export async function resolveAccount(id: string): Promise<VirtualUserRecord | nu
|
||||
return record;
|
||||
} catch (error) {
|
||||
console.error(accessLogPrefix() + " resolveAccount failed:", error);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The account, or `null` when there is none — and `null` too when the read failed. The
|
||||
* tolerant form every internal caller uses: none of them can act on the difference, and
|
||||
* connecting must not break because a query did not come back. Use {@link lookupAccount}
|
||||
* where absence is a REASON TO REFUSE.
|
||||
*/
|
||||
export async function resolveAccount(id: string): Promise<VirtualUserRecord | null> {
|
||||
try {
|
||||
return await lookupAccount(id);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@@ -983,41 +1006,36 @@ export async function createEntityDoc(id: string, scope: Scope): Promise<Nuri> {
|
||||
const indexDoc = storeOf(record, scope);
|
||||
const entityNuri = await createDoc();
|
||||
const s = await session();
|
||||
const cap = mintCap(entityNuri);
|
||||
// TWO writes, both attempted, and the failure reported only after. They are separate
|
||||
// because upstream they are two commits on two branches — `ldp:contains` on the store's
|
||||
// Main branch and `AddRepo { read_cap }` on its Store branch
|
||||
// (`engine/verifier/src/request_processor.rs:697-710`) — and because either is worth
|
||||
// having without the other: the cap alone lets the creator re-open its document, the
|
||||
// listing alone makes it findable.
|
||||
//
|
||||
// A first attempt at surfacing the failure (2026-08-07) threw on the FIRST one, and an
|
||||
// adversarial pass showed it was wrong in both directions: throwing on the listing
|
||||
// skipped the cap write, orphaning the document entirely — destroying the very recovery
|
||||
// the comment above describes — and throwing on the cap left the document LISTED with
|
||||
// no key, which is precisely the "reads empty forever" state it claimed to prevent.
|
||||
// Attempt both, keep what lands, and say exactly what state the document is in.
|
||||
const failures: string[] = [];
|
||||
try {
|
||||
await registerUpdate(
|
||||
s.sessionId,
|
||||
// NO explicit `GRAPH <…>` wrapper: write the anchored DEFAULT graph (the
|
||||
// `indexDoc` anchor scopes it) — the CANONICAL, always-safe shape the
|
||||
// anchored default-graph read queries (readUserStore below, same as
|
||||
// read-model.ts). Not a round-trip necessity on the current broker: the e2e
|
||||
// harness (`packages/sdk/e2e/`) verified an anchored `GRAPH <plainNuri>`
|
||||
// write ALSO round-trips here (same repo graph, no phantom graph); no-GRAPH
|
||||
// is kept as a simplicity/safety convention. entityNuri is a NURI stored as
|
||||
// a literal → escapeLiteral.
|
||||
// `indexDoc` anchor scopes it) — the CANONICAL, always-safe shape the anchored
|
||||
// default-graph read queries (readUserStore below, same as read-model.ts).
|
||||
// entityNuri is a NURI stored as a literal → escapeLiteral.
|
||||
`INSERT DATA { <${MAIN_BRANCH_SUBJECT}> <${P.contains}> "${escapeLiteral(entityNuri)}" }`,
|
||||
indexDoc,
|
||||
"createEntityDoc",
|
||||
);
|
||||
} catch (error) {
|
||||
// Not swallowed: a document absent from its store's listing is not a document. The
|
||||
// next session's `listMyEntityDocs` omits it and `readUnion` returns nothing for it,
|
||||
// so the caller has written content into a NURI that will read empty forever —
|
||||
// silently. Upstream `doc_create` propagates its own commit failures
|
||||
// (`engine/verifier/src/request_processor.rs:698,714`).
|
||||
console.error(accessLogPrefix() + " createEntityDoc index append failed:", error);
|
||||
throw new Error(
|
||||
"[ng-eventually] createEntityDoc: the document was created but could not be recorded " +
|
||||
`in its store, so it would be lost to the next session: ${String(error)}`,
|
||||
);
|
||||
failures.push(`it is not listed in its store (${String(error)})`);
|
||||
}
|
||||
// The second write: `AddRepo { read_cap }` on the Store branch. A separate
|
||||
// statement, not a second triple in the one above, because upstream these are two
|
||||
// commits on two branches — and because the cap must be recoverable even if the
|
||||
// listing write failed.
|
||||
//
|
||||
// One literal suffices: a ReadCap CARRIES its document (`targetOf`), so storing the
|
||||
// cap stores the pair.
|
||||
const cap = mintCap(entityNuri);
|
||||
try {
|
||||
await registerUpdate(
|
||||
s.sessionId,
|
||||
@@ -1026,12 +1044,16 @@ export async function createEntityDoc(id: string, scope: Scope): Promise<Nuri> {
|
||||
"createEntityDoc:addRepo",
|
||||
);
|
||||
} catch (error) {
|
||||
// Same reasoning as the listing above: without its cap on the Store branch the
|
||||
// creator cannot re-open its own document on a later session.
|
||||
console.error(accessLogPrefix() + " createEntityDoc cap append failed:", error);
|
||||
failures.push(`its key is not recorded (${String(error)})`);
|
||||
}
|
||||
if (failures.length > 0) {
|
||||
// The document EXISTS on the broker; what failed is the bookkeeping that makes it
|
||||
// findable and re-openable. Returning its NURI would hand back a reference that reads
|
||||
// empty on the next session, silently — so the caller is told instead.
|
||||
throw new Error(
|
||||
"[ng-eventually] createEntityDoc: the document was created but its key could not be " +
|
||||
`recorded, so its own creator would lose it: ${String(error)}`,
|
||||
`[ng-eventually] createEntityDoc: the document ${entityNuri} was created but ` +
|
||||
`${failures.join(", and ")}. It will not survive this session as it stands.`,
|
||||
);
|
||||
}
|
||||
// …and the creator holds THAT cap for this session.
|
||||
|
||||
@@ -32,7 +32,7 @@ import { subscribeDoc } from "./subscribe";
|
||||
import { ensureRepoOpen } from "../emulated-verifier/open-repo";
|
||||
import { getCaps, getCurrentUser, getStoreRegistryDeps } from "../shared-wallet/bootstrap";
|
||||
import { addLink, documentInboxAddress, isOwnInbox } from "../emulated-verifier/branch-registers";
|
||||
import { userInbox, isKnownInbox, resolveAccount } from "../shared-wallet/account-registry";
|
||||
import { userInbox, isKnownInbox, lookupAccount } from "../shared-wallet/account-registry";
|
||||
import { escapeLiteral } from "./sparql";
|
||||
import { hasReadCap, toNuri } from "../model/nuri";
|
||||
import {
|
||||
@@ -348,7 +348,12 @@ export async function share(doc: NuriLike, toUser: string): Promise<void> {
|
||||
// PUBKEY (`InboxMsg::new`, `engine/net/src/types.rs:4299`) that reached you through an
|
||||
// inbound `ContactDetails` — someone has to have reached you first. Refusing is the
|
||||
// faithful behaviour; provisioning was the invention.
|
||||
if ((await resolveAccount(toUser)) === null) {
|
||||
//
|
||||
// `lookupAccount`, not `resolveAccount`: the tolerant form answers `null` for a read
|
||||
// that FAILED as well as for one that found nothing, so it would have told a user
|
||||
// "nobody has signed in as bob" because a query timed out. A refusal must not be
|
||||
// built on a value that conflates absence with ignorance.
|
||||
if ((await lookupAccount(toUser)) === null) {
|
||||
throw new Error(
|
||||
`[ng-eventually] inbox.share: no such recipient — nobody has signed in as ` +
|
||||
`${JSON.stringify(toUser)}. Sharing does not create the person you share with.`,
|
||||
|
||||
@@ -0,0 +1,195 @@
|
||||
/**
|
||||
* The app-facing surface, pinned where it changed shape on 2026-08-10.
|
||||
*
|
||||
* Every test here exists because behaviour shipped without one and an adversarial pass
|
||||
* had to find it: an application that cannot learn its own identity, a share that invents
|
||||
* its recipient, a creation that reports success on a half-written document. They are
|
||||
* about what a CALLER sees, not about the emulation's internals.
|
||||
*/
|
||||
import { test, expect, mock, afterEach } from "bun:test";
|
||||
import { configure, ensureIdentity, storeRegistry } from "../src/index";
|
||||
import {
|
||||
resetCaps,
|
||||
resetConfig,
|
||||
resetStoreRegistry,
|
||||
setCurrentUser,
|
||||
} from "../src/shared-wallet/bootstrap";
|
||||
import { createEntityDoc as registryCreateEntityDoc, resetRegistryCache } from "../src/shared-wallet/account-registry";
|
||||
import { share } from "../src/surface/inbox";
|
||||
|
||||
const SHIM = "urn:ng-eventually:shim";
|
||||
const SESSION = { sessionId: "sid-app", privateStoreId: "PRIV-APP" };
|
||||
|
||||
interface Quad { g: string; s: string; p: string; o: string }
|
||||
|
||||
/** A stateful fake `ng`, with a switch that makes a chosen register write fail. */
|
||||
function inject(failWriteMatching?: RegExp) {
|
||||
const quads: Quad[] = [];
|
||||
let created = 0;
|
||||
const doc_create = mock(async () => `did:ng:o:doc${++created}`);
|
||||
const sparql_update = mock(async (...a: unknown[]) => {
|
||||
const query = a[1] as string;
|
||||
const anchor = a[2] as string | undefined;
|
||||
if (failWriteMatching && failWriteMatching.test(query)) throw new Error("broker refused");
|
||||
if (!anchor) return undefined;
|
||||
const gm = query.match(/GRAPH\s+<([^>]+)>\s*\{([\s\S]*)\}/);
|
||||
const body = gm ? gm[2]! : query.replace(/^\s*INSERT DATA\s*\{/, "").replace(/\}\s*$/, "");
|
||||
const sm = body.match(/<([^>]+)>/);
|
||||
if (!sm) return undefined;
|
||||
const subj = sm[1]!;
|
||||
const pairRe = /(?:a|<([^>]+)>)\s+(?:"((?:[^"\\]|\\.)*)"|<([^>]+)>)/g;
|
||||
let m: RegExpExecArray | null;
|
||||
const after = body.slice(body.indexOf(sm[0]) + sm[0].length);
|
||||
while ((m = pairRe.exec(after)) !== null) {
|
||||
quads.push({ g: anchor, s: subj, p: m[1] ?? `${SHIM}:Account`, o: m[2] ?? m[3] ?? "" });
|
||||
}
|
||||
return undefined;
|
||||
});
|
||||
const sparql_query = mock(async (...a: unknown[]) => {
|
||||
const query = a[1] as string;
|
||||
const anchor = a[3] as string | undefined;
|
||||
const byPred = (pred: string, v: string) => ({
|
||||
results: { bindings: quads.filter((q) => q.g === anchor && q.p === pred).map((q) => ({ [v]: { value: q.o } })) },
|
||||
});
|
||||
if (query.includes(`${SHIM}:shimDoc`)) return byPred(`${SHIM}:shimDoc`, "shimDoc");
|
||||
if (query.includes(`${SHIM}:readCap`)) return byPred(`${SHIM}:readCap`, "c");
|
||||
if (query.includes(`${SHIM}:contains`)) return byPred(`${SHIM}:contains`, "e");
|
||||
if (query.includes(`${SHIM}:id`)) {
|
||||
// Filter by SUBJECT when the query names one — the account read asks about ONE
|
||||
// account. A fake that ignores it hands back somebody else's record, and then
|
||||
// "bob does not see alice's document" and "share refuses an unknown name" both
|
||||
// fail for a reason that has nothing to do with the code. (Made that mistake here
|
||||
// first; it is the same one this review found in the other fakes.)
|
||||
const subjM = query.match(/<([^>]+)>\s+a\s+<urn:ng-eventually:shim:Account>/);
|
||||
const only = subjM ? subjM[1]! : null;
|
||||
const bySubject = new Map<string, Record<string, string>>();
|
||||
for (const q of quads) {
|
||||
if (q.g !== anchor) continue;
|
||||
if (only !== null && q.s !== only) continue;
|
||||
const rec = bySubject.get(q.s) ?? {};
|
||||
if (q.p === `${SHIM}:id`) rec.id = q.o;
|
||||
if (q.p === `${SHIM}:docPublic`) rec.docPublic = q.o;
|
||||
if (q.p === `${SHIM}:docProtected`) rec.docProtected = q.o;
|
||||
if (q.p === `${SHIM}:docPrivate`) rec.docPrivate = q.o;
|
||||
bySubject.set(q.s, rec);
|
||||
}
|
||||
return {
|
||||
results: {
|
||||
bindings: [...bySubject.values()].filter((r) => r.id).map((r) => ({
|
||||
id: { value: r.id! },
|
||||
docPublic: { value: r.docPublic ?? "" },
|
||||
docProtected: { value: r.docProtected ?? "" },
|
||||
docPrivate: { value: r.docPrivate ?? "" },
|
||||
})),
|
||||
},
|
||||
};
|
||||
}
|
||||
return { results: { bindings: [] } };
|
||||
});
|
||||
configure({
|
||||
ng: { doc_create, sparql_update, sparql_query } as never,
|
||||
useShape: (() => {}) as never,
|
||||
getSession: async () => SESSION,
|
||||
});
|
||||
resetRegistryCache();
|
||||
resetCaps();
|
||||
setCurrentUser(null);
|
||||
return { quads };
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
resetConfig();
|
||||
resetStoreRegistry();
|
||||
resetCaps();
|
||||
setCurrentUser(null);
|
||||
});
|
||||
|
||||
// ── identity ───────────────────────────────────────────────────────────────
|
||||
|
||||
// An application has to know which user it is — to display it, at least. Upstream it
|
||||
// does: it passes `user_id` to `session_start`, having got it from the wallet it opened.
|
||||
// Here the gate chooses, so the gate returns. Without this the example application read
|
||||
// the gate's own private storage key, which is a boundary no consumer should see.
|
||||
test("ensureIdentity returns the identity it settled", async () => {
|
||||
inject();
|
||||
setCurrentUser("alice");
|
||||
expect(await ensureIdentity()).toBe("alice");
|
||||
});
|
||||
|
||||
// The other half of the same decision: no placement call TAKES an identity, because a
|
||||
// session belongs to one user and the target's `doc_create` carries none. Calling one
|
||||
// before signing in is a caller error worth naming, not an empty result.
|
||||
test("a placement call before signing in names the mistake", async () => {
|
||||
inject();
|
||||
setCurrentUser(null);
|
||||
await expect(storeRegistry.createEntityDoc("protected")).rejects.toThrow(/ensureIdentity/i);
|
||||
await expect(storeRegistry.listMyEntityDocs("protected")).rejects.toThrow(/no identity/i);
|
||||
});
|
||||
|
||||
test("placement acts as the connected user, with nothing passed", async () => {
|
||||
inject();
|
||||
setCurrentUser("alice");
|
||||
const doc = await storeRegistry.createEntityDoc("protected");
|
||||
expect(await storeRegistry.listMyEntityDocs("protected")).toContain(doc);
|
||||
|
||||
setCurrentUser("bob");
|
||||
expect(await storeRegistry.listMyEntityDocs("protected")).not.toContain(doc);
|
||||
});
|
||||
|
||||
// ── sharing names someone who exists ───────────────────────────────────────
|
||||
|
||||
test("share refuses a recipient nobody has signed in as, instead of creating them", async () => {
|
||||
inject();
|
||||
setCurrentUser("alice");
|
||||
const doc = await storeRegistry.createEntityDoc("protected");
|
||||
// "bpb" is a typo for "bob". It used to mint that name's three stores and an inbox,
|
||||
// and the cap landed where nobody will ever look — with no error at all.
|
||||
await expect(share(doc, "bpb")).rejects.toThrow(/no such recipient/i);
|
||||
});
|
||||
|
||||
// …and the refusal must rest on ABSENCE, never on ignorance: `resolveAccount` answers
|
||||
// `null` for a failed read as well as for a missing one, so a refusal built on it would
|
||||
// tell a user "nobody has signed in as bob" because a query timed out.
|
||||
test("a failed lookup surfaces as a failure, not as 'no such recipient'", async () => {
|
||||
const { quads } = inject();
|
||||
setCurrentUser("alice");
|
||||
const doc = await storeRegistry.createEntityDoc("protected");
|
||||
setCurrentUser("bob");
|
||||
await registryCreateEntityDoc("bob", "private"); // bob genuinely exists
|
||||
setCurrentUser("alice");
|
||||
resetRegistryCache(); // force a read rather than the cache
|
||||
|
||||
const { ng } = (await import("../src/shared-wallet/bootstrap")).getConfig();
|
||||
const realQuery = ng.sparql_query;
|
||||
(ng as { sparql_query: unknown }).sparql_query = async () => {
|
||||
throw new Error("broker unreachable");
|
||||
};
|
||||
await expect(share(doc, "bob")).rejects.toThrow(/broker unreachable/i);
|
||||
(ng as { sparql_query: unknown }).sparql_query = realQuery;
|
||||
void quads;
|
||||
});
|
||||
|
||||
// ── a creation that half-worked is a failure, and says which half ──────────
|
||||
|
||||
test("createEntityDoc reports a half-written document instead of returning its reference", async () => {
|
||||
inject(/shim:contains/); // the LISTING write fails
|
||||
setCurrentUser("alice");
|
||||
await expect(storeRegistry.createEntityDoc("protected")).rejects.toThrow(/not listed in its store/i);
|
||||
});
|
||||
|
||||
// Both writes are attempted before the failure is raised — the cap must land even when
|
||||
// the listing did not, because either is worth having without the other. Throwing on the
|
||||
// first one (2026-08-07) skipped the second and orphaned the document entirely.
|
||||
test("a failed listing does not cost the document its key", async () => {
|
||||
const { quads } = inject(/shim:contains/);
|
||||
setCurrentUser("alice");
|
||||
await storeRegistry.createEntityDoc("protected").catch(() => {});
|
||||
expect(quads.some((q) => q.p === `${SHIM}:readCap`)).toBe(true);
|
||||
});
|
||||
|
||||
test("a failed key write is reported too, and names that half", async () => {
|
||||
inject(/shim:readCap/);
|
||||
setCurrentUser("alice");
|
||||
await expect(storeRegistry.createEntityDoc("protected")).rejects.toThrow(/key is not recorded/i);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user