mirror of
https://github.com/jlengrand/Maestro.git
synced 2026-03-10 08:31:19 +00:00
## CHANGES
- Symphony IPC now validates active contributions against stored sessions 🧩 - Orphaned contributions auto-filtered when sessions disappear, keeping UI clean 🧹 - `symphony:getState` returns only session-backed active items for accuracy 🎯 - `symphony:getActive` now excludes contributions tied to missing sessions 🚫 - Added reusable `filterOrphanedContributions` helper with detailed logging 🪵 - Wired `sessionsStore` dependency through main + handler registration flow 🔌 - Integration tests now mock sessions store for realistic state scenarios 🧪 - Expanded handler tests to cover missing-session filtering behavior 🛡️
This commit is contained in:
@@ -222,10 +222,17 @@ describe('Symphony Integration Tests', () => {
|
|||||||
},
|
},
|
||||||
} as unknown as BrowserWindow;
|
} as unknown as BrowserWindow;
|
||||||
|
|
||||||
|
// Setup mock sessions store (returns empty by default - no sessions)
|
||||||
|
const mockSessionsStore = {
|
||||||
|
get: vi.fn().mockReturnValue([]),
|
||||||
|
set: vi.fn(),
|
||||||
|
};
|
||||||
|
|
||||||
// Setup dependencies
|
// Setup dependencies
|
||||||
mockDeps = {
|
mockDeps = {
|
||||||
app: mockApp,
|
app: mockApp,
|
||||||
getMainWindow: () => mockMainWindow,
|
getMainWindow: () => mockMainWindow,
|
||||||
|
sessionsStore: mockSessionsStore as any,
|
||||||
};
|
};
|
||||||
|
|
||||||
// Default fetch mock (successful responses)
|
// Default fetch mock (successful responses)
|
||||||
|
|||||||
@@ -64,6 +64,7 @@ describe('Symphony IPC handlers', () => {
|
|||||||
let mockApp: App;
|
let mockApp: App;
|
||||||
let mockMainWindow: BrowserWindow;
|
let mockMainWindow: BrowserWindow;
|
||||||
let mockDeps: SymphonyHandlerDependencies;
|
let mockDeps: SymphonyHandlerDependencies;
|
||||||
|
let mockSessionsStore: { get: ReturnType<typeof vi.fn>; set: ReturnType<typeof vi.fn> };
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.clearAllMocks();
|
vi.clearAllMocks();
|
||||||
@@ -88,10 +89,17 @@ describe('Symphony IPC handlers', () => {
|
|||||||
},
|
},
|
||||||
} as unknown as BrowserWindow;
|
} as unknown as BrowserWindow;
|
||||||
|
|
||||||
|
// Setup mock sessions store (exposed for individual tests to modify)
|
||||||
|
mockSessionsStore = {
|
||||||
|
get: vi.fn().mockReturnValue([]),
|
||||||
|
set: vi.fn(),
|
||||||
|
};
|
||||||
|
|
||||||
// Setup dependencies
|
// Setup dependencies
|
||||||
mockDeps = {
|
mockDeps = {
|
||||||
app: mockApp,
|
app: mockApp,
|
||||||
getMainWindow: () => mockMainWindow,
|
getMainWindow: () => mockMainWindow,
|
||||||
|
sessionsStore: mockSessionsStore as any,
|
||||||
};
|
};
|
||||||
|
|
||||||
// Default mock for fs operations
|
// Default mock for fs operations
|
||||||
@@ -1209,7 +1217,7 @@ describe('Symphony IPC handlers', () => {
|
|||||||
expect(result.state.stats.repositoriesContributed).toEqual([]);
|
expect(result.state.stats.repositoriesContributed).toEqual([]);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should return persisted state from disk', async () => {
|
it('should return persisted state from disk (with valid sessions)', async () => {
|
||||||
const persistedState = {
|
const persistedState = {
|
||||||
active: [
|
active: [
|
||||||
{
|
{
|
||||||
@@ -1267,17 +1275,49 @@ describe('Symphony IPC handlers', () => {
|
|||||||
},
|
},
|
||||||
};
|
};
|
||||||
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(persistedState));
|
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(persistedState));
|
||||||
|
// Mock the session to exist so the active contribution is included
|
||||||
|
mockSessionsStore.get.mockReturnValue([{ id: 'session-123', name: 'Test Session' }]);
|
||||||
|
|
||||||
const handler = handlers.get('symphony:getState');
|
const handler = handlers.get('symphony:getState');
|
||||||
const result = await handler!({} as any);
|
const result = await handler!({} as any);
|
||||||
|
|
||||||
expect(result.state).toEqual(persistedState);
|
|
||||||
expect(result.state.active).toHaveLength(1);
|
expect(result.state.active).toHaveLength(1);
|
||||||
expect(result.state.active[0].id).toBe('contrib_123');
|
expect(result.state.active[0].id).toBe('contrib_123');
|
||||||
expect(result.state.history).toHaveLength(1);
|
expect(result.state.history).toHaveLength(1);
|
||||||
expect(result.state.stats.totalContributions).toBe(1);
|
expect(result.state.stats.totalContributions).toBe(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should filter out active contributions with missing sessions', async () => {
|
||||||
|
const persistedState = {
|
||||||
|
active: [
|
||||||
|
{
|
||||||
|
id: 'contrib_with_session',
|
||||||
|
repoSlug: 'owner/repo',
|
||||||
|
sessionId: 'session-exists',
|
||||||
|
status: 'running',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
id: 'contrib_orphaned',
|
||||||
|
repoSlug: 'owner/repo2',
|
||||||
|
sessionId: 'session-gone',
|
||||||
|
status: 'running',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
history: [],
|
||||||
|
stats: { totalContributions: 0 },
|
||||||
|
};
|
||||||
|
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(persistedState));
|
||||||
|
// Only one session exists
|
||||||
|
mockSessionsStore.get.mockReturnValue([{ id: 'session-exists', name: 'Existing' }]);
|
||||||
|
|
||||||
|
const handler = handlers.get('symphony:getState');
|
||||||
|
const result = await handler!({} as any);
|
||||||
|
|
||||||
|
// Only the contribution with an existing session should be returned
|
||||||
|
expect(result.state.active).toHaveLength(1);
|
||||||
|
expect(result.state.active[0].id).toBe('contrib_with_session');
|
||||||
|
});
|
||||||
|
|
||||||
it('should handle file read errors gracefully', async () => {
|
it('should handle file read errors gracefully', async () => {
|
||||||
vi.mocked(fs.readFile).mockRejectedValue(new Error('Permission denied'));
|
vi.mocked(fs.readFile).mockRejectedValue(new Error('Permission denied'));
|
||||||
|
|
||||||
@@ -1302,7 +1342,7 @@ describe('Symphony IPC handlers', () => {
|
|||||||
expect(result.contributions).toEqual([]);
|
expect(result.contributions).toEqual([]);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should return all active contributions from state', async () => {
|
it('should return active contributions that have matching sessions', async () => {
|
||||||
const stateWithActive = {
|
const stateWithActive = {
|
||||||
active: [
|
active: [
|
||||||
{
|
{
|
||||||
@@ -1310,18 +1350,25 @@ describe('Symphony IPC handlers', () => {
|
|||||||
repoSlug: 'owner/repo1',
|
repoSlug: 'owner/repo1',
|
||||||
issueNumber: 1,
|
issueNumber: 1,
|
||||||
status: 'running',
|
status: 'running',
|
||||||
|
sessionId: 'session_1',
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
id: 'contrib_2',
|
id: 'contrib_2',
|
||||||
repoSlug: 'owner/repo2',
|
repoSlug: 'owner/repo2',
|
||||||
issueNumber: 2,
|
issueNumber: 2,
|
||||||
status: 'paused',
|
status: 'paused',
|
||||||
|
sessionId: 'session_2',
|
||||||
},
|
},
|
||||||
],
|
],
|
||||||
history: [],
|
history: [],
|
||||||
stats: {},
|
stats: {},
|
||||||
};
|
};
|
||||||
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(stateWithActive));
|
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(stateWithActive));
|
||||||
|
// Mock sessions store to return matching sessions
|
||||||
|
mockSessionsStore.get.mockReturnValue([
|
||||||
|
{ id: 'session_1', name: 'Session 1' },
|
||||||
|
{ id: 'session_2', name: 'Session 2' },
|
||||||
|
]);
|
||||||
|
|
||||||
const handler = handlers.get('symphony:getActive');
|
const handler = handlers.get('symphony:getActive');
|
||||||
const result = await handler!({} as any);
|
const result = await handler!({} as any);
|
||||||
@@ -1330,6 +1377,39 @@ describe('Symphony IPC handlers', () => {
|
|||||||
expect(result.contributions[0].id).toBe('contrib_1');
|
expect(result.contributions[0].id).toBe('contrib_1');
|
||||||
expect(result.contributions[1].id).toBe('contrib_2');
|
expect(result.contributions[1].id).toBe('contrib_2');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should filter out contributions whose sessions no longer exist', async () => {
|
||||||
|
const stateWithActive = {
|
||||||
|
active: [
|
||||||
|
{
|
||||||
|
id: 'contrib_1',
|
||||||
|
repoSlug: 'owner/repo1',
|
||||||
|
issueNumber: 1,
|
||||||
|
status: 'running',
|
||||||
|
sessionId: 'session_exists',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
id: 'contrib_2',
|
||||||
|
repoSlug: 'owner/repo2',
|
||||||
|
issueNumber: 2,
|
||||||
|
status: 'paused',
|
||||||
|
sessionId: 'session_gone',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
history: [],
|
||||||
|
stats: {},
|
||||||
|
};
|
||||||
|
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(stateWithActive));
|
||||||
|
// Only return session_exists, session_gone is missing
|
||||||
|
mockSessionsStore.get.mockReturnValue([{ id: 'session_exists', name: 'Existing Session' }]);
|
||||||
|
|
||||||
|
const handler = handlers.get('symphony:getActive');
|
||||||
|
const result = await handler!({} as any);
|
||||||
|
|
||||||
|
// Only contrib_1 should be returned since contrib_2's session is gone
|
||||||
|
expect(result.contributions).toHaveLength(1);
|
||||||
|
expect(result.contributions[0].id).toBe('contrib_1');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('symphony:getCompleted', () => {
|
describe('symphony:getCompleted', () => {
|
||||||
|
|||||||
@@ -608,6 +608,7 @@ function setupIpcHandlers() {
|
|||||||
registerSymphonyHandlers({
|
registerSymphonyHandlers({
|
||||||
app,
|
app,
|
||||||
getMainWindow: () => mainWindow,
|
getMainWindow: () => mainWindow,
|
||||||
|
sessionsStore,
|
||||||
});
|
});
|
||||||
|
|
||||||
// Register tab naming handlers for automatic tab naming
|
// Register tab naming handlers for automatic tab naming
|
||||||
|
|||||||
@@ -259,6 +259,7 @@ export function registerAllHandlers(deps: HandlerDependencies): void {
|
|||||||
registerSymphonyHandlers({
|
registerSymphonyHandlers({
|
||||||
app: deps.app,
|
app: deps.app,
|
||||||
getMainWindow: deps.getMainWindow,
|
getMainWindow: deps.getMainWindow,
|
||||||
|
sessionsStore: deps.sessionsStore,
|
||||||
});
|
});
|
||||||
// Register agent error handlers (error state management)
|
// Register agent error handlers (error state management)
|
||||||
registerAgentErrorHandlers();
|
registerAgentErrorHandlers();
|
||||||
|
|||||||
@@ -11,10 +11,12 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import { ipcMain, App, BrowserWindow } from 'electron';
|
import { ipcMain, App, BrowserWindow } from 'electron';
|
||||||
|
import type Store from 'electron-store';
|
||||||
import fs from 'fs/promises';
|
import fs from 'fs/promises';
|
||||||
import path from 'path';
|
import path from 'path';
|
||||||
import { logger } from '../../utils/logger';
|
import { logger } from '../../utils/logger';
|
||||||
import { isWebContentsAvailable } from '../../utils/safe-send';
|
import { isWebContentsAvailable } from '../../utils/safe-send';
|
||||||
|
import type { SessionsData, StoredSession } from '../../stores/types';
|
||||||
import { createIpcHandler, CreateHandlerOptions } from '../../utils/ipcHandler';
|
import { createIpcHandler, CreateHandlerOptions } from '../../utils/ipcHandler';
|
||||||
import { execFileNoThrow } from '../../utils/execFile';
|
import { execFileNoThrow } from '../../utils/execFile';
|
||||||
import { getExpandedEnv } from '../../agents/path-prober';
|
import { getExpandedEnv } from '../../agents/path-prober';
|
||||||
@@ -194,6 +196,7 @@ function validateContributionParams(params: {
|
|||||||
export interface SymphonyHandlerDependencies {
|
export interface SymphonyHandlerDependencies {
|
||||||
app: App;
|
app: App;
|
||||||
getMainWindow: () => BrowserWindow | null;
|
getMainWindow: () => BrowserWindow | null;
|
||||||
|
sessionsStore: Store<SessionsData>;
|
||||||
}
|
}
|
||||||
|
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
@@ -865,6 +868,39 @@ function broadcastSymphonyUpdate(getMainWindow: () => BrowserWindow | null): voi
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Filter out orphaned contributions whose sessions no longer exist.
|
||||||
|
* Returns only contributions that have a corresponding session in the sessions store.
|
||||||
|
*/
|
||||||
|
function filterOrphanedContributions(
|
||||||
|
contributions: ActiveContribution[],
|
||||||
|
sessionsStore: Store<SessionsData>
|
||||||
|
): ActiveContribution[] {
|
||||||
|
const sessions = sessionsStore.get('sessions', []) as StoredSession[];
|
||||||
|
const sessionIds = new Set(sessions.map((s) => s.id));
|
||||||
|
|
||||||
|
const validContributions: ActiveContribution[] = [];
|
||||||
|
const orphanedIds: string[] = [];
|
||||||
|
|
||||||
|
for (const contribution of contributions) {
|
||||||
|
if (sessionIds.has(contribution.sessionId)) {
|
||||||
|
validContributions.push(contribution);
|
||||||
|
} else {
|
||||||
|
orphanedIds.push(contribution.id);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (orphanedIds.length > 0) {
|
||||||
|
logger.info(
|
||||||
|
`Filtering ${orphanedIds.length} orphaned contribution(s) with missing sessions`,
|
||||||
|
LOG_CONTEXT,
|
||||||
|
{ orphanedIds }
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
return validContributions;
|
||||||
|
}
|
||||||
|
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
// Handler Options Helper
|
// Handler Options Helper
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
@@ -882,6 +918,7 @@ const handlerOpts = (operation: string, logSuccess = true): CreateHandlerOptions
|
|||||||
export function registerSymphonyHandlers({
|
export function registerSymphonyHandlers({
|
||||||
app,
|
app,
|
||||||
getMainWindow,
|
getMainWindow,
|
||||||
|
sessionsStore,
|
||||||
}: SymphonyHandlerDependencies): void {
|
}: SymphonyHandlerDependencies): void {
|
||||||
// ─────────────────────────────────────────────────────────────────────────
|
// ─────────────────────────────────────────────────────────────────────────
|
||||||
// Registry Operations
|
// Registry Operations
|
||||||
@@ -1031,6 +1068,7 @@ export function registerSymphonyHandlers({
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Get current symphony state.
|
* Get current symphony state.
|
||||||
|
* Filters out contributions whose sessions no longer exist.
|
||||||
*/
|
*/
|
||||||
ipcMain.handle(
|
ipcMain.handle(
|
||||||
'symphony:getState',
|
'symphony:getState',
|
||||||
@@ -1038,6 +1076,8 @@ export function registerSymphonyHandlers({
|
|||||||
handlerOpts('getState', false),
|
handlerOpts('getState', false),
|
||||||
async (): Promise<{ state: SymphonyState }> => {
|
async (): Promise<{ state: SymphonyState }> => {
|
||||||
const state = await readState(app);
|
const state = await readState(app);
|
||||||
|
// Filter out orphaned contributions whose sessions are gone
|
||||||
|
state.active = filterOrphanedContributions(state.active, sessionsStore);
|
||||||
return { state };
|
return { state };
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
@@ -1045,6 +1085,7 @@ export function registerSymphonyHandlers({
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Get active contributions.
|
* Get active contributions.
|
||||||
|
* Filters out contributions whose sessions no longer exist.
|
||||||
*/
|
*/
|
||||||
ipcMain.handle(
|
ipcMain.handle(
|
||||||
'symphony:getActive',
|
'symphony:getActive',
|
||||||
@@ -1052,7 +1093,8 @@ export function registerSymphonyHandlers({
|
|||||||
handlerOpts('getActive', false),
|
handlerOpts('getActive', false),
|
||||||
async (): Promise<{ contributions: ActiveContribution[] }> => {
|
async (): Promise<{ contributions: ActiveContribution[] }> => {
|
||||||
const state = await readState(app);
|
const state = await readState(app);
|
||||||
return { contributions: state.active };
|
const validContributions = filterOrphanedContributions(state.active, sessionsStore);
|
||||||
|
return { contributions: validContributions };
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
);
|
);
|
||||||
|
|||||||
Reference in New Issue
Block a user