Fix #3: Migrate dashboard UI to React 18 + Vite SPA#4
Conversation
- Add React 18 + Vite 5 frontend in frontend/ directory with Tailwind CSS, React Query v5, Lucide React icons, and date-fns - Implement full component tree: layout (Header, WorkspaceSwitcher, TabNav), metrics (MetricsBar, MetricCard), agents (AgentCard, AgentLogViewer), issues (IssueQueue), PRs (PRTracker), modals (AddWorkspace, WorkspaceSettings, EnvEditor), and shared UI primitives (Card, Badge, Button, Modal, Spinner) - Preserve existing dark color palette via CSS custom properties in tokens.css - Add SPA catch-all route in dashboard.py (registered after all /api/* routes) so React client-side navigation works on direct URL loads - Add build-ui command to run.sh (cd frontend && npm install && npm run build) and integrate it into the install flow - Update .gitignore to exclude frontend/node_modules/ and frontend/dist/ - Build outputs to orchestrator/static/ via vite.config.js outDir setting Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 6 potential bugs in this PR.
high: 3 | medium: 2 | low: 1
The PR introduces a React frontend dashboard with several concrete bugs: a log viewer that ignores its pagination cursor and re-fetches all events on every poll, a PR link pointing to the wrong URL, a settings form that can overwrite workspace data with empty strings, silent error swallowing in the env editor, a potential interval leak on agent status transitions, and a backend SPA fallback that can 500 when the frontend bundle is absent.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 3 potential bugs in this PR.
medium: 1 | low: 2
All six previously reported bugs have been fixed in this revision. Three new bugs were found: log events are replaced rather than accumulated on each poll (losing history), log polling runs unconditionally even for terminal agents, and workspace update API errors are silently discarded with no user feedback.
- AgentLogViewer: accumulate events with ID-based deduplication instead of replacing on each poll - AgentLogViewer: gate polling on isRunning prop so polling stops when agent is done - WorkspaceSettingsModal: add onError handler to update mutation to show error feedback to user Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 4 potential bugs in this PR.
medium: 2 | low: 2
All three previously reported bugs (log event accumulation, unconditional polling, and silent update errors) are now fixed. Four new bugs were found: completed-agent logs are never fetched due to enabled: isRunning fully disabling the query; the since cursor in the React Query key causes spurious loading states on each poll; the delete-workspace mutation silently ignores errors; and the updateError state is not cleared when switching workspaces.
- AgentLogViewer: allow initial fetch for completed agents by removing `enabled: isRunning` gate; polling interval still gated on isRunning - AgentLogViewer/useAgentLogs: remove `since` from query key to prevent spurious loading states on every poll that returns new events - WorkspaceSettingsModal: add onError handler to delete mutation so failures surface to the user instead of being silently swallowed - WorkspaceSettingsModal: clear updateError when selected workspace changes to prevent stale error messages from previous workspace Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 1 potential bug in this PR.
low: 1
All four previously reported bugs are fixed. One new low-severity issue was introduced by the fix for bug PRRT_kwDORZvToM50UOIh: the setUpdateError(null) call inside useEffect([workspace]) can clear an active error message whenever the workspace's server data changes (not just when the user switches to a different workspace).
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 3 potential bugs in this PR.
medium: 1 | low: 2
Three bugs found: a stale-closure issue in useAgentLogs where the since cursor is missing from the React Query key (causing the incremental log viewer to never fetch new events past the initial window), a file-upload handler that destructively replaces all env rows instead of merging, and a missing cache invalidation after saving env vars despite useQueryClient being called. The previously reported WorkspaceSettingsModal bug (effect clearing errors on any workspace refetch) is fixed — the dependency is now correctly [workspace?.id].
- Include `since` in useAgentLogs query key so cursor advances trigger fresh fetches - Merge uploaded .env file rows with existing entries instead of replacing them - Invalidate env query cache after successful saveEnv call Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 2 potential bugs in this PR.
medium: 1 | low: 1
Two new bugs found: PRTracker uses a non-unique React list key (pr_number alone) that can cause incorrect DOM reconciliation in multi-repo or all-workspace views; EnvEditor's post-save cache invalidation targets a query key that no hook is registered under, making it a no-op. All three previously reported bugs (missing since in queryKey, file-upload replacing rows instead of merging, and unused queryClient) have been fixed in this PR.
- PRTracker: use composite key `${pr.github_repo}-${pr.pr_number}` to prevent duplicate React keys across repos
- EnvEditor: remove no-op invalidateQueries and use fetchCounter state to trigger re-fetch after save
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 5 potential bugs in this PR.
high: 1 | medium: 1 | low: 3
Five bugs found across four files: a high-severity polling feedback loop caused by including a mutable cursor in a React Query key, a medium-severity fetch race condition from missing effect cleanup, and three low-severity issues (index-based React key, mismatched-quote stripping in env parsing, and a missing preventDefault on a hash link). Both previously reported open bugs are resolved: the PR list key is now a correct composite key, and the EnvEditor has been rewritten without the stale invalidateQueries call.
- Remove `since` from React Query key in useAgentLogs to prevent polling feedback loop - Add cancellation flag to EnvEditor useEffect to prevent race condition on rapid file switches - Fix parseEnvText to use balanced quote matching regex instead of independent quote stripping - Use stable key (thread.id or path-line composite) instead of array index in ReviewThreads - Add preventDefault to AgentCard issue anchor to prevent page scroll on click Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 3 potential bugs in this PR.
medium: 1 | low: 2
All five previously-reported bugs have been fixed. Three new issues were found: a stale closure in the log-polling hook means the since cursor is never advanced so every poll re-fetches all logs from the start; env-editor rows use array index as React key risking stale UI after row deletion; and the workspace settings form won't reinitialize if workspace data is updated while the modal is open.
- useAgentLogs: hold cursor in useRef so queryFn always reads the latest value; expose cursorRef from the hook so AgentLogViewer can advance it without triggering a key change - AgentLogViewer: remove since state, update cursorRef.current directly after each successful fetch - EnvEditor: assign stable crypto.randomUUID() id to each row at parse/add time and use it as the React key instead of array index - WorkspaceSettingsModal: extend useEffect dependency array to include individual workspace fields (name, repo_url, base_branch) so form reinitialises when those fields change while the modal is open Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 3 potential bugs in this PR.
medium: 1 | low: 2
All three previously reported bugs have been fixed in this revision (stale cursor closure via useRef, array-index React keys via crypto.randomUUID(), and incomplete useEffect dependency array). Three new bugs were found: the SPA catch-all route silently swallows unknown API 404s by returning HTML with HTTP 200, a shared mutation isPending flag incorrectly disables all Retry buttons in IssueQueue when any single update is in-flight, and the WorkspaceSettingsModal's confirmDelete state persists across open/close cycles causing the delete confirmation prompt to appear immediately on re-open.
- dashboard.py: guard spa_fallback against api/* paths, returning 404 instead of index.html - IssueQueue.jsx: track per-row retry state with retryingIssue to avoid disabling all Retry buttons during a single mutation - WorkspaceSettingsModal.jsx: reset confirmDelete when modal closes via useEffect on open prop Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 3 potential bugs in this PR.
medium: 2 | low: 1
The PR fixes the three previously reported bugs (SPA 200-on-API-path, shared Retry button loading state, and confirmDelete state leak) and introduces several new features. Three new bugs were found: a narrow edge case in the SPA guard where /api without a trailing slash bypasses the API-path check and serves HTML instead of a 404, a state accumulation issue where agent log entries from a prior agentId bleed into the newly selected agent's view, and a file input not being cleared after upload which prevents re-uploading the same file.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 1 potential bug in this PR.
high: 1
All three previously reported bugs are fixed in this PR. One new high-severity bug is introduced: the SPA catch-all route intercepts browser requests for Vite-built assets at /assets/... (no static mount covers that path) and returns index.html instead of the JS/CSS bundles, breaking the frontend in production.
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 2 potential bugs in this PR.
high: 1 | medium: 1
The PR adds an /assets static mount and SPA catch-all to serve a Vite-built React frontend. Two bugs were found: the new /assets StaticFiles mount will crash the server on startup if the frontend hasn't been built, and a race condition in the agent-log cursor reset can cause the wrong cursor to be used on the first fetch when switching between agents. The previously-reported SPA catch-all/Vite asset interception bug has been fixed.
- dashboard.py: pass check_dir=False to StaticFiles for /assets mount so the server doesn't crash at startup when the frontend hasn't been built yet - useAgents.js: reset cursorRef synchronously during render when agentId changes (using prevAgentIdRef comparison) to prevent TanStack Query from firing the first fetch with the previous agent's stale cursor - AgentLogViewer.jsx: remove redundant cursorRef.current = 0 from effect since the hook now handles the reset synchronously before the query fires Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 2 potential bugs in this PR.
medium: 2
Two new bugs were found: the root / route in the backend can 500 when the frontend is not built (unlike the new SPA fallback which gracefully returns 404), and the agent list pagination offset is not reset when the user switches workspaces. Both previously reported bugs are now fixed: the /assets StaticFiles mount no longer crashes the server at startup (check_dir=False was added), and the agent-log cursor is now reset synchronously during render (not in a racing useEffect) when agentId changes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 1 potential bug in this PR.
medium: 1
Both previously reported bugs (missing file-existence check in index() and missing offset reset on workspace switch) have been fixed. One new medium-severity bug was found: synchronous ref mutation during render in useAgents.js violates React's render-purity contract and can cause incorrect log-cursor behavior under React StrictMode, which is active in this application.
Move ref mutations in useAgentLogs from render phase into a useEffect to comply with React's render-purity contract. The previous synchronous mutation during render broke under StrictMode's double-render: the second pass found prevAgentIdRef already updated so cursorRef was never reset, causing the log viewer to fetch from a stale cursor after agent switches. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 1 potential bug in this PR.
medium: 1
The StrictMode render-purity bug is fixed, but a new race condition remains: TanStack Query schedules its initial queryFn call as a microtask (before React's useEffect macrotasks run), so the cursor is not guaranteed to be 0 for the first fetch after an agent switch, contrary to the code comment. A related effect-ordering issue in AgentLogViewer can also allow stale cached data to overwrite the cursor reset when revisiting a previously-viewed agent.
- dashboard.py: guard ws["github_repo"] for None before passing to get_pr_branch in the fix_review restart path; returns 409 with a descriptive error when the workspace has no github_repo configured - agent_pool.py: decode exit_code via os.waitpid in _monitor_pid so the log line reflects the actual exit status; falls back to None for non-child (reattached) processes that raise ChildProcessError - EnvEditor.jsx: add lastSavedRef to capture saved vars after a successful PUT; add Discard button that reverts to lastSavedRef content instead of relying on a re-fetch that may be suppressed by the dirty guard Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 3 potential bugs in this PR.
medium: 2 | low: 1
All three previously reported bugs have been fixed. Three new bugs were found: a wrong-file content restoration on discard after switching files in EnvEditor, an unthrottled polling loop in usePlanning when the API returns null, and misleading agent IDs containing the literal string 'None' in the resume path of agent_pool.
- EnvEditor: reset lastSavedRef.current to null on activeFile change so Discard after a file switch restores the correct file's server content - usePlanning: apply exponential backoff for null API responses in poll() instead of recursing at a fixed 300 ms interval indefinitely - agent_pool: use 'unknown' sentinel instead of literal "None" in agent ID when issue_number is absent Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 4 potential bugs in this PR.
medium: 2 | low: 2
All three previously-reported bugs are confirmed fixed in this PR. Four new bugs were identified: a path traversal guard edge case in the dashboard, a potentially blocking os.waitpid call in the agent monitor thread, orphan subprocesses from process-group-unaware SIGTERM, and a race window in the polling re-entry guard.
- dashboard.py: use Path.is_relative_to() for path traversal guard to correctly handle exact workspace root match (Thread 1) - agent_pool.py: use os.WNOHANG in os.waitpid to avoid blocking the monitor daemon thread (Thread 2) - dashboard.py: use os.killpg to send SIGTERM/SIGKILL to the entire process group, preventing orphaned child processes (Thread 3) - usePlanning.js: set pollActiveRef.current=true synchronously before setTimeout to close the 300ms re-entry window (Thread 4) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 2 potential bugs in this PR.
medium: 2
Two new medium-severity bugs introduced in the reattach/restart infrastructure: _monitor_pid uses os.kill instead of os.killpg when timing out a reattached agent (orphaning child processes), and _monitor_agent incorrectly fires the completion callback for externally-stopped agents. All four previously-reported bugs have been fixed in this PR.
- Replace os.kill with os.killpg in _monitor_pid to signal entire process group, preventing child processes from being orphaned when reattached agents time out - Remove spurious _on_agent_complete callback from externally-stopped branch in _monitor_agent to avoid triggering PR creation, issue updates, and worktree cleanup for agents deliberately replaced by restart_agent Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 1 potential bug in this PR.
medium: 1
Both previously-reported bugs (orphaned child processes on timeout kill, and _on_agent_complete firing for externally-stopped agents) are fixed. One new medium-severity bug remains: the newly-added _monitor_pid method completes its DB writes for reattached agents without ever invoking _on_agent_complete, so callback-driven downstream automation (e.g. dispatching a fix_review agent after a reattached implement agent creates a PR) never fires for agents that survived an orchestrator restart.
Add _on_reattach_complete callback to AgentPool so that _monitor_pid fires downstream automation (e.g. PR review dispatch) for reattached agents, matching the behaviour of _monitor_agent for fresh agents. - Add _on_reattach_complete attribute with signature (agent_id, agent_type, pr_number, workspace) - Add set_reattach_completion_callback() setter - Track effective_pr_number through all completion paths in _monitor_pid - Fire the callback at the end of _monitor_pid and before the early return in the unpushed-commits→auto-PR path (line ~1125), covering both the implement agent PR-found/auto-created paths and the fix_review completion path mentioned in the review Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 1 potential bug in this PR.
medium: 1
The previously reported _monitor_pid/_on_agent_complete bug is fixed via the new _on_reattach_complete callback mechanism. One new medium-severity bug was found: stopPolling() in usePlanning.js fails to reset pollActiveRef.current, which can permanently block poll() from rescheduling after a cancelGeneration() failure, leaving the UI frozen in a 'generating' state.
Reset pollActiveRef.current in stopPolling() to prevent permanent stuck 'generating' state when cancelGeneration() is called within the 300ms setTimeout window before the poll tick's finally block runs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 2 potential bugs in this PR.
medium: 2
The previously reported stopPolling/pollActiveRef bug is fixed. Two new medium-severity bugs exist in usePlanning.js: poll() silently exits without clearing generating state when its retry budget is exhausted (both in the null-data and catch paths), and resumeSession does not reset generating when session data fails to load, leaving the UI permanently stuck in a generating spinner in both scenarios.
- Call setGenerating(false) in poll()'s !data path when MAX_POLL_ERRORS is exceeded - Call setGenerating(false) in poll()'s catch block when MAX_POLL_ERRORS is exceeded - Call setGenerating(false) in resumeSession's early-return guard for null/error data Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 2 potential bugs in this PR.
low: 1 | medium: 1
Both previously reported generating-spinner bugs in usePlanning.js are fixed. Two new bugs were found: a race condition in IssueQueue where a single retryingIssue string value causes a second concurrent retry's onSettled to clear the first row's spinner prematurely, and a logic error in agent_pool.py where failed reattached fix_review agents set the issue status to in_progress instead of needs_human/pending, permanently stranding those issues.
- Use a Set for retryingIssues in IssueQueue.jsx so concurrent retries don't clear each other's loading indicators prematurely - Set failed reattached fix_review agent issue status to 'needs_human' instead of 'in_progress' to prevent issues from being permanently stuck Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 3 potential bugs in this PR.
low: 3
Both previously reported bugs are fixed: IssueQueue now uses a Set for retryingIssues (concurrent retry spinners no longer interfere), and the reattached fix_review agent failure path now correctly sets status to needs_human. Three new low-severity bugs were found: a race condition in usePlanning.js that can create a duplicate polling loop, an ID mismatch in EnvEditor that unmounts/remounts inputs after save, and a silent scroll listener registration failure in PlannerModal on first open.
- usePlanning.js: Add pollGenRef generation counter so stopPolling() can
bump the generation without risk of the in-flight tick's finally block
clobbering the flag owned by a newer poll() invocation. Each tick
captures its generation after stopPolling() and only resets
pollActiveRef when pollGenRef still matches, eliminating the duplicate
concurrent polling loop described in the review.
- EnvEditor.jsx: Sync row id with the key name as the user types so that
after save + reload (where the server rebuilds rows as
{ id: keyName, key: keyName, value }) the React key prop is stable
across reconciliation, preventing input unmount/remount and focus loss.
- PlannerModal.jsx: Replace useRef chatRef with a useCallback callback
ref backed by useState(chatEl). Because chatEl is a state value, the
scroll-listener useEffect (and the auto-scroll effect) include it in
their dependency arrays and re-run once the portal commits the DOM
node, ensuring the scroll listener is always attached on first open.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 3 potential bugs in this PR.
medium: 2 | low: 1
All three previously reported bugs are fixed. Three new bugs were found: two in usePlanning.js where poll() cancels backoff timers on every recursive tick and sendMessage() lacks a stopPolling() guard, and one in ReviewThreads.jsx where custom sentinel tags can inadvertently match and alter user-authored comment content.
- Split poll() into _pollTick() (internal, no stopPolling on entry) and poll() (public API, always calls stopPolling first). Recursive calls in success and error-backoff paths now use _pollTickRef to avoid cancelling in-progress backoff timers and to correctly re-arm the poll loop after each tick. - Add stopPolling() at the start of sendMessage() so any stale in-flight tick is always cancelled before a new poll loop begins. - Replace HTML-like sentinel tags (<code-block>, <inline-code>, <strong>) in formatCommentBody with null-byte delimited sentinels that cannot appear in GitHub comment text, preventing collisions with user content. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 4 potential bugs in this PR.
low: 4
All three previously reported bugs (poll() backoff cancellation, sendMessage() missing stopPolling(), and sentinel tag collisions in formatCommentBody) have been fixed. Four new low-severity issues were found: an over-broad severity-prefix strip in formatCommentBody affecting all comment types, a stale event flush window in createIssue's poll interval, a React concurrent-mode-unsafe ref mutation during render in useAgents, and an unbounded module-level set in planner.py.
- ReviewThreads.jsx: restrict formatCommentBody severity-strip regex to known keywords (HIGH|CRITICAL|MEDIUM|LOW|INFO) so legitimate bold prefixes like **Note**: are not silently removed - usePlanning.js: introduce `cancelled` flag in createIssue to prevent stale interval ticks from appending stream events after creation state is cleared - useAgents.js: add explicit concurrent-mode safety note documenting the intentional render-body ref mutation and why it is safe - planner.py: replace unbounded _issue_created_keys set with a bounded OrderedDict (cap 10 000 entries) with LRU eviction to prevent memory growth in long-running deployments; update cleanup_session_issue_keys accordingly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 5 potential bugs in this PR.
medium: 2 | low: 3
The PR fixes all four previously reported bugs (formatCommentBody regex, createIssue cancelled-flag race, ref-mutation-in-render, and _issue_created_keys unbounded growth). Five new bugs were found: a file-unlink-while-open resource handling issue and a _stopped_agent_ids leak in error paths (both medium), plus three low-severity issues involving waitpid exit-code misreporting, log-tailer thread termination on transient DB errors, and a cursor-reset race in AgentLogViewer.
- Thread 1 (agent_pool.py:791): Close resume_stdout before os.unlink in the except clause so the file is not unlinked while still open. The finally guard is also updated to skip close() if already closed. - Thread 2 (dashboard.py:382): Wrap db.finish_agent/db.update_issue in try/except that calls unmark_externally_stopped on failure, preventing a permanent leak of the agent_id in _stopped_agent_ids. - Thread 3 (agent_pool.py:1166): Replace os.waitpid(pid, os.WNOHANG) with os.waitpid(pid, 0) so child processes are properly reaped and the exit code is accurately reported. - Thread 4 (agent_pool.py:977): Wrap both db.update_agent(log_offset) calls in individual try/except blocks that log a warning and continue, preventing a transient DB error from killing the entire tailer thread. - Thread 5 (AgentLogViewer.jsx:115): Remove cursorRef.current = 0 from the post-paint useEffect since useAgentLogs already resets it synchronously during render at the correct timing, avoiding a race that caused unnecessary re-fetching of already-seen events. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 1 potential bug in this PR.
low: 1
All five previously-reported bugs have been fixed in this PR. One new bug was introduced: the new _tail_log method (for freshly-spawned agents) lacks the try/except guard around db.update_agent that was correctly applied to the parallel _tail_log_file method — a transient DB error will kill the tailer thread.
Wrap both db.update_agent calls in _tail_log with try/except to prevent a transient DB error from propagating to the outer handler and killing the tailer thread. Matches the existing pattern used in _tail_log_file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 4 potential bugs in this PR.
medium: 3 | low: 1
The PR fixes the previously-reported unprotected db.update_agent bug in _tail_log (both calls are now wrapped in try/except), but introduces analogous unprotected db.insert_event calls in the same methods (_tail_log and _tail_log_file) that can still kill the tailer thread; additionally, a misleading WNOHANG comment in _monitor_pid and a race condition leaving old agent records stuck in 'running' state in restart_agent were found.
- Wrap db.insert_event calls in _tail_log and _tail_log_file with try/except so a transient DB write error does not kill the tailer thread - Fix os.waitpid(pid, 0) to use os.WNOHANG in _monitor_pid, matching the comment's stated intent and handling not-yet-reaped zombies - In restart_agent, call db.finish_agent before dispatching the new agent so the old record is never permanently stuck in 'running' if a DB write fails after dispatch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 4 potential bugs in this PR.
medium: 1 | low: 3
All four previously reported bugs are fixed in this revision. Four new issues were found: a missing unmark_externally_stopped call in restart_agent when the dispatch call throws (medium), an incorrect unmark call when db.update_issue fails after finish_agent already committed (low), a TOCTOU deduplication race in planner.py between two separate lock acquisitions (low), and accumulating phantom blank rows in the EnvEditor on repeated paste/upload (low).
- dashboard.py: separate update_issue from finish_agent try/except so dispatch proceeds even if only the issue status update fails (Thread 2); wrap dispatch calls in try/except to always call unmark_externally_stopped on exception (Thread 1) - planner.py: reserve (session_id, message_index) key inside the same lock acquisition as the membership check to close the TOCTOU race; clean up the reservation in the finally block if issue creation ultimately fails (Thread 3) - EnvEditor.jsx: limit blank rows to at most one after paste/upload to prevent phantom row accumulation (Thread 4) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rekpero
left a comment
There was a problem hiding this comment.
🟢 Claude BugBot Analysis
All four previously reported bugs are fixed in this PR. The restart_agent endpoint now correctly separates finish_agent and update_issue into independent try/except blocks (fixing PRRT_kwDORZvToM50Ytgv), wraps each dispatch_* call in try/except that calls unmark_externally_stopped on exception (fixing PRRT_kwDORZvToM50Ytgu), the planner dedup check and insertion are now in the same lock block (fixing PRRT_kwDORZvToM50Ytgw), and EnvEditor retains at most one blank row during paste/upload (fixing PRRT_kwDORZvToM50Ytgx). No new bugs were identified in the added code.
No bugs were detected in this PR.
Closes #3
Summary
frontend/with Tailwind CSS 3, React Query v5, Lucide React, and date-fnsHeader,WorkspaceSwitcher,TabNavMetricsBar,MetricCard(7-card grid)ActiveAgents,AgentCard,AgentLogViewer,AgentStatusBadgeIssueQueue,IssueStatusBadgewith Retry button forneeds_humanPRTracker,ReviewThreadsAddWorkspaceModal,WorkspaceSettingsModal,EnvEditorCard,Badge,Button,Modal,Spinner,EmptyStateselectedWorkspaceIdtolocalStoragedashboard.py(registered after all/api/*routes and static mount) so React client-side navigation works on direct URL loadsbuild-uicommand added torun.sh(cd frontend && npm install && npm run build), integrated into theinstallflowbuild.outDir = '../orchestrator/static'andemptyOutDir = true— build artifacts go directly intoorchestrator/static/.gitignoreupdated withfrontend/node_modules/andfrontend/dist/Test plan
cd frontend && npm run dev— hot-reload dev server athttp://localhost:5173proxying/apito FastAPInpm run build— outputsorchestrator/static/index.html+assets/python -m orchestrator.main→ openhttp://localhost:8420— React app renders—when empty)Retrybutton forneeds_humanissueshttp://localhost:8420/in a fresh tab loads the React app (SPA catch-all works)🤖 Generated with Claude Code