[codex] add configurable upgrade timing#1548
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| fn release_meets_minimum_age( | ||
| release: &ChannelRelease, | ||
| minimum_package_upgrade_age_seconds: u64, | ||
| now: u64, | ||
| ) -> bool { | ||
| if minimum_package_upgrade_age_seconds == 0 { | ||
| return true; | ||
| } | ||
|
|
||
| release.created_at.is_some_and(|created_at| { | ||
| now.saturating_sub(created_at) >= minimum_package_upgrade_age_seconds | ||
| }) | ||
| } |
There was a problem hiding this comment.
🚩 Missing created_at permanently blocks auto-upgrade when minimum_package_upgrade_age_seconds > 0
When minimum_package_upgrade_age_seconds > 0 is configured but the API does not provide a created_at field, release_meets_minimum_age at src/commands/upgrade.rs:379 returns false (via is_some_and on None). This causes determine_action_at to return ReleaseTooNew, and release_age_eligible_at at src/commands/upgrade.rs:391 returns None (no eligible time). The cache falls back to the regular interval. On every subsequent check, the same thing happens — the client re-fetches, gets ReleaseTooNew again, and never upgrades. This creates an infinite cycle of check → block → wait → check → block. The test test_determine_action_release_missing_created_at_blocks_minimum_age confirms this is intentional, and --force provides an escape hatch. However, this behavior could be surprising if an enterprise deploys minimum_package_upgrade_age_seconds before their releases API starts providing created_at. Consider documenting this requirement or logging a more prominent warning in the daemon path when created_at is missing.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let next_check_after = cache | ||
| .next_check_after | ||
| .filter(|_| cache.upgrade_jitter_seconds == Some(upgrade_jitter_seconds)) | ||
| .unwrap_or_else(|| { | ||
| regular_next_check_after(cache.last_checked_at, channel, upgrade_jitter_seconds) | ||
| }); |
There was a problem hiding this comment.
🟡 Cache invalidation for next_check_after ignores changes to minimum_package_upgrade_age_seconds
When a release is blocked by ReleaseTooNew, next_check_after is set to release_age_eligible_at(...) (e.g., created_at + minimum_package_upgrade_age_seconds), which can be far in the future. should_check_for_updates at src/commands/upgrade.rs:349-354 only invalidates the cached next_check_after when upgrade_jitter_seconds changes (via the .filter(|_| cache.upgrade_jitter_seconds == Some(upgrade_jitter_seconds)) check). If a user subsequently reduces or zeroes out minimum_package_upgrade_age_seconds without also changing upgrade_jitter_seconds, the old age-based next_check_after continues to suppress automatic background update checks until it expires.
For example: minimum_package_upgrade_age_seconds=86400, a release created 1 minute ago → next_check_after ≈ now+86400. User then sets minimum_package_upgrade_age_seconds=0. Automatic checks remain blocked for ~24 hours because upgrade_jitter_seconds didn't change so the filter passes and the stale next_check_after is used. Manual git-ai upgrade works immediately since it bypasses the cache.
Prompt for agents
The should_check_for_updates function at src/commands/upgrade.rs:349-354 invalidates the cached next_check_after only when upgrade_jitter_seconds changes, but the next_check_after value can also be set from release_age_eligible_at (when the action is ReleaseTooNew). If the user changes minimum_package_upgrade_age_seconds without changing upgrade_jitter_seconds, the stale age-based next_check_after is not invalidated.
One approach: also store minimum_package_upgrade_age_seconds in the UpdateCache struct, and add it to the filter condition alongside upgrade_jitter_seconds. This way, changing either setting invalidates the cached next_check_after.
Another approach: differentiate between a jitter-based next_check_after and an age-based next_check_after in the cache, so they can be validated independently.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Validation