fix: uncaughtException exit, N+1 queries, O(n²) feeds, path validation

[atomantic]

Better Audit — Bugs, Performance & Error Handling

Summary

10 findings addressed across 9 files.

Changes

• [CRITICAL] Add process.exit(1) to uncaughtException handler in errorHandler.js
• [HIGH] Fix unhandled async EventEmitter listener in agentActionExecutor.js + duplicate listener guard
• [HIGH] Replace sequential await loops (N+1) with Promise.all in memoryRetriever.js
• [HIGH] Fix O(F×I) unreadCount computation in feeds.js — pre-compute Map in one pass
• [HIGH] Fix path validation in dataManager.js — use resolve-based containment
• [MEDIUM] Fix cosRunnerClient.js event handler Map to support multiple handlers per event
• [MEDIUM] Replace Math.max spread with reduce in dataSync.js (prevent V8 arg limit)
• [MEDIUM] Use AbortSignal.timeout in memoryEmbeddings.js checkAvailability
• [MEDIUM] Clear SIGKILL fallback timeout on clean process exit in agentManagement.js

Files Modified

server/lib/errorHandler.js, server/services/{agentActionExecutor,memoryRetriever,feeds,dataManager,cosRunnerClient,dataSync,memoryEmbeddings,agentManagement}.js

Merge Order

Independent — can be merged in any order

[Copilot]

Pull request overview

This PR addresses a set of audit findings focused on improving reliability (fatal error handling, async listener safety), security (path traversal prevention), and performance (eliminating N+1 and O(n²) patterns) across server services.

Changes:

• Make process-level error handling more fail-safe (e.g., exit on uncaughtException) and harden event/listener behavior.
• Improve performance by parallelizing memory fetches, optimizing feed unread-count computation, and avoiding V8 argument limits.
• Strengthen filesystem path validation to prevent traversal and out-of-scope deletions.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
server/lib/errorHandler.js	Exits process after uncaughtException (after emitting an error event).
server/services/agentActionExecutor.js	Prevents duplicate scheduler listeners; wraps listener body to avoid unhandled async errors.
server/services/agentManagement.js	Adds SIGKILL fallback timer tracking for termination flow.
server/services/memoryRetriever.js	Replaces sequential memory fetch loops with Promise.all batches.
server/services/feeds.js	Computes unread counts via a single pass map instead of per-feed filtering.
server/services/dataManager.js	Updates purge subpath validation using resolve-based containment.
server/services/cosRunnerClient.js	Refactors runner event handling to support multiple handlers per event.
server/services/dataSync.js	Replaces Math.max(...spread) with reduce to avoid argument limits.
server/services/memoryEmbeddings.js	Uses AbortSignal.timeout for request timeouts in availability checks.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The duplicate-listener guard uses scheduleEvents.listenerCount('execute') > 0, but scheduleEvents has other legitimate listeners (e.g. server/services/socket.js listens to 'execute' to broadcast). If that listener is registered before this init runs, this function will return early and scheduled actions will never execute. Prefer a module-local initialized flag, or track/remove only this module’s specific handler (e.g. keep a reference and check listeners('execute').includes(handler)).

[Copilot]

The path containment check is vulnerable because startsWith(resolve(dirPath)) can be bypassed by sibling paths with the same prefix (e.g. /data/cosmic startsWith /data/cos). Also subPath: '.' resolves to the category root and would delete the entire category directory. Use a boundary-safe containment check (e.g. const rel = relative(dirPath, resolvedTarget) and reject if rel.startsWith('..') || path.isAbsolute(rel)), and explicitly reject resolvedTarget === resolve(dirPath) if the intent is to only allow deleting children.

[Copilot]

killTimer is stored on the agent entry but is never cleared anywhere (no .killTimer references in the repo). If the process exits quickly and the close handler is still doing async cleanup, this timer can still fire and run the SIGKILL/unregister/delete path unnecessarily. Either clear the timeout immediately when termination succeeds (e.g. in the process close handler in agentCliSpawning.js), or don’t store it here and instead clear it at the start of close handling.

Suggested change

	// Store the timer so the close handler can clear it when the process exits cleanly
	const agentEntry = activeAgents.get(agentId);
	if (agentEntry) agentEntry.killTimer = killTimer;
	// Store the timer so it can be cleared when the process exits cleanly
	const agentEntry = activeAgents.get(agentId);
	if (agentEntry) {
	agentEntry.killTimer = killTimer;
	}
	const clearKillTimer = () => {
	const currentAgentEntry = activeAgents.get(agentId);
	if (currentAgentEntry?.killTimer) {
	clearTimeout(currentAgentEntry.killTimer);
	delete currentAgentEntry.killTimer;
	return;
	}
	clearTimeout(killTimer);
	};
	agent.process.once('close', clearKillTimer);
	agent.process.once('exit', clearKillTimer);

[Copilot]

dispatch calls each registered handler directly. If any handler throws/rejects synchronously, the exception will bubble out of the Socket.IO event callback and can crash the process or prevent other handlers from running. Wrap each handler invocation in a try/catch (and optionally schedule async handlers with Promise.resolve().catch(...)) so one bad listener can’t take down the client.

Suggested change

	if (handlers) handlers.forEach(h => h(data));
	if (!handlers) return;
	handlers.forEach((h) => {
	try {
	Promise.resolve(h(data)).catch((err) => {
	console.error(`CoS Runner handler rejected for event "${event}":`, err);
	});
	} catch (err) {
	console.error(`CoS Runner handler failed for event "${event}":`, err);
	}
	});

[Copilot]

This change alters onCosRunnerEvent from single-handler-per-event to multi-handler-per-event, but the existing test suite doesn’t assert the new behavior (e.g. that multiple handlers for the same event all run, and that a throwing handler doesn’t prevent others if you add per-handler isolation). Adding targeted tests here would help prevent regressions in event fan-out semantics.