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. */