Skip to content

Commit cb0b170

Browse files
committed
fix(workspace): fence stale plugin activation
1 parent cec7cec commit cb0b170

3 files changed

Lines changed: 112 additions & 4 deletions

File tree

apps/daemon/src/plugins/registry.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,34 @@ export function workspaceTeamPluginBindingAllowsRead(
285285
&& binding.resourceState !== 'deleted';
286286
}
287287

288+
/**
289+
* Snapshot the local binding generation before an asynchronous hub read.
290+
* `resourceState` is intentionally part of the fence in addition to
291+
* `updatedAt`: two synchronous SQLite writes can share the same millisecond,
292+
* but an intervening tombstone must still invalidate an older positive read.
293+
* `null` is the generation for a binding that does not exist yet.
294+
*/
295+
export function workspaceTeamPluginBindingActivationFence(
296+
db: SqliteDb,
297+
workspaceId: string,
298+
pluginId: string,
299+
): string | null {
300+
const binding = getWorkspaceResourceByResourceId(
301+
db,
302+
'plugin',
303+
workspaceTeamPluginBindingResourceId(workspaceId, pluginId),
304+
);
305+
if (!binding) return null;
306+
return JSON.stringify([
307+
binding.workspaceId,
308+
binding.visibility,
309+
binding.resourceState ?? null,
310+
binding.updatedAt,
311+
binding.updatedByWorkspaceMemberId ?? null,
312+
binding.resourceHubResourceId ?? null,
313+
]);
314+
}
315+
288316
const WORKSPACE_TEAM_PLUGIN_BINDING_PREFIX = 'team-mirror:';
289317

290318
/**
@@ -325,7 +353,9 @@ export async function resolveWorkspaceTeamPluginWithBindingGate<T>(input: {
325353

326354
export async function resolveAndActivateWorkspaceTeamPlugin<T>(input: {
327355
resolve: () => Promise<T | null>;
356+
captureActivationFence: () => string | null;
328357
stillShared: () => Promise<boolean>;
358+
activationFenceIsCurrent: (fence: string | null) => boolean;
329359
activate: () => boolean;
330360
}): Promise<T | null> {
331361
const resolved = await input.resolve();
@@ -335,10 +365,19 @@ export async function resolveAndActivateWorkspaceTeamPlugin<T>(input: {
335365
}
336366

337367
export async function activateWorkspaceTeamPluginIfStillShared(input: {
368+
captureActivationFence: () => string | null;
338369
stillShared: () => Promise<boolean>;
370+
activationFenceIsCurrent: (fence: string | null) => boolean;
339371
activate: () => boolean;
340372
}): Promise<boolean> {
373+
const activationFence = input.captureActivationFence();
341374
if (!await input.stillShared()) return false;
375+
// The hub read above is asynchronous. A newer reconciliation may retire the
376+
// binding while it is pending, so a positive result is authoritative only
377+
// for the binding generation captured before that read. Both this final
378+
// check and `activate` are synchronous, leaving no event-loop interleave in
379+
// which a tombstone can be overwritten.
380+
if (!input.activationFenceIsCurrent(activationFence)) return false;
342381
return input.activate();
343382
}
344383

apps/daemon/src/server.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,7 @@ import {
373373
resolveAndActivateWorkspaceTeamPlugin,
374374
resolvePluginFolder,
375375
resolveWorkspaceTeamPluginWithBindingGate,
376+
workspaceTeamPluginBindingActivationFence,
376377
workspaceTeamPluginBindingAllowsRead,
377378
workspaceTeamPluginBindingResourceId,
378379
} from './plugins/registry.js';
@@ -5007,11 +5008,13 @@ export async function startServer({
50075008
resource.id,
50085009
resource.id,
50095010
);
5011+
const bindingResourceId = workspaceTeamPluginBindingResourceId(
5012+
workspaceId,
5013+
resource.id,
5014+
);
5015+
const captureActivationFence = (): string | null =>
5016+
workspaceTeamPluginBindingActivationFence(db, workspaceId, resource.id);
50105017
const markTeamSynced = (): boolean => {
5011-
const bindingResourceId = workspaceTeamPluginBindingResourceId(
5012-
workspaceId,
5013-
resource.id,
5014-
);
50155018
const existingBinding = getWorkspaceResourceByResourceId(
50165019
db,
50175020
'plugin',
@@ -5045,7 +5048,9 @@ export async function startServer({
50455048
teamResourceVersions.get(workspaceId, 'plugin', resource.id) === resource.versionId
50465049
) {
50475050
await activateWorkspaceTeamPluginIfStillShared({
5051+
captureActivationFence,
50485052
stillShared: () => teamResourceStillShared('plugin', resource, scope),
5053+
activationFenceIsCurrent: (fence) => captureActivationFence() === fence,
50495054
activate: markTeamSynced,
50505055
});
50515056
return;
@@ -5057,7 +5062,9 @@ export async function startServer({
50575062
: '';
50585063
if (fs.existsSync(targetDir) && !resource.versionId && (!remoteDescription || localDescription === remoteDescription)) {
50595064
await activateWorkspaceTeamPluginIfStillShared({
5065+
captureActivationFence,
50605066
stillShared: () => teamResourceStillShared('plugin', resource, scope),
5067+
activationFenceIsCurrent: (fence) => captureActivationFence() === fence,
50615068
activate: markTeamSynced,
50625069
});
50635070
return;
@@ -5108,7 +5115,9 @@ export async function startServer({
51085115
}
51095116
return resolved.record;
51105117
},
5118+
captureActivationFence,
51115119
stillShared: () => teamResourceStillShared('plugin', resource, scope),
5120+
activationFenceIsCurrent: (fence) => captureActivationFence() === fence,
51125121
activate: markTeamSynced,
51135122
});
51145123
if (!activated) return;

apps/daemon/tests/plugins-workspace-scope.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import path from 'node:path';
2424
import {
2525
closeDatabase,
2626
ensureWorkspaceResource,
27+
getWorkspaceResourceByResourceId,
2728
openDatabase,
2829
updateWorkspaceResource,
2930
} from '../src/db.js';
@@ -33,6 +34,7 @@ import {
3334
resolveAndActivateWorkspaceTeamPlugin,
3435
resolveWorkspaceTeamPluginWithBindingGate,
3536
upsertInstalledPlugin,
37+
workspaceTeamPluginBindingActivationFence,
3638
workspaceTeamPluginBindingAllowsRead,
3739
workspaceTeamPluginBindingResourceId,
3840
} from '../src/plugins/registry.js';
@@ -236,7 +238,9 @@ describe('listInstalledPlugins workspace scope', () => {
236238
resolveStarted();
237239
return resolveGate;
238240
},
241+
captureActivationFence: () => 'active',
239242
stillShared: async () => false,
243+
activationFenceIsCurrent: () => false,
240244
activate: () => {
241245
updateWorkspaceResource(db, 'plugin', workspaceId, bindingId, {
242246
resourceState: 'active',
@@ -276,10 +280,14 @@ describe('listInstalledPlugins workspace scope', () => {
276280
readStarted = resolve;
277281
});
278282
const oldListing = activateWorkspaceTeamPluginIfStillShared({
283+
captureActivationFence: () =>
284+
workspaceTeamPluginBindingActivationFence(db, workspaceId, pluginId),
279285
stillShared: async () => {
280286
readStarted();
281287
return authoritativeRead;
282288
},
289+
activationFenceIsCurrent: (fence) =>
290+
workspaceTeamPluginBindingActivationFence(db, workspaceId, pluginId) === fence,
283291
activate: () => {
284292
updateWorkspaceResource(db, 'plugin', workspaceId, bindingId, {
285293
resourceState: 'active',
@@ -299,4 +307,56 @@ describe('listInstalledPlugins workspace scope', () => {
299307
).toBe(false);
300308
},
301309
);
310+
311+
it('rejects a superseded positive shared read after a newer binding tombstone', async () => {
312+
const db = openDatabase(tempDir, { dataDir: tempDir });
313+
const pluginId = 'plugin-superseded-positive';
314+
const workspaceId = 'ws-team';
315+
const bindingId = workspaceTeamPluginBindingResourceId(workspaceId, pluginId);
316+
ensureWorkspaceResource(db, 'plugin', workspaceId, bindingId, {
317+
visibility: 'team',
318+
resourceState: 'active',
319+
});
320+
321+
let finishAuthoritativeRead!: (stillShared: boolean) => void;
322+
const authoritativeRead = new Promise<boolean>((resolve) => {
323+
finishAuthoritativeRead = resolve;
324+
});
325+
let readStarted!: () => void;
326+
const started = new Promise<void>((resolve) => {
327+
readStarted = resolve;
328+
});
329+
const staleActivation = activateWorkspaceTeamPluginIfStillShared({
330+
captureActivationFence: () =>
331+
workspaceTeamPluginBindingActivationFence(db, workspaceId, pluginId),
332+
stillShared: async () => {
333+
readStarted();
334+
return authoritativeRead;
335+
},
336+
activationFenceIsCurrent: (fence) =>
337+
workspaceTeamPluginBindingActivationFence(db, workspaceId, pluginId) === fence,
338+
activate: () => {
339+
updateWorkspaceResource(db, 'plugin', workspaceId, bindingId, {
340+
resourceState: 'active',
341+
});
342+
return true;
343+
},
344+
});
345+
await started;
346+
const originalUpdatedAt = Number(
347+
getWorkspaceResourceByResourceId(db, 'plugin', bindingId)?.updatedAt,
348+
);
349+
updateWorkspaceResource(db, 'plugin', workspaceId, bindingId, {
350+
resourceState: 'deleted',
351+
// Simulate two writes in the same millisecond: resourceState must keep
352+
// the tombstone visible even when updatedAt alone cannot distinguish it.
353+
updatedAt: originalUpdatedAt,
354+
});
355+
finishAuthoritativeRead(true);
356+
357+
await expect(staleActivation).resolves.toBe(false);
358+
expect(
359+
workspaceTeamPluginBindingAllowsRead(db, workspaceId, pluginId),
360+
).toBe(false);
361+
});
302362
});

0 commit comments

Comments
 (0)