From aa75e6f7ee68e97d9987eb7914918920d201f771 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Mon, 13 Jul 2026 19:12:13 +0200 Subject: [PATCH] fix(plugins): prevent automation rule data loss (#8972) Keep initialization read-only, preserve unsupported synced entries across explicit edits, and reject malformed runtime payloads. Stage mutations transactionally so failed persistence cannot leave executable rules that disappear after restart. --- .../src/core/rule-registry.spec.ts | 406 +++++++++++++++++- .../automations/src/core/rule-registry.ts | 375 ++++++++++------ 2 files changed, 629 insertions(+), 152 deletions(-) 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 c84e5a7f6b..d656690a38 100644 --- a/packages/plugin-dev/automations/src/core/rule-registry.spec.ts +++ b/packages/plugin-dev/automations/src/core/rule-registry.spec.ts @@ -6,11 +6,15 @@ import { AutomationRule } from '../types'; describe('RuleRegistry', () => { let registry: RuleRegistry; let mockPlugin: PluginAPI; + let loadSyncedDataMock: ReturnType>; + let persistDataSyncedMock: ReturnType>; beforeEach(() => { + loadSyncedDataMock = vi.fn().mockResolvedValue(null); + persistDataSyncedMock = vi.fn().mockResolvedValue(); mockPlugin = { - loadSyncedData: vi.fn().mockResolvedValue(null), - persistDataSynced: vi.fn(), + loadSyncedData: loadSyncedDataMock, + persistDataSynced: persistDataSyncedMock, log: { info: vi.fn(), warn: vi.fn(), @@ -19,15 +23,111 @@ describe('RuleRegistry', () => { } as unknown as PluginAPI; }); - it('should load empty rules initially', async () => { + it('loads empty rules without persisting defaults when synced data is missing', async () => { registry = new RuleRegistry(mockPlugin); - // Wait for async load in constructor (implementation detail: constructor is sync but calls async load) - // Ideally RuleRegistry should expose an init method, or we wait. - // Since loadRules is called in constructor without await, we need to wait for promises. - await new Promise(process.nextTick); expect(await registry.getRules()).toEqual([]); expect(registry.getInitializationError()).toBeNull(); + expect(persistDataSyncedMock).not.toHaveBeenCalled(); + }); + + it('does not persist empty rules when loading synced data fails', async () => { + const loadError = new Error('Temporary load failure'); + loadSyncedDataMock.mockRejectedValue(loadError); + + registry = new RuleRegistry(mockPlugin); + + expect(await registry.getRules()).toEqual([]); + expect(registry.getInitializationError()).toBe(loadError); + expect(persistDataSyncedMock).not.toHaveBeenCalled(); + }); + + it('does not persist empty rules when synced JSON is corrupted', async () => { + loadSyncedDataMock.mockResolvedValue('{not valid json'); + + registry = new RuleRegistry(mockPlugin); + + expect(await registry.getRules()).toEqual([]); + expect(registry.getInitializationError()?.message).toBe('Corrupted JSON in automation rules'); + expect(persistDataSyncedMock).not.toHaveBeenCalled(); + }); + + it('does not persist empty rules when synced JSON has an invalid root value', async () => { + loadSyncedDataMock.mockResolvedValue(JSON.stringify({ rules: [] })); + + registry = new RuleRegistry(mockPlugin); + + expect(await registry.getRules()).toEqual([]); + expect(registry.getInitializationError()?.message).toBe( + 'Persisted automation rules are invalid', + ); + expect(persistDataSyncedMock).not.toHaveBeenCalled(); + }); + + it('loads known rules and preserves unsupported or malformed entries on explicit edits', async () => { + const knownRule: AutomationRule = { + id: 'known', + name: 'Known rule', + isEnabled: true, + trigger: { type: 'taskCompleted' }, + conditions: [], + actions: [], + }; + const unsupportedRules = [ + { + id: 'future-trigger', + name: 'Future trigger', + isEnabled: true, + trigger: { type: 'projectArchived' }, + conditions: [], + actions: [], + }, + { + id: 'future-condition', + name: 'Future condition', + isEnabled: true, + trigger: { type: 'taskUpdated' }, + conditions: [{ type: 'projectHasLabel', value: 'important' }], + actions: [], + }, + { + id: 'future-action', + name: 'Future action', + isEnabled: true, + trigger: { type: 'taskCreated' }, + conditions: [], + actions: [{ type: 'archiveTask', value: '' }], + }, + ]; + const malformedEntry = { id: 'malformed', name: 42 }; + const persistedEntries = [ + unsupportedRules[0], + knownRule, + malformedEntry, + unsupportedRules[1], + unsupportedRules[2], + ]; + loadSyncedDataMock.mockResolvedValue(JSON.stringify(persistedEntries)); + registry = new RuleRegistry(mockPlugin); + + expect(await registry.getRules()).toEqual([knownRule]); + expect(persistDataSyncedMock).not.toHaveBeenCalled(); + + const addedRule: AutomationRule = { + id: 'added', + name: 'Added rule', + isEnabled: true, + trigger: { type: 'taskCreated' }, + conditions: [], + actions: [], + }; + await registry.addOrUpdateRule(addedRule); + + expect(persistDataSyncedMock).toHaveBeenCalledTimes(1); + expect(JSON.parse(persistDataSyncedMock.mock.calls[0][0])).toEqual([ + ...persistedEntries, + addedRule, + ]); }); it('should load existing rules', async () => { @@ -41,7 +141,7 @@ describe('RuleRegistry', () => { actions: [], }, ]; - (mockPlugin.loadSyncedData as any).mockResolvedValue(JSON.stringify(rules)); + loadSyncedDataMock.mockResolvedValue(JSON.stringify(rules)); registry = new RuleRegistry(mockPlugin); await new Promise(process.nextTick); @@ -60,7 +160,7 @@ describe('RuleRegistry', () => { actions: [], }, ]; - (mockPlugin.loadSyncedData as any).mockResolvedValue(JSON.stringify(rules)); + loadSyncedDataMock.mockResolvedValue(JSON.stringify(rules)); registry = new RuleRegistry(mockPlugin); await new Promise(process.nextTick); @@ -87,7 +187,7 @@ describe('RuleRegistry', () => { actions: [{ type: 'removeTag', value: 'in-progress' }], }, ]; - (mockPlugin.loadSyncedData as any).mockResolvedValue(JSON.stringify(rules)); + loadSyncedDataMock.mockResolvedValue(JSON.stringify(rules)); registry = new RuleRegistry(mockPlugin); await new Promise(process.nextTick); @@ -106,7 +206,7 @@ describe('RuleRegistry', () => { actions: [{ type: 'deleteTask', value: '' }], }, ]; - (mockPlugin.loadSyncedData as any).mockResolvedValue(JSON.stringify(rules)); + loadSyncedDataMock.mockResolvedValue(JSON.stringify(rules)); registry = new RuleRegistry(mockPlugin); await new Promise(process.nextTick); @@ -134,8 +234,16 @@ describe('RuleRegistry', () => { }); it('addRules persists every rule with a single persistDataSynced call', async () => { + const unsupportedRule = { + id: 'future-action', + name: 'Future action', + isEnabled: true, + trigger: { type: 'taskCreated' }, + conditions: [], + actions: [{ type: 'archiveTask', value: '' }], + }; + loadSyncedDataMock.mockResolvedValue(JSON.stringify([unsupportedRule])); registry = new RuleRegistry(mockPlugin); - await new Promise(process.nextTick); const rules: AutomationRule[] = [ { @@ -156,27 +264,186 @@ describe('RuleRegistry', () => { }, ]; - // One persistDataSynced call (constructor init) before the batch. - expect((mockPlugin.persistDataSynced as any).mock.calls.length).toBe(1); + expect(persistDataSyncedMock).not.toHaveBeenCalled(); 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); + // Exactly one persist for the whole batch — no rate-limit risk. + expect(persistDataSyncedMock).toHaveBeenCalledTimes(1); expect(await registry.getRules()).toEqual(rules); + expect(JSON.parse(persistDataSyncedMock.mock.calls[0][0])).toEqual([unsupportedRule, ...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; + const before = persistDataSyncedMock.mock.calls.length; await registry.addRules([]); - expect((mockPlugin.persistDataSynced as any).mock.calls.length).toBe(before); + expect(persistDataSyncedMock.mock.calls.length).toBe(before); }); - it('should update existing rule', async () => { + it('rejects a malformed batch atomically and accepts a later valid save', async () => { + const validRule: AutomationRule = { + id: 'valid', + name: 'Valid rule', + isEnabled: true, + trigger: { type: 'taskCreated' }, + conditions: [], + actions: [], + }; + const laterRule: AutomationRule = { + ...validRule, + id: 'later', + name: 'Later rule', + }; + registry = new RuleRegistry(mockPlugin); + + await expect( + registry.addRules([validRule, null] as unknown as AutomationRule[]), + ).rejects.toThrow('Invalid automation rules'); + + expect(await registry.getRules()).toEqual([]); + expect(persistDataSyncedMock).not.toHaveBeenCalled(); + + await registry.addOrUpdateRule(laterRule); + + expect(await registry.getRules()).toEqual([laterRule]); + expect(persistDataSyncedMock).toHaveBeenCalledWith(JSON.stringify([laterRule])); + }); + + it('rolls back a failed persistence and keeps the queue usable', async () => { + const failedRule: AutomationRule = { + id: 'failed', + name: 'Failed rule', + isEnabled: true, + trigger: { type: 'taskCreated' }, + conditions: [], + actions: [], + }; + const savedRule: AutomationRule = { + ...failedRule, + id: 'saved', + name: 'Saved rule', + }; + persistDataSyncedMock + .mockRejectedValueOnce(new Error('Temporary persistence failure')) + .mockResolvedValueOnce(); + registry = new RuleRegistry(mockPlugin); + + await expect(registry.addOrUpdateRule(failedRule)).rejects.toThrow( + 'Temporary persistence failure', + ); + expect(await registry.getRules()).toEqual([]); + + await registry.addOrUpdateRule(savedRule); + + expect(await registry.getRules()).toEqual([savedRule]); + expect(persistDataSyncedMock.mock.calls.map(([data]) => JSON.parse(data))).toEqual([ + [failedRule], + [savedRule], + ]); + }); + + it('persists concurrent mutations in invocation order', async () => { + const firstRule: AutomationRule = { + id: 'first', + name: 'First rule', + isEnabled: true, + trigger: { type: 'taskCreated' }, + conditions: [], + actions: [], + }; + const secondRule: AutomationRule = { + ...firstRule, + id: 'second', + name: 'Second rule', + }; + let resolveFirstPersist: () => void = () => undefined; + persistDataSyncedMock + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirstPersist = resolve; + }), + ) + .mockResolvedValueOnce(); + registry = new RuleRegistry(mockPlugin); + + const firstMutation = registry.addOrUpdateRule(firstRule); + await vi.waitFor(() => expect(persistDataSyncedMock).toHaveBeenCalledTimes(1)); + const secondMutation = registry.addOrUpdateRule(secondRule); + + expect(persistDataSyncedMock).toHaveBeenCalledTimes(1); + resolveFirstPersist(); + await Promise.all([firstMutation, secondMutation]); + + expect(persistDataSyncedMock.mock.calls.map(([data]) => JSON.parse(data))).toEqual([ + [firstRule], + [firstRule, secondRule], + ]); + }); + + it('blocks mutations after an initialization error to avoid overwriting synced data', async () => { + const rule: AutomationRule = { + id: 'new', + name: 'New rule', + isEnabled: true, + trigger: { type: 'taskCreated' }, + conditions: [], + actions: [], + }; + loadSyncedDataMock.mockRejectedValue(new Error('Temporary load failure')); + registry = new RuleRegistry(mockPlugin); + + await expect(registry.addOrUpdateRule(rule)).rejects.toThrow( + 'Automation rules failed to initialize', + ); + + expect(await registry.getRules()).toEqual([]); + expect(persistDataSyncedMock).not.toHaveBeenCalled(); + }); + + it('canonicalizes valid mutation payloads before storing or persisting them', async () => { + const expectedRule: AutomationRule = { + id: 'safe', + name: 'Safe rule', + isEnabled: true, + trigger: { type: 'taskUpdated' }, + conditions: [{ type: 'titleContains', value: 'test' }], + actions: [{ type: 'displaySnack', value: 'Matched' }], + }; + const messagePayload: Record = { ...expectedRule }; + messagePayload.unsupported = messagePayload; + registry = new RuleRegistry(mockPlugin); + + await registry.addOrUpdateRule(messagePayload as unknown as AutomationRule); + + expect(await registry.getRules()).toEqual([expectedRule]); + expect(persistDataSyncedMock).toHaveBeenCalledWith(JSON.stringify([expectedRule])); + }); + + it('rejects unsupported rule discriminators received through mutation messages', async () => { + const unsupportedRule = { + id: 'future', + name: 'Future rule', + isEnabled: true, + trigger: { type: 'projectArchived' }, + conditions: [], + actions: [], + }; + registry = new RuleRegistry(mockPlugin); + + await expect( + registry.addOrUpdateRule(unsupportedRule as unknown as AutomationRule), + ).rejects.toThrow('Invalid automation rule'); + + expect(await registry.getRules()).toEqual([]); + expect(persistDataSyncedMock).not.toHaveBeenCalled(); + }); + + it('rejects invalid toggle payloads without mutating or persisting rules', async () => { const rule: AutomationRule = { id: 'r1', name: 'Rule 1', @@ -185,7 +452,59 @@ describe('RuleRegistry', () => { conditions: [], actions: [], }; - (mockPlugin.loadSyncedData as any).mockResolvedValue(JSON.stringify([rule])); + loadSyncedDataMock.mockResolvedValue(JSON.stringify([rule])); + registry = new RuleRegistry(mockPlugin); + + await expect(registry.toggleRuleStatus(rule.id, 'false' as unknown as boolean)).rejects.toThrow( + 'Invalid automation rule status', + ); + + expect(await registry.getRules()).toEqual([rule]); + expect(persistDataSyncedMock).not.toHaveBeenCalled(); + }); + + it('keeps time-based rules without a value opaque and unexecutable', async () => { + const incompleteTimeRule = { + id: 'incomplete-time', + name: 'Incomplete time rule', + isEnabled: true, + trigger: { type: 'timeBased' }, + conditions: [], + actions: [], + }; + const validRule: AutomationRule = { + id: 'valid', + name: 'Valid rule', + isEnabled: true, + trigger: { type: 'taskCreated' }, + conditions: [], + actions: [], + }; + loadSyncedDataMock.mockResolvedValue(JSON.stringify([incompleteTimeRule, validRule])); + registry = new RuleRegistry(mockPlugin); + + expect(await registry.getRules()).toEqual([validRule]); + expect(persistDataSyncedMock).not.toHaveBeenCalled(); + }); + + it('should update existing rule', async () => { + const unsupportedRule = { + id: 'future-trigger', + name: 'Future trigger', + isEnabled: true, + trigger: { type: 'projectArchived' }, + conditions: [], + actions: [], + }; + const rule: AutomationRule = { + id: 'r1', + name: 'Rule 1', + isEnabled: true, + trigger: { type: 'taskCreated' }, + conditions: [], + actions: [], + }; + loadSyncedDataMock.mockResolvedValue(JSON.stringify([unsupportedRule, rule])); registry = new RuleRegistry(mockPlugin); await new Promise(process.nextTick); @@ -195,9 +514,21 @@ describe('RuleRegistry', () => { const rules = await registry.getRules(); expect(rules).toHaveLength(1); expect(rules[0].name).toBe('Updated Rule'); + expect(JSON.parse(persistDataSyncedMock.mock.calls[0][0])).toEqual([ + unsupportedRule, + updatedRule, + ]); }); it('should delete rule', async () => { + const unsupportedRule = { + id: 'future-action', + name: 'Future action', + isEnabled: true, + trigger: { type: 'taskCreated' }, + conditions: [], + actions: [{ type: 'archiveTask', value: '' }], + }; const rule: AutomationRule = { id: 'r1', name: 'Rule 1', @@ -206,13 +537,42 @@ describe('RuleRegistry', () => { conditions: [], actions: [], }; - (mockPlugin.loadSyncedData as any).mockResolvedValue(JSON.stringify([rule])); + loadSyncedDataMock.mockResolvedValue(JSON.stringify([unsupportedRule, rule])); registry = new RuleRegistry(mockPlugin); await new Promise(process.nextTick); await registry.deleteRule('r1'); expect(await registry.getRules()).toHaveLength(0); - expect(mockPlugin.persistDataSynced).toHaveBeenCalledWith(JSON.stringify([])); + expect(mockPlugin.persistDataSynced).toHaveBeenCalledWith(JSON.stringify([unsupportedRule])); + }); + + it('should toggle a known rule without deleting unsupported rules', async () => { + const unsupportedRule = { + id: 'future-condition', + name: 'Future condition', + isEnabled: true, + trigger: { type: 'taskUpdated' }, + conditions: [{ type: 'projectHasLabel', value: 'important' }], + actions: [], + }; + const rule: AutomationRule = { + id: 'r1', + name: 'Rule 1', + isEnabled: true, + trigger: { type: 'taskCreated' }, + conditions: [], + actions: [], + }; + loadSyncedDataMock.mockResolvedValue(JSON.stringify([unsupportedRule, rule])); + registry = new RuleRegistry(mockPlugin); + + await registry.toggleRuleStatus(rule.id, false); + + expect(await registry.getRules()).toEqual([{ ...rule, isEnabled: false }]); + expect(JSON.parse(persistDataSyncedMock.mock.calls[0][0])).toEqual([ + unsupportedRule, + { ...rule, isEnabled: false }, + ]); }); it('should get only enabled rules', async () => { @@ -234,7 +594,7 @@ describe('RuleRegistry', () => { actions: [], }, ]; - (mockPlugin.loadSyncedData as any).mockResolvedValue(JSON.stringify(rules)); + loadSyncedDataMock.mockResolvedValue(JSON.stringify(rules)); registry = new RuleRegistry(mockPlugin); await new Promise(process.nextTick); diff --git a/packages/plugin-dev/automations/src/core/rule-registry.ts b/packages/plugin-dev/automations/src/core/rule-registry.ts index a6d7434563..d70fa14eb9 100644 --- a/packages/plugin-dev/automations/src/core/rule-registry.ts +++ b/packages/plugin-dev/automations/src/core/rule-registry.ts @@ -47,8 +47,115 @@ const AUTOMATION_ACTION_TYPES = [ ] as const satisfies readonly ActionType[]; const _exhaustiveActions: _AssertEq = true; +const validTriggers = new Set(AUTOMATION_TRIGGER_TYPES); +const validConditions = new Set(AUTOMATION_CONDITION_TYPES); +const validActions = new Set(AUTOMATION_ACTION_TYPES); + +const isRecord = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value); + +const isAutomationTriggerType = (value: unknown): value is AutomationTriggerType => + typeof value === 'string' && validTriggers.has(value); + +const isConditionType = (value: unknown): value is ConditionType => + typeof value === 'string' && validConditions.has(value); + +const isActionType = (value: unknown): value is ActionType => + typeof value === 'string' && validActions.has(value); + +const toAutomationTrigger = (value: unknown): AutomationRule['trigger'] | null => { + if (!isRecord(value) || !isAutomationTriggerType(value.type)) { + return null; + } + + if (value.type === 'timeBased') { + return typeof value.value === 'string' ? { type: value.type, value: value.value } : null; + } + + if (value.value === undefined) { + return { type: value.type }; + } + + return typeof value.value === 'string' ? { type: value.type, value: value.value } : null; +}; + +const toCondition = (value: unknown): AutomationRule['conditions'][number] | null => { + if ( + !isRecord(value) || + !isConditionType(value.type) || + typeof value.value !== 'string' || + (value.isRegex !== undefined && typeof value.isRegex !== 'boolean') + ) { + return null; + } + + return value.isRegex === undefined + ? { type: value.type, value: value.value } + : { type: value.type, value: value.value, isRegex: value.isRegex }; +}; + +const toAction = (value: unknown): AutomationRule['actions'][number] | null => { + if (!isRecord(value) || !isActionType(value.type) || typeof value.value !== 'string') { + return null; + } + + return { type: value.type, value: value.value }; +}; + +const toAutomationRule = (value: unknown): AutomationRule | null => { + if ( + !isRecord(value) || + typeof value.id !== 'string' || + typeof value.name !== 'string' || + typeof value.isEnabled !== 'boolean' || + !Array.isArray(value.conditions) || + !Array.isArray(value.actions) + ) { + return null; + } + + const trigger = toAutomationTrigger(value.trigger); + if (!trigger) { + return null; + } + + const conditions: AutomationRule['conditions'] = []; + for (const conditionValue of value.conditions) { + const condition = toCondition(conditionValue); + if (!condition) { + return null; + } + conditions.push(condition); + } + + const actions: AutomationRule['actions'] = []; + for (const actionValue of value.actions) { + const action = toAction(actionValue); + if (!action) { + return null; + } + actions.push(action); + } + + return { + id: value.id, + name: value.name, + isEnabled: value.isEnabled, + trigger, + conditions, + actions, + }; +}; + +const isAutomationRule = (value: unknown): value is AutomationRule => + toAutomationRule(value) !== null; + export class RuleRegistry { private rules: AutomationRule[] = []; + // Preserve the complete loaded array as opaque JSON. Older clients only expose + // known rules for execution, but explicit edits must round-trip newer or malformed + // entries instead of silently deleting them from every synced device. + private persistedRules: unknown[] = []; private plugin: PluginAPI; private initPromise: Promise; private saveQueue: Promise = Promise.resolve(); @@ -63,115 +170,91 @@ export class RuleRegistry { private async loadRules() { try { const data = await this.plugin.loadSyncedData(); - if (data) { - let parsed; - try { - parsed = JSON.parse(data); - } catch (e) { - this.initError = new Error('Corrupted JSON in automation rules'); - this.plugin.log.warn('Corrupted JSON in automation rules, resetting.'); - // We don't return here, we let it fall through to initDefaultRules to reset - } + if (data === null) { + this.plugin.log.info('RuleRegistry: No rules found, using defaults.'); + this.initDefaultRules(); + return; + } - if (parsed) { - const validated = this.validateRules(parsed); - if (validated) { - this.rules = validated; - this.plugin.log.info(`RuleRegistry: loaded ${this.rules.length} rules.`); - return; - } - this.initError = new Error('Persisted automation rules are invalid'); - this.plugin.log.warn('Persisted automation rules are invalid, resetting to defaults.'); + let parsed: unknown; + try { + parsed = JSON.parse(data) as unknown; + } catch { + this.initError = new Error('Corrupted JSON in automation rules'); + this.plugin.log.warn('Corrupted JSON in automation rules; no rules loaded.'); + this.initDefaultRules(); + return; + } + + if (!Array.isArray(parsed)) { + this.initError = new Error('Persisted automation rules are invalid'); + this.plugin.log.warn('Persisted automation rules are not an array; no rules loaded.'); + this.initDefaultRules(); + return; + } + + this.persistedRules = parsed; + this.rules = []; + for (const entry of parsed) { + const rule = toAutomationRule(entry); + if (rule) { + this.rules.push(rule); } } - this.plugin.log.info('RuleRegistry: No valid rules found, initializing defaults.'); - this.initDefaultRules(); - await this.saveRules(); + const preservedCount = parsed.length - this.rules.length; + if (preservedCount > 0) { + this.plugin.log.warn( + `RuleRegistry: preserved ${preservedCount} unsupported or malformed rule entries.`, + ); + } + this.plugin.log.info(`RuleRegistry: loaded ${this.rules.length} rules.`); } catch (e) { this.initError = e instanceof Error ? e : new Error(String(e)); this.plugin.log.error('Failed to load rules', e); this.initDefaultRules(); - await this.saveRules(); } } private initDefaultRules() { // Hardcoded example rules for MVP this.rules = []; + this.persistedRules = []; } getInitializationError(): Error | null { return this.initError; } - // Guard against corrupted/foreign persisted data to keep automation runtime stable. - private validateRules(data: unknown): AutomationRule[] | null { - if (!Array.isArray(data)) { - return null; + private async persistRules(rules: unknown[]) { + try { + await this.plugin.persistDataSynced(JSON.stringify(rules)); + } catch (e) { + this.plugin.log.error('Failed to save rules', e); + throw e; } - - 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 && - typeof c === 'object' && - typeof c.type === 'string' && - validConditions.has(c.type) && - typeof c.value === 'string' && - (c.isRegex === undefined || typeof c.isRegex === 'boolean'); - - const isValidAction = (a: any) => - a && - typeof a === 'object' && - typeof a.type === 'string' && - validActions.has(a.type) && - typeof a.value === 'string'; - - for (const rule of data) { - if (!rule || typeof rule !== 'object') { - return null; - } - const r = rule as AutomationRule; - if ( - typeof r.id !== 'string' || - typeof r.name !== 'string' || - typeof r.isEnabled !== 'boolean' - ) { - return null; - } - if ( - !r.trigger || - typeof r.trigger !== 'object' || - !validTriggers.has((r.trigger as any).type) - ) { - return null; - } - if (!Array.isArray(r.conditions) || r.conditions.some((c) => !isValidCondition(c))) { - return null; - } - if (!Array.isArray(r.actions) || r.actions.some((a) => !isValidAction(a))) { - return null; - } - } - return data as AutomationRule[]; } - 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); - } - }) - .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; + private enqueueMutation(mutation: () => Promise): Promise { + const pendingMutation = this.saveQueue.then(mutation); + this.saveQueue = pendingMutation.catch(() => undefined); + return pendingMutation; + } + + private upsertPersistedRule(persistedRules: unknown[], rule: AutomationRule) { + const index = persistedRules.findIndex( + (entry) => isAutomationRule(entry) && entry.id === rule.id, + ); + if (index !== -1) { + persistedRules[index] = rule; + } else { + persistedRules.push(rule); + } + } + + private ensureInitializedForMutation() { + if (this.initError) { + throw new Error('Automation rules failed to initialize'); + } } async getRules(): Promise { @@ -184,71 +267,105 @@ export class RuleRegistry { return this.rules.filter((r) => r.isEnabled); } - async addOrUpdateRule(rule: AutomationRule) { + async addOrUpdateRule(value: unknown) { + const rule = toAutomationRule(value); + if (!rule) { + throw new Error('Invalid automation rule'); + } + await this.initPromise; - this.saveQueue = this.saveQueue.then(async () => { - const index = this.rules.findIndex((r) => r.id === rule.id); + this.ensureInitializedForMutation(); + await this.enqueueMutation(async () => { + const nextRules = [...this.rules]; + const nextPersistedRules = [...this.persistedRules]; + const index = nextRules.findIndex((r) => r.id === rule.id); if (index !== -1) { - this.rules[index] = rule; + nextRules[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); + nextRules.push(rule); } + this.upsertPersistedRule(nextPersistedRules, rule); + await this.persistRules(nextPersistedRules); + this.rules = nextRules; + this.persistedRules = nextPersistedRules; }); - await this.saveQueue; } - async addRules(rules: AutomationRule[]) { + async addRules(value: unknown) { + if (!Array.isArray(value)) { + throw new Error('Invalid automation rules'); + } + + const rules: AutomationRule[] = []; + for (const entry of value) { + const rule = toAutomationRule(entry); + if (!rule) { + throw new Error('Invalid automation rules'); + } + rules.push(rule); + } + if (rules.length === 0) return; await this.initPromise; - this.saveQueue = this.saveQueue.then(async () => { + this.ensureInitializedForMutation(); + await this.enqueueMutation(async () => { + const nextRules = [...this.rules]; + const nextPersistedRules = [...this.persistedRules]; for (const rule of rules) { - const index = this.rules.findIndex((r) => r.id === rule.id); + const index = nextRules.findIndex((r) => r.id === rule.id); if (index !== -1) { - this.rules[index] = rule; + nextRules[index] = rule; } else { - this.rules.push(rule); + nextRules.push(rule); } + this.upsertPersistedRule(nextPersistedRules, rule); } - try { - await this.plugin.persistDataSynced(JSON.stringify(this.rules)); - } catch (e) { - this.plugin.log.error('Failed to save rules', e); - } + await this.persistRules(nextPersistedRules); + this.rules = nextRules; + this.persistedRules = nextPersistedRules; }); - await this.saveQueue; } - async deleteRule(ruleId: string) { + async deleteRule(ruleId: unknown) { + if (typeof ruleId !== 'string') { + throw new Error('Invalid automation rule ID'); + } + await this.initPromise; - this.saveQueue = this.saveQueue.then(async () => { - this.rules = this.rules.filter((r) => r.id !== ruleId); - try { - await this.plugin.persistDataSynced(JSON.stringify(this.rules)); - } catch (e) { - this.plugin.log.error('Failed to save rules', e); - } + this.ensureInitializedForMutation(); + await this.enqueueMutation(async () => { + const nextRules = this.rules.filter((r) => r.id !== ruleId); + const nextPersistedRules = this.persistedRules.filter( + (entry) => !isAutomationRule(entry) || entry.id !== ruleId, + ); + await this.persistRules(nextPersistedRules); + this.rules = nextRules; + this.persistedRules = nextPersistedRules; }); - await this.saveQueue; } - async toggleRuleStatus(ruleId: string, isEnabled: boolean) { + async toggleRuleStatus(ruleId: unknown, isEnabled: unknown) { + if (typeof ruleId !== 'string') { + throw new Error('Invalid automation rule ID'); + } + if (typeof isEnabled !== 'boolean') { + throw new Error('Invalid automation rule status'); + } + await this.initPromise; - this.saveQueue = this.saveQueue.then(async () => { - const rule = this.rules.find((r) => r.id === ruleId); - if (rule) { - rule.isEnabled = isEnabled; - try { - await this.plugin.persistDataSynced(JSON.stringify(this.rules)); - } catch (e) { - this.plugin.log.error('Failed to save rules', e); - } + this.ensureInitializedForMutation(); + await this.enqueueMutation(async () => { + const index = this.rules.findIndex((r) => r.id === ruleId); + if (index !== -1) { + const nextRules = [...this.rules]; + const nextPersistedRules = [...this.persistedRules]; + const updatedRule = { ...this.rules[index], isEnabled }; + nextRules[index] = updatedRule; + this.upsertPersistedRule(nextPersistedRules, updatedRule); + await this.persistRules(nextPersistedRules); + this.rules = nextRules; + this.persistedRules = nextPersistedRules; } }); - await this.saveQueue; } }