Repository navigation
fix: enforce cooperative invocation deadlines - #783
zhongkechen wants to merge 4 commits into
Conversation
Use an invocation-local monotonic budget for SDK admission and waits. Reject newly registered descendants of completed contexts before user code or checkpoint dispatch. Preserve acknowledged terminal outcomes without issuing metadata reads after the cutoff. Keep running threads confined and document the unresolved non-cooperative cleanup boundary in #741.
| InvalidStateError: If the check result is in an invalid state | ||
| May raise operation-specific errors from check_result_status() or execute() | ||
| """ | ||
| self.state.check_operation_active(self.operation_identifier) |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_ygsoiugckiy5x5urrhaxumbgjl
[P1] Preserve completed-child replay during in-process resumes
A parallel/map branch can complete a child context with a large ReplayChildren result, then suspend on a wait while another branch remains running. When the coordinator resumes it within the same invocation, that child's terminal checkpoint has already marked its inner operations in _parent_done. This new check rejects those operations before their saved results can be read. _branch_worker reports ORPHANED, causing the coordinator to exit and checkpoint a successful parent with missing branch results. Allow saved checkpoint reads and legitimate ReplayChildren reconstruction through the orphan guard while retaining checks before new work and checkpoints. Add an integration test covering this same-invocation resume.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
| operation_update is not None | ||
| and operation_update.operation_type is OperationType.EXECUTION | ||
| and operation_update.action | ||
| in {OperationAction.SUCCEED, OperationAction.FAIL} | ||
| and self._terminal_execution_acknowledged.is_set() |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_vgbpun25a2tdxgcw3ykxpyfpeq
[P2] Reconcile terminal acknowledgments received during cleanup
If an EXECUTION SUCCEED/FAIL request remains in flight at the cutoff, this flag is false and InvocationDeadlineExceeded escapes. Cleanup still joins the batcher, so the request can then succeed and set _terminal_execution_acknowledged, but the exception path skips both check_invocation_result() calls. The wrapper and invocation-end hook therefore report RETRY even though the terminal checkpoint is known to have succeeded before return. Reconcile that request's outcome after the mandatory join, or await its specific completion once dispatched, before propagating the deadline error. Add a regression test that holds the terminal checkpoint transport across the cutoff.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
Codex AI reviewTwo findings:
Tests were not executed under the read-only review constraints. Reviewed commit |
Expired Lambda invocations could start new user work, and SDK-owned waits could keep cooperative branches pinned during invocation cleanup. This change captures one monotonic invocation budget, rejects late work, and lets those waits unwind as a retryable invocation interruption while preserving checkpointed outcomes.
Related to #741. This is a partial implementation and intentionally does not close that issue. Regression test PR #743 remains separate and unchanged.
Remaining boundary: this does not provide a hard bound on the wrapper or guarantee LMI worker reclamation. Arbitrary user code/I/O, an already-running synchronous service call (including retries), and blocked plugin hooks still have to return before their threads can be joined. No threads are detached or forcibly interrupted. Non-cooperative worker retirement needs a runtime-supported strategy and real LMI validation; #741 remains open for that work. Shared conformance requirements for that complete contract should be coordinated separately.
Current head
6aed2267also includes upstream main04daf408. The incoming change touches only the standalone MicroVM worker and its tests; LMI/core/OTel source trees remain unchanged. That worker tree is identical in both refreshed Python PRs and passed 261 tests, type checking (13 files), and formatting/lint. Current-head CI is tracked separately.Validation on the reviewed implementation:
git diff --checkpass.