fix(tasks): preserve schedules during startup initialization - #991
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughTask schedules now distinguish missing configurations from saved configurations, including explicit empty schedules. PostgreSQL persists schedule initialization state, startup resolves defaults atomically, and failures keep automatic runs idle. ChangesTask schedule persistence
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TaskManager
participant TriggerRepository
participant PostgreSQL
TaskManager->>TriggerRepository: Load or create task schedule
TriggerRepository->>PostgreSQL: Lock schedule set and read triggers
PostgreSQL-->>TriggerRepository: Saved schedule or missing schedule
TriggerRepository->>PostgreSQL: Persist defaults when missing
TriggerRepository-->>TaskManager: Return resolved schedule
Suggested reviewers: Merge Risk: ⚪ Minimal · up to Schedule initialization and explicit empty schedules are preserved across restarts, with automatic runs remaining idle when storage fails. No unresolved merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 6 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Quick104
left a comment
There was a problem hiding this comment.
The submitted persistence behavior passed focused manager tests and PostgreSQL race tests. There is one integration issue with current main, described below; I am updating this branch to address it. The broader taskmanager race run also exposes two test-helper races that reproduce on the original base, so they are not findings against this PR.
AI disclosure: review by gpt-6-astra in the OpenAI Codex app, using Codex tools, GitHub CLI, and the repository unslop skill.
|
Fixed and pushed the schedule integration concern in Startup now uses API v2's existing schedule revisions to initialize defaults, preserves an administrator edit made during default resolution, and leaves automatic runs idle after storage failures. The redundant migration is removed. The branch incorporates main, and the admin documentation describes the current API v2 behavior. Validation passed: build, vet, changed-line lint, manager persistence regressions, all taskmanager packages, PostgreSQL repository race tests, contract/fixture gates, migration validation, formatting, and web build/lint plus 598 test files / 4,600 tests. The startup regressions fail on main and pass with the repair. Full local Go testing encountered an unchanged macOS resource-capability failure reproduced on main and policy deadlines that passed on focused retry. Two existing taskmanager test-helper races also reproduce on the original base. Local-path checks passed under Bash 5 after the system Bash crashed. No actionable review findings remain. Project fit is sound: startup and administrator edits share one durable schedule owner and the same concurrency guard. No API shape or Apple, Android, or Jellyfin changes are needed. CI on the reviewed head is still running. Passing that run is the remaining merge condition; the prior head's green checks do not validate this update. AI disclosure: Mergeability: 8/10 — conditional, reviewed head |
Related issue: #999
Startup could overwrite an administrator's schedule edit made while defaults were being resolved. A failed schedule read or write could also activate defaults that were never saved. Current main already retains empty schedules through API v2 schedule revisions; this change completes the startup behavior using that same repository.
Initialize defaults with the existing revision-zero guard. If an administrator has saved a schedule, load it without changing its revision. Consult default providers only for unsaved schedules, and keep automatic runs idle after a storage error. The branch now incorporates current main and requires no additional migration or API shape changes.
Validation:
Automatic scheduling after a storage failure recovers when an administrator saves the schedule or the server restarts after storage recovers. Other running processes still reload schedules on restart. Autoscan settings saves still replace its poll schedule. No Apple, Android, or Jellyfin changes are needed.
The original implementation was tested by the maintainer. This integration update has automated validation; it has not received a new manual deployment test.
AI disclosure:
gpt-6-astrafunctions.exec,exec_command,apply_patch), GitHub CLI, Docker, repository unslop skill, Modern Go Guidelines CLI