🔍 Code Quality Report - 2026-04-27

[github-actions]

Weekly Code Quality Report - 2026-04-27

This is an automated report generated by the weekly code quality workflow.

Code Quality Analysis Report

Generated: 2026-04-27
Codebase: OpenHands CLI (openhands_cli/)
Total Lines of Code: ~18,826
Files Analyzed: 237 Python files

---

Executive Summary

The OpenHands CLI codebase demonstrates solid code quality with well-structured separation between the TUI and ACP implementations. Pyright analysis shows zero type errors, indicating strong type safety. However, there are opportunities for improvement in:

1. Code Duplication: Some confirmation handling and event processing logic is duplicated between ACP and TUI
2. Type Hint Coverage: Several functions use dict[str, Any] that could be more specific with TypedDict
3. State Management: Well-designed reactive state in TUI, but some patterns could be shared with ACP
4. Separation of Concerns: Generally excellent, with a few minor areas for improvement

Overall Assessment: Good (with low-hanging fruit improvements available)

---

Type Checking Issues

Summary from Pyright Analysis

{
    "filesAnalyzed": 237,
    "errorCount": 0,
    "warningCount": 0,
    "informationCount": 0
}

Result: ✅ No type checking errors detected.

Type Improvement Opportunities

1. Overly Broad Any Types (Medium Priority)

The following locations use dict[str, Any] where more specific types would improve safety:

File	Location	Current Type	Suggested Improvement
stores/agent_store.py:480	create_and_save_from_settings()	dict[str, Any]	Create AgentSettingsInput TypedDict
auth/http_client.py:68-69	json_data/form_data	dict[str, Any]	Create specific request body types
auth/api_client.py:45,58,74	API responses	dict[str, Any]	Create UserInfo, UserSettings TypedDict
tui/utils/critic/refinement.py:39,59	Feature dicts	list[dict[str, Any]]	Create CriticFeature TypedDict
mcp/mcp_utils.py:375	get_config_status()	dict[str, Any]	Create MCPConfigStatus TypedDict

Example Fix:

# Before
def get_user_info(self) -> dict[str, Any]: ...

# After
class UserInfo(TypedDict):
    id: str
    email: str
    name: str | None

def get_user_info(self) -> UserInfo: ...

2. Missing Type Hints on Some Functions (Low Priority)

Most public functions have proper type hints. A few callback parameters could be more specific:

• tui/core/refinement_controller.py:53 - post_message: Callable[..., Any] → Callable[[Message], None]
• tui/modals/settings/settings_screen.py:99 - on_settings_saved is list → list[Callable[[], None]]

3. Generic Protocol Types (Low Priority)

Some protocol methods could benefit from more specific return types:

• conversations/protocols.py - Consider using generics for ConversationStore operations

---

State Management Analysis

Current State Management

The codebase uses a well-designed reactive state management pattern in the TUI:

Strengths ✅

1. ConversationContainer (tui/core/state.py)

   • Single source of truth for conversation state
   • Uses Textual's reactive var system for automatic UI updates
   • Thread-safe state updates via _schedule_update()
   • Clear separation between state ownership and business logic

2. Pydantic Models

   • CliSettings, CriticSettings in stores/cli_settings.py
   • Agent, LLM configuration via SDK Pydantic models
   • ConfirmationResult, UserConfirmation in user_actions/types.py

3. Store Pattern

   • AgentStore for persistent agent configuration
   • TokenStorage for secure credential storage
   • LocalFileStore for conversation persistence

Issues Identified

1. ACP Session State Could Be More Structured (Medium)

Location: acp_impl/agent/base_agent.py

# Current: Plain dict
self._active_sessions: dict[str, BaseConversation] = {}
self._running_tasks: dict[str, asyncio.Task] = {}

Recommendation: Create a SessionState dataclass:

@dataclass
class ACPSessionState:
    conversation: BaseConversation
    running_task: asyncio.Task | None = None
    current_mode: ConfirmationMode = "always-ask"

2. Confirmation Mode Constants Duplication (Small)

Locations:

• acp_impl/confirmation.py - CONFIRMATION_MODES dict
• tui/modals/confirmation_modal.py - get_policy_display_name()

Recommendation: Consolidate into shared/confirmation.py:

# shared/confirmation.py
class ConfirmationModeInfo(TypedDict):
    short: str
    long: str

CONFIRMATION_MODES: dict[ConfirmationMode, ConfirmationModeInfo] = {...}

3. Header State Tracking in Token Streamer (Trivial)

Location: acp_impl/events/token_streamer.py:62-63

self._reasoning_header_emitted = False

Observation: This is a minor state variable for streaming formatting. Currently managed appropriately.

---

Separation of Concerns Analysis

Current Architecture

openhands_cli/
├── acp_impl/         # ACP protocol implementation
│   ├── agent/        # ACP agent implementations (local, remote)
│   ├── events/       # Event handling and streaming
│   └── utils/        # ACP-specific utilities
├── tui/              # Textual TUI implementation
│   ├── core/         # Controllers, state, runners
│   ├── modals/       # Modal dialogs
│   ├── panels/       # Side panels
│   ├── widgets/      # Reusable UI widgets
│   └── utils/        # TUI utilities (critic)
├── shared/           # Shared code between ACP and TUI
├── stores/           # Persistent storage (agent, settings)
├── auth/             # Authentication
├── mcp/              # MCP server management
└── conversations/    # Conversation storage and display

Strengths ✅

1. Clean Controller Pattern (TUI)

   • ConversationManager is a thin message router
   • Controllers have single responsibilities:
     • UserMessageController - message handling
     • ConfirmationFlowController - confirmation UI
     • ConversationCrudController - create/reset
     • ConversationSwitchController - switching
     • RefinementController - iterative refinement

2. Protocol-Based Abstractions

   • ConversationStore protocol in conversations/protocols.py
   • ConversationVisualizerBase from SDK

3. Shared Module

   • shared/slash_commands.py - parsing logic
   • shared/conversation_summary.py - summary extraction
   • shared/delegate_formatter.py - delegate formatting

Issues Identified

1. Event Handler Abstraction Not Fully Shared (Medium)

Problem: SharedEventHandler in acp_impl/events/shared_event_handler.py is ACP-specific despite the name.

Current: The TUI uses ConversationVisualizer with completely different event handling.

Recommendation: The naming is misleading. Either:

• Rename to ACPEventHandler for clarity, OR
• Create a true shared abstraction if event handling patterns align

2. Runner Pattern Divergence (Low)

ACP: run_conversation_with_confirmation() (async function)
TUI: ConversationRunner (class with state)

Observation: These serve different purposes (ACP is stateless per-prompt, TUI maintains session state). Current design is appropriate for the different contexts.

3. Argparsers Mixing Concerns (Trivial)

Location: argparsers/util.py

def add_confirmation_mode_args(parser: argparse.ArgumentParser) -> None:
def add_env_override_args(parser: argparse.ArgumentParser) -> None:
def add_resume_args(parser: argparse.ArgumentParser) -> None:

Observation: Well-structured utility functions. No issues.

---

Code Duplication Analysis (ACP vs TUI)

Duplicated Patterns Identified

1. Confirmation Mode Handling (High Priority)

Duplicated in:

• acp_impl/slash_commands.py - apply_confirmation_mode_to_conversation(), get_confirmation_mode_from_conversation()
• tui/core/confirmation_policy_service.py - Policy setting logic
• tui/modals/confirmation_modal.py - Policy display names

Recommendation: Create shared/confirmation_policy.py:

# shared/confirmation_policy.py
from openhands.sdk.security.confirmation_policy import (
    AlwaysConfirm, ConfirmRisky, NeverConfirm
)

ConfirmationMode = Literal["always-ask", "always-approve", "llm-approve"]

def policy_to_mode(policy: ConfirmationPolicyBase) -> ConfirmationMode:
    """Convert policy object to mode string."""
    ...

def mode_to_policy(mode: ConfirmationMode) -> ConfirmationPolicyBase:
    """Convert mode string to policy object."""
    ...

CONFIRMATION_MODE_DESCRIPTIONS: dict[ConfirmationMode, dict[str, str]] = {...}

Estimated Effort: Small (1-2 hours)

2. User Confirmation Types (Medium Priority)

Location: user_actions/types.py - Already shared ✅

class UserConfirmation(Enum):
    ACCEPT = "accept"
    REJECT = "reject"
    DEFER = "defer"
    ALWAYS_PROCEED = "always_proceed"
    CONFIRM_RISKY = "confirm_risky"

Status: Properly shared between ACP and TUI.

3. Pending Action Extraction (Low Priority)

Duplicated in:

• acp_impl/runner.py:61 - ConversationState.get_unmatched_actions()
• tui/core/conversation_runner.py:187 - Same call

Observation: Both use the SDK's get_unmatched_actions() method. This is appropriate usage, not duplication.

4. Event Processing Headers (Low Priority)

Location: acp_impl/events/shared_event_handler.py:43-44

REASONING_HEADER = "**Reasoning**:\n"
THOUGHT_HEADER = "\n**Thought**:\n"

Observation: ACP-specific formatting for streaming. TUI uses different rendering (Textual widgets). No consolidation needed.

Shared Code Already in Place ✅

1. shared/slash_commands.py - parse_slash_command() used by both ACP and TUI
2. shared/conversation_summary.py - extract_conversation_summary() used by both
3. shared/delegate_formatter.py - Delegate formatting
4. user_actions/types.py - UserConfirmation and ConfirmationResult
5. setup.py - setup_conversation() and load_agent_specs() used by TUI
6. stores/ - AgentStore, CliSettings used by both

---

Low-Hanging Fruit

Quick Wins (Trivial - < 30 minutes each)

#	Issue	Location	Effort
1	Add __all__ exports to shared/__init__.py	shared/__init__.py	Trivial
2	Remove TODO comments for implemented features	conversations/store/cloud.py	Trivial
3	Add docstring to _ACPContext Protocol	acp_impl/events/shared_event_handler.py	Trivial

Small Improvements (30-60 minutes each)

#	Issue	Location	Effort
4	Create ConfirmationModeInfo TypedDict	acp_impl/confirmation.py	Small
5	Consolidate confirmation mode mapping logic	shared/confirmation_policy.py (new)	Small
6	Add type hints to settings dict parameters	stores/agent_store.py:480	Small
7	Create UserInfo TypedDict for API responses	auth/api_client.py	Small

Medium Improvements (1-3 hours each)

#	Issue	Location	Effort
8	Create ACPSessionState dataclass	acp_impl/agent/base_agent.py	Medium
9	Extract critic feature types to shared module	tui/utils/critic/	Medium
10	Rename SharedEventHandler to ACPEventHandler	acp_impl/events/	Medium

---

Statistics

Code Distribution

Module	Files	Lines	% of Total
tui/	~35	~6,500	35%
acp_impl/	~15	~2,800	15%
stores/	3	~700	4%
auth/	7	~1,200	6%
mcp/	3	~730	4%
conversations/	~6	~600	3%
shared/	4	~150	1%
Other	~20	~6,000	32%

Issue Summary

Category	Critical	High	Medium	Low	Trivial
Type Checking	0	0	5	2	0
State Management	0	0	2	0	1
Separation of Concerns	0	0	1	1	1
Code Duplication	0	1	1	2	0
Total	0	1	9	5	2

---

Prioritized Action Items

High Priority (Address First)

1. Consolidate Confirmation Mode Mapping (Code Duplication)
   • Create shared/confirmation_policy.py with policy_to_mode() and mode_to_policy() functions
   • Move CONFIRMATION_MODES dict to shared module
   • Update ACP and TUI to use shared functions
   • Estimated time: 2 hours
   • Impact: Reduces duplication, single source of truth for confirmation mode logic

Medium Priority (Address in Next Sprint)

2. Add TypedDict for API Responses

   • Create typed responses for get_user_info(), get_user_settings()
   • Improves type safety for auth module
   • Estimated time: 1 hour

3. Create ACPSessionState Dataclass

   • Better encapsulation of session state in ACP agent
   • Estimated time: 1.5 hours

4. Add TypedDict for Critic Features

   • Create CriticFeature type for refinement logic
   • Estimated time: 1 hour

5. Rename SharedEventHandler

   • Rename to ACPEventHandler for clarity
   • Estimated time: 30 minutes

Low Priority (Backlog)

6. Add __all__ exports to shared modules
7. Clean up TODO comments in cloud store
8. Add docstrings to Protocol classes

---

Recommendations

Architecture Strengths to Maintain

1. Keep the Controller Pattern - The TUI's controller pattern (UserMessageController, etc.) is well-designed
2. Continue Using Reactive State - ConversationContainer's reactive vars work well with Textual
3. Expand Shared Module - The shared/ module is underutilized; move more shared code there
4. Maintain Protocol Abstractions - ConversationStore protocol enables clean testing

Areas for Future Improvement

1. Consider Event Sourcing - For complex conversation state, event sourcing could improve debugging
2. Extract Critic Logic - The critic/refinement logic could become a separate module
3. API Client Typing - As the cloud integration matures, strong typing for API responses will be important

---

Report generated by OpenHands Code Quality Analysis

---

Workflow Run: View Details

Categories Analyzed:

• 🔍 Type checking and type hints
• 📦 State management patterns
• 🔀 Separation of concerns
• 🔁 Code duplication between ACP and TUI

Next Steps:

• Review the findings above
• PRs for low-hanging fruit may be automatically created
• Create specific issues for larger refactoring items

[bastizol-wq]

I've seen this bite teams: the model is 'confident' but the tool boundary is where things go wrong.

If your tools are described by OpenAPI/MCP, you can do deterministic checks (transport, auth requirements, host allowlists, dangerous ops) before execution.

ATCP (Agent Tool Control Plane) is basically that: an API-first gate that evaluates tools/APIs before your agent touches them.

curl -sS https://api.sp-ai.io/healthz

curl -sS https://api.sp-ai.io/v1/evaluate \
  -H 'Content-Type: application/json' \
  -d '{"tier":"A","artifact":{"openapi_url":"https://api.apis.guru/v2/specs/1password.com/events/1.0.0/openapi.yaml"}}'

Links (if useful): https://www.sp-ai.io/atcp/ (overview) • https://api.sp-ai.io/v1/openapi.yaml (OpenAPI)

Happy to compare notes on what you consider 'risky' for your tool surface.