feat: add dev server log panel with real-time streaming #474
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request introduces comprehensive dev server log viewing functionality across the application stack. The backend adds log buffering with configurable scrollback history, throttled output emission via EventEmitter, and new API endpoints to fetch buffered logs and stream live output. The frontend introduces an XtermLogViewer component for terminal-style rendering, a DevServerLogsPanel modal for interactive log viewing, and a useDevServerLogs hook managing log state and WebSocket event subscriptions. Supporting API layers add typed interfaces for dev-server events and extend the Electron/HTTP client API surface. Changes
Sequence DiagramssequenceDiagram
participant Client as UI Client
participant Server as Express Server
participant Service as DevServerService
participant DB as Scrollback Buffer
Client->>Server: GET /dev-server-logs?worktreePath=...
Server->>Service: getServerLogs(worktreePath)
Service->>DB: Retrieve scrollback buffer
DB-->>Service: Return buffered logs
Service-->>Server: Return logs + metadata
Server-->>Client: 200 {logs, port, startedAt}
sequenceDiagram
participant Child as Child Process
participant Service as DevServerService
participant Emitter as EventEmitter
participant Client as UI Client
Child->>Service: stdout/stderr data
Service->>Service: handleProcessOutput()
Service->>Service: Append to scrollback + buffer
Service->>Service: Schedule throttled flush
Service->>Emitter: flushOutput() emits batch
Emitter-->>Client: WebSocket dev-server:output
Client->>Client: useDevServerLogs appends
Client->>Client: XtermLogViewer renders
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @Shironex, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant enhancement by providing a comprehensive dev server log panel. Users can now monitor their development server's output in real-time, complete with rich ANSI color formatting. The system intelligently buffers historical logs and manages output flow to ensure a smooth and responsive user experience, making it easier to debug and observe server behavior directly within the application. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a great feature addition, enabling real-time log viewing for dev servers. The implementation is solid, with good attention to performance on both the server (output throttling) and client (xterm.js with WebGL, dynamic imports). The new components and hooks on the UI side are well-structured and provide a good user experience. I have a few suggestions to improve maintainability and type safety.
| if (!worktreePath) { | ||
| res.status(400).json({ | ||
| success: false, | ||
| error: 'worktreePath query parameter is required', | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
This worktreePath validation appears to be redundant. The route is already protected by the validatePathParams('worktreePath') middleware in apps/server/src/routes/worktree/index.ts, which should handle this check. Removing this block will make the handler cleaner and centralize the validation logic in the middleware.
| devProcess.on('error', (error) => { | ||
| logger.error(`Process error:`, error); | ||
| status.error = error.message; | ||
|
|
||
| // Clean up flush timeout | ||
| if (serverInfo.flushTimeout) { | ||
| clearTimeout(serverInfo.flushTimeout); | ||
| serverInfo.flushTimeout = null; | ||
| } | ||
|
|
||
| // Emit stopped event with error (only if not already stopping) | ||
| if (this.emitter && !serverInfo.stopping) { | ||
| this.emitter.emit('dev-server:stopped', { | ||
| worktreePath, | ||
| port, | ||
| exitCode: null, | ||
| error: error.message, | ||
| timestamp: new Date().toISOString(), | ||
| }); | ||
| } | ||
|
|
||
| this.allocatedPorts.delete(port); | ||
| this.runningServers.delete(worktreePath); | ||
| }); | ||
|
|
||
| devProcess.on('exit', (code) => { | ||
| logger.info(`Process for ${worktreePath} exited with code ${code}`); | ||
| status.exited = true; | ||
|
|
||
| // Clean up flush timeout | ||
| if (serverInfo.flushTimeout) { | ||
| clearTimeout(serverInfo.flushTimeout); | ||
| serverInfo.flushTimeout = null; | ||
| } | ||
|
|
||
| // Emit stopped event (only if not already stopping - prevents duplicate events) | ||
| if (this.emitter && !serverInfo.stopping) { | ||
| this.emitter.emit('dev-server:stopped', { | ||
| worktreePath, | ||
| port, | ||
| exitCode: code, | ||
| timestamp: new Date().toISOString(), | ||
| }); | ||
| } | ||
|
|
||
| this.allocatedPorts.delete(port); | ||
| this.runningServers.delete(worktreePath); | ||
| }); |
There was a problem hiding this comment.
The cleanup logic for when a process ends (clearing timeouts, releasing ports, and removing from tracking) is duplicated in both the error and exit event handlers. This can be refactored into a single helper function to reduce duplication and improve maintainability.
const cleanupAndEmitStop = (code: number | null, error?: string) => {
if (serverInfo.flushTimeout) {
clearTimeout(serverInfo.flushTimeout);
serverInfo.flushTimeout = null;
}
if (this.emitter && !serverInfo.stopping) {
this.emitter.emit('dev-server:stopped', {
worktreePath,
port,
exitCode: code,
error,
timestamp: new Date().toISOString(),
});
}
this.allocatedPorts.delete(port);
this.runningServers.delete(worktreePath);
};
devProcess.on('error', (error) => {
logger.error(`Process error:`, error);
status.error = error.message;
cleanupAndEmitStop(null, error.message);
});
devProcess.on('exit', (code) => {
logger.info(`Process for ${worktreePath} exited with code ${code}`);
status.exited = true;
cleanupAndEmitStop(code);
});| onDevServerLogEvent: ( | ||
| callback: (event: { | ||
| type: 'dev-server:started' | 'dev-server:output' | 'dev-server:stopped'; | ||
| payload: { | ||
| worktreePath: string; | ||
| port?: number; | ||
| url?: string; | ||
| content?: string; | ||
| exitCode?: number | null; | ||
| error?: string; | ||
| timestamp: string; | ||
| }; | ||
| }) => void | ||
| ) => () => void; |
There was a problem hiding this comment.
The type definition for the onDevServerLogEvent payload is a bit loose, with many optional properties. To improve type safety and provide better autocompletion for consumers of this API, you can use a discriminated union for the event payload. This ensures that each event type has exactly the properties it's supposed to have.
onDevServerLogEvent: (
callback: (
event:
| { type: 'dev-server:started'; payload: { worktreePath: string; port: number; url: string; timestamp: string } }
| { type: 'dev-server:output'; payload: { worktreePath: string; content: string; timestamp: string } }
| { type: 'dev-server:stopped'; payload: { worktreePath: string; port: number; exitCode: number | null; error?: string; timestamp: string } }
) => void
) => () => void;
Shironex
force-pushed
the
feat/dev-server-log-panel
branch
from
6d239be to
1bd803f
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/server/src/middleware/validate-paths.ts (1)
67-70: Consider adding explicit handling for required non-string parameters.For required parameters (no
?suffix), if the value exists but is not a string (e.g., an array or object from query string parsing), the validation silently passes without callingvalidatePath. While this matches the prior behavior and downstream handlers would typically fail on malformed input, consider logging or returning an error for type mismatches on required params.💡 Optional: Explicit type validation for required params
// Handle regular parameters const value = getParamValue(req, paramName); - if (value && typeof value === 'string') { + if (value !== undefined) { + if (typeof value !== 'string') { + res.status(400).json({ + success: false, + error: `Parameter '${paramName}' must be a string`, + }); + return; + } validatePath(value); }apps/ui/src/types/electron.d.ts (1)
981-1007: Well-structured API additions.The type definitions are clear and follow the existing patterns in this file. The event callback signature with discriminated union on
typeenables type-safe event handling.Consider using discriminated unions for the payload to provide stronger type guarantees per event type (e.g.,
portis only present onstarted,contentonly onoutput,exitCodeonly onstopped), though the current approach with optional fields is pragmatic and consistent with other event types in this file.apps/ui/src/components/views/board-view/worktree-panel/components/dev-server-logs-panel.tsx (1)
271-277: Consider using theButtoncomponent for consistency.The "Scroll to bottom" button uses a native
<button>element while other buttons in the header use theButtoncomponent. For visual and behavioral consistency, consider using theButtoncomponent here.♻️ Suggested change
- <button + <Button + variant="ghost" + size="sm" onClick={scrollToBottom} - className="inline-flex items-center gap-1.5 px-2 py-1 rounded hover:bg-muted transition-colors text-primary" + className="h-auto px-2 py-1 text-xs text-primary" > <ArrowDown className="w-3 h-3" /> Scroll to bottom - </button> + </Button>apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts (1)
71-118: Consider adding stale response protection.If
worktreePathchanges while a fetch is in-flight, the old response could update state with logs from the previous worktree. This could cause a brief flash of incorrect data before the new fetch completes.♻️ Suggested fix with abort controller or stale check
const fetchLogs = useCallback(async () => { if (!worktreePath) return; + const currentPath = worktreePath; // Capture for stale check setState((prev) => ({ ...prev, isLoading: true, error: null })); try { const api = getElectronAPI(); if (!api?.worktree?.getDevServerLogs) { setState((prev) => ({ ...prev, isLoading: false, error: 'Dev server logs API not available', })); return; } const result = await api.worktree.getDevServerLogs(worktreePath); + // Check if worktreePath changed during fetch + if (currentPath !== worktreePath) return; + if (result.success && result.result) {Note: Since
worktreePathis captured in the closure, you'd need to use a ref to track the current path, or restructure to useuseReffor the latest path value.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/server/src/index.tsapps/server/src/middleware/validate-paths.tsapps/server/src/routes/worktree/index.tsapps/server/src/routes/worktree/routes/dev-server-logs.tsapps/server/src/services/dev-server-service.tsapps/ui/src/components/ui/xterm-log-viewer.tsxapps/ui/src/components/views/board-view/worktree-panel/components/dev-server-logs-panel.tsxapps/ui/src/components/views/board-view/worktree-panel/components/index.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/board-view/worktree-panel/hooks/index.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.tsapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsxapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/types/electron.d.tslibs/types/src/event.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/ui/src/components/views/board-view/worktree-panel/hooks/index.tsapps/server/src/routes/worktree/routes/dev-server-logs.tsapps/ui/src/components/views/board-view/worktree-panel/components/index.tsapps/ui/src/types/electron.d.tsapps/ui/src/components/ui/xterm-log-viewer.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.tsapps/server/src/index.tsapps/ui/src/components/views/board-view/worktree-panel/components/dev-server-logs-panel.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxlibs/types/src/event.tsapps/ui/src/lib/electron.tsapps/server/src/routes/worktree/index.tsapps/server/src/middleware/validate-paths.tsapps/server/src/services/dev-server-service.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
apps/ui/src/components/views/board-view/worktree-panel/hooks/index.tsapps/server/src/routes/worktree/routes/dev-server-logs.tsapps/ui/src/components/views/board-view/worktree-panel/components/index.tsapps/ui/src/types/electron.d.tsapps/ui/src/components/ui/xterm-log-viewer.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.tsapps/server/src/index.tsapps/ui/src/components/views/board-view/worktree-panel/components/dev-server-logs-panel.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxlibs/types/src/event.tsapps/ui/src/lib/electron.tsapps/server/src/routes/worktree/index.tsapps/server/src/middleware/validate-paths.tsapps/server/src/services/dev-server-service.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx
apps/server/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
createEventEmitter()fromlib/events.tsfor all server operations to emit events that stream to frontend via WebSocket
Files:
apps/server/src/routes/worktree/routes/dev-server-logs.tsapps/server/src/index.tsapps/server/src/routes/worktree/index.tsapps/server/src/middleware/validate-paths.tsapps/server/src/services/dev-server-service.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Applies to apps/server/src/**/*.{ts,tsx} : Use `createEventEmitter()` from `lib/events.ts` for all server operations to emit events that stream to frontend via WebSocket
📚 Learning: 2025-12-28T05:07:48.147Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Applies to apps/server/src/**/*.{ts,tsx} : Use `createEventEmitter()` from `lib/events.ts` for all server operations to emit events that stream to frontend via WebSocket
Applied to files:
apps/ui/src/types/electron.d.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.tsapps/server/src/index.tslibs/types/src/event.tsapps/server/src/services/dev-server-service.tsapps/ui/src/lib/http-api-client.ts
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/ui/xterm-log-viewer.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/components/dev-server-logs-panel.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx
🧬 Code graph analysis (8)
apps/server/src/routes/worktree/routes/dev-server-logs.ts (1)
apps/server/src/services/dev-server-service.ts (1)
getDevServerService(659-664)
apps/ui/src/components/ui/xterm-log-viewer.tsx (1)
apps/ui/src/config/terminal-themes.ts (2)
getTerminalTheme(568-578)DEFAULT_TERMINAL_FONT(60-60)
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (2)
libs/types/src/settings.ts (1)
WorktreeInfo(564-575)apps/ui/src/components/views/board-view/worktree-panel/types.ts (1)
WorktreeInfo(9-18)
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts (1)
apps/ui/src/lib/electron.ts (1)
getElectronAPI(909-918)
apps/server/src/index.ts (1)
apps/server/src/services/dev-server-service.ts (1)
getDevServerService(659-664)
apps/server/src/routes/worktree/index.ts (2)
apps/server/src/middleware/validate-paths.ts (1)
validatePathParams(38-87)apps/server/src/routes/worktree/routes/dev-server-logs.ts (1)
createGetDevServerLogsHandler(13-52)
apps/server/src/services/dev-server-service.ts (1)
apps/server/src/lib/events.ts (1)
EventEmitter(13-16)
apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx (5)
apps/ui/src/types/electron.d.ts (1)
WorktreeInfo(625-630)libs/types/src/settings.ts (1)
WorktreeInfo(564-575)apps/ui/src/components/views/board-view/worktree-panel/types.ts (1)
WorktreeInfo(9-18)apps/ui/src/components/views/board-view/worktree-panel/components/dev-server-logs-panel.tsx (1)
DevServerLogsPanel(44-289)apps/ui/src/components/views/board-view/worktree-panel/components/index.ts (1)
DevServerLogsPanel(2-2)
🔇 Additional comments (42)
apps/ui/src/lib/electron.ts (1)
1763-1777: LGTM!The mock implementations for
getDevServerLogsandonDevServerLogEventare well-structured and consistent with other mock methods in this file. The console logging for subscribe/unsubscribe actions aids development debugging, and the return types align with the expected API contract.apps/server/src/middleware/validate-paths.ts (1)
13-23: LGTM!The
getParamValuehelper cleanly abstracts parameter extraction with correct priority (body first for POST/PUT, then query for GET). Theunknownreturn type appropriately enforces type checking at call sites.apps/ui/src/components/views/board-view/worktree-panel/components/index.ts (1)
1-4: LGTM!The new
DevServerLogsPanelexport is correctly added to the barrel file, maintaining alphabetical ordering and enabling clean imports from the parent module.apps/ui/src/components/views/board-view/worktree-panel/hooks/index.ts (1)
1-7: LGTM!The
useDevServerLogshook export is correctly added to the barrel file, positioned logically near the relateduseDevServersexport.apps/server/src/routes/worktree/index.ts (2)
101-105: LGTM!The new GET route for
/dev-server-logsis correctly configured:
- Uses GET method appropriate for read operations
- Validates
worktreePathfrom query params (leveraging the updated middleware)- Grouped logically with other dev server routes
36-36: LGTM!Import follows the established pattern for route handlers in this file.
apps/server/src/index.ts (2)
70-70: LGTM!The import follows the existing pattern for service imports in this file.
180-182: DevServerService cleanup is already handled via built-in signal handlers.The service registers its own SIGTERM/SIGINT handlers (lines 669–675 of dev-server-service.ts) that call
stopAll(), so no additional cleanup is needed in the main shutdown handlers. Orphaned processes are automatically stopped when the service receives shutdown signals.libs/types/src/event.ts (1)
45-48: LGTM!The new event types follow the established
namespace:actionnaming convention and align well with the existing event type patterns (e.g.,worktree:init-*).apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx (2)
1-1: Good improvement!Replacing
// @ts-nocheckwith an explicitimport type { JSX } from 'react'is a proper fix that maintains type safety.
48-48: LGTM!The new
onViewDevServerLogsprop is correctly added to the interface, destructured in the component, and forwarded toWorktreeActionsDropdown. This follows the established pattern for other action callbacks in this component.Also applies to: 91-91, 342-342
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (3)
28-28: LGTM!The
ScrollTexticon is a suitable choice for a "View Logs" action.
60-60: LGTM!The new prop is correctly added to the interface and component signature, following the established pattern for action callbacks.
Also applies to: 88-88
153-156: LGTM!The "View Logs" menu item is well-placed in the Dev Server Running section, following the established styling patterns (
text-xs, icon sizing). It logically appears after "Open in Browser" and before "Stop Dev Server".apps/server/src/routes/worktree/routes/dev-server-logs.ts (1)
13-51: Handler implementation looks correct.The endpoint follows the established pattern in the codebase with proper error handling and response structure. The
validatePathParams('worktreePath')middleware is applied to the route and correctly validates the path parameter passed in the query string, protecting against path traversal attacks.apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx (4)
15-15: LGTM!Clean import from the components barrel export, following the established pattern in this file.
87-90: LGTM!State management for the log panel follows the established patterns. Using separate state for visibility and selected worktree allows the panel to maintain smooth close animations.
171-181: LGTM!Well-implemented handlers with appropriate
useCallbackusage. The comment explaining whylogPanelWorktreeis preserved during close (for animation smoothness) is helpful.
324-332: LGTM!The
DevServerLogsPanelis correctly wired with all necessary props. Since it uses aDialogcomponent (which portals to the document body), its placement in the JSX tree doesn't affect rendering.apps/ui/src/components/views/board-view/worktree-panel/components/dev-server-logs-panel.tsx (4)
1-18: LGTM!Imports are well-organized, using the
@/alias paths as required by coding guidelines. The component pulls in the appropriate UI primitives and the custom hook for log management.
56-70: LGTM!Smart conditional logic that only activates the hook's API calls and WebSocket subscriptions when the panel is open, preventing unnecessary resource usage.
72-98: LGTM!The effect logic handles all edge cases correctly:
- Worktree changes trigger a full rewrite
- Log clearing (shorter logs) triggers a full rewrite
- New content is incrementally appended for efficiency
The ref-based tracking is a good pattern for avoiding unnecessary rewrites.
129-130: LGTM!The line count calculation correctly handles the empty string case since
''is falsy in JavaScript, resulting in0lines.apps/ui/src/components/ui/xterm-log-viewer.tsx (5)
1-4: LGTM!Imports are correctly using
@/alias paths as required by coding guidelines. The component integrates with the app store for theme management.
88-171: Note:initialContentis only applied on initial mount.The terminal initialization effect runs once (empty dependency array) and applies
initialContentonly during that first render. IfinitialContentchanges after mount, it won't be reflected. This is likely intentional given thewritemethod exists for updates, but worth noting for consumers of this component.The lazy-loading pattern for xterm.js and graceful WebGL fallback are well implemented.
189-214: LGTM!Robust resize handling with proper cleanup. The dimension check before fitting prevents errors when the container is hidden or has zero size.
216-238: LGTM with a minor caveat.The scroll monitoring implementation is solid with passive listeners for performance. The 10px threshold for detecting "at bottom" is reasonable for handling sub-pixel rendering.
Note: The reliance on
.xterm-viewportinternal class name couples this to xterm.js implementation details, but this is a common pattern and unlikely to break.
266-276: LGTM!The
writemethod correctly usesreset()before writing to clear both the visible content and scrollback buffer, providing clean "replace all" semantics. The pending content queue ensures content isn't lost if methods are called before the terminal is ready.apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts (4)
1-6: LGTM!Imports correctly use shared packages (
@automaker/utils/logger) and@/alias paths as required by coding guidelines.
158-213: LGTM!The WebSocket subscription is well-implemented:
- Uses
pathsEqualfor cross-platform path comparison (handles Windows vs Unix paths)- Properly cleans up by returning the unsubscribe function
- Event handling covers all three lifecycle states
The pattern of clearing logs on
dev-server:startedensures a clean slate when the server restarts.
148-156: LGTM!The effect correctly triggers log fetching when the worktree path changes or subscription is enabled, and clears state when disabled. The dependency array is complete.
215-220: LGTM!The hook exposes a clean API with both state and action methods, following established React patterns. The returned
fetchLogsallows manual refresh,clearLogsenables explicit reset, andappendLogssupports programmatic log injection if needed.apps/ui/src/lib/http-api-client.ts (2)
513-559: LGTM! Well-structured event types and interfaces.The new dev-server event types and interfaces are well-defined with appropriate discriminated union typing for
DevServerLogEvent. The type definitions match the expected server-side payloads and follow the existing patterns in the codebase.
1759-1776: LGTM! Clean subscription pattern.The implementation correctly follows the existing pattern used by
onInitScriptEvent(lines 1788-1809). The combined unsubscribe function properly cleans up all three event subscriptions, preventing memory leaks.apps/server/src/services/dev-server-service.ts (8)
66-131: Solid buffer management and throttling implementation.The scrollback buffer correctly enforces the size limit by evicting oldest data. The output throttling at ~250fps with 4KB batches provides a good balance between responsiveness and preventing WebSocket flooding.
One minor note:
outputBufferhas no explicit size limit and could theoretically grow large under extreme output conditions (e.g., a misbehaving dev server flooding stdout). However, given the 4ms flush interval, this window is very small and unlikely to cause issues in practice.
346-368: Good additions for livereload cleanup and ANSI color support.The livereload port cleanup prevents port conflicts with HMR/livereload servers that use fixed ports. The
FORCE_COLOR,COLORTERM, andTERMenvironment variables enable colored output for proper ANSI rendering in xterm.js.
380-406: LGTM! Clean handler setup with early serverInfo creation.Creating
serverInfoearly allows the stdout/stderr handlers to reference it correctly. The handlers properly route throughhandleProcessOutputfor buffer management and event emission.
408-455: Proper cleanup and event emission in error/exit handlers.Both handlers correctly:
- Clear the
flushTimeoutto prevent memory leaks- Check
!serverInfo.stoppingto prevent duplicatedev-server:stoppedevents- Clean up allocated ports and running servers map
477-485: Correct timing for the started event.The
dev-server:startedevent is emitted only after verifying the process didn't fail immediately (checked at lines 460-472), ensuring clients receive accurate lifecycle notifications.
521-543: Well-implemented stop sequence.The stop logic correctly:
- Sets
stopping = truefirst to block further output events- Clears the flush timeout and output buffer
- Emits
dev-server:stoppedimmediately for responsive UI updates- Then kills the process
The
stoppingflag prevents the exit handler from emitting a duplicate stopped event when the process terminates.
602-635: Clean implementation of log retrieval API.The
getServerLogsmethod correctly returns the scrollback buffer for replay on client reconnection, with appropriate error handling when no server is running.
15-60: Acceptable pattern for receiving the event emitter.The service receives the
EventEmittervia dependency injection rather than creating it internally. The emitter is created centrally usingcreateEventEmitter()fromlib/events.tsinapps/server/src/index.ts(line 165) and passed to the service during initialization viasetEventEmitter()(line 182). This pattern satisfies the coding guideline requirement and maintains a single event bus for the application.
Add the ability to view dev server logs in a dedicated panel with: - Real-time log streaming via WebSocket events - ANSI color support using xterm.js - Scrollback buffer (50KB) for log history on reconnect - Output throttling to prevent UI flooding - "View Logs" option in worktree dropdown menu Server changes: - Add scrollback buffer and event emission to DevServerService - Add GET /api/worktree/dev-server-logs endpoint - Add dev-server:started, dev-server:output, dev-server:stopped events UI changes: - Add reusable XtermLogViewer component - Add DevServerLogsPanel dialog component - Add useDevServerLogs hook for WebSocket subscription Closes #462 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Shironex
force-pushed
the
feat/dev-server-log-panel
branch
from
1bd803f to
073f6d5
Compare
Resolved conflict in worktree-panel.tsx by combining imports: - DevServerLogsPanel from this branch - WorktreeMobileDropdown, WorktreeActionsDropdown, BranchSwitchDropdown from v0.11.0rc Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Changes
Server
DevServerServicewith scrollback buffer and WebSocket event emissionGET /api/worktree/dev-server-logsendpoint for fetching buffered logsdev-server:started,dev-server:output,dev-server:stoppedUI
XtermLogViewercomponent for rendering terminal outputDevServerLogsPaneldialog accessible via "View Logs" in worktree dropdownuseDevServerLogshook for WebSocket subscription and state managementTest plan
Closes #462
Preview
Summary by CodeRabbit
New Features
✏️ Tip: You can customize this high-level summary in your review settings.