Comments: recheck the account after a startup failure; never sign from a stale one

apps/writer/src/backend/native.test.ts

@@ -45,6 +45,8 @@ async function fixture(beforeBackend?: (client: MdbaseClient) => void) {
   return {replica, client, description, metadata, current, caller, backend, manuscript};
 }
 
+const linked = {account: {id: "fixture-account", displayName: "Fixture", identities: []}, me: {status: "linked", person: {id: "p", path: "people/me.md", name: "Me", identities: [], typeNames: ["person"]}}, people: [], invalid: []} as never;
+
 // Public watch lifecycle stand-ins only; no private SDK fault hooks.
 function controlledWatch(initialState: ChangesWatch["state"] = "active", ready: Promise<void> = Promise.resolve()) {
   let state = initialState;
@@ -380,17 +382,37 @@ describe("native Writer domain backend", () => {
     h.replica.confirmAll(); h.replica.stallConfirmations(false);
     expect((await reply).ok).toBe(true);
   });
-  it("checks the account once more after a transient directory failure (issue 91)", async () => {
+  it("checks the account again after its first identity READ is cut off (issue 91)", async () => {
     const h = await fixture();
-    const account = {id: "fixture-account", displayName: "Fixture", identities: []};
-    const directory = vi.spyOn(MdbasePeopleClient.prototype, "directory")
-      .mockRejectedValueOnce(new MdbaseError({code: "unavailable", recovery: "retry", message: "people read session changed", reason: "people_fenced"}))
-      .mockResolvedValueOnce({account, me: {status: "linked", person: {id: "p", path: "people/me.md", name: "Me", identities: [], typeNames: ["person"]}}, people: [], invalid: []} as never);
+    // What the SDK reports when the host's collection control closes mid-READ
+    // (its /identity fetch is aborted), and when the one READ slot is busy.
+    const fenced = new MdbaseError({code: "unauthenticated", recovery: "reauthorize", message: "people scope changed", reason: "people_fenced"});
+    const busy = new MdbaseError({code: "unavailable", recovery: "retry", message: "people metadata request failed", reason: "people_busy"});
+    const directory = vi.spyOn(MdbasePeopleClient.prototype, "directory").mockRejectedValueOnce(fenced).mockRejectedValueOnce(busy).mockResolvedValueOnce(linked);
     expect(await h.backend.people()).toMatchObject({signing: {kind: "linked"}, me: {name: "Me"}});
-    expect(directory).toHaveBeenCalledTimes(2);
-    directory.mockRejectedValue(new MdbaseError({code: "unavailable", recovery: "retry", message: "people read session changed", reason: "people_fenced"}));
-    expect(await h.backend.people({fresh: true})).toMatchObject({signing: {kind: "unavailable"}});
-    expect(directory).toHaveBeenCalledTimes(4);
+    expect(directory).toHaveBeenCalledTimes(3);
+  });
+  it("takes a refusal as the answer without retrying", async () => {
+    const h = await fixture();
+    const directory = vi.spyOn(MdbasePeopleClient.prototype, "directory").mockRejectedValue(new MdbaseError({code: "forbidden", recovery: "reauthorize", message: "no identity", reason: "people_access"}));
+    expect((await h.backend.people()).signing?.kind).not.toBe("linked");
+    expect(directory).toHaveBeenCalledOnce();
+  });
+  it("never signs a new comment from a failed account check (issue 91)", async () => {
+    const h = await fixture();
+    vi.useFakeTimers({toFake: ["setTimeout"]});
+    try {
+      const directory = vi.spyOn(MdbasePeopleClient.prototype, "directory").mockRejectedValue(new MdbaseError({code: "unavailable", recovery: "retry", message: "people metadata request failed", reason: "people_busy"}));
+      const checked = h.backend.people();
+      await vi.advanceTimersByTimeAsync(5_000);
+      expect(await checked).toMatchObject({signing: {kind: "unavailable"}});
+      expect(directory).toHaveBeenCalledTimes(4);
+      directory.mockResolvedValue(linked);
+    } finally {vi.useRealTimers();}
+    const created = await h.backend.createComment({document: "manuscripts/original.md", text: "Signed"});
+    if (!created.ok) throw new Error(created.message);
+    expect(created.value.createdBy).toBe("[[people/me]]");
+    expect(await h.backend.people()).toMatchObject({signing: {kind: "linked"}});
   });
   it("still refuses a held or conflicted comment body on a display read", async () => {
     const h = await fixture();

apps/writer/src/backend/native.ts

@@ -24,7 +24,9 @@ interface NativeMetadata {
   readonly files: ReadonlyMap<string, FileView>;
   readonly generation: number;
 }
-const RECONNECT_MIN_MS = 500, RECONNECT_MAX_MS = 30_000, PEOPLE_RETRY_MS = 250;
+const RECONNECT_MIN_MS = 500, RECONNECT_MAX_MS = 30_000;
+/** Pauses before each further account check after a transient failure. */
+const PEOPLE_RETRY_MS = [250, 1_000, 3_000];
 function unprotected(record: RecordView): boolean {
   return record.state.state === "confirmed" && readable(record);
 }
@@ -60,6 +62,7 @@ export class NativeWriterBackend implements WriterBackend {
   private cached: NativeMetadata | undefined;
   private metadataJob: Promise<NativeMetadata> | undefined;
   private peopleJob: Promise<People> | undefined;
+  private peopleFailed = false;
   private writing = false;
   /** This session's write whose reply was lost, so a repeated click can't duplicate it.
    * Writes from earlier sessions are the SDK write ledger's to keep and settle. */
@@ -334,25 +337,37 @@ export class NativeWriterBackend implements WriterBackend {
     });
   }
   people(options: {fresh?: boolean} = {}): Promise<People> {
-    if (!this.peopleJob || options.fresh) this.peopleJob = this.loadPeople();
+    // A failed check is never reused: the next caller (the panel's reload, or a new
+    // comment about to be signed) checks again rather than writing it unsigned.
+    if (!this.peopleJob || options.fresh || this.peopleFailed) {
+      this.peopleFailed = false;
+      const job = this.loadPeople(); this.peopleJob = job;
+      void job.then(people => {if (this.peopleJob === job && people.signing?.kind === "unavailable") this.peopleFailed = true;});
+    }
     return this.peopleJob;
   }
   private async loadPeople(): Promise<People> {
-    try {
-      this.check();
-      const directory = await this.connection.people.directory({members: "omit", signal: this.signal}).catch(async (error: unknown) => {
-        // A read session replaced mid-check (a reconnect) or a briefly incomplete
-        // inventory is transient: look once more before saying the account is unchecked.
-        if (!isMdbaseError(error, "unavailable")) throw error;
-        await new Promise(resolve => setTimeout(resolve, PEOPLE_RETRY_MS)); this.check();
-        return this.connection.people.directory({members: "omit", signal: this.signal});
-      }); this.check();
-      return peopleFromDirectory(directory);
-    } catch (error) {
-      // Names only, never a substitute account/author or guessed identity.
-      try {const names = (await this.metadata()).store.people; this.check(); return {names, signing: {kind: "unavailable", reason: "Writer could not check the account's person record."}};}
-      catch {return {...NO_PEOPLE, signing: isMdbaseError(error) && error.code === "forbidden" ? {kind: "not-approved"} : {kind: "unavailable", reason: "Writer could not check the account's person record."}};}
+    let failure: unknown;
+    for (const delay of [0, ...PEOPLE_RETRY_MS]) {
+      try {
+        if (delay) await new Promise(resolve => setTimeout(resolve, delay));
+        this.check();
+        const directory = await this.connection.people.directory({members: "omit", signal: this.signal}); this.check();
+        return peopleFromDirectory(directory);
+      } catch (error) {
+        failure = error;
+        // These are transient at startup: the SDK host's identity READ ends with its
+        // collection control (aborted when that connection is replaced: `people_fenced`),
+        // its one READ slot can be busy with launch readiness (`people_busy`), or the read
+        // session can change. Look again, briefly, before saying the account is unchecked.
+        // A refusal or an unknown failure is the answer.
+        const transient = isMdbaseError(error) && ["unavailable", "unauthenticated", "cancelled", "rate_limited"].includes(error.code);
+        if (!transient || this.signal.aborted || this.disposed) break;
+      }
     }
+    // Names only, never a substitute account/author or guessed identity.
+    try {const names = (await this.metadata()).store.people; this.check(); return {names, signing: {kind: "unavailable", reason: "Writer could not check the account's person record."}};}
+    catch {return {...NO_PEOPLE, signing: isMdbaseError(failure) && failure.code === "forbidden" ? {kind: "not-approved"} : {kind: "unavailable", reason: "Writer could not check the account's person record."}};}
   }
   reviewIdentityAccess(): Promise<Result<void>> {
     return this.safely(async () => {if (!this.options.authorize) throw new Error("Writer cannot request identity access here."); await this.options.authorize(); this.check(); this.peopleJob = undefined;});

changelog.d/signing-check-retry.md

@@ -0,0 +1,3 @@
+## Fixed
+
+- A linked account is no longer shown as "Your comments are not signed … could not be checked" when Writer opens, and comments written before pressing **Check again** are no longer saved unsigned. Writer checks the account again after a brief startup failure, and it never reuses a failed check.

docs/architecture.md

@@ -44,9 +44,15 @@
   reports each write before it confirms, and the panel reloads at once); only a
   hold or an unresolved conflict refuses them. Writes and their readbacks still
   require a confirmed record. Account checks are applied in the order they were
-  started, so a slow, older check never replaces a newer signing state, and a
-  transient `unavailable` directory failure is checked once more before the
-  notice says the account could not be checked.
+  started, so a slow, older check never replaces a newer signing state. A
+  transient people-directory failure (`unavailable`, `unauthenticated`/`people_fenced`,
+  `cancelled`, `rate_limited`) is checked again after 250 ms, 1 s and 3 s before
+  the notice says the account could not be checked, and a failed check is never
+  reused: the next panel reload or new comment checks again, so a linked account
+  never writes an unsigned comment from a stale failure. The SDK host's identity
+  READ runs under its collection control's lifetime and shares one READ slot with
+  launch readiness, so a replaced connection or a busy slot fails Writer's first
+  check at startup.
 - **Images** are collection files, referenced relative to the record
   (`![Caption](figures/plot.png){#fig-plot}`) or by wikilink (`![[plot.png]]`).