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
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ export function useBoardEffects({
if (featuresFingerprint && !isLoading) {
checkAllContexts();
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [featuresFingerprint, isLoading, checkContextExists, setFeaturesWithContext]);

// Re-check context when a feature stops, completes, or errors
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useRef, useCallback, useState } from 'react';
import { useEffect, useRef, useCallback, useState, useMemo } from 'react';
import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog';
import { Button } from '@/components/ui/button';
import {
Expand Down Expand Up @@ -54,6 +54,8 @@ export function DevServerLogsPanel({

const {
logs,
logsVersion,
didTrim,
isRunning,
isLoading,
error,
Expand Down Expand Up @@ -81,8 +83,9 @@ export function DevServerLogsPanel({
return;
}

// If logs got shorter (e.g., cleared), rewrite all
if (logs.length < lastLogsLengthRef.current) {
// If logs got shorter (e.g., cleared) or buffer was trimmed (content shifted),
// do a full rewrite so the terminal stays in sync
if (logs.length < lastLogsLengthRef.current || didTrim) {
xtermRef.current.write(logs);
lastLogsLengthRef.current = logs.length;
return;
Expand All @@ -94,7 +97,7 @@ export function DevServerLogsPanel({
xtermRef.current.append(newContent);
lastLogsLengthRef.current = logs.length;
}
}, [logs, worktree?.path]);
}, [logs, logsVersion, didTrim, worktree?.path]);

// Reset when panel opens with a new worktree
useEffect(() => {
Expand Down Expand Up @@ -123,10 +126,19 @@ export function DevServerLogsPanel({
}
}, []);

const lineCount = useMemo(() => {
if (!logs) return 0;
// Count newlines directly instead of allocating a split array
let count = 1;
for (let i = 0; i < logs.length; i++) {
if (logs.charCodeAt(i) === 10) count++;
}
return count;
}, [logs]);

if (!worktree) return null;

const formattedStartTime = formatStartedAt(startedAt);
const lineCount = logs ? logs.split('\n').length : 0;

return (
<Dialog open={open} onOpenChange={(isOpen) => !isOpen && onClose()}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,16 @@ import { pathsEqual } from '@/lib/utils';

const logger = createLogger('DevServerLogs');

// Maximum log buffer size (characters) - matches server-side MAX_SCROLLBACK_SIZE
const MAX_LOG_BUFFER_SIZE = 50_000; // ~50KB

export interface DevServerLogState {
/** The log content (buffered + live) */
logs: string;
/** Incremented whenever logs content changes (including trim+shift) */
logsVersion: number;
/** True when the latest append caused head truncation */
didTrim: boolean;
/** Whether the server is currently running */
isRunning: boolean;
/** Whether initial logs are being fetched */
Expand Down Expand Up @@ -52,6 +59,8 @@ interface UseDevServerLogsOptions {
export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevServerLogsOptions) {
const [state, setState] = useState<DevServerLogState>({
logs: '',
logsVersion: 0,
didTrim: false,
isRunning: false,
isLoading: false,
error: null,
Expand Down Expand Up @@ -123,6 +132,8 @@ export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevS
const clearLogs = useCallback(() => {
setState({
logs: '',
logsVersion: 0,
didTrim: false,
isRunning: false,
isLoading: false,
error: null,
Expand All @@ -136,13 +147,27 @@ export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevS
}, []);

/**
* Append content to logs
* 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) => ({
...prev,
logs: prev.logs + content,
}));
setState((prev) => {
const combined = prev.logs + content;
const didTrim = combined.length > MAX_LOG_BUFFER_SIZE;
let newLogs = combined;
if (didTrim) {
const slicePoint = combined.length - MAX_LOG_BUFFER_SIZE;
// Find the next newline after the slice point to avoid cutting a line in half
const firstNewlineIndex = combined.indexOf('\n', slicePoint);
newLogs = combined.slice(firstNewlineIndex > -1 ? firstNewlineIndex + 1 : slicePoint);
}
return {
...prev,
logs: newLogs,
didTrim,
logsVersion: prev.logsVersion + 1,
};
});
}, []);

// Fetch initial logs when worktreePath changes
Expand Down
1 change: 1 addition & 0 deletions apps/ui/tests/features/feature-deep-link.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ test.describe('Feature Deep Link', () => {
let projectPath: string;
let projectName: string;

// eslint-disable-next-line no-empty-pattern
test.beforeEach(async ({}, testInfo) => {
projectName = `test-project-${testInfo.workerIndex}-${Date.now()}`;
projectPath = path.join(TEST_TEMP_DIR, projectName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
waitForNetworkIdle,
authenticateForTests,
handleLoginScreenIfPresent,
dismissSandboxWarningIfVisible,
} from '../../utils';

const TEST_TEMP_DIR = createTempDirPath('responsive-modal-test');
Expand Down Expand Up @@ -100,6 +101,9 @@ test.describe('AgentOutputModal Responsive Behavior', () => {

await expect(page.locator('[data-testid="board-view"]')).toBeVisible({ timeout: 10000 });

// Dismiss sandbox warning dialog if it appears (blocks pointer events)
await dismissSandboxWarningIfVisible(page);

// Wait for the verified feature card to appear
const featureCard = page.locator(`[data-testid="kanban-card-${featureId}"]`);
await expect(featureCard).toBeVisible({ timeout: 10000 });
Expand Down
4 changes: 4 additions & 0 deletions apps/ui/tests/features/success-log-contrast.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
waitForNetworkIdle,
authenticateForTests,
handleLoginScreenIfPresent,
dismissSandboxWarningIfVisible,
} from '../utils';

/**
Expand Down Expand Up @@ -109,6 +110,9 @@ test.describe('Success log output contrast', () => {

await expect(page.locator('[data-testid="board-view"]')).toBeVisible({ timeout: 10000 });

// Dismiss sandbox warning dialog if it appears (blocks pointer events)
await dismissSandboxWarningIfVisible(page);

// Wait for the verified feature card to appear
const featureCard = page.locator(`[data-testid="kanban-card-${featureId}"]`);
await expect(featureCard).toBeVisible({ timeout: 10000 });
Expand Down
20 changes: 10 additions & 10 deletions apps/ui/tests/settings/event-hooks-settings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
*/

import { test, expect, type Page } from '@playwright/test';
import { authenticateForTests, navigateToSettings } from '../utils';
import { authenticateForTests, navigateToSettings, waitForSuccessToast } from '../utils';

// Timeout constants for maintainability
const TIMEOUTS = {
Expand Down Expand Up @@ -224,6 +224,9 @@ test.describe('Event Hooks Settings', () => {
const addButton = dialog.locator('button:has-text("Add Endpoint")').last();
await addButton.click();

// Wait for the success toast to confirm the save completed (including API call)
await waitForSuccessToast(page, 'Endpoint added', { timeout: 10000 });

// Dialog should close
await expect(dialog).toBeHidden({ timeout: TIMEOUTS.dialogHidden });

Expand Down Expand Up @@ -256,16 +259,13 @@ test.describe('Event Hooks Settings', () => {
// The endpoints tab should show either existing endpoints or the empty state
// The key is that it should NOT show "empty" if there are endpoints on the server

// Either we see "No endpoints configured" OR we see endpoint cards
const emptyState = page.locator('text=No endpoints configured');
// Either we see "No ntfy endpoints configured" OR we see endpoint cards
const emptyState = page.locator('text=No ntfy endpoints configured');
const endpointCard = page.locator('[data-testid="endpoint-card"]').first();

// One of these should be visible
await expect(
Promise.race([
emptyState.waitFor({ state: 'visible', timeout: 5000 }).then(() => 'empty'),
endpointCard.waitFor({ state: 'visible', timeout: 5000 }).then(() => 'card'),
])
).resolves.toBeDefined();
// One of these should be visible (use Playwright's .or() to match either locator)
await expect(emptyState.or(endpointCard)).toBeVisible({
timeout: TIMEOUTS.endpointVisible,
});
});
});
21 changes: 21 additions & 0 deletions apps/ui/tests/utils/components/dialogs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,27 @@ import { Page, Locator } from '@playwright/test';
import { clickElement } from '../core/interactions';
import { waitForElement, waitForElementHidden } from '../core/waiting';

/**
* Dismiss the sandbox warning dialog if it appears.
* This dialog blocks pointer events and must be accepted before interacting
* with elements behind it.
*/
export async function dismissSandboxWarningIfVisible(page: Page): Promise<void> {
const sandboxAcceptBtn = page.locator('button:has-text("I Accept the Risks")');
const sandboxVisible = await sandboxAcceptBtn
.waitFor({ state: 'visible', timeout: 2000 })
.then(() => true)
.catch(() => false);
if (sandboxVisible) {
await sandboxAcceptBtn.click();
await page
.locator('[role="dialog"][data-state="open"]')
.first()
.waitFor({ state: 'hidden', timeout: 3000 })
.catch(() => {});
Comment on lines +19 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, check the actual file structure and lines in question
echo "=== File content: apps/ui/tests/utils/components/dialogs.ts ==="
cat -n apps/ui/tests/utils/components/dialogs.ts

Repository: AutoMaker-Org/automaker

Length of output: 7804


🏁 Script executed:

#!/bin/bash
# Verify how this helper is used
echo "=== Usage of dismissSandboxWarningIfVisible ==="
rg -n -C3 'dismissSandboxWarningIfVisible\(' apps/ui/tests 2>/dev/null || echo "No matches found"

echo -e "\n=== Silent catch patterns in dialogs.ts ==="
rg -n -C3 'waitFor\(\{ state: .hidden.|catch\(\(\) => \{\}\)' apps/ui/tests/utils/components/dialogs.ts 2>/dev/null || echo "No matches found"

Repository: AutoMaker-Org/automaker

Length of output: 2669


🏁 Script executed:

#!/bin/bash
# Check for other similar patterns in test utilities
echo "=== Other silent catch patterns in test utils ==="
rg -n 'catch\(\(\) => \{\}\)' apps/ui/tests/ --type ts --type tsx 2>/dev/null | head -20 || echo "Pattern search failed"

Repository: AutoMaker-Org/automaker

Length of output: 116


Scope dialog dismissal to the specific sandbox dialog and expose timeout failures.

Line 19 targets the first open dialog globally via .first(), and line 22's silent catch hides timeout errors. If the sandbox dialog fails to close, the error is masked—but subsequent tests will fail with pointer-blocking errors, introducing hard-to-debug flakiness. Filter the wait to the specific dialog containing the accept button and remove the silent catch so dismissal failures surface immediately.

Proposed fix
 export async function dismissSandboxWarningIfVisible(page: Page): Promise<void> {
   const sandboxAcceptBtn = page.locator('button:has-text("I Accept the Risks")');
+  const sandboxDialog = page.locator('[role="dialog"]').filter({ has: sandboxAcceptBtn });
   const sandboxVisible = await sandboxAcceptBtn
     .waitFor({ state: 'visible', timeout: 2000 })
     .then(() => true)
     .catch(() => false);
   if (sandboxVisible) {
     await sandboxAcceptBtn.click();
-    await page
-      .locator('[role="dialog"][data-state="open"]')
-      .first()
-      .waitFor({ state: 'hidden', timeout: 3000 })
-      .catch(() => {});
+    await sandboxDialog.waitFor({ state: 'hidden', timeout: 3000 });
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/ui/tests/utils/components/dialogs.ts` around lines 19 - 22, The waitFor
call currently uses
.locator('[role="dialog"][data-state="open"]').first().waitFor(...) which
targets the first global open dialog and swallows errors via .catch(() => {});
update the selector to scope to the sandbox dialog that contains the accept
button (e.g., locate the accept button and call locator('..') or use a
has/hasText filter to target the dialog that contains that button) and remove
the .catch so the waitFor timeout will throw on failure; ensure you keep the
.waitFor({ state: 'hidden', timeout: 3000 }) invocation but let its errors
surface instead of being caught silently.

}
}

/**
* Check if the add feature dialog is visible
*/
Expand Down
1 change: 1 addition & 0 deletions apps/ui/tests/utils/project/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ export async function setupRealProject(
theme: 'dark',
sidebarOpen: true,
maxConcurrency: 3,
skipSandboxWarning: true,
};
localStorage.setItem('automaker-settings-cache', JSON.stringify(settingsCache));

Expand Down
Loading