diff --git a/docs/plugin-architecture.md b/docs/plugin-architecture.md index 288e8113fd..a61c216de5 100644 --- a/docs/plugin-architecture.md +++ b/docs/plugin-architecture.md @@ -462,6 +462,7 @@ type PluginUpdateCapability = { getQueue(id: string): PluginUpdateQueueSnapshot | undefined; subscribeQueue(id: string, listener: () => void): () => void; cancelQueue(id: string): boolean; + getOwnerStatus?(ownerId: PluginUpdateOwnerId): "retired" | "unknown"; // missing on older hosts }; ``` @@ -546,6 +547,23 @@ this contract does not provide cross-restart idempotency. An absent capability or an `unavailable` snapshot is not evidence that a prior ambiguous submission was rejected. A refresh does not resolve an earlier ambiguous submission. +`getOwnerStatus(ownerId)` tells a caller whether a provider lifetime it captured +earlier is over. It reports `retired` only after this host process saw that exact +provider disposed with no queued or reviewing item and no confirmed operation +left, so that lifetime can no longer open a review or install anything. A +confirmed update keeps its provider unretired until the operation finishes and +releases the shared review. Everything else is `unknown`: any provider not yet +disposed, whether current, replaced or unregistered; an ID this host never +registered; a lifetime older than the 16 most recently registered lifetimes; and +any ID after a host restart. It is a point-in-time read: retirement does not +notify subscribers. Feature-check the method; an older host without it gives no +retirement evidence. `retired` does not say +what happened to an earlier request: a `getQueueRequest()` result of `unknown` +stays unknown, and retirement is not evidence of rejection, cancellation, +installation or activation. It only means no review or operation from that +lifetime remains, so any new submission goes through the current provider and +the normal explicit review. + `requestInstall` never installs silently. It opens Settings → Plugins with the Git source prefilled and read-only, and the user still chooses **Find**, reviews the existing access review, and confirms. The promise resolves with what the user did: diff --git a/src/plugins/installer.test.ts b/src/plugins/installer.test.ts index fb87d79bb9..8121523677 100644 --- a/src/plugins/installer.test.ts +++ b/src/plugins/installer.test.ts @@ -421,6 +421,58 @@ it("isolates request identities by plugin and replays after that caller remounts } }); +it("reports an old provider retired to plugins only after its confirmed update releases the shared review", async () => { + const f = await setupUpdates(); + const replacement = createPluginUpdates(f.manager as never, async () => null); + let stopReplacement = () => {}; + try { + const market = (await f.as("example.market")).updates; + const other = (await f.as("example.other")).updates; + const ownerId = market?.ownerId; + if (!market || !other || !ownerId) throw new Error("Expected updates"); + const queue = market.queueUpdates([f.token]); + const review = f.updates.startNextReview(f.requests.tryHoldReview); + if (!queue || !review) throw new Error("Expected an open update review"); + expect(f.updates.beginOperation(queue.id, review.token)).toBeDefined(); + + stopReplacement = f.requests.registerUpdates(replacement); + f.unregister(); + f.updates.dispose(); + expect(market.ownerId).toBe(replacement.capability.ownerId); + expect(market.getOwnerStatus?.(ownerId)).toBe("unknown"); + expect(f.requests.tryHoldReview()).toBeUndefined(); + await expect( + f.core.submit(base, { id: "example.market", lifetime: () => () => {} }), + ).resolves.toBe("unavailable"); + await replacement.check(); + const next = replacement.capability.getSnapshot().candidates[0]?.token; + if (!next) throw new Error("Expected a replacement candidate"); + expect(market.queueUpdates([next])).toBeDefined(); + expect( + replacement.startNextReview(f.requests.tryHoldReview), + ).toBeUndefined(); + + expect(f.updates.finishReview(queue.id, review.token, "updated")).toBe( + true, + ); + expect(market.getOwnerStatus?.(ownerId)).toBe("retired"); + expect(other.getOwnerStatus?.(ownerId)).toBe("retired"); + await f.fibers[0]?.dispose(); + const remounted = (await f.as("example.market")).updates; + expect(remounted?.getOwnerStatus?.(ownerId)).toBe("retired"); + const replacementId = replacement.capability.ownerId; + if (!replacementId) throw new Error("Expected a replacement identity"); + expect(remounted?.getOwnerStatus?.(replacementId)).toBe("unknown"); + expect(replacement.startNextReview(f.requests.tryHoldReview)).toBeDefined(); + } finally { + stopReplacement(); + replacement.dispose(); + f.unregister(); + f.updates.dispose(); + await Promise.all(f.fibers.map((fiber) => fiber.dispose())); + } +}); + it("bounds sequential plugin refreshes without throttling host-forced checks", async () => { vi.useFakeTimers(); const f = await setupUpdates(); diff --git a/src/plugins/installer.ts b/src/plugins/installer.ts index 763c32182b..dc0bcee958 100644 --- a/src/plugins/installer.ts +++ b/src/plugins/installer.ts @@ -476,6 +476,8 @@ export function createPluginInstallerService( getQueueRequest: updates.getQueueRequest, getQueue: updates.getQueue, cancelQueue: updates.cancelQueue, + getOwnerStatus: (ownerId) => + updates.getOwnerStatus?.(ownerId) ?? "unknown", subscribe(listener) { const stop = caller.effect(() => updates.subscribe(listener)); return () => void stop(); diff --git a/src/plugins/updates.test.ts b/src/plugins/updates.test.ts index f3d8fa6a02..ae5812c804 100644 --- a/src/plugins/updates.test.ts +++ b/src/plugins/updates.test.ts @@ -933,6 +933,161 @@ it("returns unknown when a queue request's provider is replaced", async () => { replacement.dispose(); }); +it("reports a replaced provider retired only after disposal ends its queued and unopened reviews", async () => { + const f = fixture(); + const bridge = createPluginUpdateCapabilityBridge(); + const original = createPluginUpdates(f.manager, f.authorize); + bridge.register(original); + const token = (await bridge.capability.refresh()).candidates[0]?.token; + const ownerId = bridge.capability.ownerId; + if (!token || !ownerId) throw new Error("Expected a retained provider"); + const reviewing = bridge.capability.queueUpdates([token]); + const queued = bridge.capability.queueUpdates([token]); + const review = startReview(original); + if (!reviewing || !queued || !review) throw new Error("Expected a review"); + + const replacement = createPluginUpdates(f.manager, f.authorize); + const unregisterReplacement = bridge.register(replacement); + const nextOwnerId = bridge.capability.ownerId; + if (!nextOwnerId) throw new Error("Expected a replacement provider"); + expect(original.capability.getQueue(reviewing.id)?.status).toBe("reviewing"); + expect(original.capability.getQueue(queued.id)?.status).toBe("queued"); + for (const id of [ownerId, nextOwnerId, "unseen" as typeof ownerId]) + expect(bridge.capability.getOwnerStatus?.(id)).toBe("unknown"); + + original.dispose(); + expect(original.capability.getQueue(reviewing.id)?.status).toBe("cancelled"); + expect(original.capability.getQueue(queued.id)?.status).toBe("cancelled"); + for (let query = 0; query < 3; query++) + expect(bridge.capability.getOwnerStatus?.(ownerId)).toBe("retired"); + expect(bridge.forCaller("example.market").getOwnerStatus?.(ownerId)).toBe( + "retired", + ); + expect(original.beginOperation(review.queueId, review.token)).toBeUndefined(); + expect(startReview(original)).toBeUndefined(); + expect(original.capability.queueUpdates([token])).toBeUndefined(); + expect(bridge.capability.getOwnerStatus?.(nextOwnerId)).toBe("unknown"); + expect( + bridge.capability.getQueueRequest({ + ownerId, + requestId: "historical-request", + tokens: [token], + }), + ).toEqual({ status: "unknown" }); + + unregisterReplacement(); + replacement.dispose(); +}); + +it.each([ + ["finished", "installed"], + ["ended", "failed"], + ["cancelled", "cancelled"], + ["reconfirmed", "failed"], +] as const)( + "keeps a confirmed operation's provider unknown past disposal until it is %s", + async (completion, itemStatus) => { + const f = fixture(); + const bridge = createPluginUpdateCapabilityBridge(); + const original = createPluginUpdates(f.manager, f.authorize); + bridge.register(original); + const token = (await bridge.capability.refresh()).candidates[0]?.token; + const ownerId = bridge.capability.ownerId; + if (!token || !ownerId) throw new Error("Expected a retained provider"); + const queue = bridge.capability.queueUpdates([token]); + let held = false; + const statusAtRelease = vi.fn(() => + bridge.capability.getOwnerStatus?.(ownerId), + ); + const hold = () => { + held = true; + return () => { + statusAtRelease(); + held = false; + }; + }; + const review = original.startNextReview(hold); + if (!queue || !review) throw new Error("Expected a review"); + const unconfirm = original.beginOperation(queue.id, review.token); + expect(unconfirm).toEqual(expect.any(Function)); + + const replacement = createPluginUpdates(f.manager, f.authorize); + const unregisterReplacement = bridge.register(replacement); + original.dispose(); + if (completion === "finished" || completion === "ended") + expect(original.capability.cancelQueue(queue.id)).toBe(true); + expect(original.getActiveReview()).toBeDefined(); + expect(held).toBe(true); + expect(bridge.capability.getOwnerStatus?.(ownerId)).toBe("unknown"); + if (completion === "finished") { + expect(original.finishReview(queue.id, review.token, "updated")).toBe( + true, + ); + } else { + unconfirm?.(); + expect(original.getActiveReview()).toBeDefined(); + expect(bridge.capability.getOwnerStatus?.(ownerId)).toBe("unknown"); + if (completion === "ended") + expect( + original.endReview(queue.id, review.token, "install failed"), + ).toBe(true); + else if (completion === "cancelled") + expect(original.capability.cancelQueue(queue.id)).toBe(true); + else + expect(original.beginOperation(queue.id, review.token)).toBeUndefined(); + } + + expect(statusAtRelease).toHaveReturnedWith("unknown"); + expect(held).toBe(false); + expect(original.getActiveReview()).toBeUndefined(); + expect(bridge.capability.getOwnerStatus?.(ownerId)).toBe("retired"); + expect(original.capability.getQueue(queue.id)?.items[0]?.status).toBe( + itemStatus, + ); + expect(original.capability.queueUpdates([token])).toBeUndefined(); + expect(original.startNextReview(hold)).toBeUndefined(); + expect(held).toBe(false); + unregisterReplacement(); + replacement.dispose(); + }, +); + +it("keeps live, unregistered, evicted and restarted provider lifetimes unknown", async () => { + const f = fixture(); + const bridge = createPluginUpdateCapabilityBridge(); + const original = createPluginUpdates(f.manager, f.authorize); + const ownerId = original.capability.ownerId; + if (!ownerId) throw new Error("Expected a provider identity"); + expect(bridge.capability.getOwnerStatus?.(ownerId)).toBe("unknown"); + const unregister = bridge.register(original); + expect(bridge.capability.getOwnerStatus?.(ownerId)).toBe("unknown"); + unregister(); + expect(bridge.capability.getOwnerStatus?.(ownerId)).toBe("unknown"); + original.dispose(); + expect(bridge.capability.getOwnerStatus?.(ownerId)).toBe("retired"); + expect( + createPluginUpdateCapabilityBridge().capability.getOwnerStatus?.(ownerId), + ).toBe("unknown"); + + const later: ReturnType[] = []; + for (let index = 0; index < 15; index++) { + const next = createPluginUpdates(f.manager, f.authorize); + bridge.register(next)(); + next.dispose(); + later.push(next); + } + expect(bridge.capability.getOwnerStatus?.(ownerId)).toBe("retired"); + const newest = createPluginUpdates(f.manager, f.authorize); + const unregisterNewest = bridge.register(newest); + expect(bridge.capability.getOwnerStatus?.(ownerId)).toBe("unknown"); + const laterIds = later.map((next) => next.capability.ownerId); + expect(laterIds).not.toContain(undefined); + for (const id of laterIds) + if (id) expect(bridge.capability.getOwnerStatus?.(id)).toBe("retired"); + unregisterNewest(); + newest.dispose(); +}); + it("does not replay an ambiguous update request across checker lifetimes", async () => { const f = fixture(); const first = createPluginUpdates(f.manager, f.authorize); diff --git a/src/plugins/updates.ts b/src/plugins/updates.ts index 3188b2a0a3..7409514a94 100644 --- a/src/plugins/updates.ts +++ b/src/plugins/updates.ts @@ -156,6 +156,12 @@ export type PluginUpdateCapability = Readonly<{ subscribeQueue(id: string, listener: () => void): () => void; /** Stops queued/unopened reviews; an already-confirmed operation is not undone. */ cancelQueue(id: string): boolean; + /** + * `retired` only when this host process saw that exact provider lifetime end + * with no queued, reviewing, or confirmed operation left; otherwise `unknown`. + * Missing on older hosts. Says nothing about the outcome of an earlier request. + */ + getOwnerStatus?(ownerId: PluginUpdateOwnerId): "retired" | "unknown"; }>; function freezeSnapshot(snapshot: PluginUpdateSnapshot): PluginUpdateSnapshot { @@ -244,6 +250,7 @@ export const updateSourceKey = (source: GitUpdateSource) => const MAX_RETAINED_UPDATE_QUEUES = 50; const MAX_RETAINED_UPDATE_REQUESTS = 1024; +const MAX_RETAINED_UPDATE_OWNERS = 16; function uniqueQueueTokens(tokens: readonly PluginUpdateToken[]) { return [...new Set(tokens)]; @@ -260,6 +267,7 @@ export function createPluginUpdates( ) => Promise<{ repository: string; token: string } | null>, ) { const ownerId = crypto.randomUUID() as PluginUpdateOwnerId; + const lifetime = { ownerId, retired: false }; let updates: readonly PluginUpdate[] = []; let closed = false; let pending: Promise | undefined; @@ -286,6 +294,8 @@ export function createPluginUpdates( const release = releaseActiveReview; releaseActiveReview = undefined; release?.(); + // A confirmed operation keeps its review, and so this lifetime, open past dispose. + if (closed && !activeReview) lifetime.retired = true; }; const key = (update: PluginUpdate) => `${update.id}:${updateSourceKey(update.source)}:${update.candidate?.revision ?? update.error}`; @@ -949,6 +959,7 @@ export function createPluginUpdates( return { snapshot: () => updates, capability, + lifetime: lifetime as Readonly, forCaller(callerId: string): PluginUpdateCapability { return { ...capability, @@ -1019,8 +1030,16 @@ export function createPluginUpdateCapabilityBridge() { candidates: Object.freeze([]), errors: Object.freeze([]), }); - let provider: Pick | undefined; + let provider: + | Pick + | undefined; let unsubscribeProvider: (() => void) | undefined; + // Keep lifetime records, not providers, so replaced providers can be collected. + const lifetimes = new Map(); + const getOwnerStatus = (ownerId: PluginUpdateOwnerId) => + lifetimes.get(ownerId)?.retired + ? ("retired" as const) + : ("unknown" as const); const listeners = new Set<() => void>(); const notify = () => { notifySafely(listeners); @@ -1054,6 +1073,7 @@ export function createPluginUpdateCapabilityBridge() { }); }, cancelQueue: (id) => resolve()?.cancelQueue(id) ?? false, + getOwnerStatus, }; return capability; }; @@ -1066,6 +1086,11 @@ export function createPluginUpdateCapabilityBridge() { register(next: NonNullable) { unsubscribeProvider?.(); provider = next; + lifetimes.delete(next.lifetime.ownerId); + lifetimes.set(next.lifetime.ownerId, next.lifetime); + // An evicted lifetime becomes unknown, never retired. + for (const id of lifetimes.keys()) + if (lifetimes.size > MAX_RETAINED_UPDATE_OWNERS) lifetimes.delete(id); unsubscribeProvider = next.capability.subscribe(notify); notify(); return () => {