fix(login): require weekly reset drift before auto-start - #33
Conversation
ebfc856 to
38c3178
Compare
|
@codex review |
![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: 38c31786b0
ℹ️ 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".
38c3178 to
3d980dc
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d980dc90b
ℹ️ 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".
3d980dc to
dd1b826
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd1b826c56
ℹ️ 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".
| if resets_at < attempt_reset_at { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
When a retryable attempt has already seen a later reset during backoff, this only rejects values older than the original attempt. For example, after failing at reset 10, a backoff scan at reset 12 updates last_observed_reset_at, but a later response at reset 11 still passes here and becomes a new identity, so fluctuating rate-limit data can auto-start a regressed/stale window. Compare new retry identities against the latest observed reset while still allowing the original retry identity.
Useful? React with 👍 / 👎.
| if previous.is_none() || was_active { | ||
| return None; |
There was a problem hiding this comment.
This first-zero guard depends on last_observed_active, but stale dispatch recovery never records active usage: if a process crashes after setting Dispatching and the next scan sees unused=false, begin_weekly_window_attempt closes the old attempt and returns before reaching this branch. The following scan with a larger exact-zero reset then has was_active == false and dispatches on its first zero observation, contrary to the new drift predicate after active usage.
Useful? React with 👍 / 👎.
cc40f37 to
917147a
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 917147a900
ℹ️ 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".
| const MAX_STATE_BYTES: u64 = 4 * 1024; | ||
| const SUPPRESSION_SECONDS: i64 = 7 * 24 * 60 * 60; | ||
| const MAX_FAILURE_COUNT: u8 = 8; | ||
| const STATE_VERSION: u8 = 2; |
There was a problem hiding this comment.
Bumping STATE_VERSION makes every newly written weekly-window-state.json use version 2, while the previous reader rejects anything other than version 1 as incompatible even though the serialized fields did not change here. In a rollback or mixed-version install after a new build scans an account, the old scheduler will treat the account state as unavailable and stop auto-starting until the file is manually removed; keep the on-disk version at 1 or provide a compatibility path before writing v2.
Useful? React with 👍 / 👎.
| fn missing_reset_never_dispatches() { | ||
| let (_home, store, id) = test_store(/*automation_enabled*/ true); | ||
| assert_not_due(&store, &id, usage(UNUSED, None), 0); | ||
| assert_not_due(&store, &id, usage(ACTIVE, None), 1); | ||
| assert_not_due(&store, &id, usage(UNUSED, None), i64::MAX); |
There was a problem hiding this comment.
This test only locks in that resetless weekly usage can never dispatch, which is negative coverage for the resetless eligibility path this commit removed. /workspace/codex/AGENTS.md says Do not add negative tests for logic that was removed, so this should be dropped or replaced with positive coverage of the remaining dated reset-drift behavior.
Useful? React with 👍 / 👎.
917147a to
2bb454b
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
fix(login): require weekly reset drift before auto-start
fix(login): require weekly reset drift before auto-start
1 participant
Summary
Validation
Live evidence boundary
Manual account acknowledgements occurred during observation, so live account activation is not treated as proof. Deterministic state tests own this behavior.