Add manual provider subscription reminder foundation#1137
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed May 27, 2026, 12:11 PM ET / 16:11 UTC. Summary Reproducibility: yes. for the review finding: source inspection shows every subscription reminder evaluation writes result.state through SettingsStore.updateConfig, which schedules persistence and bumps config revision. Runtime proof of the intended menu/notification behavior is still not present. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance: Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge Security Review findings
Review detailsBest possible solution: Land a narrow version that only persists reminder state when it changes, keeps the manual metadata privacy boundary, and includes redacted built-app proof of the menu and notification path. Do we have a high-confidence way to reproduce the issue? Yes for the review finding: source inspection shows every subscription reminder evaluation writes result.state through SettingsStore.updateConfig, which schedules persistence and bumps config revision. Runtime proof of the intended menu/notification behavior is still not present. Is this the best way to solve the issue? No as proposed: the feature boundary is narrow, but the implementation should avoid unchanged config writes and needs redacted built-app proof before it is the best merge path. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 83ed8e405541. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 111ccbf7d5
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let backfilled = scoped.backfillingResetTimes(from: self.lastKnownResetSnapshots[provider]) | ||
| self.handleQuotaWarningTransitions(provider: provider, snapshot: backfilled) | ||
| self.handleSessionQuotaTransition(provider: provider, snapshot: backfilled) | ||
| self.handleProviderSubscriptionReminders(provider: provider) |
There was a problem hiding this comment.
handleProviderSubscriptionReminders is only called in the successful usage-fetch path here, and it is likewise absent from the failure branches in the token-account refresh paths. That means reminder events never fire when a provider is temporarily failing (offline/auth errors), even though subscriptionSnapshot is local manual config and does not depend on live usage payloads. Users can therefore miss 30/7/3/1-day and expired reminders during outages; run reminder evaluation independently of fetch success (or in both success and failure paths).
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
P2
|
Current head proof (06bde3c), redacted:
Validation on this head:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
1 participant
Narrow, manual-only foundation PR for provider subscription reminders.
Adds:
Does not add: