Repository navigation
refactor(extras): drop the resume mode fallback - #830
Conversation
Only builds from git main before #811 wrote a session launch checkpoint without resumeBeforeDelivery. The package was never published. So a missing field now reads as False, and replay ignores the current config.
The session test now covers a missing, "true", and 1 resume mode. The comment and docstring say only True is a resume mode, as the review asked.
## Summary A session launch checkpoint without `resumeBeforeDelivery` now reads as `false`. The fallback to the resume mode of the current config is removed. ## Why 1. #1042 records the resume mode in every session launch checkpoint. 2. For a checkpoint without the field, the session fell back to the mode that the current config gives. 3. Only builds from git `main` before #1042 wrote such checkpoints. 4. `@aws/durable-execution-sdk-js-extras` has never been published to npm. `npm view` returns E404. 5. So no released build can replay such a checkpoint, and the fallback can go. This is item D4 of a review of the MicroVM safeguards. The Python SDK makes the same change in aws/aws-durable-execution-sdk-python#830. ## Changes - `session.ts` reads `launched.resumeBeforeDelivery === true`. - The TSDoc of `LaunchResult.resumeBeforeDelivery` says that a session launch always records it, and a `microvm()` launch does not. So the field stays optional. - The checkpoint JSON is unchanged. ## Testing - The 5 fallback tests are replaced by one 2-case test. A checkpoint with no field, or with `"true"` as a string, makes no state check, even when the current config can suspend. - The #1042 tests for recording and replaying the field are unchanged. - Extras `npm test`: 331 tests pass. `npm run lint` and `npm run format:check` pass. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
| if isinstance(resume_before_delivery, bool) | ||
| else None | ||
| ), | ||
| resume_before_delivery=data.get("resumeBeforeDelivery"), |
There was a problem hiding this comment.
LaunchResult.resume_before_delivery is declared as bool | None. The removed isinstance check enforced that contract. Without it, a checkpointed "true" or 1 becomes a str or int in the dataclass. to_dict() then writes that value back under resumeBeforeDelivery. run_session already uses is True, so retaining the normalization does not restore the fallback. Can from_dict() keep the boolean-only read while run_session keeps the strict is True check?
Two things I checked support this:
LaunchResult.from_dict()retains"true"as astr, despite thebool | Noneannotation.- Mypy cannot detect the mismatch because
dict[str, Any].get()returnsAny.
resume_before_delivery = data.get("resumeBeforeDelivery")
return cls(
...
resume_before_delivery=(
resume_before_delivery
if isinstance(resume_before_delivery, bool)
else None
),
)The deleted test_launch_result_reads_only_a_bool_as_a_resume_mode can continue covering this invariant.
There was a problem hiding this comment.
Done in c18f693. from_dict() keeps only a bool again and reads anything else as None. run_session keeps the is True check. The bool-only test is restored.
LaunchResult.from_dict reads any non-bool resume mode as None again. This addresses the review, and the restored test covers the case.
Codex AI reviewNo actionable findings in the PR changes. Review was limited to static inspection; tests were not executed, so runtime behavior remains unverified. Reviewed commit |
Summary
A session launch checkpoint without
resumeBeforeDeliverynow reads asFalse. The fallback to the resume mode of the current config is removed.Why
mainbefore fix(extras): record the session resume mode #811 wrote such checkpoints.aws-durable-execution-sdk-python-extrashas never been published to PyPI. Its JSON API returns 404.This is item D4 of a review of the MicroVM safeguards. The JS SDK makes the same change in aws/aws-durable-execution-sdk-js#1080.
Changes
run_sessionpasseslaunched.resume_before_delivery is True.LaunchResult.from_dictpasses the field through. Theisinstance(..., bool)check existed only to choose the fallback, so it is removed.resume_before_deliverystaysbool | None. Amicrovmlaunch writes no field, as in JS. So the checkpoint JSON is unchanged for both operations.Testing
GetMicrovmcall, even when the current config can suspend.hatch run dev-extras:cov: 362 tests pass, 100 % coverage. Typecheck andhatch fmt --checkpass.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.