Repository navigation
fix(python): address review nits for loop runner, step timeout, and trailing newline - #89
Merged
Tomer Glottmann (tomergee) merged 1 commit intoOct 14, 2026
Conversation
Sunny (sunnylovestiramisu)
requested a review
from Tomer Glottmann (tomergee)
October 14, 2026 00:54
…railing newline - Guard loop.close() with thread liveness check in LoopRunner.stop - Wire FleetConfig.step_timeout_s to default_timeout_s on SandboxHandle.exec - Remove extra trailing blank line in backend/__init__.py - Add unit test verifying fleet step_timeout_s propagation
Sunny (sunnylovestiramisu)
force-pushed
the
nit-fix
branch
from
October 14, 2026 17:00
dc15295 to
80ecc61
Compare
Tomer Glottmann (tomergee)
approved these changes
Oct 14, 2026
Tomer Glottmann (tomergee)
left a comment
Collaborator
There was a problem hiding this comment.
Review at 80ecc61. Approve.
All three nits from the #80 second pass are done: loop.close() runs only when the thread has exited, step_timeout_s reaches SandboxHandle.default_timeout_s and is the default for exec and exec_async, the blank line is gone. 119 tests pass, ruff is clean, the branch merges clean with main at a82a87f and with #88.
Blocking: none. Before merge: none.
Nit
handle.py:201-209in #88 buildsSandboxHandlewithoutdefault_timeout_s, thus after both PRs land the async fleet's handles keep the 120 s default and ignorestep_timeout_s. Whichever PR merges second passesdefault_timeout_s=self.config.step_timeout_sthere too, with the same one-line test.
Tomer Glottmann (tomergee)
merged commit Oct 14, 2026
df24cef
into
agent-substrate:main
9 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Address reviewer nitpicks following PR #80:
Guard event loop closure in
LoopRunner.stop():loop.close()previously ran afterself._thread.join(timeout=2.0)regardless of whether the background thread had finished. Callingclose()on a still-running event loop raisesRuntimeError.if not self._thread.is_alive() and not self._loop.is_closed():before closing.Wire
FleetConfig.step_timeout_stoSandboxHandle.exec:step_timeout_swas previously validated inFleetConfigbut never referenced.default_timeout_s: float = 120.0toSandboxHandle, populated fromFleetConfig.step_timeout_sduringSandboxFleet._acquire_one().SandboxHandle.execandSandboxHandle.exec_asyncto defaulttimeout_s: Optional[float] = None, falling back toself.default_timeout_swhen omitted.Code style / formatting:
clients/python/src/ate_env/backend/__init__.py.Testing
test_fleet_step_timeout_applies_to_handleintest_exec_semantics.pyto verify thatstep_timeout_sset onFleetConfigpropagates correctly toSandboxHandle.default_timeout_s.pytest clients/python/tests/ # 119 passed, 29 skipped