fix(enphase): delete schedules via POST /<id>/delete, not the DELETE verb - #4429
Merged
Conversation
…verb The schedule cleanup added in the previous commit used DELETE /battery/sites/<site>/schedules/<id>, which the BatteryConfig gateway rejects with "403 Invalid CORS request" - the verb is not allowed on that resource. The API exposes deletion as POST /schedules/<id>/delete, as the integration design notes already recorded. Every attempt failed, so a window Predbat wanted to retire was retried on each cycle and never cleared. A 403 also counts as an auth failure, so each one burned a re-login: six Enphase logins in fifteen minutes in the captured log, against an account that rejects excess sessions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Enphase schedule cleanup by switching schedule deletion from the unsupported DELETE /.../schedules/<id> to the API-supported POST /.../schedules/<id>/delete, preventing persistent 403s and the resulting re-login churn in the Enphase BatteryConfig gateway path.
Changes:
- Update
EnphaseAPI._delete_schedule()to POST to the/deletesub-resource (withallow_empty=Trueto accept 204/no-body success). - Update and extend Enphase API tests to assert the correct delete endpoint/method and prevent regression.
- Bump Predbat version
v8.47.4→v8.47.5.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| apps/predbat/enphase.py | Switch schedule deletion to POST .../schedules/<id>/delete and document why (gateway rejects DELETE). |
| apps/predbat/tests/test_enphase_api.py | Add a focused regression test for the POST delete endpoint; update existing tests to match the new delete behavior. |
| apps/predbat/predbat.py | Version bump to v8.47.5. |
springfall2008
added a commit
that referenced
this pull request
Aug 8, 2026
…surface write failures (#4463) * fix(enphase): retry POST-create on conflict, disambiguate HTTP logs, surface write failures Addresses the three follow-ups from #4461, filed after #4428/#4429 shipped: - The retry-once-and-prune conflict handling from #4428 only covered the PUT (update) path. The POST (create) path - taken whenever no schedule id is cached yet, e.g. right after a disable/re-enable during the plan's Charging<->Exporting oscillation - had no retry at all, so a stray untracked schedule there could 409 nightly forever. Added _create_schedule_with_conflict_retry, mirroring the PUT path: re-read and retry once on conflict, via PUT if the re-read finds an adoptable id or POST again if not. - The generic "HTTP {status} on {path}" failure log couldn't tell a GET, a POST create and a PUT update apart when they shared the same bare /schedules path shape. request_json now logs the method and an optional caller-supplied context (schedule family/action). - Enphase writes go through an HA switch/select bridge that always looks like it "succeeded" regardless of the cloud's response, so predbat.status/binary_sensor.predbat_exporting kept reporting the intended plan with no sign a write never landed. Failed writes now bump the component's error count (visible on components_healthy) and flag a new binary_sensor.predbat_enphase_<site>_schedule_write_ok sensor naming the failing family. The debounce/settle-interval idea floated in the issue is deliberately not implemented here - it's speculative and needs real-account behaviour to design safely. Bumped v8.47.6 -> v8.47.7. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * refactor(enphase): replace per-family write patches with a single reconcile pass The previous commit on this branch (create-path retry, a pending-status debounce, and a bugfix for the debounce interacting badly with activation) kept revealing new problems in different places - and review surfaced a fourth, structural gap: a family being disabled can leave a stale, still-live window that a *different* family's write then collides with, which none of the per-family retry/prune machinery could see across families. That's the signal to stop patching and reconsider the architecture rather than add a fifth fix (systematic-debugging Phase 4.5). Replaces apply_battery_schedule with a single reconcile function, called both periodically (a new 5-minute tier in run(), so a missed/dropped write- switch trigger can't leave the cloud diverged from the plan indefinitely) and from the write switch as before: apply_battery_schedule: up to 3 read-clean-write attempts, short backoff between, then defer to the next periodic call or trigger. _reconcile_once: fresh read, then two phases - delete anything that must not survive (a family being disabled, or an enabled-but-moving family whenever more than one family is changing this pass) *before* any family writes anything, then write/activate what's still needed. The delete-vs-update-in-place split (see the design doc's "Update strategy") is what actually closes the cross-family gap: a new window for one family can otherwise collide with a different family's old, not-yet-updated window even when neither family's *new* windows overlap each other. Counting how many families are changing this pass is enough to decide - no interval- overlap math needed, and DTG/RBD are mutually exclusive by construction so it only has to reason about two families at a time, not three. Removed as no longer needed: _put_schedule_with_conflict_retry / _create_schedule_with_conflict_retry (two near-duplicate per-write retry helpers -> one outer retry), _write_and_activate, the pending-status debounce and its schedule_pending_since tracking, and the on-failure cache invalidation (every attempt re-reads from the cloud, so there is nothing local left to invalidate). Also adds a scripted live test harness (enphase.py --reconcile-sequence), driving a sequence of real schedule changes - including the cross-family case - against a real account using only midnight-05:00 windows and RBD (freeze-export) rather than a real DTG target, so nothing it does can actually charge from or export to the grid. Design: docs/superpowers/specs/2026-08-08-enphase-schedule-reconcile-design.md Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(enphase): track a failed delete during force-recreate cleanup too _cleanup_family only called _note_schedule_write_result on a delete failure when disabling a family, not when clearing it as part of the multi-family force-recreate path (Copilot review on #4463). Verified the specific claim ("dashboard stays ok while the reconcile exhausts retries") does not reproduce - _converge_family runs unconditionally for every family afterward, and since a failed delete leaves the cache untouched, it detects the mismatch and attempts a fallback PUT to the same id, whose outcome is tracked independently. The real, narrower gap: a delete failure that self-heals via that fallback PUT within the same attempt was never counted in count_errors, and cost one unnecessary retry of the whole pass. Now tracked unconditionally - _converge_family's later success still clears it if the fallback lands, same as before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <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
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.
Follow-up to #4428, which shipped the schedule-conflict fix but used the wrong delete endpoint.
Problem
The schedule cleanup in #4428 issued
DELETE /battery/sites/<site>/schedules/<id>. The BatteryConfig gateway does not allow that verb on the resource and rejects it:The API exposes deletion as
POST /schedules/<id>/delete— as the integration design notes indocs/superpowers/specs/2026-07-11-enphase-cloud-integration-design.mdalready recorded. The headers were never the problem:Origin/Refererare set for the battery_config family, and GET/PUT on the same base path succeed in the same session with the same headers. Only the verb differed.In a 15-minute deployed log: 28 delete attempts, all 403, zero successes.
Two consequences:
request_json, so each failed delete also burned a re-login — six Enphase logins in fifteen minutes, against an account that rejects excess sessions. (LOGIN_REUSE_SECONDS = 30bounded it, and no rejection or suspension was reached, but the churn is unnecessary.)What #4428 did fix
Same log, same site: zero
CONFLICTING_SCHEDULE_*errors, down from 272 in 13 hours. The id-pinning and conflict-retry changes are working; only the delete verb was wrong.Changes
_delete_schedulenow POSTs to the/deletesub-resource.test_delete_schedule_posts_to_the_delete_endpoint) that pins the endpoint so this cannot regress silently.Written test-first; the new test failed against the DELETE-verb implementation before the fix.
./run_pre_commitpasses — all hooks green, full quick suite green.Version bumped v8.47.4 → v8.47.5.
🤖 Generated with Claude Code