Deniz/debug

[dzorlu]

Summary

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation
• New environment
• Refactoring

Alignment Checklist

Before submitting, verify:

• I have read .claude/docs/PRINCIPLES.md and this PR aligns with our principles
• I have checked .claude/docs/INVARIANTS.md and no invariants are violated
• I have run /pre-submit-pr (or bash .claude/hooks/lint.sh and tests) and addressed all issues

RFC Status

• Not required (bug fix, docs, minor refactoring)
• RFC exists: #___
• RFC needed (will create before merge)

Test Plan

Claude Code Review

[greptile-apps]

Greptile Summary

This PR introduces the fleet_env integration — a client-side adapter that connects OpenEnv to Fleet-hosted remote environments via two separate planes: HTTP orchestration (FleetEnvClient) and MCP agent actions (FleetMCPTools). The design correctly follows the RFC 001 split-plane principle and RFC 003's tool-call action model. It also adds FleetTaskEnv, a Gymnasium-compatible wrapper with full telemetry (Logfire), modality-aware tool filtering, and context management utilities.

However, this branch is not merge-ready. The PR title "Deniz/debug" and an explicit "THROWAWAY DEBUG" comment in the code confirm it was pushed for debugging purposes:

• Critical bug — from_fleet_async is an infinite loop: The entire body of AsyncFleet.make() was replaced with a _tick_forever() coroutine that prints a log every 10 seconds and never returns. Since FleetTaskEnv._ensure_provisioned() calls this method, every reset() call hangs indefinitely. Additionally, async_env is initialized to None and never assigned, so even after removing the debug block the code that follows would raise AttributeError.
• Three tests have missing required arguments: from_fleet() requires data_key, data_version, and image_type but the three sync tests omit them, causing TypeError before any assertion runs.
• src/pyproject.toml is missing logfire: telemetry.py imports logfire unconditionally at module level, but the src/ package manifest's fleet extras don't include it, causing an ImportError at import time for consumers of that manifest.

Alignment flags:

• ALIGNMENT FLAG: FleetTaskEnv exposes reset() and step() sync wrappers that call asyncio.run() internally. If a caller already has a running event loop (common in async training frameworks like SkyRL), these will raise RuntimeError: This event loop is already running. The async variants reset_async() / step_async() exist and should be the primary interface.

  • Principle at stake: Predictable, composable async API (RFC 001 / RFC 002 abstractions).
  • Suggested reviewer: @darktex

• ALIGNMENT FLAG: The PR explicitly notes it does not implement the container/provider abstraction from RFC 002 (ContainerProvider, local Docker lifecycle). It is a Fleet-specific client with no pluggable backend. This may be intentional as a stepping stone, but the deviation from RFC 002's architecture should be acknowledged with either a follow-up RFC or a note in rfcs/002-env-spec.md.

  • Principle at stake: RFC 002 container/provider abstraction.
  • Suggested reviewer: @darktex

Confidence Score: 0/5

• Not safe to merge — the async provisioning path is permanently broken by intentional throwaway debug code that the author acknowledged should be removed.
• The explicit "THROWAWAY DEBUG" infinite loop in from_fleet_async makes the entire FleetTaskEnv reset path non-functional. Combined with failing tests (missing required args) and a missing logfire dependency, this branch cannot be used in production in its current state and should not be merged until the debug code is removed and the follow-on bugs are fixed.
• src/envs/fleet_env/client.py (broken from_fleet_async), tests/envs/test_fleet_env.py (failing test signatures), src/pyproject.toml (missing logfire dependency)

Important Files Changed

Filename	Overview
src/envs/fleet_env/client.py	Contains a critical throwaway debug infinite loop in from_fleet_async that completely disables environment provisioning; async_env is also never assigned, so even after removing the debug block the function would crash.
tests/envs/test_fleet_env.py	Three sync tests call FleetEnvClient.from_fleet without the required data_key, data_version, and image_type positional arguments — all three will fail with TypeError before executing any assertions.
src/pyproject.toml	Fleet extras are missing logfire, which is imported unconditionally at module level in telemetry.py; the root pyproject.toml correctly includes it but this manifest does not, causing an ImportError for consumers of the src/ package.
src/envs/fleet_env/task_env.py	Well-structured Gymnasium-compatible wrapper with solid telemetry, modality-based tool filtering, and graceful degradation for reset/screenshot failures; relies on the broken from_fleet_async so it is non-functional until the debug code is removed.
src/envs/fleet_env/mcp_tools.py	Clean retry/backoff logic and tool-owner routing; the unconditional 8-second initial_wait on every list_tools call (not just the first) will introduce significant latency in multi-episode workloads.
src/envs/fleet_env/telemetry.py	Thin Logfire wrapper with clean context-variable design; logfire is imported unconditionally at module level so the missing dependency in src/pyproject.toml will break consumers.

Sequence Diagram

sequenceDiagram
    participant SkyRL as SkyRL Generator
    participant TaskEnv as FleetTaskEnv
    participant Client as FleetEnvClient (HTTP)
    participant Tools as FleetMCPTools (MCP)
    participant Fleet as Fleet Runtime

    SkyRL->>TaskEnv: reset()
    TaskEnv->>TaskEnv: fleet_rollout_started (telemetry)
    TaskEnv->>Client: from_fleet_async() [BROKEN: infinite debug loop]
    Note over Client: ❌ _tick_forever() runs forever<br/>AsyncFleet.make() never called<br/>async_env stays None

    alt Provisioning succeeds (after fix)
        Client->>Fleet: AsyncFleet.make(env_key, image_type, ...)
        Fleet-->>Client: env handle + URLs
        Client-->>TaskEnv: (orch, tools)
        TaskEnv->>Client: orch.reset() [HTTP, short timeout]
        TaskEnv->>Tools: tools.list_tools() [8s wait + retries]
        Tools->>Fleet: MCP list_tools
        Fleet-->>Tools: tool list
        Tools-->>TaskEnv: ListToolsAction
        TaskEnv->>TaskEnv: filter tools by modality
        TaskEnv-->>SkyRL: obs {prompt, tools, screenshot?}
    end

    loop Agent turn
        SkyRL->>TaskEnv: step_async(action)
        TaskEnv->>Tools: call_tool(name, args)
        Tools->>Fleet: MCP call_tool
        Fleet-->>Tools: result
        Tools-->>TaskEnv: result
        TaskEnv-->>SkyRL: (obs, reward, done, info)
    end

    SkyRL->>TaskEnv: close()
    TaskEnv->>Client: orch.close()
    Client->>Fleet: terminate instance
    TaskEnv->>TaskEnv: fleet_rollout_completed (telemetry)

Loading

_{Last reviewed commit: 20bb13e}

[greptile-apps]

logfire missing from src/pyproject.toml fleet extras — runtime import will fail

telemetry.py unconditionally imports logfire at module level:

import logfire

The root pyproject.toml correctly includes logfire>=3.0.0 in the fleet extras, but src/pyproject.toml does not — it includes openai>=2.11.0 instead (which is only imported under TYPE_CHECKING so it's optional at runtime). Any consumer installing via the src/ package manifest will get an ImportError: No module named 'logfire' the moment they import anything from envs.fleet_env.

Suggested change

	# Fleet runtime integration (optional)
	fleet = [
	"mcp>=1.0.0",
	"fleet-python>=0.2.79",
	# Fleet runtime integration (optional)
	fleet = [
	"mcp>=1.0.0",
	"fleet-python>=0.2.79",
	"logfire>=3.0.0",
	"openai>=2.11.0",
	]

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/pyproject.toml
Line: 37-41

Comment:
**`logfire` missing from `src/pyproject.toml` fleet extras — runtime import will fail**

`telemetry.py` unconditionally imports `logfire` at module level:
```python
import logfire
```

The root `pyproject.toml` correctly includes `logfire>=3.0.0` in the `fleet` extras, but `src/pyproject.toml` does not — it includes `openai>=2.11.0` instead (which is only imported under `TYPE_CHECKING` so it's optional at runtime). Any consumer installing via the `src/` package manifest will get an `ImportError: No module named 'logfire'` the moment they import anything from `envs.fleet_env`.

```suggestion
# Fleet runtime integration (optional)
fleet = [
    "mcp>=1.0.0",
    "fleet-python>=0.2.79",
    "logfire>=3.0.0",
    "openai>=2.11.0",
]
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Additional Comments (3)

src/envs/fleet_env/mcp_tools.py
initial_wait applies on every list_tools call, not just the first — 8s penalty on every invocation

The initial_wait=8.0 sleep runs unconditionally at the start of list_tools(). Since FleetTaskEnv.reset_async() calls list_tools() at the beginning of every episode, and some callers (e.g., _call_tool_single_attempt when the owner cache is cold) may also call list_tools() on the client directly, each such call incurs a full 8-second delay.

Consider guarding the sleep so it only fires once per FleetMCPTools instance lifetime:

if self.initial_wait > 0 and not self._clients:
    logger.info(f"Waiting {self.initial_wait:.0f}s for MCP services to initialize...")
    await asyncio.sleep(self.initial_wait)

Or add a _initialized: bool = False flag and flip it after the first successful list_tools attempt.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/envs/fleet_env/mcp_tools.py
Line: 1577-1589

Comment:
**`initial_wait` applies on every `list_tools` call, not just the first — 8s penalty on every invocation**

The `initial_wait=8.0` sleep runs unconditionally at the start of `list_tools()`. Since `FleetTaskEnv.reset_async()` calls `list_tools()` at the beginning of every episode, and some callers (e.g., `_call_tool_single_attempt` when the owner cache is cold) may also call `list_tools()` on the client directly, each such call incurs a full 8-second delay.

Consider guarding the sleep so it only fires once per `FleetMCPTools` instance lifetime:

```python
if self.initial_wait > 0 and not self._clients:
    logger.info(f"Waiting {self.initial_wait:.0f}s for MCP services to initialize...")
    await asyncio.sleep(self.initial_wait)
```

Or add a `_initialized: bool = False` flag and flip it after the first successful `list_tools` attempt.

How can I resolve this? If you propose a fix, please make it concise.

---

src/envs/fleet_env/client.py
Throwaway debug code left in — from_fleet_async is completely broken

The body of the retry loop was replaced with an infinite _tick_forever() coroutine; the actual AsyncFleet.make() call was never restored. The comment says explicitly: "THROWAWAY DEBUG: do NOT call AsyncFleet.make()".

Consequences:

1. _tick_forever() is an infinite while True loop — from_fleet_async never returns.
2. FleetTaskEnv._ensure_provisioned() calls from_fleet_async, so every reset() / reset_async() on a FleetTaskEnv hangs forever.
3. async_env is initialized to None and is never assigned inside the loop. After the debug code is removed, the code immediately after the for block — async_env.urls.root and await async_env.close() — would raise AttributeError: 'NoneType' object has no attribute 'urls'.

The debug block must be removed and replaced with the actual async_env = await async_fleet.make(...) call before this branch can be merged.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/envs/fleet_env/client.py
Line: 830-844

Comment:
**Throwaway debug code left in — `from_fleet_async` is completely broken**

The body of the retry loop was replaced with an infinite `_tick_forever()` coroutine; the actual `AsyncFleet.make()` call was never restored. The comment says explicitly: *"THROWAWAY DEBUG: do NOT call AsyncFleet.make()"*.

Consequences:
1. `_tick_forever()` is an infinite `while True` loop — `from_fleet_async` **never returns**.
2. `FleetTaskEnv._ensure_provisioned()` calls `from_fleet_async`, so every `reset()` / `reset_async()` on a `FleetTaskEnv` hangs forever.
3. `async_env` is initialized to `None` and is never assigned inside the loop. After the debug code is removed, the code immediately after the `for` block — `async_env.urls.root` and `await async_env.close()` — would raise `AttributeError: 'NoneType' object has no attribute 'urls'`.

The debug block must be removed and replaced with the actual `async_env = await async_fleet.make(...)` call before this branch can be merged.

How can I resolve this? If you propose a fix, please make it concise.

---

tests/envs/test_fleet_env.py
Missing required positional arguments — tests will raise TypeError

FleetEnvClient.from_fleet declares three non-optional parameters without defaults: data_key: str, data_version: str, and image_type: str. The three sync tests in this block all call from_fleet with only two arguments, so Python will raise TypeError: from_fleet() missing 3 required positional arguments before any assertion runs.

To fix, either add default values to those parameters in the production code, or pass the missing arguments explicitly in each test call.

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/envs/test_fleet_env.py
Line: 2731-2762

Comment:
**Missing required positional arguments — tests will raise `TypeError`**

`FleetEnvClient.from_fleet` declares three non-optional parameters without defaults: `data_key: str`, `data_version: str`, and `image_type: str`. The three sync tests in this block all call `from_fleet` with only two arguments, so Python will raise `TypeError: from_fleet() missing 3 required positional arguments` before any assertion runs.

To fix, either add default values to those parameters in the production code, or pass the missing arguments explicitly in each test call.

How can I resolve this? If you propose a fix, please make it concise.