[auto] #84 feat: web dashboard with live agent terminal views - #90
Conversation
Add rust-embed dependency and create src/dashboard.rs to embed the dashboard/ directory at compile time. The SPA implements the factory floor metaphor: agents rendered as pipeline stages (planner -> implementer -> tester -> reviewer -> docs-release) with flow connectors, state-driven visual treatments (working/idle/stopped/blocked/auth-fail), pulse animations for active stages, and a throughput HUD. Wire a fallback route in serve.rs that serves embedded assets for non- /v1 GET requests when observe.dashboard is enabled, with SPA-style index.html fallback for unknown paths. The dashboard connects to /v1/health for initial snapshot and /v1/events/stream SSE for live updates, with 300ms render debounce to prevent flicker. Mobile-responsive: vertical stage stack on small screens. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds an embedded SPA "factory-floor" dashboard (HTML/CSS/JS) and server-side support to serve those assets (embedded via rust-embed) with SPA fallback; threads a Changes
Sequence DiagramsequenceDiagram
participant Browser as Browser Client
participant Server as HTTP Server
participant Health as Health Backend (/v1/health)
participant Stream as Event Stream (/v1/events/stream)
participant JS as Dashboard JS
participant DOM as Dashboard DOM
Browser->>Server: GET /
Server->>Server: check dashboard_enabled
Server-->>Browser: 200 index.html (embedded)
Browser->>JS: load /app.js
JS->>Server: GET /v1/health
Server->>Health: fetch snapshot
Health-->>Server: health data
Server-->>JS: return snapshot
JS->>JS: init appState
JS->>Stream: open SSE /v1/events/stream
Stream-->>JS: SSE open / events
loop event stream
Stream->>JS: event (agent/data)
JS->>JS: merge into appState
JS->>DOM: debounced render (300ms)
end
loop periodic refresh (15s)
JS->>Server: GET /v1/health
Server-->>JS: updated snapshot
JS->>DOM: update HUD/timestamps
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
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 review |
✅ Actions performedReview triggered.
|
Implement `tt remote` subcommand group with attach and status handlers. `tt remote attach <host>` opens an SSH port-forward tunnel to a remote tutti instance and persists the host as a [[remote]] config entry. `tt remote status` lists registered remotes with reachability probes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Log warning when GlobalConfig::load() fails instead of silently skipping remote entry persistence - Fix health probe path from /healthz to /v1/health to match serve.rs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds changelog entries for tt remote attach/status, tt serve --remote/--bind, stable FailureCategory enum, step timeline persistence, and config/health fixes. Minor version bump per VERSIONING.md (new CLI subcommands). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/cli/serve.rs (2)
427-468:⚠️ Potential issue | 🟡 MinorNormalize the request path before routing it.
You're routing on the raw URL string here. That makes
/v1look like a dashboard route because it doesn't match/v1/, and asset requests with query strings such as/app.js?v=1or/style.css?v=1missAssets::get(...)and fall through to the SPA fallback, returningindex.htmlwith200. Parse the path once without?…and use that normalized value for both the API check and the embedded-asset lookup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/serve.rs` around lines 427 - 468, Normalize the request URL path (strip the query string and use a canonical path) before any routing decisions: compute a normalized_path from request.url() without the `?...` portion and use that for the API check (replace is_api = url.starts_with("/v1/") with a check that treats "/v1" and "/v1/..." as API, e.g., normalized_path == "/v1" || normalized_path.starts_with("/v1/") ) and pass normalized_path into serve_dashboard_asset instead of the raw URL; inside serve_dashboard_asset (and its asset_path derivation) operate on the normalized path (trim leading slash and map "" to "index.html") so requests with query strings like "/app.js?v=1" correctly resolve to the embedded asset rather than falling back to index.html.
396-433:⚠️ Potential issue | 🔴 CriticalBearer mode blocks dashboard page loads and SSE connections.
The auth middleware rejects requests without a valid
Authorizationheader, but the dashboard cannot provide one: the initial page load is a plain GET request, and the browser's nativeEventSourceAPI (used bydashboard/app.jsfor/v1/events/stream) does not support custom headers—only CORS credentials like cookies. This prevents the entire dashboard flow from working in--remoteor[serve] auth = "bearer"mode.Choose an alternative for dashboard requests:
- Cookie-based auth: Store auth tokens in HTTP-only cookies; the browser sends them automatically with
EventSourceif you setwithCredentials: true. This is the simplest native option.- Query token: Exchange a long-lived bearer token for a short-lived token via an initial authenticated request, then append it as
?token=...to dashboard asset and/v1/events/streamURLs.- Fetch-event-source library: Use
@microsoft/fetch-event-sourceinstead of nativeEventSourceto send custom headers (requires a JavaScript dependency in the dashboard).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/serve.rs` around lines 396 - 433, The bearer-only check in the request handling (the expected_token / validate_bearer_auth block) blocks dashboard page loads and SSE because browsers can't send custom Authorization headers; update the auth middleware to accept an alternate credential source for dashboard flows: when request.url() is not an API path (served by serve_dashboard_asset) or when is_stream (the SSE path handled by handle_sse_request), if Authorization is missing/invalid attempt to read a cookie (e.g. "auth_token") and/or the "token" query parameter and pass that value into validate_bearer_auth before rejecting; keep validate_bearer_auth as the canonical verifier but extend the value selection logic so dashboard_enabled + non-API GET and is_stream requests can authenticate via cookie or ?token=... instead of only the Authorization header.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboard/app.js`:
- Around line 174-184: fetchHealth currently only upserts into appState.agents
so removed agents never disappear; fix by rebuilding appState.agents from the
fresh records snapshot inside fetchHealth: create a new empty map/object,
iterate records to populate it (assigning appState.agents[r.agent] = r and
updating $wsName when r.workspace), then replace the old appState.agents with
this new map before calling renderPipeline(); update the code in the fetchHealth
function and reference appState.agents, records, r, and renderPipeline when
making the change.
- Around line 18-22: The dashboard currently collapses records by agent only
(appState.agents) causing last-writer-wins across workspaces; change the agent
keying to include the workspace (e.g., use a composite key like
`${workspace}:${agent}` when storing into appState.agents and when
generating/updating appState.events) and update any rendering logic that reads
appState.agents/events (the code that builds stage cards and workspace badge
rendering) to group or render lanes per workspace instead of a single per-agent
card; alternatively, if you intend single-workspace view, restrict the client to
only accept and display records for the selected workspace by filtering incoming
health records by record.workspace before inserting into appState.agents/events.
Ensure every place referencing plain agent keys (including creation, update, and
display paths around appState.agents, events, and eventCount) is updated to use
the workspace-scoped key or to apply the workspace filter.
---
Outside diff comments:
In `@src/cli/serve.rs`:
- Around line 427-468: Normalize the request URL path (strip the query string
and use a canonical path) before any routing decisions: compute a
normalized_path from request.url() without the `?...` portion and use that for
the API check (replace is_api = url.starts_with("/v1/") with a check that treats
"/v1" and "/v1/..." as API, e.g., normalized_path == "/v1" ||
normalized_path.starts_with("/v1/") ) and pass normalized_path into
serve_dashboard_asset instead of the raw URL; inside serve_dashboard_asset (and
its asset_path derivation) operate on the normalized path (trim leading slash
and map "" to "index.html") so requests with query strings like "/app.js?v=1"
correctly resolve to the embedded asset rather than falling back to index.html.
- Around line 396-433: The bearer-only check in the request handling (the
expected_token / validate_bearer_auth block) blocks dashboard page loads and SSE
because browsers can't send custom Authorization headers; update the auth
middleware to accept an alternate credential source for dashboard flows: when
request.url() is not an API path (served by serve_dashboard_asset) or when
is_stream (the SSE path handled by handle_sse_request), if Authorization is
missing/invalid attempt to read a cookie (e.g. "auth_token") and/or the "token"
query parameter and pass that value into validate_bearer_auth before rejecting;
keep validate_bearer_auth as the canonical verifier but extend the value
selection logic so dashboard_enabled + non-API GET and is_stream requests can
authenticate via cookie or ?token=... instead of only the Authorization header.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 579af2d8-8c39-4a08-856b-57585b46f977
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
CHANGELOG.mdCargo.tomldashboard/app.jsdashboard/index.htmldashboard/style.csssrc/cli/serve.rssrc/dashboard.rssrc/main.rs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli/permissions.rs (1)
541-548: LGTM!The nanos suffix combined with process ID and
#[serial]provides effective test isolation across rapid re-runs.Consider extracting the
HomeGuardpattern to a shared test utility module since it's duplicated inserve.rs. This is a minor DRY improvement that can be deferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/permissions.rs` around lines 541 - 548, Extract the duplicated HomeGuard test pattern into a shared test utility (e.g., tests::util or crate::test_utils) by moving the temp dir creation and HOME env var override into a single HomeGuard struct with a constructor (that creates the temp dir, sets HOME to it, and returns Self) and a Drop impl that restores the original HOME and cleans up the temp; then replace the duplicated logic in permissions.rs (the temp variable and HomeGuard usage) and serve.rs to call the new shared HomeGuard constructor so both tests import and use the same utility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/cli/permissions.rs`:
- Around line 541-548: Extract the duplicated HomeGuard test pattern into a
shared test utility (e.g., tests::util or crate::test_utils) by moving the temp
dir creation and HOME env var override into a single HomeGuard struct with a
constructor (that creates the temp dir, sets HOME to it, and returns Self) and a
Drop impl that restores the original HOME and cleans up the temp; then replace
the duplicated logic in permissions.rs (the temp variable and HomeGuard usage)
and serve.rs to call the new shared HomeGuard constructor so both tests import
and use the same utility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c5edce54-e6ec-4517-84bd-f66bab74959f
📒 Files selected for processing (2)
src/cli/permissions.rssrc/cli/serve.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cli/serve.rs
Use composite keys (workspace:agent) when storing agent records in appState.agents so multi-workspace deployments no longer collide. Rebuild appState.agents from fresh /v1/health snapshots so removed agents disappear without requiring a hard reload. Resolves CodeRabbit review comments on PR #90. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
dashboard/app.js (1)
74-115:⚠️ Potential issue | 🟠 MajorComposite keys fixed storage, but the render path still collapses workspaces.
Line 110 still picks a single
agents[0]card for each stage, while Line 190 and Line 218 keep overwriting the lone workspace badge. With more than one workspace in/v1/health, the stage card can describe one workspace and the header another, so the dashboard is still not actually workspace-scoped. Either filter to one workspace before storing/rendering or render separate lanes/groups per workspace.Also applies to: 183-191, 218-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dashboard/app.js` around lines 74 - 115, The render currently picks only agents[0] for each stage (see stageAgents, STAGE_ORDER, agents[0], primary, and the stage card construction) which collapses multiple workspaces into one card; change the rendering to preserve workspace separation by grouping agents by workspace before building stageAgents (e.g., use a composite key of stage+workspace or maintain stageAgents[stage][workspace] lists) or by iterating over agents and creating a separate lane/card per workspace rather than using only primary; update the logic that sets card class, state-chip (stateLabel), and agent-runtime so it applies per workspace group (and similarly update any code using stateClass/agent-runtime elsewhere that assumes a single agent per stage).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboard/app.js`:
- Around line 39-58: The current stateClass and stateLabel functions treat
activity_state === "unknown" as idle; update both functions (stateClass and
stateLabel) to explicitly check for agent.activity_state === "unknown" and
return a distinct state (e.g., "unknown" for class and "unknown" or
"indeterminate" for label) before falling through to "idle" so agents with
activity_state "unknown" are not rendered as healthy/idle; ensure the new check
appears alongside the other activity_state checks (before the working/active and
idle fall-through) to guarantee correct rendering.
- Around line 180-193: Check res.ok in the health fetch before calling
res.json() and avoid replacing appState.agents with an empty snapshot on error:
in the fetch promise chain that uses agentKey and sets appState.agents and calls
renderPipeline(), if res.ok is false call an error handler (log to
console/process UI and return early) or throw to preserve the last-known
appState instead of setting json.data || []; and for the EventSource to
"/v1/events/stream" replace the direct EventSource usage (which cannot send
Authorization headers) with a bearer-compatible approach—either put the token in
an httpOnly, Secure cookie and open EventSource with credentials
(withCredentials:true equivalent in your client) or switch to a fetch-based SSE
client such as `@microsoft/fetch-event-source` so you can send the Authorization:
Bearer header; update the code around the EventSource creation accordingly.
---
Duplicate comments:
In `@dashboard/app.js`:
- Around line 74-115: The render currently picks only agents[0] for each stage
(see stageAgents, STAGE_ORDER, agents[0], primary, and the stage card
construction) which collapses multiple workspaces into one card; change the
rendering to preserve workspace separation by grouping agents by workspace
before building stageAgents (e.g., use a composite key of stage+workspace or
maintain stageAgents[stage][workspace] lists) or by iterating over agents and
creating a separate lane/card per workspace rather than using only primary;
update the logic that sets card class, state-chip (stateLabel), and
agent-runtime so it applies per workspace group (and similarly update any code
using stateClass/agent-runtime elsewhere that assumes a single agent per stage).
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
Automated SDLC cycle for #84.
Summary by CodeRabbit
New Features
CLI
Bug Fixes & Docs
Tests