From 9191a306468b6752fcea6298178d9a1ebabd850c Mon Sep 17 00:00:00 2001 From: Kyle Zemek Date: Tue, 10 Nov 2026 22:47:33 -0800 Subject: [PATCH] feat(plugins): report conclusively retired update providers This lets a plugin distinguish a conclusively retired update provider from an unknown submission without bypassing native review. The optional getOwnerStatus(ownerId) answers "retired" only after the host saw that exact provider lifetime disposed with no queued, reviewing or confirmed operation left; every other case stays "unknown". Co-Authored-By: Claude Opus 5.5 Signed-off-by: Kyle Zemek --- docs/plugin-architecture.md | 18 ++++ src/plugins/installer.test.ts | 52 ++++++++++++ src/plugins/installer.ts | 2 + src/plugins/updates.test.ts | 155 ++++++++++++++++++++++++++++++++++ src/plugins/updates.ts | 27 +++++- 5 files changed, 253 insertions(+), 1 deletion(-) 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 () => {