[codex] fix openenv serve exit semantics#907
Conversation
There was a problem hiding this comment.
Alignment Review Report
Two-tier review of the openenv serve exit-semantics fix (1 functional line + 2 doc updates).
Automated Checks
- Lint: PASS (for this PR) — the only changed Python file,
src/openenv/cli/commands/serve.py, passesruff format,ruff check, andusortcleanly.lint.shdoes report failures, but every flagged file lives underenvs/(e.g.chat_env,opencode_env,repl_env, …) — pre-existing formatting debt unrelated to this diff, which touches onlysrc/+ docs. - Debug code: CLEAN — no new debug code. The
console.print(...)calls are intentional Rich CLI output, and theTODO:in the docstring is pre-existing (documents the deferred status).
Open RFCs Context
Open RFCs: 000 project-phases, 001 abstractions, 002 env-spec, 003 mcp-support, 005 agentic-harnesses (all In Review), 010 echo-env-token-world-model (Draft), 004 rubrics. None govern the openenv serve CLI or its exit semantics — every "serve"/"server" reference in the RFCs concerns MCP/FastAPI servers, not this command.
Tier 1: Fixes Required
None.
Verified empirically: running openenv serve now exits 1 (was 0) and still prints the guidance. typer.Exit(1) surfaces as SystemExit, which correctly bypasses the except Exception handler in __main__.main(), so it isn't swallowed/re-wrapped as Error: .... tests/test_cli/test_main.py passes; no CI/scripts/Makefile invoke openenv serve.
Tier 2: Alignment Discussion
Principle Conflicts
None identified. This is a CLI UX / exit-code correction; it does not touch the Gym-like API, client-server separation, reward computation, the MCP boundary, or type contracts.
RFC Conflicts
None identified.
Non-blocking observations (optional follow-ups)
- Doc consistency — this PR fixes
environment-builder.mdandcli.md, but other spots still advertiseopenenv serveas a working launcher and are now inconsistent with the exit-1 behavior (all outside this diff):README.md:272— "openenv serve- Serve an environment locally with optional auto-reload"openenv serve <env>in severalenvs/*/server/app.pydocstrings (echo, unity, snake, repl, openapp, dm_control, carla)examples/openenv_using_environments.ipynb(cd envs/openspiel_env && openenv serve)
- Regression test — since the PR's entire purpose is exit semantics, a tiny
CliRunnertest assertingresult.exit_code == 1(and that guidance prints) would lock the behavior in; there's currently no test covering the serve command.
Summary
- 0 mechanical issues to fix
- 0 alignment points for human review
- 0 RFC conflicts to discuss
Clean, well-scoped change. Exit 0 for a command that can't perform its action was a false success; exit 1 correctly signals failure to scripts/CI. Only the optional doc/test follow-ups above remain.
Sent by Cursor Automation: Pre-review
| console.print(f" [dim]$ uv run --project . server --port {port}[/dim]\n") | ||
|
|
||
| raise typer.Exit(0) | ||
| raise typer.Exit(1) |
There was a problem hiding this comment.
Confirmed correct. openenv serve now exits 1 while still printing the guidance — typer.Exit(1) propagates as SystemExit, so it bypasses the except Exception handler in __main__.main() (no Error: 1 re-wrap).
Optional: add a CliRunner test asserting result.exit_code == 1 (and that the guidance is printed) to lock in these exit semantics, since that's the whole point of this change.


Summary
openenv serveexit non-zero while it remains unimplementeduv run --project . serverpathopenenv serveas the local dev-server command in the environment-builder guideWhy
openenv servecurrently prints a warning but exits successfully, which makes scripts and users think local serving worked when it did not.Closes #613.
Validation
uv run python -m openenv.cli.__main__ servenow exits with status1