refactor(backend): provider-scoped rollout Wave 2 (remediation)#29
Merged
Conversation
Convert the remediation vertical — the first with a real write — per specs/2026-06-29_feature_providerScopedRollout/spec-wave2.md. Behavior-preserving. - VoipMsRemediationRepository owns the client and exposes ensure_max_expiry(), an idempotent set-then-verify write command (no write when already at target, which preserves provider call counts). get_max_expiry() for the read. Provider failures → ProviderLookupError; unknown sub-account → ValueError. - MaxExpiryOutcome domain model; RemediationService maps it → RemediationResult (already-set / updated / verify-failed) and is provider-free for the registration-interval concern. - sync_parental_controls delegates to an INJECTED ParentalControlsService (service→service composition) so remediation doesn't construct a client. That service is unconverted (Wave 3) and raises raw VoIP.ms errors, so the sync path retains a documented transitional catch of VoipMsError/httpx — it clears when parental controls converts. - Router: registration endpoints build the repo-backed service and map ProviderLookupError → identical 502/504; sync endpoint injects the PC service. - Tests rewritten (fake repo + stub pc_service); added sync_parental_controls coverage. Standard migration note updated (remediation done; exception noted). Full suite 707 passed; ruff check + format clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR #29 review should-fix #1: the empty-accounts read-back path collapsed two distinct RemediationResult.detail strings into one, emitting "...max_expiry is still after update" (double space) instead of the original "...could not retrieve sub-account after update" — an observable /api change. - MaxExpiryOutcome.actual is now str | None (None = read-back returned no account); the service maps None → "could not retrieve sub-account after update", a value mismatch → "max_expiry is still {actual} after update". - Restore the service-level empty-verify detail assertion (review #2) and fix the repo test to assert actual is None. - Nit: restore exc_info=True on the router's unreachable/upstream remediation log lines (dropped stack context). Full suite 708 passed; ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
What
Wave 2 of the provider-scoped rollout: converts remediation — the first vertical with a real write, exercising the write/command pattern documented in the standard. Behavior-preserving (no
/apichange).Spec:
specs/2026-06-29_feature_providerScopedRollout/spec-wave2.md(passed Architect+QA persona review and the standards gate).Changes
VoipMsRemediationRepositoryowns the client and exposesensure_max_expiry(sub, target)— an idempotent set-then-verify command. The "already at target → don't write" check lives in the command, which preserves the exact provider call counts.get_max_expiry()handles the read. Provider failures →ProviderLookupError; unknown sub-account →ValueError.MaxExpiryOutcomedomain model;RemediationServicemaps it →RemediationResult(already-set / updated / verify-failed) and is provider-free for the registration-interval concern.RemediationRepositoryABC inprotocols.py.ProviderLookupError→ identical 502/504; the sync endpoint injects the PC service.The one design call:
sync_parental_controlssync_parental_controlsdelegates toParentalControlsService, a Wave 3 giant still coupled to the client. To make remediation provider-free now, it's injected (service→service composition) rather than constructed from a client.Honest scoping: that injected service is unconverted and raises raw
VoipMsError/httpx, so the sync path retains a documented transitional catch of those types — catching onlyProviderLookupErrorwould have silently changed behavior. The registration-interval concern (the actual conversion) is fully provider-free; the residual sync-path import is documented in-code and in the standard's migration note, and clears when parental controls converts in Wave 3.Verification
set_sub_accountnot called when already at target), verify-mismatch/empty → not verified, provider-error translation.RemediationResultoutcomes and — newly — allsync_parental_controlspaths.ruff check+ruff format --checkclean.Next
Wave 3 = parental controls + wizard — which also clears the remediation sync-path transitional exception.
🤖 Generated with Claude Code