fix(desktop): eliminate stale port data from dual-source race condition

[Kitenite]

Summary

• Replaced the dual-source (tRPC query + subscription → Zustand store) port data architecture with the tRPC query as the single source of truth
• Added refetchInterval: 2500ms to keep ports in sync with the scanner cycle, and changed the subscription to invalidate the query for immediate updates instead of directly mutating store state
• Removed unused port-data methods (addPort, removePort, setPorts, etc.) from the Zustand store, keeping only the UI collapse preference

Problem

The ports sidebar showed all ports from ports.json even when they weren't actively listening. The previous fix (#1488) correctly changed mergePorts to only include detected ports, but a race condition in the renderer kept stale data visible:

1. ports.subscribe receives port:remove events → store removes ports ✓
2. ports.getAll query returns (possibly stale snapshot) → setPorts() overwrites the entire store, re-adding removed ports ✗
3. Since the query had no refetchInterval, stale data persisted until the next window-focus refetch

Test plan

• Start dev servers in a workspace, verify ports appear with labels from ports.json
• Stop dev servers, verify ports disappear within ~3 seconds
• Verify ports collapse/expand toggle still works and persists across reloads
• Verify port badges still link to correct terminal panes and open in browser

Summary by CodeRabbit

• Refactor

  • Port list now refreshes on a regular poll, derives directly from detected/enriched ports, and groups/sorts by workspace name and port number.

• New Features

  • Ports include optional workspace labels when available.

• UX

  • Close/kill controls and simplified tooltips are shown consistently; bulk close/kill behaves uniformly. List collapse preference remains persisted.

[coderabbitai]

📝 Walkthrough

Walkthrough

Replaced client-side merged/static port state with a single refetching TRPC source: ports.getAll now returns EnrichedPort[] (includes optional static label) on a POLL interval; local runtime ports store and merge utilities/endpoints for static ports were removed; UI components now consume grouped EnrichedPort data.

Changes

Cohort / File(s)	Summary
Ports data hook apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.ts	Switched from local store/subscriptions to polling ports.getAll.useQuery with PORTS_REFETCH_INTERVAL_MS; grouping and sorting rebuilt from EnrichedPort[]; return shape changed to WorkspacePortGroup[].
Ports store (UI only) apps/desktop/src/renderer/stores/ports/store.ts	Removed runtime ports state and all mutators (addPort, removePort, removePortsForPane, setPorts); retained persisted UI preference isListCollapsed.
TRPC ports router apps/desktop/src/lib/trpc/routers/ports/ports.ts	getAll now returns EnrichedPort[] and enriches detected ports with per-workspace labels (uses internal label cache via loadStaticPorts); removed static-port endpoints (hasStaticConfig, getStatic, getAllStatic, subscribeStatic, subscribeAllStatic).
Static-ports main exports apps/desktop/src/main/lib/static-ports/index.ts	Removed re-exports hasStaticPortsConfig and staticPortsWatcher; now only loadStaticPorts is exported.
UI components: badges & groups apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/components/MergedPortBadge/MergedPortBadge.tsx, .../WorkspacePortGroup/WorkspacePortGroup.tsx	Prop types updated to EnrichedPort / WorkspacePortGroup; interaction guards simplified; close/kill actions always render (removed prior active-only prefilters).
Kill-port hook apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/useKillPort.ts	Now accepts EnrichedPort for single and bulk kill; removed pre-filtering of active/paneId — processes provided ports and reports per-port failures.
Removed utils & merge apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/utils/merge-ports.ts, .../utils/index.ts	Deleted mergePorts implementation and its re-export; client-side static+dynamic merge logic removed.
Shared types apps/desktop/src/shared/types/ports.ts	Added EnrichedPort (extends DetectedPort with `label: string

Sequence Diagram(s)

sequenceDiagram
  participant Renderer as Renderer (usePortsData)
  participant TRPC as TRPC ports.getAll
  participant Loader as static-ports loader
  participant DB as Port Detector

  Renderer->>TRPC: query ports.getAll (poll every PORTS_REFETCH_INTERVAL_MS)
  TRPC->>DB: fetch detected ports
  TRPC->>Loader: loadStaticPorts(worktreePath) → Map<port,label>
  Loader-->>TRPC: Map<port,label>
  TRPC-->>Renderer: EnrichedPort[] (detected ports + label)
  Renderer->>Renderer: group by workspaceId → derive workspaceName, sort
  Renderer-->>UI: render WorkspacePortGroup(s) with EnrichedPort[] lists

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• feat(desktop): add static port configuration via ports.json #683: earlier static-port/merge logic and MergedWorkspaceGroup changes that this PR removes/refactors.
• feat(desktop): generate ports.json in setup and fix static ports watcher #1479: touched static-ports watcher and subscribeAllStatic behavior—directly related to removed static-port subscriptions/exports.
• feat(desktop): replace regex-based port detection with process-based scanning #610: changes to detected port shape and scanning APIs used by the new EnrichedPort/trpc getAll payload.

Poem

🐰
I hopped through code with nimble paws,
Polled each port and learned their laws.
Labels found, grouped tidy and neat,
No extra store — the flow's complete.
A tiny hop, a happy beat.

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

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.
Merge Conflict Detection	⚠️ Warning	⚠️ Unable to check for merge conflicts: Failed to fetch base branch: From https://github.com/superset-sh/superset ! [rejected] main -> main (non-fast-forward) + d8a8897...9f10308 main -> origin/main (forced update)

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(desktop): eliminate stale port data from dual-source race condition' clearly and specifically describes the main change: fixing a race condition that caused stale port data by consolidating the dual-source architecture into a single source of truth.
Description check	✅ Passed	The PR description covers all key template sections: a clear summary of changes, the problem with detailed explanation of the race condition, and a comprehensive test plan. All required sections are present and substantively filled.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch Kitenite/ports-still-showing

⚔️ Resolve merge conflicts (beta)

• Auto-commit resolved conflicts to branch Kitenite/ports-still-showing
• Create stacked PR with resolved conflicts
• Post resolved changes as copyable diffs in a comment

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.ts (1)

30-30: ?? [] creates a new array reference on every render while loading.

While detectedPorts is undefined (initial load), detectedPorts ?? [] allocates a fresh [] each render, causing the downstream useMemo on Line 43 to recompute. This is benign (only during loading) but can be trivially avoided with a module-level constant.

Proposed fix

+const EMPTY_PORTS: EnrichedPort[] = [];
+
 export function usePortsData() {
   ...
-  const ports = detectedPorts ?? [];
+  const ports = detectedPorts ?? EMPTY_PORTS;

apps/desktop/src/lib/trpc/routers/ports/ports.ts (1)

16-25: getLabelsForPath performs synchronous file I/O on every getAll call.

Since getAll is polled every 2.5 s, loadStaticPorts (which uses readFileSync) runs on the main process each cycle for every workspace with active ports. For a desktop app with few workspaces and a small ports.json, this is tolerable, but a module-scoped cache with a TTL or file-watcher invalidation would avoid repeated disk reads when labels rarely change.

Sketch of a simple TTL cache:

const LABEL_CACHE_TTL_MS = 30_000;
const labelCacheByPath = new Map<string, { labels: Map<number, string> | null; cachedAt: number }>();

function getLabelsForPath(worktreePath: string): Map<number, string> | null {
	const now = Date.now();
	const cached = labelCacheByPath.get(worktreePath);
	if (cached && now - cached.cachedAt < LABEL_CACHE_TTL_MS) {
		return cached.labels;
	}

	const result = loadStaticPorts(worktreePath);
	if (!result.exists || result.error || !result.ports) {
		labelCacheByPath.set(worktreePath, { labels: null, cachedAt: now });
		return null;
	}

	const labels = new Map<number, string>();
	for (const p of result.ports) {
		labels.set(p.port, p.label);
	}
	labelCacheByPath.set(worktreePath, { labels, cachedAt: now });
	return labels;
}

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

• ⚠️ Neon database branch
• ⚠️ Electric Fly.io app
• ⚠️ Streams Fly.io app

Thank you for your contribution! 🎉