feat: Agent Teams UI Monitor + Fix waitForTeam stuck on teammate questions#15
Closed
Joao208 wants to merge 1 commit into
Closed
feat: Agent Teams UI Monitor + Fix waitForTeam stuck on teammate questions#15Joao208 wants to merge 1 commit into
Joao208 wants to merge 1 commit into
Conversation
…tions - New package: agent-teams-ui (React + Vite + Express) - Real-time monitoring dashboard for agent teams - SSE + file watching for live updates - Dark minimal design with framer-motion animations - Views: Work (tasks + team), Activity (timeline), Logs - Fix: waitForTeam in agent-teams-lead - Detect unanswered questions/blockers from teammates - Return teammates_need_response when questions pending - Return teammates_exited_with_pending_work when teammates exit early - teamStatus now includes needs_response and unanswered_questions
📝 WalkthroughWalkthroughThis PR expands the lead tools API response payloads with enriched team, task, and message metadata, and introduces a new React-based agent teams UI package featuring real-time updates, task management, activity tracking, and team status visualization via Server-Sent Events. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Browser Client
participant UI as React UI<br/>(useTeamState Hook)
participant API as Express Server<br/>(:3848)
participant Files as Filesystem<br/>(JSON + logs)
participant Watcher as File Watcher<br/>(Chokidar)
Client->>UI: Mount App
UI->>API: GET /api/state
API->>Files: Read team.json, tasks.json, etc.
Files-->>API: State data
API-->>UI: State payload
UI->>API: GET /api/logs?lines=200
API->>Files: Read team.log tail
Files-->>API: Log lines
API-->>UI: Logs payload
UI->>API: EventSource /api/events
API-->>UI: Connection opened (connected=true)
Watcher->>Files: Monitor agent-teams/
Files->>Watcher: JSON file change detected
Watcher->>API: File change event
API->>Files: Re-fetch state
Files-->>API: Updated state
API-->>UI: SSE message {type: "state", ...}
UI->>UI: Update state, re-render
Files->>Watcher: Log file change detected
Watcher->>API: File change event
API->>Files: Read log tail (20 lines)
Files-->>API: Log tail
API-->>UI: SSE message {type: "log", ...}
UI->>UI: Append logs, auto-scroll
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (5)
packages/agent-teams-ui/src/components/Header.tsx (1)
28-35: Add explicit button semantics for safer reuse and accessibility.At Line 28, set
type="button"to prevent accidental form submission if reused inside forms, and expose selected state witharia-pressed.Proposed refinement
<button key={v.key} + type="button" + aria-pressed={view === v.key} onClick={() => onViewChange(v.key)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-ui/src/components/Header.tsx` around lines 28 - 35, The header view toggle button currently lacks explicit semantics; update the button rendered in Header.tsx (the element using key={v.key} and onClick={() => onViewChange(v.key)}) to include type="button" to avoid accidental form submission and add an accessibility state by setting aria-pressed={view === v.key} so assistive tech can convey selection.packages/agent-teams-ui/src/components/Sidebar.tsx (1)
36-45: Per-teammate task stats are repeatedly recomputed inside render.Lines 37–45 do multiple full scans of
tasksfor each teammate. Precomputing stats once will reduce render cost and simplify this block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-ui/src/components/Sidebar.tsx` around lines 36 - 45, The render is doing repeated full scans of tasks inside active.map (computing current, done, total per teammate); precompute a lookup of per-teammate stats once (e.g., in the Sidebar component body) by reducing tasks into a Map/object keyed by assigned_to containing in_progress current task, completed count, and total count, then replace the repeated filters inside active.map to read statsById[t.id] (falling back to defaults) so the UI uses O(N) precomputation instead of O(N*M) repeated scans.packages/agent-teams-ui/src/components/Stats.tsx (1)
10-15: Status aggregation can be done in one pass.Lines 11–14 iterate the tasks array multiple times; a single reducer is simpler and scales better.
Proposed refactor
- const done = tasks.filter((t) => t.status === "completed").length; - const running = tasks.filter((t) => t.status === "in_progress").length; - const blocked = tasks.filter((t) => t.status === "blocked").length; - const queued = total - done - running - blocked; + const { done, running, blocked, queued } = tasks.reduce( + (acc, t) => { + if (t.status === "completed") acc.done += 1; + else if (t.status === "in_progress") acc.running += 1; + else if (t.status === "blocked") acc.blocked += 1; + else acc.queued += 1; + return acc; + }, + { done: 0, running: 0, blocked: 0, queued: 0 } + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-ui/src/components/Stats.tsx` around lines 10 - 15, The current implementation computes task counts by iterating tasks multiple times (tasks.filter(...) for done, running, blocked) which is inefficient; refactor to a single-pass reducer over tasks (use Array.prototype.reduce) to compute counts for done, in_progress, blocked and derive total, queued and progress from that accumulator. Update the logic that sets total, done, running, blocked, queued, and progress (the variables defined in Stats.tsx) to use the accumulator values so the array is traversed once and progress still computes Math.round((done / total) * 100) or 0 when total is 0.packages/agent-teams-ui/src/components/TaskList.tsx (1)
44-47: Set button type explicitly.At Line 44,
type="button"avoids accidental submits if this component is ever rendered inside a form.Proposed tweak
<button + type="button" onClick={() => setOpen(expanded ? null : task.id)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-ui/src/components/TaskList.tsx` around lines 44 - 47, The button inside the TaskList component doesn't set an explicit type, which can cause accidental form submissions; update the <button> element used with onClick={() => setOpen(expanded ? null : task.id)} to include type="button" so it won't act as a submit button if the component is rendered inside a form (locate the button in TaskList.tsx where setOpen, expanded and task.id are used).packages/agent-teams-ui/src/server/index.ts (1)
35-40: Consider avoiding full-file reads for log tailing.Reading the full log on every poll/watch event will degrade as
team.loggrows. A bounded tail-read approach (read from end / ring buffer) would scale better.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-ui/src/server/index.ts` around lines 35 - 40, The getLogTail function currently reads the entire team.log via readFile which will degrade as the file grows; replace the full-file read with a bounded tail-read that opens the file (fs.promises.open), obtains size (stat), calculates an offset from the end for a byte window sufficient to capture the requested number of lines, reads only that slice, and then extracts the last N lines (handling the case where the read window cuts a line in half). Update getLogTail to use this end-read strategy for efficiency while falling back to a full read if the file is smaller than the window; keep references to agentTeamsPath and "team.log" and preserve the same return type Promise<string>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/agent-teams-lead/src/tools.ts`:
- Around line 226-250: The current fast-path uses allTasksResolved (computed
from inProgress and pending) and returns done: true before checking
unreadQuestions, which lets teammate questions slip through; update the logic in
the method (around the allTasksResolved / unreadQuestions branch) so that either
(a) the unreadQuestions check runs before the allTasksResolved return, or (b)
expand allTasksResolved to also require unreadQuestions.length === 0 — then keep
the existing return block that uses startTime, completed and blocked unchanged.
This ensures unreadQuestions is considered before returning done: true.
- Around line 205-207: The polling loop currently always awaits the fixed
pollInterval (3000) which can exceed the requested timeout; update the wait
logic (where timeoutMs, pollInterval and startTime are used) to compute
remainingMs = timeoutMs - (Date.now() - startTime), clamp remainingMs to >=0,
and await Math.min(pollInterval, remainingMs) instead of the fixed pollInterval;
also ensure you break/return immediately if remainingMs <= 0 to avoid an extra
sleep — apply the same change to the other sleep call(s) referenced (the
occurrences around the other poll/sleep calls).
- Around line 200-203: The local team variable is captured before the polling
loop so it becomes stale after load() replaces this.team; update the loop to
refresh the current team from the store after each load by calling
this.store.getTeam() (or directly reference this.store.getTeam().teammates when
checking teammates at the lines around 222-224) so roster changes from
addTeammate()/removeTeammate() are reflected; in short, stop reusing the initial
team snapshot and retrieve the latest team from this.store inside the polling
iteration after load().
In `@packages/agent-teams-ui/package.json`:
- Around line 15-22: The package.json currently depends on "express" v4.x but
lists "@types/express" v5.x; update the devDependency for "@types/express" to
the 4.x typings to match the runtime Express major version (e.g., change the
"@types/express" version spec from ^5.0.0 to a 4.x range such as ^4.17.0) so
TypeScript validates against the correct Express API surface.
In `@packages/agent-teams-ui/src/components/Activity.tsx`:
- Around line 22-27: The ago(d: string) function can produce NaN or negative
values for malformed or future timestamps; update ago to validate the parsed
Date (check isNaN(new Date(d).getTime())) and handle future times by clamping
negative differences to zero (or returning "0s"/"now"), then compute
seconds/minutes/hours from the non-negative delta; locate and update the ago
function to perform the validation and clamping before the existing math so the
UI never sees NaN or negative durations.
In `@packages/agent-teams-ui/src/components/LogViewer.tsx`:
- Around line 27-30: The color variable c in LogViewer.tsx is being set by
independent if statements so a later check (e.g., the stdout check) can
overwrite an earlier error color; change the logic around the c assignments so
error highlighting has priority—either convert the separate ifs into an if/else
if chain with the /error|Error/ test first, or explicitly check for errors
before applying the stdout/stderr/spawned color, referencing the c variable and
the block in LogViewer.tsx that contains those four conditionals.
In `@packages/agent-teams-ui/src/components/TaskList.tsx`:
- Around line 13-16: The duration rendering logic that computes s/m/h and calls
setText assumes "since" parses to a valid past timestamp; validate the parsed
time (use Date.parse or new Date(since).getTime()) and guard against NaN or
future/negative diffs before computing seconds/minutes/hours in the function
using setText, returning a safe placeholder (e.g. "—" or "0s") when invalid, and
use Math.max(0, diffMs) to avoid negative values; apply the same validation/fix
to the other occurrence around lines 62-63 so both places handle
malformed/skewed timestamps consistently (refer to the setText call and the
local s/m/h calculations).
In `@packages/agent-teams-ui/src/server/index.ts`:
- Around line 55-58: The handler for GET /api/logs currently parses lines with
parseInt(req.query.lines as string) || 100 which permits negative, zero, NaN or
excessively large values; update the code in the app.get("/api/logs", ...) route
to validate and clamp the parsed value before calling getLogTail: parse the
query, fall back to a default when NaN, enforce a minimum (e.g., 1) and a safe
maximum (e.g., 1000), and then pass the clamped integer to getLogTail to prevent
negative/oversized inputs and unexpected behavior.
In `@packages/agent-teams-ui/src/useTeamState.ts`:
- Around line 59-62: The SSE error handler (eventSource.onerror) and the similar
onclose handler are scheduling reconnect retries without clearing previous
timers; create a module/state-level reconnectTimer (or ref) and before calling
setTimeout(fetchState, 3000) clearTimeout(reconnectTimer) and assign the new
timer id to reconnectTimer to debounce retries, and also
clearTimeout(reconnectTimer) in the effect/component cleanup to avoid leaked
pending timers; update the handlers referenced (eventSource.onerror and the
onclose handler around lines 69-72) to use this timer clearing logic.
- Around line 50-52: The SSE handler in useTeamState is appending the
server-sent log snapshot (data.type === "log") which causes duplicated content
on each update; change the update logic in the handler that calls setLogs to
replace the logs with the incoming snapshot (or dedupe by comparing data.log to
prev and only update when different) instead of always doing setLogs(prev =>
prev + "\n" + data.log) so the client mirrors the server snapshot without
repeated appends.
- Around line 16-35: fetchState and fetchLogs currently call res.json() without
checking HTTP status, which can set invalid state/logs and incorrectly mark
connected; update both functions to validate the response by checking res.ok (or
res.status) before parsing JSON: if !res.ok, optionally parse the error body or
throw a new Error with status/text and handle it in the catch (for fetchState
ensure setConnected(false) and do not call setState on error; for fetchLogs do
not call setLogs on error), and keep successful flows calling setState/setLogs
and setConnected(true) only when the response is OK.
---
Nitpick comments:
In `@packages/agent-teams-ui/src/components/Header.tsx`:
- Around line 28-35: The header view toggle button currently lacks explicit
semantics; update the button rendered in Header.tsx (the element using
key={v.key} and onClick={() => onViewChange(v.key)}) to include type="button" to
avoid accidental form submission and add an accessibility state by setting
aria-pressed={view === v.key} so assistive tech can convey selection.
In `@packages/agent-teams-ui/src/components/Sidebar.tsx`:
- Around line 36-45: The render is doing repeated full scans of tasks inside
active.map (computing current, done, total per teammate); precompute a lookup of
per-teammate stats once (e.g., in the Sidebar component body) by reducing tasks
into a Map/object keyed by assigned_to containing in_progress current task,
completed count, and total count, then replace the repeated filters inside
active.map to read statsById[t.id] (falling back to defaults) so the UI uses
O(N) precomputation instead of O(N*M) repeated scans.
In `@packages/agent-teams-ui/src/components/Stats.tsx`:
- Around line 10-15: The current implementation computes task counts by
iterating tasks multiple times (tasks.filter(...) for done, running, blocked)
which is inefficient; refactor to a single-pass reducer over tasks (use
Array.prototype.reduce) to compute counts for done, in_progress, blocked and
derive total, queued and progress from that accumulator. Update the logic that
sets total, done, running, blocked, queued, and progress (the variables defined
in Stats.tsx) to use the accumulator values so the array is traversed once and
progress still computes Math.round((done / total) * 100) or 0 when total is 0.
In `@packages/agent-teams-ui/src/components/TaskList.tsx`:
- Around line 44-47: The button inside the TaskList component doesn't set an
explicit type, which can cause accidental form submissions; update the <button>
element used with onClick={() => setOpen(expanded ? null : task.id)} to include
type="button" so it won't act as a submit button if the component is rendered
inside a form (locate the button in TaskList.tsx where setOpen, expanded and
task.id are used).
In `@packages/agent-teams-ui/src/server/index.ts`:
- Around line 35-40: The getLogTail function currently reads the entire team.log
via readFile which will degrade as the file grows; replace the full-file read
with a bounded tail-read that opens the file (fs.promises.open), obtains size
(stat), calculates an offset from the end for a byte window sufficient to
capture the requested number of lines, reads only that slice, and then extracts
the last N lines (handling the case where the read window cuts a line in half).
Update getLogTail to use this end-read strategy for efficiency while falling
back to a full read if the file is smaller than the window; keep references to
agentTeamsPath and "team.log" and preserve the same return type Promise<string>.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 57fd23d2-ac1c-43d2-843a-376fab1f4ef9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
packages/agent-teams-lead/src/tools.tspackages/agent-teams-ui/index.htmlpackages/agent-teams-ui/package.jsonpackages/agent-teams-ui/postcss.config.jspackages/agent-teams-ui/src/App.tsxpackages/agent-teams-ui/src/components/Activity.tsxpackages/agent-teams-ui/src/components/Header.tsxpackages/agent-teams-ui/src/components/LogViewer.tsxpackages/agent-teams-ui/src/components/Sidebar.tsxpackages/agent-teams-ui/src/components/Stats.tsxpackages/agent-teams-ui/src/components/TaskList.tsxpackages/agent-teams-ui/src/index.csspackages/agent-teams-ui/src/main.tsxpackages/agent-teams-ui/src/server/index.tspackages/agent-teams-ui/src/types.tspackages/agent-teams-ui/src/useTeamState.tspackages/agent-teams-ui/tailwind.config.jspackages/agent-teams-ui/tsconfig.jsonpackages/agent-teams-ui/vite.config.ts
Comment on lines
+200
to
+203
| const team = this.store.getTeam(); | ||
| if (!team) { | ||
| return this.ok({ error: "No active team" }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
STORE=$(rg --files | rg 'packages/agent-teams-lead/src/store\.ts$' | head -n1)
echo "== $STORE =="
rg -n -C3 'class TeamStore|async load\(|getTeam\(|addTeammate\(|removeTeammate\(|this\.team\s*=' "$STORE"Repository: arvoreeducacao/arvore-mcp-servers
Length of output: 2487
🏁 Script executed:
cat -n packages/agent-teams-lead/src/tools.ts | sed -n '190,235p'Repository: arvoreeducacao/arvore-mcp-servers
Length of output: 2045
Refresh team inside the polling loop to see roster changes.
The team variable is snapshots at line 200 before the loop, but load() at line 210 creates a new Team object (via this.team = await this.readJson(...)), leaving the loop's reference stale. Changes from addTeammate() or removeTeammate() won't be reflected. Either call team = this.store.getTeam() after each load(), or use this.store.getTeam().teammates at line 222-224 instead of reusing the snapshot.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-teams-lead/src/tools.ts` around lines 200 - 203, The local
team variable is captured before the polling loop so it becomes stale after
load() replaces this.team; update the loop to refresh the current team from the
store after each load by calling this.store.getTeam() (or directly reference
this.store.getTeam().teammates when checking teammates at the lines around
222-224) so roster changes from addTeammate()/removeTeammate() are reflected; in
short, stop reusing the initial team snapshot and retrieve the latest team from
this.store inside the polling iteration after load().
Comment on lines
+205
to
+207
| const timeoutMs = params.timeout_seconds * 1000; | ||
| const pollInterval = 3000; | ||
| const startTime = Date.now(); |
There was a problem hiding this comment.
Clamp the final sleep to the remaining timeout.
Line 300 always waits 3000 ms, so a caller asking for a 1-second timeout can still block for about 3 seconds. Sleep Math.min(pollInterval, remainingMs) instead of the fixed interval.
Proposed fix
const timeoutMs = params.timeout_seconds * 1000;
const pollInterval = 3000;
const startTime = Date.now();
@@
- await new Promise((resolve) => setTimeout(resolve, pollInterval));
+ const remainingMs = timeoutMs - (Date.now() - startTime);
+ if (remainingMs <= 0) {
+ break;
+ }
+
+ await new Promise((resolve) =>
+ setTimeout(resolve, Math.min(pollInterval, remainingMs))
+ );Also applies to: 300-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-teams-lead/src/tools.ts` around lines 205 - 207, The polling
loop currently always awaits the fixed pollInterval (3000) which can exceed the
requested timeout; update the wait logic (where timeoutMs, pollInterval and
startTime are used) to compute remainingMs = timeoutMs - (Date.now() -
startTime), clamp remainingMs to >=0, and await Math.min(pollInterval,
remainingMs) instead of the fixed pollInterval; also ensure you break/return
immediately if remainingMs <= 0 to avoid an extra sleep — apply the same change
to the other sleep call(s) referenced (the occurrences around the other
poll/sleep calls).
Comment on lines
+226
to
+250
| const allTasksResolved = | ||
| inProgress.length === 0 && pending.length === 0; | ||
|
|
||
| const pending = tasks.filter((t) => t.status === "pending"); | ||
| const inProgress = tasks.filter((t) => t.status === "in_progress"); | ||
| const completed = tasks.filter((t) => t.status === "completed"); | ||
| const blocked = tasks.filter((t) => t.status === "blocked"); | ||
|
|
||
| const allTeammatesDone = team.teammates | ||
| .filter((t) => t.status === "active") | ||
| .every((t) => !this.spawner.isRunning(t.id)); | ||
|
|
||
| if ( | ||
| (inProgress.length === 0 && pending.length === 0) || | ||
| allTeammatesDone | ||
| ) { | ||
| return this.ok({ | ||
| done: true, | ||
| reason: | ||
| inProgress.length === 0 && pending.length === 0 | ||
| ? "all_tasks_resolved" | ||
| if (allTasksResolved) { | ||
| return this.ok({ | ||
| done: true, | ||
| reason: "all_tasks_resolved", | ||
| elapsed_seconds: Math.round((Date.now() - startTime) / 1000), | ||
| completed: completed.map((t) => ({ | ||
| id: t.id, | ||
| title: t.title, | ||
| summary: t.summary, | ||
| })), | ||
| blocked: blocked.map((t) => ({ | ||
| id: t.id, | ||
| title: t.title, | ||
| notes: t.notes, | ||
| })), | ||
| }); | ||
| } | ||
|
|
||
| if (unreadQuestions.length > 0) { | ||
| return this.ok({ | ||
| done: false, | ||
| reason: "teammates_need_response", |
There was a problem hiding this comment.
Check unreadQuestions before declaring the team done.
If there are no pending or in-progress tasks, Line 229 returns done: true before Line 247 considers teammate questions. That still reproduces the PR's reported case where someone asks a question before any task exists. Move the unreadQuestions branch above this fast path, or make allTasksResolved contingent on there being nothing awaiting a lead response.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-teams-lead/src/tools.ts` around lines 226 - 250, The current
fast-path uses allTasksResolved (computed from inProgress and pending) and
returns done: true before checking unreadQuestions, which lets teammate
questions slip through; update the logic in the method (around the
allTasksResolved / unreadQuestions branch) so that either (a) the
unreadQuestions check runs before the allTasksResolved return, or (b) expand
allTasksResolved to also require unreadQuestions.length === 0 — then keep the
existing return block that uses startTime, completed and blocked unchanged. This
ensures unreadQuestions is considered before returning done: true.
Comment on lines
+15
to
+22
| "express": "^4.21.0", | ||
| "framer-motion": "^12.36.0", | ||
| "react": "^19.0.0", | ||
| "react-dom": "^19.0.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/cors": "^2.8.17", | ||
| "@types/express": "^5.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json
from pathlib import Path
pkg = json.loads(Path("packages/agent-teams-ui/package.json").read_text())
express = pkg["dependencies"]["express"]
types = pkg["devDependencies"]["@types/express"]
def major(spec: str) -> str:
return spec.lstrip("^~<>=").split(".")[0]
print({"express": express, "@types/express": types})
print("major_mismatch:", major(express) != major(types))
PYRepository: arvoreeducacao/arvore-mcp-servers
Length of output: 148
Align @types/express with the installed Express major version.
Line 15 installs Express 4.x, but line 22 installs the 5.x type package. This causes TypeScript to validate against a different API surface than the runtime uses. Pin the typings to the 4.x line to match the installed Express version.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-teams-ui/package.json` around lines 15 - 22, The package.json
currently depends on "express" v4.x but lists "@types/express" v5.x; update the
devDependency for "@types/express" to the 4.x typings to match the runtime
Express major version (e.g., change the "@types/express" version spec from
^5.0.0 to a 4.x range such as ^4.17.0) so TypeScript validates against the
correct Express API surface.
Comment on lines
+22
to
+27
| function ago(d: string): string { | ||
| const s = Math.floor((Date.now() - new Date(d).getTime()) / 1000); | ||
| if (s < 60) return `${s}s`; | ||
| const m = Math.floor(s / 60); | ||
| if (m < 60) return `${m}m`; | ||
| return `${Math.floor(m / 60)}h`; |
There was a problem hiding this comment.
Guard against invalid or future timestamps in ago().
If a timestamp is malformed, UI can render NaNh; future timestamps can render negative durations.
Suggested fix
function ago(d: string): string {
- const s = Math.floor((Date.now() - new Date(d).getTime()) / 1000);
+ const ts = Date.parse(d);
+ if (Number.isNaN(ts)) return "--";
+ const s = Math.max(0, Math.floor((Date.now() - ts) / 1000));
if (s < 60) return `${s}s`;
const m = Math.floor(s / 60);
if (m < 60) return `${m}m`;
return `${Math.floor(m / 60)}h`;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-teams-ui/src/components/Activity.tsx` around lines 22 - 27,
The ago(d: string) function can produce NaN or negative values for malformed or
future timestamps; update ago to validate the parsed Date (check isNaN(new
Date(d).getTime())) and handle future times by clamping negative differences to
zero (or returning "0s"/"now"), then compute seconds/minutes/hours from the
non-negative delta; locate and update the ago function to perform the validation
and clamping before the existing math so the UI never sees NaN or negative
durations.
Comment on lines
+13
to
+16
| const s = Math.floor((Date.now() - new Date(since).getTime()) / 1000); | ||
| const m = Math.floor(s / 60); | ||
| const h = Math.floor(m / 60); | ||
| setText(h > 0 ? `${h}h ${m % 60}m` : m > 0 ? `${m}m ${s % 60}s` : `${s}s`); |
There was a problem hiding this comment.
Duration rendering should guard invalid/negative timestamps.
Lines 13 and 62 assume valid parseable dates; malformed or skewed timestamps can render NaN or negative durations.
Proposed hardening
- const s = Math.floor((Date.now() - new Date(since).getTime()) / 1000);
+ const start = new Date(since).getTime();
+ if (!Number.isFinite(start)) {
+ setText("0s");
+ return;
+ }
+ const s = Math.max(0, Math.floor((Date.now() - start) / 1000));
...
- const m = Math.floor((new Date(task.completed_at).getTime() - new Date(task.created_at).getTime()) / 60000);
+ const end = new Date(task.completed_at).getTime();
+ const start = new Date(task.created_at).getTime();
+ if (!Number.isFinite(start) || !Number.isFinite(end)) return "0m";
+ const m = Math.max(0, Math.floor((end - start) / 60000));Also applies to: 62-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-teams-ui/src/components/TaskList.tsx` around lines 13 - 16,
The duration rendering logic that computes s/m/h and calls setText assumes
"since" parses to a valid past timestamp; validate the parsed time (use
Date.parse or new Date(since).getTime()) and guard against NaN or
future/negative diffs before computing seconds/minutes/hours in the function
using setText, returning a safe placeholder (e.g. "—" or "0s") when invalid, and
use Math.max(0, diffMs) to avoid negative values; apply the same validation/fix
to the other occurrence around lines 62-63 so both places handle
malformed/skewed timestamps consistently (refer to the setText call and the
local s/m/h calculations).
Comment on lines
+55
to
+58
| app.get("/api/logs", async (req, res) => { | ||
| try { | ||
| const lines = parseInt(req.query.lines as string) || 100; | ||
| const log = await getLogTail(lines); |
There was a problem hiding this comment.
Clamp and sanitize lines query input.
parseInt(... ) || 100 allows negative/oversized values and unexpected behavior. Bound it to a safe range.
Suggested fix
app.get("/api/logs", async (req, res) => {
try {
- const lines = parseInt(req.query.lines as string) || 100;
+ const rawLines = Number(req.query.lines);
+ const lines = Number.isFinite(rawLines)
+ ? Math.min(Math.max(Math.trunc(rawLines), 1), 2000)
+ : 100;
const log = await getLogTail(lines);
res.json({ log });
} catch {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-teams-ui/src/server/index.ts` around lines 55 - 58, The
handler for GET /api/logs currently parses lines with parseInt(req.query.lines
as string) || 100 which permits negative, zero, NaN or excessively large values;
update the code in the app.get("/api/logs", ...) route to validate and clamp the
parsed value before calling getLogTail: parse the query, fall back to a default
when NaN, enforce a minimum (e.g., 1) and a safe maximum (e.g., 1000), and then
pass the clamped integer to getLogTail to prevent negative/oversized inputs and
unexpected behavior.
Comment on lines
+16
to
+35
| const fetchState = useCallback(async () => { | ||
| try { | ||
| const res = await fetch("/api/state"); | ||
| const data = await res.json(); | ||
| setState(data); | ||
| setConnected(true); | ||
| } catch { | ||
| setConnected(false); | ||
| } | ||
| }, []); | ||
|
|
||
| const fetchLogs = useCallback(async () => { | ||
| try { | ||
| const res = await fetch("/api/logs?lines=200"); | ||
| const data = await res.json(); | ||
| setLogs(data.log); | ||
| } catch { | ||
| /* noop */ | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Validate HTTP status before consuming API payloads.
Both fetchers treat any JSON response as success. On backend errors, this can set invalid state/log values and still report connected.
Suggested fix
const fetchState = useCallback(async () => {
try {
const res = await fetch("/api/state");
- const data = await res.json();
+ if (!res.ok) throw new Error("Failed to fetch state");
+ const data: TeamState = await res.json();
setState(data);
setConnected(true);
} catch {
setConnected(false);
}
}, []);
const fetchLogs = useCallback(async () => {
try {
const res = await fetch("/api/logs?lines=200");
- const data = await res.json();
- setLogs(data.log);
+ if (!res.ok) throw new Error("Failed to fetch logs");
+ const data = (await res.json()) as { log?: string };
+ setLogs(typeof data.log === "string" ? data.log : "");
} catch {
/* noop */
}
}, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-teams-ui/src/useTeamState.ts` around lines 16 - 35, fetchState
and fetchLogs currently call res.json() without checking HTTP status, which can
set invalid state/logs and incorrectly mark connected; update both functions to
validate the response by checking res.ok (or res.status) before parsing JSON: if
!res.ok, optionally parse the error body or throw a new Error with status/text
and handle it in the catch (for fetchState ensure setConnected(false) and do not
call setState on error; for fetchLogs do not call setLogs on error), and keep
successful flows calling setState/setLogs and setConnected(true) only when the
response is OK.
Comment on lines
+50
to
+52
| if (data.type === "log") { | ||
| setLogs((prev) => prev + "\n" + data.log); | ||
| } |
There was a problem hiding this comment.
Avoid duplicating logs on SSE updates.
The server pushes a log tail snapshot, but the client appends it. This duplicates content on every .log change event.
Suggested fix
- if (data.type === "log") {
- setLogs((prev) => prev + "\n" + data.log);
- }
+ if (data.type === "log" && typeof data.log === "string") {
+ setLogs(data.log);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (data.type === "log") { | |
| setLogs((prev) => prev + "\n" + data.log); | |
| } | |
| if (data.type === "log" && typeof data.log === "string") { | |
| setLogs(data.log); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-teams-ui/src/useTeamState.ts` around lines 50 - 52, The SSE
handler in useTeamState is appending the server-sent log snapshot (data.type ===
"log") which causes duplicated content on each update; change the update logic
in the handler that calls setLogs to replace the logs with the incoming snapshot
(or dedupe by comparing data.log to prev and only update when different) instead
of always doing setLogs(prev => prev + "\n" + data.log) so the client mirrors
the server snapshot without repeated appends.
Comment on lines
+59
to
+62
| eventSource.onerror = () => { | ||
| setConnected(false); | ||
| setTimeout(fetchState, 3000); | ||
| }; |
There was a problem hiding this comment.
Debounce and clean up reconnect retry timers.
Repeated SSE errors can queue multiple retries, and pending timers are not cleared on cleanup.
Suggested fix
useEffect(() => {
+ let retryTimer: ReturnType<typeof setTimeout> | null = null;
fetchState();
fetchLogs();
const eventSource = new EventSource("/api/events");
@@
eventSource.onerror = () => {
setConnected(false);
- setTimeout(fetchState, 3000);
+ if (retryTimer) return;
+ retryTimer = setTimeout(() => {
+ retryTimer = null;
+ fetchState();
+ }, 3000);
};
@@
return () => {
eventSource.close();
+ if (retryTimer) clearTimeout(retryTimer);
clearInterval(interval);
};
}, [fetchState, fetchLogs]);Also applies to: 69-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-teams-ui/src/useTeamState.ts` around lines 59 - 62, The SSE
error handler (eventSource.onerror) and the similar onclose handler are
scheduling reconnect retries without clearing previous timers; create a
module/state-level reconnectTimer (or ref) and before calling
setTimeout(fetchState, 3000) clearTimeout(reconnectTimer) and assign the new
timer id to reconnectTimer to debounce retries, and also
clearTimeout(reconnectTimer) in the effect/component cleanup to avoid leaked
pending timers; update the handlers referenced (eventSource.onerror and the
onclose handler around lines 69-72) to use this timer clearing logic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Descricao
Duas mudanças neste PR:
1. Novo pacote:
agent-teams-uiDashboard de monitoramento real-time para agent teams. React + Vite + Express com SSE e file watching para atualizações ao vivo. Design dark minimalista inspirado em Linear/Vercel com framer-motion.
.agent-teams/*.jsone serve via REST + SSE2. Fix:
waitForTeamnoagent-teams-leadQuando teammates enviavam perguntas/blockers e saíam antes das tasks serem criadas,
waitForTeamretornavadone: trueincorretamente. Agora detecta perguntas não respondidas e retornateammates_need_response. Também retornateammates_exited_with_pending_workquando teammates saem com tasks pendentes.Etiquetas (Labels)
Historia Relacionada
N/A
Motivacao e Contexto
waitForTeamcausava times travados quando teammates faziam perguntas — o lead nunca recebia as mensagensComo Isso Foi Testado?
Testado manualmente com dados fake em
.agent-teams/e verificação visual da UI no browser. Fix dowaitForTeamverificado com buildtsc.Analise de Risco e Impacto
Capturas de Tela ou Auxilios Visuais (se apropriado)
N/A - rodar
pnpm devno pacoteagent-teams-uipara visualizarSummary by CodeRabbit