From fc0fead078499cbff82d2d6ce5ffa2853c10cc57 Mon Sep 17 00:00:00 2001 From: Nikolas de Hor Date: Wed, 25 Feb 2026 15:39:40 -0300 Subject: [PATCH 1/2] fix: add null guards for checkpoints/failedAttempts/notifications arrays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getLastCheckpoint, getCheckpoint e getStatus acessavam .checkpoints, .failedAttempts e .notifications sem verificar se existem. Quando o state é carregado de JSON sem esses campos (campos opcionais na validação), ocorre TypeError. Adiciona 12 testes de regressão cobrindo os cenários de null. Closes #513 --- .../core/execution/build-state-manager.js | 10 +- .../build-state-manager-guards.test.js | 106 ++++++++++++++++++ 2 files changed, 111 insertions(+), 5 deletions(-) create mode 100644 tests/core/execution/build-state-manager-guards.test.js diff --git a/.aios-core/core/execution/build-state-manager.js b/.aios-core/core/execution/build-state-manager.js index a2ca4ad0c9..6faac6f85b 100644 --- a/.aios-core/core/execution/build-state-manager.js +++ b/.aios-core/core/execution/build-state-manager.js @@ -397,7 +397,7 @@ class BuildStateManager { * @returns {Object|null} Last checkpoint or null */ getLastCheckpoint() { - if (!this._state || this._state.checkpoints.length === 0) { + if (!this._state || !this._state.checkpoints || this._state.checkpoints.length === 0) { return null; } return this._state.checkpoints[this._state.checkpoints.length - 1]; @@ -410,7 +410,7 @@ class BuildStateManager { * @returns {Object|null} Checkpoint or null */ getCheckpoint(checkpointId) { - if (!this._state) { + if (!this._state || !this._state.checkpoints) { return null; } return this._state.checkpoints.find((c) => c.id === checkpointId) || null; @@ -551,11 +551,11 @@ class BuildStateManager { : 0, }, metrics: state.metrics, - recentFailures: state.failedAttempts.slice(-5), + recentFailures: (state.failedAttempts || []).slice(-5), abandoned: isAbandoned, worktree: state.worktree, - checkpointCount: state.checkpoints.length, - notificationCount: state.notifications.filter((n) => !n.acknowledged).length, + checkpointCount: (state.checkpoints || []).length, + notificationCount: (state.notifications || []).filter((n) => !n.acknowledged).length, }; } diff --git a/tests/core/execution/build-state-manager-guards.test.js b/tests/core/execution/build-state-manager-guards.test.js new file mode 100644 index 0000000000..32995aae06 --- /dev/null +++ b/tests/core/execution/build-state-manager-guards.test.js @@ -0,0 +1,106 @@ +/** + * Testes para null guards em BuildStateManager + * + * Valida que getLastCheckpoint, getCheckpoint e getStatus + * não falham com TypeError quando arrays são undefined. + * + * Closes #513 + */ + +const fs = require('fs'); +const path = require('path'); + +// Mock fs para evitar I/O real +jest.spyOn(fs, 'existsSync').mockReturnValue(false); +jest.spyOn(fs, 'mkdirSync').mockImplementation(() => {}); + +const { BuildStateManager } = require('../../../.aios-core/core/execution/build-state-manager'); + +describe('BuildStateManager - null guards (#513)', () => { + let manager; + + beforeEach(() => { + manager = new BuildStateManager('test-story', { + rootPath: '/fake/project', + }); + }); + + // ============================================================ + // getLastCheckpoint + // ============================================================ + describe('getLastCheckpoint', () => { + test('retorna null quando _state é null', () => { + manager._state = null; + expect(manager.getLastCheckpoint()).toBeNull(); + }); + + test('retorna null quando checkpoints é undefined', () => { + manager._state = { status: 'running' }; + expect(manager.getLastCheckpoint()).toBeNull(); + }); + + test('retorna null quando checkpoints é array vazio', () => { + manager._state = { checkpoints: [] }; + expect(manager.getLastCheckpoint()).toBeNull(); + }); + + test('retorna último checkpoint', () => { + const checkpoint = { id: 'cp-2', phase: 'build' }; + manager._state = { + checkpoints: [{ id: 'cp-1', phase: 'setup' }, checkpoint], + }; + expect(manager.getLastCheckpoint()).toEqual(checkpoint); + }); + }); + + // ============================================================ + // getCheckpoint + // ============================================================ + describe('getCheckpoint', () => { + test('retorna null quando _state é null', () => { + manager._state = null; + expect(manager.getCheckpoint('cp-1')).toBeNull(); + }); + + test('retorna null quando checkpoints é undefined', () => { + manager._state = { status: 'running' }; + expect(manager.getCheckpoint('cp-1')).toBeNull(); + }); + + test('retorna null quando checkpoint não existe', () => { + manager._state = { checkpoints: [{ id: 'cp-1' }] }; + expect(manager.getCheckpoint('cp-999')).toBeNull(); + }); + + test('retorna checkpoint pelo ID', () => { + const target = { id: 'cp-2', phase: 'test' }; + manager._state = { + checkpoints: [{ id: 'cp-1' }, target], + }; + expect(manager.getCheckpoint('cp-2')).toEqual(target); + }); + }); + + // ============================================================ + // constructor + // ============================================================ + describe('constructor', () => { + test('requer storyId', () => { + expect(() => new BuildStateManager()).toThrow('storyId is required'); + }); + + test('aceita storyId string', () => { + const m = new BuildStateManager('story-123'); + expect(m.storyId).toBe('story-123'); + }); + + test('inicializa _state como null', () => { + expect(manager._state).toBeNull(); + }); + + test('usa rootPath padrão se não fornecido', () => { + const m = new BuildStateManager('s1'); + expect(m.rootPath).toBe(process.cwd()); + }); + }); +}); From a23e5baf88603a1c98e73b9aafff9917d2e02cac Mon Sep 17 00:00:00 2001 From: Nikolas de Hor Date: Wed, 25 Feb 2026 19:33:42 -0300 Subject: [PATCH 2/2] fix: adicionar null guards para getNotifications, acknowledgeNotification e failedAttempts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit encontrou que getNotifications(), acknowledgeNotification() e recordFailure() acessam arrays sem verificar se existem, causando TypeError quando notifications ou failedAttempts são undefined. Correções: - getNotifications: (this._state.notifications || []).filter(...) - acknowledgeNotification: guard !this._state.notifications antes de indexar - recordFailure: this._state.failedAttempts ??= [] antes de .filter/.push - _checkIfStuck: (this._state.failedAttempts || []).filter(...) Adicionados 11 novos testes cobrindo getStatus, getNotifications, acknowledgeNotification e recordFailure com arrays undefined. Total: 23 testes (era 12). --- .../core/execution/build-state-manager.js | 8 +- .../build-state-manager-guards.test.js | 135 +++++++++++++++++- 2 files changed, 139 insertions(+), 4 deletions(-) diff --git a/.aios-core/core/execution/build-state-manager.js b/.aios-core/core/execution/build-state-manager.js index 6faac6f85b..29a06e95c2 100644 --- a/.aios-core/core/execution/build-state-manager.js +++ b/.aios-core/core/execution/build-state-manager.js @@ -781,6 +781,8 @@ class BuildStateManager { throw new Error('No state loaded'); } + this._state.failedAttempts ??= []; + const failure = { subtaskId, attempt: @@ -831,7 +833,7 @@ class BuildStateManager { } // Get attempts for this subtask - const attempts = this._state.failedAttempts + const attempts = (this._state.failedAttempts || []) .filter((f) => f.subtaskId === subtaskId) .map((f) => ({ success: false, @@ -895,7 +897,7 @@ class BuildStateManager { */ getNotifications() { if (!this._state) return []; - return this._state.notifications.filter((n) => !n.acknowledged); + return (this._state.notifications || []).filter((n) => !n.acknowledged); } /** @@ -904,7 +906,7 @@ class BuildStateManager { * @param {number} index - Notification index */ acknowledgeNotification(index) { - if (!this._state || !this._state.notifications[index]) return; + if (!this._state || !this._state.notifications || !this._state.notifications[index]) return; this._state.notifications[index].acknowledged = true; this.saveState(); } diff --git a/tests/core/execution/build-state-manager-guards.test.js b/tests/core/execution/build-state-manager-guards.test.js index 32995aae06..6b3be28ce0 100644 --- a/tests/core/execution/build-state-manager-guards.test.js +++ b/tests/core/execution/build-state-manager-guards.test.js @@ -1,7 +1,8 @@ /** * Testes para null guards em BuildStateManager * - * Valida que getLastCheckpoint, getCheckpoint e getStatus + * Valida que getLastCheckpoint, getCheckpoint, getStatus, + * getNotifications, acknowledgeNotification e recordFailure * não falham com TypeError quando arrays são undefined. * * Closes #513 @@ -103,4 +104,136 @@ describe('BuildStateManager - null guards (#513)', () => { expect(m.rootPath).toBe(process.cwd()); }); }); + + // ============================================================ + // getStatus - null guards para arrays opcionais + // ============================================================ + describe('getStatus', () => { + test('retorna exists:false quando loadState retorna null', () => { + jest.spyOn(manager, 'loadState').mockReturnValue(null); + const status = manager.getStatus(); + expect(status.exists).toBe(false); + }); + + test('calcula recentFailures com failedAttempts undefined', () => { + jest.spyOn(manager, 'loadState').mockReturnValue({ + status: 'running', + startedAt: new Date().toISOString(), + currentPhase: 'build', + currentSubtask: null, + metrics: { completedSubtasks: 0, totalSubtasks: 1, totalFailures: 0, totalAttempts: 0 }, + worktree: null, + lastCheckpoint: null, + }); + jest.spyOn(manager, '_checkAbandoned').mockReturnValue(false); + const status = manager.getStatus(); + expect(status.recentFailures).toEqual([]); + expect(status.checkpointCount).toBe(0); + expect(status.notificationCount).toBe(0); + }); + + test('calcula checkpointCount e notificationCount com arrays presentes', () => { + jest.spyOn(manager, 'loadState').mockReturnValue({ + status: 'running', + startedAt: new Date().toISOString(), + currentPhase: 'build', + currentSubtask: null, + metrics: { completedSubtasks: 0, totalSubtasks: 1, totalFailures: 0, totalAttempts: 0 }, + worktree: null, + lastCheckpoint: null, + checkpoints: [{ id: 'cp-1' }, { id: 'cp-2' }], + notifications: [ + { acknowledged: false }, + { acknowledged: true }, + { acknowledged: false }, + ], + failedAttempts: [{ subtaskId: 'a' }, { subtaskId: 'b' }], + }); + jest.spyOn(manager, '_checkAbandoned').mockReturnValue(false); + const status = manager.getStatus(); + expect(status.checkpointCount).toBe(2); + expect(status.notificationCount).toBe(2); + expect(status.recentFailures).toHaveLength(2); + }); + }); + + // ============================================================ + // getNotifications - null guard + // ============================================================ + describe('getNotifications', () => { + test('retorna array vazio quando _state é null', () => { + manager._state = null; + expect(manager.getNotifications()).toEqual([]); + }); + + test('retorna array vazio quando notifications é undefined', () => { + manager._state = { status: 'running' }; + expect(manager.getNotifications()).toEqual([]); + }); + + test('filtra apenas não-acknowledged', () => { + manager._state = { + notifications: [ + { message: 'a', acknowledged: false }, + { message: 'b', acknowledged: true }, + { message: 'c', acknowledged: false }, + ], + }; + const result = manager.getNotifications(); + expect(result).toHaveLength(2); + expect(result[0].message).toBe('a'); + expect(result[1].message).toBe('c'); + }); + }); + + // ============================================================ + // acknowledgeNotification - null guard + // ============================================================ + describe('acknowledgeNotification', () => { + test('não falha quando _state é null', () => { + manager._state = null; + expect(() => manager.acknowledgeNotification(0)).not.toThrow(); + }); + + test('não falha quando notifications é undefined', () => { + manager._state = { status: 'running' }; + expect(() => manager.acknowledgeNotification(0)).not.toThrow(); + }); + + test('não falha com índice inválido', () => { + manager._state = { notifications: [{ acknowledged: false }] }; + jest.spyOn(manager, 'saveState').mockImplementation(() => {}); + expect(() => manager.acknowledgeNotification(99)).not.toThrow(); + }); + + test('marca notification como acknowledged', () => { + manager._state = { + notifications: [ + { message: 'alerta', acknowledged: false }, + ], + }; + jest.spyOn(manager, 'saveState').mockImplementation(() => {}); + manager.acknowledgeNotification(0); + expect(manager._state.notifications[0].acknowledged).toBe(true); + }); + }); + + // ============================================================ + // recordFailure - failedAttempts null guard + // ============================================================ + describe('recordFailure', () => { + test('inicializa failedAttempts quando undefined', () => { + manager._state = { + status: 'running', + metrics: { totalFailures: 0, totalAttempts: 0 }, + notifications: [], + }; + jest.spyOn(manager, 'saveState').mockImplementation(() => {}); + jest.spyOn(manager, '_logAttempt').mockImplementation(() => {}); + + const result = manager.recordFailure('subtask-1', { error: 'test error' }); + expect(result.failure.subtaskId).toBe('subtask-1'); + expect(manager._state.failedAttempts).toHaveLength(1); + }); + }); });