Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions apps/server/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -598,24 +598,23 @@ wss.on('connection', (ws: WebSocket) => {

// Subscribe to all events and forward to this client
const unsubscribe = events.subscribe((type, payload) => {
logger.info('Event received:', {
// Use debug level for high-frequency events to avoid log spam
// that causes progressive memory growth and server slowdown
const isHighFrequency =
type === 'dev-server:output' || type === 'test-runner:output' || type === 'feature:progress';
const log = isHighFrequency ? logger.debug.bind(logger) : logger.info.bind(logger);

log('Event received:', {
type,
hasPayload: !!payload,
payloadKeys: payload ? Object.keys(payload) : [],
wsReadyState: ws.readyState,
wsOpen: ws.readyState === WebSocket.OPEN,
});

if (ws.readyState === WebSocket.OPEN) {
const message = JSON.stringify({ type, payload });
logger.info('Sending event to client:', {
type,
messageLength: message.length,
sessionId: (payload as Record<string, unknown>)?.sessionId,
});
ws.send(message);
} else {
logger.info('WARNING: Cannot send event, WebSocket not open. ReadyState:', ws.readyState);
logger.warn('Cannot send event, WebSocket not open. ReadyState:', ws.readyState);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The log level for the WebSocket not-open warning is changed from info to warn. While this change aligns with the severity of the message, it's important to ensure that such warnings are appropriately handled and monitored in a production environment. Consider adding metrics or alerts to track the frequency of these warnings to proactively address potential WebSocket connectivity issues.

If the WebSocket is not opened, it might be a critical issue that needs to be addressed soon.

});

Expand Down
10 changes: 7 additions & 3 deletions apps/server/src/services/dev-server-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,13 @@ const PORT_PATTERNS: Array<{ pattern: RegExp; description: string }> = [
},
];

// Throttle output to prevent overwhelming WebSocket under heavy load
const OUTPUT_THROTTLE_MS = 4; // ~250fps max update rate for responsive feedback
const OUTPUT_BATCH_SIZE = 4096; // Smaller batches for lower latency
// Throttle output to prevent overwhelming WebSocket under heavy load.
// 100ms (~10fps) is sufficient for readable log streaming while keeping
// WebSocket traffic manageable. The previous 4ms rate (~250fps) generated
// up to 250 events/sec which caused progressive browser slowdown from
// accumulated console logs, JSON serialization overhead, and React re-renders.
Comment on lines +91 to +95
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The comments added to explain the throttling mechanism are very helpful for understanding the rationale behind the changes. However, the comment could be improved by explicitly mentioning the specific metrics or monitoring tools that can be used to observe the impact of the throttling on browser performance and server load. This would provide additional guidance for future maintenance and optimization.

It's important to monitor the impact of this change on the user experience. If the throttling is too aggressive, it could lead to a noticeable degradation in the responsiveness of the dev server logs.

const OUTPUT_THROTTLE_MS = 100; // ~10fps max update rate
const OUTPUT_BATCH_SIZE = 8192; // Larger batches to compensate for lower frequency
Comment on lines +96 to +97
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The OUTPUT_THROTTLE_MS and OUTPUT_BATCH_SIZE constants are updated to 100ms and 8192 bytes, respectively. While these values are chosen to balance log streaming readability and WebSocket traffic, it's important to consider making these configurable via environment variables. This would allow administrators to fine-tune the throttling behavior without requiring code changes.

Making these values configurable would provide flexibility to adapt to different network conditions and browser capabilities.

Suggested change
const OUTPUT_THROTTLE_MS = 100; // ~10fps max update rate
const OUTPUT_BATCH_SIZE = 8192; // Larger batches to compensate for lower frequency
const OUTPUT_THROTTLE_MS = parseInt(process.env.OUTPUT_THROTTLE_MS || '100', 10); // ~10fps max update rate
const OUTPUT_BATCH_SIZE = parseInt(process.env.OUTPUT_BATCH_SIZE || '8192', 10); // Larger batches to compensate for lower frequency


export interface DevServerInfo {
worktreePath: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ describe('DevServerService Event Types', () => {

// 2. Output & URL Detected
mockProcess.stdout.emit('data', Buffer.from('Local: http://localhost:5173/\n'));
// Throttled output needs a bit of time
await new Promise((resolve) => setTimeout(resolve, 100));
// Throttled output needs a bit of time (OUTPUT_THROTTLE_MS is 100ms)
await new Promise((resolve) => setTimeout(resolve, 250));
Comment on lines +93 to +94
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The timeout in the test is increased from 100ms to 250ms to accommodate the new throttle rate. While this addresses the immediate issue, it's important to ensure that the test remains reliable and doesn't become flaky due to timing variations. Consider adding additional assertions or retries to make the test more robust.

It's important to ensure that the test consistently passes and doesn't flake due to timing issues.

expect(emittedEvents['dev-server:output'].length).toBeGreaterThanOrEqual(1);
expect(emittedEvents['dev-server:url-detected'].length).toBe(1);
expect(emittedEvents['dev-server:url-detected'][0].url).toBe('http://localhost:5173/');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,7 @@ export function useBoardFeatures({ currentProject }: UseBoardFeaturesProps) {
const isRestoring = useIsRestoring();

// Use React Query for features
const {
data: features = [],
isLoading: isQueryLoading,
refetch: loadFeatures,
} = useFeatures(currentProject?.path);
const { data: features = [], isLoading: isQueryLoading } = useFeatures(currentProject?.path);

// Don't report loading while IDB cache restore is in progress —
// features will appear momentarily once the restore completes.
Expand Down Expand Up @@ -159,7 +155,6 @@ export function useBoardFeatures({ currentProject }: UseBoardFeaturesProps) {
});

return unsubscribe;
// eslint-disable-next-line react-hooks/exhaustive-deps -- loadFeatures is a stable ref from React Query
}, [currentProject]);

// Check for interrupted features on mount
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,27 @@ export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevS
// Keep track of whether we've fetched initial logs
const hasFetchedInitialLogs = useRef(false);

// Buffer for batching rapid output events into fewer setState calls.
// Content accumulates here and is flushed via requestAnimationFrame,
// ensuring at most one React re-render per animation frame (~60fps max).
// A fallback setTimeout ensures the buffer is flushed even when RAF is
// throttled (e.g., when the tab is in the background).
const pendingOutputRef = useRef('');
const rafIdRef = useRef<number | null>(null);
const timerIdRef = useRef<ReturnType<typeof setTimeout> | null>(null);

const resetPendingOutput = useCallback(() => {
if (rafIdRef.current !== null) {
cancelAnimationFrame(rafIdRef.current);
rafIdRef.current = null;
}
if (timerIdRef.current !== null) {
clearTimeout(timerIdRef.current);
timerIdRef.current = null;
}
pendingOutputRef.current = '';
}, []);

/**
* Fetch buffered logs from the server
*/
Expand Down Expand Up @@ -130,6 +151,7 @@ export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevS
* Clear logs and reset state
*/
const clearLogs = useCallback(() => {
resetPendingOutput();
setState({
logs: '',
logsVersion: 0,
Expand All @@ -144,13 +166,19 @@ export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevS
serverError: null,
});
hasFetchedInitialLogs.current = false;
}, []);
}, [resetPendingOutput]);

const flushPendingOutput = useCallback(() => {
// Clear both scheduling handles to prevent duplicate flushes
rafIdRef.current = null;
if (timerIdRef.current !== null) {
clearTimeout(timerIdRef.current);
timerIdRef.current = null;
}
const content = pendingOutputRef.current;
if (!content) return;
pendingOutputRef.current = '';

/**
* Append content to logs, enforcing a maximum buffer size to prevent
* unbounded memory growth and progressive UI lag.
*/
const appendLogs = useCallback((content: string) => {
setState((prev) => {
const combined = prev.logs + content;
const didTrim = combined.length > MAX_LOG_BUFFER_SIZE;
Expand All @@ -170,6 +198,48 @@ export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevS
});
}, []);

/**
* Append content to logs, enforcing a maximum buffer size to prevent
* unbounded memory growth and progressive UI lag.
*
* Uses requestAnimationFrame to batch rapid output events into at most
* one React state update per frame, preventing excessive re-renders.
* A fallback setTimeout(250ms) ensures the buffer is flushed even when
* RAF is throttled (e.g., when the tab is in the background).
* If the pending buffer reaches MAX_LOG_BUFFER_SIZE, flushes immediately
* to prevent unbounded memory growth.
*/
const appendLogs = useCallback(
(content: string) => {
pendingOutputRef.current += content;

// Flush immediately if buffer has reached the size limit
if (pendingOutputRef.current.length >= MAX_LOG_BUFFER_SIZE) {
flushPendingOutput();
return;
}

// Schedule a RAF flush if not already scheduled
if (rafIdRef.current === null) {
rafIdRef.current = requestAnimationFrame(flushPendingOutput);
}

// Schedule a fallback timer flush if not already scheduled,
// to handle cases where RAF is throttled (background tab)
if (timerIdRef.current === null) {
timerIdRef.current = setTimeout(flushPendingOutput, 250);
}
},
[flushPendingOutput]
);

// Clean up pending RAF on unmount to prevent state updates after unmount
useEffect(() => {
return () => {
resetPendingOutput();
};
}, [resetPendingOutput]);

// Fetch initial logs when worktreePath changes
useEffect(() => {
if (worktreePath && autoSubscribe) {
Expand All @@ -196,6 +266,7 @@ export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevS

switch (event.type) {
case 'dev-server:started': {
resetPendingOutput();
const { payload } = event;
logger.info('Dev server started:', payload);
setState((prev) => ({
Expand Down Expand Up @@ -245,7 +316,7 @@ export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevS
});

return unsubscribe;
}, [worktreePath, autoSubscribe, appendLogs]);
}, [worktreePath, autoSubscribe, appendLogs, resetPendingOutput]);

return {
...state,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ import { getElectronAPI } from '@/lib/electron';
import { normalizePath } from '@/lib/utils';
import { toast } from 'sonner';
import type { DevServerInfo, WorktreeInfo } from '../types';
import { useEventRecencyStore } from '@/hooks/use-event-recency';

const logger = createLogger('DevServers');

// Timeout (ms) for port detection before showing a warning to the user
const PORT_DETECTION_TIMEOUT_MS = 30_000;
// Interval (ms) for periodic state reconciliation with the backend
const STATE_RECONCILE_INTERVAL_MS = 5_000;
// Interval (ms) for periodic state reconciliation with the backend.
// 30 seconds is sufficient since WebSocket events handle real-time updates;
// reconciliation is only a fallback for missed events (PWA restart, WS gaps).
// The previous 5-second interval added unnecessary HTTP pressure.
const STATE_RECONCILE_INTERVAL_MS = 30_000;

interface UseDevServersOptions {
projectPath: string;
Expand Down Expand Up @@ -322,12 +326,24 @@ export function useDevServers({ projectPath }: UseDevServersOptions) {
return () => clearInterval(intervalId);
}, [clearPortDetectionTimer, startPortDetectionTimer]);

// Record global events so smart polling knows WebSocket is healthy.
// Without this, dev-server events don't suppress polling intervals,
// causing all queries (features, worktrees, running-agents) to poll
// at their default rates even though the WebSocket is actively connected.
const recordGlobalEvent = useEventRecencyStore((state) => state.recordGlobalEvent);

// Subscribe to all dev server lifecycle events for reactive state updates
useEffect(() => {
const api = getElectronAPI();
if (!api?.worktree?.onDevServerLogEvent) return;

const unsubscribe = api.worktree.onDevServerLogEvent((event) => {
// Record that WS is alive (but only for lifecycle events, not output -
// output fires too frequently and would trigger unnecessary store updates)
if (event.type !== 'dev-server:output') {
recordGlobalEvent();
}

if (event.type === 'dev-server:starting') {
const { worktreePath } = event.payload;
const key = normalizePath(worktreePath);
Expand Down Expand Up @@ -424,7 +440,7 @@ export function useDevServers({ projectPath }: UseDevServersOptions) {
});

return unsubscribe;
}, [clearPortDetectionTimer, startPortDetectionTimer]);
}, [clearPortDetectionTimer, startPortDetectionTimer, recordGlobalEvent]);

// Cleanup all port detection timers on unmount
useEffect(() => {
Expand Down
21 changes: 12 additions & 9 deletions apps/ui/src/lib/http-api-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -923,17 +923,20 @@ export class HttpApiClient implements ElectronAPI {
this.ws.onmessage = (event) => {
try {
const data = JSON.parse(event.data);
logger.info(
'WebSocket message:',
data.type,
'hasPayload:',
!!data.payload,
'callbacksRegistered:',
this.eventCallbacks.has(data.type)
);
// Only log non-high-frequency events to avoid progressive memory growth
// from accumulated console entries. High-frequency events (dev-server output,
// test runner output, agent progress) fire 10+ times/sec and would generate
// thousands of console entries per minute.
const isHighFrequency =
data.type === 'dev-server:output' ||
data.type === 'test-runner:output' ||
data.type === 'feature:progress' ||
(data.type === 'auto-mode:event' && data.payload?.type === 'auto_mode_progress');
if (!isHighFrequency) {
logger.info('WebSocket message:', data.type);
}
const callbacks = this.eventCallbacks.get(data.type);
if (callbacks) {
logger.info('Dispatching to', callbacks.size, 'callbacks');
callbacks.forEach((cb) => cb(data.payload));
}
} catch (error) {
Expand Down
26 changes: 21 additions & 5 deletions apps/ui/vite.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -238,24 +238,36 @@ export default defineConfig(({ command }) => {
// Inject build hash into sw.js CACHE_NAME for automatic cache busting
swCacheBuster(),
],
// Keep Vite dep-optimization cache local to apps/ui so each worktree gets
// its own pre-bundled dependencies. Shared cache state across worktrees can
// produce duplicate React instances (notably with @xyflow/react) and trigger
// "Invalid hook call" in the graph view.
cacheDir: path.resolve(__dirname, 'node_modules/.vite'),
resolve: {
alias: [
{ find: '@', replacement: path.resolve(__dirname, './src') },
// Force ALL React imports (including from nested deps like zustand@4 inside
// @xyflow/react) to resolve to the single copy in the workspace root node_modules.
// This prevents "Cannot read properties of null (reading 'useState')" caused by
// react-dom setting the hooks dispatcher on one React instance while component
// code reads it from a different instance.
// @xyflow/react) to resolve to a single copy.
// Explicit subpath aliases must come BEFORE the broad regex so Vite's
// first-match-wins resolution applies the specific match first.
{
find: /^react-dom(\/|$)/,
replacement: path.resolve(__dirname, '../../node_modules/react-dom') + '/',
},
{
find: 'react/jsx-runtime',
replacement: path.resolve(__dirname, '../../node_modules/react/jsx-runtime.js'),
},
{
find: 'react/jsx-dev-runtime',
replacement: path.resolve(__dirname, '../../node_modules/react/jsx-dev-runtime.js'),
},
{
find: /^react(\/|$)/,
replacement: path.resolve(__dirname, '../../node_modules/react') + '/',
},
],
dedupe: ['react', 'react-dom'],
dedupe: ['react', 'react-dom', 'zustand', 'use-sync-external-store', '@xyflow/react'],
},
server: {
host: process.env.HOST || '0.0.0.0',
Expand Down Expand Up @@ -355,8 +367,12 @@ export default defineConfig(({ command }) => {
include: [
'react',
'react-dom',
'react/jsx-runtime',
'react/jsx-dev-runtime',
'use-sync-external-store',
'use-sync-external-store/shim',
'use-sync-external-store/shim/with-selector',
'zustand',
'@xyflow/react',
],
},
Expand Down
Loading