Merge pull request #1333 from HOLOEYE-JZ/feat/plugin-keywords-update-remove

feat: let plugins update and remove the tags they added

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: [

lib/plugin-sandbox/host-api.ts

@@ -187,6 +187,8 @@ export const PERM_PER_METHOD: Readonly<Record<string, Permission | null>> = {
   // 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<KeywordDefinition, 'id'> &
+  Partial<Pick<KeywordDefinition, 'label' | 'color' | 'visibility'>>;
+
+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]);

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<PluginKeywordDefinition, 'color'> & { color?: string };
+type PluginKeywordDefinitionInput = Omit<PluginKeywordDefinition, 'color' | 'addedBy'> & { color?: string };
+
+type PluginKeywordPatch = Pick<PluginKeywordDefinition, 'id'> &
+  Partial<Pick<PluginKeywordDefinition, 'label' | 'color' | 'visibility'>>;
 
 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<PluginKeywordDefinition[]>,
       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<PluginKeywordDefinition[]>,
       discover: (options?: { limit?: number }) =>

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