From f1c71ec84ff5e2bbcc1cb89fa3a5fa317e9db145 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 28 Nov 2025 12:10:22 +0100 Subject: [PATCH] feat(automationPlugin): improve weekday condition logic and fix memory management --- packages/plugin-api/src/types.ts | 13 +++- .../src/core/automation-manager.ts | 14 ++++ .../automations/src/core/conditions.ts | 23 ++++++ .../automations/src/core/rule-registry.ts | 21 +++-- packages/plugin-dev/automations/src/plugin.ts | 78 ++----------------- packages/plugin-dev/automations/src/types.ts | 4 +- src/app/plugins/plugin-bridge.service.ts | 13 ++-- src/app/plugins/plugin-hooks.effects.ts | 9 ++- 8 files changed, 84 insertions(+), 91 deletions(-) diff --git a/packages/plugin-api/src/types.ts b/packages/plugin-api/src/types.ts index fad87a136..ed765b0e4 100644 --- a/packages/plugin-api/src/types.ts +++ b/packages/plugin-api/src/types.ts @@ -9,6 +9,7 @@ export interface PluginMenuEntryCfg { } export enum PluginHooks { + TASK_CREATED = 'taskCreated', TASK_COMPLETE = 'taskComplete', TASK_UPDATE = 'taskUpdate', TASK_DELETE = 'taskDelete', @@ -30,7 +31,7 @@ export interface PluginBaseCfg { isDev: boolean; lang?: { code: string; - [key: string]: any; + [key: string]: unknown; }; } @@ -118,6 +119,11 @@ export interface PluginManifest { } // Hook payload types +export interface TaskCreatedPayload { + taskId: string; + task: Task; +} + export interface TaskCompletePayload { taskId: string; task: Task; @@ -145,7 +151,7 @@ export interface FinishDayPayload { export interface LanguageChangePayload { code: string; - [key: string]: any; + [key: string]: unknown; } export interface PersistedDataUpdatePayload { @@ -154,7 +160,7 @@ export interface PersistedDataUpdatePayload { export interface ActionPayload { action: string; - payload?: any; + payload?: unknown; } export interface AnyTaskUpdatePayload { @@ -173,6 +179,7 @@ export interface ProjectListUpdatePayload { // Map hook types to their payload types export interface HookPayloadMap { + [PluginHooks.TASK_CREATED]: TaskCreatedPayload; [PluginHooks.TASK_COMPLETE]: TaskCompletePayload; [PluginHooks.TASK_UPDATE]: TaskUpdatePayload; [PluginHooks.TASK_DELETE]: TaskDeletePayload; diff --git a/packages/plugin-dev/automations/src/core/automation-manager.ts b/packages/plugin-dev/automations/src/core/automation-manager.ts index e81995f9e..26ba2b158 100644 --- a/packages/plugin-dev/automations/src/core/automation-manager.ts +++ b/packages/plugin-dev/automations/src/core/automation-manager.ts @@ -52,6 +52,7 @@ export class AutomationManager { globalRegistry.registerCondition(Conditions.ConditionTitleContains); globalRegistry.registerCondition(Conditions.ConditionProjectIs); globalRegistry.registerCondition(Conditions.ConditionHasTag); + globalRegistry.registerCondition(Conditions.ConditionWeekdayIs); // Actions globalRegistry.registerAction(Actions.ActionCreateTask); @@ -77,6 +78,10 @@ export class AutomationManager { private async checkTimeBasedRules() { try { const rules = await this.ruleRegistry.getEnabledRules(); + + // Cleanup execution times for deleted rules to prevent memory leaks + this.syncExecutionTimes(rules.map((r) => r.id)); + const now = new Date(); const currentHours = now.getHours(); const currentMinutes = now.getMinutes(); @@ -121,6 +126,15 @@ export class AutomationManager { } } + private syncExecutionTimes(activeRuleIds: string[]) { + const activeSet = new Set(activeRuleIds); + for (const id of this.lastExecutionTimes.keys()) { + if (!activeSet.has(id)) { + this.lastExecutionTimes.delete(id); + } + } + } + async onTaskEvent(event: TaskEvent) { if (!event.task) { this.plugin.log.warn(`[Automation] Event ${event.type} received without task data`); diff --git a/packages/plugin-dev/automations/src/core/conditions.ts b/packages/plugin-dev/automations/src/core/conditions.ts index 982a27ad8..df6355c75 100644 --- a/packages/plugin-dev/automations/src/core/conditions.ts +++ b/packages/plugin-dev/automations/src/core/conditions.ts @@ -30,3 +30,26 @@ export const ConditionHasTag: IAutomationCondition = { return tag ? event.task.tagIds.includes(tag.id) : false; }, }; + +export const ConditionWeekdayIs: IAutomationCondition = { + id: 'weekdayIs', + name: 'Weekday is', + description: 'Checks if the current day is one of the specified days (e.g. "Monday", "Mon,Tue")', + check: async (ctx, event, value) => { + if (!value) return false; + const days = ['sunday', 'monday', 'tuesday', 'wednesday', 'thursday', 'friday', 'saturday']; + const todayIndex = new Date().getDay(); + const todayName = days[todayIndex]; + + const allowedDays = value + .toLowerCase() + .split(',') + .map((d) => d.trim()); + + // Use exact match for full names or 3-letter abbreviations + return allowedDays.some((day) => { + if (day.length < 3) return false; // Prevent short ambiguous matches + return day === todayName || (todayName.startsWith(day) && day.length === 3); + }); + }, +}; diff --git a/packages/plugin-dev/automations/src/core/rule-registry.ts b/packages/plugin-dev/automations/src/core/rule-registry.ts index 637b91299..0904b9386 100644 --- a/packages/plugin-dev/automations/src/core/rule-registry.ts +++ b/packages/plugin-dev/automations/src/core/rule-registry.ts @@ -63,7 +63,7 @@ export class RuleRegistry { } const validTriggers = new Set(['taskCompleted', 'taskCreated', 'taskUpdated', 'timeBased']); - const validConditions = new Set(['titleContains', 'projectIs', 'hasTag']); + const validConditions = new Set(['titleContains', 'projectIs', 'hasTag', 'weekdayIs']); const validActions = new Set([ 'createTask', 'addTag', @@ -116,13 +116,18 @@ export class RuleRegistry { } private async saveRules() { - this.saveQueue = this.saveQueue.then(async () => { - try { - await this.plugin.persistDataSynced(JSON.stringify(this.rules)); - } catch (e) { - this.plugin.log.error('Failed to save rules', e); - } - }); + this.saveQueue = this.saveQueue + .then(async () => { + try { + await this.plugin.persistDataSynced(JSON.stringify(this.rules)); + } catch (e) { + this.plugin.log.error('Failed to save rules', e); + } + }) + .catch(() => { + // Catch any errors from the promise chain itself to prevent blocking future saves + this.plugin.log.error('Critical error in save queue'); + }); await this.saveQueue; } diff --git a/packages/plugin-dev/automations/src/plugin.ts b/packages/plugin-dev/automations/src/plugin.ts index dd3177a41..ed806a42b 100644 --- a/packages/plugin-dev/automations/src/plugin.ts +++ b/packages/plugin-dev/automations/src/plugin.ts @@ -3,6 +3,7 @@ import { PluginAPI, TaskCompletePayload, TaskUpdatePayload, + TaskCreatedPayload, } from '@super-productivity/plugin-api'; import type { PluginHooks } from '@super-productivity/plugin-api'; @@ -10,52 +11,23 @@ declare const plugin: PluginAPI; import { AutomationManager } from './core/automation-manager'; import { globalRegistry } from './core/registry'; -import { TASK_SHARED_ADD_TASK_ACTION } from './core/definitions'; // Plugin initialization plugin.log.info('Automation plugin initialized'); const automationManager = new AutomationManager(plugin); -// Deduplicate creation handling across multiple hooks (taskUpdate + anyTaskUpdate) -// because there is no dedicated taskCreated hook in the Plugin API. -const handledCreations = new Set(); -const markCreationHandled = (taskId?: string) => { - if (!taskId) { - return false; - } - if (handledCreations.has(taskId)) { - return true; - } - handledCreations.add(taskId); - // Avoid unbounded growth while still deduplicating bursts - setTimeout(() => handledCreations.delete(taskId), 5000); - return false; -}; - -const handleTaskCreated = (task: TaskUpdatePayload['task']) => { - if (!task) { - plugin.log.warn('Received taskCreated event without task data'); - return; - } - if (markCreationHandled(task.id)) { +// Hook into task creation +plugin.registerHook('taskCreated' as any, (payload: TaskCreatedPayload) => { + if (!payload.task) { + plugin.log.warn('Received taskCreated hook without task data'); return; } automationManager.onTaskEvent({ type: 'taskCreated', - task, + task: payload.task, }); -}; - -// Be defensive about action names from different sources/import paths. -const isAddTaskAction = (action: string | undefined) => { - if (!action) return false; - return ( - action === TASK_SHARED_ADD_TASK_ACTION || - action.toLowerCase().includes('addtask') || - action.toLowerCase().includes('add task') - ); -}; +}); // Hook into task completion plugin.registerHook('taskComplete' as any, (payload: TaskCompletePayload) => { @@ -75,12 +47,7 @@ plugin.registerHook('taskUpdate' as any, (payload: TaskUpdatePayload) => { plugin.log.warn('Received taskUpdate hook without task data'); return; } - const isCreationEvent = !payload.changes || Object.keys(payload.changes).length === 0; - if (isCreationEvent) { - plugin.log.info('[Automation] Detected creation via taskUpdate hook'); - handleTaskCreated(payload.task); - return; - } + // We no longer need to heuristically detect creation here automationManager.onTaskEvent({ type: 'taskUpdated', task: payload.task, @@ -88,35 +55,6 @@ plugin.registerHook('taskUpdate' as any, (payload: TaskUpdatePayload) => { }); }); -// Hook into task creation? -// There is no explicit TASK_CREATE hook in PluginHooks enum from types.ts (Step 23). -// We might need to infer it or use ANY_TASK_UPDATE or check if there is a missing hook. -// Looking at types.ts: TASK_COMPLETE, TASK_UPDATE, TASK_DELETE, CURRENT_TASK_CHANGE, FINISH_DAY, ... -// Wait, is there no TASK_CREATE? -// Let's check if ANY_TASK_UPDATE covers creation. -// Or maybe we need to request a new hook. -// For now, let's assume we can't easily detect creation unless we monitor ANY_TASK_UPDATE and check if it's new? -// Actually, `addTask` returns a promise with ID. -// But if the user creates a task via UI, the plugin needs to know. -// Let's check if `TASK_UPDATE` is fired on creation? Usually creation is separate. -// If TASK_CREATE is missing, I should note it. -// However, the user request explicitly asked for "Task created" trigger. -// I will use `ANY_TASK_UPDATE` and check if I can detect creation, or just leave a comment. -// Actually, let's look at `PluginHooks` again. -// Step 23: TASK_COMPLETE, TASK_UPDATE, TASK_DELETE, CURRENT_TASK_CHANGE, FINISH_DAY, LANGUAGE_CHANGE, PERSISTED_DATA_UPDATE, ACTION, ANY_TASK_UPDATE, PROJECT_LIST_UPDATE. -// No TASK_CREATE. -// Maybe `ANY_TASK_UPDATE` with a specific action? -// `AnyTaskUpdatePayload` has `action`, `taskId`, `task`, `changes`. -// If `action` is 'ADD', that might be it. - -plugin.registerHook('anyTaskUpdate' as any, (payload: AnyTaskUpdatePayload) => { - plugin.log.info(`[Automation] anyTaskUpdate action: ${payload.action}`); - - if (isAddTaskAction(payload.action) && payload.task) { - handleTaskCreated(payload.task); - } -}); - // Register UI commands if (plugin.onMessage) { plugin.onMessage(async (message: any) => { diff --git a/packages/plugin-dev/automations/src/types.ts b/packages/plugin-dev/automations/src/types.ts index 5c3cd6fcb..3af16cf90 100644 --- a/packages/plugin-dev/automations/src/types.ts +++ b/packages/plugin-dev/automations/src/types.ts @@ -10,10 +10,10 @@ export interface AutomationTrigger { export interface TaskEvent { type: AutomationTriggerType; task?: Task; - previousTaskState?: Task; // only used for "updated" + previousTaskState?: unknown; // only used for "updated" } -export type ConditionType = 'titleContains' | 'projectIs' | 'hasTag'; +export type ConditionType = 'titleContains' | 'projectIs' | 'hasTag' | 'weekdayIs'; export interface Condition { type: ConditionType; diff --git a/src/app/plugins/plugin-bridge.service.ts b/src/app/plugins/plugin-bridge.service.ts index 451cfda49..6ebae3d95 100644 --- a/src/app/plugins/plugin-bridge.service.ts +++ b/src/app/plugins/plugin-bridge.service.ts @@ -15,6 +15,7 @@ import { PluginNodeScriptResult, PluginShortcutCfg, PluginSidePanelBtnCfg, + Task, } from './plugin-api.model'; import { @@ -363,10 +364,10 @@ export class PluginBridgeService implements OnDestroy { taskData.parentId, ); - // Check if this is a subtask + let createdTask: Task; if (taskData.parentId) { // For subtasks, we need to use the addSubTask action to properly update parent - const task = this._taskService.createNewTaskWithDefaults({ + const newTask = this._taskService.createNewTaskWithDefaults({ title: taskData.title, additional: { notes: taskData.notes || '', @@ -380,16 +381,18 @@ export class PluginBridgeService implements OnDestroy { // Dispatch the addSubTask action which properly updates parent's subTaskIds this._store.dispatch( addSubTask({ - task, + task: newTask, parentId: taskData.parentId, }), ); + createdTask = newTask; PluginLog.log('PluginBridge: Subtask added successfully', { - taskId: task.id, + taskId: createdTask.id, taskData, }); - return task.id; + + return createdTask.id; } else { // For main tasks, use the regular add method const additional: Partial = { diff --git a/src/app/plugins/plugin-hooks.effects.ts b/src/app/plugins/plugin-hooks.effects.ts index 8ff9fec40..e31b15297 100644 --- a/src/app/plugins/plugin-hooks.effects.ts +++ b/src/app/plugins/plugin-hooks.effects.ts @@ -22,6 +22,7 @@ import { moveSubTaskDown, moveSubTaskToTop, moveSubTaskToBottom, + addSubTask, // Added } from '../features/tasks/store/task.actions'; import * as projectActions from '../features/project/store/project.actions'; import { updateProject } from '../features/project/store/project.actions'; @@ -131,7 +132,7 @@ export class PluginHooksEffects { taskAdd$ = createEffect( () => this.actions$.pipe( - ofType(TaskSharedActions.addTask), + ofType(TaskSharedActions.addTask, addSubTask), switchMap((action) => this.store.pipe( select(selectTaskById, { id: action.task.id }), @@ -139,11 +140,13 @@ export class PluginHooksEffects { filter((task) => !!task), tap((task: Task | undefined) => { if (task) { - this.pluginService.dispatchHook(PluginHooks.TASK_UPDATE, { + this.pluginService.dispatchHook(PluginHooks.TASK_CREATED, { taskId: task.id, task, - changes: {}, // Initial add, no changes diff }); + // Also dispatch legacy update for backward compatibility if needed, + // but generally TASK_CREATED should be preferred. + // Leaving TASK_UPDATE out as 'taskCreated' is the specific hook now. } }), map(() => EMPTY),