diff --git a/apps/writer/src/backend/native.test.ts b/apps/writer/src/backend/native.test.ts index 58d14ac..98b6303 100644 --- a/apps/writer/src/backend/native.test.ts +++ b/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 = 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(); diff --git a/apps/writer/src/backend/native.ts b/apps/writer/src/backend/native.ts index 8697321..d1a3f30 100644 --- a/apps/writer/src/backend/native.ts +++ b/apps/writer/src/backend/native.ts @@ -24,7 +24,9 @@ interface NativeMetadata { readonly files: ReadonlyMap; 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 | undefined; private peopleJob: Promise | 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 { - 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 { - 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> { 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;}); diff --git a/changelog.d/signing-check-retry.md b/changelog.d/signing-check-retry.md new file mode 100644 index 0000000..3d4cde0 --- /dev/null +++ b/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. diff --git a/docs/architecture.md b/docs/architecture.md index 222299f..b9090f3 100644 --- a/docs/architecture.md +++ b/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]]`).