Skip to content

Commit 398163f

Browse files
lefarcenlefarcen
committed
fix(workspace): close Team plugin retraction races
1 parent 4379f86 commit 398163f

5 files changed

Lines changed: 249 additions & 18 deletions

File tree

‎apps/daemon/src/plugins/registry.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,18 @@ export async function resolveWorkspaceTeamPluginWithBindingGate<T>(input: {
323323
return resolved;
324324
}
325325

326+
export async function resolveAndActivateWorkspaceTeamPlugin<T>(input: {
327+
resolve: () => Promise<T | null>;
328+
stillShared: () => Promise<boolean>;
329+
activate: () => boolean;
330+
}): Promise<T | null> {
331+
const resolved = await input.resolve();
332+
if (resolved == null) return null;
333+
if (!await input.stillShared()) return null;
334+
if (!input.activate()) return null;
335+
return resolved;
336+
}
337+
326338
/**
327339
* `workspaceId` is optional and defaults to the pre-workspace-isolation
328340
* behavior (every live installed plugin, otherwise unfiltered) so every existing caller —

‎apps/daemon/src/routes/plugins/index.ts‎

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,11 @@ export interface RegisterPluginRoutesDeps {
178178
resourceType: string,
179179
resourceId: string,
180180
) => WorkspaceResourceBindingRow | null | undefined;
181+
workspaceTeamPluginBindingAllowsRead?: (
182+
db: SqliteDbLike,
183+
workspaceId: string,
184+
pluginId: string,
185+
) => boolean;
181186
};
182187
plugins: {
183188
listInstalledPlugins: (
@@ -338,7 +343,57 @@ export function registerPluginRoutes(app: Express, deps: RegisterPluginRoutesDep
338343
)) return;
339344
return helpers.installOrUpgradePlugin(req, res, 'upgrade', authority);
340345
});
341-
app.post('/api/plugins/:id/apply', async (req, res) => { try { const authority = await resolveWorkspaceAuthority(req, res); if (authority === undefined) return; const plugin = await resolveRequestPlugin(req.params.id, authority); if (!plugin) return res.status(404).json({ error: 'plugin not found' }); const body = req.body && typeof req.body === 'object' ? req.body as Record<string, unknown> : {}; const inputs = body.inputs && typeof body.inputs === 'object' ? body.inputs : {}; const grantCaps = Array.isArray(body.grantCaps) ? body.grantCaps.filter((c: unknown): c is string => typeof c === 'string') : []; const locale = typeof body.locale === 'string' ? body.locale : undefined; const registry = await helpers.loadPluginRegistryView(); const connectorProbe = helpers.buildConnectorProbe(helpers.connectorService); const computed = plugins.applyPlugin({ plugin, inputs, registry, locale, connectorProbe }); if (grantCaps.length > 0) { const merged = new Set([...computed.result.capabilitiesGranted, ...grantCaps]); computed.result.capabilitiesGranted = Array.from(merged); computed.result.appliedPlugin.capabilitiesGranted = Array.from(merged); } res.json({ ok: true, ...computed.result, warnings: computed.warnings, manifestSourceDigest: computed.manifestSourceDigest }); } catch (err: unknown) { if (err instanceof plugins.MissingInputError) return res.status(422).json({ error: 'missing_inputs', fields: err.fields }); res.status(500).json({ error: String(err) }); } });
346+
app.post('/api/plugins/:id/apply', async (req, res) => {
347+
try {
348+
const authority = await resolveWorkspaceAuthority(req, res);
349+
if (authority === undefined) return;
350+
const plugin = await resolveRequestPlugin(req.params.id, authority);
351+
if (!plugin) return res.status(404).json({ error: 'plugin not found' });
352+
const body = req.body && typeof req.body === 'object'
353+
? req.body as Record<string, unknown>
354+
: {};
355+
const inputs = body.inputs && typeof body.inputs === 'object' ? body.inputs : {};
356+
const grantCaps = Array.isArray(body.grantCaps)
357+
? body.grantCaps.filter((c: unknown): c is string => typeof c === 'string')
358+
: [];
359+
const locale = typeof body.locale === 'string' ? body.locale : undefined;
360+
const registry = await helpers.loadPluginRegistryView();
361+
const exactWorkspaceId = authority?.workspaceId?.trim();
362+
if (
363+
typeof plugin.source === 'string' &&
364+
plugin.source.startsWith('team:plugin:') &&
365+
(
366+
!exactWorkspaceId ||
367+
!workspaceResources?.workspaceTeamPluginBindingAllowsRead ||
368+
!workspaceResources.workspaceTeamPluginBindingAllowsRead(
369+
db,
370+
exactWorkspaceId,
371+
req.params.id,
372+
)
373+
)
374+
) {
375+
return res.status(404).json({ error: 'plugin not found' });
376+
}
377+
const connectorProbe = helpers.buildConnectorProbe(helpers.connectorService);
378+
const computed = plugins.applyPlugin({ plugin, inputs, registry, locale, connectorProbe });
379+
if (grantCaps.length > 0) {
380+
const merged = new Set([...computed.result.capabilitiesGranted, ...grantCaps]);
381+
computed.result.capabilitiesGranted = Array.from(merged);
382+
computed.result.appliedPlugin.capabilitiesGranted = Array.from(merged);
383+
}
384+
res.json({
385+
ok: true,
386+
...computed.result,
387+
warnings: computed.warnings,
388+
manifestSourceDigest: computed.manifestSourceDigest,
389+
});
390+
} catch (err: unknown) {
391+
if (err instanceof plugins.MissingInputError) {
392+
return res.status(422).json({ error: 'missing_inputs', fields: err.fields });
393+
}
394+
res.status(500).json({ error: String(err) });
395+
}
396+
});
342397
app.post('/api/plugins/:id/duplicate-project', helpers.requireLocalDaemonRequest, async (req, res) => {
343398
let cleanupProjectId: string | null = null;
344399
let insertedProject = false;

‎apps/daemon/src/server.ts‎

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,7 @@ import {
369369
} from './plugins/index.js';
370370
import {
371371
pluginIdFromWorkspaceTeamPluginBinding,
372+
resolveAndActivateWorkspaceTeamPlugin,
372373
resolvePluginFolder,
373374
resolveWorkspaceTeamPluginWithBindingGate,
374375
workspaceTeamPluginBindingAllowsRead,
@@ -5077,23 +5078,30 @@ export async function startServer({
50775078
verifyStillShared: () => teamResourceStillShared('plugin', resource, scope),
50785079
});
50795080
if (materialized.status !== 'committed') return;
5080-
const resolved = await resolvePluginFolder({
5081-
folder: materialized.targetDir,
5082-
folderId: resource.id,
5083-
sourceKind: 'user',
5084-
source: teamResourceSourceKey({
5085-
kind: 'plugin',
5086-
workspaceId,
5087-
resourceId: resource.id,
5088-
}),
5081+
const activated = await resolveAndActivateWorkspaceTeamPlugin({
5082+
resolve: async () => {
5083+
const resolved = await resolvePluginFolder({
5084+
folder: materialized.targetDir,
5085+
folderId: resource.id,
5086+
sourceKind: 'user',
5087+
source: teamResourceSourceKey({
5088+
kind: 'plugin',
5089+
workspaceId,
5090+
resourceId: resource.id,
5091+
}),
5092+
});
5093+
if (!resolved.ok) {
5094+
console.warn(
5095+
`[team-resources] failed to register shared plugin ${resource.id}: ${resolved.errors.join('; ')}`,
5096+
);
5097+
return null;
5098+
}
5099+
return resolved.record;
5100+
},
5101+
stillShared: () => teamResourceStillShared('plugin', resource, scope),
5102+
activate: markTeamSynced,
50895103
});
5090-
if (!resolved.ok) {
5091-
console.warn(
5092-
`[team-resources] failed to register shared plugin ${resource.id}: ${resolved.errors.join('; ')}`,
5093-
);
5094-
return;
5095-
}
5096-
markTeamSynced();
5104+
if (!activated) return;
50975105
if (resource.versionId) {
50985106
await teamResourceVersions.set(
50995107
workspaceId,
@@ -7141,7 +7149,11 @@ export async function startServer({
71417149
conversations: conversationDeps,
71427150
fetchProjectCreationWorkspaceDirectory,
71437151
verifyWorkspaceRequestAuthority,
7144-
workspaceResources: { getWorkspaceResource, getWorkspaceResourceByResourceId },
7152+
workspaceResources: {
7153+
getWorkspaceResource,
7154+
getWorkspaceResourceByResourceId,
7155+
workspaceTeamPluginBindingAllowsRead,
7156+
},
71457157
plugins: {
71467158
listInstalledPlugins: listWorkspacePlugins,
71477159
getInstalledPlugin,

‎apps/daemon/tests/plugins-apply-workspace-retraction.test.ts‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import express from 'express';
2+
import type { AddressInfo } from 'node:net';
3+
import { afterEach, describe, expect, it, vi } from 'vitest';
4+
5+
import { registerPluginRoutes } from '../src/routes/plugins/index.js';
6+
7+
const servers: Array<ReturnType<express.Express['listen']>> = [];
8+
9+
afterEach(async () => {
10+
await Promise.all(
11+
servers.splice(0).map((server) => new Promise<void>((resolve, reject) => {
12+
server.close((error) => error ? reject(error) : resolve());
13+
})),
14+
);
15+
});
16+
17+
describe('Team plugin apply retraction gate', () => {
18+
it('does not apply a Team plugin retired while registry loading is pending', async () => {
19+
const app = express();
20+
app.use(express.json());
21+
let bindingLive = true;
22+
let finishRegistryLoad!: (value: Record<string, never>) => void;
23+
const registryGate = new Promise<Record<string, never>>((resolve) => {
24+
finishRegistryLoad = resolve;
25+
});
26+
let registryLoadStarted!: () => void;
27+
const registryStarted = new Promise<void>((resolve) => {
28+
registryLoadStarted = resolve;
29+
});
30+
const applyPlugin = vi.fn(() => ({
31+
result: { capabilitiesGranted: [], appliedPlugin: { capabilitiesGranted: [] } },
32+
warnings: [],
33+
}));
34+
const middleware: express.RequestHandler = (_req, _res, next) => next();
35+
36+
registerPluginRoutes(app, {
37+
db: {
38+
prepare: () => ({ all: () => [], get: () => null, run: () => undefined }),
39+
transaction: (run: () => unknown) => () => run(),
40+
},
41+
paths: { PROJECTS_DIR: '', PLUGIN_REGISTRY_ROOTS: [], PLUGIN_LOCKFILE_PATH: '' },
42+
ids: { randomId: () => 'unused' },
43+
projectStore: {},
44+
conversations: {},
45+
verifyWorkspaceRequestAuthority: async () => ({
46+
ok: true,
47+
context: { workspaceId: 'ws-team' },
48+
}),
49+
workspaceResources: {
50+
getWorkspaceResource: () => null,
51+
getWorkspaceResourceByResourceId: () => null,
52+
workspaceTeamPluginBindingAllowsRead: () => bindingLive,
53+
},
54+
plugins: {
55+
getInstalledPlugin: () => null,
56+
getWorkspacePlugin: async () => ({
57+
id: 'team-plugin',
58+
source: 'team:plugin:ws-team:team-plugin',
59+
}),
60+
listInstalledPlugins: () => [],
61+
applyPlugin,
62+
MissingInputError: class MissingInputError extends Error {
63+
fields: string[] = [];
64+
},
65+
},
66+
helpers: {
67+
requireLocalDaemonRequest: middleware,
68+
pluginUpload: {
69+
single: () => middleware,
70+
array: () => middleware,
71+
},
72+
loadPluginRegistryView: async () => {
73+
registryLoadStarted();
74+
return registryGate;
75+
},
76+
buildConnectorProbe: () => ({}),
77+
connectorService: {},
78+
sendApiError: (res: express.Response, status: number, code: string, message: string) =>
79+
res.status(status).json({ error: { code, message } }),
80+
},
81+
} as unknown as Parameters<typeof registerPluginRoutes>[1]);
82+
83+
const server = app.listen(0, '127.0.0.1');
84+
servers.push(server);
85+
await new Promise<void>((resolve) => server.once('listening', resolve));
86+
const { port } = server.address() as AddressInfo;
87+
const responsePromise = fetch(`http://127.0.0.1:${port}/api/plugins/team-plugin/apply`, {
88+
method: 'POST',
89+
headers: {
90+
'content-type': 'application/json',
91+
'x-od-workspace-id': 'ws-team',
92+
'x-od-workspace-type': 'team',
93+
'x-od-workspace-member-id': 'member-team',
94+
'x-od-workspace-role': 'member',
95+
'x-od-workspace-lifecycle-state': 'active',
96+
'x-od-workspace-member-status': 'active',
97+
},
98+
body: '{}',
99+
});
100+
await registryStarted;
101+
bindingLive = false;
102+
finishRegistryLoad({});
103+
104+
const response = await responsePromise;
105+
expect(response.status).toBe(404);
106+
expect(applyPlugin).not.toHaveBeenCalled();
107+
});
108+
});

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
} from '../src/db.js';
3030
import {
3131
listInstalledPlugins,
32+
resolveAndActivateWorkspaceTeamPlugin,
3233
resolveWorkspaceTeamPluginWithBindingGate,
3334
upsertInstalledPlugin,
3435
workspaceTeamPluginBindingAllowsRead,
@@ -210,4 +211,47 @@ describe('listInstalledPlugins workspace scope', () => {
210211

211212
await expect(pending).resolves.toBeNull();
212213
});
214+
215+
it('does not reactivate a Team plugin retracted while materialization is resolving', async () => {
216+
const db = openDatabase(tempDir, { dataDir: tempDir });
217+
const pluginId = 'plugin-sync-retraction';
218+
const workspaceId = 'ws-team';
219+
const bindingId = workspaceTeamPluginBindingResourceId(workspaceId, pluginId);
220+
ensureWorkspaceResource(db, 'plugin', workspaceId, bindingId, {
221+
visibility: 'team',
222+
resourceState: 'active',
223+
});
224+
225+
let finishResolve!: (value: { id: string }) => void;
226+
const resolveGate = new Promise<{ id: string }>((resolve) => {
227+
finishResolve = resolve;
228+
});
229+
let resolveStarted!: () => void;
230+
const started = new Promise<void>((resolve) => {
231+
resolveStarted = resolve;
232+
});
233+
const pending = resolveAndActivateWorkspaceTeamPlugin({
234+
resolve: async () => {
235+
resolveStarted();
236+
return resolveGate;
237+
},
238+
stillShared: async () => false,
239+
activate: () => {
240+
updateWorkspaceResource(db, 'plugin', workspaceId, bindingId, {
241+
resourceState: 'active',
242+
});
243+
return true;
244+
},
245+
});
246+
await started;
247+
updateWorkspaceResource(db, 'plugin', workspaceId, bindingId, {
248+
resourceState: 'deleted',
249+
});
250+
finishResolve({ id: pluginId });
251+
await pending;
252+
253+
expect(
254+
workspaceTeamPluginBindingAllowsRead(db, workspaceId, pluginId),
255+
).toBe(false);
256+
});
213257
});

0 commit comments

Comments
 (0)