[PATCH] Deprecate HTTP in websockets

[burtenshaw]

This PR targets #239 and removes the http client.

[Darktex]

This is Claude's review for this one:

Pull Request #252 Review

Summary

This PR deprecates the HTTP client in favor of WebSocket-only communication, renaming WebSocketEnvClient to EnvClient. The intent is correct and aligns with the concurrency architecture in PR #239, but the PR is incomplete - it deletes the HTTP client without updating any of the environment clients that depend on it.

Overall Assessment: Good direction, but broken - all environment clients will crash on import.

Critical Issues

1. All Environment Clients Still Import Deleted HTTPEnvClient

Severity: CRITICAL - Every environment client will crash with ImportError

The PR deletes src/openenv/core/http_env_client.py but doesn't update any of the clients in envs/:

Environment	Broken Import
envs/echo_env/client.py:21	from openenv.core.http_env_client import HTTPEnvClient
envs/atari_env/client.py	from openenv.core.http_env_client import HTTPEnvClient
envs/browsergym_env/client.py	from openenv.core.http_env_client import HTTPEnvClient
envs/chat_env/client.py	from openenv.core.http_env_client import HTTPEnvClient
envs/coding_env/client.py	from openenv.core.http_env_client import HTTPEnvClient
envs/connect4_env/client.py	from openenv.core.http_env_client import HTTPEnvClient
envs/dipg_safety_env/client.py	from openenv.core.http_env_client import HTTPEnvClient
envs/finrl_env/client.py	(likely same)
envs/git_env/client.py	(likely same)
envs/openspiel_env/client.py	(likely same)
envs/sumo_rl_env/client.py	(likely same)
envs/textarena_env/client.py	(likely same)

Required Fix for each client:

# Before (broken)
from openenv.core.http_env_client import HTTPEnvClient

class EchoEnv(HTTPEnvClient[EchoAction, EchoObservation]):
    ...

# After (fixed)
from openenv.core.env_client import EnvClient

class EchoEnv(EnvClient[EchoAction, EchoObservation]):
    ...

---

2. StepResult Import Also Broken

Severity: CRITICAL

Some clients import StepResult from the deleted file:

# browsergym_env/client.py, dipg_safety_env/client.py
from openenv.core.http_env_client import HTTPEnvClient, StepResult

StepResult should be imported from openenv.core.client_types instead.

---

3. Docstrings Still Reference HTTP

Severity: MINOR - Cosmetic but confusing

envs/echo_env/client.py:8-12:

"""
Echo Environment HTTP Client.

This module provides the client for connecting to an Echo Environment server
over HTTP.
"""

Should be updated to reflect WebSocket-based communication.

Similarly, server app.py files have docstrings mentioning HTTPEnvClient compatibility.

---

What's Done Correctly

• ws_env_client.py → env_client.py rename is clean
• WebSocketEnvClient → EnvClient simplifies the API
• The new EnvClient class is well-documented
• HTTP URL auto-conversion to WS is preserved (convert_to_ws_url)

---

Action Items

• CRITICAL: Update all 12 environment clients to use EnvClient instead of HTTPEnvClient
• CRITICAL: Fix StepResult imports in browsergym_env and dipg_safety_env
• Update client docstrings to remove HTTP references
• Update server app.py docstrings to remove HTTPEnvClient references
• Verify CLI templates use the new EnvClient (check src/openenv/cli/templates/)
• Add a simple test that imports each client to catch this in CI

---

Suggested Workflow

Since this PR targets PR #239 (not main), I'd suggest:

1. Merge PR [FEATURE] WebSocket-based Concurrency Architecture #239 first (after fixing its issues)
2. Then update PR [PATCH] Deprecate HTTP in websockets #252 to fix all the client imports
3. Or: fold the HTTP deprecation into PR [FEATURE] WebSocket-based Concurrency Architecture #239 directly to avoid the intermediate broken state

---

Review Decision

REQUEST_CHANGES - The PR will break every environment client on import. All clients must be updated to use the new EnvClient before this can be merged.