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 <noreply@anthropic.com>
This commit is contained in:
Jacob Zeitz
2026-10-24 08:54:23 +02:00
co-authored by Claude Opus 5.5
parent d1f4ccaccf
commit 372fc9ec71
4 changed files with 248 additions and 15 deletions
+117
View File
@@ -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: [
+109 -12
View File
@@ -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]);
+21 -3
View File
@@ -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 }) =>
+1
View File
@@ -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. */