From f94a46decf3dafc2f22f491a9c897bc81be411b2 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Thu, 14 May 2026 21:14:00 +0200 Subject: [PATCH] feat(plugin-automations): add taskStarted/taskStopped triggers and removeTag action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two new triggers and a new action to the bundled automations plugin, along with the host-side and validator changes needed to make them reliable end to end. Plugin - New triggers TriggerTaskStarted / TriggerTaskStopped that fire on timer start / stop. Switching from task A to task B emits taskStopped(A) and then taskStarted(B), so a rule subscribed to either trigger gets the full lifecycle. The trigger descriptions document this explicitly. - New ActionRemoveTag mirroring ActionAddTag, with a shared resolveTagId helper covering id/title lookup, missing-tag warning, and idempotent short-circuiting. - Import goes through a new RuleRegistry.addRules(rules[]) + addRules message, so the host's 1 call/sec persistDataSynced rate limit no longer rejects multi-rule imports. - Validators in core/rule-registry.ts and utils/rule-validator.ts share a set of `as const` arrays guarded by a TS _AssertEq exhaustiveness check against the union types in types.ts. The arrays are duplicated rather than exported from types.ts because that would turn types.ts into a shared runtime chunk and break plugin.js (which the host evaluates via `new Function`, not as an ES module). - Import validator no longer requires a truthy rule name — the UI saves empty-named rules, the load-path accepts them, and the import path needed to match. - ActionDialog gets a removeTag placeholder. Manifest's hooks list is updated to include the hooks the plugin actually registers (taskCreated, currentTaskChange) so the permission disclosure UI is accurate. Host - plugin-hooks.effects.ts onCurrentTaskChange$ now observes selectCurrentTask directly instead of only setCurrentTask / unsetCurrentTask actions. This catches transitions through reducer paths that don't dispatch those actions (loadAllData, deleteProject, bulk task delete via the shared CRUD meta-reducer). - The payload changes from the raw new Task to { current, previous }, matching the long-declared CurrentTaskChangePayload shape. The effect uses pairwise on the selector and filters same-id pairs at the event boundary (not via distinctUntilChanged on the source) so the `previous` task always carries the latest snapshot — including in-place updates a plugin made while the task was running. Without this, addTag-on-start / removeTag-on-stop only worked every other cycle because `previous` reflected the pre-mutation snapshot. Other plugins consuming currentTaskChange (voice-reminder, api-test-plugin) read payload.current instead of the raw Task. --- packages/plugin-dev/api-test-plugin/plugin.js | 4 +- .../plugin-dev/automations/src/app/App.tsx | 13 ++- .../src/app/components/ActionDialog.tsx | 2 + .../src/app/components/RuleEditor.tsx | 3 + .../automations/src/core/actions.spec.ts | 56 ++++++++++ .../automations/src/core/actions.ts | 39 +++++-- .../src/core/automation-manager.ts | 3 + .../src/core/rule-registry.spec.ts | 70 ++++++++++++ .../automations/src/core/rule-registry.ts | 89 +++++++++++---- .../automations/src/core/triggers.spec.ts | 39 ++++--- .../automations/src/core/triggers.ts | 16 +++ .../plugin-dev/automations/src/manifest.json | 2 +- packages/plugin-dev/automations/src/plugin.ts | 19 ++++ packages/plugin-dev/automations/src/types.ts | 9 +- .../src/utils/rule-validator.spec.ts | 24 ++++- .../automations/src/utils/rule-validator.ts | 46 ++++++-- packages/plugin-dev/voice-reminder/plugin.js | 6 +- src/app/plugins/plugin-hooks.effects.spec.ts | 102 +++++++++++++++--- src/app/plugins/plugin-hooks.effects.ts | 30 ++++-- 19 files changed, 481 insertions(+), 91 deletions(-) diff --git a/packages/plugin-dev/api-test-plugin/plugin.js b/packages/plugin-dev/api-test-plugin/plugin.js index 2f0dc9d155..edfddd2c63 100644 --- a/packages/plugin-dev/api-test-plugin/plugin.js +++ b/packages/plugin-dev/api-test-plugin/plugin.js @@ -235,8 +235,8 @@ PluginAPI.registerHook(PluginAPI.Hooks.TASK_DELETE, (taskData) => { console.log('API Test - TASK_DELETE hook fired:', taskData); }); -PluginAPI.registerHook(PluginAPI.Hooks.CURRENT_TASK_CHANGE, (taskData) => { - console.log('API Test - CURRENT_TASK_CHANGE hook fired:', taskData); +PluginAPI.registerHook(PluginAPI.Hooks.CURRENT_TASK_CHANGE, (payload) => { + console.log('API Test - CURRENT_TASK_CHANGE hook fired:', payload); }); // Register UI elements diff --git a/packages/plugin-dev/automations/src/app/App.tsx b/packages/plugin-dev/automations/src/app/App.tsx index cd182cb571..1738227b9c 100644 --- a/packages/plugin-dev/automations/src/app/App.tsx +++ b/packages/plugin-dev/automations/src/app/App.tsx @@ -109,10 +109,15 @@ function App() { console.warn(`Skipping ${invalidRules.length} invalid rules during import.`); } - for (const rule of validRules) { - // Always generate a new ID to ensure we add to existing rules instead of overwriting - const ruleToSave = { ...rule, id: Math.random().toString(36).substr(2, 9) }; - await sendMessage('saveRule', ruleToSave); + // Always generate new IDs so import adds to existing rules instead of + // overwriting them. Batch all rules into a single persist call to stay + // under the host's plugin-data rate limit (1/sec). + const rulesToSave = validRules.map((rule) => ({ + ...rule, + id: Math.random().toString(36).substr(2, 9), + })); + if (rulesToSave.length > 0) { + await sendMessage('addRules', rulesToSave); } await fetchRules(); diff --git a/packages/plugin-dev/automations/src/app/components/ActionDialog.tsx b/packages/plugin-dev/automations/src/app/components/ActionDialog.tsx index aaeda7ce4c..dbc8fc439b 100644 --- a/packages/plugin-dev/automations/src/app/components/ActionDialog.tsx +++ b/packages/plugin-dev/automations/src/app/components/ActionDialog.tsx @@ -18,6 +18,7 @@ export function ActionDialog(props: ActionDialogProps) { 'createTask', 'deleteTask', 'addTag', + 'removeTag', 'moveToProject', 'displaySnack', 'displayDialog', @@ -44,6 +45,7 @@ export function ActionDialog(props: ActionDialogProps) { case 'deleteTask': return 'Deletes the task that triggered this rule'; case 'addTag': + case 'removeTag': return 'e.g. "review-needed"'; case 'moveToProject': return 'e.g. "Project A"'; diff --git a/packages/plugin-dev/automations/src/app/components/RuleEditor.tsx b/packages/plugin-dev/automations/src/app/components/RuleEditor.tsx index 149550aff2..3250737d31 100644 --- a/packages/plugin-dev/automations/src/app/components/RuleEditor.tsx +++ b/packages/plugin-dev/automations/src/app/components/RuleEditor.tsx @@ -47,6 +47,8 @@ export function RuleEditor(props: RuleEditorProps) { 'taskCreated', 'taskUpdated', 'taskCompleted', + 'taskStarted', + 'taskStopped', 'timeBased', ]; @@ -65,6 +67,7 @@ export function RuleEditor(props: RuleEditorProps) { 'createTask', 'deleteTask', 'addTag', + 'removeTag', 'moveToProject', 'displaySnack', 'displayDialog', diff --git a/packages/plugin-dev/automations/src/core/actions.spec.ts b/packages/plugin-dev/automations/src/core/actions.spec.ts index a78cc6cbe3..55174887fc 100644 --- a/packages/plugin-dev/automations/src/core/actions.spec.ts +++ b/packages/plugin-dev/automations/src/core/actions.spec.ts @@ -3,6 +3,7 @@ import { ActionCreateTask, ActionDeleteTask, ActionAddTag, + ActionRemoveTag, ActionMoveToProject, ActionDisplaySnack, ActionDisplayDialog, @@ -124,6 +125,61 @@ describe('Actions', () => { }); }); + describe('ActionRemoveTag', () => { + it('should remove a tag if present on the task', async () => { + (mockDataCache.getTags as any).mockResolvedValue([{ id: 't1', title: 'Urgent' }]); + const event = { + task: { id: 'task1', tagIds: ['t1', 'other'] }, + } as unknown as TaskEvent; + + await ActionRemoveTag.execute(mockContext, event, 'Urgent'); + + expect(mockPlugin.updateTask).toHaveBeenCalledWith('task1', { + tagIds: ['other'], + }); + }); + + it('should resolve by tag id as well as title', async () => { + (mockDataCache.getTags as any).mockResolvedValue([{ id: 't1', title: 'Urgent' }]); + const event = { + task: { id: 'task1', tagIds: ['t1'] }, + } as unknown as TaskEvent; + + await ActionRemoveTag.execute(mockContext, event, 't1'); + + expect(mockPlugin.updateTask).toHaveBeenCalledWith('task1', { tagIds: [] }); + }); + + it('should warn if task is missing', async () => { + await ActionRemoveTag.execute(mockContext, { task: undefined } as TaskEvent, 'Urgent'); + expect(mockPlugin.log.warn).toHaveBeenCalledWith( + expect.stringContaining('without task context'), + ); + expect(mockPlugin.updateTask).not.toHaveBeenCalled(); + }); + + it('should warn if tag not found', async () => { + (mockDataCache.getTags as any).mockResolvedValue([]); + const event = { + task: { id: 'task1', tagIds: ['t1'] }, + } as unknown as TaskEvent; + + await ActionRemoveTag.execute(mockContext, event, 'NonExistent'); + expect(mockPlugin.log.warn).toHaveBeenCalledWith(expect.stringContaining('not found')); + expect(mockPlugin.updateTask).not.toHaveBeenCalled(); + }); + + it('should do nothing if tag not on the task', async () => { + (mockDataCache.getTags as any).mockResolvedValue([{ id: 't1', title: 'Urgent' }]); + const event = { + task: { id: 'task1', tagIds: ['other'] }, + } as unknown as TaskEvent; + + await ActionRemoveTag.execute(mockContext, event, 'Urgent'); + expect(mockPlugin.updateTask).not.toHaveBeenCalled(); + }); + }); + describe('ActionMoveToProject', () => { it('should move task to project if it exists', async () => { (mockDataCache.getProjects as any).mockResolvedValue([{ id: 'p1', title: 'Project A' }]); diff --git a/packages/plugin-dev/automations/src/core/actions.ts b/packages/plugin-dev/automations/src/core/actions.ts index e6c3f17c10..582de326a6 100644 --- a/packages/plugin-dev/automations/src/core/actions.ts +++ b/packages/plugin-dev/automations/src/core/actions.ts @@ -1,4 +1,13 @@ -import { IAutomationAction } from './definitions'; +import { AutomationContext, IAutomationAction } from './definitions'; + +const resolveTagId = async (ctx: AutomationContext, value: string): Promise => { + const tag = (await ctx.dataCache.getTags()).find((t) => t.id === value || t.title === value); + if (!tag) { + ctx.plugin.log.warn(`[Automation] Tag "${value}" not found.`); + return null; + } + return tag.id; +}; export const ActionCreateTask: IAutomationAction = { id: 'createTask', @@ -35,13 +44,8 @@ export const ActionAddTag: IAutomationAction = { ctx.plugin.log.warn(`[Automation] Cannot add tag "${value}" without task context.`); return; } - const tags = await ctx.dataCache.getTags(); - let tagId = tags.find((t) => t.id === value || t.title === value)?.id; - - if (!tagId) { - ctx.plugin.log.warn(`[Automation] Tag "${value}" not found.`); - return; - } + const tagId = await resolveTagId(ctx, value); + if (!tagId) return; if (event.task.tagIds.includes(tagId)) return; await ctx.plugin.updateTask(event.task.id, { @@ -51,6 +55,25 @@ export const ActionAddTag: IAutomationAction = { }, }; +export const ActionRemoveTag: IAutomationAction = { + id: 'removeTag', + name: 'Remove Tag', + execute: async (ctx, event, value) => { + if (!event.task || !value) { + ctx.plugin.log.warn(`[Automation] Cannot remove tag "${value}" without task context.`); + return; + } + const tagId = await resolveTagId(ctx, value); + if (!tagId) return; + if (!event.task.tagIds.includes(tagId)) return; + + await ctx.plugin.updateTask(event.task.id, { + tagIds: event.task.tagIds.filter((id) => id !== tagId), + }); + ctx.plugin.log.info(`[Automation] Action: Removed tag "${value}"`); + }, +}; + export const ActionMoveToProject: IAutomationAction = { id: 'moveToProject', name: 'Move to Project', diff --git a/packages/plugin-dev/automations/src/core/automation-manager.ts b/packages/plugin-dev/automations/src/core/automation-manager.ts index 79fdc0ef07..a55f42f4e6 100644 --- a/packages/plugin-dev/automations/src/core/automation-manager.ts +++ b/packages/plugin-dev/automations/src/core/automation-manager.ts @@ -46,6 +46,8 @@ export class AutomationManager { globalRegistry.registerTrigger(Triggers.TriggerTaskCompleted); globalRegistry.registerTrigger(Triggers.TriggerTaskCreated); globalRegistry.registerTrigger(Triggers.TriggerTaskUpdated); + globalRegistry.registerTrigger(Triggers.TriggerTaskStarted); + globalRegistry.registerTrigger(Triggers.TriggerTaskStopped); globalRegistry.registerTrigger(Triggers.TriggerTimeBased); // Conditions @@ -59,6 +61,7 @@ export class AutomationManager { globalRegistry.registerAction(Actions.ActionCreateTask); globalRegistry.registerAction(Actions.ActionDeleteTask); globalRegistry.registerAction(Actions.ActionAddTag); + globalRegistry.registerAction(Actions.ActionRemoveTag); globalRegistry.registerAction(Actions.ActionMoveToProject); globalRegistry.registerAction(Actions.ActionDisplaySnack); globalRegistry.registerAction(Actions.ActionDisplayDialog); diff --git a/packages/plugin-dev/automations/src/core/rule-registry.spec.ts b/packages/plugin-dev/automations/src/core/rule-registry.spec.ts index 0058dcb6af..c84e5a7f6b 100644 --- a/packages/plugin-dev/automations/src/core/rule-registry.spec.ts +++ b/packages/plugin-dev/automations/src/core/rule-registry.spec.ts @@ -68,6 +68,33 @@ describe('RuleRegistry', () => { expect(await registry.getRules()).toEqual(rules); }); + it('should load rules with taskStarted/taskStopped triggers and removeTag action', async () => { + const rules: AutomationRule[] = [ + { + id: 'r1', + name: 'Tag while running', + isEnabled: true, + trigger: { type: 'taskStarted' }, + conditions: [], + actions: [{ type: 'addTag', value: 'in-progress' }], + }, + { + id: 'r2', + name: 'Untag when stopped', + isEnabled: true, + trigger: { type: 'taskStopped' }, + conditions: [], + actions: [{ type: 'removeTag', value: 'in-progress' }], + }, + ]; + (mockPlugin.loadSyncedData as any).mockResolvedValue(JSON.stringify(rules)); + + registry = new RuleRegistry(mockPlugin); + await new Promise(process.nextTick); + + expect(await registry.getRules()).toEqual(rules); + }); + it('should load existing rules with deleteTask actions', async () => { const rules: AutomationRule[] = [ { @@ -106,6 +133,49 @@ describe('RuleRegistry', () => { expect(mockPlugin.persistDataSynced).toHaveBeenCalledWith(JSON.stringify([newRule])); }); + it('addRules persists every rule with a single persistDataSynced call', async () => { + registry = new RuleRegistry(mockPlugin); + await new Promise(process.nextTick); + + const rules: AutomationRule[] = [ + { + id: 'r1', + name: 'A', + isEnabled: true, + trigger: { type: 'taskStarted' }, + conditions: [], + actions: [{ type: 'addTag', value: 'in-progress' }], + }, + { + id: 'r2', + name: 'B', + isEnabled: true, + trigger: { type: 'taskStopped' }, + conditions: [], + actions: [{ type: 'removeTag', value: 'in-progress' }], + }, + ]; + + // One persistDataSynced call (constructor init) before the batch. + expect((mockPlugin.persistDataSynced as any).mock.calls.length).toBe(1); + + await registry.addRules(rules); + + // Exactly one additional persist for the whole batch — no rate-limit risk. + expect((mockPlugin.persistDataSynced as any).mock.calls.length).toBe(2); + expect(await registry.getRules()).toEqual(rules); + }); + + it('addRules is a no-op for an empty array', async () => { + registry = new RuleRegistry(mockPlugin); + await new Promise(process.nextTick); + const before = (mockPlugin.persistDataSynced as any).mock.calls.length; + + await registry.addRules([]); + + expect((mockPlugin.persistDataSynced as any).mock.calls.length).toBe(before); + }); + it('should update existing rule', async () => { const rule: AutomationRule = { id: 'r1', diff --git a/packages/plugin-dev/automations/src/core/rule-registry.ts b/packages/plugin-dev/automations/src/core/rule-registry.ts index 70c4827348..a6d7434563 100644 --- a/packages/plugin-dev/automations/src/core/rule-registry.ts +++ b/packages/plugin-dev/automations/src/core/rule-registry.ts @@ -1,6 +1,52 @@ -import { AutomationRule } from '../types'; +import { ActionType, AutomationRule, AutomationTriggerType, ConditionType } from '../types'; import { PluginAPI } from '@super-productivity/plugin-api'; +// Keep these arrays in sync with the unions in types.ts. They are duplicated +// here (and in utils/rule-validator.ts) rather than centralized in types.ts +// because types.ts must stay type-only — otherwise vite produces a shared +// runtime chunk that breaks plugin.js (which the host evaluates with +// `new Function`, not as an ES module). +// +// The `satisfies` clause catches extra / mistyped entries. The `_AssertEq` +// helper enforces the other direction: if a union gains a new member, this +// file fails to compile until the corresponding array is updated. +type _AssertEq = [T] extends [U] ? ([U] extends [T] ? true : never) : never; + +const AUTOMATION_TRIGGER_TYPES = [ + 'taskCompleted', + 'taskCreated', + 'taskUpdated', + 'taskStarted', + 'taskStopped', + 'timeBased', +] as const satisfies readonly AutomationTriggerType[]; +const _exhaustiveTriggers: _AssertEq< + AutomationTriggerType, + (typeof AUTOMATION_TRIGGER_TYPES)[number] +> = true; + +const AUTOMATION_CONDITION_TYPES = [ + 'titleContains', + 'titleStartsWith', + 'projectIs', + 'hasTag', + 'weekdayIs', +] as const satisfies readonly ConditionType[]; +const _exhaustiveConditions: _AssertEq = + true; + +const AUTOMATION_ACTION_TYPES = [ + 'createTask', + 'deleteTask', + 'addTag', + 'removeTag', + 'moveToProject', + 'displaySnack', + 'displayDialog', + 'webhook', +] as const satisfies readonly ActionType[]; +const _exhaustiveActions: _AssertEq = true; + export class RuleRegistry { private rules: AutomationRule[] = []; private plugin: PluginAPI; @@ -64,23 +110,9 @@ export class RuleRegistry { return null; } - const validTriggers = new Set(['taskCompleted', 'taskCreated', 'taskUpdated', 'timeBased']); - const validConditions = new Set([ - 'titleContains', - 'titleStartsWith', - 'projectIs', - 'hasTag', - 'weekdayIs', - ]); - const validActions = new Set([ - 'createTask', - 'deleteTask', - 'addTag', - 'moveToProject', - 'displaySnack', - 'displayDialog', - 'webhook', - ]); + const validTriggers = new Set(AUTOMATION_TRIGGER_TYPES); + const validConditions = new Set(AUTOMATION_CONDITION_TYPES); + const validActions = new Set(AUTOMATION_ACTION_TYPES); const isValidCondition = (c: any) => c && @@ -170,6 +202,27 @@ export class RuleRegistry { await this.saveQueue; } + async addRules(rules: AutomationRule[]) { + if (rules.length === 0) return; + await this.initPromise; + this.saveQueue = this.saveQueue.then(async () => { + for (const rule of rules) { + const index = this.rules.findIndex((r) => r.id === rule.id); + if (index !== -1) { + this.rules[index] = rule; + } else { + this.rules.push(rule); + } + } + try { + await this.plugin.persistDataSynced(JSON.stringify(this.rules)); + } catch (e) { + this.plugin.log.error('Failed to save rules', e); + } + }); + await this.saveQueue; + } + async deleteRule(ruleId: string) { await this.initPromise; this.saveQueue = this.saveQueue.then(async () => { diff --git a/packages/plugin-dev/automations/src/core/triggers.spec.ts b/packages/plugin-dev/automations/src/core/triggers.spec.ts index bac7b7e62d..f49df0f277 100644 --- a/packages/plugin-dev/automations/src/core/triggers.spec.ts +++ b/packages/plugin-dev/automations/src/core/triggers.spec.ts @@ -2,33 +2,30 @@ import { describe, it, expect } from 'vitest'; import { TriggerTaskCompleted, TriggerTaskCreated, + TriggerTaskStarted, + TriggerTaskStopped, TriggerTaskUpdated, TriggerTimeBased, } from './triggers'; -import { TaskEvent } from '../types'; +import { IAutomationTrigger } from './definitions'; +import { AutomationTriggerType, TaskEvent } from '../types'; describe('Triggers', () => { - it('TriggerTaskCompleted should match taskCompleted event', () => { - const event = { type: 'taskCompleted' } as TaskEvent; - expect(TriggerTaskCompleted.matches(event)).toBe(true); - expect(TriggerTaskCompleted.matches({ type: 'taskCreated' } as TaskEvent)).toBe(false); - }); + const cases: [IAutomationTrigger, AutomationTriggerType][] = [ + [TriggerTaskCompleted, 'taskCompleted'], + [TriggerTaskCreated, 'taskCreated'], + [TriggerTaskUpdated, 'taskUpdated'], + [TriggerTaskStarted, 'taskStarted'], + [TriggerTaskStopped, 'taskStopped'], + [TriggerTimeBased, 'timeBased'], + ]; - it('TriggerTaskCreated should match taskCreated event', () => { - const event = { type: 'taskCreated' } as TaskEvent; - expect(TriggerTaskCreated.matches(event)).toBe(true); - expect(TriggerTaskCreated.matches({ type: 'taskCompleted' } as TaskEvent)).toBe(false); - }); + it.each(cases)('$1 trigger matches only its own event type', (trigger, type) => { + expect(trigger.matches({ type } as TaskEvent)).toBe(true); - it('TriggerTaskUpdated should match taskUpdated event', () => { - const event = { type: 'taskUpdated' } as TaskEvent; - expect(TriggerTaskUpdated.matches(event)).toBe(true); - expect(TriggerTaskUpdated.matches({ type: 'taskCompleted' } as TaskEvent)).toBe(false); - }); - - it('TriggerTimeBased should match timeBased event', () => { - const event = { type: 'timeBased' } as TaskEvent; - expect(TriggerTimeBased.matches(event)).toBe(true); - expect(TriggerTimeBased.matches({ type: 'taskCompleted' } as TaskEvent)).toBe(false); + for (const [, otherType] of cases) { + if (otherType === type) continue; + expect(trigger.matches({ type: otherType } as TaskEvent)).toBe(false); + } }); }); diff --git a/packages/plugin-dev/automations/src/core/triggers.ts b/packages/plugin-dev/automations/src/core/triggers.ts index e018942d2a..8f3145156c 100644 --- a/packages/plugin-dev/automations/src/core/triggers.ts +++ b/packages/plugin-dev/automations/src/core/triggers.ts @@ -18,6 +18,22 @@ export const TriggerTaskUpdated: IAutomationTrigger = { matches: (event) => event.type === 'taskUpdated', }; +export const TriggerTaskStarted: IAutomationTrigger = { + id: 'taskStarted', + name: 'Task Started', + description: + 'Fires when the timer starts for a task. Switching to a different task also fires this for the new task (and Task Stopped for the previous one).', + matches: (event) => event.type === 'taskStarted', +}; + +export const TriggerTaskStopped: IAutomationTrigger = { + id: 'taskStopped', + name: 'Task Stopped', + description: + 'Fires when the timer is paused/stopped for a task (without completing it). Switching to a different task also fires this for the previous task.', + matches: (event) => event.type === 'taskStopped', +}; + export const TriggerTimeBased: IAutomationTrigger = { id: 'timeBased', name: 'Time Based', diff --git a/packages/plugin-dev/automations/src/manifest.json b/packages/plugin-dev/automations/src/manifest.json index 0484988dfc..645404c3d4 100644 --- a/packages/plugin-dev/automations/src/manifest.json +++ b/packages/plugin-dev/automations/src/manifest.json @@ -12,7 +12,7 @@ "url": "" }, "permissions": [], - "hooks": ["taskComplete", "taskUpdate", "anyTaskUpdate"], + "hooks": ["taskCreated", "taskComplete", "taskUpdate", "anyTaskUpdate", "currentTaskChange"], "iFrame": true, "sidePanel": false, "isSkipMenuEntry": false, diff --git a/packages/plugin-dev/automations/src/plugin.ts b/packages/plugin-dev/automations/src/plugin.ts index 3e1e726eb0..2dc252554a 100644 --- a/packages/plugin-dev/automations/src/plugin.ts +++ b/packages/plugin-dev/automations/src/plugin.ts @@ -1,4 +1,5 @@ import { + CurrentTaskChangePayload, PluginAPI, TaskCompletePayload, TaskUpdatePayload, @@ -53,6 +54,21 @@ plugin.registerHook('taskUpdate' as any, (payload: TaskUpdatePayload) => { }); }); +// The host provides both the new current task and the previous one on every +// transition, so we don't need to track state locally. A switch between two +// tasks arrives as { current: B, previous: A } and fires both stop+start. +plugin.registerHook( + 'currentTaskChange' as any, + ({ current, previous }: CurrentTaskChangePayload) => { + if (previous) { + automationManager.onTaskEvent({ type: 'taskStopped', task: previous }); + } + if (current) { + automationManager.onTaskEvent({ type: 'taskStarted', task: current }); + } + }, +); + // Register UI commands if (plugin.onMessage) { plugin.onMessage(async (message: any) => { @@ -74,6 +90,9 @@ if (plugin.onMessage) { case 'saveRule': await automationManager.getRegistry().addOrUpdateRule(message.payload); return { success: true }; + case 'addRules': + await automationManager.getRegistry().addRules(message.payload); + return { success: true }; case 'deleteRule': await automationManager.getRegistry().deleteRule(message.payload.id); return { success: true }; diff --git a/packages/plugin-dev/automations/src/types.ts b/packages/plugin-dev/automations/src/types.ts index 1dbead952a..a689cd2579 100644 --- a/packages/plugin-dev/automations/src/types.ts +++ b/packages/plugin-dev/automations/src/types.ts @@ -1,6 +1,12 @@ import { Task } from '@super-productivity/plugin-api'; -export type AutomationTriggerType = 'taskCompleted' | 'taskCreated' | 'taskUpdated' | 'timeBased'; +export type AutomationTriggerType = + | 'taskCompleted' + | 'taskCreated' + | 'taskUpdated' + | 'taskStarted' + | 'taskStopped' + | 'timeBased'; export interface AutomationTrigger { type: AutomationTriggerType; @@ -31,6 +37,7 @@ export type ActionType = | 'createTask' | 'deleteTask' | 'addTag' + | 'removeTag' | 'moveToProject' | 'displaySnack' | 'displayDialog' diff --git a/packages/plugin-dev/automations/src/utils/rule-validator.spec.ts b/packages/plugin-dev/automations/src/utils/rule-validator.spec.ts index 22319f9028..ab349729aa 100644 --- a/packages/plugin-dev/automations/src/utils/rule-validator.spec.ts +++ b/packages/plugin-dev/automations/src/utils/rule-validator.spec.ts @@ -29,12 +29,15 @@ describe('validateRule', () => { expect(validateRule('string')).toBe(false); }); - it('should fail if name is missing or invalid', () => { + it('should fail if name is missing or not a string', () => { expect(validateRule({ ...validRule, name: undefined })).toBe(false); - expect(validateRule({ ...validRule, name: '' })).toBe(false); expect(validateRule({ ...validRule, name: 123 })).toBe(false); }); + it('should accept an empty name (matches what the UI and load path allow)', () => { + expect(validateRule({ ...validRule, name: '' })).toBe(true); + }); + it('should fail if isEnabled is missing or invalid', () => { expect(validateRule({ ...validRule, isEnabled: undefined })).toBe(false); expect(validateRule({ ...validRule, isEnabled: 'true' })).toBe(false); @@ -96,6 +99,23 @@ describe('validateRule', () => { expect(validateRule(rule)).toBe(true); }); + it('should validate the taskStarted/taskStopped triggers and removeTag action', () => { + expect( + validateRule({ + ...validRule, + trigger: { type: 'taskStarted' }, + actions: [{ type: 'addTag', value: 'in-progress' }], + }), + ).toBe(true); + expect( + validateRule({ + ...validRule, + trigger: { type: 'taskStopped' }, + actions: [{ type: 'removeTag', value: 'in-progress' }], + }), + ).toBe(true); + }); + it('should validate regex-enabled title conditions', () => { const rule: AutomationRule = { ...validRule, diff --git a/packages/plugin-dev/automations/src/utils/rule-validator.ts b/packages/plugin-dev/automations/src/utils/rule-validator.ts index afab23020c..023915a574 100644 --- a/packages/plugin-dev/automations/src/utils/rule-validator.ts +++ b/packages/plugin-dev/automations/src/utils/rule-validator.ts @@ -1,45 +1,69 @@ -import { AutomationTriggerType, ConditionType, ActionType } from '../types'; +import { ActionType, AutomationTriggerType, ConditionType } from '../types'; -const VALID_TRIGGER_TYPES: AutomationTriggerType[] = [ +// Keep these arrays in sync with the unions in types.ts. They are duplicated +// here (and in core/rule-registry.ts) rather than centralized in types.ts +// because types.ts must stay type-only — otherwise vite produces a shared +// runtime chunk that breaks plugin.js (which the host evaluates with +// `new Function`, not as an ES module). +// +// The `satisfies` clause catches extra / mistyped entries. The `_AssertEq` +// helper enforces the other direction: if a union gains a new member, this +// file fails to compile until the corresponding array is updated. +type _AssertEq = [T] extends [U] ? ([U] extends [T] ? true : never) : never; + +const AUTOMATION_TRIGGER_TYPES = [ 'taskCompleted', 'taskCreated', 'taskUpdated', + 'taskStarted', + 'taskStopped', 'timeBased', -]; +] as const satisfies readonly AutomationTriggerType[]; +const _exhaustiveTriggers: _AssertEq< + AutomationTriggerType, + (typeof AUTOMATION_TRIGGER_TYPES)[number] +> = true; -const VALID_CONDITION_TYPES: ConditionType[] = [ +const AUTOMATION_CONDITION_TYPES = [ 'titleContains', 'titleStartsWith', 'projectIs', 'hasTag', 'weekdayIs', -]; +] as const satisfies readonly ConditionType[]; +const _exhaustiveConditions: _AssertEq = + true; -const VALID_ACTION_TYPES: ActionType[] = [ +const AUTOMATION_ACTION_TYPES = [ 'createTask', 'deleteTask', 'addTag', + 'removeTag', 'moveToProject', 'displaySnack', 'displayDialog', 'webhook', -]; +] as const satisfies readonly ActionType[]; +const _exhaustiveActions: _AssertEq = true; export const validateRule = (rule: any): boolean => { if (typeof rule !== 'object' || rule === null) return false; - if (typeof rule.name !== 'string' || !rule.name.trim()) return false; + // Match the load-path validator (RuleRegistry): require a string name but + // not a non-empty one. The UI saves rules with empty names, so the import + // path has to accept what the export path produces. + if (typeof rule.name !== 'string') return false; if (typeof rule.isEnabled !== 'boolean') return false; // Trigger validation if (!rule.trigger || typeof rule.trigger !== 'object') return false; - if (!VALID_TRIGGER_TYPES.includes(rule.trigger.type)) return false; + if (!AUTOMATION_TRIGGER_TYPES.includes(rule.trigger.type)) return false; if (rule.trigger.type === 'timeBased' && typeof rule.trigger.value !== 'string') return false; // Conditions validation if (!Array.isArray(rule.conditions)) return false; for (const condition of rule.conditions) { if (typeof condition !== 'object' || condition === null) return false; - if (!VALID_CONDITION_TYPES.includes(condition.type)) return false; + if (!AUTOMATION_CONDITION_TYPES.includes(condition.type)) return false; if (typeof condition.value !== 'string') return false; if (condition.isRegex !== undefined && typeof condition.isRegex !== 'boolean') return false; } @@ -48,7 +72,7 @@ export const validateRule = (rule: any): boolean => { if (!Array.isArray(rule.actions)) return false; for (const action of rule.actions) { if (typeof action !== 'object' || action === null) return false; - if (!VALID_ACTION_TYPES.includes(action.type)) return false; + if (!AUTOMATION_ACTION_TYPES.includes(action.type)) return false; if (typeof action.value !== 'string') return false; } diff --git a/packages/plugin-dev/voice-reminder/plugin.js b/packages/plugin-dev/voice-reminder/plugin.js index 12ccca9484..0f3312931a 100644 --- a/packages/plugin-dev/voice-reminder/plugin.js +++ b/packages/plugin-dev/voice-reminder/plugin.js @@ -85,9 +85,9 @@ if (!window.speechSynthesis) { }, intervalMs); } - // Track current task via hook - PluginAPI.registerHook('currentTaskChange', function (task) { - _vrCurrentTask = task; + // Track current task via hook. The host emits { current, previous }. + PluginAPI.registerHook('currentTaskChange', function (payload) { + _vrCurrentTask = payload && payload.current ? payload.current : null; }); // Initial load and start diff --git a/src/app/plugins/plugin-hooks.effects.spec.ts b/src/app/plugins/plugin-hooks.effects.spec.ts index 356ce2a52c..0a0474f4af 100644 --- a/src/app/plugins/plugin-hooks.effects.spec.ts +++ b/src/app/plugins/plugin-hooks.effects.spec.ts @@ -12,7 +12,6 @@ import { selectCurrentTask, selectTaskById, } from '../features/tasks/store/task.selectors'; -import { setCurrentTask, unsetCurrentTask } from '../features/tasks/store/task.actions'; describe('PluginHooksEffects', () => { let effects: PluginHooksEffects; @@ -239,30 +238,107 @@ describe('PluginHooksEffects', () => { }); describe('onCurrentTaskChange$', () => { - it('should dispatch CURRENT_TASK_CHANGE hook with the full task object on setCurrentTask', (done) => { - store.overrideSelector(selectCurrentTask, mockTask); - actions$ = of(setCurrentTask({ id: mockTask.id })); + it('should dispatch CURRENT_TASK_CHANGE with { current, previous: null } when a task becomes active from idle', (done) => { + store.overrideSelector(selectCurrentTask, null); + actions$ = of(); - effects.onCurrentTaskChange$.subscribe(() => { + const sub = effects.onCurrentTaskChange$.subscribe(); + store.overrideSelector(selectCurrentTask, mockTask); + store.refreshState(); + + // give microtasks a tick to flush + setTimeout(() => { expect(pluginServiceMock.dispatchHook).toHaveBeenCalledWith( PluginHooks.CURRENT_TASK_CHANGE, - mockTask, + { current: mockTask, previous: null }, ); + sub.unsubscribe(); done(); - }); + }, 0); }); - it('should dispatch CURRENT_TASK_CHANGE hook with null on unsetCurrentTask', (done) => { - store.overrideSelector(selectCurrentTask, null); - actions$ = of(unsetCurrentTask()); + it('should dispatch CURRENT_TASK_CHANGE with { current: null, previous } when the active task is stopped', (done) => { + store.overrideSelector(selectCurrentTask, mockTask); + actions$ = of(); - effects.onCurrentTaskChange$.subscribe(() => { + const sub = effects.onCurrentTaskChange$.subscribe(); + store.overrideSelector(selectCurrentTask, null); + store.refreshState(); + + setTimeout(() => { expect(pluginServiceMock.dispatchHook).toHaveBeenCalledWith( PluginHooks.CURRENT_TASK_CHANGE, - null, + { current: null, previous: mockTask }, ); + sub.unsubscribe(); done(); - }); + }, 0); + }); + + it('should emit both previous and current when switching between tasks', (done) => { + const otherTask = createMockTask({ id: 'task-other', title: 'Other Task' }); + store.overrideSelector(selectCurrentTask, mockTask); + actions$ = of(); + + const sub = effects.onCurrentTaskChange$.subscribe(); + store.overrideSelector(selectCurrentTask, otherTask); + store.refreshState(); + + setTimeout(() => { + expect(pluginServiceMock.dispatchHook).toHaveBeenCalledWith( + PluginHooks.CURRENT_TASK_CHANGE, + { current: otherTask, previous: mockTask }, + ); + sub.unsubscribe(); + done(); + }, 0); + }); + + it('should not re-emit when the same task is updated in place', (done) => { + store.overrideSelector(selectCurrentTask, null); + actions$ = of(); + + const sub = effects.onCurrentTaskChange$.subscribe(); + store.overrideSelector(selectCurrentTask, mockTask); + store.refreshState(); + // Same id, different object reference (e.g. title change while running). + store.overrideSelector(selectCurrentTask, { ...mockTask, title: 'Renamed' }); + store.refreshState(); + + setTimeout(() => { + expect(pluginServiceMock.dispatchHook).toHaveBeenCalledTimes(1); + expect(pluginServiceMock.dispatchHook).toHaveBeenCalledWith( + PluginHooks.CURRENT_TASK_CHANGE, + { current: mockTask, previous: null }, + ); + sub.unsubscribe(); + done(); + }, 0); + }); + + it('should carry the latest snapshot of the running task into the stop event', (done) => { + // Simulates: start task → plugin mutates task (addTag) → stop. The stop + // payload's `previous` must reflect the post-mutation task state so a + // taskStopped handler can read the freshly-added field. + store.overrideSelector(selectCurrentTask, mockTask); + actions$ = of(); + + const sub = effects.onCurrentTaskChange$.subscribe(); + + const mutatedTask = createMockTask({ ...mockTask, tagIds: ['in-progress'] }); + store.overrideSelector(selectCurrentTask, mutatedTask); + store.refreshState(); + store.overrideSelector(selectCurrentTask, null); + store.refreshState(); + + setTimeout(() => { + expect(pluginServiceMock.dispatchHook).toHaveBeenCalledWith( + PluginHooks.CURRENT_TASK_CHANGE, + { current: null, previous: mutatedTask }, + ); + sub.unsubscribe(); + done(); + }, 0); }); }); }); diff --git a/src/app/plugins/plugin-hooks.effects.ts b/src/app/plugins/plugin-hooks.effects.ts index 43d5028e94..0c0df12bfd 100644 --- a/src/app/plugins/plugin-hooks.effects.ts +++ b/src/app/plugins/plugin-hooks.effects.ts @@ -5,6 +5,8 @@ import { distinctUntilChanged, filter, map, + pairwise, + startWith, switchMap, take, tap, @@ -26,8 +28,6 @@ import { PluginHooks } from './plugin-api.model'; import { PluginI18nService } from './plugin-i18n.service'; import { TaskSharedActions } from '../root-store/meta/task-shared.actions'; import { - setCurrentTask, - unsetCurrentTask, moveSubTask, moveSubTaskUp, moveSubTaskDown, @@ -78,13 +78,29 @@ export class PluginHooksEffects { { dispatch: false }, ); + // Observe the current-task selector directly so we catch every transition, + // including those caused by reducer paths that don't dispatch + // setCurrentTask/unsetCurrentTask (e.g. loadAllData, project delete, + // bulk task delete). pairwise gives us { current, previous } for the + // payload — plugins react to a single, authoritative source of truth + // without needing to track previous state themselves. onCurrentTaskChange$ = createEffect( () => - this.actions$.pipe( - ofType(setCurrentTask, unsetCurrentTask), - withLatestFrom(this.store.pipe(select(selectCurrentTask))), - map(([action, currentTask]) => { - this.pluginService.dispatchHook(PluginHooks.CURRENT_TASK_CHANGE, currentTask); + this.store.pipe( + select(selectCurrentTask), + startWith(null as Task | null), + pairwise(), + // Only fire on id transitions (start/stop/switch). Same-id emissions + // are dropped HERE rather than via distinctUntilChanged so that + // `previous` always carries the latest snapshot of the running task + // when it stops — including updates a plugin made to it while it + // was running (e.g. addTag → state mutation → selector re-emits). + filter(([prev, curr]) => prev?.id !== curr?.id), + tap(([previous, current]) => { + this.pluginService.dispatchHook(PluginHooks.CURRENT_TASK_CHANGE, { + current, + previous, + }); }), ), { dispatch: false },