refactor(updater): simplify digest verification flows - #549
Open
magrhino wants to merge 1 commit into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughExpected-digest and digest-pin verification, refactored into helper flows, now continue checking all requirements and updates after failures. Tests confirm aggregate failure results alongside complete verification attempts. So, predictable verification becomes. ChangesDigest verification
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideRefactors digest verification in the updater to split requirement-level and update-level checks into focused helper methods while preserving existing behavior and logging, and adds regression tests to ensure all requirements/updates are processed even after failures. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
magrhino
marked this pull request as ready for review
July 29, 2026 23:33
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The new helper methods
_verify_expected_digest_imagesand_verify_digest_pin_imagesshare very similar control flow; consider extracting a generic internal helper (e.g., parametrized by expected image and digest) to reduce duplication and keep the matching/verification logic in one place. - Both
_verify_expected_digest_imagesand_verify_digest_pin_imagesnow return raw tuples (digest_result, stale_result/matched); introducing a small internal dataclass or namedtuple for these results would make the calling code more self-documenting and less error-prone than relying on positional tuple unpacking.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new helper methods `_verify_expected_digest_images` and `_verify_digest_pin_images` share very similar control flow; consider extracting a generic internal helper (e.g., parametrized by expected image and digest) to reduce duplication and keep the matching/verification logic in one place.
- Both `_verify_expected_digest_images` and `_verify_digest_pin_images` now return raw tuples (`digest_result, stale_result/matched`); introducing a small internal dataclass or namedtuple for these results would make the calling code more self-documenting and less error-prone than relying on positional tuple unpacking.
## Individual Comments
### Comment 1
<location path="src/wudup/updater_lifecycle_digest.py" line_range="237-246" />
<code_context>
+ def _verify_digest_pin_images(
</code_context>
<issue_to_address>
**suggestion:** Avoid duplicated image-verification loops by consolidating into a single helper
`_verify_digest_pin_images` duplicates most of `_verify_expected_digest_images` (iteration over `images`, `image_matches_resolved_target`, `self.digest_verifier.verify`, and early break on `digest_result.ok`), differing mainly in `allow_repo` and the digest source. Extract a single helper that takes `resolved_image`, `expected_digest`, and `allow_repo` so both paths share the verification logic and future changes only need to be made in one place.
Suggested implementation:
```python
def _verify_images(
self,
*,
resolved_image: str,
expected_digest: str,
images: Sequence[str],
allow_repo: bool,
) -> tuple[DigestCheckResult | None, bool]:
matched = False
digest_result: DigestCheckResult | None = None
for image in images:
if not image_matches_resolved_target(
image,
resolved_image,
allow_repo=allow_repo,
):
continue
matched = True
digest_result = self.digest_verifier.verify(
image=image,
resolved_image=resolved_image,
expected_digest=expected_digest,
)
if digest_result.ok:
break
return digest_result, matched
def _verify_digest_pin_images(
self,
update: DigestPinUpdate,
images: Sequence[str],
) -> tuple[DigestCheckResult | None, bool]:
return self._verify_images(
resolved_image=update.resolved_image,
expected_digest=update.expected_digest,
images=images,
allow_repo=False,
```
I can only see part of the file, so to complete the refactor you should:
1. **Update `_verify_expected_digest_images`** to use the new helper instead of its own loop. For example, replace its internal loop with something along the lines of:
```python
digest_result, matched = self._verify_images(
resolved_image=update.resolved_image,
expected_digest=update.expected_digest,
images=images,
allow_repo=True, # or False, depending on the original behavior
)
```
and return `digest_result, matched` from the function.
2. **Align the helper call signature with the existing verifier usage**:
- If `self.digest_verifier.verify` currently has a different signature (e.g. positional args instead of named, or additional parameters), update the call inside `_verify_images` to match the existing pattern.
- If `allow_repo` was previously passed to `image_matches_resolved_target` or used differently in `_verify_expected_digest_images` / `_verify_digest_pin_images`, ensure that logic is preserved when wiring through `_verify_images`.
3. **Ensure the expected digest for digest-pin paths is correctly sourced**:
- If `DigestPinUpdate` uses a different attribute than `expected_digest` for the digest to verify (e.g. `update.pin_digest` or similar), adapt the `expected_digest=...` argument in `_verify_digest_pin_images` accordingly.
Once those changes are made, both `_verify_expected_digest_images` and `_verify_digest_pin_images` will share the same verification logic via `_verify_images`, eliminating duplicated loops and centralizing future changes in one place.
</issue_to_address>
### Comment 2
<location path="tests/test_python_update_from_wud_core.py" line_range="95-104" />
<code_context>
+ def test_expected_digest_verification_checks_all_requirements(self) -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding coverage for the stale-digest path to ensure `_mark_stale_pending_digest` is still invoked and later requirements are evaluated.
This test already verifies that `_verify_expected_digests` continues evaluating requirements after an early mismatch and still returns `False`. To fully cover the behavior, add a sibling test for the `stale-digest` branch in `_verify_expected_digest_requirement` that:
- Sets `digest_verifier.verify.side_effect` so the first requirement yields a `DigestCheckResult` with `reason="stale-digest"` and `ok=False`.
- Patches `runner.lifecycle._mark_stale_pending_digest` and asserts it is called once for the first requirement.
- Asserts the second requirement is still verified and the overall return value remains `False`.
This will lock in the behavior around stale pending digests described as important in the PR.
Suggested implementation:
```python
runner.lifecycle._expected_digest_failure_reason(stack, matches),
STALE_PENDING_DIGEST_REASON,
)
def test_expected_digest_verification_marks_stale_pending_digest_and_checks_all_requirements(
self,
) -> None:
stack_dir = self.make_stack(
"app",
[
("first", "repo/first:latest", "cid-first"),
("second", "repo/second:latest", "cid-second"),
],
)
stack = ComposeStack(
index=1,
directory=stack_dir,
)
# Create a runner and two requirements mirroring the existing test setup.
runner = self._make_runner(stack) # adjust helper as used in sibling test
first_requirement = mock.Mock(name="first_requirement")
second_requirement = mock.Mock(name="second_requirement")
requirements = [first_requirement, second_requirement]
stale_result = DigestCheckResult(ok=False, reason="stale-digest")
second_result = DigestCheckResult(ok=False, reason="unexpected-digest")
with patch.object(
runner.lifecycle.digest_verifier,
"verify",
side_effect=[stale_result, second_result],
) as verify, patch.object(
runner.lifecycle,
"_mark_stale_pending_digest",
) as mark_stale:
result = runner.lifecycle._verify_expected_digests(stack, requirements)
# The stale-digest requirement should cause the pending digest to be marked as stale.
assert result is False
assert verify.call_count == 2
verify.assert_has_calls(
[
mock.call(stack, first_requirement),
mock.call(stack, second_requirement),
]
)
mark_stale.assert_called_once()
# Ensure we mark stale for the first requirement; adjust arguments to match implementation.
first_mark_call = mark_stale.call_args
assert first_mark_call is not None
def test_expected_digest_verification_checks_all_requirements(self) -> None:
```
1. This new test assumes `mock` and `patch` are available from `unittest.mock`. If they are not already imported at the top of this file, add:
- `from unittest import mock`
- `from unittest.mock import patch`
2. The helper `self._make_runner(stack)` and attribute `runner.lifecycle.digest_verifier` are inferred from the existing test patterns; align this to whatever the sibling test `test_expected_digest_verification_checks_all_requirements` uses to construct the runner and access the `digest_verifier`.
3. The call signatures for `runner.lifecycle._verify_expected_digests` and `_mark_stale_pending_digest` are inferred. If `_mark_stale_pending_digest` takes specific arguments (e.g., `(stack, requirement)` or just `(stack,)`), update the `mark_stale.assert_called_once()` / `assert_has_calls` assertions accordingly to match the real signature.
4. To keep the style consistent, you may want to mirror the exact stack/requirements setup of `test_expected_digest_verification_checks_all_requirements` (e.g., how requirements are constructed) rather than using `mock.Mock` placeholders; adjust `first_requirement` and `second_requirement` creation to match that existing test.
</issue_to_address>
### Comment 3
<location path="tests/test_python_update_from_wud_digest_pins.py" line_range="180-189" />
<code_context>
+ def test_digest_pin_verification_checks_all_updates_after_failure(self) -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Add a complementary test where the first digest-pin update fails during digest verification or image matching, not only during target resolution.
This test currently covers the case where the first update fails at target resolution. With the new split between resolution and image verification, you should also add a test where the first update resolves successfully but fails during image matching or digest verification (e.g. `DigestCheckResult` with `ok=False`, `reason="digest-mismatch"`), and the second update still resolves and verifies. For example:
- Mock `_verify_digest_pin_update_target` to succeed for both updates.
- Set `digest_verifier.verify.side_effect` so the first call fails and the second succeeds.
- Assert `digest_verifier.verify.call_count == 2` and that the overall result is `False`.
This will ensure coverage of both resolution failures and verification/matching failures under the new helper structure.
Suggested implementation:
```python
def test_digest_pin_verification_checks_all_updates_after_failure(self) -> None:
stack = ComposeStack(
index=1,
directory=self.root,
file="docker-compose.yml",
name="app",
images=("docker.io/repo/second:2.0",),
service_images=(
ServiceImage("second", "docker.io/repo/second:2.0"),
),
)
def test_digest_pin_verification_checks_all_updates_after_verification_failure(
self,
docker_manifest_resolver: DockerManifestResolver,
digest_verifier: DigestVerifier,
) -> None:
"""
Ensure that when the first digest-pin update resolves successfully but fails during
digest verification / image matching, we still attempt to verify the remaining updates.
"""
stack = ComposeStack(
index=1,
directory=self.root,
file="docker-compose.yml",
name="app",
images=(
"docker.io/repo/first:1.0",
"docker.io/repo/second:2.0",
),
service_images=(
ServiceImage("first", "docker.io/repo/first:1.0"),
ServiceImage("second", "docker.io/repo/second:2.0"),
),
)
config = load_config(self.root)
# Make target resolution succeed for both digest-pin updates.
# The helper under test is expected to call `_verify_digest_pin_update_target`
# for each candidate update before performing digest verification.
def _verify_digest_pin_update_target(update):
return DigestResolveResult(ok=True, reason=None, resolved_tag=update.target)
# First digest verification fails, second succeeds.
digest_verifier.verify.side_effect = (
DigestCheckResult(ok=False, reason="digest-mismatch"),
DigestCheckResult(ok=True, reason=None),
)
# Run the update logic that performs digest-pin verification.
# This call is expected to:
# 1. Resolve both updates via `_verify_digest_pin_update_target`
# 2. Call `digest_verifier.verify` for each resolved update
result = update_from_wud_digest_pins(
config=config,
stack=stack,
docker_manifest_resolver=docker_manifest_resolver,
digest_verifier=digest_verifier,
verify_digest_pin_update_target=_verify_digest_pin_update_target,
)
# Both updates should have been verified even though the first one failed.
assert digest_verifier.verify.call_count == 2
# Overall digest-pin verification should fail because one verification failed.
assert result.ok is False
```
To integrate this test correctly with the rest of the codebase, you will likely need to:
1. Adjust the signature of `test_digest_pin_verification_checks_all_updates_after_verification_failure` to match the existing fixture setup (e.g. if `docker_manifest_resolver`, `digest_verifier`, or `update_from_wud_digest_pins` are provided via fixtures or imported differently).
2. Import `update_from_wud_digest_pins` at the top of the test module (e.g. `from wudup.python_update_from_wud_digest_pins import update_from_wud_digest_pins`) or adapt the call to the correct function that performs digest-pin verification.
3. If `_verify_digest_pin_update_target` is an internal helper in the production code rather than something passed in, replace the injected `verify_digest_pin_update_target` parameter with the appropriate mocking strategy (e.g. monkeypatching the helper in the target module to always return `DigestResolveResult(ok=True, ...)`).
4. Ensure that `DigestCheckResult` and `DigestResolveResult` are used consistently with how they are defined in `wudup.digest_verifier` (field names and constructor usage); adapt the `reason` / `resolved_tag` attributes if their API differs slightly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+237
to
+246
| def _verify_digest_pin_images( | ||
| self, | ||
| update: DigestPinUpdate, | ||
| images: Sequence[str], | ||
| ) -> tuple[DigestCheckResult | None, bool]: | ||
| matched = False | ||
| digest_result: DigestCheckResult | None = None | ||
| for image in images: | ||
| if not image_matches_resolved_target( | ||
| image, |
There was a problem hiding this comment.
suggestion: Avoid duplicated image-verification loops by consolidating into a single helper
_verify_digest_pin_images duplicates most of _verify_expected_digest_images (iteration over images, image_matches_resolved_target, self.digest_verifier.verify, and early break on digest_result.ok), differing mainly in allow_repo and the digest source. Extract a single helper that takes resolved_image, expected_digest, and allow_repo so both paths share the verification logic and future changes only need to be made in one place.
Suggested implementation:
def _verify_images(
self,
*,
resolved_image: str,
expected_digest: str,
images: Sequence[str],
allow_repo: bool,
) -> tuple[DigestCheckResult | None, bool]:
matched = False
digest_result: DigestCheckResult | None = None
for image in images:
if not image_matches_resolved_target(
image,
resolved_image,
allow_repo=allow_repo,
):
continue
matched = True
digest_result = self.digest_verifier.verify(
image=image,
resolved_image=resolved_image,
expected_digest=expected_digest,
)
if digest_result.ok:
break
return digest_result, matched
def _verify_digest_pin_images(
self,
update: DigestPinUpdate,
images: Sequence[str],
) -> tuple[DigestCheckResult | None, bool]:
return self._verify_images(
resolved_image=update.resolved_image,
expected_digest=update.expected_digest,
images=images,
allow_repo=False,I can only see part of the file, so to complete the refactor you should:
-
Update
_verify_expected_digest_imagesto use the new helper instead of its own loop. For example, replace its internal loop with something along the lines of:digest_result, matched = self._verify_images( resolved_image=update.resolved_image, expected_digest=update.expected_digest, images=images, allow_repo=True, # or False, depending on the original behavior )
and return
digest_result, matchedfrom the function. -
Align the helper call signature with the existing verifier usage:
- If
self.digest_verifier.verifycurrently has a different signature (e.g. positional args instead of named, or additional parameters), update the call inside_verify_imagesto match the existing pattern. - If
allow_repowas previously passed toimage_matches_resolved_targetor used differently in_verify_expected_digest_images/_verify_digest_pin_images, ensure that logic is preserved when wiring through_verify_images.
- If
-
Ensure the expected digest for digest-pin paths is correctly sourced:
- If
DigestPinUpdateuses a different attribute thanexpected_digestfor the digest to verify (e.g.update.pin_digestor similar), adapt theexpected_digest=...argument in_verify_digest_pin_imagesaccordingly.
- If
Once those changes are made, both _verify_expected_digest_images and _verify_digest_pin_images will share the same verification logic via _verify_images, eliminating duplicated loops and centralizing future changes in one place.
Comment on lines
+95
to
+104
| def test_expected_digest_verification_checks_all_requirements(self) -> None: | ||
| stack_dir = self.make_stack( | ||
| "app", | ||
| [ | ||
| ("first", "repo/first:latest", "cid-first"), | ||
| ("second", "repo/second:latest", "cid-second"), | ||
| ], | ||
| ) | ||
| stack = ComposeStack( | ||
| index=1, |
There was a problem hiding this comment.
suggestion (testing): Consider adding coverage for the stale-digest path to ensure _mark_stale_pending_digest is still invoked and later requirements are evaluated.
This test already verifies that _verify_expected_digests continues evaluating requirements after an early mismatch and still returns False. To fully cover the behavior, add a sibling test for the stale-digest branch in _verify_expected_digest_requirement that:
- Sets
digest_verifier.verify.side_effectso the first requirement yields aDigestCheckResultwithreason="stale-digest"andok=False. - Patches
runner.lifecycle._mark_stale_pending_digestand asserts it is called once for the first requirement. - Asserts the second requirement is still verified and the overall return value remains
False.
This will lock in the behavior around stale pending digests described as important in the PR.
Suggested implementation:
runner.lifecycle._expected_digest_failure_reason(stack, matches),
STALE_PENDING_DIGEST_REASON,
)
def test_expected_digest_verification_marks_stale_pending_digest_and_checks_all_requirements(
self,
) -> None:
stack_dir = self.make_stack(
"app",
[
("first", "repo/first:latest", "cid-first"),
("second", "repo/second:latest", "cid-second"),
],
)
stack = ComposeStack(
index=1,
directory=stack_dir,
)
# Create a runner and two requirements mirroring the existing test setup.
runner = self._make_runner(stack) # adjust helper as used in sibling test
first_requirement = mock.Mock(name="first_requirement")
second_requirement = mock.Mock(name="second_requirement")
requirements = [first_requirement, second_requirement]
stale_result = DigestCheckResult(ok=False, reason="stale-digest")
second_result = DigestCheckResult(ok=False, reason="unexpected-digest")
with patch.object(
runner.lifecycle.digest_verifier,
"verify",
side_effect=[stale_result, second_result],
) as verify, patch.object(
runner.lifecycle,
"_mark_stale_pending_digest",
) as mark_stale:
result = runner.lifecycle._verify_expected_digests(stack, requirements)
# The stale-digest requirement should cause the pending digest to be marked as stale.
assert result is False
assert verify.call_count == 2
verify.assert_has_calls(
[
mock.call(stack, first_requirement),
mock.call(stack, second_requirement),
]
)
mark_stale.assert_called_once()
# Ensure we mark stale for the first requirement; adjust arguments to match implementation.
first_mark_call = mark_stale.call_args
assert first_mark_call is not None
def test_expected_digest_verification_checks_all_requirements(self) -> None:- This new test assumes
mockandpatchare available fromunittest.mock. If they are not already imported at the top of this file, add:from unittest import mockfrom unittest.mock import patch
- The helper
self._make_runner(stack)and attributerunner.lifecycle.digest_verifierare inferred from the existing test patterns; align this to whatever the sibling testtest_expected_digest_verification_checks_all_requirementsuses to construct the runner and access thedigest_verifier. - The call signatures for
runner.lifecycle._verify_expected_digestsand_mark_stale_pending_digestare inferred. If_mark_stale_pending_digesttakes specific arguments (e.g.,(stack, requirement)or just(stack,)), update themark_stale.assert_called_once()/assert_has_callsassertions accordingly to match the real signature. - To keep the style consistent, you may want to mirror the exact stack/requirements setup of
test_expected_digest_verification_checks_all_requirements(e.g., how requirements are constructed) rather than usingmock.Mockplaceholders; adjustfirst_requirementandsecond_requirementcreation to match that existing test.
Comment on lines
+180
to
+189
| def test_digest_pin_verification_checks_all_updates_after_failure(self) -> None: | ||
| stack = ComposeStack( | ||
| index=1, | ||
| directory=self.root, | ||
| file="docker-compose.yml", | ||
| name="app", | ||
| images=("docker.io/repo/second:2.0",), | ||
| service_images=( | ||
| ServiceImage("second", "docker.io/repo/second:2.0"), | ||
| ), |
There was a problem hiding this comment.
suggestion (testing): Add a complementary test where the first digest-pin update fails during digest verification or image matching, not only during target resolution.
This test currently covers the case where the first update fails at target resolution. With the new split between resolution and image verification, you should also add a test where the first update resolves successfully but fails during image matching or digest verification (e.g. DigestCheckResult with ok=False, reason="digest-mismatch"), and the second update still resolves and verifies. For example:
- Mock
_verify_digest_pin_update_targetto succeed for both updates. - Set
digest_verifier.verify.side_effectso the first call fails and the second succeeds. - Assert
digest_verifier.verify.call_count == 2and that the overall result isFalse.
This will ensure coverage of both resolution failures and verification/matching failures under the new helper structure.
Suggested implementation:
def test_digest_pin_verification_checks_all_updates_after_failure(self) -> None:
stack = ComposeStack(
index=1,
directory=self.root,
file="docker-compose.yml",
name="app",
images=("docker.io/repo/second:2.0",),
service_images=(
ServiceImage("second", "docker.io/repo/second:2.0"),
),
)
def test_digest_pin_verification_checks_all_updates_after_verification_failure(
self,
docker_manifest_resolver: DockerManifestResolver,
digest_verifier: DigestVerifier,
) -> None:
"""
Ensure that when the first digest-pin update resolves successfully but fails during
digest verification / image matching, we still attempt to verify the remaining updates.
"""
stack = ComposeStack(
index=1,
directory=self.root,
file="docker-compose.yml",
name="app",
images=(
"docker.io/repo/first:1.0",
"docker.io/repo/second:2.0",
),
service_images=(
ServiceImage("first", "docker.io/repo/first:1.0"),
ServiceImage("second", "docker.io/repo/second:2.0"),
),
)
config = load_config(self.root)
# Make target resolution succeed for both digest-pin updates.
# The helper under test is expected to call `_verify_digest_pin_update_target`
# for each candidate update before performing digest verification.
def _verify_digest_pin_update_target(update):
return DigestResolveResult(ok=True, reason=None, resolved_tag=update.target)
# First digest verification fails, second succeeds.
digest_verifier.verify.side_effect = (
DigestCheckResult(ok=False, reason="digest-mismatch"),
DigestCheckResult(ok=True, reason=None),
)
# Run the update logic that performs digest-pin verification.
# This call is expected to:
# 1. Resolve both updates via `_verify_digest_pin_update_target`
# 2. Call `digest_verifier.verify` for each resolved update
result = update_from_wud_digest_pins(
config=config,
stack=stack,
docker_manifest_resolver=docker_manifest_resolver,
digest_verifier=digest_verifier,
verify_digest_pin_update_target=_verify_digest_pin_update_target,
)
# Both updates should have been verified even though the first one failed.
assert digest_verifier.verify.call_count == 2
# Overall digest-pin verification should fail because one verification failed.
assert result.ok is FalseTo integrate this test correctly with the rest of the codebase, you will likely need to:
- Adjust the signature of
test_digest_pin_verification_checks_all_updates_after_verification_failureto match the existing fixture setup (e.g. ifdocker_manifest_resolver,digest_verifier, orupdate_from_wud_digest_pinsare provided via fixtures or imported differently). - Import
update_from_wud_digest_pinsat the top of the test module (e.g.from wudup.python_update_from_wud_digest_pins import update_from_wud_digest_pins) or adapt the call to the correct function that performs digest-pin verification. - If
_verify_digest_pin_update_targetis an internal helper in the production code rather than something passed in, replace the injectedverify_digest_pin_update_targetparameter with the appropriate mocking strategy (e.g. monkeypatching the helper in the target module to always returnDigestResolveResult(ok=True, ...)). - Ensure that
DigestCheckResultandDigestResolveResultare used consistently with how they are defined inwudup.digest_verifier(field names and constructor usage); adapt thereason/resolved_tagattributes if their API differs slightly.
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
1 participant
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
Reduce the cognitive complexity of digest verification without changing updater success/failure decisions, stale pending-digest handling, untrusted verification behavior, or diagnostic output.
AZ60TpT-vxKY-zkovz2k,AZ60TpT-vxKY-zkovz2l.updates.py,init_config.py, andweb_scheduler.pyfindings remain separate module/safety boundaries.Scope
Checklist
README.mdordocs/updated if install, deployment, config, or user-facing behavior changed — N/A, behavior and user-facing output are preservedtemplate.envupdated if environment variables or example config changed — N/A, no configuration changedCHANGELOG.mdupdated in ## [Unreleased] section — N/A, maintainability-only changeTest plan
tests/run-all.sh— N/A, full shell/container suite is outside this Python updater scopeFocused test(s):
.venv/bin/python -m pytest -q tests/test_python_update_from_wud_core.py tests/test_python_update_from_wud_digest_pins.py(37 passed)tests/test-docker-update-from-wud.sh(passed in the intended non-PTY mode).venv/bin/ruff check src/wudup/updater_lifecycle_digest.py tests/test_python_update_from_wud_core.py tests/test_python_update_from_wud_digest_pins.pytests/run-all.sh --python(1312 passed, 1 skipped)git diff --checkDocker or Compose validation, if container behavior changed: N/A, no container packaging behavior changed
An initial PTY invocation of
tests/test-docker-update-from-wud.shwaited at its intentional non-interactive confirmation test and was interrupted; the required non-PTY rerun passed. Sonar local file analysis was attempted for all three changed files, but the MCP analyzer returned “Failed to request analysis of the list of files.”