From 372fc9ec71b65065fed61d0f24bc8258ee34211b Mon Sep 17 00:00:00 2001 From: Jacob Zeitz Date: Wed, 21 Oct 2026 14:52:04 +0200 Subject: [PATCH] feat: let plugins update and remove the tags they added keywords.add now records the plugin on each definition it adds (addedBy). keywords.update changes the label, colour or visibility of those definitions, never their id; keywords.remove deletes them, as Settings > Tags does, and leaves the keywords on messages alone. Both need settings:write and skip every other definition, so a plugin cannot rename or delete the user's own tags or another plugin's. Co-Authored-By: Claude Opus 5.5 --- lib/__tests__/plugin-api-gates.test.ts | 117 ++++++++++++++++++++++++ lib/plugin-sandbox/host-api.ts | 121 ++++++++++++++++++++++--- lib/plugin-sandbox/runtime.tsx | 24 ++++- stores/settings-store.ts | 1 + 4 files changed, 248 insertions(+), 15 deletions(-) diff --git a/lib/__tests__/plugin-api-gates.test.ts b/lib/__tests__/plugin-api-gates.test.ts index 7e5205a4d..655fb74e3 100644 --- a/lib/__tests__/plugin-api-gates.test.ts +++ b/lib/__tests__/plugin-api-gates.test.ts @@ -154,6 +154,123 @@ describe('native keyword extension API', () => { expect(useSettingsStore.getState().emailKeywords).toEqual(before); }); + it('uses settings:write for updating and removing definitions', async () => { + expect(await gateError(plugin(), 'keywords.update', [[]])) + .toMatch(/lacks permission "settings:write"/); + expect(await gateError(plugin(), 'keywords.remove', [[]])) + .toMatch(/lacks permission "settings:write"/); + }); + + it('records the plugin that added a definition, whatever the input says', async () => { + useSettingsStore.setState({ emailKeywords: [] }); + const writer = plugin({ permissions: ['settings:write'], grantedPermissions: ['settings:write'] }); + + const result = await dispatchApiCall(writer, 'keywords.add', [[ + { id: 'fairs', label: 'Fairs', color: 'blue', addedBy: 'someone-else' }, + ]]) as { added: Array<{ addedBy?: string }> }; + + expect(result.added[0].addedBy).toBe('p1'); + expect(useSettingsStore.getState().emailKeywords).toEqual([ + { id: 'fairs', label: 'Fairs', color: 'blue', addedBy: 'p1' }, + ]); + }); + + it('updates only the definitions the plugin added, never their id', async () => { + useSettingsStore.setState({ + emailKeywords: [ + { id: 'fairs/vison-2026', label: 'Vison 2026', color: 'blue', visibility: 'show', addedBy: 'p1' }, + { id: 'work', label: 'My Work', color: 'purple' }, + { id: 'other', label: 'Other', color: 'green', addedBy: 'p2' }, + ], + }); + const writer = plugin({ permissions: ['settings:write'], grantedPermissions: ['settings:write'] }); + + const result = await dispatchApiCall(writer, 'keywords.update', [[ + { id: 'FAIRS/VISON-2026', label: ' Vision 2026 ', visibility: 'unread' }, + { id: 'work', label: 'Plugin Work' }, + { id: 'other', color: 'red' }, + { id: 'missing', label: 'Missing' }, + ]]); + + expect(result).toEqual({ + updated: [ + { id: 'fairs/vison-2026', label: 'Vision 2026', color: 'blue', visibility: 'unread', addedBy: 'p1' }, + ], + skipped: ['work', 'other', 'missing'], + }); + expect(useSettingsStore.getState().emailKeywords).toEqual([ + { id: 'fairs/vison-2026', label: 'Vision 2026', color: 'blue', visibility: 'unread', addedBy: 'p1' }, + { id: 'work', label: 'My Work', color: 'purple' }, + { id: 'other', label: 'Other', color: 'green', addedBy: 'p2' }, + ]); + }); + + it('rejects invalid updates without partially updating settings', async () => { + const original = [ + { id: 'a', label: 'A', color: 'blue', addedBy: 'p1' }, + { id: 'b', label: 'B', color: 'green', addedBy: 'p1' }, + ]; + useSettingsStore.setState({ emailKeywords: original }); + const writer = plugin({ permissions: ['settings:write'], grantedPermissions: ['settings:write'] }); + + for (const [patches, error] of [ + ['a', /must be an array/], + [[{ id: 'a', label: 'A2' }, { id: 'b', color: 'not-a-color' }], /not in the keyword palette/], + [[{ id: 'a', label: ' ' }], /label must be/], + [[{ id: 'a', visibility: 'sometimes' }], /visibility is invalid/], + [[{ id: 'a' }], /changes nothing/], + [[{ id: 'not valid', label: 'X' }], /not a valid JMAP label id/], + ] as const) { + await expect(dispatchApiCall(writer, 'keywords.update', [patches])).rejects.toThrow(error); + expect(useSettingsStore.getState().emailKeywords).toEqual(original); + } + }); + + it('removes only the definitions the plugin added', async () => { + useSettingsStore.setState({ + emailKeywords: [ + { id: 'fairs/laser-2025', label: 'Laser 2025', color: 'blue', addedBy: 'p1' }, + { id: 'work', label: 'My Work', color: 'purple' }, + { id: 'other', label: 'Other', color: 'green', addedBy: 'p2' }, + { id: 'fairs/vision-2026', label: 'Vision 2026', color: 'red', addedBy: 'p1' }, + ], + }); + const writer = plugin({ permissions: ['settings:write'], grantedPermissions: ['settings:write'] }); + + const result = await dispatchApiCall(writer, 'keywords.remove', [[ + 'FAIRS/LASER-2025', 'work', 'other', 'missing', + ]]); + + expect(result).toEqual({ removed: ['fairs/laser-2025'], skipped: ['work', 'other', 'missing'] }); + expect(useSettingsStore.getState().emailKeywords).toEqual([ + { id: 'work', label: 'My Work', color: 'purple' }, + { id: 'other', label: 'Other', color: 'green', addedBy: 'p2' }, + { id: 'fairs/vision-2026', label: 'Vision 2026', color: 'red', addedBy: 'p1' }, + ]); + }); + + it('rejects invalid remove ids without changing settings', async () => { + const original = [{ id: 'a', label: 'A', color: 'blue', addedBy: 'p1' }]; + useSettingsStore.setState({ emailKeywords: original }); + const writer = plugin({ permissions: ['settings:write'], grantedPermissions: ['settings:write'] }); + + for (const ids of ['a', ['a', 1], Array.from({ length: 501 }, (_, i) => `t${i}`)]) { + await expect(dispatchApiCall(writer, 'keywords.remove', [ids])).rejects.toThrow(/ids must be an array/); + expect(useSettingsStore.getState().emailKeywords).toEqual(original); + } + }); + + it('leaves the settings untouched when nothing matches', async () => { + const original = [{ id: 'work', label: 'My Work', color: 'purple' }]; + useSettingsStore.setState({ emailKeywords: original }); + const writer = plugin({ permissions: ['settings:write'], grantedPermissions: ['settings:write'] }); + + await dispatchApiCall(writer, 'keywords.update', [[{ id: 'work', label: 'X' }]]); + await dispatchApiCall(writer, 'keywords.remove', [['work']]); + + expect(useSettingsStore.getState().emailKeywords).toBe(original); + }); + it('reorders complete label sets without changing user metadata', async () => { useSettingsStore.setState({ emailKeywords: [ diff --git a/lib/plugin-sandbox/host-api.ts b/lib/plugin-sandbox/host-api.ts index 7676982bf..fcfafe90b 100644 --- a/lib/plugin-sandbox/host-api.ts +++ b/lib/plugin-sandbox/host-api.ts @@ -187,6 +187,8 @@ export const PERM_PER_METHOD: Readonly> = { // than a settings permission. 'keywords.list': 'settings:read', 'keywords.add': 'settings:write', + 'keywords.update': 'settings:write', + 'keywords.remove': 'settings:write', 'keywords.reorder': 'settings:write', 'keywords.discover': 'email:read', 'keywords.getCounts': 'email:read', @@ -1358,9 +1360,17 @@ function isValidKeywordDefinitionId(id: string): boolean { return true; } -function assertKeywordDefinition(value: unknown, index: number): PluginKeywordDefinitionInput { +/** The fields of a definition a plugin may set; keywords.update sends only the ones it changes. */ +type PluginKeywordPatch = Pick & + Partial>; + +function assertKeywordFields( + value: unknown, + index: number, + method: 'keywords.add' | 'keywords.update', +): PluginKeywordPatch { if (!isPlainObject(value)) { - throw new Error(`keywords.add: definitions[${index}] must be an object`); + throw new Error(`${method}: definitions[${index}] must be an object`); } const id = value.id; @@ -1369,34 +1379,50 @@ function assertKeywordDefinition(value: unknown, index: number): PluginKeywordDe const visibility = value.visibility; if (typeof id !== 'string' || !isValidKeywordDefinitionId(id)) { - throw new Error(`keywords.add: definitions[${index}].id is not a valid JMAP label id`); + throw new Error(`${method}: definitions[${index}].id is not a valid JMAP label id`); } + // add needs a label; update leaves an omitted one as it is. if ( - typeof label !== 'string' || label.trim().length === 0 || - label.length > MAX_PLUGIN_KEYWORD_LABEL_LENGTH + (label !== undefined || method === 'keywords.add') && + (typeof label !== 'string' || label.trim().length === 0 || + label.length > MAX_PLUGIN_KEYWORD_LABEL_LENGTH) ) { throw new Error( - `keywords.add: definitions[${index}].label must be 1-${MAX_PLUGIN_KEYWORD_LABEL_LENGTH} characters`, + `${method}: definitions[${index}].label must be 1-${MAX_PLUGIN_KEYWORD_LABEL_LENGTH} characters`, ); } if ( color !== undefined && (typeof color !== 'string' || !Object.prototype.hasOwnProperty.call(KEYWORD_PALETTE, color)) ) { - throw new Error(`keywords.add: definitions[${index}].color is not in the keyword palette`); + throw new Error(`${method}: definitions[${index}].color is not in the keyword palette`); } if (visibility !== undefined && !VALID_VISIBILITIES.has(visibility as KeywordVisibility)) { - throw new Error(`keywords.add: definitions[${index}].visibility is invalid`); + throw new Error(`${method}: definitions[${index}].visibility is invalid`); } return { id, - label: label.trim(), + ...(typeof label === 'string' ? { label: label.trim() } : {}), ...(color === undefined ? {} : { color }), ...(visibility === undefined ? {} : { visibility: visibility as KeywordVisibility }), }; } +function assertKeywordDefinition(value: unknown, index: number): PluginKeywordDefinitionInput { + const { label, ...rest } = assertKeywordFields(value, index, 'keywords.add'); + // assertKeywordFields requires the label for add. + return { ...rest, label: label as string }; +} + +function assertKeywordPatch(value: unknown, index: number): PluginKeywordPatch { + const patch = assertKeywordFields(value, index, 'keywords.update'); + if (patch.label === undefined && patch.color === undefined && patch.visibility === undefined) { + throw new Error(`keywords.update: definitions[${index}] changes nothing; give a label, color or visibility`); + } + return patch; +} + function assertKeywordDefinitionArray(value: unknown): PluginKeywordDefinitionInput[] { if (!Array.isArray(value)) throw new Error('keywords.add: definitions must be an array'); if (value.length > MAX_PLUGIN_KEYWORD_DEFINITIONS) { @@ -1405,11 +1431,34 @@ function assertKeywordDefinitionArray(value: unknown): PluginKeywordDefinitionIn return value.map(assertKeywordDefinition); } +function assertKeywordPatchArray(value: unknown): PluginKeywordPatch[] { + if (!Array.isArray(value)) throw new Error('keywords.update: definitions must be an array'); + if (value.length > MAX_PLUGIN_KEYWORD_DEFINITIONS) { + throw new Error(`keywords.update: at most ${MAX_PLUGIN_KEYWORD_DEFINITIONS} definitions may be updated at once`); + } + return value.map(assertKeywordPatch); +} + +function assertKeywordIdArray(value: unknown, method: string): string[] { + if ( + !Array.isArray(value) || value.length > MAX_PLUGIN_KEYWORD_DEFINITIONS || + value.some((id) => typeof id !== 'string') + ) { + throw new Error(`${method}: ids must be an array of at most ${MAX_PLUGIN_KEYWORD_DEFINITIONS} strings`); + } + return value; +} + function doKeywordsList(): KeywordDefinition[] { return useSettingsStore.getState().emailKeywords.map((keyword) => ({ ...keyword })); } -function doKeywordsAdd(value: unknown): { added: KeywordDefinition[]; skipped: string[] } { +/** Whether keywords.update and keywords.remove may touch this definition: only the plugin's own. */ +function isOwnKeyword(plugin: InstalledPlugin, keyword: KeywordDefinition): boolean { + return keyword.addedBy === plugin.id; +} + +function doKeywordsAdd(plugin: InstalledPlugin, value: unknown): { added: KeywordDefinition[]; skipped: string[] } { const definitions = assertKeywordDefinitionArray(value); const existing = useSettingsStore.getState().emailKeywords; const known = new Set(existing.map((keyword) => keyword.id.toLowerCase())); @@ -1426,7 +1475,7 @@ function doKeywordsAdd(value: unknown): { added: KeywordDefinition[]; skipped: s known.add(foldedId); const color = definition.color ?? suggestKeywordColor(definition.id, takenColors); takenColors.add(color); - added.push({ ...definition, color }); + added.push({ ...definition, color, addedBy: plugin.id }); } if (added.length > 0) { @@ -1440,6 +1489,52 @@ function doKeywordsAdd(value: unknown): { added: KeywordDefinition[]; skipped: s return { added: added.map((keyword) => ({ ...keyword })), skipped }; } +function doKeywordsUpdate(plugin: InstalledPlugin, value: unknown): { updated: KeywordDefinition[]; skipped: string[] } { + const patches = assertKeywordPatchArray(value); + const byId = new Map(patches.map((patch) => [patch.id.toLowerCase(), patch])); + let updated: KeywordDefinition[] = []; + + // Read and write in one functional update, as reorder does, so a concurrent + // settings write is not overwritten by a list built from an older state. + useSettingsStore.setState((state) => { + updated = []; + const emailKeywords = state.emailKeywords.map((keyword) => { + const patch = isOwnKeyword(plugin, keyword) ? byId.get(keyword.id.toLowerCase()) : undefined; + if (!patch) return keyword; + // The id is what the messages carry, so it never changes here. + const next = { ...keyword, ...patch, id: keyword.id }; + updated.push(next); + return next; + }); + return updated.length > 0 ? { emailKeywords } : state; + }); + + const done = new Set(updated.map((keyword) => keyword.id.toLowerCase())); + const skipped = patches.filter((patch) => !done.has(patch.id.toLowerCase())).map((patch) => patch.id); + return { updated: updated.map((keyword) => ({ ...keyword })), skipped }; +} + +function doKeywordsRemove(plugin: InstalledPlugin, value: unknown): { removed: string[]; skipped: string[] } { + const ids = assertKeywordIdArray(value, 'keywords.remove'); + const wanted = new Set(ids.map((id) => id.toLowerCase())); + let removed: string[] = []; + + // Drops the definitions only, as Settings → Tags does; the keywords on + // messages stay (jmap.setKeywords removes those, with email:write). + useSettingsStore.setState((state) => { + removed = []; + const emailKeywords = state.emailKeywords.filter((keyword) => { + const drop = isOwnKeyword(plugin, keyword) && wanted.has(keyword.id.toLowerCase()); + if (drop) removed.push(keyword.id); + return !drop; + }); + return removed.length > 0 ? { emailKeywords } : state; + }); + + const done = new Set(removed.map((id) => id.toLowerCase())); + return { removed, skipped: ids.filter((id) => !done.has(id.toLowerCase())) }; +} + function doKeywordsReorder(value: unknown, rawOptions?: unknown): KeywordDefinition[] { if (!Array.isArray(value)) { throw new Error('keywords.reorder: ids must be an array'); @@ -1986,7 +2081,9 @@ export async function dispatchApiCall( } case 'keywords.list': return doKeywordsList(); - case 'keywords.add': return doKeywordsAdd(args[0]); + case 'keywords.add': return doKeywordsAdd(plugin, args[0]); + case 'keywords.update': return doKeywordsUpdate(plugin, args[0]); + case 'keywords.remove': return doKeywordsRemove(plugin, args[0]); case 'keywords.reorder': return doKeywordsReorder(args[0], args[1]); case 'keywords.discover': return doKeywordsDiscover(args[0]); case 'keywords.getCounts': return doKeywordsGetCounts(args[0]); diff --git a/lib/plugin-sandbox/runtime.tsx b/lib/plugin-sandbox/runtime.tsx index 6f91bcab2..bd3c91e4a 100644 --- a/lib/plugin-sandbox/runtime.tsx +++ b/lib/plugin-sandbox/runtime.tsx @@ -297,9 +297,14 @@ interface PluginKeywordDefinition { label: string; color: string; visibility?: PluginKeywordVisibility; + /** The plugin that added it with keywords.add; absent on the user's own tags. */ + addedBy?: string; } -type PluginKeywordDefinitionInput = Omit & { color?: string }; +type PluginKeywordDefinitionInput = Omit & { color?: string }; + +type PluginKeywordPatch = Pick & + Partial>; interface PluginKeywordCounts { total: number; @@ -666,8 +671,9 @@ function buildPluginApi(manifest: PluginManifest) { }, // Native sidebar tag definitions. Definition reads/writes use the existing // settings permissions; server discovery and message counts use email:read. - // add() is intentionally append-only. reorder() requires a complete - // permutation of existing ids. Neither method overwrites or removes tags. + // add() never overwrites a tag that is there. update() and remove() reach + // only the tags this plugin added, never the user's own or another + // plugin's. reorder() requires a complete permutation of existing ids. keywords: { list: () => callApi('keywords.list', []) as Promise, add: (definitions: PluginKeywordDefinitionInput[]) => @@ -675,6 +681,18 @@ function buildPluginApi(manifest: PluginManifest) { added: PluginKeywordDefinition[]; skipped: string[]; }>, + /** Change the label, color or visibility of tags this plugin added; the id stays. */ + update: (definitions: PluginKeywordPatch[]) => + callApi('keywords.update', [definitions]) as Promise<{ + updated: PluginKeywordDefinition[]; + skipped: string[]; + }>, + /** Delete tags this plugin added. Messages keep their keywords. */ + remove: (ids: string[]) => + callApi('keywords.remove', [ids]) as Promise<{ + removed: string[]; + skipped: string[]; + }>, reorder: (ids: string[], options?: { caseSensitive?: boolean }) => callApi('keywords.reorder', [ids, options]) as Promise, discover: (options?: { limit?: number }) => diff --git a/stores/settings-store.ts b/stores/settings-store.ts index 33344211a..17e37bdf3 100644 --- a/stores/settings-store.ts +++ b/stores/settings-store.ts @@ -358,6 +358,7 @@ export interface KeywordDefinition { label: string; // Display name color: string; // Key from KEYWORD_PALETTE visibility?: KeywordVisibility; // Absent on tags stored before this was configurable + addedBy?: string; // Id of the plugin that added it with keywords.add; only that plugin may update or remove it } /** Resolves the sidebar visibility of a tag, defaulting to always shown. */