Skip to content

refactor(effects): share retry transitions without weakening lease fencing - #98

Merged
LIghtJUNction merged 1 commit into
mainfrom
refactor/issue-85-notification-retries
Sep 27, 2026
Merged

LIghtJUNction merged 1 commit into
mainfrom
refactor/issue-85-notification-retries

Conversation

@LIghtJUNction

Copy link
Copy Markdown
Member

Refs #85 and #81. Small behavior-preserving refactor of duplicated mail/Webhook retry handling; no umbrella issue closure.

Changes

_retry now owns the transaction, current-state/attempt fence, exponential delay, attempt limit and job-status write. Channel wrappers retain their existing status codes. This removes duplicate transition code rather than moving authority or business operations out of the executor.

Characterization evidence

Before changing production code, added 30 real PostgreSQL tests, all passing. After refactoring, the same 30 tests passed again locally (Python 3.13.5, PostgreSQL 16.15).

The matrix covers both channels, attempts 1–9, bounded exponential delay, eight-attempt exhaustion, exact channel status codes, duplicate late completions, already pending/done/failed/uncertain jobs, a newer execution attempt, and expired running leases. Old workers cannot reschedule a new lease or overwrite an existing terminal decision; an expired execution remains uncertain rather than being automatically requeued.

Preserved boundaries

No migration, signature/ACL/credential changes, sender wire changes, network access or production deployment. _claim, _finish, current-principal authorization and recovery-drill denial retain their original behavior. Full Python 3.15 repository CI and existing Webhook/mail regressions are required before merge. Other #85 transport/Registry/journal/ledger refactors remain separately tracked.

Refs #85 and #81. Preserve per-channel status codes, attempt limits, backoff, lease fencing and uncertain execution semantics. Add PostgreSQL characterization tests proven before and after the refactor.
Copilot AI lite review requested due to automatic review settings September 27, 2026 20:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review ✅ Completed 2026-09-27T20:27:53.483184Z e0d0d62 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

LIghtJUNction commented Sep 27, 2026 •

Copy link
Copy Markdown
Member Author

Follow-up regression run completed locally: python -m pytest -q tests/test_effect_retry_state.py tests/test_webhook_delivery.py tests/test_webhook_domain_events.py — 46 passed in 33.35s, using Python 3.13.5 and real isolated PostgreSQL 16.15. This combines the 30 before/after state-transition characterizations with existing opt-in delivery, current authorization, subscription/revocation and Webhook uncertainty regressions.

A subsequent combined run of all six proposed changes (#93–#98) passed 100 tests in 68.32s, with zero failures: webhook_identity, webhook_delivery, webhook_domain_events, ssh_process_identity, ssh_rss, tui, tui_recovery, registry_schema_ownership, backup_policy, backup_drill, backup_daemon and effect_retry_state. This verifies the combined local patch set rather than only each change in isolation. The JUnit report was generated at /mnt/data/issues81-85-combined.xml in the review environment.

These are targeted Python 3.13 results, not a replacement for each PR's complete Python 3.15 CI. No production deployment was performed; umbrella issues remain open for their missing acceptance items.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants