revert(tbench2): disable unsafe idle-session reaper

[cursor]

Summary

Reverts #1000 until core session destruction can terminate or invalidate its live WebSocket handler. The enabled reaper currently destroys the environment/container while the handler retains session_env, so a slow client can continue against a closed environment and receive an opaque error; half-open handlers can also remain blocked.

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 focused lint 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

• PYTHONPATH=src:envs .venv/bin/python -m pytest tests/envs/test_tbench2_env.py -q — 26 passed, 1 skipped
• .venv/bin/python -m ruff format --check envs/tbench2_env/server/app.py
• .venv/bin/python -m ruff check envs/tbench2_env/server/app.py
• Import smoke: from tbench2_env.server.app import app

Claude Code Review

The one-file rollback restores the pre-#1000 configuration and is conflict-free on current main. A future core fix should track active WebSockets, refresh MCP-session activity, send a defined termination signal before environment cleanup, and prove half-open handler exit before re-enabling session_timeout for TBench2.

  

---

Note

Medium Risk
Disabling idle reaping can leave half-open WebSocket sessions occupying concurrency slots longer, but re-enabling timeout without core WebSocket teardown was judged worse for client correctness.

Overview
Rolls back TBench2’s idle-session reaper by stopping use of ConcurrencyConfig and SESSION_IDLE_TIMEOUT_S in envs/tbench2_env/server/app.py.

create_app is wired again with only max_concurrent_envs (from MAX_CONCURRENT_ENVS); docs no longer mention session idle timeout. This matches a revert of the behavior that destroyed environments/containers on idle while WebSocket handlers could still hold session_env.

^{Reviewed by Cursor Bugbot for commit 2c74c7a. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Alignment Review Report

Heads-up: this PR was squash-merged to main as 05217002 (~11:35 UTC) — its head tree is byte-identical to current origin/main. Posting for the record; the one actionable item below is a core follow-up, not a change to this PR.

Automated Checks

• Lint: PASS on the changed file — ruff format --check ✓ and ruff check ✓ for envs/tbench2_env/server/app.py. usort would reorder the standalone-import fallback block, but that is pre-existing (base 45ad8ae fails identically) and envs/** is outside both CI and the hook's usort scope, so nothing is introduced or enforced here.
• Debug code: CLEAN — no prints/breakpoints added (check-debug.sh scans src/ only; this is an envs/ change).
• Tests: 26 passed, 1 skipped (tests/envs/test_tbench2_env.py), matching the PR's test plan. Import smoke (from tbench2_env.server.app import app) succeeds in both TB2_MODE=local and TB2_MODE=docker.

Revert correctness (verified)

• create_app(..., max_concurrent_envs=max_concurrent) is valid API usage — create_app accepts max_concurrent_envs directly (http_server.py:1770) and internally builds ConcurrencyConfig(..., session_timeout=None), so the reaper becomes a no-op.
• Revert is complete: zero residual ConcurrencyConfig / session_timeout / SESSION_IDLE_TIMEOUT_S references anywhere under envs/tbench2_env/ (grep clean); imports + docstring cleaned up.
• The two style(tbench2): ... commits cancel out (blank lines removed then re-added), so the net diff is only the reaper removal.

Open RFCs Context

• RFC 005 – Agentic Harnesses (In Review), L622-623: "Delays between turns are transparent... there is no concept of 'idle time'"; its session_timeout_s is a per-turn wall-clock cap.
• RFC 002 – Env Spec (In Review), Cloud-Sandbox amendment S6 / L198: "idle timeouts can drop a live transport mid-episode, so providers choose conservative defaults for RL rollouts."
• No RFC specifies the core ConcurrencyConfig.session_timeout idle-reaper behavior itself.

Tier 1: Fixes Required

• None. (Optional / non-blocking / out-of-CI-scope: the standalone-import fallback block isn't usort-ordered — pre-existing; the openenv third-party import should precede the models/server first-party imports if you ever want envs/ usort-clean.)

Tier 2: Alignment Discussion

Principle Conflicts

None. The revert trades the reaper's slot-reclamation ("Production-readiness") for client correctness, which is the right call given the failure mode below. No invariant is touched (Gym API signatures, agent isolation, client-server separation, rewards-in-env, credential handling all unaffected).

RFC Conflicts

ALIGNMENT FLAG: The "unsafe" behavior lives in core, not tbench2 — the reaper destroys live WS-owned sessions

• Principle/RFC at stake: RFC 005 L622-623 ("no concept of idle time / delays between turns are transparent"); RFC 002 S6 / L198 ("idle timeouts can drop a live transport mid-episode"); PRINCIPLES "Production-readiness".
• The concern: http_server.py::_reap_idle_sessions only exempts sessions in _session_websocket_attachments (L530). A default /ws connection takes the owns_session=True branch (L1558-1559) and is never added to that set, so once idle past session_timeout the reaper calls _destroy_session (L547) and tears down the env/container while the handler still holds session_env (captured L1558) blocked at receive_text() (L1578) → a slow client resumes against a dead env and gets an opaque error; half-open handlers can stay blocked. That is exactly the PR's rationale. tbench2 was the sole opt-in (#1000); this revert removes its exposure, but the core bug stays latent for any future session_timeout user. Per the PR's own future-work: re-enable only after core tracks active WebSockets, refreshes activity on /ws traffic, and invalidates/terminates the handler before env cleanup.
• Minor (naming): core ConcurrencyConfig.session_timeout (idle-reap) vs RFC 005 HarnessConfig.session_timeout_s (per-turn wall-clock) — same-ish name, different meaning; worth reconciling when the core fix lands.
• Suggested reviewer: git blame swappy (reaper author) + Sachin (WS-session/attachment author) + @Darktex (session-lifecycle & dual-API invariants); cc @Shi-Dong (#1000 author) and @burtenshaw (env owner / revert co-author).

Summary

• 0 mechanical issues to fix — revert is clean, complete, and tested; both import modes construct successfully.
• 1 alignment point for human review — the underlying reaper bug is in core http_server.py and remains for future opt-ins; track a proper core fix before re-enabling session_timeout anywhere.
• 1 RFC alignment note (positive) — this revert is consistent with RFC 005's "no idle-time" model and RFC 002's idle-timeout warning.

  

_{Sent by Cursor Automation: Pre-review}

[cursor]

Revert is correct. create_app accepts max_concurrent_envs directly (http_server.py:1770) and defaults session_timeout=None, so the idle reaper becomes a no-op — verified the app constructs in both TB2_MODE=local and docker, and tests/envs/test_tbench2_env.py is 26 passed / 1 skipped.

The root cause is in core http_server.py: a default /ws session takes the owns_session=True path (L1558-1559) and is never added to _session_websocket_attachments, so _reap_idle_sessions (L530/L547) can destroy it mid-flight while this handler still holds session_env. Please keep session_timeout off for tbench2 until core tracks the live WebSocket and invalidates the handler before teardown.