Skip to content

[codex] Restore v2 resource monitor#4314

Merged
Kitenite merged 7 commits into
mainfrom
resource-monitor-v2
May 10, 2026
Merged

[codex] Restore v2 resource monitor#4314
Kitenite merged 7 commits into
mainfrom
resource-monitor-v2

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented May 9, 2026

Summary

Restores the desktop resource monitor for v2 while preserving the existing v1 path.

  • Adds a surface option to resource metrics so the renderer can request v1 or v2 snapshots.
  • Splits resource metrics session sourcing into a dedicated session-sources module.
  • Adds a daemon-backed host-service /terminal/resource-sessions endpoint for v2 terminal PID discovery.
  • Carries optional terminal titles through the metrics payload and falls back to v2 pane title overrides in the renderer.
  • Keeps the actual CPU/memory aggregation daemon-rooted and process-tree based.

Why

The resource monitor was hidden for v2. The earlier v1-only collection path depended on local pane/session state, while v2 terminal ownership lives behind host-service and the PTY daemon. The new v2 path queries the daemon-owned terminal list and joins it with active terminal DB rows, which is a more accurate source of root PIDs than host-service renderer-facing session state alone.

Validation

  • bun run --cwd apps/desktop typecheck
  • bun run --cwd packages/host-service typecheck
  • bun run lint
  • bun test apps/desktop/src/lib/trpc/routers/resource-metrics.schema.test.ts apps/desktop/src/main/lib/resource-metrics/process-tree.test.ts packages/host-service/test/integration/terminal.integration.test.ts

Summary by cubic

Restores the desktop Resource Monitor for v2 with daemon-sourced terminal PIDs and a redesigned UI. The monitor shows terminal titles, severity dots, a RAM share bar, and appears in the v2 sidebar header.

  • New Features

    • Add surface (v1 | v2) and optional organizationId to resourceMetrics.getSnapshot, with per-surface+org caching and inflight dedupe.
    • New host-service route /terminal/resource-sessions joins live daemon sessions with active DB rows and returns terminalId, workspaceId, pid, and title.
    • Renderer enables the monitor for v2: passes surface and organizationId, hydrates v2 project/workspace names, applies terminal title overrides keyed by paneId with user-set titles preferred over daemon titles, defers querying until v2 metadata is ready, and supports navigation to v2 terminal tabs.
    • UI refresh: severity dots replace badges, inline RAM share progress bar, tighter typography/separators, and soft hovers. The monitor is placed next to NavigationControls in the v2 sidebar header (history clock hidden), with a TopBar fallback.
  • Refactors

    • Extract session collection and workspace metadata into session-sources with v1 and v2 providers; CPU/memory aggregation remains process-tree based.
    • Tighten snapshot normalization (including optional title) and remove direct local DB reads from the collector for v2; add tests for title preservation, v2 parsing, and host-service integration.

Written for commit b586f14. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Optional session titles added to resource metrics and surfaced in the UI
    • Snapshot collection accepts a surface (v1/v2) and organization scope for scoped snapshots
    • V2-aware naming, navigation, and workspace/session metadata reconciliation
  • Refactor

    • Snapshot caching keyed per-scope; v2 session sources and parsing added
    • Resource-consumption UI components reorganized and styling updated
  • Tests

    • Unit and integration tests for title preservation, listing, and v2 parsing

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: af22dc90-d187-43da-b5ca-a8588d24fb23

📥 Commits

Reviewing files that changed from the base of the PR and between 3a89e37 and b586f14.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx

📝 Walkthrough

Walkthrough

This PR extends resource metrics to support v1/v2 session discovery with per-scope caching, threads optional session.title through parsing/collection/validation, exposes a host endpoint for joined terminal sessions, forwards surface/org via tRPC, and updates UI components to present and override session titles for v2.

Changes

Multi-Surface Resource Metrics with Session Titles

Layer / File(s) Summary
Data Types & Contracts
apps/desktop/src/lib/trpc/routers/resource-metrics.schema.ts, apps/desktop/src/renderer/.../ResourceConsumption/types.ts, apps/desktop/src/main/lib/resource-metrics/session-normalization.ts, packages/host-service/src/terminal/resource-sessions.ts
Resource metrics/session shapes updated with optional `title: string
Session Collection Helpers
apps/desktop/src/main/lib/resource-metrics/session-sources.ts
Implements v1 collection from runtime registry and v2 per-org HTTP endpoints, merges WorkspaceSessionMap results, and exposes surface-aware getWorkspaceMetadata.
Parsing & Normalization
apps/desktop/src/main/lib/resource-metrics/session-normalization.ts, apps/desktop/src/renderer/.../ResourceConsumption/utils/normalizeSnapshot.ts
Adds V2 payload parsing (parseV2ResourceSessions) with PID validation and normalizeOptionalTitle/normalizeOptionalString to coerce titles to trimmed strings or null.
Host Service Resource Sessions
packages/host-service/src/terminal/resource-sessions.ts, packages/host-service/src/terminal/terminal.ts
New listTerminalResourceSessions joins live daemon sessions to active DB rows and GET /terminal/resource-sessions exposes joined sessions with nullable title.
Metrics Collection & Caching
apps/desktop/src/main/lib/resource-metrics/index.ts
collectResourceMetrics accepts surface and organizationId, uses Map-based per-scope cachedSnapshots and inflightCollections, delegates to parameterized worker, and includes session.title in normalized output.
tRPC Router
apps/desktop/src/lib/trpc/routers/resource-metrics.ts
getSnapshot input schema extended with optional surface (`"v1"
Client UI Components
apps/desktop/src/renderer/.../TopBar/TopBar.tsx, .../ResourceConsumption/ResourceConsumption.tsx, .../WorkspaceResourceSection/WorkspaceResourceSection.tsx, .../NavigationControls/NavigationControls.tsx, .../DashboardSidebarHeader/DashboardSidebarHeader.tsx
ResourceConsumption accepts surface, derives organizationId for v2, requests scoped snapshots, computes terminal title overrides from pane layout, reconciles v2 metadata, and displays session.title; TopBar and sidebar render ResourceConsumption appropriately; NavigationControls gains showHistoryDropdown prop.
Tests & Validation
apps/desktop/src/lib/trpc/routers/resource-metrics.schema.test.ts, apps/desktop/src/main/lib/resource-metrics/session-sources.test.ts, packages/host-service/test/integration/terminal.integration.test.ts
New tests: schema test ensures optional title preserved through validation; V2 parsing tests validate grouping and title normalization; integration test verifies daemon-sourced sessions join to active DB rows and include title.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

  • Possibly related PRs
    • superset-sh/superset#2111: Overlapping resource-metrics additions (session.title, collectResourceMetrics options, normalization, and session-sources).

🐰 Across surfaces v1 and v2, metrics now flow,
Titles trim to meaning where once blanks would grow,
Per-scope caches hum, concurrent and neat,
Host lists and endpoints make the session map complete,
UI listens, reconciles, and shows each terminal's name.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title '[codex] Restore v2 resource monitor' clearly and concisely summarizes the main change: restoring the resource monitor functionality for v2 while preserving v1.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering summary, rationale, and validation steps. It addresses the core changes, new features, refactors, and includes both standard and auto-generated summaries.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch resource-monitor-v2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch

Thank you for your contribution! 🎉

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 9, 2026

Greptile Summary

Restores the resource monitor for v2 by routing terminal PID discovery through the PTY daemon via a new host-service endpoint, while keeping the existing v1 path untouched. Project/workspace display names are hydrated in the renderer from Electric collections, and terminal titles flow from daemon session state with pane-layout overrides as a fallback.

  • New session-sources.ts splits v1 (workspace runtime registry) and v2 (daemon-backed HTTP) session collection, with parseV2ResourceSessions doing safe defensive parsing of the network payload.
  • collectResourceMetrics gains per-surface+org cache keys and inflight deduplication with Maps, replacing the single shared variables.
  • The host-service /terminal/resource-sessions endpoint joins live daemon SessionInfo with active DB rows and is correctly protected by the existing app.use(\"/terminal/*\", wsAuth) wildcard middleware.

Confidence Score: 4/5

Safe to merge; the new v2 path is additive and the v1 path is unchanged.

The core collection logic, auth, and daemon integration are all correct. The two open questions are whether v2 workspace pane layouts live in collections.workspaces and whether a snapshot arriving before Electric hydrates will temporarily misgroup v2 workspaces. Neither causes data loss or crashes, and both self-correct once collections load.

apps/desktop/src/main/lib/resource-metrics/session-sources.ts (stub v2 metadata) and apps/desktop/src/renderer/.../ResourceConsumption.tsx (title-override collection source and hydration-race grouping)

Important Files Changed

Filename Overview
apps/desktop/src/main/lib/resource-metrics/session-sources.ts New module cleanly separates v1 (registry-based) and v2 (daemon-backed HTTP) session collection. parseV2ResourceSessions is defensive with runtime type guards. getWorkspaceMetadata returns stub data for v2 that the renderer is expected to hydrate.
apps/desktop/src/main/lib/resource-metrics/index.ts Refactored to per-surface+org caching and inflight deduplication with Maps. normalizeSnapshot is called twice (once inside collectResourceMetricsNow and once in the .then handler), which is idempotent but redundant — pre-existing pattern, not a regression.
packages/host-service/src/terminal/resource-sessions.ts New function joins live daemon sessions with active DB rows, correctly filtering by status=active and non-empty originWorkspaceId. Math.floor applied to pid; dead-pane guard validated by tests.
packages/host-service/src/terminal/terminal.ts New /terminal/resource-sessions GET route is correctly protected by the existing app.use("/terminal/*", wsAuth) middleware. Calls getDaemonClient(); any daemon error propagates as a 500 that the caller in session-sources.ts already handles gracefully.
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx Largest change — adds v2 support with Electric hydration of project/workspace names and pane-title overrides. normalizedSnapshot is now a useMemo above the !enabled early return (correct React hooks ordering). v2 workspace grouping may briefly fall back to stub project during Electric hydration.
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx ResourceConsumption now always renders with surface prop — removes the v2 conditional hide. Clean one-line change.
packages/host-service/test/integration/terminal.integration.test.ts Good coverage: active, disposed, exited, orphan, unknown, and duplicate-with-dead-alive terminals are all exercised. pid floor (123.9→123) and title propagation validated.
apps/desktop/src/lib/trpc/routers/resource-metrics.schema.ts Adds nullable optional title to session schema; zod.string().nullable().optional() is the correct typing.

Sequence Diagram

sequenceDiagram
    participant R as Renderer
    participant T as tRPC Router
    participant RM as resource-metrics/index.ts
    participant SS as session-sources.ts
    participant HS as host-service /terminal/resource-sessions
    participant D as PTY Daemon
    participant DB as Host DB

    R->>T: "getSnapshot({ surface, organizationId, mode })"
    T->>RM: "collectResourceMetrics({ surface, organizationId })"
    RM->>RM: check cachedSnapshots[surface:orgId]
    alt cache miss
        RM->>SS: "collectWorkspaceSessionMap({ surface, organizationId })"
        alt "surface == v2"
            SS->>HS: GET /terminal/resource-sessions (Bearer secret)
            HS->>D: daemon.list()
            D-->>HS: SessionInfo[]
            HS->>DB: SELECT terminalSessions WHERE id IN (liveIds)
            DB-->>HS: rows (status, originWorkspaceId)
            HS-->>SS: sessions: TerminalResourceSession[]
            SS->>SS: parseV2ResourceSessions + merge
        else "surface == v1"
            SS->>SS: registry.getDefault().terminal.management.listSessions()
        end
        SS-->>RM: WorkspaceSessionMap
        RM->>RM: captureProcessSnapshot + CPU/mem aggregation
        RM->>RM: normalizeSnapshot → cachedSnapshots.set
    end
    RM-->>T: ResourceMetricsSnapshot
    T-->>R: snapshot (with title, stub projectId for v2)
    R->>R: hydrate projectName/workspaceName from Electric
    R->>R: apply terminalTitleOverrides from paneLayout
Loading

Comments Outside Diff (2)

  1. apps/desktop/src/main/lib/resource-metrics/session-sources.ts, line 471-478 (link)

    P2 Stub projectId leaks into renderer grouping on hydration race

    For v2, all workspaces are seeded with projectId: "v2" as a placeholder. In the renderer the real value is resolved via workspaceById.get(workspace.workspaceId)?.projectId ?? workspace.projectId. If the Electric collection hasn't loaded yet when the first snapshot arrives, every v2 workspace falls back to projectId: "v2", and WorkspaceResourceSection groups them all under a single "V2 Workspaces" bucket — including workspaces that belong to different real projects. Once the collection hydrates, a subsequent useMemo recompute fixes the grouping, but the initial render shows incorrect project attribution. A defensive workaround is to gate rendering the workspace list on the Electric collections being non-empty (similar to how shouldQueryMetrics already guards the query), so the first painted snapshot always has accurate metadata.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/desktop/src/main/lib/resource-metrics/session-sources.ts
    Line: 471-478
    
    Comment:
    **Stub `projectId` leaks into renderer grouping on hydration race**
    
    For v2, all workspaces are seeded with `projectId: "v2"` as a placeholder. In the renderer the real value is resolved via `workspaceById.get(workspace.workspaceId)?.projectId ?? workspace.projectId`. If the Electric collection hasn't loaded yet when the first snapshot arrives, every v2 workspace falls back to `projectId: "v2"`, and `WorkspaceResourceSection` groups them all under a single "V2 Workspaces" bucket — including workspaces that belong to different real projects. Once the collection hydrates, a subsequent `useMemo` recompute fixes the grouping, but the initial render shows incorrect project attribution. A defensive workaround is to gate rendering the workspace list on the Electric collections being non-empty (similar to how `shouldQueryMetrics` already guards the query), so the first painted snapshot always has accurate metadata.
    
    How can I resolve this? If you propose a fix, please make it concise.
  2. apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx, line 593-596 (link)

    P2 terminalTitleOverrides built from v1 workspace collection may not include v2 pane layouts

    rawSidebarWorkspaces is sourced from collections.workspaces, which is the v1/local workspace collection. If v2 workspace pane layouts (containing the per-pane titleOverride and data.terminalId) are stored only in collections.v2Workspaces, then getTerminalTitleOverrides will iterate over rows that have no v2 terminal panes and the override map will always be empty for v2. The fallback is graceful (terminals show their daemon-reported title or null), but intentional user-set title overrides silently won't appear. Consider sourcing the override scan from rawV2Workspaces (or a combined query) when isV2 is true.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx
    Line: 593-596
    
    Comment:
    **`terminalTitleOverrides` built from v1 workspace collection may not include v2 pane layouts**
    
    `rawSidebarWorkspaces` is sourced from `collections.workspaces`, which is the v1/local workspace collection. If v2 workspace pane layouts (containing the per-pane `titleOverride` and `data.terminalId`) are stored only in `collections.v2Workspaces`, then `getTerminalTitleOverrides` will iterate over rows that have no v2 terminal panes and the override map will always be empty for v2. The fallback is graceful (terminals show their daemon-reported title or `null`), but intentional user-set title overrides silently won't appear. Consider sourcing the override scan from `rawV2Workspaces` (or a combined query) when `isV2` is true.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
apps/desktop/src/main/lib/resource-metrics/session-sources.ts:471-478
**Stub `projectId` leaks into renderer grouping on hydration race**

For v2, all workspaces are seeded with `projectId: "v2"` as a placeholder. In the renderer the real value is resolved via `workspaceById.get(workspace.workspaceId)?.projectId ?? workspace.projectId`. If the Electric collection hasn't loaded yet when the first snapshot arrives, every v2 workspace falls back to `projectId: "v2"`, and `WorkspaceResourceSection` groups them all under a single "V2 Workspaces" bucket — including workspaces that belong to different real projects. Once the collection hydrates, a subsequent `useMemo` recompute fixes the grouping, but the initial render shows incorrect project attribution. A defensive workaround is to gate rendering the workspace list on the Electric collections being non-empty (similar to how `shouldQueryMetrics` already guards the query), so the first painted snapshot always has accurate metadata.

### Issue 2 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx:593-596
**`terminalTitleOverrides` built from v1 workspace collection may not include v2 pane layouts**

`rawSidebarWorkspaces` is sourced from `collections.workspaces`, which is the v1/local workspace collection. If v2 workspace pane layouts (containing the per-pane `titleOverride` and `data.terminalId`) are stored only in `collections.v2Workspaces`, then `getTerminalTitleOverrides` will iterate over rows that have no v2 terminal panes and the override map will always be empty for v2. The fallback is graceful (terminals show their daemon-reported title or `null`), but intentional user-set title overrides silently won't appear. Consider sourcing the override scan from `rawV2Workspaces` (or a combined query) when `isV2` is true.

Reviews (1): Last reviewed commit: "Restore v2 resource monitor" | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/main/lib/resource-metrics/index.ts (1)

137-143: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Normalize blank session titles to null.

The renderer falls back on session.title with ??, so preserving "" here produces an empty label instead of the Terminal … fallback. Coerce empty/whitespace-only titles to null during normalization.

Suggested patch
 		const sessions = workspace.sessions.map((session) => ({
 			sessionId: session.sessionId,
 			paneId: session.paneId,
 			pid: Math.max(0, Math.floor(normalizeFiniteNumber(session.pid))),
-			title: typeof session.title === "string" ? session.title : null,
+			title:
+				typeof session.title === "string" &&
+				session.title.trim().length > 0
+					? session.title
+					: null,
 			cpu: normalizeFiniteNumber(session.cpu),
 			memory: normalizeFiniteNumber(session.memory),
 		}));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/main/lib/resource-metrics/index.ts` around lines 137 - 143,
The session mapping currently preserves empty or whitespace-only session.title
(in the sessions array mapping in resource-metrics index.ts), which prevents the
renderer's fallback from applying; update the title normalization in that
mapping so that session.title is coerced to null when it's not a non-empty
string (e.g., check typeof session.title === "string" &&
session.title.trim().length > 0 ? session.title : null) so empty/whitespace-only
titles become null; keep other normalizations (pid, cpu, memory) unchanged and
ensure you update the title expression where sessions are built.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/desktop/src/main/lib/resource-metrics/session-sources.ts`:
- Around line 129-136: The v2 fetch to
`http://127.0.0.1:${connection.port}/terminal/resource-sessions` is unbounded;
wrap the call with an AbortController (pass controller.signal to fetch), start a
timeout (e.g., configurable default like 5s) that calls controller.abort(), and
clear the timeout after fetch resolves; ensure the code around this call
(references: connection.port, connection.secret, the fetch invocation in
session-sources) properly catches AbortError and handles/logs a timeout case
instead of hanging.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx`:
- Around line 218-223: In the sessions mapping that builds title, replace the
lookup of terminalTitleOverrides using session.sessionId with session.paneId
(the v2 terminal identifier) and change the precedence so that
terminalTitleOverrides.get(session.paneId) wins over session.title (i.e., use
override first, then fall back to session.title, then null); update the code
around sessions: workspace.sessions.map(...) where session.title is assigned to
implement this lookup and precedence change.

In `@packages/host-service/src/terminal/resource-sessions.ts`:
- Around line 20-23: The current guard allows fractional PIDs (e.g., 0.4) that
get floored later and become 0; change the predicate to require an integer PID
so fractions are rejected: replace the check using Number.isFinite(session.pid)
with Number.isInteger(session.pid) (and keep session.pid > 0) where session.pid
is validated, and ensure the later Math.floor usage (the flooring at the other
PID usage) either is removed or only applied to an already-validated integer PID
to avoid creating a 0 pid from a fractional input.

---

Outside diff comments:
In `@apps/desktop/src/main/lib/resource-metrics/index.ts`:
- Around line 137-143: The session mapping currently preserves empty or
whitespace-only session.title (in the sessions array mapping in resource-metrics
index.ts), which prevents the renderer's fallback from applying; update the
title normalization in that mapping so that session.title is coerced to null
when it's not a non-empty string (e.g., check typeof session.title === "string"
&& session.title.trim().length > 0 ? session.title : null) so
empty/whitespace-only titles become null; keep other normalizations (pid, cpu,
memory) unchanged and ensure you update the title expression where sessions are
built.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fd36b4bc-b1d2-4a31-b849-4bbe622a7c7e

📥 Commits

Reviewing files that changed from the base of the PR and between 4bd6357 and 9d6ed16.

📒 Files selected for processing (13)
  • apps/desktop/src/lib/trpc/routers/resource-metrics.schema.test.ts
  • apps/desktop/src/lib/trpc/routers/resource-metrics.schema.ts
  • apps/desktop/src/lib/trpc/routers/resource-metrics.ts
  • apps/desktop/src/main/lib/resource-metrics/index.ts
  • apps/desktop/src/main/lib/resource-metrics/session-sources.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/components/WorkspaceResourceSection/WorkspaceResourceSection.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/types.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/utils/normalizeSnapshot.ts
  • packages/host-service/src/terminal/resource-sessions.ts
  • packages/host-service/src/terminal/terminal.ts
  • packages/host-service/test/integration/terminal.integration.test.ts

Comment thread apps/desktop/src/main/lib/resource-metrics/session-sources.ts
Comment thread packages/host-service/src/terminal/resource-sessions.ts
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 13 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/host-service/src/terminal/terminal.ts">

<violation number="1" location="packages/host-service/src/terminal/terminal.ts:970">
P2: Handle daemon failures explicitly in this route. `getDaemonClient()` and `daemon.list()` can reject, and without a local catch this endpoint falls through the generic error path instead of returning a stable response payload.

(Based on your team's feedback about handling daemon-list failures explicitly.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/desktop/src/main/lib/resource-metrics/session-sources.ts">

<violation number="1" location="apps/desktop/src/main/lib/resource-metrics/session-sources.ts:129">
P2: Add an abort timeout for this host-service request. Without a timeout, a stalled `/terminal/resource-sessions` call can keep the v2 collection promise pending and block snapshot refreshes for the same cache key.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread packages/host-service/src/terminal/terminal.ts
Comment thread apps/desktop/src/main/lib/resource-metrics/session-sources.ts
Kitenite added 4 commits May 9, 2026 16:36
Tighter typography, hairline separators, severity dots in place of badges,
soft hover states, and an inline RAM share progress bar in the popover header.
Biome reformatted nested cn() calls onto single lines, and the severity
dot now uses role="img" so its aria-label is valid.
Renders ResourceConsumption beside NavigationControls in the v2 sidebar
header when it hosts chrome, and falls back to the TopBar otherwise. The
HistoryDropdown clock icon is suppressed in both locations to free up
the slot.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/components/WorkspaceResourceSection/WorkspaceResourceSection.tsx (1)

172-225: ⚡ Quick win

Add aria-expanded on the collapse/expand toggles.

The project (Lines 172-179) and workspace (Lines 209-222) toggle buttons only flip aria-label between “Expand …” and “Collapse …”. The canonical ARIA pattern for a button that shows/hides a region is aria-expanded (and ideally aria-controls pointing at the region). Screen readers will announce state transitions consistently with that attribute, while a label swap is easy to miss.

♿ Suggested change
 <button
     type="button"
     onClick={() => toggleProject(project.projectId)}
     className="group w-full flex items-center justify-between px-2 py-1.5 hover:bg-foreground/[0.04] transition-colors"
-    aria-label={
-        isProjectCollapsed ? "Expand project" : "Collapse project"
-    }
+    aria-expanded={!isProjectCollapsed}
+    aria-label={`${isProjectCollapsed ? "Expand" : "Collapse"} project ${project.projectName}`}
 >
 <button
     type="button"
     onClick={() => toggleWorkspace(workspace.workspaceId)}
     className="flex items-center justify-center h-7 w-5 ml-3.5 shrink-0 text-muted-foreground/60 hover:text-muted-foreground transition-colors"
-    aria-label={
-        isCollapsed ? "Expand workspace" : "Collapse workspace"
-    }
+    aria-expanded={!isCollapsed}
+    aria-label={`${isCollapsed ? "Expand" : "Collapse"} workspace ${workspace.workspaceName}`}
 >
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/components/WorkspaceResourceSection/WorkspaceResourceSection.tsx`
around lines 172 - 225, The project and workspace toggle buttons (the button
using toggleProject and the button using toggleWorkspace) need aria-expanded and
aria-controls for proper screen-reader state; add
aria-expanded={isProjectCollapsed ? "false" : "true"} on the project toggle
(using isProjectCollapsed) and aria-expanded={!isCollapsed} on the workspace
toggle (using isCollapsed), generate stable IDs for the collapsible regions
(e.g. `project-panel-${project.projectId}` and
`workspace-panel-${workspace.workspaceId}`) and add aria-controls on each button
pointing to its region ID, then set the matching id attributes on the DOM
elements that contain the collapsible content (the container that renders the
workspaces for the project and the container that renders sessions for the
workspace).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/components/WorkspaceResourceSection/WorkspaceResourceSection.tsx`:
- Around line 172-225: The project and workspace toggle buttons (the button
using toggleProject and the button using toggleWorkspace) need aria-expanded and
aria-controls for proper screen-reader state; add
aria-expanded={isProjectCollapsed ? "false" : "true"} on the project toggle
(using isProjectCollapsed) and aria-expanded={!isCollapsed} on the workspace
toggle (using isCollapsed), generate stable IDs for the collapsible regions
(e.g. `project-panel-${project.projectId}` and
`workspace-panel-${workspace.workspaceId}`) and add aria-controls on each button
pointing to its region ID, then set the matching id attributes on the DOM
elements that contain the collapsible content (the container that renders the
workspaces for the project and the container that renders sessions for the
workspace).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 96e2c3c2-15b7-4ff0-90b7-c966ac15dc71

📥 Commits

Reviewing files that changed from the base of the PR and between 9c6304c and 3a89e37.

📒 Files selected for processing (9)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarHeader/DashboardSidebarHeader.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/NavigationControls/NavigationControls.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/components/AppResourceSection/AppResourceSection.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/components/MetricBadge/MetricBadge.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/components/UsageSeverityBadge/UsageSeverityBadge.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/components/WorkspaceResourceSection/WorkspaceResourceSection.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/utils/resourceSeverity.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx

The override map is keyed by terminal ID, which the v2 navigation path
treats as `session.paneId`. The previous lookup used `sessionId` and
silently missed every override. Also reorder so a user-set rename wins
over the daemon-reported title.
@Kitenite Kitenite merged commit 3365030 into main May 10, 2026
17 checks passed
saddlepaddle added a commit that referenced this pull request May 10, 2026
#4370)

* fix(desktop): restore recently-viewed dropdown for v2 + tighten topbar chrome

Recently-viewed dropdown was permanently disabled in v2: the path matcher
only knew about /workspace/$id (v1) and the slug capture greedily consumed
search params and nested subroutes, so /tasks/SUP-123?tab=… and
/v2-workspace/$id never resolved to a known entity. Add a v2-workspace
resource type, an automation resource type, strip search/hash before
matching, anchor to the first path segment, and dedupe by canonical path.
Filter cross-version workspace entries so clicking a v1 entry while in v2
mode no longer drops you on the cross-version mismatch screen. Re-show
the dropdown trigger in both topbar and v2 sidebar header (was hardcoded
to false in #4314 because it was always rendering as the disabled icon).

Also: collapse the resource monitor trigger to an icon-only ghost button
and float it to the far right of its row in both surfaces.

* fix(desktop): hide automation entries in recently-viewed when v1 is active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant