test(backend): cover agent orchestration + sandbox lifecycle (#115)

[lucastononro]

Summary

• Adds backend/tests/test_sandbox.py — unit tests for run_code's success/timeout/exception paths, with modal.Sandbox faked end-to-end (create/stdout/stderr/wait).
• Adds backend/tests/test_runner.py — unit tests for run_agent's reachable orchestration state machine (spawn → run → complete/error/cancel) and its interplay with the real task registry (register_task/abort_agent/is_agent_running/cleanup_session) in services/agent/tasks.py.
• Both modules were previously near-untested despite being the two most critical paths in the agent runtime (every sandbox execution and every agent run goes through them).

Changes

• test_sandbox.py fakes modal.Sandbox (a _FakeSandbox/_FakeStream pair mimicking the real async stdout/stderr streams) and sandbox_span (a recording fake context manager), then exercises services/sandbox.py::run_code for real:
  • success: correct stdout/stderr/returncode, span attributes (sandbox.code_chars, sandbox.returncode, sandbox.elapsed_s, no error), clean record_sandbox_usage(is_error=False).
  • streaming/dispatch: real parse_stdout_line routing — a JSON metrics line goes to persist_and_publish, plain text doesn't; no stage → no dispatch at all.
  • "timeout" (Modal kills the container, returns a non-zero exit code rather than raising): run_code must not raise, but must flag record_sandbox_usage(is_error=True) and tag the span with error=True.
  • exception paths (sandbox creation itself blows up; a mid-stream stdout failure): span gets error=True, exception is re-raised, usage is never recorded.
  • a regression guard that gpu/timeout reach modal.Sandbox.create unchanged.
• test_runner.py fakes only _drive_provider (the LLM/provider boundary — its internals are already covered in depth by the existing test_drive_provider.py) and drives run_agent for real against a real in-memory SQLite DB:
  • success → {stage}_running then {stage}_done, publish_artifacts/post_stage_hook called, text returned.
  • TimeoutError → contained (no raise), agent_timeout event + timed_out terminal state, post-run hooks skipped.
  • generic Exception → agent_error event + failed state, re-raised (unlike Timeout/CancelledError).
  • CancelledError → contained, gated by the _silent_aborts set (abort_agent(..., silent=True)'s "quiet swap" path from routers/sessions.py), one-shot consumed.
  • two tests use real asyncio.Tasks (not mocks) wrapping run_agent, wired through the real register_task/abort_agent/is_agent_running to verify the task-registry bookkeeping end-to-end for both the complete and the abort cycle, plus a stale-task-swap test for the "followup message while still running" case.

Test plan

• Existing: full backend suite still green — 311 passed, 8 skipped (was 296 passed, 8 skipped before this PR; the 15 new tests account for the delta, no regressions).
• New: 7 tests in test_sandbox.py, 8 tests in test_runner.py, all passing and re-run multiple times for scheduling-order stability (asyncio interleaving is inherently order-sensitive; verified deterministic).
• ruff check / ruff format --check clean on both new files.
• Mutation example: flip is_error=(sb.returncode != 0) to is_error=False (i.e. stop tagging non-zero exits as errors) in services/sandbox.py::run_code → test_run_code_nonzero_returncode_marks_error_but_does_not_raise fails immediately (usage_kwargs["is_error"] assertion). Similarly, removing the raise in run_agent's generic except Exception branch → test_run_agent_exception_marks_failed_and_reraises's pytest.raises(RuntimeError, ...) fails.

Closes #115

🤖 Generated with Claude Code

Greptile Summary

This PR adds 15 new unit tests (7 in test_sandbox.py, 8 in test_runner.py) covering the two previously near-untested critical paths in the agent runtime: Modal sandbox lifecycle and the agent orchestration state machine.

• test_sandbox.py introduces _FakeSandbox/_FakeStream/_FakeSpanCtx fakes and exercises run_code end-to-end across success, non-zero returncode, streaming dispatch, sandbox creation failure, and mid-stream stdout failure paths — asserting on span attributes, usage recording flags, and re-raise semantics.
• test_runner.py fakes only _drive_provider and drives run_agent against a real in-memory SQLite DB; includes real asyncio.Task registry tests (spawn/complete, spawn/abort, stale-task swap) and addresses both concerns raised in the prior review round — an autouse fixture now explicitly clears module-level registry state, and the stale-task cancellation is awaited with asyncio.wait_for rather than relying on event-loop scheduling order.
• The PR is test-only; no production code is changed. The test count increases from 296 to 311 passed with no regressions.

Confidence Score: 5/5

Test-only PR; no production code is modified. All new test logic is internally consistent with the source under test.

Both new test files exercise real code paths with well-constructed fakes. The _FakeStream correctly uses asyncio.sleep(0) to reflect real Modal stream interleaving so the stderr drain task gets scheduled. The _drive_provider patch works on the correct module namespace. The autouse _clear_task_registry fixture and the explicit asyncio.wait_for on stale-task cancellation both address the issues flagged in the prior review. No production code changes, and the test assertions map directly to the behaviour enforced by the source.

No files require special attention.

Important Files Changed

Filename	Overview
backend/tests/test_sandbox.py	Adds 7 unit tests for run_code's sandbox lifecycle; fakes modal.Sandbox.create, stdout/stderr streams, and sandbox_span. Covers success, streaming/dispatch, non-zero returncode, create failure, and mid-stream stdout failure paths with correct assertions on span attributes, usage recording, and re-raise semantics.
backend/tests/test_runner.py	Adds 8 unit tests for run_agent's orchestration state machine and task registry; only fakes _drive_provider and post-run hooks, driving everything else (DB, event publishing, task registry) for real. Includes autouse registry-cleanup and real-asyncio-Task tests for spawn/complete/abort/swap cycles.

_{Reviews (2): Last reviewed commit: "fix(review): address Greptile findings o..." | Re-trigger Greptile}

[lucastononro]

Greptile review findings addressed (befc75f)

Fixed (2/2):

1. [P2] test_runner.py:65 — no fixture clearing module-level task-registry state. Valid: _running_tasks, _silent_aborts, and _session_task_locks only empty via cleanup_session on the happy path, so a mid-flight test failure could leak entries across tests. Added an autouse _clear_task_registry fixture that clears all three after each test.

2. [P2] test_runner.py:366 — stale-task cancellation asserted without an explicit await. Valid: the assertion relied on the event loop having delivered the stale task's CancelledError during one of new_task's suspension points. Now explicitly await asyncio.wait_for(stale_task, timeout=1.0) (expecting CancelledError) before asserting stale_cancelled.

Dismissed: none.

Validation: full backend suite pytest tests/ — 311 passed, 8 skipped (e2e-gated); ruff check and ruff format --check clean on both PR files.

[lucastononro]

Merged into integration branch staging-v0.0.5 (see the STAGING-v0.0.5.md ledger on that branch for merge order, Greptile follow-ups and test results). These changes will land on main via the staging merge — closing to clear the queue.