fix: un rejet tardif ne ferme plus un canal vivant, un registre illisible ne perd plus toutes les inbox

2026-08-17 12:17:59 +02:00
parent 6eaff0b985
commit 7b35300723
5 changed files with 364 additions and 62 deletions
@@ -482,20 +482,73 @@ export async function readInboxCapsFor(doc: Nuri): Promise<Nuri | undefined> {
 */
 // @provenance myInboxes kind=aligned level=1 ref=engine/repo/src/types.rs:AddInboxCapV0 — the User branch answers 'which inboxes may I read'. The document-inbox half of this list is declared-not-wired — see `readInboxCapPairs`
 export async function myInboxes(): Promise<Nuri[]> {
+  const { inboxes, incomplete } = await enumerateMyInboxes();
+  // The whole list or the failure that stopped it — the contract this function has always
+  // had, and the one `connect.connectedUser` is built on: not knowing which queues exist is
+  // the session failing to establish, and a short list would silently leave a delivered
+  // share un-drained. The original error is re-thrown, not wrapped: its caller reads it.
+  if (incomplete !== null) throw incomplete.error;
+  return inboxes;
+}
+
+/**
+ * What could be listed of {@link myInboxes}, and whether that is ALL of it.
+ *
+ * `incomplete` is `null` when the list is whole, and otherwise carries what stopped the rest
+ * from being listed. Not an error CODE and not a flag on the array: the point is that a
+ * caller cannot read this answer without meeting the question "was there a failure", which
+ * is exactly what a short array on its own let everybody skip.
+ */
+export interface InboxEnumeration {
+  /** Every inbox this holder may read that COULD be listed — possibly not all of them. */
+  inboxes: Nuri[];
+  /** What stopped the rest from being listed, or `null` when nothing did. */
+  incomplete: { error: unknown } | null;
+}
+
+/**
+ * {@link myInboxes}, for the caller that can use a PARTIAL answer — the live observation.
+ *
+ * The list is built from two independent registers: the account record, which names the
+ * user's own two store inboxes, and the User branch, which names one per document it opened
+ * an inbox on. Built in one `try`, one unreachable register discarded BOTH halves — the two
+ * user inboxes were already in hand when the second read threw, and the throw dropped them
+ * on the floor. A broker hiccup spanning sign-in therefore left the identity connected with
+ * ZERO inboxes watched, not with the one register it could not reach missing: every deposit,
+ * including the ones addressed to the person by name, waited for the next connection.
+ *
+ * So each half answers for itself, and what came back is returned WITH the failure rather
+ * than instead of it. That is the line this package draws everywhere: reaching a register is
+ * infrastructure and may fail, but a failure must never come back looking like an absence —
+ * hence {@link InboxEnumeration.incomplete}, which the caller has to look at.
+ */
+export async function enumerateMyInboxes(): Promise<InboxEnumeration> {
  const holder = getCurrentUser();
-  if (holder === null) return [];
+  if (holder === null) return { inboxes: [], incomplete: null };
  const out: Nuri[] = [];
-  // BOTH of the user's inboxes — public and protected — since upstream a site carries
-  // one on each of those two store repos (`engine/verifier/src/site.rs:127-152`).
-  if ((await resolveAccount(holder)) !== null) {
-    for (const scope of ["public", "protected"] as const) out.push(await userInbox(holder, scope));
+  try {
+    // BOTH of the user's inboxes — public and protected — since upstream a site carries
+    // one on each of those two store repos (`engine/verifier/src/site.rs:127-152`).
+    if ((await resolveAccount(holder)) !== null) {
+      for (const scope of ["public", "protected"] as const) {
+        out.push(await userInbox(holder, scope));
+      }
+    }
+  } catch (error) {
+    // The account record is what the other half reads THROUGH (`readInboxCapPairs` resolves
+    // the same record to find the private store), so there is no second half to attempt.
+    return { inboxes: out, incomplete: { error } };
  }
-  for (const { inbox } of await readInboxCapPairs()) {
-    // The record entitles this holder to read it — see the note above.
-    fileOwnInbox(holder, inbox);
-    out.push(inbox);
+  try {
+    for (const { inbox } of await readInboxCapPairs()) {
+      // The record entitles this holder to read it — see the note above.
+      fileOwnInbox(holder, inbox);
+      out.push(inbox);
+    }
+  } catch (error) {
+    return { inboxes: out, incomplete: { error } };
  }
-  return out;
+  return { inboxes: out, incomplete: null };
 }

 /**
@@ -57,7 +57,7 @@

 import { getCaps, getCurrentUser } from "../shared-wallet/bootstrap";
 import { accessLogPrefix, logStage, shortNuri } from "../shared-wallet/access-log";
-import { myInboxes } from "./branch-registers";
+import { enumerateMyInboxes, type InboxEnumeration } from "./branch-registers";
 import { drainInboxSerially, drainsSettled } from "./inbox-drain";
 import { lookupAccount } from "../shared-wallet/account-registry";
 import { processInbox } from "../surface/inbox";
@@ -78,12 +78,6 @@ interface Observation {
  register: Unsubscribe | null;
  /** Unsubscribe from the held-caps change signal — the second "which inboxes" channel. */
  holdings: Unsubscribe | null;
-  /**
-   * The inboxes whose subscription has already been re-attempted once after failing to be
-   * OPENED. What bounds the repair below to one extra try per inbox, so a document the broker
-   * genuinely cannot serve costs two calls rather than a loop.
-   */
-  reattempted: Set<Nuri>;
  /** True while a re-enumeration is running, so its own effects do not restart it. */
  enumerating: boolean;
  /** A trigger that arrived mid-cycle: the running one repeats once rather than lose it. */
@@ -171,19 +165,35 @@ async function applyWhatArrived(obs: Observation, inbox: Nuri): Promise<void> {
 *
 * Never removes: the list only grows within a session (an `AddInboxCap` record is durable),
 * and dropping a subscription on a guess would silently stop applying an inbox.
+ *
+ * ── A list that came back SHORT still gets watched ────────────────────────
+ * `enumerateMyInboxes` and not `myInboxes`: the list is read from two independent registers
+ * and one of them being unreachable used to discard the other's answer too, so a broker
+ * hiccup spanning sign-in left the identity connected with ZERO inboxes watched rather than
+ * with one register's worth missing. Whatever came back is watched here and now — including,
+ * crucially, the user's own two inboxes, which are where a share addressed to a PERSON
+ * lands, and therefore the channel by which a read-only identity converges at all.
+ *
+ * The shortfall is reported, never passed over: an incomplete list is not a complete one,
+ * and the inboxes it is missing are watched on the next enumeration — which is what the
+ * register push and the holdings signal are for. Applying what lands in a user inbox is
+ * itself one of those signals (it files a cap), so the ordinary case repairs itself.
 */
 async function watchTheInboxes(obs: Observation): Promise<void> {
  if (!current(obs)) return;
-  let inboxes: Nuri[];
+  let listed: InboxEnumeration;
  try {
-    inboxes = await myInboxes();
+    listed = await enumerateMyInboxes();
  } catch (error) {
-    // Not knowing WHICH inboxes exist is the whole observation failing, not one queue.
+    // Not knowing WHICH inboxes exist AT ALL is the whole observation failing, not one queue.
    reportUnobserved("the inboxes to watch could not be listed", error);
    return;
  }
  if (!current(obs)) return;
-  for (const inbox of inboxes) {
+  if (listed.incomplete !== null) {
+    reportUnobserved("the inboxes to watch could not all be listed", listed.incomplete.error);
+  }
+  for (const inbox of listed.inboxes) {
    if (obs.inboxes.has(inbox)) continue;
    try {
      // The push is the signal; the drain is the work. The FIRST push is the initial
@@ -212,20 +222,28 @@ async function watchTheInboxes(obs: Observation): Promise<void> {
 }

 /**
- * The subscription on `inbox` could not be OPENED — report it, forget it, and try once more.
+ * The subscription on `inbox` could not be OPENED — report it, and forget it.
 *
 * **Forget it**, because the entry this observation holds is the whole record of "already
 * watched": leaving a dead one in place is how one rejection at connection turned into a
 * session-long silence. Removed, the next enumeration subscribes again as if it had never
 * been attempted.
 *
- * **Try once more**, because the usual cause is a repo the verifier has not synced yet, and
- * the whole cost of finding out is one call. Exactly one extra attempt per inbox, tracked in
- * {@link Observation.reattempted}: a broker that genuinely cannot serve this document must
- * not be asked again and again by a loop that has no reason to stop. If the second attempt
- * fails too, the failure has been reported twice and the inbox waits for the next
- * enumeration signal — or for the next connection, where an unapplied deposit has always
- * waited.
+ * ── And that is ALL it does: recovery waits for the next SIGNAL ───────────
+ * Until 2026-08-17 this also re-enumerated on the spot, once per inbox. That call runs
+ * synchronously out of the very rejection it is reacting to, so it asks the broker that has
+ * just refused, in the same turn, with nothing having changed — and spends the one extra
+ * attempt it was allowed doing it. Deleting it left the whole suite green, which is the
+ * measure of what it achieved.
+ *
+ * What actually repairs this is an event, and this module already listens to the two that
+ * exist: the register push (`watchTheRegister`) and the held-caps signal
+ * (`CapRegistry.onChange`), each of which re-enters {@link enumerate} and finds this inbox
+ * unwatched. Applying anything at all fires the second one, so a session that is doing
+ * something catches up on its own. A session that is doing nothing waits for its next
+ * connection — where an unapplied deposit has always waited, since an inbox is not consumed
+ * by being unread. Polling for it is ruled out here for the same reason it is in
+ * `inbox-processor.ts`: this regime is push-driven, and a timer is what it replaced.
 */
 function watchFailed(obs: Observation, inbox: Nuri, error: unknown): void {
  if (!current(obs)) return;
@@ -239,9 +257,6 @@ function watchFailed(obs: Observation, inbox: Nuri, error: unknown): void {
    }
  }
  reportUnobserved("this inbox could not be watched: " + shortNuri(inbox), error);
-  if (obs.reattempted.has(inbox)) return;
-  obs.reattempted.add(inbox);
-  track(obs, enumerate(obs));
 }

 /**
@@ -337,7 +352,6 @@ export async function startObservingInboxes(): Promise<void> {
    inboxes: new Map(),
    register: null,
    holdings: null,
-    reattempted: new Set(),
    enumerating: false,
    enumerateAgain: false,
    pending: new Set(),
@@ -175,12 +175,26 @@ interface DocFanOut {
   * It therefore stays true once the setup has SUCCEEDED: the attempt still stands.
   *
   * Cleared on the two ways the attempt stops standing: the last listener leaving
-   * ({@link releaseFanOut}), and a setup that FAILED ({@link reportSetupFailure}). Left true
-   * on failure it stopped meaning "one is already running" and started meaning "this NURI is
-   * finished" — no later joiner ever attempted it again, for the whole session, over one
+   * ({@link releaseFanOut}), and the CURRENT setup FAILING ({@link reportSetupFailure}). Left
+   * true on failure it stopped meaning "one is already running" and started meaning "this NURI
+   * is finished" — no later joiner ever attempted it again, for the whole session, over one
   * transient rejection.
   */
  establishing: boolean;
+  /**
+   * WHICH establish this fan-out is waiting on — bumped by {@link beginEstablish} every time
+   * one is kicked off, and captured by that call for the whole of its life.
+   *
+   * Two establishes can be in flight over the SAME entry: {@link resubscribeDocs} re-opens the
+   * channel of a fan-out whose first attempt has not settled yet, and it re-opens it on the
+   * entry rather than on a fresh one, precisely so the listeners are kept. So `entry` identity
+   * — the only currency this module had — answers "is this fan-out still the one for this
+   * NURI", and cannot answer "is this attempt still the one this fan-out is waiting on". A
+   * counter can, and that second question is the one a LATE outcome has to ask: the first
+   * attempt rejecting long after the second SUCCEEDED is not this document failing, it is one
+   * superseded call finally answering.
+   */
+  attempt: number;
  /**
   * The most recent `State` push, replayed to a listener that joins later.
   * Its own `doc_subscribe` would have pushed one; the fan-out owes it the same.
@@ -216,6 +230,25 @@ function fanOut(nuri: Nuri, entry: DocFanOut, resp: DocChange, type: DocChangeTy
  }
 }

+/** Is `attempt` still the establish this fan-out is waiting on? */
+function isCurrentAttempt(nuri: Nuri, entry: DocFanOut, attempt: number): boolean {
+  return fanOuts.get(nuri) === entry && entry.attempt === attempt;
+}
+
+/**
+ * Kick off the one real subscription for `entry` — the ONLY way an establish is started.
+ *
+ * It stamps the attempt (see {@link DocFanOut.attempt}) as it starts it, so an outcome that
+ * arrives late can tell whether it is still the one being waited on. Being the only door is
+ * what makes that true: a second call site that forgot to bump the counter would hand its
+ * establish the number of the one it just superseded.
+ */
+function beginEstablish(nuri: Nuri, entry: DocFanOut, ng: { doc_subscribe?: unknown }): void {
+  entry.establishing = true;
+  entry.attempt += 1;
+  void establish(nuri, entry, ng, entry.attempt);
+}
+
 /**
 * The one real subscription could not be OPENED: forget the attempt, and say so.
 *
@@ -226,11 +259,24 @@ function fanOut(nuri: Nuri, entry: DocFanOut, resp: DocChange, type: DocChangeTy
 * **Visible** — the log always, plus the callers who asked to be told, so a caller whose job
 * depends on the subscription (the inbox observation) can report it and try again rather
 * than sit on a dead entry.
+ *
+ * ── Only for the attempt still being WAITED ON ─────────────────────────────
+ * Both halves act on the fan-out as it stands NOW, so a SUPERSEDED attempt must do neither.
+ * A rejection that arrives after {@link resubscribeDocs} has already re-opened the channel is
+ * not this document failing — the channel is open and pushing. Told anyway, the inbox
+ * observation dropped its entry and released it, which closed the WORKING subscription,
+ * printed "this inbox could not be watched" about an inbox that was, and left the identity
+ * with a silent queue. Clearing `establishing` was the same mistake one level down: it says
+ * "nothing is running" while the second attempt is, so the next joiner opens a third — and a
+ * second `doc_subscribe` on a branch EVICTS the one before it.
+ *
+ * Superseded, it is therefore LOGGED and nothing else: the call that replaced it owns the
+ * outcome, and reports its own failure if it has one.
 */
-function reportSetupFailure(nuri: Nuri, entry: DocFanOut, error: unknown): void {
+function reportSetupFailure(nuri: Nuri, entry: DocFanOut, error: unknown, attempt: number): void {
  console.error("[subscribe] doc_subscribe failed for", nuri, error);
+  if (!isCurrentAttempt(nuri, entry, attempt)) return;
  entry.establishing = false;
-  if (fanOuts.get(nuri) !== entry) return;
  for (const listener of [...entry.listeners]) {
    const tell = listener.onSetupFailed;
    if (tell === null || !entry.listeners.has(listener)) continue;
@@ -262,7 +308,12 @@ function releaseFanOut(nuri: Nuri, entry: DocFanOut): void {
 * Errors are isolated to this document (they never reject a shared batch — see
 * {@link subscribeDocs}); a failed setup simply leaves the document silent.
 */
-async function establish(nuri: Nuri, entry: DocFanOut, ng: { doc_subscribe?: unknown }): Promise<void> {
+async function establish(
+  nuri: Nuri,
+  entry: DocFanOut,
+  ng: { doc_subscribe?: unknown },
+  attempt: number,
+): Promise<void> {
  // No reactive primitive on the injected `ng` (the fake in the unit suite): there is
  // nothing to call, so this document simply never pushes — the same documented no-op
  // `openRepoUnguarded` takes for the same injection, and not a failure to report. A real
@@ -287,7 +338,7 @@ async function establish(nuri: Nuri, entry: DocFanOut, ng: { doc_subscribe?: unk
    }
    entry.realUnsub = typeof unsub === "function" ? unsub : null;
  } catch (error) {
-    reportSetupFailure(nuri, entry, error);
+    reportSetupFailure(nuri, entry, error, attempt);
  }
 }

@@ -345,8 +396,7 @@ export function resubscribeDocs(): void {
        console.error("[subscribe] unsubscribe failed for", nuri, error);
      }
    }
-    entry.establishing = true;
-    void establish(nuri, entry, ng);
+    beginEstablish(nuri, entry, ng);
  }
 }

@@ -422,7 +472,13 @@ export function subscribeDocUnguarded(

  let entry = fanOuts.get(nuri);
  if (!entry) {
-    entry = { listeners: new Set(), realUnsub: null, establishing: false, lastState: null };
+    entry = {
+      listeners: new Set(),
+      realUnsub: null,
+      establishing: false,
+      attempt: 0,
+      lastState: null,
+    };
    fanOuts.set(nuri, entry);
  }
  const joined = entry;
@@ -443,8 +499,7 @@ export function subscribeDocUnguarded(
  }

  if (!joined.establishing) {
-    joined.establishing = true;
-    void establish(nuri, joined, ng as { doc_subscribe?: unknown });
+    beginEstablish(nuri, joined, ng as { doc_subscribe?: unknown });
  }

  let stopped = false;