Skip to content

feat(inspector): Central Stage Empty State & Connect Button [TASK-017] - #154

Merged
gabrypavanello merged 6 commits into
mainfrom
feat/central-stage-empty-state
Feb 6, 2026
Merged

feat(inspector): Central Stage Empty State & Connect Button [TASK-017]#154
gabrypavanello merged 6 commits into
mainfrom
feat/central-stage-empty-state

Conversation

@gabrypavanello

Copy link
Copy Markdown
Contributor

Summary

Replace NoWidgetPlaceholder with a 3-state stepped tutorial that guides users through connecting a server and agent.

Changes

  • Backend: Added agent-initialize event type and maybeRecordInitialize() helper to intercept MCP initialize requests in both standalone and dual servers
  • Frontend: Updated NoWidgetPlaceholder with 3-state UI (no-server → server-connected → agent-connected)
  • UX: TabBar hidden when no connections, central stage becomes the entry point

3-State Flow

  1. No server: Star logo + "Debug MCP servers alongside your Agent" + "Connect the server you want to inspect" + [Connect Server] button
  2. Server connected: "Connect your Agent to this MCP Server" + subtext about capturing tool calls
  3. Agent connected: "Ready to Test" + "with {clientName}"

Testing

  • ✅ Build passes
  • ✅ 2974 tests pass
  • ✅ Altair reviewed: all 8 criteria met
  • ✅ Polaris verified

Files Changed

  • inspector-event-types.ts — agent-initialize event type
  • connection.ts — maybeRecordInitialize helper
  • standalone-server.ts + dual-server.ts — initialize interception
  • NoWidgetPlaceholder.tsx — 3-state UI
  • TabBar.tsx — conditional visibility
  • InspectorDashboard.tsx — state wiring

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Feb 6, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Detect and log agent initialization handshakes (agent-initialize event).
    • Dashboard shows three connection states: no server, server connected, agent connected.
    • Displays connected agent names and adds a "Connect" action in the placeholder UI.
  • Bug Fixes / Improvements

    • Tab bar now safely handles empty-tab cases without rendering.
  • Tests

    • Added tests covering detection and recording of agent initialization events.

Walkthrough

Adds detection and recording of MCP JSON-RPC initialize requests (agent-initialize events), wires that into server request handling, updates the dashboard placeholder to a three-state connection UI, and introduces a new event type for agent initialization.

Changes

Cohort / File(s) Summary
Connection manager & event types
packages/inspector/src/connection.ts, packages/inspector/src/types/inspector-event-types.ts
Added maybeRecordInitialize(jsonRpcBody: unknown): boolean to validate JSON-RPC initialize requests and record agent-initialize events; added agent-initialize to InspectorEventType and summary/category handling.
Server MCP handling
packages/inspector/src/dual-server.ts, packages/inspector/src/standalone-server.ts
Call maybeRecordInitialize when MCP bodies are parsed (both /agent/mcp and /apps/mcp paths); wrapped parsing in try/catch and consolidated connection manager retrieval in standalone server.
Dashboard UI
packages/inspector/src/dashboard/react/InspectorDashboard.tsx, packages/inspector/src/dashboard/react/components/NoWidgetPlaceholder.tsx, packages/inspector/src/dashboard/react/components/TabBar.tsx
NoWidgetPlaceholder changed to props-driven component with ConnectionState ("no-server"
Tests
packages/inspector/tests/connection.test.ts
Added tests for maybeRecordInitialize covering valid initialize payloads, missing params/clientInfo, non-initialize methods, and invalid structures.

Sequence Diagram

sequenceDiagram
    participant Client as MCP Client
    participant Server as Dual/Standalone Server
    participant ConnMgr as ConnectionManager
    participant EventSystem as Event System
    participant UI as Dashboard UI

    Client->>Server: POST JSON-RPC (body)
    activate Server
    Server->>Server: Parse request body (try/catch)
    Server->>ConnMgr: maybeRecordInitialize(jsonRpcBody)
    activate ConnMgr
    ConnMgr->>ConnMgr: Validate JSON-RPC initialize via zod
    alt valid initialize
        ConnMgr->>EventSystem: recordAgentEvent("agent-initialize", payload)
        activate EventSystem
        EventSystem->>EventSystem: store/publish event
        deactivate EventSystem
        ConnMgr-->>Server: true
    else not initialize/invalid
        ConnMgr-->>Server: false
    end
    deactivate ConnMgr
    Server->>Server: continue handling request/response
    Server-->>Client: JSON-RPC response
    deactivate Server
    Note right of EventSystem: UI subscribes to events
    EventSystem->>UI: publish agent-initialize
    activate UI
    UI->>UI: update connectionState -> "agent-connected"
    UI->>UI: display clientName
    deactivate UI
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly reflects the main PR objective of creating a central stage empty state UI with a connect button, matching the substantial frontend changes to NoWidgetPlaceholder and related components.
Description check ✅ Passed The description comprehensively covers the changeset, detailing both backend event tracking and frontend state UI changes, and explicitly lists all modified files with clear explanations of their purpose.

✏️ 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 feat/central-stage-empty-state

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

@claude

claude Bot commented Feb 6, 2026

Copy link
Copy Markdown
Contributor

Code Review: PR #154 - Central Stage Empty State & Connect Button

Summary

This PR implements a well-structured 3-state tutorial flow for the inspector dashboard, replacing the static placeholder with a progressive onboarding experience. The implementation is clean and follows the repository's TypeScript standards.


Strengths

  1. Type Safety: Strong TypeScript usage throughout

    • Proper union type for ConnectionState ("no-server" | "server-connected" | "agent-connected")
    • Correct type annotations on React components and props
    • No use of any types (adheres to repo policy)
  2. Clean Architecture:

    • Event-driven design with agent-initialize event type properly integrated into the event system
    • Clear separation of concerns: backend detection logic, frontend state management, UI rendering
    • Follows existing patterns in the codebase (event types, connection manager methods)
  3. Code Quality:

    • Well-documented with JSDoc comments
    • Consistent code style matching the repository conventions
    • Proper error handling with graceful fallbacks
  4. UX Design:

    • Progressive disclosure with 3 clear states
    • Helpful guidance at each stage
    • Clean visual design with proper spacing and typography

⚠️ Issues & Recommendations

1. Missing Test Coverage (CRITICAL)

The new maybeRecordInitialize method in ConnectionManager has no test coverage. This is a blocking issue per repo policy:

  • AGENTS.md line 56: "ALWAYS add/adjust tests under the owning package's tests/ when changing behavior; coverage thresholds are enforced at the repo level."

Required actions:

  • Add tests to packages/inspector/tests/connection.test.ts covering:
    • Valid initialize request with clientInfo
    • Initialize request missing clientInfo (should still record event)
    • Invalid JSON-RPC structure (should return false)
    • Non-initialize requests (should return false)
    • Verify agent-initialize event is recorded with correct payload

Example test structure:

describe('maybeRecordInitialize', () => {
  it('should record agent-initialize event with clientInfo', () => {
    const manager = new ConnectionManager();
    const spy = vi.spyOn(manager, 'recordAgentEvent');

    const result = manager.maybeRecordInitialize({
      method: 'initialize',
      params: {
        clientInfo: { name: 'claude-code', version: '1.0.0' }
      }
    });

    expect(result).toBe(true);
    expect(spy).toHaveBeenCalledWith('agent-initialize', {
      clientName: 'claude-code',
      clientVersion: '1.0.0'
    });
  });

  // Additional test cases...
});

2. Type Safety: Unsafe Type Assertions

connection.ts:1220-1240 uses multiple type assertions without proper validation:

const body = jsonRpcBody as Record<string, unknown>;
const paramsObj = params as Record<string, unknown>;
const clientInfoObj = clientInfo as Record<string, unknown>;

Recommendation: Use Zod schema validation (the repo uses Zod v4):

import { z } from 'zod';

const InitializeRequestSchema = z.object({
  method: z.literal('initialize'),
  params: z.object({
    clientInfo: z.object({
      name: z.string().optional(),
      version: z.string().optional(),
    }).optional(),
  }).optional(),
});

// In maybeRecordInitialize:
const parsed = InitializeRequestSchema.safeParse(jsonRpcBody);
if (!parsed.success) {
  return false;
}
const clientInfo = parsed.data.params?.clientInfo;

This provides runtime validation and eliminates the need for type assertions.

3. React Performance: Missing Memoization

NoWidgetPlaceholder.tsx:83-131 - The button event handlers are created on every render:

onMouseEnter={(e) => { /* ... */ }}
onMouseLeave={(e) => { /* ... */ }}

Recommendation: Extract to useCallback or define outside component (these are pure functions):

const handleButtonHover = (e: React.MouseEvent<HTMLButtonElement>) => {
  e.currentTarget.style.opacity = "0.9";
  e.currentTarget.style.transform = "scale(1.02)";
};

const handleButtonLeave = (e: React.MouseEvent<HTMLButtonElement>) => {
  e.currentTarget.style.opacity = "1";
  e.currentTarget.style.transform = "scale(1)";
};

Minor optimization, but follows React best practices.

4. Accessibility

NoWidgetPlaceholder.tsx:93-105 - The "Connect Server" button lacks proper accessibility attributes:

Recommendations:

  • Add aria-label for screen readers
  • Consider keyboard focus styling (currently only has hover)
  • The heading uses <h2> but there may not be an <h1> in the parent scope
<button
  type="button"
  aria-label="Connect to MCP server"
  style={localStyles.connectButton}
  onClick={onConnect}
>
  Connect Server
</button>

5. Error Handling in Server Interceptors

dual-server.ts:721 and standalone-server.ts:817 - Silent error swallowing:

} catch {
  // Not valid JSON, ignore
}

Recommendation: At minimum, log in debug mode:

} catch (err) {
  if (config.debug) {
    console.log('[inspector] Failed to parse JSON-RPC body:', err);
  }
}

This aids debugging without being noisy in production.


🔍 Additional Observations

  1. TabBar Conditional Rendering: The change to return null when no tabs (TabBar.tsx:139-141) is clean, but ensure this doesn't break any parent component layout assumptions.

  2. State Management: The useMemo hooks for connectionState and agentClientName in InspectorDashboard.tsx are well-optimized.

  3. Event System Integration: The agent-initialize event type is properly integrated:

    • Added to InspectorEventType union
    • Categorized as "agent" in getEventCategory
    • Has summary logic in getEventSummary

📋 Checklist Before Merge

  • Add test coverage for maybeRecordInitialize method
  • Consider Zod validation for type safety
  • Add accessibility attributes to button
  • Improve error logging in catch blocks
  • Run pnpm build && pnpm test && pnpm lint && pnpm typecheck (per AGENTS.md line 53)

🎯 Verdict

Requires Changes - The implementation is solid, but missing test coverage is a blocking issue per repository policy. Once tests are added and the minor type safety improvements are made, this will be ready to merge.

Estimated effort to address: ~30-45 minutes for comprehensive test coverage + minor fixes.


Great work on the progressive UX flow! The 3-state tutorial approach is intuitive and the code is generally clean. Just need to add those tests before merging. 🚀

@claude

claude Bot commented Feb 6, 2026

Copy link
Copy Markdown
Contributor

Code Review for PR #154: Central Stage Empty State & Connect Button

Overall Assessment

This PR successfully implements a 3-state stepped tutorial UI for the inspector dashboard. The implementation is well-structured, type-safe, and follows repository conventions. All tests pass (2974), and the code demonstrates good practices in TypeScript, React, and testing.


Strengths

1. Excellent Type Safety

  • Proper use of Zod v4 for JSON-RPC validation in connection.ts:1190-1202
  • Strict TypeScript types throughout (no any usage)
  • Well-defined ConnectionState union type with clear semantics
  • Type-safe payload extraction using Zod safeParse

2. Comprehensive Test Coverage

  • 6 test cases covering all edge cases in maybeRecordInitialize
  • Tests validate positive cases (with/without clientInfo), negative cases (invalid method, missing fields)
  • Clear test descriptions and assertions
  • Tests align with repository 50% coverage thresholds

3. Clean Architecture

  • Proper separation of concerns (backend event detection, frontend state management, UI rendering)
  • Module-level constants for event handlers optimize for React rendering
  • Immutable state updates using useMemo and useCallback

4. Accessibility and UX

  • aria-label attributes on interactive elements
  • Semantic HTML (h2, p, button)
  • Keyboard navigation support in TabBar component
  • Clear visual hierarchy

Code Quality Analysis

Backend Changes

connection.ts - maybeRecordInitialize (lines 1213-1243)

  • Uses Zod for safe validation instead of manual type guards
  • Returns boolean indicating success/failure
  • Handles optional fields gracefully
  • Debug logging with client info

dual-server.ts and standalone-server.ts - Initialize interception

  • Correctly placed before request forwarding
  • Silent error handling for non-JSON bodies (expected in SSE/streaming)
  • Clear comments explaining error handling
  • Null-safe checks for getActiveConnectionManager()

Security: JSON.parse is safe because input is validated by Zod schema and only whitelisted fields are extracted.


Frontend Changes

NoWidgetPlaceholder.tsx - 3-state UI

  • Module-level style objects prevent re-creation on each render
  • Type-safe props interface with JSDoc comments
  • Hover handlers extracted to module scope for memoization
  • as const assertion for flexDirection ensures type safety

InspectorDashboard.tsx - State computation

  • Proper use of useMemo to avoid recomputing connection state
  • Type assertion for payload is safe (validated by Zod upstream)
  • Dependencies array is correct

Potential Race Condition: There could be brief UI flicker if agent-initialize event hasn't propagated to displayAgentEvents yet. Likely acceptable minor issue.

TabBar.tsx - Conditional rendering

  • Clean early return pattern
  • Return type updated to React.ReactElement | null

Security Considerations

No Security Issues Found

  1. Input Validation: Zod schema ensures only expected fields are processed
  2. XSS Prevention: React automatically escapes clientName in JSX
  3. No SQL/Command Injection: No database queries or shell commands
  4. DoS Protection: body.length check prevents processing empty payloads

Performance Considerations

Well-Optimized

  1. No N+1 Issues: Single .some() call for event detection
  2. Memoization: useMemo prevents recalculation on unrelated re-renders
  3. Event Handlers: Module-level handlers avoid re-creation
  4. Bundle Size: No new dependencies added

Potential Improvement: If displayAgentEvents grows to 1000+ events, consider using a Set for faster lookup. However, this is premature optimization for current use case.


Test Coverage

Excellent Coverage

Total: 6 test cases for maybeRecordInitialize

  • Valid initialize with clientInfo (name + version)
  • Valid initialize without clientInfo
  • Invalid structure (no method field)
  • Wrong method (tools/list)
  • Missing params
  • Edge cases handled

Suggestions:

  1. Integration Test: Consider adding end-to-end test verifying MCP initialize request creates agent-initialize event
  2. React Component Test: Add tests for NoWidgetPlaceholder rendering and button click behavior

Repository Compliance

Follows AGENTS.md Rules

  • Zod v4 used correctly (not v3)
  • No any types introduced
  • Exports contract maintained (no deep imports)
  • Tests added under owning package
  • Type-safe throughout (strict mode compliance)

Build and Test Status

  • Build passes
  • 2974 tests pass
  • Altair reviewed (8 criteria met)
  • Polaris verified

Code Style

Excellent

  • Consistent formatting (Prettier compliance)
  • Clear comments explaining non-obvious logic
  • JSDoc annotations on exported types/functions
  • Meaningful variable names

Recommendations

Critical: None

High Priority: None

Medium Priority:

  1. Add React Component Tests for NoWidgetPlaceholder to verify rendering states and button interactions
  2. Consider Performance Optimization if event list is expected to grow beyond 1000 items

Low Priority:

  1. Extract Debug Formatting into helper function if pattern repeats
  2. Clarify "Agent" Fallback - consider "Unknown Agent" for better debugging clarity

Final Verdict

APPROVE with minor suggestions

This PR demonstrates excellent engineering practices:

  • Type-safe implementation following strict TypeScript rules
  • Comprehensive test coverage with edge cases
  • Clean architecture with proper separation of concerns
  • No security vulnerabilities or performance issues
  • Follows repository conventions (AGENTS.md)
  • Well-documented code with clear comments

The suggestions above are optional improvements, not blockers. The code is production-ready as-is.

Great work!


Summary for Reviewer

Category Rating Notes
Code Quality 5/5 Clean, well-structured, type-safe
Test Coverage 5/5 Comprehensive with edge cases
Security 5/5 No vulnerabilities found
Performance 5/5 Well-optimized, minor suggestions
Documentation 4/5 Good comments, could add component tests
Repository Compliance 5/5 Fully compliant with AGENTS.md

Overall: 5/5 - Recommended for merge

@gabrypavanello
gabrypavanello merged commit 1b26f52 into main Feb 6, 2026
4 checks passed
@gabrypavanello
gabrypavanello deleted the feat/central-stage-empty-state branch February 6, 2026 12:09
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